Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Root app middleware receives wrong app context #2550

Merged
merged 5 commits into from
Nov 27, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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