From 20674321370e8bd0d44d7f660a4bdbfde14486f3 Mon Sep 17 00:00:00 2001 From: Tal Levy Date: Mon, 2 Mar 2020 14:05:18 -0800 Subject: [PATCH 1/2] Wire up geo_bounds aggregation to ValuesSourceRegistry This commit updates the geo_bounds aggregation to depend on registering itself in the ValuesSourceRegistry relates #42949. --- .../elasticsearch/search/SearchModule.java | 3 +- .../metrics/GeoBoundsAggregationBuilder.java | 5 +++ .../metrics/GeoBoundsAggregatorFactory.java | 22 ++++++++--- .../metrics/GeoBoundsAggregatorSupplier.java | 37 +++++++++++++++++++ 4 files changed, 61 insertions(+), 6 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoBoundsAggregatorSupplier.java diff --git a/server/src/main/java/org/elasticsearch/search/SearchModule.java b/server/src/main/java/org/elasticsearch/search/SearchModule.java index 090aa2ad71d7c..075f254edc99d 100644 --- a/server/src/main/java/org/elasticsearch/search/SearchModule.java +++ b/server/src/main/java/org/elasticsearch/search/SearchModule.java @@ -440,7 +440,8 @@ private void registerAggregations(List plugins) { registerAggregation(new AggregationSpec(TopHitsAggregationBuilder.NAME, TopHitsAggregationBuilder::new, TopHitsAggregationBuilder::parse).addResultReader(InternalTopHits::new)); registerAggregation(new AggregationSpec(GeoBoundsAggregationBuilder.NAME, GeoBoundsAggregationBuilder::new, - GeoBoundsAggregationBuilder::parse).addResultReader(InternalGeoBounds::new)); + GeoBoundsAggregationBuilder::parse).addResultReader(InternalGeoBounds::new) + .setAggregatorRegistrar(GeoBoundsAggregationBuilder::registerAggregators)); registerAggregation(new AggregationSpec(GeoCentroidAggregationBuilder.NAME, GeoCentroidAggregationBuilder::new, GeoCentroidAggregationBuilder::parse).addResultReader(InternalGeoCentroid::new)); registerAggregation(new AggregationSpec(ScriptedMetricAggregationBuilder.NAME, ScriptedMetricAggregationBuilder::new, diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoBoundsAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoBoundsAggregationBuilder.java index f77d802f1b469..835ca71bffb1d 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoBoundsAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoBoundsAggregationBuilder.java @@ -32,6 +32,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 GeoBoundsAggregationBuilder(aggregationName), null); } + public static void registerAggregators(ValuesSourceRegistry valuesSourceRegistry) { + GeoBoundsAggregatorFactory.registerAggregators(valuesSourceRegistry); + } + private boolean wrapLongitude = true; public GeoBoundsAggregationBuilder(String name) { diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoBoundsAggregatorFactory.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoBoundsAggregatorFactory.java index 7d167e938a288..99893c2a0ff61 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoBoundsAggregatorFactory.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoBoundsAggregatorFactory.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; @@ -64,11 +67,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(), GeoBoundsAggregationBuilder.NAME); + + if (aggregatorSupplier instanceof GeoBoundsAggregatorSupplier == false) { + throw new AggregationExecutionException("Registry miss-match - expected " + + GeoBoundsAggregatorSupplier.class.getName() + ", found [" + aggregatorSupplier.getClass().toString() + "]"); } - return new GeoBoundsAggregator(name, searchContext, parent, (ValuesSource.GeoPoint) valuesSource, wrapLongitude, - pipelineAggregators, metaData); + + return ((GeoBoundsAggregatorSupplier) aggregatorSupplier).build(name, searchContext, parent, (ValuesSource.GeoPoint) valuesSource, + wrapLongitude, pipelineAggregators, metaData); + } + + static void registerAggregators(ValuesSourceRegistry valuesSourceRegistry) { + valuesSourceRegistry.register(GeoBoundsAggregationBuilder.NAME, CoreValuesSourceType.GEOPOINT, + (GeoBoundsAggregatorSupplier) GeoBoundsAggregator::new); } } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoBoundsAggregatorSupplier.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoBoundsAggregatorSupplier.java new file mode 100644 index 0000000000000..acb4a0f232641 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoBoundsAggregatorSupplier.java @@ -0,0 +1,37 @@ +/* + * 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; + +public interface GeoBoundsAggregatorSupplier extends AggregatorSupplier { + + GeoBoundsAggregator build(String name, SearchContext aggregationContext, Aggregator parent, + ValuesSource.GeoPoint valuesSource, boolean wrapLongitude, List pipelineAggregators, + Map metaData) throws IOException; +} From 46e83f455584ebbbf88e4f5ba38009017e4ee91e Mon Sep 17 00:00:00 2001 From: Tal Levy Date: Tue, 3 Mar 2020 10:41:29 -0800 Subject: [PATCH 2/2] update --- .../aggregations/metrics/GeoBoundsAggregatorFactory.java | 8 +++++--- .../aggregations/metrics/GeoBoundsAggregatorSupplier.java | 3 ++- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoBoundsAggregatorFactory.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoBoundsAggregatorFactory.java index 99893c2a0ff61..375fd050cfb39 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoBoundsAggregatorFactory.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoBoundsAggregatorFactory.java @@ -75,12 +75,14 @@ protected Aggregator doCreateInternal(ValuesSource valuesSource, + GeoBoundsAggregatorSupplier.class.getName() + ", found [" + aggregatorSupplier.getClass().toString() + "]"); } - return ((GeoBoundsAggregatorSupplier) aggregatorSupplier).build(name, searchContext, parent, (ValuesSource.GeoPoint) valuesSource, - wrapLongitude, pipelineAggregators, metaData); + return ((GeoBoundsAggregatorSupplier) aggregatorSupplier).build(name, searchContext, parent, valuesSource, wrapLongitude, + pipelineAggregators, metaData); } static void registerAggregators(ValuesSourceRegistry valuesSourceRegistry) { valuesSourceRegistry.register(GeoBoundsAggregationBuilder.NAME, CoreValuesSourceType.GEOPOINT, - (GeoBoundsAggregatorSupplier) GeoBoundsAggregator::new); + (GeoBoundsAggregatorSupplier) (name, aggregationContext, parent, valuesSource, wrapLongitude, pipelineAggregators, metaData) + -> new GeoBoundsAggregator(name, aggregationContext, parent, (ValuesSource.GeoPoint) valuesSource, + wrapLongitude, pipelineAggregators, metaData)); } } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoBoundsAggregatorSupplier.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoBoundsAggregatorSupplier.java index acb4a0f232641..4061f2022eb83 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoBoundsAggregatorSupplier.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoBoundsAggregatorSupplier.java @@ -29,9 +29,10 @@ import java.util.List; import java.util.Map; +@FunctionalInterface public interface GeoBoundsAggregatorSupplier extends AggregatorSupplier { GeoBoundsAggregator build(String name, SearchContext aggregationContext, Aggregator parent, - ValuesSource.GeoPoint valuesSource, boolean wrapLongitude, List pipelineAggregators, + ValuesSource valuesSource, boolean wrapLongitude, List pipelineAggregators, Map metaData) throws IOException; }