Fix duplicate resources between data and included.

Author: beauby

CHANGELOG.md

@@ -20,6 +20,7 @@ Features:
 - [#1050](https://github.com/rails-api/active_model_serializers/pull/1050) Add support for toplevel jsonapi member (@beauby, @bf4)
 
 Fixes:
+- [#1239](https://github.com/rails-api/active_model_serializers/pull/1239) Fix duplicates in JSON API compound documents (@beauby)
 - [#1214](https://github.com/rails-api/active_model_serializers/pull/1214) retrieve the key from the reflection options when building associations (@NullVoxPopuli, @hut8)
 
 Misc:

lib/active_model/serializer/adapter/json_api.rb

@@ -84,13 +84,15 @@ def serializable_hash_for_collection(options)
           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]
 
-            if result[:included]
-              hash[:included] ||= []
-              hash[:included] |= result[:included]
-            end
+            hash[:included] ||= []
+            hash[:included] |= result[:included]
           end
 
+          hash[:included].delete_if { |resource| Array(hash[:data]).include?(resource) } if hash[:included]
+          hash.delete(:included) if hash[:included] && hash[:included].empty?
+
           if serializer.paginated?
             hash[:links] ||= {}
             hash[:links].update(links_for(serializer, options))
@@ -102,9 +104,10 @@ def serializable_hash_for_collection(options)
         def serializable_hash_for_single_resource(options)
           primary_data = primary_data_for(serializer, options)
           relationships = relationships_for(serializer)
-          included = included_resources(@include_tree)
+          primary_data[:relationships] = relationships if relationships.any?
           hash = { data: primary_data }
-          hash[:data][:relationships] = relationships if relationships.any?
+
+          included = included_resources(@include_tree, Array(primary_data))
           hash[:included] = included if included.any?
 
           hash
@@ -171,31 +174,31 @@ def relationships_for(serializer)
           end
         end
 
-        def included_resources(include_tree)
+        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], included)
+            add_included_resources_for(association.serializer, include_tree[association.key], primary_data, included)
           end
 
           included
         end
 
-        def add_included_resources_for(serializer, include_tree, included)
+        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, included) }
+            serializer.each { |s| add_included_resources_for(s, include_tree, primary_data, included) }
           else
             return unless serializer && serializer.object
 
-            primary_data = primary_data_for(serializer, instance_options)
+            resource_object = primary_data_for(serializer, instance_options)
             relationships = relationships_for(serializer)
-            primary_data[:relationships] = relationships if relationships.any?
+            resource_object[:relationships] = relationships if relationships.any?
 
-            return if included.include?(primary_data)
-            included.push(primary_data)
+            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], included)
+              add_included_resources_for(association.serializer, include_tree[association.key], primary_data, included)
             end
           end
         end

test/adapter/json_api/linked_test.rb

@@ -277,6 +277,79 @@ def test_nil_link_with_specified_serializer
             assert_equal expected, adapter.serializable_hash
           end
         end
+
+        class NoDuplicatesTest < Minitest::Test
+          Post = Class.new(::Model)
+          Author = Class.new(::Model)
+
+          class PostSerializer < ActiveModel::Serializer
+            type 'posts'
+            class AuthorSerializer < ActiveModel::Serializer
+              type 'authors'
+              has_many :posts, serializer: PostSerializer
+            end
+            belongs_to :author, serializer: AuthorSerializer
+          end
+
+          def setup
+            @author = Author.new(id: 1, posts: [], roles: [], bio: nil)
+            @post1 = Post.new(id: 1, author: @author)
+            @post2 = Post.new(id: 2, author: @author)
+            @author.posts << @post1
+            @author.posts << @post2
+          end
+
+          def test_no_duplicates
+            hash = ActiveModel::SerializableResource.new(@post1, adapter: :json_api,
+                                                                 serializer: PostSerializer,
+                                                                 include: '*.*')
+                   .serializable_hash
+            expected = [
+              {
+                type: 'authors', id: '1',
+                relationships: {
+                  posts: {
+                    data: [
+                      { type: 'posts', id: '1' },
+                      { type: 'posts', id: '2' }
+                    ]
+                  }
+                }
+              },
+              {
+                type: 'posts', id: '2',
+                relationships: {
+                  author: {
+                    data: { type: 'authors', id: '1' }
+                  }
+                }
+              }
+            ]
+            assert_equal(expected, hash[:included])
+          end
+
+          def test_no_duplicates_collection
+            hash = ActiveModel::SerializableResource.new(
+              [@post1, @post2], adapter: :json_api,
+                                each_serializer: PostSerializer,
+                                include: '*.*')
+                   .serializable_hash
+            expected = [
+              {
+                type: 'authors', id: '1',
+                relationships: {
+                  posts: {
+                    data: [
+                      { type: 'posts', id: '1' },
+                      { type: 'posts', id: '2' }
+                    ]
+                  }
+                }
+              }
+            ]
+            assert_equal(expected, hash[:included])
+          end
+        end
       end
     end
   end