Skip to content

Commit

Permalink
Fix java time formatters that round up (elastic#37604)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@spinscale
spinscale committed Jan 22, 2019
1 parent e82e6e5 commit 44134a2
Show file tree
Hide file tree
Showing 6 changed files with 124 additions and 39 deletions.
6 changes: 4 additions & 2 deletions server/src/main/java/org/elasticsearch/common/time/DateFormatter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,11 @@ static DateFormatter forPattern(String input) {
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);
}
Expand Down
44 changes: 23 additions & 21 deletions server/src/main/java/org/elasticsearch/common/time/DateFormatters.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@
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;
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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);

/*
Expand Down Expand Up @@ -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()
Expand All @@ -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)
);
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);

/*
Expand Down Expand Up @@ -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)
);

Expand Down Expand Up @@ -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);

Expand All @@ -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()
Expand All @@ -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);

/*
Expand Down Expand Up @@ -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()
Expand All @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -1453,18 +1452,21 @@ static JavaDateFormatter merge(String pattern, List<DateFormatter> formatters) {
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);
Expand Down
2 changes: 2 additions & 0 deletions server/src/main/java/org/elasticsearch/common/time/EpochTime.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,11 @@ public long getFrom(TemporalAccessor temporal) {
.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 {
Expand Down
41 changes: 31 additions & 10 deletions server/src/main/java/org/elasticsearch/common/time/JavaDateFormatter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.function.Consumer;

class JavaDateFormatter implements DateFormatter {

Expand All @@ -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");
}
Expand All @@ -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() {
Expand All @@ -100,7 +127,7 @@ public DateFormatter withZone(ZoneId zoneId) {
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
Expand All @@ -110,7 +137,7 @@ public DateFormatter withLocale(Locale locale) {
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
Expand All @@ -123,12 +150,6 @@ public String pattern() {
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();
Expand All @@ -141,7 +162,7 @@ public ZoneId zone() {

@Override
public DateMathParser toDateMathParser() {
return new JavaDateMathParser(this, this.parseDefaulting(ROUND_UP_BASE_FIELDS));
return new JavaDateMathParser(parser, roundupParser);
}

@Override
Expand Down
16 changes: 10 additions & 6 deletions server/src/main/java/org/elasticsearch/common/time/JavaDateMathParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -28,6 +29,7 @@
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;
Expand All @@ -44,12 +46,10 @@
*/
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;
Expand Down Expand Up @@ -208,7 +208,11 @@ private long parseMath(final String mathString, final long time, final boolean r
}

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();
Expand Down
Loading

0 comments on commit 44134a2

Please sign in to comment.