operations: Encapsulate git2go merge logic into its own function

This moves the code in UserMergeBranch that actually performs the merge
into a new mergeWithGit2Go() function. This way it will be easier to
later implement new merge code that doesn't use git2goExecutor.Merge()
to perform the merge, and remove this code path eventually.

Co-authored-by: John Cai <jcai@gitlab.com>

1 files changed, 46 insertions, 18 deletions

diff --git a/internal/gitaly/service/operations/merge.go b/internal/gitaly/service/operations/merge.go
index 94192a7c4..d1a1f6b8b 100644
--- a/internal/gitaly/service/operations/merge.go
+++ b/internal/gitaly/service/operations/merge.go
@@ -5,10 +5,12 @@ import (
 	"errors"
 	"fmt"
 	"strings"
+	"time"
 
 	"github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus"
 	"github.com/sirupsen/logrus"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/updateref"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git2go"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/hook"
@@ -44,7 +46,39 @@ func validateMergeBranchRequest(request *gitalypb.UserMergeBranchRequest) error
 	return nil
 }
 
-//nolint:revive // This is unintentionally missing documentation.
+func (s *Server) mergeWithGit2Go(
+	ctx context.Context,
+	repoPath string,
+	quarantineRepo *localrepo.Repo,
+	authorName string,
+	authorMail string,
+	authorDate time.Time,
+	message string,
+	ours string,
+	theirs string,
+) (string, error) {
+	mergeResult, err := s.git2goExecutor.Merge(ctx, quarantineRepo, git2go.MergeCommand{
+		Repository: repoPath,
+		AuthorName: authorName,
+		AuthorMail: authorMail,
+		AuthorDate: authorDate,
+		Message:    message,
+		Ours:       ours,
+		Theirs:     theirs,
+	})
+
+	if err != nil {
+		if errors.Is(err, git2go.ErrInvalidArgument) {
+			return "", helper.ErrInvalidArgument(err)
+		}
+
+		return "", err
+	}
+
+	return mergeResult.CommitID, nil
+}
+
+//nolint: revive,stylecheck // This is unintentionally missing documentation.
 func (s *Server) UserMergeBranch(stream gitalypb.OperationService_UserMergeBranchServer) error {
 	ctx := stream.Context()
 
@@ -79,20 +113,14 @@ func (s *Server) UserMergeBranch(stream gitalypb.OperationService_UserMergeBranc
 		return helper.ErrInvalidArgument(err)
 	}
 
-	merge, err := s.git2goExecutor.Merge(ctx, quarantineRepo, git2go.MergeCommand{
-		Repository: repoPath,
-		AuthorName: string(firstRequest.User.Name),
-		AuthorMail: string(firstRequest.User.Email),
-		AuthorDate: authorDate,
-		Message:    string(firstRequest.Message),
-		Ours:       revision.String(),
-		Theirs:     firstRequest.CommitId,
-	})
+	commitID, err := s.mergeWithGit2Go(ctx, repoPath, quarantineRepo,
+		string(firstRequest.User.Name),
+		string(firstRequest.User.Email),
+		authorDate,
+		string(firstRequest.Message),
+		revision.String(),
+		firstRequest.CommitId)
 	if err != nil {
-		if errors.Is(err, git2go.ErrInvalidArgument) {
-			return helper.ErrInvalidArgument(err)
-		}
-
 		var conflictErr git2go.ConflictingFilesError
 		if errors.As(err, &conflictErr) {
 			conflictingFiles := make([][]byte, 0, len(conflictErr.ConflictingFiles))
@@ -121,16 +149,16 @@ func (s *Server) UserMergeBranch(stream gitalypb.OperationService_UserMergeBranc
 			return detailedErr
 		}
 
-		return helper.ErrInternal(err)
+		return err
 	}
 
-	mergeOID, err := git.ObjectHashSHA1.FromHex(merge.CommitID)
+	mergeOID, err := git.ObjectHashSHA1.FromHex(commitID)
 	if err != nil {
 		return helper.ErrInternalf("could not parse merge ID: %w", err)
 	}
 
 	if err := stream.Send(&gitalypb.UserMergeBranchResponse{
-		CommitId: merge.CommitID,
+		CommitId: mergeOID.String(),
 	}); err != nil {
 		return err
 	}
@@ -212,7 +240,7 @@ func (s *Server) UserMergeBranch(stream gitalypb.OperationService_UserMergeBranc
 
 	if err := stream.Send(&gitalypb.UserMergeBranchResponse{
 		BranchUpdate: &gitalypb.OperationBranchUpdate{
-			CommitId:      merge.CommitID,
+			CommitId:      mergeOID.String(),
 			RepoCreated:   false,
 			BranchCreated: false,
 		},