[jsscripting] Wrapper: Always enable for actions, configurable for conditions

bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/GraalJSScriptEngineConfiguration.java

@@ -30,17 +30,17 @@ public class GraalJSScriptEngineConfiguration {
 
     private static final String CFG_INJECTION_ENABLED = "injectionEnabledV2";
     private static final String CFG_INJECTION_CACHING_ENABLED = "injectionCachingEnabled";
-    private static final String CFG_WRAPPER_ENABLED = "wrapperEnabled";
+    private static final String CFG_SCRIPT_CONDITION_WRAPPER_ENABLED = "scriptConditionWrapperEnabled";
     private static final String CFG_EVENT_CONVERSION_ENABLED = "eventConversionEnabled";
     private static final String CFG_DEPENDENCY_TRACKING_ENABLED = "dependencyTrackingEnabled";
 
-    private static final int INJECTION_ENABLED_FOR_UI_BASED_SCRIPTS_ONLY = 1;
-    private static final int INJECTION_ENABLED_FOR_UI_BASED_SCRIPTS_AND_TRANSFORMATIONS = 2;
+    private static final int INJECTION_ENABLED_FOR_SCRIPT_MODULES_ONLY = 1;
+    private static final int INJECTION_ENABLED_FOR_SCRIPT_MODULES_AND_TRANSFORMATIONS = 2;
     private static final int INJECTION_ENABLED_FOR_ALL_SCRIPTS = 3;
 
     private int injectionEnabled = INJECTION_ENABLED_FOR_ALL_SCRIPTS;
     private boolean injectionCachingEnabled = true;
-    private boolean wrapperEnabled = true;
+    private boolean scriptConditionWrapperEnabled = false;
     private boolean eventConversionEnabled = true;
     private boolean dependencyTrackingEnabled = true;
 
@@ -59,15 +59,15 @@ public GraalJSScriptEngineConfiguration(Map<String, ?> config) {
      * @param config configuration parameters to apply to JavaScript
      */
     void modified(Map<String, ?> config) {
-        boolean oldInjectionEnabledForUiBasedScript = isInjectionEnabledForUiBasedScript();
+        boolean oldInjectionEnabledForUiBasedScript = isInjectionEnabledForScriptModules();
         boolean oldDependencyTrackingEnabled = dependencyTrackingEnabled;
-        boolean oldWrapperEnabled = wrapperEnabled;
+        boolean oldScriptConditionWrapperEnabled = scriptConditionWrapperEnabled;
         boolean oldEventConversionEnabled = eventConversionEnabled;
 
         this.update(config);
 
-        if (oldInjectionEnabledForUiBasedScript != isInjectionEnabledForUiBasedScript()
-                && !isInjectionEnabledForUiBasedScript() && isEventConversionEnabled()) {
+        if (oldInjectionEnabledForUiBasedScript != isInjectionEnabledForScriptModules()
+                && !isInjectionEnabledForScriptModules() && isEventConversionEnabled()) {
             logger.warn(
                     "Injection disabled for UI-based scripts, but event conversion is enabled. Event conversion will not work.");
         }
@@ -76,13 +76,13 @@ void modified(Map<String, ?> config) {
                     "{} dependency tracking for JavaScript Scripting. Please resave your scripts to apply this change.",
                     dependencyTrackingEnabled ? "Enabled" : "Disabled");
         }
-        if (oldWrapperEnabled != wrapperEnabled) {
+        if (oldScriptConditionWrapperEnabled != scriptConditionWrapperEnabled) {
             logger.info(
-                    "{} wrapper for JavaScript Scripting. Please resave your UI-based scripts to apply this change.",
-                    wrapperEnabled ? "Enabled" : "Disabled");
+                    "{} script condition wrapper for JavaScript Scripting. Please resave your rules with JavaScript script conditions to apply this change.",
+                    scriptConditionWrapperEnabled ? "Enabled" : "Disabled");
         }
         if (oldEventConversionEnabled != eventConversionEnabled) {
-            if (eventConversionEnabled && !isInjectionEnabledForUiBasedScript()) {
+            if (eventConversionEnabled && !isInjectionEnabledForScriptModules()) {
                 logger.warn(
                         "Enabled event conversion for UI-based scripts, but auto-injection is disabled. Event conversion will not work.");
             }
@@ -102,24 +102,42 @@ private void update(Map<String, ?> config) {
         logger.debug("JavaScript Script Engine Configuration: {}", config);
 
         injectionEnabled = ConfigParser.valueAsOrElse(config.get(CFG_INJECTION_ENABLED), Integer.class,
-                INJECTION_ENABLED_FOR_UI_BASED_SCRIPTS_ONLY);
+                INJECTION_ENABLED_FOR_SCRIPT_MODULES_ONLY);
         injectionCachingEnabled = ConfigParser.valueAsOrElse(config.get(CFG_INJECTION_CACHING_ENABLED), Boolean.class,
                 true);
-        wrapperEnabled = ConfigParser.valueAsOrElse(config.get(CFG_WRAPPER_ENABLED), Boolean.class, true);
+        scriptConditionWrapperEnabled = ConfigParser.valueAsOrElse(config.get(CFG_SCRIPT_CONDITION_WRAPPER_ENABLED),
+                Boolean.class, false);
         eventConversionEnabled = ConfigParser.valueAsOrElse(config.get(CFG_EVENT_CONVERSION_ENABLED), Boolean.class,
                 true);
         dependencyTrackingEnabled = ConfigParser.valueAsOrElse(config.get(CFG_DEPENDENCY_TRACKING_ENABLED),
                 Boolean.class, true);
     }
 
-    public boolean isInjectionEnabledForUiBasedScript() {
-        return injectionEnabled >= INJECTION_ENABLED_FOR_UI_BASED_SCRIPTS_ONLY;
+    /**
+     * Whether injection is enabled for script modules, i.e. scripts executed by an implementation of
+     * {@link org.openhab.core.automation.module.script.internal.handler.AbstractScriptModuleHandler}.
+     * 
+     * @return whether injection is enabled for script modules
+     */
+    public boolean isInjectionEnabledForScriptModules() {
+        return injectionEnabled >= INJECTION_ENABLED_FOR_SCRIPT_MODULES_ONLY;
     }
 
+    /**
+     * Whether injection is enabled for transformations, i.e. scripts executed by the
+     * {@link org.openhab.core.automation.module.script.ScriptTransformationService}.
+     * 
+     * @return whether injection is enabled for transformations
+     */
     public boolean isInjectionEnabledForTransformations() {
-        return injectionEnabled >= INJECTION_ENABLED_FOR_UI_BASED_SCRIPTS_AND_TRANSFORMATIONS;
+        return injectionEnabled >= INJECTION_ENABLED_FOR_SCRIPT_MODULES_AND_TRANSFORMATIONS;
     }
 
+    /**
+     * Whether injection is enabled for all scripts, i.e. script modules, transformations and script files.
+     * 
+     * @return whether injection is enabled for all scripts
+     */
     public boolean isInjectionEnabledForAllScripts() {
         return injectionEnabled == INJECTION_ENABLED_FOR_ALL_SCRIPTS;
     }
@@ -128,8 +146,14 @@ public boolean isInjectionCachingEnabled() {
         return injectionCachingEnabled;
     }
 
-    public boolean isWrapperEnabled() {
-        return wrapperEnabled;
+    /**
+     * Whether the wrapper is enabled for script conditions (see
+     * {@link org.openhab.core.automation.module.script.internal.handler.ScriptConditionHandler}).
+     * 
+     * @return whether the wrapper is enabled for script conditions
+     */
+    public boolean isScriptConditionWrapperEnabled() {
+        return scriptConditionWrapperEnabled;
     }
 
     public boolean isEventConversionEnabled() {

bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java

@@ -30,6 +30,7 @@
 import java.time.Duration;
 import java.time.Instant;
 import java.time.ZonedDateTime;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
@@ -38,6 +39,7 @@
 import java.util.concurrent.locks.ReentrantLock;
 import java.util.function.Consumer;
 import java.util.function.Function;
+import java.util.regex.Pattern;
 
 import javax.script.ScriptContext;
 import javax.script.ScriptException;
@@ -56,6 +58,9 @@
 import org.openhab.automation.jsscripting.internal.scriptengine.InvocationInterceptingScriptEngineWithInvocableAndCompilableAndAutoCloseable;
 import org.openhab.automation.jsscripting.internal.scriptengine.helper.LifecycleTracker;
 import org.openhab.core.automation.module.script.ScriptExtensionAccessor;
+import org.openhab.core.automation.module.script.internal.handler.AbstractScriptModuleHandler;
+import org.openhab.core.automation.module.script.internal.handler.ScriptActionHandler;
+import org.openhab.core.automation.module.script.internal.handler.ScriptConditionHandler;
 import org.openhab.core.items.Item;
 import org.openhab.core.library.types.QuantityType;
 import org.slf4j.Logger;
@@ -101,6 +106,8 @@ public class OpenhabGraalJSScriptEngine
     }
     private static final String OPENHAB_JS_INJECTION_CODE = "Object.assign(this, require('openhab'));";
     private static final String EVENT_CONVERSION_CODE = "this.event = (typeof this.rules?._getTriggeredData === 'function') ? rules._getTriggeredData(ctx, true) : this.event";
+    private static final Pattern USE_WRAPPER_DIRECTIVE = Pattern
+            .compile("^\\s*([\"'])use wrapper(?:=(?<enabled>true|false))?\\1;?\\s*$");
 
     private static final String REQUIRE_WRAPPER_NAME = "__wraprequire__";
     /** Shared Polyglot {@link Engine} across all instances of {@link OpenhabGraalJSScriptEngine} */
@@ -304,13 +311,26 @@ protected void beforeInvocation() {
 
         initialized = true;
 
+        if (logger.isDebugEnabled()) {
+            logger.debug(
+                    "Engine '{}': isScriptFile(): {}, isScriptModule(): {}, isScriptAction(): {}, isScriptCondition(): {}, isTransformation(): {}",
+                    engineIdentifier, isScriptFile(), isScriptModule(), isScriptAction(), isScriptCondition(),
+                    isTransformation());
+        }
+
+        if (!isScriptFile() && !isScriptModule() && !isTransformation()) {
+            logger.warn(
+                    "Unknown script environment detected for engine '{}': Neither script file, script module nor transformation.",
+                    engineIdentifier);
+        }
+
         try {
             logger.debug("Evaluating cached global script for engine '{}' ...", engineIdentifier);
             delegate.getPolyglotContext().eval(GLOBAL_SOURCE);
 
             if (configuration.isInjectionEnabledForAllScripts()
-                    || (isUiBasedScript() && configuration.isInjectionEnabledForUiBasedScript())
-                    || (isTransformationScript() && configuration.isInjectionEnabledForTransformations())) {
+                    || (isScriptModule() && configuration.isInjectionEnabledForScriptModules())
+                    || (isTransformation() && configuration.isInjectionEnabledForTransformations())) {
                 if (configuration.isInjectionCachingEnabled()) {
                     logger.debug("Evaluating cached openhab-js injection for engine '{}' ...", engineIdentifier);
                     delegate.getPolyglotContext().eval(OPENHAB_JS_SOURCE);
@@ -328,7 +348,7 @@ protected void beforeInvocation() {
 
     @Override
     protected String onScript(String script) {
-        if (!isUiBasedScript()) {
+        if (!isScriptModule()) {
             return super.onScript(script);
         }
 
@@ -337,7 +357,31 @@ protected String onScript(String script) {
             logger.debug("Injecting event conversion code into script for engine '{}'.", engineIdentifier);
             newScript = EVENT_CONVERSION_CODE + System.lineSeparator() + newScript;
         }
-        if (configuration.isWrapperEnabled()) {
+
+        // keep this extendable for more directives by checking the first n lines (n = number of directives)
+        // up to two directives: "use strict" (handled by Graal) and "use wrapper"
+        List<String> header = script.lines().limit(2).toList();
+        boolean useWrapper = isScriptAction()
+                || (isScriptCondition() && configuration.isScriptConditionWrapperEnabled());
+        for (String line : header) {
+            var matcher = USE_WRAPPER_DIRECTIVE.matcher(line);
+            if (!matcher.matches()) {
+                continue;
+            }
+            var enabled = matcher.group("enabled");
+            if (enabled == null || enabled.isBlank()) {
+                useWrapper = true;
+            } else if ("false".equals(enabled)) {
+                useWrapper = false;
+            } else if ("true".equals(enabled)) {
+                useWrapper = true;
+            } else {
+                logger.warn("Invalid value '{}' for 'use wrapper' directive in script for engine '{}'.", enabled,
+                        engineIdentifier);
+            }
+        }
+
+        if (useWrapper) {
             logger.debug("Wrapping script for engine '{}' ...", engineIdentifier);
             newScript = "(function() {" + System.lineSeparator() + newScript + System.lineSeparator() + "})()";
         }
@@ -381,27 +425,73 @@ public void close() throws Exception {
     }
 
     /**
-     * Tests if the current script is a UI-based script, i.e. it is neither loaded from a file nor a transformation.
+     * Tests if the script is a script file, i.e. it is loaded from a JavaScript file.
      * 
-     * @return true if the script is UI-based, false otherwise
+     * @return true if the script is loaded from a JavaScript file, false otherwise
      */
-    private boolean isUiBasedScript() {
+    private boolean isScriptFile() {
         ScriptContext ctx = delegate.getContext();
         if (ctx == null) {
             logger.warn("Failed to retrieve script context from engine '{}'.", engineIdentifier);
             return false;
         }
-        return ctx.getAttribute("javax.script.filename") == null
-                && !engineIdentifier.startsWith(OPENHAB_TRANSFORMATION_SCRIPT);
+        return ctx.getAttribute("javax.script.filename") != null;
+    }
+
+    /**
+     * Get the module type id (if any) of the module executing the script.
+     * 
+     * @return the module type id (if any) of the module executing the script, or null if the script is not a module
+     */
+    private @Nullable String getModuleTypeId() {
+        ScriptContext ctx = delegate.getContext();
+        if (ctx == null) {
+            logger.warn("Failed to retrieve script context from engine '{}'.", engineIdentifier);
+            return null;
+        }
+
+        Object value = ctx.getAttribute(AbstractScriptModuleHandler.CONTEXT_KEY_MODULE_TYPE_ID);
+        if (value instanceof String str) {
+            return str;
+        }
+        return null;
+    }
+
+    /**
+     * Tests if the script is a script module, i.e. executed by an implementation of
+     * {@link AbstractScriptModuleHandler}.
+     * 
+     * @return true if the script is a script module, false otherwise
+     */
+    private boolean isScriptModule() {
+        String moduleTypeId = getModuleTypeId();
+        return moduleTypeId != null && moduleTypeId.startsWith("script.");
+    }
+
+    /**
+     * Tests if a script is a script action, i.e. executed by the ScriptActionHandler.
+     * 
+     * @return true if the script is a script action, false otherwise
+     */
+    private boolean isScriptAction() {
+        return ScriptActionHandler.TYPE_ID.equals(getModuleTypeId());
+    }
+
+    /**
+     * Tests if the script is a script condition, i.e. executed by the ScriptConditionHandler.
+     * 
+     * @return true if the script is a script condition, false otherwise
+     */
+    private boolean isScriptCondition() {
+        return ScriptConditionHandler.TYPE_ID.equals(getModuleTypeId());
     }
 
     /**
-     * Tests if the current script is a transformation script, i.e. it is created from the script transformation
-     * service.
+     * Tests if the script is a transformation script, i.e. created from the script transformation service.
      * 
-     * @return true if the script is a transformation script, false otherwise
+     * @return true if it is a transformation script, false otherwise
      */
-    private boolean isTransformationScript() {
+    private boolean isTransformation() {
         ScriptContext ctx = delegate.getContext();
         if (ctx == null) {
             logger.warn("Failed to retrieve script context from engine '{}'.", engineIdentifier);

bundles/org.openhab.automation.jsscripting/src/main/resources/OH-INF/config/config.xml

@@ -29,14 +29,14 @@
 			</options>
 			<default>3</default>
 		</parameter>
-		<parameter name="wrapperEnabled" type="boolean" required="true" groupName="environment">
-			<label>Wrap UI-based scripts in Self-Executing Function</label>
+		<parameter name="scriptConditionWrapperEnabled" type="boolean" required="true" groupName="environment">
+			<label>Wrap Script Conditions in Self-Executing Function</label>
 			<description><![CDATA[
-			Wrapping UI-based scripts in a self-executing function allows the use of the <code>let</code> and <code>const</code> variable declarations,
+			Wrapping script conditions in a self-executing function allows the use of the <code>let</code> and <code>const</code> variable declarations,
 			as well as the use of <code>function</code> and <code>class</code> declarations.<br>
-			With this option enabled, you can also use <code>return</code> statements in your scripts to abort execution at any point.
+			With this option enabled, you need to use <code>return</code> statements in your script condition to return true or false.
 			]]></description>
-			<default>true</default>
+			<default>false</default>
 			<advanced>true</advanced>
 		</parameter>
 		<parameter name="eventConversionEnabled" type="boolean" required="true" groupName="environment">

bundles/org.openhab.automation.jsscripting/src/main/resources/OH-INF/i18n/jsscripting.properties

@@ -21,5 +21,5 @@ automation.config.jsscripting.injectionEnabledV2.option.3 = Auto injection for a
 automation.config.jsscripting.injectionEnabledV2.option.2 = Auto injection for UI-based scripts and transformations
 automation.config.jsscripting.injectionEnabledV2.option.1 = Auto injection only for UI-based scripts (recommended)
 automation.config.jsscripting.injectionEnabledV2.option.0 = Disable auto-injection and import manually instead
-automation.config.jsscripting.wrapperEnabled.label = Wrap UI-based scripts in Self-Executing Function
-automation.config.jsscripting.wrapperEnabled.description = Wrapping UI-based scripts in a self-executing function allows the use of the <code>let</code> and <code>const</code> variable declarations, as well as the use of <code>function</code> and <code>class</code> declarations.<br> With this option enabled, you can also use <code>return</code> statements in your scripts to abort execution at any point.
+automation.config.jsscripting.scriptConditionWrapperEnabled.label = Wrap Script Conditions in Self-Executing Function
+automation.config.jsscripting.scriptConditionWrapperEnabled.description = Wrapping script conditions in a self-executing function allows the use of the <code>let</code> and <code>const</code> variable declarations, as well as the use of <code>function</code> and <code>class</code> declarations.<br> With this option enabled, you need to use <code>return</code> statements in your script condition to return true or false.