Eagerly compile condition script at processor creation (#58882)

Ingest script processors were changed to eagerly compile their scripts
when the ingest pipeline is saved, but conditional scripts were missed.
This commit adds eager compilation to ingest conditional scripts, which
will help surface errors before runtime, as well as adds tests for each
case we might encounter between inline and stored script compilation
failures.

closes #58864

server/src/main/java/org/elasticsearch/ingest/ConditionalProcessor.java

@@ -23,7 +23,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;
@@ -40,6 +42,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 = DeprecationLogger.getLogger(DynamicMap.class);
@@ -53,12 +57,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);
@@ -72,6 +75,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
@@ -108,8 +123,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)));
     }
 

server/src/main/java/org/elasticsearch/script/ScriptService.java

@@ -443,7 +443,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) {

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,59 @@ 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), List.of(), "", "lang", null);
+        }, Map.of());
+        Script script = new Script(ScriptType.INLINE, "lang", "foo", 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<String, StoredScriptSource> storedScripts = new HashMap<>();
+        storedScripts.put("foo", new StoredScriptSource("lang", "", Map.of()));
+        ScriptService scriptService = MockScriptService.singleContext(IngestConditionalScript.CONTEXT, code -> {
+            if (fail.get()) {
+                throw new ScriptException("bad script", new ParseException("error", 0), List.of(), "", "lang", null);
+            } else {
+                return params -> new IngestConditionalScript(params) {
+                    @Override
+                    public boolean execute(Map<String, Object> ctx) {
+                        return false;
+                    }
+                };
+            }
+        }, storedScripts);
+        Script script = new Script(ScriptType.STORED, null, "foo", Map.of());
+        var 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", Map.of()));
+        IngestDocument ingestDoc = new IngestDocument(Map.of(), 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<String, Object> ctx) {
+                    throw new IllegalArgumentException("runtime problem");
+                }
+            }, Map.of());
+        Script script = new Script(ScriptType.INLINE, "lang", "foo", Map.of());
+        var processor = new ConditionalProcessor(null, null, script, scriptService, null);
+        IngestDocument ingestDoc = new IngestDocument(Map.of(), Map.of());
+        processor.execute(ingestDoc, (doc, e) -> {
+            assertThat(e.getMessage(), equalTo("runtime problem"));
+        });
+    }
+
     private static void assertMutatingCtxThrows(Consumer<Map<String, Object>> mutation) throws Exception {
         String scriptName = "conditionalScript";
         CompletableFuture<Exception> expectedException = new CompletableFuture<>();

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

@@ -97,7 +97,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());
             }

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,31 @@ public class MockScriptService extends ScriptService {
     boolean compilationLimitsEnabled() {
         return false;
     }
+
+    public static <T> MockScriptService singleContext(ScriptContext<T> context, Function<String, T> compile,
+                                                      Map<String, StoredScriptSource> storedLookup) {
+        ScriptEngine engine = new ScriptEngine() {
+            @Override
+            public String getType() {
+                return "lang";
+            }
+
+            @Override
+            public <FactoryType> FactoryType compile(String name, String code, ScriptContext<FactoryType> context,
+                                                     Map<String, String> params) {
+                return context.factoryClazz.cast(compile.apply(code));
+            }
+
+            @Override
+            public Set<ScriptContext<?>> getSupportedContexts() {
+                return Set.of(context);
+            }
+        };
+        return new MockScriptService(Settings.EMPTY, Map.of("lang", engine), Map.of(context.name, context)) {
+            @Override
+            protected StoredScriptSource getScriptFromClusterState(String id) {
+                return storedLookup.get(id);
+            }
+        };
+    }
 }