From de84efd81f2cb04a37dd0da8123a157c2e071c1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cristi=20V=C3=AEjdea?= Date: Wed, 20 Dec 2017 12:50:54 +0100 Subject: [PATCH] Add django 2 variant for all tests and fix trailing slash bug --- rest_framework/urlpatterns.py | 5 +- tests/test_urlpatterns.py | 157 ++++++++++++++++++++++++++-------- 2 files changed, 122 insertions(+), 40 deletions(-) diff --git a/rest_framework/urlpatterns.py b/rest_framework/urlpatterns.py index 719eef1ea..ab3a74978 100644 --- a/rest_framework/urlpatterns.py +++ b/rest_framework/urlpatterns.py @@ -68,14 +68,11 @@ def apply_suffix_patterns(urlpatterns, suffix_pattern, suffix_required, suffix_r if not suffix_required: ret.append(urlpattern) - # we create a new RegexPattern url; if the original pattern - # was a RoutePattern we need to preserve its converters - # if the original pattern was a RoutePattern we need to preserve it if is_route_pattern(urlpattern): assert path is not None assert suffix_route is not None - route = str(urlpattern.pattern) + suffix_route + route = str(urlpattern.pattern).rstrip('$').rstrip('/') + suffix_route new_pattern = path(route, view, kwargs, name) else: new_pattern = url(regex, view, kwargs, name) diff --git a/tests/test_urlpatterns.py b/tests/test_urlpatterns.py index 18141ead9..a4f54d738 100644 --- a/tests/test_urlpatterns.py +++ b/tests/test_urlpatterns.py @@ -24,41 +24,29 @@ class FormatSuffixTests(TestCase): Tests `format_suffix_patterns` against different URLPatterns to ensure the URLs still resolve properly, including any captured parameters. """ - def _resolve_urlpatterns(self, urlpatterns, test_paths): + def _resolve_urlpatterns(self, urlpatterns, test_paths, allowed=None): factory = APIRequestFactory() try: - urlpatterns = format_suffix_patterns(urlpatterns) + urlpatterns = format_suffix_patterns(urlpatterns, allowed=allowed) except Exception: self.fail("Failed to apply `format_suffix_patterns` on the supplied urlpatterns") resolver = make_url_resolver(r'^/', urlpatterns) for test_path in test_paths: - request = factory.get(test_path.path) try: - callback, callback_args, callback_kwargs = resolver.resolve(request.path_info) - except Exception: - self.fail("Failed to resolve URL: %s" % request.path_info) - assert callback_args == test_path.args - assert callback_kwargs == test_path.kwargs + test_path, expected_resolved = test_path + except (TypeError, ValueError): + expected_resolved = True - def test_trailing_slash(self): - factory = APIRequestFactory() - urlpatterns = format_suffix_patterns([ - url(r'^test/$', dummy_view), - ]) - resolver = make_url_resolver(r'^/', urlpatterns) - - test_paths = [ - (URLTestPath('/test.api', (), {'format': 'api'}), True), - (URLTestPath('/test/.api', (), {'format': 'api'}), False), - (URLTestPath('/test.api/', (), {'format': 'api'}), True), - ] - - for test_path, expected_resolved in test_paths: request = factory.get(test_path.path) try: callback, callback_args, callback_kwargs = resolver.resolve(request.path_info) except Resolver404: callback, callback_args, callback_kwargs = (None, None, None) + if expected_resolved: + raise + except Exception: + self.fail("Failed to resolve URL: %s" % request.path_info) + if not expected_resolved: assert callback is None continue @@ -66,10 +54,28 @@ class FormatSuffixTests(TestCase): assert callback_args == test_path.args assert callback_kwargs == test_path.kwargs - def test_format_suffix(self): - urlpatterns = [ - url(r'^test$', dummy_view), + def _test_trailing_slash(self, urlpatterns): + test_paths = [ + (URLTestPath('/test.api', (), {'format': 'api'}), True), + (URLTestPath('/test/.api', (), {'format': 'api'}), False), + (URLTestPath('/test.api/', (), {'format': 'api'}), True), ] + self._resolve_urlpatterns(urlpatterns, test_paths) + + def test_trailing_slash(self): + urlpatterns = [ + url(r'^test/$', dummy_view), + ] + self._test_trailing_slash(urlpatterns) + + @unittest.skipUnless(path, 'needs Django 2') + def test_trailing_slash_django2(self): + urlpatterns = [ + path('test/', dummy_view), + ] + self._test_trailing_slash(urlpatterns) + + def _test_format_suffix(self, urlpatterns): test_paths = [ URLTestPath('/test', (), {}), URLTestPath('/test.api', (), {'format': 'api'}), @@ -77,17 +83,26 @@ class FormatSuffixTests(TestCase): ] self._resolve_urlpatterns(urlpatterns, test_paths) + def test_format_suffix(self): + urlpatterns = [ + url(r'^test$', dummy_view), + ] + self._test_format_suffix(urlpatterns) + @unittest.skipUnless(path, 'needs Django 2') - def test_django2_path(self): + def test_format_suffix_django2(self): urlpatterns = [ path('test', dummy_view), + ] + self._test_format_suffix(urlpatterns) + + @unittest.skipUnless(path, 'needs Django 2') + def test_format_suffix_django2_args(self): + urlpatterns = [ path('convtest/', dummy_view), re_path(r'^retest/(?P[0-9]+)$', dummy_view), ] test_paths = [ - URLTestPath('/test', (), {}), - URLTestPath('/test.api', (), {'format': 'api'}), - URLTestPath('/test.asdf', (), {'format': 'asdf'}), URLTestPath('/convtest/42', (), {'pk': 42}), URLTestPath('/convtest/42.api', (), {'pk': 42, 'format': 'api'}), URLTestPath('/convtest/42.asdf', (), {'pk': 42, 'format': 'asdf'}), @@ -97,10 +112,7 @@ class FormatSuffixTests(TestCase): ] self._resolve_urlpatterns(urlpatterns, test_paths) - def test_default_args(self): - urlpatterns = [ - url(r'^test$', dummy_view, {'foo': 'bar'}), - ] + def _test_default_args(self, urlpatterns): test_paths = [ URLTestPath('/test', (), {'foo': 'bar', }), URLTestPath('/test.api', (), {'foo': 'bar', 'format': 'api'}), @@ -108,6 +120,27 @@ class FormatSuffixTests(TestCase): ] self._resolve_urlpatterns(urlpatterns, test_paths) + def test_default_args(self): + urlpatterns = [ + url(r'^test$', dummy_view, {'foo': 'bar'}), + ] + self._test_default_args(urlpatterns) + + @unittest.skipUnless(path, 'needs Django 2') + def test_default_args_django2(self): + urlpatterns = [ + path('test', dummy_view, {'foo': 'bar'}), + ] + self._test_default_args(urlpatterns) + + def _test_included_urls(self, urlpatterns): + test_paths = [ + URLTestPath('/test/path', (), {'foo': 'bar', }), + URLTestPath('/test/path.api', (), {'foo': 'bar', 'format': 'api'}), + URLTestPath('/test/path.asdf', (), {'foo': 'bar', 'format': 'asdf'}), + ] + self._resolve_urlpatterns(urlpatterns, test_paths) + def test_included_urls(self): nested_patterns = [ url(r'^path$', dummy_view) @@ -115,9 +148,61 @@ class FormatSuffixTests(TestCase): urlpatterns = [ url(r'^test/', include(nested_patterns), {'foo': 'bar'}), ] + self._test_included_urls(urlpatterns) + + @unittest.skipUnless(path, 'needs Django 2') + def test_included_urls_django2(self): + nested_patterns = [ + path('path', dummy_view) + ] + urlpatterns = [ + path('test/', include(nested_patterns), {'foo': 'bar'}), + ] + self._test_included_urls(urlpatterns) + + @unittest.skipUnless(path, 'needs Django 2') + def test_included_urls_django2_mixed(self): + nested_patterns = [ + path('path', dummy_view) + ] + urlpatterns = [ + url('^test/', include(nested_patterns), {'foo': 'bar'}), + ] + self._test_included_urls(urlpatterns) + + @unittest.skipUnless(path, 'needs Django 2') + def test_included_urls_django2_mixed_args(self): + nested_patterns = [ + path('path/', dummy_view) + ] + urlpatterns = [ + url('^test/(?P[a-z0-9]+)/', include(nested_patterns), {'foo': 'bar'}), + ] test_paths = [ - URLTestPath('/test/path', (), {'foo': 'bar', }), - URLTestPath('/test/path.api', (), {'foo': 'bar', 'format': 'api'}), - URLTestPath('/test/path.asdf', (), {'foo': 'bar', 'format': 'asdf'}), + URLTestPath('/test/pid/path/42', (), {'parent': 'pid', 'child': 42, 'foo': 'bar', }), + URLTestPath('/test/pid/path/42.api', (), {'parent': 'pid', 'child': 42, 'foo': 'bar', 'format': 'api'}), + URLTestPath('/test/pid/path/42.asdf', (), {'parent': 'pid', 'child': 42, 'foo': 'bar', 'format': 'asdf'}), ] self._resolve_urlpatterns(urlpatterns, test_paths) + + def _test_allowed_formats(self, urlpatterns): + allowed_formats = ['good', 'ugly'] + test_paths = [ + (URLTestPath('/test.good/', (), {'format': 'good'}), True), + (URLTestPath('/test.bad', (), {}), False), + (URLTestPath('/test.ugly', (), {'format': 'ugly'}), True), + ] + self._resolve_urlpatterns(urlpatterns, test_paths, allowed=allowed_formats) + + def test_allowed_formats(self): + urlpatterns = [ + url('^test$', dummy_view), + ] + self._test_allowed_formats(urlpatterns) + + @unittest.skipUnless(path, 'needs Django 2') + def test_allowed_formats_django2(self): + urlpatterns = [ + path('test', dummy_view), + ] + self._test_allowed_formats(urlpatterns)