Fix wrong NaN comparison (#61795) (#61811)

Fixes wrong NaN comparison in error message generator in GeoPolygonDecomposer and PolygonBuilder.

Supersedes #48207

Co-authored-by: Pedro Luiz Cabral Salomon Prado <pedroprado010@users.noreply.github.com>
This commit is contained in:
Igor Motov 2020-09-01 15:50:38 -04:00 committed by GitHub
parent 8613bde780
commit 48e53cca94
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 28 additions and 10 deletions

View File

@ -528,6 +528,7 @@ public class GeoPolygonDecomposer {
if (visitedEdge.containsKey(current.coordinate)) { if (visitedEdge.containsKey(current.coordinate)) {
partitionPoint[0] = current.coordinate.getX(); partitionPoint[0] = current.coordinate.getX();
partitionPoint[1] = current.coordinate.getY(); partitionPoint[1] = current.coordinate.getY();
partitionPoint[2] = current.coordinate.getZ();
if (connectedComponents > 0 && current.next != edge) { if (connectedComponents > 0 && current.next != edge) {
throw new InvalidShapeException("Shape contains more than one shared point"); throw new InvalidShapeException("Shape contains more than one shared point");
} }
@ -576,7 +577,7 @@ public class GeoPolygonDecomposer {
} }
// First and last coordinates must be equal // First and last coordinates must be equal
if (coordinates[0].equals(coordinates[coordinates.length - 1]) == false) { if (coordinates[0].equals(coordinates[coordinates.length - 1]) == false) {
if (partitionPoint[2] == Double.NaN) { if (Double.isNaN(partitionPoint[2])) {
throw new InvalidShapeException("Self-intersection at or near point [" throw new InvalidShapeException("Self-intersection at or near point ["
+ partitionPoint[0] + "," + partitionPoint[1] + "]"); + partitionPoint[0] + "," + partitionPoint[1] + "]");
} else { } else {

View File

@ -436,7 +436,7 @@ public class PolygonBuilder extends ShapeBuilder<JtsGeometry, org.elasticsearch.
} }
// First and last coordinates must be equal // First and last coordinates must be equal
if (coordinates[0].equals(coordinates[coordinates.length - 1]) == false) { if (coordinates[0].equals(coordinates[coordinates.length - 1]) == false) {
if (partitionPoint[2] == Double.NaN) { if (Double.isNaN(partitionPoint[2])) {
throw new InvalidShapeException("Self-intersection at or near point [" throw new InvalidShapeException("Self-intersection at or near point ["
+ partitionPoint[0] + "," + partitionPoint[1] + "]"); + partitionPoint[0] + "," + partitionPoint[1] + "]");
} else { } else {

View File

@ -19,6 +19,17 @@
package org.elasticsearch.common.geo; 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.apache.lucene.index.IndexableField;
import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentFactory;
@ -37,14 +48,7 @@ import org.elasticsearch.geometry.Polygon;
import org.elasticsearch.geometry.Rectangle; import org.elasticsearch.geometry.Rectangle;
import org.elasticsearch.index.mapper.GeoShapeIndexer; import org.elasticsearch.index.mapper.GeoShapeIndexer;
import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.ESTestCase;
import java.io.IOException; import org.locationtech.spatial4j.exception.InvalidShapeException;
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.instanceOf;
public class GeometryIndexerTests extends ESTestCase { public class GeometryIndexerTests extends ESTestCase {
@ -357,6 +361,15 @@ public class GeometryIndexerTests extends ESTestCase {
actual(polygon(randomBoolean() ? null : randomBoolean(), 20, 0, 20, 10, -20, 10, -20, 0, 20, 0), randomBoolean())); actual(polygon(randomBoolean() ? null : randomBoolean(), 20, 0, 20, 10, -20, 10, -20, 0, 20, 0), randomBoolean()));
} }
public void testInvalidSelfCrossingPolygon() {
Polygon polygon = new Polygon(new LinearRing(
new double[]{0, 0, 1, 0.5, 1.5, 1, 2, 2, 0}, new double[]{0, 2, 1.9, 1.8, 1.8, 1.9, 2, 0, 0}
));
Exception e = expectThrows(InvalidShapeException.class, () -> indexer.prepareForIndexing(polygon));
assertThat(e.getMessage(), containsString("Self-intersection at or near point ["));
assertThat(e.getMessage(), not(containsString("NaN")));
}
private XContentBuilder polygon(Boolean orientation, double... val) throws IOException { private XContentBuilder polygon(Boolean orientation, double... val) throws IOException {
XContentBuilder pointGeoJson = XContentFactory.jsonBuilder().startObject(); XContentBuilder pointGeoJson = XContentFactory.jsonBuilder().startObject();
{ {

View File

@ -43,6 +43,8 @@ import static org.elasticsearch.test.hamcrest.ElasticsearchGeoAssertions.assertM
import static org.elasticsearch.test.hamcrest.ElasticsearchGeoAssertions.assertMultiPolygon; import static org.elasticsearch.test.hamcrest.ElasticsearchGeoAssertions.assertMultiPolygon;
import static org.elasticsearch.test.hamcrest.ElasticsearchGeoAssertions.assertPolygon; import static org.elasticsearch.test.hamcrest.ElasticsearchGeoAssertions.assertPolygon;
import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.not;
/** /**
* Tests for {@link ShapeBuilder} * Tests for {@link ShapeBuilder}
*/ */
@ -775,8 +777,10 @@ public class ShapeBuilderTests extends ESTestCase {
); );
Exception e = expectThrows(InvalidShapeException.class, () -> builder.close().buildS4J()); Exception e = expectThrows(InvalidShapeException.class, () -> builder.close().buildS4J());
assertThat(e.getMessage(), containsString("Self-intersection at or near point [")); assertThat(e.getMessage(), containsString("Self-intersection at or near point ["));
assertThat(e.getMessage(), not(containsString("NaN")));
e = expectThrows(InvalidShapeException.class, () -> buildGeometry(builder.close())); e = expectThrows(InvalidShapeException.class, () -> buildGeometry(builder.close()));
assertThat(e.getMessage(), containsString("Self-intersection at or near point [")); assertThat(e.getMessage(), containsString("Self-intersection at or near point ["));
assertThat(e.getMessage(), not(containsString("NaN")));
} }
public Object buildGeometry(ShapeBuilder<?, ?, ?> builder) { public Object buildGeometry(ShapeBuilder<?, ?, ?> builder) {