From 04279684722af67c7cf3f74fb3f0db0434d8d1e1 Mon Sep 17 00:00:00 2001 From: Florian Hotze Date: Sun, 5 Oct 2025 21:03:50 +0200 Subject: [PATCH 1/7] [jsscripting] Always enable wrapper for actions & Make it configurable for conditions with default disabled See https://community.openhab.org/t/rules-and-return-codes/166438 for discussion. Signed-off-by: Florian Hotze --- .../GraalJSScriptEngineConfiguration.java | 19 +++--- .../internal/OpenhabGraalJSScriptEngine.java | 68 +++++++++++++++---- .../main/resources/OH-INF/config/config.xml | 8 +-- .../OH-INF/i18n/jsscripting.properties | 4 +- 4 files changed, 71 insertions(+), 28 deletions(-) diff --git a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/GraalJSScriptEngineConfiguration.java b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/GraalJSScriptEngineConfiguration.java index f7c97907cb44e..59d5a12d2427b 100644 --- a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/GraalJSScriptEngineConfiguration.java +++ b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/GraalJSScriptEngineConfiguration.java @@ -30,7 +30,7 @@ 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"; @@ -40,7 +40,7 @@ public class GraalJSScriptEngineConfiguration { private int injectionEnabled = INJECTION_ENABLED_FOR_ALL_SCRIPTS; private boolean injectionCachingEnabled = true; - private boolean wrapperEnabled = true; + private boolean scriptConditionWrapperEnabled = true; private boolean eventConversionEnabled = true; private boolean dependencyTrackingEnabled = true; @@ -61,7 +61,7 @@ public GraalJSScriptEngineConfiguration(Map config) { void modified(Map config) { boolean oldInjectionEnabledForUiBasedScript = isInjectionEnabledForUiBasedScript(); boolean oldDependencyTrackingEnabled = dependencyTrackingEnabled; - boolean oldWrapperEnabled = wrapperEnabled; + boolean oldScriptConditionWrapperEnabled = scriptConditionWrapperEnabled; boolean oldEventConversionEnabled = eventConversionEnabled; this.update(config); @@ -76,10 +76,10 @@ void modified(Map 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()) { @@ -105,7 +105,8 @@ private void update(Map config) { INJECTION_ENABLED_FOR_UI_BASED_SCRIPTS_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, true); eventConversionEnabled = ConfigParser.valueAsOrElse(config.get(CFG_EVENT_CONVERSION_ENABLED), Boolean.class, true); dependencyTrackingEnabled = ConfigParser.valueAsOrElse(config.get(CFG_DEPENDENCY_TRACKING_ENABLED), @@ -128,8 +129,8 @@ public boolean isInjectionCachingEnabled() { return injectionCachingEnabled; } - public boolean isWrapperEnabled() { - return wrapperEnabled; + public boolean isScriptConditionWrapperEnabled() { + return scriptConditionWrapperEnabled; } public boolean isEventConversionEnabled() { diff --git a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java index 82b8529ec5c84..0e7bbe18f9c55 100644 --- a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java +++ b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java @@ -56,6 +56,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; @@ -304,13 +307,17 @@ protected void beforeInvocation() { initialized = true; + logger.debug( + "Engine '{}': isScriptFile(): {}, isScriptAction(): {}, isScriptCondition(): {}, isTransformation(): {}", + engineIdentifier, isScriptFile(), isScriptAction(), isScriptCondition(), isTransformation()); + try { logger.debug("Evaluating cached global script for engine '{}' ...", engineIdentifier); delegate.getPolyglotContext().eval(GLOBAL_SOURCE); if (configuration.isInjectionEnabledForAllScripts() - || (isUiBasedScript() && configuration.isInjectionEnabledForUiBasedScript()) - || (isTransformationScript() && configuration.isInjectionEnabledForTransformations())) { + || (!isScriptFile() && configuration.isInjectionEnabledForUiBasedScript()) + || (isTransformation() && configuration.isInjectionEnabledForTransformations())) { if (configuration.isInjectionCachingEnabled()) { logger.debug("Evaluating cached openhab-js injection for engine '{}' ...", engineIdentifier); delegate.getPolyglotContext().eval(OPENHAB_JS_SOURCE); @@ -328,7 +335,7 @@ protected void beforeInvocation() { @Override protected String onScript(String script) { - if (!isUiBasedScript()) { + if (isScriptFile() || isTransformation()) { return super.onScript(script); } @@ -337,7 +344,8 @@ 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()) { + + if (isScriptAction() || (isScriptCondition() && configuration.isScriptConditionWrapperEnabled())) { logger.debug("Wrapping script for engine '{}' ...", engineIdentifier); newScript = "(function() {" + System.lineSeparator() + newScript + System.lineSeparator() + "})()"; } @@ -381,27 +389,61 @@ 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 file. * - * @return true if the script is UI-based, false otherwise + * @return */ - 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 + */ + private @Nullable String getModuleTypeId() { + ScriptContext ctx = delegate.getContext(); + if (ctx == null) { + logger.warn("Failed to retrieve script context from engine '{}'.", engineIdentifier); + } + + Object value = ctx.getAttribute(AbstractScriptModuleHandler.CONTEXT_KEY_MODULE_TYPE_ID); + if (value instanceof String str) { + return str; + } + return null; + } + + /** + * Tests if a script is a script action, i.e. executed by the ScriptActionHandler. + * + * @return + */ + private boolean isScriptAction() { + return ScriptActionHandler.TYPE_ID.equals(getModuleTypeId()); + } + + /** + * Tests if the script is a script condition, i.e. executed by the ScriptConditionHandler. + * + * @return + */ + 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 */ - private boolean isTransformationScript() { + private boolean isTransformation() { ScriptContext ctx = delegate.getContext(); if (ctx == null) { logger.warn("Failed to retrieve script context from engine '{}'.", engineIdentifier); diff --git a/bundles/org.openhab.automation.jsscripting/src/main/resources/OH-INF/config/config.xml b/bundles/org.openhab.automation.jsscripting/src/main/resources/OH-INF/config/config.xml index d3a36ea5c1a07..e65cd2eadd2ef 100644 --- a/bundles/org.openhab.automation.jsscripting/src/main/resources/OH-INF/config/config.xml +++ b/bundles/org.openhab.automation.jsscripting/src/main/resources/OH-INF/config/config.xml @@ -29,12 +29,12 @@ 3 - - + + let and const variable declarations, + Wrapping script conditions in a self-executing function allows the use of the let and const variable declarations, as well as the use of function and class declarations.
- With this option enabled, you can also use return statements in your scripts to abort execution at any point. + With this option enabled, you need to use return statements in your script condition to return true or false. ]]> true true diff --git a/bundles/org.openhab.automation.jsscripting/src/main/resources/OH-INF/i18n/jsscripting.properties b/bundles/org.openhab.automation.jsscripting/src/main/resources/OH-INF/i18n/jsscripting.properties index e9ef61340ffaa..cf7fa51bd3916 100644 --- a/bundles/org.openhab.automation.jsscripting/src/main/resources/OH-INF/i18n/jsscripting.properties +++ b/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 let and const variable declarations, as well as the use of function and class declarations.
With this option enabled, you can also use return 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 let and const variable declarations, as well as the use of function and class declarations.
With this option enabled, you need to use return statements in your script condition to return true or false. From 1eb5bfa716bcd696c9c44281e2e509ed424863bd Mon Sep 17 00:00:00 2001 From: Florian Hotze Date: Sun, 5 Oct 2025 21:42:57 +0200 Subject: [PATCH 2/7] [jsscripting] Add "use wrapper" directive Signed-off-by: Florian Hotze --- .../internal/OpenhabGraalJSScriptEngine.java | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java index 0e7bbe18f9c55..5a9f1c6da64f5 100644 --- a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java +++ b/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; @@ -104,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(?:=(?true|false))?[\"'];?"); private static final String REQUIRE_WRAPPER_NAME = "__wraprequire__"; /** Shared Polyglot {@link Engine} across all instances of {@link OpenhabGraalJSScriptEngine} */ @@ -345,7 +349,28 @@ protected String onScript(String script) { newScript = EVENT_CONVERSION_CODE + System.lineSeparator() + newScript; } - if (isScriptAction() || (isScriptCondition() && configuration.isScriptConditionWrapperEnabled())) { + // keep this extendable for more directives by checking the first n lines (n = number or directives) + List header = script.lines().limit(1).toList(); + boolean useWrapper = 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 (isScriptAction() || (isScriptCondition() && useWrapper)) { logger.debug("Wrapping script for engine '{}' ...", engineIdentifier); newScript = "(function() {" + System.lineSeparator() + newScript + System.lineSeparator() + "})()"; } From c5a1a718cc8f09b04bba0cfd9c402713c09bbac0 Mon Sep 17 00:00:00 2001 From: Florian Hotze Date: Sun, 5 Oct 2025 21:50:16 +0200 Subject: [PATCH 3/7] [jsscripting] Disable script condition wrapper by default Signed-off-by: Florian Hotze --- .../jsscripting/internal/GraalJSScriptEngineConfiguration.java | 2 +- .../src/main/resources/OH-INF/config/config.xml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/GraalJSScriptEngineConfiguration.java b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/GraalJSScriptEngineConfiguration.java index 59d5a12d2427b..a178f4beaa8e1 100644 --- a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/GraalJSScriptEngineConfiguration.java +++ b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/GraalJSScriptEngineConfiguration.java @@ -40,7 +40,7 @@ public class GraalJSScriptEngineConfiguration { private int injectionEnabled = INJECTION_ENABLED_FOR_ALL_SCRIPTS; private boolean injectionCachingEnabled = true; - private boolean scriptConditionWrapperEnabled = true; + private boolean scriptConditionWrapperEnabled = false; private boolean eventConversionEnabled = true; private boolean dependencyTrackingEnabled = true; diff --git a/bundles/org.openhab.automation.jsscripting/src/main/resources/OH-INF/config/config.xml b/bundles/org.openhab.automation.jsscripting/src/main/resources/OH-INF/config/config.xml index e65cd2eadd2ef..e0611c223dd80 100644 --- a/bundles/org.openhab.automation.jsscripting/src/main/resources/OH-INF/config/config.xml +++ b/bundles/org.openhab.automation.jsscripting/src/main/resources/OH-INF/config/config.xml @@ -36,7 +36,7 @@ as well as the use of function and class declarations.
With this option enabled, you need to use return statements in your script condition to return true or false. ]]> - true + false true From c1a849f6ef2afe5698b8ab2925eca9fa7c3ff832 Mon Sep 17 00:00:00 2001 From: Florian Hotze Date: Mon, 6 Oct 2025 23:46:46 +0200 Subject: [PATCH 4/7] Address review Signed-off-by: Florian Hotze --- .../GraalJSScriptEngineConfiguration.java | 43 ++++++++++++++----- .../internal/OpenhabGraalJSScriptEngine.java | 29 +++++++------ 2 files changed, 49 insertions(+), 23 deletions(-) diff --git a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/GraalJSScriptEngineConfiguration.java b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/GraalJSScriptEngineConfiguration.java index a178f4beaa8e1..470184e9cffd7 100644 --- a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/GraalJSScriptEngineConfiguration.java +++ b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/GraalJSScriptEngineConfiguration.java @@ -34,8 +34,8 @@ public class GraalJSScriptEngineConfiguration { 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; @@ -59,15 +59,15 @@ public GraalJSScriptEngineConfiguration(Map config) { * @param config configuration parameters to apply to JavaScript */ void modified(Map config) { - boolean oldInjectionEnabledForUiBasedScript = isInjectionEnabledForUiBasedScript(); + boolean oldInjectionEnabledForUiBasedScript = isInjectionEnabledForScriptModules(); boolean oldDependencyTrackingEnabled = dependencyTrackingEnabled; 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."); } @@ -82,7 +82,7 @@ void modified(Map config) { 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,7 +102,7 @@ private void update(Map 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); scriptConditionWrapperEnabled = ConfigParser.valueAsOrElse(config.get(CFG_SCRIPT_CONDITION_WRAPPER_ENABLED), @@ -113,14 +113,31 @@ private void update(Map config) { 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; } @@ -129,6 +146,12 @@ public boolean isInjectionCachingEnabled() { return injectionCachingEnabled; } + /** + * 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; } diff --git a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java index 5a9f1c6da64f5..dc137ac25bfd4 100644 --- a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java +++ b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java @@ -107,7 +107,7 @@ 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(?:=(?true|false))?[\"'];?"); + .compile("^\\s*([\"'])use wrapper(?:=(?true|false))?\\1;?\\s*$"); private static final String REQUIRE_WRAPPER_NAME = "__wraprequire__"; /** Shared Polyglot {@link Engine} across all instances of {@link OpenhabGraalJSScriptEngine} */ @@ -311,16 +311,18 @@ protected void beforeInvocation() { initialized = true; - logger.debug( - "Engine '{}': isScriptFile(): {}, isScriptAction(): {}, isScriptCondition(): {}, isTransformation(): {}", - engineIdentifier, isScriptFile(), isScriptAction(), isScriptCondition(), isTransformation()); + if (logger.isDebugEnabled()) { + logger.debug( + "Engine '{}': isScriptFile(): {}, isScriptAction(): {}, isScriptCondition(): {}, isTransformation(): {}", + engineIdentifier, isScriptFile(), isScriptAction(), isScriptCondition(), isTransformation()); + } try { logger.debug("Evaluating cached global script for engine '{}' ...", engineIdentifier); delegate.getPolyglotContext().eval(GLOBAL_SOURCE); if (configuration.isInjectionEnabledForAllScripts() - || (!isScriptFile() && configuration.isInjectionEnabledForUiBasedScript()) + || (!isScriptFile() && configuration.isInjectionEnabledForScriptModules()) || (isTransformation() && configuration.isInjectionEnabledForTransformations())) { if (configuration.isInjectionCachingEnabled()) { logger.debug("Evaluating cached openhab-js injection for engine '{}' ...", engineIdentifier); @@ -349,8 +351,9 @@ protected String onScript(String script) { newScript = EVENT_CONVERSION_CODE + System.lineSeparator() + newScript; } - // keep this extendable for more directives by checking the first n lines (n = number or directives) - List header = script.lines().limit(1).toList(); + // 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 header = script.lines().limit(2).toList(); boolean useWrapper = configuration.isScriptConditionWrapperEnabled(); for (String line : header) { var matcher = USE_WRAPPER_DIRECTIVE.matcher(line); @@ -414,9 +417,9 @@ public void close() throws Exception { } /** - * Tests if the script is a script file, i.e. it is loaded from a file. + * Tests if the script is a script file, i.e. it is loaded from a JavaScript file. * - * @return + * @return true if the script is loaded from a JavaScript file, false otherwise */ private boolean isScriptFile() { ScriptContext ctx = delegate.getContext(); @@ -430,7 +433,7 @@ private boolean isScriptFile() { /** * Get the module type id (if any) of the module executing the script. * - * @return + * @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(); @@ -448,7 +451,7 @@ private boolean isScriptFile() { /** * Tests if a script is a script action, i.e. executed by the ScriptActionHandler. * - * @return + * @return true if the script is a script action, false otherwise */ private boolean isScriptAction() { return ScriptActionHandler.TYPE_ID.equals(getModuleTypeId()); @@ -457,7 +460,7 @@ private boolean isScriptAction() { /** * Tests if the script is a script condition, i.e. executed by the ScriptConditionHandler. * - * @return + * @return true if the script is a script condition, false otherwise */ private boolean isScriptCondition() { return ScriptConditionHandler.TYPE_ID.equals(getModuleTypeId()); @@ -466,7 +469,7 @@ private boolean isScriptCondition() { /** * Tests if the script is a transformation script, i.e. created from the script transformation service. * - * @return + * @return true if it is a transformation script, false otherwise */ private boolean isTransformation() { ScriptContext ctx = delegate.getContext(); From 7fb691b276dbdcb6bad8a251bf9b13af4aae12e0 Mon Sep 17 00:00:00 2001 From: Florian Hotze Date: Tue, 7 Oct 2025 00:02:14 +0200 Subject: [PATCH 5/7] Futher renaming of methods for technical correctness Signed-off-by: Florian Hotze --- .../internal/OpenhabGraalJSScriptEngine.java | 26 ++++++++++++++++--- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java index dc137ac25bfd4..38e2172ad1228 100644 --- a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java +++ b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java @@ -313,8 +313,15 @@ protected void beforeInvocation() { if (logger.isDebugEnabled()) { logger.debug( - "Engine '{}': isScriptFile(): {}, isScriptAction(): {}, isScriptCondition(): {}, isTransformation(): {}", - engineIdentifier, isScriptFile(), isScriptAction(), isScriptCondition(), isTransformation()); + "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 { @@ -322,7 +329,7 @@ protected void beforeInvocation() { delegate.getPolyglotContext().eval(GLOBAL_SOURCE); if (configuration.isInjectionEnabledForAllScripts() - || (!isScriptFile() && configuration.isInjectionEnabledForScriptModules()) + || (isScriptModule() && configuration.isInjectionEnabledForScriptModules()) || (isTransformation() && configuration.isInjectionEnabledForTransformations())) { if (configuration.isInjectionCachingEnabled()) { logger.debug("Evaluating cached openhab-js injection for engine '{}' ...", engineIdentifier); @@ -341,7 +348,7 @@ protected void beforeInvocation() { @Override protected String onScript(String script) { - if (isScriptFile() || isTransformation()) { + if (!isScriptModule()) { return super.onScript(script); } @@ -448,6 +455,17 @@ private boolean isScriptFile() { 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. * From 49e82b96c27af741dad832415b0bca26f641733b Mon Sep 17 00:00:00 2001 From: Florian Hotze Date: Tue, 7 Oct 2025 12:36:45 +0200 Subject: [PATCH 6/7] Enable the directive for script actions as well Signed-off-by: Florian Hotze --- .../jsscripting/internal/OpenhabGraalJSScriptEngine.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java index 38e2172ad1228..e233244e1bda5 100644 --- a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java +++ b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java @@ -361,7 +361,8 @@ protected String onScript(String script) { // 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 header = script.lines().limit(2).toList(); - boolean useWrapper = configuration.isScriptConditionWrapperEnabled(); + boolean useWrapper = isScriptAction() + || (isScriptCondition() && configuration.isScriptConditionWrapperEnabled()); for (String line : header) { var matcher = USE_WRAPPER_DIRECTIVE.matcher(line); if (!matcher.matches()) { @@ -380,7 +381,7 @@ protected String onScript(String script) { } } - if (isScriptAction() || (isScriptCondition() && useWrapper)) { + if (useWrapper) { logger.debug("Wrapping script for engine '{}' ...", engineIdentifier); newScript = "(function() {" + System.lineSeparator() + newScript + System.lineSeparator() + "})()"; } From 192d550bc0540567ac3e1bc00bbd96f26ef9d6d2 Mon Sep 17 00:00:00 2001 From: Florian Hotze Date: Wed, 8 Oct 2025 01:25:39 +0200 Subject: [PATCH 7/7] Address Copilot review Signed-off-by: Florian Hotze --- .../jsscripting/internal/GraalJSScriptEngineConfiguration.java | 2 +- .../jsscripting/internal/OpenhabGraalJSScriptEngine.java | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/GraalJSScriptEngineConfiguration.java b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/GraalJSScriptEngineConfiguration.java index 470184e9cffd7..f94b8d3227ef2 100644 --- a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/GraalJSScriptEngineConfiguration.java +++ b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/GraalJSScriptEngineConfiguration.java @@ -106,7 +106,7 @@ private void update(Map config) { injectionCachingEnabled = ConfigParser.valueAsOrElse(config.get(CFG_INJECTION_CACHING_ENABLED), Boolean.class, true); scriptConditionWrapperEnabled = ConfigParser.valueAsOrElse(config.get(CFG_SCRIPT_CONDITION_WRAPPER_ENABLED), - Boolean.class, true); + Boolean.class, false); eventConversionEnabled = ConfigParser.valueAsOrElse(config.get(CFG_EVENT_CONVERSION_ENABLED), Boolean.class, true); dependencyTrackingEnabled = ConfigParser.valueAsOrElse(config.get(CFG_DEPENDENCY_TRACKING_ENABLED), diff --git a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java index e233244e1bda5..be5d992ff7f6b 100644 --- a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java +++ b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java @@ -447,6 +447,7 @@ private boolean isScriptFile() { 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);