Skip to content

Commit

Permalink
Fix a bug with scheduling next runs on daylight savings change days (#…
Browse files Browse the repository at this point in the history 
…435)

* Altering and renaming roundToMidnight to make it DST aware

* Udpate go.mod

* Try again

* Update unit tests, clean up implementation

* Remove dead function

* Use camelCase
  • Loading branch information
@andyy-gci
andyy-gci authored Mar 16, 2023
1 parent fb8a255 commit 708b009
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 26 deletions.
48 changes: 22 additions & 26 deletions scheduler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -239,8 +239,6 @@ func (s *Scheduler) durationToNextRun(lastRun time.Time, job *Job) nextRun {
}

func (s *Scheduler) calculateMonths(job *Job, lastRun time.Time) nextRun {
lastRunRoundedMidnight := s.roundToMidnight(lastRun)

// Special case: the last day of the month
if len(job.daysOfTheMonth) == 1 && job.daysOfTheMonth[0] == -1 {
return calculateNextRunForLastDayOfMonth(s, job, lastRun)
Expand All @@ -264,7 +262,7 @@ func (s *Scheduler) calculateMonths(job *Job, lastRun time.Time) nextRun {

return nextRunResult
}
next := lastRunRoundedMidnight.Add(job.getFirstAtTime()).AddDate(0, job.getInterval(), 0)
next := s.roundToMidnightAndAddDSTAware(lastRun, job.getFirstAtTime()).AddDate(0, job.getInterval(), 0)
return nextRun{duration: until(lastRun, next), dateTime: next}
}

Expand All @@ -275,7 +273,7 @@ func calculateNextRunForLastDayOfMonth(s *Scheduler, job *Job, lastRun time.Time
addMonth := job.getInterval()
atTime := job.getAtTime(lastRun)
if testDate := lastRun.AddDate(0, 0, 1); testDate.Month() != lastRun.Month() &&
!s.roundToMidnight(lastRun).Add(atTime).After(lastRun) {
!s.roundToMidnightAndAddDSTAware(lastRun, atTime).After(lastRun) {
// Our last run was on the last day of this month.
addMonth++
atTime = job.getFirstAtTime()
Expand All @@ -291,7 +289,10 @@ func calculateNextRunForLastDayOfMonth(s *Scheduler, job *Job, lastRun time.Time
func calculateNextRunForMonth(s *Scheduler, job *Job, lastRun time.Time, dayOfMonth int) nextRun {
atTime := job.getAtTime(lastRun)
natTime := atTime
jobDay := time.Date(lastRun.Year(), lastRun.Month(), dayOfMonth, 0, 0, 0, 0, s.Location()).Add(atTime)

hours, minutes, seconds := s.deconstructDuration(atTime)
jobDay := time.Date(lastRun.Year(), lastRun.Month(), dayOfMonth, hours, minutes, seconds, 0, s.Location())

difference := absDuration(lastRun.Sub(jobDay))
next := lastRun
if jobDay.Before(lastRun) { // shouldn't run this month; schedule for next interval minus day difference
Expand Down Expand Up @@ -320,13 +321,13 @@ func (s *Scheduler) calculateWeekday(job *Job, lastRun time.Time) nextRun {
if totalDaysDifference > 0 {
acTime = job.getFirstAtTime()
}
next := s.roundToMidnight(lastRun).Add(acTime).AddDate(0, 0, totalDaysDifference)
next := s.roundToMidnightAndAddDSTAware(lastRun, acTime).AddDate(0, 0, totalDaysDifference)
return nextRun{duration: until(lastRun, next), dateTime: next}
}

func (s *Scheduler) calculateWeeks(job *Job, lastRun time.Time) nextRun {
totalDaysDifference := int(job.getInterval()) * 7
next := s.roundToMidnight(lastRun).Add(job.getFirstAtTime()).AddDate(0, 0, totalDaysDifference)
next := s.roundToMidnightAndAddDSTAware(lastRun, job.getFirstAtTime()).AddDate(0, 0, totalDaysDifference)
return nextRun{duration: until(lastRun, next), dateTime: next}
}

Expand Down Expand Up @@ -354,14 +355,14 @@ func (s *Scheduler) calculateTotalDaysDifference(lastRun time.Time, daysToWeekda

func (s *Scheduler) calculateDays(job *Job, lastRun time.Time) nextRun {
if job.getInterval() == 1 {
lastRunDayPlusJobAtTime := s.roundToMidnight(lastRun).Add(job.getAtTime(lastRun))
lastRunDayPlusJobAtTime := s.roundToMidnightAndAddDSTAware(lastRun, job.getAtTime(lastRun))

if shouldRunToday(lastRun, lastRunDayPlusJobAtTime) {
return nextRun{duration: until(lastRun, lastRunDayPlusJobAtTime), dateTime: lastRunDayPlusJobAtTime}
}
}

nextRunAtTime := s.roundToMidnight(lastRun).Add(job.getFirstAtTime()).AddDate(0, 0, job.getInterval()).In(s.Location())
nextRunAtTime := s.roundToMidnightAndAddDSTAware(lastRun, job.getFirstAtTime()).AddDate(0, 0, job.getInterval()).In(s.Location())
return nextRun{duration: until(lastRun, nextRunAtTime), dateTime: nextRunAtTime}
}

Expand All @@ -386,14 +387,6 @@ func in(scheduleWeekdays []time.Weekday, weekday time.Weekday) bool {
}

func (s *Scheduler) calculateDuration(job *Job) time.Duration {
if job.neverRan() && shouldRunAtSpecificTime(job) { // ugly. in order to avoid this we could prohibit setting .At() and allowing only .StartAt() when dealing with Duration types
now := s.time.Now(s.location)
next := time.Date(now.Year(), now.Month(), now.Day(), 0, 0, 0, 0, s.Location()).Add(job.getFirstAtTime())
if now.Before(next) || now.Equal(next) {
return next.Sub(now)
}
}

interval := job.getInterval()
switch job.getUnit() {
case milliseconds:
Expand All @@ -407,11 +400,6 @@ func (s *Scheduler) calculateDuration(job *Job) time.Duration {
}
}

func shouldRunAtSpecificTime(job *Job) bool {
jobLastRun := job.LastRun()
return job.getAtTime(jobLastRun) != 0
}

func (s *Scheduler) remainingDaysToWeekday(lastRun time.Time, job *Job) int {
weekDays := job.Weekdays()
sort.Slice(weekDays, func(i, j int) bool {
Expand All @@ -429,7 +417,7 @@ func (s *Scheduler) remainingDaysToWeekday(lastRun time.Time, job *Job) int {
})
// check atTime
if equals {
if s.roundToMidnight(lastRun).Add(job.getAtTime(lastRun)).After(lastRun) {
if s.roundToMidnightAndAddDSTAware(lastRun, job.getAtTime(lastRun)).After(lastRun) {
return 0
}
index++
Expand All @@ -450,9 +438,17 @@ func absDuration(a time.Duration) time.Duration {
return -a
}

// roundToMidnight truncates time to midnight
func (s *Scheduler) roundToMidnight(t time.Time) time.Time {
return time.Date(t.Year(), t.Month(), t.Day(), 0, 0, 0, 0, s.Location())
func (s *Scheduler) deconstructDuration(d time.Duration) (hours int, minutes int, seconds int) {
hours = int(d.Seconds()) / int(time.Hour/time.Second)
minutes = (int(d.Seconds()) % int(time.Hour/time.Second)) / int(time.Minute/time.Second)
seconds = int(d.Seconds()) % int(time.Minute/time.Second)
return
}

// roundToMidnightAndAddDSTAware truncates time to midnight and "adds" duration in a DST aware manner
func (s *Scheduler) roundToMidnightAndAddDSTAware(t time.Time, d time.Duration) time.Time {
hours, minutes, seconds := s.deconstructDuration(d)
return time.Date(t.Year(), t.Month(), t.Day(), hours, minutes, seconds, 0, s.Location())
}

// NextRun datetime when the next Job should run.
Expand Down
65 changes: 65 additions & 0 deletions scheduler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,71 @@ func TestWeekdayAt(t *testing.T) {
})
}

func TestDaylightSavingsScheduled(t *testing.T) {
loc, err := time.LoadLocation("US/Pacific")
assert.NoError(t, err)

fromRFC3339 := func(ts string) time.Time {
dt, err := time.Parse(time.RFC3339, ts)
assert.NoError(t, err)
return dt.In(loc)
}

beforeToEDT := fakeTime{onNow: func(l *time.Location) time.Time {
return time.Date(2023, 3, 12, 1, 50, 50, 0, loc)
}}
toEDT := fakeTime{onNow: func(l *time.Location) time.Time {
return time.Date(2023, 3, 12, 5, 0, 0, 0, loc)
}}
beforeToEST := fakeTime{onNow: func(l *time.Location) time.Time {
return time.Date(2022, 11, 6, 1, 50, 50, 0, loc)
}}
afterToEST := fakeTime{onNow: func(l *time.Location) time.Time {
return fromRFC3339("2022-11-06T09:00:01.000Z")
}}
toEST := fakeTime{onNow: func(l *time.Location) time.Time {
return time.Date(2022, 11, 6, 4, 0, 0, 0, loc)
}}

testCases := []struct {
description string
ft fakeTime
jobCreateFn func(s *Scheduler) *Scheduler
expectedNextRun time.Time
}{
{"EST no change", beforeToEDT, func(s *Scheduler) *Scheduler { return s.Every(1).Day().At("01:59") }, time.Date(2023, 3, 12, 1, 59, 0, 0, loc)},
{"EST->EDT every day", toEDT, func(s *Scheduler) *Scheduler { return s.Every(1).Day().At("20:00") }, time.Date(2023, 3, 12, 20, 0, 0, 0, loc)},
{"EST->EDT every 2 week", toEDT, func(s *Scheduler) *Scheduler { return s.Every(2).Week().At("17:00") }, time.Date(2023, 3, 26, 17, 0, 0, 0, loc)},
{"EST->EDT every 2 Tuesday", toEDT, func(s *Scheduler) *Scheduler { return s.Every(2).Tuesday().At("16:00") }, time.Date(2023, 3, 14, 16, 0, 0, 0, loc)},
{"EST->EDT every Sunday", toEDT, func(s *Scheduler) *Scheduler { return s.Every(1).Sunday().At("04:30") }, time.Date(2023, 3, 19, 4, 30, 0, 0, loc)},
{"EST->EDT every month", toEDT, func(s *Scheduler) *Scheduler { return s.Every(3).Month(12).At("14:00") }, time.Date(2023, 6, 12, 14, 0, 0, 0, loc)},
{"EST->EDT every last day of month", toEDT, func(s *Scheduler) *Scheduler { return s.Every(1).MonthLastDay().At("13:00") }, time.Date(2023, 3, 31, 13, 0, 0, 0, loc)},

{"EDT no change", beforeToEST, func(s *Scheduler) *Scheduler { return s.Every(1).Day().At("01:59") }, time.Date(2022, 11, 6, 1, 59, 0, 0, loc)},
{"EDT->EST every day", toEST, func(s *Scheduler) *Scheduler { return s.Every(1).Day().At("20:00") }, time.Date(2022, 11, 6, 20, 0, 0, 0, loc)},
{"EDT->EST every 2 week", toEST, func(s *Scheduler) *Scheduler { return s.Every(2).Week().At("18:00") }, time.Date(2022, 11, 20, 18, 0, 0, 0, loc)},
{"EDT->EST every 2 Tuesday", toEST, func(s *Scheduler) *Scheduler { return s.Every(2).Tuesday().At("15:00") }, time.Date(2022, 11, 8, 15, 0, 0, 0, loc)},
{"EDT->EST every Sunday", afterToEST, func(s *Scheduler) *Scheduler { return s.Every(1).Sunday().At("01:30") }, time.Date(2022, 11, 13, 1, 30, 0, 0, loc)},
{"EDT->EST every month", toEST, func(s *Scheduler) *Scheduler { return s.Every(3).Month(6).At("14:30") }, time.Date(2023, 2, 6, 14, 30, 0, 0, loc)},
{"EDT->EST every last day of month", toEST, func(s *Scheduler) *Scheduler { return s.Every(1).MonthLastDay().At("13:15") }, time.Date(2022, 11, 30, 13, 15, 0, 0, loc)},
}

for _, tc := range testCases {
t.Run(tc.description, func(t *testing.T) {
s := NewScheduler(loc)
s.time = tc.ft

job, err := tc.jobCreateFn(s).Do(func() {})
assert.NoError(t, err)

s.setRunning(true)
s.scheduleNextRun(job)

assert.Equal(t, tc.expectedNextRun, job.NextRun())
})
}
}

func TestScheduler_Remove(t *testing.T) {
t.Run("remove from non-running", func(t *testing.T) {
s := NewScheduler(time.UTC)
Expand Down

0 comments on commit 708b009

Please sign in to comment.