From 486d282922efa33760a5a58ff8911fe158465cc4 Mon Sep 17 00:00:00 2001 From: twinturbo Date: Mon, 16 Jul 2012 13:41:15 +0200 Subject: [PATCH 1/2] Raise error when associations cannot be included include! only works when the source serializer has a root set. The as_json method sets up some state for the include! method. If a child association has associations with `:include => true` or `root foo, :include => true` would cause an undefined method error for `NilClass`. This is entirely unhelpful for the end user. This commit raise an error when this situation occurs. It makes it clear that it's not a problem with AMS but the serialization graph. --- lib/active_model/serializer.rb | 16 +++++++- test/serializer_test.rb | 75 ++++++++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 1 deletion(-) diff --git a/lib/active_model/serializer.rb b/lib/active_model/serializer.rb index 21e73358..af2ec277 100644 --- a/lib/active_model/serializer.rb +++ b/lib/active_model/serializer.rb @@ -108,6 +108,18 @@ module ActiveModel # end # class Serializer + class IncludeError < StandardError + attr_reader :source, :association + + def initialize(source, association) + @source, @association = source, association + end + + def to_s + "Cannot serialize #{association} when #{source} does not have a root!" + end + end + module Associations #:nodoc: class Config #:nodoc: class_attribute :options @@ -502,7 +514,9 @@ module ActiveModel if association.embed_ids? node[association.key] = association.serialize_ids - if association.embed_in_root? + if association.embed_in_root? && hash.nil? + raise IncludeError.new(self.class, association.name) + elsif association.embed_in_root? merge_association hash, association.root, association.serialize_many, unique_values end elsif association.embed_objects? diff --git a/test/serializer_test.rb b/test/serializer_test.rb index 60bb123e..3e269be8 100644 --- a/test/serializer_test.rb +++ b/test/serializer_test.rb @@ -1169,4 +1169,79 @@ class SerializerTest < ActiveModel::TestCase } }, actual) end + + def test_raises_an_error_when_a_child_serializer_includes_associations_when_the_source_doesnt + attachment_serializer = Class.new(ActiveModel::Serializer) do + attributes :name + end + + fruit_serializer = Class.new(ActiveModel::Serializer) do + embed :ids, :include => true + has_one :attachment, :serializer => attachment_serializer + attribute :color + end + + banana_class = Class.new Model do + def self.to_s + 'banana' + end + + def attachment + @attributes[:attachment] + end + + define_method :active_model_serializer do + fruit_serializer + end + end + + strawberry_class = Class.new Model do + def self.to_s + 'strawberry' + end + + def attachment + @attributes[:attachment] + end + + define_method :active_model_serializer do + fruit_serializer + end + end + + smoothie = Class.new do + attr_reader :base, :flavor + + def initialize(base, flavor) + @base, @flavor = base, flavor + end + end + + smoothie_serializer = Class.new(ActiveModel::Serializer) do + root false + embed :ids, :include => true + + has_one :base, :polymorphic => true + has_one :flavor, :polymorphic => true + end + + banana_attachment = Attachment.new({ + :name => 'banana_blending.md', + :id => 3, + }) + + strawberry_attachment = Attachment.new({ + :name => 'strawberry_cleaning.doc', + :id => 4 + }) + + banana = banana_class.new :color => "yellow", :id => 1, :attachment => banana_attachment + strawberry = strawberry_class.new :color => "red", :id => 2, :attachment => strawberry_attachment + + smoothie = smoothie_serializer.new(smoothie.new(banana, strawberry)) + + assert_raise ActiveModel::Serializer::IncludeError do + smoothie.as_json + end + end end From 6f3b250dc99c0e8780f93a6c416ebfe3e5d0bfcd Mon Sep 17 00:00:00 2001 From: twinturbo Date: Mon, 16 Jul 2012 15:08:01 +0200 Subject: [PATCH 2/2] Don't include empty polymoprhic associations Take this serializer: class TodoSerializer < ActiveModel::Serializer root :todo, :include => true has_one :reference, :polymorphic => true end A nil reference would generate this JSON: { "todo": { "reference": null }, "nil_classes": [] } This commit prevents the `nil_classes` key from being added when serializing and including nil polymoprhic associations. --- lib/active_model/serializer.rb | 14 +++++++++++++- test/serializer_test.rb | 20 ++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/lib/active_model/serializer.rb b/lib/active_model/serializer.rb index af2ec277..117b89de 100644 --- a/lib/active_model/serializer.rb +++ b/lib/active_model/serializer.rb @@ -194,6 +194,10 @@ module ActiveModel option(:include, source_serializer._root_embed) end + def embeddable? + !associated_object.nil? + end + protected def find_serializable(object) @@ -228,6 +232,14 @@ module ActiveModel end class HasOne < Config #:nodoc: + def embeddable? + if polymorphic? && associated_object.nil? + false + else + true + end + end + def polymorphic? option :polymorphic end @@ -516,7 +528,7 @@ module ActiveModel if association.embed_in_root? && hash.nil? raise IncludeError.new(self.class, association.name) - elsif association.embed_in_root? + elsif association.embed_in_root? && association.embeddable? merge_association hash, association.root, association.serialize_many, unique_values end elsif association.embed_objects? diff --git a/test/serializer_test.rb b/test/serializer_test.rb index 3e269be8..b785e3b6 100644 --- a/test/serializer_test.rb +++ b/test/serializer_test.rb @@ -1244,4 +1244,24 @@ class SerializerTest < ActiveModel::TestCase smoothie.as_json end end + + def tests_includes_does_not_include_nil_polymoprhic_associations + post_serializer = Class.new(ActiveModel::Serializer) do + root :post + embed :ids, :include => true + has_one :author, :polymorphic => true + attributes :title + end + + post = Post.new(:title => 'Foo') + + actual = post_serializer.new(post).as_json + + assert_equal({ + :post => { + :title => 'Foo', + :author => nil + } + }, actual) + end end