diff --git a/odoo/addons/base/models/ir_model.py b/odoo/addons/base/models/ir_model.py index f99a94b8fbf..c9cf5e87ff0 100644 --- a/odoo/addons/base/models/ir_model.py +++ b/odoo/addons/base/models/ir_model.py @@ -779,11 +779,11 @@ class IrModelFields(models.Model): return # remove pending write of this field - # DLE P16: if there are pending towrite of the field we currently try to unlink, pop them out from the towrite queue + # DLE P16: if there are pending updates of the field we currently try to unlink, pop them out from the cache # test `test_unlink_with_dependant` for record in self: - for record_values in self.env.all.towrite[record.model].values(): - record_values.pop(record.name, None) + field = self.pool[record.model]._fields[record.name] + self.env.cache.clear_dirty_field(field) # remove fields from registry, and check that views are not broken fields = [self.env[record.model]._pop_field(record.name) for record in self] domain = expression.OR([('arch_db', 'like', record.name)] for record in self) diff --git a/odoo/addons/base/models/ir_translation.py b/odoo/addons/base/models/ir_translation.py index 70aef57b756..cb74e377eae 100644 --- a/odoo/addons/base/models/ir_translation.py +++ b/odoo/addons/base/models/ir_translation.py @@ -593,7 +593,7 @@ class IrTranslation(models.Model): # When assigning a translation to a field # e.g. email.with_context(lang='fr_FR').label = "bonjour" # and then search on translations for this translation, must flush as the translation has not yet been written in database - if any(self.env[model]._fields[field].translate for model, ids in self.env.all.towrite.items() for record_id, fields in ids.items() for field in fields): + if any(field.translate for field in self.env.cache.get_dirty_fields()): self.env.flush_all() return super(IrTranslation, self)._search(args, offset=offset, limit=limit, order=order, count=count, access_rights_uid=access_rights_uid) diff --git a/odoo/api.py b/odoo/api.py index 4a1dc46d31c..854d597bb26 100644 --- a/odoo/api.py +++ b/odoo/api.py @@ -682,7 +682,7 @@ class Environment(Mapping): :rtype: str """ - return self.context.get('lang') + return self.context.get('lang') or None def clear(self): """ Clear all record caches, and discard all fields to recompute. @@ -720,7 +720,7 @@ class Environment(Mapping): def flush_all(self): """ Flush all pending computations and updates to the database. """ self._recompute_all() - for model_name in list(self.all.towrite): + for model_name in OrderedSet(field.model_name for field in self.cache.get_dirty_fields()): self[model_name].flush_model() def is_protected(self, field, record): @@ -810,6 +810,8 @@ class Environment(Mapping): return self.company.id elif key == 'uid': return (self.uid, self.su) + elif key == 'lang': + return get_context('lang') or None elif key == 'active_test': return get_context('active_test', field.context.get('active_test', True)) else: @@ -844,8 +846,6 @@ class Transaction: self.protected = StackMap() # pending computations {field: ids} self.tocompute = defaultdict(OrderedSet) - # pending updates {model: {id: {field: value}}} - self.towrite = defaultdict(lambda: defaultdict(dict)) def flush(self): """ Flush pending computations and updates in the transaction. """ @@ -860,9 +860,8 @@ class Transaction: def clear(self): """ Clear the caches and pending computations and updates in the translations. """ - self.cache.invalidate() + self.cache.clear() self.tocompute.clear() - self.towrite.clear() def reset(self): """ Reset the transaction. This clears the transaction, and reassigns @@ -882,11 +881,55 @@ EMPTY_DICT = frozendict() class Cache(object): - """ Implementation of the cache of records. """ + """ Implementation of the cache of records. + + For most fields, the cache is simply a mapping from a record and a field to + a value. In the case of context-dependent fields, the mapping also depends + on the environment of the given record. For the sake of performance, the + cache is first partitioned by field, then by record. This makes some + common ORM operations pretty fast, like determining which records have a + value for a given field, or invalidating a given field on all possible + records. + + The cache can also mark some entries as "dirty". Dirty entries essentially + marks values that are different from the database. They represent database + updates that haven't been done yet. Note that dirty entries only make + sense for stored fields. Note also that if a field is dirty on a given + record, and the field is context-dependent, then all the values of the + record for that field are considered dirty. For the sake of consistency, + the values that should be in the database must be in a context where all + the field's context keys are ``None``. + """ + def __init__(self): # {field: {record_id: value}, field: {context_key: {record_id: value}}} self._data = defaultdict(dict) + # {field: set[id]} stores the fields and ids that are changed in the + # cache, but not yet written in the database; their changed values are + # in `_data` + self._dirty = defaultdict(OrderedSet) + + def __repr__(self): + # for debugging: show the cache content and dirty flags as stars + data = {} + for field, field_cache in sorted(self._data.items(), key=lambda item: str(item[0])): + dirty_ids = self._dirty.get(field, ()) + if field_cache and isinstance(next(iter(field_cache)), tuple): + data[field] = { + key: { + Starred(id_) if id_ in dirty_ids else id_: val + for id_, val in key_cache.items() + } + for key, key_cache in field_cache.items() + } + else: + data[field] = { + Starred(id_) if id_ in dirty_ids else id_: val + for id_, val in field_cache.items() + } + return repr(data) + def _get_field_cache(self, model, field): """ Return the field cache of the given field, but not for modifying it. """ field_cache = self._data.get(field, EMPTY_DICT) @@ -915,15 +958,63 @@ class Cache(object): raise CacheMiss(record, field) return default - def set(self, record, field, value): - """ Set the value of ``field`` for ``record``. """ + def set(self, record, field, value, dirty=False, check_dirty=True): + """ Set the value of ``field`` for ``record``. + One can normally make a clean field dirty but not the other way around. + Updating a dirty field without ``dirty=True`` is a programming error and + raises an exception. + + :param dirty: whether ``field`` must be made dirty on ``record`` after + the update + :param check_dirty: whether updating a dirty field without making it + dirty must raise an exception + """ field_cache = self._set_field_cache(record, field) field_cache[record._ids[0]] = value + if not check_dirty: + return + if dirty: + assert field.column_type and field.store and record.id + self._dirty[field].add(record.id) + if record.pool.field_depends_context[field]: + # put the values under conventional context key values {'context_key': None}, + # in order to ease the retrieval of those values to flush them + context_none = dict.fromkeys(record.pool.field_depends_context[field]) + record = record.with_env(record.env(context=context_none)) + field_cache = self._set_field_cache(record, field) + field_cache[record._ids[0]] = value + elif record.id in self._dirty.get(field, ()): + _logger.error("cache.set() removing flag dirty on %s.%s", record, field.name, stack_info=True) - def update(self, records, field, values): - """ Set the values of ``field`` for several ``records``. """ + def update(self, records, field, values, dirty=False, check_dirty=True): + """ Set the values of ``field`` for several ``records``. + One can normally make a clean field dirty but not the other way around. + Updating a dirty field without ``dirty=True`` is a programming error and + raises an exception. + + :param dirty: whether ``field`` must be made dirty on ``record`` after + the update + :param check_dirty: whether updating a dirty field without making it + dirty must raise an exception + """ field_cache = self._set_field_cache(records, field) field_cache.update(zip(records._ids, values)) + if not check_dirty: + return + if dirty: + assert field.column_type and field.store and all(records._ids) + self._dirty[field].update(records._ids) + if records.pool.field_depends_context[field]: + # put the values under conventional context key values {'context_key': None}, + # in order to ease the retrieval of those values to flush them + context_none = dict.fromkeys(records.pool.field_depends_context[field]) + records = records.with_env(records.env(context=context_none)) + field_cache = self._set_field_cache(records, field) + field_cache.update(zip(records._ids, values)) + else: + dirty_ids = self._dirty.get(field) + if dirty_ids and not dirty_ids.isdisjoint(records._ids): + _logger.error("cache.update() removing flag dirty on %s.%s", records, field.name, stack_info=True) def insert_missing(self, records, field, values): """ Set the values of ``field`` for the records in ``records`` that @@ -936,6 +1027,7 @@ class Cache(object): def remove(self, record, field): """ Remove the value of ``field`` for ``record``. """ + assert record.id not in self._dirty.get(field, ()) try: field_cache = self._set_field_cache(record, field) del field_cache[record._ids[0]] @@ -994,8 +1086,48 @@ class Cache(object): if record_id not in field_cache: yield record_id + def get_dirty_fields(self): + """ Return the fields that have dirty records in cache. """ + return self._dirty.keys() + + def has_dirty_fields(self, records, fields=None): + """ Return whether any of the given records has dirty fields. + + :param fields: a collection of fields or ``None``; the value ``None`` is + interpreted as any field on ``records`` + """ + if fields is None: + return any( + not ids.isdisjoint(records._ids) + for field, ids in self._dirty.items() + if field.model_name == records._name + ) + else: + return any( + field in self._dirty and not self._dirty[field].isdisjoint(records._ids) + for field in fields + ) + + def clear_dirty_field(self, field): + """ Make the given field clean on all records, and return the ids of the + formerly dirty records for the field. + """ + return self._dirty.pop(field, ()) + def invalidate(self, spec=None): - """ Invalidate the cache, partially or totally depending on ``spec``. """ + """ Invalidate the cache, partially or totally depending on ``spec``. + + If a field is context-dependent, invalidating it for a given record + actually invalidates all the values of that field on the record. In + other words, the field is invalidated for the record in all + environments. + + This operation is unsafe by default, and must be used with care. + Indeed, invalidating a dirty field on a record may lead to an error, + because doing so drops the value to be written in database. + + spec = [(field, ids), (field, None), ...] + """ if spec is None: self._data.clear() elif spec: @@ -1011,6 +1143,11 @@ class Cache(object): for id_ in ids: field_cache.pop(id_, None) + def clear(self): + """ Invalidate the cache and its dirty flags. """ + self._data.clear() + self._dirty.clear() + def check(self, env): """ Check the consistency of the cache for the given environment. """ depends_context = env.registry.field_depends_context @@ -1018,8 +1155,8 @@ class Cache(object): def process(model, field, field_cache): # ignore new records and records to flush - towrite = env.all.towrite.get(model._name) or {} - ids = [id_ for id_ in field_cache if id_ and field.name not in towrite.get(id_, ())] + dirty_ids = self._dirty.get(field, ()) + ids = [id_ for id_ in field_cache if id_ and id_ not in dirty_ids] if not ids: return @@ -1063,6 +1200,17 @@ class Cache(object): _logger.warning("Invalid cache: %s", pformat(invalids)) +class Starred: + """ Simple helper class to ``repr`` a value with a star suffix. """ + __slots__ = ['value'] + + def __init__(self, value): + self.value = value + + def __repr__(self): + return f"{self.value!r}*" + + # keep those imports here in order to handle cyclic dependencies correctly from odoo import SUPERUSER_ID from odoo.modules.registry import Registry diff --git a/odoo/fields.py b/odoo/fields.py index 0ebb328d140..8a1fe92e5cf 100644 --- a/odoo/fields.py +++ b/odoo/fields.py @@ -1087,16 +1087,8 @@ class Field(MetaField('DummyField', (object,), {})): return records # update the cache - cache.update(records, self, itertools.repeat(cache_value)) - - # update towrite - if self.store: - towrite = records.env.all.towrite[self.model_name] - record = records[:1] - write_value = self.convert_to_write(cache_value, record) - column_value = self.convert_to_column(write_value, record) - for record in records.filtered('id'): - towrite[record.id][self.name] = column_value + dirty = self.store and any(records._ids) + cache.update(records, self, itertools.repeat(cache_value), dirty=dirty) return records @@ -1681,29 +1673,29 @@ class _String(Field): if not records: return records - lang = records.env.lang or None # used in _update_translations below + lang = records.env.lang installed = records.env['res.lang'].get_installed() single_lang = installed[0][0] if len(installed) <= 1 else None # modify the column (source) if single_lang or lang in (None, 'en_US') or callable(self.translate) or not cache_value: - towrite = records.env.all.towrite[self.model_name] - for rid in records._ids: - # cache_value is already in database format - towrite[rid][self.name] = cache_value if self.translate is True and cache_value: tname = f"{self.model_name},{self.name}" records.env['ir.translation']._set_source(tname, records._ids, value) # invalidate the field in all languages because the fallback value # for translations is modified cache.invalidate([(self, records.ids)]) - if single_lang or lang == 'en_US': - # modifying with lang=None also updates the installed language, - # and modifying with a lang also updates for lang=None - others = records.with_context(lang=None if lang else single_lang) - cache.update(others, self, itertools.repeat(cache_value)) - - cache.update(records, self, itertools.repeat(cache_value)) + cache.update(records, self, itertools.repeat(cache_value), dirty=True) + if single_lang and not lang: + # modifying with lang=None also updates the installed language + others = records.with_context(lang=single_lang) + cache.update(others, self, itertools.repeat(cache_value), dirty=True) + else: + # Ignore the dirty flag when updating the value in cache. This is + # necessary for translated fields, when you have to update the + # field's value in a language without making it dirty while the + # field's column value is already dirty. + cache.update(records, self, itertools.repeat(cache_value), check_dirty=False) if callable(self.translate): # the source value of self has been updated, synchronize translated @@ -2222,7 +2214,8 @@ class Binary(Field): except (TypeError): pass cache_value = self.convert_to_cache(value, record) - cache.set(record, self, cache_value) + dirty = self.column_type and self.store and any(records._ids) + cache.set(record, self, cache_value, dirty=dirty) except CacheMiss: pass else: @@ -2363,7 +2356,8 @@ class Image(Binary): super(Image, self).write(records, new_value) cache_value = self.convert_to_cache(value if self.related else new_value, records) - records.env.cache.update(records, self, itertools.repeat(cache_value)) + dirty = self.column_type and self.store and any(records._ids) + records.env.cache.update(records, self, itertools.repeat(cache_value), dirty=dirty) def _image_process(self, value): if self.readonly and not self.max_width and not self.max_height: @@ -2928,14 +2922,8 @@ class Many2one(_Relational): self._remove_inverses(records, cache_value) # update the cache of self - cache.update(records, self, itertools.repeat(cache_value)) - - # update towrite - if self.store: - towrite = records.env.all.towrite[self.model_name] - for record in records.filtered('id'): - # cache_value is already in database format - towrite[record.id][self.name] = cache_value + dirty = self.store and any(records._ids) + cache.update(records, self, itertools.repeat(cache_value), dirty=dirty) # update the cache of one2many fields of new corecord self._update_inverses(records, cache_value) diff --git a/odoo/models.py b/odoo/models.py index c2e5fbaf816..3c7b1d36cb0 100644 --- a/odoo/models.py +++ b/odoo/models.py @@ -5534,7 +5534,8 @@ class BaseModel(metaclass=MetaModel): # convert monetary fields after other columns for correct value rounding for field, value in sorted(field_values, key=lambda item: item[0].write_sequence): - cache.set(self, field, field.convert_to_cache(value, self, validate)) + value = field.convert_to_cache(value, self, validate) + cache.set(self, field, value, check_dirty=False) # set inverse fields on new records in the comodel if field.relational: @@ -5832,17 +5833,8 @@ class BaseModel(metaclass=MetaModel): :param fnames: optional iterable of field names to flush """ self._recompute_recordset(fnames) - towrite = self.env.all.towrite.get(self._name) - if fnames is None: - must_flush = towrite and any(id_ in towrite for id_ in self._ids) - else: - must_flush = towrite and any( - fname in vals - for id_ in self._ids - for vals in towrite.get(id_, ()) - for fname in fnames - ) - if must_flush: + fields_ = None if fnames is None else (self._fields[fname] for fname in fnames) + if self.env.cache.has_dirty_fields(self, fields_): self._flush(fnames) def _flush(self, fnames=None): @@ -5875,13 +5867,32 @@ class BaseModel(metaclass=MetaModel): model_fields[field.related_field.model_name].append(field.related_field) for model_name, fields_ in model_fields.items(): - if any( - field.name in vals - for vals in self.env.all.towrite.get(model_name, {}).values() - for field in fields_ - ): - id_vals = self.env.all.towrite.pop(model_name) - process(self.env[model_name], id_vals) + dirty_fields = self.env.cache.get_dirty_fields() + if any(field in dirty_fields for field in fields_): + # if any field is context-dependent, the values to flush should + # be found with a context where the context keys are all None + context_none = dict.fromkeys( + key + for field in fields_ + for key in self.pool.field_depends_context[field] + ) + model = self.env(context=context_none)[model_name] + id_vals = defaultdict(dict) + for field in model._fields.values(): + ids = self.env.cache.clear_dirty_field(field) + if not ids: + continue + records = model.browse(ids) + values = list(self.env.cache.get_values(records, field)) + assert len(values) == len(records), \ + f"Could not find all values of {field} to flush them\n" \ + f" Context: {self.env.context}\n" \ + f" Cache: {self.env.cache!r}" + for record, value in zip(records, values): + value = field.convert_to_write(value, record) + value = field.convert_to_column(value, record) + id_vals[record.id][field.name] = value + process(model, id_vals) # flush the inverse of one2many fields, too for field in fields: @@ -6209,7 +6220,9 @@ class BaseModel(metaclass=MetaModel): spec = [] for field in fields: spec.append((field, ids)) + # TODO VSC: used to remove the inverse of many_to_one from the cache, though we might not need it anymore for invf in self.pool.field_inverses[field]: + self.env[invf.model_name].flush_model([invf.name]) spec.append((invf, None)) self.env.cache.invalidate(spec)