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 ec83fcfc6..40f09a50b 100644 --- a/lib/active_model/serializable_resource.rb +++ b/lib/active_model/serializable_resource.rb @@ -16,6 +16,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 def serialization_scope=(scope) diff --git a/lib/active_model/serializer.rb b/lib/active_model/serializer.rb index 64bff0eca..6835e978d 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 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/lib/active_model/serializer/lint.rb b/lib/active_model/serializer/lint.rb index b2bc48ff9..b791d40da 100644 --- a/lib/active_model/serializer/lint.rb +++ b/lib/active_model/serializer/lint.rb @@ -129,6 +129,20 @@ def test_model_name 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 e75e30a0d..bf2dac136 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 841185f00..c0631400b 100644 --- a/lib/active_model_serializers/adapter/json_api.rb +++ b/lib/active_model_serializers/adapter/json_api.rb @@ -8,6 +8,7 @@ class JsonApi < Base 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 000000000..74321baf7 --- /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 000000000..c6ba2dc3a --- /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 3043c389e..629713929 100644 --- a/lib/active_model_serializers/model.rb +++ b/lib/active_model_serializers/model.rb @@ -6,10 +6,11 @@ class Model 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 @@ def read_attribute_for_serialization(key) 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 000000000..d67661012 --- /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 8dfbc9f3f..4fe870bea 100644 --- a/test/active_model_serializers/adapter_for_test.rb +++ b/test/active_model_serializers/adapter_for_test.rb @@ -102,7 +102,8 @@ def test_adapter_map '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 @@ def test_adapters '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 000000000..64acc7eb7 --- /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 000000000..1348a4570 --- /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 445530e40..c40b1ca61 100644 --- a/test/fixtures/poro.rb +++ b/test/fixtures/poro.rb @@ -27,6 +27,17 @@ def cache_key_with_digest 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 0ebdda647..9d0f2bc87 100644 --- a/test/lint_test.rb +++ b/test/lint_test.rb @@ -27,6 +27,15 @@ def id 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 698795040..dcee0d802 100644 --- a/test/serializable_resource_test.rb +++ b/test/serializable_resource_test.rb @@ -31,5 +31,37 @@ def test_use_adapter_with_adapter_option 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 40f09a50b..673bdc3b3 100644 --- a/lib/active_model/serializable_resource.rb +++ b/lib/active_model/serializable_resource.rb @@ -16,18 +16,13 @@ def initialize(resource, options = {}) @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 @@ def serialization_scope_name=(scope_name) 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 @@ def serializer @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 7f0a8ac02..b1db25da2 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 000000000..f180d117d --- /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 673bdc3b3..ec83fcfc6 100644 --- a/lib/active_model/serializable_resource.rb +++ b/lib/active_model/serializable_resource.rb @@ -18,14 +18,6 @@ def initialize(resource, options = {}) 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 @@ def serialization_scope_name=(scope_name) 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 @@ def serializer @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 6835e978d..1471876f5 100644 --- a/lib/active_model/serializer.rb +++ b/lib/active_model/serializer.rb @@ -2,6 +2,7 @@ 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 e4451afef..bfbd1ec0e 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 000000000..eaab0a147 --- /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 c0631400b..57179a04d 100644 --- a/lib/active_model_serializers/adapter/json_api.rb +++ b/lib/active_model_serializers/adapter/json_api.rb @@ -13,7 +13,7 @@ class JsonApi < Base # 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 @@ def serializable_hash(options = nil) 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 74321baf7..fbe2654d3 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: - -- 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. + # definition: + # ☐ toplevel_errors array (required) + # ☑ toplevel_meta + # ☑ toplevel_jsonapi + def serializable_hash(*) + hash = {} + # PR Please :) + # Jsonapi.add!(hash) -An error object MAY have the following members: + # 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 -- 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. + # @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 -=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 d67661012..09fd3099a 100644 --- a/test/action_controller/json_api/errors_test.rb +++ b/test/action_controller/json_api/errors_test.rb @@ -8,7 +8,7 @@ def test_active_model_with_multiple_errors 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 @@ def render_resource_with_errors 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 1348a4570..0ab596f9d 100644 --- a/test/adapter/json_api/errors_test.rb +++ b/test/adapter/json_api/errors_test.rb @@ -23,7 +23,7 @@ def test_active_model_with_error assert_equal serializable_resource.serializer_instance.object, @resource expected_errors_object = - { 'errors'.freeze => + { :errors => [ { source: { pointer: '/data/attributes/name' }, @@ -49,7 +49,7 @@ def test_active_model_with_multiple_errors 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 @@ def test_active_model_with_multiple_errors } 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 dcee0d802..14ac73b63 100644 --- a/test/serializable_resource_test.rb +++ b/test/serializable_resource_test.rb @@ -37,9 +37,13 @@ def test_serializable_resource_with_errors 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 @@ def test_serializable_resource_with_collection_containing_errors 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 f180d117d..74f3b1667 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 1471876f5..b0ca3fa6c 100644 --- a/lib/active_model/serializer.rb +++ b/lib/active_model/serializer.rb @@ -118,6 +118,10 @@ def initialize(object, options = {}) 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 c1edfeafc..022588381 100644 --- a/lib/active_model/serializer/collection_serializer.rb +++ b/lib/active_model/serializer/collection_serializer.rb @@ -22,6 +22,10 @@ def initialize(resources, options = {}) 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 bfbd1ec0e..c12dfd370 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 eaab0a147..4b67bae83 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 @@ def initialize(resources, options = {}) 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 57179a04d..4f82835f7 100644 --- a/lib/active_model_serializers/adapter/json_api.rb +++ b/lib/active_model_serializers/adapter/json_api.rb @@ -53,7 +53,14 @@ def initialize(serializer, options = {}) 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 @@ def serializable_hash(options = nil) 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 fbe2654d3..c8383d216 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 @@ def self.error_source(source_type, attribute_name) 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 09fd3099a..d58901103 100644 --- a/test/action_controller/json_api/errors_test.rb +++ b/test/action_controller/json_api/errors_test.rb @@ -30,7 +30,7 @@ def render_resource_with_errors 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 4fe870bea..8dfbc9f3f 100644 --- a/test/active_model_serializers/adapter_for_test.rb +++ b/test/active_model_serializers/adapter_for_test.rb @@ -102,8 +102,7 @@ def test_adapter_map '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 @@ def test_adapters '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 0ab596f9d..da7eff9be 100644 --- a/test/adapter/json_api/errors_test.rb +++ b/test/adapter/json_api/errors_test.rb @@ -13,7 +13,7 @@ def setup 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 @@ def test_active_model_with_error 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 14ac73b63..4c683f9b7 100644 --- a/test/serializable_resource_test.rb +++ b/test/serializable_resource_test.rb @@ -40,7 +40,7 @@ def test_serializable_resource_with_errors 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 @@ def test_serializable_resource_with_collection_containing_errors 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 5bd18c868..dbd227579 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 74f3b1667..1d15dde01 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 72b149484..ba718ec0c 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 dfaabc39b..154b71fdb 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 Adapter 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 058f06031..0336e0b50 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 Adapter 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 4f82835f7..0407ef095 100644 --- a/lib/active_model_serializers/adapter/json_api.rb +++ b/lib/active_model_serializers/adapter/json_api.rb @@ -13,6 +13,7 @@ class JsonApi < Base # 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 @@ def initialize(serializer, options = {}) @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 @@ def serializable_hash(options = nil) 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 @@ def success_document(options) hash end + # {http://jsonapi.org/format/#errors JSON API Errors} # TODO: look into caching # rubocop:disable Style/AsciiComments # definition: @@ -117,6 +122,7 @@ def fragment_cache(cached_hash, non_cached_hash) private + # {http://jsonapi.org/format/#document-resource-objects Primary data} def resource_objects_for(serializers) @primary = [] @included = [] @@ -158,10 +164,12 @@ def process_relationship(serializer, include_tree) 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 @@ def resource_object_for(serializer) 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 @@ def relationships_for(serializer, requested_associations) 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 c6ba2dc3a..a262f3b28 100644 --- a/lib/active_model_serializers/json_pointer.rb +++ b/lib/active_model_serializers/json_pointer.rb @@ -4,7 +4,7 @@ module JsonPointer POINTERS = { attribute: '/data/attributes/%s'.freeze, - primary_data: '/data'.freeze + primary_data: '/data%s'.freeze }.freeze def new(pointer_type, value = nil)