From 826a47e9ddb8e3116d81f48613be2fa15eb8eb59 Mon Sep 17 00:00:00 2001 From: Faddey Date: Thu, 21 Jan 2016 14:56:38 +0200 Subject: [PATCH 1/3] [#3859] ability to add prefixes before lookups in per-object routes --- rest_framework/routers.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/rest_framework/routers.py b/rest_framework/routers.py index 39605923d..3424aa526 100644 --- a/rest_framework/routers.py +++ b/rest_framework/routers.py @@ -218,16 +218,18 @@ class SimpleRouter(BaseRouter): https://github.com/alanjds/drf-nested-routers """ - base_regex = '(?P<{lookup_prefix}{lookup_url_kwarg}>{lookup_value})' + base_regex = '{pre_lookup_prefix}(?P<{lookup_prefix}{lookup_url_kwarg}>{lookup_value})' # Use `pk` as default field, unset set. Default regex should not # consume `.json` style suffixes and should break at '/' boundaries. lookup_field = getattr(viewset, 'lookup_field', 'pk') lookup_url_kwarg = getattr(viewset, 'lookup_url_kwarg', None) or lookup_field lookup_value = getattr(viewset, 'lookup_value_regex', '[^/.]+') + pre_lookup_prefix = getattr(viewset, 'pre_lookup_prefix', '') return base_regex.format( lookup_prefix=lookup_prefix, lookup_url_kwarg=lookup_url_kwarg, - lookup_value=lookup_value + lookup_value=lookup_value, + pre_lookup_prefix=pre_lookup_prefix ) def get_urls(self): From 725dd6ba3f561d74b0de705c26dcb73b40d2d47d Mon Sep 17 00:00:00 2001 From: Faddey Date: Thu, 21 Jan 2016 17:15:09 +0200 Subject: [PATCH 2/3] [#3859] added tests for this feature --- tests/test_routers.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/test_routers.py b/tests/test_routers.py index ae2639bf2..9e05fbddc 100644 --- a/tests/test_routers.py +++ b/tests/test_routers.py @@ -47,6 +47,12 @@ class MockViewSet(viewsets.ModelViewSet): serializer_class = None +class PrefixedLookupsViewSet(viewsets.ModelViewSet): + queryset = RouterTestModel.objects.all() + serializer_class = NoteSerializer + pre_lookup_prefix = '~' + + notes_router = SimpleRouter() notes_router.register(r'notes', NoteViewSet) @@ -56,11 +62,15 @@ kwarged_notes_router.register(r'notes', KWargedNoteViewSet) namespaced_router = DefaultRouter() namespaced_router.register(r'example', MockViewSet, base_name='example') +prefixed_lookups_router = DefaultRouter() +prefixed_lookups_router.register(r'example', PrefixedLookupsViewSet) + urlpatterns = [ url(r'^non-namespaced/', include(namespaced_router.urls)), url(r'^namespaced/', include(namespaced_router.urls, namespace='example')), url(r'^example/', include(notes_router.urls)), url(r'^example2/', include(kwarged_notes_router.urls)), + url(r'^prefixed/', include(prefixed_lookups_router.urls)), ] @@ -218,6 +228,24 @@ class TestLookupUrlKwargs(TestCase): ) +class TestPreLookupPrefixes(TestCase): + + urls = 'tests.test_routers' + + def setUp(self): + self.obj = RouterTestModel.objects.create(uuid='123', text='foo bar') + + def test_prefixed_lookup(self): + response = self.client.get('/prefixed/example/~{}/'.format(self.obj.id)) + self.assertEqual( + response.data, + { + "url": "http://testserver/example/notes/123/", + "uuid": "123", "text": "foo bar" + } + ) + + class TestTrailingSlashIncluded(TestCase): def setUp(self): class NoteViewSet(viewsets.ModelViewSet): From cc5859e2befef2723b92f5aa2e0ea8d3893138cb Mon Sep 17 00:00:00 2001 From: Faddey Date: Thu, 21 Jan 2016 18:24:37 +0200 Subject: [PATCH 3/3] added more tests that demonstrates rationale of this feature --- tests/test_routers.py | 79 +++++++++++++++++++++++++++++++++++++++---- 1 file changed, 73 insertions(+), 6 deletions(-) diff --git a/tests/test_routers.py b/tests/test_routers.py index 9e05fbddc..c5c49d13e 100644 --- a/tests/test_routers.py +++ b/tests/test_routers.py @@ -47,9 +47,19 @@ class MockViewSet(viewsets.ModelViewSet): serializer_class = None -class PrefixedLookupsViewSet(viewsets.ModelViewSet): +class SomeActionViewSet(viewsets.ModelViewSet): queryset = RouterTestModel.objects.all() serializer_class = NoteSerializer + lookup_field = 'text' + + @list_route() + def some_action(self, request, *args, **kwargs): + return Response({ + 'view_name': 'some_action' + }) + + +class PrefixedLookupViewSet(SomeActionViewSet): pre_lookup_prefix = '~' @@ -63,7 +73,17 @@ namespaced_router = DefaultRouter() namespaced_router.register(r'example', MockViewSet, base_name='example') prefixed_lookups_router = DefaultRouter() -prefixed_lookups_router.register(r'example', PrefixedLookupsViewSet) +prefixed_lookups_router.register(r'clashing', SomeActionViewSet) +prefixed_lookups_router.register(r'example', PrefixedLookupViewSet) + +reordered_router = DefaultRouter() +reordered_router.routes = [ + SimpleRouter.routes[0], + SimpleRouter.routes[2], # detail route place before dynamic list routes + SimpleRouter.routes[1], # dynamic list route place after detail route + SimpleRouter.routes[3], +] +reordered_router.register(r'clashing', SomeActionViewSet) urlpatterns = [ url(r'^non-namespaced/', include(namespaced_router.urls)), @@ -71,6 +91,7 @@ urlpatterns = [ url(r'^example/', include(notes_router.urls)), url(r'^example2/', include(kwarged_notes_router.urls)), url(r'^prefixed/', include(prefixed_lookups_router.urls)), + url(r'^reordered/', include(reordered_router.urls)), ] @@ -233,15 +254,61 @@ class TestPreLookupPrefixes(TestCase): urls = 'tests.test_routers' def setUp(self): - self.obj = RouterTestModel.objects.create(uuid='123', text='foo bar') + self.obj = RouterTestModel.objects.create( + uuid='123', + text='some_action' + ) - def test_prefixed_lookup(self): - response = self.client.get('/prefixed/example/~{}/'.format(self.obj.id)) + def test_clashing_with_routes_order_as_is(self): + """ + Demonstrates that we cannot access instance. + """ + response = self.client.get('/prefixed/clashing/some_action/') + self.assertEqual( + response.data, + { + 'view_name': 'some_action' + } + ) + + def test_clashing_with_routes_reordered(self): + """ + Demonstrates that we cannot access view. + """ + response = self.client.get('/reordered/clashing/some_action/') self.assertEqual( response.data, { "url": "http://testserver/example/notes/123/", - "uuid": "123", "text": "foo bar" + "uuid": "123", "text": "some_action" + } + ) + self.obj.delete() + response = self.client.get('/reordered/clashing/some_action/') + self.assertEqual( + response.data, + { + 'detail': 'Not found.' + } + ) + + def test_prefixed_lookup(self): + """ + Demonstrates how prefixing helps to get rid of clashing + """ + response = self.client.get('/prefixed/example/~some_action/') + self.assertEqual( + response.data, + { + "url": "http://testserver/example/notes/123/", + "uuid": "123", "text": "some_action" + } + ) + response = self.client.get('/prefixed/clashing/some_action/') + self.assertEqual( + response.data, + { + 'view_name': 'some_action' } )