Skip to content

Commit

Permalink
Support optional parsers in any order with DateMathParser Backport(46…
Browse files Browse the repository at this point in the history 
…654) (elastic#47217)

Currently DateMathParser with roundUp = true is relying on the DateFormatter build with
combined optional sub parsers with defaulted fields (depending on the formatter).
That means that for yyyy-MM-dd'T'HH:mm:ss||yyyy-MM-dd'T'HH:mm:ss.SSS
Java.time implementation expects optional parsers in order from most specific to
least specific (reverse in the example above).
It is causing a problem because the first parsing succeeds but does not consume the full input.
The second parser should be used.
We can work around this with keeping a list of RoundUpParsers and iterate over them choosing
the one that parsed full input. The same approach we used for regular (non date math) in
relates elastic#40100
The jdk is not considering this to be a bug https://bugs.openjdk.java.net/browse/JDK-8188771

Those below will expect this change first
relates elastic#46242
relates elastic#45284
backport elastic#46654
  • Loading branch information
@pgomulka
pgomulka committed Dec 9, 2019
1 parent 0f2f5e2 commit c0fbdda
Show file tree
Hide file tree
Showing 8 changed files with 212 additions and 61 deletions.
2 changes: 1 addition & 1 deletion server/src/main/java/org/elasticsearch/common/time/DateFormatter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ static DateFormatter forPattern(String input) {
return formatters.get(0);
}

return DateFormatters.merge(input, formatters);
return JavaDateFormatter.combined(input, formatters);
}

static List<String> splitCombinedPatterns(String input) {
Expand Down
25 changes: 1 addition & 24 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 @@ -40,8 +40,6 @@
import java.time.temporal.TemporalAdjusters;
import java.time.temporal.TemporalQueries;
import java.time.temporal.WeekFields;
import java.util.ArrayList;
import java.util.List;
import java.util.Locale;

import static java.time.temporal.ChronoField.DAY_OF_MONTH;
Expand Down Expand Up @@ -1021,7 +1019,7 @@ public class DateFormatters {
new DateTimeFormatterBuilder().appendValue(WeekFields.ISO.weekBasedYear()).toFormatter(Locale.ROOT));

/*
* Returns a formatter for a four digit weekyear. (uuuu)
* Returns a formatter for a four digit year. (uuuu)
*/
private static final DateFormatter YEAR = new JavaDateFormatter("year",
new DateTimeFormatterBuilder().appendValue(ChronoField.YEAR).toFormatter(Locale.ROOT));
Expand Down Expand Up @@ -1593,27 +1591,6 @@ public static DateFormatter forPattern(String input) {
}
}

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;
if (printer == null) {
printer = javaDateFormatter.getPrinter();
}
dateTimeFormatters.addAll(javaDateFormatter.getParsers());
roundupBuilder.appendOptional(javaDateFormatter.getRoundupParser());
}
DateTimeFormatter roundUpParser = roundupBuilder.toFormatter(Locale.ROOT);

return new JavaDateFormatter(pattern, printer, builder -> builder.append(roundUpParser),
dateTimeFormatters.toArray(new DateTimeFormatter[0]));
}

private static final LocalDate LOCALDATE_EPOCH = LocalDate.of(1970, 1, 1);

/**
Expand Down
89 changes: 74 additions & 15 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 @@ -29,6 +29,7 @@
import java.time.temporal.ChronoField;
import java.time.temporal.TemporalAccessor;
import java.time.temporal.TemporalField;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
Expand All @@ -38,6 +39,7 @@
import java.util.Map;
import java.util.Objects;
import java.util.function.Consumer;
import java.util.stream.Collectors;

class JavaDateFormatter implements DateFormatter {

Expand All @@ -56,13 +58,33 @@ class JavaDateFormatter implements DateFormatter {
private final String format;
private final DateTimeFormatter printer;
private final List<DateTimeFormatter> parsers;
private final DateTimeFormatter roundupParser;
private final JavaDateFormatter roundupParser;

static class RoundUpFormatter extends JavaDateFormatter{

RoundUpFormatter(String format, List<DateTimeFormatter> roundUpParsers) {
super(format, firstFrom(roundUpParsers),null, roundUpParsers);
}

private static DateTimeFormatter firstFrom(List<DateTimeFormatter> roundUpParsers) {
return roundUpParsers.get(0);
}

@Override
JavaDateFormatter getRoundupParser() {
throw new UnsupportedOperationException("RoundUpFormatter does not have another roundUpFormatter");
}
}

// named formatters use default roundUpParser
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,
// subclasses override roundUpParser
JavaDateFormatter(String format,
DateTimeFormatter printer,
Consumer<DateTimeFormatterBuilder> roundupParserConsumer,
DateTimeFormatter... parsers) {
if (printer == null) {
throw new IllegalArgumentException("printer may not be null");
Expand All @@ -83,20 +105,51 @@ class JavaDateFormatter implements DateFormatter {
} else {
this.parsers = Arrays.asList(parsers);
}
//this is when the RoundUp Formatter is created. In further merges (with ||) it will only append this one to a list.
List<DateTimeFormatter> roundUp = createRoundUpParser(format, roundupParserConsumer);
this.roundupParser = new RoundUpFormatter(format, roundUp) ;
}

DateTimeFormatterBuilder builder = new DateTimeFormatterBuilder();
private List<DateTimeFormatter> createRoundUpParser(String format,
Consumer<DateTimeFormatterBuilder> roundupParserConsumer) {
if (format.contains("||") == false) {
DateTimeFormatterBuilder builder = new DateTimeFormatterBuilder();
builder.append(this.parsers.get(0));
roundupParserConsumer.accept(builder);
return Arrays.asList(builder.toFormatter(locale()));
}
roundupParserConsumer.accept(builder);
DateTimeFormatter roundupFormatter = builder.toFormatter(locale());
if (printer.getZone() != null) {
roundupFormatter = roundupFormatter.withZone(zone());
return null;
}

public static DateFormatter combined(String input, List<DateFormatter> formatters) {
assert formatters.size() > 0;

List<DateTimeFormatter> parsers = new ArrayList<>(formatters.size());
List<DateTimeFormatter> roundUpParsers = new ArrayList<>(formatters.size());

DateTimeFormatter printer = null;
for (DateFormatter formatter : formatters) {
assert formatter instanceof JavaDateFormatter;
JavaDateFormatter javaDateFormatter = (JavaDateFormatter) formatter;
if (printer == null) {
printer = javaDateFormatter.getPrinter();
}
parsers.addAll(javaDateFormatter.getParsers());
roundUpParsers.addAll(javaDateFormatter.getRoundupParser().getParsers());
}
this.roundupParser = roundupFormatter;

return new JavaDateFormatter(input, printer, roundUpParsers, parsers);
}

DateTimeFormatter getRoundupParser() {
private JavaDateFormatter(String format, DateTimeFormatter printer, List<DateTimeFormatter> roundUpParsers,
List<DateTimeFormatter> parsers) {
this.format = format;
this.printer = printer;
this.roundupParser = roundUpParsers != null ? new RoundUpFormatter(format, roundUpParsers ) : null;
this.parsers = parsers;
}

JavaDateFormatter getRoundupParser() {
return roundupParser;
}

Expand Down Expand Up @@ -155,9 +208,12 @@ public DateFormatter withZone(ZoneId zoneId) {
if (zoneId.equals(zone())) {
return this;
}

return new JavaDateFormatter(format, printer.withZone(zoneId),
parsers.stream().map(p -> p.withZone(zoneId)).toArray(size -> new DateTimeFormatter[size]));
List<DateTimeFormatter> parsers = this.parsers.stream().map(p -> p.withZone(zoneId)).collect(Collectors.toList());
List<DateTimeFormatter> roundUpParsers = this.roundupParser.getParsers()
.stream()
.map(p -> p.withZone(zoneId))
.collect(Collectors.toList());
return new JavaDateFormatter(format, printer.withZone(zoneId), roundUpParsers, parsers);
}

@Override
Expand All @@ -166,9 +222,12 @@ public DateFormatter withLocale(Locale locale) {
if (locale.equals(locale())) {
return this;
}

return new JavaDateFormatter(format, printer.withLocale(locale),
parsers.stream().map(p -> p.withLocale(locale)).toArray(size -> new DateTimeFormatter[size]));
List<DateTimeFormatter> parsers = this.parsers.stream().map(p -> p.withLocale(locale)).collect(Collectors.toList());
List<DateTimeFormatter> roundUpParsers = this.roundupParser.getParsers()
.stream()
.map(p -> p.withLocale(locale))
.collect(Collectors.toList());
return new JavaDateFormatter(format, printer.withLocale(locale), roundUpParsers, parsers);
}

@Override
Expand Down
16 changes: 7 additions & 9 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 @@ -29,13 +29,11 @@
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;
import java.time.temporal.TemporalQueries;
import java.util.Objects;
import java.util.function.Function;
import java.util.function.LongSupplier;

/**
Expand All @@ -48,14 +46,14 @@
public class JavaDateMathParser implements DateMathParser {

private final JavaDateFormatter formatter;
private final DateTimeFormatter roundUpFormatter;
private final String format;
private final JavaDateFormatter roundupParser;

JavaDateMathParser(String format, JavaDateFormatter formatter, DateTimeFormatter roundUpFormatter) {
Objects.requireNonNull(formatter);
JavaDateMathParser(String format, JavaDateFormatter formatter, JavaDateFormatter roundupParser) {
this.format = format;
this.roundupParser = roundupParser;
Objects.requireNonNull(formatter);
this.formatter = formatter;
this.roundUpFormatter = roundUpFormatter;
}

@Override
Expand Down Expand Up @@ -215,12 +213,12 @@ private long parseDateTime(String value, ZoneId timeZone, boolean roundUpIfNoTim
throw new IllegalArgumentException("cannot parse empty date");
}

Function<String,TemporalAccessor> formatter = roundUpIfNoTime ? this.roundUpFormatter::parse : this.formatter::parse;
DateFormatter formatter = roundUpIfNoTime ? this.roundupParser : this.formatter;
try {
if (timeZone == null) {
return DateFormatters.from(formatter.apply(value)).toInstant().toEpochMilli();
return DateFormatters.from(formatter.parse(value)).toInstant().toEpochMilli();
} else {
TemporalAccessor accessor = formatter.apply(value);
TemporalAccessor accessor = formatter.parse(value);
ZoneId zoneId = TemporalQueries.zone().queryFrom(accessor);
if (zoneId != null) {
timeZone = zoneId;
Expand Down

0 comments on commit c0fbdda

Please sign in to comment.