diff options
author | Tiago Botelho <tiagonbotelho@hotmail.com> | 2018-09-25 18:02:45 +0300 |
---|---|---|
committer | Tiago Botelho <tiagonbotelho@hotmail.com> | 2018-09-27 14:58:42 +0300 |
commit | 6ba3498a2c96f947ff37909980ae4a4afb3c8562 (patch) | |
tree | 5a52bc587a3f98d2b61a36c28329196ad7cd1572 | |
parent | 46e6eae9951b7c1740e518a22d04f41bb9a69eae (diff) |
Bumps gitaly-proto version
-rw-r--r-- | internal/git/lstree/last_commits.go | 49 | ||||
-rw-r--r-- | internal/git/lstree/last_commits_test.go | 120 | ||||
-rw-r--r-- | internal/git/lstree/testdata/z-lstree-irregular.txt | bin | 0 -> 191 bytes | |||
-rw-r--r-- | internal/service/commit/list_last_commits_for_tree.go | 16 | ||||
-rw-r--r-- | internal/service/commit/list_last_commits_for_tree_test.go | 102 | ||||
-rw-r--r-- | vendor/gitlab.com/gitlab-org/gitaly-proto/go/VERSION | 2 | ||||
-rw-r--r-- | vendor/vendor.json | 10 |
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 Binary files differnew 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: ×tamp.Timestamp{Seconds: 1393491451}, + }, + Committer: &pb.CommitAuthor{ + Name: []byte("Dmitriy Zaporozhets"), + Email: []byte("dmitriy.zaporozhets@gmail.com"), + Date: ×tamp.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=", |