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:
(r"^(?P<parent>[a-zA-Z0-9_-])/something(?:/(?P<child>\d+))?/*$", something),
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.