Skip to content

Commit 0513d8d

Browse files
authored
Add SPI jvm option to SystemJvmOptions (#50916)
Adding back accidentally removed jvm option that is required to enforce start of the week = Monday in IsoCalendarDataProvider. Adding a `feature` to yml test in order to skip running it in JDK8 commit that removed it 398c802 commit that backports SystemJvmOptions c4fbda3 relates 7.x backport of code that enforces CalendarDataProvider use #48349
1 parent 43ed244 commit 0513d8d

File tree

5 files changed

+95
-8
lines changed

5 files changed

+95
-8
lines changed

distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/SystemJvmOptions.java

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919

2020
package org.elasticsearch.tools.launchers;
2121

22+
import org.elasticsearch.tools.java_version_checker.JavaVersion;
23+
2224
import java.util.Arrays;
2325
import java.util.Collections;
2426
import java.util.List;
@@ -60,11 +62,26 @@ static List<String> systemJvmOptions() {
6062
// log4j 2
6163
"-Dlog4j.shutdownHookEnabled=false",
6264
"-Dlog4j2.disable.jmx=true",
63-
/*
64-
* Due to internationalization enhancements in JDK 9 Elasticsearch need to set the provider to COMPAT otherwise time/date
65-
* parsing will break in an incompatible way for some date patterns and locales.
66-
*/
67-
"-Djava.locale.providers=COMPAT"));
65+
66+
javaLocaleProviders()));
67+
}
68+
69+
private static String javaLocaleProviders() {
70+
/**
71+
* SPI setting is used to allow loading custom CalendarDataProvider
72+
* in jdk8 it has to be loaded from jre/lib/ext,
73+
* in jdk9+ it is already within ES project and on a classpath
74+
*
75+
* Due to internationalization enhancements in JDK 9 Elasticsearch need to set the provider to COMPAT otherwise time/date
76+
* parsing will break in an incompatible way for some date patterns and locales.
77+
* //TODO COMPAT will be deprecated in jdk14 https://bugs.openjdk.java.net/browse/JDK-8232906
78+
* See also: documentation in <code>server/org.elasticsearch.common.time.IsoCalendarDataProvider</code>
79+
*/
80+
if(JavaVersion.majorVersion(JavaVersion.CURRENT) == 8){
81+
return "-Djava.locale.providers=SPI,JRE";
82+
} else {
83+
return "-Djava.locale.providers=SPI,COMPAT";
84+
}
6885
}
6986

7087
}
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
2+
setup:
3+
- skip:
4+
version: " - 7.5.99"
5+
reason: "Start of the week Monday was backported to 7.6"
6+
features: "spi_on_classpath_jdk9"
7+
8+
- do:
9+
indices.create:
10+
index: test
11+
body:
12+
mappings:
13+
properties:
14+
date:
15+
type: date
16+
- do:
17+
index:
18+
index: test
19+
id: 1
20+
body: { "date": "2009-11-15T14:12:12" }
21+
- do:
22+
indices.refresh:
23+
index: [test]
24+
---
25+
# The inserted document has a field date=2009-11-15T14:12:12 which is Sunday.
26+
# When aggregating per day of the week this should be considered as last day of the week (7)
27+
# and this value should be used in 'key_as_string'
28+
"Date aggregation per day of week":
29+
- do:
30+
search:
31+
rest_total_hits_as_int: true
32+
index: test
33+
body:
34+
aggregations:
35+
test:
36+
"date_histogram": {
37+
"field": "date",
38+
"calendar_interval": "day",
39+
"format": "e",
40+
"offset": 0
41+
}
42+
- match: {hits.total: 1}
43+
- length: { aggregations.test.buckets: 1 }
44+
- match: { aggregations.test.buckets.0.key_as_string: "7" }

server/src/main/java/org/elasticsearch/common/time/IsoCalendarDataProvider.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,13 @@
2222
import java.util.Locale;
2323
import java.util.spi.CalendarDataProvider;
2424

25+
/**
26+
* This class is loaded by JVM SPI mechanism in order to provide ISO compatible behaviour for week calculations using java.time.
27+
* It defines start of the week as Monday and requires 4 days in the first week of the year.
28+
* This class overrides default behaviour for Locale.ROOT (start of the week Sunday, minimum 1 day in the first week of the year).
29+
* Java SPI mechanism requires java.locale.providers to contain SPI value (i.e. java.locale.providers=SPI,COMPAT)
30+
* @see <a href="https://en.wikipedia.org/wiki/ISO_week_date">ISO week date</a>
31+
*/
2532
public class IsoCalendarDataProvider extends CalendarDataProvider {
2633

2734
@Override

server/src/test/java/org/elasticsearch/common/joda/JavaJodaTimeDuellingTests.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import org.joda.time.DateTimeZone;
3030
import org.joda.time.format.DateTimeFormat;
3131
import org.joda.time.format.ISODateTimeFormat;
32+
import org.junit.BeforeClass;
3233

3334
import java.time.ZoneId;
3435
import java.time.ZoneOffset;
@@ -47,6 +48,14 @@ protected boolean enableWarningsCheck() {
4748
return false;
4849
}
4950

51+
@BeforeClass
52+
public static void checkJvmProperties(){
53+
boolean runtimeJdk8 = JavaVersion.current().getVersion().get(0) == 8;
54+
assert (runtimeJdk8 && ("SPI,JRE".equals(System.getProperty("java.locale.providers"))))
55+
|| (false == runtimeJdk8 && ("SPI,COMPAT".equals(System.getProperty("java.locale.providers"))))
56+
: "`-Djava.locale.providers` needs to be set";
57+
}
58+
5059
public void testTimezoneParsing() {
5160
/** this testcase won't work in joda. See comment in {@link #testPartialTimeParsing()}
5261
* assertSameDateAs("2016-11-30T+01", "strict_date_optional_time", "strict_date_optional_time");

test/framework/src/main/java/org/elasticsearch/test/rest/yaml/Features.java

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,12 @@
1919

2020
package org.elasticsearch.test.rest.yaml;
2121

22+
import org.elasticsearch.bootstrap.JavaVersion;
23+
import org.elasticsearch.test.rest.ESRestTestCase;
24+
2225
import java.util.Arrays;
2326
import java.util.List;
2427

25-
import org.elasticsearch.test.rest.ESRestTestCase;
26-
2728
import static java.util.Collections.unmodifiableList;
2829

2930
/**
@@ -50,6 +51,7 @@ public final class Features {
5051
"transform_and_set",
5152
"arbitrary_key"
5253
));
54+
private static final String SPI_ON_CLASSPATH_SINCE_JDK_9 = "spi_on_classpath_jdk9";
5355

5456
private Features() {
5557

@@ -68,10 +70,18 @@ public static boolean areAllSupported(List<String> features) {
6870
if (ESRestTestCase.hasXPack()) {
6971
return false;
7072
}
71-
} else if (false == SUPPORTED.contains(feature)) {
73+
} else if (false == isSupported(feature)) {
7274
return false;
7375
}
7476
}
7577
return true;
7678
}
79+
80+
private static boolean isSupported(String feature) {
81+
if(feature.equals(SPI_ON_CLASSPATH_SINCE_JDK_9) &&
82+
JavaVersion.current().compareTo(JavaVersion.parse("9")) >= 0) {
83+
return true;
84+
}
85+
return SUPPORTED.contains(feature) ;
86+
}
7787
}

0 commit comments

Comments
 (0)