Fixed #22207 -- Added support for GenericRelation reverse lookups

GenericRelation now supports an optional related_query_name argument.
Setting related_query_name adds a relation from the related object back to
the content type for filtering, ordering and other query operations.

Thanks to Loic Bistuer for spotting a couple of important issues in
his review.

django/contrib/admin/utils.py

@@ -168,7 +168,7 @@ class NestedObjects(Collector):
 
     def collect(self, objs, source=None, source_attr=None, **kwargs):
         for obj in objs:
-            if source_attr:
+            if source_attr and not source_attr.endswith('+'):
                 related_name = source_attr % {
                     'class': source._meta.model_name,
                     'app_label': source._meta.app_label,

django/contrib/contenttypes/fields.py

@@ -6,7 +6,7 @@ from django.core import checks
 from django.core.exceptions import ObjectDoesNotExist
 from django.db import connection
 from django.db import models, router, transaction, DEFAULT_DB_ALIAS
-from django.db.models import signals, FieldDoesNotExist
+from django.db.models import signals, FieldDoesNotExist, DO_NOTHING
 from django.db.models.base import ModelBase
 from django.db.models.fields.related import ForeignObject, ForeignObjectRel
 from django.db.models.related import PathInfo
@@ -243,8 +243,10 @@ class GenericRelation(ForeignObject):
     def __init__(self, to, **kwargs):
         kwargs['verbose_name'] = kwargs.get('verbose_name', None)
         kwargs['rel'] = GenericRel(
-            self, to, related_name=kwargs.pop('related_name', None),
+            self, to,
-            limit_choices_to=kwargs.pop('limit_choices_to', None),)
+            related_query_name=kwargs.pop('related_query_name', None),
+            limit_choices_to=kwargs.pop('limit_choices_to', None),
+        )
         # Override content-type/object-id field names on the related class
         self.object_id_field_name = kwargs.pop("object_id_field", "object_id")
         self.content_type_field_name = kwargs.pop("content_type_field", "content_type")
@@ -300,11 +302,16 @@ class GenericRelation(ForeignObject):
         return [(self.rel.to._meta.get_field_by_name(self.object_id_field_name)[0],
                  self.model._meta.pk)]
 
-    def get_reverse_path_info(self):
+    def get_path_info(self):
         opts = self.rel.to._meta
         target = opts.get_field_by_name(self.object_id_field_name)[0]
         return [PathInfo(self.model._meta, opts, (target,), self.rel, True, False)]
 
+    def get_reverse_path_info(self):
+        opts = self.model._meta
+        from_opts = self.rel.to._meta
+        return [PathInfo(from_opts, opts, (opts.pk,), self, not self.unique, False)]
+
     def get_choices_default(self):
         return super(GenericRelation, self).get_choices(include_blank=False)
 
@@ -312,13 +319,6 @@ class GenericRelation(ForeignObject):
         qs = getattr(obj, self.name).all()
         return smart_text([instance._get_pk_val() for instance in qs])
 
-    def get_joining_columns(self, reverse_join=False):
-        if not reverse_join:
-            # This error message is meant for the user, and from user
-            # perspective this is a reverse join along the GenericRelation.
-            raise ValueError('Joining in reverse direction not allowed.')
-        return super(GenericRelation, self).get_joining_columns(reverse_join)
-
     def contribute_to_class(self, cls, name):
         super(GenericRelation, self).contribute_to_class(cls, name, virtual_only=True)
         # Save a reference to which model this class is on for future use
@@ -326,9 +326,6 @@ class GenericRelation(ForeignObject):
         # Add the descriptor for the relation
         setattr(cls, self.name, ReverseGenericRelatedObjectsDescriptor(self, self.for_concrete_model))
 
-    def contribute_to_related_class(self, cls, related):
-        pass
-
     def set_attributes_from_rel(self):
         pass
 
@@ -527,5 +524,7 @@ def create_generic_related_manager(superclass):
 
 
 class GenericRel(ForeignObjectRel):
-    def __init__(self, field, to, related_name=None, limit_choices_to=None):
+    def __init__(self, field, to, related_name=None, limit_choices_to=None, related_query_name=None):
-        super(GenericRel, self).__init__(field, to, related_name, limit_choices_to)
+        super(GenericRel, self).__init__(field=field, to=to, related_name=related_query_name or '+',
+                                         limit_choices_to=limit_choices_to, on_delete=DO_NOTHING,
+                                         related_query_name=related_query_name)

django/db/models/options.py

@@ -449,9 +449,7 @@ class Options(object):
         for f, model in self.get_fields_with_model():
             cache[f.name] = cache[f.attname] = (f, model, True, False)
         for f in self.virtual_fields:
-            if hasattr(f, 'related'):
+            cache[f.name] = (f, None if f.model == self.model else f.model, True, False)
-                cache[f.name] = cache[f.attname] = (
-                    f.related, None if f.model == self.model else f.model, True, False)
         if apps.ready:
             self._name_map = cache
         return cache
@@ -530,8 +528,9 @@ class Options(object):
         proxy_cache = cache.copy()
         for klass in self.apps.get_models(include_auto_created=True):
             if not klass._meta.swapped:
-                for f in klass._meta.local_fields:
+                for f in klass._meta.local_fields + klass._meta.virtual_fields:
-                    if f.rel and not isinstance(f.rel.to, six.string_types) and f.generate_reverse_relation:
+                    if (hasattr(f, 'rel') and f.rel and not isinstance(f.rel.to, six.string_types)
+                            and f.generate_reverse_relation):
                         if self == f.rel.to._meta:
                             cache[f.related] = None
                             proxy_cache[f.related] = None

docs/ref/contrib/contenttypes.txt

@@ -373,6 +373,15 @@ Reverse generic relations
 
         This class used to be defined in ``django.contrib.contenttypes.generic``.
 
+    .. attribute:: related_query_name
+
+        .. versionadded:: 1.7
+
+        The relation on the related object back to this object doesn't exist by
+        default. Setting ``related_query_name`` creates a relation from the
+        related object back to this one. This allows querying and filtering
+        from the related object.
+
 If you know which models you'll be using most often, you can also add
 a "reverse" generic relationship to enable an additional API. For example::
 
@@ -392,6 +401,20 @@ be used to retrieve their associated ``TaggedItems``::
     >>> b.tags.all()
     [<TaggedItem: django>, <TaggedItem: python>]
 
+.. versionadded:: 1.7
+
+Defining :class:`~django.contrib.contenttypes.fields.GenericRelation` with
+``related_query_name`` set allows querying from the related object::
+
+    tags = GenericRelation(TaggedItem, related_query_name='bookmarks')
+
+This enables filtering, ordering, and other query operations on ``Bookmark``
+from ``TaggedItem``::
+
+    >>> # Get all tags belonging to books containing `django` in the url
+    >>> TaggedItem.objects.filter(bookmarks__url__contains='django')
+    [<TaggedItem: django>, <TaggedItem: python>]
+
 Just as :class:`~django.contrib.contenttypes.fields.GenericForeignKey`
 accepts the names of the content-type and object-ID fields as
 arguments, so too does

docs/releases/1.7.txt

@@ -1165,6 +1165,11 @@ Miscellaneous
 * The ``shortcut`` view in ``django.contrib.contenttypes.views`` now supports
   protocol-relative URLs (e.g. ``//example.com``).
 
+* :class:`~django.contrib.contenttypes.fields.GenericRelation` now supports an
+  optional ``related_query_name`` argument. Setting ``related_query_name`` adds
+  a relation from the related object back to the content type for filtering,
+  ordering and other query operations.
+
 .. _deprecated-features-1.7:
 
 Features deprecated in 1.7

tests/generic_relations/models.py

@@ -68,7 +68,7 @@ class Animal(models.Model):
     common_name = models.CharField(max_length=150)
     latin_name = models.CharField(max_length=150)
 
-    tags = GenericRelation(TaggedItem)
+    tags = GenericRelation(TaggedItem, related_query_name='animal')
     comparisons = GenericRelation(Comparison,
                                   object_id_field="object_id1",
                                   content_type_field="content_type1")
@@ -116,7 +116,7 @@ class Rock(Mineral):
 
 class ManualPK(models.Model):
     id = models.IntegerField(primary_key=True)
-    tags = GenericRelation(TaggedItem)
+    tags = GenericRelation(TaggedItem, related_query_name='manualpk')
 
 
 class ForProxyModelModel(models.Model):

tests/generic_relations/tests.py

@@ -3,6 +3,7 @@ from __future__ import unicode_literals
 from django import forms
 from django.contrib.contenttypes.forms import generic_inlineformset_factory
 from django.contrib.contenttypes.models import ContentType
+from django.core.exceptions import FieldError
 from django.test import TestCase
 from django.utils import six
 
@@ -42,6 +43,17 @@ class GenericRelationsTests(TestCase):
         # You can easily access the content object like a foreign key.
         t = TaggedItem.objects.get(tag="salty")
         self.assertEqual(t.content_object, bacon)
+        qs = TaggedItem.objects.filter(animal__isnull=False).order_by('animal__common_name', 'tag')
+        self.assertQuerysetEqual(
+            qs, ["<TaggedItem: hairy>", "<TaggedItem: yellow>", "<TaggedItem: fatty>"]
+        )
+        mpk = ManualPK.objects.create(id=1)
+        mpk.tags.create(tag='mpk')
+        from django.db.models import Q
+        qs = TaggedItem.objects.filter(Q(animal__isnull=False) | Q(manualpk__id=1)).order_by('tag')
+        self.assertQuerysetEqual(
+            qs, ["fatty", "hairy", "mpk", "yellow"], lambda x: x.tag)
+        mpk.delete()
 
         # Recall that the Mineral class doesn't have an explicit GenericRelation
         # defined. That's OK, because you can create TaggedItems explicitly.
@@ -151,6 +163,12 @@ class GenericRelationsTests(TestCase):
             "<Animal: Platypus>"
         ])
 
+    def test_generic_relation_related_name_default(self):
+        # Test that GenericRelation by default isn't usable from
+        # the reverse side.
+        with self.assertRaises(FieldError):
+            TaggedItem.objects.filter(vegetable__isnull=True)
+
     def test_multiple_gfk(self):
         # Simple tests for multiple GenericForeignKeys
         # only uses one model, since the above tests should be sufficient.

tests/generic_relations_regress/tests.py

@@ -245,5 +245,5 @@ class GenericRelationTests(TestCase):
         form = GenericRelationForm({'links': None})
         self.assertTrue(form.is_valid())
         form.save()
-        links = HasLinkThing._meta.get_field_by_name('links')[0].field
+        links = HasLinkThing._meta.get_field_by_name('links')[0]
         self.assertEqual(links.save_form_data_calls, 1)

tests/prefetch_related/models.py

@@ -145,11 +145,11 @@ class TaggedItem(models.Model):
 
 class Bookmark(models.Model):
     url = models.URLField()
-    tags = GenericRelation(TaggedItem, related_name='bookmarks')
+    tags = GenericRelation(TaggedItem, related_query_name='bookmarks')
     favorite_tags = GenericRelation(TaggedItem,
                                     content_type_field='favorite_ct',
                                     object_id_field='favorite_fkey',
-                                    related_name='favorite_bookmarks')
+                                    related_query_name='favorite_bookmarks')
 
     class Meta:
         ordering = ['id']