diff --git a/lib/active_model/serializer/reflection.rb b/lib/active_model/serializer/reflection.rb index 3e476484..d0ab2847 100644 --- a/lib/active_model/serializer/reflection.rb +++ b/lib/active_model/serializer/reflection.rb @@ -47,6 +47,8 @@ module ActiveModel # # So you can inspect reflections in your Adapters. class Reflection < Field + REFLECTION_OPTIONS = %i(key links polymorphic meta serializer virtual_value namespace).freeze + def initialize(*) super options[:links] = {} @@ -71,8 +73,8 @@ module ActiveModel # meta ids: ids # end # end - def link(name, value = nil, &block) - options[:links][name] = block || value + def link(name, value = nil) + options[:links][name] = block_given? ? Proc.new : value :nil end @@ -86,8 +88,8 @@ module ActiveModel # href object.blog.id.to_s # meta(id: object.blog.id) # end - def meta(value = nil, &block) - options[:meta] = block || value + def meta(value = nil) + options[:meta] = block_given? ? Proc.new : value :nil end @@ -119,23 +121,6 @@ module ActiveModel :nil end - # @param serializer [ActiveModel::Serializer] - # @yield [ActiveModel::Serializer] - # @return [:nil, associated resource or resource collection] - def value(serializer, include_slice) - @object = serializer.object - @scope = serializer.scope - - 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 - end - # Build association. This method is used internally to # build serializer's association by its reflection. # @@ -157,35 +142,31 @@ module ActiveModel # # @api private def build_association(parent_serializer, parent_serializer_options, include_slice = {}) - reflection_options = options.dup + reflection_options = settings.merge(include_data: include_data?(include_slice)) unless block? + association_options = build_association_options(parent_serializer, parent_serializer_options[:namespace], include_slice) + association_value = association_options[:association_value] + serializer_class = association_options[:association_serializer] - # Pass the parent's namespace onto the child serializer - reflection_options[:namespace] ||= parent_serializer_options[:namespace] - - association_value = value(parent_serializer, include_slice) - serializer_class = parent_serializer.class.serializer_for(association_value, reflection_options) - reflection_options[:include_data] = include_data?(include_slice) - reflection_options[:links] = options[:links] - reflection_options[:meta] = options[:meta] + reflection_options ||= settings.merge(include_data: include_data?(include_slice)) # Needs to be after association_value is evaluated unless reflection.block.nil? if serializer_class - serializer = catch(:no_serializer) do - serializer_class.new( - association_value, - serializer_options(parent_serializer, parent_serializer_options, reflection_options) - ) - end - if serializer.nil? - reflection_options[:virtual_value] = association_value.try(:as_json) || association_value - else + if (serializer = build_association_serializer(parent_serializer, parent_serializer_options, association_value, serializer_class)) reflection_options[:serializer] = serializer + else + # BUG: per #2027, JSON API resource relationships are only id and type, and hence either + # *require* a serializer or we need to be a little clever about figuring out the id/type. + # In either case, returning the raw virtual value will almost always be incorrect. + # + # Should be reflection_options[:virtual_value] or adapter needs to figure out what to do + # with an object that is non-nil and has no defined serializer. + reflection_options[:virtual_value] = association_value.try(:as_json) || association_value end elsif !association_value.nil? && !association_value.instance_of?(Object) reflection_options[:virtual_value] = association_value end - block = nil - Association.new(name, reflection_options, block) + association_block = nil + Association.new(name, reflection_options, association_block) end protected @@ -193,7 +174,34 @@ module ActiveModel # used in instance exec attr_accessor :object, :scope - private + def settings + options.dup.reject { |k, _| !REFLECTION_OPTIONS.include?(k) } + end + + # Evaluation of the reflection.block will mutate options. + # So, the settings cannot be used until the block is evaluated. + # This means that each time the block is evaluated, it may set a new + # value in the reflection instance. This is not thread-safe. + # @example + # has_many :likes do + # meta liked: object.likes.any? + # include_data: object.loaded? + # end + def block? + !block.nil? + end + + def serializer? + options.key?(:serializer) + end + + def serializer + options[:serializer] + end + + def namespace + options[:namespace] + end def include_data?(include_slice) include_data_setting = options[:include_data_setting] @@ -205,13 +213,49 @@ module ActiveModel end end - def serializer_options(parent_serializer, parent_serializer_options, reflection_options) - serializer = reflection_options.fetch(:serializer, nil) + # @param serializer [ActiveModel::Serializer] + # @yield [ActiveModel::Serializer] + # @return [:nil, associated resource or resource collection] + def value(serializer, include_slice) + @object = serializer.object + @scope = serializer.scope - serializer_options = parent_serializer_options.except(:serializer) - serializer_options[:serializer] = serializer if serializer - serializer_options[:serializer_context_class] = parent_serializer.class - serializer_options + 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 + end + + def build_association_options(parent_serializer, parent_serializer_namespace_option, include_slice) + serializer_for_options = { + # Pass the parent's namespace onto the child serializer + namespace: namespace || parent_serializer_namespace_option + } + serializer_for_options[:serializer] = serializer if serializer? + association_value = value(parent_serializer, include_slice) + { + association_value: association_value, + association_serializer: parent_serializer.class.serializer_for(association_value, serializer_for_options) + } + end + + # NOTE(BF): This serializer throw/catch should only happen when the serializer is a collection + # serializer. This is a good reason for the reflection to have a to_many? or collection? type method. + # + # @return [ActiveModel::Serializer, nil] + def build_association_serializer(parent_serializer, parent_serializer_options, association_value, serializer_class) + catch(:no_serializer) do + # Make all the parent serializer instance options available to associations + # except ActiveModelSerializers-specific ones we don't want. + serializer_options = parent_serializer_options.except(:serializer) + serializer_options[:serializer_context_class] = parent_serializer.class + serializer_options[:serializer] = serializer if serializer + serializer_class.new(association_value, serializer_options) + end end end end diff --git a/test/serializers/reflection_test.rb b/test/serializers/reflection_test.rb index e5932aba..4cff40a9 100644 --- a/test/serializers/reflection_test.rb +++ b/test/serializers/reflection_test.rb @@ -57,7 +57,7 @@ module ActiveModel assert_equal true, reflection.options.fetch(:include_data_setting) include_slice = :does_not_matter - assert_equal @model.blog, reflection.value(serializer_instance, include_slice) + assert_equal @model.blog, reflection.send(:value, serializer_instance, include_slice) end def test_reflection_value_block @@ -77,7 +77,7 @@ module ActiveModel assert_equal true, reflection.options.fetch(:include_data_setting) include_slice = :does_not_matter - assert_equal @model.blog, reflection.value(serializer_instance, include_slice) + assert_equal @model.blog, reflection.send(:value, serializer_instance, include_slice) end def test_reflection_value_block_with_explicit_include_data_true @@ -98,7 +98,7 @@ module ActiveModel assert_equal true, reflection.options.fetch(:include_data_setting) include_slice = :does_not_matter - assert_equal @model.blog, reflection.value(serializer_instance, include_slice) + assert_equal @model.blog, reflection.send(:value, serializer_instance, include_slice) end def test_reflection_value_block_with_include_data_false_mutates_the_reflection_include_data @@ -117,7 +117,7 @@ module ActiveModel assert_respond_to reflection.block, :call assert_equal true, reflection.options.fetch(:include_data_setting) include_slice = :does_not_matter - assert_nil reflection.value(serializer_instance, include_slice) + assert_nil reflection.send(:value, serializer_instance, include_slice) assert_equal false, reflection.options.fetch(:include_data_setting) end @@ -137,7 +137,7 @@ module ActiveModel assert_respond_to reflection.block, :call assert_equal true, reflection.options.fetch(:include_data_setting) include_slice = {} - assert_nil reflection.value(serializer_instance, include_slice) + assert_nil reflection.send(:value, serializer_instance, include_slice) assert_equal :if_sideloaded, reflection.options.fetch(:include_data_setting) end @@ -157,7 +157,7 @@ module ActiveModel assert_respond_to reflection.block, :call assert_equal true, reflection.options.fetch(:include_data_setting) include_slice = { blog: :does_not_matter } - assert_equal @model.blog, reflection.value(serializer_instance, include_slice) + assert_equal @model.blog, reflection.send(:value, serializer_instance, include_slice) assert_equal :if_sideloaded, reflection.options.fetch(:include_data_setting) end