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

2 files changed, 44 insertions, 18 deletions

diff --git a/odoo/addons/test_new_api/models/test_new_api.py b/odoo/addons/test_new_api/models/test_new_api.py
index 1f36dfe7bad..b02ef005015 100644
--- a/odoo/addons/test_new_api/models/test_new_api.py
+++ b/odoo/addons/test_new_api/models/test_new_api.py
@@ -663,19 +663,27 @@ class ComputeUnassigned(models.Model):
 
     @api.depends('foo')
     def _compute_bar(self):
-        pass
+        for record in self:
+            if record.foo == "assign":
+                record.bar = record.foo
 
     @api.depends('foo')
     def _compute_bare(self):
-        pass
+        for record in self:
+            if record.foo == "assign":
+                record.bare = record.foo
 
     @api.depends('foo')
     def _compute_bars(self):
-        pass
+        for record in self:
+            if record.foo == "assign":
+                record.bars = record.foo
 
     @api.depends('foo')
     def _compute_bares(self):
-        pass
+        for record in self:
+            if record.foo == "assign":
+                record.bares = record.foo
 
 
 class ModelBinary(models.Model):
diff --git a/odoo/addons/test_new_api/tests/test_new_fields.py b/odoo/addons/test_new_api/tests/test_new_fields.py
index 2191bee7096..f77bcc261b0 100644
--- a/odoo/addons/test_new_api/tests/test_new_fields.py
+++ b/odoo/addons/test_new_api/tests/test_new_fields.py
@@ -703,27 +703,45 @@ class TestFields(TransactionCaseWithUserDemo):
         self.assertEqual(record.bares, False)
 
     def test_16_compute_unassigned_access_error(self):
-        # create a real record
-        record = self.env['test_new_api.compute.unassigned'].create({})
-        record.flush()
+        # create two records
+        records = self.env['test_new_api.compute.unassigned'].create([{}, {}])
+        records.flush()
 
-        # alter access rights: regular users cannot read 'record'
+        # alter access rights: regular users cannot read 'records'
         access = self.env.ref('test_new_api.access_test_new_api_compute_unassigned')
         access.perm_read = False
+        access.flush()
 
         # switch to environment with user demo
-        record = record.with_user(self.user_demo)
-        record.env.cache.invalidate()
+        records = records.with_user(self.user_demo)
+        records.env.cache.invalidate()
 
-        # check that the record is not accessible
+        # check that records are not accessible
         with self.assertRaises(AccessError):
-            record.bars
-
-        # modify the record and flush() changes with the current environment:
-        # this should not trigger an access error, even if unassigned computed
-        # fields are fetched from database
-        record.foo = "X"
-        record.flush()
+            records[0].bars
+        with self.assertRaises(AccessError):
+            records[1].bars
+
+        # Modify the records and flush() changes with the current environment:
+        # this should not trigger an access error, whatever the order in which
+        # records are considered.  It may fail in the following scenario:
+        #  - mark field 'bars' to compute on records
+        #  - access records[0].bars
+        #     - recompute bars on records (both) -> assign records[0] only
+        #     - return records[0].bars from cache
+        #  - access records[1].bars
+        #     - recompute nothing (done already)
+        #     - records[1].bars is not in cache
+        #     - fetch records[1].bars -> access error
+        records[0].foo = "assign"
+        records[1].foo = "x"
+        records.flush()
+
+        # try the other way around, too
+        records.env.cache.invalidate()
+        records[0].foo = "x"
+        records[1].foo = "assign"
+        records.flush()
 
     def test_20_float(self):
         """ test rounding of float fields """