From 25fad1b28f2b1d8e24c13809e16b37b4aa08691a Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Tue, 4 Oct 2016 19:04:33 +0300 Subject: [PATCH 01/12] Work on dispatcher refactoring --- aiohttp/web_urldispatcher.py | 288 ++++++++++++++--------------------- 1 file changed, 117 insertions(+), 171 deletions(-) diff --git a/aiohttp/web_urldispatcher.py b/aiohttp/web_urldispatcher.py index e71a79e14f9..05130fbc085 100644 --- a/aiohttp/web_urldispatcher.py +++ b/aiohttp/web_urldispatcher.py @@ -10,9 +10,8 @@ from collections.abc import Container, Iterable, Sized from pathlib import Path from types import MappingProxyType -from urllib.parse import urlencode -from yarl import quote, unquote +from yarl import URL, quote, unquote from . import hdrs from .abc import AbstractMatchInfo, AbstractRouter, AbstractView @@ -24,9 +23,8 @@ __all__ = ('UrlDispatcher', 'UrlMappingMatchInfo', 'AbstractResource', 'Resource', 'PlainResource', 'DynamicResource', - 'ResourceAdapter', 'AbstractRoute', 'ResourceRoute', - 'Route', 'PlainRoute', 'DynamicRoute', 'StaticRoute', 'View') + 'Route', 'StaticResource', 'View') PY_35 = sys.version_info >= (3, 5) @@ -46,6 +44,17 @@ def name(self): @abc.abstractmethod # pragma: no branch def url(self, **kwargs): + """Construct url for resource with additional params. + + Deprecated, use url_for() instead. + + """ + warnings.warn(".url(...) is deprecated, use .url_for instead", + DeprecationWarning, + stacklevel=3) + + @abc.abstractmethod # pragma: no branch + def url_for(self, **kwargs): """Construct url for resource with additional params.""" @asyncio.coroutine @@ -59,13 +68,6 @@ def resolve(self, method, path): def get_info(self): """Return a dict with additional info useful for introspection""" - @staticmethod - def _append_query(url, query): - if query: - return url + "?" + urlencode(query) - else: - return url - class AbstractRoute(abc.ABC): @@ -196,40 +198,6 @@ def _defaultExpectHandler(request): raise HTTPExpectationFailed(text="Unknown Expect: %s" % expect) -class ResourceAdapter(AbstractResource): - - def __init__(self, route): - assert isinstance(route, Route), \ - 'Instance of Route class is required, got {!r}'.format(route) - super().__init__(name=route.name) - self._route = route - route._resource = self - - def url(self, **kwargs): - return self._route.url(**kwargs) - - @asyncio.coroutine - def resolve(self, method, path): - route_method = self._route.method - allowed_methods = set() - match_dict = self._route.match(path) - if match_dict is not None: - allowed_methods.add(route_method) - if route_method == hdrs.METH_ANY or route_method == method: - return (UrlMappingMatchInfo(match_dict, self._route), - allowed_methods) - return None, allowed_methods - - def get_info(self): - return self._route.get_info() - - def __len__(self): - return 1 - - def __iter__(self): - yield self._route - - class Resource(AbstractResource): def __init__(self, *, name=None): @@ -296,7 +264,11 @@ def get_info(self): return {'path': self._path} def url(self, *, query=None): - return self._append_query(self._path, query) + super().url() + return self.url_for().with_query(query) + + def url_for(self): + return URL(self._path) def __repr__(self): name = "'" + self.name + "' " if self.name is not None else "" @@ -323,9 +295,13 @@ def get_info(self): return {'formatter': self._formatter, 'pattern': self._pattern} - def url(self, *, parts, query=None): + def url_for(self, **parts): url = self._formatter.format_map(parts) - return self._append_query(url, query) + return URL(url) + + def url(self, *, parts, query=None): + super().url(**parts) + return self.url_for(**parts).with_query(query) def __repr__(self): name = "'" + self.name + "' " if self.name is not None else "" @@ -333,120 +309,28 @@ def __repr__(self): .format(name=name, formatter=self._formatter)) -class ResourceRoute(AbstractRoute): - """A route with resource""" - - def __init__(self, method, handler, resource, *, - expect_handler=None): - super().__init__(method, handler, expect_handler=expect_handler, - resource=resource) - - def __repr__(self): - return " {handler!r}".format( - method=self.method, resource=self._resource, - handler=self.handler) - - @property - def name(self): - return self._resource.name - - def url(self, **kwargs): - """Construct url for route with additional params.""" - return self._resource.url(**kwargs) - - def get_info(self): - return self._resource.get_info() - - _append_query = staticmethod(Resource._append_query) - - -class Route(AbstractRoute): - """Old fashion route""" - - def __init__(self, method, handler, name, *, expect_handler=None): - super().__init__(method, handler, expect_handler=expect_handler) - self._name = name - - @property - def name(self): - return self._name - - @abc.abstractmethod - def match(self, path): - """Return dict with info for given path or - None if route cannot process path.""" - - _append_query = staticmethod(Resource._append_query) - - -class PlainRoute(Route): - - def __init__(self, method, handler, name, path, *, expect_handler=None): - super().__init__(method, handler, name, expect_handler=expect_handler) - self._path = path - - def match(self, path): - # string comparison is about 10 times faster than regexp matching - if self._path == path: - return {} - else: - return None - - def url(self, *, query=None): - return self._append_query(self._path, query) - - def get_info(self): - return {'path': self._path} - - def __repr__(self): - 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) - - -class DynamicRoute(Route): - - def __init__(self, method, handler, name, pattern, formatter, *, - expect_handler=None): - super().__init__(method, handler, name, expect_handler=expect_handler) - self._pattern = pattern - self._formatter = formatter - - def match(self, path): - match = self._pattern.match(path) - if match is None: - return None - else: - return match.groupdict() +class PrefixResource(AbstractResource): - def url(self, *, parts, query=None): - url = self._formatter.format_map(parts) - return self._append_query(url, query) - - def get_info(self): - return {'formatter': self._formatter, - 'pattern': self._pattern} + def __init__(self, prefix, *, name=None): + assert prefix.startswith('/'), prefix + assert prefix.endswith('/'), prefix + super().__init__(name=name) + self._prefix = quote(prefix, safe='/') + self._prefix_len = len(self._prefix) def __repr__(self): name = "'" + self.name + "' " if self.name is not None else "" - return (" {handler!r}" - .format(name=name, method=self.method, - formatter=self._formatter, handler=self.handler)) + return (" {directory!r}".format( + fmt = " {directory!r}" + return fmt.format( name=name, method=self.method, path=self._prefix, directory=self._directory) +class ResourceRoute(AbstractRoute): + """A route with resource""" + + def __init__(self, method, handler, resource, *, + expect_handler=None): + super().__init__(method, handler, expect_handler=expect_handler, + resource=resource) + + def __repr__(self): + return " {handler!r}".format( + method=self.method, resource=self._resource, + handler=self.handler) + + @property + def name(self): + return self._resource.name + + def url(self, **kwargs): + """Construct url for route with additional params.""" + super().url(**kwargs) + return self._resource.url(**kwargs) + + def get_info(self): + return self._resource.get_info() + + +class Route(AbstractRoute): + """Old fashion route""" + + def __init__(self, method, handler, name, *, expect_handler=None): + super().__init__(method, handler, expect_handler=expect_handler) + self._name = name + + @property + def name(self): + return self._name + + @abc.abstractmethod + def match(self, path): + """Return dict with info for given path or + None if route cannot process path.""" + + class SystemRoute(Route): def __init__(self, http_exception): @@ -699,11 +650,6 @@ def named_routes(self): warnings.warn("Use .named_resources instead", DeprecationWarning) return self.named_resources() - def register_route(self, route): - warnings.warn("Use resource-based interface", DeprecationWarning) - resource = ResourceAdapter(route) - self._reg_resource(resource) - def _reg_resource(self, resource): assert isinstance(resource, AbstractResource), \ 'Instance of AbstractResource class is required, got {!r}'.format( @@ -784,14 +730,14 @@ def add_static(self, prefix, path, *, name=None, expect_handler=None, assert prefix.startswith('/') if not prefix.endswith('/'): prefix += '/' - prefix = quote(prefix, safe='/') - route = StaticRoute(name, prefix, path, - expect_handler=expect_handler, - chunk_size=chunk_size, - response_factory=response_factory, - show_index=show_index) - self.register_route(route) - return route + resource = StaticResource(prefix, path, + name=name, + expect_handler=expect_handler, + chunk_size=chunk_size, + response_factory=response_factory, + show_index=show_index) + self._reg_resource(resource) + return resource def add_head(self, *args, **kwargs): """ From e71d325dbc7b851abdac3e4ceded10781f7f9a81 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Tue, 4 Oct 2016 23:18:30 +0300 Subject: [PATCH 02/12] Work on --- aiohttp/web_urldispatcher.py | 25 ++-- tests/test_urldispatch.py | 171 ++++---------------------- tests/test_web_sendfile_functional.py | 4 +- tests/test_web_urldispatcher.py | 24 +--- 4 files changed, 38 insertions(+), 186 deletions(-) diff --git a/aiohttp/web_urldispatcher.py b/aiohttp/web_urldispatcher.py index 05130fbc085..fea406af5c2 100644 --- a/aiohttp/web_urldispatcher.py +++ b/aiohttp/web_urldispatcher.py @@ -265,7 +265,7 @@ def get_info(self): def url(self, *, query=None): super().url() - return self.url_for().with_query(query) + return str(self.url_for().with_query(query)) def url_for(self): return URL(self._path) @@ -301,7 +301,7 @@ def url_for(self, **parts): def url(self, *, parts, query=None): super().url(**parts) - return self.url_for(**parts).with_query(query) + return str(self.url_for(**parts).with_query(query)) def __repr__(self): name = "'" + self.name + "' " if self.name is not None else "" @@ -326,7 +326,7 @@ def __repr__(self): class StaticResource(PrefixResource): - def __init__(self, name, prefix, directory, *, + def __init__(self, prefix, directory, *, name=None, expect_handler=None, chunk_size=256*1024, response_factory=StreamResponse, show_index=False): @@ -346,14 +346,14 @@ def __init__(self, name, prefix, directory, *, chunk_size=chunk_size) self._show_index = show_index - self._routes = {'GET': ResourceRoute('GET', self._handler, self, + self._routes = {'GET': ResourceRoute('GET', self._handle, self, expect_handler=expect_handler), - 'HEAD': ResourceRoute('HEAD', self._handler, self, + 'HEAD': ResourceRoute('HEAD', self._handle, self, expect_handler=expect_handler)} def url(self, *, filename, query=None): - return self.url_for(filename=filename).with_query(query) + return str(self.url_for(filename=filename).with_query(query)) def url_for(self, *, filename): if isinstance(filename, Path): @@ -371,7 +371,7 @@ def get_info(self): def resolve(self, method, path): allowed_methods = {'GET', 'HEAD'} if not path.startswith(self._prefix): - return None, allowed_methods + return None, set() if method not in allowed_methods: return None, allowed_methods @@ -456,10 +456,8 @@ def _directory_as_html(self, filepath): def __repr__(self): name = "'" + self.name + "' " if self.name is not None else "" - fmt = " {directory!r}" - return fmt.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 ResourceRoute(AbstractRoute): @@ -645,11 +643,6 @@ def routes(self): def named_resources(self): return MappingProxyType(self._named_resources) - def named_routes(self): - # NB: it's ambiguous but it's really resources. - warnings.warn("Use .named_resources instead", DeprecationWarning) - return self.named_resources() - def _reg_resource(self, resource): assert isinstance(resource, AbstractResource), \ 'Instance of AbstractResource class is required, got {!r}'.format( diff --git a/tests/test_urldispatch.py b/tests/test_urldispatch.py index 7fcdfa91369..794091f51d0 100644 --- a/tests/test_urldispatch.py +++ b/tests/test_urldispatch.py @@ -11,8 +11,8 @@ from aiohttp.test_utils import make_mocked_request from aiohttp.web import (HTTPMethodNotAllowed, HTTPNotFound, Response, UrlDispatcher) -from aiohttp.web_urldispatcher import (AbstractResource, DynamicRoute, - PlainRoute, ResourceRoute, SystemRoute, +from aiohttp.web_urldispatcher import (AbstractResource, + ResourceRoute, SystemRoute, View, _defaultExpectHandler) @@ -37,25 +37,6 @@ def handler(request): return handler - 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') - self.router.register_route(route) - self.assertRaises(ValueError, self.router.register_route, route) - - route = PlainRoute('GET', handler, '1bad name', '/handler/to/path') - self.assertRaises(ValueError, self.router.register_route, route) - - route = PlainRoute('GET', handler, 'return', '/handler/to/path') - self.assertRaises(ValueError, self.router.register_route, route) - - route = PlainRoute('GET', handler, 'test.test:test-test', - '/handler/to/path') - self.router.register_route(route) - def test_register_uncommon_http_methods(self): handler = self.make_handler() @@ -72,7 +53,7 @@ def test_register_uncommon_http_methods(self): } for method in uncommon_http_methods: - PlainRoute(method, handler, 'url', '/handler/to/path') + self.router.add_route(method, '/handler/to/path', handler) def test_add_route_root(self): handler = self.make_handler() @@ -305,13 +286,13 @@ def test_route_with_qs(self): self.assertEqual('/get?a=b&c=1', url) def test_add_static(self): - route = self.router.add_static('/st', - os.path.dirname(aiohttp.__file__), - name='static') - resource = self.router['static'] + resource = self.router.add_static('/st', + os.path.dirname(aiohttp.__file__), + name='static') + assert self.router['static'] is resource url = resource.url(filename='/dir/a.txt') self.assertEqual('/st/dir/a.txt', url) - self.assertIs(route, next(iter(resource))) + assert len(resource) == 2 def test_plain_not_match(self): handler = self.make_handler() @@ -328,8 +309,9 @@ def test_dynamic_not_match(self): 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._route.match('/another/path')) + resource = self.router['name'] + self.assertIsNone(self.loop.run_until_complete( + resource.resolve('/another/path'))) def test_dynamic_with_trailing_slash(self): handler = self.make_handler() @@ -356,24 +338,11 @@ def test_contains(self): self.assertIn('name1', self.router) self.assertNotIn('name3', self.router) - def test_plain_repr(self): - handler = self.make_handler() - route = PlainRoute('GET', handler, 'name', '/get/path') - self.assertRegex(repr(route), - r"', '/{path}') - self.router.register_route(route) - self.assertEqual('/path?arg=1', route.url(parts={'path': 'path'}, - query={'arg': 1})) - - def test_dynamic_route_match_not_found(self): - route = DynamicRoute('GET', lambda req: None, None, - re.compile('/path/(?P.+)'), '/path/{to}') - self.router.register_route(route) - self.assertEqual(None, route.match('/another/path')) - - def test_dynamic_route_match_found(self): - route = DynamicRoute('GET', lambda req: None, None, - re.compile('/path/(?P.+)'), '/path/{to}') - self.router.register_route(route) - self.assertEqual({'to': 'to'}, route.match('/path/to')) - - def test_deprecate_register_route(self): - route = PlainRoute('GET', lambda req: None, None, '/path') - with self.assertWarns(DeprecationWarning): - self.router.register_route(route) - def test_error_on_double_route_adding(self): resource = self.router.add_resource('/path') @@ -819,23 +714,9 @@ def test_match_info_get_info_dynamic(self): def test_resource_adapter_get_info(self): directory = pathlib.Path(aiohttp.__file__).parent - route = self.router.add_static('/st', directory) - self.assertEqual(route.resource.get_info(), {'directory': directory, - 'prefix': '/st/'}) - - def test_plain_old_style_route_get_info(self): - handler = self.make_handler() - route = PlainRoute('GET', handler, 'test', '/handler/to/path') - self.router.register_route(route) - self.assertEqual(route.get_info(), {'path': '/handler/to/path'}) - - def test_dynamic_old_style_get_info(self): - handler = self.make_handler() - route = DynamicRoute('GET', handler, 'name', - '', '/get/{path}') - self.router.register_route(route) - self.assertEqual(route.get_info(), {'formatter': '/get/{path}', - 'pattern': ''}) + resource = self.router.add_static('/st', directory) + self.assertEqual(resource.get_info(), {'directory': directory, + 'prefix': '/st/'}) def test_system_route_get_info(self): handler = self.make_handler() @@ -881,21 +762,19 @@ def test_static_route_points_to_file(self): with self.assertRaises(ValueError): self.router.add_static('/st', here) - def test_404_for_resource_adapter(self): - route = self.router.add_static('/st', - os.path.dirname(aiohttp.__file__)) - resource = route.resource + def test_404_for_static_resource(self): + resource = self.router.add_static('/st', + os.path.dirname(aiohttp.__file__)) ret = self.loop.run_until_complete( resource.resolve('GET', '/unknown/path')) self.assertEqual((None, set()), ret) def test_405_for_resource_adapter(self): - route = self.router.add_static('/st', - os.path.dirname(aiohttp.__file__)) - resource = route.resource + resource = self.router.add_static('/st', + os.path.dirname(aiohttp.__file__)) ret = self.loop.run_until_complete( resource.resolve('POST', '/st/abc.py')) - self.assertEqual((None, {'GET'}), ret) + self.assertEqual((None, {'HEAD', 'GET'}), ret) def test_check_allowed_method_for_found_resource(self): handler = self.make_handler() diff --git a/tests/test_web_sendfile_functional.py b/tests/test_web_sendfile_functional.py index bf720c6f969..cf2807a43ff 100644 --- a/tests/test_web_sendfile_functional.py +++ b/tests/test_web_sendfile_functional.py @@ -339,11 +339,11 @@ def go(dirname, relpath): def test_static_route_path_existence_check(self): directory = os.path.dirname(__file__) - web.StaticRoute(None, "/", directory) + web.StaticResource("/", directory) nodirectory = os.path.join(directory, "nonexistent-uPNiOEAg5d") with self.assertRaises(ValueError): - web.StaticRoute(None, "/", nodirectory) + web.StaticResource("/", nodirectory) def test_static_file_huge(self): diff --git a/tests/test_web_urldispatcher.py b/tests/test_web_urldispatcher.py index 290c50535a3..57a1f4f3259 100644 --- a/tests/test_web_urldispatcher.py +++ b/tests/test_web_urldispatcher.py @@ -9,9 +9,8 @@ import pytest import aiohttp.web -from aiohttp.test_utils import make_mocked_request -from aiohttp.web import HTTPCreated, Response -from aiohttp.web_urldispatcher import PlainRoute, SystemRoute, UrlDispatcher +from aiohttp.web import HTTPCreated +from aiohttp.web_urldispatcher import SystemRoute @pytest.fixture(scope='function') @@ -232,22 +231,3 @@ def test_system_route(): assert "" == repr(route) assert 201 == route.status assert 'test' == route.reason - - -@asyncio.coroutine -def test_register_route(): - @asyncio.coroutine - def handler(request): - return Response() - - route = PlainRoute('GET', handler, 'test', '/handler/to/path') - router = UrlDispatcher() - router.register_route(route) - - req = make_mocked_request('GET', '/handler/to/path') - info = yield from router.resolve(req) - assert info is not None - assert 0 == len(info) - assert route is info.route - assert handler is info.handler - assert info.route.name == 'test' From c8d7b0da0cb966d9a527d232546b8faa12b66d89 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Wed, 5 Oct 2016 17:45:50 +0300 Subject: [PATCH 03/12] Fix test --- 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 fea406af5c2..4bfa6141de8 100644 --- a/aiohttp/web_urldispatcher.py +++ b/aiohttp/web_urldispatcher.py @@ -455,7 +455,7 @@ def _directory_as_html(self, filepath): return html def __repr__(self): - name = "'" + self.name + "' " if self.name is not None else "" + name = "'" + self.name + "'" if self.name is not None else "" return " {directory!r}".format( name=name, path=self._prefix, directory=self._directory) From 2d4608fc1025c4b3652ea88c096b4b5b0bc7842d Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Wed, 5 Oct 2016 17:54:10 +0300 Subject: [PATCH 04/12] Fix more tests --- tests/test_urldispatch.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/tests/test_urldispatch.py b/tests/test_urldispatch.py index 794091f51d0..d8736d55bcb 100644 --- a/tests/test_urldispatch.py +++ b/tests/test_urldispatch.py @@ -310,8 +310,9 @@ def test_static_not_match(self): self.router.add_static('/pre', os.path.dirname(aiohttp.__file__), name='name') resource = self.router['name'] - self.assertIsNone(self.loop.run_until_complete( - resource.resolve('/another/path'))) + self.assertEqual((None, set()), + self.loop.run_until_complete( + resource.resolve('GET', '/another/path'))) def test_dynamic_with_trailing_slash(self): handler = self.make_handler() @@ -591,9 +592,9 @@ def fill_routes(self): route1 = self.router.add_route('GET', '/plain', self.make_handler()) route2 = self.router.add_route('GET', '/variable/{name}', self.make_handler()) - route3 = self.router.add_static('/static', - os.path.dirname(aiohttp.__file__)) - return route1, route2, route3 + resource = self.router.add_static('/static', + os.path.dirname(aiohttp.__file__)) + return [route1, route2] + list(resource) def test_routes_view_len(self): self.fill_routes() From e7d32f01164a818d72958c63a5f0c5a806a733aa Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Wed, 5 Oct 2016 17:55:48 +0300 Subject: [PATCH 05/12] Fix failed test --- tests/test_urldispatch.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/test_urldispatch.py b/tests/test_urldispatch.py index d8736d55bcb..0bef38523bf 100644 --- a/tests/test_urldispatch.py +++ b/tests/test_urldispatch.py @@ -624,10 +624,6 @@ def fill_named_resources(self): name='route3') return route1.name, route2.name, route3.name - def test_named_routes_abc(self): - self.assertIsInstance(self.router.named_routes(), Mapping) - self.assertNotIsInstance(self.router.named_routes(), MutableMapping) - def test_named_resources_abc(self): self.assertIsInstance(self.router.named_resources(), Mapping) self.assertNotIsInstance(self.router.named_resources(), MutableMapping) From 98d3448c9435732cdab92b1a2e2b5baf97920687 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Wed, 5 Oct 2016 18:14:27 +0300 Subject: [PATCH 06/12] Fix the last failed test --- tests/test_urldispatch.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_urldispatch.py b/tests/test_urldispatch.py index 0bef38523bf..da890f59fd9 100644 --- a/tests/test_urldispatch.py +++ b/tests/test_urldispatch.py @@ -282,7 +282,7 @@ def test_route_with_qs(self): handler = self.make_handler() self.router.add_route('GET', '/get', handler, name='name') - url = self.router['name'].url(query=[('a', 'b'), ('c', 1)]) + url = self.router['name'].url(query=[('a', 'b'), ('c', '1')]) self.assertEqual('/get?a=b&c=1', url) def test_add_static(self): From 9e5a90814a9da8dfb075a66f77bf5e0a6120d39c Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Thu, 6 Oct 2016 13:15:02 +0300 Subject: [PATCH 07/12] Drop old Route class --- aiohttp/web_urldispatcher.py | 48 +++++++++++++++++---------------- tests/test_web_urldispatcher.py | 5 +++- 2 files changed, 29 insertions(+), 24 deletions(-) diff --git a/aiohttp/web_urldispatcher.py b/aiohttp/web_urldispatcher.py index 4bfa6141de8..17eb70e377e 100644 --- a/aiohttp/web_urldispatcher.py +++ b/aiohttp/web_urldispatcher.py @@ -24,7 +24,7 @@ __all__ = ('UrlDispatcher', 'UrlMappingMatchInfo', 'AbstractResource', 'Resource', 'PlainResource', 'DynamicResource', 'AbstractRoute', 'ResourceRoute', - 'Route', 'StaticResource', 'View') + 'StaticResource', 'View') PY_35 = sys.version_info >= (3, 5) @@ -131,9 +131,20 @@ def get_info(self): """Return a dict with additional info useful for introspection""" @abc.abstractmethod # pragma: no branch - def url(self, **kwargs): + def url_for(self, *args, **kwargs): """Construct url for route with additional params.""" + @abc.abstractmethod # pragma: no branch + def url(self, **kwargs): + """Construct url for resource with additional params. + + Deprecated, use url_for() instead. + + """ + warnings.warn(".url(...) is deprecated, use .url_for instead", + DeprecationWarning, + stacklevel=3) + @asyncio.coroutine def handle_expect_header(self, request): return (yield from self._expect_handler(request)) @@ -477,6 +488,10 @@ def __repr__(self): def name(self): return self._resource.name + def url_for(self, *args, **kwargs): + """Construct url for route with additional params.""" + return self._resource.url_for(*args, **kwargs) + def url(self, **kwargs): """Construct url for route with additional params.""" super().url(**kwargs) @@ -486,33 +501,20 @@ def get_info(self): return self._resource.get_info() -class Route(AbstractRoute): - """Old fashion route""" - - def __init__(self, method, handler, name, *, expect_handler=None): - super().__init__(method, handler, expect_handler=expect_handler) - self._name = name - - @property - def name(self): - return self._name - - @abc.abstractmethod - def match(self, path): - """Return dict with info for given path or - None if route cannot process path.""" - - -class SystemRoute(Route): +class SystemRoute(AbstractRoute): def __init__(self, http_exception): - super().__init__(hdrs.METH_ANY, self._handler, None) + super().__init__(hdrs.METH_ANY, self._handler) self._http_exception = http_exception - def url(self, **kwargs): + def url_for(self, *args, **kwargs): + raise RuntimeError(".url_for() is not allowed for SystemRoute") + + def url(self, *args, **kwargs): raise RuntimeError(".url() is not allowed for SystemRoute") - def match(self, path): + @property + def name(self): return None def get_info(self): diff --git a/tests/test_web_urldispatcher.py b/tests/test_web_urldispatcher.py index 57a1f4f3259..3df91fd2dc3 100644 --- a/tests/test_web_urldispatcher.py +++ b/tests/test_web_urldispatcher.py @@ -225,9 +225,12 @@ def handler(data, request): def test_system_route(): route = SystemRoute(HTTPCreated(reason='test')) - assert route.match('any') is None with pytest.raises(RuntimeError): route.url() + with pytest.raises(RuntimeError): + route.url_for() + assert route.name is None + assert route.resource is None assert "" == repr(route) assert 201 == route.status assert 'test' == route.reason From 16481d484bd5c928610e914fe30354e7046e185b Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Thu, 6 Oct 2016 13:26:03 +0300 Subject: [PATCH 08/12] Drop useless repr --- aiohttp/web_urldispatcher.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/aiohttp/web_urldispatcher.py b/aiohttp/web_urldispatcher.py index 17eb70e377e..4882e21f654 100644 --- a/aiohttp/web_urldispatcher.py +++ b/aiohttp/web_urldispatcher.py @@ -329,11 +329,6 @@ def __init__(self, prefix, *, name=None): self._prefix = quote(prefix, safe='/') self._prefix_len = len(self._prefix) - def __repr__(self): - name = "'" + self.name + "' " if self.name is not None else "" - return (" Date: Thu, 6 Oct 2016 15:05:38 +0300 Subject: [PATCH 09/12] Convert urldispatcher tests from unittest to pytest style --- tests/test_urldispatch.py | 1577 +++++++++++++++++++------------------ 1 file changed, 832 insertions(+), 745 deletions(-) diff --git a/tests/test_urldispatch.py b/tests/test_urldispatch.py index da890f59fd9..1d67d4170bb 100644 --- a/tests/test_urldispatch.py +++ b/tests/test_urldispatch.py @@ -2,10 +2,11 @@ import os import pathlib import re -import unittest from collections.abc import Container, Iterable, Mapping, MutableMapping, Sized from urllib.parse import unquote +import pytest + import aiohttp.web from aiohttp import hdrs from aiohttp.test_utils import make_mocked_request @@ -16,767 +17,853 @@ View, _defaultExpectHandler) -class TestUrlDispatcher(unittest.TestCase): - - def setUp(self): - self.loop = asyncio.new_event_loop() - asyncio.set_event_loop(None) - self.router = UrlDispatcher() - - def tearDown(self): - self.loop.close() - - def make_request(self, method, path): - return make_mocked_request(method, path) - - def make_handler(self): - - @asyncio.coroutine - def handler(request): - return Response(request) # pragma: no cover - - return handler - - def test_register_uncommon_http_methods(self): - handler = self.make_handler() - - uncommon_http_methods = { - 'PROPFIND', - 'PROPPATCH', - 'COPY', - 'LOCK', - 'UNLOCK' - 'MOVE', - 'SUBSCRIBE', - 'UNSUBSCRIBE', - 'NOTIFY' - } - - for method in uncommon_http_methods: - self.router.add_route(method, '/handler/to/path', handler) - - def test_add_route_root(self): - handler = self.make_handler() - self.router.add_route('GET', '/', handler) - 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) - - def test_add_route_simple(self): - handler = self.make_handler() - self.router.add_route('GET', '/handler/to/path', handler) - 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) - - def test_add_with_matchdict(self): - handler = self.make_handler() - self.router.add_route('GET', '/handler/{to}', handler) - 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) - - def test_add_route_with_add_get_shortcut(self): - handler = self.make_handler() - self.router.add_get('/handler/to/path', handler) - 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) - - def test_add_route_with_add_post_shortcut(self): - handler = self.make_handler() - self.router.add_post('/handler/to/path', handler) - req = self.make_request('POST', '/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) - - def test_add_route_with_add_put_shortcut(self): - handler = self.make_handler() - self.router.add_put('/handler/to/path', handler) - req = self.make_request('PUT', '/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) - - def test_add_route_with_add_patch_shortcut(self): - handler = self.make_handler() - self.router.add_patch('/handler/to/path', handler) - req = self.make_request('PATCH', '/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) - - def test_add_route_with_add_delete_shortcut(self): - handler = self.make_handler() - self.router.add_delete('/handler/to/path', handler) - req = self.make_request('DELETE', '/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) - - def test_add_with_name(self): - handler = self.make_handler() - self.router.add_route('GET', '/handler/to/path', handler, - name='name') - req = self.make_request('GET', '/handler/to/path') - info = self.loop.run_until_complete(self.router.resolve(req)) - self.assertIsNotNone(info) - self.assertEqual('name', info.route.name) - - def test_add_with_tailing_slash(self): - handler = self.make_handler() - self.router.add_route('GET', '/handler/to/path/', handler) - 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) - - def test_add_invalid_path(self): - handler = self.make_handler() - with self.assertRaises(ValueError): - self.router.add_route('GET', '/{/', handler) - - def test_add_url_invalid1(self): - handler = self.make_handler() - with self.assertRaises(ValueError): - self.router.add_route('post', '/post/{id', handler) - - def test_add_url_invalid2(self): - handler = self.make_handler() - with self.assertRaises(ValueError): - self.router.add_route('post', '/post/{id{}}', handler) - - def test_add_url_invalid3(self): - handler = self.make_handler() - with self.assertRaises(ValueError): - self.router.add_route('post', '/post/{id{}', handler) - - def test_add_url_invalid4(self): - handler = self.make_handler() - with self.assertRaises(ValueError): - self.router.add_route('post', '/post/{id"}', handler) - - def test_add_url_escaping(self): - handler = self.make_handler() - self.router.add_route('GET', '/+$', handler) - - req = self.make_request('GET', '/+$') - info = self.loop.run_until_complete(self.router.resolve(req)) - self.assertIsNotNone(info) - self.assertIs(handler, info.handler) - - def test_any_method(self): - handler = self.make_handler() - route = self.router.add_route(hdrs.METH_ANY, '/', handler) - - req = self.make_request('GET', '/') - info1 = self.loop.run_until_complete(self.router.resolve(req)) - self.assertIsNotNone(info1) - self.assertIs(route, info1.route) - - req = self.make_request('POST', '/') - info2 = self.loop.run_until_complete(self.router.resolve(req)) - self.assertIsNotNone(info2) - - self.assertIs(info1.route, info2.route) - - 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) - 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) - - 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) - req = self.make_request('PUT', '/') - - match_info = self.loop.run_until_complete(self.router.resolve(req)) - self.assertIsInstance(match_info.route, SystemRoute) - self.assertEqual({}, match_info) - - with self.assertRaises(HTTPMethodNotAllowed) as ctx: - self.loop.run_until_complete(match_info.handler(req)) - - exc = ctx.exception - self.assertEqual('PUT', exc.method) - self.assertEqual(405, exc.status) - self.assertEqual({'POST', 'GET'}, exc.allowed_methods) - - def test_raise_method_not_found(self): - handler = self.make_handler() - self.router.add_route('GET', '/a', handler) - req = self.make_request('GET', '/b') - - match_info = self.loop.run_until_complete(self.router.resolve(req)) - self.assertIsInstance(match_info.route, SystemRoute) - self.assertEqual({}, match_info) - - with self.assertRaises(HTTPNotFound) as ctx: - self.loop.run_until_complete(match_info.handler(req)) - - 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') - - regexp = ("Duplicate 'name', already handled by") - with self.assertRaisesRegex(ValueError, regexp): - self.router.add_route('GET', '/get_other', handler2, name='name') - - def test_route_plain(self): - handler = self.make_handler() - route = self.router.add_route('GET', '/get', handler, name='name') - route2 = next(iter(self.router['name'])) - url = route2.url() - self.assertEqual('/get', url) - self.assertIs(route, route2) - - def test_route_unknown_route_name(self): - with self.assertRaises(KeyError): - self.router['unknown'] - - def test_route_dynamic(self): - handler = self.make_handler() - route = self.router.add_route('GET', '/get/{name}', handler, - name='name') - - route2 = next(iter(self.router['name'])) - url = route2.url(parts={'name': 'John'}) - self.assertEqual('/get/John', url) - self.assertIs(route, route2) - - def test_route_with_qs(self): - handler = self.make_handler() - self.router.add_route('GET', '/get', handler, name='name') - - url = self.router['name'].url(query=[('a', 'b'), ('c', '1')]) - self.assertEqual('/get?a=b&c=1', url) - - def test_add_static(self): - resource = self.router.add_static('/st', - os.path.dirname(aiohttp.__file__), - name='static') - assert self.router['static'] is resource - url = resource.url(filename='/dir/a.txt') - self.assertEqual('/st/dir/a.txt', url) - assert len(resource) == 2 - - def test_plain_not_match(self): - handler = self.make_handler() - self.router.add_route('GET', '/get/path', handler, name='name') - route = self.router['name'] - self.assertIsNone(route._match('/another/path')) - - def test_dynamic_not_match(self): - handler = self.make_handler() - self.router.add_route('GET', '/get/{name}', handler, name='name') - route = self.router['name'] - self.assertIsNone(route._match('/another/path')) - - def test_static_not_match(self): - self.router.add_static('/pre', os.path.dirname(aiohttp.__file__), - name='name') - resource = self.router['name'] - self.assertEqual((None, set()), - self.loop.run_until_complete( - resource.resolve('GET', '/another/path'))) - - def test_dynamic_with_trailing_slash(self): - handler = self.make_handler() - self.router.add_route('GET', '/get/{name}/', handler, name='name') - route = self.router['name'] - self.assertEqual({'name': 'John'}, route._match('/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.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.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.assertIn('name1', self.router) - self.assertNotIn('name3', self.router) - - def test_static_repr(self): - self.router.add_static('/get', os.path.dirname(aiohttp.__file__), - name='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') - - 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') - - 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) - - url = route.url(parts={'one': 1, 'two': 2}) - self.assertEqual('/1/2', url) - - def test_regular_match_info(self): - - @asyncio.coroutine - def go(): - handler = self.make_handler() - self.router.add_route('GET', '/get/{name}', handler) - - req = self.make_request('GET', '/get/john') - match_info = yield from self.router.resolve(req) - self.assertEqual({'name': 'john'}, match_info) - self.maxDiff = None - self.assertRegex(repr(match_info), - ">") - - self.loop.run_until_complete(go()) - - def test_not_found_repr(self): - - @asyncio.coroutine - def go(): - req = self.make_request('POST', '/path/to') - match_info = yield from self.router.resolve(req) - self.assertEqual("", - repr(match_info)) - - self.loop.run_until_complete(go()) - - def test_not_allowed_repr(self): - - @asyncio.coroutine - def go(): - handler = self.make_handler() - self.router.add_route('GET', '/path/to', handler) - - handler2 = self.make_handler() - self.router.add_route('POST', '/path/to', handler2) - - req = self.make_request('PUT', '/path/to') - match_info = yield from self.router.resolve(req) - self.assertEqual("", - repr(match_info)) - - 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) - - def test_custom_expect_handler_plain(self): - - @asyncio.coroutine - def handler(request): - pass - - route = self.router.add_route( - 'GET', '/', self.make_handler(), expect_handler=handler) - self.assertIs(route._expect_handler, handler) - self.assertIsInstance(route, ResourceRoute) - - def test_custom_expect_handler_dynamic(self): - - @asyncio.coroutine - def handler(request): - pass - - route = self.router.add_route( - 'GET', '/get/{name}', self.make_handler(), expect_handler=handler) - self.assertIs(route._expect_handler, handler) - self.assertIsInstance(route, ResourceRoute) - - def test_expect_handler_non_coroutine(self): - - def handler(request): - pass - - self.assertRaises( - AssertionError, self.router.add_route, - 'GET', '/', self.make_handler(), 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) - req = self.make_request( - 'GET', - '/%D1%80%D1%83%D1%81%20%D1%82%D0%B5%D0%BA%D1%81%D1%82') - match_info = yield from self.router.resolve(req) - self.assertEqual({'var': 'рус текст'}, match_info) - - self.loop.run_until_complete(go()) - - def test_dynamic_match_with_static_part(self): - - @asyncio.coroutine - def go(): - handler = self.make_handler() - self.router.add_route('GET', '/{name}.html', handler) - req = self.make_request('GET', '/file.html') - match_info = yield from self.router.resolve(req) - self.assertEqual({'name': 'file'}, match_info) - - self.loop.run_until_complete(go()) - - def test_dynamic_match_two_part2(self): - - @asyncio.coroutine - def go(): - handler = self.make_handler() - self.router.add_route('GET', '/{name}.{ext}', handler) - req = self.make_request('GET', '/file.html') - match_info = yield from self.router.resolve(req) - self.assertEqual({'name': 'file', 'ext': 'html'}, match_info) - - self.loop.run_until_complete(go()) - - def test_dynamic_match_unquoted_path(self): - - @asyncio.coroutine - def go(): - handler = self.make_handler() - self.router.add_route('GET', '/{path}/{subpath}', handler) - 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) - self.assertEqual(match_info, { - 'path': 'path', - 'subpath': unquote(resource_id) - }) - - self.loop.run_until_complete(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): - - sample_bad_methods = { - 'BAD METHOD', - 'B@D_METHOD', - '[BAD_METHOD]', - '{BAD_METHOD}', - '(BAD_METHOD)', - 'B?D_METHOD', - } - - for bad_method in sample_bad_methods: - with self.assertRaises(ValueError): - handler = self.make_handler() - self.router.add_route(bad_method, '/path', handler) - - def fill_routes(self): - route1 = self.router.add_route('GET', '/plain', self.make_handler()) - route2 = self.router.add_route('GET', '/variable/{name}', - self.make_handler()) - resource = self.router.add_static('/static', - os.path.dirname(aiohttp.__file__)) +def make_request(method, path): + return make_mocked_request(method, path) + + +def make_handler(): + + @asyncio.coroutine + def handler(request): + return Response(request) # pragma: no cover + + return handler + + +@pytest.fixture +def router(): + return UrlDispatcher() + + +@pytest.fixture +def fill_routes(router): + def go(): + route1 = router.add_route('GET', '/plain', make_handler()) + route2 = router.add_route('GET', '/variable/{name}', + make_handler()) + resource = router.add_static('/static', + os.path.dirname(aiohttp.__file__)) return [route1, route2] + list(resource) + return go + + +def test_register_uncommon_http_methods(router): + uncommon_http_methods = { + 'PROPFIND', + 'PROPPATCH', + 'COPY', + 'LOCK', + 'UNLOCK' + 'MOVE', + 'SUBSCRIBE', + 'UNSUBSCRIBE', + 'NOTIFY' + } + + for method in uncommon_http_methods: + router.add_route(method, '/handler/to/path', make_handler()) + + +@asyncio.coroutine +def test_add_route_root(router): + handler = make_handler() + router.add_route('GET', '/', handler) + req = make_request('GET', '/') + info = yield from router.resolve(req) + assert info is not None + assert 0 == len(info) + assert handler is info.handler + assert info.route.name is None + + +@asyncio.coroutine +def test_add_route_simple(router): + handler = make_handler() + router.add_route('GET', '/handler/to/path', handler) + req = make_request('GET', '/handler/to/path') + info = yield from router.resolve(req) + assert info is not None + assert 0 == len(info) + assert handler is info.handler + assert info.route.name is None + + +@asyncio.coroutine +def test_add_with_matchdict(router): + handler = make_handler() + router.add_route('GET', '/handler/{to}', handler) + req = make_request('GET', '/handler/tail') + info = yield from router.resolve(req) + assert info is not None + assert {'to': 'tail'} == info + assert handler is info.handler + assert info.route.name is None + + +@asyncio.coroutine +def test_add_route_with_add_get_shortcut(router): + handler = make_handler() + router.add_get('/handler/to/path', handler) + req = make_request('GET', '/handler/to/path') + info = yield from router.resolve(req) + assert info is not None + assert 0 == len(info) + assert handler is info.handler + assert info.route.name is None + + +@asyncio.coroutine +def test_add_route_with_add_post_shortcut(router): + handler = make_handler() + router.add_post('/handler/to/path', handler) + req = make_request('POST', '/handler/to/path') + info = yield from router.resolve(req) + assert info is not None + assert 0 == len(info) + assert handler is info.handler + assert info.route.name is None + + +@asyncio.coroutine +def test_add_route_with_add_put_shortcut(router): + handler = make_handler() + router.add_put('/handler/to/path', handler) + req = make_request('PUT', '/handler/to/path') + info = yield from router.resolve(req) + assert info is not None + assert 0 == len(info) + assert handler is info.handler + assert info.route.name is None + + +@asyncio.coroutine +def test_add_route_with_add_patch_shortcut(router): + handler = make_handler() + router.add_patch('/handler/to/path', handler) + req = make_request('PATCH', '/handler/to/path') + info = yield from router.resolve(req) + assert info is not None + assert 0 == len(info) + assert handler is info.handler + assert info.route.name is None + + +@asyncio.coroutine +def test_add_route_with_add_delete_shortcut(router): + handler = make_handler() + router.add_delete('/handler/to/path', handler) + req = make_request('DELETE', '/handler/to/path') + info = yield from router.resolve(req) + assert info is not None + assert 0 == len(info) + assert handler is info.handler + assert info.route.name is None + + +@asyncio.coroutine +def test_add_route_with_add_head_shortcut(router): + handler = make_handler() + router.add_head('/handler/to/path', handler) + req = make_request('HEAD', '/handler/to/path') + info = yield from router.resolve(req) + assert info is not None + assert 0 == len(info) + assert handler is info.handler + assert info.route.name is None + + +@asyncio.coroutine +def test_add_with_name(router): + handler = make_handler() + router.add_route('GET', '/handler/to/path', handler, + name='name') + req = make_request('GET', '/handler/to/path') + info = yield from router.resolve(req) + assert info is not None + assert 'name' == info.route.name + + +@asyncio.coroutine +def test_add_with_tailing_slash(router): + handler = make_handler() + router.add_route('GET', '/handler/to/path/', handler) + req = make_request('GET', '/handler/to/path/') + info = yield from router.resolve(req) + assert info is not None + assert {} == info + assert handler is info.handler + + +def test_add_invalid_path(router): + handler = make_handler() + with pytest.raises(ValueError): + router.add_route('GET', '/{/', handler) + + +def test_add_url_invalid1(router): + handler = make_handler() + with pytest.raises(ValueError): + router.add_route('post', '/post/{id', handler) + + +def test_add_url_invalid2(router): + handler = make_handler() + with pytest.raises(ValueError): + router.add_route('post', '/post/{id{}}', handler) + + +def test_add_url_invalid3(router): + handler = make_handler() + with pytest.raises(ValueError): + router.add_route('post', '/post/{id{}', handler) + + +def test_add_url_invalid4(router): + handler = make_handler() + with pytest.raises(ValueError): + router.add_route('post', '/post/{id"}', handler) + + +@asyncio.coroutine +def test_add_url_escaping(router): + handler = make_handler() + router.add_route('GET', '/+$', handler) + + req = make_request('GET', '/+$') + info = yield from router.resolve(req) + assert info is not None + assert handler is info.handler + + +@asyncio.coroutine +def test_any_method(router): + handler = make_handler() + route = router.add_route(hdrs.METH_ANY, '/', handler) + + req = make_request('GET', '/') + info1 = yield from router.resolve(req) + assert info1 is not None + assert route is info1.route + + req = make_request('POST', '/') + info2 = yield from router.resolve(req) + assert info2 is not None + + assert info1.route is info2.route + + +@asyncio.coroutine +def test_match_second_result_in_table(router): + handler1 = make_handler() + handler2 = make_handler() + router.add_route('GET', '/h1', handler1) + router.add_route('POST', '/h2', handler2) + req = make_request('POST', '/h2') + info = yield from router.resolve(req) + assert info is not None + assert {} == info + assert handler2 is info.handler + + +@asyncio.coroutine +def test_raise_method_not_allowed(router): + handler1 = make_handler() + handler2 = make_handler() + router.add_route('GET', '/', handler1) + router.add_route('POST', '/', handler2) + req = make_request('PUT', '/') + + match_info = yield from router.resolve(req) + assert isinstance(match_info.route, SystemRoute) + assert {} == match_info + + with pytest.raises(HTTPMethodNotAllowed) as ctx: + yield from match_info.handler(req) + + exc = ctx.value + assert 'PUT' == exc.method + assert 405 == exc.status + assert {'POST', 'GET'} == exc.allowed_methods + + +@asyncio.coroutine +def test_raise_method_not_found(router): + handler = make_handler() + router.add_route('GET', '/a', handler) + req = make_request('GET', '/b') + + match_info = yield from router.resolve(req) + assert isinstance(match_info.route, SystemRoute) + assert {} == match_info + + with pytest.raises(HTTPNotFound) as ctx: + yield from match_info.handler(req) + + exc = ctx.value + assert 404 == exc.status + + +def test_double_add_url_with_the_same_name(router): + handler1 = make_handler() + handler2 = make_handler() + router.add_route('GET', '/get', handler1, name='name') + + regexp = ("Duplicate 'name', already handled by") + with pytest.raises(ValueError) as ctx: + router.add_route('GET', '/get_other', handler2, name='name') + assert re.match(regexp, str(ctx.value)) + + +def test_route_plain(router): + handler = make_handler() + route = router.add_route('GET', '/get', handler, name='name') + route2 = next(iter(router['name'])) + url = route2.url() + assert '/get' == url + assert route is route2 + + +def test_route_unknown_route_name(router): + with pytest.raises(KeyError): + router['unknown'] + + +def test_route_dynamic(router): + handler = make_handler() + route = router.add_route('GET', '/get/{name}', handler, + name='name') + + route2 = next(iter(router['name'])) + url = route2.url(parts={'name': 'John'}) + assert '/get/John' == url + assert route is route2 + + +def test_route_with_qs(router): + handler = make_handler() + router.add_route('GET', '/get', handler, name='name') + + url = router['name'].url(query=[('a', 'b'), ('c', '1')]) + assert '/get?a=b&c=1' == url + + +def test_add_static(router): + resource = router.add_static('/st', + os.path.dirname(aiohttp.__file__), + name='static') + assert router['static'] is resource + url = resource.url(filename='/dir/a.txt') + assert '/st/dir/a.txt' == url + assert len(resource) == 2 + + +def test_plain_not_match(router): + handler = make_handler() + router.add_route('GET', '/get/path', handler, name='name') + route = router['name'] + assert route._match('/another/path') is None + + +def test_dynamic_not_match(router): + handler = make_handler() + router.add_route('GET', '/get/{name}', handler, name='name') + route = router['name'] + assert route._match('/another/path') is None + + +@asyncio.coroutine +def test_static_not_match(router): + router.add_static('/pre', os.path.dirname(aiohttp.__file__), + name='name') + resource = router['name'] + ret = yield from resource.resolve('GET', '/another/path') + assert (None, set()) == ret + + +def test_dynamic_with_trailing_slash(router): + handler = make_handler() + router.add_route('GET', '/get/{name}/', handler, name='name') + route = router['name'] + assert {'name': 'John'} == route._match('/get/John/') + + +def test_len(router): + handler = make_handler() + router.add_route('GET', '/get1', handler, name='name1') + router.add_route('GET', '/get2', handler, name='name2') + assert 2 == len(router) + + +def test_iter(router): + handler = make_handler() + router.add_route('GET', '/get1', handler, name='name1') + router.add_route('GET', '/get2', handler, name='name2') + assert {'name1', 'name2'} == set(iter(router)) + + +def test_contains(router): + handler = make_handler() + router.add_route('GET', '/get1', handler, name='name1') + router.add_route('GET', '/get2', handler, name='name2') + assert 'name1' in router + assert 'name3' not in router + + +def test_static_repr(router): + router.add_static('/get', os.path.dirname(aiohttp.__file__), + name='name') + assert re.match(r"+++)': nothing to repeat") + assert ctx.value.__cause__ is None + + +def test_route_dynamic_with_regex_spec(router): + handler = make_handler() + route = router.add_route('GET', '/get/{num:^\d+}', handler, + name='name') + + url = route.url(parts={'num': '123'}) + assert '/get/123' == url + + +def test_route_dynamic_with_regex_spec_and_trailing_slash(router): + handler = make_handler() + route = router.add_route('GET', '/get/{num:^\d+}/', handler, + name='name') + + url = route.url(parts={'num': '123'}) + assert '/get/123/' == url - def test_routes_view_len(self): - self.fill_routes() - self.assertEqual(4, len(self.router.routes())) - def test_routes_view_iter(self): - routes = self.fill_routes() - self.assertEqual(list(routes), list(self.router.routes())) +def test_route_dynamic_with_regex(router): + handler = make_handler() + route = router.add_route('GET', r'/{one}/{two:.+}', handler) - def test_routes_view_contains(self): - routes = self.fill_routes() - for route in routes: - self.assertIn(route, self.router.routes()) + url = route.url(parts={'one': 1, 'two': 2}) + assert '/1/2' == url - def test_routes_abc(self): - self.assertIsInstance(self.router.routes(), Sized) - self.assertIsInstance(self.router.routes(), Iterable) - self.assertIsInstance(self.router.routes(), Container) - def fill_named_resources(self): - route1 = self.router.add_route('GET', '/plain', self.make_handler(), - name='route1') - route2 = self.router.add_route('GET', '/variable/{name}', - self.make_handler(), name='route2') - route3 = self.router.add_static('/static', - os.path.dirname(aiohttp.__file__), - name='route3') - return route1.name, route2.name, route3.name +@asyncio.coroutine +def test_regular_match_info(router): + handler = make_handler() + router.add_route('GET', '/get/{name}', handler) - def test_named_resources_abc(self): - self.assertIsInstance(self.router.named_resources(), Mapping) - self.assertNotIsInstance(self.router.named_resources(), MutableMapping) + req = make_request('GET', '/get/john') + match_info = yield from router.resolve(req) + assert {'name': 'john'} == match_info + assert re.match(">", + repr(match_info)) - def test_named_resources(self): - names = self.fill_named_resources() - self.assertEqual(3, len(self.router.named_resources())) +@asyncio.coroutine +def test_not_found_repr(router): + req = make_request('POST', '/path/to') + match_info = yield from router.resolve(req) + assert "" == repr(match_info) - for name in names: - self.assertIn(name, self.router.named_resources()) - self.assertIsInstance(self.router.named_resources()[name], - AbstractResource) - def test_resource_iter(self): - resource = self.router.add_resource('/path') - r1 = resource.add_route('GET', lambda req: None) - r2 = resource.add_route('POST', lambda req: None) - self.assertEqual(2, len(resource)) - self.assertEqual([r1, r2], list(resource)) +@asyncio.coroutine +def test_not_allowed_repr(router): + handler = make_handler() + router.add_route('GET', '/path/to', handler) - def test_deprecate_bare_generators(self): - resource = self.router.add_resource('/path') + handler2 = make_handler() + router.add_route('POST', '/path/to', handler2) - def gen(request): - yield + req = make_request('PUT', '/path/to') + match_info = yield from router.resolve(req) + assert "" == repr(match_info) - with self.assertWarns(DeprecationWarning): - resource.add_route('GET', gen) - def test_view_route(self): - resource = self.router.add_resource('/path') +def test_default_expect_handler(router): + route = router.add_route('GET', '/', make_handler()) + assert route._expect_handler is _defaultExpectHandler - route = resource.add_route('GET', View) - self.assertIs(View, route.handler) - def test_resource_route_match(self): - resource = self.router.add_resource('/path') - route = resource.add_route('GET', lambda req: None) - self.assertEqual({}, route.resource._match('/path')) +def test_custom_expect_handler_plain(router): - def test_error_on_double_route_adding(self): - resource = self.router.add_resource('/path') + @asyncio.coroutine + def handler(request): + pass + route = router.add_route( + 'GET', '/', make_handler(), expect_handler=handler) + assert route._expect_handler is handler + assert isinstance(route, ResourceRoute) + + +def test_custom_expect_handler_dynamic(router): + + @asyncio.coroutine + def handler(request): + pass + + route = router.add_route( + 'GET', '/get/{name}', make_handler(), expect_handler=handler) + assert route._expect_handler is handler + assert isinstance(route, ResourceRoute) + + +def test_expect_handler_non_coroutine(router): + + def handler(request): + pass + + with pytest.raises(AssertionError): + router.add_route('GET', '/', make_handler(), + expect_handler=handler) + + +@asyncio.coroutine +def test_dynamic_match_non_ascii(router): + handler = make_handler() + router.add_route('GET', '/{var}', handler) + req = make_request( + 'GET', + '/%D1%80%D1%83%D1%81%20%D1%82%D0%B5%D0%BA%D1%81%D1%82') + match_info = yield from router.resolve(req) + assert {'var': 'рус текст'} == match_info + + +@asyncio.coroutine +def test_dynamic_match_with_static_part(router): + handler = make_handler() + router.add_route('GET', '/{name}.html', handler) + req = make_request('GET', '/file.html') + match_info = yield from router.resolve(req) + assert {'name': 'file'} == match_info + + +@asyncio.coroutine +def test_dynamic_match_two_part2(router): + handler = make_handler() + router.add_route('GET', '/{name}.{ext}', handler) + req = make_request('GET', '/file.html') + match_info = yield from router.resolve(req) + assert {'name': 'file', 'ext': 'html'} == match_info + + +@asyncio.coroutine +def test_dynamic_match_unquoted_path(router): + handler = make_handler() + router.add_route('GET', '/{path}/{subpath}', handler) + resource_id = 'my%2Fpath%7Cwith%21some%25strange%24characters' + req = make_request('GET', '/path/{0}'.format(resource_id)) + match_info = yield from router.resolve(req) + assert match_info == { + 'path': 'path', + 'subpath': unquote(resource_id) + } + + +def test_add_route_not_started_with_slash(router): + with pytest.raises(ValueError): + handler = make_handler() + router.add_route('GET', 'invalid_path', handler) + + +def test_add_route_invalid_method(router): + + sample_bad_methods = { + 'BAD METHOD', + 'B@D_METHOD', + '[BAD_METHOD]', + '{BAD_METHOD}', + '(BAD_METHOD)', + 'B?D_METHOD', + } + + for bad_method in sample_bad_methods: + with pytest.raises(ValueError): + handler = make_handler() + router.add_route(bad_method, '/path', handler) + + +def test_routes_view_len(router, fill_routes): + fill_routes() + assert 4 == len(router.routes()) + + +def test_routes_view_iter(router, fill_routes): + routes = fill_routes() + assert list(routes) == list(router.routes()) + + +def test_routes_view_contains(router, fill_routes): + routes = fill_routes() + for route in routes: + assert route in router.routes() + + +def test_routes_abc(router): + assert isinstance(router.routes(), Sized) + assert isinstance(router.routes(), Iterable) + assert isinstance(router.routes(), Container) + + +def test_named_resources_abc(router): + assert isinstance(router.named_resources(), Mapping) + assert not isinstance(router.named_resources(), MutableMapping) + + +def test_named_resources(router): + route1 = router.add_route('GET', '/plain', make_handler(), + name='route1') + route2 = router.add_route('GET', '/variable/{name}', + make_handler(), name='route2') + route3 = router.add_static('/static', + os.path.dirname(aiohttp.__file__), + name='route3') + names = {route1.name, route2.name, route3.name} + + assert 3 == len(router.named_resources()) + + for name in names: + assert name in router.named_resources() + assert isinstance(router.named_resources()[name], + AbstractResource) + + +def test_resource_iter(router): + resource = router.add_resource('/path') + r1 = resource.add_route('GET', lambda req: None) + r2 = resource.add_route('POST', lambda req: None) + assert 2 == len(resource) + assert [r1, r2] == list(resource) + + +def test_deprecate_bare_generators(router): + resource = router.add_resource('/path') + + def gen(request): + yield + + with pytest.warns(DeprecationWarning): + resource.add_route('GET', gen) + + +def test_view_route(router): + resource = router.add_resource('/path') + + route = resource.add_route('GET', View) + assert View is route.handler + + +def test_resource_route_match(router): + resource = router.add_resource('/path') + route = resource.add_route('GET', lambda req: None) + assert {} == route.resource._match('/path') + + +def test_error_on_double_route_adding(router): + resource = router.add_resource('/path') + + resource.add_route('GET', lambda: None) + with pytest.raises(RuntimeError): resource.add_route('GET', lambda: None) - with self.assertRaises(RuntimeError): - resource.add_route('GET', lambda: None) - - def test_error_on_adding_route_after_wildcard(self): - resource = self.router.add_resource('/path') - - resource.add_route('*', lambda: None) - with self.assertRaises(RuntimeError): - resource.add_route('GET', lambda: None) - - def test_http_exception_is_none_when_resolved(self): - handler = self.make_handler() - self.router.add_route('GET', '/', handler) - req = self.make_request('GET', '/') - info = self.loop.run_until_complete(self.router.resolve(req)) - self.assertIsNone(info.http_exception) - - def test_http_exception_is_not_none_when_not_resolved(self): - handler = self.make_handler() - self.router.add_route('GET', '/', handler) - req = self.make_request('GET', '/abc') - info = self.loop.run_until_complete(self.router.resolve(req)) - self.assertEqual(info.http_exception.status, 404) - - def test_match_info_get_info_plain(self): - handler = self.make_handler() - self.router.add_route('GET', '/', handler) - req = self.make_request('GET', '/') - info = self.loop.run_until_complete(self.router.resolve(req)) - self.assertEqual(info.get_info(), {'path': '/'}) - - def test_match_info_get_info_dynamic(self): - handler = self.make_handler() - self.router.add_route('GET', '/{a}', handler) - req = self.make_request('GET', '/value') - info = self.loop.run_until_complete(self.router.resolve(req)) - self.assertEqual(info.get_info(), - {'pattern': re.compile('^\\/(?P[^{}/]+)$'), - 'formatter': '/{a}'}) - - def test_resource_adapter_get_info(self): - directory = pathlib.Path(aiohttp.__file__).parent - resource = self.router.add_static('/st', directory) - self.assertEqual(resource.get_info(), {'directory': directory, - 'prefix': '/st/'}) - - def test_system_route_get_info(self): - handler = self.make_handler() - self.router.add_route('GET', '/', handler) - req = self.make_request('GET', '/abc') - info = self.loop.run_until_complete(self.router.resolve(req)) - self.assertEqual(info.get_info()['http_exception'].status, 404) - - def fill_resources(self): - resource1 = self.router.add_resource('/plain') - resource2 = self.router.add_resource('/variable/{name}') - return resource1, resource2 - - def test_resources_view_len(self): - self.fill_resources() - self.assertEqual(2, len(self.router.resources())) - - def test_resources_view_iter(self): - resources = self.fill_resources() - self.assertEqual(list(resources), list(self.router.resources())) - - def test_resources_view_contains(self): - resources = self.fill_resources() - for resource in resources: - self.assertIn(resource, self.router.resources()) - - def test_resources_abc(self): - self.assertIsInstance(self.router.resources(), Sized) - self.assertIsInstance(self.router.resources(), Iterable) - self.assertIsInstance(self.router.resources(), Container) - - def test_static_route_user_home(self): - here = pathlib.Path(aiohttp.__file__).parent - home = pathlib.Path(os.path.expanduser('~')) - if not str(here).startswith(str(home)): # pragma: no cover - self.skipTest("aiohttp folder is not placed in user's HOME") - static_dir = '~/' + str(here.relative_to(home)) - route = self.router.add_static('/st', static_dir) - self.assertEqual(here, route.get_info()['directory']) - - def test_static_route_points_to_file(self): - here = pathlib.Path(aiohttp.__file__).parent / '__init__.py' - with self.assertRaises(ValueError): - self.router.add_static('/st', here) - - def test_404_for_static_resource(self): - resource = self.router.add_static('/st', - os.path.dirname(aiohttp.__file__)) - ret = self.loop.run_until_complete( - resource.resolve('GET', '/unknown/path')) - self.assertEqual((None, set()), ret) - - def test_405_for_resource_adapter(self): - resource = self.router.add_static('/st', - os.path.dirname(aiohttp.__file__)) - ret = self.loop.run_until_complete( - resource.resolve('POST', '/st/abc.py')) - self.assertEqual((None, {'HEAD', 'GET'}), ret) - - def test_check_allowed_method_for_found_resource(self): - handler = self.make_handler() - resource = self.router.add_resource('/') - resource.add_route('GET', handler) - ret = self.loop.run_until_complete(resource.resolve('GET', '/')) - self.assertIsNotNone(ret[0]) - self.assertEqual({'GET'}, ret[1]) + + +def test_error_on_adding_route_after_wildcard(router): + resource = router.add_resource('/path') + + resource.add_route('*', lambda: None) + with pytest.raises(RuntimeError): + resource.add_route('GET', lambda: None) + + +@asyncio.coroutine +def test_http_exception_is_none_when_resolved(router): + handler = make_handler() + router.add_route('GET', '/', handler) + req = make_request('GET', '/') + info = yield from router.resolve(req) + assert info.http_exception is None + + +@asyncio.coroutine +def test_http_exception_is_not_none_when_not_resolved(router): + handler = make_handler() + router.add_route('GET', '/', handler) + req = make_request('GET', '/abc') + info = yield from router.resolve(req) + assert info.http_exception.status == 404 + + +@asyncio.coroutine +def test_match_info_get_info_plain(router): + handler = make_handler() + router.add_route('GET', '/', handler) + req = make_request('GET', '/') + info = yield from router.resolve(req) + assert info.get_info() == {'path': '/'} + + +@asyncio.coroutine +def test_match_info_get_info_dynamic(router): + handler = make_handler() + router.add_route('GET', '/{a}', handler) + req = make_request('GET', '/value') + info = yield from router.resolve(req) + assert info.get_info() == { + 'pattern': re.compile('^\\/(?P[^{}/]+)$'), + 'formatter': '/{a}'} + + +def test_static_resource_get_info(router): + directory = pathlib.Path(aiohttp.__file__).parent + resource = router.add_static('/st', directory) + assert resource.get_info() == {'directory': directory, + 'prefix': '/st/'} + + +@asyncio.coroutine +def test_system_route_get_info(router): + handler = make_handler() + router.add_route('GET', '/', handler) + req = make_request('GET', '/abc') + info = yield from router.resolve(req) + assert info.get_info()['http_exception'].status == 404 + + +def test_resources_view_len(router): + router.add_resource('/plain') + router.add_resource('/variable/{name}') + assert 2 == len(router.resources()) + + +def test_resources_view_iter(router): + resource1 = router.add_resource('/plain') + resource2 = router.add_resource('/variable/{name}') + resources = [resource1, resource2] + assert list(resources) == list(router.resources()) + + +def test_resources_view_contains(router): + resource1 = router.add_resource('/plain') + resource2 = router.add_resource('/variable/{name}') + resources = [resource1, resource2] + for resource in resources: + assert resource in router.resources() + + +def test_resources_abc(router): + assert isinstance(router.resources(), Sized) + assert isinstance(router.resources(), Iterable) + assert isinstance(router.resources(), Container) + + +def test_static_route_user_home(router): + here = pathlib.Path(aiohttp.__file__).parent + home = pathlib.Path(os.path.expanduser('~')) + if not str(here).startswith(str(home)): # pragma: no cover + pytest.skip("aiohttp folder is not placed in user's HOME") + static_dir = '~/' + str(here.relative_to(home)) + route = router.add_static('/st', static_dir) + assert here == route.get_info()['directory'] + + +def test_static_route_points_to_file(router): + here = pathlib.Path(aiohttp.__file__).parent / '__init__.py' + with pytest.raises(ValueError): + router.add_static('/st', here) + + +@asyncio.coroutine +def test_404_for_static_resource(router): + resource = router.add_static('/st', + os.path.dirname(aiohttp.__file__)) + ret = yield from resource.resolve('GET', '/unknown/path') + assert (None, set()) == ret + + +@asyncio.coroutine +def test_405_for_resource_adapter(router): + resource = router.add_static('/st', + os.path.dirname(aiohttp.__file__)) + ret = yield from resource.resolve('POST', '/st/abc.py') + assert (None, {'HEAD', 'GET'}) == ret + + +@asyncio.coroutine +def test_check_allowed_method_for_found_resource(router): + handler = make_handler() + resource = router.add_resource('/') + resource.add_route('GET', handler) + ret = yield from resource.resolve('GET', '/') + assert ret[0] is not None + assert {'GET'} == ret[1] From cfa0ad95e165a6d9fe25925298f4371d76e7cb55 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Thu, 6 Oct 2016 15:18:02 +0300 Subject: [PATCH 10/12] Fix test coverage degradation --- tests/test_urldispatch.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/test_urldispatch.py b/tests/test_urldispatch.py index 1d67d4170bb..57ddc08965f 100644 --- a/tests/test_urldispatch.py +++ b/tests/test_urldispatch.py @@ -6,6 +6,7 @@ from urllib.parse import unquote import pytest +from yarl import URL import aiohttp.web from aiohttp import hdrs @@ -867,3 +868,22 @@ def test_check_allowed_method_for_found_resource(router): ret = yield from resource.resolve('GET', '/') assert ret[0] is not None assert {'GET'} == ret[1] + + +def test_url_for_in_static_resource(router): + resource = router.add_static('/static', + os.path.dirname(aiohttp.__file__)) + assert URL('/static/file.txt') == resource.url_for(filename='file.txt') + + +def test_url_for_in_static_resource_pathlib(router): + resource = router.add_static('/static', + os.path.dirname(aiohttp.__file__)) + assert URL('/static/file.txt') == resource.url_for( + filename=pathlib.Path('file.txt')) + + +def test_url_for_in_resource_route(router): + route = router.add_route('GET', '/get/{name}', make_handler(), + name='name') + assert URL('/get/John') == route.url_for(name='John') From 7ed8b47c5b73e6d33dcfc633ef4fd4804166f337 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Thu, 6 Oct 2016 18:28:33 +0300 Subject: [PATCH 11/12] Update doc --- docs/web.rst | 11 ++-- docs/web_reference.rst | 143 ++++++++++++++++++++++------------------- 2 files changed, 82 insertions(+), 72 deletions(-) diff --git a/docs/web.rst b/docs/web.rst index b68a542e516..230a441df87 100644 --- a/docs/web.rst +++ b/docs/web.rst @@ -172,8 +172,9 @@ Routes can also be given a *name*:: Which can then be used to access and build a *URL* for that resource later (e.g. in a :ref:`request handler `):: - >>> request.app.router.named_resources()['root'].url(query={"a": "b", "c": "d"}) - '/root?a=b&c=d' + >>> request.app.router.named_resources()['root'].url_for() + ... .with_query({"a": "b", "c": "d"}) + URL('/root?a=b&c=d') A more interesting example is building *URLs* for :ref:`variable resources `:: @@ -183,9 +184,8 @@ resources `:: In this case you can also pass in the *parts* of the route:: - >>> request.app.router['user-info'].url( - ... parts={'user': 'john_doe'}, - ... query="?a=b") + >>> request.app.router['user-info'].url_for(user='john_doe')\ + ... .with_query("a=b") '/john_doe/info?a=b' @@ -322,6 +322,7 @@ The following example shows custom routing based on the *HTTP Accept* header:: chooser.reg_acceptor('application/json', handle_json) chooser.reg_acceptor('application/xml', handle_xml) +.. _aiohttp-web-static-file-handling: Static file handling -------------------- diff --git a/docs/web_reference.rst b/docs/web_reference.rst index 129f1c06707..1dbe5e3e66c 100644 --- a/docs/web_reference.rst +++ b/docs/web_reference.rst @@ -1650,7 +1650,7 @@ Resource with a *name* is called *named resource*. The main purpose of *named resource* is constructing URL by route name for passing it into *template engine* for example:: - url = app.router['resource_name'].url(query={'a': 1, 'b': 2}) + url = app.router['resource_name'].url_for().with_query({'a': 1, 'b': 2}) Resource classes hierarchy:: @@ -1658,7 +1658,7 @@ Resource classes hierarchy:: Resource PlainResource DynamicResource - ResourceAdapter + StaticResource .. class:: AbstractResource @@ -1692,6 +1692,23 @@ Resource classes hierarchy:: request is resolved or ``None`` if no :term:`route` is found. + .. method:: get_info() + + A resource description, e.g. ``{'path': '/path/to'}`` or + ``{'formatter': '/path/{to}', 'pattern': + re.compile(r'^/path/(?P[a-zA-Z][_a-zA-Z0-9]+)$`` + + .. method:: url_for(*args, **kwargs) + + Construct an URL for route with additional params. + + *args* and **kwargs** depend on a parameters list accepted by + inherited resource class. + + :return: :class:`~yarl.URL` -- resulting URL instance. + + .. versionadded:: 1.1 + .. method:: url(**kwargs) Construct an URL for route with additional params. @@ -1699,7 +1716,11 @@ Resource classes hierarchy:: **kwargs** depends on a list accepted by inherited resource class parameters. - :return: :class:`str` -- resulting URL. + :return: :class:`str` -- resulting URL string. + + .. deprecated:: 1.1 + + Use :meth:`url_for` instead. .. class:: Resource @@ -1730,28 +1751,59 @@ Resource classes hierarchy:: .. class:: PlainResource - A new-style resource, inherited from :class:`Resource`. + A resource, inherited from :class:`Resource`. The class corresponds to resources with plain-text matching, ``'/path/to'`` for example. + .. method:: url_for() + + Returns a :class:`~yarl.URL` for the resource. + + .. versionadded:: 1.1 + + .. class:: DynamicResource - A new-style resource, inherited from :class:`Resource`. + A resource, inherited from :class:`Resource`. The class corresponds to resources with :ref:`variable ` matching, e.g. ``'/path/{to}/{param}'`` etc. -.. class:: ResourceAdapter + .. method:: url_for(**params) + + Returns a :class:`~yarl.URL` for the resource. + + :param params: -- a variable substitutions for dynamic resource. + + E.g. for ``'/path/{to}/{param}'`` pattern the method should + be called as ``resource.url_for(to='val1', param='val2')`` + + + .. versionadded:: 1.1 + +.. class:: StaticResource + + A resource, inherited from :class:`Resource`. - An adapter for old-style routes. + The class corresponds to resources for :ref:`static file serving + `. - The adapter is used by ``router.register_route()`` call, the method - is deprecated and will be removed eventually. + .. method:: url_for(filename) + Returns a :class:`~yarl.URL` for file path under resource prefix. + + :param filename: -- a file name substitution for static file handler. + + Accepts both :class:`str` and :class:`pathlib.Path`. + + E.g. an URL for ``'/prefix/dir/file.txt'`` should + be generated as ``resource.url_for(filename='dir/file.txt')`` + + .. versionadded:: 1.1 .. _aiohttp-web-route: @@ -1767,22 +1819,11 @@ Route classes hierarchy:: AbstractRoute ResourceRoute - Route - PlainRoute - DynamicRoute - StaticRoute - -:class:`ResourceRoute` is the route used for new-style resources, -:class:`PlainRoute` and :class:`DynamicRoute` serves old-style -routes kept for backward compatibility only. - -:class:`StaticRoute` is used for static file serving -(:meth:`UrlDispatcher.add_static`). Don't rely on the route -implementation too hard, static file handling most likely will be -rewritten eventually. + SystemRoute -So the only non-deprecated and not internal route is -:class:`ResourceRoute` only. +:class:`ResourceRoute` is the route used for resources, +:class:`SystemRoute` serves URL resolving errors like *404 Not Found* +and *405 Method Not Allowed*. .. class:: AbstractRoute @@ -1802,24 +1843,14 @@ So the only non-deprecated and not internal route is .. attribute:: resource - Resource instance which holds the route. + Resource instance which holds the route, ``None`` for + :class:`SystemRoute`. - .. method:: url(*, query=None, **kwargs) + .. method:: url_for(*args, **kwargs) Abstract method for constructing url handled by the route. - *query* is a mapping or list of *(name, value)* pairs for - specifying *query* part of url (parameter is processed by - :func:`~urllib.parse.urlencode`). - - Other available parameters depends on concrete route class and - described in descendant classes. - - - .. note:: - - The method is kept for sake of backward compatibility, usually - you should use :meth:`Resource.url` instead. + Actually it's a shortcut for ``route.resource.url_for(...)``. .. coroutinemethod:: handle_expect_header(request) @@ -1829,42 +1860,20 @@ So the only non-deprecated and not internal route is The route class for handling different HTTP methods for :class:`Resource`. -.. class:: PlainRoute - The route class for handling plain *URL path*, e.g. ``"/a/b/c"`` +.. class:: SystemRoute - .. method:: url(*, parts, query=None) + The route class for handling URL resolution errors like like *404 Not Found* + and *405 Method Not Allowed*. - Construct url, doesn't accepts extra parameters:: - - >>> route.url(query={'d': 1, 'e': 2}) - '/a/b/c/?d=1&e=2' - -.. class:: DynamicRoute - - The route class for handling :ref:`variable - path`, e.g. ``"/a/{name1}/{name2}"`` - - .. method:: url(*, parts, query=None) - - Construct url with given *dynamic parts*:: - - >>> route.url(parts={'name1': 'b', 'name2': 'c'}, - query={'d': 1, 'e': 2}) - '/a/b/c/?d=1&e=2' - - -.. class:: StaticRoute + .. attribute:: status - The route class for handling static files, created by - :meth:`UrlDispatcher.add_static` call. + HTTP status code - .. method:: url(*, filename, query=None) + .. attribute:: reason - Construct url for given *filename*:: + HTTP status reason - >>> route.url(filename='img/logo.png', query={'param': 1}) - '/path/to/static/img/logo.png?param=1' MatchInfo From 417e49165e50195f36b62a36133cfa6b77fb42a6 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Thu, 6 Oct 2016 18:42:38 +0300 Subject: [PATCH 12/12] Update CHANGES --- CHANGES.rst | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index e37fb0a6c4f..80a4b2be82d 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -23,7 +23,16 @@ CHANGES - Properly format IPv6 addresses by `aiohttp.web.run_app` #1139 -- +- Use `yarl.URL` by server API #1288 + + * Introduce `resource.url_for()`, deprecate `resource.url()`. + + * Implement `StaticResource`. + + * Inherit `SystemRoute` from `AbstractRoute` + + * Drop old-style routes: `Route`, `PlainRoute`, `DynamicRoute`, + `StaticRoute`, `ResourceAdapter`. -