Scripting: Rework joda time backcompat (#33486)

This commit switches the joda time backcompat in scripting to use augmentation over ZonedDateTime. The augmentation methods provide compatibility with the missing methods between joda's DateTime and java's ZonedDateTime. Due to getDayOfWeek returning an enum in the java API, ZonedDateTime is wrapped so that the method can return int like the joda time does. The java time api version is renamed to getDayOfWeekEnum, which will be kept through 7.x for compatibility while users switch back to getDayOfWeek once joda compatibility is removed.
elastic · Sep 17, 2018 · 758a6f5 · 758a6f5
1 parent fab45a4
commit 758a6f5
Show file tree

Hide file tree

Showing 24 changed files with 903 additions and 217 deletions.
diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy
@@ -827,9 +827,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'
 

diff --git a/docs/build.gradle b/docs/build.gradle
@@ -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'
 
   // TODO: remove this deprecation compatibility setting for 7.0

diff --git a/docs/painless/painless-getting-started.asciidoc b/docs/painless/painless-getting-started.asciidoc
@@ -234,11 +234,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]]

diff --git a/docs/reference/aggregations/bucket/datehistogram-aggregation.asciidoc b/docs/reference/aggregations/bucket/datehistogram-aggregation.asciidoc
@@ -416,7 +416,7 @@ POST /sales/_search?size=0
             "terms": {
                 "script": {
                     "lang": "painless",
-                    "source": "doc['date'].value.dayOfWeek"
+                    "source": "doc['date'].value.dayOfWeekEnum.value"
                 }
             }
         }
@@ -425,7 +425,6 @@ 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]
 
 Response:
 

diff --git a/modules/lang-painless/build.gradle b/modules/lang-painless/build.gradle
@@ -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'
 }
 

diff --git a/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/Whitelist.java b/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/Whitelist.java
@@ -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 =

diff --git a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/joda.time.txt b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/joda.time.txt
diff --git a/...les/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.txt b/...les/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.txt
@@ -74,15 +74,99 @@ class org.elasticsearch.index.fielddata.ScriptDocValues$Longs {
   Long get(int)
   long getValue()
   List getValues()
-  Object getDate()
+  JodaCompatibleZonedDateTime getDate()
   List getDates()
 }
 
+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()
+  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 methods that exist in java time
+  boolean equals(Object)
+  int hashCode()
+  boolean isAfter(ZonedDateTime)
+  boolean isBefore(ZonedDateTime)
+  boolean isEqual(ZonedDateTime)
+  String toString()
+
+  #### 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()
-  Object getDate()
+  JodaCompatibleZonedDateTime getDate()
   List getDates()
 }
 

diff --git a/modules/lang-painless/src/test/java/org/elasticsearch/painless/FunctionRefTests.java b/modules/lang-painless/src/test/java/org/elasticsearch/painless/FunctionRefTests.java
@@ -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;
@@ -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() {
@@ -197,10 +195,10 @@ 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(java.time.Instant::bogus); return l.get(0);", false);
         });
         assertThat(e.getMessage(),
-                containsString("function reference [org.joda.time.ReadableDateTime::bogus/2] matching [java.util.Comparator"));
+                containsString("function reference [java.time.Instant::bogus/2] matching [java.util.Comparator, compare/2"));
     }
 
     public void testClassMissing() {

diff --git a/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/20_scriptfield.yml b/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/20_scriptfield.yml
@@ -108,7 +108,7 @@ setup:
                 script_fields:
                     bar:
                         script:
-                            source: "doc.date.value.dayOfWeek.value"
+                            source: "doc.date.value.dayOfWeekEnum.value"
 
     - match: { hits.hits.0.fields.bar.0: 7}
 
@@ -123,7 +123,7 @@ setup:
                             source: >
                               StringBuilder b = new StringBuilder();
                               for (def date : doc.dates) {
-                                b.append(" ").append(date.getDayOfWeek().value);
+                                b.append(" ").append(date.getDayOfWeekEnum().value);
                               }
                               return b.toString().trim()
 

diff --git a/server/src/main/java/org/elasticsearch/common/io/stream/StreamOutput.java b/server/src/main/java/org/elasticsearch/common/io/stream/StreamOutput.java
@@ -39,6 +39,7 @@
 import org.elasticsearch.common.text.Text;
 import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.common.util.concurrent.EsRejectedExecutionException;
+import org.elasticsearch.script.JodaCompatibleZonedDateTime;
 import org.joda.time.DateTimeZone;
 import org.joda.time.ReadableInstant;
 
@@ -680,6 +681,15 @@ public final <K, V> void writeMap(final Map<K, V> map, final Writer<K> keyWriter
             o.writeString(zonedDateTime.getZone().getId());
             o.writeLong(zonedDateTime.toInstant().toEpochMilli());
         });
+        writers.put(JodaCompatibleZonedDateTime.class, (o, v) -> {
+            // write the joda compatibility datetime as joda datetime
+            o.writeByte((byte) 13);
+            final JodaCompatibleZonedDateTime zonedDateTime = (JodaCompatibleZonedDateTime) v;
+            String zoneId = zonedDateTime.getZonedDateTime().getZone().getId();
+            // joda does not understand "Z" for utc, so we must special case
+            o.writeString(zoneId.equals("Z") ? DateTimeZone.UTC.getID() : zoneId);
+            o.writeLong(zonedDateTime.toInstant().toEpochMilli());
+        });
         WRITERS = Collections.unmodifiableMap(writers);
     }
 

diff --git a/server/src/main/java/org/elasticsearch/common/xcontent/XContentElasticsearchExtension.java b/server/src/main/java/org/elasticsearch/common/xcontent/XContentElasticsearchExtension.java
@@ -25,6 +25,7 @@
 import org.elasticsearch.common.time.DateFormatters;
 import org.elasticsearch.common.unit.ByteSizeValue;
 import org.elasticsearch.common.unit.TimeValue;
+import org.elasticsearch.script.JodaCompatibleZonedDateTime;
 import org.joda.time.DateTime;
 import org.joda.time.DateTimeZone;
 import org.joda.time.Instant;
@@ -93,6 +94,7 @@ public Map<Class<?>, XContentBuilder.Writer> getXContentWriters() {
         writers.put(Year.class, (b, v) -> b.value(v.toString()));
         writers.put(Duration.class, (b, v) -> b.value(v.toString()));
         writers.put(Period.class, (b, v) -> b.value(v.toString()));
+        writers.put(JodaCompatibleZonedDateTime.class, XContentBuilder::timeValue);
 
         writers.put(BytesReference.class, (b, v) -> {
             if (v == null) {
@@ -141,6 +143,8 @@ public Map<Class<?>, Function<Object, Object>> getDateTransformers() {
             d -> DEFAULT_FORMATTER.format(ZonedDateTime.ofInstant((java.time.Instant) d, ZoneOffset.UTC)));
         transformers.put(LocalDate.class, d -> ((LocalDate) d).toString());
         transformers.put(LocalTime.class, d -> LOCAL_TIME_FORMATTER.format((LocalTime) d));
+        transformers.put(JodaCompatibleZonedDateTime.class,
+            d -> DEFAULT_FORMATTER.format(((JodaCompatibleZonedDateTime) d).getZonedDateTime()));
         return transformers;
     }
 }
diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/AtomicFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/AtomicFieldData.java
@@ -28,10 +28,17 @@
 public interface AtomicFieldData extends Accountable, Releasable {
 
     /**
-     * Returns a "scripting" based values.
+     * Returns field values for use in scripting.
      */
     ScriptDocValues<?> getScriptValues();
 
+    /**
+     * Returns field values for use by returned hits.
+     */
+    default ScriptDocValues<?> getLegacyFieldValues() {
+        return getScriptValues();
+    }
+
     /**
      * Return a String representation of the values.
      */