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

Support nested associations for Json and Attributes adapters + Refactor Attributes adapter #1127

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 @@ -23,6 +23,7 @@ Features:
* adds support for `pagination links` at top level of JsonApi adapter [@bacarini]
* adds extended format for `include` option to JsonApi adapter [@beauby]
* adds support for wildcards in `include` option [@beauby]
* adds support for nested associations for JSON and Attributes adapters via the `include` option [@NullVoxPopuli, @beauby]

Fixes:

Expand Down
67 changes: 36 additions & 31 deletions lib/active_model/serializer/adapter/attributes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,40 +9,12 @@ def initialize(serializer, options = {})

def serializable_hash(options = nil)
options ||= {}

if serializer.respond_to?(:each)
result = serializer.map { |s| Attributes.new(s, instance_options).serializable_hash(options) }
serializable_hash_for_collection(options)
else
hash = {}

core = cache_check(serializer) do
serializer.attributes(options)
end

serializer.associations(@include_tree).each do |association|
serializer = association.serializer
association_options = association.options

if serializer.respond_to?(:each)
array_serializer = serializer
hash[association.key] = array_serializer.map do |item|
cache_check(item) do
item.attributes(association_options)
end
end
else
hash[association.key] =
if serializer && serializer.object
cache_check(serializer) do
serializer.attributes(options)
end
elsif association_options[:virtual_value]
association_options[:virtual_value]
end
end
end
result = core.merge hash
serializable_hash_for_single_resource(options)
end
result
end

def fragment_cache(cached_hash, non_cached_hash)
Expand All @@ -51,10 +23,43 @@ def fragment_cache(cached_hash, non_cached_hash)

private

def serializable_hash_for_collection(options)
serializer.map { |s| Attributes.new(s, instance_options).serializable_hash(options) }
end

def serializable_hash_for_single_resource(options)
resource = resource_object_for(options)
relationships = resource_relationships(options)
resource.merge!(relationships)
end

def resource_relationships(options)
relationships = {}
serializer.associations(@include_tree).each do |association|
relationships[association.key] = relationship_value_for(association, options)
end

relationships
end

def relationship_value_for(association, options)
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

return association.options[:virtual_value] if association.options[:virtual_value]

if association.serializer && association.serializer.object
  opts = instance_options.merge(include: @include_tree[association.key])
  Attributes.new(association.serializer, opts).serializable_hash(options)
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess if this isn't the kind of method that is ever going to be overridden when it moves to Adapter::Base, I can get behind this. It at least looks a little better

Copy link
Contributor

Choose a reason for hiding this comment

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

There's very little chance, as the JsonApi adapter works in a very different way wrt relationships inclusion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

'fixed'

rubocop through a weird thing at me - and got around it by turning the if into a guard clause.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh yeah, right, rubocop favors guard clauses. Works as well.

return association.options[:virtual_value] if association.options[:virtual_value]
return unless association.serializer && association.serializer.object

opts = instance_options.merge(include: @include_tree[association.key])
Attributes.new(association.serializer, opts).serializable_hash(options)
end

# no-op: Attributes adapter does not include meta data, because it does not support root.
def include_meta(json)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this was here before. What is it used for?

Copy link
Member

Choose a reason for hiding this comment

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

guess :) include_meta includes meta. In the super class

      def include_meta(json)
        json[meta_key] = meta if meta
        json
      end

So, really, if we do a better job of making this a no-op by testing for an attribute, we wouldn't need it in the Attributes adapter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah.

json
end

def resource_object_for(options)
cache_check(serializer) do
serializer.attributes(options)
end
end
end
end
end
Expand Down
34 changes: 34 additions & 0 deletions lib/active_model/serializer/include_tree.rb
Original file line number Diff line number Diff line change
@@ -1,16 +1,48 @@
module ActiveModel
class Serializer
# TODO: description of this class, and overview of how it's used
class IncludeTree
module Parsing
module_function

# Translates a comma separated list of dot separated paths (JSON API format) into a Hash.
#
# @example
# `'posts.author, posts.comments.upvotes, posts.comments.author'`
#
# would become
#
# `{ posts: { author: {}, comments: { author: {}, upvotes: {} } } }`.
#
# @param [String] included
# @return [Hash] a Hash representing the same tree structure
def include_string_to_hash(included)
# TODO: Needs comment walking through the process of what all this is doing.
included.delete(' ').split(',').reduce({}) do |hash, path|
include_tree = path.split('.').reverse_each.reduce({}) { |a, e| { e.to_sym => a } }
hash.deep_merge!(include_tree)
end
end

# Translates the arguments passed to the include option into a Hash. The format can be either
# a String (see #include_string_to_hash), an Array of Symbols and Hashes, or a mix of both.
#
# @example
# `posts: [:author, comments: [:author, :upvotes]]`
#
# would become
#
# `{ posts: { author: {}, comments: { author: {}, upvotes: {} } } }`.
#
# @example
# `[:author, :comments => [:author]]`
#
# would become
#
# `{:author => {}, :comments => { author: {} } }`
#
# @param [Symbol, Hash, Array, String] included
# @return [Hash] a Hash representing the same tree structure
def include_args_to_hash(included)
case included
when Symbol
Expand Down Expand Up @@ -47,6 +79,8 @@ def self.from_string(included)
# @return [IncludeTree]
#
def self.from_include_args(included)
return included if included.is_a?(IncludeTree)

new(Parsing.include_args_to_hash(included))
end

Expand Down
167 changes: 167 additions & 0 deletions test/action_controller/json/include_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
require 'test_helper'

module ActionController
module Serialization
class Json
class IncludeTest < ActionController::TestCase
class IncludeTestController < ActionController::Base
def setup_data
ActionController::Base.cache_store.clear

@author = Author.new(id: 1, name: 'Steve K.')

@post = Post.new(id: 42, title: 'New Post', body: 'Body')
@first_comment = Comment.new(id: 1, body: 'ZOMG A COMMENT')
@second_comment = Comment.new(id: 2, body: 'ZOMG ANOTHER COMMENT')

@post.comments = [@first_comment, @second_comment]
@post.author = @author

@first_comment.post = @post
@second_comment.post = @post

@blog = Blog.new(id: 1, name: 'My Blog!!')
@post.blog = @blog
@author.posts = [@post]

@first_comment.author = @author
@second_comment.author = @author
@author.comments = [@first_comment, @second_comment]
@author.roles = []
@author.bio = {}
end

def render_without_include
setup_data
render json: @author, adapter: :json
end

def render_resource_with_include_hash
setup_data
render json: @author, include: { posts: :comments }, adapter: :json
end

def render_resource_with_include_string
setup_data
render json: @author, include: 'posts.comments', adapter: :json
end

def render_resource_with_deep_include
setup_data
render json: @author, include: 'posts.comments.author', adapter: :json
end
end

tests IncludeTestController

def test_render_without_include
get :render_without_include
response = JSON.parse(@response.body)
expected = {
'author' => {
'id' => 1,
'name' => 'Steve K.',
'posts' => [
{
'id' => 42, 'title' => 'New Post', 'body' => 'Body'
}
],
'roles' => [],
'bio' => {}
}
}

assert_equal(expected, response)
end

def test_render_resource_with_include_hash
get :render_resource_with_include_hash
response = JSON.parse(@response.body)
expected = {
'author' => {
'id' => 1,
'name' => 'Steve K.',
'posts' => [
{
'id' => 42, 'title' => 'New Post', 'body' => 'Body',
'comments' => [
{
'id' => 1, 'body' => 'ZOMG A COMMENT'
},
{
'id' => 2, 'body' => 'ZOMG ANOTHER COMMENT'
}
]
}
]
}
}

assert_equal(expected, response)
end

def test_render_resource_with_include_string
get :render_resource_with_include_string

response = JSON.parse(@response.body)
expected = {
'author' => {
'id' => 1,
'name' => 'Steve K.',
'posts' => [
{
'id' => 42, 'title' => 'New Post', 'body' => 'Body',
'comments' => [
{
'id' => 1, 'body' => 'ZOMG A COMMENT'
},
{
'id' => 2, 'body' => 'ZOMG ANOTHER COMMENT'
}
]
}
]
}
}

assert_equal(expected, response)
end

def test_render_resource_with_deep_include
get :render_resource_with_deep_include

response = JSON.parse(@response.body)
expected = {
'author' => {
'id' => 1,
'name' => 'Steve K.',
'posts' => [
{
'id' => 42, 'title' => 'New Post', 'body' => 'Body',
'comments' => [
{
'id' => 1, 'body' => 'ZOMG A COMMENT',
'author' => {
'id' => 1,
'name' => 'Steve K.'
}
},
{
'id' => 2, 'body' => 'ZOMG ANOTHER COMMENT',
'author' => {
'id' => 1,
'name' => 'Steve K.'
}
}
]
}
]
}
}

assert_equal(expected, response)
end
end
end
end
end
51 changes: 51 additions & 0 deletions test/include_tree/include_args_to_hash_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
require 'test_helper'
Copy link
Contributor

Choose a reason for hiding this comment

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

👍


module ActiveModel
class Serializer
class IncludeTree
module Parsing
class IncludeArgsToHashTest < MiniTest::Test
def test_include_args_to_hash_from_symbol
expected = { author: {} }
input = :author
actual = Parsing.include_args_to_hash(input)

assert_equal(expected, actual)
end

def test_include_args_to_hash_from_array
expected = { author: {}, comments: {} }
input = [:author, :comments]
actual = Parsing.include_args_to_hash(input)

assert_equal(expected, actual)
end

def test_include_args_to_hash_from_nested_array
expected = { author: {}, comments: { author: {} } }
input = [:author, comments: [:author]]
actual = Parsing.include_args_to_hash(input)

assert_equal(expected, actual)
end

def test_include_args_to_hash_from_array_of_hashes
expected = {
author: {},
blogs: { posts: { contributors: {} } },
comments: { author: { blogs: { posts: {} } } }
}
input = [
:author,
blogs: [posts: :contributors],
comments: { author: { blogs: :posts } }
]
actual = Parsing.include_args_to_hash(input)

assert_equal(expected, actual)
end
end
end
end
end
end