From 8041a87288906e4b10b86a9a2ab9039036243a5d Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Fri, 24 Nov 2017 11:23:14 +0100 Subject: [PATCH] Drastically improve project search performance by no longer searching namespace name --- app/models/project.rb | 28 +++++-------------- .../dm-project-search-performance.yml | 6 ++++ lib/gitlab/search_results.rb | 2 +- spec/finders/admin/projects_finder_spec.rb | 2 +- spec/models/project_spec.rb | 18 ------------ spec/services/search/global_service_spec.rb | 4 +-- 6 files changed, 17 insertions(+), 43 deletions(-) create mode 100644 changelogs/unreleased/dm-project-search-performance.yml diff --git a/app/models/project.rb b/app/models/project.rb index 43a4d86c138..e276bd2422d 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -18,6 +18,7 @@ class Project < ActiveRecord::Base include SelectForProjectAuthorization include Routable include GroupDescendant + include Gitlab::SQL::Pattern extend Gitlab::ConfigHelper extend Gitlab::CurrentSettings @@ -424,32 +425,17 @@ class Project < ActiveRecord::Base # # query - The search query as a String. def search(query) - ptable = arel_table - ntable = Namespace.arel_table - pattern = "%#{query}%" + pattern = to_pattern(query) - # unscoping unnecessary conditions that'll be applied - # when executing `where("projects.id IN (#{union.to_sql})")` - projects = unscoped.select(:id).where( - ptable[:path].matches(pattern) - .or(ptable[:name].matches(pattern)) - .or(ptable[:description].matches(pattern)) + where( + arel_table[:path].matches(pattern) + .or(arel_table[:name].matches(pattern)) + .or(arel_table[:description].matches(pattern)) ) - - namespaces = unscoped.select(:id) - .joins(:namespace) - .where(ntable[:name].matches(pattern)) - - union = Gitlab::SQL::Union.new([projects, namespaces]) - - where("projects.id IN (#{union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection end def search_by_title(query) - pattern = "%#{query}%" - table = Project.arel_table - - non_archived.where(table[:name].matches(pattern)) + non_archived.where(arel_table[:name].matches(to_pattern(query))) end def visibility_levels diff --git a/changelogs/unreleased/dm-project-search-performance.yml b/changelogs/unreleased/dm-project-search-performance.yml new file mode 100644 index 00000000000..b533043b163 --- /dev/null +++ b/changelogs/unreleased/dm-project-search-performance.yml @@ -0,0 +1,6 @@ +--- +title: Drastically improve project search performance by no longer searching namespace + name +merge_request: +author: +type: performance diff --git a/lib/gitlab/search_results.rb b/lib/gitlab/search_results.rb index efe8095beea..fef9d3e31d4 100644 --- a/lib/gitlab/search_results.rb +++ b/lib/gitlab/search_results.rb @@ -30,7 +30,7 @@ module Gitlab def initialize(current_user, limit_projects, query) @current_user = current_user @limit_projects = limit_projects || Project.all - @query = Shellwords.shellescape(query) if query.present? + @query = query end def objects(scope, page = nil) diff --git a/spec/finders/admin/projects_finder_spec.rb b/spec/finders/admin/projects_finder_spec.rb index 4b67203a0df..7901d5fee28 100644 --- a/spec/finders/admin/projects_finder_spec.rb +++ b/spec/finders/admin/projects_finder_spec.rb @@ -136,7 +136,7 @@ describe Admin::ProjectsFinder do context 'filter by name' do let(:params) { { name: 'C' } } - it { is_expected.to match_array([shared_project, public_project, private_project]) } + it { is_expected.to match_array([public_project]) } end context 'sorting' do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index f7f19d464d1..549c97a9afd 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1254,24 +1254,6 @@ describe Project do expect(described_class.search(project.path.upcase)).to eq([project]) end - it 'returns projects with a matching namespace name' do - expect(described_class.search(project.namespace.name)).to eq([project]) - end - - it 'returns projects with a partially matching namespace name' do - expect(described_class.search(project.namespace.name[0..2])).to eq([project]) - end - - it 'returns projects with a matching namespace name regardless of the casing' do - expect(described_class.search(project.namespace.name.upcase)).to eq([project]) - end - - it 'returns projects when eager loading namespaces' do - relation = described_class.all.includes(:namespace) - - expect(relation.search(project.namespace.name)).to eq([project]) - end - describe 'with pending_delete project' do let(:pending_delete_project) { create(:project, pending_delete: true) } diff --git a/spec/services/search/global_service_spec.rb b/spec/services/search/global_service_spec.rb index 1309240b430..d8dba26e194 100644 --- a/spec/services/search/global_service_spec.rb +++ b/spec/services/search/global_service_spec.rb @@ -35,8 +35,8 @@ describe Search::GlobalService do expect(results.objects('projects')).to match_array [internal_project, public_project] end - it 'namespace name is searchable' do - results = described_class.new(user, search: found_project.namespace.path).execute + it 'project name is searchable' do + results = described_class.new(user, search: found_project.name).execute expect(results.objects('projects')).to match_array [found_project] end