changeset e7526f0686ec in trytond:default
details: https://hg.tryton.org/trytond?cmd=changeset&node=e7526f0686ec
description:
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
diffstat:
CHANGELOG | 1 +
trytond/model/fields/dict.py | 26 +++++++++++++++++++++++++-
trytond/model/fields/multiselection.py | 7 ++++++-
trytond/tests/modelview.py | 13 ++++++++++++-
trytond/tests/test_field_dict.py | 13 +++++++++++++
trytond/tests/test_field_multiselection.py | 14 +++++++-------
trytond/tests/test_modelstorage.py | 2 +-
trytond/tests/test_modelview.py | 18 +++++++++++++++++-
8 files changed, 82 insertions(+), 12 deletions(-)
diffs (267 lines):
diff -r 7a4a537f0f87 -r e7526f0686ec CHANGELOG
--- a/CHANGELOG Mon Apr 12 17:44:16 2021 +0200
+++ b/CHANGELOG Mon Apr 12 19:39:25 2021 +0200
@@ -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
diff -r 7a4a537f0f87 -r e7526f0686ec trytond/model/fields/dict.py
--- a/trytond/model/fields/dict.py Mon Apr 12 17:44:16 2021 +0200
+++ b/trytond/model/fields/dict.py Mon Apr 12 19:39:25 2021 +0200
@@ -17,6 +17,25 @@
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 @@
# 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 @@
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:
diff -r 7a4a537f0f87 -r e7526f0686ec trytond/model/fields/multiselection.py
--- a/trytond/model/fields/multiselection.py Mon Apr 12 17:44:16 2021 +0200
+++ b/trytond/model/fields/multiselection.py Mon Apr 12 19:39:25 2021 +0200
@@ -61,7 +61,7 @@
# 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 @@
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))
diff -r 7a4a537f0f87 -r e7526f0686ec trytond/tests/modelview.py
--- a/trytond/tests/modelview.py Mon Apr 12 17:44:16 2021 +0200
+++ b/trytond/tests/modelview.py Mon Apr 12 19:39:25 2021 +0200
@@ -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 @@
'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 @@
def register(module):
Pool.register(
ModelViewChangedValues,
+ ModelViewChangedValuesDictSchema,
ModelViewChangedValuesTarget,
ModelViewChangedValuesStoredTarget,
ModelViewStoredChangedValues,
diff -r 7a4a537f0f87 -r e7526f0686ec trytond/tests/test_field_dict.py
--- a/trytond/tests/test_field_dict.py Mon Apr 12 17:44:16 2021 +0200
+++ b/trytond/tests/test_field_dict.py Mon Apr 12 19:39:25 2021 +0200
@@ -886,6 +886,19 @@
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")
diff -r 7a4a537f0f87 -r e7526f0686ec trytond/tests/test_field_multiselection.py
--- a/trytond/tests/test_field_multiselection.py Mon Apr 12 17:44:16
2021 +0200
+++ b/trytond/tests/test_field_multiselection.py Mon Apr 12 19:39:25
2021 +0200
@@ -28,7 +28,7 @@
'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 @@
with self.assertRaises(SelectionValidationError):
Selection.create([{
- 'selects': ['invalid'],
+ 'selects': ('invalid'),
}])
@with_transaction()
@@ -54,8 +54,8 @@
'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 @@
'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 @@
'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 @@
'selects': ['foo', 'bar'],
})
- self.assertEqual(selection.selects, ['bar', 'foo'])
+ self.assertEqual(selection.selects, ('bar', 'foo'))
@with_transaction()
def test_string(self):
diff -r 7a4a537f0f87 -r e7526f0686ec trytond/tests/test_modelstorage.py
--- a/trytond/tests/test_modelstorage.py Mon Apr 12 17:44:16 2021 +0200
+++ b/trytond/tests/test_modelstorage.py Mon Apr 12 19:39:25 2021 +0200
@@ -638,7 +638,7 @@
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):
diff -r 7a4a537f0f87 -r e7526f0686ec trytond/tests/test_modelview.py
--- a/trytond/tests/test_modelview.py Mon Apr 12 17:44:16 2021 +0200
+++ b/trytond/tests/test_modelview.py Mon Apr 12 19:39:25 2021 +0200
@@ -34,6 +34,8 @@
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 @@
(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 @@
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 @@
'remove': [5, 6, 7],
'add': [(0, {'id': 9}), (1, {'id': 10})],
},
+ 'multiselection': ('a', 'b'),
+ 'dictionary': {'key': 'another value'},
})
# change only one2many record
