From ec59629757bdbb31223b97089c03f76de080b512 Mon Sep 17 00:00:00 2001 From: Chris Beer Date: Tue, 2 Dec 2014 08:56:23 -0800 Subject: [PATCH] Extract Blacklight::SolrRepository for mediating requests to solr --- lib/blacklight.rb | 1 + lib/blacklight/catalog.rb | 3 +- lib/blacklight/configuration.rb | 5 + lib/blacklight/request_builders.rb | 140 +++++++++- lib/blacklight/solr_helper.rb | 290 +++++--------------- lib/blacklight/solr_repository.rb | 69 +++++ spec/lib/blacklight/solr_helper_spec.rb | 144 +++------- spec/lib/blacklight/solr_repository_spec.rb | 113 ++++++++ 8 files changed, 442 insertions(+), 323 deletions(-) create mode 100644 lib/blacklight/solr_repository.rb create mode 100644 spec/lib/blacklight/solr_repository_spec.rb diff --git a/lib/blacklight.rb b/lib/blacklight.rb index d750ba3fff..bbfa9d0418 100644 --- a/lib/blacklight.rb +++ b/lib/blacklight.rb @@ -10,6 +10,7 @@ module Blacklight autoload :Solr, 'blacklight/solr' autoload :SolrHelper, 'blacklight/solr_helper' + autoload :SolrRepository, 'blacklight/solr_repository' autoload :RequestBuilders, 'blacklight/request_builders' autoload :Exceptions, 'blacklight/exceptions' diff --git a/lib/blacklight/catalog.rb b/lib/blacklight/catalog.rb index 89474ffe69..cdb8b6d334 100644 --- a/lib/blacklight/catalog.rb +++ b/lib/blacklight/catalog.rb @@ -11,6 +11,7 @@ module Blacklight::Catalog include Blacklight::Base include Blacklight::Catalog::ComponentConfiguration + include Blacklight::Facet SearchHistoryWindow = 100 # how many searches to save in session history @@ -46,7 +47,7 @@ def index # get single document from the solr index def show - @response, @document = get_solr_response_for_doc_id + @response, @document = get_solr_response_for_doc_id params[:id] respond_to do |format| format.html {setup_next_and_previous_documents} diff --git a/lib/blacklight/configuration.rb b/lib/blacklight/configuration.rb index 51e5767fa8..41f69846f6 100644 --- a/lib/blacklight/configuration.rb +++ b/lib/blacklight/configuration.rb @@ -32,6 +32,7 @@ def default_values :default_solr_params => {}, # the model to load solr response documents into; set below in #initialize_default_values :solr_document_model => nil, + :solr_response_model => nil, # The solr rqeuest handler to use when requesting only a single document :document_solr_request_handler => 'document', # THe path to send single document requests to solr @@ -142,6 +143,10 @@ def solr_document_model super || SolrDocument end + def solr_response_model + super || Blacklight::SolrResponse + end + ## # DSL helper def configure diff --git a/lib/blacklight/request_builders.rb b/lib/blacklight/request_builders.rb index 3a753d9f11..6961da22e9 100644 --- a/lib/blacklight/request_builders.rb +++ b/lib/blacklight/request_builders.rb @@ -6,6 +6,8 @@ module Blacklight # module RequestBuilders extend ActiveSupport::Concern + extend Deprecation + self.deprecation_horizon = 'blacklight 6.0' included do # We want to install a class-level place to keep @@ -21,7 +23,11 @@ module RequestBuilders # CatalogController.include ModuleDefiningNewMethod # CatalogController.solr_search_params_logic += [:new_method] # CatalogController.solr_search_params_logic.delete(:we_dont_want) - self.solr_search_params_logic = [:default_solr_parameters , :add_query_to_solr, :add_facet_fq_to_solr, :add_facetting_to_solr, :add_solr_fields_to_query, :add_paging_to_solr, :add_sorting_to_solr, :add_group_config_to_solr ] + self.solr_search_params_logic = [:default_solr_parameters, :add_query_to_solr, :add_facet_fq_to_solr, :add_facetting_to_solr, :add_solr_fields_to_query, :add_paging_to_solr, :add_sorting_to_solr, :add_group_config_to_solr ] + + if self.respond_to?(:helper_method) + helper_method(:facet_limit_for) + end end # @returns a params hash for searching solr. @@ -47,6 +53,86 @@ def solr_search_params(user_params = params || {}) end end + ## + # Retrieve the results for a list of document ids + def solr_document_ids_params(ids = []) + solr_documents_by_field_values_params blacklight_config.solr_document_model.unique_key, ids + end + + ## + # Retrieve the results for a list of document ids + # @deprecated + def solr_documents_by_field_values_params(field, values) + q = if Array(values).empty? + "{!lucene}NOT *:*" + else + "{!lucene}#{field}:(#{ Array(values).map { |x| solr_param_quote(x) }.join(" OR ")})" + end + + { q: q, spellcheck: 'false', fl: "*" } + end + + ## + # Retrieve a facet's paginated values. + def solr_facet_params(facet_field, user_params=params || {}, extra_controller_params={}) + input = user_params.deep_merge(extra_controller_params) + facet_config = blacklight_config.facet_fields[facet_field] + + solr_params = {} + + # Now override with our specific things for fetching facet values + solr_params[:"facet.field"] = with_ex_local_param((facet_config.ex if facet_config.respond_to?(:ex)), facet_field) + + limit = if respond_to?(:facet_list_limit) + facet_list_limit.to_s.to_i + elsif solr_params["facet.limit"] + solr_params["facet.limit"].to_i + else + 20 + end + + # Need to set as f.facet_field.facet.* to make sure we + # override any field-specific default in the solr request handler. + solr_params[:"f.#{facet_field}.facet.limit"] = limit + 1 + solr_params[:"f.#{facet_field}.facet.offset"] = ( input.fetch(Blacklight::Solr::FacetPaginator.request_keys[:page] , 1).to_i - 1 ) * ( limit ) + solr_params[:"f.#{facet_field}.facet.sort"] = input[ Blacklight::Solr::FacetPaginator.request_keys[:sort] ] if input[ Blacklight::Solr::FacetPaginator.request_keys[:sort] ] + solr_params[:rows] = 0 + + solr_params + end + + ## + # Opensearch autocomplete parameters for plucking a field's value from the results + def solr_opensearch_params(field=nil) + if field.nil? + Deprecation.warn(Blacklight::RequestBuilders, "Calling Blacklight::RequestBuilders#solr_opensearch_params without a field name is deprecated and will be required in Blacklight 6.0.") + end + + solr_params = {} + solr_params[:rows] ||= 10 + solr_params[:fl] = field || blacklight_config.view_config('opensearch').title_field + solr_params + end + + ## + # Pagination parameters for selecting the previous and next documents + # out of a result set. + def previous_and_next_document_params(index, window = 1) + solr_params = {} + + if index > 0 + solr_params[:start] = index - window # get one before + solr_params[:rows] = 2*window + 1 # and one after + else + solr_params[:start] = 0 # there is no previous doc + solr_params[:rows] = 2*window # but there should be one after + end + + solr_params[:fl] = '*' + solr_params[:facet] = false + solr_params + end + #### # Start with general defaults from BL config. Need to use custom # merge to dup values, to avoid later mutating the original by mistake. @@ -55,7 +141,7 @@ def default_solr_parameters(solr_parameters, user_params) solr_parameters[key] = value.dup rescue value end end - + ## # Take the user-entered query, and put it in the solr params, # including config's "search field" params for current search field. @@ -71,7 +157,7 @@ def add_query_to_solr(solr_parameters, user_parameters) # rspec'd. solr_parameters[:qt] = user_parameters[:qt] if user_parameters[:qt] - search_field_def = search_field_def_for_key(user_parameters[:search_field]) + search_field_def = blacklight_config.search_fields[user_parameters[:search_field]] if (search_field_def) solr_parameters[:qt] = search_field_def.qt solr_parameters.merge!( search_field_def.solr_parameters) if search_field_def.solr_parameters @@ -253,8 +339,55 @@ def with_ex_local_param(ex, value) end end + + DEFAULT_FACET_LIMIT = 10 + + # Look up facet limit for given facet_field. Will look at config, and + # if config is 'true' will look up from Solr @response if available. If + # no limit is avaialble, returns nil. Used from #solr_search_params + # to supply f.fieldname.facet.limit values in solr request (no @response + # available), and used in display (with @response available) to create + # a facet paginator with the right limit. + def facet_limit_for(facet_field) + facet = blacklight_config.facet_fields[facet_field] + return if facet.blank? + + if facet.limit and @response and @response.facet_by_field_name(facet_field) + limit = @response.facet_by_field_name(facet_field).limit + + if limit.nil? # we didn't get or a set a limit, so infer one. + facet.limit if facet.limit != true + elsif limit == -1 # limit -1 is solr-speak for unlimited + nil + else + limit.to_i - 1 # we added 1 to find out if we needed to paginate + end + elsif facet.limit + facet.limit == true ? DEFAULT_FACET_LIMIT : facet.limit + end + end + + ## + # A helper method used for generating solr LocalParams, put quotes + # around the term unless it's a bare-word. Escape internal quotes + # if needed. + def solr_param_quote(val, options = {}) + options[:quote] ||= '"' + unless val =~ /^[a-zA-Z0-9$_\-\^]+$/ + val = options[:quote] + + # Yes, we need crazy escaping here, to deal with regexp esc too! + val.gsub("'", "\\\\\'").gsub('"', "\\\\\"") + + options[:quote] + end + return val + end + private + def should_add_to_solr field_name, field + field.include_in_request || (field.include_in_request.nil? && blacklight_config.add_field_configuration_to_solr_request) + end + ## # Convert a facet/value pair into a solr fq parameter def facet_value_to_fq_string(facet_field, value) @@ -284,7 +417,6 @@ def facet_value_to_fq_string(facet_field, value) "{!raw f=#{facet_field}#{(" " + local_params.join(" ")) unless local_params.empty?}}#{value}" end - end end end diff --git a/lib/blacklight/solr_helper.rb b/lib/blacklight/solr_helper.rb index 282e928df7..d3605b3358 100644 --- a/lib/blacklight/solr_helper.rb +++ b/lib/blacklight/solr_helper.rb @@ -47,71 +47,42 @@ module Blacklight::SolrHelper extend ActiveSupport::Concern extend Deprecation - include Blacklight::SearchFields - include Blacklight::Facet - include ActiveSupport::Benchmarkable + self.deprecation_horizon = 'blacklight 6.0' - included do - if self.respond_to?(:helper_method) - helper_method(:facet_limit_for) - end - - include Blacklight::RequestBuilders - - # ActiveSupport::Benchmarkable requires a logger method - unless instance_methods.include? :logger - def logger - nil - end - end - end + include Blacklight::RequestBuilders ## # Execute a solr query - # @see [RSolr::Client#send_and_receive] - # @overload find(solr_path, params) - # Execute a solr query at the given path with the parameters - # @param [String] solr path (defaults to blacklight_config.solr_path) - # @param [Hash] parameters for RSolr::Client#send_and_receive - # @overload find(params) - # @param [Hash] parameters for RSolr::Client#send_and_receive + # @see [Blacklight::SolrRepository#send_and_receive] # @return [Blacklight::SolrResponse] the solr response object - def find(*args) - # In later versions of Rails, the #benchmark method can do timing - # better for us. - benchmark("Solr fetch", level: :debug) do - solr_params = args.extract_options! - path = args.first || blacklight_config.solr_path - solr_params[:qt] ||= blacklight_config.qt - # delete these parameters, otherwise rsolr will pass them through. - key = blacklight_config.http_method == :post ? :data : :params - res = blacklight_solr.send_and_receive(path, {key=>solr_params.to_hash, method:blacklight_config.http_method}) - - solr_response = Blacklight::SolrResponse.new(res, solr_params, solr_document_model: blacklight_config.solr_document_model) - - Rails.logger.debug("Solr query: #{solr_params.inspect}") - Rails.logger.debug("Solr response: #{solr_response.inspect}") if defined?(::BLACKLIGHT_VERBOSE_LOGGING) and ::BLACKLIGHT_VERBOSE_LOGGING - solr_response - end - rescue Errno::ECONNREFUSED => e - raise Blacklight::Exceptions::ECONNREFUSED.new("Unable to connect to Solr instance using #{blacklight_solr.inspect}") + def find *args + solr_params = args.extract_options! + path = args.first || blacklight_config.solr_path + + solr_params[:qt] ||= blacklight_config.qt + + solr_repository.send_and_receive path, solr_params end - - - # A helper method used for generating solr LocalParams, put quotes - # around the term unless it's a bare-word. Escape internal quotes - # if needed. - def solr_param_quote(val, options = {}) - options[:quote] ||= '"' - unless val =~ /^[a-zA-Z0-9$_\-\^]+$/ - val = options[:quote] + - # Yes, we need crazy escaping here, to deal with regexp esc too! - val.gsub("'", "\\\\\'").gsub('"', "\\\\\"") + - options[:quote] - end - return val + deprecation_deprecate :find + + # returns a params hash for finding a single solr document (CatalogController #show action) + def solr_doc_params(id=nil) + id ||= params[:id] + + # add our document id to the document_unique_id_param query parameter + p = blacklight_config.default_document_solr_params.merge({ + # this assumes the request handler will map the unique id param + # to the unique key field using either solr local params, the + # real-time get handler, etc. + blacklight_config.document_unique_id_param => id + }) + + p[:qt] ||= blacklight_config.document_solr_request_handler + + p end - + deprecation_deprecate :solr_doc_params + # a solr query method # given a user query, return a solr response containing both result docs and facets # - mixes in the Blacklight::Solr::SpellingSuggestions module @@ -138,111 +109,52 @@ def get_search_results(user_params = params || {}, extra_controller_params = {}) def query_solr(user_params = params || {}, extra_controller_params = {}) solr_params = self.solr_search_params(user_params).merge(extra_controller_params) - find(solr_params) + solr_repository.search(solr_params) end - - # returns a params hash for finding a single solr document (CatalogController #show action) - # If the id arg is nil, then the value is fetched from params[:id] - # This method is primary called by the get_solr_response_for_doc_id method. - def solr_doc_params(id=nil) - id ||= params[:id] - # add our document id to the document_unique_id_param query parameter - p = blacklight_config.default_document_solr_params.merge({ - # this assumes the request handler will map the unique id param - # to the unique key field using either solr local params, the - # real-time get handler, etc. - blacklight_config.document_unique_id_param => id - }) - - p[:qt] ||= blacklight_config.document_solr_request_handler - - p - end - # a solr query method # retrieve a solr document, given the doc id # @return [Blacklight::SolrResponse, Blacklight::SolrDocument] the solr response object and the first document def get_solr_response_for_doc_id(id=nil, extra_controller_params={}) - solr_params = solr_doc_params(id).merge(extra_controller_params) - solr_response = find(blacklight_config.document_solr_path, solr_params) - raise Blacklight::Exceptions::InvalidSolrID.new if solr_response.documents.empty? + solr_response = solr_repository.find id, extra_controller_params [solr_response, solr_response.documents.first] end - def get_solr_response_for_document_ids(ids=[], extra_solr_params = {}) - get_solr_response_for_field_values(blacklight_config.solr_document_model.unique_key, ids, extra_solr_params) - end - - # given a field name and array of values, get the matching SOLR documents - # @return [Blacklight::SolrResponse, Array] the solr response object and a list of solr documents - def get_solr_response_for_field_values(field, values, extra_solr_params = {}) - q = if Array(values).empty? - "NOT *:*" + ## + # Retrieve a set of documents by id + # @overload get_solr_response_for_document_ids(ids, extra_controller_params) + # @overload get_solr_response_for_document_ids(ids, user_params, extra_controller_params) + def get_solr_response_for_document_ids(ids=[], *args) + # user_params = params || {}, extra_controller_params = {} + if args.length == 1 + user_params = params + extra_controller_params = args.first || {} else - "#{field}:(#{ Array(values).map { |x| solr_param_quote(x)}.join(" OR ")})" + user_params, extra_controller_params = args + user_params ||= params + extra_controller_params ||= {} end - solr_params = { - :defType => "lucene", # need boolean for OR - :q => q, - # not sure why fl * is neccesary, why isn't default solr_search_params - # sufficient, like it is for any other search results solr request? - # But tests fail without this. I think because some functionality requires - # this to actually get solr_doc_params, not solr_search_params. Confused - # semantics again. - :fl => "*", - :facet => 'false', - :spellcheck => 'false' - }.merge(extra_solr_params) - - solr_response = find(self.solr_search_params().merge(solr_params) ) + solr_response = query_solr(user_params, extra_controller_params.merge(solr_document_ids_params(ids))) + [solr_response, solr_response.documents] end - - # returns a params hash for a single facet field solr query. - # used primary by the get_facet_pagination method. - # Looks up Facet Paginator request params from current request - # params to figure out sort and offset. - # Default limit for facet list can be specified by defining a controller - # method facet_list_limit, otherwise 20. - def solr_facet_params(facet_field, user_params=params || {}, extra_controller_params={}) - input = user_params.deep_merge(extra_controller_params) - facet_config = blacklight_config.facet_fields[facet_field] - - # First start with a standard solr search params calculations, - # for any search context in our request params. - solr_params = solr_search_params(user_params).merge(extra_controller_params) - - # Now override with our specific things for fetching facet values - solr_params[:"facet.field"] = with_ex_local_param((facet_config.ex if facet_config.respond_to?(:ex)), facet_field) - - limit = - if respond_to?(:facet_list_limit) - facet_list_limit.to_s.to_i - elsif solr_params["facet.limit"] - solr_params["facet.limit"].to_i - else - 20 - end - - # Need to set as f.facet_field.facet.* to make sure we - # override any field-specific default in the solr request handler. - solr_params[:"f.#{facet_field}.facet.limit"] = limit + 1 - solr_params[:"f.#{facet_field}.facet.offset"] = ( input.fetch(Blacklight::Solr::FacetPaginator.request_keys[:page] , 1).to_i - 1 ) * ( limit ) - solr_params[:"f.#{facet_field}.facet.sort"] = input[ Blacklight::Solr::FacetPaginator.request_keys[:sort] ] if input[ Blacklight::Solr::FacetPaginator.request_keys[:sort] ] - solr_params[:rows] = 0 - - return solr_params + + # given a field name and array of values, get the matching SOLR documents + # @return [Blacklight::SolrResponse, Array] the solr response object and a list of solr documents + def get_solr_response_for_field_values(field, values, extra_controller_params = {}) + solr_response = query_solr(params, extra_controller_params.merge(solr_documents_by_field_values_params(field, values))) + + [solr_response, solr_response.documents] end - + deprecation_deprecate :get_solr_response_for_field_values + ## # 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 = {}) solr_params = solr_facet_params(facet_field, user_params, extra_controller_params) - # Make the solr call - find(solr_params) + query_solr(user_params, extra_controller_params.merge(solr_facet_params(facet_field, user_params, extra_controller_params))) end # a solr query method @@ -257,28 +169,28 @@ def get_facet_pagination(facet_field, user_params=params || {}, extra_controller # Actually create the paginator! # NOTE: The sniffing of the proper sort from the solr response is not # currently tested for, tricky to figure out how to test, since the - # default setup we test against doesn't use this feature. - return Blacklight::Solr::FacetPaginator.new(response.facets.first.items, - :offset => response.params[:"f.#{facet_field}.facet.offset"], + # default setup we test against doesn't use this feature. + return Blacklight::Solr::FacetPaginator.new(response.facets.first.items, + :offset => response.params[:"f.#{facet_field}.facet.offset"], :limit => limit, :sort => response.params[:"f.#{facet_field}.facet.sort"] || response.params["facet.sort"] ) end deprecation_deprecate :get_facet_pagination - + # a solr query method - # this is used when selecting a search result: we have a query and a + # this is used when selecting a search result: we have a query and a # position in the search results and possibly some facets # Pass in an index where 1 is the first document in the list, and - # the Blacklight app-level request params that define the search. + # 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) solr_params = solr_search_params(request_params) - solr_params[:start] = (index - 1) # start at 0 to get 1st doc, 1 to get 2nd. + solr_params[:start] = (index - 1) # start at 0 to get 1st doc, 1 to get 2nd. solr_params[:rows] = 1 solr_params[:fl] = '*' - solr_response = find(solr_params) + solr_response = solr_repository.search(solr_params) solr_response.documents.first end deprecation_deprecate :get_single_doc_via_search @@ -287,19 +199,7 @@ def get_single_doc_via_search(index, request_params) # @return [Blacklight::SolrResponse, Array] the solr response and a list of the first and last document def get_previous_and_next_documents_for_search(index, request_params, extra_controller_params={}) - solr_params = solr_search_params(request_params).merge(extra_controller_params) - - if index > 0 - solr_params[:start] = index - 1 # get one before - solr_params[:rows] = 3 # and one after - else - solr_params[:start] = 0 # there is no previous doc - solr_params[:rows] = 2 # but there should be one after - end - - solr_params[:fl] = '*' - solr_params[:facet] = false - solr_response = find(solr_params) + solr_response = query_solr(request_params, extra_controller_params.merge(previous_and_next_document_params(index))) document_list = solr_response.documents @@ -309,16 +209,6 @@ def get_previous_and_next_documents_for_search(index, request_params, extra_cont [solr_response, [prev_doc, next_doc]] end - - # returns a solr params hash - # the :fl (solr param) is set to the "field" value. - # per_page is set to 10 - def solr_opensearch_params(field=nil) - solr_params = solr_search_params - solr_params[:per_page] = 10 - solr_params[:fl] = field || blacklight_config.view_config('opensearch').title_field - solr_params - end # a solr query method # does a standard search but returns a simplified object. @@ -326,38 +216,12 @@ def solr_opensearch_params(field=nil) # the second item is an other array. This second array contains # all of the field values for each of the documents... # where the field is the "field" argument passed in. - def get_opensearch_response(field=nil, extra_controller_params={}) - solr_params = solr_opensearch_params().merge(extra_controller_params) - response = find(solr_params) - a = [solr_params[:q]] - a << response.documents.map {|doc| doc[solr_params[:fl]].to_s } - end - - DEFAULT_FACET_LIMIT = 10 - - # Look up facet limit for given facet_field. Will look at config, and - # if config is 'true' will look up from Solr @response if available. If - # no limit is avaialble, returns nil. Used from #solr_search_params - # to supply f.fieldname.facet.limit values in solr request (no @response - # available), and used in display (with @response available) to create - # a facet paginator with the right limit. - def facet_limit_for(facet_field) - facet = blacklight_config.facet_fields[facet_field] - return if facet.blank? - - if facet.limit and @response and @response.facet_by_field_name(facet_field) - limit = @response.facet_by_field_name(facet_field).limit - - if limit.nil? # we didn't get or a set a limit, so infer one. - facet.limit if facet.limit != true - elsif limit == -1 # limit -1 is solr-speak for unlimited - nil - else - limit.to_i - 1 # we added 1 to find out if we needed to paginate - end - elsif facet.limit - facet.limit == true ? DEFAULT_FACET_LIMIT : facet.limit - end + def get_opensearch_response(field=nil, request_params = params || {}, extra_controller_params={}) + field ||= blacklight_config.view_config('opensearch').title_field + + response = query_solr(request_params, solr_opensearch_params(field).merge(extra_controller_params)) + + [response.params[:q], response.documents.flat_map {|doc| doc[field] }.uniq] end ## @@ -366,18 +230,8 @@ def grouped_key_for_results blacklight_config.index.group end - def blacklight_solr - @solr ||= RSolr.connect(blacklight_solr_config) - end - - def blacklight_solr_config - Blacklight.solr_config - end - - private - - def should_add_to_solr field_name, field - field.include_in_request || (field.include_in_request.nil? && blacklight_config.add_field_configuration_to_solr_request) + def solr_repository + @solr_repository ||= Blacklight::SolrRepository.new(blacklight_config) end end diff --git a/lib/blacklight/solr_repository.rb b/lib/blacklight/solr_repository.rb new file mode 100644 index 0000000000..8a5d2b3247 --- /dev/null +++ b/lib/blacklight/solr_repository.rb @@ -0,0 +1,69 @@ +module Blacklight + class SolrRepository + attr_accessor :blacklight_config, :blacklight_solr + + # ActiveSupport::Benchmarkable requires a logger method + attr_accessor :logger + + include ActiveSupport::Benchmarkable + + def initialize blacklight_config + @blacklight_config = blacklight_config + end + + ## + # Find a single solr document result (by id) using the document configuration + # @param [String] document's unique key value + # @param [Hash] additional solr query parameters + def find id, params = {} + solr_response = send_and_receive blacklight_config.document_solr_path || blacklight_config.solr_path, {qt: blacklight_config.document_solr_request_handler}.merge(blacklight_config.default_document_solr_params.merge(params).merge(blacklight_config.document_unique_id_param => id)) + raise Blacklight::Exceptions::InvalidSolrID.new if solr_response.documents.empty? + solr_response + end + + ## + # Execute a search query against solr + # @param [Hash] solr query parameters + def search params = {} + send_and_receive blacklight_config.solr_path, { qt: blacklight_config.qt }.merge(params) + end + + ## + # Execute a solr query + # @see [RSolr::Client#send_and_receive] + # @overload find(solr_path, params) + # Execute a solr query at the given path with the parameters + # @param [String] solr path (defaults to blacklight_config.solr_path) + # @param [Hash] parameters for RSolr::Client#send_and_receive + # @overload find(params) + # @param [Hash] parameters for RSolr::Client#send_and_receive + # @return [Blacklight::SolrResponse] the solr response object + def send_and_receive(path, solr_params = {}) + benchmark("Solr fetch", level: :debug) do + key = blacklight_config.http_method == :post ? :data : :params + res = blacklight_solr.send_and_receive(path, {key=>solr_params.to_hash, method:blacklight_config.http_method}) + + solr_response = blacklight_config.solr_response_model.new(res, solr_params, solr_document_model: blacklight_config.solr_document_model) + + Rails.logger.debug("Solr query: #{solr_params.inspect}") + Rails.logger.debug("Solr response: #{solr_response.inspect}") if defined?(::BLACKLIGHT_VERBOSE_LOGGING) and ::BLACKLIGHT_VERBOSE_LOGGING + solr_response + end + rescue Errno::ECONNREFUSED => e + raise Blacklight::Exceptions::ECONNREFUSED.new("Unable to connect to Solr instance using #{blacklight_solr.inspect}") + end + + def blacklight_solr + @blacklight_solr ||= RSolr.connect(blacklight_solr_config) + end + + protected + def blacklight_solr_config + @blacklight_solr_config ||= Blacklight.solr_config + end + + def logger + @logger ||= Rails.logger if defined? Rails + end + end +end \ No newline at end of file diff --git a/spec/lib/blacklight/solr_helper_spec.rb b/spec/lib/blacklight/solr_helper_spec.rb index df8adde323..beb7720528 100644 --- a/spec/lib/blacklight/solr_helper_spec.rb +++ b/spec/lib/blacklight/solr_helper_spec.rb @@ -18,11 +18,12 @@ class SolrHelperTestClass include Blacklight::SolrHelper attr_accessor :blacklight_config - attr_accessor :blacklight_solr + attr_accessor :solr_repository def initialize blacklight_config, blacklight_solr self.blacklight_config = blacklight_config - self.blacklight_solr = blacklight_solr + self.solr_repository = Blacklight::SolrRepository.new(blacklight_config) + self.solr_repository.blacklight_solr = blacklight_solr end def params @@ -48,70 +49,6 @@ def params @subject_search_params = {:commit=>"search", :search_field=>"subject", :action=>"index", :"controller"=>"catalog", :"rows"=>"10", :"q"=>"wome"} end - describe "#find" do - it "should use the configured solr path" do - blacklight_config.solr_path = 'xyz' - allow(blacklight_solr).to receive(:send_and_receive).with('xyz', anything).and_return("{}".to_json) - expect(subject.find({})).to be_a_kind_of Blacklight::SolrResponse - end - - it "should override the configured solr path" do - blacklight_config.solr_path = 'xyz' - allow(blacklight_solr).to receive(:send_and_receive).with('abc', anything).and_return("{}".to_json) - expect(subject.find('abc', {})).to be_a_kind_of Blacklight::SolrResponse - end - - it "should use a default :qt param" do - blacklight_config.qt = 'xyz' - allow(blacklight_solr).to receive(:send_and_receive).with('select', hash_including(params: { qt: 'xyz'})).and_return("{}".to_json) - expect(subject.find({})).to be_a_kind_of Blacklight::SolrResponse - end - - it "should use the provided :qt param" do - blacklight_config.qt = 'xyz' - allow(blacklight_solr).to receive(:send_and_receive).with('select', hash_including(params: { qt: 'abc'})).and_return("{}".to_json) - expect(subject.find({qt: 'abc'})).to be_a_kind_of Blacklight::SolrResponse - end - - describe "http_method configuration" do - describe "using default" do - - it "defaults to get" do - expect(blacklight_config.http_method).to eq :get - allow(blacklight_solr).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.find(:q => @all_docs_query) - end - end - - describe "setting to post" do - let (:blacklight_config) {config = Blacklight::Configuration.new; config.http_method=:post; config} - - it "keep value set to post" do - expect(blacklight_config.http_method).to eq :post - allow(blacklight_solr).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.find(:q => @all_docs_query) - end - end - end - end - - describe "http_method configuration", :integration => true do - let (:blacklight_config) {config = Blacklight::Configuration.new; config.http_method=:post; config} - - it "should send a post request to solr and get a response back" do - response = subject.find(:q => @all_docs_query) - expect(response.docs.length).to be >= 1 - end - end - # SPECS for actual search parameter generation describe "solr_search_params" do it "allows customization of solr_search_params_logic" do @@ -684,28 +621,19 @@ def params expect(solr_params[:"f.#{@facet_field}.facet.sort"]).to be_blank end - it "uses the field-specific sort" do - solr_params = subject.solr_facet_params('format_ordered') - expect(solr_params[:"f.format_ordered.facet.sort"]).to eq :count - end - it 'uses sort provided in the parameters' do solr_params = subject.solr_facet_params(@facet_field, @sort_key => "index") expect(solr_params[:"f.#{@facet_field}.facet.sort"]).to eq 'index' end + it "comes up with the same params as #solr_search_params to constrain context for facet list" do search_params = {:q => 'tibetan history', :f=> {:format=>'Book', :language_facet=>'Tibetan'}} - solr_search_params = subject.solr_search_params( search_params ) solr_facet_params = subject.solr_facet_params('format', search_params) - solr_search_params.each_pair do |key, value| - # The specific params used for fetching the facet list we - # don't care about. - next if ['facets', "facet.field", 'rows', 'facet.limit', 'facet.offset', 'facet.sort'].include?(key) - # Everything else should match - expect(solr_facet_params[key]).to eq value - end - + expect(solr_facet_params).to include :"facet.field" => "format" + expect(solr_facet_params).to include :"f.format.facet.limit" => 21 + expect(solr_facet_params).to include :"f.format.facet.offset" => 0 + expect(solr_facet_params).to include :"rows" => 0 end end describe "for facet limit parameters config ed" do @@ -1075,27 +1003,35 @@ def params describe "solr_doc_params" do it "should default to using the 'document' requestHandler" do - doc_params = subject.solr_doc_params('asdfg') - expect(doc_params[:qt]).to eq 'document' + Deprecation.silence(Blacklight::SolrHelper) do + doc_params = subject.solr_doc_params('asdfg') + expect(doc_params[:qt]).to eq 'document' + end end it "should default to using the id parameter when sending solr queries" do - doc_params = subject.solr_doc_params('asdfg') - expect(doc_params[:id]).to eq 'asdfg' + Deprecation.silence(Blacklight::SolrHelper) do + doc_params = subject.solr_doc_params('asdfg') + expect(doc_params[:id]).to eq 'asdfg' + end end it "should use the document_unique_id_param configuration" do - allow(blacklight_config).to receive_messages(document_unique_id_param: :ids) - doc_params = subject.solr_doc_params('asdfg') - expect(doc_params[:ids]).to eq 'asdfg' + Deprecation.silence(Blacklight::SolrHelper) do + allow(blacklight_config).to receive_messages(document_unique_id_param: :ids) + doc_params = subject.solr_doc_params('asdfg') + expect(doc_params[:ids]).to eq 'asdfg' + end end describe "blacklight config's default_document_solr_parameters" do it "should use parameters from the controller's default_document_solr_parameters" do - blacklight_config.default_document_solr_params = { :qt => 'my_custom_handler', :asdf => '1234' } - doc_params = subject.solr_doc_params('asdfg') - expect(doc_params[:qt]).to eq 'my_custom_handler' - expect(doc_params[:asdf]).to eq '1234' + Deprecation.silence(Blacklight::SolrHelper) do + blacklight_config.default_document_solr_params = { :qt => 'my_custom_handler', :asdf => '1234' } + doc_params = subject.solr_doc_params('asdfg') + expect(doc_params[:qt]).to eq 'my_custom_handler' + expect(doc_params[:asdf]).to eq '1234' + end end end @@ -1112,8 +1048,10 @@ def params end =end it "should respect the configuration-supplied unique id" do - doc_params = subject.solr_doc_params('"Strong Medicine speaks"') - expect(doc_params[:id]).to eq '"Strong Medicine speaks"' + Deprecation.silence(Blacklight::SolrHelper) do + doc_params = subject.solr_doc_params('"Strong Medicine speaks"') + expect(doc_params[:id]).to eq '"Strong Medicine speaks"' + end end end @@ -1273,18 +1211,24 @@ def params allow(@mock_response).to receive_messages(documents: []) end it "should contruct a solr query based on the field and value pair" do - allow(subject).to receive(:find).with(hash_including(:q => "field_name:(value)")).and_return(@mock_response) - subject.get_solr_response_for_field_values('field_name', 'value') + Deprecation.silence(Blacklight::SolrHelper) do + allow(subject.solr_repository).to receive(:send_and_receive).with('select', hash_including("q" => "{!lucene}field_name:(value)")).and_return(@mock_response) + subject.get_solr_response_for_field_values('field_name', 'value') + end end it "should OR multiple values together" do - allow(subject).to receive(:find).with(hash_including(:q => "field_name:(a OR b)")).and_return(@mock_response) - subject.get_solr_response_for_field_values('field_name', ['a', 'b']) + Deprecation.silence(Blacklight::SolrHelper) do + allow(subject.solr_repository).to receive(:send_and_receive).with('select', hash_including("q" => "{!lucene}field_name:(a OR b)")).and_return(@mock_response) + subject.get_solr_response_for_field_values('field_name', ['a', 'b']) + end end it "should escape crazy identifiers" do - allow(subject).to receive(:find).with(hash_including(:q => "field_name:(\"h://\\\"\\\'\")")).and_return(@mock_response) - subject.get_solr_response_for_field_values('field_name', 'h://"\'') + Deprecation.silence(Blacklight::SolrHelper) do + allow(subject.solr_repository).to receive(:send_and_receive).with('select', hash_including("q" => "{!lucene}field_name:(\"h://\\\"\\\'\")")).and_return(@mock_response) + subject.get_solr_response_for_field_values('field_name', 'h://"\'') + end end end @@ -1297,7 +1241,7 @@ def params # nearby on shelf it "should raise a Blacklight exception if RSolr can't connect to the Solr instance" do allow(blacklight_solr).to receive(:send_and_receive).and_raise(Errno::ECONNREFUSED) - expect { subject.find(:a => 123) }.to raise_exception(/Unable to connect to Solr instance/) + expect { subject.query_solr }.to raise_exception(/Unable to connect to Solr instance/) end describe "grouped_key_for_results" do diff --git a/spec/lib/blacklight/solr_repository_spec.rb b/spec/lib/blacklight/solr_repository_spec.rb new file mode 100644 index 0000000000..bb335a6f84 --- /dev/null +++ b/spec/lib/blacklight/solr_repository_spec.rb @@ -0,0 +1,113 @@ +require 'spec_helper' + +describe Blacklight::SolrRepository do + + let :blacklight_config do + CatalogController.blacklight_config.deep_copy + end + + subject do + Blacklight::SolrRepository.new blacklight_config + end + + let :mock_response do + { response: { docs: [document]}} + end + + let :document do + {} + end + + describe "#find" do + it "should use the document-specific solr path" do + blacklight_config.document_solr_path = 'abc' + blacklight_config.solr_path = 'xyz' + allow(subject.blacklight_solr).to receive(:send_and_receive).with('abc', anything).and_return(mock_response) + expect(subject.find("123")).to be_a_kind_of Blacklight::SolrResponse + end + + it "should use the default solr path" do + blacklight_config.solr_path = 'xyz' + allow(subject.blacklight_solr).to receive(:send_and_receive).with('xyz', anything).and_return(mock_response) + expect(subject.find("123")).to be_a_kind_of Blacklight::SolrResponse + end + + it "should use a default :qt param" do + allow(subject.blacklight_solr).to receive(:send_and_receive).with('select', hash_including(params: { id: '123', qt: 'document'})).and_return(mock_response) + expect(subject.find("123", {})).to be_a_kind_of Blacklight::SolrResponse + end + + it "should use the provided :qt param" do + blacklight_config.document_solr_request_handler = 'xyz' + allow(subject.blacklight_solr).to receive(:send_and_receive).with('select', hash_including(params: { id: '123', qt: 'abc'})).and_return(mock_response) + expect(subject.find("123", {qt: 'abc'})).to be_a_kind_of Blacklight::SolrResponse + end + end + + describe "#search" do + it "should use the search-specific solr path" do + blacklight_config.solr_path = 'xyz' + allow(subject.blacklight_solr).to receive(:send_and_receive).with('xyz', anything).and_return(mock_response) + expect(subject.search({})).to be_a_kind_of Blacklight::SolrResponse + end + + it "should use the default solr path" do + allow(subject.blacklight_solr).to receive(:send_and_receive).with('select', anything).and_return(mock_response) + expect(subject.search({})).to be_a_kind_of Blacklight::SolrResponse + end + + it "should use a default :qt param" do + blacklight_config.qt = 'xyz' + allow(subject.blacklight_solr).to receive(:send_and_receive).with('select', hash_including(params: { qt: 'xyz'})).and_return(mock_response) + expect(subject.search({})).to be_a_kind_of Blacklight::SolrResponse + end + + it "should use the provided :qt param" do + blacklight_config.qt = 'xyz' + allow(subject.blacklight_solr).to receive(:send_and_receive).with('select', hash_including(params: { qt: 'abc'})).and_return(mock_response) + expect(subject.search({qt: 'abc'})).to be_a_kind_of Blacklight::SolrResponse + end + end + + describe "#send_and_receive" do + describe "http_method configuration" do + describe "using default" do + + it "defaults to get" do + expect(blacklight_config.http_method).to eq :get + allow(subject.blacklight_solr).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) + end + end + + describe "setting to post" do + let (:blacklight_config) {config = Blacklight::Configuration.new; config.http_method=:post; config} + + it "keep value set to post" do + expect(blacklight_config.http_method).to eq :post + allow(subject.blacklight_solr).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) + end + end + end + end + + describe "http_method configuration", integration: true do + let (:blacklight_config) {config = Blacklight::Configuration.new; config.http_method=:post; config} + + it "should send a post request to solr and get a response back" do + response = subject.search(:q => @all_docs_query) + expect(response.docs.length).to be >= 1 + end + end + + +end \ No newline at end of file