Incorporate feedback

3 files changed, 35 insertions, 55 deletions

diff --git a/internal/service/ref/refs.go b/internal/service/ref/refs.go
index b739279b3..756055f02 100644
--- a/internal/service/ref/refs.go
+++ b/internal/service/ref/refs.go
@@ -10,7 +10,6 @@ import (
 	"github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus"
 
 	log "github.com/sirupsen/logrus"
-	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 
@@ -275,31 +274,22 @@ func (s *server) FindAllBranches(in *pb.FindAllBranchesRequest, stream pb.RefSer
 // which contain the SHA1 passed as argument
 func (*server) ListBranchNamesContainingCommit(in *pb.ListBranchNamesContainingCommitRequest, stream pb.RefService_ListBranchNamesContainingCommitServer) error {
 	if !validCommitID(in.GetCommitId()) {
-		return grpc.Errorf(codes.InvalidArgument, "commit id was not a 40 character hexidecimal")
+		return status.Errorf(codes.InvalidArgument, "commit id was not a 40 character hexidecimal")
 	}
 
-	dirPrefix := "refs/heads/"
-	writer := func(refs [][]byte) error {
-		trimmed := make([][]byte, len(refs)-1)
-		// Filter out empty lines and trim the prefixes
-		// TODO find out why `%(refname:short)` does not work
-		for _, ref := range refs {
-			if len(ref) == 0 {
-				continue
-			}
+	args := []string{fmt.Sprintf("--contains=%s", in.GetCommitId()), "--format=%(refname:strip=2)"}
+	if in.GetLimit() != 0 {
+		args = append(args, fmt.Sprintf("--count=%d", in.GetLimit()))
+	}
 
-			trimmed = append(trimmed, bytes.TrimPrefix(ref, []byte(dirPrefix)))
-		}
-		return stream.Send(&pb.ListBranchNamesContainingCommitResponse{BranchNames: trimmed})
+	writer := func(refs [][]byte) error {
+		return stream.Send(&pb.ListBranchNamesContainingCommitResponse{BranchNames: refs})
 	}
 
-	return findRefs(stream.Context(), writer, in.Repository, []string{dirPrefix},
+	return findRefs(stream.Context(), writer, in.Repository, []string{"refs/heads"},
 		&findRefsOpts{
-			cmdArgs: []string{
-				fmt.Sprintf("--count=%d", in.GetLimit()),
-				fmt.Sprintf("--contains=%s", in.GetCommitId()),
-				"--format=%(refname)"},
-			delim: []byte("\n"),
+			cmdArgs: args,
+			delim:   []byte("\n"),
 		})
 }
 
@@ -307,32 +297,22 @@ func (*server) ListBranchNamesContainingCommit(in *pb.ListBranchNamesContainingC
 // which contain the SHA1 passed as argument
 func (*server) ListTagNamesContainingCommit(in *pb.ListTagNamesContainingCommitRequest, stream pb.RefService_ListTagNamesContainingCommitServer) error {
 	if !validCommitID(in.GetCommitId()) {
-		return grpc.Errorf(codes.InvalidArgument, "commit id was not a 40 character hexidecimal")
+		return status.Errorf(codes.InvalidArgument, "commit id was not a 40 character hexidecimal")
 	}
 
-	dirPrefix := "refs/tags/"
-	writer := func(refs [][]byte) error {
-		trimmed := make([][]byte, len(refs)-1)
-		// Filter out empty lines and trim the prefixes
-		// TODO find out why `%(refname:short)` does not work
-		for _, ref := range refs {
-			if len(ref) == 0 {
-				continue
-			}
+	args := []string{fmt.Sprintf("--contains=%s", in.GetCommitId()), "--format=%(refname:strip=2)"}
+	if in.GetLimit() != 0 {
+		args = append(args, fmt.Sprintf("--count=%d", in.GetLimit()))
+	}
 
-			trimmed = append(trimmed, bytes.TrimPrefix(ref, []byte(dirPrefix)))
-		}
-		return stream.Send(&pb.ListTagNamesContainingCommitResponse{TagNames: trimmed})
+	writer := func(refs [][]byte) error {
+		return stream.Send(&pb.ListTagNamesContainingCommitResponse{TagNames: refs})
 	}
 
-	return findRefs(stream.Context(), writer, in.Repository, []string{dirPrefix},
+	return findRefs(stream.Context(), writer, in.Repository, []string{"refs/tags"},
 		&findRefsOpts{
-			cmdArgs: []string{
-				fmt.Sprintf("--count=%d", in.GetLimit()),
-				fmt.Sprintf("--contains=%s", in.GetCommitId()),
-				"--format=%(refname)",
-			},
-			delim: []byte("\n"),
+			cmdArgs: args,
+			delim:   []byte("\n"),
 		})
 }
 
diff --git a/internal/service/ref/refs_test.go b/internal/service/ref/refs_test.go
index 0f84891ec..74f09d875 100644
--- a/internal/service/ref/refs_test.go
+++ b/internal/service/ref/refs_test.go
@@ -981,7 +981,7 @@ func TestListTagNamesContainingCommit(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.description, func(t *testing.T) {
-			ctx, cancel := context.WithCancel(context.Background())
+			ctx, cancel := testhelper.Context()
 			defer cancel()
 
 			request := &pb.ListTagNamesContainingCommitRequest{Repository: testRepo, CommitId: tc.commitID}
@@ -989,7 +989,7 @@ func TestListTagNamesContainingCommit(t *testing.T) {
 			c, err := client.ListTagNamesContainingCommit(ctx, request)
 			require.NoError(t, err)
 
-			set := make(map[string]bool)
+			var names []string
 			for {
 				r, err := c.Recv()
 				if err == io.EOF {
@@ -1002,15 +1002,13 @@ func TestListTagNamesContainingCommit(t *testing.T) {
 				require.NoError(t, err)
 
 				for _, name := range r.GetTagNames() {
-					set[string(name)] = true
+					names = append(names, string(name))
 				}
 			}
 
 			// Test for inclusion instead of equality because new refs
 			// will get added to the gitlab-test repo over time.
-			for _, name := range tc.tags {
-				require.True(t, set[name], fmt.Sprintf("%s was not found in %v", name, set))
-			}
+			require.Subset(t, names, tc.tags)
 		})
 	}
 }
@@ -1029,6 +1027,7 @@ func TestListBranchNamesContainingCommit(t *testing.T) {
 		description string
 		commitID    string
 		code        codes.Code
+		limit       uint32
 		branches    []string
 	}{
 		{
@@ -1057,6 +1056,13 @@ func TestListBranchNamesContainingCommit(t *testing.T) {
 				"100%branch",
 			},
 		},
+		{
+			description: "init commit",
+			commitID:    "1a0b36b3cdad1d2ee32457c102a8c0b7056fa863",
+			code:        codes.OK,
+			limit:       1,
+			branches:    []string{"'test'"},
+		},
 	}
 
 	for _, tc := range testCases {
@@ -1069,7 +1075,7 @@ func TestListBranchNamesContainingCommit(t *testing.T) {
 			c, err := client.ListBranchNamesContainingCommit(ctx, request)
 			require.NoError(t, err)
 
-			set := make(map[string]bool)
+			var names []string
 			for {
 				r, err := c.Recv()
 				if err == io.EOF {
@@ -1082,15 +1088,13 @@ func TestListBranchNamesContainingCommit(t *testing.T) {
 				require.NoError(t, err)
 
 				for _, name := range r.GetBranchNames() {
-					set[string(name)] = true
+					names = append(names, string(name))
 				}
 			}
 
 			// Test for inclusion instead of equality because new refs
 			// will get added to the gitlab-test repo over time.
-			for _, name := range tc.branches {
-				require.True(t, set[name], fmt.Sprintf("%s was not found in %v", name, set))
-			}
+			require.Subset(t, names, tc.branches)
 		})
 	}
 }
diff --git a/internal/service/repository/server.go b/internal/service/repository/server.go
index d13484d5b..13d859902 100644
--- a/internal/service/repository/server.go
+++ b/internal/service/repository/server.go
@@ -14,7 +14,3 @@ type server struct {
 func NewServer(rs *rubyserver.Server) pb.RepositoryServiceServer {
 	return &server{rs}
 }
-
-func (s *server) CreateRepositoryFromBundle(pb.RepositoryService_CreateRepositoryFromBundleServer) error {
-	return nil
-}