Unified storage bugfix legacy folders getting first page (#109554)

* When creating a new continue token, it defaults to page 1. Also use constants for default limit and page number.

* Update tests for continue token.

* When listing legacy folders, the continue token will have all paging info in it. Simplifies paging logic and fixes bug when limit not specified.

* Adds regression test to ensure default page limit is enforced.

* remove test comment

pkg/registry/apis/folders/continue.go

@@ -14,9 +14,13 @@ type continueToken struct {
 	limit int64
 }
 
+const defaultPageLimit = 100
+const defaultPageNumber = 1
+
 func readContinueToken(options *internalversion.ListOptions) (*continueToken, error) {
 	t := &continueToken{
-		limit: 100, // default page size
+		limit: defaultPageLimit,  // default page size
+		page:  defaultPageNumber, // default page number
 	}
 	if options.Continue == "" {
 		if options.Limit > 0 {

pkg/registry/apis/folders/continue_test.go

@@ -10,15 +10,15 @@ import (
 func TestContinueToken(t *testing.T) {
 	token, err := readContinueToken(&internalversion.ListOptions{})
 	require.NoError(t, err)
-	require.Equal(t, int64(100), token.limit)
+	require.Equal(t, int64(defaultPageLimit), token.limit)
-	require.Equal(t, int64(0), token.page)
+	require.Equal(t, int64(defaultPageNumber), token.page)
 
 	next := token.GetNextPageToken()
-	require.Equal(t, "MTAwfDE=", next)
+	require.Equal(t, "MTAwfDI=", next)
 	token, err = readContinueToken(&internalversion.ListOptions{Continue: next})
 	require.NoError(t, err)
-	require.Equal(t, int64(100), token.limit)
+	require.Equal(t, int64(defaultPageLimit), token.limit)
-	require.Equal(t, int64(1), token.page) // <<< +1
+	require.Equal(t, int64(defaultPageNumber+1), token.page) // <<< +1
 
 	// Error if the limit has changed
 	_, err = readContinueToken(&internalversion.ListOptions{Continue: next, Limit: 50})

pkg/registry/apis/folders/legacy_storage.go

@@ -95,16 +95,10 @@ func (s *legacyStorage) List(ctx context.Context, options *internalversion.ListO
 		SignedInUser: user,
 		OrgID:        orgId,
 	}
-	if options.Continue != "" {
+
-		query.Page = paging.page
+	// paging is always retrieved from the continue token
-		query.Limit = paging.limit
+	query.Limit = paging.limit
-	} else if options.Limit > 0 {
+	query.Page = paging.page
-		query.Limit = options.Limit
-		query.Page = 1
-		// also need to update the paging token so the continue token is correct
-		paging.limit = options.Limit
-		paging.page = 1
-	}
 
 	if options.LabelSelector != nil && options.LabelSelector.Matches(labels.Set{utils.LabelGetFullpath: "true"}) {
 		query.WithFullpath = true

pkg/registry/apis/folders/legacy_storage_test.go

@@ -121,6 +121,31 @@ func TestLegacyStorage_List_Pagination(t *testing.T) {
 		require.Equal(t, int64(2), folderService.LastQuery.Limit)
 		require.Equal(t, int64(1), folderService.LastQuery.Page)
 	})
+
+	t.Run("should set page limit to default when no limit is specified in options", func(t *testing.T) {
+		options := &metainternalversion.ListOptions{}
+		folders := make([]*folder.Folder, defaultPageLimit)
+		for i := range folders {
+			folders[i] = &folder.Folder{
+				UID:   fmt.Sprintf("folder-%d", i),
+				Title: fmt.Sprintf("Folder %d", i),
+			}
+		}
+		folderService.ExpectedFolders = folders
+
+		result, err := storage.List(ctx, options)
+		require.NoError(t, err)
+
+		list, ok := result.(*folderv1.FolderList)
+		require.True(t, ok)
+
+		// assert returned continue token is correct and previous paging was correct
+		token, err := base64.StdEncoding.DecodeString(list.Continue)
+		require.NoError(t, err)
+		require.Equal(t, "100|2", string(token))
+		require.Equal(t, int64(defaultPageLimit), folderService.LastQuery.Limit)
+		require.Equal(t, int64(1), folderService.LastQuery.Page)
+	})
 }
 
 func TestLegacyStorage_List_LabelSelector(t *testing.T) {