[FIX] fields: clash with automatic fields and inheritance

Consider a model M that defines `display_name` as a computed stored
field, then an extension of M that introduces a mixin model A before
that definition in the MRO of M's class.  The model A is expected to
have an automatic, non stored field `display_name`, while M must have
its non-automatic, stored field `display_name`.

closes odoo/odoo#36427

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

odoo/addons/test_new_api/ir.model.access.csv

@@ -40,3 +40,4 @@ access_test_new_api_model_b,access_test_new_api_model_b,model_test_new_api_model
 access_test_new_api_model_parent,access_test_new_api_model_parent,model_test_new_api_model_parent,,1,1,1,1
 access_test_new_api_model_child,access_test_new_api_model_child,model_test_new_api_model_child,,1,1,1,1
 access_test_new_api_model_child_nocheck,access_test_new_api_model_child_nocheck,model_test_new_api_model_child_nocheck,,1,1,1,1
+access_test_new_api_display,access_test_new_api_display,model_test_new_api_display,,1,1,1,1

odoo/addons/test_new_api/models.py

@@ -682,3 +682,28 @@ class ModelChildNoCheck(models.Model):
     name = fields.Char()
     company_id = fields.Many2one('res.company', required=True)
     parent_id = fields.Many2one('test_new_api.model_parent', check_company=False)
+
+
+# model with explicit and stored field 'display_name'
+class Display(models.Model):
+    _name = 'test_new_api.display'
+    _description = 'Model that overrides display_name'
+
+    display_name = fields.Char(compute='_compute_display_name', store=True)
+
+    def _compute_display_name(self):
+        for record in self:
+            record.display_name = 'My id is %s' % (record.id)
+
+
+# abstract model with automatic and non-stored field 'display_name'
+class Mixin(models.AbstractModel):
+    _name = 'test_new_api.mixin'
+    _description = 'Dummy mixin model'
+
+
+# in this model extension, the field 'display_name' should not be inherited from
+# 'test_new_api.mixin'
+class ExtendedDisplay(models.Model):
+    _name = 'test_new_api.display'
+    _inherit = ['test_new_api.mixin', 'test_new_api.display']

odoo/addons/test_new_api/tests/test_new_fields.py

@@ -2,6 +2,7 @@
 # test cases for new-style fields
 #
 import base64
+from collections import OrderedDict
 from datetime import date, datetime, time
 import io
 from PIL import Image
@@ -1709,6 +1710,43 @@ class TestMagicFields(common.TransactionCase):
         self.assertEqual(record.create_uid, self.env.user)
         self.assertEqual(record.write_uid, self.env.user)
 
+    def test_mro_mixin(self):
+        #                               Mixin
+        #                                |
+        #                                |
+        #                                |
+        #   ExtendedDisplay    'test_new_api.mixin'    Display    'base'
+        #         |                      |                |         |
+        #         +----------------------+-+--------------+---------+
+        #                                  |
+        #                       'test_new_api.display'
+        #
+        # The field 'display_name' is defined as store=True on the class Display
+        # above.  The field 'display_name' on the model 'test_new_api.mixin' is
+        # expected to be automatic and non-stored.  But the field 'display_name'
+        # on the model 'test_new_api.display' should not be automatic: it must
+        # correspond to the definition given in class Display, even if the MRO
+        # of the model shows the automatic field on the mixin model before the
+        # actual definition.
+        registry = self.env.registry
+        models = registry.models
+
+        # check setup of models in alphanumeric order
+        self.patch(registry, 'models', OrderedDict(sorted(models.items())))
+        registry.model_cache.clear()
+        registry.setup_models(self.cr)
+        field = registry['test_new_api.display'].display_name
+        self.assertFalse(field.automatic)
+        self.assertTrue(field.store)
+
+        # check setup of models in reverse alphanumeric order
+        self.patch(registry, 'models', OrderedDict(sorted(models.items(), reverse=True)))
+        registry.model_cache.clear()
+        registry.setup_models(self.cr)
+        field = registry['test_new_api.display'].display_name
+        self.assertFalse(field.automatic)
+        self.assertTrue(field.store)
+
 
 class TestParentStore(common.TransactionCase):
 

odoo/fields.py

@@ -49,11 +49,12 @@ def first(records):
 
 def resolve_mro(model, name, predicate):
     """ Return the list of successively overridden values of attribute ``name``
-        in mro order on ``model`` that satisfy ``predicate``.
+        in mro order on ``model`` that satisfy ``predicate``.  Model classes
+        (the ones that appear in the registry) are ignored.
     """
     result = []
     for cls in type(model).__mro__:
-        if name in cls.__dict__:
+        if not getattr(cls, 'pool', None) and name in cls.__dict__:
             value = cls.__dict__[name]
             if not predicate(value):
                 break
@@ -392,6 +393,9 @@ class Field(MetaField('DummyField', (object,), {})):
         # determine all inherited field attributes
         modules = set()
         attrs = {}
+        if self.args.get('automatic') and resolve_mro(model, name, self._can_setup_from):
+            # prevent an automatic field from overriding a real field
+            self.args.clear()
         if not (self.args.get('automatic') or self.args.get('manual')):
             # magic and custom fields do not inherit from parent classes
             for field in reversed(resolve_mro(model, name, self._can_setup_from)):