From dc133040ff31b325549c010caa1f9f095ab826f2 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Sun, 25 Dec 2016 10:26:00 -0800 Subject: [PATCH 1/2] remove Application dependency from UrlDispatcher --- aiohttp/web.py | 20 +++++++++++++++++- aiohttp/web_urldispatcher.py | 41 +++++++++++++++++------------------- tests/test_urldispatch.py | 8 +------ 3 files changed, 39 insertions(+), 30 deletions(-) diff --git a/aiohttp/web.py b/aiohttp/web.py index d2457a626f1..0f95f7aee93 100644 --- a/aiohttp/web.py +++ b/aiohttp/web.py @@ -18,6 +18,7 @@ from .web_reqrep import * # noqa from .web_server import Server from .web_urldispatcher import * # noqa +from .web_urldispatcher import PrefixedSubAppResource, _wrap_add_subbapp from .web_ws import * # noqa __all__ = (web_reqrep.__all__ + @@ -39,7 +40,7 @@ def __init__(self, *, logger=web_logger, loop=None, router = web_urldispatcher.UrlDispatcher() assert isinstance(router, AbstractRouter), router - router.post_init(self) + router.add_subapp = _wrap_add_subbapp(self) if debug is ...: debug = loop.get_debug() @@ -125,6 +126,23 @@ def handler(app): reg_handler('on_shutdown') reg_handler('on_cleanup') + def add_subapp(self, prefix, subapp): + if self.frozen: + raise RuntimeError( + "Cannot add sub application to frozen application") + if subapp.frozen: + raise RuntimeError("Cannot add frozen application") + if prefix.endswith('/'): + prefix = prefix[:-1] + if prefix in ('', '/'): + raise ValueError("Prefix cannot be empty") + + resource = PrefixedSubAppResource(prefix, subapp) + self.reg_resource(resource) + subapp._reg_subapp_signals(subapp) + subapp.freeze() + return resource + @property def on_response_prepare(self): return self._on_response_prepare diff --git a/aiohttp/web_urldispatcher.py b/aiohttp/web_urldispatcher.py index 017452d0b6f..b0e443475c5 100644 --- a/aiohttp/web_urldispatcher.py +++ b/aiohttp/web_urldispatcher.py @@ -714,11 +714,6 @@ def __init__(self): super().__init__() self._resources = [] self._named_resources = {} - self._app = None - - def post_init(self, app): - assert app is not None - self._app = app @asyncio.coroutine def resolve(self, request): @@ -759,16 +754,13 @@ def routes(self): def named_resources(self): return MappingProxyType(self._named_resources) - def _reg_resource(self, resource): + def reg_resource(self, resource): assert isinstance(resource, AbstractResource), \ 'Instance of AbstractResource class is required, got {!r}'.format( resource) - if self._app is None: - raise RuntimeError(".post_init() should be called before " - "first resource registering") if self.frozen: - raise RuntimeError("Cannot register a resource into " - "frozen router.") + raise RuntimeError( + "Cannot register a resource into frozen router.") name = resource.name @@ -792,7 +784,7 @@ def add_resource(self, path, *, name=None): raise ValueError("path should be started with / or be empty") if not ('{' in path or '}' in path or self.ROUTE_RE.search(path)): resource = PlainResource(quote(path, safe='/'), name=name) - self._reg_resource(resource) + self.reg_resource(resource) return resource pattern = '' @@ -823,7 +815,7 @@ def add_resource(self, path, *, name=None): raise ValueError( "Bad pattern '{}': {}".format(pattern, exc)) from None resource = DynamicResource(compiled, formatter, name=name) - self._reg_resource(resource) + self.reg_resource(resource) return resource def add_route(self, method, path, handler, @@ -852,7 +844,7 @@ def add_static(self, prefix, path, *, name=None, expect_handler=None, response_factory=response_factory, show_index=show_index, follow_symlinks=follow_symlinks) - self._reg_resource(resource) + self.reg_resource(resource) return resource def add_head(self, *args, **kwargs): @@ -891,20 +883,25 @@ def add_delete(self, *args, **kwargs): """ return self.add_route(hdrs.METH_DELETE, *args, **kwargs) - def add_subapp(self, prefix, subapp): + def freeze(self): + super().freeze() + for resource in self._resources: + resource.freeze() + + +def _wrap_add_subbapp(app): + + def add_subapp(prefix, subapp): if subapp.frozen: - raise RuntimeError("Cannod add frozen application") + raise RuntimeError("Cannot add frozen application") if prefix.endswith('/'): prefix = prefix[:-1] if prefix in ('', '/'): raise ValueError("Prefix cannot be empty") resource = PrefixedSubAppResource(prefix, subapp) - self._reg_resource(resource) - self._app._reg_subapp_signals(subapp) + app.router.reg_resource(resource) + app._reg_subapp_signals(subapp) subapp.freeze() return resource - def freeze(self): - super().freeze() - for resource in self._resources: - resource.freeze() + return add_subapp diff --git a/tests/test_urldispatch.py b/tests/test_urldispatch.py index 996a8c29582..a3819d09232 100644 --- a/tests/test_urldispatch.py +++ b/tests/test_urldispatch.py @@ -13,7 +13,7 @@ from aiohttp.test_utils import make_mocked_request from aiohttp.web import HTTPMethodNotAllowed, HTTPNotFound, Response from aiohttp.web_urldispatcher import (AbstractResource, ResourceRoute, - SystemRoute, UrlDispatcher, View, + SystemRoute, View, _defaultExpectHandler) @@ -1011,9 +1011,3 @@ def test_convert_empty_path_to_slash_on_freezing(router): assert resource.get_info() == {'path': ''} router.freeze() assert resource.get_info() == {'path': '/'} - - -def test_add_to_non_initialized_router(): - router = UrlDispatcher() - with pytest.raises(RuntimeError): - router.add_get('/', make_handler()) From 12ce02b018247e7f466d4105e81411c4fe9f57b5 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Tue, 27 Dec 2016 09:13:01 -0800 Subject: [PATCH 2/2] use app.add_subapp instead of router.add_subapp --- CHANGES.rst | 3 ++- aiohttp/__init__.py | 2 +- aiohttp/web.py | 17 ++++++++++--- aiohttp/web_urldispatcher.py | 18 -------------- docs/web.rst | 8 +++--- tests/test_urldispatch.py | 47 +++++++++++++++++++++++------------- tests/test_web_functional.py | 38 ++++++++++++++--------------- 7 files changed, 70 insertions(+), 63 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 4f96f0101dd..1ade316ed2a 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -6,8 +6,9 @@ CHANGES - Fix polls demo run application #1487 -- +- remove `web.Application` dependency from `web.UrlDispatcher` #1510 + 1.2.0 (2016-12-17) ------------------ diff --git a/aiohttp/__init__.py b/aiohttp/__init__.py index 21265cb96bb..c1956b9a7ac 100644 --- a/aiohttp/__init__.py +++ b/aiohttp/__init__.py @@ -1,4 +1,4 @@ -__version__ = '1.2.0' +__version__ = '1.2.1a' # Deprecated, keep it here for a while for backward compatibility. import multidict # noqa diff --git a/aiohttp/web.py b/aiohttp/web.py index 0f95f7aee93..93bb16d2f5e 100644 --- a/aiohttp/web.py +++ b/aiohttp/web.py @@ -18,7 +18,7 @@ from .web_reqrep import * # noqa from .web_server import Server from .web_urldispatcher import * # noqa -from .web_urldispatcher import PrefixedSubAppResource, _wrap_add_subbapp +from .web_urldispatcher import PrefixedSubAppResource from .web_ws import * # noqa __all__ = (web_reqrep.__all__ + @@ -40,6 +40,7 @@ def __init__(self, *, logger=web_logger, loop=None, router = web_urldispatcher.UrlDispatcher() assert isinstance(router, AbstractRouter), router + # backward compatibility until full deprecation router.add_subapp = _wrap_add_subbapp(self) if debug is ...: @@ -138,8 +139,8 @@ def add_subapp(self, prefix, subapp): raise ValueError("Prefix cannot be empty") resource = PrefixedSubAppResource(prefix, subapp) - self.reg_resource(resource) - subapp._reg_subapp_signals(subapp) + self.router.reg_resource(resource) + self._reg_subapp_signals(subapp) subapp.freeze() return resource @@ -282,6 +283,16 @@ def __repr__(self): return "".format(id(self)) +def _wrap_add_subbapp(app): + # backward compatibility + + def add_subapp(prefix, subapp): + warnings.warn("Use app.add_subapp() instead", DeprecationWarning) + return app.add_subapp(prefix, subapp) + + return add_subapp + + def run_app(app, *, host='0.0.0.0', port=None, shutdown_timeout=60.0, ssl_context=None, print=print, backlog=128, access_log_format=None, diff --git a/aiohttp/web_urldispatcher.py b/aiohttp/web_urldispatcher.py index b0e443475c5..a54153b2a2d 100644 --- a/aiohttp/web_urldispatcher.py +++ b/aiohttp/web_urldispatcher.py @@ -887,21 +887,3 @@ def freeze(self): super().freeze() for resource in self._resources: resource.freeze() - - -def _wrap_add_subbapp(app): - - def add_subapp(prefix, subapp): - if subapp.frozen: - raise RuntimeError("Cannot add frozen application") - if prefix.endswith('/'): - prefix = prefix[:-1] - if prefix in ('', '/'): - raise ValueError("Prefix cannot be empty") - resource = PrefixedSubAppResource(prefix, subapp) - app.router.reg_resource(resource) - app._reg_subapp_signals(subapp) - subapp.freeze() - return resource - - return add_subapp diff --git a/docs/web.rst b/docs/web.rst index 0a1bfc7658d..a4da6ede7f8 100644 --- a/docs/web.rst +++ b/docs/web.rst @@ -971,12 +971,12 @@ toolbar URLs are served by prefix like ``/admin``. Thus we'll create a totally separate application named ``admin`` and connect it to main app with prefix by -:meth:`~aiohttp.web.UrlDispatcher.add_subapp`:: +:meth:`~aiohttp.web.Application.add_subapp`:: admin = web.Application() # setup admin routes, signals and middlewares - app.router.add_subapp('/admin/', admin) + app.add_subapp('/admin/', admin) Middlewares and signals from ``app`` and ``admin`` are chained. @@ -1006,7 +1006,7 @@ But for getting URL sub-application's router should be used:: admin = web.Application() admin.router.add_get('/resource', handler, name='name') - app.router.add_subapp('/admin/', admin) + app.add_subapp('/admin/', admin) url = admin.router['name'].url_for() @@ -1019,7 +1019,7 @@ use the following explicit technique:: admin = web.Application() admin.router.add_get('/resource', handler, name='name') - app.router.add_subapp('/admin/', admin) + app.add_subapp('/admin/', admin) app['admin'] = admin async def handler(request): # main application's handler diff --git a/tests/test_urldispatch.py b/tests/test_urldispatch.py index a3819d09232..db36d219e3c 100644 --- a/tests/test_urldispatch.py +++ b/tests/test_urldispatch.py @@ -907,46 +907,52 @@ def test_url_for_in_resource_route(router): assert URL('/get/John') == route.url_for(name='John') -def test_subapp_get_info(router, loop): +def test_subapp_get_info(app, loop): subapp = web.Application(loop=loop) - resource = router.add_subapp('/pre', subapp) + resource = subapp.add_subapp('/pre', subapp) assert resource.get_info() == {'prefix': '/pre', 'app': subapp} -def test_subapp_url(router, loop): +def test_subapp_backward_compatible(router, loop): subapp = web.Application(loop=loop) resource = router.add_subapp('/pre', subapp) + assert resource.get_info() == {'prefix': '/pre', 'app': subapp} + + +def test_subapp_url(app, loop): + subapp = web.Application(loop=loop) + resource = app.add_subapp('/pre', subapp) with pytest.raises(RuntimeError): resource.url() -def test_subapp_url_for(router, loop): +def test_subapp_url_for(app, loop): subapp = web.Application(loop=loop) - resource = router.add_subapp('/pre', subapp) + resource = app.add_subapp('/pre', subapp) with pytest.raises(RuntimeError): resource.url_for() -def test_subapp_repr(router, loop): +def test_subapp_repr(app, loop): subapp = web.Application(loop=loop) - resource = router.add_subapp('/pre', subapp) + resource = app.add_subapp('/pre', subapp) assert repr(resource).startswith( '