jsonapi-rb / jsonapi-rails

Rails gem for fast jsonapi-compliant APIs.
http://jsonapi-rb.org
MIT License
319 stars 63 forks source link

Parse standard JSON API request instead of requiring _jsonapi key #79

Closed TylerRick closed 2 years ago

TylerRick commented 6 years ago

Started looking into using this project in my Rails app but right out of the gate I started getting

NoMethodError (undefined method `to_unsafe_hash' for nil:NilClass)

errors with even the simplest test request made to my create action.

Tracked it down to this source code:

          before_action(options) do |controller| 
            # TODO(lucas): Fail with helpful error message if _jsonapi not 
            #   present. 
            hash = controller.params[:_jsonapi].to_unsafe_hash 

Indeed failing with a helpful error message would be better than a Ruby exception, but why is it requiring a _jsonapi key in the params in the first place?

It seems rather non-standard (I don't see mention of a _jsonapi key in the http://jsonapi.org/ spec) and surprising and I don't see anything about it in the Readme nor on http://jsonapi-rb.org/guides/getting_started/rails.html guide either nor http://jsonapi-rb.org/guides/deserialization/deserializing.html.

Why does it expect the payload to be wrapped with this extra key? And how are such payloads expected to be generated? Maybe I'm just missing something but it would be nice if the getting started code was a little more complete and included an example of producing a payload in addition to consuming it (not that I plan on using this library to produce the payload—planning to use an Angular service to make the request—but it would at least serve as an example). The example shows a create action but doesn't show how you would make a request that it can understand.

What I would expect instead

I would expect that a JSON payload formatted according to the JSON API spec would be parsed without error, so that I could use this in the backend with any frontend framework that speaks JSON API.

Example of a valid payload for creating a resource (from http://jsonapi.org/format/#crud-creating):

POST /photos HTTP/1.1
Content-Type: application/vnd.api+json
Accept: application/vnd.api+json

{
  "data": {
    "type": "photos",
    "attributes": {
      "title": "Ember Hamster",
      "src": "http://example.com/images/productivity.png"
    },
    "relationships": {
      "photographer": {
        "data": { "type": "people", "id": "9" }
      }
    }
  }
}

My test example

class TestRecordsController < ApplicationController
  deserializable_resource :test_record, only: [:create]

  def create
    test_record = TestRecord.new(test_record_params)
    ... # doesn't even get this far
  end

  def test_record_params
    params.require(:test_record).permit(
      :counter
    )
  end
end

Using versions:

    jsonapi-rails (0.3.1)
      jsonapi-parser (~> 0.1.0)
      jsonapi-rb (~> 0.5.0)
    rails (5.1.4)  
JoeWoodward commented 6 years ago

+1, running into the same issue.

JoeWoodward commented 6 years ago

Hey @TylerRick I found out what our issue is. In https://github.com/jsonapi-rb/jsonapi-rails/blob/master/lib/jsonapi/rails/railtie.rb#L13 you can see the parser is inserting the data object into a _jsonapi object. You need to specify the correct MIME type. You will also need to register the MIME type with rails

# config/initializers/register_mime_types.rb
api_mime_types = %W(
  application/vnd.api+json
  text/x-json
  application/json
)
Mime::Type.register 'application/vnd.api+json', :json, api_mime_types

EDIT: You actually don't need to register the mime type. The railtie already does this

Then in all your requests you'll need to specify Content-Type : application/vnd.api+json in the headers

You probably want to set this up for your specs too. For RSpec I created a file spec/support/api_default_headers.rb

# frozen_string_literal: true
module ApiDefaultHeaders
  extend ActiveSupport::Concern

  HTTP_METHODS = %w(get post put delete patch).freeze

  included do
    # make requests xhr requests for all tests
    let(:default_headers) {
      {
        HTTP_ACCEPT: 'application/vnd.api+json',
        CONTENT_TYPE: 'application/vnd.api+json'
      }
    }

    HTTP_METHODS.each do |m|
      define_method(m) do |path, *args|
        args[0] ||= {}
        args[0][:headers] ||= {}
        args[0][:headers].merge!(default_headers)

        super(path, *args)
      end
    end
  end
end

Then in spec/rails_helper.rb you will need to register this. If you are building an app using rails new --api you can use this for all specs, otherwise I would use a RSpec meta tag and call it :api

# spec/rails_helper.rb
RSpec.configure do |config|
  # use application/vnd.api+json as the default headers
  config.include ApiDefaultHeaders #, api: true
  # to use in api specs...
  # RSpec.describe SomeController, type: request, api: true do
end

EDIT #2 You'll also need to send the params as json so build the hash and then call .to_json on it. I added this to the ApiDefaultHeaders module even though the name doesn't fit. You can refactor as you see fit..

# frozen_string_literal: true
module ApiDefaultHeaders
  extend ActiveSupport::Concern

  HTTP_METHODS = %w(get post put delete patch).freeze

  included do
    # make requests jsonapi compliant
    let(:default_headers) {
      {
        HTTP_ACCEPT: 'application/vnd.api+json',
        CONTENT_TYPE: 'application/vnd.api+json'
      }
    }

    HTTP_METHODS.each do |m|
      define_method(m) do |path, *args|
        args[0] ||= {}
        args[0][:headers] ||= {}
        args[0][:headers].merge!(default_headers)
        # convert hash to json if not already
        args[0][:params] ||= {}
        args[0][:params].to_json unless args[0][:params].is_a? String

        super(path, *args)
      end
    end
  end
end

Now you can write params as you normally would with a hash and the request should work correctly

beauby commented 6 years ago

@TylerRick Thanks for providing this clear writeup. What's happening is that the lib does not, indeed, expect the payload to be non standard, but the deserialization process does wrap it within a _jsonapi key, so that 1. the pristine payload is still accessible and 2. there is no name clash when we actually deserialize resources and make then available at the top level with an arbitrary name (c.f. deserializable_resource :foo).

TylerRick commented 6 years ago

Thanks for the response. I'm afraid I still don't understand but that's okay, I didn't end up using this library in my project since it was was taking too long to figure out. This may trip up other newcomers though, so it still may be worth improving (or clarifying in the docs).

The docs (http://jsonapi-rb.org/guides/deserialization/deserializing.html, for example) make it sound like the payload just needs to be at the top-level like params[:post] if you do deserializable_resource :post, but when I tried that, I got the undefined method 'to_unsafe_hash' for nil:NilClass error.

(I noticed that error was actually replaced with a more specific message in master (unreleased, at the time I tried it at least): Unable to deserialize #{key} because no JSON API payload was found.)

So again, it seems like deserializable_resource requires the payload to actually be under _jsonapi. Am I mistaken? And does that mean it (deserialization) is only intended to be used with JSON API requests that are serialized by this lib only — and not intended to be used with JSON API requests from just anywhere?

hugofloss commented 6 years ago

@TylerRick you're right and not mistaken. I'm running into the same issue when doing an external request, even when providing the application/vnd.api+json headers.

hugofloss commented 6 years ago

Okay, it seems that @JoeWoodward's solution for registering/overriding the :json mime type was actually the cause of my problem. That's probably because jsonapi-rails is already registering the mime type under the :jsonapi name. When I remove the code, I can successfully do external request again without explicitly adding the _jsonapi key. Of course you need to add the correct application/vnd.api+json headers to the request itself.

To fix the specs you don't have to do any changes in mime types or headers. Just do a normal request in :json format:

post :create, params: request_params, format: :json

and define request_params with the _jsonapi key:

let(:request_params) do
      {
        _jsonapi: {
          data: {
            type: 'foobar',
            attributes: {
              foo: 'bar'
            },
            relationships: {
              barfoo: {
                data: {
                  type: "barfoo",
                  id: "10"
                }
              }
            }
          }
        }
      }
    end

You don't have to call .to_json on request_params, the gem does that for you.

Unfortunately it doesn't help setting the correct Accept and Content-Type headers when running the specs, nor using the :jsonapi format when doing the create request.

I hope that solves your problem too @TylerRick.

ujh commented 6 years ago

The actual issue here is that controller test (I use rspec controller specs) are broken out of the box. I assumed that if I switched to using format: :jsonapi then it would work, but that's not the case. One could argue that people should just use request specs, but there should at least be a note somewhere that clarifies that standard controller specs won't work.

TylerRick commented 6 years ago

In my case, I was running into this in development, not in tests (though I might run into it there too).

I don't understand why it doesn't accept/deserialize requests from external (non-Rails) sources, in standard JSONAPI format, which doesn't require a _jsonapi key in the payload. Was trying to get an Angular app to talk with Rails using this gem (but gave up on that).

ujh commented 6 years ago

That is odd. Are you sure that you're correctly setting the content type of your requests? I only have the issue in controller tests. The actual application is working fine.

TylerRick commented 6 years ago

Thanks for the suggestion. I don't have the test app anymore so I can't test it, but that might have been the problem. I probably didn't do anything to set the content type (or it might have used a JSON content type, I can't remember).

jkeen commented 6 years ago

Looks like the _jsonapi key gets put in if the call was intercepted correctly by the mime type handler when using deserializable_resource. This gem registers a mime type of jsonapi and then uses a parameter parser to prepare the data,

I ran into problems because in thrashing with getting rails and jsonapi to work well together and in trying all the gems I had an stray initializer that was registering the mimetype application/vnd.api+json as :json, so the mime type handler that this gem uses wasn't getting called.

Startouf commented 5 years ago

I had this line previously in my initializer, but I just realized it broken again after updating one of our other gems, maybe this can still be helpful

ActionDispatch::Request.parameter_parsers[:jsonapi] =
  ActionDispatch::Request.parameter_parsers[:json]
caomania commented 4 years ago

As @Startouf points out, the parser, registered by the gem in the railtie can be overwritten in your initializer. Setting it to the default json parser prevents your params from being nested within an _jsonapi attribute.

# rails 5.x only!
ActionDispatch::Request.parameter_parsers[:jsonapi] = ActionDispatch::Request.parameter_parsers[:json]