From 15b0158e393544bd215688f257ab03604cd07950 Mon Sep 17 00:00:00 2001 From: Malcolm Tredinnick Date: Sat, 27 Sep 2008 06:14:11 +0000 Subject: [PATCH] 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 | 38 ++++++++++--------- .../urlpatterns_reverse/tests.py | 11 ++++++ .../urlpatterns_reverse/urls.py | 9 ++++- .../urlpatterns_reverse/views.py | 6 +++ 4 files changed, 45 insertions(+), 19 deletions(-) diff --git a/django/core/urlresolvers.py b/django/core/urlresolvers.py index 0aef3e91c3..5b62b88578 100644 --- a/django/core/urlresolvers.py +++ b/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[pattern.name] = bits, p_pattern + self._reverse_dict.appendlist(pattern.callback, (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, [(), ()]) - for result, params in possibilities: - if args: - if len(args) != len(params): - continue - unicode_args = [force_unicode(val) for val in args] - candidate = result % dict(zip(params, unicode_args)) - 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 + possibilities = self.reverse_dict.getlist(lookup_view) + for possibility, pattern in possibilities: + for result, params in possibility: + if args: + if len(args) != len(params): + continue + unicode_args = [force_unicode(val) for val in args] + candidate = result % dict(zip(params, unicode_args)) + 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)) diff --git a/tests/regressiontests/urlpatterns_reverse/tests.py b/tests/regressiontests/urlpatterns_reverse/tests.py index f743af36dd..6ccf1443dc 100644 --- a/tests/regressiontests/urlpatterns_reverse/tests.py +++ b/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): diff --git a/tests/regressiontests/urlpatterns_reverse/urls.py b/tests/regressiontests/urlpatterns_reverse/urls.py index 0f66346337..b3aed4f22d 100644 --- a/tests/regressiontests/urlpatterns_reverse/urls.py +++ b/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\d+)/$', 'kwargs_view'), + url(r'absolute_arg_view/(?P\d+)/$', absolute_kwargs_view), + url(r'absolute_arg_view/$', absolute_kwargs_view), + ) diff --git a/tests/regressiontests/urlpatterns_reverse/views.py b/tests/regressiontests/urlpatterns_reverse/views.py index 65be705023..99c00bde70 100644 --- a/tests/regressiontests/urlpatterns_reverse/views.py +++ b/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