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

This commit is contained in:
Jack Cushman 2019-12-21 13:22:18 -05:00 committed by Mariusz Felisiak
parent ceecd0556d
commit eb629f4c02
5 changed files with 57 additions and 10 deletions

View File

@ -632,11 +632,18 @@ class URLResolver:
candidate_subs = kwargs candidate_subs = kwargs
# Convert the candidate subs to text using Converter.to_url(). # Convert the candidate subs to text using Converter.to_url().
text_candidate_subs = {} text_candidate_subs = {}
match = True
for k, v in candidate_subs.items(): for k, v in candidate_subs.items():
if k in converters: 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: else:
text_candidate_subs[k] = str(v) text_candidate_subs[k] = str(v)
if not match:
continue
# WSGI provides decoded URLs, without %xx escapes, and the URL # WSGI provides decoded URLs, without %xx escapes, and the URL
# resolver operates on such URLs. First substitute arguments # resolver operates on such URLs. First substitute arguments
# without quoting to build a decoded URL and look for a match. # without quoting to build a decoded URL and look for a match.

View File

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

View File

@ -156,7 +156,14 @@ A converter is a class that includes the following:
user unless another URL pattern matches. user unless another URL pattern matches.
* A ``to_url(self, value)`` method, which handles converting the Python type * 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:: For example::
@ -666,7 +673,9 @@ included at all).
You may also use the same name for multiple URL patterns if they differ in 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()` 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: .. _topics-http-defining-url-namespaces:

View File

@ -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 = [ urlpatterns = [
# Different number of arguments. # Different number of arguments.
@ -18,4 +20,15 @@ urlpatterns = [
# Different regular expressions. # Different regular expressions.
re_path(r'^regex/uppercase/([A-Z]+)/', views.empty_view, name='regex'), re_path(r'^regex/uppercase/([A-Z]+)/', views.empty_view, name='regex'),
re_path(r'^regex/lowercase/([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',
),
] ]

View File

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