diff --git a/CHANGES/3569.feature b/CHANGES/3569.feature new file mode 100644 index 00000000000..041d5d4a2e0 --- /dev/null +++ b/CHANGES/3569.feature @@ -0,0 +1,2 @@ +Make new style middleware default, deprecate the @middleware decorator and +remove support for old-style middleware. diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index 3a60762f902..a7faded9fa7 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -37,6 +37,7 @@ Andrii Soldatenko Antoine Pietri Anton Kasyanov Anton Zhdan-Pushkin +Artem Yushkovskiy Arthur Darcet Ben Bader Benedikt Reinartz diff --git a/aiohttp/web_app.py b/aiohttp/web_app.py index b28a6087a4f..e0c1be25849 100644 --- a/aiohttp/web_app.py +++ b/aiohttp/web_app.py @@ -53,12 +53,9 @@ _RespPrepareSignal = Signal[Callable[[Request, StreamResponse], Awaitable[None]]] _Handler = Callable[[Request], Awaitable[StreamResponse]] - _Middleware = Union[Callable[[Request, _Handler], - Awaitable[StreamResponse]], - Callable[['Application', _Handler], # old-style - Awaitable[_Handler]]] + _Middleware = Callable[[Request, _Handler], Awaitable[StreamResponse]] _Middlewares = FrozenList[_Middleware] - _MiddlewaresHandlers = Optional[Sequence[Tuple[_Middleware, bool]]] + _MiddlewaresHandlers = Sequence[_Middleware] _Subapps = List['Application'] else: # No type checker mode, skip types @@ -67,7 +64,7 @@ _Handler = Callable _Middleware = Callable _Middlewares = FrozenList - _MiddlewaresHandlers = Optional[Sequence] + _MiddlewaresHandlers = Sequence _Subapps = List @@ -104,7 +101,7 @@ def __init__(self, *, self._middlewares = FrozenList(middlewares) # type: _Middlewares # initialized on freezing - self._middlewares_handlers = None # type: _MiddlewaresHandlers + self._middlewares_handlers = tuple() # type: _MiddlewaresHandlers # initialized on freezing self._run_middlewares = None # type: Optional[bool] @@ -390,17 +387,9 @@ def _make_request(self, message: RawRequestMessage, self._loop, client_max_size=self._client_max_size) - def _prepare_middleware(self) -> Iterator[Tuple[_Middleware, bool]]: - for m in reversed(self._middlewares): - if getattr(m, '__middleware_version__', None) == 1: - yield m, True - else: - warnings.warn('old-style middleware "{!r}" deprecated, ' - 'see #2252'.format(m), - DeprecationWarning, stacklevel=2) - yield m, False - - yield _fix_request_current_app(self), True + def _prepare_middleware(self) -> Iterator[_Middleware]: + yield from reversed(self._middlewares) + yield _fix_request_current_app(self) async def _handle(self, request: Request) -> StreamResponse: loop = asyncio.get_event_loop() @@ -426,11 +415,9 @@ async def _handle(self, request: Request) -> StreamResponse: if self._run_middlewares: for app in match_info.apps[::-1]: - for m, new_style in app._middlewares_handlers: # type: ignore # noqa - if new_style: - handler = partial(m, handler=handler) - else: - handler = await m(app, handler) # type: ignore + assert app.pre_frozen, "middleware handlers are not ready" + for m in app._middlewares_handlers: # noqa + handler = partial(m, handler=handler) resp = await handler(request) diff --git a/aiohttp/web_middlewares.py b/aiohttp/web_middlewares.py index 17064f6fb31..fff5b8ad7b4 100644 --- a/aiohttp/web_middlewares.py +++ b/aiohttp/web_middlewares.py @@ -1,4 +1,5 @@ import re +import warnings from typing import TYPE_CHECKING, Awaitable, Callable, Tuple, Type, TypeVar from .web_exceptions import HTTPMove, HTTPMovedPermanently @@ -31,7 +32,9 @@ async def _check_request_resolves(request: Request, def middleware(f: _Func) -> _Func: - f.__middleware_version__ = 1 # type: ignore + warnings.warn( + 'Middleware decorator is deprecated and its behaviour is default, ' + 'you can simply remove this decorator.', DeprecationWarning) return f @@ -76,7 +79,6 @@ def normalize_path_middleware( correct_configuration = not (append_slash and remove_slash) assert correct_configuration, "Cannot both remove and append slash" - @middleware async def impl(request: Request, handler: _Handler) -> StreamResponse: if isinstance(request.match_info.route, SystemRoute): paths_to_check = [] @@ -113,7 +115,6 @@ async def impl(request: Request, handler: _Handler) -> StreamResponse: def _fix_request_current_app(app: 'Application') -> _Middleware: - @middleware async def impl(request: Request, handler: _Handler) -> StreamResponse: with request.match_info.set_current_app(app): return await handler(request) diff --git a/docs/faq.rst b/docs/faq.rst index 7d1ef8ae54e..c33a9e85229 100644 --- a/docs/faq.rst +++ b/docs/faq.rst @@ -353,7 +353,6 @@ response:: # don't do this! cached = web.Response(status=200, text='Hi, I am cached!') - @web.middleware async def middleware(request, handler): # ignoring response for the sake of this example _res = handler(request) diff --git a/docs/web_advanced.rst b/docs/web_advanced.rst index 65cc7e69125..9910c51a214 100644 --- a/docs/web_advanced.rst +++ b/docs/web_advanced.rst @@ -483,12 +483,18 @@ response. For example, here's a simple *middleware* which appends from aiohttp.web import middleware - @middleware async def middleware(request, handler): resp = await handler(request) resp.text = resp.text + ' wink' return resp +.. warning:: + + As of version ``4.0.0`` "new-style" middleware is default and the + ``@middleware`` decorator is not required (and is deprecated), you can + simply remove the decorator. "Old-style" middleware (a coroutine which + returned a coroutine) is no longer supported. + .. note:: The example won't work with streamed responses or websockets @@ -531,14 +537,12 @@ The following code demonstrates middlewares execution order:: print('Handler function called') return web.Response(text="Hello") - @web.middleware async def middleware1(request, handler): print('Middleware 1 called') response = await handler(request) print('Middleware 1 finished') return response - @web.middleware async def middleware2(request, handler): print('Middleware 2 called') response = await handler(request) @@ -567,7 +571,6 @@ a JSON REST service:: from aiohttp import web - @web.middleware async def error_middleware(request, handler): try: response = await handler(request) @@ -586,17 +589,19 @@ a JSON REST service:: Middleware Factory ^^^^^^^^^^^^^^^^^^ -A *middleware factory* is a function that creates a middleware with passed arguments. For example, here's a trivial *middleware factory*:: +A *middleware factory* is a function that creates a middleware with passed +arguments. For example, here's a trivial *middleware factory*:: def middleware_factory(text): - @middleware async def sample_middleware(request, handler): resp = await handler(request) resp.text = resp.text + text return resp return sample_middleware -Remember that contrary to regular middlewares you need the result of a middleware factory not the function itself. So when passing a middleware factory to an app you actually need to call it:: +Note that in contrast to regular middlewares, a middleware factory should +return the function, not the value. So when passing a middleware factory +to the app you actually need to call it:: app = web.Application(middlewares=[middleware_factory(' wink')]) diff --git a/examples/web_rewrite_headers_middleware.py b/examples/web_rewrite_headers_middleware.py index 86f272fbb38..ad1efc9c816 100755 --- a/examples/web_rewrite_headers_middleware.py +++ b/examples/web_rewrite_headers_middleware.py @@ -10,7 +10,6 @@ async def handler(request): return web.Response(text="Everything is fine") -@web.middleware async def middleware(request, handler): try: response = await handler(request) diff --git a/tests/test_web_app.py b/tests/test_web_app.py index b58cbb3a085..4822512d8c2 100644 --- a/tests/test_web_app.py +++ b/tests/test_web_app.py @@ -246,7 +246,6 @@ def test_app_run_middlewares() -> None: root.freeze() assert root._run_middlewares is False - @web.middleware async def middleware(request, handler): return await handler(request) diff --git a/tests/test_web_functional.py b/tests/test_web_functional.py index d182b060d0b..1acde160453 100644 --- a/tests/test_web_functional.py +++ b/tests/test_web_functional.py @@ -1211,35 +1211,39 @@ async def handler(request): assert resp.status == 500 -async def test_subapp_middlewares(aiohttp_client) -> None: +async def test_old_style_subapp_middlewares(aiohttp_client) -> None: order = [] async def handler(request): return web.Response(text='OK') - async def middleware_factory(app, handler): - - async def middleware(request): - order.append((1, app)) + with pytest.warns( + DeprecationWarning, match='Middleware decorator is deprecated' + ): + @web.middleware + async def middleware(request, handler): + order.append((1, request.app['name'])) resp = await handler(request) assert 200 == resp.status - order.append((2, app)) + order.append((2, request.app['name'])) return resp - return middleware - app = web.Application(middlewares=[middleware_factory]) - subapp1 = web.Application(middlewares=[middleware_factory]) - subapp2 = web.Application(middlewares=[middleware_factory]) + app = web.Application(middlewares=[middleware]) + subapp1 = web.Application(middlewares=[middleware]) + subapp2 = web.Application(middlewares=[middleware]) + app['name'] = 'app' + subapp1['name'] = 'subapp1' + subapp2['name'] = 'subapp2' + subapp2.router.add_get('/to', handler) - with pytest.warns(DeprecationWarning): - subapp1.add_subapp('/b/', subapp2) - app.add_subapp('/a/', subapp1) - client = await aiohttp_client(app) + subapp1.add_subapp('/b/', subapp2) + app.add_subapp('/a/', subapp1) + client = await aiohttp_client(app) resp = await client.get('/a/b/to') assert resp.status == 200 - assert [(1, app), (1, subapp1), (1, subapp2), - (2, subapp2), (2, subapp1), (2, app)] == order + assert [(1, 'app'), (1, 'subapp1'), (1, 'subapp2'), + (2, 'subapp2'), (2, 'subapp1'), (2, 'app')] == order async def test_subapp_on_response_prepare(aiohttp_client) -> None: @@ -1348,7 +1352,6 @@ async def test_subapp_middleware_context(aiohttp_client, values = [] def show_app_context(appname): - @web.middleware async def middleware(request, handler): values.append('{}: {}'.format( appname, request.app['my_value'])) diff --git a/tests/test_web_middleware.py b/tests/test_web_middleware.py index af199190abf..fa434c74549 100644 --- a/tests/test_web_middleware.py +++ b/tests/test_web_middleware.py @@ -1,5 +1,3 @@ -import re - import pytest from yarl import URL @@ -10,7 +8,6 @@ async def test_middleware_modifies_response(loop, aiohttp_client) -> None: async def handler(request): return web.Response(body=b'OK') - @web.middleware async def middleware(request, handler): resp = await handler(request) assert 200 == resp.status @@ -32,7 +29,6 @@ async def test_middleware_handles_exception(loop, aiohttp_client) -> None: async def handler(request): raise RuntimeError('Error text') - @web.middleware async def middleware(request, handler): with pytest.raises(RuntimeError) as ctx: await handler(request) @@ -54,7 +50,6 @@ async def handler(request): return web.Response(text='OK') def make_middleware(num): - @web.middleware async def middleware(request, handler): resp = await handler(request) resp.text = resp.text + '[{}]'.format(num) @@ -298,128 +293,33 @@ async def paymethod(request): async def test_old_style_middleware(loop, aiohttp_client) -> None: - async def handler(request): + async def view_handler(request): return web.Response(body=b'OK') - async def middleware_factory(app, handler): - - async def middleware(request): + with pytest.warns( + DeprecationWarning, match='Middleware decorator is deprecated' + ): + @web.middleware + async def middleware(request, handler): resp = await handler(request) assert 200 == resp.status resp.set_status(201) resp.text = resp.text + '[old style middleware]' return resp - return middleware - - with pytest.warns(DeprecationWarning) as warning_checker: - app = web.Application() - app.middlewares.append(middleware_factory) - app.router.add_route('GET', '/', handler) - client = await aiohttp_client(app) - resp = await client.get('/') - assert 201 == resp.status - txt = await resp.text() - assert 'OK[old style middleware]' == txt - - assert len(warning_checker) == 1 - msg = str(warning_checker.list[0].message) - assert re.match('^old-style middleware ' - '".' - 'middleware_factory at 0x[0-9a-fA-F]+>" ' - 'deprecated, see #2252$', - msg) - - -async def test_mixed_middleware(loop, aiohttp_client) -> None: - async def handler(request): - return web.Response(body=b'OK') - - async def m_old1(app, handler): - async def middleware(request): - resp = await handler(request) - resp.text += '[old style 1]' - return resp - return middleware - - @web.middleware - async def m_new1(request, handler): - resp = await handler(request) - resp.text += '[new style 1]' - return resp - - async def m_old2(app, handler): - async def middleware(request): - resp = await handler(request) - resp.text += '[old style 2]' - return resp - return middleware - @web.middleware - async def m_new2(request, handler): - resp = await handler(request) - resp.text += '[new style 2]' - return resp - - middlewares = m_old1, m_new1, m_old2, m_new2 - - with pytest.warns(DeprecationWarning) as w: - app = web.Application(middlewares=middlewares) - app.router.add_route('GET', '/', handler) - client = await aiohttp_client(app) - resp = await client.get('/') - assert 200 == resp.status - txt = await resp.text() - assert 'OK[new style 2][old style 2][new style 1][old style 1]' == txt - - assert len(w) == 2 - tmpl = ('^old-style middleware ' - '".' - '{} at 0x[0-9a-fA-F]+>" ' - 'deprecated, see #2252$') - p1 = tmpl.format('m_old1') - p2 = tmpl.format('m_old2') - - assert re.match(p2, str(w.list[0].message)) - assert re.match(p1, str(w.list[1].message)) - - -async def test_old_style_middleware_class(loop, aiohttp_client) -> None: - async def handler(request): - return web.Response(body=b'OK') - - class Middleware: - async def __call__(self, app, handler): - async def middleware(request): - resp = await handler(request) - assert 200 == resp.status - resp.set_status(201) - resp.text = resp.text + '[old style middleware]' - return resp - return middleware - - with pytest.warns(DeprecationWarning) as warning_checker: - app = web.Application() - app.middlewares.append(Middleware()) - app.router.add_route('GET', '/', handler) - client = await aiohttp_client(app) - resp = await client.get('/') - assert 201 == resp.status - txt = await resp.text() - assert 'OK[old style middleware]' == txt - - assert len(warning_checker) == 1 - msg = str(warning_checker.list[0].message) - assert re.match('^old-style middleware ' - '".Middleware object ' - 'at 0x[0-9a-fA-F]+>" deprecated, see #2252$', msg) + app = web.Application(middlewares=[middleware]) + app.router.add_route('GET', '/', view_handler) + client = await aiohttp_client(app) + resp = await client.get('/') + assert 201 == resp.status + txt = await resp.text() + assert 'OK[old style middleware]' == txt async def test_new_style_middleware_class(loop, aiohttp_client) -> None: async def handler(request): return web.Response(body=b'OK') - @web.middleware class Middleware: async def __call__(self, request, handler): resp = await handler(request) @@ -446,7 +346,6 @@ async def handler(request): return web.Response(body=b'OK') class Middleware: - @web.middleware async def call(self, request, handler): resp = await handler(request) assert 200 == resp.status