From 1a1bb4707d6971a1b5154d3c155d379acc7fa3e1 Mon Sep 17 00:00:00 2001 From: Igor Motov Date: Tue, 30 Jul 2019 12:36:39 -0400 Subject: [PATCH] Geo: move indexShape to AbstractGeometryFieldMapper.Indexer (#44979) Move indexShape functionality into AbstractGeometryFieldMapper to make it more unit testable. Relates to #43644 --- .../mapper/AbstractGeometryFieldMapper.java | 12 +- .../index/mapper/GeoShapeFieldMapper.java | 129 +----------------- .../index/mapper/GeoShapeIndexer.java | 109 ++++++++++++++- .../mapper/LegacyGeoShapeFieldMapper.java | 36 +---- .../index/mapper/LegacyGeoShapeIndexer.java | 37 +++++ .../query/VectorGeoShapeQueryProcessor.java | 4 +- .../common/geo/BaseGeoParsingTestCase.java | 2 +- .../common/geo/GeoJsonShapeParserTests.java | 2 +- .../common/geo/GeoWKTShapeParserTests.java | 2 +- .../common/geo/GeometryIndexerTests.java | 6 +- .../common/geo/ShapeBuilderTests.java | 2 +- 11 files changed, 171 insertions(+), 170 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 29b71f87c83..db5e49a99c4 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java @@ -42,6 +42,7 @@ import org.elasticsearch.index.query.QueryShardException; import java.io.IOException; import java.text.ParseException; +import java.util.ArrayList; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -77,6 +78,7 @@ public abstract class AbstractGeometryFieldMapper extends Fie Class processedClass(); + List indexShape(ParseContext context, Processed shape); } /** @@ -395,8 +397,6 @@ public abstract class AbstractGeometryFieldMapper extends Fie return ((AbstractGeometryFieldType)fieldType).orientation(); } - protected abstract void indexShape(ParseContext context, Processed shape); - /** parsing logic for geometry indexing */ @Override public void parse(ParseContext context) throws IOException { @@ -413,7 +413,13 @@ public abstract class AbstractGeometryFieldMapper extends Fie } shape = geometryIndexer.prepareForIndexing(geometry); } - indexShape(context, shape); + + List fields = new ArrayList<>(); + fields.addAll(geometryIndexer.indexShape(context, shape)); + createFieldNamesField(context, fields); + for (IndexableField field : fields) { + context.doc().add(field); + } } catch (Exception e) { if (ignoreMalformed.value() == false) { throw new MapperParsingException("failed to parse field [{}] of type [{}]", e, fieldType().name(), 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 7bd5eba115a..d1e770eab3a 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java @@ -18,29 +18,14 @@ */ package org.elasticsearch.index.mapper; -import org.apache.lucene.document.Field; import org.apache.lucene.document.LatLonShape; -import org.apache.lucene.geo.Line; -import org.apache.lucene.geo.Polygon; -import org.apache.lucene.index.IndexableField; import org.elasticsearch.common.Explicit; import org.elasticsearch.common.geo.GeometryParser; import org.elasticsearch.common.geo.builders.ShapeBuilder; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.geo.geometry.Circle; import org.elasticsearch.geo.geometry.Geometry; -import org.elasticsearch.geo.geometry.GeometryCollection; -import org.elasticsearch.geo.geometry.GeometryVisitor; -import org.elasticsearch.geo.geometry.LinearRing; -import org.elasticsearch.geo.geometry.MultiLine; -import org.elasticsearch.geo.geometry.MultiPoint; -import org.elasticsearch.geo.geometry.MultiPolygon; -import org.elasticsearch.geo.geometry.Point; import org.elasticsearch.index.query.VectorGeoShapeQueryProcessor; -import java.util.ArrayList; -import java.util.Arrays; - /** * FieldMapper for indexing {@link LatLonShape}s. *

@@ -80,12 +65,14 @@ public class GeoShapeFieldMapper extends AbstractGeometryFieldMapper geometryParser.parse(parser)); - ((GeoShapeFieldType)fieldType()).setGeometryQueryBuilder(new VectorGeoShapeQueryProcessor()); + GeometryParser geometryParser = new GeometryParser(orientation, coerce(context).value(), ignoreZValue().value()); + + fieldType.setGeometryIndexer(new GeoShapeIndexer(orientation, fieldType.name())); + fieldType.setGeometryParser( (parser, mapper) -> geometryParser.parse(parser)); + fieldType.setGeometryQueryBuilder(new VectorGeoShapeQueryProcessor()); } } @@ -107,11 +94,6 @@ public class GeoShapeFieldMapper extends AbstractGeometryFieldMapper geometryIndexer() { - return new GeoShapeIndexer(orientation == ShapeBuilder.Orientation.RIGHT); - } } public GeoShapeFieldMapper(String simpleName, MappedFieldType fieldType, MappedFieldType defaultFieldType, @@ -127,103 +109,6 @@ public class GeoShapeFieldMapper extends AbstractGeometryFieldMapper { - private ParseContext context; - - private LuceneGeometryIndexer(ParseContext context) { - this.context = context; - } - - @Override - public Void visit(Circle circle) { - throw new IllegalArgumentException("invalid shape type found [Circle] while indexing shape"); - } - - @Override - public Void visit(GeometryCollection collection) { - for (Geometry geometry : collection) { - geometry.visit(this); - } - return null; - } - - @Override - public Void visit(org.elasticsearch.geo.geometry.Line line) { - indexFields(context, LatLonShape.createIndexableFields(name(), new Line(line.getLats(), line.getLons()))); - return null; - } - - @Override - public Void visit(LinearRing ring) { - throw new IllegalArgumentException("invalid shape type found [LinearRing] while indexing shape"); - } - - @Override - public Void visit(MultiLine multiLine) { - for (org.elasticsearch.geo.geometry.Line line : multiLine) { - visit(line); - } - return null; - } - - @Override - public Void visit(MultiPoint multiPoint) { - for(Point point : multiPoint) { - visit(point); - } - return null; - } - - @Override - public Void visit(MultiPolygon multiPolygon) { - for(org.elasticsearch.geo.geometry.Polygon polygon : multiPolygon) { - visit(polygon); - } - return null; - } - - @Override - public Void visit(Point point) { - indexFields(context, LatLonShape.createIndexableFields(name(), point.getLat(), point.getLon())); - return null; - } - - @Override - public Void visit(org.elasticsearch.geo.geometry.Polygon polygon) { - indexFields(context, LatLonShape.createIndexableFields(name(), toLucenePolygon(polygon))); - return null; - } - - @Override - public Void visit(org.elasticsearch.geo.geometry.Rectangle r) { - Polygon p = new Polygon(new double[]{r.getMinLat(), r.getMinLat(), r.getMaxLat(), r.getMaxLat(), r.getMinLat()}, - new double[]{r.getMinLon(), r.getMaxLon(), r.getMaxLon(), r.getMinLon(), r.getMinLon()}); - indexFields(context, LatLonShape.createIndexableFields(name(), p)); - return null; - } - } - - public static Polygon toLucenePolygon(org.elasticsearch.geo.geometry.Polygon polygon) { - Polygon[] holes = new Polygon[polygon.getNumberOfHoles()]; - for(int i = 0; i flist = new ArrayList<>(Arrays.asList(fields)); - createFieldNamesField(context, flist); - for (IndexableField f : flist) { - context.doc().add(f); - } - } - @Override protected String contentType() { return CONTENT_TYPE; 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 0880ad7390d..6e1264b0e19 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeIndexer.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeIndexer.java @@ -20,6 +20,8 @@ package org.elasticsearch.index.mapper; +import org.apache.lucene.document.LatLonShape; +import org.apache.lucene.index.IndexableField; import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.geo.geometry.Circle; import org.elasticsearch.geo.geometry.Geometry; @@ -60,8 +62,11 @@ public final class GeoShapeIndexer implements AbstractGeometryFieldMapper.Indexe private final boolean orientation; - public GeoShapeIndexer(boolean orientation) { + private final String name; + + public GeoShapeIndexer(boolean orientation, String name) { this.orientation = orientation; + this.name = name; } public Geometry prepareForIndexing(Geometry geometry) { @@ -181,6 +186,13 @@ public final class GeoShapeIndexer implements AbstractGeometryFieldMapper.Indexe return Geometry.class; } + @Override + public List indexShape(ParseContext context, Geometry shape) { + LuceneGeometryIndexer visitor = new LuceneGeometryIndexer(name); + shape.visit(visitor); + return visitor.fields(); + } + /** * Calculate the intersection of a line segment and a vertical dateline. * @@ -936,4 +948,99 @@ public final class GeoShapeIndexer implements AbstractGeometryFieldMapper.Indexe return points; } + + private static class LuceneGeometryIndexer implements GeometryVisitor { + private List fields = new ArrayList<>(); + private String name; + + private LuceneGeometryIndexer(String name) { + this.name = name; + } + + List fields() { + return fields; + } + + @Override + public Void visit(Circle circle) { + throw new IllegalArgumentException("invalid shape type found [Circle] while indexing shape"); + } + + @Override + public Void visit(GeometryCollection collection) { + for (Geometry geometry : collection) { + geometry.visit(this); + } + return null; + } + + @Override + public Void visit(org.elasticsearch.geo.geometry.Line line) { + addFields(LatLonShape.createIndexableFields(name, new org.apache.lucene.geo.Line(line.getLats(), line.getLons()))); + return null; + } + + @Override + public Void visit(LinearRing ring) { + throw new IllegalArgumentException("invalid shape type found [LinearRing] while indexing shape"); + } + + @Override + public Void visit(MultiLine multiLine) { + for (org.elasticsearch.geo.geometry.Line line : multiLine) { + visit(line); + } + return null; + } + + @Override + public Void visit(MultiPoint multiPoint) { + for(Point point : multiPoint) { + visit(point); + } + return null; + } + + @Override + public Void visit(MultiPolygon multiPolygon) { + for(org.elasticsearch.geo.geometry.Polygon polygon : multiPolygon) { + visit(polygon); + } + return null; + } + + @Override + public Void visit(Point point) { + addFields(LatLonShape.createIndexableFields(name, point.getLat(), point.getLon())); + return null; + } + + @Override + public Void visit(org.elasticsearch.geo.geometry.Polygon polygon) { + addFields(LatLonShape.createIndexableFields(name, toLucenePolygon(polygon))); + return null; + } + + @Override + public Void visit(org.elasticsearch.geo.geometry.Rectangle r) { + org.apache.lucene.geo.Polygon p = new org.apache.lucene.geo.Polygon( + new double[]{r.getMinLat(), r.getMinLat(), r.getMaxLat(), r.getMaxLat(), r.getMinLat()}, + new double[]{r.getMinLon(), r.getMaxLon(), r.getMaxLon(), r.getMinLon(), r.getMinLon()}); + addFields(LatLonShape.createIndexableFields(name, p)); + return null; + } + + private void addFields(IndexableField[] fields) { + this.fields.addAll(Arrays.asList(fields)); + } + } + + + public static org.apache.lucene.geo.Polygon toLucenePolygon(org.elasticsearch.geo.geometry.Polygon polygon) { + org.apache.lucene.geo.Polygon[] holes = new org.apache.lucene.geo.Polygon[polygon.getNumberOfHoles()]; + for(int i = 0; i shapes = ((XShapeCollection) shape).getShapes(); - for (Shape s : shapes) { - doIndexShape(context, 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"); - } - } - doIndexShape(context, shape); - } - - private void doIndexShape(ParseContext context, Shape shape) { - List fields = new ArrayList<>(Arrays.asList(fieldType().defaultPrefixTreeStrategy().createIndexableFields(shape))); - createFieldNamesField(context, fields); - for (IndexableField field : fields) { - context.doc().add(field); - } - } - @Override protected void doXContentBody(XContentBuilder builder, boolean includeDefaults, Params params) throws IOException { super.doXContentBody(builder, includeDefaults, params); diff --git a/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeIndexer.java b/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeIndexer.java index 80e3a505f5c..bccccb1e0c5 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeIndexer.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeIndexer.java @@ -19,10 +19,26 @@ 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(); @@ -32,4 +48,25 @@ public class LegacyGeoShapeIndexer implements AbstractGeometryFieldMapper.Indexe public Class processedClass() { return Shape.class; } + + @Override + public List indexShape(ParseContext context, Shape shape) { + if (fieldType.pointsOnly() == true) { + // 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/main/java/org/elasticsearch/index/query/VectorGeoShapeQueryProcessor.java b/server/src/main/java/org/elasticsearch/index/query/VectorGeoShapeQueryProcessor.java index 68772d0e4d2..5ca03a7fab3 100644 --- a/server/src/main/java/org/elasticsearch/index/query/VectorGeoShapeQueryProcessor.java +++ b/server/src/main/java/org/elasticsearch/index/query/VectorGeoShapeQueryProcessor.java @@ -43,7 +43,7 @@ import org.elasticsearch.index.mapper.GeoShapeFieldMapper; import org.elasticsearch.index.mapper.GeoShapeIndexer; import org.elasticsearch.index.mapper.MappedFieldType; -import static org.elasticsearch.index.mapper.GeoShapeFieldMapper.toLucenePolygon; +import static org.elasticsearch.index.mapper.GeoShapeIndexer.toLucenePolygon; public class VectorGeoShapeQueryProcessor implements AbstractGeometryFieldMapper.QueryProcessor { @@ -59,7 +59,7 @@ public class VectorGeoShapeQueryProcessor implements AbstractGeometryFieldMapper } protected Query getVectorQueryFromShape(Geometry queryShape, String fieldName, ShapeRelation relation, QueryShardContext context) { - GeoShapeIndexer geometryIndexer = new GeoShapeIndexer(true); + GeoShapeIndexer geometryIndexer = new GeoShapeIndexer(true, fieldName); Geometry processedShape = geometryIndexer.prepareForIndexing(queryShape); diff --git a/server/src/test/java/org/elasticsearch/common/geo/BaseGeoParsingTestCase.java b/server/src/test/java/org/elasticsearch/common/geo/BaseGeoParsingTestCase.java index 2a2f7ce75a3..313d12dc223 100644 --- a/server/src/test/java/org/elasticsearch/common/geo/BaseGeoParsingTestCase.java +++ b/server/src/test/java/org/elasticsearch/common/geo/BaseGeoParsingTestCase.java @@ -67,7 +67,7 @@ abstract class BaseGeoParsingTestCase extends ESTestCase { } else { GeometryParser geometryParser = new GeometryParser(true, true, true); org.elasticsearch.geo.geometry.Geometry shape = geometryParser.parse(parser); - shape = new GeoShapeIndexer(true).prepareForIndexing(shape); + shape = new GeoShapeIndexer(true, "name").prepareForIndexing(shape); ElasticsearchGeoAssertions.assertEquals(expected, shape); } } diff --git a/server/src/test/java/org/elasticsearch/common/geo/GeoJsonShapeParserTests.java b/server/src/test/java/org/elasticsearch/common/geo/GeoJsonShapeParserTests.java index 9b1f229e187..6cee71c25ae 100644 --- a/server/src/test/java/org/elasticsearch/common/geo/GeoJsonShapeParserTests.java +++ b/server/src/test/java/org/elasticsearch/common/geo/GeoJsonShapeParserTests.java @@ -1426,7 +1426,7 @@ public class GeoJsonShapeParserTests extends BaseGeoParsingTestCase { public Geometry parse(XContentParser parser) throws IOException, ParseException { GeometryParser geometryParser = new GeometryParser(true, true, true); - GeoShapeIndexer indexer = new GeoShapeIndexer(true); + GeoShapeIndexer indexer = new GeoShapeIndexer(true, "name"); return indexer.prepareForIndexing(geometryParser.parse(parser)); } } diff --git a/server/src/test/java/org/elasticsearch/common/geo/GeoWKTShapeParserTests.java b/server/src/test/java/org/elasticsearch/common/geo/GeoWKTShapeParserTests.java index 11177060596..57800236efe 100644 --- a/server/src/test/java/org/elasticsearch/common/geo/GeoWKTShapeParserTests.java +++ b/server/src/test/java/org/elasticsearch/common/geo/GeoWKTShapeParserTests.java @@ -471,7 +471,7 @@ public class GeoWKTShapeParserTests extends BaseGeoParsingTestCase { } else { GeometryCollectionBuilder gcb = RandomShapeGenerator.createGeometryCollection(random()); assertExpected(gcb.buildS4J(), gcb, true); - assertExpected(new GeoShapeIndexer(true).prepareForIndexing(gcb.buildGeometry()), gcb, false); + assertExpected(new GeoShapeIndexer(true, "name").prepareForIndexing(gcb.buildGeometry()), gcb, false); } } 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 12a3432eb36..cb79ebe4fd0 100644 --- a/server/src/test/java/org/elasticsearch/common/geo/GeometryIndexerTests.java +++ b/server/src/test/java/org/elasticsearch/common/geo/GeometryIndexerTests.java @@ -43,7 +43,7 @@ import java.util.Collections; public class GeometryIndexerTests extends ESTestCase { - GeoShapeIndexer indexer = new GeoShapeIndexer(true); + GeoShapeIndexer indexer = new GeoShapeIndexer(true, "test"); private static final WellKnownText WKT = new WellKnownText(true, geometry -> { }); @@ -209,13 +209,13 @@ public class GeometryIndexerTests extends ESTestCase { private Geometry actual(String wkt, boolean rightOrientation) throws IOException, ParseException { Geometry shape = parseGeometry(wkt, rightOrientation); - return new GeoShapeIndexer(true).prepareForIndexing(shape); + return new GeoShapeIndexer(true, "test").prepareForIndexing(shape); } private Geometry actual(XContentBuilder geoJson, boolean rightOrientation) throws IOException, ParseException { Geometry shape = parseGeometry(geoJson, rightOrientation); - return new GeoShapeIndexer(true).prepareForIndexing(shape); + return new GeoShapeIndexer(true, "test").prepareForIndexing(shape); } private Geometry parseGeometry(String wkt, boolean rightOrientation) throws IOException, ParseException { diff --git a/server/src/test/java/org/elasticsearch/common/geo/ShapeBuilderTests.java b/server/src/test/java/org/elasticsearch/common/geo/ShapeBuilderTests.java index 40ff3e8d44b..6022eaec3d5 100644 --- a/server/src/test/java/org/elasticsearch/common/geo/ShapeBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/common/geo/ShapeBuilderTests.java @@ -779,6 +779,6 @@ public class ShapeBuilderTests extends ESTestCase { } public Object buildGeometry(ShapeBuilder builder) { - return new GeoShapeIndexer(true).prepareForIndexing(builder.buildGeometry()); + return new GeoShapeIndexer(true, "name").prepareForIndexing(builder.buildGeometry()); } }