diff --git a/server/src/main/java/org/elasticsearch/common/geo/GeoUtils.java b/server/src/main/java/org/elasticsearch/common/geo/GeoUtils.java index 655b259c81074..ce0098ea9722f 100644 --- a/server/src/main/java/org/elasticsearch/common/geo/GeoUtils.java +++ b/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,51 @@ 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". + * + * The precision is expressed as a number between 1 and 12 and indicates the length of geohash + * used to represent geo points. + * + * @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(precision); + } catch (NumberFormatException e) { + // try to parse as a distance value + final int parsedPrecision = GeoUtils.geoHashLevelsForPrecision(precision); + try { + return checkPrecisionRange(parsedPrecision); + } catch (IllegalArgumentException e2) { + // this happens when distance too small, so precision > 12. We'd like to see the original string + throw new IllegalArgumentException("precision too high [" + precision + "]", e2); + } + } + } + } + + /** + * 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) { diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoGridAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoGridAggregationBuilder.java index f91dde8877093..2f66531834d38 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoGridAggregationBuilder.java +++ b/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 implements MultiBucketAggregationBuilder { public static final String NAME = "geohash_grid"; @@ -64,29 +66,8 @@ public class GeoGridAggregationBuilder extends ValuesSourceAggregationBuilder(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; } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridParams.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridParams.java index e4b8d753c4018..ff3b21a3a7bae 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridParams.java +++ b/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"); } diff --git a/server/src/main/java/org/elasticsearch/search/suggest/completion/context/GeoQueryContext.java b/server/src/main/java/org/elasticsearch/search/suggest/completion/context/GeoQueryContext.java index 151dcc9173f28..259446cb0c1df 100644 --- a/server/src/main/java/org/elasticsearch/search/suggest/completion/context/GeoQueryContext.java +++ b/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")); } diff --git a/server/src/test/java/org/elasticsearch/common/geo/GeoUtilTests.java b/server/src/test/java/org/elasticsearch/common/geo/GeoUtilTests.java new file mode 100644 index 0000000000000..efec56e788da1 --- /dev/null +++ b/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 + *

+ * 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 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; + } +} diff --git a/server/src/test/java/org/elasticsearch/search/suggest/completion/GeoQueryContextTests.java b/server/src/test/java/org/elasticsearch/search/suggest/completion/GeoQueryContextTests.java index 1d058350a98a5..7764f269a03b3 100644 --- a/server/src/test/java/org/elasticsearch/search/suggest/completion/GeoQueryContextTests.java +++ b/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 { @@ -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()); + } }