From f2cc404d7f003ec7043623a1ab3a0e262dc3931e Mon Sep 17 00:00:00 2001 From: John Piasetzki Date: Wed, 19 Oct 2016 23:41:22 -0400 Subject: [PATCH 01/10] Remove simple router --- sanic/router.py | 31 ------------------------------- 1 file changed, 31 deletions(-) diff --git a/sanic/router.py b/sanic/router.py index e6c580d7..a4ac68bb 100644 --- a/sanic/router.py +++ b/sanic/router.py @@ -113,34 +113,3 @@ class Router: return route.handler, args, kwargs else: raise NotFound("Requested URL {} not found".format(request.url)) - - -class SimpleRouter: - """ - Simple router records and reads all routes from a dictionary - It does not support parameters in routes, but is very fast - """ - 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)) From 50e4dd167e465af2c26ff4d092b49bbf5edd71d6 Mon Sep 17 00:00:00 2001 From: John Piasetzki Date: Wed, 19 Oct 2016 23:43:31 -0400 Subject: [PATCH 02/10] Extract constant --- sanic/router.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/sanic/router.py b/sanic/router.py index a4ac68bb..0d04365a 100644 --- a/sanic/router.py +++ b/sanic/router.py @@ -5,6 +5,13 @@ from .exceptions import NotFound, InvalidUsage Route = namedtuple("Route", ['handler', 'methods', 'pattern', 'parameters']) Parameter = namedtuple("Parameter", ['name', 'cast']) +REGEX_TYPES = { + "string": (None, "[^/]+"), + "int": (int, "\d+"), + "number": (float, "[0-9\\.]+"), + "alpha": (None, "[A-Za-z]+"), +} + class Router: """ @@ -25,12 +32,6 @@ class Router: I should feel bad """ routes = None - regex_types = { - "string": (None, "[^/]+"), - "int": (int, "\d+"), - "number": (float, "[0-9\\.]+"), - "alpha": (None, "[A-Za-z]+"), - } def __init__(self): self.routes = [] @@ -63,7 +64,7 @@ class Router: parameter_pattern = 'string' # Pull from pre-configured types - parameter_regex = self.regex_types.get(parameter_pattern) + parameter_regex = REGEX_TYPES.get(parameter_pattern) if parameter_regex: parameter_type, parameter_pattern = parameter_regex else: From 04a6cc9416b77ae99a1fe41d20e01e015a20ab3e Mon Sep 17 00:00:00 2001 From: John Piasetzki Date: Wed, 19 Oct 2016 23:51:40 -0400 Subject: [PATCH 03/10] Refactor add parameter --- sanic/router.py | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/sanic/router.py b/sanic/router.py index 0d04365a..2b997abe 100644 --- a/sanic/router.py +++ b/sanic/router.py @@ -56,24 +56,16 @@ class Router: def add_parameter(match): # We could receive NAME or NAME:PATTERN - parts = match.group(1).split(':') - if len(parts) == 2: - parameter_name, parameter_pattern = parts - else: - parameter_name = parts[0] - parameter_pattern = 'string' + parameter_name = match.group(1) + parameter_pattern = 'string' + if ':' in parameter_name: + parameter_name, parameter_pattern = parameter_name.split(':', 1) + default = (None, parameter_pattern) # Pull from pre-configured types - parameter_regex = REGEX_TYPES.get(parameter_pattern) - if parameter_regex: - 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) + parameter_type, parameter_pattern = REGEX_TYPES.get(parameter_pattern, default) + parameters.append(Parameter(name=parameter_name, cast=parameter_type)) + return '({})'.format(parameter_pattern) pattern_string = re.sub("<(.+?)>", add_parameter, uri) pattern = re.compile("^{}$".format(pattern_string)) From e25e1c0e4ba9a841216ba7c003d1cd5154cba46c Mon Sep 17 00:00:00 2001 From: John Piasetzki Date: Wed, 19 Oct 2016 23:56:23 -0400 Subject: [PATCH 04/10] Convert string formats --- sanic/router.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/sanic/router.py b/sanic/router.py index 2b997abe..65ade57a 100644 --- a/sanic/router.py +++ b/sanic/router.py @@ -2,14 +2,14 @@ import re from collections import namedtuple from .exceptions import NotFound, InvalidUsage -Route = namedtuple("Route", ['handler', 'methods', 'pattern', 'parameters']) -Parameter = namedtuple("Parameter", ['name', 'cast']) +Route = namedtuple('Route', ['handler', 'methods', 'pattern', 'parameters']) +Parameter = namedtuple('Parameter', ['name', 'cast']) REGEX_TYPES = { - "string": (None, "[^/]+"), - "int": (int, "\d+"), - "number": (float, "[0-9\\.]+"), - "alpha": (None, "[A-Za-z]+"), + 'string': (None, r'[^/]+'), + 'int': (int, r'\d+'), + 'number': (float, r'[0-9\\.]+'), + 'alpha': (None, r'[A-Za-z]+'), } @@ -67,8 +67,8 @@ class Router: parameters.append(Parameter(name=parameter_name, cast=parameter_type)) return '({})'.format(parameter_pattern) - pattern_string = re.sub("<(.+?)>", add_parameter, uri) - 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, @@ -101,8 +101,8 @@ class Router: if route: if route.methods and request.method not in route.methods: raise InvalidUsage( - "Method {} not allowed for URL {}".format( + '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)) + raise NotFound('Requested URL {} not found'.format(request.url)) From baf1ce95b18176dfd452dbafecc0552cdc1487ef Mon Sep 17 00:00:00 2001 From: John Piasetzki Date: Thu, 20 Oct 2016 00:05:55 -0400 Subject: [PATCH 05/10] Refactor get --- sanic/router.py | 35 +++++++++++++---------------------- 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/sanic/router.py b/sanic/router.py index 65ade57a..ece5caad 100644 --- a/sanic/router.py +++ b/sanic/router.py @@ -6,10 +6,10 @@ Route = namedtuple('Route', ['handler', 'methods', 'pattern', 'parameters']) Parameter = namedtuple('Parameter', ['name', 'cast']) REGEX_TYPES = { - 'string': (None, r'[^/]+'), + 'string': (str, r'[^/]+'), 'int': (int, r'\d+'), 'number': (float, r'[0-9\\.]+'), - 'alpha': (None, r'[A-Za-z]+'), + 'alpha': (str, r'[A-Za-z]+'), } @@ -61,7 +61,7 @@ class Router: if ':' in parameter_name: parameter_name, parameter_pattern = parameter_name.split(':', 1) - default = (None, parameter_pattern) + default = (str, parameter_pattern) # Pull from pre-configured types parameter_type, parameter_pattern = REGEX_TYPES.get(parameter_pattern, default) parameters.append(Parameter(name=parameter_name, cast=parameter_type)) @@ -82,27 +82,18 @@ class Router: :param request: Request object :return: handler, arguments, keyword arguments """ - route = None - args = [] - kwargs = {} - for _route in self.routes: - match = _route.pattern.match(request.url) + for route in self.routes: + match = route.pattern.match(request.url) if match: - for index, parameter in enumerate(_route.parameters, start=1): - value = match.group(index) - if parameter.cast: - kwargs[parameter.name] = parameter.cast(value) - else: - kwargs[parameter.name] = value - route = _route break - - 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, args, kwargs else: raise NotFound('Requested URL {} not found'.format(request.url)) + + 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) + + kwargs = {p.name: p.cast(value) for value, p in zip(match.groups(1), route.parameters)} + return route.handler, [], kwargs From d1beabfc8fde95811c205ed2360871ef3ccf5814 Mon Sep 17 00:00:00 2001 From: John Piasetzki Date: Thu, 20 Oct 2016 00:07:07 -0400 Subject: [PATCH 06/10] Add lru_cache to get --- sanic/config.py | 1 + sanic/router.py | 3 +++ 2 files changed, 4 insertions(+) diff --git a/sanic/config.py b/sanic/config.py index 8261c2c0..3dbf06c8 100644 --- a/sanic/config.py +++ b/sanic/config.py @@ -22,3 +22,4 @@ class Config: """ REQUEST_MAX_SIZE = 100000000 # 100 megababies REQUEST_TIMEOUT = 60 # 60 seconds + ROUTER_CACHE_SIZE = 1024 diff --git a/sanic/router.py b/sanic/router.py index ece5caad..45248cde 100644 --- a/sanic/router.py +++ b/sanic/router.py @@ -1,5 +1,7 @@ import re from collections import namedtuple +from functools import lru_cache +from .config import Config from .exceptions import NotFound, InvalidUsage Route = namedtuple('Route', ['handler', 'methods', 'pattern', 'parameters']) @@ -75,6 +77,7 @@ class Router: parameters=parameters) self.routes.append(route) + @lru_cache(maxsize=Config.ROUTER_CACHE_SIZE) def get(self, request): """ Gets a request handler based on the URL of the request, or raises an From f4b45deb7ffa299b5f17fde787c7b9152ba96b14 Mon Sep 17 00:00:00 2001 From: John Piasetzki Date: Thu, 20 Oct 2016 00:16:16 -0400 Subject: [PATCH 07/10] Convert dict to set --- sanic/router.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/sanic/router.py b/sanic/router.py index 45248cde..2d04c9b9 100644 --- a/sanic/router.py +++ b/sanic/router.py @@ -50,9 +50,8 @@ class Router: """ # 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 = [] @@ -73,7 +72,7 @@ class Router: 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) From fc4c192237f484dae03ead98174a7ec081009d4e Mon Sep 17 00:00:00 2001 From: John Piasetzki Date: Thu, 20 Oct 2016 01:07:16 -0400 Subject: [PATCH 08/10] Add simple uri hash to lookup --- sanic/router.py | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/sanic/router.py b/sanic/router.py index 2d04c9b9..f0f7de43 100644 --- a/sanic/router.py +++ b/sanic/router.py @@ -1,5 +1,5 @@ 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 @@ -15,6 +15,10 @@ REGEX_TYPES = { } +def url_hash(url): + return '/'.join(':' for s in url.split('/')) + + class Router: """ Router supports basic routing with parameters and method checks @@ -36,7 +40,7 @@ class Router: routes = None def __init__(self): - self.routes = [] + self.routes = defaultdict(list) def add(self, uri, methods, handler): """ @@ -74,7 +78,10 @@ class Router: route = Route( handler=handler, methods=methods, pattern=pattern, parameters=parameters) - self.routes.append(route) + + if parameters: + uri = url_hash(uri) + self.routes[uri].append(route) @lru_cache(maxsize=Config.ROUTER_CACHE_SIZE) def get(self, request): @@ -85,17 +92,22 @@ class Router: :return: handler, arguments, keyword arguments """ route = None - for route in self.routes: - match = route.pattern.match(request.url) - if match: - break + url = request.url + if url in self.routes: + route = self.routes[url][0] + match = route.pattern.match(url) else: - raise NotFound('Requested URL {} not found'.format(request.url)) + for route in self.routes[url_hash(url)]: + match = route.pattern.match(url) + if match: + break + else: + raise NotFound('Requested URL {} not found'.format(url)) 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) + request.method, url), status_code=405) kwargs = {p.name: p.cast(value) for value, p in zip(match.groups(1), route.parameters)} return route.handler, [], kwargs From f51055088857e004c48f24577415b1d8bf12b94a Mon Sep 17 00:00:00 2001 From: John Piasetzki Date: Thu, 20 Oct 2016 01:33:59 -0400 Subject: [PATCH 09/10] Fix flake8 --- sanic/router.py | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/sanic/router.py b/sanic/router.py index f0f7de43..67b4ca30 100644 --- a/sanic/router.py +++ b/sanic/router.py @@ -61,16 +61,16 @@ class Router: def add_parameter(match): # We could receive NAME or NAME:PATTERN - parameter_name = match.group(1) - parameter_pattern = 'string' - if ':' in parameter_name: - parameter_name, parameter_pattern = parameter_name.split(':', 1) + name = match.group(1) + pattern = 'string' + if ':' in name: + name, pattern = name.split(':', 1) - default = (str, parameter_pattern) + default = (str, pattern) # Pull from pre-configured types - parameter_type, parameter_pattern = REGEX_TYPES.get(parameter_pattern, default) - parameters.append(Parameter(name=parameter_name, cast=parameter_type)) - return '({})'.format(parameter_pattern) + _type, pattern = REGEX_TYPES.get(pattern, default) + parameters.append(Parameter(name=name, cast=_type)) + return '({})'.format(pattern) pattern_string = re.sub(r'<(.+?)>', add_parameter, uri) pattern = re.compile(r'^{}$'.format(pattern_string)) @@ -109,5 +109,7 @@ class Router: 'Method {} not allowed for URL {}'.format( request.method, url), status_code=405) - kwargs = {p.name: p.cast(value) for value, p in zip(match.groups(1), route.parameters)} + kwargs = {p.name: p.cast(value) + for value, p + in zip(match.groups(1), route.parameters)} return route.handler, [], kwargs From d4e2d94816f56dbbd5264da22219855e3cbf047e Mon Sep 17 00:00:00 2001 From: Channel Cat Date: Thu, 20 Oct 2016 11:33:28 +0000 Subject: [PATCH 10/10] Added support for routes with / in custom regexes and updated lru to use url and method --- sanic/router.py | 75 +++++++++++++++++++++++++++++++------------- tests/test_routes.py | 66 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 118 insertions(+), 23 deletions(-) diff --git a/sanic/router.py b/sanic/router.py index 67b4ca30..951b49bc 100644 --- a/sanic/router.py +++ b/sanic/router.py @@ -16,7 +16,11 @@ REGEX_TYPES = { def url_hash(url): - return '/'.join(':' for s in url.split('/')) + return url.count('/') + + +class RouteExists(Exception): + pass class Router: @@ -31,16 +35,16 @@ class Router: function provided Parameters can also have a type by appending :type to the . 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 + routes_dynamic = None + routes_always_check = None def __init__(self): - self.routes = defaultdict(list) + self.routes_all = {} + self.routes_static = {} + self.routes_dynamic = defaultdict(list) + self.routes_always_check = [] def add(self, uri, methods, handler): """ @@ -52,12 +56,15 @@ 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 if methods: methods = frozenset(methods) parameters = [] + properties = {"unhashable": None} def add_parameter(match): # We could receive NAME or NAME:PATTERN @@ -69,7 +76,13 @@ class Router: default = (str, pattern) # Pull from pre-configured types _type, pattern = REGEX_TYPES.get(pattern, default) - parameters.append(Parameter(name=name, cast=_type)) + parameter = Parameter(name=name, cast=_type) + parameters.append(parameter) + + # Mark the whole route as unhashable if it has the hash key in it + if re.search('(^|[^^]){1}/', pattern): + properties['unhashable'] = True + return '({})'.format(pattern) pattern_string = re.sub(r'<(.+?)>', add_parameter, uri) @@ -79,11 +92,14 @@ class Router: handler=handler, methods=methods, pattern=pattern, parameters=parameters) - if parameters: - uri = url_hash(uri) - self.routes[uri].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 - @lru_cache(maxsize=Config.ROUTER_CACHE_SIZE) def get(self, request): """ Gets a request handler based on the URL of the request, or raises an @@ -91,23 +107,40 @@ class Router: :param request: Request object :return: handler, arguments, keyword arguments """ - route = None - url = request.url - if url in self.routes: - route = self.routes[url][0] + return self._get(request.url, request.method) + + @lru_cache(maxsize=Config.ROUTER_CACHE_SIZE) + def _get(self, url, method): + """ + Gets a request handler based on the URL of the request, or raises an + error. Internal method for caching. + :param url: Request URL + :param method: Request method + :return: handler, arguments, keyword arguments + """ + # Check against known static routes + route = self.routes_static.get(url) + if route: match = route.pattern.match(url) else: - for route in self.routes[url_hash(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: - raise NotFound('Requested URL {} not found'.format(url)) + # 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 request.method not in route.methods: + if route.methods and method not in route.methods: raise InvalidUsage( 'Method {} not allowed for URL {}'.format( - request.method, url), status_code=405) + method, url), status_code=405) kwargs = {p.name: p.cast(value) for value, p diff --git a/tests/test_routes.py b/tests/test_routes.py index 640f3422..4759e450 100644 --- a/tests/test_routes.py +++ b/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//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//') + async def handler1(request, dynamic): + pass + + @app.route('/test//') + async def handler2(request, dynamic): + pass