diff options
| author | Raphael Collet <rco@odoo.com> | 2018-07-13 14:52:35 +0200 |
|---|---|---|
| committer | Raphael Collet <rco@odoo.com> | 2018-07-24 16:58:14 +0200 |
| commit | b1e83fd7b87fa874816553c2cc99f86b3c9658bf (patch) | |
| tree | 36f76368e9d1d34ccceebaed5433828e5a9031c1 /odoo | |
| parent | 79a2da0be9df7854aec43799ae63b8272a8f1c7e (diff) | |
[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.
Diffstat (limited to 'odoo')
| -rw-r--r-- | odoo/addons/base/tests/test_orm.py | 42 | ||||
| -rw-r--r-- | odoo/addons/test_new_api/tests/test_new_fields.py | 9 | ||||
| -rw-r--r-- | odoo/addons/test_performance/tests/test_performance.py | 2 | ||||
| -rw-r--r-- | odoo/api.py | 89 | ||||
| -rw-r--r-- | odoo/fields.py | 38 | ||||
| -rw-r--r-- | odoo/models.py | 286 |
6 files changed, 323 insertions, 143 deletions
diff --git a/odoo/addons/base/tests/test_orm.py b/odoo/addons/base/tests/test_orm.py index d9a358d3c1e..0094f58677b 100644 --- a/odoo/addons/base/tests/test_orm.py +++ b/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; diff --git a/odoo/addons/test_new_api/tests/test_new_fields.py b/odoo/addons/test_new_api/tests/test_new_fields.py index 1ce7d1a75e2..fc117e7ced3 100644 --- a/odoo/addons/test_new_api/tests/test_new_fields.py +++ b/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') diff --git a/odoo/addons/test_performance/tests/test_performance.py b/odoo/addons/test_performance/tests/test_performance.py index f3ba19a68ae..c592e3c7af9 100644 --- a/odoo/addons/test_performance/tests/test_performance.py +++ b/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)], diff --git a/odoo/api.py b/odoo/api.py index 2d2bae74e1a..688b301cf02 100644 --- a/odoo/api.py +++ b/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() diff --git a/odoo/fields.py b/odoo/fields.py index 3f7b734796d..251956246ba 100644 --- a/odoo/fields.py +++ b/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) diff --git a/odoo/models.py b/odoo/models.py index c720f1cd5d2..0fda0e7ff46 100644 --- a/odoo/models.py +++ b/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 new records for the model. - Creates a new record for the model. + The new records are initialized using the values from the list of dicts + ``vals_list``, and if necessary those from :meth:`~.default_get`. - The new record is initialized using the values from ``vals`` and - if necessary those from :meth:`~.default_get`. + :param list vals_list: + values for the model's fields, as a list of dictionaries:: - :param dict vals: - values for the model's fields, as a dictionary:: + [{'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() + + # 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])) - # 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])) + 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) - - # create record with stored fields - record = self._create(store_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]) + + 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 + + # 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(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']) + ] - 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)) + # 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] - # 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 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) - # trick: no need to mark non-stored fields as modified, thanks to - # the transitive closure made over non-stored dependencies + # trick: no need to mark non-stored fields as modified, thanks + # to the transitive closure made over non-stored dependencies - # check Python constraints for inversed fields - record._validate_fields(set(inverse_vals) - set(store_vals)) + # 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() + # recompute fields + if self.env.recompute and self._context.get('recompute', True): + self.recompute() - return record + 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) - - # 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 + for data in data_list: + data['stored'].setdefault(name, False) - 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')"))) + + 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 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), + # 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) |
