OpenNTI / nti.dataserver

Other
0 stars 0 forks source link

Handling Accept header values which don't match #446

Open cutz opened 3 years ago

cutz commented 3 years ago

Some of our API currently allows requesting objects by ntiid with an accept type to alter the thing you get back. For example you given an ntiid that points at a question set, you can request that object via /dataserver2/objects/<ntiid> with Accept: application/vnd.nextthought.pageinfo+json to get the PageInfo object that question set exists on. If a page info does not exist you get a 406 back.

From what I can tell this currently gets implemented on a view by view basis and it's up to the view to deal with the 406 (example)

The implication of this is that if a client requests an ntiid with an Accept header that resolves to a context whose view isn't doing their own Accept header handling, the view will return something the client isn't expecting. They'll likely get an object back that isn't of the type they requested and they will also not get an error. That seems in conflict with the spec.

If no Accept header field is present, then it is assumed that the client accepts all media types. If an Accept header field is present, and if the server cannot send a response which is acceptable according to the combined Accept field value, then the server SHOULD send a 406 (not acceptable) response.

It would be really nice if we could figure out how to handle this at a lower level in the framework so that we aren't reliant on all views being written to do the right thing. There's obvious complexities and potentially application specific logic that has to happen which is why I guess it makes sense pyramid doesn't do anything by default. For example if the client asks for a vnd.nextthought.contentpackage is it valid to return an object that is a subtype of that which might normally have a mime type of vnd.nextthought.renderablecontentpackage? It seems reasonable to allow that but it ups the complexity.

Could we do something in our renderer that looks at the Accept header and what it is about to render? Is there another place (view_mapper?) that would make sense to do this?

Perhaps this is just a bad api to start with, but even so, it seems bad that in order to follow the spec (granted the normative term is SHOULD) all views have to watch for Accept headers. That doesn't seem scalable and as we saw recently is fraught with potential issue.

Curious as to people's thoughts here.

jamadden commented 3 years ago

The typical use of this pattern is to request data related to the resource you already have. That is, you have the URL of some resource (object) and want some other data, but you don't specifically know the URL for that data (or maybe the data doesn't even have a URL). We want clients to be data-driven, so we don't want them making up the URL on their own.

In some cases, we can directly anticipate a client's needs and provide a link on some object that points them to where they want to be.

That doesn't always work though. If you just have a URL, you don't have the link data, and it's really nice to avoid making an extra round trip just to request data that you're going to throw away, all except for the link that you then have to fetch. It's also not always possible to anticipate the data that that the client will want so that it can go in a link at the right time and place (again, assuming it has a URL of its own); that's leaving aside the performance issues that having dozens or hundreds of link providers can cause that can make it unwise to always provide a link to every related thing.

I could imagine the renderer doing something like checking the final MIME type, and if it "badly mismatches" (to be defined) the Accept header, doing something like component.queryMultiAdapter((request, context), IAlternateData, name=request.accept[0]). BUT: from a conceptual standpoint, render time seems far too late to try to do that. That really should be the responsibility of a view.

Better would be if we could select a view based on the Accept header. In the past, it wasn't possible to use Accept header as a predicate; I think it may be possible to do that now? But I also think it slows down view selection. And if you're concerned about some (default) view forgetting to send back a 406, the only way that works is if you can exhaustively list the available Accept headers and forbid any mismatches.

It's not clear to me why that would need to be done, though. It's the client's responsibility to check that the data it got is acceptable for its query. A 406 is a big help with that, but it's not sufficient; few servers are strict about that (they take the 'SHOULD' seriously):

$ http -v https://www.python.org/static/img/python-logo.png Accept:image/jpeg
GET /static/img/python-logo.png HTTP/1.1
Accept: image/jpeg
Accept-Encoding: gzip, deflate
Connection: keep-alive
Host: www.python.org
User-Agent: HTTPie/1.0.3

HTTP/1.1 200 OK
Accept-Ranges: bytes
Age: 570295
Content-Length: 10102
Content-Type: image/png
…

Going back a point, if it's possible to define "mismatched MIME types" sufficiently, I could imagine checking that being one of the responsibilities of the renderer and raising 406 errors in that case. I think that's better than it trying to find new data somehow. It even has a reasonable conceptual argument, from the standpoint of treating this as an API and not letting "errors" pass silently. But that definition is tricky…(and still doesn't remove all responsibility from the client: trust but verify).

I'm sure it would be possible to add some Accept-header helpers to one or more of the base view classes we typically inherit from. It could even be a part of early view selection or even traversal; one could imagine that the last step of traversal is performing that queryMultiAdapter(…,IAlternateData) if the Accept header is non-generic. But anything that's automatic strikes me as likely to be insufficiently flexible...