diff --git a/lib/blacklight/exceptions.rb b/lib/blacklight/exceptions.rb index 1ef0d8748a..dadfec3392 100644 --- a/lib/blacklight/exceptions.rb +++ b/lib/blacklight/exceptions.rb @@ -16,6 +16,8 @@ class ExpiredSessionToken < StandardError class ECONNREFUSED < ::Errno::ECONNREFUSED; end + class RepositoryTimeout < Timeout::Error; end + class IconNotFound < StandardError end end diff --git a/lib/blacklight/solr/repository.rb b/lib/blacklight/solr/repository.rb index af4f4b11bb..0da8db1b99 100644 --- a/lib/blacklight/solr/repository.rb +++ b/lib/blacklight/solr/repository.rb @@ -59,30 +59,40 @@ def ping # @return [Blacklight::Solr::Response] the solr response object def send_and_receive(path, solr_params = {}) benchmark("Solr fetch", level: :debug) do - res = if solr_params[:json].present? - connection.send_and_receive( - path, - data: { params: solr_params.to_hash.except(:json) }.merge(solr_params[:json]).to_json, - method: :post, - headers: { 'Content-Type' => 'application/json' } - ) - else - key = blacklight_config.http_method == :post ? :data : :params - connection.send_and_receive(path, { key => solr_params.to_hash, method: blacklight_config.http_method }) - end - + res = connection.send_and_receive(path, build_solr_request(solr_params)) solr_response = blacklight_config.response_model.new(res, solr_params, document_model: blacklight_config.document_model, blacklight_config: blacklight_config) Blacklight.logger&.debug("Solr query: #{blacklight_config.http_method} #{path} #{solr_params.to_hash.inspect}") Blacklight.logger&.debug("Solr response: #{solr_response.inspect}") if defined?(::BLACKLIGHT_VERBOSE_LOGGING) && ::BLACKLIGHT_VERBOSE_LOGGING solr_response end + rescue *defined_rsolr_timeout_exceptions => e + raise Blacklight::Exceptions::RepositoryTimeout, "Timeout connecting to Solr instance using #{connection.inspect}: #{e.inspect}" rescue Errno::ECONNREFUSED => e + # intended for and likely to be a RSolr::Error:ConnectionRefused, specifically. raise Blacklight::Exceptions::ECONNREFUSED, "Unable to connect to Solr instance using #{connection.inspect}: #{e.inspect}" rescue RSolr::Error::Http => e raise Blacklight::Exceptions::InvalidRequest, e.message end + # @return [Hash] + # @!visibility private + def build_solr_request(solr_params) + if solr_params[:json].present? + { + data: { params: solr_params.to_hash.except(:json) }.merge(solr_params[:json]).to_json, + method: :post, + headers: { 'Content-Type' => 'application/json' } + } + else + key = blacklight_config.http_method == :post ? :data : :params + { + key => solr_params.to_hash, + method: blacklight_config.http_method + } + end + end + private ## @@ -98,5 +108,19 @@ def suggester_name def build_connection RSolr.connect(connection_config.merge(adapter: connection_config[:http_adapter])) end + + # RSolr 2.4.0+ has a RSolr::Error::Timeout that we'd like to treat specially + # instead of lumping into RSolr::Error::Http. Before that we can not rescue + # specially, so return an empty array. + # + # @return [Array] that can be used, with a splat, as argument + # to a ruby rescue + def defined_rsolr_timeout_exceptions + if defined?(RSolr::Error::Timeout) + [RSolr::Error::Timeout] + else + [] + end + end end end diff --git a/spec/models/blacklight/solr/repository_spec.rb b/spec/models/blacklight/solr/repository_spec.rb index 5cc7a91e12..751eea5bbc 100644 --- a/spec/models/blacklight/solr/repository_spec.rb +++ b/spec/models/blacklight/solr/repository_spec.rb @@ -9,6 +9,8 @@ CatalogController.blacklight_config.deep_copy end + let(:all_docs_query) { '' } + let :mock_response do { response: { docs: [document] } } end @@ -104,19 +106,30 @@ expect(response).to be_a_kind_of Blacklight::Solr::Response expect(response.params).to be_a_kind_of ActiveSupport::HashWithIndifferentAccess end + + it "calls send_and_receive with params returned from request factory method" do + expect(blacklight_config.http_method).to eq :get + input_params = { q: all_docs_query } + allow(subject.connection).to receive(:send_and_receive) do |path, params| + expect(path).to eq 'select' + expect(params[:method]).to eq :get + expect(params[:params]).to include input_params + end.and_return('response' => { 'docs' => [] }) + subject.search(input_params) + end end - describe "#send_and_receive" do + describe "#build_solr_request" do + let(:input_params) { { q: all_docs_query } } + let(:actual_params) { subject.build_solr_request(input_params) } + describe "http_method configuration" do describe "using default" do it "defaults to get" do expect(blacklight_config.http_method).to eq :get - allow(subject.connection).to receive(:send_and_receive) do |path, params| - expect(path).to eq 'select' - expect(params[:method]).to eq :get - expect(params[:params]).to include(:q) - end.and_return('response' => { 'docs' => [] }) - subject.search(q: @all_docs_query) + expect(actual_params[:method]).to eq :get + expect(actual_params[:params]).to include input_params + expect(actual_params).not_to have_key :data end end @@ -125,25 +138,20 @@ it "keep value set to post" do expect(blacklight_config.http_method).to eq :post - allow(subject.connection).to receive(:send_and_receive) do |path, params| - expect(path).to eq 'select' - expect(params[:method]).to eq :post - expect(params[:data]).to include(:q) - end.and_return('response' => { 'docs' => [] }) - subject.search(q: @all_docs_query) + expect(actual_params[:method]).to eq :post + expect(actual_params[:data]).to include input_params + expect(actual_params).not_to have_key :params end end end context 'with json parameters' do + let(:input_params) { { json: { query: { bool: {} } } } } + it 'sends a post request with some json' do - allow(subject.connection).to receive(:send_and_receive) do |path, params| - expect(path).to eq 'select' - expect(params[:method]).to eq :post - expect(JSON.parse(params[:data]).with_indifferent_access).to include(query: { bool: {} }) - expect(params[:headers]).to include({ 'Content-Type' => 'application/json' }) - end.and_return('response' => { 'docs' => [] }) - subject.search(json: { query: { bool: {} } }) + expect(actual_params[:method]).to eq :post + expect(JSON.parse(actual_params[:data]).with_indifferent_access).to include(query: { bool: {} }) + expect(actual_params[:headers]).to include({ 'Content-Type' => 'application/json' }) end end end @@ -152,7 +160,7 @@ let (:blacklight_config) { config = Blacklight::Configuration.new; config.http_method = :post; config } it "sends a post request to solr and get a response back" do - response = subject.search(q: @all_docs_query) + response = subject.search(q: all_docs_query) expect(response.docs.length).to be >= 1 end end diff --git a/spec/services/blacklight/search_service_spec.rb b/spec/services/blacklight/search_service_spec.rb index 2faebe0215..bdbb16209d 100644 --- a/spec/services/blacklight/search_service_spec.rb +++ b/spec/services/blacklight/search_service_spec.rb @@ -384,6 +384,14 @@ expect { service.repository.search }.to raise_exception(/Unable to connect to Solr instance/) end + it "raises a Blacklight exception if RSolr raises a timeout error connecting to Solr instance" do + rsolr_timeout = RSolr::Error::Timeout.new(nil, nil) + allow(rsolr_timeout).to receive(:to_s).and_return("mocked RSolr timeout") + + allow(blacklight_solr).to receive(:send_and_receive).and_raise(rsolr_timeout) + expect { service.repository.search }.to raise_exception(Blacklight::Exceptions::RepositoryTimeout, /Timeout connecting to Solr instance/) + end + describe "#previous_and_next_documents_for_search" do let(:user_params) { { q: '', per_page: 100 } }