From 5b9aa47d4046e2422560803cfc98d474a5450eaf Mon Sep 17 00:00:00 2001 From: kares Date: Mon, 2 Nov 2020 14:36:23 +0100 Subject: [PATCH 01/38] [refactor] remove nil check - RubyNil.toJava will do --- .../org/jruby/embed/jsr223/JRubyCompiledScript.java | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/core/src/main/java/org/jruby/embed/jsr223/JRubyCompiledScript.java b/core/src/main/java/org/jruby/embed/jsr223/JRubyCompiledScript.java index 3b02a551b19..c18f78f15f1 100644 --- a/core/src/main/java/org/jruby/embed/jsr223/JRubyCompiledScript.java +++ b/core/src/main/java/org/jruby/embed/jsr223/JRubyCompiledScript.java @@ -51,16 +51,14 @@ public class JRubyCompiledScript extends CompiledScript { private final JRubyEngine engine; private final EmbedEvalUnit unit; - JRubyCompiledScript(ScriptingContainer container, - JRubyEngine engine, String script) { + JRubyCompiledScript(ScriptingContainer container, JRubyEngine engine, String script) { this.container = container; this.engine = engine; Utils.preEval(container, engine.getContext()); unit = container.parse(script); } - JRubyCompiledScript(ScriptingContainer container, - JRubyEngine engine, Reader reader) { + JRubyCompiledScript(ScriptingContainer container, JRubyEngine engine, Reader reader) { this.container = container; this.engine = engine; String filename = System.getProperty(ScriptEngine.FILENAME); @@ -84,10 +82,7 @@ public Object eval(ScriptContext context) throws ScriptException { } Utils.preEval(container, context); IRubyObject ret = unit.run(); - if (!(ret instanceof RubyNil)) { - return JavaEmbedUtils.rubyToJava(ret); - } - return null; + return JavaEmbedUtils.rubyToJava(ret); } catch (Exception e) { throw wrapException(e); } finally { @@ -99,7 +94,7 @@ public Object eval(ScriptContext context) throws ScriptException { } } - private ScriptException wrapException(Exception e) throws ScriptException { + private static ScriptException wrapException(Exception e) { return new ScriptException(e); } From a27a3eb07a19cbaaea6264c8b2366a832c85422f Mon Sep 17 00:00:00 2001 From: kares Date: Mon, 2 Nov 2020 14:37:12 +0100 Subject: [PATCH 02/38] [refactor] no need to use internal Scope class --- .../org/jruby/embed/jsr223/JRubyContext.java | 148 ++++++------------ 1 file changed, 52 insertions(+), 96 deletions(-) diff --git a/core/src/main/java/org/jruby/embed/jsr223/JRubyContext.java b/core/src/main/java/org/jruby/embed/jsr223/JRubyContext.java index 03189d2d0ba..de0aee0b44a 100644 --- a/core/src/main/java/org/jruby/embed/jsr223/JRubyContext.java +++ b/core/src/main/java/org/jruby/embed/jsr223/JRubyContext.java @@ -29,13 +29,12 @@ */ package org.jruby.embed.jsr223; -import java.io.InputStreamReader; -import java.io.PrintWriter; import java.io.Reader; import java.io.Writer; -import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.stream.Collectors; import javax.script.Bindings; import javax.script.ScriptContext; import javax.script.SimpleBindings; @@ -47,6 +46,12 @@ * @author Yoko Harada */ class JRubyContext implements ScriptContext { + + private static final int[] SCOPES = new int[] { ScriptContext.ENGINE_SCOPE, ScriptContext.GLOBAL_SCOPE }; + + private static final List SCOPE_LIST = + Collections.unmodifiableList( Arrays.stream(SCOPES).boxed().collect(Collectors.toList()) ); + private final ScriptingContainer container; private final List scopeList; private Bindings globalMap = null; @@ -54,44 +59,16 @@ class JRubyContext implements ScriptContext { private Reader reader = null; private Writer writer = null; private Writer errorWriter = null; - - public enum Scope { - - ENGINE(ScriptContext.ENGINE_SCOPE), - GLOBAL(ScriptContext.GLOBAL_SCOPE); - private final int priority; - - Scope(int priority) { - this.priority = priority; - } - - int getPriority() { - return priority; - } - } JRubyContext(ScriptingContainer container) { this.container = container; - List list = new ArrayList(); - for (Scope scope : Scope.values()) { - list.add(scope.getPriority()); - } - scopeList = Collections.unmodifiableList(list); - } - - private void checkName(String name) { - if (name == null) { - throw new NullPointerException("name is null"); - } - if (name.length() == 0) { - throw new IllegalArgumentException("name is empty"); - } + this.scopeList = SCOPE_LIST; } public Object getAttribute(String name) { Object ret = null; - for (Scope scope : Scope.values()) { - ret = getAttributeFromScope(scope.getPriority(), name); + for (int scope : SCOPES) { + ret = getAttributeFromScope(scope, name); if (ret != null) { return ret; } @@ -99,23 +76,23 @@ public Object getAttribute(String name) { return ret; } - private Object getAttributeFromScope(int priority, String name) { - checkName(name); + private Object getAttributeFromScope(final int scope, String name) { Object value; - if (priority == Scope.ENGINE.getPriority()) { - value = engineMap.get(name); - if (value == null && Utils.isRubyVariable(container, name)) { - value = container.get(Utils.getReceiver(this), name); - engineMap.put(name, value); - } - return value; - } else if (priority == Scope.GLOBAL.getPriority()) { - if (globalMap == null) { - return null; - } - return globalMap.get(name); - } else { - throw new IllegalArgumentException("invalid scope"); + switch (scope) { + case ScriptContext.ENGINE_SCOPE: + value = engineMap.get(name); + if (value == null && Utils.isRubyVariable(container, name)) { + value = container.get(Utils.getReceiver(this), name); + engineMap.put(name, value); + } + return value; + case ScriptContext.GLOBAL_SCOPE: + if (globalMap == null) { + return null; + } + return globalMap.get(name); + default: + throw new IllegalArgumentException("invalid scope"); } } @@ -124,33 +101,24 @@ public Object getAttribute(String name, int scope) { } public int getAttributesScope(String name) { - for (Scope scope : Scope.values()) { - Object ret = getAttributeFromScope(scope.getPriority(), name); - if (ret != null) { - return scope.getPriority(); - } + for (int scope : SCOPES) { + Object ret = getAttributeFromScope(scope, name); + if (ret != null) return scope; } return -1; } - public Bindings getBindings(int priority) { - if (priority == Scope.ENGINE.getPriority()) { - return engineMap; - } else if (priority == Scope.GLOBAL.getPriority()) { - return globalMap; - } else { - throw new IllegalArgumentException("invalid scope"); + public Bindings getBindings(int scope) { + switch (scope) { + case ScriptContext.ENGINE_SCOPE: + return engineMap; + case ScriptContext.GLOBAL_SCOPE: + return globalMap; + default: + throw new IllegalArgumentException("invalid scope"); } } - Bindings getEngineScopeBindings() { - return engineMap; - } - - Bindings getGlobalScopeBindings() { - return globalMap; - } - public Writer getErrorWriter() { return errorWriter; } @@ -167,17 +135,16 @@ public Writer getWriter() { return writer; } - public Object removeAttribute(String name, int priority) { - checkName(name); - Bindings bindings = getBindings(priority); + public Object removeAttribute(String name, int scope) { + Bindings bindings = getBindings(scope); if (bindings == null) { return null; } return bindings.remove(name); } - public void setAttribute(String key, Object value, int priority) { - Bindings bindings = getBindings(priority); + public void setAttribute(String key, Object value, int scope) { + Bindings bindings = getBindings(scope); if (bindings == null) { return; } @@ -185,24 +152,17 @@ public void setAttribute(String key, Object value, int priority) { } public void setBindings(Bindings bindings, int scope) { - if (scope == Scope.ENGINE.getPriority() && bindings == null) { - throw new NullPointerException("null bindings in ENGINE scope"); + switch (scope) { + case ScriptContext.ENGINE_SCOPE: + if (bindings == null) { + throw new NullPointerException("null bindings in ENGINE scope"); + } + engineMap = bindings; break; + case ScriptContext.GLOBAL_SCOPE: + globalMap = bindings; break; + default: + throw new IllegalArgumentException("invalid scope"); } - if (scope == Scope.ENGINE.getPriority()) { - engineMap = bindings; - } else if (scope == Scope.GLOBAL.getPriority()) { - globalMap = bindings; - } else { - throw new IllegalArgumentException("invalid scope"); - } - } - - void setEngineScopeBindings(Bindings bindings) { - engineMap = bindings; - } - - void setGlobalScopeBindings(Bindings bindings) { - globalMap = bindings; } public void setErrorWriter(Writer errorWriter) { @@ -216,10 +176,6 @@ public void setErrorWriter(Writer errorWriter) { } public void setReader(Reader reader) { - setReader(reader, true); - } - - void setReader(Reader reader, boolean updateContainer) { if (reader == null) { return; } From d37a3154fbf3124c2b8254a50d35b927f27b0314 Mon Sep 17 00:00:00 2001 From: kares Date: Mon, 2 Nov 2020 15:27:28 +0100 Subject: [PATCH 03/38] let scripring container throw raise exception --- .../src/main/java/org/jruby/embed/ScriptingContainer.java | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/core/src/main/java/org/jruby/embed/ScriptingContainer.java b/core/src/main/java/org/jruby/embed/ScriptingContainer.java index f6b08fc039b..55890917700 100644 --- a/core/src/main/java/org/jruby/embed/ScriptingContainer.java +++ b/core/src/main/java/org/jruby/embed/ScriptingContainer.java @@ -237,13 +237,7 @@ public ScriptingContainer(LocalContextScope scope, LocalVariableBehavior behavio public ScriptingContainer(LocalContextScope scope, LocalVariableBehavior behavior, boolean lazy) { this.provider = getProviderInstance(scope, behavior, lazy); this.scope = scope; - try { - initRubyInstanceConfig(); - } - catch (RaiseException ex) { - // TODO this seems useless - except that we get a Java stack trace - throw new RuntimeException(ex); - } + initRubyInstanceConfig(); basicProperties = getBasicProperties(); } From 172142499cd3ccc07e92af62d160dcbaa93f49e6 Mon Sep 17 00:00:00 2001 From: kares Date: Mon, 2 Nov 2020 19:07:59 +0100 Subject: [PATCH 04/38] Refactor: code re-use + less params to pass around --- .../org/jruby/embed/internal/BiVariableMap.java | 4 ++-- .../jruby/embed/internal/EmbedEvalUnitImpl.java | 15 +++++++-------- .../internal/EmbedRubyObjectAdapterImpl.java | 14 ++++++-------- .../internal/EmbedRubyRuntimeAdapterImpl.java | 8 +++----- .../jruby/embed/variable/VariableInterceptor.java | 6 ++---- 5 files changed, 20 insertions(+), 27 deletions(-) diff --git a/core/src/main/java/org/jruby/embed/internal/BiVariableMap.java b/core/src/main/java/org/jruby/embed/internal/BiVariableMap.java index 1a55536f943..a82b7ade580 100644 --- a/core/src/main/java/org/jruby/embed/internal/BiVariableMap.java +++ b/core/src/main/java/org/jruby/embed/internal/BiVariableMap.java @@ -380,8 +380,8 @@ public IRubyObject[] getLocalVarValues() { return localVarValues.toArray( new IRubyObject[ localVarValues.size() ] ); } - void inject(final ManyVarsDynamicScope scope, final int depth, final IRubyObject receiver) { - VariableInterceptor.inject(this, provider.getRuntime(), scope, depth, receiver); + void inject(final ManyVarsDynamicScope scope) { + VariableInterceptor.inject(this, provider.getRuntime(), scope); } void retrieve(final IRubyObject receiver) { diff --git a/core/src/main/java/org/jruby/embed/internal/EmbedEvalUnitImpl.java b/core/src/main/java/org/jruby/embed/internal/EmbedEvalUnitImpl.java index 923bf86f38d..93dff82e2e5 100644 --- a/core/src/main/java/org/jruby/embed/internal/EmbedEvalUnitImpl.java +++ b/core/src/main/java/org/jruby/embed/internal/EmbedEvalUnitImpl.java @@ -101,13 +101,13 @@ public IRubyObject run() { } final Ruby runtime = container.getProvider().getRuntime(); final BiVariableMap vars = container.getVarMap(); - final boolean sharing_variables = isSharingVariables(); + final boolean sharing_variables = isSharingVariables(container); // Keep reference to current context to prevent it being collected. - final ThreadContext threadContext = runtime.getCurrentContext(); + final ThreadContext context = runtime.getCurrentContext(); if (sharing_variables) { - vars.inject(scope, 0, null); - threadContext.pushScope(scope); + vars.inject(scope); + context.pushScope(scope); } try { final IRubyObject ret; @@ -137,7 +137,7 @@ public IRubyObject run() { } finally { if (sharing_variables) { - threadContext.popScope(); + context.popScope(); } vars.terminate(); /* Below lines doesn't work. Neither does classCache.flush(). How to clear cache? @@ -147,10 +147,9 @@ public IRubyObject run() { } } - private boolean isSharingVariables() { + static boolean isSharingVariables(ScriptingContainer container) { final Object sharing = container.getAttribute(AttributeName.SHARING_VARIABLES); - if ( sharing != null && sharing instanceof Boolean && - ((Boolean) sharing).booleanValue() == false ) { + if ( sharing instanceof Boolean && ((Boolean) sharing).booleanValue() == false ) { return false; } return true; diff --git a/core/src/main/java/org/jruby/embed/internal/EmbedRubyObjectAdapterImpl.java b/core/src/main/java/org/jruby/embed/internal/EmbedRubyObjectAdapterImpl.java index f771677efac..acadedb142d 100644 --- a/core/src/main/java/org/jruby/embed/internal/EmbedRubyObjectAdapterImpl.java +++ b/core/src/main/java/org/jruby/embed/internal/EmbedRubyObjectAdapterImpl.java @@ -54,6 +54,8 @@ import org.jruby.runtime.builtin.IRubyObject; import org.jruby.runtime.scope.ManyVarsDynamicScope; +import static org.jruby.embed.internal.EmbedEvalUnitImpl.isSharingVariables; + /** * Implementation of {@link EmbedRubyObjectAdapter}. Users get an instance of this * class by newObjectAdapter() method of {@link ScriptingContainer}. @@ -65,7 +67,7 @@ public class EmbedRubyObjectAdapterImpl implements EmbedRubyObjectAdapter { private final RubyObjectAdapter adapter = JavaEmbedUtils.newObjectAdapter(); private final ScriptingContainer container; - public static enum MethodType { + public enum MethodType { CALLMETHOD_NOARG, CALLMETHOD, CALLMETHOD_WITHBLOCK, @@ -289,19 +291,15 @@ private T call(MethodType type, Class returnType, RubyObject rubyReceiver if (methodName == null || methodName.length()==0) { return null; } - Ruby runtime = container.getProvider().getRuntime(); - boolean sharing_variables = true; - Object obj = container.getAttribute(AttributeName.SHARING_VARIABLES); - if (obj != null && obj instanceof Boolean && ((Boolean) obj) == false) { - sharing_variables = false; - } + final Ruby runtime = container.getProvider().getRuntime(); + final boolean sharing_variables = isSharingVariables(container); if (sharing_variables) { ManyVarsDynamicScope scope; if (unit != null && unit.getScope() != null) scope = unit.getScope(); else scope = EmbedRubyRuntimeAdapterImpl.getManyVarsDynamicScope(container, 0); - container.getVarMap().inject(scope, 0, rubyReceiver); + container.getVarMap().inject(scope); runtime.getCurrentContext().pushScope(scope); } diff --git a/core/src/main/java/org/jruby/embed/internal/EmbedRubyRuntimeAdapterImpl.java b/core/src/main/java/org/jruby/embed/internal/EmbedRubyRuntimeAdapterImpl.java index 2f1fb12394b..3e247ad2b44 100644 --- a/core/src/main/java/org/jruby/embed/internal/EmbedRubyRuntimeAdapterImpl.java +++ b/core/src/main/java/org/jruby/embed/internal/EmbedRubyRuntimeAdapterImpl.java @@ -65,6 +65,8 @@ import org.jruby.runtime.load.LoadService; import org.jruby.runtime.scope.ManyVarsDynamicScope; +import static org.jruby.embed.internal.EmbedEvalUnitImpl.isSharingVariables; + /** * * @author Yoko Harada @@ -173,11 +175,7 @@ private EmbedEvalUnit runParser(Object input, String filename, int... lines) { } try { ManyVarsDynamicScope scope = null; - boolean sharing_variables = true; - Object obj = container.getAttribute(AttributeName.SHARING_VARIABLES); - if (obj instanceof Boolean && !((Boolean) obj)) { - sharing_variables = false; - } + final boolean sharing_variables = isSharingVariables(container); if (sharing_variables) { scope = getManyVarsDynamicScope(container, 0); } diff --git a/core/src/main/java/org/jruby/embed/variable/VariableInterceptor.java b/core/src/main/java/org/jruby/embed/variable/VariableInterceptor.java index d29f8941efc..8d12bf11130 100644 --- a/core/src/main/java/org/jruby/embed/variable/VariableInterceptor.java +++ b/core/src/main/java/org/jruby/embed/variable/VariableInterceptor.java @@ -124,17 +124,15 @@ private static BiVariable resolve(BiVariable[] entries) { * @param map a variable map that has name-value pairs to be injected * @param runtime Ruby runtime * @param scope scope to inject local variable values - * @param depth depth of a frame to inject local variable values - * @param receiver a receiver when the script has been evaluated once */ - public static void inject(BiVariableMap map, Ruby runtime, ManyVarsDynamicScope scope, int depth, IRubyObject receiver) { + public static void inject(BiVariableMap map, Ruby runtime, ManyVarsDynamicScope scope) { // lvar might not be given while parsing but be given when evaluating. // to avoid ArrayIndexOutOfBoundsException, checks the length of scope.getValues() if (scope != null && scope.getValues().length > 0) { IRubyObject[] values4Injection = map.getLocalVarValues(); if (values4Injection != null && values4Injection.length > 0) { for (int i = 0; i < values4Injection.length; i++) { - scope.setValue(i, values4Injection[i], depth); + scope.setValue(i, values4Injection[i], 0); } } } From 7481ff7baa31d4343fc770a97c5d36af832b3417 Mon Sep 17 00:00:00 2001 From: kares Date: Mon, 2 Nov 2020 19:08:16 +0100 Subject: [PATCH 05/38] Refactor: code cleanup --- .../embed/variable/VariableInterceptor.java | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/core/src/main/java/org/jruby/embed/variable/VariableInterceptor.java b/core/src/main/java/org/jruby/embed/variable/VariableInterceptor.java index 8d12bf11130..775e27852d2 100644 --- a/core/src/main/java/org/jruby/embed/variable/VariableInterceptor.java +++ b/core/src/main/java/org/jruby/embed/variable/VariableInterceptor.java @@ -30,9 +30,7 @@ package org.jruby.embed.variable; import java.util.Collection; -import java.util.Iterator; import java.util.List; -import java.util.Map; import org.jruby.Ruby; import org.jruby.RubyObject; @@ -47,20 +45,6 @@ * @author Yoko Harada */ public class VariableInterceptor { - //private LocalVariableBehavior behavior; - - /** - * Constructs an instance with a given local variable behavior. - * - * @param behavior local variable behavior - */ - //public VariableInterceptor(LocalVariableBehavior behavior) { - // this.behavior = behavior; - //} - - //public LocalVariableBehavior getLocalVariableBehavior() { - // return behavior; - //} /** * Returns an appropriate type of a variable instance to the specified local @@ -218,7 +202,7 @@ public static void terminateGlobalVariables(LocalVariableBehavior behavior, Coll if (BiVariable.Type.LocalGlobalVariable == var.getType()) { String name = var.getName(); name = name.startsWith("$") ? name : "$" + name; - runtime.getGlobalVariables().set(name, runtime.getNil()); + runtime.getGlobalVariables().clear(name); } } } From b58daed7787c80a1e82441616d5a96340a9e7ab1 Mon Sep 17 00:00:00 2001 From: kares Date: Tue, 3 Nov 2020 09:01:48 +0100 Subject: [PATCH 06/38] [refactor] code a bit (unnecessary null checks) also throw on missing or empty method name ... --- .../internal/EmbedRubyObjectAdapterImpl.java | 98 ++++++++----------- .../jruby/embed/ScriptingContainerTest.java | 2 +- 2 files changed, 42 insertions(+), 58 deletions(-) diff --git a/core/src/main/java/org/jruby/embed/internal/EmbedRubyObjectAdapterImpl.java b/core/src/main/java/org/jruby/embed/internal/EmbedRubyObjectAdapterImpl.java index acadedb142d..a07f4b3e138 100644 --- a/core/src/main/java/org/jruby/embed/internal/EmbedRubyObjectAdapterImpl.java +++ b/core/src/main/java/org/jruby/embed/internal/EmbedRubyObjectAdapterImpl.java @@ -32,11 +32,9 @@ import org.jruby.Ruby; import org.jruby.RubyInteger; import org.jruby.RubyModule; -import org.jruby.RubyNil; import org.jruby.RubyObject; import org.jruby.RubyObjectAdapter; import org.jruby.RubyString; -import org.jruby.embed.AttributeName; import org.jruby.embed.EmbedEvalUnit; import org.jruby.embed.EmbedRubyObjectAdapter; import org.jruby.embed.InvokeFailedException; @@ -146,8 +144,7 @@ public IRubyObject callSuper(IRubyObject receiver, IRubyObject[] args, Block blo public T callMethod(Object receiver, String methodName, Class returnType) { try { - RubyObject rubyReceiver = getReceiverObject(receiver); - return call(MethodType.CALLMETHOD_NOARG, returnType, rubyReceiver, methodName, null, null); + return call(MethodType.CALLMETHOD_NOARG, returnType, getReceiverObject(receiver), methodName, null, null); } catch (InvokeFailedException e) { throw e; } catch (Throwable e) { @@ -157,8 +154,7 @@ public T callMethod(Object receiver, String methodName, Class returnType) public T callMethod(Object receiver, String methodName, Object singleArg, Class returnType) { try { - RubyObject rubyReceiver = getReceiverObject(receiver); - return call(MethodType.CALLMETHOD, returnType, rubyReceiver, methodName, null, null, singleArg); + return call(MethodType.CALLMETHOD, returnType, getReceiverObject(receiver), methodName, null, null, singleArg); } catch (InvokeFailedException e) { throw e; } catch (Throwable e) { @@ -168,8 +164,7 @@ public T callMethod(Object receiver, String methodName, Object singleArg, Cl public T callMethod(Object receiver, String methodName, Object[] args, Class returnType) { try { - RubyObject rubyReceiver = getReceiverObject(receiver); - return call(MethodType.CALLMETHOD, returnType, rubyReceiver, methodName, null, null, args); + return call(MethodType.CALLMETHOD, returnType, getReceiverObject(receiver), methodName, null, null, args); } catch (InvokeFailedException e) { throw e; } catch (Throwable e) { @@ -179,8 +174,7 @@ public T callMethod(Object receiver, String methodName, Object[] args, Class public T callMethod(Object receiver, String methodName, Object[] args, Block block, Class returnType) { try { - RubyObject rubyReceiver = getReceiverObject(receiver); - return call(MethodType.CALLMETHOD_WITHBLOCK, returnType, rubyReceiver, methodName, block, null, args); + return call(MethodType.CALLMETHOD_WITHBLOCK, returnType, getReceiverObject(receiver), methodName, block, null, args); } catch (InvokeFailedException e) { throw e; } catch (Throwable e) { @@ -190,8 +184,7 @@ public T callMethod(Object receiver, String methodName, Object[] args, Block public T callMethod(Object receiver, String methodName, Class returnType, EmbedEvalUnit unit) { try { - RubyObject rubyReceiver = getReceiverObject(receiver); - return call(MethodType.CALLMETHOD_NOARG, returnType, rubyReceiver, methodName, null, unit); + return call(MethodType.CALLMETHOD_NOARG, returnType, getReceiverObject(receiver), methodName, null, unit); } catch (InvokeFailedException e) { throw e; } catch (Throwable e) { @@ -201,8 +194,7 @@ public T callMethod(Object receiver, String methodName, Class returnType, public T callMethod(Object receiver, String methodName, Object[] args, Class returnType, EmbedEvalUnit unit) { try { - RubyObject rubyReceiver = getReceiverObject(receiver); - return call(MethodType.CALLMETHOD, returnType, rubyReceiver, methodName, null, unit, args); + return call(MethodType.CALLMETHOD, returnType, getReceiverObject(receiver), methodName, null, unit, args); } catch (InvokeFailedException e) { throw e; } catch (Throwable e) { @@ -212,8 +204,7 @@ public T callMethod(Object receiver, String methodName, Object[] args, Class public T callMethod(Object receiver, String methodName, Object[] args, Block block, Class returnType, EmbedEvalUnit unit) { try { - RubyObject rubyReceiver = getReceiverObject(receiver); - return call(MethodType.CALLMETHOD_WITHBLOCK, returnType, rubyReceiver, methodName, block, unit, args); + return call(MethodType.CALLMETHOD_WITHBLOCK, returnType, getReceiverObject(receiver), methodName, block, unit, args); } catch (InvokeFailedException e) { throw e; } catch (Throwable e) { @@ -223,8 +214,7 @@ public T callMethod(Object receiver, String methodName, Object[] args, Block public T callSuper(Object receiver, Object[] args, Class returnType) { try { - RubyObject rubyReceiver = getReceiverObject(receiver); - return call(MethodType.CALLSUPER, returnType, rubyReceiver, null, null, null, args); + return call(MethodType.CALLSUPER, returnType, getReceiverObject(receiver), null, null, null, args); } catch (InvokeFailedException e) { throw e; } catch (Throwable e) { @@ -234,8 +224,7 @@ public T callSuper(Object receiver, Object[] args, Class returnType) { public T callSuper(Object receiver, Object[] args, Block block, Class returnType) { try { - RubyObject rubyReceiver = getReceiverObject(receiver); - return call(MethodType.CALLSUPER_WITHBLOCK, returnType, rubyReceiver, null, block, null, args); + return call(MethodType.CALLSUPER_WITHBLOCK, returnType, getReceiverObject(receiver), null, block, null, args); } catch (InvokeFailedException e) { throw e; } catch (Throwable e) { @@ -245,11 +234,10 @@ public T callSuper(Object receiver, Object[] args, Block block, Class ret public Object callMethod(Object receiver, String methodName, Object... args) { try { - RubyObject rubyReceiver = getReceiverObject(receiver); if (args.length == 0) { - return call(MethodType.CALLMETHOD_NOARG, Object.class, rubyReceiver, methodName, null, null); + return call(MethodType.CALLMETHOD_NOARG, Object.class, getReceiverObject(receiver), methodName, null, null); } else { - return call(MethodType.CALLMETHOD, Object.class, rubyReceiver, methodName, null, null, args); + return call(MethodType.CALLMETHOD, Object.class, getReceiverObject(receiver), methodName, null, null, args); } } catch (InvokeFailedException e) { throw e; @@ -263,8 +251,7 @@ public Object callMethod(Object receiver, String methodName, Block block, Object if (args.length == 0) { throw new IllegalArgumentException("needs at least one argument in a method"); } - RubyObject rubyReceiver = getReceiverObject(receiver); - return call(MethodType.CALLMETHOD_WITHBLOCK, Object.class, rubyReceiver, methodName, block, null, args); + return call(MethodType.CALLMETHOD_WITHBLOCK, Object.class, getReceiverObject(receiver), methodName, block, null, args); } catch (InvokeFailedException e) { throw e; } catch (Throwable e) { @@ -287,9 +274,9 @@ public T runRubyMethod(Class returnType, Object receiver, String methodNa } } - private T call(MethodType type, Class returnType, RubyObject rubyReceiver, String methodName, Block block, EmbedEvalUnit unit, Object... args) { - if (methodName == null || methodName.length()==0) { - return null; + private T call(MethodType type, Class returnType, IRubyObject rubyReceiver, String methodName, Block block, EmbedEvalUnit unit, Object... args) { + if (methodName == null || methodName.isEmpty()) { + throw new IllegalArgumentException("no method name"); } final Ruby runtime = container.getProvider().getRuntime(); @@ -304,13 +291,13 @@ private T call(MethodType type, Class returnType, RubyObject rubyReceiver } try { - IRubyObject result = callEachType(type, rubyReceiver, methodName, block, args); + IRubyObject result = callImpl(runtime, type, rubyReceiver, methodName, block, args); if (sharing_variables) { container.getVarMap().retrieve(rubyReceiver); } - if (!(result instanceof RubyNil) && returnType != null) { + if (returnType != null) { Object ret = JavaEmbedUtils.rubyToJava(runtime, result, returnType); - return ret != null ? returnType.cast(ret) : null; + return returnType.cast(ret); } return null; } catch (RaiseException e) { @@ -325,19 +312,15 @@ private T call(MethodType type, Class returnType, RubyObject rubyReceiver } } - private RubyObject getReceiverObject(Object receiver) { - Ruby runtime = container.getProvider().getRuntime(); - if (receiver == null || !(receiver instanceof IRubyObject)) { - return (RubyObject)runtime.getTopSelf(); - } - else if (receiver instanceof RubyObject) return (RubyObject)receiver; - else return (RubyObject)((IRubyObject)receiver).getRuntime().getTopSelf(); + private IRubyObject getReceiverObject(Object receiver) { + if (receiver instanceof IRubyObject) return (IRubyObject) receiver; + return container.getProvider().getRuntime().getTopSelf(); } - private IRubyObject callEachType(MethodType type, IRubyObject rubyReceiver, String methodName, Block block, Object... args) { - Ruby runtime = rubyReceiver.getRuntime(); - IRubyObject[] rubyArgs = null; - if (args != null && args.length > 0) { + private static IRubyObject callImpl(final Ruby runtime, + MethodType type, IRubyObject rubyReceiver, String methodName, Block block, Object... args) { + IRubyObject[] rubyArgs; + if (args.length > 0) { rubyArgs = JavaUtil.convertJavaArrayToRuby(runtime, args); for (int i = 0; i < rubyArgs.length; i++) { IRubyObject obj = rubyArgs[i]; @@ -345,22 +328,23 @@ private IRubyObject callEachType(MethodType type, IRubyObject rubyReceiver, Stri rubyArgs[i] = Java.wrap(runtime, obj); } } + } else { + rubyArgs = IRubyObject.NULL_ARRAY; } ThreadContext context = runtime.getCurrentContext(); - switch (type) { - case CALLMETHOD_NOARG: - return Helpers.invoke(context, rubyReceiver, methodName); - case CALLMETHOD: - return Helpers.invoke(context, rubyReceiver, methodName, rubyArgs); - case CALLMETHOD_WITHBLOCK: - return Helpers.invoke(context, rubyReceiver, methodName, rubyArgs, block); - case CALLSUPER: - return Helpers.invokeSuper(context, rubyReceiver, rubyArgs, Block.NULL_BLOCK); - case CALLSUPER_WITHBLOCK: - return Helpers.invokeSuper(context, rubyReceiver, rubyArgs, block); - default: - break; - } - return null; + switch (type) { + case CALLMETHOD_NOARG: + return Helpers.invoke(context, rubyReceiver, methodName); + case CALLMETHOD: + return Helpers.invoke(context, rubyReceiver, methodName, rubyArgs); + case CALLMETHOD_WITHBLOCK: + return Helpers.invoke(context, rubyReceiver, methodName, rubyArgs, block); + case CALLSUPER: + return Helpers.invokeSuper(context, rubyReceiver, rubyArgs, Block.NULL_BLOCK); + case CALLSUPER_WITHBLOCK: + return Helpers.invokeSuper(context, rubyReceiver, rubyArgs, block); + default: + throw new AssertionError("unexpected method type"); + } } } diff --git a/core/src/test/java/org/jruby/embed/ScriptingContainerTest.java b/core/src/test/java/org/jruby/embed/ScriptingContainerTest.java index 0b06af69a84..7e3cb37b054 100644 --- a/core/src/test/java/org/jruby/embed/ScriptingContainerTest.java +++ b/core/src/test/java/org/jruby/embed/ScriptingContainerTest.java @@ -991,7 +991,7 @@ public void testCallMethod_4args_2() { logger1.info("callMethod(receiver, methodName, args, returnType)"); Object receiver = null; String methodName = ""; - Object[] args = null; + Object[] args = new Object[0]; Class returnType = null; ScriptingContainer instance = new ScriptingContainer(LocalContextScope.THREADSAFE); instance.setError(pstream); From b0b4ad0466041fc355d39e2dd6c2d2adb1f7775c Mon Sep 17 00:00:00 2001 From: kares Date: Tue, 3 Nov 2020 09:04:17 +0100 Subject: [PATCH 07/38] [refactor] actually do check down the line --- .../org/jruby/embed/internal/EmbedRubyObjectAdapterImpl.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/core/src/main/java/org/jruby/embed/internal/EmbedRubyObjectAdapterImpl.java b/core/src/main/java/org/jruby/embed/internal/EmbedRubyObjectAdapterImpl.java index a07f4b3e138..bbbb988b2db 100644 --- a/core/src/main/java/org/jruby/embed/internal/EmbedRubyObjectAdapterImpl.java +++ b/core/src/main/java/org/jruby/embed/internal/EmbedRubyObjectAdapterImpl.java @@ -275,10 +275,6 @@ public T runRubyMethod(Class returnType, Object receiver, String methodNa } private T call(MethodType type, Class returnType, IRubyObject rubyReceiver, String methodName, Block block, EmbedEvalUnit unit, Object... args) { - if (methodName == null || methodName.isEmpty()) { - throw new IllegalArgumentException("no method name"); - } - final Ruby runtime = container.getProvider().getRuntime(); final boolean sharing_variables = isSharingVariables(container); From 31a134fd517515a8b1a4ad5df889713867ff9b15 Mon Sep 17 00:00:00 2001 From: kares Date: Tue, 3 Nov 2020 09:10:39 +0100 Subject: [PATCH 08/38] [refactor] use Java's service loader --- .../jsr223/JRubyScriptEngineManager.java | 76 +++++------ .../org/jruby/embed/jsr223/ServiceFinder.java | 129 ------------------ 2 files changed, 35 insertions(+), 170 deletions(-) delete mode 100644 core/src/main/java/org/jruby/embed/jsr223/ServiceFinder.java diff --git a/core/src/main/java/org/jruby/embed/jsr223/JRubyScriptEngineManager.java b/core/src/main/java/org/jruby/embed/jsr223/JRubyScriptEngineManager.java index e89822a8332..a07581d562a 100644 --- a/core/src/main/java/org/jruby/embed/jsr223/JRubyScriptEngineManager.java +++ b/core/src/main/java/org/jruby/embed/jsr223/JRubyScriptEngineManager.java @@ -30,16 +30,18 @@ package org.jruby.embed.jsr223; import java.util.ArrayList; -import java.util.Collection; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; +import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Objects; +import java.util.ServiceLoader; import javax.script.Bindings; import javax.script.ScriptContext; import javax.script.ScriptEngine; import javax.script.ScriptEngineFactory; -import javax.script.ScriptException; import javax.script.SimpleBindings; /** @@ -54,51 +56,46 @@ */ public class JRubyScriptEngineManager { - static final String SERVICE = "META-INF/services/javax.script.ScriptEngineFactory"; - - private final Collection factories; + private final ScriptEngineFactory[] factories; private final Map nameMap; private final Map extensionMap; private final Map mimetypeMap; private Bindings globalMap; - public JRubyScriptEngineManager() throws ScriptException { + public JRubyScriptEngineManager() { this(null); } - public JRubyScriptEngineManager(ClassLoader loader) throws ScriptException { - nameMap = new HashMap(); - extensionMap = new HashMap(); - mimetypeMap = new HashMap(); + public JRubyScriptEngineManager(ClassLoader loader) { + nameMap = new HashMap<>(); + extensionMap = new HashMap<>(); + mimetypeMap = new HashMap<>(); globalMap = new SimpleBindings(); - try { - factories = new ServiceFinder(SERVICE, loader).getServices(); - if ( factories.isEmpty() ) { - System.err.println("no factory"); // TODO this is fatal, right? - } - prepareMaps(); - } - catch (Exception e) { - e.printStackTrace(); - throw new ScriptException(e); - } - } - private void prepareMaps() { - for (ScriptEngineFactory factory : factories) { - List names = factory.getNames(); - for (String name : names) { + ArrayList factories = new ArrayList<>(); + // lookup from: META-INF/services/javax.script.ScriptEngineFactory + Iterator i = ServiceLoader.load(javax.script.ScriptEngineFactory.class, loader).iterator(); + while (i.hasNext()) { + ScriptEngineFactory factory = i.next(); + + for (String name : factory.getNames()) { nameMap.put(name, factory); } - List extensions = factory.getExtensions(); - for (String extension : extensions) { + for (String extension : factory.getExtensions()) { extensionMap.put(extension, factory); } - List mimeTypes = factory.getMimeTypes(); - for (String mimeType : mimeTypes) { + for (String mimeType : factory.getMimeTypes()) { mimetypeMap.put(mimeType, factory); } + + factories.add(factory); } + + if (factories.isEmpty()) { + throw new IllegalStateException("no javax.script.ScriptEngineFactory service"); + } + + this.factories = factories.toArray(new ScriptEngineFactory[factories.size()]); } public void setBindings(final Bindings bindings) { @@ -122,7 +119,7 @@ public Object get(String key) { public ScriptEngine getEngineByName(String shortName) { if (shortName == null) { - throw new NullPointerException("Null symbolicName"); + throw new NullPointerException("Null name"); } ScriptEngineFactory factory = nameMap.get(shortName); if (factory == null) { @@ -160,27 +157,24 @@ public ScriptEngine getEngineByMimeType(String mimeType) { } public List getEngineFactories() { - return Collections.unmodifiableList(new ArrayList(factories)); + return Collections.unmodifiableList(Arrays.asList(factories)); } public void registerEngineName(String name, ScriptEngineFactory factory) { - if (name == null || factory == null) { - throw new NullPointerException("name and/or factory is null."); - } + Objects.requireNonNull(name, "name"); + Objects.requireNonNull(factory, "factory"); nameMap.put(name, factory); } public void registerEngineMimeType(String type, ScriptEngineFactory factory) { - if (type == null || factory == null) { - throw new NullPointerException("type and/or factory is null."); - } + Objects.requireNonNull(type, "type"); + Objects.requireNonNull(factory, "factory"); mimetypeMap.put(type, factory); } public void registerEngineExtension(String extension, ScriptEngineFactory factory) { - if (extension == null || factory == null) { - throw new NullPointerException("extension and/or factory is null."); - } + Objects.requireNonNull(extension, "extension"); + Objects.requireNonNull(factory, "factory"); extensionMap.put(extension, factory); } } \ No newline at end of file diff --git a/core/src/main/java/org/jruby/embed/jsr223/ServiceFinder.java b/core/src/main/java/org/jruby/embed/jsr223/ServiceFinder.java deleted file mode 100644 index c9624ecf8a1..00000000000 --- a/core/src/main/java/org/jruby/embed/jsr223/ServiceFinder.java +++ /dev/null @@ -1,129 +0,0 @@ -/** - * **** BEGIN LICENSE BLOCK ***** - * Version: EPL 2.0/GPL 2.0/LGPL 2.1 - * - * The contents of this file are subject to the Eclipse Public - * License Version 2.0 (the "License"); you may not use this file - * except in compliance with the License. You may obtain a copy of - * the License at http://www.eclipse.org/legal/epl-v20.html - * - * Software distributed under the License is distributed on an "AS - * IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or - * implied. See the License for the specific language governing - * rights and limitations under the License. - * - * Copyright (C) 2009 Yoko Harada - * - * Alternatively, the contents of this file may be used under the terms of - * either of the GNU General Public License Version 2 or later (the "GPL"), - * or the GNU Lesser General Public License Version 2.1 or later (the "LGPL"), - * in which case the provisions of the GPL or the LGPL are applicable instead - * of those above. If you wish to allow use of your version of this file only - * under the terms of either the GPL or the LGPL, and not to allow others to - * use your version of this file under the terms of the EPL, indicate your - * decision by deleting the provisions above and replace them with the notice - * and other provisions required by the GPL or the LGPL. If you do not delete - * the provisions above, a recipient may use your version of this file under - * the terms of any one of the EPL, the GPL or the LGPL. - * **** END LICENSE BLOCK ***** - */ -package org.jruby.embed.jsr223; - -import java.io.BufferedReader; -import java.io.IOException; -import java.io.InputStreamReader; -import java.net.URL; -import java.util.ArrayList; -import java.util.Collection; -import java.util.Enumeration; -import java.util.HashSet; -import java.util.List; -import java.util.Set; -import java.util.StringTokenizer; - -/** - * Finds SPI based services from classpath. - * - * @author Yoko Harada - */ -class ServiceFinder { - - private final Set services; - - ServiceFinder(final String serviceName, ClassLoader loader) throws IOException { - final Enumeration urls; - - if (loader == null) { - loader = ClassLoader.getSystemClassLoader(); - urls = ClassLoader.getSystemResources(serviceName); - } else { - urls = loader.getResources(serviceName); - } - - List classNames = readClassNames(urls); - services = instantiateClasses(classNames, loader); - } - - Collection getServices() { return services; } - - private static List readClassNames(Enumeration urls) { - String encoding = System.getProperty("file.encoding"); - ArrayList names = new ArrayList(); - while ( urls.hasMoreElements() ) { - URL url = null; - try { - url = urls.nextElement(); - BufferedReader reader = - new BufferedReader(new InputStreamReader(url.openStream(), encoding)); - String line; - while ((line = reader.readLine()) != null) { - if ((line = deleteComments(line)) != null) { - names.add(line); - } - } - } - catch (IOException e) { - System.err.println("Failed to get a class name from " + url); - continue; - } - } - return names; - } - - private static String deleteComments(final String line) { - if ( line.startsWith("#") ) return null; - if ( line.length() < 1 ) return null; - StringTokenizer st = new StringTokenizer(line, "#"); - return ((String) st.nextElement()).trim(); - } - - private Set instantiateClasses(final Collection names, - final ClassLoader loader) { - HashSet instances = new HashSet( names.size() ); - for (String name : names) { - try { - @SuppressWarnings("unchecked") - Class clazz = (Class) Class.forName(name, true, loader); - instances.add( clazz.getConstructor().newInstance() ); - } - catch (ClassNotFoundException e) { - System.err.println(name + " was not found"); - continue; - } - catch (InstantiationException e) { - System.err.println(name + " was not instantiated"); - continue; - } - catch (IllegalAccessException e) { - System.err.println(name + " committed illegal access"); - continue; - } - catch (Throwable e) { - System.err.println("failed to instantiate " + name); - continue; - } - } - return instances; - } - -} \ No newline at end of file From bc150bd0f384b3aadbcfa74ed7cd55c3dff50494 Mon Sep 17 00:00:00 2001 From: kares Date: Sat, 7 Nov 2020 14:35:48 +0100 Subject: [PATCH 09/38] [refactor] get rid of class -> name -> class --- .../internal/EmbedRubyInterfaceAdapterImpl.java | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/core/src/main/java/org/jruby/embed/internal/EmbedRubyInterfaceAdapterImpl.java b/core/src/main/java/org/jruby/embed/internal/EmbedRubyInterfaceAdapterImpl.java index 0be2beeb20d..ecd8bd7d35b 100644 --- a/core/src/main/java/org/jruby/embed/internal/EmbedRubyInterfaceAdapterImpl.java +++ b/core/src/main/java/org/jruby/embed/internal/EmbedRubyInterfaceAdapterImpl.java @@ -32,7 +32,6 @@ import org.jruby.Ruby; import org.jruby.RubyNil; import org.jruby.embed.EmbedRubyInterfaceAdapter; -import org.jruby.embed.InvokeFailedException; import org.jruby.embed.ScriptingContainer; import org.jruby.javasupport.JavaEmbedUtils; import org.jruby.javasupport.JavaUtil; @@ -73,16 +72,7 @@ public T getInstance(Object receiver, Class clazz) { IRubyObject rubyReceiver = JavaUtil.convertJavaToRuby(runtime, receiver); obj = JavaEmbedUtils.rubyToJava(runtime, rubyReceiver, clazz); } - final String name = clazz.getName(); - final ClassLoader loader = obj.getClass().getClassLoader(); - try { - @SuppressWarnings("unchecked") - Class klass = (Class) Class.forName(name, true, loader); - return klass.cast(obj); - } - catch (ClassNotFoundException e) { - throw new InvokeFailedException(e); - } + return clazz.cast(obj); } } From 68569f2ab3e2b03cf20e1a623547188900d4e6f1 Mon Sep 17 00:00:00 2001 From: kares Date: Sun, 8 Nov 2020 11:38:06 +0100 Subject: [PATCH 10/38] [refactor] lower type of local-variable scope --- .../java/org/jruby/embed/EmbedEvalUnit.java | 8 +++- .../jruby/embed/internal/BiVariableMap.java | 11 ++--- .../embed/internal/EmbedEvalUnitImpl.java | 9 ++-- .../internal/EmbedRubyObjectAdapterImpl.java | 3 +- .../internal/EmbedRubyRuntimeAdapterImpl.java | 41 +++++++++---------- .../embed/variable/VariableInterceptor.java | 24 +++++++++++ 6 files changed, 63 insertions(+), 33 deletions(-) diff --git a/core/src/main/java/org/jruby/embed/EmbedEvalUnit.java b/core/src/main/java/org/jruby/embed/EmbedEvalUnit.java index 194c66a2b26..5c572908bb1 100644 --- a/core/src/main/java/org/jruby/embed/EmbedEvalUnit.java +++ b/core/src/main/java/org/jruby/embed/EmbedEvalUnit.java @@ -31,6 +31,7 @@ import org.jruby.ast.Node; import org.jruby.javasupport.JavaEmbedUtils; +import org.jruby.runtime.DynamicScope; import org.jruby.runtime.scope.ManyVarsDynamicScope; /** @@ -54,5 +55,10 @@ public interface EmbedEvalUnit extends JavaEmbedUtils.EvalUnit { * * @return scope to refer local variables. */ - public ManyVarsDynamicScope getScope(); + DynamicScope getLocalVarScope(); + + @Deprecated + default ManyVarsDynamicScope getScope() { + return (ManyVarsDynamicScope) getLocalVarScope(); + } } diff --git a/core/src/main/java/org/jruby/embed/internal/BiVariableMap.java b/core/src/main/java/org/jruby/embed/internal/BiVariableMap.java index a82b7ade580..5c45182df0f 100644 --- a/core/src/main/java/org/jruby/embed/internal/BiVariableMap.java +++ b/core/src/main/java/org/jruby/embed/internal/BiVariableMap.java @@ -44,6 +44,7 @@ import org.jruby.embed.LocalVariableBehavior; import org.jruby.embed.variable.BiVariable; import org.jruby.embed.variable.VariableInterceptor; +import org.jruby.runtime.DynamicScope; import org.jruby.runtime.builtin.IRubyObject; import org.jruby.runtime.scope.ManyVarsDynamicScope; import static org.jruby.util.StringSupport.EMPTY_STRING_ARRAY; @@ -353,7 +354,7 @@ public Object put(Object receiver, String key, Object value) { public String[] getLocalVarNames() { if ( variables == null ) return EMPTY_STRING_ARRAY; - List localVarNames = new ArrayList(); + List localVarNames = new ArrayList<>(variables.size()); for ( final BiVariable var : variables ) { if ( var.getType() == BiVariable.Type.LocalVariable ) { localVarNames.add( var.getName() ); @@ -371,7 +372,7 @@ public String[] getLocalVarNames() { public IRubyObject[] getLocalVarValues() { if ( variables == null ) return IRubyObject.NULL_ARRAY; - List localVarValues = new ArrayList(); + List localVarValues = new ArrayList<>(variables.size()); for ( final BiVariable var : variables ) { if ( var.getType() == BiVariable.Type.LocalVariable ) { localVarValues.add( var.getRubyObject() ); @@ -380,8 +381,8 @@ public IRubyObject[] getLocalVarValues() { return localVarValues.toArray( new IRubyObject[ localVarValues.size() ] ); } - void inject(final ManyVarsDynamicScope scope) { - VariableInterceptor.inject(this, provider.getRuntime(), scope); + void inject(final DynamicScope scope) { + VariableInterceptor.inject(this, scope); } void retrieve(final IRubyObject receiver) { @@ -425,7 +426,7 @@ public Object removeFrom(final Object receiver, final Object key) { checkKey(key); final RubyObject robj = getReceiverObject(receiver); for ( int i = 0; i < size(); i++ ) { - if ( ((String) key).equals( varNames.get(i) ) ) { + if ( key.equals( varNames.get(i) ) ) { final BiVariable var = variables.get(i); if ( var.isReceiverIdentical(robj) ) { varNames.remove(i); diff --git a/core/src/main/java/org/jruby/embed/internal/EmbedEvalUnitImpl.java b/core/src/main/java/org/jruby/embed/internal/EmbedEvalUnitImpl.java index 93dff82e2e5..e3c7bbbd6c9 100644 --- a/core/src/main/java/org/jruby/embed/internal/EmbedEvalUnitImpl.java +++ b/core/src/main/java/org/jruby/embed/internal/EmbedEvalUnitImpl.java @@ -39,6 +39,7 @@ import org.jruby.embed.EvalFailedException; import org.jruby.embed.ScriptingContainer; import org.jruby.exceptions.RaiseException; +import org.jruby.runtime.DynamicScope; import org.jruby.runtime.ThreadContext; import org.jruby.runtime.builtin.IRubyObject; import org.jruby.runtime.scope.ManyVarsDynamicScope; @@ -57,14 +58,14 @@ public class EmbedEvalUnitImpl implements EmbedEvalUnit { private final ScriptingContainer container; private final Node node; - private final ManyVarsDynamicScope scope; + private final DynamicScope scope; private final Script script; - public EmbedEvalUnitImpl(ScriptingContainer container, Node node, ManyVarsDynamicScope scope) { + public EmbedEvalUnitImpl(ScriptingContainer container, Node node, DynamicScope scope) { this(container, node, scope, null); } - public EmbedEvalUnitImpl(ScriptingContainer container, Node node, ManyVarsDynamicScope scope, Script script) { + public EmbedEvalUnitImpl(ScriptingContainer container, Node node, DynamicScope scope, Script script) { this.container = container; this.node = node; this.scope = scope; @@ -86,7 +87,7 @@ public Node getNode() { * * @return a scope to refer local variables */ - public ManyVarsDynamicScope getScope() { + public DynamicScope getLocalVarScope() { return scope; } diff --git a/core/src/main/java/org/jruby/embed/internal/EmbedRubyObjectAdapterImpl.java b/core/src/main/java/org/jruby/embed/internal/EmbedRubyObjectAdapterImpl.java index bbbb988b2db..d91b5e0549a 100644 --- a/core/src/main/java/org/jruby/embed/internal/EmbedRubyObjectAdapterImpl.java +++ b/core/src/main/java/org/jruby/embed/internal/EmbedRubyObjectAdapterImpl.java @@ -53,6 +53,7 @@ import org.jruby.runtime.scope.ManyVarsDynamicScope; import static org.jruby.embed.internal.EmbedEvalUnitImpl.isSharingVariables; +import static org.jruby.embed.internal.EmbedRubyRuntimeAdapterImpl.createLocalVarScope; /** * Implementation of {@link EmbedRubyObjectAdapter}. Users get an instance of this @@ -281,7 +282,7 @@ private T call(MethodType type, Class returnType, IRubyObject rubyReceive if (sharing_variables) { ManyVarsDynamicScope scope; if (unit != null && unit.getScope() != null) scope = unit.getScope(); - else scope = EmbedRubyRuntimeAdapterImpl.getManyVarsDynamicScope(container, 0); + else scope = createLocalVarScope(runtime, container.getVarMap().getLocalVarNames()); container.getVarMap().inject(scope); runtime.getCurrentContext().pushScope(scope); } diff --git a/core/src/main/java/org/jruby/embed/internal/EmbedRubyRuntimeAdapterImpl.java b/core/src/main/java/org/jruby/embed/internal/EmbedRubyRuntimeAdapterImpl.java index 3e247ad2b44..e12d19c69c4 100644 --- a/core/src/main/java/org/jruby/embed/internal/EmbedRubyRuntimeAdapterImpl.java +++ b/core/src/main/java/org/jruby/embed/internal/EmbedRubyRuntimeAdapterImpl.java @@ -174,10 +174,9 @@ private EmbedEvalUnit runParser(Object input, String filename, int... lines) { line = lines[0]; } try { - ManyVarsDynamicScope scope = null; - final boolean sharing_variables = isSharingVariables(container); - if (sharing_variables) { - scope = getManyVarsDynamicScope(container, 0); + DynamicScope scope = null; + if (isSharingVariables(container)) { + scope = createLocalVarScope(runtime, container.getVarMap().getLocalVarNames()); } final Node node; if (input instanceof String) { @@ -212,19 +211,28 @@ private EmbedEvalUnit runParser(Object input, String filename, int... lines) { } } - static ManyVarsDynamicScope getManyVarsDynamicScope(ScriptingContainer container, int depth) { + public IRubyObject eval(Ruby runtime, String script) { + return adapter.eval(runtime, script); + } + + public EvalUnit parse(Ruby runtime, String script, String filename, int lineNumber) { + return adapter.parse(runtime, script, filename, lineNumber); + } + + public EvalUnit parse(Ruby runtime, InputStream istream, String filename, int lineNumber) { + return adapter.parse(runtime, istream, filename, lineNumber); + } + + static DynamicScope createLocalVarScope(Ruby runtime, final String[] varNames) { ManyVarsDynamicScope scope; - StaticScopeFactory scopeFactory = container.getProvider().getRuntime().getStaticScopeFactory(); + StaticScopeFactory scopeFactory = runtime.getStaticScopeFactory(); // root our parsing scope with a dummy scope StaticScope topStaticScope = scopeFactory.newLocalScope(null); - topStaticScope.setModule(container.getProvider().getRuntime().getObject()); + topStaticScope.setModule(runtime.getObject()); DynamicScope currentScope = new ManyVarsDynamicScope(topStaticScope, null); - String[] names4Injection = container.getVarMap().getLocalVarNames(); - StaticScope evalScope = names4Injection == null || names4Injection.length == 0 ? - scopeFactory.newEvalScope(currentScope.getStaticScope()) : - scopeFactory.newEvalScope(currentScope.getStaticScope(), names4Injection); + StaticScope evalScope = scopeFactory.newEvalScope(currentScope.getStaticScope(), varNames); scope = new ManyVarsDynamicScope(evalScope, currentScope); // JRUBY-5501: ensure we've set up a cref for the scope too @@ -233,15 +241,4 @@ static ManyVarsDynamicScope getManyVarsDynamicScope(ScriptingContainer container return scope; } - public IRubyObject eval(Ruby runtime, String script) { - return adapter.eval(runtime, script); - } - - public EvalUnit parse(Ruby runtime, String script, String filename, int lineNumber) { - return adapter.parse(runtime, script, filename, lineNumber); - } - - public EvalUnit parse(Ruby runtime, InputStream istream, String filename, int lineNumber) { - return adapter.parse(runtime, istream, filename, lineNumber); - } } diff --git a/core/src/main/java/org/jruby/embed/variable/VariableInterceptor.java b/core/src/main/java/org/jruby/embed/variable/VariableInterceptor.java index 775e27852d2..e9d7e628eca 100644 --- a/core/src/main/java/org/jruby/embed/variable/VariableInterceptor.java +++ b/core/src/main/java/org/jruby/embed/variable/VariableInterceptor.java @@ -36,6 +36,7 @@ import org.jruby.RubyObject; import org.jruby.embed.internal.BiVariableMap; import org.jruby.embed.LocalVariableBehavior; +import org.jruby.runtime.DynamicScope; import org.jruby.runtime.builtin.IRubyObject; import org.jruby.runtime.scope.ManyVarsDynamicScope; @@ -109,6 +110,7 @@ private static BiVariable resolve(BiVariable[] entries) { * @param runtime Ruby runtime * @param scope scope to inject local variable values */ + @Deprecated public static void inject(BiVariableMap map, Ruby runtime, ManyVarsDynamicScope scope) { // lvar might not be given while parsing but be given when evaluating. // to avoid ArrayIndexOutOfBoundsException, checks the length of scope.getValues() @@ -124,6 +126,28 @@ public static void inject(BiVariableMap map, Ruby runtime, ManyVarsDynamicScope for ( final BiVariable var : variables ) var.inject(); } + /** + * Injects variable values from Java to Ruby just before an evaluation or + * method invocation. + * + * @param map a variable map that has name-value pairs to be injected + * @param scope scope to inject local variable values + */ + public static void inject(BiVariableMap map, DynamicScope scope) { + // lvar might not be given while parsing but be given when evaluating. + // to avoid ArrayIndexOutOfBoundsException, checks the length of scope.getValues() + if (scope.getStaticScope().getNumberOfVariables() > 0) { + IRubyObject[] values4Injection = map.getLocalVarValues(); + if (values4Injection != null && values4Injection.length > 0) { + for (int i = 0; i < values4Injection.length; i++) { + scope.setValue(i, values4Injection[i], 0); + } + } + } + Collection variables = map.getVariables(); + for ( final BiVariable var : variables ) var.inject(); + } + /** * Retrieves variable/constant names and values after the evaluation or method * invocation. From e36d32403cf4d255885925f167c2719f0994a49a Mon Sep 17 00:00:00 2001 From: kares Date: Sun, 8 Nov 2020 11:59:58 +0100 Subject: [PATCH 11/38] [refactor] do not close a stream we do not own --- .../jruby/embed/internal/EmbedRubyRuntimeAdapterImpl.java | 8 -------- 1 file changed, 8 deletions(-) diff --git a/core/src/main/java/org/jruby/embed/internal/EmbedRubyRuntimeAdapterImpl.java b/core/src/main/java/org/jruby/embed/internal/EmbedRubyRuntimeAdapterImpl.java index e12d19c69c4..c2cd7fa506b 100644 --- a/core/src/main/java/org/jruby/embed/internal/EmbedRubyRuntimeAdapterImpl.java +++ b/core/src/main/java/org/jruby/embed/internal/EmbedRubyRuntimeAdapterImpl.java @@ -200,14 +200,6 @@ private EmbedEvalUnit runParser(Object input, String filename, int... lines) { throw new ParseFailedException(e.getMessage(), e); } catch (Throwable e) { throw new ParseFailedException(e); - } finally { - try { - if (input instanceof FileInputStream) { - ((InputStream)input).close(); - } - } catch (IOException ex) { - throw new ParseFailedException(ex); - } } } From 4028ae4a80f68a65cd94b692c7300216a01ea9ed Mon Sep 17 00:00:00 2001 From: kares Date: Sun, 8 Nov 2020 12:00:19 +0100 Subject: [PATCH 12/38] [refactor] input is never null here --- .../org/jruby/embed/internal/EmbedRubyRuntimeAdapterImpl.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/core/src/main/java/org/jruby/embed/internal/EmbedRubyRuntimeAdapterImpl.java b/core/src/main/java/org/jruby/embed/internal/EmbedRubyRuntimeAdapterImpl.java index c2cd7fa506b..e9ba1dc7fbc 100644 --- a/core/src/main/java/org/jruby/embed/internal/EmbedRubyRuntimeAdapterImpl.java +++ b/core/src/main/java/org/jruby/embed/internal/EmbedRubyRuntimeAdapterImpl.java @@ -158,9 +158,6 @@ public EmbedEvalUnit parse(InputStream istream, String filename, int... lines) { } private EmbedEvalUnit runParser(Object input, String filename, int... lines) { - if (input == null) { - return null; - } if (filename == null || filename.length() == 0) { filename = container.getScriptFilename(); } From f178cc24c56f9c67068f2ac055b38881f5ac1585 Mon Sep 17 00:00:00 2001 From: kares Date: Sun, 8 Nov 2020 12:56:04 +0100 Subject: [PATCH 13/38] [test] that parse throws org.jruby.exceptions.SyntaxError --- .../jruby/embed/ScriptingContainerTest.java | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/core/src/test/java/org/jruby/embed/ScriptingContainerTest.java b/core/src/test/java/org/jruby/embed/ScriptingContainerTest.java index 7e3cb37b054..9df7ab12ece 100644 --- a/core/src/test/java/org/jruby/embed/ScriptingContainerTest.java +++ b/core/src/test/java/org/jruby/embed/ScriptingContainerTest.java @@ -40,6 +40,7 @@ import org.jruby.embed.internal.SingleThreadLocalContextProvider; import org.jruby.embed.internal.SingletonLocalContextProvider; import org.jruby.embed.internal.ThreadSafeLocalContextProvider; +import org.jruby.exceptions.SyntaxError; import org.jruby.javasupport.JavaEmbedUtils; import org.jruby.runtime.Constants; import org.jruby.runtime.builtin.IRubyObject; @@ -561,8 +562,7 @@ public void testParse_3args_1() throws FileNotFoundException { try { instance.parse(reader, filename, 2); } catch (Exception e) { - logger1.info(sw.toString()); - assertTrue(sw.toString().contains(filename + ":7:")); + assertTrue(e.toString().contains(filename + ":7:")); } instance.getVarMap().clear(); @@ -590,9 +590,9 @@ public void testParse_3args_2() { try { result = instance.parse(type, filename, lines); - } catch (Throwable t) { - assertTrue(t.getCause() instanceof FileNotFoundException); - t.printStackTrace(new PrintStream(outStream)); + } catch (Exception e) { + assertTrue(e instanceof FileNotFoundException); + e.printStackTrace(new PrintStream(outStream)); } filename = basedir + "/core/src/test/ruby/org/jruby/embed/ruby/next_year.rb"; @@ -627,8 +627,7 @@ public void testParse_3args_2() { try { instance.parse(PathType.RELATIVE, filename, 2); } catch (Exception e) { - logger1.info(sw.toString()); - assertTrue(sw.toString().contains(filename + ":7:")); + assertTrue(e.toString().contains(filename + ":7:")); } instance.getVarMap().clear(); @@ -680,8 +679,7 @@ public void testParse_3args_3() throws FileNotFoundException { try { instance.parse(istream, filename, 2); } catch (Exception e) { - logger1.info(sw.toString()); - assertTrue(sw.toString().contains(filename + ":7:")); + assertTrue(e.toString().contains(filename + ":7:")); } instance.getVarMap().clear(); @@ -813,8 +811,8 @@ public void testRunScriptlet_PathType_String() { Object result; try { result = instance.parse(type, filename); - } catch (Throwable e) { - assertTrue(e.getCause() instanceof FileNotFoundException); + } catch (Exception e) { + assertTrue(e instanceof FileNotFoundException); e.printStackTrace(new PrintStream(outStream)); } @@ -2267,7 +2265,8 @@ public void testSetScriptFilename() { try { instance.runScriptlet("puts \"Hello"); } catch (RuntimeException e) { - assertTrue(sw.toString().contains(filename)); + assertTrue(e instanceof SyntaxError); + assertTrue(e.toString().contains(filename)); } instance.terminate(); From bd5eb02aa648114d00c84e5e5370769eb74cf61a Mon Sep 17 00:00:00 2001 From: kares Date: Sun, 8 Nov 2020 14:01:36 +0100 Subject: [PATCH 14/38] [test] we're expected to fail on callMethod("") --- .../jruby/embed/ScriptingContainerTest.java | 60 ++++++++----------- 1 file changed, 25 insertions(+), 35 deletions(-) diff --git a/core/src/test/java/org/jruby/embed/ScriptingContainerTest.java b/core/src/test/java/org/jruby/embed/ScriptingContainerTest.java index 9df7ab12ece..42a5d5fd91e 100644 --- a/core/src/test/java/org/jruby/embed/ScriptingContainerTest.java +++ b/core/src/test/java/org/jruby/embed/ScriptingContainerTest.java @@ -40,6 +40,8 @@ import org.jruby.embed.internal.SingleThreadLocalContextProvider; import org.jruby.embed.internal.SingletonLocalContextProvider; import org.jruby.embed.internal.ThreadSafeLocalContextProvider; +import org.jruby.exceptions.NoMethodError; +import org.jruby.exceptions.RaiseException; import org.jruby.exceptions.SyntaxError; import org.jruby.javasupport.JavaEmbedUtils; import org.jruby.runtime.Constants; @@ -908,7 +910,6 @@ public void testNewObjectAdapter() { public void testCallMethod_3args() { logger1.info("callMethod(receiver, methodName, returnType)"); Object receiver = null; - String methodName = ""; Class returnType = null; String[] paths = {basedir + "/lib/ruby/stdlib"}; ScriptingContainer instance = new ScriptingContainer(LocalContextScope.THREADSAFE); @@ -918,9 +919,12 @@ public void testCallMethod_3args() { instance.setWriter(writer); instance.setErrorWriter(writer); - Object expResult = null; - Object result = instance.callMethod(receiver, methodName, returnType); - assertEquals(expResult, result); + try { + Object result = instance.callMethod(receiver, "", returnType); + fail("expected to raise NoMethodError, but got result: " + result); + } catch (NoMethodError ex) { + // pass + } String filename = "src/test/ruby/org/jruby/embed/ruby/next_year_1.rb"; receiver = instance.runScriptlet(PathType.RELATIVE, filename); @@ -953,28 +957,21 @@ public void testCallMethod_3args() { @Test public void testCallMethod_4args_1() { logger1.info("callMethod(receiver, methodName, singleArg, returnType)"); - Object receiver = null; - String methodName = ""; - Object singleArg = null; - Class returnType = null; ScriptingContainer instance = new ScriptingContainer(LocalContextScope.THREADSAFE); instance.setError(pstream); instance.setOutput(pstream); instance.setWriter(writer); instance.setErrorWriter(writer); - Object expResult = null; - Object result = instance.callMethod(receiver, methodName, singleArg, returnType); - assertEquals(expResult, result); String filename = "src/test/ruby/org/jruby/embed/ruby/list_printer_1.rb"; - receiver = instance.runScriptlet(PathType.RELATIVE, filename); - methodName = "print_list"; + Object receiver = instance.runScriptlet(PathType.RELATIVE, filename); + String methodName = "print_list"; String[] hellos = {"你好", "こんにちは", "Hello", "Здравствуйте"}; - singleArg = Arrays.asList(hellos); + Object singleArg = Arrays.asList(hellos); StringWriter sw = new StringWriter(); instance.setWriter(sw); instance.callMethod(receiver, methodName, singleArg, null); - expResult = "Hello >> Здравствуйте >> こんにちは >> 你好: 4 in total"; + Object expResult = "Hello >> Здравствуйте >> こんにちは >> 你好: 4 in total"; assertEquals(expResult, sw.toString().trim()); instance.getVarMap().clear(); @@ -987,23 +984,16 @@ public void testCallMethod_4args_1() { @Test public void testCallMethod_4args_2() { logger1.info("callMethod(receiver, methodName, args, returnType)"); - Object receiver = null; - String methodName = ""; - Object[] args = new Object[0]; - Class returnType = null; ScriptingContainer instance = new ScriptingContainer(LocalContextScope.THREADSAFE); instance.setError(pstream); instance.setOutput(pstream); instance.setWriter(writer); instance.setErrorWriter(writer); - Object expResult = null; - Object result = instance.callMethod(receiver, methodName, args, returnType); - assertEquals(expResult, result); String filename = "src/test/ruby/org/jruby/embed/ruby/quadratic_formula.rb"; - receiver = instance.runScriptlet(PathType.RELATIVE, filename); - methodName = "solve"; - args = new Double[]{12.0, -21.0, -6.0}; + Object receiver = instance.runScriptlet(PathType.RELATIVE, filename); + String methodName = "solve"; + Object[] args = new Double[]{12.0, -21.0, -6.0}; List solutions = instance.callMethod(receiver, methodName, args, List.class); assertEquals(2, solutions.size()); assertEquals(new Double(-0.25), solutions.get(0)); @@ -1011,7 +1001,7 @@ public void testCallMethod_4args_2() { args = new Double[]{1.0, 1.0, 1.0}; try { - solutions = instance.callMethod(receiver, methodName, args, List.class); + instance.callMethod(receiver, methodName, args, List.class); } catch (RuntimeException e) { Throwable t = e.getCause(); assertTrue(t.getMessage().contains("RangeError")); @@ -1058,8 +1048,12 @@ public void testCallMethod_4args_3() { instance.setErrorWriter(writer); // Verify that empty message name returns null - Object result = instance.callMethod(null, "", returnType, unit); - assertEquals(null, result); + try { + Object result = instance.callMethod(null, "", returnType, unit); + fail("expected to raise NoMethodError, but got result: " + result); + } catch (NoMethodError ex) { + // pass + } String text = "songs:\n"+ @@ -1090,22 +1084,18 @@ public void testCallMethod_4args_3() { @Test public void testCallMethod_without_returnType() { logger1.info("callMethod no returnType"); - Object receiver = null; - String methodName = ""; ScriptingContainer instance = new ScriptingContainer(LocalContextScope.THREADSAFE); instance.setError(pstream); instance.setOutput(pstream); instance.setWriter(writer); instance.setErrorWriter(writer); - Object expResult = null; - Object result = instance.callMethod(receiver, methodName); - assertEquals(expResult, result); + String script = "def say_something\n" + "return \"Oh, well. I'm stucked\"" + "end"; - receiver = instance.runScriptlet(script); - methodName = "say_something"; + Object receiver = instance.runScriptlet(script); + String methodName = "say_something"; String something = (String)instance.callMethod(receiver, methodName); assertEquals("Oh, well. I'm stucked", something); From 6b3ebe7fde6d2fce051dadc328b0e419c959febf Mon Sep 17 00:00:00 2001 From: kares Date: Sun, 8 Nov 2020 14:25:14 +0100 Subject: [PATCH 15/38] [test] adjust exception expected - no longer wrapping --- core/src/test/java/org/jruby/embed/ScriptingContainerTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core/src/test/java/org/jruby/embed/ScriptingContainerTest.java b/core/src/test/java/org/jruby/embed/ScriptingContainerTest.java index 42a5d5fd91e..c2b09151eb4 100644 --- a/core/src/test/java/org/jruby/embed/ScriptingContainerTest.java +++ b/core/src/test/java/org/jruby/embed/ScriptingContainerTest.java @@ -1003,8 +1003,7 @@ public void testCallMethod_4args_2() { try { instance.callMethod(receiver, methodName, args, List.class); } catch (RuntimeException e) { - Throwable t = e.getCause(); - assertTrue(t.getMessage().contains("RangeError")); + assertTrue(e.getMessage().contains("RangeError")); } instance.getVarMap().clear(); From cab21854c612e1c6f13521ca49ff3d5bab202210 Mon Sep 17 00:00:00 2001 From: kares Date: Sun, 8 Nov 2020 15:05:45 +0100 Subject: [PATCH 16/38] [test] ScriptEngine behavior on missing method call --- .../org/jruby/embed/jsr223/JRubyEngineTest.java | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/core/src/test/java/org/jruby/embed/jsr223/JRubyEngineTest.java b/core/src/test/java/org/jruby/embed/jsr223/JRubyEngineTest.java index 33a7e4a5096..e8d572ec20f 100644 --- a/core/src/test/java/org/jruby/embed/jsr223/JRubyEngineTest.java +++ b/core/src/test/java/org/jruby/embed/jsr223/JRubyEngineTest.java @@ -56,6 +56,7 @@ import org.jruby.embed.PositionFunction; import org.jruby.embed.RadioActiveDecay; import org.jruby.embed.internal.BiVariableMap; +import org.jruby.exceptions.NoMethodError; import org.junit.After; import org.junit.AfterClass; import org.junit.Before; @@ -598,6 +599,18 @@ public void testInvokeMethod() throws Exception { instance.getBindings(ScriptContext.ENGINE_SCOPE).clear(); } + @Test + public void testInvokeMethodNonExistent() throws ScriptException { + ScriptEngine instance = newScriptEngine(); + Object receiver = instance.eval("self"); + try { + ((Invocable)instance).invokeMethod(receiver, "non_existent"); + fail("Expected NoSuchMethodException"); + } catch (NoSuchMethodException ex) { + assertTrue(ex.getCause() instanceof NoMethodError); + } + } + /** * Test of invokeMethod method throwing an exception with a null message. */ From e973924be03201c22ff7dfa38f62953eae46d327 Mon Sep 17 00:00:00 2001 From: kares Date: Sun, 8 Nov 2020 15:22:57 +0100 Subject: [PATCH 17/38] [refactor] avoid (unnecesarily) interning strings --- .../main/java/org/jruby/embed/internal/BiVariableMap.java | 7 +++---- .../java/org/jruby/embed/variable/AbstractVariable.java | 4 ++-- .../java/org/jruby/embed/variable/InstanceVariable.java | 2 +- .../java/org/jruby/embed/variable/LocalGlobalVariable.java | 2 +- 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/org/jruby/embed/internal/BiVariableMap.java b/core/src/main/java/org/jruby/embed/internal/BiVariableMap.java index 5c45182df0f..3214cc0ad9a 100644 --- a/core/src/main/java/org/jruby/embed/internal/BiVariableMap.java +++ b/core/src/main/java/org/jruby/embed/internal/BiVariableMap.java @@ -331,16 +331,15 @@ public Object put(String key, Object value) { public Object put(Object receiver, String key, Object value) { checkKey(key); final RubyObject robj = getReceiverObject(receiver); - final String name = key.intern(); - BiVariable var = getVariable(robj, name); + BiVariable var = getVariable(robj, key); Object oldValue = null; if ( var != null ) { // updates oldValue = var.getJavaObject(); var.setJavaObject(robj.getRuntime(), value); } else { // creates new value - var = VariableInterceptor.getVariableInstance(provider.getLocalVariableBehavior(), robj, name, value); - if ( var != null ) update(name, var); + var = VariableInterceptor.getVariableInstance(provider.getLocalVariableBehavior(), robj, key, value); + if ( var != null ) update(key, var); } return oldValue; } diff --git a/core/src/main/java/org/jruby/embed/variable/AbstractVariable.java b/core/src/main/java/org/jruby/embed/variable/AbstractVariable.java index ac13d8c25f6..48037e69b8f 100644 --- a/core/src/main/java/org/jruby/embed/variable/AbstractVariable.java +++ b/core/src/main/java/org/jruby/embed/variable/AbstractVariable.java @@ -64,7 +64,7 @@ abstract class AbstractVariable implements BiVariable { */ protected AbstractVariable(IRubyObject receiver, String name, boolean fromRuby) { this.receiver = receiver; - this.name = name.intern(); + this.name = name; this.fromRuby = fromRuby; } @@ -79,7 +79,7 @@ protected AbstractVariable(IRubyObject receiver, String name, boolean fromRuby) */ protected AbstractVariable(IRubyObject receiver, String name, boolean fromRuby, IRubyObject rubyObject) { this.receiver = receiver; - this.name = name.intern(); + this.name = name; this.fromRuby = fromRuby; this.rubyObject = rubyObject; } diff --git a/core/src/main/java/org/jruby/embed/variable/InstanceVariable.java b/core/src/main/java/org/jruby/embed/variable/InstanceVariable.java index 9638099bcbb..22691f67e5c 100644 --- a/core/src/main/java/org/jruby/embed/variable/InstanceVariable.java +++ b/core/src/main/java/org/jruby/embed/variable/InstanceVariable.java @@ -166,7 +166,7 @@ public static boolean isValidName(Object name) { */ @Override public void inject() { - ((RubyObject) getReceiver()).setInstanceVariable(name.intern(), getRubyObject()); + ((RubyObject) getReceiver()).setInstanceVariable(name, getRubyObject()); } /** diff --git a/core/src/main/java/org/jruby/embed/variable/LocalGlobalVariable.java b/core/src/main/java/org/jruby/embed/variable/LocalGlobalVariable.java index df4765498a4..c9486114134 100644 --- a/core/src/main/java/org/jruby/embed/variable/LocalGlobalVariable.java +++ b/core/src/main/java/org/jruby/embed/variable/LocalGlobalVariable.java @@ -114,7 +114,7 @@ public static void retrieveByKey(final Ruby runtime, final GlobalVariables globalVars = runtime.getGlobalVariables(); // if the specified key doesn't exist, this method is called before the // evaluation. Don't update value in this case. - final String gName = ("$" + name).intern(); + final String gName = ('$' + name); if ( ! globalVars.getNames().contains(gName) ) return; // the specified key is found, so let's update From ec4c1ab43b4746507324e7a150a9efaac9a27c52 Mon Sep 17 00:00:00 2001 From: kares Date: Sun, 8 Nov 2020 15:23:16 +0100 Subject: [PATCH 18/38] [refactor] cleanup method into a one liner --- .../main/java/org/jruby/embed/internal/BiVariableMap.java | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/core/src/main/java/org/jruby/embed/internal/BiVariableMap.java b/core/src/main/java/org/jruby/embed/internal/BiVariableMap.java index 3214cc0ad9a..bdc0c08a4e0 100644 --- a/core/src/main/java/org/jruby/embed/internal/BiVariableMap.java +++ b/core/src/main/java/org/jruby/embed/internal/BiVariableMap.java @@ -231,11 +231,7 @@ public Object get(Object receiver, Object key) { } private RubyObject getReceiverObject(final Object receiver) { - if ( receiver instanceof RubyObject ) return (RubyObject) receiver; - //if ( receiver instanceof IRubyObject ) { - // return (RubyObject) ( (IRubyObject) receiver ).getRuntime().getTopSelf(); - //} - return getTopSelf(); + return receiver instanceof RubyObject ? (RubyObject) receiver : getTopSelf(); } private RubyObject getTopSelf() { From 5e1a33ba15cf381c9082434c62bd26f45589e7a3 Mon Sep 17 00:00:00 2001 From: kares Date: Sun, 8 Nov 2020 15:24:25 +0100 Subject: [PATCH 19/38] [feat] BREAKING: avoid InvokeFailedException wrapping --- .../jruby/embed/InvokeFailedException.java | 1 + .../internal/EmbedRubyObjectAdapterImpl.java | 127 ++++-------------- 2 files changed, 25 insertions(+), 103 deletions(-) diff --git a/core/src/main/java/org/jruby/embed/InvokeFailedException.java b/core/src/main/java/org/jruby/embed/InvokeFailedException.java index 38ea14e91bb..1106a80b4c9 100644 --- a/core/src/main/java/org/jruby/embed/InvokeFailedException.java +++ b/core/src/main/java/org/jruby/embed/InvokeFailedException.java @@ -35,6 +35,7 @@ * * @author Yoko Harada */ +@Deprecated public class InvokeFailedException extends RuntimeException { public InvokeFailedException(Throwable cause) { super(cause); diff --git a/core/src/main/java/org/jruby/embed/internal/EmbedRubyObjectAdapterImpl.java b/core/src/main/java/org/jruby/embed/internal/EmbedRubyObjectAdapterImpl.java index d91b5e0549a..65ac5676ccb 100644 --- a/core/src/main/java/org/jruby/embed/internal/EmbedRubyObjectAdapterImpl.java +++ b/core/src/main/java/org/jruby/embed/internal/EmbedRubyObjectAdapterImpl.java @@ -37,20 +37,18 @@ import org.jruby.RubyString; import org.jruby.embed.EmbedEvalUnit; import org.jruby.embed.EmbedRubyObjectAdapter; -import org.jruby.embed.InvokeFailedException; import org.jruby.embed.ScriptingContainer; import org.jruby.embed.variable.BiVariable; import org.jruby.embed.variable.InstanceVariable; -import org.jruby.exceptions.RaiseException; import org.jruby.javasupport.Java; import org.jruby.javasupport.JavaEmbedUtils; import org.jruby.javasupport.JavaObject; import org.jruby.javasupport.JavaUtil; +import org.jruby.runtime.DynamicScope; import org.jruby.runtime.Helpers; import org.jruby.runtime.Block; import org.jruby.runtime.ThreadContext; import org.jruby.runtime.builtin.IRubyObject; -import org.jruby.runtime.scope.ManyVarsDynamicScope; import static org.jruby.embed.internal.EmbedEvalUnitImpl.isSharingVariables; import static org.jruby.embed.internal.EmbedRubyRuntimeAdapterImpl.createLocalVarScope; @@ -144,134 +142,62 @@ public IRubyObject callSuper(IRubyObject receiver, IRubyObject[] args, Block blo } public T callMethod(Object receiver, String methodName, Class returnType) { - try { - return call(MethodType.CALLMETHOD_NOARG, returnType, getReceiverObject(receiver), methodName, null, null); - } catch (InvokeFailedException e) { - throw e; - } catch (Throwable e) { - throw new InvokeFailedException(e); - } + return call(MethodType.CALLMETHOD_NOARG, returnType, getReceiverObject(receiver), methodName, null, null); } public T callMethod(Object receiver, String methodName, Object singleArg, Class returnType) { - try { - return call(MethodType.CALLMETHOD, returnType, getReceiverObject(receiver), methodName, null, null, singleArg); - } catch (InvokeFailedException e) { - throw e; - } catch (Throwable e) { - throw new InvokeFailedException(e); - } + return call(MethodType.CALLMETHOD, returnType, getReceiverObject(receiver), methodName, null, null, singleArg); } public T callMethod(Object receiver, String methodName, Object[] args, Class returnType) { - try { - return call(MethodType.CALLMETHOD, returnType, getReceiverObject(receiver), methodName, null, null, args); - } catch (InvokeFailedException e) { - throw e; - } catch (Throwable e) { - throw new InvokeFailedException(e); - } + return call(MethodType.CALLMETHOD, returnType, getReceiverObject(receiver), methodName, null, null, args); } public T callMethod(Object receiver, String methodName, Object[] args, Block block, Class returnType) { - try { - return call(MethodType.CALLMETHOD_WITHBLOCK, returnType, getReceiverObject(receiver), methodName, block, null, args); - } catch (InvokeFailedException e) { - throw e; - } catch (Throwable e) { - throw new InvokeFailedException(e); - } + return call(MethodType.CALLMETHOD_WITHBLOCK, returnType, getReceiverObject(receiver), methodName, block, null, args); } public T callMethod(Object receiver, String methodName, Class returnType, EmbedEvalUnit unit) { - try { - return call(MethodType.CALLMETHOD_NOARG, returnType, getReceiverObject(receiver), methodName, null, unit); - } catch (InvokeFailedException e) { - throw e; - } catch (Throwable e) { - throw new InvokeFailedException(e); - } + return call(MethodType.CALLMETHOD_NOARG, returnType, getReceiverObject(receiver), methodName, null, unit); } public T callMethod(Object receiver, String methodName, Object[] args, Class returnType, EmbedEvalUnit unit) { - try { - return call(MethodType.CALLMETHOD, returnType, getReceiverObject(receiver), methodName, null, unit, args); - } catch (InvokeFailedException e) { - throw e; - } catch (Throwable e) { - throw new InvokeFailedException(e); - } + return call(MethodType.CALLMETHOD, returnType, getReceiverObject(receiver), methodName, null, unit, args); } public T callMethod(Object receiver, String methodName, Object[] args, Block block, Class returnType, EmbedEvalUnit unit) { - try { - return call(MethodType.CALLMETHOD_WITHBLOCK, returnType, getReceiverObject(receiver), methodName, block, unit, args); - } catch (InvokeFailedException e) { - throw e; - } catch (Throwable e) { - throw new InvokeFailedException(e); - } + return call(MethodType.CALLMETHOD_WITHBLOCK, returnType, getReceiverObject(receiver), methodName, block, unit, args); } public T callSuper(Object receiver, Object[] args, Class returnType) { - try { - return call(MethodType.CALLSUPER, returnType, getReceiverObject(receiver), null, null, null, args); - } catch (InvokeFailedException e) { - throw e; - } catch (Throwable e) { - throw new InvokeFailedException(e); - } + return call(MethodType.CALLSUPER, returnType, getReceiverObject(receiver), null, null, null, args); } public T callSuper(Object receiver, Object[] args, Block block, Class returnType) { - try { - return call(MethodType.CALLSUPER_WITHBLOCK, returnType, getReceiverObject(receiver), null, block, null, args); - } catch (InvokeFailedException e) { - throw e; - } catch (Throwable e) { - throw new InvokeFailedException(e); - } + return call(MethodType.CALLSUPER_WITHBLOCK, returnType, getReceiverObject(receiver), null, block, null, args); } public Object callMethod(Object receiver, String methodName, Object... args) { - try { - if (args.length == 0) { - return call(MethodType.CALLMETHOD_NOARG, Object.class, getReceiverObject(receiver), methodName, null, null); - } else { - return call(MethodType.CALLMETHOD, Object.class, getReceiverObject(receiver), methodName, null, null, args); - } - } catch (InvokeFailedException e) { - throw e; - } catch (Throwable e) { - throw new InvokeFailedException(e); + if (args.length == 0) { + return call(MethodType.CALLMETHOD_NOARG, Object.class, getReceiverObject(receiver), methodName, null, null); + } else { + return call(MethodType.CALLMETHOD, Object.class, getReceiverObject(receiver), methodName, null, null, args); } } public Object callMethod(Object receiver, String methodName, Block block, Object... args) { - try { - if (args.length == 0) { - throw new IllegalArgumentException("needs at least one argument in a method"); - } - return call(MethodType.CALLMETHOD_WITHBLOCK, Object.class, getReceiverObject(receiver), methodName, block, null, args); - } catch (InvokeFailedException e) { - throw e; - } catch (Throwable e) { - throw new InvokeFailedException(e); + if (args.length == 0) { + throw new IllegalArgumentException("needs at least one argument in a method"); } + return call(MethodType.CALLMETHOD_WITHBLOCK, Object.class, getReceiverObject(receiver), methodName, block, null, args); } public T runRubyMethod(Class returnType, Object receiver, String methodName, Block block, Object... args) { - try { - RubyObject rubyReceiver = (RubyObject)JavaEmbedUtils.javaToRuby(container.getProvider().getRuntime(), receiver); - if (args.length == 0) { - return call(MethodType.CALLMETHOD_NOARG, returnType, rubyReceiver, methodName, block, null); - } else { - return call(MethodType.CALLMETHOD, returnType, rubyReceiver, methodName, block, null, args); - } - } catch (InvokeFailedException e) { - throw e; - } catch (Throwable e) { - throw new InvokeFailedException(e); + RubyObject rubyReceiver = (RubyObject)JavaEmbedUtils.javaToRuby(container.getProvider().getRuntime(), receiver); + if (args.length == 0) { + return call(MethodType.CALLMETHOD_NOARG, returnType, rubyReceiver, methodName, block, null); + } else { + return call(MethodType.CALLMETHOD, returnType, rubyReceiver, methodName, block, null, args); } } @@ -280,8 +206,8 @@ private T call(MethodType type, Class returnType, IRubyObject rubyReceive final boolean sharing_variables = isSharingVariables(container); if (sharing_variables) { - ManyVarsDynamicScope scope; - if (unit != null && unit.getScope() != null) scope = unit.getScope(); + DynamicScope scope; + if (unit != null && unit.getLocalVarScope() != null) scope = unit.getLocalVarScope(); else scope = createLocalVarScope(runtime, container.getVarMap().getLocalVarNames()); container.getVarMap().inject(scope); runtime.getCurrentContext().pushScope(scope); @@ -297,11 +223,6 @@ private T call(MethodType type, Class returnType, IRubyObject rubyReceive return returnType.cast(ret); } return null; - } catch (RaiseException e) { - runtime.printError(e.getException()); - throw new InvokeFailedException(e.getMessage(), e); - } catch (Throwable e) { - throw new InvokeFailedException(e); } finally { if (sharing_variables) { runtime.getCurrentContext().popScope(); From 3306cc1fdf522ac022a99db1c79cd88742abf9ba Mon Sep 17 00:00:00 2001 From: kares Date: Sun, 8 Nov 2020 15:25:06 +0100 Subject: [PATCH 20/38] [feat] BREAKING: avoid embed ParseFailedException wrapping --- .../org/jruby/embed/ParseFailedException.java | 1 + .../internal/EmbedRubyRuntimeAdapterImpl.java | 50 ++++++++----------- 2 files changed, 23 insertions(+), 28 deletions(-) diff --git a/core/src/main/java/org/jruby/embed/ParseFailedException.java b/core/src/main/java/org/jruby/embed/ParseFailedException.java index 1d01aab3a42..ad84e725aad 100644 --- a/core/src/main/java/org/jruby/embed/ParseFailedException.java +++ b/core/src/main/java/org/jruby/embed/ParseFailedException.java @@ -36,6 +36,7 @@ * * @author Yoko Harada */ +@Deprecated public class ParseFailedException extends RuntimeException { public ParseFailedException(Throwable cause) { super(cause); diff --git a/core/src/main/java/org/jruby/embed/internal/EmbedRubyRuntimeAdapterImpl.java b/core/src/main/java/org/jruby/embed/internal/EmbedRubyRuntimeAdapterImpl.java index e9ba1dc7fbc..429100dbb7c 100644 --- a/core/src/main/java/org/jruby/embed/internal/EmbedRubyRuntimeAdapterImpl.java +++ b/core/src/main/java/org/jruby/embed/internal/EmbedRubyRuntimeAdapterImpl.java @@ -47,12 +47,10 @@ import org.jruby.embed.AttributeName; import org.jruby.embed.EmbedEvalUnit; import org.jruby.embed.EmbedRubyRuntimeAdapter; -import org.jruby.embed.ParseFailedException; import org.jruby.embed.PathType; import org.jruby.embed.ScriptingContainer; import org.jruby.embed.io.ReaderInputStream; import org.jruby.embed.util.SystemPropertyCatcher; -import org.jruby.exceptions.RaiseException; import org.jruby.internal.runtime.GlobalVariable; import org.jruby.internal.runtime.ValueAccessor; import org.jruby.javasupport.JavaEmbedUtils; @@ -60,6 +58,7 @@ import org.jruby.parser.StaticScope; import org.jruby.parser.StaticScopeFactory; import org.jruby.runtime.DynamicScope; +import org.jruby.runtime.Helpers; import org.jruby.runtime.IAccessor; import org.jruby.runtime.builtin.IRubyObject; import org.jruby.runtime.load.LoadService; @@ -141,7 +140,8 @@ public EmbedEvalUnit parse(PathType type, String filename, int... lines) { } return parse(istream, filename, lines); } catch (FileNotFoundException e) { - throw new ParseFailedException(e); + Helpers.throwException(e); + return null; } finally { if (istream != null) { try {istream.close();} catch (IOException ioe) {} @@ -170,34 +170,28 @@ private EmbedEvalUnit runParser(Object input, String filename, int... lines) { if (lines != null && lines.length > 0) { line = lines[0]; } - try { - DynamicScope scope = null; - if (isSharingVariables(container)) { - scope = createLocalVarScope(runtime, container.getVarMap().getLocalVarNames()); - } - final Node node; - if (input instanceof String) { - node = runtime.parseEval((String)input, filename, scope, line); + + DynamicScope scope = null; + if (isSharingVariables(container)) { + scope = createLocalVarScope(runtime, container.getVarMap().getLocalVarNames()); + } + final Node node; + if (input instanceof String) { + node = runtime.parseEval((String)input, filename, scope, line); + } else { + node = runtime.parseFile((InputStream)input, filename, scope, line); + } + CompileMode compileMode = runtime.getInstanceConfig().getCompileMode(); + if (compileMode == CompileMode.FORCE) { + // CON FIXME: We may need to force heap variables here so the compiled script uses our provided scope + Script script = runtime.tryCompile(node); + if (script != null) { + return new EmbedEvalUnitImpl(container, node, scope, script); } else { - node = runtime.parseFile((InputStream)input, filename, scope, line); - } - CompileMode compileMode = runtime.getInstanceConfig().getCompileMode(); - if (compileMode == CompileMode.FORCE) { - // CON FIXME: We may need to force heap variables here so the compiled script uses our provided scope - Script script = runtime.tryCompile(node); - if (script != null) { - return new EmbedEvalUnitImpl(container, node, scope, script); - } else { - return new EmbedEvalUnitImpl(container, node, scope); - } + return new EmbedEvalUnitImpl(container, node, scope); } - return new EmbedEvalUnitImpl(container, node, scope); - } catch (RaiseException e) { - runtime.printError(e.getException()); - throw new ParseFailedException(e.getMessage(), e); - } catch (Throwable e) { - throw new ParseFailedException(e); } + return new EmbedEvalUnitImpl(container, node, scope); } public IRubyObject eval(Ruby runtime, String script) { From 4b1bdb5c8dc9bdd0f2472c74f93e215eac46c466 Mon Sep 17 00:00:00 2001 From: kares Date: Sun, 8 Nov 2020 15:26:55 +0100 Subject: [PATCH 21/38] [feat] adjust JSR-223 impl to not wrapping Ruby exceptions --- .../org/jruby/embed/jsr223/JRubyEngine.java | 99 ++++++++++--------- 1 file changed, 54 insertions(+), 45 deletions(-) diff --git a/core/src/main/java/org/jruby/embed/jsr223/JRubyEngine.java b/core/src/main/java/org/jruby/embed/jsr223/JRubyEngine.java index db7b9bed178..3e3719fe0ed 100644 --- a/core/src/main/java/org/jruby/embed/jsr223/JRubyEngine.java +++ b/core/src/main/java/org/jruby/embed/jsr223/JRubyEngine.java @@ -30,6 +30,7 @@ package org.jruby.embed.jsr223; import java.io.Reader; +import java.util.Objects; import javax.script.Bindings; import javax.script.Compilable; import javax.script.CompiledScript; @@ -42,6 +43,8 @@ import javax.script.SimpleScriptContext; import org.jruby.embed.EmbedEvalUnit; import org.jruby.embed.ScriptingContainer; +import org.jruby.exceptions.NoMethodError; +import org.jruby.exceptions.RaiseException; import org.jruby.javasupport.JavaEmbedUtils; import org.jruby.runtime.builtin.IRubyObject; @@ -62,6 +65,7 @@ public class JRubyEngine implements Compilable, Invocable, ScriptEngine { this.context = new JRubyContext(container); } + @Override public CompiledScript compile(String script) throws ScriptException { if (script == null) { throw new NullPointerException("script is null"); @@ -69,6 +73,7 @@ public CompiledScript compile(String script) throws ScriptException { return new JRubyCompiledScript(container, this, script); } + @Override public CompiledScript compile(Reader reader) throws ScriptException { if (reader == null) { throw new NullPointerException("reader is null"); @@ -76,6 +81,7 @@ public CompiledScript compile(Reader reader) throws ScriptException { return new JRubyCompiledScript(container, this, reader); } + @Override public Object eval(String script, ScriptContext context) throws ScriptException { if (script == null || context == null) { throw new NullPointerException("either script or context is null"); @@ -100,20 +106,17 @@ public Object eval(String script, ScriptContext context) throws ScriptException } } - private ScriptException wrapException(Exception e) { - return new ScriptException(e); - } - + @Override public Object eval(Reader reader, ScriptContext context) throws ScriptException { - if (reader == null || context == null) { - throw new NullPointerException("either reader or context is null"); + Objects.requireNonNull(reader, "reader"); + Objects.requireNonNull(context, "context"); + + if (Utils.isClearVariablesOn(context)) { + container.clear(); } - String filename = Utils.getFilename(context); + final String filename = Utils.getFilename(context); + Utils.preEval(container, context); try { - if (Utils.isClearVariablesOn(context)) { - container.clear(); - } - Utils.preEval(container, context); EmbedEvalUnit unit = container.parse(reader, filename, Utils.getLineNumber(context)); IRubyObject ret = unit.run(); return JavaEmbedUtils.rubyToJava(ret); @@ -128,20 +131,24 @@ public Object eval(Reader reader, ScriptContext context) throws ScriptException } } + @Override public Object eval(String script, Bindings bindings) throws ScriptException { ScriptContext context = getScriptContext(bindings); return eval(script, context); } + @Override public Object eval(Reader reader, Bindings bindings) throws ScriptException { ScriptContext context = getScriptContext(bindings); return eval(reader, context); } + @Override public Object eval(String script) throws ScriptException { return eval(script, context); } + @Override public Object eval(Reader reader) throws ScriptException { return eval(reader, context); } @@ -164,86 +171,79 @@ protected ScriptContext getScriptContext(Bindings bindings) { return newContext; } + @Override public Object get(String key) { return context.getAttribute(key, ScriptContext.ENGINE_SCOPE); } + @Override public void put(String key, Object value) { context.getBindings(ScriptContext.ENGINE_SCOPE).put(key, value); } + @Override public Bindings getBindings(int scope) { return context.getBindings(scope); } + @Override public void setBindings(Bindings bindings, int scope) { context.setBindings(bindings, scope); } + @Override public Bindings createBindings() { return new SimpleBindings(); } + @Override public ScriptContext getContext() { return context; } - public void setContext(ScriptContext ctx) { - if (ctx == null) { - throw new NullPointerException("context is null"); - } - context = ctx; + @Override + public void setContext(ScriptContext context) { + Objects.requireNonNull(context, "context"); + this.context = context; } + @Override public ScriptEngineFactory getFactory() { return factory; } - public Object invokeMethod(Object receiver, String method, Object... args) - throws ScriptException, NoSuchMethodException { - if (method == null) { - throw new NullPointerException("method is null"); - } - if (receiver == null) { - throw new NullPointerException("receiver is null"); - } + @Override + public Object invokeMethod(Object receiver, String method, Object... args) throws ScriptException, NoSuchMethodException { + Objects.requireNonNull(method, "method"); + Objects.requireNonNull(receiver, "receiver"); + Utils.preEval(container, context); try { - Utils.preEval(container, context); if (args == null || args.length == 0) { return container.callMethod(receiver, method, Object.class); } return container.callMethod(receiver, method, args, Object.class); + } catch (NoMethodError e) { + throw wrapMethodException(e); } catch (Exception e) { - if (e.getCause() != null && e.getCause().getMessage() != null - && e.getCause().getMessage().contains("undefined method")) { - throw wrapMethodException(e); - } throw wrapException(e); } finally { Utils.postEval(container, context); } } - private static NoSuchMethodException wrapMethodException(Exception e) { - return (NoSuchMethodException) new NoSuchMethodException(e.getCause().getMessage()).initCause(e); - } - - public Object invokeFunction(String method, Object... args) - throws ScriptException, NoSuchMethodException { - if (method == null) { - throw new NullPointerException("method is null"); - } + @Override + public Object invokeFunction(String method, Object... args) throws ScriptException, NoSuchMethodException { + Objects.requireNonNull(method, "method"); + Utils.preEval(container, context); try { - Utils.preEval(container, context); + IRubyObject receiver = container.getProvider().getRuntime().getTopSelf(); if (args == null || args.length == 0) { - return container.callMethod(container.getProvider().getRuntime().getTopSelf(), method, Object.class); + return container.callMethod(receiver, method, Object.class); } - return container.callMethod(container.getProvider().getRuntime().getTopSelf(), method, args, Object.class); + return container.callMethod(receiver, method, args, Object.class); + } catch (NoMethodError e) { + throw wrapMethodException(e); } catch (Exception e) { - if (e.getCause() != null && e.getCause().getMessage() != null - && e.getCause().getMessage().contains("undefined method")) { - throw wrapMethodException(e); - } throw wrapException(e); } finally { Utils.postEval(container, context); @@ -257,4 +257,13 @@ public T getInterface(Class returnType) { public T getInterface(Object receiver, Class returnType) { return container.getInstance(receiver, returnType); } + + private static ScriptException wrapException(Exception e) { + return new ScriptException(e); + } + + private static NoSuchMethodException wrapMethodException(RaiseException e) { + return (NoSuchMethodException) new NoSuchMethodException(e.getMessage()).initCause(e); + } + } \ No newline at end of file From 313dc2e2971e88f9d182a30eb4d07ea8a3f8a2a4 Mon Sep 17 00:00:00 2001 From: kares Date: Fri, 4 Dec 2020 10:42:47 +0100 Subject: [PATCH 22/38] [refactor] deprecate enum usage - there's 2 types really --- .../org/jruby/embed/ScriptingContainer.java | 2 +- .../internal/EmbedRubyObjectAdapterImpl.java | 53 +++++++++---------- 2 files changed, 26 insertions(+), 29 deletions(-) diff --git a/core/src/main/java/org/jruby/embed/ScriptingContainer.java b/core/src/main/java/org/jruby/embed/ScriptingContainer.java index 55890917700..b68b55efe9f 100644 --- a/core/src/main/java/org/jruby/embed/ScriptingContainer.java +++ b/core/src/main/java/org/jruby/embed/ScriptingContainer.java @@ -1569,7 +1569,7 @@ public T callSuper(Object receiver, Object[] args, Block block, Class ret * @return an instance of requested Java type */ public T runRubyMethod(Class returnType, Object receiver, String methodName, Object... args) { - return objectAdapter.runRubyMethod(returnType, receiver, methodName, null, args); + return objectAdapter.runRubyMethod(returnType, receiver, methodName, Block.NULL_BLOCK, args); } /** diff --git a/core/src/main/java/org/jruby/embed/internal/EmbedRubyObjectAdapterImpl.java b/core/src/main/java/org/jruby/embed/internal/EmbedRubyObjectAdapterImpl.java index 65ac5676ccb..28314634fef 100644 --- a/core/src/main/java/org/jruby/embed/internal/EmbedRubyObjectAdapterImpl.java +++ b/core/src/main/java/org/jruby/embed/internal/EmbedRubyObjectAdapterImpl.java @@ -65,11 +65,14 @@ public class EmbedRubyObjectAdapterImpl implements EmbedRubyObjectAdapter { private final ScriptingContainer container; public enum MethodType { - CALLMETHOD_NOARG, CALLMETHOD, - CALLMETHOD_WITHBLOCK, CALLSUPER, - CALLSUPER_WITHBLOCK + @Deprecated + CALLMETHOD_NOARG, + @Deprecated + CALLMETHOD_WITHBLOCK, + @Deprecated + CALLSUPER_WITHBLOCK, } public EmbedRubyObjectAdapterImpl(ScriptingContainer container) { @@ -142,63 +145,56 @@ public IRubyObject callSuper(IRubyObject receiver, IRubyObject[] args, Block blo } public T callMethod(Object receiver, String methodName, Class returnType) { - return call(MethodType.CALLMETHOD_NOARG, returnType, getReceiverObject(receiver), methodName, null, null); + return call(MethodType.CALLMETHOD, returnType, getReceiverObject(receiver), methodName, null, null); } public T callMethod(Object receiver, String methodName, Object singleArg, Class returnType) { - return call(MethodType.CALLMETHOD, returnType, getReceiverObject(receiver), methodName, null, null, singleArg); + return call(MethodType.CALLMETHOD, returnType, getReceiverObject(receiver), methodName, Block.NULL_BLOCK, null, singleArg); } public T callMethod(Object receiver, String methodName, Object[] args, Class returnType) { - return call(MethodType.CALLMETHOD, returnType, getReceiverObject(receiver), methodName, null, null, args); + return call(MethodType.CALLMETHOD, returnType, getReceiverObject(receiver), methodName, Block.NULL_BLOCK, null, args); } public T callMethod(Object receiver, String methodName, Object[] args, Block block, Class returnType) { - return call(MethodType.CALLMETHOD_WITHBLOCK, returnType, getReceiverObject(receiver), methodName, block, null, args); + return call(MethodType.CALLMETHOD, returnType, getReceiverObject(receiver), methodName, block, null, args); } public T callMethod(Object receiver, String methodName, Class returnType, EmbedEvalUnit unit) { - return call(MethodType.CALLMETHOD_NOARG, returnType, getReceiverObject(receiver), methodName, null, unit); + return call(MethodType.CALLMETHOD, returnType, getReceiverObject(receiver), methodName, Block.NULL_BLOCK, unit); } public T callMethod(Object receiver, String methodName, Object[] args, Class returnType, EmbedEvalUnit unit) { - return call(MethodType.CALLMETHOD, returnType, getReceiverObject(receiver), methodName, null, unit, args); + return call(MethodType.CALLMETHOD, returnType, getReceiverObject(receiver), methodName, Block.NULL_BLOCK, unit, args); } public T callMethod(Object receiver, String methodName, Object[] args, Block block, Class returnType, EmbedEvalUnit unit) { - return call(MethodType.CALLMETHOD_WITHBLOCK, returnType, getReceiverObject(receiver), methodName, block, unit, args); + return call(MethodType.CALLMETHOD, returnType, getReceiverObject(receiver), methodName, block, unit, args); } public T callSuper(Object receiver, Object[] args, Class returnType) { - return call(MethodType.CALLSUPER, returnType, getReceiverObject(receiver), null, null, null, args); + return call(MethodType.CALLSUPER, returnType, getReceiverObject(receiver), null, Block.NULL_BLOCK, null, args); } public T callSuper(Object receiver, Object[] args, Block block, Class returnType) { - return call(MethodType.CALLSUPER_WITHBLOCK, returnType, getReceiverObject(receiver), null, block, null, args); + return call(MethodType.CALLSUPER, returnType, getReceiverObject(receiver), null, block, null, args); } public Object callMethod(Object receiver, String methodName, Object... args) { - if (args.length == 0) { - return call(MethodType.CALLMETHOD_NOARG, Object.class, getReceiverObject(receiver), methodName, null, null); - } else { - return call(MethodType.CALLMETHOD, Object.class, getReceiverObject(receiver), methodName, null, null, args); - } + return call(MethodType.CALLMETHOD, Object.class, getReceiverObject(receiver), methodName, Block.NULL_BLOCK, null, args); } public Object callMethod(Object receiver, String methodName, Block block, Object... args) { if (args.length == 0) { throw new IllegalArgumentException("needs at least one argument in a method"); } - return call(MethodType.CALLMETHOD_WITHBLOCK, Object.class, getReceiverObject(receiver), methodName, block, null, args); + return call(MethodType.CALLMETHOD, Object.class, getReceiverObject(receiver), methodName, block, null, args); } public T runRubyMethod(Class returnType, Object receiver, String methodName, Block block, Object... args) { - RubyObject rubyReceiver = (RubyObject)JavaEmbedUtils.javaToRuby(container.getProvider().getRuntime(), receiver); - if (args.length == 0) { - return call(MethodType.CALLMETHOD_NOARG, returnType, rubyReceiver, methodName, block, null); - } else { - return call(MethodType.CALLMETHOD, returnType, rubyReceiver, methodName, block, null, args); - } + assert block != null; + IRubyObject rubyReceiver = JavaEmbedUtils.javaToRuby(container.getProvider().getRuntime(), receiver); + return call(MethodType.CALLMETHOD, returnType, rubyReceiver, methodName, block, null, args); } private T call(MethodType type, Class returnType, IRubyObject rubyReceiver, String methodName, Block block, EmbedEvalUnit unit, Object... args) { @@ -251,14 +247,15 @@ private static IRubyObject callImpl(final Ruby runtime, } ThreadContext context = runtime.getCurrentContext(); switch (type) { - case CALLMETHOD_NOARG: - return Helpers.invoke(context, rubyReceiver, methodName); case CALLMETHOD: return Helpers.invoke(context, rubyReceiver, methodName, rubyArgs); - case CALLMETHOD_WITHBLOCK: - return Helpers.invoke(context, rubyReceiver, methodName, rubyArgs, block); case CALLSUPER: return Helpers.invokeSuper(context, rubyReceiver, rubyArgs, Block.NULL_BLOCK); + // deprecated - unused paths + case CALLMETHOD_NOARG: + return Helpers.invoke(context, rubyReceiver, methodName); + case CALLMETHOD_WITHBLOCK: + return Helpers.invoke(context, rubyReceiver, methodName, rubyArgs, block); case CALLSUPER_WITHBLOCK: return Helpers.invokeSuper(context, rubyReceiver, rubyArgs, block); default: From 89312ff6c111767187e2c7e340fe769b48c05436 Mon Sep 17 00:00:00 2001 From: kares Date: Fri, 4 Dec 2020 12:32:44 +0100 Subject: [PATCH 23/38] [refactor] meaningless null args passing in tests --- .../test/java/org/jruby/embed/jsr223/JRubyEngineTest.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/core/src/test/java/org/jruby/embed/jsr223/JRubyEngineTest.java b/core/src/test/java/org/jruby/embed/jsr223/JRubyEngineTest.java index e8d572ec20f..63cf924530d 100644 --- a/core/src/test/java/org/jruby/embed/jsr223/JRubyEngineTest.java +++ b/core/src/test/java/org/jruby/embed/jsr223/JRubyEngineTest.java @@ -577,7 +577,7 @@ public void testInvokeMethod() throws Exception { Reader reader = new FileReader(filename); Object receiver = instance.eval(reader); String method = "to_s"; - Object[] args = null; + Object[] args = new Object[0]; String expResult = "Cherry blossom is a round shaped,"; String result = (String) ((Invocable)instance).invokeMethod(receiver, method, args); assertTrue(result.startsWith(expResult)); @@ -645,8 +645,7 @@ public void testInvokeFunction() throws Exception { bindings.put("@month", 12); bindings.put("@day", 3); instance.setBindings(bindings, ScriptContext.ENGINE_SCOPE); - Object[] args = null; - Object result = ((Invocable)instance).invokeFunction(method, args); + Object result = ((Invocable)instance).invokeFunction(method); assertTrue(((String)result).startsWith("Happy") || ((String) result).startsWith("You have")); instance.getBindings(ScriptContext.ENGINE_SCOPE).clear(); From e56c28db91aa419941e23f60cbfd74fe18ccc61f Mon Sep 17 00:00:00 2001 From: kares Date: Fri, 4 Dec 2020 12:40:10 +0100 Subject: [PATCH 24/38] [refactor] get rid of MethodType enum completely --- .../internal/EmbedRubyObjectAdapterImpl.java | 116 ++++++++++-------- 1 file changed, 66 insertions(+), 50 deletions(-) diff --git a/core/src/main/java/org/jruby/embed/internal/EmbedRubyObjectAdapterImpl.java b/core/src/main/java/org/jruby/embed/internal/EmbedRubyObjectAdapterImpl.java index 28314634fef..a4c72002aa6 100644 --- a/core/src/main/java/org/jruby/embed/internal/EmbedRubyObjectAdapterImpl.java +++ b/core/src/main/java/org/jruby/embed/internal/EmbedRubyObjectAdapterImpl.java @@ -64,14 +64,12 @@ public class EmbedRubyObjectAdapterImpl implements EmbedRubyObjectAdapter { private final RubyObjectAdapter adapter = JavaEmbedUtils.newObjectAdapter(); private final ScriptingContainer container; + @Deprecated // not used public enum MethodType { CALLMETHOD, CALLSUPER, - @Deprecated CALLMETHOD_NOARG, - @Deprecated CALLMETHOD_WITHBLOCK, - @Deprecated CALLSUPER_WITHBLOCK, } @@ -99,7 +97,7 @@ public IRubyObject setInstanceVariable(IRubyObject obj, String variableName, IRu BiVariableMap map = container.getVarMap(); synchronized (map) { if (map.containsKey(variableName)) { - BiVariable bv = map.getVariable((RubyObject)container.getProvider().getRuntime().getTopSelf(), variableName); + BiVariable bv = map.getVariable((RubyObject) getTopSelf(), variableName); bv.setRubyObject(value); } else { InstanceVariable iv = new InstanceVariable(obj, variableName, value); @@ -113,7 +111,7 @@ public IRubyObject getInstanceVariable(IRubyObject obj, String variableName) { BiVariableMap map = container.getVarMap(); synchronized (map) { if (map.containsKey(variableName)) { - BiVariable bv = map.getVariable((RubyObject)container.getProvider().getRuntime().getTopSelf(), variableName); + BiVariable bv = map.getVariable((RubyObject) getTopSelf(), variableName); return bv.getRubyObject(); } } @@ -145,72 +143,66 @@ public IRubyObject callSuper(IRubyObject receiver, IRubyObject[] args, Block blo } public T callMethod(Object receiver, String methodName, Class returnType) { - return call(MethodType.CALLMETHOD, returnType, getReceiverObject(receiver), methodName, null, null); + return doInvokeMethod(returnType, getReceiverObject(receiver), methodName, Block.NULL_BLOCK, null); } public T callMethod(Object receiver, String methodName, Object singleArg, Class returnType) { - return call(MethodType.CALLMETHOD, returnType, getReceiverObject(receiver), methodName, Block.NULL_BLOCK, null, singleArg); + return doInvokeMethod(returnType, getReceiverObject(receiver), methodName, Block.NULL_BLOCK, null, singleArg); } public T callMethod(Object receiver, String methodName, Object[] args, Class returnType) { - return call(MethodType.CALLMETHOD, returnType, getReceiverObject(receiver), methodName, Block.NULL_BLOCK, null, args); + return doInvokeMethod(returnType, getReceiverObject(receiver), methodName, Block.NULL_BLOCK, null, args); } public T callMethod(Object receiver, String methodName, Object[] args, Block block, Class returnType) { - return call(MethodType.CALLMETHOD, returnType, getReceiverObject(receiver), methodName, block, null, args); + return doInvokeMethod(returnType, getReceiverObject(receiver), methodName, block, null, args); } public T callMethod(Object receiver, String methodName, Class returnType, EmbedEvalUnit unit) { - return call(MethodType.CALLMETHOD, returnType, getReceiverObject(receiver), methodName, Block.NULL_BLOCK, unit); + return doInvokeMethod(returnType, getReceiverObject(receiver), methodName, Block.NULL_BLOCK, unit); } public T callMethod(Object receiver, String methodName, Object[] args, Class returnType, EmbedEvalUnit unit) { - return call(MethodType.CALLMETHOD, returnType, getReceiverObject(receiver), methodName, Block.NULL_BLOCK, unit, args); + return doInvokeMethod(returnType, getReceiverObject(receiver), methodName, Block.NULL_BLOCK, unit, args); } public T callMethod(Object receiver, String methodName, Object[] args, Block block, Class returnType, EmbedEvalUnit unit) { - return call(MethodType.CALLMETHOD, returnType, getReceiverObject(receiver), methodName, block, unit, args); + return doInvokeMethod(returnType, getReceiverObject(receiver), methodName, block, unit, args); } public T callSuper(Object receiver, Object[] args, Class returnType) { - return call(MethodType.CALLSUPER, returnType, getReceiverObject(receiver), null, Block.NULL_BLOCK, null, args); + return doInvokeSuper(returnType, getReceiverObject(receiver), Block.NULL_BLOCK, null, args); } public T callSuper(Object receiver, Object[] args, Block block, Class returnType) { - return call(MethodType.CALLSUPER, returnType, getReceiverObject(receiver), null, block, null, args); + return doInvokeSuper(returnType, getReceiverObject(receiver), block, null, args); } public Object callMethod(Object receiver, String methodName, Object... args) { - return call(MethodType.CALLMETHOD, Object.class, getReceiverObject(receiver), methodName, Block.NULL_BLOCK, null, args); + return doInvokeMethod(Object.class, getReceiverObject(receiver), methodName, Block.NULL_BLOCK, null, args); } public Object callMethod(Object receiver, String methodName, Block block, Object... args) { - if (args.length == 0) { - throw new IllegalArgumentException("needs at least one argument in a method"); - } - return call(MethodType.CALLMETHOD, Object.class, getReceiverObject(receiver), methodName, block, null, args); + return doInvokeMethod(Object.class, getReceiverObject(receiver), methodName, block, null, args); } public T runRubyMethod(Class returnType, Object receiver, String methodName, Block block, Object... args) { assert block != null; IRubyObject rubyReceiver = JavaEmbedUtils.javaToRuby(container.getProvider().getRuntime(), receiver); - return call(MethodType.CALLMETHOD, returnType, rubyReceiver, methodName, block, null, args); + return doInvokeMethod(returnType, rubyReceiver, methodName, block, null, args); } - private T call(MethodType type, Class returnType, IRubyObject rubyReceiver, String methodName, Block block, EmbedEvalUnit unit, Object... args) { + private T doInvokeSuper(Class returnType, IRubyObject rubyReceiver, Block block, EmbedEvalUnit unit, Object... args) { final Ruby runtime = container.getProvider().getRuntime(); + final ThreadContext context = runtime.getCurrentContext(); final boolean sharing_variables = isSharingVariables(container); if (sharing_variables) { - DynamicScope scope; - if (unit != null && unit.getLocalVarScope() != null) scope = unit.getLocalVarScope(); - else scope = createLocalVarScope(runtime, container.getVarMap().getLocalVarNames()); - container.getVarMap().inject(scope); - runtime.getCurrentContext().pushScope(scope); + beforeSharingVariablesCall(context, unit); } try { - IRubyObject result = callImpl(runtime, type, rubyReceiver, methodName, block, args); + IRubyObject result = Helpers.invokeSuper(context, rubyReceiver, convertArgs(runtime, args), block); if (sharing_variables) { container.getVarMap().retrieve(rubyReceiver); } @@ -221,45 +213,69 @@ private T call(MethodType type, Class returnType, IRubyObject rubyReceive return null; } finally { if (sharing_variables) { - runtime.getCurrentContext().popScope(); + afterSharingVariablesCall(context); } } } + private T doInvokeMethod(Class returnType, IRubyObject rubyReceiver, String methodName, Block block, EmbedEvalUnit unit, Object... args) { + final Ruby runtime = container.getProvider().getRuntime(); + final ThreadContext context = runtime.getCurrentContext(); + final boolean sharing_variables = isSharingVariables(container); + + if (sharing_variables) { + beforeSharingVariablesCall(context, unit); + } + try { + IRubyObject result = Helpers.invoke(context, rubyReceiver, methodName, convertArgs(runtime, args), block); + if (sharing_variables) { + container.getVarMap().retrieve(rubyReceiver); + } + if (returnType != null) { + Object ret = JavaEmbedUtils.rubyToJava(runtime, result, returnType); + return returnType.cast(ret); + } + return null; + } finally { + if (sharing_variables) { + afterSharingVariablesCall(context); + } + } + } + + private void beforeSharingVariablesCall(final ThreadContext context, EmbedEvalUnit unit) { + DynamicScope scope; + if (unit != null && unit.getLocalVarScope() != null) scope = unit.getLocalVarScope(); + else scope = createLocalVarScope(context.runtime, container.getVarMap().getLocalVarNames()); + container.getVarMap().inject(scope); + context.pushScope(scope); + } + + private void afterSharingVariablesCall(final ThreadContext context) { + context.popScope(); + } + private IRubyObject getReceiverObject(Object receiver) { if (receiver instanceof IRubyObject) return (IRubyObject) receiver; + return getTopSelf(); + } + + private IRubyObject getTopSelf() { return container.getProvider().getRuntime().getTopSelf(); } - private static IRubyObject callImpl(final Ruby runtime, - MethodType type, IRubyObject rubyReceiver, String methodName, Block block, Object... args) { - IRubyObject[] rubyArgs; + private static IRubyObject[] convertArgs(final Ruby runtime, final Object[] args) { if (args.length > 0) { - rubyArgs = JavaUtil.convertJavaArrayToRuby(runtime, args); + IRubyObject[] rubyArgs = JavaUtil.convertJavaArrayToRuby(runtime, args); for (int i = 0; i < rubyArgs.length; i++) { IRubyObject obj = rubyArgs[i]; if (obj instanceof JavaObject) { rubyArgs[i] = Java.wrap(runtime, obj); } } - } else { - rubyArgs = IRubyObject.NULL_ARRAY; - } - ThreadContext context = runtime.getCurrentContext(); - switch (type) { - case CALLMETHOD: - return Helpers.invoke(context, rubyReceiver, methodName, rubyArgs); - case CALLSUPER: - return Helpers.invokeSuper(context, rubyReceiver, rubyArgs, Block.NULL_BLOCK); - // deprecated - unused paths - case CALLMETHOD_NOARG: - return Helpers.invoke(context, rubyReceiver, methodName); - case CALLMETHOD_WITHBLOCK: - return Helpers.invoke(context, rubyReceiver, methodName, rubyArgs, block); - case CALLSUPER_WITHBLOCK: - return Helpers.invokeSuper(context, rubyReceiver, rubyArgs, block); - default: - throw new AssertionError("unexpected method type"); + return rubyArgs; } + return IRubyObject.NULL_ARRAY; } + } From 26eef3871fdfa9f788c26a890e495c7237e673e7 Mon Sep 17 00:00:00 2001 From: kares Date: Fri, 4 Dec 2020 13:56:42 +0100 Subject: [PATCH 25/38] [refactor] get rid of unused CallMethodType enum --- .../jruby/embed/internal/CallMethodType.java | 45 ------------------- 1 file changed, 45 deletions(-) delete mode 100644 core/src/main/java/org/jruby/embed/internal/CallMethodType.java diff --git a/core/src/main/java/org/jruby/embed/internal/CallMethodType.java b/core/src/main/java/org/jruby/embed/internal/CallMethodType.java deleted file mode 100644 index 839162e6749..00000000000 --- a/core/src/main/java/org/jruby/embed/internal/CallMethodType.java +++ /dev/null @@ -1,45 +0,0 @@ -/** - * **** BEGIN LICENSE BLOCK ***** - * Version: EPL 2.0/GPL 2.0/LGPL 2.1 - * - * The contents of this file are subject to the Eclipse Public - * License Version 2.0 (the "License"); you may not use this file - * except in compliance with the License. You may obtain a copy of - * the License at http://www.eclipse.org/legal/epl-v20.html - * - * Software distributed under the License is distributed on an "AS - * IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or - * implied. See the License for the specific language governing - * rights and limitations under the License. - * - * Copyright (C) 2009 Yoko Harada - * - * Alternatively, the contents of this file may be used under the terms of - * either of the GNU General Public License Version 2 or later (the "GPL"), - * or the GNU Lesser General Public License Version 2.1 or later (the "LGPL"), - * in which case the provisions of the GPL or the LGPL are applicable instead - * of those above. If you wish to allow use of your version of this file only - * under the terms of either the GPL or the LGPL, and not to allow others to - * use your version of this file under the terms of the EPL, indicate your - * decision by deleting the provisions above and replace them with the notice - * and other provisions required by the GPL or the LGPL. If you do not delete - * the provisions above, a recipient may use your version of this file under - * the terms of any one of the EPL, the GPL or the LGPL. - * **** END LICENSE BLOCK ***** - */ -package org.jruby.embed.internal; - -import java.lang.annotation.ElementType; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; -import java.lang.annotation.Target; - -/** - * - * @author Yoko Harada - */ -@Retention(RetentionPolicy.RUNTIME) -@Target(ElementType.METHOD) -public @interface CallMethodType { - int methodType(); -} From 747e9c33f0d228232db98508df0273be9cbda37b Mon Sep 17 00:00:00 2001 From: kares Date: Fri, 4 Dec 2020 14:46:02 +0100 Subject: [PATCH 26/38] [test] adjust meaningless (null arg) tests should not expect behavior will null reader/filename --- .../jruby/embed/ScriptingContainerTest.java | 24 ++++++------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/core/src/test/java/org/jruby/embed/ScriptingContainerTest.java b/core/src/test/java/org/jruby/embed/ScriptingContainerTest.java index c2b09151eb4..37f9124ddd3 100644 --- a/core/src/test/java/org/jruby/embed/ScriptingContainerTest.java +++ b/core/src/test/java/org/jruby/embed/ScriptingContainerTest.java @@ -74,6 +74,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.logging.ConsoleHandler; import java.util.logging.Handler; @@ -535,22 +536,17 @@ public void testParse_String_intArr() { @Test public void testParse_3args_1() throws FileNotFoundException { logger1.info("parse(reader, filename, lines)"); - Reader reader = null; String filename = ""; - int[] lines = null; ScriptingContainer instance = new ScriptingContainer(LocalContextScope.THREADSAFE); instance.setError(pstream); instance.setOutput(pstream); instance.setWriter(writer); instance.setErrorWriter(writer); - EmbedEvalUnit expResult = null; - EmbedEvalUnit result = instance.parse(reader, filename, lines); - assertEquals(expResult, result); filename = basedir + "/core/src/test/ruby/org/jruby/embed/ruby/iteration.rb"; - reader = new FileReader(filename); + FileReader reader = new FileReader(filename); instance.put("@t", 2); - result = instance.parse(reader, filename); + EmbedEvalUnit result = instance.parse(reader, filename); IRubyObject ret = result.run(); String expStringResult = "Trick or Treat!\nTrick or Treat!\n\nHmmm...I'd like trick."; @@ -704,10 +700,8 @@ public void testRunScriptlet_String() { Object result = instance.runScriptlet(script); assertEquals(expResult, result); script = ""; - expResult = ""; result = instance.runScriptlet(script); - // Maybe bug. This should return "", but RubyNil. - //assertEquals(expResult, result); + assertNull(result); script = "# -*- coding: utf-8 -*-\n" + "def say_something()" + @@ -737,22 +731,18 @@ public void testRunScriptlet_String() { @Test public void testRunScriptlet_Reader_String() throws FileNotFoundException { logger1.info("runScriptlet(reader, filename)"); - Reader reader = null; String filename = ""; ScriptingContainer instance = new ScriptingContainer(LocalContextScope.THREADSAFE); instance.setError(pstream); instance.setOutput(pstream); instance.setWriter(writer); instance.setErrorWriter(writer); - Object expResult = null; - Object result = instance.runScriptlet(reader, filename); - assertEquals(expResult, result); filename = basedir + "/core/src/test/ruby/org/jruby/embed/ruby/iteration.rb"; - reader = new FileReader(filename); + Reader reader = new FileReader(filename); instance.put("@t", 3); - result = instance.runScriptlet(reader, filename); - expResult = + Object result = instance.runScriptlet(reader, filename); + Object expResult = "Trick or Treat!\nTrick or Treat!\nTrick or Treat!\n\nHmmm...I'd like trick."; assertEquals(expResult, result); From 4d1b13b0b35ebe559a5e1af64562eb2baaaa8eb4 Mon Sep 17 00:00:00 2001 From: kares Date: Fri, 4 Dec 2020 14:47:17 +0100 Subject: [PATCH 27/38] restore (old) behavior w scripting container exceptions --- .../org/jruby/embed/ScriptingContainer.java | 17 +++-- .../internal/EmbedRubyObjectAdapterImpl.java | 16 +++++ .../internal/EmbedRubyRuntimeAdapterImpl.java | 72 +++++++++++-------- .../embed/jsr223/JRubyEngineFactory.java | 5 +- .../jruby/embed/ScriptingContainerTest.java | 13 ++-- 5 files changed, 81 insertions(+), 42 deletions(-) diff --git a/core/src/main/java/org/jruby/embed/ScriptingContainer.java b/core/src/main/java/org/jruby/embed/ScriptingContainer.java index b68b55efe9f..6b691b7fc51 100644 --- a/core/src/main/java/org/jruby/embed/ScriptingContainer.java +++ b/core/src/main/java/org/jruby/embed/ScriptingContainer.java @@ -184,9 +184,9 @@ public class ScriptingContainer implements EmbedRubyInstanceConfigAdapter { private final Map basicProperties; private final LocalContextScope scope; private final LocalContextProvider provider; - private final EmbedRubyRuntimeAdapter runtimeAdapter = new EmbedRubyRuntimeAdapterImpl(this); - private final EmbedRubyObjectAdapter objectAdapter = new EmbedRubyObjectAdapterImpl(this); - private final EmbedRubyInterfaceAdapter interfaceAdapter = new EmbedRubyInterfaceAdapterImpl(this); + private final EmbedRubyRuntimeAdapter runtimeAdapter; + private final EmbedRubyObjectAdapter objectAdapter; + private final EmbedRubyInterfaceAdapter interfaceAdapter; /** * Constructs a ScriptingContainer with a default values. @@ -225,8 +225,7 @@ public ScriptingContainer(LocalContextScope scope, LocalVariableBehavior behavio } /** - * Constructs a ScriptingContainer with a specified local context scope, - * local variable behavior and laziness. + * Constructs a ScriptingContainer with a specified local context scope, local variable behavior and laziness. * * @param scope is one of a local context scope defined by {@link LocalContextScope} * @param behavior is one of a local variable behavior defined by {@link LocalVariableBehavior} @@ -235,10 +234,18 @@ public ScriptingContainer(LocalContextScope scope, LocalVariableBehavior behavio * get as many variables/constants as possible from Ruby runtime. */ public ScriptingContainer(LocalContextScope scope, LocalVariableBehavior behavior, boolean lazy) { + this(scope, behavior, lazy, true); // wrapping exceptions due backwards compatibility + } + + public ScriptingContainer(LocalContextScope scope, LocalVariableBehavior behavior, boolean lazy, boolean wrapExceptions) { this.provider = getProviderInstance(scope, behavior, lazy); this.scope = scope; initRubyInstanceConfig(); basicProperties = getBasicProperties(); + + runtimeAdapter = new EmbedRubyRuntimeAdapterImpl(this, wrapExceptions); + objectAdapter = new EmbedRubyObjectAdapterImpl(this, wrapExceptions); + interfaceAdapter = new EmbedRubyInterfaceAdapterImpl(this); } static LocalContextProvider getProviderInstance(LocalContextScope scope, LocalVariableBehavior behavior, boolean lazy) { diff --git a/core/src/main/java/org/jruby/embed/internal/EmbedRubyObjectAdapterImpl.java b/core/src/main/java/org/jruby/embed/internal/EmbedRubyObjectAdapterImpl.java index a4c72002aa6..1edc75a87d7 100644 --- a/core/src/main/java/org/jruby/embed/internal/EmbedRubyObjectAdapterImpl.java +++ b/core/src/main/java/org/jruby/embed/internal/EmbedRubyObjectAdapterImpl.java @@ -37,9 +37,11 @@ import org.jruby.RubyString; import org.jruby.embed.EmbedEvalUnit; import org.jruby.embed.EmbedRubyObjectAdapter; +import org.jruby.embed.InvokeFailedException; import org.jruby.embed.ScriptingContainer; import org.jruby.embed.variable.BiVariable; import org.jruby.embed.variable.InstanceVariable; +import org.jruby.internal.runtime.methods.InvokeDynamicMethodFactory; import org.jruby.javasupport.Java; import org.jruby.javasupport.JavaEmbedUtils; import org.jruby.javasupport.JavaObject; @@ -63,6 +65,7 @@ public class EmbedRubyObjectAdapterImpl implements EmbedRubyObjectAdapter { private final RubyObjectAdapter adapter = JavaEmbedUtils.newObjectAdapter(); private final ScriptingContainer container; + private final boolean wrapExceptions; @Deprecated // not used public enum MethodType { @@ -74,7 +77,12 @@ public enum MethodType { } public EmbedRubyObjectAdapterImpl(ScriptingContainer container) { + this(container, false); + } + + public EmbedRubyObjectAdapterImpl(ScriptingContainer container, boolean wrapExceptions) { this.container = container; + this.wrapExceptions = wrapExceptions; } public boolean isKindOf(IRubyObject value, RubyModule rubyModule) { @@ -211,6 +219,10 @@ private T doInvokeSuper(Class returnType, IRubyObject rubyReceiver, Block return returnType.cast(ret); } return null; + } catch (Exception e) { + if (e instanceof InvokeFailedException) throw e; + if (wrapExceptions) throw new InvokeFailedException(e); + Helpers.throwException(e); return null; // never returns } finally { if (sharing_variables) { afterSharingVariablesCall(context); @@ -236,6 +248,10 @@ private T doInvokeMethod(Class returnType, IRubyObject rubyReceiver, Stri return returnType.cast(ret); } return null; + } catch (Exception e) { + if (e instanceof InvokeFailedException) throw e; + if (wrapExceptions) throw new InvokeFailedException(e); + Helpers.throwException(e); return null; // never returns } finally { if (sharing_variables) { afterSharingVariablesCall(context); diff --git a/core/src/main/java/org/jruby/embed/internal/EmbedRubyRuntimeAdapterImpl.java b/core/src/main/java/org/jruby/embed/internal/EmbedRubyRuntimeAdapterImpl.java index 429100dbb7c..c762387c303 100644 --- a/core/src/main/java/org/jruby/embed/internal/EmbedRubyRuntimeAdapterImpl.java +++ b/core/src/main/java/org/jruby/embed/internal/EmbedRubyRuntimeAdapterImpl.java @@ -37,6 +37,7 @@ import java.io.Reader; import java.io.StringReader; import java.net.URL; +import java.util.Objects; import org.jruby.Ruby; import org.jruby.RubyInstanceConfig.CompileMode; @@ -47,10 +48,12 @@ import org.jruby.embed.AttributeName; import org.jruby.embed.EmbedEvalUnit; import org.jruby.embed.EmbedRubyRuntimeAdapter; +import org.jruby.embed.ParseFailedException; import org.jruby.embed.PathType; import org.jruby.embed.ScriptingContainer; import org.jruby.embed.io.ReaderInputStream; import org.jruby.embed.util.SystemPropertyCatcher; +import org.jruby.exceptions.RaiseException; import org.jruby.internal.runtime.GlobalVariable; import org.jruby.internal.runtime.ValueAccessor; import org.jruby.javasupport.JavaEmbedUtils; @@ -74,9 +77,15 @@ public class EmbedRubyRuntimeAdapterImpl implements EmbedRubyRuntimeAdapter { private final RubyRuntimeAdapter adapter = JavaEmbedUtils.newRuntimeAdapter(); private final ScriptingContainer container; + private final boolean wrapExceptions; public EmbedRubyRuntimeAdapterImpl(ScriptingContainer container) { + this(container, false); + } + + public EmbedRubyRuntimeAdapterImpl(ScriptingContainer container, boolean wrapExceptions) { this.container = container; + this.wrapExceptions = wrapExceptions; } public EmbedEvalUnit parse(String script, int... lines) { @@ -97,21 +106,17 @@ public EmbedEvalUnit parse(String script, int... lines) { } public EmbedEvalUnit parse(Reader reader, String filename, int... lines) { - if (reader != null) { - InputStream istream = new ReaderInputStream(reader); - return runParser(istream, filename, lines); - } else { - return null; - } + Objects.requireNonNull(reader, "reader"); + + InputStream istream = new ReaderInputStream(reader); + return runParser(istream, filename, lines); } public EmbedEvalUnit parse(PathType type, String filename, int... lines) { - if (filename == null) { - return null; - } - if (type == null) { - type = PathType.ABSOLUTE; - } + Objects.requireNonNull(filename, "filename"); + + if (type == null) type = PathType.ABSOLUTE; + InputStream istream = null; try { switch (type) { @@ -140,11 +145,12 @@ public EmbedEvalUnit parse(PathType type, String filename, int... lines) { } return parse(istream, filename, lines); } catch (FileNotFoundException e) { - Helpers.throwException(e); - return null; + if (wrapExceptions) throw new ParseFailedException(e); + // NOTE: we do not declare throws IOException due source + Helpers.throwException(e); return null; } finally { if (istream != null) { - try {istream.close();} catch (IOException ioe) {} + try { istream.close(); } catch (IOException ioe) {} } } } @@ -175,23 +181,31 @@ private EmbedEvalUnit runParser(Object input, String filename, int... lines) { if (isSharingVariables(container)) { scope = createLocalVarScope(runtime, container.getVarMap().getLocalVarNames()); } - final Node node; - if (input instanceof String) { - node = runtime.parseEval((String)input, filename, scope, line); - } else { - node = runtime.parseFile((InputStream)input, filename, scope, line); - } - CompileMode compileMode = runtime.getInstanceConfig().getCompileMode(); - if (compileMode == CompileMode.FORCE) { - // CON FIXME: We may need to force heap variables here so the compiled script uses our provided scope - Script script = runtime.tryCompile(node); - if (script != null) { - return new EmbedEvalUnitImpl(container, node, scope, script); + try { + final Node node; + if (input instanceof String) { + node = runtime.parseEval((String) input, filename, scope, line); } else { - return new EmbedEvalUnitImpl(container, node, scope); + node = runtime.parseFile((InputStream) input, filename, scope, line); + } + CompileMode compileMode = runtime.getInstanceConfig().getCompileMode(); + if (compileMode == CompileMode.FORCE) { + // CON FIXME: We may need to force heap variables here so the compiled script uses our provided scope + Script script = runtime.tryCompile(node); + if (script != null) { + return new EmbedEvalUnitImpl(container, node, scope, script); + } else { + return new EmbedEvalUnitImpl(container, node, scope); + } } + return new EmbedEvalUnitImpl(container, node, scope); + } catch (RaiseException e) { + if (wrapExceptions) throw new ParseFailedException(e.getMessage(), e); + throw e; + } catch (RuntimeException e) { + if (wrapExceptions) throw new ParseFailedException(e); + throw e; } - return new EmbedEvalUnitImpl(container, node, scope); } public IRubyObject eval(Ruby runtime, String script) { diff --git a/core/src/main/java/org/jruby/embed/jsr223/JRubyEngineFactory.java b/core/src/main/java/org/jruby/embed/jsr223/JRubyEngineFactory.java index ccd780ffb94..9a7784411be 100644 --- a/core/src/main/java/org/jruby/embed/jsr223/JRubyEngineFactory.java +++ b/core/src/main/java/org/jruby/embed/jsr223/JRubyEngineFactory.java @@ -167,11 +167,10 @@ public ScriptEngine getScriptEngine() { LocalContextScope scope = SystemPropertyCatcher.getScope(LocalContextScope.SINGLETON); LocalVariableBehavior behavior = SystemPropertyCatcher.getBehavior(LocalVariableBehavior.PERSISTENT); boolean lazy = SystemPropertyCatcher.isLazy(false); - ScriptingContainer container = new ScriptingContainer(scope, behavior, lazy); + ScriptingContainer container = new ScriptingContainer(scope, behavior, lazy, false); SystemPropertyCatcher.setClassLoader(container); SystemPropertyCatcher.setConfiguration(container); - JRubyEngine engine = new JRubyEngine(container, this); - return (ScriptEngine)engine; + return new JRubyEngine(container, this); } } diff --git a/core/src/test/java/org/jruby/embed/ScriptingContainerTest.java b/core/src/test/java/org/jruby/embed/ScriptingContainerTest.java index 37f9124ddd3..9a6c7648998 100644 --- a/core/src/test/java/org/jruby/embed/ScriptingContainerTest.java +++ b/core/src/test/java/org/jruby/embed/ScriptingContainerTest.java @@ -578,7 +578,7 @@ public void testParse_3args_2() { int[] lines = null; String[] paths = {basedir + "/lib", basedir + "/lib/ruby/stdlib"}; - ScriptingContainer instance = new ScriptingContainer(LocalContextScope.THREADSAFE); + ScriptingContainer instance = new ScriptingContainer(LocalContextScope.THREADSAFE, LocalVariableBehavior.TRANSIENT, true, false); instance.setLoadPaths(Arrays.asList(paths)); instance.setError(pstream); instance.setOutput(pstream); @@ -589,7 +589,7 @@ public void testParse_3args_2() { try { result = instance.parse(type, filename, lines); } catch (Exception e) { - assertTrue(e instanceof FileNotFoundException); + assertTrue(e.getClass().toString(), e instanceof FileNotFoundException); e.printStackTrace(new PrintStream(outStream)); } @@ -804,7 +804,8 @@ public void testRunScriptlet_PathType_String() { try { result = instance.parse(type, filename); } catch (Exception e) { - assertTrue(e instanceof FileNotFoundException); + assertTrue(e.toString(), e instanceof ParseFailedException); + assertTrue(Objects.toString(e.getCause()), e.getCause() instanceof FileNotFoundException); e.printStackTrace(new PrintStream(outStream)); } @@ -902,7 +903,7 @@ public void testCallMethod_3args() { Object receiver = null; Class returnType = null; String[] paths = {basedir + "/lib/ruby/stdlib"}; - ScriptingContainer instance = new ScriptingContainer(LocalContextScope.THREADSAFE); + ScriptingContainer instance = new ScriptingContainer(LocalContextScope.THREADSAFE, LocalVariableBehavior.TRANSIENT, true, false); instance.setLoadPaths(Arrays.asList(paths)); instance.setError(pstream); instance.setOutput(pstream); @@ -1040,6 +1041,8 @@ public void testCallMethod_4args_3() { try { Object result = instance.callMethod(null, "", returnType, unit); fail("expected to raise NoMethodError, but got result: " + result); + } catch (InvokeFailedException ex) { + assertTrue(Objects.toString(ex.getCause()), ex.getCause() instanceof NoMethodError); } catch (NoMethodError ex) { // pass } @@ -2237,7 +2240,7 @@ public void testSetScriptFilename() { instance.terminate(); filename = "["+this.getClass().getCanonicalName()+"]"; - instance = new ScriptingContainer(LocalContextScope.SINGLETHREAD); + instance = new ScriptingContainer(LocalContextScope.SINGLETHREAD, LocalVariableBehavior.TRANSIENT, true, false); instance.setScriptFilename(filename); StringWriter sw = new StringWriter(); instance.setErrorWriter(sw); From e3dc010a1cd4a7c6758311733a794e1583adce08 Mon Sep 17 00:00:00 2001 From: kares Date: Sun, 6 Dec 2020 16:35:54 +0100 Subject: [PATCH 28/38] un-deprecate embed exceptions - as they're still used --- core/src/main/java/org/jruby/embed/InvokeFailedException.java | 1 - core/src/main/java/org/jruby/embed/ParseFailedException.java | 1 - 2 files changed, 2 deletions(-) diff --git a/core/src/main/java/org/jruby/embed/InvokeFailedException.java b/core/src/main/java/org/jruby/embed/InvokeFailedException.java index 1106a80b4c9..38ea14e91bb 100644 --- a/core/src/main/java/org/jruby/embed/InvokeFailedException.java +++ b/core/src/main/java/org/jruby/embed/InvokeFailedException.java @@ -35,7 +35,6 @@ * * @author Yoko Harada */ -@Deprecated public class InvokeFailedException extends RuntimeException { public InvokeFailedException(Throwable cause) { super(cause); diff --git a/core/src/main/java/org/jruby/embed/ParseFailedException.java b/core/src/main/java/org/jruby/embed/ParseFailedException.java index ad84e725aad..1d01aab3a42 100644 --- a/core/src/main/java/org/jruby/embed/ParseFailedException.java +++ b/core/src/main/java/org/jruby/embed/ParseFailedException.java @@ -36,7 +36,6 @@ * * @author Yoko Harada */ -@Deprecated public class ParseFailedException extends RuntimeException { public ParseFailedException(Throwable cause) { super(cause); From 138eeccc8f23b72517eb27210fcc622f3e10321c Mon Sep 17 00:00:00 2001 From: kares Date: Wed, 9 Dec 2020 07:44:17 +0100 Subject: [PATCH 29/38] [test] make sure embed cases are properly isolated setting system properties in one does not leak to other --- .../java/org/jruby/embed/jsr223/BaseTest.java | 37 ++++++++++ .../embed/jsr223/JRubyCompiledScriptTest.java | 55 +++----------- .../embed/jsr223/JRubyEngineFactoryTest.java | 74 +++---------------- .../jruby/embed/jsr223/JRubyEngineTest.java | 67 ++--------------- 4 files changed, 64 insertions(+), 169 deletions(-) create mode 100644 core/src/test/java/org/jruby/embed/jsr223/BaseTest.java diff --git a/core/src/test/java/org/jruby/embed/jsr223/BaseTest.java b/core/src/test/java/org/jruby/embed/jsr223/BaseTest.java new file mode 100644 index 00000000000..4880a5eed37 --- /dev/null +++ b/core/src/test/java/org/jruby/embed/jsr223/BaseTest.java @@ -0,0 +1,37 @@ +package org.jruby.embed.jsr223; + +import org.junit.After; +import org.junit.Before; + +import java.io.File; +import java.util.LinkedHashMap; +import java.util.Map; + +public abstract class BaseTest { + + static final String basedir = new File(System.getProperty("user.dir")).getParent(); + + final Map systemPropertiesMemo = new LinkedHashMap<>(); + + @Before + public void setUp() throws Exception { + String name; + systemPropertiesMemo.clear(); + name = "org.jruby.embed.localcontext.scope"; + systemPropertiesMemo.put(name, System.getProperty(name)); + name = "org.jruby.embed.localvariable.behavior"; + systemPropertiesMemo.put(name, System.getProperty(name)); + } + + @After + public void tearDown() throws Exception { + systemPropertiesMemo.forEach((name, value) -> { + if (value == null) { + System.clearProperty(name); + } else { + System.setProperty(name, value); + } + }); + } + +} diff --git a/core/src/test/java/org/jruby/embed/jsr223/JRubyCompiledScriptTest.java b/core/src/test/java/org/jruby/embed/jsr223/JRubyCompiledScriptTest.java index 73b4f181331..43231f36ddc 100644 --- a/core/src/test/java/org/jruby/embed/jsr223/JRubyCompiledScriptTest.java +++ b/core/src/test/java/org/jruby/embed/jsr223/JRubyCompiledScriptTest.java @@ -29,26 +29,15 @@ package org.jruby.embed.jsr223; -import java.io.FileNotFoundException; -import java.io.FileOutputStream; -import java.io.OutputStream; import java.io.StringWriter; import javax.script.Bindings; import javax.script.ScriptContext; import javax.script.ScriptException; import javax.script.SimpleBindings; import javax.script.SimpleScriptContext; -import java.util.logging.ConsoleHandler; -import java.util.logging.Handler; -import java.util.logging.Level; -import java.util.logging.Logger; -import java.util.logging.SimpleFormatter; -import java.util.logging.StreamHandler; import org.jruby.embed.AttributeName; import org.junit.After; -import org.junit.AfterClass; import org.junit.Before; -import org.junit.BeforeClass; import org.junit.Test; import static org.junit.Assert.*; @@ -56,40 +45,18 @@ * * @author Yoko Harada */ -public class JRubyCompiledScriptTest { - String basedir = System.getProperty("user.dir"); - static Logger logger0 = Logger.getLogger(JRubyCompiledScriptTest.class.getName()); - static Logger logger1 = Logger.getLogger(JRubyCompiledScriptTest.class.getName()); - static OutputStream outStream = null; - - public JRubyCompiledScriptTest() { - } - - @BeforeClass - public static void setUpClass() throws Exception { - } - - @AfterClass - public static void tearDownClass() throws Exception { - outStream.close(); - } +public class JRubyCompiledScriptTest extends BaseTest { @Before - public void setUp() throws FileNotFoundException { - System.setProperty("org.jruby.embed.localcontext.scope", "threadsafe"); + public void setUp() throws Exception { + super.setUp(); - outStream = new FileOutputStream(basedir + "/target/run-junit-embed.log", true); - Handler handler = new StreamHandler(outStream, new SimpleFormatter()); - logger0.addHandler(handler); - logger0.setUseParentHandlers(false); - logger0.setLevel(Level.INFO); - logger1.setUseParentHandlers(false); - logger1.addHandler(new ConsoleHandler()); - logger1.setLevel(Level.WARNING); + System.setProperty("org.jruby.embed.localcontext.scope", "threadsafe"); } @After - public void tearDown() { + public void tearDown() throws Exception { + super.tearDown(); } /** @@ -97,7 +64,6 @@ public void tearDown() { */ @Test public void testEval() throws Exception { - logger1.info("eval"); JRubyEngineFactory factory = new JRubyEngineFactory(); JRubyEngine engine = (JRubyEngine) factory.getScriptEngine(); String script = "return \"Hello World!!!\""; @@ -113,8 +79,8 @@ public void testEval() throws Exception { */ @Test public void testEval_context() throws Exception { - logger1.info("eval with context"); System.setProperty("org.jruby.embed.localvariable.behavior", "transient"); + JRubyEngineFactory factory = new JRubyEngineFactory(); JRubyEngine engine = (JRubyEngine) factory.getScriptEngine(); ScriptContext context = new SimpleScriptContext(); @@ -171,8 +137,8 @@ public void testEval_context() throws Exception { */ @Test public void testEval_bindings() throws Exception { - logger1.info("eval with bindings"); System.setProperty("org.jruby.embed.localvariable.behavior", "transient"); + JRubyEngineFactory factory = new JRubyEngineFactory(); JRubyEngine engine = (JRubyEngine) factory.getScriptEngine(); Bindings bindings = new SimpleBindings(); @@ -207,7 +173,6 @@ public void testEval_bindings() throws Exception { */ @Test public void testGetEngine() throws ScriptException { - logger1.info("getEngine"); JRubyEngineFactory factory = new JRubyEngineFactory(); JRubyEngine engine = (JRubyEngine) factory.getScriptEngine(); String script = "puts \"Hello World!!!\""; @@ -220,8 +185,7 @@ public void testGetEngine() throws ScriptException { @Test public void testTerminate() throws ScriptException { - logger1.info("termination"); - JRubyEngine engine = null; + JRubyEngine engine; synchronized (this) { System.setProperty("org.jruby.embed.localcontext.scope", "singlethread"); System.setProperty("org.jruby.embed.localvariable.behavior", "transient"); @@ -247,4 +211,5 @@ public void testTerminate() throws ScriptException { assertEquals(expResult, writer.toString().trim()); engine.getContext().setAttribute(AttributeName.TERMINATION.toString(), false, ScriptContext.ENGINE_SCOPE); } + } diff --git a/core/src/test/java/org/jruby/embed/jsr223/JRubyEngineFactoryTest.java b/core/src/test/java/org/jruby/embed/jsr223/JRubyEngineFactoryTest.java index f7442b49a70..a330d11c64f 100644 --- a/core/src/test/java/org/jruby/embed/jsr223/JRubyEngineFactoryTest.java +++ b/core/src/test/java/org/jruby/embed/jsr223/JRubyEngineFactoryTest.java @@ -29,26 +29,13 @@ package org.jruby.embed.jsr223; -import java.io.FileNotFoundException; -import java.io.FileOutputStream; -import java.io.OutputStream; -import java.lang.reflect.Method; -import java.util.ArrayList; import java.util.Arrays; import java.util.List; -import java.util.logging.ConsoleHandler; -import java.util.logging.Handler; -import java.util.logging.Level; -import java.util.logging.Logger; -import java.util.logging.SimpleFormatter; -import java.util.logging.StreamHandler; import javax.script.ScriptEngine; import javax.script.ScriptException; import org.jruby.runtime.Constants; import org.junit.After; -import org.junit.AfterClass; import org.junit.Before; -import org.junit.BeforeClass; import org.junit.Test; import static org.junit.Assert.*; @@ -56,41 +43,18 @@ * * @author Yoko Harada */ -public class JRubyEngineFactoryTest { - String basedir = System.getProperty("user.dir"); - - static Logger logger0 = Logger.getLogger(JRubyEngineFactoryTest.class.getName()); - static Logger logger1 = Logger.getLogger(JRubyEngineFactoryTest.class.getName()); - static OutputStream outStream = null; - - public JRubyEngineFactoryTest() { - } - - @BeforeClass - public static void setUpClass() throws Exception { - } - - @AfterClass - public static void tearDownClass() throws Exception { - outStream.close(); - } +public class JRubyEngineFactoryTest extends BaseTest { @Before - public void setUp() throws FileNotFoundException { - System.setProperty("org.jruby.embed.localcontext.scope", "threadsafe"); + public void setUp() throws Exception { + super.setUp(); - outStream = new FileOutputStream(basedir + "/target/run-junit-embed.log", true); - Handler handler = new StreamHandler(outStream, new SimpleFormatter()); - logger0.addHandler(handler); - logger0.setUseParentHandlers(false); - logger0.setLevel(Level.INFO); - logger1.setUseParentHandlers(false); - logger1.addHandler(new ConsoleHandler()); - logger1.setLevel(Level.WARNING); + System.setProperty("org.jruby.embed.localcontext.scope", "threadsafe"); } @After - public void tearDown() { + public void tearDown() throws Exception { + super.tearDown(); } /** @@ -98,13 +62,10 @@ public void tearDown() { */ @Test public void testGetEngineName() { - logger1.info("getEngineName"); JRubyEngineFactory instance = new JRubyEngineFactory(); String expResult = "JSR 223 JRuby Engine"; String result = instance.getEngineName(); assertEquals(expResult, result); - - instance = null; } /** @@ -112,7 +73,6 @@ public void testGetEngineName() { */ @Test public void testGetEngineVersion() { - logger1.info("getEngineVersion"); JRubyEngineFactory instance = new JRubyEngineFactory(); String expResult = org.jruby.runtime.Constants.VERSION; String result = instance.getEngineVersion(); @@ -124,7 +84,6 @@ public void testGetEngineVersion() { */ @Test public void testGetExtensions() { - logger1.info("getExtensions"); JRubyEngineFactory instance = new JRubyEngineFactory(); List result = instance.getExtensions(); assertEquals(Arrays.asList("rb"), result); @@ -135,7 +94,6 @@ public void testGetExtensions() { */ @Test public void testGetLanguageName() { - logger1.info("getLanguageName"); JRubyEngineFactory instance = new JRubyEngineFactory(); String expResult = "ruby"; String result = instance.getLanguageName(); @@ -147,12 +105,10 @@ public void testGetLanguageName() { */ @Test public void testGetLanguageVersion() { - logger1.info("getLanguageVersion"); JRubyEngineFactory instance = new JRubyEngineFactory(); String expResult = "jruby " + Constants.VERSION; String result = instance.getLanguageVersion(); assertTrue(result.startsWith(expResult)); - logger1.info(result); } /** @@ -160,7 +116,6 @@ public void testGetLanguageVersion() { */ @Test public void testGetMethodCallSyntax() { - logger1.info("getMethodCallSyntax"); String obj = "receiver"; String m = "establish_connection"; String[] args = {"localhost", "1099"}; @@ -175,7 +130,6 @@ public void testGetMethodCallSyntax() { */ @Test public void testGetMimeTypes() { - logger1.info("getMimeTypes"); JRubyEngineFactory instance = new JRubyEngineFactory(); List result = instance.getMimeTypes(); assertEquals(Arrays.asList("application/x-ruby"), result); @@ -186,7 +140,6 @@ public void testGetMimeTypes() { */ @Test public void testGetNames() { - logger1.info("getNames"); JRubyEngineFactory instance = new JRubyEngineFactory(); assertEquals(Arrays.asList("ruby", "jruby"), instance.getNames()); } @@ -196,7 +149,6 @@ public void testGetNames() { */ @Test public void testGetOutputStatement() { - logger1.info("getOutputStatement"); String toDisplay = "abc"; JRubyEngineFactory instance = new JRubyEngineFactory(); String expResult = "puts abc\nor\nprint abc"; @@ -208,7 +160,6 @@ public void testGetOutputStatement() { */ @Test public void testGetParameter() { - logger1.info("getParameter"); String key = ""; JRubyEngineFactory instance = new JRubyEngineFactory(); Object expResult = null; @@ -251,10 +202,10 @@ public void testGetParameter() { */ @Test public void testGetProgram() { - logger1.info("getProgram"); - String[] statements = - {"1.upto(7) {|i| print i, \" \"}", - "hh = {\"p\" => 3.14, \"e\" => 2.22}"}; + String[] statements = { + "1.upto(7) {|i| print i, \" \"}", + "hh = {\"p\" => 3.14, \"e\" => 2.22}" + }; System.setProperty("org.jruby.embed.localcontext.scope", "singlethread"); JRubyEngineFactory instance = new JRubyEngineFactory(); String expResult = "1.upto(7) {|i| print i, \" \"}\nhh = {\"p\" => 3.14, \"e\" => 2.22}\n"; @@ -267,7 +218,6 @@ public void testGetProgram() { */ @Test public void testGetScriptEngine() { - logger1.info("getScriptEngine"); JRubyEngineFactory instance = new JRubyEngineFactory(); ScriptEngine engine = instance.getScriptEngine(); assertSame(instance, engine.getFactory()); @@ -291,9 +241,7 @@ public void testEmbedIntegration() throws ScriptException { assertTrue( command instanceof org.jruby.RubyMethod ); - engine.eval( - "$command.call \"third to pass\n\"" - ); + engine.eval("$command.call \"third to pass\n\""); assertEquals("first\n2\nthird to pass\n", $this.toString()); } diff --git a/core/src/test/java/org/jruby/embed/jsr223/JRubyEngineTest.java b/core/src/test/java/org/jruby/embed/jsr223/JRubyEngineTest.java index 63cf924530d..75ce14fe1b3 100644 --- a/core/src/test/java/org/jruby/embed/jsr223/JRubyEngineTest.java +++ b/core/src/test/java/org/jruby/embed/jsr223/JRubyEngineTest.java @@ -68,42 +68,16 @@ * * @author Yoko Harada */ -public class JRubyEngineTest { - - private static final String basedir = new File(System.getProperty("user.dir")).getParent(); - - static final Logger logger0 = Logger.getLogger(JRubyEngineTest.class.getName()); - static final Logger logger1 = Logger.getLogger(JRubyEngineTest.class.getName()); - - static OutputStream outStream = null; - - FileWriter writer; - - @BeforeClass - public static void setUpClass() throws Exception { - outStream = new FileOutputStream(basedir + "/core/target/run-junit-embed.log", true); - Handler handler = new StreamHandler(outStream, new SimpleFormatter()); - logger0.addHandler(handler); - logger0.setUseParentHandlers(false); - logger0.setLevel(Level.INFO); - logger1.setUseParentHandlers(false); - logger1.addHandler(new ConsoleHandler()); - logger1.setLevel(Level.WARNING); - } - - @AfterClass - public static void tearDownClass() throws Exception { - outStream.close(); - } +public class JRubyEngineTest extends BaseTest { @Before - public void setUp() throws FileNotFoundException, IOException { - writer = new FileWriter(basedir + "/core/target/run-junit-embed.txt", true); + public void setUp() throws Exception { + super.setUp(); } @After - public void tearDown() throws IOException { - writer.close(); + public void tearDown() throws Exception { + super.tearDown(); } /** @@ -111,7 +85,6 @@ public void tearDown() throws IOException { */ @Test public void testCompile_String() throws Exception { - logger1.info("[compile string]"); ScriptEngine instance; synchronized (this) { System.setProperty("org.jruby.embed.localcontext.scope", "singlethread"); @@ -143,7 +116,6 @@ public void testCompile_String() throws Exception { */ @Test public void testCompile_Reader() throws Exception { - logger1.info("[compile reader]"); ScriptEngine instance; synchronized(this) { System.setProperty("org.jruby.embed.localcontext.scope", "singlethread"); @@ -174,7 +146,6 @@ public void testCompile_Reader() throws Exception { */ @Test public void testEval_String_ScriptContext() throws Exception { - logger1.info("[eval String with ScriptContext]"); ScriptEngine instance; synchronized (this) { System.setProperty("org.jruby.embed.localcontext.scope", "singlethread"); @@ -220,7 +191,6 @@ public void testEval_String_ScriptContext() throws Exception { */ @Test public void testEval_String_ScriptContext2() throws Exception { - logger1.info("[eval String with ScriptContext 2]"); ScriptEngine instance; synchronized(this) { System.setProperty("org.jruby.embed.localcontext.scope", "singlethread"); @@ -249,7 +219,6 @@ public void testEval_String_ScriptContext2() throws Exception { */ @Test public void testEval_Reader_ScriptContext() throws Exception { - logger1.info("[eval Reader with ScriptContext]"); ScriptEngine instance; synchronized(this) { System.setProperty("org.jruby.embed.localcontext.scope", "singlethread"); @@ -277,7 +246,6 @@ public void testEval_Reader_ScriptContext() throws Exception { */ @Test public void testEval_String() throws Exception { - logger1.info("eval String"); ScriptEngine instance; synchronized(this) { System.setProperty("org.jruby.embed.localcontext.scope", "singlethread"); @@ -285,7 +253,6 @@ public void testEval_String() throws Exception { ScriptEngineManager manager = new ScriptEngineManager(); instance = manager.getEngineByName("jruby"); } - instance.getContext().setWriter(writer); instance.eval("p=9.0"); instance.eval("q = Math.sqrt p"); instance.eval("puts \"square root of #{p} is #{q}\""); @@ -301,7 +268,6 @@ public void testEval_String() throws Exception { */ @Test public void testEval_Reader() throws Exception { - logger1.info("eval Reader"); ScriptEngine instance; synchronized (this) { System.setProperty("org.jruby.embed.localcontext.scope", "singlethread"); @@ -319,7 +285,7 @@ public void testEval_Reader() throws Exception { instance.getBindings(ScriptContext.ENGINE_SCOPE).clear(); } - private int getNextYear() { + private static int getNextYear() { Calendar calendar = Calendar.getInstance(); int this_year = calendar.get(Calendar.YEAR); return this_year + 1; @@ -330,7 +296,6 @@ private int getNextYear() { */ @Test public void testEval_String_Bindings() throws Exception { - logger1.info("eval String with Bindings"); ScriptEngine instance; synchronized(this) { System.setProperty("org.jruby.embed.localcontext.scope", "singlethread"); @@ -357,7 +322,6 @@ public void testEval_String_Bindings() throws Exception { */ @Test public void testEval_Reader_Bindings() throws Exception { - logger1.info("eval Reader with Bindings"); ScriptEngine instance; synchronized(this) { System.setProperty("org.jruby.embed.localcontext.scope", "singlethread"); @@ -382,7 +346,6 @@ public void testEval_Reader_Bindings() throws Exception { */ @Test public void testGet() { - logger1.info("get"); ScriptEngine instance; synchronized(this) { System.setProperty("org.jruby.embed.localcontext.scope", "singlethread"); @@ -417,7 +380,6 @@ public void testGet() { */ @Test public void testPut() { - logger1.info("put"); String key = ""; Object value = null; ScriptEngineManager manager = new ScriptEngineManager(); @@ -437,7 +399,6 @@ public void testPut() { */ @Test public void testGetBindings() throws ScriptException { - logger1.info("getBindings"); ScriptEngine instance; synchronized(this) { System.setProperty("org.jruby.embed.localcontext.scope", "singlethread"); @@ -474,7 +435,6 @@ public void testGetBindings() throws ScriptException { */ @Test public void testSetBindings() throws ScriptException { - logger1.info("setBindings"); ScriptEngine instance; synchronized(this) { System.setProperty("org.jruby.embed.localcontext.scope", "singlethread"); @@ -503,7 +463,6 @@ public void testSetBindings() throws ScriptException { */ @Test public void testCreateBindings() { - logger1.info("createBindings"); ScriptEngineManager manager = new ScriptEngineManager(); ScriptEngine instance = manager.getEngineByName("jruby"); Bindings bindings = instance.getBindings(ScriptContext.ENGINE_SCOPE); @@ -518,7 +477,6 @@ public void testCreateBindings() { */ @Test public void testGetContext() { - logger1.info("getContext"); ScriptEngineManager manager = new ScriptEngineManager(); ScriptEngine instance = manager.getEngineByName("jruby"); ScriptContext result = instance.getContext(); @@ -532,7 +490,6 @@ public void testGetContext() { */ @Test public void testSetContext() { - logger1.info("setContext"); ScriptEngineManager manager = new ScriptEngineManager(); ScriptEngine instance = manager.getEngineByName("jruby"); ScriptContext ctx = new SimpleScriptContext(); @@ -554,7 +511,6 @@ public void testSetContext() { */ @Test public void testGetFactory() { - logger1.info("getFactory"); ScriptEngineManager manager = new ScriptEngineManager(); ScriptEngine instance = manager.getEngineByName("jruby"); ScriptEngineFactory result = instance.getFactory(); @@ -571,7 +527,6 @@ public void testGetFactory() { */ @Test public void testInvokeMethod() throws Exception { - logger1.info("invokeMethod"); ScriptEngine instance = newScriptEngine(); String filename = basedir + "/core/src/test/ruby/org/jruby/embed/ruby/tree.rb"; Reader reader = new FileReader(filename); @@ -631,7 +586,6 @@ public void testInvokeMethodWithJavaException() throws Exception { */ @Test public void testInvokeFunction() throws Exception { - logger1.info("invokeFunction"); ScriptEngine instance = newScriptEngine(); String filename = basedir + "/core/src/test/ruby/org/jruby/embed/ruby/count_down.rb"; Reader reader = new FileReader(filename); @@ -671,7 +625,6 @@ public void testInvokeFunctionWithJavaException() throws Exception { */ @Test public void testGetInterface_Class() throws FileNotFoundException, ScriptException { - logger1.info("getInterface (no receiver)"); ScriptEngine instance = newScriptEngine(); Class returnType = RadioActiveDecay.class; String filename = basedir + "/core/src/test/ruby/org/jruby/embed/ruby/radioactive_decay.rb"; @@ -693,7 +646,6 @@ public void testGetInterface_Class() throws FileNotFoundException, ScriptExcepti */ @Test public void testGetInterface_Object_Class() throws FileNotFoundException, ScriptException { - logger1.info("getInterface (with receiver)"); ScriptEngine instance = newScriptEngine(); String filename = basedir + "/core/src/test/ruby/org/jruby/embed/ruby/position_function.rb"; Reader reader = new FileReader(filename); @@ -720,9 +672,7 @@ public void testGetInterface_Object_Class() throws FileNotFoundException, Script */ @Test public void testARGV() throws ScriptException { - logger1.info("ScriptEngine.ARGV"); ScriptEngine instance = newScriptEngine(); - instance.getContext().setErrorWriter(writer); String script = "" + "if ARGV.length == 0\n" + " raise 'Error No argv passed'\n" + @@ -736,9 +686,7 @@ public void testARGV() throws ScriptException { */ @Test public void testARGV_2() throws ScriptException { - logger1.info("ScriptEngine.ARGV before initialization"); ScriptEngine instance = newScriptEngine(); - instance.getContext().setErrorWriter(writer); Bindings bindings = instance.getBindings(ScriptContext.ENGINE_SCOPE); bindings.put(ScriptEngine.ARGV, new String[]{"init params"}); String script = "" + @@ -754,7 +702,6 @@ public void testARGV_2() throws ScriptException { // raised at "Object obj1 = engine1.eval("$Value + 2010.to_s");" //@Test public void testMultipleEngineStates() throws ScriptException { - logger1.info("Multiple Engine States"); ScriptEngine engine1; ScriptEngine engine2; synchronized (this) { @@ -781,7 +728,6 @@ public void testMultipleEngineStates() throws ScriptException { @Test public void testTermination() throws ScriptException { - logger1.info("Termination Test"); ScriptEngineManager manager = new ScriptEngineManager(); JRubyEngine instance = (JRubyEngine) manager.getEngineByName("jruby"); StringWriter sw = new StringWriter(); @@ -801,7 +747,6 @@ public void testTermination() throws ScriptException { @Test public void testClearVariables() throws ScriptException { - logger1.info("Clear Variables Test"); final ScriptEngine instance = newScriptEngine("singlethread", "global"); instance.put("gvar", ":Gvar"); From 433fdaab0fda69079bc0e2eb41257d7ac65f8216 Mon Sep 17 00:00:00 2001 From: kares Date: Wed, 9 Dec 2020 09:56:44 +0100 Subject: [PATCH 30/38] [test] enable skipped tests (due embed properties leak) --- core/src/test/java/org/jruby/embed/jsr223/JRubyEngineTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/test/java/org/jruby/embed/jsr223/JRubyEngineTest.java b/core/src/test/java/org/jruby/embed/jsr223/JRubyEngineTest.java index 75ce14fe1b3..d2d088d8efa 100644 --- a/core/src/test/java/org/jruby/embed/jsr223/JRubyEngineTest.java +++ b/core/src/test/java/org/jruby/embed/jsr223/JRubyEngineTest.java @@ -700,7 +700,7 @@ public void testARGV_2() throws ScriptException { // This code worked successfully on command-line but never as JUnit test //