Add django 2 variant for all tests and fix trailing slash bug

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)

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/<int:pk>', dummy_view),
             re_path(r'^retest/(?P<pk>[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/<int:child>', dummy_view)
+        ]
+        urlpatterns = [
+            url('^test/(?P<parent>[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)