Watcher: Properly find next valid date in cron expressions (#32734)

When a list/an array of cron expressions is provided, and one of those addresses
is already expired, the expired one will be considered as an option
instead of the valid next one.

This commit also reduces the visibility of the CronnableSchedule and
refactors a comparator to look like java 8.

Author: spinscale

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> CRON_COMPARATOR = new Comparator<Cron>() {
-        @Override
-        public int compare(Cron c1, Cron c2) {
-            return c1.expression().compareTo(c2.expression());
-        }
-    };
+    private static final Comparator<Cron> 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;
     }

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 {