Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Scripting: Rework joda time backcompat #33486

Merged
merged 15 commits into from
Sep 17, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -825,9 +825,6 @@ class BuildPlugin implements Plugin<Project> {
}
}

// TODO: remove this once joda time is removed from scripting in 7.0
systemProperty 'es.scripting.use_java_time', 'true'

// TODO: remove this once ctx isn't added to update script params in 7.0
systemProperty 'es.scripting.update.ctx_in_params', 'false'

Expand Down
1 change: 0 additions & 1 deletion docs/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ integTestCluster {
setting 'reindex.remote.whitelist', '127.0.0.1:*'

// TODO: remove this for 7.0, this exists to allow the doc examples in 6.x to continue using the defaults
systemProperty 'es.scripting.use_java_time', 'false'
systemProperty 'es.scripting.update.ctx_in_params', 'false'
}

Expand Down
5 changes: 0 additions & 5 deletions docs/painless/painless-getting-started.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -220,11 +220,6 @@ GET hockey/_search
}
----------------------------------------------------------------
// CONSOLE
// TEST[warning:The joda time api for doc values is deprecated. Use -Des.scripting.use_java_time=true to use the java time api for date field doc values]

NOTE: Date fields are changing in 7.0 to be exposed as `ZonedDateTime`
from Java 8's time API. To switch to this functionality early,
add `-Des.scripting.use_java_time=true` to `jvm.options`.

[float]
[[modules-scripting-painless-regex]]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ POST /sales/_search?size=0
--------------------------------------------------
// CONSOLE
// TEST[setup:sales]
// TEST[warning:The joda time api for doc values is deprecated. Use -Des.scripting.use_java_time=true to use the java time api for date field doc values]
// TEST[warning:getDayOfWeek() is incompatible with the java time api. Use getDayOfWeekEnum().getValue()."]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe "the return type of getDayOfWeek will change in 7.0. Call getDayOfWeekEnum().getValue() to be compatible with 7.0."


Response:

Expand Down
1 change: 0 additions & 1 deletion modules/lang-painless/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ esplugin {

integTestCluster {
module project.project(':modules:mapper-extras')
systemProperty 'es.scripting.use_java_time', 'true'
systemProperty 'es.scripting.update.ctx_in_params', 'false'
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,7 @@ public final class Whitelist {
"java.util.txt",
"java.util.function.txt",
"java.util.regex.txt",
"java.util.stream.txt",
"joda.time.txt"
"java.util.stream.txt"
};

public static final List<Whitelist> BASE_WHITELISTS =
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,85 @@ class org.elasticsearch.index.fielddata.ScriptDocValues$Longs {
List getValues()
}

class org.elasticsearch.script.JodaCompatibleZonedDateTime {
##### ZonedDateTime methods
int getDayOfMonth()
int getDayOfYear()
int getHour()
LocalDate toLocalDate()
LocalDateTime toLocalDateTime()
int getMinute()
Month getMonth()
int getMonthValue()
int getNano()
int getSecond()
int getYear()
ZonedDateTime minus(TemporalAmount)
ZonedDateTime minus(long,TemporalUnit)
ZonedDateTime minusYears(long)
ZonedDateTime minusMonths(long)
ZonedDateTime minusWeeks(long)
ZonedDateTime minusDays(long)
ZonedDateTime minusHours(long)
ZonedDateTime minusMinutes(long)
ZonedDateTime minusSeconds(long)
ZonedDateTime minusNanos(long)
ZonedDateTime plus(TemporalAmount)
ZonedDateTime plus(long,TemporalUnit)
ZonedDateTime plusDays(long)
ZonedDateTime plusHours(long)
ZonedDateTime plusMinutes(long)
ZonedDateTime plusMonths(long)
ZonedDateTime plusNanos(long)
ZonedDateTime plusSeconds(long)
ZonedDateTime plusWeeks(long)
ZonedDateTime plusYears(long)
Instant toInstant()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where is this one coming from?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ignore this, misread it as being something for BWC, but it's a new method

OffsetDateTime toOffsetDateTime()
ZonedDateTime truncatedTo(TemporalUnit)
ZonedDateTime with(TemporalAdjuster)
ZonedDateTime with(TemporalField,long)
ZonedDateTime withDayOfMonth(int)
ZonedDateTime withDayOfYear(int)
ZonedDateTime withEarlierOffsetAtOverlap()
ZonedDateTime withFixedOffsetZone()
ZonedDateTime withHour(int)
ZonedDateTime withLaterOffsetAtOverlap()
ZonedDateTime withMinute(int)
ZonedDateTime withMonth(int)
ZonedDateTime withNano(int)
ZonedDateTime withSecond(int)
ZonedDateTime withYear(int)
ZonedDateTime withZoneSameLocal(ZoneId)
ZonedDateTime withZoneSameInstant(ZoneId)

#### Joda time methods
long getMillis()
int getCenturyOfEra()
int getEra()
int getHourOfDay()
int getMillisOfDay()
int getMillisOfSecond()
int getMinuteOfDay()
int getMinuteOfHour()
int getMonthOfYear()
int getSecondOfDay()
int getSecondOfMinute()
int getWeekOfWeekyear()
int getWeekyear()
int getYearOfCentury()
int getYearOfEra()
String toString(String)
String toString(String,Locale)

# conflicting methods
DayOfWeek getDayOfWeekEnum()
int getDayOfWeek()
}

class org.elasticsearch.index.fielddata.ScriptDocValues$Dates {
Object get(int)
Object getValue()
JodaCompatibleZonedDateTime get(int)
JodaCompatibleZonedDateTime getValue()
List getValues()
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,8 @@

package org.elasticsearch.painless;

import org.joda.time.DateTime;
import org.joda.time.DateTimeZone;

import java.lang.invoke.LambdaConversionException;
import java.time.Instant;

import static java.util.Collections.singletonMap;
import static org.hamcrest.Matchers.containsString;
Expand Down Expand Up @@ -59,15 +57,15 @@ public void testQualifiedStaticMethodReferenceDef() {
public void testQualifiedVirtualMethodReference() {
long instant = randomLong();
assertEquals(instant, exec(
"List l = [params.d]; return l.stream().mapToLong(org.joda.time.ReadableDateTime::getMillis).sum()",
singletonMap("d", new DateTime(instant, DateTimeZone.UTC)), true));
"List l = [params.d]; return l.stream().mapToLong(Instant::toEpochMilli).sum()",
singletonMap("d", Instant.ofEpochMilli(instant)), true));
}

public void testQualifiedVirtualMethodReferenceDef() {
long instant = randomLong();
assertEquals(instant, exec(
"def l = [params.d]; return l.stream().mapToLong(org.joda.time.ReadableDateTime::getMillis).sum()",
singletonMap("d", new DateTime(instant, DateTimeZone.UTC)), true));
"def l = [params.d]; return l.stream().mapToLong(Instant::toEpochMilli).sum()",
singletonMap("d", Instant.ofEpochMilli(instant)), true));
}

public void testCtorMethodReference() {
Expand Down Expand Up @@ -197,7 +195,7 @@ public void testMethodMissing() {

public void testQualifiedMethodMissing() {
Exception e = expectScriptThrows(IllegalArgumentException.class, () -> {
exec("List l = [2, 1]; l.sort(org.joda.time.ReadableDateTime::bogus); return l.get(0);", false);
exec("List l = [2, 1]; l.sort(Instant::bogus); return l.get(0);", false);
});
assertThat(e.getMessage(),
containsString("function reference [org.joda.time.ReadableDateTime::bogus/2] matching [java.util.Comparator"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,26 +26,17 @@
import org.elasticsearch.common.geo.GeoHashUtils;
import org.elasticsearch.common.geo.GeoPoint;
import org.elasticsearch.common.geo.GeoUtils;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.logging.ESLoggerFactory;
import org.joda.time.DateTimeZone;
import org.joda.time.MutableDateTime;
import org.elasticsearch.script.JodaCompatibleZonedDateTime;

import java.io.IOException;
import java.security.AccessController;
import java.security.PrivilegedAction;
import java.time.Instant;
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.util.AbstractList;
import java.util.Arrays;
import java.util.Comparator;
import java.util.List;
import java.util.function.Consumer;
import java.util.function.UnaryOperator;

import static org.elasticsearch.common.Booleans.parseBoolean;

/**
* Script level doc values, the assumption is that any implementation will
* implement a <code>getValue</code> and a <code>getValues</code> that return
Expand Down Expand Up @@ -147,55 +138,28 @@ public int size() {
}
}

public static final class Dates extends ScriptDocValues<Object> {

/** Whether scripts should expose dates as java time objects instead of joda time. */
private static final boolean USE_JAVA_TIME = parseBoolean(System.getProperty("es.scripting.use_java_time"), false);

private static final DeprecationLogger deprecationLogger = new DeprecationLogger(ESLoggerFactory.getLogger(Dates.class));
public static final class Dates extends ScriptDocValues<JodaCompatibleZonedDateTime> {

private final SortedNumericDocValues in;

/**
* Method call to add deprecation message. Normally this is
* {@link #deprecationLogger} but tests override.
* Values wrapped in {@link java.time.ZonedDateTime} objects.
*/
private final Consumer<String> deprecationCallback;

/**
* Whether java time or joda time should be used. This is normally {@link #USE_JAVA_TIME} but tests override it.
*/
private final boolean useJavaTime;

/**
* Values wrapped in a date time object. The concrete type depends on the system property {@code es.scripting.use_java_time}.
* When that system property is {@code false}, the date time objects are of type {@link MutableDateTime}. When the system
* property is {@code true}, the date time objects are of type {@link java.time.ZonedDateTime}.
*/
private Object[] dates;
private JodaCompatibleZonedDateTime[] dates;
private int count;

/**
* Standard constructor.
*/
public Dates(SortedNumericDocValues in) {
this(in, message -> deprecationLogger.deprecatedAndMaybeLog("scripting_joda_time_deprecation", message), USE_JAVA_TIME);
}

/**
* Constructor for testing with a deprecation callback.
*/
Dates(SortedNumericDocValues in, Consumer<String> deprecationCallback, boolean useJavaTime) {
this.in = in;
this.deprecationCallback = deprecationCallback;
this.useJavaTime = useJavaTime;
}

/**
* Fetch the first field value or 0 millis after epoch if there are no
* in.
*/
public Object getValue() {
public JodaCompatibleZonedDateTime getValue() {
if (count == 0) {
throw new IllegalStateException("A document doesn't have a value for a field! " +
"Use doc[<field>].size()==0 to check if a document is missing a field!");
Expand All @@ -204,7 +168,7 @@ public Object getValue() {
}

@Override
public Object get(int index) {
public JodaCompatibleZonedDateTime get(int index) {
if (index >= count) {
throw new IndexOutOfBoundsException(
"attempted to fetch the [" + index + "] date when there are only ["
Expand Down Expand Up @@ -235,41 +199,13 @@ void refreshArray() throws IOException {
if (count == 0) {
return;
}
if (useJavaTime) {
if (dates == null || count > dates.length) {
// Happens for the document. We delay allocating dates so we can allocate it with a reasonable size.
dates = new ZonedDateTime[count];
}
for (int i = 0; i < count; ++i) {
dates[i] = ZonedDateTime.ofInstant(Instant.ofEpochMilli(in.nextValue()), ZoneOffset.UTC);
}
} else {
deprecated("The joda time api for doc values is deprecated. Use -Des.scripting.use_java_time=true" +
" to use the java time api for date field doc values");
if (dates == null || count > dates.length) {
// Happens for the document. We delay allocating dates so we can allocate it with a reasonable size.
dates = new MutableDateTime[count];
}
for (int i = 0; i < count; i++) {
dates[i] = new MutableDateTime(in.nextValue(), DateTimeZone.UTC);
}
if (dates == null || count > dates.length) {
// Happens for the document. We delay allocating dates so we can allocate it with a reasonable size.
dates = new JodaCompatibleZonedDateTime[count];
}
for (int i = 0; i < count; ++i) {
dates[i] = new JodaCompatibleZonedDateTime(Instant.ofEpochMilli(in.nextValue()), ZoneOffset.UTC);
}
}

/**
* Log a deprecation log, with the server's permissions, not the permissions of the
* script calling this method. We need to do this to prevent errors when rolling
* the log file.
*/
private void deprecated(String message) {
// Intentionally not calling SpecialPermission.check because this is supposed to be called by scripts
AccessController.doPrivileged(new PrivilegedAction<Void>() {
@Override
public Void run() {
deprecationCallback.accept(message);
return null;
}
});
}
}

Expand Down
Loading