[jsscripting] Minor fixes & improvements (openhab#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 <[email protected]> Signed-off-by: Doug Culnane <[email protected]>
dougculnane · Dec 26, 2022 · a19c641 · a19c641
1 parent 8bc0d96
commit a19c641
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 15 deletions.
diff --git a/...src/main/java/org/openhab/automation/jsscripting/internal/GraalJSScriptEngineFactory.java b/...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 GraalJSScriptEngineFactory(final @Reference JSScriptServiceUtil jsScriptS
 
     @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 void scopeValues(ScriptEngine scriptEngine, Map<String, Object> scopeValu
 
     @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/...src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java b/...src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java
@@ -178,6 +178,8 @@ public Path toRealPath(Path path, LinkOption... linkOptions) throws IOException
 
     @Override
     protected void beforeInvocation() {
+        super.beforeInvocation();
+
         lock.lock();
 
         if (initialized) {
@@ -235,13 +237,13 @@ protected void beforeInvocation() {
     @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/...nternal/scriptengine/InvocationInterceptingScriptEngineWithInvocableAndAutoCloseable.java b/...nternal/scriptengine/InvocationInterceptingScriptEngineWithInvocableAndAutoCloseable.java
@@ -54,6 +54,8 @@ public Object eval(String s, ScriptContext scriptContext) throws ScriptException
             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 Object eval(Reader reader, ScriptContext scriptContext) throws ScriptExce
             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 Object eval(String s) throws ScriptException {
             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 Object eval(Reader reader) throws ScriptException {
             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 Object eval(String s, Bindings bindings) throws ScriptException {
             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 Object eval(Reader reader, Bindings bindings) throws ScriptException {
             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
         }
     }