Hyrax config option for solr http method (#5937)

Add configuration option solr_default_method for use in specifying a default http method across blacklight and activefedora

.dassie/app/controllers/catalog_controller.rb

@@ -25,7 +25,7 @@ def self.modified_field
 
     # Because too many times on Samvera tech people raise a problem regarding a failed query to SOLR.
     # Often, it's because they inadvertently exceeded the character limit of a GET request.
-    config.http_method = :post
+    config.http_method = Hyrax.config.solr_default_method
 
     ## Default parameters to send to solr for all search-like requests. See also SolrHelper#solr_search_params
     config.default_solr_params = {

.dassie/config/initializers/hyrax.rb

@@ -46,6 +46,10 @@
   # Set the system-wide virus scanner
   config.virus_scanner = Hyrax::VirusScanner
 
+  # The default method used for Solr queries. Values are :get or :post.
+  # Post is suggested to prevent issues with URL length.
+  config.solr_default_method = :post
+
   ##
   # To index to the Valkyrie core, uncomment the following lines.
   # config.query_index_from_valkyrie = true

.koppie/app/controllers/catalog_controller.rb

@@ -29,7 +29,7 @@ def self.modified_field
 
     # Because too many times on Samvera tech people raise a problem regarding a failed query to SOLR.
     # Often, it's because they inadvertently exceeded the character limit of a GET request.
-    config.http_method = :post
+    config.http_method = Hyrax.config.solr_default_method
 
     ## Default parameters to send to solr for all search-like requests. See also SolrHelper#solr_search_params
     config.default_solr_params = {

app/services/hyrax/solr_query_service.rb

@@ -26,21 +26,29 @@ def self.document_model
     end
 
     ##
+    # execute the query using a GET request
     # @return [Hash] the results returned from solr for the current query
     def get
       solr_service.get(build)
     end
 
+    ##
+    # execute the solr query and return results
+    # @return [Hash] the results returned from solr for the current query
+    def query_result
+      solr_service.query_result(build)
+    end
+
     ##
     # @return [Enumerable<SolrDocument>]
     def solr_documents
-      get['response']['docs'].map { |doc| self.class.document_model.new(doc) }
+      query_result['response']['docs'].map { |doc| self.class.document_model.new(doc) }
     end
 
     ##
     # @return [Array<String>] ids of documents matching the current query
     def get_ids # rubocop:disable Naming/AccessorMethodName
-      results = get
+      results = query_result
       results['response']['docs'].map { |doc| doc['id'] }
     end
 

app/services/hyrax/solr_service.rb

@@ -38,7 +38,7 @@ def select_path
       end
 
       delegate :add, :commit, :count, :delete, :get, :instance, :ping, :post,
-               :query, :delete_by_query, :search_by_id, :wipe!, to: :new
+               :query, :query_result, :delete_by_query, :search_by_id, :wipe!, to: :new
     end
 
     # Wraps rsolr get
@@ -72,21 +72,27 @@ def post(query = nil, **args)
       connection.post(solr_path, data: args)
     end
 
-    # Wraps get by default
+    # Query solr using the provided or default http method, returning the result as a hash.
+    # @return [Hash] raw query result from solr
+    def query_result(query, **args)
+      Hyrax.logger.warn rows_warning unless args.key?(:rows)
+      # Use the provided solr query method, or fall back to the configured default
+      method = args.delete(:method) || Hyrax.config.solr_default_method
+
+      case method
+      when :get
+        get(query, **args)
+      when :post
+        post(query, **args)
+      else
+        raise "Unsupported HTTP method for querying SolrService (#{method.inspect})"
+      end
+    end
+
+    # Execute the provided query. Uses the configured http method by default.
     # @return [Array<SolrHit>] the response docs wrapped in SolrHit objects
     def query(query, **args)
-      Hyrax.logger.warn rows_warning unless args.key?(:rows)
-      method = args.delete(:method) || :get
-
-      result = case method
-               when :get
-                 get(query, **args)
-               when :post
-                 post(query, **args)
-               else
-                 raise "Unsupported HTTP method for querying SolrService (#{method.inspect})"
-               end
-      result['response']['docs'].map do |doc|
+      query_result(query, **args)['response']['docs'].map do |doc|
         ::SolrHit.new(doc)
       end
     end
@@ -114,10 +120,10 @@ def add(solr_doc, commit: true)
     end
 
     # Wraps rsolr count
-    # @return [Hash] the hash straight form rsolr
+    # @return [Hash] the hash straight from rsolr
     def count(query)
       args = { rows: 0 }
-      get(query, **args)['response']['numFound'].to_i
+      query_result(query, **args)['response']['numFound'].to_i
     end
 
     # Wraps ActiveFedora::Base#search_by_id(id, opts)

app/services/hyrax/workflow/status_list_service.rb

@@ -69,7 +69,7 @@ def search_solr
         actionable_roles = roles_for_user
         Hyrax.logger.debug("Actionable roles for #{@user.user_key} are #{actionable_roles}")
         return [] if actionable_roles.empty?
-        WorkRelation.new.search_with_conditions(query(actionable_roles), method: :post)
+        WorkRelation.new.search_with_conditions(query(actionable_roles), method: Hyrax.config.solr_default_method)
       end
 
       def query(actionable_roles)

lib/generators/hyrax/templates/catalog_controller.rb

@@ -19,7 +19,7 @@ def self.modified_field
 
     # Because too many times on Samvera tech people raise a problem regarding a failed query to SOLR.
     # Often, it's because they inadvertently exceeded the character limit of a GET request.
-    config.http_method = :post
+    config.http_method = Hyrax.config.solr_default_method
 
     ## Default parameters to send to solr for all search-like requests. See also SolrHelper#solr_search_params
     config.default_solr_params = {

lib/hyrax/configuration.rb

@@ -846,6 +846,11 @@ def solr_select_path
       @solr_select_path ||= ActiveFedora.solr_config.fetch(:select_path, 'select')
     end
 
+    attr_writer :solr_default_method
+    def solr_default_method
+      @solr_default_method ||= :post
+    end
+
     attr_writer :identifier_registrars
     def identifier_registrars
       @identifier_registrars ||= {}

spec/services/hyrax/solr_service_spec.rb

@@ -91,11 +91,16 @@
       allow(described_class).to receive(:instance).and_return(double("instance", conn: mock_conn))
     end
 
-    it "defaults to HTTP GET method" do
-      expect(mock_conn).to receive(:get).with('select', params: { rows: 2, q: 'querytext', qt: 'standard' }).and_return(stub_result)
+    it "defaults to HTTP POST method" do
+      expect(mock_conn).to receive(:post).with('select', data: { rows: 2, q: 'querytext', qt: 'standard' }).and_return(stub_result)
       described_class.query('querytext', rows: 2)
     end
 
+    it "allows callers to specify HTTP GET method" do
+      expect(mock_conn).to receive(:get).with('select', params: { rows: 2, q: 'querytext', qt: 'standard' }).and_return(stub_result)
+      described_class.query('querytext', rows: 2, method: :get)
+    end
+
     it "allows callers to specify HTTP POST method" do
       expect(mock_conn).to receive(:post).with('select', data: { rows: 2, q: 'querytext', qt: 'standard' }).and_return(stub_result)
       described_class.query('querytext', rows: 2, method: :post)
@@ -109,14 +114,14 @@
     end
 
     it "wraps the solr response documents in Solr hits" do
-      expect(mock_conn).to receive(:get).with('select', params: { rows: 2, q: 'querytext', qt: 'standard' }).and_return(stub_result)
+      expect(mock_conn).to receive(:post).with('select', data: { rows: 2, q: 'querytext', qt: 'standard' }).and_return(stub_result)
       result = described_class.query('querytext', rows: 2)
       expect(result.size).to eq 1
       expect(result.first.id).to eq 'x'
     end
 
     it "warns about not passing rows" do
-      allow(mock_conn).to receive(:get).and_return(stub_result)
+      allow(mock_conn).to receive(:post).and_return(stub_result)
       expect(Hyrax.logger).to receive(:warn).with(/^Calling Hyrax::SolrService\.get without passing an explicit value for ':rows' is not recommended/)
       described_class.query('querytext')
     end
@@ -127,7 +132,7 @@
       let(:doc) { { 'id' => 'valkyrie-x' } }
 
       it "uses valkyrie solr based on config query_index_from_valkyrie" do
-        expect(mock_conn).to receive(:get).with('select', params: { q: 'querytext' }).and_return(stub_result)
+        expect(mock_conn).to receive(:post).with('select', data: { q: 'querytext' }).and_return(stub_result)
 
         result = service.query('querytext')
         expect(result.first.id).to eq 'valkyrie-x'
@@ -249,7 +254,7 @@
     let(:stub_result) { { 'response' => { 'numFound' => '2' } } }
 
     it "calls solr" do
-      expect(mock_conn).to receive(:get).with('select', params: { rows: 0, q: 'querytext', qt: 'standard' }).and_return(stub_result)
+      expect(mock_conn).to receive(:post).with('select', data: { rows: 0, q: 'querytext', qt: 'standard' }).and_return(stub_result)
       allow(described_class).to receive(:instance).and_return(double("instance", conn: mock_conn))
       expect(described_class.count('querytext')).to eq 2
     end
@@ -258,7 +263,7 @@
       subject(:service) { described_class.new(use_valkyrie: true) }
 
       it "uses valkyrie solr based on config query_index_from_valkyrie" do
-        expect(mock_conn).to receive(:get).with('select', params: { rows: 0, q: 'querytext' }).and_return(stub_result)
+        expect(mock_conn).to receive(:post).with('select', data: { rows: 0, q: 'querytext' }).and_return(stub_result)
         expect(service.count('querytext')).to eq 2
       end
     end