Always use CLDR locale on ES v9 (#113184)

thecoop · web-flow · commit f9aa6f40cd84 · 2024-09-23T11:05:08.000+01:00
Regardless of JDK version, ES should always use CLDR locale database from 9.0.0.
This also removes IsoCalendarDataProvider used to override week-date calculations for the root locale only.
diff --git a/build-tools-internal/src/main/groovy/elasticsearch.ide.gradle b/build-tools-internal/src/main/groovy/elasticsearch.ide.gradle
@@ -168,7 +168,7 @@ if (providers.systemProperty('idea.active').getOrNull() == 'true') {
             vmParameters = [
               '-ea',
               '-Djava.security.manager=allow',
-              '-Djava.locale.providers=SPI,CLDR',
+              '-Djava.locale.providers=CLDR',
               '-Des.nativelibs.path="' + testLibraryPath + '"',
               // TODO: only open these for mockito when it is modularized
               '--add-opens=java.base/java.security.cert=ALL-UNNAMED',
diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/ElasticsearchTestBasePlugin.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/ElasticsearchTestBasePlugin.java
@@ -93,7 +93,7 @@ public void execute(Task t) {
                     mkdirs(test.getWorkingDir().toPath().resolve("temp").toFile());
 
                     // TODO remove once jvm.options are added to test system properties
-                    test.systemProperty("java.locale.providers", "SPI,CLDR");
+                    test.systemProperty("java.locale.providers", "CLDR");
                 }
             });
             test.getJvmArgumentProviders().add(nonInputProperties);
diff --git a/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/SystemJvmOptions.java b/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/SystemJvmOptions.java
@@ -11,7 +11,6 @@
 
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.util.concurrent.EsExecutors;
-import org.elasticsearch.core.UpdateForV9;
 
 import java.util.List;
 import java.util.Map;
@@ -61,7 +60,7 @@ static List<String> systemJvmOptions(Settings nodeSettings, final Map<String, St
                 "-Dlog4j.shutdownHookEnabled=false",
                 "-Dlog4j2.disable.jmx=true",
                 "-Dlog4j2.formatMsgNoLookups=true",
-                "-Djava.locale.providers=" + getLocaleProviders(),
+                "-Djava.locale.providers=CLDR",
                 maybeEnableNativeAccess(),
                 maybeOverrideDockerCgroup(distroType),
                 maybeSetActiveProcessorCount(nodeSettings),
@@ -73,16 +72,6 @@ static List<String> systemJvmOptions(Settings nodeSettings, final Map<String, St
         ).filter(e -> e.isEmpty() == false).collect(Collectors.toList());
     }
 
-    @UpdateForV9    // only use CLDR in v9+
-    private static String getLocaleProviders() {
-        /*
-         * Specify SPI to load IsoCalendarDataProvider (see #48209), specifying the first day of week as Monday.
-         * When on pre-23, use COMPAT instead to maintain existing date formats as much as we can.
-         * When on JDK 23+, use the default CLDR locale database, as COMPAT was removed in JDK 23.
-         */
-        return Runtime.version().feature() >= 23 ? "SPI,CLDR" : "SPI,COMPAT";
-    }
-
     /*
      * The virtual file /proc/self/cgroup should list the current cgroup
      * membership. For each hierarchy, you can follow the cgroup path from
diff --git a/modules/aggregations/build.gradle b/modules/aggregations/build.gradle
@@ -45,3 +45,7 @@ dependencies {
   compileOnly(project(':modules:lang-painless:spi'))
   clusterModules(project(':modules:lang-painless'))
 }
+
+tasks.named("yamlRestCompatTestTransform").configure({ task ->
+  task.skipTest("aggregations/date_agg_per_day_of_week/Date aggregartion per day of week", "week-date behaviour has changed")
+})
diff --git a/modules/aggregations/src/yamlRestTest/resources/rest-api-spec/test/aggregations/date_agg_per_day_of_week.yml b/modules/aggregations/src/yamlRestTest/resources/rest-api-spec/test/aggregations/date_agg_per_day_of_week.yml
diff --git a/modules/ingest-common/build.gradle b/modules/ingest-common/build.gradle
@@ -50,3 +50,7 @@ tasks.named("thirdPartyAudit").configure {
     'org.apache.commons.logging.LogFactory',
   )
 }
+
+tasks.named("yamlRestCompatTestTransform").configure({ task ->
+  task.skipTest("ingest/30_date_processor/Test week based date parsing", "week-date behaviour has changed")
+})
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
@@ -75,17 +75,17 @@ public void testParseJavaDefaultYear() {
     public void testParseWeekBasedYearAndWeek() {
         String format = "YYYY-ww";
         ZoneId timezone = DateUtils.of("Europe/Amsterdam");
-        Function<String, ZonedDateTime> javaFunction = DateFormat.Java.getFunction(format, timezone, Locale.ROOT);
+        Function<String, ZonedDateTime> javaFunction = DateFormat.Java.getFunction(format, timezone, Locale.ENGLISH);
         ZonedDateTime dateTime = javaFunction.apply("2020-33");
-        assertThat(dateTime, equalTo(ZonedDateTime.of(2020, 8, 10, 0, 0, 0, 0, timezone)));
+        assertThat(dateTime, equalTo(ZonedDateTime.of(2020, 8, 9, 0, 0, 0, 0, timezone)));
     }
 
     public void testParseWeekBasedYear() {
         String format = "YYYY";
         ZoneId timezone = DateUtils.of("Europe/Amsterdam");
-        Function<String, ZonedDateTime> javaFunction = DateFormat.Java.getFunction(format, timezone, Locale.ROOT);
+        Function<String, ZonedDateTime> javaFunction = DateFormat.Java.getFunction(format, timezone, Locale.ENGLISH);
         ZonedDateTime dateTime = javaFunction.apply("2019");
-        assertThat(dateTime, equalTo(ZonedDateTime.of(2018, 12, 31, 0, 0, 0, 0, timezone)));
+        assertThat(dateTime, equalTo(ZonedDateTime.of(2018, 12, 30, 0, 0, 0, 0, timezone)));
     }
 
     public void testParseWeekBasedWithLocale() {
diff --git a/modules/ingest-common/src/yamlRestTest/resources/rest-api-spec/test/ingest/30_date_processor.yml b/modules/ingest-common/src/yamlRestTest/resources/rest-api-spec/test/ingest/30_date_processor.yml
@@ -246,7 +246,7 @@ teardown:
           }
   - length: { docs: 1 }
   - match: { docs.0.doc._source.date_source_field: "2020-33" }
-  - match: { docs.0.doc._source.date_target_field: "2020-08-10T00:00:00.000Z" }
+  - match: { docs.0.doc._source.date_target_field: "2020-08-09T00:00:00.000Z" }
   - length: { docs.0.doc._ingest: 1 }
   - is_true: docs.0.doc._ingest.timestamp
 
@@ -262,7 +262,7 @@ teardown:
         index: test
         id: "1"
   - match: { _source.date_source_field: "2020-33" }
-  - match: { _source.date_target_field: "2020-08-10T00:00:00.000Z" }
+  - match: { _source.date_target_field: "2020-08-09T00:00:00.000Z" }
 
 ---
 "Test week based date parsing with locale":
diff --git a/server/src/main/java/module-info.java b/server/src/main/java/module-info.java
@@ -404,7 +404,6 @@
     exports org.elasticsearch.telemetry;
     exports org.elasticsearch.telemetry.metric;
 
-    provides java.util.spi.CalendarDataProvider with org.elasticsearch.common.time.IsoCalendarDataProvider;
     provides org.elasticsearch.xcontent.ErrorOnUnknown with org.elasticsearch.common.xcontent.SuggestingErrorOnUnknown;
     provides org.elasticsearch.xcontent.XContentBuilderExtension with org.elasticsearch.common.xcontent.XContentElasticsearchExtension;
     provides org.elasticsearch.cli.CliToolProvider
diff --git a/server/src/main/java/org/elasticsearch/common/time/DateFormatters.java b/server/src/main/java/org/elasticsearch/common/time/DateFormatters.java
@@ -2324,8 +2324,6 @@ static DateFormatter forPattern(String input) {
         } else if (FormatNames.STRICT_YEAR_MONTH_DAY.matches(input)) {
             return STRICT_YEAR_MONTH_DAY;
         } else {
-            DateUtils.checkTextualDateFormats(input);
-
             try {
                 return newDateFormatter(
                     input,
diff --git a/server/src/main/java/org/elasticsearch/common/time/DateUtils.java b/server/src/main/java/org/elasticsearch/common/time/DateUtils.java
@@ -11,9 +11,6 @@
 
 import org.elasticsearch.common.logging.DeprecationCategory;
 import org.elasticsearch.common.logging.DeprecationLogger;
-import org.elasticsearch.core.Predicates;
-import org.elasticsearch.core.UpdateForV9;
-import org.elasticsearch.logging.LogManager;
 
 import java.time.Clock;
 import java.time.Duration;
@@ -23,8 +20,6 @@
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
-import java.util.function.Predicate;
-import java.util.regex.Pattern;
 
 import static java.util.Map.entry;
 import static org.elasticsearch.common.time.DateUtilsRounding.getMonthOfYear;
@@ -388,16 +383,4 @@ public static ZonedDateTime nowWithMillisResolution(Clock clock) {
         Clock millisResolutionClock = Clock.tick(clock, Duration.ofMillis(1));
         return ZonedDateTime.now(millisResolutionClock);
     }
-
-    // check for all textual fields, and localized zone offset
-    private static final Predicate<String> CONTAINS_CHANGING_TEXT_SPECIFIERS = System.getProperty("java.locale.providers", "")
-        .contains("COMPAT") ? Pattern.compile("[EcGaO]|MMM|LLL|eee|ccc|QQQ|ZZZZ").asPredicate() : Predicates.never();
-
-    @UpdateForV9    // this can be removed, we will only use CLDR on v9
-    static void checkTextualDateFormats(String format) {
-        if (CONTAINS_CHANGING_TEXT_SPECIFIERS.test(format)) {
-            LogManager.getLogger(DateFormatter.class)
-                .warn("Date format [{}] contains textual field specifiers that could change in JDK 23", format);
-        }
-    }
 }
diff --git a/server/src/main/java/org/elasticsearch/common/time/IsoCalendarDataProvider.java b/server/src/main/java/org/elasticsearch/common/time/IsoCalendarDataProvider.java
diff --git a/server/src/main/resources/META-INF/services/java.util.spi.CalendarDataProvider b/server/src/main/resources/META-INF/services/java.util.spi.CalendarDataProvider
diff --git a/server/src/test/java/org/elasticsearch/common/time/DateFormattersTests.java b/server/src/test/java/org/elasticsearch/common/time/DateFormattersTests.java
@@ -25,6 +25,7 @@
 import java.time.format.DateTimeFormatter;
 import java.time.format.DateTimeParseException;
 import java.time.temporal.ChronoField;
+import java.time.temporal.ChronoUnit;
 import java.time.temporal.TemporalAccessor;
 import java.util.List;
 import java.util.Locale;
@@ -85,38 +86,32 @@ private void assertDateMathEquals(String text, String expected, String pattern)
     }
 
     private void assertDateMathEquals(String text, String expected, String pattern, Locale locale) {
-        long gotMillisJava = dateMathToMillis(text, DateFormatter.forPattern(pattern), locale);
-        long expectedMillis = DateFormatters.from(DateFormatter.forPattern("strict_date_optional_time").withLocale(locale).parse(expected))
-            .toInstant()
-            .toEpochMilli();
+        Instant gotInstant = dateMathToInstant(text, DateFormatter.forPattern(pattern), locale).truncatedTo(ChronoUnit.MILLIS);
+        Instant expectedInstant = DateFormatters.from(
+            DateFormatter.forPattern("strict_date_optional_time").withLocale(locale).parse(expected)
+        ).toInstant().truncatedTo(ChronoUnit.MILLIS);
 
-        assertThat(gotMillisJava, equalTo(expectedMillis));
+        assertThat(gotInstant, equalTo(expectedInstant));
     }
 
     public void testWeekBasedDates() {
-        // as per WeekFields.ISO first week starts on Monday and has minimum 4 days
+        // the years and weeks this outputs depends on where the first day of the first week is for each year
         DateFormatter dateFormatter = DateFormatters.forPattern("YYYY-ww");
 
-        // first week of 2016 starts on Monday 2016-01-04 as previous week in 2016 has only 3 days
         assertThat(
-            DateFormatters.from(dateFormatter.parse("2016-01")),
-            equalTo(ZonedDateTime.of(2016, 01, 04, 0, 0, 0, 0, ZoneOffset.UTC))
+            DateFormatters.from(dateFormatter.parse("2016-02")),
+            equalTo(ZonedDateTime.of(2016, 01, 03, 0, 0, 0, 0, ZoneOffset.UTC))
         );
 
-        // first week of 2015 starts on Monday 2014-12-29 because 4days belong to 2019
         assertThat(
-            DateFormatters.from(dateFormatter.parse("2015-01")),
-            equalTo(ZonedDateTime.of(2014, 12, 29, 0, 0, 0, 0, ZoneOffset.UTC))
+            DateFormatters.from(dateFormatter.parse("2015-02")),
+            equalTo(ZonedDateTime.of(2015, 01, 04, 0, 0, 0, 0, ZoneOffset.UTC))
         );
 
-        // as per WeekFields.ISO first week starts on Monday and has minimum 4 days
         dateFormatter = DateFormatters.forPattern("YYYY");
 
-        // first week of 2016 starts on Monday 2016-01-04 as previous week in 2016 has only 3 days
-        assertThat(DateFormatters.from(dateFormatter.parse("2016")), equalTo(ZonedDateTime.of(2016, 01, 04, 0, 0, 0, 0, ZoneOffset.UTC)));
-
-        // first week of 2015 starts on Monday 2014-12-29 because 4days belong to 2019
-        assertThat(DateFormatters.from(dateFormatter.parse("2015")), equalTo(ZonedDateTime.of(2014, 12, 29, 0, 0, 0, 0, ZoneOffset.UTC)));
+        assertThat(DateFormatters.from(dateFormatter.parse("2016")), equalTo(ZonedDateTime.of(2015, 12, 27, 0, 0, 0, 0, ZoneOffset.UTC)));
+        assertThat(DateFormatters.from(dateFormatter.parse("2015")), equalTo(ZonedDateTime.of(2014, 12, 28, 0, 0, 0, 0, ZoneOffset.UTC)));
     }
 
     public void testEpochMillisParser() {
@@ -600,8 +595,8 @@ public void testYearWithoutMonthRoundUp() {
         assertDateMathEquals("1500", "1500-01-01T23:59:59.999", "uuuu");
         assertDateMathEquals("2022", "2022-01-01T23:59:59.999", "uuuu");
         assertDateMathEquals("2022", "2022-01-01T23:59:59.999", "yyyy");
-        // cannot reliably default week based years due to locale changing. See JavaDateFormatter javadocs
-        assertDateMathEquals("2022", "2022-01-03T23:59:59.999", "YYYY", Locale.ROOT);
+        // weird locales can change this to epoch-based
+        assertDateMathEquals("2022", "2021-12-26T23:59:59.999", "YYYY", Locale.ROOT);
     }
 
     private void assertRoundupFormatter(String format, String input, long expectedMilliSeconds) {
@@ -789,30 +784,28 @@ public void testExceptionWhenCompositeParsingFailsDateMath() {
         String text = "2014-06-06T12:01:02.123";
         ElasticsearchParseException e1 = expectThrows(
             ElasticsearchParseException.class,
-            () -> dateMathToMillis(text, DateFormatter.forPattern(pattern), randomLocale(random()))
+            () -> dateMathToInstant(text, DateFormatter.forPattern(pattern), randomLocale(random()))
         );
         assertThat(e1.getMessage(), containsString(pattern));
         assertThat(e1.getMessage(), containsString(text));
     }
 
-    private long dateMathToMillis(String text, DateFormatter dateFormatter, Locale locale) {
+    private Instant dateMathToInstant(String text, DateFormatter dateFormatter, Locale locale) {
         DateFormatter javaFormatter = dateFormatter.withLocale(locale);
         DateMathParser javaDateMath = javaFormatter.toDateMathParser();
-        return javaDateMath.parse(text, () -> 0, true, (ZoneId) null).toEpochMilli();
+        return javaDateMath.parse(text, () -> 0, true, null);
     }
 
     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 javaFormatter = DateFormatter.forPattern("8e").withZone(ZoneOffset.UTC);
-        assertThat(javaFormatter.format(now), equalTo("7"));
+        assertThat(javaFormatter.format(now), equalTo("1"));
     }
 
     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 javaFormatter = DateFormatter.forPattern("8YYYY-ww").withZone(ZoneOffset.UTC);
-        assertThat(javaFormatter.format(now), equalTo("2019-21"));
+        assertThat(javaFormatter.format(now), equalTo("2019-22"));
     }
 
     // these parsers should allow both ',' and '.' as a decimal point
diff --git a/server/src/test/java/org/elasticsearch/common/time/JavaDateMathParserTests.java b/server/src/test/java/org/elasticsearch/common/time/JavaDateMathParserTests.java
diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/NamedDateTimeProcessorTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/NamedDateTimeProcessorTests.java

Original file line number	Diff line number	Diff line change
`@@ -93,7 +93,7 @@ public void execute(Task t) {`
`93`	`93`	`mkdirs(test.getWorkingDir().toPath().resolve("temp").toFile());`
`94`	`94`
`95`	`95`	`// TODO remove once jvm.options are added to test system properties`
`96`		`- test.systemProperty("java.locale.providers", "SPI,CLDR");`
	`96`	`+ test.systemProperty("java.locale.providers", "CLDR");`
`97`	`97`	`}`
`98`	`98`	`});`
`99`	`99`	`test.getJvmArgumentProviders().add(nonInputProperties);`
Original file line number	Diff line number	Diff line change
`@@ -45,3 +45,7 @@ dependencies {`
`45`	`45`	`compileOnly(project(':modules:lang-painless:spi'))`
`46`	`46`	`clusterModules(project(':modules:lang-painless'))`
`47`	`47`	`}`
	`48`	`+`
	`49`	`+tasks.named("yamlRestCompatTestTransform").configure({ task ->`
	`50`	`+ task.skipTest("aggregations/date_agg_per_day_of_week/Date aggregartion per day of week", "week-date behaviour has changed")`
	`51`	`+})`
Original file line number	Diff line number	Diff line change
`@@ -50,3 +50,7 @@ tasks.named("thirdPartyAudit").configure {`
`50`	`50`	`'org.apache.commons.logging.LogFactory',`
`51`	`51`	`)`
`52`	`52`	`}`
	`53`	`+`
	`54`	`+tasks.named("yamlRestCompatTestTransform").configure({ task ->`
	`55`	`+ task.skipTest("ingest/30_date_processor/Test week based date parsing", "week-date behaviour has changed")`
	`56`	`+})`