Skip to content

CronSequenceGenerator.next() is not implemented as documented [SPR-14589] #19158

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

Closed
spring-projects-issues opened this issue Aug 15, 2016 · 1 comment
Assignees
Labels
status: declined A suggestion or change that we don't feel we should currently apply

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Aug 15, 2016

Philippe Perrault opened SPR-14589 and commented

I believe I have found a bug in the CronSequenceGenerator. According to your documentation in the next(...) method the plan is to first round up to the next whole second instead the code just sets the milliseconds filed to 0. I believe the originalTimestamp should be 'long originalTimestamp = calendar.getTimeInMillis() + 1000;' in order to round up to the next second per "The Plan"

Here is the FQDN of the class
org.springframework.scheduling.support.CronSequenceGenerator

Here is the method in question:
public Date next(Date date) {
/*
The plan:

	1 Round up to the next whole second

	2 If seconds match move on, otherwise find the next match:
	2.1 If next match is in the next minute then roll forwards

	3 If minute matches move on, otherwise find the next match
	3.1 If next match is in the next hour then roll forwards
	3.2 Reset the seconds and go to 2

	4 If hour matches move on, otherwise find the next match
	4.1 If next match is in the next day then roll forwards,
	4.2 Reset the minutes and seconds and go to 2

	...
	*/

	Calendar calendar = new GregorianCalendar();
	calendar.setTimeZone(this.timeZone);
	calendar.setTime(date);

	// First, just reset the milliseconds and try to calculate from there...
	calendar.set(Calendar.MILLISECOND, 0);
	long originalTimestamp = calendar.getTimeInMillis();
	doNext(calendar, calendar.get(Calendar.YEAR));

	if (calendar.getTimeInMillis() == originalTimestamp) {
		// We arrived at the original timestamp - round up to the next whole second and try again...
		calendar.add(Calendar.SECOND, 1);
		doNext(calendar, calendar.get(Calendar.YEAR));
	}

	return calendar.getTime();
}

Affects: 3.2.17, 4.2.7, 4.3.1

Issue Links:

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Aug 16, 2016

Juergen Hoeller commented

This works as designed: #14094 intentionally refined the rounding-up there. I'll update that outdated "plan" comment but since it isn't even in javadoc this isn't really a track-worthy issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

No branches or pull requests

2 participants