diff --git a/CHANGELOG b/CHANGELOG index 0723167d37..0c8b3a5bda 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,4 @@ +* Remove implicit fields names in ModelStorage.read * Check read access on field in search domain (issue7766) * Add active field on ModelAccess, ModelFieldAccess and Group * Use write mode by default to check create and delete of resources diff --git a/doc/ref/models/models.rst b/doc/ref/models/models.rst index 28f2a82cdc..847732761c 100644 --- a/doc/ref/models/models.rst +++ b/doc/ref/models/models.rst @@ -252,10 +252,11 @@ Class methods: Trigger create actions. It will call actions defined in ``ir.trigger`` if ``on_create`` is set and ``condition`` is true. -.. classmethod:: ModelStorage.read(ids[, fields_names]) +.. classmethod:: ModelStorage.read(ids, fields_names) - Return a list of values for the ids. If ``fields_names`` is set, there will - be only values for these fields otherwise it will be for all fields. + Return a list of dictionary for the record ids. The dictionary is composed + of the fields as key and their values. + The order of the returned list is not guaranteed. .. classmethod:: ModelStorage.write(records, values, [[records, values], ...]) diff --git a/trytond/ir/lang.py b/trytond/ir/lang.py index eb1e2d1ab0..37959645bd 100644 --- a/trytond/ir/lang.py +++ b/trytond/ir/lang.py @@ -89,17 +89,17 @@ class Lang(DeactivableMixin, ModelSQL, ModelView): return [('name',) + tuple(clause[1:])] @classmethod - def read(cls, ids, fields_names=None): + def read(cls, ids, fields_names): pool = Pool() Translation = pool.get('ir.translation') Config = pool.get('ir.configuration') - res = super(Lang, cls).read(ids, fields_names=fields_names) + res = super(Lang, cls).read(ids, fields_names) if (Transaction().context.get('translate_name') and (not fields_names or 'name' in fields_names)): with Transaction().set_context( language=Config.get_language(), translate_name=False): - res2 = cls.read(ids, fields_names=['id', 'code', 'name']) + res2 = cls.read(ids, ['id', 'code', 'name']) for record2 in res2: for record in res: if record['id'] == record2['id']: diff --git a/trytond/ir/model.py b/trytond/ir/model.py index acc8179ce4..82e09eb93d 100644 --- a/trytond/ir/model.py +++ b/trytond/ir/model.py @@ -330,16 +330,13 @@ class ModelField(ModelSQL, ModelView): ] @classmethod - def read(cls, ids, fields_names=None): + def read(cls, ids, fields_names): pool = Pool() Translation = pool.get('ir.translation') Model = pool.get('ir.model') to_delete = [] if Transaction().context.get('language'): - if fields_names is None: - fields_names = list(cls._fields.keys()) - if 'field_description' in fields_names \ or 'help' in fields_names: if 'model' not in fields_names: @@ -349,7 +346,7 @@ class ModelField(ModelSQL, ModelView): fields_names.append('name') to_delete.append('name') - res = super(ModelField, cls).read(ids, fields_names=fields_names) + res = super(ModelField, cls).read(ids, fields_names) if (Transaction().context.get('language') and ('field_description' in fields_names diff --git a/trytond/ir/resource.py b/trytond/ir/resource.py index 1b796dc82a..4aaac956ff 100644 --- a/trytond/ir/resource.py +++ b/trytond/ir/resource.py @@ -79,9 +79,9 @@ class ResourceMixin(ModelSQL, ModelView): (model, {'create': 'write', 'delete': 'write'}.get(mode, mode))] @classmethod - def read(cls, ids, fields_names=None): + def read(cls, ids, fields_names): cls.check_access(ids, mode='read') - return super(ResourceMixin, cls).read(ids, fields_names=fields_names) + return super(ResourceMixin, cls).read(ids, fields_names) @classmethod def delete(cls, records): diff --git a/trytond/model/modelsingleton.py b/trytond/model/modelsingleton.py index f025b57458..e50ef63417 100644 --- a/trytond/model/modelsingleton.py +++ b/trytond/model/modelsingleton.py @@ -28,11 +28,9 @@ class ModelSingleton(ModelStorage): return [singleton] @classmethod - def read(cls, ids, fields_names=None): + def read(cls, ids, fields_names): singleton = cls.get_singleton() if not singleton: - if not fields_names: - fields_names = list(cls._fields.keys()) fname_no_rec_name = [f for f in fields_names if '.' not in f] res = cls.default_get(fname_no_rec_name, with_rec_name=len(fname_no_rec_name) != len(fields_names)) @@ -44,8 +42,7 @@ class ModelSingleton(ModelStorage): del res[field_name] res['id'] = ids[0] return [res] - res = super(ModelSingleton, cls).read([singleton.id], - fields_names=fields_names) + res = super(ModelSingleton, cls).read([singleton.id], fields_names) res[0]['id'] = ids[0] return res diff --git a/trytond/model/modelsql.py b/trytond/model/modelsql.py index 6fd3643adc..8c237f7924 100644 --- a/trytond/model/modelsql.py +++ b/trytond/model/modelsql.py @@ -656,17 +656,10 @@ class ModelSQL(ModelStorage): return records @classmethod - def read(cls, ids, fields_names=None): + def read(cls, ids, fields_names): pool = Pool() Rule = pool.get('ir.rule') Translation = pool.get('ir.translation') - ModelAccess = pool.get('ir.model.access') - if not fields_names: - fields_names = [] - for field_name in list(cls._fields.keys()): - if ModelAccess.check_relation(cls.__name__, field_name, - mode='read'): - fields_names.append(field_name) super(ModelSQL, cls).read(ids, fields_names=fields_names) transaction = Transaction() cursor = Transaction().connection.cursor() diff --git a/trytond/model/modelstorage.py b/trytond/model/modelstorage.py index 797d2c89dd..2677116256 100644 --- a/trytond/model/modelstorage.py +++ b/trytond/model/modelstorage.py @@ -128,23 +128,14 @@ class ModelStorage(Model): Trigger.trigger_action(triggers, trigger) @classmethod - def read(cls, ids, fields_names=None): + def read(cls, ids, fields_names): ''' Read fields_names of record ids. - If fields_names is None, it read all fields. The order is not guaranteed. ''' pool = Pool() ModelAccess = pool.get('ir.model.access') ModelFieldAccess = pool.get('ir.model.field.access') - - if not fields_names: - fields_names = [] - for field_name in cls._fields.keys(): - if ModelAccess.check_relation(cls.__name__, field_name, - mode='read'): - fields_names.append(field_name) - ModelAccess.check(cls.__name__, 'read') ModelFieldAccess.check(cls.__name__, fields_names, 'read') return [] diff --git a/trytond/res/user.py b/trytond/res/user.py index fb2b24b7b8..52c3529cfc 100644 --- a/trytond/res/user.py +++ b/trytond/res/user.py @@ -352,8 +352,8 @@ class User(DeactivableMixin, ModelSQL, ModelView): return vals @classmethod - def read(cls, ids, fields_names=None): - result = super(User, cls).read(ids, fields_names=fields_names) + def read(cls, ids, fields_names): + result = super(User, cls).read(ids, fields_names) if not fields_names or 'password_hash' in fields_names: for values in result: values['password_hash'] = None diff --git a/trytond/tests/test_access.py b/trytond/tests/test_access.py index 5d0c1877fe..0a3f183ecc 100644 --- a/trytond/tests/test_access.py +++ b/trytond/tests/test_access.py @@ -259,14 +259,14 @@ class ModelAccessReadTestCase(_ModelAccessTestCase): def _assert(self, record): pool = Pool() TestAccess = pool.get('test.access') - TestAccess.read([record.id]) + TestAccess.read([record.id], ['field1']) TestAccess.search([]) def _assert_raises(self, record): pool = Pool() TestAccess = pool.get('test.access') with self.assertRaises(UserError): - TestAccess.read([record.id]) + TestAccess.read([record.id], ['field1']) with self.assertRaises(UserError): TestAccess.search([]) diff --git a/trytond/tests/test_history.py b/trytond/tests/test_history.py index 9eb93f8ee2..23f82d1d5c 100644 --- a/trytond/tests/test_history.py +++ b/trytond/tests/test_history.py @@ -73,7 +73,7 @@ class HistoryTestCase(unittest.TestCase): self.assertEqual(history.value, value) with Transaction().set_context(_datetime=datetime.datetime.min): - self.assertRaises(UserError, History.read, [history_id]) + self.assertRaises(UserError, History.read, [history_id], ['value']) @unittest.skipUnless(backend.name() == 'postgresql', 'CURRENT_TIMESTAMP as transaction_timestamp is specific to postgresql') @@ -174,7 +174,7 @@ class HistoryTestCase(unittest.TestCase): transaction.rollback() History.restore_history([history_id], datetime.datetime.min) - self.assertRaises(UserError, History.read, [history_id]) + self.assertRaises(UserError, History.read, [history_id], ['value']) transaction.rollback() @@ -183,7 +183,7 @@ class HistoryTestCase(unittest.TestCase): transaction.commit() History.restore_history([history_id], datetime.datetime.max) - self.assertRaises(UserError, History.read, [history_id]) + self.assertRaises(UserError, History.read, [history_id], ['value']) @with_transaction() def test_restore_history_before(self): diff --git a/trytond/tests/test_rule.py b/trytond/tests/test_rule.py index ccd9498997..fe13dd2a3d 100644 --- a/trytond/tests/test_rule.py +++ b/trytond/tests/test_rule.py @@ -228,7 +228,7 @@ class ModelRuleTestCase(unittest.TestCase): test, = TestRule.create([{'field': 'foo'}]) - TestRule.read([test.id]) + TestRule.read([test.id], ['field']) @with_transaction() def test_perm_read_with_rule(self): @@ -253,7 +253,7 @@ class ModelRuleTestCase(unittest.TestCase): }]) test, = TestRule.create([{'field': 'bar'}]) - TestRule.read([test.id]) + TestRule.read([test.id], ['field']) @with_transaction() def test_perm_read_with_rule_fail(self): @@ -279,7 +279,7 @@ class ModelRuleTestCase(unittest.TestCase): test, = TestRule.create([{'field': 'foo'}]) with self.assertRaises(UserError): - TestRule.read([test.id]) + TestRule.read([test.id], ['field']) @with_transaction() def test_search_without_rule(self):