From cff60ebb65e35a28b481969a8e91b72e95a9f649 Mon Sep 17 00:00:00 2001 From: Alexey Popravka Date: Tue, 28 Nov 2017 00:11:08 +0200 Subject: [PATCH] Root app middleware receives wrong app context (#2550) * add failing test showing invalid request.app in middleware * implement UrlMappingMatchInfo.current_app attribute with context manger setter * fix bad inlined-inlined-loop) * Add changes --- CHANGES/2550.bugfix | 1 + aiohttp/web.py | 8 ++++++-- aiohttp/web_middlewares.py | 13 ++++++++++-- aiohttp/web_request.py | 4 ++-- aiohttp/web_urldispatcher.py | 20 +++++++++++++++++++ tests/test_web_functional.py | 38 ++++++++++++++++++++++++++++++++++++ 6 files changed, 78 insertions(+), 6 deletions(-) create mode 100644 CHANGES/2550.bugfix diff --git a/CHANGES/2550.bugfix b/CHANGES/2550.bugfix new file mode 100644 index 00000000000..de39a572ee6 --- /dev/null +++ b/CHANGES/2550.bugfix @@ -0,0 +1 @@ +Make `request.app` point to proper application instance when using nested applications (with middlewares). diff --git a/aiohttp/web.py b/aiohttp/web.py index 28c0cf82510..14affe0edca 100644 --- a/aiohttp/web.py +++ b/aiohttp/web.py @@ -24,6 +24,7 @@ from .web_exceptions import * # noqa from .web_fileresponse import * # noqa from .web_middlewares import * # noqa +from .web_middlewares import _fix_request_current_app from .web_protocol import * # noqa from .web_request import * # noqa from .web_response import * # noqa @@ -294,6 +295,8 @@ def _prepare_middleware(self): 'see #2252'.format(m), DeprecationWarning, stacklevel=2) yield m, False + if self._middlewares: + yield _fix_request_current_app(self), True @asyncio.coroutine def _handle(self, request): @@ -326,8 +329,9 @@ def _handle(self, request): ("Handler {!r} should return response instance, " "got {!r} [middlewares {!r}]").format( match_info.handler, type(resp), - [middleware for middleware in app.middlewares - for app in match_info.apps]) + [middleware + for app in match_info.apps + for middleware in app.middlewares]) return resp def __call__(self): diff --git a/aiohttp/web_middlewares.py b/aiohttp/web_middlewares.py index 3fc7a0437ad..efdb42f797d 100644 --- a/aiohttp/web_middlewares.py +++ b/aiohttp/web_middlewares.py @@ -54,7 +54,7 @@ def normalize_path_middleware( @asyncio.coroutine @middleware - def normalize_path_middleware(request, handler): + def impl(request, handler): if isinstance(request.match_info.route, SystemRoute): paths_to_check = [] if '?' in request.raw_path: @@ -81,4 +81,13 @@ def normalize_path_middleware(request, handler): return (yield from handler(request)) - return normalize_path_middleware + return impl + + +def _fix_request_current_app(app): + + @middleware + async def impl(request, handler): + with request.match_info.set_current_app(app): + return await handler(request) + return impl diff --git a/aiohttp/web_request.py b/aiohttp/web_request.py index 7d15f6f0d89..ea94c4ce7e5 100644 --- a/aiohttp/web_request.py +++ b/aiohttp/web_request.py @@ -623,10 +623,10 @@ def match_info(self): """Result of route resolving.""" return self._match_info - @reify + @property def app(self): """Application instance.""" - return self._match_info.apps[-1] + return self._match_info.current_app @asyncio.coroutine def _prepare_hook(self, response): diff --git a/aiohttp/web_urldispatcher.py b/aiohttp/web_urldispatcher.py index c10a2de61fe..96500e538d2 100644 --- a/aiohttp/web_urldispatcher.py +++ b/aiohttp/web_urldispatcher.py @@ -10,6 +10,7 @@ import warnings from collections import namedtuple from collections.abc import Container, Iterable, Sequence, Sized +from contextlib import contextmanager from functools import wraps from pathlib import Path from types import MappingProxyType @@ -195,6 +196,7 @@ def __init__(self, match_dict, route): super().__init__(match_dict) self._route = route self._apps = () + self._current_app = None self._frozen = False @property @@ -223,8 +225,26 @@ def apps(self): def add_app(self, app): if self._frozen: raise RuntimeError("Cannot change apps stack after .freeze() call") + if self._current_app is None: + self._current_app = app self._apps = (app,) + self._apps + @property + def current_app(self): + return self._current_app + + @contextmanager + def set_current_app(self, app): + assert app in self._apps, ( + "Expected one of the following apps {!r}, got {!r}" + .format(self._apps, app)) + prev = self._current_app + self._current_app = app + try: + yield + finally: + self._current_app = prev + def freeze(self): self._frozen = True diff --git a/tests/test_web_functional.py b/tests/test_web_functional.py index fece36eb6cd..526a2214720 100644 --- a/tests/test_web_functional.py +++ b/tests/test_web_functional.py @@ -1401,6 +1401,44 @@ def on_signal(app): assert [app, subapp1, subapp2] == order +@pytest.mark.parametrize('route,expected', [ + ('/sub/', ['app see root', 'subapp see sub']), + ('/', ['app see root']), +]) +@asyncio.coroutine +def test_subapp_middleware_context(loop, test_client, route, expected): + values = [] + + def show_app_context(appname): + @web.middleware + @asyncio.coroutine + def middleware(request, handler): + values.append('{} see {}'.format(appname, request.app['my_value'])) + return (yield from handler(request)) + return middleware + + @asyncio.coroutine + def handler(request): + return web.Response(text='Ok') + + app = web.Application() + app['my_value'] = 'root' + app.middlewares.append(show_app_context('app')) + app.router.add_get('/', handler) + + subapp = web.Application() + subapp['my_value'] = 'sub' + subapp.middlewares.append(show_app_context('subapp')) + subapp.router.add_get('/', handler) + app.add_subapp('/sub/', subapp) + + client = yield from test_client(app) + resp = yield from client.get(route) + assert 200 == resp.status + assert 'Ok' == (yield from resp.text()) + assert expected == values + + @asyncio.coroutine def test_custom_date_header(loop, test_client):