AbstractScriptModuleHandler: Recompile scripts on dependency change (#4922)

* AbstractScriptModuleHandler: Recompile scripts on dependency change

When a script's dependency changes, it should recompile the compiled script similarly to resetting the engine for uncompiled scripts.
Fixing this behaviour fixes an issue where compiled scripts stopped working after a dependency changed.

This also simplifies the code a bit.

Signed-off-by: Florian Hotze <dev@florianhotze.com>

Author: florian-h05

bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/factory/ScriptModuleHandlerFactory.java

@@ -17,6 +17,8 @@
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 
+import javax.script.ScriptException;
+
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
 import org.openhab.core.automation.Action;
@@ -100,6 +102,11 @@ public void onDependencyChange(String engineIdentifier) {
         if (handler != null) {
             logger.debug("Resetting script engine for script {}", engineIdentifier);
             handler.resetScriptEngine();
+            try {
+                handler.compile();
+            } catch (ScriptException e) {
+                logger.error("Failed to recompile script for rule {}: {}", handler.getRuleUID(), e.getMessage());
+            }
         }
     }
 }

bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/handler/AbstractScriptModuleHandler.java

@@ -93,15 +93,10 @@ private static String getValidConfigParameter(String parameter, Configuration co
      * {@link Compilable}.
      */
     protected void compileScript() throws ScriptException {
-        if (compiledScript.isPresent()) {
-            return;
-        }
-        if (!scriptEngineManager.isSupported(this.type)) {
-            logger.debug(
-                    "ScriptEngine for language '{}' could not be found, skipping compilation of script for identifier: {}",
-                    type, engineIdentifier);
+        if (compiledScript.isPresent() || script.isEmpty()) {
             return;
         }
+
         Optional<ScriptEngine> engine = getScriptEngine();
         if (engine.isPresent()) {
             ScriptEngine scriptEngine = engine.get();
@@ -119,11 +114,20 @@ public void dispose() {
 
     /**
      * Reset the script engine to force a script reload
-     *
      */
     public synchronized void resetScriptEngine() {
         scriptEngineManager.removeEngine(engineIdentifier);
         scriptEngine = Optional.empty();
+        compiledScript = Optional.empty();
+    }
+
+    /**
+     * Gets the unique identifier of the rule this module handler is used for.
+     * 
+     * @return the UID of the rule
+     */
+    public String getRuleUID() {
+        return ruleUID;
     }
 
     /**
@@ -135,10 +139,20 @@ public String getEngineIdentifier() {
         return engineIdentifier;
     }
 
+    /**
+     * Get the script engine instance used by this module handler.
+     * 
+     * @return the script engine instance if available, otherwise Optional.empty()
+     */
     protected Optional<ScriptEngine> getScriptEngine() {
         return scriptEngine.isPresent() ? scriptEngine : createScriptEngine();
     }
 
+    /**
+     * Creates a new script engine for the type defined in the module configuration.
+     * 
+     * @return the script engine if available, otherwise Optional.empty()
+     */
     private Optional<ScriptEngine> createScriptEngine() {
         ScriptEngineContainer container = scriptEngineManager.createScriptEngine(type, engineIdentifier);
 
@@ -203,13 +217,15 @@ protected void resetExecutionContext(ScriptEngine engine, Map<String, ?> context
     }
 
     /**
-     * Evaluates the passed script with the ScriptEngine.
+     * Evaluates the script with the given script engine.
      *
      * @param engine the script engine that is used
-     * @param script the script to evaluate
      * @return the value returned from the execution of the script
      */
-    protected @Nullable Object eval(ScriptEngine engine, String script) {
+    protected @Nullable Object eval(ScriptEngine engine) {
+        if (script.isEmpty()) {
+            return null;
+        }
         try {
             if (compiledScript.isPresent()) {
                 logger.debug("Executing pre-compiled script of rule with UID '{}'", ruleUID);

bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/handler/ScriptActionHandler.java

@@ -91,7 +91,7 @@ public void compile() throws ScriptException {
             }
             try {
                 setExecutionContext(scriptEngine, context);
-                Object result = eval(scriptEngine, script);
+                Object result = eval(scriptEngine);
                 resultMap.put("result", result);
                 resetExecutionContext(scriptEngine, context);
             } finally { // Make sure that Lock is unlocked regardless of an exception being thrown or not to avoid

bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/handler/ScriptConditionHandler.java

@@ -75,7 +75,7 @@ public boolean isSatisfied(final Map<String, Object> context) {
             }
             try {
                 setExecutionContext(scriptEngine, context);
-                Object returnVal = eval(scriptEngine, script);
+                Object returnVal = eval(scriptEngine);
                 if (returnVal instanceof Boolean boolean1) {
                     result = boolean1;
                 } else {