Browse Source

remove ability to set field value in script-processor configuration (#19981)

Tal Levy 9 years ago
parent
commit
84bf24b1e9

+ 3 - 9
modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/ScriptProcessor.java

@@ -51,13 +51,11 @@ public final class ScriptProcessor extends AbstractProcessor {
 
     private final Script script;
     private final ScriptService scriptService;
-    private final String field;
 
-    ScriptProcessor(String tag, Script script, ScriptService scriptService, String field)  {
+    ScriptProcessor(String tag, Script script, ScriptService scriptService)  {
         super(tag);
         this.script = script;
         this.scriptService = scriptService;
-        this.field = field;
     }
 
     @Override
@@ -66,10 +64,7 @@ public final class ScriptProcessor extends AbstractProcessor {
         vars.put("ctx", document.getSourceAndMetadata());
         CompiledScript compiledScript = scriptService.compile(script, ScriptContext.Standard.INGEST, emptyMap());
         ExecutableScript executableScript = scriptService.executable(compiledScript, vars);
-        Object value = executableScript.run();
-        if (field != null) {
-            document.setFieldValue(field, value);
-        }
+        executableScript.run();
     }
 
     @Override
@@ -88,7 +83,6 @@ public final class ScriptProcessor extends AbstractProcessor {
         @Override
         public ScriptProcessor create(Map<String, Processor.Factory> registry, String processorTag,
                                       Map<String, Object> config) throws Exception {
-            String field = readOptionalStringProperty(TYPE, processorTag, config, "field");
             String lang = readStringProperty(TYPE, processorTag, config, "lang");
             String inline = readOptionalStringProperty(TYPE, processorTag, config, "inline");
             String file = readOptionalStringProperty(TYPE, processorTag, config, "file");
@@ -116,7 +110,7 @@ public final class ScriptProcessor extends AbstractProcessor {
                 throw newConfigurationException(TYPE, processorTag, null, "Could not initialize script");
             }
 
-            return new ScriptProcessor(processorTag, script, scriptService, field);
+            return new ScriptProcessor(processorTag, script, scriptService);
         }
     }
 }

+ 0 - 2
modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ScriptProcessorFactoryTests.java

@@ -52,7 +52,6 @@ public class ScriptProcessorFactoryTests extends ESTestCase {
 
         configMap.put(randomType, "foo");
         configMap.put(otherRandomType, "bar");
-        configMap.put("field", "my_field");
         configMap.put("lang", "mockscript");
 
         ElasticsearchException exception = expectThrows(ElasticsearchException.class,
@@ -62,7 +61,6 @@ public class ScriptProcessorFactoryTests extends ESTestCase {
 
     public void testFactoryValidationAtLeastOneScriptingType() throws Exception {
         Map<String, Object> configMap = new HashMap<>();
-        configMap.put("field", "my_field");
         configMap.put("lang", "mockscript");
 
         ElasticsearchException exception = expectThrows(ElasticsearchException.class,

+ 17 - 8
modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ScriptProcessorTests.java

@@ -29,37 +29,46 @@ import org.elasticsearch.script.ExecutableScript;
 import org.elasticsearch.script.Script;
 import org.elasticsearch.script.ScriptService;
 import org.elasticsearch.test.ESTestCase;
+import org.mockito.stubbing.Answer;
 
 import static org.hamcrest.Matchers.hasKey;
 import static org.hamcrest.core.Is.is;
 import static org.mockito.Mockito.any;
+import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 public class ScriptProcessorTests extends ESTestCase {
 
     public void testScripting() throws Exception {
-        int randomInt = randomInt();
+        int randomBytesIn = randomInt();
+        int randomBytesOut = randomInt();
+        int randomBytesTotal = randomBytesIn + randomBytesOut;
+
         ScriptService scriptService = mock(ScriptService.class);
         CompiledScript compiledScript = mock(CompiledScript.class);
         Script script = new Script("_script");
         when(scriptService.compile(any(), any(), any())).thenReturn(compiledScript);
         ExecutableScript executableScript = mock(ExecutableScript.class);
         when(scriptService.executable(any(), any())).thenReturn(executableScript);
-        when(executableScript.run()).thenReturn(randomInt);
-
-        ScriptProcessor processor = new ScriptProcessor(randomAsciiOfLength(10), script,
-            scriptService, "bytes_total");
 
         Map<String, Object> document = new HashMap<>();
-        document.put("bytes_in", 1234);
-        document.put("bytes_out", 4321);
+        document.put("bytes_in", randomInt());
+        document.put("bytes_out", randomInt());
         IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document);
+
+        doAnswer(invocationOnMock ->  {
+            ingestDocument.setFieldValue("bytes_total", randomBytesTotal);
+            return null;
+        }).when(executableScript).run();
+
+        ScriptProcessor processor = new ScriptProcessor(randomAsciiOfLength(10), script, scriptService);
+
         processor.execute(ingestDocument);
 
         assertThat(ingestDocument.getSourceAndMetadata(), hasKey("bytes_in"));
         assertThat(ingestDocument.getSourceAndMetadata(), hasKey("bytes_out"));
         assertThat(ingestDocument.getSourceAndMetadata(), hasKey("bytes_total"));
-        assertThat(ingestDocument.getSourceAndMetadata().get("bytes_total"), is(randomInt));
+        assertThat(ingestDocument.getSourceAndMetadata().get("bytes_total"), is(randomBytesTotal));
     }
 }

+ 2 - 5
qa/smoke-test-ingest-with-all-dependencies/src/test/resources/rest-api-spec/test/ingest/50_script_processor_using_painless.yaml

@@ -9,9 +9,8 @@
             "processors": [
               {
                 "script" : {
-                  "field" : "bytes_total",
                   "lang" : "painless",
-                  "inline": "return ctx.bytes_in + ctx.bytes_out"
+                  "inline": "ctx.bytes_total = ctx.bytes_in + ctx.bytes_out"
                 }
               }
             ]
@@ -46,7 +45,6 @@
             "processors": [
               {
                 "script" : {
-                  "field" : "bytes_total",
                   "lang" : "painless",
                   "file": "master"
                 }
@@ -80,7 +78,7 @@
         lang: "painless"
         body: >
           {
-            "script" : "return ctx.bytes_in + ctx.bytes_out"
+            "script" : "ctx.bytes_total = ctx.bytes_in + ctx.bytes_out"
           }
   - match: { acknowledged: true }
 
@@ -93,7 +91,6 @@
             "processors": [
               {
                 "script" : {
-                  "field" : "bytes_total",
                   "lang" : "painless",
                   "id" : "sum_bytes"
                 }

+ 1 - 1
qa/smoke-test-ingest-with-all-dependencies/src/test/resources/scripts/master.painless

@@ -1 +1 @@
-return ctx.bytes_in + ctx.bytes_out
+ctx.bytes_total = ctx.bytes_in + ctx.bytes_out