From 6d5da2d2d3f0630b507a37f0b282cfa4f3f86bfd Mon Sep 17 00:00:00 2001 From: Raphael Collet Date: Tue, 2 Jun 2020 14:24:01 +0000 Subject: [PATCH] [FIX] core: prefetching of context-dependent fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Consider a context-dependent field, and successively access it on a recordset with different contexts. On the first context, the field is correctly computed in batch. After that, the field is always computed one by one. The bug is in the method that determines which records in a given set have no value in cache. On the first context, the cache is empty for the field, so all records are returned. After that, the method considers that all records have a value in cache: they do, but for another context key! Simply using the context key when looking up the cache fixes the issue. closes odoo/odoo#52360 X-original-commit: 35d69589d9b43afe6c6fc9779458323f7180153e Signed-off-by: Xavier Dollé (xdo) Co-authored-by: Xavier-Do --- odoo/addons/test_performance/models/models.py | 7 +++++++ .../tests/test_performance.py | 18 ++++++++++++++++++ odoo/api.py | 19 +++++++++++++++---- 3 files changed, 40 insertions(+), 4 deletions(-) diff --git a/odoo/addons/test_performance/models/models.py b/odoo/addons/test_performance/models/models.py index aecca064ee9..95c0ef9bb54 100644 --- a/odoo/addons/test_performance/models/models.py +++ b/odoo/addons/test_performance/models/models.py @@ -11,6 +11,7 @@ class BaseModel(models.Model): name = fields.Char() value = fields.Integer(default=0) value_pc = fields.Float(compute="_value_pc", store=True) + value_ctx = fields.Float(compute="_value_ctx") partner_id = fields.Many2one('res.partner', string='Customer') line_ids = fields.One2many('test_performance.line', 'base_id') @@ -22,6 +23,12 @@ class BaseModel(models.Model): for record in self: record.value_pc = float(record.value) / 100 + @api.depends_context('key') + def _value_ctx(self): + self.env.cr.execute('SELECT 42') # one dummy query per batch + for record in self: + record.value_ctx = self.env.context.get('key') + @api.depends('line_ids.value') def _total(self): for record in self: diff --git a/odoo/addons/test_performance/tests/test_performance.py b/odoo/addons/test_performance/tests/test_performance.py index 01bab7187e2..7ace0d3ea96 100644 --- a/odoo/addons/test_performance/tests/test_performance.py +++ b/odoo/addons/test_performance/tests/test_performance.py @@ -65,6 +65,24 @@ class TestPerformance(SavepointCaseWithUserDemo): for record in records: record.value_pc + @warmup + def test_read_base_depends_context(self): + """ Compute in batch even when in cache in another context. """ + records = self.env['test_performance.base'].search([]) + self.assertEqual(len(records), 5) + + with self.assertQueryCount(1): + for record in records.with_context(key=1): + self.assertEqual(record.value_ctx, 1) + + with self.assertQueryCount(1): + for record in records.with_context(key=2): + self.assertEqual(record.value_ctx, 2) + + with self.assertQueryCount(1): + for record in records: + self.assertEqual(record.with_context(key=3).value_ctx, 3) + @users('__system__', 'demo') @warmup def test_write_base(self): diff --git a/odoo/api.py b/odoo/api.py index 9daf85f0beb..9dd5873d556 100644 --- a/odoo/api.py +++ b/odoo/api.py @@ -878,15 +878,26 @@ class Cache(object): def get_records(self, model, field): """ Return the records of ``model`` that have a value for ``field``. """ - ids = list(self._data[field]) + field_cache = self._data[field] + if field.depends_context: + key = model.env.cache_key(field) + ids = [id_ for id_, value in field_cache.items() if key in value] + else: + ids = list(field_cache) return model.browse(ids) def get_missing_ids(self, records, field): """ Return the ids of ``records`` that have no value for ``field``. """ field_cache = self._data[field] - for record_id in records._ids: - if record_id not in field_cache: - yield record_id + if field.depends_context: + key = records.env.cache_key(field) + for record_id in records._ids: + if key not in field_cache.get(record_id, ()): + yield record_id + else: + for record_id in records._ids: + if record_id not in field_cache: + yield record_id def invalidate(self, spec=None): """ Invalidate the cache, partially or totally depending on ``spec``. """