Require explicit adapter/serializer to render JSON API errors

- Separate collection errors from resource errors in adapter
- Refactor to ErrorsSerializer; first-class json error methods
- DOCS
- Rails 4.0 requires assert exact exception class, boo

docs/README.md

@@ -14,7 +14,9 @@ This is the documentation of ActiveModelSerializers, it's focused on the **0.10.
 - [Caching](general/caching.md)
 - [Logging](general/logging.md)
 - [Instrumentation](general/instrumentation.md)
-- [JSON API Schema](jsonapi/schema.md)
+- JSON API
+  - [Schema](jsonapi/schema.md)
+  - [Errors](jsonapi/errors.md)
 - [ARCHITECTURE](ARCHITECTURE.md)
 
 ## How to

docs/jsonapi/errors.md

@@ -0,0 +1,57 @@
+[Back to Guides](../README.md)
+
+# JSON API Errors
+
+Rendering error documents requires specifying the serializer and the adapter:
+
+- `adapter: :'json_api/error'`
+- Serializer:
+  - For a single resource: `serializer: ActiveModel::Serializer::ErrorSerializer`.
+  - For a collection: `serializer: ActiveModel::Serializer::ErrorsSerializer`, `each_serializer: ActiveModel::Serializer::ErrorSerializer`.
+
+The resource **MUST** have a non-empty associated `#errors` object.
+The `errors` object must have a `#messages` method that returns a hash of error name to array of
+descriptions.
+
+## Use in controllers
+
+```ruby
+resource = Profile.new(name: 'Name 1',
+                       description: 'Description 1',
+                       comments: 'Comments 1')
+resource.errors.add(:name, 'cannot be nil')
+resource.errors.add(:name, 'must be longer')
+resource.errors.add(:id, 'must be a uuid')
+
+render json: resource, status: 422, adapter: 'json_api/error', serializer: ActiveModel::Serializer::ErrorSerializer
+# #=>
+#  { :errors =>
+#    [
+#      { :source => { :pointer => '/data/attributes/name' }, :detail => 'cannot be nil' },
+#      { :source => { :pointer => '/data/attributes/name' }, :detail => 'must be longer' },
+#      { :source => { :pointer => '/data/attributes/id' }, :detail => 'must be a uuid' }
+#    ]
+#  }.to_json
+```
+
+## Direct error document generation
+
+```ruby
+options = nil
+resource = ModelWithErrors.new
+resource.errors.add(:name, 'must be awesome')
+
+serializable_resource = ActiveModel::SerializableResource.new(
+  resource, {
+    serializer: ActiveModel::Serializer::ErrorSerializer,
+    adapter: 'json_api/error'
+  })
+serializable_resource.as_json(options)
+# #=>
+# {
+#   :errors =>
+#     [
+#       { :source => { :pointer => '/data/attributes/name' }, :detail => 'must be awesome' }
+#     ]
+# }
+```

lib/active_model/serializable_resource.rb

@@ -18,14 +18,6 @@ module ActiveModel
         options.partition { |k, _| ADAPTER_OPTION_KEYS.include? k }.map { |h| Hash[h] }
     end
 
-    def errors?
-      if resource.respond_to?(:each)
-        resource.any? { |elem| elem.respond_to?(:errors) && !elem.errors.empty? }
-      else
-        resource.respond_to?(:errors) && !resource.errors.empty?
-      end
-    end
-
     def serialization_scope=(scope)
       serializer_opts[:scope] = scope
     end
@@ -39,11 +31,7 @@ module ActiveModel
     end
 
     def adapter
-      @adapter ||=
-        begin
-          adapter_opts[:adapter] = :'json_api/error' if errors?
-          ActiveModelSerializers::Adapter.create(serializer_instance, adapter_opts)
-        end
+      @adapter ||= ActiveModelSerializers::Adapter.create(serializer_instance, adapter_opts)
     end
     alias_method :adapter_instance, :adapter
 
@@ -58,7 +46,6 @@ module ActiveModel
       @serializer ||=
         begin
           @serializer = serializer_opts.delete(:serializer)
-          @serializer = ActiveModel::Serializer::ErrorSerializer if errors?
           @serializer ||= ActiveModel::Serializer.serializer_for(resource)
 
           if serializer_opts.key?(:each_serializer)

lib/active_model/serializer.rb

@@ -2,6 +2,7 @@ require 'thread_safe'
 require 'active_model/serializer/collection_serializer'
 require 'active_model/serializer/array_serializer'
 require 'active_model/serializer/error_serializer'
+require 'active_model/serializer/errors_serializer'
 require 'active_model/serializer/include_tree'
 require 'active_model/serializer/associations'
 require 'active_model/serializer/attributes'

lib/active_model/serializer/error_serializer.rb

@@ -1,2 +1,6 @@
 class ActiveModel::Serializer::ErrorSerializer < ActiveModel::Serializer
+  # @return [Hash<field_name,Array<error_message>>]
+  def as_json
+    object.errors.messages
+  end
 end

lib/active_model/serializer/errors_serializer.rb

@@ -0,0 +1,23 @@
+require 'active_model/serializer/error_serializer'
+class ActiveModel::Serializer::ErrorsSerializer < ActiveModel::Serializer
+  include Enumerable
+  delegate :each, to: :@serializers
+  attr_reader :object, :root
+
+  def initialize(resources, options = {})
+    @root = options[:root]
+    @object = resources
+    @serializers = resources.map do |resource|
+      serializer_class = options.fetch(:serializer) { ActiveModel::Serializer::ErrorSerializer }
+      serializer_class.new(resource, options.except(:serializer))
+    end
+  end
+
+  def json_key
+    nil
+  end
+
+  protected
+
+  attr_reader :serializers
+end

lib/active_model_serializers/adapter/json_api.rb

@@ -13,7 +13,7 @@ module ActiveModelSerializers
       # TODO: if we like this abstraction and other API objects to it,
       # then extract to its own file and require it.
       module ApiObjects
-        module JsonApi
+        module Jsonapi
           ActiveModelSerializers.config.jsonapi_version = '1.0'
           ActiveModelSerializers.config.jsonapi_toplevel_meta = {}
           # Make JSON API top-level jsonapi member opt-in
@@ -62,7 +62,7 @@ module ActiveModelSerializers
         hash[:data] = is_collection ? primary_data : primary_data[0]
         hash[:included] = included if included.any?
 
-        ApiObjects::JsonApi.add!(hash)
+        ApiObjects::Jsonapi.add!(hash)
 
         if instance_options[:links]
           hash[:links] ||= {}

lib/active_model_serializers/adapter/json_api/error.rb

@@ -2,90 +2,101 @@ module ActiveModelSerializers
   module Adapter
     class JsonApi < Base
       class Error < Base
-=begin
-## http://jsonapi.org/format/#document-top-level
+        UnknownSourceTypeError = Class.new(ArgumentError)
+        # rubocop:disable Style/AsciiComments
+        # TODO: look into caching
 
-A document MUST contain at least one of the following top-level members:
+        # definition:
+        #   ☐ toplevel_errors array (required)
+        #   ☑ toplevel_meta
+        #   ☑ toplevel_jsonapi
+        def serializable_hash(*)
+          hash = {}
+          # PR Please :)
+          # Jsonapi.add!(hash)
 
-- data: the document's "primary data"
-- errors: an array of error objects
-- meta: a meta object that contains non-standard meta-information.
+          # 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
 
-The members data and errors MUST NOT coexist in the same document.
+        # @param [ActiveModel::Serializer::ErrorSerializer]
+        # @return [Array<Symbol, Array<String>] i.e. attribute_name, [attribute_errors]
+        def self.resource_errors(error_serializer)
+          error_serializer.as_json.flat_map do |attribute_name, attribute_errors|
+            attribute_error_objects(attribute_name, attribute_errors)
+          end
+        end
 
-## http://jsonapi.org/format/#error-objects
-
-Error objects provide additional information about problems encountered while performing an operation. Error objects MUST be returned as an array keyed by errors in the top level of a JSON API document.
-
-An error object MAY have the following members:
-
-- id: a unique identifier for this particular occurrence of the problem.
-- links: a links object containing the following members:
-- about: a link that leads to further details about this particular occurrence of the problem.
-- status: the HTTP status code applicable to this problem, expressed as a string value.
-- code: an application-specific error code, expressed as a string value.
-- title: a short, human-readable summary of the problem that SHOULD NOT change from occurrence to occurrence of the problem, except for purposes of localization.
-- detail: a human-readable explanation specific to this occurrence of the problem.
-- source: an object containing references to the source of the error, optionally including any of the following members:
--  pointer: a JSON Pointer [RFC6901] to the associated entity in the request document [e.g. "/data" for a primary data object, or "/data/attributes/title" for a specific attribute].
--  parameter: a string indicating which query parameter caused the error.
-- meta: a meta object containing non-standard meta-information about the error.
-
-=end
-        def self.attributes(attribute_name, attribute_errors)
+        # definition:
+        #   JSON Object
+        #
+        # properties:
+        #   ☐ id      : String
+        #   ☐ status  : String
+        #   ☐ code    : String
+        #   ☐ title   : String
+        #   ☑ detail  : String
+        #   ☐ links
+        #   ☐ meta
+        #   ☑ error_source
+        #
+        # description:
+        #   id     : A unique identifier for this particular occurrence of the problem.
+        #   status : The HTTP status code applicable to this problem, expressed as a string value
+        #   code   : An application-specific error code, expressed as a string value.
+        #   title  : A short, human-readable summary of the problem. It **SHOULD NOT** change from
+        #     occurrence to occurrence of the problem, except for purposes of localization.
+        #   detail : A human-readable explanation specific to this occurrence of the problem.
+        def self.attribute_error_objects(attribute_name, attribute_errors)
           attribute_errors.map do |attribute_error|
             {
-              source: { pointer: ActiveModelSerializers::JsonPointer.new(:attribute, attribute_name) },
+              source: error_source(:pointer, attribute_name),
               detail: attribute_error
             }
           end
         end
 
-        def serializable_hash(*)
-          @result = []
-          # TECHDEBT: clean up single vs. collection of resources
-          if serializer.object.respond_to?(:each)
-            @result = collection_errors.flat_map do |collection_error|
-              collection_error.flat_map do |attribute_name, attribute_errors|
-                attribute_error_objects(attribute_name, attribute_errors)
-              end
-            end
+        # description:
+        #   oneOf
+        #     ☑ pointer   : String
+        #     ☑ parameter : String
+        #
+        # description:
+        #   pointer: A JSON Pointer RFC6901 to the associated entity in the request document e.g. "/data"
+        #   for a primary data object, or "/data/attributes/title" for a specific attribute.
+        #   https://tools.ietf.org/html/rfc6901
+        #
+        #   parameter: A string indicating which query parameter caused the error
+        def self.error_source(source_type, attribute_name)
+          case source_type
+          when :pointer
+            {
+              pointer: ActiveModelSerializers::JsonPointer.new(:attribute, attribute_name)
+            }
+          when :parameter
+            {
+              parameter: attribute_name
+            }
           else
-            @result = object_errors.flat_map do |attribute_name, attribute_errors|
-              attribute_error_objects(attribute_name, attribute_errors)
-            end
+            fail UnknownSourceTypeError, "Unknown source type '#{source_type}' for attribute_name '#{attribute_name}'"
           end
-          { root => @result }
-        end
-
-        def fragment_cache(cached_hash, non_cached_hash)
-          JsonApi::FragmentCache.new.fragment_cache(root, cached_hash, non_cached_hash)
-        end
-
-        def root
-          'errors'.freeze
         end
 
         private
 
-        # @return [Array<symbol, Array<String>] i.e. attribute_name, [attribute_errors]
-        def object_errors
-          cache_check(serializer) do
-            serializer.object.errors.messages
-          end
-        end
-
+        # @return [Array<#object_errors>]
         def collection_errors
-          cache_check(serializer) do
-            serializer.object.flat_map do |elem|
-              elem.errors.messages
-            end
+          serializer.flat_map do |error_serializer|
+            Error.resource_errors(error_serializer)
           end
         end
 
-        def attribute_error_objects(attribute_name, attribute_errors)
-          Error.attributes(attribute_name, attribute_errors)
-        end
+        # rubocop:enable Style/AsciiComments
       end
     end
   end

test/action_controller/json_api/errors_test.rb

@@ -8,7 +8,7 @@ module ActionController
           get :render_resource_with_errors
 
           expected_errors_object =
-            { 'errors'.freeze =>
+            { :errors =>
               [
                 { :source => { :pointer => '/data/attributes/name' }, :detail => 'cannot be nil' },
                 { :source => { :pointer => '/data/attributes/name' }, :detail => 'must be longer' },
@@ -30,7 +30,7 @@ module ActionController
             resource.errors.add(:name, 'cannot be nil')
             resource.errors.add(:name, 'must be longer')
             resource.errors.add(:id, 'must be a uuid')
-            render json: resource, adapter: :json_api
+            render json: resource, adapter: 'json_api/error', serializer: ActiveModel::Serializer::ErrorSerializer
           end
         end
 

test/adapter/json_api/errors_test.rb

@@ -23,7 +23,7 @@ module ActiveModelSerializers
           assert_equal serializable_resource.serializer_instance.object, @resource
 
           expected_errors_object =
-            { 'errors'.freeze =>
+            { :errors =>
               [
                 {
                   source: { pointer: '/data/attributes/name' },
@@ -49,7 +49,7 @@ module ActiveModelSerializers
           assert_equal serializable_resource.serializer_instance.object, @resource
 
           expected_errors_object =
-            { 'errors'.freeze =>
+            { :errors =>
               [
                 { :source => { :pointer => '/data/attributes/name' }, :detail => 'cannot be nil' },
                 { :source => { :pointer => '/data/attributes/name' }, :detail => 'must be longer' },
@@ -58,6 +58,20 @@ module ActiveModelSerializers
           }
           assert_equal serializable_resource.as_json, expected_errors_object
         end
+
+        # see http://jsonapi.org/examples/
+        def test_parameter_source_type_error
+          parameter = 'auther'
+          error_source = ActiveModelSerializers::Adapter::JsonApi::Error.error_source(:parameter, parameter)
+          assert_equal({ parameter: parameter }, error_source)
+        end
+
+        def test_unknown_source_type_error
+          value = 'auther'
+          assert_raises(ActiveModelSerializers::Adapter::JsonApi::Error::UnknownSourceTypeError) do
+            ActiveModelSerializers::Adapter::JsonApi::Error.error_source(:hyper, value)
+          end
+        end
       end
     end
   end

test/serializable_resource_test.rb

@@ -37,9 +37,13 @@ module ActiveModel
         options = nil
         resource = ModelWithErrors.new
         resource.errors.add(:name, 'must be awesome')
-        serializable_resource = ActiveModel::SerializableResource.new(resource)
+        serializable_resource = ActiveModel::SerializableResource.new(
+          resource, {
+            serializer: ActiveModel::Serializer::ErrorSerializer,
+            adapter: 'json_api/error'
+          })
         expected_response_document =
-          { 'errors'.freeze =>
+          { :errors =>
             [
               { :source => { :pointer => '/data/attributes/name' }, :detail => 'must be awesome' }
             ]
@@ -53,9 +57,14 @@ module ActiveModel
         resources << resource = ModelWithErrors.new
         resource.errors.add(:title, 'must be amazing')
         resources << ModelWithErrors.new
-        serializable_resource = ActiveModel::SerializableResource.new(resources)
+        serializable_resource = ActiveModel::SerializableResource.new(
+          resources, {
+            serializer: ActiveModel::Serializer::ErrorsSerializer,
+            each_serializer: ActiveModel::Serializer::ErrorSerializer,
+            adapter: 'json_api/error'
+          })
         expected_response_document =
-          { 'errors'.freeze =>
+          { :errors =>
             [
               { :source => { :pointer => '/data/attributes/title' }, :detail => 'must be amazing' }
             ]