From 921be54fdea25e4054dad7a58e41fc17ead22681 Mon Sep 17 00:00:00 2001 From: memwey <10784701+memwey@users.noreply.github.com> Date: Sun, 4 Sep 2022 23:47:04 +0800 Subject: [PATCH 1/3] Use time.Duration & Allow custom cron.Parser --- common_test.go | 2 ++ cronrange.go | 60 ++++++++++++++++++++++++++++++++--------------- cronrange_test.go | 42 +++++++++++++++++++++++++++++++++ serialize.go | 37 ++++++++++++++++++++++++++--- serialize_test.go | 50 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 169 insertions(+), 22 deletions(-) diff --git a/common_test.go b/common_test.go index 094a36c..3ff0e69 100644 --- a/common_test.go +++ b/common_test.go @@ -57,6 +57,8 @@ var ( crThirdDayEachMonthHonolulu, _ = New("0 0 3 * *", timeZoneHonolulu, 1440) crFirstHourFeb29, _ = New("0 0 29 2 *", "", 60) crFirstHourFeb28OrSun, _ = New("0 0 28 2 0", "", 60) + crEvery1MinV2, _ = Create(exprEveryMin, emptyString, time.Minute*1, cronParser) + crEveryNewYearsDayTokyoV2, _ = Create(exprEveryNewYear, timeZoneTokyo, time.Hour*24, cronParser) ) type tempTestWithPointer struct { diff --git a/cronrange.go b/cronrange.go index fb44e3a..34b366f 100644 --- a/cronrange.go +++ b/cronrange.go @@ -16,12 +16,18 @@ var ( errZeroDuration = errors.New("duration should be positive") ) +const ( + Version1 = 0 + Version2 = 1 +) + // CronRange consists of cron expression along with time zone and duration info. type CronRange struct { cronExpression string timeZone string duration time.Duration schedule cron.Schedule + version int } // TimeRange represents a time range between starting time and ending time. @@ -34,8 +40,31 @@ type TimeRange struct { // // It returns an error if duration is not positive number, or cron expression is invalid, or time zone doesn't exist. func New(cronExpr, timeZone string, durationMin uint64) (cr *CronRange, err error) { + cr, err = new(cronExpr, timeZone, time.Duration(durationMin)*time.Minute, cronParser) + return +} + +// Duration returns the duration of the CronRange. +func (cr *CronRange) Duration() time.Duration { + cr.checkPrecondition() + return cr.duration +} + +// TimeZone returns the time zone string of the CronRange. +func (cr *CronRange) TimeZone() string { + cr.checkPrecondition() + return cr.timeZone +} + +// CronExpression returns the Cron expression of the CronRange. +func (cr *CronRange) CronExpression() string { + cr.checkPrecondition() + return cr.cronExpression +} + +func new(cronExpr, timeZone string, duration time.Duration, cp cron.Parser) (cr *CronRange, err error) { // Precondition check - if durationMin == 0 { + if duration <= 0 { err = errZeroDuration return } @@ -53,33 +82,26 @@ func New(cronExpr, timeZone string, durationMin uint64) (cr *CronRange, err erro // Validate & retrieve crontab schedule var schedule cron.Schedule - if schedule, err = cronParser.Parse(cronSpec); err != nil { + if schedule, err = cp.Parse(cronSpec); err != nil { return } cr = &CronRange{ cronExpression: cronExpr, timeZone: timeZone, - duration: time.Minute * time.Duration(durationMin), + duration: duration, schedule: schedule, } return } -// Duration returns the duration of the CronRange. -func (cr *CronRange) Duration() time.Duration { - cr.checkPrecondition() - return cr.duration -} - -// TimeZone returns the time zone string of the CronRange. -func (cr *CronRange) TimeZone() string { - cr.checkPrecondition() - return cr.timeZone -} - -// CronExpression returns the Cron expression of the CronRange. -func (cr *CronRange) CronExpression() string { - cr.checkPrecondition() - return cr.cronExpression +// Create returns a CronRange instance with given config, time zone can be empty for local time zone. +// +// It returns an error if duration is not positive number, or cron expression is invalid, or time zone doesn't exist. +func Create(cronExpr, timeZone string, duration time.Duration, cp cron.Parser) (cr *CronRange, err error) { + cr, err = new(cronExpr, timeZone, duration, cp) + if err == nil { + cr.version = Version2 + } + return } diff --git a/cronrange_test.go b/cronrange_test.go index 8da4d76..b2d5e20 100644 --- a/cronrange_test.go +++ b/cronrange_test.go @@ -3,6 +3,8 @@ package cronrange import ( "testing" "time" + + "github.com/robfig/cron/v3" ) func TestNew(t *testing.T) { @@ -43,6 +45,46 @@ func TestNew(t *testing.T) { } } +func TestCreate(t *testing.T) { + type args struct { + cronExpr string + timeZone string + durationMin uint64 + duration time.Duration + cp cron.Parser + } + tests := []struct { + name string + args args + wantCr bool + wantErr bool + }{ + {"Empty cronExpr", args{emptyString, emptyString, 5, time.Minute * 5, cronParser}, false, true}, + {"Invalid cronExpr", args{"h e l l o", emptyString, 5, time.Minute * 5, cronParser}, false, true}, + {"Incomplete cronExpr", args{"* * * *", emptyString, 5, time.Minute * 5, cronParser}, false, true}, + {"Nonexistent time zone", args{exprEveryMin, "Mars", 5, time.Minute * 5, cronParser}, false, true}, + {"Zero durationMin", args{exprEveryMin, emptyString, 0, time.Minute * 0, cronParser}, false, true}, + {"Normal without time zone", args{exprEveryMin, emptyString, 5, time.Minute * 5, cronParser}, true, false}, + {"Normal with local time zone", args{exprEveryMin, " Local ", 5, time.Minute * 5, cronParser}, true, false}, + {"Normal with 5 min in Bangkok", args{exprEveryMin, timeZoneBangkok, 5, time.Minute * 5, cronParser}, true, false}, + {"Normal with 1 day in Tokyo", args{exprEveryNewYear, timeZoneTokyo, 1440, time.Minute * 1440, cronParser}, true, false}, + {"Normal with large duration", args{exprEveryMin, timeZoneBangkok, 5259000, time.Minute * 5259000, cronParser}, true, false}, + {"Normal with complicated cron expression", args{exprVeryComplicated, timeZoneHonolulu, 5258765, time.Minute * 5258765, cronParser}, true, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotCr, err := Create(tt.args.cronExpr, tt.args.timeZone, tt.args.duration, tt.args.cp) + if (err != nil) != tt.wantErr { + t.Errorf("New() error = %v, wantErr %v", err, tt.wantErr) + return + } + if (gotCr != nil) != tt.wantCr { + t.Errorf("New() gotCr = %v, wantCr %v", gotCr, tt.wantCr) + } + }) + } +} + func BenchmarkNew(b *testing.B) { for i := 0; i < b.N; i++ { _, _ = New(exprEveryMin, timeZoneBangkok, 10) diff --git a/serialize.go b/serialize.go index 90d5444..2865d12 100644 --- a/serialize.go +++ b/serialize.go @@ -7,6 +7,8 @@ import ( "strconv" "strings" "time" + + "github.com/robfig/cron/v3" ) var ( @@ -28,7 +30,11 @@ func (cr CronRange) String() string { sb.Grow(36) if cr.duration > 0 { sb.WriteString(strMarkDuration) - sb.WriteString(strconv.FormatUint(uint64(cr.duration/time.Minute), 10)) + if cr.version == Version2 { + sb.WriteString(cr.duration.String()) + } else { + sb.WriteString(strconv.FormatUint(uint64(cr.duration/time.Minute), 10)) + } sb.WriteString(strSemicolon) sb.WriteString(strSingleWhitespace) } @@ -44,6 +50,11 @@ func (cr CronRange) String() string { // ParseString attempts to deserialize the given expression or return failure if any parsing errors occur. func ParseString(s string) (cr *CronRange, err error) { + cr, err = parseString(s, cronParser) + return +} + +func parseString(s string, cp cron.Parser) (cr *CronRange, err error) { if s == "" { err = errEmptyExpr return @@ -54,6 +65,8 @@ func ParseString(s string) (cr *CronRange, err error) { durMin uint64 parts = strings.Split(s, strSemicolon) idxExpr = len(parts) - 1 + version int + duration time.Duration ) if idxExpr == 0 { err = errIncompleteExpr @@ -74,8 +87,17 @@ PL: cronExpr = part case strings.HasPrefix(part, strMarkDuration): durStr = part[len(strMarkDuration):] + if duration, err = time.ParseDuration(durStr); err == nil { + if duration > 0 { + version = Version2 + } + } if durMin, err = strconv.ParseUint(durStr, 10, 64); err != nil { - break PL + if version != Version2 { + break PL + } else { + err = nil + } } case strings.HasPrefix(part, strMarkTimeZone): timeZone = part[len(strMarkTimeZone):] @@ -86,7 +108,11 @@ PL: if err == nil { if len(durStr) > 0 { - cr, err = New(cronExpr, timeZone, durMin) + if version == Version2 { + cr, err = Create(cronExpr, timeZone, duration, cp) + } else { + cr, err = new(cronExpr, timeZone, time.Duration(durMin)*time.Minute, cp) + } } else { err = errMissDurationExpr } @@ -94,6 +120,11 @@ PL: return } +func ParseStringWithCronParser(s string, cp cron.Parser) (cr *CronRange, err error) { + cr, err = parseString(s, cronParser) + return +} + // MarshalJSON implements the encoding/json.Marshaler interface for serialization of CronRange. func (cr CronRange) MarshalJSON() ([]byte, error) { expr := cr.String() diff --git a/serialize_test.go b/serialize_test.go index 061cafb..85dd310 100644 --- a/serialize_test.go +++ b/serialize_test.go @@ -6,6 +6,8 @@ import ( "strings" "testing" "time" + + "github.com/robfig/cron/v3" ) func TestCronRange_String(t *testing.T) { @@ -19,12 +21,14 @@ func TestCronRange_String(t *testing.T) { {"Use string() instead of sprintf", crEvery1Min, "DR=1; * * * * *"}, {"Use instance instead of pointer", crEvery1Min, "DR=1; * * * * *"}, {"1min duration without time zone", crEvery1Min, "DR=1; * * * * *"}, + {"1min duration without time zone V2", crEvery1MinV2, "DR=1m0s; * * * * *"}, {"5min duration without time zone", crEvery5Min, "DR=5; */5 * * * *"}, {"10min duration with local time zone", crEvery10MinLocal, "DR=10; */10 * * * *"}, {"10min duration with time zone", crEvery10MinBangkok, "DR=10; TZ=Asia/Bangkok; */10 * * * *"}, {"Every Xmas morning in NYC", crEveryXmasMorningNYC, "DR=240; TZ=America/New_York; 0 8 25 12 *"}, {"Every New Year's Day in Bangkok", crEveryNewYearsDayBangkok, "DR=1440; TZ=Asia/Bangkok; 0 0 1 1 *"}, {"Every New Year's Day in Tokyo", crEveryNewYearsDayTokyo, "DR=1440; TZ=Asia/Tokyo; 0 0 1 1 *"}, + {"Every New Year's Day in Tokyo V2", crEveryNewYearsDayTokyoV2, "DR=24h0m0s; TZ=Asia/Tokyo; 0 0 1 1 *"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -97,6 +101,52 @@ func TestParseString(t *testing.T) { } } +var deserializeV2TestCases = []struct { + name string + inputS string + wantS string + wantErr bool + cp cron.Parser +}{ + {"Empty string", emptyString, emptyString, true, cronParser}, + {"Invalid expression", "hello", emptyString, true, cronParser}, + {"Missing duration", "; * * * * *", emptyString, true, cronParser}, + {"Invalid duration=0", "DR=0m;* * * * *", emptyString, true, cronParser}, + {"Invalid duration=-5", "DR=-5m;* * * * *", emptyString, true, cronParser}, + {"Invalid with Mars time zone", "DR=5m;TZ=Mars;* * * * *", emptyString, true, cronParser}, + {"Invalid with unknown part", "DR=10m; TZ=Pacific/Honolulu; SET=1; * * * * *", emptyString, true, cronParser}, + {"Invalid with lower case", "dr=5m;* * * * *", emptyString, true, cronParser}, + {"Invalid with wrong order", "* * * * *; DR=5m;", emptyString, true, cronParser}, + {"Normal without timezone", "DR=5m;* * * * *", "DR=5m0s; * * * * *", false, cronParser}, + {"Normal with extra whitespaces", " DR=6m ; * * * * * ", "DR=6m0s; * * * * *", false, cronParser}, + {"Normal with empty parts", "; DR=7m;;; ;; ;; ;* * * * * ", "DR=7m0s; * * * * *", false, cronParser}, + {"Normal with different order", "TZ=Asia/Tokyo; DR=1440m; 0 0 1 1 *", "DR=24h0m0s; TZ=Asia/Tokyo; 0 0 1 1 *", false, cronParser}, + {"Normal with local time zone", "DR=8m;TZ=Local;* * * * *", "DR=8m0s; * * * * *", false, cronParser}, + {"Normal with UTC time zone", "DR=9m;TZ=Etc/UTC;* * * * *", "DR=9m0s; TZ=Etc/UTC; * * * * *", false, cronParser}, + {"Normal with Honolulu time zone", "DR=10m;TZ=Pacific/Honolulu;* * * * *", "DR=10m0s; TZ=Pacific/Honolulu; * * * * *", false, cronParser}, + {"Normal with Honolulu time zone in different order", "TZ=Pacific/Honolulu; DR=10m; * * * * *", "DR=10m0s; TZ=Pacific/Honolulu; * * * * *", false, cronParser}, + {"Normal with complicated expression", "DR=5258765m; TZ=Pacific/Honolulu; 4,8,22,27,33,38,47,50 3,11,14-16,19,21,22 */10 1,3,5,6,9-11 1-5", "DR=87646h5m0s; TZ=Pacific/Honolulu; 4,8,22,27,33,38,47,50 3,11,14-16,19,21,22 */10 1,3,5,6,9-11 1-5", false, cronParser}, +} + +func TestParseStringWithCronParser(t *testing.T) { + for _, tt := range deserializeV2TestCases { + t.Run(tt.name, func(t *testing.T) { + gotCr, err := ParseStringWithCronParser(tt.inputS, tt.cp) + if (err != nil) != tt.wantErr { + t.Errorf("ParseString() error: %v, wantErr: %v", err, tt.wantErr) + return + } + if !tt.wantErr && (gotCr == nil || gotCr.schedule == nil || gotCr.duration == 0) { + t.Errorf("ParseString() incomplete gotCr: %v", gotCr) + return + } + if !tt.wantErr && gotCr.String() != tt.wantS { + t.Errorf("ParseString() gotCr: %s, want: %s", gotCr.String(), tt.wantS) + } + }) + } +} + func BenchmarkParseString(b *testing.B) { rs := "DR=10;TZ=Pacific/Honolulu;;* * * * *" b.ResetTimer() From 02579b87e9d4955961a9c2f53afc98605de0a4f2 Mon Sep 17 00:00:00 2001 From: memwey <10784701+memwey@users.noreply.github.com> Date: Mon, 5 Sep 2022 00:04:56 +0800 Subject: [PATCH 2/3] Fix some problems --- cronrange.go | 6 +++--- serialize.go | 5 ++--- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/cronrange.go b/cronrange.go index 34b366f..18ff567 100644 --- a/cronrange.go +++ b/cronrange.go @@ -62,9 +62,9 @@ func (cr *CronRange) CronExpression() string { return cr.cronExpression } -func new(cronExpr, timeZone string, duration time.Duration, cp cron.Parser) (cr *CronRange, err error) { +func new(cronExpr, timeZone string, dur time.Duration, cp cron.Parser) (cr *CronRange, err error) { // Precondition check - if duration <= 0 { + if dur <= 0 { err = errZeroDuration return } @@ -89,7 +89,7 @@ func new(cronExpr, timeZone string, duration time.Duration, cp cron.Parser) (cr cr = &CronRange{ cronExpression: cronExpr, timeZone: timeZone, - duration: duration, + duration: dur, schedule: schedule, } return diff --git a/serialize.go b/serialize.go index 2865d12..d6b5492 100644 --- a/serialize.go +++ b/serialize.go @@ -95,9 +95,8 @@ PL: if durMin, err = strconv.ParseUint(durStr, 10, 64); err != nil { if version != Version2 { break PL - } else { - err = nil } + err = nil } case strings.HasPrefix(part, strMarkTimeZone): timeZone = part[len(strMarkTimeZone):] @@ -121,7 +120,7 @@ PL: } func ParseStringWithCronParser(s string, cp cron.Parser) (cr *CronRange, err error) { - cr, err = parseString(s, cronParser) + cr, err = parseString(s, cp) return } From 9a08120a6af208d3db25dc3945e2c3489ade325a Mon Sep 17 00:00:00 2001 From: memwey <10784701+memwey@users.noreply.github.com> Date: Mon, 5 Sep 2022 00:46:50 +0800 Subject: [PATCH 3/3] Fix some problems --- cronrange.go | 22 +++++++++++----------- function.go | 2 +- serialize.go | 8 ++++---- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/cronrange.go b/cronrange.go index 18ff567..48189c1 100644 --- a/cronrange.go +++ b/cronrange.go @@ -40,7 +40,7 @@ type TimeRange struct { // // It returns an error if duration is not positive number, or cron expression is invalid, or time zone doesn't exist. func New(cronExpr, timeZone string, durationMin uint64) (cr *CronRange, err error) { - cr, err = new(cronExpr, timeZone, time.Duration(durationMin)*time.Minute, cronParser) + cr, err = internalNew(cronExpr, timeZone, time.Duration(durationMin)*time.Minute, cronParser) return } @@ -62,22 +62,22 @@ func (cr *CronRange) CronExpression() string { return cr.cronExpression } -func new(cronExpr, timeZone string, dur time.Duration, cp cron.Parser) (cr *CronRange, err error) { +func internalNew(cronExpr, tz string, td time.Duration, cp cron.Parser) (cr *CronRange, err error) { // Precondition check - if dur <= 0 { + if td <= 0 { err = errZeroDuration return } // Clean up string parameters - cronExpr, timeZone = strings.TrimSpace(cronExpr), strings.TrimSpace(timeZone) + cronExpr, tz = strings.TrimSpace(cronExpr), strings.TrimSpace(tz) // Append time zone into cron spec if necessary cronSpec := cronExpr - if strings.ToLower(timeZone) == "local" { - timeZone = "" - } else if len(timeZone) > 0 { - cronSpec = fmt.Sprintf("CRON_TZ=%s %s", timeZone, cronExpr) + if strings.ToLower(tz) == "local" { + tz = "" + } else if len(tz) > 0 { + cronSpec = fmt.Sprintf("CRON_TZ=%s %s", tz, cronExpr) } // Validate & retrieve crontab schedule @@ -88,8 +88,8 @@ func new(cronExpr, timeZone string, dur time.Duration, cp cron.Parser) (cr *Cron cr = &CronRange{ cronExpression: cronExpr, - timeZone: timeZone, - duration: dur, + timeZone: tz, + duration: td, schedule: schedule, } return @@ -99,7 +99,7 @@ func new(cronExpr, timeZone string, dur time.Duration, cp cron.Parser) (cr *Cron // // It returns an error if duration is not positive number, or cron expression is invalid, or time zone doesn't exist. func Create(cronExpr, timeZone string, duration time.Duration, cp cron.Parser) (cr *CronRange, err error) { - cr, err = new(cronExpr, timeZone, duration, cp) + cr, err = internalNew(cronExpr, timeZone, duration, cp) if err == nil { cr.version = Version2 } diff --git a/function.go b/function.go index f8f5580..5b49b47 100644 --- a/function.go +++ b/function.go @@ -48,7 +48,7 @@ func (cr *CronRange) IsWithin(t time.Time) (within bool) { cr.checkPrecondition() within = false - searchStart := t.Add(-(cr.duration + 1*time.Second)) + searchStart := t.Add(-(cr.duration + 1*time.Second - 1*time.Nanosecond)) rangeStart := cr.schedule.Next(searchStart) rangeEnd := rangeStart.Add(cr.duration) diff --git a/serialize.go b/serialize.go index d6b5492..c135c75 100644 --- a/serialize.go +++ b/serialize.go @@ -50,11 +50,11 @@ func (cr CronRange) String() string { // ParseString attempts to deserialize the given expression or return failure if any parsing errors occur. func ParseString(s string) (cr *CronRange, err error) { - cr, err = parseString(s, cronParser) + cr, err = internalParseString(s, cronParser) return } -func parseString(s string, cp cron.Parser) (cr *CronRange, err error) { +func internalParseString(s string, cp cron.Parser) (cr *CronRange, err error) { if s == "" { err = errEmptyExpr return @@ -110,7 +110,7 @@ PL: if version == Version2 { cr, err = Create(cronExpr, timeZone, duration, cp) } else { - cr, err = new(cronExpr, timeZone, time.Duration(durMin)*time.Minute, cp) + cr, err = internalNew(cronExpr, timeZone, time.Duration(durMin)*time.Minute, cp) } } else { err = errMissDurationExpr @@ -120,7 +120,7 @@ PL: } func ParseStringWithCronParser(s string, cp cron.Parser) (cr *CronRange, err error) { - cr, err = parseString(s, cp) + cr, err = internalParseString(s, cp) return }