From f1d60d2e528284c4d4401d40e362f1b45ac32211 Mon Sep 17 00:00:00 2001 From: Raphael Collet Date: Thu, 18 Jan 2018 12:15:15 +0100 Subject: [PATCH] [IMP] api: make the implementation of field protection more robust --- odoo/addons/test_new_api/ir.model.access.csv | 1 + odoo/addons/test_new_api/models.py | 12 +++++ .../test_new_api/tests/test_new_fields.py | 40 ++++++++++++++++ odoo/api.py | 13 +++--- odoo/tools/misc.py | 46 ++++++++++++++++++- 5 files changed, 104 insertions(+), 8 deletions(-) diff --git a/odoo/addons/test_new_api/ir.model.access.csv b/odoo/addons/test_new_api/ir.model.access.csv index a88e202a86d..fd10a216720 100644 --- a/odoo/addons/test_new_api/ir.model.access.csv +++ b/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 diff --git a/odoo/addons/test_new_api/models.py b/odoo/addons/test_new_api/models.py index 14712030dcc..274d8257d04 100644 --- a/odoo/addons/test_new_api/models.py +++ b/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' 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 20827674977..c0fa1bc7b59 100644 --- a/odoo/addons/test_new_api/tests/test_new_fields.py +++ b/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'] diff --git a/odoo/api.py b/odoo/api.py index e9657b798c0..e0ba72a7e51 100644 --- a/odoo/api.py +++ b/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``. """ diff --git a/odoo/tools/misc.py b/odoo/tools/misc.py index ac3a56b3825..a9b66117f03 100644 --- a/odoo/tools/misc.py +++ b/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"" % 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']