Bumps gitaly-proto version

7 files changed, 224 insertions, 75 deletions

diff --git a/internal/git/lstree/last_commits.go b/internal/git/lstree/last_commits.go
index c51a33678..09c9c9d96 100644
--- a/internal/git/lstree/last_commits.go
+++ b/internal/git/lstree/last_commits.go
@@ -3,18 +3,20 @@ package lstree
 import (
 	"bufio"
 	"bytes"
-	"fmt"
+	"errors"
 	"io"
 )
 
-type objectType int
+// Enum for the type of object in an ls-tree entry
+// can be tree, blob or commit
+type ObjectType int
 
 // Entry represents a single ls-tree entry
 type Entry struct {
-	Mode   []byte
-	Type   objectType
-	Object string
-	Path   string
+	Mode []byte
+	Type ObjectType
+	Oid  string
+	Path string
 }
 
 // Entries holds every ls-tree Entry
@@ -26,12 +28,14 @@ type Parser struct {
 }
 
 const (
-	delimiter            = 0
-	tree      objectType = iota + 1
+	tree ObjectType = iota
 	blob
 	submodule
 )
 
+// ErrParse is returned when the parse of an entry was unsuccessful
+var ErrParse = errors.New("Failed to parse git ls-tree response")
+
 func (e Entries) Len() int {
 	return len(e)
 }
@@ -55,39 +59,40 @@ func NewParser(src io.Reader) *Parser {
 // NextEntry reads from git ls-tree --z --full-name command
 // parses the tree entry and returns a *Entry.
 func (p *Parser) NextEntry() (*Entry, error) {
-	result := &Entry{}
-
-	data, err := p.reader.ReadBytes(delimiter)
+	data, err := p.reader.ReadBytes(0x00)
 	if err != nil {
 		return nil, err
 	}
 
 	// We expect each `data` to be <mode> SP <type> SP <object> TAB <path>\0.
 	split := bytes.SplitN(data, []byte(" "), 3)
-	objectAndFile := bytes.SplitN(split[len(split)-1], []byte(" \t"), 2)
-	split = append(split[:len(split)-1], objectAndFile...)
+	if len(split) != 3 {
+		return nil, ErrParse
+	}
 
-	if len(split) != 4 {
-		return nil, fmt.Errorf("error parsing %q", data)
+	objectAndFile := bytes.SplitN(split[len(split)-1], []byte("\t"), 2)
+	parsedEntry := append(split[:len(split)-1], objectAndFile...)
+	if len(parsedEntry) != 4 {
+		return nil, ErrParse
 	}
 
-	objectType, err := toEnum(string(split[1]))
+	objectType, err := toEnum(string(parsedEntry[1]))
 	if err != nil {
 		return nil, err
 	}
 
-	result.Mode = split[0]
+	result := &Entry{}
+	result.Mode = parsedEntry[0]
 	result.Type = objectType
-	result.Object = string(split[2])
+	result.Oid = string(parsedEntry[2])
 
 	// We know that the last byte in 'path' will be a zero byte.
-	path := split[3]
-	result.Path = string(path[:len(path)-1])
+	result.Path = string(bytes.TrimRight(parsedEntry[3], "\x00"))
 
 	return result, nil
 }
 
-func toEnum(s string) (objectType, error) {
+func toEnum(s string) (ObjectType, error) {
 	switch s {
 	case "tree":
 		return tree, nil
@@ -96,6 +101,6 @@ func toEnum(s string) (objectType, error) {
 	case "commit":
 		return submodule, nil
 	default:
-		return -1, fmt.Errorf("Error in objectType conversion %q", s)
+		return -1, ErrParse
 	}
 }
diff --git a/internal/git/lstree/last_commits_test.go b/internal/git/lstree/last_commits_test.go
index 6bb402743..38b1c5bdb 100644
--- a/internal/git/lstree/last_commits_test.go
+++ b/internal/git/lstree/last_commits_test.go
@@ -9,54 +9,92 @@ import (
 )
 
 func TestParser(t *testing.T) {
-	file, err := os.Open("testdata/z-lstree.txt")
-
-	require.NoError(t, err)
-	defer file.Close()
-
-	var parsedEntries Entries
-
-	parser := NewParser(file)
-
-	for {
-		entry, err := parser.NextEntry()
-		if err == io.EOF {
-			break
-		}
-
-		require.NoError(t, err)
-		parsedEntries = append(parsedEntries, entry)
-	}
-
-	expectedEntries := Entries{
+	testCases := []struct {
+		desc     string
+		filename string
+		entries  Entries
+	}{
 		{
-			Mode:   []byte("100644"),
-			Type:   1,
-			Object: "b78f2bdd90e85de463bd091622efcc70489de893",
-			Path:   ".gitmodules",
+			desc:     "regular entries",
+			filename: "testdata/z-lstree.txt",
+			entries: Entries{
+				{
+					Mode: []byte("100644"),
+					Type: 1,
+					Oid:  "b78f2bdd90e85de463bd091622efcc70489de893",
+					Path: ".gitmodules",
+				},
+				{
+					Mode: []byte("040000"),
+					Type: 0,
+					Oid:  "85ecfbd13807e6374407ba97d252bfe0cf2403fe",
+					Path: "_locales",
+				},
+				{
+					Mode: []byte("160000"),
+					Type: 2,
+					Oid:  "b2291647b9346873501cedf482270495cd85b7b9",
+					Path: "bar",
+				},
+			},
 		},
 		{
-			Mode:   []byte("040000"),
-			Type:   0,
-			Object: "85ecfbd13807e6374407ba97d252bfe0cf2403fe",
-			Path:   "_locales",
-		},
-		{
-			Mode:   []byte("160000"),
-			Type:   2,
-			Object: "b2291647b9346873501cedf482270495cd85b7b9",
-			Path:   "bar",
+			desc:     "irregular path",
+			filename: "testdata/z-lstree-irregular.txt",
+			entries: Entries{
+				{
+					Mode: []byte("100644"),
+					Type: 1,
+					Oid:  "b78f2bdd90e85de463bd091622efcc70489de893",
+					Path: ".gitmodules",
+				},
+				{
+					Mode: []byte("040000"),
+					Type: 0,
+					Oid:  "85ecfbd13807e6374407ba97d252bfe0cf2403fe",
+					Path: "_locales",
+				},
+				{
+					Mode: []byte("160000"),
+					Type: 2,
+					Oid:  "b2291647b9346873501cedf482270495cd85b7b9",
+					Path: "foo bar",
+				},
+			},
 		},
 	}
 
-	require.Equal(t, len(expectedEntries), len(parsedEntries))
+	for _, testCase := range testCases {
+		t.Run(testCase.desc, func(t *testing.T) {
+			file, err := os.Open(testCase.filename)
+
+			require.NoError(t, err)
+			defer file.Close()
+
+			parsedEntries := Entries{}
+
+			parser := NewParser(file)
+			for {
+				entry, err := parser.NextEntry()
+				if err == io.EOF {
+					break
+				}
+
+				require.NoError(t, err)
+				parsedEntries = append(parsedEntries, *entry)
+			}
+
+			expectedEntries := testCase.entries
+			require.Equal(t, len(expectedEntries), len(parsedEntries))
 
-	for index, parsedEntry := range parsedEntries {
-		expectedEntry := expectedEntries[index]
+			for index, parsedEntry := range parsedEntries {
+				expectedEntry := expectedEntries[index]
 
-		require.Equal(t, expectedEntry.Mode, parsedEntry.Mode)
-		require.Equal(t, expectedEntry.Type, parsedEntry.Type)
-		require.Equal(t, expectedEntry.Object, parsedEntry.Object)
-		require.Equal(t, expectedEntry.Path, parsedEntry.Path)
+				require.Equal(t, expectedEntry.Mode, parsedEntry.Mode)
+				require.Equal(t, expectedEntry.Type, parsedEntry.Type)
+				require.Equal(t, expectedEntry.Oid, parsedEntry.Oid)
+				require.Equal(t, expectedEntry.Path, parsedEntry.Path)
+			}
+		})
 	}
 }
diff --git a/internal/git/lstree/testdata/z-lstree-irregular.txt b/internal/git/lstree/testdata/z-lstree-irregular.txt
new file mode 100644
index 000000000..2e506668b
--- /dev/null
+++ b/internal/git/lstree/testdata/z-lstree-irregular.txt
diff --git a/internal/service/commit/list_last_commits_for_tree.go b/internal/service/commit/list_last_commits_for_tree.go
index f995b71b2..537c03c28 100644
--- a/internal/service/commit/list_last_commits_for_tree.go
+++ b/internal/service/commit/list_last_commits_for_tree.go
@@ -16,7 +16,7 @@ import (
 )
 
 var (
-	maxNumStatBatchSize = 25
+	maxNumStatBatchSize = 10
 )
 
 func (s *server) ListLastCommitsForTree(in *pb.ListLastCommitsForTreeRequest, stream pb.CommitService_ListLastCommitsForTreeServer) error {
@@ -26,23 +26,27 @@ func (s *server) ListLastCommitsForTree(in *pb.ListLastCommitsForTreeRequest, st
 
 	cmd, parser, err := newLSTreeParser(in, stream)
 	if err != nil {
-		return err
+		if _, ok := status.FromError(err); ok {
+			return err
+		}
+
+		return status.Errorf(codes.Internal, "ListLastCommitsForTree: gitCommand: %v", err)
 	}
 
-	var batch []*pb.ListLastCommitsForTreeResponse_CommitForTree
+	batch := make([]*pb.ListLastCommitsForTreeResponse_CommitForTree, 0, maxNumStatBatchSize)
 	entries, err := getLSTreeEntries(parser)
 	if err != nil {
 		return err
 	}
 
 	offset := int(in.GetOffset())
-	if offset > len(entries) {
+	if offset >= len(entries) {
 		entries = lstree.Entries{}
 	}
 
 	limit := offset + int(in.GetLimit())
 	if limit > len(entries) {
-		limit = len(entries) - offset
+		limit = len(entries)
 	}
 
 	for _, entry := range entries[offset:limit] {
@@ -62,7 +66,7 @@ func (s *server) ListLastCommitsForTree(in *pb.ListLastCommitsForTreeRequest, st
 				return err
 			}
 
-			batch = nil
+			batch = batch[0:0]
 		}
 	}
 
diff --git a/internal/service/commit/list_last_commits_for_tree_test.go b/internal/service/commit/list_last_commits_for_tree_test.go
new file mode 100644
index 000000000..d97fcecfe
--- /dev/null
+++ b/internal/service/commit/list_last_commits_for_tree_test.go
@@ -0,0 +1,102 @@
+package commit
+
+import (
+	"io"
+	"testing"
+
+	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
+
+	pb "gitlab.com/gitlab-org/gitaly-proto/go"
+
+	"github.com/golang/protobuf/ptypes/timestamp"
+	"github.com/stretchr/testify/require"
+	"golang.org/x/net/context"
+	"google.golang.org/grpc/codes"
+)
+
+func TestSuccessfulListLastCommitsForTreeRequest(t *testing.T) {
+	server, serverSockerPath := startTestServices(t)
+	defer server.Stop()
+
+	client, conn := newCommitServiceClient(t, serverSockerPath)
+	defer conn.Close()
+
+	testRepo, _, cleanupFn := testhelper.NewTestRepo(t)
+	defer cleanupFn()
+
+	commit := &pb.GitCommit{
+		Id:      "570e7b2abdd848b95f2f578043fc23bd6f6fd24d",
+		Subject: []byte("Change some files"),
+		Body:    []byte("Change some files\n\nSigned-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>\n"),
+		Author: &pb.CommitAuthor{
+			Name:  []byte("Dmitriy Zaporozhets"),
+			Email: []byte("dmitriy.zaporozhets@gmail.com"),
+			Date:  &timestamp.Timestamp{Seconds: 1393491451},
+		},
+		Committer: &pb.CommitAuthor{
+			Name:  []byte("Dmitriy Zaporozhets"),
+			Email: []byte("dmitriy.zaporozhets@gmail.com"),
+			Date:  &timestamp.Timestamp{Seconds: 1393491451},
+		},
+		ParentIds: []string{"6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9"},
+		BodySize:  86,
+	}
+
+	testCases := []struct {
+		desc     string
+		revision string
+		path     []byte
+		commit   *pb.GitCommit
+		limit    int32
+		offset   int32
+	}{
+		{
+			desc:     "path is '/'",
+			revision: "570e7b2abdd848b95f2f578043fc23bd6f6fd24d",
+			path:     []byte("/"),
+			commit:   commit,
+			limit:    25,
+			offset:   0,
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Tun(testCase.desc, func(t *testing.T) {
+			request := &pb.ListLastCommitsForTreeRequest{
+				Repository: testRepo,
+				Revision:   testCase.revision,
+				Path:       testCase.path,
+				Limit:      testCase.limit,
+				Offset:     testCase.offset,
+			}
+
+			ctx, cancel := context.WithCancel(context.Background())
+			defer cancel()
+
+			stream, err := client.ListLastCommitsForTree(ctx, request)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			for {
+				fetchedCommits, err := stream.Recv()
+				if err == io.EOF {
+					break
+				}
+
+				require.NoError(t, err)
+
+				fmt.Println(fetchedCommits)
+
+				commits := fetchedCommits.getCommits()
+				for index, fetchedCommit := range commits {
+					expectedPath := testCase.path
+					expectedCommit := testCase.commit
+
+					require.Equal(t, expectedCommit, fetchedCommit.Commit)
+					require.Equal(t, expectedPath, fetchedCommit.Path)
+				}
+			}
+		})
+	}
+}
diff --git a/vendor/gitlab.com/gitlab-org/gitaly-proto/go/VERSION b/vendor/gitlab.com/gitlab-org/gitaly-proto/go/VERSION
index a38b3bd31..f14311864 100644
--- a/vendor/gitlab.com/gitlab-org/gitaly-proto/go/VERSION
+++ b/vendor/gitlab.com/gitlab-org/gitaly-proto/go/VERSION
@@ -1 +1 @@
-0.117.0
+0.118.1
diff --git a/vendor/vendor.json b/vendor/vendor.json
index 9813fc0ff..8b9aeeb1f 100644
--- a/vendor/vendor.json
+++ b/vendor/vendor.json
@@ -201,12 +201,12 @@
 			"revisionTime": "2017-12-31T12:27:32Z"
 		},
 		{
-			"checksumSHA1": "J5+e1uiso5SbD8uR9VFB18dC0Ag=",
+			"checksumSHA1": "uLhcLmhcc35eZMhJTf5CXhjrRzw=",
 			"path": "gitlab.com/gitlab-org/gitaly-proto/go",
-			"revision": "9b48e996a05d5347013dfc1f845b5c82abeb4450",
-			"revisionTime": "2018-09-06T18:35:31Z",
-			"version": "v0.117.0",
-			"versionExact": "v0.117.0"
+			"revision": "566ea63b045ce9705dbf21b9ca0ed1f1eb989bab",
+			"revisionTime": "2018-09-12T14:37:39Z",
+			"version": "v0.118.1",
+			"versionExact": "v0.118.1"
 		},
 		{
 			"checksumSHA1": "nqWNlnMmVpt628zzvyo6Yv2CX5Q=",