Skip to content

Commit

Permalink
Merge branch 'master' into ccr
Browse files Browse the repository at this point in the history
* master:
  Replace Ingest ScriptContext with Custom Interface (#32003)
  Mute failing tests
  • Loading branch information
dnhatn committed Jul 13, 2018
2 parents e26f3e0 + 3679d00 commit 89a590a
Show file tree
Hide file tree
Showing 10 changed files with 110 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
import org.elasticsearch.ingest.AbstractProcessor;
import org.elasticsearch.ingest.IngestDocument;
import org.elasticsearch.ingest.Processor;
import org.elasticsearch.script.ExecutableScript;
import org.elasticsearch.script.IngestScript;
import org.elasticsearch.script.Script;
import org.elasticsearch.script.ScriptException;
import org.elasticsearch.script.ScriptService;
Expand Down Expand Up @@ -73,10 +73,8 @@ public final class ScriptProcessor extends AbstractProcessor {
*/
@Override
public void execute(IngestDocument document) {
ExecutableScript.Factory factory = scriptService.compile(script, ExecutableScript.INGEST_CONTEXT);
ExecutableScript executableScript = factory.newInstance(script.getParams());
executableScript.setNextVar("ctx", document.getSourceAndMetadata());
executableScript.run();
IngestScript.Factory factory = scriptService.compile(script, IngestScript.CONTEXT);
factory.newInstance(script.getParams()).execute(document.getSourceAndMetadata());
}

@Override
Expand Down Expand Up @@ -108,7 +106,7 @@ public ScriptProcessor create(Map<String, Processor.Factory> registry, String pr

// verify script is able to be compiled before successfully creating processor.
try {
scriptService.compile(script, ExecutableScript.INGEST_CONTEXT);
scriptService.compile(script, IngestScript.CONTEXT);
} catch (ScriptException e) {
throw newConfigurationException(TYPE, processorTag, null, e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,7 @@ protected boolean ignoreExternalCluster() {
public static class CustomScriptPlugin extends MockScriptPlugin {
@Override
protected Map<String, Function<Map<String, Object>, Object>> pluginScripts() {
return Collections.singletonMap("my_script", script -> {
@SuppressWarnings("unchecked")
Map<String, Object> ctx = (Map<String, Object>) script.get("ctx");
return Collections.singletonMap("my_script", ctx -> {
ctx.put("z", 0);
return null;
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,47 +19,51 @@

package org.elasticsearch.ingest.common;

import java.util.Collections;
import java.util.HashMap;
import java.util.Map;

import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.ingest.IngestDocument;
import org.elasticsearch.ingest.RandomDocumentPicks;
import org.elasticsearch.script.ExecutableScript;
import org.elasticsearch.script.MockScriptEngine;
import org.elasticsearch.script.Script;
import org.elasticsearch.script.ScriptModule;
import org.elasticsearch.script.ScriptService;
import org.elasticsearch.script.ScriptType;
import org.elasticsearch.test.ESTestCase;

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 randomBytesIn = randomInt();
int randomBytesOut = randomInt();
int randomBytesTotal = randomBytesIn + randomBytesOut;

ScriptService scriptService = mock(ScriptService.class);
Script script = mockScript("_script");
ExecutableScript.Factory factory = mock(ExecutableScript.Factory.class);
ExecutableScript executableScript = mock(ExecutableScript.class);
when(scriptService.compile(script, ExecutableScript.INGEST_CONTEXT)).thenReturn(factory);
when(factory.newInstance(any())).thenReturn(executableScript);
String scriptName = "script";
ScriptService scriptService = new ScriptService(Settings.builder().build(),
Collections.singletonMap(
Script.DEFAULT_SCRIPT_LANG, new MockScriptEngine(
Script.DEFAULT_SCRIPT_LANG,
Collections.singletonMap(
scriptName, ctx -> {
ctx.put("bytes_total", randomBytesTotal);
return null;
}
)
)
),
new HashMap<>(ScriptModule.CORE_CONTEXTS)
);
Script script = new Script(ScriptType.INLINE, Script.DEFAULT_SCRIPT_LANG, scriptName, Collections.emptyMap());

Map<String, Object> document = new HashMap<>();
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(randomAlphaOfLength(10), script, scriptService);

processor.execute(ingestDocument);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@ setup:
---
"Nested inner hits":
- skip:
version: " - 6.1.99"
reason: "<= 6.1 nodes don't always include index or id in nested inner hits"
version: "all"
reason: "https://github.com/elastic/elasticsearch/issues/32055"

- do:
index:
index: test
Expand Down Expand Up @@ -45,8 +46,8 @@ setup:
"Nested doc version and seqIDs":

- skip:
version: " - 6.3.99"
reason: "object notation for docvalue_fields was introduced in 6.4"
version: "all"
reason: "https://github.com/elastic/elasticsearch/issues/32055"

- do:
index:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,9 @@ setup:

---
"field collapsing and inner_hits":
- skip:
version: "all"
reason: "https://github.com/elastic/elasticsearch/issues/32055"

- do:
search:
Expand Down Expand Up @@ -146,6 +149,9 @@ setup:

---
"field collapsing, inner_hits and maxConcurrentGroupRequests":
- skip:
version: "all"
reason: "https://github.com/elastic/elasticsearch/issues/32055"

- do:
search:
Expand Down Expand Up @@ -226,6 +232,9 @@ setup:

---
"no hits and inner_hits":
- skip:
version: "all"
reason: "https://github.com/elastic/elasticsearch/issues/32055"

- do:
search:
Expand All @@ -240,6 +249,9 @@ setup:

---
"field collapsing and multiple inner_hits":
- skip:
version: "all"
reason: "https://github.com/elastic/elasticsearch/issues/32055"

- do:
search:
Expand Down Expand Up @@ -292,10 +304,9 @@ setup:

---
"field collapsing, inner_hits and version":

- skip:
version: " - 6.1.0"
reason: "bug fixed in 6.1.1"
version: "all"
reason: "https://github.com/elastic/elasticsearch/issues/32055"

- do:
search:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,5 +50,4 @@ interface Factory {
// TODO: remove these once each has its own script interface
ScriptContext<Factory> AGGS_CONTEXT = new ScriptContext<>("aggs_executable", Factory.class);
ScriptContext<Factory> UPDATE_CONTEXT = new ScriptContext<>("update", Factory.class);
ScriptContext<Factory> INGEST_CONTEXT = new ScriptContext<>("ingest", Factory.class);
}
52 changes: 52 additions & 0 deletions server/src/main/java/org/elasticsearch/script/IngestScript.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@

/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

package org.elasticsearch.script;

import java.util.Map;

/**
* A script used by the Ingest Script Processor.
*/
public abstract class IngestScript {

public static final String[] PARAMETERS = { "ctx" };

/** The context used to compile {@link IngestScript} factories. */
public static final ScriptContext<Factory> CONTEXT = new ScriptContext<>("ingest", Factory.class);

/** The generic runtime parameters for the script. */
private final Map<String, Object> params;

public IngestScript(Map<String, Object> params) {
this.params = params;
}

/** Return the parameters for this script. */
public Map<String, Object> getParams() {
return params;
}

public abstract void execute(Map<String, Object> ctx);

public interface Factory {
IngestScript newInstance(Map<String, Object> params);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public class ScriptModule {
ExecutableScript.CONTEXT,
ExecutableScript.AGGS_CONTEXT,
ExecutableScript.UPDATE_CONTEXT,
ExecutableScript.INGEST_CONTEXT,
IngestScript.CONTEXT,
FilterScript.CONTEXT,
SimilarityScript.CONTEXT,
SimilarityWeightScript.CONTEXT,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ public void testAllowAllScriptContextSettings() throws IOException {
assertCompileAccepted("painless", "script", ScriptType.INLINE, SearchScript.CONTEXT);
assertCompileAccepted("painless", "script", ScriptType.INLINE, SearchScript.AGGS_CONTEXT);
assertCompileAccepted("painless", "script", ScriptType.INLINE, ExecutableScript.UPDATE_CONTEXT);
assertCompileAccepted("painless", "script", ScriptType.INLINE, ExecutableScript.INGEST_CONTEXT);
assertCompileAccepted("painless", "script", ScriptType.INLINE, IngestScript.CONTEXT);
}

public void testAllowSomeScriptTypeSettings() throws IOException {
Expand Down Expand Up @@ -209,13 +209,13 @@ public void testAllowNoScriptContextSettings() throws IOException {
}

public void testCompileNonRegisteredContext() throws IOException {
contexts.remove(ExecutableScript.INGEST_CONTEXT.name);
contexts.remove(IngestScript.CONTEXT.name);
buildScriptService(Settings.EMPTY);

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

public void testCompileCountedInCompilationStats() throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,14 @@ public <T> T compile(String name, String source, ScriptContext<T> context, Map<S
} else if (context.instanceClazz.equals(ExecutableScript.class)) {
ExecutableScript.Factory factory = mockCompiled::createExecutableScript;
return context.factoryClazz.cast(factory);
} else if (context.instanceClazz.equals(IngestScript.class)) {
IngestScript.Factory factory = parameters -> new IngestScript(parameters) {
@Override
public void execute(Map<String, Object> ctx) {
script.apply(ctx);
}
};
return context.factoryClazz.cast(factory);
} else if (context.instanceClazz.equals(TemplateScript.class)) {
TemplateScript.Factory factory = vars -> {
// TODO: need a better way to implement all these new contexts
Expand Down

0 comments on commit 89a590a

Please sign in to comment.