Delete deprecated getValues from ScriptDocValues (#36183)

* Adds deprecation logging to ScriptDocValues#getValues.

First commit addressing issue #22919.

`ScriptDocValues#getValues` was added for backwards compatibility but no
longer needed. Scripts using the syntax `doc['foo'].values` when
`doc['foo']` is a list should be using `doc['foo']` instead.

* Fixes two build errors in #34279

* Removes unused import in ScriptDocValuesDatesTest
* Removes used of `.values` in example in diversified-sampler-aggregation.asciidoc

* Removes use of .values from painless test.

Part of #34279

* Updates tests to use `doc[foo]` syntax rather than `doc[foo].values`.

* Removes use of `getValues()` and replaces use of `doc[foo].values` with `doc[foo]`.

* Indentation fix.

* Remove unnecessary list construction at previous `getValues()` callsite in ScriptDocValues.GeoPoints.

* Update migration doc and add link to `getValue` in ScriptDocValues javadoc.

* Fix compile

* Fix javadoc issue

* Removes ScriptDocValues#getValues usage from painless whitelist.

Author: jeffhajewski

docs/reference/migration/migrate_7_0/scripting.asciidoc

@@ -29,3 +29,9 @@ To check if a document is missing a value, you can use
 Malformed scripts, either in search templates, ingest pipelines or search 
 requests, return `400 - Bad request` while they would previously return
 `500 - Internal Server Error`. This also applies for stored scripts.
+
+[float]
+==== getValues() removed
+
+The `ScriptDocValues#getValues()` method is deprecated in 6.6 and will
+be removed in 7.0. Use `doc["foo"]` in place of `doc["foo"].values`.

modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.txt

@@ -67,13 +67,11 @@ class org.elasticsearch.common.geo.GeoPoint {
 class org.elasticsearch.index.fielddata.ScriptDocValues$Strings {
   String get(int)
   String getValue()
-  List getValues()
 }
 
 class org.elasticsearch.index.fielddata.ScriptDocValues$Longs {
   Long get(int)
   long getValue()
-  List getValues()
 }
 
 class org.elasticsearch.script.JodaCompatibleZonedDateTime {
@@ -163,19 +161,16 @@ class org.elasticsearch.script.JodaCompatibleZonedDateTime {
 class org.elasticsearch.index.fielddata.ScriptDocValues$Dates {
   JodaCompatibleZonedDateTime get(int)
   JodaCompatibleZonedDateTime getValue()
-  List getValues()
 }
 
 class org.elasticsearch.index.fielddata.ScriptDocValues$Doubles {
   Double get(int)
   double getValue()
-  List getValues()
 }
 
 class org.elasticsearch.index.fielddata.ScriptDocValues$GeoPoints {
   org.elasticsearch.common.geo.GeoPoint get(int)
   org.elasticsearch.common.geo.GeoPoint getValue()
-  List getValues()
   double getLat()
   double getLon()
   double[] getLats()
@@ -193,13 +188,11 @@ class org.elasticsearch.index.fielddata.ScriptDocValues$GeoPoints {
 class org.elasticsearch.index.fielddata.ScriptDocValues$Booleans {
   Boolean get(int)
   boolean getValue()
-  List getValues()
 }
 
 class org.elasticsearch.index.fielddata.ScriptDocValues$BytesRefs {
   BytesRef get(int)
   BytesRef getValue()
-  List getValues()
 }
 
 class org.apache.lucene.util.BytesRef {
@@ -213,7 +206,6 @@ class org.apache.lucene.util.BytesRef {
 class org.elasticsearch.index.mapper.IpFieldMapper$IpFieldType$IpScriptDocValues {
   String get(int)
   String getValue()
-  List getValues()
 }
 
 class org.elasticsearch.search.lookup.FieldLookup {
@@ -268,4 +260,4 @@ static_import {
   int staticAddIntsTest(int, int) from_class org.elasticsearch.painless.StaticTest
   float staticAddFloatsTest(float, float) from_class org.elasticsearch.painless.FeatureTest
   int testAddWithState(int, int, int, double) bound_to org.elasticsearch.painless.BindingTest
-}
+}

server/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java

@@ -19,73 +19,38 @@
 
 package org.elasticsearch.index.fielddata;
 
-import org.apache.logging.log4j.LogManager;
 import org.apache.lucene.index.SortedNumericDocValues;
 import org.apache.lucene.util.ArrayUtil;
 import org.apache.lucene.util.BytesRef;
 import org.apache.lucene.util.BytesRefBuilder;
 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.script.JodaCompatibleZonedDateTime;
 
 import java.io.IOException;
-import java.security.AccessController;
-import java.security.PrivilegedAction;
 import java.time.Instant;
 import java.time.ZoneOffset;
 import java.util.AbstractList;
 import java.util.Arrays;
 import java.util.Comparator;
-import java.util.List;
-import java.util.function.BiConsumer;
 import java.util.function.UnaryOperator;
 
 /**
  * Script level doc values, the assumption is that any implementation will
- * implement a <code>getValue</code> and a <code>getValues</code> that return
- * the relevant type that then can be used in scripts.
+ * implement a {@link Longs#getValue getValue} method.
  *
  * Implementations should not internally re-use objects for the values that they
  * return as a single {@link ScriptDocValues} instance can be reused to return
  * values form multiple documents.
  */
 public abstract class ScriptDocValues<T> extends AbstractList<T> {
 
-    private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(ScriptDocValues.class));
-    /**
-     * Callback for deprecated fields. In production this should always point to
-     * {@link #deprecationLogger} but tests will override it so they can test
-     * that we use the required permissions when calling it.
-     */
-    private final BiConsumer<String, String> deprecationCallback;
-
-    public ScriptDocValues() {
-        deprecationCallback = deprecationLogger::deprecatedAndMaybeLog;
-    }
-
-    /**
-     * Constructor for testing deprecation callback.
-     */
-    ScriptDocValues(BiConsumer<String, String> deprecationCallback) {
-        this.deprecationCallback = deprecationCallback;
-    }
-
     /**
      * Set the current doc ID.
      */
     public abstract void setNextDocId(int docId) throws IOException;
 
-    /**
-     * Return a copy of the list of the values for the current document.
-     */
-    public final List<T> getValues() {
-        deprecated("ScriptDocValues#getValues", "Deprecated getValues used, the field is a list and should be accessed directly."
-                + " For example, use doc['foo'] instead of doc['foo'].values.");
-        return this;
-    }
-
     // Throw meaningful exceptions if someone tries to modify the ScriptDocValues.
     @Override
     public final void add(int index, T element) {
@@ -112,21 +77,6 @@ public final void sort(Comparator<? super T> c) {
         throw new UnsupportedOperationException("doc values are unmodifiable");
     }
 
-    /**
-     * Log a deprecation log, with the server's permissions and 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 key, String message) {
-        AccessController.doPrivileged(new PrivilegedAction<Void>() {
-            @Override
-            public Void run() {
-                deprecationCallback.accept(key, message);
-                return null;
-            }
-        });
-    }
-
     public static final class Longs extends ScriptDocValues<Long> {
         private final SortedNumericDocValues in;
         private long[] values = new long[0];
@@ -139,14 +89,6 @@ public Longs(SortedNumericDocValues in) {
             this.in = in;
         }
 
-        /**
-         * Constructor for testing deprecation callback.
-         */
-        Longs(SortedNumericDocValues in, BiConsumer<String, String> deprecationCallback) {
-            super(deprecationCallback);
-            this.in = in;
-        }
-
         @Override
         public void setNextDocId(int docId) throws IOException {
             if (in.advanceExact(docId)) {
@@ -204,14 +146,6 @@ public Dates(SortedNumericDocValues in) {
             this.in = in;
         }
 
-        /**
-         * Constructor for testing deprecation callback.
-         */
-        Dates(SortedNumericDocValues in, BiConsumer<String, String> deprecationCallback) {
-            super(deprecationCallback);
-            this.in = in;
-        }
-
         /**
          * Fetch the first field value or 0 millis after epoch if there are no
          * in.
@@ -330,14 +264,6 @@ public GeoPoints(MultiGeoPointValues in) {
             this.in = in;
         }
 
-        /**
-         * Constructor for testing deprecation callback.
-         */
-        GeoPoints(MultiGeoPointValues in, BiConsumer<String, String> deprecationCallback) {
-            super(deprecationCallback);
-            this.in =  in;
-        }
-
         @Override
         public void setNextDocId(int docId) throws IOException {
             if (in.advanceExact(docId)) {
@@ -379,19 +305,17 @@ public double getLat() {
         }
 
         public double[] getLats() {
-            List<GeoPoint> points = getValues();
-            double[] lats = new double[points.size()];
-            for (int i = 0; i < points.size(); i++) {
-                lats[i] = points.get(i).lat();
+            double[] lats = new double[size()];
+            for (int i = 0; i < size(); i++) {
+                lats[i] = get(i).lat();
             }
             return lats;
         }
 
         public double[] getLons() {
-            List<GeoPoint> points = getValues();
-            double[] lons = new double[points.size()];
-            for (int i = 0; i < points.size(); i++) {
-                lons[i] = points.get(i).lon();
+            double[] lons = new double[size()];
+            for (int i = 0; i < size(); i++) {
+                lons[i] = get(i).lon();
             }
             return lons;
         }