summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTiago Botelho <tiagonbotelho@hotmail.com>2017-11-28 12:46:29 +0000
committerTiago Botelho <tiagonbotelho@hotmail.com>2017-11-28 12:46:29 +0000
commit9a65e9b2605c8d9c878ffa844a7c7b6331f0df19 (patch)
tree3712eb9e63d5a2fcf33ce5eeb48c0a6eb1149182
parent1613a631868fe6347236dc25fa83dea01fa1b0c7 (diff)
parent47c3d360e2857bb6715c7d79bd177bc8cb84ac61 (diff)
downloadgitlab-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.rb30
-rw-r--r--changelogs/unreleased/40291-ignore-hashed-repos-cleanup-repositories.yml5
-rw-r--r--changelogs/unreleased/40352-ignore-hashed-repos-cleanup-dirs.yml5
-rw-r--r--changelogs/unreleased/dm-project-search-performance.yml6
-rw-r--r--lib/gitlab/search_results.rb2
-rw-r--r--lib/tasks/gitlab/cleanup.rake7
-rw-r--r--spec/finders/admin/projects_finder_spec.rb2
-rw-r--r--spec/models/project_spec.rb18
-rw-r--r--spec/services/search/global_service_spec.rb4
-rw-r--r--spec/tasks/gitlab/cleanup_rake_spec.rb46
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