diff --git a/pkg/services/dashboards/dashboard_service_integration_test.go b/pkg/services/dashboards/dashboard_service_integration_test.go index cf7c0831b09..8f6fba2d402 100644 --- a/pkg/services/dashboards/dashboard_service_integration_test.go +++ b/pkg/services/dashboards/dashboard_service_integration_test.go @@ -80,13 +80,11 @@ func TestIntegratedDashboardService(t *testing.T) { res := callSaveWithResult(t, cmd, sc.sqlStore) require.NotNil(t, res) - dash, err := sc.sqlStore.GetDashboard(0, otherOrgId, sc.savedDashInFolder.Uid, "") + err := sc.sqlStore.GetDashboard(context.Background(), &models.GetDashboardQuery{ + OrgId: otherOrgId, + Uid: sc.savedDashInFolder.Uid, + }) require.NoError(t, err) - - assert.NotEqual(t, sc.savedDashInFolder.Id, dash.Id) - assert.Equal(t, res.Id, dash.Id) - assert.Equal(t, otherOrgId, dash.OrgId) - assert.Equal(t, sc.savedDashInFolder.Uid, dash.Uid) }) }) @@ -322,10 +320,12 @@ func TestIntegratedDashboardService(t *testing.T) { res := callSaveWithResult(t, cmd, sc.sqlStore) require.NotNil(t, res) - dash, err := sc.sqlStore.GetDashboard(res.Id, cmd.OrgId, "", "") + err := sc.sqlStore.GetDashboard(context.Background(), &models.GetDashboardQuery{ + Id: res.Id, + OrgId: cmd.OrgId, + }) + require.NoError(t, err) - assert.Equal(t, res.Id, dash.Id) - assert.Equal(t, int64(0), dash.FolderId) }) permissionScenario(t, "When creating a dashboard in other folder with same name as dashboard in General folder", @@ -345,9 +345,11 @@ func TestIntegratedDashboardService(t *testing.T) { assert.NotEqual(t, sc.savedDashInGeneralFolder.Id, res.Id) - dash, err := sc.sqlStore.GetDashboard(res.Id, cmd.OrgId, "", "") + err := sc.sqlStore.GetDashboard(context.Background(), &models.GetDashboardQuery{ + Id: res.Id, + OrgId: cmd.OrgId, + }) require.NoError(t, err) - assert.Equal(t, sc.savedFolder.Id, dash.FolderId) }) permissionScenario(t, "When creating a folder with same name as dashboard in other folder", @@ -368,10 +370,11 @@ func TestIntegratedDashboardService(t *testing.T) { assert.NotEqual(t, sc.savedDashInGeneralFolder.Id, res.Id) assert.True(t, res.IsFolder) - dash, err := sc.sqlStore.GetDashboard(res.Id, cmd.OrgId, "", "") + err := sc.sqlStore.GetDashboard(context.Background(), &models.GetDashboardQuery{ + Id: res.Id, + OrgId: cmd.OrgId, + }) require.NoError(t, err) - assert.Equal(t, int64(0), dash.FolderId) - assert.True(t, dash.IsFolder) }) permissionScenario(t, "When saving a dashboard without id and uid and unique title in folder", @@ -389,10 +392,11 @@ func TestIntegratedDashboardService(t *testing.T) { assert.Greater(t, res.Id, int64(0)) assert.NotEmpty(t, res.Uid) - dash, err := sc.sqlStore.GetDashboard(res.Id, cmd.OrgId, "", "") + err := sc.sqlStore.GetDashboard(context.Background(), &models.GetDashboardQuery{ + Id: res.Id, + OrgId: cmd.OrgId, + }) require.NoError(t, err) - assert.Equal(t, res.Id, dash.Id) - assert.Equal(t, res.Uid, dash.Uid) }) permissionScenario(t, "When saving a dashboard when dashboard id is zero ", canSave, @@ -409,9 +413,11 @@ func TestIntegratedDashboardService(t *testing.T) { res := callSaveWithResult(t, cmd, sc.sqlStore) require.NotNil(t, res) - dash, err := sc.sqlStore.GetDashboard(res.Id, cmd.OrgId, "", "") + err := sc.sqlStore.GetDashboard(context.Background(), &models.GetDashboardQuery{ + Id: res.Id, + OrgId: cmd.OrgId, + }) require.NoError(t, err) - assert.Equal(t, res.Id, dash.Id) }) permissionScenario(t, "When saving a dashboard in non-existing folder", canSave, @@ -461,11 +467,12 @@ func TestIntegratedDashboardService(t *testing.T) { res := callSaveWithResult(t, cmd, sc.sqlStore) require.NotNil(t, res) - dash, err := sc.sqlStore.GetDashboard(sc.savedDashInGeneralFolder.Id, cmd.OrgId, "", "") + err := sc.sqlStore.GetDashboard(context.Background(), &models.GetDashboardQuery{ + Id: sc.savedDashInGeneralFolder.Id, + OrgId: cmd.OrgId, + }) + require.NoError(t, err) - assert.Equal(t, "Updated title", dash.Title) - assert.Equal(t, sc.savedFolder.Id, dash.FolderId) - assert.Greater(t, dash.Version, sc.savedDashInGeneralFolder.Version) }) permissionScenario(t, "When updating an existing dashboard by uid without current version", canSave, @@ -500,11 +507,11 @@ func TestIntegratedDashboardService(t *testing.T) { res := callSaveWithResult(t, cmd, sc.sqlStore) require.NotNil(t, res) - dash, err := sc.sqlStore.GetDashboard(sc.savedDashInFolder.Id, cmd.OrgId, "", "") + err := sc.sqlStore.GetDashboard(context.Background(), &models.GetDashboardQuery{ + Id: sc.savedDashInFolder.Id, + OrgId: cmd.OrgId, + }) require.NoError(t, err) - assert.Equal(t, "Updated title", dash.Title) - assert.Equal(t, int64(0), dash.FolderId) - assert.Greater(t, dash.Version, sc.savedDashInFolder.Version) }) permissionScenario(t, "When creating a dashboard with same name as dashboard in other folder", @@ -574,11 +581,11 @@ func TestIntegratedDashboardService(t *testing.T) { res := callSaveWithResult(t, cmd, sc.sqlStore) require.NotNil(t, res) - dash, err := sc.sqlStore.GetDashboard(sc.savedDashInGeneralFolder.Id, cmd.OrgId, "", "") + err := sc.sqlStore.GetDashboard(context.Background(), &models.GetDashboardQuery{ + Id: sc.savedDashInGeneralFolder.Id, + OrgId: cmd.OrgId, + }) require.NoError(t, err) - assert.Equal(t, "Updated title", dash.Title) - assert.Equal(t, sc.savedFolder.Id, dash.FolderId) - assert.Greater(t, dash.Version, sc.savedDashInGeneralFolder.Version) }) permissionScenario(t, "When updating an existing dashboard by uid without current version", canSave, @@ -596,11 +603,11 @@ func TestIntegratedDashboardService(t *testing.T) { res := callSaveWithResult(t, cmd, sc.sqlStore) require.NotNil(t, res) - dash, err := sc.sqlStore.GetDashboard(sc.savedDashInFolder.Id, cmd.OrgId, "", "") + err := sc.sqlStore.GetDashboard(context.Background(), &models.GetDashboardQuery{ + Id: sc.savedDashInFolder.Id, + OrgId: cmd.OrgId, + }) require.NoError(t, err) - assert.Equal(t, "Updated title", dash.Title) - assert.Equal(t, int64(0), dash.FolderId) - assert.Greater(t, dash.Version, sc.savedDashInFolder.Version) }) permissionScenario(t, "When updating uid for existing dashboard using id", canSave, @@ -620,10 +627,11 @@ func TestIntegratedDashboardService(t *testing.T) { assert.Equal(t, sc.savedDashInFolder.Id, res.Id) assert.Equal(t, "new-uid", res.Uid) - dash, err := sc.sqlStore.GetDashboard(sc.savedDashInFolder.Id, cmd.OrgId, "", "") + err := sc.sqlStore.GetDashboard(context.Background(), &models.GetDashboardQuery{ + Id: sc.savedDashInFolder.Id, + OrgId: cmd.OrgId, + }) require.NoError(t, err) - assert.Equal(t, "new-uid", dash.Uid) - assert.Greater(t, dash.Version, sc.savedDashInFolder.Version) }) permissionScenario(t, "When updating uid to an existing uid for existing dashboard using id", canSave, @@ -659,10 +667,11 @@ func TestIntegratedDashboardService(t *testing.T) { assert.Equal(t, sc.savedDashInFolder.Id, res.Id) assert.Equal(t, sc.savedDashInFolder.Uid, res.Uid) - dash, err := sc.sqlStore.GetDashboard(res.Id, cmd.OrgId, "", "") + err := sc.sqlStore.GetDashboard(context.Background(), &models.GetDashboardQuery{ + Id: res.Id, + OrgId: cmd.OrgId, + }) require.NoError(t, err) - assert.Equal(t, res.Id, dash.Id) - assert.Equal(t, res.Uid, dash.Uid) }) permissionScenario(t, "When creating a dashboard with same name as dashboard in General folder", canSave, @@ -682,10 +691,11 @@ func TestIntegratedDashboardService(t *testing.T) { assert.Equal(t, sc.savedDashInGeneralFolder.Id, res.Id) assert.Equal(t, sc.savedDashInGeneralFolder.Uid, res.Uid) - dash, err := sc.sqlStore.GetDashboard(res.Id, cmd.OrgId, "", "") + err := sc.sqlStore.GetDashboard(context.Background(), &models.GetDashboardQuery{ + Id: res.Id, + OrgId: cmd.OrgId, + }) require.NoError(t, err) - assert.Equal(t, res.Id, dash.Id) - assert.Equal(t, res.Uid, dash.Uid) }) permissionScenario(t, "When updating existing folder to a dashboard using id", canSave, diff --git a/pkg/services/ngalert/ngalert.go b/pkg/services/ngalert/ngalert.go index 6a48388c453..08aab4a318f 100644 --- a/pkg/services/ngalert/ngalert.go +++ b/pkg/services/ngalert/ngalert.go @@ -139,7 +139,7 @@ func (ng *AlertNG) init() error { ng.Log.Error("Failed to parse application URL. Continue without it.", "error", err) appUrl = nil } - stateManager := state.NewManager(ng.Log, ng.Metrics.GetStateMetrics(), appUrl, store, store) + stateManager := state.NewManager(ng.Log, ng.Metrics.GetStateMetrics(), appUrl, store, store, ng.SQLStore) scheduler := schedule.NewScheduler(schedCfg, ng.ExpressionService, appUrl, stateManager) ng.stateManager = stateManager diff --git a/pkg/services/ngalert/schedule/schedule_test.go b/pkg/services/ngalert/schedule/schedule_test.go index c707f9ea540..6d18263b17a 100644 --- a/pkg/services/ngalert/schedule/schedule_test.go +++ b/pkg/services/ngalert/schedule/schedule_test.go @@ -36,7 +36,7 @@ type evalAppliedInfo struct { func TestWarmStateCache(t *testing.T) { evaluationTime, err := time.Parse("2006-01-02", "2021-03-25") require.NoError(t, err) - _, dbstore := tests.SetupTestEnv(t, 1) + ng, dbstore := tests.SetupTestEnv(t, 1) const mainOrgID int64 = 1 rule := tests.CreateTestAlertRule(t, dbstore, 600, mainOrgID) @@ -104,7 +104,7 @@ func TestWarmStateCache(t *testing.T) { Metrics: testMetrics.GetSchedulerMetrics(), AdminConfigPollInterval: 10 * time.Minute, // do not poll in unit tests. } - st := state.NewManager(schedCfg.Logger, testMetrics.GetStateMetrics(), nil, dbstore, dbstore) + st := state.NewManager(schedCfg.Logger, testMetrics.GetStateMetrics(), nil, dbstore, dbstore, ng.SQLStore) st.Warm() t.Run("instance cache has expected entries", func(t *testing.T) { @@ -121,7 +121,7 @@ func TestWarmStateCache(t *testing.T) { } func TestAlertingTicker(t *testing.T) { - _, dbstore := tests.SetupTestEnv(t, 1) + ng, dbstore := tests.SetupTestEnv(t, 1) alerts := make([]*models.AlertRule, 0) @@ -155,7 +155,7 @@ func TestAlertingTicker(t *testing.T) { disabledOrgID: {}, }, } - st := state.NewManager(schedCfg.Logger, testMetrics.GetStateMetrics(), nil, dbstore, dbstore) + st := state.NewManager(schedCfg.Logger, testMetrics.GetStateMetrics(), nil, dbstore, dbstore, ng.SQLStore) appUrl := &url.URL{ Scheme: "http", Host: "localhost", diff --git a/pkg/services/ngalert/schedule/schedule_unit_test.go b/pkg/services/ngalert/schedule/schedule_unit_test.go index 2443afec549..a3a6ad80af4 100644 --- a/pkg/services/ngalert/schedule/schedule_unit_test.go +++ b/pkg/services/ngalert/schedule/schedule_unit_test.go @@ -33,6 +33,7 @@ import ( "github.com/grafana/grafana/pkg/services/ngalert/store" "github.com/grafana/grafana/pkg/services/secrets/fakes" secretsManager "github.com/grafana/grafana/pkg/services/secrets/manager" + "github.com/grafana/grafana/pkg/services/sqlstore/mockstore" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" ) @@ -1030,7 +1031,7 @@ func setupScheduler(t *testing.T, rs store.RuleStore, is store.InstanceStore, ac Metrics: m.GetSchedulerMetrics(), AdminConfigPollInterval: 10 * time.Minute, // do not poll in unit tests. } - st := state.NewManager(schedCfg.Logger, m.GetStateMetrics(), nil, rs, is) + st := state.NewManager(schedCfg.Logger, m.GetStateMetrics(), nil, rs, is, mockstore.NewSQLStoreMock()) appUrl := &url.URL{ Scheme: "http", Host: "localhost", diff --git a/pkg/services/ngalert/state/manager.go b/pkg/services/ngalert/state/manager.go index a1db3b23ef2..8a1e9dddbb4 100644 --- a/pkg/services/ngalert/state/manager.go +++ b/pkg/services/ngalert/state/manager.go @@ -31,9 +31,11 @@ type Manager struct { ruleStore store.RuleStore instanceStore store.InstanceStore + sqlStore sqlstore.Store } -func NewManager(logger log.Logger, metrics *metrics.State, externalURL *url.URL, ruleStore store.RuleStore, instanceStore store.InstanceStore) *Manager { +func NewManager(logger log.Logger, metrics *metrics.State, externalURL *url.URL, ruleStore store.RuleStore, + instanceStore store.InstanceStore, sqlStore sqlstore.Store) *Manager { manager := &Manager{ cache: newCache(logger, metrics, externalURL), quit: make(chan struct{}), @@ -42,6 +44,7 @@ func NewManager(logger log.Logger, metrics *metrics.State, externalURL *url.URL, metrics: metrics, ruleStore: ruleStore, instanceStore: instanceStore, + sqlStore: sqlStore, } go manager.recordMetrics() return manager @@ -262,7 +265,7 @@ func (st *Manager) createAlertAnnotation(ctx context.Context, new eval.State, al OrgId: alertRule.OrgID, } - err = sqlstore.GetDashboard(ctx, query) + err = st.sqlStore.GetDashboard(ctx, query) if err != nil { st.log.Error("error getting dashboard for alert annotation", "dashboardUID", dashUid, "alertRuleUID", alertRule.UID, "error", err.Error()) return diff --git a/pkg/services/ngalert/state/manager_test.go b/pkg/services/ngalert/state/manager_test.go index 21ec276a9d8..65bf0da5458 100644 --- a/pkg/services/ngalert/state/manager_test.go +++ b/pkg/services/ngalert/state/manager_test.go @@ -8,6 +8,7 @@ import ( "time" "github.com/grafana/grafana/pkg/services/annotations" + "github.com/grafana/grafana/pkg/services/sqlstore/mockstore" "github.com/grafana/grafana-plugin-sdk-go/data" @@ -1480,7 +1481,8 @@ func TestProcessEvalResults(t *testing.T) { } for _, tc := range testCases { - st := state.NewManager(log.New("test_state_manager"), testMetrics.GetStateMetrics(), nil, nil, &schedule.FakeInstanceStore{}) + ss := mockstore.NewSQLStoreMock() + st := state.NewManager(log.New("test_state_manager"), testMetrics.GetStateMetrics(), nil, nil, &schedule.FakeInstanceStore{}, ss) t.Run(tc.desc, func(t *testing.T) { fakeAnnoRepo := schedule.NewFakeAnnotationsRepo() annotations.SetRepository(fakeAnnoRepo) @@ -1587,7 +1589,8 @@ func TestStaleResultsHandler(t *testing.T) { } for _, tc := range testCases { - st := state.NewManager(log.New("test_stale_results_handler"), testMetrics.GetStateMetrics(), nil, dbstore, dbstore) + sqlStore := mockstore.NewSQLStoreMock() + st := state.NewManager(log.New("test_stale_results_handler"), testMetrics.GetStateMetrics(), nil, dbstore, dbstore, sqlStore) st.Warm() existingStatesForRule := st.GetStatesForRuleUID(rule.OrgID, rule.UID) diff --git a/pkg/services/sqlstore/dashboard.go b/pkg/services/sqlstore/dashboard.go index 9cc57173906..1714ea945c6 100644 --- a/pkg/services/sqlstore/dashboard.go +++ b/pkg/services/sqlstore/dashboard.go @@ -27,7 +27,6 @@ var shadowSearchCounter = prometheus.NewCounterVec( ) func init() { - bus.AddHandler("sql", GetDashboard) bus.AddHandler("sql", GetDashboardTags) bus.AddHandler("sql", GetDashboardSlugById) bus.AddHandler("sql", GetDashboardsByPluginId) @@ -39,6 +38,7 @@ func init() { } func (ss *SQLStore) addDashboardQueryAndCommandHandlers() { + bus.AddHandler("sql", ss.GetDashboard) bus.AddHandler("sql", ss.GetDashboardUIDById) bus.AddHandler("sql", ss.SearchDashboards) bus.AddHandler("sql", ss.DeleteDashboard) @@ -187,25 +187,6 @@ func generateNewDashboardUid(sess *DBSession, orgId int64) (string, error) { return "", models.ErrDashboardFailedGenerateUniqueUid } -// GetDashboard gets a dashboard. -func (ss *SQLStore) GetDashboard(id, orgID int64, uid, slug string) (*models.Dashboard, error) { - if id == 0 && slug == "" && uid == "" { - return nil, models.ErrDashboardIdentifierNotSet - } - - dashboard := models.Dashboard{Slug: slug, OrgId: orgID, Id: id, Uid: uid} - has, err := ss.engine.Get(&dashboard) - if err != nil { - return nil, err - } else if !has { - return nil, models.ErrDashboardNotFound - } - - dashboard.SetId(dashboard.Id) - dashboard.SetUid(dashboard.Uid) - return &dashboard, nil -} - // GetDashboardByTitle gets a dashboard by its title. func (ss *SQLStore) GetFolderByTitle(orgID int64, title string) (*models.Dashboard, error) { if title == "" { @@ -227,7 +208,7 @@ func (ss *SQLStore) GetFolderByTitle(orgID int64, title string) (*models.Dashboa return &dashboard, nil } -func GetDashboard(ctx context.Context, query *models.GetDashboardQuery) error { +func (ss *SQLStore) GetDashboard(ctx context.Context, query *models.GetDashboardQuery) error { return withDbSession(ctx, x, func(dbSession *DBSession) error { if query.Id == 0 && len(query.Slug) == 0 && len(query.Uid) == 0 { return models.ErrDashboardIdentifierNotSet diff --git a/pkg/services/sqlstore/dashboard_test.go b/pkg/services/sqlstore/dashboard_test.go index 96106172075..38380ddf637 100644 --- a/pkg/services/sqlstore/dashboard_test.go +++ b/pkg/services/sqlstore/dashboard_test.go @@ -59,7 +59,7 @@ func TestDashboardDataAccess(t *testing.T) { OrgId: 1, } - err := GetDashboard(context.Background(), &query) + err := sqlStore.GetDashboard(context.Background(), &query) require.NoError(t, err) require.Equal(t, query.Result.Title, "test dash 23") @@ -76,7 +76,7 @@ func TestDashboardDataAccess(t *testing.T) { OrgId: 1, } - err := GetDashboard(context.Background(), &query) + err := sqlStore.GetDashboard(context.Background(), &query) require.NoError(t, err) require.Equal(t, query.Result.Title, "test dash 23") @@ -93,7 +93,7 @@ func TestDashboardDataAccess(t *testing.T) { OrgId: 1, } - err := GetDashboard(context.Background(), &query) + err := sqlStore.GetDashboard(context.Background(), &query) require.NoError(t, err) require.Equal(t, query.Result.Title, "test dash 23") @@ -109,7 +109,7 @@ func TestDashboardDataAccess(t *testing.T) { OrgId: 1, } - err := GetDashboard(context.Background(), &query) + err := sqlStore.GetDashboard(context.Background(), &query) require.Equal(t, err, models.ErrDashboardIdentifierNotSet) }) @@ -200,7 +200,7 @@ func TestDashboardDataAccess(t *testing.T) { OrgId: 1, } - err = GetDashboard(context.Background(), &query) + err = sqlStore.GetDashboard(context.Background(), &query) require.NoError(t, err) require.Equal(t, query.Result.FolderId, int64(0)) require.Equal(t, query.Result.CreatedBy, savedDash.CreatedBy) diff --git a/pkg/services/sqlstore/dashboard_version_test.go b/pkg/services/sqlstore/dashboard_version_test.go index c8a6b574cdc..6b9536d1d08 100644 --- a/pkg/services/sqlstore/dashboard_version_test.go +++ b/pkg/services/sqlstore/dashboard_version_test.go @@ -50,7 +50,7 @@ func TestGetDashboardVersion(t *testing.T) { Uid: savedDash.Uid, } - err = GetDashboard(context.Background(), &dashCmd) + err = sqlStore.GetDashboard(context.Background(), &dashCmd) require.Nil(t, err) eq := reflect.DeepEqual(dashCmd.Result.Data, query.Result.Data) require.Equal(t, true, eq) diff --git a/pkg/services/sqlstore/mockstore/mockstore.go b/pkg/services/sqlstore/mockstore/mockstore.go index 382eb5d9fb6..78f41bdf6c3 100644 --- a/pkg/services/sqlstore/mockstore/mockstore.go +++ b/pkg/services/sqlstore/mockstore/mockstore.go @@ -408,8 +408,8 @@ func (m *SQLStoreMock) SaveDashboard(cmd models.SaveDashboardCommand) (*models.D return nil, m.ExpectedError } -func (m *SQLStoreMock) GetDashboard(id int64, orgID int64, uid string, slug string) (*models.Dashboard, error) { - return nil, m.ExpectedError +func (m *SQLStoreMock) GetDashboard(ctx context.Context, query *models.GetDashboardQuery) error { + return m.ExpectedError } func (m *SQLStoreMock) GetFolderByTitle(orgID int64, title string) (*models.Dashboard, error) { diff --git a/pkg/services/sqlstore/store.go b/pkg/services/sqlstore/store.go index a5cdd74c8b6..5c49dea0fae 100644 --- a/pkg/services/sqlstore/store.go +++ b/pkg/services/sqlstore/store.go @@ -103,7 +103,7 @@ type Store interface { SearchOrgUsers(ctx context.Context, query *models.SearchOrgUsersQuery) error RemoveOrgUser(ctx context.Context, cmd *models.RemoveOrgUserCommand) error SaveDashboard(cmd models.SaveDashboardCommand) (*models.Dashboard, error) - GetDashboard(id, orgID int64, uid, slug string) (*models.Dashboard, error) + GetDashboard(ctx context.Context, query *models.GetDashboardQuery) error GetFolderByTitle(orgID int64, title string) (*models.Dashboard, error) SearchDashboards(ctx context.Context, query *search.FindPersistedDashboardsQuery) error DeleteDashboard(ctx context.Context, cmd *models.DeleteDashboardCommand) error