From d3cfb47285ef12b9c14cbc953ff8f50d6276812e Mon Sep 17 00:00:00 2001
From: Raphael Collet <rco@openerp.com>
Date: Wed, 23 Mar 2016 13:47:53 +0100
Subject: [PATCH 01/11] [IMP] fields: simple code improvements (add blank
 lines, remove unused imports)

---
 openerp/fields.py | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/openerp/fields.py b/openerp/fields.py
index 5fec7ec90ae..501ecee6887 100644
--- a/openerp/fields.py
+++ b/openerp/fields.py
@@ -1255,6 +1255,7 @@ class Char(_String):
             return False
         return ustr(value)[:self.size]
 
+
 class Text(_String):
     """ Very similar to :class:`~.Char` but used for longer contents, does not
     have a size and usually displayed as a multiline text box.
@@ -1272,6 +1273,7 @@ class Text(_String):
             return False
         return ustr(value)
 
+
 class Html(_String):
     type = 'html'
     _slots = {
@@ -2016,8 +2018,9 @@ class Id(Field):
     def __set__(self, record, value):
         raise TypeError("field 'id' cannot be assigned")
 
+
 # imported here to avoid dependency cycle issues
 from openerp import SUPERUSER_ID
-from .exceptions import Warning, AccessError, MissingError
-from .models import check_pg_name, BaseModel, MAGIC_COLUMNS
+from .exceptions import AccessError, MissingError
+from .models import check_pg_name, BaseModel
 from .osv import fields

From cb8d432bb2211ee666b27ece6de86c9ef9f4f4ea Mon Sep 17 00:00:00 2001
From: Raphael Collet <rco@openerp.com>
Date: Tue, 12 Apr 2016 17:07:05 +0200
Subject: [PATCH 02/11] [IMP] fields: do not validate values when not necessary

---
 openerp/fields.py | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/openerp/fields.py b/openerp/fields.py
index 501ecee6887..4d59bad64bf 100644
--- a/openerp/fields.py
+++ b/openerp/fields.py
@@ -1165,11 +1165,12 @@ class Monetary(Field):
             "Field %s with unknown currency_field %r" % (self, self.currency_field)
 
     def convert_to_cache(self, value, record, validate=True):
-        currency = record[self.currency_field]
-        # FIXME @rco-odoo: currency may not be already initialized if it is a
-        # function or related field!
-        if currency:
-            return currency.round(float(value or 0.0))
+        if validate:
+            currency = record[self.currency_field]
+            # FIXME @rco-odoo: currency may not be already initialized if it is
+            # a function or related field!
+            if currency:
+                return currency.round(float(value or 0.0))
         return float(value or 0.0)
 
 
@@ -1572,8 +1573,7 @@ class Reference(Selection):
 
     def convert_to_cache(self, value, record, validate=True):
         if isinstance(value, BaseModel):
-            if ((not validate or value._name in self.get_values(record.env))
-                    and len(value) <= 1):
+            if not validate or (value._name in self.get_values(record.env) and len(value) <= 1):
                 return value.with_env(record.env) or False
         elif isinstance(value, basestring):
             res_model, res_id = value.split(',')
@@ -1691,7 +1691,7 @@ class Many2one(_Relational):
         if isinstance(value, (NoneType, int, long)):
             return record.env[self.comodel_name].browse(value)
         if isinstance(value, BaseModel):
-            if value._name == self.comodel_name and len(value) <= 1:
+            if not validate or (value._name == self.comodel_name and len(value) <= 1):
                 return value.with_env(record.env)
             raise ValueError("Wrong value for %s: %r" % (self, value))
         elif isinstance(value, tuple):
@@ -1757,7 +1757,7 @@ class _RelationalMulti(_Relational):
 
     def convert_to_cache(self, value, record, validate=True):
         if isinstance(value, BaseModel):
-            if value._name == self.comodel_name:
+            if not validate or (value._name == self.comodel_name):
                 return value.with_env(record.env)
         elif isinstance(value, list):
             # value is a list of record ids or commands

From 6037f3a56d3992cf0a6ff5ea1cb374dcbac03ea0 Mon Sep 17 00:00:00 2001
From: Raphael Collet <rco@openerp.com>
Date: Wed, 23 Mar 2016 13:48:43 +0100
Subject: [PATCH 03/11] [REF] fields: change API and semantics of converter
 methods

The cache data format has been split between a cache format and a record format
(returned by accessing a field on a record).  We have the following converters:

                                   ANY FORMAT
                                       |
                                       | convert_to_cache
                                       v
                                  CACHE FORMAT
                                       |
                                       | convert_to_record
                                       v
                                 RECORD FORMAT
                                       |
                                       | convert_to_read
                                       | convert_to_write
                                       | convert_to_onchange
                                       | convert_to_export
                                       | convert_to_display_name
                                       v
                                 OTHER FORMATS

Modify the API of the converter methods to make them uniform (pass value and
record), and adapt their documentation.
---
 .../wizard/event_edit_registration.py         |   1 -
 addons/mail/wizard/mail_compose_message.py    |   2 +-
 openerp/addons/base/ir/ir_values.py           |   2 +-
 openerp/fields.py                             | 169 +++++++++---------
 openerp/models.py                             |  22 +--
 5 files changed, 98 insertions(+), 98 deletions(-)

diff --git a/addons/event_sale/wizard/event_edit_registration.py b/addons/event_sale/wizard/event_edit_registration.py
index 98f6553f5a6..989737405b8 100644
--- a/addons/event_sale/wizard/event_edit_registration.py
+++ b/addons/event_sale/wizard/event_edit_registration.py
@@ -41,7 +41,6 @@ class SaleOrderEventRegistration(models.TransientModel):
                     'sale_order_line_id': so_line.id,
                 }])
         res['event_registration_ids'] = attendee_list
-        res = self._convert_to_cache(res, validate=False)
         res = self._convert_to_write(res)
         return res
 
diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py
index 34d59736185..6537d6d1cf6 100644
--- a/addons/mail/wizard/mail_compose_message.py
+++ b/addons/mail/wizard/mail_compose_message.py
@@ -398,7 +398,7 @@ class MailComposer(models.TransientModel):
         # ORM handle the assignation of command list on new onchange (api.v8),
         # this force the complete replacement of x2many field with
         # command and is compatible with onchange api.v7
-        values = self._convert_to_write(self._convert_to_cache(values))
+        values = self._convert_to_write(values)
 
         return {'value': values}
 
diff --git a/openerp/addons/base/ir/ir_values.py b/openerp/addons/base/ir/ir_values.py
index 3af0f92249f..ce9f7328ede 100644
--- a/openerp/addons/base/ir/ir_values.py
+++ b/openerp/addons/base/ir/ir_values.py
@@ -402,7 +402,7 @@ class IrValues(models.Model):
             # FIXME: needs cleanup
             try:
                 action_def = {
-                    field: action._fields[field].convert_to_read(action[field])
+                    field: action._fields[field].convert_to_read(action[field], action)
                     for field in fields
                 }
                 if action._name in ('ir.actions.report.xml', 'ir.actions.act_window'):
diff --git a/openerp/fields.py b/openerp/fields.py
index 4d59bad64bf..63e2f2c3df4 100644
--- a/openerp/fields.py
+++ b/openerp/fields.py
@@ -81,7 +81,7 @@ 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)))
+        return api.model(lambda model: field.convert_to_write(value(model), model))
     else:
         return value
 
@@ -758,57 +758,56 @@ class Field(object):
     # Conversion of values
     #
 
-    def null(self, env):
-        """ return the null value for this field in the given environment """
+    def null(self, record):
+        """ Return the null value for this field in the record format. """
         return False
 
     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`
+        """ Convert ``value`` to the cache format; ``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
+        :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`
+    def convert_to_record(self, value, record):
+        """ Convert ``value`` from the cache format to the record format. """
+        return value
 
-            :param bool use_name_get: when True, value's diplay name will
-                be computed using :meth:`BaseModel.name_get`, if relevant
-                for the field
+    def convert_to_read(self, value, record, use_name_get=True):
+        """ Convert ``value`` from the record format to the format returned by
+        method :meth:`BaseModel.read`.
+
+        :param bool use_name_get: when True, the value's display name will be
+            computed using :meth:`BaseModel.name_get`, if relevant for the field
         """
         return False if value is None else value
 
-    def convert_to_write(self, value):
-        """ convert ``value`` from the cache to a valid value for method
-            :meth:`BaseModel.write`.
+    def convert_to_write(self, value, record):
+        """ Convert ``value`` from the record format to the format of method
+        :meth:`BaseModel.write`.
         """
-        return self.convert_to_read(value)
+        return self.convert_to_read(value, record)
 
-    def convert_to_onchange(self, value, fnames=None):
-        """ convert ``value`` from the cache to a value as returned by method
-            :meth:`BaseModel.onchange`.
+    def convert_to_onchange(self, value, record, fnames=()):
+        """ Convert ``value`` from the record format to the format returned by
+        method :meth:`BaseModel.onchange`.
 
-            :param fnames: an optional collection of field names to convert
-                (for relational fields only)
+        :param fnames: an optional collection of field names to convert
+            (for relational fields only)
         """
-        return self.convert_to_read(value)
+        return self.convert_to_read(value, record)
 
-    def convert_to_export(self, value, env):
-        """ convert ``value`` from the cache to a valid value for export. The
-            parameter ``env`` is given for managing translations.
-        """
+    def convert_to_export(self, value, record):
+        """ Convert ``value`` from the record format to the export format. """
         if not value:
             return ''
-        return value if env.context.get('export_raw_data') else ustr(value)
+        return value if record._context.get('export_raw_data') else ustr(value)
 
-    def convert_to_display_name(self, value, record=None):
-        """ convert ``value`` from the cache to a suitable display name. """
+    def convert_to_display_name(self, value, record):
+        """ Convert ``value`` from the record format to a suitable display name. """
         return ustr(value)
 
     ############################################################################
@@ -823,26 +822,22 @@ class Field(object):
 
         if not record:
             # null record -> return the null value for this field
-            return self.null(record.env)
+            return self.null(record)
 
         # only a single record may be accessed
         record.ensure_one()
 
         try:
-            return record._cache[self]
+            value = record._cache[self]
         except KeyError:
-            pass
+            # cache miss, determine value and retrieve it
+            if record.id:
+                self.determine_value(record)
+            else:
+                self.determine_draft_value(record)
+            value = record._cache[self]
 
-        # cache miss, retrieve value
-        if record.id:
-            # normal record -> read or compute value for this field
-            self.determine_value(record)
-        else:
-            # draft record -> compute the value or let it be null
-            self.determine_draft_value(record)
-
-        # the result should be in cache now
-        return record._cache[self]
+        return self.convert_to_record(value, record)
 
     def __set__(self, record, value):
         """ set the value of field ``self`` on ``record`` """
@@ -872,7 +867,8 @@ class Field(object):
 
         else:
             # simply write to the database, and update cache
-            record.write({self.name: self.convert_to_write(value)})
+            write_value = self.convert_to_write(self.convert_to_record(value, record), record)
+            record.write({self.name: write_value})
             record._cache[self] = value
 
     ############################################################################
@@ -885,7 +881,8 @@ class Field(object):
         # initialize the fields to their corresponding null value in cache
         computed = records._field_computed[self]
         for field in computed:
-            records._cache[field] = field.null(records.env)
+            for record in records:
+                record._cache[field] = field.convert_to_cache(False, record, validate=False)
             records.env.computed[field].update(records._ids)
         if isinstance(self.compute, basestring):
             getattr(records, self.compute)()
@@ -947,14 +944,15 @@ class Field(object):
 
         else:
             # this is a non-stored non-computed field
-            record._cache[self] = self.null(env)
+            record._cache[self] = self.convert_to_cache(False, record, validate=False)
 
     def determine_draft_value(self, record):
         """ Determine the value of ``self`` for the given draft ``record``. """
         if self.compute:
             self._compute_value(record)
         else:
-            record._cache[self] = SpecialValue(self.null(record.env))
+            null = self.convert_to_cache(False, record, validate=False)
+            record._cache[self] = SpecialValue(null)
 
     def determine_inverse(self, records):
         """ Given the value of ``self`` on ``records``, inverse the computation. """
@@ -1047,8 +1045,8 @@ class Boolean(Field):
     def convert_to_cache(self, value, record, validate=True):
         return bool(value)
 
-    def convert_to_export(self, value, env):
-        if env.context.get('export_raw_data'):
+    def convert_to_export(self, value, record):
+        if record._context.get('export_raw_data'):
             return value
         return ustr(value)
 
@@ -1068,7 +1066,7 @@ class Integer(Field):
             return value.get('id', False)
         return int(value or 0)
 
-    def convert_to_read(self, value, use_name_get=True):
+    def convert_to_read(self, value, record, use_name_get=True):
         # Integer values greater than 2^31-1 are not supported in pure XMLRPC,
         # so we have to pass them as floats :-(
         if value and value > xmlrpclib.MAXINT:
@@ -1079,9 +1077,9 @@ class Integer(Field):
         # special case, when an integer field is used as inverse for a one2many
         records._cache[self] = value.id or 0
 
-    def convert_to_export(self, value, env):
+    def convert_to_export(self, value, record):
         if value or value == 0:
-            return value if env.context.get('export_raw_data') else ustr(value)
+            return value if record._context.get('export_raw_data') else ustr(value)
         return ''
 
 
@@ -1125,9 +1123,9 @@ class Float(Field):
         digits = self.digits
         return float_round(value, precision_digits=digits[1]) if digits else value
 
-    def convert_to_export(self, value, env):
+    def convert_to_export(self, value, record):
         if value or value == 0.0:
-            return value if env.context.get('export_raw_data') else ustr(value)
+            return value if record._context.get('export_raw_data') else ustr(value)
         return ''
 
 
@@ -1365,10 +1363,10 @@ class Date(Field):
             return value[:DATE_LENGTH]
         return self.to_string(value)
 
-    def convert_to_export(self, value, env):
+    def convert_to_export(self, value, record):
         if not value:
             return ''
-        return self.from_string(value) if env.context.get('export_raw_data') else ustr(value)
+        return self.from_string(value) if record._context.get('export_raw_data') else ustr(value)
 
 
 class Datetime(Field):
@@ -1436,12 +1434,12 @@ class Datetime(Field):
             return value
         return self.to_string(value)
 
-    def convert_to_export(self, value, env):
+    def convert_to_export(self, value, record):
         if not value:
             return ''
-        return self.from_string(value) if env.context.get('export_raw_data') else ustr(value)
+        return self.from_string(value) if record._context.get('export_raw_data') else ustr(value)
 
-    def convert_to_display_name(self, value, record=None):
+    def convert_to_display_name(self, value, record):
         assert record, 'Record expected'
         return Datetime.to_string(Datetime.context_timestamp(record, Datetime.from_string(value)))
 
@@ -1547,11 +1545,11 @@ class Selection(Field):
             return False
         raise ValueError("Wrong value for %s: %r" % (self, value))
 
-    def convert_to_export(self, value, env):
+    def convert_to_export(self, value, record):
         if not isinstance(self.selection, list):
             # FIXME: this reproduces an existing buggy behavior!
             return value if value else ''
-        for item in self._description_selection(env):
+        for item in self._description_selection(record.env):
             if item[0] == value:
                 return item[1]
         return False
@@ -1582,13 +1580,13 @@ class Reference(Selection):
             return False
         raise ValueError("Wrong value for %s: %r" % (self, value))
 
-    def convert_to_read(self, value, use_name_get=True):
+    def convert_to_read(self, value, record, use_name_get=True):
         return "%s,%s" % (value._name, value.id) if value else False
 
-    def convert_to_export(self, value, env):
+    def convert_to_export(self, value, record):
         return value.name_get()[0][1] if value else ''
 
-    def convert_to_display_name(self, value, record=None):
+    def convert_to_display_name(self, value, record):
         return ustr(value and value.display_name)
 
 
@@ -1627,8 +1625,8 @@ class _Relational(Field):
     _column_domain = property(attrgetter('domain'))
     _column_context = property(attrgetter('context'))
 
-    def null(self, env):
-        return env[self.comodel_name]
+    def null(self, record):
+        return record.env[self.comodel_name]
 
     def modified(self, records):
         # Invalidate cache for inverse fields, too. Note that the recomputation
@@ -1699,9 +1697,9 @@ class Many2one(_Relational):
         elif isinstance(value, dict):
             return record.env[self.comodel_name].new(value)
         else:
-            return self.null(record.env)
+            return record.env[self.comodel_name]
 
-    def convert_to_read(self, value, use_name_get=True):
+    def convert_to_read(self, value, record, use_name_get=True):
         if use_name_get and value:
             # evaluate name_get() as superuser, because the visibility of a
             # many2one field value (id and name) depends on the current record's
@@ -1718,13 +1716,13 @@ class Many2one(_Relational):
         else:
             return value.id
 
-    def convert_to_write(self, value):
+    def convert_to_write(self, value, record):
         return value.id
 
-    def convert_to_export(self, value, env):
+    def convert_to_export(self, value, record):
         return value.name_get()[0][1] if value else ''
 
-    def convert_to_display_name(self, value, record=None):
+    def convert_to_display_name(self, value, record):
         return ustr(value.display_name)
 
 
@@ -1790,13 +1788,13 @@ class _RelationalMulti(_Relational):
             # return result as a recordset
             return comodel.browse(list(ids))
         elif not value:
-            return self.null(record.env)
+            return record.env[self.comodel_name]
         raise ValueError("Wrong value for %s: %s" % (self, value))
 
-    def convert_to_read(self, value, use_name_get=True):
+    def convert_to_read(self, value, record, use_name_get=True):
         return value.ids
 
-    def convert_to_write(self, value):
+    def convert_to_write(self, value, record):
         # make result with new and existing records
         result = [(5,)]
         for record in value:
@@ -1805,21 +1803,22 @@ class _RelationalMulti(_Relational):
                 values = record._convert_to_write(values)
                 result.append((0, 0, values))
             elif record._is_dirty():
-                values = {k: record._cache[k] for k in record._get_dirty()}
+                values = {name: record[name] for name in record._get_dirty()}
                 values = record._convert_to_write(values)
                 result.append((1, record.id, values))
             else:
                 result.append((4, record.id))
         return result
 
-    def convert_to_onchange(self, value, fnames=None):
+    def convert_to_onchange(self, value, record, fnames=()):
         # return the recordset value as a list of commands; the commands may
         # give all fields values, the client is responsible for figuring out
         # which fields are actually dirty
-        fields = [(name, value._fields[name]) for name in (fnames or []) if name != 'id']
+        converters = [(name, value._fields[name].convert_to_onchange)
+                      for name in fnames if name != 'id']
         result = [(5,)]
         for record in value:
-            vals = {name: field.convert_to_onchange(record[name]) for name, field in fields}
+            vals = {name: convert(record[name], record) for name, convert in converters}
             if not record.id:
                 result.append((0, 0, vals))
             elif vals:
@@ -1828,10 +1827,10 @@ class _RelationalMulti(_Relational):
                 result.append((4, record.id))
         return result
 
-    def convert_to_export(self, value, env):
+    def convert_to_export(self, value, record):
         return ','.join(name for id, name in value.name_get()) if value else ''
 
-    def convert_to_display_name(self, value, record=None):
+    def convert_to_display_name(self, value, record):
         raise NotImplementedError()
 
     def _compute_related(self, records):
@@ -1900,11 +1899,11 @@ class One2many(_RelationalMulti):
     _column_auto_join = property(attrgetter('auto_join'))
     _column_limit = property(attrgetter('limit'))
 
-    def convert_to_onchange(self, value, fnames=None):
+    def convert_to_onchange(self, value, record, fnames=()):
         if fnames:
             # do not serialize self's inverse field
             fnames = [name for name in fnames if name != self.inverse_name]
-        return super(One2many, self).convert_to_onchange(value, fnames)
+        return super(One2many, self).convert_to_onchange(value, record, fnames)
 
 
 class Many2many(_RelationalMulti):
diff --git a/openerp/models.py b/openerp/models.py
index 0f49f5f222e..6d88734a913 100644
--- a/openerp/models.py
+++ b/openerp/models.py
@@ -945,7 +945,7 @@ class BaseModel(object):
                     # this part could be simpler, but it has to be done this way
                     # in order to reproduce the former behavior
                     if not isinstance(value, BaseModel):
-                        current[i] = field.convert_to_export(value, self.env)
+                        current[i] = field.convert_to_export(value, record)
                     else:
                         primary_done.append(name)
 
@@ -1383,7 +1383,6 @@ class BaseModel(object):
                 parent_fields[field.model_name].append(field.name)
 
         # convert default values to the right format
-        defaults = self._convert_to_cache(defaults, validate=False)
         defaults = self._convert_to_write(defaults)
 
         # add default values for inherited fields
@@ -3393,7 +3392,7 @@ class BaseModel(object):
             try:
                 values = {'id': record.id}
                 for name, field in name_fields:
-                    values[name] = field.convert_to_read(record[name], use_name_get)
+                    values[name] = field.convert_to_read(record[name], record, use_name_get)
                 result.append(values)
             except MissingError:
                 pass
@@ -5650,7 +5649,10 @@ class BaseModel(object):
         result = {}
         for name, value in values.iteritems():
             if name in fields:
-                value = fields[name].convert_to_write(value)
+                field = fields[name]
+                value = field.convert_to_cache(value, self, validate=False)
+                value = field.convert_to_record(value, self)
+                value = field.convert_to_write(value, self)
                 if not isinstance(value, NewId):
                     result[name] = value
         return result
@@ -5696,8 +5698,8 @@ class BaseModel(object):
         recs = self
         for name in name_seq.split('.'):
             field = recs._fields[name]
-            null = field.null(self.env)
-            recs = recs.mapped(lambda rec: rec._cache.get(field, null))
+            null = field.convert_to_cache(False, self, validate=False)
+            recs = recs.mapped(lambda rec: field.convert_to_record(rec._cache.get(field, null), rec))
         return recs
 
     def filtered(self, func):
@@ -6111,9 +6113,9 @@ class BaseModel(object):
                 def __init__(self, record):
                     self._record = record
                 def __getitem__(self, name):
-                    field = self._record._fields[name]
-                    value = self._record[name]
-                    return field.convert_to_write(value)
+                    record = self._record
+                    field = record._fields[name]
+                    return field.convert_to_write(record[name], record)
                 def __getattr__(self, name):
                     return self[name]
 
@@ -6228,7 +6230,7 @@ class BaseModel(object):
 
         # collect values from dirty fields
         result['value'] = {
-            name: self._fields[name].convert_to_onchange(record[name], subfields.get(name))
+            name: self._fields[name].convert_to_onchange(record[name], record, subfields[name])
             for name in dirty
         }
 

From 8bc7d8565c3e4f954c479e3b51b863e4c03dc582 Mon Sep 17 00:00:00 2001
From: Raphael Collet <rco@openerp.com>
Date: Wed, 23 Mar 2016 17:12:59 +0100
Subject: [PATCH 04/11] [IMP] models: attach prefetching data to recordsets
 instead of environments

The prefetching now has a more intuitive and controllable model: every
recordset is linked to a "prefetch object" that tells which record ids to
prefetch for any model.  The prefetch object of a given recordset is shared
with the records obtained by iteration and relational field access.  All other
operations instantiate recordsets with a fresh prefetch object.

    records = model.browse(ids)
    assert records._prefetch is not model._prefetch

    for record in records:
        assert record._prefetch is records._prefetch
        assert record.relation._prefetch is record._prefetch
---
 openerp/addons/base/tests/test_api.py         | 84 +++++++++++++++----
 .../test_new_api/tests/test_new_fields.py     | 19 +++--
 openerp/api.py                                |  4 +-
 openerp/fields.py                             | 47 ++++++-----
 openerp/models.py                             | 41 +++++----
 5 files changed, 130 insertions(+), 65 deletions(-)

diff --git a/openerp/addons/base/tests/test_api.py b/openerp/addons/base/tests/test_api.py
index ec4be747a73..5bd098b1e88 100644
--- a/openerp/addons/base/tests/test_api.py
+++ b/openerp/addons/base/tests/test_api.py
@@ -324,29 +324,77 @@ class TestAPI(common.TransactionCase):
         self.env.check_cache()
 
     @mute_logger('openerp.models')
-    def test_60_cache_prefetching(self):
+    def test_60_prefetch(self):
         """ Check the record cache prefetching """
-        self.env.invalidate_all()
+        partners = self.env['res.partner'].search([], limit=models.PREFETCH_MAX)
+        self.assertTrue(len(partners) > 1)
 
-        # all the records of an instance already have an entry in cache
-        partners = self.env['res.partner'].search([])
-        partner_ids = self.env.prefetch['res.partner']
-        self.assertEqual(set(partners.ids), set(partner_ids))
-
-        # countries have not been fetched yet; their cache must be empty
-        countries = self.env['res.country'].browse()
-        self.assertFalse(self.env.prefetch['res.country'])
+        # all the records in partners are ready for prefetching
+        self.assertItemsEqual(partners.ids, partners._prefetch['res.partner'])
 
         # reading ONE partner should fetch them ALL
-        countries |= partners[0].country_id
-        country_cache = self.env.cache[partners._fields['country_id']]
-        self.assertLessEqual(set(partners._ids), set(country_cache))
+        partner = next(p for p in partners)
+        partner.country_id
+        country_id_cache = self.env.cache[type(partners).country_id]
+        self.assertItemsEqual(partners.ids, country_id_cache)
 
-        # read all partners, and check that the cache already contained them
-        country_ids = list(self.env.prefetch['res.country'])
-        for p in partners:
-            countries |= p.country_id
-        self.assertLessEqual(set(countries.ids), set(country_ids))
+        # partners' countries are ready for prefetching
+        country_ids = set(c.id for c in country_id_cache.itervalues() if c)
+        self.assertTrue(len(country_ids) > 1)
+        self.assertItemsEqual(country_ids, partners._prefetch['res.country'])
+
+        # reading ONE partner country should fetch ALL partners' countries
+        country = next(p.country_id for p in partners if p.country_id)
+        country.name
+        name_cache = self.env.cache[type(country).name]
+        self.assertItemsEqual(country_ids, name_cache)
+
+    @mute_logger('openerp.models')
+    def test_60_prefetch_object(self):
+        """ Check the prefetching model. """
+        partners = self.env['res.partner'].search([], limit=models.PREFETCH_MAX)
+        self.assertTrue(partners)
+
+        def same_prefetch(a, b):
+            self.assertIs(a._prefetch, b._prefetch)
+        def diff_prefetch(a, b):
+            self.assertIsNot(a._prefetch, b._prefetch)
+
+        # the recordset operations below should create new prefetch objects
+        diff_prefetch(partners, partners.browse())
+        diff_prefetch(partners, partners.browse(partners.ids))
+        diff_prefetch(partners, partners[0])
+        diff_prefetch(partners, partners[:10])
+
+        # iterating and reading relational fields should pass the prefetch object
+        self.assertEqual(type(partners).country_id.type, 'many2one')
+        self.assertEqual(type(partners).bank_ids.type, 'one2many')
+        self.assertEqual(type(partners).category_id.type, 'many2many')
+
+        vals0 = {
+            'name': 'Empty relational fields',
+            'country_id': False,
+            'bank_ids': [],
+            'category_id': [],
+        }
+        vals1 = {
+            'name': 'Non-empty relational fields',
+            'country_id': self.ref('base.be'),
+            'bank_ids': [(0, 0, {'acc_number': 'FOO42'})],
+            'category_id': [(4, self.ref('base.res_partner_category_0'))],
+        }
+        partners = partners.create(vals0) + partners.create(vals1)
+        for partner in partners:
+            same_prefetch(partners, partner)
+            same_prefetch(partners, partner.country_id)
+            same_prefetch(partners, partner.bank_ids)
+            same_prefetch(partners, partner.category_id)
+
+        # same with empty recordsets
+        empty = partners.browse()
+        same_prefetch(empty, empty.country_id)
+        same_prefetch(empty, empty.bank_ids)
+        same_prefetch(empty, empty.category_id)
 
     @mute_logger('openerp.models')
     def test_70_one(self):
diff --git a/openerp/addons/test_new_api/tests/test_new_fields.py b/openerp/addons/test_new_api/tests/test_new_fields.py
index ed3e340e681..174fabf2be1 100644
--- a/openerp/addons/test_new_api/tests/test_new_fields.py
+++ b/openerp/addons/test_new_api/tests/test_new_fields.py
@@ -385,20 +385,21 @@ class TestNewFields(common.TransactionCase):
     def test_31_prefetch(self):
         """ test prefetch of records handle AccessError """
         Category = self.env['test_new_api.category']
-        cat_1 = Category.create({'name': 'NOACCESS'}).id
-        cat_2 = Category.create({'name': 'ACCESS', 'parent': cat_1}).id
+        cat1 = Category.create({'name': 'NOACCESS'})
+        cat2 = Category.create({'name': 'ACCESS', 'parent': cat1.id})
+        cats = cat1 + cat2
 
         self.env.clear()
 
-        cat = Category.browse(cat_2)
-        self.assertEqual(cat.name, 'ACCESS')
-        # both categories should be in prefetch ids
-        self.assertSetEqual(self.env.prefetch[Category._name], set([cat_1, cat_2]))
+        cat1, cat2 = cats
+        self.assertEqual(cat2.name, 'ACCESS')
+        # both categories should be ready for prefetching
+        self.assertItemsEqual(cat2._prefetch[Category._name], cats.ids)
         # but due to our (lame) overwrite of `read`, it should not forbid us to read records we have access to
-        self.assertFalse(len(cat.discussions))
-        self.assertEqual(cat.parent.id, cat_1)
+        self.assertFalse(cat2.discussions)
+        self.assertEqual(cat2.parent, cat1)
         with self.assertRaises(AccessError):
-            Category.browse(cat_1).name
+            cat1.name
 
     def test_40_new(self):
         """ test new records. """
diff --git a/openerp/api.py b/openerp/api.py
index a5a389d9104..1707fc05e6a 100644
--- a/openerp/api.py
+++ b/openerp/api.py
@@ -865,7 +865,6 @@ class Environment(object):
         self.cr, self.uid, self.context = self.args = (cr, uid, frozendict(context))
         self.registry = RegistryManager.get(cr.dbname)
         self.cache = defaultdict(dict)      # {field: {id: value, ...}, ...}
-        self.prefetch = defaultdict(set)    # {model_name: set(id), ...}
         self.computed = defaultdict(set)    # {field: set(id), ...}
         self.dirty = defaultdict(set)       # {record: set(field_name), ...}
         self.all = envs
@@ -878,7 +877,7 @@ class Environment(object):
 
     def __getitem__(self, model_name):
         """ Return an empty recordset from the given model. """
-        return self.registry[model_name]._browse(self, ())
+        return self.registry[model_name]._browse((), self)
 
     def __iter__(self):
         """ Return an iterator on model names. """
@@ -973,7 +972,6 @@ class Environment(object):
         """ Clear the cache of all environments. """
         for env in list(self.all):
             env.cache.clear()
-            env.prefetch.clear()
             env.computed.clear()
             env.dirty.clear()
 
diff --git a/openerp/fields.py b/openerp/fields.py
index 63e2f2c3df4..62b8d47e384 100644
--- a/openerp/fields.py
+++ b/openerp/fields.py
@@ -773,7 +773,10 @@ class Field(object):
         return value
 
     def convert_to_record(self, value, record):
-        """ Convert ``value`` from the cache format to the record format. """
+        """ Convert ``value`` from the cache format to the record format.
+        If the value represents a recordset, it should share the prefetching of
+        ``record``.
+        """
         return value
 
     def convert_to_read(self, value, record, use_name_get=True):
@@ -820,22 +823,21 @@ class Field(object):
         if record is None:
             return self         # the field is accessed through the owner class
 
-        if not record:
+        if record:
+            # only a single record may be accessed
+            record.ensure_one()
+            try:
+                value = record._cache[self]
+            except KeyError:
+                # cache miss, determine value and retrieve it
+                if record.id:
+                    self.determine_value(record)
+                else:
+                    self.determine_draft_value(record)
+                value = record._cache[self]
+        else:
             # null record -> return the null value for this field
-            return self.null(record)
-
-        # only a single record may be accessed
-        record.ensure_one()
-
-        try:
-            value = record._cache[self]
-        except KeyError:
-            # cache miss, determine value and retrieve it
-            if record.id:
-                self.determine_value(record)
-            else:
-                self.determine_draft_value(record)
-            value = record._cache[self]
+            value = self.convert_to_cache(False, record, validate=False)
 
         return self.convert_to_record(value, record)
 
@@ -1699,16 +1701,18 @@ class Many2one(_Relational):
         else:
             return record.env[self.comodel_name]
 
+    def convert_to_record(self, value, record):
+        return value.with_prefetch(record._prefetch)
+
     def convert_to_read(self, value, record, use_name_get=True):
         if use_name_get and value:
             # evaluate name_get() as superuser, because the visibility of a
             # many2one field value (id and name) depends on the current record's
             # access rights, and not the value's access rights.
             try:
-                value_sudo = value.sudo()
-                # performance trick: make sure that all records of the same
-                # model as value in value.env will be prefetched in value_sudo.env
-                value_sudo.env.prefetch[value._name].update(value.env.prefetch[value._name])
+                # performance trick: make sure that all records prefetched with
+                # value will be prefetched with value_sudo
+                value_sudo = value.sudo().with_prefetch(value._prefetch)
                 return value_sudo.name_get()[0]
             except MissingError:
                 # Should not happen, unless the foreign key is missing.
@@ -1791,6 +1795,9 @@ class _RelationalMulti(_Relational):
             return record.env[self.comodel_name]
         raise ValueError("Wrong value for %s: %s" % (self, value))
 
+    def convert_to_record(self, value, record):
+        return value.with_prefetch(record._prefetch)
+
     def convert_to_read(self, value, record, use_name_get=True):
         return value.ids
 
diff --git a/openerp/models.py b/openerp/models.py
index 6d88734a913..ce0da71c226 100644
--- a/openerp/models.py
+++ b/openerp/models.py
@@ -5509,24 +5509,30 @@ class BaseModel(object):
     #
 
     @classmethod
-    def _browse(cls, env, ids):
-        """ Create an instance attached to ``env``; ``ids`` is a tuple of record
-            ids.
+    def _browse(cls, ids, env, prefetch=None):
+        """ Create a recordset instance.
+
+        :param ids: a tuple of record ids
+        :param env: an environment
+        :param prefetch: an optional prefetch object
         """
         records = object.__new__(cls)
         records.env = env
         records._ids = ids
-        env.prefetch[cls._name].update(ids)
+        if prefetch is None:
+            prefetch = defaultdict(set)         # {model_name: set(ids)}
+        records._prefetch = prefetch
+        prefetch[cls._name].update(ids)
         return records
 
     @api.v7
     def browse(self, cr, uid, arg=None, context=None):
         ids = _normalize_ids(arg)
         #assert all(isinstance(id, IdType) for id in ids), "Browsing invalid ids: %s" % ids
-        return self._browse(Environment(cr, uid, context or {}), ids)
+        return self._browse(ids, Environment(cr, uid, context or {}))
 
     @api.v8
-    def browse(self, arg=None):
+    def browse(self, arg=None, prefetch=None):
         """ browse([ids]) -> records
 
         Returns a recordset for the ids provided as parameter in the current
@@ -5536,7 +5542,7 @@ class BaseModel(object):
         """
         ids = _normalize_ids(arg)
         #assert all(isinstance(id, IdType) for id in ids), "Browsing invalid ids: %s" % ids
-        return self._browse(self.env, ids)
+        return self._browse(ids, self.env, prefetch)
 
     #
     # Internal properties, for manipulating the instance's implementation
@@ -5577,7 +5583,7 @@ class BaseModel(object):
 
         :type env: :class:`~openerp.api.Environment`
         """
-        return self._browse(env, self._ids)
+        return self._browse(self._ids, env)
 
     def sudo(self, user=SUPERUSER_ID):
         """ sudo([user=SUPERUSER])
@@ -5628,6 +5634,14 @@ class BaseModel(object):
         context = dict(args[0] if args else self._context, **kwargs)
         return self.with_env(self.env(context=context))
 
+    def with_prefetch(self, prefetch=None):
+        """ with_prefetch([prefetch]) -> records
+
+        Return a new version of this recordset that uses the given prefetch
+        object, or a new prefetch object if not given.
+        """
+        return self._browse(self._ids, self.env, prefetch)
+
     def _convert_to_cache(self, values, update=False, validate=True):
         """ Convert the ``values`` dictionary into cached values.
 
@@ -5799,7 +5813,7 @@ class BaseModel(object):
     def __iter__(self):
         """ Return an iterator over ``self``. """
         for id in self._ids:
-            yield self._browse(self.env, (id,))
+            yield self._browse((id,), self.env, self._prefetch)
 
     def __contains__(self, item):
         """ Test whether ``item`` (record or field name) is an element of ``self``.
@@ -5909,9 +5923,9 @@ class BaseModel(object):
             # important: one must call the field's getter
             return self._fields[key].__get__(self, type(self))
         elif isinstance(key, slice):
-            return self._browse(self.env, self._ids[key])
+            return self._browse(self._ids[key], self.env)
         else:
-            return self._browse(self.env, (self._ids[key],))
+            return self._browse((self._ids[key],), self.env)
 
     def __setitem__(self, key, value):
         """ Assign the field ``key`` to ``value`` in record ``self``. """
@@ -5933,10 +5947,7 @@ class BaseModel(object):
             the records of model ``self`` in cache that have no value for ``field``
             (:class:`Field` instance).
         """
-        env = self.env
-        prefetch_ids = env.prefetch[self._name]
-        prefetch_ids.update(self._ids)
-        ids = filter(None, prefetch_ids - set(env.cache[field]))
+        ids = filter(None, self._prefetch[self._name] - set(self.env.cache[field]))
         return self.browse(ids)
 
     @api.model

From f9b08f09b2c66eb24be79e6df2ef7fefaa6dac73 Mon Sep 17 00:00:00 2001
From: Raphael Collet <rco@openerp.com>
Date: Thu, 24 Mar 2016 16:30:45 +0100
Subject: [PATCH 05/11] [IMP] models: make `with_env`, `sudo`, `with_context`
 preserve the prefetch object

---
 openerp/addons/base/tests/test_api.py | 5 +++++
 openerp/fields.py                     | 6 ++----
 openerp/models.py                     | 8 +++++++-
 3 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/openerp/addons/base/tests/test_api.py b/openerp/addons/base/tests/test_api.py
index 5bd098b1e88..6f623b3f3f4 100644
--- a/openerp/addons/base/tests/test_api.py
+++ b/openerp/addons/base/tests/test_api.py
@@ -366,6 +366,11 @@ class TestAPI(common.TransactionCase):
         diff_prefetch(partners, partners[0])
         diff_prefetch(partners, partners[:10])
 
+        # the recordset operations below should pass the prefetch object
+        same_prefetch(partners, partners.sudo(self.env.ref('base.user_demo')))
+        same_prefetch(partners, partners.with_context(active_test=False))
+        same_prefetch(partners, partners[:10].with_prefetch(partners._prefetch))
+
         # iterating and reading relational fields should pass the prefetch object
         self.assertEqual(type(partners).country_id.type, 'many2one')
         self.assertEqual(type(partners).bank_ids.type, 'one2many')
diff --git a/openerp/fields.py b/openerp/fields.py
index 62b8d47e384..9c9e6cdf7a1 100644
--- a/openerp/fields.py
+++ b/openerp/fields.py
@@ -1710,10 +1710,8 @@ class Many2one(_Relational):
             # many2one field value (id and name) depends on the current record's
             # access rights, and not the value's access rights.
             try:
-                # performance trick: make sure that all records prefetched with
-                # value will be prefetched with value_sudo
-                value_sudo = value.sudo().with_prefetch(value._prefetch)
-                return value_sudo.name_get()[0]
+                # performance: value.sudo() prefetches the same records as value
+                return value.sudo().name_get()[0]
             except MissingError:
                 # Should not happen, unless the foreign key is missing.
                 return False
diff --git a/openerp/models.py b/openerp/models.py
index ce0da71c226..8c9c183903e 100644
--- a/openerp/models.py
+++ b/openerp/models.py
@@ -5580,10 +5580,11 @@ class BaseModel(object):
             The new environment will not benefit from the current
             environment's data cache, so later data access may incur extra
             delays while re-fetching from the database.
+            The returned recordset has the same prefetch object as ``self``.
 
         :type env: :class:`~openerp.api.Environment`
         """
-        return self._browse(self._ids, env)
+        return self._browse(self._ids, env, self._prefetch)
 
     def sudo(self, user=SUPERUSER_ID):
         """ sudo([user=SUPERUSER])
@@ -5611,6 +5612,7 @@ class BaseModel(object):
             re-evaluated, the new recordset will not benefit from the current
             environment's data cache, so later data access may incur extra
             delays while re-fetching from the database.
+            The returned recordset has the same prefetch object as ``self``.
 
         """
         return self.with_env(self.env(user=user))
@@ -5630,6 +5632,10 @@ class BaseModel(object):
             # -> r2._context is {'key2': True}
             r2 = records.with_context(key2=True)
             # -> r2._context is {'key1': True, 'key2': True}
+
+        .. note:
+
+            The returned recordset has the same prefetch object as ``self``.
         """
         context = dict(args[0] if args else self._context, **kwargs)
         return self.with_env(self.env(context=context))

From d035828778cb0654bbc13ad1fe594fac10e2b917 Mon Sep 17 00:00:00 2001
From: Raphael Collet <rco@openerp.com>
Date: Fri, 25 Mar 2016 13:57:24 +0100
Subject: [PATCH 06/11] [FIX] models, fields: make `read` add secondary records
 for prefetching

Make the following statements add secondary records (values of relational fields)
in the prefetching object of ``self``:

    self.read(fields)
    self.partner_id             # implicitly invokes read()

This makes sure that traversing relational fields on records automatically
feeds the prefetching of secondary records.
---
 openerp/fields.py | 17 +++++++++--------
 openerp/models.py | 12 +++++++-----
 2 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/openerp/fields.py b/openerp/fields.py
index 9c9e6cdf7a1..8f3d9cd10fc 100644
--- a/openerp/fields.py
+++ b/openerp/fields.py
@@ -765,7 +765,8 @@ class Field(object):
     def convert_to_cache(self, value, record, validate=True):
         """ Convert ``value`` to the cache format; ``value`` may come from an
         assignment, or have the format of methods :meth:`BaseModel.read` or
-        :meth:`BaseModel.write`.
+        :meth:`BaseModel.write`. If the value represents a recordset, it should
+        be added for prefetching on ``record``.
 
         :param bool validate: when True, field-specific validation of ``value``
             will be performed
@@ -1574,10 +1575,10 @@ class Reference(Selection):
     def convert_to_cache(self, value, record, validate=True):
         if isinstance(value, BaseModel):
             if not validate or (value._name in self.get_values(record.env) and len(value) <= 1):
-                return value.with_env(record.env) or False
+                return value.with_env(record.env).with_prefetch(record._prefetch) or False
         elif isinstance(value, basestring):
             res_model, res_id = value.split(',')
-            return record.env[res_model].browse(int(res_id))
+            return record.env[res_model].browse(int(res_id), record._prefetch)
         elif not value:
             return False
         raise ValueError("Wrong value for %s: %r" % (self, value))
@@ -1689,13 +1690,13 @@ class Many2one(_Relational):
 
     def convert_to_cache(self, value, record, validate=True):
         if isinstance(value, (NoneType, int, long)):
-            return record.env[self.comodel_name].browse(value)
+            return record.env[self.comodel_name].browse(value, record._prefetch)
         if isinstance(value, BaseModel):
             if not validate or (value._name == self.comodel_name and len(value) <= 1):
-                return value.with_env(record.env)
+                return value.with_env(record.env).with_prefetch(record._prefetch)
             raise ValueError("Wrong value for %s: %r" % (self, value))
         elif isinstance(value, tuple):
-            return record.env[self.comodel_name].browse(value[0])
+            return record.env[self.comodel_name].browse(value[0], record._prefetch)
         elif isinstance(value, dict):
             return record.env[self.comodel_name].new(value)
         else:
@@ -1758,7 +1759,7 @@ class _RelationalMulti(_Relational):
     def convert_to_cache(self, value, record, validate=True):
         if isinstance(value, BaseModel):
             if not validate or (value._name == self.comodel_name):
-                return value.with_env(record.env)
+                return value.with_env(record.env).with_prefetch(record._prefetch)
         elif isinstance(value, list):
             # value is a list of record ids or commands
             comodel = record.env[self.comodel_name]
@@ -1788,7 +1789,7 @@ class _RelationalMulti(_Relational):
                 else:
                     ids.add(command)
             # return result as a recordset
-            return comodel.browse(list(ids))
+            return comodel.browse(list(ids), record._prefetch)
         elif not value:
             return record.env[self.comodel_name]
         raise ValueError("Wrong value for %s: %s" % (self, value))
diff --git a/openerp/models.py b/openerp/models.py
index 8c9c183903e..721cce01edc 100644
--- a/openerp/models.py
+++ b/openerp/models.py
@@ -3380,7 +3380,8 @@ class BaseModel(object):
             else:
                 _logger.warning("%s.read() with unknown field '%s'", self._name, name)
 
-        # fetch stored fields from the database to the cache
+        # fetch stored fields from the database to the cache; this should feed
+        # the prefetching of secondary records
         self._read_from_database(stored, inherited)
 
         # retrieve results from records; this takes values from the cache and
@@ -3449,6 +3450,7 @@ class BaseModel(object):
 
         # fetch records with read()
         assert self in records and field in fs
+        records = records.with_prefetch(self._prefetch)
         result = []
         try:
             result = records.read([f.name for f in fs], load='_classic_write')
@@ -3459,7 +3461,7 @@ class BaseModel(object):
         # check the cache, and update it if necessary
         if field not in self._cache:
             for values in result:
-                record = self.browse(values.pop('id'))
+                record = self.browse(values.pop('id'), self._prefetch)
                 record._cache.update(record._convert_to_cache(values, validate=False))
             if not self._cache.contains(field):
                 e = AccessError("No value found for %s.%s" % (self, field.name))
@@ -3545,7 +3547,7 @@ class BaseModel(object):
 
             # store result in cache for POST fields
             for vals in result:
-                record = self.browse(vals['id'])
+                record = self.browse(vals['id'], self._prefetch)
                 record._cache.update(record._convert_to_cache(vals, validate=False))
 
             # determine the fields that must be processed now;
@@ -3587,7 +3589,7 @@ class BaseModel(object):
 
         # store result in cache
         for vals in result:
-            record = self.browse(vals.pop('id'))
+            record = self.browse(vals.pop('id'), self._prefetch)
             record._cache.update(record._convert_to_cache(vals, validate=False))
 
         # store failed values in cache for the records that could not be read
@@ -5656,7 +5658,7 @@ class BaseModel(object):
             :param validate: whether values must be checked
         """
         fields = self._fields
-        target = self if update else self.browse()
+        target = self if update else self.browse([], self._prefetch)
         return {
             name: fields[name].convert_to_cache(value, target, validate=validate)
             for name, value in values.iteritems()

From 1ec7b28a09bead5b5f1f28b38ca4bf203ecd8b09 Mon Sep 17 00:00:00 2001
From: Raphael Collet <rco@openerp.com>
Date: Wed, 30 Mar 2016 11:10:32 +0200
Subject: [PATCH 07/11] [FIX] models, fields, api: do not mix the cache format
 with the record format

---
 openerp/api.py    |  8 ++++++--
 openerp/fields.py | 35 +++++++++++++++++++----------------
 openerp/models.py | 13 +++++++++++--
 3 files changed, 36 insertions(+), 20 deletions(-)

diff --git a/openerp/api.py b/openerp/api.py
index 1707fc05e6a..0ec3342cd74 100644
--- a/openerp/api.py
+++ b/openerp/api.py
@@ -1036,6 +1036,8 @@ class Environment(object):
 
     def check_cache(self):
         """ Check the cache consistency. """
+        from openerp.fields import SpecialValue
+
         # make a full copy of the cache, and invalidate it
         cache_dump = dict(
             (field, dict(field_cache))
@@ -1051,9 +1053,11 @@ class Environment(object):
             for record in records:
                 try:
                     cached = field_dump[record.id]
+                    cached = cached.get() if isinstance(cached, SpecialValue) else cached
+                    value = field.convert_to_record(cached, record)
                     fetched = record[field.name]
-                    if fetched != cached:
-                        info = {'cached': cached, 'fetched': fetched}
+                    if fetched != value:
+                        info = {'cached': value, 'fetched': fetched}
                         invalids.append((field, record, info))
                 except (AccessError, MissingError):
                     pass
diff --git a/openerp/fields.py b/openerp/fields.py
index 8f3d9cd10fc..9bb67944254 100644
--- a/openerp/fields.py
+++ b/openerp/fields.py
@@ -52,11 +52,12 @@ def copy_cache(records, env):
         if record not in done:
             done.add(record)
             target = record.with_env(env)
-            for name, value in record._cache.iteritems():
+            for name in record._cache:
+                field = record._fields[name]
+                value = record[name]
                 if isinstance(value, BaseModel):
                     todo.update(value)
-                    value = value.with_env(env)
-                target._cache[name] = value
+                target._cache[name] = field.convert_to_cache(value, target, validate=False)
 
 
 def resolve_mro(model, name, predicate):
@@ -88,12 +89,9 @@ def default_new_to_old(field, 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,
-        )
+        return lambda model: value(model._model, model._cr, model._uid, model._context)
     else:
-        return lambda model: field.convert_to_cache(value, model, validate=False)
+        return lambda model: value
 
 def default_old_to_old(field, value):
     """ Convert the old-API default ``value`` to the old API. """
@@ -860,7 +858,7 @@ class Field(object):
             record._cache[self] = value
             if env.in_onchange:
                 for invf in record._field_inverses[self]:
-                    invf._update(value, record)
+                    invf._update(record[self.name], record)
                 record._set_dirty(self.name)
 
             # determine more dependent fields, and invalidate them
@@ -1685,8 +1683,11 @@ class Many2one(_Relational):
     _column_auto_join = property(attrgetter('auto_join'))
 
     def _update(self, records, value):
-        """ Update the cached value of ``self`` for ``records`` with ``value``. """
-        records._cache[self] = value
+        """ Update the cached value of ``self`` for ``records`` with ``value``.
+        This is used to reflect the assignment ``value[name] = records``, where
+        ``name`` is the inverse field of ``self``.
+        """
+        records._cache[self] = self.convert_to_cache(value, records, validate=False)
 
     def convert_to_cache(self, value, record, validate=True):
         if isinstance(value, (NoneType, int, long)):
@@ -1741,8 +1742,9 @@ class UnionUpdate(SpecialValue):
         # in order to read the current field's value, remove self from cache
         del record._cache[field]
         # read the current field's value, and update it in cache only
-        record._cache[field] = new_value = record[field.name] | value
-        return new_value
+        value = field.convert_to_cache(record[field.name] | value, record, validate=False)
+        record._cache[field] = value
+        return value
 
 
 class _RelationalMulti(_Relational):
@@ -1752,9 +1754,10 @@ class _RelationalMulti(_Relational):
         """ Update the cached value of ``self`` for ``records`` with ``value``. """
         for record in records:
             if self in record._cache:
-                record._cache[self] = record[self.name] | value
+                value = self.convert_to_cache(record[self.name] | value, record, validate=False)
             else:
-                record._cache[self] = UnionUpdate(self, record, value)
+                value = UnionUpdate(self, record, value)
+            record._cache[self] = value
 
     def convert_to_cache(self, value, record, validate=True):
         if isinstance(value, BaseModel):
@@ -1805,7 +1808,7 @@ class _RelationalMulti(_Relational):
         result = [(5,)]
         for record in value:
             if not record.id:
-                values = dict(record._cache)
+                values = {name: record[name] for name in record._cache}
                 values = record._convert_to_write(values)
                 result.append((0, 0, values))
             elif record._is_dirty():
diff --git a/openerp/models.py b/openerp/models.py
index 721cce01edc..a37195693d9 100644
--- a/openerp/models.py
+++ b/openerp/models.py
@@ -5665,6 +5665,15 @@ class BaseModel(object):
             if name in fields
         }
 
+    def _convert_to_record(self, values):
+        """ Convert the ``values`` dictionary from the cache format to the
+        record format.
+        """
+        return {
+            name: self._fields[name].convert_to_record(value, self)
+            for name, value in values.iteritems()
+        }
+
     def _convert_to_write(self, values):
         """ Convert the ``values`` dictionary into the format of :meth:`write`. """
         fields = self._fields
@@ -6099,7 +6108,7 @@ class BaseModel(object):
                 return
             if res.get('value'):
                 res['value'].pop('id', None)
-                self.update(self._convert_to_cache(res['value'], validate=False))
+                self.update(res['value'])
             if res.get('domain'):
                 result.setdefault('domain', {}).update(res['domain'])
             if res.get('warning'):
@@ -6188,7 +6197,7 @@ class BaseModel(object):
         # create a new record with values, and attach ``self`` to it
         with env.do_in_onchange():
             record = self.new(values)
-            values = dict(record._cache)
+            values = {name: record[name] for name in record._cache}
             # attach ``self`` with a different context (for cache consistency)
             record._origin = self.with_context(__onchange=True)
 

From 84cf3ceb449a8adb4d6a182b19f48395b8db636c Mon Sep 17 00:00:00 2001
From: Raphael Collet <rco@openerp.com>
Date: Wed, 30 Mar 2016 11:13:55 +0200
Subject: [PATCH 08/11] [FIX] base_action_rule, mail, membership,
 point_of_sale: do not mix cache format with record format

---
 addons/base_action_rule/models/base_action_rule.py | 2 +-
 addons/mail/tests/test_mail_template.py            | 2 +-
 addons/membership/membership.py                    | 2 +-
 addons/point_of_sale/models/pos_order.py           | 4 ++--
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/addons/base_action_rule/models/base_action_rule.py b/addons/base_action_rule/models/base_action_rule.py
index 9d731493e64..8206224001e 100644
--- a/addons/base_action_rule/models/base_action_rule.py
+++ b/addons/base_action_rule/models/base_action_rule.py
@@ -287,7 +287,7 @@ class BaseActionRule(models.Model):
                     if res:
                         if 'value' in res:
                             res['value'].pop('id', None)
-                            self.update(self._convert_to_cache(res['value'], validate=False))
+                            self.update(res['value'])
                         if 'domain' in res:
                             result.setdefault('domain', {}).update(res['domain'])
                         if 'warning' in res:
diff --git a/addons/mail/tests/test_mail_template.py b/addons/mail/tests/test_mail_template.py
index afd93018cf1..485f81bb519 100644
--- a/addons/mail/tests/test_mail_template.py
+++ b/addons/mail/tests/test_mail_template.py
@@ -52,7 +52,7 @@ class TestMailTemplate(TestMail):
 
         values = composer.onchange_template_id(self.email_template.id, 'comment', 'mail.channel', self.group_pigs.id)['value']
         # use _convert_to_cache to return a browse record list from command list or id list for x2many fields
-        values = self.env['mail.compose.message']._convert_to_cache(values)
+        values = composer._convert_to_record(composer._convert_to_cache(values))
         recipients = values['partner_ids']
         attachments = values['attachment_ids']
 
diff --git a/addons/membership/membership.py b/addons/membership/membership.py
index 4155c7b5626..e3892cfe7fe 100644
--- a/addons/membership/membership.py
+++ b/addons/membership/membership.py
@@ -363,7 +363,7 @@ class Partner(osv.osv):
             # create a record in cache, apply onchange then revert back to a dictionnary
             invoice_line = invoice_line_obj.new(cr, uid, line_values, context=context)
             invoice_line._onchange_product_id()
-            line_values = invoice_line._convert_to_write(invoice_line._cache)
+            line_values = invoice_line._convert_to_write({name: invoice_line[name] for name in invoice_line._cache})
             line_values['price_unit'] = amount
             invoice_obj.write(cr, uid, [invoice_id], {'invoice_line_ids': [(0, 0, line_values)]}, context=context)
             invoice_list.append(invoice_id)
diff --git a/addons/point_of_sale/models/pos_order.py b/addons/point_of_sale/models/pos_order.py
index b318a09f625..04804ef8794 100644
--- a/addons/point_of_sale/models/pos_order.py
+++ b/addons/point_of_sale/models/pos_order.py
@@ -156,7 +156,7 @@ class PosOrder(models.Model):
         invoice_line.invoice_line_tax_ids = invoice_line.invoice_line_tax_ids.ids
         # We convert a new id object back to a dictionary to write to
         # bridge between old and new api
-        inv_line = invoice_line._convert_to_write(invoice_line._cache)
+        inv_line = invoice_line._convert_to_write({name: invoice_line[name] for name in invoice_line._cache})
         inv_line.update(price_unit=line.price_unit, discount=line.discount)
         return InvoiceLine.sudo().create(inv_line)
 
@@ -410,7 +410,7 @@ class PosOrder(models.Model):
             invoice._onchange_partner_id()
             invoice.fiscal_position_id = order.fiscal_position_id
 
-            inv = invoice._convert_to_write(invoice._cache)
+            inv = invoice._convert_to_write({name: invoice[name] for name in invoice._cache})
             new_invoice = Invoice.with_context(local_context).sudo().create(inv)
             order.write({'invoice_id': new_invoice.id, 'state': 'invoiced'})
             Invoice += new_invoice

From 318fc6f4b12472573c30c57dfe2cc22277ff9446 Mon Sep 17 00:00:00 2001
From: Raphael Collet <rco@openerp.com>
Date: Wed, 30 Mar 2016 15:56:41 +0200
Subject: [PATCH 09/11] [FIX] check values passed to `update`

---
 addons/base_action_rule/models/base_action_rule.py | 2 +-
 openerp/models.py                                  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/addons/base_action_rule/models/base_action_rule.py b/addons/base_action_rule/models/base_action_rule.py
index 8206224001e..50cbfced8fb 100644
--- a/addons/base_action_rule/models/base_action_rule.py
+++ b/addons/base_action_rule/models/base_action_rule.py
@@ -287,7 +287,7 @@ class BaseActionRule(models.Model):
                     if res:
                         if 'value' in res:
                             res['value'].pop('id', None)
-                            self.update(res['value'])
+                            self.update({key: val for key, val in res['value'].iteritems() if key in self._fields})
                         if 'domain' in res:
                             result.setdefault('domain', {}).update(res['domain'])
                         if 'warning' in res:
diff --git a/openerp/models.py b/openerp/models.py
index a37195693d9..3b3c093c0a7 100644
--- a/openerp/models.py
+++ b/openerp/models.py
@@ -6108,7 +6108,7 @@ class BaseModel(object):
                 return
             if res.get('value'):
                 res['value'].pop('id', None)
-                self.update(res['value'])
+                self.update({key: val for key, val in res['value'].iteritems() if key in self._fields})
             if res.get('domain'):
                 result.setdefault('domain', {}).update(res['domain'])
             if res.get('warning'):

From 7a8a8097f689360bda543f4e6cb00a410f163c09 Mon Sep 17 00:00:00 2001
From: Raphael Collet <rco@openerp.com>
Date: Mon, 11 Apr 2016 14:24:53 +0200
Subject: [PATCH 10/11] [IMP] fields: change the cache format of relational
 fields

Relational fields are now cached as tuples of ids instead of recordsets.
---
 openerp/addons/base/tests/test_api.py |  2 +-
 openerp/fields.py                     | 48 ++++++++++++++++++---------
 openerp/models.py                     |  4 +--
 3 files changed, 35 insertions(+), 19 deletions(-)

diff --git a/openerp/addons/base/tests/test_api.py b/openerp/addons/base/tests/test_api.py
index 6f623b3f3f4..645c5d4ad9b 100644
--- a/openerp/addons/base/tests/test_api.py
+++ b/openerp/addons/base/tests/test_api.py
@@ -339,7 +339,7 @@ class TestAPI(common.TransactionCase):
         self.assertItemsEqual(partners.ids, country_id_cache)
 
         # partners' countries are ready for prefetching
-        country_ids = set(c.id for c in country_id_cache.itervalues() if c)
+        country_ids = set(cid for cids in country_id_cache.itervalues() for cid in cids)
         self.assertTrue(len(country_ids) > 1)
         self.assertItemsEqual(country_ids, partners._prefetch['res.country'])
 
diff --git a/openerp/fields.py b/openerp/fields.py
index 9bb67944254..bafd4081314 100644
--- a/openerp/fields.py
+++ b/openerp/fields.py
@@ -1571,16 +1571,24 @@ class Reference(Selection):
             "Reference field %s with non-integer size %r" % (self, self.size)
 
     def convert_to_cache(self, value, record, validate=True):
+        # cache format: (res_model, res_id) or False
+        def process(res_model, res_id):
+            record._prefetch[res_model].add(res_id)
+            return (res_model, res_id)
+
         if isinstance(value, BaseModel):
             if not validate or (value._name in self.get_values(record.env) and len(value) <= 1):
-                return value.with_env(record.env).with_prefetch(record._prefetch) or False
+                return process(value._name, value.id) if value else False
         elif isinstance(value, basestring):
             res_model, res_id = value.split(',')
-            return record.env[res_model].browse(int(res_id), record._prefetch)
+            return process(res_model, int(res_id))
         elif not value:
             return False
         raise ValueError("Wrong value for %s: %r" % (self, value))
 
+    def convert_to_record(self, value, record):
+        return value and record.env[value[0]].browse([value[1]], record._prefetch)
+
     def convert_to_read(self, value, record, use_name_get=True):
         return "%s,%s" % (value._name, value.id) if value else False
 
@@ -1690,21 +1698,25 @@ class Many2one(_Relational):
         records._cache[self] = self.convert_to_cache(value, records, validate=False)
 
     def convert_to_cache(self, value, record, validate=True):
-        if isinstance(value, (NoneType, int, long)):
-            return record.env[self.comodel_name].browse(value, record._prefetch)
-        if isinstance(value, BaseModel):
+        # cache format: tuple(ids)
+        def process(ids):
+            return record._prefetch[self.comodel_name].update(ids) or ids
+
+        if type(value) in IdType:
+            return process((value,))
+        elif isinstance(value, BaseModel):
             if not validate or (value._name == self.comodel_name and len(value) <= 1):
-                return value.with_env(record.env).with_prefetch(record._prefetch)
+                return process(value._ids)
             raise ValueError("Wrong value for %s: %r" % (self, value))
         elif isinstance(value, tuple):
-            return record.env[self.comodel_name].browse(value[0], record._prefetch)
+            return process((value[0],))
         elif isinstance(value, dict):
-            return record.env[self.comodel_name].new(value)
+            return process(record.env[self.comodel_name].new(value)._ids)
         else:
-            return record.env[self.comodel_name]
+            return ()
 
     def convert_to_record(self, value, record):
-        return value.with_prefetch(record._prefetch)
+        return record.env[self.comodel_name]._browse(value, record.env, record._prefetch)
 
     def convert_to_read(self, value, record, use_name_get=True):
         if use_name_get and value:
@@ -1760,9 +1772,13 @@ class _RelationalMulti(_Relational):
             record._cache[self] = value
 
     def convert_to_cache(self, value, record, validate=True):
+        # cache format: tuple(ids)
+        def process(ids):
+            return record._prefetch[self.comodel_name].update(ids) or ids
+
         if isinstance(value, BaseModel):
             if not validate or (value._name == self.comodel_name):
-                return value.with_env(record.env).with_prefetch(record._prefetch)
+                return process(value._ids)
         elif isinstance(value, list):
             # value is a list of record ids or commands
             comodel = record.env[self.comodel_name]
@@ -1791,14 +1807,14 @@ class _RelationalMulti(_Relational):
                     ids.add(comodel.new(command).id)
                 else:
                     ids.add(command)
-            # return result as a recordset
-            return comodel.browse(list(ids), record._prefetch)
+            # return result as a tuple
+            return process(tuple(ids))
         elif not value:
-            return record.env[self.comodel_name]
+            return ()
         raise ValueError("Wrong value for %s: %s" % (self, value))
 
     def convert_to_record(self, value, record):
-        return value.with_prefetch(record._prefetch)
+        return record.env[self.comodel_name]._browse(value, record.env, record._prefetch)
 
     def convert_to_read(self, value, record, use_name_get=True):
         return value.ids
@@ -2030,5 +2046,5 @@ class Id(Field):
 # imported here to avoid dependency cycle issues
 from openerp import SUPERUSER_ID
 from .exceptions import AccessError, MissingError
-from .models import check_pg_name, BaseModel
+from .models import check_pg_name, BaseModel, IdType
 from .osv import fields
diff --git a/openerp/models.py b/openerp/models.py
index 3b3c093c0a7..35356269004 100644
--- a/openerp/models.py
+++ b/openerp/models.py
@@ -269,7 +269,7 @@ class NewId(object):
     def __nonzero__(self):
         return False
 
-IdType = (int, long, basestring, NewId)
+IdType = (int, long, str, unicode, NewId)
 
 
 # maximum number of prefetched records
@@ -6422,7 +6422,7 @@ PGERROR_TO_OE = defaultdict(
     '23505': convert_pgerror_23505,
 })
 
-def _normalize_ids(arg, atoms={int, long, str, unicode, NewId}):
+def _normalize_ids(arg, atoms=set(IdType)):
     """ Normalizes the ids argument for ``browse`` (v7 and v8) to a tuple.
 
     Various implementations were tested on the corpus of all browse() calls

From b684f0b521cbb0ed220209bd76c21cd2771bb308 Mon Sep 17 00:00:00 2001
From: Raphael Collet <rco@openerp.com>
Date: Thu, 14 Apr 2016 15:40:05 +0200
Subject: [PATCH 11/11] [IMP] doc: add some basic description of record
 prefetching

---
 doc/reference/orm.rst | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/doc/reference/orm.rst b/doc/reference/orm.rst
index 10ac368e950..9d232b417da 100644
--- a/doc/reference/orm.rst
+++ b/doc/reference/orm.rst
@@ -79,6 +79,45 @@ Accessing a relational field (:class:`~openerp.fields.Many2one`,
         # 1 database update
         records.write({'a': 1, 'b': 2, 'c': 3})
 
+Record cache and prefetching
+----------------------------
+
+Odoo maintains a cache for the fields of the records, so that not every field
+access issues a database request, which would be terrible for performance. The
+following example queries the database only for the first statement::
+
+    record.name             # first access reads value from database
+    record.name             # second access gets value from cache
+
+To avoid reading one field on one record at a time, Odoo *prefetches* records
+and fields following some heuristics to get good performance. Once a field must
+be read on a given record, the ORM actually reads that field on a larger
+recordset, and stores the returned values in cache for later use. The prefetched
+recordset is usually the recordset from which the record comes by iteration.
+Moreover, all simple stored fields (boolean, integer, float, char, text, date,
+datetime, selection, many2one) are fetched altogether; they correspond to the
+columns of the model's table, and are fetched efficiently in the same query.
+
+Consider the following example, where ``partners`` is a recordset of 1000
+records. Without prefetching, the loop would make 2000 queries to the database.
+With prefetching, only one query is made::
+
+    for partner in partners:
+        print partner.name          # first pass prefetches 'name' and 'lang'
+                                    # (and other fields) on all 'partners'
+        print partner.lang
+
+The prefetching also works on *secondary records*: when relational fields are
+read, their values (which are records) are  subscribed for future prefetching.
+Accessing one of those secondary records prefetches all secondary records from
+the same model. This makes the following example generate only two queries, one
+for partners and one for countries::
+
+    countries = set()
+    for partner in partners:
+        country = partner.country_id        # first pass prefetches all partners
+        countries.add(country.name)         # first pass prefetches all countries
+
 Set operations
 --------------