From 40dbebba28bfa1c55737da7354542c3bdca4e1a1 Mon Sep 17 00:00:00 2001 From: Lawrence Pit Date: Mon, 14 Jul 2008 11:53:41 +1000 Subject: Allow deep merging of hash values for nested with_options. [#490 state:resolved] Signed-off-by: Pratik Naik --- activesupport/lib/active_support/core_ext/hash.rb | 3 ++- .../lib/active_support/core_ext/hash/deep_merge.rb | 23 ++++++++++++++++++ activesupport/lib/active_support/option_merger.rb | 10 +------- activesupport/test/core_ext/hash_ext_test.rb | 10 ++++++++ activesupport/test/option_merger_test.rb | 27 ++++++++++++++++++++++ 5 files changed, 63 insertions(+), 10 deletions(-) create mode 100644 activesupport/lib/active_support/core_ext/hash/deep_merge.rb diff --git a/activesupport/lib/active_support/core_ext/hash.rb b/activesupport/lib/active_support/core_ext/hash.rb index 6cbd9dd378..a6065ab48e 100644 --- a/activesupport/lib/active_support/core_ext/hash.rb +++ b/activesupport/lib/active_support/core_ext/hash.rb @@ -1,10 +1,11 @@ -%w(keys indifferent_access reverse_merge conversions diff slice except).each do |ext| +%w(keys indifferent_access deep_merge reverse_merge conversions diff slice except).each do |ext| require "active_support/core_ext/hash/#{ext}" end class Hash #:nodoc: include ActiveSupport::CoreExtensions::Hash::Keys include ActiveSupport::CoreExtensions::Hash::IndifferentAccess + include ActiveSupport::CoreExtensions::Hash::DeepMerge include ActiveSupport::CoreExtensions::Hash::ReverseMerge include ActiveSupport::CoreExtensions::Hash::Conversions include ActiveSupport::CoreExtensions::Hash::Diff diff --git a/activesupport/lib/active_support/core_ext/hash/deep_merge.rb b/activesupport/lib/active_support/core_ext/hash/deep_merge.rb new file mode 100644 index 0000000000..f8842ba57a --- /dev/null +++ b/activesupport/lib/active_support/core_ext/hash/deep_merge.rb @@ -0,0 +1,23 @@ +module ActiveSupport #:nodoc: + module CoreExtensions #:nodoc: + module Hash #:nodoc: + # Allows for deep merging + module DeepMerge + # Returns a new hash with +self+ and +other_hash+ merged recursively. + def deep_merge(other_hash) + self.merge(other_hash) do |key, oldval, newval| + oldval = oldval.to_hash if oldval.respond_to?(:to_hash) + newval = newval.to_hash if newval.respond_to?(:to_hash) + oldval.class.to_s == 'Hash' && newval.class.to_s == 'Hash' ? oldval.deep_merge(newval) : newval + end + end + + # Returns a new hash with +self+ and +other_hash+ merged recursively. + # Modifies the receiver in place. + def deep_merge!(other_hash) + replace(deep_merge(other_hash)) + end + end + end + end +end diff --git a/activesupport/lib/active_support/option_merger.rb b/activesupport/lib/active_support/option_merger.rb index 1a4ff9db9a..c77bca1ac9 100644 --- a/activesupport/lib/active_support/option_merger.rb +++ b/activesupport/lib/active_support/option_merger.rb @@ -10,16 +10,8 @@ module ActiveSupport private def method_missing(method, *arguments, &block) - merge_argument_options! arguments + arguments << (arguments.last.respond_to?(:to_hash) ? @options.deep_merge(arguments.pop) : @options.dup) @context.send!(method, *arguments, &block) end - - def merge_argument_options!(arguments) - arguments << if arguments.last.respond_to? :to_hash - @options.merge(arguments.pop) - else - @options.dup - end - end end end diff --git a/activesupport/test/core_ext/hash_ext_test.rb b/activesupport/test/core_ext/hash_ext_test.rb index 26e65075eb..5d2053f106 100644 --- a/activesupport/test/core_ext/hash_ext_test.rb +++ b/activesupport/test/core_ext/hash_ext_test.rb @@ -245,6 +245,16 @@ class HashExtTest < Test::Unit::TestCase assert(!indiff.keys.any? {|k| k.kind_of? String}, "A key was converted to a string!") end + def test_deep_merge + hash_1 = { :a => "a", :b => "b", :c => { :c1 => "c1", :c2 => "c2", :c3 => { :d1 => "d1" } } } + hash_2 = { :a => 1, :c => { :c1 => 2, :c3 => { :d2 => "d2" } } } + expected = { :a => 1, :b => "b", :c => { :c1 => 2, :c2 => "c2", :c3 => { :d1 => "d1", :d2 => "d2" } } } + assert_equal expected, hash_1.deep_merge(hash_2) + + hash_1.deep_merge!(hash_2) + assert_equal expected, hash_1 + end + def test_reverse_merge defaults = { :a => "x", :b => "y", :c => 10 }.freeze options = { :a => 1, :b => 2 } diff --git a/activesupport/test/option_merger_test.rb b/activesupport/test/option_merger_test.rb index 509c6d3bad..0d72314880 100644 --- a/activesupport/test/option_merger_test.rb +++ b/activesupport/test/option_merger_test.rb @@ -38,6 +38,33 @@ class OptionMergerTest < Test::Unit::TestCase end end + def test_nested_method_with_options_containing_hashes_merge + with_options :conditions => { :method => :get } do |outer| + outer.with_options :conditions => { :domain => "www" } do |inner| + expected = { :conditions => { :method => :get, :domain => "www" } } + assert_equal expected, inner.method_with_options + end + end + end + + def test_nested_method_with_options_containing_hashes_overwrite + with_options :conditions => { :method => :get, :domain => "www" } do |outer| + outer.with_options :conditions => { :method => :post } do |inner| + expected = { :conditions => { :method => :post, :domain => "www" } } + assert_equal expected, inner.method_with_options + end + end + end + + def test_nested_method_with_options_containing_hashes_going_deep + with_options :html => { :class => "foo", :style => { :margin => 0, :display => "block" } } do |outer| + outer.with_options :html => { :title => "bar", :style => { :margin => "1em", :color => "#fff" } } do |inner| + expected = { :html => { :class => "foo", :title => "bar", :style => { :margin => "1em", :display => "block", :color => "#fff" } } } + assert_equal expected, inner.method_with_options + end + end + end + # Needed when counting objects with the ObjectSpace def test_option_merger_class_method assert_equal ActiveSupport::OptionMerger, ActiveSupport::OptionMerger.new('', '').class -- cgit v1.2.3