default_format :json not working

[bobek]

Hi,

I am playing with https://github.com/djones/grape-goliath-example . My current API class looks like:

class Posts < Grape::API
  version 'v1'
  format :json
  default_format :json

  resource 'user' do
    post "/create" do
      params.except('route_info')
    end
  end
end

The original version of example is using grape 0.2.2, where both call shown below work as expected. When updated the 0.4.1 (bundle update grape) the following call works:

curl -X POST -d '{"post":{"title":"David Jones","body":"this is my message"}}' http://localhost:9000/posts/create -H Content-Type:application/json

returning

{"post":{"title":"David Jones","body":"this is my message"}}

but when application/json is ommited

curl -X POST -d '{"post":{"title":"David Jones","body":"this is my message"}}' http://localhost:9000/posts/create

the call returns

{"{\"post\":{\"title\":\"David Jones\",\"body\":\"this is my message\"}}":null}

[dblock]

This is actually not a bug, but it did take a me a while to figure out what's going on.

You're posting data to the server. Since you don't specify that the data is JSON, Grape doesn't parse it. If you say Content-type: application/json, Grape selects the json parser, otherwise it does nothing.

So what's params when you specify the content-type?

#<Hashie::Mash post=#<Hashie::Mash body="this is my message" title="David Jones"> version="v1">

and otherwise?

#<Hashie::Mash version="v1" {"post":{"title":"David Jones","body":"this is my message"}}=nil>

The key is the body, the value is nil. Not very useful. The JSON of that is the same:

{"{\"post\":{\"title\":\"David Jones\",\"body\":\"this is my message\"}}"=>nil, "version"=>"v1"}

That's what's returned from the API.

I am happy to discuss whether default_format should also mean the default parser format. It seems that it could make sense.

[dblock]

I convinced myself that the most logical thing to do would be to apply default_format to the parser as well (which is exactly how you expected it to work). Fixed on HEAD.

[csoreff]

Still behaving like this for me...format and default_format are :json yet this same exact thing is still happening without Content-Type.

curl --include \
     --request POST \
   --data-binary "{
    \"email\": \"test@test.com\",
    \"password\": \"password\"
}" \
'localhost:3000/api/v1/login'

results in:

#<Hashie::Mash {
    "email": "test@test.com",
    "password": "password"
}=nil>

Looks like the format to parse with is being set to whatever the request's media type is, which is defaulting to "application/x-www-form-urlencoded"

request.media_type
"application/x-www-form-urlencoded"

from grape/lib/grape/middleware/formatter.rb #read_rack_input

fmt = mime_types[request.media_type] if request.media_type
fmt ||= options[:default_format]

So it never sets it to options[:default_format]

[dblock]

Try to (re)write a spec like this @csoreff and PR it? Maybe with a fix?

[csoreff]

The test doesn't handle when content type isn't supplied at all, it's given as a blank string:

    it 'parses data in default format' do
      subject.post '/data' do
        { x: params[:x] }
      end
      post '/data', '{"x":42}', 'CONTENT_TYPE' => ''
      expect(last_response.status).to eq(201)
      expect(last_response.body).to eq('{"x":42}')
    end

When I remove that content type and post without it, it breaks without even making it to #read_rack_input and test fails.

These are both false in #read_body_input so it returns here:

(!request.form_data? || !request.media_type)

request.media_type defaulted to "application/x-www-form-urlencoded", same as my earlier example, a default setting from Rails I believe.

I'll look at it more later. Not sure off the top of my head on how to solve this on Grape's side, since at this point we can't really differentiate between a defaulted media type and if that was actually the type given for form data (unless there's a way to check the exact original request?).

[dblock]

You should reopen this as a new issue.

[dblock]

Re-reading this, POST with a default content type is not a thing for Rack, it won't parse input as you can see. However, Grape could apply default_format to the parser IMO, so JSON should work without a content-type I think.

[csoreff]

Grape can't apply default_format because it doesn't even make it to the point where it does so.

[dblock]

@csoreff We could make it get there, right? ;)

[csoreff]

Oh haha yea, I suppose I read that wrong. :P