[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 4b1cb41, 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 made more specific: it only invalidates the field that needs
   to be recomputed.

Fixes #76946, #77042

closes #82933

Opw: 2657461
X-original-commit: 3c8a6e7
Signed-off-by: Raphael Collet <rco@odoo.com>

addons/product/models/product.py

@@ -336,11 +336,7 @@ def write(self, values):
         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

@@ -584,8 +584,8 @@ def write(self, values):
         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()
+        self.env['product.template'].invalidate_cache(['price'])
+        self.env['product.product'].invalidate_cache(['price'])
         return res
 
     def _compute_price(self, price, price_uom, product, quantity=1.0, partner=False):

addons/product/tests/test_variants.py

@@ -4,6 +4,8 @@
 import base64
 from collections import OrderedDict
 import io
+import unittest.mock
+
 from PIL import Image
 
 from . import common
@@ -1179,3 +1181,52 @@ def _assert_0color_x_0size(self, variants=None):
         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', 'description': 'Foo'})
+        self.assertEqual(product.name, 'Foo')
+        self.assertEqual(product.description, 'Foo')
+
+        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 'description': due to some programmed cache
+            # invalidation, the second field may not be properly assigned
+            product.write({'name': 'Bar', 'description': 'Bar'})
+            self.assertEqual(product.name, 'Bar')
+            self.assertEqual(product.description, 'Bar')