Folders: Add pagination to list (#102334)

pkg/registry/apis/folders/legacy_storage.go

@@ -90,14 +90,22 @@ func (s *legacyStorage) List(ctx context.Context, options *internalversion.ListO
 		return nil, err
 	}
 
-	// List must return all folders
-	hits, err := s.service.GetFoldersLegacy(ctx, folder.GetFoldersQuery{
+	query := folder.GetFoldersQuery{
 		SignedInUser: user,
 		OrgID:        orgId,
-		// TODO: enable pagination
-		// Limit:        paging.page,
-		// Page:         paging.limit,
-	})
+	}
+	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
+	}
+
+	hits, err := s.service.GetFoldersLegacy(ctx, query)
 	if err != nil {
 		return nil, err
 	}

pkg/registry/apis/folders/legacy_storage_test.go

@@ -2,12 +2,15 @@ package folders
 
 import (
 	"context"
+	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/require"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metainternalversion "k8s.io/apimachinery/pkg/apis/meta/internalversion"
 
+	"encoding/base64"
+
 	"github.com/grafana/grafana/pkg/apimachinery/identity"
 	"github.com/grafana/grafana/pkg/apis/folder/v0alpha1"
 	"github.com/grafana/grafana/pkg/services/folder"
@@ -57,3 +60,62 @@ func TestLegacyStorageList(t *testing.T) {
 	}
 	require.ElementsMatch(t, uidsFromServiceFolder, uidsReturnedByList)
 }
+
+func TestLegacyStorage_List_Pagination(t *testing.T) {
+	usr := &user.SignedInUser{UserID: 1}
+	ctx := identity.WithRequester(context.Background(), usr)
+	folderService := &foldertest.FakeService{}
+	storage := legacyStorage{
+		service:    folderService,
+		namespacer: func(_ int64) string { return "1" },
+	}
+
+	t.Run("should set correct continue token", func(t *testing.T) {
+		options := &metainternalversion.ListOptions{
+			Limit: 2,
+		}
+		folders := make([]*folder.Folder, 2)
+		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.(*v0alpha1.FolderList)
+		require.True(t, ok)
+		token, err := base64.StdEncoding.DecodeString(list.Continue)
+		require.NoError(t, err)
+		require.Equal(t, "2|2", string(token))
+		require.Equal(t, folderService.LastQuery.Limit, int64(2))
+		require.Equal(t, folderService.LastQuery.Page, int64(1))
+	})
+
+	t.Run("should set page to 1 when limit is set without continue token", func(t *testing.T) {
+		options := &metainternalversion.ListOptions{
+			Limit: 2,
+		}
+		folders := make([]*folder.Folder, 2)
+		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.(*v0alpha1.FolderList)
+		require.True(t, ok)
+		token, err := base64.StdEncoding.DecodeString(list.Continue)
+		require.NoError(t, err)
+		require.Equal(t, "2|2", string(token))
+		require.Equal(t, int64(2), folderService.LastQuery.Limit)
+		require.Equal(t, int64(1), folderService.LastQuery.Page)
+	})
+}

pkg/services/folder/folderimpl/sqlstore.go

@@ -498,7 +498,11 @@ func (ss *FolderStoreImpl) GetFolders(ctx context.Context, q folder.GetFoldersFr
 			}
 
 			if len(q.AncestorUIDs) == 0 {
-				if q.OrderByTitle {
+				if q.Limit > 0 {
+					s.WriteString(` ORDER BY f0.title ASC`)
+					s.WriteString(` LIMIT ? OFFSET ?`)
+					args = append(args, q.Limit, (q.Page-1)*q.Limit)
+				} else if q.OrderByTitle {
 					s.WriteString(` ORDER BY f0.title ASC`)
 				}
 

pkg/services/folder/folderimpl/sqlstore_test.go

@@ -878,7 +878,7 @@ func TestIntegrationGetFolders(t *testing.T) {
 	for i := 0; i < foldersNum; i++ {
 		uid := util.GenerateShortUID()
 		f, err := folderStore.Create(context.Background(), folder.CreateFolderCommand{
-			Title:       folderTitle,
+			Title:       folderTitle + fmt.Sprintf("-%d", i),
 			Description: folderDsc,
 			OrgID:       orgID,
 			UID:         uid,
@@ -982,6 +982,23 @@ func TestIntegrationGetFolders(t *testing.T) {
 		assert.NotEmpty(t, actualFolder.Updated)
 		assert.NotEmpty(t, actualFolder.URL)
 	})
+
+	t.Run("get folders with limit and page should work as expected", func(t *testing.T) {
+		q := folder.NewGetFoldersQuery(folder.GetFoldersQuery{
+			OrgID: orgID,
+			UIDs:  uids,
+			Limit: 3,
+			Page:  2,
+		})
+
+		actualFolders, err := folderStore.GetFolders(context.Background(), q)
+		require.NoError(t, err)
+		assert.Equal(t, 3, len(actualFolders))
+
+		for i, actualFolder := range actualFolders {
+			assert.Equal(t, fmt.Sprintf("folder1-%d", i+3), actualFolder.Title)
+		}
+	})
 }
 
 func CreateOrg(t *testing.T, db db.DB, cfg *setting.Cfg) int64 {

pkg/services/folder/foldertest/foldertest.go

@@ -13,6 +13,7 @@ type FakeService struct {
 	ExpectedHitList          model.HitList
 	ExpectedError            error
 	ExpectedDescendantCounts map[string]int64
+	LastQuery                folder.GetFoldersQuery
 }
 
 func NewFakeService() *FakeService {
@@ -90,5 +91,6 @@ func (s *FakeService) SearchFolders(ctx context.Context, q folder.SearchFoldersQ
 }
 
 func (s *FakeService) GetFoldersLegacy(ctx context.Context, q folder.GetFoldersQuery) ([]*folder.Folder, error) {
+	s.LastQuery = q
 	return s.ExpectedFolders, s.ExpectedError
 }

pkg/services/folder/model.go

@@ -175,6 +175,10 @@ type GetFoldersQuery struct {
 	WithFullpathUIDs bool
 	BatchSize        uint64
 
+	// Pagination options
+	Limit int64
+	Page  int64
+
 	// OrderByTitle is used to sort the folders by title
 	// Set to true when ordering is meaningful (used for listing folders)
 	// otherwise better to keep it false since ordering can have a performance impact