[FIX] models: behavior with multiple inverse/compute mixed on same fields

6 files changed, 105 insertions, 34 deletions

diff --git a/addons/base_address_extended/models/base_address_extended.py b/addons/base_address_extended/models/base_address_extended.py
index a2c31249951..1ca0178ba20 100644
--- a/addons/base_address_extended/models/base_address_extended.py
+++ b/addons/base_address_extended/models/base_address_extended.py
@@ -74,12 +74,7 @@ class Partner(models.Model):
 
             # add trailing chars in street_format
             street_value += street_format[previous_pos:]
-
-            # /!\ Note that we must use a sql query to bypass the orm as it would call _split_street()
-            # that would try to set the fields we just modified.
-            self._cr.execute('UPDATE res_partner SET street = %s WHERE ID = %s', (street_value, partner.id))
-            #invalidate the cache for the field we manually set
-            self.invalidate_cache(['street'], [partner.id])
+            partner.street = street_value
 
     @api.multi
     @api.depends('street')
@@ -133,7 +128,6 @@ class Partner(models.Model):
             else:
                 vals[field_name] = street_raw
             # assign the values to the fields
-            # /!\ Note that a write(vals) would cause a recursion since it would bypass the cache
             for k, v in vals.items():
                 partner[k] = v
 
diff --git a/odoo/addons/test_new_api/ir.model.access.csv b/odoo/addons/test_new_api/ir.model.access.csv
index fd10a216720..46a658c818d 100644
--- a/odoo/addons/test_new_api/ir.model.access.csv
+++ b/odoo/addons/test_new_api/ir.model.access.csv
@@ -18,5 +18,6 @@ access_test_new_api_company,access_test_new_api_company,model_test_new_api_compa
 access_test_new_api_company_attr,access_test_new_api_company_attr,model_test_new_api_company_attr,,1,1,1,1
 access_test_new_api_compute_inverse,access_test_new_api_compute_inverse,model_test_new_api_compute_inverse,,1,1,1,1
 access_test_new_api_compute_protected,access_test_new_api_compute_protected,model_test_new_api_compute_protected,,1,1,1,1
+access_test_new_api_multi_compute_inverse,access_test_new_api_multi_compute_inverse,model_test_new_api_multi_compute_inverse,,1,1,1,1
 access_test_new_api_recursive,access_test_new_api_recursive,model_test_new_api_recursive,,1,1,1,1
 access_test_new_api_cascade,access_test_new_api_cascade,model_test_new_api_cascade,,1,1,1,1
diff --git a/odoo/addons/test_new_api/models.py b/odoo/addons/test_new_api/models.py
index 274d8257d04..70e1d1b2d2c 100644
--- a/odoo/addons/test_new_api/models.py
+++ b/odoo/addons/test_new_api/models.py
@@ -342,28 +342,54 @@ class ComputeProtected(models.Model):
 class ComputeInverse(models.Model):
     _name = 'test_new_api.compute.inverse'
 
-    counts = {'compute': 0, 'inverse': 0}
-
     foo = fields.Char()
     bar = fields.Char(compute='_compute_bar', inverse='_inverse_bar', store=True)
 
     @api.depends('foo')
     def _compute_bar(self):
-        self.counts['compute'] += 1
+        self._context.get('log', []).append('compute')
         for record in self:
             record.bar = record.foo
 
     def _inverse_bar(self):
-        self.counts['inverse'] += 1
+        self._context.get('log', []).append('inverse')
         for record in self:
             record.foo = record.bar
 
 
+class MultiComputeInverse(models.Model):
+    """ Model with the same inverse method for several fields. """
+    _name = 'test_new_api.multi_compute_inverse'
+
+    foo = fields.Char(default='', required=True)
+    bar1 = fields.Char(compute='_compute_bars', inverse='_inverse_bar1', store=True)
+    bar2 = fields.Char(compute='_compute_bars', inverse='_inverse_bar23', store=True)
+    bar3 = fields.Char(compute='_compute_bars', inverse='_inverse_bar23', store=True)
+
+    @api.depends('foo')
+    def _compute_bars(self):
+        self._context.get('log', []).append('compute')
+        for record in self:
+            substrs = record.foo.split('/') + ['', '', '']
+            record.bar1, record.bar2, record.bar3 = substrs[:3]
+
+    def _inverse_bar1(self):
+        self._context.get('log', []).append('inverse1')
+        for record in self:
+            record.write({'foo': '/'.join([record.bar1, record.bar2, record.bar3])})
+
+    def _inverse_bar23(self):
+        self._context.get('log', []).append('inverse23')
+        for record in self:
+            record.write({'foo': '/'.join([record.bar1, record.bar2, record.bar3])})
+
+
 class CompanyDependent(models.Model):
     _name = 'test_new_api.company'
 
     foo = fields.Char(company_dependent=True)
 
+
 class CompanyDependentAttribute(models.Model):
     _name = 'test_new_api.company.attr'
 
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 c0fa1bc7b59..09605f37aa2 100644
--- a/odoo/addons/test_new_api/tests/test_new_fields.py
+++ b/odoo/addons/test_new_api/tests/test_new_fields.py
@@ -278,33 +278,70 @@ class TestFields(common.TransactionCase):
         self.assertEqual(ewan.parent, cath)
         self.assertEqual(ewan.name, "Erwan")
 
-        record = self.env['test_new_api.compute.inverse']
-
         # create/write on 'foo' should only invoke the compute method
-        record.counts.update(compute=0, inverse=0)
-        record = record.create({'foo': 'Hi'})
+        log = []
+        model = self.env['test_new_api.compute.inverse'].with_context(log=log)
+        record = model.create({'foo': 'Hi'})
         self.assertEqual(record.foo, 'Hi')
         self.assertEqual(record.bar, 'Hi')
-        self.assertEqual(record.counts, {'compute': 1, 'inverse': 0})
+        self.assertCountEqual(log, ['compute'])
 
-        record.counts.update(compute=0, inverse=0)
+        log.clear()
         record.write({'foo': 'Ho'})
         self.assertEqual(record.foo, 'Ho')
         self.assertEqual(record.bar, 'Ho')
-        self.assertEqual(record.counts, {'compute': 1, 'inverse': 0})
+        self.assertCountEqual(log, ['compute'])
 
         # create/write on 'bar' should only invoke the inverse method
-        record.counts.update(compute=0, inverse=0)
-        record = record.create({'bar': 'Hi'})
+        log.clear()
+        record = model.create({'bar': 'Hi'})
         self.assertEqual(record.foo, 'Hi')
         self.assertEqual(record.bar, 'Hi')
-        self.assertEqual(record.counts, {'compute': 0, 'inverse': 1})
+        self.assertCountEqual(log, ['inverse'])
 
-        record.counts.update(compute=0, inverse=0)
+        log.clear()
         record.write({'bar': 'Ho'})
         self.assertEqual(record.foo, 'Ho')
         self.assertEqual(record.bar, 'Ho')
-        self.assertEqual(record.counts, {'compute': 0, 'inverse': 1})
+        self.assertCountEqual(log, ['inverse'])
+
+        # Test compatibility multiple compute/inverse fields
+        log = []
+        model = self.env['test_new_api.multi_compute_inverse'].with_context(log=log)
+        record = model.create({
+            'bar1': '1',
+            'bar2': '2',
+            'bar3': '3',
+        })
+        self.assertEqual(record.foo, '1/2/3')
+        self.assertEqual(record.bar1, '1')
+        self.assertEqual(record.bar2, '2')
+        self.assertEqual(record.bar3, '3')
+        self.assertCountEqual(log, ['inverse1', 'inverse23'])
+
+        log.clear()
+        record.write({'bar2': '4', 'bar3': '5'})
+        self.assertEqual(record.foo, '1/4/5')
+        self.assertEqual(record.bar1, '1')
+        self.assertEqual(record.bar2, '4')
+        self.assertEqual(record.bar3, '5')
+        self.assertCountEqual(log, ['inverse23'])
+
+        log.clear()
+        record.write({'bar1': '6', 'bar2': '7'})
+        self.assertEqual(record.foo, '6/7/5')
+        self.assertEqual(record.bar1, '6')
+        self.assertEqual(record.bar2, '7')
+        self.assertEqual(record.bar3, '5')
+        self.assertCountEqual(log, ['inverse1', 'inverse23'])
+
+        log.clear()
+        record.write({'foo': 'A/B/C'})
+        self.assertEqual(record.foo, 'A/B/C')
+        self.assertEqual(record.bar1, 'A')
+        self.assertEqual(record.bar2, 'B')
+        self.assertEqual(record.bar3, 'C')
+        self.assertCountEqual(log, ['compute'])
 
     def test_14_search(self):
         """ test search on computed fields """
diff --git a/odoo/models.py b/odoo/models.py
index 0af021810cc..3dcfeb74315 100644
--- a/odoo/models.py
+++ b/odoo/models.py
@@ -50,7 +50,7 @@ from . import tools
 from .exceptions import AccessError, MissingError, ValidationError, UserError
 from .osv.query import Query
 from .tools import frozendict, lazy_classproperty, lazy_property, ormcache, \
-                   Collector, LastOrderedSet, OrderedSet, pycompat
+                   Collector, LastOrderedSet, OrderedSet, pycompat, groupby
 from .tools.config import config
 from .tools.func import frame_codeinfo
 from .tools.misc import CountingStream, DEFAULT_SERVER_DATETIME_FORMAT, DEFAULT_SERVER_DATE_FORMAT
@@ -2978,6 +2978,7 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})):
         inverse_vals = {}
         inherited_vals = defaultdict(dict)      # {modelname: {fieldname: value}}
         unknown_names = []
+        inverse_fields = []
         protected_fields = []
         for key, val in vals.items():
             if key in bad_names:
@@ -2992,7 +2993,8 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})):
                 inherited_vals[field.related_field.model_name][key] = val
             elif field.inverse:
                 inverse_vals[key] = val
-                protected_fields.append(field)
+                inverse_fields.append(field)
+                protected_fields.extend(self._field_computed.get(field, [field]))
 
         if unknown_names:
             _logger.warning("%s.write() with unknown fields: %s",
@@ -3021,8 +3023,11 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})):
                 self.modified(set(inverse_vals) - set(store_vals))
                 for record in self:
                     record._cache.update(record._convert_to_cache(inverse_vals, update=True))
-                for key in inverse_vals:
-                    self._fields[key].determine_inverse(self)
+
+                # in case several fields use the same inverse method, call it once
+                for fields in groupby(attrgetter('inverse'), inverse_fields):
+                    fields[0].determine_inverse(self)
+
                 self.modified(set(inverse_vals) - set(store_vals))
 
                 # check Python constraints for inversed fields
@@ -3174,6 +3179,7 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})):
         inverse_vals = {}
         inherited_vals = defaultdict(dict)      # {modelname: {fieldname: value}}
         unknown_names = []
+        inverse_fields = []
         protected_fields = []
         for key, val in vals.items():
             if key in bad_names:
@@ -3188,7 +3194,8 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})):
                 inherited_vals[field.related_field.model_name][key] = val
             elif field.inverse:
                 inverse_vals[key] = val
-                protected_fields.append(field)
+                inverse_fields.append(field)
+                protected_fields.extend(self._field_computed.get(field, [field]))
 
         if unknown_names:
             _logger.warning("%s.create() with unknown fields: %s",
@@ -3209,8 +3216,11 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})):
         with self.env.protecting(protected_fields, record):
             # put the values of inverse fields in cache, and inverse them
             record._cache.update(record._convert_to_cache(inverse_vals))
-            for key in inverse_vals:
-                self._fields[key].determine_inverse(record)
+
+            # in case several fields use the same inverse method, call it once
+            for fields in groupby(attrgetter('inverse'), inverse_fields):
+                fields[0].determine_inverse(record)
+
             record.modified(set(inverse_vals) - set(store_vals))
 
             # check Python constraints for inversed fields
@@ -3303,10 +3313,6 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})):
             # check Python constraints
             self._validate_fields(vals)
 
-            # recompute fields
-            if self.env.recompute and self._context.get('recompute', True):
-                self.recompute()
-
         self.check_access_rule('create')
 
         # add translations
diff --git a/odoo/tools/misc.py b/odoo/tools/misc.py
index a9b66117f03..5fc1f18e1b2 100644
--- a/odoo/tools/misc.py
+++ b/odoo/tools/misc.py
@@ -1062,6 +1062,13 @@ class LastOrderedSet(OrderedSet):
         OrderedSet.discard(self, elem)
         OrderedSet.add(self, elem)
 
+def groupby(key, elems):
+    """ Return a partition of ``elems`` based on ``key(elem)``. """
+    groups = defaultdict(list)
+    for elem in elems:
+        groups[key(elem)].append(elem)
+    return groups.values()
+
 def unique(it):
     """ "Uniquifier" for the provided iterable: will output each element of
     the iterable once.