From 384ec28dfd293a088ef3b2f0642e4d1915460add Mon Sep 17 00:00:00 2001 From: owensmallwood Date: Wed, 13 Aug 2025 08:50:28 -0600 Subject: [PATCH] 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 | 6 ++++- pkg/registry/apis/folders/continue_test.go | 10 ++++---- pkg/registry/apis/folders/legacy_storage.go | 14 +++-------- .../apis/folders/legacy_storage_test.go | 25 +++++++++++++++++++ 4 files changed, 39 insertions(+), 16 deletions(-) diff --git a/pkg/registry/apis/folders/continue.go b/pkg/registry/apis/folders/continue.go index 8ff86b627ea..cb186f91612 100644 --- a/pkg/registry/apis/folders/continue.go +++ b/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 { diff --git a/pkg/registry/apis/folders/continue_test.go b/pkg/registry/apis/folders/continue_test.go index 2763c3522c6..7bde6d4b4e6 100644 --- a/pkg/registry/apis/folders/continue_test.go +++ b/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(0), token.page) + require.Equal(t, int64(defaultPageLimit), token.limit) + 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(1), token.page) // <<< +1 + require.Equal(t, int64(defaultPageLimit), token.limit) + require.Equal(t, int64(defaultPageNumber+1), token.page) // <<< +1 // Error if the limit has changed _, err = readContinueToken(&internalversion.ListOptions{Continue: next, Limit: 50}) diff --git a/pkg/registry/apis/folders/legacy_storage.go b/pkg/registry/apis/folders/legacy_storage.go index f71993bfea2..d8ebda1fd65 100644 --- a/pkg/registry/apis/folders/legacy_storage.go +++ b/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 - query.Limit = paging.limit - } else if options.Limit > 0 { - 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 - } + + // paging is always retrieved from the continue token + query.Limit = paging.limit + query.Page = paging.page if options.LabelSelector != nil && options.LabelSelector.Matches(labels.Set{utils.LabelGetFullpath: "true"}) { query.WithFullpath = true diff --git a/pkg/registry/apis/folders/legacy_storage_test.go b/pkg/registry/apis/folders/legacy_storage_test.go index 4871c511a20..0edabebb791 100644 --- a/pkg/registry/apis/folders/legacy_storage_test.go +++ b/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) {