From 4409f6fe0c6b0d0af514ac5a4e283185c4d644ef Mon Sep 17 00:00:00 2001 From: Nima Mashhadi Date: Thu, 1 Dec 2022 22:37:29 +0330 Subject: [PATCH 1/8] add validation function --- cron.go | 212 +++++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 201 insertions(+), 11 deletions(-) diff --git a/cron.go b/cron.go index 4683864..32cd5c2 100644 --- a/cron.go +++ b/cron.go @@ -5,6 +5,7 @@ import ( "fmt" "math" "math/bits" + "regexp" "strconv" "strings" "time" @@ -28,19 +29,28 @@ const ( ) type fieldType struct { - Field cronField - MinValue int - MaxValue int + Field cronField + MinValue int + MaxValue int + SpecialCharacters map[string]struct{} } var ( - nanoSecond = fieldType{cronFieldNanoSecond, 0, 999999999} - second = fieldType{cronFieldSecond, 0, 59} - minute = fieldType{cronFieldMinute, 0, 59} - hour = fieldType{cronFieldHour, 0, 23} - dayOfMonth = fieldType{cronFieldDayOfMonth, 1, 31} - month = fieldType{cronFieldMonth, 1, 12} - dayOfWeek = fieldType{cronFieldDayOfWeek, 1, 7} + nanoSecond = fieldType{cronFieldNanoSecond, 0, 999999999, + map[string]struct{}{ + ",": struct{}{}, "*": struct{}{}, "/": struct{}{}, "-": struct{}{}}} + second = fieldType{cronFieldSecond, 0, 59, map[string]struct{}{ + ",": struct{}{}, "*": struct{}{}, "/": struct{}{}, "-": struct{}{}}} + minute = fieldType{cronFieldMinute, 0, 59, map[string]struct{}{ + ",": struct{}{}, "*": struct{}{}, "/": struct{}{}, "-": struct{}{}}} + hour = fieldType{cronFieldHour, 0, 23, map[string]struct{}{ + ",": struct{}{}, "*": struct{}{}, "/": struct{}{}, "-": struct{}{}}} + dayOfMonth = fieldType{cronFieldDayOfMonth, 1, 31, map[string]struct{}{ + ",": struct{}{}, "*": struct{}{}, "/": struct{}{}, "-": struct{}{}, "L": struct{}{}, "W": struct{}{}, "?": struct{}{}}} + month = fieldType{cronFieldMonth, 1, 12, map[string]struct{}{ + ",": struct{}{}, "*": struct{}{}, "/": struct{}{}, "-": struct{}{}}} + dayOfWeek = fieldType{cronFieldDayOfWeek, 1, 7, map[string]struct{}{ + ",": struct{}{}, "*": struct{}{}, "/": struct{}{}, "-": struct{}{}, "L": struct{}{}, "#": struct{}{}, "?": struct{}{}}} ) var cronFieldTypes = []fieldType{ @@ -229,7 +239,6 @@ func parseField(value string, fieldType fieldType) (*cronFieldBits, error) { if step <= 0 { return nil, fmt.Errorf("step must be 1 or higher in \"%s\"", value) } - } else { var err error valueRange, err = parseRange(field, fieldType) @@ -446,3 +455,184 @@ func getFieldMaxValue(t time.Time, fieldType fieldType) int { func isLeapYear(year int) bool { return year%400 == 0 || year%100 != 0 && year%4 == 0 } + +// IsValid returns nil if the cron expression is valid +func IsValid(expression string) error { + if len(expression) == 0 { + return errors.New("cron expression must not be empty") + } + + fields := strings.Fields(expression) + + if len(fields) != 6 { + if len(fields) == 7 { + return errors.New("cron expression must consist of 6 fields: Chrono isn't support for 7 fields") + } + return fmt.Errorf("cron expression must consist of 6 fields: found %d fields in \"%s\"", len(fields), expression) + } + + for i, field := range fields { + err := validateSubExpression(field, cronFieldTypes[i], cronFieldTypes[i].SpecialCharacters) + if err != nil { + return err + } + } + + return nil +} + +func validateSubExpression(subExpression string, fieldType fieldType, specialCharacters map[string]struct{}) error { + specialCharactersTmp := specialCharacters + numberMatched, err := regexp.MatchString("^[0-9]+$", subExpression) + if err != nil { + return err + } + + stringMatched, err := regexp.MatchString("^[a-z,A-Z]+$", strings.ToUpper(subExpression)) + if err != nil { + return err + } + + if strings.Contains(subExpression, ",") { + if _, ok := specialCharactersTmp[","]; !ok { + return fmt.Errorf("the character %q is not allowed in field %s", "\",\"", fieldType.Field) + } + subExp := strings.Split(subExpression, ",") + + delete(specialCharactersTmp, ",") + delete(specialCharactersTmp, "*") + delete(specialCharactersTmp, "?") + for _, subField := range subExp { + err := validateSubExpression(subField, fieldType, specialCharactersTmp) + if err != nil { + return err + } + } + } else if strings.Contains(subExpression, "-") { + if _, ok := specialCharactersTmp["-"]; !ok { + return fmt.Errorf("the character %q is not allowed in field %s", "\"-\"", fieldType.Field) + } + subExp := strings.Split(subExpression, "-") + if len(subExp) != 2 { + return fmt.Errorf("invalid cron expression: %s", subExpression) + } + delete(specialCharactersTmp, ",") + delete(specialCharactersTmp, "*") + delete(specialCharactersTmp, "/") + delete(specialCharactersTmp, "-") + delete(specialCharactersTmp, "?") + err := validateSubExpression(subExp[0], fieldType, specialCharactersTmp) + if err != nil { + return err + } + err = validateSubExpression(subExp[1], fieldType, specialCharactersTmp) + if err != nil { + return err + } + } else if strings.Contains(subExpression, "/") { + if _, ok := specialCharactersTmp["/"]; !ok { + return fmt.Errorf("the character %q is not allowed in field %s", "\"/\"", fieldType.Field) + } + subExp := strings.Split(subExpression, "/") + if len(subExp) != 2 { + return fmt.Errorf("invalid cron expression: %s", subExpression) + } + delete(specialCharactersTmp, ",") + delete(specialCharactersTmp, "/") + delete(specialCharactersTmp, "-") + delete(specialCharactersTmp, "#") + delete(specialCharactersTmp, "?") + delete(specialCharactersTmp, "L") + delete(specialCharactersTmp, "W") + specialCharactersTmp["*"] = struct{}{} + err := validateSubExpression(subExp[0], fieldType, specialCharactersTmp) + if err != nil { + return err + } + + delete(specialCharactersTmp, "*") + err = validateSubExpression(subExp[1], fieldType, specialCharactersTmp) + if err != nil { + return err + } + } else if strings.Contains(subExpression, "#") { + if _, ok := specialCharactersTmp["#"]; !ok { + return fmt.Errorf("the character %q is not allowed in field %s", "\"#\"", fieldType.Field) + } + subExp := strings.Split(subExpression, "#") + if len(subExp) != 2 { + return fmt.Errorf("invalid cron expression: %s", subExpression) + } + err := validateSubExpression(subExp[0], fieldType, map[string]struct{}{}) + if err != nil { + return err + } + err = validateSubExpression(subExp[1], fieldType, map[string]struct{}{}) + if err != nil { + return err + } + } else if strings.Contains(subExpression, "L") && !strings.Contains(subExpression, "JUL") { + if _, ok := specialCharactersTmp["L"]; !ok { + return fmt.Errorf("the character %q is not allowed in field %s", "\"L\"", fieldType.Field) + } + + return errors.New("L is not supported") + } else if strings.Contains(subExpression, "W") { + if _, ok := specialCharactersTmp["W"]; !ok { + return fmt.Errorf("the character %q is not allowed in field %s", "\"W\"", fieldType.Field) + } + + return errors.New("W is not supported") + } else if strings.Contains(subExpression, "?") { + if _, ok := specialCharactersTmp["?"]; !ok { + return fmt.Errorf("the character %q is not allowed in field %s", "\"?\"", fieldType.Field) + } + + return errors.New("? is not supported") + } else if strings.Contains(subExpression, "*") { + if _, ok := specialCharactersTmp["*"]; !ok { + return fmt.Errorf("the character %q is not allowed in field %s", "\"*\"", fieldType.Field) + } + + return nil + } else if numberMatched { + value, err := strconv.Atoi(subExpression) + if err != nil { + return err + } + if value < fieldType.MinValue || value > fieldType.MaxValue { + return fmt.Errorf("value %d is out of range for field %s", value, fieldType.Field) + } + } else if stringMatched { + if fieldType.Field != cronFieldMonth && fieldType.Field != cronFieldDayOfWeek { + return fmt.Errorf("invalid cron expression: %s", subExpression) + } + if fieldType.Field == cronFieldMonth { + find := false + for _, month := range months { + if month == strings.ToUpper(subExpression) { + find = true + break + } + } + if !find { + return fmt.Errorf("invalid cron expression: %s", subExpression) + } + } else if fieldType.Field == cronFieldDayOfWeek { + find := false + for _, day := range days { + if day == strings.ToUpper(subExpression) { + find = true + break + } + } + if !find { + return fmt.Errorf("invalid cron expression: %s", subExpression) + } + } + } else { + return fmt.Errorf("invalid cron expression: %s", subExpression) + } + + return nil +} From 6a2f1e5ddffc0c47311e9b23b45a33434c6f53ff Mon Sep 17 00:00:00 2001 From: Nima Mashhadi Date: Fri, 2 Dec 2022 00:37:29 +0330 Subject: [PATCH 2/8] update validation function --- cron.go | 71 +++++++++++++++++++++++++++++++-------------------------- 1 file changed, 39 insertions(+), 32 deletions(-) diff --git a/cron.go b/cron.go index 32cd5c2..441665a 100644 --- a/cron.go +++ b/cron.go @@ -482,7 +482,11 @@ func IsValid(expression string) error { } func validateSubExpression(subExpression string, fieldType fieldType, specialCharacters map[string]struct{}) error { - specialCharactersTmp := specialCharacters + specialCharactersTmp := make(map[string]struct{}) + for k, v := range specialCharacters { + specialCharactersTmp[k] = v + } + numberMatched, err := regexp.MatchString("^[0-9]+$", subExpression) if err != nil { return err @@ -494,8 +498,8 @@ func validateSubExpression(subExpression string, fieldType fieldType, specialCha } if strings.Contains(subExpression, ",") { - if _, ok := specialCharactersTmp[","]; !ok { - return fmt.Errorf("the character %q is not allowed in field %s", "\",\"", fieldType.Field) + if _, ok := specialCharacters[","]; !ok { + return fmt.Errorf("the character %s is not allowed in field %s", "\",\"", fieldType.Field) } subExp := strings.Split(subExpression, ",") @@ -508,56 +512,58 @@ func validateSubExpression(subExpression string, fieldType fieldType, specialCha return err } } - } else if strings.Contains(subExpression, "-") { - if _, ok := specialCharactersTmp["-"]; !ok { - return fmt.Errorf("the character %q is not allowed in field %s", "\"-\"", fieldType.Field) + } else if strings.Contains(subExpression, "/") { + if _, ok := specialCharacters["/"]; !ok { + return fmt.Errorf("the character %s is not allowed in field %s", "\"/\"", fieldType.Field) } - subExp := strings.Split(subExpression, "-") + subExp := strings.Split(subExpression, "/") if len(subExp) != 2 { return fmt.Errorf("invalid cron expression: %s", subExpression) } delete(specialCharactersTmp, ",") - delete(specialCharactersTmp, "*") delete(specialCharactersTmp, "/") - delete(specialCharactersTmp, "-") + delete(specialCharactersTmp, "#") delete(specialCharactersTmp, "?") + specialCharactersTmp["*"] = struct{}{} err := validateSubExpression(subExp[0], fieldType, specialCharactersTmp) if err != nil { return err } - err = validateSubExpression(subExp[1], fieldType, specialCharactersTmp) + + delete(specialCharactersTmp, "*") + delete(specialCharactersTmp, "L") + delete(specialCharactersTmp, "W") + fieldTypeTmp := fieldType + fieldTypeTmp.MinValue = 1 + fieldTypeTmp.Field = cronField("step of " + string(fieldType.Field)) + err = validateSubExpression(subExp[1], fieldTypeTmp, specialCharactersTmp) if err != nil { return err } - } else if strings.Contains(subExpression, "/") { - if _, ok := specialCharactersTmp["/"]; !ok { - return fmt.Errorf("the character %q is not allowed in field %s", "\"/\"", fieldType.Field) + } else if strings.Contains(subExpression, "-") { + if _, ok := specialCharacters["-"]; !ok { + return fmt.Errorf("the character %s is not allowed in field %s", "-", fieldType.Field) } - subExp := strings.Split(subExpression, "/") + subExp := strings.Split(subExpression, "-") if len(subExp) != 2 { return fmt.Errorf("invalid cron expression: %s", subExpression) } delete(specialCharactersTmp, ",") + delete(specialCharactersTmp, "*") delete(specialCharactersTmp, "/") delete(specialCharactersTmp, "-") - delete(specialCharactersTmp, "#") delete(specialCharactersTmp, "?") - delete(specialCharactersTmp, "L") - delete(specialCharactersTmp, "W") - specialCharactersTmp["*"] = struct{}{} err := validateSubExpression(subExp[0], fieldType, specialCharactersTmp) if err != nil { return err } - - delete(specialCharactersTmp, "*") err = validateSubExpression(subExp[1], fieldType, specialCharactersTmp) if err != nil { return err } } else if strings.Contains(subExpression, "#") { - if _, ok := specialCharactersTmp["#"]; !ok { - return fmt.Errorf("the character %q is not allowed in field %s", "\"#\"", fieldType.Field) + if _, ok := specialCharacters["#"]; !ok { + return fmt.Errorf("the character %s is not allowed in field %s", "#", fieldType.Field) } subExp := strings.Split(subExpression, "#") if len(subExp) != 2 { @@ -572,26 +578,26 @@ func validateSubExpression(subExpression string, fieldType fieldType, specialCha return err } } else if strings.Contains(subExpression, "L") && !strings.Contains(subExpression, "JUL") { - if _, ok := specialCharactersTmp["L"]; !ok { - return fmt.Errorf("the character %q is not allowed in field %s", "\"L\"", fieldType.Field) + if _, ok := specialCharacters["L"]; !ok { + return fmt.Errorf("the character %s is not allowed in field %s", "L", fieldType.Field) } return errors.New("L is not supported") } else if strings.Contains(subExpression, "W") { - if _, ok := specialCharactersTmp["W"]; !ok { - return fmt.Errorf("the character %q is not allowed in field %s", "\"W\"", fieldType.Field) + if _, ok := specialCharacters["W"]; !ok { + return fmt.Errorf("the character %s is not allowed in field %s", "\"W\"", fieldType.Field) } return errors.New("W is not supported") } else if strings.Contains(subExpression, "?") { - if _, ok := specialCharactersTmp["?"]; !ok { - return fmt.Errorf("the character %q is not allowed in field %s", "\"?\"", fieldType.Field) + if _, ok := specialCharacters["?"]; !ok { + return fmt.Errorf("the character %s is not allowed in field %s", "\"?\"", fieldType.Field) } return errors.New("? is not supported") } else if strings.Contains(subExpression, "*") { - if _, ok := specialCharactersTmp["*"]; !ok { - return fmt.Errorf("the character %q is not allowed in field %s", "\"*\"", fieldType.Field) + if _, ok := specialCharacters["*"]; !ok { + return fmt.Errorf("the character %s is not allowed in field %s", "*", fieldType.Field) } return nil @@ -601,11 +607,12 @@ func validateSubExpression(subExpression string, fieldType fieldType, specialCha return err } if value < fieldType.MinValue || value > fieldType.MaxValue { - return fmt.Errorf("value %d is out of range for field %s", value, fieldType.Field) + return fmt.Errorf("the value %d in %s must be between %d and %d", + value, fieldType.Field, fieldType.MinValue, fieldType.MaxValue) } } else if stringMatched { if fieldType.Field != cronFieldMonth && fieldType.Field != cronFieldDayOfWeek { - return fmt.Errorf("invalid cron expression: %s", subExpression) + return fmt.Errorf("the value in %s must be number: %s", fieldType.Field, subExpression) } if fieldType.Field == cronFieldMonth { find := false From 4c02d4def99f2213fb12475313ee5ce278d705e9 Mon Sep 17 00:00:00 2001 From: Nima Mashhadi Date: Fri, 2 Dec 2022 00:37:49 +0330 Subject: [PATCH 3/8] add tests for new errors --- cron_test.go | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/cron_test.go b/cron_test.go index 30c8cb2..a97a28b 100644 --- a/cron_test.go +++ b/cron_test.go @@ -1,9 +1,10 @@ package chrono import ( - "github.com/stretchr/testify/assert" "testing" "time" + + "github.com/stretchr/testify/assert" ) const timeLayout = "2006-01-02 15:04:05" @@ -631,21 +632,26 @@ func TestParseCronExpression_Errors(t *testing.T) { errorString string }{ {expression: "", errorString: "cron expression must not be empty"}, - {expression: "test * * * * *", errorString: "the value in field SECOND must be number : test"}, - {expression: "5 * * * *", errorString: "cron expression must consist of 6 fields : found 5 in \"5 * * * *\""}, - {expression: "61 * * * * *", errorString: "the value in field SECOND must be between 0 and 59"}, - {expression: "61 * * * * *", errorString: "the value in field SECOND must be between 0 and 59"}, - {expression: "* 65 * * * *", errorString: "the value in field MINUTE must be between 0 and 59"}, - {expression: "* * * 0 * *", errorString: "the value in field DAY_OF_MONTH must be between 1 and 31"}, - {expression: "* * 1-12/0 * * *", errorString: "step must be 1 or higher in \"1-12/0\""}, - {expression: "* * 0-32/5 * * *", errorString: "the value in field HOUR must be between 0 and 23"}, - {expression: "* * * * 0-10/2 *", errorString: "the value in field MONTH must be between 1 and 12"}, - {expression: "* * 1-12/test * * *", errorString: "step must be number : \"test\""}, + {expression: "test * * * * *", errorString: "the value in SECOND must be number: test"}, + {expression: "5 * * * *", errorString: "cron expression must consist of 6 fields: found 5 fields in \"5 * * * *\""}, + {expression: "61 * * * * *", errorString: "the value 61 in SECOND must be between 0 and 59"}, + {expression: "* 65 * * * *", errorString: "the value 65 in MINUTE must be between 0 and 59"}, + {expression: "* * * 0 * *", errorString: "the value 0 in DAY_OF_MONTH must be between 1 and 31"}, + {expression: "* * 1-12/0 * * *", errorString: "the value 0 in step of HOUR must be between 1 and 23"}, + {expression: "* * 0-32/5 * * *", errorString: "the value 32 in HOUR must be between 0 and 23"}, + {expression: "* * * * 0-10/2 *", errorString: "the value 0 in MONTH must be between 1 and 12"}, + {expression: "* * 1-12/test * * *", errorString: "the value in step of HOUR must be number: test"}, + {expression: "* * * L * *", errorString: "L is not supported"}, + {expression: "* * * 1W * *", errorString: "W is not supported"}, + {expression: "* * * ? * *", errorString: "? is not supported"}, + {expression: "* * * * * L/2", errorString: "L is not supported"}, + {expression: "* * * * * 2/", errorString: "invalid cron expression: "}, + {expression: "* * * * * 2/2/2", errorString: "invalid cron expression: 2/2/2"}, + {expression: "* 2,3,5,6,* * * * *", errorString: "the character * is not allowed in field MINUTE"}, } for _, testCase := range testCases { - exp, err := ParseCronExpression(testCase.expression) - assert.Nil(t, exp, "expression must have been parsed : %s", testCase.expression) + err := IsValid(testCase.expression) assert.NotNil(t, err, "an error must have been occurred") assert.Equal(t, testCase.errorString, err.Error(), "error string must not match, expected : %s, actual :%s", testCase.errorString, err.Error()) From 4256558e2432dc2a9b216529df1bf8202439367a Mon Sep 17 00:00:00 2001 From: Nima Mashhadi Date: Fri, 2 Dec 2022 00:42:08 +0330 Subject: [PATCH 4/8] add tests for new errors --- cron.go | 2 +- cron_test.go | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/cron.go b/cron.go index 441665a..e90670c 100644 --- a/cron.go +++ b/cron.go @@ -468,7 +468,7 @@ func IsValid(expression string) error { if len(fields) == 7 { return errors.New("cron expression must consist of 6 fields: Chrono isn't support for 7 fields") } - return fmt.Errorf("cron expression must consist of 6 fields: found %d fields in \"%s\"", len(fields), expression) + return fmt.Errorf("cron expression must consist of 6 fields: found %d fields in '%s'", len(fields), expression) } for i, field := range fields { diff --git a/cron_test.go b/cron_test.go index a97a28b..8d9eedf 100644 --- a/cron_test.go +++ b/cron_test.go @@ -632,8 +632,10 @@ func TestParseCronExpression_Errors(t *testing.T) { errorString string }{ {expression: "", errorString: "cron expression must not be empty"}, + {expression: "* *", errorString: "cron expression must consist of 6 fields: found 2 fields in '* *'"}, + {expression: "* * * * * * *", errorString: "cron expression must consist of 6 fields: Chrono isn't support for 7 fields"}, {expression: "test * * * * *", errorString: "the value in SECOND must be number: test"}, - {expression: "5 * * * *", errorString: "cron expression must consist of 6 fields: found 5 fields in \"5 * * * *\""}, + {expression: "5 * * * *", errorString: "cron expression must consist of 6 fields: found 5 fields in '5 * * * *'"}, {expression: "61 * * * * *", errorString: "the value 61 in SECOND must be between 0 and 59"}, {expression: "* 65 * * * *", errorString: "the value 65 in MINUTE must be between 0 and 59"}, {expression: "* * * 0 * *", errorString: "the value 0 in DAY_OF_MONTH must be between 1 and 31"}, From c63ed264211a63fb97aca085d8c12436603421e6 Mon Sep 17 00:00:00 2001 From: Nima Mashhadi Date: Fri, 2 Dec 2022 00:48:39 +0330 Subject: [PATCH 5/8] remove old validations --- cron.go | 64 +++++++++++++++++++++------------------------------------ 1 file changed, 23 insertions(+), 41 deletions(-) diff --git a/cron.go b/cron.go index e90670c..f678a63 100644 --- a/cron.go +++ b/cron.go @@ -162,16 +162,13 @@ func (expression *CronExpression) nextField(field *cronFieldBits, t time.Time) t } func ParseCronExpression(expression string) (*CronExpression, error) { - if len(expression) == 0 { - return nil, errors.New("cron expression must not be empty") + err := IsValid(expression) + if err != nil { + return nil, err } fields := strings.Fields(expression) - if len(fields) != 6 { - return nil, fmt.Errorf("cron expression must consist of 6 fields : found %d in \"%s\"", len(fields), expression) - } - cronExpression := newCronExpression() for index, cronFieldType := range cronFieldTypes { @@ -194,10 +191,6 @@ func ParseCronExpression(expression string) (*CronExpression, error) { } func parseField(value string, fieldType fieldType) (*cronFieldBits, error) { - if len(value) == 0 { - return nil, fmt.Errorf("value must not be empty") - } - if fieldType.Field == cronFieldMonth { value = replaceOrdinals(value, months) } else if fieldType.Field == cronFieldDayOfWeek { @@ -231,14 +224,6 @@ func parseField(value string, fieldType fieldType) (*cronFieldBits, error) { stepStr := field[slashPos+1:] step, err = strconv.Atoi(stepStr) - - if err != nil { - return nil, fmt.Errorf("step must be number : \"%s\"", stepStr) - } - - if step <= 0 { - return nil, fmt.Errorf("step must be 1 or higher in \"%s\"", value) - } } else { var err error valueRange, err = parseRange(field, fieldType) @@ -272,8 +257,7 @@ func parseRange(value string, fieldType fieldType) (valueRange, error) { hyphenPos := strings.Index(value, "-") if hyphenPos == -1 { - result, err := checkValidValue(value, fieldType) - + result, err := strconv.Atoi(value) if err != nil { return valueRange{}, err } @@ -283,14 +267,12 @@ func parseRange(value string, fieldType fieldType) (valueRange, error) { maxStr := value[hyphenPos+1:] minStr := value[0:hyphenPos] - min, err := checkValidValue(minStr, fieldType) - + min, err := strconv.Atoi(minStr) if err != nil { return valueRange{}, err } - var max int - max, err = checkValidValue(maxStr, fieldType) + max, err := strconv.Atoi(maxStr) if err != nil { return valueRange{}, err } @@ -315,23 +297,23 @@ func replaceOrdinals(value string, list []string) string { return value } -func checkValidValue(value string, fieldType fieldType) (int, error) { - result, err := strconv.Atoi(value) - - if err != nil { - return 0, fmt.Errorf("the value in field %s must be number : %s", fieldType.Field, value) - } - - if fieldType.Field == cronFieldDayOfWeek && result == 0 { - return result, nil - } - - if result >= fieldType.MinValue && result <= fieldType.MaxValue { - return result, nil - } - - return 0, fmt.Errorf("the value in field %s must be between %d and %d", fieldType.Field, fieldType.MinValue, fieldType.MaxValue) -} +//func checkValidValue(value string, fieldType fieldType) (int, error) { +// result, err := strconv.Atoi(value) +// +// if err != nil { +// return 0, fmt.Errorf("the value in field %s must be number : %s", fieldType.Field, value) +// } +// +// if fieldType.Field == cronFieldDayOfWeek && result == 0 { +// return result, nil +// } +// +// if result >= fieldType.MinValue && result <= fieldType.MaxValue { +// return result, nil +// } +// +// return 0, fmt.Errorf("the value in field %s must be between %d and %d", fieldType.Field, fieldType.MinValue, fieldType.MaxValue) +//} func getTimeValue(t time.Time, field cronField) int { From 55e60db0b2c120a01aef3b5e90b56315f6f457a2 Mon Sep 17 00:00:00 2001 From: Nima Mashhadi Date: Fri, 2 Dec 2022 13:34:40 +0330 Subject: [PATCH 6/8] fix boundaries of day of week --- cron.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cron.go b/cron.go index f678a63..dd5d197 100644 --- a/cron.go +++ b/cron.go @@ -49,7 +49,7 @@ var ( ",": struct{}{}, "*": struct{}{}, "/": struct{}{}, "-": struct{}{}, "L": struct{}{}, "W": struct{}{}, "?": struct{}{}}} month = fieldType{cronFieldMonth, 1, 12, map[string]struct{}{ ",": struct{}{}, "*": struct{}{}, "/": struct{}{}, "-": struct{}{}}} - dayOfWeek = fieldType{cronFieldDayOfWeek, 1, 7, map[string]struct{}{ + dayOfWeek = fieldType{cronFieldDayOfWeek, 0, 6, map[string]struct{}{ ",": struct{}{}, "*": struct{}{}, "/": struct{}{}, "-": struct{}{}, "L": struct{}{}, "#": struct{}{}, "?": struct{}{}}} ) @@ -191,6 +191,10 @@ func ParseCronExpression(expression string) (*CronExpression, error) { } func parseField(value string, fieldType fieldType) (*cronFieldBits, error) { + if len(value) == 0 { + return nil, fmt.Errorf("value must not be empty") + } + if fieldType.Field == cronFieldMonth { value = replaceOrdinals(value, months) } else if fieldType.Field == cronFieldDayOfWeek { From 855709328295ea5429a009cd0e19ba107aaacec5 Mon Sep 17 00:00:00 2001 From: Nima Mashhadi Date: Fri, 2 Dec 2022 13:46:11 +0330 Subject: [PATCH 7/8] remove checkValidValue function --- cron.go | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/cron.go b/cron.go index dd5d197..706fe54 100644 --- a/cron.go +++ b/cron.go @@ -301,24 +301,6 @@ func replaceOrdinals(value string, list []string) string { return value } -//func checkValidValue(value string, fieldType fieldType) (int, error) { -// result, err := strconv.Atoi(value) -// -// if err != nil { -// return 0, fmt.Errorf("the value in field %s must be number : %s", fieldType.Field, value) -// } -// -// if fieldType.Field == cronFieldDayOfWeek && result == 0 { -// return result, nil -// } -// -// if result >= fieldType.MinValue && result <= fieldType.MaxValue { -// return result, nil -// } -// -// return 0, fmt.Errorf("the value in field %s must be between %d and %d", fieldType.Field, fieldType.MinValue, fieldType.MaxValue) -//} - func getTimeValue(t time.Time, field cronField) int { switch field { From 89814b2327e70ceffb09e9111e79933c8d9aefdb Mon Sep 17 00:00:00 2001 From: Nima Mashhadi Date: Fri, 2 Dec 2022 13:49:16 +0330 Subject: [PATCH 8/8] fix TestParseCronExpression_Errors test --- cron_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cron_test.go b/cron_test.go index 8d9eedf..3fc6912 100644 --- a/cron_test.go +++ b/cron_test.go @@ -653,7 +653,8 @@ func TestParseCronExpression_Errors(t *testing.T) { } for _, testCase := range testCases { - err := IsValid(testCase.expression) + exp, err := ParseCronExpression(testCase.expression) + assert.Nil(t, exp, "expression must have been parsed : %s", testCase.expression) assert.NotNil(t, err, "an error must have been occurred") assert.Equal(t, testCase.errorString, err.Error(), "error string must not match, expected : %s, actual :%s", testCase.errorString, err.Error())