fastapi icon indicating copy to clipboard operation
fastapi copied to clipboard

[FEATURE] FastAPI-native class-based middleware

Open tuukkamustonen opened this issue 4 years ago • 8 comments

There are two ways to define middleware:

  1. Decorator-based via @app.middleware()
  2. Starlette's class-based via constructor or app.add_middleware()

While decorator-based middleware get FastAPI's Request object:

@app.middleware("http")
async def my_middleware(request: Request, call_next):
    ...
    response = await call_next(request)
    ...
    return response

Starlette's lower-level implementation feels a bit less lean:

class MyMiddleware:
    def __init__(self, app: ASGIApp) -> None:
        self.app = app

    async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None:
        if scope["type"] == "http":
            ...
            await self.app(scope, receive, send)
            ...
        else:
            await self.app(scope, receive, send)

(Something like that, I'm new to Starlette/FastAPI so not sure of exact details.)

How about supporting "FastAPI-native" middleware, where the signature would be similar to that of decorator's:

    async def __call__(self, request: Request, call_next):
        ...
        response = await call_next(request)
        ...
        return response

tuukkamustonen avatar Jun 02 '20 09:06 tuukkamustonen

You could do that today if you need class semantics. Here's a minimal example:

import uvicorn
from fastapi import FastAPI
from starlette.requests import Request

app = FastAPI()

@app.post('/')
async def test():
    pass


class MyMiddleware:
    async def __call__(self, request: Request, call_next):
        response = await call_next(request)
        print("I've been called!")
        return response


my_middleware = MyMiddleware()  # Do whatever you need the class for here
app.middleware("http")(my_middleware)


if __name__ == '__main__':
    uvicorn.run(app)

Note that by adding __call__ to a class, instances of that class effectively become functions with a bunch of other stuff on them. So you can register them with app.middleware just like any other function.

dbanty avatar Jun 02 '20 13:06 dbanty

True, didn't remember @ is just syntactic sugar.

It looks a bit ugly though? Although I guess some would say it looks "pythonic" instead. Native support would be better, or at least this should/could be mentioned in the docs.

I guess the details could be hidden like:

class MyMiddleware:
    def __init__(self, app: ASGIApp):
        app.middleware("http")(self)

    async def __call__(self, request: Request, call_next):
        response = await call_next(request)
        print("I've been called!")
        return response


MyMiddleware(app)

Looks funny, still.

It actually seems like this "simpler" method signature is applicable for http traffic only - websocket etc. need Starlette's signature. However, maybe a convenience base class or similar could be provided?

tuukkamustonen avatar Jun 02 '20 13:06 tuukkamustonen

The "Starlette way" as you put it is just a raw ASGI callable middleware. Most Starlette-specific middleware instead do it pretty much as you and @dbanty suggested, using the BaseHTTPMiddleware class:

class CustomHeaderMiddleware(BaseHTTPMiddleware):
    async def dispatch(self, request, call_next):
        response = await call_next(request)
        response.headers['Custom'] = 'Example'
        return response

middleware = [
    Middleware(CustomHeaderMiddleware)
]

app = Starlette(routes=routes, middleware=middleware)

And with FastAPI:

app = FastAPI()
app.add_middleware(CustomHeaderMiddleware, init_arg1='foo', init_arg2='bar')

The FastAPI decorator style is actually just a shortcut for this:

async def my_middleware(self, request, call_next):
        response = await call_next(request)
        response.headers['Custom'] = 'Example'
        return response

app.add_middleware(BaseHTTPMiddleware, dispatch=my_middleware)

sm-Fifteen avatar Jun 03 '20 12:06 sm-Fifteen

Ok so just extend BaseHTTPMiddleware, I see. Thanks for the pointers!

So unless I'm mistaken there are at least 3 ways to specify

async def request_id(request: Request, call_next):
    ...
    return response

class RequestIdMiddleware(BaseHTTPMiddleware):
    def __init__(self, param=1, *args, **kwargs):
        super().__init__(*args, **kwargs)
        self.param = param

    async def dispatch(request: Request, call_next):
        ...
        return response

class RequestIdCallable:
    def __init__(self, param=1):
        self.param = param

    async def __call__(self, request: Request, call_next):
        ...
        return response

class RequestIdSelfRegister:
    def __init__(self, app: FastAPI, param=1):
        self.param = param
        app.middleware("http")(self.dispatch)

    async def __call__(self, request: Request, call_next):
        ...
        return response

app = FastAPI()
app.middleware("http")(tx_id)
app.add_middleware(RequestIdMiddleware, param=3)
app.middleware("http")(RequestIdCallable(param=3))
RequestIdSelfRegister(app, param=3)

(Plus "FastAPI decorator style" that @sm-Fifteen showed above, but let's leave that out.)

Obviously the first one is quickest to write and use, but it cannot be parameterized, which is something you often want went "generalizing" code (into a separate lib perhaps).

For re-usable and parameterizable middleware, I kinda like the last two (RequestIdCallable, RequestIdSelfRegister) more, because a) there's less to write than with RequestIdMiddleware and IDE auto-complete/analysis works with them, too.

For non-http middleware, ASGI middleware via add_middleware() seems to be the only option.

Would it make sense to add such a quick comparison or recommendation regarding these - when to use which?

tuukkamustonen avatar Jun 03 '20 14:06 tuukkamustonen

For non-http middleware, ASGI middleware via add_middleware() seems to be the only option.

It's not just non-http mddleware, it's for middleware that that tries to be compatible with ASGI in general (instead of only targeting Starlette/FastAPI) in order to work seamlessly with Quart, Django Channels, ASGI Sanic and any other ASGI framework out there.

The 3 variants I'm personally seeing here are:

Stateless HTTP middleware
# @app.middleware("http")
async def request_id(request: Request, call_next):
    ...
    return response

app.add_middleware(BaseHTTPMiddleware, dispatch=request_id)

Note that the decorator merely calls app.add_middleware with BaseHTTPMiddleware, as above: https://github.com/encode/starlette/blob/519f5750b5e797bb3d4805fd29657674304ce397/starlette/applications.py#L191-L200

Instantiated/Parametrized HTTP middleware
class RequestIdMiddleware(BaseHTTPMiddleware):
    def __init__(self, app, param='Example'):
        super().__init__(app)
        self.param = param

    async def dispatch(request: Request, call_next):
        ...
        return response

# Starlette middleware stack pushing
app.add_middleware(RequestIdMiddleware, param='foo')
Generic ASGI middleware
class RequestIdASGIMiddleware:
    def __init__(self, app, param=1):
        self.app = app
        self.param = param

    async def __call__(self, scope, receive, send):
        if scope["type"] == "http":
            ...
            await self.app(scope, receive, send)
            ...
        else:
            await self.app(scope, receive, send)

# ASGI application wrapping
asgi_app = Starlette()
asgi_app = RequestIdASGIMiddleware(asgi_app, param=3)

I'm really not convinced your self-registering style is a good idea, putting side-effects in a class constructors sounds like it's going to cause all sorts of invisible problems at some point.

sm-Fifteen avatar Jun 03 '20 14:06 sm-Fifteen

Yeah, like that. I like the "names" that you gave for the approaches.

putting side-effects in a class constructors sounds like it's going to cause all sorts of invisible problems at some point.

Could you elaborate what "side-effects" we are discussing here? Is the problem with constructor, in specific (there could be separate init() method or whatever, but I don't think is needed). I think I've seen similar pattern used for Flask middleware but maybe it's just me (no problems so far with using that style for Flask).

tuukkamustonen avatar Jun 03 '20 15:06 tuukkamustonen

Could you elaborate what "side-effects" we are discussing here? Is the problem with constructor, in specific (there could be separate init() method or whatever, but I don't think is needed). I think I've seen similar pattern used for Flask middleware but maybe it's just me (no problems so far with using that style for Flask).

I mean having __init__() alter the application object you passed in, which is generally considered bad form by making explicit registration implicit. You're mutating something in a function whose purpose is to create an object, that's called a "side-effect"; it means running the same function multiple times with the same parameters will produce different results because something has been modified behind the curtains. For a constructor/object initialization method, I'm tempted to say that's almost never the right thing to do.

sm-Fifteen avatar Jun 08 '20 14:06 sm-Fifteen

I agree, it's not the perfect model (but it can be sufficient). Note it is suggested in Flask docs, see https://flask.palletsprojects.com/en/1.1.x/extensiondev/#the-extension-code:

class SQLite3:
    def __init__(self, app=None):
        self.app = app
        if app is not None:
            self.init_app(app)

    def init_app(self, app):
        ...

This allows middleware to be declared outside application factory, then initialized inside app factory. But it also allows directly constructing and initializing the middleware directly. If no one should do that, why is it made possible... (I guess because it works, and saves one method call. That's why I've been following that model, too.)

tuukkamustonen avatar Jun 10 '20 07:06 tuukkamustonen