fix: trigger schedule fill (#1116)

api/dto/triggers.go

@@ -7,6 +7,7 @@ import (
 	"net/http"
 	"regexp"
 	"strconv"
+	"strings"
 	"time"
 	"unicode/utf8"
 
@@ -40,6 +41,9 @@ var (
 
 	// errAsteriskPatternNotAllowed is returned then one of Trigger.Patterns contain only "*".
 	errAsteriskPatternNotAllowed = errors.New("pattern \"*\" is not allowed to use")
+
+	// errNoAllowedDays is returned then all days disabled in moira.ScheduleData.
+	errNoAllowedDays = errors.New("no allowed days in trigger schedule")
 )
 
 // TODO(litleleprikon): Remove after https://github.com/moira-alert/moira/issues/550 will be resolved.
@@ -257,6 +261,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 +289,50 @@ 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) {
+	newSchedule := moira.NewDefaultScheduleData()
+
+	scheduleDaysMap := make(map[moira.DayName]bool, len(newSchedule.Days))
+	for _, day := range newSchedule.Days {
+		scheduleDaysMap[day.Name] = false
+	}
+
+	badDayNames := make([]string, 0)
+	for _, day := range gotSchedule.Days {
+		_, validDayName := scheduleDaysMap[day.Name]
+		if validDayName {
+			scheduleDaysMap[day.Name] = day.Enabled
+		} else {
+			badDayNames = append(badDayNames, string(day.Name))
+		}
+	}
+
+	if len(badDayNames) != 0 {
+		return nil, fmt.Errorf("bad day names in schedule: %s", strings.Join(badDayNames, ", "))
+	}
+
+	someDayEnabled := false
+	for i := range newSchedule.Days {
+		newSchedule.Days[i].Enabled = scheduleDaysMap[newSchedule.Days[i].Name]
+
+		if newSchedule.Days[i].Enabled {
+			someDayEnabled = true
+		}
+	}
+
+	if !someDayEnabled {
+		return nil, errNoAllowedDays
+	}
+
+	newSchedule.TimezoneOffset = gotSchedule.TimezoneOffset
+	newSchedule.StartOffset = gotSchedule.StartOffset
+	newSchedule.EndOffset = gotSchedule.EndOffset
+
+	return newSchedule, nil
+}
+
 func checkTTLSanity(trigger *Trigger, metricsSource metricSource.MetricSource) error {
 	maximumAllowedTTL := metricsSource.GetMetricsTTLSeconds()
 

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,197 @@ 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 valid schedule", func() {
+			givenSchedule := moira.NewDefaultScheduleData()
+
+			givenSchedule.Days[len(givenSchedule.Days)-1].Enabled = false
+			givenSchedule.TimezoneOffset += 1
+			givenSchedule.StartOffset += 1
+			givenSchedule.EndOffset += 1
+
+			gotSchedule, err := checkScheduleFilling(givenSchedule)
+
+			So(err, ShouldBeNil)
+			So(gotSchedule, ShouldResemble, givenSchedule)
+		})
+
+		Convey("With not all days, missing days filled with false", func() {
+			days := moira.GetFilledScheduleDataDays(true)
+
+			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("With some days repeated, there is no repeated days and missing days filled with false", func() {
+			days := moira.GetFilledScheduleDataDays(true)
+
+			days[4].Name = moira.Monday
+			days[6].Name = moira.Monday
+
+			givenSchedule := &moira.ScheduleData{
+				Days:           days,
+				TimezoneOffset: defaultSchedule.TimezoneOffset,
+				StartOffset:    defaultSchedule.StartOffset,
+				EndOffset:      defaultSchedule.EndOffset,
+			}
+
+			expectedDays := moira.GetFilledScheduleDataDays(true)
+
+			expectedDays[4].Enabled = false
+			expectedDays[6].Enabled = false
+
+			expectedSchedule := &moira.ScheduleData{
+				Days:           expectedDays,
+				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 := moira.GetFilledScheduleDataDays(true)
+
+			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 := moira.GetFilledScheduleDataDays(true)
+
+			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 := moira.GetFilledScheduleDataDays(true)
+
+			var (
+				badMondayName moira.DayName = "Monday"
+				badFridayName moira.DayName = "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, fmt.Errorf("bad day names in schedule: %s, %s", badMondayName, badFridayName))
+			So(gotSchedule, ShouldBeNil)
+		})
+
+		Convey("With no enabled days error returned", func() {
+			days := moira.GetFilledScheduleDataDays(false)
+
+			givenSchedule := &moira.ScheduleData{
+				Days:           days,
+				TimezoneOffset: defaultSchedule.TimezoneOffset,
+				StartOffset:    defaultSchedule.StartOffset,
+				EndOffset:      defaultSchedule.EndOffset,
+			}
+
+			gotSchedule, err := checkScheduleFilling(givenSchedule)
+
+			So(err, ShouldResemble, errNoAllowedDays)
+			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
+}

api/handler/triggers_test.go

@@ -72,17 +72,24 @@ func TestGetTriggerFromRequest(t *testing.T) {
 		ttlState := moira.TTLState("NODATA")
 		triggerDTO := dto.Trigger{
 			TriggerModel: dto.TriggerModel{
-				ID:             "test_id",
-				Name:           "Test trigger",
-				Desc:           new(string),
-				Targets:        []string{"foo.bar"},
-				WarnValue:      &triggerWarnValue,
-				ErrorValue:     &triggerErrorValue,
-				TriggerType:    "rising",
-				Tags:           []string{"Normal", "DevOps", "DevOpsGraphite-duty"},
-				TTLState:       &ttlState,
-				TTL:            0,
-				Schedule:       &moira.ScheduleData{},
+				ID:          "test_id",
+				Name:        "Test trigger",
+				Desc:        new(string),
+				Targets:     []string{"foo.bar"},
+				WarnValue:   &triggerWarnValue,
+				ErrorValue:  &triggerErrorValue,
+				TriggerType: "rising",
+				Tags:        []string{"Normal", "DevOps", "DevOpsGraphite-duty"},
+				TTLState:    &ttlState,
+				TTL:         0,
+				Schedule: &moira.ScheduleData{
+					Days: []moira.ScheduleDataDay{
+						{
+							Name:    "Mon",
+							Enabled: true,
+						},
+					},
+				},
 				Expression:     "",
 				Patterns:       []string{},
 				TriggerSource:  moira.GraphiteLocal,
@@ -102,6 +109,9 @@ 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.Days = moira.GetFilledScheduleDataDays(false)
+		triggerDTO.Schedule.Days[0].Enabled = true
+
 		Convey("It should be parsed successfully", func() {
 			triggerDTO.TTL = moira.DefaultTTL