From f012e337698c89e4c35b3a75243b26e3823e757e Mon Sep 17 00:00:00 2001 From: Raphael Collet Date: Mon, 22 Aug 2022 13:11:44 +0200 Subject: [PATCH] [IMP] core: warn about compute methods mixing stored and non-stored fields We explicitly discourage a compute method used for both stored and non-stored fields, because it may be unexpectedly update the database. Indeed, we don't expect the computation of a non-stored field to update the database. Allowing this kind of compute method makes reasoning about readonly code much more difficult, and would prevent readonly transactions with such fields. For instance, consider a compute method for both fields F (non-stored) and G (stored), and assume that none of those fields are in cache. Whenever F is accessed, the compute method is invoked, and both fields are assigned, which potentially generates an SQL update for field G. This is problematic if we require the current transaction to be readonly. Part-of: odoo/odoo#98565 --- .../test_new_api/tests/test_new_fields.py | 2 +- odoo/modules/registry.py | 26 ++++++++++++++++--- 2 files changed, 23 insertions(+), 5 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 fd4207467a8..805d94dd21f 100644 --- a/odoo/addons/test_new_api/tests/test_new_fields.py +++ b/odoo/addons/test_new_api/tests/test_new_fields.py @@ -4468,7 +4468,7 @@ class TestPrecomputeModel(common.TransactionCase): # see what happens if not both are precompute self.addCleanup(self.registry.reset_changes) self.patch(Model.upper, 'precompute', False) - with self.assertLogs('odoo.modules.registry', level='WARNING'): + with self.assertWarns(UserWarning): self.registry.setup_models(self.cr) self.registry.field_computed diff --git a/odoo/modules/registry.py b/odoo/modules/registry.py index 0c0c700cc65..40bcf2942bc 100644 --- a/odoo/modules/registry.py +++ b/odoo/modules/registry.py @@ -345,12 +345,30 @@ class Registry(Mapping): computed[field] = group = groups[field.compute] group.append(field) for fields in groups.values(): + if len(fields) < 2: + continue if len({field.compute_sudo for field in fields}) > 1: - _logger.warning("%s: inconsistent 'compute_sudo' for computed fields: %s", - model_name, ", ".join(field.name for field in fields)) + fnames = ", ".join(field.name for field in fields) + warnings.warn( + f"{model_name}: inconsistent 'compute_sudo' for computed fields {fnames}. " + f"Either set 'compute_sudo' to the same value on all those fields, or " + f"use distinct compute methods for sudoed and non-sudoed fields." + ) if len({field.precompute for field in fields}) > 1: - _logger.warning("%s: inconsistent 'precompute' for computed fields: %s", - model_name, ", ".join(field.name for field in fields)) + fnames = ", ".join(field.name for field in fields) + warnings.warn( + f"{model_name}: inconsistent 'precompute' for computed fields {fnames}. " + f"Either set all fields as precompute=True (if possible), or " + f"use distinct compute methods for precomputed and non-precomputed fields." + ) + if len({field.store for field in fields}) > 1: + fnames1 = ", ".join(field.name for field in fields if not field.store) + fnames2 = ", ".join(field.name for field in fields if field.store) + warnings.warn( + f"{model_name}: inconsistent 'store' for computed fields, " + f"accessing {fnames1} may recompute and update {fnames2}. " + f"Use distinct compute methods for stored and non-stored fields." + ) return computed def get_trigger_tree(self, fields: list, select=bool) -> "TriggerTree":