changeset bfef7a2472b2 in trytond:default details: https://hg.tryton.org/trytond?cmd=changeset;node=bfef7a2472b2 description: Remove implicit fields names in ModelStorage.read
It is a bad practice to not explicit the fields to read. So we should not support it. issue7830 review54421002 diffstat: CHANGELOG | 1 + doc/ref/models/models.rst | 7 ++++--- trytond/ir/lang.py | 6 +++--- trytond/ir/model.py | 7 ++----- trytond/ir/resource.py | 4 ++-- trytond/model/modelsingleton.py | 7 ++----- trytond/model/modelsql.py | 9 +-------- trytond/model/modelstorage.py | 11 +---------- trytond/res/user.py | 4 ++-- trytond/tests/test_access.py | 4 ++-- trytond/tests/test_history.py | 6 +++--- trytond/tests/test_rule.py | 6 +++--- 12 files changed, 26 insertions(+), 46 deletions(-) diffs (265 lines): diff -r d1dca83b5330 -r bfef7a2472b2 CHANGELOG --- a/CHANGELOG Wed Nov 28 10:02:50 2018 +0100 +++ b/CHANGELOG Wed Nov 28 10:05:01 2018 +0100 @@ -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 -r d1dca83b5330 -r bfef7a2472b2 doc/ref/models/models.rst --- a/doc/ref/models/models.rst Wed Nov 28 10:02:50 2018 +0100 +++ b/doc/ref/models/models.rst Wed Nov 28 10:05:01 2018 +0100 @@ -252,10 +252,11 @@ 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 -r d1dca83b5330 -r bfef7a2472b2 trytond/ir/lang.py --- a/trytond/ir/lang.py Wed Nov 28 10:02:50 2018 +0100 +++ b/trytond/ir/lang.py Wed Nov 28 10:05:01 2018 +0100 @@ -89,17 +89,17 @@ 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 -r d1dca83b5330 -r bfef7a2472b2 trytond/ir/model.py --- a/trytond/ir/model.py Wed Nov 28 10:02:50 2018 +0100 +++ b/trytond/ir/model.py Wed Nov 28 10:05:01 2018 +0100 @@ -330,16 +330,13 @@ ] @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 @@ 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 -r d1dca83b5330 -r bfef7a2472b2 trytond/ir/resource.py --- a/trytond/ir/resource.py Wed Nov 28 10:02:50 2018 +0100 +++ b/trytond/ir/resource.py Wed Nov 28 10:05:01 2018 +0100 @@ -79,9 +79,9 @@ (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 -r d1dca83b5330 -r bfef7a2472b2 trytond/model/modelsingleton.py --- a/trytond/model/modelsingleton.py Wed Nov 28 10:02:50 2018 +0100 +++ b/trytond/model/modelsingleton.py Wed Nov 28 10:05:01 2018 +0100 @@ -28,11 +28,9 @@ 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 @@ 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 -r d1dca83b5330 -r bfef7a2472b2 trytond/model/modelsql.py --- a/trytond/model/modelsql.py Wed Nov 28 10:02:50 2018 +0100 +++ b/trytond/model/modelsql.py Wed Nov 28 10:05:01 2018 +0100 @@ -656,17 +656,10 @@ 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 -r d1dca83b5330 -r bfef7a2472b2 trytond/model/modelstorage.py --- a/trytond/model/modelstorage.py Wed Nov 28 10:02:50 2018 +0100 +++ b/trytond/model/modelstorage.py Wed Nov 28 10:05:01 2018 +0100 @@ -128,23 +128,14 @@ 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 -r d1dca83b5330 -r bfef7a2472b2 trytond/res/user.py --- a/trytond/res/user.py Wed Nov 28 10:02:50 2018 +0100 +++ b/trytond/res/user.py Wed Nov 28 10:05:01 2018 +0100 @@ -352,8 +352,8 @@ 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 -r d1dca83b5330 -r bfef7a2472b2 trytond/tests/test_access.py --- a/trytond/tests/test_access.py Wed Nov 28 10:02:50 2018 +0100 +++ b/trytond/tests/test_access.py Wed Nov 28 10:05:01 2018 +0100 @@ -259,14 +259,14 @@ 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 -r d1dca83b5330 -r bfef7a2472b2 trytond/tests/test_history.py --- a/trytond/tests/test_history.py Wed Nov 28 10:02:50 2018 +0100 +++ b/trytond/tests/test_history.py Wed Nov 28 10:05:01 2018 +0100 @@ -73,7 +73,7 @@ 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 @@ 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 @@ 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 -r d1dca83b5330 -r bfef7a2472b2 trytond/tests/test_rule.py --- a/trytond/tests/test_rule.py Wed Nov 28 10:02:50 2018 +0100 +++ b/trytond/tests/test_rule.py Wed Nov 28 10:05:01 2018 +0100 @@ -228,7 +228,7 @@ 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 @@ }]) 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 @@ 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):