Skip to content

Commit 2c20f7a

Browse files
imotovimotov
authored
Allow using distance measure in the geo context precision (#29273)
Adds support for distance measure, such as "4km", "5m" in the precision field of the geo location context in context suggesters. Fixes #24807
1 parent e2d771e commit 2c20f7a

File tree

6 files changed

+164
-38
lines changed

6 files changed

+164
-38
lines changed

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

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,10 @@
2424
import org.apache.lucene.spatial.prefix.tree.QuadPrefixTree;
2525
import org.apache.lucene.util.SloppyMath;
2626
import org.elasticsearch.ElasticsearchParseException;
27-
import org.elasticsearch.common.Strings;
2827
import org.elasticsearch.common.unit.DistanceUnit;
2928
import org.elasticsearch.common.xcontent.XContentParser;
3029
import org.elasticsearch.common.xcontent.XContentParser.Token;
30+
import org.elasticsearch.common.xcontent.support.XContentMapValues;
3131
import org.elasticsearch.index.fielddata.FieldData;
3232
import org.elasticsearch.index.fielddata.GeoPointValues;
3333
import org.elasticsearch.index.fielddata.MultiGeoPointValues;
@@ -459,6 +459,51 @@ public static GeoPoint parseGeoPoint(XContentParser parser, GeoPoint point, fina
459459
}
460460
}
461461

462+
/**
463+
* Parse a precision that can be expressed as an integer or a distance measure like "1km", "10m".
464+
*
465+
* The precision is expressed as a number between 1 and 12 and indicates the length of geohash
466+
* used to represent geo points.
467+
*
468+
* @param parser {@link XContentParser} to parse the value from
469+
* @return int representing precision
470+
*/
471+
public static int parsePrecision(XContentParser parser) throws IOException, ElasticsearchParseException {
472+
XContentParser.Token token = parser.currentToken();
473+
if (token.equals(XContentParser.Token.VALUE_NUMBER)) {
474+
return XContentMapValues.nodeIntegerValue(parser.intValue());
475+
} else {
476+
String precision = parser.text();
477+
try {
478+
// we want to treat simple integer strings as precision levels, not distances
479+
return XContentMapValues.nodeIntegerValue(precision);
480+
} catch (NumberFormatException e) {
481+
// try to parse as a distance value
482+
final int parsedPrecision = GeoUtils.geoHashLevelsForPrecision(precision);
483+
try {
484+
return checkPrecisionRange(parsedPrecision);
485+
} catch (IllegalArgumentException e2) {
486+
// this happens when distance too small, so precision > 12. We'd like to see the original string
487+
throw new IllegalArgumentException("precision too high [" + precision + "]", e2);
488+
}
489+
}
490+
}
491+
}
492+
493+
/**
494+
* Checks that the precision is within range supported by elasticsearch - between 1 and 12
495+
*
496+
* Returns the precision value if it is in the range and throws an IllegalArgumentException if it
497+
* is outside the range.
498+
*/
499+
public static int checkPrecisionRange(int precision) {
500+
if ((precision < 1) || (precision > 12)) {
501+
throw new IllegalArgumentException("Invalid geohash aggregation precision of " + precision
502+
+ ". Must be between 1 and 12.");
503+
}
504+
return precision;
505+
}
506+
462507
/** Returns the maximum distance/radius (in meters) from the point 'center' before overlapping */
463508
public static double maxRadialDistanceMeters(final double centerLat, final double centerLon) {
464509
if (Math.abs(centerLat) == MAX_LAT) {

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

Lines changed: 5 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,8 @@
5454
import java.util.Map;
5555
import java.util.Objects;
5656

57+
import static org.elasticsearch.common.geo.GeoUtils.parsePrecision;
58+
5759
public class GeoGridAggregationBuilder extends ValuesSourceAggregationBuilder<ValuesSource.GeoPoint, GeoGridAggregationBuilder>
5860
implements MultiBucketAggregationBuilder {
5961
public static final String NAME = "geohash_grid";
@@ -64,29 +66,8 @@ public class GeoGridAggregationBuilder extends ValuesSourceAggregationBuilder<Va
6466
static {
6567
PARSER = new ObjectParser<>(GeoGridAggregationBuilder.NAME);
6668
ValuesSourceParserHelper.declareGeoFields(PARSER, false, false);
67-
PARSER.declareField((parser, builder, context) -> {
68-
XContentParser.Token token = parser.currentToken();
69-
if (token.equals(XContentParser.Token.VALUE_NUMBER)) {
70-
builder.precision(XContentMapValues.nodeIntegerValue(parser.intValue()));
71-
} else {
72-
String precision = parser.text();
73-
try {
74-
// we want to treat simple integer strings as precision levels, not distances
75-
builder.precision(XContentMapValues.nodeIntegerValue(Integer.parseInt(precision)));
76-
} catch (NumberFormatException e) {
77-
// try to parse as a distance value
78-
try {
79-
builder.precision(GeoUtils.geoHashLevelsForPrecision(precision));
80-
} catch (NumberFormatException e2) {
81-
// can happen when distance unit is unknown, in this case we simply want to know the reason
82-
throw e2;
83-
} catch (IllegalArgumentException e3) {
84-
// this happens when distance too small, so precision > 12. We'd like to see the original string
85-
throw new IllegalArgumentException("precision too high [" + precision + "]", e3);
86-
}
87-
}
88-
}
89-
}, GeoHashGridParams.FIELD_PRECISION, org.elasticsearch.common.xcontent.ObjectParser.ValueType.INT);
69+
PARSER.declareField((parser, builder, context) -> builder.precision(parsePrecision(parser)), GeoHashGridParams.FIELD_PRECISION,
70+
org.elasticsearch.common.xcontent.ObjectParser.ValueType.INT);
9071
PARSER.declareInt(GeoGridAggregationBuilder::size, GeoHashGridParams.FIELD_SIZE);
9172
PARSER.declareInt(GeoGridAggregationBuilder::shardSize, GeoHashGridParams.FIELD_SHARD_SIZE);
9273
}
@@ -133,7 +114,7 @@ protected void innerWriteTo(StreamOutput out) throws IOException {
133114
}
134115

135116
public GeoGridAggregationBuilder precision(int precision) {
136-
this.precision = GeoHashGridParams.checkPrecision(precision);
117+
this.precision = GeoUtils.checkPrecisionRange(precision);
137118
return this;
138119
}
139120

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

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,15 +30,6 @@ final class GeoHashGridParams {
3030
static final ParseField FIELD_SIZE = new ParseField("size");
3131
static final ParseField FIELD_SHARD_SIZE = new ParseField("shard_size");
3232

33-
34-
static int checkPrecision(int precision) {
35-
if ((precision < 1) || (precision > 12)) {
36-
throw new IllegalArgumentException("Invalid geohash aggregation precision of " + precision
37-
+ ". Must be between 1 and 12.");
38-
}
39-
return precision;
40-
}
41-
4233
private GeoHashGridParams() {
4334
throw new AssertionError("No instances intended");
4435
}

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import java.util.List;
3434
import java.util.Objects;
3535

36+
import static org.elasticsearch.common.geo.GeoUtils.parsePrecision;
3637
import static org.elasticsearch.search.suggest.completion.context.GeoContextMapping.CONTEXT_BOOST;
3738
import static org.elasticsearch.search.suggest.completion.context.GeoContextMapping.CONTEXT_NEIGHBOURS;
3839
import static org.elasticsearch.search.suggest.completion.context.GeoContextMapping.CONTEXT_PRECISION;
@@ -115,10 +116,10 @@ public static Builder builder() {
115116
static {
116117
GEO_CONTEXT_PARSER.declareField((parser, geoQueryContext, geoContextMapping) -> geoQueryContext.setGeoPoint(GeoUtils.parseGeoPoint(parser)), new ParseField(CONTEXT_VALUE), ObjectParser.ValueType.OBJECT);
117118
GEO_CONTEXT_PARSER.declareInt(GeoQueryContext.Builder::setBoost, new ParseField(CONTEXT_BOOST));
118-
// TODO : add string support for precision for GeoUtils.geoHashLevelsForPrecision()
119-
GEO_CONTEXT_PARSER.declareInt(GeoQueryContext.Builder::setPrecision, new ParseField(CONTEXT_PRECISION));
120-
// TODO : add string array support for precision for GeoUtils.geoHashLevelsForPrecision()
121-
GEO_CONTEXT_PARSER.declareIntArray(GeoQueryContext.Builder::setNeighbours, new ParseField(CONTEXT_NEIGHBOURS));
119+
GEO_CONTEXT_PARSER.declareField((parser, builder, context) -> builder.setPrecision(parsePrecision(parser)),
120+
new ParseField(CONTEXT_PRECISION), ObjectParser.ValueType.INT);
121+
GEO_CONTEXT_PARSER.declareFieldArray(GeoQueryContext.Builder::setNeighbours, (parser, builder) -> parsePrecision(parser),
122+
new ParseField(CONTEXT_NEIGHBOURS), ObjectParser.ValueType.INT_ARRAY);
122123
GEO_CONTEXT_PARSER.declareDouble(GeoQueryContext.Builder::setLat, new ParseField("lat"));
123124
GEO_CONTEXT_PARSER.declareDouble(GeoQueryContext.Builder::setLon, new ParseField("lon"));
124125
}

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

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
/*
2+
* Licensed to Elasticsearch under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.elasticsearch.common.geo;
20+
21+
import org.elasticsearch.common.CheckedConsumer;
22+
import org.elasticsearch.common.bytes.BytesReference;
23+
import org.elasticsearch.common.xcontent.XContentBuilder;
24+
import org.elasticsearch.common.xcontent.XContentParser;
25+
import org.elasticsearch.common.xcontent.json.JsonXContent;
26+
import org.elasticsearch.test.ESTestCase;
27+
28+
import java.io.IOException;
29+
30+
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
31+
32+
public class GeoUtilTests extends ESTestCase {
33+
34+
public void testPrecisionParser() throws IOException {
35+
assertEquals(10, parsePrecision(builder -> builder.field("test", 10)));
36+
assertEquals(10, parsePrecision(builder -> builder.field("test", 10.2)));
37+
assertEquals(6, parsePrecision(builder -> builder.field("test", "6")));
38+
assertEquals(7, parsePrecision(builder -> builder.field("test", "1km")));
39+
assertEquals(7, parsePrecision(builder -> builder.field("test", "1.1km")));
40+
}
41+
42+
public void testIncorrectPrecisionParser() {
43+
expectThrows(NumberFormatException.class, () -> parsePrecision(builder -> builder.field("test", "10.1.1.1")));
44+
expectThrows(NumberFormatException.class, () -> parsePrecision(builder -> builder.field("test", "364.4smoots")));
45+
assertEquals(
46+
"precision too high [0.01mm]",
47+
expectThrows(IllegalArgumentException.class, () -> parsePrecision(builder -> builder.field("test", "0.01mm"))).getMessage()
48+
);
49+
}
50+
51+
/**
52+
* Invokes GeoUtils.parsePrecision parser on the value generated by tokenGenerator
53+
* <p>
54+
* The supplied tokenGenerator should generate a single field that contains the precision in
55+
* one of the supported formats or malformed precision value if error handling is tested. The
56+
* method return the parsed value or throws an exception, if precision value is malformed.
57+
*/
58+
private int parsePrecision(CheckedConsumer<XContentBuilder, IOException> tokenGenerator) throws IOException {
59+
XContentBuilder builder = jsonBuilder().startObject();
60+
tokenGenerator.accept(builder);
61+
builder.endObject();
62+
XContentParser parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(builder));
63+
assertEquals(XContentParser.Token.START_OBJECT, parser.nextToken()); // {
64+
assertEquals(XContentParser.Token.FIELD_NAME, parser.nextToken()); // field name
65+
assertTrue(parser.nextToken().isValue()); // field value
66+
int precision = GeoUtils.parsePrecision(parser);
67+
assertEquals(XContentParser.Token.END_OBJECT, parser.nextToken()); // }
68+
assertNull(parser.nextToken()); // no more tokens
69+
return precision;
70+
}
71+
}

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,20 @@
1919

2020
package org.elasticsearch.search.suggest.completion;
2121

22+
import org.elasticsearch.common.bytes.BytesReference;
2223
import org.elasticsearch.common.geo.GeoPoint;
24+
import org.elasticsearch.common.xcontent.XContentBuilder;
2325
import org.elasticsearch.common.xcontent.XContentParser;
26+
import org.elasticsearch.common.xcontent.json.JsonXContent;
2427
import org.elasticsearch.search.suggest.completion.context.GeoQueryContext;
2528

2629
import java.io.IOException;
2730
import java.util.ArrayList;
31+
import java.util.Arrays;
2832
import java.util.Collections;
2933
import java.util.List;
3034

35+
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
3136
import static org.hamcrest.Matchers.equalTo;
3237

3338
public class GeoQueryContextTests extends QueryContextTestCase<GeoQueryContext> {
@@ -105,4 +110,36 @@ public void testIllegalArguments() {
105110
assertEquals(e.getMessage(), "neighbour value must be between 1 and 12");
106111
}
107112
}
113+
114+
public void testStringPrecision() throws IOException {
115+
XContentBuilder builder = jsonBuilder().startObject();
116+
{
117+
builder.startObject("context").field("lat", 23.654242).field("lon", 90.047153).endObject();
118+
builder.field("boost", 10);
119+
builder.field("precision", 12);
120+
builder.array("neighbours", 1, 2);
121+
}
122+
builder.endObject();
123+
XContentParser parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(builder));
124+
parser.nextToken();
125+
GeoQueryContext queryContext = fromXContent(parser);
126+
assertEquals(10, queryContext.getBoost());
127+
assertEquals(12, queryContext.getPrecision());
128+
assertEquals(Arrays.asList(1, 2), queryContext.getNeighbours());
129+
130+
builder = jsonBuilder().startObject();
131+
{
132+
builder.startObject("context").field("lat", 23.654242).field("lon", 90.047153).endObject();
133+
builder.field("boost", 10);
134+
builder.field("precision", "12m");
135+
builder.array("neighbours", "4km", "10km");
136+
}
137+
builder.endObject();
138+
parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(builder));
139+
parser.nextToken();
140+
queryContext = fromXContent(parser);
141+
assertEquals(10, queryContext.getBoost());
142+
assertEquals(9, queryContext.getPrecision());
143+
assertEquals(Arrays.asList(6, 5), queryContext.getNeighbours());
144+
}
108145
}

0 commit comments

Comments
 (0)