Set parsesArrayValue to true for Shape field mappers (#73940)

bring shape and point implementation close together.

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

@@ -35,7 +35,7 @@ public class GeoFormatterFactory {
     public static Function<Geometry, Object> getFormatter(String name) {
         Function<Geometry, Object> format = FORMATTERS.get(name);
         if (format == null) {
-            throw new IllegalArgumentException("Unrecognized geometry format [" + format + "].");
+            throw new IllegalArgumentException("Unrecognized geometry format [" + name + "].");
         }
         return format;
     }

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

@@ -8,12 +8,11 @@
 package org.elasticsearch.index.mapper;
 
 import org.apache.lucene.search.Query;
-import org.apache.lucene.util.SetOnce;
-import org.elasticsearch.core.CheckedConsumer;
 import org.elasticsearch.common.Explicit;
 import org.elasticsearch.common.geo.GeoFormatterFactory;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.common.xcontent.support.MapXContentParser;
+import org.elasticsearch.core.CheckedConsumer;
 import org.elasticsearch.index.analysis.NamedAnalyzer;
 import org.elasticsearch.index.query.SearchExecutionContext;
 
@@ -65,12 +64,10 @@ public abstract class AbstractGeometryFieldMapper<T> extends FieldMapper {
     public abstract static class AbstractGeometryFieldType<T> extends MappedFieldType {
 
         protected final Parser<T> geometryParser;
-        protected final boolean parsesArrayValue;
 
         protected AbstractGeometryFieldType(String name, boolean indexed, boolean stored, boolean hasDocValues,
-                                            boolean parsesArrayValue, Parser<T> geometryParser, Map<String, String> meta) {
+                                            Parser<T> geometryParser, Map<String, String> meta) {
             super(name, indexed, stored, hasDocValues, TextSearchInfo.NONE, meta);
-            this.parsesArrayValue = parsesArrayValue;
             this.geometryParser = geometryParser;
         }
 
@@ -81,32 +78,21 @@ public abstract class AbstractGeometryFieldMapper<T> extends FieldMapper {
         }
 
         /**
-         * Gets the formatter. If the method return null, then the format is unsupported and an error is thrown.
+         * Gets the formatter by name.
          */
         protected abstract Function<T, Object> getFormatter(String format);
 
         @Override
         public ValueFetcher valueFetcher(SearchExecutionContext context, String format) {
             Function<T, Object> formatter = getFormatter(format != null ? format : GeoFormatterFactory.GEOJSON);
-            if (parsesArrayValue) {
-                return new ArraySourceValueFetcher(name(), context) {
-                    @Override
-                    protected Object parseSourceValue(Object value) {
-                        List<Object> values = new ArrayList<>();
-                        geometryParser.fetchFromSource(value, values::add, formatter);
-                        return values;
-                    }
-                };
-            } else {
-                return new SourceValueFetcher(name(), context) {
-                    @Override
-                    protected Object parseSourceValue(Object value) {
-                        SetOnce<Object> holder = new SetOnce<>();
-                        geometryParser.fetchFromSource(value, holder::set, formatter);
-                        return holder.get();
-                    }
-                };
-            }
+            return new ArraySourceValueFetcher(name(), context) {
+                @Override
+                protected Object parseSourceValue(Object value) {
+                    List<Object> values = new ArrayList<>();
+                    geometryParser.fetchFromSource(value, values::add, formatter);
+                    return values;
+                }
+            };
         }
     }
 
@@ -187,4 +173,9 @@ public abstract class AbstractGeometryFieldMapper<T> extends FieldMapper {
     public boolean ignoreZValue() {
         return ignoreZValue.value();
     }
+
+    @Override
+    public final boolean parsesArrayValue() {
+        return true;
+    }
 }

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

@@ -46,11 +46,6 @@ public abstract class AbstractPointGeometryFieldMapper<T> extends AbstractGeomet
         this.nullValue = null;
     }
 
-    @Override
-    public final boolean parsesArrayValue() {
-        return true;
-    }
-
     public T getNullValue() {
         return nullValue;
     }

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

@@ -38,9 +38,8 @@ public abstract class AbstractShapeGeometryFieldMapper<T> extends AbstractGeomet
         private final Orientation orientation;
 
         protected AbstractShapeGeometryFieldType(String name, boolean isSearchable, boolean isStored, boolean hasDocValues,
-                                                 boolean parsesArrayValue, Parser<T> parser,
-                                                 Orientation orientation, Map<String, String> meta) {
-            super(name, isSearchable, isStored, hasDocValues, parsesArrayValue, parser, meta);
+                                                 Parser<T> parser, Orientation orientation, Map<String, String> meta) {
+            super(name, isSearchable, isStored, hasDocValues, parser, meta);
             this.orientation = orientation;
         }
 
@@ -70,11 +69,6 @@ public abstract class AbstractShapeGeometryFieldMapper<T> extends AbstractGeomet
             ignoreMalformed, coerce, ignoreZValue, orientation, multiFields, copyTo, parser);
     }
 
-    @Override
-    public final boolean parsesArrayValue() {
-        return false;
-    }
-
     public boolean coerce() {
         return coerce.value();
     }

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

@@ -224,7 +224,7 @@ public class GeoPointFieldMapper extends AbstractPointGeometryFieldMapper<GeoPoi
 
         private GeoPointFieldType(String name, boolean indexed, boolean stored, boolean hasDocValues,
                                   Parser<GeoPoint> parser, FieldValues<GeoPoint> scriptValues, Map<String, String> meta) {
-            super(name, indexed, stored, hasDocValues, true, parser, meta);
+            super(name, indexed, stored, hasDocValues, parser, meta);
             this.scriptValues = scriptValues;
         }
 

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

@@ -118,7 +118,7 @@ public class GeoShapeFieldMapper extends AbstractShapeGeometryFieldMapper<Geomet
 
         public GeoShapeFieldType(String name, boolean indexed, Orientation orientation,
                                  Parser<Geometry> parser, Map<String, String> meta) {
-            super(name, indexed, false, false, false, parser, orientation, meta);
+            super(name, indexed, false, false, parser, orientation, meta);
         }
 
         @Override

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

@@ -32,7 +32,13 @@ public class GeoShapeParser extends AbstractGeometryFieldMapper.Parser<Geometry>
         Consumer<Exception> onMalformed
     ) throws IOException {
         try {
-            consumer.accept(geometryParser.parse(parser));
+            if (parser.currentToken() == XContentParser.Token.START_ARRAY) {
+                while (parser.nextToken() != XContentParser.Token.END_ARRAY) {
+                    parse(parser, consumer, onMalformed);
+                }
+            } else {
+                consumer.accept(geometryParser.parse(parser));
+            }
         } catch (ParseException | ElasticsearchParseException | IllegalArgumentException e) {
             onMalformed.accept(e);
         }

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

@@ -325,7 +325,13 @@ public class LegacyGeoShapeFieldMapper extends AbstractShapeGeometryFieldMapper<
             Consumer<Exception> onMalformed
         ) throws IOException {
             try {
-                consumer.accept(ShapeParser.parse(parser));
+                if (parser.currentToken() == XContentParser.Token.START_ARRAY) {
+                    while (parser.nextToken() != XContentParser.Token.END_ARRAY) {
+                        parse(parser, consumer, onMalformed);
+                    }
+                } else {
+                    consumer.accept(ShapeParser.parse(parser));
+                }
             } catch (ElasticsearchParseException e) {
                 onMalformed.accept(e);
             }
@@ -351,7 +357,7 @@ public class LegacyGeoShapeFieldMapper extends AbstractShapeGeometryFieldMapper<
 
         private GeoShapeFieldType(String name, boolean indexed, Orientation orientation,
                                   LegacyGeoShapeParser parser, Map<String, String> meta) {
-            super(name, indexed, false, false, false, parser, orientation, meta);
+            super(name, indexed, false, false, parser, orientation, meta);
             this.queryProcessor = new LegacyGeoShapeQueryProcessor(this);
         }
 

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

@@ -35,7 +35,7 @@ public class CartesianFormatterFactory {
     public static Function<Geometry, Object> getFormatter(String name) {
         Function<Geometry, Object> format = FORMATTERS.get(name);
         if (format == null) {
-            throw new IllegalArgumentException("Unrecognized geometry format [" + format + "].");
+            throw new IllegalArgumentException("Unrecognized geometry format [" + name + "].");
         }
         return format;
     }

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

@@ -140,7 +140,7 @@ public class GeoShapeWithDocValuesFieldMapper extends AbstractShapeGeometryField
 
         public GeoShapeWithDocValuesFieldType(String name, boolean indexed, boolean hasDocValues,
                                               Orientation orientation, GeoShapeParser parser, Map<String, String> meta) {
-            super(name, indexed, false, hasDocValues, false, parser, orientation, meta);
+            super(name, indexed, false, hasDocValues, parser, orientation, meta);
         }
 
         public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName, Supplier<SearchLookup> searchLookup) {

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

@@ -164,7 +164,7 @@ public class PointFieldMapper extends AbstractPointGeometryFieldMapper<Cartesian
 
         private PointFieldType(String name, boolean indexed, boolean stored, boolean hasDocValues,
                                CartesianPointParser parser, Map<String, String> meta) {
-            super(name, indexed, stored, hasDocValues, true, parser, meta);
+            super(name, indexed, stored, hasDocValues, parser, meta);
             this.queryProcessor = new ShapeQueryPointProcessor();
         }
 

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

@@ -109,7 +109,7 @@ public class ShapeFieldMapper extends AbstractShapeGeometryFieldMapper<Geometry>
 
         public ShapeFieldType(String name, boolean indexed, Orientation orientation,
                               Parser<Geometry> parser, Map<String, String> meta) {
-            super(name, indexed, false, false, false, parser, orientation, meta);
+            super(name, indexed, false, false, parser, orientation, meta);
             this.queryProcessor = new ShapeQueryProcessor();
         }