Move push_remote_branches from Gitlab::Shell to Gitlab::Git::Repository

4 files changed, 56 insertions, 52 deletions

diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index 37d67b6052c..e159a795960 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -21,6 +21,7 @@ module Gitlab
       REBASE_WORKTREE_PREFIX = 'rebase'.freeze
       SQUASH_WORKTREE_PREFIX = 'squash'.freeze
       GITALY_INTERNAL_URL = 'ssh://gitaly/internal.git'.freeze
+      GITLAB_PROJECTS_TIMEOUT = Gitlab.config.gitlab_shell.git_timeout
 
       NoRepository = Class.new(StandardError)
       InvalidBlobName = Class.new(StandardError)
@@ -83,7 +84,7 @@ module Gitlab
       # Rugged repo object
       attr_reader :rugged
 
-      attr_reader :storage, :gl_repository, :relative_path
+      attr_reader :gitlab_projects, :storage, :gl_repository, :relative_path
 
       # This initializer method is only used on the client side (gitlab-ce).
       # Gitaly-ruby uses a different initializer.
@@ -93,6 +94,12 @@ module Gitlab
         @gl_repository = gl_repository
 
         storage_path = Gitlab.config.repositories.storages[@storage]['path']
+        @gitlab_projects = Gitlab::Git::GitlabProjects.new(
+          storage_path,
+          relative_path,
+          global_hooks_path: Gitlab.config.gitlab_shell.hooks_path,
+          logger: Rails.logger
+        )
         @path = File.join(storage_path, @relative_path)
         @name = @relative_path.split("/").last
         @attributes = Gitlab::Git::Attributes.new(path)
@@ -1266,6 +1273,12 @@ module Gitlab
         fresh_worktree?(worktree_path(SQUASH_WORKTREE_PREFIX, squash_id))
       end
 
+      def push_remote_branches(remote_name, branch_names, forced: true)
+        success = @gitlab_projects.push_branches(remote_name, GITLAB_PROJECTS_TIMEOUT, forced, branch_names)
+
+        success || gitlab_projects_error
+      end
+
       def gitaly_repository
         Gitlab::GitalyClient::Util.repository(@storage, @relative_path, @gl_repository)
       end
@@ -1943,6 +1956,10 @@ module Gitlab
       def fetch_remote(remote_name = 'origin', env: nil)
         run_git(['fetch', remote_name], env: env).last.zero?
       end
+
+      def gitlab_projects_error
+        raise CommandError, @gitlab_projects.output
+      end
     end
   end
 end
diff --git a/lib/gitlab/shell.rb b/lib/gitlab/shell.rb
index 9cdd3d22f18..8dbe8ba5df0 100644
--- a/lib/gitlab/shell.rb
+++ b/lib/gitlab/shell.rb
@@ -306,27 +306,6 @@ module Gitlab
       end
     end
 
-    # Push branch to remote repository
-    #
-    # storage - project's storage path
-    # project_name - project's disk path
-    # remote_name - remote name
-    # branch_names - remote branch names to push
-    # forced - should we use --force flag
-    #
-    # Ex.
-    #   push_remote_branches('/path/to/storage', 'gitlab-org/gitlab-test' 'upstream', ['feature'])
-    #
-    def push_remote_branches(storage, project_name, remote_name, branch_names, forced: true)
-      cmd = gitlab_projects(storage, "#{project_name}.git")
-
-      success = cmd.push_branches(remote_name, git_timeout, forced, branch_names)
-
-      raise Error, cmd.output unless success
-
-      success
-    end
-
     # Delete branch from remote repository
     #
     # storage - project's storage path
diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb
index 0e4292026df..12ec7b1fdc4 100644
--- a/spec/lib/gitlab/git/repository_spec.rb
+++ b/spec/lib/gitlab/git/repository_spec.rb
@@ -18,9 +18,10 @@ describe Gitlab::Git::Repository, seed_helper: true do
   end
 
   let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') }
+  let(:storage_path) { TestEnv.repos_path }
 
   describe '.create_hooks' do
-    let(:repo_path) { File.join(TestEnv.repos_path, 'hook-test.git') }
+    let(:repo_path) { File.join(storage_path, 'hook-test.git') }
     let(:hooks_dir) { File.join(repo_path, 'hooks') }
     let(:target_hooks_dir) { Gitlab.config.gitlab_shell.hooks_path }
     let(:existing_target) { File.join(repo_path, 'foobar') }
@@ -645,7 +646,7 @@ describe Gitlab::Git::Repository, seed_helper: true do
     end
 
     after do
-      Gitlab::Shell.new.remove_repository(TestEnv.repos_path, 'my_project')
+      Gitlab::Shell.new.remove_repository(storage_path, 'my_project')
     end
 
     it 'fetches a repository as a mirror remote' do
@@ -1884,6 +1885,41 @@ describe Gitlab::Git::Repository, seed_helper: true do
     end
   end
 
+  describe '#gitlab_projects' do
+    subject { repository.gitlab_projects }
+
+    it { expect(subject.shard_path).to eq(storage_path) }
+    it { expect(subject.repository_relative_path).to eq(repository.relative_path) }
+  end
+
+  context 'gitlab_projects commands' do
+    let(:gitlab_projects) { repository.gitlab_projects }
+    let(:timeout) { Gitlab.config.gitlab_shell.git_timeout }
+
+    describe '#push_remote_branches' do
+      subject do
+        repository.push_remote_branches('downstream-remote', ['master'])
+      end
+
+      it 'executes the command' do
+        expect(gitlab_projects).to receive(:push_branches)
+          .with('downstream-remote', timeout, true, ['master'])
+          .and_return(true)
+
+        is_expected.to be_truthy
+      end
+
+      it 'raises an error if the command fails' do
+        allow(gitlab_projects).to receive(:output) { 'error' }
+        expect(gitlab_projects).to receive(:push_branches)
+          .with('downstream-remote', timeout, true, ['master'])
+          .and_return(false)
+
+        expect { subject }.to raise_error(Gitlab::Git::CommandError, 'error')
+      end
+    end
+  end
+
   def create_remote_branch(repository, remote_name, branch_name, source_branch_name)
     source_branch = repository.branches.find { |branch| branch.name == source_branch_name }
     rugged = repository.rugged
diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb
index dd779b04741..7c2c02505b3 100644
--- a/spec/lib/gitlab/shell_spec.rb
+++ b/spec/lib/gitlab/shell_spec.rb
@@ -348,34 +348,6 @@ describe Gitlab::Shell do
       end
     end
 
-    describe '#push_remote_branches' do
-      subject(:result) do
-        gitlab_shell.push_remote_branches(
-          project.repository_storage_path,
-          project.disk_path,
-          'downstream-remote',
-          ['master']
-        )
-      end
-
-      it 'executes the command' do
-        expect(gitlab_projects).to receive(:push_branches)
-          .with('downstream-remote', timeout, true, ['master'])
-          .and_return(true)
-
-        is_expected.to be_truthy
-      end
-
-      it 'fails to execute the command' do
-        allow(gitlab_projects).to receive(:output) { 'error' }
-        expect(gitlab_projects).to receive(:push_branches)
-          .with('downstream-remote', timeout, true, ['master'])
-          .and_return(false)
-
-        expect { result }.to raise_error(Gitlab::Shell::Error, 'error')
-      end
-    end
-
     describe '#delete_remote_branches' do
       subject(:result) do
         gitlab_shell.delete_remote_branches(