diff --git a/lib/active_model/serializable_resource.rb b/lib/active_model/serializable_resource.rb index 56453dd98..1fce9d38f 100644 --- a/lib/active_model/serializable_resource.rb +++ b/lib/active_model/serializable_resource.rb @@ -9,6 +9,19 @@ def initialize(resource, options = {}) @resource = resource @adapter_opts, @serializer_opts = options.partition { |k, _| ADAPTER_OPTION_KEYS.include? k }.map { |h| Hash[h] } + + # TECHDEBT: clean up single vs. collection of resources + if resource.respond_to?(:each) + if resource.any? { |elem| elem.respond_to?(:errors) && !elem.errors.empty? } + @serializer_opts[:serializer] = ActiveModel::Serializer::ErrorSerializer + @adapter_opts[:adapter] = :'json_api/error' + end + else + if resource.respond_to?(:errors) && !resource.errors.empty? + @serializer_opts[:serializer] = ActiveModel::Serializer::ErrorSerializer + @adapter_opts[:adapter] = :'json_api/error' + end + end end delegate :serializable_hash, :as_json, :to_json, to: :adapter diff --git a/lib/active_model/serializer.rb b/lib/active_model/serializer.rb index 643167a77..e0382d6ce 100644 --- a/lib/active_model/serializer.rb +++ b/lib/active_model/serializer.rb @@ -1,5 +1,6 @@ require 'thread_safe' require 'active_model/serializer/array_serializer' +require 'active_model/serializer/error_serializer' require 'active_model/serializer/include_tree' require 'active_model/serializer/associations' require 'active_model/serializer/configuration' @@ -27,11 +28,15 @@ class Serializer ) /x + class_attribute :_attributes, instance_writer: false + self._attributes ||= [] + class_attribute :_attributes_keys, instance_writer: false, instance_reader: false + self._attributes_keys ||= {} + class_attribute :_type, instance_writer: false + class_attribute :_fragmented, instance_writer: false + class << self - attr_accessor :_attributes - attr_accessor :_attributes_keys attr_accessor :_cache - attr_accessor :_fragmented attr_accessor :_cache_key attr_accessor :_cache_only attr_accessor :_cache_except @@ -40,8 +45,8 @@ class << self end def self.inherited(base) - base._attributes = _attributes.try(:dup) || [] - base._attributes_keys = _attributes_keys.try(:dup) || {} + base._attributes = _attributes.dup + base._attributes_keys = _attributes_keys.dup base._cache_digest = digest_caller_file(caller.first) super end @@ -122,7 +127,6 @@ def self.get_serializer_for(klass) end attr_accessor :object, :root, :scope - class_attribute :_type, instance_writer: false def initialize(object, options = {}) self.object = object @@ -143,13 +147,13 @@ def json_key end def attributes - attributes = self.class._attributes.dup + attributes = _attributes.dup attributes.each_with_object({}) do |name, hash| - unless self.class._fragmented - hash[name] = send(name) + if _fragmented + hash[name] = _fragmented.public_send(name) else - hash[name] = self.class._fragmented.public_send(name) + hash[name] = send(name) end end end diff --git a/lib/active_model/serializer/adapter/json_api.rb b/lib/active_model/serializer/adapter/json_api.rb index 2792af094..cdbd633d2 100644 --- a/lib/active_model/serializer/adapter/json_api.rb +++ b/lib/active_model/serializer/adapter/json_api.rb @@ -5,6 +5,7 @@ class JsonApi < Base extend ActiveSupport::Autoload autoload :PaginationLinks autoload :FragmentCache + require 'active_model/serializer/adapter/json_api/error' # TODO: if we like this abstraction and other API objects to it, # then extract to its own file and require it. diff --git a/lib/active_model/serializer/adapter/json_api/error.rb b/lib/active_model/serializer/adapter/json_api/error.rb new file mode 100644 index 000000000..2944a666d --- /dev/null +++ b/lib/active_model/serializer/adapter/json_api/error.rb @@ -0,0 +1,60 @@ +module ActiveModel + class Serializer + module Adapter + class JsonApi < Base + class Error < Base + 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 + else + @result = object_errors.flat_map do |attribute_name, attribute_errors| + attribute_error_objects(attribute_name, attribute_errors) + end + 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] i.e. attribute_name, [attribute_errors] + def object_errors + cache_check(serializer) do + serializer.object.errors.messages + end + end + + def collection_errors + cache_check(serializer) do + serializer.object.flat_map do |elem| + elem.errors.messages + end + end + end + + def attribute_error_objects(attribute_name, attribute_errors) + attribute_errors.map do |attribute_error| + { + source: { pointer: "data/attributes/#{attribute_name}" }, + detail: attribute_error + } + end + end + end + end + end + end +end diff --git a/lib/active_model/serializer/error_serializer.rb b/lib/active_model/serializer/error_serializer.rb new file mode 100644 index 000000000..e4451afef --- /dev/null +++ b/lib/active_model/serializer/error_serializer.rb @@ -0,0 +1,2 @@ +class ActiveModel::Serializer::ErrorSerializer < ActiveModel::Serializer +end diff --git a/test/adapter/json_api/errors_test.rb b/test/adapter/json_api/errors_test.rb new file mode 100644 index 000000000..a1f33e618 --- /dev/null +++ b/test/adapter/json_api/errors_test.rb @@ -0,0 +1,97 @@ +require 'test_helper' + +=begin +## http://jsonapi.org/format/#document-top-level + +A document MUST contain at least one of the following top-level members: + +- data: the document's "primary data" +- errors: an array of error objects +- meta: a meta object that contains non-standard meta-information. + +The members data and errors MUST NOT coexist in the same document. + +## 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 + +module ActiveModel + class Serializer + module Adapter + class JsonApi < Base + class ErrorsTest < Minitest::Test + include ActiveModel::Serializer::Lint::Tests + + def setup + @resource = ModelWithErrors.new + end + + def test_active_model_with_error + options = { + serializer: ActiveModel::Serializer::ErrorSerializer, + adapter: :'json_api/error' + } + + @resource.errors.add(:name, 'cannot be nil') + + serializable_resource = ActiveModel::SerializableResource.new(@resource, options) + assert_equal serializable_resource.serializer_instance.attributes, {} + assert_equal serializable_resource.serializer_instance.object, @resource + + expected_errors_object = + { 'errors'.freeze => + [ + { + source: { pointer: 'data/attributes/name' }, + detail: 'cannot be nil' + } + ] + } + assert_equal serializable_resource.as_json, expected_errors_object + end + + def test_active_model_with_multiple_errors + options = { + serializer: ActiveModel::Serializer::ErrorSerializer, + adapter: :'json_api/error' + } + + @resource.errors.add(:name, 'cannot be nil') + @resource.errors.add(:name, 'must be longer') + @resource.errors.add(:id, 'must be a uuid') + + serializable_resource = ActiveModel::SerializableResource.new(@resource, options) + assert_equal serializable_resource.serializer_instance.attributes, {} + assert_equal serializable_resource.serializer_instance.object, @resource + + expected_errors_object = + { 'errors'.freeze => + [ + { :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' } + ] + } + assert_equal serializable_resource.as_json, expected_errors_object + end + end + end + end + end +end diff --git a/test/fixtures/poro.rb b/test/fixtures/poro.rb index bfb9c84a7..6e2d68fe7 100644 --- a/test/fixtures/poro.rb +++ b/test/fixtures/poro.rb @@ -53,6 +53,53 @@ def updated_at end end +# see +# https://github.com/rails/rails/blob/4-2-stable/activemodel/lib/active_model/errors.rb +# The below allows you to do: +# +# model = ModelWithErrors.new +# model.validate! # => ["cannot be nil"] +# model.errors.full_messages # => ["name cannot be nil"] +class ModelWithErrors + include ActiveModel::Serialization + def attributes + { + errors: errors, + name: name + } + end + + def id + object_id + end + + def cache_key + "#{self.class.name.downcase}/#{id}-#{Time.now.utc.strftime("%Y%m%d%H%M%S%9N")}" + end + + # Required dependency for ActiveModel::Errors + extend ActiveModel::Naming + def initialize + @errors = ActiveModel::Errors.new(self) + end + attr_accessor :name + attr_reader :errors + + # The following methods are needed to be minimally implemented + + def read_attribute_for_validation(attr) + send(attr) + end + + def self.human_attribute_name(attr, options = {}) + attr + end + + def self.lookup_ancestors + [self] + end +end + class Profile < Model end diff --git a/test/serializable_resource_test.rb b/test/serializable_resource_test.rb index 9f695a130..ee6be26b0 100644 --- a/test/serializable_resource_test.rb +++ b/test/serializable_resource_test.rb @@ -23,5 +23,37 @@ def test_serializable_resource_delegates_as_json_to_the_adapter options = nil assert_equal @adapter.as_json(options), @serializable_resource.as_json(options) end + + class SerializableResourceErrorsTest < Minitest::Test + def test_serializable_resource_with_errors + options = nil + resource = ModelWithErrors.new + resource.errors.add(:name, 'must be awesome') + serializable_resource = ActiveModel::SerializableResource.new(resource) + expected_response_document = + { 'errors'.freeze => + [ + { :source => { :pointer => 'data/attributes/name' }, :detail => 'must be awesome' } + ] + } + assert_equal serializable_resource.as_json(options), expected_response_document + end + + def test_serializable_resource_with_collection_containing_errors + options = nil + resources = [] + resources << resource = ModelWithErrors.new + resource.errors.add(:title, 'must be amazing') + resources << ModelWithErrors.new + serializable_resource = ActiveModel::SerializableResource.new(resources) + expected_response_document = + { 'errors'.freeze => + [ + { :source => { :pointer => 'data/attributes/title' }, :detail => 'must be amazing' } + ] + } + assert_equal serializable_resource.as_json(options), expected_response_document + end + end end end diff --git a/test/serializers/adapter_for_test.rb b/test/serializers/adapter_for_test.rb index ecb837770..c09753807 100644 --- a/test/serializers/adapter_for_test.rb +++ b/test/serializers/adapter_for_test.rb @@ -63,8 +63,10 @@ def test_adapter_map expected_adapter_map = { 'null'.freeze => ActiveModel::Serializer::Adapter::Null, 'json'.freeze => ActiveModel::Serializer::Adapter::Json, - 'attributes'.freeze => ActiveModel::Serializer::Adapter::Attributes, - 'json_api'.freeze => ActiveModel::Serializer::Adapter::JsonApi + 'attributes'.freeze => ActiveModel::Serializer::Adapter::Attributes, + 'json_api'.freeze => ActiveModel::Serializer::Adapter::JsonApi, + 'json_api/error'.freeze => ActiveModel::Serializer::Adapter::JsonApi::Error, + 'null'.freeze => ActiveModel::Serializer::Adapter::Null } actual = ActiveModel::Serializer::Adapter.adapter_map assert_equal actual, expected_adapter_map @@ -75,6 +77,7 @@ def test_adapters 'attributes'.freeze, 'json'.freeze, 'json_api'.freeze, + 'json_api/error'.freeze, 'null'.freeze ] end