Geo: Fix Empty Geometry Collection Handling (#37978)

Fixes handling empty geometry collection and re-enables
testParseGeometryCollection test.

Fixes #37894
This commit is contained in:
Igor Motov 2019-01-30 09:20:30 -05:00 committed by GitHub
parent 53e80e9814
commit 23805fa41a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 65 additions and 20 deletions

View File

@ -121,6 +121,10 @@ public class WellKnownText {
@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());

View File

@ -27,6 +27,7 @@ import org.elasticsearch.common.geo.parsers.ShapeParser;
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 class GeometryCollectionBuilder extends ShapeBuilder<Shape,
@Override
public org.elasticsearch.geo.geometry.GeometryCollection<org.elasticsearch.geo.geometry.Geometry> buildGeometry() {
if (this.shapes.isEmpty()) {
return GeometryCollection.EMPTY;
}
List<org.elasticsearch.geo.geometry.Geometry> shapes = new ArrayList<>(this.shapes.size());
for (ShapeBuilder shape : this.shapes) {

View File

@ -151,6 +151,9 @@ public class MultiLineStringBuilder extends ShapeBuilder<JtsGeometry, org.elasti
@Override
public org.elasticsearch.geo.geometry.Geometry buildGeometry() {
if (lines.isEmpty()) {
return MultiLine.EMPTY;
}
if (wrapdateline) {
List<org.elasticsearch.geo.geometry.Line> parts = new ArrayList<>();
for (LineStringBuilder line : lines) {

View File

@ -45,6 +45,13 @@ public class MultiPointBuilder extends ShapeBuilder<XShapeCollection<Point>, Mul
super(coordinates);
}
/**
* Creates a new empty MultiPoint builder
*/
public MultiPointBuilder() {
super();
}
/**
* Read from a stream.
*/
@ -77,6 +84,9 @@ public class MultiPointBuilder extends ShapeBuilder<XShapeCollection<Point>, Mul
@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()));
}

View File

@ -198,6 +198,9 @@ public class MultiPolygonBuilder extends ShapeBuilder<Shape, MultiPolygon, Multi
shapes.add((org.elasticsearch.geo.geometry.Polygon)poly);
}
}
if (shapes.isEmpty()) {
return MultiPolygon.EMPTY;
}
return new MultiPolygon(shapes);
}

View File

@ -198,7 +198,7 @@ public class GeoWKTParser {
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 @@ public class GeoWKTParser {
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));

View File

@ -40,6 +40,7 @@ import org.elasticsearch.common.settings.Settings;
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,12 +113,32 @@ public class GeoWKTShapeParserTests extends BaseGeoParsingTestCase {
@Override
public void testParseMultiPoint() throws IOException {
int numPoints = randomIntBetween(2, 100);
int numPoints = randomIntBetween(0, 100);
List<Coordinate> coordinates = new ArrayList<>(numPoints);
for (int i = 0; i < numPoints; ++i) {
coordinates.add(new Coordinate(GeoTestUtil.nextLongitude(), GeoTestUtil.nextLatitude()));
}
List<org.elasticsearch.geo.geometry.Point> points = new ArrayList<>(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));
}
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);
@ -125,14 +146,6 @@ public class GeoWKTShapeParserTests extends BaseGeoParsingTestCase {
}
ShapeCollection<?> expected = shapeCollection(shapes);
assertExpected(expected, new MultiPointBuilder(coordinates), true);
List<org.elasticsearch.geo.geometry.Point> points = new ArrayList<>(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));
}
assertExpected(new MultiPoint(points), new MultiPointBuilder(coordinates), false);
assertMalformed(new MultiPointBuilder(coordinates));
}
private List<Coordinate> randomLineStringCoords() {
@ -163,7 +176,7 @@ public class GeoWKTShapeParserTests extends BaseGeoParsingTestCase {
@Override
public void testParseMultiLineString() throws IOException {
int numLineStrings = randomIntBetween(2, 8);
int numLineStrings = randomIntBetween(0, 8);
List<LineString> lineStrings = new ArrayList<>(numLineStrings);
MultiLineStringBuilder builder = new MultiLineStringBuilder();
for (int j = 0; j < numLineStrings; ++j) {
@ -173,18 +186,27 @@ public class GeoWKTShapeParserTests extends BaseGeoParsingTestCase {
builder.linestring(new LineStringBuilder(lsc));
}
MultiLineString expected = GEOMETRY_FACTORY.createMultiLineString(
lineStrings.toArray(new LineString[lineStrings.size()]));
assertExpected(jtsGeom(expected), builder, true);
List<Line> 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 class GeoWKTShapeParserTests extends BaseGeoParsingTestCase {
@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 class GeoWKTShapeParserTests extends BaseGeoParsingTestCase {
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 class GeoWKTShapeParserTests extends BaseGeoParsingTestCase {
assertValidException(builder, IllegalArgumentException.class);
}
@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/37894")
@Override
public void testParseGeometryCollection() throws IOException {
if (rarely()) {