Query DSL: Enforce distance is greater than 0 in geo distance query

Validation is not done as part of the distance setter method and tested in GeoDistanceQueryBuilderTests. Fixed GeoDistanceTests to adapt to the new validation.

Closes #15135

core/src/main/java/org/elasticsearch/index/query/GeoDistanceQueryBuilder.java

@@ -128,7 +128,11 @@ public class GeoDistanceQueryBuilder extends AbstractQueryBuilder<GeoDistanceQue
         if (unit == null) {
             throw new IllegalArgumentException("distance unit must not be null");
         }
-        this.distance = DistanceUnit.parse(distance, unit, DistanceUnit.DEFAULT);
+        double newDistance = DistanceUnit.parse(distance, unit, DistanceUnit.DEFAULT);
+        if (newDistance <= 0.0) {
+            throw new IllegalArgumentException("distance must be greater than zero");
+        }
+        this.distance = newDistance;
         return this;
     }
 
@@ -172,7 +176,7 @@ public class GeoDistanceQueryBuilder extends AbstractQueryBuilder<GeoDistanceQue
      **/
     public GeoDistanceQueryBuilder optimizeBbox(String optimizeBbox) {
         if (optimizeBbox == null) {
-            throw new IllegalArgumentException("optimizeBox must not be null");
+            throw new IllegalArgumentException("optimizeBbox must not be null");
         }
         switch (optimizeBbox) {
             case "none":

core/src/test/java/org/elasticsearch/index/query/GeoDistanceQueryBuilderTests.java

@@ -33,9 +33,7 @@ import org.elasticsearch.test.geo.RandomShapeGenerator;
 
 import java.io.IOException;
 
-import static org.hamcrest.Matchers.closeTo;
+import static org.hamcrest.Matchers.*;
-import static org.hamcrest.Matchers.equalTo;
-import static org.hamcrest.Matchers.instanceOf;
 
 public class GeoDistanceQueryBuilderTests extends AbstractQueryTestCase<GeoDistanceQueryBuilder> {
 
@@ -86,7 +84,7 @@ public class GeoDistanceQueryBuilderTests extends AbstractQueryTestCase<GeoDista
             }
             fail("must not be null or empty");
         } catch (IllegalArgumentException ex) {
-            // expected
+            assertThat(ex.getMessage(), equalTo("fieldName must not be null or empty"));
         }
 
         GeoDistanceQueryBuilder query = new GeoDistanceQueryBuilder("fieldName");
@@ -98,7 +96,7 @@ public class GeoDistanceQueryBuilderTests extends AbstractQueryTestCase<GeoDista
             }
             fail("must not be null or empty");
         } catch (IllegalArgumentException ex) {
-            // expected
+            assertThat(ex.getMessage(), equalTo("distance must not be null or empty"));
         }
 
         try {
@@ -107,44 +105,52 @@ public class GeoDistanceQueryBuilderTests extends AbstractQueryTestCase<GeoDista
             } else {
                 query.distance(null, DistanceUnit.DEFAULT);
             }
-            fail("must not be null or empty");
+            fail("distance must not be null or empty");
         } catch (IllegalArgumentException ex) {
-            // expected
+            assertThat(ex.getMessage(), equalTo("distance must not be null or empty"));
         }
 
         try {
+            if (randomBoolean()) {
                 query.distance("1", null);
-            fail("unit must not be null");
+            } else {
-        } catch (IllegalArgumentException ex) {
-            // expected
-        }
-
-        try {
                 query.distance(1, null);
-            fail("unit must not be null");
+            }
+            fail("distance must not be null");
         } catch (IllegalArgumentException ex) {
-            // expected
+            assertThat(ex.getMessage(), equalTo("distance unit must not be null"));
         }
 
         try {
+            query.distance(randomIntBetween(Integer.MIN_VALUE, 0), DistanceUnit.DEFAULT);
+            fail("distance must be greater than zero");
+        } catch (IllegalArgumentException ex) {
+            assertThat(ex.getMessage(), equalTo("distance must be greater than zero"));
+        }
+
+        try {
+            if (randomBoolean()) {
                 query.geohash(null);
+            } else {
+                query.geohash("");
+            }
             fail("geohash must not be null");
         } catch (IllegalArgumentException ex) {
-            // expected
+            assertThat(ex.getMessage(), equalTo("geohash must not be null or empty"));
         }
 
         try {
             query.geoDistance(null);
             fail("geodistance must not be null");
         } catch (IllegalArgumentException ex) {
-            // expected
+            assertThat(ex.getMessage(), equalTo("geoDistance must not be null"));
         }
 
         try {
             query.optimizeBbox(null);
             fail("optimizeBbox must not be null");
         } catch (IllegalArgumentException ex) {
-            // expected
+            assertThat(ex.getMessage(), equalTo("optimizeBbox must not be null"));
         }
     }
 

plugins/lang-groovy/src/test/java/org/elasticsearch/messy/tests/GeoDistanceTests.java

@@ -739,7 +739,7 @@ public class GeoDistanceTests extends ESIntegTestCase {
         for (int i = 0; i < 10; ++i) {
             final double originLat = randomLat();
             final double originLon = randomLon();
-            final String distance = DistanceUnit.KILOMETERS.toString(randomInt(10000));
+            final String distance = DistanceUnit.KILOMETERS.toString(randomIntBetween(1, 10000));
             for (GeoDistance geoDistance : Arrays.asList(GeoDistance.ARC, GeoDistance.SLOPPY_ARC)) {
                 logger.info("Now testing GeoDistance={}, distance={}, origin=({}, {})", geoDistance, distance, originLat, originLon);
                 GeoDistanceQueryBuilder qb = QueryBuilders.geoDistanceQuery("location").point(originLat, originLon).distance(distance).geoDistance(geoDistance);