From d72603bc6a16112008959c5267839f819c2bc43a Mon Sep 17 00:00:00 2001 From: Alex Burgel Date: Wed, 5 Jun 2013 17:39:14 -0400 Subject: [PATCH 1/8] Add support for collection routes to SimpleRouter --- rest_framework/decorators.py | 26 +++++++++++++++ rest_framework/routers.py | 33 +++++++++++++++++-- rest_framework/tests/test_routers.py | 48 +++++++++++++++++++++++++++- 3 files changed, 103 insertions(+), 4 deletions(-) diff --git a/rest_framework/decorators.py b/rest_framework/decorators.py index c69756a43..dacd380fb 100644 --- a/rest_framework/decorators.py +++ b/rest_framework/decorators.py @@ -113,6 +113,7 @@ def link(**kwargs): """ def decorator(func): func.bind_to_methods = ['get'] + func.collection = False func.kwargs = kwargs return func return decorator @@ -124,6 +125,31 @@ def action(methods=['post'], **kwargs): """ def decorator(func): func.bind_to_methods = methods + func.collection = False + func.kwargs = kwargs + return func + return decorator + + +def collection_link(**kwargs): + """ + Used to mark a method on a ViewSet that should be routed for GET requests. + """ + def decorator(func): + func.bind_to_methods = ['get'] + func.collection = True + func.kwargs = kwargs + return func + return decorator + + +def collection_action(methods=['post'], **kwargs): + """ + Used to mark a method on a ViewSet that should be routed for POST requests. + """ + def decorator(func): + func.bind_to_methods = methods + func.collection = True func.kwargs = kwargs return func return decorator diff --git a/rest_framework/routers.py b/rest_framework/routers.py index 930011d39..9b859a7c7 100644 --- a/rest_framework/routers.py +++ b/rest_framework/routers.py @@ -88,6 +88,17 @@ class SimpleRouter(BaseRouter): name='{basename}-list', initkwargs={'suffix': 'List'} ), + # Dynamically generated collection routes. + # Generated using @collection_action or @collection_link decorators + # on methods of the viewset. + Route( + url=r'^{prefix}/{methodname}{trailing_slash}$', + mapping={ + '{httpmethod}': '{methodname}', + }, + name='{basename}-collection-{methodnamehyphen}', + initkwargs={} + ), # Detail route. Route( url=r'^{prefix}/{lookup}{trailing_slash}$', @@ -107,7 +118,7 @@ class SimpleRouter(BaseRouter): mapping={ '{httpmethod}': '{methodname}', }, - name='{basename}-{methodnamehyphen}', + name='{basename}-dynamic-{methodnamehyphen}', initkwargs={} ), ] @@ -142,20 +153,25 @@ class SimpleRouter(BaseRouter): known_actions = flatten([route.mapping.values() for route in self.routes]) # Determine any `@action` or `@link` decorated methods on the viewset + collection_routes = [] dynamic_routes = [] for methodname in dir(viewset): attr = getattr(viewset, methodname) httpmethods = getattr(attr, 'bind_to_methods', None) + collection = getattr(attr, 'collection', False) if httpmethods: if methodname in known_actions: raise ImproperlyConfigured('Cannot use @action or @link decorator on ' 'method "%s" as it is an existing route' % methodname) httpmethods = [method.lower() for method in httpmethods] - dynamic_routes.append((httpmethods, methodname)) + if collection: + collection_routes.append((httpmethods, methodname)) + else: + dynamic_routes.append((httpmethods, methodname)) ret = [] for route in self.routes: - if route.mapping == {'{httpmethod}': '{methodname}'}: + if route.name == '{basename}-dynamic-{methodnamehyphen}': # Dynamic routes (@link or @action decorator) for httpmethods, methodname in dynamic_routes: initkwargs = route.initkwargs.copy() @@ -166,6 +182,17 @@ class SimpleRouter(BaseRouter): name=replace_methodname(route.name, methodname), initkwargs=initkwargs, )) + elif route.name == '{basename}-collection-{methodnamehyphen}': + # Dynamic routes (@collection_link or @collection_action decorator) + for httpmethods, methodname in collection_routes: + initkwargs = route.initkwargs.copy() + initkwargs.update(getattr(viewset, methodname).kwargs) + ret.append(Route( + url=replace_methodname(route.url, methodname), + mapping=dict((httpmethod, methodname) for httpmethod in httpmethods), + name=replace_methodname(route.name, methodname), + initkwargs=initkwargs, + )) else: # Standard route ret.append(route) diff --git a/rest_framework/tests/test_routers.py b/rest_framework/tests/test_routers.py index 5fcccb741..60f150d2c 100644 --- a/rest_framework/tests/test_routers.py +++ b/rest_framework/tests/test_routers.py @@ -4,7 +4,7 @@ from django.test import TestCase from django.core.exceptions import ImproperlyConfigured from rest_framework import serializers, viewsets, permissions from rest_framework.compat import include, patterns, url -from rest_framework.decorators import link, action +from rest_framework.decorators import link, action, collection_link, collection_action from rest_framework.response import Response from rest_framework.routers import SimpleRouter, DefaultRouter from rest_framework.test import APIRequestFactory @@ -214,3 +214,49 @@ class TestActionAppliedToExistingRoute(TestCase): with self.assertRaises(ImproperlyConfigured): self.router.urls + + +class StaticAndDynamicViewSet(viewsets.ViewSet): + def list(self, request, *args, **kwargs): + return Response({'method': 'list'}) + + @collection_action() + def collection_action(self, request, *args, **kwargs): + return Response({'method': 'action1'}) + + @action() + def dynamic_action(self, request, *args, **kwargs): + return Response({'method': 'action2'}) + + @collection_link() + def collection_link(self, request, *args, **kwargs): + return Response({'method': 'link1'}) + + @link() + def dynamic_link(self, request, *args, **kwargs): + return Response({'method': 'link2'}) + + +class TestStaticAndDynamicRouter(TestCase): + def setUp(self): + self.router = SimpleRouter() + + def test_link_and_action_decorator(self): + routes = self.router.get_routes(StaticAndDynamicViewSet) + decorator_routes = [r for r in routes if not (r.name.endswith('-list') or r.name.endswith('-detail'))] + # Make sure all these endpoints exist and none have been clobbered + for i, endpoint in enumerate(['collection_action', 'collection_link', 'dynamic_action', 'dynamic_link']): + route = decorator_routes[i] + # check url listing + if endpoint.startswith('collection_'): + self.assertEqual(route.url, + '^{{prefix}}/{0}{{trailing_slash}}$'.format(endpoint)) + else: + self.assertEqual(route.url, + '^{{prefix}}/{{lookup}}/{0}{{trailing_slash}}$'.format(endpoint)) + # check method to function mapping + if endpoint.endswith('action'): + method_map = 'post' + else: + method_map = 'get' + self.assertEqual(route.mapping[method_map], endpoint) From 5b11e23f6fb35834057fba35832a597ce443cc77 Mon Sep 17 00:00:00 2001 From: Alex Burgel Date: Wed, 5 Jun 2013 17:41:29 -0400 Subject: [PATCH 2/8] Add docs for collection routes --- docs/api-guide/viewsets.md | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/docs/api-guide/viewsets.md b/docs/api-guide/viewsets.md index 47e59e2b2..9fa6615ba 100644 --- a/docs/api-guide/viewsets.md +++ b/docs/api-guide/viewsets.md @@ -92,7 +92,9 @@ The default routers included with REST framework will provide routes for a stand def destroy(self, request, pk=None): pass -If you have ad-hoc methods that you need to be routed to, you can mark them as requiring routing using the `@link` or `@action` decorators. The `@link` decorator will route `GET` requests, and the `@action` decorator will route `POST` requests. +If you have ad-hoc methods that you need to be routed to, you can mark them as requiring routing using the `@collection_link`, `@collection_action`, `@link`, or `@action` decorators. The `@collection_link` and `@link` decorator will route `GET` requests, and the `@collection_action` and `@action` decorator will route `POST` requests. + +The `@link` and `@action` decorators contain `pk` in their URL pattern and are intended for methods which require a single instance. The `@collection_link` and `@collection_action` decorators are intended for methods which operate on a collection of objects. For example: @@ -121,13 +123,20 @@ For example: return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) -The `@action` and `@link` decorators can additionally take extra arguments that will be set for the routed view only. For example... + @collection_link() + def recent_users(self, request): + recent_users = User.objects.all().order('-last_login') + page = self.paginate_queryset(recent_users) + serializer = self.get_pagination_serializer(page) + return Response(serializer.data) + +The decorators can additionally take extra arguments that will be set for the routed view only. For example... @action(permission_classes=[IsAdminOrIsSelf]) def set_password(self, request, pk=None): ... -The `@action` decorator will route `POST` requests by default, but may also accept other HTTP methods, by using the `method` argument. For example: +The `@collection_action` and `@action` decorators will route `POST` requests by default, but may also accept other HTTP methods, by using the `method` argument. For example: @action(methods=['POST', 'DELETE']) def unset_password(self, request, pk=None): From 57cf8b5fa4f62f9b58912f10536a7ae5076ce54c Mon Sep 17 00:00:00 2001 From: Alex Burgel Date: Thu, 6 Jun 2013 11:51:52 -0400 Subject: [PATCH 3/8] Rework extra routes doc for better readability --- docs/api-guide/viewsets.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/api-guide/viewsets.md b/docs/api-guide/viewsets.md index 9fa6615ba..e83487fb0 100644 --- a/docs/api-guide/viewsets.md +++ b/docs/api-guide/viewsets.md @@ -92,7 +92,7 @@ The default routers included with REST framework will provide routes for a stand def destroy(self, request, pk=None): pass -If you have ad-hoc methods that you need to be routed to, you can mark them as requiring routing using the `@collection_link`, `@collection_action`, `@link`, or `@action` decorators. The `@collection_link` and `@link` decorator will route `GET` requests, and the `@collection_action` and `@action` decorator will route `POST` requests. +If you have ad-hoc methods that you need to be routed to, you can mark them as requiring routing using the `@link`, `@action`, `@collection_link`, or `@collection_action` decorators. The `@link` and `@collection_link` decorators will route `GET` requests, and the `@action` and `@collection_action` decorators will route `POST` requests. The `@link` and `@action` decorators contain `pk` in their URL pattern and are intended for methods which require a single instance. The `@collection_link` and `@collection_action` decorators are intended for methods which operate on a collection of objects. @@ -136,7 +136,7 @@ The decorators can additionally take extra arguments that will be set for the ro def set_password(self, request, pk=None): ... -The `@collection_action` and `@action` decorators will route `POST` requests by default, but may also accept other HTTP methods, by using the `method` argument. For example: +The `@action` and `@collection_action` decorators will route `POST` requests by default, but may also accept other HTTP methods, by using the `methods` argument. For example: @action(methods=['POST', 'DELETE']) def unset_password(self, request, pk=None): From 8d521c068a254cef604df1f15690275dca986778 Mon Sep 17 00:00:00 2001 From: Alex Burgel Date: Sun, 16 Jun 2013 12:43:59 -0400 Subject: [PATCH 4/8] Revert route name change and add key to Route object to identify different route types --- rest_framework/routers.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/rest_framework/routers.py b/rest_framework/routers.py index 9b859a7c7..541df4a9d 100644 --- a/rest_framework/routers.py +++ b/rest_framework/routers.py @@ -25,7 +25,7 @@ from rest_framework.reverse import reverse from rest_framework.urlpatterns import format_suffix_patterns -Route = namedtuple('Route', ['url', 'mapping', 'name', 'initkwargs']) +Route = namedtuple('Route', ['key', 'url', 'mapping', 'name', 'initkwargs']) def replace_methodname(format_string, methodname): @@ -80,6 +80,7 @@ class SimpleRouter(BaseRouter): routes = [ # List route. Route( + key='list', url=r'^{prefix}{trailing_slash}$', mapping={ 'get': 'list', @@ -92,15 +93,17 @@ class SimpleRouter(BaseRouter): # Generated using @collection_action or @collection_link decorators # on methods of the viewset. Route( + key='collection', url=r'^{prefix}/{methodname}{trailing_slash}$', mapping={ '{httpmethod}': '{methodname}', }, - name='{basename}-collection-{methodnamehyphen}', + name='{basename}-{methodnamehyphen}', initkwargs={} ), # Detail route. Route( + key='detail', url=r'^{prefix}/{lookup}{trailing_slash}$', mapping={ 'get': 'retrieve', @@ -114,11 +117,12 @@ class SimpleRouter(BaseRouter): # Dynamically generated routes. # Generated using @action or @link decorators on methods of the viewset. Route( + key='dynamic', url=r'^{prefix}/{lookup}/{methodname}{trailing_slash}$', mapping={ '{httpmethod}': '{methodname}', }, - name='{basename}-dynamic-{methodnamehyphen}', + name='{basename}-{methodnamehyphen}', initkwargs={} ), ] @@ -171,23 +175,25 @@ class SimpleRouter(BaseRouter): ret = [] for route in self.routes: - if route.name == '{basename}-dynamic-{methodnamehyphen}': + if route.key == 'dynamic': # Dynamic routes (@link or @action decorator) for httpmethods, methodname in dynamic_routes: initkwargs = route.initkwargs.copy() initkwargs.update(getattr(viewset, methodname).kwargs) ret.append(Route( + key=route.key, url=replace_methodname(route.url, methodname), mapping=dict((httpmethod, methodname) for httpmethod in httpmethods), name=replace_methodname(route.name, methodname), initkwargs=initkwargs, )) - elif route.name == '{basename}-collection-{methodnamehyphen}': + elif route.key == 'collection': # Dynamic routes (@collection_link or @collection_action decorator) for httpmethods, methodname in collection_routes: initkwargs = route.initkwargs.copy() initkwargs.update(getattr(viewset, methodname).kwargs) ret.append(Route( + key=route.key, url=replace_methodname(route.url, methodname), mapping=dict((httpmethod, methodname) for httpmethod in httpmethods), name=replace_methodname(route.name, methodname), From f02274307826ebf98998e502fecca171bb0de696 Mon Sep 17 00:00:00 2001 From: Alex Burgel Date: Sun, 16 Jun 2013 12:51:33 -0400 Subject: [PATCH 5/8] Rename router collection test case --- rest_framework/tests/test_routers.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rest_framework/tests/test_routers.py b/rest_framework/tests/test_routers.py index 60f150d2c..e0a7e292e 100644 --- a/rest_framework/tests/test_routers.py +++ b/rest_framework/tests/test_routers.py @@ -216,7 +216,7 @@ class TestActionAppliedToExistingRoute(TestCase): self.router.urls -class StaticAndDynamicViewSet(viewsets.ViewSet): +class CollectionAndDynamicViewSet(viewsets.ViewSet): def list(self, request, *args, **kwargs): return Response({'method': 'list'}) @@ -237,12 +237,12 @@ class StaticAndDynamicViewSet(viewsets.ViewSet): return Response({'method': 'link2'}) -class TestStaticAndDynamicRouter(TestCase): +class TestCollectionAndDynamicRouter(TestCase): def setUp(self): self.router = SimpleRouter() def test_link_and_action_decorator(self): - routes = self.router.get_routes(StaticAndDynamicViewSet) + routes = self.router.get_routes(CollectionAndDynamicViewSet) decorator_routes = [r for r in routes if not (r.name.endswith('-list') or r.name.endswith('-detail'))] # Make sure all these endpoints exist and none have been clobbered for i, endpoint in enumerate(['collection_action', 'collection_link', 'dynamic_action', 'dynamic_link']): From e14cbaf6961ad9c94deaf0417d8e8ce5ec96d0ac Mon Sep 17 00:00:00 2001 From: Alex Burgel Date: Sat, 13 Jul 2013 11:11:53 -0400 Subject: [PATCH 6/8] Changed collection_* decorators to list_* --- docs/api-guide/viewsets.md | 10 ++++----- rest_framework/decorators.py | 16 +++++++------- rest_framework/routers.py | 31 ++++++++++++++-------------- rest_framework/tests/test_routers.py | 24 ++++++++++----------- 4 files changed, 41 insertions(+), 40 deletions(-) diff --git a/docs/api-guide/viewsets.md b/docs/api-guide/viewsets.md index e83487fb0..6d6bb1334 100644 --- a/docs/api-guide/viewsets.md +++ b/docs/api-guide/viewsets.md @@ -92,15 +92,15 @@ The default routers included with REST framework will provide routes for a stand def destroy(self, request, pk=None): pass -If you have ad-hoc methods that you need to be routed to, you can mark them as requiring routing using the `@link`, `@action`, `@collection_link`, or `@collection_action` decorators. The `@link` and `@collection_link` decorators will route `GET` requests, and the `@action` and `@collection_action` decorators will route `POST` requests. +If you have ad-hoc methods that you need to be routed to, you can mark them as requiring routing using the `@link`, `@action`, `@list_link`, or `@list_action` decorators. The `@link` and `@list_link` decorators will route `GET` requests, and the `@action` and `@list_action` decorators will route `POST` requests. -The `@link` and `@action` decorators contain `pk` in their URL pattern and are intended for methods which require a single instance. The `@collection_link` and `@collection_action` decorators are intended for methods which operate on a collection of objects. +The `@link` and `@action` decorators contain `pk` in their URL pattern and are intended for methods which require a single instance. The `@list_link` and `@list_action` decorators are intended for methods which operate on a list of objects. For example: from django.contrib.auth.models import User from rest_framework import viewsets - from rest_framework.decorators import action + from rest_framework.decorators import action, list_link from rest_framework.response import Response from myapp.serializers import UserSerializer, PasswordSerializer @@ -123,7 +123,7 @@ For example: return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) - @collection_link() + @list_link() def recent_users(self, request): recent_users = User.objects.all().order('-last_login') page = self.paginate_queryset(recent_users) @@ -136,7 +136,7 @@ The decorators can additionally take extra arguments that will be set for the ro def set_password(self, request, pk=None): ... -The `@action` and `@collection_action` decorators will route `POST` requests by default, but may also accept other HTTP methods, by using the `methods` argument. For example: +The `@action` and `@list_action` decorators will route `POST` requests by default, but may also accept other HTTP methods, by using the `methods` argument. For example: @action(methods=['POST', 'DELETE']) def unset_password(self, request, pk=None): diff --git a/rest_framework/decorators.py b/rest_framework/decorators.py index dacd380fb..92f551db7 100644 --- a/rest_framework/decorators.py +++ b/rest_framework/decorators.py @@ -109,11 +109,11 @@ def permission_classes(permission_classes): def link(**kwargs): """ - Used to mark a method on a ViewSet that should be routed for GET requests. + Used to mark a method on a ViewSet that should be routed for detail GET requests. """ def decorator(func): func.bind_to_methods = ['get'] - func.collection = False + func.detail = True func.kwargs = kwargs return func return decorator @@ -121,35 +121,35 @@ def link(**kwargs): def action(methods=['post'], **kwargs): """ - Used to mark a method on a ViewSet that should be routed for POST requests. + Used to mark a method on a ViewSet that should be routed for detail POST requests. """ def decorator(func): func.bind_to_methods = methods - func.collection = False + func.detail = True func.kwargs = kwargs return func return decorator -def collection_link(**kwargs): +def list_link(**kwargs): """ Used to mark a method on a ViewSet that should be routed for GET requests. """ def decorator(func): func.bind_to_methods = ['get'] - func.collection = True + func.detail = False func.kwargs = kwargs return func return decorator -def collection_action(methods=['post'], **kwargs): +def list_action(methods=['post'], **kwargs): """ Used to mark a method on a ViewSet that should be routed for POST requests. """ def decorator(func): func.bind_to_methods = methods - func.collection = True + func.detail = False func.kwargs = kwargs return func return decorator diff --git a/rest_framework/routers.py b/rest_framework/routers.py index 541df4a9d..c8f711e91 100644 --- a/rest_framework/routers.py +++ b/rest_framework/routers.py @@ -89,8 +89,8 @@ class SimpleRouter(BaseRouter): name='{basename}-list', initkwargs={'suffix': 'List'} ), - # Dynamically generated collection routes. - # Generated using @collection_action or @collection_link decorators + # Dynamically generated list routes. + # Generated using @list_action or @list_link decorators # on methods of the viewset. Route( key='collection', @@ -114,7 +114,7 @@ class SimpleRouter(BaseRouter): name='{basename}-detail', initkwargs={'suffix': 'Instance'} ), - # Dynamically generated routes. + # Dynamically generated detail routes. # Generated using @action or @link decorators on methods of the viewset. Route( key='dynamic', @@ -157,27 +157,28 @@ class SimpleRouter(BaseRouter): known_actions = flatten([route.mapping.values() for route in self.routes]) # Determine any `@action` or `@link` decorated methods on the viewset - collection_routes = [] - dynamic_routes = [] + detail_routes = [] + list_routes = [] for methodname in dir(viewset): attr = getattr(viewset, methodname) httpmethods = getattr(attr, 'bind_to_methods', None) - collection = getattr(attr, 'collection', False) + detail = getattr(attr, 'detail', True) if httpmethods: if methodname in known_actions: - raise ImproperlyConfigured('Cannot use @action or @link decorator on ' - 'method "%s" as it is an existing route' % methodname) + raise ImproperlyConfigured('Cannot use @action, @link, @list_action ' + 'or @list_link decorator on method "%s" ' + 'as it is an existing route' % methodname) httpmethods = [method.lower() for method in httpmethods] - if collection: - collection_routes.append((httpmethods, methodname)) + if detail: + detail_routes.append((httpmethods, methodname)) else: - dynamic_routes.append((httpmethods, methodname)) + list_routes.append((httpmethods, methodname)) ret = [] for route in self.routes: if route.key == 'dynamic': - # Dynamic routes (@link or @action decorator) - for httpmethods, methodname in dynamic_routes: + # Dynamic detail routes (@link or @action decorator) + for httpmethods, methodname in detail_routes: initkwargs = route.initkwargs.copy() initkwargs.update(getattr(viewset, methodname).kwargs) ret.append(Route( @@ -188,8 +189,8 @@ class SimpleRouter(BaseRouter): initkwargs=initkwargs, )) elif route.key == 'collection': - # Dynamic routes (@collection_link or @collection_action decorator) - for httpmethods, methodname in collection_routes: + # Dynamic list routes (@list_link or @list_action decorator) + for httpmethods, methodname in list_routes: initkwargs = route.initkwargs.copy() initkwargs.update(getattr(viewset, methodname).kwargs) ret.append(Route( diff --git a/rest_framework/tests/test_routers.py b/rest_framework/tests/test_routers.py index e0a7e292e..393101763 100644 --- a/rest_framework/tests/test_routers.py +++ b/rest_framework/tests/test_routers.py @@ -4,7 +4,7 @@ from django.test import TestCase from django.core.exceptions import ImproperlyConfigured from rest_framework import serializers, viewsets, permissions from rest_framework.compat import include, patterns, url -from rest_framework.decorators import link, action, collection_link, collection_action +from rest_framework.decorators import link, action, list_link, list_action from rest_framework.response import Response from rest_framework.routers import SimpleRouter, DefaultRouter from rest_framework.test import APIRequestFactory @@ -216,39 +216,39 @@ class TestActionAppliedToExistingRoute(TestCase): self.router.urls -class CollectionAndDynamicViewSet(viewsets.ViewSet): +class DynamicListAndDetailViewSet(viewsets.ViewSet): def list(self, request, *args, **kwargs): return Response({'method': 'list'}) - @collection_action() - def collection_action(self, request, *args, **kwargs): + @list_action() + def list_action(self, request, *args, **kwargs): return Response({'method': 'action1'}) @action() - def dynamic_action(self, request, *args, **kwargs): + def detail_action(self, request, *args, **kwargs): return Response({'method': 'action2'}) - @collection_link() - def collection_link(self, request, *args, **kwargs): + @list_link() + def list_link(self, request, *args, **kwargs): return Response({'method': 'link1'}) @link() - def dynamic_link(self, request, *args, **kwargs): + def detail_link(self, request, *args, **kwargs): return Response({'method': 'link2'}) -class TestCollectionAndDynamicRouter(TestCase): +class TestDynamicListAndDetailRouter(TestCase): def setUp(self): self.router = SimpleRouter() def test_link_and_action_decorator(self): - routes = self.router.get_routes(CollectionAndDynamicViewSet) + routes = self.router.get_routes(DynamicListAndDetailViewSet) decorator_routes = [r for r in routes if not (r.name.endswith('-list') or r.name.endswith('-detail'))] # Make sure all these endpoints exist and none have been clobbered - for i, endpoint in enumerate(['collection_action', 'collection_link', 'dynamic_action', 'dynamic_link']): + for i, endpoint in enumerate(['list_action', 'list_link', 'detail_action', 'detail_link']): route = decorator_routes[i] # check url listing - if endpoint.startswith('collection_'): + if endpoint.startswith('list_'): self.assertEqual(route.url, '^{{prefix}}/{0}{{trailing_slash}}$'.format(endpoint)) else: From ca7ba07b4e42bd1c7c6bb8088c0c5a2c434b56ee Mon Sep 17 00:00:00 2001 From: Alex Burgel Date: Sat, 13 Jul 2013 11:12:59 -0400 Subject: [PATCH 7/8] Introduce DynamicDetailRoute and DynamicListRoute to distinguish between different route types --- rest_framework/routers.py | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/rest_framework/routers.py b/rest_framework/routers.py index c8f711e91..b8f19b66a 100644 --- a/rest_framework/routers.py +++ b/rest_framework/routers.py @@ -25,7 +25,9 @@ from rest_framework.reverse import reverse from rest_framework.urlpatterns import format_suffix_patterns -Route = namedtuple('Route', ['key', 'url', 'mapping', 'name', 'initkwargs']) +Route = namedtuple('Route', ['url', 'mapping', 'name', 'initkwargs']) +DynamicDetailRoute = namedtuple('DynamicDetailRoute', ['url', 'name', 'initkwargs']) +DynamicListRoute = namedtuple('DynamicListRoute', ['url', 'name', 'initkwargs']) def replace_methodname(format_string, methodname): @@ -80,7 +82,6 @@ class SimpleRouter(BaseRouter): routes = [ # List route. Route( - key='list', url=r'^{prefix}{trailing_slash}$', mapping={ 'get': 'list', @@ -92,18 +93,13 @@ class SimpleRouter(BaseRouter): # Dynamically generated list routes. # Generated using @list_action or @list_link decorators # on methods of the viewset. - Route( - key='collection', + DynamicListRoute( url=r'^{prefix}/{methodname}{trailing_slash}$', - mapping={ - '{httpmethod}': '{methodname}', - }, name='{basename}-{methodnamehyphen}', initkwargs={} ), # Detail route. Route( - key='detail', url=r'^{prefix}/{lookup}{trailing_slash}$', mapping={ 'get': 'retrieve', @@ -116,12 +112,8 @@ class SimpleRouter(BaseRouter): ), # Dynamically generated detail routes. # Generated using @action or @link decorators on methods of the viewset. - Route( - key='dynamic', + DynamicDetailRoute( url=r'^{prefix}/{lookup}/{methodname}{trailing_slash}$', - mapping={ - '{httpmethod}': '{methodname}', - }, name='{basename}-{methodnamehyphen}', initkwargs={} ), @@ -154,7 +146,7 @@ class SimpleRouter(BaseRouter): Returns a list of the Route namedtuple. """ - known_actions = flatten([route.mapping.values() for route in self.routes]) + known_actions = flatten([route.mapping.values() for route in self.routes if isinstance(route, Route)]) # Determine any `@action` or `@link` decorated methods on the viewset detail_routes = [] @@ -176,25 +168,23 @@ class SimpleRouter(BaseRouter): ret = [] for route in self.routes: - if route.key == 'dynamic': + if isinstance(route, DynamicDetailRoute): # Dynamic detail routes (@link or @action decorator) for httpmethods, methodname in detail_routes: initkwargs = route.initkwargs.copy() initkwargs.update(getattr(viewset, methodname).kwargs) ret.append(Route( - key=route.key, url=replace_methodname(route.url, methodname), mapping=dict((httpmethod, methodname) for httpmethod in httpmethods), name=replace_methodname(route.name, methodname), initkwargs=initkwargs, )) - elif route.key == 'collection': + elif isinstance(route, DynamicListRoute): # Dynamic list routes (@list_link or @list_action decorator) for httpmethods, methodname in list_routes: initkwargs = route.initkwargs.copy() initkwargs.update(getattr(viewset, methodname).kwargs) ret.append(Route( - key=route.key, url=replace_methodname(route.url, methodname), mapping=dict((httpmethod, methodname) for httpmethod in httpmethods), name=replace_methodname(route.name, methodname), From eaae8fb2d973769a827214e0606a7e41028d5d34 Mon Sep 17 00:00:00 2001 From: Alex Burgel Date: Mon, 15 Jul 2013 18:35:13 -0400 Subject: [PATCH 8/8] Combined link_* and action_* decorators into detail_route and list_route, marked the originals as deprecated. --- docs/api-guide/routers.md | 16 +++++----- docs/api-guide/viewsets.md | 16 +++++----- docs/tutorial/6-viewsets-and-routers.md | 8 ++--- rest_framework/decorators.py | 19 +++++++----- rest_framework/routers.py | 14 ++++----- rest_framework/tests/test_routers.py | 40 ++++++++++++------------- 6 files changed, 59 insertions(+), 54 deletions(-) diff --git a/docs/api-guide/routers.md b/docs/api-guide/routers.md index 865829057..f196dc3cd 100644 --- a/docs/api-guide/routers.md +++ b/docs/api-guide/routers.md @@ -35,12 +35,12 @@ The example above would generate the following URL patterns: * URL pattern: `^accounts/$` Name: `'account-list'` * URL pattern: `^accounts/{pk}/$` Name: `'account-detail'` -### Extra link and actions +### Registering additional routes -Any methods on the viewset decorated with `@link` or `@action` will also be routed. +Any methods on the viewset decorated with `@detail_route` or `@list_route` will also be routed. For example, a given method like this on the `UserViewSet` class: - @action(permission_classes=[IsAdminOrIsSelf]) + @detail_route(methods=['post'], permission_classes=[IsAdminOrIsSelf]) def set_password(self, request, pk=None): ... @@ -52,7 +52,7 @@ The following URL pattern would additionally be generated: ## SimpleRouter -This router includes routes for the standard set of `list`, `create`, `retrieve`, `update`, `partial_update` and `destroy` actions. The viewset can also mark additional methods to be routed, using the `@link` or `@action` decorators. +This router includes routes for the standard set of `list`, `create`, `retrieve`, `update`, `partial_update` and `destroy` actions. The viewset can also mark additional methods to be routed, using the `@detail_route` or `@list_route` decorators. @@ -62,8 +62,8 @@ This router includes routes for the standard set of `list`, `create`, `retrieve` - - + +
URL StyleHTTP MethodActionURL Name
PUTupdate
PATCHpartial_update
DELETEdestroy
{prefix}/{lookup}/{methodname}/GET@link decorated method{basename}-{methodname}
POST@action decorated method
{prefix}/{lookup}/{methodname}/GET@detail_route decorated method{basename}-{methodname}
POST@detail_route decorated method
By default the URLs created by `SimpleRouter` are appending with a trailing slash. @@ -86,8 +86,8 @@ This router is similar to `SimpleRouter` as above, but additionally includes a d PUTupdate PATCHpartial_update DELETEdestroy - {prefix}/{lookup}/{methodname}/[.format]GET@link decorated method{basename}-{methodname} - POST@action decorated method + {prefix}/{lookup}/{methodname}/[.format]GET@detail_route decorated method{basename}-{methodname} + POST@detail_route decorated method As with `SimpleRouter` the trailing slashs on the URL routes can be removed by setting the `trailing_slash` argument to `False` when instantiating the router. diff --git a/docs/api-guide/viewsets.md b/docs/api-guide/viewsets.md index 6d6bb1334..7a8d5979b 100644 --- a/docs/api-guide/viewsets.md +++ b/docs/api-guide/viewsets.md @@ -92,15 +92,15 @@ The default routers included with REST framework will provide routes for a stand def destroy(self, request, pk=None): pass -If you have ad-hoc methods that you need to be routed to, you can mark them as requiring routing using the `@link`, `@action`, `@list_link`, or `@list_action` decorators. The `@link` and `@list_link` decorators will route `GET` requests, and the `@action` and `@list_action` decorators will route `POST` requests. +If you have ad-hoc methods that you need to be routed to, you can mark them as requiring routing using the `@detail_route` or `@list_route` decorators. -The `@link` and `@action` decorators contain `pk` in their URL pattern and are intended for methods which require a single instance. The `@list_link` and `@list_action` decorators are intended for methods which operate on a list of objects. +The `@detail_route` decorator contains `pk` in its URL pattern and is intended for methods which require a single instance. The `@list_route` decorator is intended for methods which operate on a list of objects. For example: from django.contrib.auth.models import User from rest_framework import viewsets - from rest_framework.decorators import action, list_link + from rest_framework.decorators import detail_route, list_route from rest_framework.response import Response from myapp.serializers import UserSerializer, PasswordSerializer @@ -111,7 +111,7 @@ For example: queryset = User.objects.all() serializer_class = UserSerializer - @action() + @detail_route(methods=['post']) def set_password(self, request, pk=None): user = self.get_object() serializer = PasswordSerializer(data=request.DATA) @@ -123,7 +123,7 @@ For example: return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) - @list_link() + @list_route() def recent_users(self, request): recent_users = User.objects.all().order('-last_login') page = self.paginate_queryset(recent_users) @@ -132,13 +132,13 @@ For example: The decorators can additionally take extra arguments that will be set for the routed view only. For example... - @action(permission_classes=[IsAdminOrIsSelf]) + @detail_route(methods=['post'], permission_classes=[IsAdminOrIsSelf]) def set_password(self, request, pk=None): ... -The `@action` and `@list_action` decorators will route `POST` requests by default, but may also accept other HTTP methods, by using the `methods` argument. For example: +By default, the decorators will route `GET` requests, but may also accept other HTTP methods, by using the `methods` argument. For example: - @action(methods=['POST', 'DELETE']) + @detail_route(methods=['post', 'delete']) def unset_password(self, request, pk=None): ... --- diff --git a/docs/tutorial/6-viewsets-and-routers.md b/docs/tutorial/6-viewsets-and-routers.md index f16add39d..f126ba045 100644 --- a/docs/tutorial/6-viewsets-and-routers.md +++ b/docs/tutorial/6-viewsets-and-routers.md @@ -25,7 +25,7 @@ Here we've used `ReadOnlyModelViewSet` class to automatically provide the defaul Next we're going to replace the `SnippetList`, `SnippetDetail` and `SnippetHighlight` view classes. We can remove the three views, and again replace them with a single class. - from rest_framework.decorators import link + from rest_framework.decorators import detail_route class SnippetViewSet(viewsets.ModelViewSet): """ @@ -39,7 +39,7 @@ Next we're going to replace the `SnippetList`, `SnippetDetail` and `SnippetHighl permission_classes = (permissions.IsAuthenticatedOrReadOnly, IsOwnerOrReadOnly,) - @link(renderer_classes=[renderers.StaticHTMLRenderer]) + @detail_route(renderer_classes=[renderers.StaticHTMLRenderer]) def highlight(self, request, *args, **kwargs): snippet = self.get_object() return Response(snippet.highlighted) @@ -49,9 +49,9 @@ Next we're going to replace the `SnippetList`, `SnippetDetail` and `SnippetHighl This time we've used the `ModelViewSet` class in order to get the complete set of default read and write operations. -Notice that we've also used the `@link` decorator to create a custom action, named `highlight`. This decorator can be used to add any custom endpoints that don't fit into the standard `create`/`update`/`delete` style. +Notice that we've also used the `@detail_route` decorator to create a custom action, named `highlight`. This decorator can be used to add any custom endpoints that don't fit into the standard `create`/`update`/`delete` style. -Custom actions which use the `@link` decorator will respond to `GET` requests. We could have instead used the `@action` decorator if we wanted an action that responded to `POST` requests. +Custom actions which use the `@detail_route` decorator will respond to `GET` requests. We can use the `methods` argument if we wanted an action that responded to `POST` requests. ## Binding ViewSets to URLs explicitly diff --git a/rest_framework/decorators.py b/rest_framework/decorators.py index 92f551db7..1ca176f2c 100644 --- a/rest_framework/decorators.py +++ b/rest_framework/decorators.py @@ -3,13 +3,14 @@ The most important decorator in this module is `@api_view`, which is used for writing function-based views with REST framework. There are also various decorators for setting the API policies on function -based views, as well as the `@action` and `@link` decorators, which are +based views, as well as the `@detail_route` and `@list_route` decorators, which are used to annotate methods on viewsets that should be included by routers. """ from __future__ import unicode_literals from rest_framework.compat import six from rest_framework.views import APIView import types +import warnings def api_view(http_method_names): @@ -111,6 +112,8 @@ def link(**kwargs): """ Used to mark a method on a ViewSet that should be routed for detail GET requests. """ + msg = 'link is pending deprecation. Use detail_route instead.' + warnings.warn(msg, PendingDeprecationWarning, stacklevel=2) def decorator(func): func.bind_to_methods = ['get'] func.detail = True @@ -123,6 +126,8 @@ def action(methods=['post'], **kwargs): """ Used to mark a method on a ViewSet that should be routed for detail POST requests. """ + msg = 'action is pending deprecation. Use detail_route instead.' + warnings.warn(msg, PendingDeprecationWarning, stacklevel=2) def decorator(func): func.bind_to_methods = methods func.detail = True @@ -131,21 +136,21 @@ def action(methods=['post'], **kwargs): return decorator -def list_link(**kwargs): +def detail_route(methods=['get'], **kwargs): """ - Used to mark a method on a ViewSet that should be routed for GET requests. + Used to mark a method on a ViewSet that should be routed for detail requests. """ def decorator(func): - func.bind_to_methods = ['get'] - func.detail = False + func.bind_to_methods = methods + func.detail = True func.kwargs = kwargs return func return decorator -def list_action(methods=['post'], **kwargs): +def list_route(methods=['get'], **kwargs): """ - Used to mark a method on a ViewSet that should be routed for POST requests. + Used to mark a method on a ViewSet that should be routed for list requests. """ def decorator(func): func.bind_to_methods = methods diff --git a/rest_framework/routers.py b/rest_framework/routers.py index b8f19b66a..b761ba9ae 100644 --- a/rest_framework/routers.py +++ b/rest_framework/routers.py @@ -91,7 +91,7 @@ class SimpleRouter(BaseRouter): initkwargs={'suffix': 'List'} ), # Dynamically generated list routes. - # Generated using @list_action or @list_link decorators + # Generated using @list_route decorator # on methods of the viewset. DynamicListRoute( url=r'^{prefix}/{methodname}{trailing_slash}$', @@ -111,7 +111,7 @@ class SimpleRouter(BaseRouter): initkwargs={'suffix': 'Instance'} ), # Dynamically generated detail routes. - # Generated using @action or @link decorators on methods of the viewset. + # Generated using @detail_route decorator on methods of the viewset. DynamicDetailRoute( url=r'^{prefix}/{lookup}/{methodname}{trailing_slash}$', name='{basename}-{methodnamehyphen}', @@ -148,7 +148,7 @@ class SimpleRouter(BaseRouter): known_actions = flatten([route.mapping.values() for route in self.routes if isinstance(route, Route)]) - # Determine any `@action` or `@link` decorated methods on the viewset + # Determine any `@detail_route` or `@list_route` decorated methods on the viewset detail_routes = [] list_routes = [] for methodname in dir(viewset): @@ -157,8 +157,8 @@ class SimpleRouter(BaseRouter): detail = getattr(attr, 'detail', True) if httpmethods: if methodname in known_actions: - raise ImproperlyConfigured('Cannot use @action, @link, @list_action ' - 'or @list_link decorator on method "%s" ' + raise ImproperlyConfigured('Cannot use @detail_route or @list_route ' + 'decorators on method "%s" ' 'as it is an existing route' % methodname) httpmethods = [method.lower() for method in httpmethods] if detail: @@ -169,7 +169,7 @@ class SimpleRouter(BaseRouter): ret = [] for route in self.routes: if isinstance(route, DynamicDetailRoute): - # Dynamic detail routes (@link or @action decorator) + # Dynamic detail routes (@detail_route decorator) for httpmethods, methodname in detail_routes: initkwargs = route.initkwargs.copy() initkwargs.update(getattr(viewset, methodname).kwargs) @@ -180,7 +180,7 @@ class SimpleRouter(BaseRouter): initkwargs=initkwargs, )) elif isinstance(route, DynamicListRoute): - # Dynamic list routes (@list_link or @list_action decorator) + # Dynamic list routes (@list_route decorator) for httpmethods, methodname in list_routes: initkwargs = route.initkwargs.copy() initkwargs.update(getattr(viewset, methodname).kwargs) diff --git a/rest_framework/tests/test_routers.py b/rest_framework/tests/test_routers.py index 393101763..c3597e389 100644 --- a/rest_framework/tests/test_routers.py +++ b/rest_framework/tests/test_routers.py @@ -4,7 +4,7 @@ from django.test import TestCase from django.core.exceptions import ImproperlyConfigured from rest_framework import serializers, viewsets, permissions from rest_framework.compat import include, patterns, url -from rest_framework.decorators import link, action, list_link, list_action +from rest_framework.decorators import detail_route, list_route from rest_framework.response import Response from rest_framework.routers import SimpleRouter, DefaultRouter from rest_framework.test import APIRequestFactory @@ -18,23 +18,23 @@ class BasicViewSet(viewsets.ViewSet): def list(self, request, *args, **kwargs): return Response({'method': 'list'}) - @action() + @detail_route(methods=['post']) def action1(self, request, *args, **kwargs): return Response({'method': 'action1'}) - @action() + @detail_route(methods=['post']) def action2(self, request, *args, **kwargs): return Response({'method': 'action2'}) - @action(methods=['post', 'delete']) + @detail_route(methods=['post', 'delete']) def action3(self, request, *args, **kwargs): return Response({'method': 'action2'}) - @link() + @detail_route() def link1(self, request, *args, **kwargs): return Response({'method': 'link1'}) - @link() + @detail_route() def link2(self, request, *args, **kwargs): return Response({'method': 'link2'}) @@ -175,7 +175,7 @@ class TestActionKeywordArgs(TestCase): class TestViewSet(viewsets.ModelViewSet): permission_classes = [] - @action(permission_classes=[permissions.AllowAny]) + @detail_route(methods=['post'], permission_classes=[permissions.AllowAny]) def custom(self, request, *args, **kwargs): return Response({ 'permission_classes': self.permission_classes @@ -196,14 +196,14 @@ class TestActionKeywordArgs(TestCase): class TestActionAppliedToExistingRoute(TestCase): """ - Ensure `@action` decorator raises an except when applied + Ensure `@detail_route` decorator raises an except when applied to an existing route """ def test_exception_raised_when_action_applied_to_existing_route(self): class TestViewSet(viewsets.ModelViewSet): - @action() + @detail_route(methods=['post']) def retrieve(self, request, *args, **kwargs): return Response({ 'hello': 'world' @@ -220,20 +220,20 @@ class DynamicListAndDetailViewSet(viewsets.ViewSet): def list(self, request, *args, **kwargs): return Response({'method': 'list'}) - @list_action() - def list_action(self, request, *args, **kwargs): + @list_route(methods=['post']) + def list_route_post(self, request, *args, **kwargs): return Response({'method': 'action1'}) - @action() - def detail_action(self, request, *args, **kwargs): + @detail_route(methods=['post']) + def detail_route_post(self, request, *args, **kwargs): return Response({'method': 'action2'}) - @list_link() - def list_link(self, request, *args, **kwargs): + @list_route() + def list_route_get(self, request, *args, **kwargs): return Response({'method': 'link1'}) - @link() - def detail_link(self, request, *args, **kwargs): + @detail_route() + def detail_route_get(self, request, *args, **kwargs): return Response({'method': 'link2'}) @@ -241,11 +241,11 @@ class TestDynamicListAndDetailRouter(TestCase): def setUp(self): self.router = SimpleRouter() - def test_link_and_action_decorator(self): + def test_list_and_detail_route_decorators(self): routes = self.router.get_routes(DynamicListAndDetailViewSet) decorator_routes = [r for r in routes if not (r.name.endswith('-list') or r.name.endswith('-detail'))] # Make sure all these endpoints exist and none have been clobbered - for i, endpoint in enumerate(['list_action', 'list_link', 'detail_action', 'detail_link']): + for i, endpoint in enumerate(['list_route_get', 'list_route_post', 'detail_route_get', 'detail_route_post']): route = decorator_routes[i] # check url listing if endpoint.startswith('list_'): @@ -255,7 +255,7 @@ class TestDynamicListAndDetailRouter(TestCase): self.assertEqual(route.url, '^{{prefix}}/{{lookup}}/{0}{{trailing_slash}}$'.format(endpoint)) # check method to function mapping - if endpoint.endswith('action'): + if endpoint.endswith('_post'): method_map = 'post' else: method_map = 'get'