Fix variable binding in ScriptTransformationService (#3464)

by setting all attributes before compiling Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>
openhab · Mar 19, 2023 · cb38d19 · cb38d19
1 parent 1e55914
commit cb38d19
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 7 deletions.
diff --git a/.../src/main/java/org/openhab/core/automation/module/script/ScriptTransformationService.java b/.../src/main/java/org/openhab/core/automation/module/script/ScriptTransformationService.java
@@ -139,13 +139,6 @@ public void deactivate() {
             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 void deactivate() {
                     }
                 }
 
+                // 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/.../test/java/org/openhab/core/automation/module/script/ScriptTransformationServiceTest.java b/.../test/java/org/openhab/core/automation/module/script/ScriptTransformationServiceTest.java
@@ -22,6 +22,7 @@
 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.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 void scriptExecutionParametersAreInjectedIntoEngineContext() throws Trans
         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");