Merge pull request #953 from j4mie/prevent-duplicate-routing

Prevent dynamically routing to a method that is already routed to.

2 files changed, 36 insertions, 0 deletions

diff --git a/rest_framework/routers.py b/rest_framework/routers.py
index c222f504..930011d3 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 fe0711fa..d375f4a8 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