Merge pull request #12581 from cbuescher/fix/12531

Fix setting timezone on default DateTime formatter

core/src/main/java/org/elasticsearch/common/rounding/TimeZoneRounding.java

@@ -64,6 +64,9 @@ public abstract class TimeZoneRounding extends Rounding {
         }
 
         public Builder timeZone(DateTimeZone timeZone) {
+            if (timeZone == null) {
+                throw new IllegalArgumentException("Setting null as timezone is not supported");
+            }
             this.timeZone = timeZone;
             return this;
         }

core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramParser.java

@@ -33,10 +33,7 @@ import org.elasticsearch.search.aggregations.AggregatorFactory;
 import org.elasticsearch.search.aggregations.support.ValueType;
 import org.elasticsearch.search.aggregations.support.ValuesSourceConfig;
 import org.elasticsearch.search.aggregations.support.ValuesSourceParser;
-import org.elasticsearch.search.aggregations.support.format.ValueFormatter.DateTime;
 import org.elasticsearch.search.internal.SearchContext;
-import org.joda.time.DateTimeZone;
-
 import java.io.IOException;
 
 /**
@@ -45,7 +42,6 @@ import java.io.IOException;
 public class DateHistogramParser implements Aggregator.Parser {
 
     static final ParseField EXTENDED_BOUNDS = new ParseField("extended_bounds");
-    static final ParseField TIME_ZONE = new ParseField("time_zone");
     static final ParseField OFFSET = new ParseField("offset");
     static final ParseField INTERVAL = new ParseField("interval");
 
@@ -83,6 +79,7 @@ public class DateHistogramParser implements Aggregator.Parser {
         ValuesSourceParser vsParser = ValuesSourceParser.numeric(aggregationName, InternalDateHistogram.TYPE, context)
                 .targetValueType(ValueType.DATE)
                 .formattable(true)
+                .timezoneAware(true)
                 .build();
 
         boolean keyed = false;
@@ -90,7 +87,6 @@ public class DateHistogramParser implements Aggregator.Parser {
         ExtendedBounds extendedBounds = null;
         InternalOrder order = (InternalOrder) Histogram.Order.KEY_ASC;
         String interval = null;
-        DateTimeZone timeZone = DateTimeZone.UTC;
         long offset = 0;
 
         XContentParser.Token token;
@@ -101,9 +97,7 @@ public class DateHistogramParser implements Aggregator.Parser {
             } else if (vsParser.token(currentFieldName, token, parser)) {
                 continue;
             } else if (token == XContentParser.Token.VALUE_STRING) {
-                if (context.parseFieldMatcher().match(currentFieldName, TIME_ZONE)) {
+                if (context.parseFieldMatcher().match(currentFieldName, OFFSET)) {
-                    timeZone = DateTimeZone.forID(parser.text());
-                } else if (context.parseFieldMatcher().match(currentFieldName, OFFSET)) {
                     offset = parseOffset(parser.text());
                 } else if (context.parseFieldMatcher().match(currentFieldName, INTERVAL)) {
                     interval = parser.text();
@@ -121,8 +115,6 @@ public class DateHistogramParser implements Aggregator.Parser {
             } else if (token == XContentParser.Token.VALUE_NUMBER) {
                 if ("min_doc_count".equals(currentFieldName) || "minDocCount".equals(currentFieldName)) {
                     minDocCount = parser.longValue();
-                } else if ("time_zone".equals(currentFieldName) || "timeZone".equals(currentFieldName)) {
-                    timeZone = DateTimeZone.forOffsetHours(parser.intValue());
                 } else {
                     throw new SearchParseException(context, "Unknown key for a " + token + " in [" + aggregationName + "]: ["
                             + currentFieldName + "].", parser.getTokenLocation());
@@ -193,13 +185,10 @@ public class DateHistogramParser implements Aggregator.Parser {
         }
 
         Rounding rounding = tzRoundingBuilder
-                .timeZone(timeZone)
+                .timeZone(vsParser.input().timezone())
                 .offset(offset).build();
 
         ValuesSourceConfig config = vsParser.config();
-        if (config.formatter()!=null) {
-            ((DateTime) config.formatter()).setTimeZone(timeZone);
-        }
         return new HistogramAggregator.Factory(aggregationName, config, rounding, order, keyed, minDocCount, extendedBounds,
                 new InternalDateHistogram.Factory());
 

core/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceParser.java

@@ -20,6 +20,7 @@
 package org.elasticsearch.search.aggregations.support;
 
 import org.elasticsearch.common.Nullable;
+import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.index.fielddata.IndexFieldData;
 import org.elasticsearch.index.fielddata.IndexGeoPointFieldData;
@@ -39,6 +40,7 @@ import org.elasticsearch.search.SearchParseException;
 import org.elasticsearch.search.aggregations.InternalAggregation;
 import org.elasticsearch.search.aggregations.support.format.ValueFormat;
 import org.elasticsearch.search.internal.SearchContext;
+import org.joda.time.DateTimeZone;
 
 import java.io.IOException;
 import java.util.Map;
@@ -50,6 +52,8 @@ import static com.google.common.collect.Maps.newHashMap;
  */
 public class ValuesSourceParser<VS extends ValuesSource> {
 
+    static final ParseField TIME_ZONE = new ParseField("time_zone");
+
     public static Builder any(String aggName, InternalAggregation.Type aggType, SearchContext context) {
         return new Builder<>(aggName, aggType, context, ValuesSource.class);
     }
@@ -66,14 +70,19 @@ public class ValuesSourceParser<VS extends ValuesSource> {
         return new Builder<>(aggName, aggType, context, ValuesSource.GeoPoint.class).targetValueType(ValueType.GEOPOINT).scriptable(false);
     }
 
-    private static class Input {
+    public static class Input {
-        String field = null;
+        private String field = null;
-        Script script = null;
+        private Script script = null;
         @Deprecated
-        Map<String, Object> params = null; // TODO Remove in 3.0
+        private Map<String, Object> params = null; // TODO Remove in 3.0
-        ValueType valueType = null;
+        private ValueType valueType = null;
-        String format = null;
+        private String format = null;
-        Object missing = null;
+        private Object missing = null;
+        private DateTimeZone timezone = DateTimeZone.UTC;
+
+        public DateTimeZone timezone() {
+            return this.timezone;
+        }
     }
 
     private final String aggName;
@@ -83,6 +92,7 @@ public class ValuesSourceParser<VS extends ValuesSource> {
 
     private boolean scriptable = true;
     private boolean formattable = false;
+    private boolean timezoneAware = false;
     private ValueType targetValueType = null;
     private ScriptParameterParser scriptParameterParser = new ScriptParameterParser();
 
@@ -105,6 +115,8 @@ public class ValuesSourceParser<VS extends ValuesSource> {
                 input.field = parser.text();
             } else if (formattable && "format".equals(currentFieldName)) {
                 input.format = parser.text();
+            } else if (timezoneAware && context.parseFieldMatcher().match(currentFieldName, TIME_ZONE)) {
+                input.timezone = DateTimeZone.forID(parser.text());
             } else if (scriptable) {
                 if ("value_type".equals(currentFieldName) || "valueType".equals(currentFieldName)) {
                     input.valueType = ValueType.resolveForScript(parser.text());
@@ -123,6 +135,14 @@ public class ValuesSourceParser<VS extends ValuesSource> {
             }
             return true;
         }
+        if (token == XContentParser.Token.VALUE_NUMBER) {
+            if (timezoneAware && context.parseFieldMatcher().match(currentFieldName, TIME_ZONE)) {
+                input.timezone = DateTimeZone.forOffsetHours(parser.intValue());
+            } else {
+                return false;
+            }
+            return true;
+        }
         if (scriptable && token == XContentParser.Token.START_OBJECT) {
             if (context.parseFieldMatcher().match(currentFieldName, ScriptField.SCRIPT)) {
                 input.script = Script.parse(parser, context.parseFieldMatcher());
@@ -203,7 +223,7 @@ public class ValuesSourceParser<VS extends ValuesSource> {
         config.fieldContext = new FieldContext(input.field, indexFieldData, fieldType);
         config.missing = input.missing;
         config.script = createScript();
-        config.format = resolveFormat(input.format, fieldType);
+        config.format = resolveFormat(input.format, input.timezone, fieldType);
         return config;
     }
 
@@ -222,9 +242,9 @@ public class ValuesSourceParser<VS extends ValuesSource> {
         return valueFormat;
     }
 
-    private static ValueFormat resolveFormat(@Nullable String format, MappedFieldType fieldType) {
+    private static ValueFormat resolveFormat(@Nullable String format, @Nullable DateTimeZone timezone,  MappedFieldType fieldType) {
         if (fieldType instanceof  DateFieldMapper.DateFieldType) {
-            return format != null ? ValueFormat.DateTime.format(format) : ValueFormat.DateTime.mapper((DateFieldMapper.DateFieldType) fieldType);
+            return format != null ? ValueFormat.DateTime.format(format, timezone) : ValueFormat.DateTime.mapper((DateFieldMapper.DateFieldType) fieldType, timezone);
         }
         if (fieldType instanceof IpFieldMapper.IpFieldType) {
             return ValueFormat.IPv4;
@@ -238,6 +258,10 @@ public class ValuesSourceParser<VS extends ValuesSource> {
         return ValueFormat.RAW;
     }
 
+    public Input input() {
+        return this.input;
+    }
+
     public static class Builder<VS extends ValuesSource> {
 
         private final ValuesSourceParser<VS> parser;
@@ -256,6 +280,11 @@ public class ValuesSourceParser<VS extends ValuesSource> {
             return this;
         }
 
+        public Builder<VS> timezoneAware(boolean timezoneAware) {
+            parser.timezoneAware = timezoneAware;
+            return this;
+        }
+
         public Builder<VS> targetValueType(ValueType valueType) {
             parser.targetValueType = valueType;
             return this;

core/src/main/java/org/elasticsearch/search/aggregations/support/format/ValueFormat.java

@@ -20,6 +20,7 @@
 package org.elasticsearch.search.aggregations.support.format;
 
 import org.elasticsearch.index.mapper.core.DateFieldMapper;
+import org.joda.time.DateTimeZone;
 
 /**
  *
@@ -67,12 +68,12 @@ public class ValueFormat {
 
         public static final DateTime DEFAULT = new DateTime(DateFieldMapper.Defaults.DATE_TIME_FORMATTER.format(), ValueFormatter.DateTime.DEFAULT, ValueParser.DateMath.DEFAULT);
 
-        public static DateTime format(String format) {
+        public static DateTime format(String format, DateTimeZone timezone) {
-            return new DateTime(format, new ValueFormatter.DateTime(format), new ValueParser.DateMath(format));
+            return new DateTime(format, new ValueFormatter.DateTime(format, timezone), new ValueParser.DateMath(format));
         }
 
-        public static DateTime mapper(DateFieldMapper.DateFieldType fieldType) {
+        public static DateTime mapper(DateFieldMapper.DateFieldType fieldType, DateTimeZone timezone) {
-            return new DateTime(fieldType.dateTimeFormatter().format(), ValueFormatter.DateTime.mapper(fieldType), ValueParser.DateMath.mapper(fieldType));
+            return new DateTime(fieldType.dateTimeFormatter().format(), ValueFormatter.DateTime.mapper(fieldType, timezone), ValueParser.DateMath.mapper(fieldType));
         }
 
         public DateTime(String pattern, ValueFormatter formatter, ValueParser parser) {
@@ -81,7 +82,7 @@ public class ValueFormat {
 
         @Override
         public DateTime create(String pattern) {
-            return format(pattern);
+            return format(pattern, DateTimeZone.UTC);
         }
     }
 

core/src/main/java/org/elasticsearch/search/aggregations/support/format/ValueFormatter.java

@@ -33,7 +33,6 @@ import java.text.DecimalFormat;
 import java.text.DecimalFormatSymbols;
 import java.text.NumberFormat;
 import java.util.Locale;
-import java.util.TimeZone;
 
 /**
  * A strategy for formatting time represented as millis long value to string
@@ -61,7 +60,6 @@ public interface ValueFormatter extends Streamable {
     String format(long value);
 
     /**
-     * The 
      * @param value double The double value to format.
      * @return      The formatted value as string
      */
@@ -104,8 +102,8 @@ public interface ValueFormatter extends Streamable {
         public static final ValueFormatter DEFAULT = new ValueFormatter.DateTime(DateFieldMapper.Defaults.DATE_TIME_FORMATTER);
         private DateTimeZone timeZone = DateTimeZone.UTC;
 
-        public static DateTime mapper(DateFieldMapper.DateFieldType fieldType) {
+        public static DateTime mapper(DateFieldMapper.DateFieldType fieldType, DateTimeZone timezone) {
-            return new DateTime(fieldType.dateTimeFormatter());
+            return new DateTime(fieldType.dateTimeFormatter(), timezone);
         }
 
         static final byte ID = 2;
@@ -122,15 +120,21 @@ public interface ValueFormatter extends Streamable {
             this.formatter = formatter;
         }
 
+        public DateTime(String format, DateTimeZone timezone) {
+            this.formatter = Joda.forPattern(format);
+            this.timeZone = timezone != null ? timezone : DateTimeZone.UTC;
+        }
+
+        public DateTime(FormatDateTimeFormatter formatter, DateTimeZone timezone) {
+            this.formatter = formatter;
+            this.timeZone = timezone != null ? timezone : DateTimeZone.UTC;
+        }
+
         @Override
         public String format(long time) {
             return formatter.printer().withZone(timeZone).print(time);
         }
 
-        public void setTimeZone(DateTimeZone timeZone) {
-            this.timeZone = timeZone;
-        }
-
         @Override
         public String format(double value) {
             return format((long) value);
@@ -264,7 +268,7 @@ public interface ValueFormatter extends Streamable {
 
         }
     }
-    
+
     static class BooleanFormatter implements ValueFormatter {
 
         static final byte ID = 10;