Merge pull request rails-api#2026 from bf4/refactor_association

Refactor Association to make it eval reflection JIT
mkon · Apr 30, 2017 · 0f59d64 · 0f59d64
2 parents 3fb72d9 + 8761904
commit 0f59d64
Show file tree

Hide file tree

Showing 14 changed files with 298 additions and 193 deletions.
diff --git a/lib/active_model/serializer.rb b/lib/active_model/serializer.rb
@@ -332,50 +332,24 @@ def attributes(requested_attrs = nil, reload = false)
     # @param [JSONAPI::IncludeDirective] include_directive (defaults to the
     #   +default_include_directive+ config value when not provided)
     # @return [Enumerator<Association>]
-    #
     def associations(include_directive = ActiveModelSerializers.default_include_directive, include_slice = nil)
       include_slice ||= include_directive
-      return unless object
+      return Enumerator.new unless object
 
       Enumerator.new do |y|
-        self.class._reflections.values.each do |reflection|
+        self.class._reflections.each do |key, reflection|
           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, include_slice)
+          association = reflection.build_association(self, instance_options, include_slice)
+          y.yield association
         end
       end
     end
 
     # @return [Hash] containing the attributes and first level
     # associations, similar to how ActiveModel::Serializers::JSON is used
     # in ActiveRecord::Base.
-    #
-    # TODO: Include <tt>ActiveModel::Serializers::JSON</tt>.
-    # So that the below is true:
-    #   @param options [nil, Hash] The same valid options passed to `serializable_hash`
-    #      (:only, :except, :methods, and :include).
-    #
-    #     See
-    #     https://github.com/rails/rails/blob/v5.0.0.beta2/activemodel/lib/active_model/serializers/json.rb#L17-L101
-    #     https://github.com/rails/rails/blob/v5.0.0.beta2/activemodel/lib/active_model/serialization.rb#L85-L123
-    #     https://github.com/rails/rails/blob/v5.0.0.beta2/activerecord/lib/active_record/serialization.rb#L11-L17
-    #     https://github.com/rails/rails/blob/v5.0.0.beta2/activesupport/lib/active_support/core_ext/object/json.rb#L147-L162
-    #
-    #   @example
-    #     # The :only and :except options can be used to limit the attributes included, and work
-    #     # similar to the attributes method.
-    #     serializer.as_json(only: [:id, :name])
-    #     serializer.as_json(except: [:id, :created_at, :age])
-    #
-    #     # To include the result of some method calls on the model use :methods:
-    #     serializer.as_json(methods: :permalink)
-    #
-    #     # To include associations use :include:
-    #     serializer.as_json(include: :posts)
-    #     # Second level and higher order associations work as well:
-    #     serializer.as_json(include: { posts: { include: { comments: { only: :body } }, only: :title } })
     def serializable_hash(adapter_options = nil, options = {}, adapter_instance = self.class.serialization_adapter_instance)
       adapter_options ||= {}
       options[:include_directive] ||= ActiveModel::Serializer.include_directive_from_options(adapter_options)
@@ -387,13 +361,6 @@ def serializable_hash(adapter_options = nil, options = {}, adapter_instance = se
     alias to_h serializable_hash
 
     # @see #serializable_hash
-    # TODO: When moving attributes adapter logic here, @see #serializable_hash
-    # So that the below is true:
-    #   @param options [nil, Hash] The same valid options passed to `as_json`
-    #      (:root, :only, :except, :methods, and :include).
-    #   The default for `root` is nil.
-    #   The default value for include_root is false. You can change it to true if the given
-    #   JSON string includes a single root node.
     def as_json(adapter_opts = nil)
       serializable_hash(adapter_opts)
     end
@@ -424,14 +391,12 @@ def attributes_hash(_adapter_options, options, adapter_instance)
 
     # @api private
     def associations_hash(adapter_options, options, adapter_instance)
-      relationships = {}
       include_directive = options.fetch(:include_directive)
-      associations(include_directive).each do |association|
-        adapter_opts = adapter_options.merge(include_directive: include_directive[association.key])
-        relationships[association.key] ||= association.serializable_hash(adapter_opts, adapter_instance)
+      include_slice = options[:include_slice]
+      associations(include_directive, include_slice).each_with_object({}) do |association, relationships|
+        adapter_opts = adapter_options.merge(include_directive: include_directive[association.key], adapter_instance: adapter_instance)
+        relationships[association.key] = association.serializable_hash(adapter_opts, adapter_instance)
       end
-
-      relationships
     end
 
     protected

diff --git a/lib/active_model/serializer/association.rb b/lib/active_model/serializer/association.rb
@@ -1,50 +1,67 @@
+require 'active_model/serializer/lazy_association'
+
 module ActiveModel
   class Serializer
     # This class holds all information about serializer's association.
     #
-    # @attr [Symbol] name
-    # @attr [Hash{Symbol => Object}] options
-    # @attr [block]
-    #
-    # @example
-    #  Association.new(:comments, { serializer: CommentSummarySerializer })
-    #
-    class Association < Field
+    # @api private
+    Association = Struct.new(:reflection, :association_options) do
+      attr_reader :lazy_association
+      delegate :object, :include_data?, :virtual_value, :collection?, to: :lazy_association
+
+      def initialize(*)
+        super
+        @lazy_association = LazyAssociation.new(reflection, association_options)
+      end
+
+      # @return [Symbol]
+      delegate :name, to: :reflection
+
       # @return [Symbol]
       def key
-        options.fetch(:key, name)
+        reflection_options.fetch(:key, name)
       end
 
-      # @return [ActiveModel::Serializer, nil]
-      def serializer
-        options[:serializer]
+      # @return [True,False]
+      def key?
+        reflection_options.key?(:key)
       end
 
       # @return [Hash]
       def links
-        options.fetch(:links) || {}
+        reflection_options.fetch(:links) || {}
       end
 
       # @return [Hash, nil]
+      # This gets mutated, so cannot use the cached reflection_options
       def meta
-        options[:meta]
+        reflection.options[:meta]
+      end
+
+      def polymorphic?
+        true == reflection_options[:polymorphic]
       end
 
       # @api private
       def serializable_hash(adapter_options, adapter_instance)
-        return options[:virtual_value] if options[:virtual_value]
-        object = serializer && serializer.object
-        return unless object
+        association_serializer = lazy_association.serializer
+        return virtual_value if virtual_value
+        association_object = association_serializer && association_serializer.object
+        return unless association_object
 
-        serialization = serializer.serializable_hash(adapter_options, {}, adapter_instance)
+        serialization = association_serializer.serializable_hash(adapter_options, {}, adapter_instance)
 
-        if options[:polymorphic] && serialization
-          polymorphic_type = object.class.name.underscore
+        if polymorphic? && serialization
+          polymorphic_type = association_object.class.name.underscore
           serialization = { type: polymorphic_type, polymorphic_type.to_sym => serialization }
         end
 
         serialization
       end
+
+      private
+
+      delegate :reflection_options, to: :lazy_association
     end
   end
 end
diff --git a/lib/active_model/serializer/belongs_to_reflection.rb b/lib/active_model/serializer/belongs_to_reflection.rb
@@ -1,7 +1,7 @@
 module ActiveModel
   class Serializer
     # @api private
-    class BelongsToReflection < SingularReflection
+    class BelongsToReflection < Reflection
     end
   end
 end
diff --git a/lib/active_model/serializer/collection_reflection.rb b/lib/active_model/serializer/collection_reflection.rb
diff --git a/lib/active_model/serializer/concerns/caching.rb b/lib/active_model/serializer/concerns/caching.rb
@@ -193,12 +193,14 @@ def object_cache_keys(collection_serializer, adapter_instance, include_directive
             cache_keys << object_cache_key(serializer, adapter_instance)
 
             serializer.associations(include_directive).each do |association|
-              if association.serializer.respond_to?(:each)
-                association.serializer.each do |sub_serializer|
+              # TODO(BF): Process relationship without evaluating lazy_association
+              association_serializer = association.lazy_association.serializer
+              if association_serializer.respond_to?(:each)
+                association_serializer.each do |sub_serializer|
                   cache_keys << object_cache_key(sub_serializer, adapter_instance)
                 end
               else
-                cache_keys << object_cache_key(association.serializer, adapter_instance)
+                cache_keys << object_cache_key(association_serializer, adapter_instance)
               end
             end
           end

diff --git a/lib/active_model/serializer/has_many_reflection.rb b/lib/active_model/serializer/has_many_reflection.rb
@@ -1,7 +1,10 @@
 module ActiveModel
   class Serializer
     # @api private
-    class HasManyReflection < CollectionReflection
+    class HasManyReflection < Reflection
+      def collection?
+        true
+      end
     end
   end
 end
diff --git a/lib/active_model/serializer/has_one_reflection.rb b/lib/active_model/serializer/has_one_reflection.rb
@@ -1,7 +1,7 @@
 module ActiveModel
   class Serializer
     # @api private
-    class HasOneReflection < SingularReflection
+    class HasOneReflection < Reflection
     end
   end
 end
diff --git a/lib/active_model/serializer/lazy_association.rb b/lib/active_model/serializer/lazy_association.rb
@@ -0,0 +1,95 @@
+module ActiveModel
+  class Serializer
+    # @api private
+    LazyAssociation = Struct.new(:reflection, :association_options) do
+      REFLECTION_OPTIONS = %i(key links polymorphic meta serializer virtual_value namespace).freeze
+
+      delegate :collection?, to: :reflection
+
+      def reflection_options
+        @reflection_options ||= reflection.options.dup.reject { |k, _| !REFLECTION_OPTIONS.include?(k) }
+      end
+
+      def object
+        @object ||= reflection.value(
+          association_options.fetch(:parent_serializer),
+          association_options.fetch(:include_slice)
+        )
+      end
+      alias_method :eval_reflection_block, :object
+
+      def include_data?
+        eval_reflection_block if reflection.block
+        reflection.include_data?(
+          association_options.fetch(:include_slice)
+        )
+      end
+
+      # @return [ActiveModel::Serializer, nil]
+      def serializer
+        return @serializer if defined?(@serializer)
+        if serializer_class
+          serialize_object!(object)
+        elsif !object.nil? && !object.instance_of?(Object)
+          cached_result[:virtual_value] = object
+        end
+        @serializer = cached_result[:serializer]
+      end
+
+      def virtual_value
+        cached_result[:virtual_value] || reflection_options[:virtual_value]
+      end
+
+      def serializer_class
+        return @serializer_class if defined?(@serializer_class)
+        serializer_for_options = { namespace: namespace }
+        serializer_for_options[:serializer] = reflection_options[:serializer] if reflection_options.key?(:serializer)
+        @serializer_class = association_options.fetch(:parent_serializer).class.serializer_for(object, serializer_for_options)
+      end
+
+      private
+
+      def cached_result
+        @cached_result ||= {}
+      end
+
+      def serialize_object!(object)
+        if collection?
+          if (serializer = instantiate_collection_serializer(object)).nil?
+            # 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.
+            cached_result[:virtual_value] = object.try(:as_json) || object
+          else
+            cached_result[:serializer] = serializer
+          end
+        else
+          cached_result[:serializer] = instantiate_serializer(object)
+        end
+      end
+
+      def instantiate_serializer(object)
+        serializer_options = association_options.fetch(:parent_serializer_options).except(:serializer)
+        serializer_options[:serializer_context_class] = association_options.fetch(:parent_serializer).class
+        serializer = reflection_options.fetch(:serializer, nil)
+        serializer_options[:serializer] = serializer if serializer
+        serializer_class.new(object, serializer_options)
+      end
+
+      def instantiate_collection_serializer(object)
+        serializer = catch(:no_serializer) do
+          instantiate_serializer(object)
+        end
+        serializer
+      end
+
+      def namespace
+        reflection_options[:namespace] ||
+          association_options.fetch(:parent_serializer_options)[:namespace]
+      end
+    end
+  end
+end