[jsscripting] Minor fixes & improvements (#13960)

* [jsscripting] Correct wrong `createScriptEngine` implementation
* [jsscripting] Also unlock lock on unexpected exceptions (rethrow them)
* [jsscripting] Call super methods from their overrides
* [jsscripting] Move superclass call of `beforeInvocation`

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

diff --git a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/GraalJSScriptEngineFactory.java b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/GraalJSScriptEngineFactory.java
index 1bdd8a23779ad1ede203e0309f716d94c2d789cc..9883dd4e983b3a18caefac725ae804a11bcdcc31 100644 (file)
--- a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/GraalJSScriptEngineFactory.java
+++ b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/GraalJSScriptEngineFactory.java
@@ -44,9 +44,21 @@ public final class GraalJSScriptEngineFactory implements ScriptEngineFactory {
     private static final String INJECTION_CODE = "Object.assign(this, require('openhab'));";
     private boolean injectionEnabled = true;
 
+    /*
+     * Whilst we run in parallel with Nashorn, we use a custom mime-type to avoid
+     * disrupting Nashorn scripts. When Nashorn is removed, we take over the standard
+     * JS runtime.
+     */
+
+    // GraalJSEngineFactory graalJSEngineFactory = new GraalJSEngineFactory();
+    //
+    // scriptTypes.addAll(graalJSEngineFactory.getMimeTypes());
+    // scriptTypes.addAll(graalJSEngineFactory.getExtensions());
+
     public static final String MIME_TYPE = "application/javascript;version=ECMAScript-2021";
     private static final String ALT_MIME_TYPE = "text/javascript;version=ECMAScript-2021";
     private static final String ALIAS = "graaljs";
+    private static final List<String> SCRIPT_TYPES = List.of(MIME_TYPE, ALT_MIME_TYPE, ALIAS);
 
     private final JSScriptServiceUtil jsScriptServiceUtil;
     private final JSDependencyTracker jsDependencyTracker;
@@ -61,19 +73,7 @@ public final class GraalJSScriptEngineFactory implements ScriptEngineFactory {
 
     @Override
     public List<String> getScriptTypes() {
-
-        /*
-         * Whilst we run in parallel with Nashorn, we use a custom mime-type to avoid
-         * disrupting Nashorn scripts. When Nashorn is removed, we take over the standard
-         * JS runtime.
-         */
-
-        // GraalJSEngineFactory graalJSEngineFactory = new GraalJSEngineFactory();
-        //
-        // scriptTypes.addAll(graalJSEngineFactory.getMimeTypes());
-        // scriptTypes.addAll(graalJSEngineFactory.getExtensions());
-
-        return List.of(MIME_TYPE, ALT_MIME_TYPE, ALIAS);
+        return SCRIPT_TYPES;
     }
 
     @Override
@@ -83,6 +83,9 @@ public final class GraalJSScriptEngineFactory implements ScriptEngineFactory {
 
     @Override
     public @Nullable ScriptEngine createScriptEngine(String scriptType) {
+        if (!SCRIPT_TYPES.contains(scriptType)) {
+            return null;
+        }
         return new DebuggingGraalScriptEngine<>(
                 new OpenhabGraalJSScriptEngine(injectionEnabled ? INJECTION_CODE : null, jsScriptServiceUtil));
     }

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 bd401d0c2eccc6743bd2580dc79c48b5f5328e7a..b541819df849d3ef3d010c5064be65653286afd1 100644 (file)
--- 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
@@ -178,6 +178,8 @@ public class OpenhabGraalJSScriptEngine
 
     @Override
     protected void beforeInvocation() {
+        super.beforeInvocation();
+
         lock.lock();
 
         if (initialized) {
@@ -235,13 +237,13 @@ public class OpenhabGraalJSScriptEngine
     @Override
     protected Object afterInvocation(Object obj) {
         lock.unlock();
-        return obj;
+        return super.afterInvocation(obj);
     }
 
     @Override
     protected Exception afterThrowsInvocation(Exception e) {
         lock.unlock();
-        return e;
+        return super.afterThrowsInvocation(e);
     }
 
     @Override

diff --git a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/scriptengine/InvocationInterceptingScriptEngineWithInvocableAndAutoCloseable.java b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/scriptengine/InvocationInterceptingScriptEngineWithInvocableAndAutoCloseable.java
index b283192cd39a02a2d2d86300b648a56721ccee28..18ac1f9eee338e0e532ec7921135fa21b4c3f6c6 100644 (file)
--- a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/scriptengine/InvocationInterceptingScriptEngineWithInvocableAndAutoCloseable.java
+++ b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/scriptengine/InvocationInterceptingScriptEngineWithInvocableAndAutoCloseable.java
@@ -54,6 +54,8 @@ public abstract class InvocationInterceptingScriptEngineWithInvocableAndAutoClos
             return afterInvocation(super.eval(s, scriptContext));
         } catch (ScriptException se) {
             throw (ScriptException) afterThrowsInvocation(se);
+        } catch (Exception e) {
+            throw new UndeclaredThrowableException(afterThrowsInvocation(e)); // Wrap and rethrow other exceptions
         }
     }
 
@@ -64,6 +66,8 @@ public abstract class InvocationInterceptingScriptEngineWithInvocableAndAutoClos
             return afterInvocation(super.eval(reader, scriptContext));
         } catch (ScriptException se) {
             throw (ScriptException) afterThrowsInvocation(se);
+        } catch (Exception e) {
+            throw new UndeclaredThrowableException(afterThrowsInvocation(e)); // Wrap and rethrow other exceptions
         }
     }
 
@@ -74,6 +78,8 @@ public abstract class InvocationInterceptingScriptEngineWithInvocableAndAutoClos
             return afterInvocation(super.eval(s));
         } catch (ScriptException se) {
             throw (ScriptException) afterThrowsInvocation(se);
+        } catch (Exception e) {
+            throw new UndeclaredThrowableException(afterThrowsInvocation(e)); // Wrap and rethrow other exceptions
         }
     }
 
@@ -84,6 +90,8 @@ public abstract class InvocationInterceptingScriptEngineWithInvocableAndAutoClos
             return afterInvocation(super.eval(reader));
         } catch (ScriptException se) {
             throw (ScriptException) afterThrowsInvocation(se);
+        } catch (Exception e) {
+            throw new UndeclaredThrowableException(afterThrowsInvocation(e)); // Wrap and rethrow other exceptions
         }
     }
 
@@ -94,6 +102,8 @@ public abstract class InvocationInterceptingScriptEngineWithInvocableAndAutoClos
             return afterInvocation(super.eval(s, bindings));
         } catch (ScriptException se) {
             throw (ScriptException) afterThrowsInvocation(se);
+        } catch (Exception e) {
+            throw new UndeclaredThrowableException(afterThrowsInvocation(e)); // Wrap and rethrow other exceptions
         }
     }
 
@@ -104,6 +114,8 @@ public abstract class InvocationInterceptingScriptEngineWithInvocableAndAutoClos
             return afterInvocation(super.eval(reader, bindings));
         } catch (ScriptException se) {
             throw (ScriptException) afterThrowsInvocation(se);
+        } catch (Exception e) {
+            throw new UndeclaredThrowableException(afterThrowsInvocation(e)); // Wrap and rethrow other exceptions
         }
     }