Send only the required fields when fetching a readonly view definition

issue11134
review380081002

CHANGELOG

@@ -1,3 +1,6 @@
+* Use a set for field's depends
+* Relax the constraint on a field's depends
+* Include only needed fields when fetching a view definition
 * Make warning check re-entrant
 * Manage DatabaseDataError in ModelSQL
 * Remove support for pysqlite2

doc/ref/fields.rst

@@ -112,13 +112,28 @@ All are optional except :attr:`~Field.string`.
 
 .. attribute:: Field.depends
 
-   A list of field names on which the field depends.
+   A :py:class:`set` of extra field names on which the field depends.
 
    This means that the client read also these fields even if they are not
    defined on the view.
    :attr:`Field.depends` is used for example to ensure that
    :class:`~trytond.pyson.PYSON` statement could be evaluated.
 
+.. attribute:: Field.display_depends
+
+   A computed set of field names on which the field depends when being
+   displayed in a read only view.
+
+.. attribute:: Field.edition_depends
+
+   A computed set of field names on which the field depends when being
+   displayed in a writable view.
+
+.. attribute:: Field.validation_depends
+
+   A computed set of field names on which the field depends when being
+   validated.
+
 .. attribute:: Field.context
 
    A dictionary which updates the current context for *relation field*.

trytond/ir/email_.py

@@ -332,7 +332,7 @@ class EmailTemplate(ModelSQL, ModelView):
                         ],
                     ]
                 ]
-            field.depends.append('model')
+            field.depends.add('model')
         cls.__rpc__.update({
                 'get': RPC(instantiate=0),
                 'get_default': RPC(),

trytond/model/active.py

@@ -32,8 +32,6 @@ class DeactivableMixin(Model):
                 field.states['readonly'] |= inactive
             else:
                 field.states['readonly'] = inactive
-            if 'active' not in field.depends:
-                field.depends.append('active')
 
         if issubclass(cls, ModelView):
             for states in cls._buttons.values():

trytond/model/fields/date.py

@@ -1,13 +1,14 @@
 # This file is part of Tryton.  The COPYRIGHT file at the top level of
 # this repository contains the full copyright notices and license terms.
 import datetime
+from functools import cached_property
 
 from sql.functions import AtTimeZone, Function
 
 from trytond import backend
-from trytond.pyson import PYSONEncoder
+from trytond.pyson import PYSON, PYSONEncoder
 
-from .field import Field
+from .field import Field, get_eval_fields
 
 
 class SQLite_Date(Function):
@@ -59,6 +60,20 @@ class FormatMixin(Field):
         definition['format'] = encoder.encode(self.format)
         return definition
 
+    @cached_property
+    def display_depends(self):
+        depends = super().display_depends
+        if isinstance(self.format, PYSON):
+            depends |= get_eval_fields(self.format)
+        return depends
+
+    @cached_property
+    def validation_depends(self):
+        depends = super().display_depends
+        if isinstance(self.format, PYSON):
+            depends |= get_eval_fields(self.format)
+        return depends
+
 
 class Timestamp(FormatMixin, Field):
     '''

trytond/model/fields/field.py

@@ -16,6 +16,7 @@ from trytond.const import OPERATORS
 from trytond.pool import Pool
 from trytond.pyson import PYSON, Eval, PYSONDecoder, PYSONEncoder
 from trytond.rpc import RPC
+from trytond.tools import cached_property
 from trytond.tools.string_ import LazyString, StringPartitioned
 from trytond.transaction import Transaction
 
@@ -62,7 +63,7 @@ def states_validate(value):
 
 
 def depends_validate(value):
-    assert isinstance(value, list), 'depends must be a list'
+    assert isinstance(value, set), 'depends must be a set'
 
 
 def context_validate(value):
@@ -244,7 +245,7 @@ class Field(object):
         :param on_change_with: A list of values. Like ``on_change``, but
             defined the other way around. The list contains all the fields that
             must update the current field.
-        :param depends: A list of field name on which this one depends.
+        :param depends: A set of field name on which this one depends.
         :param context: A dictionary which will be given to open the relation
             fields.
         :param loading: Define how the field must be loaded:
@@ -274,7 +275,7 @@ class Field(object):
                 DeprecationWarning, stacklevel=3)
             self.on_change_with.update(on_change_with)
         self.__depends = None
-        self.depends = depends or []
+        self.depends = depends or set()
         self.__context = None
         self.context = context or {}
         assert loading in ('lazy', 'eager'), \
@@ -322,11 +323,30 @@ class Field(object):
         return self.__depends
 
     def _set_depends(self, value):
+        value = set(value)
         depends_validate(value)
         self.__depends = value
 
     depends = property(_get_depends, _set_depends)
 
+    @cached_property
+    def display_depends(self):
+        depends = get_eval_fields(self.states.get('invisible'))
+        return self.depends | depends
+
+    @cached_property
+    def edition_depends(self):
+        depends = get_eval_fields(self.domain)
+        depends |= get_eval_fields(self.states.get('readonly'))
+        depends |= get_eval_fields(self.states.get('required'))
+        return self.depends | depends
+
+    @cached_property
+    def validation_depends(self):
+        depends = get_eval_fields(self.domain)
+        depends |= get_eval_fields(self.states.get('required'))
+        return self.depends | depends
+
     def _get_context(self):
         return self.__context
 

trytond/model/fields/float.py

@@ -1,8 +1,9 @@
 # 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.pyson import PYSON, PYSONEncoder
+from trytond.tools import cached_property
 
-from .field import Field
+from .field import Field, get_eval_fields
 
 
 def digits_validate(value):
@@ -18,6 +19,13 @@ def digits_validate(value):
                         'PYSON digits must return an integer'
 
 
+def _get_digits_depends(field):
+    if isinstance(field.digits, str):
+        return {field.digits}
+    else:
+        return get_eval_fields(field.digits)
+
+
 class Float(Field):
     '''
     Define a float field (``float``).
@@ -63,3 +71,15 @@ class Float(Field):
         definition = super().definition(model, language)
         definition['digits'] = encoder.encode(self.digits)
         return definition
+
+    @cached_property
+    def display_depends(self):
+        return super().display_depends | _get_digits_depends(self)
+
+    @cached_property
+    def edition_depends(self):
+        return super().edition_depends | _get_digits_depends(self)
+
+    @cached_property
+    def validation_depends(self):
+        return super().validation_depends | _get_digits_depends(self)

trytond/model/fields/many2many.py

@@ -9,12 +9,13 @@ from sql.functions import Position, Substring
 
 from trytond.pool import Pool
 from trytond.pyson import PYSONEncoder
-from trytond.tools import grouped_slice
+from trytond.tools import cached_property, grouped_slice
 from trytond.transaction import Transaction
 
 from .field import (
-    Field, context_validate, domain_validate, instanciate_values,
-    instantiate_context, search_order_validate, size_validate)
+    Field, context_validate, domain_validate, get_eval_fields,
+    instanciate_values, instantiate_context, search_order_validate,
+    size_validate)
 from .function import Function
 
 
@@ -109,6 +110,18 @@ class Many2Many(Field):
     def add_remove(self):
         return self.domain
 
+    @cached_property
+    def edition_depends(self):
+        depends = super().edition_depends
+        depends |= get_eval_fields(self.size)
+        return depends
+
+    @cached_property
+    def validation_depends(self):
+        depends = super().validation_depends
+        depends |= get_eval_fields(self.size)
+        return depends
+
     def sql_type(self):
         return None
 

trytond/model/fields/one2many.py

@@ -9,12 +9,13 @@ from sql.functions import Position, Substring
 
 from trytond.pool import Pool
 from trytond.pyson import PYSONEncoder
-from trytond.tools import grouped_slice
+from trytond.tools import cached_property, grouped_slice
 from trytond.transaction import Transaction
 
 from .field import (
-    Field, context_validate, domain_validate, instanciate_values,
-    instantiate_context, search_order_validate, size_validate)
+    Field, context_validate, domain_validate, get_eval_fields,
+    instanciate_values, instantiate_context, search_order_validate,
+    size_validate)
 from .function import Function
 
 
@@ -107,6 +108,17 @@ class One2Many(Field):
         context_validate(value)
         self.__search_context = value
 
+    @cached_property
+    def edition_depends(self):
+        depends = super().edition_depends
+        for attribute in ['add_remove', 'size']:
+            depends |= get_eval_fields(getattr(self, attribute))
+        return depends
+
+    @cached_property
+    def validation_depends(self):
+        return super().validation_depends | get_eval_fields(self.size)
+
     def sql_type(self):
         return None
 

trytond/model/fields/reference.py

@@ -9,10 +9,11 @@ from trytond.pool import Pool
 from trytond.pyson import PYSONEncoder
 from trytond.rpc import RPC
 from trytond.transaction import Transaction
+from trytond.tools import cached_property
 
 from .field import (
-    Field, context_validate, domain_validate, instantiate_context,
-    search_order_validate, with_inactive_records)
+    Field, context_validate, domain_validate, get_eval_fields,
+    instantiate_context, search_order_validate, with_inactive_records)
 from .selection import SelectionMixin
 
 
@@ -39,11 +40,6 @@ class Reference(SelectionMixin, Field):
         :param search_order: The order to use when searching for a record
         :param search_context: The context to use when searching for a record
         '''
-        if datetime_field:
-            if depends:
-                depends.append(datetime_field)
-            else:
-                depends = [datetime_field]
         super(Reference, self).__init__(string=string, help=help,
             required=required, readonly=readonly, domain=domain, states=states,
             select=select, on_change=on_change, on_change_with=on_change_with,
@@ -101,6 +97,13 @@ class Reference(SelectionMixin, Field):
         context_validate(value)
         self.__search_context = value
 
+    @cached_property
+    def display_depends(self):
+        depends = super().display_depends
+        if self.datetime_field:
+            depends.add(self.datetime_field)
+        return depends
+
     def set_rpc(self, model):
         super(Reference, self).set_rpc(model)
         if not isinstance(self.selection, (list, tuple)):

trytond/model/modelstorage.py

@@ -1320,11 +1320,10 @@ class ModelStorage(Model):
         ctx_pref['active_test'] = False
         with Transaction().set_context(ctx_pref):
             for field_name, field in cls._fields.items():
-                depends = set(field.depends)
                 if (field_names
                         and field_name not in field_names
-                        and not (depends & field_names)
-                        and not (depends & function_fields)):
+                        and not (field.validation_depends & field_names)
+                        and not (field.validation_depends & function_fields)):
                     continue
                 if isinstance(field, fields.Function):
                     continue
@@ -1656,7 +1655,7 @@ class ModelStorage(Model):
             if field.context:
                 eval_fields = fields.get_eval_fields(field.context)
                 for context_field_name in eval_fields:
-                    if context_field_name not in field.depends:
+                    if context_field_name not in field.validation_depends:
                         continue
                     if context_field_name not in ffields:
                         context_field = self._fields.get(context_field_name)

trytond/model/modelview.py

@@ -313,6 +313,12 @@ class ModelView(Model):
         ModelAccess = pool.get('ir.model.access')
         FieldAccess = pool.get('ir.model.field.access')
 
+        tree_root = tree.getroottree().getroot()
+        readonly_view = (
+            tree_root.tag == 'board'
+            or (tree_root.tag == 'tree'
+                and not int(tree_root.attrib.get('editable', '0'))))
+
         encoder = PYSONEncoder()
         if view_depends is None:
             view_depends = []
@@ -320,6 +326,8 @@ class ModelView(Model):
             view_depends = view_depends.copy()
         with Transaction().set_context(view_id=view_id):
             for xpath, attribute, value, *extra in cls.view_attributes():
+                if readonly_view and attribute in {'required', 'readonly'}:
+                    continue
                 depends = []
                 if extra:
                     depends, = extra
@@ -426,13 +434,17 @@ class ModelView(Model):
                 fields_def.setdefault(depend, {'name': depend})
 
         field_names = list(fields_def.keys())
-        while field_names:
-            field_name = field_names.pop()
-            if field_name in cls._fields:
-                field = cls._fields[field_name]
-            else:
+
+        for field_name in field_names:
+            if field_name not in cls._fields:
                 continue
-            for depend in field.depends:
+            field = cls._fields[field_name]
+            field_depends = field.display_depends.copy()
+            if not readonly_view:
+                field_depends |= field.edition_depends
+            if 'context' in field_depends and 'context' not in cls._fields:
+                field_depends.discard('context')
+            for depend in field_depends:
                 if depend not in fields_def:
                     fields_def[depend] = {'name': depend}
                     field_names.append(depend)

trytond/tests/modelview.py

@@ -260,6 +260,21 @@ class ModelViewViewAttributesDepends(ModelView):
             ]
 
 
+class ModelViewStatesDepends(ModelView):
+    "ModelView States Depends"
+    __name__ = 'test.modelview.states_depends'
+
+    foo = fields.Char("Foo",
+        states={
+            'invisible': Eval('bar', True),
+            'readonly': Eval('baz', True),
+            },
+        depends=['quux'])
+    bar = fields.Boolean("Bar")
+    baz = fields.Boolean("Baz")
+    quux = fields.Char("Quux")
+
+
 def register(module):
     Pool.register(
         ModelViewChangedValues,
@@ -280,4 +295,5 @@ def register(module):
         ModeViewDependsDepends,
         ModelViewViewAttributes,
         ModelViewViewAttributesDepends,
+        ModelViewStatesDepends,
         module=module, type_='model')

trytond/tests/modelview.xml

@@ -87,4 +87,28 @@
             ]]>
         </field>
     </record>
+
+    <record model="ir.ui.view" id="test_modelview_states_depends_tree">
+        <field name="model">test.modelview.states_depends</field>
+        <field name="type">tree</field>
+        <field name="arch" type="xml">
+            <![CDATA[
+            <tree>
+                <field name="foo"/>
+            </tree>
+            ]]>
+        </field>
+    </record>
+    <record model="ir.ui.view" id="test_modelview_states_depends_form">
+        <field name="model">test.modelview.states_depends</field>
+        <field name="type">form</field>
+        <field name="arch" type="xml">
+            <![CDATA[
+            <form>
+                <label name="foo"/>
+                <field name="foo"/>
+            </form>
+            ]]>
+        </field>
+    </record>
 </tryton>

trytond/tests/test_modelview.py

@@ -668,6 +668,28 @@ class ModelView(unittest.TestCase):
 
         self.assertIn('bar', fields)
 
+    @with_transaction()
+    def test_states_depends_display(self):
+        "Test that fields used in readonly states are included"
+        pool = Pool()
+        Depends = pool.get('test.modelview.states_depends')
+
+        readonly_fields = Depends.fields_view_get(view_type='tree')['fields']
+        self.assertIn('bar', readonly_fields)
+        self.assertNotIn('baz', readonly_fields)
+        self.assertIn('quux', readonly_fields)
+
+    @with_transaction()
+    def test_states_depends_edition(self):
+        "Test that fields used in editable states are included"
+        pool = Pool()
+        Depends = pool.get('test.modelview.states_depends')
+
+        writeable_fields = Depends.fields_view_get(view_type='form')['fields']
+        self.assertIn('bar', writeable_fields)
+        self.assertIn('baz', writeable_fields)
+        self.assertIn('quux', writeable_fields)
+
 
 def suite():
     func = unittest.TestLoader().loadTestsFromTestCase

trytond/tests/test_tryton.py

@@ -343,26 +343,9 @@ class ModuleTestCase(unittest.TestCase):
             if not isregisteredby(model, self.module):
                 continue
             for fname, field in model._fields.items():
-                fields = set()
-                fields |= get_eval_fields(field.domain)
-                if hasattr(field, 'digits'):
-                    if isinstance(field.digits, str):
-                        fields.add(field.digits)
-                    else:
-                        fields |= get_eval_fields(field.digits)
-                if hasattr(field, 'add_remove'):
-                    fields |= get_eval_fields(field.add_remove)
-                if hasattr(field, 'size'):
-                    fields |= get_eval_fields(field.size)
-                fields.discard(fname)
-                fields.discard('context')
-                fields.discard('active_model')
-                fields.discard('active_id')
-                depends = set(field.depends)
+                depends = {
+                    f for f in field.depends if not f.startswith('_parent_')}
                 with self.subTest(model=mname, field=fname):
-                    self.assertLessEqual(fields, depends,
-                        msg='Missing depends %s in "%s"."%s"' % (
-                            list(fields - depends), mname, fname))
                     self.assertLessEqual(depends, set(model._fields),
                         msg='Unknown depends %s in "%s"."%s"' % (
                             list(depends - set(model._fields)), mname, fname))