From 905343c900b5480f434215f2d20d4e446f26123d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Krier?= Date: Sun, 26 Jan 2014 14:27:24 +0100 Subject: [PATCH] Add fields.depends decorator issue3487 review2301002 --- CHANGELOG | 3 ++ doc/ref/models/fields.rst | 25 ++++++++--- trytond/ir/attachment.py | 4 +- trytond/ir/trigger.py | 16 +++---- trytond/model/fields/char.py | 23 +++------- trytond/model/fields/field.py | 71 +++++++++++++++++-------------- trytond/model/fields/selection.py | 9 +++- trytond/model/model.py | 53 ++++++++++++++++++++--- trytond/model/modelstorage.py | 7 ++- trytond/tests/test.py | 3 +- 10 files changed, 142 insertions(+), 72 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 87d32ba8ee..459c305ae9 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,6 @@ +* Deprecate on_change, on_change_with, selection_change_with and autocomplete + field arguments +* Add fields.depends decorator * Add run-tests * Validate only modified and dependant fields on model write * Improve error messages by showing the failing value diff --git a/doc/ref/models/fields.rst b/doc/ref/models/fields.rst index a69a0a35be..702c60d051 100644 --- a/doc/ref/models/fields.rst +++ b/doc/ref/models/fields.rst @@ -77,7 +77,7 @@ If true, the content of the field will be indexed. .. attribute:: Field.on_change -A list of field names. If this attribute is set, the client will call the +A set of field names. If this attribute is set, the client will call the method ``on_change_`` of the model when the user changes the current field value and will give the values of each fields in this list. The method signature is:: @@ -88,15 +88,17 @@ This method must return a dictionary with the values of fields to be updated. .. note:: - The on_change_ methods are runnin in a rollbacked transaction. + The on_change_ methods are running in a rollbacked transaction. .. +The set of field names could be filled by using the decorator :meth:`depends`. + ``on_change_with`` ------------------ .. attribute:: Field.on_change_with -A list of field names. Same like :attr:`on_change`, but defined the other way +A set of field names. Same like :attr:`on_change`, but defined the other way around. If this attribute is set, the client will call the method ``on_change_with_`` of the model when the user changes one of the fields defined in the list and will give the values of each fields in this @@ -112,6 +114,8 @@ This method must return the new value of the field. .. +The set of field names could be filled by using the decorator :meth:`depends`. + ``depends`` ----------- @@ -189,6 +193,15 @@ The method signature is:: Where ``tables`` is a nested dictionary, see :meth:`~Field.convert_domain`. +Depends +======= + +.. method:: depends([\*fields[, methods]]) + +A decorator to define the field names on which the decorated method depends. +The `methods` argument can be used to duplicate the field names from other +fields. This is usefull if the decorated method calls another method. + Field types =========== @@ -234,7 +247,7 @@ A single line string field. .. attribute:: Char.autocomplete - A list of field names. If this attribute is set, the client will call the + A set of field names. If this attribute is set, the client will call the method ``autocomplete_`` of the model when the user changes one of those field value. The method signature is:: @@ -242,6 +255,7 @@ A single line string field. This method must return a list of string that will populate the ComboboxEntry in the client. + The set of field names could be filled by using the decorator :meth:`depends`. Sha --- @@ -395,10 +409,11 @@ A string field with limited values to choice. .. attribute:: Selection.selection_change_with - A list of field names. If this attribute is set, the client will call the + A set of field names. If this attribute is set, the client will call the ``selection`` method of the model when the user changes on of the fields defined in the list and will give the values of each fields in the list. The ``selection`` method should be an instance method. + The set of field names could be filled by using the decorator :meth:`depends`. .. attribute:: Selection.translate_selection diff --git a/trytond/ir/attachment.py b/trytond/ir/attachment.py index dbb982e800..b4b928d0ab 100644 --- a/trytond/ir/attachment.py +++ b/trytond/ir/attachment.py @@ -39,8 +39,7 @@ class Attachment(ModelSQL, ModelView): 'invisible': Eval('type') != 'data', }, depends=['type']), 'get_data', setter='set_data') description = fields.Text('Description') - summary = fields.Function(fields.Char('Summary', - on_change_with=['description']), 'on_change_with_summary') + summary = fields.Function(fields.Char('Summary'), 'on_change_with_summary') resource = fields.Reference('Resource', selection='models_get', select=True) link = fields.Char('Link', states={ @@ -191,6 +190,7 @@ class Attachment(ModelSQL, ModelView): 'collision': collision, }) + @fields.depends('description') def on_change_with_summary(self, name=None): return firstline(self.description or '') diff --git a/trytond/ir/trigger.py b/trytond/ir/trigger.py index a821ca0fd0..739df0c9fa 100644 --- a/trytond/ir/trigger.py +++ b/trytond/ir/trigger.py @@ -29,20 +29,16 @@ class Trigger(ModelSQL, ModelView): 'invisible': (Eval('on_create', False) or Eval('on_write', False) or Eval('on_delete', False)), - }, depends=['on_create', 'on_write', 'on_delete'], - on_change=['on_time']) + }, depends=['on_create', 'on_write', 'on_delete']) on_create = fields.Boolean('On Create', select=True, states={ 'invisible': Eval('on_time', False), - }, depends=['on_time'], - on_change=['on_create']) + }, depends=['on_time']) on_write = fields.Boolean('On Write', select=True, states={ 'invisible': Eval('on_time', False), - }, depends=['on_time'], - on_change=['on_write']) + }, depends=['on_time']) on_delete = fields.Boolean('On Delete', select=True, states={ 'invisible': Eval('on_time', False), - }, depends=['on_time'], - on_change=['on_delete']) + }, depends=['on_time']) condition = fields.Char('Condition', required=True, help='A Python statement evaluated with record represented by ' '"self"\nIt triggers the action if true.') @@ -97,6 +93,7 @@ class Trigger(ModelSQL, ModelView): def default_limit_number(): return 0 + @fields.depends('on_time') def on_change_on_time(self): if self.on_time: return { @@ -106,6 +103,7 @@ class Trigger(ModelSQL, ModelView): } return {} + @fields.depends('on_create') def on_change_on_create(self): if self.on_create: return { @@ -113,6 +111,7 @@ class Trigger(ModelSQL, ModelView): } return {} + @fields.depends('on_write') def on_change_on_write(self): if self.on_write: return { @@ -120,6 +119,7 @@ class Trigger(ModelSQL, ModelView): } return {} + @fields.depends('on_delete') def on_change_on_delete(self): if self.on_delete: return { diff --git a/trytond/model/fields/char.py b/trytond/model/fields/char.py index 64f1c1f478..1168cac80b 100644 --- a/trytond/model/fields/char.py +++ b/trytond/model/fields/char.py @@ -1,16 +1,13 @@ #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 warnings + from sql import Query, Expression from ...config import CONFIG from .field import Field, FieldTranslate, size_validate, SQLType -def autocomplete_validate(value): - if value: - assert isinstance(value, list), 'autocomplete must be a list' - - class Char(FieldTranslate): ''' Define a char field (``unicode``). @@ -31,22 +28,16 @@ class Char(FieldTranslate): readonly=readonly, domain=domain, states=states, select=select, on_change=on_change, on_change_with=on_change_with, depends=depends, context=context, loading=loading) - self.__autocomplete = None - self.autocomplete = autocomplete if autocomplete else None + self.autocomplete = set() + if autocomplete: + warnings.warn('autocomplete argument is deprecated, use the ' + 'depends decorator', DeprecationWarning, stacklevel=2) + self.autocomplete |= set(autocomplete) self.translate = translate self.__size = None self.size = size __init__.__doc__ += Field.__init__.__doc__ - def _get_autocomplete(self): - return self.__autocomplete - - def _set_autocomplete(self, value): - autocomplete_validate(value) - self.__autocomplete = value - - autocomplete = property(_get_autocomplete, _set_autocomplete) - def _get_size(self): return self.__size diff --git a/trytond/model/fields/field.py b/trytond/model/fields/field.py index aa64ca2a23..fc4c18577a 100644 --- a/trytond/model/fields/field.py +++ b/trytond/model/fields/field.py @@ -1,6 +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 collections import namedtuple +import warnings +from functools import wraps + from sql import operators, Column, Literal, Select, CombiningQuery from sql.conditionals import Coalesce, NullIf from sql.operators import Concat @@ -45,16 +48,6 @@ def states_validate(value): 'values of states must return boolean' -def on_change_validate(value): - if value: - assert isinstance(value, list), 'on_change must be a list' - - -def on_change_with_validate(value): - if value: - assert isinstance(value, list), 'on_change_with must be a list' - - def depends_validate(value): assert isinstance(value, list), 'depends must be a list' @@ -70,6 +63,31 @@ def size_validate(value): assert value.types() == set([int]), \ 'size must return integer' + +def depends(*fields, **kwargs): + methods = kwargs.pop('methods', None) + assert not kwargs + + def decorator(func): + depends = getattr(func, 'depends', set()) + depends |= set(fields) + setattr(func, 'depends', depends) + + if methods: + depend_methods = getattr(func, 'depend_methods', set()) + depend_methods |= set(methods) + setattr(func, 'depend_methods', depend_methods) + + @wraps(func) + def wrapper(self, *args, **kwargs): + for field in fields: + if not hasattr(self, field): + setattr(self, field, None) + return func(self, *args, **kwargs) + return wrapper + return decorator + + SQL_OPERATORS = { '=': operators.Equal, '!=': operators.NotEqual, @@ -128,10 +146,18 @@ class Field(object): self.__states = None self.states = states or {} self.select = bool(select) - self.__on_change = None - self.on_change = on_change - self.__on_change_with = None - self.on_change_with = on_change_with + self.on_change = set() + if on_change: + warnings.warn('on_change argument is deprecated, ' + 'use the depends decorator', + DeprecationWarning, stacklevel=3) + self.on_change |= set(on_change) + self.on_change_with = set() + if on_change_with: + warnings.warn('on_change_with argument is deprecated, ' + 'use the depends decorator', + DeprecationWarning, stacklevel=3) + self.on_change_with |= set(on_change_with) self.__depends = None self.depends = depends or [] self.__context = None @@ -159,23 +185,6 @@ class Field(object): states = property(_get_states, _set_states) - def _get_on_change(self): - return self.__on_change - - def _set_on_change(self, value): - on_change_validate(value) - self.__on_change = value - on_change = property(_get_on_change, _set_on_change) - - def _get_on_change_with(self): - return self.__on_change_with - - def _set_on_change_with(self, value): - on_change_with_validate(value) - self.__on_change_with = value - - on_change_with = property(_get_on_change_with, _set_on_change_with) - def _get_depends(self): return self.__depends diff --git a/trytond/model/fields/selection.py b/trytond/model/fields/selection.py index ba44c00a6e..e4b8f411aa 100644 --- a/trytond/model/fields/selection.py +++ b/trytond/model/fields/selection.py @@ -1,5 +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. +import warnings + from sql import Column from sql.conditionals import Case @@ -32,7 +34,12 @@ class Selection(Field): self.selection = selection.copy() else: self.selection = selection - self.selection_change_with = selection_change_with + self.selection_change_with = set() + if selection_change_with: + warnings.warn('selection_change_with argument is deprecated, ' + 'use the depends decorator', + DeprecationWarning, stacklevel=2) + self.selection_change_with |= set(selection_change_with) self.sort = sort self.translate_selection = translate __init__.__doc__ += Field.__init__.__doc__ diff --git a/trytond/model/model.py b/trytond/model/model.py index 60034650c6..8c927c0a55 100644 --- a/trytond/model/model.py +++ b/trytond/model/model.py @@ -35,12 +35,39 @@ class Model(WarningErrorMixin, URLMixin, PoolBase): } cls._error_messages = {} - # Copy fields + if hasattr(cls, '__depend_methods'): + cls.__depend_methods = cls.__depend_methods.copy() + else: + cls.__depend_methods = collections.defaultdict(set) + + # Copy fields and update depends for attr in dir(cls): if attr.startswith('_'): continue - if isinstance(getattr(cls, attr), fields.Field): - setattr(cls, attr, copy.deepcopy(getattr(cls, attr))) + if not isinstance(getattr(cls, attr), fields.Field): + continue + field_name = attr + field = copy.deepcopy(getattr(cls, field_name)) + setattr(cls, field_name, field) + + for attribute in ('on_change', 'on_change_with', 'autocomplete', + 'selection_change_with'): + if attribute == 'selection_change_with': + if (isinstance(field, fields.Selection) + and isinstance(field.selection, basestring)): + function_name = field.selection + else: + continue + else: + function_name = '%s_%s' % (attribute, field_name) + function = getattr(cls, function_name, None) + if function: + if getattr(function, 'depends', None): + setattr(field, attribute, + getattr(field, attribute) | function.depends) + if getattr(function, 'depend_methods', None): + cls.__depend_methods[(field_name, attribute)] |= \ + function.depend_methods @classmethod def __post_setup__(cls): @@ -78,9 +105,19 @@ class Model(WarningErrorMixin, URLMixin, PoolBase): for attribute in ('on_change', 'on_change_with', 'autocomplete'): function_name = '%s_%s' % (attribute, field_name) - if getattr(field, attribute, False): + if getattr(cls, function_name, None): cls.__rpc__.setdefault(function_name, RPC(instantiate=0)) + # Update depend on methods + for (field_name, attribute), others in ( + cls.__depend_methods.iteritems()): + field = getattr(cls, field_name) + for other in others: + other_field = getattr(cls, other) + setattr(field, attribute, + getattr(field, attribute) + | getattr(other_field, attribute)) + # Set name to fields for name, field in cls._fields.iteritems(): if field.name is None: @@ -238,8 +275,12 @@ class Model(WarningErrorMixin, URLMixin, PoolBase): 'selection_change_with', ): if getattr(cls._fields[field], arg, None) is not None: - res[field][arg] = copy.copy(getattr(cls._fields[field], - arg)) + value = getattr(cls._fields[field], arg) + if isinstance(value, set): + value = list(value) + else: + value = copy.copy(value) + res[field][arg] = value if not accesses.get(field, {}).get('write', True): res[field]['readonly'] = True if res[field].get('states') and \ diff --git a/trytond/model/modelstorage.py b/trytond/model/modelstorage.py index 463be4222d..9e8d8e2d7b 100644 --- a/trytond/model/modelstorage.py +++ b/trytond/model/modelstorage.py @@ -1167,15 +1167,16 @@ class ModelStorage(Model): def __init__(self, id=None, **kwargs): _ids = kwargs.pop('_ids', None) _local_cache = kwargs.pop('_local_cache', None) - super(ModelStorage, self).__init__(id, **kwargs) self._cursor = Transaction().cursor self._user = Transaction().user self._context = Transaction().context + if id is not None: + id = int(id) if _ids is not None: self._ids = _ids assert id in _ids else: - self._ids = [self.id] + self._ids = [id] self._cursor_cache = self._cursor.get_cache(self._context) @@ -1185,6 +1186,8 @@ class ModelStorage(Model): self._local_cache = LRUDict(RECORD_CACHE_SIZE) self._local_cache.counter = Transaction().counter + super(ModelStorage, self).__init__(id, **kwargs) + @property def _cache(self): cache = self._cursor_cache diff --git a/trytond/tests/test.py b/trytond/tests/test.py index 6efeba4725..e95810ad33 100644 --- a/trytond/tests/test.py +++ b/trytond/tests/test.py @@ -625,10 +625,11 @@ class Selection(ModelSQL): ('', ''), ('arabic', 'Arabic'), ('hexa', 'Hexadecimal')], 'Selection') dyn_select = fields.Selection('get_selection', - 'Instance Dynamic Selection', selection_change_with=['select']) + 'Instance Dynamic Selection') dyn_select_static = fields.Selection('static_selection', 'Static Selection') + @fields.depends('select') def get_selection(self): if self.select == 'arabic': return [('', '')] + [(str(i), str(i)) for i in range(1, 11)]