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

ExecutionTime.forCron fails on pattern "0 0/30 * * * ?" #78

Closed
rwiesemann opened this issue Apr 6, 2016 · 15 comments
Closed

ExecutionTime.forCron fails on pattern "0 0/30 * * * ?" #78

rwiesemann opened this issue Apr 6, 2016 · 15 comments

Comments

@rwiesemann
Copy link

I use cronutils version 3.6.1.
The Crontype.QUARTZ pattern "0 0/30 * * * ?" fails on

CronDefinition cronDefinition = CronDefinitionBuilder.instanceDefinitionFor( CronType.QUARTZ );
CronParser parser = new CronParser( cronDefinition );
Cron cron = parser.parse( "0 0/30 * * * ?" );
ExecutionTime time = ExecutionTime.forCron( cron );
Duration timeToNextExecution = time.timeToNextExecution( DateTime.now() );

with exception

14:22:35,227 ERROR [STDERR] com.cronutils.model.time.generator.NoSuchValueException
14:22:35,228 ERROR [STDERR]     at com.cronutils.model.time.generator.EveryFieldValueGenerator.generateNextValue(EveryFieldValueGenerator.java:43)
14:22:35,228 ERROR [STDERR]     at com.cronutils.model.time.generator.EveryFieldValueGenerator.generateCandidatesNotIncludingIntervalExtremes(EveryFieldValueGenerator.java:67)
14:22:35,229 ERROR [STDERR]     at com.cronutils.model.time.generator.FieldValueGenerator.generateCandidates(FieldValueGenerator.java:55)
14:22:35,229 ERROR [STDERR]     at com.cronutils.model.time.ExecutionTimeBuilder.forMinutesMatching(ExecutionTimeBuilder.java:51)
14:22:35,230 ERROR [STDERR]     at com.cronutils.model.time.ExecutionTime.forCron(ExecutionTime.java:92)
14:22:35,230 ERROR [STDERR]     at de.psi.messaging.purge.MessagePurgeTrigger.getDelay(MessagePurgeTrigger.java:35)
@jmrozanec
Copy link
Owner

@rwiesemann thank you for reporting this. You are welcome to help on the issue contributing a test for it.

@maehld
Copy link

maehld commented May 19, 2016

I've run into the same problem. Apparently your code is throwing NoSuchValueExceptions but catches them in EveryFieldValueGenerator.generateCandidatesNotIncludingIntervalExtremes. As the resulting execution times are OK in my testing I would recommend not logging the exception as it seems to be a normal case with intervals. Getting rid of the exception completely would be perhaps even better.

FYI the test cases also encounter the exception when running but as you've not configured logging in your test cases they do not show on the console.

If you're interesed I would be happy to provide a pull request for a rework without the exception.

Thanks

Dominik

@kostapc
Copy link
Contributor

kostapc commented May 19, 2016

Same issue with any intervals.
test code: https://gist.github.com/kostapc/3917d8ec50af8bf665023145456db5d5

@kostapc
Copy link
Contributor

kostapc commented May 19, 2016

@maehld can you please public your fixes, maybe on your fork - i will pick it. Right now i work with this issue.

@maehld
Copy link

maehld commented May 19, 2016

Hi. I will try to get a fork working by tomorrow.

@jmrozanec
Copy link
Owner

@maehld @kostapc Thank you both for working on this issue. I would be happy to receive your PR and merge to master if you provide a working solution for it.

@jmrozanec
Copy link
Owner

@maehld @kostapc I just merged a bigger refactor into master branch. Please check and update your forks, to avoid working on a stale version. Looking forward to your patch!

@jmrozanec
Copy link
Owner

@maehld @kostapc Just created a new branch, where some changes were introduced to fix issue #58 that may be related to this same problem. Not merging into master yet, since there are some tests failing. A review over introduced changes and additional fixes would be welcome.

@kostapc
Copy link
Contributor

kostapc commented May 22, 2016

@jmrozanec thanks a lot for you work!

@maehld
Copy link

maehld commented May 22, 2016

I will have a look tomorrow at the branch. Sorry for not getting something to work by friday but work got in the way.

@jmrozanec
Copy link
Owner

@maehld sure! No problem!

@jmrozanec
Copy link
Owner

@maehld @kostapc Just merged a new refactor into master. Minor changes were introduced into the model on how we represent Always, Every and Between; in order to have a more natural representation of the cron in the model.

@kostapc
Copy link
Contributor

kostapc commented May 25, 2016

i fix log level in some cases and provide extended tests valued for me. @jmrozanec - you do all main work! Thank a lot! Waiting for next release in maven repo.

@jmrozanec
Copy link
Owner

@kostapc @maehld @rwiesemann Merged! Thank you for contributing to cron-utils!

@jmrozanec
Copy link
Owner

jmrozanec commented May 25, 2016

@kostapc @maehld @rwiesemann 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

4 participants