[FIX] core: avoid access error when flushing

This takes a former patch one step further.

Assume you recompute a field on two records, and the computation assigns
the first record but not the second one.  Before this patch, the
recomputation process was accessing the field on both records.  When
accessing the first record, both records are recomputed, and the value
of the first one is found in cache.  When accessing the second record,
no recomputation is made (it has been done already), and the cache is
empty. The field is thus fetched from database, and this crashes because
of insufficient access rights.

This scenario was causing some obscure nondeterministic crashes in
tests. The nondeterminism comes from the choice of the environment for
flushing (it should not be in superuser mode); the order of records in
the set to compute; the order in which records are assigned in the
compute method; the fact that the compute method should assign some
records and some not.

The fix consists in adding a method that processes pending computations,
without doing anything special for unassigned records.  The method is
used in field accessors (__get__ and mapped) and in method recompute().
The access error is gone because the recomputation no longer tries to
get the value of a field to compute.

closes odoo/odoo#64250

Signed-off-by: Raphael Collet (rco) <rco@openerp.com>

1 files changed, 33 insertions, 28 deletions

diff --git a/odoo/fields.py b/odoo/fields.py
index 4514c62baab..710663866d0 100644
--- a/odoo/fields.py
+++ b/odoo/fields.py
@@ -929,20 +929,9 @@ class Field(MetaField('DummyField', (object,), {})):
         # only a single record may be accessed
         record.ensure_one()
 
-        recomputed = False
-        if self.compute and (record.id in env.all.tocompute.get(self, ())) \
-                and not env.is_protected(self, record):
-            # self must be computed on record
-            if self.recursive:
-                recs = record
-            else:
-                ids = expand_ids(record.id, env.all.tocompute[self])
-                recs = record.browse(itertools.islice(ids, PREFETCH_MAX))
-            try:
-                self.compute_value(recs)
-            except (AccessError, MissingError):
-                self.compute_value(record)
-            recomputed = True
+        if self.compute and self.store:
+            # process pending computations
+            self.recompute(record)
 
         try:
             value = env.cache.get(record, self)
@@ -970,8 +959,6 @@ class Field(MetaField('DummyField', (object,), {})):
             if self.store and record.id:
                 # real record: fetch from database
                 recs = record._in_cache_without(self)
-                if recomputed and self.compute_sudo:
-                    recs = recs.sudo()
                 try:
                     recs._fetch_field(self)
                 except AccessError:
@@ -1050,16 +1037,9 @@ class Field(MetaField('DummyField', (object,), {})):
             # not stored in cache
             return list(records._ids)
 
-        if self.compute:
-            # Force the computation of the subset of 'records' to compute. This
-            # is necessary because the values in cache are not valid for the
-            # records to compute. Note that this explicitly prevents an infinite
-            # loop upon recompute, which invokes mapped() on the records, which
-            # fetches record values, which calls flush, which calls recompute.
-            to_compute_ids = records.env.all.tocompute.get(self, ())
-            ids = [id_ for id_ in records._ids if id_ in to_compute_ids]
-            for record in records.browse(ids):
-                self.__get__(record, type(records))
+        if self.compute and self.store:
+            # process pending computations
+            self.recompute(records)
 
         # retrieve values in cache, and fetch missing ones
         vals = records.env.cache.get_until_miss(records, self)
@@ -1118,6 +1098,29 @@ class Field(MetaField('DummyField', (object,), {})):
     # Computation of field values
     #
 
+    def recompute(self, records):
+        """ Process the pending computations of ``self`` on ``records``. This
+        should be called only if ``self`` is computed and stored.
+        """
+        to_compute_ids = records.env.all.tocompute.get(self)
+        if not to_compute_ids:
+            return
+
+        if self.recursive:
+            for record in records:
+                if record.id in to_compute_ids:
+                    self.compute_value(record)
+            return
+
+        for record in records:
+            if record.id in to_compute_ids:
+                ids = expand_ids(record.id, to_compute_ids)
+                recs = record.browse(itertools.islice(ids, PREFETCH_MAX))
+                try:
+                    self.compute_value(recs)
+                except (AccessError, MissingError):
+                    self.compute_value(record)
+
     def compute_value(self, records):
         """ Invoke the compute method on ``records``; the results are in cache. """
         env = records.env
@@ -1131,14 +1134,16 @@ class Field(MetaField('DummyField', (object,), {})):
         # with _read(), which will flush() it. If the field is still to compute,
         # the latter flush() will recursively compute this field!
         for field in fields:
-            env.remove_to_compute(field, records)
+            if field.store:
+                env.remove_to_compute(field, records)
 
         try:
             with records.env.protecting(fields, records):
                 records._compute_field_value(self)
         except Exception:
             for field in fields:
-                env.add_to_compute(field, records)
+                if field.store:
+                    env.add_to_compute(field, records)
             raise
 
     def determine_inverse(self, records):