From 4fbe3ecce654614232f94362d51184e241ac07c6 Mon Sep 17 00:00:00 2001 From: Ryan P Kilby Date: Sat, 18 Nov 2017 13:54:48 -0500 Subject: [PATCH 1/8] Add request.current_app handling to drf reverse --- rest_framework/reverse.py | 17 +++++++ tests/test_reverse.py | 100 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 112 insertions(+), 5 deletions(-) diff --git a/rest_framework/reverse.py b/rest_framework/reverse.py index 54c463553..d5123d351 100644 --- a/rest_framework/reverse.py +++ b/rest_framework/reverse.py @@ -60,10 +60,27 @@ def _reverse(viewname, args=None, kwargs=None, request=None, format=None, **extr if format is not None: kwargs = kwargs or {} kwargs['format'] = format + if request: + extra.setdefault('current_app', current_app(request)) url = django_reverse(viewname, args=args, kwargs=kwargs, **extra) if request: return request.build_absolute_uri(url) return url +def current_app(request): + """ + Get the current app for the request. + + This code is copied from the URL tag. + """ + try: + return request.current_app + except AttributeError: + try: + return request.resolver_match.namespace + except AttributeError: + return None + + reverse_lazy = lazy(reverse, six.text_type) diff --git a/tests/test_reverse.py b/tests/test_reverse.py index 145b1a54f..473a82ffd 100644 --- a/tests/test_reverse.py +++ b/tests/test_reverse.py @@ -1,21 +1,29 @@ from __future__ import unicode_literals -from django.conf.urls import url +from django.conf.urls import include, url +from django.http import HttpResponse from django.test import TestCase, override_settings from django.urls import NoReverseMatch -from rest_framework.reverse import reverse +from rest_framework.reverse import current_app, reverse from rest_framework.test import APIRequestFactory factory = APIRequestFactory() -def null_view(request): - pass +def mock_view(request): + return HttpResponse('') + + +apppatterns = ([ + url(r'^home$', mock_view, name='home'), +], 'app') urlpatterns = [ - url(r'^view$', null_view, name='view'), + url(r'^view$', mock_view, name='view'), + url(r'^app2/', include(apppatterns, namespace='app2')), + url(r'^app1/', include(apppatterns, namespace='app1')), ] @@ -54,3 +62,85 @@ class ReverseTests(TestCase): url = reverse('view', request=request) assert url == 'http://testserver/view' + + +@override_settings(ROOT_URLCONF='tests.test_reverse') +class NamespaceTests(TestCase): + """ + Ensure reverse can handle namespaces. + + Note: It's necessary to use self.client() here, as the + RequestFactory does not setup the resolver_match. + """ + + def request(self, url): + return self.client.get(url).wsgi_request + + def test_application_namespace(self): + url = reverse('app:home') + assert url == '/app1/home' + + # instance namespace provided by current_app + url = reverse('app:home', current_app='app2') + assert url == '/app2/home' + + def test_instance_namespace(self): + url = reverse('app1:home') + assert url == '/app1/home' + + url = reverse('app2:home') + assert url == '/app2/home' + + def test_application_namespace_with_request(self): + # request's current app affects result + request1 = self.request('/app1/home') + request2 = self.request('/app2/home') + + # sanity check + assert current_app(request1) == 'app1' + assert current_app(request2) == 'app2' + + assert reverse('app:home', request=request1) == 'http://testserver/app1/home' + assert reverse('app:home', request=request2) == 'http://testserver/app2/home' + + def test_instance_namespace_with_request(self): + # request's current app is not relevant + request1 = self.request('/app1/home') + request2 = self.request('/app2/home') + + # sanity check + assert current_app(request1) == 'app1' + assert current_app(request2) == 'app2' + + assert reverse('app1:home', request=request1) == 'http://testserver/app1/home' + assert reverse('app2:home', request=request1) == 'http://testserver/app2/home' + assert reverse('app1:home', request=request2) == 'http://testserver/app1/home' + assert reverse('app2:home', request=request2) == 'http://testserver/app2/home' + + +@override_settings(ROOT_URLCONF='tests.test_reverse') +class CurrentAppTests(TestCase): + """ + Test current_app() function. + + Note: It's necessary to use self.client() here, as the + RequestFactory does not setup the resolver_match. + """ + + def request(self, url): + return self.client.get(url).wsgi_request + + def test_no_namespace(self): + request = self.request('/view') + assert current_app(request) == '' + + def test_namespace(self): + request = self.request('/app1/home') + assert current_app(request) == 'app1' + + request = self.request('/app2/home') + assert current_app(request) == 'app2' + + def test_factory_incompatibility(self): + request = factory.get('/app1/home') + assert current_app(request) is None From ec7f35801038e0ad5d1346d69468bc729afc9204 Mon Sep 17 00:00:00 2001 From: Ryan P Kilby Date: Sat, 18 Nov 2017 17:27:33 -0500 Subject: [PATCH 2/8] Add default app namespace to drf reverse --- rest_framework/reverse.py | 21 +++++++++++++++++++++ tests/test_reverse.py | 38 ++++++++++++++++++++++++++++---------- 2 files changed, 49 insertions(+), 10 deletions(-) diff --git a/rest_framework/reverse.py b/rest_framework/reverse.py index d5123d351..2a119afd5 100644 --- a/rest_framework/reverse.py +++ b/rest_framework/reverse.py @@ -34,10 +34,29 @@ def preserve_builtin_query_params(url, request=None): def reverse(viewname, args=None, kwargs=None, request=None, format=None, **extra): """ + Extends `django.urls.reverse` with behavior specific to rest framework. + + The `viewname` will be prepended with the 'rest_framework' application + namespace if no namspace is included in the `viewname` argument. The + framework fundamentally assumes that the router urls will be included with + the 'rest_framework' namespace, so ensure that your root url patterns are + configured accordingly. Assuming you use the default router, you can check + this with: + + from django.urls import reverse + + reverse('rest_framework:api-root') + If versioning is being used then we pass any `reverse` calls through to the versioning scheme instance, so that the resulting URL can be modified if needed. + + Optionally takes a `request` object (see `_reverse` for details). """ + # prepend the 'rest_framework' application namespace + if ':' not in viewname: + viewname = 'rest_framework:' + viewname + scheme = getattr(request, 'versioning_scheme', None) if scheme is not None: try: @@ -56,6 +75,8 @@ def _reverse(viewname, args=None, kwargs=None, request=None, format=None, **extr """ Same as `django.urls.reverse`, but optionally takes a request and returns a fully qualified URL, using the request to get the base URL. + + Additionally, the request is used to determine the `current_app` instance. """ if format is not None: kwargs = kwargs or {} diff --git a/tests/test_reverse.py b/tests/test_reverse.py index 473a82ffd..673f46002 100644 --- a/tests/test_reverse.py +++ b/tests/test_reverse.py @@ -19,11 +19,16 @@ apppatterns = ([ url(r'^home$', mock_view, name='home'), ], 'app') +apipatterns = ([ + url(r'^root$', mock_view, name='root'), +], 'rest_framework') + urlpatterns = [ url(r'^view$', mock_view, name='view'), url(r'^app2/', include(apppatterns, namespace='app2')), url(r'^app1/', include(apppatterns, namespace='app1')), + url(r'^api/', include(apipatterns)), ] @@ -36,7 +41,7 @@ class MockVersioningScheme(object): if self.raise_error: raise NoReverseMatch() - return 'http://scheme-reversed/view' + return 'http://scheme-reversed/api/root' @override_settings(ROOT_URLCONF='tests.test_reverse') @@ -44,24 +49,33 @@ class ReverseTests(TestCase): """ Tests for fully qualified URLs when using `reverse`. """ + + def test_reverse_non_drf_view(self): + request = factory.get('/api/root') + + # DRF reverse should not match non-DRF views + with self.assertRaises(NoReverseMatch): + reverse('view', request=request) + def test_reversed_urls_are_fully_qualified(self): - request = factory.get('/view') - url = reverse('view', request=request) - assert url == 'http://testserver/view' + request = factory.get('/api/root') + + url = reverse('root', request=request) + assert url == 'http://testserver/api/root' def test_reverse_with_versioning_scheme(self): - request = factory.get('/view') + request = factory.get('/api/root') request.versioning_scheme = MockVersioningScheme() - url = reverse('view', request=request) - assert url == 'http://scheme-reversed/view' + url = reverse('root', request=request) + assert url == 'http://scheme-reversed/api/root' def test_reverse_with_versioning_scheme_fallback_to_default_on_error(self): - request = factory.get('/view') + request = factory.get('/api/root') request.versioning_scheme = MockVersioningScheme(raise_error=True) - url = reverse('view', request=request) - assert url == 'http://testserver/view' + url = reverse('root', request=request) + assert url == 'http://testserver/api/root' @override_settings(ROOT_URLCONF='tests.test_reverse') @@ -76,6 +90,10 @@ class NamespaceTests(TestCase): def request(self, url): return self.client.get(url).wsgi_request + def test_default_namespace(self): + url = reverse('root') + assert url == '/api/root' + def test_application_namespace(self): url = reverse('app:home') assert url == '/app1/home' From 1de241d9bf409a5e951437f28d9528b0de9d0880 Mon Sep 17 00:00:00 2001 From: Ryan P Kilby Date: Sat, 18 Nov 2017 20:14:43 -0500 Subject: [PATCH 3/8] Add tests for hyperlink serializer+namespaced urls --- tests/test_relations_hyperlink.py | 38 ++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/tests/test_relations_hyperlink.py b/tests/test_relations_hyperlink.py index 887a6f423..ddfbc165d 100644 --- a/tests/test_relations_hyperlink.py +++ b/tests/test_relations_hyperlink.py @@ -1,6 +1,6 @@ from __future__ import unicode_literals -from django.conf.urls import url +from django.conf.urls import include, url from django.test import TestCase, override_settings from rest_framework import serializers @@ -449,3 +449,39 @@ class HyperlinkedNullableOneToOneTests(TestCase): {'url': 'http://testserver/onetoonetarget/2/', 'name': 'target-2', 'nullable_source': None}, ] assert serializer.data == expected + + +class HyperlinkedNamespaceTests(TestCase): + def setUp(self): + target = ForeignKeyTarget(name='target-1') + target.save() + new_target = ForeignKeyTarget(name='target-2') + new_target.save() + for idx in range(1, 4): + source = ForeignKeySource(name='source-%d' % idx, target=target) + source.save() + + def results(self): + queryset = ForeignKeySource.objects.all() + serializer = ForeignKeySourceSerializer(queryset, many=True, context={'request': request}) + expected = [ + {'url': 'http://testserver/foreignkeysource/1/', 'name': 'source-1', 'target': 'http://testserver/foreignkeytarget/1/'}, + {'url': 'http://testserver/foreignkeysource/2/', 'name': 'source-2', 'target': 'http://testserver/foreignkeytarget/1/'}, + {'url': 'http://testserver/foreignkeysource/3/', 'name': 'source-3', 'target': 'http://testserver/foreignkeytarget/1/'} + ] + with self.assertNumQueries(1): + assert serializer.data == expected + + def test_foreign_key_retrieve_no_namespace(self): + patterns = [ + url(r'^', include(urlpatterns, namespace=None)) + ] + with override_settings(ROOT_URLCONF=patterns): + self.results() + + def test_foreign_key_retrieve_namespace(self): + patterns = [ + url(r'^', include(urlpatterns, namespace='api')) + ] + with override_settings(ROOT_URLCONF=patterns): + self.results() From 09343c18743ac669a848a5da60061d1fd2db2a70 Mon Sep 17 00:00:00 2001 From: Ryan P Kilby Date: Sat, 18 Nov 2017 20:26:28 -0500 Subject: [PATCH 4/8] Convert drf to expect 'rest_framework' namespace --- rest_framework/relations.py | 3 ++- rest_framework/routers.py | 3 --- rest_framework/versioning.py | 7 +++++++ tests/test_routers.py | 10 +++++++--- 4 files changed, 16 insertions(+), 7 deletions(-) diff --git a/rest_framework/relations.py b/rest_framework/relations.py index c87b9299a..fc66d8704 100644 --- a/rest_framework/relations.py +++ b/rest_framework/relations.py @@ -339,7 +339,8 @@ class HyperlinkedRelatedField(RelatedField): self.view_name, request ) except AttributeError: - expected_viewname = self.view_name + # by default, expect the 'rest_framework' namespace + expected_viewname = 'rest_framework:' + self.view_name if match.view_name != expected_viewname: self.fail('incorrect_match') diff --git a/rest_framework/routers.py b/rest_framework/routers.py index 9007788f8..beb3c2911 100644 --- a/rest_framework/routers.py +++ b/rest_framework/routers.py @@ -305,10 +305,7 @@ class APIRootView(views.APIView): def get(self, request, *args, **kwargs): # Return a plain {"name": "hyperlink"} response. ret = OrderedDict() - namespace = request.resolver_match.namespace for key, url_name in self.api_root_dict.items(): - if namespace: - url_name = namespace + ':' + url_name try: ret[key] = reverse( url_name, diff --git a/rest_framework/versioning.py b/rest_framework/versioning.py index 5c9a7ade1..384f1eb75 100644 --- a/rest_framework/versioning.py +++ b/rest_framework/versioning.py @@ -135,6 +135,13 @@ class NamespaceVersioning(BaseVersioning): ) def get_versioned_viewname(self, viewname, request): + """ + The incoming `viewname` should be prefixed with the 'rest_framework' + application namespace. We want to replace this with the version + instance namespace. + """ + if viewname.startswith('rest_framework:'): + viewname = viewname[len('rest_framework:'):] return request.version + ':' + viewname diff --git a/tests/test_routers.py b/tests/test_routers.py index 36255f48f..f39b9c694 100644 --- a/tests/test_routers.py +++ b/tests/test_routers.py @@ -146,12 +146,16 @@ class TestSimpleRouter(TestCase): class TestRootView(URLPatternsTestCase, TestCase): urlpatterns = [ url(r'^non-namespaced/', include(namespaced_router.urls)), - url(r'^namespaced/', include((namespaced_router.urls, 'namespaced'), namespace='namespaced')), + url(r'^namespaced1/', include((namespaced_router.urls, 'namespaced1'), namespace='namespaced1')), + url(r'^namespaced2/', include((namespaced_router.urls, 'namespaced2'), namespace='namespaced2')), ] def test_retrieve_namespaced_root(self): - response = self.client.get('/namespaced/') - assert response.data == {"example": "http://testserver/namespaced/example/"} + response = self.client.get('/namespaced1/') + assert response.data == {"example": "http://testserver/namespaced1/example/"} + + response = self.client.get('/namespaced2/') + assert response.data == {"example": "http://testserver/namespaced2/example/"} def test_retrieve_non_namespaced_root(self): response = self.client.get('/non-namespaced/') From 87f382ada7bcb1e17cfa2e7f3c1dd6b7c7d3ce4c Mon Sep 17 00:00:00 2001 From: Ryan P Kilby Date: Sat, 18 Nov 2017 20:27:48 -0500 Subject: [PATCH 5/8] Add BaseRouter.urlpatterns --- rest_framework/routers.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/rest_framework/routers.py b/rest_framework/routers.py index beb3c2911..b14f2b866 100644 --- a/rest_framework/routers.py +++ b/rest_framework/routers.py @@ -101,6 +101,10 @@ class BaseRouter(object): self._urls = self.get_urls() return self._urls + @property + def urlpatterns(self): + return (self.urls, 'rest_framework') + class SimpleRouter(BaseRouter): From fee200ebf0baa2828624d92790b20be3812066f8 Mon Sep 17 00:00:00 2001 From: Ryan P Kilby Date: Sat, 18 Nov 2017 20:28:22 -0500 Subject: [PATCH 6/8] Move auth urls to 'rest_framework_auth' namespace --- rest_framework/templatetags/rest_framework.py | 6 +++--- rest_framework/urls.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/rest_framework/templatetags/rest_framework.py b/rest_framework/templatetags/rest_framework.py index 2a2459b37..abf7d4d74 100644 --- a/rest_framework/templatetags/rest_framework.py +++ b/rest_framework/templatetags/rest_framework.py @@ -96,7 +96,7 @@ def optional_login(request): Include a login snippet if REST framework's login view is in the URLconf. """ try: - login_url = reverse('rest_framework:login') + login_url = reverse('rest_framework_auth:login') except NoReverseMatch: return '' @@ -112,7 +112,7 @@ def optional_docs_login(request): Include a login snippet if REST framework's login view is in the URLconf. """ try: - login_url = reverse('rest_framework:login') + login_url = reverse('rest_framework_auth:login') except NoReverseMatch: return 'log in' @@ -128,7 +128,7 @@ def optional_logout(request, user): Include a logout snippet if REST framework's logout view is in the URLconf. """ try: - logout_url = reverse('rest_framework:logout') + logout_url = reverse('rest_framework_auth:logout') except NoReverseMatch: snippet = format_html('
  • {user}
    • ', user=escape(user)) return mark_safe(snippet) diff --git a/rest_framework/urls.py b/rest_framework/urls.py index 80fce5dc4..8d40727c4 100644 --- a/rest_framework/urls.py +++ b/rest_framework/urls.py @@ -27,7 +27,7 @@ else: logout = views.LogoutView.as_view() -app_name = 'rest_framework' +app_name = 'rest_framework_auth' urlpatterns = [ url(r'^login/$', login, login_kwargs, name='login'), url(r'^logout/$', logout, name='logout'), From ebb92cff78a97dafa20c9aa48f1fd6deb0ca472d Mon Sep 17 00:00:00 2001 From: Ryan P Kilby Date: Sat, 18 Nov 2017 20:30:43 -0500 Subject: [PATCH 7/8] Update urlpatterns throughout tests --- tests/test_lazy_hyperlinks.py | 8 ++++++-- tests/test_relations.py | 6 ++++-- tests/test_relations_hyperlink.py | 18 +++++++++++------- tests/test_routers.py | 18 +++++++++--------- tests/test_versioning.py | 27 ++++++++++++++++++--------- tests/test_viewsets.py | 2 +- 6 files changed, 49 insertions(+), 30 deletions(-) diff --git a/tests/test_lazy_hyperlinks.py b/tests/test_lazy_hyperlinks.py index cf3ee735f..a502ce946 100644 --- a/tests/test_lazy_hyperlinks.py +++ b/tests/test_lazy_hyperlinks.py @@ -1,4 +1,4 @@ -from django.conf.urls import url +from django.conf.urls import include, url from django.db import models from django.test import TestCase, override_settings @@ -28,10 +28,14 @@ def dummy_view(request): pass -urlpatterns = [ +patterns = [ url(r'^example/(?P[0-9]+)/$', dummy_view, name='example-detail'), ] +urlpatterns = [ + url(r'^', include((patterns, 'rest_framework'))) +] + @override_settings(ROOT_URLCONF='tests.test_lazy_hyperlinks') class TestLazyHyperlinkNames(TestCase): diff --git a/tests/test_relations.py b/tests/test_relations.py index fd3256e89..b686bea17 100644 --- a/tests/test_relations.py +++ b/tests/test_relations.py @@ -2,7 +2,7 @@ import uuid import pytest from _pytest.monkeypatch import MonkeyPatch -from django.conf.urls import url +from django.conf.urls import include, url from django.core.exceptions import ImproperlyConfigured from django.test import override_settings from django.utils.datastructures import MultiValueDict @@ -146,7 +146,9 @@ class TestProxiedPrimaryKeyRelatedField(APISimpleTestCase): @override_settings(ROOT_URLCONF=[ - url(r'^example/(?P.+)/$', lambda: None, name='example'), + url(r'^', include( + ([url(r'^example/(?P.+)/$', lambda: None, name='example')], 'rest_framework') + )), ]) class TestHyperlinkedRelatedField(APISimpleTestCase): def setUp(self): diff --git a/tests/test_relations_hyperlink.py b/tests/test_relations_hyperlink.py index ddfbc165d..45faf90a2 100644 --- a/tests/test_relations_hyperlink.py +++ b/tests/test_relations_hyperlink.py @@ -18,7 +18,7 @@ def dummy_view(request, pk): pass -urlpatterns = [ +patterns = [ url(r'^dummyurl/(?P[0-9]+)/$', dummy_view, name='dummy-url'), url(r'^manytomanysource/(?P[0-9]+)/$', dummy_view, name='manytomanysource-detail'), url(r'^manytomanytarget/(?P[0-9]+)/$', dummy_view, name='manytomanytarget-detail'), @@ -29,6 +29,10 @@ urlpatterns = [ url(r'^nullableonetoonesource/(?P[0-9]+)/$', dummy_view, name='nullableonetoonesource-detail'), ] +urlpatterns = [ + url(r'^', include((patterns, 'rest_framework'))) +] + # ManyToMany class ManyToManyTargetSerializer(serializers.HyperlinkedModelSerializer): @@ -473,15 +477,15 @@ class HyperlinkedNamespaceTests(TestCase): assert serializer.data == expected def test_foreign_key_retrieve_no_namespace(self): - patterns = [ - url(r'^', include(urlpatterns, namespace=None)) + url_conf = [ + url(r'^', include((patterns, 'rest_framework'), namespace=None)) ] - with override_settings(ROOT_URLCONF=patterns): + with override_settings(ROOT_URLCONF=url_conf): self.results() def test_foreign_key_retrieve_namespace(self): - patterns = [ - url(r'^', include(urlpatterns, namespace='api')) + url_conf = [ + url(r'^', include((patterns, 'rest_framework'), namespace='api')) ] - with override_settings(ROOT_URLCONF=patterns): + with override_settings(ROOT_URLCONF=url_conf): self.results() diff --git a/tests/test_routers.py b/tests/test_routers.py index f39b9c694..e1dafa9bd 100644 --- a/tests/test_routers.py +++ b/tests/test_routers.py @@ -145,9 +145,9 @@ class TestSimpleRouter(TestCase): class TestRootView(URLPatternsTestCase, TestCase): urlpatterns = [ - url(r'^non-namespaced/', include(namespaced_router.urls)), - url(r'^namespaced1/', include((namespaced_router.urls, 'namespaced1'), namespace='namespaced1')), - url(r'^namespaced2/', include((namespaced_router.urls, 'namespaced2'), namespace='namespaced2')), + url(r'^non-namespaced/', include(namespaced_router.urlpatterns)), + url(r'^namespaced1/', include(namespaced_router.urlpatterns, namespace='namespaced1')), + url(r'^namespaced2/', include(namespaced_router.urlpatterns, namespace='namespaced2')), ] def test_retrieve_namespaced_root(self): @@ -167,8 +167,8 @@ class TestCustomLookupFields(URLPatternsTestCase, TestCase): Ensure that custom lookup fields are correctly routed. """ urlpatterns = [ - url(r'^example/', include(notes_router.urls)), - url(r'^example2/', include(kwarged_notes_router.urls)), + url(r'^example/', include(notes_router.urlpatterns)), + url(r'^example2/', include(kwarged_notes_router.urlpatterns)), ] def setUp(self): @@ -225,8 +225,8 @@ class TestLookupUrlKwargs(URLPatternsTestCase, TestCase): Setup a deep lookup_field, but map it to a simple URL kwarg. """ urlpatterns = [ - url(r'^example/', include(notes_router.urls)), - url(r'^example2/', include(kwarged_notes_router.urls)), + url(r'^example/', include(notes_router.urlpatterns)), + url(r'^example2/', include(kwarged_notes_router.urlpatterns)), ] def setUp(self): @@ -414,7 +414,7 @@ class TestDynamicListAndDetailRouter(TestCase): class TestEmptyPrefix(URLPatternsTestCase, TestCase): urlpatterns = [ - url(r'^empty-prefix/', include(empty_prefix_router.urls)), + url(r'^empty-prefix/', include(empty_prefix_router.urlpatterns)), ] def test_empty_prefix_list(self): @@ -431,7 +431,7 @@ class TestEmptyPrefix(URLPatternsTestCase, TestCase): class TestRegexUrlPath(URLPatternsTestCase, TestCase): urlpatterns = [ - url(r'^regex/', include(regex_url_path_router.urls)), + url(r'^regex/', include(regex_url_path_router.urlpatterns)), ] def test_regex_url_path_list(self): diff --git a/tests/test_versioning.py b/tests/test_versioning.py index 7e650e275..3a19cdaac 100644 --- a/tests/test_versioning.py +++ b/tests/test_versioning.py @@ -143,17 +143,21 @@ class TestRequestVersion: class TestURLReversing(URLPatternsTestCase, APITestCase): - included = [ + versioned = [ url(r'^namespaced/$', dummy_view, name='another'), url(r'^example/(?P\d+)/$', dummy_pk_view, name='example-detail') ] - urlpatterns = [ - url(r'^v1/', include((included, 'v1'), namespace='v1')), + included = [ url(r'^another/$', dummy_view, name='another'), url(r'^(?P[v1|v2]+)/another/$', dummy_view, name='another'), ] + urlpatterns = [ + url(r'^v1/', include((versioned, 'rest_framework'), namespace='v1')), + url(r'^', include((included, 'rest_framework'))), + ] + def test_reverse_unversioned(self): view = ReverseView.as_view() @@ -314,8 +318,8 @@ class TestHyperlinkedRelatedField(URLPatternsTestCase, APITestCase): ] urlpatterns = [ - url(r'^v1/', include((included, 'v1'), namespace='v1')), - url(r'^v2/', include((included, 'v2'), namespace='v2')) + url(r'^v1/', include((included, 'rest_framework'), namespace='v1')), + url(r'^v2/', include((included, 'rest_framework'), namespace='v2')) ] def setUp(self): @@ -344,17 +348,21 @@ class TestNamespaceVersioningHyperlinkedRelatedFieldScheme(URLPatternsTestCase, nested = [ url(r'^namespaced/(?P\d+)/$', dummy_pk_view, name='nested'), ] - included = [ + versioned = [ url(r'^namespaced/(?P\d+)/$', dummy_pk_view, name='namespaced'), url(r'^nested/', include((nested, 'nested-namespace'), namespace='nested-namespace')) ] - urlpatterns = [ - url(r'^v1/', include((included, 'restframeworkv1'), namespace='v1')), - url(r'^v2/', include((included, 'restframeworkv2'), namespace='v2')), + included = [ url(r'^non-api/(?P\d+)/$', dummy_pk_view, name='non-api-view') ] + urlpatterns = [ + url(r'^v1/', include((versioned, 'rest_framework'), namespace='v1')), + url(r'^v2/', include((versioned, 'rest_framework'), namespace='v2')), + url(r'^', include((included, 'rest_framework'))), + ] + def _create_field(self, view_name, version): request = factory.get("/") request.versioning_scheme = NamespaceVersioning() @@ -381,6 +389,7 @@ class TestNamespaceVersioningHyperlinkedRelatedFieldScheme(URLPatternsTestCase, def test_non_api_url_is_properly_reversed_regardless_of_the_version(self): """ Regression test for #2711 + Note: non-api-views still need to be included in the 'rest_framework' namespace """ field = self._create_field('non-api-view', 'v1') assert field.to_representation(PKOnlyObject(10)) == 'http://testserver/non-api/10/' diff --git a/tests/test_viewsets.py b/tests/test_viewsets.py index 25feb0f37..30bb03144 100644 --- a/tests/test_viewsets.py +++ b/tests/test_viewsets.py @@ -62,7 +62,7 @@ router.register(r'actions-alt', ActionViewSet, base_name='actions-alt') urlpatterns = [ - url(r'^api/', include(router.urls)), + url(r'^api/', include(router.urlpatterns)), ] From 00f2f72e0dbb47405a15e265984e440a9dd9a930 Mon Sep 17 00:00:00 2001 From: Ryan P Kilby Date: Fri, 26 Jan 2018 01:56:20 -0500 Subject: [PATCH 8/8] Add tests for root urlconf --- tests/test_urlconf.py | 127 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 127 insertions(+) create mode 100644 tests/test_urlconf.py diff --git a/tests/test_urlconf.py b/tests/test_urlconf.py new file mode 100644 index 000000000..6d3e02e42 --- /dev/null +++ b/tests/test_urlconf.py @@ -0,0 +1,127 @@ +""" +Test rest framework assumptions about the configuration of the root urlconf. +""" +from django.conf.urls import include, url + +from rest_framework import routers, viewsets +from rest_framework.reverse import NoReverseMatch, reverse +from rest_framework.test import APITestCase, URLPatternsTestCase + + +class MockViewSet(viewsets.ViewSet): + def list(self, request, *args, **kwargs): + pass + + +router = routers.DefaultRouter(trailing_slash=False) +router.register(r'example', MockViewSet, base_name='example') + + +class TestUrlsAppNameRequired(APITestCase, URLPatternsTestCase): + urlpatterns = [ + url(r'^api/', include(router.urls)), + ] + + def test_reverse(self): + """ + The 'rest_framework' namespace must be present. + """ + with self.assertRaises(NoReverseMatch): + reverse('example-list') + + +class TestUrlpatternsAppName(APITestCase, URLPatternsTestCase): + urlpatterns = [ + url(r'^api/', include(router.urlpatterns)), + ] + + def test_reverse(self): + self.assertEqual(reverse('example-list'), '/api/example') + + def test_app_name_reverse(self): + self.assertEqual(reverse('rest_framework:example-list'), '/api/example') + + +class TestUrlpatternsNamespace(APITestCase, URLPatternsTestCase): + urlpatterns = [ + url(r'^api/v1/', include(router.urlpatterns, namespace='v1')), + url(r'^api/v2/', include(router.urlpatterns, namespace='v2')), + ] + + def test_reverse(self): + self.assertEqual(reverse('example-list'), '/api/v2/example') + + def test_app_name_reverse(self): + self.assertEqual(reverse('rest_framework:example-list'), '/api/v2/example') + + def test_namespace_reverse(self): + self.assertEqual(reverse('v1:example-list'), '/api/v1/example') + self.assertEqual(reverse('v2:example-list'), '/api/v2/example') + + +class TestAppUrlpatternsAppName(APITestCase, URLPatternsTestCase): + apppatterns = ([ + url(r'^api/', include(router.urlpatterns)), + ], 'api') + + urlpatterns = [ + url(r'^', include(apppatterns)), + ] + + def test_reverse(self): + """ + Nesting the router.urlpatterns in an app with an app_name will + break url resolution. + """ + with self.assertRaises(NoReverseMatch): + reverse('example-list') + + +class TestAppUrlpatterns(APITestCase, URLPatternsTestCase): + apppatterns = ([ + url(r'^api/', include(router.urlpatterns)), + ], None) + + urlpatterns = [ + url(r'^', include(apppatterns)), + ] + + def test_reverse(self): + self.assertEqual(reverse('example-list'), '/api/example') + + +class TestAppUrlsAppName(APITestCase, URLPatternsTestCase): + apppatterns = ([ + url(r'^api/', include(router.urls)), + ], 'rest_framework') + + urlpatterns = [ + url(r'^', include(apppatterns)), + ] + + def test_reverse(self): + self.assertEqual(reverse('example-list'), '/api/example') + + def test_app_name_reverse(self): + self.assertEqual(reverse('rest_framework:example-list'), '/api/example') + + +class TestAppUrlsNamespace(APITestCase, URLPatternsTestCase): + apppatterns = ([ + url(r'^', include(router.urls)), + ], 'rest_framework') + + urlpatterns = [ + url(r'^api/v1/', include(apppatterns, namespace='v1')), + url(r'^api/v2/', include(apppatterns, namespace='v2')), + ] + + def test_reverse(self): + self.assertEqual(reverse('example-list'), '/api/v2/example') + + def test_app_name_reverse(self): + self.assertEqual(reverse('rest_framework:example-list'), '/api/v2/example') + + def test_namespace_reverse(self): + self.assertEqual(reverse('v1:example-list'), '/api/v1/example') + self.assertEqual(reverse('v2:example-list'), '/api/v2/example')