From b77f26313cddbfde20dcf2661e9bd35458c2d1bd Mon Sep 17 00:00:00 2001 From: Gabe Jackson Date: Tue, 4 Mar 2014 12:23:32 +0100 Subject: [PATCH] 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 | 2 +- django/contrib/contenttypes/fields.py | 31 ++++++++++++------------ django/db/models/options.py | 9 +++---- docs/ref/contrib/contenttypes.txt | 23 ++++++++++++++++++ docs/releases/1.7.txt | 5 ++++ tests/generic_relations/models.py | 4 +-- tests/generic_relations/tests.py | 18 ++++++++++++++ tests/generic_relations_regress/tests.py | 2 +- tests/prefetch_related/models.py | 4 +-- 9 files changed, 71 insertions(+), 27 deletions(-) diff --git a/django/contrib/admin/utils.py b/django/contrib/admin/utils.py index 91a7eb248c..0c8ac1c6fc 100644 --- a/django/contrib/admin/utils.py +++ b/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, diff --git a/django/contrib/contenttypes/fields.py b/django/contrib/contenttypes/fields.py index c68379b6ab..db45b79fb4 100644 --- a/django/contrib/contenttypes/fields.py +++ b/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), - limit_choices_to=kwargs.pop('limit_choices_to', None),) + self, to, + 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): - super(GenericRel, self).__init__(field, to, related_name, limit_choices_to) + def __init__(self, field, to, related_name=None, limit_choices_to=None, related_query_name=None): + 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) diff --git a/django/db/models/options.py b/django/db/models/options.py index 4e0307fc4f..4eec2df17c 100644 --- a/django/db/models/options.py +++ b/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] = cache[f.attname] = ( - f.related, None if f.model == self.model else f.model, True, False) + cache[f.name] = (f, 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: - if f.rel and not isinstance(f.rel.to, six.string_types) and f.generate_reverse_relation: + for f in klass._meta.local_fields + klass._meta.virtual_fields: + 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 diff --git a/docs/ref/contrib/contenttypes.txt b/docs/ref/contrib/contenttypes.txt index d01fc6baa5..bc8f538857 100644 --- a/docs/ref/contrib/contenttypes.txt +++ b/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() [, ] +.. 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') + [, ] + Just as :class:`~django.contrib.contenttypes.fields.GenericForeignKey` accepts the names of the content-type and object-ID fields as arguments, so too does diff --git a/docs/releases/1.7.txt b/docs/releases/1.7.txt index 4c0457e7c5..6c1ab36c90 100644 --- a/docs/releases/1.7.txt +++ b/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 diff --git a/tests/generic_relations/models.py b/tests/generic_relations/models.py index 436f26afb6..5d24e017a2 100644 --- a/tests/generic_relations/models.py +++ b/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): diff --git a/tests/generic_relations/tests.py b/tests/generic_relations/tests.py index ed12c5e92b..ae90ace34a 100644 --- a/tests/generic_relations/tests.py +++ b/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, ["", "", ""] + ) + 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): "" ]) + 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. diff --git a/tests/generic_relations_regress/tests.py b/tests/generic_relations_regress/tests.py index eaa8a743bd..481101f83f 100644 --- a/tests/generic_relations_regress/tests.py +++ b/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) diff --git a/tests/prefetch_related/models.py b/tests/prefetch_related/models.py index 7d300ea84c..8fec5d4b7b 100644 --- a/tests/prefetch_related/models.py +++ b/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']