[IMP] api: make the implementation of field protection more robust

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

@@ -17,5 +17,6 @@ access_test_new_api_related,access_test_new_api_related,model_test_new_api_relat
 access_test_new_api_company,access_test_new_api_company,model_test_new_api_company,,1,1,1,1
 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_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

odoo/addons/test_new_api/models.py

@@ -327,6 +327,18 @@ class Related(models.Model):
     related_related_name = fields.Char(related='related_name', string='A related on a related on Name')
 
 
+class ComputeProtected(models.Model):
+    _name = 'test_new_api.compute.protected'
+
+    foo = fields.Char(default='')
+    bar = fields.Char(compute='_compute_bar', store=True)
+
+    @api.depends('foo')
+    def _compute_bar(self):
+        for record in self:
+            record.bar = record.foo
+
+
 class ComputeInverse(models.Model):
     _name = 'test_new_api.compute.inverse'
 

odoo/addons/test_new_api/tests/test_new_fields.py

@@ -130,6 +130,46 @@ class TestFields(common.TransactionCase):
         })
         check_stored(discussion3)
 
+    def test_11_stored_protected(self):
+        """ test protection against recomputation """
+        model = self.env['test_new_api.compute.protected']
+        field = model._fields['bar']
+
+        record = model.create({'foo': 'unprotected #1'})
+        self.assertEqual(record.bar, 'unprotected #1')
+
+        record.write({'foo': 'unprotected #2'})
+        self.assertEqual(record.bar, 'unprotected #2')
+
+        # by protecting 'bar', we prevent it from being recomputed
+        with self.env.protecting([field], record):
+            record.write({'foo': 'protected'})
+            self.assertEqual(record.bar, 'unprotected #2')
+
+            # also works when nested
+            with self.env.protecting([field], record):
+                record.write({'foo': 'protected'})
+                self.assertEqual(record.bar, 'unprotected #2')
+
+            record.write({'foo': 'protected'})
+            self.assertEqual(record.bar, 'unprotected #2')
+
+        record.write({'foo': 'unprotected #3'})
+        self.assertEqual(record.bar, 'unprotected #3')
+
+        # also works with duplicated fields
+        with self.env.protecting([field, field], record):
+            record.write({'foo': 'protected'})
+            self.assertEqual(record.bar, 'unprotected #3')
+
+        record.write({'foo': 'unprotected #4'})
+        self.assertEqual(record.bar, 'unprotected #4')
+
+        # we protect 'bar' on a different record
+        with self.env.protecting([field], record):
+            record2 = model.create({'foo': 'unprotected'})
+            self.assertEqual(record2.bar, 'unprotected')
+
     def test_11_computed_access(self):
         """ test computed fields with access right errors """
         User = self.env['res.users']

odoo/api.py

@@ -55,7 +55,7 @@ from weakref import WeakSet
 from decorator import decorator
 from werkzeug.local import Local, release_local
 
-from odoo.tools import frozendict, classproperty
+from odoo.tools import frozendict, classproperty, StackMap
 
 _logger = logging.getLogger(__name__)
 
@@ -741,7 +741,7 @@ class Environment(Mapping):
         self.cr, self.uid, self.context = self.args = (cr, uid, frozendict(context))
         self.registry = Registry(cr.dbname)
         self.cache = envs.cache
-        self._protected = defaultdict(frozenset)    # {field: ids, ...}
+        self._protected = StackMap()                # {field: ids, ...}
         self.dirty = defaultdict(set)               # {record: set(field_name), ...}
         self.all = envs
         envs.add(self)
@@ -861,14 +861,15 @@ class Environment(Mapping):
     @contextmanager
     def protecting(self, fields, records):
         """ Prevent the invalidation or recomputation of ``fields`` on ``records``. """
-        saved = {}
+        protected = self._protected
         try:
+            protected.pushmap()
             for field in fields:
-                ids = saved[field] = self._protected[field]
-                self._protected[field] = ids.union(records._ids)
+                ids = protected.get(field, frozenset())
+                protected[field] = ids.union(records._ids)
             yield
         finally:
-            self._protected.update(saved)
+            protected.popmap()
 
     def field_todo(self, field):
         """ Return a recordset with all records to recompute for ``field``. """

odoo/tools/misc.py

@@ -25,8 +25,7 @@ import types
 import unicodedata
 import werkzeug.utils
 import zipfile
-
-from collections import defaultdict, Iterable, Mapping, MutableSet, OrderedDict
+from collections import defaultdict, Iterable, Mapping, MutableMapping, MutableSet, OrderedDict
 from itertools import islice, groupby, repeat
 from lxml import etree
 
@@ -998,6 +997,49 @@ class Collector(Mapping):
     def __len__(self):
         return len(self._map)
 
+
+@pycompat.implements_to_string
+class StackMap(MutableMapping):
+    """ A stack of mappings behaving as a single mapping, and used to implement
+        nested scopes. The lookups search the stack from top to bottom, and
+        returns the first value found. Mutable operations modify the topmost
+        mapping only.
+    """
+    __slots__ = ['_maps']
+
+    def __init__(self, m=None):
+        self._maps = [] if m is None else [m]
+
+    def __getitem__(self, key):
+        for mapping in reversed(self._maps):
+            try:
+                return mapping[key]
+            except KeyError:
+                pass
+        raise KeyError(key)
+
+    def __setitem__(self, key, val):
+        self._maps[-1][key] = val
+
+    def __delitem__(self, key):
+        del self._maps[-1][key]
+
+    def __iter__(self):
+        return iter({key for mapping in self._maps for key in mapping})
+
+    def __len__(self):
+        return sum(1 for key in self)
+
+    def __str__(self):
+        return u"<StackMap %s>" % self._maps
+
+    def pushmap(self, m=None):
+        self._maps.append({} if m is None else m)
+
+    def popmap(self):
+        return self._maps.pop()
+
+
 class OrderedSet(MutableSet):
     """ A set collection that remembers the elements first insertion order. """
     __slots__ = ['_map']