From 20ddc5e102f45b3f731159ea9bc86b9dc6a96b10 Mon Sep 17 00:00:00 2001
From: Lucas Hosseini <lucas.hosseini@gmail.com>
Date: Wed, 20 Jan 2016 00:28:07 +0100
Subject: [PATCH] Refactor JsonApi adapter to avoid redundant computations.

---
 .../serializer/adapter/json_api.rb            | 103 ++++++++----------
 1 file changed, 45 insertions(+), 58 deletions(-)

diff --git a/lib/active_model/serializer/adapter/json_api.rb b/lib/active_model/serializer/adapter/json_api.rb
index 744d62e4..1343e123 100644
--- a/lib/active_model/serializer/adapter/json_api.rb
+++ b/lib/active_model/serializer/adapter/json_api.rb
@@ -52,12 +52,13 @@ module ActiveModel
         def serializable_hash(options = nil)
           options ||= {}
 
-          hash =
-            if serializer.respond_to?(:each)
-              serializable_hash_for_collection(options)
-            else
-              serializable_hash_for_single_resource
-            end
+          is_collection = serializer.respond_to?(:each)
+          serializers = is_collection ? serializer : [serializer]
+          primary_data, included = resource_objects_for(serializers)
+
+          hash = {}
+          hash[:data] = is_collection ? primary_data : primary_data[0]
+          hash[:included] = included if included.any?
 
           ApiObjects::JsonApi.add!(hash)
 
@@ -66,6 +67,11 @@ module ActiveModel
             hash[:links].update(instance_options[:links])
           end
 
+          if is_collection && serializer.paginated?
+            hash[:links] ||= {}
+            hash[:links].update(pagination_links_for(serializer, options))
+          end
+
           hash
         end
 
@@ -80,37 +86,45 @@ module ActiveModel
 
         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]
+        def resource_objects_for(serializers)
+          @primary = []
+          @included = []
+          @resource_identifiers = Set.new
+          serializers.each { |serializer| process_resource(serializer, true) }
+          serializers.each { |serializer| process_relationships(serializer, @include_tree) }
 
-            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))
-          end
-
-          hash
+          [@primary, @included]
         end
 
-        def serializable_hash_for_single_resource
-          primary_data = resource_object_for(serializer)
+        def process_resource(serializer, primary)
+          resource_identifier = resource_identifier_for(serializer)
+          return false unless @resource_identifiers.add?(resource_identifier)
 
-          hash = { data: primary_data }
+          resource_object = resource_object_for(serializer)
+          if primary
+            @primary << resource_object
+          else
+            @included << resource_object
+          end
 
-          included = included_resources(@include_tree, [primary_data])
-          hash[:included] = included if included.any?
+          true
+        end
 
-          hash
+        def process_relationships(serializer, include_tree)
+          serializer.associations(include_tree).each do |association|
+            process_relationship(association.serializer, include_tree[association.key])
+          end
+        end
+
+        def process_relationship(serializer, include_tree)
+          if serializer.respond_to?(:each)
+            serializer.each { |s| process_relationship(s, include_tree) }
+            return
+          end
+          return unless serializer && serializer.object
+          return unless process_resource(serializer, false)
+
+          process_relationships(serializer, include_tree)
         end
 
         def resource_identifier_type_for(serializer)
@@ -181,33 +195,6 @@ module ActiveModel
           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
-          end
-        end
-
         def links_for(serializer)
           serializer._links.each_with_object({}) do |(name, value), hash|
             hash[name] = Link.new(serializer, value).as_json