Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#29415 closed Bug (fixed)

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

Reported by: Xaroth Owned by: nobody
Component: Core (URLs) Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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).

Change History (7)

comment:1 by Xaroth, 6 years ago

Version: 2.1master

comment:2 by Xaroth, 6 years ago

Added an initial test case for this ticket: https://github.com/Xaroth/django/tree/ticket_29415

comment:3 by Xaroth, 6 years ago

Has patch: set

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.

comment:4 by Claude Paroz, 6 years ago

Triage Stage: UnreviewedAccepted

comment:5 by Tim Graham <timograham@…>, 6 years ago

Resolution: fixed
Status: newclosed

In 39283c8e:

Fixed #29415 -- Fixed detection of custom URL converters in included patterns.

comment:6 by Tim Graham <timograham@…>, 6 years ago

In 39e61669:

[2.1.x] Fixed #29415 -- Fixed detection of custom URL converters in included patterns.

Backport of 39283c8edbc5991b589d48a8e17152042193f2df from master

comment:7 by Tim Graham <timograham@…>, 6 years ago

In 1adac35:

[2.0.x] Fixed #29415 -- Fixed detection of custom URL converters in included patterns.

Backport of 39283c8edbc5991b589d48a8e17152042193f2df from master

Note: See TracTickets for help on using tickets.
Back to Top