Skip to content

Commit

Permalink
[jsscripting] Minor fixes & improvements (openhab#13960)
Browse files Browse the repository at this point in the history
* [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>
Signed-off-by: Andras Uhrin <andras.uhrin@gmail.com>
  • Loading branch information
florian-h05 authored and andrasU committed Jan 6, 2024
1 parent b7cd96e commit 4caecfb
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,8 @@ public Path toRealPath(Path path, LinkOption... linkOptions) throws IOException

@Override
protected void beforeInvocation() {
super.beforeInvocation();

lock.lock();

if (initialized) {
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand All @@ -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
}
}

Expand All @@ -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
}
}

Expand All @@ -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
}
}

Expand All @@ -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
}
}

Expand All @@ -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
}
}

Expand Down

0 comments on commit 4caecfb

Please sign in to comment.