From 48b0deef4fc95b688ea9f683edd3e9c75374c272 Mon Sep 17 00:00:00 2001 From: Luca Cavanna Date: Wed, 20 Mar 2019 16:14:08 +0100 Subject: [PATCH] Remove throws IOException from PipelineAggregationBuilder#create (#40222) IOException are never thrown in any of the existing pipeline aggregation builders. Removing the throws IOException from the create method allows to remove it also from a couple of other methods which ends up simplifying AggregationPhase (one less catch). --- .../search/aggregations/AggregationPhase.java | 24 ++++++++----------- .../aggregations/AggregatorFactories.java | 2 +- .../PipelineAggregationBuilder.java | 3 +-- .../AbstractPipelineAggregationBuilder.java | 4 ++-- .../AvgBucketPipelineAggregationBuilder.java | 2 +- ...cketMetricsPipelineAggregationBuilder.java | 2 +- ...ucketScriptPipelineAggregationBuilder.java | 2 +- ...ketSelectorPipelineAggregationBuilder.java | 2 +- .../BucketSortPipelineAggregationBuilder.java | 2 +- ...mulativeSumPipelineAggregationBuilder.java | 2 +- .../DerivativePipelineAggregationBuilder.java | 2 +- ...StatsBucketPipelineAggregationBuilder.java | 2 +- .../MaxBucketPipelineAggregationBuilder.java | 2 +- .../MinBucketPipelineAggregationBuilder.java | 2 +- .../MovAvgPipelineAggregationBuilder.java | 2 +- .../MovFnPipelineAggregationBuilder.java | 2 +- ...tilesBucketPipelineAggregationBuilder.java | 2 +- .../SerialDiffPipelineAggregationBuilder.java | 2 +- ...StatsBucketPipelineAggregationBuilder.java | 2 +- .../SumBucketPipelineAggregationBuilder.java | 2 +- .../search/SearchModuleTests.java | 2 +- .../InternalAggregationsTests.java | 4 ++-- 22 files changed, 33 insertions(+), 38 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/AggregationPhase.java b/server/src/main/java/org/elasticsearch/search/aggregations/AggregationPhase.java index 75ef3c9199a..4c614626d1d 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/AggregationPhase.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/AggregationPhase.java @@ -133,22 +133,18 @@ public class AggregationPhase implements SearchPhase { } } context.queryResult().aggregations(new InternalAggregations(aggregations)); - try { - List pipelineAggregators = context.aggregations().factories().createPipelineAggregators(); - List siblingPipelineAggregators = new ArrayList<>(pipelineAggregators.size()); - for (PipelineAggregator pipelineAggregator : pipelineAggregators) { - if (pipelineAggregator instanceof SiblingPipelineAggregator) { - siblingPipelineAggregators.add((SiblingPipelineAggregator) pipelineAggregator); - } else { - throw new AggregationExecutionException("Invalid pipeline aggregation named [" + pipelineAggregator.name() - + "] of type [" + pipelineAggregator.getWriteableName() + "]. Only sibling pipeline aggregations are " - + "allowed at the top level"); - } + List pipelineAggregators = context.aggregations().factories().createPipelineAggregators(); + List siblingPipelineAggregators = new ArrayList<>(pipelineAggregators.size()); + for (PipelineAggregator pipelineAggregator : pipelineAggregators) { + if (pipelineAggregator instanceof SiblingPipelineAggregator) { + siblingPipelineAggregators.add((SiblingPipelineAggregator) pipelineAggregator); + } else { + throw new AggregationExecutionException("Invalid pipeline aggregation named [" + pipelineAggregator.name() + + "] of type [" + pipelineAggregator.getWriteableName() + "]. Only sibling pipeline aggregations are " + + "allowed at the top level"); } - context.queryResult().pipelineAggregators(siblingPipelineAggregators); - } catch (IOException e) { - throw new AggregationExecutionException("Failed to build top level pipeline aggregators", e); } + context.queryResult().pipelineAggregators(siblingPipelineAggregators); // disable aggregations so that they don't run on next pages in case of scrolling context.aggregations(null); diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/AggregatorFactories.java b/server/src/main/java/org/elasticsearch/search/aggregations/AggregatorFactories.java index 9683651391c..5c1120452f6 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/AggregatorFactories.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/AggregatorFactories.java @@ -175,7 +175,7 @@ public class AggregatorFactories { this.pipelineAggregatorFactories = pipelineAggregators; } - public List createPipelineAggregators() throws IOException { + public List createPipelineAggregators() { List pipelineAggregators = new ArrayList<>(this.pipelineAggregatorFactories.size()); for (PipelineAggregationBuilder factory : this.pipelineAggregatorFactories) { pipelineAggregators.add(factory.create()); diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/PipelineAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/PipelineAggregationBuilder.java index 8b66738848f..08938a5b9b9 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/PipelineAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/PipelineAggregationBuilder.java @@ -24,7 +24,6 @@ import org.elasticsearch.common.xcontent.ToXContentFragment; import org.elasticsearch.search.aggregations.AggregatorFactories.Builder; import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator; -import java.io.IOException; import java.util.Collection; import java.util.Map; @@ -76,7 +75,7 @@ public abstract class PipelineAggregationBuilder implements NamedWriteable, Base * * @return The created aggregator */ - protected abstract PipelineAggregator create() throws IOException; + protected abstract PipelineAggregator create(); /** Associate metadata with this {@link PipelineAggregationBuilder}. */ @Override diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/AbstractPipelineAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/AbstractPipelineAggregationBuilder.java index 409f5201035..8a278ead95a 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/AbstractPipelineAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/AbstractPipelineAggregationBuilder.java @@ -89,7 +89,7 @@ public abstract class AbstractPipelineAggregationBuilder metaData) throws IOException; + protected abstract PipelineAggregator createInternal(Map metaData); /** * Creates the pipeline aggregator @@ -97,7 +97,7 @@ public abstract class AbstractPipelineAggregationBuilder metaData) throws IOException { + protected PipelineAggregator createInternal(Map metaData) { return new AvgBucketPipelineAggregator(name, bucketsPaths, gapPolicy(), formatter(), metaData); } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/BucketMetricsPipelineAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/BucketMetricsPipelineAggregationBuilder.java index 27da9dea530..eddc48c6fdc 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/BucketMetricsPipelineAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/BucketMetricsPipelineAggregationBuilder.java @@ -104,7 +104,7 @@ public abstract class BucketMetricsPipelineAggregationBuilder metaData) throws IOException; + protected abstract PipelineAggregator createInternal(Map metaData); @Override public void doValidate(AggregatorFactory parent, Collection aggBuilders, diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/BucketScriptPipelineAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/BucketScriptPipelineAggregationBuilder.java index db56779559a..c8ea3553752 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/BucketScriptPipelineAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/BucketScriptPipelineAggregationBuilder.java @@ -139,7 +139,7 @@ public class BucketScriptPipelineAggregationBuilder extends AbstractPipelineAggr } @Override - protected PipelineAggregator createInternal(Map metaData) throws IOException { + protected PipelineAggregator createInternal(Map metaData) { return new BucketScriptPipelineAggregator(name, bucketsPathsMap, script, formatter(), gapPolicy, metaData); } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/BucketSelectorPipelineAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/BucketSelectorPipelineAggregationBuilder.java index f0497932b21..a6627be1cfe 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/BucketSelectorPipelineAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/BucketSelectorPipelineAggregationBuilder.java @@ -108,7 +108,7 @@ public class BucketSelectorPipelineAggregationBuilder extends AbstractPipelineAg } @Override - protected PipelineAggregator createInternal(Map metaData) throws IOException { + protected PipelineAggregator createInternal(Map metaData) { return new BucketSelectorPipelineAggregator(name, bucketsPathsMap, script, gapPolicy, metaData); } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/BucketSortPipelineAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/BucketSortPipelineAggregationBuilder.java index 0ce4c087206..4dcd42934fc 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/BucketSortPipelineAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/BucketSortPipelineAggregationBuilder.java @@ -139,7 +139,7 @@ public class BucketSortPipelineAggregationBuilder extends AbstractPipelineAggreg } @Override - protected PipelineAggregator createInternal(Map metaData) throws IOException { + protected PipelineAggregator createInternal(Map metaData) { return new BucketSortPipelineAggregator(name, sorts, from, size, gapPolicy, metaData); } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/CumulativeSumPipelineAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/CumulativeSumPipelineAggregationBuilder.java index 207b1e35bef..a8b51d9c1a5 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/CumulativeSumPipelineAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/CumulativeSumPipelineAggregationBuilder.java @@ -88,7 +88,7 @@ public class CumulativeSumPipelineAggregationBuilder extends AbstractPipelineAgg } @Override - protected PipelineAggregator createInternal(Map metaData) throws IOException { + protected PipelineAggregator createInternal(Map metaData) { return new CumulativeSumPipelineAggregator(name, bucketsPaths, formatter(), metaData); } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/DerivativePipelineAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/DerivativePipelineAggregationBuilder.java index 68ec9085df5..25f30bc8343 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/DerivativePipelineAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/DerivativePipelineAggregationBuilder.java @@ -129,7 +129,7 @@ public class DerivativePipelineAggregationBuilder extends AbstractPipelineAggreg } @Override - protected PipelineAggregator createInternal(Map metaData) throws IOException { + protected PipelineAggregator createInternal(Map metaData) { DocValueFormat formatter; if (format != null) { formatter = new DocValueFormat.Decimal(format); diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/ExtendedStatsBucketPipelineAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/ExtendedStatsBucketPipelineAggregationBuilder.java index 10347e40354..3d16cf91ee0 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/ExtendedStatsBucketPipelineAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/ExtendedStatsBucketPipelineAggregationBuilder.java @@ -75,7 +75,7 @@ public class ExtendedStatsBucketPipelineAggregationBuilder } @Override - protected PipelineAggregator createInternal(Map metaData) throws IOException { + protected PipelineAggregator createInternal(Map metaData) { return new ExtendedStatsBucketPipelineAggregator(name, bucketsPaths, sigma, gapPolicy(), formatter(), metaData); } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/MaxBucketPipelineAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/MaxBucketPipelineAggregationBuilder.java index 852a3e378d0..b335c15865d 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/MaxBucketPipelineAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/MaxBucketPipelineAggregationBuilder.java @@ -46,7 +46,7 @@ public class MaxBucketPipelineAggregationBuilder extends BucketMetricsPipelineAg } @Override - protected PipelineAggregator createInternal(Map metaData) throws IOException { + protected PipelineAggregator createInternal(Map metaData) { return new MaxBucketPipelineAggregator(name, bucketsPaths, gapPolicy(), formatter(), metaData); } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/MinBucketPipelineAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/MinBucketPipelineAggregationBuilder.java index b44ee869e2c..405285993c0 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/MinBucketPipelineAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/MinBucketPipelineAggregationBuilder.java @@ -46,7 +46,7 @@ public class MinBucketPipelineAggregationBuilder extends BucketMetricsPipelineAg } @Override - protected PipelineAggregator createInternal(Map metaData) throws IOException { + protected PipelineAggregator createInternal(Map metaData) { return new MinBucketPipelineAggregator(name, bucketsPaths, gapPolicy(), formatter(), metaData); } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/MovAvgPipelineAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/MovAvgPipelineAggregationBuilder.java index 02fa9795633..4a552f47d53 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/MovAvgPipelineAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/MovAvgPipelineAggregationBuilder.java @@ -250,7 +250,7 @@ public class MovAvgPipelineAggregationBuilder extends AbstractPipelineAggregatio } @Override - protected PipelineAggregator createInternal(Map metaData) throws IOException { + protected PipelineAggregator createInternal(Map metaData) { // If the user doesn't set a preference for cost minimization, ask // what the model prefers boolean minimize = this.minimize == null ? model.minimizeByDefault() : this.minimize; diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/MovFnPipelineAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/MovFnPipelineAggregationBuilder.java index 998f6e387a8..3f589d8a089 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/MovFnPipelineAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/MovFnPipelineAggregationBuilder.java @@ -179,7 +179,7 @@ public class MovFnPipelineAggregationBuilder extends AbstractPipelineAggregation } @Override - protected PipelineAggregator createInternal(Map metaData) throws IOException { + protected PipelineAggregator createInternal(Map metaData) { return new MovFnPipelineAggregator(name, bucketsPathString, script, window, formatter(), gapPolicy, metaData); } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/PercentilesBucketPipelineAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/PercentilesBucketPipelineAggregationBuilder.java index a3ac2201777..31848d21591 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/PercentilesBucketPipelineAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/PercentilesBucketPipelineAggregationBuilder.java @@ -112,7 +112,7 @@ public class PercentilesBucketPipelineAggregationBuilder } @Override - protected PipelineAggregator createInternal(Map metaData) throws IOException { + protected PipelineAggregator createInternal(Map metaData) { return new PercentilesBucketPipelineAggregator(name, percents, keyed, bucketsPaths, gapPolicy(), formatter(), metaData); } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/SerialDiffPipelineAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/SerialDiffPipelineAggregationBuilder.java index f634493749d..019026740f8 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/SerialDiffPipelineAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/SerialDiffPipelineAggregationBuilder.java @@ -135,7 +135,7 @@ public class SerialDiffPipelineAggregationBuilder extends AbstractPipelineAggreg } @Override - protected PipelineAggregator createInternal(Map metaData) throws IOException { + protected PipelineAggregator createInternal(Map metaData) { return new SerialDiffPipelineAggregator(name, bucketsPaths, formatter(), gapPolicy, lag, metaData); } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/StatsBucketPipelineAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/StatsBucketPipelineAggregationBuilder.java index f943f3318fc..904cc16c290 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/StatsBucketPipelineAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/StatsBucketPipelineAggregationBuilder.java @@ -47,7 +47,7 @@ public class StatsBucketPipelineAggregationBuilder extends BucketMetricsPipeline } @Override - protected PipelineAggregator createInternal(Map metaData) throws IOException { + protected PipelineAggregator createInternal(Map metaData) { return new StatsBucketPipelineAggregator(name, bucketsPaths, gapPolicy(), formatter(), metaData); } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/SumBucketPipelineAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/SumBucketPipelineAggregationBuilder.java index 920f7e9b0ac..eb075d43689 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/SumBucketPipelineAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/SumBucketPipelineAggregationBuilder.java @@ -46,7 +46,7 @@ public class SumBucketPipelineAggregationBuilder extends BucketMetricsPipelineAg } @Override - protected PipelineAggregator createInternal(Map metaData) throws IOException { + protected PipelineAggregator createInternal(Map metaData) { return new SumBucketPipelineAggregator(name, bucketsPaths, gapPolicy(), formatter(), metaData); } diff --git a/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java b/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java index 2a54cda752a..3594022e223 100644 --- a/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java +++ b/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java @@ -442,7 +442,7 @@ public class SearchModuleTests extends ESTestCase { } @Override - protected PipelineAggregator createInternal(Map metaData) throws IOException { + protected PipelineAggregator createInternal(Map metaData) { return null; } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/InternalAggregationsTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/InternalAggregationsTests.java index 3212c18cf27..45107fa4a19 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/InternalAggregationsTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/InternalAggregationsTests.java @@ -53,7 +53,7 @@ public class InternalAggregationsTests extends ESTestCase { assertNull(InternalAggregations.reduce(aggs, Collections.emptyList(), reduceContext)); } - public void testNonFinalReduceTopLevelPipelineAggs() throws IOException { + public void testNonFinalReduceTopLevelPipelineAggs() { InternalAggregation terms = new StringTerms("name", BucketOrder.key(true), 10, 1, Collections.emptyList(), Collections.emptyMap(), DocValueFormat.RAW, 25, false, 10, Collections.emptyList(), 0); List aggs = Collections.singletonList(new InternalAggregations(Collections.singletonList(terms))); @@ -66,7 +66,7 @@ public class InternalAggregationsTests extends ESTestCase { assertEquals(1, reducedAggs.aggregations.size()); } - public void testFinalReduceTopLevelPipelineAggs() throws IOException { + public void testFinalReduceTopLevelPipelineAggs() { InternalAggregation terms = new StringTerms("name", BucketOrder.key(true), 10, 1, Collections.emptyList(), Collections.emptyMap(), DocValueFormat.RAW, 25, false, 10, Collections.emptyList(), 0);