Skip to content

Add top-level jsonapi member #1050

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

Merged
merged 1 commit into from
Oct 2, 2015
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ Features:
- [#1158](https://github.com/rails-api/active_model_serializers/pull/1158) Add support for wildcards in `include` option (@beauby)
- [#1127](https://github.com/rails-api/active_model_serializers/pull/1127) Add support for nested
associations for JSON and Attributes adapters via the `include` option (@NullVoxPopuli, @beauby).
- [#1050](https://github.com/rails-api/active_model_serializers/pull/1050) Add support for toplevel jsonapi member (@beauby, @bf4)

Fixes:

Expand Down
8 changes: 8 additions & 0 deletions docs/general/configuration_options.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,11 @@ The following configuration options can be set on `ActiveModel::Serializer.confi
## JSON API

- `jsonapi_resource_type`: Whether the `type` attributes of resources should be singular or plural. Possible values: `:singular, :plural`. Default: `:plural`.
- `jsonapi_include_toplevel_object`: Whether to include a [top level JSON API member](http://jsonapi.org/format/#document-jsonapi-object)
in the response document.
Default: `false`.
- Used when `jsonapi_include_toplevel_object` is `true`:
- `jsonapi_version`: The latest version of the spec the API conforms to.
Default: `'1.0'`.
- `jsonapi_toplevel_meta`: Optional metadata. Not included if empty.
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 do think there's a latent json_api config object this exposes

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed there is.

Copy link
Member Author

Choose a reason for hiding this comment

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

update config to encapsulate in json_api

Copy link
Member Author

Choose a reason for hiding this comment

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

rename to jsonapi_toplevel_object_meta?

Copy link
Member Author

Choose a reason for hiding this comment

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

top level meta wouldn't be set in a global config.. but name could be confusing

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed.

Default: `{}`.
2 changes: 1 addition & 1 deletion lib/active_model/serializer.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
require 'thread_safe'
require 'active_model/serializer/adapter'
require 'active_model/serializer/array_serializer'
require 'active_model/serializer/include_tree'
require 'active_model/serializer/associations'
Expand All @@ -11,6 +10,7 @@ module ActiveModel
class Serializer
include Configuration
include Associations
require 'active_model/serializer/adapter'

# Matches
# "c:/git/emberjs/ember-crm-backend/app/serializers/lead_serializer.rb:1:in `<top (required)>'"
Expand Down
50 changes: 45 additions & 5 deletions lib/active_model/serializer/adapter/json_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,41 @@ class JsonApi < Base
autoload :PaginationLinks
autoload :FragmentCache

# 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
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the JsonApi namespace?

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's the JsonApi Object in the JSON API

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, but it's already the JsonApiObject Object in AMS, so why double namespace?

Copy link
Member Author

Choose a reason for hiding this comment

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

good call. my intention was that I was considering having a whole bunch of JsonApiObjects like JsonApi, Link, Error, Meta, Data, etc.

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably is a latent jsonapi config object here, or at least JsonApi.config.version via class JsonApi < Base; def config; ActiveModel::Serializer.config.jsonapi; end;

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed, as mentioned in #1097, we should somehow namespace the config options.

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe we should do that in this PR before its too late?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, it would make sense. Any specific way you'd like it to be?

ActiveModel::Serializer.config.jsonapi_version = '1.0'
ActiveModel::Serializer.config.jsonapi_toplevel_meta = {}
# Make JSON API top-level jsonapi member opt-in
# ref: http://jsonapi.org/format/#document-top-level
Copy link
Member Author

Choose a reason for hiding this comment

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

@beauby :)

ActiveModel::Serializer.config.jsonapi_include_toplevel_object = false

module_function

def add!(hash)
hash.merge!(object) if include_object?
end

def include_object?
ActiveModel::Serializer.config.jsonapi_include_toplevel_object
end

# TODO: see if we can cache this
def object
Copy link
Contributor

Choose a reason for hiding this comment

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

How about something like:

def object
  return @object if @object
  object = {
    jsonapi: {
      version: ActiveModel::Serializer.config.jsonapi_version,
      meta: ActiveModel::Serializer.config.jsonapi_toplevel_meta
    }
  }
  object[:jsonapi].reject! { |_, v| v.blank? }

  @object = object
  @object
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.

since we add meta in some of the tests, I'd have to write a way to clear the cache, which I don't want to do right now, could be in a test helper I guess.

Copy link
Member Author

Choose a reason for hiding this comment

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

which is why I left it as a todo :)

object = {
jsonapi: {
version: ActiveModel::Serializer.config.jsonapi_version,
meta: ActiveModel::Serializer.config.jsonapi_toplevel_meta
Copy link
Contributor

Choose a reason for hiding this comment

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

Meta is optional as per the spec, so it should't be included by default.

}
}
object[:jsonapi].reject! { |_, v| v.blank? }

object
end
end
end

def initialize(serializer, options = {})
super
@include_tree = IncludeTree.from_include_args(options[:include])
Expand All @@ -21,11 +56,16 @@ def initialize(serializer, options = {})
def serializable_hash(options = nil)
options ||= {}

if serializer.respond_to?(:each)
serializable_hash_for_collection(options)
else
serializable_hash_for_single_resource(options)
end
hash =
if serializer.respond_to?(:each)
serializable_hash_for_collection(options)
else
serializable_hash_for_single_resource(options)
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.

someday we'll reify these two latent objects..


ApiObjects::JsonApi.add!(hash)
Copy link
Member Author

Choose a reason for hiding this comment

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

do we prefer this after?


hash
end

def fragment_cache(cached_hash, non_cached_hash)
Expand Down
2 changes: 2 additions & 0 deletions lib/active_model/serializer/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ module Configuration
include ActiveSupport::Configurable
extend ActiveSupport::Concern

# Configuration options may also be set in
# Serializers and Adapters
included do |base|
base.config.array_serializer = ActiveModel::Serializer::ArraySerializer
base.config.adapter = :attributes
Expand Down
84 changes: 84 additions & 0 deletions test/adapter/json_api/toplevel_jsonapi_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
require 'test_helper'

module ActiveModel
class Serializer
module Adapter
class JsonApi
class TopLevelJsonApiTest < Minitest::Test
def setup
@author = Author.new(id: 1, name: 'Steve K.')
@author.bio = nil
@author.roles = []
@blog = Blog.new(id: 23, name: 'AMS Blog')
@post = Post.new(id: 42, title: 'New Post', body: 'Body')
@anonymous_post = Post.new(id: 43, title: 'Hello!!', body: 'Hello, world!!')
@comment = Comment.new(id: 1, body: 'ZOMG A COMMENT')
@post.comments = [@comment]
@post.blog = @blog
@anonymous_post.comments = []
@anonymous_post.blog = nil
@comment.post = @post
@comment.author = nil
@post.author = @author
@anonymous_post.author = nil
@blog = Blog.new(id: 1, name: 'My Blog!!')
@blog.writer = @author
@blog.articles = [@post, @anonymous_post]
@author.posts = []
end

def test_toplevel_jsonapi_defaults_to_false
assert_equal config.fetch(:jsonapi_include_toplevel_object), false
end

def test_disable_toplevel_jsonapi
with_config(jsonapi_include_toplevel_object: false) do
hash = serialize(@post)
assert_nil(hash[:jsonapi])
end
end

def test_enable_toplevel_jsonapi
with_config(jsonapi_include_toplevel_object: true) do
hash = serialize(@post)
refute_nil(hash[:jsonapi])
end
end

def test_default_toplevel_jsonapi_version
with_config(jsonapi_include_toplevel_object: true) do
hash = serialize(@post)
assert_equal('1.0', hash[:jsonapi][:version])
end
end

def test_toplevel_jsonapi_no_meta
with_config(jsonapi_include_toplevel_object: true) do
hash = serialize(@post)
assert_nil(hash[:jsonapi][:meta])
end
end

def test_toplevel_jsonapi_meta
new_config = {
jsonapi_include_toplevel_object: true,
jsonapi_toplevel_meta: {
'copyright' => 'Copyright 2015 Example Corp.'
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

The value of meta must be a hash

with_config(new_config) do
hash = serialize(@post)
assert_equal(new_config[:jsonapi_toplevel_meta], hash.fetch(:jsonapi).fetch(:meta))
end
end

private

def serialize(resource, options = {})
serializable(resource, { adapter: :json_api }.merge!(options)).serializable_hash
end
end
end
end
end
end
16 changes: 16 additions & 0 deletions test/support/serialization_testing.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
module SerializationTesting
def config
ActiveModel::Serializer.config
end

private

def generate_cached_serializer(obj)
Expand All @@ -18,6 +22,18 @@ def with_adapter(adapter)
ActiveModel::Serializer.config.adapter = old_adapter
end
alias_method :with_configured_adapter, :with_adapter

def with_config(hash)
old_config = config.dup
ActiveModel::Serializer.config.update(hash)
yield
ensure
ActiveModel::Serializer.config.replace(old_config)
end

def serializable(resource, options = {})
ActiveModel::SerializableResource.new(resource, options)
end
end

class Minitest::Test
Expand Down