Fixed to_xml across the board to use nice indention, better skip_attributes workings, no type on strings, and cleaned tests [DHH]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@3829 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

8 files changed, 153 insertions, 32 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 760863de45..c3fdbe38c6 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,42 @@
 *SVN*
 
+* Added Base#to_xml that'll turn the current record into a XML representation [DHH]. Example:
+
+    topic.to_xml
+  
+  ...returns:
+  
+    <?xml version="1.0" encoding="UTF-8"?>
+    <topic>
+      <title>The First Topic</title>
+      <author-name>David</author-name>
+      <id type="integer">1</id>
+      <approved type="boolean">false</approved>
+      <replies-count type="integer">0</replies-count>
+      <bonus-time type="datetime">2000-01-01 08:28:00</bonus-time>
+      <written-on type="datetime">2003-07-16 09:28:00</written-on>
+      <content>Have a nice day</content>
+      <author-email-address>david@loudthinking.com</author-email-address>
+      <parent-id></parent-id>
+      <last-read type="date">2004-04-15</last-read>
+    </topic>
+  
+  ...and you can configure with:
+  
+    topic.to_xml(:skip_instruct => true, :skip_attributes => [ :id, bonus_time, :written_on, replies_count ])
+  
+  ...that'll return:
+  
+    <topic>
+      <title>The First Topic</title>
+      <author-name>David</author-name>
+      <approved type="boolean">false</approved>
+      <content>Have a nice day</content>
+      <author-email-address>david@loudthinking.com</author-email-address>
+      <parent-id></parent-id>
+      <last-read type="date">2004-04-15</last-read>
+    </topic>
+
 * Allow :counter_cache to take a column name for custom counter cache columns [Jamis Buck]
 
 * Documentation fixes for :dependent [robby@planetargon.com]
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 1203068aa8..61f7e94d89 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -1504,6 +1504,20 @@ module ActiveRecord #:nodoc:
         @readonly = true
       end
 
+      # Turns this record into XML 
+      def to_xml(options = {})
+        options[:skip_attributes] = Array(options[:skip_attributes])
+        options[:skip_attributes] << :type
+        options[:skip_attributes].collect! { |attribute| attribute.to_s }
+
+        attributes_to_be_xmled = attributes
+        options[:skip_attributes].each { |attribute_name| attributes_to_be_xmled.delete(attribute_name) }
+
+        options[:root] ||= self.class.to_s.underscore
+
+        attributes_to_be_xmled.to_xml(options)
+      end
+
     private
       def create_or_update
         if new_record? then create else update end
diff --git a/activerecord/test/base_test.rb b/activerecord/test/base_test.rb
index fe74bfa7e3..d82bde433d 100755
--- a/activerecord/test/base_test.rb
+++ b/activerecord/test/base_test.rb
@@ -1154,6 +1154,34 @@ class BasicsTest < Test::Unit::TestCase
     assert_no_queries { assert true }
   end
 
+  def test_to_xml
+    xml = Topic.find(:first).to_xml(:indent => 0, :skip_instruct => true)
+    assert_equal "<topic>", xml.first(7)
+    assert xml.include?(%(<title>The First Topic</title>))
+    assert xml.include?(%(<author-name>David</author-name>))
+    assert xml.include?(%(<id type="integer">1</id>))
+    assert xml.include?(%(<approved type="boolean">false</approved>))
+    assert xml.include?(%(<replies-count type="integer">0</replies-count>))
+    assert xml.include?(%(<bonus-time type="datetime">2000-01-01 08:28:00</bonus-time>))
+    assert xml.include?(%(<written-on type="datetime">2003-07-16 09:28:00</written-on>))
+    assert xml.include?(%(<content>Have a nice day</content>))
+    assert xml.include?(%(<author-email-address>david@loudthinking.com</author-email-address>))
+    assert xml.include?(%(<parent-id></parent-id>))
+    assert xml.include?(%(<last-read type="date">2004-04-15</last-read>))
+  end
+  
+  def test_to_xml_skipping_attributes
+    xml = Topic.find(:first).to_xml(:indent => 0, :skip_instruct => true, :skip_attributes => :title)
+    breakpoint
+    assert_equal "<topic>", xml.first(7)
+    assert !xml.include?(%(<title>The First Topic</title>))
+    assert xml.include?(%(<author-name>David</author-name>))    
+
+    xml = Topic.find(:first).to_xml(:indent => 0, :skip_instruct => true, :skip_attributes => [ :title, :author_name ])
+    assert !xml.include?(%(<title>The First Topic</title>))
+    assert !xml.include?(%(<author-name>David</author-name>))    
+  end
+
   # FIXME: this test ought to run, but it needs to run sandboxed so that it
   # doesn't b0rk the current test environment by undefing everything.
   #
diff --git a/activesupport/CHANGELOG b/activesupport/CHANGELOG
index ce3df8969c..e39a9b6f43 100644
--- a/activesupport/CHANGELOG
+++ b/activesupport/CHANGELOG
@@ -10,8 +10,8 @@ approximations and shouldn't be used for critical calculations. [Koz]
   ...returns:
 
       <person>
-        <street-name type="string">Paulina</street>
-        <name type="string">David</name>
+        <street-name>Paulina</street-name>
+        <name>David</name>
         <age type="integer">26</age>
         <moved-on type="date">2005-11-15</moved-on>
       </person>
diff --git a/activesupport/lib/active_support/core_ext/array/conversions.rb b/activesupport/lib/active_support/core_ext/array/conversions.rb
index faa995bc9c..6f83231487 100644
--- a/activesupport/lib/active_support/core_ext/array/conversions.rb
+++ b/activesupport/lib/active_support/core_ext/array/conversions.rb
@@ -28,9 +28,14 @@ module ActiveSupport #:nodoc:
         
         def to_xml(options = {})
           raise "Not all elements respond to to_xml" unless all? { |e| e.respond_to? :to_xml }
-          options[:root] ||= all? { |e| e.is_a? first.class } ? first.class.to_s.underscore.pluralize : "records"
-          xml = options[:builder] || Builder::XmlMarkup.new
-          xml.__send__(options[:root]) { each { |e| e.to_xml(:builder => xml) } }
+
+          options[:root]    ||= all? { |e| e.is_a? first.class } ? first.class.to_s.underscore.pluralize : "records"
+          options[:indent]  ||= 2
+          options[:builder] ||= Builder::XmlMarkup.new(:indent => options[:indent])
+
+          options[:builder].instruct! unless options.delete(:skip_instruct)
+          root = options.delete(:root)
+          options[:builder].__send__(root) { each { |e| e.to_xml(options.merge({ :skip_instruct => true })) } }
         end
       end
     end
diff --git a/activesupport/lib/active_support/core_ext/hash/conversions.rb b/activesupport/lib/active_support/core_ext/hash/conversions.rb
index 3218f6302c..584a7ec7cc 100644
--- a/activesupport/lib/active_support/core_ext/hash/conversions.rb
+++ b/activesupport/lib/active_support/core_ext/hash/conversions.rb
@@ -3,10 +3,11 @@ module ActiveSupport #:nodoc:
     module Hash #:nodoc:
       module Conversions
         XML_TYPE_NAMES = {
-          "String" => "string",
-          "Fixnum" => "integer",
-          "Date"   => "date",
-          "Time"   => "datetime"
+          "Fixnum"     => "integer",
+          "Date"       => "date",
+          "Time"       => "datetime",
+          "TrueClass"  => "boolean",
+          "FalseClass" => "boolean"
         }
         
         XML_FORMATTING = {
@@ -15,19 +16,22 @@ module ActiveSupport #:nodoc:
         }
         
         def to_xml(options = {})
-          options.reverse_merge!({ :builder => Builder::XmlMarkup.new, :root => "hash" })
+          options[:indent] ||= 2
+          options.reverse_merge!({ :builder => Builder::XmlMarkup.new(:indent => options[:indent]), :root => "hash" })
+          options[:builder].instruct! unless options.delete(:skip_instruct)
 
           options[:builder].__send__(options[:root]) do
             for key in keys
               value = self[key]
 
               if value.is_a?(self.class)
-                value.to_xml(:builder => options[:builder], :root => key)
+                value.to_xml(options.merge({ :root => key, :skip_instruct => true }))
               else
                 type_name = XML_TYPE_NAMES[value.class.to_s]
+
                 options[:builder].__send__(key.to_s.dasherize, 
                   XML_FORMATTING[type_name] ? XML_FORMATTING[type_name].call(value) : value,
-                  value.nil? ? { } : { :type => type_name }
+                  options[:skip_types] || value.nil? || type_name.nil? ? { } : { :type => type_name }
                 )
               end
             end
diff --git a/activesupport/test/core_ext/array_ext_test.rb b/activesupport/test/core_ext/array_ext_test.rb
index 956c10665d..af566179e9 100644
--- a/activesupport/test/core_ext/array_ext_test.rb
+++ b/activesupport/test/core_ext/array_ext_test.rb
@@ -1,5 +1,5 @@
 require 'test/unit'
-require File.dirname(__FILE__) + '/../../lib/active_support/core_ext/array'
+require File.dirname(__FILE__) + '/../../lib/active_support'
 
 class ArrayExtToParamTests < Test::Unit::TestCase
   def test_string_array
@@ -71,10 +71,26 @@ end
 
 class ArraToXmlTests < Test::Unit::TestCase
   def test_to_xml
-    a = [ { :name => "David", :street_address => "Paulina" }, { :name => "Jason", :street_address => "Evergreen" } ]
-    assert_equal(
-      "<hashes><hash><street-address type=\"string\">Paulina</street-address><name type=\"string\">David</name></hash><hash><street-address type=\"string\">Evergreen</street-address><name type=\"string\">Jason</name></hash></hashes>",
-      a.to_xml
-    )
+    xml = [
+      { :name => "David", :age => 26 }, { :name => "Jason", :age => 31 }
+    ].to_xml(:skip_instruct => true, :indent => 0)
+
+    assert_equal "<hashes><hash>", xml.first(14)
+    assert xml.include?(%(<age type="integer">26</age>))
+    assert xml.include?(%(<name>David</name>))
+    assert xml.include?(%(<age type="integer">31</age>))
+    assert xml.include?(%(<name>Jason</name>))
+  end
+  
+  def test_to_xml_with_options
+    xml = [ 
+      { :name => "David", :street_address => "Paulina" }, { :name => "Jason", :street_address => "Evergreen" }
+    ].to_xml(:skip_instruct => true, :skip_types => true, :indent => 0)
+
+    assert_equal "<hashes><hash>", xml.first(14)
+    assert xml.include?(%(<street-address>Paulina</street-address>))
+    assert xml.include?(%(<name>David</name>))
+    assert xml.include?(%(<street-address>Evergreen</street-address>))
+    assert xml.include?(%(<name>Jason</name>))
   end
 end
\ No newline at end of file
diff --git a/activesupport/test/core_ext/hash_ext_test.rb b/activesupport/test/core_ext/hash_ext_test.rb
index b7d024f754..1aa3838152 100644
--- a/activesupport/test/core_ext/hash_ext_test.rb
+++ b/activesupport/test/core_ext/hash_ext_test.rb
@@ -167,29 +167,46 @@ class HashExtTest < Test::Unit::TestCase
 end
 
 class HashToXmlTest < Test::Unit::TestCase
+  def setup
+    @xml_options = { :root => :person, :skip_instruct => true, :indent => 0 }
+  end
+
   def test_one_level
-    h = { :name => "David", :street => "Paulina" }
-    assert_equal %(<person><street type="string">Paulina</street><name type="string">David</name></person>), h.to_xml(:root => :person)
+    xml = { :name => "David", :street => "Paulina" }.to_xml(@xml_options)
+    assert_equal "<person>", xml.first(8)
+    assert xml.include?(%(<street>Paulina</street>))
+    assert xml.include?(%(<name>David</name>))
   end
 
   def test_one_level_with_types
-    h = { :name => "David", :street => "Paulina", :age => 26, :moved_on => Date.new(2005, 11, 15) }
-    assert_equal(
-      "<person><street type=\"string\">Paulina</street><name type=\"string\">David</name><age type=\"integer\">26</age><moved-on type=\"date\">2005-11-15</moved-on></person>", 
-      h.to_xml(:root => :person)
-    )
+    xml = { :name => "David", :street => "Paulina", :age => 26, :moved_on => Date.new(2005, 11, 15) }.to_xml(@xml_options)
+    assert_equal "<person>", xml.first(8)
+    assert xml.include?(%(<street>Paulina</street>))
+    assert xml.include?(%(<name>David</name>))
+    assert xml.include?(%(<age type="integer">26</age>))
+    assert xml.include?(%(<moved-on type="date">2005-11-15</moved-on>))
   end
 
   def test_one_level_with_nils
-    h = { :name => "David", :street => "Paulina", :age => nil }
-    assert_equal(
-      "<person><street type=\"string\">Paulina</street><name type=\"string\">David</name><age></age></person>", 
-      h.to_xml(:root => :person)
-    )
+    xml = { :name => "David", :street => "Paulina", :age => nil }.to_xml(@xml_options)
+    assert_equal "<person>", xml.first(8)
+    assert xml.include?(%(<street>Paulina</street>))
+    assert xml.include?(%(<name>David</name>))
+    assert xml.include?(%(<age></age>))
+  end
+
+  def test_one_level_with_skipping_types
+    xml = { :name => "David", :street => "Paulina", :age => nil }.to_xml(@xml_options.merge(:skip_types => true))
+    assert_equal "<person>", xml.first(8)
+    assert xml.include?(%(<street>Paulina</street>))
+    assert xml.include?(%(<name>David</name>))
+    assert xml.include?(%(<age></age>))
   end
 
   def test_two_levels
-    h = { :name => "David", :address => { :street => "Paulina" } }
-    assert_equal %(<person><address><street type="string">Paulina</street></address><name type="string">David</name></person>), h.to_xml(:root => :person)
+    xml = { :name => "David", :address => { :street => "Paulina" } }.to_xml(@xml_options)
+    assert_equal "<person>", xml.first(8)
+    assert xml.include?(%(<address><street>Paulina</street></address>))
+    assert xml.include?(%(<name>David</name>))
   end
 end
\ No newline at end of file