Merge pull request #1510 from KeepSafe/refactor-subapps

remove Application dependency from UrlDispatcher

Author: Nikolay Kim

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)
 ------------------
 

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

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
 from .web_ws import *  # noqa
 
 __all__ = (web_reqrep.__all__ +
@@ -39,7 +40,8 @@ def __init__(self, *, logger=web_logger, loop=None,
             router = web_urldispatcher.UrlDispatcher()
         assert isinstance(router, AbstractRouter), router
 
-        router.post_init(self)
+        # backward compatibility until full deprecation
+        router.add_subapp = _wrap_add_subbapp(self)
 
         if debug is ...:
             debug = loop.get_debug()
@@ -125,6 +127,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.router.reg_resource(resource)
+        self._reg_subapp_signals(subapp)
+        subapp.freeze()
+        return resource
+
     @property
     def on_response_prepare(self):
         return self._on_response_prepare
@@ -264,6 +283,16 @@ def __repr__(self):
         return "<Application 0x{:x}>".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,

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,19 +883,6 @@ def add_delete(self, *args, **kwargs):
         """
         return self.add_route(hdrs.METH_DELETE, *args, **kwargs)
 
-    def add_subapp(self, prefix, subapp):
-        if subapp.frozen:
-            raise RuntimeError("Cannod 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)
-        subapp.freeze()
-        return resource
-
     def freeze(self):
         super().freeze()
         for resource in self._resources:

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

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)
 
 
@@ -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(
         '<PrefixedSubAppResource /pre -> <Application')
 
 
-def test_subapp_len(router, loop):
+def test_subapp_len(app, loop):
     subapp = web.Application(loop=loop)
     subapp.router.add_get('/', make_handler())
     subapp.router.add_post('/', make_handler())
-    resource = router.add_subapp('/pre', subapp)
+    resource = app.add_subapp('/pre', subapp)
     assert len(resource) == 2
 
 
-def test_subapp_iter(router, loop):
+def test_subapp_iter(app, loop):
     subapp = web.Application(loop=loop)
     r1 = subapp.router.add_get('/', make_handler())
     r2 = subapp.router.add_post('/', make_handler())
-    resource = router.add_subapp('/pre', subapp)
+    resource = app.add_subapp('/pre', subapp)
     assert list(resource) == [r1, r2]
 
 
@@ -961,11 +967,18 @@ def test_frozen_router(router):
         router.add_get('/', make_handler())
 
 
-def test_frozen_router_subapp(loop, router):
+def test_frozen_router_subapp(app, loop):
     subapp = web.Application(loop=loop)
     subapp.freeze()
     with pytest.raises(RuntimeError):
-        router.add_subapp('/', subapp)
+        app.add_subapp('/', subapp)
+
+
+def test_frozen_app_on_subapp(app, loop):
+    app.freeze()
+    subapp = web.Application(loop=loop)
+    with pytest.raises(RuntimeError):
+        app.add_subapp('/', subapp)
 
 
 def test_set_options_route(router):
@@ -991,16 +1004,16 @@ def test_dynamic_url_with_name_started_from_undescore(router):
     assert URL('/get/John') == route.url_for(_name='John')
 
 
-def test_cannot_add_subapp_with_empty_prefix(router, loop):
+def test_cannot_add_subapp_with_empty_prefix(app, loop):
     subapp = web.Application(loop=loop)
     with pytest.raises(ValueError):
-        router.add_subapp('', subapp)
+        app.add_subapp('', subapp)
 
 
-def test_cannot_add_subapp_with_slash_prefix(router, loop):
+def test_cannot_add_subapp_with_slash_prefix(app, loop):
     subapp = web.Application(loop=loop)
     with pytest.raises(ValueError):
-        router.add_subapp('/', subapp)
+        app.add_subapp('/', subapp)
 
 
 @asyncio.coroutine
@@ -1011,9 +1024,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())