Implement RefService.FindAllRemoteBranches RPC

6 files changed, 240 insertions, 12 deletions

diff --git a/changelogs/unreleased/remote-branches.yml b/changelogs/unreleased/remote-branches.yml
new file mode 100644
index 000000000..8f1327d7d
--- /dev/null
+++ b/changelogs/unreleased/remote-branches.yml
@@ -0,0 +1,5 @@
+---
+title: Implement RefService.FindAllRemoteBranches RPC
+merge_request: 799
+author:
+type: added
diff --git a/internal/service/ref/refs_test.go b/internal/service/ref/refs_test.go
index 076606ff2..f9fa0fd35 100644
--- a/internal/service/ref/refs_test.go
+++ b/internal/service/ref/refs_test.go
@@ -736,11 +736,6 @@ func TestEmptyFindLocalBranchesRequest(t *testing.T) {
 	}
 }
 
-func createRemoteBranch(t *testing.T, repoPath, remoteName, branchName, ref string) {
-	testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "update-ref",
-		"refs/remotes/"+remoteName+"/"+branchName, ref)
-}
-
 func TestSuccessfulFindAllBranchesRequest(t *testing.T) {
 	server, serverSocketPath := runRefServiceServer(t)
 	defer server.Stop()
@@ -769,8 +764,8 @@ func TestSuccessfulFindAllBranchesRequest(t *testing.T) {
 	testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t)
 	defer cleanupFn()
 
-	createRemoteBranch(t, testRepoPath, "origin", "fake-remote-branch",
-		remoteBranch.Target.Id)
+	testhelper.CreateRemoteBranch(t, testRepoPath, "origin",
+		"fake-remote-branch", remoteBranch.Target.Id)
 
 	request := &pb.FindAllBranchesRequest{Repository: testRepo}
 	client, conn := newRefServiceClient(t, serverSocketPath)
diff --git a/internal/service/ref/remote_branches.go b/internal/service/ref/remote_branches.go
index 525d34efa..82162aa4f 100644
--- a/internal/service/ref/remote_branches.go
+++ b/internal/service/ref/remote_branches.go
@@ -1,10 +1,48 @@
 package ref
 
 import (
+	"fmt"
+	"strings"
+
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
-	"gitlab.com/gitlab-org/gitaly/internal/helper"
+	"gitlab.com/gitlab-org/gitaly/internal/git/catfile"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 )
 
-func (s *server) FindAllRemoteBranches(in *pb.FindAllRemoteBranchesRequest, stream pb.RefService_FindAllRemoteBranchesServer) error {
-	return helper.Unimplemented
+func (s *server) FindAllRemoteBranches(req *pb.FindAllRemoteBranchesRequest, stream pb.RefService_FindAllRemoteBranchesServer) error {
+	if err := validateFindAllRemoteBranchesRequest(req); err != nil {
+		return status.Errorf(codes.InvalidArgument, "FindAllRemoteBranches: %v", err)
+	}
+
+	args := []string{
+		"--format=" + strings.Join(localBranchFormatFields, "%00"),
+	}
+
+	patterns := []string{"refs/remotes/" + req.GetRemoteName()}
+
+	ctx := stream.Context()
+	c, err := catfile.New(ctx, req.GetRepository())
+	if err != nil {
+		return err
+	}
+
+	opts := &findRefsOpts{
+		cmdArgs: args,
+	}
+	writer := newFindAllRemoteBranchesWriter(stream, c)
+
+	return findRefs(ctx, writer, req.GetRepository(), patterns, opts)
+}
+
+func validateFindAllRemoteBranchesRequest(req *pb.FindAllRemoteBranchesRequest) error {
+	if req.GetRepository() == nil {
+		return fmt.Errorf("empty Repository")
+	}
+
+	if len(req.GetRemoteName()) == 0 {
+		return fmt.Errorf("empty RemoteName")
+	}
+
+	return nil
 }
diff --git a/internal/service/ref/remote_branches_test.go b/internal/service/ref/remote_branches_test.go
new file mode 100644
index 000000000..d9c2bffd0
--- /dev/null
+++ b/internal/service/ref/remote_branches_test.go
@@ -0,0 +1,147 @@
+package ref
+
+import (
+	"io"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	pb "gitlab.com/gitlab-org/gitaly-proto/go"
+	"gitlab.com/gitlab-org/gitaly/internal/git/log"
+	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
+	"golang.org/x/net/context"
+	"google.golang.org/grpc/codes"
+)
+
+func TestSuccessfulFindAllRemoteBranchesRequest(t *testing.T) {
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	server, serverSocketPath := runRefServiceServer(t)
+	defer server.Stop()
+
+	client, conn := newRefServiceClient(t, serverSocketPath)
+	defer conn.Close()
+
+	testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t)
+	defer cleanupFn()
+
+	remoteName := "my-remote"
+	expectedBranches := map[string]string{
+		"foo": "c7fbe50c7c7419d9701eebe64b1fdacc3df5b9dd",
+		"bar": "60ecb67744cb56576c30214ff52294f8ce2def98",
+	}
+	excludedRemote := "my-remote-2"
+	excludedBranches := map[string]string{
+		"from-another-remote": "5937ac0a7beb003549fc5fd26fc247adbce4a52e",
+	}
+
+	for branchName, commitID := range expectedBranches {
+		testhelper.CreateRemoteBranch(t, testRepoPath, remoteName, branchName, commitID)
+	}
+
+	for branchName, commitID := range excludedBranches {
+		testhelper.CreateRemoteBranch(t, testRepoPath, excludedRemote, branchName, commitID)
+	}
+
+	request := &pb.FindAllRemoteBranchesRequest{Repository: testRepo, RemoteName: remoteName}
+
+	c, err := client.FindAllRemoteBranches(ctx, request)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	branches := readFindAllRemoteBranchesResponsesFromClient(t, c)
+	require.Len(t, branches, len(expectedBranches))
+
+	for branchName, commitID := range expectedBranches {
+		targetCommit, err := log.GetCommit(ctx, testRepo, commitID)
+		require.NoError(t, err)
+
+		expectedBranch := &pb.Branch{
+			Name:         []byte("refs/remotes/" + remoteName + "/" + branchName),
+			TargetCommit: targetCommit,
+		}
+
+		require.Contains(t, branches, expectedBranch)
+	}
+
+	for branchName, commitID := range excludedBranches {
+		targetCommit, err := log.GetCommit(ctx, testRepo, commitID)
+		require.NoError(t, err)
+
+		excludedBranch := &pb.Branch{
+			Name:         []byte("refs/remotes/" + excludedRemote + "/" + branchName),
+			TargetCommit: targetCommit,
+		}
+
+		require.NotContains(t, branches, excludedBranch)
+	}
+}
+
+func TestInvalidFindAllRemoteBranchesRequest(t *testing.T) {
+	server, serverSocketPath := runRefServiceServer(t)
+	defer server.Stop()
+
+	client, conn := newRefServiceClient(t, serverSocketPath)
+	defer conn.Close()
+
+	testRepo, _, cleanupFn := testhelper.NewTestRepo(t)
+	defer cleanupFn()
+
+	testCases := []struct {
+		description string
+		request     pb.FindAllRemoteBranchesRequest
+	}{
+		{
+			description: "Invalid repo",
+			request: pb.FindAllRemoteBranchesRequest{
+				Repository: &pb.Repository{
+					StorageName:  "fake",
+					RelativePath: "repo",
+				},
+			},
+		},
+		{
+			description: "Empty repo",
+			request:     pb.FindAllRemoteBranchesRequest{RemoteName: "myRemote"},
+		},
+		{
+			description: "Empty remote name",
+			request:     pb.FindAllRemoteBranchesRequest{Repository: testRepo},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.description, func(t *testing.T) {
+			ctx, cancel := context.WithCancel(context.Background())
+			defer cancel()
+			c, err := client.FindAllRemoteBranches(ctx, &tc.request)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			var recvError error
+			for recvError == nil {
+				_, recvError = c.Recv()
+			}
+
+			testhelper.RequireGrpcError(t, recvError, codes.InvalidArgument)
+		})
+	}
+}
+
+func readFindAllRemoteBranchesResponsesFromClient(t *testing.T, c pb.RefService_FindAllRemoteBranchesClient) []*pb.Branch {
+	var branches []*pb.Branch
+
+	for {
+		r, err := c.Recv()
+		if err == io.EOF {
+			break
+		}
+		require.NoError(t, err)
+
+		branches = append(branches, r.GetBranches()...)
+	}
+
+	return branches
+}
diff --git a/internal/service/ref/util.go b/internal/service/ref/util.go
index 6d328e615..24d679521 100644
--- a/internal/service/ref/util.go
+++ b/internal/service/ref/util.go
@@ -47,7 +47,7 @@ func buildLocalBranch(name []byte, target *pb.GitCommit) *pb.FindLocalBranchResp
 	return response
 }
 
-func buildBranch(c *catfile.Batch, elements [][]byte) (*pb.FindAllBranchesResponse_Branch, error) {
+func buildAllBranchesBranch(c *catfile.Batch, elements [][]byte) (*pb.FindAllBranchesResponse_Branch, error) {
 	target, err := log.GetCommitCatfile(c, string(elements[1]))
 	if err != nil {
 		return nil, err
@@ -59,6 +59,18 @@ func buildBranch(c *catfile.Batch, elements [][]byte) (*pb.FindAllBranchesRespon
 	}, nil
 }
 
+func buildBranch(c *catfile.Batch, elements [][]byte) (*pb.Branch, error) {
+	target, err := log.GetCommitCatfile(c, string(elements[1]))
+	if err != nil {
+		return nil, err
+	}
+
+	return &pb.Branch{
+		Name:         elements[0],
+		TargetCommit: target,
+	}, nil
+}
+
 func newFindAllBranchNamesWriter(stream pb.Ref_FindAllBranchNamesServer) lines.Sender {
 	return func(refs [][]byte) error {
 		return stream.Send(&pb.FindAllBranchNamesResponse{Names: refs})
@@ -101,7 +113,7 @@ func newFindAllBranchesWriter(stream pb.RefService_FindAllBranchesServer, c *cat
 			if err != nil {
 				return err
 			}
-			branch, err := buildBranch(c, elements)
+			branch, err := buildAllBranchesBranch(c, elements)
 			if err != nil {
 				return err
 			}
@@ -110,3 +122,23 @@ func newFindAllBranchesWriter(stream pb.RefService_FindAllBranchesServer, c *cat
 		return stream.Send(&pb.FindAllBranchesResponse{Branches: branches})
 	}
 }
+
+func newFindAllRemoteBranchesWriter(stream pb.RefService_FindAllRemoteBranchesServer, c *catfile.Batch) lines.Sender {
+	return func(refs [][]byte) error {
+		var branches []*pb.Branch
+
+		for _, ref := range refs {
+			elements, err := parseRef(ref)
+			if err != nil {
+				return err
+			}
+			branch, err := buildBranch(c, elements)
+			if err != nil {
+				return err
+			}
+			branches = append(branches, branch)
+		}
+
+		return stream.Send(&pb.FindAllRemoteBranchesResponse{Branches: branches})
+	}
+}
diff --git a/internal/testhelper/branch.go b/internal/testhelper/branch.go
new file mode 100644
index 000000000..c4ac2e557
--- /dev/null
+++ b/internal/testhelper/branch.go
@@ -0,0 +1,11 @@
+package testhelper
+
+import (
+	"testing"
+)
+
+// CreateRemoteBranch creates a new remote branch
+func CreateRemoteBranch(t *testing.T, repoPath, remoteName, branchName, ref string) {
+	MustRunCommand(t, nil, "git", "-C", repoPath, "update-ref",
+		"refs/remotes/"+remoteName+"/"+branchName, ref)
+}