diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/trigger/schedule/CronnableSchedule.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/trigger/schedule/CronnableSchedule.java index ec309c69476cc..695c9b192eaa6 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/trigger/schedule/CronnableSchedule.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/trigger/schedule/CronnableSchedule.java @@ -13,20 +13,15 @@ public abstract class CronnableSchedule implements Schedule { - private static final Comparator CRON_COMPARATOR = new Comparator() { - @Override - public int compare(Cron c1, Cron c2) { - return c1.expression().compareTo(c2.expression()); - } - }; + private static final Comparator CRON_COMPARATOR = Comparator.comparing(Cron::expression); protected final Cron[] crons; - public CronnableSchedule(String... expressions) { + CronnableSchedule(String... expressions) { this(crons(expressions)); } - public CronnableSchedule(Cron... crons) { + private CronnableSchedule(Cron... crons) { assert crons.length > 0; this.crons = crons; Arrays.sort(crons, CRON_COMPARATOR); @@ -37,7 +32,15 @@ public long nextScheduledTimeAfter(long startTime, long time) { assert time >= startTime; long nextTime = Long.MAX_VALUE; for (Cron cron : crons) { - nextTime = Math.min(nextTime, cron.getNextValidTimeAfter(time)); + long nextValidTimeAfter = cron.getNextValidTimeAfter(time); + + boolean previousCronExpired = nextTime == -1; + boolean currentCronValid = nextValidTimeAfter > -1; + if (previousCronExpired && currentCronValid) { + nextTime = nextValidTimeAfter; + } else { + nextTime = Math.min(nextTime, nextValidTimeAfter); + } } return nextTime; } diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/trigger/schedule/CronScheduleTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/trigger/schedule/CronScheduleTests.java index 1ade767410b53..6cbdf6e122656 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/trigger/schedule/CronScheduleTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/trigger/schedule/CronScheduleTests.java @@ -11,11 +11,15 @@ import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.json.JsonXContent; +import java.time.ZoneOffset; +import java.time.ZonedDateTime; + import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.hamcrest.Matchers.arrayWithSize; import static org.hamcrest.Matchers.hasItemInArray; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; public class CronScheduleTests extends ScheduleTestCase { public void testInvalid() throws Exception { @@ -54,18 +58,25 @@ public void testParseMultiple() throws Exception { assertThat(crons, hasItemInArray("0 0/3 * * * ?")); } + public void testMultipleCronsNextScheduledAfter() { + CronSchedule schedule = new CronSchedule("0 5 9 1 1 ? 2019", "0 5 9 1 1 ? 2020", "0 5 9 1 1 ? 2017"); + ZonedDateTime start2019 = ZonedDateTime.of(2019, 1, 1, 0, 0, 0, 0, ZoneOffset.UTC); + ZonedDateTime start2020 = ZonedDateTime.of(2020, 1, 1, 0, 0, 0, 0, ZoneOffset.UTC); + long firstSchedule = schedule.nextScheduledTimeAfter(0, start2019.toInstant().toEpochMilli()); + long secondSchedule = schedule.nextScheduledTimeAfter(0, start2020.toInstant().toEpochMilli()); + + assertThat(firstSchedule, is(not(-1L))); + assertThat(secondSchedule, is(not(-1L))); + assertThat(firstSchedule, is(not(secondSchedule))); + } + public void testParseInvalidBadExpression() throws Exception { XContentBuilder builder = jsonBuilder().value("0 0/5 * * ?"); BytesReference bytes = BytesReference.bytes(builder); XContentParser parser = createParser(JsonXContent.jsonXContent, bytes); parser.nextToken(); - try { - new CronSchedule.Parser().parse(parser); - fail("expected cron parsing to fail when using invalid cron expression"); - } catch (ElasticsearchParseException pe) { - // expected - assertThat(pe.getCause(), instanceOf(IllegalArgumentException.class)); - } + ElasticsearchParseException e = expectThrows(ElasticsearchParseException.class, () -> new CronSchedule.Parser().parse(parser)); + assertThat(e.getCause(), instanceOf(IllegalArgumentException.class)); } public void testParseInvalidEmpty() throws Exception {