Allow using distance measure in the geo context precision

server/src/main/java/org/elasticsearch/common/geo/GeoUtils.java

@@ -24,10 +24,10 @@
 import org.apache.lucene.spatial.prefix.tree.QuadPrefixTree;
 import org.apache.lucene.util.SloppyMath;
 import org.elasticsearch.ElasticsearchParseException;
-import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.unit.DistanceUnit;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.common.xcontent.XContentParser.Token;
+import org.elasticsearch.common.xcontent.support.XContentMapValues;
 import org.elasticsearch.index.fielddata.FieldData;
 import org.elasticsearch.index.fielddata.GeoPointValues;
 import org.elasticsearch.index.fielddata.MultiGeoPointValues;
@@ -459,6 +459,50 @@ public static GeoPoint parseGeoPoint(XContentParser parser, GeoPoint point, fina
         }
     }
 
+    /**
+     * Parse a precision that can be expressed as an integer or a distance measure like "1km", "10m".
+     *
+     * @param parser {@link XContentParser} to parse the value from
+     * @return int representing precision
+     */
+    public static int parsePrecision(XContentParser parser) throws IOException, ElasticsearchParseException {
+        XContentParser.Token token = parser.currentToken();
+        if (token.equals(XContentParser.Token.VALUE_NUMBER)) {
+            return XContentMapValues.nodeIntegerValue(parser.intValue());
+        } else {
+            String precision = parser.text();
+            try {
+                // we want to treat simple integer strings as precision levels, not distances
+                return XContentMapValues.nodeIntegerValue(Integer.parseInt(precision));
+            } catch (NumberFormatException e) {
+                // try to parse as a distance value
+                try {
+                    return checkPrecisionRange(GeoUtils.geoHashLevelsForPrecision(precision));
+                } catch (NumberFormatException e2) {
+                    // can happen when distance unit is unknown, in this case we simply want to know the reason
+                    throw e2;
+                } catch (IllegalArgumentException e3) {
+                    // this happens when distance too small, so precision > 12. We'd like to see the original string
+                    throw new IllegalArgumentException("precision too high [" + precision + "]", e3);
+                }
+            }
+        }
+    }
+
+    /**
+     * Checks that the precision is within range supported by elasticsearch - between 1 and 12
+     *
+     * Returns the precision value if it is in the range and throws an IllegalArgumentException if it
+     * is outside the range.
+     */
+    public static int checkPrecisionRange(int precision) {
+        if ((precision < 1) || (precision > 12)) {
+            throw new IllegalArgumentException("Invalid geohash aggregation precision of " + precision
+                + ". Must be between 1 and 12.");
+        }
+        return precision;
+    }
+
     /** Returns the maximum distance/radius (in meters) from the point 'center' before overlapping */
     public static double maxRadialDistanceMeters(final double centerLat, final double centerLon) {
       if (Math.abs(centerLat) == MAX_LAT) {

server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoGridAggregationBuilder.java

@@ -54,6 +54,8 @@
 import java.util.Map;
 import java.util.Objects;
 
+import static org.elasticsearch.common.geo.GeoUtils.parsePrecision;
+
 public class GeoGridAggregationBuilder extends ValuesSourceAggregationBuilder<ValuesSource.GeoPoint, GeoGridAggregationBuilder>
         implements MultiBucketAggregationBuilder {
     public static final String NAME = "geohash_grid";
@@ -64,29 +66,8 @@ public class GeoGridAggregationBuilder extends ValuesSourceAggregationBuilder<Va
     static {
         PARSER = new ObjectParser<>(GeoGridAggregationBuilder.NAME);
         ValuesSourceParserHelper.declareGeoFields(PARSER, false, false);
-        PARSER.declareField((parser, builder, context) -> {
-            XContentParser.Token token = parser.currentToken();
-            if (token.equals(XContentParser.Token.VALUE_NUMBER)) {
-                builder.precision(XContentMapValues.nodeIntegerValue(parser.intValue()));
-            } else {
-                String precision = parser.text();
-                try {
-                    // we want to treat simple integer strings as precision levels, not distances
-                    builder.precision(XContentMapValues.nodeIntegerValue(Integer.parseInt(precision)));
-                } catch (NumberFormatException e) {
-                    // try to parse as a distance value
-                    try {
-                        builder.precision(GeoUtils.geoHashLevelsForPrecision(precision));
-                    } catch (NumberFormatException e2) {
-                        // can happen when distance unit is unknown, in this case we simply want to know the reason
-                        throw e2;
-                    } catch (IllegalArgumentException e3) {
-                        // this happens when distance too small, so precision > 12. We'd like to see the original string
-                        throw new IllegalArgumentException("precision too high [" + precision + "]", e3);
-                    }
-                }
-            }
-        }, GeoHashGridParams.FIELD_PRECISION, org.elasticsearch.common.xcontent.ObjectParser.ValueType.INT);
+        PARSER.declareField((parser, builder, context) -> builder.precision(parsePrecision(parser)), GeoHashGridParams.FIELD_PRECISION,
+            org.elasticsearch.common.xcontent.ObjectParser.ValueType.INT);
         PARSER.declareInt(GeoGridAggregationBuilder::size, GeoHashGridParams.FIELD_SIZE);
         PARSER.declareInt(GeoGridAggregationBuilder::shardSize, GeoHashGridParams.FIELD_SHARD_SIZE);
     }
@@ -133,7 +114,7 @@ protected void innerWriteTo(StreamOutput out) throws IOException {
     }
 
     public GeoGridAggregationBuilder precision(int precision) {
-        this.precision = GeoHashGridParams.checkPrecision(precision);
+        this.precision = GeoUtils.checkPrecisionRange(precision);
         return this;
     }
 

server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridParams.java

@@ -30,15 +30,6 @@ final class GeoHashGridParams {
     static final ParseField FIELD_SIZE = new ParseField("size");
     static final ParseField FIELD_SHARD_SIZE = new ParseField("shard_size");
 
-
-    static int checkPrecision(int precision) {
-        if ((precision < 1) || (precision > 12)) {
-            throw new IllegalArgumentException("Invalid geohash aggregation precision of " + precision
-                    + ". Must be between 1 and 12.");
-        }
-        return precision;
-    }
-
     private GeoHashGridParams() {
         throw new AssertionError("No instances intended");
     }

server/src/main/java/org/elasticsearch/search/suggest/completion/context/GeoQueryContext.java

@@ -33,6 +33,7 @@
 import java.util.List;
 import java.util.Objects;
 
+import static org.elasticsearch.common.geo.GeoUtils.parsePrecision;
 import static org.elasticsearch.search.suggest.completion.context.GeoContextMapping.CONTEXT_BOOST;
 import static org.elasticsearch.search.suggest.completion.context.GeoContextMapping.CONTEXT_NEIGHBOURS;
 import static org.elasticsearch.search.suggest.completion.context.GeoContextMapping.CONTEXT_PRECISION;
@@ -115,10 +116,10 @@ public static Builder builder() {
     static {
         GEO_CONTEXT_PARSER.declareField((parser, geoQueryContext, geoContextMapping) -> geoQueryContext.setGeoPoint(GeoUtils.parseGeoPoint(parser)), new ParseField(CONTEXT_VALUE), ObjectParser.ValueType.OBJECT);
         GEO_CONTEXT_PARSER.declareInt(GeoQueryContext.Builder::setBoost, new ParseField(CONTEXT_BOOST));
-        // TODO : add string support for precision for GeoUtils.geoHashLevelsForPrecision()
-        GEO_CONTEXT_PARSER.declareInt(GeoQueryContext.Builder::setPrecision, new ParseField(CONTEXT_PRECISION));
-        // TODO : add string array support for precision for GeoUtils.geoHashLevelsForPrecision()
-        GEO_CONTEXT_PARSER.declareIntArray(GeoQueryContext.Builder::setNeighbours, new ParseField(CONTEXT_NEIGHBOURS));
+        GEO_CONTEXT_PARSER.declareField((parser, builder, context) -> builder.setPrecision(parsePrecision(parser)),
+            new ParseField(CONTEXT_PRECISION), ObjectParser.ValueType.INT);
+        GEO_CONTEXT_PARSER.declareFieldArray(GeoQueryContext.Builder::setNeighbours, (parser, builder) -> parsePrecision(parser),
+            new ParseField(CONTEXT_NEIGHBOURS), ObjectParser.ValueType.INT_ARRAY);
         GEO_CONTEXT_PARSER.declareDouble(GeoQueryContext.Builder::setLat, new ParseField("lat"));
         GEO_CONTEXT_PARSER.declareDouble(GeoQueryContext.Builder::setLon, new ParseField("lon"));
     }

server/src/test/java/org/elasticsearch/common/geo/GeoUtilTests.java

@@ -0,0 +1,71 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.elasticsearch.common.geo;
+
+import org.elasticsearch.common.CheckedConsumer;
+import org.elasticsearch.common.bytes.BytesReference;
+import org.elasticsearch.common.xcontent.XContentBuilder;
+import org.elasticsearch.common.xcontent.XContentParser;
+import org.elasticsearch.common.xcontent.json.JsonXContent;
+import org.elasticsearch.test.ESTestCase;
+
+import java.io.IOException;
+
+import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
+
+public class GeoUtilTests extends ESTestCase {
+
+    public void testPrecisionParser() throws IOException {
+        assertEquals(10, parsePrecision(builder -> builder.field("test", 10)));
+        assertEquals(10, parsePrecision(builder -> builder.field("test", 10.2)));
+        assertEquals(6, parsePrecision(builder -> builder.field("test", "6")));
+        assertEquals(7, parsePrecision(builder -> builder.field("test", "1km")));
+        assertEquals(7, parsePrecision(builder -> builder.field("test", "1.1km")));
+    }
+
+    public void testIncorrectPrecisionParser() {
+        expectThrows(NumberFormatException.class, () -> parsePrecision(builder -> builder.field("test", "10.1.1.1")));
+        expectThrows(NumberFormatException.class, () -> parsePrecision(builder -> builder.field("test", "364.4smoots")));
+        assertEquals(
+            "precision too high [0.01mm]",
+            expectThrows(IllegalArgumentException.class, () -> parsePrecision(builder -> builder.field("test", "0.01mm"))).getMessage()
+        );
+    }
+
+    /**
+     * Invokes GeoUtils.parsePrecision parser on the value generated by tokenGenerator
+     * <p>
+     * The supplied tokenGenerator should generate a single field that contains the precision in
+     * one of the supported formats or malformed precision value if error handling is tested. The
+     * method return the parsed value or throws an exception, if precision value is malformed.
+     */
+    private int parsePrecision(CheckedConsumer<XContentBuilder, IOException> tokenGenerator) throws IOException {
+        XContentBuilder builder = jsonBuilder().startObject();
+        tokenGenerator.accept(builder);
+        builder.endObject();
+        XContentParser parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(builder));
+        assertEquals(XContentParser.Token.START_OBJECT, parser.nextToken()); // {
+        assertEquals(XContentParser.Token.FIELD_NAME, parser.nextToken()); // field name
+        assertTrue(parser.nextToken().isValue()); // field value
+        int precision = GeoUtils.parsePrecision(parser);
+        assertEquals(XContentParser.Token.END_OBJECT, parser.nextToken()); // }
+        assertNull(parser.nextToken()); // no more tokens
+        return precision;
+    }
+}

server/src/test/java/org/elasticsearch/search/suggest/completion/GeoQueryContextTests.java

@@ -19,15 +19,20 @@
 
 package org.elasticsearch.search.suggest.completion;
 
+import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.geo.GeoPoint;
+import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentParser;
+import org.elasticsearch.common.xcontent.json.JsonXContent;
 import org.elasticsearch.search.suggest.completion.context.GeoQueryContext;
 
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 
+import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
 import static org.hamcrest.Matchers.equalTo;
 
 public class GeoQueryContextTests extends QueryContextTestCase<GeoQueryContext> {
@@ -105,4 +110,36 @@ public void testIllegalArguments() {
             assertEquals(e.getMessage(), "neighbour value must be between 1 and 12");
         }
     }
+
+    public void testStringPrecision() throws IOException {
+        XContentBuilder builder = jsonBuilder().startObject();
+        {
+            builder.startObject("context").field("lat", 23.654242).field("lon", 90.047153).endObject();
+            builder.field("boost", 10);
+            builder.field("precision", 12);
+            builder.array("neighbours", 1, 2);
+        }
+        builder.endObject();
+        XContentParser parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(builder));
+        parser.nextToken();
+        GeoQueryContext queryContext = fromXContent(parser);
+        assertEquals(10, queryContext.getBoost());
+        assertEquals(12, queryContext.getPrecision());
+        assertEquals(Arrays.asList(1, 2), queryContext.getNeighbours());
+
+        builder = jsonBuilder().startObject();
+        {
+            builder.startObject("context").field("lat", 23.654242).field("lon", 90.047153).endObject();
+            builder.field("boost", 10);
+            builder.field("precision", "12m");
+            builder.array("neighbours", "4km", "10km");
+        }
+        builder.endObject();
+        parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(builder));
+        parser.nextToken();
+        queryContext = fromXContent(parser);
+        assertEquals(10, queryContext.getBoost());
+        assertEquals(9, queryContext.getPrecision());
+        assertEquals(Arrays.asList(6, 5), queryContext.getNeighbours());
+    }
 }