[FIX] fields: convert_to_cache() on *2many fields must take record's current value

The existing code was buggy when writing on *2many fields with a list of
commands: the value was converted for the cache, but taking an empty recordset
as the current value of the field.

diff --git a/openerp/fields.py b/openerp/fields.py
index d85ce82..e9fd908 100644 (file)
--- a/openerp/fields.py
+++ b/openerp/fields.py
@@ -609,11 +609,13 @@ class Field(object):
         """ return the null value for this field in the given environment """
         return False
 
-    def convert_to_cache(self, value, env, validate=True):
+    def convert_to_cache(self, value, record, 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 record: the target record for the assignment, or an empty recordset
+
             :param bool validate: when True, field-specific validation of
                 `value` will be performed
         """
@@ -698,7 +700,7 @@ class Field(object):
         record.ensure_one()
 
         # adapt value to the cache level
-        value = self.convert_to_cache(value, env)
+        value = self.convert_to_cache(value, record)
 
         if env.in_draft or not record.id:
             # determine dependent fields
@@ -861,7 +863,7 @@ class Boolean(Field):
     """ Boolean field. """
     type = 'boolean'
 
-    def convert_to_cache(self, value, env, validate=True):
+    def convert_to_cache(self, value, record, validate=True):
         return bool(value)
 
     def convert_to_export(self, value, env):
@@ -874,7 +876,7 @@ class Integer(Field):
     """ Integer field. """
     type = 'integer'
 
-    def convert_to_cache(self, value, env, validate=True):
+    def convert_to_cache(self, value, record, validate=True):
         return int(value or 0)
 
     def convert_to_read(self, value, use_name_get=True):
@@ -914,7 +916,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, validate=True):
+    def convert_to_cache(self, value, record, 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])
@@ -948,7 +950,7 @@ class Char(_String):
     _related_size = property(attrgetter('size'))
     _description_size = property(attrgetter('size'))
 
-    def convert_to_cache(self, value, env, validate=True):
+    def convert_to_cache(self, value, record, validate=True):
         return bool(value) and ustr(value)[:self.size]
 
 
@@ -962,7 +964,7 @@ class Text(_String):
     """
     type = 'text'
 
-    def convert_to_cache(self, value, env, validate=True):
+    def convert_to_cache(self, value, record, validate=True):
         return bool(value) and ustr(value)
 
 
@@ -970,7 +972,7 @@ class Html(_String):
     """ Html field. """
     type = 'html'
 
-    def convert_to_cache(self, value, env, validate=True):
+    def convert_to_cache(self, value, record, validate=True):
         return bool(value) and html_sanitize(value)
 
 
@@ -1019,7 +1021,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, validate=True):
+    def convert_to_cache(self, value, record, validate=True):
         if not value:
             return False
         if isinstance(value, basestring):
@@ -1084,7 +1086,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, validate=True):
+    def convert_to_cache(self, value, record, validate=True):
         if not value:
             return False
         if isinstance(value, basestring):
@@ -1164,10 +1166,10 @@ class Selection(Field):
             selection = selection(env[self.model_name])
         return [value for value, _ in selection]
 
-    def convert_to_cache(self, value, env, validate=True):
+    def convert_to_cache(self, value, record, validate=True):
         if not validate:
             return value or False
-        if value in self.get_values(env):
+        if value in self.get_values(record.env):
             return value
         elif not value:
             return False
@@ -1204,14 +1206,14 @@ class Reference(Selection):
 
     _column_size = property(attrgetter('size'))
 
-    def convert_to_cache(self, value, env, validate=True):
+    def convert_to_cache(self, value, record, validate=True):
         if isinstance(value, BaseModel):
-            if ((not validate or value._name in self.get_values(env))
+            if ((not validate or value._name in self.get_values(record.env))
                     and len(value) <= 1):
-                return value.with_env(env) or False
+                return value.with_env(record.env) or False
         elif isinstance(value, basestring):
             res_model, res_id = value.split(',')
-            return env[res_model].browse(int(res_id))
+            return record.env[res_model].browse(int(res_id))
         elif not value:
             return False
         raise ValueError("Wrong value for %s: %r" % (self, value))
@@ -1302,19 +1304,19 @@ class Many2one(_Relational):
         """ Update the cached value of `self` for `records` with `value`. """
         records._cache[self] = value
 
-    def convert_to_cache(self, value, env, validate=True):
+    def convert_to_cache(self, value, record, validate=True):
         if isinstance(value, (NoneType, int)):
-            return env[self.comodel_name].browse(value)
+            return record.env[self.comodel_name].browse(value)
         if isinstance(value, BaseModel):
             if value._name == self.comodel_name and len(value) <= 1:
-                return value.with_env(env)
+                return value.with_env(record.env)
             raise ValueError("Wrong value for %s: %r" % (self, value))
         elif isinstance(value, tuple):
-            return env[self.comodel_name].browse(value[0])
+            return record.env[self.comodel_name].browse(value[0])
         elif isinstance(value, dict):
-            return env[self.comodel_name].new(value)
+            return record.env[self.comodel_name].new(value)
         else:
-            return env[self.comodel_name].browse(value)
+            return record.env[self.comodel_name].browse(value)
 
     def convert_to_read(self, value, use_name_get=True):
         if use_name_get and value:
@@ -1355,27 +1357,30 @@ class _RelationalMulti(_Relational):
         for record in records:
             record._cache[self] = record[self.name] | value
 
-    def convert_to_cache(self, value, env, validate=True):
+    def convert_to_cache(self, value, record, validate=True):
         if isinstance(value, BaseModel):
             if value._name == self.comodel_name:
-                return value.with_env(env)
+                return value.with_env(record.env)
         elif isinstance(value, list):
             # value is a list of record ids or commands
-            result = env[self.comodel_name]
+            if not record.id:
+                record = record.browse()        # new record has no value
+            result = record[self.name]
+            # modify result with the commands;
+            # beware to not introduce duplicates in result
             for command in value:
                 if isinstance(command, (tuple, list)):
                     if command[0] == 0:
                         result += result.new(command[2])
                     elif command[0] == 1:
-                        record = result.browse(command[1])
-                        record.update(command[2])
-                        result += record
+                        result.browse(command[1]).update(command[2])
                     elif command[0] == 2:
-                        pass
+                        # note: the record will be deleted by write()
+                        result -= result.browse(command[1])
                     elif command[0] == 3:
-                        pass
+                        result -= result.browse(command[1])
                     elif command[0] == 4:
-                        result += result.browse(command[1])
+                        result += result.browse(command[1]) - result
                     elif command[0] == 5:
                         result = result.browse()
                     elif command[0] == 6:
@@ -1383,10 +1388,10 @@ class _RelationalMulti(_Relational):
                 elif isinstance(command, dict):
                     result += result.new(command)
                 else:
-                    result += result.browse(command)
+                    result += result.browse(command) - result
             return result
         elif not value:
-            return self.null(env)
+            return self.null(record.env)
         raise ValueError("Wrong value for %s: %s" % (self, value))
 
     def convert_to_read(self, value, use_name_get=True):

diff --git a/openerp/models.py b/openerp/models.py
index 7d328de..2f3b90a 100644 (file)
--- a/openerp/models.py
+++ b/openerp/models.py
@@ -3627,7 +3627,8 @@ class BaseModel(object):
 
         # put the values of pure new-style fields into cache, and inverse them
         if new_vals:
-            self._cache.update(self._convert_to_cache(new_vals))
+            for record in self:
+                record._cache.update(record._convert_to_cache(new_vals, update=True))
             for key in new_vals:
                 self._fields[key].determine_inverse(self)
 
@@ -5098,11 +5099,17 @@ 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, validate=True):
-        """ Convert the `values` dictionary into cached values. """
+    def _convert_to_cache(self, values, update=False, validate=True):
+        """ Convert the `values` dictionary into cached values.
+
+            :param update: whether the conversion is made for updating `self`;
+                this is necessary for interpreting the commands of *2many fields
+            :param validate: whether values must be checked
+        """
         fields = self._fields
+        target = self if update else self.browse()
         return {
-            name: fields[name].convert_to_cache(value, self.env, validate=validate)
+            name: fields[name].convert_to_cache(value, target, validate=validate)
             for name, value in values.iteritems()
             if name in fields
         }
@@ -5191,7 +5198,7 @@ class BaseModel(object):
             exist in the database.
         """
         record = self.browse([NewId()])
-        record._cache.update(self._convert_to_cache(values))
+        record._cache.update(record._convert_to_cache(values, update=True))
 
         if record.env.in_onchange:
             # The cache update does not set inverse fields, so do it manually.