[FIX] models, fields: cache consistency for one2many fields

Issue: after many2one updates, the cache of the corresponding one2many
fields was inconsistent when the latter depends on `active_test`.  One
of the values in cache was updated, while the other was left intact.

We simplify the cache by not making the field depend on context: the
cache value contains all the records in the relation (corresponding to
`active_test=False`).  The value of the field is automatically filtered
by the `active` field when the value is accessed.

This makes it easier to maintain the cache value, guarantees its
consistency, and avoids queries to read the one2many field with
`active_test=False`, after having set it with `active_test=True`.

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

@@ -41,3 +41,4 @@ access_test_new_api_model_parent,access_test_new_api_model_parent,model_test_new
 access_test_new_api_model_child,access_test_new_api_model_child,model_test_new_api_model_child,,1,1,1,1
 access_test_new_api_model_child_nocheck,access_test_new_api_model_child_nocheck,model_test_new_api_model_child_nocheck,,1,1,1,1
 access_test_new_api_display,access_test_new_api_display,model_test_new_api_display,,1,1,1,1
+access_test_new_api_model_active_field,access_test_new_api_model_active_field,model_test_new_api_model_active_field,,1,1,1,1

odoo/addons/test_new_api/models.py

@@ -138,7 +138,7 @@ class Message(models.Model):
 
     @api.constrains('author', 'discussion')
     def _check_author(self):
-        for message in self:
+        for message in self.with_context(active_test=False):
             if message.discussion and message.author not in message.discussion.participants:
                 raise ValidationError(_("Author must be among the discussion participants."))
 
@@ -716,3 +716,12 @@ class Mixin(models.AbstractModel):
 class ExtendedDisplay(models.Model):
     _name = 'test_new_api.display'
     _inherit = ['test_new_api.mixin', 'test_new_api.display']
+
+
+class ModelActiveField(models.Model):
+    _name = 'test_new_api.model_active_field'
+    _description = 'A model with active field'
+
+    active = fields.Boolean(default=True)
+    parent_id = fields.Many2one('test_new_api.model_active_field')
+    children_ids = fields.One2many('test_new_api.model_active_field', 'parent_id')

odoo/addons/test_new_api/tests/test_new_fields.py

@@ -1539,6 +1539,50 @@ class TestX2many(common.TransactionCase):
         record_a.unlink()
         self.assertFalse(record_a.exists())
 
+    def test_12_active_test_one2many(self):
+        Model = self.env['test_new_api.model_active_field']
+
+        parent = Model.create({})
+        self.assertFalse(parent.children_ids)
+
+        # create with implicit active_test=True in context
+        child1, child2 = Model.create([
+            {'parent_id': parent.id, 'active': True},
+            {'parent_id': parent.id, 'active': False},
+        ])
+        act_children = child1
+        all_children = child1 + child2
+        self.assertEqual(parent.children_ids, act_children)
+        self.assertEqual(parent.with_context(active_test=True).children_ids, act_children)
+        self.assertEqual(parent.with_context(active_test=False).children_ids, all_children)
+
+        # create with active_test=False in context
+        child3, child4 = Model.with_context(active_test=False).create([
+            {'parent_id': parent.id, 'active': True},
+            {'parent_id': parent.id, 'active': False},
+        ])
+        act_children = child1 + child3
+        all_children = child1 + child2 + child3 + child4
+        self.assertEqual(parent.children_ids, act_children)
+        self.assertEqual(parent.with_context(active_test=True).children_ids, act_children)
+        self.assertEqual(parent.with_context(active_test=False).children_ids, all_children)
+
+        # replace active children
+        parent.write({'children_ids': [(6, 0, [child1.id])]})
+        act_children = child1
+        all_children = child1 + child2 + child4
+        self.assertEqual(parent.children_ids, act_children)
+        self.assertEqual(parent.with_context(active_test=True).children_ids, act_children)
+        self.assertEqual(parent.with_context(active_test=False).children_ids, all_children)
+
+        # replace all children
+        parent.with_context(active_test=False).write({'children_ids': [(6, 0, [child1.id])]})
+        act_children = child1
+        all_children = child1
+        self.assertEqual(parent.children_ids, act_children)
+        self.assertEqual(parent.with_context(active_test=True).children_ids, act_children)
+        self.assertEqual(parent.with_context(active_test=False).children_ids, all_children)
+
     def test_search_many2many(self):
         """ Tests search on many2many fields. """
         tags = self.env['test_new_api.multi.tag']

odoo/fields.py

@@ -2562,6 +2562,10 @@ class Many2oneReference(Integer):
 class _RelationalMulti(_Relational):
     """ Abstract class for relational fields *2many. """
 
+    # Important: the cache contains the ids of all the records in the relation,
+    # including inactive records.  Inactive records are filtered out by
+    # convert_to_record(), depending on the context.
+
     def _update(self, records, value):
         """ Update the cached value of ``self`` for ``records`` with ``value``,
             and return whether everything is in cache.
@@ -2575,26 +2579,17 @@ class _RelationalMulti(_Relational):
                 return
             records = model.browse(records)
 
-        cache = records.env.cache
         result = True
-        if 'active_test' in (self.depends_context or ()):
-            updates = [
-                (value.sudo().filtered('active'), records.with_context(active_test=True)),
-                (value, records.with_context(active_test=False)),
-            ]
-        else:
-            updates = [(value, records)]
 
-        for value, recs in updates:
-            if not value:
-                continue
-            for record in recs:
+        if value:
+            cache = records.env.cache
+            for record in records:
                 if cache.contains(record, self):
                     val = self.convert_to_cache(record[self.name] | value, record, validate=False)
                     cache.set(record, self, val)
                 else:
                     result = False
-            recs.modified([self.name])
+            records.modified([self.name])
 
         return result
 
@@ -2654,7 +2649,10 @@ class _RelationalMulti(_Relational):
     def convert_to_record(self, value, record):
         # use registry to avoid creating a recordset for the model
         prefetch_ids = IterableGenerator(prefetch_x2many_ids, record, self)
-        return record.pool[self.comodel_name]._browse(record.env, value, prefetch_ids)
+        corecords = record.pool[self.comodel_name]._browse(record.env, value, prefetch_ids)
+        if 'active' in corecords and record.env.context.get('active_test', True):
+            corecords = corecords.filtered('active').with_prefetch(prefetch_ids)
+        return corecords
 
     def convert_to_read(self, value, record, use_name_get=True):
         return value.ids
@@ -2711,9 +2709,6 @@ class _RelationalMulti(_Relational):
                 for arg in self.domain
                 if isinstance(arg, (tuple, list)) and isinstance(arg[0], str)
             )
-        # make self depend on 'active_test' if there is a field 'active' in the comodel
-        if 'active' in model.env[self.comodel_name] and 'active_test' not in (self.depends_context or ()):
-            self.depends_context = (self.depends_context or ()) + ('active_test',)
 
     def create(self, record_values):
         """ Write the value of ``self`` on the given records, which have just
@@ -2824,7 +2819,9 @@ class One2many(_RelationalMulti):
 
     def read(self, records):
         # retrieve the lines in the comodel
-        comodel = records.env[self.comodel_name].with_context(**self.context)
+        context = {'active_test': False}
+        context.update(self.context)
+        comodel = records.env[self.comodel_name].with_context(**context)
         inverse = self.inverse_name
         inverse_field = comodel._fields[inverse]
         get_id = (lambda rec: rec.id) if inverse_field.type == 'many2one' else int
@@ -3184,7 +3181,9 @@ class Many2many(_RelationalMulti):
             reflect(model, '%s_%s_fkey' % (self.relation, self.column2), 'f', None, self._module)
 
     def read(self, records):
-        comodel = records.env[self.comodel_name].with_context(**self.context)
+        context = {'active_test': False}
+        context.update(self.context)
+        comodel = records.env[self.comodel_name].with_context(**context)
         domain = self.get_domain_list(records)
         wquery = comodel._where_calc(domain)
         comodel._apply_ir_rules(wquery, 'read')