From 6ed6a18a033ba78de9418f5b56b80f8b3d3aaf11 Mon Sep 17 00:00:00 2001 From: Claude Paroz Date: Wed, 12 Dec 2012 20:34:59 +0100 Subject: [PATCH] Fixed #19432 -- Provided better error message for get_object_or_404 Thanks Kit Sunde for the report and Brian Holdefehr for the initial patch. --- django/shortcuts/__init__.py | 10 ++++++++- tests/modeltests/get_object_or_404/tests.py | 25 +++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/django/shortcuts/__init__.py b/django/shortcuts/__init__.py index 0746e843a3..9f896347a4 100644 --- a/django/shortcuts/__init__.py +++ b/django/shortcuts/__init__.py @@ -7,6 +7,7 @@ for convenience's sake. from django.template import loader, RequestContext from django.http import HttpResponse, Http404 from django.http import HttpResponseRedirect, HttpResponsePermanentRedirect +from django.db.models.base import ModelBase from django.db.models.manager import Manager from django.db.models.query import QuerySet from django.core import urlresolvers @@ -72,13 +73,20 @@ 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. """ if isinstance(klass, QuerySet): return klass elif isinstance(klass, Manager): manager = klass - else: + elif isinstance(klass, ModelBase): manager = klass._default_manager + else: + klass__name = klass.__name__ if isinstance(klass, type) \ + else 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): diff --git a/tests/modeltests/get_object_or_404/tests.py b/tests/modeltests/get_object_or_404/tests.py index 280720fd15..3b234c6cd3 100644 --- a/tests/modeltests/get_object_or_404/tests.py +++ b/tests/modeltests/get_object_or_404/tests.py @@ -80,3 +80,28 @@ class GetObjectOr404Tests(TestCase): get_list_or_404(Article.objects.all(), title__icontains="Run"), [article] ) + + def test_bad_class(self): + # Given an argument klass that is not a Model, Manager, or Queryset + # raises a helpful ValueError message + self.assertRaisesMessage(ValueError, + "Object is of type 'str', but must be a Django Model, Manager, " + "or QuerySet", + get_object_or_404, "Article", title__icontains="Run" + ) + + class CustomClass(object): + pass + + self.assertRaisesMessage(ValueError, + "Object is of type 'CustomClass', but must be a Django Model, " + "Manager, or QuerySet", + get_object_or_404, CustomClass, title__icontains="Run" + ) + + # Works for lists too + self.assertRaisesMessage(ValueError, + "Object is of type 'list', but must be a Django Model, Manager, " + "or QuerySet", + get_list_or_404, [Article], title__icontains="Run" + )