From 5c1e65256c9771ddab239ee6445a782a96fe44a1 Mon Sep 17 00:00:00 2001 From: ionutboangiu Date: Wed, 17 Apr 2024 19:05:22 +0300 Subject: [PATCH] Don't trim single digit values when parsing cron Fixes an issue where 0 values would become empty inside the cron expressions. Added unit tests for the edge cases and grouped them together with the previous ones under the same table test. When creating the StartTime field, assign time.Now() to a centralised variable and reuse it instead of calling time.Now() repeatedly. --- apier/v1/apier.go | 29 ++--- engine/rateinterval.go | 12 +- engine/rateinterval_test.go | 220 +++++++++++++++++++++--------------- engine/tpreader.go | 29 ++--- 4 files changed, 171 insertions(+), 119 deletions(-) diff --git a/apier/v1/apier.go b/apier/v1/apier.go index 6941ed54b..a34028aa2 100644 --- a/apier/v1/apier.go +++ b/apier/v1/apier.go @@ -877,7 +877,8 @@ func verifyFormat(tStr string) bool { // checkDefaultTiming will check the tStr if it's of the the default timings ( the same as in TPReader ) // and will compute it properly func checkDefaultTiming(tStr string) (rTm *engine.RITiming, isDefault bool) { - startTime := time.Now().Format("15:04:05") + currentTime := time.Now() + fmtTime := currentTime.Format("15:04:05") switch tStr { case utils.MetaEveryMinute: return &engine.RITiming{ @@ -886,7 +887,7 @@ func checkDefaultTiming(tStr string) (rTm *engine.RITiming, isDefault bool) { Months: utils.Months{}, MonthDays: utils.MonthDays{}, WeekDays: utils.WeekDays{}, - StartTime: utils.ConcatenatedKey(utils.Meta, utils.Meta, strconv.Itoa(time.Now().Second())), + StartTime: utils.ConcatenatedKey(utils.Meta, utils.Meta, strconv.Itoa(currentTime.Second())), EndTime: "", }, true case utils.MetaHourly: @@ -896,7 +897,7 @@ func checkDefaultTiming(tStr string) (rTm *engine.RITiming, isDefault bool) { Months: utils.Months{}, MonthDays: utils.MonthDays{}, WeekDays: utils.WeekDays{}, - StartTime: utils.ConcatenatedKey(utils.Meta, strconv.Itoa(time.Now().Minute()), strconv.Itoa(time.Now().Second())), + StartTime: utils.ConcatenatedKey(utils.Meta, strconv.Itoa(currentTime.Minute()), strconv.Itoa(currentTime.Second())), EndTime: "", }, true case utils.MetaDaily: @@ -906,7 +907,7 @@ func checkDefaultTiming(tStr string) (rTm *engine.RITiming, isDefault bool) { Months: utils.Months{}, MonthDays: utils.MonthDays{}, WeekDays: utils.WeekDays{}, - StartTime: startTime, + StartTime: fmtTime, EndTime: ""}, true case utils.MetaWeekly: return &engine.RITiming{ @@ -914,8 +915,8 @@ func checkDefaultTiming(tStr string) (rTm *engine.RITiming, isDefault bool) { Years: utils.Years{}, Months: utils.Months{}, MonthDays: utils.MonthDays{}, - WeekDays: utils.WeekDays{time.Now().Weekday()}, - StartTime: startTime, + WeekDays: utils.WeekDays{currentTime.Weekday()}, + StartTime: fmtTime, EndTime: "", }, true case utils.MetaMonthly: @@ -923,9 +924,9 @@ func checkDefaultTiming(tStr string) (rTm *engine.RITiming, isDefault bool) { ID: utils.MetaMonthly, Years: utils.Years{}, Months: utils.Months{}, - MonthDays: utils.MonthDays{time.Now().Day()}, + MonthDays: utils.MonthDays{currentTime.Day()}, WeekDays: utils.WeekDays{}, - StartTime: startTime, + StartTime: fmtTime, EndTime: "", }, true case utils.MetaMonthlyEstimated: @@ -933,9 +934,9 @@ func checkDefaultTiming(tStr string) (rTm *engine.RITiming, isDefault bool) { ID: utils.MetaMonthlyEstimated, Years: utils.Years{}, Months: utils.Months{}, - MonthDays: utils.MonthDays{time.Now().Day()}, + MonthDays: utils.MonthDays{currentTime.Day()}, WeekDays: utils.WeekDays{}, - StartTime: startTime, + StartTime: fmtTime, EndTime: "", }, true case utils.MetaMonthEnd: @@ -945,17 +946,17 @@ func checkDefaultTiming(tStr string) (rTm *engine.RITiming, isDefault bool) { Months: utils.Months{}, MonthDays: utils.MonthDays{-1}, WeekDays: utils.WeekDays{}, - StartTime: startTime, + StartTime: fmtTime, EndTime: "", }, true case utils.MetaYearly: return &engine.RITiming{ ID: utils.MetaYearly, Years: utils.Years{}, - Months: utils.Months{time.Now().Month()}, - MonthDays: utils.MonthDays{time.Now().Day()}, + Months: utils.Months{currentTime.Month()}, + MonthDays: utils.MonthDays{currentTime.Day()}, WeekDays: utils.WeekDays{}, - StartTime: startTime, + StartTime: fmtTime, EndTime: "", }, true default: diff --git a/engine/rateinterval.go b/engine/rateinterval.go index 14e468c78..a1ed6b29a 100644 --- a/engine/rateinterval.go +++ b/engine/rateinterval.go @@ -70,9 +70,15 @@ func (rit *RITiming) CronString() string { } else { hour, min, sec = "*", "*", "*" } - hour = strings.TrimPrefix(hour, "0") - min = strings.TrimPrefix(min, "0") - sec = strings.TrimPrefix(sec, "0") + if len(hour) > 1 { + hour = strings.TrimPrefix(hour, "0") + } + if len(min) > 1 { + min = strings.TrimPrefix(min, "0") + } + if len(sec) > 1 { + sec = strings.TrimPrefix(sec, "0") + } } if len(rit.MonthDays) == 0 { monthday = "*" diff --git a/engine/rateinterval_test.go b/engine/rateinterval_test.go index 8402e6ab2..1e8a952a8 100644 --- a/engine/rateinterval_test.go +++ b/engine/rateinterval_test.go @@ -296,73 +296,138 @@ func TestRateStrigyfy(t *testing.T) { } } -func TestRateIntervalCronAll(t *testing.T) { - rit := &RITiming{ - Years: utils.Years{2012}, - Months: utils.Months{time.February}, - MonthDays: utils.MonthDays{1}, - WeekDays: []time.Weekday{time.Sunday}, - StartTime: "14:30:00", +func TestRateInterval_CronStringTT(t *testing.T) { + testCases := []struct { + name string + ri *RITiming + wantCron string + }{ + { + name: "SingleYearMonthMonthDayWeekDay", + ri: &RITiming{ + Years: utils.Years{2012}, + Months: utils.Months{time.February}, + MonthDays: utils.MonthDays{1}, + WeekDays: []time.Weekday{time.Sunday}, + StartTime: "14:30:00", + }, + wantCron: "0 30 14 1 2 0 2012", + }, + { + name: "MultipleYearsMonthsMonthsDaysWeekDays", + ri: &RITiming{ + Years: utils.Years{2012, 2014}, + Months: utils.Months{time.February, time.January}, + MonthDays: utils.MonthDays{15, 16}, + WeekDays: []time.Weekday{time.Sunday, time.Monday}, + StartTime: "14:30:00", + }, + wantCron: "0 30 14 15,16 2,1 0,1 2012,2014", + }, + { + name: "WildcardStartTime", + ri: &RITiming{ + StartTime: "*:30:00", + }, + wantCron: "0 30 * * * * *", + }, + { + name: "AllFieldsEmpty", + ri: &RITiming{}, + wantCron: "* * * * * * *", + }, + { + name: "EveryMinute", + ri: &RITiming{ + StartTime: utils.MetaEveryMinute, + }, + wantCron: "0 * * * * * *", + }, + { + name: "Hourly", + ri: &RITiming{ + StartTime: utils.MetaHourly, + }, + wantCron: "0 0 * * * * *", + }, + { + name: "SingleDigit0Hour", + ri: &RITiming{ + StartTime: "0:49:25", + }, + wantCron: "25 49 0 * * * *", + }, + { + name: "SingleDigit0Minute", + ri: &RITiming{ + StartTime: "*:0:49", + }, + wantCron: "49 0 * * * * *", + }, + { + name: "SingleDigit0Second", + ri: &RITiming{ + StartTime: "*:49:0", + }, + wantCron: "0 49 * * * * *", + }, + { + name: "DoubleDigit0Hour", + ri: &RITiming{ + StartTime: "00:49:25", + }, + wantCron: "25 49 0 * * * *", + }, + { + name: "DoubleDigit0Minute", + ri: &RITiming{ + StartTime: "*:00:49", + }, + wantCron: "49 0 * * * * *", + }, + { + name: "DoubleDigit0Second", + ri: &RITiming{ + StartTime: "*:49:00", + }, + wantCron: "0 49 * * * * *", + }, + { + name: "InvalidStartTimeFormat", + ri: &RITiming{ + StartTime: "223000", + }, + wantCron: "* * * * * * *", + }, + { + name: "LastDayOfMonthSpecifiedAsNegative", + ri: &RITiming{ + StartTime: "223000", + MonthDays: utils.MonthDays{-1}, + }, + wantCron: "* * * L * * *", + }, + { + name: "PatternWithZeros", + ri: &RITiming{ + Years: utils.Years{2020}, + Months: utils.Months{1}, + MonthDays: utils.MonthDays{0}, + WeekDays: []time.Weekday{time.Monday}, + StartTime: "00:00:00", + }, + wantCron: "0 0 0 0 1 1 2020", + }, } - expected := "0 30 14 1 2 0 2012" - cron := rit.CronString() - if cron != expected { - t.Errorf("Expected %s was %s", expected, cron) - } -} -func TestRateIntervalCronMultiple(t *testing.T) { - rit := &RITiming{ - Years: utils.Years{2012, 2014}, - Months: utils.Months{time.February, time.January}, - MonthDays: utils.MonthDays{15, 16}, - WeekDays: []time.Weekday{time.Sunday, time.Monday}, - StartTime: "14:30:00", - } - expected := "0 30 14 15,16 2,1 0,1 2012,2014" - cron := rit.CronString() - - if cron != expected { - t.Errorf("Expected %s was %s", expected, cron) - } -} - -func TestRateIntervalCronStar(t *testing.T) { - rit := &RITiming{ - StartTime: "*:30:00", - } - expected := "0 30 * * * * *" - cron := rit.CronString() - - if cron != expected { - t.Errorf("Expected %s was %s", expected, cron) - } -} - -func TestRateIntervalCronEmpty(t *testing.T) { - rit := &RITiming{} - expected := "* * * * * * *" - cron := rit.CronString() - - if cron != expected { - t.Errorf("Expected %s was %s", expected, cron) - } -} - -func TestRITimingCronEveryX(t *testing.T) { - rit := &RITiming{ - StartTime: utils.MetaEveryMinute, - } - eCronStr := "0 * * * * * *" - if cronStr := rit.CronString(); cronStr != eCronStr { - t.Errorf("Expecting: <%s>, received: <%s>", eCronStr, cronStr) - } - rit = &RITiming{ - StartTime: utils.MetaHourly, - } - eCronStr = "0 0 * * * * *" - if cronStr := rit.CronString(); cronStr != eCronStr { - t.Errorf("Expecting: <%s>, received: <%s>", eCronStr, cronStr) + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got := tc.ri.CronString() + if got != tc.wantCron { + t.Errorf("RITiming=%s\nCronString()=%q, want %q", utils.ToJSON(tc.ri), got, tc.wantCron) + } + }) } } @@ -576,27 +641,6 @@ func BenchmarkRateIntervalContainsDate(b *testing.B) { } } -func TestRateIntervalCronStringDefault(t *testing.T) { - rit := &RITiming{ - StartTime: "223000", - } - cron := rit.CronString() - if !reflect.DeepEqual(cron, "* * * * * * *") { - t.Errorf("\nExpecting: <* * * * * * *>,\n Received: <%+v>", cron) - } -} - -func TestRateIntervalCronStringMonthDayNegative(t *testing.T) { - rit := &RITiming{ - StartTime: "223000", - MonthDays: utils.MonthDays{-1}, - } - cron := rit.CronString() - if !reflect.DeepEqual(cron, "* * * L * * *") { - t.Errorf("\nExpecting: <* * * L * * *>,\n Received: <%+v>", cron) - } -} - func TestRateIntervalIsActiveAt(t *testing.T) { rit := &RITiming{} cronActive := rit.IsActive() @@ -620,7 +664,7 @@ func TestRateIntervalFieldAsInterfaceError(t *testing.T) { Value: 2.2, } _, err := rateTest.FieldAsInterface([]string{"FALSE"}) - if err == nil && err.Error() != "unsupported field prefix: " { + if err == nil || err.Error() != "unsupported field prefix: " { t.Errorf("\nExpecting: >,\n Received: <%+v>", err) } } @@ -629,7 +673,7 @@ func TestRateIntervalFieldAsInterfaceError2(t *testing.T) { rateTest := &RGRate{} _, err := rateTest.FieldAsInterface([]string{"value1", "value2"}) - if err == nil && err != utils.ErrNotFound { + if err != utils.ErrNotFound { t.Errorf("\nExpecting: ,\n Received: <%+v>", err) } } diff --git a/engine/tpreader.go b/engine/tpreader.go index e5f157103..606261f14 100644 --- a/engine/tpreader.go +++ b/engine/tpreader.go @@ -2398,13 +2398,14 @@ func (tpr *TpReader) addDefaultTimings() { StartTime: utils.MetaASAP, EndTime: "", } + currentTime := time.Now() tpr.timings[utils.MetaEveryMinute] = &utils.TPTiming{ ID: utils.MetaEveryMinute, Years: utils.Years{}, Months: utils.Months{}, MonthDays: utils.MonthDays{}, WeekDays: utils.WeekDays{}, - StartTime: utils.ConcatenatedKey(utils.Meta, utils.Meta, strconv.Itoa(time.Now().Second())), + StartTime: utils.ConcatenatedKey(utils.Meta, utils.Meta, strconv.Itoa(currentTime.Second())), EndTime: "", } tpr.timings[utils.MetaHourly] = &utils.TPTiming{ @@ -2413,17 +2414,17 @@ func (tpr *TpReader) addDefaultTimings() { Months: utils.Months{}, MonthDays: utils.MonthDays{}, WeekDays: utils.WeekDays{}, - StartTime: utils.ConcatenatedKey(utils.Meta, strconv.Itoa(time.Now().Minute()), strconv.Itoa(time.Now().Second())), + StartTime: utils.ConcatenatedKey(utils.Meta, strconv.Itoa(currentTime.Minute()), strconv.Itoa(currentTime.Second())), EndTime: "", } - startTime := time.Now().Format("15:04:05") + fmtTime := currentTime.Format("15:04:05") tpr.timings[utils.MetaDaily] = &utils.TPTiming{ ID: utils.MetaDaily, Years: utils.Years{}, Months: utils.Months{}, MonthDays: utils.MonthDays{}, WeekDays: utils.WeekDays{}, - StartTime: startTime, + StartTime: fmtTime, EndTime: "", } tpr.timings[utils.MetaWeekly] = &utils.TPTiming{ @@ -2431,26 +2432,26 @@ func (tpr *TpReader) addDefaultTimings() { Years: utils.Years{}, Months: utils.Months{}, MonthDays: utils.MonthDays{}, - WeekDays: utils.WeekDays{time.Now().Weekday()}, - StartTime: startTime, + WeekDays: utils.WeekDays{currentTime.Weekday()}, + StartTime: fmtTime, EndTime: "", } tpr.timings[utils.MetaMonthly] = &utils.TPTiming{ ID: utils.MetaMonthly, Years: utils.Years{}, Months: utils.Months{}, - MonthDays: utils.MonthDays{time.Now().Day()}, + MonthDays: utils.MonthDays{currentTime.Day()}, WeekDays: utils.WeekDays{}, - StartTime: startTime, + StartTime: fmtTime, EndTime: "", } tpr.timings[utils.MetaMonthlyEstimated] = &utils.TPTiming{ ID: utils.MetaMonthlyEstimated, Years: utils.Years{}, Months: utils.Months{}, - MonthDays: utils.MonthDays{time.Now().Day()}, + MonthDays: utils.MonthDays{currentTime.Day()}, WeekDays: utils.WeekDays{}, - StartTime: startTime, + StartTime: fmtTime, EndTime: "", } tpr.timings[utils.MetaMonthEnd] = &utils.TPTiming{ @@ -2459,16 +2460,16 @@ func (tpr *TpReader) addDefaultTimings() { Months: utils.Months{}, MonthDays: utils.MonthDays{-1}, WeekDays: utils.WeekDays{}, - StartTime: startTime, + StartTime: fmtTime, EndTime: "", } tpr.timings[utils.MetaYearly] = &utils.TPTiming{ ID: utils.MetaYearly, Years: utils.Years{}, - Months: utils.Months{time.Now().Month()}, - MonthDays: utils.MonthDays{time.Now().Day()}, + Months: utils.Months{currentTime.Month()}, + MonthDays: utils.MonthDays{currentTime.Day()}, WeekDays: utils.WeekDays{}, - StartTime: startTime, + StartTime: fmtTime, EndTime: "", }