elastic · imotov · Aug 5, 2021 · Aug 5, 2021
diff --git a/server/src/main/java/org/elasticsearch/common/geo/GeoFormatterFactory.java b/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);
     }
 }
diff --git a/server/src/main/java/org/elasticsearch/common/geo/SimpleFeatureFactory.java b/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;

diff --git a/server/src/main/java/org/elasticsearch/common/geo/SimpleVectorTileFormatter.java b/server/src/main/java/org/elasticsearch/common/geo/SimpleVectorTileFormatter.java
@@ -0,0 +1,66 @@
+/*
+ * 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.Collections;
+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 -> Collections.singletonList(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};
+    }
+}
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/GeoPointFieldMapper.java b/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<>(
+            Collections.singletonList(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 -> org.elasticsearch.core.List.of(featureFactory.points(points));
-                });
+            return GEO_FORMATTER_FACTORY.getFormatter(format, p -> new Point(p.getLon(), p.getLat()));
         }
 
         @Override

diff --git a/...ch/xpack/spatial/VectorTileExtension.java → ...k/spatial/GeometryFormatterExtension.java b/...ch/xpack/spatial/VectorTileExtension.java → ...k/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();
 }
diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/SpatialPlugin.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/SpatialPlugin.java
@@ -10,8 +10,10 @@
 import org.elasticsearch.action.ActionRequest;
 import org.elasticsearch.action.ActionResponse;
 import org.elasticsearch.common.inject.Module;
+import org.elasticsearch.common.geo.GeoFormatterFactory;
 import org.elasticsearch.common.xcontent.ContextParser;
 import org.elasticsearch.geo.GeoPlugin;
+import org.elasticsearch.geometry.Geometry;
 import org.elasticsearch.index.mapper.Mapper;
 import org.elasticsearch.ingest.Processor;
 import org.elasticsearch.license.LicenseUtils;
@@ -54,6 +56,7 @@
 import org.elasticsearch.xpack.spatial.search.aggregations.support.GeoShapeValuesSourceType;
 
 import java.util.Collection;
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
@@ -77,7 +80,7 @@ protected XPackLicenseState getLicenseState() {
     }
 
     // register the vector tile factory from a different module
-    private final SetOnce<VectorTileExtension> vectorTileExtension = new SetOnce<>();
+    private final SetOnce<GeoFormatterFactory<Geometry>> geoFormatterFactory = new SetOnce<>();
 
     @Override
     public List<ActionPlugin.ActionHandler<? extends ActionRequest, ? extends ActionResponse>> getActions() {
@@ -90,7 +93,7 @@ public Map<String, Mapper.TypeParser> getMappers() {
         mappers.put(ShapeFieldMapper.CONTENT_TYPE, ShapeFieldMapper.PARSER);
         mappers.put(PointFieldMapper.CONTENT_TYPE, PointFieldMapper.PARSER);
         mappers.put(GeoShapeWithDocValuesFieldMapper.CONTENT_TYPE,
-            new GeoShapeWithDocValuesFieldMapper.TypeParser(vectorTileExtension.get()));
+            new GeoShapeWithDocValuesFieldMapper.TypeParser(geoFormatterFactory.get()));
         return Collections.unmodifiableMap(mappers);
     }
 
@@ -215,6 +218,9 @@ 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);
+        List<GeoFormatterFactory.FormatterFactory<Geometry>> formatterFactories = new ArrayList<>();
+        loader.loadExtensions(GeometryFormatterExtension.class).stream().map(GeometryFormatterExtension::getGeometryFormatterFactories)
+            .forEach(formatterFactories::addAll);
+        geoFormatterFactory.set(new GeoFormatterFactory<>(formatterFactories));
     }
 }
diff --git a/...n/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapper.java b/...n/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapper.java
@@ -18,14 +18,15 @@
 import org.elasticsearch.common.geo.GeoFormatterFactory;
 import org.elasticsearch.common.geo.GeoShapeUtils;
 import org.elasticsearch.common.geo.GeometryParser;
-import org.elasticsearch.common.geo.ShapeRelation;
 import org.elasticsearch.common.geo.Orientation;
+import org.elasticsearch.common.geo.ShapeRelation;
 import org.elasticsearch.common.logging.DeprecationCategory;
 import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.geometry.Geometry;
 import org.elasticsearch.index.fielddata.IndexFieldData;
 import org.elasticsearch.index.mapper.AbstractShapeGeometryFieldMapper;
 import org.elasticsearch.index.mapper.ContentPath;
+import org.elasticsearch.index.mapper.DocumentParserContext;
 import org.elasticsearch.index.mapper.FieldMapper;
 import org.elasticsearch.index.mapper.GeoShapeFieldMapper;
 import org.elasticsearch.index.mapper.GeoShapeIndexer;
@@ -34,13 +35,11 @@
 import org.elasticsearch.index.mapper.LegacyGeoShapeFieldMapper;
 import org.elasticsearch.index.mapper.MappedFieldType;
 import org.elasticsearch.index.mapper.Mapper;
-import org.elasticsearch.index.mapper.DocumentParserContext;
 import org.elasticsearch.index.mapper.MapperParsingException;
 import org.elasticsearch.index.mapper.MappingParserContext;
 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.index.fielddata.plain.AbstractLatLonShapeIndexFieldData;
 import org.elasticsearch.xpack.spatial.search.aggregations.support.GeoShapeValuesSourceType;
 
@@ -95,13 +94,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 GeoFormatterFactory<Geometry> geoFormatterFactory;
 
         public Builder(String name, Version version, boolean ignoreMalformedByDefault, boolean coerceByDefault,
-                       VectorTileExtension vectorTileExtension) {
+                       GeoFormatterFactory<Geometry> geoFormatterFactory) {
             super(name);
             this.version = version;
-            this.vectorTileExtension = vectorTileExtension;
+            this.geoFormatterFactory = geoFormatterFactory;
             this.ignoreMalformed = ignoreMalformedParam(m -> builder(m).ignoreMalformed.get(), ignoreMalformedByDefault);
             this.coerce = coerceParam(m -> builder(m).coerce.get(), coerceByDefault);
             this.hasDocValues
@@ -133,7 +132,7 @@ public GeoShapeWithDocValuesFieldMapper build(ContentPath contentPath) {
                 hasDocValues.get(),
                 orientation.get().value(),
                 parser,
-                vectorTileExtension,
+                geoFormatterFactory,
                 meta.get());
             return new GeoShapeWithDocValuesFieldMapper(name, ft,
                 multiFieldsBuilder.build(this, contentPath), copyTo.build(),
@@ -144,13 +143,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) {
+                                              GeoFormatterFactory<Geometry> geoFormatterFactory, Map<String, String> meta) {
             super(name, indexed, false, hasDocValues, parser, orientation, meta);
-            this.vectorTileExtension = vectorTileExtension;
+            this.geoFormatterFactory = geoFormatterFactory;
         }
 
         public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName, Supplier<SearchLookup> searchLookup) {
@@ -184,22 +182,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 GeoFormatterFactory<Geometry> geoFormatterFactory;
 
-        public TypeParser(VectorTileExtension vectorTileExtension) {
-            this.vectorTileExtension = vectorTileExtension;
+        public TypeParser(GeoFormatterFactory<Geometry> geoFormatterFactory) {
+            this.geoFormatterFactory = geoFormatterFactory;
         }
 
         @Override
@@ -221,7 +213,7 @@ public Mapper.Builder parse(String name, Map<String, Object> node, MappingParser
                     parserContext.indexVersionCreated(),
                     ignoreMalformedByDefault,
                     coerceByDefault,
-                    vectorTileExtension);
+                    geoFormatterFactory);
             }
             builder.parse(name, parserContext, node);
             return builder;
@@ -276,7 +268,7 @@ public FieldMapper.Builder getMergeBuilder() {
             builder.version,
             builder.ignoreMalformed.getDefaultValue().value(),
             builder.coerce.getDefaultValue().value(),
-            builder.vectorTileExtension
+            builder.geoFormatterFactory
         ).init(this);
     }