Remove MVT-specific logic from GeoFormatterFactory

GeoFormatterFactory doesn't need to know that the extension points were created
specifically for the purpose of generating vector tiles. We can make it support
an arbitrary formats by moving all MVT-specific logic into formatter itself.

Follow up for elastic#75367

server/src/main/java/org/elasticsearch/common/geo/GeoFormatterFactory.java

@@ -9,62 +9,71 @@
 package org.elasticsearch.common.geo;
 
 import org.elasticsearch.geometry.Geometry;
-import org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileUtils;
 
+import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
-import java.util.Locale;
+import java.util.Map;
 import java.util.function.Function;
 
 /**
- * Output formatters for geo fields. Adds support for vector tiles.
+ * Output formatters for geo fields support extensions such as vector tiles.
+ *
+ * This class is an extensible version of a static GeometryFormatterFactory
  */
-public class GeoFormatterFactory {
+public class GeoFormatterFactory<T> {
 
-    @FunctionalInterface
-    public interface VectorTileEngine<T>  {
+    /**
+     * Defines an extension point for geometry formatter
+     * @param <T>
+     */
+    public interface FormatterFactory<T> {
         /**
-         * Returns a formatter for a specific tile.
+         * Format name
          */
-        Function<List<T>, List<Object>> getFormatter(int z, int x, int y, int extent);
+        String getName();
+
+        /**
+         * Generates a formatter builder that parses the formatter configuration and generates a formatter
+         */
+        Function<String, Function<List<T>, List<Object>>> getFormatterBuilder();
     }
 
-    private static final String MVT = "mvt";
+    private final Map<String, Function<String, Function<List<T>, List<Object>>>> factories;
+
+    /**
+     * Creates an extensible geo formatter. The extension points can be added as a list of factories
+     */
+    public GeoFormatterFactory(List<FormatterFactory<T>> factories) {
+        Map<String, Function<String, Function<List<T>, List<Object>>>> factoriesBuilder = new HashMap<>();
+        for (FormatterFactory<T> factory : factories) {
+            if(factoriesBuilder.put(factory.getName(), factory.getFormatterBuilder()) != null) {
+                throw new IllegalArgumentException("More then one formatter factory with the name [" + factory.getName() +
+                    "] was configured");
+            }
+
+        }
+        this.factories = Collections.unmodifiableMap(factoriesBuilder);
+    }
 
     /**
      * Returns a formatter by name
+     *
+     * The format can contain an optional parameters in parentheses such as "mvt(1/2/3)". Parameterless formats are getting resolved
+     * using standard GeometryFormatterFactory and formats with parameters are getting resolved using factories specified during
+     * construction.
      */
-    public static <T> Function<List<T>, List<Object>> getFormatter(String format, Function<T, Geometry> toGeometry,
-                                                                          VectorTileEngine<T> mvt) {
+    public Function<List<T>, List<Object>> getFormatter(String format, Function<T, Geometry> toGeometry) {
         final int start = format.indexOf('(');
         if (start == -1)  {
             return GeometryFormatterFactory.getFormatter(format, toGeometry);
         }
         final String formatName = format.substring(0, start);
-        if (MVT.equals(formatName) == false) {
+        Function<String, Function<List<T>, List<Object>>> factory = factories.get(formatName);
+        if (factory == null) {
             throw new IllegalArgumentException("Invalid format: " + formatName);
         }
         final String param = format.substring(start + 1, format.length() - 1);
-        // we expect either z/x/y or z/x/y@extent
-        final String[] parts = param.split("@", 3);
-        if (parts.length > 2) {
-            throw new IllegalArgumentException(
-                "Invalid mvt formatter parameter [" + param + "]. Must have the form \"zoom/x/y\" or \"zoom/x/y@extent\"."
-            );
-        }
-        final int extent = parts.length == 2 ? Integer.parseInt(parts[1]) : 4096;
-        final String[] tileBits = parts[0].split("/", 4);
-        if (tileBits.length != 3) {
-            throw new IllegalArgumentException(
-                "Invalid tile string [" + parts[0] + "]. Must be three integers in a form \"zoom/x/y\"."
-            );
-        }
-        final int z = GeoTileUtils.checkPrecisionRange(Integer.parseInt(tileBits[0]));
-        final int tiles = 1 << z;
-        final int x = Integer.parseInt(tileBits[1]);
-        final int y = Integer.parseInt(tileBits[2]);
-        if (x < 0 || y < 0 || x >= tiles || y >= tiles) {
-            throw new IllegalArgumentException(String.format(Locale.ROOT, "Zoom/X/Y combination is not valid: %d/%d/%d", z, x, y));
-        }
-        return mvt.getFormatter(z, x, y, extent);
+        return factory.apply(param);
     }
 }

server/src/main/java/org/elasticsearch/common/geo/SimpleFeatureFactory.java

@@ -9,7 +9,6 @@
 package org.elasticsearch.common.geo;
 
 import org.apache.lucene.util.BitUtil;
-import org.elasticsearch.common.geo.SphericalMercatorUtils;
 import org.elasticsearch.common.io.stream.BytesStreamOutput;
 import org.elasticsearch.geometry.Rectangle;
 import org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileUtils;

server/src/main/java/org/elasticsearch/common/geo/SimpleVectorTileFormatter.java

@@ -0,0 +1,65 @@
+/*
+ * 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 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+package org.elasticsearch.common.geo;
+
+import org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileUtils;
+
+import java.util.List;
+import java.util.Locale;
+import java.util.function.Function;
+
+/**
+ * A facade for SimpleFeatureFactory that converts it into FormatterFactory for use in GeoPointFieldMapper
+ */
+public class SimpleVectorTileFormatter implements GeoFormatterFactory.FormatterFactory<GeoPoint> {
+
+    public static final String MVT = "mvt";
+
+    @Override
+    public String getName() {
+        return MVT;
+    }
+
+    @Override
+    public Function<String, Function<List<GeoPoint>, List<Object>>> getFormatterBuilder() {
+        return  params -> {
+            int[] parsed = parse(params);
+            final SimpleFeatureFactory featureFactory = new SimpleFeatureFactory(parsed[0], parsed[1], parsed[2], parsed[3]);
+            return points -> List.of(featureFactory.points(points));
+        };
+    }
+
+    /**
+     * Parses string in the format we expect either z/x/y or z/x/y@extent to an array of integer parameters
+     */
+    public static int[] parse(String param) {
+        // we expect either z/x/y or z/x/y@extent
+        final String[] parts = param.split("@", 3);
+        if (parts.length > 2) {
+            throw new IllegalArgumentException(
+                "Invalid mvt formatter parameter [" + param + "]. Must have the form \"zoom/x/y\" or \"zoom/x/y@extent\"."
+            );
+        }
+        final int extent = parts.length == 2 ? Integer.parseInt(parts[1]) : 4096;
+        final String[] tileBits = parts[0].split("/", 4);
+        if (tileBits.length != 3) {
+            throw new IllegalArgumentException(
+                "Invalid tile string [" + parts[0] + "]. Must be three integers in a form \"zoom/x/y\"."
+            );
+        }
+        final int z = GeoTileUtils.checkPrecisionRange(Integer.parseInt(tileBits[0]));
+        final int tiles = 1 << z;
+        final int x = Integer.parseInt(tileBits[1]);
+        final int y = Integer.parseInt(tileBits[2]);
+        if (x < 0 || y < 0 || x >= tiles || y >= tiles) {
+            throw new IllegalArgumentException(String.format(Locale.ROOT, "Zoom/X/Y combination is not valid: %d/%d/%d", z, x, y));
+        }
+        return new int[]{z, x, y, extent};
+    }
+}

server/src/main/java/org/elasticsearch/index/mapper/GeoPointFieldMapper.java

@@ -25,7 +25,7 @@
 import org.elasticsearch.common.geo.GeoUtils;
 import org.elasticsearch.common.geo.GeometryFormatterFactory;
 import org.elasticsearch.common.geo.ShapeRelation;
-import org.elasticsearch.common.geo.SimpleFeatureFactory;
+import org.elasticsearch.common.geo.SimpleVectorTileFormatter;
 import org.elasticsearch.common.unit.DistanceUnit;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.common.xcontent.support.MapXContentParser;
@@ -225,6 +225,10 @@ protected String contentType() {
 
     public static class GeoPointFieldType extends AbstractGeometryFieldType<GeoPoint> implements GeoShapeQueryable {
 
+        private static final GeoFormatterFactory<GeoPoint> GEO_FORMATTER_FACTORY = new GeoFormatterFactory<>(
+            List.of(new SimpleVectorTileFormatter())
+        );
+
         private final FieldValues<GeoPoint> scriptValues;
 
         private GeoPointFieldType(String name, boolean indexed, boolean stored, boolean hasDocValues,
@@ -245,11 +249,7 @@ public String typeName() {
 
         @Override
         protected  Function<List<GeoPoint>, List<Object>> getFormatter(String format) {
-            return GeoFormatterFactory.getFormatter(format, p -> new Point(p.getLon(), p.getLat()),
-                (z, x, y, extent) -> {
-                    final SimpleFeatureFactory featureFactory = new SimpleFeatureFactory(z, x, y, extent);
-                    return points -> List.of(featureFactory.points(points));
-                });
+            return GEO_FORMATTER_FACTORY.getFormatter(format, p -> new Point(p.getLon(), p.getLat()));
         }
 
         @Override

x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/VectorTileExtension.java → x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/GeometryFormatterExtension.java

@@ -10,11 +10,14 @@
 import org.elasticsearch.common.geo.GeoFormatterFactory;
 import org.elasticsearch.geometry.Geometry;
 
+import java.util.List;
 
-public interface VectorTileExtension {
+/**
+ * Extension point for geometry formatters
+ */
+public interface GeometryFormatterExtension {
     /**
-     * Get the vector tile engine. This is called when user ask for the MVT format on the field API.
-     * We are only expecting one instance of a vector tile engine coming from the vector tile module.
+     * Get a list of geometry formatters.
      */
-    GeoFormatterFactory.VectorTileEngine<Geometry> getVectorTileEngine();
+    List<GeoFormatterFactory.FormatterFactory<Geometry>> getGeometryFormatterFactories();
 }

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

@@ -36,9 +36,6 @@
 import org.elasticsearch.xpack.spatial.action.SpatialInfoTransportAction;
 import org.elasticsearch.xpack.spatial.action.SpatialStatsTransportAction;
 import org.elasticsearch.xpack.spatial.action.SpatialUsageTransportAction;
-import org.elasticsearch.xpack.spatial.search.aggregations.bucket.geogrid.UnboundedGeoTileGridTiler;
-import org.elasticsearch.xpack.spatial.search.aggregations.bucket.geogrid.UnboundedGeoHashGridTiler;
-import org.elasticsearch.xpack.spatial.search.aggregations.metrics.GeoShapeCentroidAggregator;
 import org.elasticsearch.xpack.spatial.index.mapper.GeoShapeWithDocValuesFieldMapper;
 import org.elasticsearch.xpack.spatial.index.mapper.PointFieldMapper;
 import org.elasticsearch.xpack.spatial.index.mapper.ShapeFieldMapper;
@@ -52,7 +49,10 @@
 import org.elasticsearch.xpack.spatial.search.aggregations.bucket.geogrid.GeoShapeCellIdSource;
 import org.elasticsearch.xpack.spatial.search.aggregations.bucket.geogrid.GeoShapeHashGridAggregator;
 import org.elasticsearch.xpack.spatial.search.aggregations.bucket.geogrid.GeoShapeTileGridAggregator;
+import org.elasticsearch.xpack.spatial.search.aggregations.bucket.geogrid.UnboundedGeoHashGridTiler;
+import org.elasticsearch.xpack.spatial.search.aggregations.bucket.geogrid.UnboundedGeoTileGridTiler;
 import org.elasticsearch.xpack.spatial.search.aggregations.metrics.GeoShapeBoundsAggregator;
+import org.elasticsearch.xpack.spatial.search.aggregations.metrics.GeoShapeCentroidAggregator;
 import org.elasticsearch.xpack.spatial.search.aggregations.support.GeoShapeValuesSource;
 import org.elasticsearch.xpack.spatial.search.aggregations.support.GeoShapeValuesSourceType;
 
@@ -73,7 +73,7 @@ protected XPackLicenseState getLicenseState() {
         return XPackPlugin.getSharedLicenseState();
     }
     // register the vector tile factory from a different module
-    private final SetOnce<VectorTileExtension> vectorTileExtension = new SetOnce<>();
+    private final SetOnce<GeometryFormatterExtension> vectorTileExtension = new SetOnce<>();
 
     @Override
     public List<ActionPlugin.ActionHandler<? extends ActionRequest, ? extends ActionResponse>> getActions() {
@@ -214,6 +214,6 @@ private <T> ContextParser<String, T> checkLicense(ContextParser<String, T> realP
     @Override
     public void loadExtensions(ExtensionLoader loader) {
         // we only expect one vector tile extension that comes from the vector tile module.
-        loader.loadExtensions(VectorTileExtension.class).forEach(vectorTileExtension::set);
+        loader.loadExtensions(GeometryFormatterExtension.class).forEach(vectorTileExtension::set);
     }
 }

x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapper.java

@@ -40,7 +40,7 @@
 import org.elasticsearch.index.query.QueryShardException;
 import org.elasticsearch.index.query.SearchExecutionContext;
 import org.elasticsearch.search.lookup.SearchLookup;
-import org.elasticsearch.xpack.spatial.VectorTileExtension;
+import org.elasticsearch.xpack.spatial.GeometryFormatterExtension;
 import org.elasticsearch.xpack.spatial.index.fielddata.plain.AbstractLatLonShapeIndexFieldData;
 import org.elasticsearch.xpack.spatial.search.aggregations.support.GeoShapeValuesSourceType;
 
@@ -96,13 +96,13 @@ public static class Builder extends FieldMapper.Builder {
         final Parameter<Map<String, String>> meta = Parameter.metaParam();
 
         private final Version version;
-        private final VectorTileExtension vectorTileExtension;
+        private final GeometryFormatterExtension geometryFormatterExtension;
 
         public Builder(String name, Version version, boolean ignoreMalformedByDefault, boolean coerceByDefault,
-                      VectorTileExtension vectorTileExtension) {
+                      GeometryFormatterExtension geometryFormatterExtension) {
             super(name);
             this.version = version;
-            this.vectorTileExtension = vectorTileExtension;
+            this.geometryFormatterExtension = geometryFormatterExtension;
             this.ignoreMalformed = ignoreMalformedParam(m -> builder(m).ignoreMalformed.get(), ignoreMalformedByDefault);
             this.coerce = coerceParam(m -> builder(m).coerce.get(), coerceByDefault);
             this.hasDocValues
@@ -134,7 +134,7 @@ public GeoShapeWithDocValuesFieldMapper build(ContentPath contentPath) {
                 hasDocValues.get(),
                 orientation.get().value(),
                 parser,
-                vectorTileExtension,
+                geometryFormatterExtension,
                 meta.get());
             return new GeoShapeWithDocValuesFieldMapper(name, ft,
                 multiFieldsBuilder.build(this, contentPath), copyTo.build(),
@@ -145,12 +145,12 @@ public GeoShapeWithDocValuesFieldMapper build(ContentPath contentPath) {
 
     public static final class GeoShapeWithDocValuesFieldType extends AbstractShapeGeometryFieldType<Geometry> implements GeoShapeQueryable {
 
-        private final VectorTileExtension vectorTileExtension;
+        private final GeoFormatterFactory<Geometry> geoFormatterFactory;
         public GeoShapeWithDocValuesFieldType(String name, boolean indexed, boolean hasDocValues,
                                               Orientation orientation, GeoShapeParser parser,
-                                              VectorTileExtension vectorTileExtension, Map<String, String> meta) {
+                                              GeometryFormatterExtension geometryFormatterExtension, Map<String, String> meta) {
             super(name, indexed, false, hasDocValues, parser, orientation, meta);
-            this.vectorTileExtension = vectorTileExtension;
+            this.geoFormatterFactory = new GeoFormatterFactory<>(geometryFormatterExtension.getGeometryFormatterFactories());
         }
 
         public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName, Supplier<SearchLookup> searchLookup) {
@@ -184,22 +184,16 @@ public Query geoShapeQuery(Geometry shape, String fieldName, ShapeRelation relat
 
         @Override
         protected Function<List<Geometry>, List<Object>> getFormatter(String format) {
-            return GeoFormatterFactory.getFormatter(format, Function.identity(),
-                (z, x, y, extent) -> {
-                    if (vectorTileExtension == null) {
-                        throw new IllegalArgumentException("vector tile format is not supported");
-                    }
-                    return vectorTileExtension.getVectorTileEngine().getFormatter(z, x, y, extent);
-                });
+            return geoFormatterFactory.getFormatter(format, Function.identity());
         }
     }
 
     public static class TypeParser implements Mapper.TypeParser {
 
-        private final VectorTileExtension vectorTileExtension;
+        private final GeometryFormatterExtension geometryFormatterExtension;
 
-        public TypeParser(VectorTileExtension vectorTileExtension) {
-            this.vectorTileExtension = vectorTileExtension;
+        public TypeParser(GeometryFormatterExtension geometryFormatterExtension) {
+            this.geometryFormatterExtension = geometryFormatterExtension;
         }
 
         @Override
@@ -226,7 +220,7 @@ public Mapper.Builder parse(String name, Map<String, Object> node, MappingParser
                     parserContext.indexVersionCreated(),
                     ignoreMalformedByDefault,
                     coerceByDefault,
-                    vectorTileExtension);
+                    geometryFormatterExtension);
             }
             builder.parse(name, parserContext, node);
             return builder;
@@ -281,7 +275,7 @@ public FieldMapper.Builder getMergeBuilder() {
             builder.version,
             builder.ignoreMalformed.getDefaultValue().value(),
             builder.coerce.getDefaultValue().value(),
-            builder.vectorTileExtension
+            builder.geometryFormatterExtension
         ).init(this);
     }
 

x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldTypeTests.java

@@ -15,7 +15,7 @@
 import org.elasticsearch.index.mapper.FieldTypeTestCase;
 import org.elasticsearch.index.mapper.GeoShapeFieldMapper;
 import org.elasticsearch.index.mapper.MappedFieldType;
-import org.elasticsearch.xpack.vectortile.SpatialVectorTileExtension;
+import org.elasticsearch.xpack.vectortile.SpatialGeometryFormatterExtension;
 import org.elasticsearch.xpack.vectortile.feature.FeatureFactory;
 import org.hamcrest.Matchers;
 
@@ -90,7 +90,7 @@ public void testFetchVectorTile() throws IOException {
 
     private void fetchVectorTile(Geometry geometry) throws IOException {
         final MappedFieldType mapper
-            = new GeoShapeWithDocValuesFieldMapper.Builder("field", Version.CURRENT, false, false, new SpatialVectorTileExtension())
+            = new GeoShapeWithDocValuesFieldMapper.Builder("field", Version.CURRENT, false, false, new SpatialGeometryFormatterExtension())
             .build(new ContentPath()).fieldType();
         final int z = randomIntBetween(1, 10);
         int x = randomIntBetween(0, (1 << z) - 1);