Merge branch '29843-project-subgroup-transfer' into 'security'

Use full path for moving directories when changing namespace path

See merge request !2078

7 files changed, 99 insertions, 24 deletions

diff --git a/app/models/namespace.rb b/app/models/namespace.rb
index 826ded22ae5..1d4b1f7d590 100644
--- a/app/models/namespace.rb
+++ b/app/models/namespace.rb
@@ -120,10 +120,10 @@ class Namespace < ActiveRecord::Base
     # Move the namespace directory in all storages paths used by member projects
     repository_storage_paths.each do |repository_storage_path|
       # Ensure old directory exists before moving it
-      gitlab_shell.add_namespace(repository_storage_path, path_was)
+      gitlab_shell.add_namespace(repository_storage_path, full_path_was)
 
-      unless gitlab_shell.mv_namespace(repository_storage_path, path_was, path)
-        Rails.logger.error "Exception moving path #{repository_storage_path} from #{path_was} to #{path}"
+      unless gitlab_shell.mv_namespace(repository_storage_path, full_path_was, full_path)
+        Rails.logger.error "Exception moving path #{repository_storage_path} from #{full_path_was} to #{full_path}"
 
         # if we cannot move namespace directory we should rollback
         # db changes in order to prevent out of sync between db and fs
@@ -131,8 +131,8 @@ class Namespace < ActiveRecord::Base
       end
     end
 
-    Gitlab::UploadsTransfer.new.rename_namespace(path_was, path)
-    Gitlab::PagesTransfer.new.rename_namespace(path_was, path)
+    Gitlab::UploadsTransfer.new.rename_namespace(full_path_was, full_path)
+    Gitlab::PagesTransfer.new.rename_namespace(full_path_was, full_path)
 
     remove_exports!
 
@@ -155,7 +155,7 @@ class Namespace < ActiveRecord::Base
 
   def send_update_instructions
     projects.each do |project|
-      project.send_move_instructions("#{path_was}/#{project.path}")
+      project.send_move_instructions("#{full_path_was}/#{project.path}")
     end
   end
 
@@ -230,10 +230,10 @@ class Namespace < ActiveRecord::Base
     old_repository_storage_paths.each do |repository_storage_path|
       # Move namespace directory into trash.
       # We will remove it later async
-      new_path = "#{path}+#{id}+deleted"
+      new_path = "#{full_path}+#{id}+deleted"
 
-      if gitlab_shell.mv_namespace(repository_storage_path, path, new_path)
-        message = "Namespace directory \"#{path}\" moved to \"#{new_path}\""
+      if gitlab_shell.mv_namespace(repository_storage_path, full_path, new_path)
+        message = "Namespace directory \"#{full_path}\" moved to \"#{new_path}\""
         Gitlab::AppLogger.info message
 
         # Remove namespace directroy async with delay so
diff --git a/changelogs/unreleased/29843-project-subgroup-transfer.yml b/changelogs/unreleased/29843-project-subgroup-transfer.yml
new file mode 100644
index 00000000000..1cf83517591
--- /dev/null
+++ b/changelogs/unreleased/29843-project-subgroup-transfer.yml
@@ -0,0 +1,4 @@
+---
+title: Correctly update paths when changing a child group
+merge_request:
+author:
diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example
index 0b523ad0527..f8fe208500c 100644
--- a/config/gitlab.yml.example
+++ b/config/gitlab.yml.example
@@ -583,6 +583,8 @@ test:
     # user: YOUR_USERNAME
   satellites:
     path: tmp/tests/gitlab-satellites/
+  pages:
+    path: tmp/tests/pages
   repositories:
     storages:
       default:
diff --git a/lib/gitlab/uploads_transfer.rb b/lib/gitlab/uploads_transfer.rb
index 81701831a6a..7d0c47c5361 100644
--- a/lib/gitlab/uploads_transfer.rb
+++ b/lib/gitlab/uploads_transfer.rb
@@ -1,7 +1,7 @@
 module Gitlab
   class UploadsTransfer < ProjectTransfer
     def root_dir
-      File.join(Rails.root, "public", "uploads")
+      File.join(CarrierWave.root, GitlabUploader.base_dir)
     end
   end
 end
diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb
index 757f3921450..bfcc162bb0a 100644
--- a/spec/models/namespace_spec.rb
+++ b/spec/models/namespace_spec.rb
@@ -129,10 +129,10 @@ describe Namespace, models: true do
     end
   end
 
-  describe '#move_dir' do
+  describe '#move_dir', repository: true do
     before do
       @namespace = create :namespace
-      @project = create(:empty_project, namespace: @namespace)
+      @project = create(:project_empty_repo, namespace: @namespace)
       allow(@namespace).to receive(:path_changed?).and_return(true)
     end
 
@@ -141,9 +141,9 @@ describe Namespace, models: true do
     end
 
     it "moves dir if path changed" do
-      new_path = @namespace.path + "_new"
-      allow(@namespace).to receive(:path_was).and_return(@namespace.path)
-      allow(@namespace).to receive(:path).and_return(new_path)
+      new_path = @namespace.full_path + "_new"
+      allow(@namespace).to receive(:full_path_was).and_return(@namespace.full_path)
+      allow(@namespace).to receive(:full_path).and_return(new_path)
       expect(@namespace).to receive(:remove_exports!)
       expect(@namespace.move_dir).to be_truthy
     end
@@ -161,16 +161,75 @@ describe Namespace, models: true do
 
       it { expect { @namespace.move_dir }.to raise_error('Namespace cannot be moved, because at least one project has tags in container registry') }
     end
+
+    context 'renaming a sub-group' do
+      let(:parent) { create(:group, name: 'parent', path: 'parent') }
+      let(:child) { create(:group, name: 'child', path: 'child', parent: parent) }
+      let!(:project) { create(:project_empty_repo, path: 'the-project', namespace: child) }
+      let(:uploads_dir) { File.join(CarrierWave.root, 'uploads', 'parent') }
+      let(:pages_dir) { File.join(TestEnv.pages_path, 'parent') }
+
+      before do
+        FileUtils.mkdir_p(File.join(uploads_dir, 'child', 'the-project'))
+        FileUtils.mkdir_p(File.join(pages_dir, 'child', 'the-project'))
+      end
+
+      it 'correctly moves the repository, uploads and pages' do
+        expected_repository_path = File.join(TestEnv.repos_path, 'parent', 'renamed', 'the-project.git')
+        expected_upload_path = File.join(uploads_dir, 'renamed', 'the-project')
+        expected_pages_path = File.join(pages_dir, 'renamed', 'the-project')
+
+        child.update_attributes!(path: 'renamed')
+
+        expect(File.directory?(expected_repository_path)).to be(true)
+        expect(File.directory?(expected_upload_path)).to be(true)
+        expect(File.directory?(expected_pages_path)).to be(true)
+      end
+    end
   end
 
-  describe :rm_dir do
-    let!(:project) { create(:empty_project, namespace: namespace) }
-    let!(:path) { File.join(Gitlab.config.repositories.storages.default['path'], namespace.full_path) }
+  describe '#rm_dir', repository: true do
+    let!(:project) { create(:project_empty_repo, namespace: namespace) }
+    let(:repository_storage_path) { Gitlab.config.repositories.storages.default['path'] }
+    let(:path_in_dir) { File.join(repository_storage_path, namespace.full_path) }
+    let(:deleted_path) { namespace.full_path.gsub(namespace.path, "#{namespace.full_path}+#{namespace.id}+deleted") }
+    let(:deleted_path_in_dir) { File.join(repository_storage_path, deleted_path) }
+
+    it 'renames its dirs when deleted' do
+      allow(GitlabShellWorker).to receive(:perform_in)
 
-    it "removes its dirs when deleted" do
       namespace.destroy
 
-      expect(File.exist?(path)).to be(false)
+      expect(File.exist?(deleted_path_in_dir)).to be(true)
+    end
+
+    it 'schedules the namespace for deletion' do
+      expect(GitlabShellWorker).to receive(:perform_in).with(5.minutes, :rm_namespace, repository_storage_path, deleted_path)
+
+      namespace.destroy
+    end
+
+    context 'in sub-groups' do
+      let(:parent) { create(:namespace, path: 'parent') }
+      let(:child) { create(:namespace, parent: parent, path: 'child') }
+      let!(:project) { create(:project_empty_repo, namespace: child) }
+      let(:path_in_dir) { File.join(repository_storage_path, 'parent', 'child') }
+      let(:deleted_path) { File.join('parent', "child+#{child.id}+deleted") }
+      let(:deleted_path_in_dir) { File.join(repository_storage_path, deleted_path) }
+
+      it 'renames its dirs when deleted' do
+        allow(GitlabShellWorker).to receive(:perform_in)
+
+        child.destroy
+
+        expect(File.exist?(deleted_path_in_dir)).to be(true)
+      end
+
+      it 'schedules the namespace for deletion' do
+        expect(GitlabShellWorker).to receive(:perform_in).with(5.minutes, :rm_namespace, repository_storage_path, deleted_path)
+
+        child.destroy
+      end
     end
 
     it 'removes the exports folder' do
diff --git a/spec/support/stored_repositories.rb b/spec/support/stored_repositories.rb
new file mode 100644
index 00000000000..df18926d58c
--- /dev/null
+++ b/spec/support/stored_repositories.rb
@@ -0,0 +1,5 @@
+RSpec.configure do |config|
+  config.before(:each, :repository) do
+    TestEnv.clean_test_path
+  end
+end
diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb
index f1d226b6ae3..9da54cb886d 100644
--- a/spec/support/test_env.rb
+++ b/spec/support/test_env.rb
@@ -60,9 +60,6 @@ module TestEnv
 
     clean_test_path
 
-    FileUtils.mkdir_p(repos_path)
-    FileUtils.mkdir_p(backup_path)
-
     # Setup GitLab shell for test instance
     setup_gitlab_shell
 
@@ -94,10 +91,14 @@ module TestEnv
     tmp_test_path = Rails.root.join('tmp', 'tests', '**')
 
     Dir[tmp_test_path].each do |entry|
-      unless File.basename(entry) =~ /\Agitlab-(shell|test|test-fork)\z/
+      unless File.basename(entry) =~ /\Agitlab-(shell|test|test_bare|test-fork)\z/
         FileUtils.rm_rf(entry)
       end
     end
+
+    FileUtils.mkdir_p(repos_path)
+    FileUtils.mkdir_p(backup_path)
+    FileUtils.mkdir_p(pages_path)
   end
 
   def setup_gitlab_shell
@@ -150,6 +151,10 @@ module TestEnv
     Gitlab.config.backup.path
   end
 
+  def pages_path
+    Gitlab.config.pages.path
+  end
+
   def copy_forked_repo_with_submodules(project)
     base_repo_path = File.expand_path(forked_repo_path_bare)
     target_repo_path = File.expand_path(project.repository_storage_path + "/#{project.full_path}.git")