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.bar` should be
using `doc.foo.bar` instead.

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

@@ -19,22 +19,27 @@
 
 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;
 
 /**
@@ -48,6 +53,25 @@
  */
 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.
      */
@@ -57,6 +81,8 @@ public abstract class ScriptDocValues<T> extends AbstractList<T> {
      * Return a copy of the list of the values for the current document.
      */
     public final List<T> getValues() {
+        deprecated("ScriptDocValues#getValues", "Deprecated getValues used, values should be accessed directly "
+                + "(e.g., doc.foo.bar instead of doc.foo.values.bar)");
         return this;
     }
 
@@ -86,6 +112,21 @@ 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];
@@ -98,6 +139,14 @@ 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)) {
@@ -155,6 +204,14 @@ 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.
@@ -273,6 +330,14 @@ 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)) {

server/src/test/java/org/elasticsearch/index/fielddata/ScriptDocValuesDatesTests.java

@@ -0,0 +1,93 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.index.fielddata;
+
+import org.elasticsearch.index.fielddata.ScriptDocValues.Dates;
+import org.elasticsearch.test.ESTestCase;
+
+import java.security.AccessControlContext;
+import java.security.AccessController;
+import java.security.PermissionCollection;
+import java.security.Permissions;
+import java.security.PrivilegedAction;
+import java.security.ProtectionDomain;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.function.BiConsumer;
+
+import static org.hamcrest.Matchers.contains;
+
+public class ScriptDocValuesDatesTests extends ESTestCase {
+
+    public void testGetValues() {
+        Set<String> keys = new HashSet<>();
+        Set<String> warnings = new HashSet<>();
+
+        Dates dates = biconsumerWrap((deprecationKey, deprecationMessage) -> {
+            keys.add(deprecationKey);
+            warnings.add(deprecationMessage);
+
+            // Create a temporary directory to prove we are running with the server's permissions.
+            createTempDir();
+        });
+
+        /*
+         * Invoke getValues() without any permissions to verify it still works.
+         * This is done using the callback created above, which creates a temp
+         * directory, which is not possible with "noPermission".
+         */
+        PermissionCollection noPermissions = new Permissions();
+        AccessControlContext noPermissionsAcc = new AccessControlContext(
+            new ProtectionDomain[] {
+                new ProtectionDomain(null, noPermissions)
+            }
+        );
+        AccessController.doPrivileged(new PrivilegedAction<Void>(){
+            public Void run() {
+                dates.getValues();
+                return null;
+            }
+        }, noPermissionsAcc);
+
+        assertThat(warnings, contains(
+            "Deprecated getValues used, values should be accessed directly (e.g., doc.foo.bar instead of doc.foo.values.bar)"));
+        assertThat(keys, contains("ScriptDocValues#getValues"));
+
+
+    }
+
+    private Dates biconsumerWrap(BiConsumer<String, String> deprecationHandler) {
+        return new Dates(new AbstractSortedNumericDocValues() {
+            @Override
+            public boolean advanceExact(int doc) {
+                return true;
+            }
+            @Override
+            public int docValueCount() {
+                return 0;
+            }
+            @Override
+            public long nextValue() {
+                return 0L;
+            }
+        }, deprecationHandler);
+    }
+}

server/src/test/java/org/elasticsearch/index/fielddata/ScriptDocValuesGeoPointsTests.java

@@ -21,10 +21,22 @@
 
 import org.elasticsearch.common.geo.GeoPoint;
 import org.elasticsearch.common.geo.GeoUtils;
+import org.elasticsearch.index.fielddata.ScriptDocValues.GeoPoints;
 import org.elasticsearch.test.ESTestCase;
 
 import java.io.IOException;
+import java.security.AccessControlContext;
+import java.security.AccessController;
+import java.security.PermissionCollection;
+import java.security.Permissions;
+import java.security.PrivilegedAction;
+import java.security.ProtectionDomain;
 import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.function.BiConsumer;
+
+import static org.hamcrest.Matchers.contains;
 
 public class ScriptDocValuesGeoPointsTests extends ESTestCase {
 
@@ -70,8 +82,18 @@ public void testGeoGetLatLon() throws IOException {
         final double lat2 = randomLat();
         final double lon1 = randomLon();
         final double lon2 = randomLon();
+
+        Set<String> warnings = new HashSet<>();
+        Set<String> keys = new HashSet<>();
+
         final MultiGeoPointValues values = wrap(new GeoPoint(lat1, lon1), new GeoPoint(lat2, lon2));
-        final ScriptDocValues.GeoPoints script = new ScriptDocValues.GeoPoints(values);
+        final ScriptDocValues.GeoPoints script = geoPointsWrap(values, (deprecationKey, deprecationMessage)  -> {
+            keys.add(deprecationKey);
+            warnings.add(deprecationMessage);
+
+            // Create a temporary directory to prove we are running with the server's permissions.
+            createTempDir();
+        });
         script.setNextDocId(1);
         assertEquals(true, script.isEmpty());
         script.setNextDocId(0);
@@ -82,6 +104,29 @@ public void testGeoGetLatLon() throws IOException {
         assertEquals(lon1, script.getLon(), 0);
         assertTrue(Arrays.equals(new double[] {lat1, lat2}, script.getLats()));
         assertTrue(Arrays.equals(new double[] {lon1, lon2}, script.getLons()));
+
+        /*
+         * Invoke getValues() without any permissions to verify it still works.
+         * This is done using the callback created above, which creates a temp
+         * directory, which is not possible with "noPermission".
+         */
+        PermissionCollection noPermissions = new Permissions();
+        AccessControlContext noPermissionsAcc = new AccessControlContext(
+            new ProtectionDomain[] {
+                new ProtectionDomain(null, noPermissions)
+            }
+        );
+        AccessController.doPrivileged(new PrivilegedAction<Void>(){
+            public Void run() {
+                script.getValues();
+                return null;
+            }
+        }, noPermissionsAcc);
+
+        assertThat(warnings, contains(
+            "Deprecated getValues used, values should be accessed directly (e.g., doc.foo.bar instead of doc.foo.values.bar)"));
+        assertThat(keys, contains("ScriptDocValues#getValues"));
+
     }
 
     public void testGeoDistance() throws IOException {
@@ -110,4 +155,8 @@ public void testGeoDistance() throws IOException {
         assertEquals(42, emptyScript.planeDistanceWithDefault(otherLat, otherLon, 42), 0);
     }
 
+    private GeoPoints geoPointsWrap(MultiGeoPointValues in, BiConsumer<String, String> deprecationHandler) {
+        return new GeoPoints(in, deprecationHandler);
+    }
+
 }

server/src/test/java/org/elasticsearch/index/fielddata/ScriptDocValuesLongsTests.java

@@ -23,6 +23,17 @@
 import org.elasticsearch.test.ESTestCase;
 
 import java.io.IOException;
+import java.security.AccessControlContext;
+import java.security.AccessController;
+import java.security.PermissionCollection;
+import java.security.Permissions;
+import java.security.PrivilegedAction;
+import java.security.ProtectionDomain;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.function.BiConsumer;
+
+import static org.hamcrest.Matchers.contains;
 
 public class ScriptDocValuesLongsTests extends ESTestCase {
     public void testLongs() throws IOException {
@@ -33,7 +44,17 @@ public void testLongs() throws IOException {
                 values[d][i] = randomLong();
             }
         }
-        Longs longs = wrap(values);
+
+        Set<String> warnings = new HashSet<>();
+        Set<String> keys = new HashSet<>();
+
+        Longs longs = wrap(values, (deprecationKey, deprecationMessage) -> {
+            keys.add(deprecationKey);
+            warnings.add(deprecationMessage);
+
+            // Create a temporary directory to prove we are running with the server's permissions.
+            createTempDir();
+        });
 
         for (int round = 0; round < 10; round++) {
             int d = between(0, values.length - 1);
@@ -55,9 +76,32 @@ public void testLongs() throws IOException {
             Exception e = expectThrows(UnsupportedOperationException.class, () -> longs.getValues().add(100L));
             assertEquals("doc values are unmodifiable", e.getMessage());
         }
+
+        /*
+         * Invoke getValues() without any permissions to verify it still works.
+         * This is done using the callback created above, which creates a temp
+         * directory, which is not possible with "noPermission".
+         */
+        PermissionCollection noPermissions = new Permissions();
+        AccessControlContext noPermissionsAcc = new AccessControlContext(
+            new ProtectionDomain[] {
+                new ProtectionDomain(null, noPermissions)
+            }
+        );
+        AccessController.doPrivileged(new PrivilegedAction<Void>(){
+            public Void run() {
+                longs.getValues();
+                return null;
+            }
+        }, noPermissionsAcc);
+
+        assertThat(warnings, contains(
+            "Deprecated getValues used, values should be accessed directly (e.g., doc.foo.bar instead of doc.foo.values.bar)"));
+        assertThat(keys, contains("ScriptDocValues#getValues"));
+
     }
 
-    private Longs wrap(long[][] values) {
+    private Longs wrap(long[][] values, BiConsumer<String, String> deprecationCallback) {
         return new Longs(new AbstractSortedNumericDocValues() {
             long[] current;
             int i;
@@ -76,6 +120,6 @@ public int docValueCount() {
             public long nextValue() {
                 return current[i++];
             }
-        });
+        }, deprecationCallback);
     }
 }