Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix bug with StartAt() in v0.5.0 #103

Merged
merged 3 commits into from
Jan 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion job.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down
29 changes: 15 additions & 14 deletions scheduler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -140,11 +140,7 @@ func (s *Scheduler) scheduleNextRun(job *Job) {
now := s.now()
lastRun := job.LastRun()

// job can be scheduled with .StartAt()
if job.neverRan() {
if !job.NextRun().IsZero() {
return // scheduled for future run and should skip scheduling
}
lastRun = now
}

Expand All @@ -156,28 +152,33 @@ func (s *Scheduler) scheduleNextRun(job *Job) {
}))
}

func (s *Scheduler) durationToNextRun(t time.Time, job *Job) time.Duration {
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
switch job.unit {
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()
}
Expand Down Expand Up @@ -293,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)
Expand Down Expand Up @@ -464,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
}
Expand All @@ -477,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
Expand Down
46 changes: 33 additions & 13 deletions scheduler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -403,19 +403,39 @@ 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) {
Expand Down