From d2f1d585e10e3e728f968ceae6b275e4d9bc59f4 Mon Sep 17 00:00:00 2001 From: Jan Provaznik Date: Fri, 15 Dec 2017 11:21:12 +0100 Subject: [PATCH] Skip projects filter on merge requests search 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 --- app/services/search/global_service.rb | 5 ++++- app/services/search/group_service.rb | 1 + .../unreleased/15955-improve-search-query.yml | 5 +++++ lib/gitlab/search_results.rb | 15 +++++++++++++-- spec/lib/gitlab/search_results_spec.rb | 11 +++++++++++ 5 files changed, 34 insertions(+), 3 deletions(-) create mode 100644 changelogs/unreleased/15955-improve-search-query.yml diff --git a/app/services/search/global_service.rb b/app/services/search/global_service.rb index ff188102b62..92a32e703af 100644 --- a/app/services/search/global_service.rb +++ b/app/services/search/global_service.rb @@ -1,13 +1,16 @@ module Search class GlobalService 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 - Gitlab::SearchResults.new(current_user, projects, params[:search]) + Gitlab::SearchResults.new(current_user, projects, params[:search], + default_project_filter: default_project_filter) end def projects diff --git a/app/services/search/group_service.rb b/app/services/search/group_service.rb index 29478e3251f..b4efba68715 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/changelogs/unreleased/15955-improve-search-query.yml b/changelogs/unreleased/15955-improve-search-query.yml new file mode 100644 index 00000000000..80cb8af617f --- /dev/null +++ b/changelogs/unreleased/15955-improve-search-query.yml @@ -0,0 +1,5 @@ +--- +title: Improve search query for merge requests. +merge_request: +author: +type: performance 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