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 b3dfd095f2a..7d292000479 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 939bbd8621f..14795e7c868 100644 --- a/odoo/addons/test_new_api/tests/test_new_fields.py +++ b/odoo/addons/test_new_api/tests/test_new_fields.py @@ -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 """ diff --git a/odoo/fields.py b/odoo/fields.py index a515fd7ea5c..7892ebf8dfe 100644 --- a/odoo/fields.py +++ b/odoo/fields.py @@ -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): diff --git a/odoo/models.py b/odoo/models.py index 8c6e9a65a50..8ee8b03610f 100644 --- a/odoo/models.py +++ b/odoo/models.py @@ -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...