Fixed #10532 -- Relaxed hard-type checking in get_object/list_or_404 shortcuts

Thanks Anssi Kääriäinen for the patch suggestion, and Tim Graham for the review.
This commit is contained in:
Claude Paroz 2016-03-27 22:35:57 +02:00
parent 4b2cf1cd27
commit 12ba20d83c
2 changed files with 25 additions and 25 deletions

View File

@ -3,9 +3,6 @@ This module collects helper functions and classes that "span" multiple levels
of MVC. In other words, these functions/classes introduce controlled coupling of MVC. In other words, these functions/classes introduce controlled coupling
for convenience's sake. for convenience's sake.
""" """
from django.db.models.base import ModelBase
from django.db.models.manager import Manager
from django.db.models.query import QuerySet
from django.http import ( from django.http import (
Http404, HttpResponse, HttpResponsePermanentRedirect, HttpResponseRedirect, Http404, HttpResponse, HttpResponsePermanentRedirect, HttpResponseRedirect,
) )
@ -61,25 +58,15 @@ def redirect(to, *args, **kwargs):
def _get_queryset(klass): def _get_queryset(klass):
""" """
Returns a QuerySet from a Model, Manager, or QuerySet. Created to make Return a QuerySet or a Manager.
get_object_or_404 and get_list_or_404 more DRY. Duck typing in action: any class with a `get()` method (for
get_object_or_404) or a `filter()` method (for get_list_or_404) might do
Raises a ValueError if klass is not a Model, Manager, or QuerySet. the job.
""" """
if isinstance(klass, QuerySet): # If it is a model class or anything else with ._default_manager
return klass if hasattr(klass, '_default_manager'):
elif isinstance(klass, Manager): return klass._default_manager.all()
manager = klass return klass
elif isinstance(klass, ModelBase):
manager = klass._default_manager
else:
if isinstance(klass, type):
klass__name = klass.__name__
else:
klass__name = klass.__class__.__name__
raise ValueError("Object is of type '%s', but must be a Django Model, "
"Manager, or QuerySet" % klass__name)
return manager.all()
def get_object_or_404(klass, *args, **kwargs): def get_object_or_404(klass, *args, **kwargs):
@ -96,6 +83,12 @@ def get_object_or_404(klass, *args, **kwargs):
queryset = _get_queryset(klass) queryset = _get_queryset(klass)
try: try:
return queryset.get(*args, **kwargs) return queryset.get(*args, **kwargs)
except AttributeError:
klass__name = klass.__name__ if isinstance(klass, type) else klass.__class__.__name__
raise ValueError(
"First argument to get_object_or_404() must be a Model, Manager, "
"or QuerySet, not '%s'." % klass__name
)
except queryset.model.DoesNotExist: except queryset.model.DoesNotExist:
raise Http404('No %s matches the given query.' % queryset.model._meta.object_name) raise Http404('No %s matches the given query.' % queryset.model._meta.object_name)
@ -109,7 +102,14 @@ def get_list_or_404(klass, *args, **kwargs):
arguments and keyword arguments are used in the filter() query. arguments and keyword arguments are used in the filter() query.
""" """
queryset = _get_queryset(klass) queryset = _get_queryset(klass)
obj_list = list(queryset.filter(*args, **kwargs)) try:
obj_list = list(queryset.filter(*args, **kwargs))
except AttributeError:
klass__name = klass.__name__ if isinstance(klass, type) else klass.__class__.__name__
raise ValueError(
"First argument to get_list_or_404() must be a Model, Manager, or "
"QuerySet, not '%s'." % klass__name
)
if not obj_list: if not obj_list:
raise Http404('No %s matches the given query.' % queryset.model._meta.object_name) raise Http404('No %s matches the given query.' % queryset.model._meta.object_name)
return obj_list return obj_list

View File

@ -81,18 +81,18 @@ class GetObjectOr404Tests(TestCase):
def test_bad_class(self): def test_bad_class(self):
# Given an argument klass that is not a Model, Manager, or Queryset # Given an argument klass that is not a Model, Manager, or Queryset
# raises a helpful ValueError message # raises a helpful ValueError message
msg = "Object is of type 'str', but must be a Django Model, Manager, or QuerySet" msg = "First argument to get_object_or_404() must be a Model, Manager, or QuerySet, not 'str'."
with self.assertRaisesMessage(ValueError, msg): with self.assertRaisesMessage(ValueError, msg):
get_object_or_404(str("Article"), title__icontains="Run") get_object_or_404(str("Article"), title__icontains="Run")
class CustomClass(object): class CustomClass(object):
pass pass
msg = "Object is of type 'CustomClass', but must be a Django Model, Manager, or QuerySet" msg = "First argument to get_object_or_404() must be a Model, Manager, or QuerySet, not 'CustomClass'."
with self.assertRaisesMessage(ValueError, msg): with self.assertRaisesMessage(ValueError, msg):
get_object_or_404(CustomClass, title__icontains="Run") get_object_or_404(CustomClass, title__icontains="Run")
# Works for lists too # Works for lists too
msg = "Object is of type 'list', but must be a Django Model, Manager, or QuerySet" msg = "First argument to get_list_or_404() must be a Model, Manager, or QuerySet, not 'list'."
with self.assertRaisesMessage(ValueError, msg): with self.assertRaisesMessage(ValueError, msg):
get_list_or_404([Article], title__icontains="Run") get_list_or_404([Article], title__icontains="Run")