summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/finders/notes_finder.rb12
-rw-r--r--app/views/search/_category.html.haml8
-rw-r--r--changelogs/unreleased/jprovazn-scoped-limit.yml6
-rw-r--r--lib/gitlab/project_search_results.rb29
-rw-r--r--lib/gitlab/search_results.rb16
-rw-r--r--spec/finders/notes_finder_spec.rb12
-rw-r--r--spec/lib/gitlab/project_search_results_spec.rb41
-rw-r--r--spec/lib/gitlab/search_results_spec.rb36
8 files changed, 97 insertions, 63 deletions
diff --git a/app/finders/notes_finder.rb b/app/finders/notes_finder.rb
index 33ee1e975b9..35f4ff2f62f 100644
--- a/app/finders/notes_finder.rb
+++ b/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) }
diff --git a/app/views/search/_category.html.haml b/app/views/search/_category.html.haml
index 915e648a5d3..7d43fd61081 100644
--- a/app/views/search/_category.html.haml
+++ b/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
diff --git a/changelogs/unreleased/jprovazn-scoped-limit.yml b/changelogs/unreleased/jprovazn-scoped-limit.yml
new file mode 100644
index 00000000000..45724bb3479
--- /dev/null
+++ b/changelogs/unreleased/jprovazn-scoped-limit.yml
@@ -0,0 +1,6 @@
+---
+title: Optimize search queries on the search page by setting a limit for matching
+ records in project scope
+merge_request:
+author:
+type: performance
diff --git a/lib/gitlab/project_search_results.rb b/lib/gitlab/project_search_results.rb
index cf0935dbd9a..29277ec6481 100644
--- a/lib/gitlab/project_search_results.rb
+++ b/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
diff --git a/lib/gitlab/search_results.rb b/lib/gitlab/search_results.rb
index 781783f4d97..757ef71b95a 100644
--- a/lib/gitlab/search_results.rb
+++ b/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
diff --git a/spec/finders/notes_finder_spec.rb b/spec/finders/notes_finder_spec.rb
index 7b43494eea2..f1ae2c7ab65 100644
--- a/spec/finders/notes_finder_spec.rb
+++ b/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 }
diff --git a/spec/lib/gitlab/project_search_results_spec.rb b/spec/lib/gitlab/project_search_results_spec.rb
index d8250e4b4c6..c46bb8edebf 100644
--- a/spec/lib/gitlab/project_search_results_spec.rb
+++ b/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:
diff --git a/spec/lib/gitlab/search_results_spec.rb b/spec/lib/gitlab/search_results_spec.rb
index 9dbab95f70e..87288baedb0 100644
--- a/spec/lib/gitlab/search_results_spec.rb
+++ b/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
View Lorry Controller Status