Migrate GitlabProject (re)move project endpoints

Migration is done through a small refactoring, which makes us call
endpoins which are performing the same actions for namespaces.
Tests are added to ensure only the project is removed that should be
removed.

Closes gitlab-org/gitaly#873

4 files changed, 41 insertions, 90 deletions

diff --git a/lib/gitlab/git/gitlab_projects.rb b/lib/gitlab/git/gitlab_projects.rb
index cba638c06db..976fa1ddfe6 100644
--- a/lib/gitlab/git/gitlab_projects.rb
+++ b/lib/gitlab/git/gitlab_projects.rb
@@ -41,36 +41,6 @@ module Gitlab
         io.read
       end
 
-      def rm_project
-        logger.info "Removing repository <#{repository_absolute_path}>."
-        FileUtils.rm_rf(repository_absolute_path)
-      end
-
-      # Move repository from one directory to another
-      #
-      # Example:  gitlab/gitlab-ci.git -> randx/six.git
-      #
-      # Won't work if target namespace directory does not exist
-      #
-      def mv_project(new_path)
-        new_absolute_path = File.join(shard_path, new_path)
-
-        # verify that the source repo exists
-        unless File.exist?(repository_absolute_path)
-          logger.error "mv-project failed: source path <#{repository_absolute_path}> does not exist."
-          return false
-        end
-
-        # ...and that the target repo does not exist
-        if File.exist?(new_absolute_path)
-          logger.error "mv-project failed: destination path <#{new_absolute_path}> already exists."
-          return false
-        end
-
-        logger.info "Moving repository from <#{repository_absolute_path}> to <#{new_absolute_path}>."
-        FileUtils.mv(repository_absolute_path, new_absolute_path)
-      end
-
       # Import project via git clone --bare
       # URL must be publicly cloneable
       def import_project(source, timeout)
diff --git a/lib/gitlab/shell.rb b/lib/gitlab/shell.rb
index a8a4ec996c4..766b93ad507 100644
--- a/lib/gitlab/shell.rb
+++ b/lib/gitlab/shell.rb
@@ -136,7 +136,10 @@ module Gitlab
       end
     end
 
-    # Move repository
+    # Move repository reroutes to mv_directory which is an alias for
+    # mv_namespace. Given the underlying implementation is a move action,
+    # indescriminate of what the folders might be.
+    #
     # storage - project's storage path
     # path - project disk path
     # new_path - new project disk path
@@ -146,7 +149,9 @@ module Gitlab
     #
     # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/873
     def mv_repository(storage, path, new_path)
-      gitlab_projects(storage, "#{path}.git").mv_project("#{new_path}.git")
+      return false if path.empty? || new_path.empty?
+
+      !!mv_directory(storage, "#{path}.git", "#{new_path}.git")
     end
 
     # Fork repository to new path
@@ -164,7 +169,9 @@ module Gitlab
         .fork_repository(forked_to_storage, "#{forked_to_disk_path}.git")
     end
 
-    # Remove repository from file system
+    # Removes a repository from file system, using rm_diretory which is an alias
+    # for rm_namespace. Given the underlying implementation removes the name
+    # passed as second argument on the passed storage.
     #
     # storage - project's storage path
     # name - project disk path
@@ -174,7 +181,12 @@ module Gitlab
     #
     # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/873
     def remove_repository(storage, name)
-      gitlab_projects(storage, "#{name}.git").rm_project
+      return false if name.empty?
+
+      !!rm_directory(storage, "#{name}.git")
+    rescue ArgumentError => e
+      Rails.logger.warn("Repository does not exist: #{e} at: #{name}.git")
+      false
     end
 
     # Add new key to gitlab-shell
@@ -255,6 +267,7 @@ module Gitlab
     rescue GRPC::InvalidArgument => e
       raise ArgumentError, e.message
     end
+    alias_method :rm_directory, :rm_namespace
 
     # Move namespace directory inside repositories storage
     #
@@ -274,6 +287,7 @@ module Gitlab
     rescue GRPC::InvalidArgument
       false
     end
+    alias_method :mv_directory, :mv_namespace
 
     def url_to_repo(path)
       Gitlab.config.gitlab_shell.ssh_path_prefix + "#{path}.git"
diff --git a/spec/lib/gitlab/git/gitlab_projects_spec.rb b/spec/lib/gitlab/git/gitlab_projects_spec.rb
index a798b188a0d..beef843537d 100644
--- a/spec/lib/gitlab/git/gitlab_projects_spec.rb
+++ b/spec/lib/gitlab/git/gitlab_projects_spec.rb
@@ -25,51 +25,6 @@ describe Gitlab::Git::GitlabProjects do
     it { expect(gl_projects.logger).to eq(logger) }
   end
 
-  describe '#mv_project' do
-    let(:new_repo_path) { File.join(tmp_repos_path, 'repo.git') }
-
-    it 'moves a repo directory' do
-      expect(File.exist?(tmp_repo_path)).to be_truthy
-
-      message = "Moving repository from <#{tmp_repo_path}> to <#{new_repo_path}>."
-      expect(logger).to receive(:info).with(message)
-
-      expect(gl_projects.mv_project('repo.git')).to be_truthy
-
-      expect(File.exist?(tmp_repo_path)).to be_falsy
-      expect(File.exist?(new_repo_path)).to be_truthy
-    end
-
-    it "fails if the source path doesn't exist" do
-      expected_source_path = File.join(tmp_repos_path, 'bad-src.git')
-      expect(logger).to receive(:error).with("mv-project failed: source path <#{expected_source_path}> does not exist.")
-
-      result = build_gitlab_projects(tmp_repos_path, 'bad-src.git').mv_project('repo.git')
-      expect(result).to be_falsy
-    end
-
-    it 'fails if the destination path already exists' do
-      FileUtils.mkdir_p(File.join(tmp_repos_path, 'already-exists.git'))
-
-      expected_distination_path = File.join(tmp_repos_path, 'already-exists.git')
-      message = "mv-project failed: destination path <#{expected_distination_path}> already exists."
-      expect(logger).to receive(:error).with(message)
-
-      expect(gl_projects.mv_project('already-exists.git')).to be_falsy
-    end
-  end
-
-  describe '#rm_project' do
-    it 'removes a repo directory' do
-      expect(File.exist?(tmp_repo_path)).to be_truthy
-      expect(logger).to receive(:info).with("Removing repository <#{tmp_repo_path}>.")
-
-      expect(gl_projects.rm_project).to be_truthy
-
-      expect(File.exist?(tmp_repo_path)).to be_falsy
-    end
-  end
-
   describe '#push_branches' do
     let(:remote_name) { 'remote-name' }
     let(:branch_name) { 'master' }
diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb
index ffd2d2c7afc..74268419259 100644
--- a/spec/lib/gitlab/shell_spec.rb
+++ b/spec/lib/gitlab/shell_spec.rb
@@ -149,32 +149,44 @@ describe Gitlab::Shell do
     end
 
     describe '#remove_repository' do
-      subject { gitlab_shell.remove_repository(project.repository_storage_path, project.disk_path) }
+      let!(:project) { create(:project, :repository) }
+      let(:disk_path) { "#{project.disk_path}.git" }
 
       it 'returns true when the command succeeds' do
-        expect(gitlab_projects).to receive(:rm_project) { true }
+        expect(gitlab_shell.exists?(project.repository_storage_path, disk_path)).to be(true)
 
-        is_expected.to be_truthy
+        expect(gitlab_shell.remove_repository(project.repository_storage_path, project.disk_path)).to be(true)
+
+        expect(gitlab_shell.exists?(project.repository_storage_path, disk_path)).to be(false)
       end
 
-      it 'returns false when the command fails' do
-        expect(gitlab_projects).to receive(:rm_project) { false }
+      it 'keeps the namespace directory' do
+        gitlab_shell.remove_repository(project.repository_storage_path, project.disk_path)
 
-        is_expected.to be_falsy
+        expect(gitlab_shell.exists?(project.repository_storage_path, disk_path)).to be(false)
+        expect(gitlab_shell.exists?(project.repository_storage_path, project.disk_path.gsub(project.name, ''))).to be(true)
       end
     end
 
     describe '#mv_repository' do
+      let!(:project2) { create(:project, :repository) }
+
       it 'returns true when the command succeeds' do
-        expect(gitlab_projects).to receive(:mv_project).with('project/newpath.git') { true }
+        old_path = project2.disk_path
+        new_path = "project/new_path"
+
+        expect(gitlab_shell.exists?(project2.repository_storage_path, "#{old_path}.git")).to be(true)
+        expect(gitlab_shell.exists?(project2.repository_storage_path, "#{new_path}.git")).to be(false)
 
-        expect(gitlab_shell.mv_repository(project.repository_storage_path, project.disk_path, 'project/newpath')).to be_truthy
+        expect(gitlab_shell.mv_repository(project2.repository_storage_path, old_path, new_path)).to be_truthy
+
+        expect(gitlab_shell.exists?(project2.repository_storage_path, "#{old_path}.git")).to be(false)
+        expect(gitlab_shell.exists?(project2.repository_storage_path, "#{new_path}.git")).to be(true)
       end
 
       it 'returns false when the command fails' do
-        expect(gitlab_projects).to receive(:mv_project).with('project/newpath.git') { false }
-
-        expect(gitlab_shell.mv_repository(project.repository_storage_path, project.disk_path, 'project/newpath')).to be_falsy
+        expect(gitlab_shell.mv_repository(project2.repository_storage_path, project2.disk_path, '')).to be_falsy
+        expect(gitlab_shell.exists?(project2.repository_storage_path, "#{project2.disk_path}.git")).to be(true)
       end
     end