From 9f49895d9e837e7164e5d0f7f78f1a28efd229bb Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 13 Jun 2024 13:19:05 +1000 Subject: [PATCH] fix: evaluate if current day of week is valid correctly (#1763) 2 issues found - our cron implementation did not evaluate if the current time matched the day of the week properly - discovered in this PR: https://github.com/TBD54566975/ftl/pull/1733/files - this revealed that the new shorthand day of the week logic was doing the equivalent of `* * * * * x *` (every second of the chosen day) rather than `0 0 0 * * x *` (midnight of the chosen day) fyi, plan is to replace with an actual cron lib anyway... --- internal/cron/cron.go | 4 ++-- internal/cron/cron_test.go | 29 +++++++++++++---------------- internal/cron/pattern.go | 3 +++ 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/internal/cron/cron.go b/internal/cron/cron.go index c70a0847bd..9ce8340c30 100644 --- a/internal/cron/cron.go +++ b/internal/cron/cron.go @@ -197,10 +197,10 @@ func isCurrentValueAllowedForDayOfWeekStep(step Step, values componentValues, t } results := slices.Map(days, func(day int) bool { - if values[t] < start || values[t] > end { + if value < start || value > end { return false } - if (values[t]-start)%incr != 0 { + if (value-start)%incr != 0 { return false } return true diff --git a/internal/cron/cron_test.go b/internal/cron/cron_test.go index 0599d81d9d..bd794090a2 100644 --- a/internal/cron/cron_test.go +++ b/internal/cron/cron_test.go @@ -148,22 +148,19 @@ func TestNext(t *testing.T) { time.Date(2025, 6, 6, 0, 0, 0, 0, time.UTC), }, }}, - // TODO: These two are failing on the NextAfter with inclusive=true - /* - // Every wednesday - {"0 0 0 * * 3 *", [][]time.Time{ - { // 2024-06-09 is a Sunday - time.Date(2024, 6, 9, 0, 0, 0, 0, time.UTC), - time.Date(2024, 6, 12, 0, 0, 0, 0, time.UTC), - }, - }}, - {"Wednesday", [][]time.Time{ - { // 2024-06-09 is a Sunday - time.Date(2024, 6, 9, 0, 0, 0, 0, time.UTC), - time.Date(2024, 6, 12, 0, 0, 0, 0, time.UTC), - }, - }}, - */ + // Every wednesday + {"0 0 0 * * 3 *", [][]time.Time{ + { // 2024-06-09 is a Sunday + time.Date(2024, 6, 9, 0, 0, 0, 0, time.UTC), + time.Date(2024, 6, 12, 0, 0, 0, 0, time.UTC), + }, + }}, + {"Wed", [][]time.Time{ + { // 2024-06-09 is a Sunday + time.Date(2024, 6, 9, 0, 0, 0, 0, time.UTC), + time.Date(2024, 6, 12, 0, 0, 0, 0, time.UTC), + }, + }}, } { t.Run(fmt.Sprintf("CronSeries:%s", tt.str), func(t *testing.T) { pattern, err := Parse(tt.str) diff --git a/internal/cron/pattern.go b/internal/cron/pattern.go index ae834e691b..44a272a555 100644 --- a/internal/cron/pattern.go +++ b/internal/cron/pattern.go @@ -84,6 +84,9 @@ func (p Pattern) standardizedComponents() ([]Component, error) { } components := newComponentsFilled() + components[0] = newComponentWithValue(0) // seconds + components[1] = newComponentWithValue(0) // minutes + components[2] = newComponentWithValue(0) // hours components[5] = newComponentWithValue(dayOfWeekInt) return components, nil }