diff --git a/odoo/addons/base/models/ir_model.py b/odoo/addons/base/models/ir_model.py index f2be141cbb6..a91b605f3be 100644 --- a/odoo/addons/base/models/ir_model.py +++ b/odoo/addons/base/models/ir_model.py @@ -775,7 +775,7 @@ class IrModelFields(models.Model): else: # field hasn't been loaded (yet?) continue - for dep in model._dependent_fields(field): + for dep in self.pool.get_dependent_fields(field): if dep.manual: failed_dependencies.append((field, dep)) for inverse in model.pool.field_inverses[field]: @@ -846,23 +846,7 @@ class IrModelFields(models.Model): # clean the registry from the fields to remove self.pool.registry_invalidated = True - - # discard the removed fields from field triggers - def discard_fields(tree): - # discard fields from the tree's root node - tree.get(None, set()).difference_update(fields) - # discard subtrees labelled with any of the fields - for field in fields: - tree.pop(field, None) - # discard fields from remaining subtrees - for field, subtree in tree.items(): - if field is not None: - discard_fields(subtree) - - discard_fields(self.pool.field_triggers) - - # discard the removed fields from field inverses - self.pool.field_inverses.discard_keys_and_values(fields) + self.pool._discard_fields(fields) # discard the removed fields from fields to compute for field in fields: 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 a7d2bff6677..cf3efe50040 100644 --- a/odoo/addons/test_new_api/tests/test_new_fields.py +++ b/odoo/addons/test_new_api/tests/test_new_fields.py @@ -212,25 +212,25 @@ class TestFields(TransactionCaseWithUserDemo): } ) fields = self.env["test_new_api.foo"]._fields - triggers = self.env.registry.field_triggers + get_trigger_tree = self.registry.get_trigger_tree value1 = fields["value1"] valid_depends = fields["x_computed_custom_valid_depends"] valid_transitive_depends = fields["x_computed_custom_valid_transitive_depends"] invalid_depends = fields["x_computed_custom_invalid_depends"] invalid_transitive_depends = fields["x_computed_custom_invalid_transitive_depends"] # `x_computed_custom_valid_depends` in the triggers of the field `value1` - self.assertTrue(valid_depends in triggers[value1][None]) + self.assertTrue(valid_depends in get_trigger_tree([value1])[None]) # `x_computed_custom_valid_transitive_depends` in the triggers `x_computed_custom_valid_depends` and `value1` - self.assertTrue(valid_transitive_depends in triggers[valid_depends][None]) - self.assertTrue(valid_transitive_depends in triggers[value1][None]) + self.assertTrue(valid_transitive_depends in get_trigger_tree([valid_depends])[None]) + self.assertTrue(valid_transitive_depends in get_trigger_tree([value1])[None]) # `x_computed_custom_invalid_depends` not in any triggers, as it was invalid and was skipped self.assertEqual( - sum(invalid_depends in field_triggers.get(None, []) for field_triggers in triggers.values()), 0 + sum(invalid_depends in get_trigger_tree([field]).get(None, ()) for field in fields.values()), 0 ) # `x_computed_custom_invalid_transitive_depends` in the triggers of `x_computed_custom_invalid_depends` only - self.assertTrue(invalid_transitive_depends in triggers[invalid_depends][None]) + self.assertTrue(invalid_transitive_depends in get_trigger_tree([invalid_depends])[None]) self.assertEqual( - sum(invalid_transitive_depends in field_triggers.get(None, []) for field_triggers in triggers.values()), 1 + sum(invalid_transitive_depends in get_trigger_tree([field]).get(None, ()) for field in fields.values()), 1 ) @mute_logger('odoo.fields') @@ -4003,7 +4003,7 @@ class TestPrecomputeModel(common.TransactionCase): self.patch(Model.upper, 'precompute', False) with self.assertWarns(UserWarning): self.registry.setup_models(self.cr) - self.registry.field_triggers + self.registry.get_trigger_tree(Model._fields.values()) def test_precompute_dependencies_many2one(self): Model = self.registry['test_new_api.precompute'] @@ -4027,7 +4027,7 @@ class TestPrecomputeModel(common.TransactionCase): self.patch(Line.size, 'precompute', False) with self.assertWarns(UserWarning): self.registry.setup_models(self.cr) - self.registry.field_triggers + self.registry.get_trigger_tree(Model._fields.values()) class TestPrecompute(common.TransactionCase): diff --git a/odoo/models.py b/odoo/models.py index bfcb35e8378..17d9890ab67 100644 --- a/odoo/models.py +++ b/odoo/models.py @@ -132,34 +132,6 @@ def fix_import_export_id_paths(fieldname): fixed_external_id = re.sub(r'([^/]):id', r'\1/id', fixed_db_id) return fixed_external_id.split('/') -def merge_trigger_trees(trees: list, select=bool) -> dict: - """ Merge trigger trees list into a final tree. The function ``select`` is - called on every field to determine which fields should be kept in the tree - nodes. This enables to discard some fields from the tree nodes. - """ - result_tree = {} # the resulting tree - root_fields = OrderedSet() # the fields in the root node - subtrees_to_merge = defaultdict(list) # the subtrees to merge grouped by key - - for tree in trees: - for key, val in tree.items(): - if key is None: - root_fields.update(val) - else: - subtrees_to_merge[key].append(val) - - # the root node contains the collected fields for which select is true - root_node = [field for field in root_fields if select(field)] - if root_node: - result_tree[None] = root_node - - for key, subtrees in subtrees_to_merge.items(): - subtree = merge_trigger_trees(subtrees, select) - if subtree: - result_tree[key] = subtree - - return result_tree - class MetaModel(api.Meta): """ The metaclass of all model classes. @@ -3680,7 +3652,7 @@ class BaseModel(metaclass=MetaModel): # order to avoid an inconsistent update. self[fname] determine_inverses[field.inverse].append(field) - if field in self.pool.fields_modifying_relations: + if self.pool.is_modifying_relations(field): fnames_modifying_relations.append(fname) if field.inverse or (field.compute and not field.readonly): if field.store or field.type not in ('one2many', 'many2many'): @@ -5995,61 +5967,55 @@ class BaseModel(metaclass=MetaModel): # - mark H to recompute on inverse(X, records), # - mark I to recompute on inverse(W, inverse(X, records)), # - mark J to recompute on inverse(Y, records). - fields = [self._fields[fname] for fname in fnames] - field_triggers = self.pool.field_triggers - trees = [field_triggers[field] for field in fields if field in field_triggers] - - if not trees: - return + # The fields' trigger trees are merged in order to evaluate all triggers + # at once. For non-stored computed fields, `_modified_triggers` might + # traverse the tree (at the cost of extra queries) only to know which + # records to invalidate in cache. But in many cases, most of these + # fields have no data in cache, so they can be ignored from the start. + # This allows us to discard subtrees from the merged tree when they + # only contain such fields. cache = self.env.cache - - # Merge dependency trees to evaluate all triggers at once. - # For non-stored computed fields, `_modified_triggers` might traverse - # the tree (at the cost of extra queries) only to know which records to - # invalidate in cache. But in many cases, most of these fields have no - # data in cache, so they can be ignored from the start. This allows us - # to discard subtrees from the merged tree when they only contain such - # fields. - tree = merge_trigger_trees( - trees, + tree = self.pool.get_trigger_tree( + [self._fields[fname] for fname in fnames], select=lambda field: (field.compute and field.store) or cache.contains_field(field), ) + if not tree: + return - if tree: - # determine what to compute (through an iterator) - tocompute = self.sudo().with_context(active_test=False)._modified_triggers(tree, create) + # determine what to compute (through an iterator) + tocompute = self.sudo().with_context(active_test=False)._modified_triggers(tree, create) - # When called after modification, one should traverse backwards - # dependencies by taking into account all fields already known to be - # recomputed. In that case, we mark fieds to compute as soon as - # possible. - # - # When called before modification, one should mark fields to compute - # after having inversed all dependencies. This is because we - # determine what currently depends on self, and it should not be - # recomputed before the modification! - if before: - tocompute = list(tocompute) + # When called after modification, one should traverse backwards + # dependencies by taking into account all fields already known to be + # recomputed. In that case, we mark fieds to compute as soon as + # possible. + # + # When called before modification, one should mark fields to compute + # after having inversed all dependencies. This is because we + # determine what currently depends on self, and it should not be + # recomputed before the modification! + if before: + tocompute = list(tocompute) - # process what to compute - for field, records, create in tocompute: - records -= self.env.protected(field) - if not records: - continue - if field.compute and field.store: - if field.recursive: - recursively_marked = self.env.not_to_compute(field, records) - self.env.add_to_compute(field, records) - else: - # Don't force the recomputation of compute fields which are - # not stored as this is not really necessary. - if field.recursive: - recursively_marked = records & self.env.cache.get_records(records, field) - self.env.cache.invalidate([(field, records._ids)]) - # recursively trigger recomputation of field's dependents + # process what to compute + for field, records, create in tocompute: + records -= self.env.protected(field) + if not records: + continue + if field.compute and field.store: if field.recursive: - recursively_marked.modified([field.name], create) + recursively_marked = self.env.not_to_compute(field, records) + self.env.add_to_compute(field, records) + else: + # Don't force the recomputation of compute fields which are + # not stored as this is not really necessary. + if field.recursive: + recursively_marked = records & self.env.cache.get_records(records, field) + self.env.cache.invalidate([(field, records._ids)]) + # recursively trigger recomputation of field's dependents + if field.recursive: + recursively_marked.modified([field.name], create) def _modified_triggers(self, tree, create=False): """ Return an iterator traversing a tree of field triggers on ``self``, @@ -6064,49 +6030,51 @@ class BaseModel(metaclass=MetaModel): yield field, self, create # then traverse dependencies backwards, and proceed recursively - for key, val in tree.items(): - if key is None: + for field, subtree in tree.items(): + if field is None: continue - elif create and key.type in ('many2one', 'many2one_reference'): + + if create and field.type in ('many2one', 'many2one_reference'): # upon creation, no other record has a reference to self continue - else: - # val is another tree of dependencies - model = self.env[key.model_name] - for invf in model.pool.field_inverses[key]: - # use an inverse of field without domain - if not (invf.type in ('one2many', 'many2many') and invf.domain): - if invf.type == 'many2one_reference': - rec_ids = OrderedSet() - for rec in self: - try: - if rec[invf.model_field] == key.model_name: - rec_ids.add(rec[invf.name]) - except MissingError: - continue - records = model.browse(rec_ids) - else: - try: - records = self[invf.name] - except MissingError: - records = self.exists()[invf.name] - # TODO: find a better fix - if key.model_name == records._name: - if not any(self._ids): - # if self are new, records should be new as well - records = records.browse(it and NewId(it) for it in records._ids) - break - else: - new_records = self.filtered(lambda r: not r.id) - real_records = self - new_records - records = model.browse() - if real_records: - records = model.search([(key.name, 'in', real_records.ids)], order='id') - if new_records: - cache_records = self.env.cache.get_records(model, key) - records |= cache_records.filtered(lambda r: set(r[key.name]._ids) & set(self._ids)) - yield from records._modified_triggers(val) + # subtree is another tree of dependencies + model = self.env[field.model_name] + for invf in model.pool.field_inverses[field]: + # use an inverse of field without domain + if not (invf.type in ('one2many', 'many2many') and invf.domain): + if invf.type == 'many2one_reference': + rec_ids = OrderedSet() + for rec in self: + try: + if rec[invf.model_field] == field.model_name: + rec_ids.add(rec[invf.name]) + except MissingError: + continue + records = model.browse(rec_ids) + else: + try: + records = self[invf.name] + except MissingError: + records = self.exists()[invf.name] + + # TODO: find a better fix + if field.model_name == records._name: + if not any(self._ids): + # if self are new, records should be new as well + records = records.browse(it and NewId(it) for it in records._ids) + break + else: + new_records = self.filtered(lambda r: not r.id) + real_records = self - new_records + records = model.browse() + if real_records: + records = model.search([(field.name, 'in', real_records.ids)], order='id') + if new_records: + cache_records = self.env.cache.get_records(model, field) + records |= cache_records.filtered(lambda r: set(r[field.name]._ids) & set(self._ids)) + + yield from records._modified_triggers(subtree) @api.model def recompute(self, fnames=None, records=None): @@ -6185,23 +6153,13 @@ class BaseModel(metaclass=MetaModel): # Generic onchange method # - @classmethod - def _dependent_fields(cls, field): - """ Return an iterator on the fields that depend on ``field``. """ - def traverse(node): - for key, val in node.items(): - if key is None: - yield from val - else: - yield from traverse(val) - return traverse(cls.pool.field_triggers.get(field, {})) - def _has_onchange(self, field, other_fields): """ Return whether ``field`` should trigger an onchange event in the presence of ``other_fields``. """ return (field.name in self._onchange_methods) or any( - dep in other_fields for dep in self._dependent_fields(field.base_field) + dep in other_fields + for dep in self.pool.get_dependent_fields(field.base_field) ) def _onchange_eval(self, field_name, onchange, result): diff --git a/odoo/modules/registry.py b/odoo/modules/registry.py index dea7f3a6f76..6b274169fb1 100644 --- a/odoo/modules/registry.py +++ b/odoo/modules/registry.py @@ -141,6 +141,9 @@ class Registry(Mapping): self.field_depends_context = Collector() self.field_inverses = Collector() + # cache of method is_modifying_relations() + self._is_modifying_relations = {} + # Inter-process signaling: # The `base_registry_signaling` sequence indicates the whole registry # must be reloaded. @@ -232,6 +235,7 @@ class Registry(Mapping): self.__cache.clear() lazy_property.reset_all(self) + self._is_modifying_relations.clear() # Instantiate registered classes (via the MetaModel automatic discovery # or via explicit constructor call), and add them to the pool. @@ -260,6 +264,7 @@ class Registry(Mapping): self.__cache.clear() lazy_property.reset_all(self) + self._is_modifying_relations.clear() self.registry_invalidated = True # we must setup ir.model before adding manual fields because _add_manual_models may @@ -325,6 +330,49 @@ class Registry(Mapping): model_name, ", ".join(field.name for field in fields)) return computed + def get_trigger_tree(self, fields: list, select=bool): + """ Return the trigger tree to traverse when ``fields`` have been modified. + The function ``select`` is called on every field to determine which fields + should be kept in the tree nodes. This enables to discard some unnecessary + fields from the tree nodes. + """ + field_triggers = self.field_triggers + trees = [field_triggers[field] for field in fields if field in field_triggers] + if not trees: + return {} + return merge_trigger_trees(trees, select) + + def get_dependent_fields(self, field): + """ Return an iterator on the fields that depend on ``field``. """ + def traverse(tree): + for key, val in tree.items(): + if key is None: + yield from val + else: + yield from traverse(val) + return traverse(self.field_triggers.get(field) or {}) + + def _discard_fields(self, fields: list): + """ Discard the given fields from the registry's internal data structures. """ + + # discard fields from field triggers + def discard(tree): + # discard fields from the tree's root node + tree.get(None, set()).difference_update(fields) + # discard subtrees labelled with any of the fields + for field in fields: + tree.pop(field, None) + # discard fields from remaining subtrees + for field, subtree in tree.items(): + if field is not None: + discard(subtree) + + discard(self.field_triggers) + self._is_modifying_relations.clear() + + # discard fields from field inverses + self.field_inverses.discard_keys_and_values(fields) + @lazy_property def field_triggers(self): # determine field dependencies @@ -367,29 +415,21 @@ class Registry(Mapping): return triggers - @lazy_property - def fields_modifying_relations(self): - ''' - Return the union of the set of relational fields that are dependencies - of other fields, with the set of non-relational fields that are - dependencies of relational fields. - ''' - result = set() - - for field in self.field_triggers: - # If the field is itself a relational field, it is also considered - # as triggering relational fields. - if field.relational or self.field_inverses[field]: - result.add(field) - continue - - Model = self.models[field.model_name] - for dep in Model._dependent_fields(field): - if dep.relational or self.field_inverses[dep]: - result.add(field) - break - - return result + def is_modifying_relations(self, field): + """ Return whether ``field`` has dependent fields on some records, and + that modifying ``field`` might change the dependent records. + """ + try: + return self._is_modifying_relations[field] + except KeyError: + result = field in self.field_triggers and ( + field.relational or self.field_inverses[field] or any( + dep.relational or self.field_inverses[dep] + for dep in self.get_dependent_fields(field) + ) + ) + self._is_modifying_relations[field] = result + return result def post_init(self, func, *args, **kwargs): """ Register a function to call at the end of :meth:`~.init_models`. """ @@ -789,3 +829,32 @@ class DummyRLock(object): self.acquire() def __exit__(self, type, value, traceback): self.release() + + +def merge_trigger_trees(trees: list, select=bool) -> dict: + """ Merge trigger trees list into a final tree. The function ``select`` is + called on every field to determine which fields should be kept in the tree + nodes. This enables to discard some fields from the tree nodes. + """ + result_tree = {} # the resulting tree + root_fields = OrderedSet() # the fields in the root node + subtrees_to_merge = defaultdict(list) # the subtrees to merge grouped by key + + for tree in trees: + for key, val in tree.items(): + if key is None: + root_fields.update(val) + else: + subtrees_to_merge[key].append(val) + + # the root node contains the collected fields for which select is true + root_node = [field for field in root_fields if select(field)] + if root_node: + result_tree[None] = root_node + + for key, subtrees in subtrees_to_merge.items(): + subtree = merge_trigger_trees(subtrees, select) + if subtree: + result_tree[key] = subtree + + return result_tree