Refactor Association into Field like everything else (#1897)

* Make assocations asserts easier to understand

* Refactor Association into Field like everything else

* Make assocation serializer/links/meta lazier

* Push association deeper into relationship

* Simplify association usage in relationships

* Better naming of reflection parent serializer

* Easier to read association method

Author: bf4

lib/active_model/serializer/association.rb

@@ -3,17 +3,32 @@ class Serializer
     # This class hold all information about serializer's association.
     #
     # @attr [Symbol] name
-    # @attr [ActiveModel::Serializer] serializer
     # @attr [Hash{Symbol => Object}] options
+    # @attr [block]
     #
     # @example
-    #  Association.new(:comments, CommentSummarySerializer)
+    #  Association.new(:comments, { serializer: CommentSummarySerializer })
     #
-    Association = Struct.new(:name, :serializer, :options, :links, :meta) do
+    class Association < Field
       # @return [Symbol]
       def key
         options.fetch(:key, name)
       end
+
+      # @return [ActiveModel::Serializer, nil]
+      def serializer
+        options[:serializer]
+      end
+
+      # @return [Hash]
+      def links
+        options.fetch(:links) || {}
+      end
+
+      # @return [Hash, nil]
+      def meta
+        options[:meta]
+      end
     end
   end
 end

lib/active_model/serializer/concerns/associations.rb

@@ -74,8 +74,8 @@ def has_one(name, options = {}, &block) # rubocop:disable Style/PredicateName
         # @api private
         #
         def associate(reflection)
-          key = reflection.options[:key]
-          key ? self._reflections[key] = reflection : self._reflections[reflection.name] = reflection
+          key = reflection.options[:key] || reflection.name
+          self._reflections[key] = reflection
         end
       end
 

lib/active_model/serializer/reflection.rb

@@ -88,7 +88,7 @@ def value(serializer)
       # Build association. This method is used internally to
       # build serializer's association by its reflection.
       #
-      # @param [Serializer] subject is a parent serializer for given association
+      # @param [Serializer] parent_serializer for given association
       # @param [Hash{Symbol => Object}] parent_serializer_options
       #
       # @example
@@ -106,17 +106,19 @@ def value(serializer)
       #
       # @api private
       #
-      def build_association(subject, parent_serializer_options)
-        association_value = value(subject)
+      def build_association(parent_serializer, parent_serializer_options)
+        association_value = value(parent_serializer)
         reflection_options = options.dup
-        serializer_class = subject.class.serializer_for(association_value, reflection_options)
+        serializer_class = parent_serializer.class.serializer_for(association_value, reflection_options)
         reflection_options[:include_data] = @_include_data
+        reflection_options[:links] = @_links
+        reflection_options[:meta] = @_meta
 
         if serializer_class
           begin
-            serializer = serializer_class.new(
+            reflection_options[:serializer] = serializer_class.new(
               association_value,
-              serializer_options(subject, parent_serializer_options, reflection_options)
+              serializer_options(parent_serializer, parent_serializer_options, reflection_options)
             )
           rescue ActiveModel::Serializer::CollectionSerializer::NoSerializerError
             reflection_options[:virtual_value] = association_value.try(:as_json) || association_value
@@ -125,7 +127,8 @@ def build_association(subject, parent_serializer_options)
           reflection_options[:virtual_value] = association_value
         end
 
-        Association.new(name, serializer, reflection_options, @_links, @_meta)
+        block = nil
+        Association.new(name, reflection_options, block)
       end
 
       protected
@@ -134,12 +137,12 @@ def build_association(subject, parent_serializer_options)
 
       private
 
-      def serializer_options(subject, parent_serializer_options, reflection_options)
+      def serializer_options(parent_serializer, parent_serializer_options, reflection_options)
         serializer = reflection_options.fetch(:serializer, nil)
 
         serializer_options = parent_serializer_options.except(:serializer)
         serializer_options[:serializer] = serializer if serializer
-        serializer_options[:serializer_context_class] = subject.class
+        serializer_options[:serializer_context_class] = parent_serializer.class
         serializer_options
       end
     end

lib/active_model_serializers/adapter/json_api/relationship.rb

@@ -7,49 +7,55 @@ class Relationship
         # {http://jsonapi.org/format/#document-resource-object-linkage Document Resource Relationship Linkage}
         # {http://jsonapi.org/format/#document-meta Document Meta}
         def initialize(parent_serializer, serializable_resource_options, association)
-          serializer = association.serializer
-          options = association.options
-          links = association.links
-          meta = association.meta
-          @object = parent_serializer.object
-          @scope = parent_serializer.scope
-          @association_options = options || {}
+          @parent_serializer = parent_serializer
+          @association = association
           @serializable_resource_options = serializable_resource_options
-          @data = data_for(serializer)
-          @links = (links || {}).each_with_object({}) do |(key, value), hash|
-            result = Link.new(parent_serializer, value).as_json
-            hash[key] = result if result
-          end
-          @meta = meta.respond_to?(:call) ? parent_serializer.instance_eval(&meta) : meta
         end
 
         def as_json
           hash = {}
-          hash[:data] = data if association_options[:include_data]
-          links = self.links
+
+          if association.options[:include_data]
+            hash[:data] = data_for(association)
+          end
+
+          links = links_for(association)
           hash[:links] = links if links.any?
-          meta = self.meta
+
+          meta = meta_for(association)
           hash[:meta] = meta if meta
 
           hash
         end
 
         protected
 
-        attr_reader :object, :scope, :data, :serializable_resource_options,
-          :association_options, :links, :meta
+        attr_reader :parent_serializer, :serializable_resource_options, :association
 
         private
 
-        def data_for(serializer)
+        def data_for(association)
+          serializer = association.serializer
           if serializer.respond_to?(:each)
             serializer.map { |s| ResourceIdentifier.new(s, serializable_resource_options).as_json }
-          elsif association_options[:virtual_value]
-            association_options[:virtual_value]
+          elsif (virtual_value = association.options[:virtual_value])
+            virtual_value
           elsif serializer && serializer.object
             ResourceIdentifier.new(serializer, serializable_resource_options).as_json
           end
         end
+
+        def links_for(association)
+          association.links.each_with_object({}) do |(key, value), hash|
+            result = Link.new(parent_serializer, value).as_json
+            hash[key] = result if result
+          end
+        end
+
+        def meta_for(association)
+          meta = association.meta
+          meta.respond_to?(:call) ? parent_serializer.instance_eval(&meta) : meta
+        end
       end
     end
   end

test/adapter/json_api/relationship_test.rb

@@ -155,15 +155,17 @@ def test_relationship(expected, test_options = {})
 
           serializable_resource_options = {} # adapter.instance_options
 
-          meta = test_options.delete(:meta)
-          options = test_options.delete(:options)
-          links = test_options.delete(:links)
+          options = test_options.delete(:options) || {}
+          options[:links] = test_options.delete(:links)
+          options[:meta] = test_options.delete(:meta)
           association_serializer = @serializer
           if association_serializer && association_serializer.object
             association_name = association_serializer.json_key.to_sym
-            association = ::ActiveModel::Serializer::Association.new(association_name, association_serializer, options, links, meta)
+            options[:serializer] = association_serializer
+            association = ::ActiveModel::Serializer::Association.new(association_name, options, nil)
           else
-            association = ::ActiveModel::Serializer::Association.new(:association_name_not_used, association, options, links, meta)
+            options[:serializer] = association
+            association = ::ActiveModel::Serializer::Association.new(:association_name_not_used, options, nil)
           end
 
           relationship = Relationship.new(parent_serializer, serializable_resource_options, association)

test/serializers/associations_test.rb

@@ -31,13 +31,13 @@ def test_has_many_and_has_one
 
           case key
           when :posts
-            assert_equal({ include_data: true }, options)
+            assert_equal true, options.fetch(:include_data)
             assert_kind_of(ActiveModelSerializers.config.collection_serializer, serializer)
           when :bio
-            assert_equal({ include_data: true }, options)
+            assert_equal true, options.fetch(:include_data)
             assert_nil serializer
           when :roles
-            assert_equal({ include_data: true }, options)
+            assert_equal true, options.fetch(:include_data)
             assert_kind_of(ActiveModelSerializers.config.collection_serializer, serializer)
           else
             flunk "Unknown association: #{key}"
@@ -79,7 +79,7 @@ def test_belongs_to
             flunk "Unknown association: #{key}"
           end
 
-          assert_equal({ include_data: true }, association.options)
+          assert_equal true, association.options.fetch(:include_data)
         end
       end
 
@@ -291,11 +291,23 @@ def test_illegal_conditional_associations
       end
 
       class InheritedSerializerTest < ActiveSupport::TestCase
+        class PostSerializer < ActiveModel::Serializer
+          belongs_to :author
+          has_many :comments
+          belongs_to :blog
+        end
+
         class InheritedPostSerializer < PostSerializer
           belongs_to :author, polymorphic: true
           has_many :comments, key: :reviews
         end
 
+        class AuthorSerializer < ActiveModel::Serializer
+          has_many :posts
+          has_many :roles
+          has_one :bio
+        end
+
         class InheritedAuthorSerializer < AuthorSerializer
           has_many :roles, polymorphic: true
           has_one :bio, polymorphic: true
@@ -333,9 +345,18 @@ def setup
         end
 
         test 'a serializer inheriting from another serializer can redefine belongs_to associations' do
-          expected = [:author, :comments, :blog].sort
-          result = (@inherited_post_associations - @post_associations).map(&:name).sort
-          assert_equal(result, expected)
+          assert_equal [:author, :comments, :blog], @post_associations.map(&:name)
+          assert_equal [:author, :comments, :blog, :comments], @inherited_post_associations.map(&:name)
+
+          refute @post_associations.detect { |assoc| assoc.name == :author }.options.key?(:polymorphic)
+          assert_equal true, @inherited_post_associations.detect { |assoc| assoc.name == :author }.options.fetch(:polymorphic)
+
+          refute @post_associations.detect { |assoc| assoc.name == :comments }.options.key?(:key)
+          original_comment_assoc, new_comments_assoc = @inherited_post_associations.select { |assoc| assoc.name == :comments }
+          refute original_comment_assoc.options.key?(:key)
+          assert_equal :reviews, new_comments_assoc.options.fetch(:key)
+
+          assert_equal @post_associations.detect { |assoc| assoc.name == :blog }, @inherited_post_associations.detect { |assoc| assoc.name == :blog }
         end
 
         test 'a serializer inheriting from another serializer can have an additional association with the same name but with different key' do