Rails Lighthouse ticket #2521

allow_setting_default_dasherize_on_hash_to_xml.diff

From 5bc866f77303144a0b5ed55141152c270283e24a Mon Sep 17 00:00:00 2001
From: John Small <[email protected]>
Date: Fri, 24 Apr 2009 17:39:08 +0100
Subject: [PATCH] This patch fixes lighthouse ticket #2521.

People using ActiveResource & REST to integrate with other systems need to be able to control the default dasherize behavior of Hash.to_xml.
Currently there is no test for a default value, but existing code asssumes it's true. This patch adds tests for the default value and adds
cattr_accessor for :dasherize_xml and :camelize_xml. These class attributes set the defaults for :dasherize and :camelize in rename_keys inside
Hash#to_xml. The tests have been changed to separate out the testing of the parameter options for :camelize
and :dasherize so that we only test one thing at a time. We also test default values for :camelize_xml and :dasherize_xml.

The class attribute dasherize_xml is set to true in this patch to maintain existing code. But at some point in the future it should be set to false
because Hash#to_xml probably should not set underscores to dashes by default.
---
 .../active_support/core_ext/hash/conversions.rb    |   18 ++++-
 activesupport/test/core_ext/hash_ext_test.rb       |   77 ++++++++++++++++++--
 2 files changed, 86 insertions(+), 9 deletions(-)

diff --git a/activesupport/lib/active_support/core_ext/hash/conversions.rb b/activesupport/lib/active_support/core_ext/hash/conversions.rb
index f9dddec..a0c6e10 100644
--- a/activesupport/lib/active_support/core_ext/hash/conversions.rb
+++ b/activesupport/lib/active_support/core_ext/hash/conversions.rb
@@ -3,8 +3,17 @@ require 'active_support/core_ext/object/conversions'
 require 'active_support/core_ext/array/conversions'
 require 'active_support/core_ext/hash/reverse_merge'
 require 'active_support/core/time'
+require 'active_support/core_ext/class/attribute_accessors'
 
 class Hash
+  # these accessors are here because people using ActiveResource and REST to integrate with other systems
+  # have to be able to control the default behavior of rename_key. dasherize_xml is set to true to emulate
+  # existing behavior. In a future version it should be set to false by default. 
+  cattr_accessor :dasherize_xml
+  cattr_accessor :camelize_xml
+  self.dasherize_xml = true
+  self.camelize_xml  = false
+  
   # This module exists to decorate files deserialized using Hash.from_xml with
   # the <tt>original_filename</tt> and <tt>content_type</tt> methods.
   module FileLike #:nodoc:
@@ -139,10 +148,11 @@ class Hash
   end
 
   def rename_key(key, options = {})
-    camelize = options.has_key?(:camelize) && options[:camelize]
-    dasherize = !options.has_key?(:dasherize) || options[:dasherize]
-    key = key.camelize if camelize
-    dasherize ? key.dasherize : key
+    camelize  = options.has_key?(:camelize) ? options[:camelize]   : self.camelize_xml
+    dasherize = options.has_key?(:dasherize) ? options[:dasherize] : self.dasherize_xml
+    key = key.camelize if camelize 
+    key = key.dasherize if dasherize
+    key
   end
 
   class << self
diff --git a/activesupport/test/core_ext/hash_ext_test.rb b/activesupport/test/core_ext/hash_ext_test.rb
index d65a532..b3cc096 100644
--- a/activesupport/test/core_ext/hash_ext_test.rb
+++ b/activesupport/test/core_ext/hash_ext_test.rb
@@ -404,34 +404,101 @@ class HashToXmlTest < Test::Unit::TestCase
   def setup
     @xml_options = { :root => :person, :skip_instruct => true, :indent => 0 }
   end
-
+  
+  def test_default_values_for_rename_keys
+   h = { :name => "David", :street_name => "Paulina" }
+   assert_equal true,h.class.dasherize_xml
+   assert_equal false,h.camelize_xml
+  end
+  
   def test_one_level
     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
-
+  # we add :camelize => false because otherwise we'd be accidentally testing the default value for :camelize
   def test_one_level_dasherize_false
-    xml = { :name => "David", :street_name => "Paulina" }.to_xml(@xml_options.merge(:dasherize => false))
+    xml = { :name => "David", :street_name => "Paulina" }.to_xml(@xml_options.merge(:dasherize => false,:camelize=>false))
     assert_equal "<person>", xml.first(8)
     assert xml.include?(%(<street_name>Paulina</street_name>))
     assert xml.include?(%(<name>David</name>))
   end
 
   def test_one_level_dasherize_true
-    xml = { :name => "David", :street_name => "Paulina" }.to_xml(@xml_options.merge(:dasherize => true))
+    xml = { :name => "David", :street_name => "Paulina" }.to_xml(@xml_options.merge(:dasherize => true,:camelize=>false))
     assert_equal "<person>", xml.first(8)
     assert xml.include?(%(<street-name>Paulina</street-name>))
     assert xml.include?(%(<name>David</name>))
   end
+  
+  def test_one_level_dasherize_default_false
+    current_default = Hash.dasherize_xml
+    begin
+    Hash.dasherize_xml = false    
+    xml = { :name => "David", :street_name => "Paulina" }.to_xml(@xml_options.merge(:camelize=>false))
+    assert_equal "<person>", xml.first(8)
+    assert xml.include?(%(<street_name>Paulina</street_name>))
+    assert xml.include?(%(<name>David</name>))
+    ensure
+    Hash.dasherize_xml = current_default
+    end
+  end
+  
+  def test_one_level_dasherize_default_true
+    current_default = Hash.dasherize_xml
+    begin
+    Hash.dasherize_xml = true
+    xml = { :name => "David", :street_name => "Paulina" }.to_xml(@xml_options.merge(:camelize=>false))
+    assert_equal "<person>", xml.first(8)
+    assert xml.include?(%(<street-name>Paulina</street-name>))
+    assert xml.include?(%(<name>David</name>))
+    ensure
+    Hash.dasherize_xml = current_default
+    end
+  end
 
   def test_one_level_camelize_true
-    xml = { :name => "David", :street_name => "Paulina" }.to_xml(@xml_options.merge(:camelize => true))
+    xml = { :name => "David", :street_name => "Paulina" }.to_xml(@xml_options.merge(:camelize => true,:dasherize => false))
     assert_equal "<Person>", xml.first(8)
     assert xml.include?(%(<StreetName>Paulina</StreetName>))
     assert xml.include?(%(<Name>David</Name>))
   end
+  
+  #camelize=>false is already tested above
+  
+  def test_one_level_camelize_default_false 
+    current_default = Hash.camelize_xml
+    begin
+    Hash.camelize_xml = false 
+    xml = { :name => "David", :street_name => "Paulina" }.to_xml(@xml_options.merge(:dasherize => false))
+    assert_equal "<person>", xml.first(8)
+    assert xml.include?(%(<street_name>Paulina</street_name>))
+    assert xml.include?(%(<name>David</name>))
+    ensure
+    Hash.camelize_xml = current_default
+    end
+  end
+  
+  def test_one_level_camelize_default_true
+    current_default = Hash.camelize_xml
+    begin
+    Hash.camelize_xml = true 
+    xml = { :name => "David", :street_name => "Paulina" }.to_xml(@xml_options.merge(:dasherize => false))
+    assert_equal "<Person>", xml.first(8)
+    assert xml.include?(%(<StreetName>Paulina</StreetName>))
+    assert xml.include?(%(<Name>David</Name>))
+    ensure
+    Hash.camelize_xml = current_default
+    end
+  end
+  
+  def test_one_level_camelize_true_dasherize_true 
+    xml = { :name => "David", :street_name => "Paulina" }.to_xml(@xml_options.merge(:dasherize => true,:camelize=>true))
+    assert_equal "<Person>", xml.first(8)
+    assert xml.include?(%(<StreetName>Paulina</StreetName>))
+    assert xml.include?(%(<Name>David</Name>))
+  end  
 
   def test_one_level_with_types
     xml = { :name => "David", :street => "Paulina", :age => 26, :age_in_millis => 820497600000, :moved_on => Date.new(2005, 11, 15), :resident => :yes }.to_xml(@xml_options)
-- 
1.5.6.3


From f6d17f5c7bdc45f528003e1dd8f5f46c132badc5 Mon Sep 17 00:00:00 2001
From: John Small <[email protected]>
Date: Sat, 25 Apr 2009 07:07:56 +0100
Subject: [PATCH] This refers to lighthouse ticket #2521.

Changed documentation on ActiveResource#to_xml to correctly describe the behaviour of Hash#to_xml. The previous documentation said that
the default for :dasherize was false, in fact it was and still is true, but we now have a way to change the default. I've also added
documentation for the :camelize option.
---
 activeresource/lib/active_resource/base.rb |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/activeresource/lib/active_resource/base.rb b/activeresource/lib/active_resource/base.rb
index 2e74226..746a0a8 100644
--- a/activeresource/lib/active_resource/base.rb
+++ b/activeresource/lib/active_resource/base.rb
@@ -842,7 +842,14 @@ module ActiveResource
     #
     # * <tt>:indent</tt> - Set the indent level for the XML output (default is +2+).
     # * <tt>:dasherize</tt> - Boolean option to determine whether or not element names should
-    #   replace underscores with dashes (default is <tt>false</tt>).
+    #   replace underscores with dashes. Default is <tt>true</tt>. The default can be set to <tt>false</tt>
+    #   by setting the Hash class attribute <tt>Hash.dasherize_xml = false</tt> in an initializer. Because save
+    #   uses this method, and there are no options on save, then you will have to set the default if you don't
+    #   want underscores in element names to become dashes when the resource is saved. This is important when
+    #   integrating with non-Rails applications. 
+    # * <tt>:camelize</tt> - Boolean option to determine whether or not element names should be converted
+    #   to camel case, e.g some_name to SomeName. Default is <tt>false</tt>. Like <tt>:dasherize</tt> you can
+    #   change the default by setting the class attribute <tt>Hash.camelise_xml = true</tt> in an initializer.  
     # * <tt>:skip_instruct</tt> - Toggle skipping the +instruct!+ call on the XML builder
     #   that generates the XML declaration (default is <tt>false</tt>).
     #
-- 
1.5.6.3

hash_to_xml_patches.tar.gz