Pass limit when searching for commits

changelogs/unreleased/216908-pass-limit-and-offset-when-searching-for-commits.yml

+---
+title: Improve performance of commit search by limiting the number of results requested
+merge_request: 32260
+author:
+type: performance

lib/gitlab/project_search_results.rb

@@ -20,7 +20,7 @@ module Gitlab
       when 'wiki_blobs'
         paginated_wiki_blobs(wiki_blobs(limit: limit_up_to_page(page, per_page)), page, per_page)
       when 'commits'
-        Kaminari.paginate_array(commits).page(page).per(per_page)
+        paginated_commits(page, per_page)
       when 'users'
         users.page(page).per(per_page)
       else
@@ -86,6 +86,12 @@ module Gitlab
 
     private
 
+    def paginated_commits(page, per_page)
+      results = commits(limit: limit_up_to_page(page, per_page))
+
+      Kaminari.paginate_array(results).page(page).per(per_page)
+    end
+
     def paginated_blobs(blobs, page, per_page)
       results = Kaminari.paginate_array(blobs).page(page).per(per_page)
 
@@ -139,21 +145,21 @@ module Gitlab
     end
     # rubocop: enable CodeReuse/ActiveRecord
 
-    def commits
-      @commits ||= find_commits(query)
+    def commits(limit: count_limit)
+      @commits ||= find_commits(query, limit: limit)
     end
 
-    def find_commits(query)
+    def find_commits(query, limit:)
       return [] unless Ability.allowed?(@current_user, :download_code, @project)
 
-      commits = find_commits_by_message(query)
+      commits = find_commits_by_message(query, limit: limit)
       commit_by_sha = find_commit_by_sha(query)
       commits |= [commit_by_sha] if commit_by_sha
       commits
     end
 
-    def find_commits_by_message(query)
-      project.repository.find_commits_by_message(query)
+    def find_commits_by_message(query, limit:)
+      project.repository.find_commits_by_message(query, nil, nil, limit)
     end
 
     def find_commit_by_sha(query)

spec/lib/gitlab/project_search_results_spec.rb

@@ -452,6 +452,54 @@ describe Gitlab::ProjectSearchResults do
   end
 
   describe 'commit search' do
+    context 'pagination' do
+      let(:project) { create(:project, :public, :repository) }
+
+      it 'returns the correct results for each page' do
+        expect(results_page(1)).to contain_exactly(commit('b83d6e391c22777fca1ed3012fce84f633d7fed0'))
+
+        expect(results_page(2)).to contain_exactly(commit('498214de67004b1da3d820901307bed2a68a8ef6'))
+
+        expect(results_page(3)).to contain_exactly(commit('1b12f15a11fc6e62177bef08f47bc7b5ce50b141'))
+      end
+
+      it 'returns the correct number of pages' do
+        expect(results_page(1).total_pages).to eq(project.repository.commit_count)
+      end
+
+      context 'limiting requested commits' do
+        context 'on page 1' do
+          it "limits to #{described_class::COUNT_LIMIT}" do
+            expect(project.repository)
+              .to receive(:find_commits_by_message)
+              .with(anything, anything, anything, described_class::COUNT_LIMIT)
+              .and_call_original
+
+            results_page(1)
+          end
+        end
+
+        context 'on subsequent pages' do
+          it "limits to #{described_class::COUNT_LIMIT} plus page offset" do
+            expect(project.repository)
+              .to receive(:find_commits_by_message)
+              .with(anything, anything, anything, described_class::COUNT_LIMIT + 1)
+              .and_call_original
+
+            results_page(2)
+          end
+        end
+      end
+
+      def results_page(page)
+        described_class.new(user, project, '.').objects('commits', per_page: 1, page: page)
+      end
+
+      def commit(hash)
+        project.repository.commit(hash)
+      end
+    end
+
     context 'by commit message' do
       let(:project) { create(:project, :public, :repository) }
       let(:commit) { project.repository.commit('59e29889be61e6e0e5e223bfa9ac2721d31605b8') }