From 110ba56668784acb531a637f280c4ba28ed9fa68 Mon Sep 17 00:00:00 2001 From: John Roesler Date: Fri, 15 Jan 2021 09:25:23 -0600 Subject: [PATCH 1/3] fix bug with StartAt() in v0.5.0 --- scheduler.go | 8 +++++--- scheduler_test.go | 45 ++++++++++++++++++++++++++++++++------------- 2 files changed, 37 insertions(+), 16 deletions(-) diff --git a/scheduler.go b/scheduler.go index 91d6e2ba..6c825272 100644 --- a/scheduler.go +++ b/scheduler.go @@ -142,9 +142,6 @@ func (s *Scheduler) scheduleNextRun(job *Job) { // job can be scheduled with .StartAt() if job.neverRan() { - if !job.NextRun().IsZero() { - return // scheduled for future run and should skip scheduling - } lastRun = now } @@ -157,6 +154,11 @@ func (s *Scheduler) scheduleNextRun(job *Job) { } func (s *Scheduler) durationToNextRun(t time.Time, job *Job) time.Duration { + // job can be scheduled with .StartAt() and already has a next run + if job.nextRun.After(s.time.Now(s.Location())) { + return job.nextRun.Sub(s.time.Now(s.Location())) + } + var duration time.Duration switch job.unit { case seconds, minutes, hours: diff --git a/scheduler_test.go b/scheduler_test.go index c4766b3f..87217ab3 100644 --- a/scheduler_test.go +++ b/scheduler_test.go @@ -403,19 +403,38 @@ func TestScheduler_Stop(t *testing.T) { } func TestScheduler_StartAt(t *testing.T) { - scheduler := NewScheduler(time.Local) - now := time.Now() - - // With StartAt - job, _ := scheduler.Every(3).Seconds().StartAt(now.Add(time.Second * 5)).Do(func() {}) - assert.False(t, job.getStartsImmediately()) - scheduler.start() - assert.Equal(t, now.Add(time.Second*5), job.NextRun()) - scheduler.stop() - - // Without StartAt - job, _ = scheduler.Every(3).Seconds().Do(func() {}) - assert.True(t, job.getStartsImmediately()) + t.Run("scheduling", func(t *testing.T) { + s := NewScheduler(time.Local) + now := time.Now() + + // With StartAt + job, _ := s.Every(3).Seconds().StartAt(now.Add(time.Second * 5)).Do(func() {}) + assert.False(t, job.getStartsImmediately()) + s.start() + assert.Equal(t, now.Add(time.Second*5).Truncate(time.Second), job.NextRun().Truncate(time.Second)) + s.stop() + + // Without StartAt + job, _ = s.Every(3).Seconds().Do(func() {}) + assert.True(t, job.getStartsImmediately()) + }) + + t.Run("run", func(t *testing.T) { + s := NewScheduler(time.UTC) + semaphore := make(chan bool) + + s.Every(1).Day().StartAt(s.time.Now(s.location).Add(time.Second)).Do(func() { + semaphore <- true }) + + s.StartAsync() + + select { + case <-time.After(2 * time.Second): + t.Fatal("job did not run at 1 second") + case <-semaphore: + // test passed + } + }) } func TestScheduler_CalculateNextRun(t *testing.T) { From b26e08c71d357b0b57a1da41811ae76e3e453762 Mon Sep 17 00:00:00 2001 From: John Roesler Date: Fri, 15 Jan 2021 09:30:04 -0600 Subject: [PATCH 2/3] gofmt --- scheduler_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scheduler_test.go b/scheduler_test.go index 87217ab3..890a0081 100644 --- a/scheduler_test.go +++ b/scheduler_test.go @@ -424,7 +424,8 @@ func TestScheduler_StartAt(t *testing.T) { semaphore := make(chan bool) s.Every(1).Day().StartAt(s.time.Now(s.location).Add(time.Second)).Do(func() { - semaphore <- true }) + semaphore <- true + }) s.StartAsync() From eddba148ffc30f29c31561e872aadd42859eaa75 Mon Sep 17 00:00:00 2001 From: John Roesler Date: Fri, 15 Jan 2021 10:15:28 -0600 Subject: [PATCH 3/3] add startAtTime on job struct --- job.go | 16 +++++++++++++++- scheduler.go | 27 +++++++++++++-------------- 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/job.go b/job.go index 1c128fe8..162e52d6 100644 --- a/job.go +++ b/job.go @@ -15,7 +15,8 @@ type Job struct { unit timeUnit // time units, ,e.g. 'minutes', 'hours'... startsImmediately bool // if the Job should run upon scheduler start jobFunc string // the Job jobFunc to run, func[jobFunc] - atTime time.Duration // optional time at which this Job runs + atTime time.Duration // optional time at which this Job runs when interval is day + startAtTime time.Time // optional time at which the Job starts err error // error related to Job lastRun time.Time // datetime of last run nextRun time.Time // datetime of next run @@ -98,6 +99,18 @@ func (j *Job) setAtTime(t time.Duration) { j.atTime = t } +func (j *Job) getStartAtTime() time.Time { + j.RLock() + defer j.RUnlock() + return j.startAtTime +} + +func (j *Job) setStartAtTime(t time.Time) { + j.Lock() + defer j.Unlock() + j.startAtTime = t +} + // Err returns an error if one occurred while creating the Job func (j *Job) Err() error { j.RLock() @@ -241,6 +254,7 @@ func (j *Job) getMaxRuns() int { return j.runConfig.maxRuns } +// TODO: this method seems unnecessary as we could always remove after the run count has expired. Maybe remove this in the future? func (j *Job) getRemoveAfterLastRun() bool { j.RLock() defer j.RUnlock() diff --git a/scheduler.go b/scheduler.go index 6c825272..d7fd86f9 100644 --- a/scheduler.go +++ b/scheduler.go @@ -68,7 +68,7 @@ func (s *Scheduler) runJobs(jobs []*Job) { j.setStartsImmediately(false) } if !j.shouldRun() { - if j.getRemoveAfterLastRun() { // TODO: this method seems unnecessary as we could always remove after the run cout has expired. Maybe remove this in the future? + if j.getRemoveAfterLastRun() { s.RemoveByReference(j) } continue @@ -140,7 +140,6 @@ func (s *Scheduler) scheduleNextRun(job *Job) { now := s.now() lastRun := job.LastRun() - // job can be scheduled with .StartAt() if job.neverRan() { lastRun = now } @@ -153,10 +152,10 @@ func (s *Scheduler) scheduleNextRun(job *Job) { })) } -func (s *Scheduler) durationToNextRun(t time.Time, job *Job) time.Duration { - // job can be scheduled with .StartAt() and already has a next run - if job.nextRun.After(s.time.Now(s.Location())) { - return job.nextRun.Sub(s.time.Now(s.Location())) +func (s *Scheduler) durationToNextRun(lastRun time.Time, job *Job) time.Duration { + // job can be scheduled with .StartAt() + if job.getStartAtTime().After(lastRun) { + return job.getStartAtTime().Sub(s.now()) } var duration time.Duration @@ -164,22 +163,22 @@ func (s *Scheduler) durationToNextRun(t time.Time, job *Job) time.Duration { case seconds, minutes, hours: duration = s.calculateDuration(job) case days: - duration = s.calculateDays(job, t) + duration = s.calculateDays(job, lastRun) case weeks: if job.scheduledWeekday != nil { // weekday selected, Every().Monday(), for example - duration = s.calculateWeekday(job, t) + duration = s.calculateWeekday(job, lastRun) } else { - duration = s.calculateWeeks(job, t) + duration = s.calculateWeeks(job, lastRun) } case months: - duration = s.calculateMonths(job, t) + duration = s.calculateMonths(job, lastRun) } return duration } func (s *Scheduler) getJobLastRun(job *Job) time.Time { if job.neverRan() { - return s.time.Now(s.Location()) + return s.now() } return job.LastRun() } @@ -295,7 +294,7 @@ func (s *Scheduler) roundToMidnight(t time.Time) time.Time { // NextRun datetime when the next Job should run. func (s *Scheduler) NextRun() (*Job, time.Time) { if len(s.Jobs()) <= 0 { - return nil, s.time.Now(s.Location()) + return nil, s.now() } sort.Sort(s) @@ -466,7 +465,7 @@ func (s *Scheduler) SetTag(t []string) *Scheduler { // StartAt schedules the next run of the Job func (s *Scheduler) StartAt(t time.Time) *Scheduler { job := s.getCurrentJob() - job.setNextRun(t) + job.setStartAtTime(t) job.startsImmediately = false return s } @@ -479,7 +478,7 @@ func (s *Scheduler) shouldRun(j *Job) bool { s.RemoveByReference(j) } - return j.shouldRun() && s.time.Now(s.Location()).Unix() >= j.NextRun().Unix() + return j.shouldRun() && s.now().Unix() >= j.NextRun().Unix() } // setUnit sets the unit type