Fixed #30995 -- Allowed converter.to_url() to raise ValueError to indicate no match.

django/urls/resolvers.py

@@ -632,11 +632,18 @@ def _reverse_with_prefix(self, lookup_view, _prefix, *args, **kwargs):
                     candidate_subs = kwargs
                 # Convert the candidate subs to text using Converter.to_url().
                 text_candidate_subs = {}
+                match = True
                 for k, v in candidate_subs.items():
                     if k in converters:
-                        text_candidate_subs[k] = converters[k].to_url(v)
+                        try:
+                            text_candidate_subs[k] = converters[k].to_url(v)
+                        except ValueError:
+                            match = False
+                            break
                     else:
                         text_candidate_subs[k] = str(v)
+                if not match:
+                    continue
                 # WSGI provides decoded URLs, without %xx escapes, and the URL
                 # resolver operates on such URLs. First substitute arguments
                 # without quoting to build a decoded URL and look for a match.

docs/releases/3.1.txt

@@ -294,7 +294,8 @@ Tests
 URLs
 ~~~~
 
-* ...
+* :ref:`Path converters <registering-custom-path-converters>` can now raise
+  ``ValueError`` in ``to_url()`` to indicate no match when reversing URLs.
 
 Utilities
 ~~~~~~~~~

docs/topics/http/urls.txt

@@ -156,7 +156,14 @@ A converter is a class that includes the following:
   user unless another URL pattern matches.
 
 * A ``to_url(self, value)`` method, which handles converting the Python type
-  into a string to be used in the URL.
+  into a string to be used in the URL. It should raise ``ValueError`` if it
+  can't convert the given value. A ``ValueError`` is interpreted as no match
+  and as a consequence :func:`~django.urls.reverse` will raise
+  :class:`~django.urls.NoReverseMatch` unless another URL pattern matches.
+
+  .. versionchanged:: 3.1
+
+    Support for raising ``ValueError`` to indicate no match was added.
 
 For example::
 
@@ -666,7 +673,9 @@ included at all).
 
 You may also use the same name for multiple URL patterns if they differ in
 their arguments. In addition to the URL name, :func:`~django.urls.reverse()`
-matches the number of arguments and the names of the keyword arguments.
+matches the number of arguments and the names of the keyword arguments. Path
+converters can also raise ``ValueError`` to indicate no match, see
+:ref:`registering-custom-path-converters` for details.
 
 .. _topics-http-defining-url-namespaces:
 

tests/urlpatterns/path_same_name_urls.py

@@ -1,6 +1,8 @@
-from django.urls import path, re_path
+from django.urls import path, re_path, register_converter
 
-from . import views
+from . import converters, views
+
+register_converter(converters.DynamicConverter, 'to_url_value_error')
 
 urlpatterns = [
     # Different number of arguments.
@@ -18,4 +20,15 @@
     # Different regular expressions.
     re_path(r'^regex/uppercase/([A-Z]+)/', views.empty_view, name='regex'),
     re_path(r'^regex/lowercase/([a-z]+)/', views.empty_view, name='regex'),
+    # converter.to_url() raises ValueError (no match).
+    path(
+        'converter_to_url/int/<value>/',
+        views.empty_view,
+        name='converter_to_url',
+    ),
+    path(
+        'converter_to_url/tiny_int/<to_url_value_error:value>/',
+        views.empty_view,
+        name='converter_to_url',
+    ),
 ]

tests/urlpatterns/tests.py

@@ -3,7 +3,7 @@
 from django.core.exceptions import ImproperlyConfigured
 from django.test import SimpleTestCase
 from django.test.utils import override_settings
-from django.urls import Resolver404, path, resolve, reverse
+from django.urls import NoReverseMatch, Resolver404, path, resolve, reverse
 
 from .converters import DynamicConverter
 from .views import empty_view
@@ -203,6 +203,12 @@ def test_nonmatching_urls(self):
 @override_settings(ROOT_URLCONF='urlpatterns.path_same_name_urls')
 class SameNameTests(SimpleTestCase):
     def test_matching_urls_same_name(self):
+        @DynamicConverter.register_to_url
+        def requires_tiny_int(value):
+            if value > 5:
+                raise ValueError
+            return value
+
         tests = [
             ('number_of_args', [
                 ([], {}, '0/'),
@@ -227,6 +233,10 @@ def test_matching_urls_same_name(self):
                 (['ABC'], {}, 'uppercase/ABC/'),
                 (['abc'], {}, 'lowercase/abc/'),
             ]),
+            ('converter_to_url', [
+                ([6], {}, 'int/6/'),
+                ([1], {}, 'tiny_int/1/'),
+            ]),
         ]
         for url_name, cases in tests:
             for args, kwargs, url_suffix in cases:
@@ -272,9 +282,16 @@ def raises_type_error(value):
         with self.assertRaisesMessage(TypeError, 'This type error propagates.'):
             resolve('/dynamic/abc/')
 
-    def test_reverse_value_error_propagates(self):
+    def test_reverse_value_error_means_no_match(self):
         @DynamicConverter.register_to_url
         def raises_value_error(value):
-            raise ValueError('This value error propagates.')
-        with self.assertRaisesMessage(ValueError, 'This value error propagates.'):
+            raise ValueError
+        with self.assertRaises(NoReverseMatch):
+            reverse('dynamic', kwargs={'value': object()})
+
+    def test_reverse_type_error_propagates(self):
+        @DynamicConverter.register_to_url
+        def raises_type_error(value):
+            raise TypeError('This type error propagates.')
+        with self.assertRaisesMessage(TypeError, 'This type error propagates.'):
             reverse('dynamic', kwargs={'value': object()})