Code

Opened 5 years ago

Closed 5 years ago

#10798 closed (wontfix)

RegexUrlResolver's _get_reverse_dict appends objects of unknown type to the reverse dict

Reported by: fas Owned by: fas
Component: Core (Other) Version: 1.0
Severity: Keywords: url, urlresolver, regexurlpattern, regexurlresolver
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

In _get_reverse_dict:

if isinstance(pattern, RegexURLResolver):
    # ...
else:
    bits = normalize(p_pattern)
    self._reverse_dict.appendlist(pattern.callback, (bits, p_pattern))
    self._reverse_dict.appendlist(pattern.name, (bits, p_pattern))

In the else branch it is implicitly assumed that pattern is of type RegexURLPattern (it uses its properties). This does not have to be the case. For example, one could create a different kind of RegexURLResolver which does not inherit RegexURLResolver. This resolver would be handled as a RegexURLPattern instance and added to the reverse dict.
Patch is a short one: replace "else" with "elif isinstance(pattern, RegexURLPattern):".

Attachments (1)

urlresolvers_patch.diff (795 bytes) - added by fas 5 years ago.

Download all attachments as: .zip

Change History (3)

Changed 5 years ago by fas

comment:1 Changed 5 years ago by Alex

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Why? What's the usecase here?

comment:2 Changed 5 years ago by mtredinnick

  • Resolution set to wontfix
  • Status changed from new to closed

Whilst supporting more generic URL resolving classes is a goal, I don't think this is the solution. We need to fix the reversing support to allow handing off to lower-level classes in those cases and this means coming up with an internal interface protocol for what they need to support (probably just a reverse method, but it needs a bit of planning). So I'm going to wontfix this particular item and then in the 1.2 timeframe or thereabouts, we can work out a general plan for this. The current ticket and patch assume a solution, rather than necessarily fleshing out that it's solving the right problem and I think the problem is a little broader than that.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.