Add SimplePathRouter (#6789)

* Allow usage of Django 2.x path in SimpleRouter

* Use path in Default router

* Update docs/api-guide/routers.md

Co-authored-by: Éric <[email protected]>

* Update docs/api-guide/routers.md

Co-authored-by: Éric <[email protected]>

* Add tests also for default router with path

* Use a more relevant attribute for lookup when using path converters

Co-authored-by: Asif Saif Uddin <[email protected]>
Co-authored-by: Éric <[email protected]>

Author: 3 people

docs/api-guide/routers.md

@@ -167,12 +167,23 @@ This behavior can be modified by setting the `trailing_slash` argument to `False
 
 Trailing slashes are conventional in Django, but are not used by default in some other frameworks such as Rails.  Which style you choose to use is largely a matter of preference, although some javascript frameworks may expect a particular routing style.
 
-The router will match lookup values containing any characters except slashes and period characters.  For a more restrictive (or lenient) lookup pattern, set the `lookup_value_regex` attribute on the viewset.  For example, you can limit the lookup to valid UUIDs:
+By default the URLs created by `SimpleRouter` use regular expressions. This behavior can be modified by setting the `use_regex_path` argument to `False` when instantiating the router, in this case [path converters][path-converters-topic-reference] are used. For example:
+
+    router = SimpleRouter(use_regex_path=False)
+
+**Note**: `use_regex_path=False` only works with Django 2.x or above, since this feature was introduced in 2.0.0. See [release note][simplified-routing-release-note]
+
+
+The router will match lookup values containing any characters except slashes and period characters.  For a more restrictive (or lenient) lookup pattern, set the `lookup_value_regex` attribute on the viewset or `lookup_value_converter` if using path converters.  For example, you can limit the lookup to valid UUIDs:
 
     class MyModelViewSet(mixins.RetrieveModelMixin, viewsets.GenericViewSet):
         lookup_field = 'my_model_id'
         lookup_value_regex = '[0-9a-f]{32}'
 
+    class MyPathModelViewSet(mixins.RetrieveModelMixin, viewsets.GenericViewSet):
+        lookup_field = 'my_model_uuid'
+        lookup_value_converter = 'uuid'
+
 ## DefaultRouter
 
 This router is similar to `SimpleRouter` as above, but additionally includes a default API root view, that returns a response containing hyperlinks to all the list views.  It also generates routes for optional `.json` style format suffixes.
@@ -340,3 +351,5 @@ The [`DRF-extensions` package][drf-extensions] provides [routers][drf-extensions
 [drf-extensions-customizable-endpoint-names]: https://chibisov.github.io/drf-extensions/docs/#controller-endpoint-name
 [url-namespace-docs]: https://docs.djangoproject.com/en/4.0/topics/http/urls/#url-namespaces
 [include-api-reference]: https://docs.djangoproject.com/en/4.0/ref/urls/#include
+[simplified-routing-release-note]: https://docs.djangoproject.com/en/2.0/releases/2.0/#simplified-url-routing-syntax
+[path-converters-topic-reference]: https://docs.djangoproject.com/en/2.0/topics/http/urls/#path-converters

rest_framework/routers.py

@@ -17,7 +17,7 @@
 from collections import OrderedDict, namedtuple
 
 from django.core.exceptions import ImproperlyConfigured
-from django.urls import NoReverseMatch, re_path
+from django.urls import NoReverseMatch, path, re_path
 
 from rest_framework import views
 from rest_framework.response import Response
@@ -135,8 +135,29 @@ class SimpleRouter(BaseRouter):
         ),
     ]
 
-    def __init__(self, trailing_slash=True):
+    def __init__(self, trailing_slash=True, use_regex_path=True):
         self.trailing_slash = '/' if trailing_slash else ''
+        self._use_regex = use_regex_path
+        if use_regex_path:
+            self._base_pattern = '(?P<{lookup_prefix}{lookup_url_kwarg}>{lookup_value})'
+            self._default_value_pattern = '[^/.]+'
+            self._url_conf = re_path
+        else:
+            self._base_pattern = '<{lookup_value}:{lookup_prefix}{lookup_url_kwarg}>'
+            self._default_value_pattern = 'path'
+            self._url_conf = path
+            # remove regex characters from routes
+            _routes = []
+            for route in self.routes:
+                url_param = route.url
+                if url_param[0] == '^':
+                    url_param = url_param[1:]
+                if url_param[-1] == '$':
+                    url_param = url_param[:-1]
+
+                _routes.append(route._replace(url=url_param))
+            self.routes = _routes
+
         super().__init__()
 
     def get_default_basename(self, viewset):
@@ -225,13 +246,18 @@ def get_lookup_regex(self, viewset, lookup_prefix=''):
 
         https://github.com/alanjds/drf-nested-routers
         """
-        base_regex = '(?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', '[^/.]+')
-        return base_regex.format(
+        lookup_value = None
+        if not self._use_regex:
+            # try to get a more appropriate attribute when not using regex
+            lookup_value = getattr(viewset, 'lookup_value_converter', None)
+        if lookup_value is None:
+            # fallback to legacy
+            lookup_value = getattr(viewset, 'lookup_value_regex', self._default_value_pattern)
+        return self._base_pattern.format(
             lookup_prefix=lookup_prefix,
             lookup_url_kwarg=lookup_url_kwarg,
             lookup_value=lookup_value
@@ -265,8 +291,12 @@ def get_urls(self):
                 #   controlled by project's urls.py and the router is in an app,
                 #   so a slash in the beginning will (A) cause Django to give
                 #   warnings and (B) generate URLS that will require using '//'.
-                if not prefix and regex[:2] == '^/':
-                    regex = '^' + regex[2:]
+                if not prefix:
+                    if self._url_conf is path:
+                        if regex[0] == '/':
+                            regex = regex[1:]
+                    elif regex[:2] == '^/':
+                        regex = '^' + regex[2:]
 
                 initkwargs = route.initkwargs.copy()
                 initkwargs.update({
@@ -276,7 +306,7 @@ def get_urls(self):
 
                 view = viewset.as_view(mapping, **initkwargs)
                 name = route.name.format(basename=basename)
-                ret.append(re_path(regex, view, name=name))
+                ret.append(self._url_conf(regex, view, name=name))
 
         return ret
 
@@ -351,7 +381,7 @@ def get_urls(self):
 
         if self.include_root_view:
             view = self.get_api_root_view(api_urls=urls)
-            root_url = re_path(r'^$', view, name=self.root_view_name)
+            root_url = path('', view, name=self.root_view_name)
             urls.append(root_url)
 
         if self.include_format_suffixes:

tests/test_routers.py

@@ -10,7 +10,9 @@
 from rest_framework.decorators import action
 from rest_framework.response import Response
 from rest_framework.routers import DefaultRouter, SimpleRouter
-from rest_framework.test import APIRequestFactory, URLPatternsTestCase
+from rest_framework.test import (
+    APIClient, APIRequestFactory, URLPatternsTestCase
+)
 from rest_framework.utils import json
 
 factory = APIRequestFactory()
@@ -85,9 +87,28 @@ def regex_url_path_detail(self, request, *args, **kwargs):
         return Response({'pk': pk, 'kwarg': kwarg})
 
 
+class UrlPathViewSet(viewsets.ViewSet):
+    @action(detail=False, url_path='list/<int:kwarg>')
+    def url_path_list(self, request, *args, **kwargs):
+        kwarg = self.kwargs.get('kwarg', '')
+        return Response({'kwarg': kwarg})
+
+    @action(detail=True, url_path='detail/<int:kwarg>')
+    def url_path_detail(self, request, *args, **kwargs):
+        pk = self.kwargs.get('pk', '')
+        kwarg = self.kwargs.get('kwarg', '')
+        return Response({'pk': pk, 'kwarg': kwarg})
+
+
 notes_router = SimpleRouter()
 notes_router.register(r'notes', NoteViewSet)
 
+notes_path_router = SimpleRouter(use_regex_path=False)
+notes_path_router.register('notes', NoteViewSet)
+
+notes_path_default_router = DefaultRouter(use_regex_path=False)
+notes_path_default_router.register('notes', NoteViewSet)
+
 kwarged_notes_router = SimpleRouter()
 kwarged_notes_router.register(r'notes', KWargedNoteViewSet)
 
@@ -100,6 +121,9 @@ def regex_url_path_detail(self, request, *args, **kwargs):
 regex_url_path_router = SimpleRouter()
 regex_url_path_router.register(r'', RegexUrlPathViewSet, basename='regex')
 
+url_path_router = SimpleRouter(use_regex_path=False)
+url_path_router.register('', UrlPathViewSet, basename='path')
+
 
 class BasicViewSet(viewsets.ViewSet):
     def list(self, request, *args, **kwargs):
@@ -469,6 +493,81 @@ def test_regex_url_path_detail(self):
         assert json.loads(response.content.decode()) == {'pk': pk, 'kwarg': kwarg}
 
 
+class TestUrlPath(URLPatternsTestCase, TestCase):
+    client_class = APIClient
+    urlpatterns = [
+        path('path/', include(url_path_router.urls)),
+        path('default/', include(notes_path_default_router.urls)),
+        path('example/', include(notes_path_router.urls)),
+    ]
+
+    def setUp(self):
+        RouterTestModel.objects.create(uuid='123', text='foo bar')
+        RouterTestModel.objects.create(uuid='a b', text='baz qux')
+
+    def test_create(self):
+        new_note = {
+            'uuid': 'foo',
+            'text': 'example'
+        }
+        response = self.client.post('/example/notes/', data=new_note)
+        assert response.status_code == 201
+        assert response['location'] == 'http://testserver/example/notes/foo/'
+        assert response.data == {"url": "http://testserver/example/notes/foo/", "uuid": "foo", "text": "example"}
+        assert RouterTestModel.objects.filter(uuid='foo').exists()
+
+    def test_retrieve(self):
+        for url in ('/example/notes/123/', '/default/notes/123/'):
+            with self.subTest(url=url):
+                response = self.client.get(url)
+                assert response.status_code == 200
+                # only gets example path since was the last to be registered
+                assert response.data == {"url": "http://testserver/example/notes/123/", "uuid": "123", "text": "foo bar"}
+
+    def test_list(self):
+        for url in ('/example/notes/', '/default/notes/'):
+            with self.subTest(url=url):
+                response = self.client.get(url)
+                assert response.status_code == 200
+                # only gets example path since was the last to be registered
+                assert response.data == [
+                    {"url": "http://testserver/example/notes/123/", "uuid": "123", "text": "foo bar"},
+                    {"url": "http://testserver/example/notes/a%20b/", "uuid": "a b", "text": "baz qux"},
+                ]
+
+    def test_update(self):
+        updated_note = {
+            'text': 'foo bar example'
+        }
+        response = self.client.patch('/example/notes/123/', data=updated_note)
+        assert response.status_code == 200
+        assert response.data == {"url": "http://testserver/example/notes/123/", "uuid": "123", "text": "foo bar example"}
+
+    def test_delete(self):
+        response = self.client.delete('/example/notes/123/')
+        assert response.status_code == 204
+        assert not RouterTestModel.objects.filter(uuid='123').exists()
+
+    def test_list_extra_action(self):
+        kwarg = 1234
+        response = self.client.get('/path/list/{}/'.format(kwarg))
+        assert response.status_code == 200
+        assert json.loads(response.content.decode()) == {'kwarg': kwarg}
+
+    def test_detail_extra_action(self):
+        pk = '1'
+        kwarg = 1234
+        response = self.client.get('/path/{}/detail/{}/'.format(pk, kwarg))
+        assert response.status_code == 200
+        assert json.loads(response.content.decode()) == {'pk': pk, 'kwarg': kwarg}
+
+    def test_defaultrouter_root(self):
+        response = self.client.get('/default/')
+        assert response.status_code == 200
+        # only gets example path since was the last to be registered
+        assert response.data == {"notes": "http://testserver/example/notes/"}
+
+
 class TestViewInitkwargs(URLPatternsTestCase, TestCase):
     urlpatterns = [
         path('example/', include(notes_router.urls)),