Skip to content

Commit 4fb68ea

Browse files
spinscalespinscale
authored
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.
1 parent 3e1e1b0 commit 4fb68ea

File tree

6 files changed

+124
-39
lines changed

6 files changed

+124
-39
lines changed

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,9 +133,11 @@ static DateFormatter forPattern(String input) {
133133
return Joda.forPattern(input);
134134
}
135135

136-
// force java 8 date format
136+
// dates starting with 8 will not be using joda but java time formatters
137+
input = input.substring(1);
138+
137139
List<DateFormatter> formatters = new ArrayList<>();
138-
for (String pattern : Strings.delimitedListToStringArray(input.substring(1), "||")) {
140+
for (String pattern : Strings.delimitedListToStringArray(input, "||")) {
139141
if (Strings.hasLength(pattern) == false) {
140142
throw new IllegalArgumentException("Cannot have empty element in multi date format pattern: " + input);
141143
}

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

Lines changed: 23 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@
4646
import static java.time.temporal.ChronoField.DAY_OF_WEEK;
4747
import static java.time.temporal.ChronoField.DAY_OF_YEAR;
4848
import static java.time.temporal.ChronoField.HOUR_OF_DAY;
49-
import static java.time.temporal.ChronoField.MILLI_OF_SECOND;
5049
import static java.time.temporal.ChronoField.MINUTE_OF_HOUR;
5150
import static java.time.temporal.ChronoField.MONTH_OF_YEAR;
5251
import static java.time.temporal.ChronoField.NANO_OF_SECOND;
@@ -90,7 +89,7 @@ public class DateFormatters {
9089
.appendLiteral('T')
9190
.append(STRICT_HOUR_MINUTE_SECOND_FORMATTER)
9291
.optionalStart()
93-
.appendFraction(MILLI_OF_SECOND, 3, 3, true)
92+
.appendFraction(NANO_OF_SECOND, 3, 3, true)
9493
.optionalEnd()
9594
.optionalStart()
9695
.appendZoneOrOffsetId()
@@ -159,14 +158,14 @@ public class DateFormatters {
159158
.appendValue(HOUR_OF_DAY, 2, 2, SignStyle.NOT_NEGATIVE)
160159
.appendValue(MINUTE_OF_HOUR, 2, 2, SignStyle.NOT_NEGATIVE)
161160
.appendValue(SECOND_OF_MINUTE, 2, 2, SignStyle.NOT_NEGATIVE)
162-
.appendFraction(MILLI_OF_SECOND, 1, 3, true)
161+
.appendFraction(NANO_OF_SECOND, 1, 3, true)
163162
.toFormatter(Locale.ROOT);
164163

165164
private static final DateTimeFormatter BASIC_TIME_PRINTER = new DateTimeFormatterBuilder()
166165
.appendValue(HOUR_OF_DAY, 2, 2, SignStyle.NOT_NEGATIVE)
167166
.appendValue(MINUTE_OF_HOUR, 2, 2, SignStyle.NOT_NEGATIVE)
168167
.appendValue(SECOND_OF_MINUTE, 2, 2, SignStyle.NOT_NEGATIVE)
169-
.appendFraction(MILLI_OF_SECOND, 3, 3, true)
168+
.appendFraction(NANO_OF_SECOND, 3, 3, true)
170169
.toFormatter(Locale.ROOT);
171170

172171
/*
@@ -372,7 +371,7 @@ public class DateFormatters {
372371
.appendValue(HOUR_OF_DAY, 2, 2, SignStyle.NOT_NEGATIVE)
373372
.appendValue(MINUTE_OF_HOUR, 2, 2, SignStyle.NOT_NEGATIVE)
374373
.appendValue(SECOND_OF_MINUTE, 2, 2, SignStyle.NOT_NEGATIVE)
375-
.appendFraction(MILLI_OF_SECOND, 3, 3, true)
374+
.appendFraction(NANO_OF_SECOND, 3, 3, true)
376375
.appendZoneOrOffsetId()
377376
.toFormatter(Locale.ROOT),
378377
new DateTimeFormatterBuilder()
@@ -381,7 +380,7 @@ public class DateFormatters {
381380
.appendValue(HOUR_OF_DAY, 2, 2, SignStyle.NOT_NEGATIVE)
382381
.appendValue(MINUTE_OF_HOUR, 2, 2, SignStyle.NOT_NEGATIVE)
383382
.appendValue(SECOND_OF_MINUTE, 2, 2, SignStyle.NOT_NEGATIVE)
384-
.appendFraction(MILLI_OF_SECOND, 3, 3, true)
383+
.appendFraction(NANO_OF_SECOND, 3, 3, true)
385384
.append(TIME_ZONE_FORMATTER_NO_COLON)
386385
.toFormatter(Locale.ROOT)
387386
);
@@ -438,7 +437,7 @@ public class DateFormatters {
438437
.appendLiteral('T')
439438
.append(STRICT_HOUR_MINUTE_SECOND_FORMATTER)
440439
.optionalStart()
441-
.appendFraction(MILLI_OF_SECOND, 3, 3, true)
440+
.appendFraction(NANO_OF_SECOND, 3, 3, true)
442441
.optionalEnd()
443442
.toFormatter(Locale.ROOT);
444443

@@ -495,12 +494,12 @@ public class DateFormatters {
495494
// NOTE: this is not a strict formatter to retain the joda time based behaviour, even though it's named like this
496495
private static final DateTimeFormatter STRICT_HOUR_MINUTE_SECOND_MILLIS_FORMATTER = new DateTimeFormatterBuilder()
497496
.append(STRICT_HOUR_MINUTE_SECOND_FORMATTER)
498-
.appendFraction(MILLI_OF_SECOND, 1, 3, true)
497+
.appendFraction(NANO_OF_SECOND, 1, 3, true)
499498
.toFormatter(Locale.ROOT);
500499

501500
private static final DateTimeFormatter STRICT_HOUR_MINUTE_SECOND_MILLIS_PRINTER = new DateTimeFormatterBuilder()
502501
.append(STRICT_HOUR_MINUTE_SECOND_FORMATTER)
503-
.appendFraction(MILLI_OF_SECOND, 3, 3, true)
502+
.appendFraction(NANO_OF_SECOND, 3, 3, true)
504503
.toFormatter(Locale.ROOT);
505504

506505
/*
@@ -534,7 +533,7 @@ public class DateFormatters {
534533
.appendLiteral("T")
535534
.append(STRICT_HOUR_MINUTE_SECOND_FORMATTER)
536535
// this one here is lenient as well to retain joda time based bwc compatibility
537-
.appendFraction(MILLI_OF_SECOND, 1, 3, true)
536+
.appendFraction(NANO_OF_SECOND, 1, 3, true)
538537
.toFormatter(Locale.ROOT)
539538
);
540539

@@ -562,7 +561,7 @@ public class DateFormatters {
562561
.optionalStart()
563562
.appendLiteral(':')
564563
.appendValue(SECOND_OF_MINUTE, 2, 2, SignStyle.NOT_NEGATIVE)
565-
.appendFraction(MILLI_OF_SECOND, 3, 3, true)
564+
.appendFraction(NANO_OF_SECOND, 3, 3, true)
566565
.optionalEnd()
567566
.toFormatter(Locale.ROOT);
568567

@@ -586,7 +585,7 @@ public class DateFormatters {
586585
.appendValue(MINUTE_OF_HOUR, 2, 2, SignStyle.NOT_NEGATIVE)
587586
.appendLiteral(':')
588587
.appendValue(SECOND_OF_MINUTE, 2, 2, SignStyle.NOT_NEGATIVE)
589-
.appendFraction(MILLI_OF_SECOND, 1, 3, true)
588+
.appendFraction(NANO_OF_SECOND, 1, 3, true)
590589
.toFormatter(Locale.ROOT);
591590

592591
private static final DateTimeFormatter STRICT_TIME_PRINTER = new DateTimeFormatterBuilder()
@@ -595,7 +594,7 @@ public class DateFormatters {
595594
.appendValue(MINUTE_OF_HOUR, 2, 2, SignStyle.NOT_NEGATIVE)
596595
.appendLiteral(':')
597596
.appendValue(SECOND_OF_MINUTE, 2, 2, SignStyle.NOT_NEGATIVE)
598-
.appendFraction(MILLI_OF_SECOND, 3, 3, true)
597+
.appendFraction(NANO_OF_SECOND, 3, 3, true)
599598
.toFormatter(Locale.ROOT);
600599

601600
/*
@@ -819,7 +818,7 @@ public class DateFormatters {
819818
.appendValue(SECOND_OF_MINUTE, 1, 2, SignStyle.NOT_NEGATIVE)
820819
.optionalEnd()
821820
.optionalStart()
822-
.appendFraction(MILLI_OF_SECOND, 1, 3, true)
821+
.appendFraction(NANO_OF_SECOND, 1, 3, true)
823822
.optionalEnd()
824823
.optionalStart().appendZoneOrOffsetId().optionalEnd()
825824
.optionalStart().appendOffset("+HHmm", "Z").optionalEnd()
@@ -840,7 +839,7 @@ public class DateFormatters {
840839
.appendValue(MINUTE_OF_HOUR, 1, 2, SignStyle.NOT_NEGATIVE)
841840
.appendLiteral(':')
842841
.appendValue(SECOND_OF_MINUTE, 1, 2, SignStyle.NOT_NEGATIVE)
843-
.appendFraction(MILLI_OF_SECOND, 1, 3, true)
842+
.appendFraction(NANO_OF_SECOND, 1, 3, true)
844843
.toFormatter(Locale.ROOT);
845844

846845
private static final DateTimeFormatter ORDINAL_DATE_FORMATTER = new DateTimeFormatterBuilder()
@@ -875,7 +874,7 @@ public class DateFormatters {
875874

876875
private static final DateTimeFormatter TIME_PREFIX = new DateTimeFormatterBuilder()
877876
.append(TIME_NO_MILLIS_FORMATTER)
878-
.appendFraction(MILLI_OF_SECOND, 1, 3, true)
877+
.appendFraction(NANO_OF_SECOND, 1, 3, true)
879878
.toFormatter(Locale.ROOT);
880879

881880
private static final DateTimeFormatter WEEK_DATE_FORMATTER = new DateTimeFormatterBuilder()
@@ -963,7 +962,7 @@ public class DateFormatters {
963962
.optionalStart()
964963
.appendLiteral(':')
965964
.appendValue(SECOND_OF_MINUTE, 1, 2, SignStyle.NOT_NEGATIVE)
966-
.appendFraction(MILLI_OF_SECOND, 1, 3, true)
965+
.appendFraction(NANO_OF_SECOND, 1, 3, true)
967966
.optionalEnd()
968967
.toFormatter(Locale.ROOT);
969968

@@ -1068,7 +1067,7 @@ public class DateFormatters {
10681067
.optionalStart()
10691068
.appendLiteral(':')
10701069
.appendValue(SECOND_OF_MINUTE, 1, 2, SignStyle.NOT_NEGATIVE)
1071-
.appendFraction(MILLI_OF_SECOND, 1, 3, true)
1070+
.appendFraction(NANO_OF_SECOND, 1, 3, true)
10721071
.optionalEnd()
10731072
.toFormatter(Locale.ROOT);
10741073

@@ -1453,18 +1452,21 @@ static JavaDateFormatter merge(String pattern, List<DateFormatter> formatters) {
14531452
assert formatters.size() > 0;
14541453

14551454
List<DateTimeFormatter> dateTimeFormatters = new ArrayList<>(formatters.size());
1455+
DateTimeFormatterBuilder roundupBuilder = new DateTimeFormatterBuilder();
14561456
DateTimeFormatter printer = null;
14571457
for (DateFormatter formatter : formatters) {
14581458
assert formatter instanceof JavaDateFormatter;
14591459
JavaDateFormatter javaDateFormatter = (JavaDateFormatter) formatter;
1460-
DateTimeFormatter dateTimeFormatter = javaDateFormatter.getParser();
14611460
if (printer == null) {
14621461
printer = javaDateFormatter.getPrinter();
14631462
}
1464-
dateTimeFormatters.add(dateTimeFormatter);
1463+
dateTimeFormatters.add(javaDateFormatter.getParser());
1464+
roundupBuilder.appendOptional(javaDateFormatter.getRoundupParser());
14651465
}
1466+
DateTimeFormatter roundUpParser = roundupBuilder.toFormatter(Locale.ROOT);
14661467

1467-
return new JavaDateFormatter(pattern, printer, dateTimeFormatters.toArray(new DateTimeFormatter[0]));
1468+
return new JavaDateFormatter(pattern, printer, builder -> builder.append(roundUpParser),
1469+
dateTimeFormatters.toArray(new DateTimeFormatter[0]));
14681470
}
14691471

14701472
private static final ZonedDateTime EPOCH_ZONED_DATE_TIME = Instant.EPOCH.atZone(ZoneOffset.UTC);

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,9 +153,11 @@ public long getFrom(TemporalAccessor temporal) {
153153
.toFormatter(Locale.ROOT);
154154

155155
static final DateFormatter SECONDS_FORMATTER = new JavaDateFormatter("epoch_second", SECONDS_FORMATTER3,
156+
builder -> builder.parseDefaulting(ChronoField.NANO_OF_SECOND, 999_999_999L),
156157
SECONDS_FORMATTER1, SECONDS_FORMATTER2, SECONDS_FORMATTER3);
157158

158159
static final DateFormatter MILLIS_FORMATTER = new JavaDateFormatter("epoch_millis", MILLISECONDS_FORMATTER3,
160+
builder -> builder.parseDefaulting(EpochTime.NANOS_OF_MILLI, 999_999L),
159161
MILLISECONDS_FORMATTER1, MILLISECONDS_FORMATTER2, MILLISECONDS_FORMATTER3);
160162

161163
private abstract static class EpochField implements TemporalField {

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

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import java.util.Locale;
3333
import java.util.Map;
3434
import java.util.Objects;
35+
import java.util.function.Consumer;
3536

3637
class JavaDateFormatter implements DateFormatter {
3738

@@ -43,14 +44,27 @@ class JavaDateFormatter implements DateFormatter {
4344
ROUND_UP_BASE_FIELDS.put(ChronoField.HOUR_OF_DAY, 23L);
4445
ROUND_UP_BASE_FIELDS.put(ChronoField.MINUTE_OF_HOUR, 59L);
4546
ROUND_UP_BASE_FIELDS.put(ChronoField.SECOND_OF_MINUTE, 59L);
46-
ROUND_UP_BASE_FIELDS.put(ChronoField.MILLI_OF_SECOND, 999L);
47+
ROUND_UP_BASE_FIELDS.put(ChronoField.NANO_OF_SECOND, 999_999_999L);
4748
}
4849

4950
private final String format;
5051
private final DateTimeFormatter printer;
5152
private final DateTimeFormatter parser;
53+
private final DateTimeFormatter roundupParser;
54+
55+
private JavaDateFormatter(String format, DateTimeFormatter printer, DateTimeFormatter roundupParser, DateTimeFormatter parser) {
56+
this.format = format;
57+
this.printer = printer;
58+
this.roundupParser = roundupParser;
59+
this.parser = parser;
60+
}
5261

5362
JavaDateFormatter(String format, DateTimeFormatter printer, DateTimeFormatter... parsers) {
63+
this(format, printer, builder -> ROUND_UP_BASE_FIELDS.forEach(builder::parseDefaulting), parsers);
64+
}
65+
66+
JavaDateFormatter(String format, DateTimeFormatter printer, Consumer<DateTimeFormatterBuilder> roundupParserConsumer,
67+
DateTimeFormatter... parsers) {
5468
if (printer == null) {
5569
throw new IllegalArgumentException("printer may not be null");
5670
}
@@ -75,6 +89,19 @@ class JavaDateFormatter implements DateFormatter {
7589
}
7690
this.format = format;
7791
this.printer = printer;
92+
93+
DateTimeFormatterBuilder builder = new DateTimeFormatterBuilder();
94+
builder.append(this.parser);
95+
roundupParserConsumer.accept(builder);
96+
DateTimeFormatter roundupFormatter = builder.toFormatter(parser.getLocale());
97+
if (printer.getZone() != null) {
98+
roundupFormatter = roundupFormatter.withZone(printer.getZone());
99+
}
100+
this.roundupParser = roundupFormatter;
101+
}
102+
103+
DateTimeFormatter getRoundupParser() {
104+
return roundupParser;
78105
}
79106

80107
DateTimeFormatter getParser() {
@@ -100,7 +127,7 @@ public DateFormatter withZone(ZoneId zoneId) {
100127
return this;
101128
}
102129

103-
return new JavaDateFormatter(format, printer.withZone(zoneId), parser.withZone(zoneId));
130+
return new JavaDateFormatter(format, printer.withZone(zoneId), roundupParser.withZone(zoneId), parser.withZone(zoneId));
104131
}
105132

106133
@Override
@@ -110,7 +137,7 @@ public DateFormatter withLocale(Locale locale) {
110137
return this;
111138
}
112139

113-
return new JavaDateFormatter(format, printer.withLocale(locale), parser.withLocale(locale));
140+
return new JavaDateFormatter(format, printer.withLocale(locale), roundupParser.withLocale(locale), parser.withLocale(locale));
114141
}
115142

116143
@Override
@@ -123,12 +150,6 @@ public String pattern() {
123150
return format;
124151
}
125152

126-
JavaDateFormatter parseDefaulting(Map<TemporalField, Long> fields) {
127-
final DateTimeFormatterBuilder parseDefaultingBuilder = new DateTimeFormatterBuilder().append(printer);
128-
fields.forEach(parseDefaultingBuilder::parseDefaulting);
129-
return new JavaDateFormatter(format, parseDefaultingBuilder.toFormatter(Locale.ROOT));
130-
}
131-
132153
@Override
133154
public Locale locale() {
134155
return this.printer.getLocale();
@@ -141,7 +162,7 @@ public ZoneId zone() {
141162

142163
@Override
143164
public DateMathParser toDateMathParser() {
144-
return new JavaDateMathParser(this, this.parseDefaulting(ROUND_UP_BASE_FIELDS));
165+
return new JavaDateMathParser(parser, roundupParser);
145166
}
146167

147168
@Override

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

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
package org.elasticsearch.common.time;
2121

2222
import org.elasticsearch.ElasticsearchParseException;
23+
import org.elasticsearch.common.Strings;
2324

2425
import java.time.DateTimeException;
2526
import java.time.DayOfWeek;
@@ -28,6 +29,7 @@
2829
import java.time.ZoneId;
2930
import java.time.ZoneOffset;
3031
import java.time.ZonedDateTime;
32+
import java.time.format.DateTimeFormatter;
3133
import java.time.temporal.ChronoField;
3234
import java.time.temporal.TemporalAccessor;
3335
import java.time.temporal.TemporalAdjusters;
@@ -44,12 +46,10 @@
4446
*/
4547
public class JavaDateMathParser implements DateMathParser {
4648

49+
private final DateTimeFormatter formatter;
50+
private final DateTimeFormatter roundUpFormatter;
4751

48-
49-
private final DateFormatter formatter;
50-
private final DateFormatter roundUpFormatter;
51-
52-
public JavaDateMathParser(DateFormatter formatter, DateFormatter roundUpFormatter) {
52+
public JavaDateMathParser(DateTimeFormatter formatter, DateTimeFormatter roundUpFormatter) {
5353
Objects.requireNonNull(formatter);
5454
this.formatter = formatter;
5555
this.roundUpFormatter = roundUpFormatter;
@@ -208,7 +208,11 @@ private long parseMath(final String mathString, final long time, final boolean r
208208
}
209209

210210
private long parseDateTime(String value, ZoneId timeZone, boolean roundUpIfNoTime) {
211-
DateFormatter formatter = roundUpIfNoTime ? this.roundUpFormatter : this.formatter;
211+
if (Strings.isNullOrEmpty(value)) {
212+
throw new IllegalArgumentException("cannot parse empty date");
213+
}
214+
215+
DateTimeFormatter formatter = roundUpIfNoTime ? this.roundUpFormatter : this.formatter;
212216
try {
213217
if (timeZone == null) {
214218
return DateFormatters.toZonedDateTime(formatter.parse(value)).toInstant().toEpochMilli();

0 commit comments

Comments
 (0)