From 198b30168d4e94af42e0dc7967bd3259b5c5790b Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Sat, 25 Jan 2025 00:12:17 -0500 Subject: [PATCH] Fixed #36135 -- Fixed reverse GenericRelation prefetching. The get_(local|foreign)_related_value methods of GenericRelation must be reversed because it defines (from|to)_fields and associated related_fields in the reversed order as it's effectively a reverse GenericForeignKey itself. The related value methods must also account for the fact that referenced primary key values might be stored as a string on the model defining the GenericForeignKey but as integer on the model defining the GenericRelation. This is achieved by calling the to_python method of the involved content type in get_foreign_related_value just like GenericRelatedObjectManager does. Lastly reverse many-to-one manager's prefetch_related_querysets should use set_cached_value instead of direct attribute assignment as direct assignment might are disallowed on ReverseManyToOneDescriptor descriptors. This is likely something that was missed in f5233dc (refs #32511) when the is_cached guard was added. Thanks 1xinghuan for the report. --- django/contrib/contenttypes/fields.py | 14 ++++++++++++++ django/db/models/fields/related_descriptors.py | 2 +- tests/prefetch_related/tests.py | 14 ++++++++++++++ 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/django/contrib/contenttypes/fields.py b/django/contrib/contenttypes/fields.py index 0e68c2eef0..272cb2b02c 100644 --- a/django/contrib/contenttypes/fields.py +++ b/django/contrib/contenttypes/fields.py @@ -389,6 +389,20 @@ class GenericRelation(ForeignObject): ) ] + def get_local_related_value(self, instance): + return self.get_instance_value_for_fields(instance, self.foreign_related_fields) + + def get_foreign_related_value(self, instance): + # We (possibly) need to convert object IDs to the type of the + # instances' PK in order to match up instances during prefetching. + return tuple( + foreign_field.to_python(val) + for foreign_field, val in zip( + self.foreign_related_fields, + self.get_instance_value_for_fields(instance, self.local_related_fields), + ) + ) + def _get_path_info_with_parent(self, filtered_relation): """ Return the path that joins the current model through any parent models. diff --git a/django/db/models/fields/related_descriptors.py b/django/db/models/fields/related_descriptors.py index e4698b4092..c69c998049 100644 --- a/django/db/models/fields/related_descriptors.py +++ b/django/db/models/fields/related_descriptors.py @@ -758,7 +758,7 @@ def create_reverse_many_to_one_manager(superclass, rel): for rel_obj in queryset: if not self.field.is_cached(rel_obj): instance = instances_dict[rel_obj_attr(rel_obj)] - setattr(rel_obj, self.field.name, instance) + self.field.set_cached_value(rel_obj, instance) cache_name = self.field.remote_field.cache_name return queryset, rel_obj_attr, instance_attr, False, cache_name, False diff --git a/tests/prefetch_related/tests.py b/tests/prefetch_related/tests.py index b44a34c456..d3c096574a 100644 --- a/tests/prefetch_related/tests.py +++ b/tests/prefetch_related/tests.py @@ -1254,6 +1254,20 @@ class GenericRelationTests(TestCase): ], ) + def test_reverse_generic_relation(self): + # Create two distinct bookmarks to ensure the bookmark and + # tagged item models primary are offset. + first_bookmark = Bookmark.objects.create() + second_bookmark = Bookmark.objects.create() + TaggedItem.objects.create( + content_object=first_bookmark, favorite=second_bookmark + ) + with self.assertNumQueries(2): + obj = TaggedItem.objects.prefetch_related("favorite_bookmarks").get() + with self.assertNumQueries(0): + prefetched_bookmarks = obj.favorite_bookmarks.all() + self.assertQuerySetEqual(prefetched_bookmarks, [second_bookmark]) + class MultiTableInheritanceTest(TestCase): @classmethod