diff --git a/server/src/main/java/org/elasticsearch/common/time/DateFormatter.java b/server/src/main/java/org/elasticsearch/common/time/DateFormatter.java index 3ed6d95efe5..bacb12e680c 100644 --- a/server/src/main/java/org/elasticsearch/common/time/DateFormatter.java +++ b/server/src/main/java/org/elasticsearch/common/time/DateFormatter.java @@ -147,7 +147,7 @@ public interface DateFormatter { return formatters.get(0); } - return DateFormatters.merge(input, formatters); + return JavaDateFormatter.combined(input, formatters); } static List splitCombinedPatterns(String input) { diff --git a/server/src/main/java/org/elasticsearch/common/time/DateFormatters.java b/server/src/main/java/org/elasticsearch/common/time/DateFormatters.java index 601bb7fff96..0cd0e9dff78 100644 --- a/server/src/main/java/org/elasticsearch/common/time/DateFormatters.java +++ b/server/src/main/java/org/elasticsearch/common/time/DateFormatters.java @@ -40,8 +40,6 @@ import java.time.temporal.TemporalAccessor; 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; @@ -1033,7 +1031,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)); @@ -1605,27 +1603,6 @@ public class DateFormatters { } } - static JavaDateFormatter merge(String pattern, List formatters) { - assert formatters.size() > 0; - - List 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); /** diff --git a/server/src/main/java/org/elasticsearch/common/time/JavaDateFormatter.java b/server/src/main/java/org/elasticsearch/common/time/JavaDateFormatter.java index 35136005822..e1ac6db1196 100644 --- a/server/src/main/java/org/elasticsearch/common/time/JavaDateFormatter.java +++ b/server/src/main/java/org/elasticsearch/common/time/JavaDateFormatter.java @@ -29,6 +29,7 @@ import java.time.format.DateTimeParseException; 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; @@ -57,19 +58,33 @@ class JavaDateFormatter implements DateFormatter { private final String format; private final DateTimeFormatter printer; private final List parsers; - private final DateTimeFormatter roundupParser; + private final JavaDateFormatter roundupParser; - private JavaDateFormatter(String format, DateTimeFormatter printer, DateTimeFormatter roundupParser, List parsers) { - this.format = format; - this.printer = printer; - this.roundupParser = roundupParser; - this.parsers = parsers; + static class RoundUpFormatter extends JavaDateFormatter{ + + RoundUpFormatter(String format, List roundUpParsers) { + super(format, firstFrom(roundUpParsers),null, roundUpParsers); + } + + private static DateTimeFormatter firstFrom(List 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 roundupParserConsumer, + // subclasses override roundUpParser + JavaDateFormatter(String format, + DateTimeFormatter printer, + Consumer roundupParserConsumer, DateTimeFormatter... parsers) { if (printer == null) { throw new IllegalArgumentException("printer may not be null"); @@ -90,20 +105,51 @@ class JavaDateFormatter implements DateFormatter { } else { this.parsers = Arrays.asList(parsers); } - - DateTimeFormatterBuilder builder = new DateTimeFormatterBuilder(); - if (format.contains("||") == false) { - builder.append(this.parsers.get(0)); - } - roundupParserConsumer.accept(builder); - DateTimeFormatter roundupFormatter = builder.toFormatter(locale()); - if (printer.getZone() != null) { - roundupFormatter = roundupFormatter.withZone(zone()); - } - this.roundupParser = roundupFormatter; + //this is when the RoundUp Formatter is created. In further merges (with ||) it will only append this one to a list. + List roundUp = createRoundUpParser(format, roundupParserConsumer); + this.roundupParser = new RoundUpFormatter(format, roundUp) ; } - DateTimeFormatter getRoundupParser() { + private List createRoundUpParser(String format, + Consumer roundupParserConsumer) { + if (format.contains("||") == false) { + DateTimeFormatterBuilder builder = new DateTimeFormatterBuilder(); + builder.append(this.parsers.get(0)); + roundupParserConsumer.accept(builder); + return Arrays.asList(builder.toFormatter(locale())); + } + return null; + } + + public static DateFormatter combined(String input, List formatters) { + assert formatters.size() > 0; + + List parsers = new ArrayList<>(formatters.size()); + List 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()); + } + + return new JavaDateFormatter(input, printer, roundUpParsers, parsers); + } + + private JavaDateFormatter(String format, DateTimeFormatter printer, List roundUpParsers, + List parsers) { + this.format = format; + this.printer = printer; + this.roundupParser = roundUpParsers != null ? new RoundUpFormatter(format, roundUpParsers ) : null; + this.parsers = parsers; + } + + JavaDateFormatter getRoundupParser() { return roundupParser; } @@ -162,8 +208,12 @@ class JavaDateFormatter implements DateFormatter { if (zoneId.equals(zone())) { return this; } - return new JavaDateFormatter(format, printer.withZone(zoneId), getRoundupParser().withZone(zoneId), - parsers.stream().map(p -> p.withZone(zoneId)).collect(Collectors.toList())); + List parsers = this.parsers.stream().map(p -> p.withZone(zoneId)).collect(Collectors.toList()); + List roundUpParsers = this.roundupParser.getParsers() + .stream() + .map(p -> p.withZone(zoneId)) + .collect(Collectors.toList()); + return new JavaDateFormatter(format, printer.withZone(zoneId), roundUpParsers, parsers); } @Override @@ -172,8 +222,12 @@ class JavaDateFormatter implements DateFormatter { if (locale.equals(locale())) { return this; } - return new JavaDateFormatter(format, printer.withLocale(locale), getRoundupParser().withLocale(locale), - parsers.stream().map(p -> p.withLocale(locale)).collect(Collectors.toList())); + List parsers = this.parsers.stream().map(p -> p.withLocale(locale)).collect(Collectors.toList()); + List roundUpParsers = this.roundupParser.getParsers() + .stream() + .map(p -> p.withLocale(locale)) + .collect(Collectors.toList()); + return new JavaDateFormatter(format, printer.withLocale(locale), roundUpParsers, parsers); } @Override diff --git a/server/src/main/java/org/elasticsearch/common/time/JavaDateMathParser.java b/server/src/main/java/org/elasticsearch/common/time/JavaDateMathParser.java index 78d4f10d87c..b3fd8fa0f27 100644 --- a/server/src/main/java/org/elasticsearch/common/time/JavaDateMathParser.java +++ b/server/src/main/java/org/elasticsearch/common/time/JavaDateMathParser.java @@ -28,14 +28,12 @@ import java.time.LocalTime; import java.time.ZoneId; import java.time.ZoneOffset; 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.time.temporal.TemporalAdjusters; import java.time.temporal.TemporalQueries; import java.util.Objects; -import java.util.function.Function; import java.util.function.LongSupplier; /** @@ -48,14 +46,14 @@ import java.util.function.LongSupplier; 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) { + JavaDateMathParser(String format, JavaDateFormatter formatter, JavaDateFormatter roundupParser) { this.format = format; + this.roundupParser = roundupParser; Objects.requireNonNull(formatter); this.formatter = formatter; - this.roundUpFormatter = roundUpFormatter; } @Override @@ -217,12 +215,12 @@ public class JavaDateMathParser implements DateMathParser { throw new ElasticsearchParseException("cannot parse empty date"); } - Function 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(); + return DateFormatters.from(formatter.parse(value)).toInstant(); } else { - TemporalAccessor accessor = formatter.apply(value); + TemporalAccessor accessor = formatter.parse(value); ZoneId zoneId = TemporalQueries.zone().queryFrom(accessor); if (zoneId != null) { timeZone = zoneId; diff --git a/server/src/test/java/org/elasticsearch/common/joda/JavaJodaTimeDuellingTests.java b/server/src/test/java/org/elasticsearch/common/joda/JavaJodaTimeDuellingTests.java index 9e2a6de5b64..e686193645f 100644 --- a/server/src/test/java/org/elasticsearch/common/joda/JavaJodaTimeDuellingTests.java +++ b/server/src/test/java/org/elasticsearch/common/joda/JavaJodaTimeDuellingTests.java @@ -19,15 +19,18 @@ package org.elasticsearch.common.joda; +import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.bootstrap.JavaVersion; import org.elasticsearch.common.time.DateFormatter; import org.elasticsearch.common.time.DateFormatters; +import org.elasticsearch.common.time.DateMathParser; import org.elasticsearch.test.ESTestCase; import org.joda.time.DateTime; import org.joda.time.DateTimeZone; -import org.joda.time.format.ISODateTimeFormat; import org.joda.time.format.DateTimeFormat; +import org.joda.time.format.ISODateTimeFormat; +import java.time.ZoneId; import java.time.ZoneOffset; import java.time.ZonedDateTime; import java.time.format.DateTimeFormatter; @@ -39,6 +42,34 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; public class JavaJodaTimeDuellingTests extends ESTestCase { + public void testCompositeDateMathParsing(){ + //in all these examples the second pattern will be used + assertDateMathEquals("2014-06-06T12:01:02.123", "yyyy-MM-dd'T'HH:mm:ss||yyyy-MM-dd'T'HH:mm:ss.SSS"); + assertDateMathEquals("2014-06-06T12:01:02.123", "strictDateTimeNoMillis||yyyy-MM-dd'T'HH:mm:ss.SSS"); + assertDateMathEquals("2014-06-06T12:01:02.123", "yyyy-MM-dd'T'HH:mm:ss+HH:MM||yyyy-MM-dd'T'HH:mm:ss.SSS"); + } + + public void testExceptionWhenCompositeParsingFailsDateMath(){ + //both parsing failures should contain pattern and input text in exception + //both patterns fail parsing the input text due to only 2 digits of millis. Hence full text was not parsed. + String pattern = "yyyy-MM-dd'T'HH:mm:ss||yyyy-MM-dd'T'HH:mm:ss.SS"; + String text = "2014-06-06T12:01:02.123"; + ElasticsearchParseException e1 = expectThrows(ElasticsearchParseException.class, + () -> dateMathToMillis(text, DateFormatter.forPattern(pattern))); + assertThat(e1.getMessage(), containsString(pattern)); + assertThat(e1.getMessage(), containsString(text)); + + ElasticsearchParseException e2 = expectThrows(ElasticsearchParseException.class, + () -> dateMathToMillis(text, Joda.forPattern(pattern))); + assertThat(e2.getMessage(), containsString(pattern)); + assertThat(e2.getMessage(), containsString(text)); + } + + private long dateMathToMillis(String text, DateFormatter dateFormatter) { + DateFormatter javaFormatter = dateFormatter.withLocale(randomLocale(random())); + DateMathParser javaDateMath = javaFormatter.toDateMathParser(); + return javaDateMath.parse(text, () -> 0, true, (ZoneId) null).toEpochMilli(); + } //these parsers should allow both ',' and '.' as a decimal point public void testDecimalPointParsing(){ @@ -871,4 +902,11 @@ public class JavaJodaTimeDuellingTests extends ESTestCase { assertThat(e.getMessage(), containsString(input)); assertThat(e.getMessage(), containsString(format)); } + + private void assertDateMathEquals(String text, String pattern) { + long gotMillisJava = dateMathToMillis(text, DateFormatter.forPattern(pattern)); + long gotMillisJoda = dateMathToMillis(text, Joda.forPattern(pattern)); + + assertEquals(gotMillisJoda, gotMillisJava); + } } diff --git a/server/src/test/java/org/elasticsearch/common/joda/JodaDateMathParserTests.java b/server/src/test/java/org/elasticsearch/common/joda/JodaDateMathParserTests.java index f6382b92343..d1a90328dfb 100644 --- a/server/src/test/java/org/elasticsearch/common/joda/JodaDateMathParserTests.java +++ b/server/src/test/java/org/elasticsearch/common/joda/JodaDateMathParserTests.java @@ -27,6 +27,7 @@ import org.joda.time.DateTimeZone; import java.time.Instant; import java.time.ZoneId; +import java.time.ZoneOffset; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.LongSupplier; @@ -59,6 +60,19 @@ public class JodaDateMathParserTests extends ESTestCase { } } + public void testOverridingLocaleOrZoneAndCompositeRoundUpParser() { + //the pattern has to be composite and the match should not be on the first one + DateFormatter formatter = Joda.forPattern("date||epoch_millis").withLocale(randomLocale(random())); + DateMathParser parser = formatter.toDateMathParser(); + long gotMillis = parser.parse("297276785531", () -> 0, true, (ZoneId) null).toEpochMilli(); + assertDateEquals(gotMillis, "297276785531", "297276785531"); + + formatter = Joda.forPattern("date||epoch_millis").withZone(ZoneOffset.UTC); + parser = formatter.toDateMathParser(); + gotMillis = parser.parse("297276785531", () -> 0, true, (ZoneId) null).toEpochMilli(); + assertDateEquals(gotMillis, "297276785531", "297276785531"); + } + public void testBasicDates() { assertDateMathEquals("2014", "2014-01-01T00:00:00.000"); assertDateMathEquals("2014-05", "2014-05-01T00:00:00.000"); diff --git a/server/src/test/java/org/elasticsearch/common/time/DateFormattersTests.java b/server/src/test/java/org/elasticsearch/common/time/DateFormattersTests.java index 8f2a6616643..c5956e5c29a 100644 --- a/server/src/test/java/org/elasticsearch/common/time/DateFormattersTests.java +++ b/server/src/test/java/org/elasticsearch/common/time/DateFormattersTests.java @@ -26,7 +26,6 @@ import java.time.Instant; 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.util.Locale; @@ -253,7 +252,7 @@ public class DateFormattersTests extends ESTestCase { public void testRoundupFormatterWithEpochDates() { assertRoundupFormatter("epoch_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(); + JavaDateFormatter roundUpFormatter = ((JavaDateFormatter) DateFormatter.forPattern("8epoch_millis")).getRoundupParser(); Instant epochMilliInstant = DateFormatters.from(roundUpFormatter.parse("1234567890")).toInstant(); assertThat(epochMilliInstant.getLong(ChronoField.NANO_OF_SECOND), is(890_999_999L)); @@ -266,7 +265,7 @@ public class DateFormattersTests extends ESTestCase { assertRoundupFormatter("epoch_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(); + JavaDateFormatter epochSecondRoundupParser = ((JavaDateFormatter) DateFormatter.forPattern("8epoch_second")).getRoundupParser(); Instant epochSecondInstant = DateFormatters.from(epochSecondRoundupParser.parse("1234567890")).toInstant(); assertThat(epochSecondInstant.getLong(ChronoField.NANO_OF_SECOND), is(999_999_999L)); @@ -280,7 +279,7 @@ public class DateFormattersTests extends ESTestCase { private void assertRoundupFormatter(String format, String input, long expectedMilliSeconds) { JavaDateFormatter dateFormatter = (JavaDateFormatter) DateFormatter.forPattern(format); dateFormatter.parse(input); - DateTimeFormatter roundUpFormatter = dateFormatter.getRoundupParser(); + JavaDateFormatter roundUpFormatter = dateFormatter.getRoundupParser(); long millis = DateFormatters.from(roundUpFormatter.parse(input)).toInstant().toEpochMilli(); assertThat(millis, is(expectedMilliSeconds)); } @@ -290,8 +289,8 @@ public class DateFormattersTests extends ESTestCase { String format = randomFrom("epoch_second", "epoch_millis", "strict_date_optional_time", "uuuu-MM-dd'T'HH:mm:ss.SSS", "strict_date_optional_time||date_optional_time"); JavaDateFormatter formatter = (JavaDateFormatter) DateFormatter.forPattern(format).withZone(zoneId); - DateTimeFormatter roundUpFormatter = formatter.getRoundupParser(); - assertThat(roundUpFormatter.getZone(), is(zoneId)); + JavaDateFormatter roundUpFormatter = formatter.getRoundupParser(); + assertThat(roundUpFormatter.zone(), is(zoneId)); assertThat(formatter.zone(), is(zoneId)); } @@ -300,8 +299,8 @@ public class DateFormattersTests extends ESTestCase { String format = randomFrom("epoch_second", "epoch_millis", "strict_date_optional_time", "uuuu-MM-dd'T'HH:mm:ss.SSS", "strict_date_optional_time||date_optional_time"); JavaDateFormatter formatter = (JavaDateFormatter) DateFormatter.forPattern(format).withLocale(locale); - DateTimeFormatter roundupParser = formatter.getRoundupParser(); - assertThat(roundupParser.getLocale(), is(locale)); + JavaDateFormatter roundupParser = formatter.getRoundupParser(); + assertThat(roundupParser.locale(), is(locale)); assertThat(formatter.locale(), is(locale)); } diff --git a/server/src/test/java/org/elasticsearch/common/time/JavaDateMathParserTests.java b/server/src/test/java/org/elasticsearch/common/time/JavaDateMathParserTests.java index 2fb52460896..c50ec185dc5 100644 --- a/server/src/test/java/org/elasticsearch/common/time/JavaDateMathParserTests.java +++ b/server/src/test/java/org/elasticsearch/common/time/JavaDateMathParserTests.java @@ -44,7 +44,7 @@ public class JavaDateMathParserTests extends ESTestCase { long gotMillis = parser.parse("297276785531", () -> 0, true, (ZoneId) null).toEpochMilli(); assertDateEquals(gotMillis, "297276785531", "297276785531"); - formatter = DateFormatter.forPattern("date||epoch_millis").withZone(randomZone()); + formatter = DateFormatter.forPattern("date||epoch_millis").withZone(ZoneOffset.UTC); parser = formatter.toDateMathParser(); gotMillis = parser.parse("297276785531", () -> 0, true, (ZoneId) null).toEpochMilli(); assertDateEquals(gotMillis, "297276785531", "297276785531"); @@ -301,6 +301,11 @@ public class JavaDateMathParserTests extends ESTestCase { } private void assertDateMathEquals(String toTest, String expected, final long now, boolean roundUp, ZoneId timeZone) { + assertDateMathEquals(parser, toTest, expected, now, roundUp, timeZone); + } + + private void assertDateMathEquals(DateMathParser parser, String toTest, String expected, final long now, + boolean roundUp, ZoneId timeZone) { long gotMillis = parser.parse(toTest, () -> now, roundUp, timeZone).toEpochMilli(); assertDateEquals(gotMillis, toTest, expected); }