From 156d741e3da42e87c64f251d39fa9f18e4d67d3a Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Wed, 15 Jul 2020 11:51:21 -0700 Subject: [PATCH 1/9] Support spatial fields in field retrieval API. Although we accept a variety of formats during indexing, spatial data is returned in a single consistent format. This is GeoJSON by default, but well-known text is also supported by passing the option 'format: wkt'. Note that points (in addition to shapes) are returned in GeoJSON by default. The reasoning is that this gives better consistency, and is the most convenient format for most REST API users. --- docs/reference/mapping/types.asciidoc | 1 + docs/reference/search/search-fields.asciidoc | 10 +++-- .../rest-api-spec/test/geo_shape/10_basic.yml | 23 ++++++++++ .../org/elasticsearch/common/geo/GeoJson.java | 17 +++++++ .../mapper/AbstractGeometryFieldMapper.java | 45 ++++++++++++++++++- .../AbstractPointGeometryFieldMapper.java | 32 +++++++++++++ .../index/mapper/GeoPointFieldMapper.java | 9 +++- .../index/mapper/GeoShapeFieldMapper.java | 1 + .../index/mapper/GeoShapeFormatter.java | 43 ++++++++++++++++++ .../mapper/LegacyGeoShapeFieldMapper.java | 22 +++++++++ .../mapper/GeoPointFieldMapperTests.java | 40 +++++++++++++++++ .../mapper/GeoShapeFieldMapperTests.java | 40 +++++++++++++++++ .../LegacyGeoShapeFieldMapperTests.java | 40 +++++++++++++++++ .../subphase/FieldValueRetrieverTests.java | 29 +++++------- .../xpack/spatial/common/CartesianPoint.java | 9 +--- .../GeoShapeWithDocValuesFieldMapper.java | 2 + .../index/mapper/PointFieldMapper.java | 9 +++- .../index/mapper/ShapeFieldMapper.java | 2 + .../index/mapper/PointFieldMapperTests.java | 42 +++++++++++++++++ .../index/mapper/ShapeFieldMapperTests.java | 41 +++++++++++++++++ 20 files changed, 427 insertions(+), 30 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFormatter.java diff --git a/docs/reference/mapping/types.asciidoc b/docs/reference/mapping/types.asciidoc index 5c1671cb30dd3..ca0bc115c21c6 100644 --- a/docs/reference/mapping/types.asciidoc +++ b/docs/reference/mapping/types.asciidoc @@ -22,6 +22,7 @@ string:: <>, <> and <>:: `nested` for arrays of JSON objects [float] +[[_spatial_datatypes]] === Spatial data types <>:: `geo_point` for lat/lon points diff --git a/docs/reference/search/search-fields.asciidoc b/docs/reference/search/search-fields.asciidoc index 99ae0f53af8c7..66423549c2927 100644 --- a/docs/reference/search/search-fields.asciidoc +++ b/docs/reference/search/search-fields.asciidoc @@ -87,9 +87,13 @@ POST twitter/_search <1> Both full field names and wildcard patterns are accepted. <2> Using object notation, you can pass a `format` parameter to apply a custom - format for the field's values. This is currently supported for - <> and <>, which - accept a <>. + format for the field's values. The date fields + <> and <> accept a + <>. <<_spatial_datatypes, Spatial fields>> + accept either `geojson` for http://www.geojson.org[GeoJSON] (the default) + or `wkt` for + https://en.wikipedia.org/wiki/Well-known_text_representation_of_geometry[Well Known Text]. + Other field types do not support the `format` parameter. The values are returned as a flat list in the `fields` section in each hit: diff --git a/modules/geo/src/yamlRestTest/resources/rest-api-spec/test/geo_shape/10_basic.yml b/modules/geo/src/yamlRestTest/resources/rest-api-spec/test/geo_shape/10_basic.yml index 7ddfa5832d4c5..aaa692ba933b7 100644 --- a/modules/geo/src/yamlRestTest/resources/rest-api-spec/test/geo_shape/10_basic.yml +++ b/modules/geo/src/yamlRestTest/resources/rest-api-spec/test/geo_shape/10_basic.yml @@ -57,3 +57,26 @@ setup: field: location - match: {hits.total: 1} + +--- +"Test retrieve geo_shape field": + - do: + search: + index: test + body: + fields: [location] + _source: false + + - match: { hits.hits.0.fields.location.0.type: "Point" } + - match: { hits.hits.0.fields.location.0.coordinates: [1.0, 1.0] } + + - do: + search: + index: test + body: + fields: + - field: location + format: wkt + _source: false + + - match: { hits.hits.0.fields.location.0: "POINT (1.0 1.0)" } diff --git a/server/src/main/java/org/elasticsearch/common/geo/GeoJson.java b/server/src/main/java/org/elasticsearch/common/geo/GeoJson.java index 06bd67e7a5f3b..fe6811d82f6ed 100644 --- a/server/src/main/java/org/elasticsearch/common/geo/GeoJson.java +++ b/server/src/main/java/org/elasticsearch/common/geo/GeoJson.java @@ -22,15 +22,21 @@ import org.elasticsearch.ElasticsearchException; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.geo.parsers.ShapeParser; +import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.unit.DistanceUnit; import org.elasticsearch.common.xcontent.ConstructingObjectParser; +import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; +import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.ObjectParser; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentSubParser; +import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.geometry.Circle; import org.elasticsearch.geometry.Geometry; import org.elasticsearch.geometry.GeometryCollection; @@ -50,6 +56,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Locale; +import java.util.Map; import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg; import static org.elasticsearch.common.xcontent.ConstructingObjectParser.optionalConstructorArg; @@ -610,4 +617,14 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws } } + public static Map toXContentMap(Geometry geometry) throws IOException { + XContentBuilder builder = XContentFactory.jsonBuilder(); + GeoJson.toXContent(geometry, builder, ToXContent.EMPTY_PARAMS); + StreamInput input = BytesReference.bytes(builder).streamInput(); + + try (XContentParser parser = XContentType.JSON.xContent() + .createParser(NamedXContentRegistry.EMPTY, LoggingDeprecationHandler.INSTANCE, input)) { + return parser.map(); + } + } } 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 7020ae0dcaa06..990e5b33df178 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java @@ -30,16 +30,21 @@ import org.elasticsearch.common.geo.ShapeRelation; import org.elasticsearch.common.geo.SpatialStrategy; import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; +import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.common.xcontent.support.MapXContentParser; import org.elasticsearch.common.xcontent.support.XContentMapValues; import org.elasticsearch.geometry.Geometry; import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.index.query.QueryShardException; import java.io.IOException; +import java.io.UncheckedIOException; import java.text.ParseException; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.Iterator; import java.util.List; @@ -82,6 +87,11 @@ public interface Parser { Parsed parse(XContentParser parser, AbstractGeometryFieldMapper mapper) throws IOException, ParseException; } + public interface Formatter { + Object formatGeoJson(Parsed value); + Object formatWKT(Parsed value); + } + public abstract static class Builder, FT extends AbstractGeometryFieldType> extends FieldMapper.Builder { protected Boolean ignoreMalformed; @@ -143,7 +153,31 @@ public Builder ignoreZValue(final boolean ignoreZValue) { @Override protected Object parseSourceValue(Object value, String format) { - throw new UnsupportedOperationException(); + AbstractGeometryFieldType mappedFieldType = fieldType(); + Parser geometryParser = mappedFieldType.geometryParser(); + Formatter geometryFormatter = mappedFieldType.geometryFormatter(); + + 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 = geometryParser.parse(parser, this); + } catch (IOException e) { + throw new UncheckedIOException(e); + } catch (ParseException e) { + throw new RuntimeException(e); + } + + if (format == null || format.equals("geojson")) { + return geometryFormatter.formatGeoJson(geometry); + } else if (format.equals("wkt")) { + return geometryFormatter.formatWKT(geometry); + } else { + throw new IllegalArgumentException("Encountered an unsupported format [" + format + "] when " + + "loading values for the [" + contentType() +"] field named [" + name() + "]"); + } } public abstract static class TypeParser implements Mapper.TypeParser { @@ -187,6 +221,7 @@ public T parse(String name, Map node, ParserContext parserContex public abstract static class AbstractGeometryFieldType extends MappedFieldType { protected Indexer geometryIndexer; protected Parser geometryParser; + protected Formatter geometryFormatter; protected QueryProcessor geometryQueryBuilder; protected AbstractGeometryFieldType(String name, boolean indexed, boolean hasDocValues, Map meta) { @@ -213,6 +248,14 @@ protected Parser geometryParser() { return geometryParser; } + public void setGeometryFormatter(Formatter geometryFormatter) { + this.geometryFormatter = geometryFormatter; + } + + protected Formatter geometryFormatter() { + return geometryFormatter; + } + public QueryProcessor geometryQueryBuilder() { return geometryQueryBuilder; } 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 f9c27ae2e01c8..31e5dace53742 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/AbstractPointGeometryFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/AbstractPointGeometryFieldMapper.java @@ -22,12 +22,17 @@ import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.common.Explicit; import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.geo.GeoJson; import org.elasticsearch.common.geo.GeoPoint; import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.geometry.Geometry; +import org.elasticsearch.geometry.Point; +import org.elasticsearch.geometry.utils.WellKnownText; import java.io.IOException; +import java.io.UncheckedIOException; import java.text.ParseException; import java.util.ArrayList; import java.util.Iterator; @@ -158,6 +163,7 @@ 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; } @@ -239,4 +245,30 @@ public List

parse(XContentParser parser, AbstractGeometryFieldMapper geometry } } } + + public static class PointFormatter

implements Formatter> { + @Override + public Object formatGeoJson(List

points) { + List result = new ArrayList<>(); + for (ParsedPoint point : points) { + try { + Geometry geometry = point.asGeometry(); + result.add(GeoJson.toXContentMap(geometry)); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + return result; + } + + @Override + public Object formatWKT(List

points) { + List result = new ArrayList<>(); + for (ParsedPoint point : points) { + Geometry geometry = point.asGeometry(); + result.add(WellKnownText.INSTANCE.toWKT(geometry)); + } + return result; + } + } } 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 00e90ddd1081b..49ddf862e3f32 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/GeoPointFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/GeoPointFieldMapper.java @@ -29,10 +29,11 @@ import org.elasticsearch.common.geo.GeoPoint; import org.elasticsearch.common.geo.GeoUtils; import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.geometry.Point; import org.elasticsearch.index.fielddata.IndexFieldData; import org.elasticsearch.index.fielddata.plain.AbstractLatLonPointIndexFieldData; -import org.elasticsearch.index.query.VectorGeoPointShapeQueryProcessor; import org.elasticsearch.index.mapper.GeoPointFieldMapper.ParsedGeoPoint; +import org.elasticsearch.index.query.VectorGeoPointShapeQueryProcessor; import org.elasticsearch.search.aggregations.support.CoreValuesSourceType; import java.io.IOException; @@ -49,6 +50,7 @@ public class GeoPointFieldMapper extends AbstractPointGeometryFieldMapper, List> { public static final String CONTENT_TYPE = "geo_point"; public static final FieldType FIELD_TYPE = new FieldType(); + static { FIELD_TYPE.setStored(false); FIELD_TYPE.setIndexOptions(IndexOptions.DOCS); @@ -70,6 +72,7 @@ public GeoPointFieldMapper build(BuilderContext context, String simpleName, Fiel GeoPointFieldType ft = new GeoPointFieldType(buildFullName(context), indexed, hasDocValues, meta); ft.setGeometryParser(new PointParser<>()); ft.setGeometryIndexer(new GeoPointIndexer(ft)); + ft.setGeometryFormatter(new PointFormatter<>()); ft.setGeometryQueryBuilder(new VectorGeoPointShapeQueryProcessor()); return new GeoPointFieldMapper(name, fieldType, ft, multiFields, ignoreMalformed, ignoreZValue, nullValue, copyTo); } @@ -218,6 +221,10 @@ 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; 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 6ad1de5b868cf..543f64c056fa3 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java @@ -73,6 +73,7 @@ private GeoShapeFieldType buildFieldType(BuilderContext context) { ignoreZValue().value()); ft.setGeometryParser((parser, mapper) -> geometryParser.parse(parser)); ft.setGeometryIndexer(new GeoShapeIndexer(orientation().value().getAsBoolean(), buildFullName(context))); + ft.setGeometryFormatter(new GeoShapeFormatter()); ft.setGeometryQueryBuilder(new VectorGeoShapeQueryProcessor()); ft.setOrientation(orientation == null ? Defaults.ORIENTATION.value() : orientation); return ft; diff --git a/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFormatter.java b/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFormatter.java new file mode 100644 index 0000000000000..2b51ffe90fe03 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFormatter.java @@ -0,0 +1,43 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.index.mapper; + +import org.elasticsearch.common.geo.GeoJson; +import org.elasticsearch.geometry.Geometry; +import org.elasticsearch.geometry.utils.WellKnownText; + +import java.io.IOException; +import java.io.UncheckedIOException; + +public class GeoShapeFormatter implements AbstractGeometryFieldMapper.Formatter { + @Override + public Object formatGeoJson(Geometry value) { + try { + return GeoJson.toXContentMap(value); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + + @Override + public Object formatWKT(Geometry value) { + return WellKnownText.INSTANCE.toWKT(value); + } +} 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 5ee6f12232f27..06ce727526a51 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapper.java @@ -31,6 +31,7 @@ import org.elasticsearch.Version; import org.elasticsearch.common.Explicit; import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.geo.GeoJson; import org.elasticsearch.common.geo.GeoUtils; import org.elasticsearch.common.geo.ShapesAvailability; import org.elasticsearch.common.geo.SpatialStrategy; @@ -43,10 +44,13 @@ import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.support.XContentMapValues; +import org.elasticsearch.geometry.Geometry; +import org.elasticsearch.geometry.utils.WellKnownText; import org.elasticsearch.index.query.LegacyGeoShapeQueryProcessor; import org.locationtech.spatial4j.shape.Shape; import java.io.IOException; +import java.io.UncheckedIOException; import java.util.Collections; import java.util.List; import java.util.Map; @@ -256,6 +260,7 @@ private GeoShapeFieldType buildFieldType(BuilderContext context) { setupPrefixTrees(ft); ft.setGeometryIndexer(new LegacyGeoShapeIndexer(ft)); ft.setGeometryParser(ShapeParser::parse); + ft.setGeometryFormatter(new LegacyGeoShapeFormatter()); ft.setGeometryQueryBuilder(new LegacyGeoShapeQueryProcessor(ft)); ft.setOrientation(orientation == null ? Defaults.ORIENTATION.value() : orientation); return ft; @@ -277,6 +282,23 @@ public LegacyGeoShapeFieldMapper build(BuilderContext context) { } } + private static class LegacyGeoShapeFormatter implements Formatter> { + @Override + public Object formatGeoJson(ShapeBuilder value) { + try { + Geometry geometry = value.buildGeometry(); + return GeoJson.toXContentMap(geometry); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + + @Override + public String formatWKT(ShapeBuilder value) { + return WellKnownText.INSTANCE.toWKT(value.buildGeometry()); + } + } + public static final class GeoShapeFieldType extends AbstractShapeGeometryFieldType, Shape> { private String tree = DeprecatedParameters.Defaults.TREE; 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 abe4805bce088..2d3a0a3a9851c 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/GeoPointFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/GeoPointFieldMapperTests.java @@ -20,23 +20,30 @@ import org.apache.lucene.index.IndexableField; import org.apache.lucene.util.BytesRef; +import org.elasticsearch.Version; import org.elasticsearch.action.admin.indices.create.CreateIndexRequestBuilder; import org.elasticsearch.action.search.SearchResponse; +import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.common.Priority; import org.elasticsearch.common.Strings; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.geo.GeoPoint; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.search.lookup.SourceLookup; import org.elasticsearch.test.InternalSettingsPlugin; import org.elasticsearch.test.geo.RandomGeoGenerator; import org.hamcrest.CoreMatchers; import java.io.IOException; import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Map; import java.util.Set; import static org.elasticsearch.action.support.WriteRequest.RefreshPolicy.IMMEDIATE; @@ -586,6 +593,39 @@ public void testInvalidGeopointValuesIgnored() throws Exception { ), XContentType.JSON)).rootDoc().getField("location"), nullValue()); } + public void testParseSourceValue() { + Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT.id).build(); + Mapper.BuilderContext context = new Mapper.BuilderContext(settings, new ContentPath()); + + AbstractGeometryFieldMapper mapper = new GeoPointFieldMapper.Builder("field").build(context); + SourceLookup sourceLookup = new SourceLookup(); + + Map jsonPoint = Map.of("type", "Point", "coordinates", List.of(42.0, 27.1)); + Map otherJsonPoint = Map.of("type", "Point", "coordinates", List.of(30.0, 50.0)); + String wktPoint = "POINT (42.0 27.1)"; + String otherWktPoint = "POINT (30.0 50.0)"; + + // Test a single point in [lon, lat] array format. + sourceLookup.setSource(Collections.singletonMap("field", List.of(42.0, 27.1))); + assertEquals(List.of(jsonPoint), mapper.lookupValues(sourceLookup, null)); + assertEquals(List.of(wktPoint), mapper.lookupValues(sourceLookup, "wkt")); + + // Test a single point in "lat, lon" string format. + sourceLookup.setSource(Collections.singletonMap("field", "27.1,42.0")); + assertEquals(List.of(jsonPoint), mapper.lookupValues(sourceLookup, null)); + assertEquals(List.of(wktPoint), mapper.lookupValues(sourceLookup, "wkt")); + + // Test a list of points in [lon, lat] array format. + sourceLookup.setSource(Collections.singletonMap("field", List.of(List.of(42.0, 27.1), List.of(30.0, 50.0)))); + assertEquals(List.of(jsonPoint, otherJsonPoint), mapper.lookupValues(sourceLookup, null)); + assertEquals(List.of(wktPoint, otherWktPoint), mapper.lookupValues(sourceLookup, "wkt")); + + // Test a single point in well-known text format. + sourceLookup.setSource(Collections.singletonMap("field", "POINT (42.0 27.1)")); + assertEquals(List.of(jsonPoint), mapper.lookupValues(sourceLookup, null)); + assertEquals(List.of(wktPoint), mapper.lookupValues(sourceLookup, "wkt")); + } + @Override protected GeoPointFieldMapper.Builder newBuilder() { return new GeoPointFieldMapper.Builder("geo"); diff --git a/server/src/test/java/org/elasticsearch/index/mapper/GeoShapeFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/GeoShapeFieldMapperTests.java index 98369d5f1a15b..8e1a88fe6b9c9 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/GeoShapeFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/GeoShapeFieldMapperTests.java @@ -19,16 +19,20 @@ package org.elasticsearch.index.mapper; import org.apache.lucene.index.IndexableField; +import org.elasticsearch.Version; +import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.common.Explicit; import org.elasticsearch.common.Strings; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.geo.builders.ShapeBuilder; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.search.lookup.SourceLookup; import org.elasticsearch.test.InternalSettingsPlugin; import org.elasticsearch.test.TestGeoShapeFieldMapperPlugin; import org.junit.Before; @@ -36,6 +40,8 @@ import java.io.IOException; import java.util.Collection; import java.util.Collections; +import java.util.List; +import java.util.Map; import java.util.Set; import static org.elasticsearch.index.mapper.AbstractGeometryFieldMapper.Names.IGNORE_Z_VALUE; @@ -356,4 +362,38 @@ public String toXContentString(GeoShapeFieldMapper mapper, boolean includeDefaul public String toXContentString(GeoShapeFieldMapper mapper) throws IOException { return toXContentString(mapper, true); } + + public void testParseSourceValue() { + Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT.id).build(); + Mapper.BuilderContext context = new Mapper.BuilderContext(settings, new ContentPath()); + + GeoShapeFieldMapper mapper = new GeoShapeFieldMapper.Builder("field").build(context); + SourceLookup sourceLookup = new SourceLookup(); + + Map jsonLineString = Map.of("type", "LineString", "coordinates", + List.of(List.of(42.0, 27.1), List.of(30.0, 50.0))); + Map jsonPoint = Map.of("type", "Point", "coordinates", List.of(14.0, 15.0)); + String wktLineString = "LINESTRING (42.0 27.1, 30.0 50.0)"; + String wktPoint = "POINT (14.0 15.0)"; + + // Test a single shape in geojson format. + sourceLookup.setSource(Collections.singletonMap("field", jsonLineString)); + assertEquals(List.of(jsonLineString), mapper.lookupValues(sourceLookup, null)); + assertEquals(List.of(wktLineString), mapper.lookupValues(sourceLookup, "wkt")); + + // Test a list of shapes in geojson format. + sourceLookup.setSource(Collections.singletonMap("field", List.of(jsonLineString, jsonPoint))); + assertEquals(List.of(jsonLineString, jsonPoint), mapper.lookupValues(sourceLookup, null)); + assertEquals(List.of(wktLineString, wktPoint), mapper.lookupValues(sourceLookup, "wkt")); + + // Test a single shape in wkt format. + sourceLookup.setSource(Collections.singletonMap("field", wktLineString)); + assertEquals(List.of(jsonLineString), mapper.lookupValues(sourceLookup, null)); + assertEquals(List.of(wktLineString), mapper.lookupValues(sourceLookup, "wkt")); + + // Test a list of shapes in wkt format. + sourceLookup.setSource(Collections.singletonMap("field", List.of(wktLineString, wktPoint))); + assertEquals(List.of(jsonLineString, jsonPoint), mapper.lookupValues(sourceLookup, null)); + assertEquals(List.of(wktLineString, wktPoint), mapper.lookupValues(sourceLookup, "wkt")); + } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapperTests.java index 56d33659db52e..bd54d663541fe 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapperTests.java @@ -25,6 +25,8 @@ import org.apache.lucene.spatial.prefix.tree.QuadPrefixTree; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.ElasticsearchParseException; +import org.elasticsearch.Version; +import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.common.Explicit; import org.elasticsearch.common.Strings; import org.elasticsearch.common.bytes.BytesReference; @@ -33,6 +35,7 @@ import org.elasticsearch.common.geo.ShapeRelation; import org.elasticsearch.common.geo.SpatialStrategy; import org.elasticsearch.common.geo.builders.ShapeBuilder; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; @@ -40,6 +43,7 @@ import org.elasticsearch.geometry.Point; import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.search.lookup.SourceLookup; import org.elasticsearch.test.InternalSettingsPlugin; import org.elasticsearch.test.TestGeoShapeFieldMapperPlugin; import org.junit.Before; @@ -47,6 +51,8 @@ import java.io.IOException; import java.util.Collection; import java.util.Collections; +import java.util.List; +import java.util.Map; import java.util.Set; import static org.elasticsearch.index.mapper.AbstractGeometryFieldMapper.Names.IGNORE_Z_VALUE; @@ -839,4 +845,38 @@ public String toXContentString(LegacyGeoShapeFieldMapper mapper, boolean include public String toXContentString(LegacyGeoShapeFieldMapper mapper) throws IOException { return toXContentString(mapper, true); } + + public void testParseSourceValue() { + Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT.id).build(); + Mapper.BuilderContext context = new Mapper.BuilderContext(settings, new ContentPath()); + + LegacyGeoShapeFieldMapper mapper = new LegacyGeoShapeFieldMapper.Builder("field").build(context); + SourceLookup sourceLookup = new SourceLookup(); + + Map jsonLineString = Map.of("type", "LineString", "coordinates", + List.of(List.of(42.0, 27.1), List.of(30.0, 50.0))); + Map jsonPoint = Map.of("type", "Point", "coordinates", List.of(14.0, 15.0)); + String wktLineString = "LINESTRING (42.0 27.1, 30.0 50.0)"; + String wktPoint = "POINT (14.0 15.0)"; + + // Test a single shape in geojson format. + sourceLookup.setSource(Collections.singletonMap("field", jsonLineString)); + assertEquals(List.of(jsonLineString), mapper.lookupValues(sourceLookup, null)); + assertEquals(List.of(wktLineString), mapper.lookupValues(sourceLookup, "wkt")); + + // Test a list of shapes in geojson format. + sourceLookup.setSource(Collections.singletonMap("field", List.of(jsonLineString, jsonPoint))); + assertEquals(List.of(jsonLineString, jsonPoint), mapper.lookupValues(sourceLookup, null)); + assertEquals(List.of(wktLineString, wktPoint), mapper.lookupValues(sourceLookup, "wkt")); + + // Test a single shape in wkt format. + sourceLookup.setSource(Collections.singletonMap("field", wktLineString)); + assertEquals(List.of(jsonLineString), mapper.lookupValues(sourceLookup, null)); + assertEquals(List.of(wktLineString), mapper.lookupValues(sourceLookup, "wkt")); + + // Test a list of shapes in wkt format. + sourceLookup.setSource(Collections.singletonMap("field", List.of(wktLineString, wktPoint))); + assertEquals(List.of(jsonLineString, jsonPoint), mapper.lookupValues(sourceLookup, null)); + assertEquals(List.of(wktLineString, wktPoint), mapper.lookupValues(sourceLookup, "wkt")); + } } diff --git a/server/src/test/java/org/elasticsearch/search/fetch/subphase/FieldValueRetrieverTests.java b/server/src/test/java/org/elasticsearch/search/fetch/subphase/FieldValueRetrieverTests.java index 199df988fd6b6..3826e0ef09c07 100644 --- a/server/src/test/java/org/elasticsearch/search/fetch/subphase/FieldValueRetrieverTests.java +++ b/server/src/test/java/org/elasticsearch/search/fetch/subphase/FieldValueRetrieverTests.java @@ -120,35 +120,30 @@ public void testArrayValueMappers() throws IOException { MapperService mapperService = createMapperService(); XContentBuilder source = XContentFactory.jsonBuilder().startObject() - .array("completion", "first", "second") + .array("geo_point", 27.1, 42.0) .endObject(); - Map fields = retrieveFields(mapperService, source, "completion"); + Map fields = retrieveFields(mapperService, source, "geo_point"); assertThat(fields.size(), equalTo(1)); - DocumentField field = fields.get("completion"); + DocumentField field = fields.get("geo_point"); assertNotNull(field); - assertThat(field.getValues().size(), equalTo(2)); - assertThat(field.getValues(), hasItems("first", "second")); + assertThat(field.getValues().size(), equalTo(1)); // Test a field with multiple geo-points. source = XContentFactory.jsonBuilder().startObject() - .startObject("completion") - .array("input", "first", "second") - .field("weight", "2.718") - .endObject() + .startArray("geo_point") + .startArray().value(27.1).value(42.0).endArray() + .startArray().value(31.4).value(42.0).endArray() + .endArray() .endObject(); - fields = retrieveFields(mapperService, source, "completion"); + fields = retrieveFields(mapperService, source, "geo_point"); assertThat(fields.size(), equalTo(1)); - field = fields.get("completion"); + field = fields.get("geo_point"); assertNotNull(field); - assertThat(field.getValues().size(), equalTo(1)); - - Map expected = Map.of("input", List.of("first", "second"), - "weight", "2.718"); - assertThat(field.getValues().get(0), equalTo(expected)); + assertThat(field.getValues().size(), equalTo(2)); } public void testFieldNamesWithWildcard() throws IOException { @@ -355,7 +350,7 @@ public MapperService createMapperService() throws IOException { .startObject("field").field("type", "keyword").endObject() .startObject("integer_field").field("type", "integer").endObject() .startObject("date_field").field("type", "date").endObject() - .startObject("completion").field("type", "completion").endObject() + .startObject("geo_point").field("type", "geo_point").endObject() .startObject("float_range").field("type", "float_range").endObject() .startObject("object") .startObject("properties") diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/common/CartesianPoint.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/common/CartesianPoint.java index 000ca6a1b2451..68339f130cffb 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/common/CartesianPoint.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/common/CartesianPoint.java @@ -24,6 +24,7 @@ import java.io.IOException; import java.util.Collections; import java.util.Locale; +import java.util.Objects; import static org.elasticsearch.index.mapper.AbstractGeometryFieldMapper.Names.IGNORE_Z_VALUE; @@ -141,13 +142,7 @@ public boolean equals(Object o) { @Override public int hashCode() { - int result; - int temp; - temp = x != +0.0f ? Float.floatToIntBits(x) : 0; - result = Integer.hashCode(temp); - temp = y != +0.0f ? Float.floatToIntBits(y) : 0; - result = 31 * result + Integer.hashCode(temp); - return result; + return Objects.hash(x, y); } @Override 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 644b5b5713aff..e727588b625c4 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 @@ -21,6 +21,7 @@ import org.elasticsearch.index.fielddata.IndexFieldData; import org.elasticsearch.index.mapper.AbstractShapeGeometryFieldMapper; import org.elasticsearch.index.mapper.GeoShapeFieldMapper; +import org.elasticsearch.index.mapper.GeoShapeFormatter; import org.elasticsearch.index.mapper.GeoShapeIndexer; import org.elasticsearch.index.mapper.LegacyGeoShapeFieldMapper; import org.elasticsearch.index.mapper.MappedFieldType; @@ -92,6 +93,7 @@ public GeoShapeWithDocValuesFieldMapper build(BuilderContext context) { ignoreZValue().value()); ft.setGeometryParser((parser, mapper) -> geometryParser.parse(parser)); ft.setGeometryIndexer(new GeoShapeIndexer(orientation().value().getAsBoolean(), ft.name())); + ft.setGeometryFormatter(new GeoShapeFormatter()); ft.setGeometryQueryBuilder(new VectorGeoShapeQueryProcessor()); ft.setOrientation(orientation().value()); return new GeoShapeWithDocValuesFieldMapper(name, fieldType, ft, ignoreMalformed(context), coerce(context), 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 556bb4c636d05..5cdd54201a2a9 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 @@ -12,6 +12,7 @@ import org.apache.lucene.index.IndexableField; import org.elasticsearch.common.Explicit; import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.geometry.Point; import org.elasticsearch.index.mapper.AbstractPointGeometryFieldMapper; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.mapper.ParseContext; @@ -46,6 +47,7 @@ public PointFieldMapper build(BuilderContext context, String simpleName, FieldTy PointFieldType ft = new PointFieldType(buildFullName(context), indexed, hasDocValues, meta); ft.setGeometryParser(new PointParser<>()); ft.setGeometryIndexer(new PointIndexer(ft)); + ft.setGeometryFormatter(new PointFormatter<>()); ft.setGeometryQueryBuilder(new ShapeQueryPointProcessor()); return new PointFieldMapper(simpleName, fieldType, ft, multiFields, ignoreMalformed, ignoreZValue(context), nullValue, copyTo); @@ -135,7 +137,7 @@ public String typeName() { return CONTENT_TYPE; } } - + // 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 { @@ -164,6 +166,11 @@ public void resetCoords(double x, double y) { this.reset((float)x, (float)y); } + @Override + public Point asGeometry() { + return new Point(getX(), getY()); + } + @Override public boolean equals(Object other) { double oX; 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 09cdd1ed63fe4..902f06e169b80 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 @@ -12,6 +12,7 @@ import org.elasticsearch.common.geo.builders.ShapeBuilder.Orientation; import org.elasticsearch.geometry.Geometry; import org.elasticsearch.index.mapper.AbstractShapeGeometryFieldMapper; +import org.elasticsearch.index.mapper.GeoShapeFormatter; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.mapper.MapperParsingException; import org.elasticsearch.index.mapper.ParseContext; @@ -55,6 +56,7 @@ public ShapeFieldMapper build(BuilderContext context) { = new GeometryParser(orientation().value().getAsBoolean(), coerce().value(), ignoreZValue().value()); ft.setGeometryParser((parser, mapper) -> geometryParser.parse(parser)); ft.setGeometryIndexer(new ShapeIndexer(ft.name())); + ft.setGeometryFormatter(new GeoShapeFormatter()); ft.setGeometryQueryBuilder(new ShapeQueryProcessor()); ft.setOrientation(orientation().value()); return new ShapeFieldMapper(name, fieldType, ft, ignoreMalformed(context), coerce(context), diff --git a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/PointFieldMapperTests.java b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/PointFieldMapperTests.java index 2c1998f12aa64..fbcfcaa8325cf 100644 --- a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/PointFieldMapperTests.java +++ b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/PointFieldMapperTests.java @@ -6,20 +6,29 @@ package org.elasticsearch.xpack.spatial.index.mapper; import org.apache.lucene.util.BytesRef; +import org.elasticsearch.Version; +import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.common.Strings; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.compress.CompressedXContent; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.index.mapper.AbstractGeometryFieldMapper; +import org.elasticsearch.index.mapper.ContentPath; import org.elasticsearch.index.mapper.DocumentMapper; import org.elasticsearch.index.mapper.Mapper; import org.elasticsearch.index.mapper.ParsedDocument; import org.elasticsearch.index.mapper.SourceToParse; +import org.elasticsearch.search.lookup.SourceLookup; import org.elasticsearch.xpack.spatial.common.CartesianPoint; import org.hamcrest.CoreMatchers; import java.io.IOException; +import java.util.Collections; +import java.util.List; +import java.util.Map; import static org.elasticsearch.index.mapper.AbstractPointGeometryFieldMapper.Names.IGNORE_Z_VALUE; import static org.elasticsearch.index.mapper.AbstractPointGeometryFieldMapper.Names.NULL_VALUE; @@ -295,4 +304,37 @@ public void testIgnoreZValue() throws IOException { ignoreZValue = ((PointFieldMapper)fieldMapper).ignoreZValue().value(); assertThat(ignoreZValue, equalTo(false)); } + + public void testParseSourceValue() { + Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT.id).build(); + Mapper.BuilderContext context = new Mapper.BuilderContext(settings, new ContentPath()); + + AbstractGeometryFieldMapper mapper = new PointFieldMapper.Builder("field").build(context); + SourceLookup sourceLookup = new SourceLookup(); + + Map jsonPoint = Map.of("type", "Point", "coordinates", List.of(42.0, 27.100000381469727)); + String wktPoint = "POINT (42.0 27.100000381469727)"; + Map otherJsonPoint = Map.of("type", "Point", "coordinates", List.of(30.0, 50.0)); + String otherWktPoint = "POINT (30.0 50.0)"; + + // Test a single point in [x, y] array format. + sourceLookup.setSource(Collections.singletonMap("field", List.of(42.0, 27.1))); + assertEquals(List.of(jsonPoint), mapper.lookupValues(sourceLookup, null)); + assertEquals(List.of(wktPoint), mapper.lookupValues(sourceLookup, "wkt")); + + // Test a single point in "x, y" string format. + sourceLookup.setSource(Collections.singletonMap("field", "42.0,27.1")); + assertEquals(List.of(jsonPoint), mapper.lookupValues(sourceLookup, null)); + assertEquals(List.of(wktPoint), mapper.lookupValues(sourceLookup, "wkt")); + + // Test a list of points in [x, y] array format. + sourceLookup.setSource(Collections.singletonMap("field", List.of(List.of(42.0, 27.1), List.of(30.0, 50.0)))); + assertEquals(List.of(jsonPoint, otherJsonPoint), mapper.lookupValues(sourceLookup, null)); + assertEquals(List.of(wktPoint, otherWktPoint), mapper.lookupValues(sourceLookup, "wkt")); + + // Test a single point in well-known text format. + sourceLookup.setSource(Collections.singletonMap("field", "POINT (42.0 27.1)")); + assertEquals(List.of(jsonPoint), mapper.lookupValues(sourceLookup, null)); + assertEquals(List.of(wktPoint), mapper.lookupValues(sourceLookup, "wkt")); + } } diff --git a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/ShapeFieldMapperTests.java b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/ShapeFieldMapperTests.java index 496c7b6df6e6f..a6b6a3fe3840e 100644 --- a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/ShapeFieldMapperTests.java +++ b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/ShapeFieldMapperTests.java @@ -6,25 +6,32 @@ package org.elasticsearch.xpack.spatial.index.mapper; import org.apache.lucene.index.IndexableField; +import org.elasticsearch.Version; +import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.common.Explicit; import org.elasticsearch.common.Strings; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.geo.builders.ShapeBuilder; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.index.mapper.AbstractShapeGeometryFieldMapper; +import org.elasticsearch.index.mapper.ContentPath; import org.elasticsearch.index.mapper.DocumentMapper; import org.elasticsearch.index.mapper.DocumentMapperParser; import org.elasticsearch.index.mapper.Mapper; import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.index.mapper.ParsedDocument; import org.elasticsearch.index.mapper.SourceToParse; +import org.elasticsearch.search.lookup.SourceLookup; import java.io.IOException; import java.util.Collections; +import java.util.List; +import java.util.Map; import static org.elasticsearch.index.mapper.AbstractPointGeometryFieldMapper.Names.IGNORE_Z_VALUE; import static org.hamcrest.Matchers.equalTo; @@ -324,4 +331,38 @@ public String toXContentString(ShapeFieldMapper mapper, boolean includeDefaults) public String toXContentString(ShapeFieldMapper mapper) throws IOException { return toXContentString(mapper, true); } + + public void testParseSourceValue() { + Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT.id).build(); + Mapper.BuilderContext context = new Mapper.BuilderContext(settings, new ContentPath()); + + ShapeFieldMapper mapper = new ShapeFieldMapper.Builder("field").build(context); + SourceLookup sourceLookup = new SourceLookup(); + + Map jsonLineString = Map.of("type", "LineString", "coordinates", + List.of(List.of(42.0, 27.1), List.of(30.0, 50.0))); + Map jsonPoint = Map.of("type", "Point", "coordinates", List.of(14.3, 15.0)); + String wktLineString = "LINESTRING (42.0 27.1, 30.0 50.0)"; + String wktPoint = "POINT (14.3 15.0)"; + + // Test a single shape in geojson format. + sourceLookup.setSource(Collections.singletonMap("field", jsonLineString)); + assertEquals(List.of(jsonLineString), mapper.lookupValues(sourceLookup, null)); + assertEquals(List.of(wktLineString), mapper.lookupValues(sourceLookup, "wkt")); + + // Test a list of shapes in geojson format. + sourceLookup.setSource(Collections.singletonMap("field", List.of(jsonLineString, jsonPoint))); + assertEquals(List.of(jsonLineString, jsonPoint), mapper.lookupValues(sourceLookup, null)); + assertEquals(List.of(wktLineString, wktPoint), mapper.lookupValues(sourceLookup, "wkt")); + + // Test a single shape in wkt format. + sourceLookup.setSource(Collections.singletonMap("field", wktLineString)); + assertEquals(List.of(jsonLineString), mapper.lookupValues(sourceLookup, null)); + assertEquals(List.of(wktLineString), mapper.lookupValues(sourceLookup, "wkt")); + + // Test a list of shapes in wkt format. + sourceLookup.setSource(Collections.singletonMap("field", List.of(wktLineString, wktPoint))); + assertEquals(List.of(jsonLineString, jsonPoint), mapper.lookupValues(sourceLookup, null)); + assertEquals(List.of(wktLineString, wktPoint), mapper.lookupValues(sourceLookup, "wkt")); + } } From d50fa6ff30216c66c8a38fea34bc30bffc1b117f Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Fri, 17 Jul 2020 18:32:48 -0700 Subject: [PATCH 2/9] In CartesianPoint, store coordinates as doubles. This gives more predictable values when parsing and formatting a point. It matches the behavior for GeoPoint and the Point geometry. --- .../xpack/spatial/common/CartesianPoint.java | 56 +++++++++---------- .../index/mapper/PointFieldMapper.java | 14 ++--- .../index/mapper/PointFieldMapperTests.java | 4 +- 3 files changed, 37 insertions(+), 37 deletions(-) diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/common/CartesianPoint.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/common/CartesianPoint.java index 68339f130cffb..aa96ec40455d5 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/common/CartesianPoint.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/common/CartesianPoint.java @@ -37,18 +37,18 @@ public class CartesianPoint implements ToXContentFragment { private static final ParseField Y_FIELD = new ParseField("y"); private static final ParseField Z_FIELD = new ParseField("z"); - protected float x; - protected float y; + protected double x; + protected double y; public CartesianPoint() { } - public CartesianPoint(float x, float y) { + public CartesianPoint(double x, double y) { this.x = x; this.y = y; } - public CartesianPoint reset(float x, float y) { + public CartesianPoint reset(double x, double y) { this.x = x; this.y = y; return this; @@ -69,11 +69,11 @@ public CartesianPoint resetFromCoordinates(String value, final boolean ignoreZVa throw new ElasticsearchParseException("failed to parse [{}], expected 2 or 3 coordinates " + "but found: [{}]", vals, vals.length); } - final float x; - final float y; + final double x; + final double y; try { - x = Float.parseFloat(vals[0].trim()); - if (Float.isFinite(x) == false) { + x = Double.parseDouble(vals[0].trim()); + if (Double.isFinite(x) == false) { throw new ElasticsearchParseException("invalid [{}] value [{}]; " + "must be between -3.4028234663852886E38 and 3.4028234663852886E38", X_FIELD.getPreferredName(), @@ -83,8 +83,8 @@ public CartesianPoint resetFromCoordinates(String value, final boolean ignoreZVa throw new ElasticsearchParseException("[{}]] must be a number", X_FIELD.getPreferredName()); } try { - y = Float.parseFloat(vals[1].trim()); - if (Float.isFinite(y) == false) { + y = Double.parseDouble(vals[1].trim()); + if (Double.isFinite(y) == false) { throw new ElasticsearchParseException("invalid [{}] value [{}]; " + "must be between -3.4028234663852886E38 and 3.4028234663852886E38", Y_FIELD.getPreferredName(), @@ -95,7 +95,7 @@ public CartesianPoint resetFromCoordinates(String value, final boolean ignoreZVa } if (vals.length > 2) { try { - CartesianPoint.assertZValue(ignoreZValue, Float.parseFloat(vals[2].trim())); + CartesianPoint.assertZValue(ignoreZValue, Double.parseDouble(vals[2].trim())); } catch (NumberFormatException ex) { throw new ElasticsearchParseException("[{}]] must be a number", Y_FIELD.getPreferredName()); } @@ -116,14 +116,14 @@ private CartesianPoint resetFromWKT(String value, boolean ignoreZValue) { "but found {}", PointFieldMapper.CONTENT_TYPE, geometry.type()); } org.elasticsearch.geometry.Point point = (org.elasticsearch.geometry.Point) geometry; - return reset((float) point.getX(), (float) point.getY()); + return reset(point.getX(), point.getY()); } - public float getX() { + public double getX() { return this.x; } - public float getY() { + public double getY() { return this.y; } @@ -134,8 +134,8 @@ public boolean equals(Object o) { CartesianPoint point = (CartesianPoint) o; - if (Float.compare(point.x, x) != 0) return false; - if (Float.compare(point.y, y) != 0) return false; + if (Double.compare(point.x, x) != 0) return false; + if (Double.compare(point.y, y) != 0) return false; return true; } @@ -157,8 +157,8 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws public static CartesianPoint parsePoint(XContentParser parser, CartesianPoint point, boolean ignoreZvalue) throws IOException, ElasticsearchParseException { - float x = Float.NaN; - float y = Float.NaN; + double x = Double.NaN; + double y = Double.NaN; NumberFormatException numberFormatException = null; if(parser.currentToken() == XContentParser.Token.START_OBJECT) { @@ -172,7 +172,7 @@ public static CartesianPoint parsePoint(XContentParser parser, CartesianPoint po case VALUE_NUMBER: case VALUE_STRING: try { - x = subParser.floatValue(true); + x = subParser.doubleValue(true); } catch (NumberFormatException e) { numberFormatException = e; } @@ -187,7 +187,7 @@ public static CartesianPoint parsePoint(XContentParser parser, CartesianPoint po case VALUE_NUMBER: case VALUE_STRING: try { - y = subParser.floatValue(true); + y = subParser.doubleValue(true); } catch (NumberFormatException e) { numberFormatException = e; } @@ -202,7 +202,7 @@ public static CartesianPoint parsePoint(XContentParser parser, CartesianPoint po case VALUE_NUMBER: case VALUE_STRING: try { - CartesianPoint.assertZValue(ignoreZvalue, subParser.floatValue(true)); + CartesianPoint.assertZValue(ignoreZvalue, subParser.doubleValue(true)); } catch (NumberFormatException e) { numberFormatException = e; } @@ -222,12 +222,12 @@ public static CartesianPoint parsePoint(XContentParser parser, CartesianPoint po } } if (numberFormatException != null) { - throw new ElasticsearchParseException("[{}] and [{}] must be valid float values", numberFormatException, + throw new ElasticsearchParseException("[{}] and [{}] must be valid double values", numberFormatException, X_FIELD.getPreferredName(), Y_FIELD.getPreferredName()); - } else if (Float.isNaN(x)) { + } else if (Double.isNaN(x)) { throw new ElasticsearchParseException("field [{}] missing", X_FIELD.getPreferredName()); - } else if (Float.isNaN(y)) { + } else if (Double.isNaN(y)) { throw new ElasticsearchParseException("field [{}] missing", Y_FIELD.getPreferredName()); } else { return point.reset(x, y); @@ -240,9 +240,9 @@ public static CartesianPoint parsePoint(XContentParser parser, CartesianPoint po if (subParser.currentToken() == XContentParser.Token.VALUE_NUMBER) { element++; if (element == 1) { - x = subParser.floatValue(); + x = subParser.doubleValue(); } else if (element == 2) { - y = subParser.floatValue(); + y = subParser.doubleValue(); } else { throw new ElasticsearchParseException("[{}}] field type does not accept > 2 dimensions", PointFieldMapper.CONTENT_TYPE); @@ -277,12 +277,12 @@ public static CartesianPoint parsePoint(Object value, CartesianPoint point, bool } } - public static double assertZValue(final boolean ignoreZValue, float zValue) { + public static double assertZValue(final boolean ignoreZValue, double zValue) { if (ignoreZValue == false) { throw new ElasticsearchParseException("Exception parsing coordinates: found Z value [{}] but [{}] " + "parameter is [{}]", zValue, IGNORE_Z_VALUE, ignoreZValue); } - if (Float.isFinite(zValue) == false) { + if (Double.isFinite(zValue) == false) { throw new ElasticsearchParseException("invalid [{}] value [{}]; " + "must be between -3.4028234663852886E38 and 3.4028234663852886E38", Z_FIELD.getPreferredName(), 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 5cdd54201a2a9..5a34e71d2bbf2 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 @@ -71,10 +71,10 @@ protected ParsedPoint parseNullValue(Object nullValue, boolean ignoreZValue, boo ParsedCartesianPoint point = new ParsedCartesianPoint(); CartesianPoint.parsePoint(nullValue, point, ignoreZValue); if (ignoreMalformed == false) { - if (Float.isFinite(point.getX()) == false) { + if (Double.isFinite(point.getX()) == false) { throw new IllegalArgumentException("illegal x value [" + point.getX() + "]"); } - if (Float.isFinite(point.getY()) == false) { + if (Double.isFinite(point.getY()) == false) { throw new IllegalArgumentException("illegal y value [" + point.getY() + "]"); } } @@ -108,7 +108,7 @@ protected void addStoredFields(ParseContext context, List points, List fields, ParseContext context) { for (CartesianPoint point : points) { - context.doc().add(new XYDocValuesField(fieldType().name(), point.getX(), point.getY())); + context.doc().add(new XYDocValuesField(fieldType().name(), (float) point.getX(), (float) point.getY())); } } @@ -143,10 +143,10 @@ public String typeName() { protected static class ParsedCartesianPoint extends CartesianPoint implements AbstractPointGeometryFieldMapper.ParsedPoint { @Override public void validate(String fieldName) { - if (Float.isFinite(getX()) == false) { + if (Double.isFinite(getX()) == false) { throw new IllegalArgumentException("illegal x value [" + getX() + "] for " + fieldName); } - if (Float.isFinite(getY()) == false) { + if (Double.isFinite(getY()) == false) { throw new IllegalArgumentException("illegal y value [" + getY() + "] for " + fieldName); } } @@ -163,7 +163,7 @@ public boolean isNormalizable(double coord) { @Override public void resetCoords(double x, double y) { - this.reset((float)x, (float)y); + this.reset(x, y); } @Override @@ -223,7 +223,7 @@ public Class> processedClass() { public List indexShape(ParseContext context, List points) { ArrayList fields = new ArrayList<>(1); for (CartesianPoint point : points) { - fields.add(new XYPointField(fieldType.name(), point.getX(), point.getY())); + fields.add(new XYPointField(fieldType.name(), (float) point.getX(), (float) point.getY())); } return fields; } diff --git a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/PointFieldMapperTests.java b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/PointFieldMapperTests.java index fbcfcaa8325cf..56f2d3fd8d33c 100644 --- a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/PointFieldMapperTests.java +++ b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/PointFieldMapperTests.java @@ -312,8 +312,8 @@ public void testParseSourceValue() { AbstractGeometryFieldMapper mapper = new PointFieldMapper.Builder("field").build(context); SourceLookup sourceLookup = new SourceLookup(); - Map jsonPoint = Map.of("type", "Point", "coordinates", List.of(42.0, 27.100000381469727)); - String wktPoint = "POINT (42.0 27.100000381469727)"; + Map jsonPoint = Map.of("type", "Point", "coordinates", List.of(42.0, 27.1)); + String wktPoint = "POINT (42.0 27.1)"; Map otherJsonPoint = Map.of("type", "Point", "coordinates", List.of(30.0, 50.0)); String otherWktPoint = "POINT (30.0 50.0)"; From 804a2956cf50b9e9bf7aba61cb71aa9072927181 Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Mon, 20 Jul 2020 13:54:52 -0700 Subject: [PATCH 3/9] Fix naming of 'spatial datatypes' anchor. --- docs/reference/mapping/types.asciidoc | 2 +- docs/reference/search/search-fields.asciidoc | 2 +- .../xpack/spatial/index/mapper/PointFieldMapper.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/reference/mapping/types.asciidoc b/docs/reference/mapping/types.asciidoc index ca0bc115c21c6..afdb943f4df4a 100644 --- a/docs/reference/mapping/types.asciidoc +++ b/docs/reference/mapping/types.asciidoc @@ -22,7 +22,7 @@ string:: <>, <> and <>:: `nested` for arrays of JSON objects [float] -[[_spatial_datatypes]] +[[spatial_datatypes]] === Spatial data types <>:: `geo_point` for lat/lon points diff --git a/docs/reference/search/search-fields.asciidoc b/docs/reference/search/search-fields.asciidoc index 66423549c2927..bea01e3bb390e 100644 --- a/docs/reference/search/search-fields.asciidoc +++ b/docs/reference/search/search-fields.asciidoc @@ -89,7 +89,7 @@ POST twitter/_search <2> Using object notation, you can pass a `format` parameter to apply a custom format for the field's values. The date fields <> and <> accept a - <>. <<_spatial_datatypes, Spatial fields>> + <>. <> accept either `geojson` for http://www.geojson.org[GeoJSON] (the default) or `wkt` for https://en.wikipedia.org/wiki/Well-known_text_representation_of_geometry[Well Known Text]. 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 5a34e71d2bbf2..78af37da2a469 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 @@ -137,7 +137,7 @@ public String typeName() { return CONTENT_TYPE; } } - + // 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 { From 6f9e82a7dca40673e8acd767d99812e6e5d5a7a3 Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Tue, 21 Jul 2020 11:29:24 -0700 Subject: [PATCH 4/9] Better formalize the types of geometry formats. --- .../common/geo/GeoJsonGeometryFormat.java | 60 ++++++++++++++++ .../common/geo/GeometryFormat.java | 6 +- .../common/geo/GeometryParser.java | 72 ++++++------------- .../common/geo/WKTGeometryFormat.java | 61 ++++++++++++++++ 4 files changed, 148 insertions(+), 51 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/common/geo/GeoJsonGeometryFormat.java create mode 100644 server/src/main/java/org/elasticsearch/common/geo/WKTGeometryFormat.java diff --git a/server/src/main/java/org/elasticsearch/common/geo/GeoJsonGeometryFormat.java b/server/src/main/java/org/elasticsearch/common/geo/GeoJsonGeometryFormat.java new file mode 100644 index 0000000000000..3cc988711b644 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/common/geo/GeoJsonGeometryFormat.java @@ -0,0 +1,60 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.common.geo; + +import org.elasticsearch.common.xcontent.ToXContent; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.geometry.Geometry; + +import java.io.IOException; +import java.util.Map; + +public class GeoJsonGeometryFormat implements GeometryFormat { + public static final String NAME = "geojson"; + + private final GeoJson geoJsonParser; + + public GeoJsonGeometryFormat(GeoJson geoJsonParser) { + this.geoJsonParser = geoJsonParser; + } + + @Override + public String name() { + return NAME; + } + + @Override + public Geometry fromXContent(XContentParser parser) throws IOException { + if (parser.currentToken() == XContentParser.Token.VALUE_NULL) { + return null; + } + return geoJsonParser.fromXContent(parser); + } + + @Override + public XContentBuilder toXContent(Geometry geometry, XContentBuilder builder, ToXContent.Params params) throws IOException { + if (geometry != null) { + return GeoJson.toXContent(geometry, builder, params); + } else { + return builder.nullValue(); + } + } +} diff --git a/server/src/main/java/org/elasticsearch/common/geo/GeometryFormat.java b/server/src/main/java/org/elasticsearch/common/geo/GeometryFormat.java index 4b12b8fe0ac96..01761b4761e79 100644 --- a/server/src/main/java/org/elasticsearch/common/geo/GeometryFormat.java +++ b/server/src/main/java/org/elasticsearch/common/geo/GeometryFormat.java @@ -31,6 +31,11 @@ */ public interface GeometryFormat { + /** + * The name of the format, for example 'wkt'. + */ + String name(); + /** * Parser JSON representation of a geometry */ @@ -40,5 +45,4 @@ public interface GeometryFormat { * Serializes the geometry into its JSON representation */ XContentBuilder toXContent(ParsedFormat geometry, XContentBuilder builder, ToXContent.Params params) throws IOException; - } diff --git a/server/src/main/java/org/elasticsearch/common/geo/GeometryParser.java b/server/src/main/java/org/elasticsearch/common/geo/GeometryParser.java index d39e7752a2dc3..d12bd19da3695 100644 --- a/server/src/main/java/org/elasticsearch/common/geo/GeometryParser.java +++ b/server/src/main/java/org/elasticsearch/common/geo/GeometryParser.java @@ -66,59 +66,31 @@ public Geometry parse(XContentParser parser) throws IOException, ParseException /** * Returns a geometry format object that can parse and then serialize the object back to the same format. */ - public GeometryFormat geometryFormat(XContentParser parser) { - if (parser.currentToken() == XContentParser.Token.VALUE_NULL) { - return new GeometryFormat() { - @Override - public Geometry fromXContent(XContentParser parser) throws IOException { - return null; - } - - @Override - public XContentBuilder toXContent(Geometry geometry, XContentBuilder builder, ToXContent.Params params) throws IOException { - if (geometry != null) { - // We don't know the format of the original geometry - so going with default - return GeoJson.toXContent(geometry, builder, params); - } else { - return builder.nullValue(); - } - } - }; - } else if (parser.currentToken() == XContentParser.Token.START_OBJECT) { - return new GeometryFormat() { - @Override - public Geometry fromXContent(XContentParser parser) throws IOException { - return geoJsonParser.fromXContent(parser); - } + public GeometryFormat geometryFormat(String format) { + if (format.equals(GeoJsonGeometryFormat.NAME)) { + return new GeoJsonGeometryFormat(geoJsonParser); + } else if (format.equals(WKTGeometryFormat.NAME)) { + return new WKTGeometryFormat(wellKnownTextParser); + } else { + throw new IllegalArgumentException("Unrecognized geometry format [" + format + "]."); + } + } - @Override - public XContentBuilder toXContent(Geometry geometry, XContentBuilder builder, ToXContent.Params params) throws IOException { - if (geometry != null) { - return GeoJson.toXContent(geometry, builder, params); - } else { - return builder.nullValue(); - } - } - }; + /** + * Returns a geometry format object that can parse and then serialize the object back to the same format. + * This method automatically recognizes the format by examining the provided {@link XContentParser}. + */ + public GeometryFormat geometryFormat(XContentParser parser) { + if (parser.currentToken() == XContentParser.Token.START_OBJECT) { + return new GeoJsonGeometryFormat(geoJsonParser); } else if (parser.currentToken() == XContentParser.Token.VALUE_STRING) { - return new GeometryFormat() { - @Override - public Geometry fromXContent(XContentParser parser) throws IOException, ParseException { - return wellKnownTextParser.fromWKT(parser.text()); - } - - @Override - public XContentBuilder toXContent(Geometry geometry, XContentBuilder builder, ToXContent.Params params) throws IOException { - if (geometry != null) { - return builder.value(wellKnownTextParser.toWKT(geometry)); - } else { - return builder.nullValue(); - } - } - }; - + return new WKTGeometryFormat(wellKnownTextParser); + } else if (parser.currentToken() == XContentParser.Token.VALUE_NULL) { + // We don't know the format of the original geometry - so going with default + return new GeoJsonGeometryFormat(geoJsonParser); + } else { + throw new ElasticsearchParseException("shape must be an object consisting of type and coordinates"); } - throw new ElasticsearchParseException("shape must be an object consisting of type and coordinates"); } /** diff --git a/server/src/main/java/org/elasticsearch/common/geo/WKTGeometryFormat.java b/server/src/main/java/org/elasticsearch/common/geo/WKTGeometryFormat.java new file mode 100644 index 0000000000000..d79b3da42c592 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/common/geo/WKTGeometryFormat.java @@ -0,0 +1,61 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.common.geo; + +import org.elasticsearch.common.xcontent.ToXContent; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.geometry.Geometry; +import org.elasticsearch.geometry.utils.WellKnownText; + +import java.io.IOException; +import java.text.ParseException; + +public class WKTGeometryFormat implements GeometryFormat { + public static final String NAME = "wkt"; + + private final WellKnownText wellKnownTextParser; + + public WKTGeometryFormat(WellKnownText wellKnownTextParser) { + this.wellKnownTextParser = wellKnownTextParser; + } + + @Override + public String name() { + return NAME; + } + + @Override + public Geometry fromXContent(XContentParser parser) throws IOException, ParseException { + if (parser.currentToken() == XContentParser.Token.VALUE_NULL) { + return null; + } + return wellKnownTextParser.fromWKT(parser.text()); + } + + @Override + public XContentBuilder toXContent(Geometry geometry, XContentBuilder builder, ToXContent.Params params) throws IOException { + if (geometry != null) { + return builder.value(wellKnownTextParser.toWKT(geometry)); + } else { + return builder.nullValue(); + } + } +} From 926993dd932cb6a756ab77eae2f06d6c4c1f9afb Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Tue, 21 Jul 2020 13:36:42 -0700 Subject: [PATCH 5/9] Rely on GeometryFormat for all value formatting. --- .../org/elasticsearch/common/geo/GeoJson.java | 18 -------- .../common/geo/GeoJsonGeometryFormat.java | 23 +++++++++++ .../common/geo/GeometryFormat.java | 7 ++++ .../common/geo/GeometryParser.java | 4 +- .../common/geo/WKTGeometryFormat.java | 5 +++ .../mapper/AbstractGeometryFieldMapper.java | 41 ++++++++----------- .../AbstractPointGeometryFieldMapper.java | 34 ++++++--------- .../index/mapper/GeoPointFieldMapper.java | 1 - .../index/mapper/GeoShapeFieldMapper.java | 3 +- ...hapeFormatter.java => GeoShapeParser.java} | 26 ++++++------ .../mapper/LegacyGeoShapeFieldMapper.java | 34 ++++++++------- .../GeoShapeWithDocValuesFieldMapper.java | 5 +-- .../index/mapper/PointFieldMapper.java | 3 +- .../index/mapper/ShapeFieldMapper.java | 5 +-- 14 files changed, 105 insertions(+), 104 deletions(-) rename server/src/main/java/org/elasticsearch/index/mapper/{GeoShapeFormatter.java => GeoShapeParser.java} (58%) diff --git a/server/src/main/java/org/elasticsearch/common/geo/GeoJson.java b/server/src/main/java/org/elasticsearch/common/geo/GeoJson.java index fe6811d82f6ed..85b35c6393b9b 100644 --- a/server/src/main/java/org/elasticsearch/common/geo/GeoJson.java +++ b/server/src/main/java/org/elasticsearch/common/geo/GeoJson.java @@ -22,21 +22,15 @@ import org.elasticsearch.ElasticsearchException; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.common.ParseField; -import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.geo.parsers.ShapeParser; -import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.unit.DistanceUnit; import org.elasticsearch.common.xcontent.ConstructingObjectParser; -import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; -import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.ObjectParser; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentSubParser; -import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.geometry.Circle; import org.elasticsearch.geometry.Geometry; import org.elasticsearch.geometry.GeometryCollection; @@ -56,7 +50,6 @@ import java.util.ArrayList; import java.util.List; import java.util.Locale; -import java.util.Map; import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg; import static org.elasticsearch.common.xcontent.ConstructingObjectParser.optionalConstructorArg; @@ -616,15 +609,4 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws return builder; } } - - public static Map toXContentMap(Geometry geometry) throws IOException { - XContentBuilder builder = XContentFactory.jsonBuilder(); - GeoJson.toXContent(geometry, builder, ToXContent.EMPTY_PARAMS); - StreamInput input = BytesReference.bytes(builder).streamInput(); - - try (XContentParser parser = XContentType.JSON.xContent() - .createParser(NamedXContentRegistry.EMPTY, LoggingDeprecationHandler.INSTANCE, input)) { - return parser.map(); - } - } } diff --git a/server/src/main/java/org/elasticsearch/common/geo/GeoJsonGeometryFormat.java b/server/src/main/java/org/elasticsearch/common/geo/GeoJsonGeometryFormat.java index 3cc988711b644..7a88f4936f36d 100644 --- a/server/src/main/java/org/elasticsearch/common/geo/GeoJsonGeometryFormat.java +++ b/server/src/main/java/org/elasticsearch/common/geo/GeoJsonGeometryFormat.java @@ -19,12 +19,19 @@ package org.elasticsearch.common.geo; +import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; +import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.geometry.Geometry; import java.io.IOException; +import java.io.UncheckedIOException; import java.util.Map; public class GeoJsonGeometryFormat implements GeometryFormat { @@ -57,4 +64,20 @@ public XContentBuilder toXContent(Geometry geometry, XContentBuilder builder, To return builder.nullValue(); } } + + @Override + public Map toObject(Geometry geometry) { + try { + XContentBuilder builder = XContentFactory.jsonBuilder(); + GeoJson.toXContent(geometry, builder, ToXContent.EMPTY_PARAMS); + StreamInput input = BytesReference.bytes(builder).streamInput(); + + try (XContentParser parser = XContentType.JSON.xContent() + .createParser(NamedXContentRegistry.EMPTY, LoggingDeprecationHandler.INSTANCE, input)) { + return parser.map(); + } + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } } diff --git a/server/src/main/java/org/elasticsearch/common/geo/GeometryFormat.java b/server/src/main/java/org/elasticsearch/common/geo/GeometryFormat.java index 01761b4761e79..5c77491e0fc8e 100644 --- a/server/src/main/java/org/elasticsearch/common/geo/GeometryFormat.java +++ b/server/src/main/java/org/elasticsearch/common/geo/GeometryFormat.java @@ -45,4 +45,11 @@ public interface GeometryFormat { * Serializes the geometry into its JSON representation */ XContentBuilder toXContent(ParsedFormat geometry, XContentBuilder builder, ToXContent.Params params) throws IOException; + + /** + * Serializes the geometry into a standard Java object. + * + * For example, the GeoJson format returns the geometry as a map, while WKT returns a string. + */ + Object toObject(ParsedFormat geometry); } diff --git a/server/src/main/java/org/elasticsearch/common/geo/GeometryParser.java b/server/src/main/java/org/elasticsearch/common/geo/GeometryParser.java index d12bd19da3695..ce684fd72cbd3 100644 --- a/server/src/main/java/org/elasticsearch/common/geo/GeometryParser.java +++ b/server/src/main/java/org/elasticsearch/common/geo/GeometryParser.java @@ -22,15 +22,13 @@ import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; import org.elasticsearch.common.xcontent.NamedXContentRegistry; -import org.elasticsearch.common.xcontent.ToXContent; -import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.support.MapXContentParser; import org.elasticsearch.geometry.Geometry; import org.elasticsearch.geometry.GeometryCollection; import org.elasticsearch.geometry.Point; -import org.elasticsearch.geometry.utils.StandardValidator; import org.elasticsearch.geometry.utils.GeometryValidator; +import org.elasticsearch.geometry.utils.StandardValidator; import org.elasticsearch.geometry.utils.WellKnownText; import java.io.IOException; diff --git a/server/src/main/java/org/elasticsearch/common/geo/WKTGeometryFormat.java b/server/src/main/java/org/elasticsearch/common/geo/WKTGeometryFormat.java index d79b3da42c592..c13d93b025521 100644 --- a/server/src/main/java/org/elasticsearch/common/geo/WKTGeometryFormat.java +++ b/server/src/main/java/org/elasticsearch/common/geo/WKTGeometryFormat.java @@ -58,4 +58,9 @@ public XContentBuilder toXContent(Geometry geometry, XContentBuilder builder, To return builder.nullValue(); } } + + @Override + public String toObject(Geometry geometry) { + return wellKnownTextParser.toWKT(geometry); + } } 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 990e5b33df178..d608a1f734c13 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java @@ -27,6 +27,7 @@ import org.apache.lucene.search.TermQuery; import org.elasticsearch.common.Explicit; import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.geo.GeoJsonGeometryFormat; import org.elasticsearch.common.geo.ShapeRelation; import org.elasticsearch.common.geo.SpatialStrategy; import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; @@ -81,15 +82,21 @@ public interface Indexer { } /** - * interface representing parser in geometry indexing pipeline + * Interface representing parser in geometry indexing pipeline. */ public interface Parser { + /** + * Parse the given xContent value to an object of type {@link Parsed}. + */ Parsed parse(XContentParser parser, AbstractGeometryFieldMapper mapper) throws IOException, ParseException; - } - public interface Formatter { - Object formatGeoJson(Parsed value); - Object formatWKT(Parsed value); + /** + * Given a parsed value and a format string, formats the value into a plain Java object. + * + * Supported formats include 'geojson' and 'wkt'. The different formats are defined + * as subclasses of {@link org.elasticsearch.common.geo.GeometryFormat}. + */ + Object format(Parsed value, String format); } public abstract static class Builder, FT extends AbstractGeometryFieldType> @@ -153,9 +160,12 @@ public Builder ignoreZValue(final boolean ignoreZValue) { @Override protected Object parseSourceValue(Object value, String format) { + if (format == null) { + format = GeoJsonGeometryFormat.NAME; + } + AbstractGeometryFieldType mappedFieldType = fieldType(); Parser geometryParser = mappedFieldType.geometryParser(); - Formatter geometryFormatter = mappedFieldType.geometryFormatter(); Parsed geometry; try (XContentParser parser = new MapXContentParser(NamedXContentRegistry.EMPTY, LoggingDeprecationHandler.INSTANCE, @@ -169,15 +179,7 @@ protected Object parseSourceValue(Object value, String format) { } catch (ParseException e) { throw new RuntimeException(e); } - - if (format == null || format.equals("geojson")) { - return geometryFormatter.formatGeoJson(geometry); - } else if (format.equals("wkt")) { - return geometryFormatter.formatWKT(geometry); - } else { - throw new IllegalArgumentException("Encountered an unsupported format [" + format + "] when " + - "loading values for the [" + contentType() +"] field named [" + name() + "]"); - } + return geometryParser.format(geometry, format); } public abstract static class TypeParser implements Mapper.TypeParser { @@ -221,7 +223,6 @@ public T parse(String name, Map node, ParserContext parserContex public abstract static class AbstractGeometryFieldType extends MappedFieldType { protected Indexer geometryIndexer; protected Parser geometryParser; - protected Formatter geometryFormatter; protected QueryProcessor geometryQueryBuilder; protected AbstractGeometryFieldType(String name, boolean indexed, boolean hasDocValues, Map meta) { @@ -248,14 +249,6 @@ protected Parser geometryParser() { return geometryParser; } - public void setGeometryFormatter(Formatter geometryFormatter) { - this.geometryFormatter = geometryFormatter; - } - - protected Formatter geometryFormatter() { - return geometryFormatter; - } - public QueryProcessor geometryQueryBuilder() { return geometryQueryBuilder; } 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 31e5dace53742..65344fb250db5 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/AbstractPointGeometryFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/AbstractPointGeometryFieldMapper.java @@ -22,17 +22,16 @@ import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.common.Explicit; import org.elasticsearch.common.ParseField; -import org.elasticsearch.common.geo.GeoJson; import org.elasticsearch.common.geo.GeoPoint; +import org.elasticsearch.common.geo.GeometryFormat; +import org.elasticsearch.common.geo.GeometryParser; import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.geometry.Geometry; import org.elasticsearch.geometry.Point; -import org.elasticsearch.geometry.utils.WellKnownText; import java.io.IOException; -import java.io.UncheckedIOException; import java.text.ParseException; import java.util.ArrayList; import java.util.Iterator; @@ -185,6 +184,14 @@ protected void parsePointIgnoringMalformed(XContentParser parser, ParsedPoint po /** A parser implementation that can parse the various point formats */ public static class PointParser

implements Parser> { + /** + * Note that this parser is only used for formatting values. + */ + private final GeometryParser geometryParser; + + public PointParser() { + this.geometryParser = new GeometryParser(true, true, true); + } @Override public List

parse(XContentParser parser, AbstractGeometryFieldMapper geometryMapper) throws IOException, ParseException { @@ -244,29 +251,14 @@ public List

parse(XContentParser parser, AbstractGeometryFieldMapper geometry return points; } } - } - public static class PointFormatter

implements Formatter> { @Override - public Object formatGeoJson(List

points) { + public Object format(List

points, String format) { List result = new ArrayList<>(); - for (ParsedPoint point : points) { - try { - Geometry geometry = point.asGeometry(); - result.add(GeoJson.toXContentMap(geometry)); - } catch (IOException e) { - throw new UncheckedIOException(e); - } - } - return result; - } - - @Override - public Object formatWKT(List

points) { - List result = new ArrayList<>(); + GeometryFormat geometryFormat = geometryParser.geometryFormat(format); for (ParsedPoint point : points) { Geometry geometry = point.asGeometry(); - result.add(WellKnownText.INSTANCE.toWKT(geometry)); + result.add(geometryFormat.toObject(geometry)); } return result; } 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 49ddf862e3f32..1ac9760bb90c3 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/GeoPointFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/GeoPointFieldMapper.java @@ -72,7 +72,6 @@ public GeoPointFieldMapper build(BuilderContext context, String simpleName, Fiel GeoPointFieldType ft = new GeoPointFieldType(buildFullName(context), indexed, hasDocValues, meta); ft.setGeometryParser(new PointParser<>()); ft.setGeometryIndexer(new GeoPointIndexer(ft)); - ft.setGeometryFormatter(new PointFormatter<>()); ft.setGeometryQueryBuilder(new VectorGeoPointShapeQueryProcessor()); return new GeoPointFieldMapper(name, fieldType, ft, multiFields, ignoreMalformed, ignoreZValue, nullValue, copyTo); } 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 543f64c056fa3..bebf07fa10e70 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java @@ -71,9 +71,8 @@ private GeoShapeFieldType buildFieldType(BuilderContext context) { GeoShapeFieldType ft = new GeoShapeFieldType(buildFullName(context), indexed, hasDocValues, meta); GeometryParser geometryParser = new GeometryParser(ft.orientation.getAsBoolean(), coerce().value(), ignoreZValue().value()); - ft.setGeometryParser((parser, mapper) -> geometryParser.parse(parser)); + ft.setGeometryParser(new GeoShapeParser(geometryParser)); ft.setGeometryIndexer(new GeoShapeIndexer(orientation().value().getAsBoolean(), buildFullName(context))); - ft.setGeometryFormatter(new GeoShapeFormatter()); ft.setGeometryQueryBuilder(new VectorGeoShapeQueryProcessor()); ft.setOrientation(orientation == null ? Defaults.ORIENTATION.value() : orientation); return ft; diff --git a/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFormatter.java b/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeParser.java similarity index 58% rename from server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFormatter.java rename to server/src/main/java/org/elasticsearch/index/mapper/GeoShapeParser.java index 2b51ffe90fe03..841f6bea07d63 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFormatter.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeParser.java @@ -19,25 +19,27 @@ package org.elasticsearch.index.mapper; -import org.elasticsearch.common.geo.GeoJson; +import org.elasticsearch.common.geo.GeometryParser; +import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.geometry.Geometry; -import org.elasticsearch.geometry.utils.WellKnownText; import java.io.IOException; -import java.io.UncheckedIOException; +import java.text.ParseException; + +public class GeoShapeParser implements AbstractGeometryFieldMapper.Parser { + private final GeometryParser geometryParser; + + public GeoShapeParser(GeometryParser geometryParser) { + this.geometryParser = geometryParser; + } -public class GeoShapeFormatter implements AbstractGeometryFieldMapper.Formatter { @Override - public Object formatGeoJson(Geometry value) { - try { - return GeoJson.toXContentMap(value); - } catch (IOException e) { - throw new UncheckedIOException(e); - } + public Geometry parse(XContentParser parser, AbstractGeometryFieldMapper mapper) throws IOException, ParseException { + return geometryParser.parse(parser); } @Override - public Object formatWKT(Geometry value) { - return WellKnownText.INSTANCE.toWKT(value); + public Object format(Geometry value, String format) { + return geometryParser.geometryFormat(format).toObject(value); } } 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 06ce727526a51..1b99e30aec6cd 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapper.java @@ -31,8 +31,8 @@ import org.elasticsearch.Version; import org.elasticsearch.common.Explicit; import org.elasticsearch.common.ParseField; -import org.elasticsearch.common.geo.GeoJson; import org.elasticsearch.common.geo.GeoUtils; +import org.elasticsearch.common.geo.GeometryParser; import org.elasticsearch.common.geo.ShapesAvailability; import org.elasticsearch.common.geo.SpatialStrategy; import org.elasticsearch.common.geo.builders.ShapeBuilder; @@ -43,14 +43,14 @@ import org.elasticsearch.common.unit.DistanceUnit; import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.support.XContentMapValues; import org.elasticsearch.geometry.Geometry; -import org.elasticsearch.geometry.utils.WellKnownText; import org.elasticsearch.index.query.LegacyGeoShapeQueryProcessor; import org.locationtech.spatial4j.shape.Shape; import java.io.IOException; -import java.io.UncheckedIOException; +import java.text.ParseException; import java.util.Collections; import java.util.List; import java.util.Map; @@ -259,8 +259,7 @@ private GeoShapeFieldType buildFieldType(BuilderContext context) { setupFieldTypeDeprecatedParameters(ft); setupPrefixTrees(ft); ft.setGeometryIndexer(new LegacyGeoShapeIndexer(ft)); - ft.setGeometryParser(ShapeParser::parse); - ft.setGeometryFormatter(new LegacyGeoShapeFormatter()); + ft.setGeometryParser(new LegacyGeoShapeParser()); ft.setGeometryQueryBuilder(new LegacyGeoShapeQueryProcessor(ft)); ft.setOrientation(orientation == null ? Defaults.ORIENTATION.value() : orientation); return ft; @@ -282,20 +281,25 @@ public LegacyGeoShapeFieldMapper build(BuilderContext context) { } } - private static class LegacyGeoShapeFormatter implements Formatter> { + private static class LegacyGeoShapeParser implements Parser> { + /** + * Note that this parser is only used for formatting values. + */ + private final GeometryParser geometryParser; + + private LegacyGeoShapeParser() { + this.geometryParser = new GeometryParser(true, true, true); + } + @Override - public Object formatGeoJson(ShapeBuilder value) { - try { - Geometry geometry = value.buildGeometry(); - return GeoJson.toXContentMap(geometry); - } catch (IOException e) { - throw new UncheckedIOException(e); - } + public ShapeBuilder parse(XContentParser parser, AbstractGeometryFieldMapper mapper) throws IOException, ParseException { + return ShapeParser.parse(parser); } @Override - public String formatWKT(ShapeBuilder value) { - return WellKnownText.INSTANCE.toWKT(value.buildGeometry()); + public Object format(ShapeBuilder value, String format) { + Geometry geometry = value.buildGeometry(); + return geometryParser.geometryFormat(format).toObject(geometry); } } 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 e727588b625c4..34750b8989027 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 @@ -21,8 +21,8 @@ import org.elasticsearch.index.fielddata.IndexFieldData; import org.elasticsearch.index.mapper.AbstractShapeGeometryFieldMapper; import org.elasticsearch.index.mapper.GeoShapeFieldMapper; -import org.elasticsearch.index.mapper.GeoShapeFormatter; import org.elasticsearch.index.mapper.GeoShapeIndexer; +import org.elasticsearch.index.mapper.GeoShapeParser; import org.elasticsearch.index.mapper.LegacyGeoShapeFieldMapper; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.mapper.MapperParsingException; @@ -91,9 +91,8 @@ public GeoShapeWithDocValuesFieldMapper build(BuilderContext context) { // @todo check coerce GeometryParser geometryParser = new GeometryParser(ft.orientation().getAsBoolean(), coerce().value(), ignoreZValue().value()); - ft.setGeometryParser((parser, mapper) -> geometryParser.parse(parser)); + ft.setGeometryParser(new GeoShapeParser(geometryParser)); ft.setGeometryIndexer(new GeoShapeIndexer(orientation().value().getAsBoolean(), ft.name())); - ft.setGeometryFormatter(new GeoShapeFormatter()); ft.setGeometryQueryBuilder(new VectorGeoShapeQueryProcessor()); ft.setOrientation(orientation().value()); return new GeoShapeWithDocValuesFieldMapper(name, fieldType, ft, ignoreMalformed(context), coerce(context), 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 78af37da2a469..e27a54be2d8b2 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 @@ -17,8 +17,8 @@ import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.mapper.ParseContext; import org.elasticsearch.xpack.spatial.common.CartesianPoint; -import org.elasticsearch.xpack.spatial.index.query.ShapeQueryPointProcessor; import org.elasticsearch.xpack.spatial.index.mapper.PointFieldMapper.ParsedCartesianPoint; +import org.elasticsearch.xpack.spatial.index.query.ShapeQueryPointProcessor; import java.io.IOException; import java.util.ArrayList; @@ -47,7 +47,6 @@ public PointFieldMapper build(BuilderContext context, String simpleName, FieldTy PointFieldType ft = new PointFieldType(buildFullName(context), indexed, hasDocValues, meta); ft.setGeometryParser(new PointParser<>()); ft.setGeometryIndexer(new PointIndexer(ft)); - ft.setGeometryFormatter(new PointFormatter<>()); ft.setGeometryQueryBuilder(new ShapeQueryPointProcessor()); return new PointFieldMapper(simpleName, fieldType, ft, multiFields, ignoreMalformed, ignoreZValue(context), nullValue, copyTo); 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 902f06e169b80..27b1214d9c2d8 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 @@ -12,7 +12,7 @@ import org.elasticsearch.common.geo.builders.ShapeBuilder.Orientation; import org.elasticsearch.geometry.Geometry; import org.elasticsearch.index.mapper.AbstractShapeGeometryFieldMapper; -import org.elasticsearch.index.mapper.GeoShapeFormatter; +import org.elasticsearch.index.mapper.GeoShapeParser; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.mapper.MapperParsingException; import org.elasticsearch.index.mapper.ParseContext; @@ -54,9 +54,8 @@ public ShapeFieldMapper build(BuilderContext context) { ShapeFieldType ft = new ShapeFieldType(buildFullName(context), indexed, hasDocValues, meta); GeometryParser geometryParser = new GeometryParser(orientation().value().getAsBoolean(), coerce().value(), ignoreZValue().value()); - ft.setGeometryParser((parser, mapper) -> geometryParser.parse(parser)); + ft.setGeometryParser(new GeoShapeParser(geometryParser)); ft.setGeometryIndexer(new ShapeIndexer(ft.name())); - ft.setGeometryFormatter(new GeoShapeFormatter()); ft.setGeometryQueryBuilder(new ShapeQueryProcessor()); ft.setOrientation(orientation().value()); return new ShapeFieldMapper(name, fieldType, ft, ignoreMalformed(context), coerce(context), From 4bf9bd1fddeab8e401ba5c5252cde422dea1add1 Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Tue, 21 Jul 2020 14:46:22 -0700 Subject: [PATCH 6/9] Avoid parsing shapes if they're already in the right format. --- .../mapper/AbstractGeometryFieldMapper.java | 47 ++++++++++++------- .../AbstractPointGeometryFieldMapper.java | 2 +- .../index/mapper/GeoShapeParser.java | 31 +++++++++++- .../mapper/LegacyGeoShapeFieldMapper.java | 2 +- 4 files changed, 61 insertions(+), 21 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 d608a1f734c13..c90a8ecaf7bb1 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java @@ -84,11 +84,12 @@ public interface Indexer { /** * Interface representing parser in geometry indexing pipeline. */ - public interface Parser { + public static abstract class Parser { /** - * Parse the given xContent value to an object of type {@link Parsed}. + * Parse the given xContent value to an object of type {@link Parsed}. The value can be + * in any supported format. */ - Parsed parse(XContentParser parser, AbstractGeometryFieldMapper mapper) throws IOException, ParseException; + public abstract Parsed parse(XContentParser parser, AbstractGeometryFieldMapper mapper) throws IOException, ParseException; /** * Given a parsed value and a format string, formats the value into a plain Java object. @@ -96,7 +97,30 @@ public interface Parser { * Supported formats include 'geojson' and 'wkt'. The different formats are defined * as subclasses of {@link org.elasticsearch.common.geo.GeometryFormat}. */ - Object format(Parsed value, String format); + public abstract Object format(Parsed value, String format); + + /** + * Parses the given value, then formats it according to the 'format' string. + * + * By default, this method simply parses the value using {@link Parser#parse}, then formats + * it with {@link Parser#format}. However some {@link Parser} implementations override this + * as they can avoid parsing the value if it is already in the right format. + */ + public Object parseAndFormatObject(Object value, AbstractGeometryFieldMapper mapper, 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, mapper); + } catch (IOException e) { + throw new UncheckedIOException(e); + } catch (ParseException e) { + throw new RuntimeException(e); + } + return format(geometry, format); + } } public abstract static class Builder, FT extends AbstractGeometryFieldType> @@ -166,20 +190,7 @@ protected Object parseSourceValue(Object value, String format) { AbstractGeometryFieldType mappedFieldType = fieldType(); Parser geometryParser = mappedFieldType.geometryParser(); - - 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 = geometryParser.parse(parser, this); - } catch (IOException e) { - throw new UncheckedIOException(e); - } catch (ParseException e) { - throw new RuntimeException(e); - } - return geometryParser.format(geometry, format); + return geometryParser.parseAndFormatObject(value, this, format); } public abstract static class TypeParser implements Mapper.TypeParser { 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 65344fb250db5..4ffb7b7cf764f 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/AbstractPointGeometryFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/AbstractPointGeometryFieldMapper.java @@ -183,7 +183,7 @@ protected void parsePointIgnoringMalformed(XContentParser parser, ParsedPoint po } /** A parser implementation that can parse the various point formats */ - public static class PointParser

implements Parser> { + public static class PointParser

extends Parser> { /** * Note that this parser is only used for formatting values. */ diff --git a/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeParser.java b/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeParser.java index 841f6bea07d63..bba8f4a7ad08c 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeParser.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeParser.java @@ -19,14 +19,21 @@ package org.elasticsearch.index.mapper; +import org.elasticsearch.common.geo.GeometryFormat; import org.elasticsearch.common.geo.GeometryParser; +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.geometry.Geometry; import java.io.IOException; +import java.io.UncheckedIOException; import java.text.ParseException; +import java.util.Collections; -public class GeoShapeParser implements AbstractGeometryFieldMapper.Parser { +public class GeoShapeParser extends AbstractGeometryFieldMapper.Parser { private final GeometryParser geometryParser; public GeoShapeParser(GeometryParser geometryParser) { @@ -42,4 +49,26 @@ public Geometry parse(XContentParser parser, AbstractGeometryFieldMapper mapper) public Object format(Geometry value, String format) { return geometryParser.geometryFormat(format).toObject(value); } + + @Override + public Object parseAndFormatObject(Object value, AbstractGeometryFieldMapper mapper, String format) { + 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 + + GeometryFormat geometryFormat = geometryParser.geometryFormat(parser); + if (geometryFormat.name().equals(format)) { + return value; + } + + Geometry geometry = geometryFormat.fromXContent(parser); + return format(geometry, format); + } catch (IOException e) { + throw new UncheckedIOException(e); + } catch (ParseException e) { + throw new RuntimeException(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 index 1b99e30aec6cd..87c7dfed8ca2f 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapper.java @@ -281,7 +281,7 @@ public LegacyGeoShapeFieldMapper build(BuilderContext context) { } } - private static class LegacyGeoShapeParser implements Parser> { + private static class LegacyGeoShapeParser extends Parser> { /** * Note that this parser is only used for formatting values. */ From d3d62820221c7c0c65f2aad2eca705de26d50a91 Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Tue, 21 Jul 2020 15:24:04 -0700 Subject: [PATCH 7/9] Fix checkstyle violation. --- .../elasticsearch/index/mapper/AbstractGeometryFieldMapper.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 c90a8ecaf7bb1..24d0a11ba1729 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java @@ -84,7 +84,7 @@ public interface Indexer { /** * Interface representing parser in geometry indexing pipeline. */ - public static abstract class Parser { + public abstract static class Parser { /** * Parse the given xContent value to an object of type {@link Parsed}. The value can be * in any supported format. From 1b7dad391666afceaf9e56daf54b1e788e42ca10 Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Wed, 22 Jul 2020 13:41:25 -0700 Subject: [PATCH 8/9] Rename GeometryFormat#toObject -> toXContentAsObject. --- .../org/elasticsearch/common/geo/GeoJsonGeometryFormat.java | 2 +- .../main/java/org/elasticsearch/common/geo/GeometryFormat.java | 2 +- .../java/org/elasticsearch/common/geo/WKTGeometryFormat.java | 2 +- .../index/mapper/AbstractPointGeometryFieldMapper.java | 2 +- .../java/org/elasticsearch/index/mapper/GeoShapeParser.java | 2 +- .../elasticsearch/index/mapper/LegacyGeoShapeFieldMapper.java | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/geo/GeoJsonGeometryFormat.java b/server/src/main/java/org/elasticsearch/common/geo/GeoJsonGeometryFormat.java index 7a88f4936f36d..762d65092492f 100644 --- a/server/src/main/java/org/elasticsearch/common/geo/GeoJsonGeometryFormat.java +++ b/server/src/main/java/org/elasticsearch/common/geo/GeoJsonGeometryFormat.java @@ -66,7 +66,7 @@ public XContentBuilder toXContent(Geometry geometry, XContentBuilder builder, To } @Override - public Map toObject(Geometry geometry) { + public Map toXContentAsObject(Geometry geometry) { try { XContentBuilder builder = XContentFactory.jsonBuilder(); GeoJson.toXContent(geometry, builder, ToXContent.EMPTY_PARAMS); diff --git a/server/src/main/java/org/elasticsearch/common/geo/GeometryFormat.java b/server/src/main/java/org/elasticsearch/common/geo/GeometryFormat.java index 5c77491e0fc8e..4ce53eaa8f197 100644 --- a/server/src/main/java/org/elasticsearch/common/geo/GeometryFormat.java +++ b/server/src/main/java/org/elasticsearch/common/geo/GeometryFormat.java @@ -51,5 +51,5 @@ public interface GeometryFormat { * * For example, the GeoJson format returns the geometry as a map, while WKT returns a string. */ - Object toObject(ParsedFormat geometry); + Object toXContentAsObject(ParsedFormat geometry); } diff --git a/server/src/main/java/org/elasticsearch/common/geo/WKTGeometryFormat.java b/server/src/main/java/org/elasticsearch/common/geo/WKTGeometryFormat.java index c13d93b025521..dc0671c51e1b8 100644 --- a/server/src/main/java/org/elasticsearch/common/geo/WKTGeometryFormat.java +++ b/server/src/main/java/org/elasticsearch/common/geo/WKTGeometryFormat.java @@ -60,7 +60,7 @@ public XContentBuilder toXContent(Geometry geometry, XContentBuilder builder, To } @Override - public String toObject(Geometry geometry) { + public String toXContentAsObject(Geometry geometry) { return wellKnownTextParser.toWKT(geometry); } } 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 4ffb7b7cf764f..5160d97faf21a 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/AbstractPointGeometryFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/AbstractPointGeometryFieldMapper.java @@ -258,7 +258,7 @@ public Object format(List

points, String format) { GeometryFormat geometryFormat = geometryParser.geometryFormat(format); for (ParsedPoint point : points) { Geometry geometry = point.asGeometry(); - result.add(geometryFormat.toObject(geometry)); + result.add(geometryFormat.toXContentAsObject(geometry)); } return result; } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeParser.java b/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeParser.java index bba8f4a7ad08c..9a82646b95dd6 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeParser.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeParser.java @@ -47,7 +47,7 @@ public Geometry parse(XContentParser parser, AbstractGeometryFieldMapper mapper) @Override public Object format(Geometry value, String format) { - return geometryParser.geometryFormat(format).toObject(value); + return geometryParser.geometryFormat(format).toXContentAsObject(value); } @Override 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 87c7dfed8ca2f..564302a973240 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapper.java @@ -299,7 +299,7 @@ private LegacyGeoShapeParser() { @Override public Object format(ShapeBuilder value, String format) { Geometry geometry = value.buildGeometry(); - return geometryParser.geometryFormat(format).toObject(geometry); + return geometryParser.geometryFormat(format).toXContentAsObject(geometry); } } From 9c155cf78e24b2b4f41c13ac0d363d9738fcc63d Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Wed, 22 Jul 2020 13:42:04 -0700 Subject: [PATCH 9/9] Always return Object from toXContentAsObject. --- .../org/elasticsearch/common/geo/GeoJsonGeometryFormat.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/geo/GeoJsonGeometryFormat.java b/server/src/main/java/org/elasticsearch/common/geo/GeoJsonGeometryFormat.java index 762d65092492f..fa6614d3b7419 100644 --- a/server/src/main/java/org/elasticsearch/common/geo/GeoJsonGeometryFormat.java +++ b/server/src/main/java/org/elasticsearch/common/geo/GeoJsonGeometryFormat.java @@ -32,7 +32,6 @@ import java.io.IOException; import java.io.UncheckedIOException; -import java.util.Map; public class GeoJsonGeometryFormat implements GeometryFormat { public static final String NAME = "geojson"; @@ -66,7 +65,7 @@ public XContentBuilder toXContent(Geometry geometry, XContentBuilder builder, To } @Override - public Map toXContentAsObject(Geometry geometry) { + public Object toXContentAsObject(Geometry geometry) { try { XContentBuilder builder = XContentFactory.jsonBuilder(); GeoJson.toXContent(geometry, builder, ToXContent.EMPTY_PARAMS);