fix: schedule validation

moira-alert · Oct 29, 2024 · f260674 · f260674
1 parent ae27338
commit f260674
Show file tree

Hide file tree

Showing 3 changed files with 192 additions and 0 deletions.
diff --git a/api/dto/triggers.go b/api/dto/triggers.go
@@ -7,6 +7,7 @@ import (
 	"net/http"
 	"regexp"
 	"strconv"
+	"strings"
 	"time"
 	"unicode/utf8"
 
@@ -257,6 +258,13 @@ func (trigger *Trigger) Bind(request *http.Request) error {
 
 	if trigger.Schedule == nil {
 		trigger.Schedule = moira.NewDefaultScheduleData()
+	} else {
+		correctedSchedule, err := checkScheduleFilling(trigger.Schedule)
+		if err != nil {
+			return api.ErrInvalidRequestContent{ValidationError: err}
+		}
+
+		trigger.Schedule = correctedSchedule
 	}
 
 	middleware.SetTimeSeriesNames(request, metricsDataNames)
@@ -278,6 +286,47 @@ func getDateTime(timestamp *int64) *time.Time {
 	return &datetime
 }
 
+// checkScheduleFilling ensures that all days are included to schedule, ordered from monday to sunday
+// and have proper names (one of [Mon, Tue, Wed, Thu, Fri, Sat Sun]).
+func checkScheduleFilling(gotSchedule *moira.ScheduleData) (*moira.ScheduleData, error) {
+	defaultSchedule := moira.NewDefaultScheduleData()
+
+	scheduleDaysMap := make(map[string]bool, len(defaultSchedule.Days))
+	for _, day := range defaultSchedule.Days {
+		scheduleDaysMap[day.Name] = false
+	}
+
+	badDayNames := make([]string, 0)
+	for _, day := range gotSchedule.Days {
+		_, validDay := scheduleDaysMap[day.Name]
+		if validDay && day.Enabled {
+			scheduleDaysMap[day.Name] = true
+		} else if !validDay {
+			badDayNames = append(badDayNames, day.Name)
+		}
+	}
+
+	if len(badDayNames) != 0 {
+		return nil, fmt.Errorf("bad day names in schedule: %s", strings.Join(badDayNames, ", "))
+	}
+
+	newDaysForSchedule := make([]moira.ScheduleDataDay, 0, len(scheduleDaysMap))
+	for _, day := range defaultSchedule.Days {
+		newDaysForSchedule = append(newDaysForSchedule,
+			moira.ScheduleDataDay{
+				Name:    day.Name,
+				Enabled: scheduleDaysMap[day.Name],
+			})
+	}
+
+	return &moira.ScheduleData{
+		Days:           newDaysForSchedule,
+		TimezoneOffset: gotSchedule.TimezoneOffset,
+		StartOffset:    gotSchedule.StartOffset,
+		EndOffset:      gotSchedule.EndOffset,
+	}, nil
+}
+
 func checkTTLSanity(trigger *Trigger, metricsSource metricSource.MetricSource) error {
 	maximumAllowedTTL := metricsSource.GetMetricsTTLSeconds()
 

diff --git a/api/dto/triggers_test.go b/api/dto/triggers_test.go
@@ -3,7 +3,9 @@ package dto
 import (
 	"context"
 	"fmt"
+	"math/rand"
 	"net/http"
+	"slices"
 	"strings"
 	"testing"
 	"time"
@@ -442,3 +444,136 @@ func TestCreateTriggerModel(t *testing.T) {
 		So(CreateTriggerModel(trigger), ShouldResemble, expTriggerModel)
 	})
 }
+
+func Test_checkScheduleFilling(t *testing.T) {
+	Convey("Testing checking schedule filling", t, func() {
+		defaultSchedule := moira.NewDefaultScheduleData()
+
+		Convey("With not all days missing days filled with false", func() {
+			days := slices.Clone(defaultSchedule.Days)
+
+			givenSchedule := &moira.ScheduleData{
+				Days:           days[:len(days)-1],
+				TimezoneOffset: defaultSchedule.TimezoneOffset,
+				StartOffset:    defaultSchedule.StartOffset,
+				EndOffset:      defaultSchedule.EndOffset,
+			}
+
+			days[len(days)-1].Enabled = false
+
+			expectedSchedule := &moira.ScheduleData{
+				Days:           days,
+				TimezoneOffset: defaultSchedule.TimezoneOffset,
+				StartOffset:    defaultSchedule.StartOffset,
+				EndOffset:      defaultSchedule.EndOffset,
+			}
+
+			gotSchedule, err := checkScheduleFilling(givenSchedule)
+
+			So(err, ShouldBeNil)
+			So(gotSchedule, ShouldResemble, expectedSchedule)
+		})
+
+		Convey("When days shuffled return ordered", func() {
+			days := slices.Clone(defaultSchedule.Days)
+
+			shuffledDays := shuffleArray(days)
+
+			givenSchedule := &moira.ScheduleData{
+				Days:           shuffledDays,
+				TimezoneOffset: defaultSchedule.TimezoneOffset,
+				StartOffset:    defaultSchedule.StartOffset,
+				EndOffset:      defaultSchedule.EndOffset,
+			}
+
+			expectedSchedule := &moira.ScheduleData{
+				Days:           defaultSchedule.Days,
+				TimezoneOffset: defaultSchedule.TimezoneOffset,
+				StartOffset:    defaultSchedule.StartOffset,
+				EndOffset:      defaultSchedule.EndOffset,
+			}
+
+			gotSchedule, err := checkScheduleFilling(givenSchedule)
+
+			So(err, ShouldBeNil)
+			So(gotSchedule, ShouldResemble, expectedSchedule)
+		})
+
+		Convey("When days shuffled and some are missed return ordered and filled missing", func() {
+			days := slices.Clone(defaultSchedule.Days)
+
+			shuffledDays := shuffleArray(days[:len(days)-2])
+
+			days[len(days)-1].Enabled = false
+			days[len(days)-2].Enabled = false
+
+			givenSchedule := &moira.ScheduleData{
+				Days:           shuffledDays,
+				TimezoneOffset: defaultSchedule.TimezoneOffset,
+				StartOffset:    defaultSchedule.StartOffset,
+				EndOffset:      defaultSchedule.EndOffset,
+			}
+
+			expectedSchedule := &moira.ScheduleData{
+				Days:           days,
+				TimezoneOffset: defaultSchedule.TimezoneOffset,
+				StartOffset:    defaultSchedule.StartOffset,
+				EndOffset:      defaultSchedule.EndOffset,
+			}
+
+			gotSchedule, err := checkScheduleFilling(givenSchedule)
+
+			So(err, ShouldBeNil)
+			So(gotSchedule, ShouldResemble, expectedSchedule)
+		})
+
+		Convey("With bad day names error returned", func() {
+			days := slices.Clone(defaultSchedule.Days)
+
+			badMondayName := "Monday"
+			badFridayName := "Friday"
+
+			days[0].Name = badMondayName
+			days[4].Name = badFridayName
+
+			givenSchedule := &moira.ScheduleData{
+				Days:           days,
+				TimezoneOffset: defaultSchedule.TimezoneOffset,
+				StartOffset:    defaultSchedule.StartOffset,
+				EndOffset:      defaultSchedule.EndOffset,
+			}
+
+			gotSchedule, err := checkScheduleFilling(givenSchedule)
+
+			So(err, ShouldResemble, api.ErrInvalidRequestContent{
+				ValidationError: fmt.Errorf("bad day names in schedule: %s, %s", badMondayName, badFridayName),
+			})
+			So(gotSchedule, ShouldBeNil)
+		})
+	})
+}
+
+func shuffleArray[S interface{ ~[]E }, E any](slice S) S {
+	slice = slices.Clone(slice)
+	shuffledSlice := make(S, 0, len(slice))
+
+	for len(slice) > 0 {
+		randomIdx := rand.Intn(len(slice))
+		shuffledSlice = append(shuffledSlice, slice[randomIdx])
+
+		switch {
+		case randomIdx == len(slice)-1:
+			slice = slice[:len(slice)-1]
+		case randomIdx == 0:
+			if len(slice) > 1 {
+				slice = slice[1:]
+			} else {
+				slice = nil
+			}
+		default:
+			slice = append(slice[:randomIdx], slice[randomIdx+1:]...)
+		}
+	}
+
+	return shuffledSlice
+}
diff --git a/api/handler/triggers_test.go b/api/handler/triggers_test.go
@@ -102,6 +102,14 @@ func TestGetTriggerFromRequest(t *testing.T) {
 		request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "metricSourceProvider", sourceProvider))
 		request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "limits", api.GetTestLimitsConfig()))
 
+		triggerDTO.Schedule = moira.NewDefaultScheduleData()
+		triggerDTO.Schedule.TimezoneOffset = 0
+		triggerDTO.Schedule.StartOffset = 0
+		triggerDTO.Schedule.EndOffset = 0
+		for i := range triggerDTO.Schedule.Days {
+			triggerDTO.Schedule.Days[i].Enabled = false
+		}
+
 		Convey("It should be parsed successfully", func() {
 			triggerDTO.TTL = moira.DefaultTTL