From f4bb4c81b0a7925104fd55143cd6f888de667fc6 Mon Sep 17 00:00:00 2001 From: Lucas Hosseini Date: Mon, 5 Oct 2015 08:41:31 +0200 Subject: [PATCH] Fix duplicate resources between data and included. --- CHANGELOG.md | 1 + .../serializer/adapter/json_api.rb | 33 ++--- test/adapter/json_api/linked_test.rb | 114 +++++++++++++++++- 3 files changed, 132 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2af0c463b..32663f9cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,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: diff --git a/lib/active_model/serializer/adapter/json_api.rb b/lib/active_model/serializer/adapter/json_api.rb index 2792af094..b1256fee7 100644 --- a/lib/active_model/serializer/adapter/json_api.rb +++ b/lib/active_model/serializer/adapter/json_api.rb @@ -81,16 +81,18 @@ def fragment_cache(cached_hash, non_cached_hash) 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] - if result[:included] - hash[:included] ||= [] - hash[:included] |= result[:included] - end + 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(links_for(serializer, options)) @@ -102,9 +104,10 @@ def serializable_hash_for_collection(options) def serializable_hash_for_single_resource primary_data = primary_data_for(serializer) 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, [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) + resource_object = primary_data_for(serializer) 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 diff --git a/test/adapter/json_api/linked_test.rb b/test/adapter/json_api/linked_test.rb index 32b835b4e..0bc3073cb 100644 --- a/test/adapter/json_api/linked_test.rb +++ b/test/adapter/json_api/linked_test.rb @@ -1,11 +1,16 @@ require 'test_helper' + +NestedPost = Class.new(Model) +class NestedPostSerializer < ActiveModel::Serializer + has_many :nested_posts +end + module ActiveModel class Serializer module Adapter class JsonApi class LinkedTest < Minitest::Test def setup - ActionController::Base.cache_store.clear @author1 = Author.new(id: 1, name: 'Steve K.') @author2 = Author.new(id: 2, name: 'Tenderlove') @bio1 = Bio.new(id: 1, content: 'AMS Contributor') @@ -277,6 +282,113 @@ 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 + + @nestedpost1 = ::NestedPost.new(id: 1, nested_posts: []) + @nestedpost2 = ::NestedPost.new(id: 2, nested_posts: []) + @nestedpost1.nested_posts << @nestedpost1 + @nestedpost1.nested_posts << @nestedpost2 + @nestedpost2.nested_posts << @nestedpost1 + @nestedpost2.nested_posts << @nestedpost2 + 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 + + def test_no_duplicates_global + hash = ActiveModel::SerializableResource.new( + @nestedpost1, + adapter: :json_api, + include: '*').serializable_hash + expected = [ + type: 'nested_posts', id: '2', + relationships: { + nested_posts: { + data: [ + { type: 'nested_posts', id: '1' }, + { type: 'nested_posts', id: '2' } + ] + } + } + ] + assert_equal(expected, hash[:included]) + end + + def test_no_duplicates_collection_global + hash = ActiveModel::SerializableResource.new( + [@nestedpost1, @nestedpost2], + adapter: :json_api, + include: '*').serializable_hash + assert_nil(hash[:included]) + end + end end end end