From 0067a0cb7e54fcda166709d62d22e149756ca832 Mon Sep 17 00:00:00 2001 From: Nicholas Knize Date: Fri, 14 Nov 2014 10:28:30 -0600 Subject: [PATCH] Updating to throw IllegalArgument exception for null value coordinates. Tests included. --- .../common/geo/builders/ShapeBuilder.java | 2 +- .../common/geo/GeoJSONShapeParserTests.java | 12 +++++++----- .../test/hamcrest/ElasticsearchGeoAssertions.java | 7 ++++--- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/elasticsearch/common/geo/builders/ShapeBuilder.java b/src/main/java/org/elasticsearch/common/geo/builders/ShapeBuilder.java index e186e2bb9c082..edfee171fc2f9 100644 --- a/src/main/java/org/elasticsearch/common/geo/builders/ShapeBuilder.java +++ b/src/main/java/org/elasticsearch/common/geo/builders/ShapeBuilder.java @@ -216,7 +216,7 @@ private static CoordinateNode parseCoordinates(XContentParser parser) throws IOE token = parser.nextToken(); return new CoordinateNode(new Coordinate(lon, lat)); } else if (token == XContentParser.Token.VALUE_NULL) { - return null; + throw new ElasticsearchIllegalArgumentException("coordinates cannot contain NULL values)"); } List nodes = new ArrayList<>(); diff --git a/src/test/java/org/elasticsearch/common/geo/GeoJSONShapeParserTests.java b/src/test/java/org/elasticsearch/common/geo/GeoJSONShapeParserTests.java index 1b1505ec09ac4..e3b4eff2ecd4d 100644 --- a/src/test/java/org/elasticsearch/common/geo/GeoJSONShapeParserTests.java +++ b/src/test/java/org/elasticsearch/common/geo/GeoJSONShapeParserTests.java @@ -26,6 +26,8 @@ import com.spatial4j.core.shape.jts.JtsGeometry; import com.spatial4j.core.shape.jts.JtsPoint; import com.vividsolutions.jts.geom.*; +import org.elasticsearch.ElasticsearchIllegalArgumentException; +import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.common.geo.builders.ShapeBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentParser; @@ -172,7 +174,7 @@ public void testParse_invalidPolygon() throws IOException { .endObject().string(); XContentParser parser = JsonXContent.jsonXContent.createParser(invalidPoly1); parser.nextToken(); - ElasticsearchGeoAssertions.assertValidParseException(parser); + ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class); // test case 2: create an invalid polygon with only 1 point String invalidPoly2 = XContentFactory.jsonBuilder().startObject().field("type", "polygon") @@ -185,7 +187,7 @@ public void testParse_invalidPolygon() throws IOException { parser = JsonXContent.jsonXContent.createParser(invalidPoly2); parser.nextToken(); - ElasticsearchGeoAssertions.assertValidParseException(parser); + ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class); // test case 3: create an invalid polygon with 0 points String invalidPoly3 = XContentFactory.jsonBuilder().startObject().field("type", "polygon") @@ -198,7 +200,7 @@ public void testParse_invalidPolygon() throws IOException { parser = JsonXContent.jsonXContent.createParser(invalidPoly3); parser.nextToken(); - ElasticsearchGeoAssertions.assertValidParseException(parser); + ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class); // test case 4: create an invalid polygon with null value points String invalidPoly4 = XContentFactory.jsonBuilder().startObject().field("type", "polygon") @@ -211,7 +213,7 @@ public void testParse_invalidPolygon() throws IOException { parser = JsonXContent.jsonXContent.createParser(invalidPoly4); parser.nextToken(); - ElasticsearchGeoAssertions.assertValidParseException(parser); + ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchIllegalArgumentException.class); // test case 5: create an invalid polygon with 1 invalid LinearRing String invalidPoly5 = XContentFactory.jsonBuilder().startObject().field("type", "polygon") @@ -222,7 +224,7 @@ public void testParse_invalidPolygon() throws IOException { parser = JsonXContent.jsonXContent.createParser(invalidPoly5); parser.nextToken(); - ElasticsearchGeoAssertions.assertValidParseException(parser); + ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchIllegalArgumentException.class); } @Test diff --git a/src/test/java/org/elasticsearch/test/hamcrest/ElasticsearchGeoAssertions.java b/src/test/java/org/elasticsearch/test/hamcrest/ElasticsearchGeoAssertions.java index ac8efff76b989..7ba04cb9d7823 100644 --- a/src/test/java/org/elasticsearch/test/hamcrest/ElasticsearchGeoAssertions.java +++ b/src/test/java/org/elasticsearch/test/hamcrest/ElasticsearchGeoAssertions.java @@ -249,12 +249,13 @@ private static double distance(double lat1, double lon1, double lat2, double lon return GeoDistance.ARC.calculate(lat1, lon1, lat2, lon2, DistanceUnit.DEFAULT); } - public static void assertValidParseException(XContentParser parser) { + public static void assertValidException(XContentParser parser, Class expectedException) { try { ShapeBuilder.parse(parser); - Assert.fail("process completed successfully when parse exception expected"); + Assert.fail("process completed successfully when " + expectedException.getName() + " expected"); } catch (Exception e) { - assert(e instanceof ElasticsearchParseException): "expected ElasticsearchParse exception but found " + e.getClass().getName(); + assert(e.getClass().equals(expectedException)): + "expected " + expectedException.getName() + " but found " + e.getClass().getName(); } } }