[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.

2 files changed, 31 insertions, 22 deletions

diff --git a/openerp/fields.py b/openerp/fields.py
index ff50f73d1b8..586e76db39e 100644
--- a/openerp/fields.py
+++ b/openerp/fields.py
@@ -604,16 +604,23 @@ class Field(object):
         """ return the null value for this field in the given environment """
         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
             an assignment, or have the format of methods :meth:`BaseModel.read`
             or :meth:`BaseModel.write`
+
+            :param bool validate: when True, field-specific validation of
+                `value` will be performed
         """
         return value
 
     def convert_to_read(self, value, use_name_get=True):
         """ convert `value` from the cache to a value as returned by method
             :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
 
@@ -753,7 +760,7 @@ class Field(object):
                             try:
                                 values = target._convert_to_cache({
                                     f.name: source[f.name] for f in self.computed_fields
-                                })
+                                }, validate=False)
                             except MissingError as e:
                                 values = FailedValue(e)
                             target._cache.update(values)
@@ -855,7 +862,7 @@ class Boolean(Field):
     """ Boolean field. """
     type = 'boolean'
 
-    def convert_to_cache(self, value, env):
+    def convert_to_cache(self, value, env, validate=True):
         return bool(value)
 
     def convert_to_export(self, value, env):
@@ -868,7 +875,7 @@ class Integer(Field):
     """ Integer field. """
     type = 'integer'
 
-    def convert_to_cache(self, value, env):
+    def convert_to_cache(self, value, env, validate=True):
         return int(value or 0)
 
     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_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!
         if self.digits:
             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'))
     _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]
 
 
@@ -956,7 +963,7 @@ class Text(_String):
     """
     type = 'text'
 
-    def convert_to_cache(self, value, env):
+    def convert_to_cache(self, value, env, validate=True):
         return bool(value) and ustr(value)
 
 
@@ -964,7 +971,7 @@ class Html(_String):
     """ Html field. """
     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)
 
 
@@ -1013,7 +1020,7 @@ class Date(Field):
         """ Convert a :class:`date` value into the format expected by the ORM. """
         return value.strftime(DATE_FORMAT)
 
-    def convert_to_cache(self, value, env):
+    def convert_to_cache(self, value, env, validate=True):
         if not value:
             return False
         if isinstance(value, basestring):
@@ -1078,7 +1085,7 @@ class Datetime(Field):
         """ Convert a :class:`datetime` value into the format expected by the ORM. """
         return value.strftime(DATETIME_FORMAT)
 
-    def convert_to_cache(self, value, env):
+    def convert_to_cache(self, value, env, validate=True):
         if not value:
             return False
         if isinstance(value, basestring):
@@ -1158,7 +1165,9 @@ class Selection(Field):
             selection = selection(env[self.model_name])
         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):
             return value
         elif not value:
@@ -1196,9 +1205,10 @@ class Reference(Selection):
 
     _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 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
         elif isinstance(value, basestring):
             res_model, res_id = value.split(',')
@@ -1293,7 +1303,7 @@ class Many2one(_Relational):
         """ Update the cached value of `self` for `records` with `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)):
             return env[self.comodel_name].browse(value)
         if isinstance(value, BaseModel):
@@ -1346,7 +1356,7 @@ class _RelationalMulti(_Relational):
         for record in records:
             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 value._name == self.comodel_name:
                 return value.with_env(env)
diff --git a/openerp/models.py b/openerp/models.py
index ecc8c7263fa..fffdd93a53c 100644
--- a/openerp/models.py
+++ b/openerp/models.py
@@ -3136,7 +3136,7 @@ class BaseModel(object):
         if field not in self._cache:
             for values in result:
                 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:
                 e = AccessError("No value found for %s.%s" % (self, field.name))
                 self._cache[field] = FailedValue(e)
@@ -3215,7 +3215,7 @@ class BaseModel(object):
             # store result in cache for POST fields
             for vals in result:
                 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
             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
         for vals in result:
             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
         fetched = self.browse(ids)
@@ -3596,7 +3596,6 @@ class BaseModel(object):
         if not self:
             return True
 
-        cr, uid, context = self.env.args
         self._check_concurrency(self._ids)
         self.check_access_rights('write')
 
@@ -5098,11 +5097,11 @@ class BaseModel(object):
         context = dict(args[0] if args else self._context, **kwargs)
         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. """
         fields = self._fields
         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()
             if name in fields
         }
@@ -5555,7 +5554,7 @@ class BaseModel(object):
                 return
             if 'value' in method_res:
                 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:
                 result.setdefault('domain', {}).update(method_res['domain'])
             if 'warning' in method_res: