env['api.format'] does not override formatter set by content-type

[waynerobinson]

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.

[waynerobinson]

It looks like the value of the content type overrides the format. See

grape/lib/grape/middleware/formatter.rb

Lines 55 to 58 in 02d7113

	def fetch_formatter(headers, options)
	api_format = mime_types[headers[Grape::Http::Headers::CONTENT_TYPE]] || env[Grape::Env::API_FORMAT]
	Grape::Formatter.formatter_for(api_format, **options)
	end

I'm not sure what the safest way is to reverse this? Could it literally just be reversed?

[waynerobinson]

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.

grape/spec/grape/api_spec.rb

Lines 3732 to 3739 in 7741f02

	it 'can be overwritten with an explicit content type' do
	subject.get '/meaning_of_life_with_content_type' do
	content_type 'text/plain'
	{ meaning_of_life: 42 }.to_s
	end
	get '/meaning_of_life_with_content_type'
	expect(last_response.body).to eq({ meaning_of_life: 42 }.to_s)
	end

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.

[dblock]

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?

[iabdulin]

here's my use case:
I build JSON in the database and want to return it as is, no additional formatting is needed

[waynerobinson]

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. 👍