From c5226a945b176017297e994716f47f3e8c35bc1b Mon Sep 17 00:00:00 2001 From: Leonard Gram Date: Fri, 16 Feb 2018 13:06:28 +0100 Subject: [PATCH 1/2] alerting: pausing alerts modifies updated. When pausing an alert the updated field is modified to the current time. Closes #10191 --- pkg/services/sqlstore/alert.go | 23 +++-- pkg/services/sqlstore/alert_test.go | 129 ++++++++++++++++++++++++++-- 2 files changed, 137 insertions(+), 15 deletions(-) diff --git a/pkg/services/sqlstore/alert.go b/pkg/services/sqlstore/alert.go index 96af8bc49ee..ab777f5f427 100644 --- a/pkg/services/sqlstore/alert.go +++ b/pkg/services/sqlstore/alert.go @@ -10,6 +10,9 @@ import ( m "github.com/grafana/grafana/pkg/models" ) +// timeNow makes it possible to test usage of time +var timeNow = time.Now + func init() { bus.AddHandler("sql", SaveAlerts) bus.AddHandler("sql", HandleAlertsQuery) @@ -138,7 +141,7 @@ func SaveAlerts(cmd *m.SaveAlertsCommand) error { return err } - if err := upsertAlerts(existingAlerts, cmd, sess); err != nil { + if err := updateAlerts(existingAlerts, cmd, sess); err != nil { return err } @@ -150,7 +153,7 @@ func SaveAlerts(cmd *m.SaveAlertsCommand) error { }) } -func upsertAlerts(existingAlerts []*m.Alert, cmd *m.SaveAlertsCommand, sess *DBSession) error { +func updateAlerts(existingAlerts []*m.Alert, cmd *m.SaveAlertsCommand, sess *DBSession) error { for _, alert := range cmd.Alerts { update := false var alertToUpdate *m.Alert @@ -166,7 +169,7 @@ func upsertAlerts(existingAlerts []*m.Alert, cmd *m.SaveAlertsCommand, sess *DBS if update { if alertToUpdate.ContainsUpdates(alert) { - alert.Updated = time.Now() + alert.Updated = timeNow() alert.State = alertToUpdate.State sess.MustCols("message") _, err := sess.Id(alert.Id).Update(alert) @@ -177,10 +180,10 @@ func upsertAlerts(existingAlerts []*m.Alert, cmd *m.SaveAlertsCommand, sess *DBS sqlog.Debug("Alert updated", "name", alert.Name, "id", alert.Id) } } else { - alert.Updated = time.Now() - alert.Created = time.Now() + alert.Updated = timeNow() + alert.Created = timeNow() alert.State = m.AlertStatePending - alert.NewStateDate = time.Now() + alert.NewStateDate = timeNow() _, err := sess.Insert(alert) if err != nil { @@ -244,7 +247,7 @@ func SetAlertState(cmd *m.SetAlertStateCommand) error { alert.State = cmd.State alert.StateChanges += 1 - alert.NewStateDate = time.Now() + alert.NewStateDate = timeNow() alert.EvalData = cmd.EvalData if cmd.Error == "" { @@ -267,11 +270,13 @@ func PauseAlert(cmd *m.PauseAlertCommand) error { var buffer bytes.Buffer params := make([]interface{}, 0) - buffer.WriteString(`UPDATE alert SET state = ?`) + buffer.WriteString(`UPDATE alert SET state = ?, new_state_date = ?`) if cmd.Paused { params = append(params, string(m.AlertStatePaused)) + params = append(params, timeNow()) } else { params = append(params, string(m.AlertStatePending)) + params = append(params, timeNow()) } buffer.WriteString(` WHERE id IN (?` + strings.Repeat(",?", len(cmd.AlertIds)-1) + `)`) @@ -297,7 +302,7 @@ func PauseAllAlerts(cmd *m.PauseAllAlertCommand) error { newState = string(m.AlertStatePending) } - res, err := sess.Exec(`UPDATE alert SET state = ?`, newState) + res, err := sess.Exec(`UPDATE alert SET state = ?, new_state_date = ?`, newState, timeNow()) if err != nil { return err } diff --git a/pkg/services/sqlstore/alert_test.go b/pkg/services/sqlstore/alert_test.go index 7b27f5b9ca4..361366578ab 100644 --- a/pkg/services/sqlstore/alert_test.go +++ b/pkg/services/sqlstore/alert_test.go @@ -6,9 +6,26 @@ import ( "github.com/grafana/grafana/pkg/components/simplejson" m "github.com/grafana/grafana/pkg/models" . "github.com/smartystreets/goconvey/convey" + "time" ) +func mockTimeNow() { + var timeSeed int64 + timeNow = func() time.Time { + fakeNow := time.Unix(timeSeed, 0) + timeSeed += 1 + return fakeNow + } +} + +func resetTimeNow() { + timeNow = time.Now +} + func TestAlertingDataAccess(t *testing.T) { + mockTimeNow() + defer resetTimeNow() + Convey("Testing Alerting data access", t, func() { InitTestDB(t) @@ -50,13 +67,12 @@ func TestAlertingDataAccess(t *testing.T) { So(err, ShouldBeNil) }) - Convey("can pause alert", func() { - cmd := &m.PauseAllAlertCommand{ - Paused: true, - } + alert, err := getAlertById(1) + So(err, ShouldBeNil) + stateDateBeforePause := alert.NewStateDate - err = PauseAllAlerts(cmd) - So(err, ShouldBeNil) + Convey("can pause all alerts", func() { + pauseAllAlerts(true) Convey("cannot updated paused alert", func() { cmd := &m.SetAlertStateCommand{ @@ -67,6 +83,19 @@ func TestAlertingDataAccess(t *testing.T) { err = SetAlertState(cmd) So(err, ShouldNotBeNil) }) + + Convey("pausing alerts should update their NewStateDate", func() { + alert, _ = getAlertById(1) + stateDateAfterPause := alert.NewStateDate + So(stateDateBeforePause, ShouldHappenBefore, stateDateAfterPause) + }) + + Convey("unpausing alerts should update their NewStateDate again", func() { + pauseAllAlerts(false) + alert, _ = getAlertById(1) + stateDateAfterUnpause := alert.NewStateDate + So(stateDateBeforePause, ShouldHappenBefore, stateDateAfterUnpause) + }) }) }) @@ -208,3 +237,91 @@ func TestAlertingDataAccess(t *testing.T) { }) }) } +func pauseAllAlerts(pauseState bool) error { + cmd := &m.PauseAllAlertCommand{ + Paused: pauseState, + } + err := PauseAllAlerts(cmd) + So(err, ShouldBeNil) + return err +} + +func TestPausingAlerts(t *testing.T) { + mockTimeNow() + defer resetTimeNow() + + Convey("Given an alert", t, func() { + InitTestDB(t) + + testDash := insertTestDashboard("dashboard with alerts", 1, 0, false, "alert") + alert, _ := insertTestAlert("Alerting title", "Alerting message", testDash.OrgId, testDash.Id, simplejson.New()) + + stateDateBeforePause := alert.NewStateDate + stateDateAfterPause := stateDateBeforePause + Convey("when paused", func() { + pauseAlert(testDash.OrgId, 1, true) + + Convey("the NewStateDate should be updated", func() { + alert, err := getAlertById(1) + So(err, ShouldBeNil) + + stateDateAfterPause = alert.NewStateDate + So(stateDateBeforePause, ShouldHappenBefore, stateDateAfterPause) + }) + }) + + Convey("when unpaused", func() { + pauseAlert(testDash.OrgId, 1, false) + + Convey("the NewStateDate should be updated again", func() { + alert, err := getAlertById(1) + So(err, ShouldBeNil) + + stateDateAfterUnpause := alert.NewStateDate + So(stateDateAfterPause, ShouldHappenBefore, stateDateAfterUnpause) + }) + }) + }) +} +func pauseAlert(orgId int64, alertId int64, pauseState bool) (int64, error) { + cmd := &m.PauseAlertCommand{ + OrgId: orgId, + AlertIds: []int64{alertId}, + Paused: pauseState, + } + err := PauseAlert(cmd) + So(err, ShouldBeNil) + return cmd.ResultCount, err +} +func insertTestAlert(title string, message string, orgId int64, dashId int64, settings *simplejson.Json) (*m.Alert, error) { + items := []*m.Alert{ + { + PanelId: 1, + DashboardId: dashId, + OrgId: orgId, + Name: title, + Message: message, + Settings: settings, + Frequency: 1, + }, + } + + cmd := m.SaveAlertsCommand{ + Alerts: items, + DashboardId: dashId, + OrgId: orgId, + UserId: 1, + } + + err := SaveAlerts(&cmd) + return cmd.Alerts[0], err +} + +func getAlertById(id int64) (*m.Alert, error) { + q := &m.GetAlertByIdQuery{ + Id: id, + } + err := GetAlertById(q) + So(err, ShouldBeNil) + return q.Result, err +} From 1f3442020b671cd50ce582373b2dbea392d80a83 Mon Sep 17 00:00:00 2001 From: Leonard Gram Date: Fri, 16 Feb 2018 14:31:03 +0100 Subject: [PATCH 2/2] alerts: refactoring tests --- pkg/services/sqlstore/alert_test.go | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/pkg/services/sqlstore/alert_test.go b/pkg/services/sqlstore/alert_test.go index 361366578ab..f3050256aa8 100644 --- a/pkg/services/sqlstore/alert_test.go +++ b/pkg/services/sqlstore/alert_test.go @@ -67,8 +67,7 @@ func TestAlertingDataAccess(t *testing.T) { So(err, ShouldBeNil) }) - alert, err := getAlertById(1) - So(err, ShouldBeNil) + alert, _ := getAlertById(1) stateDateBeforePause := alert.NewStateDate Convey("can pause all alerts", func() { @@ -237,14 +236,6 @@ func TestAlertingDataAccess(t *testing.T) { }) }) } -func pauseAllAlerts(pauseState bool) error { - cmd := &m.PauseAllAlertCommand{ - Paused: pauseState, - } - err := PauseAllAlerts(cmd) - So(err, ShouldBeNil) - return err -} func TestPausingAlerts(t *testing.T) { mockTimeNow() @@ -262,8 +253,7 @@ func TestPausingAlerts(t *testing.T) { pauseAlert(testDash.OrgId, 1, true) Convey("the NewStateDate should be updated", func() { - alert, err := getAlertById(1) - So(err, ShouldBeNil) + alert, _ := getAlertById(1) stateDateAfterPause = alert.NewStateDate So(stateDateBeforePause, ShouldHappenBefore, stateDateAfterPause) @@ -274,8 +264,7 @@ func TestPausingAlerts(t *testing.T) { pauseAlert(testDash.OrgId, 1, false) Convey("the NewStateDate should be updated again", func() { - alert, err := getAlertById(1) - So(err, ShouldBeNil) + alert, _ := getAlertById(1) stateDateAfterUnpause := alert.NewStateDate So(stateDateAfterPause, ShouldHappenBefore, stateDateAfterUnpause) @@ -325,3 +314,12 @@ func getAlertById(id int64) (*m.Alert, error) { So(err, ShouldBeNil) return q.Result, err } + +func pauseAllAlerts(pauseState bool) error { + cmd := &m.PauseAllAlertCommand{ + Paused: pauseState, + } + err := PauseAllAlerts(cmd) + So(err, ShouldBeNil) + return err +}