Remove support for deprecated params._agg/_aggs for scripted metric aggregations (#32979)

Author: rationull

buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy

@@ -802,8 +802,6 @@ class BuildPlugin implements Plugin<Project> {
             systemProperty 'tests.task', path
             systemProperty 'tests.security.manager', 'true'
             systemProperty 'jna.nosys', 'true'
-            // TODO: remove this deprecation compatibility setting for 7.0
-            systemProperty 'es.aggregations.enable_scripted_metric_agg_param', 'false'
             systemProperty 'compiler.java', project.ext.compilerJavaVersion.getMajorVersion()
             if (project.ext.inFipsJvm) {
                 systemProperty 'runtime.java', project.ext.runtimeJavaVersion.getMajorVersion() + "FIPS"

docs/build.gradle

@@ -41,9 +41,6 @@ integTestCluster {
   // TODO: remove this for 7.0, this exists to allow the doc examples in 6.x to continue using the defaults
   systemProperty 'es.scripting.use_java_time', 'false'
   systemProperty 'es.scripting.update.ctx_in_params', 'false'
-
-  // TODO: remove this deprecation compatibility setting for 7.0
-  systemProperty 'es.aggregations.enable_scripted_metric_agg_param', 'false'
 }
 
 // remove when https://github.com/elastic/elasticsearch/issues/31305 is fixed

docs/reference/migration/migrate_7_0/aggregations.asciidoc

@@ -21,5 +21,3 @@ has been removed. `missing_bucket` should be used instead.
 The object used to share aggregation state between the scripts in a Scripted Metric
 Aggregation is now a variable called `state` available in the script context, rather than
 being provided via the `params` object as `params._agg`.
-
-The old `params._agg` variable is still available as well.

server/build.gradle

@@ -346,16 +346,3 @@ if (isEclipse == false || project.path == ":server-tests") {
   check.dependsOn integTest
   integTest.mustRunAfter test
 }
-
-// TODO: remove these compatibility tests in 7.0
-additionalTest('testScriptedMetricAggParamsV6Compatibility') {
-  include '**/ScriptedMetricAggregatorAggStateV6CompatTests.class'
-  include '**/InternalScriptedMetricAggStateV6CompatTests.class'
-  systemProperty 'es.aggregations.enable_scripted_metric_agg_param', 'true'
-}
-
-test {
-  // these are tested explicitly in separate test tasks
-  exclude '**/ScriptedMetricAggregatorAggStateV6CompatTests.class'
-  exclude '**/InternalScriptedMetricAggStateV6CompatTests.class'
-}

server/src/main/java/org/elasticsearch/script/ScriptedMetricAggContexts.java

@@ -22,8 +22,6 @@
 import org.apache.lucene.index.LeafReaderContext;
 import org.apache.lucene.search.Scorer;
 import org.elasticsearch.ElasticsearchException;
-import org.elasticsearch.common.logging.DeprecationLogger;
-import org.elasticsearch.common.logging.Loggers;
 import org.elasticsearch.index.fielddata.ScriptDocValues;
 import org.elasticsearch.search.lookup.LeafSearchLookup;
 import org.elasticsearch.search.lookup.SearchLookup;
@@ -33,30 +31,11 @@
 import java.util.Map;
 
 public class ScriptedMetricAggContexts {
-    private static final DeprecationLogger DEPRECATION_LOGGER =
-        new DeprecationLogger(Loggers.getLogger(ScriptedMetricAggContexts.class));
-
-    // Public for access from tests
-    public static final String AGG_PARAM_DEPRECATION_WARNING =
-        "params._agg/_aggs for scripted metric aggregations are deprecated, use state/states (not in params) instead. " +
-        "Use -Des.aggregations.enable_scripted_metric_agg_param=false to disable.";
-
-    public static boolean deprecatedAggParamEnabled() {
-        boolean enabled = Boolean.parseBoolean(
-            System.getProperty("es.aggregations.enable_scripted_metric_agg_param", "true"));
-
-        if (enabled) {
-            DEPRECATION_LOGGER.deprecatedAndMaybeLog("enable_scripted_metric_agg_param", AGG_PARAM_DEPRECATION_WARNING);
-        }
-
-        return enabled;
-    }
-
     private abstract static class ParamsAndStateBase {
         private final Map<String, Object> params;
-        private final Object state;
+        private final Map<String, Object> state;
 
-        ParamsAndStateBase(Map<String, Object> params, Object state) {
+        ParamsAndStateBase(Map<String, Object> params, Map<String, Object> state) {
             this.params = params;
             this.state = state;
         }
@@ -71,14 +50,14 @@ public Object getState() {
     }
 
     public abstract static class InitScript extends ParamsAndStateBase {
-        public InitScript(Map<String, Object> params, Object state) {
+        public InitScript(Map<String, Object> params, Map<String, Object> state) {
             super(params, state);
         }
 
         public abstract void execute();
 
         public interface Factory {
-            InitScript newInstance(Map<String, Object> params, Object state);
+            InitScript newInstance(Map<String, Object> params, Map<String, Object> state);
         }
 
         public static String[] PARAMETERS = {};
@@ -89,7 +68,7 @@ public abstract static class MapScript extends ParamsAndStateBase {
         private final LeafSearchLookup leafLookup;
         private Scorer scorer;
 
-        public MapScript(Map<String, Object> params, Object state, SearchLookup lookup, LeafReaderContext leafContext) {
+        public MapScript(Map<String, Object> params, Map<String, Object> state, SearchLookup lookup, LeafReaderContext leafContext) {
             super(params, state);
 
             this.leafLookup = leafContext == null ? null : lookup.getLeafSearchLookup(leafContext);
@@ -131,22 +110,22 @@ public interface LeafFactory {
         }
 
         public interface Factory {
-            LeafFactory newFactory(Map<String, Object> params, Object state, SearchLookup lookup);
+            LeafFactory newFactory(Map<String, Object> params, Map<String, Object> state, SearchLookup lookup);
         }
 
         public static String[] PARAMETERS = new String[] {};
         public static ScriptContext<Factory> CONTEXT = new ScriptContext<>("aggs_map", Factory.class);
     }
 
     public abstract static class CombineScript extends ParamsAndStateBase {
-        public CombineScript(Map<String, Object> params, Object state) {
+        public CombineScript(Map<String, Object> params, Map<String, Object> state) {
             super(params, state);
         }
 
         public abstract Object execute();
 
         public interface Factory {
-            CombineScript newInstance(Map<String, Object> params, Object state);
+            CombineScript newInstance(Map<String, Object> params, Map<String, Object> state);
         }
 
         public static String[] PARAMETERS = {};

server/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/InternalScriptedMetric.java

@@ -95,11 +95,6 @@ public InternalAggregation doReduce(List<InternalAggregation> aggregations, Redu
                 params.putAll(firstAggregation.reduceScript.getParams());
             }
 
-            // Add _aggs to params map for backwards compatibility (redundant with a context variable on the ReduceScript created below).
-            if (ScriptedMetricAggContexts.deprecatedAggParamEnabled()) {
-                params.put("_aggs", aggregationObjects);
-            }
-
             ScriptedMetricAggContexts.ReduceScript.Factory factory = reduceContext.scriptService().compile(
                 firstAggregation.reduceScript, ScriptedMetricAggContexts.ReduceScript.CONTEXT);
             ScriptedMetricAggContexts.ReduceScript script = factory.newInstance(params, aggregationObjects);

server/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregator.java

@@ -41,10 +41,10 @@ public class ScriptedMetricAggregator extends MetricsAggregator {
     private final ScriptedMetricAggContexts.MapScript.LeafFactory mapScript;
     private final ScriptedMetricAggContexts.CombineScript combineScript;
     private final Script reduceScript;
-    private Object aggState;
+    private Map<String, Object> aggState;
 
     protected ScriptedMetricAggregator(String name, ScriptedMetricAggContexts.MapScript.LeafFactory mapScript, ScriptedMetricAggContexts.CombineScript combineScript,
-                                       Script reduceScript, Object aggState, SearchContext context, Aggregator parent,
+                                       Script reduceScript, Map<String, Object> aggState, SearchContext context, Aggregator parent,
                                        List<PipelineAggregator> pipelineAggregators, Map<String, Object> metaData)
                                        throws IOException {
         super(name, context, parent, pipelineAggregators, metaData);

server/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregatorFactory.java

@@ -80,20 +80,7 @@ public Aggregator createInternal(Aggregator parent, boolean collectsFromSingleBu
             aggParams = new HashMap<>();
         }
 
-        // Add _agg to params map for backwards compatibility (redundant with context variables on the scripts created below).
-        // When this is removed, aggState (as passed to ScriptedMetricAggregator) can be changed to Map<String, Object>, since
-        // it won't be possible to completely replace it with another type as is possible when it's an entry in params.
-        Object aggState = new HashMap<String, Object>();
-        if (ScriptedMetricAggContexts.deprecatedAggParamEnabled()) {
-            if (aggParams.containsKey("_agg") == false) {
-                // Add _agg if it wasn't added manually
-                aggParams.put("_agg", aggState);
-            } else {
-                // If it was added manually, also use it for the agg context variable to reduce the likelihood of
-                // weird behavior due to multiple different variables.
-                aggState = aggParams.get("_agg");
-            }
-        }
+        Map<String, Object> aggState = new HashMap<String, Object>();
 
         final ScriptedMetricAggContexts.InitScript initScript = this.initScript.newInstance(
             mergeParams(aggParams, initScriptParams), aggState);