Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] JSON API Errors (Initial implementation and roadmap for full feature-set) #1004

Merged
merged 6 commits into from
Mar 7, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
56 changes: 56 additions & 0 deletions docs/jsonapi/errors.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
[Back to Guides](../README.md)

# [JSON API Errors](http://jsonapi.org/format/#errors)

Rendering error documents requires specifying the error serializer(s):

- 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.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually, the errors object will be an ActiveModel::Errors

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As commented in the PR, I think it would be a good idea to add the requirement about data and errors not coexisting in the response.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@buren So, the problem is that I'm documenting how AMS works, not JSON API. I could add a link to the spec (like in #1004 (comment) ) somewhere in here or to https://github.com/rails-api/active_model_serializers/blob/master/docs/jsonapi/schema.md


## 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, 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
})
serializable_resource.as_json(options)
# #=>
# {
# :errors =>
# [
# { :source => { :pointer => '/data/attributes/name' }, :detail => 'must be awesome' }
# ]
# }
```
47 changes: 29 additions & 18 deletions docs/jsonapi/schema.md
Original file line number Diff line number Diff line change
Expand Up @@ -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),<br>attributes, relationships,<br>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(<br>`"^(?!relationships$|links$)\\w[-\\w_]*$"`),<br>any valid JSON | | AM::Serializer#attributes, AM::S::Adapter::JsonApi#resource_object_for
| relationships | patternProperties(<br>`"^\\w[-\\w_]*$"`);<br>links, relationships.data, meta | | AM::S::Adapter::JsonApi#relationships_for
| relationships.data | oneOf (relationshipToOne, relationshipToMany) | | AM::S::Adapter::JsonApi#resource_identifier_for
| attributes | patternProperties(<br>`"^(?!relationships$|links$)\\w[-\\w_]*$"`),<br>any valid JSON | | AM::Serializer#attributes, AMS::Adapter::JsonApi#resource_object_for
| relationships | patternProperties(<br>`"^\\w[-\\w_]*$"`);<br>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),<br>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),<br>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),<br>String(code), String(title),<br>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),<br>String(code), String(title),<br>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.
Expand All @@ -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"`

Expand Down Expand Up @@ -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.
6 changes: 6 additions & 0 deletions lib/active_model/serializer.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
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'
Expand Down Expand Up @@ -116,6 +118,10 @@ def initialize(object, options = {})
end
end

def success?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the purpose of this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sentinel for rendering success vs failurr doc. Search diff for usage

true
end

# Used by adapter as resource root.
def json_key
root || object.class.model_name.to_s.underscore
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions lib/active_model/serializer/collection_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ def initialize(resources, options = {})
end
end

def success?
true
end

def json_key
root || derived_root
end
Expand Down
10 changes: 10 additions & 0 deletions lib/active_model/serializer/error_serializer.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
class ActiveModel::Serializer::ErrorSerializer < ActiveModel::Serializer
# @return [Hash<field_name,Array<error_message>>]
def as_json
object.errors.messages
end
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't really need to inherit from serializer at this point.. but for interface purposes I am


def success?
false
end
end
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure at this time if I want to assign any attributes, or just use it as a 'pass through' for the adapter to make use of the 'object' (see def object_errors in the adapter).

27 changes: 27 additions & 0 deletions lib/active_model/serializer/errors_serializer.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
require 'active_model/serializer/error_serializer'
class ActiveModel::Serializer::ErrorsSerializer
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 success?
false
end

def json_key
nil
end

protected

attr_reader :serializers
end
14 changes: 14 additions & 0 deletions lib/active_model/serializer/lint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions lib/active_model_serializers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
Loading