[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 4b1cb41cf7, 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 <rco@odoo.com>

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

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.

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)