fix(notifier): fixed sending of notifications with schedule between d…

…ifferent days (#1069)
moira-alert · Aug 14, 2024 · 64f2f54 · 64f2f54
1 parent 6683a4a
commit 64f2f54
Show file tree

Hide file tree

Showing 2 changed files with 131 additions and 4 deletions.
diff --git a/notifier/scheduler.go b/notifier/scheduler.go
@@ -162,16 +162,25 @@ func calculateNextDelivery(schedule *moira.ScheduleData, nextTime time.Time) (ti
 	if len(schedule.Days) == 0 {
 		return nextTime, nil
 	}
+
 	beginOffset := time.Duration(schedule.StartOffset) * time.Minute
 	endOffset := time.Duration(schedule.EndOffset) * time.Minute
-	if schedule.EndOffset < schedule.StartOffset {
-		endOffset += time.Hour * 24
-	}
 
 	tzOffset := time.Duration(schedule.TimezoneOffset) * time.Minute
 	localNextTime := nextTime.Add(-tzOffset).Truncate(time.Minute)
 	localNextTimeDay := localNextTime.Truncate(24 * time.Hour) //nolint
 	localNextWeekday := int(localNextTimeDay.Weekday()+6) % 7  //nolint
+	timeOfDay := localNextTime.Sub(localNextTimeDay)
+
+	if schedule.EndOffset < schedule.StartOffset {
+		// The condition can only be fulfilled if the begin offset should be on the past day and not on the current day.
+		// In other variants end offset must be on the next day
+		if timeOfDay < beginOffset && timeOfDay < endOffset {
+			beginOffset -= time.Hour * 24
+		} else {
+			endOffset += time.Hour * 24
+		}
+	}
 
 	if schedule.Days[localNextWeekday].Enabled &&
 		(localNextTime.Equal(localNextTimeDay.Add(beginOffset)) || localNextTime.After(localNextTimeDay.Add(beginOffset))) &&
@@ -186,9 +195,11 @@ func calculateNextDelivery(schedule *moira.ScheduleData, nextTime time.Time) (ti
 		if localNextTime.After(nextLocalDayBegin.Add(beginOffset)) {
 			continue
 		}
+
 		if !schedule.Days[nextLocalWeekDay].Enabled {
 			continue
 		}
+
 		return nextLocalDayBegin.Add(beginOffset + tzOffset), nil
 	}
 

diff --git a/notifier/scheduler_test.go b/notifier/scheduler_test.go
@@ -256,7 +256,7 @@ func TestSubscriptionSchedule(t *testing.T) {
 		})
 	})
 
-	Convey("Test advanced schedule (e.g. 02:00 - 00:00)", t, func() {
+	Convey("Test advanced schedule during current day (e.g. 02:00 - 00:00)", t, func() {
 		// Schedule: 02:00 - 00:00 (GTM +3)
 		Convey("Time is out of range, nextTime should resemble now", func() {
 			// 2015-09-02, 14:00:00 GMT+03:00
@@ -342,6 +342,107 @@ func TestSubscriptionSchedule(t *testing.T) {
 			So(throttled, ShouldBeFalse)
 		})
 	})
+
+	Convey("Test advanced schedule between different days (e.g. 23:30 - 18:00)", t, func() {
+		// Schedule: 23:30 - 18:00 (GTM +3)
+		Convey("Time is out of range within the current day, nextTime should resemble now", func() {
+			// 2015-09-02, 23:45:00 GMT+03:00
+			now := time.Unix(1441140300, 0)
+			subscription.ThrottlingEnabled = false
+			subscription.Schedule = schedule4
+			dataBase.EXPECT().GetTriggerThrottling(event.TriggerID).Return(time.Unix(0, 0), time.Unix(0, 0))
+			dataBase.EXPECT().GetSubscription(*event.SubscriptionID).Return(subscription, nil)
+
+			next, throttled := scheduler.calculateNextDelivery(now, &event, logger)
+			// 2015-09-02, 23:45:00 GMT+03:00
+			So(next, ShouldResemble, now)
+			So(throttled, ShouldBeFalse)
+		})
+
+		Convey("Time is out of range on the next day, nextTime should resemble now", func() {
+			// 2015-09-02, 00:35:00 GMT+03:00
+			now := time.Unix(1441143300, 0)
+			subscription.ThrottlingEnabled = false
+			subscription.Schedule = schedule4
+			dataBase.EXPECT().GetTriggerThrottling(event.TriggerID).Return(time.Unix(0, 0), time.Unix(0, 0))
+			dataBase.EXPECT().GetSubscription(*event.SubscriptionID).Return(subscription, nil)
+
+			next, throttled := scheduler.calculateNextDelivery(now, &event, logger)
+			// 2015-09-02, 00:35:00 GMT+03:00
+			So(next, ShouldResemble, now)
+			So(throttled, ShouldBeFalse)
+		})
+
+		Convey("Time is in range, nextTime should resemble start of new period", func() {
+			// 2015-09-02, 20:00:00 GMT+03:00
+			now := time.Unix(1441213200, 0)
+			subscription.ThrottlingEnabled = false
+			subscription.Schedule = schedule4
+			dataBase.EXPECT().GetTriggerThrottling(event.TriggerID).Return(time.Unix(0, 0), time.Unix(0, 0))
+			dataBase.EXPECT().GetSubscription(*event.SubscriptionID).Return(subscription, nil)
+
+			next, throttled := scheduler.calculateNextDelivery(now, &event, logger)
+			// 2015-09-02, 23:30:00 GMT+03:00
+			So(next, ShouldResemble, time.Unix(1441225800, 0))
+			So(throttled, ShouldBeFalse)
+		})
+
+		Convey("Up border case, nextTime should resemble now", func() {
+			// 2015-09-02, 23:30:00 GMT+03:00
+			now := time.Unix(1441225800, 0)
+			subscription.ThrottlingEnabled = false
+			subscription.Schedule = schedule4
+			dataBase.EXPECT().GetTriggerThrottling(event.TriggerID).Return(time.Unix(0, 0), time.Unix(0, 0))
+			dataBase.EXPECT().GetSubscription(*event.SubscriptionID).Return(subscription, nil)
+
+			next, throttled := scheduler.calculateNextDelivery(now, &event, logger)
+			// 2015-09-02, 23:30:00 GMT+03:00
+			So(next, ShouldResemble, now)
+			So(throttled, ShouldBeFalse)
+		})
+
+		Convey("Low border case, nextTime should resemble start of new period", func() {
+			// 2015-09-02, 18:00:00 GMT+03:00
+			now := time.Unix(1441206000, 0)
+			subscription.ThrottlingEnabled = false
+			subscription.Schedule = schedule4
+			dataBase.EXPECT().GetTriggerThrottling(event.TriggerID).Return(time.Unix(0, 0), time.Unix(0, 0))
+			dataBase.EXPECT().GetSubscription(*event.SubscriptionID).Return(subscription, nil)
+
+			next, throttled := scheduler.calculateNextDelivery(now, &event, logger)
+			// 2015-09-02, 23:30:00 GMT+03:00
+			So(next, ShouldResemble, time.Unix(1441225800, 0))
+			So(throttled, ShouldBeFalse)
+		})
+
+		Convey("Low border case - 1 minute, nextTime should resemble now", func() {
+			// 2015-09-01, 17:59:00 GMT+03:00
+			now := time.Unix(1441205940, 0)
+			subscription.ThrottlingEnabled = false
+			subscription.Schedule = schedule4
+			dataBase.EXPECT().GetTriggerThrottling(event.TriggerID).Return(time.Unix(0, 0), time.Unix(0, 0))
+			dataBase.EXPECT().GetSubscription(*event.SubscriptionID).Return(subscription, nil)
+
+			next, throttled := scheduler.calculateNextDelivery(now, &event, logger)
+			// 2015-09-01, 17:59:00 GMT+03:00
+			So(next, ShouldResemble, now)
+			So(throttled, ShouldBeFalse)
+		})
+
+		Convey("Up border case - 1 minute, nextTime should resemble start of new period", func() {
+			// 2015-09-02, 23:29:00 GMT+03:00
+			now := time.Unix(1441225740, 0)
+			subscription.ThrottlingEnabled = false
+			subscription.Schedule = schedule4
+			dataBase.EXPECT().GetTriggerThrottling(event.TriggerID).Return(time.Unix(0, 0), time.Unix(0, 0))
+			dataBase.EXPECT().GetSubscription(*event.SubscriptionID).Return(subscription, nil)
+
+			next, throttled := scheduler.calculateNextDelivery(now, &event, logger)
+			// 2015-09-02, 23:30:00 GMT+03:00
+			So(next, ShouldResemble, time.Unix(1441225800, 0))
+			So(throttled, ShouldBeFalse)
+		})
+	})
 }
 
 var schedule1 = moira.ScheduleData{
@@ -388,3 +489,18 @@ var schedule3 = moira.ScheduleData{
 		{Enabled: true},
 	},
 }
+
+var schedule4 = moira.ScheduleData{
+	StartOffset:    1410, // 23:30
+	EndOffset:      1080, // 18:00
+	TimezoneOffset: -180, // (GMT +3)
+	Days: []moira.ScheduleDataDay{
+		{Enabled: true},
+		{Enabled: true},
+		{Enabled: true},
+		{Enabled: true},
+		{Enabled: true},
+		{Enabled: true},
+		{Enabled: true},
+	},
+}