Skip to content

Commit

Permalink
fix: atTime being incorrectly set for non compatible units (#502)
Browse files Browse the repository at this point in the history
* fix: atTime being incorrectly set for non compatible units

* fix race
  • Loading branch information
JohnRoesler committed May 30, 2023
1 parent b035fe3 commit 5bc0245
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 8 deletions.
4 changes: 4 additions & 0 deletions job.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,10 +252,14 @@ func (j *Job) addAtTime(t time.Duration) {
}

func (j *Job) getStartAtTime() time.Time {
j.mu.RLock()
defer j.mu.RUnlock()
return j.startAtTime
}

func (j *Job) setStartAtTime(t time.Time) {
j.mu.Lock()
defer j.mu.Unlock()
j.startAtTime = t
}

Expand Down
18 changes: 10 additions & 8 deletions scheduler.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,15 +205,15 @@ func (s *Scheduler) scheduleNextRun(job *Job) (bool, nextRun) {
// Increment startAtTime to the future
if !job.startAtTime.IsZero() && job.startAtTime.Before(now) {
duration := s.durationToNextRun(job.startAtTime, job).duration
job.startAtTime = job.startAtTime.Add(duration)
job.setStartAtTime(job.startAtTime.Add(duration))
if job.startAtTime.Before(now) {
diff := now.Sub(job.startAtTime)
duration := s.durationToNextRun(job.startAtTime, job).duration
count := diff / duration
if diff%duration != 0 {
count++
}
job.startAtTime = job.startAtTime.Add(duration * count)
job.setStartAtTime(job.startAtTime.Add(duration * count))
}
}
} else {
Expand Down Expand Up @@ -248,12 +248,14 @@ func (s *Scheduler) durationToNextRun(lastRun time.Time, job *Job) nextRun {
// job can be scheduled with .StartAt()
if job.getFirstAtTime() == 0 && job.getStartAtTime().After(lastRun) {
sa := job.getStartAtTime()
job.addAtTime(
time.Duration(sa.Hour())*time.Hour +
time.Duration(sa.Minute())*time.Minute +
time.Duration(sa.Second())*time.Second,
)
return nextRun{duration: job.getStartAtTime().Sub(s.now()), dateTime: job.getStartAtTime()}
if job.unit == days || job.unit == weeks || job.unit == months {
job.addAtTime(
time.Duration(sa.Hour())*time.Hour +
time.Duration(sa.Minute())*time.Minute +
time.Duration(sa.Second())*time.Second,
)
}
return nextRun{duration: sa.Sub(s.now()), dateTime: sa}
}

var next nextRun
Expand Down
14 changes: 14 additions & 0 deletions scheduler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1894,6 +1894,20 @@ func TestScheduler_Update(t *testing.T) {

assert.Equal(t, expectedNext, actualNext)
})

// Verifies https://github.com/go-co-op/gocron/issues/499
t.Run("at time is not set when updating incompatible unit jobs", func(t *testing.T) {
s := NewScheduler(time.UTC)
startAt := time.Now().UTC().Add(time.Millisecond * 250)
j, err := s.Every(24 * time.Hour).StartAt(startAt).Do(func() {})
require.NoError(t, err)
s.StartAsync()
time.Sleep(time.Millisecond * 500)

_, err = s.Job(j).StartAt(startAt.Add(1 * time.Second)).Update()
require.NoError(t, err)
s.Stop()
})
}

func TestScheduler_RunByTag(t *testing.T) {
Expand Down

0 comments on commit 5bc0245

Please sign in to comment.