Skip to content

Commit

Permalink
fix: Fix bug that causes cronpatterns with no next execution-time to …
Browse files Browse the repository at this point in the history
…fail. (#435)

Introduce an `Instant.NEVER` that represents future execution-time that
will never run, and use this for cron-patterns with no next
execution-time.

## Fixes
* #361 


## Reminders
- [ ] Added/ran automated tests
- [x] Update README and/or examples
- [x] Ran `mvn spotless:apply`
  • Loading branch information
kagkarlsson committed Dec 6, 2023
1 parent ff6c39f commit 7798c35
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import java.time.Instant;
import java.time.ZoneId;
import java.time.ZonedDateTime;
import java.time.temporal.ChronoUnit;
import java.util.Optional;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -69,19 +68,23 @@ public CronSchedule(String pattern, ZoneId zoneId, CronStyle cronStyle) {
@Override
public Instant getNextExecutionTime(ExecutionComplete executionComplete) {
lazyInitExecutionTime(); // for deserialized objects
ZonedDateTime lastDone =
ZonedDateTime.ofInstant(
executionComplete.getTimeDone(),
zoneId); // frame the 'last done' time in the context of the time zone for this schedule

// frame the 'last done' time in the context of the time zone for this schedule
// so that expressions like "0 05 13,20 * * ?" (New York) can operate in the
// context of the desired time zone
ZonedDateTime lastDone = ZonedDateTime.ofInstant(executionComplete.getTimeDone(), zoneId);

Optional<ZonedDateTime> nextTime = cronExecutionTime.nextExecution(lastDone);
if (!nextTime.isPresent()) {
LOG.error(
"Cron-pattern did not return any further execution-times. This behavior is currently not supported by the scheduler. Setting next execution-time to far-future.");
return Instant.now().plus(1000, ChronoUnit.YEARS);
"Cron-pattern did not return any further execution-times. This behavior is currently "
+ "not supported by the scheduler. Setting next execution-time to far-future, "
+ "aka \"NEVER\" ({})",
NEVER);

return Schedule.NEVER;
}

return nextTime.get().toInstant();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@

public interface Schedule {

Instant NEVER = Instant.parse("2500-01-01T12:00:00.00Z");

Instant getNextExecutionTime(ExecutionComplete executionComplete);

/** Used to get the first execution-time for a schedule. Simulates an ExecutionComplete event. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public void test_concurrency_optimistic_locking() throws InterruptedException {
// Observed failed heartbeats due to deadlock.
// FIXLATER: add retry of update heartbeats
Utils.retryOnFailed(
1,
3,
() -> {
DEBUG_LOG.info("Starting test_concurrency_optimistic_locking");
ClusterTests.testConcurrencyForPollingStrategy(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,13 @@ public void should_mark_schedule_as_disabled() {
assertFalse(Schedules.cron("0 * * * * ?").isDisabled());
}

@Test
public void should_not_fail_on_cronschedule_without_next_execution_time() {
CronSchedule cron =
new CronSchedule("0 0 0 29 2 MON#1", ZoneId.systemDefault(), CronStyle.SPRING53);
Assertions.assertEquals(Schedule.NEVER, cron.getNextExecutionTime(complete(Instant.now())));
}

private void assertNextExecutionTime(
ZonedDateTime timeDone, String cronPattern, ZonedDateTime expectedTime) {
assertNextExecutionTime(timeDone, expectedTime, new CronSchedule(cronPattern));
Expand Down

0 comments on commit 7798c35

Please sign in to comment.