From ac06013aebe4ba92c23b1364421db0900a408cb8 Mon Sep 17 00:00:00 2001 From: Lucas Hosseini Date: Wed, 16 Sep 2015 08:45:56 +0200 Subject: [PATCH] Add support for wildcard includes + improve perfs on JsonApi includes. --- CHANGELOG.md | 1 + lib/active_model/serializer.rb | 2 +- .../serializer/adapter/attributes.rb | 7 +- .../serializer/adapter/json_api.rb | 52 +++++----- lib/active_model/serializer/associations.rb | 6 +- lib/active_model/serializer/include_tree.rb | 75 +++++++++++++++ lib/active_model/serializer/utils.rb | 35 ------- .../action_controller/json_api/linked_test.rb | 6 +- test/include_tree/from_include_args_test.rb | 26 +++++ test/include_tree/from_string_test.rb | 94 +++++++++++++++++++ test/utils/include_args_to_hash_test.rb | 79 ---------------- 11 files changed, 238 insertions(+), 145 deletions(-) create mode 100644 lib/active_model/serializer/include_tree.rb delete mode 100644 lib/active_model/serializer/utils.rb create mode 100644 test/include_tree/from_include_args_test.rb create mode 100644 test/include_tree/from_string_test.rb delete mode 100644 test/utils/include_args_to_hash_test.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index bc99a7c2..2f90e7fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ Features: * adds FlattenJSON as default adapter [@joaomdmoura] * 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] Fixes: diff --git a/lib/active_model/serializer.rb b/lib/active_model/serializer.rb index 62f42751..030e47d2 100644 --- a/lib/active_model/serializer.rb +++ b/lib/active_model/serializer.rb @@ -1,11 +1,11 @@ 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' require 'active_model/serializer/configuration' require 'active_model/serializer/fieldset' require 'active_model/serializer/lint' -require 'active_model/serializer/utils' module ActiveModel class Serializer diff --git a/lib/active_model/serializer/adapter/attributes.rb b/lib/active_model/serializer/adapter/attributes.rb index 09761a4a..4331f579 100644 --- a/lib/active_model/serializer/adapter/attributes.rb +++ b/lib/active_model/serializer/adapter/attributes.rb @@ -2,6 +2,11 @@ module ActiveModel class Serializer module Adapter class Attributes < Base + def initialize(serializer, options = {}) + super + @include_tree = IncludeTree.from_include_args(options[:include] || '*') + end + def serializable_hash(options = nil) options ||= {} if serializer.respond_to?(:each) @@ -13,7 +18,7 @@ module ActiveModel serializer.attributes(options) end - serializer.associations.each do |association| + serializer.associations(@include_tree).each do |association| serializer = association.serializer association_options = association.options diff --git a/lib/active_model/serializer/adapter/json_api.rb b/lib/active_model/serializer/adapter/json_api.rb index 7a028717..3637ccb7 100644 --- a/lib/active_model/serializer/adapter/json_api.rb +++ b/lib/active_model/serializer/adapter/json_api.rb @@ -8,7 +8,8 @@ module ActiveModel def initialize(serializer, options = {}) super - @included = ActiveModel::Serializer::Utils.include_args_to_hash(instance_options[:include]) + @include_tree = IncludeTree.from_include_args(options[:include]) + fields = options.delete(:fields) if fields @fieldset = ActiveModel::Serializer::Fieldset.new(fields, serializer.json_key) @@ -19,10 +20,11 @@ module ActiveModel def serializable_hash(options = nil) options ||= {} + if serializer.respond_to?(:each) - serializable_hash_for_collection(serializer, options) + serializable_hash_for_collection(options) else - serializable_hash_for_single_resource(serializer, options) + serializable_hash_for_single_resource(options) end end @@ -34,10 +36,10 @@ module ActiveModel private ActiveModel.silence_warnings do - attr_reader :included, :fieldset + attr_reader :fieldset end - def serializable_hash_for_collection(serializer, options) + def serializable_hash_for_collection(options) hash = { data: [] } serializer.each do |s| result = self.class.new(s, instance_options.merge(fieldset: fieldset)).serializable_hash(options) @@ -57,10 +59,10 @@ module ActiveModel hash end - def serializable_hash_for_single_resource(serializer, options) + def serializable_hash_for_single_resource(options) primary_data = primary_data_for(serializer, options) relationships = relationships_for(serializer) - included = included_for(serializer) + included = included_resources(@include_tree) hash = { data: primary_data } hash[:data][:relationships] = relationships if relationships.any? hash[:included] = included if included.any? @@ -123,37 +125,37 @@ module ActiveModel end def relationships_for(serializer) - Hash[serializer.associations.map { |association| [association.key, { data: relationship_value_for(association.serializer, association.options) }] }] + serializer.associations.each_with_object({}) do |association, hash| + hash[association.key] = { data: relationship_value_for(association.serializer, association.options) } + end end - def included_for(serializer) - included.flat_map { |inc| - association = serializer.associations.find { |assoc| assoc.key == inc.first } - _included_for(association.serializer, inc.second) if association - }.uniq + def included_resources(include_tree) + included = [] + + serializer.associations(include_tree).each do |association| + add_included_resources_for(association.serializer, include_tree[association.key], included) + end + + included end - def _included_for(serializer, includes) + def add_included_resources_for(serializer, include_tree, included) if serializer.respond_to?(:each) - serializer.flat_map { |s| _included_for(s, includes) }.uniq + serializer.each { |s| add_included_resources_for(s, include_tree, included) } else - return [] unless serializer && serializer.object + return unless serializer && serializer.object primary_data = primary_data_for(serializer, instance_options) relationships = relationships_for(serializer) primary_data[:relationships] = relationships if relationships.any? - included = [primary_data] + return if included.include?(primary_data) + included.push(primary_data) - includes.each do |inc| - association = serializer.associations.find { |assoc| assoc.key == inc.first } - if association - included.concat(_included_for(association.serializer, inc.second)) - included.uniq! - end + serializer.associations(include_tree).each do |association| + add_included_resources_for(association.serializer, include_tree[association.key], included) end - - included end end diff --git a/lib/active_model/serializer/associations.rb b/lib/active_model/serializer/associations.rb index 660ff0d1..86c5752c 100644 --- a/lib/active_model/serializer/associations.rb +++ b/lib/active_model/serializer/associations.rb @@ -10,6 +10,8 @@ module ActiveModel module Associations extend ActiveSupport::Concern + DEFAULT_INCLUDE_TREE = ActiveModel::Serializer::IncludeTree.from_string('*') + included do |base| class << base attr_accessor :_reflections @@ -82,13 +84,15 @@ module ActiveModel end end + # @param [IncludeTree] include_tree (defaults to all associations when not provided) # @return [Enumerator] # - def associations + def associations(include_tree = DEFAULT_INCLUDE_TREE) return unless object Enumerator.new do |y| self.class._reflections.each do |reflection| + next unless include_tree.key?(reflection.name) y.yield reflection.build_association(self, instance_options) end end diff --git a/lib/active_model/serializer/include_tree.rb b/lib/active_model/serializer/include_tree.rb new file mode 100644 index 00000000..6d675e11 --- /dev/null +++ b/lib/active_model/serializer/include_tree.rb @@ -0,0 +1,75 @@ +module ActiveModel + class Serializer + class IncludeTree + module Parsing + module_function + + def include_string_to_hash(included) + 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 + + def include_args_to_hash(included) + case included + when Symbol + { included => {} } + when Hash + included.each_with_object({}) { |(key, value), hash| + hash[key] = include_args_to_hash(value) + } + when Array + included.reduce({}) { |a, e| a.merge!(include_args_to_hash(e)) } + when String + include_string_to_hash(included) + else + {} + end + end + end + + # Builds an IncludeTree from a comma separated list of dot separated paths (JSON API format). + # @example `'posts.author, posts.comments.upvotes, posts.comments.author'` + # + # @param [String] included + # @return [IncludeTree] + # + def self.from_string(included) + new(Parsing.include_string_to_hash(included)) + end + + # Translates the arguments passed to the include option into an IncludeTree. + # The format can be either a String (see #from_string), an Array of Symbols and Hashes, or a mix of both. + # @example `posts: [:author, comments: [:author, :upvotes]]` + # + # @param [Symbol, Hash, Array, String] included + # @return [IncludeTree] + # + def self.from_include_args(included) + new(Parsing.include_args_to_hash(included)) + end + + # @param [Hash] hash + def initialize(hash = {}) + @hash = hash + end + + def key?(key) + @hash.key?(key) || @hash.key?(:*) || @hash.key?(:**) + end + + def [](key) + # TODO(beauby): Adopt a lazy caching strategy for generating subtrees. + case + when @hash.key?(key) + self.class.new(@hash[key]) + when @hash.key?(:*) + self.class.new(@hash[:*]) + when @hash.key?(:**) + self.class.new(:** => {}) + end + end + end + end +end diff --git a/lib/active_model/serializer/utils.rb b/lib/active_model/serializer/utils.rb deleted file mode 100644 index ca54f420..00000000 --- a/lib/active_model/serializer/utils.rb +++ /dev/null @@ -1,35 +0,0 @@ -module ActiveModel::Serializer::Utils - 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) - included.delete(' ').split(',').inject({}) do |hash, path| - hash.deep_merge!(path.split('.').reverse_each.inject({}) { |a, e| { e.to_sym => a } }) - 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: {} } } }`. - # - # @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 - { included => {} } - when Hash - included.each_with_object({}) { |(key, value), hash| hash[key] = include_args_to_hash(value) } - when Array - included.inject({}) { |a, e| a.merge!(include_args_to_hash(e)) } - when String - include_string_to_hash(included) - else - {} - end - end -end diff --git a/test/action_controller/json_api/linked_test.rb b/test/action_controller/json_api/linked_test.rb index ba317e21..04ea4c99 100644 --- a/test/action_controller/json_api/linked_test.rb +++ b/test/action_controller/json_api/linked_test.rb @@ -51,9 +51,9 @@ module ActionController render json: @post, include: [comments: [:author]], adapter: :json_api end - def render_resource_with_nested_has_many_include + def render_resource_with_nested_has_many_include_wildcard setup_post - render json: @post, include: 'author.roles', adapter: :json_api + render json: @post, include: 'author.*', adapter: :json_api end def render_resource_with_missing_nested_has_many_include @@ -96,7 +96,7 @@ module ActionController end def test_render_resource_with_nested_has_many_include - get :render_resource_with_nested_has_many_include + get :render_resource_with_nested_has_many_include_wildcard response = JSON.parse(@response.body) expected_linked = [ { diff --git a/test/include_tree/from_include_args_test.rb b/test/include_tree/from_include_args_test.rb new file mode 100644 index 00000000..e8eb01a8 --- /dev/null +++ b/test/include_tree/from_include_args_test.rb @@ -0,0 +1,26 @@ +require 'test_helper' + +module ActiveModel + class Serializer + class IncludeTree + class FromStringTest < Minitest::Test + def test_simple_array + input = [:comments, :author] + actual = ActiveModel::Serializer::IncludeTree.from_include_args(input) + assert(actual.key?(:author)) + assert(actual.key?(:comments)) + end + + def test_nested_array + input = [:comments, posts: [:author, comments: [:author]]] + actual = ActiveModel::Serializer::IncludeTree.from_include_args(input) + assert(actual.key?(:posts)) + assert(actual[:posts].key?(:author)) + assert(actual[:posts].key?(:comments)) + assert(actual[:posts][:comments].key?(:author)) + assert(actual.key?(:comments)) + end + end + end + end +end diff --git a/test/include_tree/from_string_test.rb b/test/include_tree/from_string_test.rb new file mode 100644 index 00000000..3468cd57 --- /dev/null +++ b/test/include_tree/from_string_test.rb @@ -0,0 +1,94 @@ +require 'test_helper' + +module ActiveModel + class Serializer + class IncludeTree + class FromStringTest < Minitest::Test + def test_single_string + input = 'author' + actual = ActiveModel::Serializer::IncludeTree.from_string(input) + assert(actual.key?(:author)) + end + + def test_multiple_strings + input = 'author,comments' + actual = ActiveModel::Serializer::IncludeTree.from_string(input) + assert(actual.key?(:author)) + assert(actual.key?(:comments)) + end + + def test_multiple_strings_with_space + input = 'author, comments' + actual = ActiveModel::Serializer::IncludeTree.from_string(input) + assert(actual.key?(:author)) + assert(actual.key?(:comments)) + end + + def test_nested_string + input = 'posts.author' + actual = ActiveModel::Serializer::IncludeTree.from_string(input) + assert(actual.key?(:posts)) + assert(actual[:posts].key?(:author)) + end + + def test_multiple_nested_string + input = 'posts.author,posts.comments.author,comments' + actual = ActiveModel::Serializer::IncludeTree.from_string(input) + assert(actual.key?(:posts)) + assert(actual[:posts].key?(:author)) + assert(actual[:posts].key?(:comments)) + assert(actual[:posts][:comments].key?(:author)) + assert(actual.key?(:comments)) + end + + def test_toplevel_star_string + input = '*' + actual = ActiveModel::Serializer::IncludeTree.from_string(input) + assert(actual.key?(:comments)) + end + + def test_nested_star_string + input = 'posts.*' + actual = ActiveModel::Serializer::IncludeTree.from_string(input) + assert(actual.key?(:posts)) + assert(actual[:posts].key?(:comments)) + end + + def test_nested_star_middle_string + input = 'posts.*.author' + actual = ActiveModel::Serializer::IncludeTree.from_string(input) + assert(actual.key?(:posts)) + assert(actual[:posts].key?(:comments)) + assert(actual[:posts][:comments].key?(:author)) + refute(actual[:posts][:comments].key?(:unspecified)) + end + + def test_nested_star_lower_precedence_string + input = 'posts.comments.author,posts.*' + actual = ActiveModel::Serializer::IncludeTree.from_string(input) + assert(actual.key?(:posts)) + assert(actual[:posts].key?(:comments)) + assert(actual[:posts][:comments].key?(:author)) + end + + def test_toplevel_double_star_string + input = '**' + actual = ActiveModel::Serializer::IncludeTree.from_string(input) + assert(actual.key?(:posts)) + assert(actual[:posts].key?(:comments)) + assert(actual[:posts][:comments].key?(:posts)) + end + + def test_nested_double_star_string + input = 'comments, posts.**' + actual = ActiveModel::Serializer::IncludeTree.from_string(input) + assert(actual.key?(:comments)) + refute(actual[:comments].key?(:author)) + assert(actual.key?(:posts)) + assert(actual[:posts].key?(:comments)) + assert(actual[:posts][:comments].key?(:posts)) + end + end + end + end +end diff --git a/test/utils/include_args_to_hash_test.rb b/test/utils/include_args_to_hash_test.rb deleted file mode 100644 index deb87f1c..00000000 --- a/test/utils/include_args_to_hash_test.rb +++ /dev/null @@ -1,79 +0,0 @@ -require 'test_helper' - -module ActiveModel - class Serializer - module Utils - class IncludeArgsToHashTest < Minitest::Test - def test_nil - input = nil - expected = {} - actual = ActiveModel::Serializer::Utils.include_args_to_hash(input) - assert_equal(expected, actual) - end - - def test_empty_string - input = '' - expected = {} - actual = ActiveModel::Serializer::Utils.include_args_to_hash(input) - assert_equal(expected, actual) - end - - def test_single_string - input = 'author' - expected = { author: {} } - actual = ActiveModel::Serializer::Utils.include_args_to_hash(input) - assert_equal(expected, actual) - end - - def test_multiple_strings - input = 'author,comments' - expected = { author: {}, comments: {} } - actual = ActiveModel::Serializer::Utils.include_args_to_hash(input) - assert_equal(expected, actual) - end - - def test_multiple_strings_with_space - input = 'author, comments' - expected = { author: {}, comments: {} } - actual = ActiveModel::Serializer::Utils.include_args_to_hash(input) - assert_equal(expected, actual) - end - - def test_nested_string - input = 'posts.author' - expected = { posts: { author: {} } } - actual = ActiveModel::Serializer::Utils.include_args_to_hash(input) - assert_equal(expected, actual) - end - - def test_multiple_nested_string - input = 'posts.author,posts.comments.author,comments' - expected = { posts: { author: {}, comments: { author: {} } }, comments: {} } - actual = ActiveModel::Serializer::Utils.include_args_to_hash(input) - assert_equal(expected, actual) - end - - def test_empty_array - input = [] - expected = {} - actual = ActiveModel::Serializer::Utils.include_args_to_hash(input) - assert_equal(expected, actual) - end - - def test_simple_array - input = [:comments, :author] - expected = { author: {}, comments: {} } - actual = ActiveModel::Serializer::Utils.include_args_to_hash(input) - assert_equal(expected, actual) - end - - def test_nested_array - input = [:comments, posts: [:author, comments: [:author]]] - expected = { posts: { author: {}, comments: { author: {} } }, comments: {} } - actual = ActiveModel::Serializer::Utils.include_args_to_hash(input) - assert_equal(expected, actual) - end - end - end - end -end