IntenseWebs/grafana
3
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2025-02-25 18:55:37 -06:00
Code Issues Packages Projects Releases Wiki Activity

PublicDashboards: do not return errors when resource not found from store layer (#57838)

Browse Source
This commit is contained in:
Ezequiel Victorero 2022-10-31 11:22:27 -03:00 committed by GitHub
parent f0ab4bea8c
commit 46093c1267
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 147 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
pkg/services/publicdashboards/api/api.go
View File

@ -92,6 +92,12 @@ func (api *Api) ListPublicDashboards(c *models.ReqContext) response.Response {
// GetPublicDashboard Gets public dashboard configuration for dashboard
// GET /api/dashboards/uid/:uid/public-config
func (api *Api) GetPublicDashboard(c *models.ReqContext) response.Response {
// exit if we don't have a valid dashboardUid
dashboardUid := web.Params(c.Req)[":dashboardUid"]
if dashboardUid == "" || !util.IsValidShortUID(dashboardUid) {
api.handleError(c.Req.Context(), http.StatusBadRequest, "GetPublicDashboard: no valid dashboardUid", dashboards.ErrDashboardIdentifierNotSet)
}
pdc, err := api.PublicDashboardService.FindByDashboardUid(c.Req.Context(), c.OrgID, web.Params(c.Req)[":dashboardUid"])
if err != nil {
return api.handleError(c.Req.Context(), http.StatusInternalServerError, "GetPublicDashboardConfig: failed to get public dashboard config", err)
@ -105,12 +111,12 @@ func (api *Api) SavePublicDashboard(c *models.ReqContext) response.Response {
// exit if we don't have a valid dashboardUid
dashboardUid := web.Params(c.Req)[":dashboardUid"]
if dashboardUid == "" || !util.IsValidShortUID(dashboardUid) {
api.handleError(c.Req.Context(), http.StatusBadRequest, "SavePublicDashboardConfig: no dashboardUid", dashboards.ErrDashboardIdentifierNotSet)
api.handleError(c.Req.Context(), http.StatusBadRequest, "SavePublicDashboard: invalid dashboardUid", dashboards.ErrDashboardIdentifierNotSet)
}
pubdash := &PublicDashboard{}
if err := web.Bind(c.Req, pubdash); err != nil {
return response.Error(http.StatusBadRequest, "SavePublicDashboardConfig: bad request data", err)
return response.Error(http.StatusBadRequest, "SavePublicDashboard: bad request data", err)
}
// Always set the orgID and userID from the session

42
pkg/services/publicdashboards/database/database.go
View File

@ -55,19 +55,21 @@ func (d *PublicDashboardStoreImpl) FindAll(ctx context.Context, orgId int64) ([]
return resp, nil
}
func (d *PublicDashboardStoreImpl) FindDashboard(ctx context.Context, dashboardUid string, orgId int64) (*models.Dashboard, error) {
dashboard := &models.Dashboard{Uid: dashboardUid, OrgId: orgId}
// FindDashboard returns a dashboard by orgId and dashboardUid
func (d *PublicDashboardStoreImpl) FindDashboard(ctx context.Context, orgId int64, dashboardUid string) (*models.Dashboard, error) {
dashboard := &models.Dashboard{OrgId: orgId, Uid: dashboardUid}
var found bool
err := d.sqlStore.WithTransactionalDbSession(ctx, func(sess *db.Session) error {
has, err := sess.Get(dashboard)
if err != nil {
return err
}
if !has {
return ErrPublicDashboardNotFound
}
return nil
var err error
found, err = sess.Get(dashboard)
return err
})
if !found {
return nil, nil
}
return dashboard, err
}
@ -99,7 +101,7 @@ func (d *PublicDashboardStoreImpl) Find(ctx context.Context, uid string) (*Publi
// FindByAccessToken Returns public dashboard by access token or nil if not found
func (d *PublicDashboardStoreImpl) FindByAccessToken(ctx context.Context, accessToken string) (*PublicDashboard, error) {
if accessToken == "" {
return nil, ErrPublicDashboardIdentifierNotSet
return nil, nil
}
var found bool
@ -121,22 +123,20 @@ func (d *PublicDashboardStoreImpl) FindByAccessToken(ctx context.Context, access
return pdRes, err
}
// FindByDashboardUid Retrieves public dashboard by dashboard uid
// FindByDashboardUid Retrieves public dashboard by dashboard uid or nil if not found
func (d *PublicDashboardStoreImpl) FindByDashboardUid(ctx context.Context, orgId int64, dashboardUid string) (*PublicDashboard, error) {
if dashboardUid == "" {
return nil, dashboards.ErrDashboardIdentifierNotSet
if dashboardUid == "" || orgId == 0 {
return nil, nil
}
var found bool
pdRes := &PublicDashboard{OrgId: orgId, DashboardUid: dashboardUid}
err := d.sqlStore.WithTransactionalDbSession(ctx, func(sess *db.Session) error {
// publicDashboard
exists, err := sess.Get(pdRes)
var err error
found, err = sess.Get(pdRes)
if err != nil {
return err
}
if !exists {
return ErrPublicDashboardNotFound
}
return nil
})
@ -145,6 +145,10 @@ func (d *PublicDashboardStoreImpl) FindByDashboardUid(ctx context.Context, orgId
return nil, err
}
if !found {
return nil, nil
}
return pdRes, err
}

84
pkg/services/publicdashboards/database/database_test.go
View File

@ -81,7 +81,7 @@ func TestIntegrationFindDashboard(t *testing.T) {
t.Run("FindDashboard can get original dashboard by uid", func(t *testing.T) {
setup()
dashboard, err := publicdashboardStore.FindDashboard(context.Background(), savedDashboard.Uid, savedDashboard.OrgId)
dashboard, err := publicdashboardStore.FindDashboard(context.Background(), savedDashboard.OrgId, savedDashboard.Uid)
require.NoError(t, err)
require.Equal(t, savedDashboard.Uid, dashboard.Uid)
@ -228,7 +228,7 @@ func TestIntegrationFindByDashboardUid(t *testing.T) {
savedDashboard = insertTestDashboard(t, dashboardStore, "testDashie", 1, 0, true)
}
t.Run("returns isPublic and set dashboardUid and orgId", func(t *testing.T) {
t.Run("returns public dashboard by dashboardUid", func(t *testing.T) {
setup()
savedPubdash := insertPublicDashboard(t, publicdashboardStore, savedDashboard.Uid, savedDashboard.OrgId, false)
pubdash, err := publicdashboardStore.FindByDashboardUid(context.Background(), savedDashboard.OrgId, savedDashboard.Uid)
@ -236,10 +236,11 @@ func TestIntegrationFindByDashboardUid(t *testing.T) {
assert.Equal(t, savedPubdash, pubdash)
})
t.Run("returns dashboard errDashboardIdentifierNotSet", func(t *testing.T) {
t.Run("returns nil when identifier is not set", func(t *testing.T) {
setup()
_, err := publicdashboardStore.FindByDashboardUid(context.Background(), savedDashboard.OrgId, "")
require.Error(t, dashboards.ErrDashboardIdentifierNotSet, err)
pubdash, err := publicdashboardStore.FindByDashboardUid(context.Background(), savedDashboard.OrgId, "")
assert.Nil(t, err)
assert.Nil(t, pubdash)
})
t.Run("returns along with public dashboard when exists", func(t *testing.T) {
@ -267,12 +268,74 @@ func TestIntegrationFindByDashboardUid(t *testing.T) {
assert.True(t, assert.ObjectsAreEqualValues(&cmd.PublicDashboard, pubdash))
})
t.Run("returns error when public dashboard doesn't exist", func(t *testing.T) {
t.Run("returns nil when public dashboard doesn't exist", func(t *testing.T) {
setup()
pubdash, err := publicdashboardStore.FindByDashboardUid(context.Background(), 9, "fake-dashboard-uid")
require.Error(t, err)
require.Nil(t, pubdash)
assert.Equal(t, ErrPublicDashboardNotFound, err)
require.NoError(t, err)
assert.Nil(t, pubdash)
})
}
func TestIntegrationFindByAccessToken(t *testing.T) {
var sqlStore db.DB
var cfg *setting.Cfg
var dashboardStore *dashboardsDB.DashboardStore
var publicdashboardStore *PublicDashboardStoreImpl
var savedDashboard *models.Dashboard
setup := func() {
sqlStore, cfg = db.InitTestDBwithCfg(t)
dashboardStore = dashboardsDB.ProvideDashboardStore(sqlStore, cfg, featuremgmt.WithFeatures(), tagimpl.ProvideService(sqlStore, cfg))
publicdashboardStore = ProvideStore(sqlStore)
savedDashboard = insertTestDashboard(t, dashboardStore, "testDashie", 1, 0, true)
}
t.Run("returns public dashboard by accessToken", func(t *testing.T) {
setup()
savedPubdash := insertPublicDashboard(t, publicdashboardStore, savedDashboard.Uid, savedDashboard.OrgId, false)
pubdash, err := publicdashboardStore.FindByAccessToken(context.Background(), savedPubdash.AccessToken)
require.NoError(t, err)
assert.Equal(t, savedPubdash, pubdash)
})
t.Run("returns nil when identifier is not set", func(t *testing.T) {
setup()
pubdash, err := publicdashboardStore.FindByAccessToken(context.Background(), "")
assert.Nil(t, err)
assert.Nil(t, pubdash)
})
t.Run("returns along with public dashboard when exists", func(t *testing.T) {
setup()
cmd := SavePublicDashboardCommand{
PublicDashboard: PublicDashboard{
IsEnabled: true,
Uid: "pubdash-uid",
DashboardUid: savedDashboard.Uid,
OrgId: savedDashboard.OrgId,
TimeSettings: DefaultTimeSettings,
CreatedAt: DefaultTime,
CreatedBy: 7,
AccessToken: "thisisavalidaccesstoken",
},
}
// insert test public dashboard
err := publicdashboardStore.Save(context.Background(), cmd)
require.NoError(t, err)
// retrieve from db
pubdash, err := publicdashboardStore.FindByAccessToken(context.Background(), cmd.PublicDashboard.AccessToken)
require.NoError(t, err)
assert.True(t, assert.ObjectsAreEqualValues(&cmd.PublicDashboard, pubdash))
})
t.Run("returns error when public dashboard doesn't exist", func(t *testing.T) {
setup()
pubdash, err := publicdashboardStore.FindByAccessToken(context.Background(), "fake-accessToken-uid")
require.NoError(t, err)
assert.Nil(t, pubdash)
})
}
@ -336,7 +399,8 @@ func TestIntegrationSavePublicDashboard(t *testing.T) {
AccessToken: "NOTAREALUUID",
},
})
assert.Error(t, err, dashboards.ErrDashboardIdentifierNotSet)
require.Error(t, err)
assert.Equal(t, err, dashboards.ErrDashboardIdentifierNotSet)
})
}

14
pkg/services/publicdashboards/public_dashboard_service_mock.go
View File

@ -136,13 +136,13 @@ func (_m *FakePublicDashboardService) FindByDashboardUid(ctx context.Context, or
return r0, r1
}
// FindDashboard provides a mock function with given fields: ctx, dashboardUid, orgId
func (_m *FakePublicDashboardService) FindDashboard(ctx context.Context, dashboardUid string, orgId int64) (*pkgmodels.Dashboard, error) {
ret := _m.Called(ctx, dashboardUid, orgId)
// FindDashboard provides a mock function with given fields: ctx, orgId, dashboardUid
func (_m *FakePublicDashboardService) FindDashboard(ctx context.Context, orgId int64, dashboardUid string) (*pkgmodels.Dashboard, error) {
ret := _m.Called(ctx, orgId, dashboardUid)
var r0 *pkgmodels.Dashboard
if rf, ok := ret.Get(0).(func(context.Context, string, int64) *pkgmodels.Dashboard); ok {
r0 = rf(ctx, dashboardUid, orgId)
if rf, ok := ret.Get(0).(func(context.Context, int64, string) *pkgmodels.Dashboard); ok {
r0 = rf(ctx, orgId, dashboardUid)
} else {
if ret.Get(0) != nil {
r0 = ret.Get(0).(*pkgmodels.Dashboard)
@ -150,8 +150,8 @@ func (_m *FakePublicDashboardService) FindDashboard(ctx context.Context, dashboa
}
var r1 error
if rf, ok := ret.Get(1).(func(context.Context, string, int64) error); ok {
r1 = rf(ctx, dashboardUid, orgId)
if rf, ok := ret.Get(1).(func(context.Context, int64, string) error); ok {
r1 = rf(ctx, orgId, dashboardUid)
} else {
r1 = ret.Error(1)
}

14
pkg/services/publicdashboards/public_dashboard_store_mock.go
View File

@ -152,13 +152,13 @@ func (_m *FakePublicDashboardStore) FindByDashboardUid(ctx context.Context, orgI
return r0, r1
}
// FindDashboard provides a mock function with given fields: ctx, dashboardUid, orgId
func (_m *FakePublicDashboardStore) FindDashboard(ctx context.Context, dashboardUid string, orgId int64) (*pkgmodels.Dashboard, error) {
ret := _m.Called(ctx, dashboardUid, orgId)
// FindDashboard provides a mock function with given fields: ctx, orgId, dashboardUid
func (_m *FakePublicDashboardStore) FindDashboard(ctx context.Context, orgId int64, dashboardUid string) (*pkgmodels.Dashboard, error) {
ret := _m.Called(ctx, orgId, dashboardUid)
var r0 *pkgmodels.Dashboard
if rf, ok := ret.Get(0).(func(context.Context, string, int64) *pkgmodels.Dashboard); ok {
r0 = rf(ctx, dashboardUid, orgId)
if rf, ok := ret.Get(0).(func(context.Context, int64, string) *pkgmodels.Dashboard); ok {
r0 = rf(ctx, orgId, dashboardUid)
} else {
if ret.Get(0) != nil {
r0 = ret.Get(0).(*pkgmodels.Dashboard)
@ -166,8 +166,8 @@ func (_m *FakePublicDashboardStore) FindDashboard(ctx context.Context, dashboard
}
var r1 error
if rf, ok := ret.Get(1).(func(context.Context, string, int64) error); ok {
r1 = rf(ctx, dashboardUid, orgId)
if rf, ok := ret.Get(1).(func(context.Context, int64, string) error); ok {
r1 = rf(ctx, orgId, dashboardUid)
} else {
r1 = ret.Error(1)
}

4
pkg/services/publicdashboards/publicdashboard.go
View File

@ -17,7 +17,7 @@ type Service interface {
FindPublicDashboardAndDashboardByAccessToken(ctx context.Context, accessToken string) (*PublicDashboard, *models.Dashboard, error)
FindByDashboardUid(ctx context.Context, orgId int64, dashboardUid string) (*PublicDashboard, error)
FindAnnotations(ctx context.Context, reqDTO AnnotationsQueryDTO, accessToken string) ([]AnnotationEvent, error)
FindDashboard(ctx context.Context, dashboardUid string, orgId int64) (*models.Dashboard, error)
FindDashboard(ctx context.Context, orgId int64, dashboardUid string) (*models.Dashboard, error)
FindAll(ctx context.Context, u *user.SignedInUser, orgId int64) ([]PublicDashboardListResponse, error)
Save(ctx context.Context, u *user.SignedInUser, dto *SavePublicDashboardDTO) (*PublicDashboard, error)
@ -36,7 +36,7 @@ type Store interface {
Find(ctx context.Context, uid string) (*PublicDashboard, error)
FindByAccessToken(ctx context.Context, accessToken string) (*PublicDashboard, error)
FindByDashboardUid(ctx context.Context, orgId int64, dashboardUid string) (*PublicDashboard, error)
FindDashboard(ctx context.Context, dashboardUid string, orgId int64) (*models.Dashboard, error)
FindDashboard(ctx context.Context, orgId int64, dashboardUid string) (*models.Dashboard, error)
FindAll(ctx context.Context, orgId int64) ([]PublicDashboardListResponse, error)
Save(ctx context.Context, cmd SavePublicDashboardCommand) error
Update(ctx context.Context, cmd SavePublicDashboardCommand) error

14
pkg/services/publicdashboards/service/query_test.go
View File

@ -423,7 +423,7 @@ func TestGetAnnotations(t *testing.T) {
}
fakeStore.On("FindByAccessToken", mock.Anything, mock.AnythingOfType("string")).
Return(&PublicDashboard{Uid: "uid1", IsEnabled: true}, nil)
fakeStore.On("FindDashboard", mock.Anything, mock.AnythingOfType("string"), mock.Anything).
fakeStore.On("FindDashboard", mock.Anything, mock.Anything, mock.AnythingOfType("string")).
Return(grafanamodels.NewDashboard("dash1"), nil)
reqDTO := AnnotationsQueryDTO{
@ -479,7 +479,7 @@ func TestGetAnnotations(t *testing.T) {
pubdash := &PublicDashboard{Uid: "uid1", IsEnabled: true, OrgId: 1, DashboardUid: dashboard.Uid, AnnotationsEnabled: true}
fakeStore.On("FindByAccessToken", mock.Anything, mock.AnythingOfType("string")).Return(pubdash, nil)
fakeStore.On("FindDashboard", mock.Anything, mock.AnythingOfType("string"), mock.Anything).Return(dashboard, nil)
fakeStore.On("FindDashboard", mock.Anything, mock.Anything, mock.AnythingOfType("string")).Return(dashboard, nil)
annotationsRepo.On("Find", mock.Anything, mock.Anything).Return([]*annotations.ItemDTO{
{
@ -539,7 +539,7 @@ func TestGetAnnotations(t *testing.T) {
pubdash := &PublicDashboard{Uid: "uid1", IsEnabled: true, OrgId: 1, DashboardUid: dashboard.Uid, AnnotationsEnabled: true}
fakeStore.On("FindByAccessToken", mock.Anything, mock.AnythingOfType("string")).Return(pubdash, nil)
fakeStore.On("FindDashboard", mock.Anything, mock.AnythingOfType("string"), mock.Anything).Return(dashboard, nil)
fakeStore.On("FindDashboard", mock.Anything, mock.Anything, mock.AnythingOfType("string")).Return(dashboard, nil)
annotationsRepo.On("Find", mock.Anything, mock.Anything).Return([]*annotations.ItemDTO{
{
@ -611,7 +611,7 @@ func TestGetAnnotations(t *testing.T) {
pubdash := &PublicDashboard{Uid: "uid1", IsEnabled: true, OrgId: 1, DashboardUid: dashboard.Uid, AnnotationsEnabled: true}
fakeStore.On("FindByAccessToken", mock.Anything, mock.AnythingOfType("string")).Return(pubdash, nil)
fakeStore.On("FindDashboard", mock.Anything, mock.AnythingOfType("string"), mock.Anything).Return(dashboard, nil)
fakeStore.On("FindDashboard", mock.Anything, mock.Anything, mock.AnythingOfType("string")).Return(dashboard, nil)
annotationsRepo.On("Find", mock.Anything, mock.Anything).Return([]*annotations.ItemDTO{
{
@ -656,7 +656,7 @@ func TestGetAnnotations(t *testing.T) {
pubdash := &PublicDashboard{Uid: "uid1", IsEnabled: true, OrgId: 1, DashboardUid: dashboard.Uid, AnnotationsEnabled: true}
fakeStore.On("FindByAccessToken", mock.Anything, mock.AnythingOfType("string")).Return(pubdash, nil)
fakeStore.On("FindDashboard", mock.Anything, mock.AnythingOfType("string"), mock.Anything).Return(dashboard, nil)
fakeStore.On("FindDashboard", mock.Anything, mock.Anything, mock.AnythingOfType("string")).Return(dashboard, nil)
items, err := service.FindAnnotations(context.Background(), AnnotationsQueryDTO{}, "abc123")
@ -691,7 +691,7 @@ func TestGetAnnotations(t *testing.T) {
pubdash := &PublicDashboard{Uid: "uid1", IsEnabled: true, OrgId: 1, DashboardUid: dashboard.Uid, AnnotationsEnabled: false}
fakeStore.On("FindByAccessToken", mock.Anything, mock.AnythingOfType("string")).Return(pubdash, nil)
fakeStore.On("FindDashboard", mock.Anything, mock.AnythingOfType("string"), mock.Anything).Return(dashboard, nil)
fakeStore.On("FindDashboard", mock.Anything, mock.Anything, mock.AnythingOfType("string")).Return(dashboard, nil)
items, err := service.FindAnnotations(context.Background(), AnnotationsQueryDTO{}, "abc123")
@ -725,7 +725,7 @@ func TestGetAnnotations(t *testing.T) {
pubdash := &PublicDashboard{Uid: "uid1", IsEnabled: true, OrgId: 1, DashboardUid: dash.Uid, AnnotationsEnabled: true}
fakeStore.On("FindByAccessToken", mock.Anything, mock.AnythingOfType("string")).Return(pubdash, nil)
fakeStore.On("FindDashboard", mock.Anything, mock.AnythingOfType("string"), mock.Anything).Return(dash, nil)
fakeStore.On("FindDashboard", mock.Anything, mock.Anything, mock.AnythingOfType("string")).Return(dash, nil)
annotationsRepo.On("Find", mock.Anything, mock.Anything).Return(nil, errors.New("failed")).Maybe()

26
pkg/services/publicdashboards/service/service.go
View File

@ -62,13 +62,17 @@ func ProvideService(
}
// FindDashboard Gets a dashboard by Uid
func (pd *PublicDashboardServiceImpl) FindDashboard(ctx context.Context, dashboardUid string, orgId int64) (*models.Dashboard, error) {
dashboard, err := pd.store.FindDashboard(ctx, dashboardUid, orgId)
func (pd *PublicDashboardServiceImpl) FindDashboard(ctx context.Context, orgId int64, dashboardUid string) (*models.Dashboard, error) {
dash, err := pd.store.FindDashboard(ctx, orgId, dashboardUid)
if err != nil {
return nil, err
}
return dashboard, nil
if dash == nil {
return nil, dashboards.ErrDashboardNotFound
}
return dash, nil
}
// FindPublicDashboardAndDashboardByAccessToken Gets public dashboard via access token
@ -90,7 +94,7 @@ func (pd *PublicDashboardServiceImpl) FindPublicDashboardAndDashboardByAccessTok
return nil, nil, ErrPublicDashboardNotFound
}
dash, err := pd.store.FindDashboard(ctx, pubdash.DashboardUid, pubdash.OrgId)
dash, err := pd.store.FindDashboard(ctx, pubdash.OrgId, pubdash.DashboardUid)
if err != nil {
return nil, nil, err
}
@ -105,23 +109,31 @@ func (pd *PublicDashboardServiceImpl) FindPublicDashboardAndDashboardByAccessTok
// FindByDashboardUid is a helper method to retrieve the public dashboard configuration for a given dashboard from the database
func (pd *PublicDashboardServiceImpl) FindByDashboardUid(ctx context.Context, orgId int64, dashboardUid string) (*PublicDashboard, error) {
pdc, err := pd.store.FindByDashboardUid(ctx, orgId, dashboardUid)
pubdash, err := pd.store.FindByDashboardUid(ctx, orgId, dashboardUid)
if err != nil {
return nil, err
}
return pdc, nil
if pubdash == nil {
return nil, ErrPublicDashboardNotFound
}
return pubdash, nil
}
// Save is a helper method to persist the sharing config
// to the database. It handles validations for sharing config and persistence
func (pd *PublicDashboardServiceImpl) Save(ctx context.Context, u *user.SignedInUser, dto *SavePublicDashboardDTO) (*PublicDashboard, error) {
// validate if the dashboard exists
dashboard, err := pd.FindDashboard(ctx, dto.DashboardUid, u.OrgID)
dashboard, err := pd.FindDashboard(ctx, u.OrgID, dto.DashboardUid)
if err != nil {
return nil, err
}
if dashboard == nil {
return nil, dashboards.ErrDashboardNotFound
}
// set default value for time settings
if dto.PublicDashboard.TimeSettings == nil {
dto.PublicDashboard.TimeSettings = &TimeSettings{}