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

Extract IncludeTree. #1685

Merged
merged 1 commit into from
May 28, 2016
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 @@ -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)

Expand Down
2 changes: 2 additions & 0 deletions active_model_serializers.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion docs/jsonapi/schema.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/active_model/serializer.rb
Original file line number Diff line number Diff line change
@@ -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'
Expand Down
8 changes: 4 additions & 4 deletions lib/active_model/serializer/associations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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<Association>]
#
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
Expand Down
10 changes: 5 additions & 5 deletions lib/active_model/serializer/caching.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?

Expand All @@ -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)
Expand Down
9 changes: 4 additions & 5 deletions lib/active_model_serializers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
20 changes: 11 additions & 9 deletions lib/active_model_serializers/adapter/attributes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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)
Expand All @@ -38,7 +40,7 @@ def serializable_hash_for_single_resource(options)

def resource_relationships(options)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unrelated to this PR but this method could be rewritten as follows:

def resource_relationships(options)
  serializer.associations(@include_directive)
            .each_with_object({}) do |association, hash|
    hash[association.key] = relationship_value_for(association, options)
  end
end

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

Expand All @@ -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
Expand All @@ -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)
Expand Down
22 changes: 12 additions & 10 deletions lib/active_model_serializers/adapter/json_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand All @@ -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}
Expand Down Expand Up @@ -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,
Expand Down
12 changes: 6 additions & 6 deletions test/action_controller/json/include_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions test/cache_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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}" }
Expand Down
26 changes: 0 additions & 26 deletions test/include_tree/from_include_args_test.rb

This file was deleted.

94 changes: 0 additions & 94 deletions test/include_tree/from_string_test.rb

This file was deleted.

Loading