From 6035c9de69b9c039a98c3cdaec7dd0c650807207 Mon Sep 17 00:00:00 2001 From: Lucas Hosseini Date: Thu, 14 Apr 2016 15:49:38 +0200 Subject: [PATCH] Extract IncludeTree. --- CHANGELOG.md | 1 + active_model_serializers.gemspec | 2 + docs/jsonapi/schema.md | 2 +- lib/active_model/serializer.rb | 2 +- lib/active_model/serializer/associations.rb | 8 +- lib/active_model/serializer/caching.rb | 10 +- lib/active_model_serializers.rb | 9 +- .../adapter/attributes.rb | 20 ++-- .../adapter/json_api.rb | 22 +++-- test/action_controller/json/include_test.rb | 12 +-- test/cache_test.rb | 4 +- test/include_tree/from_include_args_test.rb | 26 ----- test/include_tree/from_string_test.rb | 94 ------------------- .../include_tree/include_args_to_hash_test.rb | 64 ------------- 14 files changed, 49 insertions(+), 227 deletions(-) delete mode 100644 test/include_tree/from_include_args_test.rb delete mode 100644 test/include_tree/from_string_test.rb delete mode 100644 test/include_tree/include_args_to_hash_test.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index abb7b34ea..5cb573f24 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ Fixes: Misc: - [#1734](https://github.com/rails-api/active_model_serializers/pull/1734) Adds documentation for conditional attribute (@lambda2) +- [#1685](https://github.com/rails-api/active_model_serializers/pull/1685) Replace `IncludeTree` with `IncludeDirective` from the jsonapi gem. ### [v0.10.0 (2016-05-17)](https://github.com/rails-api/active_model_serializers/compare/4a2d9853ba7...v0.10.0) diff --git a/active_model_serializers.gemspec b/active_model_serializers.gemspec index 4680735cf..dc37571a6 100644 --- a/active_model_serializers.gemspec +++ b/active_model_serializers.gemspec @@ -42,6 +42,8 @@ Gem::Specification.new do |spec| # 'minitest' # 'thread_safe' + spec.add_runtime_dependency 'jsonapi', '~> 0.1.1.beta2' + spec.add_development_dependency 'activerecord', rails_versions # arel # activesupport diff --git a/docs/jsonapi/schema.md b/docs/jsonapi/schema.md index 586fd8cdd..baffe3584 100644 --- a/docs/jsonapi/schema.md +++ b/docs/jsonapi/schema.md @@ -28,7 +28,7 @@ Example supported requests - Relationships - GET /articles/1/relationships/comments - GET /articles/1/relationships/author -- Optional: [Inclusion of related resources](http://jsonapi.org/format/#fetching-includes) `ActiveModel::Serializer::IncludeTree` +- Optional: [Inclusion of related resources](http://jsonapi.org/format/#fetching-includes) `JSONAPI::IncludeDirective` - GET /articles/1?`include`=comments - GET /articles/1?`include`=comments.author - GET /articles/1?`include`=author,comments.author diff --git a/lib/active_model/serializer.rb b/lib/active_model/serializer.rb index 31b80af70..50ac2bbda 100644 --- a/lib/active_model/serializer.rb +++ b/lib/active_model/serializer.rb @@ -1,9 +1,9 @@ require 'thread_safe' +require 'jsonapi/include_directive' 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' require 'active_model/serializer/caching' diff --git a/lib/active_model/serializer/associations.rb b/lib/active_model/serializer/associations.rb index 78448ea73..dcad7ea7f 100644 --- a/lib/active_model/serializer/associations.rb +++ b/lib/active_model/serializer/associations.rb @@ -78,18 +78,18 @@ def associate(reflection) end end - # @param [IncludeTree] include_tree (defaults to the - # default_includes config value when not provided) + # @param [JSONAPI::IncludeDirective] include_directive (defaults to the + # +default_include_directive+ config value when not provided) # @return [Enumerator] # - def associations(include_tree = ActiveModelSerializers.default_include_tree) + def associations(include_directive = ActiveModelSerializers.default_include_directive) return unless object Enumerator.new do |y| self.class._reflections.each do |reflection| next if reflection.excluded?(self) key = reflection.options.fetch(:key, reflection.name) - next unless include_tree.key?(key) + next unless include_directive.key?(key) y.yield reflection.build_association(self, instance_options) end end diff --git a/lib/active_model/serializer/caching.rb b/lib/active_model/serializer/caching.rb index b3663d631..850c346bb 100644 --- a/lib/active_model/serializer/caching.rb +++ b/lib/active_model/serializer/caching.rb @@ -163,10 +163,10 @@ def fragment_cache_enabled? # Read cache from cache_store # @return [Hash] - def cache_read_multi(collection_serializer, adapter_instance, include_tree) + def cache_read_multi(collection_serializer, adapter_instance, include_directive) return {} if ActiveModelSerializers.config.cache_store.blank? - keys = object_cache_keys(collection_serializer, adapter_instance, include_tree) + keys = object_cache_keys(collection_serializer, adapter_instance, include_directive) return {} if keys.blank? @@ -176,15 +176,15 @@ def cache_read_multi(collection_serializer, adapter_instance, include_tree) # Find all cache_key for the collection_serializer # @param serializers [ActiveModel::Serializer::CollectionSerializer] # @param adapter_instance [ActiveModelSerializers::Adapter::Base] - # @param include_tree [ActiveModel::Serializer::IncludeTree] + # @param include_directive [JSONAPI::IncludeDirective] # @return [Array] all cache_key of collection_serializer - def object_cache_keys(collection_serializer, adapter_instance, include_tree) + def object_cache_keys(collection_serializer, adapter_instance, include_directive) cache_keys = [] collection_serializer.each do |serializer| cache_keys << object_cache_key(serializer, adapter_instance) - serializer.associations(include_tree).each do |association| + serializer.associations(include_directive).each do |association| if association.serializer.respond_to?(:each) association.serializer.each do |sub_serializer| cache_keys << object_cache_key(sub_serializer, adapter_instance) diff --git a/lib/active_model_serializers.rb b/lib/active_model_serializers.rb index 8e7b5fa2e..76a32c497 100644 --- a/lib/active_model_serializers.rb +++ b/lib/active_model_serializers.rb @@ -31,11 +31,10 @@ def self.location_of_caller [file, lineno] end - # Memoized default include tree - # @return [ActiveModel::Serializer::IncludeTree] - def self.default_include_tree - @default_include_tree ||= ActiveModel::Serializer::IncludeTree - .from_include_args(config.default_includes) + # Memoized default include directive + # @return [JSONAPI::IncludeDirective] + def self.default_include_directive + @default_include_directive ||= JSONAPI::IncludeDirective.new(config.default_includes, allow_wildcard: true) end require 'active_model/serializer/version' diff --git a/lib/active_model_serializers/adapter/attributes.rb b/lib/active_model_serializers/adapter/attributes.rb index e30d2efb9..503f0245e 100644 --- a/lib/active_model_serializers/adapter/attributes.rb +++ b/lib/active_model_serializers/adapter/attributes.rb @@ -4,11 +4,13 @@ class Attributes < Base def initialize(serializer, options = {}) super @cached_attributes = options[:cache_attributes] || {} - @include_tree = - if options[:include] - ActiveModel::Serializer::IncludeTree.from_include_args(options[:include]) + @include_directive = + if options[:include_directive] + options[:include_directive] + elsif options[:include] + JSONAPI::IncludeDirective.new(options[:include], allow_wildcard: true) else - ActiveModelSerializers.default_include_tree + ActiveModelSerializers.default_include_directive end end @@ -26,8 +28,8 @@ def serializable_hash(options = nil) def serializable_hash_for_collection(options) cache_attributes - - serializer.map { |s| Attributes.new(s, instance_options).serializable_hash(options) } + opts = instance_options.merge(include_directive: @include_directive) + serializer.map { |s| Attributes.new(s, opts).serializable_hash(options) } end def serializable_hash_for_single_resource(options) @@ -38,7 +40,7 @@ def serializable_hash_for_single_resource(options) def resource_relationships(options) relationships = {} - serializer.associations(@include_tree).each do |association| + serializer.associations(@include_directive).each do |association| relationships[association.key] ||= relationship_value_for(association, options) end @@ -49,7 +51,7 @@ def relationship_value_for(association, options) 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]) + opts = instance_options.merge(include_directive: @include_directive[association.key]) relationship_value = Attributes.new(association.serializer, opts).serializable_hash(options) if association.options[:polymorphic] && relationship_value @@ -64,7 +66,7 @@ def relationship_value_for(association, options) def cache_attributes return if @cached_attributes.present? - @cached_attributes = ActiveModel::Serializer.cache_read_multi(serializer, self, @include_tree) + @cached_attributes = ActiveModel::Serializer.cache_read_multi(serializer, self, @include_directive) end def resource_object_for(options) diff --git a/lib/active_model_serializers/adapter/json_api.rb b/lib/active_model_serializers/adapter/json_api.rb index 8085bc938..a62caab49 100644 --- a/lib/active_model_serializers/adapter/json_api.rb +++ b/lib/active_model_serializers/adapter/json_api.rb @@ -33,7 +33,7 @@ class JsonApi < Base def initialize(serializer, options = {}) super - @include_tree = ActiveModel::Serializer::IncludeTree.from_include_args(options[:include]) + @include_directive = JSONAPI::IncludeDirective.new(options[:include], allow_wildcard: true) @fieldset = options[:fieldset] || ActiveModel::Serializer::Fieldset.new(options.delete(:fields)) end @@ -232,7 +232,7 @@ def resource_objects_for(serializers) @included = [] @resource_identifiers = Set.new serializers.each { |serializer| process_resource(serializer, true) } - serializers.each { |serializer| process_relationships(serializer, @include_tree) } + serializers.each { |serializer| process_relationships(serializer, @include_directive) } [@primary, @included] end @@ -251,21 +251,21 @@ def process_resource(serializer, primary) true end - def process_relationships(serializer, include_tree) - serializer.associations(include_tree).each do |association| - process_relationship(association.serializer, include_tree[association.key]) + def process_relationships(serializer, include_directive) + serializer.associations(include_directive).each do |association| + process_relationship(association.serializer, include_directive[association.key]) end end - def process_relationship(serializer, include_tree) + def process_relationship(serializer, include_directive) if serializer.respond_to?(:each) - serializer.each { |s| process_relationship(s, include_tree) } + serializer.each { |s| process_relationship(s, include_directive) } return end return unless serializer && serializer.object return unless process_resource(serializer, false) - process_relationships(serializer, include_tree) + process_relationships(serializer, include_directive) end # {http://jsonapi.org/format/#document-resource-object-attributes Document Resource Object Attributes} @@ -429,8 +429,10 @@ def resource_object_for(serializer) # meta: meta # }.reject! {|_,v| v.nil? } def relationships_for(serializer, requested_associations) - include_tree = ActiveModel::Serializer::IncludeTree.from_include_args(requested_associations) - serializer.associations(include_tree).each_with_object({}) do |association, hash| + include_directive = JSONAPI::IncludeDirective.new( + requested_associations, + allow_wildcard: true) + serializer.associations(include_directive).each_with_object({}) do |association, hash| hash[association.key] = Relationship.new( serializer, association.serializer, diff --git a/test/action_controller/json/include_test.rb b/test/action_controller/json/include_test.rb index 9d0512d15..1fc8863e5 100644 --- a/test/action_controller/json/include_test.rb +++ b/test/action_controller/json/include_test.rb @@ -226,19 +226,19 @@ def expected_deep_include_response } end - def with_default_includes(include_tree) + def with_default_includes(include_directive) original = ActiveModelSerializers.config.default_includes - ActiveModelSerializers.config.default_includes = include_tree - clear_include_tree_cache + ActiveModelSerializers.config.default_includes = include_directive + clear_include_directive_cache yield ensure ActiveModelSerializers.config.default_includes = original - clear_include_tree_cache + clear_include_directive_cache end - def clear_include_tree_cache + def clear_include_directive_cache ActiveModelSerializers - .instance_variable_set(:@default_include_tree, nil) + .instance_variable_set(:@default_include_directive, nil) end end end diff --git a/test/cache_test.rb b/test/cache_test.rb index 57173d618..ca668de7d 100644 --- a/test/cache_test.rb +++ b/test/cache_test.rb @@ -286,9 +286,9 @@ def test_cache_digest_definition def test_object_cache_keys serializable = ActiveModelSerializers::SerializableResource.new([@comment, @comment]) - include_tree = ActiveModel::Serializer::IncludeTree.from_include_args('*') + include_directive = JSONAPI::IncludeDirective.new('*', allow_wildcard: true) - actual = ActiveModel::Serializer.object_cache_keys(serializable.adapter.serializer, serializable.adapter, include_tree) + actual = ActiveModel::Serializer.object_cache_keys(serializable.adapter.serializer, serializable.adapter, include_directive) assert_equal 3, actual.size assert actual.any? { |key| key == "comment/1/#{serializable.adapter.cached_name}" } diff --git a/test/include_tree/from_include_args_test.rb b/test/include_tree/from_include_args_test.rb deleted file mode 100644 index b3b00e8cd..000000000 --- a/test/include_tree/from_include_args_test.rb +++ /dev/null @@ -1,26 +0,0 @@ -require 'test_helper' - -module ActiveModel - class Serializer - class IncludeTree - class FromStringTest < ActiveSupport::TestCase - 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 deleted file mode 100644 index 831429008..000000000 --- a/test/include_tree/from_string_test.rb +++ /dev/null @@ -1,94 +0,0 @@ -require 'test_helper' - -module ActiveModel - class Serializer - class IncludeTree - class FromStringTest < ActiveSupport::TestCase - 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/include_tree/include_args_to_hash_test.rb b/test/include_tree/include_args_to_hash_test.rb deleted file mode 100644 index 0922f5915..000000000 --- a/test/include_tree/include_args_to_hash_test.rb +++ /dev/null @@ -1,64 +0,0 @@ -require 'test_helper' - -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 - - def test_array_of_string - expected = { - comments: { author: {}, attachment: {} } - } - input = [ - 'comments.author', - 'comments.attachment' - ] - actual = Parsing.include_args_to_hash(input) - - assert_equal(expected, actual) - end - end - end - end - end -end