diff options
author | Zeger-Jan van de Weg <git@zjvandeweg.nl> | 2017-10-30 17:21:44 +0300 |
---|---|---|
committer | Kim "BKC" Carlbäcker <kim.carlbacker@gmail.com> | 2017-11-06 20:00:48 +0300 |
commit | 257d031e5634c89e8304f1d859104f2284bd8e4d (patch) | |
tree | 2cea96e1d1583bad0d02b804b110923c2c66a735 | |
parent | db266bf379bfb2fd16d7278e3f228e008ca25a47 (diff) |
Implement Wiki::GetPageVersions
- Dont share a repository between tests on Wikis
Previously we'd reuse the same repository. If two tests used the same
page or where counting the number of pages it would've broken other
test. The parent commit on this one, had that problem. This is not fixed
for the entire wiki package.
-rw-r--r-- | CHANGELOG.md | 5 | ||||
-rw-r--r-- | internal/service/wiki/delete_page_test.go | 8 | ||||
-rw-r--r-- | internal/service/wiki/find_file_test.go | 6 | ||||
-rw-r--r-- | internal/service/wiki/find_page_test.go | 12 | ||||
-rw-r--r-- | internal/service/wiki/get_page_versions.go | 43 | ||||
-rw-r--r-- | internal/service/wiki/get_page_versions_test.go | 118 | ||||
-rw-r--r-- | internal/service/wiki/server.go | 5 | ||||
-rw-r--r-- | internal/service/wiki/testhelper_test.go | 57 | ||||
-rw-r--r-- | internal/service/wiki/update_page_test.go | 10 | ||||
-rw-r--r-- | internal/service/wiki/write_page_test.go | 10 | ||||
-rw-r--r-- | internal/testhelper/testhelper.go | 46 | ||||
-rw-r--r-- | ruby/lib/gitaly_server/wiki_service.rb | 33 |
12 files changed, 324 insertions, 29 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b276b02f..198d59ddc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Gitaly changelog +UNRELEASED + +- Implement WikiGetPageVersions RPC + https://gitlab.com/gitlab-org/gitaly/merge_requests/430 + v0.52.0 - Implement WikiUpdatePage RPC diff --git a/internal/service/wiki/delete_page_test.go b/internal/service/wiki/delete_page_test.go index 29a00ffc5..2f410c16b 100644 --- a/internal/service/wiki/delete_page_test.go +++ b/internal/service/wiki/delete_page_test.go @@ -13,6 +13,9 @@ import ( ) func TestSuccessfulWikiDeletePageRequest(t *testing.T) { + wikiRepo, cleanupFunc := setupWikiRepo() + defer cleanupFunc() + ctx, cancel := testhelper.Context() defer cancel() @@ -28,7 +31,7 @@ func TestSuccessfulWikiDeletePageRequest(t *testing.T) { message := []byte("Delete " + pageName) content := []byte("It was the best of wikis, it was the worst of wikis") - writeWikiPage(t, client, pageName, content) + writeWikiPage(t, client, wikiRepo, pageName, content) request := &pb.WikiDeletePageRequest{ Repository: wikiRepo, @@ -53,6 +56,9 @@ func TestSuccessfulWikiDeletePageRequest(t *testing.T) { } func TestFailedWikiDeletePageDueToValidations(t *testing.T) { + wikiRepo, cleanupFunc := setupWikiRepo() + defer cleanupFunc() + server, serverSocketPath := runWikiServiceServer(t) defer server.Stop() diff --git a/internal/service/wiki/find_file_test.go b/internal/service/wiki/find_file_test.go index fbe0298f8..4c60fffd5 100644 --- a/internal/service/wiki/find_file_test.go +++ b/internal/service/wiki/find_file_test.go @@ -17,6 +17,9 @@ import ( ) func TestSuccessfulWikiFindFileRequest(t *testing.T) { + _, cleanupFunc := setupWikiRepo() + defer cleanupFunc() + server, serverSocketPath := runWikiServiceServer(t) defer server.Stop() @@ -131,6 +134,9 @@ func TestSuccessfulWikiFindFileRequest(t *testing.T) { } func TestFailedWikiFindFileDueToValidation(t *testing.T) { + wikiRepo, cleanupFunc := setupWikiRepo() + defer cleanupFunc() + server, serverSocketPath := runWikiServiceServer(t) defer server.Stop() diff --git a/internal/service/wiki/find_page_test.go b/internal/service/wiki/find_page_test.go index 67bde39e3..2605d99a5 100644 --- a/internal/service/wiki/find_page_test.go +++ b/internal/service/wiki/find_page_test.go @@ -15,6 +15,9 @@ import ( ) func TestSuccessfulWikiFindPageRequest(t *testing.T) { + wikiRepo, cleanupFunc := setupWikiRepo() + defer cleanupFunc() + ctx, cancel := testhelper.Context() defer cancel() @@ -30,14 +33,14 @@ func TestSuccessfulWikiFindPageRequest(t *testing.T) { page2Name := "Installing/Step 133-b" page3Name := "Installing/Step 133-c" - writeWikiPage(t, client, page1Name, content) + writeWikiPage(t, client, wikiRepo, page1Name, content) head1ID := testhelper.MustRunCommand(t, nil, "git", "-C", wikiRepoPath, "show", "--format=format:%H", "--no-patch", "HEAD") page1Commit, err := gitlog.GetCommit(ctx, wikiRepo, string(head1ID), "") require.NoError(t, err, "look up git commit after writing a wiki page") - writeWikiPage(t, client, page2Name, content) + writeWikiPage(t, client, wikiRepo, page2Name, content) - writeWikiPage(t, client, page3Name, content) + writeWikiPage(t, client, wikiRepo, page3Name, content) head3ID := testhelper.MustRunCommand(t, nil, "git", "-C", wikiRepoPath, "show", "--format=format:%H", "--no-patch", "HEAD") page3Commit, err := gitlog.GetCommit(ctx, wikiRepo, string(head3ID), "") require.NoError(t, err, "look up git commit after writing a wiki page") @@ -153,6 +156,9 @@ func TestSuccessfulWikiFindPageRequest(t *testing.T) { } func TestFailedWikiFindPageDueToValidation(t *testing.T) { + wikiRepo, cleanupFunc := setupWikiRepo() + defer cleanupFunc() + server, serverSocketPath := runWikiServiceServer(t) defer server.Stop() diff --git a/internal/service/wiki/get_page_versions.go b/internal/service/wiki/get_page_versions.go new file mode 100644 index 000000000..02a5f19ae --- /dev/null +++ b/internal/service/wiki/get_page_versions.go @@ -0,0 +1,43 @@ +package wiki + +import ( + "gitlab.com/gitlab-org/gitaly/internal/rubyserver" + + pb "gitlab.com/gitlab-org/gitaly-proto/go" + + "google.golang.org/grpc" + "google.golang.org/grpc/codes" +) + +func (s *server) WikiGetPageVersions(request *pb.WikiGetPageVersionsRequest, stream pb.WikiService_WikiGetPageVersionsServer) error { + ctx := stream.Context() + + if len(request.GetPagePath()) == 0 { + return grpc.Errorf(codes.InvalidArgument, "WikiGetPageVersions: Empty Path") + } + + client, err := s.WikiServiceClient(ctx) + if err != nil { + return err + } + + clientCtx, err := rubyserver.SetHeaders(ctx, request.GetRepository()) + if err != nil { + return err + } + + rubyStream, err := client.WikiGetPageVersions(clientCtx, request) + if err != nil { + return err + } + + return rubyserver.Proxy(func() error { + resp, err := rubyStream.Recv() + if err != nil { + md := rubyStream.Trailer() + stream.SetTrailer(md) + return err + } + return stream.Send(resp) + }) +} diff --git a/internal/service/wiki/get_page_versions_test.go b/internal/service/wiki/get_page_versions_test.go new file mode 100644 index 000000000..c28201deb --- /dev/null +++ b/internal/service/wiki/get_page_versions_test.go @@ -0,0 +1,118 @@ +package wiki + +import ( + "bytes" + "strings" + "testing" + + "gitlab.com/gitlab-org/gitaly/internal/testhelper" + + pb "gitlab.com/gitlab-org/gitaly-proto/go" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestWikiGetPageVersionsRequest(t *testing.T) { + wikiRepo, cleanupFunc := setupWikiRepo() + defer cleanupFunc() + + ctx, cancel := testhelper.Context() + defer cancel() + + server, serverSocketPath := runWikiServiceServer(t) + defer server.Stop() + + client, conn := newWikiClient(t, serverSocketPath) + defer conn.Close() + pageTitle := "WikiGetPageVersions" + + content := bytes.Repeat([]byte("Mock wiki page content"), 10000) + writeWikiPage(t, client, wikiRepo, pageTitle, content) + v1cid := testhelper.MustRunCommand(t, nil, "git", "-C", wikiRepoPath, "log", "-1", "--format=%H") + updateWikiPage(t, client, wikiRepo, pageTitle, []byte("New content")) + v2cid := testhelper.MustRunCommand(t, nil, "git", "-C", wikiRepoPath, "log", "-1", "--format=%H") + + gitAuthor := &pb.CommitAuthor{ + Name: []byte("Ahmad Sherif"), + Email: []byte("ahmad@gitlab.com"), + } + + testCases := []struct { + desc string + request *pb.WikiGetPageVersionsRequest + versions []*pb.WikiPageVersion + }{ + { + desc: "No page found", + request: &pb.WikiGetPageVersionsRequest{ + Repository: wikiRepo, + PagePath: []byte("not-found"), + }, + versions: nil, + }, + { + desc: "2 version found", + request: &pb.WikiGetPageVersionsRequest{ + Repository: wikiRepo, + PagePath: []byte(pageTitle), + }, + versions: []*pb.WikiPageVersion{ + { + Commit: &pb.GitCommit{ + Id: strings.TrimRight(string(v2cid), "\n"), + Body: []byte("Update WikiGetPageVersions"), + Subject: []byte("Update WikiGetPageVersions"), + Author: gitAuthor, + Committer: gitAuthor, + ParentIds: []string{strings.TrimRight(string(v1cid), "\n")}, + }, + Format: "markdown", + }, + { + Commit: &pb.GitCommit{ + Id: strings.TrimRight(string(v1cid), "\n"), + Body: []byte("Add WikiGetPageVersions"), + Subject: []byte("Add WikiGetPageVersions"), + Author: gitAuthor, + Committer: gitAuthor, + }, + Format: "markdown", + }, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + stream, err := client.WikiGetPageVersions(ctx, tc.request) + require.NoError(t, err) + require.NoError(t, stream.CloseSend()) + + response, err := stream.Recv() + require.NoError(t, err) + + require.Len(t, response.GetVersions(), len(tc.versions)) + for i, version := range response.GetVersions() { + v2 := tc.versions[i] + assertWikiPageVersionEqual(t, version, v2, "%d blew up", i) + } + }) + } +} + +func assertWikiPageVersionEqual(t *testing.T, expected, actual *pb.WikiPageVersion, msg ...interface{}) bool { + assert.Equal(t, expected.GetFormat(), actual.GetFormat()) + assert.NoError(t, testhelper.GitCommitEqual(expected.GetCommit(), actual.GetCommit())) + + switch len(msg) { + case 0: + t.Logf("WikiPageVersion differs: %v != %v", expected, actual) + case 1: + t.Log(msg[0]) + default: + t.Logf(msg[0].(string), msg[1:]) + } + + return t.Failed() +} diff --git a/internal/service/wiki/server.go b/internal/service/wiki/server.go index b2c589e10..24010f183 100644 --- a/internal/service/wiki/server.go +++ b/internal/service/wiki/server.go @@ -1,7 +1,6 @@ package wiki import ( - "gitlab.com/gitlab-org/gitaly/internal/helper" "gitlab.com/gitlab-org/gitaly/internal/rubyserver" pb "gitlab.com/gitlab-org/gitaly-proto/go" @@ -15,7 +14,3 @@ type server struct { func NewServer(rs *rubyserver.Server) pb.WikiServiceServer { return &server{rs} } - -func (s *server) WikiGetPageVersions(_ *pb.WikiGetPageVersionsRequest, _ pb.WikiService_WikiGetPageVersionsServer) error { - return helper.Unimplemented -} diff --git a/internal/service/wiki/testhelper_test.go b/internal/service/wiki/testhelper_test.go index d41e97607..b79ec0c37 100644 --- a/internal/service/wiki/testhelper_test.go +++ b/internal/service/wiki/testhelper_test.go @@ -19,7 +19,6 @@ import ( ) var ( - wikiRepo *pb.Repository wikiRepoPath string ) @@ -32,18 +31,6 @@ var rubyServer *rubyserver.Server func testMain(m *testing.M) int { defer testhelper.MustHaveNoChildProcess() - testhelper.ConfigureTestStorage() - storagePath := testhelper.GitlabTestStoragePath() - wikiRepoPath = path.Join(storagePath, "wiki-test.git") - - testhelper.MustRunCommand(nil, nil, "git", "init", "--bare", wikiRepoPath) - defer os.RemoveAll(wikiRepoPath) - - wikiRepo = &pb.Repository{ - StorageName: "default", - RelativePath: "wiki-test.git", - } - var err error testhelper.ConfigureRuby() rubyServer, err = rubyserver.Start() @@ -87,7 +74,7 @@ func newWikiClient(t *testing.T, serverSocketPath string) (pb.WikiServiceClient, return pb.NewWikiServiceClient(conn), conn } -func writeWikiPage(t *testing.T, client pb.WikiServiceClient, name string, content []byte) { +func writeWikiPage(t *testing.T, client pb.WikiServiceClient, wikiRepo *pb.Repository, name string, content []byte) { commitDetails := &pb.WikiCommitDetails{ Name: []byte("Ahmad Sherif"), Email: []byte("ahmad@gitlab.com"), @@ -114,6 +101,48 @@ func writeWikiPage(t *testing.T, client pb.WikiServiceClient, name string, conte require.NoError(t, err) } +func updateWikiPage(t *testing.T, client pb.WikiServiceClient, wikiRepo *pb.Repository, name string, content []byte) { + commitDetails := &pb.WikiCommitDetails{ + Name: []byte("Ahmad Sherif"), + Email: []byte("ahmad@gitlab.com"), + Message: []byte("Update " + name), + } + + request := &pb.WikiUpdatePageRequest{ + Repository: wikiRepo, + PagePath: []byte(name), + Title: []byte(name), + Format: "markdown", + CommitDetails: commitDetails, + Content: content, + } + + ctx, cancel := testhelper.Context() + defer cancel() + + stream, err := client.WikiUpdatePage(ctx) + require.NoError(t, err) + require.NoError(t, stream.Send(request)) + + _, err = stream.CloseAndRecv() + require.NoError(t, err) +} + +func setupWikiRepo() (*pb.Repository, func()) { + testhelper.ConfigureTestStorage() + storagePath := testhelper.GitlabTestStoragePath() + wikiRepoPath = path.Join(storagePath, "wiki-test.git") + + testhelper.MustRunCommand(nil, nil, "git", "init", "--bare", wikiRepoPath) + + wikiRepo := &pb.Repository{ + StorageName: "default", + RelativePath: "wiki-test.git", + } + + return wikiRepo, func() { os.RemoveAll(wikiRepoPath) } +} + func sendBytes(data []byte, chunkSize int, sender func([]byte) error) (int, error) { i := 0 for ; len(data) > 0; i++ { diff --git a/internal/service/wiki/update_page_test.go b/internal/service/wiki/update_page_test.go index 1a9ba0eb5..81d95ee7a 100644 --- a/internal/service/wiki/update_page_test.go +++ b/internal/service/wiki/update_page_test.go @@ -14,6 +14,9 @@ import ( ) func TestSuccessfulWikiUpdatePageRequest(t *testing.T) { + wikiRepo, cleanupFunc := setupWikiRepo() + defer cleanupFunc() + ctx, cancel := testhelper.Context() defer cancel() @@ -23,7 +26,7 @@ func TestSuccessfulWikiUpdatePageRequest(t *testing.T) { client, conn := newWikiClient(t, serverSocketPath) defer conn.Close() - writeWikiPage(t, client, "Installing Gitaly", []byte("foobar")) + writeWikiPage(t, client, wikiRepo, "Installing Gitaly", []byte("foobar")) content := bytes.Repeat([]byte("Mock wiki page content"), 10000) @@ -78,13 +81,16 @@ func TestSuccessfulWikiUpdatePageRequest(t *testing.T) { } func TestFailedWikiUpdatePageDueToValidations(t *testing.T) { + wikiRepo, cleanupFunc := setupWikiRepo() + defer cleanupFunc() + server, serverSocketPath := runWikiServiceServer(t) defer server.Stop() client, conn := newWikiClient(t, serverSocketPath) defer conn.Close() - writeWikiPage(t, client, "Installing Gitaly", []byte("foobar")) + writeWikiPage(t, client, wikiRepo, "Installing Gitaly", []byte("foobar")) commitDetails := &pb.WikiCommitDetails{ Name: []byte("Ahmad Sherif"), diff --git a/internal/service/wiki/write_page_test.go b/internal/service/wiki/write_page_test.go index 99b515c20..c389fb0c8 100644 --- a/internal/service/wiki/write_page_test.go +++ b/internal/service/wiki/write_page_test.go @@ -14,6 +14,9 @@ import ( ) func TestSuccessfulWikiWritePageRequest(t *testing.T) { + wikiRepo, cleanupFunc := setupWikiRepo() + defer cleanupFunc() + ctx, cancel := testhelper.Context() defer cancel() @@ -75,6 +78,9 @@ func TestSuccessfulWikiWritePageRequest(t *testing.T) { } func TestFailedWikiWritePageDueToDuplicatePage(t *testing.T) { + wikiRepo, cleanupFunc := setupWikiRepo() + defer cleanupFunc() + server, serverSocketPath := runWikiServiceServer(t) defer server.Stop() @@ -89,7 +95,7 @@ func TestFailedWikiWritePageDueToDuplicatePage(t *testing.T) { Message: []byte("Add " + pageName), } - writeWikiPage(t, client, pageName, content) + writeWikiPage(t, client, wikiRepo, pageName, content) request := &pb.WikiWritePageRequest{ Repository: wikiRepo, @@ -115,6 +121,8 @@ func TestFailedWikiWritePageDueToDuplicatePage(t *testing.T) { } func TestFailedWikiWritePageDueToValidations(t *testing.T) { + wikiRepo := &pb.Repository{} + server, serverSocketPath := runWikiServiceServer(t) defer server.Stop() diff --git a/internal/testhelper/testhelper.go b/internal/testhelper/testhelper.go index 0f1c36cfe..d75fd2030 100644 --- a/internal/testhelper/testhelper.go +++ b/internal/testhelper/testhelper.go @@ -138,11 +138,51 @@ func MustRunCommand(t *testing.T, stdin io.Reader, name string, args ...string) return output } +// authorSortofEqual tests if two `CommitAuthor`s have the same name and email. +// useful when creating commits in the tests. +func authorSortofEqual(a, b *pb.CommitAuthor) bool { + if (a == nil) != (b == nil) { + return false + } + return bytes.Equal(a.GetName(), b.GetName()) && + bytes.Equal(a.GetEmail(), b.GetEmail()) +} + // AuthorsEqual tests if two `CommitAuthor`s are equal func AuthorsEqual(a *pb.CommitAuthor, b *pb.CommitAuthor) bool { - return bytes.Equal(a.Name, b.Name) && - bytes.Equal(a.Email, b.Email) && - a.Date.Seconds == b.Date.Seconds + return authorSortofEqual(a, b) && + a.GetDate().Seconds == b.GetDate().Seconds +} + +// GitCommitEqual tests if two `GitCommit`s are equal +func GitCommitEqual(a, b *pb.GitCommit) error { + if !authorSortofEqual(a.GetAuthor(), b.GetAuthor()) { + return fmt.Errorf("Author does not match: %v != %v", a.GetAuthor(), b.GetAuthor()) + } + if !authorSortofEqual(a.GetCommitter(), b.GetCommitter()) { + return fmt.Errorf("Commiter does not match: %v != %v", a.GetCommitter(), b.GetCommitter()) + } + if !bytes.Equal(a.GetBody(), b.GetBody()) { + return fmt.Errorf("Body differs: %q != %q", a.GetBody(), b.GetBody()) + } + if !bytes.Equal(a.GetSubject(), b.GetSubject()) { + return fmt.Errorf("Subject differs: %q != %q", a.GetSubject(), b.GetSubject()) + } + if strings.Compare(a.GetId(), b.GetId()) != 0 { + return fmt.Errorf("Id does not match: %q != %q", a.GetId(), b.GetId()) + } + if len(a.GetParentIds()) != len(b.GetParentIds()) { + return fmt.Errorf("ParentId does not match: %v != %v", a.GetParentIds(), b.GetParentIds()) + } + + for i, pid := range a.GetParentIds() { + pid2 := b.GetParentIds()[i] + if strings.Compare(pid, pid2) != 0 { + return fmt.Errorf("parent id mismatch: %v != %v", pid, pid2) + } + } + + return nil } // FindLocalBranchCommitAuthorsEqual tests if two `FindLocalBranchCommitAuthor`s are equal diff --git a/ruby/lib/gitaly_server/wiki_service.rb b/ruby/lib/gitaly_server/wiki_service.rb index a22553705..13353305e 100644 --- a/ruby/lib/gitaly_server/wiki_service.rb +++ b/ruby/lib/gitaly_server/wiki_service.rb @@ -120,6 +120,39 @@ module GitalyServer end end + def wiki_get_page_versions(request, call) + bridge_exceptions do + repo = Gitlab::Git::Repository.from_call(call) + wiki = Gollum::Wiki.new(repo.path) + path = request.page_path + + page = wiki.paged(Gollum::Page.canonicalize_filename(path), File.split(path).first) + + unless page + return Enumerator.new do |y| + y.yield Gitaly::WikiGetPageVersionsResponse.new(versions: []) + end + end + + Enumerator.new do |y| + page.versions.each_slice(20) do |slice| + versions = + slice.map do |commit| + gollum_page = wiki.page(page.title, commit.id) + obj = repo.rugged.rev_parse(commit.id) + + Gitaly::WikiPageVersion.new( + commit: gitaly_commit_from_rugged(obj), + format: gollum_page&.format.to_s + ) + end + + y.yield Gitaly::WikiGetPageVersionsResponse.new(versions: versions) + end + end + end + end + def wiki_update_page(call) bridge_exceptions do repo = Gitlab::Git::Repository.from_call(call) |