From 6bac612dc4545abcbbfc86477417f711e02427ba Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Wed, 16 Jan 2019 22:40:23 -0800
Subject: [PATCH] Clean up Shell.fork_repository

Use project models instead of a list of parameters.
---
 app/workers/repository_fork_worker.rb       |  4 +---
 lib/gitlab/shell.rb                         | 19 ++++++-------------
 spec/lib/gitlab/shell_spec.rb               | 11 +++--------
 spec/workers/repository_fork_worker_spec.rb |  9 +--------
 4 files changed, 11 insertions(+), 32 deletions(-)

diff --git a/app/workers/repository_fork_worker.rb b/app/workers/repository_fork_worker.rb
index 453fca44ea2..a9b88a133be 100644
--- a/app/workers/repository_fork_worker.rb
+++ b/app/workers/repository_fork_worker.rb
@@ -25,9 +25,7 @@ class RepositoryForkWorker
 
     Gitlab::Metrics.add_event(:fork_repository)
 
-    result = gitlab_shell.fork_repository(
-      source_project.repository_storage, source_project.disk_path, source_project.full_path,
-      target_project.repository_storage, target_project.disk_path, target_project.full_path)
+    result = gitlab_shell.fork_repository(source_project, target_project)
 
     raise "Unable to fork project #{target_project.id} for repository #{source_project.disk_path} -> #{target_project.disk_path}" unless result
 
diff --git a/lib/gitlab/shell.rb b/lib/gitlab/shell.rb
index a3e0451a8fd..6f896f03947 100644
--- a/lib/gitlab/shell.rb
+++ b/lib/gitlab/shell.rb
@@ -126,20 +126,13 @@ module Gitlab
     end
 
     # Fork repository to new path
-    # forked_from_storage - forked-from project's storage name
-    # forked_from_disk_path - project disk relative path
-    # forked_to_storage - forked-to project's storage name
-    # forked_to_disk_path - forked project disk relative path
-    #
-    # Ex.
-    #  fork_repository("nfs-file06", "gitlab/gitlab-ci", "nfs-file07", "new-namespace/gitlab-ci")
-    def fork_repository(
-          forked_from_storage, forked_from_disk_path, forked_from_project_name,
-          forked_to_storage, forked_to_disk_path, forked_to_project_name)
-      forked_from_relative_path = "#{forked_from_disk_path}.git"
-      fork_args = [forked_to_storage, "#{forked_to_disk_path}.git", forked_to_project_name]
+    # source_project - forked-from Project
+    # target_project - forked-to Project
+    def fork_repository(source_project, target_project)
+      forked_from_relative_path = "#{source_project.disk_path}.git"
+      fork_args = [target_project.repository_storage, "#{target_project.disk_path}.git", target_project.full_path]
 
-      GitalyGitlabProjects.new(forked_from_storage, forked_from_relative_path, forked_from_project_name).fork_repository(*fork_args)
+      GitalyGitlabProjects.new(source_project.repository_storage, forked_from_relative_path, source_project.full_path).fork_repository(*fork_args)
     end
 
     # Removes a repository from file system, using rm_diretory which is an alias
diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb
index 303dad94f2a..033e1bf81a1 100644
--- a/spec/lib/gitlab/shell_spec.rb
+++ b/spec/lib/gitlab/shell_spec.rb
@@ -474,15 +474,10 @@ describe Gitlab::Shell do
     end
 
     describe '#fork_repository' do
+      let(:target_project) { create(:project) }
+
       subject do
-        gitlab_shell.fork_repository(
-          project.repository_storage,
-          project.disk_path,
-          project.full_path,
-          'nfs-file05',
-          'fork/path',
-          'fork/path'
-        )
+        gitlab_shell.fork_repository(project, target_project)
       end
 
       it 'returns true when the command succeeds' do
diff --git a/spec/workers/repository_fork_worker_spec.rb b/spec/workers/repository_fork_worker_spec.rb
index 97210eba127..31bfe88d0bd 100644
--- a/spec/workers/repository_fork_worker_spec.rb
+++ b/spec/workers/repository_fork_worker_spec.rb
@@ -24,14 +24,7 @@ describe RepositoryForkWorker do
       end
 
       def expect_fork_repository
-        expect(shell).to receive(:fork_repository).with(
-          'default',
-          project.disk_path,
-          project.full_path,
-          forked_project.repository_storage,
-          forked_project.disk_path,
-          forked_project.full_path
-        )
+        expect(shell).to receive(:fork_repository).with(project, forked_project)
       end
 
       describe 'when a worker was reset without cleanup' do