From 74a9c2711cd81708ae754c4d92432511efa96149 Mon Sep 17 00:00:00 2001 From: Adam Johnson Date: Mon, 14 Apr 2025 15:46:37 +0100 Subject: [PATCH] Refs #28586 -- Split descriptor from GenericForeignKey. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This makes GenericForeignKey more similar to other fields which act as descriptors, preparing it to add “fetcher protocol” support in a clear and consistent way. --- django/contrib/contenttypes/checks.py | 10 +++--- django/contrib/contenttypes/fields.py | 48 ++++++++++++++----------- docs/releases/6.1.txt | 3 +- tests/contenttypes_tests/test_checks.py | 30 ++++++++++------ tests/contenttypes_tests/test_fields.py | 7 ++-- 5 files changed, 60 insertions(+), 38 deletions(-) diff --git a/django/contrib/contenttypes/checks.py b/django/contrib/contenttypes/checks.py index aae9dcb31b..392f60a655 100644 --- a/django/contrib/contenttypes/checks.py +++ b/django/contrib/contenttypes/checks.py @@ -5,7 +5,7 @@ from django.core.checks import Error def check_generic_foreign_keys(app_configs, **kwargs): - from .fields import GenericForeignKey + from .fields import GenericForeignKeyDescriptor if app_configs is None: models = apps.get_models() @@ -14,14 +14,14 @@ def check_generic_foreign_keys(app_configs, **kwargs): app_config.get_models() for app_config in app_configs ) errors = [] - fields = ( + descriptors = ( obj for model in models for obj in vars(model).values() - if isinstance(obj, GenericForeignKey) + if isinstance(obj, GenericForeignKeyDescriptor) ) - for field in fields: - errors.extend(field.check()) + for descriptor in descriptors: + errors.extend(descriptor.field.check()) return errors diff --git a/django/contrib/contenttypes/fields.py b/django/contrib/contenttypes/fields.py index d85b61933a..f98dda1255 100644 --- a/django/contrib/contenttypes/fields.py +++ b/django/contrib/contenttypes/fields.py @@ -48,8 +48,7 @@ class GenericForeignKey(FieldCacheMixin, Field): def contribute_to_class(self, cls, name, **kwargs): super().contribute_to_class(cls, name, private_only=True, **kwargs) - # GenericForeignKey is its own descriptor. - setattr(cls, self.attname, self) + setattr(cls, self.attname, GenericForeignKeyDescriptor(self)) def get_attname_column(self): attname, column = super().get_attname_column() @@ -161,11 +160,19 @@ class GenericForeignKey(FieldCacheMixin, Field): # This should never happen. I love comments like this, don't you? raise Exception("Impossible arguments to GFK.get_content_type!") + +class GenericForeignKeyDescriptor: + def __init__(self, field): + self.field = field + + def is_cached(self, instance): + return self.field.is_cached(instance) + def get_prefetch_querysets(self, instances, querysets=None): custom_queryset_dict = {} if querysets is not None: for queryset in querysets: - ct_id = self.get_content_type( + ct_id = self.field.get_content_type( model=queryset.query.model, using=queryset.db ).pk if ct_id in custom_queryset_dict: @@ -179,12 +186,12 @@ class GenericForeignKey(FieldCacheMixin, Field): fk_dict = defaultdict(set) # We need one instance for each group in order to get the right db: instance_dict = {} - ct_attname = self.model._meta.get_field(self.ct_field).attname + ct_attname = self.field.model._meta.get_field(self.field.ct_field).attname for instance in instances: # We avoid looking for values if either ct_id or fkey value is None ct_id = getattr(instance, ct_attname) if ct_id is not None: - fk_val = getattr(instance, self.fk_field) + fk_val = getattr(instance, self.field.fk_field) if fk_val is not None: fk_dict[ct_id].add(fk_val) instance_dict[ct_id] = instance @@ -196,7 +203,7 @@ class GenericForeignKey(FieldCacheMixin, Field): ret_val.extend(custom_queryset_dict[ct_id].filter(pk__in=fkeys)) else: instance = instance_dict[ct_id] - ct = self.get_content_type(id=ct_id, using=instance._state.db) + ct = self.field.get_content_type(id=ct_id, using=instance._state.db) ret_val.extend(ct.get_all_objects_for_this_type(pk__in=fkeys)) # For doing the join in Python, we have to match both the FK val and @@ -207,17 +214,17 @@ class GenericForeignKey(FieldCacheMixin, Field): if ct_id is None: return None else: - model = self.get_content_type( + model = self.field.get_content_type( id=ct_id, using=obj._state.db ).model_class() - return str(getattr(obj, self.fk_field)), model + return str(getattr(obj, self.field.fk_field)), model return ( ret_val, lambda obj: (obj._meta.pk.value_to_string(obj), obj.__class__), gfk_key, True, - self.name, + self.field.name, False, ) @@ -229,16 +236,17 @@ class GenericForeignKey(FieldCacheMixin, Field): # reload the same ContentType over and over (#5570). Instead, get the # content type ID here, and later when the actual instance is needed, # use ContentType.objects.get_for_id(), which has a global cache. - f = self.model._meta.get_field(self.ct_field) + f = self.field.model._meta.get_field(self.field.ct_field) ct_id = getattr(instance, f.attname, None) - pk_val = getattr(instance, self.fk_field) + pk_val = getattr(instance, self.field.fk_field) - rel_obj = self.get_cached_value(instance, default=None) - if rel_obj is None and self.is_cached(instance): + rel_obj = self.field.get_cached_value(instance, default=None) + if rel_obj is None and self.field.is_cached(instance): return rel_obj if rel_obj is not None: ct_match = ( - ct_id == self.get_content_type(obj=rel_obj, using=instance._state.db).id + ct_id + == self.field.get_content_type(obj=rel_obj, using=instance._state.db).id ) pk_match = ct_match and rel_obj._meta.pk.to_python(pk_val) == rel_obj.pk if pk_match: @@ -246,26 +254,26 @@ class GenericForeignKey(FieldCacheMixin, Field): else: rel_obj = None if ct_id is not None: - ct = self.get_content_type(id=ct_id, using=instance._state.db) + ct = self.field.get_content_type(id=ct_id, using=instance._state.db) try: rel_obj = ct.get_object_for_this_type( using=instance._state.db, pk=pk_val ) except ObjectDoesNotExist: pass - self.set_cached_value(instance, rel_obj) + self.field.set_cached_value(instance, rel_obj) return rel_obj def __set__(self, instance, value): ct = None fk = None if value is not None: - ct = self.get_content_type(obj=value) + ct = self.field.get_content_type(obj=value) fk = value.pk - setattr(instance, self.ct_field, ct) - setattr(instance, self.fk_field, fk) - self.set_cached_value(instance, value) + setattr(instance, self.field.ct_field, ct) + setattr(instance, self.field.fk_field, fk) + self.field.set_cached_value(instance, value) class GenericRel(ForeignObjectRel): diff --git a/docs/releases/6.1.txt b/docs/releases/6.1.txt index c89eebebc6..309a775e74 100644 --- a/docs/releases/6.1.txt +++ b/docs/releases/6.1.txt @@ -246,7 +246,8 @@ backends. Miscellaneous ------------- -* ... +* :class:`~django.contrib.contenttypes.fields.GenericForeignKey` now uses a + separate descriptor class: the private ``GenericForeignKeyDescriptor``. .. _deprecated-features-6.1: diff --git a/tests/contenttypes_tests/test_checks.py b/tests/contenttypes_tests/test_checks.py index 50730c5a1d..c33920f6b7 100644 --- a/tests/contenttypes_tests/test_checks.py +++ b/tests/contenttypes_tests/test_checks.py @@ -19,15 +19,17 @@ class GenericForeignKeyTests(SimpleTestCase): object_id = models.PositiveIntegerField() content_object = GenericForeignKey() + field = TaggedItem._meta.get_field("content_object") + expected = [ checks.Error( "The GenericForeignKey content type references the nonexistent " "field 'TaggedItem.content_type'.", - obj=TaggedItem.content_object, + obj=field, id="contenttypes.E002", ) ] - self.assertEqual(TaggedItem.content_object.check(), expected) + self.assertEqual(field.check(), expected) def test_invalid_content_type_field(self): class Model(models.Model): @@ -35,8 +37,10 @@ class GenericForeignKeyTests(SimpleTestCase): object_id = models.PositiveIntegerField() content_object = GenericForeignKey("content_type", "object_id") + field = Model._meta.get_field("content_object") + self.assertEqual( - Model.content_object.check(), + field.check(), [ checks.Error( "'Model.content_type' is not a ForeignKey.", @@ -44,7 +48,7 @@ class GenericForeignKeyTests(SimpleTestCase): "GenericForeignKeys must use a ForeignKey to " "'contenttypes.ContentType' as the 'content_type' field." ), - obj=Model.content_object, + obj=field, id="contenttypes.E003", ) ], @@ -58,8 +62,10 @@ class GenericForeignKeyTests(SimpleTestCase): object_id = models.PositiveIntegerField() content_object = GenericForeignKey("content_type", "object_id") + field = Model._meta.get_field("content_object") + self.assertEqual( - Model.content_object.check(), + field.check(), [ checks.Error( "'Model.content_type' is not a ForeignKey to " @@ -68,7 +74,7 @@ class GenericForeignKeyTests(SimpleTestCase): "GenericForeignKeys must use a ForeignKey to " "'contenttypes.ContentType' as the 'content_type' field." ), - obj=Model.content_object, + obj=field, id="contenttypes.E004", ) ], @@ -80,13 +86,15 @@ class GenericForeignKeyTests(SimpleTestCase): # missing object_id field content_object = GenericForeignKey() + field = TaggedItem._meta.get_field("content_object") + self.assertEqual( - TaggedItem.content_object.check(), + field.check(), [ checks.Error( "The GenericForeignKey object ID references the nonexistent " "field 'object_id'.", - obj=TaggedItem.content_object, + obj=field, id="contenttypes.E001", ) ], @@ -98,12 +106,14 @@ class GenericForeignKeyTests(SimpleTestCase): object_id = models.PositiveIntegerField() content_object_ = GenericForeignKey("content_type", "object_id") + field = Model._meta.get_field("content_object_") + self.assertEqual( - Model.content_object_.check(), + field.check(), [ checks.Error( "Field names must not end with an underscore.", - obj=Model.content_object_, + obj=field, id="fields.E001", ) ], diff --git a/tests/contenttypes_tests/test_fields.py b/tests/contenttypes_tests/test_fields.py index c2b12b58bc..76830c3af3 100644 --- a/tests/contenttypes_tests/test_fields.py +++ b/tests/contenttypes_tests/test_fields.py @@ -15,13 +15,16 @@ class GenericForeignKeyTests(TestCase): class Model(models.Model): field = GenericForeignKey() - self.assertEqual(str(Model.field), "contenttypes_tests.Model.field") + field = Model._meta.get_field("field") + + self.assertEqual(str(field), "contenttypes_tests.Model.field") def test_get_content_type_no_arguments(self): + field = Answer._meta.get_field("question") with self.assertRaisesMessage( Exception, "Impossible arguments to GFK.get_content_type!" ): - Answer.question.get_content_type() + field.get_content_type() def test_get_object_cache_respects_deleted_objects(self): question = Question.objects.create(text="Who?")