Additional accept functionality

[ahopkins]

This adds some convenience to accept parsing:

• An Accept object has both has_wildcard (at least one) and is_wildcard (both parts)
• Add AcceptContainer.match_explicit (no wildcard matching)
• Add AcceptContainer.find_first (see below)

header = "application/json;q=0.8,text/csv;q=0.9,*/*;q=0.5"
acceptable = parse_accept(header)
priority = acceptable.find_first(["application/json", "text/csv"])
assert priority.match("text/csv")

---

There is some discussion that started on Discord server about what should the in operator do: match or match_explicit. TBD.

[Tronic]

The API is all different from before and from the original post on this PR, even type names were refactored. See the documentation in code for why there need to be three different ways to do the matching.

The biggest remaining problem, it would seem, is the handling of missing accept header, which leads to repeating a pattern like

# Check that either the client didn't specify accept header or that it matches
is_acceptable = not request.accept or request.accept.match("some/type")

This would be solved by defaulting */*;q=0 as the header value, rather than an empty string, where the client supplies no header. But I can see why Sanic injecting its own ideas of headers into public accessors could be problematic (even if well documented).

[Tronic]

I tested browsers and found out that they all give q=0.9 for xml, and q=0.8 for */*. This allows for an alternative implementation of this accept logic that has no special treatment for the wildcard but where the choose function out of equally q-valued options in the order of preference of Sanic (first the route format if known, then all supported formats).

• Browsers would still always get html because they support nothing else (other than via wildcard).
• Other clients, incl. curl, requests, fetch() and XHR send by default */* (q=1) and would get Sanic's preferred format (route, ...)

Neither of those cases would change. Only with a client that explicitly lists multiple supported formats would then not get the first/best one that it lists but the route format, provided that the route format is of highest possible q as well.

I will look into this for feasibility (correctness, simplicity of implementation/docs, etc).

[ahopkins]

Where is this being done? Is it ignoring q values?

[Tronic]

In the class that this docstring is for? Operator in is the one from list. Check the docs of AcceptList.match for how the order is determined (it considers q and order of both lists).

[Tronic]

I didn't check how the API docs render, but the class tries to give an overview, with the match function going to specifics of that rather complex matching logic. Hopefully that shows up decently.

[ahopkins]

There are a lot of changes in the tests. I still need to dig into them to understand the scope of this change.

[Tronic]

There are a lot of changes in the tests. I still need to dig into them to understand the scope of this change.

I suggest looking at repr(req.accept) and the debug log entries shown in guess_mime (error page logic), as well as the new test that takes advantage of this. Half of parametrized cases on some tests had to be removed due to removed functionality, IIRC, and all tests on subtype and type objects that used to support matching were removed (now they are simple str, for performance and simplicity).

[Tronic]

Regarding tests, the big thing missing is routes having error_format set or automatically determined. The test client would always give me only empty string. It is not directly related to this PR though, but such tests would have made the empty() PR just merged fail big time (and it passed without any changes to tests).

[Tronic]

NOTE: This branch is OUT OF DATE. I had been doing further development on error-format-redux, to which today's change requests are implemented to, now. It has better tests and is generally better in terms on request.accept too.

Closing this in favor of reviewing and merging #2668 to main, containing also everything done on this branch.

[ahopkins]

NOTE: This branch is OUT OF DATE. I had been doing further development on error-format-redux, to which today's change requests are implemented to, now. It has better tests and is generally better in terms on request.accept too.

Closing this in favor of reviewing and merging #2668 to main, containing also everything done on this branch.

This is not good practice. We need a single PR that does the accept changes independent of the error formatting.

[Tronic]

Yeah, I messed them up originally, sorry about that. The two features are quite intermingled, and hard to do separately, so I ended up having to sync updates between the branches and that was a lot of work, so I then ended up working only on the other PR. I couldn't have found good design for the accept header without actually using it in error format selection and one of my apps, so that had to be done concurrently.

You can still take headers.py and request.py from error-format-redux and it will run with the main branch. In this sense they could have stayed separate, but #2668 cannot work with the old request.accept at all.

Building PRs on top of PRs (whether against that other branch or main) is tedious, so I now believe it should have simply been one combined PR from the start (it is still not that big) -- but that became apparent to me only after a week of working on it.

[ahopkins]

It still needs to be its own. You can branch from a feature branch.

[Tronic]

The other PR touches one property of Request and one internal function of errorpages. It is not by any means a large one. I stick to that due to practical reasons both are better kept in the same PR, due to the two things being deeply intertwined. If it were a fork of this, it would have needed a merge to this, to the same end result, but conflicts in tests and other trouble.

End of discussion on my part here. This is a thing I will eventually have to take to others if you refuse to review that PR at all. Continue discussion of what needs to be fixed there, if you wish.