Port GetTagMessages from Gitaly-Ruby to Gitaly

- feature toggle 'get_tag_messages_go'
- go implementation
Closes: #2123

2 files changed, 41 insertions, 14 deletions

diff --git a/internal/git/log/tag.go b/internal/git/log/tag.go
index 52c340673..9750aca57 100644
--- a/internal/git/log/tag.go
+++ b/internal/git/log/tag.go
@@ -19,21 +19,23 @@ const (
 )
 
 // GetTagCatfile looks up a commit by tagID using an existing *catfile.Batch instance.
+// When 'trim' is 'true', the tag message will be trimmed to fit in a gRPC message.
+// When 'trimRightNewLine' is 'true', the tag message will be trimmed to remove all '\n' characters from right.
 // note: we pass in the tagName because the tag name from refs/tags may be different
 // than the name found in the actual tag object. We want to use the tagName found in refs/tags
-func GetTagCatfile(c *catfile.Batch, tagID, tagName string) (*gitalypb.Tag, error) {
+func GetTagCatfile(c *catfile.Batch, tagID, tagName string, trimLen, trimRightNewLine bool) (*gitalypb.Tag, error) {
 	r, err := c.Tag(tagID)
 	if err != nil {
 		return nil, err
 	}
 
-	header, body, err := splitRawTag(r)
+	header, body, err := splitRawTag(r, trimRightNewLine)
 	if err != nil {
 		return nil, err
 	}
 
 	// the tagID is the oid of the tag object
-	tag, err := buildAnnotatedTag(c, tagID, tagName, header, body)
+	tag, err := buildAnnotatedTag(c, tagID, tagName, header, body, trimLen, trimRightNewLine)
 	if err != nil {
 		return nil, err
 	}
@@ -48,7 +50,7 @@ type tagHeader struct {
 	tagger  string
 }
 
-func splitRawTag(r io.Reader) (*tagHeader, []byte, error) {
+func splitRawTag(r io.Reader, trimRightNewLine bool) (*tagHeader, []byte, error) {
 	raw, err := ioutil.ReadAll(r)
 	if err != nil {
 		return nil, nil, err
@@ -57,10 +59,13 @@ func splitRawTag(r io.Reader) (*tagHeader, []byte, error) {
 	var body []byte
 	split := bytes.SplitN(raw, []byte("\n\n"), 2)
 	if len(split) == 2 {
-		// 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")
+		body = split[1]
+		if trimRightNewLine {
+			// 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(body, "\n")
+		}
 	}
 
 	var header tagHeader
@@ -87,7 +92,7 @@ func splitRawTag(r io.Reader) (*tagHeader, []byte, error) {
 	return &header, body, nil
 }
 
-func buildAnnotatedTag(b *catfile.Batch, tagID, name string, header *tagHeader, body []byte) (*gitalypb.Tag, error) {
+func buildAnnotatedTag(b *catfile.Batch, tagID, name string, header *tagHeader, body []byte, trimLen, trimRightNewLine bool) (*gitalypb.Tag, error) {
 	tag := &gitalypb.Tag{
 		Id:          tagID,
 		Name:        []byte(name),
@@ -95,7 +100,7 @@ func buildAnnotatedTag(b *catfile.Batch, tagID, name string, header *tagHeader,
 		Message:     body,
 	}
 
-	if max := helper.MaxCommitOrTagMessageSize; len(body) > max {
+	if max := helper.MaxCommitOrTagMessageSize; trimLen && len(body) > max {
 		tag.Message = tag.Message[:max]
 	}
 
@@ -138,7 +143,7 @@ func dereferenceTag(b *catfile.Batch, Oid string) (*gitalypb.GitCommit, error) {
 				return nil, err
 			}
 
-			header, _, err := splitRawTag(r)
+			header, _, err := splitRawTag(r, true)
 			if err != nil {
 				return nil, err
 			}
diff --git a/internal/git/log/tag_test.go b/internal/git/log/tag_test.go
index daa8540b4..92750d693 100644
--- a/internal/git/log/tag_test.go
+++ b/internal/git/log/tag_test.go
@@ -25,16 +25,25 @@ func TestGetTag(t *testing.T) {
 		tagName string
 		rev     string
 		message string
+		trim    bool
 	}{
 		{
+			tagName: fmt.Sprintf("%s-v1.0.2", t.Name()),
+			rev:     "master^^^^",
+			message: strings.Repeat("a", helper.MaxCommitOrTagMessageSize+1),
+			trim:    false,
+		},
+		{
 			tagName: fmt.Sprintf("%s-v1.0.0", t.Name()),
 			rev:     "master^^^",
 			message: "Prod Release v1.0.0",
+			trim:    true,
 		},
 		{
 			tagName: fmt.Sprintf("%s-v1.0.1", t.Name()),
 			rev:     "master^^",
 			message: strings.Repeat("a", helper.MaxCommitOrTagMessageSize+1),
+			trim:    true,
 		},
 	}
 
@@ -44,9 +53,9 @@ func TestGetTag(t *testing.T) {
 		t.Run(testCase.tagName, func(t *testing.T) {
 			tagID := testhelper.CreateTag(t, testRepoPath, testCase.tagName, testCase.rev, &testhelper.CreateTagOpts{Message: testCase.message})
 
-			tag, err := GetTagCatfile(c, string(tagID), testCase.tagName)
+			tag, err := GetTagCatfile(c, tagID, testCase.tagName, testCase.trim, true)
 			require.NoError(t, err)
-			if len(testCase.message) >= helper.MaxCommitOrTagMessageSize {
+			if testCase.trim && len(testCase.message) >= helper.MaxCommitOrTagMessageSize {
 				testCase.message = testCase.message[:helper.MaxCommitOrTagMessageSize]
 			}
 
@@ -62,6 +71,7 @@ func TestSplitRawTag(t *testing.T) {
 		tagContent  string
 		header      tagHeader
 		body        []byte
+		trimNewLine bool
 	}{
 		{
 			description: "tag without a message",
@@ -107,9 +117,21 @@ func TestSplitRawTag(t *testing.T) {
 			},
 			body: []byte("Hello world\n\nThis is a message"),
 		},
+		{
+			description: "tag with message with empty line and right side new line trimming",
+			tagContent:  "object c92faf3e0a557270141be67f206d7cdb99bfc3a2\ntype commit\ntag v2.6.16.28\ntagger Adrian Bunk <bunk@stusta.de> 1156539089 +0200\n\nHello world\n\nThis is a message\n\n",
+			header: tagHeader{
+				oid:     "c92faf3e0a557270141be67f206d7cdb99bfc3a2",
+				tagType: "commit",
+				tag:     "v2.6.16.28",
+				tagger:  "Adrian Bunk <bunk@stusta.de> 1156539089 +0200",
+			},
+			body:        []byte("Hello world\n\nThis is a message"),
+			trimNewLine: true,
+		},
 	}
 	for _, tc := range testCases {
-		header, body, err := splitRawTag(bytes.NewReader([]byte(tc.tagContent)))
+		header, body, err := splitRawTag(bytes.NewReader([]byte(tc.tagContent)), tc.trimNewLine)
 		assert.Equal(t, tc.header, *header)
 		assert.Equal(t, tc.body, body)
 		assert.NoError(t, err)