From 3677e6fff4e1a5a8c3963cce5879f2cf394a9bd0 Mon Sep 17 00:00:00 2001 From: c-bata Date: Mon, 20 Mar 2017 20:25:42 +0900 Subject: [PATCH 1/4] Refactor router --- kobin/app.py | 6 +- kobin/app.pyi | 2 +- kobin/routes.py | 196 +++++++++++++++++++++++++++---------------- kobin/routes.pyi | 24 ++---- tests/test_routes.py | 85 +++++-------------- 5 files changed, 158 insertions(+), 155 deletions(-) diff --git a/kobin/app.py b/kobin/app.py index 07394ec..637af14 100644 --- a/kobin/app.py +++ b/kobin/app.py @@ -70,7 +70,7 @@ def frozen(self): def route(self, rule=None, method='GET', name=None, callback=None): def decorator(callback_func): - self.router.add(method, rule, name, callback_func) + self.router.add(rule, method, name, callback_func) return callback_func return decorator(callback) if callback else decorator @@ -94,7 +94,9 @@ def _handle(self, environ): for before_request_callback in self.before_request_callbacks: before_request_callback() - callback, kwargs = self.router.match(environ) + method = environ['REQUEST_METHOD'] + path = environ['PATH_INFO'] or '/' + callback, kwargs = self.router.match(path, method) response = callback(**kwargs) if kwargs else callback() for after_request_callback in self.after_request_callbacks: diff --git a/kobin/app.pyi b/kobin/app.pyi index 43923f1..7ef766c 100644 --- a/kobin/app.pyi +++ b/kobin/app.pyi @@ -26,7 +26,7 @@ class Kobin: def __init__(self, config: Dict[str, Any] = ...) -> None: ... @property def frozen(self) -> bool: ... - def route(self, rule: str = ..., method: str = ..., name: str = ..., + def route(self, path: str = ..., method: str = ..., name: str = ..., callback: ViewFunction = ...) -> ViewFunction: ... def before_request(self, callback: Callable[[], None]) -> Callable[[], None]: ... def after_request(self, callback: Callable[[BaseResponse], BaseResponse]) -> \ diff --git a/kobin/routes.py b/kobin/routes.py index c82523f..560fc81 100644 --- a/kobin/routes.py +++ b/kobin/routes.py @@ -73,93 +73,149 @@ def user_detail() -> Response: """ from typing import get_type_hints -from .requests import request from .responses import HTTPError -DEFAULT_ARG_TYPE = str - def split_by_slash(path): stripped_path = path.lstrip('/').rstrip('/') return stripped_path.split('/') -class Route: - def __init__(self, rule, method, name, callback): - self.rule = rule - self.method = method.upper() - self.name = name - self.callback = callback - - @property - def callback_types(self): - return get_type_hints(self.callback) +def match_url_vars_type(url_vars, type_hints): + """ Match types of url vars. - def get_typed_url_vars(self, url_vars): - typed_url_vars = {} + >>> match_url_vars_type({'user_id': '1'}, {'user_id': int}) + True, {'user_id': 1} + >>> match_url_vars_type({'user_id': 'foo'}, {'user_id': int}) + False, {} + """ + typed_url_vars = {} + try: for k, v in url_vars.items(): - arg_type = self.callback_types.get(k, DEFAULT_ARG_TYPE) - typed_url_vars[k] = arg_type(v) - return typed_url_vars - - def _match_method(self, method): - return self.method == method.upper() - - def _match_path(self, path): - split_rule = split_by_slash(self.rule) - split_path = split_by_slash(path) - url_vars = {} - - if len(split_rule) != len(split_path): - return - - for r, p in zip(split_rule, split_path): - if r.startswith('{') and r.endswith('}'): - url_vars[r[1:-1]] = p - continue - if r != p: - return - try: - typed_url_vars = self.get_typed_url_vars(url_vars) - except ValueError: - return - return typed_url_vars - - def match(self, method, path): - url_vars = self._match_path(path) - if url_vars is None: - return None # path: not matched - elif not self._match_method(method): - return False # path: matched, method: not matched - else: - return url_vars # path: matched, method: matched + arg_type = type_hints.get(k) + if arg_type and arg_type != str: + typed_url_vars[k] = arg_type(v) + else: + typed_url_vars[k] = v + except ValueError: + return False, {} + return True, typed_url_vars + + +def match_path(rule, path): + """ Match path. + + >>> match_path('/foo', '/foo') + True, {} + >>> match_path('/foo', '/bar') + False, {} + >>> match_path('/users/{user_id}', '/users/1') + True, {'user_id', 1} + >>> match_path('/users/{user_id}', '/users/not-integer') + True, {'user_id': 'not-integer'} + """ + split_rule = split_by_slash(rule) + split_path = split_by_slash(path) + url_vars = {} + + if len(split_rule) != len(split_path): + return False, {} + + for r, p in zip(split_rule, split_path): + if r.startswith('{') and r.endswith('}'): + url_vars[r[1:-1]] = p + continue + if r != p: + return False, {} + return True, url_vars class Router: def __init__(self) -> None: - self.routes = [] - - def match(self, environ): - method = environ['REQUEST_METHOD'].upper() - path = environ['PATH_INFO'] or '/' + self.endpoints = [] + + def match(self, path, method): + """ Get callback and url_vars. + + >>> from kobin import Response + >>> r = Router() + >>> def view(user_id: int) -> Response: + ... return Response(f'You are {user_id}') + ... + >>> r.add('/users/{user_id}', 'GET', 'user-detail', view) + + >>> callback, url_vars = r.match('/users/1', 'GET') + >>> url_vars + {'user_id': 1} + >>> callback(**url_vars) + You are 1 + + >>> callback, url_vars = r.match('/notfound', 'GET') + >>> callback(**url_vars) + 404 Not Found + """ + if path != '/': + path = path.rstrip('/') + method = method.upper() status = 404 - for route in self.routes: - url_vars = route.match(method, path) - if url_vars is None: # path: not matched + for p, n, m in self.endpoints: + matched, url_vars = match_path(p, path) + if not matched: # path: not matched continue - elif url_vars is False: # path: matched, method: not matched - status = 405 - else: # path: matched, method: matched - return route.callback, url_vars - raise HTTPError(status=status, body='Not found: {}'.format(request.path)) - def add(self, method, rule, name, callback): - """ Add a new rule or replace the target for an existing rule. """ - route = Route(method=method.upper(), rule=rule, name=name, callback=callback) - self.routes.append(route) + if method not in m: # path: matched, method: not matched + status = 405 + raise HTTPError(status=status, body=f'Method not found: {path} {method}') # it has security issue?? + + callback, type_hints = m[method] + type_matched, typed_url_vars = match_url_vars_type(url_vars, type_hints) + if not type_matched: + continue # path: not matched (types are different) + return callback, typed_url_vars + raise HTTPError(status=status, body=f'Not found: {path}') + + def add(self, rule, method, name, callback): + """ Add a new rule or replace the target for an existing rule. + + >>> from kobin import Response + >>> r = Router() + >>> def view(user_id: int) -> Response: + ... return Response(f'You are {user_id}') + ... + >>> r.add('/users/{user_id}', 'GET', 'user-detail', view) + >>> r.endpoints[0] + ('/users/{user_id}', 'user-detail', {'GET': (view, {'user_id': int})}) + """ + if rule != '/': + rule = rule.rstrip('/') + method = method.upper() + + for i, e in enumerate(self.endpoints): + r, n, callbacks = e + if r == rule: + assert name == n and n is not None, ( + "A same path should set a same name for reverse routing." + ) + callbacks[method] = (callback, get_type_hints(callback)) + self.endpoints[i] = (r, name, callbacks) + break + else: + e = (rule, name, {method: (callback, get_type_hints(callback))}) + self.endpoints.append(e) def reverse(self, name, **kwargs): - for route in self.routes: - if name == route.name: - return route.rule.format(**kwargs) + """ Reverse routing. + + >>> from kobin import Response + >>> r = Router() + >>> def view(user_id: int) -> Response: + ... return Response(f'You are {user_id}') + ... + >>> r.add('/users/{user_id}', 'GET', 'user-detail', view) + >>> r.reverse('user-detail', user_id=1) + '/users/1' + """ + for p, n, _ in self.endpoints: + if name == n: + return p.format(**kwargs) diff --git a/kobin/routes.pyi b/kobin/routes.pyi index 89c75d3..9a6c8ff 100644 --- a/kobin/routes.pyi +++ b/kobin/routes.pyi @@ -3,27 +3,15 @@ from typing import Callable, Dict, List, Tuple, Union, Any from .responses import BaseResponse ViewFunction = Callable[..., BaseResponse] -DEFAULT_ARG_TYPE = ... # type: type -def redirect(url: str) -> BaseResponse: ... def split_by_slash(path: str) -> List[str]: ... - -class Route: - rule: str - method: str - name: str - callback: ViewFunction - def __init__(self, rule: str, method: str, name: str, callback: ViewFunction) -> None: ... - @property - def callback_types(self) -> Dict[str, Any]: ... - def get_typed_url_vars(self, url_vars: Dict[str, str]) -> Dict[str, Any]: ... - def _match_method(self, method: str) -> bool: ... - def _match_path(self, path: str) -> Union[None, Dict[str, Any]]: ... - def match(self, method: str, path: str) -> Dict[str, Any]: ... +def match_url_vars_type(url_vars: Dict[str, str], + type_hints: Dict[str, Any]) -> Tuple[bool, Dict[str, Any]]: ... +def match_path(rule: str, path: str) -> Tuple[bool, Dict[str, str]]: ... class Router: - routes: List[Route] + endpoints: Tuple[str, Dict[str, ViewFunction], Dict[str, Any]] def __init__(self) -> None: ... - def match(self, environ: Dict[str, str]) -> Tuple[ViewFunction, Dict[str, Any]]: ... - def add(self, method: str, rule: str, name: str, callback: ViewFunction) -> None: ... + def match(self, path: str, method: str) -> Tuple[ViewFunction, Dict[str, Any]]: ... + def add(self, rule: str, method: str, name: str, callback: ViewFunction) -> None: ... def reverse(self, name: str, **kwargs: Any) -> str: ... diff --git a/tests/test_routes.py b/tests/test_routes.py index bfb2f45..614b450 100644 --- a/tests/test_routes.py +++ b/tests/test_routes.py @@ -1,84 +1,41 @@ from unittest import TestCase -from kobin.routes import Route, Router +from kobin.routes import Router from kobin.responses import HTTPError, Response -class RouteTests(TestCase): - def test_call(self): - def dummy_func(num: int) -> Response: - return Response(f'hello {num}') - route = Route('/hoge/{num}', 'GET', 'hoge', dummy_func) - actual_response = route.callback(num=1) - self.assertEqual(actual_response.body, [b'hello 1']) - - def test_match_method(self): - def dummy_func(num: int) -> Response: - return Response(f'hello {num}') - route = Route('/hoge/{num}', 'GET', 'hoge', dummy_func) - self.assertTrue(route._match_method('GET')) - - def test_match_path(self): - def dummy_func(num: int) -> Response: - return Response(f'hello {num}') - route = Route('/hoge/{num}', 'GET', 'hoge', dummy_func) - self.assertTrue(route._match_path('/hoge/1')) - - def test_match_path_with_slash(self): - def dummy_func(num: int) -> Response: - return Response(f'hello {num}') - route = Route('/hoge/{num}', 'GET', 'hoge', dummy_func) - self.assertTrue(route._match_path('hoge/1/')) - - def test_match(self): - def dummy_func(num: int) -> Response: - return Response(f'hello {num}') - route = Route('/hoge/{num}', 'GET', 'hoge', dummy_func) - self.assertTrue(route.match('GET', 'hoge/1/')) - self.assertEqual(route.match('GET', 'hoge/1'), {'num': 1}) - self.assertEqual(route.match('GET', 'homm/1'), None) - self.assertEqual(route.match('PUT', 'hoge/1'), False) +def view_int(year: int) -> Response: + return Response(f'hello {year}') -class RouterTests(TestCase): - def setUp(self): - self.router = Router() +def view_str(name): + return Response(f'hello {name}') - def test_match_dynamic_routes_with_casted_number(self): - def dummy_func(year: int) -> Response: - return Response(f'hello {year}') - self.router.add('GET', '/tests/{year}', 'hoge', dummy_func) - test_env = {'REQUEST_METHOD': 'GET', 'PATH_INFO': '/tests/2015/'} - actual_target, actual_args = self.router.match(test_env) +class RouterTests(TestCase): + def test_match_dynamic_routes_with_casted_number(self): + r = Router() + r.add('/tests/{year}', 'GET', 'hoge', view_int) + actual_target, actual_args = r.match('/tests/2015/', 'GET') self.assertEqual(actual_args, {'year': 2015}) def test_match_dynamic_routes_with_string(self): - def dummy_func(name): - return Response(f'hello {name}') - - self.router.add('GET', '/tests/{name}', 'hoge', dummy_func) - test_env = {'REQUEST_METHOD': 'GET', 'PATH_INFO': '/tests/kobin/'} - actual_target, actual_args = self.router.match(test_env) + r = Router() + r.add('/tests/{name}', 'GET', 'hoge', view_str) + actual_target, actual_args = r.match('/tests/kobin/', 'GET') self.assertEqual(actual_args, {'name': 'kobin'}) def test_404_not_found(self): - def dummy_func(name): - return Response(f'hello {name}') - - self.router.add('GET', '/tests/{name}', 'hoge', dummy_func) - test_env = {'REQUEST_METHOD': 'GET', 'PATH_INFO': '/this_is_not_found'} + r = Router() + r.add('/tests/{name}', 'GET', 'hoge', view_str) with self.assertRaises(HTTPError) as cm: - self.router.match(test_env) + r.match('/this_is_not_found', 'GET') self.assertEqual('404 Not Found', cm.exception.status) def test_405_method_not_allowed(self): - def dummy_func(name): - return Response(f'hello {name}') - - self.router.add('GET', '/tests/{name}', 'hoge', dummy_func) - test_env = {'REQUEST_METHOD': 'POST', 'PATH_INFO': '/tests/kobin'} + r = Router() + r.add('/tests/{name}', 'GET', 'hoge', view_str) with self.assertRaises(HTTPError) as cm: - self.router.match(test_env) + r.match('/tests/kobin', 'POST') self.assertEqual('405 Method Not Allowed', cm.exception.status) @@ -92,8 +49,8 @@ def index() -> Response: def user_detail(user_id: int) -> Response: return Response(f'hello user{user_id}') - self.router.add('GET', '/', 'top', index) - self.router.add('GET', '/users/{user_id}', 'user-detail', user_detail) + self.router.add('/', 'GET', 'top', index) + self.router.add('/users/{user_id}', 'GET', 'user-detail', user_detail) def test_reverse_route_without_url_vars(self): actual = self.router.reverse('top') From c7af0590b5e072cff1c2c908d101b839bf6ab8ee Mon Sep 17 00:00:00 2001 From: c-bata Date: Mon, 20 Mar 2017 21:20:35 +0900 Subject: [PATCH 2/4] Add doctest --- kobin/routes.py | 34 ++++++++++++++++++++++------------ tox.ini | 5 +++++ 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/kobin/routes.py b/kobin/routes.py index 560fc81..c2c1dc4 100644 --- a/kobin/routes.py +++ b/kobin/routes.py @@ -85,9 +85,9 @@ def match_url_vars_type(url_vars, type_hints): """ Match types of url vars. >>> match_url_vars_type({'user_id': '1'}, {'user_id': int}) - True, {'user_id': 1} + (True, {'user_id': 1}) >>> match_url_vars_type({'user_id': 'foo'}, {'user_id': int}) - False, {} + (False, {}) """ typed_url_vars = {} try: @@ -106,13 +106,13 @@ def match_path(rule, path): """ Match path. >>> match_path('/foo', '/foo') - True, {} + (True, {}) >>> match_path('/foo', '/bar') - False, {} + (False, {}) >>> match_path('/users/{user_id}', '/users/1') - True, {'user_id', 1} + (True, {'user_id': '1'}) >>> match_path('/users/{user_id}', '/users/not-integer') - True, {'user_id': 'not-integer'} + (True, {'user_id': 'not-integer'}) """ split_rule = split_by_slash(rule) split_path = split_by_slash(path) @@ -147,12 +147,14 @@ def match(self, path, method): >>> callback, url_vars = r.match('/users/1', 'GET') >>> url_vars {'user_id': 1} - >>> callback(**url_vars) - You are 1 + >>> response = callback(**url_vars) + >>> response.body + [b'You are 1'] >>> callback, url_vars = r.match('/notfound', 'GET') - >>> callback(**url_vars) - 404 Not Found + Traceback (most recent call last): + ... + kobin.responses.HTTPError """ if path != '/': path = path.rstrip('/') @@ -184,8 +186,16 @@ def add(self, rule, method, name, callback): ... return Response(f'You are {user_id}') ... >>> r.add('/users/{user_id}', 'GET', 'user-detail', view) - >>> r.endpoints[0] - ('/users/{user_id}', 'user-detail', {'GET': (view, {'user_id': int})}) + >>> path, name, methods = r.endpoints[0] + >>> path + '/users/{user_id}' + >>> name + 'user-detail' + >>> callback, type_hints = methods['GET'] + >>> view == callback + True + >>> type_hints['user_id'] == int + True """ if rule != '/': rule = rule.rstrip('/') diff --git a/tox.ini b/tox.ini index 65f7312..4a5a125 100644 --- a/tox.ini +++ b/tox.ini @@ -4,6 +4,7 @@ envlist = py37 flake8 mypy + doctest [tox:travis] 3.6=py36,flake8,mypy @@ -15,6 +16,10 @@ setenv=PYTHONPATH = {toxinidir}:{toxinidir} deps = -rrequirements/test.txt commands = python setup.py test +[testenv:doctest] +deps = pytest +commands = pytest --doctest-module -v kobin + [testenv:py37] basepython = python3.7 From 1a977057a250fd349c8c90ff8d4c7820fd21fb35 Mon Sep 17 00:00:00 2001 From: c-bata Date: Mon, 20 Mar 2017 21:24:17 +0900 Subject: [PATCH 3/4] Fix type hints --- kobin/app.pyi | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/kobin/app.pyi b/kobin/app.pyi index 7ef766c..b752741 100644 --- a/kobin/app.pyi +++ b/kobin/app.pyi @@ -24,16 +24,17 @@ class Kobin: _frozen: bool def __init__(self, config: Dict[str, Any] = ...) -> None: ... + def __call__(self, environ: WSGIEnviron, start_response: StartResponse) -> WSGIResponse: ... + @property def frozen(self) -> bool: ... - def route(self, path: str = ..., method: str = ..., name: str = ..., + def route(self, rule: str = ..., method: str = ..., name: str = ..., callback: ViewFunction = ...) -> ViewFunction: ... def before_request(self, callback: Callable[[], None]) -> Callable[[], None]: ... def after_request(self, callback: Callable[[BaseResponse], BaseResponse]) -> \ Callable[[BaseResponse], BaseResponse]: ... def _handle(self, environ: WSGIEnviron) -> BaseResponse: ... def wsgi(self, environ: WSGIEnviron, start_response: StartResponse) -> WSGIResponse: ... - def __call__(self, environ: WSGIEnviron, start_response: StartResponse) -> WSGIResponse: ... def _get_exception_message(e: BaseException, debug: bool) -> str: ... From deb1c0404a93b0f32e215a04e5ef6fa8ce1aa1bc Mon Sep 17 00:00:00 2001 From: c-bata Date: Mon, 20 Mar 2017 21:28:05 +0900 Subject: [PATCH 4/4] Fix tox.ini --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 4a5a125..21ae537 100644 --- a/tox.ini +++ b/tox.ini @@ -7,7 +7,7 @@ envlist = doctest [tox:travis] -3.6=py36,flake8,mypy +3.6=py36,flake8,mypy,doctest nightly=py37 [testenv]