From dcbe789fd89b5e23226e1730dbe73388590a15cd Mon Sep 17 00:00:00 2001 From: Ludwig Kraatz Date: Wed, 14 Nov 2012 14:54:58 +0100 Subject: [PATCH 1/6] initial redirect View handles logic --- rest_framework/views.py | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/rest_framework/views.py b/rest_framework/views.py index 1afbd6974..1d3363c74 100644 --- a/rest_framework/views.py +++ b/rest_framework/views.py @@ -13,6 +13,7 @@ from rest_framework.compat import View, apply_markdown from rest_framework.response import Response from rest_framework.request import Request from rest_framework.settings import api_settings +from rest_framework.reverse import reverse def _remove_trailing_string(content, trailing): @@ -372,3 +373,36 @@ class APIView(View): a less useful default implementation. """ return Response(self.metadata(request), status=status.HTTP_200_OK) + +class RedirectAPIView(APIView): + """ + A view that provides a redirect to a different resource endpoint + """ + permanent = True + view_name = None + + def get_redirect_url(self, request, *args, **kwargs): + """ + Return the URL redirect to. Keyword arguments from the + URL pattern match generating the redirect request + are provided as kwargs to this method. + """ + try: + return reverse(self.view_name, args=args, kwargs=kwargs, request=request) + except: + return None + + def get(self, request, *args, **kwargs): + url = self.get_redirect_url(request, *args, **kwargs) + if url: + if self.permanent: + return Response(status=status.HTTP_301_MOVED_PERMANENTLY) + else: + return Response(status=status.HTTP_302_FOUND) + else: + logger.warning('Gone: %s', self.request.path, + extra={ + 'status_code': 410, + 'request': self.request + }) + return Response(status=status.HTTP_410_GONE) \ No newline at end of file From 372e58c471730b6cb0f4569d3c560dbd0782d3c3 Mon Sep 17 00:00:00 2001 From: Ludwig Kraatz Date: Wed, 14 Nov 2012 15:21:39 +0100 Subject: [PATCH 2/6] Added tests and improved RedirectView * Fixed wrong Header handling in Response * Fixed Old tests to work again * added Redirect Test * Location Header is now in Redirect Response --- rest_framework/response.py | 5 ++++- rest_framework/tests/throttling.py | 2 +- rest_framework/tests/views.py | 28 ++++++++++++++++++++++++++-- rest_framework/views.py | 27 ++++++++++++++++++++++----- 4 files changed, 53 insertions(+), 9 deletions(-) diff --git a/rest_framework/response.py b/rest_framework/response.py index 0de01204d..53b5e15f6 100644 --- a/rest_framework/response.py +++ b/rest_framework/response.py @@ -20,9 +20,12 @@ class Response(SimpleTemplateResponse): """ super(Response, self).__init__(None, status=status) self.data = data - self.headers = headers and headers[:] or [] self.template_name = template_name self.exception = exception + + if headers: + for name,value in headers.iteritems(): + self[name] = value @property def rendered_content(self): diff --git a/rest_framework/tests/throttling.py b/rest_framework/tests/throttling.py index 0b94c25ba..4b98b9414 100644 --- a/rest_framework/tests/throttling.py +++ b/rest_framework/tests/throttling.py @@ -106,7 +106,7 @@ class ThrottlingTests(TestCase): if expect is not None: self.assertEquals(response['X-Throttle-Wait-Seconds'], expect) else: - self.assertFalse('X-Throttle-Wait-Seconds' in response.headers) + self.assertFalse('X-Throttle-Wait-Seconds' in response) def test_seconds_fields(self): """ diff --git a/rest_framework/tests/views.py b/rest_framework/tests/views.py index 43365e07a..13a70010d 100644 --- a/rest_framework/tests/views.py +++ b/rest_framework/tests/views.py @@ -1,11 +1,12 @@ import copy +from django.conf.urls.defaults import patterns, url from django.test import TestCase from django.test.client import RequestFactory from rest_framework import status from rest_framework.decorators import api_view from rest_framework.response import Response from rest_framework.settings import api_settings -from rest_framework.views import APIView +from rest_framework.views import APIView, RedirectAPIView factory = RequestFactory() @@ -17,6 +18,16 @@ class BasicView(APIView): def post(self, request, *args, **kwargs): return Response({'method': 'POST', 'data': request.DATA}) +class RedirectBasicView(RedirectAPIView): + permanent = False + view_name = 'basic-view' + + +urlpatterns = patterns('', + url(r'^basic/$', BasicView.as_view(), name='basic-view'), + url(r'^redirect_to_basic/$', RedirectBasicView.as_view(), name='old-basic-view'), +) + @api_view(['GET', 'POST', 'PUT']) def basic_view(request): @@ -39,7 +50,7 @@ def sanitise_json_error(error_dict): return ret -class ClassBasedViewIntegrationTests(TestCase): +class ClassBasedViewIntegrationTests(TestCase): def setUp(self): self.view = BasicView.as_view() @@ -95,3 +106,16 @@ class FunctionBasedViewIntegrationTests(TestCase): } self.assertEquals(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertEquals(sanitise_json_error(response.data), expected) + +class RedirectViewTests(TestCase): + urls = 'rest_framework.tests.views' + + def setUp(self): + self.view = RedirectBasicView.as_view() + + def test_redirect(self): + request = factory.get('/redirect_to_basic', content_type='application/json') + response = self.view(request) + + self.assertEquals(response.status_code, status.HTTP_302_FOUND) + self.assertEquals(response['Location'], 'http://testserver/basic/') \ No newline at end of file diff --git a/rest_framework/views.py b/rest_framework/views.py index 1d3363c74..2d6647386 100644 --- a/rest_framework/views.py +++ b/rest_framework/views.py @@ -380,10 +380,11 @@ class RedirectAPIView(APIView): """ permanent = True view_name = None + #http_method_names = ['get', 'post', 'head', 'options', 'delete', 'put'] def get_redirect_url(self, request, *args, **kwargs): """ - Return the URL redirect to. Keyword arguments from the + Return the URL redirect to. Arguments and Keyword arguments from the URL pattern match generating the redirect request are provided as kwargs to this method. """ @@ -395,14 +396,30 @@ class RedirectAPIView(APIView): def get(self, request, *args, **kwargs): url = self.get_redirect_url(request, *args, **kwargs) if url: + headers = {'Location': url} if self.permanent: - return Response(status=status.HTTP_301_MOVED_PERMANENTLY) + return Response(status=status.HTTP_301_MOVED_PERMANENTLY, headers=headers) else: - return Response(status=status.HTTP_302_FOUND) + return Response(status=status.HTTP_302_FOUND, headers=headers) else: logger.warning('Gone: %s', self.request.path, extra={ - 'status_code': 410, + 'status_code': status.HTTP_410_GONE, 'request': self.request }) - return Response(status=status.HTTP_410_GONE) \ No newline at end of file + return Response(status=status.HTTP_410_GONE) + + def head(self, request, *args, **kwargs): + return self.get(request, *args, **kwargs) + + def post(self, request, *args, **kwargs): + return self.get(request, *args, **kwargs) + + def options(self, request, *args, **kwargs): + return self.get(request, *args, **kwargs) + + def delete(self, request, *args, **kwargs): + return self.get(request, *args, **kwargs) + + def put(self, request, *args, **kwargs): + return self.get(request, *args, **kwargs) From 723241511c6c50ccae425c0b34bf93f05801c577 Mon Sep 17 00:00:00 2001 From: Ludwig Kraatz Date: Wed, 14 Nov 2012 20:24:09 +0100 Subject: [PATCH 3/6] added query string test is lost when redirected --- rest_framework/tests/views.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rest_framework/tests/views.py b/rest_framework/tests/views.py index 13a70010d..0f8cd7d1b 100644 --- a/rest_framework/tests/views.py +++ b/rest_framework/tests/views.py @@ -114,8 +114,8 @@ class RedirectViewTests(TestCase): self.view = RedirectBasicView.as_view() def test_redirect(self): - request = factory.get('/redirect_to_basic', content_type='application/json') + request = factory.get('/redirect_to_basic/?foo=bar', content_type='application/json') response = self.view(request) self.assertEquals(response.status_code, status.HTTP_302_FOUND) - self.assertEquals(response['Location'], 'http://testserver/basic/') \ No newline at end of file + self.assertEquals(response['Location'], 'http://testserver/basic/?foo=bar') \ No newline at end of file From 238fb79e6dfdbd8e4b83c8d6b835749e57528f36 Mon Sep 17 00:00:00 2001 From: Ludwig Kraatz Date: Thu, 15 Nov 2012 21:07:43 +0100 Subject: [PATCH 4/6] fixed query string loss --- rest_framework/views.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/rest_framework/views.py b/rest_framework/views.py index 2d6647386..cb733fd39 100644 --- a/rest_framework/views.py +++ b/rest_framework/views.py @@ -380,7 +380,6 @@ class RedirectAPIView(APIView): """ permanent = True view_name = None - #http_method_names = ['get', 'post', 'head', 'options', 'delete', 'put'] def get_redirect_url(self, request, *args, **kwargs): """ @@ -389,9 +388,14 @@ class RedirectAPIView(APIView): are provided as kwargs to this method. """ try: - return reverse(self.view_name, args=args, kwargs=kwargs, request=request) + url = reverse(self.view_name, args=args, kwargs=kwargs, request=request) except: return None + + query_string = self.request.META.get('QUERY_STRING', '') + if query_string: + url = '%(url)s?%(query_string)s' % {'url': url, 'query_string': query_string} + return url def get(self, request, *args, **kwargs): url = self.get_redirect_url(request, *args, **kwargs) @@ -402,11 +406,6 @@ class RedirectAPIView(APIView): else: return Response(status=status.HTTP_302_FOUND, headers=headers) else: - logger.warning('Gone: %s', self.request.path, - extra={ - 'status_code': status.HTTP_410_GONE, - 'request': self.request - }) return Response(status=status.HTTP_410_GONE) def head(self, request, *args, **kwargs): From b98bff30f930b3a75dae920cc1c9d4051b1df3da Mon Sep 17 00:00:00 2001 From: Ludwig Kraatz Date: Sat, 8 Dec 2012 11:54:36 +0100 Subject: [PATCH 5/6] changed implementation - Mixin Style changed naming of class attributes to be more explicit --- rest_framework/generics.py | 20 ++++++++++++ rest_framework/mixins.py | 40 ++++++++++++++++++++++++ rest_framework/tests/generics.py | 38 ++++++++++++++++++++++- rest_framework/tests/views.py | 28 ++--------------- rest_framework/views.py | 52 +------------------------------- 5 files changed, 100 insertions(+), 78 deletions(-) diff --git a/rest_framework/generics.py b/rest_framework/generics.py index dd8dfcf8d..69ae53ae9 100644 --- a/rest_framework/generics.py +++ b/rest_framework/generics.py @@ -213,3 +213,23 @@ class RetrieveUpdateDestroyAPIView(mixins.RetrieveModelMixin, def delete(self, request, *args, **kwargs): return self.destroy(request, *args, **kwargs) + +class RedirectAPIView(mixins.RedirectMixin, + GenericAPIView): + """ + Redirect View used to redirect requests to a different/moved endpoint + """ + def get(self, request, *args, **kwargs): + return self.redirect(request, *args, **kwargs) + + def post(self, request, *args, **kwargs): + return self.redirect(request, *args, **kwargs) + + def options(self, request, *args, **kwargs): + return self.redirect(request, *args, **kwargs) + + def delete(self, request, *args, **kwargs): + return self.redirect(request, *args, **kwargs) + + def put(self, request, *args, **kwargs): + return self.redirect(request, *args, **kwargs) \ No newline at end of file diff --git a/rest_framework/mixins.py b/rest_framework/mixins.py index 1edcfa5c9..69ba1b63b 100644 --- a/rest_framework/mixins.py +++ b/rest_framework/mixins.py @@ -7,6 +7,7 @@ which allows mixin classes to be composed in interesting ways. from django.http import Http404 from rest_framework import status from rest_framework.response import Response +from rest_framework.reverse import reverse class CreateModelMixin(object): @@ -123,3 +124,42 @@ class DestroyModelMixin(object): self.object = self.get_object() self.object.delete() return Response(status=status.HTTP_204_NO_CONTENT) + +class RedirectMixin(object): + """ + A Mixin that provides a redirect method, + redirecting to a different resource endpoint + """ + redirect_permanent = True + redirect_with_query_string = True + redirect_to_view_name = None + redirect_to_url = None + + def get_redirect_url(self, request, *args, **kwargs): + """ + Returning where to redirect to. + To url, view-name or nowhere + """ + if self.redirect_to_url: + url = self.redirect_to_url % kwargs + else: + try: + url = reverse(self.redirect_view_name, args=args, kwargs=kwargs, request=request) + except: + return None + + query_string = self.request.META.get('QUERY_STRING', '') + if query_string and self.redirect_with_query_string: + url = '%(url)s?%(query_string)s' % {'url': url, 'query_string': query_string} + return url + + def redirect(self, request, *args, **kwargs): + url = self.get_redirect_url(request, *args, **kwargs) + if url: + headers = {'Location': url} + if self.redirect_permanent: + return Response(status=status.HTTP_301_MOVED_PERMANENTLY, headers=headers) + else: + return Response(status=status.HTTP_302_FOUND, headers=headers) + else: + return Response(status=status.HTTP_410_GONE) diff --git a/rest_framework/tests/generics.py b/rest_framework/tests/generics.py index a8279ef2b..760ed0047 100644 --- a/rest_framework/tests/generics.py +++ b/rest_framework/tests/generics.py @@ -1,4 +1,5 @@ from django.test import TestCase +from django.conf.urls.defaults import patterns, url from django.test.client import RequestFactory from django.utils import simplejson as json from rest_framework import generics, serializers, status @@ -14,13 +15,19 @@ class RootView(generics.ListCreateAPIView): """ model = BasicModel - class InstanceView(generics.RetrieveUpdateDestroyAPIView): """ Example description for OPTIONS. """ model = BasicModel +class RedirectToRootView(generics.RedirectAPIView): + redirect_permanent = False + redirect_view_name = 'root-view' + +class RedirectToURL(generics.RedirectAPIView): + redirect_to_url = 'http://foo.bar' + redirect_with_query_string = False class SlugSerializer(serializers.ModelSerializer): slug = serializers.Field() # read only @@ -38,6 +45,10 @@ class SlugBasedInstanceView(InstanceView): serializer_class = SlugSerializer +urlpatterns = patterns('', + url(r'^root/$', RootView.as_view(), name='root-view'), +) + class TestRootView(TestCase): def setUp(self): """ @@ -301,3 +312,28 @@ class TestCreateModelWithAutoNowAddField(TestCase): self.assertEquals(response.status_code, status.HTTP_201_CREATED) created = self.objects.get(id=1) self.assertEquals(created.content, 'foobar') + +class RedirectViewTests(TestCase): + urls = 'rest_framework.tests.generics' + + def test_redirect(self): + request = factory.get('/redirect_to_root/?foo=bar') + response = RedirectToRootView.as_view()(request) + + self.assertEquals(response.status_code, status.HTTP_302_FOUND) + self.assertEquals(response['Location'], 'http://testserver/root/?foo=bar') + + def test_redirect_to_nowhere(self): + request = factory.get('/redirect_to_nowhere/') + response = generics.RedirectAPIView.as_view()(request) + + self.assertEquals(response.status_code, status.HTTP_410_GONE) + + def test_redirect_to_url(self): + request = factory.get('/redirect_to_url/?foo=bar') + response = RedirectToURL.as_view()(request) + + self.assertEquals(response.status_code, status.HTTP_301_MOVED_PERMANENTLY) + self.assertEquals(response['Location'], 'http://foo.bar') + + \ No newline at end of file diff --git a/rest_framework/tests/views.py b/rest_framework/tests/views.py index 72eacd62e..91c76a533 100644 --- a/rest_framework/tests/views.py +++ b/rest_framework/tests/views.py @@ -1,12 +1,11 @@ import copy -from django.conf.urls.defaults import patterns, url from django.test import TestCase from django.test.client import RequestFactory from rest_framework import status from rest_framework.decorators import api_view from rest_framework.response import Response from rest_framework.settings import api_settings -from rest_framework.views import APIView, RedirectAPIView +from rest_framework.views import APIView factory = RequestFactory() @@ -18,16 +17,6 @@ class BasicView(APIView): def post(self, request, *args, **kwargs): return Response({'method': 'POST', 'data': request.DATA}) -class RedirectBasicView(RedirectAPIView): - permanent = False - view_name = 'basic-view' - - -urlpatterns = patterns('', - url(r'^basic/$', BasicView.as_view(), name='basic-view'), - url(r'^redirect_to_basic/$', RedirectBasicView.as_view(), name='old-basic-view'), -) - @api_view(['GET', 'POST', 'PUT']) def basic_view(request): @@ -105,17 +94,4 @@ class FunctionBasedViewIntegrationTests(TestCase): 'detail': u'JSON parse error - No JSON object could be decoded' } self.assertEquals(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertEquals(sanitise_json_error(response.data), expected) - -class RedirectViewTests(TestCase): - urls = 'rest_framework.tests.views' - - def setUp(self): - self.view = RedirectBasicView.as_view() - - def test_redirect(self): - request = factory.get('/redirect_to_basic/?foo=bar', content_type='application/json') - response = self.view(request) - - self.assertEquals(response.status_code, status.HTTP_302_FOUND) - self.assertEquals(response['Location'], 'http://testserver/basic/?foo=bar') \ No newline at end of file + self.assertEquals(sanitise_json_error(response.data), expected) \ No newline at end of file diff --git a/rest_framework/views.py b/rest_framework/views.py index ea07ed7b9..09b515b72 100644 --- a/rest_framework/views.py +++ b/rest_framework/views.py @@ -13,7 +13,6 @@ from rest_framework.compat import View, apply_markdown from rest_framework.response import Response from rest_framework.request import Request from rest_framework.settings import api_settings -from rest_framework.reverse import reverse def _remove_trailing_string(content, trailing): @@ -372,53 +371,4 @@ class APIView(View): We may as well implement this as Django will otherwise provide a less useful default implementation. """ - return Response(self.metadata(request), status=status.HTTP_200_OK) - -class RedirectAPIView(APIView): - """ - A view that provides a redirect to a different resource endpoint - """ - permanent = True - view_name = None - - def get_redirect_url(self, request, *args, **kwargs): - """ - Return the URL redirect to. Arguments and Keyword arguments from the - URL pattern match generating the redirect request - are provided as kwargs to this method. - """ - try: - url = reverse(self.view_name, args=args, kwargs=kwargs, request=request) - except: - return None - - query_string = self.request.META.get('QUERY_STRING', '') - if query_string: - url = '%(url)s?%(query_string)s' % {'url': url, 'query_string': query_string} - return url - - def get(self, request, *args, **kwargs): - url = self.get_redirect_url(request, *args, **kwargs) - if url: - headers = {'Location': url} - if self.permanent: - return Response(status=status.HTTP_301_MOVED_PERMANENTLY, headers=headers) - else: - return Response(status=status.HTTP_302_FOUND, headers=headers) - else: - return Response(status=status.HTTP_410_GONE) - - def head(self, request, *args, **kwargs): - return self.get(request, *args, **kwargs) - - def post(self, request, *args, **kwargs): - return self.get(request, *args, **kwargs) - - def options(self, request, *args, **kwargs): - return self.get(request, *args, **kwargs) - - def delete(self, request, *args, **kwargs): - return self.get(request, *args, **kwargs) - - def put(self, request, *args, **kwargs): - return self.get(request, *args, **kwargs) + return Response(self.metadata(request), status=status.HTTP_200_OK) \ No newline at end of file From b572ede48ec3e5e4eb8dd07dcabc81eb255ebde6 Mon Sep 17 00:00:00 2001 From: Ludwig Kraatz Date: Sat, 8 Dec 2012 12:02:29 +0100 Subject: [PATCH 6/6] fixed some newlines to keep views.py untouched --- rest_framework/tests/views.py | 2 +- rest_framework/views.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/rest_framework/tests/views.py b/rest_framework/tests/views.py index 91c76a533..43365e07a 100644 --- a/rest_framework/tests/views.py +++ b/rest_framework/tests/views.py @@ -94,4 +94,4 @@ class FunctionBasedViewIntegrationTests(TestCase): 'detail': u'JSON parse error - No JSON object could be decoded' } self.assertEquals(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertEquals(sanitise_json_error(response.data), expected) \ No newline at end of file + self.assertEquals(sanitise_json_error(response.data), expected) diff --git a/rest_framework/views.py b/rest_framework/views.py index 09b515b72..10bdd5a53 100644 --- a/rest_framework/views.py +++ b/rest_framework/views.py @@ -371,4 +371,4 @@ class APIView(View): We may as well implement this as Django will otherwise provide a less useful default implementation. """ - return Response(self.metadata(request), status=status.HTTP_200_OK) \ No newline at end of file + return Response(self.metadata(request), status=status.HTTP_200_OK)