MrDys / blacklight

Blacklight Plugin
http://projectblacklight.org/
Other
1 stars 1 forks source link

solr_search_params refactor #492

Closed MrDys closed 12 years ago

MrDys commented 12 years ago

CODEBASE-307: refactor of solr_search_params.

Removes "extra_controller_params", instead being more clear about input hash (from blacklight rails app URI query), being mapped to output hash (of params sent in query to Solr).

If you want to add more 'simulated' input params, you can just do that, on the hash passed in as the input:

solr_search_params( params.merge(:new_one => "something"))

If you want to 'force' some output solr params, you can just merge them into the output:

solr_search_params().merge(:defType => "something")

Because of change in signature of method, may not be backwards compatible with local customizations or plugins. However, the actual mapping semantics have not changed, all existing tests still pass.

Additionally, the logic was refactored from giant spaghetti method to individual methods focusing on specific things. And a an design reminiscent of rails 'before_filter' is added to allow you to add and remove such pieces of logic, which will be helpful for implementing plugins and local features cleanly. (Before they had to over-ride solr_search_params and call super, which made things somewhat confusing to debug, and didn't provide as much control over ordering of operations).

An example from the spec:

    # Normally you'd include a new module into (eg) your CatalogController
    # but a sub-class defininig it directly is simpler for test. 
    class CustomizableHelper < MockSolrHelperContainer          
        def add_foo_to_solr_params(solr_params, user_params)
          solr_params[:foo] = "TESTING"
        end
    end 

    CustomizableHelper.solr_search_params_logic << :add_foo_to_solr_params
    # Usually CatalogController.solr_search_params << :add_foo_to_solr_params

    obj = CustomizableHelper.new
    solr_params = obj.solr_search_params

    solr_params[:foo].should == "TESTING"
MrDys commented 12 years ago

Original reporter: jrochkind

MrDys commented 12 years ago

jrochkind: About to merge this into master, but it will require some refactoring of all our 'offiical' plugins, which I'll also do.

MrDys commented 12 years ago

jrochkind: adv search seems to still work, changes were backwards compatible to it. (It's still doing some odd things with "extra_controller_params" that will probably confuse if left there forever).

MrDys commented 12 years ago

jrochkind: date range plugin likewise seems to still work, refactor was backwards compatible with it.

MrDys commented 12 years ago

jrochkind: And yeah, CQL works too.