Opened 13 years ago

Closed 5 months ago

#14761 closed New feature (needsinfo)

URL resolving / reversing design doesn't allow alternate specs

Reported by: samuel337 Owned by: Marten Kenbeek
Component: Core (URLs) Version: dev
Severity: Normal Keywords: url resolve reverse
Cc: marten.knbk@… Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Django's URL resolution system is currently based on regexps and is represented by the RegexURLPattern class. There are cases where it would be preferable to subclass this and allow alternate ways of specifying URL patterns that ultimately compile down to regexps.

Right now, this works for URL resolving as during resolution, the resolve method on the RegexURLPattern class is called, and hence subclasses can modify that behaviour. It however, doesn't work for URL reversal, because the resolver class generates the possible matches itself, instead of calling a method on RegexURLPattern or its subclasses as is the case with URL resolution.

The attached patch simply refactors urlresolvers.py so the resolver calls a method on RegexURLPattern to get possible matches. It passes all the URL tests in Django, is completely backwards-compatible and introduces no new issues or quirks.

I don't believe any new tests are required, and because the new method on RegexURLPattern is marked private (hence subject to change), no new docs are required either. This should stay as an unsupported way to extend Django's URL system until it is fully revamped (support disjunctives, URI templates etc.).

The relevant thread on django-developers is here - http://groups.google.com/group/django-developers/browse_thread/thread/c94b551ebd57fbe6/65d79a336fef04b2

Attachments (1)

ticket-14761.diff (1.4 KB ) - added by samuel337 13 years ago.

Download all attachments as: .zip

Change History (9)

by samuel337, 13 years ago

Attachment: ticket-14761.diff added

comment:1 by Russell Keith-Magee, 13 years ago

Needs tests: set
Patch needs improvement: set
Triage Stage: UnreviewedAccepted

Of course tests are required; you're adding a new piece of functionality. You need to test that the new functionality works, and will continue to work. As it stands, if this patch were applied, a future patch could go back to just calling normalize instead of calling get_possibilities, and the test suite would continue to run, but any custom URLPattern classes would be broken.

comment:2 by anonymous, 13 years ago

Severity: Normal
Type: New feature

comment:3 by Aymeric Augustin, 12 years ago

UI/UX: unset

Change UI/UX from NULL to False.

comment:4 by Aymeric Augustin, 12 years ago

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:5 by Aymeric Augustin, 11 years ago

Component: Core (Other)Core (URLs)

comment:6 by Marten Kenbeek, 9 years ago

Cc: marten.knbk@… added
Has patch: unset
Needs tests: unset
Owner: changed from nobody to Marten Kenbeek
Patch needs improvement: unset
Status: newassigned

comment:7 by Tom Carrick, 5 months ago

I'm not sure how much need for this ticket there is now that we have path converters?

in reply to:  7 comment:8 by Mariusz Felisiak, 5 months ago

Resolution: needsinfo
Status: assignedclosed
Triage Stage: AcceptedUnreviewed

Replying to Tom Carrick:

I'm not sure how much need for this ticket there is now that we have path converters?

That's true. Resolving URLs is not the same as it was 8 years ago, RegexURLResolver doesn't exist anymore, we have path converters etc. This ticket should be redefined if any changes still need to be made

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