From ff937770995a5a2e487b5150372eaf1219cb2d47 Mon Sep 17 00:00:00 2001 From: Raphael Collet Date: Mon, 20 Jun 2016 17:04:17 +0200 Subject: [PATCH] [FIX] models: inverse several computed fields based on a common computed field Consider fields G1 and G2 being computed fields with inverse methods that require the value of a common dependency F, which is also a computed field. For inversing G1 and G2, their values are put in cache, then their inverse method is called. If the inverse method of G1 requires the computation of F, setting F's value will invalidate both G1 and G2 in cache. Hence the value of G2 is lost when invoking its inverse method! The fix consists in marking G1 and G2 as being computed before invoking their inverse method, which prevents them from being invalidated by field F. --- .../addons/test_new_api/ir.model.access.csv | 2 ++ openerp/addons/test_new_api/models.py | 22 +++++++++++++++++++ .../test_new_api/tests/test_new_fields.py | 13 +++++++++++ openerp/models.py | 16 ++++++++++++-- 4 files changed, 51 insertions(+), 2 deletions(-) diff --git a/openerp/addons/test_new_api/ir.model.access.csv b/openerp/addons/test_new_api/ir.model.access.csv index 568a2703565..36227cc204c 100644 --- a/openerp/addons/test_new_api/ir.model.access.csv +++ b/openerp/addons/test_new_api/ir.model.access.csv @@ -9,3 +9,5 @@ access_mixed,test_new_api_mixed,test_new_api.model_test_new_api_mixed,,1,1,1,1 access_test_function_noinfiniterecursion,access_test_function_noinfiniterecursion,model_test_old_api_function_noinfiniterecursion,,1,1,1,1 access_test_function_counter,access_test_function_counter,model_test_old_api_function_counter,,1,1,1,1 access_domain_bool,access_domain_bool,model_domain_bool,,1,1,1,1 +access_test_new_api_foo,access_test_new_api_foo,model_test_new_api_foo,,1,1,1,1 +access_test_new_api_bar,access_test_new_api_bar,model_test_new_api_bar,,1,1,1,1 diff --git a/openerp/addons/test_new_api/models.py b/openerp/addons/test_new_api/models.py index d08bbcdeb90..80af95ab0e4 100644 --- a/openerp/addons/test_new_api/models.py +++ b/openerp/addons/test_new_api/models.py @@ -299,3 +299,25 @@ class BoolModel(models.Model): bool_true = fields.Boolean('b1', default=True) bool_false = fields.Boolean('b2', default=False) bool_undefined = fields.Boolean('b3') + + +class Foo(models.Model): + _name = 'test_new_api.foo' + + name = fields.Char() + value1 = fields.Integer() + value2 = fields.Integer() + + +class Bar(models.Model): + _name = 'test_new_api.bar' + + name = fields.Char() + foo = fields.Many2one('test_new_api.foo', compute='_compute_foo') + value1 = fields.Integer(related='foo.value1') + value2 = fields.Integer(related='foo.value2') + + @api.depends('name') + def _compute_foo(self): + for bar in self: + bar.foo = self.env['test_new_api.foo'].search([('name', '=', bar.name)], limit=1) diff --git a/openerp/addons/test_new_api/tests/test_new_fields.py b/openerp/addons/test_new_api/tests/test_new_fields.py index 16cb198c005..a38949def49 100644 --- a/openerp/addons/test_new_api/tests/test_new_fields.py +++ b/openerp/addons/test_new_api/tests/test_new_fields.py @@ -332,6 +332,19 @@ class TestNewFields(common.TransactionCase): discussion_field = discussion.fields_get(['name'])['name'] self.assertEqual(message_field['help'], discussion_field['help']) + def test_25_related_multi(self): + """ test write() on several related fields based on a common computed field. """ + foo = self.env['test_new_api.foo'].create({'name': 'A', 'value1': 1, 'value2': 2}) + bar = self.env['test_new_api.bar'].create({'name': 'A'}) + self.assertEqual(bar.foo, foo) + self.assertEqual(bar.value1, 1) + self.assertEqual(bar.value2, 2) + + foo.invalidate_cache() + bar.write({'value1': 3, 'value2': 4}) + self.assertEqual(foo.value1, 3) + self.assertEqual(foo.value2, 4) + def test_26_inherited(self): """ test inherited fields. """ # a bunch of fields are inherited from res_partner diff --git a/openerp/models.py b/openerp/models.py index 7d4f6e5220a..c03b97d2ba1 100644 --- a/openerp/models.py +++ b/openerp/models.py @@ -3787,12 +3787,18 @@ class BaseModel(object): if old_vals: self._write(old_vals) - # put the values of pure new-style fields into cache, and inverse them if new_vals: + # put the values of pure new-style fields into cache for record in self: record._cache.update(record._convert_to_cache(new_vals, update=True)) + # mark the fields as being computed, to avoid their invalidation + for key in new_vals: + self.env.computed[self._fields[key]].update(self._ids) + # inverse the fields for key in new_vals: self._fields[key].determine_inverse(self) + for key in new_vals: + self.env.computed[self._fields[key]].difference_update(self._ids) return True @@ -4093,10 +4099,16 @@ class BaseModel(object): # create record with old-style fields record = self.browse(self._create(old_vals)) - # put the values of pure new-style fields into cache, and inverse them + # put the values of pure new-style fields into cache record._cache.update(record._convert_to_cache(new_vals)) + # mark the fields as being computed, to avoid their invalidation + for key in new_vals: + self.env.computed[self._fields[key]].add(record.id) + # inverse the fields for key in new_vals: self._fields[key].determine_inverse(record) + for key in new_vals: + self.env.computed[self._fields[key]].discard(record.id) return record