diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/support/MapXContentParser.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/support/MapXContentParser.java index 1483a80d6e0b1..b87fe9c5c9b86 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/support/MapXContentParser.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/support/MapXContentParser.java @@ -11,12 +11,14 @@ import org.elasticsearch.common.xcontent.DeprecationHandler; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.XContentLocation; +import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; import java.io.IOException; import java.math.BigDecimal; import java.math.BigInteger; import java.nio.CharBuffer; +import java.util.Collections; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -30,6 +32,17 @@ public class MapXContentParser extends AbstractXContentParser { private TokenIterator iterator; private boolean closed; + public static XContentParser wrapObject(Object sourceMap) throws IOException { + XContentParser parser = new MapXContentParser( + NamedXContentRegistry.EMPTY, + DeprecationHandler.IGNORE_DEPRECATIONS, + Collections.singletonMap("dummy_field", sourceMap), XContentType.JSON); + parser.nextToken(); // start object + parser.nextToken(); // field name + parser.nextToken(); // field value + return parser; + } + public MapXContentParser(NamedXContentRegistry xContentRegistry, DeprecationHandler deprecationHandler, Map map, XContentType xContentType) { super(xContentRegistry, deprecationHandler); diff --git a/server/src/internalClusterTest/java/org/elasticsearch/index/mapper/ExternalValuesMapperIntegrationIT.java b/server/src/internalClusterTest/java/org/elasticsearch/index/mapper/ExternalValuesMapperIntegrationIT.java index 3805f01657357..6d17400b6eae5 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/index/mapper/ExternalValuesMapperIntegrationIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/index/mapper/ExternalValuesMapperIntegrationIT.java @@ -9,14 +9,11 @@ package org.elasticsearch.index.mapper; import org.elasticsearch.action.search.SearchResponse; -import org.elasticsearch.common.geo.ShapeRelation; -import org.elasticsearch.common.geo.builders.EnvelopeBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.search.fetch.subphase.highlight.HighlightBuilder; import org.elasticsearch.test.ESIntegTestCase; -import org.locationtech.jts.geom.Coordinate; import java.util.Collection; import java.util.Collections; @@ -99,19 +96,6 @@ public void testExternalValues() throws Exception { assertThat(response.getHits().getTotalHits().value, equalTo((long) 1)); - response = client().prepareSearch("test-idx") - .setPostFilter(QueryBuilders.geoDistanceQuery("field.point").point(42.0, 51.0).distance("1km")) - .execute().actionGet(); - - assertThat(response.getHits().getTotalHits().value, equalTo((long) 1)); - - response = client().prepareSearch("test-idx") - .setPostFilter(QueryBuilders.geoShapeQuery("field.shape", - new EnvelopeBuilder(new Coordinate(-101, 46), new Coordinate(-99, 44))).relation(ShapeRelation.WITHIN)) - .execute().actionGet(); - - assertThat(response.getHits().getTotalHits().value, equalTo((long) 1)); - response = client().prepareSearch("test-idx") .setPostFilter(QueryBuilders.termQuery("field.field", "foo")) .execute().actionGet(); 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 1f2015eec88b3..d30db2e06f97f 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java @@ -7,31 +7,29 @@ */ package org.elasticsearch.index.mapper; -import org.apache.lucene.index.IndexableField; import org.apache.lucene.search.Query; +import org.apache.lucene.util.SetOnce; +import org.elasticsearch.common.CheckedConsumer; import org.elasticsearch.common.Explicit; import org.elasticsearch.common.geo.GeoJsonGeometryFormat; -import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; -import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.common.xcontent.support.MapXContentParser; import org.elasticsearch.index.analysis.NamedAnalyzer; import org.elasticsearch.index.query.SearchExecutionContext; import java.io.IOException; import java.io.UncheckedIOException; -import java.text.ParseException; import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.function.Consumer; import java.util.function.Function; /** * Base field mapper class for all spatial field types */ -public abstract class AbstractGeometryFieldMapper extends FieldMapper { +public abstract class AbstractGeometryFieldMapper extends FieldMapper { public static Parameter> ignoreMalformedParam(Function> initializer, boolean ignoreMalformedByDefault) { @@ -42,24 +40,18 @@ public static Parameter> ignoreZValueParam(Function { - Processed prepareForIndexing(Parsed geometry); - Class processedClass(); - List indexShape(ParseContext context, Processed shape); - } - /** * Interface representing parser in geometry indexing pipeline. */ - public abstract static class Parser { + public abstract static class Parser { /** - * Parse the given xContent value to an object of type {@link Parsed}. The value can be + * Parse the given xContent value to one or more objects of type {@link T}. The value can be * in any supported format. */ - public abstract Parsed parse(XContentParser parser) throws IOException, ParseException; + public abstract void parse( + XContentParser parser, + CheckedConsumer consumer, + Consumer onMalformed) throws IOException; /** * Given a parsed value and a format string, formats the value into a plain Java object. @@ -67,27 +59,14 @@ public abstract static class Parser { * Supported formats include 'geojson' and 'wkt'. The different formats are defined * as subclasses of {@link org.elasticsearch.common.geo.GeometryFormat}. */ - public abstract Object format(Parsed value, String format); + public abstract Object format(T value, String format); - /** - * Parses the given value, then formats it according to the 'format' string. - * - * Used by value fetchers to validate and format geo objects - */ - public Object parseAndFormatObject(Object value, String format) { - Parsed geometry; - try (XContentParser parser = new MapXContentParser(NamedXContentRegistry.EMPTY, LoggingDeprecationHandler.INSTANCE, - Collections.singletonMap("dummy_field", value), XContentType.JSON)) { - parser.nextToken(); // start object - parser.nextToken(); // field name - parser.nextToken(); // field value - geometry = parse(parser); + private void fetchFromSource(Object sourceMap, Consumer consumer, String format) { + try (XContentParser parser = MapXContentParser.wrapObject(sourceMap)) { + parse(parser, v -> consumer.accept(format(v, format)), e -> {}); /* ignore malformed */ } catch (IOException e) { throw new UncheckedIOException(e); - } catch (ParseException e) { - throw new RuntimeException(e); } - return format(geometry, format); } } @@ -113,19 +92,22 @@ public final Query termQuery(Object value, SearchExecutionContext context) { public final ValueFetcher valueFetcher(SearchExecutionContext context, String format) { String geoFormat = format != null ? format : GeoJsonGeometryFormat.NAME; - Function valueParser = value -> geometryParser.parseAndFormatObject(value, geoFormat); if (parsesArrayValue) { return new ArraySourceValueFetcher(name(), context) { @Override protected Object parseSourceValue(Object value) { - return valueParser.apply(value); + List values = new ArrayList<>(); + geometryParser.fetchFromSource(value, values::add, geoFormat); + return values; } }; } else { return new SourceValueFetcher(name(), context) { @Override protected Object parseSourceValue(Object value) { - return valueParser.apply(value); + SetOnce holder = new SetOnce<>(); + geometryParser.fetchFromSource(value, holder::set, geoFormat); + return holder.get(); } }; } @@ -134,26 +116,24 @@ protected Object parseSourceValue(Object value) { private final Explicit ignoreMalformed; private final Explicit ignoreZValue; - private final Indexer indexer; - private final Parser parser; + private final Parser parser; protected AbstractGeometryFieldMapper(String simpleName, MappedFieldType mappedFieldType, Map indexAnalyzers, Explicit ignoreMalformed, Explicit ignoreZValue, MultiFields multiFields, CopyTo copyTo, - Indexer indexer, Parser parser) { + Parser parser) { super(simpleName, mappedFieldType, indexAnalyzers, multiFields, copyTo, false, null); this.ignoreMalformed = ignoreMalformed; this.ignoreZValue = ignoreZValue; - this.indexer = indexer; this.parser = parser; } protected AbstractGeometryFieldMapper(String simpleName, MappedFieldType mappedFieldType, Explicit ignoreMalformed, Explicit ignoreZValue, MultiFields multiFields, CopyTo copyTo, - Indexer indexer, Parser parser) { - this(simpleName, mappedFieldType, Collections.emptyMap(), ignoreMalformed, ignoreZValue, multiFields, copyTo, indexer, parser); + Parser parser) { + this(simpleName, mappedFieldType, Collections.emptyMap(), ignoreMalformed, ignoreZValue, multiFields, copyTo, parser); } @Override @@ -166,60 +146,25 @@ protected void parseCreateField(ParseContext context) throws IOException { throw new UnsupportedOperationException("Parsing is implemented in parse(), this method should NEVER be called"); } - protected abstract void addStoredFields(ParseContext context, Processed geometry); - protected abstract void addDocValuesFields(String name, Processed geometry, List fields, ParseContext context); - protected abstract void addMultiFields(ParseContext context, Processed geometry) throws IOException; + /** + * Build an index document using a parsed geometry + * @param context the ParseContext holding the document + * @param geometry the parsed geometry object + */ + protected abstract void index(ParseContext context, T geometry) throws IOException; - /** parsing logic for geometry indexing */ @Override - public void parse(ParseContext context) throws IOException { - MappedFieldType mappedFieldType = fieldType(); - - try { - Processed shape = context.parseExternalValue(indexer.processedClass()); - if (shape == null) { - Parsed geometry = parser.parse(context.parser()); - if (geometry == null) { - return; - } - shape = indexer.prepareForIndexing(geometry); - } - - List fields = new ArrayList<>(); - if (mappedFieldType.isSearchable() || mappedFieldType.hasDocValues()) { - fields.addAll(indexer.indexShape(context, shape)); - } - - // indexed: - List indexedFields = new ArrayList<>(); - if (mappedFieldType.isSearchable()) { - indexedFields.addAll(fields); - } - // stored: - if (fieldType().isStored()) { - addStoredFields(context, shape); - } - // docValues: - if (fieldType().hasDocValues()) { - addDocValuesFields(mappedFieldType.name(), shape, fields, context); - } else if (fieldType().isStored() || fieldType().isSearchable()) { - createFieldNamesField(context); - } - - // add the indexed fields to the doc: - for (IndexableField field : indexedFields) { - context.doc().add(field); - } - - // add multifields (e.g., used for completion suggester) - addMultiFields(context, shape); - } catch (Exception e) { - if (ignoreMalformed.value() == false) { - throw new MapperParsingException("failed to parse field [{}] of type [{}]", e, fieldType().name(), - fieldType().typeName()); - } - context.addIgnoredField(mappedFieldType.name()); - } + public final void parse(ParseContext context) throws IOException { + parser.parse(context.parser(), v -> index(context, v), e -> { + if (ignoreMalformed()) { + context.addIgnoredField(fieldType().name()); + } else { + throw new MapperParsingException( + "Failed to parse field [" + fieldType().name() + "] of type [" + contentType() + "]", + e + ); + } + }); } public boolean ignoreMalformed() { 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 5259f5992aaeb..8fc2f8752bc89 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/AbstractPointGeometryFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/AbstractPointGeometryFieldMapper.java @@ -9,6 +9,7 @@ import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.common.CheckedBiFunction; +import org.elasticsearch.common.CheckedConsumer; import org.elasticsearch.common.Explicit; import org.elasticsearch.common.TriFunction; import org.elasticsearch.common.geo.GeoPoint; @@ -20,15 +21,12 @@ import org.elasticsearch.index.mapper.Mapper.TypeParser.ParserContext; import java.io.IOException; -import java.text.ParseException; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; +import java.util.function.Consumer; import java.util.function.Function; import java.util.function.Supplier; /** Base class for for spatial fields that only support indexing points */ -public abstract class AbstractPointGeometryFieldMapper extends AbstractGeometryFieldMapper { +public abstract class AbstractPointGeometryFieldMapper extends AbstractGeometryFieldMapper { public static Parameter nullValueParam(Function initializer, TriFunction parser, @@ -41,8 +39,8 @@ public static Parameter nullValueParam(Function ignoreMalformed, Explicit ignoreZValue, ParsedPoint nullValue, CopyTo copyTo, - Indexer indexer, Parser parser) { - super(simpleName, mappedFieldType, ignoreMalformed, ignoreZValue, multiFields, copyTo, indexer, parser); + Parser parser) { + super(simpleName, mappedFieldType, ignoreMalformed, ignoreZValue, multiFields, copyTo, parser); this.nullValue = nullValue; } @@ -67,7 +65,7 @@ default boolean isNormalizable(double coord) { } /** A parser implementation that can parse the various point formats */ - public static class PointParser

extends Parser> { + public static class PointParser

extends Parser

{ /** * Note that this parser is only used for formatting values. */ @@ -88,7 +86,7 @@ public PointParser(String field, this.field = field; this.pointSupplier = pointSupplier; this.objectParser = objectParser; - this.nullValue = nullValue; + this.nullValue = nullValue == null ? null : process(nullValue); this.ignoreZValue = ignoreZValue; this.ignoreMalformed = ignoreMalformed; this.geometryParser = new GeometryParser(true, true, true); @@ -104,12 +102,14 @@ private P process(P in) { } @Override - public List

parse(XContentParser parser) throws IOException, ParseException { - + public void parse( + XContentParser parser, + CheckedConsumer consumer, + Consumer onMalformed + ) throws IOException { if (parser.currentToken() == XContentParser.Token.START_ARRAY) { XContentParser.Token token = parser.nextToken(); P point = pointSupplier.get(); - ArrayList

points = new ArrayList<>(); if (token == XContentParser.Token.VALUE_NUMBER) { double x = parser.doubleValue(); parser.nextToken(); @@ -122,35 +122,41 @@ public List

parse(XContentParser parser) throws IOException, ParseException { } point.resetCoords(x, y); - points.add(process(point)); + consumer.accept(process(point)); } else { while (token != XContentParser.Token.END_ARRAY) { - points.add(process(objectParser.apply(parser, point))); + parseAndConsumeFromObject(parser, point, consumer, onMalformed); point = pointSupplier.get(); token = parser.nextToken(); } } - return points; } else if (parser.currentToken() == XContentParser.Token.VALUE_NULL) { - if (nullValue == null) { - return null; - } else { - return Collections.singletonList(nullValue); + if (nullValue != null) { + consumer.accept(nullValue); } } else { - return Collections.singletonList(process(objectParser.apply(parser, pointSupplier.get()))); + parseAndConsumeFromObject(parser, pointSupplier.get(), consumer, onMalformed); + } + } + + private void parseAndConsumeFromObject( + XContentParser parser, + P point, + CheckedConsumer consumer, + Consumer onMalformed + ) { + try { + point = objectParser.apply(parser, point); + consumer.accept(process(point)); + } catch (Exception e) { + onMalformed.accept(e); } } @Override - public Object format(List

points, String format) { - List result = new ArrayList<>(); + public Object format(P point, String format) { GeometryFormat geometryFormat = geometryParser.geometryFormat(format); - for (ParsedPoint point : points) { - Geometry geometry = point.asGeometry(); - result.add(geometryFormat.toXContentAsObject(geometry)); - } - return result; + return geometryFormat.toXContentAsObject(point.asGeometry()); } } } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/AbstractShapeGeometryFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/AbstractShapeGeometryFieldMapper.java index 1413e3df7564d..57dfb6189ccf3 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/AbstractShapeGeometryFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/AbstractShapeGeometryFieldMapper.java @@ -19,7 +19,7 @@ /** * Base class for {@link GeoShapeFieldMapper} and {@link LegacyGeoShapeFieldMapper} */ -public abstract class AbstractShapeGeometryFieldMapper extends AbstractGeometryFieldMapper { +public abstract class AbstractShapeGeometryFieldMapper extends AbstractGeometryFieldMapper { public static Parameter> coerceParam(Function> initializer, boolean coerceByDefault) { @@ -56,8 +56,8 @@ protected AbstractShapeGeometryFieldMapper(String simpleName, MappedFieldType ma Explicit ignoreMalformed, Explicit coerce, Explicit ignoreZValue, Explicit orientation, MultiFields multiFields, CopyTo copyTo, - Indexer indexer, Parser parser) { - super(simpleName, mappedFieldType, indexAnalyzers, ignoreMalformed, ignoreZValue, multiFields, copyTo, indexer, parser); + Parser parser) { + super(simpleName, mappedFieldType, indexAnalyzers, ignoreMalformed, ignoreZValue, multiFields, copyTo, parser); this.coerce = coerce; this.orientation = orientation; } @@ -66,9 +66,9 @@ protected AbstractShapeGeometryFieldMapper(String simpleName, MappedFieldType ma Explicit ignoreMalformed, Explicit coerce, Explicit ignoreZValue, Explicit orientation, MultiFields multiFields, CopyTo copyTo, - Indexer indexer, Parser parser) { + Parser parser) { this(simpleName, mappedFieldType, Collections.emptyMap(), - ignoreMalformed, coerce, ignoreZValue, orientation, multiFields, copyTo, indexer, parser); + ignoreMalformed, coerce, ignoreZValue, orientation, multiFields, copyTo, parser); } @Override 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 f8c9b84c0d8a0..0f17fbb77d522 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/GeoPointFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/GeoPointFieldMapper.java @@ -11,7 +11,6 @@ import org.apache.lucene.document.LatLonPoint; import org.apache.lucene.document.StoredField; import org.apache.lucene.geo.LatLonGeometry; -import org.apache.lucene.index.IndexableField; import org.apache.lucene.search.IndexOrDocValuesQuery; import org.apache.lucene.search.MatchNoDocsQuery; import org.apache.lucene.search.Query; @@ -32,7 +31,6 @@ import org.elasticsearch.search.lookup.SearchLookup; import java.io.IOException; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -44,7 +42,7 @@ * * Uses lucene 6 LatLonPoint encoding */ -public class GeoPointFieldMapper extends AbstractPointGeometryFieldMapper, List> { +public class GeoPointFieldMapper extends AbstractPointGeometryFieldMapper { public static final String CONTENT_TYPE = "geo_point"; @@ -102,14 +100,20 @@ private static ParsedGeoPoint parseNullValue(Object nullValue, boolean ignoreZVa @Override public FieldMapper build(ContentPath contentPath) { - Parser> geoParser = new PointParser<>(name, ParsedGeoPoint::new, (parser, point) -> { - GeoUtils.parseGeoPoint(parser, point, ignoreZValue.get().value()); - return point; - }, (ParsedGeoPoint) nullValue.get(), ignoreZValue.get().value(), ignoreMalformed.get().value()); + Parser geoParser = new PointParser<>( + name, + ParsedGeoPoint::new, + (parser, point) -> { + GeoUtils.parseGeoPoint(parser, point, ignoreZValue.get().value()); + return point; + }, + (ParsedGeoPoint) nullValue.get(), + ignoreZValue.get().value(), + ignoreMalformed.get().value()); GeoPointFieldType ft = new GeoPointFieldType(buildFullName(contentPath), indexed.get(), stored.get(), hasDocValues.get(), geoParser, meta.get()); return new GeoPointFieldMapper(name, ft, multiFieldsBuilder.build(this, contentPath), - copyTo.build(), new GeoPointIndexer(ft), geoParser, this); + copyTo.build(), geoParser, this); } } @@ -120,12 +124,11 @@ public FieldMapper build(ContentPath contentPath) { public GeoPointFieldMapper(String simpleName, MappedFieldType mappedFieldType, MultiFields multiFields, CopyTo copyTo, - Indexer, List> indexer, - Parser> parser, + Parser parser, Builder builder) { super(simpleName, mappedFieldType, multiFields, builder.ignoreMalformed.get(), builder.ignoreZValue.get(), builder.nullValue.get(), - copyTo, indexer, parser); + copyTo, parser); this.builder = builder; } @@ -135,39 +138,20 @@ public FieldMapper.Builder getMergeBuilder() { } @Override - protected void addStoredFields(ParseContext context, List points) { - for (GeoPoint point : points) { - context.doc().add(new StoredField(fieldType().name(), point.toString())); + protected void index(ParseContext context, ParsedGeoPoint geometry) throws IOException { + if (fieldType().isSearchable()) { + context.doc().add(new LatLonPoint(fieldType().name(), geometry.lat(), geometry.lon())); } - } - - @Override - protected void addMultiFields(ParseContext context, List points) throws IOException { - // @todo phase out geohash (which is currently used in the CompletionSuggester) - if (points.isEmpty()) { - return; + if (fieldType().hasDocValues()) { + context.doc().add(new LatLonDocValuesField(fieldType().name(), geometry.lat(), geometry.lon())); + } else if (fieldType().isStored() || fieldType().isSearchable()) { + createFieldNamesField(context); } - - StringBuilder s = new StringBuilder(); - if (points.size() > 1) { - s.append('['); - } - s.append(points.get(0).geohash()); - for (int i = 1; i < points.size(); ++i) { - s.append(','); - s.append(points.get(i).geohash()); - } - if (points.size() > 1) { - s.append(']'); - } - multiFields.parse(this, context.createExternalValueContext(s)); - } - - @Override - protected void addDocValuesFields(String name, List points, List fields, ParseContext context) { - for (GeoPoint point : points) { - context.doc().add(new LatLonDocValuesField(fieldType().name(), point.lat(), point.lon())); + if (fieldType().isStored()) { + context.doc().add(new StoredField(fieldType().name(), geometry.toString())); } + // TODO phase out geohash (which is currently used in the CompletionSuggester) + multiFields.parse(this, context.createExternalValueContext(geometry.geohash())); } @Override @@ -286,35 +270,4 @@ public int hashCode() { return super.hashCode(); } } - - protected static class GeoPointIndexer implements Indexer, List> { - - protected final GeoPointFieldType fieldType; - - GeoPointIndexer(GeoPointFieldType fieldType) { - this.fieldType = fieldType; - } - - @Override - public List prepareForIndexing(List geoPoints) { - if (geoPoints == null || geoPoints.isEmpty()) { - return Collections.emptyList(); - } - return geoPoints; - } - - @Override - public Class> processedClass() { - return (Class>)(Object)List.class; - } - - @Override - public List indexShape(ParseContext context, List points) { - ArrayList fields = new ArrayList<>(points.size()); - for (GeoPoint point : points) { - fields.add(new LatLonPoint(fieldType.name(), point.lat(), point.lon())); - } - return fields; - } - } } 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 210c4f85364ea..6f714b3a9ddf5 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java @@ -9,7 +9,6 @@ import org.apache.lucene.document.LatLonShape; import org.apache.lucene.geo.LatLonGeometry; -import org.apache.lucene.index.IndexableField; import org.apache.lucene.search.MatchNoDocsQuery; import org.apache.lucene.search.Query; import org.elasticsearch.Version; @@ -19,9 +18,10 @@ import org.elasticsearch.common.geo.ShapeRelation; import org.elasticsearch.common.geo.builders.ShapeBuilder.Orientation; import org.elasticsearch.geometry.Geometry; -import org.elasticsearch.index.query.SearchExecutionContext; import org.elasticsearch.index.query.QueryShardException; +import org.elasticsearch.index.query.SearchExecutionContext; +import java.io.IOException; import java.util.Arrays; import java.util.List; import java.util.Map; @@ -46,7 +46,7 @@ *

* "field" : "POLYGON ((100.0 0.0, 101.0 0.0, 101.0 1.0, 100.0 1.0, 100.0 0.0)) */ -public class GeoShapeFieldMapper extends AbstractShapeGeometryFieldMapper { +public class GeoShapeFieldMapper extends AbstractShapeGeometryFieldMapper { public static final String CONTENT_TYPE = "geo_shape"; @@ -146,10 +146,12 @@ public Query geoShapeQuery(Geometry shape, String fieldName, ShapeRelation relat }; private final Builder builder; + private final GeoShapeIndexer indexer; public GeoShapeFieldMapper(String simpleName, MappedFieldType mappedFieldType, MultiFields multiFields, CopyTo copyTo, - Indexer indexer, Parser parser, Builder builder) { + GeoShapeIndexer indexer, + Parser parser, Builder builder) { super(simpleName, mappedFieldType, builder.ignoreMalformed.get(), @@ -158,9 +160,9 @@ public GeoShapeFieldMapper(String simpleName, MappedFieldType mappedFieldType, builder.orientation.get(), multiFields, copyTo, - indexer, parser); this.builder = builder; + this.indexer = indexer; } @Override @@ -184,19 +186,9 @@ protected void checkIncomingMergeType(FieldMapper mergeWith) { } @Override - protected void addStoredFields(ParseContext context, Geometry geometry) { - // noop: we currently do not store geo_shapes - // @todo store as geojson string? - } - - @Override - protected void addDocValuesFields(String name, Geometry geometry, List fields, ParseContext context) { - // we will throw a mapping exception before we get here - } - - @Override - protected void addMultiFields(ParseContext context, Geometry geometry) { - // noop (completion suggester currently not compatible with geo_shape) + protected void index(ParseContext context, Geometry geometry) throws IOException { + context.doc().addAll(indexer.indexShape(geometry)); + createFieldNamesField(context); } @Override diff --git a/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeIndexer.java b/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeIndexer.java index fa38cf7d2e8d2..5cd29ecc3e865 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeIndexer.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeIndexer.java @@ -14,8 +14,8 @@ import org.apache.lucene.index.IndexableField; import org.elasticsearch.common.geo.GeoLineDecomposer; import org.elasticsearch.common.geo.GeoPolygonDecomposer; -import org.elasticsearch.common.geo.GeoShapeUtils; import org.elasticsearch.common.geo.GeoShapeType; +import org.elasticsearch.common.geo.GeoShapeUtils; import org.elasticsearch.common.geo.GeoUtils; import org.elasticsearch.geometry.Circle; import org.elasticsearch.geometry.Geometry; @@ -32,6 +32,7 @@ import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; import static org.elasticsearch.common.geo.GeoUtils.normalizePoint; @@ -39,7 +40,7 @@ /** * Utility class that converts geometries into Lucene-compatible form for indexing in a geo_shape field. */ -public class GeoShapeIndexer implements AbstractGeometryFieldMapper.Indexer { +public class GeoShapeIndexer { private final boolean orientation; private final String name; @@ -166,14 +167,12 @@ public Geometry visit(Rectangle rectangle) { }); } - @Override - public Class processedClass() { - return Geometry.class; - } - - @Override - public List indexShape(ParseContext context, Geometry shape) { + public List indexShape(Geometry shape) { LuceneGeometryIndexer visitor = new LuceneGeometryIndexer(name); + shape = prepareForIndexing(shape); + if (shape == null) { + return Collections.emptyList(); + } shape.visit(visitor); return visitor.fields(); } 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 ea923b70c4b03..0ee9137fccbe0 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeParser.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeParser.java @@ -8,12 +8,15 @@ package org.elasticsearch.index.mapper; +import org.elasticsearch.ElasticsearchParseException; +import org.elasticsearch.common.CheckedConsumer; import org.elasticsearch.common.geo.GeometryParser; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.geometry.Geometry; import java.io.IOException; import java.text.ParseException; +import java.util.function.Consumer; public class GeoShapeParser extends AbstractGeometryFieldMapper.Parser { private final GeometryParser geometryParser; @@ -23,8 +26,16 @@ public GeoShapeParser(GeometryParser geometryParser) { } @Override - public Geometry parse(XContentParser parser) throws IOException, ParseException { - return geometryParser.parse(parser); + public void parse( + XContentParser parser, + CheckedConsumer consumer, + Consumer onMalformed + ) throws IOException { + try { + consumer.accept(geometryParser.parse(parser)); + } catch (ParseException | ElasticsearchParseException | IllegalArgumentException e) { + onMalformed.accept(e); + } } @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 8ec74e9b61065..d8daf28c21e40 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapper.java @@ -7,7 +7,6 @@ */ package org.elasticsearch.index.mapper; -import org.apache.lucene.index.IndexableField; import org.apache.lucene.search.Query; import org.apache.lucene.spatial.prefix.PrefixTreeStrategy; import org.apache.lucene.spatial.prefix.RecursivePrefixTreeStrategy; @@ -18,12 +17,14 @@ import org.apache.lucene.spatial.prefix.tree.SpatialPrefixTree; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.Version; +import org.elasticsearch.common.CheckedConsumer; import org.elasticsearch.common.Explicit; import org.elasticsearch.common.geo.GeoUtils; import org.elasticsearch.common.geo.GeometryParser; import org.elasticsearch.common.geo.ShapeRelation; import org.elasticsearch.common.geo.ShapesAvailability; import org.elasticsearch.common.geo.SpatialStrategy; +import org.elasticsearch.common.geo.XShapeCollection; import org.elasticsearch.common.geo.builders.ShapeBuilder; import org.elasticsearch.common.geo.builders.ShapeBuilder.Orientation; import org.elasticsearch.common.geo.parsers.ShapeParser; @@ -34,16 +35,18 @@ import org.elasticsearch.geometry.Geometry; import org.elasticsearch.index.query.LegacyGeoShapeQueryProcessor; import org.elasticsearch.index.query.SearchExecutionContext; +import org.locationtech.spatial4j.shape.Point; import org.locationtech.spatial4j.shape.Shape; +import org.locationtech.spatial4j.shape.jts.JtsGeometry; import java.io.IOException; -import java.text.ParseException; import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.function.Consumer; /** * FieldMapper for indexing {@link org.locationtech.spatial4j.shape.Shape}s. @@ -68,7 +71,7 @@ * @deprecated use {@link GeoShapeFieldMapper} */ @Deprecated -public class LegacyGeoShapeFieldMapper extends AbstractShapeGeometryFieldMapper, Shape> { +public class LegacyGeoShapeFieldMapper extends AbstractShapeGeometryFieldMapper> { public static final String CONTENT_TYPE = "geo_shape"; @@ -300,14 +303,12 @@ public LegacyGeoShapeFieldMapper build(ContentPath contentPath) { GeoShapeFieldType ft = buildFieldType(parser, contentPath); return new LegacyGeoShapeFieldMapper(name, ft, multiFieldsBuilder.build(this, contentPath), copyTo.build(), - new LegacyGeoShapeIndexer(ft), parser, this); + parser, this); } } private static class LegacyGeoShapeParser extends Parser> { - /** - * Note that this parser is only used for formatting values. - */ + private final GeometryParser geometryParser; private LegacyGeoShapeParser() { @@ -315,8 +316,16 @@ private LegacyGeoShapeParser() { } @Override - public ShapeBuilder parse(XContentParser parser) throws IOException, ParseException { - return ShapeParser.parse(parser); + public void parse( + XContentParser parser, + CheckedConsumer, IOException> consumer, + Consumer onMalformed + ) throws IOException { + try { + consumer.accept(ShapeParser.parse(parser)); + } catch (ElasticsearchParseException e) { + onMalformed.accept(e); + } } @Override @@ -447,11 +456,11 @@ public PrefixTreeStrategy resolvePrefixTreeStrategy(String strategyName) { public LegacyGeoShapeFieldMapper(String simpleName, MappedFieldType mappedFieldType, MultiFields multiFields, CopyTo copyTo, - LegacyGeoShapeIndexer indexer, LegacyGeoShapeParser parser, + LegacyGeoShapeParser parser, Builder builder) { super(simpleName, mappedFieldType, Collections.singletonMap(mappedFieldType.name(), Lucene.KEYWORD_ANALYZER), builder.ignoreMalformed.get(), builder.coerce.get(), builder.ignoreZValue.get(), builder.orientation.get(), - multiFields, copyTo, indexer, parser); + multiFields, copyTo, parser); this.indexCreatedVersion = builder.indexCreatedVersion; this.builder = builder; } @@ -466,18 +475,25 @@ String strategy() { } @Override - protected void addStoredFields(ParseContext context, Shape geometry) { - // noop: we do not store geo_shapes; and will not store legacy geo_shape types - } - - @Override - protected void addDocValuesFields(String name, Shape geometry, List fields, ParseContext context) { - // doc values are not supported - } - - @Override - protected void addMultiFields(ParseContext context, Shape geometry) { - // noop (completion suggester currently not compatible with geo_shape) + protected void index(ParseContext context, ShapeBuilder shapeBuilder) throws IOException { + Shape shape = shapeBuilder.buildS4J(); + if (fieldType().pointsOnly()) { + // index configured for pointsOnly + if (shape instanceof XShapeCollection && ((XShapeCollection) shape).pointsOnly()) { + // MULTIPOINT data: index each point separately + @SuppressWarnings("unchecked") List shapes = ((XShapeCollection) shape).getShapes(); + for (Shape s : shapes) { + context.doc().addAll(Arrays.asList(fieldType().defaultPrefixTreeStrategy().createIndexableFields(s))); + } + return; + } else if (shape instanceof Point == false) { + throw new MapperParsingException("[{" + fieldType().name() + "}] is configured for points only but a " + + ((shape instanceof JtsGeometry) ? ((JtsGeometry)shape).getGeom().getGeometryType() : shape.getClass()) + + " was found"); + } + } + context.doc().addAll(Arrays.asList(fieldType().defaultPrefixTreeStrategy().createIndexableFields(shape))); + createFieldNamesField(context); } @Override diff --git a/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeIndexer.java b/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeIndexer.java deleted file mode 100644 index d89b5dbb499dd..0000000000000 --- a/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeIndexer.java +++ /dev/null @@ -1,61 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0 and the Server Side Public License, v 1; you may not use this file except - * in compliance with, at your election, the Elastic License 2.0 or the Server - * Side Public License, v 1. - */ - -package org.elasticsearch.index.mapper; - -import org.apache.lucene.index.IndexableField; -import org.elasticsearch.common.geo.XShapeCollection; -import org.elasticsearch.common.geo.builders.ShapeBuilder; -import org.locationtech.spatial4j.shape.Point; -import org.locationtech.spatial4j.shape.Shape; -import org.locationtech.spatial4j.shape.jts.JtsGeometry; - -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; - -public class LegacyGeoShapeIndexer implements AbstractGeometryFieldMapper.Indexer, Shape> { - - private LegacyGeoShapeFieldMapper.GeoShapeFieldType fieldType; - - public LegacyGeoShapeIndexer(LegacyGeoShapeFieldMapper.GeoShapeFieldType fieldType) { - this.fieldType = fieldType; - } - - - @Override - public Shape prepareForIndexing(ShapeBuilder shapeBuilder) { - return shapeBuilder.buildS4J(); - } - - @Override - public Class processedClass() { - return Shape.class; - } - - @Override - public List indexShape(ParseContext context, Shape shape) { - if (fieldType.pointsOnly()) { - // index configured for pointsOnly - if (shape instanceof XShapeCollection && XShapeCollection.class.cast(shape).pointsOnly()) { - // MULTIPOINT data: index each point separately - @SuppressWarnings("unchecked") List shapes = ((XShapeCollection) shape).getShapes(); - List fields = new ArrayList<>(); - for (Shape s : shapes) { - fields.addAll(Arrays.asList(fieldType.defaultPrefixTreeStrategy().createIndexableFields(s))); - } - return fields; - } else if (shape instanceof Point == false) { - throw new MapperParsingException("[{" + fieldType.name() + "}] is configured for points only but a " - + ((shape instanceof JtsGeometry) ? ((JtsGeometry)shape).getGeom().getGeometryType() : shape.getClass()) - + " was found"); - } - } - return Arrays.asList(fieldType.defaultPrefixTreeStrategy().createIndexableFields(shape)); - } -} diff --git a/server/src/test/java/org/elasticsearch/common/geo/GeometryIndexerTests.java b/server/src/test/java/org/elasticsearch/common/geo/GeometryIndexerTests.java index 15a32f8cec0c7..440460e209bba 100644 --- a/server/src/test/java/org/elasticsearch/common/geo/GeometryIndexerTests.java +++ b/server/src/test/java/org/elasticsearch/common/geo/GeometryIndexerTests.java @@ -8,17 +8,6 @@ package org.elasticsearch.common.geo; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.instanceOf; -import static org.hamcrest.Matchers.not; - -import java.io.IOException; -import java.text.ParseException; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; -import java.util.List; - import org.apache.lucene.index.IndexableField; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; @@ -39,6 +28,17 @@ import org.elasticsearch.test.ESTestCase; import org.locationtech.spatial4j.exception.InvalidShapeException; +import java.io.IOException; +import java.text.ParseException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.not; + public class GeometryIndexerTests extends ESTestCase { GeoShapeIndexer indexer = new GeoShapeIndexer(true, "test"); @@ -293,7 +293,7 @@ public void testRectangle() { assertEquals(indexed, processed); // a rectangle is broken into two triangles - List fields = indexer.indexShape(null, indexed); + List fields = indexer.indexShape(indexed); assertEquals(fields.size(), 2); indexed = new Rectangle(179, -179, 10, -10); @@ -301,7 +301,7 @@ public void testRectangle() { assertEquals(indexed, processed); // a rectangle crossing the dateline is broken into 4 triangles - fields = indexer.indexShape(null, indexed); + fields = indexer.indexShape(indexed); assertEquals(fields.size(), 4); } @@ -311,7 +311,7 @@ public void testDegeneratedRectangles() { assertEquals(indexed, processed); // Rectangle is a line - List fields = indexer.indexShape(null, indexed); + List fields = indexer.indexShape(indexed); assertEquals(fields.size(), 1); indexed = new Rectangle(-179, -178, 10, 10); @@ -319,7 +319,7 @@ public void testDegeneratedRectangles() { assertEquals(indexed, processed); // Rectangle is a line - fields = indexer.indexShape(null, indexed); + fields = indexer.indexShape(indexed); assertEquals(fields.size(), 1); indexed = new Rectangle(-179, -179, 10, 10); @@ -327,7 +327,7 @@ public void testDegeneratedRectangles() { assertEquals(indexed, processed); // Rectangle is a point - fields = indexer.indexShape(null, indexed); + fields = indexer.indexShape(indexed); assertEquals(fields.size(), 1); indexed = new Rectangle(180, -179, 10, -10); @@ -335,7 +335,7 @@ public void testDegeneratedRectangles() { assertEquals(indexed, processed); // Rectangle crossing the dateline, one side is a line - fields = indexer.indexShape(null, indexed); + fields = indexer.indexShape(indexed); assertEquals(fields.size(), 3); indexed = new Rectangle(180, -179, 10, 10); @@ -344,7 +344,7 @@ public void testDegeneratedRectangles() { // Rectangle crossing the dateline, one side is a point, // other side a line - fields = indexer.indexShape(null, indexed); + fields = indexer.indexShape(indexed); assertEquals(fields.size(), 2); indexed = new Rectangle(-178, -180, 10, -10); @@ -352,7 +352,7 @@ public void testDegeneratedRectangles() { assertEquals(indexed, processed); // Rectangle crossing the dateline, one side is a line - fields = indexer.indexShape(null, indexed); + fields = indexer.indexShape(indexed); assertEquals(fields.size(), 3); indexed = new Rectangle(-178, -180, 10, 10); @@ -361,7 +361,7 @@ public void testDegeneratedRectangles() { // Rectangle crossing the dateline, one side is a point, // other side a line - fields = indexer.indexShape(null, indexed); + fields = indexer.indexShape(indexed); assertEquals(fields.size(), 2); indexed = new Rectangle(0.0, 1.0819389717881644E-299, 1.401298464324817E-45, 0.0); @@ -369,7 +369,7 @@ public void testDegeneratedRectangles() { assertEquals(indexed, processed); // Rectangle is a point - fields = indexer.indexShape(null, processed); + fields = indexer.indexShape(processed); assertEquals(fields.size(), 1); indexed = new Rectangle(-1.4017117476654298E-170, 0.0, 0.0, -2.415012082648633E-174); @@ -377,7 +377,7 @@ public void testDegeneratedRectangles() { assertEquals(indexed, processed); // Rectangle is a triangle but needs to be computed quantize - fields = indexer.indexShape(null, processed); + fields = indexer.indexShape(processed); assertEquals(fields.size(), 2); } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DocumentParserTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DocumentParserTests.java index 74798ada41912..c308edf42fd78 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/DocumentParserTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/DocumentParserTests.java @@ -1068,14 +1068,14 @@ public void testWithDynamicTemplates() throws Exception { ParsedDocument doc = mapper.parse(source("1", b -> b.field(field, "41.12,-71.34"), null, Map.of(field, "points"))); IndexableField[] fields = doc.rootDoc().getFields(field); assertThat(fields, arrayWithSize(2)); - assertThat(fields[0].fieldType(), sameInstance(LatLonDocValuesField.TYPE)); - assertThat(fields[1].fieldType(), sameInstance(LatLonPoint.TYPE)); + assertThat(fields[0].fieldType(), sameInstance(LatLonPoint.TYPE)); + assertThat(fields[1].fieldType(), sameInstance(LatLonDocValuesField.TYPE)); doc = mapper.parse(source("1", b -> b.field(field, new double[]{-71.34, 41.12}), null, Map.of(field, "points"))); fields = doc.rootDoc().getFields(field); assertThat(fields, arrayWithSize(2)); - assertThat(fields[0].fieldType(), sameInstance(LatLonDocValuesField.TYPE)); - assertThat(fields[1].fieldType(), sameInstance(LatLonPoint.TYPE)); + assertThat(fields[0].fieldType(), sameInstance(LatLonPoint.TYPE)); + assertThat(fields[1].fieldType(), sameInstance(LatLonDocValuesField.TYPE)); doc = mapper.parse(source("1", b -> { b.startObject(field); @@ -1085,16 +1085,16 @@ public void testWithDynamicTemplates() throws Exception { }, null, Map.of(field, "points"))); fields = doc.rootDoc().getFields(field); assertThat(fields, arrayWithSize(2)); - assertThat(fields[0].fieldType(), sameInstance(LatLonDocValuesField.TYPE)); - assertThat(fields[1].fieldType(), sameInstance(LatLonPoint.TYPE)); + assertThat(fields[0].fieldType(), sameInstance(LatLonPoint.TYPE)); + assertThat(fields[1].fieldType(), sameInstance(LatLonDocValuesField.TYPE)); doc = mapper.parse(source("1", b -> b.field(field, new String[]{"41.12,-71.34", "43,-72.34"}), null, Map.of(field, "points"))); fields = doc.rootDoc().getFields(field); assertThat(fields, arrayWithSize(4)); - assertThat(fields[0].fieldType(), sameInstance(LatLonDocValuesField.TYPE)); + assertThat(fields[0].fieldType(), sameInstance(LatLonPoint.TYPE)); assertThat(fields[1].fieldType(), sameInstance(LatLonDocValuesField.TYPE)); assertThat(fields[2].fieldType(), sameInstance(LatLonPoint.TYPE)); - assertThat(fields[3].fieldType(), sameInstance(LatLonPoint.TYPE)); + assertThat(fields[3].fieldType(), sameInstance(LatLonDocValuesField.TYPE)); doc = mapper.parse(source("1", b -> { b.startArray(field); @@ -1111,10 +1111,10 @@ public void testWithDynamicTemplates() throws Exception { }, null, Map.of(field, "points"))); fields = doc.rootDoc().getFields(field); assertThat(fields, arrayWithSize(4)); - assertThat(fields[0].fieldType(), sameInstance(LatLonDocValuesField.TYPE)); + assertThat(fields[0].fieldType(), sameInstance(LatLonPoint.TYPE)); assertThat(fields[1].fieldType(), sameInstance(LatLonDocValuesField.TYPE)); assertThat(fields[2].fieldType(), sameInstance(LatLonPoint.TYPE)); - assertThat(fields[3].fieldType(), sameInstance(LatLonPoint.TYPE)); + assertThat(fields[3].fieldType(), sameInstance(LatLonDocValuesField.TYPE)); doc = mapper.parse(source("1", b -> { b.startObject("address"); @@ -1123,8 +1123,8 @@ public void testWithDynamicTemplates() throws Exception { }, null, Map.of("address.home", "points"))); fields = doc.rootDoc().getFields("address.home"); assertThat(fields, arrayWithSize(2)); - assertThat(fields[0].fieldType(), sameInstance(LatLonDocValuesField.TYPE)); - assertThat(fields[1].fieldType(), sameInstance(LatLonPoint.TYPE)); + assertThat(fields[0].fieldType(), sameInstance(LatLonPoint.TYPE)); + assertThat(fields[1].fieldType(), sameInstance(LatLonDocValuesField.TYPE)); } public void testDynamicTemplatesNotFound() throws Exception { diff --git a/server/src/test/java/org/elasticsearch/index/mapper/ExternalFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/ExternalFieldMapperTests.java index 22ffae74cdf3e..f84466da586e0 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/ExternalFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/ExternalFieldMapperTests.java @@ -9,13 +9,11 @@ package org.elasticsearch.index.mapper; import org.apache.lucene.index.IndexableField; -import org.elasticsearch.common.geo.GeoPoint; import org.elasticsearch.plugins.Plugin; import java.util.Collection; import java.util.Collections; -import static org.hamcrest.Matchers.closeTo; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; @@ -35,13 +33,6 @@ public void testExternalValues() throws Exception { assertThat(doc.rootDoc().getField("field.bool"), notNullValue()); assertThat(doc.rootDoc().getField("field.bool").stringValue(), is("T")); - assertThat(doc.rootDoc().getField("field.point"), notNullValue()); - GeoPoint point = new GeoPoint().resetFromIndexableField(doc.rootDoc().getField("field.point")); - assertThat(point.lat(), closeTo(42.0, 1e-5)); - assertThat(point.lon(), closeTo(51.0, 1e-5)); - - assertThat(doc.rootDoc().getField("field.shape"), notNullValue()); - assertThat(doc.rootDoc().getField("field.field"), notNullValue()); assertThat(doc.rootDoc().getField("field.field").stringValue(), is("foo")); @@ -70,14 +61,6 @@ public void testExternalValuesWithMultifield() throws Exception { assertThat(doc.rootDoc().getField("field.bool"), notNullValue()); assertThat(doc.rootDoc().getField("field.bool").stringValue(), is("T")); - assertThat(doc.rootDoc().getField("field.point"), notNullValue()); - GeoPoint point = new GeoPoint().resetFromIndexableField(doc.rootDoc().getField("field.point")); - assertThat(point.lat(), closeTo(42.0, 1E-5)); - assertThat(point.lon(), closeTo(51.0, 1E-5)); - - IndexableField shape = doc.rootDoc().getField("field.shape"); - assertThat(shape, notNullValue()); - IndexableField field = doc.rootDoc().getField("field.text"); assertThat(field, notNullValue()); assertThat(field.stringValue(), is("foo")); diff --git a/server/src/test/java/org/elasticsearch/index/mapper/ExternalMapper.java b/server/src/test/java/org/elasticsearch/index/mapper/ExternalMapper.java index c8fd48e79c34b..3703a35bd5156 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/ExternalMapper.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/ExternalMapper.java @@ -10,9 +10,6 @@ import org.apache.lucene.analysis.standard.StandardAnalyzer; import org.elasticsearch.common.collect.Iterators; -import org.elasticsearch.common.geo.GeoPoint; -import org.elasticsearch.common.geo.builders.PointBuilder; -import org.elasticsearch.geometry.Point; import org.elasticsearch.index.analysis.AnalyzerScope; import org.elasticsearch.index.analysis.IndexAnalyzers; import org.elasticsearch.index.analysis.NamedAnalyzer; @@ -21,7 +18,6 @@ import java.io.IOException; import java.nio.charset.Charset; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.Iterator; @@ -54,8 +50,6 @@ public static class Builder extends FieldMapper.Builder { private final BinaryFieldMapper.Builder binBuilder = new BinaryFieldMapper.Builder(Names.FIELD_BIN); private final BooleanFieldMapper.Builder boolBuilder = new BooleanFieldMapper.Builder(Names.FIELD_BOOL, ScriptCompiler.NONE); - private final GeoPointFieldMapper.Builder latLonPointBuilder = new GeoPointFieldMapper.Builder(Names.FIELD_POINT, false); - private final GeoShapeFieldMapper.Builder shapeBuilder = new GeoShapeFieldMapper.Builder(Names.FIELD_SHAPE, false, true); private final Mapper.Builder stringBuilder; private final String generatedValue; private final String mapperName; @@ -77,13 +71,11 @@ public ExternalMapper build(ContentPath contentPath) { contentPath.add(name); BinaryFieldMapper binMapper = binBuilder.build(contentPath); BooleanFieldMapper boolMapper = boolBuilder.build(contentPath); - GeoPointFieldMapper pointMapper = (GeoPointFieldMapper) latLonPointBuilder.build(contentPath); - AbstractShapeGeometryFieldMapper shapeMapper = shapeBuilder.build(contentPath); FieldMapper stringMapper = (FieldMapper)stringBuilder.build(contentPath); contentPath.remove(); return new ExternalMapper(name, buildFullName(contentPath), generatedValue, mapperName, binMapper, boolMapper, - pointMapper, shapeMapper, stringMapper, multiFieldsBuilder.build(this, contentPath), copyTo.build()); + stringMapper, multiFieldsBuilder.build(this, contentPath), copyTo.build()); } } @@ -113,22 +105,18 @@ public ValueFetcher valueFetcher(SearchExecutionContext context, String format) private final BinaryFieldMapper binMapper; private final BooleanFieldMapper boolMapper; - private final GeoPointFieldMapper pointMapper; - private final AbstractShapeGeometryFieldMapper shapeMapper; private final FieldMapper stringMapper; public ExternalMapper(String simpleName, String contextName, String generatedValue, String mapperName, - BinaryFieldMapper binMapper, BooleanFieldMapper boolMapper, GeoPointFieldMapper pointMapper, - AbstractShapeGeometryFieldMapper shapeMapper, FieldMapper stringMapper, + BinaryFieldMapper binMapper, BooleanFieldMapper boolMapper, + FieldMapper stringMapper, MultiFields multiFields, CopyTo copyTo) { super(simpleName, new ExternalFieldType(contextName, true, true, false), multiFields, copyTo); this.generatedValue = generatedValue; this.mapperName = mapperName; this.binMapper = binMapper; this.boolMapper = boolMapper; - this.pointMapper = pointMapper; - this.shapeMapper = shapeMapper; this.stringMapper = stringMapper; } @@ -139,21 +127,6 @@ public void parse(ParseContext context) throws IOException { boolMapper.parse(context.createExternalValueContext(true)); - // Let's add a Dummy Point - double lat = 42.0; - double lng = 51.0; - ArrayList points = new ArrayList<>(); - points.add(new GeoPoint(lat, lng)); - pointMapper.parse(context.createExternalValueContext(points)); - - // Let's add a Dummy Shape - if (shapeMapper instanceof GeoShapeFieldMapper) { - shapeMapper.parse(context.createExternalValueContext(new Point(-100, 45))); - } else { - PointBuilder pb = new PointBuilder(-100, 45); - shapeMapper.parse(context.createExternalValueContext(pb.buildS4J())); - } - context = context.createExternalValueContext(generatedValue); // Let's add a Original String @@ -169,7 +142,7 @@ protected void parseCreateField(ParseContext context) { @Override public Iterator iterator() { - return Iterators.concat(super.iterator(), Arrays.asList(binMapper, boolMapper, pointMapper, shapeMapper, stringMapper).iterator()); + return Iterators.concat(super.iterator(), Arrays.asList(binMapper, boolMapper, stringMapper).iterator()); } @Override 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 b7d129d01170e..caf4d10d6f90e 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/GeoPointFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/GeoPointFieldMapperTests.java @@ -220,6 +220,21 @@ public void testMultiField() throws Exception { assertThat(doc.getField("field.latlon").stringValue(), equalTo("s093jd0k72s1")); } + public void testMultiFieldWithMultipleValues() throws Exception { + DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> { + b.field("type", "geo_point").field("doc_values", false); + b.startObject("fields"); + { + b.startObject("geohash").field("type", "keyword").field("doc_values", false).endObject(); + } + b.endObject(); + })); + ParseContext.Document doc = mapper.parse(source(b -> b.array("field", "POINT (2 3)", "POINT (4 5)"))).rootDoc(); + assertThat(doc.getFields("field.geohash"), arrayWithSize(2)); + assertThat(doc.getFields("field.geohash")[0].binaryValue().utf8ToString(), equalTo("s093jd0k72s1")); + assertThat(doc.getFields("field.geohash")[1].binaryValue().utf8ToString(), equalTo("s0fu7n0xng81")); + } + public void testNullValue() throws Exception { DocumentMapper mapper = createDocumentMapper( fieldMapping(b -> b.field("type", "geo_point")) @@ -298,7 +313,7 @@ public void testInvalidGeohashNotIgnored() throws Exception { MapperParsingException.class, () -> mapper.parse(source(b -> b.field("field", "1234.333"))) ); - assertThat(e.getMessage(), containsString("failed to parse field [field] of type [geo_point]")); + assertThat(e.getMessage(), containsString("Failed to parse field [field] of type [geo_point]")); assertThat(e.getRootCause().getMessage(), containsString("unsupported symbol [.] in geohash [1234.333]")); } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/GeoPointFieldTypeTests.java b/server/src/test/java/org/elasticsearch/index/mapper/GeoPointFieldTypeTests.java index dca68f3d9b914..55a89695fbb06 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/GeoPointFieldTypeTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/GeoPointFieldTypeTests.java @@ -38,11 +38,9 @@ public void testFetchSourceValue() throws IOException { assertEquals(List.of(wktPoint, otherWktPoint), fetchSourceValue(mapper, sourceValue, "wkt")); // Test a list of points in [lat,lon] array format with one malformed - // TODO Point Field parsers have a weird `ignore_malformed` impl that still throws errors - // on many types of malformed input - // sourceValue = List.of(List.of(42,0, 27.1), List.of("a", "b"), List.of(30.0, 50.0)); - // assertEquals(List.of(jsonPoint, otherJsonPoint), fetchSourceValue(mapper, sourceValue, null)); - // assertEquals(List.of(wktPoint, otherWktPoint), fetchSourceValue(mapper, sourceValue, "wkt")); + sourceValue = List.of(List.of(42.0, 27.1), List.of("a", "b"), List.of(30.0, 50.0)); + assertEquals(List.of(jsonPoint, otherJsonPoint), fetchSourceValue(mapper, sourceValue, null)); + assertEquals(List.of(wktPoint, otherWktPoint), fetchSourceValue(mapper, sourceValue, "wkt")); // Test a single point in well-known text format. sourceValue = "POINT (42.0 27.1)"; diff --git a/server/src/test/java/org/elasticsearch/search/geo/GeoShapeQueryTests.java b/server/src/test/java/org/elasticsearch/search/geo/GeoShapeQueryTests.java index 64f77d7addb64..fd3b17819a929 100644 --- a/server/src/test/java/org/elasticsearch/search/geo/GeoShapeQueryTests.java +++ b/server/src/test/java/org/elasticsearch/search/geo/GeoShapeQueryTests.java @@ -528,7 +528,7 @@ public void testPointsOnly() throws Exception { .setRefreshPolicy(IMMEDIATE).get(); } catch (MapperParsingException e) { // RandomShapeGenerator created something other than a POINT type, verify the correct exception is thrown - assertThat(e.getCause().getMessage(), containsString("is configured for points only")); + assertThat(e.getMessage(), containsString("is configured for points only")); return; } diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/SpatialUtils.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/SpatialUtils.java index e659f0cacfb3f..48d1690665419 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/SpatialUtils.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/SpatialUtils.java @@ -7,10 +7,10 @@ package org.elasticsearch.xpack.spatial; import org.apache.lucene.util.SloppyMath; +import org.elasticsearch.index.mapper.GeoShapeIndexer; import org.elasticsearch.geometry.Circle; import org.elasticsearch.geometry.LinearRing; import org.elasticsearch.geometry.Polygon; -import org.elasticsearch.index.mapper.GeoShapeIndexer; /** * Utility class for storing different helpful re-usable spatial functions diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/GeoShapeValues.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/GeoShapeValues.java index f709dc7349bc3..bdec83de9bd5c 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/GeoShapeValues.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/GeoShapeValues.java @@ -7,11 +7,11 @@ package org.elasticsearch.xpack.spatial.index.fielddata; +import org.elasticsearch.index.mapper.GeoShapeIndexer; import org.elasticsearch.geometry.Geometry; import org.elasticsearch.geometry.Rectangle; import org.elasticsearch.geometry.utils.GeographyValidator; import org.elasticsearch.geometry.utils.WellKnownText; -import org.elasticsearch.index.mapper.GeoShapeIndexer; import org.elasticsearch.search.aggregations.support.ValuesSourceType; import org.elasticsearch.xpack.spatial.index.mapper.BinaryGeoShapeDocValuesField; import org.elasticsearch.xpack.spatial.search.aggregations.support.GeoShapeValuesSourceType; @@ -135,7 +135,7 @@ public static GeoShapeValue missing(String missing) { final GeoShapeIndexer indexer = new GeoShapeIndexer(true, "missing"); final Geometry geometry = indexer.prepareForIndexing(MISSING_GEOMETRY_PARSER.fromWKT(missing)); final BinaryGeoShapeDocValuesField field = new BinaryGeoShapeDocValuesField("missing"); - field.add(indexer.indexShape(null, geometry), geometry); + field.add(indexer.indexShape(geometry), geometry); final GeometryDocValueReader reader = new GeometryDocValueReader(); reader.reset(field.binaryValue()); return new GeoShapeValue(reader); 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 462cd6eafe37b..c9d3b6ee3c633 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 @@ -37,6 +37,7 @@ import org.elasticsearch.xpack.spatial.index.fielddata.plain.AbstractLatLonShapeIndexFieldData; import org.elasticsearch.xpack.spatial.search.aggregations.support.GeoShapeValuesSourceType; +import java.io.IOException; import java.util.Arrays; import java.util.List; import java.util.Map; @@ -64,7 +65,7 @@ *

* "field" : "POLYGON ((100.0 0.0, 101.0 0.0, 101.0 1.0, 100.0 1.0, 100.0 0.0)) */ -public class GeoShapeWithDocValuesFieldMapper extends AbstractShapeGeometryFieldMapper { +public class GeoShapeWithDocValuesFieldMapper extends AbstractShapeGeometryFieldMapper { public static final String CONTENT_TYPE = "geo_shape"; private static Builder builder(FieldMapper in) { @@ -120,17 +121,6 @@ public GeoShapeWithDocValuesFieldMapper build(ContentPath contentPath) { } - @Override - @SuppressWarnings({"rawtypes", "unchecked"}) - protected void addDocValuesFields(String name, Geometry shape, List fields, ParseContext context) { - BinaryGeoShapeDocValuesField docValuesField = (BinaryGeoShapeDocValuesField) context.doc().getByKey(name); - if (docValuesField == null) { - docValuesField = new BinaryGeoShapeDocValuesField(name); - context.doc().addWithKey(name, docValuesField); - } - docValuesField.add(fields, shape); - } - public static final class GeoShapeWithDocValuesFieldType extends AbstractShapeGeometryFieldType implements GeoShapeQueryable { public GeoShapeWithDocValuesFieldType(String name, boolean indexed, boolean hasDocValues, @@ -191,14 +181,35 @@ public Query geoShapeQuery(Geometry shape, String fieldName, ShapeRelation relat }; private final Builder builder; + private final GeoShapeIndexer indexer; public GeoShapeWithDocValuesFieldMapper(String simpleName, MappedFieldType mappedFieldType, MultiFields multiFields, CopyTo copyTo, GeoShapeIndexer indexer, GeoShapeParser parser, Builder builder) { super(simpleName, mappedFieldType, builder.ignoreMalformed.get(), builder.coerce.get(), builder.ignoreZValue.get(), builder.orientation.get(), - multiFields, copyTo, indexer, parser); + multiFields, copyTo, parser); this.builder = builder; + this.indexer = indexer; + } + + @Override + protected void index(ParseContext context, Geometry geometry) throws IOException { + List fields = indexer.indexShape(geometry); + if (fieldType().isSearchable()) { + context.doc().addAll(fields); + } + if (fieldType().hasDocValues()) { + String name = fieldType().name(); + BinaryGeoShapeDocValuesField docValuesField = (BinaryGeoShapeDocValuesField) context.doc().getByKey(name); + if (docValuesField == null) { + docValuesField = new BinaryGeoShapeDocValuesField(name); + context.doc().addWithKey(name, docValuesField); + } + docValuesField.add(fields, geometry); + } else if (fieldType().isSearchable()) { + createFieldNamesField(context); + } } @Override @@ -221,13 +232,4 @@ public GeoShapeWithDocValuesFieldType fieldType() { return (GeoShapeWithDocValuesFieldType) super.fieldType(); } - @Override - protected void addStoredFields(ParseContext context, Geometry geometry) { - // noop (stored fields not available for geo_shape fields) - } - - @Override - protected void addMultiFields(ParseContext context, Geometry geometry) { - // noop (completion suggester currently not compatible with geo_shape) - } } 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 0cb83b0197102..d2eb7e6ef51b4 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 @@ -9,7 +9,6 @@ import org.apache.lucene.document.StoredField; import org.apache.lucene.document.XYDocValuesField; import org.apache.lucene.document.XYPointField; -import org.apache.lucene.index.IndexableField; import org.apache.lucene.search.Query; import org.elasticsearch.common.Explicit; import org.elasticsearch.common.geo.ShapeRelation; @@ -25,9 +24,8 @@ import org.elasticsearch.xpack.spatial.index.mapper.PointFieldMapper.ParsedCartesianPoint; import org.elasticsearch.xpack.spatial.index.query.ShapeQueryPointProcessor; -import java.util.ArrayList; +import java.io.IOException; import java.util.Arrays; -import java.util.Collections; import java.util.List; import java.util.Map; @@ -36,7 +34,7 @@ * * Uses lucene 8 XYPoint encoding */ -public class PointFieldMapper extends AbstractPointGeometryFieldMapper, List> { +public class PointFieldMapper extends AbstractPointGeometryFieldMapper { public static final String CONTENT_TYPE = "point"; public static class CartesianPointParser extends PointParser { @@ -99,7 +97,7 @@ public FieldMapper build(ContentPath contentPath) { PointFieldType ft = new PointFieldType(buildFullName(contentPath), indexed.get(), stored.get(), hasDocValues.get(), parser, meta.get()); return new PointFieldMapper(name, ft, multiFieldsBuilder.build(this, contentPath), - copyTo.build(), new PointIndexer(ft), parser, this); + copyTo.build(), parser, this); } } @@ -110,30 +108,25 @@ public FieldMapper build(ContentPath contentPath) { public PointFieldMapper(String simpleName, MappedFieldType mappedFieldType, MultiFields multiFields, CopyTo copyTo, - PointIndexer pointIndexer, CartesianPointParser parser, Builder builder) { + CartesianPointParser parser, Builder builder) { super(simpleName, mappedFieldType, multiFields, - builder.ignoreMalformed.get(), builder.ignoreZValue.get(), builder.nullValue.get(), copyTo, pointIndexer, parser); + builder.ignoreMalformed.get(), builder.ignoreZValue.get(), builder.nullValue.get(), copyTo, parser); this.builder = builder; } @Override - protected void addStoredFields(ParseContext context, List points) { - for (CartesianPoint point : points) { - context.doc().add(new StoredField(fieldType().name(), point.toString())); + protected void index(ParseContext context, ParsedCartesianPoint point) throws IOException { + if (fieldType().isSearchable()) { + context.doc().add(new XYPointField(fieldType().name(), (float) point.getX(), (float) point.getY())); } - } - - @Override - protected void addDocValuesFields(String name, List points, List fields, - ParseContext context) { - for (CartesianPoint point : points) { + if (fieldType().hasDocValues()) { context.doc().add(new XYDocValuesField(fieldType().name(), (float) point.getX(), (float) point.getY())); + } else if (fieldType().isStored() || fieldType().isSearchable()) { + createFieldNamesField(context); + } + if (fieldType().isStored()) { + context.doc().add(new StoredField(fieldType().name(), point.toString())); } - } - - @Override - protected void addMultiFields(ParseContext context, List points) { - // noop } @Override @@ -228,34 +221,4 @@ public int hashCode() { } } - protected static class PointIndexer implements Indexer, List> { - protected final PointFieldType fieldType; - - PointIndexer(PointFieldType fieldType) { - this.fieldType = fieldType; - } - - @Override - public List prepareForIndexing(List points) { - if (points == null || points.isEmpty()) { - return Collections.emptyList(); - } - return points; - } - - @Override - @SuppressWarnings("unchecked") - public Class> processedClass() { - return (Class>)(Object)List.class; - } - - @Override - public List indexShape(ParseContext context, List points) { - ArrayList fields = new ArrayList<>(1); - for (CartesianPoint point : points) { - fields.add(new XYPointField(fieldType.name(), (float) point.getX(), (float) point.getY())); - } - return fields; - } - } } 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 04710afd3d213..9ae24708e685e 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 @@ -7,7 +7,6 @@ package org.elasticsearch.xpack.spatial.index.mapper; import org.apache.lucene.document.XYShape; -import org.apache.lucene.index.IndexableField; import org.apache.lucene.search.Query; import org.elasticsearch.common.Explicit; import org.elasticsearch.common.geo.GeometryParser; @@ -23,6 +22,7 @@ import org.elasticsearch.index.query.SearchExecutionContext; import org.elasticsearch.xpack.spatial.index.query.ShapeQueryProcessor; +import java.io.IOException; import java.util.Arrays; import java.util.List; import java.util.Map; @@ -43,7 +43,7 @@ *

* "field" : "POLYGON ((1050.0 -1000.0, 1051.0 -1000.0, 1051.0 -1001.0, 1050.0 -1001.0, 1050.0 -1000.0)) */ -public class ShapeFieldMapper extends AbstractShapeGeometryFieldMapper { +public class ShapeFieldMapper extends AbstractShapeGeometryFieldMapper { public static final String CONTENT_TYPE = "shape"; private static Builder builder(FieldMapper in) { @@ -80,8 +80,7 @@ public ShapeFieldMapper build(ContentPath contentPath) { ShapeFieldType ft = new ShapeFieldType(buildFullName(contentPath), indexed.get(), orientation.get().value(), parser, meta.get()); return new ShapeFieldMapper(name, ft, - multiFieldsBuilder.build(this, contentPath), copyTo.build(), - new ShapeIndexer(ft.name()), parser, this); + multiFieldsBuilder.build(this, contentPath), copyTo.build(), parser, this); } } @@ -112,30 +111,22 @@ public String typeName() { } private final Builder builder; + private final ShapeIndexer indexer; public ShapeFieldMapper(String simpleName, MappedFieldType mappedFieldType, MultiFields multiFields, CopyTo copyTo, - Indexer indexer, Parser parser, Builder builder) { + Parser parser, Builder builder) { super(simpleName, mappedFieldType, builder.ignoreMalformed.get(), builder.coerce.get(), builder.ignoreZValue.get(), builder.orientation.get(), - multiFields, copyTo, indexer, parser); + multiFields, copyTo, parser); this.builder = builder; + this.indexer = new ShapeIndexer(mappedFieldType.name()); } @Override - protected void addStoredFields(ParseContext context, Geometry geometry) { - // noop: we currently do not store geo_shapes - // @todo store as geojson string? - } - - @Override - protected void addDocValuesFields(String name, Geometry geometry, List fields, ParseContext context) { - // we should throw a mapping exception before we get here - } - - @Override - protected void addMultiFields(ParseContext context, Geometry geometry) { - // noop (completion suggester currently not compatible with geo_shape) + protected void index(ParseContext context, Geometry geometry) throws IOException { + context.doc().addAll(indexer.indexShape(geometry)); + createFieldNamesField(context); } @Override diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/ShapeIndexer.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/ShapeIndexer.java index 437e46432e789..95998cc5001e4 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/ShapeIndexer.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/ShapeIndexer.java @@ -20,33 +20,24 @@ import org.elasticsearch.geometry.Point; import org.elasticsearch.geometry.Polygon; import org.elasticsearch.geometry.Rectangle; -import org.elasticsearch.index.mapper.AbstractShapeGeometryFieldMapper; -import org.elasticsearch.index.mapper.ParseContext; import org.elasticsearch.xpack.spatial.common.ShapeUtils; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; -public class ShapeIndexer implements AbstractShapeGeometryFieldMapper.Indexer { +public class ShapeIndexer { private final String name; public ShapeIndexer(String name) { this.name = name; } - @Override - public Geometry prepareForIndexing(Geometry geometry) { - return geometry; - } - - @Override - public Class processedClass() { - return Geometry.class; - } - - @Override - public List indexShape(ParseContext context, Geometry shape) { + public List indexShape(Geometry shape) { + if (shape == null) { + return Collections.emptyList(); + } LuceneGeometryVisitor visitor = new LuceneGeometryVisitor(name); shape.visit(visitor); return visitor.fields; diff --git a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/fielddata/TriangleTreeTests.java b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/fielddata/TriangleTreeTests.java index 3afa439e81a55..6bc138e95aa6b 100644 --- a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/fielddata/TriangleTreeTests.java +++ b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/fielddata/TriangleTreeTests.java @@ -26,7 +26,7 @@ public void testVisitAllTriangles() throws IOException { Geometry geometry = GeometryTestUtils.randomGeometryWithoutCircle(randomIntBetween(1, 10), false); // write tree GeoShapeIndexer indexer = new GeoShapeIndexer(true, "test"); - List fieldList = indexer.indexShape(null, indexer.prepareForIndexing(geometry)); + List fieldList = indexer.indexShape(geometry); ByteBuffersDataOutput output = new ByteBuffersDataOutput(); TriangleTreeWriter.writeTo(output, fieldList); // read tree diff --git a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/CartesianFieldMapperTests.java b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/CartesianFieldMapperTests.java index 4ee7988e564f4..f986125ba954e 100644 --- a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/CartesianFieldMapperTests.java +++ b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/CartesianFieldMapperTests.java @@ -145,7 +145,7 @@ public void testZValue() throws IOException { MapperParsingException e = expectThrows(MapperParsingException.class, () -> mapper2.parse(source(b -> b.field(FIELD_NAME, "POINT (2000.1 305.6 34567.33)"))) ); - assertThat(e.getMessage(), containsString("failed to parse field [" + FIELD_NAME + "] of type")); + assertThat(e.getMessage(), containsString("Failed to parse field [" + FIELD_NAME + "] of type")); assertThat(e.getRootCause().getMessage(), containsString("found Z value [34567.33] but [ignore_z_value] parameter is [false]")); } diff --git a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/LatLonShapeDocValuesQueryTests.java b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/LatLonShapeDocValuesQueryTests.java index d4908cddae277..5ad7e9832924a 100644 --- a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/LatLonShapeDocValuesQueryTests.java +++ b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/LatLonShapeDocValuesQueryTests.java @@ -73,8 +73,7 @@ public void testIndexSimpleShapes() throws Exception { GeometryTestUtils::randomPolygon ); Geometry geometry = geometryFunc.apply(false); - geometry = indexer.prepareForIndexing(geometry); - List fields = indexer.indexShape(null, geometry); + List fields = indexer.indexShape(geometry); for (IndexableField field : fields) { doc.add(field); } @@ -116,8 +115,7 @@ public void testIndexMultiShapes() throws Exception { for (int id = 0; id < numDocs; id++) { Document doc = new Document(); Geometry geometry = GeometryTestUtils.randomGeometryWithoutCircle(randomIntBetween(1, 5), false); - geometry = indexer.prepareForIndexing(geometry); - List fields = indexer.indexShape(null, geometry); + List fields = indexer.indexShape(geometry); for (IndexableField field : fields) { doc.add(field); } diff --git a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/PointFieldTypeTests.java b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/PointFieldTypeTests.java index da71d912c4db2..772635d2e6114 100644 --- a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/PointFieldTypeTests.java +++ b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/PointFieldTypeTests.java @@ -51,11 +51,9 @@ public void testFetchSourceValue() throws IOException { assertEquals(List.of(wktPoint), fetchSourceValue(mapper, sourceValue, "wkt")); // Test a list of points in [x, y] array format with a malformed entry - // TODO Point Field parsers have a weird `ignore_malformed` impl that still throws errors - // on many types of malformed input - // sourceValue = List.of(List.of(42.0, 27.1), List.of("a", "b"), List.of(30.0, 50.0)); - // assertEquals(List.of(jsonPoint, otherJsonPoint), fetchSourceValue(mapper, sourceValue, null)); - // assertEquals(List.of(wktPoint, otherWktPoint), fetchSourceValue(mapper, sourceValue, "wkt")); + sourceValue = List.of(List.of(42.0, 27.1), List.of("a", "b"), List.of(30.0, 50.0)); + assertEquals(List.of(jsonPoint, otherJsonPoint), fetchSourceValue(mapper, sourceValue, null)); + assertEquals(List.of(wktPoint, otherWktPoint), fetchSourceValue(mapper, sourceValue, "wkt")); } } diff --git a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/ingest/CircleProcessorTests.java b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/ingest/CircleProcessorTests.java index ab1c27500a9ce..0f512e606f24a 100644 --- a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/ingest/CircleProcessorTests.java +++ b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/ingest/CircleProcessorTests.java @@ -226,8 +226,7 @@ public void testGeoShapeQueryAcrossDateline() throws IOException { try (Directory dir = newDirectory(); RandomIndexWriter w = new RandomIndexWriter(random(), dir)) { Document doc = new Document(); GeoShapeIndexer indexer = new GeoShapeIndexer(true, fieldName); - Geometry normalized = indexer.prepareForIndexing(geometry); - for (IndexableField field : indexer.indexShape(null, normalized)) { + for (IndexableField field : indexer.indexShape(geometry)) { doc.add(field); } w.addDocument(doc); @@ -258,8 +257,7 @@ public void testShapeQuery() throws IOException { try (Directory dir = newDirectory(); RandomIndexWriter w = new RandomIndexWriter(random(), dir)) { Document doc = new Document(); ShapeIndexer indexer = new ShapeIndexer(fieldName); - Geometry normalized = indexer.prepareForIndexing(geometry); - for (IndexableField field : indexer.indexShape(null, normalized)) { + for (IndexableField field : indexer.indexShape(geometry)) { doc.add(field); } w.addDocument(doc); diff --git a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/util/GeoTestUtils.java b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/util/GeoTestUtils.java index f6e0132647ee7..3c775b7c66652 100644 --- a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/util/GeoTestUtils.java +++ b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/util/GeoTestUtils.java @@ -43,7 +43,7 @@ public static GeometryDocValueReader geometryDocValueReader(Geometry geometry, C CentroidCalculator centroidCalculator = new CentroidCalculator(); centroidCalculator.add(geometry); GeometryDocValueReader reader = new GeometryDocValueReader(); - reader.reset(GeometryDocValueWriter.write(indexer.indexShape(null, geometry), encoder, centroidCalculator)); + reader.reset(GeometryDocValueWriter.write(indexer.indexShape(geometry), encoder, centroidCalculator)); return reader; } @@ -51,7 +51,7 @@ public static BinaryGeoShapeDocValuesField binaryGeoShapeDocValuesField(String n GeoShapeIndexer indexer = new GeoShapeIndexer(true, name); geometry = indexer.prepareForIndexing(geometry); BinaryGeoShapeDocValuesField field = new BinaryGeoShapeDocValuesField(name); - field.add(indexer.indexShape(null, geometry) , geometry); + field.add(indexer.indexShape(geometry) , geometry); return field; }