From c127e63c32b2fb93d1a9422943005c1f6cc5328b Mon Sep 17 00:00:00 2001 From: Jamie Matthews Date: Wed, 26 Jun 2013 23:00:42 +0100 Subject: [PATCH] Raise exception when attempting to dynamically route to a method that is already routed to. Fixes #940 --- rest_framework/routers.py | 14 ++++++++++++++ rest_framework/tests/test_routers.py | 22 ++++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/rest_framework/routers.py b/rest_framework/routers.py index c222f5046..930011d39 100644 --- a/rest_framework/routers.py +++ b/rest_framework/routers.py @@ -15,7 +15,9 @@ For example, you might have a `urls.py` that looks something like this: """ from __future__ import unicode_literals +import itertools from collections import namedtuple +from django.core.exceptions import ImproperlyConfigured from rest_framework import views from rest_framework.compat import patterns, url from rest_framework.response import Response @@ -38,6 +40,13 @@ def replace_methodname(format_string, methodname): return ret +def flatten(list_of_lists): + """ + Takes an iterable of iterables, returns a single iterable containing all items + """ + return itertools.chain(*list_of_lists) + + class BaseRouter(object): def __init__(self): self.registry = [] @@ -130,12 +139,17 @@ class SimpleRouter(BaseRouter): Returns a list of the Route namedtuple. """ + known_actions = flatten([route.mapping.values() for route in self.routes]) + # Determine any `@action` or `@link` decorated methods on the viewset dynamic_routes = [] for methodname in dir(viewset): attr = getattr(viewset, methodname) httpmethods = getattr(attr, 'bind_to_methods', None) 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)) diff --git a/rest_framework/tests/test_routers.py b/rest_framework/tests/test_routers.py index fe0711fa2..d375f4a8c 100644 --- a/rest_framework/tests/test_routers.py +++ b/rest_framework/tests/test_routers.py @@ -2,6 +2,7 @@ from __future__ import unicode_literals from django.db import models from django.test import TestCase from django.test.client import RequestFactory +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 @@ -191,3 +192,24 @@ class TestActionKeywordArgs(TestCase): response.data, {'permission_classes': [permissions.AllowAny]} ) + +class TestActionAppliedToExistingRoute(TestCase): + """ + Ensure `@action` 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() + def retrieve(self, request, *args, **kwargs): + return Response({ + 'hello': 'world' + }) + + self.router = SimpleRouter() + self.router.register(r'test', TestViewSet, base_name='test') + + with self.assertRaises(ImproperlyConfigured): + self.router.urls