diff --git a/django/urls/resolvers.py b/django/urls/resolvers.py index 120e0396af..6e3f2443c9 100644 --- a/django/urls/resolvers.py +++ b/django/urls/resolvers.py @@ -632,11 +632,18 @@ class URLResolver: 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. diff --git a/docs/releases/3.1.txt b/docs/releases/3.1.txt index cdacbd71cd..3def8920cc 100644 --- a/docs/releases/3.1.txt +++ b/docs/releases/3.1.txt @@ -294,7 +294,8 @@ Tests URLs ~~~~ -* ... +* :ref:`Path converters ` can now raise + ``ValueError`` in ``to_url()`` to indicate no match when reversing URLs. Utilities ~~~~~~~~~ diff --git a/docs/topics/http/urls.txt b/docs/topics/http/urls.txt index ef9d4c007c..efe61d041c 100644 --- a/docs/topics/http/urls.txt +++ b/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: diff --git a/tests/urlpatterns/path_same_name_urls.py b/tests/urlpatterns/path_same_name_urls.py index 8eee949316..7b4fd2e627 100644 --- a/tests/urlpatterns/path_same_name_urls.py +++ b/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 @@ urlpatterns = [ # 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//', + views.empty_view, + name='converter_to_url', + ), + path( + 'converter_to_url/tiny_int//', + views.empty_view, + name='converter_to_url', + ), ] diff --git a/tests/urlpatterns/tests.py b/tests/urlpatterns/tests.py index 95bea58339..0de0f6f05d 100644 --- a/tests/urlpatterns/tests.py +++ b/tests/urlpatterns/tests.py @@ -3,7 +3,7 @@ import uuid 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 @@ class ConverterTests(SimpleTestCase): @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 @@ class SameNameTests(SimpleTestCase): (['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 @@ class ConversionExceptionTests(SimpleTestCase): 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()})