From 0a37b86cf2957e28144d841c27243048f6961a8b Mon Sep 17 00:00:00 2001 From: Alexander Reelsen Date: Mon, 14 Jan 2019 10:38:39 +0100 Subject: [PATCH 1/3] Add BWC compatible processing to ingest date processors The ingest date processor is currently only able to parse joda formats. However it is not using the existing elasticsearch classes but access joda directly. This means that our existing BWC layer does not notify the user about deprecated formats. This commit switches to use the exising Elasticsearch Joda methods to acquire a date format, that includes the BWC check and the ability to parse java 8 dates. The date parsing in ingest has also another extra feature, that the fallback year, when a date format without a year is used, is the current year, and not 1970 like usual. This is currently not properly supported in the DateFormatter class. As this is the only case for this feature and java time can take care of this using the toZonedDateTime() method, a workaround just for the joda time parser has been created, that can be removed soon again from 7.0. --- .../ingest/common/DateFormat.java | 41 ++++++++++++++++--- .../ingest/common/DateFormatTests.java | 12 +++--- .../common/DateIndexNameProcessorTests.java | 2 +- .../ingest/common/DateProcessorTests.java | 5 ++- .../common/joda/JodaDateFormatter.java | 32 +++++++++++---- 5 files changed, 70 insertions(+), 22 deletions(-) diff --git a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/DateFormat.java b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/DateFormat.java index bf664afb40777..220091c4baadc 100644 --- a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/DateFormat.java +++ b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/DateFormat.java @@ -19,12 +19,19 @@ package org.elasticsearch.ingest.common; +import org.elasticsearch.common.joda.Joda; +import org.elasticsearch.common.time.DateFormatter; +import org.elasticsearch.common.time.DateFormatters; +import org.elasticsearch.common.time.DateUtils; import org.joda.time.DateTime; import org.joda.time.DateTimeZone; -import org.joda.time.format.DateTimeFormat; -import org.joda.time.format.DateTimeFormatter; import org.joda.time.format.ISODateTimeFormat; +import java.time.Instant; +import java.time.LocalDate; +import java.time.ZoneOffset; +import java.time.ZonedDateTime; +import java.time.temporal.TemporalAccessor; import java.util.Locale; import java.util.function.Function; @@ -63,11 +70,33 @@ private long parseMillis(String date) { return ((base * 1000) - 10000) + (rest/1000000); } }, - Joda { + Java { @Override Function getFunction(String format, DateTimeZone timezone, Locale locale) { - DateTimeFormatter parser = DateTimeFormat.forPattern(format).withZone(timezone).withLocale(locale); - return text -> parser.withDefaultYear((new DateTime(DateTimeZone.UTC)).getYear()).parseDateTime(text); + // in case you are wondering why we do not call 'DateFormatter.forPattern(format)' for all cases here, but only for the + // non java time case: + // When the joda date formatter parses a date then a year is always set, so that no fallback can be used, like + // done in the JodaDateFormatter.withYear() code below + // This means that we leave the existing parsing logic in place, but will fall back to the new java date parsing logic, if an + // "8" is prepended to the date format string + int year = LocalDate.now(ZoneOffset.UTC).getYear(); + if (format.startsWith("8")) { + DateFormatter formatter = DateFormatter.forPattern(format) + .withLocale(locale) + .withZone(DateUtils.dateTimeZoneToZoneId(timezone)); + return text -> { + ZonedDateTime defaultZonedDateTime = Instant.EPOCH.atZone(ZoneOffset.UTC).withYear(year); + TemporalAccessor accessor = formatter.parse(text); + long millis = DateFormatters.toZonedDateTime(accessor, defaultZonedDateTime).toInstant().toEpochMilli(); + return new DateTime(millis, timezone); + }; + } else { + DateFormatter formatter = Joda.forPattern(format) + .withYear(year) + .withZone(DateUtils.dateTimeZoneToZoneId(timezone)) + .withLocale(locale); + return text -> new DateTime(formatter.parseMillis(text), timezone); + } } }; @@ -84,7 +113,7 @@ static DateFormat fromString(String format) { case "TAI64N": return Tai64n; default: - return Joda; + return Java; } } } diff --git a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/DateFormatTests.java b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/DateFormatTests.java index 415ee8720930b..27904a5586e7e 100644 --- a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/DateFormatTests.java +++ b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/DateFormatTests.java @@ -34,7 +34,7 @@ public class DateFormatTests extends ESTestCase { public void testParseJoda() { - Function jodaFunction = DateFormat.Joda.getFunction("MMM dd HH:mm:ss Z", + Function jodaFunction = DateFormat.Java.getFunction("MMM dd HH:mm:ss Z", DateTimeZone.forOffsetHours(-8), Locale.ENGLISH); assertThat(Instant.ofEpochMilli(jodaFunction.apply("Nov 24 01:29:01 -0800").getMillis()) .atZone(ZoneId.of("GMT-8")) @@ -78,13 +78,13 @@ public void testTAI64NParse() { public void testFromString() { assertThat(DateFormat.fromString("UNIX_MS"), equalTo(DateFormat.UnixMs)); - assertThat(DateFormat.fromString("unix_ms"), equalTo(DateFormat.Joda)); + assertThat(DateFormat.fromString("unix_ms"), equalTo(DateFormat.Java)); assertThat(DateFormat.fromString("UNIX"), equalTo(DateFormat.Unix)); - assertThat(DateFormat.fromString("unix"), equalTo(DateFormat.Joda)); + assertThat(DateFormat.fromString("unix"), equalTo(DateFormat.Java)); assertThat(DateFormat.fromString("ISO8601"), equalTo(DateFormat.Iso8601)); - assertThat(DateFormat.fromString("iso8601"), equalTo(DateFormat.Joda)); + assertThat(DateFormat.fromString("iso8601"), equalTo(DateFormat.Java)); assertThat(DateFormat.fromString("TAI64N"), equalTo(DateFormat.Tai64n)); - assertThat(DateFormat.fromString("tai64n"), equalTo(DateFormat.Joda)); - assertThat(DateFormat.fromString("prefix-" + randomAlphaOfLengthBetween(1, 10)), equalTo(DateFormat.Joda)); + assertThat(DateFormat.fromString("tai64n"), equalTo(DateFormat.Java)); + assertThat(DateFormat.fromString("prefix-" + randomAlphaOfLengthBetween(1, 10)), equalTo(DateFormat.Java)); } } diff --git a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/DateIndexNameProcessorTests.java b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/DateIndexNameProcessorTests.java index c97da116e3489..6555628f1da15 100644 --- a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/DateIndexNameProcessorTests.java +++ b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/DateIndexNameProcessorTests.java @@ -35,7 +35,7 @@ public class DateIndexNameProcessorTests extends ESTestCase { public void testJodaPattern() throws Exception { - Function function = DateFormat.Joda.getFunction("yyyy-MM-dd'T'HH:mm:ss.SSSZ", DateTimeZone.UTC, Locale.ROOT); + Function function = DateFormat.Java.getFunction("yyyy-MM-dd'T'HH:mm:ss.SSSZ", DateTimeZone.UTC, Locale.ROOT); DateIndexNameProcessor processor = createProcessor("_field", Collections.singletonList(function), DateTimeZone.UTC, "events-", "y", "yyyyMMdd"); IngestDocument document = new IngestDocument("_index", "_type", "_id", null, null, null, diff --git a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/DateProcessorTests.java b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/DateProcessorTests.java index 23aac797859e7..0f31143c43d0e 100644 --- a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/DateProcessorTests.java +++ b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/DateProcessorTests.java @@ -109,7 +109,7 @@ public void testInvalidJodaPattern() { fail("date processor execution should have failed"); } catch(IllegalArgumentException e) { assertThat(e.getMessage(), equalTo("unable to parse date [2010]")); - assertThat(e.getCause().getMessage(), equalTo("Illegal pattern component: i")); + assertThat(e.getCause().getMessage(), equalTo("Invalid format: [invalid pattern]: Illegal pattern component: i")); } } @@ -127,9 +127,10 @@ public void testJodaPatternLocale() { } public void testJodaPatternDefaultYear() { + String format = randomFrom("dd/MM", "8dd/MM"); DateProcessor dateProcessor = new DateProcessor(randomAlphaOfLength(10), templatize(ZoneId.of("Europe/Amsterdam")), templatize(Locale.ENGLISH), - "date_as_string", Collections.singletonList("dd/MM"), "date_as_date"); + "date_as_string", Collections.singletonList(format), "date_as_date"); Map document = new HashMap<>(); document.put("date_as_string", "12/06"); IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document); diff --git a/server/src/main/java/org/elasticsearch/common/joda/JodaDateFormatter.java b/server/src/main/java/org/elasticsearch/common/joda/JodaDateFormatter.java index 5db95b12bb437..b7eaeb614bc9a 100644 --- a/server/src/main/java/org/elasticsearch/common/joda/JodaDateFormatter.java +++ b/server/src/main/java/org/elasticsearch/common/joda/JodaDateFormatter.java @@ -33,16 +33,21 @@ import java.util.Locale; public class JodaDateFormatter implements DateFormatter { - final String pattern; + final String pattern; + private final int year; final DateTimeFormatter parser; - final DateTimeFormatter printer; - public JodaDateFormatter(String pattern, DateTimeFormatter parser, DateTimeFormatter printer) { + JodaDateFormatter(String pattern, DateTimeFormatter parser, DateTimeFormatter printer) { + this(pattern, parser, printer, 1970); + } + + private JodaDateFormatter(String pattern, DateTimeFormatter parser, DateTimeFormatter printer, int year) { this.pattern = pattern; - this.printer = printer.withDefaultYear(1970); - this.parser = parser.withDefaultYear(1970); + this.year = year; + this.printer = printer.withDefaultYear(year); + this.parser = parser.withDefaultYear(year); } @Override @@ -62,16 +67,22 @@ public DateTime parseJoda(String input) { @Override public DateFormatter withZone(ZoneId zoneId) { DateTimeZone timeZone = DateUtils.zoneIdToDateTimeZone(zoneId); + if (parser.getZone().equals(timeZone)) { + return this; + } DateTimeFormatter parser = this.parser.withZone(timeZone); DateTimeFormatter printer = this.printer.withZone(timeZone); - return new JodaDateFormatter(pattern, parser, printer); + return new JodaDateFormatter(pattern, parser, printer, year); } @Override public DateFormatter withLocale(Locale locale) { + if (parser.getLocale().equals(locale)) { + return this; + } DateTimeFormatter parser = this.parser.withLocale(locale); DateTimeFormatter printer = this.printer.withLocale(locale); - return new JodaDateFormatter(pattern, parser, printer); + return new JodaDateFormatter(pattern, parser, printer, year); } @Override @@ -89,6 +100,13 @@ public String formatMillis(long millis) { return printer.print(millis); } + public JodaDateFormatter withYear(int year) { + if (this.year == year) { + return this; + } + return new JodaDateFormatter(pattern, parser, printer, year); + } + @Override public String pattern() { return pattern; From 875b07ba0a2098c25f6c6598d682b7de2f85bf71 Mon Sep 17 00:00:00 2001 From: Alexander Reelsen Date: Mon, 14 Jan 2019 13:16:24 +0100 Subject: [PATCH 2/3] fix test --- .../rest-api-spec/test/ingest/20_combine_processors.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/qa/smoke-test-ingest-with-all-dependencies/src/test/resources/rest-api-spec/test/ingest/20_combine_processors.yml b/qa/smoke-test-ingest-with-all-dependencies/src/test/resources/rest-api-spec/test/ingest/20_combine_processors.yml index 85fa6db10ed17..f44e6ae5753a0 100644 --- a/qa/smoke-test-ingest-with-all-dependencies/src/test/resources/rest-api-spec/test/ingest/20_combine_processors.yml +++ b/qa/smoke-test-ingest-with-all-dependencies/src/test/resources/rest-api-spec/test/ingest/20_combine_processors.yml @@ -1,5 +1,10 @@ --- "Test logging": + - skip: + version: " - 6.9.99" + reason: pre-7.0.0 will send no warnings + features: "warnings" + - do: ingest.put_pipeline: id: "_id" @@ -41,6 +46,8 @@ - match: { acknowledged: true } - do: + warnings: + - "Use of 'Y' (year-of-era) will change to 'y' in the next major version of Elasticsearch. Prefix your date format with '8' to use the new specifier." index: index: test type: test From 8e30c04e30cd8192eec11ed2690a3f255328e109 Mon Sep 17 00:00:00 2001 From: Alexander Reelsen Date: Wed, 23 Jan 2019 00:39:18 +0100 Subject: [PATCH 3/3] incorporate review comment --- .../org/elasticsearch/common/joda/Joda.java | 13 +++++++------ .../common/joda/JodaDateFormatter.java | 18 ++++++------------ 2 files changed, 13 insertions(+), 18 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/joda/Joda.java b/server/src/main/java/org/elasticsearch/common/joda/Joda.java index 3c99b65a9a54f..45587f6bb3df9 100644 --- a/server/src/main/java/org/elasticsearch/common/joda/Joda.java +++ b/server/src/main/java/org/elasticsearch/common/joda/Joda.java @@ -107,8 +107,8 @@ public static JodaDateFormatter forPattern(String input) { // in this case, we have a separate parser and printer since the dataOptionalTimeParser can't print // this sucks we should use the root local by default and not be dependent on the node return new JodaDateFormatter(input, - ISODateTimeFormat.dateOptionalTimeParser().withLocale(Locale.ROOT).withZone(DateTimeZone.UTC), - ISODateTimeFormat.dateTime().withLocale(Locale.ROOT).withZone(DateTimeZone.UTC)); + ISODateTimeFormat.dateOptionalTimeParser().withLocale(Locale.ROOT).withZone(DateTimeZone.UTC).withDefaultYear(1970), + ISODateTimeFormat.dateTime().withLocale(Locale.ROOT).withZone(DateTimeZone.UTC).withDefaultYear(1970)); } else if ("dateTime".equals(input) || "date_time".equals(input)) { formatter = ISODateTimeFormat.dateTime(); } else if ("dateTimeNoMillis".equals(input) || "date_time_no_millis".equals(input)) { @@ -184,8 +184,9 @@ public static JodaDateFormatter forPattern(String input) { // in this case, we have a separate parser and printer since the dataOptionalTimeParser can't print // this sucks we should use the root local by default and not be dependent on the node return new JodaDateFormatter(input, - StrictISODateTimeFormat.dateOptionalTimeParser().withLocale(Locale.ROOT).withZone(DateTimeZone.UTC), - StrictISODateTimeFormat.dateTime().withLocale(Locale.ROOT).withZone(DateTimeZone.UTC)); + StrictISODateTimeFormat.dateOptionalTimeParser().withLocale(Locale.ROOT).withZone(DateTimeZone.UTC) + .withDefaultYear(1970), + StrictISODateTimeFormat.dateTime().withLocale(Locale.ROOT).withZone(DateTimeZone.UTC).withDefaultYear(1970)); } else if ("strictDateTime".equals(input) || "strict_date_time".equals(input)) { formatter = StrictISODateTimeFormat.dateTime(); } else if ("strictDateTimeNoMillis".equals(input) || "strict_date_time_no_millis".equals(input)) { @@ -262,7 +263,7 @@ public static JodaDateFormatter forPattern(String input) { } } - formatter = formatter.withLocale(Locale.ROOT).withZone(DateTimeZone.UTC); + formatter = formatter.withLocale(Locale.ROOT).withZone(DateTimeZone.UTC).withDefaultYear(1970); return new JodaDateFormatter(input, formatter, formatter); } @@ -311,7 +312,7 @@ public static DateFormatter getStrictStandardDateFormatter() { DateTimeFormatterBuilder builder = new DateTimeFormatterBuilder().append(longFormatter.withZone(DateTimeZone.UTC).getPrinter(), new DateTimeParser[]{longFormatter.getParser(), shortFormatter.getParser(), new EpochTimeParser(true)}); - DateTimeFormatter formatter = builder.toFormatter().withLocale(Locale.ROOT).withZone(DateTimeZone.UTC); + DateTimeFormatter formatter = builder.toFormatter().withLocale(Locale.ROOT).withZone(DateTimeZone.UTC).withDefaultYear(1970); return new JodaDateFormatter("yyyy/MM/dd HH:mm:ss||yyyy/MM/dd||epoch_millis", formatter, formatter); } diff --git a/server/src/main/java/org/elasticsearch/common/joda/JodaDateFormatter.java b/server/src/main/java/org/elasticsearch/common/joda/JodaDateFormatter.java index b7eaeb614bc9a..0c24cc3f9da02 100644 --- a/server/src/main/java/org/elasticsearch/common/joda/JodaDateFormatter.java +++ b/server/src/main/java/org/elasticsearch/common/joda/JodaDateFormatter.java @@ -35,19 +35,13 @@ public class JodaDateFormatter implements DateFormatter { final String pattern; - private final int year; final DateTimeFormatter parser; final DateTimeFormatter printer; JodaDateFormatter(String pattern, DateTimeFormatter parser, DateTimeFormatter printer) { - this(pattern, parser, printer, 1970); - } - - private JodaDateFormatter(String pattern, DateTimeFormatter parser, DateTimeFormatter printer, int year) { this.pattern = pattern; - this.year = year; - this.printer = printer.withDefaultYear(year); - this.parser = parser.withDefaultYear(year); + this.printer = printer; + this.parser = parser; } @Override @@ -72,7 +66,7 @@ public DateFormatter withZone(ZoneId zoneId) { } DateTimeFormatter parser = this.parser.withZone(timeZone); DateTimeFormatter printer = this.printer.withZone(timeZone); - return new JodaDateFormatter(pattern, parser, printer, year); + return new JodaDateFormatter(pattern, parser, printer); } @Override @@ -82,7 +76,7 @@ public DateFormatter withLocale(Locale locale) { } DateTimeFormatter parser = this.parser.withLocale(locale); DateTimeFormatter printer = this.printer.withLocale(locale); - return new JodaDateFormatter(pattern, parser, printer, year); + return new JodaDateFormatter(pattern, parser, printer); } @Override @@ -101,10 +95,10 @@ public String formatMillis(long millis) { } public JodaDateFormatter withYear(int year) { - if (this.year == year) { + if (parser.getDefaultYear() == year) { return this; } - return new JodaDateFormatter(pattern, parser, printer, year); + return new JodaDateFormatter(pattern, parser.withDefaultYear(year), printer.withDefaultYear(year)); } @Override