From 98c62abf0a656b515b55d909c9d2148c06bf5e07 Mon Sep 17 00:00:00 2001 From: Chris Beer Date: Mon, 17 Feb 2014 10:44:29 -0800 Subject: [PATCH] Add field configuration for :include_in_request. If true, the field will be explicitly added to the request made to solr. Otherwise, Blacklight assumes the appropriate configuration is made in the solr config. --- lib/blacklight/configuration.rb | 46 ++++++++++--- lib/blacklight/solr_helper.rb | 55 +++++++--------- spec/lib/blacklight/solr_helper_spec.rb | 85 ++++++++++++++++++++++--- 3 files changed, 137 insertions(+), 49 deletions(-) diff --git a/lib/blacklight/configuration.rb b/lib/blacklight/configuration.rb index c40c1f6199..69c9eb40bb 100644 --- a/lib/blacklight/configuration.rb +++ b/lib/blacklight/configuration.rb @@ -31,8 +31,15 @@ def default_values :max_per_page => 100, :per_page => [10,20,50,100], :search_history_window => Blacklight::Catalog::SearchHistoryWindow, - :add_facet_fields_to_solr_request => false, - :add_field_configuration_to_solr_request => false, + ## deprecated; use add_facet_field :include_in_request instead; + # if this is configured true, all facets will be included in the solr request + # unless explicitly disabled. + :add_facet_fields_to_solr_request => false, + + ## deprecated; use add_index_field :include_in_request instead; + # if this is configured true, all show and index will be included in the solr request + # unless explicitly disabled. + :add_field_configuration_to_solr_request => false, # deprecated :http_method => :get } end @@ -91,19 +98,40 @@ def default_sort_field end # Add any configured facet fields to the default solr parameters hash - def add_facet_fields_to_solr_request! - self.add_facet_fields_to_solr_request = true + def add_facet_fields_to_solr_request! *fields + if fields.empty? + self.add_facet_fields_to_solr_request = true + else + facet_fields.slice(*fields).each do |k,v| + v.include_in_request = true + end + end end # Add any configured facet fields to the default solr parameters hash - def add_field_configuration_to_solr_request! - self.add_field_configuration_to_solr_request = true + def add_field_configuration_to_solr_request! *fields + if fields.empty? + self.add_field_configuration_to_solr_request = true + else + index_fields.slice(*fields).each do |k,v| + v.include_in_request = true + end + + show_fields.slice(*fields).each do |k,v| + v.include_in_request = true + end + facet_fields.slice(*fields).each do |k,v| + v.include_in_request = true + end + end end + ## + # Deprecated. def facet_fields_to_add_to_solr - return facet_fields.reject { |k,v| v[:query] || v[:pivot] }.map { |k,v| v.field } if self.add_facet_fields_to_solr_request - - [] + facet_fields.select { |k,v| v.include_in_request } + .reject { |k,v| v[:query] || v[:pivot] } + .map { |k,v| v.field } end ## diff --git a/lib/blacklight/solr_helper.rb b/lib/blacklight/solr_helper.rb index 71c31d6f57..0372ab49fc 100644 --- a/lib/blacklight/solr_helper.rb +++ b/lib/blacklight/solr_helper.rb @@ -298,37 +298,30 @@ def add_facetting_to_solr(solr_parameters, user_params) 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]) } + when facet.ex + solr_parameters.append_facet_fields with_ex_local_param(facet.ex, facet.field) + else + solr_parameters.append_facet_fields facet.field + end - if blacklight_config.add_facet_fields_to_solr_request - solr_parameters[:facet] = true - solr_parameters.append_facet_fields blacklight_config.facet_fields_to_add_to_solr - end - - blacklight_config.facet_fields.each do |field_name, facet| - - if blacklight_config.add_facet_fields_to_solr_request - 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]) } - - when facet.ex - if idx = solr_parameters[:'facet.field'].index(facet.field) - solr_parameters[:'facet.field'][idx] = with_ex_local_param(facet.ex, solr_parameters[:'facet.field'][idx]) - end - end - - if facet.sort - solr_parameters[:"f.#{facet.field}.facet.sort"] = facet.sort - 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 + 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' @@ -347,9 +340,7 @@ def with_ex_local_param(ex, value) end def add_solr_fields_to_query solr_parameters, user_parameters - return unless blacklight_config.add_field_configuration_to_solr_request - - blacklight_config.show_fields.each do |field_name, field| + blacklight_config.show_fields.select { |field_name,field| field.solr_params || field.include_in_request || (field.include_in_request.nil? && blacklight_config.add_field_configuration_to_solr_request) }.each do |field_name, field| if field.solr_params field.solr_params.each do |k, v| solr_parameters[:"f.#{field.field}.#{k}"] = v @@ -357,7 +348,7 @@ def add_solr_fields_to_query solr_parameters, user_parameters end end - blacklight_config.index_fields.each do |field_name, field| + blacklight_config.index_fields.select { |field_name,field| field.solr_params || field.include_in_request || (field.include_in_request.nil? && blacklight_config.add_field_configuration_to_solr_request) }.each do |field_name, field| if field.highlight solr_parameters[:hl] = true solr_parameters.append_highlight_field field.field diff --git a/spec/lib/blacklight/solr_helper_spec.rb b/spec/lib/blacklight/solr_helper_spec.rb index d90a56531f..6587d48d5a 100644 --- a/spec/lib/blacklight/solr_helper_spec.rb +++ b/spec/lib/blacklight/solr_helper_spec.rb @@ -64,7 +64,7 @@ def params {} end before do - @produced_params = self.solr_search_params + @produced_params = self.solr_search_params.with_indifferent_access end it 'should not have a q param' do expect(@produced_params[:q]).to be_nil @@ -75,7 +75,7 @@ def params end it 'should have default facet fields' do # remove local params from the facet.field - expect(@produced_params[:"facet.field"].map { |x| x.gsub(/\{![^}]+\}/, '') }).to eq blacklight_config.facet_fields_to_add_to_solr + expect(@produced_params[:"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 @@ -329,6 +329,74 @@ def params expect(solr_parameters[:'f.some-field.facet.mincount']).to eq 15 end + + describe ":include_in_request" do + let(:blacklight_config) do + Blacklight::Configuration.new + end + + let(:solr_parameters) do + Blacklight::Solr::Request.new + 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 + add_facetting_to_solr(solr_parameters, {}) + 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! + add_facetting_to_solr(solr_parameters, {}) + 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 + Blacklight::Solr::Request.new + end + + it "should add facets to the solr request" do + blacklight_config.add_facet_fields_to_solr_request! + add_facetting_to_solr(solr_parameters, {}) + 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! + add_facetting_to_solr(solr_parameters, {}) + 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' + add_facetting_to_solr(solr_parameters, {}) + 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' + add_facetting_to_solr(solr_parameters, {}) + expect(solr_parameters[:'facet.field']).to include 'no_facet' + end + end end describe "with a complex parameter environment" do @@ -525,9 +593,8 @@ def facet_list_limit end it "uses the field-specific sort" do - @facet_field = 'format_ordered' - solr_params = solr_facet_params(@facet_field) - expect(solr_params[:"f.#{@facet_field}.facet.sort"]).to eq :count + solr_params = 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 @@ -778,9 +845,11 @@ def grouped_key_for_results it 'should have more than one facet' do expect(@facets).to have_at_least(1).facet end - it 'should have all facets specified in initializer' do - blacklight_config.facet_fields_to_add_to_solr.each do |field| - expect(@facets.find {|f| f.name == field}).not_to be_nil + it 'should have all facets specified in initializer' do + fields = blacklight_config.facet_fields.reject { |k,v| v.query || v.pivot } + expect(@facets.map { |f| f.name }).to match_array fields.map { |k, v| v.field } + fields.each do |key, field| + expect(@facets.find {|f| f.name == field.field}).not_to be_nil end end it 'should have at least one value for each facet' do