custom url converters are not picked up on reverse when part of included patterns with namespace

[Eric Brandwein]

Under normal circumstances, url converters to_url methods are called when reversing a url, however, this behavior is inconsistent when used with included sub-patterns, when these sub-patterns have themselves included sub-patterns with a namespace assigned.

To demonstrate:

from django.urls import path, include, register_converter
from django.urls.converters import StringConverter


class ReverseStringConverter(StringConverter):
    """More complex use cases are possible, but this simple case of reversing the string already shows it in effect"""
    def to_python(self, value):
        return value[::-1]

    def to_url(self, value):
        return value[::-1]


# We just want to test the reverse()
def noop(request, **kwargs):
    pass


register_converter(ReverseStringConverter, 'reverse')

second_layer_urlpatterns = [
    path('', noop, name='failure-view'),
]

first_layer_urlpatterns = [
    path('', include(
        (second_layer_urlpatterns, 'app_name'), namespace='ns')),
    path('', include(second_layer_urlpatterns)),
    path('', noop, name='success-view'),
]

urlpatterns = [
    path('<reverse:param>/', include(first_layer_urlpatterns)),
]

When running the following test, reverse works fine when reversing failure-view or success-view, but not when reversing ns:failure-view:

>>> from django.urls import reverse
>>> reverse('failure-view', args=['foo'])
'/oof/'
>>> reverse('ns:failure-view', args=['foo'])
'/foo/'
>>> reverse('success-view', args=['foo'])
'/oof/'

It probably is very similar to #29415. I tested it in version 2.1.1.

[Carlton Gibson]

Hi Eric. Can I ask for a bit more info here?

Maybe I'm just missing it, but it looks to me as if 39283c8edbc5991b589d48a8e17152042193f2df (the fix for #29415) added a test for exactly this case.

• ​Namepaced pattern.
• ​Test case data using namepace.
• ​Check that it reverses correctly.

Not attempting any reuse of the above test, this diff passes:

diff --git a/tests/urlpatterns/path_base64_urls.py b/tests/urlpatterns/path_base64_urls.py
index 9b69f929fe..52a4045281 100644
--- a/tests/urlpatterns/path_base64_urls.py
+++ b/tests/urlpatterns/path_base64_urls.py
@@ -12,4 +12,5 @@ urlpatterns = [
     path('base64/<base64:value>/', views.empty_view, name='base64'),
     path('base64/<base64:base>/subpatterns/', include(subpatterns)),
     path('base64/<base64:base>/namespaced/', include((subpatterns, 'namespaced-base64'))),
+    path('base64/<base64:base>/namespaced/', include((subpatterns, 'namespaced-base64'), namespace='instance-ns-base64')),
 ]
diff --git a/tests/urlpatterns/tests.py b/tests/urlpatterns/tests.py
index b3d97ec5b9..f108fdfb55 100644
--- a/tests/urlpatterns/tests.py
+++ b/tests/urlpatterns/tests.py
@@ -70,6 +70,16 @@ class SimplifiedURLTests(SimpleTestCase):
                 url = reverse(url_name, kwargs=kwargs)
                 self.assertEqual(url, expected)
 
+    @override_settings(ROOT_URLCONF='urlpatterns.path_base64_urls')
+    def test_converter_reverse_with_instance_namespace(self):
+        expected = '/base64/aGVsbG8=/namespaced/d29ybGQ=/'
+        url_name = 'subpattern-base64'
+        instance_ns = 'instance-ns-base64'
+        kwargs = included_kwargs
+        url_name = '%s:%s' % (instance_ns, url_name)
+        url = reverse(url_name, kwargs=kwargs)
+        self.assertEqual(url, expected)
+
     def test_path_inclusion_is_matchable(self):
         match = resolve('/included_urls/extra/something/')
         self.assertEqual(match.url_name, 'inner-extra')

Are you able to adjust the test case there to show your issue in action?

Thanks!

[Eric Brandwein]

Hi Carlton, the issue happens when you have some URL with a custom converter, in which you are including other URLs without passing a namespace to include(). These other URLs, in turn, include another list of URLs, this time with a namespace. The tests you mention seem to just test namespacing from the first level of URLs, instead of from just the second, like in the example I gave. It would seem that when using instance namespaces, the converters are only searched from one include() upwards, instead of from all the include() "tree".

I made a test for it here: ​https://github.com/ericbrandwein/django/commit/de60d4f200ef08573f92942f0269a4b22a878290. The diff is:

diff --git a/tests/urlpatterns/path_base64_urls.py b/tests/urlpatterns/path_base64_urls.py
index 9b69f929fe..c9b4f6f9ca 100644
--- a/tests/urlpatterns/path_base64_urls.py
+++ b/tests/urlpatterns/path_base64_urls.py
@@ -4,8 +4,14 @@ from . import converters, views
 
 register_converter(converters.Base64Converter, 'base64')
 
+subsubpatterns = [
+    path('<base64:last_value>/', views.empty_view, name='subsubpattern-base64'),
+]
+
 subpatterns = [
     path('<base64:value>/', views.empty_view, name='subpattern-base64'),
+    path('<base64:value>/',
+        include((subsubpatterns, 'second-layer-namespaced-base64'), 'instance-ns-base64')),
 ]
 
 urlpatterns = [
diff --git a/tests/urlpatterns/tests.py b/tests/urlpatterns/tests.py
index b3d97ec5b9..8483d225d5 100644
--- a/tests/urlpatterns/tests.py
+++ b/tests/urlpatterns/tests.py
@@ -70,6 +70,17 @@ class SimplifiedURLTests(SimpleTestCase):
                 url = reverse(url_name, kwargs=kwargs)
                 self.assertEqual(url, expected)
 
+    @override_settings(ROOT_URLCONF='urlpatterns.path_base64_urls')
+    def test_converter_reverse_with_second_layer_instance_namespace(self):
+        expected = '/base64/aGVsbG8=/namespaced/d29ybGQ=/d29ybGQ=/'
+        url_name = 'subsubpattern-base64'
+        instance_ns = 'instance-ns-base64'
+        kwargs = included_kwargs
+        kwargs['last_value'] = b'world'
+        url_name = '%s:%s' % (instance_ns, url_name)
+        url = reverse(url_name, kwargs=kwargs)
+        self.assertEqual(url, expected)
+
     def test_path_inclusion_is_matchable(self):
         match = resolve('/included_urls/extra/something/')
         self.assertEqual(match.url_name, 'inner-extra')

Maybe this test is somewhat ugly, but it shows the bug. It fails with this message:

django.urls.exceptions.NoReverseMatch: Reverse for 'subsubpattern-base64' with keyword arguments '{'base': b'hello', 'value': b'world', 'last_value': b'world'}' not found. 1 pattern(s) tried: ['base64/(?P<base>[a-zA-Z0-9+/]*={0,2})/subpatterns/(?P<value>[a-zA-Z0-9+/]*={0,2})/(?P<last_value>[a-zA-Z0-9+/]*={0,2})/$']

Which means that the converter's regex is being used, but not its to_url function.

[Carlton Gibson]

Super. Good follow-up! Will review today. Thanks.

[Carlton Gibson]

OK, yes. Good. The test case reproduces — thank you for that!

(Adding the nested instance namespace 'instance-ns-base64' for a nested include is a little strange no? But taking it out doesn't affect the test: removing it and reversing second-layer-namespaced-base64:subsubpattern-base64 has exactly the same error.)

[Eric Brandwein]

Great!

I personally was using the nested instance namespace to be able to use the subsubpatterns in two different includes. In my case, the application namespace of these was declared in another file, along with the subsubpatterns. So I think using it that way wasn't as weird as this one.

BTW, I'm sorry for any grammar mistakes I may have had; English is not my first language :/

[Tim Graham]

​PR

[Tim Graham <timograham@…>]

In b0b4aac5:

Fixed #29775 -- Fixed URL converters in a nested namespaced path.

When using include() without namespaces of some urlpatterns that
have an include() with namespace, the converters of the parent
include() weren't being used to convert the arguments of reverse().