Repository: go-co-op/gocron
Pull request #164 (2021-04-26 18:37:02)

Multiple days week
### What does this do?
This PR aims to add a list of weekdays instead of only one day per week in a weekly schedule. Basically, now a `Job` has a list of weekdays where the schedule can pass through and see which day is the next day in the schedule, given the job days.

### Which issue(s) does this PR fix/relate to?
Solves #127 


### List any changes that modify/break current functionality


### Have you included tests for your changes?
Yes, I have added 3 new tests and modified one (if there is any problem, we can check this out)

### Did you document any new/modified functionality?
I've added some comments and doc changing

- [x] Updated `example_test.go`
- [ ] Updated `README.md`

### Notes


Streppel: Great stuff indeed, thanks for your contribution @joaolcaas! Left some minor comments as well, let me know what you think.
joaolcaas: > Great stuff indeed, thanks for your contribution @joaolcaas! Left some minor comments as well, let me know what you think.
> 
> One final food for thought: what does `s.Every(2).Weekday(thursday, friday)` (sic) means? I'd say "every 2 weeks, at Thursday and Friday", so after a friday run it'd schedule the job for next Thursday in 2 weeks (then for the next day, friday). Do you guys agree?

I think that's right, I checked the current behavior and this is almost what you said, with the difference that the schedule starts the next week when the interval is `2`. Then it back to the week jump

Streppel: @joaolcaas indeed if we're to "fix" this it is out of scope of this PR. WDYT @JohnRoesler?
JohnRoesler: > @joaolcaas indeed if we're to "fix" this it is out of scope of this PR. WDYT @JohnRoesler?

Agreed. We should go ahead and open an issue though so we know it needs addressing. 

Patch:
diff --git a/job.go b/job.go
index 06bd147..98cd924 100644
--- a/job.go
+++ b/job.go
@@ -23,7 +23,7 @@ type Job struct {
 	error             error          // error related to Job
 	lastRun           time.Time      // datetime of last run
 	nextRun           time.Time      // datetime of next run
-	scheduledWeekday  *time.Weekday  // Specific day of the week to start on
+	scheduledWeekday  []time.Weekday // Specific days of the week to start on
 	dayOfTheMonth     int            // Specific day of the month to run the job
 	tags              []string       // allow the user to tag Jobs with certain labels
 	runCount          int            // number of times the job ran
@@ -178,12 +178,25 @@ func (j *Job) ScheduledAtTime() string {
 // Weekday returns which day of the week the Job will run on and
 // will return an error if the Job is not scheduled weekly
 func (j *Job) Weekday() (time.Weekday, error) {
-	if j.scheduledWeekday == nil {
+	if len(j.scheduledWeekday) == 0 {
 		return time.Sunday, ErrNotScheduledWeekday
 	}
-	return *j.scheduledWeekday, nil
+	return j.scheduledWeekday[0], nil
 }
 
+// Weekdays returns a slice of time.Weekday that the Job will run in a week and
+// will return an error if the Job is not scheduled weekly
+func (j *Job) Weekdays() []time.Weekday {
+	// appending on j.scheduledWeekday may cause a side effect
+	if len(j.scheduledWeekday) == 0 {
+		return []time.Weekday{time.Sunday}
+	}
+
+	return j.scheduledWeekday
+}
+
+// 	return j.scheduledWeekday
+// }
 // LimitRunsTo limits the number of executions of this job to n.
 // The job will remain in the scheduler.
 // Note: If a job is added to a running scheduler and this method is then used
diff --git a/scheduler.go b/scheduler.go
index 668aae2..1e68e5b 100644
--- a/scheduler.go
+++ b/scheduler.go
@@ -147,7 +147,6 @@ func (s *Scheduler) Location() *time.Location {
 func (s *Scheduler) scheduleNextRun(job *Job) {
 	now := s.now()
 	lastRun := job.LastRun()
-
 	if !s.jobPresent(job) {
 		return
 	}
@@ -172,6 +171,7 @@ func (s *Scheduler) scheduleNextRun(job *Job) {
 	}
 
 	durationToNextRun := s.durationToNextRun(lastRun, job)
+
 	job.setNextRun(lastRun.Add(durationToNextRun))
 	job.setTimer(time.AfterFunc(durationToNextRun, func() {
 		s.run(job)
@@ -179,6 +179,7 @@ func (s *Scheduler) scheduleNextRun(job *Job) {
 	}))
 }
 
+// durationToNextRun calculate how much time to the next run, depending on unit
 func (s *Scheduler) durationToNextRun(lastRun time.Time, job *Job) time.Duration {
 	// job can be scheduled with .StartAt()
 	if job.getStartAtTime().After(lastRun) {
@@ -192,7 +193,7 @@ func (s *Scheduler) durationToNextRun(lastRun time.Time, job *Job) time.Duration
 	case days:
 		d = s.calculateDays(job, lastRun)
 	case weeks:
-		if job.scheduledWeekday != nil { // weekday selected, Every().Monday(), for example
+		if len(job.scheduledWeekday) != 0 { // weekday selected, Every().Monday(), for example
 			d = s.calculateWeekday(job, lastRun)
 		} else {
 			d = s.calculateWeeks(job, lastRun)
@@ -230,7 +231,7 @@ func (s *Scheduler) calculateMonths(job *Job, lastRun time.Time) time.Duration {
 }
 
 func (s *Scheduler) calculateWeekday(job *Job, lastRun time.Time) time.Duration {
-	daysToWeekday := remainingDaysToWeekday(lastRun.Weekday(), *job.scheduledWeekday)
+	daysToWeekday := remainingDaysToWeekday(lastRun.Weekday(), job.Weekdays())
 	totalDaysDifference := s.calculateTotalDaysDifference(lastRun, daysToWeekday, job)
 	nextRun := s.roundToMidnight(lastRun).Add(job.getAtTime()).AddDate(0, 0, totalDaysDifference)
 	return until(lastRun, nextRun)
@@ -286,6 +287,18 @@ func shouldRunToday(lastRun time.Time, atTime time.Time) bool {
 	return lastRun.Before(atTime)
 }
 
+func in(scheduleWeekdays []time.Weekday, weekday time.Weekday) bool {
+	in := false
+
+	for _, weekdayInSchedule := range scheduleWeekdays {
+		if int(weekdayInSchedule) == int(weekday) {
+			in = true
+			break
+		}
+	}
+	return in
+}
+
 func (s *Scheduler) calculateDuration(job *Job) time.Duration {
 	lastRun := job.LastRun()
 	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
@@ -312,10 +325,28 @@ func shouldRunAtSpecificTime(job *Job) bool {
 	return job.getAtTime() != 0
 }
 
-func remainingDaysToWeekday(from time.Weekday, to time.Weekday) int {
-	daysUntilScheduledDay := int(to) - int(from)
-	if daysUntilScheduledDay < 0 {
-		daysUntilScheduledDay += 7
+func remainingDaysToWeekday(from time.Weekday, weekDays []time.Weekday) int {
+	var (
+		to                    time.Weekday
+		daysUntilScheduledDay int
+	)
+	if len(weekDays) == 1 {
+		to = weekDays[0]
+
+		daysUntilScheduledDay = int(to) - int(from)
+		if daysUntilScheduledDay < 0 {
+			daysUntilScheduledDay += 7
+		}
+	} else {
+		daysUntilScheduledDay = 8
+		for _, day := range weekDays {
+			differenceBetweenDays := int(day) - int(from)
+
+			// checking only if is smaller than max cause there is no way to be equals
+			if differenceBetweenDays > 0 && differenceBetweenDays < daysUntilScheduledDay {
+				daysUntilScheduledDay = differenceBetweenDays
+			}
+		}
 	}
 	return daysUntilScheduledDay
 }
@@ -566,7 +597,7 @@ func (s *Scheduler) Do(jobFun interface{}, params ...interface{}) (*Job, error)
 		job.error = wrapOrError(job.error, ErrAtTimeNotSupported)
 	}
 
-	if job.scheduledWeekday != nil && jobUnit != weeks {
+	if len(job.scheduledWeekday) != 0 && jobUnit != weeks {
 		job.error = wrapOrError(job.error, ErrWeekdayNotSupported)
 	}
 
@@ -754,10 +785,14 @@ func (s *Scheduler) Months(dayOfTheMonth int) *Scheduler {
 // spill over to the next month. Similarly, if it's less than 0,
 // it will go back to the month before
 
-// Weekday sets the start with a specific weekday weekday
-func (s *Scheduler) Weekday(startDay time.Weekday) *Scheduler {
+// Weekday sets the scheduledWeekday with a specifics weekdays
+func (s *Scheduler) Weekday(weekDay time.Weekday) *Scheduler {
 	job := s.getCurrentJob()
-	job.scheduledWeekday = &startDay
+
+	if in := in(job.scheduledWeekday, weekDay); !in {
+		job.scheduledWeekday = append(job.scheduledWeekday, weekDay)
+	}
+
 	job.startsImmediately = false
 	s.setUnit(weeks)
 	return s


Test patch:
diff --git a/scheduler_test.go b/scheduler_test.go
index b8bed71..71b20eb 100644
--- a/scheduler_test.go
+++ b/scheduler_test.go
@@ -675,12 +675,12 @@ func TestScheduler_CalculateNextRun(t *testing.T) {
 		{name: "every 2 months at day 1, starting at day 2, should run in 2 months - 1 day", job: &Job{interval: 2, unit: months, dayOfTheMonth: 1, lastRun: januaryFirst2020At(0, 0, 0).AddDate(0, 0, 1)}, wantTimeUntilNextRun: 30*day + 29*day}, // 2020 january and february
 		{name: "every 13 months at day 1, starting at day 2 run in 13 months - 1 day", job: &Job{interval: 13, unit: months, dayOfTheMonth: 1, lastRun: januaryFirst2020At(0, 0, 0).AddDate(0, 0, 1)}, wantTimeUntilNextRun: januaryFirst2020At(0, 0, 0).AddDate(0, 13, -1).Sub(januaryFirst2020At(0, 0, 0))},
 		//// WEEKDAYS
-		{name: "every weekday starting on one day before it should run this weekday", job: &Job{interval: 1, unit: weeks, scheduledWeekday: _tuesdayWeekday(), lastRun: mondayAt(0, 0, 0)}, wantTimeUntilNextRun: 1 * day},
-		{name: "every weekday starting on same weekday should run on same immediately", job: &Job{interval: 1, unit: weeks, scheduledWeekday: _tuesdayWeekday(), lastRun: mondayAt(0, 0, 0).AddDate(0, 0, 1)}, wantTimeUntilNextRun: 0},
-		{name: "every 2 weekdays counting this week's weekday should run next weekday", job: &Job{interval: 2, unit: weeks, scheduledWeekday: _tuesdayWeekday(), lastRun: mondayAt(0, 0, 0)}, wantTimeUntilNextRun: 8 * day},
-		{name: "every weekday starting on one day after should count days remaining", job: &Job{interval: 1, unit: weeks, scheduledWeekday: _tuesdayWeekday(), lastRun: mondayAt(0, 0, 0).AddDate(0, 0, 2)}, wantTimeUntilNextRun: 6 * day},
-		{name: "every weekday starting before jobs .At() time should run at same day at time", job: &Job{interval: 1, unit: weeks, atTime: _getHours(9) + _getMinutes(30), scheduledWeekday: _tuesdayWeekday(), lastRun: mondayAt(0, 0, 0).AddDate(0, 0, 1)}, wantTimeUntilNextRun: _getHours(9) + _getMinutes(30)},
-		{name: "every weekday starting at same day at time that already passed should run at next week at time", job: &Job{interval: 1, unit: weeks, atTime: _getHours(9) + _getMinutes(30), scheduledWeekday: _tuesdayWeekday(), lastRun: mondayAt(10, 30, 0).AddDate(0, 0, 1)}, wantTimeUntilNextRun: 6*day + _getHours(23) + _getMinutes(0)},
+		{name: "every weekday starting on one day before it should run this weekday", job: &Job{interval: 1, unit: weeks, scheduledWeekday: []time.Weekday{*_tuesdayWeekday()}, lastRun: mondayAt(0, 0, 0)}, wantTimeUntilNextRun: 1 * day},
+		{name: "every weekday starting on same weekday should run on same immediately", job: &Job{interval: 1, unit: weeks, scheduledWeekday: []time.Weekday{*_tuesdayWeekday()}, lastRun: mondayAt(0, 0, 0).AddDate(0, 0, 1)}, wantTimeUntilNextRun: 0},
+		{name: "every 2 weekdays counting this week's weekday should run next weekday", job: &Job{interval: 2, unit: weeks, scheduledWeekday: []time.Weekday{*_tuesdayWeekday()}, lastRun: mondayAt(0, 0, 0)}, wantTimeUntilNextRun: 8 * day},
+		{name: "every weekday starting on one day after should count days remaining", job: &Job{interval: 1, unit: weeks, scheduledWeekday: []time.Weekday{*_tuesdayWeekday()}, lastRun: mondayAt(0, 0, 0).AddDate(0, 0, 2)}, wantTimeUntilNextRun: 6 * day},
+		{name: "every weekday starting before jobs .At() time should run at same day at time", job: &Job{interval: 1, unit: weeks, atTime: _getHours(9) + _getMinutes(30), scheduledWeekday: []time.Weekday{*_tuesdayWeekday()}, lastRun: mondayAt(0, 0, 0).AddDate(0, 0, 1)}, wantTimeUntilNextRun: _getHours(9) + _getMinutes(30)},
+		{name: "every weekday starting at same day at time that already passed should run at next week at time", job: &Job{interval: 1, unit: weeks, atTime: _getHours(9) + _getMinutes(30), scheduledWeekday: []time.Weekday{*_tuesdayWeekday()}, lastRun: mondayAt(10, 30, 0).AddDate(0, 0, 1)}, wantTimeUntilNextRun: 6*day + _getHours(23) + _getMinutes(0)},
 	}
 
 	for _, tc := range testCases {
@@ -1333,3 +1333,103 @@ func TestScheduler_WaitForSchedules(t *testing.T) {
 	defer counterMutex.RUnlock()
 	assert.Equal(t, 3, counter)
 }
+
+func TestScheduler_LenWeekDays(t *testing.T) {
+
+	testCases := []struct {
+		description string
+		weekDays    []time.Weekday
+		finalLen    int
+	}{
+		{"no week day", []time.Weekday{}, 0},
+		{"equal week day", []time.Weekday{time.Friday, time.Friday, time.Friday}, 1},
+		{"more than one week day", []time.Weekday{time.Friday, time.Saturday, time.Sunday}, 3},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.description, func(t *testing.T) {
+			s := NewScheduler(time.UTC)
+			s = s.Every(1)
+			for _, weekDay := range tc.weekDays {
+				s = s.Weekday(weekDay)
+			}
+			j, err := s.Do(func() {})
+			require.NoError(t, err)
+			assert.Equal(t, len(j.scheduledWeekday), tc.finalLen)
+		})
+	}
+
+}
+
+func TestScheduler_CallNextWeekDay(t *testing.T) {
+	januaryFirst2020At := func(hour, minute, second int) time.Time {
+		return time.Date(2020, time.January, 1, hour, minute, second, 0, time.UTC)
+	}
+
+	const wantTimeUntilNextRun = time.Hour * 24 * 2
+	var lastRun = januaryFirst2020At(0, 0, 0)
+
+	testCases := []struct {
+		description string
+		weekDays    []time.Weekday
+	}{
+		{"week days not in order", []time.Weekday{time.Monday, time.Friday}},
+		{"week days in order", []time.Weekday{time.Friday, time.Monday}},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.description, func(t *testing.T) {
+
+			s := NewScheduler(time.UTC)
+			s.Every(1)
+
+			for _, weekDay := range tc.weekDays {
+				s.Weekday(weekDay)
+			}
+
+			job, err := s.Do(func() {})
+			require.NoError(t, err)
+			job.lastRun = lastRun
+
+			got := s.durationToNextRun(lastRun, job)
+			assert.Equal(t, wantTimeUntilNextRun, got)
+
+		})
+	}
+
+}
+
+func TestScheduler_CheckNextWeekDay(t *testing.T) {
+	januaryFirst2020At := func(hour, minute, second int) time.Time {
+		return time.Date(2020, time.January, 1, hour, minute, second, 0, time.UTC)
+	}
+	januarySecond2020At := func(hour, minute, second int) time.Time {
+		return time.Date(2020, time.January, 2, hour, minute, second, 0, time.UTC)
+	}
+	const (
+		wantTimeUntilNextFirstRun = 1 * time.Second
+		// all day long
+		wantTimeUntilNextSecondRun = 24 * time.Hour
+	)
+
+	t.Run("check slice next run", func(t *testing.T) {
+		s := NewScheduler(time.UTC)
+		lastRun := januaryFirst2020At(23, 59, 59)
+		secondLastRun := januarySecond2020At(0, 0, 0)
+
+		job, err := s.Every(1).Week().Friday().Thursday().Do(func() {})
+		require.NoError(t, err)
+		job.lastRun = lastRun
+
+		gotFirst := s.durationToNextRun(lastRun, job)
+		assert.Equal(t, wantTimeUntilNextFirstRun, gotFirst)
+
+		s.StartAsync()
+		time.Sleep(1 * time.Second)
+
+		job.lastRun = secondLastRun
+		gotSecond := s.durationToNextRun(secondLastRun, job)
+		assert.Equal(t, wantTimeUntilNextSecondRun, gotSecond)
+
+	})
+}