Nested folders: Improve performance of shared with me dashboards listing (#81590)

* Nested folders: Improve performance of shared with me dashboards listing

* Fix tests

* Clean up guardian
This commit is contained in:
Alexander Zobnin 2024-01-31 18:25:11 +03:00 committed by GitHub
parent b4d363e8fe
commit 1bcd597bc0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 20 additions and 43 deletions

View File

@ -598,35 +598,29 @@ func (dr *DashboardServiceImpl) filterUserSharedDashboards(ctx context.Context,
folderUIDs = append(folderUIDs, dashboard.FolderUID)
}
dashFolders, err := dr.folderStore.GetFolders(ctx, user.GetOrgID(), folderUIDs)
// GetFolders return only folders available to user. So we can use is to check access.
userDashFolders, err := dr.folderService.GetFolders(ctx, folder.GetFoldersQuery{
UIDs: folderUIDs,
OrgID: user.GetOrgID(),
SignedInUser: user,
})
if err != nil {
return nil, folder.ErrInternal.Errorf("failed to fetch parent folders from store: %w", err)
}
dashFoldersMap := make(map[string]*folder.Folder, 0)
for _, f := range userDashFolders {
dashFoldersMap[f.UID] = f
}
for _, dashboard := range userDashboards {
// Filter out dashboards if user has access to parent folder
if dashboard.FolderUID == "" {
continue
}
dashFolder, ok := dashFolders[dashboard.FolderUID]
if !ok {
dr.log.Error("failed to fetch folder by UID from store", "uid", dashboard.FolderUID)
continue
}
g, err := guardian.NewByFolder(ctx, dashFolder, user.GetOrgID(), user)
if err != nil {
dr.log.Error("failed to check folder permissions", "folder uid", dashboard.FolderUID, "error", err)
continue
}
canView, err := g.CanView()
if err != nil {
dr.log.Error("failed to fetch dashboard", "uid", dashboard.UID, "error", err)
continue
}
if !canView {
_, hasAccess := dashFoldersMap[dashboard.FolderUID]
if !hasAccess {
filteredDashboards = append(filteredDashboards, dashboard)
}
}

View File

@ -1293,7 +1293,7 @@ func TestIntegrationNestedFolderSharedWithMe(t *testing.T) {
acmock.NewMockedPermissionsService(),
dashboardPermissions,
actest.FakeAccessControl{},
foldertest.NewFakeService(),
serviceWithFlagOn,
nil,
)
require.NoError(t, err)
@ -1318,13 +1318,13 @@ func TestIntegrationNestedFolderSharedWithMe(t *testing.T) {
SignedInUser: &signedInAdminUser,
}
guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{
CanSaveValue: true,
CanViewValue: true,
})
t.Run("Should get folders shared with given user", func(t *testing.T) {
depth := 3
origNewGuardian := guardian.New
guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{
CanSaveValue: true,
CanViewValue: true,
})
ancestorFoldersWithPermissions := CreateSubtreeInStore(t, nestedFolderStore, serviceWithFlagOn, depth, "withPermissions", createCmd)
ancestorFoldersWithoutPermissions := CreateSubtreeInStore(t, nestedFolderStore, serviceWithFlagOn, depth, "withoutPermissions", createCmd)
@ -1338,17 +1338,6 @@ func TestIntegrationNestedFolderSharedWithMe(t *testing.T) {
// nolint:staticcheck
dash2 := insertTestDashboard(t, serviceWithFlagOn.dashboardStore, "dashboard in subfolder", orgID, subfolder.ID, subfolder.UID, "prod")
guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{
CanSaveValue: true,
CanViewValue: true,
CanViewUIDs: []string{
ancestorFoldersWithPermissions[0].UID,
ancestorFoldersWithPermissions[1].UID,
ancestorFoldersWithoutPermissions[1].UID,
dash1.UID,
dash2.UID,
},
})
signedInUser.Permissions[orgID][dashboards.ActionFoldersRead] = []string{
dashboards.ScopeFoldersProvider.GetResourceScopeUID(ancestorFoldersWithPermissions[0].UID),
// Add permission to the subfolder of folder with permission (to check deduplication)
@ -1390,7 +1379,7 @@ func TestIntegrationNestedFolderSharedWithMe(t *testing.T) {
require.NotContains(t, sharedDashboardsUIDs, dash2.UID)
t.Cleanup(func() {
guardian.New = origNewGuardian
//guardian.New = origNewGuardian
toDelete := make([]string, 0, len(ancestorFoldersWithPermissions)+len(ancestorFoldersWithoutPermissions))
for _, ancestor := range append(ancestorFoldersWithPermissions, ancestorFoldersWithoutPermissions...) {
toDelete = append(toDelete, ancestor.UID)
@ -1402,11 +1391,6 @@ func TestIntegrationNestedFolderSharedWithMe(t *testing.T) {
t.Run("Should get org folders visible", func(t *testing.T) {
depth := 3
origNewGuardian := guardian.New
guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{
CanSaveValue: true,
CanViewValue: true,
})
// create folder sctructure like this:
// tree1-folder-0
@ -1428,7 +1412,6 @@ func TestIntegrationNestedFolderSharedWithMe(t *testing.T) {
}
t.Cleanup(func() {
guardian.New = origNewGuardian
toDelete := make([]string, 0, len(tree1)+len(tree2))
for _, f := range append(tree1, tree2...) {
toDelete = append(toDelete, f.UID)