From 2d19f233ab36805264ec6d4655a0105d8b37f989 Mon Sep 17 00:00:00 2001 From: Devid Date: Thu, 12 Jan 2023 09:16:48 +0100 Subject: [PATCH] Add SimplePathRouter (#6789) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Allow usage of Django 2.x path in SimpleRouter * Use path in Default router * Update docs/api-guide/routers.md Co-authored-by: Éric * Update docs/api-guide/routers.md Co-authored-by: Éric * 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 Co-authored-by: Éric --- docs/api-guide/routers.md | 15 +++++- rest_framework/routers.py | 48 ++++++++++++++---- tests/test_routers.py | 101 +++++++++++++++++++++++++++++++++++++- 3 files changed, 153 insertions(+), 11 deletions(-) diff --git a/docs/api-guide/routers.md b/docs/api-guide/routers.md index 70c05fdde..91ef0b96e 100644 --- a/docs/api-guide/routers.md +++ b/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 diff --git a/rest_framework/routers.py b/rest_framework/routers.py index 10df22aa3..722fc50a6 100644 --- a/rest_framework/routers.py +++ b/rest_framework/routers.py @@ -17,7 +17,7 @@ import itertools 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 @@ class SimpleRouter(BaseRouter): 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 @@ class SimpleRouter(BaseRouter): # 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 @@ class SimpleRouter(BaseRouter): 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 @@ class DefaultRouter(SimpleRouter): 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: diff --git a/tests/test_routers.py b/tests/test_routers.py index 6b006242a..b4bde5c31 100644 --- a/tests/test_routers.py +++ b/tests/test_routers.py @@ -10,7 +10,9 @@ from rest_framework import permissions, serializers, viewsets 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 @@ class RegexUrlPathViewSet(viewsets.ViewSet): return Response({'pk': pk, 'kwarg': kwarg}) +class UrlPathViewSet(viewsets.ViewSet): + @action(detail=False, url_path='list/') + def url_path_list(self, request, *args, **kwargs): + kwarg = self.kwargs.get('kwarg', '') + return Response({'kwarg': kwarg}) + + @action(detail=True, url_path='detail/') + 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 @@ empty_prefix_router.register(r'', EmptyPrefixViewSet, basename='empty_prefix') 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 @@ class TestRegexUrlPath(URLPatternsTestCase, TestCase): 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)),