[IMP] core: do not recompute stored field on new record

The use case is a cache miss of a stored field with compute on a new
record with origin.  The field should be computed only when explicitly
triggered, i.e., when a dependency has been modified.  Otherwise it
should be fetched from the origin record.

closes odoo/odoo#47353

Signed-off-by: Raphael Collet (rco) <rco@openerp.com>

6 files changed, 131 insertions, 15 deletions

diff --git a/odoo/addons/test_new_api/models/test_new_api.py b/odoo/addons/test_new_api/models/test_new_api.py
index 14a981b3b0c..9331f6eedee 100644
--- a/odoo/addons/test_new_api/models/test_new_api.py
+++ b/odoo/addons/test_new_api/models/test_new_api.py
@@ -796,6 +796,7 @@ class ModelChildM2o(models.Model):
     parent_id = fields.Many2one('test_new_api.model_parent_m2o', ondelete='cascade')
     size1 = fields.Integer(compute='_compute_sizes', store=True)
     size2 = fields.Integer(compute='_compute_sizes', store=True)
+    cost = fields.Integer(compute='_compute_cost', store=True, readonly=False)
 
     @api.depends('parent_id.name')
     def _compute_sizes(self):
@@ -803,6 +804,11 @@ class ModelChildM2o(models.Model):
             record.size1 = len(self.parent_id.name)
             record.size2 = len(self.parent_id.name)
 
+    @api.depends('name')
+    def _compute_cost(self):
+        for record in self:
+            record.cost = len(record.name)
+
     def write(self, vals):
         res = super(ModelChildM2o, self).write(vals)
         if self.name == 'A':
@@ -816,6 +822,12 @@ class ModelParentM2o(models.Model):
 
     name = fields.Char('Name')
     child_ids = fields.One2many('test_new_api.model_child_m2o', 'parent_id', string="Children")
+    cost = fields.Integer(compute='_compute_cost', store=True)
+
+    @api.depends('child_ids.cost')
+    def _compute_cost(self):
+        for record in self:
+            record.cost = sum(child.cost for child in record.child_ids)
 
 
 class Country(models.Model):
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 c27704a47d9..7aa9b3c5a74 100644
--- a/odoo/addons/test_new_api/tests/test_new_fields.py
+++ b/odoo/addons/test_new_api/tests/test_new_fields.py
@@ -1090,6 +1090,52 @@ class TestFields(TransactionCaseWithUserDemo):
         with self.assertRaises(AccessError):
             cat1.name
 
+    def test_40_real_vs_new(self):
+        """ test field access on new records vs real records. """
+        Model = self.env['test_new_api.category']
+        real_record = Model.create({'name': 'Foo'})
+        self.env.cache.invalidate()
+        new_origin = Model.new({'name': 'Bar'}, origin=real_record)
+        new_record = Model.new({'name': 'Baz'})
+
+        # non-computed non-stored field: default value
+        real_record = real_record.with_context(default_dummy='WTF')
+        new_origin = new_origin.with_context(default_dummy='WTF')
+        new_record = new_record.with_context(default_dummy='WTF')
+        self.assertEqual(real_record.dummy, 'WTF')
+        self.assertEqual(new_origin.dummy, 'WTF')
+        self.assertEqual(new_record.dummy, 'WTF')
+
+        # non-computed stored field: origin or default if no origin
+        real_record = real_record.with_context(default_color=42)
+        new_origin = new_origin.with_context(default_color=42)
+        new_record = new_record.with_context(default_color=42)
+        self.assertEqual(real_record.color, 0)
+        self.assertEqual(new_origin.color, 0)
+        self.assertEqual(new_record.color, 42)
+
+        # computed non-stored field: always computed
+        self.assertEqual(real_record.display_name, 'Foo')
+        self.assertEqual(new_origin.display_name, 'Bar')
+        self.assertEqual(new_record.display_name, 'Baz')
+
+        # computed stored field: origin or computed if no origin
+        Model = self.env['test_new_api.recursive']
+        real_record = Model.create({'name': 'Foo'})
+        new_origin = Model.new({'name': 'Bar'}, origin=real_record)
+        new_record = Model.new({'name': 'Baz'})
+        self.assertEqual(real_record.display_name, 'Foo')
+        self.assertEqual(new_origin.display_name, 'Foo')
+        self.assertEqual(new_record.display_name, 'Baz')
+
+        # computed stored field with recomputation: always computed
+        real_record.name = 'Fool'
+        new_origin.name = 'Barr'
+        new_record.name = 'Bazz'
+        self.assertEqual(real_record.display_name, 'Fool')
+        self.assertEqual(new_origin.display_name, 'Barr')
+        self.assertEqual(new_record.display_name, 'Bazz')
+
     def test_40_new_defaults(self):
         """ Test new records with defaults. """
         user = self.env.user
diff --git a/odoo/addons/test_new_api/tests/test_onchange.py b/odoo/addons/test_new_api/tests/test_onchange.py
index 5d544bd2875..398bd537bbd 100644
--- a/odoo/addons/test_new_api/tests/test_onchange.py
+++ b/odoo/addons/test_new_api/tests/test_onchange.py
@@ -678,3 +678,31 @@ class TestComputeOnchange(common.TransactionCase):
         form.foo = "foo6"
         self.assertEqual(form.bar, "foo6")
         self.assertEqual(form.baz, "baz5")
+
+    def test_onchange_one2many(self):
+        record = self.env['test_new_api.model_parent_m2o'].create({
+            'name': 'Family',
+            'child_ids': [
+                (0, 0, {'name': 'W', 'cost': 10}),
+                (0, 0, {'name': 'X', 'cost': 10}),
+                (0, 0, {'name': 'Y'}),
+                (0, 0, {'name': 'Z'}),
+            ],
+        })
+        record.flush()
+        self.assertEqual(record.child_ids.mapped('name'), list('WXYZ'))
+        self.assertEqual(record.cost, 22)
+
+        # modifying a line should not recompute the cost on other lines
+        with common.Form(record) as form:
+            with form.child_ids.edit(1) as line:
+                line.name = 'XXX'
+            self.assertEqual(form.cost, 15)
+
+            with form.child_ids.edit(1) as line:
+                line.cost = 20
+            self.assertEqual(form.cost, 32)
+
+            with form.child_ids.edit(2) as line:
+                line.cost = 30
+            self.assertEqual(form.cost, 61)
diff --git a/odoo/addons/test_new_api/views/test_new_api_views.xml b/odoo/addons/test_new_api/views/test_new_api_views.xml
index 830210c4d87..c0ed0a7baa7 100644
--- a/odoo/addons/test_new_api/views/test_new_api_views.xml
+++ b/odoo/addons/test_new_api/views/test_new_api_views.xml
@@ -296,5 +296,28 @@
                 </form>
             </field>
         </record>
+
+        <!-- Multi form view -->
+        <record id="model_parent_form" model="ir.ui.view">
+            <field name="name">model parent form view</field>
+            <field name="model">test_new_api.model_parent_m2o</field>
+            <field name="arch" type="xml">
+                <form string="Model Parent">
+                    <sheet>
+                        <group>
+                            <field name="name"/>
+                            <field name="cost"/>
+                        </group>
+                        <label for="child_ids"/>
+                        <field name="child_ids">
+                            <tree string="Children" editable="bottom">
+                                <field name="name"/>
+                                <field name="cost"/>
+                            </tree>
+                        </field>
+                    </sheet>
+                </form>
+            </field>
+        </record>
     </data>
 </odoo>
diff --git a/odoo/fields.py b/odoo/fields.py
index 49bdedd65f2..8e57ebb13c7 100644
--- a/odoo/fields.py
+++ b/odoo/fields.py
@@ -979,8 +979,8 @@ class Field(MetaField('DummyField', (object,), {})):
             value = env.cache.get(record, self)
 
         except KeyError:
-            # real record
-            if record.id and self.store:
+            if self.store and record.id:
+                # real record: fetch from database
                 recs = record._in_cache_without(self)
                 try:
                     recs._fetch_field(self)
@@ -993,7 +993,13 @@ class Field(MetaField('DummyField', (object,), {})):
                     ]))
                 value = env.cache.get(record, self)
 
+            elif self.store and record._origin:
+                # new record with origin: fetch from origin
+                value = self.convert_to_cache(record._origin[self.name], record)
+                env.cache.set(record, self, value)
+
             elif self.compute:
+                # non-stored field or new record without origin: compute
                 if env.is_protected(self, record):
                     value = self.convert_to_cache(False, record, validate=False)
                     env.cache.set(record, self, value)
@@ -1005,12 +1011,8 @@ class Field(MetaField('DummyField', (object,), {})):
                         self.compute_value(record)
                     value = env.cache.get(record, self)
 
-            elif (not record.id) and record._origin:
-                value = self.convert_to_cache(record._origin[self.name], record)
-                env.cache.set(record, self, value)
-
-            elif (not record.id) and self.type == 'many2one' and self.delegate:
-                # the parent record is also a new record, with the same cached
+            elif self.type == 'many2one' and self.delegate and not record.id:
+                # parent record of a new record: new record, with the same
                 # values as record for the corresponding inherited fields
                 def is_inherited_field(name):
                     field = record._fields[name]
@@ -1025,15 +1027,16 @@ class Field(MetaField('DummyField', (object,), {})):
                 env.cache.set(record, self, value)
 
             else:
+                # non-stored field or stored field on new record: default value
                 value = self.convert_to_cache(False, record, validate=False)
                 env.cache.set(record, self, value)
                 defaults = record.default_get([self.name])
                 if self.name in defaults:
-                    # The null value above is necessary to convert x2many field values.
-                    # For instance, converting [(4, id)] accesses the field's current
-                    # value, then adds the given id. Without an initial value, the
-                    # conversion ends up here to determine the field's value, and this
-                    # generates an infinite recursion.
+                    # The null value above is necessary to convert x2many field
+                    # values. For instance, converting [(4, id)] accesses the
+                    # field's current value, then adds the given id. Without an
+                    # initial value, the conversion ends up here to determine
+                    # the field's value, and generates an infinite recursion.
                     value = self.convert_to_cache(defaults[self.name], record)
                     env.cache.set(record, self, value)
 
diff --git a/odoo/models.py b/odoo/models.py
index b667fef2706..3a602f2bff6 100644
--- a/odoo/models.py
+++ b/odoo/models.py
@@ -6142,8 +6142,12 @@ Record ids: %(records)s
         # make a snapshot based on the initial values of record
         snapshot0 = Snapshot(record, nametree)
 
-        # store changed values in cache, and update snapshot0
-        record._update_cache(changed_values, validate=False)
+        # store changed values in cache; also trigger recomputations based on
+        # subfields (e.g., line.a has been modified, line.b is computed stored
+        # and depends on line.a, but line.b is not in the form view)
+        record._update_cache(changed_values, validate=True)
+
+        # update snapshot0 with changed values
         for name in names:
             snapshot0.fetch(name)