Skip to content

Commit d1fac56

Browse files
original-brownbearoriginal-brownbear
committed
Replace Ingest ScriptContext with Custom Interface (elastic#32003)
* Replace Ingest ScriptContext with Custom Interface * Make org.elasticsearch.ingest.common.ScriptProcessorTests#testScripting more precise * Don't mock script factory in ScriptProcessorTests * Adjust mock script plugin in IT for new API
1 parent ba8a54a commit d1fac56

File tree

8 files changed

+91
-32
lines changed

8 files changed

+91
-32
lines changed

‎modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/ScriptProcessor.java

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
import org.elasticsearch.ingest.AbstractProcessor;
2727
import org.elasticsearch.ingest.IngestDocument;
2828
import org.elasticsearch.ingest.Processor;
29-
import org.elasticsearch.script.ExecutableScript;
29+
import org.elasticsearch.script.IngestScript;
3030
import org.elasticsearch.script.Script;
3131
import org.elasticsearch.script.ScriptException;
3232
import org.elasticsearch.script.ScriptService;
@@ -71,10 +71,8 @@ public final class ScriptProcessor extends AbstractProcessor {
7171
*/
7272
@Override
7373
public void execute(IngestDocument document) {
74-
ExecutableScript.Factory factory = scriptService.compile(script, ExecutableScript.INGEST_CONTEXT);
75-
ExecutableScript executableScript = factory.newInstance(script.getParams());
76-
executableScript.setNextVar("ctx", document.getSourceAndMetadata());
77-
executableScript.run();
74+
IngestScript.Factory factory = scriptService.compile(script, IngestScript.CONTEXT);
75+
factory.newInstance(script.getParams()).execute(document.getSourceAndMetadata());
7876
}
7977

8078
@Override
@@ -144,7 +142,7 @@ public ScriptProcessor create(Map<String, Processor.Factory> registry, String pr
144142

145143
// verify script is able to be compiled before successfully creating processor.
146144
try {
147-
scriptService.compile(script, ExecutableScript.INGEST_CONTEXT);
145+
scriptService.compile(script, IngestScript.CONTEXT);
148146
} catch (ScriptException e) {
149147
throw newConfigurationException(TYPE, processorTag, scriptPropertyUsed, e);
150148
}

‎modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/IngestRestartIT.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,7 @@ protected boolean ignoreExternalCluster() {
5858
public static class CustomScriptPlugin extends MockScriptPlugin {
5959
@Override
6060
protected Map<String, Function<Map<String, Object>, Object>> pluginScripts() {
61-
return Collections.singletonMap("my_script", script -> {
62-
@SuppressWarnings("unchecked")
63-
Map<String, Object> ctx = (Map) script.get("ctx");
61+
return Collections.singletonMap("my_script", ctx -> {
6462
ctx.put("z", 0);
6563
return null;
6664
});

‎modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ScriptProcessorTests.java

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -19,47 +19,51 @@
1919

2020
package org.elasticsearch.ingest.common;
2121

22+
import java.util.Collections;
2223
import java.util.HashMap;
2324
import java.util.Map;
2425

26+
import org.elasticsearch.common.settings.Settings;
2527
import org.elasticsearch.ingest.IngestDocument;
2628
import org.elasticsearch.ingest.RandomDocumentPicks;
27-
import org.elasticsearch.script.ExecutableScript;
29+
import org.elasticsearch.script.MockScriptEngine;
2830
import org.elasticsearch.script.Script;
31+
import org.elasticsearch.script.ScriptModule;
2932
import org.elasticsearch.script.ScriptService;
33+
import org.elasticsearch.script.ScriptType;
3034
import org.elasticsearch.test.ESTestCase;
3135

3236
import static org.hamcrest.Matchers.hasKey;
3337
import static org.hamcrest.core.Is.is;
34-
import static org.mockito.Mockito.any;
35-
import static org.mockito.Mockito.doAnswer;
36-
import static org.mockito.Mockito.mock;
37-
import static org.mockito.Mockito.when;
3838

3939
public class ScriptProcessorTests extends ESTestCase {
4040

4141
public void testScripting() throws Exception {
4242
int randomBytesIn = randomInt();
4343
int randomBytesOut = randomInt();
4444
int randomBytesTotal = randomBytesIn + randomBytesOut;
45-
46-
ScriptService scriptService = mock(ScriptService.class);
47-
Script script = mockScript("_script");
48-
ExecutableScript.Factory factory = mock(ExecutableScript.Factory.class);
49-
ExecutableScript executableScript = mock(ExecutableScript.class);
50-
when(scriptService.compile(script, ExecutableScript.INGEST_CONTEXT)).thenReturn(factory);
51-
when(factory.newInstance(any())).thenReturn(executableScript);
45+
String scriptName = "script";
46+
ScriptService scriptService = new ScriptService(Settings.builder().build(),
47+
Collections.singletonMap(
48+
Script.DEFAULT_SCRIPT_LANG, new MockScriptEngine(
49+
Script.DEFAULT_SCRIPT_LANG,
50+
Collections.singletonMap(
51+
scriptName, ctx -> {
52+
ctx.put("bytes_total", randomBytesTotal);
53+
return null;
54+
}
55+
)
56+
)
57+
),
58+
new HashMap<>(ScriptModule.CORE_CONTEXTS)
59+
);
60+
Script script = new Script(ScriptType.INLINE, Script.DEFAULT_SCRIPT_LANG, scriptName, Collections.emptyMap());
5261

5362
Map<String, Object> document = new HashMap<>();
5463
document.put("bytes_in", randomInt());
5564
document.put("bytes_out", randomInt());
5665
IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document);
5766

58-
doAnswer(invocationOnMock -> {
59-
ingestDocument.setFieldValue("bytes_total", randomBytesTotal);
60-
return null;
61-
}).when(executableScript).run();
62-
6367
ScriptProcessor processor = new ScriptProcessor(randomAlphaOfLength(10), script, scriptService);
6468

6569
processor.execute(ingestDocument);

‎server/src/main/java/org/elasticsearch/script/ExecutableScript.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,5 +50,4 @@ interface Factory {
5050
// TODO: remove these once each has its own script interface
5151
ScriptContext<Factory> AGGS_CONTEXT = new ScriptContext<>("aggs_executable", Factory.class);
5252
ScriptContext<Factory> UPDATE_CONTEXT = new ScriptContext<>("update", Factory.class);
53-
ScriptContext<Factory> INGEST_CONTEXT = new ScriptContext<>("ingest", Factory.class);
5453
}

‎server/src/main/java/org/elasticsearch/script/IngestScript.java

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
2+
/*
3+
* Licensed to Elasticsearch under one or more contributor
4+
* license agreements. See the NOTICE file distributed with
5+
* this work for additional information regarding copyright
6+
* ownership. Elasticsearch licenses this file to you under
7+
* the Apache License, Version 2.0 (the "License"); you may
8+
* not use this file except in compliance with the License.
9+
* You may obtain a copy of the License at
10+
*
11+
* http://www.apache.org/licenses/LICENSE-2.0
12+
*
13+
* Unless required by applicable law or agreed to in writing,
14+
* software distributed under the License is distributed on an
15+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
16+
* KIND, either express or implied. See the License for the
17+
* specific language governing permissions and limitations
18+
* under the License.
19+
*/
20+
21+
package org.elasticsearch.script;
22+
23+
import java.util.Map;
24+
25+
/**
26+
* A script used by the Ingest Script Processor.
27+
*/
28+
public abstract class IngestScript {
29+
30+
public static final String[] PARAMETERS = { "ctx" };
31+
32+
/** The context used to compile {@link IngestScript} factories. */
33+
public static final ScriptContext<Factory> CONTEXT = new ScriptContext<>("ingest", Factory.class);
34+
35+
/** The generic runtime parameters for the script. */
36+
private final Map<String, Object> params;
37+
38+
public IngestScript(Map<String, Object> params) {
39+
this.params = params;
40+
}
41+
42+
/** Return the parameters for this script. */
43+
public Map<String, Object> getParams() {
44+
return params;
45+
}
46+
47+
public abstract void execute(Map<String, Object> ctx);
48+
49+
public interface Factory {
50+
IngestScript newInstance(Map<String, Object> params);
51+
}
52+
}

‎server/src/main/java/org/elasticsearch/script/ScriptModule.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public class ScriptModule {
4848
ExecutableScript.CONTEXT,
4949
ExecutableScript.AGGS_CONTEXT,
5050
ExecutableScript.UPDATE_CONTEXT,
51-
ExecutableScript.INGEST_CONTEXT,
51+
IngestScript.CONTEXT,
5252
FilterScript.CONTEXT,
5353
SimilarityScript.CONTEXT,
5454
SimilarityWeightScript.CONTEXT,

‎server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ public void testAllowAllScriptContextSettings() throws IOException {
168168
assertCompileAccepted("painless", "script", ScriptType.INLINE, SearchScript.CONTEXT);
169169
assertCompileAccepted("painless", "script", ScriptType.INLINE, SearchScript.AGGS_CONTEXT);
170170
assertCompileAccepted("painless", "script", ScriptType.INLINE, ExecutableScript.UPDATE_CONTEXT);
171-
assertCompileAccepted("painless", "script", ScriptType.INLINE, ExecutableScript.INGEST_CONTEXT);
171+
assertCompileAccepted("painless", "script", ScriptType.INLINE, IngestScript.CONTEXT);
172172
}
173173

174174
public void testAllowSomeScriptTypeSettings() throws IOException {
@@ -209,13 +209,13 @@ public void testAllowNoScriptContextSettings() throws IOException {
209209
}
210210

211211
public void testCompileNonRegisteredContext() throws IOException {
212-
contexts.remove(ExecutableScript.INGEST_CONTEXT.name);
212+
contexts.remove(IngestScript.CONTEXT.name);
213213
buildScriptService(Settings.EMPTY);
214214

215215
String type = scriptEngine.getType();
216216
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () ->
217-
scriptService.compile(new Script(ScriptType.INLINE, type, "test", Collections.emptyMap()), ExecutableScript.INGEST_CONTEXT));
218-
assertThat(e.getMessage(), containsString("script context [" + ExecutableScript.INGEST_CONTEXT.name + "] not supported"));
217+
scriptService.compile(new Script(ScriptType.INLINE, type, "test", Collections.emptyMap()), IngestScript.CONTEXT));
218+
assertThat(e.getMessage(), containsString("script context [" + IngestScript.CONTEXT.name + "] not supported"));
219219
}
220220

221221
public void testCompileCountedInCompilationStats() throws IOException {

‎test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,14 @@ public <T> T compile(String name, String source, ScriptContext<T> context, Map<S
8888
} else if (context.instanceClazz.equals(ExecutableScript.class)) {
8989
ExecutableScript.Factory factory = mockCompiled::createExecutableScript;
9090
return context.factoryClazz.cast(factory);
91+
} else if (context.instanceClazz.equals(IngestScript.class)) {
92+
IngestScript.Factory factory = parameters -> new IngestScript(parameters) {
93+
@Override
94+
public void execute(Map<String, Object> ctx) {
95+
script.apply(ctx);
96+
}
97+
};
98+
return context.factoryClazz.cast(factory);
9199
} else if (context.instanceClazz.equals(TemplateScript.class)) {
92100
TemplateScript.Factory factory = vars -> {
93101
// TODO: need a better way to implement all these new contexts

0 commit comments

Comments
 (0)