From 0ba944dabf5ace576c657ec4c5e9bd326ff1277a Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 14 Jul 2015 00:29:15 -0500 Subject: [PATCH 1/6] RFC: Json Api Errors (WIP) - ActiveModelSerializers::JsonPointer - ActiveModel::Serializer::Adapter::JsonApi::Error - ActiveModel::Serializer::Adapter::JsonApi::Error.attributes - Fix rubocop config --- lib/active_model/serializable_resource.rb | 13 +++ lib/active_model/serializer.rb | 1 + .../serializer/error_serializer.rb | 2 + lib/active_model/serializer/lint.rb | 14 +++ lib/active_model_serializers.rb | 1 + .../adapter/json_api.rb | 1 + .../adapter/json_api/error.rb | 92 +++++++++++++++++++ lib/active_model_serializers/json_pointer.rb | 14 +++ lib/active_model_serializers/model.rb | 12 ++- .../action_controller/json_api/errors_test.rb | 41 +++++++++ .../adapter_for_test.rb | 4 +- .../json_pointer_test.rb | 20 ++++ test/adapter/json_api/errors_test.rb | 64 +++++++++++++ test/fixtures/poro.rb | 11 +++ test/lint_test.rb | 9 ++ test/serializable_resource_test.rb | 32 +++++++ 16 files changed, 329 insertions(+), 2 deletions(-) create mode 100644 lib/active_model/serializer/error_serializer.rb create mode 100644 lib/active_model_serializers/adapter/json_api/error.rb create mode 100644 lib/active_model_serializers/json_pointer.rb create mode 100644 test/action_controller/json_api/errors_test.rb create mode 100644 test/active_model_serializers/json_pointer_test.rb create mode 100644 test/adapter/json_api/errors_test.rb 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 From dfe162638c328ba40a5e455966296bf2ebdf2c95 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Wed, 2 Dec 2015 11:50:08 -0600 Subject: [PATCH 2/6] Generalize detection of serializable resource with errors --- lib/active_model/serializable_resource.rb | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/active_model/serializable_resource.rb b/lib/active_model/serializable_resource.rb index 40f09a50..673bdc3b 100644 --- a/lib/active_model/serializable_resource.rb +++ b/lib/active_model/serializable_resource.rb @@ -16,18 +16,13 @@ module ActiveModel @resource = resource @adapter_opts, @serializer_opts = options.partition { |k, _| ADAPTER_OPTION_KEYS.include? k }.map { |h| Hash[h] } + end - # TECHDEBT: clean up single vs. collection of resources + def errors? 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 + resource.any? { |elem| elem.respond_to?(:errors) && !elem.errors.empty? } else - if resource.respond_to?(:errors) && !resource.errors.empty? - @serializer_opts[:serializer] = ActiveModel::Serializer::ErrorSerializer - @adapter_opts[:adapter] = :'json_api/error' - end + resource.respond_to?(:errors) && !resource.errors.empty? end end @@ -44,7 +39,11 @@ module ActiveModel end def adapter - @adapter ||= ActiveModelSerializers::Adapter.create(serializer_instance, adapter_opts) + @adapter ||= + begin + adapter_opts[:adapter] = :'json_api/error' if errors? + ActiveModelSerializers::Adapter.create(serializer_instance, adapter_opts) + end end alias_method :adapter_instance, :adapter @@ -59,6 +58,7 @@ 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) From 96107c56aa262d116bf55d0b1371975b2302dbcc Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Wed, 2 Dec 2015 11:56:15 -0600 Subject: [PATCH 3/6] 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 | 4 +- docs/jsonapi/errors.md | 57 ++++++++ lib/active_model/serializable_resource.rb | 15 +- lib/active_model/serializer.rb | 1 + .../serializer/error_serializer.rb | 4 + .../serializer/errors_serializer.rb | 23 +++ .../adapter/json_api.rb | 4 +- .../adapter/json_api/error.rb | 137 ++++++++++-------- .../action_controller/json_api/errors_test.rb | 4 +- test/adapter/json_api/errors_test.rb | 18 ++- test/serializable_resource_test.rb | 17 ++- 11 files changed, 196 insertions(+), 88 deletions(-) create mode 100644 docs/jsonapi/errors.md create mode 100644 lib/active_model/serializer/errors_serializer.rb diff --git a/docs/README.md b/docs/README.md index 7f0a8ac0..b1db25da 100644 --- a/docs/README.md +++ b/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 diff --git a/docs/jsonapi/errors.md b/docs/jsonapi/errors.md new file mode 100644 index 00000000..f180d117 --- /dev/null +++ b/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' } +# ] +# } +``` diff --git a/lib/active_model/serializable_resource.rb b/lib/active_model/serializable_resource.rb index 673bdc3b..ec83fcfc 100644 --- a/lib/active_model/serializable_resource.rb +++ b/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) diff --git a/lib/active_model/serializer.rb b/lib/active_model/serializer.rb index 6835e978..1471876f 100644 --- a/lib/active_model/serializer.rb +++ b/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' diff --git a/lib/active_model/serializer/error_serializer.rb b/lib/active_model/serializer/error_serializer.rb index e4451afe..bfbd1ec0 100644 --- a/lib/active_model/serializer/error_serializer.rb +++ b/lib/active_model/serializer/error_serializer.rb @@ -1,2 +1,6 @@ class ActiveModel::Serializer::ErrorSerializer < ActiveModel::Serializer + # @return [Hash>] + def as_json + object.errors.messages + end end diff --git a/lib/active_model/serializer/errors_serializer.rb b/lib/active_model/serializer/errors_serializer.rb new file mode 100644 index 00000000..eaab0a14 --- /dev/null +++ b/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 diff --git a/lib/active_model_serializers/adapter/json_api.rb b/lib/active_model_serializers/adapter/json_api.rb index c0631400..57179a04 100644 --- a/lib/active_model_serializers/adapter/json_api.rb +++ b/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] ||= {} diff --git a/lib/active_model_serializers/adapter/json_api/error.rb b/lib/active_model_serializers/adapter/json_api/error.rb index 74321baf..fbe2654d 100644 --- a/lib/active_model_serializers/adapter/json_api/error.rb +++ b/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] 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] 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 diff --git a/test/action_controller/json_api/errors_test.rb b/test/action_controller/json_api/errors_test.rb index d6766101..09fd3099 100644 --- a/test/action_controller/json_api/errors_test.rb +++ b/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 diff --git a/test/adapter/json_api/errors_test.rb b/test/adapter/json_api/errors_test.rb index 1348a457..0ab596f9 100644 --- a/test/adapter/json_api/errors_test.rb +++ b/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 diff --git a/test/serializable_resource_test.rb b/test/serializable_resource_test.rb index dcee0d80..14ac73b6 100644 --- a/test/serializable_resource_test.rb +++ b/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' } ] From 3d986377b66f708e25a663bc85a295545da1c63f Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Thu, 11 Feb 2016 00:14:54 -0600 Subject: [PATCH 4/6] Collapse JSON API success/failure documents in one adapter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 ``` --- docs/jsonapi/errors.md | 7 ++-- lib/active_model/serializer.rb | 4 +++ .../serializer/collection_serializer.rb | 4 +++ .../serializer/error_serializer.rb | 4 +++ .../serializer/errors_serializer.rb | 6 +++- .../adapter/json_api.rb | 29 +++++++++++++++ .../adapter/json_api/error.rb | 36 +++---------------- .../action_controller/json_api/errors_test.rb | 2 +- .../adapter_for_test.rb | 4 +-- test/adapter/json_api/errors_test.rb | 4 +-- test/serializable_resource_test.rb | 4 +-- 11 files changed, 60 insertions(+), 44 deletions(-) diff --git a/docs/jsonapi/errors.md b/docs/jsonapi/errors.md index f180d117..74f3b166 100644 --- a/docs/jsonapi/errors.md +++ b/docs/jsonapi/errors.md @@ -2,9 +2,8 @@ # 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: - For a single resource: `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(: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 => # [ @@ -44,7 +43,7 @@ resource.errors.add(:name, 'must be awesome') serializable_resource = ActiveModel::SerializableResource.new( resource, { serializer: ActiveModel::Serializer::ErrorSerializer, - adapter: 'json_api/error' + adapter: :json_api }) serializable_resource.as_json(options) # #=> diff --git a/lib/active_model/serializer.rb b/lib/active_model/serializer.rb index 1471876f..b0ca3fa6 100644 --- a/lib/active_model/serializer.rb +++ b/lib/active_model/serializer.rb @@ -118,6 +118,10 @@ module ActiveModel end end + def success? + true + end + # Used by adapter as resource root. def json_key root || object.class.model_name.to_s.underscore diff --git a/lib/active_model/serializer/collection_serializer.rb b/lib/active_model/serializer/collection_serializer.rb index c1edfeaf..02258838 100644 --- a/lib/active_model/serializer/collection_serializer.rb +++ b/lib/active_model/serializer/collection_serializer.rb @@ -22,6 +22,10 @@ module ActiveModel end end + def success? + true + end + def json_key root || derived_root end diff --git a/lib/active_model/serializer/error_serializer.rb b/lib/active_model/serializer/error_serializer.rb index bfbd1ec0..c12dfd37 100644 --- a/lib/active_model/serializer/error_serializer.rb +++ b/lib/active_model/serializer/error_serializer.rb @@ -3,4 +3,8 @@ class ActiveModel::Serializer::ErrorSerializer < ActiveModel::Serializer def as_json object.errors.messages end + + def success? + false + end end diff --git a/lib/active_model/serializer/errors_serializer.rb b/lib/active_model/serializer/errors_serializer.rb index eaab0a14..4b67bae8 100644 --- a/lib/active_model/serializer/errors_serializer.rb +++ b/lib/active_model/serializer/errors_serializer.rb @@ -1,5 +1,5 @@ require 'active_model/serializer/error_serializer' -class ActiveModel::Serializer::ErrorsSerializer < ActiveModel::Serializer +class ActiveModel::Serializer::ErrorsSerializer include Enumerable delegate :each, to: :@serializers attr_reader :object, :root @@ -13,6 +13,10 @@ class ActiveModel::Serializer::ErrorsSerializer < ActiveModel::Serializer end end + def success? + false + end + def json_key nil end diff --git a/lib/active_model_serializers/adapter/json_api.rb b/lib/active_model_serializers/adapter/json_api.rb index 57179a04..4f82835f 100644 --- a/lib/active_model_serializers/adapter/json_api.rb +++ b/lib/active_model_serializers/adapter/json_api.rb @@ -53,7 +53,14 @@ module ActiveModelSerializers def serializable_hash(options = nil) options ||= {} + if serializer.success? + success_document(options) + else + failure_document + end + end + def success_document(options) is_collection = serializer.respond_to?(:each) serializers = is_collection ? serializer : [serializer] primary_data, included = resource_objects_for(serializers) @@ -77,6 +84,28 @@ module ActiveModelSerializers hash 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) root = false if instance_options.include?(:include) ActiveModelSerializers::Adapter::JsonApi::FragmentCache.new.fragment_cache(root, cached_hash, non_cached_hash) diff --git a/lib/active_model_serializers/adapter/json_api/error.rb b/lib/active_model_serializers/adapter/json_api/error.rb index fbe2654d..c8383d21 100644 --- a/lib/active_model_serializers/adapter/json_api/error.rb +++ b/lib/active_model_serializers/adapter/json_api/error.rb @@ -1,29 +1,13 @@ module ActiveModelSerializers module Adapter class JsonApi < Base - class Error < Base - UnknownSourceTypeError = Class.new(ArgumentError) + module Error # rubocop:disable Style/AsciiComments - # TODO: look into caching - - # 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 + UnknownSourceTypeError = Class.new(ArgumentError) + # Builds a JSON API Errors Object + # {http://jsonapi.org/format/#errors JSON API Errors} + # # @param [ActiveModel::Serializer::ErrorSerializer] # @return [Array] i.e. attribute_name, [attribute_errors] def self.resource_errors(error_serializer) @@ -86,16 +70,6 @@ module ActiveModelSerializers fail UnknownSourceTypeError, "Unknown source type '#{source_type}' for attribute_name '#{attribute_name}'" 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 end end diff --git a/test/action_controller/json_api/errors_test.rb b/test/action_controller/json_api/errors_test.rb index 09fd3099..d5890110 100644 --- a/test/action_controller/json_api/errors_test.rb +++ b/test/action_controller/json_api/errors_test.rb @@ -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/error', serializer: ActiveModel::Serializer::ErrorSerializer + render json: resource, adapter: :json_api, serializer: ActiveModel::Serializer::ErrorSerializer end end diff --git a/test/active_model_serializers/adapter_for_test.rb b/test/active_model_serializers/adapter_for_test.rb index 4fe870be..8dfbc9f3 100644 --- a/test/active_model_serializers/adapter_for_test.rb +++ b/test/active_model_serializers/adapter_for_test.rb @@ -102,8 +102,7 @@ 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/error'.freeze => ActiveModelSerializers::Adapter::JsonApi::Error + 'json_api'.freeze => ActiveModelSerializers::Adapter::JsonApi } actual = ActiveModelSerializers::Adapter.adapter_map assert_equal actual, expected_adapter_map @@ -114,7 +113,6 @@ module ActiveModelSerializers 'attributes'.freeze, 'json'.freeze, 'json_api'.freeze, - 'json_api/error'.freeze, 'null'.freeze ] end diff --git a/test/adapter/json_api/errors_test.rb b/test/adapter/json_api/errors_test.rb index 0ab596f9..da7eff9b 100644 --- a/test/adapter/json_api/errors_test.rb +++ b/test/adapter/json_api/errors_test.rb @@ -13,7 +13,7 @@ module ActiveModelSerializers def test_active_model_with_error options = { serializer: ActiveModel::Serializer::ErrorSerializer, - adapter: :'json_api/error' + adapter: :json_api } @resource.errors.add(:name, 'cannot be nil') @@ -37,7 +37,7 @@ module ActiveModelSerializers def test_active_model_with_multiple_errors options = { serializer: ActiveModel::Serializer::ErrorSerializer, - adapter: :'json_api/error' + adapter: :json_api } @resource.errors.add(:name, 'cannot be nil') diff --git a/test/serializable_resource_test.rb b/test/serializable_resource_test.rb index 14ac73b6..4c683f9b 100644 --- a/test/serializable_resource_test.rb +++ b/test/serializable_resource_test.rb @@ -40,7 +40,7 @@ module ActiveModel serializable_resource = ActiveModel::SerializableResource.new( resource, { serializer: ActiveModel::Serializer::ErrorSerializer, - adapter: 'json_api/error' + adapter: :json_api }) expected_response_document = { :errors => @@ -61,7 +61,7 @@ module ActiveModel resources, { serializer: ActiveModel::Serializer::ErrorsSerializer, each_serializer: ActiveModel::Serializer::ErrorSerializer, - adapter: 'json_api/error' + adapter: :json_api }) expected_response_document = { :errors => From e6ae34b84ce979932723031770dfaf8f9b6bc595 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Fri, 26 Feb 2016 00:26:02 -0600 Subject: [PATCH 5/6] Update documentation with Yard links --- CHANGELOG.md | 3 ++ docs/jsonapi/errors.md | 2 +- docs/jsonapi/schema.md | 47 ++++++++++++------- .../json_api/api_objects/relationship.rb | 4 ++ .../api_objects/resource_identifier.rb | 1 + .../adapter/json_api.rb | 12 +++++ 6 files changed, 50 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5bd18c86..dbd22757 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,9 @@ Breaking changes: Features: +- [#1004](https://github.com/rails-api/active_model_serializers/pull/1004) JSON API errors object implementation. + - Only implements `detail` and `source` as derived from `ActiveModel::Error` + - Provides checklist of remaining questions and remaining parts of the spec. - [#1515](https://github.com/rails-api/active_model_serializers/pull/1515) Adds support for symbols to the `ActiveModel::Serializer.type` method. (@groyoh) - [#1504](https://github.com/rails-api/active_model_serializers/pull/1504) Adds the changes missing from #1454 diff --git a/docs/jsonapi/errors.md b/docs/jsonapi/errors.md index 74f3b166..1d15dde0 100644 --- a/docs/jsonapi/errors.md +++ b/docs/jsonapi/errors.md @@ -1,6 +1,6 @@ [Back to Guides](../README.md) -# JSON API Errors +# [JSON API Errors](http://jsonapi.org/format/#errors) Rendering error documents requires specifying the error serializer(s): diff --git a/docs/jsonapi/schema.md b/docs/jsonapi/schema.md index 72b14948..ba718ec0 100644 --- a/docs/jsonapi/schema.md +++ b/docs/jsonapi/schema.md @@ -58,33 +58,33 @@ Example supported requests |-----------------------|----------------------------------------------------------------------------------------------------|----------|---------------------------------------| | schema | oneOf (success, failure, info) | | | success | data, included, meta, links, jsonapi | | AM::SerializableResource -| success.meta | meta | | AM::S::Adapter::Base#meta -| success.included | UniqueArray(resource) | | AM::S::Adapter::JsonApi#serializable_hash_for_collection +| success.meta | meta | | AMS::Adapter::Base#meta +| success.included | UniqueArray(resource) | | AMS::Adapter::JsonApi#serializable_hash_for_collection | success.data | data | | -| success.links | allOf (links, pagination) | | AM::S::Adapter::JsonApi#links_for +| success.links | allOf (links, pagination) | | AMS::Adapter::JsonApi#links_for | success.jsonapi | jsonapi | | -| failure | errors, meta, jsonapi | errors | -| failure.errors | UniqueArray(error) | | #1004 -| meta | Object | | -| data | oneOf (resource, UniqueArray(resource)) | | AM::S::Adapter::JsonApi#serializable_hash_for_collection,#serializable_hash_for_single_resource +| failure | errors, meta, jsonapi | errors | AMS::Adapter::JsonApi#failure_document, #1004 +| failure.errors | UniqueArray(error) | | AM::S::ErrorSerializer, #1004 +| meta | Object | | +| data | oneOf (resource, UniqueArray(resource)) | | AMS::Adapter::JsonApi#serializable_hash_for_collection,#serializable_hash_for_single_resource | resource | String(type), String(id),
attributes, relationships,
links, meta | type, id | AM::S::Adapter::JsonApi#primary_data_for | links | Uri(self), Link(related) | | #1028, #1246, #1282 | link | oneOf (linkString, linkObject) | | | link.linkString | Uri | | | link.linkObject | Uri(href), meta | href | -| attributes | patternProperties(
`"^(?!relationships$|links$)\\w[-\\w_]*$"`),
any valid JSON | | AM::Serializer#attributes, AM::S::Adapter::JsonApi#resource_object_for -| relationships | patternProperties(
`"^\\w[-\\w_]*$"`);
links, relationships.data, meta | | AM::S::Adapter::JsonApi#relationships_for -| relationships.data | oneOf (relationshipToOne, relationshipToMany) | | AM::S::Adapter::JsonApi#resource_identifier_for +| attributes | patternProperties(
`"^(?!relationships$|links$)\\w[-\\w_]*$"`),
any valid JSON | | AM::Serializer#attributes, AMS::Adapter::JsonApi#resource_object_for +| relationships | patternProperties(
`"^\\w[-\\w_]*$"`);
links, relationships.data, meta | | AMS::Adapter::JsonApi#relationships_for +| relationships.data | oneOf (relationshipToOne, relationshipToMany) | | AMS::Adapter::JsonApi#resource_identifier_for | relationshipToOne | anyOf(empty, linkage) | | | relationshipToMany | UniqueArray(linkage) | | | empty | null | | -| linkage | String(type), String(id), meta | type, id | AM::S::Adapter::JsonApi#primary_data_for -| pagination | pageObject(first), pageObject(last),
pageObject(prev), pageObject(next) | | AM::S::Adapter::JsonApi::PaginationLinks#serializable_hash +| linkage | String(type), String(id), meta | type, id | AMS::Adapter::JsonApi#primary_data_for +| pagination | pageObject(first), pageObject(last),
pageObject(prev), pageObject(next) | | AMS::Adapter::JsonApi::PaginationLinks#serializable_hash | pagination.pageObject | oneOf(Uri, null) | | -| jsonapi | String(version), meta | | AM::S::Adapter::JsonApi::ApiObjects::JsonApi -| error | String(id), links, String(status),
String(code), String(title),
String(detail), error.source, meta | | -| error.source | String(pointer), String(parameter) | | -| pointer | [JSON Pointer RFC6901](https://tools.ietf.org/html/rfc6901) | | +| jsonapi | String(version), meta | | AMS::Adapter::JsonApi::ApiObjects::JsonApi#as_json +| error | String(id), links, String(status),
String(code), String(title),
String(detail), error.source, meta | | AM::S::ErrorSerializer, AMS::Adapter::JsonApi::Error.resource_errors +| error.source | String(pointer), String(parameter) | | AMS::Adapter::JsonApi::Error.error_source +| pointer | [JSON Pointer RFC6901](https://tools.ietf.org/html/rfc6901) | | AMS::JsonPointer The [http://jsonapi.org/schema](schema/schema.json) makes a nice roadmap. @@ -102,7 +102,7 @@ The [http://jsonapi.org/schema](schema/schema.json) makes a nice roadmap. ### Failure Document - [ ] failure - - [ ] errors: array of unique items of type ` "$ref": "#/definitions/error"` + - [x] errors: array of unique items of type ` "$ref": "#/definitions/error"` - [ ] meta: `"$ref": "#/definitions/meta"` - [ ] jsonapi: `"$ref": "#/definitions/jsonapi"` @@ -137,4 +137,15 @@ The [http://jsonapi.org/schema](schema/schema.json) makes a nice roadmap. - [ ] pagination - [ ] jsonapi - [ ] meta - - [ ] error: id, links, status, code, title: detail: source [{pointer, type}, {parameter: {description, type}], meta + - [ ] error + - [ ] 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. + - [x] detail: a human-readable explanation specific to this occurrence of the problem. + - [x] source: an object containing references to the source of the error, optionally including any of the following members: + - [x] pointer: a JSON Pointer [RFC6901](https://tools.ietf.org/html/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]. + - [x] parameter: a string indicating which query parameter caused the error. + - [ ] meta: a meta object containing non-standard meta-information about the error. diff --git a/lib/active_model/serializer/adapter/json_api/api_objects/relationship.rb b/lib/active_model/serializer/adapter/json_api/api_objects/relationship.rb index dfaabc39..154b71fd 100644 --- a/lib/active_model/serializer/adapter/json_api/api_objects/relationship.rb +++ b/lib/active_model/serializer/adapter/json_api/api_objects/relationship.rb @@ -4,6 +4,10 @@ module ActiveModel class JsonApi module ApiObjects class Relationship + # {http://jsonapi.org/format/#document-resource-object-related-resource-links Document Resource Object Related Resource Links} + # {http://jsonapi.org/format/#document-links Document Links} + # {http://jsonapi.org/format/#document-resource-object-linkage Document Resource Relationship Linkage} + # {http://jsonapi.org/format/#document-meta Docment Meta} def initialize(parent_serializer, serializer, options = {}, links = {}, meta = nil) @object = parent_serializer.object @scope = parent_serializer.scope diff --git a/lib/active_model/serializer/adapter/json_api/api_objects/resource_identifier.rb b/lib/active_model/serializer/adapter/json_api/api_objects/resource_identifier.rb index 058f0603..0336e0b5 100644 --- a/lib/active_model/serializer/adapter/json_api/api_objects/resource_identifier.rb +++ b/lib/active_model/serializer/adapter/json_api/api_objects/resource_identifier.rb @@ -4,6 +4,7 @@ module ActiveModel class JsonApi module ApiObjects class ResourceIdentifier + # {http://jsonapi.org/format/#document-resource-identifier-objects Resource Identifier Objects} def initialize(serializer) @id = id_for(serializer) @type = type_for(serializer) diff --git a/lib/active_model_serializers/adapter/json_api.rb b/lib/active_model_serializers/adapter/json_api.rb index 4f82835f..0407ef09 100644 --- a/lib/active_model_serializers/adapter/json_api.rb +++ b/lib/active_model_serializers/adapter/json_api.rb @@ -13,6 +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 + # {http://jsonapi.org/format/#document-jsonapi-object Jsonapi Object} module Jsonapi ActiveModelSerializers.config.jsonapi_version = '1.0' ActiveModelSerializers.config.jsonapi_toplevel_meta = {} @@ -51,6 +52,8 @@ module ActiveModelSerializers @fieldset = options[:fieldset] || ActiveModel::Serializer::Fieldset.new(options.delete(:fields)) end + # {http://jsonapi.org/format/#crud Requests are transactional, i.e. success or failure} + # {http://jsonapi.org/format/#document-top-level data and errors MUST NOT coexist in the same document.} def serializable_hash(options = nil) options ||= {} if serializer.success? @@ -60,6 +63,7 @@ module ActiveModelSerializers end end + # {http://jsonapi.org/format/#document-top-level Primary data} def success_document(options) is_collection = serializer.respond_to?(:each) serializers = is_collection ? serializer : [serializer] @@ -84,6 +88,7 @@ module ActiveModelSerializers hash end + # {http://jsonapi.org/format/#errors JSON API Errors} # TODO: look into caching # rubocop:disable Style/AsciiComments # definition: @@ -117,6 +122,7 @@ module ActiveModelSerializers private + # {http://jsonapi.org/format/#document-resource-objects Primary data} def resource_objects_for(serializers) @primary = [] @included = [] @@ -158,10 +164,12 @@ module ActiveModelSerializers process_relationships(serializer, include_tree) end + # {http://jsonapi.org/format/#document-resource-object-attributes Document Resource Object Attributes} def attributes_for(serializer, fields) serializer.attributes(fields).except(:id) end + # {http://jsonapi.org/format/#document-resource-objects Document Resource Objects} def resource_object_for(serializer) resource_object = cache_check(serializer) do resource_object = ActiveModel::Serializer::Adapter::JsonApi::ApiObjects::ResourceIdentifier.new(serializer).as_json @@ -185,6 +193,7 @@ module ActiveModelSerializers resource_object end + # {http://jsonapi.org/format/#document-resource-object-relationships Document Resource Object Relationship} def relationships_for(serializer, requested_associations) include_tree = ActiveModel::Serializer::IncludeTree.from_include_args(requested_associations) serializer.associations(include_tree).each_with_object({}) do |association, hash| @@ -198,16 +207,19 @@ module ActiveModelSerializers end end + # {http://jsonapi.org/format/#document-links Document Links} def links_for(serializer) serializer._links.each_with_object({}) do |(name, value), hash| hash[name] = Link.new(serializer, value).as_json end end + # {http://jsonapi.org/format/#fetching-pagination Pagination Links} def pagination_links_for(serializer, options) JsonApi::PaginationLinks.new(serializer.object, options[:serialization_context]).serializable_hash(options) end + # {http://jsonapi.org/format/#document-meta Docment Meta} def meta_for(serializer) ActiveModel::Serializer::Adapter::JsonApi::Meta.new(serializer).as_json end From d03db81070d6e18c7191f4986bc0cc60b9dfc8a3 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Fri, 4 Mar 2016 20:43:16 -0500 Subject: [PATCH 6/6] add an extra format token to the primary data string so that JRuby doesn't break --- lib/active_model_serializers/json_pointer.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/active_model_serializers/json_pointer.rb b/lib/active_model_serializers/json_pointer.rb index c6ba2dc3..a262f3b2 100644 --- a/lib/active_model_serializers/json_pointer.rb +++ b/lib/active_model_serializers/json_pointer.rb @@ -4,7 +4,7 @@ module ActiveModelSerializers POINTERS = { attribute: '/data/attributes/%s'.freeze, - primary_data: '/data'.freeze + primary_data: '/data%s'.freeze }.freeze def new(pointer_type, value = nil)