From e6d4a635612feb2370b6563f94cd0eef4f551870 Mon Sep 17 00:00:00 2001 From: Raphael Collet Date: Thu, 9 Jul 2015 17:31:11 +0200 Subject: [PATCH 01/10] [IMP] fields: add attribute `origin` to avoid confusion with `column` Making a distinction between the column from which the field was created, and the one created from the field, allows to keep the second one from a registry setup to the next one. This opens a door for optimizations: the column may not need to be recomputed. --- openerp/fields.py | 15 +++++++++------ openerp/osv/fields.py | 2 +- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/openerp/fields.py b/openerp/fields.py index a0d3f59d4b7..0a6f858d5c4 100644 --- a/openerp/fields.py +++ b/openerp/fields.py @@ -262,6 +262,7 @@ class Field(object): 'automatic': False, # whether the field is automatically created ("magic" field) 'inherited': False, # whether the field is inherited (_inherits) + 'origin': None, # the column from which the field was created 'column': None, # the column corresponding to the field 'setup_done': False, # whether the field has been set up @@ -362,7 +363,7 @@ class Field(object): attrs['copy'] = attrs.get('copy', False) # fix for function fields overridden by regular columns - if not isinstance(attrs.get('column'), (NoneType, fields.function)): + if not isinstance(attrs.get('origin'), (NoneType, fields.function)): attrs.pop('store', None) for attr, value in attrs.iteritems(): @@ -670,9 +671,11 @@ class Field(object): def to_column(self): """ Return a column object corresponding to ``self``, or ``None``. """ - if not self.store and (self.compute or not self.column): + if self.column: + return self.column + + if not self.store and (self.compute or not self.origin): # non-stored computed fields do not have a corresponding column - self.column = None return None # determine column parameters @@ -688,9 +691,9 @@ class Field(object): args['type'] = self.type args['relation'] = self.comodel_name self.column = fields.property(**args) - elif self.column: - # let the column provide a valid column for the given parameters - self.column = self.column.new(_computed_field=bool(self.compute), **args) + elif self.origin: + # let the origin provide a valid column for the given parameters + self.column = self.origin.new(_computed_field=bool(self.compute), **args) else: # create a fresh new column of the right type self.column = getattr(fields, self.type)(**args) diff --git a/openerp/osv/fields.py b/openerp/osv/fields.py index e7ab8856386..04707788109 100644 --- a/openerp/osv/fields.py +++ b/openerp/osv/fields.py @@ -187,7 +187,7 @@ class _column(object): def to_field(self): """ convert column `self` to a new-style field """ from openerp.fields import Field - return Field.by_type[self._type](column=self, **self.to_field_args()) + return Field.by_type[self._type](origin=self, **self.to_field_args()) def to_field_args(self): """ return a dictionary with all the arguments to pass to the field """ From a4eaa86e440c9c5cf7ce4b3c84d98b33f70122ff Mon Sep 17 00:00:00 2001 From: Raphael Collet Date: Fri, 10 Jul 2015 11:19:27 +0200 Subject: [PATCH 02/10] [IMP] fields: split attribute `_attrs` into `args` and `_attrs` Make an explicit distinction between both semantics of `_attrs`: - the parameters given to `__init__` in `args` - the non-slot attributes (memory optimization) in `_attrs` --- openerp/addons/base/ir/ir_model.py | 2 +- openerp/fields.py | 48 ++++++++++++++++-------------- 2 files changed, 27 insertions(+), 23 deletions(-) diff --git a/openerp/addons/base/ir/ir_model.py b/openerp/addons/base/ir/ir_model.py index f7fea3a28fd..b0d7dee0173 100644 --- a/openerp/addons/base/ir/ir_model.py +++ b/openerp/addons/base/ir/ir_model.py @@ -491,7 +491,7 @@ class ir_model_fields(osv.osv): for field_name, field_patches in model_patches.iteritems(): # update field properties, and adapt corresponding column field = obj._fields[field_name] - attrs = dict(field._attrs, **field_patches) + attrs = dict(field.args, **field_patches) obj._add_field(field_name, field.new(**attrs)) # update database schema diff --git a/openerp/fields.py b/openerp/fields.py index 0a6f858d5c4..a00d0c19b46 100644 --- a/openerp/fields.py +++ b/openerp/fields.py @@ -256,9 +256,8 @@ class Field(object): relational = False # whether the field is a relational one _slots = { - '_attrs': EMPTY_DICT, # dictionary of field attributes; it contains: - # - all attributes after __init__() - # - free attributes only after set_class_name() + 'args': EMPTY_DICT, # the parameters given to __init__() + '_attrs': EMPTY_DICT, # the field's non-slot attributes 'automatic': False, # whether the field is automatically created ("magic" field) 'inherited': False, # whether the field is inherited (_inherits) @@ -302,8 +301,8 @@ class Field(object): def __init__(self, string=None, **kwargs): kwargs['string'] = string - attrs = {key: val for key, val in kwargs.iteritems() if val is not None} - self._attrs = attrs or EMPTY_DICT + args = {key: val for key, val in kwargs.iteritems() if val is not None} + self.args = args or EMPTY_DICT def __getattr__(self, name): """ Access non-slot field attribute. """ @@ -322,6 +321,15 @@ class Field(object): else: self._attrs = {name: value} # replace EMPTY_DICT + def set_all_attrs(self, attrs): + """ Set all field attributes at once (with slot defaults). """ + # optimization: we assign slots only + assign = object.__setattr__ + for key, val in self._slots.iteritems(): + assign(self, key, attrs.pop(key, val)) + if attrs: + assign(self, '_attrs', attrs) + def __delattr__(self, name): """ Remove non-slot field attribute. """ try: @@ -335,21 +343,18 @@ class Field(object): def set_class_name(self, cls, name): """ Assign the model class and field name of ``self``. """ - self_attrs = self._attrs - for attr, value in self._slots.iteritems(): - setattr(self, attr, value) - - self.model_name = cls._name - self.name = name - # determine all inherited field attributes attrs = {} for field in resolve_all_mro(cls, name, reverse=True): if isinstance(field, type(self)): - attrs.update(field._attrs) + attrs.update(field.args) else: attrs.clear() - attrs.update(self_attrs) # necessary in case self is not in cls + attrs.update(self.args) # necessary in case self is not in cls + + attrs['args'] = self.args + attrs['model_name'] = cls._name + attrs['name'] = name # initialize ``self`` with ``attrs`` if attrs.get('compute'): @@ -366,8 +371,7 @@ class Field(object): if not isinstance(attrs.get('origin'), (NoneType, fields.function)): attrs.pop('store', None) - for attr, value in attrs.iteritems(): - setattr(self, attr, value) + self.set_all_attrs(attrs) if not self.string and not self.related: # related fields get their string from their parent field @@ -391,9 +395,9 @@ class Field(object): # klass contains another value overridden by self return - if 'default' in field._attrs: + if 'default' in field.args: # take the default in field, and adapt it for cls._defaults - value = field._attrs['default'] + value = field.args['default'] if callable(value): from openerp import api self.default = value @@ -1412,11 +1416,11 @@ class Selection(Field): if isinstance(field, type(self)): # We cannot use field.selection or field.selection_add here # because those attributes are overridden by ``set_class_name``. - if 'selection' in field._attrs: - self.selection = field._attrs['selection'] - if 'selection_add' in field._attrs: + if 'selection' in field.args: + self.selection = field.args['selection'] + if 'selection_add' in field.args: # use an OrderedDict to update existing values - selection_add = field._attrs['selection_add'] + selection_add = field.args['selection_add'] self.selection = OrderedDict(self.selection + selection_add).items() else: self.selection = None From d46e9f443311f97e6d73551db6284a250b71543e Mon Sep 17 00:00:00 2001 From: Raphael Collet Date: Fri, 10 Jul 2015 14:21:14 +0200 Subject: [PATCH 03/10] [IMP] models: convert some class methods to `@api.model` methods Converting the following class methods to model methods will make them more flexible for changes, in particular by making the environment available: - `_add_field` - `_pop_field` - `_add_magic_fields` - `_init_manual_fields` (renamed to `_add_manual_fields`) - `_inherits_check` - `_init_inherited_fields` (renamed to `_add_inherited_fields`) --- openerp/addons/base/ir/ir_model.py | 8 +-- openerp/models.py | 88 +++++++++++++++--------------- 2 files changed, 49 insertions(+), 47 deletions(-) diff --git a/openerp/addons/base/ir/ir_model.py b/openerp/addons/base/ir/ir_model.py index b0d7dee0173..8fafc60c1c9 100644 --- a/openerp/addons/base/ir/ir_model.py +++ b/openerp/addons/base/ir/ir_model.py @@ -305,7 +305,7 @@ class ir_model_fields(osv.osv): if field.state == 'manual' and field.ttype == 'many2many': rel_name = model._fields[field.name].relation cr.execute('DROP table "%s"' % (rel_name)) - model._pop_field(field.name) + model._pop_field(cr, uid, field.name, context=context) return True @@ -472,8 +472,8 @@ class ir_model_fields(osv.osv): cr.execute('ALTER TABLE "%s" RENAME COLUMN "%s" TO "%s"' % rename) # This is VERY risky, but let us have this feature: # we want to change the key of field in obj._fields and obj._columns - field = obj._pop_field(rename[1]) - obj._add_field(rename[2], field) + field = obj._pop_field(cr, user, rename[1], context=context) + obj._add_field(cr, user, rename[2], field, context=context) self.pool.setup_models(cr, partial=(not self.pool.ready)) if patches: @@ -492,7 +492,7 @@ class ir_model_fields(osv.osv): # update field properties, and adapt corresponding column field = obj._fields[field_name] attrs = dict(field.args, **field_patches) - obj._add_field(field_name, field.new(**attrs)) + obj._add_field(cr, user, field_name, field.new(**attrs), context=context) # update database schema self.pool.setup_models(cr, partial=(not self.pool.ready)) diff --git a/openerp/models.py b/openerp/models.py index 3d6b9b27880..c851e53ed2d 100644 --- a/openerp/models.py +++ b/openerp/models.py @@ -435,9 +435,10 @@ class BaseModel(object): break self.invalidate_cache(cr, SUPERUSER_ID) - @classmethod - def _add_field(cls, name, field): + @api.model + def _add_field(self, name, field): """ Add the given ``field`` under the given ``name`` in the class """ + cls = type(self) # add field as an attribute and in cls._fields (for reflection) if not isinstance(getattr(cls, name, field), Field): _logger.warning("In model %r, field %r overriding existing value", cls._name, name) @@ -449,19 +450,20 @@ class BaseModel(object): # cls._columns will be updated once fields are set up - @classmethod - def _pop_field(cls, name): + @api.model + def _pop_field(self, name): """ Remove the field with the given ``name`` from the model. This method should only be used for manual fields. """ + cls = type(self) field = cls._fields.pop(name) cls._columns.pop(name, None) if hasattr(cls, name): delattr(cls, name) return field - @classmethod - def _add_magic_fields(cls): + @api.model + def _add_magic_fields(self): """ Introduce magic fields on the current class * id is a "normal" field (with a specific getter) @@ -482,19 +484,19 @@ class BaseModel(object): """ def add(name, field): """ add ``field`` with the given ``name`` if it does not exist yet """ - if name not in cls._fields: - cls._add_field(name, field) + if name not in self._fields: + self._add_field(name, field) # cyclic import from . import fields # this field 'id' must override any other column or field - cls._add_field('id', fields.Id(automatic=True)) + self._add_field('id', fields.Id(automatic=True)) add('display_name', fields.Char(string='Display Name', automatic=True, compute='_compute_display_name')) - if cls._log_access: + if self._log_access: add('create_uid', fields.Many2one('res.users', string='Created by', automatic=True)) add('create_date', fields.Datetime(string='Created on', automatic=True)) add('write_uid', fields.Many2one('res.users', string='Last Updated by', automatic=True)) @@ -504,7 +506,7 @@ class BaseModel(object): last_modified_name = 'compute_concurrency_field' # this field must override any other column or field - cls._add_field(cls.CONCURRENCY_CHECK_FIELD, fields.Datetime( + self._add_field(self.CONCURRENCY_CHECK_FIELD, fields.Datetime( string='Last Modified on', compute=last_modified_name, automatic=True)) @api.one @@ -655,12 +657,12 @@ class BaseModel(object): pool._store_function[model].append(t) pool._store_function[model].sort(key=lambda x: x[4]) - @classmethod - def _init_manual_fields(cls, cr, partial): - manual_fields = cls.pool.get_manual_fields(cr, cls._name) + @api.model + def _add_manual_fields(self, partial): + manual_fields = self.pool.get_manual_fields(self._cr, self._name) for name, field in manual_fields.iteritems(): - if name in cls._fields: + if name in self._fields: continue attrs = { 'manual': True, @@ -675,23 +677,23 @@ class BaseModel(object): elif field['ttype'] in ('selection', 'reference'): attrs['selection'] = eval(field['selection']) elif field['ttype'] == 'many2one': - if partial and field['relation'] not in cls.pool: + if partial and field['relation'] not in self.pool: continue attrs['comodel_name'] = field['relation'] attrs['ondelete'] = field['on_delete'] attrs['domain'] = eval(field['domain']) if field['domain'] else None elif field['ttype'] == 'one2many': if partial and not ( - field['relation'] in cls.pool and ( - field['relation_field'] in cls.pool[field['relation']]._fields or - field['relation_field'] in cls.pool.get_manual_fields(cr, field['relation']) + field['relation'] in self.pool and ( + field['relation_field'] in self.pool[field['relation']]._fields or + field['relation_field'] in self.pool.get_manual_fields(self._cr, field['relation']) )): continue attrs['comodel_name'] = field['relation'] attrs['inverse_name'] = field['relation_field'] attrs['domain'] = eval(field['domain']) if field['domain'] else None elif field['ttype'] == 'many2many': - if partial and field['relation'] not in cls.pool: + if partial and field['relation'] not in self.pool: continue attrs['comodel_name'] = field['relation'] _rel1 = field['relation'].replace('.', '_') @@ -700,7 +702,7 @@ class BaseModel(object): attrs['column1'] = 'id1' attrs['column2'] = 'id2' attrs['domain'] = eval(field['domain']) if field['domain'] else None - cls._add_field(name, Field.by_type[field['ttype']](**attrs)) + self._add_field(name, Field.by_type[field['ttype']](**attrs)) @classmethod def _init_constraints_onchanges(cls): @@ -2842,13 +2844,13 @@ class BaseModel(object): # Update objects that uses this one to update their _inherits fields # - @classmethod - def _init_inherited_fields(cls): + @api.model + def _add_inherited_fields(self): """ Determine inherited fields. """ # determine candidate inherited fields fields = {} - for parent_model, parent_field in cls._inherits.iteritems(): - parent = cls.pool[parent_model] + for parent_model, parent_field in self._inherits.iteritems(): + parent = self.env[parent_model] for name, field in parent._fields.iteritems(): # inherited fields are implemented as related fields, with the # following specific properties: @@ -2863,8 +2865,8 @@ class BaseModel(object): # add inherited fields that are not redefined locally for name, field in fields.iteritems(): - if name not in cls._fields: - cls._add_field(name, field) + if name not in self._fields: + self._add_field(name, field) @classmethod def _inherits_reload(cls): @@ -2892,29 +2894,29 @@ class BaseModel(object): result[k] = fields.column_info(k, col) return result - @classmethod - def _inherits_check(cls): - for table, field_name in cls._inherits.items(): - field = cls._fields.get(field_name) + @api.model + def _inherits_check(self): + for table, field_name in self._inherits.items(): + field = self._fields.get(field_name) if not field: - _logger.info('Missing many2one field definition for _inherits reference "%s" in "%s", using default one.', field_name, cls._name) + _logger.info('Missing many2one field definition for _inherits reference "%s" in "%s", using default one.', field_name, self._name) from .fields import Many2one field = Many2one(table, string="Automatically created field to link to parent %s" % table, required=True, ondelete="cascade") - cls._add_field(field_name, field) + self._add_field(field_name, field) elif not field.required or field.ondelete.lower() not in ("cascade", "restrict"): - _logger.warning('Field definition for _inherits reference "%s" in "%s" must be marked as "required" with ondelete="cascade" or "restrict", forcing it to required + cascade.', field_name, cls._name) + _logger.warning('Field definition for _inherits reference "%s" in "%s" must be marked as "required" with ondelete="cascade" or "restrict", forcing it to required + cascade.', field_name, self._name) field.required = True field.ondelete = "cascade" - # reflect fields with delegate=True in dictionary cls._inherits - for field in cls._fields.itervalues(): + # reflect fields with delegate=True in dictionary self._inherits + for field in self._fields.itervalues(): if field.type == 'many2one' and not field.related and field.delegate: if not field.required: _logger.warning("Field %s with delegate=True must be required.", field) field.required = True if field.ondelete.lower() not in ('cascade', 'restrict'): field.ondelete = 'cascade' - cls._inherits[field.comodel_name] = field.name + self._inherits[field.comodel_name] = field.name @api.model def _prepare_setup(self): @@ -2937,18 +2939,18 @@ class BaseModel(object): cls._fields = {} cls._defaults = {} for attr, field in getmembers(cls, Field.__instancecheck__): - cls._add_field(attr, field.new()) + self._add_field(attr, field.new()) # add magic and custom fields - cls._add_magic_fields() - cls._init_manual_fields(self._cr, partial) + self._add_magic_fields() + self._add_manual_fields(partial) # 2. make sure that parent models determine their own fields, then add # inherited fields to cls - cls._inherits_check() - for parent in cls._inherits: + self._inherits_check() + for parent in self._inherits: self.env[parent]._setup_base(partial) - cls._init_inherited_fields() + self._add_inherited_fields() cls._setup_done = True From c6c1824b7aff2ff5a0c3e470442444cdedaa3055 Mon Sep 17 00:00:00 2001 From: Raphael Collet Date: Fri, 10 Jul 2015 14:51:05 +0200 Subject: [PATCH 04/10] [IMP] fields: rename methods `set_class_name` and `setup`, and change their API The API of those method has been modified to take a recordset instead of a class as parameter: field.set_class_name(cls, name) -> field.setup_base(model, name) field.setup(env) -> field.setup_full(model) The recordset gives access to an environment, and will ease code reorganization in the field's setup methods. --- .../tests/test_field_conversions.py | 9 +- openerp/fields.py | 221 ++++++++++-------- openerp/models.py | 8 +- 3 files changed, 129 insertions(+), 109 deletions(-) diff --git a/openerp/addons/test_new_api/tests/test_field_conversions.py b/openerp/addons/test_new_api/tests/test_field_conversions.py index 4f9e886df41..dc6d5da372b 100644 --- a/openerp/addons/test_new_api/tests/test_field_conversions.py +++ b/openerp/addons/test_new_api/tests/test_field_conversions.py @@ -1,12 +1,13 @@ # -*- coding: utf-8 -*- -import unittest2 -from openerp import models, fields -class TestFieldToColumn(unittest2.TestCase): +from openerp import fields +from openerp.tests import common + +class TestFieldToColumn(common.TransactionCase): def test_char(self): # create a field, initialize its attributes, and convert it to a column field = fields.Char(string="test string", required=True) - field.set_class_name(models.Model, 'test') + field.setup_base(self.env['res.partner'], 'test') column = field.to_column() self.assertEqual(column.string, "test string") diff --git a/openerp/fields.py b/openerp/fields.py index a00d0c19b46..c35eaacb98c 100644 --- a/openerp/fields.py +++ b/openerp/fields.py @@ -42,16 +42,43 @@ def _check_value(value): return value.get() if isinstance(value, SpecialValue) else value -def resolve_all_mro(cls, name, reverse=False): - """ Return the (successively overridden) values of attribute ``name`` in ``cls`` - in mro order, or inverse mro order if ``reverse`` is true. +def resolve_all_mro(model, name, reverse=False): + """ Return the (successively overridden) values of attribute ``name`` in the + class of ``model`` in mro order (or inverse order if ``reverse``). """ - klasses = reversed(cls.__mro__) if reverse else cls.__mro__ + klasses = reversed(type(model).__mro__) if reverse else type(model).__mro__ for klass in klasses: if name in klass.__dict__: yield klass.__dict__[name] +def default_new_to_new(field, value): + """ Convert the new-API default ``value`` to a callable. """ + return value if callable(value) else lambda model: value + +def default_new_to_old(field, value): + """ Convert the new-API default ``value`` to the old API. """ + if callable(value): + from openerp import api + return api.model(lambda model: field.convert_to_write(value(model))) + else: + return value + +def default_old_to_new(field, value): + """ Convert the old-API default ``value`` to the new API. """ + if callable(value): + return lambda model: field.convert_to_cache( + value(model._model, model._cr, model._uid, model._context), + model, validate=False, + ) + else: + return lambda model: field.convert_to_cache(value, model, validate=False) + +def default_old_to_old(field, value): + """ Convert the old-API default ``value`` to the old API. """ + return value + + class MetaField(type): """ Metaclass for field classes. """ by_type = {} @@ -258,12 +285,12 @@ class Field(object): _slots = { 'args': EMPTY_DICT, # the parameters given to __init__() '_attrs': EMPTY_DICT, # the field's non-slot attributes + 'setup_full_done': False, # whether the field has been fully setup 'automatic': False, # whether the field is automatically created ("magic" field) 'inherited': False, # whether the field is inherited (_inherits) 'origin': None, # the column from which the field was created 'column': None, # the column corresponding to the field - 'setup_done': False, # whether the field has been set up 'name': None, # name of the field 'model_name': None, # name of the model of this field @@ -304,6 +331,10 @@ class Field(object): args = {key: val for key, val in kwargs.iteritems() if val is not None} self.args = args or EMPTY_DICT + def new(self, **kwargs): + """ Return a field of the same type as ``self``, with its own parameters. """ + return type(self)(**kwargs) + def __getattr__(self, name): """ Access non-slot field attribute. """ try: @@ -337,23 +368,38 @@ class Field(object): except KeyError: raise AttributeError(name) - def new(self, **kwargs): - """ Return a field of the same type as ``self``, with its own parameters. """ - return type(self)(**kwargs) + def __str__(self): + return "%s.%s" % (self.model_name, self.name) - def set_class_name(self, cls, name): - """ Assign the model class and field name of ``self``. """ + def __repr__(self): + return "%s.%s" % (self.model_name, self.name) + + ############################################################################ + # + # Base field setup: things that do not depend on other models/fields + # + + def setup_base(self, model, name): + """ Base setup: things that do not depend on other models/fields. """ + self._setup_attrs(model, name) + + # + # Setup field parameter attributes + # + + def _setup_attrs(self, model, name): + """ Determine field parameter attributes. """ # determine all inherited field attributes attrs = {} - for field in resolve_all_mro(cls, name, reverse=True): + for field in resolve_all_mro(model, name, reverse=True): if isinstance(field, type(self)): attrs.update(field.args) else: attrs.clear() - attrs.update(self.args) # necessary in case self is not in cls + attrs.update(self.args) # necessary in case self is not in class attrs['args'] = self.args - attrs['model_name'] = cls._name + attrs['model_name'] = model._name attrs['name'] = name # initialize ``self`` with ``attrs`` @@ -377,18 +423,16 @@ class Field(object): # related fields get their string from their parent field self.string = name.replace('_', ' ').capitalize() - # determine self.default and cls._defaults in a consistent way - self._determine_default(cls, name) + self._setup_default(model, name) - def _determine_default(self, cls, name): - """ Retrieve the default value for ``self`` in the hierarchy of ``cls``, and - determine ``self.default`` and ``cls._defaults`` accordingly. - """ + def _setup_default(self, model, name): + """ Determine ``self.default`` and the corresponding ``model._defaults``. """ self.default = None + model._defaults.pop(name, None) # traverse the class hierarchy upwards, and take the first field # definition with a default or _defaults for self - for klass in cls.__mro__: + for klass in type(model).__mro__: if name in klass.__dict__: field = klass.__dict__[name] if not isinstance(field, type(self)): @@ -396,96 +440,75 @@ class Field(object): return if 'default' in field.args: - # take the default in field, and adapt it for cls._defaults + # take the value, and adapt it for model._defaults value = field.args['default'] - if callable(value): - from openerp import api - self.default = value - cls._defaults[name] = api.model( - lambda recs: self.convert_to_write(value(recs)) - ) - else: - self.default = lambda recs: value - cls._defaults[name] = value + self.default = default_new_to_new(self, value) + model._defaults[name] = default_new_to_old(self, value) return defaults = klass.__dict__.get('_defaults') or {} if name in defaults: # take the value from _defaults, and adapt it for self.default value = defaults[name] - if callable(value): - func = lambda recs: value(recs._model, recs._cr, recs._uid, recs._context) - else: - func = lambda recs: value - self.default = lambda recs: self.convert_to_cache( - func(recs), recs, validate=False, - ) - cls._defaults[name] = value + self.default = default_old_to_new(self, value) + model._defaults[name] = default_old_to_old(self, value) return - def __str__(self): - return "%s.%s" % (self.model_name, self.name) - - def __repr__(self): - return "%s.%s" % (self.model_name, self.name) - ############################################################################ # - # Field setup + # Full field setup: everything else, except recomputation triggers # - def setup(self, env): - """ Make sure that ``self`` is set up, except for recomputation triggers. """ - if not self.setup_done: - if self.related: - self._setup_related(env) + def setup_full(self, model): + """ Full setup: everything else, except recomputation triggers. """ + if not self.setup_full_done: + if not self.related: + self._setup_regular(model) else: - self._setup_regular(env) - self.setup_done = True + self._setup_related(model) + self.setup_full_done = True # # Setup of non-related fields # - def _setup_regular(self, env): + def _setup_regular(self, model): """ Setup the attributes of a non-related field. """ - recs = env[self.model_name] - def make_depends(deps): - return tuple(deps(recs) if callable(deps) else deps) + return tuple(deps(model) if callable(deps) else deps) # convert compute into a callable and determine depends if isinstance(self.compute, basestring): # if the compute method has been overridden, concatenate all their _depends self.depends = () - for method in resolve_all_mro(type(recs), self.compute, reverse=True): + for method in resolve_all_mro(model, self.compute, reverse=True): self.depends += make_depends(getattr(method, '_depends', ())) - self.compute = getattr(type(recs), self.compute) + self.compute = getattr(type(model), self.compute) else: self.depends = make_depends(getattr(self.compute, '_depends', ())) # convert inverse and search into callables if isinstance(self.inverse, basestring): - self.inverse = getattr(type(recs), self.inverse) + self.inverse = getattr(type(model), self.inverse) if isinstance(self.search, basestring): - self.search = getattr(type(recs), self.search) + self.search = getattr(type(model), self.search) # # Setup of related fields # - def _setup_related(self, env): + def _setup_related(self, model): """ Setup the attributes of a related field. """ # fix the type of self.related if necessary if isinstance(self.related, basestring): self.related = tuple(self.related.split('.')) # determine the chain of fields, and make sure they are all set up - recs = env[self.model_name] + target = model for name in self.related: - field = recs._fields[name] - field.setup(env) - recs = recs[name] + field = target._fields[name] + field.setup_full(target) + target = target[name] self.related_field = field @@ -1051,13 +1074,13 @@ class Float(Field): else: return self._digits - def _setup_digits(self, env): + def _setup_digits(self, model): """ Setup the digits for ``self`` and its corresponding column """ pass - def _setup_regular(self, env): - super(Float, self)._setup_regular(env) - self._setup_digits(env) + def _setup_regular(self, model): + super(Float, self)._setup_regular(model) + self._setup_digits(model) _related__digits = property(attrgetter('_digits')) _related_group_operator = property(attrgetter('group_operator')) @@ -1098,11 +1121,11 @@ class Monetary(Field): _related_currency_field = property(attrgetter('currency_field')) _description_currency_field = property(attrgetter('currency_field')) - def _setup_regular(self, env): - super(Monetary, self)._setup_regular(env) + def _setup_regular(self, model): + super(Monetary, self)._setup_regular(model) if not self.currency_field: self.currency_field = 'currency_id' - assert self.currency_field in env[self.model_name]._fields, \ + assert self.currency_field in model._fields, \ "Field %s with unknown currency_field %r" % (self, self.currency_field) def convert_to_write(self, value, target=None, fnames=None): @@ -1190,8 +1213,8 @@ class Char(_String): _related_size = property(attrgetter('size')) _description_size = property(attrgetter('size')) - def _setup_regular(self, env): - super(Char, self)._setup_regular(env) + def _setup_regular(self, model): + super(Char, self)._setup_regular(model) assert isinstance(self.size, (NoneType, int)), \ "Char field %s with non-integer size %r" % (self, self.size) @@ -1399,23 +1422,23 @@ class Selection(Field): selection = api.expected(api.model, selection) super(Selection, self).__init__(selection=selection, string=string, **kwargs) - def _setup_regular(self, env): - super(Selection, self)._setup_regular(env) + def _setup_regular(self, model): + super(Selection, self)._setup_regular(model) assert self.selection is not None, "Field %s without selection" % self - def _setup_related(self, env): - super(Selection, self)._setup_related(env) + def _setup_related(self, model): + super(Selection, self)._setup_related(model) # selection must be computed on related field field = self.related_field self.selection = lambda model: field._description_selection(model.env) - def set_class_name(self, cls, name): - super(Selection, self).set_class_name(cls, name) + def _setup_attrs(self, model, name): + super(Selection, self)._setup_attrs(model, name) # determine selection (applying 'selection_add' extensions) - for field in resolve_all_mro(cls, name, reverse=True): + for field in resolve_all_mro(model, name, reverse=True): if isinstance(field, type(self)): # We cannot use field.selection or field.selection_add here - # because those attributes are overridden by ``set_class_name``. + # because those attributes are overridden by ``_setup_attrs``. if 'selection' in field.args: self.selection = field.args['selection'] if 'selection_add' in field.args: @@ -1489,8 +1512,8 @@ class Reference(Selection): _related_size = property(attrgetter('size')) _column_size = property(attrgetter('size')) - def _setup_regular(self, env): - super(Reference, self)._setup_regular(env) + def _setup_regular(self, model): + super(Reference, self)._setup_regular(model) assert isinstance(self.size, (NoneType, int)), \ "Reference field %s with non-integer size %r" % (self, self.size) @@ -1524,11 +1547,10 @@ class _Relational(Field): 'context': {}, # context for searching values } - def _setup_regular(self, env): - super(_Relational, self)._setup_regular(env) - if self.comodel_name not in env.registry: - _logger.warning("Field %s with unknown comodel_name %r" - % (self, self.comodel_name)) + def _setup_regular(self, model): + super(_Relational, self)._setup_regular(model) + if self.comodel_name not in model.pool: + _logger.warning("Field %s with unknown comodel_name %r", self, self.comodel_name) self.comodel_name = '_unknown' @property @@ -1599,11 +1621,11 @@ class Many2one(_Relational): def __init__(self, comodel_name=None, string=None, **kwargs): super(Many2one, self).__init__(comodel_name=comodel_name, string=string, **kwargs) - def set_class_name(self, cls, name): - super(Many2one, self).set_class_name(cls, name) + def _setup_attrs(self, model, name): + super(Many2one, self)._setup_attrs(model, name) # determine self.delegate if not self.delegate: - self.delegate = name in cls._inherits.values() + self.delegate = name in model._inherits.values() _column_ondelete = property(attrgetter('ondelete')) _column_auto_join = property(attrgetter('auto_join')) @@ -1808,12 +1830,12 @@ class One2many(_RelationalMulti): **kwargs ) - def _setup_regular(self, env): - super(One2many, self)._setup_regular(env) + def _setup_regular(self, model): + super(One2many, self)._setup_regular(model) if self.inverse_name: # link self to its inverse field and vice-versa - comodel = env[self.comodel_name] + comodel = model.env[self.comodel_name] invf = comodel._fields[self.inverse_name] # In some rare cases, a ``One2many`` field can link to ``Int`` field # (res_model/res_id pattern). Only inverse the field if this is @@ -1878,18 +1900,17 @@ class Many2many(_RelationalMulti): **kwargs ) - def _setup_regular(self, env): - super(Many2many, self)._setup_regular(env) + def _setup_regular(self, model): + super(Many2many, self)._setup_regular(model) if not self.relation and self.store: # retrieve self.relation from the corresponding column column = self.to_column() if isinstance(column, fields.many2many): - self.relation, self.column1, self.column2 = \ - column._sql_names(env[self.model_name]) + self.relation, self.column1, self.column2 = column._sql_names(model) if self.relation: - m2m = env.registry._m2m + m2m = model.pool._m2m # if inverse field has already been setup, it is present in m2m invf = m2m.get((self.relation, self.column2, self.column1)) if invf: diff --git a/openerp/models.py b/openerp/models.py index c851e53ed2d..40e2afff6ba 100644 --- a/openerp/models.py +++ b/openerp/models.py @@ -446,9 +446,9 @@ class BaseModel(object): cls._fields[name] = field # basic setup of field - field.set_class_name(cls, name) + field.setup_base(self, name) - # cls._columns will be updated once fields are set up + # cls._columns will be updated once fields are fully set up @api.model def _pop_field(self, name): @@ -2962,7 +2962,7 @@ class BaseModel(object): # set up fields, and determine their corresponding column cls._columns = {} for name, field in cls._fields.iteritems(): - field.setup(self.env) + field.setup_full(self) column = field.to_column() if column: cls._columns[name] = column @@ -3040,8 +3040,6 @@ class BaseModel(object): for fname, field in self._fields.iteritems(): if allfields and fname not in allfields: continue - if not field.setup_done: - continue if field.groups and not recs.user_has_groups(field.groups): continue From 9fa4c2d1d878c0385dd69a39091300e26ce54a40 Mon Sep 17 00:00:00 2001 From: Raphael Collet Date: Fri, 10 Jul 2015 16:20:31 +0200 Subject: [PATCH 05/10] [IMP] models: optimize `_setup_base()` by not retrieving proper fields The method uses the costly `inspect.getmembers` to retrieve the fields defined on a model's classes. If the model is not modified by a module installation, we simply reuse the fields retrieved in a former model setup. --- openerp/models.py | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/openerp/models.py b/openerp/models.py index 40e2afff6ba..b7ab52f8c84 100644 --- a/openerp/models.py +++ b/openerp/models.py @@ -2932,20 +2932,30 @@ class BaseModel(object): # 1. determine the proper fields of the model; duplicate them on cls to # avoid clashes with inheritance between different models - for name in getattr(cls, '_fields', {}): - delattr(cls, name) + proper_fields = getattr(cls, '_proper_fields', None) + if proper_fields is None: + # retrieve fields from parent classes, and add magic fields + cls._fields = {} + cls._defaults = {} + for name, field in getmembers(cls, Field.__instancecheck__): + self._add_field(name, field.new()) + self._add_magic_fields() + cls._proper_fields = set(cls._fields) - # retrieve fields from parent classes - cls._fields = {} - cls._defaults = {} - for attr, field in getmembers(cls, Field.__instancecheck__): - self._add_field(attr, field.new()) + else: + # redo base setup on proper fields, and remove other fields + for name in set(cls._fields) - proper_fields: + delattr(cls, name) + del cls._fields[name] + cls._defaults.pop(name, None) + for name, field in cls._fields.iteritems(): + field.setup_base(self, name) + assert proper_fields == set(cls._fields) - # add magic and custom fields - self._add_magic_fields() + # 2. add custom fields self._add_manual_fields(partial) - # 2. make sure that parent models determine their own fields, then add + # 3. make sure that parent models determine their own fields, then add # inherited fields to cls self._inherits_check() for parent in self._inherits: From 6161c85d556cfecbb39327eef850f34f8021287a Mon Sep 17 00:00:00 2001 From: Raphael Collet Date: Sun, 12 Jul 2015 20:06:45 +0200 Subject: [PATCH 06/10] [IMP] fields: optimize `setup_base()` for regular (non-related) fields Split the methods `_setup_regular` and `_setup_related` in `base` and `full` parts, such that the `base` part is done in `setup_base()`, and the `full` part is done in `setup_full()`. This optimizes the setup of models during the installation of modules: regular fields are not set up again from scratch. Only related fields (which are less common) are always set up from scratch. --- openerp/fields.py | 74 +++++++++++++++++++++++++++-------------------- 1 file changed, 42 insertions(+), 32 deletions(-) diff --git a/openerp/fields.py b/openerp/fields.py index c35eaacb98c..920c995019c 100644 --- a/openerp/fields.py +++ b/openerp/fields.py @@ -330,6 +330,7 @@ class Field(object): kwargs['string'] = string args = {key: val for key, val in kwargs.iteritems() if val is not None} self.args = args or EMPTY_DICT + self.setup_full_done = False def new(self, **kwargs): """ Return a field of the same type as ``self``, with its own parameters. """ @@ -381,7 +382,17 @@ class Field(object): def setup_base(self, model, name): """ Base setup: things that do not depend on other models/fields. """ - self._setup_attrs(model, name) + if self.setup_full_done and not self.related: + # optimization for regular fields: keep the base setup + self.setup_full_done = False + self.inverse_fields = () + self.computed_fields = () + self._triggers = () + else: + # do the base setup from scratch + self._setup_attrs(model, name) + if not self.related: + self._setup_regular_base(model) # # Setup field parameter attributes @@ -463,16 +474,16 @@ class Field(object): """ Full setup: everything else, except recomputation triggers. """ if not self.setup_full_done: if not self.related: - self._setup_regular(model) + self._setup_regular_full(model) else: - self._setup_related(model) + self._setup_related_full(model) self.setup_full_done = True # # Setup of non-related fields # - def _setup_regular(self, model): + def _setup_regular_base(self, model): """ Setup the attributes of a non-related field. """ def make_depends(deps): return tuple(deps(model) if callable(deps) else deps) @@ -493,11 +504,15 @@ class Field(object): if isinstance(self.search, basestring): self.search = getattr(type(model), self.search) + def _setup_regular_full(self, model): + """ Setup the inverse field(s) of ``self``. """ + pass + # # Setup of related fields # - def _setup_related(self, model): + def _setup_related_full(self, model): """ Setup the attributes of a related field. """ # fix the type of self.related if necessary if isinstance(self.related, basestring): @@ -569,7 +584,7 @@ class Field(object): """ Determine the domain to search on field ``self``. """ return [('.'.join(self.related), operator, value)] - # properties used by _setup_related() to copy values from related field + # properties used by _setup_related_full() to copy values from related field _related_comodel_name = property(attrgetter('comodel_name')) _related_string = property(attrgetter('string')) _related_help = property(attrgetter('help')) @@ -1074,14 +1089,6 @@ class Float(Field): else: return self._digits - def _setup_digits(self, model): - """ Setup the digits for ``self`` and its corresponding column """ - pass - - def _setup_regular(self, model): - super(Float, self)._setup_regular(model) - self._setup_digits(model) - _related__digits = property(attrgetter('_digits')) _related_group_operator = property(attrgetter('group_operator')) @@ -1121,10 +1128,13 @@ class Monetary(Field): _related_currency_field = property(attrgetter('currency_field')) _description_currency_field = property(attrgetter('currency_field')) - def _setup_regular(self, model): - super(Monetary, self)._setup_regular(model) + def _setup_regular_base(self, model): + super(Monetary, self)._setup_regular_base(model) if not self.currency_field: self.currency_field = 'currency_id' + + def _setup_regular_full(self, model): + super(Monetary, self)._setup_regular_full(model) assert self.currency_field in model._fields, \ "Field %s with unknown currency_field %r" % (self, self.currency_field) @@ -1213,8 +1223,8 @@ class Char(_String): _related_size = property(attrgetter('size')) _description_size = property(attrgetter('size')) - def _setup_regular(self, model): - super(Char, self)._setup_regular(model) + def _setup_regular_base(self, model): + super(Char, self)._setup_regular_base(model) assert isinstance(self.size, (NoneType, int)), \ "Char field %s with non-integer size %r" % (self, self.size) @@ -1422,12 +1432,12 @@ class Selection(Field): selection = api.expected(api.model, selection) super(Selection, self).__init__(selection=selection, string=string, **kwargs) - def _setup_regular(self, model): - super(Selection, self)._setup_regular(model) + def _setup_regular_base(self, model): + super(Selection, self)._setup_regular_base(model) assert self.selection is not None, "Field %s without selection" % self - def _setup_related(self, model): - super(Selection, self)._setup_related(model) + def _setup_related_full(self, model): + super(Selection, self)._setup_related_full(model) # selection must be computed on related field field = self.related_field self.selection = lambda model: field._description_selection(model.env) @@ -1512,8 +1522,8 @@ class Reference(Selection): _related_size = property(attrgetter('size')) _column_size = property(attrgetter('size')) - def _setup_regular(self, model): - super(Reference, self)._setup_regular(model) + def _setup_regular_base(self, model): + super(Reference, self)._setup_regular_base(model) assert isinstance(self.size, (NoneType, int)), \ "Reference field %s with non-integer size %r" % (self, self.size) @@ -1547,8 +1557,8 @@ class _Relational(Field): 'context': {}, # context for searching values } - def _setup_regular(self, model): - super(_Relational, self)._setup_regular(model) + def _setup_regular_base(self, model): + super(_Relational, self)._setup_regular_base(model) if self.comodel_name not in model.pool: _logger.warning("Field %s with unknown comodel_name %r", self, self.comodel_name) self.comodel_name = '_unknown' @@ -1830,9 +1840,8 @@ class One2many(_RelationalMulti): **kwargs ) - def _setup_regular(self, model): - super(One2many, self)._setup_regular(model) - + def _setup_regular_full(self, model): + super(One2many, self)._setup_regular_full(model) if self.inverse_name: # link self to its inverse field and vice-versa comodel = model.env[self.comodel_name] @@ -1900,15 +1909,16 @@ class Many2many(_RelationalMulti): **kwargs ) - def _setup_regular(self, model): - super(Many2many, self)._setup_regular(model) - + def _setup_regular_base(self, model): + super(Many2many, self)._setup_regular_base(model) if not self.relation and self.store: # retrieve self.relation from the corresponding column column = self.to_column() if isinstance(column, fields.many2many): self.relation, self.column1, self.column2 = column._sql_names(model) + def _setup_regular_full(self, model): + super(Many2many, self)._setup_regular_full(model) if self.relation: m2m = model.pool._m2m # if inverse field has already been setup, it is present in m2m From ec0636a289217c0516c65e3c475549dcf494d7d8 Mon Sep 17 00:00:00 2001 From: Raphael Collet Date: Fri, 7 Aug 2015 16:18:03 +0200 Subject: [PATCH 07/10] [IMP] fields: do not use methods bound to model classes Avoid this reference from a field to a model class; this will allow the sharing of fields across model classes. Use `getattr` on class instances instead. --- openerp/fields.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/openerp/fields.py b/openerp/fields.py index 920c995019c..b5f19cb4945 100644 --- a/openerp/fields.py +++ b/openerp/fields.py @@ -488,21 +488,24 @@ class Field(object): def make_depends(deps): return tuple(deps(model) if callable(deps) else deps) + def make_callable(name): + return lambda recs, *args, **kwargs: getattr(recs, name)(*args, **kwargs) + # convert compute into a callable and determine depends if isinstance(self.compute, basestring): # if the compute method has been overridden, concatenate all their _depends self.depends = () for method in resolve_all_mro(model, self.compute, reverse=True): self.depends += make_depends(getattr(method, '_depends', ())) - self.compute = getattr(type(model), self.compute) + self.compute = make_callable(self.compute) else: self.depends = make_depends(getattr(self.compute, '_depends', ())) # convert inverse and search into callables if isinstance(self.inverse, basestring): - self.inverse = getattr(type(model), self.inverse) + self.inverse = make_callable(self.inverse) if isinstance(self.search, basestring): - self.search = getattr(type(model), self.search) + self.search = make_callable(self.search) def _setup_regular_full(self, model): """ Setup the inverse field(s) of ``self``. """ From de363ce90943281d52b9acc83ff4c4d96d7ccea7 Mon Sep 17 00:00:00 2001 From: Raphael Collet Date: Thu, 6 Aug 2015 17:27:29 +0200 Subject: [PATCH 08/10] [IMP] fields: move `computed_fields`, `inverse_fields` and `_triggers` to models This removes an obstacle for sharing fields across registries: regular fields no longer contain metadata referring to other models/fields. --- openerp/fields.py | 57 +++++++++++++++++-------------------------- openerp/models.py | 38 ++++++++++++++--------------- openerp/tools/misc.py | 18 ++++++++++++++ 3 files changed, 59 insertions(+), 54 deletions(-) diff --git a/openerp/fields.py b/openerp/fields.py index b5f19cb4945..94634ec0ded 100644 --- a/openerp/fields.py +++ b/openerp/fields.py @@ -320,10 +320,7 @@ class Field(object): 'change_default': False, # whether the field may trigger a "user-onchange" 'deprecated': None, # whether the field is deprecated - 'inverse_fields': (), # collection of inverse fields (objects) - 'computed_fields': (), # fields computed with the same method as self 'related_field': None, # corresponding related field - '_triggers': (), # invalidation and recomputation triggers } def __init__(self, string=None, **kwargs): @@ -385,9 +382,6 @@ class Field(object): if self.setup_full_done and not self.related: # optimization for regular fields: keep the base setup self.setup_full_done = False - self.inverse_fields = () - self.computed_fields = () - self._triggers = () else: # do the base setup from scratch self._setup_attrs(model, name) @@ -608,11 +602,6 @@ class Field(object): # See method ``modified`` below for details. # - def add_trigger(self, trigger): - """ Add a recomputation trigger on ``self``. """ - if trigger not in self._triggers: - self._triggers += (trigger,) - def setup_triggers(self, env): """ Add the necessary triggers to invalidate/recompute ``self``. """ model = env[self.model_name] @@ -640,23 +629,19 @@ class Field(object): continue #_logger.debug("Add trigger on %s to recompute %s", field, self) - field.add_trigger((self, '.'.join(path0 or ['id']))) + model._field_triggers.add(field, (self, '.'.join(path0 or ['id']))) # add trigger on inverse fields, too - for invf in field.inverse_fields: + for invf in model._field_inverses[field]: #_logger.debug("Add trigger on %s to recompute %s", invf, self) - invf.add_trigger((self, '.'.join(path0 + [head]))) + invm = env[invf.model_name] + invm._field_triggers.add(invf, (self, '.'.join(path0 + [head]))) # recursively traverse the dependency if tail: comodel = env[field.comodel_name] self._setup_dependency(path0 + [head], comodel, tail) - @property - def dependents(self): - """ Return the computed fields that depend on ``self``. """ - return (field for field, path in self._triggers) - ############################################################################ # # Field description @@ -867,7 +852,7 @@ class Field(object): # set value in cache, inverse field, and mark record as dirty record._cache[self] = value if env.in_onchange: - for invf in self.inverse_fields: + for invf in record._field_inverses[self]: invf._update(value, record) record._set_dirty(self.name) @@ -889,11 +874,12 @@ class Field(object): def _compute_value(self, records): """ Invoke the compute method on ``records``. """ # initialize the fields to their corresponding null value in cache - for field in self.computed_fields: + computed = records._field_computed[self] + for field in computed: records._cache[field] = field.null(records.env) records.env.computed[field].update(records._ids) self.compute(records) - for field in self.computed_fields: + for field in computed: records.env.computed[field].difference_update(records._ids) def compute_value(self, records): @@ -923,10 +909,11 @@ class Field(object): self.compute_value(recs) # HACK: if result is in the wrong cache, copy values if recs.env != env: + computed = record._field_computed[self] for source, target in zip(recs, recs.with_env(env)): try: values = target._convert_to_cache({ - f.name: source[f.name] for f in self.computed_fields + f.name: source[f.name] for f in computed }, validate=False) except MissingError as e: values = FailedValue(e) @@ -981,7 +968,7 @@ class Field(object): """ # invalidate the fields that depend on self, and prepare recomputation spec = [(self, records._ids)] - for field, path in self._triggers: + for field, path in records._field_triggers[self]: if path and field.store: # don't move this line to function top, see log env = records.env(user=SUPERUSER_ID, context={'active_test': False}) @@ -1007,7 +994,7 @@ class Field(object): # invalidate the fields on the records in cache that depend on # ``records``, except fields currently being computed spec = [] - for field, path in self._triggers: + for field, path in records._field_triggers[self]: target = env[field.model_name] computed = target.browse(env.computed[field]) if path == 'id': @@ -1591,11 +1578,11 @@ class _Relational(Field): return env[self.comodel_name] def modified(self, records): - # Invalidate cache for self.inverse_fields, too. Note that recomputation - # of fields that depend on self.inverse_fields is already covered by the - # triggers (see above). + # Invalidate cache for inverse fields, too. Note that the recomputation + # of fields that depend on inverse fields is already covered by the + # triggers. spec = super(_Relational, self).modified(records) - for invf in self.inverse_fields: + for invf in records._field_inverses[self]: spec.append((invf, None)) return spec @@ -1772,7 +1759,8 @@ class _RelationalMulti(_Relational): if fnames is None: # take all fields in cache, except the inverses of self fnames = set(value._fields) - set(MAGIC_COLUMNS) - for invf in self.inverse_fields: + model = value.env[self.model_name] + for invf in model._field_inverses[self]: fnames.discard(invf.name) # add new and existing records @@ -1853,8 +1841,8 @@ class One2many(_RelationalMulti): # (res_model/res_id pattern). Only inverse the field if this is # a ``Many2one`` field. if isinstance(invf, Many2one): - self.inverse_fields += (invf,) - invf.inverse_fields += (self,) + model._field_inverses.add(self, invf) + comodel._field_inverses.add(invf, self) _description_relation_field = property(attrgetter('inverse_name')) @@ -1927,8 +1915,9 @@ class Many2many(_RelationalMulti): # if inverse field has already been setup, it is present in m2m invf = m2m.get((self.relation, self.column2, self.column1)) if invf: - self.inverse_fields += (invf,) - invf.inverse_fields += (self,) + comodel = model.env[self.comodel_name] + model._field_inverses.add(self, invf) + comodel._field_inverses.add(invf, self) else: # add self in m2m, so that its inverse field can find it m2m[(self.relation, self.column1, self.column2)] = self diff --git a/openerp/models.py b/openerp/models.py index b7ab52f8c84..c12b4f6e570 100644 --- a/openerp/models.py +++ b/openerp/models.py @@ -47,7 +47,7 @@ from .api import Environment from .exceptions import AccessError, MissingError, ValidationError, UserError from .osv import fields from .osv.query import Query -from .tools import frozendict, lazy_property, ormcache +from .tools import frozendict, lazy_property, ormcache, Collector from .tools.config import config from .tools.func import frame_codeinfo from .tools.misc import CountingStream, DEFAULT_SERVER_DATETIME_FORMAT, DEFAULT_SERVER_DATE_FORMAT @@ -2962,6 +2962,11 @@ class BaseModel(object): self.env[parent]._setup_base(partial) self._add_inherited_fields() + # 4. initialize more field metadata + cls._field_computed = {} # fields computed with the same method + cls._field_inverses = Collector() # inverse fields for related fields + cls._field_triggers = Collector() # list of (field, path) to invalidate + cls._setup_done = True @api.model @@ -2977,15 +2982,12 @@ class BaseModel(object): if column: cls._columns[name] = column - # determine field.computed_fields - computed_fields = defaultdict(list) + # map each field to the fields computed with the same method + groups = defaultdict(list) for field in cls._fields.itervalues(): if field.compute: - computed_fields[field.compute].append(field) - - for fields in computed_fields.itervalues(): - for field in fields: - field.computed_fields = fields + cls._field_computed[field] = group = groups[field.compute] + group.append(field) @api.model def _setup_complete(self): @@ -2998,13 +3000,12 @@ class BaseModel(object): # add invalidation triggers on model dependencies if cls._depends: - triggers = [(field, None) for field in cls._fields.itervalues()] for model_name, field_names in cls._depends.iteritems(): model = self.env[model_name] for field_name in field_names: field = model._fields[field_name] - for trigger in triggers: - field.add_trigger(trigger) + for dependent in cls._fields.itervalues(): + model._field_triggers.add(field, (dependent, None)) # determine old-api structures about inherited fields cls._inherits_reload() @@ -5427,7 +5428,7 @@ class BaseModel(object): for name in values: field = self._fields.get(name) if field: - for invf in field.inverse_fields: + for invf in self._field_inverses[field]: invf._update(record[name], record) return record @@ -5633,7 +5634,7 @@ class BaseModel(object): # invalidate fields and inverse fields, too spec = [(f, ids) for f in fields] + \ - [(invf, None) for f in fields for invf in f.inverse_fields] + [(invf, None) for f in fields for invf in self._field_inverses[f]] self.env.invalidate(spec) @api.multi @@ -5683,11 +5684,8 @@ class BaseModel(object): while self.env.has_todo(): field, recs = self.env.get_todo() # evaluate the fields to recompute, and save them to database - names = [ - f.name - for f in field.computed_fields - if f.store and self.env.field_todo(f) - ] + computed = self.env[field.model_name]._field_computed[field] + names = [f.name for f in computed if f.store and self.env.field_todo(f)] for rec in recs: try: values = rec._convert_to_write({ @@ -5698,7 +5696,7 @@ class BaseModel(object): except MissingError: pass # mark the computed fields as done - map(recs._recompute_done, field.computed_fields) + map(recs._recompute_done, computed) # # Generic onchange method @@ -5711,7 +5709,7 @@ class BaseModel(object): # test whether self has an onchange method for field, or field is a # dependency of any field in other_fields return field.name in self._onchange_methods or \ - any(dep in other_fields for dep in field.dependents) + any(dep in other_fields for dep, _ in self._field_triggers[field]) @api.model def _onchange_spec(self, view_info=None): diff --git a/openerp/tools/misc.py b/openerp/tools/misc.py index ef84a0ab45d..436eb5b47f2 100644 --- a/openerp/tools/misc.py +++ b/openerp/tools/misc.py @@ -1064,6 +1064,24 @@ class frozendict(dict): def update(self, *args, **kwargs): raise NotImplementedError("'update' not supported on frozendict") +class Collector(Mapping): + """ A mapping from keys to lists. This is essentially a space optimization + for ``defaultdict(list)``. + """ + __slots__ = ['_map'] + def __init__(self): + self._map = {} + def add(self, key, val): + vals = self._map.setdefault(key, []) + if val not in vals: + vals.append(val) + def __getitem__(self, key): + return self._map.get(key, ()) + def __iter__(self): + return iter(self._map) + def __len__(self): + return len(self._map) + class OrderedSet(OrderedDict): """ A simple collection that remembers the elements insertion order. """ def __init__(self, seq=()): From 7d6abb6f31547d404697915c1801fc531ad1959c Mon Sep 17 00:00:00 2001 From: Raphael Collet Date: Fri, 7 Aug 2015 11:43:27 +0200 Subject: [PATCH 09/10] [IMP] fields: simplify implementation of method `setup_triggers` --- openerp/fields.py | 50 +++++++++++++++-------------------------------- 1 file changed, 16 insertions(+), 34 deletions(-) diff --git a/openerp/fields.py b/openerp/fields.py index 94634ec0ded..91a65359ee7 100644 --- a/openerp/fields.py +++ b/openerp/fields.py @@ -604,43 +604,25 @@ class Field(object): def setup_triggers(self, env): """ Add the necessary triggers to invalidate/recompute ``self``. """ - model = env[self.model_name] - for path in self.depends: - self._setup_dependency([], model, path.split('.')) + for path_str in self.depends: + path = path_str.split('.') - def _setup_dependency(self, path0, model, path1): - """ Make ``self`` depend on ``model``; `path0 + path1` is a dependency of - ``self``, and ``path0`` is the sequence of field names from ``self.model`` - to ``model``. - """ - env = model.env - head, tail = path1[0], path1[1:] + # traverse path and add triggers on fields along the way + field = None + for i, name in enumerate(path): + model = env[field.comodel_name if field else self.model_name] + field = model._fields[name] + # env[self.model_name] --- path[:i] --> model with field - if head == '*': - # special case: add triggers on all fields of model (except self) - fields = set(model._fields.itervalues()) - set([self]) - else: - fields = [model._fields[head]] + if field is self: + self.recursive = True + continue - for field in fields: - if field == self: - _logger.debug("Field %s is recursively defined", self) - self.recursive = True - continue - - #_logger.debug("Add trigger on %s to recompute %s", field, self) - model._field_triggers.add(field, (self, '.'.join(path0 or ['id']))) - - # add trigger on inverse fields, too - for invf in model._field_inverses[field]: - #_logger.debug("Add trigger on %s to recompute %s", invf, self) - invm = env[invf.model_name] - invm._field_triggers.add(invf, (self, '.'.join(path0 + [head]))) - - # recursively traverse the dependency - if tail: - comodel = env[field.comodel_name] - self._setup_dependency(path0 + [head], comodel, tail) + # add trigger on field and its inverses to recompute self + model._field_triggers.add(field, (self, '.'.join(path[:i] or ['id']))) + for invf in model._field_inverses[field]: + invm = env[invf.model_name] + invm._field_triggers.add(invf, (self, '.'.join(path[:i+1]))) ############################################################################ # From 69f7f1c9ee180f77e8fc3988ab3c5d7f8260a8d5 Mon Sep 17 00:00:00 2001 From: Raphael Collet Date: Fri, 7 Aug 2015 16:21:00 +0200 Subject: [PATCH 10/10] [IMP] models: share fields across registries when possible When a model is set up, it can retrieve fields from a similar model (with the same base classes). Only proper (not inherited), regular (non-related) fields can be shared that way, since other fields may depend on other models. --- openerp/models.py | 49 ++++++++++++++++++++++++------------- openerp/modules/registry.py | 13 ++++++++++ openerp/tools/lru.py | 6 +++++ 3 files changed, 51 insertions(+), 17 deletions(-) diff --git a/openerp/models.py b/openerp/models.py index c12b4f6e570..14b55577ffb 100644 --- a/openerp/models.py +++ b/openerp/models.py @@ -604,9 +604,9 @@ class BaseModel(object): ModelClass = type(name, tuple(bases), { '_name': name, '_register': False, - '_columns': None, # recomputed in _setup_fields() - '_defaults': None, # recomputed in _setup_base() - '_fields': frozendict(), # idem + '_columns': {}, # recomputed in _setup_fields() + '_defaults': {}, # recomputed in _setup_base() + '_fields': {}, # idem '_inherits': inherits, '_depends': depends, '_constraints': constraints.values(), @@ -2930,11 +2930,34 @@ class BaseModel(object): if cls._setup_done: return - # 1. determine the proper fields of the model; duplicate them on cls to - # avoid clashes with inheritance between different models - proper_fields = getattr(cls, '_proper_fields', None) - if proper_fields is None: - # retrieve fields from parent classes, and add magic fields + # 1. determine the proper fields of the model: the fields defined on the + # class and magic fields, not the inherited or custom ones + cls0 = cls.pool.model_cache.get(cls.__bases__) + if cls0: + # cls0 is either a model class from another registry, or cls itself. + # The point is that it has the same base classes. We retrieve stuff + # from cls0 to optimize the setup of cls. cls0 is guaranteed to be + # properly set up: registries are loaded under a global lock, + # therefore two registries are never set up at the same time. + + # remove fields that are not proper to cls + for name in set(cls._fields) - cls0._proper_fields: + delattr(cls, name) + cls._fields.pop(name, None) + cls._defaults.pop(name, None) + # collect proper fields on cls0, and add them on cls + for name in cls0._proper_fields: + field = cls0._fields[name] + if field.related: + # only regular fields are shared, related fields are copied + field = field.new(**field.args) + assert not (field.setup_full_done and field.related) + self._add_field(name, field) + cls._proper_fields = set(cls._fields) + + else: + # retrieve fields from parent classes, and duplicate them on cls to + # avoid clashes with inheritance between different models cls._fields = {} cls._defaults = {} for name, field in getmembers(cls, Field.__instancecheck__): @@ -2942,15 +2965,7 @@ class BaseModel(object): self._add_magic_fields() cls._proper_fields = set(cls._fields) - else: - # redo base setup on proper fields, and remove other fields - for name in set(cls._fields) - proper_fields: - delattr(cls, name) - del cls._fields[name] - cls._defaults.pop(name, None) - for name, field in cls._fields.iteritems(): - field.setup_base(self, name) - assert proper_fields == set(cls._fields) + cls.pool.model_cache[cls.__bases__] = cls # 2. add custom fields self._add_manual_fields(partial) diff --git a/openerp/modules/registry.py b/openerp/modules/registry.py index e7af183c4fc..96d9e0a3687 100644 --- a/openerp/modules/registry.py +++ b/openerp/modules/registry.py @@ -87,6 +87,10 @@ class Registry(Mapping): """ Same as ``self[model_name]``. """ return self.models[model_name] + @lazy_property + def model_cache(self): + return RegistryManager.model_cache + @lazy_property def pure_function_fields(self): """ Return the list of pure function fields (field objects) """ @@ -269,6 +273,7 @@ class RegistryManager(object): """ _registries = None + _model_cache = None _lock = threading.RLock() _saved_lock = None @@ -290,6 +295,14 @@ class RegistryManager(object): cls._registries = LRU(size) return cls._registries + @classproperty + def model_cache(cls): + """ A cache for model classes, indexed by their base classes. """ + if cls._model_cache is None: + # we cache 256 classes per registry on average + cls._model_cache = LRU(cls.registries.count * 256) + return cls._model_cache + @classmethod def lock(cls): """ Return the current registry lock. """ diff --git a/openerp/tools/lru.py b/openerp/tools/lru.py index 9d1b5f685ff..b0ea21bba44 100644 --- a/openerp/tools/lru.py +++ b/openerp/tools/lru.py @@ -32,6 +32,12 @@ class LRU(object): def __contains__(self, obj): return obj in self.d + def get(self, obj, val=None): + try: + return self[obj] + except KeyError: + return val + @synchronized() def __getitem__(self, obj): a = self.d[obj].me