This project is archived and is in readonly mode.
[PATCH] deep_merge does not work on HashWithIndifferentAccess
Reported by david.cizek (at gmail) | May 28th, 2009 @ 10:29 AM | in 2.3.10
When I have 2 hashes (HashWithIndifferntAccess) and do deep_merge, it seems that it do not merge.
(rdb:1) params {"submit"=>"Where", "action"=>"query", "controller"=>"locations", "query"=>{"text"=>"50,14"}} (rdb:1) params_new {:query=>{:type=>"coordinates"}, :location=>{:geo_attributes=>{:coordinates=>"50.0,14.0"}}}
(rdb:1) params_after_deep_merge {"submit"=>"Where", "action"=>"query", "controller"=>"locations", "query"=>{"type"=>"coordinates"}, "location"=>{"geo_attributes"=>{"coordinates"=>"50.0,14.0"}}}
(missing "query" => {"text" ...})
When both hashes are regular Hashes (not WithIndifferentAccess), they merge ok.
{"submit"=>"Where", "action"=>"query", "controller"=>"locations", "query"=>{"type"=>"coordinates", "text"=>"50,14"}, "location"=>{"geo_attributes"=>{"coordinates"=>"50.0,14.0"}}}
Rails 2.3.2, OS X
It seems that in
File
vendor/rails/activesupport/lib/active_support/core_ext/hash/deep_merge.rb
line11:
oldval.class.to_s == 'Hash' && newval.class.to_s == 'Hash'
? oldval.deep_merge(newval) : newval
should be something like:
oldval.is_a?(Hash) && newval.is_a?(Hash) ? oldval.deep_merge(newval) : newval
Comments and changes to this ticket
-
david.cizek (at gmail) May 28th, 2009 @ 11:45 AM
My last "should be something..." was stupid, I did not mentioned?! 2 lines before, sorry. But still... does not work.
I tried to document the behaviour little bit more on:
http://gist.github.com/119217David
-
CancelProfileIsBroken August 6th, 2009 @ 02:53 PM
- Tag set to bugmash
-
Derander August 9th, 2009 @ 04:58 AM
- Tag changed from bugmash to bugmash, patch
+1 verified.
I packaged the (slightly tweaked) above methods into a patch with failing tests.
The patch should apply against 2-3-stable.
-
Derander August 9th, 2009 @ 04:59 AM
- Title changed from deep_merge does not work on HashWithIndifferentAccess to [PATCH] deep_merge does not work on HashWithIndifferentAccess
-
Rizwan Reza August 9th, 2009 @ 05:06 AM
- Tag changed from bugmash, patch to 2.3.x, bugmash, patch, verified
verified
+1 The patch applies cleanly under 2-3-stable only.
-
Kieran P August 9th, 2009 @ 05:14 AM
- Tag changed from 2.3.x, bugmash, patch, verified to bugmash, patch
+1 Verified, and patch applies to 2-3 cleanly and fixes included test.
Also ported to master. Attaching patch now.
-
Jeremy Kemper August 9th, 2009 @ 09:56 PM
Kieran, when you apply commits from 2-3-stable to master (or vice versa), use
git cherry-pick <revision>
to retain the original authorship and commit message. -
José Valim August 9th, 2009 @ 11:15 PM
We cannot put Rails current deep_merge into a module and add the module to both Hash and ActiveSupport::HashWithIndifferentAccess?
-
Derander August 9th, 2009 @ 11:41 PM
I don't think a mixin is needed because HashWithIndiff inherits from Hash.
Anyways, here is an alternate implementation of the above patch where it replaces Hash's deep_merge method
Tests all pass on master
-
Tristan Dunn August 9th, 2009 @ 11:49 PM
+1
Verified Derander's patch applies cleanly to master and tests are passing.
-
Repository August 10th, 2009 @ 01:34 AM
- State changed from new to committed
(from [ca92d44e7637ae6d28d6b88b67873d2795290cb5]) Support deep-merging HashWithIndifferentAccess.
[#2732 state:committed]
Signed-off-by: Jeremy Kemper jeremy@bitsweat.net
http://github.com/rails/rails/commit/ca92d44e7637ae6d28d6b88b67873d... -
CancelProfileIsBroken August 10th, 2009 @ 02:14 AM
- Tag changed from bugmash, patch to patch
- Milestone cleared.
-
Santiago Pastorino June 1st, 2010 @ 09:47 AM
- Milestone set to 2.3.9
- Assigned user set to Jeremy Kemper
We should apply the patch of Derander here https://rails.lighthouseapp.com/projects/8994/tickets/2732/a/239253...
-
Santiago Pastorino June 1st, 2010 @ 09:48 AM
- State changed from committed to open
-
Jeremy Kemper August 30th, 2010 @ 02:28 AM
- Milestone changed from 2.3.9 to 2.3.10
- Importance changed from to
-
Santiago Pastorino February 2nd, 2011 @ 04:32 PM
This issue has been automatically marked as stale because it has not been commented on for at least three months.
The resources of the Rails core team are limited, and so we are asking for your help. If you can still reproduce this error on the 3-0-stable branch or on master, please reply with all of the information you have about it and add "[state:open]" to your comment. This will reopen the ticket for review. Likewise, if you feel that this is a very important feature for Rails to include, please reply with your explanation so we can consider it.
Thank you for all your contributions, and we hope you will understand this step to focus our efforts where they are most helpful.
-
Santiago Pastorino February 2nd, 2011 @ 04:32 PM
- State changed from open to stale
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>
People watching this ticket
Attachments
Tags
Referenced by
- 2732 [PATCH] deep_merge does not work on HashWithIndifferentAccess [#2732 state:committed]
- 3316 has_many :through with :conditions fails The association.replace() call around line #1322 of assoc...
- 4702 Rails 2.3.8: Hash#deep_merge fails for HashWithIndifferentAccess Seems to be similar to #2732
- 4702 Rails 2.3.8: Hash#deep_merge fails for HashWithIndifferentAccess Neeraj this is not related with this issue https://rails...
- 2732 [PATCH] deep_merge does not work on HashWithIndifferentAccess We should apply the patch of Derander here https://rails...
- 4702 Rails 2.3.8: Hash#deep_merge fails for HashWithIndifferentAccess #2732