[Rollup] Validate timezones based on rules not string comparision

docs/reference/rollup/apis/get-job.asciidoc

@@ -66,7 +66,7 @@ Which will yield the following response:
                 "interval" : "1h",
                 "delay": "7d",
                 "field": "timestamp",
-                "time_zone": "UTC"
+                "time_zone": "Z"
               },
               "terms" : {
                 "fields" : [
@@ -192,7 +192,7 @@ Which will yield the following response:
                 "interval" : "1h",
                 "delay": "7d",
                 "field": "timestamp",
-                "time_zone": "UTC"
+                "time_zone": "Z"
               },
               "terms" : {
                 "fields" : [
@@ -247,7 +247,7 @@ Which will yield the following response:
                 "interval" : "1h",
                 "delay": "7d",
                 "field": "timestamp",
-                "time_zone": "UTC"
+                "time_zone": "Z"
               },
               "terms" : {
                 "fields" : [

docs/reference/rollup/apis/rollup-caps.asciidoc

@@ -121,7 +121,7 @@ Which will yield the following response:
           "timestamp" : [
             {
               "agg" : "date_histogram",
-              "time_zone" : "UTC",
+              "time_zone" : "Z",
               "interval" : "1h",
               "delay": "7d"
             }

docs/reference/rollup/apis/rollup-index-caps.asciidoc

@@ -117,7 +117,7 @@ This  will yield the following response:
           "timestamp" : [
             {
               "agg" : "date_histogram",
-              "time_zone" : "UTC",
+              "time_zone" : "Z",
               "interval" : "1h",
               "delay": "7d"
             }

docs/reference/rollup/apis/rollup-job-config.asciidoc

@@ -152,7 +152,7 @@ The `date_histogram` group has several parameters:
 
 `time_zone`::
   Defines what time_zone the rollup documents are stored as.  Unlike raw data, which can shift timezones on the fly, rolled documents have
-  to be stored with a specific timezone.  By default, rollup documents are stored in `UTC`, but this can be changed with the `time_zone`
+  to be stored with a specific timezone.  By default, rollup documents are stored in `UTC` (`"Z"`), but this can be changed with the `time_zone`
   parameter.
 
 .Calendar vs Fixed time intervals

docs/reference/rollup/rollup-search-limitations.asciidoc

@@ -131,5 +131,5 @@ thrown.  We expect the list of support queries to grow over time as more are imp
 === Timezones
 
 Rollup documents are stored in the timezone of the `date_histogram` group configuration in the job.  If no timezone is specified, the default
-is to rollup timestamps in `UTC`.
+is to rollup timestamps in `UTC` (`"Z"`).
 

server/src/main/java/org/elasticsearch/common/time/DateUtils.java

@@ -57,6 +57,127 @@ public static DateTimeZone zoneIdToDateTimeZone(ZoneId zoneId) {
         DEPRECATED_SHORT_TZ_IDS = tzs.keySet();
     }
 
+    // Map of deprecated timezones and their recommended new counterpart
+    public static final Map<String, String> DEPRECATED_LONG_TIMEZONES;
+    static {
+        Map<String, String> tzs = new HashMap<>();
+        tzs.put("Africa/Asmera","Africa/Nairobi");
+        tzs.put("Africa/Timbuktu","Africa/Abidjan");
+        tzs.put("America/Argentina/ComodRivadavia","America/Argentina/Catamarca");
+        tzs.put("America/Atka","America/Adak");
+        tzs.put("America/Buenos_Aires","America/Argentina/Buenos_Aires");
+        tzs.put("America/Catamarca","America/Argentina/Catamarca");
+        tzs.put("America/Coral_Harbour","America/Atikokan");
+        tzs.put("America/Cordoba","America/Argentina/Cordoba");
+        tzs.put("America/Ensenada","America/Tijuana");
+        tzs.put("America/Fort_Wayne","America/Indiana/Indianapolis");
+        tzs.put("America/Indianapolis","America/Indiana/Indianapolis");
+        tzs.put("America/Jujuy","America/Argentina/Jujuy");
+        tzs.put("America/Knox_IN","America/Indiana/Knox");
+        tzs.put("America/Louisville","America/Kentucky/Louisville");
+        tzs.put("America/Mendoza","America/Argentina/Mendoza");
+        tzs.put("America/Montreal","America/Toronto");
+        tzs.put("America/Porto_Acre","America/Rio_Branco");
+        tzs.put("America/Rosario","America/Argentina/Cordoba");
+        tzs.put("America/Santa_Isabel","America/Tijuana");
+        tzs.put("America/Shiprock","America/Denver");
+        tzs.put("America/Virgin","America/Port_of_Spain");
+        tzs.put("Antarctica/South_Pole","Pacific/Auckland");
+        tzs.put("Asia/Ashkhabad","Asia/Ashgabat");
+        tzs.put("Asia/Calcutta","Asia/Kolkata");
+        tzs.put("Asia/Chongqing","Asia/Shanghai");
+        tzs.put("Asia/Chungking","Asia/Shanghai");
+        tzs.put("Asia/Dacca","Asia/Dhaka");
+        tzs.put("Asia/Harbin","Asia/Shanghai");
+        tzs.put("Asia/Kashgar","Asia/Urumqi");
+        tzs.put("Asia/Katmandu","Asia/Kathmandu");
+        tzs.put("Asia/Macao","Asia/Macau");
+        tzs.put("Asia/Rangoon","Asia/Yangon");
+        tzs.put("Asia/Saigon","Asia/Ho_Chi_Minh");
+        tzs.put("Asia/Tel_Aviv","Asia/Jerusalem");
+        tzs.put("Asia/Thimbu","Asia/Thimphu");
+        tzs.put("Asia/Ujung_Pandang","Asia/Makassar");
+        tzs.put("Asia/Ulan_Bator","Asia/Ulaanbaatar");
+        tzs.put("Atlantic/Faeroe","Atlantic/Faroe");
+        tzs.put("Atlantic/Jan_Mayen","Europe/Oslo");
+        tzs.put("Australia/ACT","Australia/Sydney");
+        tzs.put("Australia/Canberra","Australia/Sydney");
+        tzs.put("Australia/LHI","Australia/Lord_Howe");
+        tzs.put("Australia/NSW","Australia/Sydney");
+        tzs.put("Australia/North","Australia/Darwin");
+        tzs.put("Australia/Queensland","Australia/Brisbane");
+        tzs.put("Australia/South","Australia/Adelaide");
+        tzs.put("Australia/Tasmania","Australia/Hobart");
+        tzs.put("Australia/Victoria","Australia/Melbourne");
+        tzs.put("Australia/West","Australia/Perth");
+        tzs.put("Australia/Yancowinna","Australia/Broken_Hill");
+        tzs.put("Brazil/Acre","America/Rio_Branco");
+        tzs.put("Brazil/DeNoronha","America/Noronha");
+        tzs.put("Brazil/East","America/Sao_Paulo");
+        tzs.put("Brazil/West","America/Manaus");
+        tzs.put("Canada/Atlantic","America/Halifax");
+        tzs.put("Canada/Central","America/Winnipeg");
+        tzs.put("Canada/East-Saskatchewan","America/Regina");
+        tzs.put("Canada/Eastern","America/Toronto");
+        tzs.put("Canada/Mountain","America/Edmonton");
+        tzs.put("Canada/Newfoundland","America/St_Johns");
+        tzs.put("Canada/Pacific","America/Vancouver");
+        tzs.put("Canada/Yukon","America/Whitehorse");
+        tzs.put("Chile/Continental","America/Santiago");
+        tzs.put("Chile/EasterIsland","Pacific/Easter");
+        tzs.put("Cuba","America/Havana");
+        tzs.put("Egypt","Africa/Cairo");
+        tzs.put("Eire","Europe/Dublin");
+        tzs.put("Europe/Belfast","Europe/London");
+        tzs.put("Europe/Tiraspol","Europe/Chisinau");
+        tzs.put("GB","Europe/London");
+        tzs.put("GB-Eire","Europe/London");
+        tzs.put("Greenwich","Etc/GMT");
+        tzs.put("Hongkong","Asia/Hong_Kong");
+        tzs.put("Iceland","Atlantic/Reykjavik");
+        tzs.put("Iran","Asia/Tehran");
+        tzs.put("Israel","Asia/Jerusalem");
+        tzs.put("Jamaica","America/Jamaica");
+        tzs.put("Japan","Asia/Tokyo");
+        tzs.put("Kwajalein","Pacific/Kwajalein");
+        tzs.put("Libya","Africa/Tripoli");
+        tzs.put("Mexico/BajaNorte","America/Tijuana");
+        tzs.put("Mexico/BajaSur","America/Mazatlan");
+        tzs.put("Mexico/General","America/Mexico_City");
+        tzs.put("NZ","Pacific/Auckland");
+        tzs.put("NZ-CHAT","Pacific/Chatham");
+        tzs.put("Navajo","America/Denver");
+        tzs.put("PRC","Asia/Shanghai");
+        tzs.put("Pacific/Johnston","Pacific/Honolulu");
+        tzs.put("Pacific/Ponape","Pacific/Pohnpei");
+        tzs.put("Pacific/Samoa","Pacific/Pago_Pago");
+        tzs.put("Pacific/Truk","Pacific/Chuuk");
+        tzs.put("Pacific/Yap","Pacific/Chuuk");
+        tzs.put("Poland","Europe/Warsaw");
+        tzs.put("Portugal","Europe/Lisbon");
+        tzs.put("ROC","Asia/Taipei");
+        tzs.put("ROK","Asia/Seoul");
+        tzs.put("Singapore","Asia/Singapore");
+        tzs.put("Turkey","Europe/Istanbul");
+        tzs.put("UCT","Etc/UCT");
+        tzs.put("US/Alaska","America/Anchorage");
+        tzs.put("US/Aleutian","America/Adak");
+        tzs.put("US/Arizona","America/Phoenix");
+        tzs.put("US/Central","America/Chicago");
+        tzs.put("US/East-Indiana","America/Indiana/Indianapolis");
+        tzs.put("US/Eastern","America/New_York");
+        tzs.put("US/Hawaii","Pacific/Honolulu");
+        tzs.put("US/Indiana-Starke","America/Indiana/Knox");
+        tzs.put("US/Michigan","America/Detroit");
+        tzs.put("US/Mountain","America/Denver");
+        tzs.put("US/Pacific","America/Los_Angeles");
+        tzs.put("US/Samoa","Pacific/Pago_Pago");
+        tzs.put("Universal","Etc/UTC");
+        tzs.put("W-SU","Europe/Moscow");
+        tzs.put("Zulu","Etc/UTC");
+        DEPRECATED_LONG_TIMEZONES = Collections.unmodifiableMap(tzs);
+    }
+
     public static ZoneId dateTimeZoneToZoneId(DateTimeZone timeZone) {
         if (timeZone == null) {
             return null;

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfig.java

@@ -5,6 +5,7 @@
  */
 package org.elasticsearch.xpack.core.rollup.job;
 
+import org.elasticsearch.Version;
 import org.elasticsearch.action.ActionRequestValidationException;
 import org.elasticsearch.action.fieldcaps.FieldCapabilities;
 import org.elasticsearch.common.Nullable;
@@ -15,6 +16,7 @@
 import org.elasticsearch.common.io.stream.Writeable;
 import org.elasticsearch.common.rounding.DateTimeUnit;
 import org.elasticsearch.common.rounding.Rounding;
+import org.elasticsearch.common.time.DateUtils;
 import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.common.xcontent.ConstructingObjectParser;
 import org.elasticsearch.common.xcontent.ToXContentObject;
@@ -25,6 +27,8 @@
 import org.joda.time.DateTimeZone;
 
 import java.io.IOException;
+import java.time.ZoneId;
+import java.time.ZoneOffset;
 import java.util.Map;
 import java.util.Objects;
 
@@ -53,7 +57,7 @@ public class DateHistogramGroupConfig implements Writeable, ToXContentObject {
     private static final String FIELD = "field";
     public static final String TIME_ZONE = "time_zone";
     public static final String DELAY = "delay";
-    private static final String DEFAULT_TIMEZONE = "UTC";
+    public static final ZoneId DEFAULT_TIMEZONE = ZoneOffset.UTC;
     private static final ConstructingObjectParser<DateHistogramGroupConfig, Void> PARSER;
     static {
         PARSER = new ConstructingObjectParser<>(NAME, a ->
@@ -67,7 +71,7 @@ public class DateHistogramGroupConfig implements Writeable, ToXContentObject {
     private final String field;
     private final DateHistogramInterval interval;
     private final DateHistogramInterval delay;
-    private final String timeZone;
+    private final ZoneId timeZone;
 
     /**
      * Create a new {@link DateHistogramGroupConfig} using the given field and interval parameters.
@@ -103,10 +107,9 @@ public DateHistogramGroupConfig(final String field,
         this.interval = interval;
         this.field = field;
         this.delay = delay;
-        this.timeZone = (timeZone != null && timeZone.isEmpty() == false) ? timeZone : DEFAULT_TIMEZONE;
-
-        // validate interval
-        createRounding(this.interval.toString(), this.timeZone);
+        // Convert to ZoneId, then validate roundings (which unfortunately requires converting to DateTimeZone)
+        this.timeZone = Strings.isNullOrEmpty(timeZone) ? DEFAULT_TIMEZONE : ZoneId.of(timeZone, ZoneId.SHORT_IDS);
+        createRounding(this.interval.toString(), DateUtils.zoneIdToDateTimeZone(this.timeZone));
         if (delay != null) {
             // and delay
             TimeValue.parseTimeValue(this.delay.toString(), DELAY);
@@ -117,15 +120,23 @@ public DateHistogramGroupConfig(final String field,
         interval = new DateHistogramInterval(in);
         field = in.readString();
         delay = in.readOptionalWriteable(DateHistogramInterval::new);
-        timeZone = in.readString();
+        // new ZoneId can deal with older DateTimeStrings, no need for version check
+        timeZone = ZoneId.of(in.readString(), ZoneId.SHORT_IDS);
     }
 
     @Override
     public void writeTo(final StreamOutput out) throws IOException {
         interval.writeTo(out);
         out.writeString(field);
         out.writeOptionalWriteable(delay);
-        out.writeString(timeZone);
+        // TODO change version after backport
+        if (out.getVersion().onOrAfter(Version.V_7_0_0)) {
+            out.writeString(timeZone.toString());
+        } else {
+            // If we're on an older version, DateTime doesn't know how to
+            // deal with new ZoneId strings (E.g. "Z" instead of "UTC")
+            out.writeString(DateUtils.zoneIdToDateTimeZone(timeZone).toString());
+        }
     }
 
     @Override
@@ -137,7 +148,7 @@ public XContentBuilder toXContent(final XContentBuilder builder, final Params pa
             if (delay != null) {
                 builder.field(DELAY, delay.toString());
             }
-            builder.field(TIME_ZONE, timeZone);
+            builder.field(TIME_ZONE, timeZone.toString());
         }
         return builder.endObject();
     }
@@ -167,14 +178,14 @@ public DateHistogramInterval getDelay() {
      * Get the timezone to apply
      */
     public String getTimeZone() {
-        return timeZone;
+        return timeZone.toString();
     }
 
     /**
      * Create the rounding for this date histogram
      */
     public Rounding createRounding() {
-        return createRounding(interval.toString(), timeZone);
+        return createRounding(interval.toString(), DateUtils.zoneIdToDateTimeZone(timeZone));
     }
 
     public void validateMappings(Map<String, Map<String, FieldCapabilities>> fieldCapsResponse,
@@ -211,12 +222,12 @@ public boolean equals(final Object other) {
         return Objects.equals(interval, that.interval)
             && Objects.equals(field, that.field)
             && Objects.equals(delay, that.delay)
-            && Objects.equals(timeZone, that.timeZone);
+            && timeZone.getRules().equals(((DateHistogramGroupConfig) other).timeZone.getRules());
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(interval, field, delay, timeZone);
+        return Objects.hash(interval, field, delay, timeZone.getRules());
     }
 
     @Override
@@ -228,24 +239,15 @@ public static DateHistogramGroupConfig fromXContent(final XContentParser parser)
         return PARSER.parse(parser, null);
     }
 
-    private static Rounding createRounding(final String expr, final String timeZone) {
+    private static Rounding createRounding(final String expr, final DateTimeZone timeZone) {
         DateTimeUnit timeUnit = DateHistogramAggregationBuilder.DATE_FIELD_UNITS.get(expr);
         final Rounding.Builder rounding;
         if (timeUnit != null) {
             rounding = new Rounding.Builder(timeUnit);
         } else {
             rounding = new Rounding.Builder(TimeValue.parseTimeValue(expr, "createRounding"));
         }
-        rounding.timeZone(toDateTimeZone(timeZone));
+        rounding.timeZone(timeZone);
         return rounding.build();
     }
-
-    private static DateTimeZone toDateTimeZone(final String timezone) {
-        try {
-            return DateTimeZone.forOffsetHours(Integer.parseInt(timezone));
-        } catch (NumberFormatException e) {
-            return DateTimeZone.forID(timezone);
-        }
-    }
-
 }

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/ConfigTestHelpers.java

@@ -71,7 +71,13 @@ public static DateHistogramGroupConfig randomDateHistogramGroupConfig(final Rand
         final String field = randomField(random);
         final DateHistogramInterval interval = randomInterval();
         final DateHistogramInterval delay = random.nextBoolean() ? randomInterval() : null;
-        final String timezone = random.nextBoolean() ? randomDateTimeZone().toString() : null;
+        // TODO extend this to full set of ZoneId's once we move away from DateTime
+        // can't do it now because some ZoneIds are not compatible with DateTime
+        final String timezone = random.nextBoolean()
+            ? (random.nextBoolean()
+                ? randomDateTimeZone().toString()
+                : "Z")
+            : null;
         return new DateHistogramGroupConfig(field, interval, delay, timezone);
     }
 

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfigSerializingTests.java

@@ -5,6 +5,7 @@
  */
 package org.elasticsearch.xpack.core.rollup.job;
 
+import org.elasticsearch.Version;
 import org.elasticsearch.action.ActionRequestValidationException;
 import org.elasticsearch.action.fieldcaps.FieldCapabilities;
 import org.elasticsearch.common.io.stream.BytesStreamOutput;
@@ -148,6 +149,7 @@ public void testBwcSerialization() throws IOException {
             final DateHistogramGroupConfig reference = ConfigTestHelpers.randomDateHistogramGroupConfig(random());
 
             final BytesStreamOutput out = new BytesStreamOutput();
+            out.setVersion(Version.V_6_4_0);
             reference.writeTo(out);
 
             // previous way to deserialize a DateHistogramGroupConfig

x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/RollupDataExtractorFactory.java

@@ -22,6 +22,8 @@
 import org.elasticsearch.xpack.core.rollup.job.DateHistogramGroupConfig;
 import org.elasticsearch.xpack.ml.datafeed.extractor.DataExtractorFactory;
 
+import java.time.ZoneId;
+import java.time.ZoneOffset;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -120,7 +122,7 @@ private static boolean validInterval(long datafeedInterval, ParsedRollupCaps rol
         if (rollupJobGroupConfig.hasDatehistogram() == false) {
             return false;
         }
-        if ("UTC".equalsIgnoreCase(rollupJobGroupConfig.getTimezone()) == false) {
+        if (ZoneId.of(rollupJobGroupConfig.getTimezone()).getRules().equals(ZoneOffset.UTC.getRules()) == false) {
             return false;
         }
         try {