refactor urlpattern simplification #373 #168

replace still incorrect regex with parsing state machine.
regex are not cut out for parenthesis counting.

drf_spectacular/plumbing.py

@@ -735,6 +735,51 @@ def modify_for_versioning(patterns, method, path, view, requested_version):
     return path
 
 
+def analyze_named_regex_pattern(path):
+    """ safely extract named groups and their pattern from given regex pattern """
+    result = {}
+    stack = 0
+    name_capture, name_buffer = False, ''
+    regex_capture, regex_buffer = False, ''
+    i = 0
+    while i < len(path):
+        # estimate state at position i
+        skip = False
+        if path[i] == '\\':
+            ff = 2
+        elif path[i:i + 4] == '(?P<':
+            skip = True
+            name_capture = True
+            ff = 4
+        elif path[i] in '(':
+            stack += 1
+            ff = 1
+        elif path[i] == '>' and name_capture:
+            assert name_buffer
+            name_capture = False
+            regex_capture = True
+            skip = True
+            ff = 1
+        elif path[i] in ')':
+            if regex_capture and not stack:
+                regex_capture = False
+                result[name_buffer] = regex_buffer
+                name_buffer, regex_buffer = '', ''
+            else:
+                stack -= 1
+            ff = 1
+        else:
+            ff = 1
+        # fill buffer based on state
+        if name_capture and not skip:
+            name_buffer += path[i:i + ff]
+        elif regex_capture and not skip:
+            regex_buffer += path[i:i + ff]
+        i += ff
+    assert not stack
+    return result
+
+
 def detype_pattern(pattern):
     """
     return an equivalent pattern that accepts arbitrary values for path parameters.
@@ -763,8 +808,14 @@ def detype_pattern(pattern):
             is_endpoint=pattern._is_endpoint
         )
     elif isinstance(pattern, RegexPattern):
+        detyped_regex = pattern._regex
+        for name, regex in analyze_named_regex_pattern(pattern._regex).items():
+            detyped_regex = detyped_regex.replace(
+                f'(?P<{name}>{regex})',
+                f'(?P<{name}>[^/]+)',
+            )
         return RegexPattern(
-            regex=re.sub(r'\(\?P<(\w+)>.+?\)', r'(?P<\1>[^/]+)', pattern._regex),
+            regex=detyped_regex,
             name=pattern.name,
             is_endpoint=pattern._is_endpoint
         )

tests/test_plumbing.py

@@ -1,4 +1,5 @@
 import json
+import re
 import sys
 import typing
 from datetime import datetime
@@ -11,7 +12,8 @@
 
 from drf_spectacular.openapi import AutoSchema
 from drf_spectacular.plumbing import (
-    detype_pattern, follow_field_source, force_instance, is_field, is_serializer, resolve_type_hint,
+    analyze_named_regex_pattern, detype_pattern, follow_field_source, force_instance, is_field,
+    is_serializer, resolve_type_hint,
 )
 from drf_spectacular.validation import validate_schema
 from tests import generate_schema
@@ -192,3 +194,17 @@ class XView(generics.RetrieveAPIView):
         serializer_class = XSerializer
 
     validate_schema(generate_schema('/x', view=XView))
+
+
+@pytest.mark.parametrize(['pattern', 'output'], [
+    ('(?P<t1><,()(())(),)', {'t1': '<,()(())(),'}),
+    (r'(?P<t1>.\\)', {'t1': r'.\\'}),
+    (r'(?P<t1>.\\\\)', {'t1': r'.\\\\'}),
+    (r'(?P<t1>.\))', {'t1': r'.\)'}),
+    (r'(?P<t1>)', {'t1': r''}),
+    (r'(?P<t1>.[\(]{2})', {'t1': r'.[\(]{2}'}),
+    (r'(?P<t1>(.))/\(t/(?P<t2>\){2}()\({2}().*)', {'t1': '(.)', 't2': r'\){2}()\({2}().*'}),
+])
+def test_analyze_named_regex_pattern(no_warnings, pattern, output):
+    re.compile(pattern)  # check validity of regex
+    assert analyze_named_regex_pattern(pattern) == output

tests/test_versioning.py

@@ -1,8 +1,10 @@
+import re
 from unittest import mock
 
 import pytest
 import yaml
 from django.conf.urls import include
+from django.db import models
 from django.urls import path, re_path
 from rest_framework import generics, mixins, routers, serializers, viewsets
 from rest_framework.test import APIClient, APIRequestFactory
@@ -100,28 +102,43 @@ def test_namespace_versioning(no_warnings, viewset_cls, version):
     assert_schema(schema, f'tests/test_versioning_{version}.yml')
 
 
-def test_namespace_versioning_urlpatterns_simplification(no_warnings):
+@pytest.mark.parametrize(['path_func', 'path_str', 'pattern', ], [
+    (path, '{id}/', '<int:pk>/'),
+    (path, '{id}/', '<pk>/'),
+    (re_path, '{id}/', r'(?P<pk>[0-9A-Fa-f-]+)/'),
+    (re_path, '{id}/', r'(?P<pk>[^/]+)/$'),
+    (re_path, '{id}/', r'(?P<pk>[a-z]{2}(-[a-z]{2})?)/'),
+    (re_path, '{field}/t/{id}/', r'^(?P<field>[^/.]+)/t/(?P<pk>[^/.]+)/'),
+    (re_path, '{field}/t/{id}/', r'^(?P<field>[A-Z\(\)]+)/t/(?P<pk>[^/.]+)/'),
+])
+def test_namespace_versioning_urlpatterns_simplification(no_warnings, path_func, path_str, pattern):
+    class LookupModel(models.Model):
+        field = models.IntegerField()
+
+    class LookupSerializer(serializers.ModelSerializer):
+        class Meta:
+            model = LookupModel
+            fields = '__all__'
+
     class NamespaceVersioningAPIView(generics.RetrieveUpdateDestroyAPIView):
         versioning_class = NamespaceVersioning
-        serializer_class = Xv1Serializer
-        queryset = SimpleModel.objects.all()
+        serializer_class = LookupSerializer
+        queryset = LookupModel.objects.all()
 
-    urls = (
-        path('x/<int:pk>/', NamespaceVersioningAPIView.as_view()),
-        path('y/<pk>/', NamespaceVersioningAPIView.as_view()),
-        re_path('z/(?P<pk>[0-9A-Fa-f-]+)/', NamespaceVersioningAPIView.as_view()),
-    )
+    # make sure regex are valid
+    if path_func == re_path:
+        re.compile(pattern)
+
+    patterns_v1 = [path_func(pattern, NamespaceVersioningAPIView.as_view())]
     generator = SchemaGenerator(
-        patterns=[path('v1/<int:some_param>/', include((urls, 'v1'))), ],
+        patterns=[path('v1/<int:some_param>/', include((patterns_v1, 'v1')))],
         api_version='v1',
     )
     schema = generator.get_schema(request=None, public=True)
 
-    for s in ['x', 'y', 'z']:
-        parameters = schema['paths'][f'/v1/{{some_param}}/{s}/{{id}}/']['get']['parameters']
-        parameters = {p['name']: p for p in parameters}
-        assert parameters['id']['schema']['type'] == 'integer'
-        assert parameters['some_param']['schema']['type'] == 'integer'
+    parameters = schema['paths']['/v1/{some_param}/' + path_str]['get']['parameters']
+    for p in parameters:
+        assert p['schema']['type'] == 'integer'
 
 
 @pytest.mark.parametrize('viewset_cls', [AcceptHeaderVersioningViewset, AcceptHeaderVersioningViewset2])