diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/BucketMetricsParser.java b/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/BucketMetricsParser.java index 80b4c981d12..f994d9314ae 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/BucketMetricsParser.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/BucketMetricsParser.java @@ -31,8 +31,11 @@ import org.elasticsearch.search.aggregations.support.format.ValueFormatter; import org.elasticsearch.search.internal.SearchContext; import java.io.IOException; +import java.text.ParseException; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; /** * A parser for parsing requests for a {@link BucketMetricsPipelineAggregator} @@ -52,12 +55,11 @@ public abstract class BucketMetricsParser implements PipelineAggregator.Parser { String[] bucketsPaths = null; String format = null; GapPolicy gapPolicy = GapPolicy.SKIP; + Map leftover = new HashMap<>(5); while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { if (token == XContentParser.Token.FIELD_NAME) { currentFieldName = parser.currentName(); - } else if (doParse(pipelineAggregatorName, currentFieldName, token, parser, context)) { - // Do nothing as subclass has stored the state for this token } else if (token == XContentParser.Token.VALUE_STRING) { if (context.parseFieldMatcher().match(currentFieldName, FORMAT)) { format = parser.text(); @@ -66,8 +68,7 @@ public abstract class BucketMetricsParser implements PipelineAggregator.Parser { } else if (context.parseFieldMatcher().match(currentFieldName, GAP_POLICY)) { gapPolicy = GapPolicy.parse(context, parser.text(), parser.getTokenLocation()); } else { - throw new SearchParseException(context, "Unknown key for a " + token + " in [" + pipelineAggregatorName + "]: [" - + currentFieldName + "].", parser.getTokenLocation()); + leftover.put(currentFieldName, parser.text()); } } else if (token == XContentParser.Token.START_ARRAY) { if (context.parseFieldMatcher().match(currentFieldName, BUCKETS_PATH)) { @@ -78,18 +79,16 @@ public abstract class BucketMetricsParser implements PipelineAggregator.Parser { } bucketsPaths = paths.toArray(new String[paths.size()]); } else { - throw new SearchParseException(context, "Unknown key for a " + token + " in [" + pipelineAggregatorName + "]: [" - + currentFieldName + "].", parser.getTokenLocation()); + leftover.put(currentFieldName, parser.list()); } } else { - throw new SearchParseException(context, "Unexpected token " + token + " in [" + pipelineAggregatorName + "].", - parser.getTokenLocation()); + leftover.put(currentFieldName, parser.objectText()); } } if (bucketsPaths == null) { throw new SearchParseException(context, "Missing required field [" + BUCKETS_PATH.getPreferredName() - + "] for derivative aggregation [" + pipelineAggregatorName + "]", parser.getTokenLocation()); + + "] for aggregation [" + pipelineAggregatorName + "]", parser.getTokenLocation()); } ValueFormatter formatter = null; @@ -99,15 +98,23 @@ public abstract class BucketMetricsParser implements PipelineAggregator.Parser { formatter = ValueFormatter.RAW; } - return buildFactory(pipelineAggregatorName, bucketsPaths, gapPolicy, formatter); + PipelineAggregatorFactory factory = null; + try { + factory = buildFactory(pipelineAggregatorName, bucketsPaths, gapPolicy, formatter, leftover); + } catch (ParseException exception) { + throw new SearchParseException(context, "Could not parse settings for aggregation [" + + pipelineAggregatorName + "].", null, exception); + } + + if (leftover.size() > 0) { + throw new SearchParseException(context, "Unexpected tokens " + leftover.keySet() + " in [" + pipelineAggregatorName + "].", null); + } + assert(factory != null); + + return factory; } protected abstract PipelineAggregatorFactory buildFactory(String pipelineAggregatorName, String[] bucketsPaths, GapPolicy gapPolicy, - ValueFormatter formatter); - - protected boolean doParse(String pipelineAggregatorName, String currentFieldName, Token token, - XContentParser parser, SearchContext context) throws IOException { - return false; - } + ValueFormatter formatter, Map unparsedParams) throws ParseException; } \ No newline at end of file diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/avg/AvgBucketParser.java b/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/avg/AvgBucketParser.java index 01811cb1746..658284f1825 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/avg/AvgBucketParser.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/avg/AvgBucketParser.java @@ -24,6 +24,8 @@ import org.elasticsearch.search.aggregations.pipeline.PipelineAggregatorFactory; import org.elasticsearch.search.aggregations.pipeline.bucketmetrics.BucketMetricsParser; import org.elasticsearch.search.aggregations.support.format.ValueFormatter; +import java.util.Map; + public class AvgBucketParser extends BucketMetricsParser { @Override public String type() { @@ -32,7 +34,7 @@ public class AvgBucketParser extends BucketMetricsParser { @Override protected PipelineAggregatorFactory buildFactory(String pipelineAggregatorName, String[] bucketsPaths, GapPolicy gapPolicy, - ValueFormatter formatter) { + ValueFormatter formatter, Map unparsedParams) { return new AvgBucketPipelineAggregator.Factory(pipelineAggregatorName, bucketsPaths, gapPolicy, formatter); } } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/max/MaxBucketParser.java b/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/max/MaxBucketParser.java index 864734b956f..683db6c7d68 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/max/MaxBucketParser.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/max/MaxBucketParser.java @@ -24,6 +24,8 @@ import org.elasticsearch.search.aggregations.pipeline.BucketHelpers.GapPolicy; import org.elasticsearch.search.aggregations.pipeline.bucketmetrics.BucketMetricsParser; import org.elasticsearch.search.aggregations.support.format.ValueFormatter; +import java.util.Map; + public class MaxBucketParser extends BucketMetricsParser { @Override @@ -32,7 +34,8 @@ public class MaxBucketParser extends BucketMetricsParser { } @Override - protected PipelineAggregatorFactory buildFactory(String pipelineAggregatorName, String[] bucketsPaths, GapPolicy gapPolicy, ValueFormatter formatter) { + protected PipelineAggregatorFactory buildFactory(String pipelineAggregatorName, String[] bucketsPaths, GapPolicy gapPolicy, + ValueFormatter formatter, Map unparsedParams) { return new MaxBucketPipelineAggregator.Factory(pipelineAggregatorName, bucketsPaths, gapPolicy, formatter); } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/min/MinBucketParser.java b/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/min/MinBucketParser.java index 0a5aa7c1123..db7bc9b0ced 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/min/MinBucketParser.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/min/MinBucketParser.java @@ -24,6 +24,8 @@ import org.elasticsearch.search.aggregations.pipeline.PipelineAggregatorFactory; import org.elasticsearch.search.aggregations.pipeline.bucketmetrics.BucketMetricsParser; import org.elasticsearch.search.aggregations.support.format.ValueFormatter; +import java.util.Map; + public class MinBucketParser extends BucketMetricsParser { @Override @@ -32,7 +34,7 @@ public class MinBucketParser extends BucketMetricsParser { } protected PipelineAggregatorFactory buildFactory(String pipelineAggregatorName, String[] bucketsPaths, GapPolicy gapPolicy, - ValueFormatter formatter) { + ValueFormatter formatter, Map unparsedParams) { return new MinBucketPipelineAggregator.Factory(pipelineAggregatorName, bucketsPaths, gapPolicy, formatter); }; diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/percentile/PercentilesBucketParser.java b/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/percentile/PercentilesBucketParser.java index 01a428873c2..7c9da5cbe70 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/percentile/PercentilesBucketParser.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/percentile/PercentilesBucketParser.java @@ -19,17 +19,14 @@ package org.elasticsearch.search.aggregations.pipeline.bucketmetrics.percentile; -import com.google.common.primitives.Doubles; import org.elasticsearch.common.ParseField; -import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.search.aggregations.pipeline.PipelineAggregatorFactory; import org.elasticsearch.search.aggregations.pipeline.bucketmetrics.BucketMetricsParser; import org.elasticsearch.search.aggregations.support.format.ValueFormatter; -import org.elasticsearch.search.internal.SearchContext; -import java.io.IOException; -import java.util.ArrayList; +import java.text.ParseException; import java.util.List; +import java.util.Map; import static org.elasticsearch.search.aggregations.pipeline.BucketHelpers.GapPolicy; @@ -37,7 +34,6 @@ import static org.elasticsearch.search.aggregations.pipeline.BucketHelpers.GapPo public class PercentilesBucketParser extends BucketMetricsParser { public static final ParseField PERCENTS = new ParseField("percents"); - double[] percents = new double[] { 1.0, 5.0, 25.0, 50.0, 75.0, 95.0, 99.0 }; @Override public String type() { @@ -46,22 +42,31 @@ public class PercentilesBucketParser extends BucketMetricsParser { @Override protected PipelineAggregatorFactory buildFactory(String pipelineAggregatorName, String[] bucketsPaths, GapPolicy gapPolicy, - ValueFormatter formatter) { + ValueFormatter formatter, Map unparsedParams) throws ParseException { + + double[] percents = new double[] { 1.0, 5.0, 25.0, 50.0, 75.0, 95.0, 99.0 }; + int counter = 0; + Object percentParam = unparsedParams.get(PERCENTS.getPreferredName()); + + if (percentParam != null) { + if (percentParam instanceof List) { + percents = new double[((List) percentParam).size()]; + for (Object p : (List) percentParam) { + if (p instanceof Double) { + percents[counter] = (Double) p; + counter += 1; + } else { + throw new ParseException("Parameter [" + PERCENTS.getPreferredName() + "] must be an array of doubles, type `" + + percentParam.getClass().getSimpleName() + "` provided instead", 0); + } + } + unparsedParams.remove(PERCENTS.getPreferredName()); + } else { + throw new ParseException("Parameter [" + PERCENTS.getPreferredName() + "] must be an array of doubles, type `" + + percentParam.getClass().getSimpleName() + "` provided instead", 0); + } + } + return new PercentilesBucketPipelineAggregator.Factory(pipelineAggregatorName, bucketsPaths, gapPolicy, formatter, percents); } - - @Override - protected boolean doParse(String pipelineAggregatorName, String currentFieldName, - XContentParser.Token token, XContentParser parser, SearchContext context) throws IOException { - if (context.parseFieldMatcher().match(currentFieldName, PERCENTS)) { - - List parsedPercents = new ArrayList<>(); - while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) { - parsedPercents.add(parser.doubleValue()); - } - percents = Doubles.toArray(parsedPercents); - return true; - } - return false; - } } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/sum/SumBucketParser.java b/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/sum/SumBucketParser.java index c71cdcf18c6..3fad95d6e51 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/sum/SumBucketParser.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/sum/SumBucketParser.java @@ -24,6 +24,8 @@ import org.elasticsearch.search.aggregations.pipeline.PipelineAggregatorFactory; import org.elasticsearch.search.aggregations.pipeline.bucketmetrics.BucketMetricsParser; import org.elasticsearch.search.aggregations.support.format.ValueFormatter; +import java.util.Map; + public class SumBucketParser extends BucketMetricsParser { @Override public String type() { @@ -32,7 +34,7 @@ public class SumBucketParser extends BucketMetricsParser { @Override protected PipelineAggregatorFactory buildFactory(String pipelineAggregatorName, String[] bucketsPaths, GapPolicy gapPolicy, - ValueFormatter formatter) { + ValueFormatter formatter, Map unparsedParams) { return new SumBucketPipelineAggregator.Factory(pipelineAggregatorName, bucketsPaths, gapPolicy, formatter); } } diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/pipeline/PercentilesBucketIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/pipeline/PercentilesBucketIT.java index bc50f88e540..507939e6858 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/pipeline/PercentilesBucketIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/pipeline/PercentilesBucketIT.java @@ -489,7 +489,6 @@ public class PercentilesBucketIT extends ESIntegTestCase { } @Test - @AwaitsFix(bugUrl="https://github.com/elastic/elasticsearch/issues/13337") public void testNested() throws Exception { SearchResponse response = client() .prepareSearch("idx")