Fix NPE when `values` is omitted on percentile_ranks agg (#26046)

An array of values is required because there is no default (or
reasonable way to set a default).  But validation for values
only happens if it is actually set.  If the values param is omitted
entirely than the agg builder will NPE.

core/src/main/java/org/elasticsearch/common/xcontent/AbstractObjectParser.java

@@ -49,7 +49,7 @@ public abstract class AbstractObjectParser<Value, Context>
     /**
      * Declares named objects in the style of aggregations. These are named
      * inside and object like this:
-     * 
+     *
      * <pre>
      * <code>
      * {
@@ -62,7 +62,7 @@ public abstract class AbstractObjectParser<Value, Context>
      * }
      * </code>
      * </pre>
-     * 
+     *
      * Unlike the other version of this method, "ordered" mode (arrays of
      * objects) is not supported.
      *
@@ -82,7 +82,7 @@ public abstract class AbstractObjectParser<Value, Context>
     /**
      * Declares named objects in the style of highlighting's field element.
      * These are usually named inside and object like this:
-     * 
+     *
      * <pre>
      * <code>
      * {
@@ -96,9 +96,9 @@ public abstract class AbstractObjectParser<Value, Context>
      * }
      * </code>
      * </pre>
-     * 
+     *
      * but, when order is important, some may be written this way:
-     * 
+     *
      * <pre>
      * <code>
      * {
@@ -112,7 +112,7 @@ public abstract class AbstractObjectParser<Value, Context>
      * }
      * </code>
      * </pre>
-     * 
+     *
      * This is because json doesn't enforce ordering. Elasticsearch reads it in
      * the order sent but tools that generate json are free to put object
      * members in an unordered Map, jumbling them. Thus, if you care about order
@@ -134,6 +134,8 @@ public abstract class AbstractObjectParser<Value, Context>
     public abstract <T> void declareNamedObjects(BiConsumer<Value, List<T>> consumer, NamedObjectParser<T, Context> namedObjectParser,
             Consumer<Value> orderedModeCallback, ParseField parseField);
 
+    public abstract String getName();
+
     public <T> void declareField(BiConsumer<Value, T> consumer, CheckedFunction<XContentParser, T, IOException> parser,
             ParseField parseField, ValueType type) {
         if (parser == null) {

core/src/main/java/org/elasticsearch/common/xcontent/ConstructingObjectParser.java

@@ -295,6 +295,10 @@ public final class ConstructingObjectParser<Value, Context> extends AbstractObje
         }
     }
 
+    public String getName() {
+        return objectParser.getName();
+    }
+
     private Consumer<Target> wrapOrderedModeCallBack(Consumer<Value> callback) {
         return (target) -> {
             if (target.targetObject != null) {

core/src/main/java/org/elasticsearch/search/aggregations/AggregationBuilders.java

@@ -247,15 +247,15 @@ public class AggregationBuilders {
         return new SignificantTermsAggregationBuilder(name, null);
     }
 
-    
+
     /**
      * Create a new {@link SignificantTextAggregationBuilder} aggregation with the given name and text field name
      */
     public static SignificantTextAggregationBuilder significantText(String name, String fieldName) {
         return new SignificantTextAggregationBuilder(name, fieldName);
-    }    
-        
-    
+    }
+
+
     /**
      * Create a new {@link DateHistogramAggregationBuilder} aggregation with the given
      * name.
@@ -304,8 +304,8 @@ public class AggregationBuilders {
     /**
      * Create a new {@link PercentileRanks} aggregation with the given name.
      */
-    public static PercentileRanksAggregationBuilder percentileRanks(String name) {
-        return new PercentileRanksAggregationBuilder(name);
+    public static PercentileRanksAggregationBuilder percentileRanks(String name, double[] values) {
+        return new PercentileRanksAggregationBuilder(name, values);
     }
 
     /**

core/src/main/java/org/elasticsearch/search/aggregations/metrics/percentiles/PercentileRanksAggregationBuilder.java

@@ -22,6 +22,8 @@ package org.elasticsearch.search.aggregations.metrics.percentiles;
 import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
+import org.elasticsearch.common.xcontent.ConstructingObjectParser;
+import org.elasticsearch.common.xcontent.ContextParser;
 import org.elasticsearch.common.xcontent.ObjectParser;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentParser;
@@ -42,7 +44,11 @@ import org.elasticsearch.search.internal.SearchContext;
 
 import java.io.IOException;
 import java.util.Arrays;
+import java.util.List;
 import java.util.Objects;
+import java.util.function.BiConsumer;
+
+import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg;
 
 public class PercentileRanksAggregationBuilder extends LeafOnly<ValuesSource.Numeric, PercentileRanksAggregationBuilder> {
     public static final String NAME = PercentileRanks.TYPE_NAME;
@@ -53,7 +59,7 @@ public class PercentileRanksAggregationBuilder extends LeafOnly<ValuesSource.Num
         Double compression;
     }
 
-    private static final ObjectParser<TDigestOptions, Void> TDIGEST_OPTIONS_PARSER =
+    private static final ObjectParser<TDigestOptions, String> TDIGEST_OPTIONS_PARSER =
             new ObjectParser<>(PercentilesMethod.TDIGEST.getParseField().getPreferredName(), TDigestOptions::new);
     static {
         TDIGEST_OPTIONS_PARSER.declareDouble((opts, compression) -> opts.compression = compression, new ParseField("compression"));
@@ -63,22 +69,22 @@ public class PercentileRanksAggregationBuilder extends LeafOnly<ValuesSource.Num
         Integer numberOfSigDigits;
     }
 
-    private static final ObjectParser<HDROptions, Void> HDR_OPTIONS_PARSER =
+    private static final ObjectParser<HDROptions, String> HDR_OPTIONS_PARSER =
             new ObjectParser<>(PercentilesMethod.HDR.getParseField().getPreferredName(), HDROptions::new);
     static {
         HDR_OPTIONS_PARSER.declareInt((opts, numberOfSigDigits) -> opts.numberOfSigDigits = numberOfSigDigits,
                 new ParseField("number_of_significant_value_digits"));
     }
 
-    private static final ObjectParser<PercentileRanksAggregationBuilder, Void> PARSER;
+    // The builder requires two parameters for the constructor: aggregation name and values array.  The
+    // agg name is supplied externally via the Parser's context (as a String), while the values array
+    // is parsed from the request and supplied to the ConstructingObjectParser as a ctor argument
+    private static final ConstructingObjectParser<PercentileRanksAggregationBuilder, String> PARSER;
     static {
-        PARSER = new ObjectParser<>(PercentileRanksAggregationBuilder.NAME);
+        PARSER = new ConstructingObjectParser<>(PercentileRanksAggregationBuilder.NAME, false,
+            (a, context) -> new PercentileRanksAggregationBuilder(context, (List) a[0]));
         ValuesSourceParserHelper.declareNumericFields(PARSER, true, false, false);
-
-        PARSER.declareDoubleArray(
-                (b, v) -> b.values(v.stream().mapToDouble(Double::doubleValue).toArray()),
-                VALUES_FIELD);
-
+        PARSER.declareDoubleArray(constructorArg(), VALUES_FIELD);
         PARSER.declareBoolean(PercentileRanksAggregationBuilder::keyed, PercentilesAggregationBuilder.KEYED_FIELD);
 
         PARSER.declareField((b, v) -> {
@@ -97,7 +103,8 @@ public class PercentileRanksAggregationBuilder extends LeafOnly<ValuesSource.Num
     }
 
     public static AggregationBuilder parse(String aggregationName, XContentParser parser) throws IOException {
-        return PARSER.parse(parser, new PercentileRanksAggregationBuilder(aggregationName), null);
+        // the aggregation name is supplied to the parser as a Context. See note at top of Parser for more details
+        return PARSER.parse(parser, aggregationName);
     }
 
     private double[] values;
@@ -106,8 +113,21 @@ public class PercentileRanksAggregationBuilder extends LeafOnly<ValuesSource.Num
     private double compression = 100.0;
     private boolean keyed = true;
 
-    public PercentileRanksAggregationBuilder(String name) {
+    private PercentileRanksAggregationBuilder(String name, List<Double> values) {
+        this(name, values.stream().mapToDouble(Double::doubleValue).toArray());
+    }
+
+    public PercentileRanksAggregationBuilder(String name, double[] values) {
         super(name, ValuesSourceType.NUMERIC, ValueType.NUMERIC);
+        if (values == null) {
+            throw new IllegalArgumentException("[values] must not be null: [" + name + "]");
+        }
+        if (values.length == 0) {
+            throw new IllegalArgumentException("[values] must not be an empty array: [" + name + "]");
+        }
+        double[] sortedValues = Arrays.copyOf(values, values.length);
+        Arrays.sort(sortedValues);
+        this.values = sortedValues;
     }
 
     /**
@@ -131,19 +151,6 @@ public class PercentileRanksAggregationBuilder extends LeafOnly<ValuesSource.Num
         method.writeTo(out);
     }
 
-    /**
-     * Set the values to compute percentiles from.
-     */
-    public PercentileRanksAggregationBuilder values(double... values) {
-        if (values == null) {
-            throw new IllegalArgumentException("[values] must not be null: [" + name + "]");
-        }
-        double[] sortedValues = Arrays.copyOf(values, values.length);
-        Arrays.sort(sortedValues);
-        this.values = sortedValues;
-        return this;
-    }
-
     /**
      * Get the values to compute percentiles from.
      */

core/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceParserHelper.java

@@ -21,6 +21,8 @@ package org.elasticsearch.search.aggregations.support;
 
 import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.ParsingException;
+import org.elasticsearch.common.xcontent.AbstractObjectParser;
+import org.elasticsearch.common.xcontent.ConstructingObjectParser;
 import org.elasticsearch.common.xcontent.ObjectParser;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.script.Script;
@@ -31,32 +33,32 @@ public final class ValuesSourceParserHelper {
 
     private ValuesSourceParserHelper() {} // utility class, no instantiation
 
-    public static void declareAnyFields(
-            ObjectParser<? extends ValuesSourceAggregationBuilder<ValuesSource, ?>, Void> objectParser,
+    public static <T> void declareAnyFields(
+            AbstractObjectParser<? extends ValuesSourceAggregationBuilder<ValuesSource, ?>, T> objectParser,
             boolean scriptable, boolean formattable) {
         declareFields(objectParser, scriptable, formattable, false, null);
     }
 
-    public static void declareNumericFields(
-            ObjectParser<? extends ValuesSourceAggregationBuilder<ValuesSource.Numeric, ?>, Void> objectParser,
+    public static <T> void declareNumericFields(
+            AbstractObjectParser<? extends ValuesSourceAggregationBuilder<ValuesSource.Numeric, ?>, T> objectParser,
             boolean scriptable, boolean formattable, boolean timezoneAware) {
         declareFields(objectParser, scriptable, formattable, timezoneAware, ValueType.NUMERIC);
     }
 
-    public static void declareBytesFields(
-            ObjectParser<? extends ValuesSourceAggregationBuilder<ValuesSource.Bytes, ?>, Void> objectParser,
+    public static <T> void declareBytesFields(
+            AbstractObjectParser<? extends ValuesSourceAggregationBuilder<ValuesSource.Bytes, ?>, T> objectParser,
             boolean scriptable, boolean formattable) {
         declareFields(objectParser, scriptable, formattable, false, ValueType.STRING);
     }
 
-    public static void declareGeoFields(
-            ObjectParser<? extends ValuesSourceAggregationBuilder<ValuesSource.GeoPoint, ?>, Void> objectParser,
+    public static <T> void declareGeoFields(
+            AbstractObjectParser<? extends ValuesSourceAggregationBuilder<ValuesSource.GeoPoint, ?>, T> objectParser,
             boolean scriptable, boolean formattable) {
         declareFields(objectParser, scriptable, formattable, false, ValueType.GEOPOINT);
     }
 
-    private static <VS extends ValuesSource> void declareFields(
-            ObjectParser<? extends ValuesSourceAggregationBuilder<VS, ?>, Void> objectParser,
+    private static <VS extends ValuesSource, T> void declareFields(
+            AbstractObjectParser<? extends ValuesSourceAggregationBuilder<VS, ?>, T> objectParser,
             boolean scriptable, boolean formattable, boolean timezoneAware, ValueType targetValueType) {
 
 
@@ -99,4 +101,6 @@ public final class ValuesSourceParserHelper {
         }
     }
 
+
+
 }

core/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentileRanksIT.java

@@ -128,8 +128,9 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase {
                                 .interval(1L)
                                 .minDocCount(0)
                                 .subAggregation(
-                                        percentileRanks("percentile_ranks").field("value").method(PercentilesMethod.HDR)
-                                        .numberOfSignificantValueDigits(sigDigits).values(10, 15)))
+                                        percentileRanks("percentile_ranks", new double[]{10, 15})
+                                            .field("value").method(PercentilesMethod.HDR)
+                                            .numberOfSignificantValueDigits(sigDigits)))
                 .execute().actionGet();
 
         assertThat(searchResponse.getHits().getTotalHits(), equalTo(2L));
@@ -152,8 +153,10 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase {
                 .prepareSearch("idx_unmapped")
                 .setQuery(matchAllQuery())
                 .addAggregation(
-                        percentileRanks("percentile_ranks").method(PercentilesMethod.HDR).numberOfSignificantValueDigits(sigDigits)
-                        .field("value").values(0, 10, 15, 100))
+                        percentileRanks("percentile_ranks", new double[]{0, 10, 15, 100})
+                            .method(PercentilesMethod.HDR)
+                            .numberOfSignificantValueDigits(sigDigits)
+                        .field("value"))
                 .execute().actionGet();
 
         assertThat(searchResponse.getHits().getTotalHits(), equalTo(0L));
@@ -175,8 +178,8 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase {
                 .prepareSearch("idx")
                 .setQuery(matchAllQuery())
                 .addAggregation(
-                        percentileRanks("percentile_ranks").method(PercentilesMethod.HDR).numberOfSignificantValueDigits(sigDigits)
-                        .field("value").values(pcts))
+                        percentileRanks("percentile_ranks", pcts).method(PercentilesMethod.HDR).numberOfSignificantValueDigits(sigDigits)
+                        .field("value"))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 10);
@@ -185,6 +188,36 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase {
         assertConsistent(pcts, values, minValue, maxValue, sigDigits);
     }
 
+    public void testNullValuesField() throws Exception {
+        int sigDigits = randomSignificantDigits();
+        final double[] pcts = null;
+        IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> client()
+            .prepareSearch("idx")
+            .setQuery(matchAllQuery())
+            .addAggregation(
+                percentileRanks("percentile_ranks", pcts)
+                    .method(PercentilesMethod.HDR)
+                    .numberOfSignificantValueDigits(sigDigits)
+                    .field("value"))
+            .execute().actionGet());
+        assertThat(e.getMessage(), equalTo("[values] must not be null: [percentile_ranks]"));
+    }
+
+    public void testEmptyValuesField() throws Exception {
+        int sigDigits = randomSignificantDigits();
+        final double[] pcts = new double[0];
+        IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> client()
+            .prepareSearch("idx")
+            .setQuery(matchAllQuery())
+            .addAggregation(
+                percentileRanks("percentile_ranks", pcts)
+                    .method(PercentilesMethod.HDR)
+                    .numberOfSignificantValueDigits(sigDigits)
+                    .field("value"))
+            .execute().actionGet());
+        assertThat(e.getMessage(), equalTo("[values] must not be an empty array: [percentile_ranks]"));
+    }
+
     @Override
     public void testSingleValuedFieldGetProperty() throws Exception {
         int sigDigits = randomSignificantDigits();
@@ -194,8 +227,10 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase {
                 .setQuery(matchAllQuery())
                 .addAggregation(
                         global("global").subAggregation(
-                                percentileRanks("percentile_ranks").method(PercentilesMethod.HDR).numberOfSignificantValueDigits(sigDigits)
-                                .field("value").values(pcts)))
+                                percentileRanks("percentile_ranks", pcts)
+                                    .method(PercentilesMethod.HDR)
+                                    .numberOfSignificantValueDigits(sigDigits)
+                                .field("value")))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 10);
@@ -221,8 +256,8 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase {
                 .prepareSearch("idx")
                 .setQuery(matchAllQuery())
                 .addAggregation(
-                        percentileRanks("percentile_ranks").method(PercentilesMethod.HDR).numberOfSignificantValueDigits(sigDigits)
-                        .field("value").values(pcts))
+                        percentileRanks("percentile_ranks", pcts).method(PercentilesMethod.HDR).numberOfSignificantValueDigits(sigDigits)
+                        .field("value"))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 10);
@@ -239,8 +274,8 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase {
                 .prepareSearch("idx", "idx_unmapped")
                 .setQuery(matchAllQuery())
                 .addAggregation(
-                        percentileRanks("percentile_ranks").method(PercentilesMethod.HDR).numberOfSignificantValueDigits(sigDigits)
-                        .field("value").values(pcts))
+                        percentileRanks("percentile_ranks", pcts).method(PercentilesMethod.HDR).numberOfSignificantValueDigits(sigDigits)
+                        .field("value"))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 10);
@@ -257,12 +292,11 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase {
                 .prepareSearch("idx")
                 .setQuery(matchAllQuery())
                 .addAggregation(
-                        percentileRanks("percentile_ranks")
+                        percentileRanks("percentile_ranks", pcts)
                                 .method(PercentilesMethod.HDR)
                                 .numberOfSignificantValueDigits(sigDigits)
                                 .field("value")
-                                .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - 1", emptyMap()))
-                                .values(pcts))
+                                .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - 1", emptyMap())))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 10);
@@ -281,12 +315,11 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase {
                 .prepareSearch("idx")
                 .setQuery(matchAllQuery())
                 .addAggregation(
-                        percentileRanks("percentile_ranks")
+                        percentileRanks("percentile_ranks", pcts)
                                 .method(PercentilesMethod.HDR)
                                 .numberOfSignificantValueDigits(sigDigits)
                                 .field("value")
-                                .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - dec", params))
-                                .values(pcts))
+                                .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - dec", params)))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 10);
@@ -303,8 +336,8 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase {
                 .prepareSearch("idx")
                 .setQuery(matchAllQuery())
                 .addAggregation(
-                        percentileRanks("percentile_ranks").method(PercentilesMethod.HDR).numberOfSignificantValueDigits(sigDigits)
-                        .field("values").values(pcts))
+                        percentileRanks("percentile_ranks", pcts).method(PercentilesMethod.HDR).numberOfSignificantValueDigits(sigDigits)
+                        .field("values"))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 10);
@@ -321,12 +354,11 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase {
                 .prepareSearch("idx")
                 .setQuery(matchAllQuery())
                 .addAggregation(
-                        percentileRanks("percentile_ranks")
+                        percentileRanks("percentile_ranks", pcts)
                                 .method(PercentilesMethod.HDR)
                                 .numberOfSignificantValueDigits(sigDigits)
                                 .field("values")
-                                .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - 1", emptyMap()))
-                                .values(pcts))
+                                .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - 1", emptyMap())))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 10);
@@ -342,12 +374,11 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase {
                 .prepareSearch("idx")
                 .setQuery(matchAllQuery())
                 .addAggregation(
-                        percentileRanks("percentile_ranks")
+                        percentileRanks("percentile_ranks", pcts)
                                 .method(PercentilesMethod.HDR)
                                 .numberOfSignificantValueDigits(sigDigits)
                                 .field("values")
-                                .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "20 - _value", emptyMap()))
-                                .values(pcts))
+                                .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "20 - _value", emptyMap())))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 10);
@@ -366,12 +397,11 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase {
                 .prepareSearch("idx")
                 .setQuery(matchAllQuery())
                 .addAggregation(
-                        percentileRanks("percentile_ranks")
+                        percentileRanks("percentile_ranks", pcts)
                                 .method(PercentilesMethod.HDR)
                                 .numberOfSignificantValueDigits(sigDigits)
                                 .field("values")
-                                .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - dec", params))
-                                .values(pcts))
+                                .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - dec", params)))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 10);
@@ -388,11 +418,10 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase {
                 .prepareSearch("idx")
                 .setQuery(matchAllQuery())
                 .addAggregation(
-                        percentileRanks("percentile_ranks")
+                        percentileRanks("percentile_ranks", pcts)
                                 .method(PercentilesMethod.HDR)
                                 .numberOfSignificantValueDigits(sigDigits)
-                                .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "doc['value'].value", emptyMap()))
-                                .values(pcts))
+                                .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "doc['value'].value", emptyMap())))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 10);
@@ -414,11 +443,10 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase {
                 .prepareSearch("idx")
                 .setQuery(matchAllQuery())
                 .addAggregation(
-                        percentileRanks("percentile_ranks")
+                        percentileRanks("percentile_ranks", pcts)
                                 .method(PercentilesMethod.HDR)
                                 .numberOfSignificantValueDigits(sigDigits)
-                                .script(script)
-                                .values(pcts))
+                                .script(script))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 10);
@@ -438,11 +466,10 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase {
                 .prepareSearch("idx")
                 .setQuery(matchAllQuery())
                 .addAggregation(
-                        percentileRanks("percentile_ranks")
+                        percentileRanks("percentile_ranks", pcts)
                                 .method(PercentilesMethod.HDR)
                                 .numberOfSignificantValueDigits(sigDigits)
-                                .script(script)
-                                .values(pcts))
+                                .script(script))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 10);
@@ -461,11 +488,10 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase {
                 .prepareSearch("idx")
                 .setQuery(matchAllQuery())
                 .addAggregation(
-                        percentileRanks("percentile_ranks")
+                        percentileRanks("percentile_ranks", pcts)
                                 .method(PercentilesMethod.HDR)
                                 .numberOfSignificantValueDigits(sigDigits)
-                                .script(script)
-                                .values(pcts))
+                                .script(script))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 10);
@@ -483,8 +509,8 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase {
                 .addAggregation(
                         histogram("histo").field("value").interval(2L)
                                 .subAggregation(
-                                        percentileRanks("percentile_ranks").field("value").method(PercentilesMethod.HDR)
-                                .numberOfSignificantValueDigits(sigDigits).values(99))
+                                        percentileRanks("percentile_ranks", new double[]{99}).field("value").method(PercentilesMethod.HDR)
+                                .numberOfSignificantValueDigits(sigDigits))
                                 .order(BucketOrder.aggregation("percentile_ranks", "99", asc))).execute().actionGet();
 
         assertHitCount(searchResponse, 10);
@@ -508,7 +534,7 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase {
         SearchResponse searchResponse = client().prepareSearch("idx").setQuery(matchAllQuery())
                 .addAggregation(terms("terms").field("value").order(BucketOrder.compound(BucketOrder.aggregation("filter>ranks.99", true)))
                         .subAggregation(filter("filter", termQuery("value", 100))
-                                .subAggregation(percentileRanks("ranks").method(PercentilesMethod.HDR).values(99).field("value"))))
+                                .subAggregation(percentileRanks("ranks", new double[]{99}).method(PercentilesMethod.HDR).field("value"))))
                 .get();
 
         assertHitCount(searchResponse, 10);
@@ -553,8 +579,10 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase {
 
         // Test that a request using a script does not get cached
         SearchResponse r = client()
-                .prepareSearch("cache_test_idx").setSize(0).addAggregation(percentileRanks("foo").method(PercentilesMethod.HDR).field("d")
-                        .values(50.0).script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - 1", emptyMap())))
+                .prepareSearch("cache_test_idx").setSize(0)
+                    .addAggregation(percentileRanks("foo", new double[]{50.0})
+                        .method(PercentilesMethod.HDR).field("d")
+                        .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - 1", emptyMap())))
                 .get();
         assertSearchResponse(r);
 
@@ -566,7 +594,7 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase {
         // To make sure that the cache is working test that a request not using
         // a script is cached
         r = client().prepareSearch("cache_test_idx").setSize(0)
-                .addAggregation(percentileRanks("foo").method(PercentilesMethod.HDR).field("d").values(50.0)).get();
+                .addAggregation(percentileRanks("foo", new double[]{50.0}).method(PercentilesMethod.HDR).field("d")).get();
         assertSearchResponse(r);
 
         assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache()

core/src/test/java/org/elasticsearch/search/aggregations/metrics/PercentileRanksTests.java

@@ -19,7 +19,6 @@
 
 package org.elasticsearch.search.aggregations.metrics;
 
-import org.elasticsearch.script.Script;
 import org.elasticsearch.search.aggregations.BaseAggregationTestCase;
 import org.elasticsearch.search.aggregations.metrics.percentiles.PercentileRanksAggregationBuilder;
 
@@ -27,16 +26,16 @@ public class PercentileRanksTests extends BaseAggregationTestCase<PercentileRank
 
     @Override
     protected PercentileRanksAggregationBuilder createTestAggregatorBuilder() {
-        PercentileRanksAggregationBuilder factory = new PercentileRanksAggregationBuilder(randomAlphaOfLengthBetween(1, 20));
-        if (randomBoolean()) {
-            factory.keyed(randomBoolean());
-        }
         int valuesSize = randomIntBetween(1, 20);
         double[] values = new double[valuesSize];
         for (int i = 0; i < valuesSize; i++) {
             values[i] = randomDouble() * 100;
         }
-        factory.values(values);
+        PercentileRanksAggregationBuilder factory = new PercentileRanksAggregationBuilder(randomAlphaOfLengthBetween(1, 20), values);
+        if (randomBoolean()) {
+            factory.keyed(randomBoolean());
+        }
+
         if (randomBoolean()) {
             factory.numberOfSignificantValueDigits(randomIntBetween(0, 5));
         }

core/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentileRanksIT.java

@@ -124,8 +124,7 @@ public class TDigestPercentileRanksIT extends AbstractNumericTestCase {
         SearchResponse searchResponse = client().prepareSearch("empty_bucket_idx")
                 .setQuery(matchAllQuery())
                 .addAggregation(histogram("histo").field("value").interval(1L).minDocCount(0)
-                        .subAggregation(randomCompression(percentileRanks("percentile_ranks").field("value"))
-                                .values(10, 15)))
+                        .subAggregation(randomCompression(percentileRanks("percentile_ranks", new double[]{10,15}).field("value"))))
                 .execute().actionGet();
 
         assertThat(searchResponse.getHits().getTotalHits(), equalTo(2L));
@@ -141,13 +140,34 @@ public class TDigestPercentileRanksIT extends AbstractNumericTestCase {
         assertThat(reversePercentiles.percent(15), equalTo(Double.NaN));
     }
 
+    public void testNullValuesField() throws Exception {
+        final double[] pcts = null;
+        IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> client()
+            .prepareSearch("idx")
+            .setQuery(matchAllQuery())
+            .addAggregation(
+                percentileRanks("percentile_ranks", pcts).method(PercentilesMethod.TDIGEST).field("value"))
+            .execute().actionGet());
+        assertThat(e.getMessage(), equalTo("[values] must not be null: [percentile_ranks]"));
+    }
+
+    public void testEmptyValuesField() throws Exception {
+        final double[] pcts = new double[0];
+        IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> client()
+            .prepareSearch("idx")
+            .setQuery(matchAllQuery())
+            .addAggregation(
+                percentileRanks("percentile_ranks", pcts).method(PercentilesMethod.TDIGEST).field("value"))
+            .execute().actionGet());
+        assertThat(e.getMessage(), equalTo("[values] must not be an empty array: [percentile_ranks]"));
+    }
+
     @Override
     public void testUnmapped() throws Exception {
         SearchResponse searchResponse = client().prepareSearch("idx_unmapped")
                 .setQuery(matchAllQuery())
-                .addAggregation(randomCompression(percentileRanks("percentile_ranks"))
-                        .field("value")
-                        .values(0, 10, 15, 100))
+                .addAggregation(randomCompression(percentileRanks("percentile_ranks", new double[]{0, 10, 15, 100}))
+                        .field("value"))
                 .execute().actionGet();
 
         assertThat(searchResponse.getHits().getTotalHits(), equalTo(0L));
@@ -166,9 +186,8 @@ public class TDigestPercentileRanksIT extends AbstractNumericTestCase {
         final double[] pcts = randomPercents(minValue, maxValue);
         SearchResponse searchResponse = client().prepareSearch("idx")
                 .setQuery(matchAllQuery())
-                .addAggregation(randomCompression(percentileRanks("percentile_ranks"))
-                        .field("value")
-                        .values(pcts))
+                .addAggregation(randomCompression(percentileRanks("percentile_ranks", pcts))
+                        .field("value"))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 10);
@@ -185,7 +204,7 @@ public class TDigestPercentileRanksIT extends AbstractNumericTestCase {
                 .setQuery(matchAllQuery())
                 .addAggregation(
                         global("global").subAggregation(
-                                randomCompression(percentileRanks("percentile_ranks")).field("value").values(pcts))).execute()
+                                randomCompression(percentileRanks("percentile_ranks", pcts)).field("value"))).execute()
                 .actionGet();
 
         assertHitCount(searchResponse, 10);
@@ -207,9 +226,8 @@ public class TDigestPercentileRanksIT extends AbstractNumericTestCase {
         final double[] pcts = new double[] {minValue - 1, maxValue + 1};
         SearchResponse searchResponse = client().prepareSearch("idx")
                 .setQuery(matchAllQuery())
-                .addAggregation(randomCompression(percentileRanks("percentile_ranks"))
-                        .field("value")
-                        .values(pcts))
+                .addAggregation(randomCompression(percentileRanks("percentile_ranks", pcts))
+                        .field("value"))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 10);
@@ -223,9 +241,8 @@ public class TDigestPercentileRanksIT extends AbstractNumericTestCase {
         final double[] pcts = randomPercents(minValue, maxValue);
         SearchResponse searchResponse = client().prepareSearch("idx", "idx_unmapped")
                 .setQuery(matchAllQuery())
-                .addAggregation(randomCompression(percentileRanks("percentile_ranks"))
-                        .field("value")
-                        .values(pcts))
+                .addAggregation(randomCompression(percentileRanks("percentile_ranks", pcts))
+                        .field("value"))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 10);
@@ -241,10 +258,9 @@ public class TDigestPercentileRanksIT extends AbstractNumericTestCase {
                 .setQuery(matchAllQuery())
                 .addAggregation(
                         randomCompression(
-                                percentileRanks("percentile_ranks"))
+                                percentileRanks("percentile_ranks", pcts))
                                 .field("value")
-                                .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - 1", emptyMap()))
-                                .values(pcts))
+                                .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - 1", emptyMap())))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 10);
@@ -262,10 +278,9 @@ public class TDigestPercentileRanksIT extends AbstractNumericTestCase {
                 .setQuery(matchAllQuery())
                 .addAggregation(
                         randomCompression(
-                                percentileRanks("percentile_ranks"))
+                                percentileRanks("percentile_ranks", pcts))
                                 .field("value")
-                                .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - dec", params))
-                                .values(pcts))
+                                .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - dec", params)))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 10);
@@ -279,9 +294,8 @@ public class TDigestPercentileRanksIT extends AbstractNumericTestCase {
         final double[] pcts = randomPercents(minValues, maxValues);
         SearchResponse searchResponse = client().prepareSearch("idx")
                 .setQuery(matchAllQuery())
-                .addAggregation(randomCompression(percentileRanks("percentile_ranks"))
-                        .field("values")
-                        .values(pcts))
+                .addAggregation(randomCompression(percentileRanks("percentile_ranks", pcts))
+                        .field("values"))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 10);
@@ -297,10 +311,9 @@ public class TDigestPercentileRanksIT extends AbstractNumericTestCase {
                 .setQuery(matchAllQuery())
                 .addAggregation(
                         randomCompression(
-                                percentileRanks("percentile_ranks"))
+                                percentileRanks("percentile_ranks", pcts))
                                 .field("values")
-                                .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - 1", emptyMap()))
-                                .values(pcts))
+                                .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - 1", emptyMap())))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 10);
@@ -315,10 +328,9 @@ public class TDigestPercentileRanksIT extends AbstractNumericTestCase {
                 .setQuery(matchAllQuery())
                 .addAggregation(
                         randomCompression(
-                                percentileRanks("percentile_ranks"))
+                                percentileRanks("percentile_ranks", pcts))
                                 .field("values")
-                                .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value * -1", emptyMap()))
-                                .values(pcts))
+                                .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value * -1", emptyMap())))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 10);
@@ -336,10 +348,9 @@ public class TDigestPercentileRanksIT extends AbstractNumericTestCase {
                 .setQuery(matchAllQuery())
                 .addAggregation(
                         randomCompression(
-                                percentileRanks("percentile_ranks"))
+                                percentileRanks("percentile_ranks", pcts))
                                 .field("values")
-                                .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - dec", params))
-                                .values(pcts))
+                                .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - dec", params)))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 10);
@@ -355,9 +366,8 @@ public class TDigestPercentileRanksIT extends AbstractNumericTestCase {
                 .setQuery(matchAllQuery())
                 .addAggregation(
                         randomCompression(
-                                percentileRanks("percentile_ranks"))
-                                .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "doc['value'].value", emptyMap()))
-                                .values(pcts))
+                                percentileRanks("percentile_ranks", pcts))
+                                .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "doc['value'].value", emptyMap())))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 10);
@@ -378,9 +388,7 @@ public class TDigestPercentileRanksIT extends AbstractNumericTestCase {
                 .setQuery(matchAllQuery())
                 .addAggregation(
                         randomCompression(
-                                percentileRanks("percentile_ranks"))
-                                .script(script)
-                                .values(pcts))
+                                percentileRanks("percentile_ranks", pcts)).script(script))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 10);
@@ -397,9 +405,8 @@ public class TDigestPercentileRanksIT extends AbstractNumericTestCase {
                 .setQuery(matchAllQuery())
                 .addAggregation(
                         randomCompression(
-                                percentileRanks("percentile_ranks"))
-                                .script(script)
-                                .values(pcts))
+                                percentileRanks("percentile_ranks", pcts))
+                                .script(script))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 10);
@@ -417,9 +424,8 @@ public class TDigestPercentileRanksIT extends AbstractNumericTestCase {
                 .setQuery(matchAllQuery())
                 .addAggregation(
                         randomCompression(
-                                percentileRanks("percentile_ranks"))
-                                .script(script)
-                                .values(pcts))
+                                percentileRanks("percentile_ranks", pcts))
+                                .script(script))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 10);
@@ -434,7 +440,7 @@ public class TDigestPercentileRanksIT extends AbstractNumericTestCase {
                 .setQuery(matchAllQuery())
                 .addAggregation(
                         histogram("histo").field("value").interval(2L)
-                            .subAggregation(randomCompression(percentileRanks("percentile_ranks").field("value").values(99)))
+                            .subAggregation(randomCompression(percentileRanks("percentile_ranks", new double[]{99}).field("value")))
                             .order(BucketOrder.aggregation("percentile_ranks", "99", asc)))
                 .execute().actionGet();
 
@@ -459,7 +465,8 @@ public class TDigestPercentileRanksIT extends AbstractNumericTestCase {
         SearchResponse searchResponse = client().prepareSearch("idx").setQuery(matchAllQuery())
                 .addAggregation(terms("terms").field("value").order(BucketOrder.compound(BucketOrder.aggregation("filter>ranks.99", true)))
                         .subAggregation(filter("filter", termQuery("value", 100))
-                                .subAggregation(percentileRanks("ranks").method(PercentilesMethod.TDIGEST).values(99).field("value"))))
+                                .subAggregation(percentileRanks("ranks", new double[]{99})
+                                    .method(PercentilesMethod.TDIGEST).field("value"))))
                 .get();
 
         assertHitCount(searchResponse, 10);
@@ -503,7 +510,9 @@ public class TDigestPercentileRanksIT extends AbstractNumericTestCase {
                 .getMissCount(), equalTo(0L));
 
         // Test that a request using a script does not get cached
-        SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(percentileRanks("foo").field("d").values(50.0)
+        SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0)
+            .addAggregation(percentileRanks("foo", new double[]{50.0})
+                .field("d")
                 .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - 1", emptyMap()))).get();
         assertSearchResponse(r);
 
@@ -514,7 +523,7 @@ public class TDigestPercentileRanksIT extends AbstractNumericTestCase {
 
         // To make sure that the cache is working test that a request not using
         // a script is cached
-        r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(percentileRanks("foo").field("d").values(50.0)).get();
+        r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(percentileRanks("foo", new double[]{50.0}).field("d")).get();
         assertSearchResponse(r);
 
         assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache()

core/src/test/java/org/elasticsearch/search/aggregations/metrics/percentiles/hdr/HDRPercentileRanksAggregatorTests.java

@@ -40,13 +40,14 @@ import org.hamcrest.Matchers;
 import java.io.IOException;
 import java.util.Iterator;
 
+import static org.hamcrest.core.IsEqual.equalTo;
+
 public class HDRPercentileRanksAggregatorTests extends AggregatorTestCase {
 
     public void testEmpty() throws IOException {
-        PercentileRanksAggregationBuilder aggBuilder = new PercentileRanksAggregationBuilder("my_agg")
+        PercentileRanksAggregationBuilder aggBuilder = new PercentileRanksAggregationBuilder("my_agg", new double[]{0.5})
                 .field("field")
-                .method(PercentilesMethod.HDR)
-                .values(0.5);
+                .method(PercentilesMethod.HDR);
         MappedFieldType fieldType = new NumberFieldMapper.NumberFieldType(NumberFieldMapper.NumberType.DOUBLE);
         fieldType.setName("field");
         try (IndexReader reader = new MultiReader()) {
@@ -67,10 +68,9 @@ public class HDRPercentileRanksAggregatorTests extends AggregatorTestCase {
                 w.addDocument(doc);
             }
 
-            PercentileRanksAggregationBuilder aggBuilder = new PercentileRanksAggregationBuilder("my_agg")
+            PercentileRanksAggregationBuilder aggBuilder = new PercentileRanksAggregationBuilder("my_agg", new double[]{0.1, 0.5, 12})
                     .field("field")
-                    .method(PercentilesMethod.HDR)
-                    .values(0.1, 0.5, 12);
+                    .method(PercentilesMethod.HDR);
             MappedFieldType fieldType = new NumberFieldMapper.NumberFieldType(NumberFieldMapper.NumberType.DOUBLE);
             fieldType.setName("field");
             try (IndexReader reader = w.getReader()) {
@@ -91,4 +91,17 @@ public class HDRPercentileRanksAggregatorTests extends AggregatorTestCase {
             }
         }
     }
+
+    public void testNullValues() throws IOException {
+        IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
+            () -> new PercentileRanksAggregationBuilder("my_agg", null).field("field").method(PercentilesMethod.HDR));
+        assertThat(e.getMessage(), Matchers.equalTo("[values] must not be null: [my_agg]"));
+    }
+
+    public void testEmptyValues() throws IOException {
+        IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
+            () -> new PercentileRanksAggregationBuilder("my_agg", new double[0]).field("field").method(PercentilesMethod.HDR));
+
+        assertThat(e.getMessage(), Matchers.equalTo("[values] must not be an empty array: [my_agg]"));
+    }
 }

core/src/test/java/org/elasticsearch/search/aggregations/metrics/percentiles/tdigest/TDigestPercentileRanksAggregatorTests.java

@@ -40,13 +40,14 @@ import org.hamcrest.Matchers;
 import java.io.IOException;
 import java.util.Iterator;
 
+import static org.hamcrest.core.IsEqual.equalTo;
+
 public class TDigestPercentileRanksAggregatorTests extends AggregatorTestCase {
 
     public void testEmpty() throws IOException {
-        PercentileRanksAggregationBuilder aggBuilder = new PercentileRanksAggregationBuilder("my_agg")
+        PercentileRanksAggregationBuilder aggBuilder = new PercentileRanksAggregationBuilder("my_agg", new double[]{0.5})
                 .field("field")
-                .method(PercentilesMethod.TDIGEST)
-                .values(0.5);
+                .method(PercentilesMethod.TDIGEST);
         MappedFieldType fieldType = new NumberFieldMapper.NumberFieldType(NumberFieldMapper.NumberType.DOUBLE);
         fieldType.setName("field");
         try (IndexReader reader = new MultiReader()) {
@@ -67,10 +68,9 @@ public class TDigestPercentileRanksAggregatorTests extends AggregatorTestCase {
                 w.addDocument(doc);
             }
 
-            PercentileRanksAggregationBuilder aggBuilder = new PercentileRanksAggregationBuilder("my_agg")
+            PercentileRanksAggregationBuilder aggBuilder = new PercentileRanksAggregationBuilder("my_agg", new double[]{0.1, 0.5, 12})
                     .field("field")
-                    .method(PercentilesMethod.TDIGEST)
-                    .values(0.1, 0.5, 12);
+                    .method(PercentilesMethod.TDIGEST);
             MappedFieldType fieldType = new NumberFieldMapper.NumberFieldType(NumberFieldMapper.NumberType.DOUBLE);
             fieldType.setName("field");
             try (IndexReader reader = w.getReader()) {
@@ -95,4 +95,17 @@ public class TDigestPercentileRanksAggregatorTests extends AggregatorTestCase {
             }
         }
     }
+
+    public void testNullValues() throws IOException {
+        IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
+            () -> new PercentileRanksAggregationBuilder("my_agg", null).field("field").method(PercentilesMethod.TDIGEST));
+        assertThat(e.getMessage(), Matchers.equalTo("[values] must not be null: [my_agg]"));
+    }
+
+    public void testEmptyValues() throws IOException {
+        IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
+            () -> new PercentileRanksAggregationBuilder("my_agg", new double[0]).field("field").method(PercentilesMethod.TDIGEST));
+
+        assertThat(e.getMessage(), Matchers.equalTo("[values] must not be an empty array: [my_agg]"));
+    }
 }

docs/reference/migration/migrate_6_0/java.asciidoc

@@ -56,3 +56,9 @@ The `org.elasticsearch.search.aggregations.bucket.terms.support` package was rem
 `org.elasticsearch.search.aggregations.bucket.terms`.
 
 The filter aggregation classes were moved to `org.elasticsearch.search.aggregations.bucket.filter`
+
+=== Constructor for `PercentileRanksAggregationBuilder` has changed
+
+It is now required to include the desired ranks as a non-null, non-empty array of doubles to the builder's constructor,
+rather than configuring them via a setter on the builder. The setter method `values()` has correspondingly
+been removed.