 |
History
| Log In
|
|
|
|
Use the following code to reproduce the problem.
It's not the parsing of the CronExpression that fails, it's the "getTimeAfter" - which is called by QuartzTrigger - as you can see in my original stacktrace. I didn't dig into the whole Quartz internals yet, but I'm still somewhat convinced that is has problems with cron expressions that go over midnight and should fire then. import java.text.ParseException; import java.util.Calendar; import java.util.Date; import org.quartz.CronExpression; public class QuartzCronExpressionProblem { private class CronExpressionTest extends CronExpression { public CronExpressionTest(String cronExpression) throws ParseException { super(cronExpression); } public Date getTimeAfterPublic(Date afterTime) { return getTimeAfter(afterTime); } } public void runTest() { CronExpressionTest cronExpression = null; try { cronExpression = new CronExpressionTest("0 0/5 21-3 ? * MON-FRI"); } catch (ParseException e) { return; } Calendar closeToMidnight = Calendar.getInstance(); closeToMidnight.set(Calendar.HOUR, 0); closeToMidnight.set(Calendar.MINUTE, 30); closeToMidnight.set(Calendar.SECOND, 0); closeToMidnight.clear(Calendar.MILLISECOND); Date timeAfterPublic = cronExpression.getTimeAfterPublic(closeToMidnight.getTime()); } public static void main(String[] args) { QuartzCronExpressionProblem obj = new QuartzCronExpressionProblem(); obj.runTest(); } } After tracing some more, the problem lies in the parsing after all. Having the following stack:
Thread [main] (Suspended) QuartzCronExpressionProblem$CronExpressionTest(org.quartz.CronExpression).addToSet(int, int, int, int) line: 959 QuartzCronExpressionProblem$CronExpressionTest(org.quartz.CronExpression).checkNext(int, java.lang.String, int, int) line: 685 QuartzCronExpressionProblem$CronExpressionTest(org.quartz.CronExpression).storeExpressionVals(int, java.lang.String, int) line: 609 QuartzCronExpressionProblem$CronExpressionTest(org.quartz.CronExpression).buildExpression(java.lang.String) line: 411 QuartzCronExpressionProblem$CronExpressionTest(org.quartz.CronExpression).<init>(java.lang.String) line: 246 QuartzCronExpressionProblem$CronExpressionTest.<init>(QuartzCronExpressionProblem, java.lang.String) line: 12 QuartzCronExpressionProblem.runTest() line: 39 QuartzCronExpressionProblem.main(java.lang.String[]) line: 56 In addToSet, startAt is 21, stopAt is 3 and the for loop at the very end of addToSet will never add anything to the hours treeset, as there are no hours bigger than 21 but smaller than 3. So I'd say, whenever you have a range where the end is "before" the start, Quartz' cron scheduler will fail. Thanks for the further investigation.
I'll dig into it - might be that it's already fixed on trunk but was a bug with 1.6; or might be that my test was oversimple for some reason. Definitely still a bug on trunk - your test case works very nicely.
Having a slow day... so the bug is, as you said, in the for loop at the end of addToSet.
A work of: "0 0/5 0-3,21-23 ? * MON-FRI" seems to be fine. Experimental test file based on Kay's original example.
Attaching my current state of code in attempting to fix the problem.
1/ There are some caveats. I presume that having 29,30,31 in the days of month set won't hurt the system when it's in February etc. I think that's a safe assumption, but I need to test. 2/ The isSatisfiedBy unit test fails with this change. Need to understand why. 3/ The following cron expression fails: "58 5 21 ? * FRI-TUE" with: java.text.ParseException: Invalid Day-of-Week sequence: 6 > 3 at org.quartz.CronExpression.storeExpressionVals(CronExpression.java:483) at org.quartz.CronExpression.buildExpression(CronExpression.java:411) at org.quartz.CronExpression.<init>(CronExpression.java:246) at TestCronExpression.<init>(Quartz601.java:39) at Quartz601.runTest(Quartz601.java:23) at Quartz601.main(Quartz601.java:15) Unix cron (or rather crontab on my OS X laptop) appears quite happy with an overflow in the named day of week, so that exception should probably be removed. Commenting out the ParseException seems to do the right thing; though also would need some more testing.
Adding a unit test to show these bugs.
Most of them cause errors, one causes an exception. Final fix/test patch for this issue. The isSatisifedBy failure was because I had broken the default case. I've adjusted it to check if the max variable has been changed from the initial -1 to decide if it should do the default case or the overlap case.
Ready for review. I need to javadoc this strongly as per the dev@ list discussion - if you use too many ranges in crons then it all gets very confusing and we've no idea what will happen there.
Then it'll get committed. New version of the patch with added javadoc note:
* <li>Overflowing ranges is supported - that is, having a larger number on * the left hand side than the right. You might do 22-2 to catch 10 o'clock * at night until 2 o'clock in the morning, or you might have NOV-FEB. It is * very important to note that overuse of overflowing ranges creates ranges * that don't make sense and no effort has been made to determine which * interpretation CronExpression chooses. An example would be * "0 0 14-6 ? * FRI-MON". </li> svn ci -m "Applying the last patch from Kay Huber's
Sending src/java/org/quartz/CronExpression.java Adding src/test/java/org/quartz/Quartz601Test.java Transmitting file data .. Committed revision 765. | ||||||||||||||||||||||||||||||||||||||||||||||||||||
 |
|
Bug
Closed
Major
CronExpression over Midnight causes exception
public void testQuartz601() throws ParseException {
CronExpression cronExpression = new CronExpression("0 0/5 21-3 ? * MON-FRI");
}
So presumably it's not your second idea.