Index: django/core/urlresolvers.py =================================================================== --- django/core/urlresolvers.py (revision 4052) +++ django/core/urlresolvers.py (working copy) @@ -11,6 +11,14 @@ from django.core.exceptions import ImproperlyConfigured, ViewDoesNotExist import re +# Set up these regular expressions outside the function so they only have to +# be compiled once. +re_bracket = re.compile(r'(?(.*?)$', grouped) - if m: # If this was a named group... - # m.group(1) is the name of the group - # m.group(2) is the regex. - try: - value = self.kwargs[m.group(1)] - except KeyError: - # It was a named group, but the arg was passed in as a - # positional arg or not at all. - try: - value = self.args[self.current_arg] - self.current_arg += 1 - except IndexError: - # The arg wasn't passed in. - raise NoReverseMatch('Not enough positional arguments passed in') - test_regex = m.group(2) - else: # Otherwise, this was a positional (unnamed) group. - try: - value = self.args[self.current_arg] - self.current_arg += 1 - except IndexError: - # The arg wasn't passed in. - raise NoReverseMatch('Not enough positional arguments passed in') - test_regex = grouped - # Note we're using re.match here on purpose because the start of - # to string needs to match. - if not re.match(test_regex + '$', str(value)): # TODO: Unicode? - raise NoReverseMatch("Value %r didn't match regular expression %r" % (value, test_regex)) - return str(value) # TODO: Unicode? +class ReverseRegexLookup(object): + def __init__(self, text): + self.has_named_groups = bool(re_has_named_group.search(text)) + self._tokenize(text) + def _tokenize(self, text): + # Recursive tokenizer for regular expression parenthesis. + def parse(text): + bits = [] + m = re_bracket.search(text) + while m: + before, text = text[:m.start()], text[m.end():] + if before: + bits.append(before) + if m.group(1) != '(': + break + inner_bits, text = parse(text) + if inner_bits: + inline = self.has_named_groups + skip = False + first_bit = inner_bits[0] + if isinstance(first_bit, str): + if first_bit.startswith('?'): + # Regex extension notation. + if first_bit.startswith('?:'): + # No need to parse this non-grouping parenthesis. + inline = True + inner_bits[0] = first_bit[2:] + elif first_bit.startswith('?P'): + inline = False + else: + # Skip all other extension notation. + skip = True + else: + skip = True + if not skip: + if inline: + bits.extend(inner_bits) + else: + bits.append(inner_bits) + m = re_bracket.search(text) + return bits, text + self.minimum_arguments = 0 + bits, text = parse(text) + if text: + bits.append(text) + # Now tokenize the bits. Each token will either be a string or a regex. + tokens = [] + for bit in bits: + if isinstance(bit, list): + # We're building the regex here so it only has to be compiled + # once. + bit = re.compile('%s$' % build_re(bit)) + tokens.append(bit) + self.tokens = tokens + + def check(self, args=[], kwargs={}): + # Note: args and kwargs will be destroyed (using .pop()) so if you need + # to keep using them, pass copies. + if self.minimum_arguments > len(args) + len(kwargs): + raise NoReverseMatch('Not enough arguments passed in') + match = [] + args = list(args) + kwargs = kwargs.copy() + for token in self.tokens: + if isinstance(token, re_type): # A regex token. + value = None + # Is it a named argument? (test by looking for a groupindex) + named_argument = self.has_named_groups and token.groupindex.keys() + if named_argument: + try: + value = kwargs.pop(named_argument[0]) + except KeyError: + # It was a named group, but the arg was passed in as a + # positional arg or not at all. + pass + if value is None: + try: + value = args.pop(0) + except IndexError: + # The arg wasn't passed in. + raise NoReverseMatch('Not enough positional arguments passed in') + value = str(value) # TODO: Unicode? + if not token.match(value): + raise NoReverseMatch("Value %r didn't match regular expression %r" % (value, token.pattern)) + match.append(value) + else: # A string token. + match.append(token) + match = ''.join(match) + # Strip unused regular expression syntax. + match = re_unused.sub('', match) + # Unescape special characters which could possibly be used in a URL. + match = re_special.sub(r'\1', match) + return match + class RegexURLPattern(object): def __init__(self, regex, callback, default_args=None): # regex is a string representing a regular expression. @@ -94,6 +165,7 @@ # which represents the path to a module and a view function name, or a # callable object (view). self.regex = re.compile(regex) + self.reverse_regex_lookup = ReverseRegexLookup(regex) if callable(callback): self._callback = callback else: @@ -141,13 +213,14 @@ return self.reverse_helper(*args, **kwargs) def reverse_helper(self, *args, **kwargs): - return reverse_helper(self.regex, *args, **kwargs) + return self.reverse_regex_lookup.check(args, kwargs) class RegexURLResolver(object): def __init__(self, regex, urlconf_name, default_kwargs=None): # regex is a string representing a regular expression. # urlconf_name is a string representing the module containing urlconfs. self.regex = re.compile(regex) + self.reverse_regex_lookup = ReverseRegexLookup(regex) self.urlconf_name = urlconf_name self.callback = None self.default_kwargs = default_kwargs or {} @@ -220,8 +293,10 @@ raise NoReverseMatch def reverse_helper(self, lookup_view, *args, **kwargs): + result = self.reverse_regex_lookup.check(args, kwargs) + # .check() swallows used args, so the resolver is checking both itself + # and its children using the one set of arguments. sub_match = self.reverse(lookup_view, *args, **kwargs) - result = reverse_helper(self.regex, *args, **kwargs) return result + sub_match def resolve(path, urlconf=None): Index: tests/regressiontests/urlpatterns_reverse/tests.py =================================================================== --- tests/regressiontests/urlpatterns_reverse/tests.py (revision 4052) +++ tests/regressiontests/urlpatterns_reverse/tests.py (working copy) @@ -21,19 +21,25 @@ ('^people/(?P\w\w)/(?P\d)/$', NoReverseMatch, [], {'state': 'il', 'name': 'adrian'}), ('^people/(?P\w\w)/(?P\w+)/$', NoReverseMatch, [], {'state': 'il'}), ('^people/(?P\w\w)/(?P\w+)/$', NoReverseMatch, [], {'name': 'adrian'}), - ('^people/(?P\w\w)/(\w+)/$', NoReverseMatch, ['il'], {'name': 'adrian'}), - ('^people/(?P\w\w)/(\w+)/$', 'people/il/adrian/', ['adrian'], {'state': 'il'}), + # Even though the next match looks like it should have worked, if a URL has + # both named and unnamed groups, only named groups should be used. + ('^people/(?P\w\w)/(\w+)/$', 'people/il/\w/', ['adrian'], {'state': 'il'}), + ('^places?/$', 'places/', [], {}), + ('^people/(?:name/)?', 'people/name/', [], {}), + (r'^product/(?P\w+)\+\(\$(?P\d+(\.\d+)?)\)/$', 'product/chocolate+($2.00)/', ['2.00'], {'product': 'chocolate'}), + (r'^places/(\d+|[a-z_]+)/', 'places/4/', [4], {}), + (r'^places/(\d+|[a-z_]+)/', 'places/harlem/', ['harlem'], {}), ) class URLPatternReverse(unittest.TestCase): def test_urlpattern_reverse(self): for regex, expected, args, kwargs in test_data: try: - got = reverse_helper(re.compile(regex), *args, **kwargs) + got = reverse_helper(regex, *args, **kwargs) except NoReverseMatch, e: self.assertEqual(expected, NoReverseMatch) else: self.assertEquals(got, expected) if __name__ == "__main__": - run_tests(1) + unittest.main()