Skip to content

Commit

Permalink
Fixed django#3006 -- Can now filter querysets on generic foreign key …
Browse files Browse the repository at this point in the history
…fields
  • Loading branch information
ryselis committed Mar 3, 2018
1 parent 683341d commit 12245e9
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 9 deletions.
34 changes: 31 additions & 3 deletions django/db/models/sql/query.py
Expand Up @@ -1263,6 +1263,8 @@ def _add_q(self, q_object, used_aliases, branch_negated=False,
negated=q_object.negated)
joinpromoter = JoinPromoter(q_object.connector, len(q_object.children), current_negated)
for child in q_object.children:
if not isinstance(child, Node):
child = self._prepare_lookup(child)
if isinstance(child, Node):
child_clause, needed_inner = self._add_q(
child, used_aliases, branch_negated,
Expand All @@ -1280,6 +1282,31 @@ def _add_q(self, q_object, used_aliases, branch_negated=False,
needed_inner = joinpromoter.update_join_types(self)
return target_clause, needed_inner

def _prepare_lookup(self, lookup):
"""
If the node is carrying generic foreign key lookup and the lookup is
simple (no double underscores), convert it to a Q node carrying the
double content_type and object_id lookup. Otherwise returns the original
lookup
:param lookup: a tuple (lookup, value)
:return: a transformed lookup - tuple (lookup, value) or a Q object
"""
arg, val = lookup
split_arg = arg.split(LOOKUP_SEP)
if len(split_arg) == 1: # if there's no double underscore
name = split_arg[0]
meta = self.get_meta()
try:
field = meta.get_field(name) # get content type field
except FieldDoesNotExist:
field = None
if field is not None and field.is_relation \
and not field.related_model:
# related field's get_forward_related_filter gets the correct
# lookup for a given val
return Q(**field.get_forward_related_filter(val))
return lookup

def build_filtered_relation_q(self, q_object, reuse, branch_negated=False, current_negated=False):
"""Add a FilteredRelation object to the current filter."""
connector = q_object.connector
Expand Down Expand Up @@ -1353,9 +1380,10 @@ def names_to_path(self, names, opts, allow_many=True, fail_on_missing=False):
if field.is_relation and not field.related_model:
raise FieldError(
"Field %r does not generate an automatic reverse "
"relation and therefore cannot be used for reverse "
"querying. If it is a GenericForeignKey, consider "
"adding a GenericRelation." % name
"relation for nested lookups and therefore cannot be "
"used for reverse querying for related properties. If "
"it is a GenericForeignKey, consider adding a "
"GenericRelation." % name
)
try:
model = field.model._meta.concrete_model
Expand Down
13 changes: 9 additions & 4 deletions docs/ref/contrib/contenttypes.txt
Expand Up @@ -344,14 +344,19 @@ remain set to their original values and the ``GenericForeignKey`` returns

Due to the way :class:`~django.contrib.contenttypes.fields.GenericForeignKey`
is implemented, you cannot use such fields directly with filters (``filter()``
and ``exclude()``, for example) via the database API. Because a
:class:`~django.contrib.contenttypes.fields.GenericForeignKey` isn't a
and ``exclude()``, for example) for nested lookups via the database API.
Because a :class:`~django.contrib.contenttypes.fields.GenericForeignKey` isn't a
normal field object, these examples will *not* work::

# This will fail
>>> TaggedItem.objects.filter(content_object=guido)
>>> TaggedItem.objects.filter(content_object__username='Guido')
# This will also fail
>>> TaggedItem.objects.get(content_object=guido)
>>> TaggedItem.objects.get(content_object__username='Guido')

Simple lookups, however, will work:

>>> TaggedItem.objects.filter(content_object=guido)
<TaggedItem: bdlf>

Likewise, :class:`~django.contrib.contenttypes.fields.GenericForeignKey`\s
does not appear in :class:`~django.forms.ModelForm`\s.
Expand Down
54 changes: 52 additions & 2 deletions tests/generic_relations/tests.py
Expand Up @@ -463,10 +463,10 @@ def test_update_or_create_defaults(self):
self.assertFalse(created)
self.assertEqual(tag.content_object.id, diamond.id)

def test_query_content_type(self):
def test_query_content_type_deep_lookup(self):
msg = "Field 'content_object' does not generate an automatic reverse relation"
with self.assertRaisesMessage(FieldError, msg):
TaggedItem.objects.get(content_object='')
TaggedItem.objects.get(content_object__name='Diamond')

def test_unsaved_instance_on_generic_foreign_key(self):
"""
Expand Down Expand Up @@ -580,3 +580,53 @@ def test_none_allowed(self):
# TaggedItem requires a content_type but initializing with None should
# be allowed.
TaggedItem(content_object=None)


class GenericForeignKeyFilterTest(TestCase):
def setUp(self):
self.bacon = Vegetable.objects.create(name="Bacon", is_yucky=False)
self.buckwheat = Vegetable.objects.create(name="Buckwheat", is_yucky=True)
self.object_with_generic_fk_field = TaggedItem.objects.create(tag="test_tag", content_object=self.bacon)

def test_filter_by_generic_foreign_key(self):
qs = TaggedItem.objects.filter(content_object=self.bacon)
self.assertEqual(qs.count(), 1)
self.assertEqual(qs[0], self.object_with_generic_fk_field)

def test_filter_by_generic_foreign_key_multiple_filters(self):
qs = TaggedItem.objects.filter(content_object=self.bacon, tag="test_tag")
self.assertEqual(qs.count(), 1)
self.assertEqual(qs[0], self.object_with_generic_fk_field)

def test_filter_by_generic_foreign_key_negated(self):
qs = TaggedItem.objects.exclude(content_object=self.bacon)
self.assertEqual(qs.count(), 0)

def test_filter_by_generic_foreign_key_q_obj(self):
qs = TaggedItem.objects.filter(Q(content_object=self.bacon))
self.assertEqual(qs.count(), 1)
self.assertEqual(qs[0], self.object_with_generic_fk_field)

def test_filter_by_generic_foreign_key_q_obj_complex(self):
qs = TaggedItem.objects.filter(Q(content_object=self.bacon) | ~Q(tag="test_tag"))
self.assertEqual(qs.count(), 1)
self.assertEqual(qs[0], self.object_with_generic_fk_field)

def test_get_generic_foreign_key(self):
item = TaggedItem.objects.get(content_object=self.bacon)
self.assertEqual(item, self.object_with_generic_fk_field)

def test_get_or_create_generic_foreign_key(self):
item, created = TaggedItem.objects.get_or_create(content_object=self.bacon)
self.assertEqual(item, self.object_with_generic_fk_field)

def test_get_or_create_generic_foreign_key_new_item(self):
item, created = TaggedItem.objects.get_or_create(content_object=self.buckwheat,
defaults={"tag": "new_tagged_item"})
self.assertEqual(created, True)
self.assertEqual(item.tag, "new_tagged_item")

def tearDown(self):
TaggedItem.objects.all().delete()
Vegetable.objects.all().delete()

0 comments on commit 12245e9

Please sign in to comment.