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

iverase · web-flow · commit 603be001a91e · 2021-06-09T11:31:33.000+02:00
bring shape and point implementation close together.
diff --git a/server/src/main/java/org/elasticsearch/common/geo/GeoFormatterFactory.java b/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;
     }
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java b/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 @@ private void fetchFromSource(Object sourceMap, Consumer<Object> consumer, Functi
     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 final Query termQuery(Object value, SearchExecutionContext context) {
         }
 
         /**
-         * 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 boolean ignoreMalformed() {
     public boolean ignoreZValue() {
         return ignoreZValue.value();
     }
+
+    @Override
+    public final boolean parsesArrayValue() {
+        return true;
+    }
 }
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/AbstractPointGeometryFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/AbstractPointGeometryFieldMapper.java
@@ -46,11 +46,6 @@ protected AbstractPointGeometryFieldMapper(String simpleName, MappedFieldType ma
         this.nullValue = null;
     }
 
-    @Override
-    public final boolean parsesArrayValue() {
-        return true;
-    }
-
     public T getNullValue() {
         return nullValue;
     }
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/AbstractShapeGeometryFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/AbstractShapeGeometryFieldMapper.java
@@ -38,9 +38,8 @@ public abstract static class AbstractShapeGeometryFieldType<T> extends AbstractG
         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 @@ protected AbstractShapeGeometryFieldMapper(String simpleName, MappedFieldType ma
             ignoreMalformed, coerce, ignoreZValue, orientation, multiFields, copyTo, parser);
     }
 
-    @Override
-    public final boolean parsesArrayValue() {
-        return false;
-    }
-
     public boolean coerce() {
         return coerce.value();
     }
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/GeoPointFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/GeoPointFieldMapper.java
@@ -224,7 +224,7 @@ public static class GeoPointFieldType extends AbstractGeometryFieldType<GeoPoint
 
         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;
         }
 
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java
@@ -118,7 +118,7 @@ public static class GeoShapeFieldType extends AbstractShapeGeometryFieldType<Geo
 
         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
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeParser.java b/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeParser.java
@@ -32,7 +32,13 @@ public void parse(
         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);
         }
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapper.java
@@ -325,7 +325,13 @@ public void parse(
             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 static final class GeoShapeFieldType extends AbstractShapeGeometryFieldTy
 
         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);
         }
 
diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/common/CartesianFormatterFactory.java b/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;
     }
diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapper.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapper.java
@@ -140,7 +140,7 @@ public static final class GeoShapeWithDocValuesFieldType extends AbstractShapeGe
 
         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) {
diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/PointFieldMapper.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/PointFieldMapper.java
@@ -164,7 +164,7 @@ public static class PointFieldType extends AbstractGeometryFieldType<CartesianPo
 
         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();
         }
 
diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/ShapeFieldMapper.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/ShapeFieldMapper.java
@@ -109,7 +109,7 @@ public static final class ShapeFieldType extends AbstractShapeGeometryFieldType<
 
         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();
         }
 

Original file line number	Diff line number	Diff line change
`@@ -35,7 +35,7 @@ public class GeoFormatterFactory {`
`35`	`35`	`public static Function<Geometry, Object> getFormatter(String name) {`
`36`	`36`	`Function<Geometry, Object> format = FORMATTERS.get(name);`
`37`	`37`	`if (format == null) {`
`38`		`- throw new IllegalArgumentException("Unrecognized geometry format [" + format + "].");`
	`38`	`+ throw new IllegalArgumentException("Unrecognized geometry format [" + name + "].");`
`39`	`39`	`}`
`40`	`40`	`return format;`
`41`	`41`	`}`
Original file line number	Diff line number	Diff line change
`@@ -46,11 +46,6 @@ protected AbstractPointGeometryFieldMapper(String simpleName, MappedFieldType ma`
`46`	`46`	`this.nullValue = null;`
`47`	`47`	`}`
`48`	`48`
`49`		`- @Override`
`50`		`- public final boolean parsesArrayValue() {`
`51`		`- return true;`
`52`		`- }`
`53`		`-`
`54`	`49`	`public T getNullValue() {`
`55`	`50`	`return nullValue;`
`56`	`51`	`}`
Original file line number	Diff line number	Diff line change
`@@ -224,7 +224,7 @@ public static class GeoPointFieldType extends AbstractGeometryFieldType<GeoPoint`
`224`	`224`
`225`	`225`	`private GeoPointFieldType(String name, boolean indexed, boolean stored, boolean hasDocValues,`
`226`	`226`	`Parser<GeoPoint> parser, FieldValues<GeoPoint> scriptValues, Map<String, String> meta) {`
`227`		`- super(name, indexed, stored, hasDocValues, true, parser, meta);`
	`227`	`+ super(name, indexed, stored, hasDocValues, parser, meta);`
`228`	`228`	`this.scriptValues = scriptValues;`
`229`	`229`	`}`
`230`	`230`
Original file line number	Diff line number	Diff line change
`@@ -118,7 +118,7 @@ public static class GeoShapeFieldType extends AbstractShapeGeometryFieldType<Geo`
`118`	`118`
`119`	`119`	`public GeoShapeFieldType(String name, boolean indexed, Orientation orientation,`
`120`	`120`	`Parser<Geometry> parser, Map<String, String> meta) {`
`121`		`- super(name, indexed, false, false, false, parser, orientation, meta);`
	`121`	`+ super(name, indexed, false, false, parser, orientation, meta);`
`122`	`122`	`}`
`123`	`123`
`124`	`124`	`@Override`
Original file line number	Diff line number	Diff line change
`@@ -35,7 +35,7 @@ public class CartesianFormatterFactory {`
`35`	`35`	`public static Function<Geometry, Object> getFormatter(String name) {`
`36`	`36`	`Function<Geometry, Object> format = FORMATTERS.get(name);`
`37`	`37`	`if (format == null) {`
`38`		`- throw new IllegalArgumentException("Unrecognized geometry format [" + format + "].");`
	`38`	`+ throw new IllegalArgumentException("Unrecognized geometry format [" + name + "].");`
`39`	`39`	`}`
`40`	`40`	`return format;`
`41`	`41`	`}`
Original file line number	Diff line number	Diff line change
`@@ -140,7 +140,7 @@ public static final class GeoShapeWithDocValuesFieldType extends AbstractShapeGe`
`140`	`140`
`141`	`141`	`public GeoShapeWithDocValuesFieldType(String name, boolean indexed, boolean hasDocValues,`
`142`	`142`	`Orientation orientation, GeoShapeParser parser, Map<String, String> meta) {`
`143`		`- super(name, indexed, false, hasDocValues, false, parser, orientation, meta);`
	`143`	`+ super(name, indexed, false, hasDocValues, parser, orientation, meta);`
`144`	`144`	`}`
`145`	`145`
`146`	`146`	`public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName, Supplier<SearchLookup> searchLookup) {`