Skip to content

Commit

Permalink
Make sure that boolean operators in json queries are actually intended
Browse files Browse the repository at this point in the history
Extends the fix from #817 to JSON DSL queries as well
  • Loading branch information
sandbergja committed Aug 3, 2023
1 parent ffed47e commit 26f0c41
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 1 deletion.
21 changes: 20 additions & 1 deletion app/models/search_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,17 @@ class SearchBuilder < Blacklight::SearchBuilder
series_title_results pul_holdings html_facets
numismatics_facets numismatics_advanced]

# mutate the solr_parameters to remove words that are
# boolean operators, but not intended as such.
def cleanup_boolean_operators(solr_parameters)
return add_advanced_parse_q_to_solr(solr_parameters) if run_advanced_parse?(solr_parameters)
solr_parameters[:q] = cleaned_query(solr_parameters[:q])
return solr_parameters unless using_json_query_dsl(solr_parameters)

solr_parameters.dig('json', 'query', 'bool', 'must').map! do |search_element|
search_element[:edismax][:query] = cleaned_query(search_element[:edismax][:query])
search_element
end
end

# Blacklight uses Parslet https://rubygems.org/gems/parslet/versions/2.0.0 to parse the user query
Expand Down Expand Up @@ -50,7 +58,7 @@ def cleaned_query(query)
def run_advanced_parse?(solr_parameters)
blacklight_params[:q].blank? ||
!blacklight_params[:q].respond_to?(:to_str) ||
cleaned_query(solr_parameters[:q]) == solr_parameters[:q]
q_param_needs_boolean_cleanup(solr_parameters)
end

def facets_for_advanced_search_form(solr_p)
Expand Down Expand Up @@ -93,4 +101,15 @@ def search_parameters?
!blacklight_params[:q].nil? || blacklight_params[:f].present? ||
blacklight_params[:search_field] == 'advanced'
end

private

def q_param_needs_boolean_cleanup(solr_parameters)
solr_parameters[:q].present? &&
cleaned_query(solr_parameters[:q]) == solr_parameters[:q]
end

def using_json_query_dsl(solr_parameters)
solr_parameters.fetch('json', nil)&.fetch('query', nil)&.fetch('bool', nil)&.fetch('must', nil)&.present?
end
end
23 changes: 23 additions & 0 deletions spec/models/search_builder_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,15 @@
expect(solr_parameters[:q]).to eq('+solr +blacklight')
end
end
context 'when q contains an all-caps phrase that happens to contain a boolean operator' do
let(:solr_parameters) do
{ q: 'I AM NOT YOUR PRINCESS' }
end
it 'knows that the user did not mean NOT in the boolean sense' do
subject.cleanup_boolean_operators(solr_parameters)
expect(solr_parameters[:q]).to eq('I AM not YOUR PRINCESS')
end
end
end
context 'when using the JSON query DSL' do
let(:solr_parameters) do
Expand All @@ -99,6 +108,20 @@
subject.cleanup_boolean_operators(solr_parameters)
end.not_to change { solr_parameters['json'] }
end
context 'when user query contains an all-caps phrase that happens to contain a boolean operator' do
let(:solr_parameters) do
{ "json" =>
{ "query" =>
{ "bool" =>
{ "must" =>
[{ edismax: { query: "I AM NOT YOUR PRINCESS" } }] } } } }
end
it 'knows that the user did not mean NOT in the boolean sense' do
allow(subject).to receive(:blacklight_params).and_return({ q: 'I AM NOT YOUR PRINCESS' })
subject.cleanup_boolean_operators(solr_parameters)
expect(solr_parameters.dig('json', 'query', 'bool', 'must', 0, :edismax, :query)).to eq('I AM not YOUR PRINCESS')
end
end
end
end
end

0 comments on commit 26f0c41

Please sign in to comment.