Merge branch 'jpiasetz-fast_router'

sanic/config.py

@@ -22,3 +22,4 @@ class Config:
 """
     REQUEST_MAX_SIZE = 100000000  # 100 megababies
     REQUEST_TIMEOUT = 60  # 60 seconds
+    ROUTER_CACHE_SIZE = 1024

sanic/router.py

@@ -1,9 +1,26 @@
 import re
-from collections import namedtuple
+from collections import defaultdict, namedtuple
+from functools import lru_cache
+from .config import Config
 from .exceptions import NotFound, InvalidUsage
 
-Route = namedtuple("Route", ['handler', 'methods', 'pattern', 'parameters'])
+Route = namedtuple('Route', ['handler', 'methods', 'pattern', 'parameters'])
-Parameter = namedtuple("Parameter", ['name', 'cast'])
+Parameter = namedtuple('Parameter', ['name', 'cast'])
+
+REGEX_TYPES = {
+    'string': (str, r'[^/]+'),
+    'int': (int, r'\d+'),
+    'number': (float, r'[0-9\\.]+'),
+    'alpha': (str, r'[A-Za-z]+'),
+}
+
+
+def url_hash(url):
+    return url.count('/')
+
+
+class RouteExists(Exception):
+    pass
 
 
 class Router:
@@ -18,22 +35,16 @@ class Router:
     function provided Parameters can also have a type by appending :type to
     the <parameter>.  If no type is provided, a string is expected.  A regular
     expression can also be passed in as the type
-
-    TODO:
-        This probably needs optimization for larger sets of routes,
-        since it checks every route until it finds a match which is bad and
-        I should feel bad
     """
-    routes = None
+    routes_static = None
-    regex_types = {
+    routes_dynamic = None
-        "string": (None, "[^/]+"),
+    routes_always_check = None
-        "int": (int, "\d+"),
-        "number": (float, "[0-9\\.]+"),
-        "alpha": (None, "[A-Za-z]+"),
-    }
 
     def __init__(self):
-        self.routes = []
+        self.routes_all = {}
+        self.routes_static = {}
+        self.routes_dynamic = defaultdict(list)
+        self.routes_always_check = []
 
     def add(self, uri, methods, handler):
         """
@@ -45,42 +56,49 @@ class Router:
         When executed, it should provide a response object.
         :return: Nothing
         """
+        if uri in self.routes_all:
+            raise RouteExists("Route already registered: {}".format(uri))
 
         # Dict for faster lookups of if method allowed
-        methods_dict = None
         if methods:
-            methods_dict = {method: True for method in methods}
+            methods = frozenset(methods)
 
         parameters = []
+        properties = {"unhashable": None}
 
         def add_parameter(match):
             # We could receive NAME or NAME:PATTERN
-            parts = match.group(1).split(':')
+            name = match.group(1)
-            if len(parts) == 2:
+            pattern = 'string'
-                parameter_name, parameter_pattern = parts
+            if ':' in name:
-            else:
+                name, pattern = name.split(':', 1)
-                parameter_name = parts[0]
-                parameter_pattern = 'string'
 
+            default = (str, pattern)
             # Pull from pre-configured types
-            parameter_regex = self.regex_types.get(parameter_pattern)
+            _type, pattern = REGEX_TYPES.get(pattern, default)
-            if parameter_regex:
+            parameter = Parameter(name=name, cast=_type)
-                parameter_type, parameter_pattern = parameter_regex
-            else:
-                parameter_type = None
-
-            parameter = Parameter(name=parameter_name, cast=parameter_type)
             parameters.append(parameter)
 
-            return "({})".format(parameter_pattern)
+            # Mark the whole route as unhashable if it has the hash key in it
+            if re.search('(^|[^^]){1}/', pattern):
+                properties['unhashable'] = True
 
-        pattern_string = re.sub("<(.+?)>", add_parameter, uri)
+            return '({})'.format(pattern)
-        pattern = re.compile("^{}$".format(pattern_string))
+
+        pattern_string = re.sub(r'<(.+?)>', add_parameter, uri)
+        pattern = re.compile(r'^{}$'.format(pattern_string))
 
         route = Route(
-            handler=handler, methods=methods_dict, pattern=pattern,
+            handler=handler, methods=methods, pattern=pattern,
             parameters=parameters)
-        self.routes.append(route)
+
+        self.routes_all[uri] = route
+        if properties['unhashable']:
+            self.routes_always_check.append(route)
+        elif parameters:
+            self.routes_dynamic[url_hash(uri)].append(route)
+        else:
+            self.routes_static[uri] = route
 
     def get(self, request):
         """
@@ -89,58 +107,42 @@ class Router:
         :param request: Request object
         :return: handler, arguments, keyword arguments
         """
+        return self._get(request.url, request.method)
 
-        route = None
+    @lru_cache(maxsize=Config.ROUTER_CACHE_SIZE)
-        args = []
+    def _get(self, url, method):
-        kwargs = {}
+        """
-        for _route in self.routes:
+        Gets a request handler based on the URL of the request, or raises an
-            match = _route.pattern.match(request.url)
+        error.  Internal method for caching.
-            if match:
+        :param url: Request URL
-                for index, parameter in enumerate(_route.parameters, start=1):
+        :param method: Request method
-                    value = match.group(index)
+        :return: handler, arguments, keyword arguments
-                    if parameter.cast:
+        """
-                        kwargs[parameter.name] = parameter.cast(value)
+        # Check against known static routes
-                    else:
+        route = self.routes_static.get(url)
-                        kwargs[parameter.name] = value
-                route = _route
-                break
-
         if route:
-            if route.methods and request.method not in route.methods:
+            match = route.pattern.match(url)
-                raise InvalidUsage(
-                    "Method {} not allowed for URL {}".format(
-                        request.method, request.url), status_code=405)
-            return route.handler, args, kwargs
         else:
-            raise NotFound("Requested URL {} not found".format(request.url))
+            # Move on to testing all regex routes
+            for route in self.routes_dynamic[url_hash(url)]:
+                match = route.pattern.match(url)
+                if match:
+                    break
+            else:
+                # Lastly, check against all regex routes that cannot be hashed
+                for route in self.routes_always_check:
+                    match = route.pattern.match(url)
+                    if match:
+                        break
+                else:
+                    raise NotFound('Requested URL {} not found'.format(url))
 
+        if route.methods and method not in route.methods:
+            raise InvalidUsage(
+                'Method {} not allowed for URL {}'.format(
+                    method, url), status_code=405)
 
-class SimpleRouter:
+        kwargs = {p.name: p.cast(value)
-    """
+                  for value, p
-    Simple router records and reads all routes from a dictionary
+                  in zip(match.groups(1), route.parameters)}
-    It does not support parameters in routes, but is very fast
+        return route.handler, [], kwargs
-    """
-    routes = None
-
-    def __init__(self):
-        self.routes = {}
-
-    def add(self, uri, methods, handler):
-        # Dict for faster lookups of method allowed
-        methods_dict = None
-        if methods:
-            methods_dict = {method: True for method in methods}
-        self.routes[uri] = Route(
-            handler=handler, methods=methods_dict, pattern=uri,
-            parameters=None)
-
-    def get(self, request):
-        route = self.routes.get(request.url)
-        if route:
-            if route.methods and request.method not in route.methods:
-                raise InvalidUsage(
-                    "Method {} not allowed for URL {}".format(
-                        request.method, request.url), status_code=405)
-            return route.handler, [], {}
-        else:
-            raise NotFound("Requested URL {} not found".format(request.url))

tests/test_routes.py

@@ -1,6 +1,8 @@
-from json import loads as json_loads, dumps as json_dumps
+import pytest
+
 from sanic import Sanic
-from sanic.response import json, text
+from sanic.response import text
+from sanic.router import RouteExists
 from sanic.utils import sanic_endpoint_test
 
 
@@ -8,6 +10,24 @@ from sanic.utils import sanic_endpoint_test
 #  UTF-8
 # ------------------------------------------------------------ #
 
+def test_static_routes():
+    app = Sanic('test_dynamic_route')
+
+    @app.route('/test')
+    async def handler1(request):
+        return text('OK1')
+
+    @app.route('/pizazz')
+    async def handler2(request):
+        return text('OK2')
+
+    request, response = sanic_endpoint_test(app, uri='/test')
+    assert response.text == 'OK1'
+
+    request, response = sanic_endpoint_test(app, uri='/pizazz')
+    assert response.text == 'OK2'
+
+
 def test_dynamic_route():
     app = Sanic('test_dynamic_route')
 
@@ -102,3 +122,45 @@ def test_dynamic_route_regex():
 
     request, response = sanic_endpoint_test(app, uri='/folder/')
     assert response.status == 200
+
+
+def test_dynamic_route_unhashable():
+    app = Sanic('test_dynamic_route_unhashable')
+
+    @app.route('/folder/<unhashable:[A-Za-z0-9/]+>/end/')
+    async def handler(request, unhashable):
+        return text('OK')
+
+    request, response = sanic_endpoint_test(app, uri='/folder/test/asdf/end/')
+    assert response.status == 200
+
+    request, response = sanic_endpoint_test(app, uri='/folder/test///////end/')
+    assert response.status == 200
+
+    request, response = sanic_endpoint_test(app, uri='/folder/test/end/')
+    assert response.status == 200
+
+    request, response = sanic_endpoint_test(app, uri='/folder/test/nope/')
+    assert response.status == 404
+
+
+def test_route_duplicate():
+    app = Sanic('test_dynamic_route')
+
+    with pytest.raises(RouteExists):
+        @app.route('/test')
+        async def handler1(request):
+            pass
+
+        @app.route('/test')
+        async def handler2(request):
+            pass
+
+    with pytest.raises(RouteExists):
+        @app.route('/test/<dynamic>/')
+        async def handler1(request, dynamic):
+            pass
+
+        @app.route('/test/<dynamic>/')
+        async def handler2(request, dynamic):
+            pass