Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Port aggregation parameter requirement changes from master branch #34254

Merged
merged 1 commit into from
Oct 3, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -41,6 +44,8 @@

public class ScriptedMetricAggregationBuilder extends AbstractAggregationBuilder<ScriptedMetricAggregationBuilder> {
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");
Expand Down Expand Up @@ -196,6 +201,13 @@ public Map<String, Object> 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
}

/**
Expand Down Expand Up @@ -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.");
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,9 @@ public void testNoDocs() throws IOException {
assertEquals(AGG_NAME, scriptedMetric.getName());
assertNotNull(scriptedMetric.aggregation());
assertEquals(0, ((HashMap<Object, String>) scriptedMetric.aggregation()).size());
} finally {
assertWarnings("[combineScript] must be provided for metric aggregations.",
"[reduceScript] must be provided for metric aggregations.");
}
}
}
Expand All @@ -163,6 +166,9 @@ public void testScriptedMetricWithoutCombine() throws IOException {
assertNotNull(scriptedMetric.aggregation());
Map<String, Object> agg = (Map<String, Object>) scriptedMetric.aggregation();
assertEquals(numDocs, ((List<Integer>) agg.get("collector")).size());
} finally {
assertWarnings("[combineScript] must be provided for metric aggregations.",
"[reduceScript] must be provided for metric aggregations.");
}
}
}
Expand All @@ -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.");
}
}
}
Expand All @@ -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.");
}
}
}
Expand All @@ -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.");
}
}
}
Expand All @@ -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.");
}
}
}
Expand Down