From b31c7d36548d831bbd1e8a621cbac8f3fc9a5a68 Mon Sep 17 00:00:00 2001 From: Serge Zaitsev Date: Fri, 8 Apr 2022 14:30:25 +0200 Subject: [PATCH] Chore: Remove bus from alerting rule (#47508) * Chore: Remove bus from alerting rule * fix alerting tests * fix provide service --- pkg/api/alerting.go | 3 +-- pkg/services/alerting/engine.go | 1 + pkg/services/alerting/engine_test.go | 4 ++++ pkg/services/alerting/extractor.go | 6 ++++-- pkg/services/alerting/extractor_test.go | 6 ++++-- pkg/services/alerting/reader.go | 2 +- pkg/services/alerting/rule.go | 13 ++++++------- pkg/services/alerting/rule_test.go | 8 ++++---- pkg/services/alerting/test_rule.go | 2 +- .../libraryelements/libraryelements_test.go | 2 +- pkg/services/librarypanels/librarypanels_test.go | 2 +- 11 files changed, 28 insertions(+), 21 deletions(-) diff --git a/pkg/api/alerting.go b/pkg/api/alerting.go index 4b58ace4bce..487d4ac9905 100644 --- a/pkg/api/alerting.go +++ b/pkg/api/alerting.go @@ -9,7 +9,6 @@ import ( "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/api/response" - "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/alerting" "github.com/grafana/grafana/pkg/services/guardian" @@ -491,7 +490,7 @@ func (hs *HTTPServer) NotificationTest(c *models.ReqContext) response.Response { SecureSettings: dto.SecureSettings, } - if err := bus.Dispatch(c.Req.Context(), cmd); err != nil { + if err := hs.AlertNotificationService.HandleNotificationTestCommand(c.Req.Context(), cmd); err != nil { if errors.Is(err, models.ErrSmtpNotEnabled) { return response.Error(412, err.Error(), err) } diff --git a/pkg/services/alerting/engine.go b/pkg/services/alerting/engine.go index cb39ff67448..9566d7725b4 100644 --- a/pkg/services/alerting/engine.go +++ b/pkg/services/alerting/engine.go @@ -30,6 +30,7 @@ type AlertStore interface { GetDashboardUIDById(context.Context, *models.GetDashboardRefByIdQuery) error SetAlertNotificationStateToCompleteCommand(context.Context, *models.SetAlertNotificationStateToCompleteCommand) error SetAlertNotificationStateToPendingCommand(context.Context, *models.SetAlertNotificationStateToPendingCommand) error + GetAlertNotificationUidWithId(context.Context, *models.GetAlertNotificationUidQuery) error GetAlertNotificationsWithUidToSend(context.Context, *models.GetAlertNotificationsWithUidToSendQuery) error GetOrCreateAlertNotificationState(context.Context, *models.GetOrCreateNotificationStateQuery) error SetAlertState(context.Context, *models.SetAlertStateCommand) error diff --git a/pkg/services/alerting/engine_test.go b/pkg/services/alerting/engine_test.go index 6e7fb674d68..3ed3ee42362 100644 --- a/pkg/services/alerting/engine_test.go +++ b/pkg/services/alerting/engine_test.go @@ -65,6 +65,10 @@ func (a *AlertStoreMock) GetAllAlertQueryHandler(c context.Context, cmd *models. return nil } +func (a *AlertStoreMock) GetAlertNotificationUidWithId(c context.Context, query *models.GetAlertNotificationUidQuery) error { + return nil +} + func (a *AlertStoreMock) GetAlertNotificationsWithUidToSend(c context.Context, cmd *models.GetAlertNotificationsWithUidToSendQuery) error { if a.getAlertNotificationsWithUidToSend != nil { return a.getAlertNotificationsWithUidToSend(c, cmd) diff --git a/pkg/services/alerting/extractor.go b/pkg/services/alerting/extractor.go index 3238b5b60ac..460489b0a3d 100644 --- a/pkg/services/alerting/extractor.go +++ b/pkg/services/alerting/extractor.go @@ -22,13 +22,15 @@ type DashAlertExtractor interface { type DashAlertExtractorService struct { datasourcePermissionsService permissions.DatasourcePermissionsService datasourceService datasources.DataSourceService + alertStore AlertStore log log.Logger } -func ProvideDashAlertExtractorService(datasourcePermissionsService permissions.DatasourcePermissionsService, datasourceService datasources.DataSourceService) *DashAlertExtractorService { +func ProvideDashAlertExtractorService(datasourcePermissionsService permissions.DatasourcePermissionsService, datasourceService datasources.DataSourceService, alertStore AlertStore) *DashAlertExtractorService { return &DashAlertExtractorService{ datasourcePermissionsService: datasourcePermissionsService, datasourceService: datasourceService, + alertStore: alertStore, log: log.New("alerting.extractor"), } } @@ -231,7 +233,7 @@ func (e *DashAlertExtractorService) getAlertFromPanels(ctx context.Context, json alert.Settings = jsonAlert // validate - _, err = NewRuleFromDBAlert(ctx, alert, logTranslationFailures) + _, err = NewRuleFromDBAlert(ctx, e.alertStore, alert, logTranslationFailures) if err != nil { return nil, err } diff --git a/pkg/services/alerting/extractor_test.go b/pkg/services/alerting/extractor_test.go index 90c7499d9e3..9b2dba8b0fc 100644 --- a/pkg/services/alerting/extractor_test.go +++ b/pkg/services/alerting/extractor_test.go @@ -12,6 +12,7 @@ import ( "github.com/grafana/grafana/pkg/services/datasources" "github.com/grafana/grafana/pkg/services/datasources/permissions" "github.com/grafana/grafana/pkg/services/sqlstore" + "github.com/grafana/grafana/pkg/services/sqlstore/mockstore" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -36,7 +37,8 @@ func TestAlertRuleExtraction(t *testing.T) { } dsService := &fakeDatasourceService{ExpectedDatasource: defaultDs} - extractor := ProvideDashAlertExtractorService(dsPermissions, dsService) + store := mockstore.NewSQLStoreMock() + extractor := ProvideDashAlertExtractorService(dsPermissions, dsService, store) t.Run("Parsing alert rules from dashboard json", func(t *testing.T) { dashJSON, err := simplejson.NewJson(json) @@ -312,7 +314,7 @@ func TestFilterPermissionsErrors(t *testing.T) { dsPermissions := permissions.NewMockDatasourcePermissionService() dsService := &fakeDatasourceService{ExpectedDatasource: defaultDs} - extractor := ProvideDashAlertExtractorService(dsPermissions, dsService) + extractor := ProvideDashAlertExtractorService(dsPermissions, dsService, nil) tc := []struct { name string diff --git a/pkg/services/alerting/reader.go b/pkg/services/alerting/reader.go index 6d4cc8dd80f..ebd41626164 100644 --- a/pkg/services/alerting/reader.go +++ b/pkg/services/alerting/reader.go @@ -38,7 +38,7 @@ func (arr *defaultRuleReader) fetch(ctx context.Context) []*Rule { res := make([]*Rule, 0) for _, ruleDef := range cmd.Result { - if model, err := NewRuleFromDBAlert(ctx, ruleDef, false); err != nil { + if model, err := NewRuleFromDBAlert(ctx, arr.sqlStore, ruleDef, false); err != nil { arr.log.Error("Could not build alert model for rule", "ruleId", ruleDef.Id, "error", err) } else { res = append(res, model) diff --git a/pkg/services/alerting/rule.go b/pkg/services/alerting/rule.go index 05ffc47805d..7dd920d2bfa 100644 --- a/pkg/services/alerting/rule.go +++ b/pkg/services/alerting/rule.go @@ -9,7 +9,6 @@ import ( "strconv" "time" - "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/models" ) @@ -145,7 +144,7 @@ func getForValue(rawFor string) (time.Duration, error) { // NewRuleFromDBAlert maps a db version of // alert to an in-memory version. -func NewRuleFromDBAlert(ctx context.Context, ruleDef *models.Alert, logTranslationFailures bool) (*Rule, error) { +func NewRuleFromDBAlert(ctx context.Context, store AlertStore, ruleDef *models.Alert, logTranslationFailures bool) (*Rule, error) { model := &Rule{} model.ID = ruleDef.Id model.OrgID = ruleDef.OrgId @@ -170,7 +169,7 @@ func NewRuleFromDBAlert(ctx context.Context, ruleDef *models.Alert, logTranslati for _, v := range ruleDef.Settings.Get("notifications").MustArray() { jsonModel := simplejson.NewFromAny(v) if id, err := jsonModel.Get("id").Int64(); err == nil { - uid, err := translateNotificationIDToUID(ctx, id, ruleDef.OrgId) + uid, err := translateNotificationIDToUID(ctx, store, id, ruleDef.OrgId) if err != nil { if !errors.Is(err, models.ErrAlertNotificationFailedTranslateUniqueID) { logger.Error("Failed to translate notification id to uid", "error", err.Error(), "dashboardId", model.DashboardID, "alert", model.Name, "panelId", model.PanelID, "notificationId", id) @@ -211,8 +210,8 @@ func NewRuleFromDBAlert(ctx context.Context, ruleDef *models.Alert, logTranslati return model, nil } -func translateNotificationIDToUID(ctx context.Context, id int64, orgID int64) (string, error) { - notificationUID, err := getAlertNotificationUIDByIDAndOrgID(ctx, id, orgID) +func translateNotificationIDToUID(ctx context.Context, store AlertStore, id int64, orgID int64) (string, error) { + notificationUID, err := getAlertNotificationUIDByIDAndOrgID(ctx, store, id, orgID) if err != nil { return "", err } @@ -220,13 +219,13 @@ func translateNotificationIDToUID(ctx context.Context, id int64, orgID int64) (s return notificationUID, nil } -func getAlertNotificationUIDByIDAndOrgID(ctx context.Context, notificationID int64, orgID int64) (string, error) { +func getAlertNotificationUIDByIDAndOrgID(ctx context.Context, store AlertStore, notificationID int64, orgID int64) (string, error) { query := &models.GetAlertNotificationUidQuery{ OrgId: orgID, Id: notificationID, } - if err := bus.Dispatch(ctx, query); err != nil { + if err := store.GetAlertNotificationUidWithId(ctx, query); err != nil { return "", err } diff --git a/pkg/services/alerting/rule_test.go b/pkg/services/alerting/rule_test.go index 55c603a4c2b..506b7cb03b6 100644 --- a/pkg/services/alerting/rule_test.go +++ b/pkg/services/alerting/rule_test.go @@ -130,7 +130,7 @@ func TestAlertRuleModel(t *testing.T) { Settings: alertJSON, } - alertRule, err := NewRuleFromDBAlert(context.Background(), alert, false) + alertRule, err := NewRuleFromDBAlert(context.Background(), sqlStore, alert, false) require.Nil(t, err) require.Len(t, alertRule.Conditions, 1) @@ -169,7 +169,7 @@ func TestAlertRuleModel(t *testing.T) { Settings: alertJSON, } - alertRule, err := NewRuleFromDBAlert(context.Background(), alert, false) + alertRule, err := NewRuleFromDBAlert(context.Background(), sqlStore, alert, false) require.Nil(t, err) require.NotContains(t, alertRule.Notifications, "999") require.Contains(t, alertRule.Notifications, "notifier2") @@ -200,7 +200,7 @@ func TestAlertRuleModel(t *testing.T) { Settings: alertJSON, } - alertRule, err := NewRuleFromDBAlert(context.Background(), alert, false) + alertRule, err := NewRuleFromDBAlert(context.Background(), sqlStore, alert, false) require.Nil(t, err) require.EqualValues(t, alertRule.Frequency, 60) }) @@ -238,7 +238,7 @@ func TestAlertRuleModel(t *testing.T) { Settings: alertJSON, } - _, err := NewRuleFromDBAlert(context.Background(), alert, false) + _, err := NewRuleFromDBAlert(context.Background(), sqlStore, alert, false) require.NotNil(t, err) require.EqualValues(t, err.Error(), "alert validation error: Neither id nor uid is specified in 'notifications' block, type assertion to string failed AlertId: 1 PanelId: 1 DashboardId: 1") }) diff --git a/pkg/services/alerting/test_rule.go b/pkg/services/alerting/test_rule.go index b8c4e2e5edf..011990faad7 100644 --- a/pkg/services/alerting/test_rule.go +++ b/pkg/services/alerting/test_rule.go @@ -25,7 +25,7 @@ func (e *AlertEngine) AlertTest(orgID int64, dashboard *simplejson.Json, panelID if alert.PanelId != panelID { continue } - rule, err := NewRuleFromDBAlert(context.Background(), alert, true) + rule, err := NewRuleFromDBAlert(context.Background(), e.sqlStore, alert, true) if err != nil { return nil, err } diff --git a/pkg/services/libraryelements/libraryelements_test.go b/pkg/services/libraryelements/libraryelements_test.go index 2358b8f6342..a4bc6ed6cfa 100644 --- a/pkg/services/libraryelements/libraryelements_test.go +++ b/pkg/services/libraryelements/libraryelements_test.go @@ -201,7 +201,7 @@ func createDashboard(t *testing.T, sqlStore *sqlstore.SQLStore, user models.Sign } dashboardStore := database.ProvideDashboardStore(sqlStore) - dashAlertExtractor := alerting.ProvideDashAlertExtractorService(nil, nil) + dashAlertExtractor := alerting.ProvideDashAlertExtractorService(nil, nil, nil) service := dashboardservice.ProvideDashboardService( setting.NewCfg(), dashboardStore, dashAlertExtractor, featuremgmt.WithFeatures(), acmock.NewPermissionsServicesMock(), diff --git a/pkg/services/librarypanels/librarypanels_test.go b/pkg/services/librarypanels/librarypanels_test.go index 0d13913c966..26f28a798c2 100644 --- a/pkg/services/librarypanels/librarypanels_test.go +++ b/pkg/services/librarypanels/librarypanels_test.go @@ -1422,7 +1422,7 @@ func createDashboard(t *testing.T, sqlStore *sqlstore.SQLStore, user *models.Sig } dashboardStore := database.ProvideDashboardStore(sqlStore) - dashAlertService := alerting.ProvideDashAlertExtractorService(nil, nil) + dashAlertService := alerting.ProvideDashAlertExtractorService(nil, nil, nil) service := dashboardservice.ProvideDashboardService( setting.NewCfg(), dashboardStore, dashAlertService, featuremgmt.WithFeatures(), acmock.NewPermissionsServicesMock(),