Refactor JsonApi adapter in order to avoid recomputation of hashes fo…

…r included resources.

lib/active_model/serializer/adapter/json_api.rb

@@ -6,59 +6,38 @@ class JsonApi < Base
         autoload :PaginationLinks
         autoload :FragmentCache
         autoload :Link
-
-        # TODO: if we like this abstraction and other API objects to it,
-        # then extract to its own file and require it.
-        module ApiObjects
-          module JsonApi
-            ActiveModel::Serializer.config.jsonapi_version = '1.0'
-            ActiveModel::Serializer.config.jsonapi_toplevel_meta = {}
-            # Make JSON API top-level jsonapi member opt-in
-            # ref: http://jsonapi.org/format/#document-top-level
-            ActiveModel::Serializer.config.jsonapi_include_toplevel_object = false
-
-            module_function
-
-            def add!(hash)
-              hash.merge!(object) if include_object?
-            end
-
-            def include_object?
-              ActiveModel::Serializer.config.jsonapi_include_toplevel_object
-            end
-
-            # TODO: see if we can cache this
-            def object
-              object = {
-                jsonapi: {
-                  version: ActiveModel::Serializer.config.jsonapi_version,
-                  meta: ActiveModel::Serializer.config.jsonapi_toplevel_meta
-                }
-              }
-              object[:jsonapi].reject! { |_, v| v.blank? }
-
-              object
-            end
-          end
-        end
+        autoload :Resource
+        autoload :ResourceIdentifier
+        autoload :Relationship
+        require 'active_model/serializer/adapter/json_api/json_api_object'
 
         def initialize(serializer, options = {})
           super
           @include_tree = IncludeTree.from_include_args(options[:include])
-          @fieldset = options[:fieldset] || ActiveModel::Serializer::Fieldset.new(options.delete(:fields))
+          @fieldset = options[:fieldset] || ActiveModel::Serializer::Fieldset.new(options[:fields])
         end
 
+        # Build JSON API document.
+        # @return [Hash] document
         def serializable_hash(options = nil)
           options ||= {}
 
-          hash =
-            if serializer.respond_to?(:each)
-              serializable_hash_for_collection(options)
-            else
-              serializable_hash_for_single_resource
-            end
+          primary_data, included = resource_objects_for(serializer)
+
+          is_collection = serializer.respond_to?(:each)
+          hash = {}
+
+          # Unpack data from `primary_data` array when serializing a single resource.
+          hash[:data] = is_collection ? primary_data.map(&:to_h) : primary_data.first.to_h
+
+          hash[:included] = included.map(&:to_h) if included.any?
+
+          JsonApiObject.add!(hash)
 
-          ApiObjects::JsonApi.add!(hash)
+          if is_collection && serializer.paginated?
+            hash[:links] ||= {}
+            hash[:links].update(pagination_links_for(serializer, options))
+          end
 
           if instance_options[:links]
             hash[:links] ||= {}
@@ -73,140 +52,107 @@ def fragment_cache(cached_hash, non_cached_hash)
           ActiveModel::Serializer::Adapter::JsonApi::FragmentCache.new.fragment_cache(root, cached_hash, non_cached_hash)
         end
 
-        protected
-
-        attr_reader :fieldset
-
         private
 
-        def serializable_hash_for_collection(options)
-          hash = { data: [] }
-          included = []
-          serializer.each do |s|
-            result = self.class.new(s, instance_options.merge(fieldset: fieldset)).serializable_hash(options)
-            hash[:data] << result[:data]
-            next unless result[:included]
-
-            included |= result[:included]
-          end
-
-          included.delete_if { |resource| hash[:data].include?(resource) }
-          hash[:included] = included if included.any?
-
-          if serializer.paginated?
-            hash[:links] ||= {}
-            hash[:links].update(pagination_links_for(serializer, options))
+        # Build the requested resource objects.
+        # @return [Array] [primary, included] Pair of arrays containing primary and included
+        #   resources objects respectively.
+        #
+        # @api private
+        def resource_objects_for(serializer)
+          resources = _resource_objects_for(serializer, @include_tree, true).values
+          resources.each_with_object([[], []]) do |resource, (primary, included)|
+            if resource[:is_primary]
+              primary.push(resource[:resource_object])
+            else
+              included.push(resource[:resource_object])
+            end
           end
-
-          hash
         end
 
-        def serializable_hash_for_single_resource
-          primary_data = resource_object_for(serializer)
-
-          hash = { data: primary_data }
-
-          included = included_resources(@include_tree, [primary_data])
-          hash[:included] = included if included.any?
-
-          hash
-        end
-
-        def resource_identifier_type_for(serializer)
-          return serializer._type if serializer._type
-          if ActiveModel::Serializer.config.jsonapi_resource_type == :singular
-            serializer.object.class.model_name.singular
-          else
-            serializer.object.class.model_name.plural
+        # Recursively build all requested resource objects and flag them as primary when applicable.
+        # @return [Hash<ResourceIdentifier, Hash>]
+        #   Hash of hashes each describing a resource object and whether it is primary or included.
+        #
+        # @api private
+        def _resource_objects_for(serializer, include_tree, is_primary, hashes = {})
+          if serializer.respond_to?(:each)
+            serializer.each { |s| _resource_objects_for(s, include_tree, is_primary, hashes) }
+            return hashes
           end
-        end
 
-        def resource_identifier_id_for(serializer)
-          if serializer.respond_to?(:id)
-            serializer.id
-          else
-            serializer.object.id
+          return hashes unless serializer && serializer.object
+
+          resource_identifier = ResourceIdentifier.from_serializer(serializer)
+          if hashes[resource_identifier]
+            hashes[resource_identifier][:is_primary] ||= is_primary
+            return hashes
           end
-        end
 
-        def resource_identifier_for(serializer)
-          type = resource_identifier_type_for(serializer)
-          id   = resource_identifier_id_for(serializer)
+          resource_object = Resource.new(
+            resource_identifier,
+            attributes_for(serializer),
+            relationships_for(serializer),
+            links_for(serializer))
+          hashes[resource_identifier] = { resource_object: resource_object, is_primary: is_primary }
 
-          { id: id.to_s, type: type }
-        end
+          serializer.associations(include_tree).each do |association|
+            _resource_objects_for(association.serializer, include_tree[association.key], false, hashes)
+          end
 
-        def attributes_for(serializer, fields)
-          serializer.attributes(fields).except(:id)
+          hashes
         end
 
-        def resource_object_for(serializer)
-          resource_object = cache_check(serializer) do
-            resource_object = resource_identifier_for(serializer)
+        # Get resource attributes.
+        # @return [Hash] attributes
+        #
+        # @api private
+        def attributes_for(serializer)
+          hash = cache_check(serializer) do
+            resource_type = ResourceIdentifier.type_for(serializer)
+            requested_fields = @fieldset.fields_for(resource_type)
+            attributes = serializer.attributes(requested_fields).except(:id)
 
-            requested_fields = fieldset && fieldset.fields_for(resource_object[:type])
-            attributes = attributes_for(serializer, requested_fields)
-            resource_object[:attributes] = attributes if attributes.any?
-            resource_object
+            # NOTE(beauby): Wrapping attributes inside a hash is currently
+            #   needed for caching.
+            { attributes: attributes }
           end
 
-          relationships = relationships_for(serializer)
-          resource_object[:relationships] = relationships if relationships.any?
-
-          links = links_for(serializer)
-          resource_object[:links] = links if links.any?
-
-          resource_object
+          hash[:attributes]
         end
 
-        def relationship_value_for(serializer, options = {})
+        # Get resource linkage for an association.
+        # @return [Hash] linkage
+        #
+        # @api private
+        def linkage_for(serializer, options = {})
           if serializer.respond_to?(:each)
-            serializer.map { |s| resource_identifier_for(s) }
+            serializer.map { |s| ResourceIdentifier.from_serializer(s) }
           else
             if options[:virtual_value]
               options[:virtual_value]
             elsif serializer && serializer.object
-              resource_identifier_for(serializer)
+              ResourceIdentifier.from_serializer(serializer)
             end
           end
         end
 
+        # Get resource relationships.
+        # @return [Hash<String, Relationship>] relationships
+        #
+        # @api private
         def relationships_for(serializer)
-          resource_type = resource_identifier_type_for(serializer)
-          requested_associations = fieldset.fields_for(resource_type) || '*'
+          resource_type = ResourceIdentifier.type_for(serializer)
+          requested_associations = @fieldset.fields_for(resource_type) || '*'
           include_tree = IncludeTree.from_include_args(requested_associations)
           serializer.associations(include_tree).each_with_object({}) do |association, hash|
-            hash[association.key] = { data: relationship_value_for(association.serializer, association.options) }
-          end
-        end
-
-        def included_resources(include_tree, primary_data)
-          included = []
-
-          serializer.associations(include_tree).each do |association|
-            add_included_resources_for(association.serializer, include_tree[association.key], primary_data, included)
-          end
-
-          included
-        end
-
-        def add_included_resources_for(serializer, include_tree, primary_data, included)
-          if serializer.respond_to?(:each)
-            serializer.each { |s| add_included_resources_for(s, include_tree, primary_data, included) }
-          else
-            return unless serializer && serializer.object
-
-            resource_object = resource_object_for(serializer)
-
-            return if included.include?(resource_object) || primary_data.include?(resource_object)
-            included.push(resource_object)
-
-            serializer.associations(include_tree).each do |association|
-              add_included_resources_for(association.serializer, include_tree[association.key], primary_data, included)
-            end
+            hash[association.key] = Relationship.new(data: linkage_for(association.serializer, association.options))
           end
         end
 
+        # Get resource links.
+        #
+        # @api private
         def links_for(serializer)
           serializer.links.each_with_object({}) do |(name, value), hash|
             hash[name] =
@@ -221,6 +167,9 @@ def links_for(serializer)
           end
         end
 
+        # Get pagination links.
+        #
+        # @api private
         def pagination_links_for(serializer, options)
           JsonApi::PaginationLinks.new(serializer.object, options[:context]).serializable_hash(options)
         end

lib/active_model/serializer/adapter/json_api/json_api_object.rb

@@ -0,0 +1,38 @@
+module ActiveModel
+  class Serializer
+    module Adapter
+      class JsonApi
+        module JsonApiObject
+          ActiveModel::Serializer.config.jsonapi_version = '1.0'
+          ActiveModel::Serializer.config.jsonapi_toplevel_meta = {}
+          # Make JSON API top-level jsonapi member opt-in
+          # ref: http://jsonapi.org/format/#document-top-level
+          ActiveModel::Serializer.config.jsonapi_include_toplevel_object = false
+
+          module_function
+
+          def add!(hash)
+            hash.merge!(object) if include_object?
+          end
+
+          def include_object?
+            ActiveModel::Serializer.config.jsonapi_include_toplevel_object
+          end
+
+          # TODO: see if we can cache this
+          def object
+            object = {
+              jsonapi: {
+                version: ActiveModel::Serializer.config.jsonapi_version,
+                meta: ActiveModel::Serializer.config.jsonapi_toplevel_meta
+              }
+            }
+            object[:jsonapi].reject! { |_, v| v.blank? }
+
+            object
+          end
+        end
+      end
+    end
+  end
+end

lib/active_model/serializer/adapter/json_api/relationship.rb

@@ -0,0 +1,27 @@
+module ActiveModel
+  class Serializer
+    module Adapter
+      class JsonApi
+        class Relationship
+          # NOTE(beauby): Currently only `data` is used.
+          attr_accessor :data, :meta, :links
+
+          def initialize(hash = {})
+            hash.each { |k, v| send("#{k}=", v) }
+          end
+
+          def to_h
+            data_hash =
+              if data.is_a?(Array)
+                data.map { |ri| ri.respond_to?(:to_h) ? ri.to_h : ri }
+              elsif data
+                data.respond_to?(:to_h) ? data.to_h : data
+              end
+
+            { data: data_hash }
+          end
+        end
+      end
+    end
+  end
+end

lib/active_model/serializer/adapter/json_api/resource.rb

@@ -0,0 +1,18 @@
+module ActiveModel
+  class Serializer
+    module Adapter
+      class JsonApi
+        Resource = Struct.new(:identifier, :attributes, :relationships, :links) do
+          def to_h
+            hash = identifier.to_h
+            hash[:attributes] = attributes if attributes.any?
+            hash[:relationships] = Hash[relationships.map { |k, v| [k, v.to_h] }] if relationships.any?
+            hash[:links] = links if links.any?
+
+            hash
+          end
+        end
+      end
+    end
+  end
+end