From 4b1cb41cf7a3f936a1e6d00a2bb6a6a29e82d711 Mon Sep 17 00:00:00 2001 From: Raphael Collet Date: Mon, 23 Sep 2019 13:41:18 +0000 Subject: [PATCH] [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 | 1 + odoo/addons/test_new_api/models.py | 11 ++++- .../test_new_api/tests/test_new_fields.py | 44 +++++++++++++++++++ odoo/fields.py | 37 ++++++++-------- 4 files changed, 73 insertions(+), 20 deletions(-) diff --git a/odoo/addons/test_new_api/ir.model.access.csv b/odoo/addons/test_new_api/ir.model.access.csv index 112b72e991d..c0fdba1ea2d 100644 --- a/odoo/addons/test_new_api/ir.model.access.csv +++ b/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 diff --git a/odoo/addons/test_new_api/models.py b/odoo/addons/test_new_api/models.py index 5e1f8d359bf..d12dddd4b8d 100644 --- a/odoo/addons/test_new_api/models.py +++ b/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') 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 845d5f4e960..7bc2ae547a3 100644 --- a/odoo/addons/test_new_api/tests/test_new_fields.py +++ b/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'] diff --git a/odoo/fields.py b/odoo/fields.py index 1bb934cd70c..78ccb5b2857 100644 --- a/odoo/fields.py +++ b/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')