Skip to content

Commit

Permalink
Add field configuration for :include_in_request. If true, the field w…
Browse files Browse the repository at this point in the history
…ill be explicitly added to the request made to solr. Otherwise, Blacklight assumes the appropriate configuration is made in the solr config.
  • Loading branch information
cbeer committed Feb 17, 2014
1 parent 006be98 commit 89dbaf7
Show file tree
Hide file tree
Showing 3 changed files with 141 additions and 50 deletions.
46 changes: 37 additions & 9 deletions lib/blacklight/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

##
Expand Down
60 changes: 27 additions & 33 deletions lib/blacklight/solr_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -298,37 +298,28 @@ 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]) }
else
solr_parameters.append_facet_fields with_ex_local_param(facet.ex, 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'
Expand All @@ -347,17 +338,15 @@ 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(&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.each do |field_name, field|
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
Expand Down Expand Up @@ -496,7 +485,6 @@ def solr_facet_params(facet_field, user_params=params || {}, extra_controller_pa
# 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
Expand Down Expand Up @@ -650,4 +638,10 @@ def blacklight_solr
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)
end
end
85 changes: 77 additions & 8 deletions spec/lib/blacklight/solr_helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 89dbaf7

Please sign in to comment.