Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Valid quartz range expression is considered invalid. Range validations should be contextual to cron definition. #35

Closed
Paqrat76 opened this issue Aug 20, 2015 · 3 comments

Comments

@Paqrat76
Copy link

Version 3.1.0
Quartz parser fails for day-of-week range of SUN-SAT. The Quartz definition appears correct but when the Quartz-based parser is created, the "DAY_OF_WEEK" constraints.stringMapping values are incorrect. MON = 1 even though mondayDoWValue = 2.

Use following test in CronParserQuartzIntegrationTest to reproduce:
@test
public void testSunToSat() {
// FAILS SUN-SAT: SUN = 7 and SAT = 6
parser.parse("0 0 12 ? * SUN-SAT");
}

Stack trace:
java.lang.IllegalArgumentException: Bad range defined! Defined range should satisfy from <= to, but was [%s, %s]
at com.cronutils.model.field.expression.Between.validate(Between.java:63)
at com.cronutils.model.field.expression.Between.(Between.java:39)
at com.cronutils.model.field.expression.Between.(Between.java:31)
at com.cronutils.parser.field.FieldParser.parseBetween(FieldParser.java:93)
at com.cronutils.parser.field.FieldParser.parse(FieldParser.java:69)
at com.cronutils.parser.field.CronParserField.parse(CronParserField.java:68)
at com.cronutils.parser.CronParser.parse(CronParser.java:96)
at com.cronutils.parser.CronParserQuartzIntegrationTest.testSunToSat(CronParserQuartzIntegrationTest.java:129)

refer to debug values below:
image

@jmrozanec
Copy link
Owner

@Paqrat76 Thank you for reporting this!
The example you present, it has two parts:

  • range values: SUN-SAT is considered invalid by the parser
  • Monday value mapping: DoW strings are mapped as MON->1 ... SUN->7, while Monday DoW value is considered 2.

Range values

Current cron-utils implementation considers ranges should be specified writing A-B, where the following property should be always met: A<=B. If A>B, range is considered invalid.
This is why the example you present fails to be parsed: SUN>SAT.
Despite at least some Unix implementations fail in such a case, Quartz executes normally. Current behavior is a bug for Quartz case. We will add flexibility on the range validation to be performed.

Monday value mapping

cron-utils always maps strings to values MON->1 ... SUN->7, regardless of cron implementation (See: FieldConstraintBuilder::daysOfWeekMapping).
Monday mapping specifies which concrete value is assigned to Monday by the cron specification. Example: Quartz considers SUN=1, MON=2. Using this knowledge, as well as if lowest value is zero or one, cron-utils will perform required mappings when necessary, translating internal representation to required values.
From the above, the fact that Monday string mapping and Monday reference value are not the same, is expected and does not affect the specific parser.

@jmrozanec jmrozanec changed the title Quartz parser fails valid schedule string Valid quartz range expression is considered invalid. Range validations should be contextual to cron definition. Sep 7, 2015
@jmrozanec
Copy link
Owner

@Paqrat76 issue solved. We rebuilt some parsing logic, and gained flexibility on how we represent crons and how we enforce validations.

@jmrozanec
Copy link
Owner

@Paqrat76 New version released! Please check details here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants