Browse Source

Fixed #30427, Fixed #16176 -- Corrected setting descriptor in Field.contribute_to_class().

Co-authored-by: Jarek Glowacki <jarekwg@gmail.com>
Carlton Gibson 3 years ago
parent
commit
225d96533a

+ 1 - 5
django/db/models/fields/__init__.py

@@ -782,11 +782,7 @@ class Field(RegisterLookupMixin):
         self.model = cls
         cls._meta.add_field(self, private=private_only)
         if self.column:
-            # Don't override classmethods with the descriptor. This means that
-            # if you have a classmethod and a field with the same name, then
-            # such fields can't be deferred (we don't have a check for this).
-            if not getattr(cls, self.attname, None):
-                setattr(cls, self.attname, self.descriptor_class(self))
+            setattr(cls, self.attname, self.descriptor_class(self))
         if self.choices is not None:
             # Don't override a get_FOO_display() method defined explicitly on
             # this class, but don't check methods derived from inheritance, to

+ 7 - 0
docs/topics/db/models.txt

@@ -1457,6 +1457,13 @@ different database tables).
 Django will raise a :exc:`~django.core.exceptions.FieldError` if you override
 any model field in any ancestor model.
 
+Note that because of the way fields are resolved during class definition, model
+fields inherited from multiple abstract parent models are resolved in a strict
+depth-first order. This contrasts with standard Python MRO, which is resolved
+breadth-first in cases of diamond shaped inheritance. This difference only
+affects complex model hierarchies, which (as per the advice above) you should
+try to avoid.
+
 Organizing models in a package
 ==============================
 

+ 6 - 0
tests/check_framework/tests.py

@@ -314,6 +314,12 @@ class CheckFrameworkReservedNamesTests(SimpleTestCase):
                 obj=ModelWithAttributeCalledCheck,
                 id='models.E020'
             ),
+            Error(
+                "The 'ModelWithFieldCalledCheck.check()' class method is "
+                "currently overridden by %r." % ModelWithFieldCalledCheck.check,
+                obj=ModelWithFieldCalledCheck,
+                id='models.E020'
+            ),
             Error(
                 "The 'ModelWithRelatedManagerCalledCheck.check()' class method is "
                 "currently overridden by %r." % ModelWithRelatedManagerCalledCheck.check,

+ 13 - 0
tests/defer/models.py

@@ -44,3 +44,16 @@ class RefreshPrimaryProxy(Primary):
             if fields.intersection(deferred_fields):
                 fields = fields.union(deferred_fields)
         super().refresh_from_db(using, fields, **kwargs)
+
+
+class ShadowParent(models.Model):
+    """
+    ShadowParent declares a scalar, rather than a field. When this is
+    overridden, the field value, rather than the scalar value must still be
+    used when the field is deferred.
+    """
+    name = 'aphrodite'
+
+
+class ShadowChild(ShadowParent):
+    name = models.CharField(default='adonis', max_length=6)

+ 6 - 0
tests/defer/tests.py

@@ -3,6 +3,7 @@ from django.test import TestCase
 
 from .models import (
     BigChild, Child, ChildProxy, Primary, RefreshPrimaryProxy, Secondary,
+    ShadowChild,
 )
 
 
@@ -165,6 +166,11 @@ class DeferTests(AssertionMixin, TestCase):
         self.assertEqual(obj.name, "c1")
         self.assertEqual(obj.value, "foo")
 
+    def test_defer_of_overridden_scalar(self):
+        ShadowChild.objects.create()
+        obj = ShadowChild.objects.defer('name').get()
+        self.assertEqual(obj.name, 'adonis')
+
 
 class BigChildDeferTests(AssertionMixin, TestCase):
     @classmethod

+ 2 - 3
tests/invalid_models_tests/test_models.py

@@ -1212,9 +1212,8 @@ class OtherModelTests(SimpleTestCase):
         class Model(models.Model):
             fk = models.ForeignKey('self', models.CASCADE)
 
-            @property
-            def fk_id(self):
-                pass
+        # Override related field accessor.
+        Model.fk_id = property(lambda self: 'ERROR')
 
         self.assertEqual(Model.check(), [
             Error(

+ 46 - 9
tests/model_inheritance/test_abstract_inheritance.py

@@ -34,7 +34,12 @@ class AbstractInheritanceTests(SimpleTestCase):
         self.assertEqual(DerivedChild._meta.get_field('name').max_length, 50)
         self.assertEqual(DerivedGrandChild._meta.get_field('name').max_length, 50)
 
-    def test_multiple_inheritance_cannot_shadow_inherited_field(self):
+    def test_multiple_inheritance_allows_inherited_field(self):
+        """
+        Single layer multiple inheritance is as expected, deriving the
+        inherited field from the first base.
+        """
+
         class ParentA(models.Model):
             name = models.CharField(max_length=255)
 
@@ -50,14 +55,46 @@ class AbstractInheritanceTests(SimpleTestCase):
         class Child(ParentA, ParentB):
             pass
 
-        self.assertEqual(Child.check(), [
-            Error(
-                "The field 'name' clashes with the field 'name' from model "
-                "'model_inheritance.child'.",
-                obj=Child._meta.get_field('name'),
-                id='models.E006',
-            ),
-        ])
+        self.assertEqual(Child.check(), [])
+        inherited_field = Child._meta.get_field('name')
+        self.assertTrue(isinstance(inherited_field, models.CharField))
+        self.assertEqual(inherited_field.max_length, 255)
+
+    def test_diamond_shaped_multiple_inheritance_is_depth_first(self):
+        """
+        In contrast to standard Python MRO, resolution of inherited fields is
+        strictly depth-first, rather than breadth-first in diamond-shaped cases.
+
+        This is because a copy of the parent field descriptor is placed onto
+        the model class in ModelBase.__new__(), rather than the attribute
+        lookup going via bases. (It only **looks** like inheritance.)
+
+        Here, Child inherits name from Root, rather than ParentB.
+        """
+
+        class Root(models.Model):
+            name = models.CharField(max_length=255)
+
+            class Meta:
+                abstract = True
+
+        class ParentA(Root):
+            class Meta:
+                abstract = True
+
+        class ParentB(Root):
+            name = models.IntegerField()
+
+            class Meta:
+                abstract = True
+
+        class Child(ParentA, ParentB):
+            pass
+
+        self.assertEqual(Child.check(), [])
+        inherited_field = Child._meta.get_field('name')
+        self.assertTrue(isinstance(inherited_field, models.CharField))
+        self.assertEqual(inherited_field.max_length, 255)
 
     def test_target_field_may_be_pushed_down(self):
         """

+ 31 - 0
tests/model_inheritance/tests.py

@@ -2,6 +2,7 @@ from operator import attrgetter
 
 from django.core.exceptions import FieldError, ValidationError
 from django.db import connection, models
+from django.db.models.query_utils import DeferredAttribute
 from django.test import SimpleTestCase, TestCase
 from django.test.utils import CaptureQueriesContext, isolate_apps
 
@@ -222,6 +223,36 @@ class ModelInheritanceTests(TestCase):
         self.assertIs(models.QuerySet[Post, Post], models.QuerySet)
         self.assertIs(models.QuerySet[Post, int, str], models.QuerySet)
 
+    def test_shadow_parent_attribute_with_field(self):
+        class ScalarParent(models.Model):
+            foo = 1
+
+        class ScalarOverride(ScalarParent):
+            foo = models.IntegerField()
+
+        self.assertEqual(type(ScalarOverride.foo), DeferredAttribute)
+
+    def test_shadow_parent_property_with_field(self):
+        class PropertyParent(models.Model):
+            @property
+            def foo(self):
+                pass
+
+        class PropertyOverride(PropertyParent):
+            foo = models.IntegerField()
+
+        self.assertEqual(type(PropertyOverride.foo), DeferredAttribute)
+
+    def test_shadow_parent_method_with_field(self):
+        class MethodParent(models.Model):
+            def foo(self):
+                pass
+
+        class MethodOverride(MethodParent):
+            foo = models.IntegerField()
+
+        self.assertEqual(type(MethodOverride.foo), DeferredAttribute)
+
 
 class ModelInheritanceDataTests(TestCase):
     @classmethod