From 5a32587f16e46a583da1a77e9d7b4b7eae6812de Mon Sep 17 00:00:00 2001 From: Raphael Collet Date: Mon, 17 Jan 2022 13:08:02 +0000 Subject: [PATCH] [FIX] product: make cache invalidation more specific Some aggressive cache invalidation in the middle of method write() where the model has children models (in the _inherits sense) causes very nasty errors that are hard to fix. Consider two models A and B, where B inherits from A. Also consider two fields a1 and a2 on A, which are thus both inherited by B. Now take a record from model B, and update both fields as: record.write({'a1': ..., 'a2': ...}) As both fields appear as related fields on model B, the method write() puts all values in cache, then it proceeds to call the inverse method of both fields. The inverse method of a1 is called, and this writes on the parent record. Now imagine that some override on A invalidates the whole cache. When the inverse method of a2 is called, the field's value on B has been invalidated, and this therefore writes the value False on the record's parent. This patch removes and adapt such cache invalidations: - Since 4b1cb41cf7a3f936a1e6d00a2bb6a6a29e82d711, the cache invalidation in method write() of product.product is no longer necessary. - The cache invalidation in method write() of product.pricelist.item has been removed as well, since field that needs to be recomputed no longer exists. Fixes #76946, #77042 closes odoo/odoo#82958 Opw: 2657461 X-original-commit: 5a73a8a3697c0f2685471efa252e5eb1af738e83 Signed-off-by: Raphael Collet --- addons/product/models/product.py | 6 +-- addons/product/models/product_pricelist.py | 7 +-- addons/product/tests/test_variants.py | 51 ++++++++++++++++++++++ 3 files changed, 53 insertions(+), 11 deletions(-) diff --git a/addons/product/models/product.py b/addons/product/models/product.py index e0801ce4967..530d920dbf6 100644 --- a/addons/product/models/product.py +++ b/addons/product/models/product.py @@ -348,11 +348,7 @@ class ProductProduct(models.Model): if 'product_template_attribute_value_ids' in values: # `_get_variant_id_for_combination` depends on `product_template_attribute_value_ids` self.clear_caches() - if 'active' in values: - # prefetched o2m have to be reloaded (because of active_test) - # (eg. product.template: product_variant_ids) - self.flush() - self.invalidate_cache() + elif 'active' in values: # `_get_first_possible_variant_id` depends on variants active state self.clear_caches() return res diff --git a/addons/product/models/product_pricelist.py b/addons/product/models/product_pricelist.py index ba2bfb4229c..ecd118c3405 100644 --- a/addons/product/models/product_pricelist.py +++ b/addons/product/models/product_pricelist.py @@ -571,12 +571,7 @@ class PricelistItem(models.Model): values.update(dict(product_id=None, categ_id=None)) elif applied_on == '0_product_variant': values.update(dict(categ_id=None)) - res = super(PricelistItem, self).write(values) - # When the pricelist changes we need the product.template price - # to be invalided and recomputed. - self.flush() - self.invalidate_cache() - return res + return super(PricelistItem, self).write(values) def _is_applicable_for(self, product, qty_in_product_uom): """Check whether the current rule is valid for the given product & qty. diff --git a/addons/product/tests/test_variants.py b/addons/product/tests/test_variants.py index 0cc9e5cb657..35f9a8e1450 100644 --- a/addons/product/tests/test_variants.py +++ b/addons/product/tests/test_variants.py @@ -5,6 +5,8 @@ import base64 from collections import OrderedDict from datetime import timedelta import io +import unittest.mock + from PIL import Image from . import common @@ -1199,3 +1201,52 @@ class TestVariantsArchive(common.TestProductCommon): variants = variants or self.template.product_variant_ids self.assertEqual(len(variants), 1) self.assertFalse(variants[0].product_template_attribute_value_ids) + + +class TestVariantWrite(TransactionCase): + + def test_active_one2many(self): + template = self.env['product.template'].create({'name': 'Foo', 'description': 'Foo'}) + self.assertEqual(len(template.product_variant_ids), 1) + + # check the consistency of one2many field product_variant_ids w.r.t. active variants + variant1 = template.product_variant_ids + variant2 = self.env['product.product'].create({'product_tmpl_id': template.id}) + self.assertEqual(template.product_variant_ids, variant1 + variant2) + + variant2.active = False + self.assertEqual(template.product_variant_ids, variant1) + + variant2.active = True + self.assertEqual(template.product_variant_ids, variant1 + variant2) + + variant1.active = False + self.assertEqual(template.product_variant_ids, variant2) + + def test_write_inherited_field(self): + product = self.env['product.product'].create({'name': 'Foo', 'sequence': 1}) + self.assertEqual(product.name, 'Foo') + self.assertEqual(product.sequence, 1) + + self.env['product.pricelist'].create({ + 'name': 'Foo', + 'item_ids': [(0, 0, {'product_id': product.id, 'fixed_price': 1})], + }) + + # patch template.write to modify pricelist items, which causes some + # cache invalidation + Template = self.registry['product.template'] + Template_write = Template.write + + def write(self, vals): + result = Template_write(self, vals) + items = self.env['product.pricelist.item'].search([('product_id', '=', product.id)]) + items.fixed_price = 2 + return result + + with unittest.mock.patch.object(Template, 'write', write): + # change both 'name' and 'sequence': due to some programmed cache + # invalidation, the second field may not be properly assigned + product.write({'name': 'Bar', 'sequence': 2}) + self.assertEqual(product.name, 'Bar') + self.assertEqual(product.sequence, 2)