Pass the Blacklight::SearchBuilder object to the repository instead o…

…f just the request parameters

lib/blacklight/abstract_repository.rb

@@ -16,7 +16,6 @@ def connection
       @connection ||= build_connection
     end
 
-
     protected
       def connection_config
         blacklight_config.connection_config

lib/blacklight/request_builders.rb

@@ -90,7 +90,7 @@ def solr_search_params(user_params = params || {}, processor_chain = search_para
     # @param [Hash] extra_params an optional hash of parameters that should be
     #                            added to the query post processing
     def build_solr_query(user_params, processor_chain, extra_params=nil)
-      search_builder(processor_chain).with(user_params).query(extra_params)
+      search_builder(processor_chain).with(user_params).merge(extra_params)
     end
     deprecation_deprecate build_solr_query: :query
 
@@ -107,7 +107,7 @@ def solr_document_ids_params(ids = [])
     # Retrieve the results for a list of document ids
     # @deprecated
     def solr_documents_by_field_values_params(field, values)
-      search_builder([:add_query_to_solr]).with(q: { field => values}).query(fl: '*')
+      search_builder([:add_query_to_solr]).with(q: { field => values}).merge(fl: '*')
     end
     deprecation_deprecate :solr_documents_by_field_values_params
 

lib/blacklight/search_builder.rb

@@ -19,35 +19,86 @@ def initialize(processor_chain, scope)
 
       @scope = scope
       @blacklight_params = {}
+      @merged_params = {}
+      @reverse_merged_params = {}
     end
 
     ##
     # Set the parameters to pass through the processor chain
     def with blacklight_params = {}
+      params_will_change!
       @blacklight_params = blacklight_params.dup
       self
     end
 
     ##
     # Update the :q (query) parameter
     def where conditions
+      params_will_change!
       @blacklight_params[:q] = conditions
       self
     end
 
     ##
     # Append additional processor chain directives
     def append *addl_processor_chain
-      self.class.new(processor_chain + addl_processor_chain, scope).with(blacklight_params)
+      params_will_change!
+      builder = self.class.new(processor_chain + addl_processor_chain, scope)
+          .with(blacklight_params)
+          .merge(@merged_params)
+          .reverse_merge(@reverse_merged_params)
+
+      builder.start(@start) if @start
+      builder.rows(@rows) if @rows
+      builder.page(@page) if @page
+
+      builder
     end
 
+    ##
+    # Merge additional, repository-specific parameters
+    def merge extra_params, &block
+      if extra_params
+        params_will_change!
+        @merged_params.merge!(extra_params.to_h, &block)
+      end
+      self
+    end
+
+    ##
+    # "Reverse merge" additional, repository-specific parameters
+    def reverse_merge extra_params, &block
+      if extra_params
+        params_will_change!
+        @reverse_merged_params.reverse_merge!(extra_params.to_h, &block)
+      end
+      self
+    end
+
+    delegate :[], :key?, to: :to_hash
+
     # a solr query method
     # @param [Hash,HashWithIndifferentAccess] extra_controller_params (nil) extra parameters to add to the search
     # @return [Blacklight::SolrResponse] the solr response object
-    def query(extra_params = nil)
-      extra_params ? processed_parameters.merge(extra_params) : processed_parameters
+    def to_hash method_extra_params = nil
+      unless method_extra_params.nil?
+        Deprecation.warn(Blacklight::SearchBuilder, "Calling SearchBuilder#query with extra parameters is deprecated. Use #merge(Hash) instead")
+        merge(method_extra_params)
+      end
+
+      if params_need_update?
+        @params = processed_parameters.
+                    reverse_merge(@reverse_merged_params).
+                    merge(@merged_params).
+                    tap { self.clear_changes }
+      else
+        @params
+      end
     end
 
+    alias_method :query, :to_hash
+    alias_method :to_h, :to_hash
+
     # @returns a params hash for searching solr.
     # The CatalogController #index action uses this.
     # Solr parameters can come from a number of places. From lowest
@@ -82,6 +133,7 @@ def blacklight_config
 
     def start start = nil
       if start
+        params_will_change!
         @start = start.to_i
         self
       else
@@ -96,6 +148,7 @@ def start start = nil
 
     def page page = nil
       if page
+        params_will_change!
         @page = page.to_i
         @page = 1 if @page < 1
         self
@@ -114,6 +167,7 @@ def page page = nil
 
     def rows rows = nil
       if rows
+        params_will_change!
         @rows = rows.to_i
         @rows = blacklight_config.max_per_page if @rows > blacklight_config.max_per_page
         self
@@ -135,11 +189,6 @@ def rows rows = nil
 
     alias_method :per, :rows
 
-    protected
-    def request
-      Blacklight::Solr::Request.new
-    end
-
     def sort
       field = if blacklight_params[:sort].blank? and sort_field = blacklight_config.default_sort_field
         # no sort param provided, use default
@@ -158,13 +207,34 @@ def sort
     def search_field
       blacklight_config.search_fields[blacklight_params[:search_field]]
     end
-
+
+    protected
+    def request
+      Blacklight::Solr::Request.new
+    end
+
     def should_add_field_to_request? field_name, field
       field.include_in_request || (field.include_in_request.nil? && blacklight_config.add_field_configuration_to_solr_request)
     end
 
     def scope
       @scope
     end
+
+    def params_will_change!
+      @dirty = true
+    end
+
+    def params_changed?
+      !!@dirty
+    end
+
+    def params_need_update?
+      params_changed? || @params.nil?
+    end
+
+    def clear_changes
+      @dirty = false
+    end
   end
 end

lib/blacklight/search_helper.rb

@@ -78,7 +78,7 @@ def solr_doc_params(id=nil)
   # Returns a two-element array (aka duple) with first the solr response object,
   # and second an array of SolrDocuments representing the response.docs
   def get_search_results(user_params = params || {}, extra_controller_params = {})
-    query = search_builder.with(user_params).query(extra_controller_params)
+    query = search_builder.with(user_params).merge(extra_controller_params)
     response = repository.search(query)
 
     case
@@ -102,7 +102,7 @@ def search_results(user_params, search_params_logic)
     builder.page(user_params[:page]) if user_params[:page]
     builder.rows(user_params[:per_page] || user_params[:rows]) if user_params[:per_page] or user_params[:rows]
 
-    response = repository.search(builder.query)
+    response = repository.search(builder)
 
     case
     when (response.grouped? && grouped_key_for_results)
@@ -119,7 +119,7 @@ def search_results(user_params, search_params_logic)
   # @param [Hash,HashWithIndifferentAccess] extra_controller_params ({}) extra parameters to add to the search
   # @return [Blacklight::SolrResponse] the solr response object
   def query_solr(user_params = params || {}, extra_controller_params = {})
-    query = search_builder.with(user_params).query(extra_controller_params)
+    query = search_builder.with(user_params).merge(extra_controller_params)
     repository.search(query)
   end
   deprecation_deprecate :query_solr
@@ -145,7 +145,7 @@ def fetch(id=nil, extra_controller_params={})
   # @return [Blacklight::SolrResponse, Array<Blacklight::SolrDocument>] the solr response object and a list of solr documents
   def get_solr_response_for_field_values(field, values, extra_controller_params = {})
     query = Deprecation.silence(Blacklight::RequestBuilders) do
-      search_builder.with(params).query(extra_controller_params.merge(solr_documents_by_field_values_params(field, values)))
+      search_builder.with(params).merge(extra_controller_params).merge(solr_documents_by_field_values_params(field, values))
     end
 
     solr_response = repository.search(query)
@@ -159,7 +159,7 @@ def get_solr_response_for_field_values(field, values, extra_controller_params =
   # Get the solr response when retrieving only a single facet field
   # @return [Blacklight::SolrResponse] the solr response
   def get_facet_field_response(facet_field, user_params = params || {}, extra_controller_params = {})
-    query = search_builder.with(user_params).query(extra_controller_params.merge(solr_facet_params(facet_field, user_params, extra_controller_params)))
+    query = search_builder.with(user_params).merge(extra_controller_params).merge(solr_facet_params(facet_field, user_params, extra_controller_params))
     repository.search(query)
   end
 
@@ -191,7 +191,7 @@ def get_facet_pagination(facet_field, user_params=params || {}, extra_controller
   # the Blacklight app-level request params that define the search.
   # @return [Blacklight::SolrDocument, nil] the found document or nil if not found
   def get_single_doc_via_search(index, request_params)
-    query = search_builder.with(request_params).start(index - 1).rows(1).query(fl: "*")
+    query = search_builder.with(request_params).start(index - 1).rows(1).merge(fl: "*")
     response = repository.search(query)
     response.documents.first
   end
@@ -202,7 +202,7 @@ def get_single_doc_via_search(index, request_params)
   def get_previous_and_next_documents_for_search(index, request_params, extra_controller_params={})
     p = previous_and_next_document_params(index)
 
-    query = search_builder.with(request_params).start(p.delete(:start)).rows(p.delete(:rows)).query(extra_controller_params.merge(p))
+    query = search_builder.with(request_params).start(p.delete(:start)).rows(p.delete(:rows)).merge(extra_controller_params).merge(p)
     response = repository.search(query)
 
     document_list = response.documents
@@ -223,7 +223,7 @@ def get_previous_and_next_documents_for_search(index, request_params, extra_cont
   def get_opensearch_response(field=nil, request_params = params || {}, extra_controller_params={})
     field ||= blacklight_config.view_config('opensearch').title_field
 
-    query = search_builder.with(request_params).query(solr_opensearch_params(field).merge(extra_controller_params))
+    query = search_builder.with(request_params).merge(solr_opensearch_params(field)).merge(extra_controller_params)
     response = repository.search(query)
 
     [response.params[:q], response.documents.flat_map {|doc| doc[field] }.uniq]
@@ -273,7 +273,8 @@ def fetch_many(ids=[], *args)
       query = search_builder.
                 with(user_params).
                 where(blacklight_config.document_model.unique_key => ids).
-                query(extra_controller_params.merge(fl: '*'))
+                merge(extra_controller_params).
+                merge(fl: '*')
       solr_response = repository.search(query)
 
       [solr_response, solr_response.documents]

spec/lib/blacklight/search_builder_spec.rb

@@ -47,10 +47,51 @@
     end
   end
 
-  describe "#query" do
+  describe "#to_hash" do
     it "should append the extra parameters to the result" do
-      actual = subject.query({a: 1})
-      expect(actual).to include a: 1
+      Deprecation.silence(Blacklight::SearchBuilder) do
+        actual = subject.to_hash({a: 1})
+        expect(actual).to include a: 1
+      end
+    end
+
+    it "should update if data is changed" do
+      subject.merge(q: 'xyz')
+      expect(subject.to_hash).to include q: 'xyz'
+      subject.merge(q: 'abc')
+      expect(subject.to_hash).to include q: 'abc'
+    end
+  end
+
+  describe "#merge" do
+    let(:processor_chain) { [:pass_through] }
+    before do
+      allow(subject).to receive(:pass_through) do |req_params|
+        req_params.replace subject.blacklight_params
+      end
+    end
+    it "should overwrite the processed parameters" do
+      actual = subject.with(q: 'abc').merge(q: 'xyz')
+      expect(actual[:q]).to eq 'xyz'
+    end
+  end
+
+  describe "#reverse_merge" do
+    let(:processor_chain) { [:pass_through] }
+    before do
+      allow(subject).to receive(:pass_through) do |req_params|
+        req_params.replace subject.blacklight_params
+      end
+    end
+
+    it "should provide default values for parameters" do
+      actual = subject.reverse_merge(a: 1)
+      expect(actual[:a]).to eq 1
+    end
+
+    it "should not overwrite the processed parameters" do
+      actual = subject.with(q: 'abc').reverse_merge(q: 'xyz')
+      expect(actual[:q]).to eq 'abc'
     end
   end
 
@@ -152,4 +193,51 @@
       expect(subject.with(search_field: 'x').send(:search_field)).to eq blacklight_config.search_fields['x']
     end
   end
+
+  describe "#params_changed?" do
+    it "should be false" do
+      expect(subject.send(:params_changed?)).to eq false
+    end
+
+    it "should be marked as changed when with() changes" do
+      subject.with(a: 1)
+      expect(subject.send(:params_changed?)).to eq true
+    end
+
+    it "should be marked as changed when where() changes" do
+      subject.where(a: 1)
+      expect(subject.send(:params_changed?)).to eq true
+    end
+
+    it "should be marked as changed when the processor chain changes" do
+      subject.append(:a)
+      expect(subject.send(:params_changed?)).to eq true
+    end
+
+    it "should be marked as changed when merged parameters are added" do
+      subject.merge(a: 1)
+      expect(subject.send(:params_changed?)).to eq true
+    end
+
+    it "should be marked as changed when reverse merged parameters are added" do
+      subject.merge(a: 1)
+      expect(subject.send(:params_changed?)).to eq true
+    end
+
+    it "should be marked as changed when pagination changes" do
+      subject.page(1)
+      expect(subject.send(:params_changed?)).to eq true
+    end
+
+    it "should be marked as changed when rows changes" do
+      subject.rows(1)
+      expect(subject.send(:params_changed?)).to eq true
+    end
+
+    it "should be marked as changed when start offset changes" do
+      subject.start(1)
+      expect(subject.send(:params_changed?)).to eq true
+    end
+
+  end
 end