[WIP] Fix path handling

* needs more tests
* maybe needs some refactoring

rest_framework/compat.py

@@ -55,6 +55,16 @@ def get_regex_pattern(urlpattern):
         return urlpattern.regex.pattern
 
 
+def is_route_pattern(urlpattern):
+    if hasattr(urlpattern, 'pattern'):
+        # Django 2.0
+        from django.urls.resolvers import RoutePattern
+        return isinstance(urlpattern.pattern, RoutePattern)
+    else:
+        # Django < 2.0
+        return False
+
+
 def make_url_resolver(regex, urlpatterns):
     try:
         # Django 2.0
@@ -274,10 +284,11 @@ except ImportError:
 
 # Django 1.x url routing syntax. Remove when dropping Django 1.11 support.
 try:
-    from django.urls import include, path, re_path # noqa
+    from django.urls import include, path, re_path, register_converter  # noqa
 except ImportError:
     from django.conf.urls import include, url # noqa
     path = None
+    register_converter = None
     re_path = url
 
 

rest_framework/urlpatterns.py

@@ -2,11 +2,39 @@ from __future__ import unicode_literals
 
 from django.conf.urls import include, url
 
-from rest_framework.compat import URLResolver, get_regex_pattern
+from rest_framework.compat import (
+    URLResolver, get_regex_pattern, is_route_pattern, path, register_converter
+)
 from rest_framework.settings import api_settings
 
 
-def apply_suffix_patterns(urlpatterns, suffix_pattern, suffix_required):
+def _get_format_path_converter(suffix_kwarg, allowed):
+    if allowed:
+        if len(allowed) == 1:
+            allowed_pattern = allowed[0]
+        else:
+            allowed_pattern = '(?:%s)' % '|'.join(allowed)
+        suffix_pattern = r"\.%s/?" % allowed_pattern
+    else:
+        suffix_pattern = r"\.[a-z0-9]+/?"
+
+    class FormatSuffixConverter:
+        regex = suffix_pattern
+
+        def to_python(self, value):
+            return value.strip('./')
+
+        def to_url(self, value):
+            return '.' + value + '/'
+
+    converter_name = 'drf_format_suffix'
+    if allowed:
+        converter_name += '_' + '_'.join(allowed)
+
+    return converter_name, FormatSuffixConverter
+
+
+def apply_suffix_patterns(urlpatterns, suffix_pattern, suffix_required, suffix_route=None):
     ret = []
     for urlpattern in urlpatterns:
         if isinstance(urlpattern, URLResolver):
@@ -18,8 +46,18 @@ def apply_suffix_patterns(urlpatterns, suffix_pattern, suffix_required):
             # Add in the included patterns, after applying the suffixes
             patterns = apply_suffix_patterns(urlpattern.url_patterns,
                                              suffix_pattern,
-                                             suffix_required)
-            ret.append(url(regex, include((patterns, app_name), namespace), kwargs))
+                                             suffix_required,
+                                             suffix_route)
+
+            # if the original pattern was a RoutePattern we need to preserve it
+            if is_route_pattern(urlpattern):
+                assert path is not None
+                route = str(urlpattern.pattern)
+                new_pattern = path(route, include((patterns, app_name), namespace), kwargs)
+            else:
+                new_pattern = url(regex, include((patterns, app_name), namespace), kwargs)
+
+            ret.append(new_pattern)
         else:
             # Regular URL pattern
             regex = get_regex_pattern(urlpattern).rstrip('$').rstrip('/') + suffix_pattern
@@ -29,7 +67,20 @@ def apply_suffix_patterns(urlpatterns, suffix_pattern, suffix_required):
             # Add in both the existing and the new urlpattern
             if not suffix_required:
                 ret.append(urlpattern)
-            ret.append(url(regex, view, kwargs, name))
+
+            # 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
+                new_pattern = path(route, view, kwargs, name)
+            else:
+                new_pattern = url(regex, view, kwargs, name)
+
+            ret.append(new_pattern)
 
     return ret
 
@@ -60,4 +111,12 @@ def format_suffix_patterns(urlpatterns, suffix_required=False, allowed=None):
     else:
         suffix_pattern = r'\.(?P<%s>[a-z0-9]+)/?$' % suffix_kwarg
 
-    return apply_suffix_patterns(urlpatterns, suffix_pattern, suffix_required)
+    if path and register_converter:
+        converter_name, suffix_converter = _get_format_path_converter(suffix_kwarg, allowed)
+        register_converter(suffix_converter, converter_name)
+
+        suffix_route = '<%s:%s>' % (converter_name, suffix_kwarg)
+    else:
+        suffix_route = None
+
+    return apply_suffix_patterns(urlpatterns, suffix_pattern, suffix_required, suffix_route)

tests/test_urlpatterns.py

@@ -91,7 +91,7 @@ class FormatSuffixTests(TestCase):
             URLTestPath('/convtest/42', (), {'pk': 42}),
             URLTestPath('/convtest/42.api', (), {'pk': 42, 'format': 'api'}),
             URLTestPath('/convtest/42.asdf', (), {'pk': 42, 'format': 'asdf'}),
-            URLTestPath('/retest', (), {'pk': '42'}),
+            URLTestPath('/retest/42', (), {'pk': '42'}),
             URLTestPath('/retest/42.api', (), {'pk': '42', 'format': 'api'}),
             URLTestPath('/retest/42.asdf', (), {'pk': '42', 'format': 'asdf'}),
         ]