Skip to content

Commit

Permalink
Root app middleware receives wrong app context (#2550)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
popravich authored and asvetlov committed Nov 27, 2017
1 parent bbefc20 commit 0710570
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGES/2550.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Make `request.app` point to proper application instance when using nested applications (with middlewares).
8 changes: 6 additions & 2 deletions aiohttp/web.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,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
Expand Down Expand Up @@ -267,6 +268,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

async def _handle(self, request):
match_info = await self._router.resolve(request)
Expand Down Expand Up @@ -298,8 +301,9 @@ async 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):
Expand Down
9 changes: 9 additions & 0 deletions aiohttp/web_middlewares.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,3 +78,12 @@ async def impl(request, handler):
return await handler(request)

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
4 changes: 2 additions & 2 deletions aiohttp/web_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -609,10 +609,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

async def _prepare_hook(self, response):
match_info = self._match_info
Expand Down
20 changes: 20 additions & 0 deletions aiohttp/web_urldispatcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -170,6 +171,7 @@ def __init__(self, match_dict, route):
super().__init__(match_dict)
self._route = route
self._apps = ()
self._current_app = None
self._frozen = False

@property
Expand Down Expand Up @@ -198,8 +200,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

Expand Down
35 changes: 35 additions & 0 deletions tests/test_web_functional.py
Original file line number Diff line number Diff line change
Expand Up @@ -1290,6 +1290,41 @@ async def on_signal(app):
assert [app, subapp1, subapp2] == order


@pytest.mark.parametrize('route,expected', [
('/sub/', ['app see root', 'subapp see sub']),
('/', ['app see root']),
])
async def test_subapp_middleware_context(loop, test_client, route, expected):
values = []

def show_app_context(appname):
@web.middleware
async def middleware(request, handler):
values.append('{} see {}'.format(appname, request.app['my_value']))
return await handler(request)
return middleware

async 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 = await test_client(app)
resp = await client.get(route)
assert 200 == resp.status
assert 'Ok' == await resp.text()
assert expected == values


async def test_custom_date_header(loop, test_client):

async def handler(request):
Expand Down

0 comments on commit 0710570

Please sign in to comment.