From ac1d4bedb91e758272fb26a1ba844eba4b04ab91 Mon Sep 17 00:00:00 2001 From: Chris Beer Date: Tue, 17 Feb 2015 10:19:18 -0800 Subject: [PATCH] Extract Blacklight::Solr::SearchBuilder with processing chain for converting blacklight parameters to solr request parameters --- lib/blacklight/configuration.rb | 7 +- lib/blacklight/request_builders.rb | 287 +-------- lib/blacklight/search_builder.rb | 69 ++- lib/blacklight/search_helper.rb | 22 +- lib/blacklight/solr.rb | 1 + lib/blacklight/solr/search_builder.rb | 273 +++++++++ spec/lib/blacklight/search_builder_spec.rb | 336 ----------- spec/lib/blacklight/search_helper_spec.rb | 234 +------- .../blacklight/solr/search_builder_spec.rb | 558 ++++++++++++++++++ 9 files changed, 929 insertions(+), 858 deletions(-) create mode 100644 lib/blacklight/solr/search_builder.rb create mode 100644 spec/lib/blacklight/solr/search_builder_spec.rb diff --git a/lib/blacklight/configuration.rb b/lib/blacklight/configuration.rb index fcc910ead2..7205275693 100644 --- a/lib/blacklight/configuration.rb +++ b/lib/blacklight/configuration.rb @@ -133,7 +133,8 @@ def default_values default_per_page: nil, # how many searches to save in session history # (TODO: move the value into the configuration?) - search_history_window: Blacklight::Catalog::SearchHistoryWindow + search_history_window: Blacklight::Catalog::SearchHistoryWindow, + default_facet_limit: 10 } end end @@ -204,6 +205,10 @@ def response_model= *args deprecation_deprecate :solr_response_model deprecation_deprecate :solr_response_model= + def default_per_page + super || per_page.first + end + ## # DSL helper def configure diff --git a/lib/blacklight/request_builders.rb b/lib/blacklight/request_builders.rb index 2f0d2b5e33..c80b5818b2 100644 --- a/lib/blacklight/request_builders.rb +++ b/lib/blacklight/request_builders.rb @@ -51,6 +51,14 @@ def solr_search_params_logic= logic deprecation_deprecate :solr_search_params_logic= end + def search_builder_class + Blacklight::Solr::SearchBuilder + end + + def search_builder processor_chain = search_params_logic + search_builder_class.new(processor_chain, self) + end + # @returns a params hash for searching solr. # The CatalogController #index action uses this. @@ -69,8 +77,7 @@ def solr_search_params_logic= logic # Incoming parameter :f is mapped to :fq solr parameter. def solr_search_params(user_params = params || {}, processor_chain = search_params_logic) Deprecation.warn(RequestBuilders, "solr_search_params is deprecated and will be removed in blacklight-6.0. Use SearchBuilder#processed_parameters instead.") - - Blacklight::SearchBuilder.new(user_params, processor_chain, self).processed_parameters + search_builder(processor_chain).with(user_params).processed_parameters end ## @@ -80,27 +87,24 @@ def solr_search_params(user_params = params || {}, processor_chain = search_para # added to the query post processing def build_solr_query(user_params, processor_chain, extra_params=nil) Deprecation.warn(RequestBuilders, "build_solr_query is deprecated and will be removed in blacklight-6.0. Use SearchBuilder#query instead") - Blacklight::SearchBuilder.new(user_params, processor_chain, self).query(extra_params) + search_builder(processor_chain).with(user_params).query(extra_params) end ## # Retrieve the results for a list of document ids def solr_document_ids_params(ids = []) - solr_documents_by_field_values_params blacklight_config.document_model.unique_key, ids + Deprecation.silence(Blacklight::RequestBuilders) do + solr_documents_by_field_values_params blacklight_config.document_model.unique_key, ids + end 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: "*" } + search_builder([:add_query_to_solr]).with(q: { field => values}).query(fl: '*') end + deprecation_deprecate :solr_documents_by_field_values_params ## # Retrieve a facet's paginated values. @@ -111,7 +115,7 @@ def solr_facet_params(facet_field, user_params=params || {}, extra_controller_pa 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) + solr_params[:"facet.field"] = search_builder.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 @@ -162,214 +166,7 @@ def previous_and_next_document_params(index, window = 1) 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. - def default_solr_parameters(solr_parameters, user_params) - blacklight_config.default_solr_params.each do |key, value| - 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. - # also include setting spellcheck.q. - def add_query_to_solr(solr_parameters, user_parameters) - ### - # Merge in search field configured values, if present, over-writing general - # defaults - ### - # legacy behavior of user param :qt is passed through, but over-ridden - # by actual search field config if present. We might want to remove - # this legacy behavior at some point. It does not seem to be currently - # rspec'd. - solr_parameters[:qt] = user_parameters[:qt] if user_parameters[:qt] - - 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 - end - - ## - # Create Solr 'q' including the user-entered q, prefixed by any - # solr LocalParams in config, using solr LocalParams syntax. - # http://wiki.apache.org/solr/LocalParams - ## - if (search_field_def && hash = search_field_def.solr_local_parameters) - local_params = hash.collect do |key, val| - key.to_s + "=" + solr_param_quote(val, :quote => "'") - end.join(" ") - solr_parameters[:q] = "{!#{local_params}}#{user_parameters[:q]}" - else - solr_parameters[:q] = user_parameters[:q] if user_parameters[:q] - end - - - ## - # Set Solr spellcheck.q to be original user-entered query, without - # our local params, otherwise it'll try and spellcheck the local - # params! Unless spellcheck.q has already been set by someone, - # respect that. - # - # TODO: Change calling code to expect this as a symbol instead of - # a string, for consistency? :'spellcheck.q' is a symbol. Right now - # rspec tests for a string, and can't tell if other code may - # insist on a string. - solr_parameters["spellcheck.q"] = user_parameters[:q] unless solr_parameters["spellcheck.q"] - end - - ## - # Add any existing facet limits, stored in app-level HTTP query - # as :f, to solr as appropriate :fq query. - def add_facet_fq_to_solr(solr_parameters, user_params) - - # convert a String value into an Array - if solr_parameters[:fq].is_a? String - solr_parameters[:fq] = [solr_parameters[:fq]] - end - - # :fq, map from :f. - if ( user_params[:f]) - f_request_params = user_params[:f] - - f_request_params.each_pair do |facet_field, value_list| - Array(value_list).each do |value| - next if value.blank? # skip empty strings - solr_parameters.append_filter_query facet_value_to_fq_string(facet_field, value) - end - end - end - end - ## - # Add appropriate Solr facetting directives in, including - # taking account of our facet paging/'more'. This is not - # about solr 'fq', this is about solr facet.* params. - def add_facetting_to_solr(solr_parameters, user_params) - # While not used by BL core behavior, legacy behavior seemed to be - # to accept incoming params as "facet.field" or "facets", and add them - # on to any existing facet.field sent to Solr. Legacy behavior seemed - # to be accepting these incoming params as arrays (in Rails URL with [] - # on end), or single values. At least one of these is used by - # Stanford for "faux hieararchial facets". - if user_params.has_key?("facet.field") || user_params.has_key?("facets") - solr_parameters[:"facet.field"].concat( [user_params["facet.field"], user_params["facets"]].flatten.compact ).uniq! - end - - blacklight_config.facet_fields.select { |field_name,facet| - facet.include_in_request || (facet.include_in_request.nil? && blacklight_config.add_facet_fields_to_solr_request) - }.each do |field_name, facet| - solr_parameters[:facet] ||= true - - case - when facet.pivot - solr_parameters.append_facet_pivot with_ex_local_param(facet.ex, facet.pivot.join(",")) - when facet.query - solr_parameters.append_facet_query facet.query.map { |k, x| with_ex_local_param(facet.ex, x[:fq]) } - else - solr_parameters.append_facet_fields with_ex_local_param(facet.ex, facet.field) - end - - if facet.sort - solr_parameters[:"f.#{facet.field}.facet.sort"] = facet.sort - end - - if facet.solr_params - facet.solr_params.each do |k, v| - solr_parameters[:"f.#{facet.field}.#{k}"] = v - end - end - - # Support facet paging and 'more' - # links, by sending a facet.limit one more than what we - # want to page at, according to configured facet limits. - solr_parameters[:"f.#{facet.field}.facet.limit"] = (facet_limit_for(field_name) + 1) if facet_limit_for(field_name) - end - end - - def add_solr_fields_to_query solr_parameters, user_parameters - blacklight_config.show_fields.select(&method(:should_add_to_solr)).each do |field_name, field| - if field.solr_params - field.solr_params.each do |k, v| - solr_parameters[:"f.#{field.field}.#{k}"] = v - end - end - end - - blacklight_config.index_fields.select(&method(:should_add_to_solr)).each do |field_name, field| - if field.highlight - solr_parameters[:hl] = true - solr_parameters.append_highlight_field field.field - end - - if field.solr_params - field.solr_params.each do |k, v| - solr_parameters[:"f.#{field.field}.#{k}"] = v - end - end - end - end - - ### - # copy paging params from BL app over to solr, changing - # app level per_page and page to Solr rows and start. - def add_paging_to_solr(solr_params, user_params) - - # user-provided parameters should override any default row - solr_params[:rows] = user_params[:rows].to_i unless user_params[:rows].blank? - solr_params[:rows] = user_params[:per_page].to_i unless user_params[:per_page].blank? - - # configuration defaults should only set a default value, not override a value set elsewhere (e.g. search field parameters) - solr_params[:rows] ||= blacklight_config.default_per_page unless blacklight_config.default_per_page.blank? - solr_params[:rows] ||= blacklight_config.per_page.first unless blacklight_config.per_page.blank? - - # set a reasonable default - Rails.logger.info "Solr :rows parameter not set (by the user, configuration, or default solr parameters); using 10 rows by default" - solr_params[:rows] ||= 10 - - # ensure we don't excede the max page size - solr_params[:rows] = blacklight_config.max_per_page if solr_params[:rows].to_i > blacklight_config.max_per_page - unless user_params[:page].blank? - solr_params[:start] = solr_params[:rows].to_i * (user_params[:page].to_i - 1) - solr_params[:start] = 0 if solr_params[:start].to_i < 0 - end - - end - - ### - # copy sorting params from BL app over to solr - def add_sorting_to_solr(solr_parameters, user_params) - if user_params[:sort].blank? and sort_field = blacklight_config.default_sort_field - # no sort param provided, use default - solr_parameters[:sort] = sort_field.sort unless sort_field.sort.blank? - elsif sort_field = blacklight_config.sort_fields[user_params[:sort]] - # check for sort field key - solr_parameters[:sort] = sort_field.sort unless sort_field.sort.blank? - else - # just pass the key through - solr_parameters[:sort] = user_params[:sort] - end - end - - # Remove the group parameter if we've faceted on the group field (e.g. for the full results for a group) - def add_group_config_to_solr solr_parameters, user_parameters - if user_parameters[:f] and user_parameters[:f][grouped_key_for_results] - solr_parameters[:group] = false - end - end - - def with_ex_local_param(ex, value) - if ex - "{!ex=#{ex}}#{value}" - else - value - end - end - - DEFAULT_FACET_LIMIT = 10 # Look up facet limit for given facet_field. Will look at config, and @@ -396,57 +193,5 @@ def facet_limit_for(facet_field) 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) - facet_config = blacklight_config.facet_fields[facet_field] - - local_params = [] - local_params << "tag=#{facet_config.tag}" if facet_config and facet_config.tag - - prefix = "" - prefix = "{!#{local_params.join(" ")}}" unless local_params.empty? - - fq = case - when (facet_config and facet_config.query) - facet_config.query[value][:fq] - when (facet_config and facet_config.date) - # in solr 3.2+, this could be replaced by a !term query - "#{prefix}#{facet_field}:#{RSolr.escape(value)}" - when (value.is_a?(DateTime) or value.is_a?(Date) or value.is_a?(Time)) - "#{prefix}#{facet_field}:#{RSolr.escape(value.to_time.utc.strftime("%Y-%m-%dT%H:%M:%SZ"))}" - when (value.is_a?(TrueClass) or value.is_a?(FalseClass) or value == 'true' or value == 'false'), - (value.is_a?(Integer) or (value.to_i.to_s == value if value.respond_to? :to_i)), - (value.is_a?(Float) or (value.to_f.to_s == value if value.respond_to? :to_f)) - "#{prefix}#{facet_field}:#{RSolr.escape(value.to_s)}" - when value.is_a?(Range) - "#{prefix}#{facet_field}:[#{value.first} TO #{value.last}]" - else - "{!raw f=#{facet_field}#{(" " + local_params.join(" ")) unless local_params.empty?}}#{value}" - end - - end end end diff --git a/lib/blacklight/search_builder.rb b/lib/blacklight/search_builder.rb index b23147588c..4813d1888c 100644 --- a/lib/blacklight/search_builder.rb +++ b/lib/blacklight/search_builder.rb @@ -1,14 +1,22 @@ module Blacklight class SearchBuilder - # @param [Hash,HashWithIndifferentAccess] user_params the user provided parameters (e.g. query, facets, sort, etc) + extend Deprecation + self.deprecation_horizon = "blacklight 6.0" + + attr_reader :blacklight_params + # @param [List] processor_chain a list of filter methods to run # @param [Object] scope the scope where the filter methods reside in. - def initialize(user_params, processor_chain, scope) - @user_params = user_params + def initialize(processor_chain, scope) @processor_chain = processor_chain @scope = scope end + def with blacklight_params + @blacklight_params = blacklight_params + self + end + # 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 @@ -34,10 +42,63 @@ def query(extra_params = nil) def processed_parameters Blacklight::Solr::Request.new.tap do |request_parameters| @processor_chain.each do |method_name| - @scope.send(method_name, request_parameters, @user_params) + if @scope.respond_to?(method_name, true) + Deprecation.warn Blacklight::SearchBuilder, "Building search parameters by calling #{method_name} on #{@scope.to_s}. This behavior will be deprecated in Blacklight 6.0" + @scope.send(method_name, request_parameters, @user_params) + else + send(method_name, request_parameters) + end end end end + def blacklight_config + @scope.blacklight_config + end + + protected + def page + blacklight_params[:page].to_i unless blacklight_params[:page].blank? + end + + def rows default = nil + # user-provided parameters should override any default row + rows = blacklight_params[:rows].to_i unless blacklight_params[:rows].blank? + rows = blacklight_params[:per_page].to_i unless blacklight_params[:per_page].blank? + + default ||= blacklight_config.default_per_page + default ||= 10 + rows ||= default + + # ensure we don't excede the max page size + rows = blacklight_config.max_per_page if rows.to_i > blacklight_config.max_per_page + + + rows + end + + def sort + field = if blacklight_params[:sort].blank? and sort_field = blacklight_config.default_sort_field + # no sort param provided, use default + sort_field.sort + elsif sort_field = blacklight_config.sort_fields[blacklight_params[:sort]] + # check for sort field key + sort_field.sort + else + # just pass the key through + blacklight_params[:sort] + end + + field unless field.blank? + end + + def search_field + blacklight_config.search_fields[blacklight_params[:search_field]] + 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 + end end diff --git a/lib/blacklight/search_helper.rb b/lib/blacklight/search_helper.rb index 4065b18e52..8a5d9fd3a4 100644 --- a/lib/blacklight/search_helper.rb +++ b/lib/blacklight/search_helper.rb @@ -79,7 +79,7 @@ def solr_doc_params(id=nil) # and second an array of SolrDocuments representing the response.docs def get_search_results(user_params = params || {}, extra_controller_params = {}) Deprecation.warn(self, "get_search_results is deprecated and will be removed in blacklight-6.0. Use `search_results' instead") - query = search_builder(user_params).query(extra_controller_params) + query = search_builder.with(user_params).query(extra_controller_params) response = repository.search(query) case @@ -98,7 +98,7 @@ def get_search_results(user_params = params || {}, extra_controller_params = {}) # @param [List] the solr response object and a list of solr documents def get_solr_response_for_field_values(field, values, extra_controller_params = {}) - query = search_builder(params).query(extra_controller_params.merge(solr_documents_by_field_values_params(field, values))) + query = search_builder.with(params).query(extra_controller_params.merge(solr_documents_by_field_values_params(field, values))) solr_response = repository.search(query) @@ -179,7 +179,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(user_params).query(extra_controller_params.merge(solr_facet_params(facet_field, user_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))) repository.search(query) end @@ -211,7 +211,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) - request_params = search_builder(request_params).processed_parameters + request_params = search_builder.with(request_params).processed_parameters request_params[:start] = (index - 1) # start at 0 to get 1st doc, 1 to get 2nd. request_params[:rows] = 1 @@ -225,7 +225,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={}) - query = search_builder(request_params).query(extra_controller_params.merge(previous_and_next_document_params(index))) + query = search_builder.with(request_params).query(extra_controller_params.merge(previous_and_next_document_params(index))) response = repository.search(query) document_list = response.documents @@ -246,16 +246,12 @@ 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(request_params).query(solr_opensearch_params(field).merge(extra_controller_params)) + query = search_builder.with(request_params).query(solr_opensearch_params(field).merge(extra_controller_params)) response = repository.search(query) [response.params[:q], response.documents.flat_map {|doc| doc[field] }.uniq] end - def search_builder(user_params, processor_chain = search_params_logic) - @search_builder ||= Blacklight::SearchBuilder.new(user_params, processor_chain, self) - end - ## # The key to use to retrieve the grouped field to display def grouped_key_for_results diff --git a/lib/blacklight/solr.rb b/lib/blacklight/solr.rb index ce212174ae..a4896934ef 100644 --- a/lib/blacklight/solr.rb +++ b/lib/blacklight/solr.rb @@ -5,5 +5,6 @@ module Blacklight::Solr autoload :FacetPaginator, 'blacklight/solr/facet_paginator' autoload :Document, 'blacklight/solr/document' autoload :Request, 'blacklight/solr/request' + autoload :SearchBuilder, 'blacklight/solr/search_builder' end diff --git a/lib/blacklight/solr/search_builder.rb b/lib/blacklight/solr/search_builder.rb new file mode 100644 index 0000000000..9468ce1961 --- /dev/null +++ b/lib/blacklight/solr/search_builder.rb @@ -0,0 +1,273 @@ +module Blacklight::Solr + class SearchBuilder < Blacklight::SearchBuilder + + #### + # Start with general defaults from BL config. Need to use custom + # merge to dup values, to avoid later mutating the original by mistake. + def default_solr_parameters(solr_parameters) + blacklight_config.default_solr_params.each do |key, value| + solr_parameters[key] = if value.respond_to? :deep_dup + value.deep_dup + elsif value.respond_to? :dup and value.duplicable? + value.dup + else + value + end + end + end + + ## + # Take the user-entered query, and put it in the solr params, + # including config's "search field" params for current search field. + # also include setting spellcheck.q. + def add_query_to_solr(solr_parameters) + ### + # Merge in search field configured values, if present, over-writing general + # defaults + ### + # legacy behavior of user param :qt is passed through, but over-ridden + # by actual search field config if present. We might want to remove + # this legacy behavior at some point. It does not seem to be currently + # rspec'd. + solr_parameters[:qt] = blacklight_params[:qt] if blacklight_params[:qt] + + if search_field + solr_parameters[:qt] = search_field.qt + solr_parameters.merge!( search_field.solr_parameters) if search_field.solr_parameters + end + + ## + # Create Solr 'q' including the user-entered q, prefixed by any + # solr LocalParams in config, using solr LocalParams syntax. + # http://wiki.apache.org/solr/LocalParams + ## + if (search_field && hash = search_field.solr_local_parameters) + local_params = hash.collect do |key, val| + key.to_s + "=" + solr_param_quote(val, :quote => "'") + end.join(" ") + solr_parameters[:q] = "{!#{local_params}}#{blacklight_params[:q]}" + elsif blacklight_params[:q].is_a? Hash + q = blacklight_params[:q] + solr_parameters[:q] = if q.values.any?(&:blank?) + # if any field parameters are empty, exclude _all_ results + "{!lucene}NOT *:*" + else + "{!lucene}" + q.map do |field, values| + "#{field}:(#{ Array(values).map { |x| solr_param_quote(x) }.join(" OR ")})" + end.join(" AND ") + end + + solr_parameters[:spellcheck] = 'false' + elsif blacklight_params[:q] + solr_parameters[:q] = blacklight_params[:q] + end + + + ## + # Set Solr spellcheck.q to be original user-entered query, without + # our local params, otherwise it'll try and spellcheck the local + # params! Unless spellcheck.q has already been set by someone, + # respect that. + # + # TODO: Change calling code to expect this as a symbol instead of + # a string, for consistency? :'spellcheck.q' is a symbol. Right now + # rspec tests for a string, and can't tell if other code may + # insist on a string. + solr_parameters["spellcheck.q"] = blacklight_params[:q] unless solr_parameters["spellcheck.q"] + end + + ## + # Add any existing facet limits, stored in app-level HTTP query + # as :f, to solr as appropriate :fq query. + def add_facet_fq_to_solr(solr_parameters) + + # convert a String value into an Array + if solr_parameters[:fq].is_a? String + solr_parameters[:fq] = [solr_parameters[:fq]] + end + + # :fq, map from :f. + if ( blacklight_params[:f]) + f_request_params = blacklight_params[:f] + + f_request_params.each_pair do |facet_field, value_list| + Array(value_list).each do |value| + next if value.blank? # skip empty strings + solr_parameters.append_filter_query facet_value_to_fq_string(facet_field, value) + end + end + end + end + + ## + # Add appropriate Solr facetting directives in, including + # taking account of our facet paging/'more'. This is not + # about solr 'fq', this is about solr facet.* params. + def add_facetting_to_solr(solr_parameters) + # While not used by BL core behavior, legacy behavior seemed to be + # to accept incoming params as "facet.field" or "facets", and add them + # on to any existing facet.field sent to Solr. Legacy behavior seemed + # to be accepting these incoming params as arrays (in Rails URL with [] + # on end), or single values. At least one of these is used by + # Stanford for "faux hieararchial facets". + if blacklight_params.has_key?("facet.field") || blacklight_params.has_key?("facets") + solr_parameters[:"facet.field"].concat( [blacklight_params["facet.field"], blacklight_params["facets"]].flatten.compact ).uniq! + end + + blacklight_config.facet_fields.select { |field_name,facet| + facet.include_in_request || (facet.include_in_request.nil? && blacklight_config.add_facet_fields_to_solr_request) + }.each do |field_name, facet| + solr_parameters[:facet] ||= true + + case + when facet.pivot + solr_parameters.append_facet_pivot with_ex_local_param(facet.ex, facet.pivot.join(",")) + when facet.query + solr_parameters.append_facet_query facet.query.map { |k, x| with_ex_local_param(facet.ex, x[:fq]) } + else + solr_parameters.append_facet_fields with_ex_local_param(facet.ex, facet.field) + end + + if facet.sort + solr_parameters[:"f.#{facet.field}.facet.sort"] = facet.sort + end + + if facet.solr_params + facet.solr_params.each do |k, v| + solr_parameters[:"f.#{facet.field}.#{k}"] = v + end + end + + # Support facet paging and 'more' + # links, by sending a facet.limit one more than what we + # want to page at, according to configured facet limits. + solr_parameters[:"f.#{facet.field}.facet.limit"] = (facet_limit_for(field_name) + 1) if facet_limit_for(field_name) + end + end + + def add_solr_fields_to_query solr_parameters + blacklight_config.show_fields.select(&method(:should_add_field_to_request?)).each do |field_name, field| + if field.solr_params + field.solr_params.each do |k, v| + solr_parameters[:"f.#{field.field}.#{k}"] = v + end + end + end + + blacklight_config.index_fields.select(&method(:should_add_field_to_request?)).each do |field_name, field| + if field.highlight + solr_parameters[:hl] = true + solr_parameters.append_highlight_field field.field + end + + if field.solr_params + field.solr_params.each do |k, v| + solr_parameters[:"f.#{field.field}.#{k}"] = v + end + end + end + end + + ### + # copy paging params from BL app over to solr, changing + # app level per_page and page to Solr rows and start. + def add_paging_to_solr(solr_params) + # user-provided parameters should override any default row + solr_params[:rows] = rows(solr_params[:rows]) + unless page.blank? + solr_params[:start] = solr_params[:rows].to_i * (page - 1) + solr_params[:start] = 0 if solr_params[:start].to_i < 0 + end + + end + + ### + # copy sorting params from BL app over to solr + def add_sorting_to_solr(solr_parameters) + solr_parameters[:sort] = sort unless sort.blank? + end + + # Remove the group parameter if we've faceted on the group field (e.g. for the full results for a group) + def add_group_config_to_solr solr_parameters + if blacklight_params[:f] and blacklight_params[:f][grouped_key_for_results] + solr_parameters[:group] = false + end + end + + def with_ex_local_param(ex, value) + if ex + "{!ex=#{ex}}#{value}" + else + value + end + end + + # 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 #add_facetting_to_solr + # 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 + facet.limit == true ? blacklight_config.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 + + ## + # Convert a facet/value pair into a solr fq parameter + def facet_value_to_fq_string(facet_field, value) + facet_config = blacklight_config.facet_fields[facet_field] + + local_params = [] + local_params << "tag=#{facet_config.tag}" if facet_config and facet_config.tag + + prefix = "" + prefix = "{!#{local_params.join(" ")}}" unless local_params.empty? + + fq = case + when (facet_config and facet_config.query) + facet_config.query[value][:fq] + when (facet_config and facet_config.date) + # in solr 3.2+, this could be replaced by a !term query + "#{prefix}#{facet_field}:#{RSolr.escape(value)}" + when (value.is_a?(DateTime) or value.is_a?(Date) or value.is_a?(Time)) + "#{prefix}#{facet_field}:#{RSolr.escape(value.to_time.utc.strftime("%Y-%m-%dT%H:%M:%SZ"))}" + when (value.is_a?(TrueClass) or value.is_a?(FalseClass) or value == 'true' or value == 'false'), + (value.is_a?(Integer) or (value.to_i.to_s == value if value.respond_to? :to_i)), + (value.is_a?(Float) or (value.to_f.to_s == value if value.respond_to? :to_f)) + "#{prefix}#{facet_field}:#{RSolr.escape(value.to_s)}" + when value.is_a?(Range) + "#{prefix}#{facet_field}:[#{value.first} TO #{value.last}]" + else + "{!raw f=#{facet_field}#{(" " + local_params.join(" ")) unless local_params.empty?}}#{value}" + end + end + + ## + # The key to use to retrieve the grouped field to display + def grouped_key_for_results + blacklight_config.index.group + end + end +end \ No newline at end of file diff --git a/spec/lib/blacklight/search_builder_spec.rb b/spec/lib/blacklight/search_builder_spec.rb index 0206f23f34..11e5558828 100644 --- a/spec/lib/blacklight/search_builder_spec.rb +++ b/spec/lib/blacklight/search_builder_spec.rb @@ -1,341 +1,5 @@ require 'spec_helper' describe Blacklight::SearchBuilder do - let(:single_facet) { { format: 'Book' } } - let(:multi_facets) { { format: 'Book', language_facet: 'Tibetan' } } - let(:mult_word_query) { 'tibetan history' } - let(:subject_search_params) { { commit: "search", search_field: "subject", action: "index", controller: "catalog", rows: "10", q: "wome" } } - let(:blacklight_config) { CatalogController.blacklight_config.deep_copy } - let(:method_chain) { CatalogController.search_params_logic } - let(:user_params) { Hash.new } - let(:context) { CatalogController.new } - - before { allow(context).to receive(:blacklight_config).and_return(blacklight_config) } - - let(:search_builder) { described_class.new(user_params, method_chain, context) } - - subject { search_builder.processed_parameters } - - context "with a complex parameter environment" do - let(:user_params) do - {:search_field => "test_field", :q => "test query", "facet.field" => "extra_facet"} - end - - let(:blacklight_config) do - Blacklight::Configuration.new.tap do |config| - config.add_search_field("test_field", - :display_label => "Test", - :key=>"test_field", - :solr_parameters => { - :qf => "fieldOne^2.3 fieldTwo fieldThree^0.4", - :pf => "", - :spellcheck => 'false', - :rows => "55", - :sort => "request_params_sort" } - ) - end - end - it "should merge parameters from search_field definition" do - expect(subject[:qf]).to eq "fieldOne^2.3 fieldTwo fieldThree^0.4" - expect(subject[:spellcheck]).to eq 'false' - end - it "should merge empty string parameters from search_field definition" do - expect(subject[:pf]).to eq "" - end - - describe "should respect proper precedence of settings, " do - it "should not put :search_field in produced params" do - expect(subject[:search_field]).to be_nil - end - - it "should fall through to BL general defaults for qt not otherwise specified " do - expect(subject[:qt]).to eq blacklight_config[:default_solr_params][:qt] - end - - it "should take rows from search field definition where specified" do - expect(subject[:rows]).to eq "55" - end - - it "should take q from request params" do - expect(subject[:q]).to eq "test query" - end - - it "should add in extra facet.field from params" do - expect(subject[:"facet.field"]).to include("extra_facet") - end - end - end - - # SPECS for actual search parameter generation - describe "#processed_parameters" do - context "when search_params_logic is customized" do - let(:method_chain) { [:add_foo_to_solr_params] } - - it "allows customization of search_params_logic" do - # Normally you'd include a new module into (eg) your CatalogController - # but a sub-class defininig it directly is simpler for test. - allow(context).to receive(:add_foo_to_solr_params) do |solr_params, user_params| - solr_params[:wt] = "TESTING" - end - - expect(subject[:wt]).to eq "TESTING" - end - end - - it "should generate a facet limit" do - expect(subject[:"f.subject_topic_facet.facet.limit"]).to eq 21 - end - - it "should handle no facet_limits in config" do - blacklight_config.facet_fields = {} - expect(subject).not_to have_key(:"f.subject_topic_facet.facet.limit") - end - - describe "with max per page enforced" do - let(:blacklight_config) do - Blacklight::Configuration.new.tap do |config| - config.max_per_page = 123 - end - end - - let(:user_params) { { per_page: 98765 } } - it "should enforce max_per_page against all parameters" do - expect(blacklight_config.max_per_page).to eq 123 - expect(subject[:rows]).to eq 123 - end - end - - describe 'for an entirely empty search' do - - it 'should not have a q param' do - expect(subject[:q]).to be_nil - expect(subject["spellcheck.q"]).to be_nil - end - it 'should have default rows' do - expect(subject[:rows]).to eq 10 - end - it 'should have default facet fields' do - # remove local params from the facet.field - expect(subject[:"facet.field"].map { |x| x.gsub(/\{![^}]+\}/, '') }).to match_array ["format", "subject_topic_facet", "pub_date", "language_facet", "lc_1letter_facet", "subject_geo_facet", "subject_era_facet"] - end - - it "should have default qt" do - expect(subject[:qt]).to eq "search" - end - it "should have no fq" do - expect(subject[:phrase_filters]).to be_blank - expect(subject[:fq]).to be_blank - end - end - - - describe "for an empty string search" do - let(:user_params) { { q: "" } } - it "should return empty string q in solr parameters" do - expect(subject[:q]).to eq "" - expect(subject["spellcheck.q"]).to eq "" - end - end - - describe "for request params also passed in as argument" do - let(:user_params) { { q: "some query", 'q' => 'another value' } } - it "should only have one value for the key 'q' regardless if a symbol or string" do - expect(subject[:q]).to eq 'some query' - expect(subject['q']).to eq 'some query' - end - end - - - describe "for one facet, no query" do - let(:user_params) { { f: single_facet } } - it "should have proper solr parameters" do - - expect(subject[:q]).to be_blank - expect(subject["spellcheck.q"]).to be_blank - - single_facet.each_value do |value| - expect(subject[:fq]).to include("{!raw f=#{single_facet.keys[0]}}#{value}") - end - end - end - - describe "for an empty facet limit param" do - let(:user_params) { { f: { "format" => [""] } } } - it "should not add any fq to solr" do - expect(subject[:fq]).to be_blank - end - end - - describe "with Multi Facets, No Query" do - let(:user_params) { { f: multi_facets } } - it 'should have fq set properly' do - multi_facets.each_pair do |facet_field, value_list| - value_list ||= [] - value_list = [value_list] unless value_list.respond_to? :each - value_list.each do |value| - expect(subject[:fq]).to include("{!raw f=#{facet_field}}#{value}" ) - end - end - - end - end - - describe "with Multi Facets, Multi Word Query" do - let(:user_params) { { q: mult_word_query, f: multi_facets } } - it 'should have fq and q set properly' do - multi_facets.each_pair do |facet_field, value_list| - value_list ||= [] - value_list = [value_list] unless value_list.respond_to? :each - value_list.each do |value| - expect(subject[:fq]).to include("{!raw f=#{facet_field}}#{value}" ) - end - end - expect(subject[:q]).to eq mult_word_query - end - end - - - describe "solr parameters for a field search from config (subject)" do - let(:user_params) { subject_search_params } - - it "should look up qt from field definition" do - expect(subject[:qt]).to eq "search" - end - - it "should not include weird keys not in field definition" do - expect(subject[:phrase_filters]).to be_nil - expect(subject[:fq]).to eq [] - expect(subject[:commit]).to be_nil - expect(subject[:action]).to be_nil - expect(subject[:controller]).to be_nil - end - - it "should include proper 'q', possibly with LocalParams" do - expect(subject[:q]).to match(/(\{[^}]+\})?wome/) - end - it "should include proper 'q' when LocalParams are used" do - if subject[:q] =~ /\{[^}]+\}/ - expect(subject[:q]).to match(/\{[^}]+\}wome/) - end - end - it "should include spellcheck.q, without LocalParams" do - expect(subject["spellcheck.q"]).to eq "wome" - end - - it "should include spellcheck.dictionary from field def solr_parameters" do - expect(subject[:"spellcheck.dictionary"]).to eq "subject" - end - it "should add on :solr_local_parameters using Solr LocalParams style" do - #q == "{!pf=$subject_pf $qf=subject_qf} wome", make sure - #the LocalParams are really there - subject[:q] =~ /^\{!([^}]+)\}/ - key_value_pairs = $1.split(" ") - expect(key_value_pairs).to include("pf=$subject_pf") - expect(key_value_pairs).to include("qf=$subject_qf") - end - end - - describe "overriding of qt parameter" do - let(:user_params) do - { qt: 'overridden' } - end - - it "should return the correct overriden parameter" do - expect(subject[:qt]).to eq "overridden" - end - end - - - describe "sorting" do - it "should send the default sort parameter to solr" do - expect(subject[:sort]).to eq 'score desc, pub_date_sort desc, title_sort asc' - end - - it "should not send a sort parameter to solr if the sort value is blank" do - blacklight_config.sort_fields = {} - blacklight_config.add_sort_field('', :label => 'test') - - expect(subject).not_to have_key(:sort) - end - - context "when the user provides sort parmeters" do - let(:user_params) { { sort: 'solr_test_field desc' } } - it "passes them through" do - expect(subject[:sort]).to eq 'solr_test_field desc' - end - end - end - - describe "for :solr_local_parameters config" do - let(:blacklight_config) do - config = Blacklight::Configuration.new - config.add_search_field( - "custom_author_key", - :display_label => "Author", - :qt => "author_qt", - :key => "custom_author_key", - :solr_local_parameters => { - :qf => "$author_qf", - :pf => "you'll have \" to escape this", - :pf2 => "$pf2_do_not_escape_or_quote" - }, - :solr_parameters => { - :qf => "someField^1000", - :ps => "2" - } - ) - return config - end - - let(:user_params) { { search_field: "custom_author_key", q: "query" } } - - it "should pass through ordinary params" do - expect(subject[:qt]).to eq "author_qt" - expect(subject[:ps]).to eq "2" - expect(subject[:qf]).to eq "someField^1000" - end - - it "should include include local params with escaping" do - expect(subject[:q]).to include('qf=$author_qf') - expect(subject[:q]).to include('pf=\'you\\\'ll have \\" to escape this\'') - expect(subject[:q]).to include('pf2=$pf2_do_not_escape_or_quote') - end - end - - describe "mapping facet.field" do - let(:blacklight_config) do - Blacklight::Configuration.new do |config| - config.add_facet_field 'some_field' - config.add_facet_fields_to_solr_request! - end - end - - context "user provides a single facet.field" do - let(:user_params) { { "facet.field" => "additional_facet" } } - it "adds the field" do - expect(subject[:"facet.field"]).to include("additional_facet") - expect(subject[:"facet.field"]).to have(2).fields - end - end - - context "user provides a multiple facet.field" do - let(:user_params) { { "facet.field" => ["add_facet1", "add_facet2"] } } - it "adds the fields" do - expect(subject[:"facet.field"]).to include("add_facet1") - expect(subject[:"facet.field"]).to include("add_facet2") - expect(subject[:"facet.field"]).to have(3).fields - end - end - - context "user provides a multiple facets" do - let(:user_params) { { "facets" => ["add_facet1", "add_facet2"] } } - it "adds the fields" do - expect(subject[:"facet.field"]).to include("add_facet1") - expect(subject[:"facet.field"]).to include("add_facet2") - expect(subject[:"facet.field"]).to have(3).fields - end - end - end - end end diff --git a/spec/lib/blacklight/search_helper_spec.rb b/spec/lib/blacklight/search_helper_spec.rb index f45866546e..43f2a51509 100644 --- a/spec/lib/blacklight/search_helper_spec.rb +++ b/spec/lib/blacklight/search_helper_spec.rb @@ -59,214 +59,11 @@ def params solr_params[:wt] = "TESTING" end - expect(Deprecation).to receive(:warn) + allow(Deprecation).to receive(:warn) expect(subject.solr_search_params({}, [:add_foo_to_solr_params])[:wt]).to eq "TESTING" end end - describe "facet_value_to_fq_string" do - - it "should use the raw handler for strings" do - expect(subject.send(:facet_value_to_fq_string, "facet_name", "my value")).to eq "{!raw f=facet_name}my value" - end - - it "should pass booleans through" do - expect(subject.send(:facet_value_to_fq_string, "facet_name", true)).to eq "facet_name:true" - end - - it "should pass boolean-like strings through" do - expect(subject.send(:facet_value_to_fq_string, "facet_name", "true")).to eq "facet_name:true" - end - - it "should pass integers through" do - expect(subject.send(:facet_value_to_fq_string, "facet_name", 1)).to eq "facet_name:1" - end - - it "should pass integer-like strings through" do - expect(subject.send(:facet_value_to_fq_string, "facet_name", "1")).to eq "facet_name:1" - end - - it "should pass floats through" do - expect(subject.send(:facet_value_to_fq_string, "facet_name", 1.11)).to eq "facet_name:1\\.11" - end - - it "should pass floats through" do - expect(subject.send(:facet_value_to_fq_string, "facet_name", "1.11")).to eq "facet_name:1\\.11" - end - - it "should escape negative integers" do - expect(subject.send(:facet_value_to_fq_string, "facet_name", -1)).to eq "facet_name:\\-1" - end - - it "should pass date-type fields through" do - allow(blacklight_config.facet_fields).to receive(:[]).with('facet_name').and_return(double(:date => true, :query => nil, :tag => nil)) - - expect(subject.send(:facet_value_to_fq_string, "facet_name", "2012-01-01")).to eq "facet_name:2012\\-01\\-01" - end - - it "should escape datetime-type fields" do - allow(blacklight_config.facet_fields).to receive(:[]).with('facet_name').and_return(double(:date => true, :query => nil, :tag => nil)) - - expect(subject.send(:facet_value_to_fq_string, "facet_name", "2003-04-09T00:00:00Z")).to eq "facet_name:2003\\-04\\-09T00\\:00\\:00Z" - end - - it "should format Date objects correctly" do - allow(blacklight_config.facet_fields).to receive(:[]).with('facet_name').and_return(double(:date => nil, :query => nil, :tag => nil)) - d = DateTime.parse("2003-04-09T00:00:00") - expect(subject.send(:facet_value_to_fq_string, "facet_name", d)).to eq "facet_name:2003\\-04\\-09T00\\:00\\:00Z" - end - - it "should handle range requests" do - expect(subject.send(:facet_value_to_fq_string, "facet_name", 1..5)).to eq "facet_name:[1 TO 5]" - end - - it "should add tag local parameters" do - allow(blacklight_config.facet_fields).to receive(:[]).with('facet_name').and_return(double(:query => nil, :tag => 'asdf', :date => nil)) - - expect(subject.send(:facet_value_to_fq_string, "facet_name", true)).to eq "{!tag=asdf}facet_name:true" - expect(subject.send(:facet_value_to_fq_string, "facet_name", "my value")).to eq "{!raw f=facet_name tag=asdf}my value" - end - end - - describe "converts a String fq into an Array" do - it "should return the correct overriden parameter" do - solr_parameters = {:fq => 'a string' } - - subject.add_facet_fq_to_solr(solr_parameters, {}) - - expect(solr_parameters[:fq]).to be_a_kind_of Array - end - end - - describe "#add_solr_fields_to_query" do - let(:blacklight_config) do - config = Blacklight::Configuration.new do |config| - - config.add_index_field 'an_index_field', solr_params: { 'hl.alternativeField' => 'field_x'} - config.add_show_field 'a_show_field', solr_params: { 'hl.alternativeField' => 'field_y'} - config.add_field_configuration_to_solr_request! - end - end - - let(:solr_parameters) do - solr_parameters = Blacklight::Solr::Request.new - - subject.add_solr_fields_to_query(solr_parameters, {}) - - solr_parameters - end - - it "should add any extra solr parameters from index and show fields" do - expect(solr_parameters[:'f.an_index_field.hl.alternativeField']).to eq "field_x" - expect(solr_parameters[:'f.a_show_field.hl.alternativeField']).to eq "field_y" - end - end - - describe "#add_facetting_to_solr" do - - let(:blacklight_config) do - config = Blacklight::Configuration.new - - config.add_facet_field 'test_field', :sort => 'count' - config.add_facet_field 'some-query', :query => {'x' => {:fq => 'some:query' }}, :ex => 'xyz' - config.add_facet_field 'some-pivot', :pivot => ['a','b'], :ex => 'xyz' - config.add_facet_field 'some-field', solr_params: { 'facet.mincount' => 15 } - config.add_facet_fields_to_solr_request! - - config - end - - let(:solr_parameters) do - solr_parameters = Blacklight::Solr::Request.new - - subject.add_facetting_to_solr(solr_parameters, {}) - - solr_parameters - end - - it "should add sort parameters" do - expect(solr_parameters[:facet]).to be true - - expect(solr_parameters[:'facet.field']).to include('test_field') - expect(solr_parameters[:'f.test_field.facet.sort']).to eq 'count' - end - - it "should add facet exclusions" do - expect(solr_parameters[:'facet.query']).to include('{!ex=xyz}some:query') - expect(solr_parameters[:'facet.pivot']).to include('{!ex=xyz}a,b') - end - - it "should add any additional solr_params" do - expect(solr_parameters[:'f.some-field.facet.mincount']).to eq 15 - end - - describe ":include_in_request" do - let(:solr_parameters) do - solr_parameters = Blacklight::Solr::Request.new - subject.add_facetting_to_solr(solr_parameters, {}) - solr_parameters - end - - it "should respect the include_in_request parameter" do - blacklight_config.add_facet_field 'yes_facet', include_in_request: true - blacklight_config.add_facet_field 'no_facet', include_in_request: false - - expect(solr_parameters[:'facet.field']).to include('yes_facet') - expect(solr_parameters[:'facet.field']).not_to include('no_facet') - end - - it "should default to including facets if add_facet_fields_to_solr_request! was called" do - blacklight_config.add_facet_field 'yes_facet' - blacklight_config.add_facet_field 'no_facet', include_in_request: false - blacklight_config.add_facet_fields_to_solr_request! - - expect(solr_parameters[:'facet.field']).to include('yes_facet') - expect(solr_parameters[:'facet.field']).not_to include('no_facet') - end - end - - describe ":add_facet_fields_to_solr_request!" do - - let(:blacklight_config) do - config = Blacklight::Configuration.new - config.add_facet_field 'yes_facet', include_in_request: true - config.add_facet_field 'no_facet', include_in_request: false - config.add_facet_field 'maybe_facet' - config.add_facet_field 'another_facet' - config - end - - let(:solr_parameters) do - solr_parameters = Blacklight::Solr::Request.new - subject.add_facetting_to_solr(solr_parameters, {}) - solr_parameters - end - - it "should add facets to the solr request" do - blacklight_config.add_facet_fields_to_solr_request! - expect(solr_parameters[:'facet.field']).to match_array ['yes_facet', 'maybe_facet', 'another_facet'] - end - - it "should not override field-specific configuration by default" do - blacklight_config.add_facet_fields_to_solr_request! - expect(solr_parameters[:'facet.field']).to_not include 'no_facet' - end - - it "should allow white-listing facets" do - blacklight_config.add_facet_fields_to_solr_request! 'maybe_facet' - expect(solr_parameters[:'facet.field']).to include 'maybe_facet' - expect(solr_parameters[:'facet.field']).not_to include 'another_facet' - end - - it "should allow white-listed facets to override any field-specific include_in_request configuration" do - blacklight_config.add_facet_fields_to_solr_request! 'no_facet' - expect(solr_parameters[:'facet.field']).to include 'no_facet' - end - end - end - - - describe "solr_facet_params" do before do @facet_field = 'format' @@ -339,22 +136,6 @@ def params end end - describe "for facet limit parameters config ed" do - # subject do - # subject.solr_search_params({ search_field: "test_field", :q => "test query"}, {}, default_method_chain) - # end - - # let(:blacklight_config) { copy_of_catalog_config } - # - # it "should include specifically configged facet limits +1" do - # expect(subject[:"f.subject_topic_facet.facet.limit"]).to eq 21 - # end - # it "should not include a facet limit for a nil key in hash" do - # expect(subject).not_to have_key(:"f.format.facet.limit") - # expect(subject).not_to have_key(:"facet.limit") - # end - end - describe "get_facet_pagination", :integration => true do before do Deprecation.silence(Blacklight::SearchHelper) do @@ -949,19 +730,6 @@ def params end end - describe "#with_tag_ex" do - it "should add an !ex local parameter if the facet configuration requests it" do - expect(subject.with_ex_local_param("xyz", "some-value")).to eq "{!ex=xyz}some-value" - end - - it "should not add an !ex local parameter if it isn't configured" do - mock_field = double() - expect(subject.with_ex_local_param(nil, "some-value")).to eq "some-value" - end - - - end - describe "#get_previous_and_next_documents_for_search" do let(:pre_query) { SearchHelperTestClass.new blacklight_config, blacklight_solr } before do diff --git a/spec/lib/blacklight/solr/search_builder_spec.rb b/spec/lib/blacklight/solr/search_builder_spec.rb new file mode 100644 index 0000000000..8fb923067f --- /dev/null +++ b/spec/lib/blacklight/solr/search_builder_spec.rb @@ -0,0 +1,558 @@ +require 'spec_helper' + +describe Blacklight::Solr::SearchBuilder do + let(:single_facet) { { format: 'Book' } } + let(:multi_facets) { { format: 'Book', language_facet: 'Tibetan' } } + let(:mult_word_query) { 'tibetan history' } + let(:subject_search_params) { { commit: "search", search_field: "subject", action: "index", controller: "catalog", rows: "10", q: "wome" } } + + let(:blacklight_config) { CatalogController.blacklight_config.deep_copy } + let(:method_chain) { CatalogController.search_params_logic } + let(:user_params) { Hash.new } + let(:context) { CatalogController.new } + + before { allow(context).to receive(:blacklight_config).and_return(blacklight_config) } + + let(:search_builder) { described_class.new(method_chain, context) } + + subject { search_builder.with(user_params) } + + context "with a complex parameter environment" do + subject { search_builder.with(user_params).processed_parameters } + + let(:user_params) do + {:search_field => "test_field", :q => "test query", "facet.field" => "extra_facet"} + end + + let(:blacklight_config) do + Blacklight::Configuration.new.tap do |config| + config.add_search_field("test_field", + :display_label => "Test", + :key=>"test_field", + :solr_parameters => { + :qf => "fieldOne^2.3 fieldTwo fieldThree^0.4", + :pf => "", + :spellcheck => 'false', + :rows => "55", + :sort => "request_params_sort" } + ) + end + end + it "should merge parameters from search_field definition" do + expect(subject[:qf]).to eq "fieldOne^2.3 fieldTwo fieldThree^0.4" + expect(subject[:spellcheck]).to eq 'false' + end + it "should merge empty string parameters from search_field definition" do + expect(subject[:pf]).to eq "" + end + + describe "should respect proper precedence of settings, " do + it "should not put :search_field in produced params" do + expect(subject[:search_field]).to be_nil + end + + it "should fall through to BL general defaults for qt not otherwise specified " do + expect(subject[:qt]).to eq blacklight_config[:default_solr_params][:qt] + end + + it "should take rows from search field definition where specified" do + expect(subject[:rows]).to eq "55" + end + + it "should take q from request params" do + expect(subject[:q]).to eq "test query" + end + + it "should add in extra facet.field from params" do + expect(subject[:"facet.field"]).to include("extra_facet") + end + end + end + + # SPECS for actual search parameter generation + describe "#processed_parameters" do + subject { search_builder.with(user_params).processed_parameters } + + context "when search_params_logic is customized" do + let(:method_chain) { [:add_foo_to_solr_params] } + + it "allows customization of search_params_logic" do + # Normally you'd include a new module into (eg) your CatalogController + # but a sub-class defininig it directly is simpler for test. + allow(context).to receive(:add_foo_to_solr_params) do |solr_params, user_params| + solr_params[:wt] = "TESTING" + end + + expect(subject[:wt]).to eq "TESTING" + end + end + + it "should generate a facet limit" do + expect(subject[:"f.subject_topic_facet.facet.limit"]).to eq 21 + end + + it "should handle no facet_limits in config" do + blacklight_config.facet_fields = {} + expect(subject).not_to have_key(:"f.subject_topic_facet.facet.limit") + end + + describe "with max per page enforced" do + let(:blacklight_config) do + Blacklight::Configuration.new.tap do |config| + config.max_per_page = 123 + end + end + + let(:user_params) { { per_page: 98765 } } + it "should enforce max_per_page against all parameters" do + expect(blacklight_config.max_per_page).to eq 123 + expect(subject[:rows]).to eq 123 + end + end + + describe 'for an entirely empty search' do + + it 'should not have a q param' do + expect(subject[:q]).to be_nil + expect(subject["spellcheck.q"]).to be_nil + end + it 'should have default rows' do + expect(subject[:rows]).to eq 10 + end + it 'should have default facet fields' do + # remove local params from the facet.field + expect(subject[:"facet.field"].map { |x| x.gsub(/\{![^}]+\}/, '') }).to match_array ["format", "subject_topic_facet", "pub_date", "language_facet", "lc_1letter_facet", "subject_geo_facet", "subject_era_facet"] + end + + it "should have default qt" do + expect(subject[:qt]).to eq "search" + end + it "should have no fq" do + expect(subject[:phrase_filters]).to be_blank + expect(subject[:fq]).to be_blank + end + end + + + describe "for an empty string search" do + let(:user_params) { { q: "" } } + it "should return empty string q in solr parameters" do + expect(subject[:q]).to eq "" + expect(subject["spellcheck.q"]).to eq "" + end + end + + describe "for request params also passed in as argument" do + let(:user_params) { { q: "some query", 'q' => 'another value' } } + it "should only have one value for the key 'q' regardless if a symbol or string" do + expect(subject[:q]).to eq 'some query' + expect(subject['q']).to eq 'some query' + end + end + + + describe "for one facet, no query" do + let(:user_params) { { f: single_facet } } + it "should have proper solr parameters" do + + expect(subject[:q]).to be_blank + expect(subject["spellcheck.q"]).to be_blank + + single_facet.each_value do |value| + expect(subject[:fq]).to include("{!raw f=#{single_facet.keys[0]}}#{value}") + end + end + end + + describe "for an empty facet limit param" do + let(:user_params) { { f: { "format" => [""] } } } + it "should not add any fq to solr" do + expect(subject[:fq]).to be_blank + end + end + + describe "with Multi Facets, No Query" do + let(:user_params) { { f: multi_facets } } + it 'should have fq set properly' do + multi_facets.each_pair do |facet_field, value_list| + value_list ||= [] + value_list = [value_list] unless value_list.respond_to? :each + value_list.each do |value| + expect(subject[:fq]).to include("{!raw f=#{facet_field}}#{value}" ) + end + end + + end + end + + describe "with Multi Facets, Multi Word Query" do + let(:user_params) { { q: mult_word_query, f: multi_facets } } + it 'should have fq and q set properly' do + multi_facets.each_pair do |facet_field, value_list| + value_list ||= [] + value_list = [value_list] unless value_list.respond_to? :each + value_list.each do |value| + expect(subject[:fq]).to include("{!raw f=#{facet_field}}#{value}" ) + end + end + expect(subject[:q]).to eq mult_word_query + end + end + + + describe "solr parameters for a field search from config (subject)" do + let(:user_params) { subject_search_params } + + it "should look up qt from field definition" do + expect(subject[:qt]).to eq "search" + end + + it "should not include weird keys not in field definition" do + expect(subject[:phrase_filters]).to be_nil + expect(subject[:fq]).to eq [] + expect(subject[:commit]).to be_nil + expect(subject[:action]).to be_nil + expect(subject[:controller]).to be_nil + end + + it "should include proper 'q', possibly with LocalParams" do + expect(subject[:q]).to match(/(\{[^}]+\})?wome/) + end + it "should include proper 'q' when LocalParams are used" do + if subject[:q] =~ /\{[^}]+\}/ + expect(subject[:q]).to match(/\{[^}]+\}wome/) + end + end + it "should include spellcheck.q, without LocalParams" do + expect(subject["spellcheck.q"]).to eq "wome" + end + + it "should include spellcheck.dictionary from field def solr_parameters" do + expect(subject[:"spellcheck.dictionary"]).to eq "subject" + end + it "should add on :solr_local_parameters using Solr LocalParams style" do + #q == "{!pf=$subject_pf $qf=subject_qf} wome", make sure + #the LocalParams are really there + subject[:q] =~ /^\{!([^}]+)\}/ + key_value_pairs = $1.split(" ") + expect(key_value_pairs).to include("pf=$subject_pf") + expect(key_value_pairs).to include("qf=$subject_qf") + end + end + + describe "overriding of qt parameter" do + let(:user_params) do + { qt: 'overridden' } + end + + it "should return the correct overriden parameter" do + expect(subject[:qt]).to eq "overridden" + end + end + + + describe "sorting" do + it "should send the default sort parameter to solr" do + expect(subject[:sort]).to eq 'score desc, pub_date_sort desc, title_sort asc' + end + + it "should not send a sort parameter to solr if the sort value is blank" do + blacklight_config.sort_fields = {} + blacklight_config.add_sort_field('', :label => 'test') + + expect(subject).not_to have_key(:sort) + end + + context "when the user provides sort parmeters" do + let(:user_params) { { sort: 'solr_test_field desc' } } + it "passes them through" do + expect(subject[:sort]).to eq 'solr_test_field desc' + end + end + end + + describe "for :solr_local_parameters config" do + let(:blacklight_config) do + config = Blacklight::Configuration.new + config.add_search_field( + "custom_author_key", + :display_label => "Author", + :qt => "author_qt", + :key => "custom_author_key", + :solr_local_parameters => { + :qf => "$author_qf", + :pf => "you'll have \" to escape this", + :pf2 => "$pf2_do_not_escape_or_quote" + }, + :solr_parameters => { + :qf => "someField^1000", + :ps => "2" + } + ) + return config + end + + let(:user_params) { { search_field: "custom_author_key", q: "query" } } + + it "should pass through ordinary params" do + expect(subject[:qt]).to eq "author_qt" + expect(subject[:ps]).to eq "2" + expect(subject[:qf]).to eq "someField^1000" + end + + it "should include include local params with escaping" do + expect(subject[:q]).to include('qf=$author_qf') + expect(subject[:q]).to include('pf=\'you\\\'ll have \\" to escape this\'') + expect(subject[:q]).to include('pf2=$pf2_do_not_escape_or_quote') + end + end + + describe "mapping facet.field" do + let(:blacklight_config) do + Blacklight::Configuration.new do |config| + config.add_facet_field 'some_field' + config.add_facet_fields_to_solr_request! + end + end + + context "user provides a single facet.field" do + let(:user_params) { { "facet.field" => "additional_facet" } } + it "adds the field" do + expect(subject[:"facet.field"]).to include("additional_facet") + expect(subject[:"facet.field"]).to have(2).fields + end + end + + context "user provides a multiple facet.field" do + let(:user_params) { { "facet.field" => ["add_facet1", "add_facet2"] } } + it "adds the fields" do + expect(subject[:"facet.field"]).to include("add_facet1") + expect(subject[:"facet.field"]).to include("add_facet2") + expect(subject[:"facet.field"]).to have(3).fields + end + end + + context "user provides a multiple facets" do + let(:user_params) { { "facets" => ["add_facet1", "add_facet2"] } } + it "adds the fields" do + expect(subject[:"facet.field"]).to include("add_facet1") + expect(subject[:"facet.field"]).to include("add_facet2") + expect(subject[:"facet.field"]).to have(3).fields + end + end + end + end + + + describe "#facet_value_to_fq_string" do + + it "should use the raw handler for strings" do + expect(subject.send(:facet_value_to_fq_string, "facet_name", "my value")).to eq "{!raw f=facet_name}my value" + end + + it "should pass booleans through" do + expect(subject.send(:facet_value_to_fq_string, "facet_name", true)).to eq "facet_name:true" + end + + it "should pass boolean-like strings through" do + expect(subject.send(:facet_value_to_fq_string, "facet_name", "true")).to eq "facet_name:true" + end + + it "should pass integers through" do + expect(subject.send(:facet_value_to_fq_string, "facet_name", 1)).to eq "facet_name:1" + end + + it "should pass integer-like strings through" do + expect(subject.send(:facet_value_to_fq_string, "facet_name", "1")).to eq "facet_name:1" + end + + it "should pass floats through" do + expect(subject.send(:facet_value_to_fq_string, "facet_name", 1.11)).to eq "facet_name:1\\.11" + end + + it "should pass floats through" do + expect(subject.send(:facet_value_to_fq_string, "facet_name", "1.11")).to eq "facet_name:1\\.11" + end + + it "should escape negative integers" do + expect(subject.send(:facet_value_to_fq_string, "facet_name", -1)).to eq "facet_name:\\-1" + end + + it "should pass date-type fields through" do + allow(blacklight_config.facet_fields).to receive(:[]).with('facet_name').and_return(double(:date => true, :query => nil, :tag => nil)) + + expect(subject.send(:facet_value_to_fq_string, "facet_name", "2012-01-01")).to eq "facet_name:2012\\-01\\-01" + end + + it "should escape datetime-type fields" do + allow(blacklight_config.facet_fields).to receive(:[]).with('facet_name').and_return(double(:date => true, :query => nil, :tag => nil)) + + expect(subject.send(:facet_value_to_fq_string, "facet_name", "2003-04-09T00:00:00Z")).to eq "facet_name:2003\\-04\\-09T00\\:00\\:00Z" + end + + it "should format Date objects correctly" do + allow(blacklight_config.facet_fields).to receive(:[]).with('facet_name').and_return(double(:date => nil, :query => nil, :tag => nil)) + d = DateTime.parse("2003-04-09T00:00:00") + expect(subject.send(:facet_value_to_fq_string, "facet_name", d)).to eq "facet_name:2003\\-04\\-09T00\\:00\\:00Z" + end + + it "should handle range requests" do + expect(subject.send(:facet_value_to_fq_string, "facet_name", 1..5)).to eq "facet_name:[1 TO 5]" + end + + it "should add tag local parameters" do + allow(blacklight_config.facet_fields).to receive(:[]).with('facet_name').and_return(double(:query => nil, :tag => 'asdf', :date => nil)) + + expect(subject.send(:facet_value_to_fq_string, "facet_name", true)).to eq "{!tag=asdf}facet_name:true" + expect(subject.send(:facet_value_to_fq_string, "facet_name", "my value")).to eq "{!raw f=facet_name tag=asdf}my value" + end + end + + describe "#add_facet_fq_to_solr" do + it "converts a String fq into an Array" do + solr_parameters = {:fq => 'a string' } + + subject.add_facet_fq_to_solr(solr_parameters) + + expect(solr_parameters[:fq]).to be_a_kind_of Array + end + end + + describe "#add_solr_fields_to_query" do + let(:blacklight_config) do + config = Blacklight::Configuration.new do |config| + + config.add_index_field 'an_index_field', solr_params: { 'hl.alternativeField' => 'field_x'} + config.add_show_field 'a_show_field', solr_params: { 'hl.alternativeField' => 'field_y'} + config.add_field_configuration_to_solr_request! + end + end + + let(:solr_parameters) do + solr_parameters = Blacklight::Solr::Request.new + + subject.add_solr_fields_to_query(solr_parameters) + + solr_parameters + end + + it "should add any extra solr parameters from index and show fields" do + expect(solr_parameters[:'f.an_index_field.hl.alternativeField']).to eq "field_x" + expect(solr_parameters[:'f.a_show_field.hl.alternativeField']).to eq "field_y" + end + end + + describe "#add_facetting_to_solr" do + + let(:blacklight_config) do + config = Blacklight::Configuration.new + + config.add_facet_field 'test_field', :sort => 'count' + config.add_facet_field 'some-query', :query => {'x' => {:fq => 'some:query' }}, :ex => 'xyz' + config.add_facet_field 'some-pivot', :pivot => ['a','b'], :ex => 'xyz' + config.add_facet_field 'some-field', solr_params: { 'facet.mincount' => 15 } + config.add_facet_fields_to_solr_request! + + config + end + + let(:solr_parameters) do + solr_parameters = Blacklight::Solr::Request.new + + subject.add_facetting_to_solr(solr_parameters) + + solr_parameters + end + + it "should add sort parameters" do + expect(solr_parameters[:facet]).to be true + + expect(solr_parameters[:'facet.field']).to include('test_field') + expect(solr_parameters[:'f.test_field.facet.sort']).to eq 'count' + end + + it "should add facet exclusions" do + expect(solr_parameters[:'facet.query']).to include('{!ex=xyz}some:query') + expect(solr_parameters[:'facet.pivot']).to include('{!ex=xyz}a,b') + end + + it "should add any additional solr_params" do + expect(solr_parameters[:'f.some-field.facet.mincount']).to eq 15 + end + + describe ":include_in_request" do + let(:solr_parameters) do + solr_parameters = Blacklight::Solr::Request.new + subject.add_facetting_to_solr(solr_parameters) + solr_parameters + end + + it "should respect the include_in_request parameter" do + blacklight_config.add_facet_field 'yes_facet', include_in_request: true + blacklight_config.add_facet_field 'no_facet', include_in_request: false + + expect(solr_parameters[:'facet.field']).to include('yes_facet') + expect(solr_parameters[:'facet.field']).not_to include('no_facet') + end + + it "should default to including facets if add_facet_fields_to_solr_request! was called" do + blacklight_config.add_facet_field 'yes_facet' + blacklight_config.add_facet_field 'no_facet', include_in_request: false + blacklight_config.add_facet_fields_to_solr_request! + + expect(solr_parameters[:'facet.field']).to include('yes_facet') + expect(solr_parameters[:'facet.field']).not_to include('no_facet') + end + end + + describe ":add_facet_fields_to_solr_request!" do + + let(:blacklight_config) do + config = Blacklight::Configuration.new + config.add_facet_field 'yes_facet', include_in_request: true + config.add_facet_field 'no_facet', include_in_request: false + config.add_facet_field 'maybe_facet' + config.add_facet_field 'another_facet' + config + end + + let(:solr_parameters) do + solr_parameters = Blacklight::Solr::Request.new + subject.add_facetting_to_solr(solr_parameters) + solr_parameters + end + + it "should add facets to the solr request" do + blacklight_config.add_facet_fields_to_solr_request! + expect(solr_parameters[:'facet.field']).to match_array ['yes_facet', 'maybe_facet', 'another_facet'] + end + + it "should not override field-specific configuration by default" do + blacklight_config.add_facet_fields_to_solr_request! + expect(solr_parameters[:'facet.field']).to_not include 'no_facet' + end + + it "should allow white-listing facets" do + blacklight_config.add_facet_fields_to_solr_request! 'maybe_facet' + expect(solr_parameters[:'facet.field']).to include 'maybe_facet' + expect(solr_parameters[:'facet.field']).not_to include 'another_facet' + end + + it "should allow white-listed facets to override any field-specific include_in_request configuration" do + blacklight_config.add_facet_fields_to_solr_request! 'no_facet' + expect(solr_parameters[:'facet.field']).to include 'no_facet' + end + end + end + + describe "#with_tag_ex" do + it "should add an !ex local parameter if the facet configuration requests it" do + expect(subject.with_ex_local_param("xyz", "some-value")).to eq "{!ex=xyz}some-value" + end + + it "should not add an !ex local parameter if it isn't configured" do + mock_field = double() + expect(subject.with_ex_local_param(nil, "some-value")).to eq "some-value" + end + end +end