Rework geo mappers to index value by value (#71696)

The various geo field mappers are organised in a hierarchy that shares
parsing and indexing code. This ends up over-complicating things,
particularly when we have some mappers that accept multiple values
and others that only accept singletons. It also leads to confusing
behaviour around ignore_malformed behaviour: geo fields will ignore
all values if a single one is badly formed, while all other field mappers
will only ignore the problem value and index the rest. Finally, this
structure makes adding index-time scripts to geo_point needlessly
complex.

This commit refactors the indexing logic of the hierarchy to move the
individual value indexing logic into the concrete implementations,
and aligns the ignore_malformed behaviour with that of other mappers.

It contains two breaking changes:

* The geo field mappers no longer check for external field values on the
  parse context. This added considerable complication to the refactored
  parse methods, and is unused anywhere in our codebase, but may
  impact plugin-based field mappers which expect to use geo fields
  as multifields
* The geo_point field mapper now passes geohashes to its multifields
  one-by-one, instead of formatting them into a comma-delimited
  string and passing them all at once. Completion multifields using
  this as an input should still behave as normal because by default
  they would split this combined geohash string on the commas in any
  case, but keyword subfields may look different.

Fixes #69601

libs/x-content/src/main/java/org/elasticsearch/common/xcontent/support/MapXContentParser.java

@@ -11,12 +11,14 @@ package org.elasticsearch.common.xcontent.support;
 import org.elasticsearch.common.xcontent.DeprecationHandler;
 import org.elasticsearch.common.xcontent.NamedXContentRegistry;
 import org.elasticsearch.common.xcontent.XContentLocation;
+import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.common.xcontent.XContentType;
 
 import java.io.IOException;
 import java.math.BigDecimal;
 import java.math.BigInteger;
 import java.nio.CharBuffer;
+import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
@@ -30,6 +32,17 @@ public class MapXContentParser extends AbstractXContentParser {
     private TokenIterator iterator;
     private boolean closed;
 
+    public static XContentParser wrapObject(Object sourceMap) throws IOException {
+        XContentParser parser = new MapXContentParser(
+            NamedXContentRegistry.EMPTY,
+            DeprecationHandler.IGNORE_DEPRECATIONS,
+            Collections.singletonMap("dummy_field", sourceMap), XContentType.JSON);
+        parser.nextToken(); // start object
+        parser.nextToken(); // field name
+        parser.nextToken(); // field value
+        return parser;
+    }
+
     public MapXContentParser(NamedXContentRegistry xContentRegistry, DeprecationHandler deprecationHandler, Map<String, Object> map,
                              XContentType xContentType) {
         super(xContentRegistry, deprecationHandler);

server/src/internalClusterTest/java/org/elasticsearch/index/mapper/ExternalValuesMapperIntegrationIT.java

@@ -9,14 +9,11 @@
 package org.elasticsearch.index.mapper;
 
 import org.elasticsearch.action.search.SearchResponse;
-import org.elasticsearch.common.geo.ShapeRelation;
-import org.elasticsearch.common.geo.builders.EnvelopeBuilder;
 import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.index.query.QueryBuilders;
 import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.search.fetch.subphase.highlight.HighlightBuilder;
 import org.elasticsearch.test.ESIntegTestCase;
-import org.locationtech.jts.geom.Coordinate;
 
 import java.util.Collection;
 import java.util.Collections;
@@ -99,19 +96,6 @@ public class ExternalValuesMapperIntegrationIT extends ESIntegTestCase {
 
         assertThat(response.getHits().getTotalHits().value, equalTo((long) 1));
 
-        response = client().prepareSearch("test-idx")
-                .setPostFilter(QueryBuilders.geoDistanceQuery("field.point").point(42.0, 51.0).distance("1km"))
-                .execute().actionGet();
-
-        assertThat(response.getHits().getTotalHits().value, equalTo((long) 1));
-
-        response = client().prepareSearch("test-idx")
-                .setPostFilter(QueryBuilders.geoShapeQuery("field.shape",
-                    new EnvelopeBuilder(new Coordinate(-101, 46), new Coordinate(-99, 44))).relation(ShapeRelation.WITHIN))
-                        .execute().actionGet();
-
-        assertThat(response.getHits().getTotalHits().value, equalTo((long) 1));
-
         response = client().prepareSearch("test-idx")
                 .setPostFilter(QueryBuilders.termQuery("field.field", "foo"))
                 .execute().actionGet();

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

@@ -7,31 +7,29 @@
  */
 package org.elasticsearch.index.mapper;
 
-import org.apache.lucene.index.IndexableField;
 import org.apache.lucene.search.Query;
+import org.apache.lucene.util.SetOnce;
+import org.elasticsearch.common.CheckedConsumer;
 import org.elasticsearch.common.Explicit;
 import org.elasticsearch.common.geo.GeoJsonGeometryFormat;
-import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
-import org.elasticsearch.common.xcontent.NamedXContentRegistry;
 import org.elasticsearch.common.xcontent.XContentParser;
-import org.elasticsearch.common.xcontent.XContentType;
 import org.elasticsearch.common.xcontent.support.MapXContentParser;
 import org.elasticsearch.index.analysis.NamedAnalyzer;
 import org.elasticsearch.index.query.SearchExecutionContext;
 
 import java.io.IOException;
 import java.io.UncheckedIOException;
-import java.text.ParseException;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.function.Consumer;
 import java.util.function.Function;
 
 /**
  * Base field mapper class for all spatial field types
  */
-public abstract class AbstractGeometryFieldMapper<Parsed, Processed> extends FieldMapper {
+public abstract class AbstractGeometryFieldMapper<T> extends FieldMapper {
 
     public static Parameter<Explicit<Boolean>> ignoreMalformedParam(Function<FieldMapper, Explicit<Boolean>> initializer,
                                                                     boolean ignoreMalformedByDefault) {
@@ -42,24 +40,18 @@ public abstract class AbstractGeometryFieldMapper<Parsed, Processed> extends Fie
         return Parameter.explicitBoolParam("ignore_z_value", true, initializer, true);
     }
 
-    /**
-     * Interface representing an preprocessor in geometry indexing pipeline
-     */
-    public interface Indexer<Parsed, Processed> {
-        Processed prepareForIndexing(Parsed geometry);
-        Class<Processed> processedClass();
-        List<IndexableField> indexShape(ParseContext context, Processed shape);
-    }
-
     /**
      * Interface representing parser in geometry indexing pipeline.
      */
-    public abstract static class Parser<Parsed> {
+    public abstract static class Parser<T> {
         /**
-         * Parse the given xContent value to an object of type {@link Parsed}. The value can be
+         * Parse the given xContent value to one or more objects of type {@link T}. The value can be
          * in any supported format.
          */
-        public abstract Parsed parse(XContentParser parser) throws IOException, ParseException;
+        public abstract void parse(
+            XContentParser parser,
+            CheckedConsumer<T, IOException> consumer,
+            Consumer<Exception> onMalformed) throws IOException;
 
         /**
          * Given a parsed value and a format string, formats the value into a plain Java object.
@@ -67,27 +59,14 @@ public abstract class AbstractGeometryFieldMapper<Parsed, Processed> extends Fie
          * Supported formats include 'geojson' and 'wkt'. The different formats are defined
          * as subclasses of {@link org.elasticsearch.common.geo.GeometryFormat}.
          */
-        public abstract Object format(Parsed value, String format);
+        public abstract Object format(T value, String format);
 
-        /**
-         * Parses the given value, then formats it according to the 'format' string.
-         *
-         * Used by value fetchers to validate and format geo objects
-         */
-        public Object parseAndFormatObject(Object value, String format) {
-            Parsed geometry;
-            try (XContentParser parser = new MapXContentParser(NamedXContentRegistry.EMPTY, LoggingDeprecationHandler.INSTANCE,
-                Collections.singletonMap("dummy_field", value), XContentType.JSON)) {
-                parser.nextToken(); // start object
-                parser.nextToken(); // field name
-                parser.nextToken(); // field value
-                geometry = parse(parser);
+        private void fetchFromSource(Object sourceMap, Consumer<Object> consumer, String format) {
+            try (XContentParser parser = MapXContentParser.wrapObject(sourceMap)) {
+                parse(parser, v -> consumer.accept(format(v, format)), e -> {}); /* ignore malformed */
             } catch (IOException e) {
                 throw new UncheckedIOException(e);
-            } catch (ParseException e) {
-                throw new RuntimeException(e);
             }
-            return format(geometry, format);
         }
     }
 
@@ -113,19 +92,22 @@ public abstract class AbstractGeometryFieldMapper<Parsed, Processed> extends Fie
         public final ValueFetcher valueFetcher(SearchExecutionContext context, String format) {
             String geoFormat = format != null ? format : GeoJsonGeometryFormat.NAME;
 
-            Function<Object, Object> valueParser = value -> geometryParser.parseAndFormatObject(value, geoFormat);
             if (parsesArrayValue) {
                 return new ArraySourceValueFetcher(name(), context) {
                     @Override
                     protected Object parseSourceValue(Object value) {
-                        return valueParser.apply(value);
+                        List<Object> values = new ArrayList<>();
+                        geometryParser.fetchFromSource(value, values::add, geoFormat);
+                        return values;
                     }
                 };
             } else {
                 return new SourceValueFetcher(name(), context) {
                     @Override
                     protected Object parseSourceValue(Object value) {
-                        return valueParser.apply(value);
+                        SetOnce<Object> holder = new SetOnce<>();
+                        geometryParser.fetchFromSource(value, holder::set, geoFormat);
+                        return holder.get();
                     }
                 };
             }
@@ -134,26 +116,24 @@ public abstract class AbstractGeometryFieldMapper<Parsed, Processed> extends Fie
 
     private final Explicit<Boolean> ignoreMalformed;
     private final Explicit<Boolean> ignoreZValue;
-    private final Indexer<Parsed, Processed> indexer;
-    private final Parser<Parsed> parser;
+    private final Parser<T> parser;
 
     protected AbstractGeometryFieldMapper(String simpleName, MappedFieldType mappedFieldType,
                                           Map<String, NamedAnalyzer> indexAnalyzers,
                                           Explicit<Boolean> ignoreMalformed, Explicit<Boolean> ignoreZValue,
                                           MultiFields multiFields, CopyTo copyTo,
-                                          Indexer<Parsed, Processed> indexer, Parser<Parsed> parser) {
+                                          Parser<T> parser) {
         super(simpleName, mappedFieldType, indexAnalyzers, multiFields, copyTo, false, null);
         this.ignoreMalformed = ignoreMalformed;
         this.ignoreZValue = ignoreZValue;
-        this.indexer = indexer;
         this.parser = parser;
     }
 
     protected AbstractGeometryFieldMapper(String simpleName, MappedFieldType mappedFieldType,
                                           Explicit<Boolean> ignoreMalformed, Explicit<Boolean> ignoreZValue,
                                           MultiFields multiFields, CopyTo copyTo,
-                                          Indexer<Parsed, Processed> indexer, Parser<Parsed> parser) {
-        this(simpleName, mappedFieldType, Collections.emptyMap(), ignoreMalformed, ignoreZValue, multiFields, copyTo, indexer, parser);
+                                          Parser<T> parser) {
+        this(simpleName, mappedFieldType, Collections.emptyMap(), ignoreMalformed, ignoreZValue, multiFields, copyTo, parser);
     }
 
     @Override
@@ -166,60 +146,25 @@ public abstract class AbstractGeometryFieldMapper<Parsed, Processed> extends Fie
         throw new UnsupportedOperationException("Parsing is implemented in parse(), this method should NEVER be called");
     }
 
-    protected abstract void addStoredFields(ParseContext context, Processed geometry);
-    protected abstract void addDocValuesFields(String name, Processed geometry, List<IndexableField> fields, ParseContext context);
-    protected abstract void addMultiFields(ParseContext context, Processed geometry) throws IOException;
+    /**
+     * Build an index document using a parsed geometry
+     * @param context   the ParseContext holding the document
+     * @param geometry  the parsed geometry object
+     */
+    protected abstract void index(ParseContext context, T geometry) throws IOException;
 
-    /** parsing logic for geometry indexing */
     @Override
-    public void parse(ParseContext context) throws IOException {
-        MappedFieldType mappedFieldType = fieldType();
-
-        try {
-            Processed shape = context.parseExternalValue(indexer.processedClass());
-            if (shape == null) {
-                Parsed geometry = parser.parse(context.parser());
-                if (geometry == null) {
-                    return;
-                }
-                shape = indexer.prepareForIndexing(geometry);
-            }
-
-            List<IndexableField> fields = new ArrayList<>();
-            if (mappedFieldType.isSearchable() || mappedFieldType.hasDocValues()) {
-                fields.addAll(indexer.indexShape(context, shape));
-            }
-
-            // indexed:
-            List<IndexableField> indexedFields = new ArrayList<>();
-            if (mappedFieldType.isSearchable()) {
-                indexedFields.addAll(fields);
-            }
-            // stored:
-            if (fieldType().isStored()) {
-                addStoredFields(context, shape);
-            }
-            // docValues:
-            if (fieldType().hasDocValues()) {
-                addDocValuesFields(mappedFieldType.name(), shape, fields, context);
-            } else if (fieldType().isStored() || fieldType().isSearchable()) {
-                createFieldNamesField(context);
-            }
-
-            // add the indexed fields to the doc:
-            for (IndexableField field : indexedFields) {
-                context.doc().add(field);
-            }
-
-            // add multifields (e.g., used for completion suggester)
-            addMultiFields(context, shape);
-        } catch (Exception e) {
-            if (ignoreMalformed.value() == false) {
-                throw new MapperParsingException("failed to parse field [{}] of type [{}]", e, fieldType().name(),
-                    fieldType().typeName());
-            }
-            context.addIgnoredField(mappedFieldType.name());
-        }
+    public final void parse(ParseContext context) throws IOException {
+      parser.parse(context.parser(), v -> index(context, v), e -> {
+          if (ignoreMalformed()) {
+              context.addIgnoredField(fieldType().name());
+          } else {
+              throw new MapperParsingException(
+                  "Failed to parse field [" + fieldType().name() + "] of type [" + contentType() + "]",
+                  e
+              );
+          }
+      });
     }
 
     public boolean ignoreMalformed() {

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

@@ -9,6 +9,7 @@ package org.elasticsearch.index.mapper;
 
 import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.common.CheckedBiFunction;
+import org.elasticsearch.common.CheckedConsumer;
 import org.elasticsearch.common.Explicit;
 import org.elasticsearch.common.TriFunction;
 import org.elasticsearch.common.geo.GeoPoint;
@@ -20,15 +21,12 @@ import org.elasticsearch.geometry.Point;
 import org.elasticsearch.index.mapper.Mapper.TypeParser.ParserContext;
 
 import java.io.IOException;
-import java.text.ParseException;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.List;
+import java.util.function.Consumer;
 import java.util.function.Function;
 import java.util.function.Supplier;
 
 /** Base class for for spatial fields that only support indexing points */
-public abstract class AbstractPointGeometryFieldMapper<Parsed, Processed> extends AbstractGeometryFieldMapper<Parsed, Processed> {
+public abstract class AbstractPointGeometryFieldMapper<T> extends AbstractGeometryFieldMapper<T> {
 
     public static Parameter<ParsedPoint> nullValueParam(Function<FieldMapper, ParsedPoint> initializer,
                                                         TriFunction<String, ParserContext, Object, ParsedPoint> parser,
@@ -41,8 +39,8 @@ public abstract class AbstractPointGeometryFieldMapper<Parsed, Processed> extend
     protected AbstractPointGeometryFieldMapper(String simpleName, MappedFieldType mappedFieldType,
                                                MultiFields multiFields, Explicit<Boolean> ignoreMalformed,
                                                Explicit<Boolean> ignoreZValue, ParsedPoint nullValue, CopyTo copyTo,
-                                               Indexer<Parsed, Processed> indexer, Parser<Parsed> parser) {
-        super(simpleName, mappedFieldType, ignoreMalformed, ignoreZValue, multiFields, copyTo, indexer, parser);
+                                               Parser<T> parser) {
+        super(simpleName, mappedFieldType, ignoreMalformed, ignoreZValue, multiFields, copyTo, parser);
         this.nullValue = nullValue;
     }
 
@@ -67,7 +65,7 @@ public abstract class AbstractPointGeometryFieldMapper<Parsed, Processed> extend
     }
 
     /** A parser implementation that can parse the various point formats */
-    public static class PointParser<P extends ParsedPoint> extends Parser<List<P>> {
+    public static class PointParser<P extends ParsedPoint> extends Parser<P> {
         /**
          * Note that this parser is only used for formatting values.
          */
@@ -88,7 +86,7 @@ public abstract class AbstractPointGeometryFieldMapper<Parsed, Processed> extend
             this.field = field;
             this.pointSupplier = pointSupplier;
             this.objectParser = objectParser;
-            this.nullValue = nullValue;
+            this.nullValue = nullValue == null ? null : process(nullValue);
             this.ignoreZValue = ignoreZValue;
             this.ignoreMalformed = ignoreMalformed;
             this.geometryParser = new GeometryParser(true, true, true);
@@ -104,12 +102,14 @@ public abstract class AbstractPointGeometryFieldMapper<Parsed, Processed> extend
         }
 
         @Override
-        public List<P> parse(XContentParser parser) throws IOException, ParseException {
-
+        public void parse(
+            XContentParser parser,
+            CheckedConsumer<P, IOException> consumer,
+            Consumer<Exception> onMalformed
+        ) throws IOException {
             if (parser.currentToken() == XContentParser.Token.START_ARRAY) {
                 XContentParser.Token token = parser.nextToken();
                 P point = pointSupplier.get();
-                ArrayList<P> points = new ArrayList<>();
                 if (token == XContentParser.Token.VALUE_NUMBER) {
                     double x = parser.doubleValue();
                     parser.nextToken();
@@ -122,35 +122,41 @@ public abstract class AbstractPointGeometryFieldMapper<Parsed, Processed> extend
                     }
 
                     point.resetCoords(x, y);
-                    points.add(process(point));
+                    consumer.accept(process(point));
                 } else {
                     while (token != XContentParser.Token.END_ARRAY) {
-                        points.add(process(objectParser.apply(parser, point)));
+                        parseAndConsumeFromObject(parser, point, consumer, onMalformed);
                         point = pointSupplier.get();
                         token = parser.nextToken();
                     }
                 }
-                return points;
             } else if (parser.currentToken() == XContentParser.Token.VALUE_NULL) {
-                if (nullValue == null) {
-                    return null;
-                } else {
-                    return Collections.singletonList(nullValue);
+                if (nullValue != null) {
+                    consumer.accept(nullValue);
                 }
             } else {
-                return Collections.singletonList(process(objectParser.apply(parser, pointSupplier.get())));
+                parseAndConsumeFromObject(parser, pointSupplier.get(), consumer, onMalformed);
+            }
+        }
+
+        private void parseAndConsumeFromObject(
+            XContentParser parser,
+            P point,
+            CheckedConsumer<P, IOException> consumer,
+            Consumer<Exception> onMalformed
+        ) {
+            try {
+                point = objectParser.apply(parser, point);
+                consumer.accept(process(point));
+            } catch (Exception e) {
+                onMalformed.accept(e);
             }
         }
 
         @Override
-        public Object format(List<P> points, String format) {
-            List<Object> result = new ArrayList<>();
+        public Object format(P point, String format) {
             GeometryFormat<Geometry> geometryFormat = geometryParser.geometryFormat(format);
-            for (ParsedPoint point : points) {
-                Geometry geometry = point.asGeometry();
-                result.add(geometryFormat.toXContentAsObject(geometry));
-            }
-            return result;
+            return geometryFormat.toXContentAsObject(point.asGeometry());
         }
     }
 }

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

@@ -19,7 +19,7 @@ import java.util.function.Function;
 /**
  * Base class for {@link GeoShapeFieldMapper} and {@link LegacyGeoShapeFieldMapper}
  */
-public abstract class AbstractShapeGeometryFieldMapper<Parsed, Processed> extends AbstractGeometryFieldMapper<Parsed, Processed> {
+public abstract class AbstractShapeGeometryFieldMapper<T> extends AbstractGeometryFieldMapper<T> {
 
     public static Parameter<Explicit<Boolean>> coerceParam(Function<FieldMapper, Explicit<Boolean>> initializer,
                                                            boolean coerceByDefault) {
@@ -56,8 +56,8 @@ public abstract class AbstractShapeGeometryFieldMapper<Parsed, Processed> extend
                                                Explicit<Boolean> ignoreMalformed, Explicit<Boolean> coerce,
                                                Explicit<Boolean> ignoreZValue, Explicit<Orientation> orientation,
                                                MultiFields multiFields, CopyTo copyTo,
-                                               Indexer<Parsed, Processed> indexer, Parser<Parsed> parser) {
-        super(simpleName, mappedFieldType, indexAnalyzers, ignoreMalformed, ignoreZValue, multiFields, copyTo, indexer, parser);
+                                               Parser<T> parser) {
+        super(simpleName, mappedFieldType, indexAnalyzers, ignoreMalformed, ignoreZValue, multiFields, copyTo, parser);
         this.coerce = coerce;
         this.orientation = orientation;
     }
@@ -66,9 +66,9 @@ public abstract class AbstractShapeGeometryFieldMapper<Parsed, Processed> extend
                                                Explicit<Boolean> ignoreMalformed, Explicit<Boolean> coerce,
                                                Explicit<Boolean> ignoreZValue, Explicit<Orientation> orientation,
                                                MultiFields multiFields, CopyTo copyTo,
-                                               Indexer<Parsed, Processed> indexer, Parser<Parsed> parser) {
+                                               Parser<T> parser) {
         this(simpleName, mappedFieldType, Collections.emptyMap(),
-            ignoreMalformed, coerce, ignoreZValue, orientation, multiFields, copyTo, indexer, parser);
+            ignoreMalformed, coerce, ignoreZValue, orientation, multiFields, copyTo, parser);
     }
 
     @Override

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

@@ -11,7 +11,6 @@ import org.apache.lucene.document.LatLonDocValuesField;
 import org.apache.lucene.document.LatLonPoint;
 import org.apache.lucene.document.StoredField;
 import org.apache.lucene.geo.LatLonGeometry;
-import org.apache.lucene.index.IndexableField;
 import org.apache.lucene.search.IndexOrDocValuesQuery;
 import org.apache.lucene.search.MatchNoDocsQuery;
 import org.apache.lucene.search.Query;
@@ -32,7 +31,6 @@ import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
 import org.elasticsearch.search.lookup.SearchLookup;
 
 import java.io.IOException;
-import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
@@ -44,7 +42,7 @@ import java.util.function.Supplier;
  *
  * Uses lucene 6 LatLonPoint encoding
  */
-public class GeoPointFieldMapper extends AbstractPointGeometryFieldMapper<List<ParsedGeoPoint>, List<? extends GeoPoint>> {
+public class GeoPointFieldMapper extends AbstractPointGeometryFieldMapper<ParsedGeoPoint> {
 
     public static final String CONTENT_TYPE = "geo_point";
 
@@ -102,14 +100,20 @@ public class GeoPointFieldMapper extends AbstractPointGeometryFieldMapper<List<P
 
         @Override
         public FieldMapper build(ContentPath contentPath) {
-            Parser<List<ParsedGeoPoint>> geoParser = new PointParser<>(name, ParsedGeoPoint::new, (parser, point) -> {
-                GeoUtils.parseGeoPoint(parser, point, ignoreZValue.get().value());
-                return point;
-            }, (ParsedGeoPoint) nullValue.get(), ignoreZValue.get().value(), ignoreMalformed.get().value());
+            Parser<ParsedGeoPoint> geoParser = new PointParser<>(
+                name,
+                ParsedGeoPoint::new,
+                (parser, point) -> {
+                    GeoUtils.parseGeoPoint(parser, point, ignoreZValue.get().value());
+                    return point;
+                },
+                (ParsedGeoPoint) nullValue.get(),
+                ignoreZValue.get().value(),
+                ignoreMalformed.get().value());
             GeoPointFieldType ft
                 = new GeoPointFieldType(buildFullName(contentPath), indexed.get(), stored.get(), hasDocValues.get(), geoParser, meta.get());
             return new GeoPointFieldMapper(name, ft, multiFieldsBuilder.build(this, contentPath),
-                copyTo.build(), new GeoPointIndexer(ft), geoParser, this);
+                copyTo.build(), geoParser, this);
         }
 
     }
@@ -120,12 +124,11 @@ public class GeoPointFieldMapper extends AbstractPointGeometryFieldMapper<List<P
 
     public GeoPointFieldMapper(String simpleName, MappedFieldType mappedFieldType,
                                MultiFields multiFields, CopyTo copyTo,
-                               Indexer<List<ParsedGeoPoint>, List<? extends GeoPoint>> indexer,
-                               Parser<List<ParsedGeoPoint>> parser,
+                               Parser<ParsedGeoPoint> parser,
                                Builder builder) {
         super(simpleName, mappedFieldType, multiFields,
             builder.ignoreMalformed.get(), builder.ignoreZValue.get(), builder.nullValue.get(),
-            copyTo, indexer, parser);
+            copyTo, parser);
         this.builder = builder;
     }
 
@@ -135,39 +138,20 @@ public class GeoPointFieldMapper extends AbstractPointGeometryFieldMapper<List<P
     }
 
     @Override
-    protected void addStoredFields(ParseContext context, List<? extends GeoPoint> points) {
-        for (GeoPoint point : points) {
-            context.doc().add(new StoredField(fieldType().name(), point.toString()));
+    protected void index(ParseContext context, ParsedGeoPoint geometry) throws IOException {
+        if (fieldType().isSearchable()) {
+            context.doc().add(new LatLonPoint(fieldType().name(), geometry.lat(), geometry.lon()));
         }
-    }
-
-    @Override
-    protected void addMultiFields(ParseContext context, List<? extends GeoPoint> points) throws IOException {
-        // @todo phase out geohash (which is currently used in the CompletionSuggester)
-        if (points.isEmpty()) {
-            return;
+        if (fieldType().hasDocValues()) {
+            context.doc().add(new LatLonDocValuesField(fieldType().name(), geometry.lat(), geometry.lon()));
+        } else if (fieldType().isStored() || fieldType().isSearchable()) {
+            createFieldNamesField(context);
         }
-
-        StringBuilder s = new StringBuilder();
-        if (points.size() > 1) {
-            s.append('[');
-        }
-        s.append(points.get(0).geohash());
-        for (int i = 1; i < points.size(); ++i) {
-            s.append(',');
-            s.append(points.get(i).geohash());
-        }
-        if (points.size() > 1) {
-            s.append(']');
-        }
-        multiFields.parse(this, context.createExternalValueContext(s));
-    }
-
-    @Override
-    protected void addDocValuesFields(String name, List<? extends GeoPoint> points, List<IndexableField> fields, ParseContext context) {
-        for (GeoPoint point : points) {
-            context.doc().add(new LatLonDocValuesField(fieldType().name(), point.lat(), point.lon()));
+        if (fieldType().isStored()) {
+            context.doc().add(new StoredField(fieldType().name(), geometry.toString()));
         }
+        // TODO phase out geohash (which is currently used in the CompletionSuggester)
+        multiFields.parse(this, context.createExternalValueContext(geometry.geohash()));
     }
 
     @Override
@@ -286,35 +270,4 @@ public class GeoPointFieldMapper extends AbstractPointGeometryFieldMapper<List<P
             return super.hashCode();
         }
     }
-
-    protected static class GeoPointIndexer implements Indexer<List<ParsedGeoPoint>, List<? extends GeoPoint>> {
-
-        protected final GeoPointFieldType fieldType;
-
-        GeoPointIndexer(GeoPointFieldType fieldType) {
-            this.fieldType = fieldType;
-        }
-
-        @Override
-        public List<? extends GeoPoint> prepareForIndexing(List<ParsedGeoPoint> geoPoints) {
-            if (geoPoints == null || geoPoints.isEmpty()) {
-                return Collections.emptyList();
-            }
-            return geoPoints;
-        }
-
-        @Override
-        public Class<List<? extends GeoPoint>> processedClass() {
-            return (Class<List<? extends GeoPoint>>)(Object)List.class;
-        }
-
-        @Override
-        public List<IndexableField> indexShape(ParseContext context, List<? extends GeoPoint> points) {
-            ArrayList<IndexableField> fields = new ArrayList<>(points.size());
-            for (GeoPoint point : points) {
-                fields.add(new LatLonPoint(fieldType.name(), point.lat(), point.lon()));
-            }
-            return fields;
-        }
-    }
 }

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

@@ -9,7 +9,6 @@ package org.elasticsearch.index.mapper;
 
 import org.apache.lucene.document.LatLonShape;
 import org.apache.lucene.geo.LatLonGeometry;
-import org.apache.lucene.index.IndexableField;
 import org.apache.lucene.search.MatchNoDocsQuery;
 import org.apache.lucene.search.Query;
 import org.elasticsearch.Version;
@@ -19,9 +18,10 @@ import org.elasticsearch.common.geo.GeometryParser;
 import org.elasticsearch.common.geo.ShapeRelation;
 import org.elasticsearch.common.geo.builders.ShapeBuilder.Orientation;
 import org.elasticsearch.geometry.Geometry;
-import org.elasticsearch.index.query.SearchExecutionContext;
 import org.elasticsearch.index.query.QueryShardException;
+import org.elasticsearch.index.query.SearchExecutionContext;
 
+import java.io.IOException;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
@@ -46,7 +46,7 @@ import java.util.Map;
  * <p>
  * "field" : "POLYGON ((100.0 0.0, 101.0 0.0, 101.0 1.0, 100.0 1.0, 100.0 0.0))
  */
-public class GeoShapeFieldMapper extends AbstractShapeGeometryFieldMapper<Geometry, Geometry> {
+public class GeoShapeFieldMapper extends AbstractShapeGeometryFieldMapper<Geometry> {
 
     public static final String CONTENT_TYPE = "geo_shape";
 
@@ -146,10 +146,12 @@ public class GeoShapeFieldMapper extends AbstractShapeGeometryFieldMapper<Geomet
     };
 
     private final Builder builder;
+    private final GeoShapeIndexer indexer;
 
     public GeoShapeFieldMapper(String simpleName, MappedFieldType mappedFieldType,
                                MultiFields multiFields, CopyTo copyTo,
-                               Indexer<Geometry, Geometry> indexer, Parser<Geometry> parser, Builder builder) {
+                               GeoShapeIndexer indexer,
+                               Parser<Geometry> parser, Builder builder) {
         super(simpleName,
             mappedFieldType,
             builder.ignoreMalformed.get(),
@@ -158,9 +160,9 @@ public class GeoShapeFieldMapper extends AbstractShapeGeometryFieldMapper<Geomet
             builder.orientation.get(),
             multiFields,
             copyTo,
-            indexer,
             parser);
         this.builder = builder;
+        this.indexer = indexer;
     }
 
     @Override
@@ -184,19 +186,9 @@ public class GeoShapeFieldMapper extends AbstractShapeGeometryFieldMapper<Geomet
     }
 
     @Override
-    protected void addStoredFields(ParseContext context, Geometry geometry) {
-        // noop: we currently do not store geo_shapes
-        // @todo store as geojson string?
-    }
-
-    @Override
-    protected void addDocValuesFields(String name, Geometry geometry, List<IndexableField> fields, ParseContext context) {
-        // we will throw a mapping exception before we get here
-    }
-
-    @Override
-    protected void addMultiFields(ParseContext context, Geometry geometry) {
-        // noop (completion suggester currently not compatible with geo_shape)
+    protected void index(ParseContext context, Geometry geometry) throws IOException {
+        context.doc().addAll(indexer.indexShape(geometry));
+        createFieldNamesField(context);
     }
 
     @Override

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

@@ -14,8 +14,8 @@ import org.apache.lucene.geo.GeoEncodingUtils;
 import org.apache.lucene.index.IndexableField;
 import org.elasticsearch.common.geo.GeoLineDecomposer;
 import org.elasticsearch.common.geo.GeoPolygonDecomposer;
-import org.elasticsearch.common.geo.GeoShapeUtils;
 import org.elasticsearch.common.geo.GeoShapeType;
+import org.elasticsearch.common.geo.GeoShapeUtils;
 import org.elasticsearch.common.geo.GeoUtils;
 import org.elasticsearch.geometry.Circle;
 import org.elasticsearch.geometry.Geometry;
@@ -32,6 +32,7 @@ import org.elasticsearch.geometry.Rectangle;
 
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 
 import static org.elasticsearch.common.geo.GeoUtils.normalizePoint;
@@ -39,7 +40,7 @@ import static org.elasticsearch.common.geo.GeoUtils.normalizePoint;
 /**
  * Utility class that converts geometries into Lucene-compatible form for indexing in a geo_shape field.
  */
-public class GeoShapeIndexer implements AbstractGeometryFieldMapper.Indexer<Geometry, Geometry> {
+public class GeoShapeIndexer {
 
     private final boolean orientation;
     private final String name;
@@ -166,14 +167,12 @@ public class GeoShapeIndexer implements AbstractGeometryFieldMapper.Indexer<Geom
         });
     }
 
-    @Override
-    public Class<Geometry> processedClass() {
-        return Geometry.class;
-    }
-
-    @Override
-    public List<IndexableField> indexShape(ParseContext context, Geometry shape) {
+    public List<IndexableField> indexShape(Geometry shape) {
         LuceneGeometryIndexer visitor = new LuceneGeometryIndexer(name);
+        shape = prepareForIndexing(shape);
+        if (shape == null) {
+            return Collections.emptyList();
+        }
         shape.visit(visitor);
         return visitor.fields();
     }

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

@@ -8,12 +8,15 @@
 
 package org.elasticsearch.index.mapper;
 
+import org.elasticsearch.ElasticsearchParseException;
+import org.elasticsearch.common.CheckedConsumer;
 import org.elasticsearch.common.geo.GeometryParser;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.geometry.Geometry;
 
 import java.io.IOException;
 import java.text.ParseException;
+import java.util.function.Consumer;
 
 public class GeoShapeParser extends AbstractGeometryFieldMapper.Parser<Geometry> {
     private final GeometryParser geometryParser;
@@ -23,8 +26,16 @@ public class GeoShapeParser extends AbstractGeometryFieldMapper.Parser<Geometry>
     }
 
     @Override
-    public Geometry parse(XContentParser parser) throws IOException, ParseException {
-        return geometryParser.parse(parser);
+    public void parse(
+        XContentParser parser,
+        CheckedConsumer<Geometry, IOException> consumer,
+        Consumer<Exception> onMalformed
+    ) throws IOException {
+        try {
+            consumer.accept(geometryParser.parse(parser));
+        } catch (ParseException | ElasticsearchParseException | IllegalArgumentException e) {
+            onMalformed.accept(e);
+        }
     }
 
     @Override

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

@@ -7,7 +7,6 @@
  */
 package org.elasticsearch.index.mapper;
 
-import org.apache.lucene.index.IndexableField;
 import org.apache.lucene.search.Query;
 import org.apache.lucene.spatial.prefix.PrefixTreeStrategy;
 import org.apache.lucene.spatial.prefix.RecursivePrefixTreeStrategy;
@@ -18,12 +17,14 @@ import org.apache.lucene.spatial.prefix.tree.QuadPrefixTree;
 import org.apache.lucene.spatial.prefix.tree.SpatialPrefixTree;
 import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.Version;
+import org.elasticsearch.common.CheckedConsumer;
 import org.elasticsearch.common.Explicit;
 import org.elasticsearch.common.geo.GeoUtils;
 import org.elasticsearch.common.geo.GeometryParser;
 import org.elasticsearch.common.geo.ShapeRelation;
 import org.elasticsearch.common.geo.ShapesAvailability;
 import org.elasticsearch.common.geo.SpatialStrategy;
+import org.elasticsearch.common.geo.XShapeCollection;
 import org.elasticsearch.common.geo.builders.ShapeBuilder;
 import org.elasticsearch.common.geo.builders.ShapeBuilder.Orientation;
 import org.elasticsearch.common.geo.parsers.ShapeParser;
@@ -34,16 +35,18 @@ import org.elasticsearch.common.xcontent.support.XContentMapValues;
 import org.elasticsearch.geometry.Geometry;
 import org.elasticsearch.index.query.LegacyGeoShapeQueryProcessor;
 import org.elasticsearch.index.query.SearchExecutionContext;
+import org.locationtech.spatial4j.shape.Point;
 import org.locationtech.spatial4j.shape.Shape;
+import org.locationtech.spatial4j.shape.jts.JtsGeometry;
 
 import java.io.IOException;
-import java.text.ParseException;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
+import java.util.function.Consumer;
 
 /**
  * FieldMapper for indexing {@link org.locationtech.spatial4j.shape.Shape}s.
@@ -68,7 +71,7 @@ import java.util.Set;
  * @deprecated use {@link GeoShapeFieldMapper}
  */
 @Deprecated
-public class LegacyGeoShapeFieldMapper extends AbstractShapeGeometryFieldMapper<ShapeBuilder<?, ?, ?>, Shape> {
+public class LegacyGeoShapeFieldMapper extends AbstractShapeGeometryFieldMapper<ShapeBuilder<?, ?, ?>> {
 
     public static final String CONTENT_TYPE = "geo_shape";
 
@@ -300,14 +303,12 @@ public class LegacyGeoShapeFieldMapper extends AbstractShapeGeometryFieldMapper<
             GeoShapeFieldType ft = buildFieldType(parser, contentPath);
             return new LegacyGeoShapeFieldMapper(name, ft,
                 multiFieldsBuilder.build(this, contentPath), copyTo.build(),
-                new LegacyGeoShapeIndexer(ft), parser, this);
+                parser, this);
         }
     }
 
     private static class LegacyGeoShapeParser extends Parser<ShapeBuilder<?, ?, ?>> {
-        /**
-         * Note that this parser is only used for formatting values.
-         */
+
         private final GeometryParser geometryParser;
 
         private LegacyGeoShapeParser() {
@@ -315,8 +316,16 @@ public class LegacyGeoShapeFieldMapper extends AbstractShapeGeometryFieldMapper<
         }
 
         @Override
-        public ShapeBuilder<?, ?, ?> parse(XContentParser parser) throws IOException, ParseException {
-            return ShapeParser.parse(parser);
+        public void parse(
+            XContentParser parser,
+            CheckedConsumer<ShapeBuilder<?, ?, ?>, IOException> consumer,
+            Consumer<Exception> onMalformed
+        ) throws IOException {
+            try {
+                consumer.accept(ShapeParser.parse(parser));
+            } catch (ElasticsearchParseException e) {
+                onMalformed.accept(e);
+            }
         }
 
         @Override
@@ -447,11 +456,11 @@ public class LegacyGeoShapeFieldMapper extends AbstractShapeGeometryFieldMapper<
 
     public LegacyGeoShapeFieldMapper(String simpleName, MappedFieldType mappedFieldType,
                                      MultiFields multiFields, CopyTo copyTo,
-                                     LegacyGeoShapeIndexer indexer, LegacyGeoShapeParser parser,
+                                     LegacyGeoShapeParser parser,
                                      Builder builder) {
         super(simpleName, mappedFieldType, Collections.singletonMap(mappedFieldType.name(), Lucene.KEYWORD_ANALYZER),
             builder.ignoreMalformed.get(), builder.coerce.get(), builder.ignoreZValue.get(), builder.orientation.get(),
-            multiFields, copyTo, indexer, parser);
+            multiFields, copyTo, parser);
         this.indexCreatedVersion = builder.indexCreatedVersion;
         this.builder = builder;
     }
@@ -466,18 +475,25 @@ public class LegacyGeoShapeFieldMapper extends AbstractShapeGeometryFieldMapper<
     }
 
     @Override
-    protected void addStoredFields(ParseContext context, Shape geometry) {
-        // noop: we do not store geo_shapes; and will not store legacy geo_shape types
-    }
-
-    @Override
-    protected void addDocValuesFields(String name, Shape geometry, List<IndexableField> fields, ParseContext context) {
-        // doc values are not supported
-    }
-
-    @Override
-    protected void addMultiFields(ParseContext context, Shape geometry) {
-        // noop (completion suggester currently not compatible with geo_shape)
+    protected void index(ParseContext context, ShapeBuilder<?, ?, ?> shapeBuilder) throws IOException {
+        Shape shape = shapeBuilder.buildS4J();
+        if (fieldType().pointsOnly()) {
+            // index configured for pointsOnly
+            if (shape instanceof XShapeCollection && ((XShapeCollection<?>) shape).pointsOnly()) {
+                // MULTIPOINT data: index each point separately
+                @SuppressWarnings("unchecked") List<Shape> shapes = ((XShapeCollection<Shape>) shape).getShapes();
+                for (Shape s : shapes) {
+                    context.doc().addAll(Arrays.asList(fieldType().defaultPrefixTreeStrategy().createIndexableFields(s)));
+                }
+                return;
+            } else if (shape instanceof Point == false) {
+                throw new MapperParsingException("[{" + fieldType().name() + "}] is configured for points only but a "
+                    + ((shape instanceof JtsGeometry) ? ((JtsGeometry)shape).getGeom().getGeometryType() : shape.getClass())
+                    + " was found");
+            }
+        }
+        context.doc().addAll(Arrays.asList(fieldType().defaultPrefixTreeStrategy().createIndexableFields(shape)));
+        createFieldNamesField(context);
     }
 
     @Override

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

@@ -1,61 +0,0 @@
-/*
- * 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.index.mapper;
-
-import org.apache.lucene.index.IndexableField;
-import org.elasticsearch.common.geo.XShapeCollection;
-import org.elasticsearch.common.geo.builders.ShapeBuilder;
-import org.locationtech.spatial4j.shape.Point;
-import org.locationtech.spatial4j.shape.Shape;
-import org.locationtech.spatial4j.shape.jts.JtsGeometry;
-
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.List;
-
-public class LegacyGeoShapeIndexer implements AbstractGeometryFieldMapper.Indexer<ShapeBuilder<?, ?, ?>, Shape> {
-
-    private LegacyGeoShapeFieldMapper.GeoShapeFieldType fieldType;
-
-    public LegacyGeoShapeIndexer(LegacyGeoShapeFieldMapper.GeoShapeFieldType fieldType) {
-        this.fieldType = fieldType;
-    }
-
-
-    @Override
-    public Shape prepareForIndexing(ShapeBuilder<?, ?, ?> shapeBuilder) {
-        return shapeBuilder.buildS4J();
-    }
-
-    @Override
-    public Class<Shape> processedClass() {
-        return Shape.class;
-    }
-
-    @Override
-    public List<IndexableField>  indexShape(ParseContext context, Shape shape) {
-        if (fieldType.pointsOnly()) {
-            // index configured for pointsOnly
-            if (shape instanceof XShapeCollection && XShapeCollection.class.cast(shape).pointsOnly()) {
-                // MULTIPOINT data: index each point separately
-                @SuppressWarnings("unchecked") List<Shape> shapes = ((XShapeCollection) shape).getShapes();
-                List<IndexableField> fields = new ArrayList<>();
-                for (Shape s : shapes) {
-                    fields.addAll(Arrays.asList(fieldType.defaultPrefixTreeStrategy().createIndexableFields(s)));
-                }
-                return fields;
-            } else if (shape instanceof Point == false) {
-                throw new MapperParsingException("[{" + fieldType.name() + "}] is configured for points only but a "
-                    + ((shape instanceof JtsGeometry) ? ((JtsGeometry)shape).getGeom().getGeometryType() : shape.getClass())
-                    + " was found");
-            }
-        }
-        return Arrays.asList(fieldType.defaultPrefixTreeStrategy().createIndexableFields(shape));
-    }
-}

server/src/test/java/org/elasticsearch/common/geo/GeometryIndexerTests.java

@@ -8,17 +8,6 @@
 
 package org.elasticsearch.common.geo;
 
-import static org.hamcrest.Matchers.containsString;
-import static org.hamcrest.Matchers.instanceOf;
-import static org.hamcrest.Matchers.not;
-
-import java.io.IOException;
-import java.text.ParseException;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.List;
-
 import org.apache.lucene.index.IndexableField;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentFactory;
@@ -39,6 +28,17 @@ import org.elasticsearch.index.mapper.GeoShapeIndexer;
 import org.elasticsearch.test.ESTestCase;
 import org.locationtech.spatial4j.exception.InvalidShapeException;
 
+import java.io.IOException;
+import java.text.ParseException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.instanceOf;
+import static org.hamcrest.Matchers.not;
+
 public class GeometryIndexerTests extends ESTestCase {
 
     GeoShapeIndexer indexer = new GeoShapeIndexer(true, "test");
@@ -293,7 +293,7 @@ public class GeometryIndexerTests extends ESTestCase {
         assertEquals(indexed, processed);
 
         // a rectangle is broken into two triangles
-        List<IndexableField> fields = indexer.indexShape(null, indexed);
+        List<IndexableField> fields = indexer.indexShape(indexed);
         assertEquals(fields.size(), 2);
 
         indexed = new Rectangle(179, -179, 10, -10);
@@ -301,7 +301,7 @@ public class GeometryIndexerTests extends ESTestCase {
         assertEquals(indexed, processed);
 
         // a rectangle crossing the dateline is broken into 4 triangles
-        fields = indexer.indexShape(null, indexed);
+        fields = indexer.indexShape(indexed);
         assertEquals(fields.size(), 4);
     }
 
@@ -311,7 +311,7 @@ public class GeometryIndexerTests extends ESTestCase {
         assertEquals(indexed, processed);
 
         // Rectangle is a line
-        List<IndexableField> fields = indexer.indexShape(null, indexed);
+        List<IndexableField> fields = indexer.indexShape(indexed);
         assertEquals(fields.size(), 1);
 
         indexed = new Rectangle(-179, -178, 10, 10);
@@ -319,7 +319,7 @@ public class GeometryIndexerTests extends ESTestCase {
         assertEquals(indexed, processed);
 
         // Rectangle is a line
-        fields = indexer.indexShape(null, indexed);
+        fields = indexer.indexShape(indexed);
         assertEquals(fields.size(), 1);
 
         indexed = new Rectangle(-179, -179, 10, 10);
@@ -327,7 +327,7 @@ public class GeometryIndexerTests extends ESTestCase {
         assertEquals(indexed, processed);
 
         // Rectangle is a point
-        fields = indexer.indexShape(null, indexed);
+        fields = indexer.indexShape(indexed);
         assertEquals(fields.size(), 1);
 
         indexed = new Rectangle(180, -179, 10, -10);
@@ -335,7 +335,7 @@ public class GeometryIndexerTests extends ESTestCase {
         assertEquals(indexed, processed);
 
         // Rectangle crossing the dateline, one side is a line
-        fields = indexer.indexShape(null, indexed);
+        fields = indexer.indexShape(indexed);
         assertEquals(fields.size(), 3);
 
         indexed = new Rectangle(180, -179, 10, 10);
@@ -344,7 +344,7 @@ public class GeometryIndexerTests extends ESTestCase {
 
         // Rectangle crossing the dateline, one side is a point,
         // other side a line
-        fields = indexer.indexShape(null, indexed);
+        fields = indexer.indexShape(indexed);
         assertEquals(fields.size(), 2);
 
         indexed = new Rectangle(-178, -180, 10, -10);
@@ -352,7 +352,7 @@ public class GeometryIndexerTests extends ESTestCase {
         assertEquals(indexed, processed);
 
         // Rectangle crossing the dateline, one side is a line
-        fields = indexer.indexShape(null, indexed);
+        fields = indexer.indexShape(indexed);
         assertEquals(fields.size(), 3);
 
         indexed = new Rectangle(-178, -180, 10, 10);
@@ -361,7 +361,7 @@ public class GeometryIndexerTests extends ESTestCase {
 
         // Rectangle crossing the dateline, one side is a point,
         // other side a line
-        fields = indexer.indexShape(null, indexed);
+        fields = indexer.indexShape(indexed);
         assertEquals(fields.size(), 2);
 
         indexed = new Rectangle(0.0, 1.0819389717881644E-299, 1.401298464324817E-45, 0.0);
@@ -369,7 +369,7 @@ public class GeometryIndexerTests extends ESTestCase {
         assertEquals(indexed, processed);
 
         // Rectangle is a point
-        fields = indexer.indexShape(null, processed);
+        fields = indexer.indexShape(processed);
         assertEquals(fields.size(), 1);
 
         indexed = new Rectangle(-1.4017117476654298E-170, 0.0, 0.0, -2.415012082648633E-174);
@@ -377,7 +377,7 @@ public class GeometryIndexerTests extends ESTestCase {
         assertEquals(indexed, processed);
 
         // Rectangle is a triangle but needs to be computed quantize
-        fields = indexer.indexShape(null, processed);
+        fields = indexer.indexShape(processed);
         assertEquals(fields.size(), 2);
     }
 

server/src/test/java/org/elasticsearch/index/mapper/DocumentParserTests.java

@@ -1068,14 +1068,14 @@ public class DocumentParserTests extends MapperServiceTestCase {
         ParsedDocument doc = mapper.parse(source("1", b -> b.field(field, "41.12,-71.34"), null, Map.of(field, "points")));
         IndexableField[] fields = doc.rootDoc().getFields(field);
         assertThat(fields, arrayWithSize(2));
-        assertThat(fields[0].fieldType(), sameInstance(LatLonDocValuesField.TYPE));
-        assertThat(fields[1].fieldType(), sameInstance(LatLonPoint.TYPE));
+        assertThat(fields[0].fieldType(), sameInstance(LatLonPoint.TYPE));
+        assertThat(fields[1].fieldType(), sameInstance(LatLonDocValuesField.TYPE));
 
         doc = mapper.parse(source("1", b -> b.field(field, new double[]{-71.34, 41.12}), null, Map.of(field, "points")));
         fields = doc.rootDoc().getFields(field);
         assertThat(fields, arrayWithSize(2));
-        assertThat(fields[0].fieldType(), sameInstance(LatLonDocValuesField.TYPE));
-        assertThat(fields[1].fieldType(), sameInstance(LatLonPoint.TYPE));
+        assertThat(fields[0].fieldType(), sameInstance(LatLonPoint.TYPE));
+        assertThat(fields[1].fieldType(), sameInstance(LatLonDocValuesField.TYPE));
 
         doc = mapper.parse(source("1", b -> {
             b.startObject(field);
@@ -1085,16 +1085,16 @@ public class DocumentParserTests extends MapperServiceTestCase {
         }, null, Map.of(field, "points")));
         fields = doc.rootDoc().getFields(field);
         assertThat(fields, arrayWithSize(2));
-        assertThat(fields[0].fieldType(), sameInstance(LatLonDocValuesField.TYPE));
-        assertThat(fields[1].fieldType(), sameInstance(LatLonPoint.TYPE));
+        assertThat(fields[0].fieldType(), sameInstance(LatLonPoint.TYPE));
+        assertThat(fields[1].fieldType(), sameInstance(LatLonDocValuesField.TYPE));
 
         doc = mapper.parse(source("1", b -> b.field(field, new String[]{"41.12,-71.34", "43,-72.34"}), null, Map.of(field, "points")));
         fields = doc.rootDoc().getFields(field);
         assertThat(fields, arrayWithSize(4));
-        assertThat(fields[0].fieldType(), sameInstance(LatLonDocValuesField.TYPE));
+        assertThat(fields[0].fieldType(), sameInstance(LatLonPoint.TYPE));
         assertThat(fields[1].fieldType(), sameInstance(LatLonDocValuesField.TYPE));
         assertThat(fields[2].fieldType(), sameInstance(LatLonPoint.TYPE));
-        assertThat(fields[3].fieldType(), sameInstance(LatLonPoint.TYPE));
+        assertThat(fields[3].fieldType(), sameInstance(LatLonDocValuesField.TYPE));
 
         doc = mapper.parse(source("1", b -> {
             b.startArray(field);
@@ -1111,10 +1111,10 @@ public class DocumentParserTests extends MapperServiceTestCase {
         }, null, Map.of(field, "points")));
         fields = doc.rootDoc().getFields(field);
         assertThat(fields, arrayWithSize(4));
-        assertThat(fields[0].fieldType(), sameInstance(LatLonDocValuesField.TYPE));
+        assertThat(fields[0].fieldType(), sameInstance(LatLonPoint.TYPE));
         assertThat(fields[1].fieldType(), sameInstance(LatLonDocValuesField.TYPE));
         assertThat(fields[2].fieldType(), sameInstance(LatLonPoint.TYPE));
-        assertThat(fields[3].fieldType(), sameInstance(LatLonPoint.TYPE));
+        assertThat(fields[3].fieldType(), sameInstance(LatLonDocValuesField.TYPE));
 
         doc = mapper.parse(source("1", b -> {
             b.startObject("address");
@@ -1123,8 +1123,8 @@ public class DocumentParserTests extends MapperServiceTestCase {
         }, null, Map.of("address.home", "points")));
         fields = doc.rootDoc().getFields("address.home");
         assertThat(fields, arrayWithSize(2));
-        assertThat(fields[0].fieldType(), sameInstance(LatLonDocValuesField.TYPE));
-        assertThat(fields[1].fieldType(), sameInstance(LatLonPoint.TYPE));
+        assertThat(fields[0].fieldType(), sameInstance(LatLonPoint.TYPE));
+        assertThat(fields[1].fieldType(), sameInstance(LatLonDocValuesField.TYPE));
     }
 
     public void testDynamicTemplatesNotFound() throws Exception {

server/src/test/java/org/elasticsearch/index/mapper/ExternalFieldMapperTests.java

@@ -9,13 +9,11 @@
 package org.elasticsearch.index.mapper;
 
 import org.apache.lucene.index.IndexableField;
-import org.elasticsearch.common.geo.GeoPoint;
 import org.elasticsearch.plugins.Plugin;
 
 import java.util.Collection;
 import java.util.Collections;
 
-import static org.hamcrest.Matchers.closeTo;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.notNullValue;
 
@@ -35,13 +33,6 @@ public class ExternalFieldMapperTests extends MapperServiceTestCase {
         assertThat(doc.rootDoc().getField("field.bool"), notNullValue());
         assertThat(doc.rootDoc().getField("field.bool").stringValue(), is("T"));
 
-        assertThat(doc.rootDoc().getField("field.point"), notNullValue());
-        GeoPoint point = new GeoPoint().resetFromIndexableField(doc.rootDoc().getField("field.point"));
-        assertThat(point.lat(), closeTo(42.0, 1e-5));
-        assertThat(point.lon(), closeTo(51.0, 1e-5));
-
-        assertThat(doc.rootDoc().getField("field.shape"), notNullValue());
-
         assertThat(doc.rootDoc().getField("field.field"), notNullValue());
         assertThat(doc.rootDoc().getField("field.field").stringValue(), is("foo"));
 
@@ -70,14 +61,6 @@ public class ExternalFieldMapperTests extends MapperServiceTestCase {
         assertThat(doc.rootDoc().getField("field.bool"), notNullValue());
         assertThat(doc.rootDoc().getField("field.bool").stringValue(), is("T"));
 
-        assertThat(doc.rootDoc().getField("field.point"), notNullValue());
-        GeoPoint point = new GeoPoint().resetFromIndexableField(doc.rootDoc().getField("field.point"));
-        assertThat(point.lat(), closeTo(42.0, 1E-5));
-        assertThat(point.lon(), closeTo(51.0, 1E-5));
-
-        IndexableField shape = doc.rootDoc().getField("field.shape");
-        assertThat(shape, notNullValue());
-
         IndexableField field = doc.rootDoc().getField("field.text");
         assertThat(field, notNullValue());
         assertThat(field.stringValue(), is("foo"));

server/src/test/java/org/elasticsearch/index/mapper/ExternalMapper.java

@@ -10,9 +10,6 @@ package org.elasticsearch.index.mapper;
 
 import org.apache.lucene.analysis.standard.StandardAnalyzer;
 import org.elasticsearch.common.collect.Iterators;
-import org.elasticsearch.common.geo.GeoPoint;
-import org.elasticsearch.common.geo.builders.PointBuilder;
-import org.elasticsearch.geometry.Point;
 import org.elasticsearch.index.analysis.AnalyzerScope;
 import org.elasticsearch.index.analysis.IndexAnalyzers;
 import org.elasticsearch.index.analysis.NamedAnalyzer;
@@ -21,7 +18,6 @@ import org.elasticsearch.script.ScriptCompiler;
 
 import java.io.IOException;
 import java.nio.charset.Charset;
-import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.Iterator;
@@ -54,8 +50,6 @@ public class ExternalMapper extends FieldMapper {
 
         private final BinaryFieldMapper.Builder binBuilder = new BinaryFieldMapper.Builder(Names.FIELD_BIN);
         private final BooleanFieldMapper.Builder boolBuilder = new BooleanFieldMapper.Builder(Names.FIELD_BOOL, ScriptCompiler.NONE);
-        private final GeoPointFieldMapper.Builder latLonPointBuilder = new GeoPointFieldMapper.Builder(Names.FIELD_POINT, false);
-        private final GeoShapeFieldMapper.Builder shapeBuilder = new GeoShapeFieldMapper.Builder(Names.FIELD_SHAPE, false, true);
         private final Mapper.Builder stringBuilder;
         private final String generatedValue;
         private final String mapperName;
@@ -77,13 +71,11 @@ public class ExternalMapper extends FieldMapper {
             contentPath.add(name);
             BinaryFieldMapper binMapper = binBuilder.build(contentPath);
             BooleanFieldMapper boolMapper = boolBuilder.build(contentPath);
-            GeoPointFieldMapper pointMapper = (GeoPointFieldMapper) latLonPointBuilder.build(contentPath);
-            AbstractShapeGeometryFieldMapper<?, ?> shapeMapper = shapeBuilder.build(contentPath);
             FieldMapper stringMapper = (FieldMapper)stringBuilder.build(contentPath);
             contentPath.remove();
 
             return new ExternalMapper(name, buildFullName(contentPath), generatedValue, mapperName, binMapper, boolMapper,
-                pointMapper, shapeMapper, stringMapper, multiFieldsBuilder.build(this, contentPath), copyTo.build());
+                stringMapper, multiFieldsBuilder.build(this, contentPath), copyTo.build());
         }
     }
 
@@ -113,22 +105,18 @@ public class ExternalMapper extends FieldMapper {
 
     private final BinaryFieldMapper binMapper;
     private final BooleanFieldMapper boolMapper;
-    private final GeoPointFieldMapper pointMapper;
-    private final AbstractShapeGeometryFieldMapper<?, ?> shapeMapper;
     private final FieldMapper stringMapper;
 
     public ExternalMapper(String simpleName, String contextName,
                           String generatedValue, String mapperName,
-                          BinaryFieldMapper binMapper, BooleanFieldMapper boolMapper, GeoPointFieldMapper pointMapper,
-                          AbstractShapeGeometryFieldMapper<?, ?> shapeMapper, FieldMapper stringMapper,
+                          BinaryFieldMapper binMapper, BooleanFieldMapper boolMapper,
+                          FieldMapper stringMapper,
                           MultiFields multiFields, CopyTo copyTo) {
         super(simpleName, new ExternalFieldType(contextName, true, true, false), multiFields, copyTo);
         this.generatedValue = generatedValue;
         this.mapperName = mapperName;
         this.binMapper = binMapper;
         this.boolMapper = boolMapper;
-        this.pointMapper = pointMapper;
-        this.shapeMapper = shapeMapper;
         this.stringMapper = stringMapper;
     }
 
@@ -139,21 +127,6 @@ public class ExternalMapper extends FieldMapper {
 
         boolMapper.parse(context.createExternalValueContext(true));
 
-        // Let's add a Dummy Point
-        double lat = 42.0;
-        double lng = 51.0;
-        ArrayList<GeoPoint> points = new ArrayList<>();
-        points.add(new GeoPoint(lat, lng));
-        pointMapper.parse(context.createExternalValueContext(points));
-
-        // Let's add a Dummy Shape
-        if (shapeMapper instanceof GeoShapeFieldMapper) {
-            shapeMapper.parse(context.createExternalValueContext(new Point(-100, 45)));
-        } else {
-            PointBuilder pb = new PointBuilder(-100, 45);
-            shapeMapper.parse(context.createExternalValueContext(pb.buildS4J()));
-        }
-
         context = context.createExternalValueContext(generatedValue);
 
         // Let's add a Original String
@@ -169,7 +142,7 @@ public class ExternalMapper extends FieldMapper {
 
     @Override
     public Iterator<Mapper> iterator() {
-        return Iterators.concat(super.iterator(), Arrays.asList(binMapper, boolMapper, pointMapper, shapeMapper, stringMapper).iterator());
+        return Iterators.concat(super.iterator(), Arrays.asList(binMapper, boolMapper, stringMapper).iterator());
     }
 
     @Override

server/src/test/java/org/elasticsearch/index/mapper/GeoPointFieldMapperTests.java

@@ -220,6 +220,21 @@ public class GeoPointFieldMapperTests extends MapperTestCase {
         assertThat(doc.getField("field.latlon").stringValue(), equalTo("s093jd0k72s1"));
     }
 
+    public void testMultiFieldWithMultipleValues() throws Exception {
+        DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> {
+            b.field("type", "geo_point").field("doc_values", false);
+            b.startObject("fields");
+            {
+                b.startObject("geohash").field("type", "keyword").field("doc_values", false).endObject();
+            }
+            b.endObject();
+        }));
+        ParseContext.Document doc = mapper.parse(source(b -> b.array("field", "POINT (2 3)", "POINT (4 5)"))).rootDoc();
+        assertThat(doc.getFields("field.geohash"), arrayWithSize(2));
+        assertThat(doc.getFields("field.geohash")[0].binaryValue().utf8ToString(), equalTo("s093jd0k72s1"));
+        assertThat(doc.getFields("field.geohash")[1].binaryValue().utf8ToString(), equalTo("s0fu7n0xng81"));
+    }
+
     public void testNullValue() throws Exception {
         DocumentMapper mapper = createDocumentMapper(
             fieldMapping(b -> b.field("type", "geo_point"))
@@ -298,7 +313,7 @@ public class GeoPointFieldMapperTests extends MapperTestCase {
             MapperParsingException.class,
             () -> mapper.parse(source(b -> b.field("field", "1234.333")))
         );
-        assertThat(e.getMessage(), containsString("failed to parse field [field] of type [geo_point]"));
+        assertThat(e.getMessage(), containsString("Failed to parse field [field] of type [geo_point]"));
         assertThat(e.getRootCause().getMessage(), containsString("unsupported symbol [.] in geohash [1234.333]"));
     }
 

server/src/test/java/org/elasticsearch/index/mapper/GeoPointFieldTypeTests.java

@@ -38,11 +38,9 @@ public class GeoPointFieldTypeTests extends FieldTypeTestCase {
         assertEquals(List.of(wktPoint, otherWktPoint), fetchSourceValue(mapper, sourceValue, "wkt"));
 
         // Test a list of points in [lat,lon] array format with one malformed
-        // TODO Point Field parsers have a weird `ignore_malformed` impl that still throws errors
-        // on many types of malformed input
-        // sourceValue = List.of(List.of(42,0, 27.1), List.of("a", "b"), List.of(30.0, 50.0));
-        // assertEquals(List.of(jsonPoint, otherJsonPoint), fetchSourceValue(mapper, sourceValue, null));
-        // assertEquals(List.of(wktPoint, otherWktPoint), fetchSourceValue(mapper, sourceValue, "wkt"));
+        sourceValue = List.of(List.of(42.0, 27.1), List.of("a", "b"), List.of(30.0, 50.0));
+        assertEquals(List.of(jsonPoint, otherJsonPoint), fetchSourceValue(mapper, sourceValue, null));
+        assertEquals(List.of(wktPoint, otherWktPoint), fetchSourceValue(mapper, sourceValue, "wkt"));
 
         // Test a single point in well-known text format.
         sourceValue = "POINT (42.0 27.1)";

server/src/test/java/org/elasticsearch/search/geo/GeoShapeQueryTests.java

@@ -528,7 +528,7 @@ public class GeoShapeQueryTests extends GeoQueryTests {
                     .setRefreshPolicy(IMMEDIATE).get();
         } catch (MapperParsingException e) {
             // RandomShapeGenerator created something other than a POINT type, verify the correct exception is thrown
-            assertThat(e.getCause().getMessage(), containsString("is configured for points only"));
+            assertThat(e.getMessage(), containsString("is configured for points only"));
             return;
         }
 

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

@@ -7,10 +7,10 @@
 package org.elasticsearch.xpack.spatial;
 
 import org.apache.lucene.util.SloppyMath;
+import org.elasticsearch.index.mapper.GeoShapeIndexer;
 import org.elasticsearch.geometry.Circle;
 import org.elasticsearch.geometry.LinearRing;
 import org.elasticsearch.geometry.Polygon;
-import org.elasticsearch.index.mapper.GeoShapeIndexer;
 
 /**
  * Utility class for storing different helpful re-usable spatial functions

x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/GeoShapeValues.java

@@ -7,11 +7,11 @@
 
 package org.elasticsearch.xpack.spatial.index.fielddata;
 
+import org.elasticsearch.index.mapper.GeoShapeIndexer;
 import org.elasticsearch.geometry.Geometry;
 import org.elasticsearch.geometry.Rectangle;
 import org.elasticsearch.geometry.utils.GeographyValidator;
 import org.elasticsearch.geometry.utils.WellKnownText;
-import org.elasticsearch.index.mapper.GeoShapeIndexer;
 import org.elasticsearch.search.aggregations.support.ValuesSourceType;
 import org.elasticsearch.xpack.spatial.index.mapper.BinaryGeoShapeDocValuesField;
 import org.elasticsearch.xpack.spatial.search.aggregations.support.GeoShapeValuesSourceType;
@@ -135,7 +135,7 @@ public abstract class GeoShapeValues {
                 final GeoShapeIndexer indexer = new GeoShapeIndexer(true, "missing");
                 final Geometry geometry = indexer.prepareForIndexing(MISSING_GEOMETRY_PARSER.fromWKT(missing));
                 final BinaryGeoShapeDocValuesField field = new BinaryGeoShapeDocValuesField("missing");
-                field.add(indexer.indexShape(null, geometry), geometry);
+                field.add(indexer.indexShape(geometry), geometry);
                 final GeometryDocValueReader reader = new GeometryDocValueReader();
                 reader.reset(field.binaryValue());
                 return new GeoShapeValue(reader);

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

@@ -37,6 +37,7 @@ import org.elasticsearch.search.lookup.SearchLookup;
 import org.elasticsearch.xpack.spatial.index.fielddata.plain.AbstractLatLonShapeIndexFieldData;
 import org.elasticsearch.xpack.spatial.search.aggregations.support.GeoShapeValuesSourceType;
 
+import java.io.IOException;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
@@ -64,7 +65,7 @@ import java.util.function.Supplier;
  * <p>
  * "field" : "POLYGON ((100.0 0.0, 101.0 0.0, 101.0 1.0, 100.0 1.0, 100.0 0.0))
  */
-public class GeoShapeWithDocValuesFieldMapper extends AbstractShapeGeometryFieldMapper<Geometry, Geometry> {
+public class GeoShapeWithDocValuesFieldMapper extends AbstractShapeGeometryFieldMapper<Geometry> {
     public static final String CONTENT_TYPE = "geo_shape";
 
     private static Builder builder(FieldMapper in) {
@@ -120,17 +121,6 @@ public class GeoShapeWithDocValuesFieldMapper extends AbstractShapeGeometryField
 
     }
 
-    @Override
-    @SuppressWarnings({"rawtypes", "unchecked"})
-    protected void addDocValuesFields(String name, Geometry shape, List<IndexableField> fields, ParseContext context) {
-        BinaryGeoShapeDocValuesField docValuesField = (BinaryGeoShapeDocValuesField) context.doc().getByKey(name);
-        if (docValuesField == null) {
-            docValuesField = new BinaryGeoShapeDocValuesField(name);
-            context.doc().addWithKey(name, docValuesField);
-        }
-        docValuesField.add(fields, shape);
-    }
-
     public static final class GeoShapeWithDocValuesFieldType extends AbstractShapeGeometryFieldType implements GeoShapeQueryable {
 
         public GeoShapeWithDocValuesFieldType(String name, boolean indexed, boolean hasDocValues,
@@ -191,14 +181,35 @@ public class GeoShapeWithDocValuesFieldMapper extends AbstractShapeGeometryField
     };
 
     private final Builder builder;
+    private final GeoShapeIndexer indexer;
 
     public GeoShapeWithDocValuesFieldMapper(String simpleName, MappedFieldType mappedFieldType,
                                             MultiFields multiFields, CopyTo copyTo,
                                             GeoShapeIndexer indexer, GeoShapeParser parser, Builder builder) {
         super(simpleName, mappedFieldType, builder.ignoreMalformed.get(), builder.coerce.get(),
             builder.ignoreZValue.get(), builder.orientation.get(),
-            multiFields, copyTo, indexer, parser);
+            multiFields, copyTo, parser);
         this.builder = builder;
+        this.indexer = indexer;
+    }
+
+    @Override
+    protected void index(ParseContext context, Geometry geometry) throws IOException {
+        List<IndexableField> fields = indexer.indexShape(geometry);
+        if (fieldType().isSearchable()) {
+            context.doc().addAll(fields);
+        }
+        if (fieldType().hasDocValues()) {
+            String name = fieldType().name();
+            BinaryGeoShapeDocValuesField docValuesField = (BinaryGeoShapeDocValuesField) context.doc().getByKey(name);
+            if (docValuesField == null) {
+                docValuesField = new BinaryGeoShapeDocValuesField(name);
+                context.doc().addWithKey(name, docValuesField);
+            }
+            docValuesField.add(fields, geometry);
+        } else if (fieldType().isSearchable()) {
+            createFieldNamesField(context);
+        }
     }
 
     @Override
@@ -221,13 +232,4 @@ public class GeoShapeWithDocValuesFieldMapper extends AbstractShapeGeometryField
         return (GeoShapeWithDocValuesFieldType) super.fieldType();
     }
 
-    @Override
-    protected void addStoredFields(ParseContext context, Geometry geometry) {
-        // noop (stored fields not available for geo_shape fields)
-    }
-
-    @Override
-    protected void addMultiFields(ParseContext context, Geometry geometry) {
-        // noop (completion suggester currently not compatible with geo_shape)
-    }
 }

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

@@ -9,7 +9,6 @@ package org.elasticsearch.xpack.spatial.index.mapper;
 import org.apache.lucene.document.StoredField;
 import org.apache.lucene.document.XYDocValuesField;
 import org.apache.lucene.document.XYPointField;
-import org.apache.lucene.index.IndexableField;
 import org.apache.lucene.search.Query;
 import org.elasticsearch.common.Explicit;
 import org.elasticsearch.common.geo.ShapeRelation;
@@ -25,9 +24,8 @@ import org.elasticsearch.xpack.spatial.common.CartesianPoint;
 import org.elasticsearch.xpack.spatial.index.mapper.PointFieldMapper.ParsedCartesianPoint;
 import org.elasticsearch.xpack.spatial.index.query.ShapeQueryPointProcessor;
 
-import java.util.ArrayList;
+import java.io.IOException;
 import java.util.Arrays;
-import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 
@@ -36,7 +34,7 @@ import java.util.Map;
  *
  * Uses lucene 8 XYPoint encoding
  */
-public class PointFieldMapper extends AbstractPointGeometryFieldMapper<List<ParsedCartesianPoint>, List<? extends CartesianPoint>> {
+public class PointFieldMapper extends AbstractPointGeometryFieldMapper<ParsedCartesianPoint> {
     public static final String CONTENT_TYPE = "point";
 
     public static class CartesianPointParser extends PointParser<ParsedCartesianPoint> {
@@ -99,7 +97,7 @@ public class PointFieldMapper extends AbstractPointGeometryFieldMapper<List<Pars
             PointFieldType ft
                 = new PointFieldType(buildFullName(contentPath), indexed.get(), stored.get(), hasDocValues.get(), parser, meta.get());
             return new PointFieldMapper(name, ft, multiFieldsBuilder.build(this, contentPath),
-                copyTo.build(), new PointIndexer(ft), parser, this);
+                copyTo.build(), parser, this);
         }
 
     }
@@ -110,30 +108,25 @@ public class PointFieldMapper extends AbstractPointGeometryFieldMapper<List<Pars
 
     public PointFieldMapper(String simpleName, MappedFieldType mappedFieldType,
                             MultiFields multiFields, CopyTo copyTo,
-                            PointIndexer pointIndexer, CartesianPointParser parser, Builder builder) {
+                            CartesianPointParser parser, Builder builder) {
         super(simpleName, mappedFieldType, multiFields,
-            builder.ignoreMalformed.get(), builder.ignoreZValue.get(), builder.nullValue.get(), copyTo, pointIndexer, parser);
+            builder.ignoreMalformed.get(), builder.ignoreZValue.get(), builder.nullValue.get(), copyTo, parser);
         this.builder = builder;
     }
 
     @Override
-    protected void addStoredFields(ParseContext context, List<? extends CartesianPoint> points) {
-        for (CartesianPoint point : points) {
-            context.doc().add(new StoredField(fieldType().name(), point.toString()));
+    protected void index(ParseContext context, ParsedCartesianPoint point) throws IOException {
+        if (fieldType().isSearchable()) {
+            context.doc().add(new XYPointField(fieldType().name(), (float) point.getX(), (float) point.getY()));
         }
-    }
-
-    @Override
-    protected void addDocValuesFields(String name, List<? extends CartesianPoint> points, List<IndexableField> fields,
-                                      ParseContext context) {
-        for (CartesianPoint point : points) {
+        if (fieldType().hasDocValues()) {
             context.doc().add(new XYDocValuesField(fieldType().name(), (float) point.getX(), (float) point.getY()));
+        } else if (fieldType().isStored() || fieldType().isSearchable()) {
+            createFieldNamesField(context);
+        }
+        if (fieldType().isStored()) {
+            context.doc().add(new StoredField(fieldType().name(), point.toString()));
         }
-    }
-
-    @Override
-    protected void addMultiFields(ParseContext context, List<? extends CartesianPoint> points) {
-        // noop
     }
 
     @Override
@@ -228,34 +221,4 @@ public class PointFieldMapper extends AbstractPointGeometryFieldMapper<List<Pars
         }
     }
 
-    protected static class PointIndexer implements Indexer<List<ParsedCartesianPoint>, List<? extends CartesianPoint>> {
-        protected final PointFieldType fieldType;
-
-        PointIndexer(PointFieldType fieldType) {
-            this.fieldType = fieldType;
-        }
-
-        @Override
-        public List<? extends CartesianPoint> prepareForIndexing(List<ParsedCartesianPoint> points) {
-            if (points == null || points.isEmpty()) {
-                return Collections.emptyList();
-            }
-            return points;
-        }
-
-        @Override
-        @SuppressWarnings("unchecked")
-        public Class<List<? extends CartesianPoint>> processedClass() {
-            return (Class<List<? extends CartesianPoint>>)(Object)List.class;
-        }
-
-        @Override
-        public List<IndexableField> indexShape(ParseContext context, List<? extends CartesianPoint> points) {
-            ArrayList<IndexableField> fields = new ArrayList<>(1);
-            for (CartesianPoint point : points) {
-                fields.add(new XYPointField(fieldType.name(), (float) point.getX(), (float) point.getY()));
-            }
-            return fields;
-        }
-    }
 }

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

@@ -7,7 +7,6 @@
 package org.elasticsearch.xpack.spatial.index.mapper;
 
 import org.apache.lucene.document.XYShape;
-import org.apache.lucene.index.IndexableField;
 import org.apache.lucene.search.Query;
 import org.elasticsearch.common.Explicit;
 import org.elasticsearch.common.geo.GeometryParser;
@@ -23,6 +22,7 @@ import org.elasticsearch.index.mapper.ParseContext;
 import org.elasticsearch.index.query.SearchExecutionContext;
 import org.elasticsearch.xpack.spatial.index.query.ShapeQueryProcessor;
 
+import java.io.IOException;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
@@ -43,7 +43,7 @@ import java.util.Map;
  * <p>
  * "field" : "POLYGON ((1050.0 -1000.0, 1051.0 -1000.0, 1051.0 -1001.0, 1050.0 -1001.0, 1050.0 -1000.0))
  */
-public class ShapeFieldMapper extends AbstractShapeGeometryFieldMapper<Geometry, Geometry> {
+public class ShapeFieldMapper extends AbstractShapeGeometryFieldMapper<Geometry> {
     public static final String CONTENT_TYPE = "shape";
 
     private static Builder builder(FieldMapper in) {
@@ -80,8 +80,7 @@ public class ShapeFieldMapper extends AbstractShapeGeometryFieldMapper<Geometry,
             ShapeFieldType ft
                 = new ShapeFieldType(buildFullName(contentPath), indexed.get(), orientation.get().value(), parser, meta.get());
             return new ShapeFieldMapper(name, ft,
-                multiFieldsBuilder.build(this, contentPath), copyTo.build(),
-                new ShapeIndexer(ft.name()), parser, this);
+                multiFieldsBuilder.build(this, contentPath), copyTo.build(), parser, this);
         }
     }
 
@@ -112,30 +111,22 @@ public class ShapeFieldMapper extends AbstractShapeGeometryFieldMapper<Geometry,
     }
 
     private final Builder builder;
+    private final ShapeIndexer indexer;
 
     public ShapeFieldMapper(String simpleName, MappedFieldType mappedFieldType,
                             MultiFields multiFields, CopyTo copyTo,
-                            Indexer<Geometry, Geometry> indexer, Parser<Geometry> parser, Builder builder) {
+                            Parser<Geometry> parser, Builder builder) {
         super(simpleName, mappedFieldType, builder.ignoreMalformed.get(),
             builder.coerce.get(), builder.ignoreZValue.get(), builder.orientation.get(),
-            multiFields, copyTo, indexer, parser);
+            multiFields, copyTo, parser);
         this.builder = builder;
+        this.indexer = new ShapeIndexer(mappedFieldType.name());
     }
 
     @Override
-    protected void addStoredFields(ParseContext context, Geometry geometry) {
-        // noop: we currently do not store geo_shapes
-        // @todo store as geojson string?
-    }
-
-    @Override
-    protected void addDocValuesFields(String name, Geometry geometry, List<IndexableField> fields, ParseContext context) {
-        // we should throw a mapping exception before we get here
-    }
-
-    @Override
-    protected void addMultiFields(ParseContext context, Geometry geometry) {
-        // noop (completion suggester currently not compatible with geo_shape)
+    protected void index(ParseContext context, Geometry geometry) throws IOException {
+        context.doc().addAll(indexer.indexShape(geometry));
+        createFieldNamesField(context);
     }
 
     @Override

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

@@ -20,33 +20,24 @@ import org.elasticsearch.geometry.MultiPolygon;
 import org.elasticsearch.geometry.Point;
 import org.elasticsearch.geometry.Polygon;
 import org.elasticsearch.geometry.Rectangle;
-import org.elasticsearch.index.mapper.AbstractShapeGeometryFieldMapper;
-import org.elasticsearch.index.mapper.ParseContext;
 import org.elasticsearch.xpack.spatial.common.ShapeUtils;
 
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 
-public class ShapeIndexer implements AbstractShapeGeometryFieldMapper.Indexer<Geometry, Geometry> {
+public class ShapeIndexer  {
     private final String name;
 
     public ShapeIndexer(String name) {
         this.name = name;
     }
 
-    @Override
-    public Geometry prepareForIndexing(Geometry geometry) {
-        return geometry;
-    }
-
-    @Override
-    public Class<Geometry> processedClass() {
-        return Geometry.class;
-    }
-
-    @Override
-    public List<IndexableField> indexShape(ParseContext context, Geometry shape) {
+    public List<IndexableField> indexShape(Geometry shape) {
+        if (shape == null) {
+            return Collections.emptyList();
+        }
         LuceneGeometryVisitor visitor = new LuceneGeometryVisitor(name);
         shape.visit(visitor);
         return visitor.fields;

x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/fielddata/TriangleTreeTests.java

@@ -26,7 +26,7 @@ public class TriangleTreeTests extends ESTestCase {
         Geometry geometry = GeometryTestUtils.randomGeometryWithoutCircle(randomIntBetween(1, 10), false);
         // write tree
         GeoShapeIndexer indexer = new GeoShapeIndexer(true, "test");
-        List<IndexableField> fieldList = indexer.indexShape(null, indexer.prepareForIndexing(geometry));
+        List<IndexableField> fieldList = indexer.indexShape(geometry);
         ByteBuffersDataOutput output = new ByteBuffersDataOutput();
         TriangleTreeWriter.writeTo(output, fieldList);
         // read tree

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

@@ -145,7 +145,7 @@ public abstract class CartesianFieldMapperTests  extends MapperTestCase {
         MapperParsingException e = expectThrows(MapperParsingException.class,
             () -> mapper2.parse(source(b -> b.field(FIELD_NAME, "POINT (2000.1 305.6 34567.33)")))
         );
-        assertThat(e.getMessage(), containsString("failed to parse field [" + FIELD_NAME + "] of type"));
+        assertThat(e.getMessage(), containsString("Failed to parse field [" + FIELD_NAME + "] of type"));
         assertThat(e.getRootCause().getMessage(),
             containsString("found Z value [34567.33] but [ignore_z_value] parameter is [false]"));
     }

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

@@ -73,8 +73,7 @@ public class LatLonShapeDocValuesQueryTests extends ESTestCase {
                 GeometryTestUtils::randomPolygon
             );
             Geometry geometry = geometryFunc.apply(false);
-            geometry = indexer.prepareForIndexing(geometry);
-            List<IndexableField> fields = indexer.indexShape(null, geometry);
+            List<IndexableField> fields = indexer.indexShape(geometry);
             for (IndexableField field : fields) {
                 doc.add(field);
             }
@@ -116,8 +115,7 @@ public class LatLonShapeDocValuesQueryTests extends ESTestCase {
         for (int id = 0; id < numDocs; id++) {
             Document doc = new Document();
             Geometry geometry = GeometryTestUtils.randomGeometryWithoutCircle(randomIntBetween(1, 5), false);
-            geometry = indexer.prepareForIndexing(geometry);
-            List<IndexableField> fields = indexer.indexShape(null, geometry);
+            List<IndexableField> fields = indexer.indexShape(geometry);
             for (IndexableField field : fields) {
                 doc.add(field);
             }

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

@@ -51,11 +51,9 @@ public class PointFieldTypeTests extends FieldTypeTestCase {
         assertEquals(List.of(wktPoint), fetchSourceValue(mapper, sourceValue, "wkt"));
 
         // Test a list of points in [x, y] array format with a malformed entry
-        // TODO Point Field parsers have a weird `ignore_malformed` impl that still throws errors
-        // on many types of malformed input
-        // sourceValue = List.of(List.of(42.0, 27.1), List.of("a", "b"), List.of(30.0, 50.0));
-        // assertEquals(List.of(jsonPoint, otherJsonPoint), fetchSourceValue(mapper, sourceValue, null));
-        // assertEquals(List.of(wktPoint, otherWktPoint), fetchSourceValue(mapper, sourceValue, "wkt"));
+        sourceValue = List.of(List.of(42.0, 27.1), List.of("a", "b"), List.of(30.0, 50.0));
+        assertEquals(List.of(jsonPoint, otherJsonPoint), fetchSourceValue(mapper, sourceValue, null));
+        assertEquals(List.of(wktPoint, otherWktPoint), fetchSourceValue(mapper, sourceValue, "wkt"));
 
     }
 }

x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/ingest/CircleProcessorTests.java

@@ -226,8 +226,7 @@ public class CircleProcessorTests extends ESTestCase {
         try (Directory dir = newDirectory(); RandomIndexWriter w = new RandomIndexWriter(random(), dir)) {
             Document doc = new Document();
             GeoShapeIndexer indexer = new GeoShapeIndexer(true, fieldName);
-            Geometry normalized = indexer.prepareForIndexing(geometry);
-            for (IndexableField field : indexer.indexShape(null, normalized)) {
+            for (IndexableField field : indexer.indexShape(geometry)) {
                 doc.add(field);
             }
             w.addDocument(doc);
@@ -258,8 +257,7 @@ public class CircleProcessorTests extends ESTestCase {
         try (Directory dir = newDirectory(); RandomIndexWriter w = new RandomIndexWriter(random(), dir)) {
             Document doc = new Document();
             ShapeIndexer indexer = new ShapeIndexer(fieldName);
-            Geometry normalized = indexer.prepareForIndexing(geometry);
-            for (IndexableField field : indexer.indexShape(null, normalized)) {
+            for (IndexableField field : indexer.indexShape(geometry)) {
                 doc.add(field);
             }
             w.addDocument(doc);

x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/util/GeoTestUtils.java

@@ -43,7 +43,7 @@ public class GeoTestUtils {
         CentroidCalculator centroidCalculator = new CentroidCalculator();
         centroidCalculator.add(geometry);
         GeometryDocValueReader reader = new GeometryDocValueReader();
-        reader.reset(GeometryDocValueWriter.write(indexer.indexShape(null, geometry), encoder, centroidCalculator));
+        reader.reset(GeometryDocValueWriter.write(indexer.indexShape(geometry), encoder, centroidCalculator));
         return reader;
     }
 
@@ -51,7 +51,7 @@ public class GeoTestUtils {
         GeoShapeIndexer indexer = new GeoShapeIndexer(true, name);
         geometry = indexer.prepareForIndexing(geometry);
         BinaryGeoShapeDocValuesField field = new BinaryGeoShapeDocValuesField(name);
-        field.add(indexer.indexShape(null, geometry) , geometry);
+        field.add(indexer.indexShape(geometry) , geometry);
         return field;
     }