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):

Reply via email to