rewrite tag all tags in go

7 files changed, 306 insertions, 19 deletions

diff --git a/changelogs/unreleased/jc-rewrite-get-all-tags.yml b/changelogs/unreleased/jc-rewrite-get-all-tags.yml
new file mode 100644
index 000000000..e2b578829
--- /dev/null
+++ b/changelogs/unreleased/jc-rewrite-get-all-tags.yml
@@ -0,0 +1,5 @@
+---
+title: Rewrite FindAllTags in Go
+merge_request: 1036
+author:
+type: performance
diff --git a/internal/git/catfile/catfile.go b/internal/git/catfile/catfile.go
index a100ed306..75e5924ca 100644
--- a/internal/git/catfile/catfile.go
+++ b/internal/git/catfile/catfile.go
@@ -49,6 +49,12 @@ func (c *Batch) Blob(revspec string) (io.Reader, error) {
 	return c.batch.reader(revspec, "blob")
 }
 
+// Tag returns a raw tag object. Caller must consume the Reader before
+// making another call on C.
+func (c *Batch) Tag(revspec string) (io.Reader, error) {
+	return c.batch.reader(revspec, "tag")
+}
+
 // New returns a new Batch instance. It is important that ctx gets canceled
 // somewhere, because if it doesn't the cat-file processes spawned by
 // New() never terminate.
diff --git a/internal/git/catfile/catfile_test.go b/internal/git/catfile/catfile_test.go
index 386f1d2b0..b0ec2064c 100644
--- a/internal/git/catfile/catfile_test.go
+++ b/internal/git/catfile/catfile_test.go
@@ -113,6 +113,41 @@ func TestCommit(t *testing.T) {
 
 }
 
+func TestTag(t *testing.T) {
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	c, err := New(ctx, testhelper.TestRepository())
+	require.NoError(t, err)
+
+	tagBytes, err := ioutil.ReadFile("testdata/tag-a509fa67c27202a2bc9dd5e014b4af7e6063ac76")
+	require.NoError(t, err)
+
+	testCases := []struct {
+		desc   string
+		spec   string
+		output string
+	}{
+		{
+			desc:   "tag",
+			spec:   "f4e6814c3e4e7a0de82a9e7cd20c626cc963a2f8",
+			output: string(tagBytes),
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			tagReader, err := c.Tag(tc.spec)
+			require.NoError(t, err)
+
+			contents, err := ioutil.ReadAll(tagReader)
+			require.NoError(t, err)
+
+			require.Equal(t, tc.output, string(contents))
+		})
+	}
+}
+
 func TestTree(t *testing.T) {
 	ctx, cancel := testhelper.Context()
 	defer cancel()
diff --git a/internal/git/catfile/testdata/tag-a509fa67c27202a2bc9dd5e014b4af7e6063ac76 b/internal/git/catfile/testdata/tag-a509fa67c27202a2bc9dd5e014b4af7e6063ac76
new file mode 100644
index 000000000..827b6e09a
--- /dev/null
+++ b/internal/git/catfile/testdata/tag-a509fa67c27202a2bc9dd5e014b4af7e6063ac76
@@ -0,0 +1,6 @@
+object 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+type commit
+tag v1.0.0
+tagger Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> 1393491299 +0200
+
+Release
diff --git a/internal/git/log/tag.go b/internal/git/log/tag.go
new file mode 100644
index 000000000..f1de94d7b
--- /dev/null
+++ b/internal/git/log/tag.go
@@ -0,0 +1,105 @@
+package log
+
+import (
+	"bufio"
+	"bytes"
+	"errors"
+	"fmt"
+	"io"
+	"io/ioutil"
+	"strings"
+
+	"gitlab.com/gitlab-org/gitaly-proto/go/gitalypb"
+	"gitlab.com/gitlab-org/gitaly/internal/git/catfile"
+	"gitlab.com/gitlab-org/gitaly/internal/helper"
+)
+
+// GetTagCatfile looks up a commit by revision using an existing *catfile.Batch instance.
+func GetTagCatfile(c *catfile.Batch, tagName string) (*gitalypb.Tag, error) {
+	info, err := c.Info(tagName)
+	if err != nil {
+		return nil, err
+	}
+
+	r, err := c.Tag(info.Oid)
+	if err != nil {
+		return nil, err
+	}
+
+	header, body, err := splitRawTag(r)
+	if err != nil {
+		return nil, err
+	}
+
+	tag, err := buildAnnotatedTag(c, info.Oid, tagName, header, body)
+	if err != nil {
+		return nil, err
+	}
+
+	return tag, nil
+}
+
+type tagHeader struct {
+	oid     string
+	tagType string
+}
+
+func splitRawTag(r io.Reader) (*tagHeader, []byte, error) {
+	raw, err := ioutil.ReadAll(r)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	split := bytes.SplitN(raw, []byte("\n\n"), 2)
+	if len(split) != 2 {
+		return nil, nil, errors.New("invalid tag object")
+	}
+
+	// Remove trailing newline, if any, to preserve existing behavior the old GitLab tag finding code.
+	// See https://gitlab.com/gitlab-org/gitaly/blob/5e94dc966ac1900c11794b107a77496552591f9b/ruby/lib/gitlab/git/repository.rb#L211.
+	// Maybe this belongs in the FindAllTags handler, or even on the gitlab-ce client side, instead of here?
+	body := bytes.TrimRight(split[1], "\n")
+
+	var header tagHeader
+	s := bufio.NewScanner(bytes.NewReader(split[0]))
+	for s.Scan() {
+		headerSplit := strings.SplitN(s.Text(), " ", 2)
+		if len(headerSplit) != 2 {
+			continue
+		}
+
+		key, value := headerSplit[0], headerSplit[1]
+		switch key {
+		case "object":
+			header.oid = value
+		case "type":
+			header.tagType = value
+		}
+	}
+
+	return &header, body, nil
+}
+
+func buildAnnotatedTag(b *catfile.Batch, tagID, name string, header *tagHeader, body []byte) (*gitalypb.Tag, error) {
+	tag := &gitalypb.Tag{
+		Id:          tagID,
+		Name:        []byte(name),
+		MessageSize: int64(len(body)),
+		Message:     body,
+	}
+
+	if max := helper.MaxCommitOrTagMessageSize; len(body) > max {
+		tag.Message = tag.Message[:max]
+	}
+
+	if header.tagType == "commit" {
+		commit, err := GetCommitCatfile(b, header.oid)
+		if err != nil {
+			return nil, fmt.Errorf("buildAnnotatedTag error when getting target commit: %v", err)
+		}
+
+		tag.TargetCommit = commit
+	}
+
+	return tag, nil
+}
diff --git a/internal/git/log/tag_test.go b/internal/git/log/tag_test.go
new file mode 100644
index 000000000..bfce2a361
--- /dev/null
+++ b/internal/git/log/tag_test.go
@@ -0,0 +1,55 @@
+package log
+
+import (
+	"fmt"
+	"strings"
+	"testing"
+
+	"gitlab.com/gitlab-org/gitaly/internal/git/catfile"
+	"gitlab.com/gitlab-org/gitaly/internal/helper"
+
+	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
+)
+
+func TestGetTag(t *testing.T) {
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	testRepo, testRepoPath, cleanup := testhelper.NewTestRepo(t)
+	defer cleanup()
+
+	testCases := []struct {
+		tagName string
+		rev     string
+		message string
+	}{
+		{
+			tagName: fmt.Sprintf("%s-v1.0.0", t.Name()),
+			rev:     "master^^^",
+			message: "Prod Release v1.0.0",
+		},
+		{
+			tagName: fmt.Sprintf("%s-v1.0.1", t.Name()),
+			rev:     "master^^",
+			message: strings.Repeat("a", helper.MaxCommitOrTagMessageSize+1),
+		},
+	}
+
+	c, err := catfile.New(ctx, testRepo)
+	require.NoError(t, err)
+	for _, testCase := range testCases {
+		t.Run(testCase.tagName, func(t *testing.T) {
+			testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "tag", "-m", testCase.message, testCase.tagName, testCase.rev)
+
+			tag, err := GetTagCatfile(c, testCase.tagName)
+			require.NoError(t, err)
+			if len(testCase.message) >= helper.MaxCommitOrTagMessageSize {
+				testCase.message = testCase.message[:helper.MaxCommitOrTagMessageSize]
+			}
+
+			require.Equal(t, testCase.message, string(tag.Message))
+			require.Equal(t, testCase.tagName, string(tag.GetName()))
+		})
+	}
+}
diff --git a/internal/service/ref/refs.go b/internal/service/ref/refs.go
index 4f5e9c569..1aab6f40d 100644
--- a/internal/service/ref/refs.go
+++ b/internal/service/ref/refs.go
@@ -4,15 +4,17 @@ import (
 	"bufio"
 	"bytes"
 	"context"
+	"errors"
 	"fmt"
 	"strings"
 
 	"gitlab.com/gitlab-org/gitaly-proto/go/gitalypb"
 	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/git/catfile"
+	gitlog "gitlab.com/gitlab-org/gitaly/internal/git/log"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
+	"gitlab.com/gitlab-org/gitaly/internal/helper/chunk"
 	"gitlab.com/gitlab-org/gitaly/internal/helper/lines"
-	"gitlab.com/gitlab-org/gitaly/internal/rubyserver"
 )
 
 var (
@@ -52,33 +54,106 @@ func findRefs(ctx context.Context, writer lines.Sender, repo *gitalypb.Repositor
 	return cmd.Wait()
 }
 
-func (s *server) FindAllTags(in *gitalypb.FindAllTagsRequest, stream gitalypb.RefService_FindAllTagsServer) error {
-	ctx := stream.Context()
+type tagSender struct {
+	tags   []*gitalypb.Tag
+	stream gitalypb.RefService_FindAllTagsServer
+}
 
-	client, err := s.RefServiceClient(ctx)
-	if err != nil {
-		return err
-	}
+func (t *tagSender) Reset() {
+	t.tags = nil
+}
 
-	clientCtx, err := rubyserver.SetHeaders(ctx, in.GetRepository())
+func (t *tagSender) Append(i chunk.Item) {
+	t.tags = append(t.tags, i.(*gitalypb.Tag))
+}
+
+func (t *tagSender) Send() error {
+	return t.stream.Send(&gitalypb.FindAllTagsResponse{
+		Tags: t.tags,
+	})
+}
+
+func parseAndReturnTags(ctx context.Context, repo *gitalypb.Repository, stream gitalypb.RefService_FindAllTagsServer) error {
+	tagsCmd, err := git.Command(ctx, repo, "for-each-ref", "--format", "%(objectname) %(objecttype) %(refname:lstrip=2)", "refs/tags/")
 	if err != nil {
-		return err
+		return fmt.Errorf("for-each-ref error: %v", err)
 	}
 
-	rubyStream, err := client.FindAllTags(clientCtx, in)
+	c, err := catfile.New(ctx, repo)
 	if err != nil {
-		return err
+		return fmt.Errorf("error creating catfile: %v", err)
 	}
 
-	return rubyserver.Proxy(func() error {
-		resp, err := rubyStream.Recv()
-		if err != nil {
-			md := rubyStream.Trailer()
-			stream.SetTrailer(md)
-			return err
+	tagChunker := chunk.New(&tagSender{stream: stream})
+
+	scanner := bufio.NewScanner(tagsCmd)
+	for scanner.Scan() {
+		fields := strings.SplitN(scanner.Text(), " ", 3)
+		if len(fields) != 3 {
+			return fmt.Errorf("invalid output from for-each-ref command: %v", scanner.Text())
 		}
-		return stream.Send(resp)
-	})
+
+		tagID, refType, refName := fields[0], fields[1], fields[2]
+
+		tag := &gitalypb.Tag{
+			Id:   tagID,
+			Name: []byte(refName),
+		}
+		switch refType {
+		// annotated tag
+		case "tag":
+			tag, err = gitlog.GetTagCatfile(c, refName)
+			if err != nil {
+				return fmt.Errorf("getting annotated tag: %v", err)
+			}
+		case "commit":
+			commit, err := gitlog.GetCommitCatfile(c, tagID)
+			if err != nil {
+				return fmt.Errorf("getting commit catfile: %v", err)
+			}
+			tag.TargetCommit = commit
+		}
+
+		if err := tagChunker.Send(tag); err != nil {
+			return fmt.Errorf("sending to chunker: %v", err)
+		}
+	}
+
+	if err := tagsCmd.Wait(); err != nil {
+		return fmt.Errorf("tag command: %v", err)
+	}
+
+	if err := tagChunker.Flush(); err != nil {
+		return fmt.Errorf("flushing chunker: %v", err)
+	}
+
+	return nil
+}
+
+func (s *server) FindAllTags(in *gitalypb.FindAllTagsRequest, stream gitalypb.RefService_FindAllTagsServer) error {
+	ctx := stream.Context()
+
+	if err := validateFindAllTagsRequest(in); err != nil {
+		return helper.ErrInvalidArgument(err)
+	}
+
+	if err := parseAndReturnTags(ctx, in.GetRepository(), stream); err != nil {
+		return helper.ErrInternal(err)
+	}
+
+	return nil
+}
+
+func validateFindAllTagsRequest(request *gitalypb.FindAllTagsRequest) error {
+	if request.GetRepository() == nil {
+		return errors.New("empty Repository")
+	}
+
+	if _, err := helper.GetRepoPath(request.GetRepository()); err != nil {
+		return fmt.Errorf("invalid git directory: %v", err)
+	}
+
+	return nil
 }
 
 func _findBranchNames(ctx context.Context, repo *gitalypb.Repository) ([][]byte, error) {