diff --git a/libs/geo/src/main/java/org/elasticsearch/geo/utils/WellKnownText.java b/libs/geo/src/main/java/org/elasticsearch/geo/utils/WellKnownText.java index 5fa585be28b24..c6e96d9bdf3ce 100644 --- a/libs/geo/src/main/java/org/elasticsearch/geo/utils/WellKnownText.java +++ b/libs/geo/src/main/java/org/elasticsearch/geo/utils/WellKnownText.java @@ -121,6 +121,10 @@ public Void visit(MultiLine multiLine) { @Override public Void visit(MultiPoint multiPoint) { + if (multiPoint.isEmpty()) { + sb.append(EMPTY); + return null; + } // walk through coordinates: sb.append(LPAREN); visitPoint(multiPoint.get(0).getLon(), multiPoint.get(0).getLat()); diff --git a/server/src/main/java/org/elasticsearch/common/geo/builders/GeometryCollectionBuilder.java b/server/src/main/java/org/elasticsearch/common/geo/builders/GeometryCollectionBuilder.java index fb3ff6203ed45..fb0cacacfae84 100644 --- a/server/src/main/java/org/elasticsearch/common/geo/builders/GeometryCollectionBuilder.java +++ b/server/src/main/java/org/elasticsearch/common/geo/builders/GeometryCollectionBuilder.java @@ -27,6 +27,7 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.geo.geometry.GeometryCollection; import org.locationtech.spatial4j.shape.Shape; import java.io.IOException; @@ -186,6 +187,9 @@ public Shape buildS4J() { @Override public org.elasticsearch.geo.geometry.GeometryCollection buildGeometry() { + if (this.shapes.isEmpty()) { + return GeometryCollection.EMPTY; + } List shapes = new ArrayList<>(this.shapes.size()); for (ShapeBuilder shape : this.shapes) { diff --git a/server/src/main/java/org/elasticsearch/common/geo/builders/MultiLineStringBuilder.java b/server/src/main/java/org/elasticsearch/common/geo/builders/MultiLineStringBuilder.java index a283cda874528..24a8b3b226f36 100644 --- a/server/src/main/java/org/elasticsearch/common/geo/builders/MultiLineStringBuilder.java +++ b/server/src/main/java/org/elasticsearch/common/geo/builders/MultiLineStringBuilder.java @@ -151,6 +151,9 @@ public JtsGeometry buildS4J() { @Override public org.elasticsearch.geo.geometry.Geometry buildGeometry() { + if (lines.isEmpty()) { + return MultiLine.EMPTY; + } if (wrapdateline) { List parts = new ArrayList<>(); for (LineStringBuilder line : lines) { diff --git a/server/src/main/java/org/elasticsearch/common/geo/builders/MultiPointBuilder.java b/server/src/main/java/org/elasticsearch/common/geo/builders/MultiPointBuilder.java index c92d67e8291ea..360447a963e80 100644 --- a/server/src/main/java/org/elasticsearch/common/geo/builders/MultiPointBuilder.java +++ b/server/src/main/java/org/elasticsearch/common/geo/builders/MultiPointBuilder.java @@ -45,6 +45,13 @@ public MultiPointBuilder(List coordinates) { super(coordinates); } + /** + * Creates a new empty MultiPoint builder + */ + public MultiPointBuilder() { + super(); + } + /** * Read from a stream. */ @@ -77,6 +84,9 @@ public XShapeCollection buildS4J() { @Override public MultiPoint buildGeometry() { + if (coordinates.isEmpty()) { + return MultiPoint.EMPTY; + } return new MultiPoint(coordinates.stream().map(coord -> new org.elasticsearch.geo.geometry.Point(coord.y, coord.x)) .collect(Collectors.toList())); } diff --git a/server/src/main/java/org/elasticsearch/common/geo/builders/MultiPolygonBuilder.java b/server/src/main/java/org/elasticsearch/common/geo/builders/MultiPolygonBuilder.java index be0741306c097..466f96c78ec8d 100644 --- a/server/src/main/java/org/elasticsearch/common/geo/builders/MultiPolygonBuilder.java +++ b/server/src/main/java/org/elasticsearch/common/geo/builders/MultiPolygonBuilder.java @@ -198,6 +198,9 @@ public MultiPolygon buildGeometry() { shapes.add((org.elasticsearch.geo.geometry.Polygon)poly); } } + if (shapes.isEmpty()) { + return MultiPolygon.EMPTY; + } return new MultiPolygon(shapes); } diff --git a/server/src/main/java/org/elasticsearch/common/geo/parsers/GeoWKTParser.java b/server/src/main/java/org/elasticsearch/common/geo/parsers/GeoWKTParser.java index bf26980c92651..2cffa417246fd 100644 --- a/server/src/main/java/org/elasticsearch/common/geo/parsers/GeoWKTParser.java +++ b/server/src/main/java/org/elasticsearch/common/geo/parsers/GeoWKTParser.java @@ -198,7 +198,7 @@ private static MultiPointBuilder parseMultiPoint(StreamTokenizer stream, final b throws IOException, ElasticsearchParseException { String token = nextEmptyOrOpen(stream); if (token.equals(EMPTY)) { - return null; + return new MultiPointBuilder(); } return new MultiPointBuilder(parseCoordinateList(stream, ignoreZValue, coerce)); } @@ -242,7 +242,7 @@ private static MultiLineStringBuilder parseMultiLine(StreamTokenizer stream, fin throws IOException, ElasticsearchParseException { String token = nextEmptyOrOpen(stream); if (token.equals(EMPTY)) { - return null; + return new MultiLineStringBuilder(); } MultiLineStringBuilder builder = new MultiLineStringBuilder(); builder.linestring(parseLine(stream, ignoreZValue, coerce)); 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 6518e05cf330c..286e1ce6ee7c5 100644 --- a/server/src/test/java/org/elasticsearch/common/geo/GeoWKTShapeParserTests.java +++ b/server/src/test/java/org/elasticsearch/common/geo/GeoWKTShapeParserTests.java @@ -40,6 +40,7 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.geo.geometry.Geometry; import org.elasticsearch.geo.geometry.Line; import org.elasticsearch.geo.geometry.MultiLine; import org.elasticsearch.geo.geometry.MultiPoint; @@ -112,27 +113,39 @@ public void testParsePoint() throws IOException { @Override public void testParseMultiPoint() throws IOException { - int numPoints = randomIntBetween(2, 100); + int numPoints = randomIntBetween(0, 100); List coordinates = new ArrayList<>(numPoints); for (int i = 0; i < numPoints; ++i) { coordinates.add(new Coordinate(GeoTestUtil.nextLongitude(), GeoTestUtil.nextLatitude())); } - Shape[] shapes = new Shape[numPoints]; + List points = new ArrayList<>(numPoints); for (int i = 0; i < numPoints; ++i) { Coordinate c = coordinates.get(i); - shapes[i] = SPATIAL_CONTEXT.makePoint(c.x, c.y); + points.add(new org.elasticsearch.geo.geometry.Point(c.y, c.x)); } - ShapeCollection expected = shapeCollection(shapes); - assertExpected(expected, new MultiPointBuilder(coordinates), true); - List points = new ArrayList<>(numPoints); + Geometry expectedGeom; + MultiPointBuilder actual; + if (numPoints == 0) { + expectedGeom = MultiPoint.EMPTY; + actual = new MultiPointBuilder(); + } else { + expectedGeom = new MultiPoint(points); + actual = new MultiPointBuilder(coordinates); + } + + assertExpected(expectedGeom, actual, false); + assertMalformed(actual); + + assumeTrue("JTS test path cannot handle empty multipoints", numPoints > 1); + Shape[] shapes = new Shape[numPoints]; for (int i = 0; i < numPoints; ++i) { Coordinate c = coordinates.get(i); - points.add(new org.elasticsearch.geo.geometry.Point(c.y, c.x)); + shapes[i] = SPATIAL_CONTEXT.makePoint(c.x, c.y); } - assertExpected(new MultiPoint(points), new MultiPointBuilder(coordinates), false); - assertMalformed(new MultiPointBuilder(coordinates)); + ShapeCollection expected = shapeCollection(shapes); + assertExpected(expected, new MultiPointBuilder(coordinates), true); } private List randomLineStringCoords() { @@ -163,7 +176,7 @@ public void testParseLineString() throws IOException { @Override public void testParseMultiLineString() throws IOException { - int numLineStrings = randomIntBetween(2, 8); + int numLineStrings = randomIntBetween(0, 8); List lineStrings = new ArrayList<>(numLineStrings); MultiLineStringBuilder builder = new MultiLineStringBuilder(); for (int j = 0; j < numLineStrings; ++j) { @@ -173,18 +186,27 @@ public void testParseMultiLineString() throws IOException { builder.linestring(new LineStringBuilder(lsc)); } - MultiLineString expected = GEOMETRY_FACTORY.createMultiLineString( - lineStrings.toArray(new LineString[lineStrings.size()])); - assertExpected(jtsGeom(expected), builder, true); - List lines = new ArrayList<>(lineStrings.size()); for (int j = 0; j < lineStrings.size(); ++j) { Coordinate[] c = lineStrings.get(j).getCoordinates(); lines.add(new Line(Arrays.stream(c).mapToDouble(i->i.y).toArray(), Arrays.stream(c).mapToDouble(i->i.x).toArray())); } - assertExpected(new MultiLine(lines), builder, false); + Geometry expectedGeom; + if (lines.isEmpty()) { + expectedGeom = MultiLine.EMPTY; + } else if (lines.size() == 1) { + expectedGeom = new Line(lines.get(0).getLats(), lines.get(0).getLons()); + } else { + expectedGeom = new MultiLine(lines); + } + assertExpected(expectedGeom, builder, false); assertMalformed(builder); + + MultiLineString expected = GEOMETRY_FACTORY.createMultiLineString( + lineStrings.toArray(new LineString[lineStrings.size()])); + assumeTrue("JTS test path cannot handle empty multilinestrings", numLineStrings > 1); + assertExpected(jtsGeom(expected), builder, true); } @Override @@ -201,7 +223,7 @@ public void testParsePolygon() throws IOException { @Override public void testParseMultiPolygon() throws IOException { - int numPolys = randomIntBetween(2, 8); + int numPolys = randomIntBetween(0, 8); MultiPolygonBuilder builder = new MultiPolygonBuilder(); PolygonBuilder pb; Coordinate[] coordinates; @@ -214,7 +236,7 @@ public void testParseMultiPolygon() throws IOException { shell = GEOMETRY_FACTORY.createLinearRing(coordinates); shapes[i] = GEOMETRY_FACTORY.createPolygon(shell, null); } - + assumeTrue("JTS test path cannot handle empty multipolygon", numPolys > 1); Shape expected = shapeCollection(shapes); assertExpected(expected, builder, true); assertMalformed(builder); @@ -429,7 +451,6 @@ public void testInvalidGeometryType() throws IOException { assertValidException(builder, IllegalArgumentException.class); } - @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/37894") @Override public void testParseGeometryCollection() throws IOException { if (rarely()) {