diff --git a/server/src/main/java/org/elasticsearch/search/SearchModule.java b/server/src/main/java/org/elasticsearch/search/SearchModule.java index 090aa2ad71d7c..8563afb1b6dfc 100644 --- a/server/src/main/java/org/elasticsearch/search/SearchModule.java +++ b/server/src/main/java/org/elasticsearch/search/SearchModule.java @@ -442,7 +442,8 @@ private void registerAggregations(List plugins) { registerAggregation(new AggregationSpec(GeoBoundsAggregationBuilder.NAME, GeoBoundsAggregationBuilder::new, GeoBoundsAggregationBuilder::parse).addResultReader(InternalGeoBounds::new)); registerAggregation(new AggregationSpec(GeoCentroidAggregationBuilder.NAME, GeoCentroidAggregationBuilder::new, - GeoCentroidAggregationBuilder::parse).addResultReader(InternalGeoCentroid::new)); + GeoCentroidAggregationBuilder::parse).addResultReader(InternalGeoCentroid::new) + .setAggregatorRegistrar(GeoCentroidAggregationBuilder::registerAggregators)); registerAggregation(new AggregationSpec(ScriptedMetricAggregationBuilder.NAME, ScriptedMetricAggregationBuilder::new, ScriptedMetricAggregationBuilder.PARSER).addResultReader(InternalScriptedMetric::new)); registerAggregation((new AggregationSpec(CompositeAggregationBuilder.NAME, CompositeAggregationBuilder::new, diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoCentroidAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoCentroidAggregationBuilder.java index 44e5dcf757a6c..94c9ece4b499d 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoCentroidAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoCentroidAggregationBuilder.java @@ -33,6 +33,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; @@ -52,6 +53,10 @@ public static AggregationBuilder parse(String aggregationName, XContentParser pa return PARSER.parse(parser, new GeoCentroidAggregationBuilder(aggregationName), null); } + public static void registerAggregators(ValuesSourceRegistry valuesSourceRegistry) { + GeoCentroidAggregatorFactory.registerAggregators(valuesSourceRegistry); + } + public GeoCentroidAggregationBuilder(String name) { super(name); } @@ -62,8 +67,7 @@ protected GeoCentroidAggregationBuilder(GeoCentroidAggregationBuilder clone, Bui @Override protected ValuesSourceType defaultValueSourceType() { - // TODO: This should probably not be BYTES, but we're not failing tests with BYTES, so needs more tests? - return CoreValuesSourceType.BYTES; + return CoreValuesSourceType.GEOPOINT; } @Override diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoCentroidAggregatorFactory.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoCentroidAggregatorFactory.java index a1f4755979862..8a109b6ebeb46 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoCentroidAggregatorFactory.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoCentroidAggregatorFactory.java @@ -25,9 +25,12 @@ 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; @@ -60,10 +63,20 @@ protected Aggregator doCreateInternal(ValuesSource valuesSource, boolean collectsFromSingleBucket, List pipelineAggregators, Map metaData) throws IOException { - if (valuesSource instanceof ValuesSource.GeoPoint == false) { - throw new AggregationExecutionException("ValuesSource type " + valuesSource.toString() + "is not supported for aggregation " + - this.name()); + + AggregatorSupplier aggregatorSupplier = queryShardContext.getValuesSourceRegistry().getAggregator(config.valueSourceType(), + GeoCentroidAggregationBuilder.NAME); + if (aggregatorSupplier instanceof GeoCentroidAggregatorSupplier == false) { + throw new AggregationExecutionException("Registry miss-match - expected " + + GeoCentroidAggregatorSupplier.class.getName() + ", found [" + aggregatorSupplier.getClass().toString() + "]"); } - return new GeoCentroidAggregator(name, searchContext, parent, (ValuesSource.GeoPoint) valuesSource, pipelineAggregators, metaData); + return ((GeoCentroidAggregatorSupplier) aggregatorSupplier).build(name, searchContext, parent, valuesSource, + pipelineAggregators, metaData); + } + + static void registerAggregators(ValuesSourceRegistry valuesSourceRegistry) { + valuesSourceRegistry.register(GeoCentroidAggregationBuilder.NAME, CoreValuesSourceType.GEOPOINT, + (GeoCentroidAggregatorSupplier) (name, context, parent, valuesSource, pipelineAggregators, metaData) -> + new GeoCentroidAggregator(name, context, parent, (ValuesSource.GeoPoint) valuesSource, pipelineAggregators, metaData)); } } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoCentroidAggregatorSupplier.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoCentroidAggregatorSupplier.java new file mode 100644 index 0000000000000..7c1d34ae20977 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoCentroidAggregatorSupplier.java @@ -0,0 +1,38 @@ +/* + * 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.search.aggregations.metrics; + +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; + +@FunctionalInterface +public interface GeoCentroidAggregatorSupplier extends AggregatorSupplier { + + GeoCentroidAggregator build(String name, SearchContext context, Aggregator parent, + ValuesSource valuesSource, List pipelineAggregators, + Map metaData) throws IOException; +} diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/GeoCentroidAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/GeoCentroidAggregatorTests.java index 4f27cc1e3099e..c692d45371039 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/GeoCentroidAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/GeoCentroidAggregatorTests.java @@ -28,11 +28,15 @@ import org.elasticsearch.common.geo.GeoPoint; import org.elasticsearch.index.mapper.GeoPointFieldMapper; import org.elasticsearch.index.mapper.MappedFieldType; +import org.elasticsearch.search.aggregations.AggregationBuilder; import org.elasticsearch.search.aggregations.AggregatorTestCase; import org.elasticsearch.search.aggregations.support.AggregationInspectionHelper; +import org.elasticsearch.search.aggregations.support.CoreValuesSourceType; +import org.elasticsearch.search.aggregations.support.ValuesSourceType; import org.elasticsearch.test.geo.RandomGeoGenerator; import java.io.IOException; +import java.util.List; public class GeoCentroidAggregatorTests extends AggregatorTestCase { @@ -178,4 +182,13 @@ private void assertCentroid(RandomIndexWriter w, GeoPoint expectedCentroid) thro } } + @Override + protected AggregationBuilder createAggBuilderForTypeTest(MappedFieldType fieldType, String fieldName) { + return new GeoCentroidAggregationBuilder("foo").field(fieldName); + } + + @Override + protected List getSupportedValuesSourceTypes() { + return List.of(CoreValuesSourceType.GEOPOINT); + } }