Collapse JSON API success/failure documents in one adapter

Idea per remear (Ben Mills) in the slack:
https://amserializers.slack.com/archives/general/p1455140474000171

remear:

    just so i understand, the adapter in `render json: resource, status: 422, adapter: 'json_api/error',
    serializer: ActiveModel::Serializer::ErrorSerializer` is a different one than, say what i’ve
    specified in a base serializer with `ActiveModel::Serializer.config.adapter = :json_api`. correct?

    and a followup question of, why not same adapter but different serializer?

me:

   With the way the code is written now, it might be possible to not require a special jsonapi adapter.
   However, the behavior is pretty different from the jsonapi adapter.

   this first draft of the PR had it automatically set the adapter if there were errors.  since that
   requires more discussion, I took a step back and made it explicit for this PR

   If I were to re-use the json api adapter and remove the error one, it think the serializable hash
   method would look like

   ```
   def serializable_hash(options = nil)
     return { errors: JsonApi::Error.collection_errors } if serializer.is_a?(ErrorsSerializer)
     return { errors: JsonApi::Error.resource_errors(serializer) } if serializer.is_a?(ErrorSerializer)
     options ||= {}
   ```

   I suppose it could be something more duckish like

   ```
   def serializable_hash(options = nil)
     if serializer.errors? # object.errors.any? || object.any? {|o| o.errors.any? }
       JsonApi::Error.new(serializer).serializable_hash
     else
       # etc
   ```
This commit is contained in:
Benjamin Fleischer 2016-02-11 00:14:54 -06:00
parent 96107c56aa
commit 3d986377b6
11 changed files with 60 additions and 44 deletions

View File

@ -2,9 +2,8 @@
# JSON API Errors # JSON API Errors
Rendering error documents requires specifying the serializer and the adapter: Rendering error documents requires specifying the error serializer(s):
- `adapter: :'json_api/error'`
- Serializer: - Serializer:
- For a single resource: `serializer: ActiveModel::Serializer::ErrorSerializer`. - For a single resource: `serializer: ActiveModel::Serializer::ErrorSerializer`.
- For a collection: `serializer: ActiveModel::Serializer::ErrorsSerializer`, `each_serializer: ActiveModel::Serializer::ErrorSerializer`. - For a collection: `serializer: ActiveModel::Serializer::ErrorsSerializer`, `each_serializer: ActiveModel::Serializer::ErrorSerializer`.
@ -23,7 +22,7 @@ resource.errors.add(:name, 'cannot be nil')
resource.errors.add(:name, 'must be longer') resource.errors.add(:name, 'must be longer')
resource.errors.add(:id, 'must be a uuid') resource.errors.add(:id, 'must be a uuid')
render json: resource, status: 422, adapter: 'json_api/error', serializer: ActiveModel::Serializer::ErrorSerializer render json: resource, status: 422, adapter: :json_api, serializer: ActiveModel::Serializer::ErrorSerializer
# #=> # #=>
# { :errors => # { :errors =>
# [ # [
@ -44,7 +43,7 @@ resource.errors.add(:name, 'must be awesome')
serializable_resource = ActiveModel::SerializableResource.new( serializable_resource = ActiveModel::SerializableResource.new(
resource, { resource, {
serializer: ActiveModel::Serializer::ErrorSerializer, serializer: ActiveModel::Serializer::ErrorSerializer,
adapter: 'json_api/error' adapter: :json_api
}) })
serializable_resource.as_json(options) serializable_resource.as_json(options)
# #=> # #=>

View File

@ -118,6 +118,10 @@ module ActiveModel
end end
end end
def success?
true
end
# Used by adapter as resource root. # Used by adapter as resource root.
def json_key def json_key
root || object.class.model_name.to_s.underscore root || object.class.model_name.to_s.underscore

View File

@ -22,6 +22,10 @@ module ActiveModel
end end
end end
def success?
true
end
def json_key def json_key
root || derived_root root || derived_root
end end

View File

@ -3,4 +3,8 @@ class ActiveModel::Serializer::ErrorSerializer < ActiveModel::Serializer
def as_json def as_json
object.errors.messages object.errors.messages
end end
def success?
false
end
end end

View File

@ -1,5 +1,5 @@
require 'active_model/serializer/error_serializer' require 'active_model/serializer/error_serializer'
class ActiveModel::Serializer::ErrorsSerializer < ActiveModel::Serializer class ActiveModel::Serializer::ErrorsSerializer
include Enumerable include Enumerable
delegate :each, to: :@serializers delegate :each, to: :@serializers
attr_reader :object, :root attr_reader :object, :root
@ -13,6 +13,10 @@ class ActiveModel::Serializer::ErrorsSerializer < ActiveModel::Serializer
end end
end end
def success?
false
end
def json_key def json_key
nil nil
end end

View File

@ -53,7 +53,14 @@ module ActiveModelSerializers
def serializable_hash(options = nil) def serializable_hash(options = nil)
options ||= {} options ||= {}
if serializer.success?
success_document(options)
else
failure_document
end
end
def success_document(options)
is_collection = serializer.respond_to?(:each) is_collection = serializer.respond_to?(:each)
serializers = is_collection ? serializer : [serializer] serializers = is_collection ? serializer : [serializer]
primary_data, included = resource_objects_for(serializers) primary_data, included = resource_objects_for(serializers)
@ -77,6 +84,28 @@ module ActiveModelSerializers
hash hash
end end
# TODO: look into caching
# rubocop:disable Style/AsciiComments
# definition:
# ☑ toplevel_errors array (required)
# ☐ toplevel_meta
# ☐ toplevel_jsonapi
# rubocop:enable Style/AsciiComments
def failure_document
hash = {}
# PR Please :)
# ApiObjects::Jsonapi.add!(hash)
if serializer.respond_to?(:each)
hash[:errors] = serializer.flat_map do |error_serializer|
Error.resource_errors(error_serializer)
end
else
hash[:errors] = Error.resource_errors(serializer)
end
hash
end
def fragment_cache(cached_hash, non_cached_hash) def fragment_cache(cached_hash, non_cached_hash)
root = false if instance_options.include?(:include) root = false if instance_options.include?(:include)
ActiveModelSerializers::Adapter::JsonApi::FragmentCache.new.fragment_cache(root, cached_hash, non_cached_hash) ActiveModelSerializers::Adapter::JsonApi::FragmentCache.new.fragment_cache(root, cached_hash, non_cached_hash)

View File

@ -1,29 +1,13 @@
module ActiveModelSerializers module ActiveModelSerializers
module Adapter module Adapter
class JsonApi < Base class JsonApi < Base
class Error < Base module Error
UnknownSourceTypeError = Class.new(ArgumentError)
# rubocop:disable Style/AsciiComments # rubocop:disable Style/AsciiComments
# TODO: look into caching UnknownSourceTypeError = Class.new(ArgumentError)
# definition:
# ☐ toplevel_errors array (required)
# ☑ toplevel_meta
# ☑ toplevel_jsonapi
def serializable_hash(*)
hash = {}
# PR Please :)
# Jsonapi.add!(hash)
# Checking object since we're not using an ArraySerializer
if serializer.object.respond_to?(:each)
hash[:errors] = collection_errors
else
hash[:errors] = Error.resource_errors(serializer)
end
hash
end
# Builds a JSON API Errors Object
# {http://jsonapi.org/format/#errors JSON API Errors}
#
# @param [ActiveModel::Serializer::ErrorSerializer] # @param [ActiveModel::Serializer::ErrorSerializer]
# @return [Array<Symbol, Array<String>] i.e. attribute_name, [attribute_errors] # @return [Array<Symbol, Array<String>] i.e. attribute_name, [attribute_errors]
def self.resource_errors(error_serializer) def self.resource_errors(error_serializer)
@ -86,16 +70,6 @@ module ActiveModelSerializers
fail UnknownSourceTypeError, "Unknown source type '#{source_type}' for attribute_name '#{attribute_name}'" fail UnknownSourceTypeError, "Unknown source type '#{source_type}' for attribute_name '#{attribute_name}'"
end end
end end
private
# @return [Array<#object_errors>]
def collection_errors
serializer.flat_map do |error_serializer|
Error.resource_errors(error_serializer)
end
end
# rubocop:enable Style/AsciiComments # rubocop:enable Style/AsciiComments
end end
end end

View File

@ -30,7 +30,7 @@ module ActionController
resource.errors.add(:name, 'cannot be nil') resource.errors.add(:name, 'cannot be nil')
resource.errors.add(:name, 'must be longer') resource.errors.add(:name, 'must be longer')
resource.errors.add(:id, 'must be a uuid') resource.errors.add(:id, 'must be a uuid')
render json: resource, adapter: 'json_api/error', serializer: ActiveModel::Serializer::ErrorSerializer render json: resource, adapter: :json_api, serializer: ActiveModel::Serializer::ErrorSerializer
end end
end end

View File

@ -102,8 +102,7 @@ module ActiveModelSerializers
'null'.freeze => ActiveModelSerializers::Adapter::Null, 'null'.freeze => ActiveModelSerializers::Adapter::Null,
'json'.freeze => ActiveModelSerializers::Adapter::Json, 'json'.freeze => ActiveModelSerializers::Adapter::Json,
'attributes'.freeze => ActiveModelSerializers::Adapter::Attributes, 'attributes'.freeze => ActiveModelSerializers::Adapter::Attributes,
'json_api'.freeze => ActiveModelSerializers::Adapter::JsonApi, 'json_api'.freeze => ActiveModelSerializers::Adapter::JsonApi
'json_api/error'.freeze => ActiveModelSerializers::Adapter::JsonApi::Error
} }
actual = ActiveModelSerializers::Adapter.adapter_map actual = ActiveModelSerializers::Adapter.adapter_map
assert_equal actual, expected_adapter_map assert_equal actual, expected_adapter_map
@ -114,7 +113,6 @@ module ActiveModelSerializers
'attributes'.freeze, 'attributes'.freeze,
'json'.freeze, 'json'.freeze,
'json_api'.freeze, 'json_api'.freeze,
'json_api/error'.freeze,
'null'.freeze 'null'.freeze
] ]
end end

View File

@ -13,7 +13,7 @@ module ActiveModelSerializers
def test_active_model_with_error def test_active_model_with_error
options = { options = {
serializer: ActiveModel::Serializer::ErrorSerializer, serializer: ActiveModel::Serializer::ErrorSerializer,
adapter: :'json_api/error' adapter: :json_api
} }
@resource.errors.add(:name, 'cannot be nil') @resource.errors.add(:name, 'cannot be nil')
@ -37,7 +37,7 @@ module ActiveModelSerializers
def test_active_model_with_multiple_errors def test_active_model_with_multiple_errors
options = { options = {
serializer: ActiveModel::Serializer::ErrorSerializer, serializer: ActiveModel::Serializer::ErrorSerializer,
adapter: :'json_api/error' adapter: :json_api
} }
@resource.errors.add(:name, 'cannot be nil') @resource.errors.add(:name, 'cannot be nil')

View File

@ -40,7 +40,7 @@ module ActiveModel
serializable_resource = ActiveModel::SerializableResource.new( serializable_resource = ActiveModel::SerializableResource.new(
resource, { resource, {
serializer: ActiveModel::Serializer::ErrorSerializer, serializer: ActiveModel::Serializer::ErrorSerializer,
adapter: 'json_api/error' adapter: :json_api
}) })
expected_response_document = expected_response_document =
{ :errors => { :errors =>
@ -61,7 +61,7 @@ module ActiveModel
resources, { resources, {
serializer: ActiveModel::Serializer::ErrorsSerializer, serializer: ActiveModel::Serializer::ErrorsSerializer,
each_serializer: ActiveModel::Serializer::ErrorSerializer, each_serializer: ActiveModel::Serializer::ErrorSerializer,
adapter: 'json_api/error' adapter: :json_api
}) })
expected_response_document = expected_response_document =
{ :errors => { :errors =>