Fixed HashWithIndifferentAccess#delete to work with both symbols and strings (closes #2176) [Caio Chassot]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@3912 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

3 files changed, 26 insertions, 36 deletions

diff --git a/activesupport/CHANGELOG b/activesupport/CHANGELOG
index ec676773bb..4d2533e782 100644
--- a/activesupport/CHANGELOG
+++ b/activesupport/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Fixed HashWithIndifferentAccess#delete to work with both symbols and strings #2176 [Caio Chassot]
+
 * Added Time#advance to do precise time time calculations for cases where a month being approximated to 30 days won't do #1860 [Rick Olson]
 
 * Enhance Inflector.underscore to convert '-' into '_' (as the inverse of Inflector.dasherize) [Jamis Buck]
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 adfba0fbf3..c84c37f87b 100644
--- a/activesupport/lib/active_support/core_ext/hash/indifferent_access.rb
+++ b/activesupport/lib/active_support/core_ext/hash/indifferent_access.rb
@@ -9,54 +9,32 @@ class HashWithIndifferentAccess < Hash
     end
   end
  
-  def default(key)
-    self[key.to_s] if key.is_a?(Symbol)
-  end  
+  convert_key_and_hashes_and_call_super = lambda { |key, *args| super(convert_key(key), *args.map{|arg| convert_value(arg) }) }
+  convert_other_hash_and_call_super     = lambda { |other_hash| super(convert_hash(other_hash)) }
 
-  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
-
-  def update(other_hash)
-    other_hash.each_pair {|key, value| regular_writer(convert_key(key), convert_value(value))}
-    self
-  end
-  alias_method :merge!, :update
-
-  def key?(key)
-    super(convert_key(key))
-  end
-
-  alias_method :include?, :key?
-  alias_method :has_key?, :key?
-  alias_method :member?, :key?
+  %w( [] []= fetch store delete has_key? include? key? member? ).each{ |method_name| define_method method_name, &convert_key_and_hashes_and_call_super }
+  %w( == eql? replace initialize_copy merge merge! update      ).each{ |method_name| define_method method_name, &convert_other_hash_and_call_super     }
 
-  def fetch(key, *extras)
-    super(convert_key(key), *extras)
-  end
-
-  def values_at(*indices)
-    indices.collect {|key| self[convert_key(key)]}
-  end
-
-  def dup
-    HashWithIndifferentAccess.new(self)
+  def invert
+    self.class.new.replace(super)
   end
   
-  def merge(hash)
-    self.dup.update(hash)
+  def values_at(*keys)
+    super *keys.map{ |key| convert_key(key) }
   end
-  
+
   protected
     def convert_key(key)
       key.kind_of?(Symbol) ? key.to_s : key
     end
+    
     def convert_value(value)
       value.is_a?(Hash) ? value.with_indifferent_access : value
     end
+    
+    def convert_hash(hash)
+      hash.is_a?(Hash) ? hash.inject({}){ |h,(k,v)| h[convert_key(k)] = convert_value(v); h } : hash
+    end
 end
 
 module ActiveSupport #:nodoc:
diff --git a/activesupport/test/core_ext/hash_ext_test.rb b/activesupport/test/core_ext/hash_ext_test.rb
index 43b0b29216..a11cef6fc4 100644
--- a/activesupport/test/core_ext/hash_ext_test.rb
+++ b/activesupport/test/core_ext/hash_ext_test.rb
@@ -135,6 +135,16 @@ class HashExtTest < Test::Unit::TestCase
     assert_equal 2, hash['b']
   end
   
+  def test_indifferent_deleting
+    get_hash = proc{ { :a => 'foo' }.with_indifferent_access }
+    hash = get_hash.call
+    assert_equal hash.delete(:a), 'foo'
+    assert_equal hash.delete(:a), nil
+    hash = get_hash.call
+    assert_equal hash.delete('a'), 'foo'
+    assert_equal hash.delete('a'), nil
+  end
+
   def test_assert_valid_keys
     assert_nothing_raised do
       { :failure => "stuff", :funny => "business" }.assert_valid_keys([ :failure, :funny ])