diff --git a/CHANGES.txt b/CHANGES.txt index a7a6c9fab60..a95531dc571 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -42,6 +42,11 @@ CHANGES - Enable access log by default #735 +- Deprecate app.router.register_route() (the method was not documented + intentionally BTW). + +- Deprecate app.router.named_routes() in favor of app.router.named_resources() + - route.add_static accepts pathlib.Path now #743 - Add command line support: `$ python -m aiohttp.web package.main` #740 diff --git a/aiohttp/abc.py b/aiohttp/abc.py index 535b4720951..9f51f223e7b 100644 --- a/aiohttp/abc.py +++ b/aiohttp/abc.py @@ -23,8 +23,8 @@ def handler(self): @property # pragma: no branch @abstractmethod - def route(self): - """Return route for match info""" + def expect_handler(self): + """Expect handler for 100-continue processing""" class AbstractView(metaclass=ABCMeta): diff --git a/aiohttp/web.py b/aiohttp/web.py index 392b3a6c581..909f7483cd2 100644 --- a/aiohttp/web.py +++ b/aiohttp/web.py @@ -33,7 +33,6 @@ class RequestHandler(ServerHttpProtocol): _meth = 'none' _path = 'none' - _request = None def __init__(self, manager, app, router, *, secure_proxy_ssl_header=None, **kwargs): @@ -59,8 +58,6 @@ def connection_lost(self, exc): self._manager.connection_lost(self, exc) super().connection_lost(exc) - if self._request is not None: - pass @asyncio.coroutine def handle_request(self, message, payload): @@ -72,40 +69,35 @@ def handle_request(self, message, payload): app, message, payload, self.transport, self.reader, self.writer, secure_proxy_ssl_header=self._secure_proxy_ssl_header) - self._request = request self._meth = request.method self._path = request.path try: - try: - match_info = yield from self._router.resolve(request) - - assert isinstance(match_info, AbstractMatchInfo), match_info - - resp = None - request._match_info = match_info - expect = request.headers.get(hdrs.EXPECT) - if expect and expect.lower() == "100-continue": - resp = ( - yield from match_info.route.handle_expect_header( - request)) - - if resp is None: - handler = match_info.handler - for factory in reversed(self._middlewares): - handler = yield from factory(app, handler) - resp = yield from handler(request) - - assert isinstance(resp, StreamResponse), \ - ("Handler {!r} should return response instance, " - "got {!r} [middlewares {!r}]").format( - match_info.handler, type(resp), self._middlewares) - except HTTPException as exc: - resp = exc - - resp_msg = yield from resp.prepare(request) - yield from resp.write_eof() - finally: - self._request = None + match_info = yield from self._router.resolve(request) + + assert isinstance(match_info, AbstractMatchInfo), match_info + + resp = None + request._match_info = match_info + expect = request.headers.get(hdrs.EXPECT) + if expect and expect.lower() == "100-continue": + resp = ( + yield from match_info.expect_handler(request)) + + if resp is None: + handler = match_info.handler + for factory in reversed(self._middlewares): + handler = yield from factory(app, handler) + resp = yield from handler(request) + + assert isinstance(resp, StreamResponse), \ + ("Handler {!r} should return response instance, " + "got {!r} [middlewares {!r}]").format( + match_info.handler, type(resp), self._middlewares) + except HTTPException as exc: + resp = exc + + resp_msg = yield from resp.prepare(request) + yield from resp.write_eof() # notify server about keep-alive self.keep_alive(resp_msg.keep_alive()) diff --git a/aiohttp/web_urldispatcher.py b/aiohttp/web_urldispatcher.py index eae4f1a5497..73d775fe616 100644 --- a/aiohttp/web_urldispatcher.py +++ b/aiohttp/web_urldispatcher.py @@ -8,6 +8,7 @@ import os import sys import inspect +import warnings from collections.abc import Sized, Iterable, Container from pathlib import Path @@ -23,6 +24,7 @@ __all__ = ('UrlDispatcher', 'UrlMappingMatchInfo', + 'Resource', 'PlainResource', 'DynamicResource', 'Route', 'PlainRoute', 'DynamicRoute', 'StaticRoute', 'View') @@ -43,6 +45,10 @@ def handler(self): def route(self): return self._route + @property + def expect_handler(self): + return self._route.handle_expect_header + def __repr__(self): return "".format(super().__repr__(), self._route) @@ -54,18 +60,200 @@ def _defaultExpectHandler(request): request.transport.write(b"HTTP/1.1 100 Continue\r\n\r\n") -class Route(metaclass=abc.ABCMeta): +class AbstractResource(Sized, Iterable): + + def __init__(self, *, name=None): + self._name = name + + @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 the path.""" + + @abc.abstractmethod # pragma: no branch + def url(self, **kwargs): + """Construct url for resource with additional params.""" + + @abc.abstractmethod # pragma: no branch + def resolve(self, method, path): + """Resolve resource + + Return (UrlMappingMatchInfo, allowed_methods) pair.""" + + @staticmethod + def _append_query(url, query): + if query is not None: + return url + "?" + urlencode(query) + else: + return url + + +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 match(self, path): + return self._route.match(path) + + def url(self, **kwargs): + return self._route.url(**kwargs) + + def resolve(self, method, path): + route_method = self._route.method + allowed_methods = {route_method} + if route_method == method or route_method == hdrs.METH_ANY: + match_dict = self._route.match(path) + if match_dict is not None: + return (UrlMappingMatchInfo(match_dict, self._route), + allowed_methods) + return None, allowed_methods + + def __len__(self): + return 1 + + def __iter__(self): + yield self._route + + +class Resource(AbstractResource): + + def __init__(self, *, name=None): + super().__init__(name=name) + self._routes = [] + + def add_route(self, method, handler, *, + expect_handler=None): + + for route in self._routes: + if route.method == method or route.method == hdrs.METH_ANY: + raise RuntimeError("Added route will never be executed, " + "method {route.method} is " + "already registered".format(route=route)) + + route = ResourceRoute(method, handler, self, + expect_handler=expect_handler) + self.register_route(route) + return route + + def register_route(self, route): + assert isinstance(route, ResourceRoute), \ + 'Instance of Route class is required, got {!r}'.format(route) + self._routes.append(route) + + def resolve(self, method, path): + allowed_methods = set() + + match_dict = self.match(path) + if match_dict is None: + return None, allowed_methods + + for route in self._routes: + route_method = route.method + + if route_method == method or route_method == hdrs.METH_ANY: + return UrlMappingMatchInfo(match_dict, route), allowed_methods + + allowed_methods.add(route_method) + else: + return None, allowed_methods + + def __len__(self): + return len(self._routes) + + def __iter__(self): + return iter(self._routes) + + +class PlainResource(Resource): + + def __init__(self, path, *, name=None): + super().__init__(name=name) + 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 __repr__(self): + name = "'" + self.name + "' " if self.name is not None else "" + return " {handler!r}".format( + method=self.method, resource=self._resource, + handler=self.handler) + + @property + def name(self): + return self._resource.name + + def match(self, path): + """Return dict with info for given path or + None if route cannot process path.""" + return self._resource.match(path) + + def url(self, **kwargs): + """Construct url for route with additional params.""" + return self._resource.url(**kwargs) + + +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 class PlainRoute(Route): @@ -362,6 +584,10 @@ def handler(self): def _not_found(self, request): raise HTTPNotFound() + @property + def expect_handler(self): + return self.route.handle_expect_header + def __repr__(self): return "" @@ -383,6 +609,10 @@ def handler(self): def _not_allowed(self, request): raise HTTPMethodNotAllowed(self._method, self._allowed_methods) + @property + def expect_handler(self): + return self.route.handle_expect_header + def __repr__(self): return ("" .format(self._method, @@ -412,19 +642,20 @@ def _raise_allowed_methods(self): class RoutesView(Sized, Iterable, Container): - __slots__ = '_urls' - - def __init__(self, urls): - self._urls = urls + def __init__(self, resources): + self._routes = [] + for resource in resources: + for route in resource: + self._routes.append(route) def __len__(self): - return len(self._urls) + return len(self._routes) def __iter__(self): - yield from self._urls + yield from self._routes def __contains__(self, route): - return route in self._urls + return route in self._routes class UrlDispatcher(AbstractRouter, collections.abc.Mapping): @@ -436,14 +667,10 @@ class UrlDispatcher(AbstractRouter, collections.abc.Mapping): ROUTE_RE = re.compile(r'(\{[_a-zA-Z][^{}]*(?:\{[^{}]*\}[^{}]*)*\})') NAME_SPLIT_RE = re.compile('[.:-]') - 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} - def __init__(self): super().__init__() - self._urls = [] - self._routes = {} + self._resources = [] + self._named_resources = {} @asyncio.coroutine def resolve(self, request): @@ -451,19 +678,12 @@ def resolve(self, request): method = request.method allowed_methods = set() - for route in self._urls: - match_dict = route.match(path) - if match_dict 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) + for resource in self._resources: + match_dict, allowed = resource.resolve(method, path) + if match_dict is not None: + return match_dict + else: + allowed_methods |= allowed else: if allowed_methods: return _MethodNotAllowedMatchInfo(method, allowed_methods) @@ -471,27 +691,39 @@ def resolve(self, request): return _NotFoundMatchInfo() def __iter__(self): - return iter(self._routes) + return iter(self._named_resources) def __len__(self): - return len(self._routes) + return len(self._named_resources) def __contains__(self, name): - return name in self._routes + return name in self._named_resources def __getitem__(self, name): - return self._routes[name] + return self._named_resources[name] def routes(self): - return RoutesView(self._urls) + return RoutesView(self._resources) + + def named_resources(self): + return MappingProxyType(self._named_resources) def named_routes(self): - return MappingProxyType(self._routes) + # NB: it's ambiguous but it's really resources. + warnings.warn("Use .named_resources instead", DeprecationWarning) + return self.named_resources() def register_route(self, route): - assert isinstance(route, Route), 'Instance of Route class is required.' + warnings.warn("Use resource-based interface", DeprecationWarning) + resource = ResourceAdapter(route) + self._reg_resource(resource) - name = route.name + def _reg_resource(self, resource): + assert isinstance(resource, AbstractResource), \ + 'Instance of AbstractResource class is required, got {!r}'.format( + resource) + + name = resource.name if name is not None: parts = self.NAME_SPLIT_RE.split(name) @@ -501,38 +733,20 @@ def register_route(self, route): 'the name should be a sequence of ' 'python identifiers separated ' 'by dash, dot or column'.format(name)) - if name in self._routes: + if name in self._named_resources: raise ValueError('Duplicate {!r}, ' 'already handled by {!r}' - .format(name, self._routes[name])) - self._routes[name] = route - self._urls.append(route) - - def add_route(self, method, path, handler, - *, name=None, expect_handler=None): + .format(name, self._named_resources[name])) + self._named_resources[name] = resource + self._resources.append(resource) + def add_resource(self, path, *, name=None): if not path.startswith('/'): raise ValueError("path should be started with /") - - assert callable(handler), handler - if asyncio.iscoroutinefunction(handler): - pass - elif inspect.isgeneratorfunction(handler): - pass - elif isinstance(handler, type) and issubclass(handler, AbstractView): - pass - else: - handler = asyncio.coroutine(handler) - - method = upstr(method) - if method not in self.METHODS: - raise ValueError("{} is not allowed HTTP method".format(method)) - if not ('{' in path or '}' in path or self.ROUTE_RE.search(path)): - route = PlainRoute( - method, handler, name, path, expect_handler=expect_handler) - self.register_route(route) - return route + resource = PlainResource(path, name=name) + self._reg_resource(resource) + return resource pattern = '' formatter = '' @@ -560,11 +774,15 @@ def add_route(self, method, path, handler, except re.error as exc: raise ValueError( "Bad pattern '{}': {}".format(pattern, exc)) from None - route = DynamicRoute( - method, handler, name, compiled, - formatter, expect_handler=expect_handler) - self.register_route(route) - return route + resource = DynamicResource(compiled, formatter, name=name) + self._reg_resource(resource) + return resource + + def add_route(self, method, path, handler, + *, name=None, expect_handler=None): + resource = self.add_resource(path, name=name) + return resource.add_route(method, handler, + expect_handler=expect_handler) def add_static(self, prefix, path, *, name=None, expect_handler=None, chunk_size=256*1024, response_factory=StreamResponse): diff --git a/docs/glossary.rst b/docs/glossary.rst index 9c2e4600716..c49c523bea1 100644 --- a/docs/glossary.rst +++ b/docs/glossary.rst @@ -47,6 +47,19 @@ It makes communication faster by getting rid of connection establishment for every request. + resource + + A concept reflects the HTTP **path**, every resource corresponsd + to *URI*. + + May have an unique name. + + Contains :term:`route`\'s for different HTTP methods. + + route + + A part of :term:`resource`, resource's *path* coupled with HTTP method. + web-handler An endpoint that returns HTTP response. diff --git a/docs/new_router.rst b/docs/new_router.rst new file mode 100644 index 00000000000..d7101abc9de --- /dev/null +++ b/docs/new_router.rst @@ -0,0 +1,84 @@ +============================= +Router refactoring in 0.21 +============================= + + +Rationale +--------- + +First generation (v1) of router mapped ``(method, path)`` pair to +:term:`web-handler`. Mapping is named **route**. Routes used to have +unique names if any. + +The main mistake with the design is coupling the **route** to +``(method, path)`` pair while really URL construction operates with +**resources** (**location** is a synonym). HTTP method is not part of URI +but applyed on sending HTTP request only. + +Having different **route names** for the same path is confusing. Moreover +**named routes** constructed for the same path should have unique +nonoverlapping names which is cumbersome is certain situations. + +From other side sometimes it's desirable to bind several HTTP methods +to the same web handler. For *v1* router it can be solved by passing '*' +as HTTP method. Class based views require '*' method also usually. + + +Implementation +-------------- + +The change introduces **resource** as first class citizen:: + + resource = router.add_resource('/path/{to}', name='name') + +*Resource* has a **path** (dynamic or constant) and optional **name**. + +The name is **unique** in router context. + +*Resource* has **routes**. + +*Route* correspons to *HTTP method* and :term:`web-handler` for the method:: + + route = resource.add_route('GET', handler) + +User still may use wildcard for accepting all HTTP methods (maybe we +will add something like ``resource.add_wildcard(handler)`` later). + +Since **names** belongs to **resources** now ``app.router['name']`` +returns a **resource** instance instead of :class:`aiohttp.web.Route`. + +**resource** has ``.url()`` method, so +``app.router['name'].url(parts={'a': 'b'}, query={'arg': 'param'})`` +still works as usual. + + +The change allows to rewrite static file handling and implement nested +applications as well. + +Decoupling of *HTTP location* and *HTTP method* makes life easier. + +Backward compatibility +----------------------- + +The refactoring is 99% compatible with previous implementation. + +99% means all example and the most of current code works without +modifications but we have subtle API backward incompatibles. + +``app.router['name']`` returns a :class:`aiohttp.web.BaseResource` +instance instead of :class:`aiohttp.web.Route` but resource has the +same ``resource.url(...)`` most useful method, so end user should feel no +difference. + +``resource.match(...)`` is supported as well (while we believe it's +not used widely). + +``app.router.add_route(method, path, handler, name='name')`` now is just +shortcut for:: + + resource = app.router.add_resource(path, name=name) + route = resource.add_route(methoc, handler) + return route + +``app.router.register_route(...)`` is still supported, it creates +:class:`aiohttp.web.ResourceAdapter` for every call (but it's deprecated now). diff --git a/docs/web_reference.rst b/docs/web_reference.rst index 47b85838afa..827fbc2cea9 100644 --- a/docs/web_reference.rst +++ b/docs/web_reference.rst @@ -1174,6 +1174,21 @@ Router is any object that implements :class:`AbstractRouter` interface. .. seealso:: :ref:`Route classes ` + .. method:: add_resource(path, *, name=None) + + Append a :term:`resource` to the end of route table. + + *path* may be either *constant* string like ``'/a/b/c'`` or + *variable rule* like ``'/a/{var}'`` (see + :ref:`handling variable pathes`) + + :param str path: resource path spec. + + :param str name: optional resource name. + + :return: created resource instance (:class:`PlainResource` or + :class:`DynamicResource`). + .. method:: add_route(method, path, handler, *, \ name=None, expect_handler=None) @@ -1307,33 +1322,175 @@ Router is any object that implements :class:`AbstractRouter` interface. .. versionadded:: 0.18 - .. method:: named_routes() + .. method:: named_resources() Returns a :obj:`dict`-like :class:`types.MappingProxyType` *view* over - *all* named routes. + *all* named **resources**. + + The view maps every named resources's **name** to the + :class:`BaseResource` instance. It supports the usual + :obj:`dict`-like operations, except for any mutable operations + (i.e. it's **read-only**):: - The view maps every named route's :attr:`Route.name` attribute to the - :class:`Route`. It supports the usual :obj:`dict`-like operations, except - for any mutable operations (i.e. it's **read-only**):: + len(app.router.named_resources()) - len(app.router.named_routes()) + for name, resource in app.router.named_resources().items(): + print(name, resource) - for name, route in app.router.named_routes().items(): - print(name, route) + "name" in app.router.named_resources() - "route_name" in app.router.named_routes() + app.router.named_resources()["name"] - app.router.named_routes()["route_name"] + .. method:: named_routes() + + An alias for :meth:`named_resources` starting from aiohttp 0.21. .. versionadded:: 0.19 + .. versionchanged:: 0.21 + + The method is an alias for :meth:`named_resources`, so it + iterates over resources instead of routes. + + .. deprecated:: 0.21 + + Please use named **resources** instead of named **routes**. + + Several routes which belongs to the same resource shares the + resource name. + + +.. _aiohttp-web-resource: + +Resource +^^^^^^^^ + +Default router :class:`UrlDispatcher` operates with :term:`resource`\s. + +Resource is an item in *routing table* which has a *path*, an optional +unique *name* and at least one :term:`route`. + +:term:`web-handler` lookup is performed in the following way: + +1. Router iterates over *resources* one-by-one. +2. If *resource* matches to requested URL the resource iterates over + own *routes*. +3. If route matches to requested HTTP method (or ``'*'`` wildcard) the + route's handler is used as found :term:`web-handler`. The lookup is + finished. +4. Otherwise router tries next resource from the *routing table*. +5. If the end of *routing table* is reached and no *resource* / + *route* pair found the *router* returns special :class:`SystemRoute` + instance with either *HTTP 404 Not Found* or *HTTP 405 + Method Not Allowed* status code. Registerd :term:`web-handler` for + *system route* raises corresponding :ref:`web exception + `. + +User should never instantiate resource classes but give it by +:meth:`UrlDispatcher.add_resource` call. + +After that he may add a :term:`route` by calling :meth:`Resource.add_route`. + +:meth:`UrlDispatcher.add_route` is just shortcut for:: + + router.add_resource(path).add_route(method, handler) + +Resource classes hierarhy:: + + AbstractResource + Resource + PlainResource + DynamicResource + ResourceAdapter + + +.. class:: AbstractResource + + A base class for all resources. + + Inherited from :class:`collections.abc.Sized` and + :class:`collections.abc.Iterable`. + + .. attribute:: name + + Read-only *name* of resource or ``None``. + + .. method:: match(path) + + :param str path: *path* part of requested URL. + + :return: :class:`dict` with info for given *path* or + ``None`` if route cannot process the path. + + .. method:: resolve(method, path) + + Resolve resource by finding appropriate :term:`web-handler` for + ``(method, path)`` combination. + + Calls :meth:`match` internally. + + :param str method: requested HTTP method. + + :return: (*match_info*, *allowed_methods*) pair. + + *allowed_methods* is a :class:`set` or HTTP methods accepted by + resource. + + *match_info* is either :class:`UrlMappingMatchInfo` if + request is resolved or ``None`` if no :term:`route` is + found. + + .. method:: url(**kwargs) + + Construct an URL for route with additional params. + + **kwargs** depends on a list accepted by inherited resource + class parameters. + + :return: :class:`str` -- resulting URL. + + +.. class:: Resource + + A base class for new-style resources, inherits :class:`AbstractResource`. + + +.. class:: PlainResource + + A new-style resource, inherited from :class:`Resource`. + + The class corresponds to resources with plain-text matching, + ``'/path/to'`` for example. + + +.. class:: DynamicResource + + A new-style resource, inherited from :class:`Resource`. + + The class corresponds to resources with + :ref:`variable ` matching, + e.g. ``'/path/{to}/{param}'`` etc. + + +.. class:: ResourceAdapter + + An adapter for old-style routes. + + The adapter is used by ``router.register_route()`` call, the method + is deprecated and will be removed eventually. + .. _aiohttp-web-route: Route ^^^^^ -Default router :class:`UrlDispatcher` operates with *routes*. +Route has HTTP method (wildcard ``'*'`` is an option), +:term:`web-handler` and optional *expect handler*. + +Every route belong to some resource. + + User should not instantiate route classes by hand but can give *named route instance* by ``router[name]`` if he have added route by diff --git a/tests/test_urldispatch.py b/tests/test_urldispatch.py index 860d693551a..061a36f8c8c 100644 --- a/tests/test_urldispatch.py +++ b/tests/test_urldispatch.py @@ -1,5 +1,6 @@ import asyncio import os +import re import unittest from collections.abc import Sized, Container, Iterable, Mapping, MutableMapping from unittest import mock @@ -13,7 +14,10 @@ from aiohttp.web_urldispatcher import (_defaultExpectHandler, DynamicRoute, PlainRoute, - SystemRoute) + SystemRoute, + ResourceRoute, + AbstractResource, + View) class TestUrlDispatcher(unittest.TestCase): @@ -241,7 +245,7 @@ def test_double_add_url_with_the_same_name(self): def test_route_plain(self): handler = self.make_handler() route = self.router.add_route('GET', '/get', handler, name='name') - route2 = self.router['name'] + route2 = next(iter(self.router['name'])) url = route2.url() self.assertEqual('/get', url) self.assertIs(route, route2) @@ -255,7 +259,7 @@ def test_route_dynamic(self): route = self.router.add_route('GET', '/get/{name}', handler, name='name') - route2 = self.router['name'] + route2 = next(iter(self.router['name'])) url = route2.url(parts={'name': 'John'}) self.assertEqual('/get/John', url) self.assertIs(route, route2) @@ -271,10 +275,10 @@ def test_add_static(self): route = self.router.add_static('/st', os.path.dirname(aiohttp.__file__), name='static') - route2 = self.router['static'] - url = route2.url(filename='/dir/a.txt') + resource = self.router['static'] + url = resource.url(filename='/dir/a.txt') self.assertEqual('/st/dir/a.txt', url) - self.assertIs(route, route2) + self.assertIs(route, next(iter(resource))) def test_plain_not_match(self): handler = self.make_handler() @@ -321,20 +325,21 @@ def test_contains(self): def test_plain_repr(self): handler = self.make_handler() - self.router.add_route('GET', '/get/path', handler, name='name') - self.assertRegex(repr(self.router['name']), + route = PlainRoute('GET', handler, 'name', '/get/path') + self.assertRegex(repr(route), r">") + ">") self.loop.run_until_complete(go()) @@ -481,7 +487,7 @@ def handler(request): route = self.router.add_route( 'GET', '/', self.make_handler(), expect_handler=handler) self.assertIs(route._expect_handler, handler) - self.assertIsInstance(route, PlainRoute) + self.assertIsInstance(route, ResourceRoute) def test_custom_expect_handler_dynamic(self): @@ -492,7 +498,7 @@ def handler(request): route = self.router.add_route( 'GET', '/get/{name}', self.make_handler(), expect_handler=handler) self.assertIs(route._expect_handler, handler) - self.assertIsInstance(route, DynamicRoute) + self.assertIsInstance(route, ResourceRoute) def test_expect_handler_non_coroutine(self): @@ -643,7 +649,7 @@ def test_routes_abc(self): self.assertIsInstance(self.router.routes(), Iterable) self.assertIsInstance(self.router.routes(), Container) - def fill_named_routes(self): + 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}', @@ -651,17 +657,134 @@ def fill_named_routes(self): route3 = self.router.add_static('/static', os.path.dirname(aiohttp.__file__), name='route3') - return route1, route2, 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) + def test_named_routes(self): - named_routes = self.fill_named_routes() + self.fill_named_resources() + + with self.assertWarns(DeprecationWarning): + self.assertEqual(3, len(self.router.named_routes())) + + def test_named_resources(self): + names = self.fill_named_resources() + + self.assertEqual(3, len(self.router.named_resources())) + + for name in names: + self.assertIn(name, self.router.named_routes()) + self.assertIsInstance(self.router.named_routes()[name], + AbstractResource) + + def test_resource_adapter_not_match(self): + route = PlainRoute('GET', lambda req: None, None, '/path') + self.router.register_route(route) + resource = route.resource + self.assertIsNotNone(resource) + self.assertIsNone(resource.match('/another/path')) + + def test_resource_adapter_resolve_not_math(self): + route = PlainRoute('GET', lambda req: None, None, '/path') + self.router.register_route(route) + resource = route.resource + self.assertEqual((None, {'GET'}), + resource.resolve('GET', '/another/path')) + + def test_resource_adapter_resolve_bad_method(self): + route = PlainRoute('POST', lambda req: None, None, '/path') + self.router.register_route(route) + resource = route.resource + self.assertEqual((None, {'POST'}), + resource.resolve('GET', '/path')) + + def test_resource_adapter_resolve_wildcard(self): + route = PlainRoute('*', lambda req: None, None, '/path') + self.router.register_route(route) + resource = route.resource + match_info, allowed = resource.resolve('GET', '/path') + self.assertEqual(allowed, {'*'}) # TODO: expand wildcard + self.assertIsNotNone(match_info) + + def test_resource_adapter_iter(self): + route = PlainRoute('GET', lambda req: None, None, '/path') + self.router.register_route(route) + resource = route.resource + self.assertEqual(1, len(resource)) + self.assertEqual([route], list(resource)) + + 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)) - self.assertEqual(3, len(self.router.named_routes())) + def test_deprecate_bare_generators(self): + resource = self.router.add_resource('/path') - for route in named_routes: - self.assertIn(route.name, self.router.named_routes()) - self.assertEqual(route, self.router.named_routes()[route.name]) + def gen(request): + yield + + with self.assertWarns(DeprecationWarning): + resource.add_route('GET', gen) + + def test_view_route(self): + resource = self.router.add_resource('/path') + + 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.match('/path')) + + def test_plain_route_url(self): + route = PlainRoute('GET', lambda req: None, None, '/path') + self.router.register_route(route) + self.assertEqual('/path?arg=1', route.url(query={'arg': 1})) + + def test_dynamic_route_url(self): + route = DynamicRoute('GET', lambda req: None, None, + '', '/{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') + + 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)