Merge branch 'ee-jprovazn-scoped-limit' into 'master'

[EE] Use limited count queries also for scoped searches

See merge request gitlab-org/gitlab-ee!4782

app/finders/notes_finder.rb

@@ -48,11 +48,23 @@ class NotesFinder
   def init_collection
     if target
       notes_on_target
+    elsif target_type
+      notes_of_target_type
     else
       notes_of_any_type
     end
   end
 
+  def notes_of_target_type
+    notes = notes_for_type(target_type)
+
+    search(notes)
+  end
+
+  def target_type
+    @params[:target_type]
+  end
+
   def notes_of_any_type
     types = %w(commit issue merge_request snippet)
     note_relations = types.map { |t| notes_for_type(t) }

app/views/search/_category.html.haml

@@ -14,25 +14,25 @@
           = link_to search_filter_path(scope: 'issues') do
             Issues
             %span.badge
-              = @search_results.issues_count
+              = limited_count(@search_results.limited_issues_count)
       - if project_search_tabs?(:merge_requests)
         %li{ class: active_when(@scope == 'merge_requests') }
           = link_to search_filter_path(scope: 'merge_requests') do
             Merge requests
             %span.badge
-              = @search_results.merge_requests_count
+              = limited_count(@search_results.limited_merge_requests_count)
       - if project_search_tabs?(:milestones)
         %li{ class: active_when(@scope == 'milestones') }
           = link_to search_filter_path(scope: 'milestones') do
             Milestones
             %span.badge
-              = @search_results.milestones_count
+              = limited_count(@search_results.limited_milestones_count)
       - if project_search_tabs?(:notes)
         %li{ class: active_when(@scope == 'notes') }
           = link_to search_filter_path(scope: 'notes') do
             Comments
             %span.badge
-              = @search_results.notes_count
+              = limited_count(@search_results.limited_notes_count)
       - if project_search_tabs?(:wiki)
         %li{ class: active_when(@scope == 'wiki_blobs') }
           = link_to search_filter_path(scope: 'wiki_blobs') do

changelogs/unreleased/jprovazn-scoped-limit.yml

+---
+title: Optimize search queries on the search page by setting a limit for matching
+  records in project scope
+merge_request:
+author:
+type: performance

ee/lib/gitlab/elastic/project_search_results.rb

@@ -36,6 +36,7 @@ module Gitlab
       def notes_count
         @notes_count ||= notes.total_count
       end
+      alias_method :limited_notes_count, :notes_count
 
       def wiki_blobs_count
         @wiki_blobs_count ||= wiki_blobs.total_count

lib/gitlab/project_search_results.rb

@@ -29,8 +29,18 @@ module Gitlab
       @blobs_count ||= blobs.count
     end
 
-    def notes_count
-      @notes_count ||= notes.count
+    def limited_notes_count
+      return @limited_notes_count if defined?(@limited_notes_count)
+
+      types = %w(issue merge_request commit snippet)
+      @limited_notes_count = 0
+
+      types.each do |type|
+        @limited_notes_count += notes_finder(type).limit(count_limit).count
+        break if @limited_notes_count >= count_limit
+      end
+
+      @limited_notes_count
     end
 
     def wiki_blobs_count
@@ -72,11 +82,12 @@ module Gitlab
     end
 
     def single_commit_result?
-      commits_count == 1 && total_result_count == 1
-    end
+      return false if commits_count != 1
 
-    def total_result_count
-      issues_count + merge_requests_count + milestones_count + notes_count + blobs_count + wiki_blobs_count + commits_count
+      counts = %i(limited_milestones_count limited_notes_count
+                  limited_merge_requests_count limited_issues_count
+                  blobs_count wiki_blobs_count)
+      counts.all? { |count_method| public_send(count_method).zero? } # rubocop:disable GitlabSecurity/PublicSend
     end
 
     private
@@ -106,7 +117,11 @@ module Gitlab
     end
 
     def notes
-      @notes ||= NotesFinder.new(project, @current_user, search: query).execute.user.order('updated_at DESC')
+      @notes ||= notes_finder(nil)
+    end
+
+    def notes_finder(type)
+      NotesFinder.new(project, @current_user, search: query, target_type: type).execute.user.order('updated_at DESC')
     end
 
     def commits

lib/gitlab/search_results.rb

@@ -62,22 +62,6 @@ module Gitlab
       without_count ? collection.without_count : collection
     end
 
-    def projects_count
-      @projects_count ||= projects.count
-    end
-
-    def issues_count
-      @issues_count ||= issues.count
-    end
-
-    def merge_requests_count
-      @merge_requests_count ||= merge_requests.count
-    end
-
-    def milestones_count
-      @milestones_count ||= milestones.count
-    end
-
     def limited_projects_count
       @limited_projects_count ||= projects.limit(count_limit).count
     end

spec/finders/notes_finder_spec.rb

@@ -75,6 +75,18 @@ describe NotesFinder do
       end
     end
 
+    context 'for target type' do
+      let(:project) { create(:project, :repository) }
+      let!(:note1) { create :note_on_issue, project: project }
+      let!(:note2) { create :note_on_commit, project: project }
+
+      it 'finds only notes for the selected type' do
+        notes = described_class.new(project, user, target_type: 'issue').execute
+
+        expect(notes).to eq([note1])
+      end
+    end
+
     context 'for target' do
       let(:project) { create(:project, :repository) }
       let(:note1) { create :note_on_commit, project: project }

spec/lib/gitlab/project_search_results_spec.rb

@@ -217,7 +217,7 @@ describe Gitlab::ProjectSearchResults do
       expect(issues).to include issue
       expect(issues).not_to include security_issue_1
       expect(issues).not_to include security_issue_2
-      expect(results.issues_count).to eq 1
+      expect(results.limited_issues_count).to eq 1
     end
 
     it 'does not list project confidential issues for project members with guest role' do
@@ -229,7 +229,7 @@ describe Gitlab::ProjectSearchResults do
       expect(issues).to include issue
       expect(issues).not_to include security_issue_1
       expect(issues).not_to include security_issue_2
-      expect(results.issues_count).to eq 1
+      expect(results.limited_issues_count).to eq 1
     end
 
     it 'lists project confidential issues for author' do
@@ -239,7 +239,7 @@ describe Gitlab::ProjectSearchResults do
       expect(issues).to include issue
       expect(issues).to include security_issue_1
       expect(issues).not_to include security_issue_2
-      expect(results.issues_count).to eq 2
+      expect(results.limited_issues_count).to eq 2
     end
 
     it 'lists project confidential issues for assignee' do
@@ -249,7 +249,7 @@ describe Gitlab::ProjectSearchResults do
       expect(issues).to include issue
       expect(issues).not_to include security_issue_1
       expect(issues).to include security_issue_2
-      expect(results.issues_count).to eq 2
+      expect(results.limited_issues_count).to eq 2
     end
 
     it 'lists project confidential issues for project members' do
@@ -261,7 +261,7 @@ describe Gitlab::ProjectSearchResults do
       expect(issues).to include issue
       expect(issues).to include security_issue_1
       expect(issues).to include security_issue_2
-      expect(results.issues_count).to eq 3
+      expect(results.limited_issues_count).to eq 3
     end
 
     it 'lists all project issues for admin' do
@@ -271,7 +271,7 @@ describe Gitlab::ProjectSearchResults do
       expect(issues).to include issue
       expect(issues).to include security_issue_1
       expect(issues).to include security_issue_2
-      expect(results.issues_count).to eq 3
+      expect(results.limited_issues_count).to eq 3
     end
   end
 
@@ -304,6 +304,35 @@ describe Gitlab::ProjectSearchResults do
     end
   end
 
+  describe '#limited_notes_count' do
+    let(:project) { create(:project, :public) }
+    let(:note) { create(:note_on_issue, project: project) }
+    let(:results) { described_class.new(user, project, note.note) }
+
+    context 'when count_limit is lower than total amount' do
+      before do
+        allow(results).to receive(:count_limit).and_return(1)
+      end
+
+      it 'calls note finder once to get the limited amount of notes' do
+        expect(results).to receive(:notes_finder).once.and_call_original
+        expect(results.limited_notes_count).to eq(1)
+      end
+    end
+
+    context 'when count_limit is higher than total amount' do
+      it 'calls note finder multiple times to get the limited amount of notes' do
+        project = create(:project, :public)
+        note = create(:note_on_issue, project: project)
+
+        results = described_class.new(user, project, note.note)
+
+        expect(results).to receive(:notes_finder).exactly(4).times.and_call_original
+        expect(results.limited_notes_count).to eq(1)
+      end
+    end
+  end
+
   # Examples for commit access level test
   #
   # params:

spec/lib/gitlab/search_results_spec.rb

@@ -29,30 +29,6 @@ describe Gitlab::SearchResults do
       end
     end
 
-    describe '#projects_count' do
-      it 'returns the total amount of projects' do
-        expect(results.projects_count).to eq(1)
-      end
-    end
-
-    describe '#issues_count' do
-      it 'returns the total amount of issues' do
-        expect(results.issues_count).to eq(1)
-      end
-    end
-
-    describe '#merge_requests_count' do
-      it 'returns the total amount of merge requests' do
-        expect(results.merge_requests_count).to eq(1)
-      end
-    end
-
-    describe '#milestones_count' do
-      it 'returns the total amount of milestones' do
-        expect(results.milestones_count).to eq(1)
-      end
-    end
-
     context "when count_limit is lower than total amount" do
       before do
         allow(results).to receive(:count_limit).and_return(1)
@@ -183,7 +159,7 @@ describe Gitlab::SearchResults do
       expect(issues).not_to include security_issue_3
       expect(issues).not_to include security_issue_4
       expect(issues).not_to include security_issue_5
-      expect(results.issues_count).to eq 1
+      expect(results.limited_issues_count).to eq 1
     end
 
     it 'does not list confidential issues for project members with guest role' do
@@ -199,7 +175,7 @@ describe Gitlab::SearchResults do
       expect(issues).not_to include security_issue_3
       expect(issues).not_to include security_issue_4
       expect(issues).not_to include security_issue_5
-      expect(results.issues_count).to eq 1
+      expect(results.limited_issues_count).to eq 1
     end
 
     it 'lists confidential issues for author' do
@@ -212,7 +188,7 @@ describe Gitlab::SearchResults do
       expect(issues).to include security_issue_3
       expect(issues).not_to include security_issue_4
       expect(issues).not_to include security_issue_5
-      expect(results.issues_count).to eq 3
+      expect(results.limited_issues_count).to eq 3
     end
 
     it 'lists confidential issues for assignee' do
@@ -225,7 +201,7 @@ describe Gitlab::SearchResults do
       expect(issues).not_to include security_issue_3
       expect(issues).to include security_issue_4
       expect(issues).not_to include security_issue_5
-      expect(results.issues_count).to eq 3
+      expect(results.limited_issues_count).to eq 3
     end
 
     it 'lists confidential issues for project members' do
@@ -241,7 +217,7 @@ describe Gitlab::SearchResults do
       expect(issues).to include security_issue_3
       expect(issues).not_to include security_issue_4
       expect(issues).not_to include security_issue_5
-      expect(results.issues_count).to eq 4
+      expect(results.limited_issues_count).to eq 4
     end
 
     it 'lists all issues for admin' do
@@ -254,7 +230,7 @@ describe Gitlab::SearchResults do
       expect(issues).to include security_issue_3
       expect(issues).to include security_issue_4
       expect(issues).not_to include security_issue_5
-      expect(results.issues_count).to eq 5
+      expect(results.limited_issues_count).to eq 5
     end
   end