From fa3a7ab980bf40762c96b424710c859a70f6d694 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Thu, 9 Jul 2015 14:19:29 -0700 Subject: [PATCH 1/4] routing subsystem refactoring --- aiohttp/hdrs.py | 1 + aiohttp/web.py | 6 +- aiohttp/web_urldispatcher.py | 252 ++++++++++++++--------- tests/test_client_functional_newstyle.py | 3 +- tests/test_urldispatch.py | 228 ++++++++++---------- tests/test_web.py | 3 +- tests/test_web_functional.py | 17 +- tests/test_web_middleware.py | 3 +- tests/test_web_websocket_functional.py | 3 +- tests/test_websocket_client.py | 4 +- 10 files changed, 298 insertions(+), 222 deletions(-) diff --git a/aiohttp/hdrs.py b/aiohttp/hdrs.py index 6ab09fee662..27a5d19f446 100644 --- a/aiohttp/hdrs.py +++ b/aiohttp/hdrs.py @@ -11,6 +11,7 @@ METH_POST = upstr('POST') METH_PUT = upstr('PUT') METH_TRACE = upstr('TRACE') +METH_PURGE = upstr('PURGE') ACCEPT = upstr('ACCEPT') ACCEPT_CHARSET = upstr('ACCEPT-CHARSET') diff --git a/aiohttp/web.py b/aiohttp/web.py index cc86ff9230e..db874f278a9 100644 --- a/aiohttp/web.py +++ b/aiohttp/web.py @@ -89,7 +89,11 @@ def handle_request(self, message, payload): "got {!r} [middlewares {!r}]").format( match_info.handler, type(resp), self._middlewares) except HTTPException as exc: - resp = exc + route = self._router.get_system_route(exc.status_code) + try: + resp = yield from route.handler(request, exc) + except: + resp = exc resp_msg = resp.start(request) yield from resp.write_eof() diff --git a/aiohttp/web_urldispatcher.py b/aiohttp/web_urldispatcher.py index 0c9b33ee96e..24daf566e51 100644 --- a/aiohttp/web_urldispatcher.py +++ b/aiohttp/web_urldispatcher.py @@ -12,7 +12,7 @@ from urllib.parse import urlencode, unquote -from . import hdrs +from . import hdrs, web_exceptions from .abc import AbstractRouter, AbstractMatchInfo from .protocol import HttpVersion11 from .web_exceptions import HTTPMethodNotAllowed, HTTPNotFound, HTTPNotModified @@ -22,13 +22,17 @@ class UrlMappingMatchInfo(dict, AbstractMatchInfo): - def __init__(self, match_dict, route): + def __init__(self, match_dict, route, handler): + # Unquote separate matching parts + match_dict = {key: unquote(value) for key, value in match_dict.items()} + super().__init__(match_dict) self._route = route + self._handler = handler @property def handler(self): - return self._route.handler + return self._handler @property def route(self): @@ -39,7 +43,7 @@ def __repr__(self): @asyncio.coroutine -def _defaultExpectHandler(request): +def defaultExpectHandler(request): """Default handler for Except: 100-continue""" if request.version == HttpVersion11: request.transport.write(b"HTTP/1.1 100 Continue\r\n\r\n") @@ -47,33 +51,22 @@ def _defaultExpectHandler(request): class Route(metaclass=abc.ABCMeta): - def __init__(self, method, handler, name, *, expect_handler=None): + def __init__(self, name, *, expect_handler=None): if expect_handler is None: - expect_handler = _defaultExpectHandler + expect_handler = defaultExpectHandler assert asyncio.iscoroutinefunction(expect_handler), \ 'Coroutine is expected, got {!r}'.format(expect_handler) - self._method = method - self._handler = handler self._name = name self._expect_handler = expect_handler - @property - def method(self): - return self._method - - @property - def handler(self): - return self._handler - @property def name(self): return self._name @abc.abstractmethod # pragma: no branch - def match(self, path): - """Return dict with info for given path or - None if route cannot process path.""" + def match(self, request): + """Return UrlMappingMatchInfo object.""" @abc.abstractmethod # pragma: no branch def url(self, **kwargs): @@ -91,53 +84,113 @@ def _append_query(url, query): return url -class PlainRoute(Route): +class ViewableRoute(Route): + + def __init__(self, name, *, expect_handler=None): + super().__init__(name, expect_handler=expect_handler) + self._views = [] + + def match_view(self, request, view_name, match_dict): + method = request.method + allowed_methods = set() + + for view in self._views: + if (view.name == view_name and + (view.method == method or + view.method == hdrs.METH_ANY)): + return UrlMappingMatchInfo(match_dict, self, view) + else: + allowed_methods.add(view.method) + + return _MethodNotAllowedMatchInfo(method, allowed_methods) + + def add_view(self, method, handler, *, name=''): + self._views.append(View(handler, name, method)) + - def __init__(self, method, handler, name, path, *, expect_handler=None): - super().__init__(method, handler, name, expect_handler=expect_handler) +class PlainRoute(ViewableRoute): + + def __init__(self, name, path, *, expect_handler=None): + super().__init__(name, expect_handler=expect_handler) self._path = path - def match(self, path): + def match(self, request): # string comparison is about 10 times faster than regexp matching - if self._path == path: - return {} + if self._path == request.raw_path: + return super().match_view(request, '', {}) else: return None - def url(self, *, query=None): + def url(self, *, query=None, view=None): return self._append_query(self._path, query) def __repr__(self): + method = ','.join(set([view.method for view in self._views])) name = "'" + self.name + "' " if self.name is not None else "" - return " {handler!r}".format( - name=name, method=self.method, path=self._path, - handler=self.handler) + return " {handler!r}" - .format(name=name, method=self.method, - formatter=self._formatter, handler=self.handler)) + method = ','.join(set([view.method for view in self._views])) + return ("".format( + status=self._status, reason=self._reason) class StaticRoute(Route): @@ -146,8 +199,7 @@ def __init__(self, name, prefix, directory, *, expect_handler=None, chunk_size=256*1024): assert prefix.startswith('/'), prefix assert prefix.endswith('/'), prefix - super().__init__( - 'GET', self.handle, name, expect_handler=expect_handler) + super().__init__(name, expect_handler=expect_handler) self._prefix = prefix self._prefix_len = len(self._prefix) self._directory = os.path.abspath(directory) + os.sep @@ -157,10 +209,16 @@ def __init__(self, name, prefix, directory, *, raise ValueError( "No directory exists at '{}'".format(self._directory)) - def match(self, path): + def match(self, request): + path = request.raw_path if not path.startswith(self._prefix): return None - return {'filename': path[self._prefix_len:]} + + if request.method != hdrs.METH_GET: + return _MethodNotAllowedMatchInfo(request.method, [hdrs.METH_GET]) + + return UrlMappingMatchInfo( + {'filename': path[self._prefix_len:]}, self, self.handler) def url(self, *, filename, query=None): while filename.startswith('/'): @@ -169,7 +227,7 @@ def url(self, *, filename, query=None): return self._append_query(url, query) @asyncio.coroutine - def handle(self, request): + def handler(self, request): filename = request.match_info['filename'] filepath = os.path.abspath(os.path.join(self._directory, filename)) if not filepath.startswith(self._directory): @@ -213,35 +271,32 @@ def handle(self, request): def __repr__(self): name = "'" + self.name + "' " if self.name is not None else "" - return " {directory!r}".format( - name=name, method=self.method, path=self._prefix, - directory=self._directory) + return " {directory!r}".format( + name=name, path=self._prefix, directory=self._directory) -class SystemRoute(Route): +class View(metaclass=abc.ABCMeta): - def __init__(self, status, reason): - super().__init__(hdrs.METH_ANY, None, None) - self._status = status - self._reason = reason - - def url(self, **kwargs): - raise RuntimeError(".url() is not allowed for SystemRoute") + def __init__(self, handler, name='', method=hdrs.METH_ANY): + self._name = name + self._method = upstr(method) + self._handler = handler - def match(self, path): - return None + @property + def method(self): + return self._method @property - def status(self): - return self._status + def handler(self): + return self._handler @property - def reason(self): - return self._reason + def name(self): + return self._name - def __repr__(self): - return "".format(status=self._status, - reason=self._reason) + @asyncio.coroutine + def __call__(self, request): + return (yield from self._handler(request)) class _NotFoundMatchInfo(UrlMappingMatchInfo): @@ -249,7 +304,7 @@ class _NotFoundMatchInfo(UrlMappingMatchInfo): route = SystemRoute(404, 'Not Found') def __init__(self): - super().__init__({}, None) + super().__init__({}, None, self._not_found) @property def handler(self): @@ -268,7 +323,7 @@ class _MethodNotAllowedMatchInfo(UrlMappingMatchInfo): route = SystemRoute(405, 'Method Not Allowed') def __init__(self, method, allowed_methods): - super().__init__({}, None) + super().__init__({}, None, self._not_allowed) self._method = method self._allowed_methods = allowed_methods @@ -294,9 +349,10 @@ class UrlDispatcher(AbstractRouter, collections.abc.Mapping): GOOD = r'[^{}/]+' ROUTE_RE = re.compile(r'(\{[_a-zA-Z][^{}]*(?:\{[^{}]*\}[^{}]*)*\})') - METHODS = {hdrs.METH_ANY, hdrs.METH_POST, - hdrs.METH_GET, hdrs.METH_PUT, hdrs.METH_DELETE, - hdrs.METH_PATCH, hdrs.METH_HEAD, hdrs.METH_OPTIONS} + _system_routes = dict( + (getattr(web_exceptions, name).status_code, + SystemRoute(getattr(web_exceptions, name).status_code)) + for name in web_exceptions.__all__) def __init__(self): super().__init__() @@ -305,28 +361,14 @@ def __init__(self): @asyncio.coroutine def resolve(self, request): - path = request.raw_path - method = request.method - allowed_methods = set() - for route in self._urls: - match_dict = route.match(path) - if match_dict is None: + match_info = route.match(request) + if match_info is None: continue - route_method = route.method - if route_method == method or route_method == hdrs.METH_ANY: - # Unquote separate matching parts - match_dict = {key: unquote(value) for key, value in - match_dict.items()} - return UrlMappingMatchInfo(match_dict, route) - - allowed_methods.add(route_method) + return match_info else: - if allowed_methods: - return _MethodNotAllowedMatchInfo(method, allowed_methods) - else: - return _NotFoundMatchInfo() + return _NotFoundMatchInfo() def __iter__(self): return iter(self._routes) @@ -340,6 +382,12 @@ def __contains__(self, name): def __getitem__(self, name): return self._routes[name] + def get_system_route(self, status_code): + if status_code not in self._system_routes: + self._system_routes[status_code] = SystemRoute(status_code) + + return self._system_routes[status_code] + def register_route(self, route): assert isinstance(route, Route), 'Instance of Route class is required.' @@ -353,24 +401,29 @@ def register_route(self, route): self._routes[name] = route self._urls.append(route) - def add_route(self, method, path, handler, - *, name=None, expect_handler=None): - - if not path.startswith('/'): - raise ValueError("path should be started with /") + def add_view(self, method, handler, *, name='', route=None): assert callable(handler), handler if (not asyncio.iscoroutinefunction(handler) and not inspect.isgeneratorfunction(handler)): handler = asyncio.coroutine(handler) - method = upstr(method) - if method not in self.METHODS: - raise ValueError("{} is not allowed HTTP method".format(method)) + if route is None: + raise ValueError('Route name is required') + + if route not in self._routes: + raise ValueError('Route is not found {}', route) + + route = self._routes[route] + route.add_view(method, handler, name=name) + + def add_route(self, name, path, *, expect_handler=None): + + if not path.startswith('/'): + raise ValueError("path should be started with /") if not ('{' in path or '}' in path or self.ROUTE_RE.search(path)): - route = PlainRoute( - method, handler, name, path, expect_handler=expect_handler) + route = PlainRoute(name, path, expect_handler=expect_handler) self.register_route(route) return route @@ -401,8 +454,7 @@ def add_route(self, method, path, handler, raise ValueError( "Bad pattern '{}': {}".format(pattern, exc)) from None route = DynamicRoute( - method, handler, name, compiled, - formatter, expect_handler=expect_handler) + name, compiled, formatter, expect_handler=expect_handler) self.register_route(route) return route diff --git a/tests/test_client_functional_newstyle.py b/tests/test_client_functional_newstyle.py index 9d178fad5c7..a4dea548e65 100644 --- a/tests/test_client_functional_newstyle.py +++ b/tests/test_client_functional_newstyle.py @@ -31,7 +31,8 @@ def find_unused_port(self): def create_server(self, method, path, handler=None): app = web.Application(loop=self.loop) if handler: - app.router.add_route(method, path, handler) + app.router.add_route('r', path) + app.router.add_view(method, handler, route='r') port = self.find_unused_port() self.handler = app.make_handler( diff --git a/tests/test_urldispatch.py b/tests/test_urldispatch.py index 7b4626577be..35abbfda0f2 100644 --- a/tests/test_urldispatch.py +++ b/tests/test_urldispatch.py @@ -9,7 +9,7 @@ HTTPMethodNotAllowed, HTTPNotFound) from aiohttp.multidict import CIMultiDict from aiohttp.protocol import HttpVersion, RawRequestMessage -from aiohttp.web_urldispatcher import (_defaultExpectHandler, +from aiohttp.web_urldispatcher import (defaultExpectHandler, DynamicRoute, PlainRoute, SystemRoute) @@ -56,7 +56,8 @@ def test_system_route(self): def test_register_route(self): handler = self.make_handler() - route = PlainRoute('GET', handler, 'test', '/handler/to/path') + route = PlainRoute('test', '/handler/to/path') + route.add_view('GET', handler) self.router.register_route(route) req = self.make_request('GET', '/handler/to/path') @@ -64,52 +65,54 @@ def test_register_route(self): self.assertIsNotNone(info) self.assertEqual(0, len(info)) self.assertIs(route, info.route) - self.assertIs(handler, info.handler) + self.assertIs(handler, info.handler.handler) self.assertEqual(info.route.name, 'test') def test_register_route_checks(self): self.assertRaises( AssertionError, self.router.register_route, object()) - handler = self.make_handler() - route = PlainRoute('GET', handler, 'test', '/handler/to/path') + route = PlainRoute('test', '/handler/to/path') self.router.register_route(route) self.assertRaises(ValueError, self.router.register_route, route) def test_add_route_root(self): handler = self.make_handler() - self.router.add_route('GET', '/', handler) + self.router.add_route('r', '/') + self.router.add_view('GET', handler, route='r') req = self.make_request('GET', '/') info = self.loop.run_until_complete(self.router.resolve(req)) self.assertIsNotNone(info) self.assertEqual(0, len(info)) - self.assertIs(handler, info.handler) - self.assertIsNone(info.route.name) + self.assertIs(handler, info.handler.handler) + self.assertEqual(info.route.name, 'r') def test_add_route_simple(self): handler = self.make_handler() - self.router.add_route('GET', '/handler/to/path', handler) + self.router.add_route('r', '/handler/to/path') + self.router.add_view('GET', handler, route='r') req = self.make_request('GET', '/handler/to/path') info = self.loop.run_until_complete(self.router.resolve(req)) self.assertIsNotNone(info) self.assertEqual(0, len(info)) - self.assertIs(handler, info.handler) - self.assertIsNone(info.route.name) + self.assertIs(handler, info.handler.handler) + self.assertEqual(info.route.name, 'r') def test_add_with_matchdict(self): handler = self.make_handler() - self.router.add_route('GET', '/handler/{to}', handler) + self.router.add_route('r', '/handler/{to}') + self.router.add_view('GET', handler, route='r') req = self.make_request('GET', '/handler/tail') info = self.loop.run_until_complete(self.router.resolve(req)) self.assertIsNotNone(info) self.assertEqual({'to': 'tail'}, info) - self.assertIs(handler, info.handler) - self.assertIsNone(info.route.name) + self.assertIs(handler, info.handler.handler) + self.assertEqual(info.route.name, 'r') def test_add_with_name(self): handler = self.make_handler() - self.router.add_route('GET', '/handler/to/path', handler, - name='name') + self.router.add_route('name', '/handler/to/path') + self.router.add_view('GET', handler, route='name') req = self.make_request('GET', '/handler/to/path') info = self.loop.run_until_complete(self.router.resolve(req)) self.assertIsNotNone(info) @@ -117,50 +120,48 @@ def test_add_with_name(self): def test_add_with_tailing_slash(self): handler = self.make_handler() - self.router.add_route('GET', '/handler/to/path/', handler) + self.router.add_route('r', '/handler/to/path/') + self.router.add_view('GET', handler, route='r') req = self.make_request('GET', '/handler/to/path/') info = self.loop.run_until_complete(self.router.resolve(req)) self.assertIsNotNone(info) self.assertEqual({}, info) - self.assertIs(handler, info.handler) + self.assertIs(handler, info.handler.handler) def test_add_invalid_path(self): - handler = self.make_handler() with self.assertRaises(ValueError): - self.router.add_route('GET', '/{/', handler) + self.router.add_route('r', '/{/') def test_add_url_invalid1(self): - handler = self.make_handler() with self.assertRaises(ValueError): - self.router.add_route('post', '/post/{id', handler) + self.router.add_route('r', '/post/{id') def test_add_url_invalid2(self): - handler = self.make_handler() with self.assertRaises(ValueError): - self.router.add_route('post', '/post/{id{}}', handler) + self.router.add_route('r', '/post/{id{}}') def test_add_url_invalid3(self): - handler = self.make_handler() with self.assertRaises(ValueError): - self.router.add_route('post', '/post/{id{}', handler) + self.router.add_route('r', '/post/{id{}') def test_add_url_invalid4(self): - handler = self.make_handler() with self.assertRaises(ValueError): - self.router.add_route('post', '/post/{id"}', handler) + self.router.add_route('r', '/post/{id"}') def test_add_url_escaping(self): handler = self.make_handler() - self.router.add_route('GET', '/+$', handler) + self.router.add_route('r', '/+$') + self.router.add_view('GET', handler, route='r') req = self.make_request('GET', '/+$') info = self.loop.run_until_complete(self.router.resolve(req)) self.assertIsNotNone(info) - self.assertIs(handler, info.handler) + self.assertIs(handler, info.handler.handler) def test_any_method(self): handler = self.make_handler() - route = self.router.add_route(hdrs.METH_ANY, '/', handler) + route = self.router.add_route('r', '/') + self.router.add_view(hdrs.METH_ANY, handler, route='r') req = self.make_request('GET', '/') info1 = self.loop.run_until_complete(self.router.resolve(req)) @@ -176,19 +177,24 @@ def test_any_method(self): def test_match_second_result_in_table(self): handler1 = self.make_handler() handler2 = self.make_handler() - self.router.add_route('GET', '/h1', handler1) - self.router.add_route('POST', '/h2', handler2) + self.router.add_route('r1', '/h1') + self.router.add_view('GET', handler1, route='r1') + self.router.add_route('r2', '/h2') + self.router.add_view('POST', handler2, route='r2') + req = self.make_request('POST', '/h2') info = self.loop.run_until_complete(self.router.resolve(req)) self.assertIsNotNone(info) self.assertEqual({}, info) - self.assertIs(handler2, info.handler) + self.assertIs(handler2, info.handler.handler) def test_raise_method_not_allowed(self): handler1 = self.make_handler() handler2 = self.make_handler() - self.router.add_route('GET', '/', handler1) - self.router.add_route('POST', '/', handler2) + + self.router.add_route('default', '/') + self.router.add_view('GET', handler1, route='default') + self.router.add_view('POST', handler2, route='default') req = self.make_request('PUT', '/') match_info = self.loop.run_until_complete(self.router.resolve(req)) @@ -205,7 +211,8 @@ def test_raise_method_not_allowed(self): def test_raise_method_not_found(self): handler = self.make_handler() - self.router.add_route('GET', '/a', handler) + self.router.add_route('test', '/a') + self.router.add_view('GET', handler, route='test') req = self.make_request('GET', '/b') match_info = self.loop.run_until_complete(self.router.resolve(req)) @@ -218,18 +225,15 @@ def test_raise_method_not_found(self): exc = ctx.exception self.assertEqual(404, exc.status) - def test_double_add_url_with_the_same_name(self): - handler1 = self.make_handler() - handler2 = self.make_handler() - self.router.add_route('GET', '/get', handler1, name='name') + def test_double_add_route_with_the_same_name(self): + self.router.add_route('name', '/get') regexp = ("Duplicate 'name', already handled by") with self.assertRaisesRegex(ValueError, regexp): - self.router.add_route('GET', '/get_other', handler2, name='name') + self.router.add_route('name', '/get_other') def test_route_plain(self): - handler = self.make_handler() - route = self.router.add_route('GET', '/get', handler, name='name') + route = self.router.add_route('name', '/get') route2 = self.router['name'] url = route2.url() self.assertEqual('/get', url) @@ -240,10 +244,7 @@ def test_route_unknown_route_name(self): self.router['unknown'] def test_route_dynamic(self): - handler = self.make_handler() - route = self.router.add_route('GET', '/get/{name}', handler, - name='name') - + route = self.router.add_route('name', '/get/{name}') route2 = self.router['name'] url = route2.url(parts={'name': 'John'}) self.assertEqual('/get/John', url) @@ -251,7 +252,8 @@ def test_route_dynamic(self): def test_route_with_qs(self): handler = self.make_handler() - self.router.add_route('GET', '/get', handler, name='name') + self.router.add_route('name', '/get') + self.router.add_view('GET', handler, route='name') url = self.router['name'].url(query=[('a', 'b'), ('c', 1)]) self.assertEqual('/get?a=b&c=1', url) @@ -267,56 +269,63 @@ def test_add_static(self): def test_plain_not_match(self): handler = self.make_handler() - self.router.add_route('GET', '/get/path', handler, name='name') + self.router.add_route('name', '/get/path') + self.router.add_view('GET', handler, route='name') route = self.router['name'] - self.assertIsNone(route.match('/another/path')) + self.assertIsNone( + route.match(self.make_request('GET', '/another/path'))) def test_dynamic_not_match(self): - handler = self.make_handler() - self.router.add_route('GET', '/get/{name}', handler, name='name') + self.router.add_route('name', '/get/{name}') + self.router.add_view('GET', self.make_handler(), route='name') route = self.router['name'] - self.assertIsNone(route.match('/another/path')) + self.assertIsNone( + route.match(self.make_request('GET', '/another/path'))) def test_static_not_match(self): self.router.add_static('/pre', os.path.dirname(aiohttp.__file__), name='name') route = self.router['name'] - self.assertIsNone(route.match('/another/path')) + req = self.make_request('GET', '/another/path') + self.assertIsNone(route.match(req)) def test_dynamic_with_trailing_slash(self): handler = self.make_handler() - self.router.add_route('GET', '/get/{name}/', handler, name='name') + self.router.add_route('name', '/get/{name}/') + self.router.add_view('GET', handler, route='name') + route = self.router['name'] - self.assertEqual({'name': 'John'}, route.match('/get/John/')) + self.assertEqual( + {'name': 'John'}, + dict(route.match(self.make_request('GET', '/get/John/')))) def test_len(self): - handler = self.make_handler() - self.router.add_route('GET', '/get1', handler, name='name1') - self.router.add_route('GET', '/get2', handler, name='name2') + self.router.add_route('name1', '/get1') + self.router.add_route('name2', '/get2') self.assertEqual(2, len(self.router)) def test_iter(self): - handler = self.make_handler() - self.router.add_route('GET', '/get1', handler, name='name1') - self.router.add_route('GET', '/get2', handler, name='name2') + self.router.add_route('name1', '/get1') + self.router.add_route('name2', '/get2') self.assertEqual({'name1', 'name2'}, set(iter(self.router))) def test_contains(self): - handler = self.make_handler() - self.router.add_route('GET', '/get1', handler, name='name1') - self.router.add_route('GET', '/get2', handler, name='name2') + self.router.add_route('name1', '/get1') + self.router.add_route('name2', '/get2') self.assertIn('name1', self.router) self.assertNotIn('name3', self.router) def test_plain_repr(self): handler = self.make_handler() - self.router.add_route('GET', '/get/path', handler, name='name') + self.router.add_route('name', '/get/path') + self.router.add_view('GET', handler, route='name') self.assertRegex(repr(self.router['name']), r"+++)': nothing to repeat"), s) self.assertIsNone(ctx.exception.__cause__) def test_route_dynamic_with_regex_spec(self): - handler = self.make_handler() - route = self.router.add_route('GET', '/get/{num:^\d+}', handler, - name='name') - + route = self.router.add_route('name', '/get/{num:^\d+}') url = route.url(parts={'num': '123'}) self.assertEqual('/get/123', url) def test_route_dynamic_with_regex_spec_and_trailing_slash(self): - handler = self.make_handler() - route = self.router.add_route('GET', '/get/{num:^\d+}/', handler, - name='name') - + route = self.router.add_route('name', '/get/{num:^\d+}/') url = route.url(parts={'num': '123'}) self.assertEqual('/get/123/', url) def test_route_dynamic_with_regex(self): - handler = self.make_handler() - route = self.router.add_route('GET', r'/{one}/{two:.+}', handler) + route = self.router.add_route('name', r'/{one}/{two:.+}') url = route.url(parts={'one': 1, 'two': 2}) self.assertEqual('/1/2', url) @@ -420,13 +427,16 @@ def test_regular_match_info(self): @asyncio.coroutine def go(): handler = self.make_handler() - self.router.add_route('GET', '/get/{name}', handler) + self.router.add_route('test', '/get/{name}') + self.router.add_view('GET', handler, route='test') req = self.make_request('GET', '/get/john') match_info = yield from self.router.resolve(req) self.maxDiff = None - self.assertRegex(repr(match_info), - ">") + self.assertEqual( + repr(match_info), + "") self.loop.run_until_complete(go()) @@ -444,11 +454,13 @@ def test_not_allowed_repr(self): @asyncio.coroutine def go(): + self.router.add_route('r1', '/path/to') + handler = self.make_handler() - self.router.add_route('GET', '/path/to', handler) + self.router.add_view('GET', handler, route='r1') handler2 = self.make_handler() - self.router.add_route('POST', '/path/to', handler2) + self.router.add_view('POST', handler2, route='r1') req = self.make_request('PUT', '/path/to') match_info = yield from self.router.resolve(req) @@ -458,8 +470,8 @@ def go(): self.loop.run_until_complete(go()) def test_default_expect_handler(self): - route = self.router.add_route('GET', '/', self.make_handler()) - self.assertIs(route._expect_handler, _defaultExpectHandler) + route = self.router.add_route('r', '/') + self.assertIs(route._expect_handler, defaultExpectHandler) def test_custom_expect_handler_plain(self): @@ -467,8 +479,7 @@ def test_custom_expect_handler_plain(self): def handler(request): pass - route = self.router.add_route( - 'GET', '/', self.make_handler(), expect_handler=handler) + route = self.router.add_route('r', '/', expect_handler=handler) self.assertIs(route._expect_handler, handler) self.assertIsInstance(route, PlainRoute) @@ -479,7 +490,7 @@ def handler(request): pass route = self.router.add_route( - 'GET', '/get/{name}', self.make_handler(), expect_handler=handler) + 'r', '/get/{name}', expect_handler=handler) self.assertIs(route._expect_handler, handler) self.assertIsInstance(route, DynamicRoute) @@ -490,14 +501,14 @@ def handler(request): self.assertRaises( AssertionError, self.router.add_route, - 'GET', '/', self.make_handler(), expect_handler=handler) + 'test', '/', expect_handler=handler) def test_dynamic_match_non_ascii(self): @asyncio.coroutine def go(): - handler = self.make_handler() - self.router.add_route('GET', '/{var}', handler) + self.router.add_route('r', '/{var}') + self.router.add_view('GET', self.make_handler(), route='r') req = self.make_request( 'GET', '/%D1%80%D1%83%D1%81%20%D1%82%D0%B5%D0%BA%D1%81%D1%82') @@ -511,7 +522,8 @@ def test_dynamic_match_with_static_part(self): @asyncio.coroutine def go(): handler = self.make_handler() - self.router.add_route('GET', '/{name}.html', handler) + self.router.add_route('name', '/{name}.html') + self.router.add_view('GET', handler, route='name') req = self.make_request('GET', '/file.html') match_info = yield from self.router.resolve(req) self.assertEqual({'name': 'file'}, match_info) @@ -522,8 +534,8 @@ def test_dynamic_match_two_part2(self): @asyncio.coroutine def go(): - handler = self.make_handler() - self.router.add_route('GET', '/{name}.{ext}', handler) + self.router.add_route('r', '/{name}.{ext}') + self.router.add_view('GET', self.make_handler(), route='r') req = self.make_request('GET', '/file.html') match_info = yield from self.router.resolve(req) self.assertEqual({'name': 'file', 'ext': 'html'}, match_info) @@ -534,8 +546,8 @@ def test_dynamic_match_unquoted_path(self): @asyncio.coroutine def go(): - handler = self.make_handler() - self.router.add_route('GET', '/{path}/{subpath}', handler) + self.router.add_route('r', '/{path}/{subpath}') + self.router.add_view('GET', self.make_handler(), route='r') resource_id = 'my%2Fpath%7Cwith%21some%25strange%24characters' req = self.make_request('GET', '/path/{0}'.format(resource_id)) match_info = yield from self.router.resolve(req) @@ -548,10 +560,4 @@ def go(): def test_add_route_not_started_with_slash(self): with self.assertRaises(ValueError): - handler = self.make_handler() - self.router.add_route('GET', 'invalid_path', handler) - - def test_add_route_invalid_method(self): - with self.assertRaises(ValueError): - handler = self.make_handler() - self.router.add_route('INVALID_METHOD', '/path', handler) + self.router.add_route('r', 'invalid_path') diff --git a/tests/test_web.py b/tests/test_web.py index b7626d80e2c..4f9ae5019d2 100644 --- a/tests/test_web.py +++ b/tests/test_web.py @@ -22,7 +22,8 @@ def test_handler_returns_not_response(self): def handler(request): return 'abc' - app.router.add_route('GET', '/', handler) + app.router.add_route('r', '/') + app.router.add_view('GET', handler, route='r') h = app.make_handler()() message = RawRequestMessage('GET', '/', HttpVersion11, CIMultiDict(), False, False) diff --git a/tests/test_web_functional.py b/tests/test_web_functional.py index c98e63cf8f8..27f3ed42d42 100644 --- a/tests/test_web_functional.py +++ b/tests/test_web_functional.py @@ -32,7 +32,8 @@ def find_unused_port(self): def create_server(self, method, path, handler=None): app = web.Application(loop=self.loop) if handler: - app.router.add_route(method, path, handler) + app.router.add_route('r', path) + app.router.add_view(method, handler, route='r') port = self.find_unused_port() self.handler = app.make_handler( @@ -576,7 +577,9 @@ def go(): app, _, url = yield from self.create_server('POST', '/') app.router.add_route( - 'POST', '/', handler, expect_handler=expect_handler) + 'r', '/', expect_handler=expect_handler) + app.router.add_view( + 'POST', handler, route='r') form = FormData() form.add_field('name', b'123', @@ -617,7 +620,9 @@ def go(): app, _, url = yield from self.create_server('POST', '/') app.router.add_route( - 'POST', '/', handler, expect_handler=expect_handler) + 'r', '/', expect_handler=expect_handler) + app.router.add_view( + 'POST', handler, route='r') form = FormData() form.add_field('name', b'123', @@ -650,7 +655,8 @@ def handler(request): @asyncio.coroutine def go(): app, _, url = yield from self.create_server('POST', '/') - app.router.add_route('POST', '/', handler) + app.router.add_route('r', '/') + app.router.add_view('POST', handler, route='r') form = FormData() form.add_field('name', b'123', @@ -675,7 +681,8 @@ def handler(request): @asyncio.coroutine def go(): app, _, url = yield from self.create_server('POST', '/') - app.router.add_route('POST', '/', handler) + app.router.add_route('r', '/') + app.router.add_view('POST', handler, route='r') form = FormData() form.add_field('name', b'123', diff --git a/tests/test_web_middleware.py b/tests/test_web_middleware.py index 6cec6b417cf..b07f38cbfd2 100644 --- a/tests/test_web_middleware.py +++ b/tests/test_web_middleware.py @@ -26,7 +26,8 @@ def find_unused_port(self): @asyncio.coroutine def create_server(self, method, path, handler, *middlewares): app = web.Application(loop=self.loop, middlewares=middlewares) - app.router.add_route(method, path, handler) + app.router.add_route('r', path) + app.router.add_view(method, handler, route='r') port = self.find_unused_port() self.handler = app.make_handler(debug=True) diff --git a/tests/test_web_websocket_functional.py b/tests/test_web_websocket_functional.py index f4e410eca7b..d95c4fda090 100644 --- a/tests/test_web_websocket_functional.py +++ b/tests/test_web_websocket_functional.py @@ -31,7 +31,8 @@ def find_unused_port(self): @asyncio.coroutine def create_server(self, method, path, handler): app = web.Application(loop=self.loop) - app.router.add_route(method, path, handler) + app.router.add_route('r', path) + app.router.add_view(method, handler, route='r') port = self.find_unused_port() srv = yield from self.loop.create_server( diff --git a/tests/test_websocket_client.py b/tests/test_websocket_client.py index debb58ca215..b12572c24ad 100644 --- a/tests/test_websocket_client.py +++ b/tests/test_websocket_client.py @@ -380,6 +380,7 @@ def test_receive_runtime_err(self): class TestWebSocketClientFunctional(unittest.TestCase): def setUp(self): + self.handler = None self.loop = asyncio.new_event_loop() asyncio.set_event_loop(None) @@ -399,7 +400,8 @@ def find_unused_port(self): @asyncio.coroutine def create_server(self, method, path, handler): app = web.Application(loop=self.loop) - app.router.add_route(method, path, handler) + app.router.add_route('r', path) + app.router.add_view(method, handler, route='r') port = self.find_unused_port() self.handler = app.make_handler() From 2fea648c2fa5b06b6e251e616cdbca2effc280ba Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Thu, 9 Jul 2015 15:46:44 -0700 Subject: [PATCH 2/4] fix typo --- aiohttp/web_urldispatcher.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aiohttp/web_urldispatcher.py b/aiohttp/web_urldispatcher.py index 24daf566e51..fe523cf6b06 100644 --- a/aiohttp/web_urldispatcher.py +++ b/aiohttp/web_urldispatcher.py @@ -172,7 +172,7 @@ def match(self, request): def handler(self, request, exc): request.match_info['exception'] = exc - if self._view or not None: + if self._view is not None: return (yield from self._view(request)) else: return exc From 4d530f6e86614ed0c9292052d25a2f17dfc8ec55 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Thu, 9 Jul 2015 15:52:12 -0700 Subject: [PATCH 3/4] fix typo --- aiohttp/web_urldispatcher.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aiohttp/web_urldispatcher.py b/aiohttp/web_urldispatcher.py index fe523cf6b06..2393bfa8b9d 100644 --- a/aiohttp/web_urldispatcher.py +++ b/aiohttp/web_urldispatcher.py @@ -412,7 +412,7 @@ def add_view(self, method, handler, *, name='', route=None): raise ValueError('Route name is required') if route not in self._routes: - raise ValueError('Route is not found {}', route) + raise ValueError('Route is not found {}'.format(route)) route = self._routes[route] route.add_view(method, handler, name=name) From 28d2048ee82ef20e69e0b7c0dda06e7e79f1b14a Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Thu, 9 Jul 2015 15:58:28 -0700 Subject: [PATCH 4/4] move handler checks to View ctor --- aiohttp/web_urldispatcher.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/aiohttp/web_urldispatcher.py b/aiohttp/web_urldispatcher.py index 2393bfa8b9d..7f2429677f8 100644 --- a/aiohttp/web_urldispatcher.py +++ b/aiohttp/web_urldispatcher.py @@ -278,6 +278,11 @@ def __repr__(self): class View(metaclass=abc.ABCMeta): def __init__(self, handler, name='', method=hdrs.METH_ANY): + assert callable(handler), handler + if (not asyncio.iscoroutinefunction(handler) and + not inspect.isgeneratorfunction(handler)): + handler = asyncio.coroutine(handler) + self._name = name self._method = upstr(method) self._handler = handler @@ -402,12 +407,6 @@ def register_route(self, route): self._urls.append(route) def add_view(self, method, handler, *, name='', route=None): - - assert callable(handler), handler - if (not asyncio.iscoroutinefunction(handler) and - not inspect.isgeneratorfunction(handler)): - handler = asyncio.coroutine(handler) - if route is None: raise ValueError('Route name is required') @@ -418,7 +417,6 @@ def add_view(self, method, handler, *, name='', route=None): route.add_view(method, handler, name=name) def add_route(self, name, path, *, expect_handler=None): - if not path.startswith('/'): raise ValueError("path should be started with /")