to_xml should also rely on serializable hash.

3 files changed, 33 insertions, 32 deletions

diff --git a/activemodel/lib/active_model/serializers/xml.rb b/activemodel/lib/active_model/serializers/xml.rb
index 64dda3bcee..d61d9d7119 100644
--- a/activemodel/lib/active_model/serializers/xml.rb
+++ b/activemodel/lib/active_model/serializers/xml.rb
@@ -15,10 +15,10 @@ module ActiveModel
         class Attribute #:nodoc:
           attr_reader :name, :value, :type
 
-          def initialize(name, serializable, raw_value=nil)
+          def initialize(name, serializable, value)
             @name, @serializable = name, serializable
-            raw_value = raw_value.in_time_zone if raw_value.respond_to?(:in_time_zone)
-            @value = raw_value || @serializable.send(name)
+            value  = value.in_time_zone if value.respond_to?(:in_time_zone)
+            @value = value
             @type  = compute_type
           end
 
@@ -49,40 +49,24 @@ module ActiveModel
         def initialize(serializable, options = nil)
           @serializable = serializable
           @options = options ? options.dup : {}
-
-          @options[:only] = Array.wrap(@options[:only]).map { |n| n.to_s }
-          @options[:except] = Array.wrap(@options[:except]).map { |n| n.to_s }
         end
 
-        # To replicate the behavior in ActiveRecord#attributes, <tt>:except</tt>
-        # takes precedence over <tt>:only</tt>. If <tt>:only</tt> is not set
-        # for a N level model but is set for the N+1 level models,
-        # then because <tt>:except</tt> is set to a default value, the second
-        # level model can have both <tt>:except</tt> and <tt>:only</tt> set. So if
-        # <tt>:only</tt> is set, always delete <tt>:except</tt>.
-        def attributes_hash
-          attributes = @serializable.attributes
-          if options[:only].any?
-            attributes.slice(*options[:only])
-          elsif options[:except].any?
-            attributes.except(*options[:except])
-          else
-            attributes
-          end
+        def serializable_hash
+          @serializable.serializable_hash(@options.except(:include))
         end
 
-        def serializable_attributes
-          attributes_hash.map do |name, value|
-            self.class::Attribute.new(name, @serializable, value)
+        def serializable_collection
+          methods = Array.wrap(options[:methods]).map(&:to_s)
+          serializable_hash.map do |name, value|
+            name = name.to_s
+            if methods.include?(name)
+              self.class::MethodAttribute.new(name, @serializable, value)
+            else
+              self.class::Attribute.new(name, @serializable, value)
+            end
           end
         end
 
-        def serializable_methods
-          Array.wrap(options[:methods]).map do |name|
-            self.class::MethodAttribute.new(name.to_s, @serializable) if @serializable.respond_to?(name.to_s)
-          end.compact
-        end
-
         def serialize
           require 'builder' unless defined? ::Builder
 
@@ -114,7 +98,7 @@ module ActiveModel
         end
 
         def add_attributes_and_methods
-          (serializable_attributes + serializable_methods).each do |attribute|
+          serializable_collection.each do |attribute|
             key = ActiveSupport::XmlMini.rename_key(attribute.name, options)
             ActiveSupport::XmlMini.to_tag(key, attribute.value,
               options.merge(attribute.decorations))
diff --git a/activemodel/test/cases/serializers/xml_serialization_test.rb b/activemodel/test/cases/serializers/xml_serialization_test.rb
index a38ef8e223..fc73d9dcd8 100644
--- a/activemodel/test/cases/serializers/xml_serialization_test.rb
+++ b/activemodel/test/cases/serializers/xml_serialization_test.rb
@@ -33,6 +33,12 @@ class Address
   end
 end
 
+class SerializableContact < Contact
+  def serializable_hash(options={})
+    super(options.merge(:only => [:name, :age]))
+  end
+end
+
 class XmlSerializationTest < ActiveModel::TestCase
   def setup
     @contact = Contact.new
@@ -96,6 +102,17 @@ class XmlSerializationTest < ActiveModel::TestCase
     assert_match %r{<createdAt},     @xml
   end
 
+  test "should use serialiable hash" do
+    @contact = SerializableContact.new
+    @contact.name = 'aaron stack'
+    @contact.age = 25
+
+    @xml = @contact.to_xml
+    assert_match %r{<name>aaron stack</name>}, @xml
+    assert_match %r{<age type="integer">25</age>}, @xml
+    assert_no_match %r{<awesome>}, @xml
+  end
+
   test "should allow skipped types" do
     @xml = @contact.to_xml :skip_types => true
     assert_match %r{<age>25</age>}, @xml
diff --git a/activerecord/lib/active_record/serializers/xml_serializer.rb b/activerecord/lib/active_record/serializers/xml_serializer.rb
index cbfa1ad609..0e7f57aa43 100644
--- a/activerecord/lib/active_record/serializers/xml_serializer.rb
+++ b/activerecord/lib/active_record/serializers/xml_serializer.rb
@@ -179,7 +179,7 @@ module ActiveRecord #:nodoc:
   class XmlSerializer < ActiveModel::Serializers::Xml::Serializer #:nodoc:
     def initialize(*args)
       super
-      options[:except] |= Array.wrap(@serializable.class.inheritance_column)
+      options[:except] = Array.wrap(options[:except]) | Array.wrap(@serializable.class.inheritance_column)
     end
 
     class Attribute < ActiveModel::Serializers::Xml::Serializer::Attribute #:nodoc: