simplifying middleware (#2252)

* simplifying middleware, fix #2225

* set depeciation message

* Update test_web_middleware.py

* allow mixed middleware styles

* use decorator for new middleware

* rename decorator to 'middleware' and add docs

* tweaking docs

* fix spelling

* 'streamed response' not 'steamed response' :-)

aiohttp/web.py

@@ -7,6 +7,7 @@
 import warnings
 from argparse import ArgumentParser
 from collections import Iterable, MutableMapping
+from functools import partial
 from importlib import import_module
 
 from yarl import URL
@@ -140,7 +141,7 @@ def freeze(self):
             return
 
         self._frozen = True
-        self._middlewares = tuple(reversed(self._middlewares))
+        self._middlewares = tuple(self._prepare_middleware())
         self._router.freeze()
         self._on_loop_available.freeze()
         self._on_pre_signal.freeze()
@@ -272,6 +273,18 @@ def _make_request(self, message, payload, protocol, writer, task,
             self._loop,
             client_max_size=self._client_max_size)
 
+    def _prepare_middleware(self):
+        for m in reversed(self._middlewares):
+            if getattr(m, '__middleware_version__', None) == 1:
+                # first argument is request or r and this is
+                # new-style middleware
+                yield m, True
+            else:
+                warnings.warn('old-style middleware "{}" deprecated, '
+                              'see #2252'.format(m.__name__),
+                              DeprecationWarning, stacklevel=2)
+                yield m, False
+
     @asyncio.coroutine
     def _handle(self, request):
         match_info = yield from self._router.resolve(request)
@@ -291,8 +304,11 @@ def _handle(self, request):
         if resp is None:
             handler = match_info.handler
             for app in match_info.apps[::-1]:
-                for factory in app._middlewares:
-                    handler = yield from factory(app, handler)
+                for m, new_style in app._middlewares:
+                    if new_style:
+                        handler = partial(m, handler=handler)
+                    else:
+                        handler = yield from m(app, handler)
 
             resp = yield from handler(request)
 

aiohttp/web_middlewares.py

@@ -6,6 +6,7 @@
 
 
 __all__ = (
+    'middleware',
     'normalize_path_middleware',
 )
 
@@ -23,6 +24,11 @@ def _check_request_resolves(request, path):
     return False, request
 
 
+def middleware(f):
+    f.__middleware_version__ = 1
+    return f
+
+
 def normalize_path_middleware(
         *, append_slash=True, merge_slashes=True,
         redirect_class=HTTPMovedPermanently):
@@ -47,37 +53,32 @@ def normalize_path_middleware(
     """
 
     @asyncio.coroutine
-    def normalize_path_factory(app, handler):
-
-        @asyncio.coroutine
-        def middleware(request):
-
-            if isinstance(request.match_info.route, SystemRoute):
-                paths_to_check = []
-                if '?' in request.raw_path:
-                    path, query = request.raw_path.split('?', 1)
-                    if query:
-                        query = '?' + query
-                else:
-                    query = ''
-                    path = request.raw_path
-
-                if merge_slashes:
-                    paths_to_check.append(re.sub('//+', '/', path))
-                if append_slash and not request.path.endswith('/'):
-                    paths_to_check.append(path + '/')
-                if merge_slashes and append_slash:
-                    paths_to_check.append(
-                        re.sub('//+', '/', path + '/'))
-
-                for path in paths_to_check:
-                    resolves, request = yield from _check_request_resolves(
-                        request, path)
-                    if resolves:
-                        return redirect_class(request.path + query)
-
-            return (yield from handler(request))
-
-        return middleware
-
-    return normalize_path_factory
+    @middleware
+    def normalize_path_middleware(request, handler):
+        if isinstance(request.match_info.route, SystemRoute):
+            paths_to_check = []
+            if '?' in request.raw_path:
+                path, query = request.raw_path.split('?', 1)
+                if query:
+                    query = '?' + query
+            else:
+                query = ''
+                path = request.raw_path
+
+            if merge_slashes:
+                paths_to_check.append(re.sub('//+', '/', path))
+            if append_slash and not request.path.endswith('/'):
+                paths_to_check.append(path + '/')
+            if merge_slashes and append_slash:
+                paths_to_check.append(
+                    re.sub('//+', '/', path + '/'))
+
+            for path in paths_to_check:
+                resolves, request = yield from _check_request_resolves(
+                    request, path)
+                if resolves:
+                    return redirect_class(request.path + query)
+
+        return (yield from handler(request))
+
+    return normalize_path_middleware

changes/2225.misc

@@ -0,0 +1,4 @@
+Simplify middleware
+
+So they are simple coroutines rather than coroutine factories
+returning coroutines.

docs/web.rst

@@ -997,69 +997,57 @@ the keyword-only ``middlewares`` parameter when creating an
    app = web.Application(middlewares=[middleware_factory_1,
                                       middleware_factory_2])
 
-A *middleware factory* is simply a coroutine that implements the logic of a
-*middleware*. For example, here's a trivial *middleware factory*::
+A *middleware* is just a coroutine that can modify either the request or
+response. For example, here's a simple *middleware* which appends
+``' wink'`` to the response::
 
-    async def middleware_factory(app, handler):
-        async def middleware_handler(request):
-            return await handler(request)
-        return middleware_handler
+    from aiohttp.web import middleware
 
-Every *middleware factory* should accept two parameters, an
-:class:`app <Application>` instance and a *handler*, and return a new handler.
+    @middleware
+    async def middleware(request, handler):
+        resp = await handler(request)
+        resp.text = resp.text + ' wink'
+        return resp
 
-The *handler* passed in to a *middleware factory* is the handler returned by
-the **next** *middleware factory*. The last *middleware factory* always receives
-the :ref:`request handler <aiohttp-web-handler>` selected by the router itself
-(by :meth:`UrlDispatcher.resolve`).
-
-.. note::
-
-   Both the outer *middleware_factory* coroutine and the inner
-   *middleware_handler* coroutine are called for every request handled.
+(Note: this example won't work with streamed responses or websockets)
 
-*Middleware factories* should return a new handler that has the same signature
-as a :ref:`request handler <aiohttp-web-handler>`. That is, it should accept a
-single :class:`Request` instance and return a :class:`Response`, or raise an
-exception.
+Every *middleware* should accept two parameters, a
+:class:`request <Request>` instance and a *handler*, and return the response.
 
 Internally, a single :ref:`request handler <aiohttp-web-handler>` is constructed
 by applying the middleware chain to the original handler in reverse order,
 and is called by the :class:`RequestHandler` as a regular *handler*.
 
-Since *middleware factories* are themselves coroutines, they may perform extra
+Since *middlewares* are themselves coroutines, they may perform extra
 ``await`` calls when creating a new handler, e.g. call database etc.
 
-*Middlewares* usually call the inner handler, but they may choose to ignore it,
+*Middlewares* usually call the handler, but they may choose to ignore it,
 e.g. displaying *403 Forbidden page* or raising :exc:`HTTPForbidden` exception
-if user has no permissions to access the underlying resource.
+if the user does not have permissions to access the underlying resource.
 They may also render errors raised by the handler, perform some pre- or
 post-processing like handling *CORS* and so on.
 
 The following code demonstrates middlewares execution order::
 
    from aiohttp import web
+
    def test(request):
        print('Handler function called')
        return web.Response(text="Hello")
 
-   async def middleware1(app, handler):
-       async def middleware_handler(request):
-           print('Middleware 1 called')
-           response = await handler(request)
-           print('Middleware 1 finished')
-
-           return response
-       return middleware_handler
+   @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(app, handler):
-       async def middleware_handler(request):
-           print('Middleware 2 called')
-           response = await handler(request)
-           print('Middleware 2 finished')
-
-           return response
-       return middleware_handler
+       print('Middleware 2 called')
+       response = await handler(request)
+       print('Middleware 2 finished')
+       return response
 
 
    app = web.Application(middlewares=[middleware1, middleware2])
@@ -1089,20 +1077,57 @@ a JSON REST service::
             body=json.dumps({'error': message}).encode('utf-8'),
             content_type='application/json')
 
+    @web.middleware
     async def error_middleware(app, handler):
+        try:
+            response = await handler(request)
+            if response.status == 404:
+                return json_error(response.message)
+            return response
+        except web.HTTPException as ex:
+            if ex.status == 404:
+                return json_error(ex.reason)
+            raise
+
+    app = web.Application(middlewares=[error_middleware])
+
+
+Old Style Middleware
+^^^^^^^^^^^^^^^^^^^^
+
+.. deprecated:: 2.3
+
+   Prior to *v2.3* middleware required an outer *middleware factory*
+   which returned the middleware coroutine. Since *v2.3* this is not
+   required; instead the ``@middleware`` decorator should
+   be used.
+
+Old style middleware (with an outer factory and no ``@middleware``
+decorator) is still supported. Furthermore, old and new style middleware
+can be mixed.
+
+A *middleware factory* is simply a coroutine that implements the logic of a
+*middleware*. For example, here's a trivial *middleware factory*::
+
+    async def middleware_factory(app, handler):
         async def middleware_handler(request):
-            try:
-                response = await handler(request)
-                if response.status == 404:
-                    return json_error(response.message)
-                return response
-            except web.HTTPException as ex:
-                if ex.status == 404:
-                    return json_error(ex.reason)
-                raise
+            resp = await handler(request)
+            resp.text = resp.text + ' wink'
+            return resp
         return middleware_handler
 
-    app = web.Application(middlewares=[error_middleware])
+A *middleware factory* should accept two parameters, an
+:class:`app <Application>` instance and a *handler*, and return a new handler.
+
+.. note::
+
+   Both the outer *middleware_factory* coroutine and the inner
+   *middleware_handler* coroutine are called for every request handled.
+
+*Middleware factories* should return a new handler that has the same signature
+as a :ref:`request handler <aiohttp-web-handler>`. That is, it should accept a
+single :class:`Request` instance and return a :class:`Response`, or raise an
+exception.
 
 .. _aiohttp-web-signals: