From 8700ac6124b8685b979b4a03570f71d498a18d36 Mon Sep 17 00:00:00 2001 From: Lee Richmond Date: Sat, 11 Jun 2016 11:50:45 -0400 Subject: [PATCH] Add include_data :if_sideloaded For JSONAPI, `include_data` currently means, "should we populate the 'data'" key for this relationship. Current options are true/false. This adds the `:if_sideloaded` option. This means "only populate the 'data' key when we are sideloading this relationship." This is because 'data' is often only relevant to sideloading, and causes a database hit. Addresses https://github.com/rails-api/active_model_serializers/issues/1555 --- CHANGELOG.md | 1 + .../serializer/concerns/associations.rb | 6 +- .../serializer/concerns/configuration.rb | 1 + lib/active_model/serializer/reflection.rb | 30 ++-- .../adapter/json_api.rb | 28 +-- .../include_data_if_sideloaded_test.rb | 166 ++++++++++++++++++ 6 files changed, 204 insertions(+), 28 deletions(-) create mode 100644 test/adapter/json_api/include_data_if_sideloaded_test.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index eb1173f07..1bf8af66f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ Features: - Added `jsonapi_namespace_separator` config option. - [#1889](https://github.com/rails-api/active_model_serializers/pull/1889) Support key transformation for Attributes adapter (@iancanderson, @danbee) - [#1917](https://github.com/rails-api/active_model_serializers/pull/1917) Add `jsonapi_pagination_links_enabled` configuration option (@richmolj) +- [#1797](https://github.com/rails-api/active_model_serializers/pull/1797) Only include 'relationships' when sideloading (@richmolj) Fixes: diff --git a/lib/active_model/serializer/concerns/associations.rb b/lib/active_model/serializer/concerns/associations.rb index c27dfeb89..ce0ea21ff 100644 --- a/lib/active_model/serializer/concerns/associations.rb +++ b/lib/active_model/serializer/concerns/associations.rb @@ -83,7 +83,8 @@ def associate(reflection) # +default_include_directive+ config value when not provided) # @return [Enumerator] # - def associations(include_directive = ActiveModelSerializers.default_include_directive) + def associations(include_directive = ActiveModelSerializers.default_include_directive, include_slice = nil) + include_slice ||= include_directive return unless object Enumerator.new do |y| @@ -91,7 +92,8 @@ def associations(include_directive = ActiveModelSerializers.default_include_dire next if reflection.excluded?(self) key = reflection.options.fetch(:key, reflection.name) next unless include_directive.key?(key) - y.yield reflection.build_association(self, instance_options) + + y.yield reflection.build_association(self, instance_options, include_slice) end end end diff --git a/lib/active_model/serializer/concerns/configuration.rb b/lib/active_model/serializer/concerns/configuration.rb index 2604033c4..c5e73e5b9 100644 --- a/lib/active_model/serializer/concerns/configuration.rb +++ b/lib/active_model/serializer/concerns/configuration.rb @@ -30,6 +30,7 @@ def config.array_serializer # Make JSON API top-level jsonapi member opt-in # ref: http://jsonapi.org/format/#document-top-level config.jsonapi_include_toplevel_object = false + config.include_data_default = true config.schema_path = 'test/support/schemas' end diff --git a/lib/active_model/serializer/reflection.rb b/lib/active_model/serializer/reflection.rb index a64a849a5..2bb5ccd55 100644 --- a/lib/active_model/serializer/reflection.rb +++ b/lib/active_model/serializer/reflection.rb @@ -37,7 +37,7 @@ class Reflection < Field def initialize(*) super @_links = {} - @_include_data = true + @_include_data = Serializer.config.include_data_default @_meta = nil end @@ -69,17 +69,15 @@ def include_data(value = true) # Blog.find(object.blog_id) # end # end - def value(serializer) + def value(serializer, include_slice) @object = serializer.object @scope = serializer.scope - if block - block_value = instance_exec(serializer, &block) - if block_value != :nil - block_value - elsif @_include_data - serializer.read_attribute_for_serialization(name) - end + block_value = instance_exec(serializer, &block) if block + return unless include_data?(include_slice) + + if block && block_value != :nil + block_value else serializer.read_attribute_for_serialization(name) end @@ -106,11 +104,11 @@ def value(serializer) # # @api private # - def build_association(parent_serializer, parent_serializer_options) - association_value = value(parent_serializer) + def build_association(parent_serializer, parent_serializer_options, include_slice = {}) + association_value = value(parent_serializer, include_slice) reflection_options = options.dup serializer_class = parent_serializer.class.serializer_for(association_value, reflection_options) - reflection_options[:include_data] = @_include_data + reflection_options[:include_data] = include_data?(include_slice) reflection_options[:links] = @_links reflection_options[:meta] = @_meta @@ -137,6 +135,14 @@ def build_association(parent_serializer, parent_serializer_options) private + def include_data?(include_slice) + if @_include_data == :if_sideloaded + include_slice.key?(name) + else + @_include_data + end + end + def serializer_options(parent_serializer, parent_serializer_options, reflection_options) serializer = reflection_options.fetch(:serializer, nil) diff --git a/lib/active_model_serializers/adapter/json_api.rb b/lib/active_model_serializers/adapter/json_api.rb index 203c12403..3d241e349 100644 --- a/lib/active_model_serializers/adapter/json_api.rb +++ b/lib/active_model_serializers/adapter/json_api.rb @@ -235,17 +235,17 @@ def resource_objects_for(serializers) @primary = [] @included = [] @resource_identifiers = Set.new - serializers.each { |serializer| process_resource(serializer, true) } + serializers.each { |serializer| process_resource(serializer, true, @include_directive) } serializers.each { |serializer| process_relationships(serializer, @include_directive) } [@primary, @included] end - def process_resource(serializer, primary) + def process_resource(serializer, primary, include_slice = {}) resource_identifier = ResourceIdentifier.new(serializer, instance_options).as_json return false unless @resource_identifiers.add?(resource_identifier) - resource_object = resource_object_for(serializer) + resource_object = resource_object_for(serializer, include_slice) if primary @primary << resource_object else @@ -255,21 +255,21 @@ def process_resource(serializer, primary) true end - def process_relationships(serializer, include_directive) - serializer.associations(include_directive).each do |association| - process_relationship(association.serializer, include_directive[association.key]) + def process_relationships(serializer, include_slice) + serializer.associations(include_slice).each do |association| + process_relationship(association.serializer, include_slice[association.key]) end end - def process_relationship(serializer, include_directive) + def process_relationship(serializer, include_slice) if serializer.respond_to?(:each) - serializer.each { |s| process_relationship(s, include_directive) } + serializer.each { |s| process_relationship(s, include_slice) } return end return unless serializer && serializer.object - return unless process_resource(serializer, false) + return unless process_resource(serializer, false, include_slice) - process_relationships(serializer, include_directive) + process_relationships(serializer, include_slice) end # {http://jsonapi.org/format/#document-resource-object-attributes Document Resource Object Attributes} @@ -293,7 +293,7 @@ def attributes_for(serializer, fields) end # {http://jsonapi.org/format/#document-resource-objects Document Resource Objects} - def resource_object_for(serializer) + def resource_object_for(serializer, include_slice = {}) resource_object = serializer.fetch(self) do resource_object = ResourceIdentifier.new(serializer, instance_options).as_json @@ -304,7 +304,7 @@ def resource_object_for(serializer) end requested_associations = fieldset.fields_for(resource_object[:type]) || '*' - relationships = relationships_for(serializer, requested_associations) + relationships = relationships_for(serializer, requested_associations, include_slice) resource_object[:relationships] = relationships if relationships.any? links = links_for(serializer) @@ -432,12 +432,12 @@ def resource_object_for(serializer) # id: 'required-id', # meta: meta # }.reject! {|_,v| v.nil? } - def relationships_for(serializer, requested_associations) + def relationships_for(serializer, requested_associations, include_slice) include_directive = JSONAPI::IncludeDirective.new( requested_associations, allow_wildcard: true ) - serializer.associations(include_directive).each_with_object({}) do |association, hash| + serializer.associations(include_directive, include_slice).each_with_object({}) do |association, hash| hash[association.key] = Relationship.new(serializer, instance_options, association).as_json end end diff --git a/test/adapter/json_api/include_data_if_sideloaded_test.rb b/test/adapter/json_api/include_data_if_sideloaded_test.rb new file mode 100644 index 000000000..88dc56abd --- /dev/null +++ b/test/adapter/json_api/include_data_if_sideloaded_test.rb @@ -0,0 +1,166 @@ +require 'test_helper' + +module ActiveModel + class Serializer + module Adapter + class JsonApi + class IncludeParamTest < ActiveSupport::TestCase + IncludeParamAuthor = Class.new(::Model) + + class CustomCommentLoader + def all + [{ foo: 'bar' }] + end + end + + class TagSerializer < ActiveModel::Serializer + attributes :id, :name + end + + class IncludeParamAuthorSerializer < ActiveModel::Serializer + class_attribute :comment_loader + + has_many :tags, serializer: TagSerializer do + link :self, '//example.com/link_author/relationships/tags' + include_data :if_sideloaded + end + + has_many :unlinked_tags, serializer: TagSerializer do + include_data :if_sideloaded + end + + has_many :posts, serializer: PostWithTagsSerializer do + include_data :if_sideloaded + end + has_many :locations do + include_data :if_sideloaded + end + has_many :comments do + include_data :if_sideloaded + IncludeParamAuthorSerializer.comment_loader.all + end + end + + def setup + IncludeParamAuthorSerializer.comment_loader = Class.new(CustomCommentLoader).new + @tag = Tag.new(id: 1337, name: 'mytag') + @author = IncludeParamAuthor.new( + id: 1337, + tags: [@tag] + ) + end + + def test_relationship_not_loaded_when_not_included + expected = { + links: { + self: '//example.com/link_author/relationships/tags' + } + } + + @author.define_singleton_method(:read_attribute_for_serialization) do |attr| + fail 'should not be called' if attr == :tags + super(attr) + end + + assert_relationship(:tags, expected) + end + + def test_relationship_included + expected = { + data: [ + { + id: '1337', + type: 'tags' + } + ], + links: { + self: '//example.com/link_author/relationships/tags' + } + } + + assert_relationship(:tags, expected, include: :tags) + end + + def test_sideloads_included + expected = [ + { + id: '1337', + type: 'tags', + attributes: { name: 'mytag' } + } + ] + hash = result(include: :tags) + assert_equal(expected, hash[:included]) + end + + def test_nested_relationship + expected = { + data: [ + { + id: '1337', + type: 'tags' + } + ], + links: { + self: '//example.com/link_author/relationships/tags' + } + } + + expected_no_data = { + links: { + self: '//example.com/link_author/relationships/tags' + } + } + + assert_relationship(:tags, expected, include: [:tags, { posts: :tags }]) + + @author.define_singleton_method(:read_attribute_for_serialization) do |attr| + fail 'should not be called' if attr == :tags + super(attr) + end + + assert_relationship(:tags, expected_no_data, include: { posts: :tags }) + end + + def test_include_params_with_no_block + @author.define_singleton_method(:read_attribute_for_serialization) do |attr| + fail 'should not be called' if attr == :locations + super(attr) + end + + expected = {} + + assert_relationship(:locations, expected) + end + + def test_block_relationship + expected = { + data: [ + { 'foo' => 'bar' } + ] + } + + assert_relationship(:comments, expected, include: [:comments]) + end + + def test_node_not_included_when_no_link + expected = nil + assert_relationship(:unlinked_tags, expected) + end + + private + + def result(opts) + opts = { adapter: :json_api }.merge(opts) + serializable(@author, opts).serializable_hash + end + + def assert_relationship(relationship_name, expected, opts = {}) + hash = result(opts) + assert_equal(expected, hash[:data][:relationships][relationship_name]) + end + end + end + end + end +end