[FIX] core: recompute fields triggered by indirectly modified relational fields

The cache currectly fails to correctly invalidate relational fields that depend
on a non-relational field. Two passes of invalidation are done, to reflect
dependencies on both the old and the new written values. In the first pass
only relational fields are considered, as explained in the comments:

> It is best explained with a simple example: consider two sales orders SO1 and
SO2. The computed total amount on sales orders indirectly depends on the
many2one field 'order_id' linking lines to their sales order.  Now consider the
following code:
>
> line = so1.line_ids[0]      # pick a line from SO1
> line.order_id = so2         # move the line to SO2
>
> In this situation, the total amount must be recomputed on *both* sales order:
the line's order before the modification, and the line's order after the
modification.

The written values can be seen as the roots of a dependency forest (a
collection of dependency trees). Before this commit all non-relational roots
and their corresponding trees were filtered out during the first pass. However,
this approach is wrong, as relational fields can also depend on non-relational
fields. Instead, the complete dependency forest has to be traversed, skipping
invalidation for non-relational fields during the first pass.

The test that was previously included accidentally succeeded because of a
separate and unrelated bug in the orm domain parser: in certain one2many or
many2many leafs the domain parser would not take into consideration the domain
included in the definition of the field. As a result, the test still passed
by accident, because the records that no longer matched the domain after the
write were still invalidated during the second pass.

The problem can clearly be demonstrated, however, when the dependency is
generated by a compute function.

closes odoo/odoo#101038

X-original-commit: d4a5827b42d80f0f830455dcd2056701eb09aed1
Signed-off-by: Rémy Voet <ryv@odoo.com>
Signed-off-by: Raphael Collet <rco@odoo.com>
Co-authored-by: Raphael Collet <rco@odoo.com>

odoo/addons/test_new_api/models/test_new_api.py

@@ -1351,6 +1351,12 @@ class ComputeContainer(models.Model):
 
     name = fields.Char()
     member_ids = fields.One2many('test_new_api.compute.member', 'container_id')
+    member_count = fields.Integer(compute='_compute_member_count', store=True)
+
+    @api.depends('member_ids')
+    def _compute_member_count(self):
+        for record in self:
+            record.member_count = len(record.member_ids)
 
 
 class ComputeMember(models.Model):

odoo/addons/test_new_api/tests/test_one2many.py

@@ -256,6 +256,18 @@ class One2manyCase(TransactionCase):
         # at this point, member.container_id must be computed for member to
         # appear in container.member_ids
         self.assertEqual(container.member_ids, member)
+        self.assertEqual(container.member_count, 1)
+
+        # Changing member.name will trigger recomputing member.container_id,
+        # container.member_ids and container.member_count. Since we are setting
+        # the name to bar, it will be detached from container, resulting in a
+        # member_count of zero on the container.
+        member.name = 'Bar'
+        self.assertEqual(container.member_count, 0)
+
+        # Reattach member to container again
+        member.name = 'Foo'
+        self.assertEqual(container.member_count, 1)
 
     def test_reward_line_delete(self):
         order = self.env['test_new_api.order'].create({

odoo/models.py

@@ -4134,7 +4134,7 @@ class BaseModel(metaclass=MetaModel):
 
         field_values = []                           # [(field, value)]
         determine_inverses = defaultdict(list)      # {inverse: fields}
-        relational_names = []
+        fnames_modifying_relations = []
         protected = set()
         check_company = False
         for fname, value in vals.items():
@@ -4152,8 +4152,8 @@ class BaseModel(metaclass=MetaModel):
                     # order to avoid an inconsistent update.
                     self[fname]
                 determine_inverses[field.inverse].append(field)
-            if field.relational or self.pool.field_inverses[field]:
-                relational_names.append(fname)
+            if field in self.pool.fields_modifying_relations:
+                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'):
                     # Protect the field from being recomputed while being
@@ -4197,7 +4197,7 @@ class BaseModel(metaclass=MetaModel):
             # In this situation, the total amount must be recomputed on *both*
             # sales order: the line's order before the modification, and the
             # line's order after the modification.
-            self.modified(relational_names, before=True)
+            self.modified(fnames_modifying_relations, before=True)
 
             real_recs = self.filtered('id')
 
@@ -6649,7 +6649,8 @@ class BaseModel(metaclass=MetaModel):
     # Generic onchange method
     #
 
-    def _dependent_fields(self, field):
+    @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():
@@ -6657,7 +6658,7 @@ class BaseModel(metaclass=MetaModel):
                     yield from val
                 else:
                     yield from traverse(val)
-        return traverse(self.pool.field_triggers.get(field, {}))
+        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

odoo/modules/registry.py

@@ -367,6 +367,30 @@ 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 post_init(self, func, *args, **kwargs):
         """ Register a function to call at the end of :meth:`~.init_models`. """
         self._post_init_queue.append(partial(func, *args, **kwargs))