diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/movavg/MovAvgBuilder.java b/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/movavg/MovAvgBuilder.java index 10adf5125a4..b3aca612fc0 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/movavg/MovAvgBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/movavg/MovAvgBuilder.java @@ -25,6 +25,7 @@ import org.elasticsearch.search.aggregations.pipeline.PipelineAggregatorBuilder; import org.elasticsearch.search.aggregations.pipeline.movavg.models.MovAvgModelBuilder; import java.io.IOException; +import java.util.Map; /** * A builder to create MovingAvg pipeline aggregations @@ -37,6 +38,7 @@ public class MovAvgBuilder extends PipelineAggregatorBuilder { private Integer window; private Integer predict; private Boolean minimize; + private Map settings; public MovAvgBuilder(String name) { super(name, MovAvgPipelineAggregator.TYPE.name()); @@ -107,6 +109,18 @@ public class MovAvgBuilder extends PipelineAggregatorBuilder { return this; } + /** + * The hash of settings that should be provided to the model when it is + * instantiated + * + * @param settings + * @return + */ + public MovAvgBuilder settings(Map settings) { + this.settings = settings; + return this; + } + @Override protected XContentBuilder internalXContent(XContentBuilder builder, Params params) throws IOException { @@ -128,6 +142,9 @@ public class MovAvgBuilder extends PipelineAggregatorBuilder { if (minimize != null) { builder.field(MovAvgParser.MINIMIZE.getPreferredName(), minimize); } + if (settings != null) { + builder.field(MovAvgParser.SETTINGS.getPreferredName(), settings); + } return builder; } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/movavg/models/EwmaModel.java b/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/movavg/models/EwmaModel.java index 728cb1697ed..84de794ceed 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/movavg/models/EwmaModel.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/movavg/models/EwmaModel.java @@ -120,9 +120,11 @@ public class EwmaModel extends MovAvgModel { } @Override - public MovAvgModel parse(@Nullable Map settings, String pipelineName, int windowSize, ParseFieldMatcher parseFieldMatcher) throws ParseException { + public MovAvgModel parse(@Nullable Map settings, String pipelineName, int windowSize, + ParseFieldMatcher parseFieldMatcher) throws ParseException { double alpha = parseDoubleParam(settings, "alpha", 0.3); + checkUnrecognizedParams(settings); return new EwmaModel(alpha); } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/movavg/models/HoltLinearModel.java b/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/movavg/models/HoltLinearModel.java index 6a3b9500e94..55611a0d000 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/movavg/models/HoltLinearModel.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/movavg/models/HoltLinearModel.java @@ -180,10 +180,12 @@ public class HoltLinearModel extends MovAvgModel { } @Override - public MovAvgModel parse(@Nullable Map settings, String pipelineName, int windowSize, ParseFieldMatcher parseFieldMatcher) throws ParseException { + public MovAvgModel parse(@Nullable Map settings, String pipelineName, int windowSize, + ParseFieldMatcher parseFieldMatcher) throws ParseException { double alpha = parseDoubleParam(settings, "alpha", 0.3); double beta = parseDoubleParam(settings, "beta", 0.1); + checkUnrecognizedParams(settings); return new HoltLinearModel(alpha, beta); } } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/movavg/models/HoltWintersModel.java b/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/movavg/models/HoltWintersModel.java index b02082bec5c..8be63e165b8 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/movavg/models/HoltWintersModel.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/movavg/models/HoltWintersModel.java @@ -356,7 +356,8 @@ public class HoltWintersModel extends MovAvgModel { } @Override - public MovAvgModel parse(@Nullable Map settings, String pipelineName, int windowSize, ParseFieldMatcher parseFieldMatcher) throws ParseException { + public MovAvgModel parse(@Nullable Map settings, String pipelineName, int windowSize, + ParseFieldMatcher parseFieldMatcher) throws ParseException { double alpha = parseDoubleParam(settings, "alpha", 0.3); double beta = parseDoubleParam(settings, "beta", 0.1); @@ -376,6 +377,7 @@ public class HoltWintersModel extends MovAvgModel { if (value != null) { if (value instanceof String) { seasonalityType = SeasonalityType.parse((String)value, parseFieldMatcher); + settings.remove("type"); } else { throw new ParseException("Parameter [type] must be a String, type `" + value.getClass().getSimpleName() + "` provided instead", 0); @@ -385,6 +387,7 @@ public class HoltWintersModel extends MovAvgModel { boolean pad = parseBoolParam(settings, "pad", seasonalityType.equals(SeasonalityType.MULTIPLICATIVE)); + checkUnrecognizedParams(settings); return new HoltWintersModel(alpha, beta, gamma, period, seasonalityType, pad); } } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/movavg/models/LinearModel.java b/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/movavg/models/LinearModel.java index f6c5a2be407..264a42509b7 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/movavg/models/LinearModel.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/movavg/models/LinearModel.java @@ -107,7 +107,9 @@ public class LinearModel extends MovAvgModel { } @Override - public MovAvgModel parse(@Nullable Map settings, String pipelineName, int windowSize, ParseFieldMatcher parseFieldMatcher) throws ParseException { + public MovAvgModel parse(@Nullable Map settings, String pipelineName, int windowSize, + ParseFieldMatcher parseFieldMatcher) throws ParseException { + checkUnrecognizedParams(settings); return new LinearModel(); } } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/movavg/models/MovAvgModel.java b/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/movavg/models/MovAvgModel.java index 1a085b37620..aba504e29dd 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/movavg/models/MovAvgModel.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/movavg/models/MovAvgModel.java @@ -155,7 +155,8 @@ public abstract class MovAvgModel { * @param parseFieldMatcher Matcher for field names * @return A fully built moving average model */ - public abstract MovAvgModel parse(@Nullable Map settings, String pipelineName, int windowSize, ParseFieldMatcher parseFieldMatcher) throws ParseException; + public abstract MovAvgModel parse(@Nullable Map settings, String pipelineName, + int windowSize, ParseFieldMatcher parseFieldMatcher) throws ParseException; /** @@ -180,6 +181,7 @@ public abstract class MovAvgModel { } else if (value instanceof Number) { double v = ((Number) value).doubleValue(); if (v >= 0 && v <= 1) { + settings.remove(name); return v; } @@ -211,6 +213,7 @@ public abstract class MovAvgModel { if (value == null) { return defaultValue; } else if (value instanceof Number) { + settings.remove(name); return ((Number) value).intValue(); } @@ -238,12 +241,19 @@ public abstract class MovAvgModel { if (value == null) { return defaultValue; } else if (value instanceof Boolean) { + settings.remove(name); return (Boolean)value; } throw new ParseException("Parameter [" + name + "] must be a boolean, type `" + value.getClass().getSimpleName() + "` provided instead", 0); } + + protected void checkUnrecognizedParams(@Nullable Map settings) throws ParseException { + if (settings != null && settings.size() > 0) { + throw new ParseException("Unrecognized parameter(s): [" + String.join(", ", settings.keySet()) + "]", 0); + } + } } } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/movavg/models/SimpleModel.java b/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/movavg/models/SimpleModel.java index 8d375561dd3..e0c7781ec4a 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/movavg/models/SimpleModel.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/movavg/models/SimpleModel.java @@ -60,7 +60,7 @@ public class SimpleModel extends MovAvgModel { protected double[] doPredict(Collection values, int numPredictions) { double[] predictions = new double[numPredictions]; - // EWMA just emits the same final prediction repeatedly. + // Simple just emits the same final prediction repeatedly. Arrays.fill(predictions, next(values)); return predictions; @@ -100,7 +100,9 @@ public class SimpleModel extends MovAvgModel { } @Override - public MovAvgModel parse(@Nullable Map settings, String pipelineName, int windowSize, ParseFieldMatcher parseFieldMatcher) throws ParseException { + public MovAvgModel parse(@Nullable Map settings, String pipelineName, int windowSize, + ParseFieldMatcher parseFieldMatcher) throws ParseException { + checkUnrecognizedParams(settings); return new SimpleModel(); } } diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/pipeline/moving/avg/MovAvgTests.java b/core/src/test/java/org/elasticsearch/search/aggregations/pipeline/moving/avg/MovAvgTests.java index 81a065455ab..1aa63898033 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/pipeline/moving/avg/MovAvgTests.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/pipeline/moving/avg/MovAvgTests.java @@ -25,7 +25,6 @@ import com.google.common.collect.EvictingQueue; import org.elasticsearch.action.index.IndexRequestBuilder; import org.elasticsearch.action.search.SearchPhaseExecutionException; import org.elasticsearch.action.search.SearchResponse; -import org.elasticsearch.search.SearchParseException; import org.elasticsearch.search.aggregations.bucket.histogram.Histogram; import org.elasticsearch.search.aggregations.bucket.histogram.InternalHistogram; import org.elasticsearch.search.aggregations.bucket.histogram.InternalHistogram.Bucket; @@ -1265,6 +1264,42 @@ public class MovAvgTests extends ElasticsearchIntegrationTest { fail("Model [" + builder.toString() + "] can be minimized, but an exception was thrown"); } } + } + + @Test + public void testUnrecognizedParams() { + + MovAvgModelBuilder[] builders = new MovAvgModelBuilder[]{ + new SimpleModel.SimpleModelBuilder(), + new LinearModel.LinearModelBuilder(), + new EwmaModel.EWMAModelBuilder(), + new HoltLinearModel.HoltLinearModelBuilder(), + new HoltWintersModel.HoltWintersModelBuilder() + }; + Map badSettings = new HashMap<>(1); + badSettings.put("abc", 1.2); + + for (MovAvgModelBuilder builder : builders) { + try { + SearchResponse response = client() + .prepareSearch("idx").setTypes("type") + .addAggregation( + histogram("histo").field(INTERVAL_FIELD).interval(interval) + .extendedBounds(0L, (long) (interval * (numBuckets - 1))) + .subAggregation(metric) + .subAggregation(movingAvg("movavg_counts") + .window(10) + .modelBuilder(builder) + .gapPolicy(gapPolicy) + .settings(badSettings) + .setBucketsPaths("_count")) + ).execute().actionGet(); + } catch (SearchPhaseExecutionException e) { + // All good + } + } + + } diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/pipeline/moving/avg/MovAvgUnitTests.java b/core/src/test/java/org/elasticsearch/search/aggregations/pipeline/moving/avg/MovAvgUnitTests.java index 575d263fa9f..fb9e5fa09aa 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/pipeline/moving/avg/MovAvgUnitTests.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/pipeline/moving/avg/MovAvgUnitTests.java @@ -611,8 +611,6 @@ public class MovAvgUnitTests extends ElasticsearchTestCase { for (MovAvgModel.AbstractModelParser parser : parsers) { for (Object v : values) { settings.put("alpha", v); - settings.put("beta", v); - settings.put("gamma", v); try { parser.parse(settings, "pipeline", 10, ParseFieldMatcher.STRICT);