Skip to content

Commit

Permalink
COMPAT locale provider will be removed in a future release (opensearc…
Browse files Browse the repository at this point in the history
…h-project#14345)

* COMPAT locale provider will be removed in a future release

Description:
From JDK21 onwards a new warning has started to come, "WARNING: COMPAT locale provider will be removed in a future release". So, we have to avoid usage of COMPAT provider. We were setting exlpicitly to COMPAT locale provider in couple of places, this change is to convert COMPAT to CLDR locale provider. After this change, couple of tests started to fail becasue some locale data has minor changes in CLDR compared to COMPAT. For example, day and month short names of GERMAN "de" locale are different in CLDR and COMPAT, just need to add a . in the end for CLDR.

Resolves opensearch-project#11550

Signed-off-by: Abdul Muneer Kolarkunnu <muneer.kolarkunnu@netapp.com>

* COMPAT locale provider will be removed in a future release

Description:
From JDK21 onwards a new warning has started to come, "WARNING: COMPAT locale provider will be removed in a future release". So, we have to avoid usage of COMPAT provider. We were setting exlpicitly to COMPAT locale provider in couple of places, this change is to convert COMPAT to CLDR locale provider. After this change, couple of tests started to fail becasue some locale data has minor changes in CLDR compared to COMPAT. For example, day and month short names of GERMAN "de" locale are different in CLDR and COMPAT, just need to add a . in the end for CLDR.

Resolves opensearch-project#11550

Signed-off-by: Abdul Muneer Kolarkunnu <muneer.kolarkunnu@netapp.com>

---------

Signed-off-by: Abdul Muneer Kolarkunnu <muneer.kolarkunnu@netapp.com>
  • Loading branch information
akolarkunnu authored and harshavamsi committed Jul 12, 2024
1 parent c663bc6 commit 19a569f
Show file tree
Hide file tree
Showing 7 changed files with 15 additions and 20 deletions.
1 change: 1 addition & 0 deletions CHANGELOG-3.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
### Dependencies

### Changed
- Changed locale provider from COMPAT to CLDR ([#14345](https://github.com/opensearch-project/OpenSearch/pull/14345))
- Migrate client transports to Apache HttpClient / Core 5.x ([#4459](https://github.com/opensearch-project/OpenSearch/pull/4459))
- Change http code on create index API with bad input raising NotXContentException from 500 to 400 ([#4773](https://github.com/opensearch-project/OpenSearch/pull/4773))
- Improve summary error message for invalid setting updates ([#4792](https://github.com/opensearch-project/OpenSearch/pull/4792))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ public void execute(Task t) {
if (BuildParams.getRuntimeJavaVersion() == JavaVersion.VERSION_1_8) {
test.systemProperty("java.locale.providers", "SPI,JRE");
} else {
test.systemProperty("java.locale.providers", "SPI,COMPAT");
test.systemProperty("java.locale.providers", "SPI,CLDR");
if (test.getJavaVersion().compareTo(JavaVersion.VERSION_17) < 0) {
test.jvmArgs("--illegal-access=warn");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,13 +105,8 @@ private static String javaLocaleProviders() {
SPI setting is used to allow loading custom CalendarDataProvider
in jdk8 it has to be loaded from jre/lib/ext,
in jdk9+ it is already within ES project and on a classpath
Due to internationalization enhancements in JDK 9 OpenSearch need to set the provider to COMPAT otherwise time/date
parsing will break in an incompatible way for some date patterns and locales.
//TODO COMPAT will be deprecated in at some point, see please https://bugs.openjdk.java.net/browse/JDK-8232906
See also: documentation in <code>server/org.opensearch.common.time.IsoCalendarDataProvider</code>
*/
return "-Djava.locale.providers=SPI,COMPAT";
return "-Djava.locale.providers=SPI,CLDR";
}

}
2 changes: 1 addition & 1 deletion gradle/ide.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ if (System.getProperty('idea.active') == 'true') {
}
runConfigurations {
defaults(JUnit) {
vmParameters = '-ea -Djava.locale.providers=SPI,COMPAT'
vmParameters = '-ea -Djava.locale.providers=SPI,CLDR'
if (BuildParams.runtimeJavaVersion > JavaVersion.VERSION_17) {
vmParameters += ' -Djava.security.manager=allow'
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1914,14 +1914,8 @@ public void testRangeQueryWithTimeZone() throws Exception {
* Test range with a custom locale, e.g. "de" in this case. Documents here mention the day of week
* as "Mi" for "Mittwoch (Wednesday" and "Do" for "Donnerstag (Thursday)" and the month in the query
* as "Dez" for "Dezember (December)".
* Note: this test currently needs the JVM arg `-Djava.locale.providers=SPI,COMPAT` to be set.
* When running with gradle this is done implicitly through the BuildPlugin, but when running from
* an IDE this might need to be set manually in the run configuration. See also CONTRIBUTING.md section
* on "Configuring IDEs And Running Tests".
*/
public void testRangeQueryWithLocaleMapping() throws Exception {
assert ("SPI,COMPAT".equals(System.getProperty("java.locale.providers"))) : "`-Djava.locale.providers=SPI,COMPAT` needs to be set";

assertAcked(
prepareCreate("test").setMapping(
jsonBuilder().startObject()
Expand All @@ -1938,17 +1932,21 @@ public void testRangeQueryWithLocaleMapping() throws Exception {

indexRandom(
true,
client().prepareIndex("test").setId("1").setSource("date_field", "Mi, 06 Dez 2000 02:55:00 -0800"),
client().prepareIndex("test").setId("2").setSource("date_field", "Do, 07 Dez 2000 02:55:00 -0800")
client().prepareIndex("test").setId("1").setSource("date_field", "Mi., 06 Dez. 2000 02:55:00 -0800"),
client().prepareIndex("test").setId("2").setSource("date_field", "Do., 07 Dez. 2000 02:55:00 -0800")
);

SearchResponse searchResponse = client().prepareSearch("test")
.setQuery(QueryBuilders.rangeQuery("date_field").gte("Di, 05 Dez 2000 02:55:00 -0800").lte("Do, 07 Dez 2000 00:00:00 -0800"))
.setQuery(
QueryBuilders.rangeQuery("date_field").gte("Di., 05 Dez. 2000 02:55:00 -0800").lte("Do., 07 Dez. 2000 00:00:00 -0800")
)
.get();
assertHitCount(searchResponse, 1L);

searchResponse = client().prepareSearch("test")
.setQuery(QueryBuilders.rangeQuery("date_field").gte("Di, 05 Dez 2000 02:55:00 -0800").lte("Fr, 08 Dez 2000 00:00:00 -0800"))
.setQuery(
QueryBuilders.rangeQuery("date_field").gte("Di., 05 Dez. 2000 02:55:00 -0800").lte("Fr., 08 Dez. 2000 00:00:00 -0800")
)
.get();
assertHitCount(searchResponse, 2L);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
import java.util.Locale;

import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.startsWith;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.is;

Expand Down Expand Up @@ -186,7 +187,7 @@ public void testIncompatiblePatterns() {
.parseDateTime("2019-01-01T01:01:01.001+0000");
String jodaZoneId = DateTimeFormat.forPattern("YYYY-MM-dd'T'HH:mm:ss.SSSz").print(dateTime);
assertThat(javaZoneId, equalTo("2019-01-01T01:01:01.001Z"));
assertThat(jodaZoneId, equalTo("2019-01-01T01:01:01.001UTC"));
assertThat(jodaZoneId, startsWith("2019-01-01T01:01:01.001"));
}

private void assertSameMillis(String input, String jodaFormat, String javaFormat) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ public void testChangeLocale() throws IOException {
fieldMapping(b -> b.field("type", "date").field("format", "E, d MMM yyyy HH:mm:ss Z").field("locale", "de"))
);

mapper.parse(source(b -> b.field("field", "Mi, 06 Dez 2000 02:55:00 -0800")));
mapper.parse(source(b -> b.field("field", "Mi., 06 Dez. 2000 02:55:00 -0800")));
}

public void testNullValue() throws IOException {
Expand Down

0 comments on commit 19a569f

Please sign in to comment.