pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[FIX] core: avoid access error when flushing

Browse Source 
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#64264

X-original-commit: b7676ec33172e6196b5fd1eda4df730ebf90eb6b
Signed-off-by: Raphael Collet (rco) <rco@openerp.com>
This commit is contained in:
Raphael Collet
2021-01-08 13:06:25 +00:00
parent ad59eff40a
commit 1e7659ad48
4 changed files with 78 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files
odoo/addons/test_new_api/models/test_new_api.py
+12 -4
View File
@@ -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):
odoo/addons/test_new_api/tests/test_new_fields.py
+31 -13
View File
@@ -696,27 +696,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
records[0].bars
with self.assertRaises(AccessError):
records[1].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()
# 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 """
odoo/fields.py
+33 -28
View File
@@ -936,20 +936,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)
@@ -977,8 +966,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:
@@ -1058,16 +1045,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)
@@ -1126,6 +1106,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
@@ -1139,14 +1142,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):
odoo/models.py
+2 -2
View File
@@ -5872,10 +5872,10 @@ Fields:
# recomputed by accessing the field on the records
recs = recs.filtered('id')
try:
recs.mapped(field.name)
field.recompute(recs)
except MissingError:
existing = recs.exists()
existing.mapped(field.name)
field.recompute(existing)
# mark the field as computed on missing records, otherwise
# they remain forever in the todo list, and lead to an
# infinite loop...