From 4ed0d16cdd726390d47744333db2c24869628f95 Mon Sep 17 00:00:00 2001 From: Julie Vogelman Date: Tue, 1 Feb 2022 21:56:43 -0800 Subject: [PATCH] PrevCronTime returns error --- common/cronutil.go | 13 +++++----- common/cronutil_test.go | 55 ++++++++++++++++++++++++----------------- sensors/listener.go | 6 ++++- 3 files changed, 45 insertions(+), 29 deletions(-) diff --git a/common/cronutil.go b/common/cronutil.go index 1f69f144f9..a46d8b032f 100644 --- a/common/cronutil.go +++ b/common/cronutil.go @@ -1,6 +1,7 @@ package common import ( + "fmt" "time" cronlib "github.com/robfig/cron/v3" @@ -13,15 +14,15 @@ const ( // For a given cron specification, return the previous activation time // If no time can be found to satisfy the schedule, return the zero time. -func PrevCronTime(cronSpec string, parser cronlib.Parser, t time.Time) time.Time { - +func PrevCronTime(cronSpec string, parser cronlib.Parser, t time.Time) (time.Time, error) { + var tm time.Time sched, err := parser.Parse(cronSpec) if err != nil { - // TBD + return tm, fmt.Errorf("can't derive previous Cron time for cron spec %s; couldn't parse; err=%v", cronSpec, err) } s, castOk := sched.(*cronlib.SpecSchedule) if !castOk { - // TBD + return tm, fmt.Errorf("can't derive previous Cron time for cron spec %s: unexpected type for %v", cronSpec, sched) } // General approach is based on approach to SpecSchedule.Next() implementation @@ -43,7 +44,7 @@ func PrevCronTime(cronSpec string, parser cronlib.Parser, t time.Time) time.Time WRAP: if t.Year() < yearLimit { - return time.Time{} + return tm, fmt.Errorf("can't derive previous Cron time for cron spec %s: no time found within %d years", cronSpec, yearLimit) } // Find the first applicable month. @@ -116,7 +117,7 @@ WRAP: } } - return t.In(origLocation) + return t.In(origLocation), nil } // dayMatches returns true if the schedule's day-of-week and day-of-month diff --git a/common/cronutil_test.go b/common/cronutil_test.go index 31d8a048d5..cf89912b5c 100644 --- a/common/cronutil_test.go +++ b/common/cronutil_test.go @@ -10,55 +10,66 @@ import ( func TestPrevCronTime(t *testing.T) { runs := []struct { - time, spec string - expected string + time, spec string + expected string + expectedErr bool }{ // Simple cases - {"Mon Jul 9 15:00 2012", "0 0/15 * * * *", "Mon Jul 9 14:45 2012"}, - {"Mon Jul 9 14:59 2012", "0 0/15 * * * *", "Mon Jul 9 14:45 2012"}, - {"Mon Jul 9 15:01:59 2012", "0 0/15 * * * *", "Mon Jul 9 15:00 2012"}, + {"Mon Jul 9 15:00 2012", "0 0/15 * * * *", "Mon Jul 9 14:45 2012", false}, + {"Mon Jul 9 14:59 2012", "0 0/15 * * * *", "Mon Jul 9 14:45 2012", false}, + {"Mon Jul 9 15:01:59 2012", "0 0/15 * * * *", "Mon Jul 9 15:00 2012", false}, // Wrap around hours - {"Mon Jul 9 15:10 2012", "0 20-35/15 * * * *", "Mon Jul 9 14:35 2012"}, + {"Mon Jul 9 15:10 2012", "0 20-35/15 * * * *", "Mon Jul 9 14:35 2012", false}, // Wrap around days - {"Tue Jul 10 00:00 2012", "0 */15 * * * *", "Tue Jul 9 23:45 2012"}, - {"Tue Jul 10 00:00 2012", "0 20-35/15 * * * *", "Tue Jul 9 23:35 2012"}, + {"Tue Jul 10 00:00 2012", "0 */15 * * * *", "Tue Jul 9 23:45 2012", false}, + {"Tue Jul 10 00:00 2012", "0 20-35/15 * * * *", "Tue Jul 9 23:35 2012", false}, // Wrap around months - {"Mon Jul 9 09:35 2012", "0 0 12 9 Apr-Oct ?", "Sat Jun 9 12:00 2012"}, + {"Mon Jul 9 09:35 2012", "0 0 12 9 Apr-Oct ?", "Sat Jun 9 12:00 2012", false}, // Leap year - {"Mon Jul 9 23:35 2018", "0 0 0 29 Feb ?", "Mon Feb 29 00:00 2016"}, + {"Mon Jul 9 23:35 2018", "0 0 0 29 Feb ?", "Mon Feb 29 00:00 2016", false}, // Daylight savings time 3am EDT (-4) -> 2am EST (-5) - {"2013-03-11T02:30:00-0400", "TZ=America/New_York 0 0 12 9 Mar ?", "2013-03-09T12:00:00-0500"}, + {"2013-03-11T02:30:00-0400", "TZ=America/New_York 0 0 12 9 Mar ?", "2013-03-09T12:00:00-0500", false}, // hourly job - {"2012-03-11T01:00:00-0500", "TZ=America/New_York 0 0 * * * ?", "2012-03-11T00:00:00-0500"}, + {"2012-03-11T01:00:00-0500", "TZ=America/New_York 0 0 * * * ?", "2012-03-11T00:00:00-0500", false}, // 2am nightly job (skipped) - {"2012-03-12T00:00:00-0400", "TZ=America/New_York 0 0 2 * * ?", "2012-03-10T02:00:00-0500"}, + {"2012-03-12T00:00:00-0400", "TZ=America/New_York 0 0 2 * * ?", "2012-03-10T02:00:00-0500", false}, // 2am nightly job - {"2012-11-04T02:00:00-0500", "TZ=America/New_York 0 0 0 * * ?", "2012-11-04T00:00:00-0400"}, - {"2012-11-05T02:00:00-0500", "TZ=America/New_York 0 0 2 * * ?", "2012-11-04T02:00:00-0500"}, + {"2012-11-04T02:00:00-0500", "TZ=America/New_York 0 0 0 * * ?", "2012-11-04T00:00:00-0400", false}, + {"2012-11-05T02:00:00-0500", "TZ=America/New_York 0 0 2 * * ?", "2012-11-04T02:00:00-0500", false}, // Unsatisfiable - {"Mon Jul 9 23:35 2012", "0 0 0 30 Feb ?", ""}, - {"Mon Jul 9 23:35 2012", "0 0 0 31 Apr ?", ""}, + {"Mon Jul 9 23:35 2012", "0 0 0 30 Feb ?", "", true}, + {"Mon Jul 9 23:35 2012", "0 0 0 31 Apr ?", "", true}, // Monthly job - {"TZ=America/New_York 2012-12-03T00:00:00-0500", "0 0 3 3 * ?", "2012-11-03T03:00:00-0400"}, + {"TZ=America/New_York 2012-12-03T00:00:00-0500", "0 0 3 3 * ?", "2012-11-03T03:00:00-0400", false}, } parser := cronlib.NewParser(cronlib.Second | cronlib.Minute | cronlib.Hour | cronlib.Dom | cronlib.Month | cronlib.DowOptional | cronlib.Descriptor) for _, c := range runs { - actual := PrevCronTime(c.spec, parser, getTime(c.time)) - expected := getTime(c.expected) - if !actual.Equal(expected) { - t.Errorf("%s, \"%s\": (expected) %v != %v (actual)", c.time, c.spec, expected, actual) + actual, err := PrevCronTime(c.spec, parser, getTime(c.time)) + if c.expectedErr { + if err == nil { + t.Errorf("%s, \"%s\": should have received error but didn't", c.time, c.spec) + } + } else { + if err != nil { + t.Errorf("%s, \"%s\": error: %v", c.time, c.spec, err) + } else { + expected := getTime(c.expected) + if !actual.Equal(expected) { + t.Errorf("%s, \"%s\": (expected) %v != %v (actual)", c.time, c.spec, expected, actual) + } + } } } } diff --git a/sensors/listener.go b/sensors/listener.go index 9bb0693e89..517af56187 100644 --- a/sensors/listener.go +++ b/sensors/listener.go @@ -257,7 +257,11 @@ func (sensorCtx *SensorContext) listenEvents(ctx context.Context) error { continue } prevTriggerTime := specSchedule.Prev(time.Now().UTC())*/ - prevTriggerTime := common.PrevCronTime(c.ByTime.Cron, cronParser, nowTime) + prevTriggerTime, err := common.PrevCronTime(c.ByTime.Cron, cronParser, nowTime) + if err != nil { + logger.Errorw("couldn't get previous cron trigger time", zap.Error(err)) + continue + } logger.Infof("previous trigger time: %v", prevTriggerTime) if prevTriggerTime.After(lastResetTime) { lastResetTime = prevTriggerTime