From 6fdbcfeec811f597cb7efb362db1bf786647c90f Mon Sep 17 00:00:00 2001 From: Luke Whiting Date: Mon, 25 Nov 2024 09:51:11 +0000 Subject: [PATCH] (#34659) - Add Timezone Configuration to Watcher (#117033) * Add timezone support to Cron objects * Add timezone support to CronnableSchedule * XContent change to support parsing and display of TimeZone fields on schedules * Case insensitive timezone parsing * Doc changes * YAML REST tests * Equals, toString and HashCode now include timezone * Additional random testing for DST transitions * Migrate Cron class to use wrapped LocalDateTime The algorithm depends on some quirks of calendar but LocalDateTime correctly ignores DST during calculations so this uses a LocalDateTime with a wrapper to emulate some of Calendar's behaviours that the Cron algorithm depends on * Additional documentation to explain discontinuity event behaviour * Remove redundant conversions from ZoneId to TimeZone following move to LocalDateTime * Add documentation warning that manual clock changes will cause unpredictable watch execution * Update docs/reference/watcher/trigger/schedule.asciidoc Co-authored-by: Lee Hinman --------- Co-authored-by: Lee Hinman --- .../watching-time-series-data.asciidoc | 8 +- .../watcher/trigger/schedule.asciidoc | 33 +- .../watcher/trigger/schedule/cron.asciidoc | 42 ++- .../watcher/trigger/schedule/daily.asciidoc | 24 ++ .../watcher/trigger/schedule/monthly.asciidoc | 24 +- .../watcher/trigger/schedule/weekly.asciidoc | 24 +- .../watcher/trigger/schedule/yearly.asciidoc | 23 ++ .../xpack/core/scheduler/Cron.java | 287 +++++++++--------- .../scheduler/LocalDateTimeLegacyWrapper.java | 130 ++++++++ .../core/scheduler/CronTimezoneTests.java | 231 ++++++++++++++ .../connector/ConnectorCustomSchedule.java | 2 +- .../connector/ConnectorScheduling.java | 2 +- .../slm/SnapshotLifecyclePolicyTests.java | 4 +- .../put_watch/11_timezoned_schedules.yml | 121 ++++++++ .../trigger/schedule/CronnableSchedule.java | 34 ++- .../trigger/schedule/ScheduleRegistry.java | 40 ++- .../trigger/schedule/ScheduleTrigger.java | 9 +- .../schedule/support/TimezoneUtils.java | 55 ++++ .../schedule/ScheduleRegistryTests.java | 15 +- .../schedule/support/TimezoneUtilsTests.java | 40 +++ 20 files changed, 975 insertions(+), 173 deletions(-) create mode 100644 x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/scheduler/LocalDateTimeLegacyWrapper.java create mode 100644 x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/scheduler/CronTimezoneTests.java create mode 100644 x-pack/plugin/watcher/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/watcher/put_watch/11_timezoned_schedules.yml create mode 100644 x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/trigger/schedule/support/TimezoneUtils.java create mode 100644 x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/trigger/schedule/support/TimezoneUtilsTests.java diff --git a/docs/reference/watcher/example-watches/watching-time-series-data.asciidoc b/docs/reference/watcher/example-watches/watching-time-series-data.asciidoc index 421c69619cfea..b1c776baae1de 100644 --- a/docs/reference/watcher/example-watches/watching-time-series-data.asciidoc +++ b/docs/reference/watcher/example-watches/watching-time-series-data.asciidoc @@ -62,20 +62,20 @@ contain the words "error" or "problem". To set up the watch: -. Define the watch trigger--a daily schedule that runs at 12:00 UTC: +. Define the watch trigger--a daily schedule that runs at 12:00 Australian Eastern Standard Time (UTC+10:00): + [source,js] -------------------------------------------------- "trigger" : { "schedule" : { + "timezone": "Australia/Brisbane", "daily" : { "at" : "12:00" } } } -------------------------------------------------- + -NOTE: In {watcher}, you specify times in UTC time. Don't forget to do the - conversion from your local time so the schedule triggers at the time - you intend. +NOTE: In {watcher}, if the timezone is omitted then schedules default to UTC. `timezone` can be specified either +as a +/-HH:mm offset from UTC or as a timezone name from the machines local IANA Time Zone Database. . Define the watch input--a search that uses a filter to constrain the results to the past day. diff --git a/docs/reference/watcher/trigger/schedule.asciidoc b/docs/reference/watcher/trigger/schedule.asciidoc index fa389409d15c4..d2bf466644e10 100644 --- a/docs/reference/watcher/trigger/schedule.asciidoc +++ b/docs/reference/watcher/trigger/schedule.asciidoc @@ -6,12 +6,42 @@ ++++ Schedule <> define when the watch execution should start based -on date and time. All times are specified in UTC time. +on date and time. All times are in UTC time unless a timezone is explicitly specified +in the schedule. {watcher} uses the system clock to determine the current time. To ensure schedules are triggered when expected, you should synchronize the clocks of all nodes in the cluster using a time service such as http://www.ntp.org/[NTP]. +NOTE: {watcher} can't correct for manual adjustments to the system clock. Be aware when making +such changes that watch execution may be affected with watches being skipped or repeated if the +adjustment covers their target execution time. This applies to changes made via NTP as well. + +When specifying a timezone for a watch, keep in mind the effect daylight savings time +transitions may have on the schedule, especially if the watch is scheduled to run +during the transition. Here's how {watcher} handles watches scheduled during discontinuities: + +==== Gap Transitions +These occur when the clock moves forward, such as when daylight savings time starts +and cause certain hours or minutes to be skipped. If your watch is scheduled to run +during a gap transition, the watch is executed at the same time as before the transition. + +Example: If a watch is scheduled to run daily at 1:30AM in the `Europe/London` time zone and +the clock moves forward one hour from 1:00AM (GMT+0) to 2:00AM (GMT+1), the watch is executed +at 2:30AM (GMT+1) which would have been 1:30AM before the transition. Subsequent executions +happen at 1:30AM (GMT+1). + +==== Overlap Transitions +These occur when the clock moves backward, such as when daylight savings time ends +and cause certain hours or minutes to be repeated. If your watch is scheduled to run +during an overlap transition, only the first occurrence of the time causes to the watch +to execute with the second being skipped. + +Example: If a watch is scheduled to run at 1:30 AM and the clock moves backward one hour +from 2:00AM to 1:00AM, the watch is executed at 1:30AM and the second occurrence after the +change is skipped. + +=== Throttling Keep in mind that the throttle period can affect when a watch is actually executed. The default throttle period is five seconds (5000 ms). If you configure a schedule that's more frequent than the throttle period, the throttle period overrides the @@ -20,6 +50,7 @@ and set the schedule to every 10 seconds, the watch is executed no more than once per minute. For more information about throttling, see <>. +=== Schedule Types {watcher} provides several types of schedule triggers: * <> diff --git a/docs/reference/watcher/trigger/schedule/cron.asciidoc b/docs/reference/watcher/trigger/schedule/cron.asciidoc index 673f350435c5f..c33bf524a8737 100644 --- a/docs/reference/watcher/trigger/schedule/cron.asciidoc +++ b/docs/reference/watcher/trigger/schedule/cron.asciidoc @@ -5,14 +5,14 @@ ++++ -Defines a <> using a <> +Defines a <> using a <> that specifiues when to execute a watch. -TIP: While cron expressions are powerful, a regularly occurring schedule -is easier to configure with the other schedule types. -If you must use a cron schedule, make sure you verify it with -<> . +TIP: While cron expressions are powerful, a regularly occurring schedule +is easier to configure with the other schedule types. +If you must use a cron schedule, make sure you verify it with +<> . ===== Configure a cron schedule with one time @@ -60,16 +60,40 @@ minute during the weekend: -------------------------------------------------- // NOTCONSOLE +[[configue_cron_time-zone]] +==== Use a different time zone for a cron schedule +By default, cron expressions are evaluated in the UTC time zone. To use a different time zone, +you can specify the `timezone` parameter in the schedule. For example, the following +`cron` schedule triggers at 6:00 AM and 6:00 PM during weekends in the `America/Los_Angeles` time zone: + + +[source,js] +-------------------------------------------------- +{ + ... + "trigger" : { + "schedule" : { + "timezone" : "America/Los_Angeles", + "cron" : [ + "0 6,18 * * * SAT-SUN", + ] + } + } + ... +} +-------------------------------------------------- +// NOTCONSOLE + [[croneval]] ===== Use croneval to validate cron expressions -{es} provides a <> command line tool -in the `$ES_HOME/bin` directory that you can use to check that your cron expressions +{es} provides a <> command line tool +in the `$ES_HOME/bin` directory that you can use to check that your cron expressions are valid and produce the expected results. -To validate a cron expression, pass it in as a parameter to `elasticsearch-croneval`: +To validate a cron expression, pass it in as a parameter to `elasticsearch-croneval`: [source,bash] -------------------------------------------------- bin/elasticsearch-croneval "0 0/1 * * * ?" --------------------------------------------------- +-------------------------------------------------- diff --git a/docs/reference/watcher/trigger/schedule/daily.asciidoc b/docs/reference/watcher/trigger/schedule/daily.asciidoc index cea2b8316e02f..d258d9c612350 100644 --- a/docs/reference/watcher/trigger/schedule/daily.asciidoc +++ b/docs/reference/watcher/trigger/schedule/daily.asciidoc @@ -97,3 +97,27 @@ or minutes as an array. For example, following `daily` schedule triggers at } -------------------------------------------------- // NOTCONSOLE + +[[specifying-time-zone-for-daily-schedule]] +===== Specifying a time zone for a daily schedule +By default, daily schedules are evaluated in the UTC time zone. To use a different time zone, +you can specify the `timezone` parameter in the schedule. For example, the following +`daily` schedule triggers at 6:00 AM and 6:00 PM in the `Pacific/Galapagos` time zone: + +[source,js] +-------------------------------------------------- +{ + "trigger" : { + "schedule" : { + "timezone" : "Pacific/Galapagos", + "daily" : { + "at" : { + "hour" : [ 6, 18 ], + "minute" : 0 + } + } + } + } +} +-------------------------------------------------- +// NOTCONSOLE diff --git a/docs/reference/watcher/trigger/schedule/monthly.asciidoc b/docs/reference/watcher/trigger/schedule/monthly.asciidoc index 7d13262ed2fa8..694c76aaee23a 100644 --- a/docs/reference/watcher/trigger/schedule/monthly.asciidoc +++ b/docs/reference/watcher/trigger/schedule/monthly.asciidoc @@ -74,4 +74,26 @@ schedule triggers at 12:00 AM and 12:00 PM on the 10th and 20th of each month. } } -------------------------------------------------- -// NOTCONSOLE \ No newline at end of file +// NOTCONSOLE + +==== Configuring time zones for monthly schedules +By default, monthly schedules are evaluated in the UTC time zone. To use a different +time zone, you can specify the `timezone` parameter in the schedule. For example, +the following `monthly` schedule triggers at 6:00 AM and 6:00 PM on the 15th of each month in +the `Asia/Tokyo` time zone: + +[source,js] +-------------------------------------------------- +{ + "trigger" : { + "schedule" : { + "timezone" : "Asia/Tokyo", + "monthly" : { + "on" : [ 15 ], + "at" : [ 6:00, 18:00 ] + } + } + } +} +-------------------------------------------------- +// NOTCONSOLE diff --git a/docs/reference/watcher/trigger/schedule/weekly.asciidoc b/docs/reference/watcher/trigger/schedule/weekly.asciidoc index 5b43de019ad25..53bd2f3167b21 100644 --- a/docs/reference/watcher/trigger/schedule/weekly.asciidoc +++ b/docs/reference/watcher/trigger/schedule/weekly.asciidoc @@ -79,4 +79,26 @@ Alternatively, you can specify days and times in an object that has `on` and } } -------------------------------------------------- -// NOTCONSOLE \ No newline at end of file +// NOTCONSOLE + +==== Use a different time zone for a weekly schedule +By default, weekly schedules are evaluated in the UTC time zone. To use a different time zone, +you can specify the `timezone` parameter in the schedule. For example, the following +`weekly` schedule triggers at 6:00 AM and 6:00 PM on Tuesdays and Fridays in the +`America/Buenos_Aires` time zone: + +[source,js] +-------------------------------------------------- +{ + "trigger" : { + "schedule" : { + "timezone" : "America/Buenos_Aires", + "weekly" : { + "on" : [ "tuesday", "friday" ], + "at" : [ "6:00", "18:00" ] + } + } + } +} +-------------------------------------------------- +// NOTCONSOLE diff --git a/docs/reference/watcher/trigger/schedule/yearly.asciidoc b/docs/reference/watcher/trigger/schedule/yearly.asciidoc index 8fce024bf9f4a..c33321ef5a7dc 100644 --- a/docs/reference/watcher/trigger/schedule/yearly.asciidoc +++ b/docs/reference/watcher/trigger/schedule/yearly.asciidoc @@ -88,3 +88,26 @@ January 20th, December 10th, and December 20th. } -------------------------------------------------- // NOTCONSOLE + +==== Configuring a yearly schedule with a different time zone +By default, the `yearly` schedule is evaluated in the UTC time zone. To use a +different time zone, you can specify the `timezone` parameter in the schedule. +For example, the following `yearly` schedule triggers at 3:30 PM and 8:30 PM +on June 4th in the `Antarctica/Troll` time zone: + +[source,js] +-------------------------------------------------- +{ + "trigger" : { + "schedule" : { + "timezone" : "Antarctica/Troll", + "yearly" : { + "in" : "june", + "on" : 4, + "at" : [ 15:30, 20:30 ] + } + } + } +} +-------------------------------------------------- +// NOTCONSOLE diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/scheduler/Cron.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/scheduler/Cron.java index b9d39aa665848..c94b90b6c0c23 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/scheduler/Cron.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/scheduler/Cron.java @@ -8,11 +8,15 @@ import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.common.time.DateFormatter; +import org.elasticsearch.core.SuppressForbidden; import org.elasticsearch.xcontent.ToXContentFragment; import org.elasticsearch.xcontent.XContentBuilder; import java.io.IOException; +import java.time.LocalDateTime; +import java.time.ZoneId; import java.time.ZoneOffset; +import java.time.temporal.ChronoField; import java.util.Calendar; import java.util.Iterator; import java.util.Locale; @@ -232,6 +236,8 @@ public class Cron implements ToXContentFragment { private final String expression; + private ZoneId timeZone; + private transient TreeSet seconds; private transient TreeSet minutes; private transient TreeSet hours; @@ -246,7 +252,20 @@ public class Cron implements ToXContentFragment { private transient boolean nearestWeekday = false; private transient int lastdayOffset = 0; - public static final int MAX_YEAR = Calendar.getInstance(UTC, Locale.ROOT).get(Calendar.YEAR) + 100; + // Restricted to 50 years as the tzdb only has correct DST transition information for countries using a lunar calendar + // for the next ~60 years + public static final int MAX_YEAR = Calendar.getInstance(UTC, Locale.ROOT).get(Calendar.YEAR) + 50; + + public Cron(String expression, ZoneId timeZone) { + this.timeZone = timeZone; + assert expression != null : "cron expression cannot be null"; + this.expression = expression.toUpperCase(Locale.ROOT); + try { + buildExpression(this.expression); + } catch (Exception e) { + throw illegalArgument("invalid cron expression [{}]", e, expression); + } + } /** * Constructs a new CronExpression based on the specified @@ -259,13 +278,7 @@ public class Cron implements ToXContentFragment { * CronExpression */ public Cron(String expression) { - assert expression != null : "cron expression cannot be null"; - this.expression = expression.toUpperCase(Locale.ROOT); - try { - buildExpression(this.expression); - } catch (Exception e) { - throw illegalArgument("invalid cron expression [{}]", e, expression); - } + this(expression, UTC.toZoneId()); } /** @@ -275,7 +288,11 @@ public Cron(String expression) { * @param cron The existing cron expression to be copied */ public Cron(Cron cron) { - this(cron.expression); + this(cron.expression, cron.timeZone); + } + + public void setTimeZone(ZoneId timeZone) { + this.timeZone = timeZone; } /** @@ -286,31 +303,25 @@ public Cron(Cron cron) { * a time that is previous to the given time) * @return the next valid time (since the epoch) */ + @SuppressForbidden(reason = "In this case, the DST ambiguity of the atZone method is desired, understood and tested") public long getNextValidTimeAfter(final long time) { - // Computation is based on Gregorian year only. - Calendar cl = new java.util.GregorianCalendar(UTC, Locale.ROOT); - - // move ahead one second, since we're computing the time *after* the - // given time - final long afterTime = time + 1000; - // CronTrigger does not deal with milliseconds - cl.setTimeInMillis(afterTime); - cl.set(Calendar.MILLISECOND, 0); + LocalDateTime afterTimeLdt = LocalDateTime.ofInstant(java.time.Instant.ofEpochMilli(time), timeZone).plusSeconds(1); + LocalDateTimeLegacyWrapper cl = new LocalDateTimeLegacyWrapper(afterTimeLdt.with(ChronoField.MILLI_OF_SECOND, 0)); boolean gotOne = false; // loop until we've computed the next time, or we've past the endTime while (gotOne == false) { - if (cl.get(Calendar.YEAR) > 2999) { // prevent endless loop... + if (cl.getYear() > 2999) { // prevent endless loop... return -1; } SortedSet st = null; int t = 0; - int sec = cl.get(Calendar.SECOND); - int min = cl.get(Calendar.MINUTE); + int sec = cl.getSecond(); + int min = cl.getMinute(); // get second................................................. st = seconds.tailSet(sec); @@ -319,12 +330,12 @@ public long getNextValidTimeAfter(final long time) { } else { sec = seconds.first(); min++; - cl.set(Calendar.MINUTE, min); + cl.setMinute(min); } - cl.set(Calendar.SECOND, sec); + cl.setSecond(sec); - min = cl.get(Calendar.MINUTE); - int hr = cl.get(Calendar.HOUR_OF_DAY); + min = cl.getMinute(); + int hr = cl.getHour(); t = -1; // get minute................................................. @@ -337,15 +348,15 @@ public long getNextValidTimeAfter(final long time) { hr++; } if (min != t) { - cl.set(Calendar.SECOND, 0); - cl.set(Calendar.MINUTE, min); - setCalendarHour(cl, hr); + cl.setSecond(0); + cl.setMinute(min); + cl.setHour(hr); continue; } - cl.set(Calendar.MINUTE, min); + cl.setMinute(min); - hr = cl.get(Calendar.HOUR_OF_DAY); - int day = cl.get(Calendar.DAY_OF_MONTH); + hr = cl.getHour(); + int day = cl.getDayOfMonth(); t = -1; // get hour................................................... @@ -358,16 +369,16 @@ public long getNextValidTimeAfter(final long time) { day++; } if (hr != t) { - cl.set(Calendar.SECOND, 0); - cl.set(Calendar.MINUTE, 0); - cl.set(Calendar.DAY_OF_MONTH, day); - setCalendarHour(cl, hr); + cl.setSecond(0); + cl.setMinute(0); + cl.setDayOfMonth(day); + cl.setHour(hr); continue; } - cl.set(Calendar.HOUR_OF_DAY, hr); + cl.setHour(hr); - day = cl.get(Calendar.DAY_OF_MONTH); - int mon = cl.get(Calendar.MONTH) + 1; + day = cl.getDayOfMonth(); + int mon = cl.getMonth() + 1; // '+ 1' because calendar is 0-based for this field, and we are // 1-based t = -1; @@ -381,32 +392,32 @@ public long getNextValidTimeAfter(final long time) { if (lastdayOfMonth) { if (nearestWeekday == false) { t = day; - day = getLastDayOfMonth(mon, cl.get(Calendar.YEAR)); + day = getLastDayOfMonth(mon, cl.getYear()); day -= lastdayOffset; if (t > day) { mon++; if (mon > 12) { mon = 1; tmon = 3333; // ensure test of mon != tmon further below fails - cl.add(Calendar.YEAR, 1); + cl.plusYears(1); } day = 1; } } else { t = day; - day = getLastDayOfMonth(mon, cl.get(Calendar.YEAR)); + day = getLastDayOfMonth(mon, cl.getYear()); day -= lastdayOffset; - Calendar tcal = Calendar.getInstance(UTC, Locale.ROOT); - tcal.set(Calendar.SECOND, 0); - tcal.set(Calendar.MINUTE, 0); - tcal.set(Calendar.HOUR_OF_DAY, 0); - tcal.set(Calendar.DAY_OF_MONTH, day); - tcal.set(Calendar.MONTH, mon - 1); - tcal.set(Calendar.YEAR, cl.get(Calendar.YEAR)); + LocalDateTimeLegacyWrapper tcal = new LocalDateTimeLegacyWrapper(LocalDateTime.now(timeZone)); + tcal.setSecond(0); + tcal.setMinute(0); + tcal.setHour(0); + tcal.setDayOfMonth(day); + tcal.setMonth(mon - 1); + tcal.setYear(cl.getYear()); - int ldom = getLastDayOfMonth(mon, cl.get(Calendar.YEAR)); - int dow = tcal.get(Calendar.DAY_OF_WEEK); + int ldom = getLastDayOfMonth(mon, cl.getYear()); + int dow = tcal.getDayOfWeek(); if (dow == Calendar.SATURDAY && day == 1) { day += 2; @@ -418,13 +429,12 @@ public long getNextValidTimeAfter(final long time) { day += 1; } - tcal.set(Calendar.SECOND, sec); - tcal.set(Calendar.MINUTE, min); - tcal.set(Calendar.HOUR_OF_DAY, hr); - tcal.set(Calendar.DAY_OF_MONTH, day); - tcal.set(Calendar.MONTH, mon - 1); - long nTime = tcal.getTimeInMillis(); - if (nTime < afterTime) { + tcal.setSecond(sec); + tcal.setMinute(min); + tcal.setHour(hr); + tcal.setDayOfMonth(day); + tcal.setMonth(mon - 1); + if (tcal.isBefore(afterTimeLdt)) { day = 1; mon++; } @@ -433,16 +443,16 @@ public long getNextValidTimeAfter(final long time) { t = day; day = daysOfMonth.first(); - Calendar tcal = Calendar.getInstance(UTC, Locale.ROOT); - tcal.set(Calendar.SECOND, 0); - tcal.set(Calendar.MINUTE, 0); - tcal.set(Calendar.HOUR_OF_DAY, 0); - tcal.set(Calendar.DAY_OF_MONTH, day); - tcal.set(Calendar.MONTH, mon - 1); - tcal.set(Calendar.YEAR, cl.get(Calendar.YEAR)); + LocalDateTimeLegacyWrapper tcal = new LocalDateTimeLegacyWrapper(LocalDateTime.now(timeZone)); + tcal.setSecond(0); + tcal.setMinute(0); + tcal.setHour(0); + tcal.setDayOfMonth(day); + tcal.setMonth(mon - 1); + tcal.setYear(cl.getYear()); - int ldom = getLastDayOfMonth(mon, cl.get(Calendar.YEAR)); - int dow = tcal.get(Calendar.DAY_OF_WEEK); + int ldom = getLastDayOfMonth(mon, cl.getYear()); + int dow = tcal.getDayOfWeek(); if (dow == Calendar.SATURDAY && day == 1) { day += 2; @@ -454,13 +464,12 @@ public long getNextValidTimeAfter(final long time) { day += 1; } - tcal.set(Calendar.SECOND, sec); - tcal.set(Calendar.MINUTE, min); - tcal.set(Calendar.HOUR_OF_DAY, hr); - tcal.set(Calendar.DAY_OF_MONTH, day); - tcal.set(Calendar.MONTH, mon - 1); - long nTime = tcal.getTimeInMillis(); - if (nTime < afterTime) { + tcal.setSecond(sec); + tcal.setMinute(min); + tcal.setHour(hr); + tcal.setDayOfMonth(day); + tcal.setMonth(mon - 1); + if (tcal.isAfter(afterTimeLdt)) { day = daysOfMonth.first(); mon++; } @@ -468,7 +477,7 @@ public long getNextValidTimeAfter(final long time) { t = day; day = st.first(); // make sure we don't over-run a short month, such as february - int lastDay = getLastDayOfMonth(mon, cl.get(Calendar.YEAR)); + int lastDay = getLastDayOfMonth(mon, cl.getYear()); if (day > lastDay) { day = daysOfMonth.first(); mon++; @@ -479,11 +488,11 @@ public long getNextValidTimeAfter(final long time) { } if (day != t || mon != tmon) { - cl.set(Calendar.SECOND, 0); - cl.set(Calendar.MINUTE, 0); - cl.set(Calendar.HOUR_OF_DAY, 0); - cl.set(Calendar.DAY_OF_MONTH, day); - cl.set(Calendar.MONTH, mon - 1); + cl.setSecond(0); + cl.setMinute(0); + cl.setHour(0); + cl.setDayOfMonth(day); + cl.setMonth(mon - 1); // '- 1' because calendar is 0-based for this field, and we // are 1-based continue; @@ -493,7 +502,7 @@ public long getNextValidTimeAfter(final long time) { // the month? int dow = daysOfWeek.first(); // desired // d-o-w - int cDow = cl.get(Calendar.DAY_OF_WEEK); // current d-o-w + int cDow = cl.getDayOfWeek(); // current d-o-w int daysToAdd = 0; if (cDow < dow) { daysToAdd = dow - cDow; @@ -502,15 +511,15 @@ public long getNextValidTimeAfter(final long time) { daysToAdd = dow + (7 - cDow); } - int lDay = getLastDayOfMonth(mon, cl.get(Calendar.YEAR)); + int lDay = getLastDayOfMonth(mon, cl.getYear()); if (day + daysToAdd > lDay) { // did we already miss the // last one? - cl.set(Calendar.SECOND, 0); - cl.set(Calendar.MINUTE, 0); - cl.set(Calendar.HOUR_OF_DAY, 0); - cl.set(Calendar.DAY_OF_MONTH, 1); - cl.set(Calendar.MONTH, mon); + cl.setSecond(0); + cl.setMinute(0); + cl.setHour(0); + cl.setDayOfMonth(1); + cl.setMonth(mon); // no '- 1' here because we are promoting the month continue; } @@ -523,11 +532,11 @@ public long getNextValidTimeAfter(final long time) { day += daysToAdd; if (daysToAdd > 0) { - cl.set(Calendar.SECOND, 0); - cl.set(Calendar.MINUTE, 0); - cl.set(Calendar.HOUR_OF_DAY, 0); - cl.set(Calendar.DAY_OF_MONTH, day); - cl.set(Calendar.MONTH, mon - 1); + cl.setSecond(0); + cl.setMinute(0); + cl.setHour(0); + cl.setDayOfMonth(day); + cl.setMonth(mon - 1); // '- 1' here because we are not promoting the month continue; } @@ -536,7 +545,7 @@ public long getNextValidTimeAfter(final long time) { // are we looking for the Nth XXX day in the month? int dow = daysOfWeek.first(); // desired // d-o-w - int cDow = cl.get(Calendar.DAY_OF_WEEK); // current d-o-w + int cDow = cl.getDayOfWeek(); // current d-o-w int daysToAdd = 0; if (cDow < dow) { daysToAdd = dow - cDow; @@ -557,25 +566,25 @@ public long getNextValidTimeAfter(final long time) { daysToAdd = (nthdayOfWeek - weekOfMonth) * 7; day += daysToAdd; - if (daysToAdd < 0 || day > getLastDayOfMonth(mon, cl.get(Calendar.YEAR))) { - cl.set(Calendar.SECOND, 0); - cl.set(Calendar.MINUTE, 0); - cl.set(Calendar.HOUR_OF_DAY, 0); - cl.set(Calendar.DAY_OF_MONTH, 1); - cl.set(Calendar.MONTH, mon); + if (daysToAdd < 0 || day > getLastDayOfMonth(mon, cl.getYear())) { + cl.setSecond(0); + cl.setMinute(0); + cl.setHour(0); + cl.setDayOfMonth(1); + cl.setMonth(mon); // no '- 1' here because we are promoting the month continue; } else if (daysToAdd > 0 || dayShifted) { - cl.set(Calendar.SECOND, 0); - cl.set(Calendar.MINUTE, 0); - cl.set(Calendar.HOUR_OF_DAY, 0); - cl.set(Calendar.DAY_OF_MONTH, day); - cl.set(Calendar.MONTH, mon - 1); + cl.setSecond(0); + cl.setMinute(0); + cl.setHour(0); + cl.setDayOfMonth(day); + cl.setMonth(mon - 1); // '- 1' here because we are NOT promoting the month continue; } } else { - int cDow = cl.get(Calendar.DAY_OF_WEEK); // current d-o-w + int cDow = cl.getDayOfWeek(); // current d-o-w int dow = daysOfWeek.first(); // desired // d-o-w st = daysOfWeek.tailSet(cDow); @@ -591,23 +600,23 @@ public long getNextValidTimeAfter(final long time) { daysToAdd = dow + (7 - cDow); } - int lDay = getLastDayOfMonth(mon, cl.get(Calendar.YEAR)); + int lDay = getLastDayOfMonth(mon, cl.getYear()); if (day + daysToAdd > lDay) { // will we pass the end of // the month? - cl.set(Calendar.SECOND, 0); - cl.set(Calendar.MINUTE, 0); - cl.set(Calendar.HOUR_OF_DAY, 0); - cl.set(Calendar.DAY_OF_MONTH, 1); - cl.set(Calendar.MONTH, mon); + cl.setSecond(0); + cl.setMinute(0); + cl.setHour(0); + cl.setDayOfMonth(1); + cl.setMonth(mon); // no '- 1' here because we are promoting the month continue; } else if (daysToAdd > 0) { // are we swithing days? - cl.set(Calendar.SECOND, 0); - cl.set(Calendar.MINUTE, 0); - cl.set(Calendar.HOUR_OF_DAY, 0); - cl.set(Calendar.DAY_OF_MONTH, day + daysToAdd); - cl.set(Calendar.MONTH, mon - 1); + cl.setSecond(0); + cl.setMinute(0); + cl.setHour(0); + cl.setDayOfMonth(day + daysToAdd); + cl.setMonth(mon - 1); // '- 1' because calendar is 0-based for this field, // and we are 1-based continue; @@ -618,12 +627,12 @@ public long getNextValidTimeAfter(final long time) { // throw new UnsupportedOperationException( // "Support for specifying both a day-of-week AND a day-of-month parameter is not implemented."); } - cl.set(Calendar.DAY_OF_MONTH, day); + cl.setDayOfMonth(day); - mon = cl.get(Calendar.MONTH) + 1; + mon = cl.getMonth() + 1; // '+ 1' because calendar is 0-based for this field, and we are // 1-based - int year = cl.get(Calendar.YEAR); + int year = cl.getYear(); t = -1; // test for expressions that never generate a valid fire date, @@ -643,21 +652,21 @@ public long getNextValidTimeAfter(final long time) { year++; } if (mon != t) { - cl.set(Calendar.SECOND, 0); - cl.set(Calendar.MINUTE, 0); - cl.set(Calendar.HOUR_OF_DAY, 0); - cl.set(Calendar.DAY_OF_MONTH, 1); - cl.set(Calendar.MONTH, mon - 1); + cl.setSecond(0); + cl.setMinute(0); + cl.setHour(0); + cl.setDayOfMonth(1); + cl.setMonth(mon - 1); // '- 1' because calendar is 0-based for this field, and we are // 1-based - cl.set(Calendar.YEAR, year); + cl.setYear(year); continue; } - cl.set(Calendar.MONTH, mon - 1); + cl.setMonth(mon - 1); // '- 1' because calendar is 0-based for this field, and we are // 1-based - year = cl.get(Calendar.YEAR); + year = cl.getYear(); t = -1; // get year................................................... @@ -671,22 +680,24 @@ public long getNextValidTimeAfter(final long time) { } if (year != t) { - cl.set(Calendar.SECOND, 0); - cl.set(Calendar.MINUTE, 0); - cl.set(Calendar.HOUR_OF_DAY, 0); - cl.set(Calendar.DAY_OF_MONTH, 1); - cl.set(Calendar.MONTH, 0); + cl.setSecond(0); + cl.setMinute(0); + cl.setHour(0); + cl.setDayOfMonth(1); + cl.setMonth(0); // '- 1' because calendar is 0-based for this field, and we are // 1-based - cl.set(Calendar.YEAR, year); + cl.setYear(year); continue; } - cl.set(Calendar.YEAR, year); + cl.setYear(year); gotOne = true; } // while( done == false ) - return cl.getTimeInMillis(); + LocalDateTime nextRuntime = cl.getLocalDateTime(); + + return nextRuntime.atZone(timeZone).toInstant().toEpochMilli(); } public String expression() { @@ -735,7 +746,7 @@ public String getExpressionSummary() { @Override public int hashCode() { - return Objects.hash(expression); + return Objects.hash(expression, timeZone); } @Override @@ -747,7 +758,7 @@ public boolean equals(Object obj) { return false; } final Cron other = (Cron) obj; - return Objects.equals(this.expression, other.expression); + return Objects.equals(this.expression, other.expression) && Objects.equals(this.timeZone, other.timeZone); } /** @@ -757,7 +768,7 @@ public boolean equals(Object obj) { */ @Override public String toString() { - return expression; + return "Cron{" + "timeZone=" + timeZone + ", expression='" + expression + '\'' + '}'; } /** @@ -1430,7 +1441,7 @@ private static int getLastDayOfMonth(int monthNum, int year) { @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - return builder.value(toString()); + return builder.value(expression); } private static class ValueSet { diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/scheduler/LocalDateTimeLegacyWrapper.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/scheduler/LocalDateTimeLegacyWrapper.java new file mode 100644 index 0000000000000..e540acc8042eb --- /dev/null +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/scheduler/LocalDateTimeLegacyWrapper.java @@ -0,0 +1,130 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.core.scheduler; + +import java.time.LocalDateTime; +import java.time.chrono.ChronoLocalDateTime; + +/** + * This class is designed to wrap the LocalDateTime class in order to make it behave, in terms of mutation, like a legacy Calendar class. + * This is to provide compatibility with the existing Cron next runtime calculation algorithm which relies on certain quirks of the Calendar + * such as days of the week being numbered starting on Sunday==1 and being able to set the current hour to 24 and have it roll over to + * midnight the next day. + */ +public class LocalDateTimeLegacyWrapper { + + private LocalDateTime ldt; + + public LocalDateTimeLegacyWrapper(LocalDateTime ldt) { + this.ldt = ldt; + } + + public int getYear() { + return ldt.getYear(); + } + + public int getDayOfMonth() { + return ldt.getDayOfMonth(); + } + + public int getHour() { + return ldt.getHour(); + } + + public int getMinute() { + return ldt.getMinute(); + } + + public int getSecond() { + return ldt.getSecond(); + } + + public int getDayOfWeek() { + return (ldt.getDayOfWeek().getValue() % 7) + 1; + } + + public int getMonth() { + return ldt.getMonthValue() - 1; + } + + public void setYear(int year) { + ldt = ldt.withYear(year); + } + + public void setDayOfMonth(int dayOfMonth) { + var lengthOfMonth = ldt.getMonth().length(ldt.toLocalDate().isLeapYear()); + if (dayOfMonth <= lengthOfMonth) { + ldt = ldt.withDayOfMonth(dayOfMonth); + } else { + var months = dayOfMonth / lengthOfMonth; + var day = dayOfMonth % lengthOfMonth; + ldt = ldt.plusMonths(months).withDayOfMonth(day); + } + } + + public void setMonth(int month) { + month++; // Months are 0-based in Calendar + if (month <= 12) { + ldt = ldt.withMonth(month); + } else { + var years = month / 12; + var monthOfYear = month % 12; + ldt = ldt.plusYears(years).withMonth(monthOfYear); + } + } + + public void setHour(int hour) { + if (hour < 24) { + ldt = ldt.withHour(hour); + } else { + var days = hour / 24; + var hourOfDay = hour % 24; + ldt = ldt.plusDays(days).withHour(hourOfDay); + } + } + + public void setMinute(int minute) { + if (minute < 60) { + ldt = ldt.withMinute(minute); + } else { + var hours = minute / 60; + var minuteOfHour = minute % 60; + ldt = ldt.plusHours(hours).withMinute(minuteOfHour); + } + } + + public void setSecond(int second) { + if (second < 60) { + ldt = ldt.withSecond(second); + } else { + var minutes = second / 60; + var secondOfMinute = second % 60; + ldt = ldt.plusMinutes(minutes).withSecond(secondOfMinute); + } + } + + public void plusYears(long years) { + ldt = ldt.plusYears(years); + } + + public void plusSeconds(long seconds) { + ldt = ldt.plusSeconds(seconds); + } + + public boolean isAfter(ChronoLocalDateTime other) { + return ldt.isAfter(other); + } + + public boolean isBefore(ChronoLocalDateTime other) { + return ldt.isBefore(other); + } + + public LocalDateTime getLocalDateTime() { + return ldt; + } +} diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/scheduler/CronTimezoneTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/scheduler/CronTimezoneTests.java new file mode 100644 index 0000000000000..1e469002457d8 --- /dev/null +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/scheduler/CronTimezoneTests.java @@ -0,0 +1,231 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.core.scheduler; + +import org.elasticsearch.test.ESTestCase; +import org.hamcrest.Matchers; + +import java.time.Instant; +import java.time.LocalDateTime; +import java.time.ZoneId; +import java.time.ZoneOffset; +import java.time.ZonedDateTime; +import java.time.temporal.ChronoUnit; +import java.time.zone.ZoneOffsetTransition; +import java.time.zone.ZoneRules; + +import static java.time.Instant.ofEpochMilli; +import static java.util.TimeZone.getTimeZone; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.not; + +public class CronTimezoneTests extends ESTestCase { + + public void testForFixedOffsetCorrectlyCalculateNextRuntime() { + Cron cron = new Cron("0 0 2 * * ?", ZoneOffset.of("+1")); + long midnightUTC = Instant.parse("2020-01-01T00:00:00Z").toEpochMilli(); + long nextValidTimeAfter = cron.getNextValidTimeAfter(midnightUTC); + assertThat(Instant.ofEpochMilli(nextValidTimeAfter), equalTo(Instant.parse("2020-01-01T01:00:00Z"))); + } + + public void testForLondonFixedDSTTransitionCheckCorrectSchedule() { + ZoneId londonZone = getTimeZone("Europe/London").toZoneId(); + + Cron cron = new Cron("0 0 2 * * ?", londonZone); + ZoneRules londonZoneRules = londonZone.getRules(); + Instant springMidnight = Instant.parse("2020-03-01T00:00:00Z"); + long timeBeforeDST = springMidnight.toEpochMilli(); + + assertThat(cron.getNextValidTimeAfter(timeBeforeDST), equalTo(Instant.parse("2020-03-01T02:00:00Z").toEpochMilli())); + + ZoneOffsetTransition zoneOffsetTransition = londonZoneRules.nextTransition(springMidnight); + + Instant timeAfterDST = zoneOffsetTransition.getDateTimeBefore() + .plusDays(1) + .atZone(ZoneOffset.UTC) + .withHour(0) + .withMinute(0) + .toInstant(); + + assertThat(cron.getNextValidTimeAfter(timeAfterDST.toEpochMilli()), equalTo(Instant.parse("2020-03-30T01:00:00Z").toEpochMilli())); + } + + public void testRandomDSTTransitionCalculateNextTimeCorrectlyRelativeToUTC() { + ZoneId timeZone = generateRandomDSTZone(); + + logger.info("Testing for timezone {}", timeZone); + + ZoneOffsetTransition zoneOffsetTransition = timeZone.getRules().nextTransition(Instant.now()); + + ZonedDateTime midnightBefore = zoneOffsetTransition.getDateTimeBefore().atZone(timeZone).minusDays(2).withHour(0).withMinute(0); + ZonedDateTime midnightAfter = zoneOffsetTransition.getDateTimeAfter().atZone(timeZone).plusDays(2).withHour(0).withMinute(0); + + long epochBefore = midnightBefore.toInstant().toEpochMilli(); + long epochAfter = midnightAfter.toInstant().toEpochMilli(); + + Cron cron = new Cron("0 0 2 * * ?", timeZone); + + long nextScheduleBefore = cron.getNextValidTimeAfter(epochBefore); + long nextScheduleAfter = cron.getNextValidTimeAfter(epochAfter); + + assertThat(nextScheduleBefore - epochBefore, equalTo(2 * 60 * 60 * 1000L)); // 2 hours + assertThat(nextScheduleAfter - epochAfter, equalTo(2 * 60 * 60 * 1000L)); // 2 hours + + ZonedDateTime utcMidnightBefore = zoneOffsetTransition.getDateTimeBefore() + .atZone(ZoneOffset.UTC) + .minusDays(2) + .withHour(0) + .withMinute(0); + + ZonedDateTime utcMidnightAfter = zoneOffsetTransition.getDateTimeAfter() + .atZone(ZoneOffset.UTC) + .plusDays(2) + .withHour(0) + .withMinute(0); + + long utcEpochBefore = utcMidnightBefore.toInstant().toEpochMilli(); + long utcEpochAfter = utcMidnightAfter.toInstant().toEpochMilli(); + + long nextUtcScheduleBefore = cron.getNextValidTimeAfter(utcEpochBefore); + long nextUtcScheduleAfter = cron.getNextValidTimeAfter(utcEpochAfter); + + assertThat(nextUtcScheduleBefore - utcEpochBefore, not(equalTo(nextUtcScheduleAfter - utcEpochAfter))); + + } + + private ZoneId generateRandomDSTZone() { + ZoneId timeZone; + int i = 0; + boolean found; + do { + timeZone = randomZone(); + found = getTimeZone(timeZone).useDaylightTime(); + i++; + } while (found == false && i <= 500); // Infinite loop prevention + + if (found == false) { + fail("Could not find a timezone with DST"); + } + + logger.debug("Testing for timezone {} after {} iterations", timeZone, i); + return timeZone; + } + + public void testForGMTGapTransitionTriggerTimeIsAsIfTransitionHasntHappenedYet() { + ZoneId london = ZoneId.of("Europe/London"); + Cron cron = new Cron("0 30 1 * * ?", london); // Every day at 1:30 + + Instant beforeTransition = Instant.parse("2025-03-30T00:00:00Z"); + long beforeTransitionEpoch = beforeTransition.toEpochMilli(); + + long nextValidTimeAfter = cron.getNextValidTimeAfter(beforeTransitionEpoch); + assertThat(ofEpochMilli(nextValidTimeAfter), equalTo(Instant.parse("2025-03-30T01:30:00Z"))); + } + + public void testForGMTOverlapTransitionTriggerSkipSecondExecution() { + ZoneId london = ZoneId.of("Europe/London"); + Cron cron = new Cron("0 30 1 * * ?", london); // Every day at 01:30 + + Instant beforeTransition = Instant.parse("2024-10-27T00:00:00Z"); + long beforeTransitionEpoch = beforeTransition.toEpochMilli(); + + long firstValidTimeAfter = cron.getNextValidTimeAfter(beforeTransitionEpoch); + assertThat(ofEpochMilli(firstValidTimeAfter), equalTo(Instant.parse("2024-10-27T00:30:00Z"))); + + long nextValidTimeAfter = cron.getNextValidTimeAfter(firstValidTimeAfter); + assertThat(ofEpochMilli(nextValidTimeAfter), equalTo(Instant.parse("2024-10-28T01:30:00Z"))); + } + + // This test checks that once per minute crons will be unaffected by a DST transition + public void testDiscontinuityResolutionForNonHourCronInRandomTimezone() { + var timezone = generateRandomDSTZone(); + + var cron = new Cron("0 * * * * ?", timezone); // Once per minute + + Instant referenceTime = randomInstantBetween(Instant.now(), Instant.now().plus(1826, ChronoUnit.DAYS)); // ~5 years + ZoneOffsetTransition transition1 = timezone.getRules().nextTransition(referenceTime); + + // Currently there are no known timezones with DST transitions shorter than 10 minutes but this guards against future changes + if (Math.abs(transition1.getOffsetBefore().getTotalSeconds() - transition1.getOffsetAfter().getTotalSeconds()) < 600) { + fail("Transition is not long enough to test"); + } + + testNonHourCronTransition(transition1, cron); + + var transition2 = timezone.getRules().nextTransition(transition1.getInstant().plus(1, ChronoUnit.DAYS)); + + testNonHourCronTransition(transition2, cron); + + } + + private static void testNonHourCronTransition(ZoneOffsetTransition transition, Cron cron) { + Instant insideTransition; + if (transition.isGap()) { + insideTransition = transition.getInstant().plus(10, ChronoUnit.MINUTES); + Instant nextTrigger = ofEpochMilli(cron.getNextValidTimeAfter(insideTransition.toEpochMilli())); + assertThat(nextTrigger, equalTo(insideTransition.plus(1, ChronoUnit.MINUTES))); + } else { + insideTransition = transition.getInstant().minus(10, ChronoUnit.MINUTES); + Instant nextTrigger = ofEpochMilli(cron.getNextValidTimeAfter(insideTransition.toEpochMilli())); + assertThat(nextTrigger, equalTo(insideTransition.plus(1, ChronoUnit.MINUTES))); + + insideTransition = insideTransition.plus(transition.getDuration()); + nextTrigger = ofEpochMilli(cron.getNextValidTimeAfter(insideTransition.toEpochMilli())); + assertThat(nextTrigger, equalTo(insideTransition.plus(1, ChronoUnit.MINUTES))); + } + } + + // This test checks that once per day crons will behave correctly during a DST transition + public void testDiscontinuityResolutionForCronInRandomTimezone() { + var timezone = generateRandomDSTZone(); + + Instant referenceTime = randomInstantBetween(Instant.now(), Instant.now().plus(1826, ChronoUnit.DAYS)); // ~5 years + ZoneOffsetTransition transition1 = timezone.getRules().nextTransition(referenceTime); + + // Currently there are no known timezones with DST transitions shorter than 10 minutes but this guards against future changes + if (Math.abs(transition1.getOffsetBefore().getTotalSeconds() - transition1.getOffsetAfter().getTotalSeconds()) < 600) { + fail("Transition is not long enough to test"); + } + + testHourCronTransition(transition1, timezone); + + var transition2 = timezone.getRules().nextTransition(transition1.getInstant().plus(1, ChronoUnit.DAYS)); + + testHourCronTransition(transition2, timezone); + } + + private static void testHourCronTransition(ZoneOffsetTransition transition, ZoneId timezone) { + if (transition.isGap()) { + LocalDateTime targetTime = transition.getDateTimeBefore().plusMinutes(10); + + var cron = new Cron("0 " + targetTime.getMinute() + " " + targetTime.getHour() + " * * ?", timezone); + + long nextTrigger = cron.getNextValidTimeAfter(transition.getInstant().minus(10, ChronoUnit.MINUTES).toEpochMilli()); + + assertThat(ofEpochMilli(nextTrigger), equalTo(transition.getInstant().plus(10, ChronoUnit.MINUTES))); + } else { + LocalDateTime targetTime = transition.getDateTimeAfter().plusMinutes(10); + var cron = new Cron("0 " + targetTime.getMinute() + " " + targetTime.getHour() + " * * ?", timezone); + + long transitionLength = Math.abs(transition.getDuration().toSeconds()); + long firstTrigger = cron.getNextValidTimeAfter( + transition.getInstant().minusSeconds(transitionLength).minus(10, ChronoUnit.MINUTES).toEpochMilli() + ); + + assertThat( + ofEpochMilli(firstTrigger), + equalTo(transition.getInstant().minusSeconds(transitionLength).plus(10, ChronoUnit.MINUTES)) + ); + + var repeatTrigger = cron.getNextValidTimeAfter(firstTrigger + (1000 * 60L)); // 1 minute + + assertThat(repeatTrigger - firstTrigger, Matchers.greaterThan(24 * 60 * 60 * 1000L)); // 24 hours + } + } + +} diff --git a/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/ConnectorCustomSchedule.java b/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/ConnectorCustomSchedule.java index 7badf6926c574..387224408a14b 100644 --- a/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/ConnectorCustomSchedule.java +++ b/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/ConnectorCustomSchedule.java @@ -140,7 +140,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws public void writeTo(StreamOutput out) throws IOException { out.writeWriteable(configurationOverrides); out.writeBoolean(enabled); - out.writeString(interval.toString()); + out.writeString(interval.expression()); out.writeOptionalInstant(lastSynced); out.writeString(name); } diff --git a/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/ConnectorScheduling.java b/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/ConnectorScheduling.java index 3c08a5ac1e218..008cbca0cd5ea 100644 --- a/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/ConnectorScheduling.java +++ b/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/ConnectorScheduling.java @@ -222,7 +222,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws @Override public void writeTo(StreamOutput out) throws IOException { out.writeBoolean(enabled); - out.writeString(interval.toString()); + out.writeString(interval.expression()); } @Override diff --git a/x-pack/plugin/slm/src/test/java/org/elasticsearch/xpack/slm/SnapshotLifecyclePolicyTests.java b/x-pack/plugin/slm/src/test/java/org/elasticsearch/xpack/slm/SnapshotLifecyclePolicyTests.java index b7674a2d60bff..0ab3e99e1efc9 100644 --- a/x-pack/plugin/slm/src/test/java/org/elasticsearch/xpack/slm/SnapshotLifecyclePolicyTests.java +++ b/x-pack/plugin/slm/src/test/java/org/elasticsearch/xpack/slm/SnapshotLifecyclePolicyTests.java @@ -64,12 +64,12 @@ public void testNextExecutionTimeSchedule() { SnapshotLifecyclePolicy p = new SnapshotLifecyclePolicy( "id", "name", - "0 1 2 3 4 ? 2099", + "0 1 2 3 4 ? 2049", "repo", Collections.emptyMap(), SnapshotRetentionConfiguration.EMPTY ); - assertThat(p.calculateNextExecution(-1, Clock.systemUTC()), equalTo(4078864860000L)); + assertThat(p.calculateNextExecution(-1, Clock.systemUTC()), equalTo(2501028060000L)); } public void testNextExecutionTimeInterval() { diff --git a/x-pack/plugin/watcher/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/watcher/put_watch/11_timezoned_schedules.yml b/x-pack/plugin/watcher/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/watcher/put_watch/11_timezoned_schedules.yml new file mode 100644 index 0000000000000..0371443367603 --- /dev/null +++ b/x-pack/plugin/watcher/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/watcher/put_watch/11_timezoned_schedules.yml @@ -0,0 +1,121 @@ +--- +setup: + - do: + cluster.health: + wait_for_status: yellow + +--- +"Test put watch api with timezone": + - do: + watcher.put_watch: + id: "my_watch" + body: > + { + "trigger": { + "schedule": { + "timezone": "America/Los_Angeles", + "hourly": { + "minute": [ 0, 5 ] + } + } + }, + "input": { + "simple": { + "payload": { + "send": "yes" + } + } + }, + "condition": { + "always": {} + }, + "actions": { + "test_index": { + "index": { + "index": "test" + } + } + } + } + - match: { _id: "my_watch" } + - do: + watcher.get_watch: + id: "my_watch" + - match: { watch.trigger.schedule.timezone: "America/Los_Angeles" } + +--- +"Test put watch api without timezone": + - do: + watcher.put_watch: + id: "my_watch" + body: > + { + "trigger": { + "schedule": { + "hourly": { + "minute": [ 0, 5 ] + } + } + }, + "input": { + "simple": { + "payload": { + "send": "yes" + } + } + }, + "condition": { + "always": {} + }, + "actions": { + "test_index": { + "index": { + "index": "test" + } + } + } + } + - match: { _id: "my_watch" } + - do: + watcher.get_watch: + id: "my_watch" + - is_false: watch.trigger.schedule.timezone + +--- +"Reject put watch with invalid timezone": + - do: + watcher.put_watch: + id: "my_watch" + body: > + { + "trigger": { + "schedule": { + "timezone": "Pangea/Tethys", + "hourly": { + "minute": [ 0, 5 ] + } + } + }, + "input": { + "simple": { + "payload": { + "send": "yes" + } + } + }, + "condition": { + "always": {} + }, + "actions": { + "test_index": { + "index": { + "index": "test" + } + } + } + } + catch: bad_request + - match: { error.type: "parse_exception" } + - match: { error.reason: "could not parse schedule. invalid timezone [Pangea/Tethys]" } + - match: { error.caused_by.type: "zone_rules_exception" } + - match: { error.caused_by.reason: "Unknown time-zone ID: Pangea/Tethys" } 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 63e9dae88de41..0db99af9b3fc2 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 @@ -8,6 +8,7 @@ import org.elasticsearch.xpack.core.scheduler.Cron; +import java.time.ZoneId; import java.util.Arrays; import java.util.Comparator; import java.util.Objects; @@ -17,6 +18,7 @@ public abstract class CronnableSchedule implements Schedule { private static final Comparator CRON_COMPARATOR = Comparator.comparing(Cron::expression); protected final Cron[] crons; + private ZoneId timeZone; CronnableSchedule(String... expressions) { this(crons(expressions)); @@ -28,6 +30,17 @@ private CronnableSchedule(Cron... crons) { Arrays.sort(crons, CRON_COMPARATOR); } + protected void setTimeZone(ZoneId timeZone) { + this.timeZone = timeZone; + for (Cron cron : crons) { + cron.setTimeZone(timeZone); + } + } + + public ZoneId getTimeZone() { + return timeZone; + } + @Override public long nextScheduledTimeAfter(long startTime, long time) { assert time >= startTime; @@ -45,21 +58,22 @@ public Cron[] crons() { return crons; } + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + CronnableSchedule that = (CronnableSchedule) o; + return Objects.deepEquals(crons, that.crons) && Objects.equals(timeZone, that.timeZone); + } + @Override public int hashCode() { - return Objects.hash((Object[]) crons); + return Objects.hash(Arrays.hashCode(crons), timeZone); } @Override - public boolean equals(Object obj) { - if (this == obj) { - return true; - } - if (obj == null || getClass() != obj.getClass()) { - return false; - } - final CronnableSchedule other = (CronnableSchedule) obj; - return Objects.deepEquals(this.crons, other.crons); + public String toString() { + return "CronnableSchedule{" + "crons=" + Arrays.toString(crons) + ", timeZone=" + timeZone + '}'; } static Cron[] crons(String... expressions) { diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/trigger/schedule/ScheduleRegistry.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/trigger/schedule/ScheduleRegistry.java index 31cf46f8abaac..5d2259db71f77 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/trigger/schedule/ScheduleRegistry.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/trigger/schedule/ScheduleRegistry.java @@ -8,8 +8,11 @@ import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.xcontent.XContentParser; +import org.elasticsearch.xpack.watcher.trigger.schedule.support.TimezoneUtils; import java.io.IOException; +import java.time.DateTimeException; +import java.time.ZoneId; import java.util.HashMap; import java.util.Map; import java.util.Set; @@ -29,9 +32,15 @@ public Schedule parse(String context, XContentParser parser) throws IOException String type = null; XContentParser.Token token; Schedule schedule = null; + ZoneId timeZone = null; // Default to UTC while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { if (token == XContentParser.Token.FIELD_NAME) { - type = parser.currentName(); + var fieldName = parser.currentName(); + if (fieldName.equals(ScheduleTrigger.TIMEZONE_FIELD)) { + timeZone = parseTimezone(parser); + } else { + type = parser.currentName(); + } } else if (type != null) { schedule = parse(context, type, parser); } else { @@ -44,9 +53,38 @@ public Schedule parse(String context, XContentParser parser) throws IOException if (schedule == null) { throw new ElasticsearchParseException("could not parse schedule. expected a schedule type field, but no fields were found"); } + + if (timeZone != null && schedule instanceof CronnableSchedule cronnableSchedule) { + cronnableSchedule.setTimeZone(timeZone); + } else if (timeZone != null) { + throw new ElasticsearchParseException( + "could not parse schedule. Timezone is not supported for schedule type [{}]", + schedule.type() + ); + } + return schedule; } + private static ZoneId parseTimezone(XContentParser parser) throws IOException { + ZoneId timeZone; + XContentParser.Token token = parser.nextToken(); + if (token == XContentParser.Token.VALUE_STRING) { + String text = parser.text(); + try { + timeZone = TimezoneUtils.parse(text); + } catch (DateTimeException e) { + throw new ElasticsearchParseException("could not parse schedule. invalid timezone [{}]", e, text); + } + } else { + throw new ElasticsearchParseException( + "could not parse schedule. expected a string value for timezone, but found [{}] instead", + token + ); + } + return timeZone; + } + public Schedule parse(String context, String type, XContentParser parser) throws IOException { Schedule.Parser scheduleParser = parsers.get(type); if (scheduleParser == null) { diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/trigger/schedule/ScheduleTrigger.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/trigger/schedule/ScheduleTrigger.java index 4a67841e6c88e..cc6ec8f5aaa57 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/trigger/schedule/ScheduleTrigger.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/trigger/schedule/ScheduleTrigger.java @@ -14,6 +14,7 @@ public class ScheduleTrigger implements Trigger { public static final String TYPE = "schedule"; + public static final String TIMEZONE_FIELD = "timezone"; private final Schedule schedule; @@ -49,7 +50,13 @@ public int hashCode() { @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - return builder.startObject().field(schedule.type(), schedule, params).endObject(); + builder.startObject(); + if (schedule instanceof CronnableSchedule cronnableSchedule && cronnableSchedule.getTimeZone() != null) { + builder.field(TIMEZONE_FIELD, cronnableSchedule.getTimeZone().getId()); + } + + builder.field(schedule.type(), schedule, params); + return builder.endObject(); } public static Builder builder(Schedule schedule) { diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/trigger/schedule/support/TimezoneUtils.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/trigger/schedule/support/TimezoneUtils.java new file mode 100644 index 0000000000000..c77fdda803bec --- /dev/null +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/trigger/schedule/support/TimezoneUtils.java @@ -0,0 +1,55 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.watcher.trigger.schedule.support; + +import java.time.DateTimeException; +import java.time.ZoneId; +import java.util.Locale; +import java.util.Map; + +import static java.util.stream.Collectors.toMap; + +/** + * Utility class for dealing with Timezone related operations. + */ +public class TimezoneUtils { + + private static final Map caseInsensitiveTZLookup; + + static { + caseInsensitiveTZLookup = ZoneId.getAvailableZoneIds() + .stream() + .collect(toMap(zoneId -> zoneId.toLowerCase(Locale.ROOT), ZoneId::of)); + } + + /** + * Parses a timezone string into a {@link ZoneId} object. The timezone string can be a valid timezone ID, or a + * timezone offset string and is case-insensitive. + * + * @param timezoneString The timezone string to parse + * @return The parsed {@link ZoneId} object + * @throws DateTimeException If the timezone string is not a valid timezone ID or offset + */ + public static ZoneId parse(String timezoneString) throws DateTimeException { + try { + return ZoneId.of(timezoneString); + } catch (DateTimeException e) { + ZoneId timeZone = caseInsensitiveTZLookup.get(timezoneString.toLowerCase(Locale.ROOT)); + if (timeZone != null) { + return timeZone; + } + try { + return ZoneId.of(timezoneString.toUpperCase(Locale.ROOT)); + } catch (DateTimeException ignored) { + // ignore + } + throw e; + } + } + +} diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/trigger/schedule/ScheduleRegistryTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/trigger/schedule/ScheduleRegistryTests.java index 7fc4739c342f1..aa39701d207c3 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/trigger/schedule/ScheduleRegistryTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/trigger/schedule/ScheduleRegistryTests.java @@ -12,6 +12,7 @@ import org.elasticsearch.xcontent.json.JsonXContent; import org.junit.Before; +import java.time.ZoneId; import java.util.HashSet; import java.util.Set; @@ -49,15 +50,23 @@ public void testParserInterval() throws Exception { } public void testParseCron() throws Exception { - Object cron = randomBoolean() ? Schedules.cron("* 0/5 * * * ?") : Schedules.cron("* 0/2 * * * ?", "* 0/3 * * * ?", "* 0/5 * * * ?"); - XContentBuilder builder = jsonBuilder().startObject().field(CronSchedule.TYPE, cron).endObject(); + var cron = randomBoolean() ? Schedules.cron("* 0/5 * * * ?") : Schedules.cron("* 0/2 * * * ?", "* 0/3 * * * ?", "* 0/5 * * * ?"); + ZoneId timeZone = null; + XContentBuilder builder = jsonBuilder().startObject().field(CronSchedule.TYPE, cron); + if (randomBoolean()) { + timeZone = randomTimeZone().toZoneId(); + cron.setTimeZone(timeZone); + builder.field(ScheduleTrigger.TIMEZONE_FIELD, timeZone.getId()); + } + builder.endObject(); BytesReference bytes = BytesReference.bytes(builder); XContentParser parser = createParser(JsonXContent.jsonXContent, bytes); parser.nextToken(); - Schedule schedule = registry.parse("ctx", parser); + CronnableSchedule schedule = (CronnableSchedule) registry.parse("ctx", parser); assertThat(schedule, notNullValue()); assertThat(schedule, instanceOf(CronSchedule.class)); assertThat(schedule, is(cron)); + assertThat(schedule.getTimeZone(), equalTo(timeZone)); } public void testParseHourly() throws Exception { diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/trigger/schedule/support/TimezoneUtilsTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/trigger/schedule/support/TimezoneUtilsTests.java new file mode 100644 index 0000000000000..aa797ec610eca --- /dev/null +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/trigger/schedule/support/TimezoneUtilsTests.java @@ -0,0 +1,40 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.watcher.trigger.schedule.support; + +import org.elasticsearch.test.ESTestCase; + +import java.time.ZoneId; +import java.util.Locale; + +import static org.hamcrest.Matchers.equalTo; + +public class TimezoneUtilsTests extends ESTestCase { + + public void testExpectedFormatParsing() { + assertThat(TimezoneUtils.parse("Europe/London").getId(), equalTo("Europe/London")); + assertThat(TimezoneUtils.parse("+1").getId(), equalTo("+01:00")); + assertThat(TimezoneUtils.parse("GMT+01:00").getId(), equalTo("GMT+01:00")); + } + + public void testParsingIsCaseInsensitive() { + ZoneId timeZone = randomTimeZone().toZoneId(); + assertThat(TimezoneUtils.parse(timeZone.getId()), equalTo(timeZone)); + assertThat(TimezoneUtils.parse(timeZone.getId().toLowerCase(Locale.ROOT)), equalTo(timeZone)); + assertThat(TimezoneUtils.parse(timeZone.getId().toUpperCase(Locale.ROOT)), equalTo(timeZone)); + } + + public void testParsingOffsets() { + ZoneId timeZone = ZoneId.of("GMT+01:00"); + assertThat(TimezoneUtils.parse("GMT+01:00"), equalTo(timeZone)); + assertThat(TimezoneUtils.parse("gmt+01:00"), equalTo(timeZone)); + assertThat(TimezoneUtils.parse("GMT+1"), equalTo(timeZone)); + + assertThat(TimezoneUtils.parse("+1"), equalTo(ZoneId.of("+01:00"))); + } +}