diff --git a/pkg/services/dashboards/database/database_test.go b/pkg/services/dashboards/database/database_test.go index 5eb66257525..da338a3c565 100644 --- a/pkg/services/dashboards/database/database_test.go +++ b/pkg/services/dashboards/database/database_test.go @@ -259,7 +259,7 @@ func TestIntegrationDashboardDataAccess(t *testing.T) { } err := publicDashboardStore.Save(context.Background(), cmd) require.NoError(t, err) - pubdashConfig, _, _ := publicDashboardStore.FindPublicDashboardAndDashboardByAccessToken(context.Background(), "an-access-token") + pubdashConfig, _ := publicDashboardStore.FindByAccessToken(context.Background(), "an-access-token") require.NotNil(t, pubdashConfig) deleteCmd := &models.DeleteDashboardCommand{Id: savedDash.Id, OrgId: savedDash.OrgId} @@ -271,8 +271,8 @@ func TestIntegrationDashboardDataAccess(t *testing.T) { require.Equal(t, getErr, dashboards.ErrDashboardNotFound) assert.Nil(t, dash) - pubdashConfig, _, err = publicDashboardStore.FindPublicDashboardAndDashboardByAccessToken(context.Background(), "an-access-token") - require.Equal(t, err, publicDashboardModels.ErrPublicDashboardNotFound) + pubdashConfig, err = publicDashboardStore.FindByAccessToken(context.Background(), "an-access-token") + require.Nil(t, err) require.Nil(t, pubdashConfig) }) @@ -294,7 +294,7 @@ func TestIntegrationDashboardDataAccess(t *testing.T) { } err := publicDashboardStore.Save(context.Background(), cmd) require.NoError(t, err) - pubdashConfig, _, _ := publicDashboardStore.FindPublicDashboardAndDashboardByAccessToken(context.Background(), "an-access-token") + pubdashConfig, _ := publicDashboardStore.FindByAccessToken(context.Background(), "an-access-token") require.NotNil(t, pubdashConfig) deleteCmd := &models.DeleteDashboardCommand{Id: savedFolder.Id, ForceDeleteFolderRules: true} @@ -308,8 +308,8 @@ func TestIntegrationDashboardDataAccess(t *testing.T) { require.NoError(t, err) require.Equal(t, len(query.Result), 0) - pubdashConfig, _, err = publicDashboardStore.FindPublicDashboardAndDashboardByAccessToken(context.Background(), "an-access-token") - require.Equal(t, err, publicDashboardModels.ErrPublicDashboardNotFound) + pubdashConfig, err = publicDashboardStore.FindByAccessToken(context.Background(), "an-access-token") + require.Nil(t, err) require.Nil(t, pubdashConfig) }) diff --git a/pkg/services/publicdashboards/database/database.go b/pkg/services/publicdashboards/database/database.go index a76bca44ccb..e1cda805fbe 100644 --- a/pkg/services/publicdashboards/database/database.go +++ b/pkg/services/publicdashboards/database/database.go @@ -71,39 +71,6 @@ func (d *PublicDashboardStoreImpl) FindDashboard(ctx context.Context, dashboardU return dashboard, err } -// FindPublicDashboardAndDashboardByAccessToken Retrieves public dashboard. This method makes 2 queries to the db so that in the -// even that the underlying dashboard is missing it will return a 404. -func (d *PublicDashboardStoreImpl) FindPublicDashboardAndDashboardByAccessToken(ctx context.Context, accessToken string) (*PublicDashboard, *models.Dashboard, error) { - if accessToken == "" { - return nil, nil, ErrPublicDashboardIdentifierNotSet - } - - // get public dashboard - pdRes := &PublicDashboard{AccessToken: accessToken} - err := d.sqlStore.WithTransactionalDbSession(ctx, func(sess *db.Session) error { - has, err := sess.Get(pdRes) - if err != nil { - return err - } - if !has { - return ErrPublicDashboardNotFound - } - return nil - }) - - if err != nil { - return nil, nil, err - } - - // find dashboard - dashRes, err := d.FindDashboard(ctx, pdRes.DashboardUid) - if err != nil { - return nil, nil, err - } - - return pdRes, dashRes, err -} - // Find Returns public dashboard configuration by Uid or nil if not found func (d *PublicDashboardStoreImpl) Find(ctx context.Context, uid string) (*PublicDashboard, error) { if uid == "" { @@ -129,7 +96,7 @@ func (d *PublicDashboardStoreImpl) Find(ctx context.Context, uid string) (*Publi return pdRes, err } -// GetPublicDashboardByAccessToken Returns public dashboard by access token or nil if not found +// 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 @@ -181,7 +148,7 @@ func (d *PublicDashboardStoreImpl) FindByDashboardUid(ctx context.Context, orgId return pdRes, err } -// Persists public dashboard configuration +// Save Persists public dashboard configuration func (d *PublicDashboardStoreImpl) Save(ctx context.Context, cmd SavePublicDashboardConfigCommand) error { if cmd.PublicDashboard.DashboardUid == "" { return dashboards.ErrDashboardIdentifierNotSet diff --git a/pkg/services/publicdashboards/database/database_test.go b/pkg/services/publicdashboards/database/database_test.go index df5a8462dae..3d3f801d654 100644 --- a/pkg/services/publicdashboards/database/database_test.go +++ b/pkg/services/publicdashboards/database/database_test.go @@ -214,76 +214,6 @@ func TestIntegrationExistsEnabledByDashboardUid(t *testing.T) { }) } -func TestIntegrationFindPublicDashboardAndDashboardByAccessToken(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 PublicDashboard and Dashboard", func(t *testing.T) { - setup() - cmd := SavePublicDashboardConfigCommand{ - PublicDashboard: PublicDashboard{ - IsEnabled: true, - AnnotationsEnabled: true, - Uid: "abc1234", - DashboardUid: savedDashboard.Uid, - OrgId: savedDashboard.OrgId, - TimeSettings: DefaultTimeSettings, - CreatedAt: DefaultTime, - CreatedBy: 7, - AccessToken: "NOTAREALUUID", - }, - } - - err := publicdashboardStore.Save(context.Background(), cmd) - require.NoError(t, err) - - pd, d, err := publicdashboardStore.FindPublicDashboardAndDashboardByAccessToken(context.Background(), "NOTAREALUUID") - require.NoError(t, err) - - assert.Equal(t, pd, &cmd.PublicDashboard) - assert.Equal(t, d.Uid, cmd.PublicDashboard.DashboardUid) - }) - - t.Run("returns ErrPublicDashboardNotFound with empty uid", func(t *testing.T) { - setup() - _, _, err := publicdashboardStore.FindPublicDashboardAndDashboardByAccessToken(context.Background(), "") - require.Error(t, ErrPublicDashboardIdentifierNotSet, err) - }) - - t.Run("returns ErrPublicDashboardNotFound when PublicDashboard not found", func(t *testing.T) { - setup() - _, _, err := publicdashboardStore.FindPublicDashboardAndDashboardByAccessToken(context.Background(), "zzzzzz") - require.Error(t, ErrPublicDashboardNotFound, err) - }) - - t.Run("returns ErrDashboardNotFound when Dashboard not found", func(t *testing.T) { - setup() - err := publicdashboardStore.Save(context.Background(), SavePublicDashboardConfigCommand{ - PublicDashboard: PublicDashboard{ - IsEnabled: true, - Uid: "abc1234", - DashboardUid: "nevergonnafindme", - OrgId: savedDashboard.OrgId, - CreatedAt: DefaultTime, - CreatedBy: 7, - }, - }) - require.NoError(t, err) - _, _, err = publicdashboardStore.FindPublicDashboardAndDashboardByAccessToken(context.Background(), "abc1234") - require.Error(t, dashboards.ErrDashboardNotFound, err) - }) -} - func TestIntegrationFindByDashboardUid(t *testing.T) { var sqlStore db.DB var cfg *setting.Cfg diff --git a/pkg/services/publicdashboards/publicdashboard.go b/pkg/services/publicdashboards/publicdashboard.go index c28d88df7aa..d4f8be58b62 100644 --- a/pkg/services/publicdashboards/publicdashboard.go +++ b/pkg/services/publicdashboards/publicdashboard.go @@ -37,7 +37,6 @@ type Store interface { FindByAccessToken(ctx context.Context, accessToken string) (*PublicDashboard, error) FindByDashboardUid(ctx context.Context, orgId int64, dashboardUid string) (*PublicDashboard, error) FindDashboard(ctx context.Context, dashboardUid string) (*models.Dashboard, error) - FindPublicDashboardAndDashboardByAccessToken(ctx context.Context, accessToken string) (*PublicDashboard, *models.Dashboard, error) FindAll(ctx context.Context, orgId int64) ([]PublicDashboardListResponse, error) Save(ctx context.Context, cmd SavePublicDashboardConfigCommand) error Update(ctx context.Context, cmd SavePublicDashboardConfigCommand) error diff --git a/pkg/services/publicdashboards/service/query_test.go b/pkg/services/publicdashboards/service/query_test.go index 9967f27cbd5..58147921e38 100644 --- a/pkg/services/publicdashboards/service/query_test.go +++ b/pkg/services/publicdashboards/service/query_test.go @@ -421,8 +421,10 @@ func TestGetAnnotations(t *testing.T) { store: &fakeStore, AnnotationsRepo: annotationsRepo, } - fakeStore.On("FindPublicDashboardAndDashboardByAccessToken", mock.Anything, mock.AnythingOfType("string")). - Return(&PublicDashboard{Uid: "uid1", IsEnabled: true}, grafanamodels.NewDashboard("dash1"), nil) + fakeStore.On("FindByAccessToken", mock.Anything, mock.AnythingOfType("string")). + Return(&PublicDashboard{Uid: "uid1", IsEnabled: true}, nil) + fakeStore.On("FindDashboard", mock.Anything, mock.AnythingOfType("string")). + Return(grafanamodels.NewDashboard("dash1"), nil) reqDTO := AnnotationsQueryDTO{ From: 1, @@ -475,7 +477,10 @@ func TestGetAnnotations(t *testing.T) { AnnotationsRepo: &annotationsRepo, } pubdash := &PublicDashboard{Uid: "uid1", IsEnabled: true, OrgId: 1, DashboardUid: dashboard.Uid, AnnotationsEnabled: true} - fakeStore.On("FindPublicDashboardAndDashboardByAccessToken", mock.Anything, mock.AnythingOfType("string")).Return(pubdash, dashboard, nil) + + fakeStore.On("FindByAccessToken", mock.Anything, mock.AnythingOfType("string")).Return(pubdash, nil) + fakeStore.On("FindDashboard", mock.Anything, mock.AnythingOfType("string")).Return(dashboard, nil) + annotationsRepo.On("Find", mock.Anything, mock.Anything).Return([]*annotations.ItemDTO{ { Id: 1, @@ -532,7 +537,10 @@ func TestGetAnnotations(t *testing.T) { AnnotationsRepo: &annotationsRepo, } pubdash := &PublicDashboard{Uid: "uid1", IsEnabled: true, OrgId: 1, DashboardUid: dashboard.Uid, AnnotationsEnabled: true} - fakeStore.On("FindPublicDashboardAndDashboardByAccessToken", mock.Anything, mock.AnythingOfType("string")).Return(pubdash, dashboard, nil) + + fakeStore.On("FindByAccessToken", mock.Anything, mock.AnythingOfType("string")).Return(pubdash, nil) + fakeStore.On("FindDashboard", mock.Anything, mock.AnythingOfType("string")).Return(dashboard, nil) + annotationsRepo.On("Find", mock.Anything, mock.Anything).Return([]*annotations.ItemDTO{ { Id: 1, @@ -601,7 +609,10 @@ func TestGetAnnotations(t *testing.T) { AnnotationsRepo: &annotationsRepo, } pubdash := &PublicDashboard{Uid: "uid1", IsEnabled: true, OrgId: 1, DashboardUid: dashboard.Uid, AnnotationsEnabled: true} - fakeStore.On("FindPublicDashboardAndDashboardByAccessToken", mock.Anything, mock.AnythingOfType("string")).Return(pubdash, dashboard, nil) + + fakeStore.On("FindByAccessToken", mock.Anything, mock.AnythingOfType("string")).Return(pubdash, nil) + fakeStore.On("FindDashboard", mock.Anything, mock.AnythingOfType("string")).Return(dashboard, nil) + annotationsRepo.On("Find", mock.Anything, mock.Anything).Return([]*annotations.ItemDTO{ { Id: 1, @@ -643,7 +654,9 @@ func TestGetAnnotations(t *testing.T) { } dashboard := grafanamodels.NewDashboard("dashWithNoAnnotations") pubdash := &PublicDashboard{Uid: "uid1", IsEnabled: true, OrgId: 1, DashboardUid: dashboard.Uid, AnnotationsEnabled: true} - fakeStore.On("FindPublicDashboardAndDashboardByAccessToken", mock.Anything, mock.AnythingOfType("string")).Return(pubdash, dashboard, nil) + + fakeStore.On("FindByAccessToken", mock.Anything, mock.AnythingOfType("string")).Return(pubdash, nil) + fakeStore.On("FindDashboard", mock.Anything, mock.AnythingOfType("string")).Return(dashboard, nil) items, err := service.FindAnnotations(context.Background(), AnnotationsQueryDTO{}, "abc123") @@ -676,7 +689,9 @@ func TestGetAnnotations(t *testing.T) { annos := []DashAnnotation{grafanaAnnotation} dashboard := AddAnnotationsToDashboard(t, dash, annos) pubdash := &PublicDashboard{Uid: "uid1", IsEnabled: true, OrgId: 1, DashboardUid: dashboard.Uid, AnnotationsEnabled: false} - fakeStore.On("FindPublicDashboardAndDashboardByAccessToken", mock.Anything, mock.AnythingOfType("string")).Return(pubdash, dashboard, nil) + + fakeStore.On("FindByAccessToken", mock.Anything, mock.AnythingOfType("string")).Return(pubdash, nil) + fakeStore.On("FindDashboard", mock.Anything, mock.AnythingOfType("string")).Return(dashboard, nil) items, err := service.FindAnnotations(context.Background(), AnnotationsQueryDTO{}, "abc123") @@ -708,7 +723,10 @@ func TestGetAnnotations(t *testing.T) { annos := []DashAnnotation{grafanaAnnotation} dash = AddAnnotationsToDashboard(t, dash, annos) pubdash := &PublicDashboard{Uid: "uid1", IsEnabled: true, OrgId: 1, DashboardUid: dash.Uid, AnnotationsEnabled: true} - fakeStore.On("FindPublicDashboardAndDashboardByAccessToken", mock.Anything, mock.AnythingOfType("string")).Return(pubdash, dash, nil) + + fakeStore.On("FindByAccessToken", mock.Anything, mock.AnythingOfType("string")).Return(pubdash, nil) + fakeStore.On("FindDashboard", mock.Anything, mock.AnythingOfType("string")).Return(dash, nil) + annotationsRepo.On("Find", mock.Anything, mock.Anything).Return(nil, errors.New("failed")).Maybe() items, err := service.FindAnnotations(context.Background(), AnnotationsQueryDTO{}, "abc123") diff --git a/pkg/services/publicdashboards/service/service.go b/pkg/services/publicdashboards/service/service.go index 92e80fb9bcc..32a386cf11e 100644 --- a/pkg/services/publicdashboards/service/service.go +++ b/pkg/services/publicdashboards/service/service.go @@ -74,9 +74,9 @@ func (pd *PublicDashboardServiceImpl) FindDashboard(ctx context.Context, dashboa // FindPublicDashboardAndDashboardByAccessToken Gets public dashboard via access token func (pd *PublicDashboardServiceImpl) FindPublicDashboardAndDashboardByAccessToken(ctx context.Context, accessToken string) (*PublicDashboard, *models.Dashboard, error) { - pubdash, dash, err := pd.store.FindPublicDashboardAndDashboardByAccessToken(ctx, accessToken) ctxLogger := pd.log.FromContext(ctx) + pubdash, err := pd.store.FindByAccessToken(ctx, accessToken) if err != nil { return nil, nil, err } @@ -86,13 +86,18 @@ func (pd *PublicDashboardServiceImpl) FindPublicDashboardAndDashboardByAccessTok return nil, nil, ErrPublicDashboardNotFound } - if dash == nil { - ctxLogger.Error("FindPublicDashboardAndDashboardByAccessToken: Dashboard not found", "accessToken", accessToken) + if !pubdash.IsEnabled { + ctxLogger.Error("FindPublicDashboardAndDashboardByAccessToken: Public dashboard is disabled", "accessToken", accessToken) return nil, nil, ErrPublicDashboardNotFound } - if !pubdash.IsEnabled { - ctxLogger.Error("FindPublicDashboardAndDashboardByAccessToken: Public dashboard is disabled", "accessToken", accessToken) + dash, err := pd.store.FindDashboard(ctx, pubdash.DashboardUid) + if err != nil { + return nil, nil, err + } + + if dash == nil { + ctxLogger.Error("FindPublicDashboardAndDashboardByAccessToken: Dashboard not found", "accessToken", accessToken) return nil, nil, ErrPublicDashboardNotFound } diff --git a/pkg/services/publicdashboards/service/service_test.go b/pkg/services/publicdashboards/service/service_test.go index 3a2f6cb885d..0239f9ad12a 100644 --- a/pkg/services/publicdashboards/service/service_test.go +++ b/pkg/services/publicdashboards/service/service_test.go @@ -101,8 +101,8 @@ func TestGetPublicDashboard(t *testing.T) { store: &fakeStore, } - fakeStore.On("FindPublicDashboardAndDashboardByAccessToken", mock.Anything, mock.Anything). - Return(test.StoreResp.pd, test.StoreResp.d, test.StoreResp.err) + fakeStore.On("FindByAccessToken", mock.Anything, mock.Anything).Return(test.StoreResp.pd, test.StoreResp.err) + fakeStore.On("FindDashboard", mock.Anything, mock.Anything).Return(test.StoreResp.d, test.StoreResp.err) pdc, dash, err := service.FindPublicDashboardAndDashboardByAccessToken(context.Background(), test.AccessToken) if test.ErrResp != nil {