diff --git a/server/src/main/java/org/elasticsearch/action/search/SearchRequest.java b/server/src/main/java/org/elasticsearch/action/search/SearchRequest.java index 761a50bfa4168..ea321ae3ec266 100644 --- a/server/src/main/java/org/elasticsearch/action/search/SearchRequest.java +++ b/server/src/main/java/org/elasticsearch/action/search/SearchRequest.java @@ -269,6 +269,11 @@ public ActionRequestValidationException validate() { addValidationError("[request_cache] cannot be used in a scroll context", validationException); } } + if (source != null) { + if (source.aggregations() != null) { + validationException = source.aggregations().validate(validationException); + } + } return validationException; } 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 1998a53ad06b0..397c6ea762cf1 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/AggregatorFactories.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/AggregatorFactories.java @@ -18,6 +18,7 @@ */ package org.elasticsearch.search.aggregations; +import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.StreamInput; @@ -283,8 +284,6 @@ public boolean mustVisitAllDocs() { return false; } - - public Builder addAggregator(AggregationBuilder factory) { if (!names.add(factory.name)) { throw new IllegalArgumentException("Two sibling aggregations cannot have the same name: [" + factory.name + "]"); @@ -298,15 +297,51 @@ public Builder addPipelineAggregator(PipelineAggregationBuilder pipelineAggregat return this; } + /** + * Validate the root of the aggregation tree. + */ + public ActionRequestValidationException validate(ActionRequestValidationException e) { + PipelineAggregationBuilder.ValidationContext context = + PipelineAggregationBuilder.ValidationContext.forTreeRoot(aggregationBuilders, pipelineAggregatorBuilders, e); + validatePipelines(context); + return validateChildren(context.getValidationException()); + } + + /** + * Validate a the pipeline aggregations in this factory. + */ + private void validatePipelines(PipelineAggregationBuilder.ValidationContext context) { + List orderedPipelineAggregators; + try { + orderedPipelineAggregators = resolvePipelineAggregatorOrder(pipelineAggregatorBuilders, aggregationBuilders); + } catch (IllegalArgumentException iae) { + context.addValidationError(iae.getMessage()); + return; + } + for (PipelineAggregationBuilder builder : orderedPipelineAggregators) { + builder.validate(context); + } + } + + /** + * Validate a the children of this factory. + */ + private ActionRequestValidationException validateChildren(ActionRequestValidationException e) { + for (AggregationBuilder agg : aggregationBuilders) { + PipelineAggregationBuilder.ValidationContext context = + PipelineAggregationBuilder.ValidationContext.forInsideTree(agg, e); + agg.factoriesBuilder.validatePipelines(context); + e = agg.factoriesBuilder.validateChildren(context.getValidationException()); + } + return e; + } + public AggregatorFactories build(QueryShardContext queryShardContext, AggregatorFactory parent) throws IOException { if (aggregationBuilders.isEmpty() && pipelineAggregatorBuilders.isEmpty()) { return EMPTY; } - List orderedpipelineAggregators = null; - orderedpipelineAggregators = resolvePipelineAggregatorOrder(this.pipelineAggregatorBuilders, this.aggregationBuilders); - for (PipelineAggregationBuilder builder : orderedpipelineAggregators) { - builder.validate(parent, aggregationBuilders, pipelineAggregatorBuilders); - } + List orderedPipelineAggregators = + resolvePipelineAggregatorOrder(pipelineAggregatorBuilders, aggregationBuilders); AggregatorFactory[] aggFactories = new AggregatorFactory[aggregationBuilders.size()]; int i = 0; @@ -314,7 +349,7 @@ public AggregatorFactories build(QueryShardContext queryShardContext, Aggregator aggFactories[i] = agg.build(queryShardContext, parent); ++i; } - return new AggregatorFactories(aggFactories, orderedpipelineAggregators); + return new AggregatorFactories(aggFactories, orderedPipelineAggregators); } private List resolvePipelineAggregatorOrder( 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 85b9a7dcb0ca5..aaffcfc440bce 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/PipelineAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/PipelineAggregationBuilder.java @@ -18,14 +18,20 @@ */ package org.elasticsearch.search.aggregations; +import org.elasticsearch.action.ActionRequestValidationException; +import org.elasticsearch.action.ValidateActions; import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.NamedWriteable; import org.elasticsearch.common.xcontent.ToXContentFragment; import org.elasticsearch.search.aggregations.AggregatorFactories.Builder; +import org.elasticsearch.search.aggregations.bucket.histogram.AutoDateHistogramAggregationBuilder; +import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramAggregationBuilder; +import org.elasticsearch.search.aggregations.bucket.histogram.HistogramAggregationBuilder; import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator; import java.util.Collection; import java.util.Map; +import java.util.Objects; /** * A factory that knows how to create an {@link PipelineAggregator} of a @@ -64,11 +70,145 @@ public final String[] getBucketsPaths() { } /** - * Internal: Validates the state of this factory (makes sure the factory is properly - * configured) + * Makes sure this builder is properly configured. */ - protected abstract void validate(AggregatorFactory parent, Collection aggregationBuilders, - Collection pipelineAggregatorBuilders); + protected abstract void validate(ValidationContext context); + public abstract static class ValidationContext { + /** + * Build the context for the root of the aggregation tree. + */ + public static ValidationContext forTreeRoot(Collection siblingAggregations, + Collection siblingPipelineAggregations, + ActionRequestValidationException validationFailuresSoFar) { + return new ForTreeRoot(siblingAggregations, siblingPipelineAggregations, validationFailuresSoFar); + } + + /** + * Build the context for a node inside the aggregation tree. + */ + public static ValidationContext forInsideTree(AggregationBuilder parent, + ActionRequestValidationException validationFailuresSoFar) { + return new ForInsideTree(parent, validationFailuresSoFar); + } + + + private ActionRequestValidationException e; + + private ValidationContext(ActionRequestValidationException validationFailuresSoFar) { + this.e = validationFailuresSoFar; + } + + private static class ForTreeRoot extends ValidationContext { + private final Collection siblingAggregations; + private final Collection siblingPipelineAggregations; + + ForTreeRoot(Collection siblingAggregations, + Collection siblingPipelineAggregations, + ActionRequestValidationException validationFailuresSoFar) { + super(validationFailuresSoFar); + this.siblingAggregations = Objects.requireNonNull(siblingAggregations); + this.siblingPipelineAggregations = Objects.requireNonNull(siblingPipelineAggregations); + } + + @Override + public Collection getSiblingAggregations() { + return siblingAggregations; + } + + @Override + public Collection getSiblingPipelineAggregations() { + return siblingPipelineAggregations; + } + + @Override + public void validateParentAggSequentiallyOrdered(String type, String name) { + addValidationError(type + " aggregation [" + name + + "] must have a histogram, date_histogram or auto_date_histogram as parent but doesn't have a parent"); + } + } + + private static class ForInsideTree extends ValidationContext { + private final AggregationBuilder parent; + + ForInsideTree(AggregationBuilder parent, ActionRequestValidationException validationFailuresSoFar) { + super(validationFailuresSoFar); + this.parent = Objects.requireNonNull(parent); + } + + @Override + public Collection getSiblingAggregations() { + return parent.getSubAggregations(); + } + + @Override + public Collection getSiblingPipelineAggregations() { + return parent.getPipelineAggregations(); + } + + @Override + public void validateParentAggSequentiallyOrdered(String type, String name) { + if (parent instanceof HistogramAggregationBuilder) { + HistogramAggregationBuilder histoParent = (HistogramAggregationBuilder) parent; + if (histoParent.minDocCount() != 0) { + addValidationError( + "parent histogram of " + type + " aggregation [" + name + "] must have min_doc_count of 0"); + } + } else if (parent instanceof DateHistogramAggregationBuilder) { + DateHistogramAggregationBuilder histoParent = (DateHistogramAggregationBuilder) parent; + if (histoParent.minDocCount() != 0) { + addValidationError( + "parent histogram of " + type + " aggregation [" + name + "] must have min_doc_count of 0"); + } + } else if (parent instanceof AutoDateHistogramAggregationBuilder) { + // Nothing to check + } else { + addValidationError( + type + " aggregation [" + name + "] must have a histogram, date_histogram or auto_date_histogram as parent"); + } + } + } + + /** + * Aggregations that are siblings to the aggregation being validated. + */ + public abstract Collection getSiblingAggregations(); + + /** + * Pipeline aggregations that are siblings to the aggregation being validated. + */ + public abstract Collection getSiblingPipelineAggregations(); + + /** + * Add a validation error to this context. All validation errors + * are accumulated in a list and, if there are any, the request + * is not executed and the entire list is returned as the error + * response. + */ + public void addValidationError(String error) { + e = ValidateActions.addValidationError(error, e); + } + + /** + * Add a validation error about the {@code buckets_path}. + */ + public void addBucketPathValidationError(String error) { + addValidationError(PipelineAggregator.Parser.BUCKETS_PATH.getPreferredName() + ' ' + error); + } + + /** + * Validates that the parent is sequentially ordered. + */ + public abstract void validateParentAggSequentiallyOrdered(String type, String name); + + /** + * The validation exception, if there is one. It'll be {@code null} + * if the context wasn't provided with any exception on creation + * and none were added. + */ + public ActionRequestValidationException getValidationException() { + return e; + } + } /** * Creates the pipeline aggregator 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 35eab881bb643..c2bffc0450677 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 @@ -22,16 +22,10 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.search.aggregations.AggregationBuilder; -import org.elasticsearch.search.aggregations.AggregatorFactory; import org.elasticsearch.search.aggregations.PipelineAggregationBuilder; -import org.elasticsearch.search.aggregations.bucket.histogram.AutoDateHistogramAggregatorFactory; -import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramAggregatorFactory; -import org.elasticsearch.search.aggregations.bucket.histogram.HistogramAggregatorFactory; import java.io.IOException; import java.util.Arrays; -import java.util.Collection; import java.util.Map; import java.util.Objects; @@ -79,16 +73,6 @@ public String type() { return type; } - /** - * Validates the state of this factory (makes sure the factory is properly - * configured) - */ - @Override - public final void validate(AggregatorFactory parent, Collection factories, - Collection pipelineAggregatorFactories) { - doValidate(parent, factories, pipelineAggregatorFactories); - } - protected abstract PipelineAggregator createInternal(Map metaData); /** @@ -102,32 +86,6 @@ public final PipelineAggregator create() { return aggregator; } - public void doValidate(AggregatorFactory parent, Collection factories, - Collection pipelineAggregatorFactories) { - } - - /** - * Validates pipeline aggregations that need sequentially ordered data. - */ - public static void validateSequentiallyOrderedParentAggs(AggregatorFactory parent, String type, String name) { - if ((parent instanceof HistogramAggregatorFactory || parent instanceof DateHistogramAggregatorFactory - || parent instanceof AutoDateHistogramAggregatorFactory) == false) { - throw new IllegalStateException( - type + " aggregation [" + name + "] must have a histogram, date_histogram or auto_date_histogram as parent"); - } - if (parent instanceof HistogramAggregatorFactory) { - HistogramAggregatorFactory histoParent = (HistogramAggregatorFactory) parent; - if (histoParent.minDocCount() != 0) { - throw new IllegalStateException("parent histogram of " + type + " aggregation [" + name + "] must have min_doc_count of 0"); - } - } else if (parent instanceof DateHistogramAggregatorFactory) { - DateHistogramAggregatorFactory histoParent = (DateHistogramAggregatorFactory) parent; - if (histoParent.minDocCount() != 0) { - throw new IllegalStateException("parent histogram of " + type + " aggregation [" + name + "] must have min_doc_count of 0"); - } - } - } - @SuppressWarnings("unchecked") @Override public PAB setMetaData(Map 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 0c4aa6f1359bd..753f3ca38821c 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 @@ -24,13 +24,10 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.search.DocValueFormat; import org.elasticsearch.search.aggregations.AggregationBuilder; -import org.elasticsearch.search.aggregations.AggregatorFactory; -import org.elasticsearch.search.aggregations.PipelineAggregationBuilder; import org.elasticsearch.search.aggregations.bucket.MultiBucketAggregationBuilder; import org.elasticsearch.search.aggregations.pipeline.BucketHelpers.GapPolicy; import java.io.IOException; -import java.util.Collection; import java.util.Map; import java.util.Objects; import java.util.Optional; @@ -107,28 +104,27 @@ public GapPolicy gapPolicy() { protected abstract PipelineAggregator createInternal(Map metaData); @Override - public void doValidate(AggregatorFactory parent, Collection aggBuilders, - Collection pipelineAggregatorFactories) { + protected void validate(ValidationContext context) { if (bucketsPaths.length != 1) { - throw new IllegalStateException(PipelineAggregator.Parser.BUCKETS_PATH.getPreferredName() - + " must contain a single entry for aggregation [" + name + "]"); + context.addBucketPathValidationError("must contain a single entry for aggregation [" + name + "]"); + return; } // Need to find the first agg name in the buckets path to check its a // multi bucket agg: aggs are split with '>' and can optionally have a // metric name after them by using '.' so need to split on both to get // just the agg name final String firstAgg = bucketsPaths[0].split("[>\\.]")[0]; - Optional aggBuilder = aggBuilders.stream().filter((builder) -> builder.getName().equals(firstAgg)) + Optional aggBuilder = context.getSiblingAggregations().stream() + .filter(builder -> builder.getName().equals(firstAgg)) .findAny(); - if (aggBuilder.isPresent()) { - if ((aggBuilder.get() instanceof MultiBucketAggregationBuilder) == false) { - throw new IllegalArgumentException("The first aggregation in " + PipelineAggregator.Parser.BUCKETS_PATH.getPreferredName() - + " must be a multi-bucket aggregation for aggregation [" + name + "] found :" - + aggBuilder.get().getClass().getName() + " for buckets path: " + bucketsPaths[0]); - } - } else { - throw new IllegalArgumentException(PipelineAggregator.Parser.BUCKETS_PATH.getPreferredName() - + " aggregation does not exist for aggregation [" + name + "]: " + bucketsPaths[0]); + if (aggBuilder.isEmpty()) { + context.addBucketPathValidationError("aggregation does not exist for aggregation [" + name + "]: " + bucketsPaths[0]); + return; + } + if ((aggBuilder.get() instanceof MultiBucketAggregationBuilder) == false) { + context.addValidationError("The first aggregation in " + PipelineAggregator.Parser.BUCKETS_PATH.getPreferredName() + + " must be a multi-bucket aggregation for aggregation [" + name + "] found :" + + aggBuilder.get().getClass().getName() + " for buckets path: " + bucketsPaths[0]); } } 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 5fddd5f1a9e1c..fa6c6b7639413 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 @@ -198,6 +198,11 @@ protected XContentBuilder internalXContent(XContentBuilder builder, Params param return builder; } + @Override + protected void validate(ValidationContext context) { + // Nothing to check + } + @Override protected boolean overrideBucketsPath() { return true; 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 6bb83c60dea83..0981acbc9bafd 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 @@ -193,6 +193,11 @@ public static BucketSelectorPipelineAggregationBuilder parse(String reducerName, return factory; } + @Override + protected void validate(ValidationContext context) { + // Nothing to check + } + @Override protected boolean overrideBucketsPath() { return true; 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 1e80c6f78c3c5..7f6b491975868 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 @@ -25,9 +25,6 @@ import org.elasticsearch.common.xcontent.ObjectParser; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.search.aggregations.AggregationBuilder; -import org.elasticsearch.search.aggregations.AggregatorFactory; -import org.elasticsearch.search.aggregations.PipelineAggregationBuilder; import org.elasticsearch.search.aggregations.pipeline.BucketHelpers.GapPolicy; import org.elasticsearch.search.builder.SearchSourceBuilder; import org.elasticsearch.search.sort.FieldSortBuilder; @@ -36,7 +33,6 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Locale; @@ -144,10 +140,9 @@ protected PipelineAggregator createInternal(Map metaData) { } @Override - public void doValidate(AggregatorFactory parent, Collection aggFactories, - Collection pipelineAggregatoractories) { + protected void validate(ValidationContext context) { if (sorts.isEmpty() && size == null && from == 0) { - throw new IllegalStateException("[" + name + "] is configured to perform nothing. Please set either of " + context.addValidationError("[" + name + "] is configured to perform nothing. Please set either of " + Arrays.asList(SearchSourceBuilder.SORT_FIELD.getPreferredName(), SIZE.getPreferredName(), FROM.getPreferredName()) + " to use " + NAME); } 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 fc9b8caf72769..c1af597b4c275 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 @@ -25,13 +25,9 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.search.DocValueFormat; -import org.elasticsearch.search.aggregations.AggregationBuilder; -import org.elasticsearch.search.aggregations.AggregatorFactory; -import org.elasticsearch.search.aggregations.PipelineAggregationBuilder; import java.io.IOException; import java.util.ArrayList; -import java.util.Collection; import java.util.List; import java.util.Map; import java.util.Objects; @@ -93,14 +89,11 @@ protected PipelineAggregator createInternal(Map metaData) { } @Override - public void doValidate(AggregatorFactory parent, Collection aggFactories, - Collection pipelineAggregatorFactories) { + protected void validate(ValidationContext context) { if (bucketsPaths.length != 1) { - throw new IllegalStateException(BUCKETS_PATH.getPreferredName() - + " must contain a single entry for aggregation [" + name + "]"); + context.addBucketPathValidationError("must contain a single entry for aggregation [" + name + "]"); } - - validateSequentiallyOrderedParentAggs(parent, NAME, name); + context.validateParentAggSequentiallyOrdered(NAME, name); } @Override 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 4aa4155ac063f..106a8a97f5728 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 @@ -28,16 +28,12 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.search.DocValueFormat; -import org.elasticsearch.search.aggregations.AggregationBuilder; -import org.elasticsearch.search.aggregations.AggregatorFactory; -import org.elasticsearch.search.aggregations.PipelineAggregationBuilder; import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramAggregationBuilder; import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramInterval; import org.elasticsearch.search.aggregations.pipeline.BucketHelpers.GapPolicy; import java.io.IOException; import java.util.ArrayList; -import java.util.Collection; import java.util.List; import java.util.Map; import java.util.Objects; @@ -152,14 +148,13 @@ protected PipelineAggregator createInternal(Map metaData) { } @Override - public void doValidate(AggregatorFactory parent, Collection aggFactories, - Collection pipelineAggregatoractories) { + protected void validate(ValidationContext context) { if (bucketsPaths.length != 1) { - throw new IllegalStateException(PipelineAggregator.Parser.BUCKETS_PATH.getPreferredName() + context.addValidationError(PipelineAggregator.Parser.BUCKETS_PATH.getPreferredName() + " must contain a single entry for aggregation [" + name + "]"); } - - validateSequentiallyOrderedParentAggs(parent, NAME, name); + + context.validateParentAggSequentiallyOrdered(NAME, name); } @Override 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 89816a1fb22e8..97948d0e8638a 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 @@ -22,12 +22,8 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.search.aggregations.AggregationBuilder; -import org.elasticsearch.search.aggregations.AggregatorFactory; -import org.elasticsearch.search.aggregations.PipelineAggregationBuilder; import java.io.IOException; -import java.util.Collection; import java.util.Map; import java.util.Objects; @@ -80,13 +76,10 @@ protected PipelineAggregator createInternal(Map metaData) { } @Override - public void doValidate(AggregatorFactory parent, Collection aggBuilders, - Collection pipelineAggregatorFactories) { - super.doValidate(parent, aggBuilders, pipelineAggregatorFactories); - - if (sigma < 0.0 ) { - throw new IllegalStateException(ExtendedStatsBucketParser.SIGMA.getPreferredName() - + " must be a non-negative double"); + protected void validate(ValidationContext context) { + super.validate(context); + if (sigma < 0.0) { + context.addValidationError(ExtendedStatsBucketParser.SIGMA.getPreferredName() + " must be a non-negative double"); } } 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 d24710631d8fd..e5ec2780b0c14 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 @@ -30,13 +30,9 @@ import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.script.Script; import org.elasticsearch.search.DocValueFormat; -import org.elasticsearch.search.aggregations.AggregationBuilder; -import org.elasticsearch.search.aggregations.AggregatorFactory; -import org.elasticsearch.search.aggregations.PipelineAggregationBuilder; import org.elasticsearch.search.aggregations.pipeline.BucketHelpers.GapPolicy; import java.io.IOException; -import java.util.Collection; import java.util.Locale; import java.util.Map; import java.util.Objects; @@ -179,13 +175,11 @@ public void setShift(int shift) { } @Override - public void doValidate(AggregatorFactory parent, Collection aggFactories, - Collection pipelineAggregatorFactories) { + protected void validate(ValidationContext context) { if (window <= 0) { - throw new IllegalArgumentException("[" + WINDOW.getPreferredName() + "] must be a positive, non-zero integer."); + context.addValidationError("[" + WINDOW.getPreferredName() + "] must be a positive, non-zero integer."); } - - validateSequentiallyOrderedParentAggs(parent, NAME, name); + context.validateParentAggSequentiallyOrdered(NAME, name); } @Override 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 06a940af89c77..a806b54772140 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 @@ -20,18 +20,15 @@ package org.elasticsearch.search.aggregations.pipeline; import com.carrotsearch.hppc.DoubleArrayList; + import org.elasticsearch.common.ParseField; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.search.aggregations.AggregationBuilder; -import org.elasticsearch.search.aggregations.AggregatorFactory; -import org.elasticsearch.search.aggregations.PipelineAggregationBuilder; import java.io.IOException; import java.util.Arrays; -import java.util.Collection; import java.util.Map; import java.util.Objects; @@ -109,14 +106,13 @@ protected PipelineAggregator createInternal(Map metaData) { } @Override - public void doValidate(AggregatorFactory parent, Collection aggFactories, - Collection pipelineAggregatorFactories) { - super.doValidate(parent, aggFactories, pipelineAggregatorFactories); - + protected void validate(ValidationContext context) { + super.validate(context); for (Double p : percents) { if (p == null || p < 0.0 || p > 100.0) { - throw new IllegalStateException(PERCENTS_FIELD.getPreferredName() + context.addValidationError(PERCENTS_FIELD.getPreferredName() + " must only contain non-null doubles from 0.0-100.0 inclusive"); + return; } } } 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 3b02edf51579a..fe6bb921b9129 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 @@ -26,14 +26,10 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.search.DocValueFormat; -import org.elasticsearch.search.aggregations.AggregationBuilder; -import org.elasticsearch.search.aggregations.AggregatorFactory; -import org.elasticsearch.search.aggregations.PipelineAggregationBuilder; import org.elasticsearch.search.aggregations.pipeline.BucketHelpers.GapPolicy; import java.io.IOException; import java.util.ArrayList; -import java.util.Collection; import java.util.List; import java.util.Map; import java.util.Objects; @@ -138,11 +134,10 @@ protected DocValueFormat formatter() { protected PipelineAggregator createInternal(Map metaData) { return new SerialDiffPipelineAggregator(name, bucketsPaths, formatter(), gapPolicy, lag, metaData); } - + @Override - public void doValidate(AggregatorFactory parent, Collection aggFactories, - Collection pipelineAggregatoractories) { - validateSequentiallyOrderedParentAggs(parent, NAME, name); + protected void validate(ValidationContext context) { + context.validateParentAggSequentiallyOrdered(NAME, name); } @Override diff --git a/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java b/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java index ff26f49a92686..48daf474d0ded 100644 --- a/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java +++ b/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java @@ -436,6 +436,9 @@ protected XContentBuilder internalXContent(XContentBuilder builder, Params param private static TestPipelineAggregationBuilder fromXContent(String name, XContentParser p) { return null; } + + @Override + protected void validate(ValidationContext context) {} } /** diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/AbstractBucketMetricsTestCase.java b/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/AbstractBucketMetricsTestCase.java index 8f268cecaedd2..d53a147261cb1 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/AbstractBucketMetricsTestCase.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/AbstractBucketMetricsTestCase.java @@ -40,5 +40,4 @@ protected final PAF createTestAggregatorFactory() { } protected abstract PAF doCreateTestAggregatorFactory(String name, String bucketsPath); - } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/AvgBucketTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/AvgBucketTests.java index 0dc10cb7a7a13..2c5f0f9f343c3 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/AvgBucketTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/AvgBucketTests.java @@ -24,10 +24,12 @@ import org.elasticsearch.search.aggregations.bucket.terms.TermsAggregationBuilder; import org.elasticsearch.search.aggregations.support.ValueType; -import java.util.Collections; import java.util.HashSet; import java.util.Set; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.nullValue; + public class AvgBucketTests extends AbstractBucketMetricsTestCase { @Override @@ -43,23 +45,18 @@ public void testValidate() { aggBuilders.add(multiBucketAgg); // First try to point to a non-existent agg - final AvgBucketPipelineAggregationBuilder builder = new AvgBucketPipelineAggregationBuilder("name", "invalid_agg>metric"); - IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, - () -> builder.validate(null, aggBuilders, Collections.emptySet())); - assertEquals(PipelineAggregator.Parser.BUCKETS_PATH.getPreferredName() - + " aggregation does not exist for aggregation [name]: invalid_agg>metric", ex.getMessage()); + assertThat(validate(aggBuilders, new AvgBucketPipelineAggregationBuilder("name", "invalid_agg>metric")), equalTo( + "Validation Failed: 1: " + PipelineAggregator.Parser.BUCKETS_PATH.getPreferredName() + + " aggregation does not exist for aggregation [name]: invalid_agg>metric;")); // Now try to point to a single bucket agg - AvgBucketPipelineAggregationBuilder builder2 = new AvgBucketPipelineAggregationBuilder("name", "global>metric"); - ex = expectThrows(IllegalArgumentException.class, () -> builder2.validate(null, aggBuilders, Collections.emptySet())); - assertEquals("The first aggregation in " + PipelineAggregator.Parser.BUCKETS_PATH.getPreferredName() + assertThat(validate(aggBuilders, new AvgBucketPipelineAggregationBuilder("name", "global>metric")), equalTo( + "Validation Failed: 1: The first aggregation in " + PipelineAggregator.Parser.BUCKETS_PATH.getPreferredName() + " must be a multi-bucket aggregation for aggregation [name] found :" + GlobalAggregationBuilder.class.getName() - + " for buckets path: global>metric", ex.getMessage()); - - // Now try to point to a valid multi-bucket agg (no exception should be thrown) - AvgBucketPipelineAggregationBuilder builder3 = new AvgBucketPipelineAggregationBuilder("name", "terms>metric"); - builder3.validate(null, aggBuilders, Collections.emptySet()); + + " for buckets path: global>metric;")); + // Now try to point to a valid multi-bucket agg which is valid + assertThat(validate(aggBuilders, new AvgBucketPipelineAggregationBuilder("name", "terms>metric")), nullValue()); } } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/BucketSortIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/BucketSortIT.java index d5e70aff07b69..ead4870731b1f 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/BucketSortIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/BucketSortIT.java @@ -19,8 +19,8 @@ package org.elasticsearch.search.aggregations.pipeline; +import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.index.IndexRequestBuilder; -import org.elasticsearch.action.search.SearchPhaseExecutionException; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.xcontent.XContentBuilder; @@ -459,21 +459,21 @@ public void testEmptyBuckets() { } public void testInvalidPath() { - SearchPhaseExecutionException e = expectThrows(SearchPhaseExecutionException.class, + Exception e = expectThrows(ActionRequestValidationException.class, () -> client().prepareSearch(INDEX) .addAggregation(terms("foos").field(TERM_FIELD) .subAggregation(bucketSort("bucketSort", Arrays.asList(new FieldSortBuilder("invalid"))))) .get()); - assertThat(e.getCause().getMessage(), containsString("No aggregation found for path [invalid]")); + assertThat(e.getMessage(), containsString("No aggregation found for path [invalid]")); } public void testNeitherSortsNorSizeSpecifiedAndFromIsDefault_ShouldThrowValidation() { - SearchPhaseExecutionException e = expectThrows(SearchPhaseExecutionException.class, + Exception e = expectThrows(ActionRequestValidationException.class, () -> client().prepareSearch(INDEX) .addAggregation(terms("foos").field(TERM_FIELD) .subAggregation(bucketSort("bucketSort", Collections.emptyList()))) .get()); - assertThat(e.getCause().getMessage(), containsString("[bucketSort] is configured to perform nothing." + + assertThat(e.getMessage(), containsString("[bucketSort] is configured to perform nothing." + " Please set either of [sort, size, from] to use bucket_sort")); } } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/CumulativeSumAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/CumulativeSumAggregatorTests.java index 9d27663d275f7..dd3e971f4400b 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/CumulativeSumAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/CumulativeSumAggregatorTests.java @@ -38,8 +38,6 @@ import org.elasticsearch.search.aggregations.AggregationBuilder; import org.elasticsearch.search.aggregations.AggregatorTestCase; import org.elasticsearch.search.aggregations.InternalAggregation; -import org.elasticsearch.search.aggregations.PipelineAggregationBuilder; -import org.elasticsearch.search.aggregations.TestAggregatorFactory; import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramAggregationBuilder; import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramInterval; import org.elasticsearch.search.aggregations.bucket.histogram.Histogram; @@ -52,10 +50,7 @@ import java.io.IOException; import java.util.Arrays; -import java.util.Collections; -import java.util.HashSet; import java.util.List; -import java.util.Set; import java.util.function.Consumer; import static org.hamcrest.Matchers.equalTo; @@ -291,34 +286,6 @@ public void testNoBuckets() throws IOException { } }); } - - /** - * The validation should verify the parent aggregation is allowed. - */ - public void testValidate() throws IOException { - final Set aggBuilders = new HashSet<>(); - aggBuilders.add(new CumulativeSumPipelineAggregationBuilder("cusum", "sum")); - - final CumulativeSumPipelineAggregationBuilder builder = new CumulativeSumPipelineAggregationBuilder("name", "valid"); - builder.validate(PipelineAggregationHelperTests.getRandomSequentiallyOrderedParentAgg(), Collections.emptySet(), aggBuilders); - } - - /** - * The validation should throw an IllegalArgumentException, since parent - * aggregation is not a type of HistogramAggregatorFactory, - * DateHistogramAggregatorFactory or AutoDateHistogramAggregatorFactory. - */ - public void testValidateException() throws IOException { - final Set aggBuilders = new HashSet<>(); - aggBuilders.add(new CumulativeSumPipelineAggregationBuilder("cusum", "sum")); - TestAggregatorFactory parentFactory = TestAggregatorFactory.createInstance(); - - final CumulativeSumPipelineAggregationBuilder builder = new CumulativeSumPipelineAggregationBuilder("name", "invalid_agg>metric"); - IllegalStateException ex = expectThrows(IllegalStateException.class, - () -> builder.validate(parentFactory, Collections.emptySet(), aggBuilders)); - assertEquals("cumulative_sum aggregation [name] must have a histogram, date_histogram or auto_date_histogram as parent", - ex.getMessage()); - } private void executeTestCase(Query query, AggregationBuilder aggBuilder, Consumer verify) throws IOException { executeTestCase(query, aggBuilder, verify, indexWriter -> { diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/CumulativeSumTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/CumulativeSumTests.java index edf879ce77f68..f51b8ec479d8b 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/CumulativeSumTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/CumulativeSumTests.java @@ -19,10 +19,18 @@ package org.elasticsearch.search.aggregations.pipeline; +import org.elasticsearch.search.aggregations.AggregationBuilder; import org.elasticsearch.search.aggregations.BasePipelineAggregationTestCase; -public class CumulativeSumTests extends BasePipelineAggregationTestCase { +import java.io.IOException; + +import static java.util.Collections.emptyList; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.nullValue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +public class CumulativeSumTests extends BasePipelineAggregationTestCase { @Override protected CumulativeSumPipelineAggregationBuilder createTestAggregatorFactory() { String name = randomAlphaOfLengthBetween(3, 20); @@ -34,4 +42,23 @@ protected CumulativeSumPipelineAggregationBuilder createTestAggregatorFactory() return factory; } + public void testValidate() throws IOException { + assertThat(validate(PipelineAggregationHelperTests.getRandomSequentiallyOrderedParentAgg(), + new CumulativeSumPipelineAggregationBuilder("name", "valid")), nullValue()); + } + + public void testInvalidParent() throws IOException { + AggregationBuilder parent = mock(AggregationBuilder.class); + when(parent.getName()).thenReturn("name"); + + assertThat(validate(parent, new CumulativeSumPipelineAggregationBuilder("name", "invalid_agg>metric")), equalTo( + "Validation Failed: 1: cumulative_sum aggregation [name] must have a histogram, date_histogram " + + "or auto_date_histogram as parent;")); + } + + public void testNoParent() throws IOException { + assertThat(validate(emptyList(), new CumulativeSumPipelineAggregationBuilder("name", "invalid_agg>metric")), equalTo( + "Validation Failed: 1: cumulative_sum aggregation [name] must have a histogram, date_histogram " + + "or auto_date_histogram as parent but doesn't have a parent;")); + } } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/DerivativeTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/DerivativeTests.java index 7fd6e1e86266b..662778315f7e4 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/DerivativeTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/DerivativeTests.java @@ -19,16 +19,20 @@ package org.elasticsearch.search.aggregations.pipeline; +import org.elasticsearch.search.aggregations.AggregationBuilder; import org.elasticsearch.search.aggregations.BasePipelineAggregationTestCase; import org.elasticsearch.search.aggregations.PipelineAggregationBuilder; -import org.elasticsearch.search.aggregations.TestAggregatorFactory; import org.elasticsearch.search.aggregations.pipeline.BucketHelpers.GapPolicy; import java.io.IOException; -import java.util.Collections; import java.util.HashSet; import java.util.Set; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.nullValue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + public class DerivativeTests extends BasePipelineAggregationTestCase { @Override @@ -51,16 +55,13 @@ protected DerivativePipelineAggregationBuilder createTestAggregatorFactory() { } return factory; } - + /** * The validation should verify the parent aggregation is allowed. */ public void testValidate() throws IOException { - final Set aggBuilders = new HashSet<>(); - aggBuilders.add(new DerivativePipelineAggregationBuilder("deriv", "der")); - - final DerivativePipelineAggregationBuilder builder = new DerivativePipelineAggregationBuilder("name", "valid"); - builder.validate(PipelineAggregationHelperTests.getRandomSequentiallyOrderedParentAgg(), Collections.emptySet(), aggBuilders); + assertThat(validate(PipelineAggregationHelperTests.getRandomSequentiallyOrderedParentAgg(), + new DerivativePipelineAggregationBuilder("name", "valid")), nullValue()); } /** @@ -71,12 +72,11 @@ public void testValidate() throws IOException { public void testValidateException() throws IOException { final Set aggBuilders = new HashSet<>(); aggBuilders.add(new DerivativePipelineAggregationBuilder("deriv", "der")); - TestAggregatorFactory parentFactory = TestAggregatorFactory.createInstance(); + AggregationBuilder parent = mock(AggregationBuilder.class); + when(parent.getName()).thenReturn("name"); - final DerivativePipelineAggregationBuilder builder = new DerivativePipelineAggregationBuilder("name", "invalid_agg>metric"); - IllegalStateException ex = expectThrows(IllegalStateException.class, - () -> builder.validate(parentFactory, Collections.emptySet(), aggBuilders)); - assertEquals("derivative aggregation [name] must have a histogram, date_histogram or auto_date_histogram as parent", - ex.getMessage()); + assertThat(validate(parent, new DerivativePipelineAggregationBuilder("name", "invalid_agg>metric")), equalTo( + "Validation Failed: 1: derivative aggregation [name] must have a histogram, " + + "date_histogram or auto_date_histogram as parent;")); } } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/ExtendedStatsBucketTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/ExtendedStatsBucketTests.java index 9930541cb007e..90cc9be95e3be 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/ExtendedStatsBucketTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/ExtendedStatsBucketTests.java @@ -26,11 +26,11 @@ import org.elasticsearch.search.aggregations.bucket.terms.TermsAggregationBuilder; import org.elasticsearch.search.aggregations.support.ValueType; -import java.util.Collections; import java.util.HashSet; import java.util.Set; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.nullValue; public class ExtendedStatsBucketTests extends AbstractBucketMetricsTestCase { @@ -68,23 +68,17 @@ public void testValidate() { aggBuilders.add(multiBucketAgg); // First try to point to a non-existent agg - final ExtendedStatsBucketPipelineAggregationBuilder builder = new ExtendedStatsBucketPipelineAggregationBuilder("name", - "invalid_agg>metric"); - IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, - () -> builder.validate(null, aggBuilders, Collections.emptySet())); - assertEquals(PipelineAggregator.Parser.BUCKETS_PATH.getPreferredName() - + " aggregation does not exist for aggregation [name]: invalid_agg>metric", ex.getMessage()); + assertThat(validate(aggBuilders, new ExtendedStatsBucketPipelineAggregationBuilder("name", "invalid_agg>metric")), equalTo( + "Validation Failed: 1: " + PipelineAggregator.Parser.BUCKETS_PATH.getPreferredName() + + " aggregation does not exist for aggregation [name]: invalid_agg>metric;")); // Now try to point to a single bucket agg - ExtendedStatsBucketPipelineAggregationBuilder builder2 = new ExtendedStatsBucketPipelineAggregationBuilder("name", "global>metric"); - ex = expectThrows(IllegalArgumentException.class, () -> builder2.validate(null, aggBuilders, Collections.emptySet())); - assertEquals("The first aggregation in " + PipelineAggregator.Parser.BUCKETS_PATH.getPreferredName() + assertThat(validate(aggBuilders, new ExtendedStatsBucketPipelineAggregationBuilder("name", "global>metric")), equalTo( + "Validation Failed: 1: The first aggregation in " + PipelineAggregator.Parser.BUCKETS_PATH.getPreferredName() + " must be a multi-bucket aggregation for aggregation [name] found :" + GlobalAggregationBuilder.class.getName() - + " for buckets path: global>metric", ex.getMessage()); + + " for buckets path: global>metric;")); - // Now try to point to a valid multi-bucket agg (no exception should be - // thrown) - ExtendedStatsBucketPipelineAggregationBuilder builder3 = new ExtendedStatsBucketPipelineAggregationBuilder("name", "terms>metric"); - builder3.validate(null, aggBuilders, Collections.emptySet()); + // Now try to point to a valid multi-bucket agg + assertThat(validate(aggBuilders, new ExtendedStatsBucketPipelineAggregationBuilder("name", "terms>metric")), nullValue()); } } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/MaxBucketTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/MaxBucketTests.java index c55152c68c3a6..edbc1cda3eae2 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/MaxBucketTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/MaxBucketTests.java @@ -24,10 +24,12 @@ import org.elasticsearch.search.aggregations.bucket.terms.TermsAggregationBuilder; import org.elasticsearch.search.aggregations.support.ValueType; -import java.util.Collections; import java.util.HashSet; import java.util.Set; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.nullValue; + public class MaxBucketTests extends AbstractBucketMetricsTestCase { @Override @@ -43,23 +45,18 @@ public void testValidate() { aggBuilders.add(multiBucketAgg); // First try to point to a non-existent agg - final MaxBucketPipelineAggregationBuilder builder = new MaxBucketPipelineAggregationBuilder("name", "invalid_agg>metric"); - IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, - () -> builder.validate(null, aggBuilders, Collections.emptySet())); - assertEquals(PipelineAggregator.Parser.BUCKETS_PATH.getPreferredName() - + " aggregation does not exist for aggregation [name]: invalid_agg>metric", ex.getMessage()); + assertThat(validate(aggBuilders, new MaxBucketPipelineAggregationBuilder("name", "invalid_agg>metric")), equalTo( + "Validation Failed: 1: " + PipelineAggregator.Parser.BUCKETS_PATH.getPreferredName() + + " aggregation does not exist for aggregation [name]: invalid_agg>metric;")); // Now try to point to a single bucket agg - MaxBucketPipelineAggregationBuilder builder2 = new MaxBucketPipelineAggregationBuilder("name", "global>metric"); - ex = expectThrows(IllegalArgumentException.class, () -> builder2.validate(null, aggBuilders, Collections.emptySet())); - assertEquals("The first aggregation in " + PipelineAggregator.Parser.BUCKETS_PATH.getPreferredName() + assertThat(validate(aggBuilders, new MaxBucketPipelineAggregationBuilder("name", "global>metric")), equalTo( + "Validation Failed: 1: The first aggregation in " + PipelineAggregator.Parser.BUCKETS_PATH.getPreferredName() + " must be a multi-bucket aggregation for aggregation [name] found :" + GlobalAggregationBuilder.class.getName() - + " for buckets path: global>metric", ex.getMessage()); + + " for buckets path: global>metric;")); - // Now try to point to a valid multi-bucket agg (no exception should be - // thrown) - MaxBucketPipelineAggregationBuilder builder3 = new MaxBucketPipelineAggregationBuilder("name", "terms>metric"); - builder3.validate(null, aggBuilders, Collections.emptySet()); + // Now try to point to a valid multi-bucket agg + assertThat(validate(aggBuilders, new MaxBucketPipelineAggregationBuilder("name", "terms>metric")), nullValue()); } } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/MinBucketTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/MinBucketTests.java index 317f1360c7845..057e074a90cfc 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/MinBucketTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/MinBucketTests.java @@ -24,10 +24,12 @@ import org.elasticsearch.search.aggregations.bucket.terms.TermsAggregationBuilder; import org.elasticsearch.search.aggregations.support.ValueType; -import java.util.Collections; import java.util.HashSet; import java.util.Set; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.nullValue; + public class MinBucketTests extends AbstractBucketMetricsTestCase { @Override @@ -43,23 +45,18 @@ public void testValidate() { aggBuilders.add(multiBucketAgg); // First try to point to a non-existent agg - final MinBucketPipelineAggregationBuilder builder = new MinBucketPipelineAggregationBuilder("name", "invalid_agg>metric"); - IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, - () -> builder.validate(null, aggBuilders, Collections.emptySet())); - assertEquals(PipelineAggregator.Parser.BUCKETS_PATH.getPreferredName() - + " aggregation does not exist for aggregation [name]: invalid_agg>metric", ex.getMessage()); + assertThat(validate(aggBuilders, new MinBucketPipelineAggregationBuilder("name", "invalid_agg>metric")), equalTo( + "Validation Failed: 1: " + PipelineAggregator.Parser.BUCKETS_PATH.getPreferredName() + + " aggregation does not exist for aggregation [name]: invalid_agg>metric;")); // Now try to point to a single bucket agg - MinBucketPipelineAggregationBuilder builder2 = new MinBucketPipelineAggregationBuilder("name", "global>metric"); - ex = expectThrows(IllegalArgumentException.class, () -> builder2.validate(null, aggBuilders, Collections.emptySet())); - assertEquals("The first aggregation in " + PipelineAggregator.Parser.BUCKETS_PATH.getPreferredName() + assertThat(validate(aggBuilders, new MinBucketPipelineAggregationBuilder("name", "global>metric")), equalTo( + "Validation Failed: 1: The first aggregation in " + PipelineAggregator.Parser.BUCKETS_PATH.getPreferredName() + " must be a multi-bucket aggregation for aggregation [name] found :" + GlobalAggregationBuilder.class.getName() - + " for buckets path: global>metric", ex.getMessage()); + + " for buckets path: global>metric;")); - // Now try to point to a valid multi-bucket agg (no exception should be - // thrown) - MinBucketPipelineAggregationBuilder builder3 = new MinBucketPipelineAggregationBuilder("name", "terms>metric"); - builder3.validate(null, aggBuilders, Collections.emptySet()); + // Now try to point to a valid multi-bucket agg + assertThat(validate(aggBuilders, new MinBucketPipelineAggregationBuilder("name", "terms>metric")), nullValue()); } } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/MovFnUnitTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/MovFnAggrgatorTests.java similarity index 79% rename from server/src/test/java/org/elasticsearch/search/aggregations/pipeline/MovFnUnitTests.java rename to server/src/test/java/org/elasticsearch/search/aggregations/pipeline/MovFnAggrgatorTests.java index 9d3b10c39b1ff..e297e8a473f5f 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/MovFnUnitTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/MovFnAggrgatorTests.java @@ -42,8 +42,6 @@ import org.elasticsearch.script.ScriptService; import org.elasticsearch.script.ScriptType; import org.elasticsearch.search.aggregations.AggregatorTestCase; -import org.elasticsearch.search.aggregations.PipelineAggregationBuilder; -import org.elasticsearch.search.aggregations.TestAggregatorFactory; import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramAggregationBuilder; import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramInterval; import org.elasticsearch.search.aggregations.bucket.histogram.Histogram; @@ -53,16 +51,14 @@ import java.io.IOException; import java.util.Arrays; import java.util.Collections; -import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Set; import java.util.function.Consumer; import java.util.stream.Collectors; import static org.hamcrest.Matchers.equalTo; -public class MovFnUnitTests extends AggregatorTestCase { +public class MovFnAggrgatorTests extends AggregatorTestCase { private static final String DATE_FIELD = "date"; private static final String INSTANT_FIELD = "instant"; @@ -172,34 +168,4 @@ private void executeTestCase(Query query, private static long asLong(String dateTime) { return DateFormatters.from(DateFieldMapper.DEFAULT_DATE_TIME_FORMATTER.parse(dateTime)).toInstant().toEpochMilli(); } - - /** - * The validation should verify the parent aggregation is allowed. - */ - public void testValidate() throws IOException { - final Set aggBuilders = new HashSet<>(); - Script script = new Script(Script.DEFAULT_SCRIPT_TYPE, "painless", "test", Collections.emptyMap()); - aggBuilders.add(new MovFnPipelineAggregationBuilder("mov_fn", "avg", script, 3)); - - final MovFnPipelineAggregationBuilder builder = new MovFnPipelineAggregationBuilder("name", "invalid_agg>metric", script, 1); - builder.validate(PipelineAggregationHelperTests.getRandomSequentiallyOrderedParentAgg(), Collections.emptySet(), aggBuilders); - } - - /** - * The validation should throw an IllegalArgumentException, since parent - * aggregation is not a type of HistogramAggregatorFactory, - * DateHistogramAggregatorFactory or AutoDateHistogramAggregatorFactory. - */ - public void testValidateException() throws IOException { - final Set aggBuilders = new HashSet<>(); - Script script = new Script(Script.DEFAULT_SCRIPT_TYPE, "painless", "test", Collections.emptyMap()); - aggBuilders.add(new MovFnPipelineAggregationBuilder("mov_fn", "avg", script, 3)); - TestAggregatorFactory parentFactory = TestAggregatorFactory.createInstance(); - - final MovFnPipelineAggregationBuilder builder = new MovFnPipelineAggregationBuilder("name", "invalid_agg>metric", script, 1); - IllegalStateException ex = expectThrows(IllegalStateException.class, - () -> builder.validate(parentFactory, Collections.emptySet(), aggBuilders)); - assertEquals("moving_fn aggregation [name] must have a histogram, date_histogram or auto_date_histogram as parent", - ex.getMessage()); - } } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/MovFnPipelineAggregationBuilderSerializationTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/MovFnPipelineAggregationBuilderSerializationTests.java index cb1e2d5249b4a..17fbac3495c56 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/MovFnPipelineAggregationBuilderSerializationTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/MovFnPipelineAggregationBuilderSerializationTests.java @@ -19,17 +19,23 @@ package org.elasticsearch.search.aggregations.pipeline; -import org.elasticsearch.common.io.stream.Writeable; -import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.script.Script; -import org.elasticsearch.test.AbstractSerializingTestCase; +import org.elasticsearch.search.aggregations.AggregationBuilder; +import org.elasticsearch.search.aggregations.BasePipelineAggregationTestCase; import java.io.IOException; +import java.util.Collections; -public class MovFnPipelineAggregationBuilderSerializationTests extends AbstractSerializingTestCase { +import static java.util.Collections.emptyList; +import static java.util.Collections.emptyMap; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.nullValue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +public class MovFnPipelineAggregationBuilderSerializationTests extends BasePipelineAggregationTestCase { @Override - protected MovFnPipelineAggregationBuilder createTestInstance() { + protected MovFnPipelineAggregationBuilder createTestAggregatorFactory() { MovFnPipelineAggregationBuilder builder = new MovFnPipelineAggregationBuilder( randomAlphaOfLength(10), "foo", @@ -40,19 +46,27 @@ protected MovFnPipelineAggregationBuilder createTestInstance() { return builder; } - @Override - protected Writeable.Reader instanceReader() { - return MovFnPipelineAggregationBuilder::new; + public void testValidParent() throws IOException { + Script script = new Script(Script.DEFAULT_SCRIPT_TYPE, "painless", "test", emptyMap()); + assertThat(validate(PipelineAggregationHelperTests.getRandomSequentiallyOrderedParentAgg(), + new MovFnPipelineAggregationBuilder("mov_fn", "avg", script, 3)), nullValue()); } - @Override - protected MovFnPipelineAggregationBuilder doParseInstance(XContentParser parser) throws IOException { - return MovFnPipelineAggregationBuilder.parse(parser); + public void testInvalidParent() throws IOException { + Script script = new Script(Script.DEFAULT_SCRIPT_TYPE, "painless", "test", Collections.emptyMap()); + AggregationBuilder parent = mock(AggregationBuilder.class); + when(parent.getName()).thenReturn("name"); + + assertThat(validate(parent, new MovFnPipelineAggregationBuilder("name", "invalid_agg>metric", script, 1)), equalTo( + "Validation Failed: 1: moving_fn aggregation [name] must have a histogram, date_histogram" + + " or auto_date_histogram as parent;")); } - @Override - protected boolean supportsUnknownFields() { - return false; + public void testNoParent() throws IOException { + Script script = new Script(Script.DEFAULT_SCRIPT_TYPE, "painless", "test", Collections.emptyMap()); + assertThat(validate(emptyList(), new MovFnPipelineAggregationBuilder("name", "invalid_agg>metric", script, 1)), equalTo( + "Validation Failed: 1: moving_fn aggregation [name] must have a histogram, date_histogram" + + " or auto_date_histogram as parent but doesn't have a parent;")); } } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/PercentilesBucketTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/PercentilesBucketTests.java index 165312a5bdef5..1918d26e1e1af 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/PercentilesBucketTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/PercentilesBucketTests.java @@ -26,11 +26,11 @@ import org.elasticsearch.search.aggregations.bucket.terms.TermsAggregationBuilder; import org.elasticsearch.search.aggregations.support.ValueType; -import java.util.Collections; import java.util.HashSet; import java.util.Set; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.nullValue; public class PercentilesBucketTests extends AbstractBucketMetricsTestCase { @@ -72,23 +72,17 @@ public void testValidate() { aggBuilders.add(multiBucketAgg); // First try to point to a non-existent agg - final PercentilesBucketPipelineAggregationBuilder builder = new PercentilesBucketPipelineAggregationBuilder("name", - "invalid_agg>metric"); - IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, - () -> builder.validate(null, aggBuilders, Collections.emptySet())); - assertEquals(PipelineAggregator.Parser.BUCKETS_PATH.getPreferredName() - + " aggregation does not exist for aggregation [name]: invalid_agg>metric", ex.getMessage()); + assertThat(validate(aggBuilders, new PercentilesBucketPipelineAggregationBuilder("name", "invalid_agg>metric")), equalTo( + "Validation Failed: 1: " + PipelineAggregator.Parser.BUCKETS_PATH.getPreferredName() + + " aggregation does not exist for aggregation [name]: invalid_agg>metric;")); // Now try to point to a single bucket agg - PercentilesBucketPipelineAggregationBuilder builder2 = new PercentilesBucketPipelineAggregationBuilder("name", "global>metric"); - ex = expectThrows(IllegalArgumentException.class, () -> builder2.validate(null, aggBuilders, Collections.emptySet())); - assertEquals("The first aggregation in " + PipelineAggregator.Parser.BUCKETS_PATH.getPreferredName() + assertThat(validate(aggBuilders, new PercentilesBucketPipelineAggregationBuilder("name", "global>metric")), equalTo( + "Validation Failed: 1: The first aggregation in " + PipelineAggregator.Parser.BUCKETS_PATH.getPreferredName() + " must be a multi-bucket aggregation for aggregation [name] found :" + GlobalAggregationBuilder.class.getName() - + " for buckets path: global>metric", ex.getMessage()); + + " for buckets path: global>metric;")); - // Now try to point to a valid multi-bucket agg (no exception should be - // thrown) - PercentilesBucketPipelineAggregationBuilder builder3 = new PercentilesBucketPipelineAggregationBuilder("name", "terms>metric"); - builder3.validate(null, aggBuilders, Collections.emptySet()); + // Now try to point to a valid multi-bucket agg + assertThat(validate(aggBuilders, new PercentilesBucketPipelineAggregationBuilder("name", "terms>metric")), nullValue()); } } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/PipelineAggregationHelperTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/PipelineAggregationHelperTests.java index 9a2d5a411d4ad..2d67735468984 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/PipelineAggregationHelperTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/PipelineAggregationHelperTests.java @@ -20,29 +20,20 @@ package org.elasticsearch.search.aggregations.pipeline; -import org.elasticsearch.common.Rounding; -import org.elasticsearch.index.query.QueryShardContext; -import org.elasticsearch.search.aggregations.AggregatorFactories; -import org.elasticsearch.search.aggregations.AggregatorFactory; -import org.elasticsearch.search.aggregations.InternalOrder; +import org.elasticsearch.search.aggregations.AggregationBuilder; import org.elasticsearch.search.aggregations.bucket.histogram.AutoDateHistogramAggregationBuilder; -import org.elasticsearch.search.aggregations.bucket.histogram.AutoDateHistogramAggregatorFactory; -import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramAggregatorFactory; -import org.elasticsearch.search.aggregations.bucket.histogram.ExtendedBounds; -import org.elasticsearch.search.aggregations.bucket.histogram.HistogramAggregatorFactory; +import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramAggregationBuilder; +import org.elasticsearch.search.aggregations.bucket.histogram.HistogramAggregationBuilder; import org.elasticsearch.search.aggregations.metrics.AvgAggregationBuilder; import org.elasticsearch.search.aggregations.metrics.MaxAggregationBuilder; import org.elasticsearch.search.aggregations.metrics.MinAggregationBuilder; import org.elasticsearch.search.aggregations.metrics.SumAggregationBuilder; import org.elasticsearch.search.aggregations.support.ValuesSourceAggregationBuilder; -import org.elasticsearch.search.aggregations.support.ValuesSourceConfig; import org.elasticsearch.test.ESTestCase; import java.io.IOException; import java.util.ArrayList; -import java.util.Collections; - -import static org.mockito.Mockito.mock; +import java.util.function.Function; /** * Provides helper methods and classes for use in PipelineAggregation tests, @@ -155,28 +146,12 @@ public static double calculateMetric(double[] values, ValuesSourceAggregationBui return 0.0; } - static AggregatorFactory getRandomSequentiallyOrderedParentAgg() throws IOException { - AggregatorFactory factory = null; - switch (randomIntBetween(0, 2)) { - case 0: - factory = new HistogramAggregatorFactory("name", mock(ValuesSourceConfig.class), 0.0d, 0.0d, - mock(InternalOrder.class), false, 0L, 0.0d, 1.0d, mock(QueryShardContext.class), null, - new AggregatorFactories.Builder(), Collections.emptyMap()); - break; - case 1: - factory = new DateHistogramAggregatorFactory("name", mock(ValuesSourceConfig.class), - mock(InternalOrder.class), false, 0L, mock(Rounding.class), mock(Rounding.class), - mock(ExtendedBounds.class), mock(QueryShardContext.class), mock(AggregatorFactory.class), - new AggregatorFactories.Builder(), Collections.emptyMap()); - break; - case 2: - default: - AutoDateHistogramAggregationBuilder.RoundingInfo[] roundings = new AutoDateHistogramAggregationBuilder.RoundingInfo[1]; - factory = new AutoDateHistogramAggregatorFactory("name", mock(ValuesSourceConfig.class), - 1, roundings, - mock(QueryShardContext.class), null, new AggregatorFactories.Builder(), Collections.emptyMap()); - } - - return factory; + static AggregationBuilder getRandomSequentiallyOrderedParentAgg() throws IOException { + @SuppressWarnings("unchecked") + Function builder = randomFrom( + HistogramAggregationBuilder::new, + DateHistogramAggregationBuilder::new, + AutoDateHistogramAggregationBuilder::new); + return builder.apply("name"); } } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/SerialDifferenceTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/SerialDifferenceTests.java index ef1d13990a308..c5b09a7a5726c 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/SerialDifferenceTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/SerialDifferenceTests.java @@ -19,16 +19,21 @@ package org.elasticsearch.search.aggregations.pipeline; +import org.elasticsearch.search.aggregations.AggregationBuilder; import org.elasticsearch.search.aggregations.BasePipelineAggregationTestCase; import org.elasticsearch.search.aggregations.PipelineAggregationBuilder; -import org.elasticsearch.search.aggregations.TestAggregatorFactory; import org.elasticsearch.search.aggregations.pipeline.BucketHelpers.GapPolicy; import java.io.IOException; -import java.util.Collections; import java.util.HashSet; import java.util.Set; +import static java.util.Collections.emptyList; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.nullValue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + public class SerialDifferenceTests extends BasePipelineAggregationTestCase { @Override @@ -47,32 +52,28 @@ protected SerialDiffPipelineAggregationBuilder createTestAggregatorFactory() { } return factory; } - + /** * The validation should verify the parent aggregation is allowed. */ public void testValidate() throws IOException { - final Set aggBuilders = new HashSet<>(); - aggBuilders.add(createTestAggregatorFactory()); - - final SerialDiffPipelineAggregationBuilder builder = new SerialDiffPipelineAggregationBuilder("name", "valid"); - builder.validate(PipelineAggregationHelperTests.getRandomSequentiallyOrderedParentAgg(), Collections.emptySet(), aggBuilders); + assertThat(validate(PipelineAggregationHelperTests.getRandomSequentiallyOrderedParentAgg(), + new SerialDiffPipelineAggregationBuilder("name", "valid")), nullValue()); } - /** - * The validation should throw an IllegalArgumentException, since parent - * aggregation is not a type of HistogramAggregatorFactory, - * DateHistogramAggregatorFactory or AutoDateHistogramAggregatorFactory. - */ - public void testValidateException() throws IOException { + public void testInvalidParent() throws IOException { final Set aggBuilders = new HashSet<>(); aggBuilders.add(createTestAggregatorFactory()); - TestAggregatorFactory parentFactory = TestAggregatorFactory.createInstance(); + AggregationBuilder parent = mock(AggregationBuilder.class); + when(parent.getName()).thenReturn("name"); + assertThat(validate(parent, new SerialDiffPipelineAggregationBuilder("name", "invalid_agg>metric")), equalTo( + "Validation Failed: 1: serial_diff aggregation [name] must have a histogram, " + + "date_histogram or auto_date_histogram as parent;")); + } - final SerialDiffPipelineAggregationBuilder builder = new SerialDiffPipelineAggregationBuilder("name", "invalid_agg>metric"); - IllegalStateException ex = expectThrows(IllegalStateException.class, - () -> builder.validate(parentFactory, Collections.emptySet(), aggBuilders)); - assertEquals("serial_diff aggregation [name] must have a histogram, date_histogram or auto_date_histogram as parent", - ex.getMessage()); + public void testNoParent() { + assertThat(validate(emptyList(), new SerialDiffPipelineAggregationBuilder("name", "invalid_agg>metric")), equalTo( + "Validation Failed: 1: serial_diff aggregation [name] must have a histogram, " + + "date_histogram or auto_date_histogram as parent but doesn't have a parent;")); } } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/StatsBucketTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/StatsBucketTests.java index bf2ef7615df66..aac81c19be6bd 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/StatsBucketTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/StatsBucketTests.java @@ -24,10 +24,12 @@ import org.elasticsearch.search.aggregations.bucket.terms.TermsAggregationBuilder; import org.elasticsearch.search.aggregations.support.ValueType; -import java.util.Collections; import java.util.HashSet; import java.util.Set; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.nullValue; + public class StatsBucketTests extends AbstractBucketMetricsTestCase { @Override @@ -44,23 +46,18 @@ public void testValidate() { aggBuilders.add(multiBucketAgg); // First try to point to a non-existent agg - final StatsBucketPipelineAggregationBuilder builder = new StatsBucketPipelineAggregationBuilder("name", "invalid_agg>metric"); - IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, - () -> builder.validate(null, aggBuilders, Collections.emptySet())); - assertEquals(PipelineAggregator.Parser.BUCKETS_PATH.getPreferredName() - + " aggregation does not exist for aggregation [name]: invalid_agg>metric", ex.getMessage()); + assertThat(validate(aggBuilders, new StatsBucketPipelineAggregationBuilder("name", "invalid_agg>metric")), equalTo( + "Validation Failed: 1: " + PipelineAggregator.Parser.BUCKETS_PATH.getPreferredName() + + " aggregation does not exist for aggregation [name]: invalid_agg>metric;")); // Now try to point to a single bucket agg - StatsBucketPipelineAggregationBuilder builder2 = new StatsBucketPipelineAggregationBuilder("name", "global>metric"); - ex = expectThrows(IllegalArgumentException.class, () -> builder2.validate(null, aggBuilders, Collections.emptySet())); - assertEquals("The first aggregation in " + PipelineAggregator.Parser.BUCKETS_PATH.getPreferredName() + assertThat(validate(aggBuilders, new StatsBucketPipelineAggregationBuilder("name", "global>metric")), equalTo( + "Validation Failed: 1: The first aggregation in " + PipelineAggregator.Parser.BUCKETS_PATH.getPreferredName() + " must be a multi-bucket aggregation for aggregation [name] found :" + GlobalAggregationBuilder.class.getName() - + " for buckets path: global>metric", ex.getMessage()); + + " for buckets path: global>metric;")); - // Now try to point to a valid multi-bucket agg (no exception should be - // thrown) - StatsBucketPipelineAggregationBuilder builder3 = new StatsBucketPipelineAggregationBuilder("name", "terms>metric"); - builder3.validate(null, aggBuilders, Collections.emptySet()); + // Now try to point to a valid multi-bucket agg + assertThat(validate(aggBuilders, new StatsBucketPipelineAggregationBuilder("name", "terms>metric")), nullValue()); } } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/SumBucketTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/SumBucketTests.java index fdba878524146..0bcbf592458aa 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/SumBucketTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/SumBucketTests.java @@ -24,10 +24,12 @@ import org.elasticsearch.search.aggregations.bucket.terms.TermsAggregationBuilder; import org.elasticsearch.search.aggregations.support.ValueType; -import java.util.Collections; import java.util.HashSet; import java.util.Set; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.nullValue; + public class SumBucketTests extends AbstractBucketMetricsTestCase { @Override @@ -43,23 +45,18 @@ public void testValidate() { aggBuilders.add(multiBucketAgg); // First try to point to a non-existent agg - final SumBucketPipelineAggregationBuilder builder = new SumBucketPipelineAggregationBuilder("name", "invalid_agg>metric"); - IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, - () -> builder.validate(null, aggBuilders, Collections.emptySet())); - assertEquals(PipelineAggregator.Parser.BUCKETS_PATH.getPreferredName() - + " aggregation does not exist for aggregation [name]: invalid_agg>metric", ex.getMessage()); + assertThat(validate(aggBuilders, new SumBucketPipelineAggregationBuilder("name", "invalid_agg>metric")), equalTo( + "Validation Failed: 1: " + PipelineAggregator.Parser.BUCKETS_PATH.getPreferredName() + + " aggregation does not exist for aggregation [name]: invalid_agg>metric;")); // Now try to point to a single bucket agg - SumBucketPipelineAggregationBuilder builder2 = new SumBucketPipelineAggregationBuilder("name", "global>metric"); - ex = expectThrows(IllegalArgumentException.class, () -> builder2.validate(null, aggBuilders, Collections.emptySet())); - assertEquals("The first aggregation in " + PipelineAggregator.Parser.BUCKETS_PATH.getPreferredName() + assertThat(validate(aggBuilders, new SumBucketPipelineAggregationBuilder("name", "global>metric")), equalTo( + "Validation Failed: 1: The first aggregation in " + PipelineAggregator.Parser.BUCKETS_PATH.getPreferredName() + " must be a multi-bucket aggregation for aggregation [name] found :" + GlobalAggregationBuilder.class.getName() - + " for buckets path: global>metric", ex.getMessage()); + + " for buckets path: global>metric;")); - // Now try to point to a valid multi-bucket agg (no exception should be - // thrown) - SumBucketPipelineAggregationBuilder builder3 = new SumBucketPipelineAggregationBuilder("name", "terms>metric"); - builder3.validate(null, aggBuilders, Collections.emptySet()); + // Now try to point to a valid multi-bucket agg + assertThat(validate(aggBuilders, new SumBucketPipelineAggregationBuilder("name", "terms>metric")), nullValue()); } } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/BasePipelineAggregationTestCase.java b/test/framework/src/main/java/org/elasticsearch/search/aggregations/BasePipelineAggregationTestCase.java similarity index 85% rename from server/src/test/java/org/elasticsearch/search/aggregations/BasePipelineAggregationTestCase.java rename to test/framework/src/main/java/org/elasticsearch/search/aggregations/BasePipelineAggregationTestCase.java index 1d59ba476986d..1b533bdab83c5 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/BasePipelineAggregationTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/search/aggregations/BasePipelineAggregationTestCase.java @@ -33,13 +33,16 @@ import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.env.Environment; import org.elasticsearch.indices.IndicesModule; +import org.elasticsearch.plugins.SearchPlugin; import org.elasticsearch.search.SearchModule; +import org.elasticsearch.search.aggregations.PipelineAggregationBuilder.ValidationContext; import org.elasticsearch.search.aggregations.pipeline.AbstractPipelineAggregationBuilder; import org.elasticsearch.test.AbstractQueryTestCase; import org.elasticsearch.test.ESTestCase; import java.io.IOException; import java.util.ArrayList; +import java.util.Collection; import java.util.List; import static java.util.Collections.emptyList; @@ -75,7 +78,7 @@ public void setUp() throws Exception { .put("node.name", AbstractQueryTestCase.class.toString()) .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir()) .build(); - SearchModule searchModule = new SearchModule(settings, emptyList()); + SearchModule searchModule = new SearchModule(settings, plugins()); List entries = new ArrayList<>(); entries.addAll(IndicesModule.getNamedWriteables()); entries.addAll(searchModule.getNamedWriteables()); @@ -89,6 +92,13 @@ public void setUp() throws Exception { } } + /** + * Plugins to add to the test. + */ + protected List plugins() { + return emptyList(); + } + /** * Generic test that creates new AggregatorFactory from the test * AggregatorFactory and checks both for equality and asserts equality on @@ -196,4 +206,34 @@ public String randomNumericField() { protected NamedXContentRegistry xContentRegistry() { return xContentRegistry; } + + /** + * Helper for testing validation. + */ + protected String validate(AggregationBuilder parent, AF builder) { + return validate(ValidationContext.forInsideTree(parent, null), builder); + } + + /** + * Helper for testing validation. + */ + protected String validate(Collection siblingAggregations, AF builder) { + return validate(siblingAggregations, emptyList(), builder); + } + + /** + * Helper for testing validation. + */ + protected String validate(Collection siblingAggregations, + Collection siblingPipelineAggregations, AF builder) { + return validate(ValidationContext.forTreeRoot(siblingAggregations, siblingPipelineAggregations, null), builder); + } + + /** + * Helper for testing validation. + */ + protected String validate(ValidationContext context, AF builder) { + builder.validate(context); + return context.getValidationException() == null ? null : context.getValidationException().getMessage(); + } } diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/cumulativecardinality/CumulativeCardinalityPipelineAggregationBuilder.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/cumulativecardinality/CumulativeCardinalityPipelineAggregationBuilder.java index 844c661aadf7e..47480d98a00e2 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/cumulativecardinality/CumulativeCardinalityPipelineAggregationBuilder.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/cumulativecardinality/CumulativeCardinalityPipelineAggregationBuilder.java @@ -10,20 +10,15 @@ import org.elasticsearch.common.xcontent.ConstructingObjectParser; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.search.DocValueFormat; -import org.elasticsearch.search.aggregations.AggregationBuilder; -import org.elasticsearch.search.aggregations.AggregatorFactory; -import org.elasticsearch.search.aggregations.PipelineAggregationBuilder; import org.elasticsearch.search.aggregations.pipeline.AbstractPipelineAggregationBuilder; import org.elasticsearch.search.aggregations.pipeline.BucketMetricsParser; import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator; import java.io.IOException; -import java.util.Collection; import java.util.Map; import java.util.Objects; import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg; -import static org.elasticsearch.search.aggregations.pipeline.PipelineAggregator.Parser.BUCKETS_PATH; import static org.elasticsearch.search.aggregations.pipeline.PipelineAggregator.Parser.FORMAT; public class CumulativeCardinalityPipelineAggregationBuilder @@ -90,14 +85,12 @@ protected PipelineAggregator createInternal(Map metaData) { } @Override - public void doValidate(AggregatorFactory parent, Collection aggFactories, - Collection pipelineAggregatorFactories) { + protected void validate(ValidationContext context) { if (bucketsPaths.length != 1) { - throw new IllegalStateException(BUCKETS_PATH.getPreferredName() - + " must contain a single entry for aggregation [" + name + "]"); + context.addBucketPathValidationError("must contain a single entry for aggregation [" + name + "]"); } - validateSequentiallyOrderedParentAggs(parent, NAME, name); + context.validateParentAggSequentiallyOrdered(NAME, name); } @Override @@ -105,6 +98,7 @@ protected final XContentBuilder internalXContent(XContentBuilder builder, Params if (format != null) { builder.field(BucketMetricsParser.FORMAT.getPreferredName(), format); } + builder.field(BUCKETS_PATH_FIELD.getPreferredName(), bucketsPaths[0]); return builder; } @@ -126,4 +120,9 @@ public boolean equals(Object obj) { public String getWriteableName() { return NAME; } + + @Override + protected boolean overrideBucketsPath() { + return true; + } } diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/StubAggregatorFactory.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/StubAggregatorFactory.java deleted file mode 100644 index ace86ddea6bb8..0000000000000 --- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/StubAggregatorFactory.java +++ /dev/null @@ -1,56 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ -package org.elasticsearch.xpack.analytics; - -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.util.BigArrays; -import org.elasticsearch.common.util.MockBigArrays; -import org.elasticsearch.common.util.MockPageCacheRecycler; -import org.elasticsearch.index.query.QueryShardContext; -import org.elasticsearch.indices.breaker.NoneCircuitBreakerService; -import org.elasticsearch.search.aggregations.Aggregator; -import org.elasticsearch.search.aggregations.AggregatorFactories; -import org.elasticsearch.search.aggregations.AggregatorFactory; -import org.elasticsearch.search.internal.SearchContext; - -import java.io.IOException; -import java.util.Collections; -import java.util.List; -import java.util.Map; - -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - -/** - * Test implementation for AggregatorFactory. - */ -public class StubAggregatorFactory extends AggregatorFactory { - - private final Aggregator aggregator; - - private StubAggregatorFactory(QueryShardContext queryShardContext, Aggregator aggregator) throws IOException { - super("_name", queryShardContext, null, new AggregatorFactories.Builder(), Collections.emptyMap()); - this.aggregator = aggregator; - } - - @Override - protected Aggregator createInternal(SearchContext searchContext, - Aggregator parent, - boolean collectsFromSingleBucket, - List list, Map metaData) throws IOException { - return aggregator; - } - - public static StubAggregatorFactory createInstance() throws IOException { - BigArrays bigArrays = new MockBigArrays(new MockPageCacheRecycler(Settings.EMPTY), new NoneCircuitBreakerService()); - QueryShardContext queryShardContext = mock(QueryShardContext.class); - when(queryShardContext.bigArrays()).thenReturn(bigArrays); - - Aggregator aggregator = mock(Aggregator.class); - - return new StubAggregatorFactory(queryShardContext, aggregator); - } -} diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/cumulativecardinality/CumulativeCardinalityAggregatorTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/cumulativecardinality/CumulativeCardinalityAggregatorTests.java index e5b4b2ce0bf07..fd7adddd4234c 100644 --- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/cumulativecardinality/CumulativeCardinalityAggregatorTests.java +++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/cumulativecardinality/CumulativeCardinalityAggregatorTests.java @@ -16,46 +16,27 @@ import org.apache.lucene.search.Query; import org.apache.lucene.store.Directory; import org.elasticsearch.common.CheckedConsumer; -import org.elasticsearch.common.Rounding; import org.elasticsearch.common.time.DateFormatters; import org.elasticsearch.index.mapper.DateFieldMapper; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.mapper.NumberFieldMapper; -import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.search.aggregations.AggregationBuilder; import org.elasticsearch.search.aggregations.AggregationExecutionException; -import org.elasticsearch.search.aggregations.AggregatorFactories; -import org.elasticsearch.search.aggregations.AggregatorFactory; import org.elasticsearch.search.aggregations.AggregatorTestCase; import org.elasticsearch.search.aggregations.InternalAggregation; -import org.elasticsearch.search.aggregations.InternalOrder; -import org.elasticsearch.search.aggregations.PipelineAggregationBuilder; -import org.elasticsearch.search.aggregations.bucket.histogram.AutoDateHistogramAggregationBuilder; -import org.elasticsearch.search.aggregations.bucket.histogram.AutoDateHistogramAggregatorFactory; import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramAggregationBuilder; -import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramAggregatorFactory; import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramInterval; -import org.elasticsearch.search.aggregations.bucket.histogram.ExtendedBounds; import org.elasticsearch.search.aggregations.bucket.histogram.Histogram; -import org.elasticsearch.search.aggregations.bucket.histogram.HistogramAggregatorFactory; import org.elasticsearch.search.aggregations.metrics.CardinalityAggregationBuilder; import org.elasticsearch.search.aggregations.metrics.SumAggregationBuilder; -import org.elasticsearch.search.aggregations.support.CoreValuesSourceType; import org.elasticsearch.search.aggregations.support.ValueType; -import org.elasticsearch.search.aggregations.support.ValuesSource; -import org.elasticsearch.search.aggregations.support.ValuesSourceConfig; -import org.elasticsearch.xpack.analytics.StubAggregatorFactory; import java.io.IOException; import java.util.Arrays; -import java.util.Collections; -import java.util.HashSet; import java.util.List; -import java.util.Set; import java.util.function.Consumer; import static org.hamcrest.Matchers.equalTo; -import static org.mockito.Mockito.mock; public class CumulativeCardinalityAggregatorTests extends AggregatorTestCase { @@ -115,53 +96,6 @@ public void testAllNull() throws IOException { }); } - public void testParentValidations() throws IOException { - ValuesSourceConfig valuesSource = new ValuesSourceConfig<>(CoreValuesSourceType.NUMERIC); - - // Histogram - Set aggBuilders = new HashSet<>(); - aggBuilders.add(new CumulativeCardinalityPipelineAggregationBuilder("cumulative_card", "sum")); - AggregatorFactory parent = new HistogramAggregatorFactory("name", valuesSource, 0.0d, 0.0d, - mock(InternalOrder.class), false, 0L, 0.0d, 1.0d, mock(QueryShardContext.class), null, - new AggregatorFactories.Builder(), Collections.emptyMap()); - CumulativeCardinalityPipelineAggregationBuilder builder - = new CumulativeCardinalityPipelineAggregationBuilder("name", "valid"); - builder.validate(parent, Collections.emptySet(), aggBuilders); - - // Date Histogram - aggBuilders.clear(); - aggBuilders.add(new CumulativeCardinalityPipelineAggregationBuilder("cumulative_card", "sum")); - parent = new DateHistogramAggregatorFactory("name", valuesSource, - mock(InternalOrder.class), false, 0L, mock(Rounding.class), mock(Rounding.class), - mock(ExtendedBounds.class), mock(QueryShardContext.class), mock(AggregatorFactory.class), - new AggregatorFactories.Builder(), Collections.emptyMap()); - builder = new CumulativeCardinalityPipelineAggregationBuilder("name", "valid"); - builder.validate(parent, Collections.emptySet(), aggBuilders); - - // Auto Date Histogram - ValuesSourceConfig numericVS = new ValuesSourceConfig<>(CoreValuesSourceType.NUMERIC); - aggBuilders.clear(); - aggBuilders.add(new CumulativeCardinalityPipelineAggregationBuilder("cumulative_card", "sum")); - AutoDateHistogramAggregationBuilder.RoundingInfo[] roundings = new AutoDateHistogramAggregationBuilder.RoundingInfo[1]; - parent = new AutoDateHistogramAggregatorFactory("name", numericVS, - 1, roundings, - mock(QueryShardContext.class), null, new AggregatorFactories.Builder(), Collections.emptyMap()); - builder = new CumulativeCardinalityPipelineAggregationBuilder("name", "valid"); - builder.validate(parent, Collections.emptySet(), aggBuilders); - - // Mocked "test" agg, should fail validation - aggBuilders.clear(); - aggBuilders.add(new CumulativeCardinalityPipelineAggregationBuilder("cumulative_card", "sum")); - StubAggregatorFactory parentFactory = StubAggregatorFactory.createInstance(); - - CumulativeCardinalityPipelineAggregationBuilder failBuilder - = new CumulativeCardinalityPipelineAggregationBuilder("name", "invalid_agg>metric"); - IllegalStateException ex = expectThrows(IllegalStateException.class, - () -> failBuilder.validate(parentFactory, Collections.emptySet(), aggBuilders)); - assertEquals("cumulative_cardinality aggregation [name] must have a histogram, date_histogram or auto_date_histogram as parent", - ex.getMessage()); - } - public void testNonCardinalityAgg() { Query query = new MatchAllDocsQuery(); diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/cumulativecardinality/CumulativeCardinalityTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/cumulativecardinality/CumulativeCardinalityTests.java new file mode 100644 index 0000000000000..d9ff43e7d06c8 --- /dev/null +++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/cumulativecardinality/CumulativeCardinalityTests.java @@ -0,0 +1,73 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.analytics.cumulativecardinality; + +import org.elasticsearch.plugins.SearchPlugin; +import org.elasticsearch.search.aggregations.AggregationBuilder; +import org.elasticsearch.search.aggregations.BasePipelineAggregationTestCase; +import org.elasticsearch.search.aggregations.bucket.histogram.AutoDateHistogramAggregationBuilder; +import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramAggregationBuilder; +import org.elasticsearch.search.aggregations.bucket.histogram.HistogramAggregationBuilder; + +import java.io.IOException; +import java.util.List; + +import static java.util.Collections.emptyList; +import static java.util.Collections.singletonList; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.nullValue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class CumulativeCardinalityTests extends BasePipelineAggregationTestCase { + @Override + protected List plugins() { + return singletonList(new SearchPlugin() { + @Override + public List getPipelineAggregations() { + return singletonList(new PipelineAggregationSpec( + CumulativeCardinalityPipelineAggregationBuilder.NAME, + CumulativeCardinalityPipelineAggregationBuilder::new, + CumulativeCardinalityPipelineAggregator::new, + CumulativeCardinalityPipelineAggregationBuilder.PARSER)); + } + }); + } + + @Override + protected CumulativeCardinalityPipelineAggregationBuilder createTestAggregatorFactory() { + String name = randomAlphaOfLengthBetween(3, 20); + String bucketsPath = randomAlphaOfLengthBetween(3, 20); + CumulativeCardinalityPipelineAggregationBuilder builder = + new CumulativeCardinalityPipelineAggregationBuilder(name, bucketsPath); + if (randomBoolean()) { + builder.format(randomAlphaOfLengthBetween(1, 10)); + } + return builder; + } + + + public void testParentValidations() throws IOException { + CumulativeCardinalityPipelineAggregationBuilder builder = + new CumulativeCardinalityPipelineAggregationBuilder("name", randomAlphaOfLength(5)); + + assertThat(validate(new HistogramAggregationBuilder("name"), builder), nullValue()); + assertThat(validate(new DateHistogramAggregationBuilder("name"), builder), nullValue()); + assertThat(validate(new AutoDateHistogramAggregationBuilder("name"), builder), nullValue()); + + // Mocked "test" agg, should fail validation + AggregationBuilder stubParent = mock(AggregationBuilder.class); + when(stubParent.getName()).thenReturn("name"); + assertThat(validate(stubParent, builder), equalTo( + "Validation Failed: 1: cumulative_cardinality aggregation [name] must have a histogram, " + + "date_histogram or auto_date_histogram as parent;")); + + assertThat(validate(emptyList(), builder), equalTo( + "Validation Failed: 1: cumulative_cardinality aggregation [name] must have a histogram, " + + "date_histogram or auto_date_histogram as parent but doesn't have a parent;")); + } +}