Throw better exception for unsupported aggregations over geo_shape fields

x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/SpatialPlugin.java

@@ -26,12 +26,8 @@
 import org.elasticsearch.search.aggregations.bucket.geogrid.GeoHashGridAggregator;
 import org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileGridAggregationBuilder;
 import org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileGridAggregator;
-import org.elasticsearch.search.aggregations.metrics.CardinalityAggregationBuilder;
-import org.elasticsearch.search.aggregations.metrics.CardinalityAggregator;
 import org.elasticsearch.search.aggregations.metrics.GeoBoundsAggregationBuilder;
 import org.elasticsearch.search.aggregations.metrics.GeoCentroidAggregationBuilder;
-import org.elasticsearch.search.aggregations.metrics.ValueCountAggregationBuilder;
-import org.elasticsearch.search.aggregations.metrics.ValueCountAggregator;
 import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
 import org.elasticsearch.search.aggregations.support.ValuesSource;
 import org.elasticsearch.search.aggregations.support.ValuesSourceRegistry;
@@ -162,9 +158,7 @@ public List<Consumer<ValuesSourceRegistry.Builder>> getAggregationExtentions() {
         return List.of(
             this::registerGeoShapeCentroidAggregator,
             this::registerGeoShapeGridAggregators,
-            SpatialPlugin::registerGeoShapeBoundsAggregator,
-            SpatialPlugin::registerValueCountAggregator,
-            SpatialPlugin::registerCardinalityAggregator
+            SpatialPlugin::registerGeoShapeBoundsAggregator
         );
     }
 
@@ -408,28 +402,6 @@ private void registerGeoShapeGridAggregators(ValuesSourceRegistry.Builder builde
         );
     }
 
-    private static void registerValueCountAggregator(ValuesSourceRegistry.Builder builder) {
-        builder.register(ValueCountAggregationBuilder.REGISTRY_KEY, GeoShapeValuesSourceType.instance(), ValueCountAggregator::new, true);
-    }
-
-    private static void registerCardinalityAggregator(ValuesSourceRegistry.Builder builder) {
-        builder.register(
-            CardinalityAggregationBuilder.REGISTRY_KEY,
-            GeoShapeValuesSourceType.instance(),
-            (name, valuesSourceConfig, precision, executionMode, context, parent, metadata) -> new CardinalityAggregator(
-                name,
-                valuesSourceConfig,
-                precision,
-                // Force execution mode to null
-                null,
-                context,
-                parent,
-                metadata
-            ),
-            true
-        );
-    }
-
     private <T> ContextParser<String, T> checkLicense(ContextParser<String, T> realParser, LicensedFeature.Momentary feature) {
         return (parser, name) -> {
             if (feature.check(getLicenseState()) == false) {

x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/search/aggregations/UnsupportedAggregationsTests.java

@@ -0,0 +1,121 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+package org.elasticsearch.xpack.spatial.search.aggregations;
+
+import org.apache.lucene.document.Document;
+import org.elasticsearch.common.geo.Orientation;
+import org.elasticsearch.geometry.Point;
+import org.elasticsearch.index.mapper.MappedFieldType;
+import org.elasticsearch.lucene.spatial.BinaryShapeDocValuesField;
+import org.elasticsearch.plugins.SearchPlugin;
+import org.elasticsearch.search.aggregations.AggregatorTestCase;
+import org.elasticsearch.search.aggregations.metrics.CardinalityAggregationBuilder;
+import org.elasticsearch.search.aggregations.metrics.ValueCountAggregationBuilder;
+import org.elasticsearch.xpack.spatial.LocalStateSpatialPlugin;
+import org.elasticsearch.xpack.spatial.index.mapper.GeoShapeWithDocValuesFieldMapper;
+import org.elasticsearch.xpack.spatial.index.mapper.ShapeFieldMapper;
+import org.elasticsearch.xpack.spatial.util.GeoTestUtils;
+
+import java.util.Collections;
+import java.util.List;
+
+import static org.hamcrest.Matchers.equalTo;
+
+public class UnsupportedAggregationsTests extends AggregatorTestCase {
+
+    @Override
+    protected List<SearchPlugin> getSearchPlugins() {
+        return List.of(new LocalStateSpatialPlugin());
+    }
+
+    public void testCardinalityAggregationOnGeoShape() {
+        MappedFieldType fieldType = new GeoShapeWithDocValuesFieldMapper.GeoShapeWithDocValuesFieldType(
+            "geometry",
+            true,
+            true,
+            randomBoolean(),
+            Orientation.RIGHT,
+            null,
+            null,
+            null,
+            false,
+            Collections.emptyMap()
+        );
+        BinaryShapeDocValuesField field = GeoTestUtils.binaryGeoShapeDocValuesField("geometry", new Point(0, 0));
+        CardinalityAggregationBuilder builder = new CardinalityAggregationBuilder("cardinality").field("geometry");
+        IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> testCase(iw -> {
+            Document doc = new Document();
+            doc.add(field);
+            iw.addDocument(doc);
+        }, agg -> {}, new AggTestConfig(builder, fieldType)));
+        assertThat(exception.getMessage(), equalTo("Field [geometry] of type [geo_shape] is not supported for aggregation [cardinality]"));
+    }
+
+    public void testCardinalityAggregationOnShape() {
+        MappedFieldType fieldType = new ShapeFieldMapper.ShapeFieldType(
+            "geometry",
+            true,
+            true,
+            Orientation.RIGHT,
+            null,
+            false,
+            Collections.emptyMap()
+        );
+        BinaryShapeDocValuesField field = GeoTestUtils.binaryCartesianShapeDocValuesField("geometry", new Point(0, 0));
+        CardinalityAggregationBuilder builder = new CardinalityAggregationBuilder("cardinality").field("geometry");
+        IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> testCase(iw -> {
+            Document doc = new Document();
+            doc.add(field);
+            iw.addDocument(doc);
+        }, agg -> {}, new AggTestConfig(builder, fieldType)));
+        assertThat(exception.getMessage(), equalTo("Field [geometry] of type [shape] is not supported for aggregation [cardinality]"));
+    }
+
+    public void testValueCountAggregationOnGeoShape() {
+        MappedFieldType fieldType = new GeoShapeWithDocValuesFieldMapper.GeoShapeWithDocValuesFieldType(
+            "geometry",
+            true,
+            true,
+            randomBoolean(),
+            Orientation.RIGHT,
+            null,
+            null,
+            null,
+            false,
+            Collections.emptyMap()
+        );
+        BinaryShapeDocValuesField field = GeoTestUtils.binaryGeoShapeDocValuesField("geometry", new Point(0, 0));
+        ValueCountAggregationBuilder builder = new ValueCountAggregationBuilder("cardinality").field("geometry");
+        IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> testCase(iw -> {
+            Document doc = new Document();
+            doc.add(field);
+            iw.addDocument(doc);
+        }, agg -> {}, new AggTestConfig(builder, fieldType)));
+        assertThat(exception.getMessage(), equalTo("Field [geometry] of type [geo_shape] is not supported for aggregation [value_count]"));
+    }
+
+    public void testValueCountAggregationOShape() {
+        MappedFieldType fieldType = new ShapeFieldMapper.ShapeFieldType(
+            "geometry",
+            true,
+            true,
+            Orientation.RIGHT,
+            null,
+            false,
+            Collections.emptyMap()
+        );
+        BinaryShapeDocValuesField field = GeoTestUtils.binaryCartesianShapeDocValuesField("geometry", new Point(0, 0));
+        ValueCountAggregationBuilder builder = new ValueCountAggregationBuilder("cardinality").field("geometry");
+        IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> testCase(iw -> {
+            Document doc = new Document();
+            doc.add(field);
+            iw.addDocument(doc);
+        }, agg -> {}, new AggTestConfig(builder, fieldType)));
+        assertThat(exception.getMessage(), equalTo("Field [geometry] of type [shape] is not supported for aggregation [value_count]"));
+    }
+}