Skip to content

Commit 65f0127

Browse files
pgomulkapgomulka
authored
Parse composite patterns using ClassicFormat.parseObject backport(#40100) (#40501)
Java-time fails parsing composite patterns when first pattern matches only the prefix of the input. It expects pattern in longest to shortest order. Because of this constructing just one DateTimeFormatter with appendOptional is not sufficient. Parsers have to be iterated and if the parsing fails, the next one in order should be used. In order to not degrade performance parsing should not be throw exceptions on failure. Format.parseObject was used as it only returns null when parsing failed and allows to check if full input was read. closes #39916 backport #40100
1 parent c990b30 commit 65f0127

File tree

4 files changed

+79
-40
lines changed

4 files changed

+79
-40
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1585,7 +1585,7 @@ static JavaDateFormatter merge(String pattern, List<DateFormatter> formatters) {
15851585
if (printer == null) {
15861586
printer = javaDateFormatter.getPrinter();
15871587
}
1588-
dateTimeFormatters.add(javaDateFormatter.getParser());
1588+
dateTimeFormatters.addAll(javaDateFormatter.getParsers());
15891589
roundupBuilder.appendOptional(javaDateFormatter.getRoundupParser());
15901590
}
15911591
DateTimeFormatter roundUpParser = roundupBuilder.toFormatter(Locale.ROOT);
@@ -1632,7 +1632,7 @@ public static ZonedDateTime from(TemporalAccessor accessor) {
16321632
if (zoneId == null) {
16331633
zoneId = ZoneOffset.UTC;
16341634
}
1635-
1635+
16361636
LocalDate localDate = accessor.query(TemporalQueries.localDate());
16371637
LocalTime localTime = accessor.query(TemporalQueries.localTime());
16381638
boolean isLocalDateSet = localDate != null;

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

Lines changed: 59 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
import org.elasticsearch.common.Strings;
2323

24+
import java.text.ParsePosition;
2425
import java.time.ZoneId;
2526
import java.time.format.DateTimeFormatter;
2627
import java.time.format.DateTimeFormatterBuilder;
@@ -29,7 +30,10 @@
2930
import java.time.temporal.TemporalAccessor;
3031
import java.time.temporal.TemporalField;
3132
import java.util.Arrays;
33+
import java.util.Collection;
34+
import java.util.Collections;
3235
import java.util.HashMap;
36+
import java.util.List;
3337
import java.util.Locale;
3438
import java.util.Map;
3539
import java.util.Objects;
@@ -39,6 +43,7 @@ class JavaDateFormatter implements DateFormatter {
3943

4044
// base fields which should be used for default parsing, when we round up for date math
4145
private static final Map<TemporalField, Long> ROUND_UP_BASE_FIELDS = new HashMap<>(6);
46+
4247
{
4348
ROUND_UP_BASE_FIELDS.put(ChronoField.MONTH_OF_YEAR, 1L);
4449
ROUND_UP_BASE_FIELDS.put(ChronoField.DAY_OF_MONTH, 1L);
@@ -50,22 +55,15 @@ class JavaDateFormatter implements DateFormatter {
5055

5156
private final String format;
5257
private final DateTimeFormatter printer;
53-
private final DateTimeFormatter parser;
58+
private final List<DateTimeFormatter> parsers;
5459
private final DateTimeFormatter roundupParser;
5560

56-
private JavaDateFormatter(String format, DateTimeFormatter printer, DateTimeFormatter roundupParser, DateTimeFormatter parser) {
57-
this.format = format;
58-
this.printer = printer;
59-
this.roundupParser = roundupParser;
60-
this.parser = parser;
61-
}
62-
6361
JavaDateFormatter(String format, DateTimeFormatter printer, DateTimeFormatter... parsers) {
6462
this(format, printer, builder -> ROUND_UP_BASE_FIELDS.forEach(builder::parseDefaulting), parsers);
6563
}
6664

6765
JavaDateFormatter(String format, DateTimeFormatter printer, Consumer<DateTimeFormatterBuilder> roundupParserConsumer,
68-
DateTimeFormatter... parsers) {
66+
DateTimeFormatter... parsers) {
6967
if (printer == null) {
7068
throw new IllegalArgumentException("printer may not be null");
7169
}
@@ -79,26 +77,21 @@ private JavaDateFormatter(String format, DateTimeFormatter printer, DateTimeForm
7977
}
8078
this.printer = printer;
8179
this.format = format;
80+
8281
if (parsers.length == 0) {
83-
this.parser = printer;
84-
} else if (parsers.length == 1) {
85-
this.parser = parsers[0];
82+
this.parsers = Collections.singletonList(printer);
8683
} else {
87-
DateTimeFormatterBuilder builder = new DateTimeFormatterBuilder();
88-
for (DateTimeFormatter parser : parsers) {
89-
builder.appendOptional(parser);
90-
}
91-
this.parser = builder.toFormatter(Locale.ROOT);
84+
this.parsers = Arrays.asList(parsers);
9285
}
9386

9487
DateTimeFormatterBuilder builder = new DateTimeFormatterBuilder();
9588
if (format.contains("||") == false) {
96-
builder.append(this.parser);
89+
builder.append(this.parsers.get(0));
9790
}
9891
roundupParserConsumer.accept(builder);
99-
DateTimeFormatter roundupFormatter = builder.toFormatter(parser.getLocale());
92+
DateTimeFormatter roundupFormatter = builder.toFormatter(locale());
10093
if (printer.getZone() != null) {
101-
roundupFormatter = roundupFormatter.withZone(printer.getZone());
94+
roundupFormatter = roundupFormatter.withZone(zone());
10295
}
10396
this.roundupParser = roundupFormatter;
10497
}
@@ -107,10 +100,6 @@ DateTimeFormatter getRoundupParser() {
107100
return roundupParser;
108101
}
109102

110-
DateTimeFormatter getParser() {
111-
return parser;
112-
}
113-
114103
DateTimeFormatter getPrinter() {
115104
return printer;
116105
}
@@ -122,30 +111,64 @@ public TemporalAccessor parse(String input) {
122111
}
123112

124113
try {
125-
return parser.parse(input);
114+
return doParse(input);
126115
} catch (DateTimeParseException e) {
127116
throw new IllegalArgumentException("failed to parse date field [" + input + "] with format [" + format + "]", e);
128117
}
129118
}
130119

120+
/**
121+
* Attempt parsing the input without throwing exception. If multiple parsers are provided,
122+
* it will continue iterating if the previous parser failed. The pattern must fully match, meaning whole input was used.
123+
* This also means that this method depends on <code>DateTimeFormatter.ClassicFormat.parseObject</code>
124+
* which does not throw exceptions when parsing failed.
125+
*
126+
* The approach with collection of parsers was taken because java-time requires ordering on optional (composite)
127+
* patterns. Joda does not suffer from this.
128+
* https://bugs.openjdk.java.net/browse/JDK-8188771
129+
*
130+
* @param input An arbitrary string resembling the string representation of a date or time
131+
* @return a TemporalAccessor if parsing was successful.
132+
* @throws DateTimeParseException when unable to parse with any parsers
133+
*/
134+
private TemporalAccessor doParse(String input) {
135+
if (parsers.size() > 1) {
136+
for (DateTimeFormatter formatter : parsers) {
137+
ParsePosition pos = new ParsePosition(0);
138+
Object object = formatter.toFormat().parseObject(input, pos);
139+
if (parsingSucceeded(object, input, pos) == true) {
140+
return (TemporalAccessor) object;
141+
}
142+
}
143+
throw new DateTimeParseException("Failed to parse with all enclosed parsers", input, 0);
144+
}
145+
return this.parsers.get(0).parse(input);
146+
}
147+
148+
private boolean parsingSucceeded(Object object, String input, ParsePosition pos) {
149+
return object != null && pos.getIndex() == input.length();
150+
}
151+
131152
@Override
132153
public DateFormatter withZone(ZoneId zoneId) {
133154
// shortcurt to not create new objects unnecessarily
134-
if (zoneId.equals(parser.getZone())) {
155+
if (zoneId.equals(zone())) {
135156
return this;
136157
}
137158

138-
return new JavaDateFormatter(format, printer.withZone(zoneId), roundupParser.withZone(zoneId), parser.withZone(zoneId));
159+
return new JavaDateFormatter(format, printer.withZone(zoneId),
160+
parsers.stream().map(p -> p.withZone(zoneId)).toArray(size -> new DateTimeFormatter[size]));
139161
}
140162

141163
@Override
142164
public DateFormatter withLocale(Locale locale) {
143165
// shortcurt to not create new objects unnecessarily
144-
if (locale.equals(parser.getLocale())) {
166+
if (locale.equals(locale())) {
145167
return this;
146168
}
147169

148-
return new JavaDateFormatter(format, printer.withLocale(locale), roundupParser.withLocale(locale), parser.withLocale(locale));
170+
return new JavaDateFormatter(format, printer.withLocale(locale),
171+
parsers.stream().map(p -> p.withLocale(locale)).toArray(size -> new DateTimeFormatter[size]));
149172
}
150173

151174
@Override
@@ -170,7 +193,7 @@ public ZoneId zone() {
170193

171194
@Override
172195
public DateMathParser toDateMathParser() {
173-
return new JavaDateMathParser(format, parser, roundupParser);
196+
return new JavaDateMathParser(format, this, getRoundupParser());
174197
}
175198

176199
@Override
@@ -186,12 +209,16 @@ public boolean equals(Object obj) {
186209
JavaDateFormatter other = (JavaDateFormatter) obj;
187210

188211
return Objects.equals(format, other.format) &&
189-
Objects.equals(locale(), other.locale()) &&
190-
Objects.equals(this.printer.getZone(), other.printer.getZone());
212+
Objects.equals(locale(), other.locale()) &&
213+
Objects.equals(this.printer.getZone(), other.printer.getZone());
191214
}
192215

193216
@Override
194217
public String toString() {
195218
return String.format(Locale.ROOT, "format[%s] locale[%s]", format, locale());
196219
}
220+
221+
Collection<DateTimeFormatter> getParsers() {
222+
return parsers;
223+
}
197224
}

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

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import java.time.temporal.TemporalAdjusters;
3636
import java.time.temporal.TemporalQueries;
3737
import java.util.Objects;
38+
import java.util.function.Function;
3839
import java.util.function.LongSupplier;
3940

4041
/**
@@ -46,11 +47,11 @@
4647
*/
4748
public class JavaDateMathParser implements DateMathParser {
4849

49-
private final DateTimeFormatter formatter;
50+
private final JavaDateFormatter formatter;
5051
private final DateTimeFormatter roundUpFormatter;
5152
private final String format;
5253

53-
JavaDateMathParser(String format, DateTimeFormatter formatter, DateTimeFormatter roundUpFormatter) {
54+
JavaDateMathParser(String format, JavaDateFormatter formatter, DateTimeFormatter roundUpFormatter) {
5455
this.format = format;
5556
Objects.requireNonNull(formatter);
5657
this.formatter = formatter;
@@ -215,20 +216,20 @@ private Instant parseDateTime(String value, ZoneId timeZone, boolean roundUpIfNo
215216
throw new ElasticsearchParseException("cannot parse empty date");
216217
}
217218

218-
DateTimeFormatter formatter = roundUpIfNoTime ? this.roundUpFormatter : this.formatter;
219+
Function<String,TemporalAccessor> formatter = roundUpIfNoTime ? this.roundUpFormatter::parse : this.formatter::parse;
219220
try {
220221
if (timeZone == null) {
221-
return DateFormatters.from(formatter.parse(value)).toInstant();
222+
return DateFormatters.from(formatter.apply(value)).toInstant();
222223
} else {
223-
TemporalAccessor accessor = formatter.parse(value);
224+
TemporalAccessor accessor = formatter.apply(value);
224225
ZoneId zoneId = TemporalQueries.zone().queryFrom(accessor);
225226
if (zoneId != null) {
226227
timeZone = zoneId;
227228
}
228229

229230
return DateFormatters.from(accessor).withZoneSameLocal(timeZone).toInstant();
230231
}
231-
} catch (DateTimeParseException e) {
232+
} catch (IllegalArgumentException | DateTimeParseException e) {
232233
throw new ElasticsearchParseException("failed to parse date field [{}] with format [{}]: [{}]",
233234
e, value, format, e.getMessage());
234235
}

‎server/src/test/java/org/elasticsearch/common/joda/JavaJodaTimeDuellingTests.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -343,6 +343,17 @@ public void testDuellingFormatsValidParsing() {
343343
assertSameDate("2012-W1-1", "weekyear_week_day");
344344
}
345345

346+
public void testCompositeParsing(){
347+
//in all these examples the second pattern will be used
348+
assertSameDate("2014-06-06T12:01:02.123", "yyyy-MM-dd'T'HH:mm:ss||yyyy-MM-dd'T'HH:mm:ss.SSS");
349+
assertSameDate("2014-06-06T12:01:02.123", "strictDateTimeNoMillis||yyyy-MM-dd'T'HH:mm:ss.SSS");
350+
assertSameDate("2014-06-06T12:01:02.123", "yyyy-MM-dd'T'HH:mm:ss+HH:MM||yyyy-MM-dd'T'HH:mm:ss.SSS");
351+
}
352+
353+
public void testExceptionWhenCompositeParsingFails(){
354+
assertParseException("2014-06-06T12:01:02.123", "yyyy-MM-dd'T'HH:mm:ss||yyyy-MM-dd'T'HH:mm:ss.SS");
355+
}
356+
346357
public void testDuelingStrictParsing() {
347358
assertSameDate("2018W313", "strict_basic_week_date");
348359
assertParseException("18W313", "strict_basic_week_date");

0 commit comments

Comments
 (0)