Code review refactoring

2 files changed, 37 insertions, 28 deletions

diff --git a/internal/gitaly/service/commit/tree_entries.go b/internal/gitaly/service/commit/tree_entries.go
index f7510e6f3..b3f8d25f2 100644
--- a/internal/gitaly/service/commit/tree_entries.go
+++ b/internal/gitaly/service/commit/tree_entries.go
@@ -335,46 +335,55 @@ func paginateTreeEntries(entries []*gitalypb.TreeEntry, p *gitalypb.PaginationPa
 
 	newPageToken, err := encodePageToken(paginated[len(paginated)-1])
 	if err != nil {
-		return nil, "", fmt.Errorf("cannot marshal JSON token: %s", err)
+		return nil, "", fmt.Errorf("encode page token: %w", err)
 	}
 
 	return paginated, newPageToken, nil
 }
 
-func buildEntryToken(entry *gitalypb.TreeEntry, tokenType string) string {
-	if tokenType == "oid" {
+func buildEntryToken(entry *gitalypb.TreeEntry, tokenType pageTokenType) string {
+	if tokenType == pageTokenTypeOID {
 		return entry.GetOid()
 	}
 
-	if tokenType == "filename" {
-		return string(entry.GetPath())
-	}
-
-	return ""
+	return string(entry.GetPath())
 }
 
 type pageToken struct {
-	FileName string
+	// FileName is the name of the tree entry that acts as continuation point.
+	FileName string `json:"file_name"`
 }
 
-// Decodes Base64 encoded, json marshalled string
-func decodePageToken(token string) (string, string) {
+type pageTokenType int
+
+const (
+	// pageTokenTypeOID is an old-style page token that contains the object ID a tree
+	// entry is pointing to. This is ambiguous and thus deprecated.
+	pageTokenTypeOID = pageTokenType(iota)
+	// pageTokenTypeFilename is a page token that contains the tree entry path.
+	pageTokenTypeFilename
+)
+
+
+// decodePageToken decodes the given Base64-encoded page token. It returns the
+// continuation point of the token and its type.
+func decodePageToken(token string) (string, pageTokenType) {
 	var pageToken pageToken
 
 	decodedString, err := base64.StdEncoding.DecodeString(token)
 	if err != nil {
-		return token, "oid"
+		return token, pageTokenTypeOID
 	}
 
 	if err := json.Unmarshal(decodedString, &pageToken); err != nil {
-		return token, "oid"
+		return token, pageTokenTypeOID
 	}
 
-	return pageToken.FileName, "filename"
+	return pageToken.FileName, pageTokenTypeFilename
 }
 
-// Oid is not a unique value and cannot be used for the cursor generation.
-// Instead we use the file name that should be unique
+// encodePageToken returns a page token with the TreeEntry's path as the continuation point for
+// the next page. The page token serialized by first JSON marshaling it and then base64 encoding it.
 func encodePageToken(entry *gitalypb.TreeEntry) (string, error) {
 	jsonEncoded, err := json.Marshal(pageToken{FileName: string(entry.GetPath())})
 	if err != nil {
diff --git a/internal/gitaly/service/commit/tree_entries_test.go b/internal/gitaly/service/commit/tree_entries_test.go
index 3d7e2cf13..436836bcd 100644
--- a/internal/gitaly/service/commit/tree_entries_test.go
+++ b/internal/gitaly/service/commit/tree_entries_test.go
@@ -342,6 +342,15 @@ func TestGetTreeEntries_successful(t *testing.T) {
 		return strippedEntries
 	}
 
+	getPageToken := func(t testing.TB, entry *gitalypb.TreeEntry) string {
+		t.Helper()
+
+		pageToken, err := encodePageToken(entry)
+		require.NoError(t, err)
+
+		return pageToken
+	}
+
 	testCases := []struct {
 		description     string
 		revision        []byte
@@ -353,7 +362,6 @@ func TestGetTreeEntries_successful(t *testing.T) {
 		pageToken       string
 		pageLimit       int32
 		cursor          string
-		entryPath       []byte
 		skipFlatPaths   bool
 	}{
 		{
@@ -435,7 +443,7 @@ func TestGetTreeEntries_successful(t *testing.T) {
 			entries:     sortedAndPaginated,
 			pageLimit:   4,
 			sortBy:      gitalypb.GetTreeEntriesRequest_TREES_FIRST,
-			cursor:      "eyJGaWxlTmFtZSI6Ii5EU19TdG9yZSJ9",
+			cursor:      "eyJmaWxlX25hbWUiOiIuRFNfU3RvcmUifQ==",
 		},
 		{
 			description:     "with pagination parameters",
@@ -445,7 +453,7 @@ func TestGetTreeEntries_successful(t *testing.T) {
 			legacyPageToken: "fdaada1754989978413d618ee1fb1c0469d6a664",
 			pageToken:       getPageToken(t, rootEntries[2]),
 			pageLimit:       3,
-			cursor:          "eyJGaWxlTmFtZSI6IkxJQ0VOU0UifQ==",
+			cursor:          "eyJmaWxlX25hbWUiOiJMSUNFTlNFIn0=",
 		},
 		{
 			description:     "with pagination parameters larger than length",
@@ -480,7 +488,7 @@ func TestGetTreeEntries_successful(t *testing.T) {
 			pageToken:   "",
 			entries:     rootEntries[0:2],
 			pageLimit:   2,
-			cursor:      "eyJGaWxlTmFtZSI6Ii5naXRpZ25vcmUifQ==",
+			cursor:      "eyJmaWxlX25hbWUiOiIuZ2l0aWdub3JlIn0=",
 		},
 	}
 
@@ -524,7 +532,6 @@ func TestGetTreeEntries_successful(t *testing.T) {
 
 					if testCase.pageLimit > 0 && len(testCase.entries) < len(rootEntries) {
 						require.NotNil(t, cursor)
-
 						require.Equal(t, testCase.cursor, cursor.NextCursor)
 					}
 				})
@@ -533,13 +540,6 @@ func TestGetTreeEntries_successful(t *testing.T) {
 	}
 }
 
-func getPageToken(t *testing.T, entry *gitalypb.TreeEntry) string {
-	pageToken, err := encodePageToken(entry)
-	require.NoError(t, err)
-
-	return pageToken
-}
-
 func TestGetTreeEntries_unsuccessful(t *testing.T) {
 	t.Parallel()
 	ctx := testhelper.Context(t)