NoReverseMatch for optional arguments
|Reported by:||Owned by:||Malcolm Tredinnick|
|Cc:||Triage Stage:||Design decision needed|
|Has patch:||no||Needs documentation:||no|
|Needs tests:||no||Patch needs improvement:||no|
A number of URLs of my site have the form /<object_name>/something. The objects can have several representations, one of which is canonical, so I have a decorator that redirects requests with non-canonical names. The decorator (simplified) looks like this:
def url_from_view(view, **kwargs): return django.core.urlresolvers.reverse(view, kwargs=kwargs) def decorator(view): def wrapper(request, obj_name, **kwargs): obj = load_object(obj_name) if canonical_name(obj) != obj_name: return HttpResponseRedirect(url_from_view(wrapper, obj_name=canonical_name(obj), **kwargs) return view(request, obj, obj_name, **kwargs) return wrapper
Everything worked excellently, until I added something like this to my urlpatterns:
For /parent_noncanonical/something/42 it calls something(parent="parent_noncanonical", child="42") which then reverses the URL and redirects correctly to /parent_canonical/something/42.
However, if the child part is omitted, because it’s optional in the regex, as in /parent_noncanonical/something, the view gets called as something(parent="parent_noncanonical", child=None) which then fails to reverse.
The problem is that the reverse operation turns out not to be defined for some of data that the regular expressions can produce: when I pass it the same view and the same data that have been chosen by the urlpatterns, it fails to produce a reverse match.
The reverse matching engine should handle None parameters by finding matches which do not contain the corresponding groups. For example, for r"(?P<first>[a-z])?(?P<second>[0-9])?(?P<third>[A-Z])?" and first="d", second=None, third="J" the correct reverse match is "dJ".
Here’s the relevant Django code (core/urlresolvers.py, 227–241):
for result, params in possibilities: if args: # skipped else: if set(kwargs.keys()) != set(params): continue unicode_kwargs = dict([(k, force_unicode(v)) for (k, v) in kwargs.items()]) candidate = result % unicode_kwargs if re.search(u'^%s' % pattern, candidate, re.UNICODE): return candidate raise NoReverseMatch("Reverse for '%s' with arguments '%s' and keyword " "arguments '%s' not found." % (lookup_view, args, kwargs))
For the URL pattern above, the possibilities are correctly enumerated:
[(u'%(parent)s/something', parent?), (u'%(parent)s/something/%(child)s', ['parent', 'child'])]
But then it fails to take into account the fact child is None, and chooses the wrong possibility.
Perhaps simply eliminating None by changing set(kwargs.keys()) != set(params) to set([k for (k, v) in kwargs.items() if v]) != set(params) could do the job.
Change History (9)
comment:1 Changed 8 years ago by
|Patch needs improvement:||unset|
comment:4 Changed 8 years ago by
|Status:||closed → reopened|
|Triage Stage:||Unreviewed → Accepted|
comment:5 Changed 8 years ago by
|Owner:||changed from nobody to Malcolm Tredinnick|
|Status:||reopened → new|