Exact URL Matching. Allow Syntactically Correct Patterns. Better Tests

lib/routing.py

@@ -81,7 +81,17 @@ def route_for(self, path):
         if path.startswith(self.base_url):
             path = path.split(self.base_url, 1)[1]
 
-        for view_fun, rules in iter(list(self._rules.items())):
+        # only list convert once
+        list_rules = list(self._rules.items())
+
+        # first, search for exact matches
+        for view_fun, rules in iter(list_rules):
+            for rule in rules:
+                if rule.exact_match(path):
+                    return view_fun
+
+        # then, search for regex matches
+        for view_fun, rules in iter(list_rules):
             for rule in rules:
                 if rule.match(path) is not None:
                     return view_fun
@@ -96,8 +106,8 @@ def url_for(self, func, *args, **kwargs):
                 path = rule.make_path(*args, **kwargs)
                 if path is not None:
                     return self.url_for_path(path)
-        raise RoutingError("No known paths to '{0}' with args {1} and "
-                           "kwargs {2}".format(func.__name__, args, kwargs))
+        raise RoutingError("No known paths to '{0}' with args {1} "
+                           "and kwargs {2}".format(func.__name__, args, kwargs))
 
     def url_for_path(self, path):
         """
@@ -133,27 +143,41 @@ def redirect(self, path):
         self._dispatch(path)
 
     def _dispatch(self, path):
-        for view_func, rules in iter(list(self._rules.items())):
+        list_rules = list(self._rules.items())
+        for view_func, rules in iter(list_rules):
+            for rule in rules:
+                if not rule.exact_match(path):
+                    continue
+                log("Dispatching to '%s', exact match" % view_func.__name__)
+                view_func()
+                return
+
+        # then, search for regex matches
+        for view_func, rules in iter(list_rules):
             for rule in rules:
                 kwargs = rule.match(path)
-                if kwargs is not None:
-                    log("Dispatching to '%s', args: %s" % (view_func.__name__, kwargs))
-                    view_func(**kwargs)
-                    return
+                if kwargs is None:
+                    continue
+                log("Dispatching to '%s', args: %s" % (view_func.__name__, kwargs))
+                view_func(**kwargs)
+                return
         raise RoutingError('No route to path "%s"' % path)
 
 
 class UrlRule:
 
     def __init__(self, pattern):
         pattern = pattern.rstrip('/')
-        kw_pattern = r'<(?:[^:]+:)?([A-z]+)>'
+        arg_regex = re.compile('<([A-z_][A-z0-9_]*)>')
+        self._has_args = bool(arg_regex.search(pattern))
+
+        kw_pattern = r'<(?:[^:]+:)?([A-z_][A-z0-9_]*)>'
         self._pattern = re.sub(kw_pattern, '{\\1}', pattern)
         self._keywords = re.findall(kw_pattern, pattern)
 
-        p = re.sub('<([A-z]+)>', '<string:\\1>', pattern)
-        p = re.sub('<string:([A-z]+)>', '(?P<\\1>[^/]+?)', p)
-        p = re.sub('<path:([A-z]+)>', '(?P<\\1>.*)', p)
+        p = re.sub('<([A-z_][A-z0-9_]*)>', '<string:\\1>', pattern)
+        p = re.sub('<string:([A-z_][A-z0-9_]*)>', '(?P<\\1>[^/]+?)', p)
+        p = re.sub('<path:([A-z_][A-z0-9_]*)>', '(?P<\\1>.*)', p)
         self._compiled_pattern = p
         self._regex = re.compile('^' + p + '$')
 
@@ -166,14 +190,17 @@ def match(self, path):
         match = self._regex.search(path)
         return match.groupdict() if match else None
 
+    def exact_match(self, path):
+        return not self._has_args and self._pattern == path
+
     def make_path(self, *args, **kwargs):
         """Construct a path from arguments."""
         if args and kwargs:
             return None  # can't use both args and kwargs
         if args:
             # Replace the named groups %s and format
             try:
-                return re.sub(r'{[A-z]+}', r'%s', self._pattern) % args
+                return re.sub(r'{[A-z_][A-z0-9_]*}', r'%s', self._pattern) % args
             except TypeError:
                 return None
 

lib/tests.py

@@ -39,9 +39,15 @@ def test_make_path():
     assert rule.make_path(1) is None
 
 
-def test_make_path_should_urlencode_args():
-    rule = UrlRule("/foo")
-    assert rule.make_path(bar="b a&r") == "/foo?bar=b+a%26r"
+def test_make_path_should_urlencode_args(plugin):
+    f = mock.create_autospec(lambda: None)
+    plugin.route('/foo')(f)
+
+    assert plugin.url_for(f, bar='b a&r+c') == \
+        plugin.base_url + '/foo?bar=b+a%26r%2Bc'
+    plugin.run(['plugin://py.test/foo', '0', '?bar=b+a%26r%2Bc'])
+    f.assert_called_with()
+    assert plugin.args['bar'][0] == 'b a&r+c'
 
 
 def test_url_for_path():
@@ -56,15 +62,15 @@ def test_url_for(plugin):
 
 
 def test_url_for_kwargs(plugin):
-    f = lambda a, b: None
-    plugin.route("/foo/<a>/<b>")(f)
-    assert plugin.url_for(f, a=1, b=2) == plugin.base_url + "/foo/1/2"
+    f = lambda a, var_with_num_underscore2: None
+    plugin.route("/foo/<a>/<var_with_num_underscore2>")(f)
+    assert plugin.url_for(f, a=1, var_with_num_underscore2=2) == plugin.base_url + "/foo/1/2"
 
 
 def test_url_for_args(plugin):
-    f = lambda a, b: None
-    plugin.route("/<a>/<b>")(f)
-    assert plugin.url_for(f, 1, 2) == plugin.base_url + "/1/2"
+    f = lambda a, var_with_num_underscore2, c, d: None
+    plugin.route("/<a>/<var_with_num_underscore2>/<c>/<d>")(f)
+    assert plugin.url_for(f, 1, 2.6, True, 'baz') == plugin.base_url + "/1/2.6/True/baz"
 
 
 def test_route_for(plugin):
@@ -74,16 +80,21 @@ def test_route_for(plugin):
 
 
 def test_route_for_args(plugin):
-    f = lambda: None
-    plugin.route("/foo/<a>/<b>")(f)
+    f = lambda a, var_with_num_underscore2: None
+    g = lambda: (None, None)  # just to make sure that they are easily different
+    plugin.route("/foo/<a>/<var_with_num_underscore2>")(f)
+    plugin.route("/foo/a/b")(g)
+
     assert plugin.route_for(plugin.base_url + "/foo/1/2") is f
+    assert plugin.route_for(plugin.base_url + "/foo/a/b") is g
 
 
 def test_dispatch(plugin):
     f = mock.create_autospec(lambda: None)
     plugin.route("/foo")(f)
     plugin.run(['plugin://py.test/foo', '0', '?bar=baz'])
     f.assert_called_with()
+    assert plugin.args['bar'][0] == 'baz'
 
 
 def test_path(plugin):
@@ -111,8 +122,9 @@ def test_no_route(plugin):
 def test_arg_parsing(plugin):
     f = mock.create_autospec(lambda: None)
     plugin.route("/foo")(f)
-    plugin.run(['plugin://py.test/foo', '0', '?bar=baz'])
-    assert plugin.args['bar'][0] == 'baz'
+    plugin.run(['plugin://py.test/foo', '0', '?bar=baz&bar2=baz2'])
+    assert plugin.args['bar'][0] == 'baz' and plugin.args['bar2'][0] == 'baz2'
+
 
 def test_trailing_slash_in_route_definition(plugin):
     """ Should call registered route with trailing slash. """
@@ -121,13 +133,15 @@ def test_trailing_slash_in_route_definition(plugin):
     plugin.run(['plugin://py.test/foo', '0'])
     assert f.call_count == 1
 
+
 def test_trailing_slashes_in_run(plugin):
     """ Should call registered route without trailing slash. """
     f = mock.create_autospec(lambda: None)
     plugin.route("/foo")(f)
     plugin.run(['plugin://py.test/foo/', '0'])
     assert f.call_count == 1
 
+
 def test_trailing_slash_handling_for_root(plugin):
     f = mock.create_autospec(lambda: None)
     plugin.route("/<a>")(lambda: None)