diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/AggregatorFactories.java b/core/src/main/java/org/elasticsearch/search/aggregations/AggregatorFactories.java index 0a76f1f7e86..6a1cd27e0b2 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/AggregatorFactories.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/AggregatorFactories.java @@ -244,5 +244,13 @@ public class AggregatorFactories { orderedPipelineAggregators.add(factory); } } + + AggregatorFactory[] getAggregatorFactories() { + return this.factories.toArray(new AggregatorFactory[this.factories.size()]); + } + + List getPipelineAggregatorFactories() { + return this.pipelineAggregatorFactories; + } } } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/AggregatorParsers.java b/core/src/main/java/org/elasticsearch/search/aggregations/AggregatorParsers.java index bba7be2ad1f..257fef89cd2 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/AggregatorParsers.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/AggregatorParsers.java @@ -80,7 +80,7 @@ public class AggregatorParsers { /** * Returns the parser that is registered under the given pipeline aggregator * type. - * + * * @param type * The pipeline aggregator type * @return The parser associated with the given pipeline aggregator type. @@ -228,6 +228,10 @@ public class AggregatorParsers { throw new SearchParseException(context, "Aggregation [" + aggregationName + "] cannot define sub-aggregations", parser.getTokenLocation()); } + if (level == 0) { + pipelineAggregatorFactory + .validate(null, factories.getAggregatorFactories(), factories.getPipelineAggregatorFactories()); + } factories.addPipelineAggregator(pipelineAggregatorFactory); } } diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/pipeline/PercentilesBucketIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/pipeline/PercentilesBucketIT.java index 507939e6858..f7c1d060bd3 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/pipeline/PercentilesBucketIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/pipeline/PercentilesBucketIT.java @@ -19,10 +19,10 @@ package org.elasticsearch.search.aggregations.pipeline; * under the License. */ +import org.elasticsearch.ElasticsearchException; import org.elasticsearch.action.index.IndexRequestBuilder; import org.elasticsearch.action.search.SearchPhaseExecutionException; import org.elasticsearch.action.search.SearchResponse; -import org.elasticsearch.search.SearchParseException; import org.elasticsearch.search.aggregations.bucket.histogram.Histogram; import org.elasticsearch.search.aggregations.bucket.terms.Terms; import org.elasticsearch.search.aggregations.metrics.percentiles.Percentile; @@ -41,9 +41,9 @@ import static org.elasticsearch.search.aggregations.AggregationBuilders.histogra import static org.elasticsearch.search.aggregations.AggregationBuilders.sum; import static org.elasticsearch.search.aggregations.AggregationBuilders.terms; import static org.elasticsearch.search.aggregations.pipeline.PipelineAggregatorBuilders.percentilesBucket; -import static org.elasticsearch.search.aggregations.pipeline.PipelineAggregatorBuilders.sumBucket; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.core.IsNull.notNullValue; @@ -433,30 +433,22 @@ public class PercentilesBucketIT extends ESIntegTestCase { } @Test - @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/13179") public void testBadPercents() throws Exception { Double[] badPercents = {-1.0, 110.0}; try { - SearchResponse response = client().prepareSearch("idx") + client().prepareSearch("idx") .addAggregation(terms("terms").field("tag").subAggregation(sum("sum").field(SINGLE_VALUED_FIELD_NAME))) .addAggregation(percentilesBucket("percentiles_bucket") .setBucketsPaths("terms>sum") .percents(badPercents)).execute().actionGet(); - assertSearchResponse(response); - - Terms terms = response.getAggregations().get("terms"); - assertThat(terms, notNullValue()); - assertThat(terms.getName(), equalTo("terms")); - List buckets = terms.getBuckets(); - assertThat(buckets.size(), equalTo(0)); - - PercentilesBucket percentilesBucketValue = response.getAggregations().get("percentiles_bucket"); - fail("Illegal percent's were provided but no exception was thrown."); } catch (SearchPhaseExecutionException exception) { - // All good + ElasticsearchException[] rootCauses = exception.guessRootCauses(); + assertThat(rootCauses.length, equalTo(1)); + ElasticsearchException rootCause = rootCauses[0]; + assertThat(rootCause.getMessage(), containsString("must only contain non-null doubles from 0.0-100.0 inclusive")); } } @@ -466,7 +458,7 @@ public class PercentilesBucketIT extends ESIntegTestCase { Double[] badPercents = {-1.0, 110.0}; try { - SearchResponse response = client() + client() .prepareSearch("idx") .addAggregation( terms("terms") @@ -479,11 +471,12 @@ public class PercentilesBucketIT extends ESIntegTestCase { .setBucketsPaths("histo>_count") .percents(badPercents))).execute().actionGet(); - PercentilesBucket percentilesBucketValue = response.getAggregations().get("percentiles_bucket"); - fail("Illegal percent's were provided but no exception was thrown."); } catch (SearchPhaseExecutionException exception) { - // All good + ElasticsearchException[] rootCauses = exception.guessRootCauses(); + assertThat(rootCauses.length, equalTo(1)); + ElasticsearchException rootCause = rootCauses[0]; + assertThat(rootCause.getMessage(), containsString("must only contain non-null doubles from 0.0-100.0 inclusive")); } }