From 094f193ee2a22a9f957418229a320f108a46acef Mon Sep 17 00:00:00 2001 From: Ignacio Vera <iverase@gmail.com> Date: Fri, 4 Jun 2021 17:24:02 +0200 Subject: [PATCH] Remove ParsedPoint interface from point field mappers (#73710) --- .../mapper/AbstractGeometryFieldMapper.java | 18 +-- .../AbstractPointGeometryFieldMapper.java | 105 ++++++----------- .../AbstractShapeGeometryFieldMapper.java | 4 +- .../index/mapper/GeoPointFieldMapper.java | 98 ++++++++-------- .../index/mapper/GeoShapeFieldMapper.java | 2 +- .../mapper/LegacyGeoShapeFieldMapper.java | 2 +- .../query/LegacyGeoShapeQueryProcessor.java | 8 +- .../mapper/GeoPointFieldMapperTests.java | 4 +- .../GeoShapeWithDocValuesFieldMapper.java | 2 +- .../index/mapper/PointFieldMapper.java | 109 ++++++++---------- .../index/mapper/ShapeFieldMapper.java | 2 +- 11 files changed, 150 insertions(+), 204 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java index 160e29f002ced..2d651aa9923e5 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java @@ -70,13 +70,13 @@ private void fetchFromSource(Object sourceMap, Consumer<Object> consumer, String } } - public abstract static class AbstractGeometryFieldType extends MappedFieldType { + public abstract static class AbstractGeometryFieldType<T> extends MappedFieldType { - protected final Parser<?> geometryParser; + protected final Parser<T> geometryParser; protected final boolean parsesArrayValue; protected AbstractGeometryFieldType(String name, boolean indexed, boolean stored, boolean hasDocValues, - boolean parsesArrayValue, Parser<?> geometryParser, Map<String, String> meta) { + boolean parsesArrayValue, Parser<T> geometryParser, Map<String, String> meta) { super(name, indexed, stored, hasDocValues, TextSearchInfo.NONE, meta); this.parsesArrayValue = parsesArrayValue; this.geometryParser = geometryParser; @@ -116,13 +116,13 @@ protected Object parseSourceValue(Object value) { private final Explicit<Boolean> ignoreMalformed; private final Explicit<Boolean> ignoreZValue; - private final Parser<? extends T> 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, - Parser<? extends T> parser) { + Parser<T> parser) { super(simpleName, mappedFieldType, indexAnalyzers, multiFields, copyTo, false, null); this.ignoreMalformed = ignoreMalformed; this.ignoreZValue = ignoreZValue; @@ -132,7 +132,7 @@ protected AbstractGeometryFieldMapper(String simpleName, MappedFieldType mappedF protected AbstractGeometryFieldMapper(String simpleName, MappedFieldType mappedFieldType, Explicit<Boolean> ignoreMalformed, Explicit<Boolean> ignoreZValue, MultiFields multiFields, CopyTo copyTo, - Parser<? extends T> parser) { + Parser<T> parser) { this(simpleName, mappedFieldType, Collections.emptyMap(), ignoreMalformed, ignoreZValue, multiFields, copyTo, parser); } @@ -141,7 +141,7 @@ protected AbstractGeometryFieldMapper( MappedFieldType mappedFieldType, MultiFields multiFields, CopyTo copyTo, - Parser<? extends T> parser, + Parser<T> parser, String onScriptError ) { super(simpleName, mappedFieldType, Collections.emptyMap(), multiFields, copyTo, true, onScriptError); @@ -151,8 +151,8 @@ protected AbstractGeometryFieldMapper( } @Override - public AbstractGeometryFieldType fieldType() { - return (AbstractGeometryFieldType) mappedFieldType; + public AbstractGeometryFieldType<T> fieldType() { + return (AbstractGeometryFieldType<T>) mappedFieldType; } @Override diff --git a/server/src/main/java/org/elasticsearch/index/mapper/AbstractPointGeometryFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/AbstractPointGeometryFieldMapper.java index 69332e7cd937e..71e643f9d691d 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/AbstractPointGeometryFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/AbstractPointGeometryFieldMapper.java @@ -12,12 +12,7 @@ import org.elasticsearch.common.CheckedConsumer; import org.elasticsearch.common.Explicit; import org.elasticsearch.common.TriFunction; -import org.elasticsearch.common.geo.GeoPoint; -import org.elasticsearch.common.geo.GeometryFormat; -import org.elasticsearch.common.geo.GeometryParser; import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.geometry.Geometry; -import org.elasticsearch.geometry.Point; import org.elasticsearch.index.mapper.Mapper.TypeParser.ParserContext; import java.io.IOException; @@ -28,25 +23,25 @@ /** Base class for for spatial fields that only support indexing points */ public abstract class AbstractPointGeometryFieldMapper<T> extends AbstractGeometryFieldMapper<T> { - public static Parameter<ParsedPoint> nullValueParam(Function<FieldMapper, ParsedPoint> initializer, - TriFunction<String, ParserContext, Object, ParsedPoint> parser, - Supplier<ParsedPoint> def) { - return new Parameter<>("null_value", false, def, parser, initializer); + public static <T> Parameter<T> nullValueParam(Function<FieldMapper, T> initializer, + TriFunction<String, ParserContext, Object, T> parser, + Supplier<T> def) { + return new Parameter<T>("null_value", false, def, parser, initializer); } - protected final ParsedPoint nullValue; + protected final T nullValue; protected AbstractPointGeometryFieldMapper(String simpleName, MappedFieldType mappedFieldType, MultiFields multiFields, Explicit<Boolean> ignoreMalformed, - Explicit<Boolean> ignoreZValue, ParsedPoint nullValue, CopyTo copyTo, - Parser<? extends T> parser) { + Explicit<Boolean> ignoreZValue, T nullValue, CopyTo copyTo, + Parser<T> parser) { super(simpleName, mappedFieldType, ignoreMalformed, ignoreZValue, multiFields, copyTo, parser); this.nullValue = nullValue; } protected AbstractPointGeometryFieldMapper(String simpleName, MappedFieldType mappedFieldType, MultiFields multiFields, CopyTo copyTo, - Parser<? extends T> parser, String onScriptError) { + Parser<T> parser, String onScriptError) { super(simpleName, mappedFieldType, multiFields, copyTo, parser, onScriptError); this.nullValue = null; } @@ -56,80 +51,62 @@ public final boolean parsesArrayValue() { return true; } - public ParsedPoint getNullValue() { + public T getNullValue() { return nullValue; } - /** represents a Point that has been parsed by {@link PointParser} */ - public interface ParsedPoint { - void validate(String fieldName); - void normalize(String fieldName); - void resetCoords(double x, double y); - Point asGeometry(); - default boolean isNormalizable(double coord) { - return Double.isNaN(coord) == false && Double.isInfinite(coord) == false; - } - } - - /** A parser implementation that can parse the various point formats */ - public static class PointParser<P extends ParsedPoint> extends Parser<P> { - /** - * Note that this parser is only used for formatting values. - */ - private final GeometryParser geometryParser; - private final String field; - private final Supplier<P> pointSupplier; - private final CheckedBiFunction<XContentParser, P, P, IOException> objectParser; - private final P nullValue; + /** A base parser implementation for point formats */ + protected abstract static class PointParser<T> extends Parser<T> { + protected final String field; + private final Supplier<T> pointSupplier; + private final CheckedBiFunction<XContentParser, T, T, IOException> objectParser; + private final T nullValue; private final boolean ignoreZValue; - private final boolean ignoreMalformed; - - public PointParser(String field, - Supplier<P> pointSupplier, - CheckedBiFunction<XContentParser, P, P, IOException> objectParser, - P nullValue, - boolean ignoreZValue, - boolean ignoreMalformed) { + protected final boolean ignoreMalformed; + + protected PointParser(String field, + Supplier<T> pointSupplier, + CheckedBiFunction<XContentParser, T, T, IOException> objectParser, + T nullValue, + boolean ignoreZValue, + boolean ignoreMalformed) { this.field = field; this.pointSupplier = pointSupplier; this.objectParser = objectParser; - this.nullValue = nullValue == null ? null : process(nullValue); + this.nullValue = nullValue == null ? null : validate(nullValue); this.ignoreZValue = ignoreZValue; this.ignoreMalformed = ignoreMalformed; - this.geometryParser = new GeometryParser(true, true, true); } - private P process(P in) { - if (ignoreMalformed == false) { - in.validate(field); - } else { - in.normalize(field); - } - return in; - } + protected abstract T validate(T in); + + protected abstract void reset(T in, double x, double y); @Override public void parse( XContentParser parser, - CheckedConsumer<P, IOException> consumer, + CheckedConsumer<T, IOException> consumer, Consumer<Exception> onMalformed ) throws IOException { if (parser.currentToken() == XContentParser.Token.START_ARRAY) { XContentParser.Token token = parser.nextToken(); - P point = pointSupplier.get(); + T point = pointSupplier.get(); if (token == XContentParser.Token.VALUE_NUMBER) { double x = parser.doubleValue(); parser.nextToken(); double y = parser.doubleValue(); token = parser.nextToken(); if (token == XContentParser.Token.VALUE_NUMBER) { - GeoPoint.assertZValue(ignoreZValue, parser.doubleValue()); + if (ignoreZValue == false) { + throw new ElasticsearchParseException("Exception parsing coordinates: found Z value [{}] but [ignore_z_value] " + + "parameter is [{}]", parser.doubleValue(), ignoreZValue); + } } else if (token != XContentParser.Token.END_ARRAY) { throw new ElasticsearchParseException("field type does not accept > 3 dimensions"); } - point.resetCoords(x, y); - consumer.accept(process(point)); + reset(point, x, y); + consumer.accept(validate(point)); } else { while (token != XContentParser.Token.END_ARRAY) { parseAndConsumeFromObject(parser, point, consumer, onMalformed); @@ -148,22 +125,16 @@ public void parse( private void parseAndConsumeFromObject( XContentParser parser, - P point, - CheckedConsumer<P, IOException> consumer, + T point, + CheckedConsumer<T, IOException> consumer, Consumer<Exception> onMalformed ) { try { point = objectParser.apply(parser, point); - consumer.accept(process(point)); + consumer.accept(validate(point)); } catch (Exception e) { onMalformed.accept(e); } } - - @Override - public Object format(P point, String format) { - GeometryFormat<Geometry> geometryFormat = geometryParser.geometryFormat(format); - return geometryFormat.toXContentAsObject(point.asGeometry()); - } } } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/AbstractShapeGeometryFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/AbstractShapeGeometryFieldMapper.java index 57dfb6189ccf3..6a8ca9630cbc8 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/AbstractShapeGeometryFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/AbstractShapeGeometryFieldMapper.java @@ -34,12 +34,12 @@ public static Parameter<Explicit<Orientation>> orientationParam(Function<FieldMa .setSerializer((b, f, v) -> b.field(f, v.value()), v -> v.value().toString()); } - public abstract static class AbstractShapeGeometryFieldType extends AbstractGeometryFieldType { + public abstract static class AbstractShapeGeometryFieldType<T> extends AbstractGeometryFieldType<T> { private final Orientation orientation; protected AbstractShapeGeometryFieldType(String name, boolean isSearchable, boolean isStored, boolean hasDocValues, - boolean parsesArrayValue, Parser<?> parser, + boolean parsesArrayValue, Parser<T> parser, Orientation orientation, Map<String, String> meta) { super(name, isSearchable, isStored, hasDocValues, parsesArrayValue, parser, meta); this.orientation = orientation; diff --git a/server/src/main/java/org/elasticsearch/index/mapper/GeoPointFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/GeoPointFieldMapper.java index 5fd3fe4eebe4c..3bbec171f2da5 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/GeoPointFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/GeoPointFieldMapper.java @@ -16,6 +16,7 @@ import org.apache.lucene.search.MatchNoDocsQuery; import org.apache.lucene.search.Query; import org.elasticsearch.ElasticsearchParseException; +import org.elasticsearch.common.CheckedBiFunction; import org.elasticsearch.common.Explicit; import org.elasticsearch.common.geo.GeoJsonGeometryFormat; import org.elasticsearch.common.geo.GeoPoint; @@ -25,6 +26,7 @@ import org.elasticsearch.common.geo.GeometryParser; import org.elasticsearch.common.geo.ShapeRelation; import org.elasticsearch.common.unit.DistanceUnit; +import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.support.MapXContentParser; import org.elasticsearch.geometry.Geometry; import org.elasticsearch.geometry.Point; @@ -64,7 +66,7 @@ public static class Builder extends FieldMapper.Builder { final Parameter<Explicit<Boolean>> ignoreMalformed; final Parameter<Explicit<Boolean>> ignoreZValue = ignoreZValueParam(m -> builder(m).ignoreZValue.get()); - final Parameter<ParsedPoint> nullValue; + final Parameter<GeoPoint> nullValue; final Parameter<Boolean> indexed = Parameter.indexParam(m -> builder(m).indexed.get(), true); final Parameter<Boolean> hasDocValues = Parameter.docValuesParam(m -> builder(m).hasDocValues.get(), true); final Parameter<Boolean> stored = Parameter.storeParam(m -> builder(m).stored.get(), false); @@ -96,11 +98,11 @@ public Builder docValues(boolean hasDocValues) { return this; } - private static ParsedGeoPoint parseNullValue(Object nullValue, boolean ignoreZValue, boolean ignoreMalformed) { + private static GeoPoint parseNullValue(Object nullValue, boolean ignoreZValue, boolean ignoreMalformed) { if (nullValue == null) { return null; } - ParsedGeoPoint point = new ParsedGeoPoint(); + GeoPoint point = new GeoPoint(); GeoUtils.parseGeoPoint(nullValue, point, ignoreZValue); if (ignoreMalformed == false) { if (point.lat() > 90.0 || point.lat() < -90.0) { @@ -128,14 +130,14 @@ private FieldValues<GeoPoint> scriptValues() { @Override public FieldMapper build(ContentPath contentPath) { - Parser<ParsedGeoPoint> geoParser = new PointParser<>( + Parser<GeoPoint> geoParser = new GeoPointParser( name, - ParsedGeoPoint::new, + GeoPoint::new, (parser, point) -> { GeoUtils.parseGeoPoint(parser, point, ignoreZValue.get().value()); return point; }, - (ParsedGeoPoint) nullValue.get(), + nullValue.get(), ignoreZValue.get().value(), ignoreMalformed.get().value()); GeoPointFieldType ft = new GeoPointFieldType( @@ -163,7 +165,7 @@ public FieldMapper build(ContentPath contentPath) { public GeoPointFieldMapper(String simpleName, MappedFieldType mappedFieldType, MultiFields multiFields, CopyTo copyTo, - Parser<ParsedGeoPoint> parser, + Parser<GeoPoint> parser, Builder builder) { super(simpleName, mappedFieldType, multiFields, builder.ignoreMalformed.get(), builder.ignoreZValue.get(), builder.nullValue.get(), @@ -173,7 +175,7 @@ public GeoPointFieldMapper(String simpleName, MappedFieldType mappedFieldType, } public GeoPointFieldMapper(String simpleName, MappedFieldType mappedFieldType, - Parser<ParsedGeoPoint> parser, Builder builder) { + Parser<GeoPoint> parser, Builder builder) { super(simpleName, mappedFieldType, MultiFields.empty(), CopyTo.empty(), parser, builder.onScriptError.get()); this.builder = builder; this.scriptValues = builder.scriptValues(); @@ -217,13 +219,13 @@ protected String contentType() { return CONTENT_TYPE; } - public static class GeoPointFieldType extends AbstractGeometryFieldType implements GeoShapeQueryable { + public static class GeoPointFieldType extends AbstractGeometryFieldType<GeoPoint> implements GeoShapeQueryable { private static final GeometryParser PARSER = new GeometryParser(true, true, true); private final FieldValues<GeoPoint> scriptValues; private GeoPointFieldType(String name, boolean indexed, boolean stored, boolean hasDocValues, - Parser<ParsedGeoPoint> parser, FieldValues<GeoPoint> scriptValues, Map<String, String> meta) { + Parser<GeoPoint> parser, FieldValues<GeoPoint> scriptValues, Map<String, String> meta) { super(name, indexed, stored, hasDocValues, true, parser, meta); this.scriptValues = scriptValues; } @@ -287,62 +289,52 @@ public Query distanceFeatureQuery(Object origin, String pivot, SearchExecutionCo } } - // Eclipse requires the AbstractPointGeometryFieldMapper prefix or it can't find ParsedPoint - // See https://bugs.eclipse.org/bugs/show_bug.cgi?id=565255 - protected static class ParsedGeoPoint extends GeoPoint implements AbstractPointGeometryFieldMapper.ParsedPoint { - @Override - public void validate(String fieldName) { - if (lat() > 90.0 || lat() < -90.0) { - throw new IllegalArgumentException("illegal latitude value [" + lat() + "] for " + fieldName); - } - if (lon() > 180.0 || lon() < -180) { - throw new IllegalArgumentException("illegal longitude value [" + lon() + "] for " + fieldName); - } + /** GeoPoint parser implementation */ + private static class GeoPointParser extends PointParser<GeoPoint> { + // Note that this parser is only used for formatting values. + private final GeometryParser geometryParser; + + GeoPointParser(String field, + Supplier<GeoPoint> pointSupplier, + CheckedBiFunction<XContentParser, GeoPoint, GeoPoint, IOException> objectParser, + GeoPoint nullValue, + boolean ignoreZValue, + boolean ignoreMalformed) { + super(field, pointSupplier, objectParser, nullValue, ignoreZValue, ignoreMalformed); + this.geometryParser = new GeometryParser(true, true, true); } - @Override - public void normalize(String name) { - if (isNormalizable(lat()) && isNormalizable(lon())) { - GeoUtils.normalizePoint(this); + protected GeoPoint validate(GeoPoint in) { + if (ignoreMalformed == false) { + if (in.lat() > 90.0 || in.lat() < -90.0) { + throw new IllegalArgumentException("illegal latitude value [" + in.lat() + "] for " + field); + } + if (in.lon() > 180.0 || in.lon() < -180) { + throw new IllegalArgumentException("illegal longitude value [" + in.lon() + "] for " + field); + } } else { - throw new ElasticsearchParseException("cannot normalize the point - not a number"); + if (isNormalizable(in.lat()) && isNormalizable(in.lon())) { + GeoUtils.normalizePoint(in); + } else { + throw new ElasticsearchParseException("cannot normalize the point - not a number"); + } } + return in; } - @Override - public boolean isNormalizable(double coord) { + private boolean isNormalizable(double coord) { return Double.isNaN(coord) == false && Double.isInfinite(coord) == false; } @Override - public void resetCoords(double x, double y) { - this.reset(y, x); - } - - public Point asGeometry() { - return new Point(lon(), lat()); - } - - @Override - public boolean equals(Object other) { - double oLat; - double oLon; - if (other instanceof GeoPoint) { - GeoPoint o = (GeoPoint)other; - oLat = o.lat(); - oLon = o.lon(); - } else { - return false; - } - if (Double.compare(oLat, lat) != 0) return false; - if (Double.compare(oLon, lon) != 0) return false; - - return true; + protected void reset(GeoPoint in, double x, double y) { + in.reset(y, x); } @Override - public int hashCode() { - return super.hashCode(); + public Object format(GeoPoint point, String format) { + GeometryFormat<Geometry> geometryFormat = geometryParser.geometryFormat(format); + return geometryFormat.toXContentAsObject(new Point(point.lon(), point.lat())); } } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java index 9817ce0497f8f..f196905d285b0 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java @@ -112,7 +112,7 @@ public GeoShapeFieldMapper build(ContentPath contentPath) { } } - public static class GeoShapeFieldType extends AbstractShapeGeometryFieldType implements GeoShapeQueryable { + public static class GeoShapeFieldType extends AbstractShapeGeometryFieldType<Geometry> implements GeoShapeQueryable { public GeoShapeFieldType(String name, boolean indexed, Orientation orientation, Parser<Geometry> parser, Map<String, String> meta) { diff --git a/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapper.java index 0ff9b009f1ec0..f15cad16fe8c1 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapper.java @@ -340,7 +340,7 @@ public Object format(ShapeBuilder<?, ?, ?> value, String format) { } } - public static final class GeoShapeFieldType extends AbstractShapeGeometryFieldType implements GeoShapeQueryable { + public static final class GeoShapeFieldType extends AbstractShapeGeometryFieldType<ShapeBuilder<?, ?, ?>> implements GeoShapeQueryable { private String tree = Defaults.TREE; private SpatialStrategy strategy = Defaults.STRATEGY; diff --git a/server/src/main/java/org/elasticsearch/index/query/LegacyGeoShapeQueryProcessor.java b/server/src/main/java/org/elasticsearch/index/query/LegacyGeoShapeQueryProcessor.java index b0e16ea83da24..e01f4a7d7838d 100644 --- a/server/src/main/java/org/elasticsearch/index/query/LegacyGeoShapeQueryProcessor.java +++ b/server/src/main/java/org/elasticsearch/index/query/LegacyGeoShapeQueryProcessor.java @@ -41,7 +41,6 @@ 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.LegacyGeoShapeFieldMapper; import org.locationtech.jts.geom.Coordinate; import org.locationtech.spatial4j.shape.Shape; @@ -53,10 +52,10 @@ public class LegacyGeoShapeQueryProcessor { - private AbstractShapeGeometryFieldMapper.AbstractShapeGeometryFieldType ft; + private final LegacyGeoShapeFieldMapper.GeoShapeFieldType shapeFieldType; - public LegacyGeoShapeQueryProcessor(AbstractShapeGeometryFieldMapper.AbstractShapeGeometryFieldType ft) { - this.ft = ft; + public LegacyGeoShapeQueryProcessor(LegacyGeoShapeFieldMapper.GeoShapeFieldType shapeFieldType) { + this.shapeFieldType = shapeFieldType; } public Query geoShapeQuery(Geometry shape, String fieldName, SpatialStrategy strategy, @@ -66,7 +65,6 @@ public Query geoShapeQuery(Geometry shape, String fieldName, SpatialStrategy str + ALLOW_EXPENSIVE_QUERIES.getKey() + "' is set to false."); } - LegacyGeoShapeFieldMapper.GeoShapeFieldType shapeFieldType = (LegacyGeoShapeFieldMapper.GeoShapeFieldType) ft; SpatialStrategy spatialStrategy = shapeFieldType.strategy(); if (strategy != null) { spatialStrategy = strategy; diff --git a/server/src/test/java/org/elasticsearch/index/mapper/GeoPointFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/GeoPointFieldMapperTests.java index a574b57a9e89f..c4aa06686c795 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/GeoPointFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/GeoPointFieldMapperTests.java @@ -262,7 +262,7 @@ public void testNullValue() throws Exception { fieldMapper = mapper.mappers().getMapper("field"); assertThat(fieldMapper, instanceOf(GeoPointFieldMapper.class)); - AbstractPointGeometryFieldMapper.ParsedPoint nullValue = ((GeoPointFieldMapper) fieldMapper).nullValue; + GeoPoint nullValue = ((GeoPointFieldMapper) fieldMapper).nullValue; assertThat(nullValue, equalTo(new GeoPoint(1, 2))); doc = mapper.parse(source(b -> b.nullField("field"))); @@ -294,7 +294,7 @@ public void testNullValueWithIgnoreMalformed() throws Exception { Mapper fieldMapper = mapper.mappers().getMapper("field"); assertThat(fieldMapper, instanceOf(GeoPointFieldMapper.class)); - AbstractPointGeometryFieldMapper.ParsedPoint nullValue = ((GeoPointFieldMapper) fieldMapper).nullValue; + GeoPoint nullValue = ((GeoPointFieldMapper) fieldMapper).nullValue; // geo_point [91, 181] should have been normalized to [89, 1] assertThat(nullValue, equalTo(new GeoPoint(89, 1))); } 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 index 0a670cb54437e..69270ad0f5231 100644 --- 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 @@ -134,7 +134,7 @@ public GeoShapeWithDocValuesFieldMapper build(ContentPath contentPath) { } - public static final class GeoShapeWithDocValuesFieldType extends AbstractShapeGeometryFieldType implements GeoShapeQueryable { + public static final class GeoShapeWithDocValuesFieldType extends AbstractShapeGeometryFieldType<Geometry> implements GeoShapeQueryable { public GeoShapeWithDocValuesFieldType(String name, boolean indexed, boolean hasDocValues, Orientation orientation, GeoShapeParser parser, Map<String, String> meta) { 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 index c2850b2ff8e04..9e2923f8a87a4 100644 --- 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 @@ -10,10 +10,14 @@ import org.apache.lucene.document.XYDocValuesField; import org.apache.lucene.document.XYPointField; import org.apache.lucene.search.Query; +import org.elasticsearch.common.CheckedBiFunction; import org.elasticsearch.common.Explicit; +import org.elasticsearch.common.geo.GeometryFormat; +import org.elasticsearch.common.geo.GeometryParser; import org.elasticsearch.common.geo.ShapeRelation; import org.elasticsearch.common.logging.DeprecationCategory; import org.elasticsearch.common.logging.DeprecationLogger; +import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.geometry.Geometry; import org.elasticsearch.geometry.Point; import org.elasticsearch.index.mapper.AbstractPointGeometryFieldMapper; @@ -24,34 +28,24 @@ import org.elasticsearch.index.mapper.ParseContext; import org.elasticsearch.index.query.SearchExecutionContext; 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.io.IOException; import java.util.Arrays; import java.util.List; import java.util.Map; +import java.util.function.Supplier; /** * Field Mapper for point type. * * Uses lucene 8 XYPoint encoding */ -public class PointFieldMapper extends AbstractPointGeometryFieldMapper<ParsedCartesianPoint> { +public class PointFieldMapper extends AbstractPointGeometryFieldMapper<CartesianPoint> { public static final String CONTENT_TYPE = "point"; private static final DeprecationLogger DEPRECATION_LOGGER = DeprecationLogger.getLogger(GeoShapeFieldMapper.class); - public static class CartesianPointParser extends PointParser<ParsedCartesianPoint> { - - public CartesianPointParser(String name, ParsedPoint nullValue, boolean ignoreZValue, boolean ignoreMalformed) { - super(name, ParsedCartesianPoint::new, (parser, point) -> { - ParsedCartesianPoint.parsePoint(parser, point, ignoreZValue); - return point; - }, (ParsedCartesianPoint) nullValue, ignoreZValue, ignoreMalformed); - } - } - private static Builder builder(FieldMapper in) { return ((PointFieldMapper)in).builder; } @@ -64,7 +58,7 @@ public static class Builder extends FieldMapper.Builder { final Parameter<Explicit<Boolean>> ignoreMalformed; final Parameter<Explicit<Boolean>> ignoreZValue = ignoreZValueParam(m -> builder(m).ignoreZValue.get()); - final Parameter<ParsedPoint> nullValue; + final Parameter<CartesianPoint> nullValue; final Parameter<Map<String, String>> meta = Parameter.metaParam(); public Builder(String name, boolean ignoreMalformedByDefault) { @@ -81,8 +75,8 @@ protected List<Parameter<?>> getParameters() { return Arrays.asList(indexed, hasDocValues, stored, ignoreMalformed, ignoreZValue, nullValue, meta); } - private static ParsedPoint parseNullValue(Object nullValue, boolean ignoreZValue, boolean ignoreMalformed) { - ParsedCartesianPoint point = new ParsedCartesianPoint(); + private static CartesianPoint parseNullValue(Object nullValue, boolean ignoreZValue, boolean ignoreMalformed) { + CartesianPoint point = new CartesianPoint(); CartesianPoint.parsePoint(nullValue, point, ignoreZValue); if (ignoreMalformed == false) { if (Double.isFinite(point.getX()) == false) { @@ -105,7 +99,15 @@ public FieldMapper build(ContentPath contentPath) { ); } CartesianPointParser parser - = new CartesianPointParser(name, nullValue.get(), ignoreZValue.get().value(), ignoreMalformed.get().value()); + = new CartesianPointParser( + name, + CartesianPoint::new, + (p, point) -> { + CartesianPoint.parsePoint(p, point, ignoreZValue.get().value()); + return point; + }, + nullValue.get(), + ignoreZValue.get().value(), ignoreMalformed.get().value()); PointFieldType ft = new PointFieldType(buildFullName(contentPath), indexed.get(), stored.get(), hasDocValues.get(), parser, meta.get()); return new PointFieldMapper(name, ft, multiFieldsBuilder.build(this, contentPath), @@ -127,7 +129,7 @@ public PointFieldMapper(String simpleName, MappedFieldType mappedFieldType, } @Override - protected void index(ParseContext context, ParsedCartesianPoint point) throws IOException { + protected void index(ParseContext context, CartesianPoint point) throws IOException { if (fieldType().isSearchable()) { context.doc().add(new XYPointField(fieldType().name(), (float) point.getX(), (float) point.getY())); } @@ -156,7 +158,7 @@ public FieldMapper.Builder getMergeBuilder() { return new Builder(simpleName(), builder.ignoreMalformed.getDefaultValue().value()).init(this); } - public static class PointFieldType extends AbstractGeometryFieldType implements ShapeQueryable { + public static class PointFieldType extends AbstractGeometryFieldType<CartesianPoint> implements ShapeQueryable { private final ShapeQueryPointProcessor queryProcessor; @@ -177,60 +179,43 @@ public Query shapeQuery(Geometry shape, String fieldName, ShapeRelation relation } } - // Eclipse requires the AbstractPointGeometryFieldMapper prefix or it can't find ParsedPoint - // See https://bugs.eclipse.org/bugs/show_bug.cgi?id=565255 - protected static class ParsedCartesianPoint extends CartesianPoint implements AbstractPointGeometryFieldMapper.ParsedPoint { - @Override - public void validate(String fieldName) { - if (Double.isFinite(getX()) == false) { - throw new IllegalArgumentException("illegal x value [" + getX() + "] for " + fieldName); - } - if (Double.isFinite(getY()) == false) { - throw new IllegalArgumentException("illegal y value [" + getY() + "] for " + fieldName); - } - } - - @Override - public void normalize(String fieldName) { - // noop - } - - @Override - public boolean isNormalizable(double coord) { - return false; - } + /** CartesianPoint parser implementation */ + private static class CartesianPointParser extends PointParser<CartesianPoint> { + // Note that this parser is only used for formatting values. + private final GeometryParser geometryParser; - @Override - public void resetCoords(double x, double y) { - this.reset(x, y); + CartesianPointParser(String field, + Supplier<CartesianPoint> pointSupplier, + CheckedBiFunction<XContentParser, CartesianPoint, CartesianPoint, IOException> objectParser, + CartesianPoint nullValue, + boolean ignoreZValue, + boolean ignoreMalformed) { + super(field, pointSupplier, objectParser, nullValue, ignoreZValue, ignoreMalformed); + this.geometryParser = new GeometryParser(true, true, true); } @Override - public Point asGeometry() { - return new Point(getX(), getY()); + protected CartesianPoint validate(CartesianPoint in) { + if (ignoreMalformed == false) { + if (Double.isFinite(in.getX()) == false) { + throw new IllegalArgumentException("illegal x value [" + in.getX() + "] for " + field); + } + if (Double.isFinite(in.getY()) == false) { + throw new IllegalArgumentException("illegal y value [" + in.getY() + "] for " + field); + } + } + return in; } @Override - public boolean equals(Object other) { - double oX; - double oY; - if (other instanceof CartesianPoint) { - CartesianPoint o = (CartesianPoint)other; - oX = o.getX(); - oY = o.getY(); - } else { - return false; - } - if (Double.compare(oX, x) != 0) return false; - if (Double.compare(oY, y) != 0) return false; - - return true; + protected void reset(CartesianPoint in, double x, double y) { + in.reset(x, y); } @Override - public int hashCode() { - return super.hashCode(); + public Object format(CartesianPoint value, String format) { + GeometryFormat<Geometry> geometryFormat = geometryParser.geometryFormat(format); + return geometryFormat.toXContentAsObject(new Point(value.getX(), value.getY())); } } - } 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 index 3e6b369842d52..7acca5e65e3e4 100644 --- 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 @@ -100,7 +100,7 @@ public ShapeFieldMapper build(ContentPath contentPath) { IGNORE_MALFORMED_SETTING.get(c.getSettings()), COERCE_SETTING.get(c.getSettings()))); - public static final class ShapeFieldType extends AbstractShapeGeometryFieldType + public static final class ShapeFieldType extends AbstractShapeGeometryFieldType<Geometry> implements ShapeQueryable { private final ShapeQueryProcessor queryProcessor;