Use immutable datastructures for Dict and MultiSelection fields

In case a Dict field is sent during an on_change, modification of its value
won't be detected because a shallow copy will keep its reference in the
pseudo-record

issue10105
review347541002

CHANGELOG

@@ -1,3 +1,4 @@
+* Use immutable datastructures for Dict and MultiSelection fields
 * Skip warnings for non-interactive operations
 * Check rule only if _check_access is set
 * Add statistics to Cache

trytond/model/fields/dict.py

@@ -17,6 +17,25 @@ dumps = partial(
     json.dumps, cls=JSONEncoder, separators=(',', ':'), sort_keys=True)
 
 
+class ImmutableDict(dict):
+
+    __slots__ = ()
+
+    def _not_allowed(cls, *args, **kwargs):
+        raise TypeError("Operation not allowed on ImmutableDict")
+
+    __setitem__ = _not_allowed
+    __delitem__ = _not_allowed
+    __ior__ = _not_allowed
+    clear = _not_allowed
+    pop = _not_allowed
+    popitem = _not_allowed
+    setdefault = _not_allowed
+    update = _not_allowed
+
+    del _not_allowed
+
+
 class Dict(Field):
     'Define dict field.'
     _type = 'dict'
@@ -41,7 +60,7 @@ class Dict(Field):
                 # If stored as JSON conversion is done on backend
                 if isinstance(data, str):
                     data = json.loads(data, object_hook=JSONDecoder())
-                dicts[value['id']] = data
+                dicts[value['id']] = ImmutableDict(data)
         return dicts
 
     def sql_format(self, value):
@@ -57,6 +76,11 @@ class Dict(Field):
             value = dumps(d)
         return value
 
+    def __set__(self, inst, value):
+        if value:
+            value = ImmutableDict(value)
+        super().__set__(inst, value)
+
     def translated(self, name=None, type_='values'):
         "Return a descriptor for the translated value of the field"
         if name is None:

trytond/model/fields/multiselection.py

@@ -61,7 +61,7 @@ class MultiSelection(SelectionMixin, Field):
                 # If stored as JSON conversion is done on backend
                 if isinstance(data, str):
                     data = json.loads(data)
-                lists[value['id']] = data
+                lists[value['id']] = tuple(data)
         return lists
 
     def sql_format(self, value):
@@ -70,6 +70,11 @@ class MultiSelection(SelectionMixin, Field):
             value = dumps(sorted(set(value)))
         return value
 
+    def __set__(self, inst, value):
+        if value:
+            value = tuple(value)
+        super().__set__(inst, value)
+
     def _domain_column(self, operator, column):
         database = Transaction().database
         return database.json_get(super()._domain_column(operator, column))

trytond/tests/modelview.py

@@ -1,7 +1,7 @@
 # This file is part of Tryton.  The COPYRIGHT file at the top level of
 # this repository contains the full copyright notices and license terms.
 
-from trytond.model import ModelView, ModelSQL, fields
+from trytond.model import ModelView, ModelSQL, DictSchemaMixin, fields
 from trytond.pool import Pool
 from trytond.pyson import If, Eval
 
@@ -20,6 +20,16 @@ class ModelViewChangedValues(ModelView):
         'Targets')
     m2m_targets = fields.Many2Many('test.modelview.changed_values.target',
         None, None, 'Targets')
+    multiselection = fields.MultiSelection([
+            ('a', 'A'), ('b', 'B'),
+            ], "MultiSelection")
+    dictionary = fields.Dict(
+        'test.modelview.changed_values.dictionary', "Dictionary")
+
+
+class ModelViewChangedValuesDictSchema(DictSchemaMixin, ModelSQL):
+    'ModelView Changed Values Dict Schema'
+    __name__ = 'test.modelview.changed_values.dictionary'
 
 
 class ModelViewChangedValuesTarget(ModelView):
@@ -253,6 +263,7 @@ class ModelViewViewAttributesDepends(ModelView):
 def register(module):
     Pool.register(
         ModelViewChangedValues,
+        ModelViewChangedValuesDictSchema,
         ModelViewChangedValuesTarget,
         ModelViewChangedValuesStoredTarget,
         ModelViewStoredChangedValues,

trytond/tests/test_field_dict.py

@@ -886,6 +886,19 @@ class FieldDictTestCase(unittest.TestCase):
         self.assertDictEqual(
             dict_.dico_string_keys, {'a': 'A', 'type': "Type"})
 
+    @with_transaction
+    def test_set_key(self):
+        "Test setting a key of dict"
+        Dict = Pool().get('test.dict')
+        self.create_schema()
+
+        dict_, = Dict.create([{
+                    'dico': {'a': 1, 'type': 'arabic'},
+                    }])
+
+        with self.assertRaises(TypeError):
+            dict.dico['a'] = 5
+
 
 @unittest.skipUnless(backend.name == 'postgresql',
     "unaccent works only on postgresql")

trytond/tests/test_field_multiselection.py

@@ -28,7 +28,7 @@ class FieldMultiSelectionTestCase(unittest.TestCase):
                     'selects': None,
                     }])
 
-        self.assertEqual(selection.selects, ['bar', 'foo'])
+        self.assertEqual(selection.selects, ('bar', 'foo'))
         self.assertEqual(selection_none.selects, None)
 
     @with_transaction()
@@ -38,7 +38,7 @@ class FieldMultiSelectionTestCase(unittest.TestCase):
 
         with self.assertRaises(SelectionValidationError):
             Selection.create([{
-                        'selects': ['invalid'],
+                        'selects': ('invalid'),
                         }])
 
     @with_transaction()
@@ -54,8 +54,8 @@ class FieldMultiSelectionTestCase(unittest.TestCase):
                     'dyn_selects': ['baz'],
                     }])
 
-        self.assertEqual(selection_foo.dyn_selects, ['foo'])
-        self.assertEqual(selection_bar.dyn_selects, ['baz'])
+        self.assertEqual(selection_foo.dyn_selects, ('foo',))
+        self.assertEqual(selection_bar.dyn_selects, ('baz',))
 
     @with_transaction()
     def test_create_dynamic_none(self):
@@ -89,7 +89,7 @@ class FieldMultiSelectionTestCase(unittest.TestCase):
                     'static_selects': ['foo', 'bar'],
                     }])
 
-        self.assertEqual(selection.static_selects, ['bar', 'foo'])
+        self.assertEqual(selection.static_selects, ('bar', 'foo'))
 
     @with_transaction()
     def test_create_static_none(self):
@@ -121,7 +121,7 @@ class FieldMultiSelectionTestCase(unittest.TestCase):
                     'selects': ['foo', 'bar'],
                     }])
 
-        self.assertEqual(selection.selects, ['bar', 'foo'])
+        self.assertEqual(selection.selects, ('bar', 'foo'))
 
     @with_transaction()
     def test_create_required_without_value(self):
@@ -163,7 +163,7 @@ class FieldMultiSelectionTestCase(unittest.TestCase):
                 'selects': ['foo', 'bar'],
                 })
 
-        self.assertEqual(selection.selects, ['bar', 'foo'])
+        self.assertEqual(selection.selects, ('bar', 'foo'))
 
     @with_transaction()
     def test_string(self):

trytond/tests/test_modelstorage.py

@@ -638,7 +638,7 @@ class EvalEnvironmentTestCase(unittest.TestCase):
         record = Model(multiselection=['value1', 'value2'])
         env = EvalEnvironment(record, Model)
 
-        self.assertEqual(env.get('multiselection'), ['value1', 'value2'])
+        self.assertEqual(env.get('multiselection'), ('value1', 'value2'))
 
     @with_transaction()
     def test_parent_field(self):

trytond/tests/test_modelview.py

@@ -34,6 +34,8 @@ class ModelView(unittest.TestCase):
         record.target = Target(1)
         record.ref_target = Target(2)
         record.targets = [Target(name='bar')]
+        record.multiselection = ['a']
+        record.dictionary = {'key': 'value'}
         self.assertEqual(record._changed_values, {
                 'name': 'foo',
                 'target': 1,
@@ -43,12 +45,18 @@ class ModelView(unittest.TestCase):
                         (0, {'id': None, 'name': 'bar'}),
                         ],
                     },
+                'multiselection': ('a',),
+                'dictionary': {'key': 'value'},
                 })
 
         record = Model(name='test', target=1, targets=[
                 {'id': 1, 'name': 'foo'},
                 {'id': 2},
-                ], m2m_targets=[5, 6, 7])
+                ],
+            m2m_targets=[5, 6, 7],
+            multiselection=['a'],
+            dictionary={'key': 'value'},
+            )
 
         self.assertEqual(record._changed_values, {})
 
@@ -56,6 +64,12 @@ class ModelView(unittest.TestCase):
         target.name = 'bar'
         record.targets = [target]
         record.m2m_targets = [Target(9), Target(10)]
+        ms = list(record.multiselection)
+        ms.append('b')
+        record.multiselection = ms
+        dico = record.dictionary.copy()
+        dico['key'] = 'another value'
+        record.dictionary = dico
         self.assertEqual(record._changed_values, {
                 'targets': {
                     'update': [{'id': 1, 'name': 'bar'}],
@@ -65,6 +79,8 @@ class ModelView(unittest.TestCase):
                     'remove': [5, 6, 7],
                     'add': [(0, {'id': 9}), (1, {'id': 10})],
                     },
+                'multiselection': ('a', 'b'),
+                'dictionary': {'key': 'another value'},
                 })
 
         # change only one2many record