diff options
author | Tiago Botelho <tiagonbotelho@hotmail.com> | 2017-11-28 12:46:29 +0000 |
---|---|---|
committer | Tiago Botelho <tiagonbotelho@hotmail.com> | 2017-11-28 12:46:29 +0000 |
commit | 9a65e9b2605c8d9c878ffa844a7c7b6331f0df19 (patch) | |
tree | 3712eb9e63d5a2fcf33ce5eeb48c0a6eb1149182 | |
parent | 1613a631868fe6347236dc25fa83dea01fa1b0c7 (diff) | |
parent | 47c3d360e2857bb6715c7d79bd177bc8cb84ac61 (diff) | |
download | gitlab-ce-9a65e9b2605c8d9c878ffa844a7c7b6331f0df19.tar.gz |
Merge branch '10-2-stable-patch-3' of https://gitlab.com/gitlab-org/gitlab-ce into 10-2-stable-patch-3
-rw-r--r-- | app/models/project.rb | 30 | ||||
-rw-r--r-- | changelogs/unreleased/40291-ignore-hashed-repos-cleanup-repositories.yml | 5 | ||||
-rw-r--r-- | changelogs/unreleased/40352-ignore-hashed-repos-cleanup-dirs.yml | 5 | ||||
-rw-r--r-- | changelogs/unreleased/dm-project-search-performance.yml | 6 | ||||
-rw-r--r-- | lib/gitlab/search_results.rb | 2 | ||||
-rw-r--r-- | lib/tasks/gitlab/cleanup.rake | 7 | ||||
-rw-r--r-- | spec/finders/admin/projects_finder_spec.rb | 2 | ||||
-rw-r--r-- | spec/models/project_spec.rb | 18 | ||||
-rw-r--r-- | spec/services/search/global_service_spec.rb | 4 | ||||
-rw-r--r-- | spec/tasks/gitlab/cleanup_rake_spec.rb | 46 |
10 files changed, 69 insertions, 56 deletions
diff --git a/app/models/project.rb b/app/models/project.rb index c48c1078095..f6bf29e772f 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}%" - - # 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)) - ) - - namespaces = unscoped.select(:id) - .joins(:namespace) - .where(ntable[:name].matches(pattern)) - - union = Gitlab::SQL::Union.new([projects, namespaces]) + pattern = to_pattern(query) - where("projects.id IN (#{union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection + where( + arel_table[:path].matches(pattern) + .or(arel_table[:name].matches(pattern)) + .or(arel_table[:description].matches(pattern)) + ) 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/40291-ignore-hashed-repos-cleanup-repositories.yml b/changelogs/unreleased/40291-ignore-hashed-repos-cleanup-repositories.yml new file mode 100644 index 00000000000..1e3f52b3a9c --- /dev/null +++ b/changelogs/unreleased/40291-ignore-hashed-repos-cleanup-repositories.yml @@ -0,0 +1,5 @@ +--- +title: Ensure that rake gitlab:cleanup:repos task does not mess with hashed repositories +merge_request: 15520 +author: +type: fixed diff --git a/changelogs/unreleased/40352-ignore-hashed-repos-cleanup-dirs.yml b/changelogs/unreleased/40352-ignore-hashed-repos-cleanup-dirs.yml new file mode 100644 index 00000000000..0ccbc699729 --- /dev/null +++ b/changelogs/unreleased/40352-ignore-hashed-repos-cleanup-dirs.yml @@ -0,0 +1,5 @@ +--- +title: Ensure that rake gitlab:cleanup:dirs task does not mess with hashed repositories +merge_request: 15600 +author: +type: fixed 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/lib/tasks/gitlab/cleanup.rake b/lib/tasks/gitlab/cleanup.rake index a0c3e6adae8..57ee2eb3f7b 100644 --- a/lib/tasks/gitlab/cleanup.rake +++ b/lib/tasks/gitlab/cleanup.rake @@ -1,11 +1,14 @@ namespace :gitlab do namespace :cleanup do + HASHED_REPOSITORY_NAME = '@hashed'.freeze + desc "GitLab | Cleanup | Clean namespaces" task dirs: :environment do warn_user_is_not_gitlab remove_flag = ENV['REMOVE'] - namespaces = Namespace.pluck(:path) + namespaces = Namespace.pluck(:path) + namespaces << HASHED_REPOSITORY_NAME # add so that it will be ignored Gitlab.config.repositories.storages.each do |name, repository_storage| git_base_path = repository_storage['path'] all_dirs = Dir.glob(git_base_path + '/*') @@ -62,7 +65,7 @@ namespace :gitlab do # TODO ignoring hashed repositories for now. But revisit to fully support # possible orphaned hashed repos - next if repo_with_namespace.start_with?('@hashed/') || Project.find_by_full_path(repo_with_namespace) + next if repo_with_namespace.start_with?("#{HASHED_REPOSITORY_NAME}/") || Project.find_by_full_path(repo_with_namespace) new_path = path + move_suffix puts path.inspect + ' -> ' + new_path.inspect 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 diff --git a/spec/tasks/gitlab/cleanup_rake_spec.rb b/spec/tasks/gitlab/cleanup_rake_spec.rb index 641eccfd334..9e746ceddd6 100644 --- a/spec/tasks/gitlab/cleanup_rake_spec.rb +++ b/spec/tasks/gitlab/cleanup_rake_spec.rb @@ -5,7 +5,7 @@ describe 'gitlab:cleanup rake tasks' do Rake.application.rake_require 'tasks/gitlab/cleanup' end - context 'cleanup repositories' do + describe 'cleanup' do let(:gitaly_address) { Gitlab.config.repositories.storages.default.gitaly_address } let(:storages) do { @@ -22,20 +22,46 @@ describe 'gitlab:cleanup rake tasks' do FileUtils.rm_rf(Settings.absolute('tmp/tests/default_storage')) end - it 'moves it to an orphaned path' do - FileUtils.mkdir_p(Settings.absolute('tmp/tests/default_storage/broken/project.git')) - run_rake_task('gitlab:cleanup:repos') - repo_list = Dir['tmp/tests/default_storage/broken/*'] + describe 'cleanup:repos' do + before do + FileUtils.mkdir_p(Settings.absolute('tmp/tests/default_storage/broken/project.git')) + FileUtils.mkdir_p(Settings.absolute('tmp/tests/default_storage/@hashed/12/34/5678.git')) + end - expect(repo_list.first).to include('+orphaned+') + it 'moves it to an orphaned path' do + run_rake_task('gitlab:cleanup:repos') + repo_list = Dir['tmp/tests/default_storage/broken/*'] + + expect(repo_list.first).to include('+orphaned+') + end + + it 'ignores @hashed repos' do + run_rake_task('gitlab:cleanup:repos') + + expect(Dir.exist?(Settings.absolute('tmp/tests/default_storage/@hashed/12/34/5678.git'))).to be_truthy + end end - it 'ignores @hashed repos' do - FileUtils.mkdir_p(Settings.absolute('tmp/tests/default_storage/@hashed/12/34/5678.git')) + describe 'cleanup:dirs' do + it 'removes missing namespaces' do + FileUtils.mkdir_p(Settings.absolute("tmp/tests/default_storage/namespace_1/project.git")) + FileUtils.mkdir_p(Settings.absolute("tmp/tests/default_storage/namespace_2/project.git")) + allow(Namespace).to receive(:pluck).and_return('namespace_1') + + stub_env('REMOVE', 'true') + run_rake_task('gitlab:cleanup:dirs') + + expect(Dir.exist?(Settings.absolute('tmp/tests/default_storage/namespace_1'))).to be_truthy + expect(Dir.exist?(Settings.absolute('tmp/tests/default_storage/namespace_2'))).to be_falsey + end + + it 'ignores @hashed directory' do + FileUtils.mkdir_p(Settings.absolute('tmp/tests/default_storage/@hashed/12/34/5678.git')) - run_rake_task('gitlab:cleanup:repos') + run_rake_task('gitlab:cleanup:dirs') - expect(Dir.exist?(Settings.absolute('tmp/tests/default_storage/@hashed/12/34/5678.git'))).to be_truthy + expect(Dir.exist?(Settings.absolute('tmp/tests/default_storage/@hashed/12/34/5678.git'))).to be_truthy + end end end end |