From 4c18c5fb6e518e9b36f6f9bb9c279e530ecde255 Mon Sep 17 00:00:00 2001 From: Raphael Collet Date: Wed, 9 Jul 2014 17:06:29 +0200 Subject: [PATCH] [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. --- openerp/fields.py | 71 +++++++++++++++++++++++++---------------------- openerp/models.py | 17 ++++++++---- 2 files changed, 50 insertions(+), 38 deletions(-) diff --git a/openerp/fields.py b/openerp/fields.py index d85ce82eccb..e9fd90805f2 100644 --- 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 7d328dea39b..2f3b90a89b3 100644 --- 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.