raise ImproperlyConfigured exception if basename is not unique (#8438)

* raise ImproperlyConfigured if basename already exists

* rename already_registered function; return True/False

* additional basename tests

* additional basename tests

* Update rest_framework/routers.py

Co-authored-by: David Graves <[email protected]>
Co-authored-by: Asif Saif Uddin <[email protected]>

Author: 3 people

rest_framework/routers.py

@@ -52,12 +52,24 @@ def __init__(self):
     def register(self, prefix, viewset, basename=None):
         if basename is None:
             basename = self.get_default_basename(viewset)
+
+        if self.is_already_registered(basename):
+            msg = (f'Router with basename "{basename}" is already registered. '
+                   f'Please provide a unique basename for viewset "{viewset}"')
+            raise ImproperlyConfigured(msg)
+
         self.registry.append((prefix, viewset, basename))
 
         # invalidate the urls cache
         if hasattr(self, '_urls'):
             del self._urls
 
+    def is_already_registered(self, new_basename):
+        """
+        Check if `basename` is already registered
+        """
+        return any(basename == new_basename for _prefix, _viewset, basename in self.registry)
+
     def get_default_basename(self, viewset):
         """
         If `basename` is not specified, attempt to automatically determine

tests/test_routers.py

@@ -21,6 +21,11 @@ class RouterTestModel(models.Model):
     text = models.CharField(max_length=200)
 
 
+class BasenameTestModel(models.Model):
+    uuid = models.CharField(max_length=20)
+    text = models.CharField(max_length=200)
+
+
 class NoteSerializer(serializers.HyperlinkedModelSerializer):
     url = serializers.HyperlinkedIdentityField(view_name='routertestmodel-detail', lookup_field='uuid')
 
@@ -42,6 +47,11 @@ class KWargedNoteViewSet(viewsets.ModelViewSet):
     lookup_url_kwarg = 'text'
 
 
+class BasenameViewSet(viewsets.ModelViewSet):
+    queryset = BasenameTestModel.objects.all()
+    serializer_class = None
+
+
 class MockViewSet(viewsets.ModelViewSet):
     queryset = None
     serializer_class = None
@@ -156,7 +166,7 @@ def test_multiple_action_handlers(self):
     def test_register_after_accessing_urls(self):
         self.router.register(r'notes', NoteViewSet)
         assert len(self.router.urls) == 2  # list and detail
-        self.router.register(r'notes_bis', NoteViewSet)
+        self.router.register(r'notes_bis', NoteViewSet, basename='notes_bis')
         assert len(self.router.urls) == 4
 
 
@@ -481,3 +491,103 @@ def test_basename(self):
         initkwargs = match.func.initkwargs
 
         assert initkwargs['basename'] == 'routertestmodel'
+
+
+class BasenameTestCase:
+    def test_conflicting_autogenerated_basenames(self):
+        """
+        Ensure 2 routers with the same model, and no basename specified
+        throws an ImproperlyConfigured exception
+        """
+        self.router.register(r'notes', NoteViewSet)
+
+        with pytest.raises(ImproperlyConfigured):
+            self.router.register(r'notes_kwduplicate', KWargedNoteViewSet)
+
+        with pytest.raises(ImproperlyConfigured):
+            self.router.register(r'notes_duplicate', NoteViewSet)
+
+    def test_conflicting_mixed_basenames(self):
+        """
+        Ensure 2 routers with the same model, and no basename specified on 1
+        throws an ImproperlyConfigured exception
+        """
+        self.router.register(r'notes', NoteViewSet)
+
+        with pytest.raises(ImproperlyConfigured):
+            self.router.register(r'notes_kwduplicate', KWargedNoteViewSet, basename='routertestmodel')
+
+        with pytest.raises(ImproperlyConfigured):
+            self.router.register(r'notes_duplicate', NoteViewSet, basename='routertestmodel')
+
+    def test_nonconflicting_mixed_basenames(self):
+        """
+        Ensure 2 routers with the same model, and a distinct basename
+        specified on the second router does not fail
+        """
+        self.router.register(r'notes', NoteViewSet)
+        self.router.register(r'notes_kwduplicate', KWargedNoteViewSet, basename='routertestmodel_kwduplicate')
+        self.router.register(r'notes_duplicate', NoteViewSet, basename='routertestmodel_duplicate')
+
+    def test_conflicting_specified_basename(self):
+        """
+        Ensure 2 routers with the same model, and the same basename specified
+        on both throws an ImproperlyConfigured exception
+        """
+        self.router.register(r'notes', NoteViewSet, basename='notes')
+
+        with pytest.raises(ImproperlyConfigured):
+            self.router.register(r'notes_kwduplicate', KWargedNoteViewSet, basename='notes')
+
+        with pytest.raises(ImproperlyConfigured):
+            self.router.register(r'notes_duplicate', KWargedNoteViewSet, basename='notes')
+
+    def test_nonconflicting_specified_basename(self):
+        """
+        Ensure 2 routers with the same model, and a distinct basename specified
+        on each does not throw an exception
+        """
+        self.router.register(r'notes', NoteViewSet, basename='notes')
+        self.router.register(r'notes_kwduplicate', KWargedNoteViewSet, basename='notes_kwduplicate')
+        self.router.register(r'notes_duplicate', NoteViewSet, basename='notes_duplicate')
+
+    def test_nonconflicting_specified_basename_different_models(self):
+        """
+        Ensure 2 routers with different models, and a distinct basename specified
+        on each does not throw an exception
+        """
+        self.router.register(r'notes', NoteViewSet, basename='notes')
+        self.router.register(r'notes_basename', BasenameViewSet, basename='notes_basename')
+
+    def test_conflicting_specified_basename_different_models(self):
+        """
+        Ensure 2 routers with different models, and a conflicting basename specified
+        throws an exception
+        """
+        self.router.register(r'notes', NoteViewSet)
+        with pytest.raises(ImproperlyConfigured):
+            self.router.register(r'notes_basename', BasenameViewSet, basename='routertestmodel')
+
+    def test_nonconflicting_autogenerated_basename_different_models(self):
+        """
+        Ensure 2 routers with different models, and a distinct basename specified
+        on each does not throw an exception
+        """
+        self.router.register(r'notes', NoteViewSet)
+        self.router.register(r'notes_basename', BasenameViewSet)
+
+
+class TestDuplicateBasenameSimpleRouter(BasenameTestCase, TestCase):
+    def setUp(self):
+        self.router = SimpleRouter(trailing_slash=False)
+
+
+class TestDuplicateBasenameDefaultRouter(BasenameTestCase, TestCase):
+    def setUp(self):
+        self.router = DefaultRouter()
+
+
+class TestDuplicateBasenameDefaultRouterRootViewName(BasenameTestCase, TestCase):
+    def setUp(self):
+        self.router = DefaultRouter()
+        self.router.root_view_name = 'nameable-root'