From d589268f952f9840c4b0253eb2c5f2acd56b6ae0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Moura?= Date: Mon, 22 Jun 2015 03:15:05 -0300 Subject: [PATCH 1/8] adding new tests to cover array and object rendering --- test/action_controller/serialization_test.rb | 22 ++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/test/action_controller/serialization_test.rb b/test/action_controller/serialization_test.rb index 08b320358..a8c8f8279 100644 --- a/test/action_controller/serialization_test.rb +++ b/test/action_controller/serialization_test.rb @@ -47,6 +47,14 @@ def render_object_with_cache_enabled render json: @post end + def render_json_object_without_serializer + render json: {error: 'Result is Invalid'} + end + + def render_json_array_object_without_serializer + render json: [{error: 'Result is Invalid'}] + end + def update_and_render_object_with_cache_enabled @post.updated_at = DateTime.now @@ -160,6 +168,20 @@ def test_render_using_default_root assert_equal expected.to_json, @response.body end + def test_render_json_object_without_serializer + get :render_json_object_without_serializer + + assert_equal 'application/json', @response.content_type + assert_equal ({error: 'Result is Invalid'}).to_json, @response.body + end + + def test_render_json_array_object_without_serializer + get :render_json_array_object_without_serializer + + assert_equal 'application/json', @response.content_type + assert_equal ([{error: 'Result is Invalid'}]).to_json, @response.body + end + def test_render_array_using_implicit_serializer get :render_array_using_implicit_serializer assert_equal 'application/json', @response.content_type From 189b79523c73baa6bda62cc5ad6383913b52759a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Moura?= Date: Mon, 22 Jun 2015 03:15:37 -0300 Subject: [PATCH 2/8] fixing array rendering when elements doesn't have a serializer --- lib/action_controller/serialization.rb | 16 +++++++++------- lib/active_model/serializer/array_serializer.rb | 7 +++++-- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/lib/action_controller/serialization.rb b/lib/action_controller/serialization.rb index 415167ef1..2ad9ab002 100644 --- a/lib/action_controller/serialization.rb +++ b/lib/action_controller/serialization.rb @@ -39,17 +39,19 @@ def use_adapter? options.partition { |k, _| ADAPTER_OPTION_KEYS.include? k }.map { |h| Hash[h] } if use_adapter? && (serializer = get_serializer(resource)) - @_serializer_opts[:scope] ||= serialization_scope @_serializer_opts[:scope_name] = _serialization_scope - # omg hax - object = serializer.new(resource, @_serializer_opts) - adapter = ActiveModel::Serializer::Adapter.create(object, @_adapter_opts) - super(adapter, options) - else - super(resource, options) + object = serializer.new(resource, @_serializer_opts) + + if serializer == ActiveModel::Serializer.config.array_serializer + resource = ActiveModel::Serializer::Adapter.create(object, @_adapter_opts) unless object.objects.all? {|i| i.nil?} + else + resource = ActiveModel::Serializer::Adapter.create(object, @_adapter_opts) + end end + + super(resource, options) end end diff --git a/lib/active_model/serializer/array_serializer.rb b/lib/active_model/serializer/array_serializer.rb index 174e16fcc..d12b8f483 100644 --- a/lib/active_model/serializer/array_serializer.rb +++ b/lib/active_model/serializer/array_serializer.rb @@ -4,7 +4,7 @@ class ArraySerializer include Enumerable delegate :each, to: :@objects - attr_reader :meta, :meta_key + attr_reader :meta, :meta_key, :objects def initialize(objects, options = {}) @resource = objects @@ -13,7 +13,10 @@ def initialize(objects, options = {}) :serializer, ActiveModel::Serializer.serializer_for(object) ) - serializer_class.new(object, options.except(:serializer)) + + unless serializer_class.nil? + serializer_class.new(object, options.except(:serializer)) + end end @meta = options[:meta] @meta_key = options[:meta_key] From 3710c32ceee7cc071402995d3e914ce7fcb3af2a Mon Sep 17 00:00:00 2001 From: Justin Aiken <60tonangel@gmail.com> Date: Fri, 12 Jun 2015 15:05:33 -0600 Subject: [PATCH 3/8] Add some failing tests around has_many assocs... ..where no serializer is defined for the thing that is has_many'd --- test/adapter/json/has_many_test.rb | 9 ++++++++- test/adapter/json_api/has_many_test.rb | 8 ++++++++ test/fixtures/poro.rb | 7 +++++++ test/serializers/associations_test.rb | 8 ++++++++ 4 files changed, 31 insertions(+), 1 deletion(-) diff --git a/test/adapter/json/has_many_test.rb b/test/adapter/json/has_many_test.rb index 19fe16cd3..5e10358af 100644 --- a/test/adapter/json/has_many_test.rb +++ b/test/adapter/json/has_many_test.rb @@ -17,6 +17,8 @@ def setup @second_comment.post = @post @blog = Blog.new(id: 1, name: "My Blog!!") @post.blog = @blog + @tag = Tag.new(id: 1, name: "#hash_tag") + @post.tags = [@tag] @serializer = PostSerializer.new(@post) @adapter = ActiveModel::Serializer::Adapter::Json.new(@serializer) @@ -28,9 +30,14 @@ def test_has_many {id: 2, body: 'ZOMG ANOTHER COMMENT'} ], @adapter.serializable_hash[:post][:comments]) end + + def test_has_many_with_no_serializer + serializer = PostWithTagsSerializer.new(@post) + adapter = ActiveModel::Serializer::Adapter::Json.new(serializer) + assert_includes(adapter.as_json, :tags) + end end end end end end - diff --git a/test/adapter/json_api/has_many_test.rb b/test/adapter/json_api/has_many_test.rb index a544fc800..9104e8399 100644 --- a/test/adapter/json_api/has_many_test.rb +++ b/test/adapter/json_api/has_many_test.rb @@ -27,6 +27,8 @@ def setup @blog.articles = [@post] @post.blog = @blog @post_without_comments.blog = nil + @tag = Tag.new(id: 1, name: "#hash_tag") + @post.tags = [@tag] @serializer = PostSerializer.new(@post) @adapter = ActiveModel::Serializer::Adapter::JsonApi.new(@serializer) @@ -103,6 +105,12 @@ def test_include_type_for_association_when_different_than_name } assert_equal expected, actual end + + def test_has_many_with_no_serializer + serializer = PostWithTagsSerializer.new(@post) + adapter = ActiveModel::Serializer::Adapter::JsonApi.new(serializer) + assert_includes(adapter.serializable_hash, :tags) + end end end end diff --git a/test/fixtures/poro.rb b/test/fixtures/poro.rb index ee1913ec6..4f0b513a3 100644 --- a/test/fixtures/poro.rb +++ b/test/fixtures/poro.rb @@ -76,6 +76,7 @@ class ProfilePreviewSerializer < ActiveModel::Serializer User = Class.new(Model) Location = Class.new(Model) Place = Class.new(Model) +Tag = Class.new(Model) Comment = Class.new(Model) do # Uses a custom non-time-based cache key def cache_key @@ -224,6 +225,12 @@ def self.root_name belongs_to :author, serializer: AuthorPreviewSerializer end +PostWithTagsSerializer = Class.new(ActiveModel::Serializer) do + attributes :id + + has_many :tags +end + Spam::UnrelatedLinkSerializer = Class.new(ActiveModel::Serializer) do attributes :id end diff --git a/test/serializers/associations_test.rb b/test/serializers/associations_test.rb index ab481de7d..d87ebae7c 100644 --- a/test/serializers/associations_test.rb +++ b/test/serializers/associations_test.rb @@ -29,8 +29,10 @@ def setup @author.roles = [] @blog = Blog.new({ name: 'AMS Blog' }) @post = Post.new({ title: 'New Post', body: 'Body' }) + @tag = Tag.new({name: '#hashtagged'}) @comment = Comment.new({ id: 1, body: 'ZOMG A COMMENT' }) @post.comments = [@comment] + @post.tags = [@tag] @post.blog = @blog @comment.post = @post @comment.author = nil @@ -65,6 +67,12 @@ def test_has_many_and_has_one end end + def test_has_many_with_no_serializer + PostWithTagsSerializer.new(@post).each_association do |name, serializer, options| + puts "The line above will crash this test" + end + end + def test_serializer_options_are_passed_into_associations_serializers @post_serializer.each_association do |name, association| if name == :comments From cf77786da2415d0982ce5f7e9a22d48007cea355 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Sun, 21 Jun 2015 02:55:07 -0500 Subject: [PATCH 4/8] Fix #955 --- lib/active_model/serializer.rb | 20 ++++++++++++++++---- test/adapter/json/has_many_test.rb | 16 ++++++++++------ test/adapter/json_api/has_many_test.rb | 13 +++++++++++-- test/serializers/associations_test.rb | 4 +++- 4 files changed, 40 insertions(+), 13 deletions(-) diff --git a/lib/active_model/serializer.rb b/lib/active_model/serializer.rb index 1e59cc23e..5a75af42b 100644 --- a/lib/active_model/serializer.rb +++ b/lib/active_model/serializer.rb @@ -206,10 +206,22 @@ def each_association(&block) serializer_class = ActiveModel::Serializer.serializer_for(association_value, association_options) if serializer_class - serializer = serializer_class.new( - association_value, - options.except(:serializer).merge(serializer_from_options(association_options)) - ) + begin + serializer = serializer_class.new( + association_value, + options.except(:serializer).merge(serializer_from_options(association_options)) + ) + rescue NoMethodError + # 1. Failure to serialize an element in a collection, e.g. [ {hi: "Steve" } ] will fail + # with NoMethodError when the ArraySerializer finds no serializer for the hash { hi: "Steve" }, + # and tries to call new on that nil. + # 2. Convert association_value to hash using implicit as_json + # 3. Set as virtual value (serializer is nil) + # 4. Consider warning when this happens + virtual_value = association_value + virtual_value = virtual_value.as_json if virtual_value.respond_to?(:as_json) + association_options[:association_options][:virtual_value] = virtual_value + end elsif !association_value.nil? && !association_value.instance_of?(Object) association_options[:association_options][:virtual_value] = association_value end diff --git a/test/adapter/json/has_many_test.rb b/test/adapter/json/has_many_test.rb index 5e10358af..eeff41df8 100644 --- a/test/adapter/json/has_many_test.rb +++ b/test/adapter/json/has_many_test.rb @@ -8,7 +8,7 @@ class HasManyTestTest < Minitest::Test def setup ActionController::Base.cache_store.clear @author = Author.new(id: 1, name: 'Steve K.') - @post = Post.new(title: 'New Post', body: 'Body') + @post = Post.new(id: 42, title: 'New Post', body: 'Body') @first_comment = Comment.new(id: 1, body: 'ZOMG A COMMENT') @second_comment = Comment.new(id: 2, body: 'ZOMG ANOTHER COMMENT') @post.comments = [@first_comment, @second_comment] @@ -19,22 +19,26 @@ def setup @post.blog = @blog @tag = Tag.new(id: 1, name: "#hash_tag") @post.tags = [@tag] - - @serializer = PostSerializer.new(@post) - @adapter = ActiveModel::Serializer::Adapter::Json.new(@serializer) end def test_has_many + serializer = PostSerializer.new(@post) + adapter = ActiveModel::Serializer::Adapter::Json.new(serializer) assert_equal([ {id: 1, body: 'ZOMG A COMMENT'}, {id: 2, body: 'ZOMG ANOTHER COMMENT'} - ], @adapter.serializable_hash[:post][:comments]) + ], adapter.serializable_hash[:post][:comments]) end def test_has_many_with_no_serializer serializer = PostWithTagsSerializer.new(@post) adapter = ActiveModel::Serializer::Adapter::Json.new(serializer) - assert_includes(adapter.as_json, :tags) + assert_equal({ + id: 42, + tags: [ + {"attributes"=>{"id"=>1, "name"=>"#hash_tag"}} + ] + }, adapter.serializable_hash[:post_with_tags]) end end end diff --git a/test/adapter/json_api/has_many_test.rb b/test/adapter/json_api/has_many_test.rb index 9104e8399..5381a080d 100644 --- a/test/adapter/json_api/has_many_test.rb +++ b/test/adapter/json_api/has_many_test.rb @@ -29,7 +29,6 @@ def setup @post_without_comments.blog = nil @tag = Tag.new(id: 1, name: "#hash_tag") @post.tags = [@tag] - @serializer = PostSerializer.new(@post) @adapter = ActiveModel::Serializer::Adapter::JsonApi.new(@serializer) end @@ -97,6 +96,7 @@ def test_include_type_for_association_when_different_than_name serializer = BlogSerializer.new(@blog) adapter = ActiveModel::Serializer::Adapter::JsonApi.new(serializer) actual = adapter.serializable_hash[:data][:relationships][:articles] + expected = { data: [{ type: "posts", @@ -109,7 +109,16 @@ def test_include_type_for_association_when_different_than_name def test_has_many_with_no_serializer serializer = PostWithTagsSerializer.new(@post) adapter = ActiveModel::Serializer::Adapter::JsonApi.new(serializer) - assert_includes(adapter.serializable_hash, :tags) + + assert_equal({ + data: { + id: "1", + type: "posts", + relationships: { + tags: {:data=>nil} + } + } + }, adapter.serializable_hash) end end end diff --git a/test/serializers/associations_test.rb b/test/serializers/associations_test.rb index d87ebae7c..bf468931b 100644 --- a/test/serializers/associations_test.rb +++ b/test/serializers/associations_test.rb @@ -69,7 +69,9 @@ def test_has_many_and_has_one def test_has_many_with_no_serializer PostWithTagsSerializer.new(@post).each_association do |name, serializer, options| - puts "The line above will crash this test" + assert_equal name, :tags + assert_equal serializer, nil + assert_equal options, {:virtual_value=>[{"attributes"=>{"name"=>"#hashtagged"}}]} end end From e5d1e40dbd0d314ac68d1ef5dd5b432d50f5b5d6 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Mon, 22 Jun 2015 15:26:19 -0500 Subject: [PATCH 5/8] Handle special-case of Array serializer with unserializable elements --- lib/action_controller/serialization.rb | 7 +++---- lib/active_model/serializer.rb | 2 +- lib/active_model/serializer/array_serializer.rb | 5 ++++- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/action_controller/serialization.rb b/lib/action_controller/serialization.rb index 2ad9ab002..a659c8d74 100644 --- a/lib/action_controller/serialization.rb +++ b/lib/action_controller/serialization.rb @@ -42,10 +42,9 @@ def use_adapter? @_serializer_opts[:scope] ||= serialization_scope @_serializer_opts[:scope_name] = _serialization_scope - object = serializer.new(resource, @_serializer_opts) - - if serializer == ActiveModel::Serializer.config.array_serializer - resource = ActiveModel::Serializer::Adapter.create(object, @_adapter_opts) unless object.objects.all? {|i| i.nil?} + begin + object = serializer.new(resource, @_serializer_opts) + rescue ActiveModel::Serializer::ArraySerializer::Error else resource = ActiveModel::Serializer::Adapter.create(object, @_adapter_opts) end diff --git a/lib/active_model/serializer.rb b/lib/active_model/serializer.rb index 5a75af42b..8594d14e1 100644 --- a/lib/active_model/serializer.rb +++ b/lib/active_model/serializer.rb @@ -211,7 +211,7 @@ def each_association(&block) association_value, options.except(:serializer).merge(serializer_from_options(association_options)) ) - rescue NoMethodError + rescue ActiveModel::Serializer::ArraySerializer::Error # 1. Failure to serialize an element in a collection, e.g. [ {hi: "Steve" } ] will fail # with NoMethodError when the ArraySerializer finds no serializer for the hash { hi: "Steve" }, # and tries to call new on that nil. diff --git a/lib/active_model/serializer/array_serializer.rb b/lib/active_model/serializer/array_serializer.rb index d12b8f483..bd73fd9ea 100644 --- a/lib/active_model/serializer/array_serializer.rb +++ b/lib/active_model/serializer/array_serializer.rb @@ -1,6 +1,7 @@ module ActiveModel class Serializer class ArraySerializer + Error = Class.new(StandardError) include Enumerable delegate :each, to: :@objects @@ -14,7 +15,9 @@ def initialize(objects, options = {}) ActiveModel::Serializer.serializer_for(object) ) - unless serializer_class.nil? + if serializer_class.nil? + fail Error, "No serializer found for object: #{object.inspect}" + else serializer_class.new(object, options.except(:serializer)) end end From d3649d5b4ef04f1856cbe88978341b947e599efe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Moura?= Date: Mon, 22 Jun 2015 17:53:46 -0300 Subject: [PATCH 6/8] Renaming Error to NoSerializerError --- lib/action_controller/serialization.rb | 6 +++--- lib/active_model/serializer.rb | 8 +------- lib/active_model/serializer/array_serializer.rb | 6 +++--- 3 files changed, 7 insertions(+), 13 deletions(-) diff --git a/lib/action_controller/serialization.rb b/lib/action_controller/serialization.rb index a659c8d74..acafc0fdd 100644 --- a/lib/action_controller/serialization.rb +++ b/lib/action_controller/serialization.rb @@ -43,10 +43,10 @@ def use_adapter? @_serializer_opts[:scope_name] = _serialization_scope begin - object = serializer.new(resource, @_serializer_opts) - rescue ActiveModel::Serializer::ArraySerializer::Error + serialized = serializer.new(resource, @_serializer_opts) + rescue ActiveModel::Serializer::ArraySerializer::NoSerializerError else - resource = ActiveModel::Serializer::Adapter.create(object, @_adapter_opts) + resource = ActiveModel::Serializer::Adapter.create(serialized, @_adapter_opts) end end diff --git a/lib/active_model/serializer.rb b/lib/active_model/serializer.rb index 8594d14e1..7e4eb0d69 100644 --- a/lib/active_model/serializer.rb +++ b/lib/active_model/serializer.rb @@ -211,13 +211,7 @@ def each_association(&block) association_value, options.except(:serializer).merge(serializer_from_options(association_options)) ) - rescue ActiveModel::Serializer::ArraySerializer::Error - # 1. Failure to serialize an element in a collection, e.g. [ {hi: "Steve" } ] will fail - # with NoMethodError when the ArraySerializer finds no serializer for the hash { hi: "Steve" }, - # and tries to call new on that nil. - # 2. Convert association_value to hash using implicit as_json - # 3. Set as virtual value (serializer is nil) - # 4. Consider warning when this happens + rescue ActiveModel::Serializer::ArraySerializer::NoSerializerError virtual_value = association_value virtual_value = virtual_value.as_json if virtual_value.respond_to?(:as_json) association_options[:association_options][:virtual_value] = virtual_value diff --git a/lib/active_model/serializer/array_serializer.rb b/lib/active_model/serializer/array_serializer.rb index bd73fd9ea..fa66e290a 100644 --- a/lib/active_model/serializer/array_serializer.rb +++ b/lib/active_model/serializer/array_serializer.rb @@ -1,11 +1,11 @@ module ActiveModel class Serializer class ArraySerializer - Error = Class.new(StandardError) + NoSerializerError = Class.new(StandardError) include Enumerable delegate :each, to: :@objects - attr_reader :meta, :meta_key, :objects + attr_reader :meta, :meta_key def initialize(objects, options = {}) @resource = objects @@ -16,7 +16,7 @@ def initialize(objects, options = {}) ) if serializer_class.nil? - fail Error, "No serializer found for object: #{object.inspect}" + fail NoSerializerError, "No serializer found for object: #{object.inspect}" else serializer_class.new(object, options.except(:serializer)) end From 741c4a4b51d68f949b5d369d93e70bfa2bca2193 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Moura?= Date: Mon, 22 Jun 2015 18:44:01 -0300 Subject: [PATCH 7/8] updating tests to work with new virtual_value implementation --- test/adapter/json_api/has_many_test.rb | 2 +- test/serializers/associations_test.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/adapter/json_api/has_many_test.rb b/test/adapter/json_api/has_many_test.rb index 5381a080d..3cef3ef8b 100644 --- a/test/adapter/json_api/has_many_test.rb +++ b/test/adapter/json_api/has_many_test.rb @@ -115,7 +115,7 @@ def test_has_many_with_no_serializer id: "1", type: "posts", relationships: { - tags: {:data=>nil} + tags: { data: nil } } } }, adapter.serializable_hash) diff --git a/test/serializers/associations_test.rb b/test/serializers/associations_test.rb index bf468931b..f92e5a906 100644 --- a/test/serializers/associations_test.rb +++ b/test/serializers/associations_test.rb @@ -71,7 +71,7 @@ def test_has_many_with_no_serializer PostWithTagsSerializer.new(@post).each_association do |name, serializer, options| assert_equal name, :tags assert_equal serializer, nil - assert_equal options, {:virtual_value=>[{"attributes"=>{"name"=>"#hashtagged"}}]} + assert_equal [{ attributes: { name: "#hashtagged" }}].as_json, options[:virtual_value] end end From 17d560eae4d71b2273995ac161a02ab868f3390e Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Thu, 25 Jun 2015 23:03:56 -0500 Subject: [PATCH 8/8] Account for different handling of symbol keys in Rails 4.0 Comparing as a JSON string vs. as the Hash that is convert to JSON works around the different Hash representations. This likely has to do with the introduction of config.action_dispatch.perform_deep_munge in Rails 4.1 See Rails issue 13420 1) Failure: ActiveModel::Serializer::Adapter::Json::HasManyTestTest#test_has_many_with_no_serializer [active_model_serializers/test/adapter/json/has_many_test.rb:36]: --- expected +++ actual @@ -1 +1 @@ -{:id=>42, :tags=>[{"attributes"=>{"id"=>1, "name"=>"#hash_tag"}}]} +{:id=>42, :tags=>[{"attributes"=>{:id=>1, :name=>"#hash_tag"}}]} 2) Failure: ActiveModel::Serializer::AssociationsTest#test_has_many_with_no_serializer [active_model_serializers/test/serializers/associations_test.rb:74]: --- expected +++ actual @@ -1 +1 @@ -[{"attributes"=>{"name"=>"#hashtagged"}}] +[{"attributes"=>{:name=>"#hashtagged"}}] --- test/adapter/json/has_many_test.rb | 2 +- test/serializers/associations_test.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/adapter/json/has_many_test.rb b/test/adapter/json/has_many_test.rb index eeff41df8..a88b2f544 100644 --- a/test/adapter/json/has_many_test.rb +++ b/test/adapter/json/has_many_test.rb @@ -38,7 +38,7 @@ def test_has_many_with_no_serializer tags: [ {"attributes"=>{"id"=>1, "name"=>"#hash_tag"}} ] - }, adapter.serializable_hash[:post_with_tags]) + }.to_json, adapter.serializable_hash[:post_with_tags].to_json) end end end diff --git a/test/serializers/associations_test.rb b/test/serializers/associations_test.rb index f92e5a906..04681d2c4 100644 --- a/test/serializers/associations_test.rb +++ b/test/serializers/associations_test.rb @@ -71,7 +71,7 @@ def test_has_many_with_no_serializer PostWithTagsSerializer.new(@post).each_association do |name, serializer, options| assert_equal name, :tags assert_equal serializer, nil - assert_equal [{ attributes: { name: "#hashtagged" }}].as_json, options[:virtual_value] + assert_equal [{ attributes: { name: "#hashtagged" }}].to_json, options[:virtual_value].to_json end end