diff --git a/README.md b/README.md index 979f71e6..38c4d8a0 100644 --- a/README.md +++ b/README.md @@ -6,13 +6,13 @@ Cron V3 has been released! To download the specific tagged release, run: - - go get github.com/robfig/cron/v3@v3.0.0 - +```bash +go get github.com/robfig/cron/v3@v3.0.0 +``` Import it in your program as: - - import "github.com/robfig/cron/v3" - +```go +import "github.com/robfig/cron/v3" +``` It requires Go 1.11 or later due to usage of Go Modules. Refer to the documentation here: @@ -65,15 +65,15 @@ It is backwards incompatible with both v1 and v2. These updates are required: UPDATING: To retain the old behavior, construct your Cron with a custom parser: - - // Seconds field, required - cron.New(cron.WithSeconds()) - - // Seconds field, optional - cron.New( - cron.WithParser( - cron.SecondOptional | cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow | cron.Descriptor)) - +```go +// Seconds field, required +cron.New(cron.WithSeconds()) + +// Seconds field, optional +cron.New(cron.WithParser(cron.NewParser( + cron.SecondOptional | cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow | cron.Descriptor, +))) +``` - The Cron type now accepts functional options on construction rather than the previous ad-hoc behavior modification mechanisms (setting a field, calling a setter). @@ -92,20 +92,20 @@ It is backwards incompatible with both v1 and v2. These updates are required: has been removed to accommodate the more general JobWrapper type. UPDATING: To opt into panic recovery and configure the panic logger: - - cron.New(cron.WithChain( - cron.Recover(logger), // or use cron.DefaultLogger - )) - +```go +cron.New(cron.WithChain( + cron.Recover(logger), // or use cron.DefaultLogger +)) +``` - In adding support for https://github.com/go-logr/logr, `cron.WithVerboseLogger` was removed, since it is duplicative with the leveled logging. UPDATING: Callers should use `WithLogger` and specify a logger that does not discard `Info` logs. For convenience, one is provided that wraps `*log.Logger`: - - cron.New( - cron.WithLogger(cron.VerbosePrintfLogger(logger))) - +```go +cron.New( + cron.WithLogger(cron.VerbosePrintfLogger(logger))) +``` ### Background - Cron spec format @@ -118,7 +118,7 @@ There are two cron spec formats in common usage: jobs in Java software [the Cron wikipedia page]: https://en.wikipedia.org/wiki/Cron -[the Quartz Scheduler]: http://www.quartz-scheduler.org/documentation/quartz-2.x/tutorials/crontrigger.html +[the Quartz Scheduler]: http://www.quartz-scheduler.org/documentation/quartz-2.3.0/tutorials/tutorial-lesson-06.html The original version of this package included an optional "seconds" field, which made it incompatible with both of these formats. Now, the "standard" format is diff --git a/chain.go b/chain.go index 118e5bbe..9c087b7b 100644 --- a/chain.go +++ b/chain.go @@ -76,14 +76,14 @@ func DelayIfStillRunning(logger Logger) JobWrapper { // SkipIfStillRunning skips an invocation of the Job if a previous invocation is // still running. It logs skips to the given logger at Info level. func SkipIfStillRunning(logger Logger) JobWrapper { - var ch = make(chan struct{}, 1) - ch <- struct{}{} return func(j Job) Job { + var ch = make(chan struct{}, 1) + ch <- struct{}{} return FuncJob(func() { select { case v := <-ch: + defer func() { ch <- v }() j.Run() - ch <- v default: logger.Info("skip") } diff --git a/chain_test.go b/chain_test.go index 2561bd7f..ec910975 100644 --- a/chain_test.go +++ b/chain_test.go @@ -218,4 +218,25 @@ func TestChainSkipIfStillRunning(t *testing.T) { } }) + t.Run("different jobs independent", func(t *testing.T) { + var j1, j2 countJob + j1.delay = 10 * time.Millisecond + j2.delay = 10 * time.Millisecond + chain := NewChain(SkipIfStillRunning(DiscardLogger)) + wrappedJob1 := chain.Then(&j1) + wrappedJob2 := chain.Then(&j2) + for i := 0; i < 11; i++ { + go wrappedJob1.Run() + go wrappedJob2.Run() + } + time.Sleep(100 * time.Millisecond) + var ( + done1 = j1.Done() + done2 = j2.Done() + ) + if done1 != 1 || done2 != 1 { + t.Error("expected both jobs executed once, got", done1, "and", done2) + } + }) + } diff --git a/cron.go b/cron.go index f6e451db..c7e91766 100644 --- a/cron.go +++ b/cron.go @@ -21,11 +21,16 @@ type Cron struct { logger Logger runningMu sync.Mutex location *time.Location - parser Parser + parser ScheduleParser nextID EntryID jobWaiter sync.WaitGroup } +// ScheduleParser is an interface for schedule spec parsers that return a Schedule +type ScheduleParser interface { + Parse(spec string) (Schedule, error) +} + // Job is an interface for submitted cron jobs. type Job interface { Run() diff --git a/doc.go b/doc.go index aa5a76f2..fa5d08b4 100644 --- a/doc.go +++ b/doc.go @@ -21,7 +21,7 @@ them in their own goroutines. c := cron.New() c.AddFunc("30 * * * *", func() { fmt.Println("Every hour on the half hour") }) c.AddFunc("30 3-6,20-23 * * *", func() { fmt.Println(".. in the range 3-6am, 8-11pm") }) - c.AddFunc("CRON_TZ=Asia/Tokyo 30 04 * * * *", func() { fmt.Println("Runs at 04:30 Tokyo time every day") }) + c.AddFunc("CRON_TZ=Asia/Tokyo 30 04 * * *", func() { fmt.Println("Runs at 04:30 Tokyo time every day") }) c.AddFunc("@hourly", func() { fmt.Println("Every hour, starting an hour from now") }) c.AddFunc("@every 1h30m", func() { fmt.Println("Every hour thirty, starting an hour thirty from now") }) c.Start() diff --git a/option.go b/option.go index 07638201..09e4278e 100644 --- a/option.go +++ b/option.go @@ -23,7 +23,7 @@ func WithSeconds() Option { } // WithParser overrides the parser used for interpreting job schedules. -func WithParser(p Parser) Option { +func WithParser(p ScheduleParser) Option { return func(c *Cron) { c.parser = p } diff --git a/parser.go b/parser.go index 3cf8879f..9a294fba 100644 --- a/parser.go +++ b/parser.go @@ -61,11 +61,11 @@ type Parser struct { // sched, err := specParser.Parse("0 0 15 */3 *") // // // Same as above, just excludes time fields -// subsParser := NewParser(Dom | Month | Dow) +// specParser := NewParser(Dom | Month | Dow) // sched, err := specParser.Parse("15 */3 *") // // // Same as above, just makes Dow optional -// subsParser := NewParser(Dom | Month | DowOptional) +// specParser := NewParser(Dom | Month | DowOptional) // sched, err := specParser.Parse("15 */3") // func NewParser(options ParseOption) Parser { @@ -313,6 +313,9 @@ func getRange(expr string, r bounds) (uint64, error) { if step == 0 { return 0, fmt.Errorf("step of range should be a positive number: %s", expr) } + if step > r.max-r.min { + return 0, fmt.Errorf("step (%d) above maximum (%d): %s", step, r.max-r.min, expr) + } return getBits(start, end, step) | extra, nil } diff --git a/parser_test.go b/parser_test.go index 41c8c520..8d48a46b 100644 --- a/parser_test.go +++ b/parser_test.go @@ -334,6 +334,18 @@ func TestStandardSpecSchedule(t *testing.T) { expr: "* * * *", err: "expected exactly 5 fields", }, + { + // Step value larger than the field range should be rejected. + // */90 in the minute field (range 0-59) would only ever fire at + // minute 0, silently ignoring the intent. See issue #543. + expr: "*/90 * * * *", + err: "step (90) above maximum (59)", + }, + { + // Same check for hours: */30 is the max valid step (0-23 → range 23). + expr: "* */30 * * *", + err: "step (30) above maximum (23)", + }, } for _, c := range entries {