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
@@ -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

@@ -56,15 +56,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, b2: None
+    plugin.route("/foo/<a>/<b2>")(f)
+    assert plugin.url_for(f, a=1, b2=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, b2, c, d: None
+    plugin.route("/<a>/<b2>/<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):
@@ -75,8 +75,14 @@ def test_route_for(plugin):
 
 def test_route_for_args(plugin):
     f = lambda: None
-    plugin.route("/foo/<a>/<b>")(f)
-    assert plugin.route_for(plugin.base_url + "/foo/1/2") is f
+    g = lambda: (None, None)  # just to make sure that they are easily different
+    plugin.route("/foo/<a>/<b2>")(f)
+    plugin.route("/foo/a/b")(g)
+
+    # due to the unpredictable sorting of dict, just do it 100 times to see if it fails
+    for _ in range(0, 100):
+        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):
@@ -111,8 +117,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 +128,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)