From 2ab4ffe7d38fd548d715fbfb56d644c73ea9a244 Mon Sep 17 00:00:00 2001 From: albendz Date: Tue, 2 Oct 2018 20:34:37 -0700 Subject: [PATCH] Port aggregation parameter requirement changes from master branch --- .../scripted/ScriptedMetricAggregationBuilder.java | 12 ++++++++++++ ...iptedMetricAggregatorAggStateV6CompatTests.java | 6 ++++-- .../scripted/ScriptedMetricAggregatorTests.java | 14 ++++++++++++++ 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregationBuilder.java index 8b6d834184d73..31d978965e13f 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregationBuilder.java @@ -19,10 +19,12 @@ package org.elasticsearch.search.aggregations.metrics.scripted; +import org.apache.logging.log4j.Logger; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.query.QueryShardContext; @@ -33,6 +35,7 @@ import org.elasticsearch.search.aggregations.AggregatorFactories.Builder; import org.elasticsearch.search.aggregations.AggregatorFactory; import org.elasticsearch.search.internal.SearchContext; +import org.elasticsearch.common.logging.DeprecationLogger; import java.io.IOException; import java.util.Collections; @@ -41,6 +44,8 @@ public class ScriptedMetricAggregationBuilder extends AbstractAggregationBuilder { public static final String NAME = "scripted_metric"; + private static final Logger logger = Loggers.getLogger(ScriptedMetricAggregationBuilder.class); + private static final DeprecationLogger deprecationLogger = new DeprecationLogger(logger); private static final ParseField INIT_SCRIPT_FIELD = new ParseField("init_script"); private static final ParseField MAP_SCRIPT_FIELD = new ParseField("map_script"); @@ -196,6 +201,13 @@ public Map params() { protected ScriptedMetricAggregatorFactory doBuild(SearchContext context, AggregatorFactory parent, Builder subfactoriesBuilder) throws IOException { + if (combineScript == null) { + deprecationLogger.deprecated("[combineScript] must be provided for metric aggregations."); + } + if(reduceScript == null) { + deprecationLogger.deprecated("[reduceScript] must be provided for metric aggregations."); + } + QueryShardContext queryShardContext = context.getQueryShardContext(); // Extract params from scripts and pass them along to ScriptedMetricAggregatorFactory, since it won't have diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregatorAggStateV6CompatTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregatorAggStateV6CompatTests.java index 15e5748cdc644..3e0413b3da562 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregatorAggStateV6CompatTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregatorAggStateV6CompatTests.java @@ -132,7 +132,8 @@ public void testWithImplicitAggParam() throws IOException { } } - assertWarnings(ScriptedMetricAggContexts.AGG_PARAM_DEPRECATION_WARNING); + assertWarnings(ScriptedMetricAggContexts.AGG_PARAM_DEPRECATION_WARNING, + "[reduceScript] must be provided for metric aggregations."); } /** @@ -161,7 +162,8 @@ public void testWithExplicitAggParam() throws IOException { } } - assertWarnings(ScriptedMetricAggContexts.AGG_PARAM_DEPRECATION_WARNING); + assertWarnings(ScriptedMetricAggContexts.AGG_PARAM_DEPRECATION_WARNING, + "[reduceScript] must be provided for metric aggregations."); } /** diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregatorTests.java index 43d10247e4853..034d6a129db14 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregatorTests.java @@ -139,6 +139,9 @@ public void testNoDocs() throws IOException { assertEquals(AGG_NAME, scriptedMetric.getName()); assertNotNull(scriptedMetric.aggregation()); assertEquals(0, ((HashMap) scriptedMetric.aggregation()).size()); + } finally { + assertWarnings("[combineScript] must be provided for metric aggregations.", + "[reduceScript] must be provided for metric aggregations."); } } } @@ -163,6 +166,9 @@ public void testScriptedMetricWithoutCombine() throws IOException { assertNotNull(scriptedMetric.aggregation()); Map agg = (Map) scriptedMetric.aggregation(); assertEquals(numDocs, ((List) agg.get("collector")).size()); + } finally { + assertWarnings("[combineScript] must be provided for metric aggregations.", + "[reduceScript] must be provided for metric aggregations."); } } } @@ -185,6 +191,8 @@ public void testScriptedMetricWithCombine() throws IOException { assertEquals(AGG_NAME, scriptedMetric.getName()); assertNotNull(scriptedMetric.aggregation()); assertEquals(numDocs, scriptedMetric.aggregation()); + } finally { + assertWarnings("[reduceScript] must be provided for metric aggregations."); } } } @@ -208,6 +216,8 @@ public void testScriptedMetricWithCombineAccessesScores() throws IOException { assertNotNull(scriptedMetric.aggregation()); // all documents have score of 1.0 assertEquals((double) numDocs, scriptedMetric.aggregation()); + } finally { + assertWarnings("[reduceScript] must be provided for metric aggregations."); } } } @@ -227,6 +237,8 @@ public void testScriptParamsPassedThrough() throws IOException { // The result value depends on the script params. assertEquals(306, scriptedMetric.aggregation()); + } finally { + assertWarnings("[reduceScript] must be provided for metric aggregations."); } } } @@ -250,6 +262,8 @@ public void testConflictingAggAndScriptParams() throws IOException { ); assertEquals("Parameter name \"" + CONFLICTING_PARAM_NAME + "\" used in both aggregation and script parameters", ex.getMessage()); + } finally { + assertWarnings("[reduceScript] must be provided for metric aggregations."); } } }