Don't use .format suffix in paths if you only have one format

[ajvondrak]

For example, say you have the following API:

require 'grape'

class API < Grape::API
  format :json

  get :hello do
    { hello: 'world' }
  end
end

run API

If you hit /hello, you'd get back the JSON {"hello":"world"}. This makes sense: you specifically say that your format is JSON.

However, before this commit, you could also hit /hello.xml, which would confusingly return the JSON {"hello":"world"}. Why bother exposing this path as a valid API endpoint when we're just going to ignore the format anyway?

In this commit, /hello.xml (or /hello.json or /hello.foobar) will all be 404s. We don't even entertain the notion of an optional format path parameter.

Old behavior is still preserved if you add different content types after your format :xxx declaration. So, if you have the following API:

require 'grape'

class API < Grape::API
  format :json
  content_type :xml, 'application/xml'

  get :hello do
    { hello: 'world' }
  end
end

run API

then /hello, /hello.json, /hello.xml, and /hello.foobar all return the JSON {"hello":"world"}. I don't know why you'd want this behavior either, but it was being enforced by the specs, so I preserved it.

What do you think?

[dblock]

I like this. It needs a CHANGELOG entry and possibly a README update around format selection.

[ajvondrak]

Updated the docs. Do they all seem right?

[dblock]

Before I merge this...

Don't you think it's a little strange that 1 format = no automatic suffix, multiple formats = automatic suffix? What if default_format made that selection?

[ajvondrak]

So if I understand you right...

class MyAPI < Grape::API
  default_format :json
  get :hello do { hello: 'world' } end
end

As it is: /hello returns JSON (that's the default format), /hello.xml returns XML (that's still a supported format), /hello.xls returns JSON (XLS is unsupported, JSON is the default).

Are you saying that if default_format made that selection: /hello returns JSON, /hello.xml is a 404, /hello.xls is a 404...?

It makes sense to me that 1 format = no automatic suffix, because there's nothing the suffix would do - you have no choice but to have the API return JSON (short of env['api.format'] mangling and such). Unless I'm misunderstanding what format :json is supposed to do. The way I understand, it's saying "this is the one and only format for the whole API". Whereas default_format is saying "we have a bunch of formats available, but fall back to this one".

[dblock]

Ok, I completely agree with you, thanks. Merging.

[dblock]

@ajvondrak: This needs an entry to UPGRADING, please. Before you had /foobar.json working and that now has become /foobar whereas .json is going to return a 404.

[ajvondrak]

@dblock Done.

[dblock]

Can you make a new PR for that please?

[ur5us]

I just tried to upgrade one of our apps from Grape 0.9.0 to ~> 0.10.0. It took me hours to debug the issue as to why .json in the URL wouldn't work, I simply missed how important this change was. Also, this change breaks Swagger UI as described in https://github.com/tim-vandecasteele/grape-swagger/issues/187. I'm not sure whether it's just our installation of Swagger UI but pointing the Swagger UI demo at our own API didn't work either (probably that's outdated, too?). Moreover, using grape-swagger-rails is not really an option as it'd downgrade the grape-swagger gem.

Changing format :json to

content_type :json, 'application/json'
default_format :json

actually fixes the problem. Now, with that change the following URLs will work:

/users.json
/users

and both will return the same. To be honest, that's what I'd expect when using format :json. It should actually respond to both URLs, with and without the suffix. Moreover, for any other format I'd actually expect a 406 Not acceptable header instead of an JSON response. So in my opinion this change goes too far for the whole suffix discussion and not far enough when it comes to asking for a response format which is actually not supported.

[dblock]

I had a similar sentiment a few times. I would be open to undoing this change carefully as you describe, if you want to try a PR.

[ur5us]

Alright, though I'll probably aim for several PRs as a lot of the things I've described are actually different/non-related things.