From 5bc866f77303144a0b5ed55141152c270283e24a Mon Sep 17 00:00:00 2001 From: John Small 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 original_filename and content_type 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 "", xml.first(8) assert xml.include?(%(Paulina)) assert xml.include?(%(David)) 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 "", xml.first(8) assert xml.include?(%(Paulina)) assert xml.include?(%(David)) 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 "", xml.first(8) assert xml.include?(%(Paulina)) assert xml.include?(%(David)) 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 "", xml.first(8) + assert xml.include?(%(Paulina)) + assert xml.include?(%(David)) + 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 "", xml.first(8) + assert xml.include?(%(Paulina)) + assert xml.include?(%(David)) + 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 "", xml.first(8) assert xml.include?(%(Paulina)) assert xml.include?(%(David)) 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 "", xml.first(8) + assert xml.include?(%(Paulina)) + assert xml.include?(%(David)) + 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 "", xml.first(8) + assert xml.include?(%(Paulina)) + assert xml.include?(%(David)) + 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 "", xml.first(8) + assert xml.include?(%(Paulina)) + assert xml.include?(%(David)) + 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 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 # # * :indent - Set the indent level for the XML output (default is +2+). # * :dasherize - Boolean option to determine whether or not element names should - # replace underscores with dashes (default is false). + # replace underscores with dashes. Default is true. The default can be set to false + # by setting the Hash class attribute Hash.dasherize_xml = false 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. + # * :camelize - Boolean option to determine whether or not element names should be converted + # to camel case, e.g some_name to SomeName. Default is false. Like :dasherize you can + # change the default by setting the class attribute Hash.camelise_xml = true in an initializer. # * :skip_instruct - Toggle skipping the +instruct!+ call on the XML builder # that generates the XML declaration (default is false). # -- 1.5.6.3