Commit 4a7e0821 authored by Sean McGivern's avatar Sean McGivern

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

Use limited count queries also for scoped searches

Closes #43242

See merge request gitlab-org/gitlab-ce!17452
parents f56de375 741caf93
...@@ -48,11 +48,23 @@ class NotesFinder ...@@ -48,11 +48,23 @@ class NotesFinder
def init_collection def init_collection
if target if target
notes_on_target notes_on_target
elsif target_type
notes_of_target_type
else else
notes_of_any_type notes_of_any_type
end end
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 def notes_of_any_type
types = %w(commit issue merge_request snippet) types = %w(commit issue merge_request snippet)
note_relations = types.map { |t| notes_for_type(t) } note_relations = types.map { |t| notes_for_type(t) }
......
...@@ -14,25 +14,25 @@ ...@@ -14,25 +14,25 @@
= link_to search_filter_path(scope: 'issues') do = link_to search_filter_path(scope: 'issues') do
Issues Issues
%span.badge %span.badge
= @search_results.issues_count = limited_count(@search_results.limited_issues_count)
- if project_search_tabs?(:merge_requests) - if project_search_tabs?(:merge_requests)
%li{ class: active_when(@scope == 'merge_requests') } %li{ class: active_when(@scope == 'merge_requests') }
= link_to search_filter_path(scope: 'merge_requests') do = link_to search_filter_path(scope: 'merge_requests') do
Merge requests Merge requests
%span.badge %span.badge
= @search_results.merge_requests_count = limited_count(@search_results.limited_merge_requests_count)
- if project_search_tabs?(:milestones) - if project_search_tabs?(:milestones)
%li{ class: active_when(@scope == 'milestones') } %li{ class: active_when(@scope == 'milestones') }
= link_to search_filter_path(scope: 'milestones') do = link_to search_filter_path(scope: 'milestones') do
Milestones Milestones
%span.badge %span.badge
= @search_results.milestones_count = limited_count(@search_results.limited_milestones_count)
- if project_search_tabs?(:notes) - if project_search_tabs?(:notes)
%li{ class: active_when(@scope == 'notes') } %li{ class: active_when(@scope == 'notes') }
= link_to search_filter_path(scope: 'notes') do = link_to search_filter_path(scope: 'notes') do
Comments Comments
%span.badge %span.badge
= @search_results.notes_count = limited_count(@search_results.limited_notes_count)
- if project_search_tabs?(:wiki) - if project_search_tabs?(:wiki)
%li{ class: active_when(@scope == 'wiki_blobs') } %li{ class: active_when(@scope == 'wiki_blobs') }
= link_to search_filter_path(scope: 'wiki_blobs') do = link_to search_filter_path(scope: 'wiki_blobs') do
......
---
title: Optimize search queries on the search page by setting a limit for matching
records in project scope
merge_request:
author:
type: performance
...@@ -29,8 +29,18 @@ module Gitlab ...@@ -29,8 +29,18 @@ module Gitlab
@blobs_count ||= blobs.count @blobs_count ||= blobs.count
end end
def notes_count def limited_notes_count
@notes_count ||= 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 end
def wiki_blobs_count def wiki_blobs_count
...@@ -72,11 +82,12 @@ module Gitlab ...@@ -72,11 +82,12 @@ module Gitlab
end end
def single_commit_result? def single_commit_result?
commits_count == 1 && total_result_count == 1 return false if commits_count != 1
end
def total_result_count counts = %i(limited_milestones_count limited_notes_count
issues_count + merge_requests_count + milestones_count + notes_count + blobs_count + wiki_blobs_count + commits_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 end
private private
...@@ -106,7 +117,11 @@ module Gitlab ...@@ -106,7 +117,11 @@ module Gitlab
end end
def notes 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 end
def commits def commits
......
...@@ -62,22 +62,6 @@ module Gitlab ...@@ -62,22 +62,6 @@ module Gitlab
without_count ? collection.without_count : collection without_count ? collection.without_count : collection
end 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 def limited_projects_count
@limited_projects_count ||= projects.limit(count_limit).count @limited_projects_count ||= projects.limit(count_limit).count
end end
......
...@@ -75,6 +75,18 @@ describe NotesFinder do ...@@ -75,6 +75,18 @@ describe NotesFinder do
end end
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 context 'for target' do
let(:project) { create(:project, :repository) } let(:project) { create(:project, :repository) }
let(:note1) { create :note_on_commit, project: project } let(:note1) { create :note_on_commit, project: project }
......
...@@ -217,7 +217,7 @@ describe Gitlab::ProjectSearchResults do ...@@ -217,7 +217,7 @@ describe Gitlab::ProjectSearchResults do
expect(issues).to include issue expect(issues).to include issue
expect(issues).not_to include security_issue_1 expect(issues).not_to include security_issue_1
expect(issues).not_to include security_issue_2 expect(issues).not_to include security_issue_2
expect(results.issues_count).to eq 1 expect(results.limited_issues_count).to eq 1
end end
it 'does not list project confidential issues for project members with guest role' do it 'does not list project confidential issues for project members with guest role' do
...@@ -229,7 +229,7 @@ describe Gitlab::ProjectSearchResults do ...@@ -229,7 +229,7 @@ describe Gitlab::ProjectSearchResults do
expect(issues).to include issue expect(issues).to include issue
expect(issues).not_to include security_issue_1 expect(issues).not_to include security_issue_1
expect(issues).not_to include security_issue_2 expect(issues).not_to include security_issue_2
expect(results.issues_count).to eq 1 expect(results.limited_issues_count).to eq 1
end end
it 'lists project confidential issues for author' do it 'lists project confidential issues for author' do
...@@ -239,7 +239,7 @@ describe Gitlab::ProjectSearchResults do ...@@ -239,7 +239,7 @@ describe Gitlab::ProjectSearchResults do
expect(issues).to include issue expect(issues).to include issue
expect(issues).to include security_issue_1 expect(issues).to include security_issue_1
expect(issues).not_to include security_issue_2 expect(issues).not_to include security_issue_2
expect(results.issues_count).to eq 2 expect(results.limited_issues_count).to eq 2
end end
it 'lists project confidential issues for assignee' do it 'lists project confidential issues for assignee' do
...@@ -249,7 +249,7 @@ describe Gitlab::ProjectSearchResults do ...@@ -249,7 +249,7 @@ describe Gitlab::ProjectSearchResults do
expect(issues).to include issue expect(issues).to include issue
expect(issues).not_to include security_issue_1 expect(issues).not_to include security_issue_1
expect(issues).to include security_issue_2 expect(issues).to include security_issue_2
expect(results.issues_count).to eq 2 expect(results.limited_issues_count).to eq 2
end end
it 'lists project confidential issues for project members' do it 'lists project confidential issues for project members' do
...@@ -261,7 +261,7 @@ describe Gitlab::ProjectSearchResults do ...@@ -261,7 +261,7 @@ describe Gitlab::ProjectSearchResults do
expect(issues).to include issue expect(issues).to include issue
expect(issues).to include security_issue_1 expect(issues).to include security_issue_1
expect(issues).to include security_issue_2 expect(issues).to include security_issue_2
expect(results.issues_count).to eq 3 expect(results.limited_issues_count).to eq 3
end end
it 'lists all project issues for admin' do it 'lists all project issues for admin' do
...@@ -271,7 +271,7 @@ describe Gitlab::ProjectSearchResults do ...@@ -271,7 +271,7 @@ describe Gitlab::ProjectSearchResults do
expect(issues).to include issue expect(issues).to include issue
expect(issues).to include security_issue_1 expect(issues).to include security_issue_1
expect(issues).to include security_issue_2 expect(issues).to include security_issue_2
expect(results.issues_count).to eq 3 expect(results.limited_issues_count).to eq 3
end end
end end
...@@ -304,6 +304,35 @@ describe Gitlab::ProjectSearchResults do ...@@ -304,6 +304,35 @@ describe Gitlab::ProjectSearchResults do
end end
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 # Examples for commit access level test
# #
# params: # params:
......
...@@ -29,30 +29,6 @@ describe Gitlab::SearchResults do ...@@ -29,30 +29,6 @@ describe Gitlab::SearchResults do
end end
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 context "when count_limit is lower than total amount" do
before do before do
allow(results).to receive(:count_limit).and_return(1) allow(results).to receive(:count_limit).and_return(1)
...@@ -183,7 +159,7 @@ describe Gitlab::SearchResults do ...@@ -183,7 +159,7 @@ describe Gitlab::SearchResults do
expect(issues).not_to include security_issue_3 expect(issues).not_to include security_issue_3
expect(issues).not_to include security_issue_4 expect(issues).not_to include security_issue_4
expect(issues).not_to include security_issue_5 expect(issues).not_to include security_issue_5
expect(results.issues_count).to eq 1 expect(results.limited_issues_count).to eq 1
end end
it 'does not list confidential issues for project members with guest role' do it 'does not list confidential issues for project members with guest role' do
...@@ -199,7 +175,7 @@ describe Gitlab::SearchResults do ...@@ -199,7 +175,7 @@ describe Gitlab::SearchResults do
expect(issues).not_to include security_issue_3 expect(issues).not_to include security_issue_3
expect(issues).not_to include security_issue_4 expect(issues).not_to include security_issue_4
expect(issues).not_to include security_issue_5 expect(issues).not_to include security_issue_5
expect(results.issues_count).to eq 1 expect(results.limited_issues_count).to eq 1
end end
it 'lists confidential issues for author' do it 'lists confidential issues for author' do
...@@ -212,7 +188,7 @@ describe Gitlab::SearchResults do ...@@ -212,7 +188,7 @@ describe Gitlab::SearchResults do
expect(issues).to include security_issue_3 expect(issues).to include security_issue_3
expect(issues).not_to include security_issue_4 expect(issues).not_to include security_issue_4
expect(issues).not_to include security_issue_5 expect(issues).not_to include security_issue_5
expect(results.issues_count).to eq 3 expect(results.limited_issues_count).to eq 3
end end
it 'lists confidential issues for assignee' do it 'lists confidential issues for assignee' do
...@@ -225,7 +201,7 @@ describe Gitlab::SearchResults do ...@@ -225,7 +201,7 @@ describe Gitlab::SearchResults do
expect(issues).not_to include security_issue_3 expect(issues).not_to include security_issue_3
expect(issues).to include security_issue_4 expect(issues).to include security_issue_4
expect(issues).not_to include security_issue_5 expect(issues).not_to include security_issue_5
expect(results.issues_count).to eq 3 expect(results.limited_issues_count).to eq 3
end end
it 'lists confidential issues for project members' do it 'lists confidential issues for project members' do
...@@ -241,7 +217,7 @@ describe Gitlab::SearchResults do ...@@ -241,7 +217,7 @@ describe Gitlab::SearchResults do
expect(issues).to include security_issue_3 expect(issues).to include security_issue_3
expect(issues).not_to include security_issue_4 expect(issues).not_to include security_issue_4
expect(issues).not_to include security_issue_5 expect(issues).not_to include security_issue_5
expect(results.issues_count).to eq 4 expect(results.limited_issues_count).to eq 4
end end
it 'lists all issues for admin' do it 'lists all issues for admin' do
...@@ -254,7 +230,7 @@ describe Gitlab::SearchResults do ...@@ -254,7 +230,7 @@ describe Gitlab::SearchResults do
expect(issues).to include security_issue_3 expect(issues).to include security_issue_3
expect(issues).to include security_issue_4 expect(issues).to include security_issue_4
expect(issues).not_to include security_issue_5 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
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment