Storage: Add maxFiles to list functions (#76414)

* Storage: Add maxFiles to list functions * Add maxDataPoints argument to listFiles function * Add maxFiles to ResourceDimensionEditor * Update pkg/services/store/http.go * rename First to Limit --------- Co-authored-by: jennyfana <110450222+jennyfana@users.noreply.github.com> Co-authored-by: nmarrs <nathanielmarrs@gmail.com> Co-authored-by: Ryan McKinley <ryantxu@gmail.com>
2025-02-25 18:55:37 -06:00 · 2023-11-01 09:42:24 -07:00
parent f6d3238505
commit 9116043453
20 changed files with 78 additions and 52 deletions
--- a/pkg/infra/filestorage/api.go
+++ b/pkg/infra/filestorage/api.go
@@ -93,8 +93,10 @@ type FileMetadata struct {
 }

 type Paging struct {
+	// The number of items to return
+	Limit int
+	// Starting after the key
 	After string
-	First int
 }

 type UpsertFileCommand struct {
--- a/pkg/infra/filestorage/cdk_blob_filestorage.go
+++ b/pkg/infra/filestorage/cdk_blob_filestorage.go
@@ -292,7 +292,7 @@ func (c cdkBlobStorage) list(ctx context.Context, folderPath string, paging *Pag
 	})}

 	recursive := options.Recursive
-	pageSize := paging.First
+	pageSize := paging.Limit

 	foundCursor := true
 	if paging.After != "" {
--- a/pkg/infra/filestorage/db_filestorage.go
+++ b/pkg/infra/filestorage/db_filestorage.go
@@ -345,7 +345,7 @@ func (s dbFileStorage) List(ctx context.Context, folderPath string, paging *Pagi

 		sess.OrderBy("path")

-		pageSize := paging.First
+		pageSize := paging.Limit
 		sess.Limit(pageSize + 1)

 		if cursor != "" {
--- a/pkg/infra/filestorage/fs_integration_test.go
+++ b/pkg/infra/filestorage/fs_integration_test.go
@@ -467,7 +467,7 @@ func TestIntegrationFsStorage(t *testing.T) {
 						},
 					},
 					queryListFiles{
-						input: queryListFilesInput{path: "/", options: &ListOptions{Recursive: true}, paging: &Paging{First: 2, After: ""}},
+						input: queryListFilesInput{path: "/", options: &ListOptions{Recursive: true}, paging: &Paging{Limit: 2, After: ""}},
 						list:  checks(listSize(2), listHasMore(true), listLastPath("/folder1/b")),
 						files: [][]any{
 							checks(fPath("/folder1/a")),
@@ -475,7 +475,7 @@ func TestIntegrationFsStorage(t *testing.T) {
 						},
 					},
 					queryListFiles{
-						input: queryListFilesInput{path: "/", options: &ListOptions{Recursive: true, WithFiles: true, WithFolders: true}, paging: &Paging{First: 2, After: ""}},
+						input: queryListFilesInput{path: "/", options: &ListOptions{Recursive: true, WithFiles: true, WithFolders: true}, paging: &Paging{Limit: 2, After: ""}},
 						list:  checks(listSize(2), listHasMore(true), listLastPath("/folder1/a")),
 						files: [][]any{
 							checks(fPath("/folder1"), fMimeType(DirectoryMimeType)),
@@ -483,49 +483,49 @@ func TestIntegrationFsStorage(t *testing.T) {
 						},
 					},
 					queryListFiles{
-						input: queryListFilesInput{path: "/", options: &ListOptions{Recursive: true, WithFiles: true, WithFolders: true}, paging: &Paging{First: 1, After: "/folder1"}},
+						input: queryListFilesInput{path: "/", options: &ListOptions{Recursive: true, WithFiles: true, WithFolders: true}, paging: &Paging{Limit: 1, After: "/folder1"}},
 						list:  checks(listSize(1), listHasMore(true), listLastPath("/folder1/a")),
 						files: [][]any{
 							checks(fPath("/folder1/a")),
 						},
 					},
 					queryListFiles{
-						input: queryListFilesInput{path: "/", options: &ListOptions{Recursive: true}, paging: &Paging{First: 1, After: "/folder1/a"}},
+						input: queryListFilesInput{path: "/", options: &ListOptions{Recursive: true}, paging: &Paging{Limit: 1, After: "/folder1/a"}},
 						list:  checks(listSize(1), listHasMore(true), listLastPath("/folder1/b")),
 						files: [][]any{
 							checks(fPath("/folder1/b")),
 						},
 					},
 					queryListFiles{
-						input: queryListFilesInput{path: "/", options: &ListOptions{Recursive: true, WithFiles: true, WithFolders: true}, paging: &Paging{First: 1, After: "/folder1/a"}},
+						input: queryListFilesInput{path: "/", options: &ListOptions{Recursive: true, WithFiles: true, WithFolders: true}, paging: &Paging{Limit: 1, After: "/folder1/a"}},
 						list:  checks(listSize(1), listHasMore(true), listLastPath("/folder1/b")),
 						files: [][]any{
 							checks(fPath("/folder1/b")),
 						},
 					},
 					queryListFiles{
-						input: queryListFilesInput{path: "/", options: &ListOptions{Recursive: true}, paging: &Paging{First: 1, After: "/folder1/b"}},
+						input: queryListFilesInput{path: "/", options: &ListOptions{Recursive: true}, paging: &Paging{Limit: 1, After: "/folder1/b"}},
 						list:  checks(listSize(1), listHasMore(false), listLastPath("/folder2/c")),
 						files: [][]any{
 							checks(fPath("/folder2/c")),
 						},
 					},
 					queryListFiles{
-						input: queryListFilesInput{path: "/", options: &ListOptions{Recursive: true, WithFiles: true, WithFolders: true}, paging: &Paging{First: 1, After: "/folder1/b"}},
+						input: queryListFilesInput{path: "/", options: &ListOptions{Recursive: true, WithFiles: true, WithFolders: true}, paging: &Paging{Limit: 1, After: "/folder1/b"}},
 						list:  checks(listSize(1), listHasMore(true), listLastPath("/folder2")),
 						files: [][]any{
 							checks(fPath("/folder2"), fMimeType(DirectoryMimeType)),
 						},
 					},
 					queryListFiles{
-						input: queryListFilesInput{path: "/", options: &ListOptions{Recursive: true, WithFiles: true, WithFolders: true}, paging: &Paging{First: 1, After: "/folder2"}},
+						input: queryListFilesInput{path: "/", options: &ListOptions{Recursive: true, WithFiles: true, WithFolders: true}, paging: &Paging{Limit: 1, After: "/folder2"}},
 						list:  checks(listSize(1), listHasMore(false), listLastPath("/folder2/c")),
 						files: [][]any{
 							checks(fPath("/folder2/c")),
 						},
 					},
 					queryListFiles{
-						input: queryListFilesInput{path: "/", options: &ListOptions{Recursive: true}, paging: &Paging{First: 5, After: ""}},
+						input: queryListFilesInput{path: "/", options: &ListOptions{Recursive: true}, paging: &Paging{Limit: 5, After: ""}},
 						list:  checks(listSize(3), listHasMore(false), listLastPath("/folder2/c")),
 						files: [][]any{
 							checks(fPath("/folder1/a")),
@@ -534,7 +534,7 @@ func TestIntegrationFsStorage(t *testing.T) {
 						},
 					},
 					queryListFiles{
-						input: queryListFilesInput{path: "/", options: &ListOptions{Recursive: true, WithFiles: true, WithFolders: true}, paging: &Paging{First: 5, After: ""}},
+						input: queryListFilesInput{path: "/", options: &ListOptions{Recursive: true, WithFiles: true, WithFolders: true}, paging: &Paging{Limit: 5, After: ""}},
 						list:  checks(listSize(5), listHasMore(false), listLastPath("/folder2/c")),
 						files: [][]any{
 							checks(fPath("/folder1"), fMimeType(DirectoryMimeType)),
@@ -545,19 +545,19 @@ func TestIntegrationFsStorage(t *testing.T) {
 						},
 					},
 					queryListFiles{
-						input: queryListFilesInput{path: "/", options: &ListOptions{Recursive: true}, paging: &Paging{First: 5, After: "/folder2"}},
+						input: queryListFilesInput{path: "/", options: &ListOptions{Recursive: true}, paging: &Paging{Limit: 5, After: "/folder2"}},
 						list:  checks(listSize(1), listHasMore(false)),
 					},
 					queryListFiles{
-						input: queryListFilesInput{path: "/", options: &ListOptions{Recursive: true, WithFiles: true, WithFolders: true}, paging: &Paging{First: 5, After: "/folder2"}},
+						input: queryListFilesInput{path: "/", options: &ListOptions{Recursive: true, WithFiles: true, WithFolders: true}, paging: &Paging{Limit: 5, After: "/folder2"}},
 						list:  checks(listSize(1), listHasMore(false)),
 					},
 					queryListFiles{
-						input: queryListFilesInput{path: "/", options: &ListOptions{Recursive: true}, paging: &Paging{First: 5, After: "/folder2/c"}},
+						input: queryListFilesInput{path: "/", options: &ListOptions{Recursive: true}, paging: &Paging{Limit: 5, After: "/folder2/c"}},
 						list:  checks(listSize(0), listHasMore(false)),
 					},
 					queryListFiles{
-						input: queryListFilesInput{path: "/", options: &ListOptions{Recursive: true, WithFiles: true, WithFolders: true}, paging: &Paging{First: 5, After: "/folder2/c"}},
+						input: queryListFilesInput{path: "/", options: &ListOptions{Recursive: true, WithFiles: true, WithFolders: true}, paging: &Paging{Limit: 5, After: "/folder2/c"}},
 						list:  checks(listSize(0), listHasMore(false)),
 					},
 				},
--- a/pkg/infra/filestorage/test_utils.go
+++ b/pkg/infra/filestorage/test_utils.go
@@ -321,7 +321,7 @@ func handleQuery(t *testing.T, ctx context.Context, query interface{}, queryName
 		}
 		resp, err := fs.List(ctx, inputPath, &Paging{
 			After: "",
-			First: 100000,
+			Limit: 100000,
 		}, opts)
 		require.NotNil(t, resp)
 		require.NoError(t, err, "%s: should be able to list folders in %s", queryName, inputPath)
--- a/pkg/infra/filestorage/wrapper.go
+++ b/pkg/infra/filestorage/wrapper.go
@@ -200,12 +200,12 @@ func (b wrapper) Upsert(ctx context.Context, file *UpsertFileCommand) error {
 func (b wrapper) pagingOptionsWithDefaults(paging *Paging) *Paging {
 	if paging == nil {
 		return &Paging{
-			First: 100,
+			Limit: 100,
 		}
 	}

-	if paging.First <= 0 {
-		paging.First = 100
+	if paging.Limit <= 0 {
+		paging.Limit = 100
 	}
 	if paging.After != "" {
 		paging.After = b.addRoot(paging.After)
@@ -381,7 +381,7 @@ func (b wrapper) List(ctx context.Context, folderPath string, paging *Paging, op
 }

 func (b wrapper) isFolderEmpty(ctx context.Context, path string) (bool, error) {
-	resp, err := b.List(ctx, path, &Paging{First: 1}, &ListOptions{Recursive: true, WithFolders: true, WithFiles: true})
+	resp, err := b.List(ctx, path, &Paging{Limit: 1}, &ListOptions{Recursive: true, WithFolders: true, WithFiles: true})
 	if err != nil {
 		return false, err
 	}
--- a/pkg/services/store/http.go
+++ b/pkg/services/store/http.go
@@ -260,7 +260,8 @@ func (s *standardStorageService) doCreateFolder(c *contextmodel.ReqContext) resp
 func (s *standardStorageService) list(c *contextmodel.ReqContext) response.Response {
 	params := web.Params(c.Req)
 	path := params["*"]
-	frame, err := s.List(c.Req.Context(), c.SignedInUser, path)
+	// maxFiles of 0 will result in default behaviour from wrapper
+	frame, err := s.List(c.Req.Context(), c.SignedInUser, path, 0)
 	if err != nil {
 		return response.Error(400, "error reading path", err)
 	}
--- a/pkg/services/store/service.go
+++ b/pkg/services/store/service.go
@@ -64,7 +64,7 @@ type StorageService interface {
 	RegisterHTTPRoutes(routing.RouteRegister)

 	// List folder contents
-	List(ctx context.Context, user *user.SignedInUser, path string) (*StorageListFrame, error)
+	List(ctx context.Context, user *user.SignedInUser, path string, maxFiles int) (*StorageListFrame, error)

 	// Read raw file contents out of the store
 	Read(ctx context.Context, user *user.SignedInUser, path string) (*filestorage.File, error)
@@ -340,9 +340,9 @@ func getOrgId(user *user.SignedInUser) int64 {
 	return user.OrgID
 }

-func (s *standardStorageService) List(ctx context.Context, user *user.SignedInUser, path string) (*StorageListFrame, error) {
+func (s *standardStorageService) List(ctx context.Context, user *user.SignedInUser, path string, maxFiles int) (*StorageListFrame, error) {
 	guardian := s.authService.newGuardian(ctx, user, getFirstSegment(path))
-	return s.tree.ListFolder(ctx, getOrgId(user), path, guardian.getPathFilter(ActionFilesRead))
+	return s.tree.ListFolder(ctx, getOrgId(user), path, maxFiles, guardian.getPathFilter(ActionFilesRead))
 }

 func (s *standardStorageService) Read(ctx context.Context, user *user.SignedInUser, path string) (*filestorage.File, error) {
--- a/pkg/services/store/service_test.go
+++ b/pkg/services/store/service_test.go
@@ -74,7 +74,7 @@ func TestListFiles(t *testing.T) {
 	store := newStandardStorageService(db.InitTestDB(t), roots, func(orgId int64) []storageRuntime {
 		return make([]storageRuntime, 0)
 	}, allowAllAuthService, cfg, nil)
-	frame, err := store.List(context.Background(), dummyUser, "public/maps")
+	frame, err := store.List(context.Background(), dummyUser, "public/maps", 0)
 	require.NoError(t, err)

 	experimental.CheckGoldenJSONFrame(t, "testdata", "public_testdata.golden", frame.Frame, true)
@@ -95,7 +95,7 @@ func TestListFilesWithoutPermissions(t *testing.T) {
 	store := newStandardStorageService(db.InitTestDB(t), roots, func(orgId int64) []storageRuntime {
 		return make([]storageRuntime, 0)
 	}, denyAllAuthService, cfg, nil)
-	frame, err := store.List(context.Background(), dummyUser, "public/maps")
+	frame, err := store.List(context.Background(), dummyUser, "public/maps", 0)
 	require.NoError(t, err)
 	rowLen, err := frame.RowLen()
 	require.NoError(t, err)
@@ -371,7 +371,7 @@ func TestContentRootWithNestedStorage(t *testing.T) {
 				Files: []*filestorage.File{},
 			}, nil)

-			_, err := store.List(context.Background(), test.user, RootContent+"/"+test.nestedRoot)
+			_, err := store.List(context.Background(), test.user, RootContent+"/"+test.nestedRoot, 0)
 			require.NoError(t, err)
 		})

@@ -387,7 +387,7 @@ func TestContentRootWithNestedStorage(t *testing.T) {
 				Files: []*filestorage.File{},
 			}, nil)

-			_, err := store.List(context.Background(), test.user, strings.Join([]string{RootContent, test.nestedRoot, "folder1", "folder2"}, "/"))
+			_, err := store.List(context.Background(), test.user, strings.Join([]string{RootContent, test.nestedRoot, "folder1", "folder2"}, "/"), 0)
 			require.NoError(t, err)
 		})

@@ -434,16 +434,16 @@ func TestContentRootWithNestedStorage(t *testing.T) {
 				Files: []*filestorage.File{},
 			}, nil)

-			_, err := store.List(context.Background(), test.user, strings.Join([]string{RootContent, "not-nested-content"}, "/"))
+			_, err := store.List(context.Background(), test.user, strings.Join([]string{RootContent, "not-nested-content"}, "/"), 0)
 			require.NoError(t, err)

-			_, err = store.List(context.Background(), test.user, strings.Join([]string{RootContent, "a", "b", "c"}, "/"))
+			_, err = store.List(context.Background(), test.user, strings.Join([]string{RootContent, "a", "b", "c"}, "/"), 0)
 			require.NoError(t, err)

-			_, err = store.List(context.Background(), test.user, strings.Join([]string{RootContent, test.nestedRoot + "a"}, "/"))
+			_, err = store.List(context.Background(), test.user, strings.Join([]string{RootContent, test.nestedRoot + "a"}, "/"), 0)
 			require.NoError(t, err)

-			_, err = store.List(context.Background(), test.user, strings.Join([]string{RootContent, test.nestedRoot + "a", "b"}, "/"))
+			_, err = store.List(context.Background(), test.user, strings.Join([]string{RootContent, test.nestedRoot + "a", "b"}, "/"), 0)
 			require.NoError(t, err)
 		})

@@ -536,7 +536,7 @@ func TestShadowingExistingFolderByNestedContentRoot(t *testing.T) {
 		AllowUnsanitizedSvgUpload: true,
 	}

-	resp, err := store.List(ctx, globalUser, "content/nested")
+	resp, err := store.List(ctx, globalUser, "content/nested", 0)
 	require.NoError(t, err)
 	require.NotNil(t, resp)

@@ -544,7 +544,7 @@ func TestShadowingExistingFolderByNestedContentRoot(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, 0, rowLen) // nested storage is empty

-	resp, err = store.List(ctx, globalUser, "content")
+	resp, err = store.List(ctx, globalUser, "content", 0)
 	require.NoError(t, err)
 	require.NotNil(t, resp)

--- a/pkg/services/store/tree.go
+++ b/pkg/services/store/tree.go
@@ -165,7 +165,7 @@ func (t *nestedTree) getStorages(orgId int64) []storageRuntime {
 	return storages
 }

-func (t *nestedTree) ListFolder(ctx context.Context, orgId int64, path string, accessFilter filestorage.PathFilter) (*StorageListFrame, error) {
+func (t *nestedTree) ListFolder(ctx context.Context, orgId int64, path string, maxFiles int, accessFilter filestorage.PathFilter) (*StorageListFrame, error) {
 	if path == "" || path == "/" {
 		t.assureOrgIsInitialized(orgId)

@@ -224,12 +224,16 @@ func (t *nestedTree) ListFolder(ctx context.Context, orgId int64, path string, a
 		)
 	}

-	listResponse, err := store.List(ctx, path, nil, &filestorage.ListOptions{
-		Recursive:   false,
-		WithFolders: true,
-		WithFiles:   true,
-		Filter:      pathFilter,
-	})
+	listResponse, err := store.List(ctx, path,
+		&filestorage.Paging{
+			Limit: maxFiles,
+		},
+		&filestorage.ListOptions{
+			Recursive:   false,
+			WithFolders: true,
+			WithFiles:   true,
+			Filter:      pathFilter,
+		})

 	if err != nil {
 		return nil, err
--- a/pkg/services/store/types.go
+++ b/pkg/services/store/types.go
@@ -40,7 +40,7 @@ type WriteValueResponse struct {

 type storageTree interface {
 	GetFile(ctx context.Context, orgId int64, path string) (*filestorage.File, error)
-	ListFolder(ctx context.Context, orgId int64, path string, accessFilter filestorage.PathFilter) (*StorageListFrame, error)
+	ListFolder(ctx context.Context, orgId int64, path string, maxFiles int, accessFilter filestorage.PathFilter) (*StorageListFrame, error)
 }

 //-------------------------------------------
--- a/pkg/tsdb/grafanads/grafana.go
+++ b/pkg/tsdb/grafanads/grafana.go
@@ -124,7 +124,8 @@ func (s *Service) doListQuery(ctx context.Context, query backend.DataQuery) back
 	}

 	path := store.RootPublicStatic + "/" + q.Path
-	listFrame, err := s.store.List(ctx, nil, path)
+	maxFiles := int(query.MaxDataPoints)
+	listFrame, err := s.store.List(ctx, nil, path, maxFiles)
 	response.Error = err
 	if listFrame != nil {
 		response.Frames = data.Frames{listFrame.Frame}