Merge branch 'jc-rewrite-get-commit-message' into 'master'

Rewrite get commit message

See merge request gitlab-org/gitaly!1012

4 files changed, 77 insertions, 29 deletions

diff --git a/changelogs/unreleased/jc-rewrite-get-commit-message.yml b/changelogs/unreleased/jc-rewrite-get-commit-message.yml
new file mode 100644
index 000000000..71a1080c2
--- /dev/null
+++ b/changelogs/unreleased/jc-rewrite-get-commit-message.yml
@@ -0,0 +1,5 @@
+---
+title: Rewrite get commit message
+merge_request: 1012
+author: 
+type: other
diff --git a/internal/git/log/commit.go b/internal/git/log/commit.go
index 0ded22414..531f8448b 100644
--- a/internal/git/log/commit.go
+++ b/internal/git/log/commit.go
@@ -4,6 +4,7 @@ import (
 	"bufio"
 	"bytes"
 	"context"
+	"io"
 	"io/ioutil"
 	"strconv"
 	"strings"
@@ -42,15 +43,42 @@ func GetCommitCatfile(c *catfile.Batch, revision string) (*gitalypb.GitCommit, e
 		return nil, err
 	}
 
-	raw, err := ioutil.ReadAll(r)
+	return parseRawCommit(r, info)
+}
+
+// GetCommitMessage looks up a commit message and returns it in its entirety.
+func GetCommitMessage(c *catfile.Batch, repo *gitalypb.Repository, revision string) ([]byte, error) {
+	info, err := c.Info(revision + "^{commit}")
+	if err != nil {
+		return nil, err
+	}
+
+	r, err := c.Commit(info.Oid)
+	if err != nil {
+		return nil, err
+	}
+
+	_, body, err := splitRawCommit(r)
 	if err != nil {
 		return nil, err
 	}
+	return body, nil
+}
 
-	return parseRawCommit(raw, info)
+func parseRawCommit(r io.Reader, info *catfile.ObjectInfo) (*gitalypb.GitCommit, error) {
+	header, body, err := splitRawCommit(r)
+	if err != nil {
+		return nil, err
+	}
+	return buildCommit(header, body, info)
 }
 
-func parseRawCommit(raw []byte, info *catfile.ObjectInfo) (*gitalypb.GitCommit, error) {
+func splitRawCommit(r io.Reader) ([]byte, []byte, error) {
+	raw, err := ioutil.ReadAll(r)
+	if err != nil {
+		return nil, nil, err
+	}
+
 	split := bytes.SplitN(raw, []byte("\n\n"), 2)
 
 	header := split[0]
@@ -59,13 +87,18 @@ func parseRawCommit(raw []byte, info *catfile.ObjectInfo) (*gitalypb.GitCommit,
 		body = split[1]
 	}
 
+	return header, body, nil
+}
+
+func buildCommit(header, body []byte, info *catfile.ObjectInfo) (*gitalypb.GitCommit, error) {
 	commit := &gitalypb.GitCommit{
 		Id:       info.Oid,
+		BodySize: int64(len(body)),
 		Body:     body,
 		Subject:  subjectFromBody(body),
-		BodySize: int64(len(body)),
 	}
-	if max := helper.MaxCommitOrTagMessageSize; len(commit.Body) > max {
+
+	if max := helper.MaxCommitOrTagMessageSize; len(body) > max {
 		commit.Body = commit.Body[:max]
 	}
 
diff --git a/internal/git/log/commit_test.go b/internal/git/log/commit_test.go
index b3144aced..7a73b7282 100644
--- a/internal/git/log/commit_test.go
+++ b/internal/git/log/commit_test.go
@@ -1,6 +1,7 @@
 package log
 
 import (
+	"bytes"
 	"testing"
 
 	"github.com/golang/protobuf/ptypes/timestamp"
@@ -93,9 +94,10 @@ func TestParseRawCommit(t *testing.T) {
 	for _, tc := range testCases {
 		t.Run(tc.desc, func(t *testing.T) {
 			info.Size = int64(len(tc.in))
-			out, err := parseRawCommit(tc.in, info)
+			out, err := parseRawCommit(bytes.NewBuffer(tc.in), info)
 			require.NoError(t, err, "parse error")
-			require.Equal(t, *tc.out, *out)
+			require.Equal(t, tc.out, out)
+
 		})
 	}
 }
diff --git a/internal/service/commit/commit_messages.go b/internal/service/commit/commit_messages.go
index 7d041ae94..b37a9545d 100644
--- a/internal/service/commit/commit_messages.go
+++ b/internal/service/commit/commit_messages.go
@@ -1,45 +1,53 @@
 package commit
 
 import (
+	"bytes"
 	"fmt"
+	"io"
 
 	"gitlab.com/gitlab-org/gitaly-proto/go/gitalypb"
-	"gitlab.com/gitlab-org/gitaly/internal/rubyserver"
-	"google.golang.org/grpc/codes"
-	"google.golang.org/grpc/status"
+	"gitlab.com/gitlab-org/gitaly/internal/git/catfile"
+	"gitlab.com/gitlab-org/gitaly/internal/git/log"
+	"gitlab.com/gitlab-org/gitaly/internal/helper"
+	"gitlab.com/gitlab-org/gitaly/streamio"
 )
 
 func (s *server) GetCommitMessages(request *gitalypb.GetCommitMessagesRequest, stream gitalypb.CommitService_GetCommitMessagesServer) error {
 	if err := validateGetCommitMessagesRequest(request); err != nil {
-		return status.Errorf(codes.InvalidArgument, "GetCommitMessages: %v", err)
+		return helper.ErrInvalidArgument(err)
 	}
-
-	ctx := stream.Context()
-
-	client, err := s.CommitServiceClient(ctx)
-	if err != nil {
-		return err
+	if err := getAndStreamCommitMessages(request, stream); err != nil {
+		return helper.ErrInternal(err)
 	}
 
-	clientCtx, err := rubyserver.SetHeaders(ctx, request.GetRepository())
-	if err != nil {
-		return err
-	}
+	return nil
+}
 
-	rubyStream, err := client.GetCommitMessages(clientCtx, request)
+func getAndStreamCommitMessages(request *gitalypb.GetCommitMessagesRequest, stream gitalypb.CommitService_GetCommitMessagesServer) error {
+	ctx := stream.Context()
+	c, err := catfile.New(ctx, request.GetRepository())
 	if err != nil {
 		return err
 	}
-
-	return rubyserver.Proxy(func() error {
-		resp, err := rubyStream.Recv()
+	for _, commitID := range request.GetCommitIds() {
+		msg, err := log.GetCommitMessage(c, request.GetRepository(), commitID)
 		if err != nil {
-			md := rubyStream.Trailer()
-			stream.SetTrailer(md)
+			return fmt.Errorf("failed to get commit message: %v", err)
+		}
+		msgReader := bytes.NewReader(msg)
+
+		if err := stream.Send(&gitalypb.GetCommitMessagesResponse{CommitId: commitID}); err != nil {
 			return err
 		}
-		return stream.Send(resp)
-	})
+		sw := streamio.NewWriter(func(p []byte) error {
+			return stream.Send(&gitalypb.GetCommitMessagesResponse{Message: p})
+		})
+		_, err = io.Copy(sw, msgReader)
+		if err != nil {
+			return fmt.Errorf("failed to send response: %v", err)
+		}
+	}
+	return nil
 }
 
 func validateGetCommitMessagesRequest(request *gitalypb.GetCommitMessagesRequest) error {