[FIX] fields: do not revalidate field values unless they are being modified
In the previous implementation of the new API fields, both fields.Selection and fields.Reference were performing early validation of their `value` as soon as it entered the cache, either by being read, written, or computed. This is a source of trouble and performance problems, and is unnecessary, as we should consider that the database always contains valid values. If that is not the case it means it was modified externally and is an exception that should be handled externally as well. Revalidating selection/reference values can be expensive when the domain of values is dynamic and requires extra database queries, with extra access rights control, etc. This patch adds a `validate` parameter to `convert_to_cache`, allowing to turn off the re-validation on demand. The ORM will turn off validation whenever the value being converted is supposed to be already validated, such as when reading it from the database. The parameter is currently ignored by all other fields, and defaults to True so validation is performed in all other caes.
This commit is contained in:
parent
42f3575bb3
commit
8974e928fa
|
@ -604,16 +604,23 @@ class Field(object):
|
||||||
""" return the null value for this field in the given environment """
|
""" return the null value for this field in the given environment """
|
||||||
return False
|
return False
|
||||||
|
|
||||||
def convert_to_cache(self, value, env):
|
def convert_to_cache(self, value, env, validate=True):
|
||||||
""" convert `value` to the cache level in `env`; `value` may come from
|
""" convert `value` to the cache level in `env`; `value` may come from
|
||||||
an assignment, or have the format of methods :meth:`BaseModel.read`
|
an assignment, or have the format of methods :meth:`BaseModel.read`
|
||||||
or :meth:`BaseModel.write`
|
or :meth:`BaseModel.write`
|
||||||
|
|
||||||
|
:param bool validate: when True, field-specific validation of
|
||||||
|
`value` will be performed
|
||||||
"""
|
"""
|
||||||
return value
|
return value
|
||||||
|
|
||||||
def convert_to_read(self, value, use_name_get=True):
|
def convert_to_read(self, value, use_name_get=True):
|
||||||
""" convert `value` from the cache to a value as returned by method
|
""" convert `value` from the cache to a value as returned by method
|
||||||
:meth:`BaseModel.read`
|
:meth:`BaseModel.read`
|
||||||
|
|
||||||
|
:param bool use_name_get: when True, value's diplay name will
|
||||||
|
be computed using :meth:`BaseModel.name_get`, if relevant
|
||||||
|
for the field
|
||||||
"""
|
"""
|
||||||
return value
|
return value
|
||||||
|
|
||||||
|
@ -753,7 +760,7 @@ class Field(object):
|
||||||
try:
|
try:
|
||||||
values = target._convert_to_cache({
|
values = target._convert_to_cache({
|
||||||
f.name: source[f.name] for f in self.computed_fields
|
f.name: source[f.name] for f in self.computed_fields
|
||||||
})
|
}, validate=False)
|
||||||
except MissingError as e:
|
except MissingError as e:
|
||||||
values = FailedValue(e)
|
values = FailedValue(e)
|
||||||
target._cache.update(values)
|
target._cache.update(values)
|
||||||
|
@ -855,7 +862,7 @@ class Boolean(Field):
|
||||||
""" Boolean field. """
|
""" Boolean field. """
|
||||||
type = 'boolean'
|
type = 'boolean'
|
||||||
|
|
||||||
def convert_to_cache(self, value, env):
|
def convert_to_cache(self, value, env, validate=True):
|
||||||
return bool(value)
|
return bool(value)
|
||||||
|
|
||||||
def convert_to_export(self, value, env):
|
def convert_to_export(self, value, env):
|
||||||
|
@ -868,7 +875,7 @@ class Integer(Field):
|
||||||
""" Integer field. """
|
""" Integer field. """
|
||||||
type = 'integer'
|
type = 'integer'
|
||||||
|
|
||||||
def convert_to_cache(self, value, env):
|
def convert_to_cache(self, value, env, validate=True):
|
||||||
return int(value or 0)
|
return int(value or 0)
|
||||||
|
|
||||||
def convert_to_read(self, value, use_name_get=True):
|
def convert_to_read(self, value, use_name_get=True):
|
||||||
|
@ -908,7 +915,7 @@ class Float(Field):
|
||||||
_column_digits = property(lambda self: not callable(self._digits) and self._digits)
|
_column_digits = property(lambda self: not callable(self._digits) and self._digits)
|
||||||
_column_digits_compute = property(lambda self: callable(self._digits) and self._digits)
|
_column_digits_compute = property(lambda self: callable(self._digits) and self._digits)
|
||||||
|
|
||||||
def convert_to_cache(self, value, env):
|
def convert_to_cache(self, value, env, validate=True):
|
||||||
# apply rounding here, otherwise value in cache may be wrong!
|
# apply rounding here, otherwise value in cache may be wrong!
|
||||||
if self.digits:
|
if self.digits:
|
||||||
return float_round(float(value or 0.0), precision_digits=self.digits[1])
|
return float_round(float(value or 0.0), precision_digits=self.digits[1])
|
||||||
|
@ -942,7 +949,7 @@ class Char(_String):
|
||||||
_related_size = property(attrgetter('size'))
|
_related_size = property(attrgetter('size'))
|
||||||
_description_size = property(attrgetter('size'))
|
_description_size = property(attrgetter('size'))
|
||||||
|
|
||||||
def convert_to_cache(self, value, env):
|
def convert_to_cache(self, value, env, validate=True):
|
||||||
return bool(value) and ustr(value)[:self.size]
|
return bool(value) and ustr(value)[:self.size]
|
||||||
|
|
||||||
|
|
||||||
|
@ -956,7 +963,7 @@ class Text(_String):
|
||||||
"""
|
"""
|
||||||
type = 'text'
|
type = 'text'
|
||||||
|
|
||||||
def convert_to_cache(self, value, env):
|
def convert_to_cache(self, value, env, validate=True):
|
||||||
return bool(value) and ustr(value)
|
return bool(value) and ustr(value)
|
||||||
|
|
||||||
|
|
||||||
|
@ -964,7 +971,7 @@ class Html(_String):
|
||||||
""" Html field. """
|
""" Html field. """
|
||||||
type = 'html'
|
type = 'html'
|
||||||
|
|
||||||
def convert_to_cache(self, value, env):
|
def convert_to_cache(self, value, env, validate=True):
|
||||||
return bool(value) and html_sanitize(value)
|
return bool(value) and html_sanitize(value)
|
||||||
|
|
||||||
|
|
||||||
|
@ -1013,7 +1020,7 @@ class Date(Field):
|
||||||
""" Convert a :class:`date` value into the format expected by the ORM. """
|
""" Convert a :class:`date` value into the format expected by the ORM. """
|
||||||
return value.strftime(DATE_FORMAT)
|
return value.strftime(DATE_FORMAT)
|
||||||
|
|
||||||
def convert_to_cache(self, value, env):
|
def convert_to_cache(self, value, env, validate=True):
|
||||||
if not value:
|
if not value:
|
||||||
return False
|
return False
|
||||||
if isinstance(value, basestring):
|
if isinstance(value, basestring):
|
||||||
|
@ -1078,7 +1085,7 @@ class Datetime(Field):
|
||||||
""" Convert a :class:`datetime` value into the format expected by the ORM. """
|
""" Convert a :class:`datetime` value into the format expected by the ORM. """
|
||||||
return value.strftime(DATETIME_FORMAT)
|
return value.strftime(DATETIME_FORMAT)
|
||||||
|
|
||||||
def convert_to_cache(self, value, env):
|
def convert_to_cache(self, value, env, validate=True):
|
||||||
if not value:
|
if not value:
|
||||||
return False
|
return False
|
||||||
if isinstance(value, basestring):
|
if isinstance(value, basestring):
|
||||||
|
@ -1158,7 +1165,9 @@ class Selection(Field):
|
||||||
selection = selection(env[self.model_name])
|
selection = selection(env[self.model_name])
|
||||||
return [value for value, _ in selection]
|
return [value for value, _ in selection]
|
||||||
|
|
||||||
def convert_to_cache(self, value, env):
|
def convert_to_cache(self, value, env, validate=True):
|
||||||
|
if not validate:
|
||||||
|
return value or False
|
||||||
if value in self.get_values(env):
|
if value in self.get_values(env):
|
||||||
return value
|
return value
|
||||||
elif not value:
|
elif not value:
|
||||||
|
@ -1196,9 +1205,10 @@ class Reference(Selection):
|
||||||
|
|
||||||
_column_size = property(attrgetter('size'))
|
_column_size = property(attrgetter('size'))
|
||||||
|
|
||||||
def convert_to_cache(self, value, env):
|
def convert_to_cache(self, value, env, validate=True):
|
||||||
if isinstance(value, BaseModel):
|
if isinstance(value, BaseModel):
|
||||||
if value._name in self.get_values(env) and len(value) <= 1:
|
if ((not validate or value._name in self.get_values(env))
|
||||||
|
and len(value) <= 1):
|
||||||
return value.with_env(env) or False
|
return value.with_env(env) or False
|
||||||
elif isinstance(value, basestring):
|
elif isinstance(value, basestring):
|
||||||
res_model, res_id = value.split(',')
|
res_model, res_id = value.split(',')
|
||||||
|
@ -1293,7 +1303,7 @@ class Many2one(_Relational):
|
||||||
""" Update the cached value of `self` for `records` with `value`. """
|
""" Update the cached value of `self` for `records` with `value`. """
|
||||||
records._cache[self] = value
|
records._cache[self] = value
|
||||||
|
|
||||||
def convert_to_cache(self, value, env):
|
def convert_to_cache(self, value, env, validate=True):
|
||||||
if isinstance(value, (NoneType, int)):
|
if isinstance(value, (NoneType, int)):
|
||||||
return env[self.comodel_name].browse(value)
|
return env[self.comodel_name].browse(value)
|
||||||
if isinstance(value, BaseModel):
|
if isinstance(value, BaseModel):
|
||||||
|
@ -1346,7 +1356,7 @@ class _RelationalMulti(_Relational):
|
||||||
for record in records:
|
for record in records:
|
||||||
record._cache[self] = record[self.name] | value
|
record._cache[self] = record[self.name] | value
|
||||||
|
|
||||||
def convert_to_cache(self, value, env):
|
def convert_to_cache(self, value, env, validate=True):
|
||||||
if isinstance(value, BaseModel):
|
if isinstance(value, BaseModel):
|
||||||
if value._name == self.comodel_name:
|
if value._name == self.comodel_name:
|
||||||
return value.with_env(env)
|
return value.with_env(env)
|
||||||
|
|
|
@ -3136,7 +3136,7 @@ class BaseModel(object):
|
||||||
if field not in self._cache:
|
if field not in self._cache:
|
||||||
for values in result:
|
for values in result:
|
||||||
record = self.browse(values.pop('id'))
|
record = self.browse(values.pop('id'))
|
||||||
record._cache.update(record._convert_to_cache(values))
|
record._cache.update(record._convert_to_cache(values, validate=False))
|
||||||
if field not in self._cache:
|
if field not in self._cache:
|
||||||
e = AccessError("No value found for %s.%s" % (self, field.name))
|
e = AccessError("No value found for %s.%s" % (self, field.name))
|
||||||
self._cache[field] = FailedValue(e)
|
self._cache[field] = FailedValue(e)
|
||||||
|
@ -3215,7 +3215,7 @@ class BaseModel(object):
|
||||||
# store result in cache for POST fields
|
# store result in cache for POST fields
|
||||||
for vals in result:
|
for vals in result:
|
||||||
record = self.browse(vals['id'])
|
record = self.browse(vals['id'])
|
||||||
record._cache.update(record._convert_to_cache(vals))
|
record._cache.update(record._convert_to_cache(vals, validate=False))
|
||||||
|
|
||||||
# determine the fields that must be processed now
|
# determine the fields that must be processed now
|
||||||
fields_post = [f for f in field_names if not self._columns[f]._classic_write]
|
fields_post = [f for f in field_names if not self._columns[f]._classic_write]
|
||||||
|
@ -3256,7 +3256,7 @@ class BaseModel(object):
|
||||||
# store result in cache
|
# store result in cache
|
||||||
for vals in result:
|
for vals in result:
|
||||||
record = self.browse(vals.pop('id'))
|
record = self.browse(vals.pop('id'))
|
||||||
record._cache.update(record._convert_to_cache(vals))
|
record._cache.update(record._convert_to_cache(vals, validate=False))
|
||||||
|
|
||||||
# store failed values in cache for the records that could not be read
|
# store failed values in cache for the records that could not be read
|
||||||
fetched = self.browse(ids)
|
fetched = self.browse(ids)
|
||||||
|
@ -3596,7 +3596,6 @@ class BaseModel(object):
|
||||||
if not self:
|
if not self:
|
||||||
return True
|
return True
|
||||||
|
|
||||||
cr, uid, context = self.env.args
|
|
||||||
self._check_concurrency(self._ids)
|
self._check_concurrency(self._ids)
|
||||||
self.check_access_rights('write')
|
self.check_access_rights('write')
|
||||||
|
|
||||||
|
@ -5098,11 +5097,11 @@ class BaseModel(object):
|
||||||
context = dict(args[0] if args else self._context, **kwargs)
|
context = dict(args[0] if args else self._context, **kwargs)
|
||||||
return self.with_env(self.env(context=context))
|
return self.with_env(self.env(context=context))
|
||||||
|
|
||||||
def _convert_to_cache(self, values):
|
def _convert_to_cache(self, values, validate=True):
|
||||||
""" Convert the `values` dictionary into cached values. """
|
""" Convert the `values` dictionary into cached values. """
|
||||||
fields = self._fields
|
fields = self._fields
|
||||||
return {
|
return {
|
||||||
name: fields[name].convert_to_cache(value, self.env)
|
name: fields[name].convert_to_cache(value, self.env, validate=validate)
|
||||||
for name, value in values.iteritems()
|
for name, value in values.iteritems()
|
||||||
if name in fields
|
if name in fields
|
||||||
}
|
}
|
||||||
|
@ -5555,7 +5554,7 @@ class BaseModel(object):
|
||||||
return
|
return
|
||||||
if 'value' in method_res:
|
if 'value' in method_res:
|
||||||
method_res['value'].pop('id', None)
|
method_res['value'].pop('id', None)
|
||||||
self.update(self._convert_to_cache(method_res['value']))
|
self.update(self._convert_to_cache(method_res['value'], validate=False))
|
||||||
if 'domain' in method_res:
|
if 'domain' in method_res:
|
||||||
result.setdefault('domain', {}).update(method_res['domain'])
|
result.setdefault('domain', {}).update(method_res['domain'])
|
||||||
if 'warning' in method_res:
|
if 'warning' in method_res:
|
||||||
|
|
Loading…
Reference in New Issue