diff --git a/activesupport/CHANGELOG b/activesupport/CHANGELOG index 6d81723705..62450807c2 100644 --- a/activesupport/CHANGELOG +++ b/activesupport/CHANGELOG @@ -1,5 +1,7 @@ *SVN* +* HashWithIndifferentAccess shouldn't confuse false and nil. #5601 [shugo@ruby-lang.org] + * Fixed HashWithIndifferentAccess#default #5586 [chris@seagul.co.uk] * More compatible Hash.create_from_xml. #5523 [nunemaker@gmail.com] diff --git a/activesupport/lib/active_support/core_ext/hash/indifferent_access.rb b/activesupport/lib/active_support/core_ext/hash/indifferent_access.rb index 9af626c8da..9a3b02e5d5 100644 --- a/activesupport/lib/active_support/core_ext/hash/indifferent_access.rb +++ b/activesupport/lib/active_support/core_ext/hash/indifferent_access.rb @@ -10,15 +10,18 @@ class HashWithIndifferentAccess < Hash super(constructor) end end - + def default(key = nil) - value = self[key.to_s] if key.is_a?(Symbol) - value ? value : super + if key.is_a?(Symbol) && include?(key = key.to_s) + self[key] + else + super + end end alias_method :regular_writer, :[]= unless method_defined?(:regular_writer) alias_method :regular_update, :update unless method_defined?(:regular_update) - + def []=(key, value) regular_writer(convert_key(key), convert_value(value)) end @@ -27,7 +30,7 @@ class HashWithIndifferentAccess < Hash other_hash.each_pair { |key, value| regular_writer(convert_key(key), convert_value(value)) } self end - + alias_method :merge!, :update def key?(key) @@ -49,7 +52,7 @@ class HashWithIndifferentAccess < Hash def dup HashWithIndifferentAccess.new(self) end - + def merge(hash) self.dup.update(hash) end @@ -60,7 +63,7 @@ class HashWithIndifferentAccess < Hash def stringify_keys!; self end def symbolize_keys!; self end - + protected def convert_key(key) key.kind_of?(Symbol) ? key.to_s : key diff --git a/activesupport/test/core_ext/hash_ext_test.rb b/activesupport/test/core_ext/hash_ext_test.rb index 327d884ecd..28b4304500 100644 --- a/activesupport/test/core_ext/hash_ext_test.rb +++ b/activesupport/test/core_ext/hash_ext_test.rb @@ -50,18 +50,18 @@ class HashExtTest < Test::Unit::TestCase @strings = @strings.with_indifferent_access @symbols = @symbols.with_indifferent_access @mixed = @mixed.with_indifferent_access - + assert_equal 'a', @strings.send(:convert_key, :a) - + assert_equal 1, @strings.fetch('a') assert_equal 1, @strings.fetch(:a.to_s) assert_equal 1, @strings.fetch(:a) - + hashes = { :@strings => @strings, :@symbols => @symbols, :@mixed => @mixed } method_map = { :'[]' => 1, :fetch => 1, :values_at => [1], :has_key? => true, :include? => true, :key? => true, :member? => true } - + hashes.each do |name, hash| method_map.sort_by { |m| m.to_s }.each do |meth, expected| assert_equal(expected, hash.send(meth, 'a'), @@ -70,7 +70,7 @@ class HashExtTest < Test::Unit::TestCase "Calling #{name}.#{meth} :a") end end - + assert_equal [1, 2], @strings.values_at('a', 'b') assert_equal [1, 2], @strings.values_at(:a, :b) assert_equal [1, 2], @symbols.values_at('a', 'b') @@ -78,13 +78,41 @@ class HashExtTest < Test::Unit::TestCase assert_equal [1, 2], @mixed.values_at('a', 'b') assert_equal [1, 2], @mixed.values_at(:a, :b) end - + + def test_indifferent_reading + hash = HashWithIndifferentAccess.new + hash["a"] = 1 + hash["b"] = true + hash["c"] = false + hash["d"] = nil + + assert_equal 1, hash[:a] + assert_equal true, hash[:b] + assert_equal false, hash[:c] + assert_equal nil, hash[:d] + assert_equal nil, hash[:e] + end + + def test_indifferent_reading_with_nonnil_default + hash = HashWithIndifferentAccess.new(1) + hash["a"] = 1 + hash["b"] = true + hash["c"] = false + hash["d"] = nil + + assert_equal 1, hash[:a] + assert_equal true, hash[:b] + assert_equal false, hash[:c] + assert_equal nil, hash[:d] + assert_equal 1, hash[:e] + end + def test_indifferent_writing hash = HashWithIndifferentAccess.new hash[:a] = 1 hash['b'] = 2 hash[3] = 3 - + assert_equal hash['a'], 1 assert_equal hash['b'], 2 assert_equal hash[:a], 1 @@ -96,7 +124,7 @@ class HashExtTest < Test::Unit::TestCase hash = HashWithIndifferentAccess.new hash[:a] = 'a' hash['b'] = 'b' - + updated_with_strings = hash.update(@strings) updated_with_symbols = hash.update(@symbols) updated_with_mixed = hash.update(@mixed) @@ -119,21 +147,21 @@ class HashExtTest < Test::Unit::TestCase hash = HashWithIndifferentAccess.new hash[:a] = 'failure' hash['b'] = 'failure' - + other = { 'a' => 1, :b => 2 } - + merged = hash.merge(other) - + assert_equal HashWithIndifferentAccess, merged.class assert_equal 1, merged[:a] assert_equal 2, merged['b'] - + hash.update(other) - + assert_equal 1, hash[:a] assert_equal 2, hash['b'] end - + def test_indifferent_deleting get_hash = proc{ { :a => 'foo' }.with_indifferent_access } hash = get_hash.call @@ -160,7 +188,7 @@ class HashExtTest < Test::Unit::TestCase { :failure => "stuff", :funny => "business" }.assert_valid_keys([ :failure, :funny ]) { :failure => "stuff", :funny => "business" }.assert_valid_keys(:failure, :funny) end - + assert_raises(ArgumentError, "Unknown key(s): failore") do { :failore => "stuff", :funny => "business" }.assert_valid_keys([ :failure, :funny ]) { :failore => "stuff", :funny => "business" }.assert_valid_keys(:failure, :funny) @@ -170,7 +198,7 @@ class HashExtTest < Test::Unit::TestCase def test_indifferent_subhashes h = {'user' => {'id' => 5}}.with_indifferent_access ['user', :user].each {|user| [:id, 'id'].each {|id| assert_equal 5, h[user][id], "h[#{user.inspect}][#{id.inspect}] should be 5"}} - + h = {:user => {:id => 5}}.with_indifferent_access ['user', :user].each {|user| [:id, 'id'].each {|id| assert_equal 5, h[user][id], "h[#{user.inspect}][#{id.inspect}] should be 5"}} end @@ -265,7 +293,7 @@ class HashToXmlTest < Test::Unit::TestCase assert xml.include?(%(