Skip to content

Splitting json adapter into two #958

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 14 commits into from
Jun 16, 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
2 changes: 1 addition & 1 deletion lib/action_controller/serialization.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module Serialization

include ActionController::Renderers

ADAPTER_OPTION_KEYS = [:include, :fields, :root, :adapter]
ADAPTER_OPTION_KEYS = [:include, :fields, :adapter]
Copy link
Member

Choose a reason for hiding this comment

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

This is a pretty big change. Prior to this, the root option always went to the adapter. Now it will always go to the serializer. Which code path should be dead? root to serializer or root to adapter?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah @bf4 this is indeed a big change.
The idea is that root shouldn't be an option, but another adapter by itself.
This make it even easier to integrate with some js frameworks.
We also removed the option to set a specific root key, so the root isn't "going" to the serializer, but is the serializer the one that knows what it should be and the adapter the one the uses it or not.

Copy link
Member

Choose a reason for hiding this comment

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

I'll have to read through the code some more to make sure I understand the changes as they affect tests that were deleted or whose expectations were changed. It didn't break #954 (once I rebased it), but I think this is the kind of thing we should include in the README between 'how it works' and 'intended usage'...

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, we realized we forgot about the README, but I'm already updating it.
I'm glad it didn't broke #954 😄


included do
class_attribute :_serialization_scope
Expand Down
16 changes: 2 additions & 14 deletions lib/active_model/serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -145,14 +145,6 @@ def self.adapter
adapter_class
end

def self._root
@@root ||= false
end

def self._root=(root)
@@root = root
end

def self.root_name
name.demodulize.underscore.sub(/_serializer$/, '') if name
end
Expand All @@ -162,7 +154,7 @@ def self.root_name
def initialize(object, options = {})
@object = object
@options = options
@root = options[:root] || (self.class._root ? self.class.root_name : false)
@root = options[:root]
@meta = options[:meta]
@meta_key = options[:meta_key]
@scope = options[:scope]
Expand All @@ -176,11 +168,7 @@ def initialize(object, options = {})
end

def json_key
if root == true || root.nil?
self.class.root_name
else
root
end
self.class.root_name
end

def id
Expand Down
10 changes: 6 additions & 4 deletions lib/active_model/serializer/adapter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ class Serializer
class Adapter
extend ActiveSupport::Autoload
autoload :Json
autoload :FlattenJson
autoload :Null
autoload :JsonApi

Expand All @@ -21,7 +22,8 @@ def serializable_hash(options = {})

def as_json(options = {})
hash = serializable_hash(options)
include_meta(hash)
include_meta(hash) unless self.class == FlattenJson
hash
end

def self.create(resource, options = {})
Expand All @@ -48,7 +50,7 @@ def cache_check(serializer)
yield
end
elsif is_fragment_cached?
FragmentCache.new(self, @cached_serializer, @options, @root).fetch
FragmentCache.new(self, @cached_serializer, @options).fetch
else
yield
end
Expand Down Expand Up @@ -82,11 +84,11 @@ def meta_key
end

def root
@options.fetch(:root) { serializer.json_key }
serializer.json_key.to_sym if serializer.json_key
end

def include_meta(json)
json[meta_key] = meta if meta && root
json[meta_key] = meta if meta
json
end
end
Expand Down
12 changes: 12 additions & 0 deletions lib/active_model/serializer/adapter/flatten_json.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
module ActiveModel
class Serializer
class Adapter
class FlattenJson < Json
def serializable_hash(options = {})
super
@result
end
end
end
end
end
3 changes: 1 addition & 2 deletions lib/active_model/serializer/adapter/fragment_cache.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@ class FragmentCache

attr_reader :serializer

def initialize(adapter, serializer, options, root)
@root = root
def initialize(adapter, serializer, options)
@options = options
@adapter = adapter
@serializer = serializer
Expand Down
15 changes: 6 additions & 9 deletions lib/active_model/serializer/adapter/json.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ class Adapter
class Json < Adapter
def serializable_hash(options = {})
if serializer.respond_to?(:each)
@result = serializer.map{|s| self.class.new(s).serializable_hash }
@result = serializer.map{|s| FlattenJson.new(s).serializable_hash }
else
@hash = {}

Expand Down Expand Up @@ -37,16 +37,13 @@ def serializable_hash(options = {})
@result = @core.merge @hash
end

if root
@result = { root => @result }
else
@result
end
{ root => @result }
end

def fragment_cache(cached_hash, non_cached_hash)
Json::FragmentCache.new().fragment_cache(cached_hash, non_cached_hash)
end
end

def fragment_cache(cached_hash, non_cached_hash)
Json::FragmentCache.new().fragment_cache(cached_hash, non_cached_hash)
end
end
end
Expand Down
1 change: 0 additions & 1 deletion lib/active_model/serializer/adapter/json_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ class Adapter
class JsonApi < Adapter
def initialize(serializer, options = {})
super
serializer.root = true
@hash = { data: [] }

if fields = options.delete(:fields)
Expand Down
11 changes: 7 additions & 4 deletions lib/active_model/serializer/array_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,8 @@ class ArraySerializer
attr_reader :meta, :meta_key

def initialize(objects, options = {})
options.merge!(root: nil)

@objects = objects.map do |object|
@resource = objects
@objects = objects.map do |object|
serializer_class = options.fetch(
:serializer,
ActiveModel::Serializer.serializer_for(object)
Expand All @@ -21,7 +20,11 @@ def initialize(objects, options = {})
end

def json_key
@objects.first.json_key if @objects.first
if @objects.first
@objects.first.json_key.pluralize
else
@resource.name.downcase.pluralize if @resource.try(:name)
end
end

def root=(root)
Expand Down
2 changes: 1 addition & 1 deletion lib/active_model/serializer/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module Configuration

included do |base|
base.config.array_serializer = ActiveModel::Serializer::ArraySerializer
base.config.adapter = :json
base.config.adapter = :flatten_json
end
end
end
Expand Down
4 changes: 2 additions & 2 deletions lib/active_model/serializer/fieldset.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ def fields
end

def fields_for(serializer)
key = serializer.json_key || serializer.class.root_name
fields[key.to_sym]
key = serializer.json_key
fields[key.to_sym] || fields[key.pluralize.to_sym]
end

private
Expand Down
10 changes: 4 additions & 6 deletions test/action_controller/explicit_serializer_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,10 @@ def test_render_array_using_explicit_serializer
get :render_array_using_explicit_serializer
assert_equal 'application/json', @response.content_type

expected = {
'paginated' => [
{ 'name' => 'Name 1' },
{ 'name' => 'Name 2' }
]
}
expected = [
{ 'name' => 'Name 1' },
{ 'name' => 'Name 2' }
]

assert_equal expected.to_json, @response.body
end
Expand Down
57 changes: 15 additions & 42 deletions test/action_controller/serialization_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,17 @@ def render_using_implicit_serializer
end

def render_using_custom_root
@profile = Profile.new({ name: 'Name 1', description: 'Description 1', comments: 'Comments 1' })
render json: @profile, root: "custom_root"
with_adapter ActiveModel::Serializer::Adapter::Json do
@profile = Profile.new({ name: 'Name 1', description: 'Description 1', comments: 'Comments 1' })
render json: @profile, root: "custom_root"
end
end

def render_using_custom_root_and_meta
@profile = Profile.new({ name: 'Name 1', description: 'Description 1', comments: 'Comments 1' })
render json: @profile, root: "custom_root", meta: { total: 10 }
with_adapter ActiveModel::Serializer::Adapter::Json do
@profile = Profile.new({ name: 'Name 1', description: 'Description 1', comments: 'Comments 1' })
render json: @profile, root: "custom_root", meta: { total: 10 }
end
end

def render_using_default_adapter_root
Expand All @@ -34,11 +38,13 @@ def render_using_custom_root_in_adapter_with_a_default
end

def render_array_using_custom_root_and_meta
array = [
Profile.new({ name: 'Name 1', description: 'Description 1', comments: 'Comments 1' }),
Profile.new({ name: 'Name 2', description: 'Description 2', comments: 'Comments 2' })
]
render json: array, root: "custom_root", meta: { total: 10 }
with_adapter ActiveModel::Serializer::Adapter::Json do
array = [
Profile.new({ name: 'Name 1', description: 'Description 1', comments: 'Comments 1' }),
Profile.new({ name: 'Name 2', description: 'Description 2', comments: 'Comments 2' })
]
render json: array, root: "custom_root", meta: { total: 10 }
end
end

def render_array_using_implicit_serializer
Expand Down Expand Up @@ -167,20 +173,6 @@ def test_render_using_implicit_serializer
assert_equal expected.to_json, @response.body
end

def test_render_using_custom_root
get :render_using_custom_root

assert_equal 'application/json', @response.content_type
assert_equal '{"custom_root":{"name":"Name 1","description":"Description 1"}}', @response.body
end

def test_render_using_custom_root_and_meta
get :render_using_custom_root_and_meta

assert_equal 'application/json', @response.content_type
assert_equal '{"custom_root":{"name":"Name 1","description":"Description 1"},"meta":{"total":10}}', @response.body
end

def test_render_using_default_root
get :render_using_default_adapter_root

Expand Down Expand Up @@ -217,25 +209,6 @@ def test_render_using_custom_root_in_adapter_with_a_default
assert_equal expected.to_json, @response.body
end

def test_render_array_using_custom_root_and_meta
get :render_array_using_custom_root_and_meta
assert_equal 'application/json', @response.content_type

expected = { custom_root: [
{
name: 'Name 1',
description: 'Description 1',
},
{
name: 'Name 2',
description: 'Description 2',
}],
meta: { total: 10 }
}

assert_equal expected.to_json, @response.body
end

def test_render_array_using_implicit_serializer
get :render_array_using_implicit_serializer
assert_equal 'application/json', @response.content_type
Expand Down
2 changes: 1 addition & 1 deletion test/adapter/fragment_cache_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ def setup
@role = Role.new(name: 'Great Author', description:nil)
@role.author = [@author]
@role_serializer = RoleSerializer.new(@role)
@role_hash = FragmentCache.new(RoleSerializer.adapter.new(@role_serializer), @role_serializer, {}, nil)
@role_hash = FragmentCache.new(RoleSerializer.adapter.new(@role_serializer), @role_serializer, {})
end

def test_fragment_fetch_with_virtual_attributes
Expand Down
6 changes: 3 additions & 3 deletions test/adapter/json/belongs_to_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,21 +25,21 @@ def setup
end

def test_includes_post
assert_equal({id: 42, title: 'New Post', body: 'Body'}, @adapter.serializable_hash[:post])
assert_equal({id: 42, title: 'New Post', body: 'Body'}, @adapter.serializable_hash[:comment][:post])
end

def test_include_nil_author
serializer = PostSerializer.new(@anonymous_post)
adapter = ActiveModel::Serializer::Adapter::Json.new(serializer)

assert_equal({title: "Hello!!", body: "Hello, world!!", id: 43, comments: [], blog: {id: 999, name: "Custom blog"}, author: nil}, adapter.serializable_hash)
assert_equal({post: {title: "Hello!!", body: "Hello, world!!", id: 43, comments: [], blog: {id: 999, name: "Custom blog"}, author: nil}}, adapter.serializable_hash)
end

def test_include_nil_author_with_specified_serializer
serializer = PostPreviewSerializer.new(@anonymous_post)
adapter = ActiveModel::Serializer::Adapter::Json.new(serializer)

assert_equal({title: "Hello!!", body: "Hello, world!!", id: 43, comments: [], author: nil}, adapter.serializable_hash)
assert_equal({posts: {title: "Hello!!", body: "Hello, world!!", id: 43, comments: [], author: nil}}, adapter.serializable_hash)
end
end
end
Expand Down
8 changes: 4 additions & 4 deletions test/adapter/json/collection_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,16 @@ def test_with_serializer_option
@serializer = ArraySerializer.new([@blog], serializer: CustomBlogSerializer)
@adapter = ActiveModel::Serializer::Adapter::Json.new(@serializer)

expected = [{
expected = {custom_blogs:[{
id: 1,
special_attribute: "Special",
articles: [{id: 1,title: "Hello!!", body: "Hello, world!!"}, {id: 2, title: "New Post", body: "Body"}]
}]
}]}
assert_equal expected, @adapter.serializable_hash
end

def test_include_multiple_posts
expected = [{
expected = { posts: [{
title: "Hello!!",
body: "Hello, world!!",
id: 1,
Expand All @@ -63,7 +63,7 @@ def test_include_multiple_posts
id: 999,
name: "Custom blog"
}
}]
}]}
assert_equal expected, @adapter.serializable_hash
end
end
Expand Down
2 changes: 1 addition & 1 deletion test/adapter/json/has_many_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def test_has_many
assert_equal([
{id: 1, body: 'ZOMG A COMMENT'},
{id: 2, body: 'ZOMG ANOTHER COMMENT'}
], @adapter.serializable_hash[:comments])
], @adapter.serializable_hash[:post][:comments])
end
end
end
Expand Down
1 change: 0 additions & 1 deletion test/adapter/json_api/collection_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ def test_limiting_fields
}
}
]

assert_equal(expected, @adapter.serializable_hash[:data])
end

Expand Down
2 changes: 1 addition & 1 deletion test/adapter/json_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def test_has_many
assert_equal([
{id: 1, body: 'ZOMG A COMMENT'},
{id: 2, body: 'ZOMG ANOTHER COMMENT'}
], @adapter.serializable_hash[:comments])
], @adapter.serializable_hash[:post][:comments])
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion test/adapter_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def test_adapter_class_for_unknown_adapter

def test_create_adapter
adapter = ActiveModel::Serializer::Adapter.create(@serializer)
assert_equal ActiveModel::Serializer::Adapter::Json, adapter.class
assert_equal ActiveModel::Serializer::Adapter::FlattenJson, adapter.class
end

def test_create_adapter_with_override
Expand Down
Loading