[FIX] core: field recomputed on more records than expected

The issue occurs when a computed field depends on a many2many field with
a corresponding inverse field on its comodel.  Consider two models like

class User(models.Model):
    _name = _description = 'test_new_api.user'

    group_ids = fields.Many2many('test_new_api.group')
    group_count = fields.Integer(compute='_compute_group_count', store=True)

    @api.depends('group_ids')
    def _compute_group_count(self):
        for user in self:
            user.group_count = len(user.group_ids)

class Group(models.Model):
    _name = _description = 'test_new_api.group'

    user_ids = fields.Many2many('test_new_api.user')

When a user is added to a group with

    group.write({'user_ids': [Command.link(user.id)]})

we expect the field `group_count` to be recomputed on `user` only, but
it is actually triggered on *all* the records in `group.user_ids`.  This
is a real performance issue when there are many records in the relation.

The explanation comes from the fact that
 - the framework considers the field `user_ids` is modified on `group`;
 - the field `group_count` implicitly depends on `group_ids.user_ids`,
   which makes it triggered on the users `u` such that `u.group_ids`
   intersects `group`.

The solution consists in handling the dependencies on inverse many2many
field in the field itself.  The field no longer adds the implicit
dependency on its inverse field in the trigger tree, but instead
determines which records in the comodel are actually impacted by the
relation change in the method field.write().

closes odoo/odoo#111943

X-original-commit: bb3a6e378b5f6b2e14b74ce4efb6d749ad54cb1e
Signed-off-by: Raphael Collet <rco@odoo.com>

odoo/addons/test_new_api/models/test_new_api.py

@@ -1379,6 +1379,24 @@ class ComputeMember(models.Model):
             member.container_id = container.search([('name', '=', member.name)], limit=1)
 
 
+class User(models.Model):
+    _name = _description = 'test_new_api.user'
+
+    group_ids = fields.Many2many('test_new_api.group')
+    group_count = fields.Integer(compute='_compute_group_count', store=True)
+
+    @api.depends('group_ids')
+    def _compute_group_count(self):
+        for user in self:
+            user.group_count = len(user.group_ids)
+
+
+class Group(models.Model):
+    _name = _description = 'test_new_api.group'
+
+    user_ids = fields.Many2many('test_new_api.user')
+
+
 class ComputeEditable(models.Model):
     _name = _description = 'test_new_api.compute_editable'
 

odoo/addons/test_new_api/security/ir.model.access.csv

@@ -76,6 +76,8 @@ access_test_new_api_model_shared_cache_compute_parent,access_test_new_api.model_
 access_test_new_api_model_shared_cache_compute_line,access_test_new_api.model_shared_cache_compute_line,model_test_new_api_model_shared_cache_compute_line,,1,1,1,1
 access_test_new_api_compute_container,access_test_new_api_compute_container,model_test_new_api_compute_container,,1,1,1,1
 access_test_new_api_compute_member,access_test_new_api_compute_member,model_test_new_api_compute_member,,1,1,1,1
+access_test_new_api_user,access_test_new_api_user,model_test_new_api_user,,1,1,1,1
+access_test_new_api_group,access_test_new_api_group,model_test_new_api_group,,1,1,1,1
 access_test_new_api_compute_editable,access_test_new_api_compute_editable,model_test_new_api_compute_editable,,1,1,1,1
 access_test_new_api_compute_editable_line,access_test_new_api_compute_editable_line,model_test_new_api_compute_editable_line,,1,1,1,1
 access_test_new_api_model_constrained_unlinks,access_test_new_api_model_constrained_unlinks,model_test_new_api_model_constrained_unlinks,,1,1,1,1

odoo/addons/test_new_api/tests/test_new_fields.py

@@ -820,6 +820,44 @@ class TestFields(TransactionCaseWithUserDemo):
         records[1].foo = "assign"
         self.env.flush_all()
 
+    def test_17_compute_depends_on_many2many(self):
+        user1, user2, user3 = self.env['test_new_api.user'].create([{}, {}, {}])
+        group = self.env['test_new_api.group'].create({'user_ids': [Command.link(user1.id)]})
+        self.env.flush_all()
+
+        field = type(user1).group_count
+        self.assertFalse(self.env.records_to_compute(field))
+
+        # should mark user2 and user3 to compute only
+        group.write({'user_ids': [Command.link(user1.id), Command.link(user2.id), Command.link(user3.id)]})
+        self.assertEqual(self.env.records_to_compute(field), user2 + user3)
+
+        # should mark user2 to compute only
+        self.env.flush_all()
+        group.write({'user_ids': [Command.unlink(user2.id)]})
+        self.assertEqual(self.env.records_to_compute(field), user2)
+
+        # should mark user2 and user3 to compute only
+        self.env.flush_all()
+        group.write({'user_ids': [Command.set([user1.id, user2.id])]})
+        self.assertEqual(self.env.records_to_compute(field), user2 + user3)
+
+        # should mark user3 to compute only
+        self.env.flush_all()
+        user3.write({'group_ids': [Command.link(group.id)]})
+        self.assertEqual(self.env.records_to_compute(field), user3)
+
+        # similar with new records, but only check recomputation
+        user1 = self.env['test_new_api.user'].new({})
+        user2 = self.env['test_new_api.user'].new({})
+        group = self.env['test_new_api.group'].new({'user_ids': [user1.id]})
+        self.assertEqual(user1.group_count, 1)
+        self.assertEqual(user2.group_count, 0)
+
+        group.user_ids += user2
+        self.assertEqual(user1.group_count, 1)
+        self.assertEqual(user2.group_count, 1)
+
     def test_20_float(self):
         """ test rounding of float fields """
         record = self.env['test_new_api.mixed'].create({})

odoo/fields.py

@@ -804,7 +804,7 @@ class Field(MetaField('DummyField', (object,), {})):
                 if not (field is self and not index):
                     yield tuple(field_seq)
 
-                if field.type in ('one2many', 'many2many'):
+                if field.type == 'one2many':
                     for inv_field in Model.pool.field_inverses[field]:
                         yield tuple(field_seq) + (inv_field,)
 
@@ -4823,6 +4823,9 @@ class Many2many(_RelationalMulti):
         for record in records:
             cache.set(record, self, tuple(new_relation[record.id]))
 
+        # determine the corecords for which the relation has changed
+        modified_corecord_ids = set()
+
         # process pairs to add (beware of duplicates)
         pairs = [(x, y) for x, ys in new_relation.items() for y in ys - old_relation[x]]
         if pairs:
@@ -4838,6 +4841,7 @@ class Many2many(_RelationalMulti):
             y_to_xs = defaultdict(set)
             for x, y in pairs:
                 y_to_xs[y].add(x)
+                modified_corecord_ids.add(y)
             for invf in records.pool.field_inverses[self]:
                 domain = invf.get_domain_list(comodel)
                 valid_ids = set(records.filtered_domain(domain)._ids)
@@ -4858,6 +4862,7 @@ class Many2many(_RelationalMulti):
             y_to_xs = defaultdict(set)
             for x, y in pairs:
                 y_to_xs[y].add(x)
+                modified_corecord_ids.add(y)
 
             if self.store:
                 # express pairs as the union of cartesian products:
@@ -4890,6 +4895,16 @@ class Many2many(_RelationalMulti):
                     except KeyError:
                         pass
 
+        if modified_corecord_ids:
+            # trigger the recomputation of fields that depend on the inverse
+            # fields of self on the modified corecords
+            corecords = comodel.browse(modified_corecord_ids)
+            corecords.modified([
+                invf.name
+                for invf in model.pool.field_inverses[self]
+                if invf.model_name == self.comodel_name
+            ])
+
         return records.filtered(
             lambda record: new_relation[record.id] != old_relation[record.id]
         )
@@ -4949,6 +4964,9 @@ class Many2many(_RelationalMulti):
         for record in records:
             cache.set(record, self, tuple(new_relation[record.id]))
 
+        # determine the corecords for which the relation has changed
+        modified_corecord_ids = set()
+
         # process pairs to add (beware of duplicates)
         pairs = [(x, y) for x, ys in new_relation.items() for y in ys - old_relation[x]]
         if pairs:
@@ -4956,6 +4974,7 @@ class Many2many(_RelationalMulti):
             y_to_xs = defaultdict(set)
             for x, y in pairs:
                 y_to_xs[y].add(x)
+                modified_corecord_ids.add(y)
             for invf in records.pool.field_inverses[self]:
                 domain = invf.get_domain_list(comodel)
                 valid_ids = set(records.filtered_domain(domain)._ids)
@@ -4977,6 +4996,7 @@ class Many2many(_RelationalMulti):
             y_to_xs = defaultdict(set)
             for x, y in pairs:
                 y_to_xs[y].add(x)
+                modified_corecord_ids.add(y)
             for invf in records.pool.field_inverses[self]:
                 for y, xs in y_to_xs.items():
                     corecord = comodel.browse([y])
@@ -4987,6 +5007,16 @@ class Many2many(_RelationalMulti):
                     except KeyError:
                         pass
 
+        if modified_corecord_ids:
+            # trigger the recomputation of fields that depend on the inverse
+            # fields of self on the modified corecords
+            corecords = comodel.browse(modified_corecord_ids)
+            corecords.modified([
+                invf.name
+                for invf in model.pool.field_inverses[self]
+                if invf.model_name == self.comodel_name
+            ])
+
         return records.filtered(
             lambda record: new_relation[record.id] != old_relation[record.id]
         )