Switch default time format for ingest from Joda to Java for v7 (#37934)

droberts195 · web-flow · commit 2f7776c8b78c · 2019-01-30T16:26:28.000Z
Date formats with and without the "8" prefix are now all treated
as Java time formats, so that ingest does the same as mappings
in this respect.
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
@@ -19,7 +19,6 @@
 
 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;
@@ -73,30 +72,22 @@ private long parseMillis(String date) {
     Java {
         @Override
         Function<String, DateTime> getFunction(String format, DateTimeZone timezone, Locale locale) {
-            // 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();
+
+            // support the 6.x BWC compatible way of parsing java 8 dates
             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);
+                format = format.substring(1);
             }
+
+            int year = LocalDate.now(ZoneOffset.UTC).getYear();
+            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);
+            };
         }
     };
 
diff --git a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/DateIndexNameProcessor.java b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/DateIndexNameProcessor.java
@@ -157,7 +157,7 @@ public DateIndexNameProcessor create(Map<String, Processor.Factory> registry, St
             }
             List<String> dateFormatStrings = ConfigurationUtils.readOptionalList(TYPE, tag, config, "date_formats");
             if (dateFormatStrings == null) {
-                dateFormatStrings = Collections.singletonList("yyyy-MM-dd'T'HH:mm:ss.SSSZ");
+                dateFormatStrings = Collections.singletonList("yyyy-MM-dd'T'HH:mm:ss.SSSXX");
             }
             List<Function<String, DateTime>> dateFormats = new ArrayList<>(dateFormatStrings.size());
             for (String format : dateFormatStrings) {
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
@@ -34,8 +34,8 @@
 
 public class DateIndexNameProcessorTests extends ESTestCase {
 
-    public void testJodaPattern() throws Exception {
-        Function<String, DateTime> function = DateFormat.Java.getFunction("yyyy-MM-dd'T'HH:mm:ss.SSSZ", DateTimeZone.UTC, Locale.ROOT);
+    public void testJavaPattern() throws Exception {
+        Function<String, DateTime> function = DateFormat.Java.getFunction("yyyy-MM-dd'T'HH:mm:ss.SSSXX", 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,
@@ -82,7 +82,7 @@ public void testUnix()throws Exception {
     public void testTemplatedFields() throws Exception {
         String indexNamePrefix = randomAlphaOfLength(10);
         String dateRounding = randomFrom("y", "M", "w", "d", "h", "m", "s");
-        String indexNameFormat = randomFrom("yyyy-MM-dd'T'HH:mm:ss.SSSZ", "yyyyMMdd", "MM/dd/yyyy");
+        String indexNameFormat = randomFrom("yyyy-MM-dd'T'HH:mm:ss.SSSXX", "yyyyMMdd", "MM/dd/yyyy");
         String date = Integer.toString(randomInt());
         Function<String, DateTime> dateTimeFunction = DateFormat.Unix.getFunction(null, DateTimeZone.UTC, 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
@@ -49,18 +49,19 @@ private TemplateScript.Factory templatize(ZoneId timezone) {
         String id = timezone.equals(ZoneOffset.UTC) ? "UTC" : timezone.getId();
         return new TestTemplateService.MockTemplateScript.Factory(id);
     }
-    public void testJodaPattern() {
+
+    public void testJavaPattern() {
         DateProcessor dateProcessor = new DateProcessor(randomAlphaOfLength(10),
             templatize(ZoneId.of("Europe/Amsterdam")), templatize(Locale.ENGLISH),
-                "date_as_string", Collections.singletonList("yyyy dd MM hh:mm:ss"), "date_as_date");
+                "date_as_string", Collections.singletonList("yyyy dd MM HH:mm:ss"), "date_as_date");
         Map<String, Object> document = new HashMap<>();
         document.put("date_as_string", "2010 12 06 11:05:15");
         IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document);
         dateProcessor.execute(ingestDocument);
         assertThat(ingestDocument.getFieldValue("date_as_date", String.class), equalTo("2010-06-12T11:05:15.000+02:00"));
     }
 
-    public void testJodaPatternMultipleFormats() {
+    public void testJavaPatternMultipleFormats() {
         List<String> matchFormats = new ArrayList<>();
         matchFormats.add("yyyy dd MM");
         matchFormats.add("dd/MM/yyyy");
@@ -98,7 +99,7 @@ public void testJodaPatternMultipleFormats() {
         }
     }
 
-    public void testInvalidJodaPattern() {
+    public void testInvalidJavaPattern() {
         try {
             DateProcessor processor = new DateProcessor(randomAlphaOfLength(10),
                 templatize(ZoneOffset.UTC), templatize(randomLocale(random())),
@@ -109,24 +110,22 @@ 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("Invalid format: [invalid pattern]: Illegal pattern component: i"));
+            assertThat(e.getCause().getMessage(), equalTo("Invalid format: [invalid pattern]: Unknown pattern letter: i"));
         }
     }
 
-    public void testJodaPatternLocale() {
-        //TODO investigate if this is a bug in Joda
-        assumeFalse("Can't run in a FIPS JVM, Joda parse date error", inFipsJvm());
-            DateProcessor dateProcessor = new DateProcessor(randomAlphaOfLength(10),
+    public void testJavaPatternLocale() {
+        DateProcessor dateProcessor = new DateProcessor(randomAlphaOfLength(10),
             templatize(ZoneId.of("Europe/Amsterdam")), templatize(Locale.ITALIAN),
-                "date_as_string", Collections.singletonList("yyyy dd MMM"), "date_as_date");
+                "date_as_string", Collections.singletonList("yyyy dd MMMM"), "date_as_date");
         Map<String, Object> document = new HashMap<>();
         document.put("date_as_string", "2010 12 giugno");
         IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document);
         dateProcessor.execute(ingestDocument);
         assertThat(ingestDocument.getFieldValue("date_as_date", String.class), equalTo("2010-06-12T00:00:00.000+02:00"));
     }
 
-    public void testJodaPatternDefaultYear() {
+    public void testJavaPatternDefaultYear() {
         String format = randomFrom("dd/MM", "8dd/MM");
         DateProcessor dateProcessor = new DateProcessor(randomAlphaOfLength(10),
             templatize(ZoneId.of("Europe/Amsterdam")), templatize(Locale.ENGLISH),
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
@@ -1,8 +1,8 @@
 ---
-"Test logging":
+"Test with date processor":
   - skip:
       version: " - 6.9.99"
-      reason: pre-7.0.0 will send no warnings
+      reason: pre-7.0.0 requires the 8 prefix for Java time formats, so would treat the format in this test as a Joda time format
       features: "warnings"
 
   - do:
@@ -33,7 +33,7 @@
                 "date" : {
                   "field" : "timestamp",
                   "target_field" : "timestamp",
-                  "formats" : ["dd/MMM/YYYY:HH:mm:ss Z"]
+                  "formats" : ["dd/MMM/yyyy:HH:mm:ss xx"]
                 }
               },
               {
@@ -46,8 +46,6 @@
   - 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

Original file line number	Diff line number	Diff line change
`@@ -157,7 +157,7 @@ public DateIndexNameProcessor create(Map<String, Processor.Factory> registry, St`
`157`	`157`	`}`
`158`	`158`	`List<String> dateFormatStrings = ConfigurationUtils.readOptionalList(TYPE, tag, config, "date_formats");`
`159`	`159`	`if (dateFormatStrings == null) {`
`160`		`- dateFormatStrings = Collections.singletonList("yyyy-MM-dd'T'HH:mm:ss.SSSZ");`
	`160`	`+ dateFormatStrings = Collections.singletonList("yyyy-MM-dd'T'HH:mm:ss.SSSXX");`
`161`	`161`	`}`
`162`	`162`	`List<Function<String, DateTime>> dateFormats = new ArrayList<>(dateFormatStrings.size());`
`163`	`163`	`for (String format : dateFormatStrings) {`