[REF] models: make create work in batch

Add a decorator `model_create_multi` to indicate whether a `create` method is
implemented to work in batch.

odoo/addons/base/tests/test_orm.py

@@ -202,6 +202,48 @@ class TestORM(TransactionCase):
         group_user.write({'users': [(3, user.id)]})
         self.assertTrue(user.share)
 
+    def test_create_multi(self):
+        """ create for multiple records """
+        # assumption: 'res.bank' does not override 'create'
+        vals_list = [{'name': name} for name in ('Foo', 'Bar', 'Baz')]
+        vals_list[0]['email'] = 'foo@example.com'
+        for vals in vals_list:
+            record = self.env['res.bank'].create(vals)
+            self.assertEqual(len(record), 1)
+            self.assertEqual(record.name, vals['name'])
+            self.assertEqual(record.email, vals.get('email', False))
+
+        records = self.env['res.bank'].create([])
+        self.assertFalse(records)
+
+        records = self.env['res.bank'].create(vals_list)
+        self.assertEqual(len(records), len(vals_list))
+        for record, vals in pycompat.izip(records, vals_list):
+            self.assertEqual(record.name, vals['name'])
+            self.assertEqual(record.email, vals.get('email', False))
+
+        # create countries and states
+        vals_list = [{
+            'name': 'Foo',
+            'state_ids': [
+                (0, 0, {'name': 'North Foo', 'code': 'NF'}),
+                (0, 0, {'name': 'South Foo', 'code': 'SF'}),
+                (0, 0, {'name': 'West Foo', 'code': 'WF'}),
+                (0, 0, {'name': 'East Foo', 'code': 'EF'}),
+            ],
+        }, {
+            'name': 'Bar',
+            'state_ids': [
+                (0, 0, {'name': 'North Bar', 'code': 'NB'}),
+                (0, 0, {'name': 'South Bar', 'code': 'SB'}),
+            ],
+        }]
+        foo, bar = self.env['res.country'].create(vals_list)
+        self.assertEqual(foo.name, 'Foo')
+        self.assertCountEqual(foo.mapped('state_ids.code'), ['NF', 'SF', 'WF', 'EF'])
+        self.assertEqual(bar.name, 'Bar')
+        self.assertCountEqual(bar.mapped('state_ids.code'), ['NB', 'SB'])
+
 
 class TestInherits(TransactionCase):
     """ test the behavior of the orm for models that use _inherits;

odoo/addons/test_new_api/tests/test_new_fields.py

@@ -291,9 +291,12 @@ class TestFields(common.TransactionCase):
         self.assertEqual(ewan.parent, cath)
         self.assertEqual(ewan.name, "Erwan")
 
-        # write on non-stored inverse field on severals records
-        foo1 = Category.create({'name': 'Foo'})
-        foo2 = Category.create({'name': 'Foo'})
+        # check create/write with several records
+        vals = {'name': 'None', 'display_name': 'Foo'}
+        foo1, foo2 = Category.create([vals, vals])
+        self.assertEqual(foo1.name, 'Foo')
+        self.assertEqual(foo2.name, 'Foo')
+
         (foo1 + foo2).write({'display_name': 'Bar'})
         self.assertEqual(foo1.name, 'Bar')
         self.assertEqual(foo2.name, 'Bar')

odoo/addons/test_performance/tests/test_performance.py

@@ -59,7 +59,7 @@ class TestPerformance(TransactionCase):
     @warmup
     def test_create_base_with_lines(self):
         """ Create records with one2many lines. """
-        with self.assertQueryCount(admin=38, demo=38):
+        with self.assertQueryCount(admin=20, demo=20):
             self.env['test_performance.base'].create({
                 'name': 'X',
                 'line_ids': [(0, 0, {'value': val}) for val in range(10)],

odoo/api.py

@@ -52,7 +52,7 @@ from inspect import currentframe, getargspec
 from pprint import pformat
 from weakref import WeakSet
 
-from decorator import decorator
+from decorator import decorate, decorator
 from werkzeug.local import Local, release_local
 
 from odoo.tools import frozendict, classproperty, StackMap
@@ -316,6 +316,8 @@ def model(method):
 
         Notice that no ``ids`` are passed to the method in the traditional style.
     """
+    if method.__name__ == 'create':
+        return model_create_single(method)
     method._api = 'model'
     return method
 
@@ -419,6 +421,50 @@ def model_cr_context(method):
     return method
 
 
+_create_logger = logging.getLogger(__name__ + '.create')
+
+
+def _model_create_single(create, self, arg):
+    # 'create' expects a dict and returns a record
+    if isinstance(arg, Mapping):
+        return create(self, arg)
+    if len(arg) > 1:
+        _create_logger.debug("%s.create() called with %d dicts", self, len(arg))
+    return self.browse().concat(*(create(self, vals) for vals in arg))
+
+
+def model_create_single(method):
+    """ Decorate a method that takes a dictionary and creates a single record.
+        The method may be called with either a single dict or a list of dicts::
+
+            record = model.create(vals)
+            records = model.create([vals, ...])
+    """
+    wrapper = decorate(method, _model_create_single)
+    wrapper._api = 'model_create'
+    return wrapper
+
+
+def _model_create_multi(create, self, arg):
+    # 'create' expects a list of dicts and returns a recordset
+    if isinstance(arg, Mapping):
+        return create(self, [arg])
+    return create(self, arg)
+
+
+def model_create_multi(method):
+    """ Decorate a method that takes a list of dictionaries and creates multiple
+        records. The method may be called with either a single dict or a list of
+        dicts::
+
+            record = model.create(vals)
+            records = model.create([vals, ...])
+    """
+    wrapper = decorate(method, _model_create_multi)
+    wrapper._api = 'model_create'
+    return wrapper
+
+
 def cr(method):
     """ Decorate a traditional-style method that takes ``cr`` as a parameter.
         Such a method may be called in both record and traditional styles, like::
@@ -665,15 +711,24 @@ def expected(decorator, func):
     return decorator(func) if not hasattr(func, '_api') else func
 
 
-
-def call_kw_model(method, self, args, kwargs):
+def _call_kw_model(method, self, args, kwargs):
     context, args, kwargs = split_context(method, args, kwargs)
     recs = self.with_context(context or {})
     _logger.debug("call %s.%s(%s)", recs, method.__name__, Params(args, kwargs))
     result = method(recs, *args, **kwargs)
     return downgrade(method, result, recs, args, kwargs)
 
-def call_kw_multi(method, self, args, kwargs):
+
+def _call_kw_model_create(method, self, args, kwargs):
+    # special case for method 'create'
+    context, args, kwargs = split_context(method, args, kwargs)
+    recs = self.with_context(context or {})
+    _logger.debug("call %s.%s(%s)", recs, method.__name__, Params(args, kwargs))
+    result = method(recs, *args, **kwargs)
+    return result.id if isinstance(args[0], Mapping) else result.ids
+
+
+def _call_kw_multi(method, self, args, kwargs):
     ids, args = args[0], args[1:]
     context, args, kwargs = split_context(method, args, kwargs)
     recs = self.with_context(context or {}).browse(ids)
@@ -681,13 +736,17 @@ def call_kw_multi(method, self, args, kwargs):
     result = method(recs, *args, **kwargs)
     return downgrade(method, result, recs, args, kwargs)
 
+
 def call_kw(model, name, args, kwargs):
     """ Invoke the given method ``name`` on the recordset ``model``. """
     method = getattr(type(model), name)
-    if getattr(method, '_api', None) == 'model':
-        return call_kw_model(method, model, args, kwargs)
+    api = getattr(method, '_api', None)
+    if api == 'model':
+        return _call_kw_model(method, model, args, kwargs)
+    elif api == 'model_create':
+        return _call_kw_model_create(method, model, args, kwargs)
     else:
-        return call_kw_multi(method, model, args, kwargs)
+        return _call_kw_multi(method, model, args, kwargs)
 
 
 class Environment(Mapping):
@@ -860,14 +919,20 @@ class Environment(Mapping):
         return self[field.model_name].browse(self._protected.get(field, ()))
 
     @contextmanager
-    def protecting(self, fields, records):
-        """ Prevent the invalidation or recomputation of ``fields`` on ``records``. """
+    def protecting(self, what, records=None):
+        """ Prevent the invalidation or recomputation of fields on records.
+            The parameters are either:
+             - ``what`` a collection of fields and ``records`` a recordset, or
+             - ``what`` a collection of pairs ``(fields, records)``.
+        """
         protected = self._protected
         try:
             protected.pushmap()
-            for field in fields:
-                ids = protected.get(field, frozenset())
-                protected[field] = ids.union(records._ids)
+            what = what if records is None else [(what, records)]
+            for fields, records in what:
+                for field in fields:
+                    ids = protected.get(field, frozenset())
+                    protected[field] = ids.union(records._ids)
             yield
         finally:
             protected.popmap()

odoo/fields.py

@@ -1725,16 +1725,19 @@ class Binary(Field):
             if value:
                 # update the existing attachments
                 atts.write({'datas': value})
+                atts_records = records.browse(atts.mapped('res_id'))
                 # create the missing attachments
-                for record in (records - records.browse(atts.mapped('res_id'))):
-                    atts.create({
-                        'name': self.name,
-                        'res_model': record._name,
-                        'res_field': self.name,
-                        'res_id': record.id,
-                        'type': 'binary',
-                        'datas': value,
-                    })
+                if len(atts_records) < len(records):
+                    atts.create([{
+                            'name': self.name,
+                            'res_model': record._name,
+                            'res_field': self.name,
+                            'res_id': record.id,
+                            'type': 'binary',
+                            'datas': value,
+                        }
+                        for record in (records - atts_records)
+                    ])
             else:
                 atts.unlink()
 
@@ -2283,13 +2286,13 @@ class One2many(_RelationalMulti):
     def write(self, records, value, create=False):
         comodel = records.env[self.comodel_name].with_context(**self.context)
         inverse = self.inverse_name
+        vals_list = []                  # vals for lines to create in batch
 
         with records.env.norecompute():
             for act in (value or []):
                 if act[0] == 0:
                     for record in records:
-                        act[2][inverse] = record.id
-                        comodel.create(act[2])
+                        vals_list.append(dict(act[2], **{inverse: record.id}))
                 elif act[0] == 1:
                     comodel.browse(act[1]).write(act[2])
                 elif act[0] == 2:
@@ -2325,6 +2328,10 @@ class One2many(_RelationalMulti):
                     else:
                         comodel.search(domain).write({inverse: False})
 
+            # create lines in batch
+            if vals_list:
+                comodel.create(vals_list)
+
 
 class Many2many(_RelationalMulti):
     """ Many2many field; the value of such a field is the recordset.
@@ -2484,13 +2491,13 @@ class Many2many(_RelationalMulti):
 
         clear = False           # whether the relation should be cleared
         links = {}              # {id: True (link it) or False (unlink it)}
+        vals_list = []          # vals for lines to create in batch
 
         for act in (value or []):
             if not isinstance(act, (list, tuple)) or not act:
                 continue
             if act[0] == 0:
-                for record in records:
-                    links[comodel.create(act[2]).id] = True
+                vals_list.append(act[2])
             elif act[0] == 1:
                 comodel.browse(act[1]).write(act[2])
             elif act[0] == 2:
@@ -2506,6 +2513,11 @@ class Many2many(_RelationalMulti):
                 clear = True
                 links = dict.fromkeys(act[2], True)
 
+        # create lines in batch
+        if vals_list:
+            for line in comodel.create(vals_list):
+                links[line.id] = True
+
         if clear and not create:
             # remove all records for which user has access rights
             clauses, params, tables = comodel.env['ir.rule'].domain_get(comodel._name)

odoo/models.py

@@ -3268,59 +3268,75 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})):
 
         return True
 
-    @api.model
+    @api.model_create_multi
     @api.returns('self', lambda value: value.id)
-    def create(self, vals):
-        """ create(vals) -> record
+    def create(self, vals_list):
+        """ create(vals_list) -> records
 
-        Creates a new record for the model.
+        Creates new records for the model.
 
-        The new record is initialized using the values from ``vals`` and
-        if necessary those from :meth:`~.default_get`.
+        The new records are initialized using the values from the list of dicts
+        ``vals_list``, and if necessary those from :meth:`~.default_get`.
 
-        :param dict vals:
-            values for the model's fields, as a dictionary::
+        :param list vals_list:
+            values for the model's fields, as a list of dictionaries::
 
-                {'field_name': field_value, ...}
+                [{'field_name': field_value, ...}, ...]
+
+            For backward compatibility, ``vals_list`` may be a dictionary.
+            It is treated as a singleton list ``[vals]``, and a single record
+            is returned.
 
             see :meth:`~.write` for details
-        :return: new record created
+
+        :return: the created records
         :raise AccessError: * if user has no create rights on the requested object
                             * if user tries to bypass access rules for create on the requested object
         :raise ValidateError: if user tries to enter invalid value for a field that is not in selection
         :raise UserError: if a loop would be created in a hierarchy of objects a result of the operation (such as setting an object as its own parent)
         """
-        self.check_access_rights('create')
+        if not vals_list:
+            return self.browse()
 
-        # add missing defaults
-        vals = self._add_missing_default_values(vals)
+        self = self.browse()
+        self.check_access_rights('create')
 
         bad_names = {'id', 'parent_path'}
         if self._log_access:
             bad_names.update(LOG_ACCESS_COLUMNS)
+        unknown_names = set()
 
-        # distribute fields into sets for various purposes
-        store_vals = {}
-        inverse_vals = {}
-        inherited_vals = defaultdict(dict)      # {modelname: {fieldname: value}}
-        unknown_names = []
-        inverse_fields = []
-        protected_fields = []
-        for key, val in vals.items():
-            if key in bad_names:
-                continue
-            field = self._fields.get(key)
-            if not field:
-                unknown_names.append(key)
-                continue
-            if field.store:
-                store_vals[key] = val
-            if field.inherited:
-                inherited_vals[field.related_field.model_name][key] = val
-            elif field.inverse:
-                inverse_vals[key] = val
-                inverse_fields.append(field)
-                protected_fields.extend(self._field_computed.get(field, [field]))
+        # classify fields for each record
+        data_list = []
+        inversed_fields = set()
+
+        for vals in vals_list:
+            # add missing defaults
+            vals = self._add_missing_default_values(vals)
+
+            # distribute fields into sets for various purposes
+            data = {}
+            data['stored'] = stored = {}
+            data['inversed'] = inversed = {}
+            data['inherited'] = inherited = defaultdict(dict)
+            data['protected'] = protected = set()
+            for key, val in vals.items():
+                if key in bad_names:
+                    continue
+                field = self._fields.get(key)
+                if not field:
+                    unknown_names.add(key)
+                    continue
+                if field.store:
+                    stored[key] = val
+                if field.inherited:
+                    inherited[field.related_field.model_name][key] = val
+                elif field.inverse:
+                    inversed[key] = val
+                    inversed_fields.add(field)
+                    protected.update(self._field_computed.get(field, [field]))
+
+            data_list.append(data)
 
         if unknown_names:
             _logger.warning("%s.create() with unknown fields: %s",
@@ -3328,123 +3344,163 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})):
 
         # create or update parent records
         for model_name, parent_name in self._inherits.items():
-            parent_record = self.env[model_name].browse(store_vals.get(parent_name))
-            parent_vals = inherited_vals[model_name]
-            if not parent_record:
-                store_vals[parent_name] = parent_record.create(parent_vals).id
-            elif parent_vals:
-                parent_record.write(parent_vals)
+            parent_data_list = []
+            for data in data_list:
+                if not data['stored'].get(parent_name):
+                    parent_data_list.append(data)
+                elif data['inherited'][model_name]:
+                    parent = self.env[model_name].browse(data['stored'][parent_name])
+                    parent.write(data['inherited'][model_name])
 
-        # create record with stored fields
-        record = self._create(store_vals)
+            if parent_data_list:
+                parents = self.env[model_name].create([
+                    data['inherited'][model_name]
+                    for data in parent_data_list
+                ])
+                for parent, data in pycompat.izip(parents, parent_data_list):
+                    data['stored'][parent_name] = parent.id
 
-        with self.env.protecting(protected_fields, record):
-            # put the values of inverse fields in cache, and inverse them
-            record._cache.update(record._convert_to_cache(inverse_vals))
+        # create records with stored fields
+        records = self._create(data_list)
 
+        # determine which fields to protect on which records
+        protected = [(data['protected'], data['record']) for data in data_list]
+        with self.env.protecting(protected):
             # in case several fields use the same inverse method, call it once
-            for _inv, fields in groupby(inverse_fields, attrgetter('inverse')):
-                fields[0].determine_inverse(record)
+            for _inv, fields in groupby(inversed_fields, attrgetter('inverse')):
+                # determine which records to inverse for those fields
+                inv_names = {field.name for field in fields}
+                rec_vals = [
+                    (data['record'], data['inversed'])
+                    for data in data_list
+                    if not inv_names.isdisjoint(data['inversed'])
+                ]
 
-            # trick: no need to mark non-stored fields as modified, thanks to
-            # the transitive closure made over non-stored dependencies
+                # If a field is not stored, its inverse method will probably
+                # write on its dependencies, which will invalidate the field on
+                # all records. We therefore inverse the field record by record.
+                if all(field.store for field in fields):
+                    batches = [rec_vals]
+                else:
+                    batches = [[rec_data] for rec_data in rec_vals]
 
-            # check Python constraints for inversed fields
-            record._validate_fields(set(inverse_vals) - set(store_vals))
+                for batch in batches:
+                    for record, vals in batch:
+                        record._cache.update(record._convert_to_cache(vals))
+                    batch_recs = self.concat(*(record for record, vals in batch))
+                    fields[0].determine_inverse(batch_recs)
 
-            # recompute fields
-            if self.env.recompute and self._context.get('recompute', True):
-                self.recompute()
+                # trick: no need to mark non-stored fields as modified, thanks
+                # to the transitive closure made over non-stored dependencies
 
-        return record
+        # check Python constraints for non-stored inversed fields
+        for data in data_list:
+            data['record']._validate_fields(set(data['inversed']) - set(data['stored']))
+
+        # recompute fields
+        if self.env.recompute and self._context.get('recompute', True):
+            self.recompute()
+
+        return records
 
     @api.model
-    def _create(self, vals):
-        self = self.browse()
+    def _create(self, data_list):
+        """ Create records from the stored field values in ``data_list``. """
+        assert data_list
         cr = self.env.cr
+        quote = '"{}"'.format
 
-        # set boolean fields to False by default (to make search more powerful)
+        # set boolean fields to False by default (avoid NULL in database)
         for name, field in self._fields.items():
             if field.type == 'boolean' and field.store:
-                vals.setdefault(name, False)
+                for data in data_list:
+                    data['stored'].setdefault(name, False)
 
-        # determine SQL values
-        columns = []                    # list of (column_name, format, value)
-        other_fields = []               # list of non-column fields
-        protected_fields = []           # list of fields to not recompute on self
-        translated_names = []           # list of translated field names
-
-        columns.append(('id', "nextval(%s)", self._sequence))
-
-        for name, val in vals.items():
-            field = self._fields[name]
-            assert field.store
-
-            if field.column_type:
-                column_val = field.convert_to_column(val, self, vals)
-                columns.append((name, field.column_format, column_val))
-                if field.translate is True:
-                    translated_names.append(name)
-            else:
-                other_fields.append(field)
-
-            if field.inverse:
-                protected_fields.append(field)
+        # insert rows
+        ids = []                        # ids of created records
+        other_fields = set()            # non-column fields
+        translated_fields = set()       # translated fields
 
+        # column names, formats and values (for common fields)
+        columns0 = [('id', "nextval(%s)", self._sequence)]
         if self._log_access:
-            columns.append(('create_uid', '%s', self._uid))
-            columns.append(('write_uid', '%s', self._uid))
-            columns.append(('create_date', '%s', AsIs("(now() at time zone 'UTC')")))
-            columns.append(('write_date', '%s', AsIs("(now() at time zone 'UTC')")))
+            columns0.append(('create_uid', "%s", self._uid))
+            columns0.append(('create_date', "%s", AsIs("(now() at time zone 'UTC')")))
+            columns0.append(('write_uid', "%s", self._uid))
+            columns0.append(('write_date', "%s", AsIs("(now() at time zone 'UTC')")))
 
-        # insert a row for this record
-        query = """INSERT INTO "%s" (%s) VALUES(%s) RETURNING id""" % (
-                self._table,
-                ', '.join('"%s"' % column[0] for column in columns),
-                ', '.join(column[1] for column in columns),
+        for data in data_list:
+            # determine column values
+            columns = list(columns0)
+            for name, val in sorted(data['stored'].items()):
+                field = self._fields[name]
+                assert field.store
+
+                if field.column_type:
+                    col_val = field.convert_to_column(val, self, data['stored'])
+                    columns.append((name, field.column_format, col_val))
+                    if field.translate is True:
+                        translated_fields.add(field)
+                else:
+                    other_fields.add(field)
+
+            # insert a row with the given columns
+            query = "INSERT INTO {} ({}) VALUES ({}) RETURNING id".format(
+                quote(self._table),
+                ", ".join(quote(name) for name, fmt, val in columns),
+                ", ".join(fmt for name, fmt, val in columns),
             )
-        cr.execute(query, [column[2] for column in columns])
+            params = [val for name, fmt, val in columns]
+            cr.execute(query, params)
+            ids.append(cr.fetchone()[0])
 
-        # from now on, self is the new record
-        self = self.browse(cr.fetchone()[0])
+        # the new records
+        records = self.browse(ids)
+        for data, record in pycompat.izip(data_list, records):
+            data['record'] = record
 
         # update parent_path
-        self._parent_store_create()
+        records._parent_store_create()
 
-        with self.env.protecting(protected_fields, self):
+        protected = [(data['protected'], data['record']) for data in data_list]
+        with self.env.protecting(protected):
             # mark fields to recompute; do this before setting other fields,
             # because the latter can require the value of computed fields, e.g.,
             # a one2many checking constraints on records
-            self.modified(self._fields)
+            records.modified(self._fields)
 
-            # set the value of non-column fields
             if other_fields:
-                # discard default values from context
-                other = self.with_context({
+                # discard default values from context for other fields
+                others = records.with_context({
                     key: val
                     for key, val in self._context.items()
                     if not key.startswith('default_')
                 })
-
                 for field in sorted(other_fields, key=attrgetter('_sequence')):
-                    field.write(other, vals[field.name], create=True)
+                    for other, data in pycompat.izip(others, data_list):
+                        if field.name in data['stored']:
+                            field.write(other, data['stored'][field.name], create=True)
 
                 # mark fields to recompute
-                self.modified(field.name for field in other_fields)
+                records.modified([field.name for field in other_fields])
 
-            # check Python constraints
-            self._validate_fields(vals)
+            # check Python constraints for stored fields
+            records._validate_fields(name for data in data_list for name in data['stored'])
 
-        self.check_access_rule('create')
+        records.check_access_rule('create')
 
         # add translations
         if self.env.lang and self.env.lang != 'en_US':
-            for name in translated_names:
-                tname = "%s,%s" % (self._name, name)
-                val = vals[name]
-                self.env['ir.translation']._set_ids(tname, 'model', self.env.lang, self.ids, val, val)
+            Translations = self.env['ir.translation']
+            for field in translated_fields:
+                tname = "%s,%s" % (field.model_name, field.name)
+                for data in data_list:
+                    if field.name in data['stored']:
+                        record = data['record']
+                        val = data['stored'][field.name]
+                        Translations._set_ids(tname, 'model', self.env.lang, record.ids, val, val)
 
-        return self
+        return records
 
     def _parent_store_create(self):
         """ Set the parent_path field on ``self`` after its creation. """
@@ -3881,6 +3937,7 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})):
                 if old.env.lang and callable(field.translate):
                     # the new value *without lang* must be the old value without lang
                     new_wo_lang[name] = old_wo_lang[name]
+                vals_list = []
                 for vals in Translation.search_read(domain):
                     del vals['id']
                     del vals['source']      # remove source to avoid triggering _set_src
@@ -3892,7 +3949,8 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})):
                             vals['source'] = old_wo_lang[name]
                         # the value should be the new value (given by copy())
                         vals['value'] = new_val
-                    Translation.create(vals)
+                    vals_list.append(vals)
+                Translation.create(vals_list)
 
     @api.multi
     @api.returns('self', lambda value: value.id)