Endpoint class decorators

[azmeuk]

I need to implement a decorator for an endpoint, and for some technical reasons, I would like to implement it with a class. I also would like my decorator to not need parenthesis when called. (i.e. prefer @my_decorator over @my_decorator()).

Here is a simple implementation:

from starlette.responses import JSONResponse
from starlette.applications import Starlette
from starlette.routing import Route


class foobar:
    def __init__(self, func):
        self.func = func

    def __call__(self, *args, **kwargs):
        return self.func(*args, **kwargs)


@foobar
def my_endpoint(request):
    return JSONResponse({"response": "my_endpoint"})


app = Starlette(
    debug=True,
    routes=[
        Route('/my_endpoint', my_endpoint),
    ],
)

But accessing to /my_endpoint produces this traceback:

Traceback (most recent call last):
  File "~/myproject/env/lib/python3.8/site-packages/uvicorn/protocols/http/httptools_impl.py", line 385, in run_asgi
    result = await app(self.scope, self.receive, self.send)
  File "~/myproject/env/lib/python3.8/site-packages/uvicorn/middleware/proxy_headers.py", line 45, in __call__
    return await self.app(scope, receive, send)
  File "~/myproject/env/lib/python3.8/site-packages/starlette/applications.py", line 102, in __call__
    await self.middleware_stack(scope, receive, send)
  File "~/myproject/env/lib/python3.8/site-packages/starlette/middleware/errors.py", line 178, in __call__
    raise exc from None
  File "~/myproject/env/lib/python3.8/site-packages/starlette/middleware/errors.py", line 156, in __call__
    await self.app(scope, receive, _send)
  File "~/myproject/env/lib/python3.8/site-packages/starlette/exceptions.py", line 82, in __call__
    raise exc from None
  File "~/myproject/env/lib/python3.8/site-packages/starlette/exceptions.py", line 71, in __call__
    await self.app(scope, receive, sender)
  File "~/myproject/env/lib/python3.8/site-packages/starlette/routing.py", line 551, in __call__
    await route.handle(scope, receive, send)
  File "~/myproject/env/lib/python3.8/site-packages/starlette/routing.py", line 228, in handle
    await self.app(scope, receive, send)
  File "./app.py", line 11, in __call__
    return self.func(*args, **kwargs)
TypeError: my_endpoint() takes 1 positional argument but 3 were given

I am not really sure what is going wrong here. I also did not find documentation about endpoint decorators with starlette.
Do I make mistakes when defining my decorator or is it something not supported by starlette?

PS: I used starlette 0.13 and python 3.8

Important

• We're using Polar.sh so you can upvote and help fund this issue.
• We receive the funding once the issue is completed & confirmed by you.
• Thank you in advance for helping prioritize & fund our backlog.

[ianhoffman]

Looks like what you're seeing is caused by this block in starlette.routing.Route:

        if inspect.isfunction(endpoint) or inspect.ismethod(endpoint):
            # Endpoint is function or method. Treat it as `func(request) -> response`.
            self.app = request_response(endpoint)
            if methods is None:
                methods = ["GET"]
        else:
            # Endpoint is a class. Treat it as ASGI.
            self.app = endpoint

Basically, you seem to expect that your class decorator will get wrapped with request_response (as it would be if it was a function or method). But it doesn't get wrapped, because it's a class, and so it's called with three parameters — scope, receive, and send — instead of a single Request object, as you seem to expect.

So it seems class decorators are currently unsupported.

[lovelydinosaur]

Thanks @ianhoffman that's entirely correct.

One way we could resolve this would be if we had something more explicit in the routing, rather that "guessing" if the callable is a request/response callable or an ASGI app. Eg. Route(url, func=...) and Route(url, app=...).

[ianhoffman]

@tomchristie That seems reasonable. Are there worries about backward compatibility?

IMO it's strange that routes can even accept an ASGI app as an argument. Doesn't an app have many routes, which each map to a given function (or view, in Django parlance)? It's cool that ASGI supports nested ASGI apps — I guess the idea is to simplify composing middleware? I'm just curious if passing an app to a route is even something which Starlette needs to support. ASGI apps also seem like a lower-level interface which the average developer using Starlette shouldn't need to care about. I may be rambling here, though.

If you do want to adopt func and app as keyword arguments, I'd happily put together a PR.

[florimondmanca]

@ianhoffman An ASGI app doesn't have to support routing in itself — e.g. PlainTextResponse (and other response classes in Starlette) are ASGI apps (since they implement __call__(scope, receive, send)).

AFAIU the purpose of the ASGI part of Route() is to support HTTPEndpoint and others — i.e. class-based views, which do implement ASGI. (For full-fledged Starlette apps, Mount() is probably more suitable, but it seems true that there's nothing preventing us from passing a Starlette instance to Route().)

I think removing the guess from Route() is probably a good idea too, though in this case it will result in you having to pass app=... when really your original endpoint is a function.

@azmeuk Have you considered using functools.update_wrapper()? Would that make the class behave "like a function", eg make inspect.isfunction() return True?

^ Just tried it out myself, it won't work:

Code (click to expand)

import functools
import inspect

class decorator:
    def __init__(self, func):
        functools.update_wrapper(self, func)
        self.func = func

    def __call__(self, *args, **kwargs):
        return self.func(*args, **kwargs)


@decorator
def add(x, y):
        return x + y

inspect.isfunction(add)  # False

[ianhoffman]

@florimondmanca this is a bigger change, but couldn't HTTPEndpoint (and other CBVs) be modified to accept a request object instead of a 3-tuple? The request holds information about scope, receive, and send, so it should be functionally equivalent. Then every callable passed to route can accept the same arguments.

Obviously that might break a lot of existing contracts, so I get it if it's impractical. But it does seem like a cleaner API. Any reasons HTTPEndpoint must implement ASGI?

[florimondmanca]

@ianhoffman Now that I think of it, my usual workaround for endpoint decorators in Starlette is just to switch to HTTPEndpoint. So long as the decorator takes an ASGI app and spits out an ASGI app, you should be fine (Starlette will always treat the endpoint as ASGI, not as a request -> response function).

So, in your example:

@foobar
class MyEndpoint(HTTPEndpoint):
    async def get(self, request):
        return JSONResponse({"response": "my_endpoint"})

This is a constraint I had to impose to the API of asgi-caches, for example (source):

from asgi_caches.decorators import cached
from starlette.endpoints import HTTPEndpoint

@cached(cache)
class UserDetail(HTTPEndpoint):
    async def get(self, request):
        ...

[ianhoffman]

Gotcha — using a CBV instead of a functional view does seem like a reasonable fix. That's probably good enough given this isn't likely to be a common issue.

[lovelydinosaur]

AFAIU the purpose of the ASGI part of Route() is to support HTTPEndpoint and others — i.e. class-based views, which do implement ASGI.

Right. And actually thinking about it, we don’t need to be doing that.

The request/response function to Route needs to accept a request and return a response. I’d assumed we can do that on CBV’s because eg init just returns the class instance. (So eg in Django there’s a ViewClass.as_view() hoop that users jump through.)

My solution had been “okay, let’s have Route accept either a request/response function, or an ASGI app. Then CBVs can be implemented as mini ASGI apps)

Happily, in our case I think instantiating the CBV can be the request/response function, because “response” here just means, an ASGI callable. So... if the CBV init method accepts a request, and if call implements the ASGI interface and handles dispatch, then the classes are request/response callables.

We can then drop the “Route can also be an ASGI app” case, because we don’t really need it. Mount is appropriate for pointing at apps, Route is appropriate for pointing as req/response callables.

[florimondmanca]

One question, then — will Starlette still support decorating endpoints at the ASGI level?

The fact that HTTPEndpoint subclasses are ASGI apps is crucial for asgi-caches (and potentially other packages) to be compatible with Starlette and its descendants (FastAPI, etc), without having to depend on the specifics of Starlette's request/response models.

It's a crucial fact, but of course it doesn't mean it's the only way to do things. So, lemme try to see if this would work…

Previously, an ASGI-level CBV decorator could basically be a proxy to an ASGI middleware, so…

def decorate(endpoint: ASGIApp) -> ASGIApp:
    return SomeASGIMiddleware(endpoint)

From @tomchristie's description, I understand the proposed change would be to make CBVs look something like this…

class HTTPEndpoint:
    def __init__(self, request: Request):
        self.request = request

    def __await__(self) -> Iterator[ASGIApp]:
        handler = getattr(self, self.request.method)
        return handler(self.request).__await__()

    # Example user code
    async def get(self, request) -> Response:
        return PlainTextResponse("...")

This way HTTPEndpoint subclasses are Request -> Response async callables, just like their function-based counterparts — which makes both approaches consistent from an interface POV, which sounds legit.

But when previously an HTTPEndpoint subclass itself was an ASGI app, you now have to 1) instanciate it, 2) await the instance in order to access an ASGI app, i.e. their signature is:

ASGIView = Callable[[...], Awaitable[ASGIApp]]

Decorators would end up looking like:

def decorate(view: ASGIView) -> ASGIView:
    async def wrapper(*args: Any, **kwargs: Any) -> ASGIApp:
        app = await view(*args, **kwargs)  # e.g. a Starlette 'Response' instance...
        return SomeASGIMiddleware(app)

    return wrapper

It's a bit more involved, but the good thing is that this would work with both class- and function-based views (because they'd now have the same signature by design).

(I don't think this would work out of the box with any view interface though. Eg. FastAPI's view interface doesn't return an ASGI app instance, so ASGI decorators in the form of the above won't work in that case. But that's a different problem — FastAPI doesn't provide a way to decorate views at the ASGI level today anyway.)

So, uh, to answer my own question…

Will Starlette still support decorating endpoints at the ASGI level?

Yes (but differently).

[lovelydinosaur]

Actually it’s more complicated then that, since the request/response function needs to be awaitable. I think we can still do this, but needs a bit more investigation.

Failing that we could always do something like Django’s as_view

[florimondmanca]

@tomchristie

since the request/response function needs to be awaitable.

I believe the solution would be to implement __await__() directly, as hinted in my previous comment.

[lovelydinosaur]

Ah yeah absolutely. Sorry, I’d see the issue, and had already been meaning to respond, didn’t see your comment getting there in the meantime!

[Davmuz]

Hi, same problem using dependency injection:

from starlette import applications
from starlette import responses
from starlette import routing

class Ping:
    def __init__(self, db):
        self._db = db

    async def __call__(self, request):
        ...
        return responses.PlainTextResponse("fails")

db = ...
myview = Ping(db)

app = applications.Starlette(routes=[routing.Route("/", myview, methods=["GET"])])

Raises:

TypeError: __call__() takes 2 positional arguments but 4 were given

The expected behavior is that the method is called like a function.

The workaround using HTTPEndpoint complicates the implementation without gains and makes even more complex to build handler factories.

[invokermain]

Just to check, currently the only way to implement endpoint decorators is by rewriting them as a CBV?

On that point, is there recommended best practice around using a function endpoint or a class endpoint? Surprised there isn't an opinion in the docs.

Currently, it seems like for simple endpoints the FBV is equivalent but requires less code so would be best practice? But then would be strange to have a few endpoints as CBVs just to implement a decorator and the rest as FBVs? Just thinking out loud around this topic.

For the record I'm trying to implement user scopes, with certain endpoints requiring certain scopes to be found under request.user. Would be neatest to have a decorator @requires_scope(2) etc which returns a 403 if the user does not have the scope. Is the CBV with decorator for those endpoints the easiest way to implement this?