This project is archived and is in readonly mode.
Rails 2.3.8: Hash#deep_merge fails for HashWithIndifferentAccess
Reported by Georg Ledermann | May 26th, 2010 @ 08:11 AM | in 2.3.9
The following test fails with Rails 2.3.8
class HashTest < ActiveSupport::TestCase
def test_deep_merge_for_hash_with_indifferent_access
hash1 = { :k => { :v1 => 1 } }.with_indifferent_access
hash2 = { :k => { :v2 => 2 } }
assert_equal({ "k" => { "v1" => 1, "v2" => 2 } }, hash1.deep_merge(hash2))
end
end
Seems to be similar to #2732
Comments and changes to this ticket
-
Neeraj Singh May 26th, 2010 @ 03:53 PM
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 Pastorino May 26th, 2010 @ 05:00 PM
- Milestone set to 2.3.9
- State changed from new to open
Please do a patch file acording to the guidelines and a patch could be nice too ;).
You have the code there :).
Thanks. -
Neeraj Singh May 26th, 2010 @ 07:10 PM
@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.
-
Santiago Pastorino May 26th, 2010 @ 07:24 PM
Neeraj please review the latests tickets i think this one is duplicated
-
Neeraj Singh May 27th, 2010 @ 02:13 AM
I could not find any other open ticket about the same issue.
Anyway here is a patch with test against rails 2-3-stable.
-
Georg Ledermann May 27th, 2010 @ 10:36 AM
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
-
Neeraj Singh May 27th, 2010 @ 02:28 PM
@Georg Good catch.
Attached is an updated patch. I have included your test condition in the test this time. Thanks for the review.
-
Lawrence Pit May 31st, 2010 @ 05:40 AM
If it's properly fixed in rails 3, why can't that code be used instead of the code proposed in the patch?
-
Neeraj Singh May 31st, 2010 @ 05:59 AM
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.
-
Santiago Pastorino May 31st, 2010 @ 10:21 PM
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
-
Neeraj Singh June 1st, 2010 @ 08:40 AM
going to class Hash from module Hash requires a lot more changes than it seems.
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>