Skip to content

Commit

Permalink
Fixed #30427, Fixed #16176 -- Corrected setting descriptor in Field.c…
Browse files Browse the repository at this point in the history
…ontribute_to_class().

Co-authored-by: Jarek Glowacki <jarekwg@gmail.com>
  • Loading branch information
carltongibson and jarekwg committed Jun 15, 2021
1 parent 0c0240a commit 225d965
Show file tree
Hide file tree
Showing 8 changed files with 112 additions and 17 deletions.
6 changes: 1 addition & 5 deletions django/db/models/fields/__init__.py
Expand Up @@ -782,11 +782,7 @@ def contribute_to_class(self, cls, name, private_only=False):
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
Expand Down
7 changes: 7 additions & 0 deletions docs/topics/db/models.txt
Expand Up @@ -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
==============================

Expand Down
6 changes: 6 additions & 0 deletions tests/check_framework/tests.py
Expand Up @@ -314,6 +314,12 @@ class ModelWithDescriptorCalledCheck(models.Model):
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,
Expand Down
13 changes: 13 additions & 0 deletions tests/defer/models.py
Expand Up @@ -44,3 +44,16 @@ def refresh_from_db(self, using=None, fields=None, **kwargs):
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 changes: 6 additions & 0 deletions tests/defer/tests.py
Expand Up @@ -3,6 +3,7 @@

from .models import (
BigChild, Child, ChildProxy, Primary, RefreshPrimaryProxy, Secondary,
ShadowChild,
)


Expand Down Expand Up @@ -165,6 +166,11 @@ def test_only_baseclass_when_subclass_has_no_added_fields(self):
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
Expand Down
5 changes: 2 additions & 3 deletions tests/invalid_models_tests/test_models.py
Expand Up @@ -1212,9 +1212,8 @@ def test_property_and_related_field_accessor_clash(self):
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(
Expand Down
55 changes: 46 additions & 9 deletions tests/model_inheritance/test_abstract_inheritance.py
Expand Up @@ -34,7 +34,12 @@ class DerivedGrandChild(AbstractDescendant):
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)

Expand All @@ -50,14 +55,46 @@ class Meta:
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):
"""
Expand Down
31 changes: 31 additions & 0 deletions tests/model_inheritance/tests.py
Expand Up @@ -2,6 +2,7 @@

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

Expand Down Expand Up @@ -222,6 +223,36 @@ def test_queryset_class_getitem(self):
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
Expand Down

0 comments on commit 225d965

Please sign in to comment.