diff --git a/lib/active_model/serializable_resource.rb b/lib/active_model/serializable_resource.rb index ec83fcfc..40f09a50 100644 --- a/lib/active_model/serializable_resource.rb +++ b/lib/active_model/serializable_resource.rb @@ -16,6 +16,19 @@ module ActiveModel @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 def serialization_scope=(scope) diff --git a/lib/active_model/serializer.rb b/lib/active_model/serializer.rb index 64bff0ec..6835e978 100644 --- a/lib/active_model/serializer.rb +++ b/lib/active_model/serializer.rb @@ -1,6 +1,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/include_tree' require 'active_model/serializer/associations' require 'active_model/serializer/attributes' diff --git a/lib/active_model/serializer/error_serializer.rb b/lib/active_model/serializer/error_serializer.rb new file mode 100644 index 00000000..e4451afe --- /dev/null +++ b/lib/active_model/serializer/error_serializer.rb @@ -0,0 +1,2 @@ +class ActiveModel::Serializer::ErrorSerializer < ActiveModel::Serializer +end diff --git a/lib/active_model/serializer/lint.rb b/lib/active_model/serializer/lint.rb index b2bc48ff..b791d40d 100644 --- a/lib/active_model/serializer/lint.rb +++ b/lib/active_model/serializer/lint.rb @@ -129,6 +129,20 @@ module ActiveModel::Serializer::Lint assert_instance_of resource_class.model_name, ActiveModel::Name end + def test_active_model_errors + assert_respond_to resource, :errors + end + + def test_active_model_errors_human_attribute_name + assert_respond_to resource.class, :human_attribute_name + assert_equal(-2, resource.class.method(:human_attribute_name).arity) + end + + def test_active_model_errors_lookup_ancestors + assert_respond_to resource.class, :lookup_ancestors + assert_equal 0, resource.class.method(:lookup_ancestors).arity + end + private def resource diff --git a/lib/active_model_serializers.rb b/lib/active_model_serializers.rb index e75e30a0..bf2dac13 100644 --- a/lib/active_model_serializers.rb +++ b/lib/active_model_serializers.rb @@ -10,6 +10,7 @@ module ActiveModelSerializers autoload :Logging autoload :Test autoload :Adapter + autoload :JsonPointer class << self; attr_accessor :logger; end self.logger = ActiveSupport::TaggedLogging.new(ActiveSupport::Logger.new(STDOUT)) diff --git a/lib/active_model_serializers/adapter/json_api.rb b/lib/active_model_serializers/adapter/json_api.rb index 841185f0..c0631400 100644 --- a/lib/active_model_serializers/adapter/json_api.rb +++ b/lib/active_model_serializers/adapter/json_api.rb @@ -8,6 +8,7 @@ module ActiveModelSerializers require 'active_model/serializer/adapter/json_api/meta' autoload :Deserialization require 'active_model/serializer/adapter/json_api/api_objects' + autoload :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_serializers/adapter/json_api/error.rb b/lib/active_model_serializers/adapter/json_api/error.rb new file mode 100644 index 00000000..74321baf --- /dev/null +++ b/lib/active_model_serializers/adapter/json_api/error.rb @@ -0,0 +1,92 @@ +module ActiveModelSerializers + module Adapter + class JsonApi < Base + class Error < Base +=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 + def self.attributes(attribute_name, attribute_errors) + attribute_errors.map do |attribute_error| + { + source: { pointer: ActiveModelSerializers::JsonPointer.new(:attribute, 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 + 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) + Error.attributes(attribute_name, attribute_errors) + end + end + end + end +end diff --git a/lib/active_model_serializers/json_pointer.rb b/lib/active_model_serializers/json_pointer.rb new file mode 100644 index 00000000..c6ba2dc3 --- /dev/null +++ b/lib/active_model_serializers/json_pointer.rb @@ -0,0 +1,14 @@ +module ActiveModelSerializers + module JsonPointer + module_function + + POINTERS = { + attribute: '/data/attributes/%s'.freeze, + primary_data: '/data'.freeze + }.freeze + + def new(pointer_type, value = nil) + format(POINTERS[pointer_type], value) + end + end +end diff --git a/lib/active_model_serializers/model.rb b/lib/active_model_serializers/model.rb index 3043c389..62971392 100644 --- a/lib/active_model_serializers/model.rb +++ b/lib/active_model_serializers/model.rb @@ -6,10 +6,11 @@ module ActiveModelSerializers include ActiveModel::Model include ActiveModel::Serializers::JSON - attr_reader :attributes + attr_reader :attributes, :errors def initialize(attributes = {}) @attributes = attributes + @errors = ActiveModel::Errors.new(self) super end @@ -35,5 +36,14 @@ module ActiveModelSerializers attributes[key] end end + + # The following methods are needed to be minimally implemented for ActiveModel::Errors + def self.human_attribute_name(attr, _options = {}) + attr + end + + def self.lookup_ancestors + [self] + end end end diff --git a/test/action_controller/json_api/errors_test.rb b/test/action_controller/json_api/errors_test.rb new file mode 100644 index 00000000..d6766101 --- /dev/null +++ b/test/action_controller/json_api/errors_test.rb @@ -0,0 +1,41 @@ +require 'test_helper' + +module ActionController + module Serialization + class JsonApi + class ErrorsTest < ActionController::TestCase + def test_active_model_with_multiple_errors + get :render_resource_with_errors + + 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' } + ] + }.to_json + assert_equal json_reponse_body.to_json, expected_errors_object + end + + def json_reponse_body + JSON.load(@response.body) + end + + class ErrorsTestController < ActionController::Base + def render_resource_with_errors + 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, adapter: :json_api + end + end + + tests ErrorsTestController + end + end + end +end diff --git a/test/active_model_serializers/adapter_for_test.rb b/test/active_model_serializers/adapter_for_test.rb index 8dfbc9f3..4fe870be 100644 --- a/test/active_model_serializers/adapter_for_test.rb +++ b/test/active_model_serializers/adapter_for_test.rb @@ -102,7 +102,8 @@ module ActiveModelSerializers 'null'.freeze => ActiveModelSerializers::Adapter::Null, 'json'.freeze => ActiveModelSerializers::Adapter::Json, '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 assert_equal actual, expected_adapter_map @@ -113,6 +114,7 @@ module ActiveModelSerializers 'attributes'.freeze, 'json'.freeze, 'json_api'.freeze, + 'json_api/error'.freeze, 'null'.freeze ] end diff --git a/test/active_model_serializers/json_pointer_test.rb b/test/active_model_serializers/json_pointer_test.rb new file mode 100644 index 00000000..64acc7eb --- /dev/null +++ b/test/active_model_serializers/json_pointer_test.rb @@ -0,0 +1,20 @@ +require 'test_helper' + +class ActiveModelSerializers::JsonPointerTest < ActiveSupport::TestCase + def test_attribute_pointer + attribute_name = 'title' + pointer = ActiveModelSerializers::JsonPointer.new(:attribute, attribute_name) + assert_equal '/data/attributes/title', pointer + end + + def test_primary_data_pointer + pointer = ActiveModelSerializers::JsonPointer.new(:primary_data) + assert_equal '/data', pointer + end + + def test_unkown_data_pointer + assert_raises(TypeError) do + ActiveModelSerializers::JsonPointer.new(:unknown) + end + end +end diff --git a/test/adapter/json_api/errors_test.rb b/test/adapter/json_api/errors_test.rb new file mode 100644 index 00000000..1348a457 --- /dev/null +++ b/test/adapter/json_api/errors_test.rb @@ -0,0 +1,64 @@ +require 'test_helper' + +module ActiveModelSerializers + 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 diff --git a/test/fixtures/poro.rb b/test/fixtures/poro.rb index 445530e4..c40b1ca6 100644 --- a/test/fixtures/poro.rb +++ b/test/fixtures/poro.rb @@ -27,6 +27,17 @@ class Model < ActiveModelSerializers::Model 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 < ::ActiveModelSerializers::Model + attr_accessor :name +end + class Profile < Model end diff --git a/test/lint_test.rb b/test/lint_test.rb index 0ebdda64..9d0f2bc8 100644 --- a/test/lint_test.rb +++ b/test/lint_test.rb @@ -27,6 +27,15 @@ module ActiveModel def updated_at end + def errors + end + + def self.human_attribute_name(attr, options = {}) + end + + def self.lookup_ancestors + end + def self.model_name @_model_name ||= ActiveModel::Name.new(self) end diff --git a/test/serializable_resource_test.rb b/test/serializable_resource_test.rb index 69879504..dcee0d80 100644 --- a/test/serializable_resource_test.rb +++ b/test/serializable_resource_test.rb @@ -31,5 +31,37 @@ module ActiveModel def test_use_adapter_with_adapter_option_as_false refute ActiveModel::SerializableResource.new(@resource, { adapter: false }).use_adapter? 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