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

Wrong parsing version 4.1.1 #103

Closed
benoitbb opened this issue Jul 14, 2016 · 7 comments
Closed

Wrong parsing version 4.1.1 #103

benoitbb opened this issue Jul 14, 2016 · 7 comments
Milestone

Comments

@benoitbb
Copy link

String cron = "* 0/1 * ? * TUE";
CronDefinition cronDefinition = CronDefinitionBuilder.instanceDefinitionFor(CronType.QUARTZ);
CronDescriptor descriptor = CronDescriptor.instance(Locale.UK);
CronParser parser = new CronParser(cronDefinition);
final Cron parse = parser.parse(cron);
final String describe = descriptor.describe(parse);

--> output :

every minute at Wednesday day

Is it a bug ?

Can you have a look please ?

Best regards,

@jmrozanec
Copy link
Owner

@benoitbb Just checked - it is a bug. Thank you for reporting it. If you have some time, you are welcome to contribute a test for it, or even a solution :) Thanks!

@albuhuba
Copy link
Contributor

The problem is in the FieldConstraintsBuilder#daysOfWeekMapping. If I change the mapping to be from 0 to 6 this test is working properly, but others are now breaking. Could the problem start from the fact that the java version was changed to 8 and somehow the internal calendar is handling differently the days of the week?

On the other hand I've seen in the code that there were some other problems with the day-of-week. Can you explain to me what was the problem and what was the solution?

@jmrozanec
Copy link
Owner

@albuhuba almost all computations are based on Jodatime. We abstract internal date representations from those related to crons. The issue is most probably related to a cron specific mapping. For the other DoW issues, please check the corresponding issues and commits. Thanks!

@jmrozanec
Copy link
Owner

@albuhuba Today we updated all Jodatime references to Java8 ZonedDateTime. If working on this issue, please consider updating the branch.

@lpbak lpbak mentioned this issue Sep 13, 2016
@lpbak
Copy link
Contributor

lpbak commented Sep 13, 2016

This issue seems to be caused by the fact, that while com.cronutils.model.definition.CronDefinition for quartz-like cron expressions is built with a +1 offset in a DAY_OF_WEEK field (call to withMondayDoWValue(2) in com.cronutils.model.definition.CronDefinitionBuilder.quartz()), this offset is not in any way taken into account by com.cronutils.descriptor.CronDescriptor. Unfortunately, fixing this issue (without resorting to some ugly hacks) would probably require some refactoring. Currently, the information about the aforementioned offset seems to be completely lost after com.cronutils.model.definition.CronDefinition is built, and hence isn't accessible to the com.cronutils.descriptor.CronDescriptor.

@pangyikhei
Copy link
Contributor

pangyikhei commented Jul 4, 2017

In 6.0.2 the expression "* 0/1 * ? * TUE" does output Tuesday, but doesn't that definition mean every second of every minute? Currently it outputs every minute at Tuesday day.

@jmrozanec
Copy link
Owner

@pangyikhei yes, you are right. The description is not accurate. We should add a test for this case too. Thanks!

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

5 participants