Fixed #9038 -- Correctly handle URL patterns with the same name (or view name),

declared independently and that differ only by argument signatures.

Patch from Russell Keith-Magee.


git-svn-id: http://code.djangoproject.com/svn/django/trunk@9087 bcc190cf-cafb-0310-a4f2-bffc1f526a37

django/core/urlresolvers.py

@@ -11,6 +11,7 @@ import re
 
 from django.http import Http404
 from django.core.exceptions import ImproperlyConfigured, ViewDoesNotExist
+from django.utils.datastructures import MultiValueDict
 from django.utils.encoding import iri_to_uri, force_unicode, smart_str
 from django.utils.functional import memoize
 from django.utils.regex_helper import normalize
@@ -144,7 +145,7 @@ class RegexURLResolver(object):
         self.urlconf_name = urlconf_name
         self.callback = None
         self.default_kwargs = default_kwargs or {}
-        self._reverse_dict = {}
+        self._reverse_dict = MultiValueDict()
 
     def __repr__(self):
         return '<%s %s %s>' % (self.__class__.__name__, self.urlconf_name, self.regex.pattern)
@@ -162,11 +163,11 @@ class RegexURLResolver(object):
                         for piece, p_args in parent:
                             new_matches.extend([(piece + suffix, p_args + args)
                                     for (suffix, args) in matches])
-                        self._reverse_dict[name] = new_matches, p_pattern + pat
+                        self._reverse_dict.appendlist(name, (new_matches, p_pattern + pat))
                 else:
                     bits = normalize(p_pattern)
-                    self._reverse_dict[pattern.callback] = bits, p_pattern
+                    self._reverse_dict.appendlist(pattern.callback, (bits, p_pattern))
-                    self._reverse_dict[pattern.name] = bits, p_pattern
+                    self._reverse_dict.appendlist(pattern.name, (bits, p_pattern))
         return self._reverse_dict
     reverse_dict = property(_get_reverse_dict)
 
@@ -223,20 +224,21 @@ class RegexURLResolver(object):
             lookup_view = get_callable(lookup_view, True)
         except (ImportError, AttributeError), e:
             raise NoReverseMatch("Error importing '%s': %s." % (lookup_view, e))
-        possibilities, pattern = self.reverse_dict.get(lookup_view, [(), ()])
+        possibilities = self.reverse_dict.getlist(lookup_view)
-        for result, params in possibilities:
+        for possibility, pattern in possibilities:
-            if args:
+            for result, params in possibility:
-                if len(args) != len(params):
+                if args:
-                    continue
+                    if len(args) != len(params):
-                unicode_args = [force_unicode(val) for val in args]
+                        continue
-                candidate =  result % dict(zip(params, unicode_args))
+                    unicode_args = [force_unicode(val) for val in args]
-            else:
+                    candidate =  result % dict(zip(params, unicode_args))
-                if set(kwargs.keys()) != set(params):
+                else:
-                    continue
+                    if set(kwargs.keys()) != set(params):
-                unicode_kwargs = dict([(k, force_unicode(v)) for (k, v) in kwargs.items()])
+                        continue
-                candidate = result % unicode_kwargs
+                    unicode_kwargs = dict([(k, force_unicode(v)) for (k, v) in kwargs.items()])
-            if re.search(u'^%s' % pattern, candidate, re.UNICODE):
+                    candidate = result % unicode_kwargs
-                return candidate
+                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))
 

tests/regressiontests/urlpatterns_reverse/tests.py

@@ -65,6 +65,17 @@ test_data = (
     ('extra-places', '/e-places/10/', ['10'], {}),
     ('extra-people', '/e-people/fred/', ['fred'], {}),
     ('extra-people', '/e-people/fred/', [], {'name': 'fred'}),
+
+    # Regression for #9038
+    # These views are resolved by method name. Each method is deployed twice -
+    # once with an explicit argument, and once using the default value on
+    # the method. This is potentially ambiguous, as you have to pick the
+    # correct view for the arguments provided.
+    ('kwargs_view', '/arg_view/', [], {}),
+    ('kwargs_view', '/arg_view/10/', [], {'arg1':10}),
+    ('regressiontests.urlpatterns_reverse.views.absolute_kwargs_view', '/absolute_arg_view/', [], {}),
+    ('regressiontests.urlpatterns_reverse.views.absolute_kwargs_view', '/absolute_arg_view/10/', [], {'arg1':10}),
+
 )
 
 class URLPatternReverse(TestCase):

tests/regressiontests/urlpatterns_reverse/urls.py

@@ -1,5 +1,5 @@
 from django.conf.urls.defaults import *
-from views import empty_view
+from views import empty_view, absolute_kwargs_view
 
 urlpatterns = patterns('',
     url(r'^places/(\d+)/$', empty_view, name='places'),
@@ -45,4 +45,11 @@ urlpatterns = patterns('',
 
     # This is non-reversible, but we shouldn't blow up when parsing it.
     url(r'^(?:foo|bar)(\w+)/$', empty_view, name="disjunction"),
+
+    # Regression views for #9038. See tests for more details
+    url(r'arg_view/$', 'kwargs_view'),
+    url(r'arg_view/(?P<arg1>\d+)/$', 'kwargs_view'),
+    url(r'absolute_arg_view/(?P<arg1>\d+)/$', absolute_kwargs_view),
+    url(r'absolute_arg_view/$', absolute_kwargs_view),
+
 )

tests/regressiontests/urlpatterns_reverse/views.py

@@ -1,2 +1,8 @@
 def empty_view(request, *args, **kwargs):
     pass
+
+def kwargs_view(request, arg1=1, arg2=2):
+    pass
+
+def absolute_kwargs_view(request, arg1=1, arg2=2):
+    pass