Blacklight::Solr::Request broken with regard to merge

[jrochkind]

Apparently Blacklight::Solr::Request is a hash-like class. It mostly masquerades as a Hash.

But real Hashes have a #merge method which optionally takes a block to provide custom merge behavior.

Blacklight::Solr::Request#merge seems to ignore a block argument.

A part of BlacklightAdvancedSearch, a solr_search_params_logic method with the usual solr_params, user_params args, assumes it can call merge with a block on solr_params. But solr_params are a Blacklight::Solr::Request, and it doesn't work. (It used to in older versions of BL)

Demonstrate with a stupid example:

real_hash = {"a" => [1]}

# silly custom merge logic that replaces everything with "FORCED"
real_hash.merge({"a" => "new"}) {|key, old, new| "FORCED" }
# => {"a"=>"FORCED"}

bl_hash = Blacklight::Solr::Request.new({"a" => [1]})
# => {"a"=>[1], "facet.field"=>[], "facet.query"=>[], "facet.pivot"=>[], "fq"=>[], "hl.fl"=>[]}
# Again with the silly logic
bl_hash.merge({"a" => "new"}) {|key, old, new| "FORCED" }
# But our silly logic was ignored, it is the same result as if we had not supplied
# a block argument. 
# => {"a"=>"new", "facet.field"=>[], "facet.query"=>[], "facet.pivot"=>[], "fq"=>[], "hl.fl"=>[]}

[jrochkind]

I am in Rails 3.2.17.

I tried with a straight HashWithIndifferentAccess -- a hwia is not broken, merge with a block arg works:

hwia = {"a" => "old"}
hwia.merge({"a" => "new"}) {|key, old, new| "FORCED" }
# => {"a"=>"FORCED"}

[cbeer]

@jrochkind are you really using HashWithIndifferentAccess (and just left something out of your example)? Here's what I get:

hwia = {"a" => "old"}.with_indifferent_access
irb(main):011:0> hwia.merge({"a" => "new"}) {|key, old, new| "FORCED" }
=> {"a"=>"new"}

[jrochkind]

ah, you're right, I got confused -- plain old HWIA is broken with regard to merge-with-block-arg in Rails 3.2.17 but apparently not 4. Grr.

So I guess it's a Rails bug, not a BL bug? Broke blacklight_advanced_search in a really confusing way just the same of course, in a really confusing way!

[cbeer]

Yep, they fixed it in Rails 4: rails/rails@edab820

[cbeer]

Is it worth backporting and/or monkey-patching HWIA in Blacklight? I hope not, but it's an option if it's a blocker to getting advanced search working.

[jrochkind]

It is not a blocker to advanced search, I simply rewrote the functionality to not use #merge, but 'reinvent the wheel' doing what merge was supposed to do. Not a problem.

projectblacklight/blacklight_advanced_search@6d3b49a

It might be a gotcha for other people, if they're expecting something that behaves like a hash there, and happen to want to use merge-with-block. I dunno. Such is life. I have no opinion on whether we should fix HWIA in general or Blacklight::Solr::Request specifically in BL. If anything, probably just Blacklight::Solr::Request specifically, since that's our responsibility, rather than trying to "fix Rails" for Rails.

But I've worked around it where I ran into it.

[cbeer]

Ok. I'm going to close this issue, but if it becomes a problem for any other Rails 3.x-based apps, let's reopen it and fix it in one of those places.

[atz]

FYI, I added HWIA testing and reworked deep_merge in projectblacklight/blacklight_advanced_search#59

For the most part I don't think we have to worry about rails 3 anymore.