From ceb301ff049f2eec8dfebc4b427cb4c63fcc679c Mon Sep 17 00:00:00 2001 From: Pavlo Strokov Date: Mon, 18 Nov 2019 17:44:46 +0000 Subject: Port GetTagMessages from Gitaly-Ruby to Gitaly - feature toggle 'get_tag_messages_go' - go implementation Closes: #2123 --- internal/git/log/tag.go | 27 ++++++++++++++++----------- internal/git/log/tag_test.go | 28 +++++++++++++++++++++++++--- 2 files changed, 41 insertions(+), 14 deletions(-) (limited to 'internal/git') 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 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 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) -- cgit v1.2.3