From cb38d1936006798d4eb0f71651bd1cb7c34bd158 Mon Sep 17 00:00:00 2001 From: jimtng <2554958+jimtng@users.noreply.github.com> Date: Mon, 20 Mar 2023 05:05:49 +1000 Subject: [PATCH] Fix variable binding in ScriptTransformationService (#3464) by setting all attributes before compiling Signed-off-by: Jimmy Tanagra --- .../script/ScriptTransformationService.java | 16 +++++++------- .../ScriptTransformationServiceTest.java | 21 +++++++++++++++++++ 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/ScriptTransformationService.java b/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/ScriptTransformationService.java index a94ff083d4..5faad600b2 100644 --- a/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/ScriptTransformationService.java +++ b/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/ScriptTransformationService.java @@ -139,13 +139,6 @@ public class ScriptTransformationService implements TransformationService, Regis try { CompiledScript compiledScript = scriptRecord.compiledScript; - if (compiledScript == null && scriptEngineContainer.getScriptEngine() instanceof Compilable) { - // no compiled script available but compiling is supported - compiledScript = ((Compilable) scriptEngineContainer.getScriptEngine()) - .compile(scriptRecord.script); - scriptRecord.compiledScript = compiledScript; - } - ScriptEngine engine = compiledScript != null ? compiledScript.getEngine() : scriptEngineContainer.getScriptEngine(); ScriptContext executionContext = engine.getContext(); @@ -165,6 +158,15 @@ public class ScriptTransformationService implements TransformationService, Regis } } + // compile the script here _after_ setting context attributes, so that the script engine + // can bind the attributes as variables during compilation. This primarily affects jruby. + if (compiledScript == null && scriptEngineContainer.getScriptEngine() instanceof Compilable) { + // no compiled script available but compiling is supported + compiledScript = ((Compilable) scriptEngineContainer.getScriptEngine()) + .compile(scriptRecord.script); + scriptRecord.compiledScript = compiledScript; + } + Object result = compiledScript != null ? compiledScript.eval() : engine.eval(scriptRecord.script); return result == null ? null : result.toString(); } catch (ScriptException e) { diff --git a/bundles/org.openhab.core.automation.module.script/src/test/java/org/openhab/core/automation/module/script/ScriptTransformationServiceTest.java b/bundles/org.openhab.core.automation.module.script/src/test/java/org/openhab/core/automation/module/script/ScriptTransformationServiceTest.java index f339903120..832b444dc2 100644 --- a/bundles/org.openhab.core.automation.module.script/src/test/java/org/openhab/core/automation/module/script/ScriptTransformationServiceTest.java +++ b/bundles/org.openhab.core.automation.module.script/src/test/java/org/openhab/core/automation/module/script/ScriptTransformationServiceTest.java @@ -22,6 +22,7 @@ import static org.mockito.Mockito.*; import java.util.Map; import java.util.Objects; +import javax.script.Compilable; import javax.script.ScriptContext; import javax.script.ScriptEngine; import javax.script.ScriptException; @@ -30,6 +31,7 @@ import org.eclipse.jdt.annotation.NonNullByDefault; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InOrder; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import org.mockito.junit.jupiter.MockitoSettings; @@ -109,6 +111,25 @@ public class ScriptTransformationServiceTest { verifyNoMoreInteractions(scriptContext); } + @Test + public void scriptSetAttributesBeforeCompiling() throws TransformationException, ScriptException { + abstract class CompilableScriptEngine implements ScriptEngine, Compilable { + } + scriptEngine = mock(CompilableScriptEngine.class); + + when(scriptEngineContainer.getScriptEngine()).thenReturn(scriptEngine); + when(scriptEngine.getContext()).thenReturn(scriptContext); + + InOrder inOrder = inOrder(scriptContext, scriptEngine); + + service.transform(SCRIPT_LANGUAGE + ":" + SCRIPT_UID + "?param1=value1", "input"); + + inOrder.verify(scriptContext, times(2)).setAttribute(anyString(), anyString(), eq(ScriptContext.ENGINE_SCOPE)); + inOrder.verify((Compilable) scriptEngine).compile(SCRIPT); + inOrder.verify(scriptEngine).eval(SCRIPT); + inOrder.verifyNoMoreInteractions(); + } + @Test public void invalidScriptExecutionParametersAreDiscarded() throws TransformationException { service.transform(SCRIPT_LANGUAGE + ":" + SCRIPT_UID + "?param1=value1&invalid", "input");