scripted_metric _agg parameter disappears if params are provided (#27159

) * Fixes #19768: scripted_metric _agg parameter disappears if params are provided * Test case for #19768 * Compare boolean to false instead of negating it * Added mocked script in ScriptedMetricIT * Fix test in ScriptedMetricIT for implicit _agg map
elastic · Nov 8, 2017 · f321c6d · f321c6d
1 parent de61189
commit f321c6d
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 0 deletions.
diff --git a/...g/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregatorFactory.java b/...g/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregatorFactory.java
@@ -70,6 +70,8 @@ public Aggregator createInternal(Aggregator parent, boolean collectsFromSingleBu
             params = deepCopyParams(params, context);
         } else {
             params = new HashMap<>();
+        }
+        if (params.containsKey("_agg") == false) {
             params.put("_agg", new HashMap<String, Object>());
         }
 

diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/ScriptedMetricIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/ScriptedMetricIT.java
@@ -94,6 +94,10 @@ protected Map<String, Function<Map<String, Object>, Object>> pluginScripts() {
             scripts.put("_agg.add(1)", vars ->
                     aggScript(vars, agg -> ((List) agg).add(1)));
 
+            scripts.put("_agg[param1] = param2", vars ->
+                    aggScript(vars, agg -> ((Map) agg).put(XContentMapValues.extractValue("params.param1", vars),
+                        XContentMapValues.extractValue("params.param2", vars))));
+
             scripts.put("vars.multiplier = 3", vars ->
                     ((Map<String, Object>) vars.get("vars")).put("multiplier", 3));
 
@@ -356,6 +360,52 @@ public void testMapWithParams() {
         assertThat(totalCount, equalTo(numDocs));
     }
 
+    public void testMapWithParamsAndImplicitAggMap() {
+        Map<String, Object> params = new HashMap<>();
+        // don't put any _agg map in params
+        params.put("param1", "12");
+        params.put("param2", 1);
+
+        // The _agg hashmap will be available even if not declared in the params map
+        Script mapScript = new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "_agg[param1] = param2", params);
+
+        SearchResponse response = client().prepareSearch("idx")
+            .setQuery(matchAllQuery())
+            .addAggregation(scriptedMetric("scripted").params(params).mapScript(mapScript))
+            .get();
+        assertSearchResponse(response);
+        assertThat(response.getHits().getTotalHits(), equalTo(numDocs));
+
+        Aggregation aggregation = response.getAggregations().get("scripted");
+        assertThat(aggregation, notNullValue());
+        assertThat(aggregation, instanceOf(ScriptedMetric.class));
+        ScriptedMetric scriptedMetricAggregation = (ScriptedMetric) aggregation;
+        assertThat(scriptedMetricAggregation.getName(), equalTo("scripted"));
+        assertThat(scriptedMetricAggregation.aggregation(), notNullValue());
+        assertThat(scriptedMetricAggregation.aggregation(), instanceOf(ArrayList.class));
+        List<?> aggregationList = (List<?>) scriptedMetricAggregation.aggregation();
+        assertThat(aggregationList.size(), equalTo(getNumShards("idx").numPrimaries));
+        int numShardsRun = 0;
+        for (Object object : aggregationList) {
+            assertThat(object, notNullValue());
+            assertThat(object, instanceOf(Map.class));
+            Map<?,?> map = (Map<?,?>) object;
+            for (Map.Entry<?,?> entry : map.entrySet()) {
+                assertThat(entry, notNullValue());
+                assertThat(entry.getKey(), notNullValue());
+                assertThat(entry.getKey(), instanceOf(String.class));
+                assertThat(entry.getValue(), notNullValue());
+                assertThat(entry.getValue(), instanceOf(Number.class));
+                String stringValue = (String) entry.getKey();
+                assertThat(stringValue, equalTo("12"));
+                Number numberValue = (Number) entry.getValue();
+                assertThat(numberValue, equalTo((Number) 1));
+                numShardsRun++;
+            }
+        }
+        assertThat(numShardsRun, greaterThan(0));
+    }
+
     public void testInitMapWithParams() {
         Map<String, Object> varsMap = new HashMap<>();
         varsMap.put("multiplier", 1);