summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJan Provaznik <jprovaznik@gitlab.com>2017-12-15 11:21:12 +0100
committerJan Provaznik <jprovaznik@gitlab.com>2017-12-19 09:06:33 +0100
commit7cc421d703f29ac55f18d16c6301c02199351c91 (patch)
tree65c8b9cb4dd02a6b57b6fa83c677bd0c99eb540e
parent0a395a0885a46e07a8e9f51dfb8b7b4b3f8362f6 (diff)
downloadgitlab-ce-jprovazn-search-ee.tar.gz
Skip projects filter on merge requests searchjprovazn-search-ee
When searching for merge requests, an additional subquery is added which by default filters only merge requests which belong to source or target project user has permission for. This filter is not needed because more restrictive filter which checks if user has permission for target project is used in the query. So unless a custom projects filter is used by user, it's possible to skip the default projects filter and speed up the final query. Related to #40540
-rw-r--r--app/services/search/global_service.rb5
-rw-r--r--app/services/search/group_service.rb1
-rw-r--r--lib/gitlab/search_results.rb15
-rw-r--r--spec/lib/gitlab/search_results_spec.rb11
4 files changed, 29 insertions, 3 deletions
diff --git a/app/services/search/global_service.rb b/app/services/search/global_service.rb
index 20e7aae903f..51fb8738e86 100644
--- a/app/services/search/global_service.rb
+++ b/app/services/search/global_service.rb
@@ -3,16 +3,19 @@ module Search
include Gitlab::CurrentSettings
attr_accessor :current_user, :params
+ attr_reader :default_project_filter
def initialize(user, params)
@current_user, @params = user, params.dup
+ @default_project_filter = true
end
def execute
if current_application_settings.elasticsearch_search?
Gitlab::Elastic::SearchResults.new(current_user, params[:search], elastic_projects, elastic_global)
else
- Gitlab::SearchResults.new(current_user, projects, params[:search])
+ Gitlab::SearchResults.new(current_user, projects, params[:search],
+ default_project_filter: default_project_filter)
end
end
diff --git a/app/services/search/group_service.rb b/app/services/search/group_service.rb
index 6273594cdbc..933ae66fc1c 100644
--- a/app/services/search/group_service.rb
+++ b/app/services/search/group_service.rb
@@ -5,6 +5,7 @@ module Search
def initialize(user, group, params)
super(user, params)
+ @default_project_filter = false
@group = group
end
diff --git a/lib/gitlab/search_results.rb b/lib/gitlab/search_results.rb
index fef9d3e31d4..7037e2e61cc 100644
--- a/lib/gitlab/search_results.rb
+++ b/lib/gitlab/search_results.rb
@@ -27,10 +27,17 @@ module Gitlab
# It allows us to search only for projects user has access to
attr_reader :limit_projects
- def initialize(current_user, limit_projects, query)
+ # Whether a custom filter is used to restrict scope of projects.
+ # If the default filter (which lists all projects user has access to)
+ # is used, we can skip it when filtering merge requests and optimize the
+ # query
+ attr_reader :default_project_filter
+
+ def initialize(current_user, limit_projects, query, default_project_filter: false)
@current_user = current_user
@limit_projects = limit_projects || Project.all
@query = query
+ @default_project_filter = default_project_filter
end
def objects(scope, page = nil)
@@ -94,7 +101,11 @@ module Gitlab
end
def merge_requests
- merge_requests = MergeRequestsFinder.new(current_user).execute.in_projects(project_ids_relation)
+ merge_requests = MergeRequestsFinder.new(current_user).execute
+ unless default_project_filter
+ merge_requests = merge_requests.in_projects(project_ids_relation)
+ end
+
merge_requests =
if query =~ /[#!](\d+)\z/
merge_requests.where(iid: $1)
diff --git a/spec/lib/gitlab/search_results_spec.rb b/spec/lib/gitlab/search_results_spec.rb
index e44a7c23452..a958baab44f 100644
--- a/spec/lib/gitlab/search_results_spec.rb
+++ b/spec/lib/gitlab/search_results_spec.rb
@@ -51,6 +51,17 @@ describe Gitlab::SearchResults do
expect(results.objects('merge_requests')).to include merge_request_2
end
+
+ it 'includes project filter by default' do
+ expect(results).to receive(:project_ids_relation).and_call_original
+ results.objects('merge_requests')
+ end
+
+ it 'it skips project filter if default is used' do
+ allow(results).to receive(:default_project_filter).and_return(true)
+ expect(results).not_to receive(:project_ids_relation)
+ results.objects('merge_requests')
+ end
end
it 'does not list issues on private projects' do