From b09ef28959fe63351f0dd24564b7d2d344b44fa3 Mon Sep 17 00:00:00 2001 From: Brandon Cazander Date: Sat, 24 Jan 2015 01:37:23 -0800 Subject: [PATCH 1/5] Add failing test for request.version AttributeError in BrowsableAPI. --- tests/browsable_api/auth_urls.py | 9 +++++++- tests/browsable_api/test_browsable_api.py | 10 +++++++++ tests/browsable_api/views.py | 27 +++++++++++++++++++++++ 3 files changed, 45 insertions(+), 1 deletion(-) diff --git a/tests/browsable_api/auth_urls.py b/tests/browsable_api/auth_urls.py index bce7dcf91..098a99acc 100644 --- a/tests/browsable_api/auth_urls.py +++ b/tests/browsable_api/auth_urls.py @@ -1,10 +1,17 @@ from __future__ import unicode_literals from django.conf.urls import patterns, url, include +from rest_framework import routers -from .views import MockView +from .views import MockView, FooViewSet, BarViewSet + +router = routers.SimpleRouter() +router.register(r'foo', FooViewSet) +router.register(r'bar', BarViewSet) urlpatterns = patterns( '', (r'^$', MockView.as_view()), + url(r'^', include(router.urls)), + url(r'^bar/(?P\d+)/$', BarViewSet, name='bar-list'), url(r'^auth/', include('rest_framework.urls', namespace='rest_framework')), ) diff --git a/tests/browsable_api/test_browsable_api.py b/tests/browsable_api/test_browsable_api.py index 5f2647838..31907f84e 100644 --- a/tests/browsable_api/test_browsable_api.py +++ b/tests/browsable_api/test_browsable_api.py @@ -3,6 +3,7 @@ from django.contrib.auth.models import User from django.test import TestCase from rest_framework.test import APIClient +from .models import Foo, Bar class DropdownWithAuthTests(TestCase): @@ -16,6 +17,8 @@ class DropdownWithAuthTests(TestCase): self.email = 'lennon@thebeatles.com' self.password = 'password' self.user = User.objects.create_user(self.username, self.email, self.password) + foo = Foo.objects.create(name='Foo') + Bar.objects.create(foo=foo) def tearDown(self): self.client.logout() @@ -25,6 +28,13 @@ class DropdownWithAuthTests(TestCase): response = self.client.get('/') self.assertContains(response, 'john') + def test_bug_2455_clone_request(self): + self.client.login(username=self.username, password=self.password) + json_response = self.client.get('/foo/1/?format=json') + self.assertEqual(json_response.status_code, 200) + browsable_api_response = self.client.get('/foo/1/') + self.assertEqual(browsable_api_response.status_code, 200) + def test_logout_shown_when_logged_in(self): self.client.login(username=self.username, password=self.password) response = self.client.get('/') diff --git a/tests/browsable_api/views.py b/tests/browsable_api/views.py index 000f4e804..f06f7c40a 100644 --- a/tests/browsable_api/views.py +++ b/tests/browsable_api/views.py @@ -1,9 +1,14 @@ from __future__ import unicode_literals from rest_framework.views import APIView +from rest_framework.viewsets import ModelViewSet from rest_framework import authentication from rest_framework import renderers from rest_framework.response import Response +from rest_framework.renderers import BrowsableAPIRenderer, JSONRenderer +from rest_framework.versioning import NamespaceVersioning +from .models import Foo, Bar +from .serializers import FooSerializer, BarSerializer class MockView(APIView): @@ -13,3 +18,25 @@ class MockView(APIView): def get(self, request): return Response({'a': 1, 'b': 2, 'c': 3}) + + +class SerializerClassMixin(object): + def get_serializer_class(self): + # Get base name of serializer + self.request.version + return self.serializer_class + + +class FooViewSet(SerializerClassMixin, ModelViewSet): + versioning_class = NamespaceVersioning + model = Foo + queryset = Foo.objects.all() + serializer_class = FooSerializer + renderer_classes = (BrowsableAPIRenderer, JSONRenderer) + + +class BarViewSet(SerializerClassMixin, ModelViewSet): + model = Bar + queryset = Bar.objects.all() + serializer_class = BarSerializer + renderer_classes = (BrowsableAPIRenderer, ) From 0ee2edc0a14c4d14b8aa6e4b63ccbd0c2cc78024 Mon Sep 17 00:00:00 2001 From: Brandon Cazander Date: Sat, 24 Jan 2015 01:44:09 -0800 Subject: [PATCH 2/5] Add missed files for test. --- tests/browsable_api/models.py | 9 +++++++++ tests/browsable_api/serializers.py | 14 ++++++++++++++ 2 files changed, 23 insertions(+) create mode 100644 tests/browsable_api/models.py create mode 100644 tests/browsable_api/serializers.py diff --git a/tests/browsable_api/models.py b/tests/browsable_api/models.py new file mode 100644 index 000000000..05c6c23b4 --- /dev/null +++ b/tests/browsable_api/models.py @@ -0,0 +1,9 @@ +from django.db import models + + +class Foo(models.Model): + name = models.CharField(max_length=30) + + +class Bar(models.Model): + foo = models.ForeignKey("Foo", editable=False) diff --git a/tests/browsable_api/serializers.py b/tests/browsable_api/serializers.py new file mode 100644 index 000000000..e83645404 --- /dev/null +++ b/tests/browsable_api/serializers.py @@ -0,0 +1,14 @@ +from .models import Foo, Bar +from rest_framework.serializers import HyperlinkedModelSerializer, HyperlinkedIdentityField + + +class FooSerializer(HyperlinkedModelSerializer): + bar = HyperlinkedIdentityField(view_name='bar-list') + + class Meta: + model = Foo + + +class BarSerializer(HyperlinkedModelSerializer): + class Meta: + model = Bar From ed04725822d5dc9a90c9c6e5c14d85083ae6ff28 Mon Sep 17 00:00:00 2001 From: Brandon Cazander Date: Sat, 24 Jan 2015 01:44:40 -0800 Subject: [PATCH 3/5] Use enhanced request when cloning requests for checking permissions on other methods. Fixes #2455 --- rest_framework/request.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rest_framework/request.py b/rest_framework/request.py index cfbbdeccd..d5b56ada1 100644 --- a/rest_framework/request.py +++ b/rest_framework/request.py @@ -86,7 +86,7 @@ def clone_request(request, method): Internal helper method to clone a request, replacing with a different HTTP method. Used for checking permissions against other methods. """ - ret = Request(request=request._request, + ret = Request(request=request, parsers=request.parsers, authenticators=request.authenticators, negotiator=request.negotiator, From 6c083b12a1162bf8e0f51e6c52ff13a1bd621cf2 Mon Sep 17 00:00:00 2001 From: Brandon Cazander Date: Sat, 24 Jan 2015 11:00:36 -0800 Subject: [PATCH 4/5] Streamline test for #2455 --- tests/browsable_api/auth_urls.py | 8 +------ tests/browsable_api/models.py | 9 -------- tests/browsable_api/serializers.py | 14 ------------ tests/browsable_api/test_browsable_api.py | 10 --------- tests/browsable_api/views.py | 27 ----------------------- tests/test_metadata.py | 15 +++++++++++++ 6 files changed, 16 insertions(+), 67 deletions(-) delete mode 100644 tests/browsable_api/models.py delete mode 100644 tests/browsable_api/serializers.py diff --git a/tests/browsable_api/auth_urls.py b/tests/browsable_api/auth_urls.py index 098a99acc..97bc10360 100644 --- a/tests/browsable_api/auth_urls.py +++ b/tests/browsable_api/auth_urls.py @@ -1,17 +1,11 @@ from __future__ import unicode_literals from django.conf.urls import patterns, url, include -from rest_framework import routers -from .views import MockView, FooViewSet, BarViewSet +from .views import MockView -router = routers.SimpleRouter() -router.register(r'foo', FooViewSet) -router.register(r'bar', BarViewSet) urlpatterns = patterns( '', (r'^$', MockView.as_view()), - url(r'^', include(router.urls)), - url(r'^bar/(?P\d+)/$', BarViewSet, name='bar-list'), url(r'^auth/', include('rest_framework.urls', namespace='rest_framework')), ) diff --git a/tests/browsable_api/models.py b/tests/browsable_api/models.py deleted file mode 100644 index 05c6c23b4..000000000 --- a/tests/browsable_api/models.py +++ /dev/null @@ -1,9 +0,0 @@ -from django.db import models - - -class Foo(models.Model): - name = models.CharField(max_length=30) - - -class Bar(models.Model): - foo = models.ForeignKey("Foo", editable=False) diff --git a/tests/browsable_api/serializers.py b/tests/browsable_api/serializers.py deleted file mode 100644 index e83645404..000000000 --- a/tests/browsable_api/serializers.py +++ /dev/null @@ -1,14 +0,0 @@ -from .models import Foo, Bar -from rest_framework.serializers import HyperlinkedModelSerializer, HyperlinkedIdentityField - - -class FooSerializer(HyperlinkedModelSerializer): - bar = HyperlinkedIdentityField(view_name='bar-list') - - class Meta: - model = Foo - - -class BarSerializer(HyperlinkedModelSerializer): - class Meta: - model = Bar diff --git a/tests/browsable_api/test_browsable_api.py b/tests/browsable_api/test_browsable_api.py index 31907f84e..5f2647838 100644 --- a/tests/browsable_api/test_browsable_api.py +++ b/tests/browsable_api/test_browsable_api.py @@ -3,7 +3,6 @@ from django.contrib.auth.models import User from django.test import TestCase from rest_framework.test import APIClient -from .models import Foo, Bar class DropdownWithAuthTests(TestCase): @@ -17,8 +16,6 @@ class DropdownWithAuthTests(TestCase): self.email = 'lennon@thebeatles.com' self.password = 'password' self.user = User.objects.create_user(self.username, self.email, self.password) - foo = Foo.objects.create(name='Foo') - Bar.objects.create(foo=foo) def tearDown(self): self.client.logout() @@ -28,13 +25,6 @@ class DropdownWithAuthTests(TestCase): response = self.client.get('/') self.assertContains(response, 'john') - def test_bug_2455_clone_request(self): - self.client.login(username=self.username, password=self.password) - json_response = self.client.get('/foo/1/?format=json') - self.assertEqual(json_response.status_code, 200) - browsable_api_response = self.client.get('/foo/1/') - self.assertEqual(browsable_api_response.status_code, 200) - def test_logout_shown_when_logged_in(self): self.client.login(username=self.username, password=self.password) response = self.client.get('/') diff --git a/tests/browsable_api/views.py b/tests/browsable_api/views.py index f06f7c40a..000f4e804 100644 --- a/tests/browsable_api/views.py +++ b/tests/browsable_api/views.py @@ -1,14 +1,9 @@ from __future__ import unicode_literals from rest_framework.views import APIView -from rest_framework.viewsets import ModelViewSet from rest_framework import authentication from rest_framework import renderers from rest_framework.response import Response -from rest_framework.renderers import BrowsableAPIRenderer, JSONRenderer -from rest_framework.versioning import NamespaceVersioning -from .models import Foo, Bar -from .serializers import FooSerializer, BarSerializer class MockView(APIView): @@ -18,25 +13,3 @@ class MockView(APIView): def get(self, request): return Response({'a': 1, 'b': 2, 'c': 3}) - - -class SerializerClassMixin(object): - def get_serializer_class(self): - # Get base name of serializer - self.request.version - return self.serializer_class - - -class FooViewSet(SerializerClassMixin, ModelViewSet): - versioning_class = NamespaceVersioning - model = Foo - queryset = Foo.objects.all() - serializer_class = FooSerializer - renderer_classes = (BrowsableAPIRenderer, JSONRenderer) - - -class BarViewSet(SerializerClassMixin, ModelViewSet): - model = Bar - queryset = Bar.objects.all() - serializer_class = BarSerializer - renderer_classes = (BrowsableAPIRenderer, ) diff --git a/tests/test_metadata.py b/tests/test_metadata.py index 972a896a4..bdc84edf1 100644 --- a/tests/test_metadata.py +++ b/tests/test_metadata.py @@ -1,6 +1,7 @@ from __future__ import unicode_literals from rest_framework import exceptions, serializers, status, views from rest_framework.request import Request +from rest_framework.renderers import BrowsableAPIRenderer from rest_framework.test import APIRequestFactory request = Request(APIRequestFactory().options('/')) @@ -168,3 +169,17 @@ class TestMetadata: response = view(request=request) assert response.status_code == status.HTTP_200_OK assert list(response.data['actions'].keys()) == ['POST'] + + def test_bug_2455_clone_request(self): + class ExampleView(views.APIView): + renderer_classes = (BrowsableAPIRenderer,) + + def post(self, request): + pass + + def get_serializer(self): + assert hasattr(self.request, 'version') + return serializers.Serializer() + + view = ExampleView.as_view() + view(request=request) From bf58c1265ddf06deb435d049fae01ed324a310e0 Mon Sep 17 00:00:00 2001 From: Brandon Cazander Date: Mon, 26 Jan 2015 22:56:57 -0800 Subject: [PATCH 5/5] Set a version attribute on cloned requests if necessary. --- rest_framework/request.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/rest_framework/request.py b/rest_framework/request.py index d5b56ada1..ce2fcb476 100644 --- a/rest_framework/request.py +++ b/rest_framework/request.py @@ -86,7 +86,7 @@ def clone_request(request, method): Internal helper method to clone a request, replacing with a different HTTP method. Used for checking permissions against other methods. """ - ret = Request(request=request, + ret = Request(request=request._request, parsers=request.parsers, authenticators=request.authenticators, negotiator=request.negotiator, @@ -107,6 +107,8 @@ def clone_request(request, method): ret.accepted_renderer = request.accepted_renderer if hasattr(request, 'accepted_media_type'): ret.accepted_media_type = request.accepted_media_type + if hasattr(request, 'version'): + ret.version = request.version return ret