VS refactoring: convert Boxplot to new registry

x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/AnalyticsPlugin.java

@@ -75,7 +75,8 @@ public List<AggregationSpec> getAggregations() {
                 BoxplotAggregationBuilder.NAME,
                 BoxplotAggregationBuilder::new,
                 BoxplotAggregationBuilder.PARSER)
-                .addResultReader(InternalBoxplot::new),
+                .addResultReader(InternalBoxplot::new)
+                .setAggregatorRegistrar(BoxplotAggregationBuilder::registerAggregators),
             new AggregationSpec(
                 TopMetricsAggregationBuilder.NAME,
                 TopMetricsAggregationBuilder::new,

x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/boxplot/BoxplotAggregationBuilder.java

@@ -20,6 +20,7 @@
 import org.elasticsearch.search.aggregations.support.ValuesSourceAggregationBuilder;
 import org.elasticsearch.search.aggregations.support.ValuesSourceConfig;
 import org.elasticsearch.search.aggregations.support.ValuesSourceParserHelper;
+import org.elasticsearch.search.aggregations.support.ValuesSourceRegistry;
 import org.elasticsearch.search.aggregations.support.ValuesSourceType;
 
 import java.io.IOException;
@@ -51,6 +52,10 @@ protected BoxplotAggregationBuilder(BoxplotAggregationBuilder clone,
         this.compression = clone.compression;
     }
 
+    public static void registerAggregators(ValuesSourceRegistry valuesSourceRegistry) {
+        BoxplotAggregatorFactory.registerAggregators(valuesSourceRegistry);
+    }
+
     @Override
     protected AggregationBuilder shallowCopy(AggregatorFactories.Builder factoriesBuilder, Map<String, Object> metaData) {
         return new BoxplotAggregationBuilder(this, factoriesBuilder, metaData);

x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/boxplot/BoxplotAggregatorFactory.java

@@ -7,13 +7,18 @@
 package org.elasticsearch.xpack.analytics.boxplot;
 
 import org.elasticsearch.index.query.QueryShardContext;
+import org.elasticsearch.search.DocValueFormat;
+import org.elasticsearch.search.aggregations.AggregationExecutionException;
 import org.elasticsearch.search.aggregations.Aggregator;
 import org.elasticsearch.search.aggregations.AggregatorFactories;
 import org.elasticsearch.search.aggregations.AggregatorFactory;
 import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator;
+import org.elasticsearch.search.aggregations.support.AggregatorSupplier;
+import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
 import org.elasticsearch.search.aggregations.support.ValuesSource;
 import org.elasticsearch.search.aggregations.support.ValuesSourceAggregatorFactory;
 import org.elasticsearch.search.aggregations.support.ValuesSourceConfig;
+import org.elasticsearch.search.aggregations.support.ValuesSourceRegistry;
 import org.elasticsearch.search.internal.SearchContext;
 
 import java.io.IOException;
@@ -24,6 +29,24 @@ public class BoxplotAggregatorFactory extends ValuesSourceAggregatorFactory {
 
     private final double compression;
 
+    static void registerAggregators(ValuesSourceRegistry valuesSourceRegistry) {
+        valuesSourceRegistry.register(BoxplotAggregationBuilder.NAME,
+            CoreValuesSourceType.NUMERIC, new BoxplotAggregatorSupplier() {
+                @Override
+                public Aggregator build(String name,
+                                        ValuesSource valuesSource,
+                                        DocValueFormat formatter,
+                                        double compression,
+                                        SearchContext context,
+                                        Aggregator parent,
+                                        List<PipelineAggregator> pipelineAggregators,
+                                        Map<String, Object> metaData) throws IOException {
+                    return new BoxplotAggregator(name, valuesSource, formatter, compression, context, parent,
+                        pipelineAggregators, metaData);
+                }
+            });
+    }
+
     BoxplotAggregatorFactory(String name,
                              ValuesSourceConfig config,
                              double compression,
@@ -52,7 +75,14 @@ protected Aggregator doCreateInternal(ValuesSource valuesSource,
                                           boolean collectsFromSingleBucket,
                                           List<PipelineAggregator> pipelineAggregators,
                                           Map<String, Object> metaData) throws IOException {
-        return new BoxplotAggregator(name, valuesSource, config.format(), compression, searchContext, parent,
-            pipelineAggregators, metaData);
+        AggregatorSupplier aggregatorSupplier = queryShardContext.getValuesSourceRegistry().getAggregator(config.valueSourceType(),
+            BoxplotAggregationBuilder.NAME);
+
+        if (aggregatorSupplier instanceof BoxplotAggregatorSupplier == false) {
+            throw new AggregationExecutionException("Registry miss-match - expected BoxplotAggregatorSupplier, found [" +
+                aggregatorSupplier.getClass().toString() + "]");
+        }
+        return ((BoxplotAggregatorSupplier) aggregatorSupplier).build(name, valuesSource, config.format(), compression,
+            searchContext, parent, pipelineAggregators, metaData);
     }
 }

x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/boxplot/BoxplotAggregatorSupplier.java

@@ -0,0 +1,30 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+package org.elasticsearch.xpack.analytics.boxplot;
+
+import org.elasticsearch.search.DocValueFormat;
+import org.elasticsearch.search.aggregations.Aggregator;
+import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator;
+import org.elasticsearch.search.aggregations.support.AggregatorSupplier;
+import org.elasticsearch.search.aggregations.support.ValuesSource;
+import org.elasticsearch.search.internal.SearchContext;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+
+public interface BoxplotAggregatorSupplier extends AggregatorSupplier {
+    Aggregator build(String name,
+                     ValuesSource valuesSource,
+                     DocValueFormat formatter,
+                     double compression,
+                     SearchContext context,
+                     Aggregator parent,
+                     List<PipelineAggregator> pipelineAggregators,
+                     Map<String, Object> metaData) throws IOException;
+
+}

x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/boxplot/BoxplotAggregatorTests.java

@@ -19,9 +19,16 @@
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.util.BytesRef;
 import org.elasticsearch.common.CheckedConsumer;
+import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.index.mapper.KeywordFieldMapper;
 import org.elasticsearch.index.mapper.MappedFieldType;
 import org.elasticsearch.index.mapper.NumberFieldMapper;
+import org.elasticsearch.script.MockScriptEngine;
+import org.elasticsearch.script.Script;
+import org.elasticsearch.script.ScriptEngine;
+import org.elasticsearch.script.ScriptModule;
+import org.elasticsearch.script.ScriptService;
+import org.elasticsearch.script.ScriptType;
 import org.elasticsearch.search.aggregations.AggregationBuilder;
 import org.elasticsearch.search.aggregations.AggregatorTestCase;
 import org.elasticsearch.search.aggregations.InternalAggregation;
@@ -30,15 +37,57 @@
 import org.elasticsearch.search.aggregations.bucket.histogram.HistogramAggregationBuilder;
 import org.elasticsearch.search.aggregations.bucket.histogram.InternalHistogram;
 import org.elasticsearch.search.aggregations.support.AggregationInspectionHelper;
+import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
+import org.elasticsearch.search.aggregations.support.ValuesSourceType;
+import org.junit.BeforeClass;
 
 import java.io.IOException;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
 import java.util.function.Consumer;
+import java.util.function.Function;
 
 import static java.util.Collections.singleton;
 import static org.hamcrest.Matchers.equalTo;
 
 public class BoxplotAggregatorTests extends AggregatorTestCase {
 
+    /** Script to return the {@code _value} provided by aggs framework. */
+    public static final String VALUE_SCRIPT = "_value";
+
+
+    @BeforeClass()
+    public static void registerBuilder() {
+        BoxplotAggregationBuilder.registerAggregators(valuesSourceRegistry);
+    }
+
+    @Override
+    protected AggregationBuilder createAggBuilderForTypeTest(MappedFieldType fieldType, String fieldName) {
+        return new BoxplotAggregationBuilder("foo").field(fieldName);
+    }
+
+    @Override
+    protected List<ValuesSourceType> getSupportedValuesSourceTypes() {
+        return List.of(CoreValuesSourceType.NUMERIC,
+            CoreValuesSourceType.HISTOGRAM);
+    }
+
+    @Override
+    protected ScriptService getMockScriptService() {
+        Map<String, Function<Map<String, Object>, Object>> scripts = new HashMap<>();
+
+        scripts.put(VALUE_SCRIPT, vars -> ((Number) vars.get("_value")).doubleValue() + 1);
+
+        MockScriptEngine scriptEngine = new MockScriptEngine(MockScriptEngine.NAME,
+            scripts,
+            Collections.emptyMap());
+        Map<String, ScriptEngine> engines = Collections.singletonMap(scriptEngine.getType(), scriptEngine);
+
+        return new ScriptService(Settings.EMPTY, engines, ScriptModule.CORE_CONTEXTS);
+    }
+
     public void testNoMatchingField() throws IOException {
         testCase(new MatchAllDocsQuery(), iw -> {
             iw.addDocument(singleton(new SortedNumericDocValuesField("wrong_number", 7)));
@@ -154,7 +203,8 @@ public void testUnsupportedType() {
             }, (Consumer<InternalBoxplot>) boxplot -> {
                 fail("Should have thrown exception");
             }, fieldType));
-        assertEquals(e.getMessage(), "Expected numeric type on field [not_a_number], but got [keyword]");
+        assertEquals(e.getMessage(), "Field [not_a_number] of type [keyword(indexed,tokenized)] " +
+            "is not supported for aggregation [boxplot]");
     }
 
     public void testBadMissingField() {
@@ -291,6 +341,68 @@ public void testGetProperty() throws IOException {
         }, fieldType);
     }
 
+    public void testValueScript() throws IOException {
+        BoxplotAggregationBuilder aggregationBuilder = new BoxplotAggregationBuilder("boxplot")
+            .field("number")
+            .script(new Script(ScriptType.INLINE, MockScriptEngine.NAME, VALUE_SCRIPT, Collections.emptyMap()));
+
+        MappedFieldType fieldType = new NumberFieldMapper.NumberFieldType(NumberFieldMapper.NumberType.INTEGER);
+        fieldType.setName("number");
+
+        testCase(aggregationBuilder, new MatchAllDocsQuery(), iw -> {
+            iw.addDocument(singleton(new NumericDocValuesField("number", 7)));
+            iw.addDocument(singleton(new NumericDocValuesField("number", 1)));
+        }, (Consumer<InternalBoxplot>) boxplot -> {
+            assertEquals(2, boxplot.getMin(), 0);
+            assertEquals(8, boxplot.getMax(), 0);
+            assertEquals(2, boxplot.getQ1(), 0);
+            assertEquals(5, boxplot.getQ2(), 0);
+            assertEquals(8, boxplot.getQ3(), 0);
+        }, fieldType);
+    }
+
+    public void testValueScriptUnmapped() throws IOException {
+        BoxplotAggregationBuilder aggregationBuilder = new BoxplotAggregationBuilder("boxplot")
+            .field("does_not_exist")
+            .script(new Script(ScriptType.INLINE, MockScriptEngine.NAME, VALUE_SCRIPT, Collections.emptyMap()));
+
+        MappedFieldType fieldType = new NumberFieldMapper.NumberFieldType(NumberFieldMapper.NumberType.INTEGER);
+        fieldType.setName("number");
+
+        testCase(aggregationBuilder, new MatchAllDocsQuery(), iw -> {
+            iw.addDocument(singleton(new NumericDocValuesField("number", 7)));
+            iw.addDocument(singleton(new NumericDocValuesField("number", 1)));
+        }, (Consumer<InternalBoxplot>) boxplot -> {
+            assertEquals(Double.POSITIVE_INFINITY, boxplot.getMin(), 0);
+            assertEquals(Double.NEGATIVE_INFINITY, boxplot.getMax(), 0);
+            assertEquals(Double.NaN, boxplot.getQ1(), 0);
+            assertEquals(Double.NaN, boxplot.getQ2(), 0);
+            assertEquals(Double.NaN, boxplot.getQ3(), 0);
+        }, fieldType);
+    }
+
+    public void testValueScriptUnmappedMissing() throws IOException {
+        BoxplotAggregationBuilder aggregationBuilder = new BoxplotAggregationBuilder("boxplot")
+            .field("does_not_exist")
+            .script(new Script(ScriptType.INLINE, MockScriptEngine.NAME, VALUE_SCRIPT, Collections.emptyMap()))
+            .missing(1.0);
+
+        MappedFieldType fieldType = new NumberFieldMapper.NumberFieldType(NumberFieldMapper.NumberType.INTEGER);
+        fieldType.setName("number");
+
+        testCase(aggregationBuilder, new MatchAllDocsQuery(), iw -> {
+            iw.addDocument(singleton(new NumericDocValuesField("number", 7)));
+            iw.addDocument(singleton(new NumericDocValuesField("number", 1)));
+        }, (Consumer<InternalBoxplot>) boxplot -> {
+            // Note: the way scripts, missing and unmapped interact, these will be the missing value and the script is not invoked
+            assertEquals(1.0, boxplot.getMin(), 0);
+            assertEquals(1.0, boxplot.getMax(), 0);
+            assertEquals(1.0, boxplot.getQ1(), 0);
+            assertEquals(1.0, boxplot.getQ2(), 0);
+            assertEquals(1.0, boxplot.getQ3(), 0);
+        }, fieldType);
+    }
+
     private void testCase(Query query,
                           CheckedConsumer<RandomIndexWriter, IOException> buildIndex,
                           Consumer<InternalBoxplot> verify) throws IOException {