From 12ba20d83c8faa51823e1471ab35fb46688ca577 Mon Sep 17 00:00:00 2001 From: Claude Paroz Date: Sun, 27 Mar 2016 22:35:57 +0200 Subject: [PATCH] Fixed #10532 -- Relaxed hard-type checking in get_object/list_or_404 shortcuts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Thanks Anssi Kääriäinen for the patch suggestion, and Tim Graham for the review. --- django/shortcuts.py | 44 ++++++++++++++++---------------- tests/get_object_or_404/tests.py | 6 ++--- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/django/shortcuts.py b/django/shortcuts.py index bd6362e8de..f4fa0600ac 100644 --- a/django/shortcuts.py +++ b/django/shortcuts.py @@ -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 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 ( Http404, HttpResponse, HttpResponsePermanentRedirect, HttpResponseRedirect, ) @@ -61,25 +58,15 @@ def redirect(to, *args, **kwargs): def _get_queryset(klass): """ - Returns a QuerySet from a Model, Manager, or QuerySet. Created to make - get_object_or_404 and get_list_or_404 more DRY. - - Raises a ValueError if klass is not a Model, Manager, or QuerySet. + Return a QuerySet or a Manager. + 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 + the job. """ - if isinstance(klass, QuerySet): - return klass - elif isinstance(klass, Manager): - manager = 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() + # If it is a model class or anything else with ._default_manager + if hasattr(klass, '_default_manager'): + return klass._default_manager.all() + return klass def get_object_or_404(klass, *args, **kwargs): @@ -96,6 +83,12 @@ def get_object_or_404(klass, *args, **kwargs): queryset = _get_queryset(klass) try: 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: 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. """ 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: raise Http404('No %s matches the given query.' % queryset.model._meta.object_name) return obj_list diff --git a/tests/get_object_or_404/tests.py b/tests/get_object_or_404/tests.py index ac4b2d4738..bf10ea3109 100644 --- a/tests/get_object_or_404/tests.py +++ b/tests/get_object_or_404/tests.py @@ -81,18 +81,18 @@ class GetObjectOr404Tests(TestCase): def test_bad_class(self): # Given an argument klass that is not a Model, Manager, or Queryset # 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): get_object_or_404(str("Article"), title__icontains="Run") class CustomClass(object): 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): get_object_or_404(CustomClass, title__icontains="Run") # 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): get_list_or_404([Article], title__icontains="Run")