Dashboards: Optimize get shared with me (#103013)
This commit is contained in:
@@ -23,14 +23,14 @@ type DashboardService interface {
|
||||
// To fetch a dashboard under root by title should set the folder UID to point to an empty string
|
||||
// eg. util.Pointer("")
|
||||
GetDashboard(ctx context.Context, query *GetDashboardQuery) (*Dashboard, error)
|
||||
GetDashboards(ctx context.Context, query *GetDashboardsQuery) ([]*Dashboard, error)
|
||||
GetDashboards(ctx context.Context, query *GetDashboardsQuery) ([]*Dashboard, error) // use sparely only if you truly need dashboard.Data
|
||||
GetDashboardTags(ctx context.Context, query *GetDashboardTagsQuery) ([]*DashboardTagCloudItem, error)
|
||||
GetDashboardUIDByID(ctx context.Context, query *GetDashboardRefByIDQuery) (*DashboardRef, error)
|
||||
ImportDashboard(ctx context.Context, dto *SaveDashboardDTO) (*Dashboard, error)
|
||||
SaveDashboard(ctx context.Context, dto *SaveDashboardDTO, allowUiUpdate bool) (*Dashboard, error)
|
||||
SearchDashboards(ctx context.Context, query *FindPersistedDashboardsQuery) (model.HitList, error)
|
||||
CountInFolders(ctx context.Context, orgID int64, folderUIDs []string, user identity.Requester) (int64, error)
|
||||
GetDashboardsSharedWithUser(ctx context.Context, user identity.Requester) ([]*Dashboard, error)
|
||||
GetDashboardsSharedWithUser(ctx context.Context, user identity.Requester) ([]*DashboardRef, error)
|
||||
GetAllDashboards(ctx context.Context) ([]*Dashboard, error)
|
||||
GetAllDashboardsByOrgId(ctx context.Context, orgID int64) ([]*Dashboard, error)
|
||||
SoftDeleteDashboard(ctx context.Context, orgID int64, dashboardUid string) error
|
||||
|
||||
@@ -377,23 +377,23 @@ func (_m *FakeDashboardService) GetDashboards(ctx context.Context, query *GetDas
|
||||
}
|
||||
|
||||
// GetDashboardsSharedWithUser provides a mock function with given fields: ctx, user
|
||||
func (_m *FakeDashboardService) GetDashboardsSharedWithUser(ctx context.Context, user identity.Requester) ([]*Dashboard, error) {
|
||||
func (_m *FakeDashboardService) GetDashboardsSharedWithUser(ctx context.Context, user identity.Requester) ([]*DashboardRef, error) {
|
||||
ret := _m.Called(ctx, user)
|
||||
|
||||
if len(ret) == 0 {
|
||||
panic("no return value specified for GetDashboardsSharedWithUser")
|
||||
}
|
||||
|
||||
var r0 []*Dashboard
|
||||
var r0 []*DashboardRef
|
||||
var r1 error
|
||||
if rf, ok := ret.Get(0).(func(context.Context, identity.Requester) ([]*Dashboard, error)); ok {
|
||||
if rf, ok := ret.Get(0).(func(context.Context, identity.Requester) ([]*DashboardRef, error)); ok {
|
||||
return rf(ctx, user)
|
||||
}
|
||||
if rf, ok := ret.Get(0).(func(context.Context, identity.Requester) []*Dashboard); ok {
|
||||
if rf, ok := ret.Get(0).(func(context.Context, identity.Requester) []*DashboardRef); ok {
|
||||
r0 = rf(ctx, user)
|
||||
} else {
|
||||
if ret.Get(0) != nil {
|
||||
r0 = ret.Get(0).([]*Dashboard)
|
||||
r0 = ret.Get(0).([]*DashboardRef)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -906,7 +906,7 @@ func (d *dashboardStore) GetDashboardUIDByID(ctx context.Context, query *dashboa
|
||||
|
||||
us := &dashboards.DashboardRef{}
|
||||
err := d.store.WithDbSession(ctx, func(sess *db.Session) error {
|
||||
var rawSQL = `SELECT uid, slug from dashboard WHERE Id=?`
|
||||
var rawSQL = `SELECT uid, slug, folder_uid from dashboard WHERE Id=?`
|
||||
exists, err := sess.SQL(rawSQL, query.ID).Get(us)
|
||||
if err != nil {
|
||||
return err
|
||||
|
||||
@@ -188,6 +188,7 @@ func TestIntegrationDashboardDataAccess(t *testing.T) {
|
||||
queryResult, err := dashboardStore.GetDashboardUIDByID(context.Background(), &query)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, queryResult.UID, savedDash.UID)
|
||||
require.Equal(t, queryResult.FolderUID, savedFolder.UID)
|
||||
})
|
||||
|
||||
t.Run("Shouldn't be able to get a dashboard with just an OrgID", func(t *testing.T) {
|
||||
|
||||
@@ -288,8 +288,9 @@ type GetDashboardsByPluginIDQuery struct {
|
||||
}
|
||||
|
||||
type DashboardRef struct {
|
||||
UID string `xorm:"uid"`
|
||||
Slug string
|
||||
UID string `xorm:"uid"`
|
||||
Slug string
|
||||
FolderUID string `xorm:"folder_uid"`
|
||||
}
|
||||
|
||||
type GetDashboardRefByIDQuery struct {
|
||||
|
||||
@@ -1345,12 +1345,13 @@ func (dr *DashboardServiceImpl) GetDashboardUIDByID(ctx context.Context, query *
|
||||
return nil, fmt.Errorf("unexpected number of dashboards found: %d. desired: 1", len(result))
|
||||
}
|
||||
|
||||
return &dashboards.DashboardRef{UID: result[0].UID, Slug: result[0].Slug}, nil
|
||||
return &dashboards.DashboardRef{UID: result[0].UID, Slug: result[0].Slug, FolderUID: result[0].FolderUID}, nil
|
||||
}
|
||||
|
||||
return dr.dashboardStore.GetDashboardUIDByID(ctx, query)
|
||||
}
|
||||
|
||||
// expensive query in new flow !! use sparingly - only if you truly need dashboard.Data
|
||||
func (dr *DashboardServiceImpl) GetDashboards(ctx context.Context, query *dashboards.GetDashboardsQuery) ([]*dashboards.Dashboard, error) {
|
||||
if dr.features.IsEnabledGlobally(featuremgmt.FlagKubernetesClientDashboardsFolders) {
|
||||
if query.OrgID == 0 {
|
||||
@@ -1386,17 +1387,16 @@ func (dr *DashboardServiceImpl) GetDashboards(ctx context.Context, query *dashbo
|
||||
return dr.dashboardStore.GetDashboards(ctx, query)
|
||||
}
|
||||
|
||||
func (dr *DashboardServiceImpl) GetDashboardsSharedWithUser(ctx context.Context, user identity.Requester) ([]*dashboards.Dashboard, error) {
|
||||
func (dr *DashboardServiceImpl) GetDashboardsSharedWithUser(ctx context.Context, user identity.Requester) ([]*dashboards.DashboardRef, error) {
|
||||
return dr.getDashboardsSharedWithUser(ctx, user)
|
||||
}
|
||||
|
||||
func (dr *DashboardServiceImpl) getDashboardsSharedWithUser(ctx context.Context, user identity.Requester) ([]*dashboards.Dashboard, error) {
|
||||
func (dr *DashboardServiceImpl) getDashboardsSharedWithUser(ctx context.Context, user identity.Requester) ([]*dashboards.DashboardRef, error) {
|
||||
ctx, span := tracer.Start(ctx, "dashboards.service.getDashboardsSharedWithUser")
|
||||
defer span.End()
|
||||
|
||||
permissions := user.GetPermissions()
|
||||
dashboardPermissions := permissions[dashboards.ActionDashboardsRead]
|
||||
sharedDashboards := make([]*dashboards.Dashboard, 0)
|
||||
dashboardUids := make([]string, 0)
|
||||
for _, p := range dashboardPermissions {
|
||||
if dashboardUid, found := strings.CutPrefix(p, dashboards.ScopeDashboardsPrefix); found {
|
||||
@@ -1407,27 +1407,42 @@ func (dr *DashboardServiceImpl) getDashboardsSharedWithUser(ctx context.Context,
|
||||
}
|
||||
|
||||
if len(dashboardUids) == 0 {
|
||||
return sharedDashboards, nil
|
||||
return []*dashboards.DashboardRef{}, nil
|
||||
}
|
||||
|
||||
dashboardsQuery := &dashboards.GetDashboardsQuery{
|
||||
DashboardUIDs: dashboardUids,
|
||||
OrgID: user.GetOrgID(),
|
||||
}
|
||||
sharedDashboards, err := dr.GetDashboards(ctx, dashboardsQuery)
|
||||
|
||||
var err error
|
||||
var dashs []*dashboards.Dashboard
|
||||
if dr.features.IsEnabledGlobally(featuremgmt.FlagKubernetesClientDashboardsFolders) {
|
||||
dashs, err = dr.searchDashboardsThroughK8s(ctx, &dashboards.FindPersistedDashboardsQuery{
|
||||
DashboardUIDs: dashboardUids,
|
||||
OrgId: user.GetOrgID(),
|
||||
})
|
||||
} else {
|
||||
dashs, err = dr.dashboardStore.GetDashboards(ctx, dashboardsQuery)
|
||||
}
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
sharedDashboards := make([]*dashboards.DashboardRef, len(dashs))
|
||||
for i, d := range dashs {
|
||||
sharedDashboards[i] = &dashboards.DashboardRef{UID: d.UID, Slug: d.Slug, FolderUID: d.FolderUID}
|
||||
}
|
||||
|
||||
return dr.filterUserSharedDashboards(ctx, user, sharedDashboards)
|
||||
}
|
||||
|
||||
// filterUserSharedDashboards filter dashboards directly assigned to user, but not located in folders with view permissions
|
||||
func (dr *DashboardServiceImpl) filterUserSharedDashboards(ctx context.Context, user identity.Requester, userDashboards []*dashboards.Dashboard) ([]*dashboards.Dashboard, error) {
|
||||
func (dr *DashboardServiceImpl) filterUserSharedDashboards(ctx context.Context, user identity.Requester, userDashboards []*dashboards.DashboardRef) ([]*dashboards.DashboardRef, error) {
|
||||
ctx, span := tracer.Start(ctx, "dashboards.service.filterUserSharedDashboards")
|
||||
defer span.End()
|
||||
|
||||
filteredDashboards := make([]*dashboards.Dashboard, 0)
|
||||
|
||||
filteredDashboards := make([]*dashboards.DashboardRef, 0)
|
||||
folderUIDs := make([]string, 0)
|
||||
for _, dashboard := range userDashboards {
|
||||
folderUIDs = append(folderUIDs, dashboard.FolderUID)
|
||||
|
||||
@@ -7,6 +7,7 @@ import (
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/prometheus/client_golang/prometheus"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/mock"
|
||||
"github.com/stretchr/testify/require"
|
||||
@@ -40,6 +41,7 @@ import (
|
||||
"github.com/grafana/grafana/pkg/services/user"
|
||||
"github.com/grafana/grafana/pkg/setting"
|
||||
"github.com/grafana/grafana/pkg/storage/unified/resource"
|
||||
"golang.org/x/exp/slices"
|
||||
)
|
||||
|
||||
func TestDashboardService(t *testing.T) {
|
||||
@@ -1558,6 +1560,7 @@ func TestSearchDashboards(t *testing.T) {
|
||||
cfg: setting.NewCfg(),
|
||||
dashboardStore: &fakeStore,
|
||||
folderService: fakeFolders,
|
||||
metrics: newDashboardsMetrics(prometheus.NewRegistry()),
|
||||
}
|
||||
|
||||
expectedResult := model.HitList{
|
||||
@@ -1676,6 +1679,109 @@ func TestSearchDashboards(t *testing.T) {
|
||||
require.Equal(t, expectedResult, result)
|
||||
k8sCliMock.AssertExpectations(t)
|
||||
})
|
||||
|
||||
t.Run("Should handle Shared with me folder correctly", func(t *testing.T) {
|
||||
ctx, k8sCliMock := setupK8sDashboardTests(service)
|
||||
service.features = featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders, featuremgmt.FlagKubernetesClientDashboardsFolders)
|
||||
k8sCliMock.On("GetNamespace", mock.Anything, mock.Anything).Return("default")
|
||||
k8sCliMock.On("Search", mock.Anything, int64(1), mock.MatchedBy(func(req *resource.ResourceSearchRequest) bool {
|
||||
if len(req.Options.Fields) == 0 {
|
||||
return false
|
||||
}
|
||||
// make sure the search request includes the shared folders
|
||||
for _, field := range req.Options.Fields {
|
||||
if field.Key == resource.SEARCH_FIELD_NAME {
|
||||
return slices.Equal(field.Values, []string{"shared-uid1", "shared-uid2"})
|
||||
}
|
||||
}
|
||||
return false
|
||||
})).Return(&resource.ResourceSearchResponse{
|
||||
Results: &resource.ResourceTable{
|
||||
Columns: []*resource.ResourceTableColumnDefinition{
|
||||
{
|
||||
Name: "title",
|
||||
Type: resource.ResourceTableColumnDefinition_STRING,
|
||||
},
|
||||
{
|
||||
Name: "folder",
|
||||
Type: resource.ResourceTableColumnDefinition_STRING,
|
||||
},
|
||||
{
|
||||
Name: "tags",
|
||||
Type: resource.ResourceTableColumnDefinition_STRING,
|
||||
},
|
||||
},
|
||||
Rows: []*resource.ResourceTableRow{
|
||||
{
|
||||
Key: &resource.ResourceKey{
|
||||
Name: "shared-uid1",
|
||||
Resource: "dashboard",
|
||||
},
|
||||
Cells: [][]byte{
|
||||
[]byte("Shared Dashboard 1"),
|
||||
[]byte("f1"),
|
||||
[]byte("[\"shared\"]"),
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
TotalHits: 1,
|
||||
}, nil).Once()
|
||||
k8sCliMock.On("Search", mock.Anything, int64(1), mock.MatchedBy(func(req *resource.ResourceSearchRequest) bool {
|
||||
if len(req.Options.Fields) == 0 {
|
||||
return false
|
||||
}
|
||||
for _, field := range req.Options.Fields {
|
||||
if field.Key == resource.SEARCH_FIELD_NAME {
|
||||
return slices.Equal(field.Values, []string{"shared-uid1"})
|
||||
}
|
||||
}
|
||||
return false
|
||||
})).Return(&resource.ResourceSearchResponse{
|
||||
Results: &resource.ResourceTable{
|
||||
Columns: []*resource.ResourceTableColumnDefinition{
|
||||
{
|
||||
Name: "title",
|
||||
Type: resource.ResourceTableColumnDefinition_STRING,
|
||||
},
|
||||
{
|
||||
Name: "folder",
|
||||
Type: resource.ResourceTableColumnDefinition_STRING,
|
||||
},
|
||||
{
|
||||
Name: "tags",
|
||||
Type: resource.ResourceTableColumnDefinition_STRING,
|
||||
},
|
||||
},
|
||||
Rows: []*resource.ResourceTableRow{
|
||||
{
|
||||
Key: &resource.ResourceKey{
|
||||
Name: "shared-uid1",
|
||||
Resource: "dashboard",
|
||||
},
|
||||
Cells: [][]byte{
|
||||
[]byte("Shared Dashboard 1"),
|
||||
[]byte("f1"),
|
||||
[]byte("[\"shared\"]"),
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
TotalHits: 1,
|
||||
}, nil)
|
||||
fakeFolders.ExpectedFolders = []*folder.Folder{}
|
||||
query := dashboards.FindPersistedDashboardsQuery{
|
||||
FolderUIDs: []string{folder.SharedWithMeFolderUID},
|
||||
SignedInUser: &user.SignedInUser{OrgID: 1, Permissions: map[int64]map[string][]string{1: {dashboards.ActionDashboardsRead: {dashboards.ScopeDashboardsPrefix + "shared-uid1", dashboards.ScopeDashboardsPrefix + "shared-uid2"}}}},
|
||||
}
|
||||
|
||||
result, err := service.SearchDashboards(ctx, &query)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, result, 1)
|
||||
require.Equal(t, "shared-uid1", result[0].UID)
|
||||
require.Equal(t, "Shared Dashboard 1", result[0].Title)
|
||||
k8sCliMock.AssertExpectations(t)
|
||||
})
|
||||
}
|
||||
|
||||
func TestGetDashboards(t *testing.T) {
|
||||
@@ -1815,8 +1921,9 @@ func TestGetDashboardUIDByID(t *testing.T) {
|
||||
}
|
||||
|
||||
expectedResult := &dashboards.DashboardRef{
|
||||
UID: "uid1",
|
||||
Slug: "dashboard-1",
|
||||
UID: "uid1",
|
||||
Slug: "dashboard-1",
|
||||
FolderUID: "folder1",
|
||||
}
|
||||
query := &dashboards.GetDashboardRefByIDQuery{
|
||||
ID: 1,
|
||||
@@ -1977,6 +2084,119 @@ func TestGetDashboardTags(t *testing.T) {
|
||||
})
|
||||
}
|
||||
|
||||
func TestGetDashboardsSharedWithUser(t *testing.T) {
|
||||
fakeStore := dashboards.FakeDashboardStore{}
|
||||
defer fakeStore.AssertExpectations(t)
|
||||
service := &DashboardServiceImpl{
|
||||
cfg: setting.NewCfg(),
|
||||
dashboardStore: &fakeStore,
|
||||
folderService: &foldertest.FakeService{},
|
||||
}
|
||||
|
||||
user := &user.SignedInUser{
|
||||
OrgID: 1,
|
||||
Permissions: map[int64]map[string][]string{
|
||||
1: {
|
||||
dashboards.ActionDashboardsRead: {
|
||||
dashboards.ScopeDashboardsPrefix + "dashboard1",
|
||||
dashboards.ScopeDashboardsPrefix + "dashboard2",
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
expectedDashboards := []*dashboards.Dashboard{
|
||||
{
|
||||
UID: "dashboard1",
|
||||
Slug: "dashboard-1",
|
||||
FolderUID: "folder1",
|
||||
},
|
||||
{
|
||||
UID: "dashboard2",
|
||||
Slug: "dashboard-2",
|
||||
FolderUID: "folder2",
|
||||
},
|
||||
}
|
||||
|
||||
expectedFolderRefs := []*dashboards.DashboardRef{
|
||||
{
|
||||
UID: "dashboard1",
|
||||
Slug: "dashboard-1",
|
||||
FolderUID: "folder1",
|
||||
},
|
||||
{
|
||||
UID: "dashboard2",
|
||||
Slug: "dashboard-2",
|
||||
FolderUID: "folder2",
|
||||
},
|
||||
}
|
||||
|
||||
t.Run("Should fallback to dashboard store if Kubernetes feature flags are not enabled", func(t *testing.T) {
|
||||
service.features = featuremgmt.WithFeatures()
|
||||
fakeStore.On("GetDashboards", mock.Anything, &dashboards.GetDashboardsQuery{
|
||||
DashboardUIDs: []string{"dashboard1", "dashboard2"},
|
||||
OrgID: 1,
|
||||
}).Return(expectedDashboards, nil).Once()
|
||||
|
||||
result, err := service.GetDashboardsSharedWithUser(context.Background(), user)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, expectedFolderRefs, result)
|
||||
fakeStore.AssertExpectations(t)
|
||||
})
|
||||
|
||||
t.Run("Should use Kubernetes client if feature flags are enabled", func(t *testing.T) {
|
||||
ctx, k8sCliMock := setupK8sDashboardTests(service)
|
||||
service.features = featuremgmt.WithFeatures(featuremgmt.FlagKubernetesClientDashboardsFolders)
|
||||
|
||||
k8sCliMock.On("GetNamespace", mock.Anything, mock.Anything).Return("default")
|
||||
k8sCliMock.On("Search", mock.Anything, int64(1), mock.MatchedBy(func(req *resource.ResourceSearchRequest) bool {
|
||||
return req.Options.Fields[0].Key == "name" &&
|
||||
slices.Equal(req.Options.Fields[0].Values, []string{"dashboard1", "dashboard2"})
|
||||
})).Return(&resource.ResourceSearchResponse{
|
||||
Results: &resource.ResourceTable{
|
||||
Columns: []*resource.ResourceTableColumnDefinition{
|
||||
{
|
||||
Name: "title",
|
||||
Type: resource.ResourceTableColumnDefinition_STRING,
|
||||
},
|
||||
{
|
||||
Name: "folder",
|
||||
Type: resource.ResourceTableColumnDefinition_STRING,
|
||||
},
|
||||
},
|
||||
Rows: []*resource.ResourceTableRow{
|
||||
{
|
||||
Key: &resource.ResourceKey{
|
||||
Name: "dashboard1",
|
||||
Resource: "dashboard",
|
||||
},
|
||||
Cells: [][]byte{
|
||||
[]byte("Dashboard 1"),
|
||||
[]byte("folder1"),
|
||||
},
|
||||
},
|
||||
{
|
||||
Key: &resource.ResourceKey{
|
||||
Name: "dashboard2",
|
||||
Resource: "dashboard",
|
||||
},
|
||||
Cells: [][]byte{
|
||||
[]byte("Dashboard 2"),
|
||||
[]byte("folder2"),
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
TotalHits: 2,
|
||||
}, nil)
|
||||
|
||||
result, err := service.GetDashboardsSharedWithUser(ctx, user)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, expectedFolderRefs, result)
|
||||
k8sCliMock.AssertExpectations(t)
|
||||
})
|
||||
}
|
||||
|
||||
func TestQuotaCount(t *testing.T) {
|
||||
fakeStore := dashboards.FakeDashboardStore{}
|
||||
defer fakeStore.AssertExpectations(t)
|
||||
|
||||
Reference in New Issue
Block a user