[Django] #29415: custom url converters are not picked up on reverse when part of included patterns
Django
patterns
---------------------------------------+------------------------
Reporter: Xaroth | Owner: nobody
Type: Bug | Status: new
Component: Core (URLs) | Version: 2.1
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
---------------------------------------+------------------------
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
To demonstrate:
{{{
from django.urls import path, include, register_converter
from django.urls.converters import StringConverter
class ReverseConverter(StringConverter):
def to_python(self, value):
"""More complex use cases are possible, but this simple case of
reversing the string already shows it in effect"""
return value[::-1]
def to_url(self, obj):
"""More complex use cases are possible, but this simple case of
reversing the string already shows it in effect"""
return obj[::-1]
register_converter(ReverseConverter, 'rstr')
def noop(request): # A noop since we are merely testing reverse()
pass
# Patterns in here will
included_patterns = [
path('', noop, name='fail-case-1'),
path('<rstr:itemb>', noop, name='fail-case-2'),
]
direct_patterns = [
path('<rstr:item>', noop, name='success-case-1'),
path('<rstr:item>/<rstr:itemb>', noop, name='success-case-2'),
]
urlpatterns = [
path('<rstr:item>/', include(included_patterns)),
path('', include(direct_patterns)),
path('<rstr:item>', noop, name='success-case-3'),
]
}}}
When running the following test, both patterns -should- return the same
URL, however, they do not:
{{{
In [1]: from django.urls import reverse
In [2]: reverse('success-case-1', kwargs={'item': 'abc123'}) # Expected:
'/321cba'
Out[2]: '/321cba'
In [3]: reverse('success-case-2', kwargs={'item': 'abc123', 'itemb':
'xyz789'}) # Expected: '/321cba/987zyx'
Out[3]: '/321cba/987zyx'
In [4]: reverse('success-case-3', kwargs={'item': 'abc123'}) # Expected:
'/321cba'
Out[4]: '/321cba'
In [5]: reverse('fail-case-1', kwargs={'item': 'abc123'}) # Expected:
'/321cba/'
Out[5]: '/abc123/'
In [6]: reverse('fail-case-2', kwargs={'item': 'abc123', 'itemb':
'xyz789'}) # Expected: '/321cba/987zyx'
Out[6]: '/abc123/987zyx'
}}}
Note that at the last case, `item` is not reversed, where `itemb` is.
The main culprit (I think) is that when populating the URLResolver's
reverse_dict, included sub-resolvers are not updated with the converters
of the resolver that includes them, as such, they skip the converters for
the base resolver when reversing the URL.
I have confirmed this inconsistency in versions 2.0 through 2.0.5, 2.1a1
as well as master (@9f6188b).
--
Ticket URL: <https://code.djangoproject.com/ticket/29415>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
patterns
Reporter: Xaroth | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Xaroth):
* version: 2.1 => master
--
Ticket URL: <https://code.djangoproject.com/ticket/29415#comment:1>
Django
patterns
Reporter: Xaroth | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Xaroth):
Added an initial test case for this ticket:
https://github.com/Xaroth/django/tree/ticket_29415
--
Ticket URL: <https://code.djangoproject.com/ticket/29415#comment:2>
Django
patterns
Reporter: Xaroth | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Xaroth):
* has_patch: 0 => 1
Comment:
Added a patch in the form of a pull request:
https://github.com/django/django/pull/9965
Main changes made:
- Push converters from parent sections onto namespace url resolvers when
reverse()'ing
- Add converters from the base url pattern to sub-urls for included
urlpatterns
- Add tests for reversing custom url converters
I am not entirely sure on the cleanliness of the method used, so it might
need reviewing.
--
Ticket URL: <https://code.djangoproject.com/ticket/29415#comment:3>
Django
patterns
Reporter: Xaroth | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Claude Paroz):
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/29415#comment:4>
Django
patterns
Reporter: Xaroth | Owner: nobody
Type: Bug | Status: closed
Component: Core (URLs) | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"39283c8edbc5991b589d48a8e17152042193f2df" 39283c8e]:
{{{
#!CommitTicketReference repository=""
revision="39283c8edbc5991b589d48a8e17152042193f2df"
Fixed #29415 -- Fixed detection of custom URL converters in included
patterns.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29415#comment:5>
Django
patterns
Reporter: Xaroth | Owner: nobody
Type: Bug | Status: closed
Component: Core (URLs) | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"39e61669e0c4299228dca751852002946ebe5d5e" 39e61669]:
{{{
#!CommitTicketReference repository=""
revision="39e61669e0c4299228dca751852002946ebe5d5e"
[2.1.x] Fixed #29415 -- Fixed detection of custom URL converters in
included patterns.
Backport of 39283c8edbc5991b589d48a8e17152042193f2df from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29415#comment:6>
Django
patterns
Reporter: Xaroth | Owner: nobody
Type: Bug | Status: closed
Component: Core (URLs) | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"1adac352d3cd64d2193f9cd56e1e97b0022d6c48" 1adac35]:
{{{
#!CommitTicketReference repository=""
revision="1adac352d3cd64d2193f9cd56e1e97b0022d6c48"
[2.0.x] Fixed #29415 -- Fixed detection of custom URL converters in
included patterns.
Backport of 39283c8edbc5991b589d48a8e17152042193f2df from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29415#comment:7>