Merge pull request #20828 from Sirupsen/hash-indifferent-dup-default-proc

active_support/indifferent_access: fix not raising when default_proc does

4 files changed, 58 insertions, 13 deletions

diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md
index 6ebbdbc3db..4ffaa666b9 100644
--- a/activesupport/CHANGELOG.md
+++ b/activesupport/CHANGELOG.md
@@ -1,3 +1,12 @@
+*   Fix not calling `#default` on `HashWithIndifferentAcess#to_hash` when only
+    `default_proc` is set, which could raise.
+
+    *Simon Eskildsen*
+
+*   Fix setting `default_proc` on `HashWithIndifferentAccess#dup`
+
+    *Simon Eskildsen*
+
 *   Fix a range of values for parameters of the Time#change
 
     *Nikolay Kondratyev*
diff --git a/activesupport/lib/active_support/hash_with_indifferent_access.rb b/activesupport/lib/active_support/hash_with_indifferent_access.rb
index 4f71f13971..63690a1342 100644
--- a/activesupport/lib/active_support/hash_with_indifferent_access.rb
+++ b/activesupport/lib/active_support/hash_with_indifferent_access.rb
@@ -188,7 +188,7 @@ module ActiveSupport
     #   dup[:a][:c]  # => "c"
     def dup
       self.class.new(self).tap do |new_hash|
-        new_hash.default = default
+        set_defaults(new_hash)
       end
     end
 
@@ -247,7 +247,9 @@ module ActiveSupport
 
     # Convert to a regular hash with string keys.
     def to_hash
-      _new_hash = Hash.new(default)
+      _new_hash = Hash.new
+      set_defaults(_new_hash)
+
       each do |key, value|
         _new_hash[key] = convert_value(value, for: :to_hash)
       end
@@ -275,6 +277,14 @@ module ActiveSupport
           value
         end
       end
+
+      def set_defaults(target)
+        if default_proc
+          target.default_proc = default_proc.dup
+        else
+          target.default = default
+        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 5d210c958e..febe41ec4d 100644
--- a/activesupport/test/core_ext/hash_ext_test.rb
+++ b/activesupport/test/core_ext/hash_ext_test.rb
@@ -523,6 +523,12 @@ class HashExtTest < ActiveSupport::TestCase
     assert_equal 5, merged[:b]
   end
 
+  def test_reverse_merge
+    hash = HashWithIndifferentAccess.new key: :old_value
+    hash.reverse_merge! key: :new_value
+    assert_equal :old_value, hash[:key]
+  end
+
   def test_indifferent_reverse_merging
     hash = HashWithIndifferentAccess.new('some' => 'value', 'other' => 'value')
     hash.reverse_merge!(:some => 'noclobber', :another => 'clobber')
@@ -999,6 +1005,37 @@ class HashExtTest < ActiveSupport::TestCase
     assert hash.key?('a')
     assert_equal 1, hash[:a]
   end
+
+  def test_dup_with_default_proc
+    hash = HashWithIndifferentAccess.new
+    hash.default_proc = proc { |h, v| raise "walrus" }
+    assert_nothing_raised { hash.dup }
+  end
+
+  def test_dup_with_default_proc_sets_proc
+    hash = HashWithIndifferentAccess.new
+    hash.default_proc = proc { |h, k| k + 1 }
+    new_hash = hash.dup
+
+    assert_equal 3, new_hash[2]
+
+    new_hash.default = 2
+    assert_equal 2, new_hash[:non_existant]
+  end
+
+  def test_to_hash_with_raising_default_proc
+    hash = HashWithIndifferentAccess.new
+    hash.default_proc = proc { |h, k| raise "walrus" }
+
+    assert_nothing_raised { hash.to_hash }
+  end
+
+  def test_new_from_hash_copying_default_should_not_raise_when_default_proc_does
+    hash = Hash.new
+    hash.default_proc = proc { |h, k| raise "walrus" }
+
+    assert_nothing_raised { HashWithIndifferentAccess.new_from_hash_copying_default(hash) }
+  end
 end
 
 class IWriteMyOwnXML
diff --git a/activesupport/test/hash_with_indifferent_access_test.rb b/activesupport/test/hash_with_indifferent_access_test.rb
deleted file mode 100644
index 1facd691fa..0000000000
--- a/activesupport/test/hash_with_indifferent_access_test.rb
+++ /dev/null
@@ -1,11 +0,0 @@
-require 'abstract_unit'
-require 'active_support/hash_with_indifferent_access'
-
-class HashWithIndifferentAccessTest < ActiveSupport::TestCase
-  def test_reverse_merge
-    hash = HashWithIndifferentAccess.new key: :old_value
-    hash.reverse_merge! key: :new_value
-    assert_equal :old_value, hash[:key]
-  end
-
-end