aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--rest_framework/generics.py13
-rw-r--r--rest_framework/relations.py35
-rw-r--r--rest_framework/tests/generics.py10
3 files changed, 40 insertions, 18 deletions
diff --git a/rest_framework/generics.py b/rest_framework/generics.py
index afcb8a9f..9ccc7898 100644
--- a/rest_framework/generics.py
+++ b/rest_framework/generics.py
@@ -6,7 +6,7 @@ from __future__ import unicode_literals
from django.core.exceptions import ImproperlyConfigured, PermissionDenied
from django.core.paginator import Paginator, InvalidPage
from django.http import Http404
-from django.shortcuts import get_object_or_404
+from django.shortcuts import get_object_or_404 as _get_object_or_404
from django.utils.translation import ugettext as _
from rest_framework import views, mixins, exceptions
from rest_framework.request import clone_request
@@ -14,6 +14,17 @@ from rest_framework.settings import api_settings
import warnings
+def get_object_or_404(queryset, **filter_kwargs):
+ """
+ Same as Django's standard shortcut, but make sure to raise 404
+ if the filter_kwargs don't match the required types.
+ """
+ try:
+ return _get_object_or_404(queryset, **filter_kwargs)
+ except (TypeError, ValueError):
+ raise Http404
+
+
class GenericAPIView(views.APIView):
"""
Base class for all other generic views.
diff --git a/rest_framework/relations.py b/rest_framework/relations.py
index c4271e33..41707efc 100644
--- a/rest_framework/relations.py
+++ b/rest_framework/relations.py
@@ -518,8 +518,6 @@ class HyperlinkedIdentityField(Field):
request = self.context.get('request', None)
format = self.context.get('format', None)
view_name = self.view_name or self.parent.opts.view_name
- lookup_field = getattr(obj, self.lookup_field)
- kwargs = {self.lookup_field: lookup_field}
if request is None:
warnings.warn("Using `HyperlinkedIdentityField` without including the "
@@ -539,27 +537,30 @@ class HyperlinkedIdentityField(Field):
if format and self.format and self.format != format:
format = self.format
+ lookup_field = getattr(obj, self.lookup_field)
+ kwargs = {self.lookup_field: lookup_field}
try:
return reverse(view_name, kwargs=kwargs, request=request, format=format)
except NoReverseMatch:
pass
- slug = getattr(obj, self.slug_field, None)
-
- if not slug:
- raise Exception('Could not resolve URL for field using view name "%s"' % view_name)
-
- kwargs = {self.slug_url_kwarg: slug}
- try:
- return reverse(view_name, kwargs=kwargs, request=request, format=format)
- except NoReverseMatch:
- pass
+ if self.pk_url_kwarg != 'pk':
+ # Only try pk lookup if it has been explicitly set.
+ # Otherwise, the default `lookup_field = 'pk'` has us covered.
+ kwargs = {self.pk_url_kwarg: obj.pk}
+ try:
+ return reverse(view_name, kwargs=kwargs, request=request, format=format)
+ except NoReverseMatch:
+ pass
- kwargs = {self.pk_url_kwarg: obj.pk, self.slug_url_kwarg: slug}
- try:
- return reverse(view_name, kwargs=kwargs, request=request, format=format)
- except NoReverseMatch:
- pass
+ slug = getattr(obj, self.slug_field, None)
+ if slug:
+ # Only use slug lookup if a slug field exists on the model
+ kwargs = {self.slug_url_kwarg: slug}
+ try:
+ return reverse(view_name, kwargs=kwargs, request=request, format=format)
+ except NoReverseMatch:
+ pass
raise Exception('Could not resolve URL for field using view name "%s"' % view_name)
diff --git a/rest_framework/tests/generics.py b/rest_framework/tests/generics.py
index f091d0db..37734195 100644
--- a/rest_framework/tests/generics.py
+++ b/rest_framework/tests/generics.py
@@ -279,6 +279,16 @@ class TestInstanceView(TestCase):
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(response.data, expected)
+ def test_get_instance_view_incorrect_arg(self):
+ """
+ GET requests with an incorrect pk type, should raise 404, not 500.
+ Regression test for #890.
+ """
+ request = factory.get('/a')
+ with self.assertNumQueries(0):
+ response = self.view(request, pk='a').render()
+ self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
+
def test_put_cannot_set_id(self):
"""
PUT requests to create a new object should not be able to set the id.