diff --git a/changelogs/unreleased/id-fix-page-token-for-committerdate-sort.yml b/changelogs/unreleased/id-fix-page-token-for-committerdate-sort.yml new file mode 100644 index 0000000000000000000000000000000000000000..5cca77ebb7960d59620348d9a2bb7cb410fdaad7 --- /dev/null +++ b/changelogs/unreleased/id-fix-page-token-for-committerdate-sort.yml @@ -0,0 +1,5 @@ +--- +title: Fix refs pagination for updated-at sort +merge_request: 2932 +author: +type: fixed diff --git a/internal/gitaly/service/ref/refs.go b/internal/gitaly/service/ref/refs.go index 000d738e2d23758450ae4e40e629cf24ecff43f9..c4984f6407db7ba342cd9382e1c309ea7e129b81 100644 --- a/internal/gitaly/service/ref/refs.go +++ b/internal/gitaly/service/ref/refs.go @@ -480,7 +480,10 @@ func paginationParamsToOpts(p *gitalypb.PaginationParameter) *findRefsOpts { } if p.GetPageToken() != "" { - opts.IsPageToken = func(l []byte) bool { return bytes.Compare(l, []byte(p.GetPageToken())) >= 0 } + pageToken := []byte(p.GetPageToken()) + opts.IsPageToken = func(l []byte) bool { + return bytes.HasPrefix(l, pageToken) + } } return opts diff --git a/internal/gitaly/service/ref/refs_test.go b/internal/gitaly/service/ref/refs_test.go index b8ab9f8154fa107d92ab06010c399d93e0c8e5c4..f5b87d2b71221237f22b59ecd79ddc626e4aab9f 100644 --- a/internal/gitaly/service/ref/refs_test.go +++ b/internal/gitaly/service/ref/refs_test.go @@ -898,54 +898,93 @@ func TestFindLocalBranchesPagination(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() - limit := 1 - rpcRequest := &gitalypb.FindLocalBranchesRequest{ - Repository: testRepo, - PaginationParams: &gitalypb.PaginationParameter{ - Limit: int32(limit), - PageToken: "refs/heads/gitaly/squash-test", + testCases := []struct { + desc string + sortBy gitalypb.FindLocalBranchesRequest_SortBy + pageToken string + limit int32 + expectedBranch string + }{ + { + desc: "sorted by name", + sortBy: gitalypb.FindLocalBranchesRequest_NAME, + pageToken: "refs/heads/gitaly/squash-test", + limit: 1, + expectedBranch: "refs/heads/improve/awesome", + }, + { + desc: "sorted by committer date asc", + sortBy: gitalypb.FindLocalBranchesRequest_UPDATED_ASC, + pageToken: "refs/heads/feature_conflict", + limit: 1, + expectedBranch: "refs/heads/'test'", + }, + { + desc: "sorted by committer date desc", + sortBy: gitalypb.FindLocalBranchesRequest_UPDATED_DESC, + pageToken: "refs/heads/empty-branch", + limit: 1, + expectedBranch: "refs/heads/'test'", + }, + { + desc: "pageToken does not exist", + sortBy: gitalypb.FindLocalBranchesRequest_NAME, + pageToken: "refs/heads/gitaly/squash-test1", // does not exist + limit: 1, + expectedBranch: "refs/heads/improve/awesome", }, - } - c, err := client.FindLocalBranches(ctx, rpcRequest) - if err != nil { - t.Fatal(err) - } - - var branches []*gitalypb.FindLocalBranchResponse - for { - r, err := c.Recv() - if err == io.EOF { - break - } - require.NoError(t, err) - if err != nil { - t.Fatal(err) - } - branches = append(branches, r.GetBranches()...) } - require.Len(t, branches, limit) + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + rpcRequest := &gitalypb.FindLocalBranchesRequest{ + Repository: testRepo, + SortBy: tc.sortBy, + PaginationParams: &gitalypb.PaginationParameter{ + Limit: tc.limit, + PageToken: tc.pageToken, + }, + } + c, err := client.FindLocalBranches(ctx, rpcRequest) + if err != nil { + t.Fatal(err) + } - expectedBranch := "refs/heads/improve/awesome" - target := localBranches[expectedBranch] + var branches []*gitalypb.FindLocalBranchResponse + for { + r, err := c.Recv() + if err == io.EOF { + break + } + require.NoError(t, err) + if err != nil { + t.Fatal(err) + } + branches = append(branches, r.GetBranches()...) + } - branch := &gitalypb.FindLocalBranchResponse{ - Name: []byte(expectedBranch), - CommitId: target.Id, - CommitSubject: target.Subject, - CommitAuthor: &gitalypb.FindLocalBranchCommitAuthor{ - Name: target.Author.Name, - Email: target.Author.Email, - Date: target.Author.Date, - }, - CommitCommitter: &gitalypb.FindLocalBranchCommitAuthor{ - Name: target.Committer.Name, - Email: target.Committer.Email, - Date: target.Committer.Date, - }, - Commit: target, + require.Len(t, branches, int(tc.limit)) + + target := localBranches[tc.expectedBranch] + branch := &gitalypb.FindLocalBranchResponse{ + Name: []byte(tc.expectedBranch), + CommitId: target.Id, + CommitSubject: target.Subject, + CommitAuthor: &gitalypb.FindLocalBranchCommitAuthor{ + Name: target.Author.Name, + Email: target.Author.Email, + Date: target.Author.Date, + }, + CommitCommitter: &gitalypb.FindLocalBranchCommitAuthor{ + Name: target.Committer.Name, + Email: target.Committer.Email, + Date: target.Committer.Date, + }, + Commit: target, + } + assertContainsLocalBranch(t, branches, branch) + }) } - assertContainsLocalBranch(t, branches, branch) } // Test that `s` contains the elements in `relativeOrder` in that order