Enhancement: Resource representation response based on `Accept` header about litestar

[provinzkraut]

@provinzkraut wouldn't the way you've suggested result in the users having to implement both get_json and get_xml? If that's the case, they can already do that. The only benefit of the approach you suggested would be that Litestar would handle the serialization properly. I think what users would find useful is just do:

@get("/")
async def get_foo() -> dict[str, str]:
    ...

Then litestar would automatically figure out which format to serialize into based on the accept header. This would be the simplest from the user perspective.

That feels like a different feature to me so maybe we're not talking about the same stuff here? 👀

If we want that, wouldn't the easiest solution to allow specifying additional response serialisers (ideally as layered parameters)?

@get("/")
async def get_foo() -> dict[str, str]:
    ...


app = Litestar(
  [get_foo],
  serializers={MediaType.XML: to_xml}
)

and then Litestar checks if for a given Accept header, an appropriate serialiser is registered?

response shouldn't need to worry about the request

I didn't understand this. We already have a dependency on the Request object when we convert a Response into an ASGIResponse via the to_asgi_response. What would be the potential issues if we pass the Request object into the Response directly (optionally)?

Yeah I don't like that either 😬

It's something we currently need to do, but it's also only a workaround for some other issues we were facing. I can't actually remember anymore what exactly this was.. @peterschutt I remember we were talking about this a lot during the 2.0 churn when this was implemented and I think we agreed that it was the path of least resistance but there was some other way of handling this better.. Would have to dig up the conversation from back then 👀

[peterschutt]

Agree it would be nice to have more flexibility with response encoding.

I'm not sure about specifying multiple response class types b/c I think this would worsen the UI for defining a customized response class. E.g,.:

# currently possible

class MyCustomResponse(Response):
    type_encoders = {...}  # example response customization

app = Litestar(..., response_class=MyCustomResponse)

If multiple response class types were defined, that customization would have to be applied to all of them.

I think I noticed a mention in discord about passing request context to the response object in order to be able to discriminate how the response content should be encoded? So how about if we added a method like Response.get_encoder() that housed our default implementation and could be overridden to extend support?

E.g.,

class Response:
    def __init__(self, ..., request_context: ...) -> None: ...

    def get_encoder(self) -> Callable[[Any], bytes]: ...

    def render(self, ...) -> bytes:
        encoder = self.get_content_encoder()

[guacs]

So how about if we added a method like Response.get_encoder() that housed our default implementation and could be overridden to extend support?

In this case, the user would need to then give an encoder based on any custom logic they have right? I like this, but I think we could also consider taking in a mapping of accept headers to encoders at all layers, and then the default implementation wouldn't be to default to serializing as JSON but to find the encoder based on the accept header and then use that for the encoding (this is only in the case where media_type is not explicitly defined by the user).

[provinzkraut]

@kedod We already have an Accept header class that allows media-type based matching

I think I noticed a mention in discord about passing request context to the response object in order to be able to discriminate how the response content should be encoded?

I don't really like this idea as it complicates things further. The response shouldn't need to worry about the request. If we were doing this, IMO this functionality should be in the place where the response is created.

---

Seeing how this falls generally in the category of "metadata based routing", why don't we just do that? We can already define multiple handlers for the same path, given they differ in their method, why not allow the same for another set of parameters, such as the accept header (or any other type of header really)? That would give the users the most flexibility and make for a rather straightforward implementation on our side.

@get("/", match=[Accept(MediaType.JSON)])
async def get_json() -> dict[str, str]:
 ...


@get("/", match=[Accept(MediaType.XML)])
async def get_xml() -> dict[str, str]:
 ...

[guacs]

@provinzkraut wouldn't the way you've suggested result in the users having to implement both get_json and get_xml? If that's the case, they can already do that. The only benefit of the approach you suggested would be that Litestar would handle the serialization properly. I think what users would find useful is just do:

@get("/")
async def get_foo() -> dict[str, str]:
    ...

Then litestar would automatically figure out which format to serialize into based on the accept header. This would be the simplest from the user perspective.

response shouldn't need to worry about the request

I didn't understand this. We already have a dependency on the Request object when we convert a Response into an ASGIResponse via the to_asgi_response. What would be the potential issues if we pass the Request object into the Response directly (optionally)?

[kedod]

If we want that, wouldn't the easiest solution to allow specifying additional response serialisers (ideally as layered parameters)?

I like this idea the most.
I assume we can pass matched serializer to the response_class and use it in render method later.
We can set default serializers to keep consistency with the actual Litestar behaviour too.