Rails 2.3.8: Hash#deep_merge fails for HashWithIndifferentAccess

It is properly fixed in rails3. However if you need this functionality in rails 2.3.8 then a quick hack would be like this. I tested it and it is working although did not do any extensive test.

class HashWithIndifferentAccess 
  def deep_merge(other_hash)
    orig_hash = self
    other_hash.each_pair do |key, value| 
      key = convert_key(key)
      oldval = orig_hash[key]
      newval = convert_value(value)
      
      oldval = oldval.to_hash if oldval.respond_to?(:to_hash)
      newval = newval.to_hash if newval.respond_to?(:to_hash)

      merged_value = oldval.kind_of?(Hash) && newval.kind_of?(Hash) ?  oldval.deep_merge(newval) : newval
      orig_hash.regular_writer(key, merged_value)
    end
    self
  end
end

@Santiago I will have the patch and the test soon. Did not think that the fix would be backported since it is already fixed in rails3.

Neeraj please review the latests tickets i think this one is duplicated

I could not find any other open ticket about the same issue.

Anyway here is a patch with test against rails 2-3-stable.

IMHO this patch introduces another bug, because it changes self.

Here is a failing test (based on my test above):

class HashTest < ActiveSupport::TestCase
  def test_deep_merge_for_hash_with_indifferent_access
    hash1 = { :k => { :v1 => 1 } }.with_indifferent_access
    hash2 = { :k => { :v2 => 2 } }

    # First, check the result of a deep merge => OK
    assert_equal({ "k" => { "v1" => 1, "v2" => 2 } }, hash1.deep_merge(hash2))

    # Second, check if the originals are unchanged =====> FAILS!
    assert_equal({ "k" => { "v1" => 1 } }, hash1)
  end 
end

@Georg Good catch.

Attached is an updated patch. I have included your test condition in the test this time. Thanks for the review.

If it's properly fixed in rails 3, why can't that code be used instead of the code proposed in the patch?

In rails3 the fix is much more elegant and requires changing a few more files.

Patch that I have attached basically does the same thing.

Neeraj this is not related with this issue https://rails.lighthouseapp.com/projects/8994/tickets/2732-deep_mer... i didn't review the issues but seems similar, please take a look

going to class Hash from module Hash requires a lot more changes than it seems.