From 001daddd21900dc5c4f4693eb007736a735f1de0 Mon Sep 17 00:00:00 2001 From: Przemyslaw Gomulka Date: Thu, 12 Dec 2019 14:40:07 +0100 Subject: [PATCH 1/5] draft --- .../common/time/DateFormatters.java | 50 +++++------ .../joda/JavaJodaTimeDuellingTests.java | 84 +++++++++++++++---- 2 files changed, 94 insertions(+), 40 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 d09493849ce85..c3f51f483a044 100644 --- a/server/src/main/java/org/elasticsearch/common/time/DateFormatters.java +++ b/server/src/main/java/org/elasticsearch/common/time/DateFormatters.java @@ -107,31 +107,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() + .optionalEnd() + .optionalStart() + .appendZoneOrOffsetId() + .optionalEnd() + .optionalStart() + .append(TIME_ZONE_FORMATTER_NO_COLON) + .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 1ead658ba7165..d9099d87035cd 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,15 @@ 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.ISODateTimeFormat; +import org.junit.BeforeClass; import java.time.LocalDateTime; +import java.time.Instant; import java.time.ZoneId; import java.time.ZoneOffset; import java.time.ZonedDateTime; @@ -46,9 +49,40 @@ 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() { +// //with Timezone +// assertSameDateAs("2016-11-30T+01", "strict_date_optional_time", "strict_date_optional_time"); +// assertSameDateAs("2016-11-30T+01", "strict_date_optional_time", "strict_date_optional_time"); +// assertSameDateAs("2016-11-30T+01", "strict_date_optional_time", "strict_date_optional_time"); +// assertSameDateAs("2016-11-30T+01", "strict_date_optional_time", "strict_date_optional_time"); +// assertSameDateAs("2016-11-30T+01", "strict_date_optional_time", "strict_date_optional_time"); +// +// } + + public void testPartialTimeParsing() { + //with Timezone + assertSameDateAs("2016-11-30T+01", "strict_date_optional_time", "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"); @@ -167,19 +201,18 @@ 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")); + public void testCustomLocales() { // -// 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); -// } + // 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"); @@ -859,9 +892,24 @@ 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"))) { @@ -880,6 +928,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 610e7d663e54bd092158379db1834d337ef41404 Mon Sep 17 00:00:00 2001 From: Przemyslaw Gomulka Date: Fri, 13 Dec 2019 14:38:38 +0100 Subject: [PATCH 2/5] more tests --- .../joda/JavaJodaTimeDuellingTests.java | 37 +++++++++++-------- 1 file changed, 21 insertions(+), 16 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 d9099d87035cd..2e633735b3cee 100644 --- a/server/src/test/java/org/elasticsearch/common/joda/JavaJodaTimeDuellingTests.java +++ b/server/src/test/java/org/elasticsearch/common/joda/JavaJodaTimeDuellingTests.java @@ -55,19 +55,25 @@ public static void checkJvmProperties(){ : "`-Djava.locale.providers=SPI,COMPAT` needs to be set"; } -// public void testTimezoneParsing() { -// //with Timezone -// assertSameDateAs("2016-11-30T+01", "strict_date_optional_time", "strict_date_optional_time"); -// assertSameDateAs("2016-11-30T+01", "strict_date_optional_time", "strict_date_optional_time"); -// assertSameDateAs("2016-11-30T+01", "strict_date_optional_time", "strict_date_optional_time"); -// assertSameDateAs("2016-11-30T+01", "strict_date_optional_time", "strict_date_optional_time"); -// assertSameDateAs("2016-11-30T+01", "strict_date_optional_time", "strict_date_optional_time"); -// -// } + public void testTimezoneParsing() { + //with Timezone +// 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() { - //with Timezone - assertSameDateAs("2016-11-30T+01", "strict_date_optional_time", "strict_date_optional_time"); + + // 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"); + // However java.time implementation does not suffer from this + ZonedDateTime zonedDateTime = DateFormatters.from(DateFormatter.forPattern("strict_date_optional_time") + .parse("2016-11-30T+01")); + assertEquals(ZonedDateTime.of(2016,11,29,23,00,00,00,ZoneOffset.UTC), + zonedDateTime.withZoneSameInstant(ZoneOffset.UTC)); + 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"); @@ -202,12 +208,11 @@ public void testCustomTimeFormats() { } 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")); + 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); From 2fa45990316435de94ccc72cf5a5ad0e2600492a Mon Sep 17 00:00:00 2001 From: Przemyslaw Gomulka Date: Mon, 16 Dec 2019 09:17:30 +0100 Subject: [PATCH 3/5] checkstyle --- .../common/joda/JavaJodaTimeDuellingTests.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) 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 2e633735b3cee..27fe12d59b658 100644 --- a/server/src/test/java/org/elasticsearch/common/joda/JavaJodaTimeDuellingTests.java +++ b/server/src/test/java/org/elasticsearch/common/joda/JavaJodaTimeDuellingTests.java @@ -910,7 +910,11 @@ private void assertSamePrinterOutput(String format, ZonedDateTime javaDate, Date assertSamePrinterOutput(format, javaDate, jodaDate, dateFormatter, jodaDateFormatter); } - private void assertSamePrinterOutput(String format, ZonedDateTime javaDate, DateTime jodaDate, DateFormatter dateFormatter, 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); From 3ae4b130ff48b68fcb154ad3a55d32142416d9c4 Mon Sep 17 00:00:00 2001 From: Przemyslaw Gomulka Date: Thu, 19 Dec 2019 08:49:18 +0100 Subject: [PATCH 4/5] javadoc --- .../common/joda/JavaJodaTimeDuellingTests.java | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 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 27fe12d59b658..0cca3017f80d0 100644 --- a/server/src/test/java/org/elasticsearch/common/joda/JavaJodaTimeDuellingTests.java +++ b/server/src/test/java/org/elasticsearch/common/joda/JavaJodaTimeDuellingTests.java @@ -31,8 +31,8 @@ import org.joda.time.format.ISODateTimeFormat; import org.junit.BeforeClass; -import java.time.LocalDateTime; import java.time.Instant; +import java.time.LocalDateTime; import java.time.ZoneId; import java.time.ZoneOffset; import java.time.ZonedDateTime; @@ -56,19 +56,21 @@ public static void checkJvmProperties(){ } public void testTimezoneParsing() { - //with Timezone -// assertSameDateAs("2016-11-30T+01", "strict_date_optional_time", "strict_date_optional_time"); + /** 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"); - // However java.time implementation does not suffer from this + /* + 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"); + However ES java.time implementation does not suffer from this + */ ZonedDateTime zonedDateTime = DateFormatters.from(DateFormatter.forPattern("strict_date_optional_time") .parse("2016-11-30T+01")); assertEquals(ZonedDateTime.of(2016,11,29,23,00,00,00,ZoneOffset.UTC), From 856209de5a3e4614a846a93c6af7baaa82052b10 Mon Sep 17 00:00:00 2001 From: Przemyslaw Gomulka Date: Fri, 3 Jan 2020 09:47:45 +0100 Subject: [PATCH 5/5] do not allow parsing timezone without at least an hour --- .../elasticsearch/common/time/DateFormatters.java | 12 ++++++------ .../common/joda/JavaJodaTimeDuellingTests.java | 11 +++++------ 2 files changed, 11 insertions(+), 12 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 c3f51f483a044..ac190241b0c9a 100644 --- a/server/src/main/java/org/elasticsearch/common/time/DateFormatters.java +++ b/server/src/main/java/org/elasticsearch/common/time/DateFormatters.java @@ -125,12 +125,12 @@ public class DateFormatters { .optionalEnd() .optionalEnd() .optionalEnd() - .optionalEnd() - .optionalStart() - .appendZoneOrOffsetId() - .optionalEnd() - .optionalStart() - .append(TIME_ZONE_FORMATTER_NO_COLON) + .optionalStart() + .appendZoneOrOffsetId() + .optionalEnd() + .optionalStart() + .append(TIME_ZONE_FORMATTER_NO_COLON) + .optionalEnd() .optionalEnd() .optionalEnd() .toFormatter(Locale.ROOT) 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 0cca3017f80d0..dc8e80660ce97 100644 --- a/server/src/test/java/org/elasticsearch/common/joda/JavaJodaTimeDuellingTests.java +++ b/server/src/test/java/org/elasticsearch/common/joda/JavaJodaTimeDuellingTests.java @@ -69,12 +69,10 @@ 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"); - However ES java.time implementation does not suffer from this - */ - ZonedDateTime zonedDateTime = DateFormatters.from(DateFormatter.forPattern("strict_date_optional_time") - .parse("2016-11-30T+01")); - assertEquals(ZonedDateTime.of(2016,11,29,23,00,00,00,ZoneOffset.UTC), - zonedDateTime.withZoneSameInstant(ZoneOffset.UTC)); + 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"); @@ -88,6 +86,7 @@ public void testPartialTimeParsing() { 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 testPartialDateParsing() {