Implement OperationService.UserFFMerge

4 files changed, 237 insertions, 9 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7555fb662..ca1622dda 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,8 @@
 
 UNRELEASED
 
+- Implement OperationService.UserFFMerge
+  https://gitlab.com/gitlab-org/gitaly/merge_requests/426
 - Implement WikiFindFile RPC
   https://gitlab.com/gitlab-org/gitaly/merge_requests/425
 - Implement WikiDeletePage RPC
diff --git a/internal/service/operations/merge.go b/internal/service/operations/merge.go
index e20f0e4f1..51be90296 100644
--- a/internal/service/operations/merge.go
+++ b/internal/service/operations/merge.go
@@ -1,12 +1,14 @@
 package operations
 
 import (
+	"fmt"
+
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
 
-	"gitlab.com/gitlab-org/gitaly/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/internal/rubyserver"
-
 	"golang.org/x/net/context"
+	"google.golang.org/grpc"
+	"google.golang.org/grpc/codes"
 )
 
 func (s *server) UserMergeBranch(bidi pb.OperationService_UserMergeBranchServer) error {
@@ -56,6 +58,36 @@ func (s *server) UserMergeBranch(bidi pb.OperationService_UserMergeBranchServer)
 	)
 }
 
-func (s *server) UserFFBranch(_ context.Context, _ *pb.UserFFBranchRequest) (*pb.UserFFBranchResponse, error) {
-	return nil, helper.Unimplemented
+func validateFFRequest(in *pb.UserFFBranchRequest) error {
+	if len(in.Branch) == 0 {
+		return fmt.Errorf("empty branch name")
+	}
+
+	if in.User == nil {
+		return fmt.Errorf("empty user")
+	}
+
+	if in.CommitId == "" {
+		return fmt.Errorf("empty commit id")
+	}
+
+	return nil
+}
+
+func (s *server) UserFFBranch(ctx context.Context, in *pb.UserFFBranchRequest) (*pb.UserFFBranchResponse, error) {
+	if err := validateFFRequest(in); err != nil {
+		return nil, grpc.Errorf(codes.InvalidArgument, "UserFFBranch: %v", err)
+	}
+
+	client, err := s.OperationServiceClient(ctx)
+	if err != nil {
+		return nil, err
+	}
+
+	clientCtx, err := rubyserver.SetHeaders(ctx, in.GetRepository())
+	if err != nil {
+		return nil, err
+	}
+
+	return client.UserFFBranch(clientCtx, in)
 }
diff --git a/internal/service/operations/merge_test.go b/internal/service/operations/merge_test.go
index 9d119cff1..871baad64 100644
--- a/internal/service/operations/merge_test.go
+++ b/internal/service/operations/merge_test.go
@@ -6,10 +6,13 @@ import (
 	"io/ioutil"
 	"os"
 	"os/exec"
+	"path"
 	"strings"
 	"testing"
 	"time"
 
+	"google.golang.org/grpc/codes"
+
 	gitlog "gitlab.com/gitlab-org/gitaly/internal/git/log"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 
@@ -170,6 +173,171 @@ func TestAbortedMerge(t *testing.T) {
 	}
 }
 
+func TestSuccessfulUserFFBranchRequest(t *testing.T) {
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	server, serverSocketPath := runOperationServiceServer(t)
+	defer server.Stop()
+
+	client, conn := newOperationClient(t, serverSocketPath)
+	defer conn.Close()
+
+	commitID := "cfe32cf61b73a0d5e9f13e774abde7ff789b1660"
+	branchName := "test-ff-target-branch"
+	request := &pb.UserFFBranchRequest{
+		Repository: testRepo,
+		CommitId:   commitID,
+		Branch:     []byte(branchName),
+		User:       mergeUser,
+	}
+	expectedResponse := &pb.UserFFBranchResponse{
+		BranchUpdate: &pb.OperationBranchUpdate{
+			RepoCreated:   false,
+			BranchCreated: false,
+			CommitId:      commitID,
+		},
+	}
+
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", "-f", branchName, "6d394385cf567f80a8fd85055db1ab4c5295806f")
+	defer exec.Command("git", "-C", testRepoPath, "branch", "-d", branchName).Run()
+
+	resp, err := client.UserFFBranch(ctx, request)
+	require.NoError(t, err)
+	require.Equal(t, expectedResponse, resp)
+	newBranchHead := testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "rev-parse", branchName)
+	require.Equal(t, commitID, strings.TrimSpace(string(newBranchHead)), "branch head not updated")
+}
+
+func TestFailedUserFFBranchRequest(t *testing.T) {
+	server, serverSocketPath := runOperationServiceServer(t)
+	defer server.Stop()
+
+	client, conn := newOperationClient(t, serverSocketPath)
+	defer conn.Close()
+
+	commitID := "cfe32cf61b73a0d5e9f13e774abde7ff789b1660"
+	branchName := "test-ff-target-branch"
+
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", "-f", branchName, "6d394385cf567f80a8fd85055db1ab4c5295806f")
+	defer exec.Command("git", "-C", testRepoPath, "branch", "-d", branchName).Run()
+
+	testCases := []struct {
+		desc     string
+		user     *pb.User
+		branch   []byte
+		commitID string
+		repo     *pb.Repository
+		code     codes.Code
+	}{
+		{
+			desc:     "empty repository",
+			user:     mergeUser,
+			branch:   []byte(branchName),
+			commitID: commitID,
+			code:     codes.InvalidArgument,
+		},
+		{
+			desc:     "empty user",
+			repo:     testRepo,
+			branch:   []byte(branchName),
+			commitID: commitID,
+			code:     codes.InvalidArgument,
+		},
+		{
+			desc:   "empty commit",
+			repo:   testRepo,
+			user:   mergeUser,
+			branch: []byte(branchName),
+			code:   codes.InvalidArgument,
+		},
+		{
+			desc:     "non-existing commit",
+			repo:     testRepo,
+			user:     mergeUser,
+			branch:   []byte(branchName),
+			commitID: "f001",
+			code:     codes.InvalidArgument,
+		},
+		{
+			desc:     "empty branch",
+			repo:     testRepo,
+			user:     mergeUser,
+			commitID: commitID,
+			code:     codes.InvalidArgument,
+		},
+		{
+			desc:     "non-existing branch",
+			repo:     testRepo,
+			user:     mergeUser,
+			branch:   []byte("this-isnt-real"),
+			commitID: commitID,
+			code:     codes.InvalidArgument,
+		},
+		{
+			desc:     "commit is not a descendant of branch head",
+			repo:     testRepo,
+			user:     mergeUser,
+			branch:   []byte(branchName),
+			commitID: "1a0b36b3cdad1d2ee32457c102a8c0b7056fa863",
+			code:     codes.FailedPrecondition,
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.desc, func(t *testing.T) {
+			ctx, cancel := testhelper.Context()
+			defer cancel()
+
+			request := &pb.UserFFBranchRequest{
+				Repository: testCase.repo,
+				User:       testCase.user,
+				Branch:     testCase.branch,
+				CommitId:   testCase.commitID,
+			}
+			_, err := client.UserFFBranch(ctx, request)
+			testhelper.AssertGrpcError(t, err, testCase.code, "")
+		})
+	}
+}
+
+func TestFailedUserFFBranchDueToHooks(t *testing.T) {
+	server, serverSocketPath := runOperationServiceServer(t)
+	defer server.Stop()
+
+	client, conn := newOperationClient(t, serverSocketPath)
+	defer conn.Close()
+
+	commitID := "cfe32cf61b73a0d5e9f13e774abde7ff789b1660"
+	branchName := "test-ff-target-branch"
+	request := &pb.UserFFBranchRequest{
+		Repository: testRepo,
+		CommitId:   commitID,
+		Branch:     []byte(branchName),
+		User:       mergeUser,
+	}
+
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", "-f", branchName, "6d394385cf567f80a8fd85055db1ab4c5295806f")
+	defer exec.Command("git", "-C", testRepoPath, "branch", "-d", branchName).Run()
+
+	hookContent := []byte("#!/bin/sh\necho 'failure'\nexit 1")
+
+	for _, hookName := range gitlabPreHooks {
+		t.Run(hookName, func(t *testing.T) {
+			hookPath := path.Join(testRepoPath, "hooks", hookName)
+			ioutil.WriteFile(hookPath, hookContent, 0755)
+			defer os.Remove(hookPath)
+
+			ctx, cancel := testhelper.Context()
+			defer cancel()
+
+			resp, err := client.UserFFBranch(ctx, request)
+			require.Nil(t, err)
+			require.Contains(t, resp.PreReceiveError, "failure")
+		})
+	}
+}
+
 func prepareMergeBranch(t *testing.T) {
 	deleteBranch(mergeBranchName)
 	out, err := exec.Command("git", "-C", testRepoPath, "branch", mergeBranchName, mergeBranchHeadBefore).CombinedOutput()
diff --git a/ruby/lib/gitaly_server/operations_service.rb b/ruby/lib/gitaly_server/operations_service.rb
index a20679f99..19c546a8e 100644
--- a/ruby/lib/gitaly_server/operations_service.rb
+++ b/ruby/lib/gitaly_server/operations_service.rb
@@ -122,15 +122,41 @@ module GitalyServer
             end
           end
 
-          branch_update = Gitaly::OperationBranchUpdate.new(
-            commit_id: result.newrev,
-            repo_created: result.repo_created,
-            branch_created: result.branch_created
-          )
+          branch_update = branch_update_result(result)
 
           y << Gitaly::UserMergeBranchResponse.new(branch_update: branch_update)
         end
       end
     end
+
+    def user_ff_branch(request, call)
+      bridge_exceptions do
+        begin
+          repo = Gitlab::Git::Repository.from_call(call)
+          user = Gitlab::Git::User.from_gitaly(request.user)
+
+          result = repo.ff_merge(user, request.commit_id, request.branch)
+          branch_update = branch_update_result(result)
+
+          Gitaly::UserFFBranchResponse.new(branch_update: branch_update)
+        rescue Gitlab::Git::CommitError => e
+          raise GRPC::FailedPrecondition.new(e.to_s)
+        rescue ArgumentError => e
+          raise GRPC::InvalidArgument.new(e.to_s)
+        rescue Gitlab::Git::HooksService::PreReceiveError => e
+          Gitaly::UserFFBranchResponse.new(pre_receive_error: e.message)
+        end
+      end
+    end
+
+    private
+
+    def branch_update_result(gitlab_update_result)
+      Gitaly::OperationBranchUpdate.new(
+        commit_id: gitlab_update_result.newrev,
+        repo_created: gitlab_update_result.repo_created,
+        branch_created: gitlab_update_result.branch_created
+      )
+    end
   end
 end