Chore: Remove bus from alerting rule (#47508)

* Chore: Remove bus from alerting rule

* fix alerting tests

* fix provide service
This commit is contained in:
Serge Zaitsev 2022-04-08 14:30:25 +02:00 committed by GitHub
parent ad432108e6
commit b31c7d3654
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 28 additions and 21 deletions

View File

@ -9,7 +9,6 @@ import (
"github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/api/dtos"
"github.com/grafana/grafana/pkg/api/response" "github.com/grafana/grafana/pkg/api/response"
"github.com/grafana/grafana/pkg/bus"
"github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/alerting" "github.com/grafana/grafana/pkg/services/alerting"
"github.com/grafana/grafana/pkg/services/guardian" "github.com/grafana/grafana/pkg/services/guardian"
@ -491,7 +490,7 @@ func (hs *HTTPServer) NotificationTest(c *models.ReqContext) response.Response {
SecureSettings: dto.SecureSettings, 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) { if errors.Is(err, models.ErrSmtpNotEnabled) {
return response.Error(412, err.Error(), err) return response.Error(412, err.Error(), err)
} }

View File

@ -30,6 +30,7 @@ type AlertStore interface {
GetDashboardUIDById(context.Context, *models.GetDashboardRefByIdQuery) error GetDashboardUIDById(context.Context, *models.GetDashboardRefByIdQuery) error
SetAlertNotificationStateToCompleteCommand(context.Context, *models.SetAlertNotificationStateToCompleteCommand) error SetAlertNotificationStateToCompleteCommand(context.Context, *models.SetAlertNotificationStateToCompleteCommand) error
SetAlertNotificationStateToPendingCommand(context.Context, *models.SetAlertNotificationStateToPendingCommand) error SetAlertNotificationStateToPendingCommand(context.Context, *models.SetAlertNotificationStateToPendingCommand) error
GetAlertNotificationUidWithId(context.Context, *models.GetAlertNotificationUidQuery) error
GetAlertNotificationsWithUidToSend(context.Context, *models.GetAlertNotificationsWithUidToSendQuery) error GetAlertNotificationsWithUidToSend(context.Context, *models.GetAlertNotificationsWithUidToSendQuery) error
GetOrCreateAlertNotificationState(context.Context, *models.GetOrCreateNotificationStateQuery) error GetOrCreateAlertNotificationState(context.Context, *models.GetOrCreateNotificationStateQuery) error
SetAlertState(context.Context, *models.SetAlertStateCommand) error SetAlertState(context.Context, *models.SetAlertStateCommand) error

View File

@ -65,6 +65,10 @@ func (a *AlertStoreMock) GetAllAlertQueryHandler(c context.Context, cmd *models.
return nil 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 { func (a *AlertStoreMock) GetAlertNotificationsWithUidToSend(c context.Context, cmd *models.GetAlertNotificationsWithUidToSendQuery) error {
if a.getAlertNotificationsWithUidToSend != nil { if a.getAlertNotificationsWithUidToSend != nil {
return a.getAlertNotificationsWithUidToSend(c, cmd) return a.getAlertNotificationsWithUidToSend(c, cmd)

View File

@ -22,13 +22,15 @@ type DashAlertExtractor interface {
type DashAlertExtractorService struct { type DashAlertExtractorService struct {
datasourcePermissionsService permissions.DatasourcePermissionsService datasourcePermissionsService permissions.DatasourcePermissionsService
datasourceService datasources.DataSourceService datasourceService datasources.DataSourceService
alertStore AlertStore
log log.Logger 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{ return &DashAlertExtractorService{
datasourcePermissionsService: datasourcePermissionsService, datasourcePermissionsService: datasourcePermissionsService,
datasourceService: datasourceService, datasourceService: datasourceService,
alertStore: alertStore,
log: log.New("alerting.extractor"), log: log.New("alerting.extractor"),
} }
} }
@ -231,7 +233,7 @@ func (e *DashAlertExtractorService) getAlertFromPanels(ctx context.Context, json
alert.Settings = jsonAlert alert.Settings = jsonAlert
// validate // validate
_, err = NewRuleFromDBAlert(ctx, alert, logTranslationFailures) _, err = NewRuleFromDBAlert(ctx, e.alertStore, alert, logTranslationFailures)
if err != nil { if err != nil {
return nil, err return nil, err
} }

View File

@ -12,6 +12,7 @@ import (
"github.com/grafana/grafana/pkg/services/datasources" "github.com/grafana/grafana/pkg/services/datasources"
"github.com/grafana/grafana/pkg/services/datasources/permissions" "github.com/grafana/grafana/pkg/services/datasources/permissions"
"github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/sqlstore/mockstore"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
) )
@ -36,7 +37,8 @@ func TestAlertRuleExtraction(t *testing.T) {
} }
dsService := &fakeDatasourceService{ExpectedDatasource: defaultDs} 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) { t.Run("Parsing alert rules from dashboard json", func(t *testing.T) {
dashJSON, err := simplejson.NewJson(json) dashJSON, err := simplejson.NewJson(json)
@ -312,7 +314,7 @@ func TestFilterPermissionsErrors(t *testing.T) {
dsPermissions := permissions.NewMockDatasourcePermissionService() dsPermissions := permissions.NewMockDatasourcePermissionService()
dsService := &fakeDatasourceService{ExpectedDatasource: defaultDs} dsService := &fakeDatasourceService{ExpectedDatasource: defaultDs}
extractor := ProvideDashAlertExtractorService(dsPermissions, dsService) extractor := ProvideDashAlertExtractorService(dsPermissions, dsService, nil)
tc := []struct { tc := []struct {
name string name string

View File

@ -38,7 +38,7 @@ func (arr *defaultRuleReader) fetch(ctx context.Context) []*Rule {
res := make([]*Rule, 0) res := make([]*Rule, 0)
for _, ruleDef := range cmd.Result { 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) arr.log.Error("Could not build alert model for rule", "ruleId", ruleDef.Id, "error", err)
} else { } else {
res = append(res, model) res = append(res, model)

View File

@ -9,7 +9,6 @@ import (
"strconv" "strconv"
"time" "time"
"github.com/grafana/grafana/pkg/bus"
"github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/components/simplejson"
"github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/models"
) )
@ -145,7 +144,7 @@ func getForValue(rawFor string) (time.Duration, error) {
// NewRuleFromDBAlert maps a db version of // NewRuleFromDBAlert maps a db version of
// alert to an in-memory version. // 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 := &Rule{}
model.ID = ruleDef.Id model.ID = ruleDef.Id
model.OrgID = ruleDef.OrgId 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() { for _, v := range ruleDef.Settings.Get("notifications").MustArray() {
jsonModel := simplejson.NewFromAny(v) jsonModel := simplejson.NewFromAny(v)
if id, err := jsonModel.Get("id").Int64(); err == nil { 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 err != nil {
if !errors.Is(err, models.ErrAlertNotificationFailedTranslateUniqueID) { 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) 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 return model, nil
} }
func translateNotificationIDToUID(ctx context.Context, id int64, orgID int64) (string, error) { func translateNotificationIDToUID(ctx context.Context, store AlertStore, id int64, orgID int64) (string, error) {
notificationUID, err := getAlertNotificationUIDByIDAndOrgID(ctx, id, orgID) notificationUID, err := getAlertNotificationUIDByIDAndOrgID(ctx, store, id, orgID)
if err != nil { if err != nil {
return "", err return "", err
} }
@ -220,13 +219,13 @@ func translateNotificationIDToUID(ctx context.Context, id int64, orgID int64) (s
return notificationUID, nil 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{ query := &models.GetAlertNotificationUidQuery{
OrgId: orgID, OrgId: orgID,
Id: notificationID, Id: notificationID,
} }
if err := bus.Dispatch(ctx, query); err != nil { if err := store.GetAlertNotificationUidWithId(ctx, query); err != nil {
return "", err return "", err
} }

View File

@ -130,7 +130,7 @@ func TestAlertRuleModel(t *testing.T) {
Settings: alertJSON, Settings: alertJSON,
} }
alertRule, err := NewRuleFromDBAlert(context.Background(), alert, false) alertRule, err := NewRuleFromDBAlert(context.Background(), sqlStore, alert, false)
require.Nil(t, err) require.Nil(t, err)
require.Len(t, alertRule.Conditions, 1) require.Len(t, alertRule.Conditions, 1)
@ -169,7 +169,7 @@ func TestAlertRuleModel(t *testing.T) {
Settings: alertJSON, Settings: alertJSON,
} }
alertRule, err := NewRuleFromDBAlert(context.Background(), alert, false) alertRule, err := NewRuleFromDBAlert(context.Background(), sqlStore, alert, false)
require.Nil(t, err) require.Nil(t, err)
require.NotContains(t, alertRule.Notifications, "999") require.NotContains(t, alertRule.Notifications, "999")
require.Contains(t, alertRule.Notifications, "notifier2") require.Contains(t, alertRule.Notifications, "notifier2")
@ -200,7 +200,7 @@ func TestAlertRuleModel(t *testing.T) {
Settings: alertJSON, Settings: alertJSON,
} }
alertRule, err := NewRuleFromDBAlert(context.Background(), alert, false) alertRule, err := NewRuleFromDBAlert(context.Background(), sqlStore, alert, false)
require.Nil(t, err) require.Nil(t, err)
require.EqualValues(t, alertRule.Frequency, 60) require.EqualValues(t, alertRule.Frequency, 60)
}) })
@ -238,7 +238,7 @@ func TestAlertRuleModel(t *testing.T) {
Settings: alertJSON, Settings: alertJSON,
} }
_, err := NewRuleFromDBAlert(context.Background(), alert, false) _, err := NewRuleFromDBAlert(context.Background(), sqlStore, alert, false)
require.NotNil(t, err) 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") 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")
}) })

View File

@ -25,7 +25,7 @@ func (e *AlertEngine) AlertTest(orgID int64, dashboard *simplejson.Json, panelID
if alert.PanelId != panelID { if alert.PanelId != panelID {
continue continue
} }
rule, err := NewRuleFromDBAlert(context.Background(), alert, true) rule, err := NewRuleFromDBAlert(context.Background(), e.sqlStore, alert, true)
if err != nil { if err != nil {
return nil, err return nil, err
} }

View File

@ -201,7 +201,7 @@ func createDashboard(t *testing.T, sqlStore *sqlstore.SQLStore, user models.Sign
} }
dashboardStore := database.ProvideDashboardStore(sqlStore) dashboardStore := database.ProvideDashboardStore(sqlStore)
dashAlertExtractor := alerting.ProvideDashAlertExtractorService(nil, nil) dashAlertExtractor := alerting.ProvideDashAlertExtractorService(nil, nil, nil)
service := dashboardservice.ProvideDashboardService( service := dashboardservice.ProvideDashboardService(
setting.NewCfg(), dashboardStore, dashAlertExtractor, setting.NewCfg(), dashboardStore, dashAlertExtractor,
featuremgmt.WithFeatures(), acmock.NewPermissionsServicesMock(), featuremgmt.WithFeatures(), acmock.NewPermissionsServicesMock(),

View File

@ -1422,7 +1422,7 @@ func createDashboard(t *testing.T, sqlStore *sqlstore.SQLStore, user *models.Sig
} }
dashboardStore := database.ProvideDashboardStore(sqlStore) dashboardStore := database.ProvideDashboardStore(sqlStore)
dashAlertService := alerting.ProvideDashAlertExtractorService(nil, nil) dashAlertService := alerting.ProvideDashAlertExtractorService(nil, nil, nil)
service := dashboardservice.ProvideDashboardService( service := dashboardservice.ProvideDashboardService(
setting.NewCfg(), dashboardStore, dashAlertService, setting.NewCfg(), dashboardStore, dashAlertService,
featuremgmt.WithFeatures(), acmock.NewPermissionsServicesMock(), featuremgmt.WithFeatures(), acmock.NewPermissionsServicesMock(),