diff --git a/addons/sale_timesheet/tests/test_project_billing.py b/addons/sale_timesheet/tests/test_project_billing.py index d3c210172a0..7765541c674 100644 --- a/addons/sale_timesheet/tests/test_project_billing.py +++ b/addons/sale_timesheet/tests/test_project_billing.py @@ -295,6 +295,9 @@ class TestProjectBilling(TestCommonSaleTimesheet): self.assertTrue(task.allow_billable, "Task in project 'task rate' should be billed at task rate") self.assertEqual(task.sale_line_id, self.so1_line_deliver_no_task, "The task should keep the same SOL since the partner_id has not changed when the project of the task has changed.") self.assertEqual(task.partner_id, self.partner_a, "Task created in a project billed on 'employee rate' should have the same customer when it has been created.") + # the `subtask.sale_line_id` is consider to be recompute, + # but the result differ after the write of display_project_id without depend on it + task.flush_model(["sale_line_id"]) # move subtask into task rate project subtask.write({ @@ -379,6 +382,9 @@ class TestProjectBilling(TestCommonSaleTimesheet): }) self.assertEqual(subtask.display_project_id, timesheet2.project_id, "The timesheet is in the subtask project") self.assertFalse(timesheet2.so_line, "The timesheet should not be linked to SOL as it's a non billable project") + # the `subtask.sale_line_id` is consider to be recompute, + # but the result differ after the write of project_id + task.flush_model(["sale_line_id"]) # move task and subtask into task rate project task.write({ 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 7e64d8c0d99..3eb16dabbbb 100644 --- a/odoo/addons/test_new_api/tests/test_new_fields.py +++ b/odoo/addons/test_new_api/tests/test_new_fields.py @@ -4117,50 +4117,22 @@ class TestModifiedPerformance(common.TransactionCase): cls.env.invalidate_all() # Clean the cache def test_modified_trigger_related(self): - with self.assertQueries([""" - SELECT "test_new_api_modified_line".id - FROM "test_new_api_modified_line" - WHERE ("test_new_api_modified_line"."modified_id" IN %s) - ORDER BY "test_new_api_modified_line"."id" - """, - """ - SELECT "test_new_api_modified_line"."id" AS "id", "test_new_api_modified_line"."modified_id" AS "modified_id" - FROM "test_new_api_modified_line" - WHERE "test_new_api_modified_line".id IN %s - """], flush=False): - # Two queries to fetch the `line_ids` in order to invalidate the cache for `modified_name` - # (even if the cache is empty for this field) + with self.assertQueryCount(0, flush=False): + # No queries because `modified_name` has a empty cache self.modified_a.name = "Other" self.assertEqual(self.modified_line_a.modified_name, 'Other') # check def test_modified_trigger_no_store_compute(self): - with self.assertQueries([""" - SELECT "test_new_api_modified_line"."id" AS "id", "test_new_api_modified_line"."modified_id" AS "modified_id", - "test_new_api_modified_line"."quantity" AS "quantity", "test_new_api_modified_line"."price" AS "price", - "test_new_api_modified_line"."parent_id" AS "parent_id", "test_new_api_modified_line"."create_uid" AS "create_uid", - "test_new_api_modified_line"."create_date" AS "create_date", "test_new_api_modified_line"."write_uid" AS "write_uid", - "test_new_api_modified_line"."write_date" AS "write_date" - FROM "test_new_api_modified_line" - WHERE "test_new_api_modified_line".id IN %s - """], flush=False): - # One query to fetch `modified_id` in order to invalidate `total_quantity` + with self.assertQueryCount(0, flush=False): + # No queries because `total_quantity` has a empty cache self.modified_line_a.quantity = 8 self.assertEqual(self.modified_a.total_quantity, 18) def test_modified_trigger_recursive_empty_cache(self): - with self.assertQueries([""" - SELECT "test_new_api_modified_line"."id" AS "id", "test_new_api_modified_line"."modified_id" AS "modified_id", - "test_new_api_modified_line"."quantity" AS "quantity", "test_new_api_modified_line"."price" AS "price", - "test_new_api_modified_line"."parent_id" AS "parent_id", "test_new_api_modified_line"."create_uid" AS "create_uid", - "test_new_api_modified_line"."create_date" AS "create_date", "test_new_api_modified_line"."write_uid" AS "write_uid", - "test_new_api_modified_line"."write_date" AS "write_date" - FROM "test_new_api_modified_line" - WHERE "test_new_api_modified_line".id IN %s - """], flush=False): - # One query to fetch `parent_id` in order to invalidate `total_price` recursively - # but because no record has already recompute `total_price` no need to invalidate recursivly + with self.assertQueryCount(0, flush=False): + # No queries because `total_price` has a empty cache self.modified_line_a_child_child.price = 4 self.assertEqual(self.modified_line_a.total_price, 7) diff --git a/odoo/addons/test_new_api/tests/test_properties.py b/odoo/addons/test_new_api/tests/test_properties.py index ab552b9e2a5..2486e6d8ca1 100644 --- a/odoo/addons/test_new_api/tests/test_properties.py +++ b/odoo/addons/test_new_api/tests/test_properties.py @@ -905,12 +905,8 @@ class PropertiesCase(TransactionCase): # so the value in cache is not verified (see models.py@_read) self.message_1.attributes - expected = [ - 'SELECT "test_new_api_emailmessage".id FROM "test_new_api_emailmessage" WHERE ("test_new_api_emailmessage"."message" in %s) ORDER BY "test_new_api_emailmessage"."id"', - 'UPDATE "test_new_api_message" SET "attributes" = %s, "write_date" = %s, "write_uid" = %s WHERE id IN %s', - ] - - with self.assertQueryCount(2), self.assertQueries(expected): + expected = ['UPDATE "test_new_api_message" SET "attributes" = %s, "write_date" = %s, "write_uid" = %s WHERE id IN %s'] + with self.assertQueryCount(1), self.assertQueries(expected): self.message_1.attributes = [ { "name": "discussion_color_code", diff --git a/odoo/api.py b/odoo/api.py index 854d597bb26..1b2203ab72c 100644 --- a/odoo/api.py +++ b/odoo/api.py @@ -948,6 +948,16 @@ class Cache(object): """ Return whether ``record`` has a value for ``field``. """ return record.id in self._get_field_cache(record, field) + def contains_field(self, field): + """ Return whether ``field`` has a value for at least one record. """ + cache = self._data.get(field) + if not cache: + return False + # 'cache' keys are tuples if 'field' is context-dependent, record ids otherwise + if isinstance(next(iter(cache)), tuple): + return any(value for value in cache.values()) + return True + def get(self, record, field, default=NOTHING): """ Return the value of ``field`` for ``record``. """ try: diff --git a/odoo/models.py b/odoo/models.py index d073f6d486c..48c84673333 100644 --- a/odoo/models.py +++ b/odoo/models.py @@ -132,15 +132,33 @@ def fix_import_export_id_paths(fieldname): fixed_external_id = re.sub(r'([^/]):id', r'\1/id', fixed_db_id) return fixed_external_id.split('/') -def trigger_tree_merge(node1, node2): - """ Merge two trigger trees. """ - for key, val in node2.items(): - if key is None: - node1.setdefault(None, OrderedSet()) - node1[None].update(val) - else: - node1.setdefault(key, {}) - trigger_tree_merge(node1[key], node2[key]) +def merge_trigger_trees(trees: list, select=bool) -> dict: + """ Merge trigger trees list into a final tree. The function ``select`` is + called on every field to determine which fields should be kept in the tree + nodes. This enables to discard some fields from the tree nodes. + """ + result_tree = {} # the resulting tree + root_fields = OrderedSet() # the fields in the root node + subtrees_to_merge = defaultdict(list) # the subtrees to merge grouped by key + + for tree in trees: + for key, val in tree.items(): + if key is None: + root_fields.update(val) + else: + subtrees_to_merge[key].append(val) + + # the root node contains the collected fields for which select is true + root_node = [field for field in root_fields if select(field)] + if root_node: + result_tree[None] = root_node + + for key, subtrees in subtrees_to_merge.items(): + subtree = merge_trigger_trees(subtrees, select) + if subtree: + result_tree[key] = subtree + + return result_tree class MetaModel(api.Meta): @@ -6260,15 +6278,26 @@ class BaseModel(metaclass=MetaModel): # - mark H to recompute on inverse(X, records), # - mark I to recompute on inverse(W, inverse(X, records)), # - mark J to recompute on inverse(Y, records). - if len(fnames) == 1: - tree = self.pool.field_triggers.get(self._fields[next(iter(fnames))]) - else: - # merge dependency trees to evaluate all triggers at once - tree = {} - for fname in fnames: - node = self.pool.field_triggers.get(self._fields[fname]) - if node: - trigger_tree_merge(tree, node) + fields = [self._fields[fname] for fname in fnames] + field_triggers = self.pool.field_triggers + trees = [field_triggers[field] for field in fields if field in field_triggers] + + if not trees: + return + + cache = self.env.cache + + # Merge dependency trees to evaluate all triggers at once. + # For non-stored computed fields, `_modified_triggers` might traverse + # the tree (at the cost of extra queries) only to know which records to + # invalidate in cache. But in many cases, most of these fields have no + # data in cache, so they can be ignored from the start. This allows us + # to discard subtrees from the merged tree when they only contain such + # fields. + tree = merge_trigger_trees( + trees, + select=lambda field: (field.compute and field.store) or cache.contains_field(field), + ) if tree: # determine what to compute (through an iterator)