litestar
litestar copied to clipboard
litestar-org
Feature: Support for middleware parameter specification
Thinking about ways to make parameter specification and parsing / validation available to the middleware stack.
Perhaps something like:
@runtime_checkable
class MiddlewareProtocol(Protocol):
parameters: dict[str, tuple] = {"middleware_required_header": (int, Parameter(header="required-header"))}
def __init__(self, app: ASGIApp): # pragma: no cover
...
async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: # pragma: no cover
# this obv. not actually defined in the protocol, but just for demo
if self.validated_paraeters.get("middleware_required_header") != "13":
...
else:
...
@property
def validated_parameters(self) -> dict[str, Any]:
# this mapping actually constructed dynamically
return {"middleware_required_header": 13}
Lets discuss:)
So, there is a problem with defining concrete attributes on protocols - pydantic doesn't play nice with this.
How about adding a container type for Middleware that allows defining stuff like parameters, descriptions etc?
Otherwise we might need to use an abstract class instead of protocol.
So, there is a problem with defining concrete attributes on protocols - pydantic doesn't play nice with this.
OK cool, Also there is the nature of a protocol being that it's just an interface, I.e, shouldn't need to extend from the base to fulfil the protocol.
How about adding a container type for Middleware that allows defining stuff like parameters, descriptions etc?
Can you give example of how this looks?
Otherwise we might need to use an abstract class instead of protocol.
This is intuitive to me, but we shouldn't need to do away with the protocol altogether, we could just have the base class that implements the protocol that users could extend for their custom middleware that allows us to handle parameter validation/parsing/docs for the parameters required by the middleware?
So, as a container I'd try to do something like this probably:
from pydantic import validate_arguments
from starlite.params import Parameter
from starlette.middleware.base import BaseHTTPMiddleware
from starlette.middleware import Middleware as StarletteMiddleware
class DefinedMiddleware:
@validate_arguments(config={"arbitrary_types_allowed": True})
def __init__(
self,
middleware: Union[StarletteMiddleware, Type[BaseHTTPMiddleware], Type[MiddlewareProtocol]],
parameters: Optional[Dict[str, Tuple[Any, Parameter]]] = None,
description: Optional[str] = None,
tags: Optional[List[str]] = None,
response_headers: Optional[Dict[str, ResponseHeader]] = None,
):
self.middleware = middleware
self.parameters = parameters
self.description = description
self.tags = tags
self.response_headers = response_headers
It will wrap the middleware, which is required, and then receive ooptional values for parameters, description, tags and response_headers. Im not 100% sure about description here, but the rest of the values do make sense to me:
basically we will use the parameters to do validation and perhaps also calculate the required kwargs including the middleware component. We wil also use them to generate the OpenAPI schema, and this is where the other components (tags, response_headers etc.) come in.
Also, we can create a function that creates the DefinedMiddleware, or just give it a different name, to make more appealing to users.
So, as a container I'd try to do something like this probably:
from pydantic import validate_arguments from starlite.params import Parameter from starlette.middleware.base import BaseHTTPMiddleware from starlette.middleware import Middleware as StarletteMiddleware class DefinedMiddleware: @validate_arguments(config={"arbitrary_types_allowed": True}) def __init__( self, middleware: Union[StarletteMiddleware, Type[BaseHTTPMiddleware], Type[MiddlewareProtocol]], parameters: Optional[Dict[str, Tuple[Any, Parameter]]] = None, description: Optional[str] = None, tags: Optional[List[str]] = None, response_headers: Optional[Dict[str, ResponseHeader]] = None, ): self.middleware = middleware self.parameters = parameters self.description = description self.tags = tags self.response_headers = response_headersIt will wrap the middleware, which is required, and then receive ooptional values for
parameters,description,tagsandresponse_headers. Im not 100% sure about description here, but the rest of the values do make sense to me: basically we will use the parameters to do validation and perhaps also calculate the required kwargs including the middleware component. We wil also use them to generate the OpenAPI schema, and this is where the other components (tags, response_headers etc.) come in.Also, we can create a function that creates the DefinedMiddleware, or just give it a different name, to make more appealing to users.
I see how this will give us info about the parameters for documentation, and an avenue to set other docs properties, but what about having access to the parsed/validated values inside the middleware?
This is where I think having a concrete base class in addition to the protocol might be a good idea. For example,
class StarliteBaseMiddleware(MiddlewareProtocol, Generic[T_model]):
parameter_model: type[T_model]
@property
def parameters(self) -> T_model:
...
...
... so the parameters could be accessed in __call__() and we can handle the client error for validation.
Why would we want validated access to parameters inside a middleware? It just seems natural if we are bookkeeping all the types anyway that we could offer an interface to the validated values.
The wrapper approach would be better if using existing/3rd party middlewares as it would provide a way to tell us what the supplied middleware requires for generating the docs.
The base-class approach would be better for custom built middleware as validated and parsed parameter values would be available to the logic inside the middleware, info for docs would be available, and also the boilerplate required to check that a header or query param or whatever exists and is valid would be abstracted away.
But why would we need a base class for this? We can simply use a wrapper as I suggested and use the existing stuff. The base class is not required for this. I dont understand what using a base class gives us here.
OK say that I want build middleware that logs some info drawn from a header and it should be an error if a client is missing that header value (I haven't run this code at all btw so might not actually run):
class MyRequestLoggingMiddleware(MiddlewareProtocol):
def __init__(self, app: ASGIApp):
super().__init__(app)
self.app = app
async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None:
if scope["type"] == "http":
headers = Headers(scope)
calling_app = headers.get("caller")
if calling_app is None:
raise ValidationException
try:
calling_app_uuid = UUID(calling_app)
except ValueError:
raise ValidationException
request = Request(scope)
logger.info("%s - %s - %s", request.method, request.url, calling_app_uuid)
await self.app(scope, receive, send)
This could be:
class HeaderModel(BaseModel):
caller: UUID
class MyRequestLoggingMiddleware(StarliteBaseMiddleware[HeaderModel]):
param_model = HeaderModel
def __init__(self, app: ASGIApp):
super().__init__(app)
self.app = app
async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None:
if scope["type"] == "http":
request = Request(scope)
logger.info("%s - %s - %s", request.method, request.url, self.parameters.caller)
await self.app(scope, receive, send)
I don't understand how wrapping a custom middlware in a container class after it is written can assist the logic inside the class itself like the base class approach does.
I see what you mean now. You want some way to extend typing info regarding parameters to be useful for Middleware. I'm not sure how viable this is tbh. We need to try it out and run some tests etc.
Yes, a way that could provide type info for the middleware required params for documenting (I think that was what the original convo in discord was about) and so just exploring doing it in a way that could provide runtime support for the middleware logic too. Not a hill I'm willing to die on though.
