From 4caecfb781a35ab3630d1b4c0c19086f3412f80b Mon Sep 17 00:00:00 2001 From: Florian Hotze Date: Tue, 20 Dec 2022 09:15:43 +0100 Subject: [PATCH] [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 Signed-off-by: Andras Uhrin --- .../internal/GraalJSScriptEngineFactory.java | 29 ++++++++++--------- .../internal/OpenhabGraalJSScriptEngine.java | 6 ++-- ...ptEngineWithInvocableAndAutoCloseable.java | 12 ++++++++ 3 files changed, 32 insertions(+), 15 deletions(-) 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 1bdd8a23779ad..9883dd4e983b3 100644 --- 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 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 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 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/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 bd401d0c2eccc..b541819df849d 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 @@ -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/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 b283192cd39a0..18ac1f9eee338 100644 --- 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 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 } }