Merge branch '45035-force-commit' into 'master'

Accept Force option for UserCommitFiles to overwrite branch on commit

See merge request gitlab-org/gitaly!1077

7 files changed, 96 insertions, 17 deletions

diff --git a/changelogs/unreleased/45035-force-commit.yml b/changelogs/unreleased/45035-force-commit.yml
new file mode 100644
index 000000000..23928f4a7
--- /dev/null
+++ b/changelogs/unreleased/45035-force-commit.yml
@@ -0,0 +1,5 @@
+---
+title: Accept Force option for UserCommitFiles to overwrite branch on commit
+merge_request: 1077
+author:
+type: added
diff --git a/internal/service/operations/commit_files_test.go b/internal/service/operations/commit_files_test.go
index 3026c431d..e26a05946 100644
--- a/internal/service/operations/commit_files_test.go
+++ b/internal/service/operations/commit_files_test.go
@@ -3,6 +3,7 @@ package operations_test
 import (
 	"fmt"
 	"strconv"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -132,12 +133,6 @@ func TestSuccessfulUserCommitFilesRequest(t *testing.T) {
 	}
 }
 
-func setAuthorAndEmail(headerRequest *gitalypb.UserCommitFilesRequest, authorName, authorEmail []byte) {
-	header := headerRequest.UserCommitFilesRequestPayload.(*gitalypb.UserCommitFilesRequest_Header).Header
-	header.CommitAuthorName = authorName
-	header.CommitAuthorEmail = authorEmail
-}
-
 func TestSuccessfulUserCommitFilesRequestMove(t *testing.T) {
 	server, serverSocketPath := runFullServer(t)
 	defer server.Stop()
@@ -201,6 +196,58 @@ func TestSuccessfulUserCommitFilesRequestMove(t *testing.T) {
 	}
 }
 
+func TestSuccessfulUserCommitFilesRequestForceCommit(t *testing.T) {
+	server, serverSocketPath := runFullServer(t)
+	defer server.Stop()
+
+	client, conn := operations.NewOperationClient(t, serverSocketPath)
+	defer conn.Close()
+
+	testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t)
+	defer cleanupFn()
+
+	ctxOuter, cancel := testhelper.Context()
+	defer cancel()
+
+	md := testhelper.GitalyServersMetadata(t, serverSocketPath)
+	ctx := metadata.NewOutgoingContext(ctxOuter, md)
+	authorName := []byte("Jane Doe")
+	authorEmail := []byte("janedoe@gitlab.com")
+	targetBranchName := "feature"
+	startBranchName := []byte("master")
+
+	startBranchCommit, err := log.GetCommit(ctxOuter, testRepo, string(startBranchName))
+	require.NoError(t, err)
+
+	targetBranchCommit, err := log.GetCommit(ctxOuter, testRepo, targetBranchName)
+	require.NoError(t, err)
+
+	mergeBaseOut := testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "merge-base", targetBranchCommit.Id, startBranchCommit.Id)
+	mergeBaseID := strings.TrimSuffix(string(mergeBaseOut), "\n")
+	require.NotEqual(t, mergeBaseID, targetBranchCommit.Id, "expected %s not to be an ancestor of %s", targetBranchCommit.Id, startBranchCommit.Id)
+
+	headerRequest := headerRequest(testRepo, user, targetBranchName, commitFilesMessage)
+	setAuthorAndEmail(headerRequest, authorName, authorEmail)
+	setStartBranchName(headerRequest, startBranchName)
+	setForce(headerRequest, true)
+
+	stream, err := client.UserCommitFiles(ctx)
+	require.NoError(t, err)
+	require.NoError(t, stream.Send(headerRequest))
+	require.NoError(t, stream.Send(createFileHeaderRequest("TEST.md")))
+	require.NoError(t, stream.Send(actionContentRequest("Test")))
+
+	r, err := stream.CloseAndRecv()
+	require.NoError(t, err)
+
+	update := r.GetBranchUpdate()
+	newTargetBranchCommit, err := log.GetCommit(ctxOuter, testRepo, targetBranchName)
+	require.NoError(t, err)
+
+	require.Equal(t, newTargetBranchCommit.Id, update.CommitId)
+	require.Equal(t, newTargetBranchCommit.ParentIds, []string{startBranchCommit.Id})
+}
+
 func TestFailedUserCommitFilesRequestDueToHooks(t *testing.T) {
 	server, serverSocketPath := runFullServer(t)
 	defer server.Stop()
@@ -383,6 +430,26 @@ func headerRequest(repo *gitalypb.Repository, user *gitalypb.User, branchName st
 	}
 }
 
+func setAuthorAndEmail(headerRequest *gitalypb.UserCommitFilesRequest, authorName, authorEmail []byte) {
+	header := getHeader(headerRequest)
+	header.CommitAuthorName = authorName
+	header.CommitAuthorEmail = authorEmail
+}
+
+func setStartBranchName(headerRequest *gitalypb.UserCommitFilesRequest, startBranchName []byte) {
+	header := getHeader(headerRequest)
+	header.StartBranchName = startBranchName
+}
+
+func setForce(headerRequest *gitalypb.UserCommitFilesRequest, force bool) {
+	header := getHeader(headerRequest)
+	header.Force = force
+}
+
+func getHeader(headerRequest *gitalypb.UserCommitFilesRequest) *gitalypb.UserCommitFilesRequestHeader {
+	return headerRequest.UserCommitFilesRequestPayload.(*gitalypb.UserCommitFilesRequest_Header).Header
+}
+
 func createFileHeaderRequest(filePath string) *gitalypb.UserCommitFilesRequest {
 	return actionRequest(&gitalypb.UserCommitFilesAction{
 		UserCommitFilesActionPayload: &gitalypb.UserCommitFilesAction_Header{
diff --git a/ruby/Gemfile b/ruby/Gemfile
index fa9213a5c..8a8ce5771 100644
--- a/ruby/Gemfile
+++ b/ruby/Gemfile
@@ -6,7 +6,7 @@ gem 'bundler', '>= 1.16.5'
 gem 'rugged', '~> 0.28'
 gem 'github-linguist', '~> 6.1', require: 'linguist'
 gem 'gitlab-markup', '~> 1.6.5'
-gem 'gitaly-proto', '~> 1.12.0'
+gem 'gitaly-proto', '~> 1.13.0'
 gem 'activesupport', '~> 5.0.2'
 gem 'rdoc', '~> 4.2'
 gem 'gitlab-gollum-lib', '~> 4.2', require: false
diff --git a/ruby/Gemfile.lock b/ruby/Gemfile.lock
index 7c1a67447..4deb64c4c 100644
--- a/ruby/Gemfile.lock
+++ b/ruby/Gemfile.lock
@@ -36,7 +36,7 @@ GEM
     ffi (1.10.0)
     gemojione (3.3.0)
       json
-    gitaly-proto (1.12.0)
+    gitaly-proto (1.13.0)
       grpc (~> 1.0)
     github-linguist (6.2.0)
       charlock_holmes (~> 0.7.6)
@@ -64,7 +64,7 @@ GEM
     gollum-grit_adapter (1.0.1)
       gitlab-grit (~> 2.7, >= 2.7.1)
     google-protobuf (3.6.1)
-    googleapis-common-protos-types (1.0.2)
+    googleapis-common-protos-types (1.0.3)
       google-protobuf (~> 3.0)
     grpc (1.15.0)
       google-protobuf (~> 3.1)
@@ -182,7 +182,7 @@ DEPENDENCIES
   bundler (>= 1.16.5)
   factory_bot
   faraday (~> 0.12)
-  gitaly-proto (~> 1.12.0)
+  gitaly-proto (~> 1.13.0)
   github-linguist (~> 6.1)
   gitlab-gollum-lib (~> 4.2)
   gitlab-gollum-rugged_adapter (~> 0.4.4)
diff --git a/ruby/lib/gitaly_server/operations_service.rb b/ruby/lib/gitaly_server/operations_service.rb
index 9e52d06ff..64b52f6fb 100644
--- a/ruby/lib/gitaly_server/operations_service.rb
+++ b/ruby/lib/gitaly_server/operations_service.rb
@@ -321,7 +321,8 @@ module GitalyServer
       optional_fields = {
         start_branch_name: 'start_branch_name',
         author_name: 'commit_author_name',
-        author_email: 'commit_author_email'
+        author_email: 'commit_author_email',
+        force: 'force'
       }.transform_values { |v| header[v].presence }
 
       opts.merge(optional_fields)
diff --git a/ruby/lib/gitlab/git/operation_service.rb b/ruby/lib/gitlab/git/operation_service.rb
index ac6998dc2..74951ade0 100644
--- a/ruby/lib/gitlab/git/operation_service.rb
+++ b/ruby/lib/gitlab/git/operation_service.rb
@@ -77,6 +77,7 @@ module Gitlab
         branch_name,
         start_branch_name: nil,
         start_repository: repository,
+        force: false,
         &block
       )
 
@@ -87,7 +88,7 @@ module Gitlab
 
         raise ArgumentError, "Cannot find branch #{start_branch_name} in #{start_repository.relative_path}" if start_branch_name && !start_repository.branch_exists?(start_branch_name)
 
-        update_branch_with_hooks(branch_name) do
+        update_branch_with_hooks(branch_name, force) do
           repository.with_repo_branch_commit(
             start_repository,
             start_branch_name || branch_name,
@@ -130,7 +131,7 @@ module Gitlab
       private
 
       # Returns [newrev, should_run_after_create, should_run_after_create_branch]
-      def update_branch_with_hooks(branch_name)
+      def update_branch_with_hooks(branch_name, force)
         update_autocrlf_option
 
         was_empty = repository.empty?
@@ -141,7 +142,7 @@ module Gitlab
         raise Gitlab::Git::CommitError.new('Failed to create commit') unless newrev
 
         branch = repository.find_branch(branch_name)
-        oldrev = find_oldrev_from_branch(newrev, branch)
+        oldrev = find_oldrev_from_branch(newrev, branch, force)
 
         ref = Gitlab::Git::BRANCH_REF_PREFIX + branch_name
         update_ref_in_hooks(ref, newrev, oldrev)
@@ -149,11 +150,13 @@ module Gitlab
         BranchUpdate.new(newrev, was_empty, was_empty || Gitlab::Git.blank_ref?(oldrev))
       end
 
-      def find_oldrev_from_branch(newrev, branch)
+      def find_oldrev_from_branch(newrev, branch, force)
         return Gitlab::Git::BLANK_SHA unless branch
 
         oldrev = branch.target
 
+        return oldrev if force
+
         merge_base = repository.merge_base(newrev, branch.target)
         raise Gitlab::Git::Repository::InvalidRef unless merge_base
 
diff --git a/ruby/lib/gitlab/git/repository.rb b/ruby/lib/gitlab/git/repository.rb
index 8576ee8c4..6f4bd9910 100644
--- a/ruby/lib/gitlab/git/repository.rb
+++ b/ruby/lib/gitlab/git/repository.rb
@@ -537,16 +537,18 @@ module Gitlab
         end
       end
 
+      # rubocop:disable Metrics/ParameterLists
       def multi_action(
         user, branch_name:, message:, actions:,
         author_email: nil, author_name: nil,
-        start_branch_name: nil, start_repository: self
+        start_branch_name: nil, start_repository: self, force: false
       )
 
         OperationService.new(user, self).with_branch(
           branch_name,
           start_branch_name: start_branch_name,
-          start_repository: start_repository
+          start_repository: start_repository,
+          force: force
         ) do |start_commit|
 
           index = Gitlab::Git::Index.new(self)
@@ -572,6 +574,7 @@ module Gitlab
           create_commit(options)
         end
       end
+      # rubocop:enable Metrics/ParameterLists
 
       def with_repo_branch_commit(start_repository, start_branch_name)
         Gitlab::Git.check_namespace!(start_repository)