From 1faa8b451c8f5490284f7f92ffa87777167f43c8 Mon Sep 17 00:00:00 2001 From: Przemyslaw Gomulka Date: Wed, 8 Jan 2020 14:05:22 +0100 Subject: [PATCH 1/3] Allow parsing timezone without fully provided time (#50178) strict_date_optional_time changes to have optional minute part. It already allowed optional second and fraction of second part. This allows parsing 2018-01-01T00+01 , 2018-01-01T00:00+01 , 2018-01-01T00:00:00+01 , 2018-01-01T00:00:00.000+01 It won't allow parsing a timezone without an hour part as this is not allowed by iso8601 spec closes #49351 --- .../common/time/DateFormatters.java | 50 ++++---- .../joda/JavaJodaTimeDuellingTests.java | 115 +++++++++++++++--- 2 files changed, 124 insertions(+), 41 deletions(-) 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 26ec15559b803..b936329f0f650 100644 --- a/server/src/main/java/org/elasticsearch/common/time/DateFormatters.java +++ b/server/src/main/java/org/elasticsearch/common/time/DateFormatters.java @@ -108,31 +108,31 @@ public class DateFormatters { private static final DateTimeFormatter STRICT_DATE_OPTIONAL_TIME_FORMATTER = new DateTimeFormatterBuilder() .append(STRICT_YEAR_MONTH_DAY_FORMATTER) .optionalStart() - .appendLiteral('T') - .optionalStart() - .appendValue(HOUR_OF_DAY, 2, 2, SignStyle.NOT_NEGATIVE) - .optionalStart() - .appendLiteral(':') - .appendValue(MINUTE_OF_HOUR, 2, 2, SignStyle.NOT_NEGATIVE) - .optionalStart() - .appendLiteral(':') - .appendValue(SECOND_OF_MINUTE, 2, 2, SignStyle.NOT_NEGATIVE) - .optionalStart() - .appendFraction(NANO_OF_SECOND, 1, 9, true) - .optionalEnd() - .optionalStart() - .appendLiteral(',') - .appendFraction(NANO_OF_SECOND, 1, 9, false) - .optionalEnd() - .optionalEnd() - .optionalStart() - .appendZoneOrOffsetId() - .optionalEnd() - .optionalStart() - .append(TIME_ZONE_FORMATTER_NO_COLON) - .optionalEnd() - .optionalEnd() - .optionalEnd() + .appendLiteral('T') + .optionalStart() + .appendValue(HOUR_OF_DAY, 2, 2, SignStyle.NOT_NEGATIVE) + .optionalStart() + .appendLiteral(':') + .appendValue(MINUTE_OF_HOUR, 2, 2, SignStyle.NOT_NEGATIVE) + .optionalStart() + .appendLiteral(':') + .appendValue(SECOND_OF_MINUTE, 2, 2, SignStyle.NOT_NEGATIVE) + .optionalStart() + .appendFraction(NANO_OF_SECOND, 1, 9, true) + .optionalEnd() + .optionalStart() + .appendLiteral(',') + .appendFraction(NANO_OF_SECOND, 1, 9, false) + .optionalEnd() + .optionalEnd() + .optionalEnd() + .optionalStart() + .appendZoneOrOffsetId() + .optionalEnd() + .optionalStart() + .append(TIME_ZONE_FORMATTER_NO_COLON) + .optionalEnd() + .optionalEnd() .optionalEnd() .toFormatter(Locale.ROOT) .withResolverStyle(ResolverStyle.STRICT); 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 49e069ceb6f5a..1aeae53ae43ba 100644 --- a/server/src/test/java/org/elasticsearch/common/joda/JavaJodaTimeDuellingTests.java +++ b/server/src/test/java/org/elasticsearch/common/joda/JavaJodaTimeDuellingTests.java @@ -24,12 +24,16 @@ import org.elasticsearch.common.time.DateFormatter; import org.elasticsearch.common.time.DateFormatters; import org.elasticsearch.common.time.DateMathParser; +import org.elasticsearch.common.util.LocaleUtils; import org.elasticsearch.test.ESTestCase; import org.joda.time.DateTime; import org.joda.time.DateTimeZone; import org.joda.time.format.DateTimeFormat; import org.joda.time.format.ISODateTimeFormat; +import org.junit.BeforeClass; +import java.time.Instant; +import java.time.LocalDateTime; import java.time.ZoneId; import java.time.ZoneOffset; import java.time.ZonedDateTime; @@ -46,9 +50,47 @@ public class JavaJodaTimeDuellingTests extends ESTestCase { protected boolean enableWarningsCheck() { return false; } + @BeforeClass + public static void checkJvmProperties(){ + assert ("SPI,COMPAT".equals(System.getProperty("java.locale.providers"))) + : "`-Djava.locale.providers=SPI,COMPAT` needs to be set"; + } + + public void testTimezoneParsing() { + /** this testcase won't work in joda. See comment in {@link #testPartialTimeParsing()} + * assertSameDateAs("2016-11-30T+01", "strict_date_optional_time", "strict_date_optional_time"); + */ + assertSameDateAs("2016-11-30T00+01", "strict_date_optional_time", "strict_date_optional_time"); + assertSameDateAs("2016-11-30T00+0100", "strict_date_optional_time", "strict_date_optional_time"); + assertSameDateAs("2016-11-30T00+01:00", "strict_date_optional_time", "strict_date_optional_time"); + } + + public void testPartialTimeParsing() { + /* + This does not work in Joda as it reports 2016-11-30T01:00:00Z + because StrictDateOptionalTime confuses +01 with an hour (which is a signed fixed length digit) + assertSameDateAs("2016-11-30T+01", "strict_date_optional_time", "strict_date_optional_time"); + ES java.time implementation does not suffer from this, + but we intentionally not allow parsing timezone without an time part as it is not allowed in iso8601 + */ + assertJavaTimeParseException("2016-11-30T+01","strict_date_optional_time"); + + assertSameDateAs("2016-11-30T12+01", "strict_date_optional_time", "strict_date_optional_time"); + assertSameDateAs("2016-11-30T12:00+01", "strict_date_optional_time", "strict_date_optional_time"); + assertSameDateAs("2016-11-30T12:00:00+01", "strict_date_optional_time", "strict_date_optional_time"); + assertSameDateAs("2016-11-30T12:00:00.000+01", "strict_date_optional_time", "strict_date_optional_time"); + + //without timezone + assertSameDateAs("2016-11-30T", "strict_date_optional_time", "strict_date_optional_time"); + assertSameDateAs("2016-11-30T12", "strict_date_optional_time", "strict_date_optional_time"); + assertSameDateAs("2016-11-30T12:00", "strict_date_optional_time", "strict_date_optional_time"); + assertSameDateAs("2016-11-30T12:00:00", "strict_date_optional_time", "strict_date_optional_time"); + assertSameDateAs("2016-11-30T12:00:00.000", "strict_date_optional_time", "strict_date_optional_time"); + } + // date_optional part of a parser names "strict_date_optional_time" or "date_optional"time // means that date part can be partially parsed. - public void testPartialParsing() { + public void testPartialDateParsing() { assertSameDateAs("2001", "strict_date_optional_time_nanos", "strict_date_optional_time"); assertSameDateAs("2001-01", "strict_date_optional_time_nanos", "strict_date_optional_time"); assertSameDateAs("2001-01-01", "strict_date_optional_time_nanos", "strict_date_optional_time"); @@ -98,6 +140,24 @@ private long dateMathToMillis(String text, DateFormatter dateFormatter) { return javaDateMath.parse(text, () -> 0, true, (ZoneId) null).toEpochMilli(); } + public void testDayOfWeek() { + //7 (ok joda) vs 1 (java by default) but 7 with customized org.elasticsearch.common.time.IsoLocale.ISO8601 + ZonedDateTime now = LocalDateTime.of(2009,11,15,1,32,8,328402) + .atZone(ZoneOffset.UTC); //Sunday + DateFormatter jodaFormatter = Joda.forPattern("e").withLocale(Locale.ROOT).withZone(ZoneOffset.UTC); + DateFormatter javaFormatter = DateFormatter.forPattern("8e").withZone(ZoneOffset.UTC); + assertThat(jodaFormatter.format(now), equalTo(javaFormatter.format(now))); + } + + public void testStartOfWeek() { + //2019-21 (ok joda) vs 2019-22 (java by default) but 2019-21 with customized org.elasticsearch.common.time.IsoLocale.ISO8601 + ZonedDateTime now = LocalDateTime.of(2019,5,26,1,32,8,328402) + .atZone(ZoneOffset.UTC); + DateFormatter jodaFormatter = Joda.forPattern("xxxx-ww").withLocale(Locale.ROOT).withZone(ZoneOffset.UTC); + DateFormatter javaFormatter = DateFormatter.forPattern("8YYYY-ww").withZone(ZoneOffset.UTC); + assertThat(jodaFormatter.format(now), equalTo(javaFormatter.format(now))); + } + //these parsers should allow both ',' and '.' as a decimal point public void testDecimalPointParsing(){ assertSameDate("2001-01-01T00:00:00.123Z", "strict_date_optional_time"); @@ -191,19 +251,17 @@ public void testCustomTimeFormats() { assertSameDate("Nov 24 01:29:01 -0800", "MMM dd HH:mm:ss Z"); } - // this test requires tests to run with -Djava.locale.providers=COMPAT in order to work -// public void testCustomLocales() { -// -// // also ensure that locale based dates are the same -// assertSameDate("Di., 05 Dez. 2000 02:55:00 -0800", "E, d MMM yyyy HH:mm:ss Z", LocaleUtils.parse("de")); -// assertSameDate("Mi., 06 Dez. 2000 02:55:00 -0800", "E, d MMM yyyy HH:mm:ss Z", LocaleUtils.parse("de")); -// assertSameDate("Do., 07 Dez. 2000 00:00:00 -0800", "E, d MMM yyyy HH:mm:ss Z", LocaleUtils.parse("de")); -// assertSameDate("Fr., 08 Dez. 2000 00:00:00 -0800", "E, d MMM yyyy HH:mm:ss Z", LocaleUtils.parse("de")); -// -// DateTime dateTimeNow = DateTime.now(DateTimeZone.UTC); -// ZonedDateTime javaTimeNow = Instant.ofEpochMilli(dateTimeNow.getMillis()).atZone(ZoneOffset.UTC); -// assertSamePrinterOutput("E, d MMM yyyy HH:mm:ss Z", LocaleUtils.parse("de"), javaTimeNow, dateTimeNow); -// } + public void testCustomLocales() { + // also ensure that locale based dates are the same + assertSameDate("Di, 05 Dez 2000 02:55:00 -0800", "E, d MMM yyyy HH:mm:ss Z", LocaleUtils.parse("de")); + assertSameDate("Mi, 06 Dez 2000 02:55:00 -0800", "E, d MMM yyyy HH:mm:ss Z", LocaleUtils.parse("de")); + assertSameDate("Do, 07 Dez 2000 00:00:00 -0800", "E, d MMM yyyy HH:mm:ss Z", LocaleUtils.parse("de")); + assertSameDate("Fr, 08 Dez 2000 00:00:00 -0800", "E, d MMM yyyy HH:mm:ss Z", LocaleUtils.parse("de")); + + DateTime dateTimeNow = DateTime.now(DateTimeZone.UTC); + ZonedDateTime javaTimeNow = Instant.ofEpochMilli(dateTimeNow.getMillis()).atZone(ZoneOffset.UTC); + assertSamePrinterOutput("E, d MMM yyyy HH:mm:ss Z", javaTimeNow, dateTimeNow, LocaleUtils.parse("de")); + } public void testDuellingFormatsValidParsing() { assertSameDate("1522332219", "epoch_second"); @@ -883,9 +941,28 @@ public void testParsingMissingTimezone() { } private void assertSamePrinterOutput(String format, ZonedDateTime javaDate, DateTime jodaDate) { + DateFormatter dateFormatter = DateFormatter.forPattern(format); + JodaDateFormatter jodaDateFormatter = Joda.forPattern(format); + + assertSamePrinterOutput(format, javaDate, jodaDate, dateFormatter, jodaDateFormatter); + } + + private void assertSamePrinterOutput(String format, ZonedDateTime javaDate, DateTime jodaDate, Locale locale) { + DateFormatter dateFormatter = DateFormatter.forPattern(format).withLocale(locale); + DateFormatter jodaDateFormatter = Joda.forPattern(format).withLocale(locale); + + assertSamePrinterOutput(format, javaDate, jodaDate, dateFormatter, jodaDateFormatter); + } + + private void assertSamePrinterOutput(String format, + ZonedDateTime javaDate, + DateTime jodaDate, + DateFormatter dateFormatter, + DateFormatter jodaDateFormatter) { + String javaTimeOut = dateFormatter.format(javaDate); + String jodaTimeOut = jodaDateFormatter.formatJoda(jodaDate); + assertThat(jodaDate.getMillis(), is(javaDate.toInstant().toEpochMilli())); - String javaTimeOut = DateFormatter.forPattern(format).format(javaDate); - String jodaTimeOut = Joda.forPattern(format).formatJoda(jodaDate); if (JavaVersion.current().getVersion().get(0) == 8 && javaTimeOut.endsWith(".0") && (format.equals("epoch_second") || format.equals("epoch_millis"))) { @@ -904,6 +981,12 @@ private void assertSameDate(String input, String format) { assertSameDate(input, format, jodaFormatter, javaFormatter); } + private void assertSameDate(String input, String format, Locale locale) { + DateFormatter jodaFormatter = Joda.forPattern(format).withLocale(locale); + DateFormatter javaFormatter = DateFormatter.forPattern(format).withLocale(locale); + assertSameDate(input, format, jodaFormatter, javaFormatter); + } + private void assertSameDate(String input, String format, DateFormatter jodaFormatter, DateFormatter javaFormatter) { DateTime jodaDateTime = jodaFormatter.parseJoda(input); From 092231cb144fe23224a9f01b2ceebb985260338f Mon Sep 17 00:00:00 2001 From: Przemyslaw Gomulka Date: Wed, 8 Jan 2020 17:25:20 +0100 Subject: [PATCH 2/3] joda tests do not require spi flag --- .../joda/JavaJodaTimeDuellingTests.java | 47 +++++-------------- 1 file changed, 13 insertions(+), 34 deletions(-) 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 1aeae53ae43ba..a3863faf8e58a 100644 --- a/server/src/test/java/org/elasticsearch/common/joda/JavaJodaTimeDuellingTests.java +++ b/server/src/test/java/org/elasticsearch/common/joda/JavaJodaTimeDuellingTests.java @@ -50,11 +50,6 @@ public class JavaJodaTimeDuellingTests extends ESTestCase { protected boolean enableWarningsCheck() { return false; } - @BeforeClass - public static void checkJvmProperties(){ - assert ("SPI,COMPAT".equals(System.getProperty("java.locale.providers"))) - : "`-Djava.locale.providers=SPI,COMPAT` needs to be set"; - } public void testTimezoneParsing() { /** this testcase won't work in joda. See comment in {@link #testPartialTimeParsing()} @@ -140,24 +135,6 @@ private long dateMathToMillis(String text, DateFormatter dateFormatter) { return javaDateMath.parse(text, () -> 0, true, (ZoneId) null).toEpochMilli(); } - public void testDayOfWeek() { - //7 (ok joda) vs 1 (java by default) but 7 with customized org.elasticsearch.common.time.IsoLocale.ISO8601 - ZonedDateTime now = LocalDateTime.of(2009,11,15,1,32,8,328402) - .atZone(ZoneOffset.UTC); //Sunday - DateFormatter jodaFormatter = Joda.forPattern("e").withLocale(Locale.ROOT).withZone(ZoneOffset.UTC); - DateFormatter javaFormatter = DateFormatter.forPattern("8e").withZone(ZoneOffset.UTC); - assertThat(jodaFormatter.format(now), equalTo(javaFormatter.format(now))); - } - - public void testStartOfWeek() { - //2019-21 (ok joda) vs 2019-22 (java by default) but 2019-21 with customized org.elasticsearch.common.time.IsoLocale.ISO8601 - ZonedDateTime now = LocalDateTime.of(2019,5,26,1,32,8,328402) - .atZone(ZoneOffset.UTC); - DateFormatter jodaFormatter = Joda.forPattern("xxxx-ww").withLocale(Locale.ROOT).withZone(ZoneOffset.UTC); - DateFormatter javaFormatter = DateFormatter.forPattern("8YYYY-ww").withZone(ZoneOffset.UTC); - assertThat(jodaFormatter.format(now), equalTo(javaFormatter.format(now))); - } - //these parsers should allow both ',' and '.' as a decimal point public void testDecimalPointParsing(){ assertSameDate("2001-01-01T00:00:00.123Z", "strict_date_optional_time"); @@ -251,17 +228,19 @@ public void testCustomTimeFormats() { assertSameDate("Nov 24 01:29:01 -0800", "MMM dd HH:mm:ss Z"); } - public void testCustomLocales() { - // also ensure that locale based dates are the same - assertSameDate("Di, 05 Dez 2000 02:55:00 -0800", "E, d MMM yyyy HH:mm:ss Z", LocaleUtils.parse("de")); - assertSameDate("Mi, 06 Dez 2000 02:55:00 -0800", "E, d MMM yyyy HH:mm:ss Z", LocaleUtils.parse("de")); - assertSameDate("Do, 07 Dez 2000 00:00:00 -0800", "E, d MMM yyyy HH:mm:ss Z", LocaleUtils.parse("de")); - assertSameDate("Fr, 08 Dez 2000 00:00:00 -0800", "E, d MMM yyyy HH:mm:ss Z", LocaleUtils.parse("de")); - - DateTime dateTimeNow = DateTime.now(DateTimeZone.UTC); - ZonedDateTime javaTimeNow = Instant.ofEpochMilli(dateTimeNow.getMillis()).atZone(ZoneOffset.UTC); - assertSamePrinterOutput("E, d MMM yyyy HH:mm:ss Z", javaTimeNow, dateTimeNow, LocaleUtils.parse("de")); - } + // this test requires tests to run with -Djava.locale.providers=COMPAT in order to work +// public void testCustomLocales() { +// +// // also ensure that locale based dates are the same +// assertSameDate("Di., 05 Dez. 2000 02:55:00 -0800", "E, d MMM yyyy HH:mm:ss Z", LocaleUtils.parse("de")); +// assertSameDate("Mi., 06 Dez. 2000 02:55:00 -0800", "E, d MMM yyyy HH:mm:ss Z", LocaleUtils.parse("de")); +// assertSameDate("Do., 07 Dez. 2000 00:00:00 -0800", "E, d MMM yyyy HH:mm:ss Z", LocaleUtils.parse("de")); +// assertSameDate("Fr., 08 Dez. 2000 00:00:00 -0800", "E, d MMM yyyy HH:mm:ss Z", LocaleUtils.parse("de")); +// +// DateTime dateTimeNow = DateTime.now(DateTimeZone.UTC); +// ZonedDateTime javaTimeNow = Instant.ofEpochMilli(dateTimeNow.getMillis()).atZone(ZoneOffset.UTC); +// assertSamePrinterOutput("E, d MMM yyyy HH:mm:ss Z", LocaleUtils.parse("de"), javaTimeNow, dateTimeNow); +// } public void testDuellingFormatsValidParsing() { assertSameDate("1522332219", "epoch_second"); From 3e8bfb78b03e3919b3af85b918b8817225d33e55 Mon Sep 17 00:00:00 2001 From: Przemyslaw Gomulka Date: Wed, 8 Jan 2020 19:10:47 +0100 Subject: [PATCH 3/3] imports --- .../elasticsearch/common/joda/JavaJodaTimeDuellingTests.java | 4 ---- 1 file changed, 4 deletions(-) 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 a3863faf8e58a..7e3ed3abcb30f 100644 --- a/server/src/test/java/org/elasticsearch/common/joda/JavaJodaTimeDuellingTests.java +++ b/server/src/test/java/org/elasticsearch/common/joda/JavaJodaTimeDuellingTests.java @@ -24,16 +24,12 @@ import org.elasticsearch.common.time.DateFormatter; import org.elasticsearch.common.time.DateFormatters; import org.elasticsearch.common.time.DateMathParser; -import org.elasticsearch.common.util.LocaleUtils; import org.elasticsearch.test.ESTestCase; import org.joda.time.DateTime; import org.joda.time.DateTimeZone; import org.joda.time.format.DateTimeFormat; import org.joda.time.format.ISODateTimeFormat; -import org.junit.BeforeClass; -import java.time.Instant; -import java.time.LocalDateTime; import java.time.ZoneId; import java.time.ZoneOffset; import java.time.ZonedDateTime;