changes from review

This commit is contained in:
Colin Goodheart-Smithe 2016-06-14 13:39:20 +01:00
parent cfd3356ee3
commit bec621d46f
6 changed files with 21 additions and 22 deletions

View File

@ -98,7 +98,7 @@ public class GeoGridAggregationBuilder extends ValuesSourceAggregationBuilder<Va
}
public GeoGridAggregationBuilder shardSize(int shardSize) {
if (shardSize < -1 || shardSize == 0) {
if (shardSize <= 0) {
throw new IllegalArgumentException(
"[shardSize] must be greater than 0. Found [" + shardSize + "] in [" + name + "]");
}
@ -140,7 +140,9 @@ public class GeoGridAggregationBuilder extends ValuesSourceAggregationBuilder<Va
protected XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
builder.field(GeoHashGridParams.FIELD_PRECISION.getPreferredName(), precision);
builder.field(GeoHashGridParams.FIELD_SIZE.getPreferredName(), requiredSize);
if (shardSize > -1) {
builder.field(GeoHashGridParams.FIELD_SHARD_SIZE.getPreferredName(), shardSize);
}
return builder;
}

View File

@ -113,8 +113,8 @@ public class SignificantTermsAggregationBuilder extends ValuesSourceAggregationB
* (defaults to 10)
*/
public SignificantTermsAggregationBuilder size(int size) {
if (size < 0) {
throw new IllegalArgumentException("[size] must be greater than or equal to 0. Found [" + size + "] in [" + name + "]");
if (size <= 0) {
throw new IllegalArgumentException("[size] must be greater than 0. Found [" + size + "] in [" + name + "]");
}
bucketCountThresholds.setRequiredSize(size);
return this;
@ -127,9 +127,9 @@ public class SignificantTermsAggregationBuilder extends ValuesSourceAggregationB
* results are.
*/
public SignificantTermsAggregationBuilder shardSize(int shardSize) {
if (shardSize < 0) {
if (shardSize <= 0) {
throw new IllegalArgumentException(
"[shardSize] must be greater than or equal to 0. Found [" + shardSize + "] in [" + name + "]");
"[shardSize] must be greater than 0. Found [" + shardSize + "] in [" + name + "]");
}
bucketCountThresholds.setShardSize(shardSize);
return this;

View File

@ -110,8 +110,8 @@ public class TermsAggregationBuilder extends ValuesSourceAggregationBuilder<Valu
* (defaults to 10)
*/
public TermsAggregationBuilder size(int size) {
if (size < 0) {
throw new IllegalArgumentException("[size] must be greater than or equal to 0. Found [" + size + "] in [" + name + "]");
if (size <= 0) {
throw new IllegalArgumentException("[size] must be greater than 0. Found [" + size + "] in [" + name + "]");
}
bucketCountThresholds.setRequiredSize(size);
return this;
@ -124,9 +124,9 @@ public class TermsAggregationBuilder extends ValuesSourceAggregationBuilder<Valu
* results are.
*/
public TermsAggregationBuilder shardSize(int shardSize) {
if (shardSize < 0) {
if (shardSize <= 0) {
throw new IllegalArgumentException(
"[shardSize] must be greater than or equal to 0. Found [" + shardSize + "] in [" + name + "]");
"[shardSize] must be greater than 0. Found [" + shardSize + "] in [" + name + "]");
}
bucketCountThresholds.setShardSize(shardSize);
return this;

View File

@ -236,13 +236,12 @@ public class DoubleTermsTests extends AbstractTermsTestCase {
// the main purpose of this test is to make sure we're not allocating 2GB of memory per shard
public void testSizeIsZero() {
ElasticsearchException exception = expectThrows(ElasticsearchException.class,
IllegalArgumentException exception = expectThrows(IllegalArgumentException.class,
() -> client()
.prepareSearch("idx").setTypes("high_card_type").addAggregation(terms("terms").field(SINGLE_VALUED_FIELD_NAME)
.minDocCount(randomInt(1)).size(0).collectMode(randomFrom(SubAggCollectionMode.values())))
.execute().actionGet());
assertThat(exception.getDetailedMessage(),
containsString("parameters [required_size] and [shard_size] must be >0 in terms aggregation."));
assertThat(exception.getMessage(), containsString("[size] must be greater than 0. Found [0] in [terms]"));
}
public void testSingleValueField() throws Exception {
@ -588,7 +587,7 @@ public class DoubleTermsTests extends AbstractTermsTestCase {
SearchResponse response = client().prepareSearch("idx_unmapped").setTypes("type")
.addAggregation(terms("terms")
.field(SINGLE_VALUED_FIELD_NAME)
.size(randomInt(5))
.size(randomIntBetween(1, 5))
.collectMode(randomFrom(SubAggCollectionMode.values())))
.execute().actionGet();

View File

@ -238,7 +238,7 @@ public class LongTermsTests extends AbstractTermsTestCase {
// the main purpose of this test is to make sure we're not allocating 2GB of memory per shard
public void testSizeIsZero() {
ElasticsearchException exception = expectThrows(ElasticsearchException.class,
IllegalArgumentException exception = expectThrows(IllegalArgumentException.class,
() -> client().prepareSearch("idx").setTypes("high_card_type")
.addAggregation(terms("terms")
.field(SINGLE_VALUED_FIELD_NAME)
@ -246,8 +246,7 @@ public class LongTermsTests extends AbstractTermsTestCase {
.minDocCount(randomInt(1))
.size(0))
.execute().actionGet());
assertThat(exception.getDetailedMessage(),
containsString("parameters [required_size] and [shard_size] must be >0 in terms aggregation."));
assertThat(exception.getMessage(), containsString("[size] must be greater than 0. Found [0] in [terms]"));
}
public void testSingleValueField() throws Exception {
@ -593,7 +592,7 @@ public class LongTermsTests extends AbstractTermsTestCase {
SearchResponse response = client().prepareSearch("idx_unmapped").setTypes("type")
.addAggregation(terms("terms")
.field(SINGLE_VALUED_FIELD_NAME)
.size(randomInt(5))
.size(randomIntBetween(1, 5))
.collectMode(randomFrom(SubAggCollectionMode.values())))
.execute().actionGet();

View File

@ -203,15 +203,14 @@ public class StringTermsTests extends AbstractTermsTestCase {
// the main purpose of this test is to make sure we're not allocating 2GB of memory per shard
public void testSizeIsZero() {
final int minDocCount = randomInt(1);
ElasticsearchException exception = expectThrows(ElasticsearchException.class, () -> client()
IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> client()
.prepareSearch("idx")
.setTypes("high_card_type")
.addAggregation(
terms("terms").executionHint(randomExecutionHint()).field(SINGLE_VALUED_FIELD_NAME)
.collectMode(randomFrom(SubAggCollectionMode.values())).minDocCount(minDocCount).size(0)).execute()
.actionGet());
assertThat(exception.getDetailedMessage(),
containsString("parameters [required_size] and [shard_size] must be >0 in terms aggregation."));
assertThat(exception.getMessage(), containsString("[size] must be greater than 0. Found [0] in [terms]"));
}
public void testSingleValueField() throws Exception {
@ -756,7 +755,7 @@ public class StringTermsTests extends AbstractTermsTestCase {
.prepareSearch("idx_unmapped")
.setTypes("type")
.addAggregation(
terms("terms").executionHint(randomExecutionHint()).size(randomInt(5)).field(SINGLE_VALUED_FIELD_NAME)
terms("terms").executionHint(randomExecutionHint()).size(randomIntBetween(1, 5)).field(SINGLE_VALUED_FIELD_NAME)
.collectMode(randomFrom(SubAggCollectionMode.values()))).execute().actionGet();
assertSearchResponse(response);