Fix java time formatters that round up (#37604)

In order to be able to parse epoch seconds and epoch milli seconds own
java time fields had been introduced. These fields are however not
compatible with the way that java time allows one to configure default
fields (when a part of a timestamp cannot be read then a default value
is added), which is used for the formatters that are rounding up to the
next value.

This commit allows java date formatters to configure its round up parsing 
by setting default values via a consumer. By default all formats are setting 
JavaDateFormatter.ROUND_UP_BASE_FIELDS for rounding up. The epoch
however parsers both need to set different fields. The merged date
formatters do not set any fields, they just append all the round up formatters.

Also the formatter now properly copies the locale and the timezone, 
fractional parsing has been set to nano seconds with proper width.

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

@@ -133,9 +133,11 @@ public interface DateFormatter {
             return Joda.forPattern(input);
         }
 
-        // force java 8 date format
+        // dates starting with 8 will not be using joda but java time formatters
+        input = input.substring(1);
+
         List<DateFormatter> formatters = new ArrayList<>();
-        for (String pattern : Strings.delimitedListToStringArray(input.substring(1), "||")) {
+        for (String pattern : Strings.delimitedListToStringArray(input, "||")) {
             if (Strings.hasLength(pattern) == false) {
                 throw new IllegalArgumentException("Cannot have empty element in multi date format pattern: " + input);
             }

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

@@ -46,7 +46,6 @@ import static java.time.temporal.ChronoField.DAY_OF_MONTH;
 import static java.time.temporal.ChronoField.DAY_OF_WEEK;
 import static java.time.temporal.ChronoField.DAY_OF_YEAR;
 import static java.time.temporal.ChronoField.HOUR_OF_DAY;
-import static java.time.temporal.ChronoField.MILLI_OF_SECOND;
 import static java.time.temporal.ChronoField.MINUTE_OF_HOUR;
 import static java.time.temporal.ChronoField.MONTH_OF_YEAR;
 import static java.time.temporal.ChronoField.NANO_OF_SECOND;
@@ -90,7 +89,7 @@ public class DateFormatters {
         .appendLiteral('T')
         .append(STRICT_HOUR_MINUTE_SECOND_FORMATTER)
         .optionalStart()
-        .appendFraction(MILLI_OF_SECOND, 3, 3, true)
+        .appendFraction(NANO_OF_SECOND, 3, 3, true)
         .optionalEnd()
         .optionalStart()
         .appendZoneOrOffsetId()
@@ -159,14 +158,14 @@ public class DateFormatters {
         .appendValue(HOUR_OF_DAY, 2, 2, SignStyle.NOT_NEGATIVE)
         .appendValue(MINUTE_OF_HOUR, 2, 2, SignStyle.NOT_NEGATIVE)
         .appendValue(SECOND_OF_MINUTE, 2, 2, SignStyle.NOT_NEGATIVE)
-        .appendFraction(MILLI_OF_SECOND, 1, 3, true)
+        .appendFraction(NANO_OF_SECOND, 1, 3, true)
         .toFormatter(Locale.ROOT);
 
     private static final DateTimeFormatter BASIC_TIME_PRINTER = new DateTimeFormatterBuilder()
         .appendValue(HOUR_OF_DAY, 2, 2, SignStyle.NOT_NEGATIVE)
         .appendValue(MINUTE_OF_HOUR, 2, 2, SignStyle.NOT_NEGATIVE)
         .appendValue(SECOND_OF_MINUTE, 2, 2, SignStyle.NOT_NEGATIVE)
-        .appendFraction(MILLI_OF_SECOND, 3, 3, true)
+        .appendFraction(NANO_OF_SECOND, 3, 3, true)
         .toFormatter(Locale.ROOT);
 
     /*
@@ -372,7 +371,7 @@ public class DateFormatters {
             .appendValue(HOUR_OF_DAY, 2, 2, SignStyle.NOT_NEGATIVE)
             .appendValue(MINUTE_OF_HOUR, 2, 2, SignStyle.NOT_NEGATIVE)
             .appendValue(SECOND_OF_MINUTE, 2, 2, SignStyle.NOT_NEGATIVE)
-            .appendFraction(MILLI_OF_SECOND, 3, 3, true)
+            .appendFraction(NANO_OF_SECOND, 3, 3, true)
             .appendZoneOrOffsetId()
             .toFormatter(Locale.ROOT),
         new DateTimeFormatterBuilder()
@@ -381,7 +380,7 @@ public class DateFormatters {
             .appendValue(HOUR_OF_DAY, 2, 2, SignStyle.NOT_NEGATIVE)
             .appendValue(MINUTE_OF_HOUR, 2, 2, SignStyle.NOT_NEGATIVE)
             .appendValue(SECOND_OF_MINUTE, 2, 2, SignStyle.NOT_NEGATIVE)
-            .appendFraction(MILLI_OF_SECOND, 3, 3, true)
+            .appendFraction(NANO_OF_SECOND, 3, 3, true)
             .append(TIME_ZONE_FORMATTER_NO_COLON)
             .toFormatter(Locale.ROOT)
     );
@@ -438,7 +437,7 @@ public class DateFormatters {
         .appendLiteral('T')
         .append(STRICT_HOUR_MINUTE_SECOND_FORMATTER)
         .optionalStart()
-        .appendFraction(MILLI_OF_SECOND, 3, 3, true)
+        .appendFraction(NANO_OF_SECOND, 3, 3, true)
         .optionalEnd()
         .toFormatter(Locale.ROOT);
 
@@ -495,12 +494,12 @@ public class DateFormatters {
     // NOTE: this is not a strict formatter to retain the joda time based behaviour, even though it's named like this
     private static final DateTimeFormatter STRICT_HOUR_MINUTE_SECOND_MILLIS_FORMATTER = new DateTimeFormatterBuilder()
         .append(STRICT_HOUR_MINUTE_SECOND_FORMATTER)
-        .appendFraction(MILLI_OF_SECOND, 1, 3, true)
+        .appendFraction(NANO_OF_SECOND, 1, 3, true)
         .toFormatter(Locale.ROOT);
 
     private static final DateTimeFormatter STRICT_HOUR_MINUTE_SECOND_MILLIS_PRINTER = new DateTimeFormatterBuilder()
         .append(STRICT_HOUR_MINUTE_SECOND_FORMATTER)
-        .appendFraction(MILLI_OF_SECOND, 3, 3, true)
+        .appendFraction(NANO_OF_SECOND, 3, 3, true)
         .toFormatter(Locale.ROOT);
 
     /*
@@ -534,7 +533,7 @@ public class DateFormatters {
             .appendLiteral("T")
             .append(STRICT_HOUR_MINUTE_SECOND_FORMATTER)
             //  this one here is lenient as well to retain joda time based bwc compatibility
-            .appendFraction(MILLI_OF_SECOND, 1, 3, true)
+            .appendFraction(NANO_OF_SECOND, 1, 3, true)
             .toFormatter(Locale.ROOT)
     );
 
@@ -562,7 +561,7 @@ public class DateFormatters {
         .optionalStart()
         .appendLiteral(':')
         .appendValue(SECOND_OF_MINUTE, 2, 2, SignStyle.NOT_NEGATIVE)
-        .appendFraction(MILLI_OF_SECOND, 3, 3, true)
+        .appendFraction(NANO_OF_SECOND, 3, 3, true)
         .optionalEnd()
         .toFormatter(Locale.ROOT);
 
@@ -586,7 +585,7 @@ public class DateFormatters {
         .appendValue(MINUTE_OF_HOUR, 2, 2, SignStyle.NOT_NEGATIVE)
         .appendLiteral(':')
         .appendValue(SECOND_OF_MINUTE, 2, 2, SignStyle.NOT_NEGATIVE)
-        .appendFraction(MILLI_OF_SECOND, 1, 3, true)
+        .appendFraction(NANO_OF_SECOND, 1, 3, true)
         .toFormatter(Locale.ROOT);
 
     private static final DateTimeFormatter STRICT_TIME_PRINTER = new DateTimeFormatterBuilder()
@@ -595,7 +594,7 @@ public class DateFormatters {
         .appendValue(MINUTE_OF_HOUR, 2, 2, SignStyle.NOT_NEGATIVE)
         .appendLiteral(':')
         .appendValue(SECOND_OF_MINUTE, 2, 2, SignStyle.NOT_NEGATIVE)
-        .appendFraction(MILLI_OF_SECOND, 3, 3, true)
+        .appendFraction(NANO_OF_SECOND, 3, 3, true)
         .toFormatter(Locale.ROOT);
 
     /*
@@ -819,7 +818,7 @@ public class DateFormatters {
             .appendValue(SECOND_OF_MINUTE, 1, 2, SignStyle.NOT_NEGATIVE)
             .optionalEnd()
             .optionalStart()
-            .appendFraction(MILLI_OF_SECOND, 1, 3, true)
+            .appendFraction(NANO_OF_SECOND, 1, 3, true)
             .optionalEnd()
             .optionalStart().appendZoneOrOffsetId().optionalEnd()
             .optionalStart().appendOffset("+HHmm", "Z").optionalEnd()
@@ -840,7 +839,7 @@ public class DateFormatters {
         .appendValue(MINUTE_OF_HOUR, 1, 2, SignStyle.NOT_NEGATIVE)
         .appendLiteral(':')
         .appendValue(SECOND_OF_MINUTE, 1, 2, SignStyle.NOT_NEGATIVE)
-        .appendFraction(MILLI_OF_SECOND, 1, 3, true)
+        .appendFraction(NANO_OF_SECOND, 1, 3, true)
         .toFormatter(Locale.ROOT);
 
     private static final DateTimeFormatter ORDINAL_DATE_FORMATTER = new DateTimeFormatterBuilder()
@@ -875,7 +874,7 @@ public class DateFormatters {
 
     private static final DateTimeFormatter TIME_PREFIX = new DateTimeFormatterBuilder()
         .append(TIME_NO_MILLIS_FORMATTER)
-        .appendFraction(MILLI_OF_SECOND, 1, 3, true)
+        .appendFraction(NANO_OF_SECOND, 1, 3, true)
         .toFormatter(Locale.ROOT);
 
     private static final DateTimeFormatter WEEK_DATE_FORMATTER = new DateTimeFormatterBuilder()
@@ -963,7 +962,7 @@ public class DateFormatters {
         .optionalStart()
         .appendLiteral(':')
         .appendValue(SECOND_OF_MINUTE, 1, 2, SignStyle.NOT_NEGATIVE)
-        .appendFraction(MILLI_OF_SECOND, 1, 3, true)
+        .appendFraction(NANO_OF_SECOND, 1, 3, true)
         .optionalEnd()
         .toFormatter(Locale.ROOT);
 
@@ -1068,7 +1067,7 @@ public class DateFormatters {
         .optionalStart()
         .appendLiteral(':')
         .appendValue(SECOND_OF_MINUTE, 1, 2, SignStyle.NOT_NEGATIVE)
-        .appendFraction(MILLI_OF_SECOND, 1, 3, true)
+        .appendFraction(NANO_OF_SECOND, 1, 3, true)
         .optionalEnd()
         .toFormatter(Locale.ROOT);
 
@@ -1453,18 +1452,21 @@ public class DateFormatters {
         assert formatters.size() > 0;
 
         List<DateTimeFormatter> dateTimeFormatters = new ArrayList<>(formatters.size());
+        DateTimeFormatterBuilder roundupBuilder = new DateTimeFormatterBuilder();
         DateTimeFormatter printer = null;
         for (DateFormatter formatter : formatters) {
             assert formatter instanceof JavaDateFormatter;
             JavaDateFormatter javaDateFormatter = (JavaDateFormatter) formatter;
-            DateTimeFormatter dateTimeFormatter = javaDateFormatter.getParser();
             if (printer == null) {
                 printer = javaDateFormatter.getPrinter();
             }
-            dateTimeFormatters.add(dateTimeFormatter);
+            dateTimeFormatters.add(javaDateFormatter.getParser());
+            roundupBuilder.appendOptional(javaDateFormatter.getRoundupParser());
         }
+        DateTimeFormatter roundUpParser = roundupBuilder.toFormatter(Locale.ROOT);
 
-        return new JavaDateFormatter(pattern, printer, dateTimeFormatters.toArray(new DateTimeFormatter[0]));
+        return new JavaDateFormatter(pattern, printer, builder -> builder.append(roundUpParser),
+            dateTimeFormatters.toArray(new DateTimeFormatter[0]));
     }
 
     private static final ZonedDateTime EPOCH_ZONED_DATE_TIME = Instant.EPOCH.atZone(ZoneOffset.UTC);

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

@@ -153,9 +153,11 @@ class EpochTime {
         .toFormatter(Locale.ROOT);
 
     static final DateFormatter SECONDS_FORMATTER = new JavaDateFormatter("epoch_second", SECONDS_FORMATTER3,
+        builder -> builder.parseDefaulting(ChronoField.NANO_OF_SECOND, 999_999_999L),
         SECONDS_FORMATTER1, SECONDS_FORMATTER2, SECONDS_FORMATTER3);
 
     static final DateFormatter MILLIS_FORMATTER = new JavaDateFormatter("epoch_millis", MILLISECONDS_FORMATTER3,
+        builder -> builder.parseDefaulting(EpochTime.NANOS_OF_MILLI, 999_999L),
         MILLISECONDS_FORMATTER1, MILLISECONDS_FORMATTER2, MILLISECONDS_FORMATTER3);
 
     private abstract static class EpochField implements TemporalField {

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

@@ -32,6 +32,7 @@ import java.util.HashMap;
 import java.util.Locale;
 import java.util.Map;
 import java.util.Objects;
+import java.util.function.Consumer;
 
 class JavaDateFormatter implements DateFormatter {
 
@@ -43,14 +44,27 @@ class JavaDateFormatter implements DateFormatter {
         ROUND_UP_BASE_FIELDS.put(ChronoField.HOUR_OF_DAY, 23L);
         ROUND_UP_BASE_FIELDS.put(ChronoField.MINUTE_OF_HOUR, 59L);
         ROUND_UP_BASE_FIELDS.put(ChronoField.SECOND_OF_MINUTE, 59L);
-        ROUND_UP_BASE_FIELDS.put(ChronoField.MILLI_OF_SECOND, 999L);
+        ROUND_UP_BASE_FIELDS.put(ChronoField.NANO_OF_SECOND, 999_999_999L);
     }
 
     private final String format;
     private final DateTimeFormatter printer;
     private final DateTimeFormatter parser;
+    private final DateTimeFormatter roundupParser;
+
+    private JavaDateFormatter(String format, DateTimeFormatter printer, DateTimeFormatter roundupParser, DateTimeFormatter parser) {
+        this.format = format;
+        this.printer = printer;
+        this.roundupParser = roundupParser;
+        this.parser = parser;
+    }
 
     JavaDateFormatter(String format, DateTimeFormatter printer, DateTimeFormatter... parsers) {
+        this(format, printer, builder -> ROUND_UP_BASE_FIELDS.forEach(builder::parseDefaulting), parsers);
+    }
+
+    JavaDateFormatter(String format, DateTimeFormatter printer, Consumer<DateTimeFormatterBuilder> roundupParserConsumer,
+                              DateTimeFormatter... parsers) {
         if (printer == null) {
             throw new IllegalArgumentException("printer may not be null");
         }
@@ -75,6 +89,19 @@ class JavaDateFormatter implements DateFormatter {
         }
         this.format = format;
         this.printer = printer;
+
+        DateTimeFormatterBuilder builder = new DateTimeFormatterBuilder();
+        builder.append(this.parser);
+        roundupParserConsumer.accept(builder);
+        DateTimeFormatter roundupFormatter = builder.toFormatter(parser.getLocale());
+        if (printer.getZone() != null) {
+            roundupFormatter = roundupFormatter.withZone(printer.getZone());
+        }
+        this.roundupParser = roundupFormatter;
+    }
+
+    DateTimeFormatter getRoundupParser() {
+        return roundupParser;
     }
 
     DateTimeFormatter getParser() {
@@ -100,7 +127,7 @@ class JavaDateFormatter implements DateFormatter {
             return this;
         }
 
-        return new JavaDateFormatter(format, printer.withZone(zoneId), parser.withZone(zoneId));
+        return new JavaDateFormatter(format, printer.withZone(zoneId), roundupParser.withZone(zoneId), parser.withZone(zoneId));
     }
 
     @Override
@@ -110,7 +137,7 @@ class JavaDateFormatter implements DateFormatter {
             return this;
         }
 
-        return new JavaDateFormatter(format, printer.withLocale(locale), parser.withLocale(locale));
+        return new JavaDateFormatter(format, printer.withLocale(locale), roundupParser.withLocale(locale), parser.withLocale(locale));
     }
 
     @Override
@@ -123,12 +150,6 @@ class JavaDateFormatter implements DateFormatter {
         return format;
     }
 
-    JavaDateFormatter parseDefaulting(Map<TemporalField, Long> fields) {
-        final DateTimeFormatterBuilder parseDefaultingBuilder = new DateTimeFormatterBuilder().append(printer);
-        fields.forEach(parseDefaultingBuilder::parseDefaulting);
-        return new JavaDateFormatter(format, parseDefaultingBuilder.toFormatter(Locale.ROOT));
-    }
-
     @Override
     public Locale locale() {
         return this.printer.getLocale();
@@ -141,7 +162,7 @@ class JavaDateFormatter implements DateFormatter {
 
     @Override
     public DateMathParser toDateMathParser() {
-        return new JavaDateMathParser(this, this.parseDefaulting(ROUND_UP_BASE_FIELDS));
+        return new JavaDateMathParser(parser, roundupParser);
     }
 
     @Override

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

@@ -20,6 +20,7 @@
 package org.elasticsearch.common.time;
 
 import org.elasticsearch.ElasticsearchParseException;
+import org.elasticsearch.common.Strings;
 
 import java.time.DateTimeException;
 import java.time.DayOfWeek;
@@ -28,6 +29,7 @@ import java.time.LocalTime;
 import java.time.ZoneId;
 import java.time.ZoneOffset;
 import java.time.ZonedDateTime;
+import java.time.format.DateTimeFormatter;
 import java.time.temporal.ChronoField;
 import java.time.temporal.TemporalAccessor;
 import java.time.temporal.TemporalAdjusters;
@@ -44,12 +46,10 @@ import java.util.function.LongSupplier;
  */
 public class JavaDateMathParser implements DateMathParser {
 
+    private final DateTimeFormatter formatter;
+    private final DateTimeFormatter roundUpFormatter;
 
-
-    private final DateFormatter formatter;
-    private final DateFormatter roundUpFormatter;
-
-    public JavaDateMathParser(DateFormatter formatter, DateFormatter roundUpFormatter) {
+    public JavaDateMathParser(DateTimeFormatter formatter, DateTimeFormatter roundUpFormatter) {
         Objects.requireNonNull(formatter);
         this.formatter = formatter;
         this.roundUpFormatter = roundUpFormatter;
@@ -208,7 +208,11 @@ public class JavaDateMathParser implements DateMathParser {
     }
 
     private long parseDateTime(String value, ZoneId timeZone, boolean roundUpIfNoTime) {
-        DateFormatter formatter = roundUpIfNoTime ? this.roundUpFormatter : this.formatter;
+        if (Strings.isNullOrEmpty(value)) {
+            throw new IllegalArgumentException("cannot parse empty date");
+        }
+
+        DateTimeFormatter formatter = roundUpIfNoTime ? this.roundUpFormatter : this.formatter;
         try {
             if (timeZone == null) {
                 return DateFormatters.toZonedDateTime(formatter.parse(value)).toInstant().toEpochMilli();

server/src/test/java/org/elasticsearch/common/time/DateFormattersTests.java

@@ -24,7 +24,9 @@ import org.elasticsearch.test.ESTestCase;
 import java.time.Instant;
 import java.time.ZoneId;
 import java.time.ZonedDateTime;
+import java.time.format.DateTimeFormatter;
 import java.time.format.DateTimeParseException;
+import java.time.temporal.ChronoField;
 import java.time.temporal.TemporalAccessor;
 import java.util.Locale;
 
@@ -157,4 +159,56 @@ public class DateFormattersTests extends ESTestCase {
         formatter.format(formatter.parse("2018-05-15T17:14:56.123456789+0100"));
         formatter.format(formatter.parse("2018-05-15T17:14:56.123456789+01:00"));
     }
+
+    public void testRoundupFormatterWithEpochDates() {
+        assertRoundupFormatter("8epoch_millis", "1234567890", 1234567890L);
+        // also check nanos of the epoch_millis formatter if it is rounded up to the nano second
+        DateTimeFormatter roundUpFormatter = ((JavaDateFormatter) DateFormatter.forPattern("8epoch_millis")).getRoundupParser();
+        Instant epochMilliInstant = DateFormatters.toZonedDateTime(roundUpFormatter.parse("1234567890")).toInstant();
+        assertThat(epochMilliInstant.getLong(ChronoField.NANO_OF_SECOND), is(890_999_999L));
+
+        assertRoundupFormatter("8strict_date_optional_time||epoch_millis", "2018-10-10T12:13:14.123Z", 1539173594123L);
+        assertRoundupFormatter("8strict_date_optional_time||epoch_millis", "1234567890", 1234567890L);
+        assertRoundupFormatter("8uuuu-MM-dd'T'HH:mm:ss.SSS||epoch_millis", "2018-10-10T12:13:14.123", 1539173594123L);
+        assertRoundupFormatter("8uuuu-MM-dd'T'HH:mm:ss.SSS||epoch_millis", "1234567890", 1234567890L);
+
+        assertRoundupFormatter("8epoch_second", "1234567890", 1234567890999L);
+        // also check nanos of the epoch_millis formatter if it is rounded up to the nano second
+        DateTimeFormatter epochSecondRoundupParser = ((JavaDateFormatter) DateFormatter.forPattern("8epoch_second")).getRoundupParser();
+        Instant epochSecondInstant = DateFormatters.toZonedDateTime(epochSecondRoundupParser.parse("1234567890")).toInstant();
+        assertThat(epochSecondInstant.getLong(ChronoField.NANO_OF_SECOND), is(999_999_999L));
+
+        assertRoundupFormatter("8strict_date_optional_time||epoch_second", "2018-10-10T12:13:14.123Z", 1539173594123L);
+        assertRoundupFormatter("8strict_date_optional_time||epoch_second", "1234567890", 1234567890999L);
+        assertRoundupFormatter("8uuuu-MM-dd'T'HH:mm:ss.SSS||epoch_second", "2018-10-10T12:13:14.123", 1539173594123L);
+        assertRoundupFormatter("8uuuu-MM-dd'T'HH:mm:ss.SSS||epoch_second", "1234567890", 1234567890999L);
+    }
+
+    private void assertRoundupFormatter(String format, String input, long expectedMilliSeconds) {
+        JavaDateFormatter dateFormatter = (JavaDateFormatter) DateFormatter.forPattern(format);
+        dateFormatter.parse(input);
+        DateTimeFormatter roundUpFormatter = dateFormatter.getRoundupParser();
+        long millis = DateFormatters.toZonedDateTime(roundUpFormatter.parse(input)).toInstant().toEpochMilli();
+        assertThat(millis, is(expectedMilliSeconds));
+    }
+
+    public void testRoundupFormatterZone() {
+        ZoneId zoneId = randomZone();
+        String format = randomFrom("8epoch_second", "8epoch_millis", "8strict_date_optional_time", "8uuuu-MM-dd'T'HH:mm:ss.SSS",
+            "8strict_date_optional_time||date_optional_time");
+        JavaDateFormatter formatter = (JavaDateFormatter) DateFormatter.forPattern(format).withZone(zoneId);
+        DateTimeFormatter roundUpFormatter = formatter.getRoundupParser();
+        assertThat(roundUpFormatter.getZone(), is(zoneId));
+        assertThat(formatter.zone(), is(zoneId));
+    }
+
+    public void testRoundupFormatterLocale() {
+        Locale locale = randomLocale(random());
+        String format = randomFrom("8epoch_second", "8epoch_millis", "8strict_date_optional_time", "8uuuu-MM-dd'T'HH:mm:ss.SSS",
+            "8strict_date_optional_time||date_optional_time");
+        JavaDateFormatter formatter = (JavaDateFormatter) DateFormatter.forPattern(format).withLocale(locale);
+        DateTimeFormatter roundupParser = formatter.getRoundupParser();
+        assertThat(roundupParser.getLocale(), is(locale));
+        assertThat(formatter.locale(), is(locale));
+    }
 }