Merge branch '326318-fix-blobs-n-plus-1' into 'master'

Fix blobs search N+1 [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!57826

app/presenters/search_service_presenter.rb

@@ -11,7 +11,9 @@ class SearchServicePresenter < Gitlab::View::Presenter::Delegated
     merge_requests: :with_web_entity_associations,
     epics: :with_web_entity_associations,
     notes: :with_web_entity_associations,
-    milestones: :with_web_entity_associations
+    milestones: :with_web_entity_associations,
+    commits: :with_web_entity_associations,
+    blobs: :with_web_entity_associations
   }.freeze
 
   SORT_ENABLED_SCOPES = %w(issues merge_requests).freeze

ee/changelogs/unreleased/326318-fix-blobs-n-plus-1.yml

+---
+title: Fix blobs search N+1
+merge_request: 57826
+author:
+type: performance

ee/lib/elastic/latest/git_class_proxy.rb

@@ -27,11 +27,11 @@ module Elastic
       end
 
       # @return [Kaminari::PaginatableArray]
-      def elastic_search_as_found_blob(query, page: 1, per: 20, options: {})
+      def elastic_search_as_found_blob(query, page: 1, per: 20, options: {}, preload_method: nil)
         # Highlight is required for parse_search_result to locate relevant line
         options = options.merge(highlight: true)
 
-        elastic_search_and_wrap(query, type: es_type, page: page, per: per, options: options) do |result, project|
+        elastic_search_and_wrap(query, type: es_type, page: page, per: per, options: options, preload_method: preload_method) do |result, project|
           ::Gitlab::Elastic::SearchResults.parse_search_result(result, project)
         end
       end

ee/lib/elastic/latest/git_instance_proxy.rb

@@ -22,10 +22,10 @@ module Elastic
       end
 
       # @return [Kaminari::PaginatableArray]
-      def elastic_search_as_found_blob(query, page: 1, per: 20, options: {})
+      def elastic_search_as_found_blob(query, page: 1, per: 20, options: {}, preload_method: nil)
         options = repository_specific_options(options)
 
-        self.class.elastic_search_as_found_blob(query, page: page, per: per, options: options)
+        self.class.elastic_search_as_found_blob(query, page: page, per: per, options: options, preload_method: preload_method)
       end
 
       def delete_index_for_commits_and_blobs(wiki: false)

ee/lib/gitlab/elastic/project_search_results.rb

@@ -17,7 +17,7 @@ module Gitlab
 
       private
 
-      def blobs(page: 1, per_page: DEFAULT_PER_PAGE, count_only: false)
+      def blobs(page: 1, per_page: DEFAULT_PER_PAGE, count_only: false, preload_method: nil)
         return Kaminari.paginate_array([]) unless Ability.allowed?(@current_user, :download_code, project)
         return Kaminari.paginate_array([]) if project.empty_repo? || query.blank?
         return Kaminari.paginate_array([]) unless root_ref?
@@ -27,7 +27,8 @@ module Gitlab
             query,
             page: (page || 1).to_i,
             per: per_page,
-            options: { count_only: count_only }
+            options: { count_only: count_only },
+            preload_method: preload_method
           )
         end
       end

ee/lib/gitlab/elastic/search_results.rb

@@ -38,7 +38,7 @@ module Gitlab
         when 'notes'
           eager_load(notes, page, per_page, preload_method, project: [:route, :namespace])
         when 'blobs'
-          blobs(page: page, per_page: per_page)
+          blobs(page: page, per_page: per_page, preload_method: preload_method)
         when 'wiki_blobs'
           wiki_blobs(page: page, per_page: per_page)
         when 'commits'
@@ -289,7 +289,7 @@ module Gitlab
         scope_results :notes, Note, count_only: count_only
       end
 
-      def blobs(page: 1, per_page: DEFAULT_PER_PAGE, count_only: false)
+      def blobs(page: 1, per_page: DEFAULT_PER_PAGE, count_only: false, preload_method: nil)
         return Kaminari.paginate_array([]) if query.blank?
 
         strong_memoize(memoize_key(:blobs, count_only: count_only)) do
@@ -297,7 +297,8 @@ module Gitlab
             query,
             page: (page || 1).to_i,
             per: per_page,
-            options: base_options.merge(count_only: count_only)
+            options: base_options.merge(count_only: count_only),
+            preload_method: preload_method
           )
         end
       end

ee/spec/features/search/elastic/global_search_spec.rb

@@ -82,6 +82,28 @@ RSpec.describe 'Global elastic search', :elastic, :sidekiq_inline do
 
       it_behaves_like 'an efficient database result'
     end
+
+    context 'searching code' do
+      let(:path) { search_path(search: '*', scope: 'blobs') }
+
+      it 'avoids N+1 database queries' do
+        project.repository.index_commits_and_blobs
+
+        ensure_elasticsearch_index!
+
+        control = ActiveRecord::QueryRecorder.new { visit path }
+        expect(page).to have_css('.search-results') # Confirm there are search results to prevent false positives
+
+        projects.each do |project|
+          project.repository.index_commits_and_blobs
+        end
+
+        ensure_elasticsearch_index!
+
+        expect { visit path }.not_to exceed_query_limit(control.count)
+        expect(page).to have_css('.search-results') # Confirm there are search results to prevent false positives
+      end
+    end
   end
 
   describe 'I search through the issues and I see pagination' do

ee/spec/lib/elastic/latest/git_instance_proxy_spec.rb

@@ -56,7 +56,8 @@ RSpec.describe Elastic::Latest::GitInstanceProxy do
       {
         page: 2,
         per: 30,
-        options: { foo: :bar }
+        options: { foo: :bar },
+        preload_method: nil
       }
     end