Core: Rework multi date formatter merging (#36447)

This commit moves the MergedDateFormatter to a package private class and
reworks joda DateFormatter instances to use that instead of a single
DateTimeFormatter with multiple parsers. This will allow the java and
joda multi formats to share the same format parsing method in a
followup.

server/src/main/java/org/elasticsearch/common/joda/Joda.java

@@ -48,10 +48,6 @@
 
 public class Joda {
 
-    public static JodaDateFormatter forPattern(String input) {
-        return forPattern(input, Locale.ROOT);
-    }
-
     /**
      * Parses a joda based pattern, including some named ones (similar to the built in Joda ISO ones).
      */
@@ -231,27 +227,7 @@ public static JodaDateFormatter forPattern(String input, Locale locale) {
             formatter = StrictISODateTimeFormat.yearMonth();
         } else if ("strictYearMonthDay".equals(input) || "strict_year_month_day".equals(input)) {
             formatter = StrictISODateTimeFormat.yearMonthDay();
-        } else if (Strings.hasLength(input) && input.contains("||")) {
-                String[] formats = Strings.delimitedListToStringArray(input, "||");
-                DateTimeParser[] parsers = new DateTimeParser[formats.length];
-
-                if (formats.length == 1) {
-                    formatter = forPattern(input, locale).parser;
-                } else {
-                    DateTimeFormatter dateTimeFormatter = null;
-                    for (int i = 0; i < formats.length; i++) {
-                        JodaDateFormatter currentFormatter = forPattern(formats[i], locale);
-                        DateTimeFormatter currentParser = currentFormatter.parser;
-                        if (dateTimeFormatter == null) {
-                            dateTimeFormatter = currentFormatter.printer;
-                        }
-                        parsers[i] = currentParser.getParser();
-                    }
-
-                    DateTimeFormatterBuilder builder = new DateTimeFormatterBuilder()
-                        .append(dateTimeFormatter.withZone(DateTimeZone.UTC).getPrinter(), parsers);
-                    formatter = builder.toFormatter();
-                }
+
         } else {
             try {
                 formatter = DateTimeFormat.forPattern(input);

server/src/main/java/org/elasticsearch/common/joda/JodaDateFormatter.java

@@ -47,7 +47,7 @@ public JodaDateFormatter(String pattern, DateTimeFormatter parser, DateTimeForma
 
     @Override
     public TemporalAccessor parse(String input) {
-        DateTime dt = parser.parseDateTime(input);
+        final DateTime dt = parser.parseDateTime(input);
         return ZonedDateTime.ofInstant(Instant.ofEpochMilli(dt.getMillis()), DateUtils.dateTimeZoneToZoneId(dt.getZone()));
     }
 

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

@@ -19,17 +19,19 @@
 
 package org.elasticsearch.common.time;
 
-import org.elasticsearch.ElasticsearchParseException;
+import org.elasticsearch.common.Strings;
+import org.elasticsearch.common.joda.Joda;
 import org.joda.time.DateTime;
 
 import java.time.Instant;
 import java.time.ZoneId;
+import java.time.ZoneOffset;
 import java.time.ZonedDateTime;
 import java.time.format.DateTimeParseException;
 import java.time.temporal.TemporalAccessor;
-import java.util.Arrays;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Locale;
-import java.util.stream.Collectors;
 
 public interface DateFormatter {
 
@@ -85,7 +87,7 @@ default DateTime parseJoda(String input) {
      * Return the given millis-since-epoch formatted with this format.
      */
     default String formatMillis(long millis) {
-        return format(Instant.ofEpochMilli(millis));
+        return format(ZonedDateTime.ofInstant(Instant.ofEpochMilli(millis), ZoneOffset.UTC));
     }
 
     /**
@@ -123,94 +125,23 @@ default String formatJoda(DateTime dateTime) {
      */
     DateMathParser toDateMathParser();
 
-    /**
-     * Merge several date formatters into a single one. Useful if you need to have several formatters with
-     * different formats act as one, for example when you specify a
-     * format like <code>date_hour||epoch_millis</code>
-     *
-     * @param formatters The list of date formatters to be merged together
-     * @return           The new date formtter containing the specified date formatters
-     */
-    static DateFormatter merge(DateFormatter... formatters) {
-        return new MergedDateFormatter(formatters);
+    static DateFormatter forPattern(String input) {
+        return forPattern(input, Locale.ROOT);
     }
 
-    class MergedDateFormatter implements DateFormatter {
-
-        private final String format;
-        private final DateFormatter[] formatters;
-        private final DateMathParser[] dateMathParsers;
-
-        MergedDateFormatter(DateFormatter... formatters) {
-            this.formatters = formatters;
-            this.format = Arrays.stream(formatters).map(DateFormatter::pattern).collect(Collectors.joining("||"));
-            this.dateMathParsers = Arrays.stream(formatters).map(DateFormatter::toDateMathParser).toArray(DateMathParser[]::new);
+    static DateFormatter forPattern(String input, Locale locale) {
+        if (Strings.hasLength(input) == false) {
+            throw new IllegalArgumentException("No date pattern provided");
         }
-
-        @Override
-        public TemporalAccessor parse(String input) {
-            DateTimeParseException failure = null;
-            for (DateFormatter formatter : formatters) {
-                try {
-                    return formatter.parse(input);
-                } catch (DateTimeParseException e) {
-                    if (failure == null) {
-                        failure = e;
-                    } else {
-                        failure.addSuppressed(e);
-                    }
-                }
-            }
-            throw failure;
-        }
-
-        @Override
-        public DateFormatter withZone(ZoneId zoneId) {
-            return new MergedDateFormatter(Arrays.stream(formatters).map(f -> f.withZone(zoneId)).toArray(DateFormatter[]::new));
+        List<DateFormatter> formatters = new ArrayList<>();
+        for (String pattern : Strings.delimitedListToStringArray(input, "||")) {
+            formatters.add(Joda.forPattern(pattern, locale));
         }
 
-        @Override
-        public DateFormatter withLocale(Locale locale) {
-            return new MergedDateFormatter(Arrays.stream(formatters).map(f -> f.withLocale(locale)).toArray(DateFormatter[]::new));
+        if (formatters.size() == 1) {
+            return formatters.get(0);
         }
+        return new DateFormatters.MergedDateFormatter(input, formatters);
 
-        @Override
-        public String format(TemporalAccessor accessor) {
-            return formatters[0].format(accessor);
-        }
-
-        @Override
-        public String pattern() {
-            return format;
-        }
-
-        @Override
-        public Locale locale() {
-            return formatters[0].locale();
-        }
-
-        @Override
-        public ZoneId zone() {
-            return formatters[0].zone();
-        }
-
-        @Override
-        public DateMathParser toDateMathParser() {
-            return (text, now, roundUp, tz) -> {
-                ElasticsearchParseException failure = null;
-                for (DateMathParser parser : dateMathParsers) {
-                    try {
-                        return parser.parse(text, now, roundUp, tz);
-                    } catch (ElasticsearchParseException e) {
-                        if (failure == null) {
-                            failure = e;
-                        } else {
-                            failure.addSuppressed(e);
-                        }
-                    }
-                }
-                throw failure;
-            };
-        }
     }
 }

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

@@ -19,24 +19,31 @@
 
 package org.elasticsearch.common.time;
 
+import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.common.Strings;
 
 import java.time.DateTimeException;
 import java.time.DayOfWeek;
 import java.time.Instant;
 import java.time.LocalDate;
+import java.time.ZoneId;
 import java.time.ZoneOffset;
 import java.time.ZonedDateTime;
 import java.time.format.DateTimeFormatter;
 import java.time.format.DateTimeFormatterBuilder;
+import java.time.format.DateTimeParseException;
 import java.time.format.ResolverStyle;
 import java.time.format.SignStyle;
 import java.time.temporal.ChronoField;
 import java.time.temporal.IsoFields;
 import java.time.temporal.TemporalAccessor;
 import java.time.temporal.TemporalAdjusters;
 import java.time.temporal.WeekFields;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
 import java.util.Locale;
+import java.util.stream.Collectors;
 
 import static java.time.temporal.ChronoField.DAY_OF_MONTH;
 import static java.time.temporal.ChronoField.DAY_OF_WEEK;
@@ -1442,12 +1449,12 @@ private static DateFormatter forPattern(String input, Locale locale) {
                 return forPattern(formats[0], locale);
             } else {
                 try {
-                    DateFormatter[] formatters = new DateFormatter[formats.length];
+                    List<DateFormatter> formatters = new ArrayList<>(formats.length);
                     for (int i = 0; i < formats.length; i++) {
-                        formatters[i] = forPattern(formats[i], locale);
+                        formatters.add(forPattern(formats[i], locale));
                     }
 
-                    return DateFormatter.merge(formatters);
+                    return new MergedDateFormatter(input, formatters);
                 } catch (IllegalArgumentException e) {
                     throw new IllegalArgumentException("Invalid format: [" + input + "]: " + e.getMessage(), e);
                 }
@@ -1461,6 +1468,91 @@ private static DateFormatter forPattern(String input, Locale locale) {
         }
     }
 
+    static class MergedDateFormatter implements DateFormatter {
+
+        private final String pattern;
+        private final List<DateFormatter> formatters;
+        private final List<DateMathParser> dateMathParsers;
+
+        MergedDateFormatter(String pattern, List<DateFormatter> formatters) {
+            assert formatters.size() > 0;
+            this.pattern = pattern;
+            this.formatters = Collections.unmodifiableList(formatters);
+            this.dateMathParsers = formatters.stream().map(DateFormatter::toDateMathParser).collect(Collectors.toList());
+        }
+
+        @Override
+        public TemporalAccessor parse(String input) {
+            IllegalArgumentException failure = null;
+            for (DateFormatter formatter : formatters) {
+                try {
+                    return formatter.parse(input);
+                    // TODO: remove DateTimeParseException when JavaDateFormatter throws IAE
+                } catch (IllegalArgumentException | DateTimeParseException e) {
+                    if (failure == null) {
+                        // wrap so the entire multi format is in the message
+                        failure = new IllegalArgumentException("failed to parse date field [" + input + "] with format [" + pattern + "]",
+                            e);
+                    } else {
+                        failure.addSuppressed(e);
+                    }
+                }
+            }
+            throw failure;
+        }
+
+        @Override
+        public DateFormatter withZone(ZoneId zoneId) {
+            return new MergedDateFormatter(pattern, formatters.stream().map(f -> f.withZone(zoneId)).collect(Collectors.toList()));
+        }
+
+        @Override
+        public DateFormatter withLocale(Locale locale) {
+            return new MergedDateFormatter(pattern, formatters.stream().map(f -> f.withLocale(locale)).collect(Collectors.toList()));
+        }
+
+        @Override
+        public String format(TemporalAccessor accessor) {
+            return formatters.get(0).format(accessor);
+        }
+
+        @Override
+        public String pattern() {
+            return pattern;
+        }
+
+        @Override
+        public Locale locale() {
+            return formatters.get(0).locale();
+        }
+
+        @Override
+        public ZoneId zone() {
+            return formatters.get(0).zone();
+        }
+
+        @Override
+        public DateMathParser toDateMathParser() {
+            return (text, now, roundUp, tz) -> {
+                ElasticsearchParseException failure = null;
+                for (DateMathParser parser : dateMathParsers) {
+                    try {
+                        return parser.parse(text, now, roundUp, tz);
+                    } catch (ElasticsearchParseException e) {
+                        if (failure == null) {
+                            // wrap so the entire multi format is in the message
+                            failure = new ElasticsearchParseException("failed to parse date field [" + text + "] with format ["
+                                + pattern + "]", e);
+                        } else {
+                            failure.addSuppressed(e);
+                        }
+                    }
+                }
+                throw failure;
+            };
+        }
+    }
+
     private static final ZonedDateTime EPOCH_ZONED_DATE_TIME = Instant.EPOCH.atZone(ZoneOffset.UTC);
 
     public static ZonedDateTime toZonedDateTime(TemporalAccessor accessor) {

server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java

@@ -36,7 +36,6 @@
 import org.elasticsearch.common.Explicit;
 import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.geo.ShapeRelation;
-import org.elasticsearch.common.joda.Joda;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.time.DateFormatter;
 import org.elasticsearch.common.time.DateMathParser;
@@ -65,7 +64,7 @@
 public class DateFieldMapper extends FieldMapper {
 
     public static final String CONTENT_TYPE = "date";
-    public static final DateFormatter DEFAULT_DATE_TIME_FORMATTER = Joda.forPattern(
+    public static final DateFormatter DEFAULT_DATE_TIME_FORMATTER = DateFormatter.forPattern(
             "strict_date_optional_time||epoch_millis", Locale.ROOT);
 
     public static class Defaults {
@@ -381,7 +380,7 @@ public Object valueForDisplay(Object value) {
         public DocValueFormat docValueFormat(@Nullable String format, DateTimeZone timeZone) {
             DateFormatter dateTimeFormatter = this.dateTimeFormatter;
             if (format != null) {
-                dateTimeFormatter = Joda.forPattern(format);
+                dateTimeFormatter = DateFormatter.forPattern(format);
             }
             if (timeZone == null) {
                 timeZone = DateTimeZone.UTC;

server/src/main/java/org/elasticsearch/index/mapper/TypeParsers.java

@@ -21,7 +21,6 @@
 
 import org.apache.lucene.index.IndexOptions;
 import org.elasticsearch.ElasticsearchParseException;
-import org.elasticsearch.common.joda.Joda;
 import org.elasticsearch.common.time.DateFormatter;
 import org.elasticsearch.common.xcontent.support.XContentMapValues;
 import org.elasticsearch.index.analysis.NamedAnalyzer;
@@ -265,7 +264,7 @@ private static IndexOptions nodeIndexOptionValue(final Object propNode) {
 
     public static DateFormatter parseDateTimeFormatter(Object node) {
         if (node instanceof String) {
-            return Joda.forPattern((String) node);
+            return DateFormatter.forPattern((String) node);
         }
         throw new IllegalArgumentException("Invalid format: [" + node.toString() + "]: expected string value");
     }

server/src/main/java/org/elasticsearch/index/query/RangeQueryBuilder.java

@@ -29,7 +29,6 @@
 import org.elasticsearch.common.geo.ShapeRelation;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
-import org.elasticsearch.common.joda.Joda;
 import org.elasticsearch.common.lucene.BytesRefs;
 import org.elasticsearch.common.time.DateFormatter;
 import org.elasticsearch.common.time.DateMathParser;
@@ -105,7 +104,7 @@ public RangeQueryBuilder(StreamInput in) throws IOException {
         timeZone = in.readOptionalTimeZone();
         String formatString = in.readOptionalString();
         if (formatString != null) {
-            format = Joda.forPattern(formatString);
+            format = DateFormatter.forPattern(formatString);
         }
         String relationString = in.readOptionalString();
         if (relationString != null) {
@@ -290,7 +289,7 @@ public RangeQueryBuilder format(String format) {
         if (format == null) {
             throw new IllegalArgumentException("format cannot be null");
         }
-        this.format = Joda.forPattern(format);
+        this.format = DateFormatter.forPattern(format);
         return this;
     }