From c6c79473dd60db708f0f0a6ccdbd1298a60d4b00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Voet=20=28ryv=29?= Date: Tue, 13 Dec 2022 15:58:58 +0000 Subject: [PATCH] [FIX] core: unexpected MissingError when prefetching record MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is a rare case where method _read() raises a MissingError instead of just ignoring it. The issue is triggered by several conditions on a model M: - at least one ir.rule on M with a domain using a column field on M; - one deleted record Y which is in the prefetch set of a record X; - one reads a non-column field on record X. Fix method _read() to manage that case. It adds an extra call to exists() in that case, but adds no overhead in the general case. closes odoo/odoo#108050 X-original-commit: 1876dc87e5c88c711c9b3882c39be704ffc46532 Signed-off-by: Raphael Collet Signed-off-by: Rémy Voet --- .../test_new_api/tests/test_new_fields.py | 50 ++++++++++++++++++- odoo/fields.py | 2 +- odoo/models.py | 13 ++++- 3 files changed, 61 insertions(+), 4 deletions(-) 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 5fa808f61b9..027209ed0b9 100644 --- a/odoo/addons/test_new_api/tests/test_new_fields.py +++ b/odoo/addons/test_new_api/tests/test_new_fields.py @@ -13,7 +13,7 @@ import psycopg2 from odoo import models, fields, Command from odoo.addons.base.tests.common import TransactionCaseWithUserDemo -from odoo.exceptions import AccessError, UserError, ValidationError +from odoo.exceptions import AccessError, MissingError, UserError, ValidationError from odoo.tests import common from odoo.tools import mute_logger, float_repr from odoo.tools.date_utils import add, subtract, start_of, end_of @@ -1624,6 +1624,54 @@ class TestFields(TransactionCaseWithUserDemo): with self.assertRaises(AccessError): cat1.name + def test_32_prefetch_missing_error(self): + """ Test that prefetching non-column fields works in the presence of deleted records. """ + Discussion = self.env['test_new_api.discussion'] + + # add an ir.rule that forces reading field 'name' + self.env['ir.rule'].create({ + 'model_id': self.env['ir.model']._get(Discussion._name).id, + 'groups': [self.env.ref('base.group_user').id], + 'domain_force': "[('name', '!=', 'Super Secret discution')]", + }) + + records = Discussion.with_user(self.user_demo).create([ + {'name': 'EXISTING'}, + {'name': 'MISSING'}, + ]) + + # unpack to keep the prefetch on each recordset + existing, deleted = records + self.assertEqual(existing._prefetch_ids, records._ids) + + # this invalidates the caches but the prefetching remains the same + deleted.unlink() + + # this should not trigger a MissingError + existing.categories + + # invalidate 'categories' for the assertQueryCount + records.invalidate_model(['categories']) + with self.assertQueryCount(4): + # .__get__(existing) + # -> records._fetch_field(['categories']) + # -> records._read(['categories']) + # -> records.check_access_rule('read') + # -> records._filter_access_rules_python('read') + # -> records.filtered_domain(...) + # -> .__get__(existing) + # -> records._fetch_field(['name']) + # -> records._read(['name', ...]) + # -> ONE QUERY to read ['name', ...] of records + # -> ONE QUERY for deleted.exists() / code: forbidden = missing.exists() + # -> ONE QUERY for records.exists() / code: self = self.exists() + # -> ONE QUERY to read the many2many of existing + existing.categories + + # this one must trigger a MissingError + with self.assertRaises(MissingError): + deleted.categories + def test_40_real_vs_new(self): """ test field access on new records vs real records. """ Model = self.env['test_new_api.category'] diff --git a/odoo/fields.py b/odoo/fields.py index 39dbd549b6e..cf522bdae31 100644 --- a/odoo/fields.py +++ b/odoo/fields.py @@ -1167,7 +1167,7 @@ class Field(MetaField('DummyField', (object,), {})): recs._fetch_field(self) except AccessError: record._fetch_field(self) - if not env.cache.contains(record, self) and not record.exists(): + if not env.cache.contains(record, self): raise MissingError("\n".join([ _("Record does not exist or has been deleted."), _("(Record: %s, User: %s)") % (record, env.uid), diff --git a/odoo/models.py b/odoo/models.py index f2777a98eac..691e3e86ad4 100644 --- a/odoo/models.py +++ b/odoo/models.py @@ -3221,8 +3221,17 @@ class BaseModel(metaclass=MetaModel): cr.execute(query_str, params + [sub_ids]) result += cr.fetchall() else: - self.check_access_rule('read') - result = [(id_,) for id_ in self.ids] + try: + self.check_access_rule('read') + except MissingError: + # Method _read() should never raise a MissingError, but method + # check_access_rule() can, because it must read fields on self. + # So we restrict 'self' to existing records (to avoid an extra + # exists() at the end of the method). + self = self.exists() + self.check_access_rule('read') + + result = [(id_,) for id_ in self._ids] fetched = self.browse() if result: