From 8ca93bead061f499a9ba3727113af99a431a62cd Mon Sep 17 00:00:00 2001 From: Jane Sandberg Date: Thu, 2 Jan 2025 14:05:09 -0800 Subject: [PATCH] Simplify query for facet more link When we use the "more" link on a facet, we send a solr query to gather the facet values. Prior to this commit, we also sent some extra parameters for other facets. For example, we always sent the facet.query params for the publication date and cataloged date, even if we were looking at a totally different facet, say, language. Working locally with a branch for #4025 and catalog-staging, the median speed for a suggestion query goes down from 2192 ms to 1258 ms. Helps with #4025, so that the feature is performant enough to be useful --- app/models/search_builder.rb | 21 ++++++++- spec/models/search_builder_spec.rb | 69 ++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 1 deletion(-) diff --git a/app/models/search_builder.rb b/app/models/search_builder.rb index 0751ede04..4c4ae2ce7 100644 --- a/app/models/search_builder.rb +++ b/app/models/search_builder.rb @@ -12,7 +12,7 @@ class SearchBuilder < Blacklight::SearchBuilder only_home_facets prepare_left_anchor_search series_title_results pul_holdings html_facets numismatics_facets numismatics_advanced - adjust_mm] + adjust_mm remove_unneeded_facets] # mutate the solr_parameters to remove words that are # boolean operators, but not intended as such. @@ -107,6 +107,18 @@ def adjust_mm(solr_parameters) solr_parameters['mm'] = 0 end + # When the user is viewing the values of a specific facet + # by clicking the "more" link in a facet, solr doesn't + # need to perform expensive calculations related to other + # facets that the user is not displaying + # :reek:FeatureEnvy + def remove_unneeded_facets(solr_parameters) + return unless facet + remove_unneeded_stats(solr_parameters) + solr_parameters.delete('facet.pivot') unless solr_parameters['facet.pivot']&.split(',')&.include? facet + solr_parameters.delete('facet.query') unless solr_parameters['facet.query']&.any? { |query| query.partition(':').first == facet } + end + private def search_query_present? @@ -137,4 +149,11 @@ def add_edismax(advanced_fields:) blacklight_config.search_fields[field]['clause_params'] = { edismax: } end end + + # :reek:FeatureEnvy + def remove_unneeded_stats(solr_parameters) + return if solr_parameters['stats.field'].to_a.include? facet + solr_parameters.delete('stats') + solr_parameters.delete('stats.field') + end end diff --git a/spec/models/search_builder_spec.rb b/spec/models/search_builder_spec.rb index 019563af8..c72499dd7 100644 --- a/spec/models/search_builder_spec.rb +++ b/spec/models/search_builder_spec.rb @@ -169,4 +169,73 @@ end end end + + describe '#remove_unneeded_facets' do + let(:blacklight_config) do + Blacklight::Configuration.new do |config| + config.add_facet_field 'pub_date_start_sort', label: 'Publication year', single: true, range: { + num_segments: 10, + assumed_boundaries: [1100, Time.zone.now.year + 1], + segments: true + } + config.add_facet_field 'classification_pivot_field', label: 'Classification', pivot: %w[lc_1letter_facet lc_rest_facet] + config.add_facet_field 'recently_added_facet', label: 'Recently added', home: true, query: { + weeks_one: { label: 'Within 1 week', fq: 'cataloged_tdt:[NOW/DAY-7DAYS TO NOW/DAY+1DAY]' }, + weeks_two: { label: 'Within 2 weeks', fq: 'cataloged_tdt:[NOW/DAY-14DAYS TO NOW/DAY+1DAY]' } + } + end + end + context 'when viewing a facet that is not a pivot or stats facet' do + before { search_builder.facet('language_facet') } + it 'removes expensive stats configuration' do + solr_parameters = { 'stats' => true, 'stats.field' => ['pub_date_start_sort'] } + search_builder.remove_unneeded_facets(solr_parameters) + expect(solr_parameters.keys).not_to include 'stats' + expect(solr_parameters.keys).not_to include 'stats.field' + end + it 'removes expensive facet.pivot configuration' do + solr_parameters = { 'facet.pivot' => 'lc_1letter_facet,lc_rest_facet' } + search_builder.remove_unneeded_facets(solr_parameters) + expect(solr_parameters.keys).not_to include 'facet.pivot' + end + it 'removes expensive facet.query configuration' do + solr_parameters = { 'facet.query' => ['cataloged_tdt:[NOW/DAY-7DAYS+TO+NOW/DAY+1DAY]'] } + search_builder.remove_unneeded_facets(solr_parameters) + expect(solr_parameters.keys).not_to include 'facet.query' + end + end + context 'when viewing a stats facet' do + before { search_builder.facet('pub_date_start_sort') } + it 'keeps the stats configuration' do + solr_parameters = { 'stats' => true, 'stats.field' => ['pub_date_start_sort'] } + search_builder.remove_unneeded_facets(solr_parameters) + expect(solr_parameters['stats']).to be true + expect(solr_parameters['stats.field']).to eq ['pub_date_start_sort'] + end + end + context 'when viewing a pivot facet' do + before { search_builder.facet('lc_1letter_facet') } + it 'keeps the facet.pivot configuration' do + solr_parameters = { 'facet.pivot' => 'lc_1letter_facet,lc_rest_facet' } + search_builder.remove_unneeded_facets(solr_parameters) + expect(solr_parameters['facet.pivot']).to eq 'lc_1letter_facet,lc_rest_facet' + end + end + context 'when viewing a facet that needs a facet.query' do + before { search_builder.facet('cataloged_tdt') } + it 'keeps the facet.query configuration' do + solr_parameters = { 'facet.query' => ['cataloged_tdt:[NOW/DAY-7DAYS+TO+NOW/DAY+1DAY]'] } + search_builder.remove_unneeded_facets(solr_parameters) + expect(solr_parameters['facet.query']).to eq ['cataloged_tdt:[NOW/DAY-7DAYS+TO+NOW/DAY+1DAY]'] + end + end + context 'when we are not doing a facet view' do + it 'does not modify the solr_parameters' do + solr_parameters = { 'stats' => true, 'stats.field' => ['pub_date_start_sort'] } + expect do + search_builder.remove_unneeded_facets(solr_parameters) + end.not_to change { solr_parameters } + end + end + end end