diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridBuilder.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridBuilder.java index 9f382d86906..a1f12f465ca 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridBuilder.java @@ -30,8 +30,8 @@ public class GeoHashGridBuilder extends AggregationBuilder { private String field; - private int precision = GeoHashGridParser.DEFAULT_PRECISION; - private int requiredSize = GeoHashGridParser.DEFAULT_MAX_NUM_CELLS; + private int precision = GeoHashGridParams.DEFAULT_PRECISION; + private int requiredSize = GeoHashGridParams.DEFAULT_MAX_NUM_CELLS; private int shardSize = 0; /** @@ -54,11 +54,7 @@ public class GeoHashGridBuilder extends AggregationBuilder { * precision, the more fine-grained this aggregation will be. */ public GeoHashGridBuilder precision(int precision) { - if ((precision < 1) || (precision > 12)) { - throw new IllegalArgumentException("Invalid geohash aggregation precision of " + precision - + "must be between 1 and 12"); - } - this.precision = precision; + this.precision = GeoHashGridParams.checkPrecision(precision); return this; } @@ -85,14 +81,14 @@ public class GeoHashGridBuilder extends AggregationBuilder { if (field != null) { builder.field("field", field); } - if (precision != GeoHashGridParser.DEFAULT_PRECISION) { - builder.field("precision", precision); + if (precision != GeoHashGridParams.DEFAULT_PRECISION) { + builder.field(GeoHashGridParams.FIELD_PRECISION.getPreferredName(), precision); } - if (requiredSize != GeoHashGridParser.DEFAULT_MAX_NUM_CELLS) { - builder.field("size", requiredSize); + if (requiredSize != GeoHashGridParams.DEFAULT_MAX_NUM_CELLS) { + builder.field(GeoHashGridParams.FIELD_SIZE.getPreferredName(), requiredSize); } if (shardSize != 0) { - builder.field("shard_size", shardSize); + builder.field(GeoHashGridParams.FIELD_SHARD_SIZE.getPreferredName(), shardSize); } return builder.endObject(); diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridParams.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridParams.java new file mode 100644 index 00000000000..640418920d2 --- /dev/null +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridParams.java @@ -0,0 +1,48 @@ +/* + * 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.search.aggregations.bucket.geogrid; + +import org.elasticsearch.common.ParseField; + +/** + * Encapsulates relevant parameter defaults and validations for the geo hash grid aggregation. + */ +final class GeoHashGridParams { + /* default values */ + public static final int DEFAULT_PRECISION = 5; + public static final int DEFAULT_MAX_NUM_CELLS = 10000; + + /* recognized field names in JSON */ + public static final ParseField FIELD_PRECISION = new ParseField("precision"); + public static final ParseField FIELD_SIZE = new ParseField("size"); + public static final ParseField FIELD_SHARD_SIZE = new ParseField("shard_size"); + + + public 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/core/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridParser.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridParser.java index 97c68be3cb4..109301fdbff 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridParser.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridParser.java @@ -21,6 +21,7 @@ package org.elasticsearch.search.aggregations.bucket.geogrid; import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.index.SortedNumericDocValues; import org.apache.lucene.util.GeoHashUtils; +import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.geo.GeoPoint; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.fielddata.MultiGeoPointValues; @@ -28,6 +29,7 @@ import org.elasticsearch.index.fielddata.SortedBinaryDocValues; import org.elasticsearch.index.fielddata.SortedNumericDoubleValues; import org.elasticsearch.index.fielddata.SortingNumericDocValues; import org.elasticsearch.index.query.GeoBoundingBoxQueryBuilder; +import org.elasticsearch.search.SearchParseException; import org.elasticsearch.search.aggregations.Aggregator; import org.elasticsearch.search.aggregations.AggregatorFactory; import org.elasticsearch.search.aggregations.InternalAggregation; @@ -58,16 +60,13 @@ public class GeoHashGridParser implements Aggregator.Parser { return InternalGeoHashGrid.TYPE.name(); } - public static final int DEFAULT_PRECISION = 5; - public static final int DEFAULT_MAX_NUM_CELLS = 10000; - @Override public AggregatorFactory parse(String aggregationName, XContentParser parser, SearchContext context) throws IOException { ValuesSourceParser vsParser = ValuesSourceParser.geoPoint(aggregationName, InternalGeoHashGrid.TYPE, context).build(); - int precision = DEFAULT_PRECISION; - int requiredSize = DEFAULT_MAX_NUM_CELLS; + int precision = GeoHashGridParams.DEFAULT_PRECISION; + int requiredSize = GeoHashGridParams.DEFAULT_MAX_NUM_CELLS; int shardSize = -1; XContentParser.Token token; @@ -77,14 +76,18 @@ public class GeoHashGridParser implements Aggregator.Parser { currentFieldName = parser.currentName(); } else if (vsParser.token(currentFieldName, token, parser)) { continue; - } else if (token == XContentParser.Token.VALUE_NUMBER) { - if ("precision".equals(currentFieldName)) { - precision = parser.intValue(); - } else if ("size".equals(currentFieldName)) { + } else if (token == XContentParser.Token.VALUE_NUMBER || + token == XContentParser.Token.VALUE_STRING) { //Be lenient and also allow numbers enclosed in quotes + if (context.parseFieldMatcher().match(currentFieldName, GeoHashGridParams.FIELD_PRECISION)) { + precision = GeoHashGridParams.checkPrecision(parser.intValue()); + } else if (context.parseFieldMatcher().match(currentFieldName, GeoHashGridParams.FIELD_SIZE)) { requiredSize = parser.intValue(); - } else if ("shard_size".equals(currentFieldName) || "shardSize".equals(currentFieldName)) { + } else if (context.parseFieldMatcher().match(currentFieldName, GeoHashGridParams.FIELD_SHARD_SIZE)) { shardSize = parser.intValue(); } + } else if (token != XContentParser.Token.START_OBJECT) { + throw new SearchParseException(context, "Unexpected token " + token + " in [" + aggregationName + "].", + parser.getTokenLocation()); } } @@ -112,9 +115,9 @@ public class GeoHashGridParser implements Aggregator.Parser { static class GeoGridFactory extends ValuesSourceAggregatorFactory { - private int precision; - private int requiredSize; - private int shardSize; + private final int precision; + private final int requiredSize; + private final int shardSize; public GeoGridFactory(String name, ValuesSourceConfig config, int precision, int requiredSize, int shardSize) { super(name, InternalGeoHashGrid.TYPE.name(), config); diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridParserTests.java b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridParserTests.java new file mode 100644 index 00000000000..cd7dadd7eeb --- /dev/null +++ b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridParserTests.java @@ -0,0 +1,84 @@ +/* + * 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.search.aggregations.bucket.geogrid; + +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.common.xcontent.json.JsonXContent; +import org.elasticsearch.search.SearchParseException; +import org.elasticsearch.search.internal.SearchContext; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.TestSearchContext; + +public class GeoHashGridParserTests extends ESTestCase { + public void testParseValidFromInts() throws Exception { + SearchContext searchContext = new TestSearchContext(); + int precision = randomIntBetween(1, 12); + XContentParser stParser = JsonXContent.jsonXContent.createParser( + "{\"field\":\"my_loc\", \"precision\":" + precision + ", \"size\": 500, \"shard_size\": 550}"); + GeoHashGridParser parser = new GeoHashGridParser(); + // can create a factory + assertNotNull(parser.parse("geohash_grid", stParser, searchContext)); + } + + public void testParseValidFromStrings() throws Exception { + SearchContext searchContext = new TestSearchContext(); + int precision = randomIntBetween(1, 12); + XContentParser stParser = JsonXContent.jsonXContent.createParser( + "{\"field\":\"my_loc\", \"precision\":\"" + precision + "\", \"size\": \"500\", \"shard_size\": \"550\"}"); + GeoHashGridParser parser = new GeoHashGridParser(); + // can create a factory + assertNotNull(parser.parse("geohash_grid", stParser, searchContext)); + } + + public void testParseErrorOnNonIntPrecision() throws Exception { + SearchContext searchContext = new TestSearchContext(); + XContentParser stParser = JsonXContent.jsonXContent.createParser("{\"field\":\"my_loc\", \"precision\":\"2.0\"}"); + GeoHashGridParser parser = new GeoHashGridParser(); + try { + parser.parse("geohash_grid", stParser, searchContext); + fail(); + } catch (NumberFormatException ex) { + assertEquals("For input string: \"2.0\"", ex.getMessage()); + } + } + + public void testParseErrorOnBooleanPrecision() throws Exception { + SearchContext searchContext = new TestSearchContext(); + XContentParser stParser = JsonXContent.jsonXContent.createParser("{\"field\":\"my_loc\", \"precision\":false}"); + GeoHashGridParser parser = new GeoHashGridParser(); + try { + parser.parse("geohash_grid", stParser, searchContext); + fail(); + } catch (SearchParseException ex) { + assertEquals("Unexpected token VALUE_BOOLEAN in [geohash_grid].", ex.getMessage()); + } + } + + public void testParseErrorOnPrecisionOutOfRange() throws Exception { + SearchContext searchContext = new TestSearchContext(); + XContentParser stParser = JsonXContent.jsonXContent.createParser("{\"field\":\"my_loc\", \"precision\":\"13\"}"); + GeoHashGridParser parser = new GeoHashGridParser(); + try { + parser.parse("geohash_grid", stParser, searchContext); + fail(); + } catch (IllegalArgumentException ex) { + assertEquals("Invalid geohash aggregation precision of 13. Must be between 1 and 12.", ex.getMessage()); + } + } +} \ No newline at end of file