Rails Lighthouse ticket #3217

hash_from_xml.diff

From dac554e759dcec715117e55ba0d8d9dc148bd869 Mon Sep 17 00:00:00 2001
From: Niranjan Paranjape <niranjan@c42.in>
Date: Fri, 18 Feb 2011 20:48:49 +0530
Subject: [PATCH] Fixing xml to hash conversion with extra attributes on root node along with "type", [#3217]

---
 .../active_support/core_ext/hash/conversions.rb    |    2 +-
 activesupport/test/core_ext/hash_ext_test.rb       |   49 ++++++++++++++++++++
 2 files changed, 50 insertions(+), 1 deletions(-)

diff --git a/activesupport/lib/active_support/core_ext/hash/conversions.rb b/activesupport/lib/active_support/core_ext/hash/conversions.rb
index 3005fef..1f40b3e 100644
--- a/activesupport/lib/active_support/core_ext/hash/conversions.rb
+++ b/activesupport/lib/active_support/core_ext/hash/conversions.rb
@@ -95,7 +95,7 @@ class Hash
         case value.class.to_s
           when 'Hash'
             if value['type'] == 'array'
-              _, entries = Array.wrap(value.detect { |k,v| k != 'type' })
+              _, entries = Array.wrap(value.detect { |k,v| not v.is_a?(String) })
               if entries.nil? || (c = value['__content__'] && c.blank?)
                 []
               else
diff --git a/activesupport/test/core_ext/hash_ext_test.rb b/activesupport/test/core_ext/hash_ext_test.rb
index a0479d4..e796408 100644
--- a/activesupport/test/core_ext/hash_ext_test.rb
+++ b/activesupport/test/core_ext/hash_ext_test.rb
@@ -776,6 +776,55 @@ class HashToXmlTest < Test::Unit::TestCase
     assert_equal expected_topic_hash, Hash.from_xml(topics_xml)["topics"].first
   end
 
+  def test_multiple_records_from_xml_with_attributes_other_than_type_ignores_them_without_exploding
+    topics_xml = <<-EOT
+      <topics type="array" page="1" page-count="1000" per-page="2">
+        <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>
+          <replies-close-in type="integer">2592000000</replies-close-in>
+          <written-on type="date">2003-07-16</written-on>
+          <viewed-at type="datetime">2003-07-16T09:28:00+0000</viewed-at>
+          <content>Have a nice day</content>
+          <author-email-address>david@loudthinking.com</author-email-address>
+          <parent-id nil="true"></parent-id>
+        </topic>
+        <topic>
+          <title>The Second Topic</title>
+          <author-name>Jason</author-name>
+          <id type="integer">1</id>
+          <approved type="boolean">false</approved>
+          <replies-count type="integer">0</replies-count>
+          <replies-close-in type="integer">2592000000</replies-close-in>
+          <written-on type="date">2003-07-16</written-on>
+          <viewed-at type="datetime">2003-07-16T09:28:00+0000</viewed-at>
+          <content>Have a nice day</content>
+          <author-email-address>david@loudthinking.com</author-email-address>
+          <parent-id></parent-id>
+        </topic>
+      </topics>
+    EOT
+
+    expected_topic_hash = {
+      :title => "The First Topic",
+      :author_name => "David",
+      :id => 1,
+      :approved => false,
+      :replies_count => 0,
+      :replies_close_in => 2592000000,
+      :written_on => Date.new(2003, 7, 16),
+      :viewed_at => Time.utc(2003, 7, 16, 9, 28),
+      :content => "Have a nice day",
+      :author_email_address => "david@loudthinking.com",
+      :parent_id => nil
+    }.stringify_keys
+
+    assert_equal expected_topic_hash, Hash.from_xml(topics_xml)["topics"].first
+  end
+
   def test_single_record_from_xml_with_attributes_other_than_type
     topic_xml = <<-EOT
     <rsp stat="ok">
-- 
1.7.4

hash_xml_parse_collection_with_extra_attributes.diff

diff --git a/actionpack/test/fixtures/layout_tests/layouts/symlinked b/actionpack/test/fixtures/layout_tests/layouts/symlinked
deleted file mode 120000
index 0ddc115..0000000
--- a/actionpack/test/fixtures/layout_tests/layouts/symlinked
+++ /dev/null
@@ -1 +0,0 @@
-../../symlink_parent
\ No newline at end of file
diff --git a/activesupport/lib/active_support/core_ext/hash/conversions.rb b/activesupport/lib/active_support/core_ext/hash/conversions.rb
index 1f8bd7c..836fa42 100644
--- a/activesupport/lib/active_support/core_ext/hash/conversions.rb
+++ b/activesupport/lib/active_support/core_ext/hash/conversions.rb
@@ -155,7 +155,7 @@ class Hash
         case value.class.to_s
           when 'Hash'
             if value['type'] == 'array'
-              child_key, entries = value.detect { |k,v| k != 'type' }   # child_key is throwaway
+              child_key, entries = value.detect { |k,v| not v.is_a?(String) }   # child_key is throwaway and we want to ignore any other attributes on the node
               if entries.nil? || (c = value['__content__'] && c.blank?)
                 []
               else
diff --git a/activesupport/test/core_ext/hash_ext_test.rb b/activesupport/test/core_ext/hash_ext_test.rb
index eb4c37a..9aa1577 100644
--- a/activesupport/test/core_ext/hash_ext_test.rb
+++ b/activesupport/test/core_ext/hash_ext_test.rb
@@ -658,6 +658,55 @@ class HashToXmlTest < Test::Unit::TestCase
     assert_equal expected_topic_hash, Hash.from_xml(topic_xml)["rsp"]["photos"]["photo"]
   end
   
+  def test_multiple_records_from_xml_with_attributes_other_than_type_ignores_them_without_exploding
+    topics_xml = <<-EOT
+      <topics type="array" page="1" page-count="1000" per-page="2">
+        <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>
+          <replies-close-in type="integer">2592000000</replies-close-in>
+          <written-on type="date">2003-07-16</written-on>
+          <viewed-at type="datetime">2003-07-16T09:28:00+0000</viewed-at>
+          <content>Have a nice day</content>
+          <author-email-address>david@loudthinking.com</author-email-address>
+          <parent-id nil="true"></parent-id>
+        </topic>
+        <topic>
+          <title>The Second Topic</title>
+          <author-name>Jason</author-name>
+          <id type="integer">1</id>
+          <approved type="boolean">false</approved>
+          <replies-count type="integer">0</replies-count>
+          <replies-close-in type="integer">2592000000</replies-close-in>
+          <written-on type="date">2003-07-16</written-on>
+          <viewed-at type="datetime">2003-07-16T09:28:00+0000</viewed-at>
+          <content>Have a nice day</content>
+          <author-email-address>david@loudthinking.com</author-email-address>
+          <parent-id></parent-id>
+        </topic>
+      </topics>
+    EOT
+
+    expected_topic_hash = {
+      :title => "The First Topic",
+      :author_name => "David",
+      :id => 1,
+      :approved => false,
+      :replies_count => 0,
+      :replies_close_in => 2592000000,
+      :written_on => Date.new(2003, 7, 16),
+      :viewed_at => Time.utc(2003, 7, 16, 9, 28),
+      :content => "Have a nice day",
+      :author_email_address => "david@loudthinking.com",
+      :parent_id => nil
+    }.stringify_keys
+
+    assert_equal expected_topic_hash, Hash.from_xml(topics_xml)["topics"].first
+  end
+    
   def test_all_caps_key_from_xml
     test_xml = <<-EOT
       <ABC3XYZ>