Watcher: Remove deprecated GeneralScriptException (elastic/elasticsearch#4012)

Also removes an unused exception method in the Exceptions class.

Original commit: elastic/x-pack-elasticsearch@72dea031bb

elasticsearch/src/main/java/org/elasticsearch/xpack/watcher/condition/ScriptCondition.java

@@ -21,7 +21,7 @@ import java.io.IOException;
 import java.util.Collections;
 import java.util.Map;
 
-import static org.elasticsearch.xpack.watcher.support.Exceptions.invalidScript;
+import static org.elasticsearch.xpack.watcher.support.Exceptions.illegalState;
 
 /**
  * This class executes a script against the ctx payload and returns a boolean
@@ -84,7 +84,7 @@ public final class ScriptCondition extends Condition {
         if (value instanceof Boolean) {
             return (Boolean) value ? MET : UNMET;
         }
-        throw invalidScript("condition [{}] must return a boolean value (true|false) but instead returned [{}]", type(), ctx.watch().id(),
+        throw illegalState("condition [{}] must return a boolean value (true|false) but instead returned [{}]", type(), ctx.watch().id(),
                 script, value);
     }
 

elasticsearch/src/main/java/org/elasticsearch/xpack/watcher/support/Exceptions.java

@@ -5,9 +5,6 @@
  */
 package org.elasticsearch.xpack.watcher.support;
 
-import org.elasticsearch.common.settings.SettingsException;
-import org.elasticsearch.script.GeneralScriptException;
-
 import java.io.IOException;
 
 import static org.elasticsearch.common.logging.LoggerMessageFormat.format;
@@ -40,15 +37,4 @@ public class Exceptions {
     public static IOException ioException(String msg, Throwable cause, Object... args) {
         return new IOException(format(msg, args), cause);
     }
-
-
-    //todo remove once GeneralScriptException supports varargs
-    public static GeneralScriptException invalidScript(String msg, Object... args) {
-        throw new GeneralScriptException(format(msg, args));
-    }
-
-    //todo remove once SettingsException supports varargs
-    public static SettingsException invalidSettings(String msg, Object... args) {
-        throw new SettingsException(format(msg, args));
-    }
 }

elasticsearch/src/main/java/org/elasticsearch/xpack/watcher/transform/script/ExecutableScriptTransform.java

@@ -22,7 +22,6 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 
-import static org.elasticsearch.xpack.watcher.support.Exceptions.invalidScript;
 import static org.elasticsearch.xpack.watcher.support.Variables.createCtxModel;
 import static org.elasticsearch.xpack.watcher.transform.script.ScriptTransform.TYPE;
 
@@ -35,11 +34,7 @@ public class ExecutableScriptTransform extends ExecutableTransform<ScriptTransfo
         super(transform, logger);
         this.scriptService = scriptService;
         Script script = transform.getScript();
-        try {
+        compiledScript = scriptService.compile(script, Watcher.SCRIPT_CONTEXT, Collections.emptyMap());
-            compiledScript = scriptService.compile(script, Watcher.SCRIPT_CONTEXT, Collections.emptyMap());
-        } catch (Exception e) {
-            throw invalidScript("failed to compile script [{}]", e, script, e);
-        }
     }
 
     @Override

elasticsearch/src/test/java/org/elasticsearch/xpack/watcher/condition/ScriptConditionTests.java

@@ -15,7 +15,6 @@ import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.common.xcontent.support.XContentMapValues;
 import org.elasticsearch.env.Environment;
-import org.elasticsearch.script.GeneralScriptException;
 import org.elasticsearch.script.MockScriptEngine;
 import org.elasticsearch.script.Script;
 import org.elasticsearch.script.ScriptContext;
@@ -24,8 +23,8 @@ import org.elasticsearch.script.ScriptEngineRegistry;
 import org.elasticsearch.script.ScriptEngineService;
 import org.elasticsearch.script.ScriptException;
 import org.elasticsearch.script.ScriptService;
-import org.elasticsearch.script.ScriptType;
 import org.elasticsearch.script.ScriptSettings;
+import org.elasticsearch.script.ScriptType;
 import org.elasticsearch.search.internal.InternalSearchResponse;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.xpack.watcher.execution.WatchExecutionContext;
@@ -192,7 +191,7 @@ public class ScriptConditionTests extends ESTestCase {
         ScriptCondition condition = new ScriptCondition(new Script("return new Object()"), scriptService);
         SearchResponse response = new SearchResponse(InternalSearchResponse.empty(), "", 3, 3, 500L, new ShardSearchFailure[0]);
         WatchExecutionContext ctx = mockExecutionContext("_name", new Payload.XContent(response));
-        Exception exception = expectThrows(GeneralScriptException.class, () -> condition.execute(ctx));
+        Exception exception = expectThrows(IllegalStateException.class, () -> condition.execute(ctx));
         assertThat(exception.getMessage(),
                 containsString("condition [script] must return a boolean value (true|false) but instead returned [_name]"));
     }

elasticsearch/src/test/java/org/elasticsearch/xpack/watcher/transform/script/ScriptTransformTests.java

@@ -12,8 +12,8 @@ import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.common.xcontent.json.JsonXContent;
 import org.elasticsearch.script.CompiledScript;
 import org.elasticsearch.script.ExecutableScript;
-import org.elasticsearch.script.GeneralScriptException;
 import org.elasticsearch.script.Script;
+import org.elasticsearch.script.ScriptException;
 import org.elasticsearch.script.ScriptService;
 import org.elasticsearch.script.ScriptType;
 import org.elasticsearch.test.ESTestCase;
@@ -44,6 +44,8 @@ import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasEntry;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.notNullValue;
+import static org.mockito.Matchers.anyObject;
+import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -60,7 +62,7 @@ public class ScriptTransformTests extends ESTestCase {
         tp.shutdownNow();
     }
 
-    public void testExecute_MapValue() throws Exception {
+    public void testExecuteMapValue() throws Exception {
         ScriptService service = mock(ScriptService.class);
         ScriptType type = randomFrom(ScriptType.values());
         Map<String, Object> params = Collections.emptyMap();
@@ -168,35 +170,24 @@ public class ScriptTransformTests extends ESTestCase {
     }
 
     public void testScriptConditionParserBadScript() throws Exception {
-        ScriptTransformFactory transformFactory = new ScriptTransformFactory(Settings.builder().build(), createScriptService(tp));
+        ScriptService scriptService = mock(ScriptService.class);
-        ScriptType scriptType = randomFrom(ScriptType.values());
+        String errorMessage = "expected error message";
-        String script;
+        ScriptException scriptException = new ScriptException(errorMessage, new RuntimeException("foo"),
-        switch (scriptType) {
+                Collections.emptyList(), "whatever", "whatever");
-            case STORED:
+        when(scriptService.compile(anyObject(), eq(Watcher.SCRIPT_CONTEXT), anyObject())).thenThrow(scriptException);
-            case FILE:
+
-                script = "nonExisting_script";
+        ScriptTransformFactory transformFactory = new ScriptTransformFactory(Settings.builder().build(), scriptService);
-                break;
-            case INLINE:
-            default:
-                script = "foo = = 1";
-        }
 
         XContentBuilder builder = jsonBuilder().startObject()
-                .field(scriptTypeField(scriptType), script)
+                .field(scriptTypeField(randomFrom(ScriptType.values())), "whatever")
-                .field("lang", "groovy")
                 .startObject("params").field("key", "value").endObject()
                 .endObject();
 
         XContentParser parser = XContentFactory.xContent(builder.bytes()).createParser(builder.bytes());
         parser.nextToken();
         ScriptTransform scriptTransform = transformFactory.parseTransform("_watch", parser, false);
-        try {
+        Exception e = expectThrows(ScriptException.class, () -> transformFactory.createExecutable(scriptTransform));
-            transformFactory.createExecutable(scriptTransform);
+        assertThat(e.getMessage(), containsString(errorMessage));
-            fail("expected a transform validation exception trying to create an executable with a bad or missing script");
-        } catch (GeneralScriptException e) {
-            // I don't think this is what this test intended to check!
-            assertThat(e.getMessage(), containsString("script_lang not supported [groovy]"));
-        }
     }
 
     public void testScriptConditionParserBadLang() throws Exception {
@@ -213,12 +204,8 @@ public class ScriptTransformTests extends ESTestCase {
         XContentParser parser = XContentFactory.xContent(builder.bytes()).createParser(builder.bytes());
         parser.nextToken();
         ScriptTransform scriptCondition = transformFactory.parseTransform("_watch", parser, false);
-        try {
+        Exception e = expectThrows(IllegalArgumentException.class, () -> transformFactory.createExecutable(scriptCondition));
-            transformFactory.createExecutable(scriptCondition);
+        assertThat(e.getMessage(), containsString("script_lang not supported [not_a_valid_lang]"));
-            fail("expected a transform validation exception trying to create an executable with an invalid language");
-        } catch (GeneralScriptException e) {
-            assertThat(e.getMessage(), containsString("script_lang not supported [not_a_valid_lang]"));
-        }
     }
 
     static String scriptTypeField(ScriptType type) {