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
This commit is contained in:
Przemyslaw Gomulka 2020-01-21 09:02:21 +01:00 committed by GitHub
parent 43ed244a04
commit 0513d8dca3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 95 additions and 8 deletions

View File

@ -19,6 +19,8 @@
package org.elasticsearch.tools.launchers;
import org.elasticsearch.tools.java_version_checker.JavaVersion;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
@ -60,11 +62,26 @@ final class SystemJvmOptions {
// log4j 2
"-Dlog4j.shutdownHookEnabled=false",
"-Dlog4j2.disable.jmx=true",
/*
javaLocaleProviders()));
}
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 Elasticsearch 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 jdk14 https://bugs.openjdk.java.net/browse/JDK-8232906
* See also: documentation in <code>server/org.elasticsearch.common.time.IsoCalendarDataProvider</code>
*/
"-Djava.locale.providers=COMPAT"));
if(JavaVersion.majorVersion(JavaVersion.CURRENT) == 8){
return "-Djava.locale.providers=SPI,JRE";
} else {
return "-Djava.locale.providers=SPI,COMPAT";
}
}
}

View File

@ -0,0 +1,44 @@
setup:
- skip:
version: " - 7.5.99"
reason: "Start of the week Monday was backported to 7.6"
features: "spi_on_classpath_jdk9"
- do:
indices.create:
index: test
body:
mappings:
properties:
date:
type: date
- do:
index:
index: test
id: 1
body: { "date": "2009-11-15T14:12:12" }
- do:
indices.refresh:
index: [test]
---
# The inserted document has a field date=2009-11-15T14:12:12 which is Sunday.
# When aggregating per day of the week this should be considered as last day of the week (7)
# and this value should be used in 'key_as_string'
"Date aggregation per day of week":
- do:
search:
rest_total_hits_as_int: true
index: test
body:
aggregations:
test:
"date_histogram": {
"field": "date",
"calendar_interval": "day",
"format": "e",
"offset": 0
}
- match: {hits.total: 1}
- length: { aggregations.test.buckets: 1 }
- match: { aggregations.test.buckets.0.key_as_string: "7" }

View File

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

View File

@ -29,6 +29,7 @@ import org.joda.time.DateTime;
import org.joda.time.DateTimeZone;
import org.joda.time.format.DateTimeFormat;
import org.joda.time.format.ISODateTimeFormat;
import org.junit.BeforeClass;
import java.time.ZoneId;
import java.time.ZoneOffset;
@ -47,6 +48,14 @@ public class JavaJodaTimeDuellingTests extends ESTestCase {
return false;
}
@BeforeClass
public static void checkJvmProperties(){
boolean runtimeJdk8 = JavaVersion.current().getVersion().get(0) == 8;
assert (runtimeJdk8 && ("SPI,JRE".equals(System.getProperty("java.locale.providers"))))
|| (false == runtimeJdk8 && ("SPI,COMPAT".equals(System.getProperty("java.locale.providers"))))
: "`-Djava.locale.providers` needs to be set";
}
public void testTimezoneParsing() {
/** this testcase won't work in joda. See comment in {@link #testPartialTimeParsing()}
* assertSameDateAs("2016-11-30T+01", "strict_date_optional_time", "strict_date_optional_time");

View File

@ -19,11 +19,12 @@
package org.elasticsearch.test.rest.yaml;
import org.elasticsearch.bootstrap.JavaVersion;
import org.elasticsearch.test.rest.ESRestTestCase;
import java.util.Arrays;
import java.util.List;
import org.elasticsearch.test.rest.ESRestTestCase;
import static java.util.Collections.unmodifiableList;
/**
@ -50,6 +51,7 @@ public final class Features {
"transform_and_set",
"arbitrary_key"
));
private static final String SPI_ON_CLASSPATH_SINCE_JDK_9 = "spi_on_classpath_jdk9";
private Features() {
@ -68,10 +70,18 @@ public final class Features {
if (ESRestTestCase.hasXPack()) {
return false;
}
} else if (false == SUPPORTED.contains(feature)) {
} else if (false == isSupported(feature)) {
return false;
}
}
return true;
}
private static boolean isSupported(String feature) {
if(feature.equals(SPI_ON_CLASSPATH_SINCE_JDK_9) &&
JavaVersion.current().compareTo(JavaVersion.parse("9")) >= 0) {
return true;
}
return SUPPORTED.contains(feature) ;
}
}