grape
grape copied to clipboard
env['api.format'] does not override formatter set by content-type
In reference to #1989 (sorry, I have commented in there before I realised the issue was closed).
I'm having exactly the same issue.
If this is used:
env['api.format'] = :binary # or :txt
content_type 'application/json' # or header 'Content-Type', 'application/json'
{key: 123}.to_json
Then it still calls the JSON formatter. If the content_type method is left off, it doesn't.
Here is an example using grape-on-rack:
https://github.com/waynerobinson/grape-on-rack
Please take a look at the extra methods added to Acme::Ping.
The only one that doesn't call the JSON formatter is the one where the content type is set to text/plain.
It looks like the value of the content type overrides the format. See https://github.com/ruby-grape/grape/blob/02d7113d09eb9fcb4264c841d1fdd305e3e8adb5/lib/grape/middleware/formatter.rb#L55-L58
I'm not sure what the safest way is to reverse this? Could it literally just be reversed?
If this test shows the intended behaviour, I don't think we'll be able to override the output by just changing the order the api_format is looked up in.
https://github.com/ruby-grape/grape/blob/7741f022b5928f0ddcee27de08555d95ea4556f8/spec/grape/api_spec.rb#L3732-L3739
If we did, it would break the behaviour where there is an explicit format set, but the implementation expects to be able to override it by changing the content type.
For example:
format :json
def action
content_type "text/plain"
"plain text"
end
would still output as JSON if we reversed the lookup to api_format = env[Grape::Env::API_FORMAT] || mime_types[headers[Grape::Http::Headers::CONTENT_TYPE]] because it would always pull out :json first.
I don't think there's any non-breaking way to override the behaviour of one of the content-type associated formatters at the moment on a per-request basis.
So it looks like a bug because we explicitly document You can override this process (content negotiation) explicitly by specifying env['api.format'] in the API itself. Since you already have a bunch of examples, turn them into specs in Grape? We can discuss how to fix it, flipping the order on that line should be a start. And it does look like it would be breaking either way, so maybe we should remove support for env['api.format'] that doesn't work, and find another way to do it.
Related, what's a real use-case where you want to do this? Is it that you want to produce actual JSON and leave the formatter alone? Why not rely on the formatter to do this work?
here's my use case: I build JSON in the database and want to return it as is, no additional formatting is needed
Yeah. Our use case is similar. We have cached data that's already JSON that we'd like to just return vs having to parse and re-encode.
I'll take a look at starting a PR tomorrow with some options. 👍