Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Accept weights #426

Open
nyarly opened this issue Apr 28, 2020 · 3 comments
Open

Implement Accept weights #426

nyarly opened this issue Apr 28, 2020 · 3 comments

Comments

@nyarly
Copy link
Contributor

nyarly commented Apr 28, 2020

As of #424 we can check for multiple types in an Accept header, but currently ignore client q parameters for selecting a response.

Gotham should respect the quality parameter and provide a response type as per https://tools.ietf.org/html/rfc7231#section-5.3

@msrd0
Copy link
Member

msrd0 commented May 21, 2020

I'm happy to help with this issue, however, I don't have a clear picture of what you are trying to acchieve. I guess this is what came to my mind after thinking about this for a while:

  1. Add feedback from the RouteMatcher to the handler, possibly by inserting a certain feedback type into the State. This will keep the current handler matching in place, but allow the handler to produce different responses without parsing the Accept header again.

  2. Change the return type of the RouteMatcher to include a match quality. Say the client requests image/webp,image/*;q=0.8, means he'd prefer an image in webp format over any other type of image. If we now have one handler with a matcher for image/png and one for image/webp, and the RouteMatcher gave the quality back to the router, it could make sure the handler for the image/webp type is preferred for above request.

  3. Parse and store the mime types with their assigned quality (e.g. in the State) before calling the matcher or the handler. This way we can keep the current handler matching, but give the handler access to the qualities, basically resulting in the same outcome as 1.

I guess my only use case right now is to allow modern browsers to use modern image types yet keep support for older browsers (basically the example in 2). However, I have never actually attempted to do this, neither with gotham or any other framework. Might be interesting to look at how other frameworks handle this.

@msrd0 msrd0 added this to the 0.6 milestone Aug 26, 2020
@nyarly
Copy link
Contributor Author

nyarly commented Oct 6, 2020

One fresh look here: a very reasonable place for a consumer app to handle Accept is in IntoRespone implementations. On the other hand, it's possible that different content types would require significantly different processing.

I'm going to suggest that we break out to separate issues:

Simpler: a middleware to inject a q-param sorted list of content types out of the Accept header, so that IntoResponse implementers can find the first type they can represent as a general pattern.

More complex: a kind of Accept router - something like setting a scope, and then matching only on accept headers, but such that the q-param is honored.

I'm also noting that it seems to me that the easiest way to add an AcceptRouterMatcher is with add_route_matcher, and I wonder if it would be worth looking at defining traits like ExtendRouteWithAccept and implementing them adjacent to their matchers.

@gotham-rs/gotham-core Looking for opinions on each of those thoughts. I'll likely create the sorted Accept list issue later today.

@msrd0
Copy link
Member

msrd0 commented Oct 6, 2020

I think there are several places where accessing the Accept header makes sense. If you want to switch between json and xml, you could easily do that in IntoResponse, however, if you want to load a different file (image.png vs image.webp), you'd need access in the handler itself. Also, the matcher needs access to it of course. It'd be cool if we could parse the Accept header into the State lazily to not slow down requests that don't need the info but have it in a central place for those that require it, eventhough I don't think that's possible right now.

Nevertheless, I think we need some kind of match quality "feedback" from the matcher, otherwise having one route with different accept header matchers, at least when used from a brower (that almost always adds a */*;q=0.1 at the end), is not behaving as expected.

@msrd0 msrd0 removed this from the 0.6 milestone Mar 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants