diff --git a/server/src/main/java/org/elasticsearch/ingest/ConditionalProcessor.java b/server/src/main/java/org/elasticsearch/ingest/ConditionalProcessor.java index f38d0cc07e9..3179830dd8b 100644 --- a/server/src/main/java/org/elasticsearch/ingest/ConditionalProcessor.java +++ b/server/src/main/java/org/elasticsearch/ingest/ConditionalProcessor.java @@ -24,7 +24,9 @@ import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.script.DynamicMap; import org.elasticsearch.script.IngestConditionalScript; import org.elasticsearch.script.Script; +import org.elasticsearch.script.ScriptException; import org.elasticsearch.script.ScriptService; +import org.elasticsearch.script.ScriptType; import java.util.ArrayList; import java.util.Arrays; @@ -41,6 +43,8 @@ import java.util.function.Function; import java.util.function.LongSupplier; import java.util.stream.Collectors; +import static org.elasticsearch.ingest.ConfigurationUtils.newConfigurationException; + public class ConditionalProcessor extends AbstractProcessor implements WrappingProcessor { private static final DeprecationLogger deprecationLogger = @@ -55,12 +59,11 @@ public class ConditionalProcessor extends AbstractProcessor implements WrappingP static final String TYPE = "conditional"; private final Script condition; - private final ScriptService scriptService; - private final Processor processor; private final IngestMetric metric; private final LongSupplier relativeTimeProvider; + private final IngestConditionalScript precompiledConditionScript; ConditionalProcessor(String tag, String description, Script script, ScriptService scriptService, Processor processor) { this(tag, description, script, scriptService, processor, System::nanoTime); @@ -74,6 +77,18 @@ public class ConditionalProcessor extends AbstractProcessor implements WrappingP this.processor = processor; this.metric = new IngestMetric(); this.relativeTimeProvider = relativeTimeProvider; + + try { + final IngestConditionalScript.Factory factory = scriptService.compile(script, IngestConditionalScript.CONTEXT); + if (ScriptType.INLINE.equals(script.getType())) { + precompiledConditionScript = factory.newInstance(script.getParams()); + } else { + // stored script, so will have to compile at runtime + precompiledConditionScript = null; + } + } catch (ScriptException e) { + throw newConfigurationException(TYPE, tag, null, e); + } } @Override @@ -110,8 +125,11 @@ public class ConditionalProcessor extends AbstractProcessor implements WrappingP } boolean evaluate(IngestDocument ingestDocument) { - IngestConditionalScript script = - scriptService.compile(condition, IngestConditionalScript.CONTEXT).newInstance(condition.getParams()); + IngestConditionalScript script = precompiledConditionScript; + if (script == null) { + IngestConditionalScript.Factory factory = scriptService.compile(condition, IngestConditionalScript.CONTEXT); + script = factory.newInstance(condition.getParams()); + } return script.execute(new UnmodifiableIngestData(new DynamicMap(ingestDocument.getSourceAndMetadata(), FUNCTIONS))); } diff --git a/server/src/main/java/org/elasticsearch/script/ScriptService.java b/server/src/main/java/org/elasticsearch/script/ScriptService.java index f78ab24b75e..1f752942d74 100644 --- a/server/src/main/java/org/elasticsearch/script/ScriptService.java +++ b/server/src/main/java/org/elasticsearch/script/ScriptService.java @@ -415,7 +415,7 @@ public class ScriptService implements Closeable, ClusterStateApplier { return scriptMetadata.getStoredScripts(); } - StoredScriptSource getScriptFromClusterState(String id) { + protected StoredScriptSource getScriptFromClusterState(String id) { ScriptMetadata scriptMetadata = clusterState.metadata().custom(ScriptMetadata.TYPE); if (scriptMetadata == null) { diff --git a/server/src/test/java/org/elasticsearch/ingest/ConditionalProcessorTests.java b/server/src/test/java/org/elasticsearch/ingest/ConditionalProcessorTests.java index 6bcbba09ab0..829c4f2b943 100644 --- a/server/src/test/java/org/elasticsearch/ingest/ConditionalProcessorTests.java +++ b/server/src/test/java/org/elasticsearch/ingest/ConditionalProcessorTests.java @@ -20,13 +20,18 @@ package org.elasticsearch.ingest; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.script.IngestConditionalScript; import org.elasticsearch.script.MockScriptEngine; +import org.elasticsearch.script.MockScriptService; import org.elasticsearch.script.Script; +import org.elasticsearch.script.ScriptException; import org.elasticsearch.script.ScriptModule; import org.elasticsearch.script.ScriptService; import org.elasticsearch.script.ScriptType; +import org.elasticsearch.script.StoredScriptSource; import org.elasticsearch.test.ESTestCase; +import java.text.ParseException; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; @@ -34,6 +39,7 @@ import java.util.List; import java.util.Map; import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Consumer; import java.util.function.LongSupplier; @@ -195,6 +201,63 @@ public class ConditionalProcessorTests extends ESTestCase { assertWarnings("[types removal] Looking up doc types [_type] in scripts is deprecated."); } + public void testPrecompiledError() { + ScriptService scriptService = MockScriptService.singleContext(IngestConditionalScript.CONTEXT, code -> { + throw new ScriptException("bad script", new ParseException("error", 0), + org.elasticsearch.common.collect.List.of(), "", "lang", null); + }, org.elasticsearch.common.collect.Map.of()); + Script script = new Script(ScriptType.INLINE, "lang", "foo", org.elasticsearch.common.collect.Map.of()); + ScriptException e = expectThrows(ScriptException.class, () -> + new ConditionalProcessor(null, null, script, scriptService, null)); + assertThat(e.getMessage(), equalTo("bad script")); + } + + public void testRuntimeCompileError() { + AtomicBoolean fail = new AtomicBoolean(false); + Map storedScripts = new HashMap<>(); + storedScripts.put("foo", new StoredScriptSource("lang", "", org.elasticsearch.common.collect.Map.of())); + ScriptService scriptService = MockScriptService.singleContext(IngestConditionalScript.CONTEXT, code -> { + if (fail.get()) { + throw new ScriptException("bad script", new ParseException("error", 0), + org.elasticsearch.common.collect.List.of(), "", "lang", null); + } else { + return params -> new IngestConditionalScript(params) { + @Override + public boolean execute(Map ctx) { + return false; + } + }; + } + }, storedScripts); + Script script = new Script(ScriptType.STORED, null, "foo", org.elasticsearch.common.collect.Map.of()); + ConditionalProcessor processor = new ConditionalProcessor(null, null, script, scriptService, null); + fail.set(true); + // must change the script source or the cached version will be used + storedScripts.put("foo", new StoredScriptSource("lang", "changed", org.elasticsearch.common.collect.Map.of())); + IngestDocument ingestDoc = new IngestDocument(org.elasticsearch.common.collect.Map.of(), + org.elasticsearch.common.collect.Map.of()); + processor.execute(ingestDoc, (doc, e) -> { + assertThat(e.getMessage(), equalTo("bad script")); + }); + } + + public void testRuntimeError() { + ScriptService scriptService = MockScriptService.singleContext(IngestConditionalScript.CONTEXT, + code -> params -> new IngestConditionalScript(params) { + @Override + public boolean execute(Map ctx) { + throw new IllegalArgumentException("runtime problem"); + } + }, org.elasticsearch.common.collect.Map.of()); + Script script = new Script(ScriptType.INLINE, "lang", "foo", org.elasticsearch.common.collect.Map.of()); + ConditionalProcessor processor = new ConditionalProcessor(null, null, script, scriptService, null); + IngestDocument ingestDoc = new IngestDocument(org.elasticsearch.common.collect.Map.of(), + org.elasticsearch.common.collect.Map.of()); + processor.execute(ingestDoc, (doc, e) -> { + assertThat(e.getMessage(), equalTo("runtime problem")); + }); + } + private static void assertMutatingCtxThrows(Consumer> mutation) throws Exception { String scriptName = "conditionalScript"; CompletableFuture expectedException = new CompletableFuture<>(); diff --git a/server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java b/server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java index 820f7e5de1e..5f137bee996 100644 --- a/server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java +++ b/server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java @@ -96,7 +96,7 @@ public class ScriptServiceTests extends ESTestCase { } @Override - StoredScriptSource getScriptFromClusterState(String id) { + protected StoredScriptSource getScriptFromClusterState(String id) { //mock the script that gets retrieved from an index return new StoredScriptSource("test", "1+1", Collections.emptyMap()); } diff --git a/test/framework/src/main/java/org/elasticsearch/script/MockScriptService.java b/test/framework/src/main/java/org/elasticsearch/script/MockScriptService.java index 9089683bbfe..70dfd05361d 100644 --- a/test/framework/src/main/java/org/elasticsearch/script/MockScriptService.java +++ b/test/framework/src/main/java/org/elasticsearch/script/MockScriptService.java @@ -24,6 +24,8 @@ import org.elasticsearch.node.MockNode; import org.elasticsearch.plugins.Plugin; import java.util.Map; +import java.util.Set; +import java.util.function.Function; public class MockScriptService extends ScriptService { /** @@ -39,4 +41,32 @@ public class MockScriptService extends ScriptService { boolean compilationLimitsEnabled() { return false; } + + public static MockScriptService singleContext(ScriptContext context, Function compile, + Map storedLookup) { + ScriptEngine engine = new ScriptEngine() { + @Override + public String getType() { + return "lang"; + } + + @Override + public FactoryType compile(String name, String code, ScriptContext context, + Map params) { + return context.factoryClazz.cast(compile.apply(code)); + } + + @Override + public Set> getSupportedContexts() { + return org.elasticsearch.common.collect.Set.of(context); + } + }; + return new MockScriptService(Settings.EMPTY, org.elasticsearch.common.collect.Map.of("lang", engine), + org.elasticsearch.common.collect.Map.of(context.name, context)) { + @Override + protected StoredScriptSource getScriptFromClusterState(String id) { + return storedLookup.get(id); + } + }; + } }