jsonapi-rb / jsonapi-rails

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

If no JSON API payload was found should render JSON API compliant error #80

Open JoeWoodward opened 6 years ago

JoeWoodward commented 6 years ago

Currently have strong params in place and when a request comes through without a compliant structure I end up raising ActionController::ParameterMissing because my strong params doesn't have anything to work from.

How should I be handling this? Should the gem maybe handle this?

JoeWoodward commented 6 years ago

For now I'm monkey patching and rendering a jsonapi_error when the parsing fails. Not sure if this is the correct thing to do. The docs state that we MAY return 403 and can include errors.

# frozen_string_literal: false
# Override the default failure behaviour.
# With this patch if hash.nil? the controller will render jsonapi_errors
require 'jsonapi/rails/controller'

module Extensions
  module JSONAPI
    module Rails
      module Controller
        module Deserialization
          extend ActiveSupport::Concern

          class_methods do
            def deserializable_resource(key, options = {}, &block)
              options = options.dup
              klass = options.delete(:class) ||
                      Class.new(::JSONAPI::Rails::DeserializableResource, &block)

              before_action(options) do |controller|
                hash = controller.params.to_unsafe_hash[:_jsonapi]
                if hash.nil?
                  ::JSONAPI::Rails.logger.warn do
                    "Unable to deserialize #{key} because no JSON API payload w" \
                    "as found. (#{controller.controller_name}##{params[:action]})"
                  end
                  controller.render jsonapi_errors: {
                    title: 'Non-compliant Request Body',
                    detail: 'The request was not formatted in compliance with ' \
                      'the application/vnd.api+json spec',
                    links: { about: 'http://jsonapi.org/format/' }
                  }, status: 403
                  next
                end

                ActiveSupport::Notifications
                  .instrument('parse.jsonapi-rails',
                              key: key, payload: hash, class: klass) do
                  ::JSONAPI::Parser::Resource.parse!(hash)
                  resource = klass.new(hash[:data])
                  controller.request.env[JSONAPI_POINTERS_KEY] =
                    resource.reverse_mapping
                  controller.params[key.to_sym] = resource.to_hash
                end
              end
            end
          end
        end
      end
    end
  end
end

JSONAPI::Rails::Controller.include(
  Extensions::JSONAPI::Rails::Controller::Deserialization
)

I feel like this should probably be configurable though

JoeWoodward commented 6 years ago

Before rendering jsonapi_errors should also check if the current Content-Type is actually application/vnd.api+json

beauby commented 6 years ago

Agreed that the lib should at least raise a custom exception so that people can rescue from it with a sensible json:api error. Would you mind issuing a PR for this?

veelenga commented 6 years ago

@beauby any suggestion on how to add a status and links to the error response?


if event.save
  render jsonapi: event, status: :created
else
  render jsonapi_errors: event.errors, status: :unprocessable_entity
end
{
    "errors": [
        {
            "title": "Invalid user",
            "detail": "user must exist",
            "source": {}
        }
    ],
    "jsonapi": {
        "version": "1.0"
    }
}

status and links are the attributes required by JSON API Spec.

JoeWoodward commented 6 years ago

Didn't see this response. I will try to come up with an elegant solution

JoeWoodward commented 6 years ago

Maybe rather than using an exception to handle the flow control the controller should have a customizable error response.

i.e.

In the config file you can set the default error response hash for this situation config[:jsonapi_payload_error_hash]

Then we add a hook to the controller that would call controller.jsonapi_invalid_payload_error

Which could look something like

def jsonapi_payload_error
  render_error_for_action = "jsonapi_payload_error_for_#{controller.action_name}"
  return public_send(render_error_for_action) if controller.responds_to?(render_error_for_action)
  render jsonapi_errors: JSONAPI::Rails.config[:jsonapi_payload_error_hash]
end

What do you think @beauby

JoeWoodward commented 6 years ago

@veelenga your question is related to this https://github.com/jsonapi-rb/jsonapi-rails/issues/86, it's not related to the issue outlined here.

However, one note is that none of the errors attributes are actually required, they are optional

An error object MAY have the following members:

That said I think there should be a way to map these values. I will try to conceive a means of doing so