Repository: tinkerpop Updated Branches: refs/heads/master 2d30a76aa -> 005d2eb42
Fixed bug in script engine which wasn't registering globals The secure configs for gremlin server weren't working. There was a change in the ScriptEngine plugin system that wasn't properly accounted for. Needed to register the types. Also added a little something to detect if this feature is even needed. That wasn't possible to do prior to 3.3.0. CTR Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/4247e7df Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/4247e7df Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/4247e7df Branch: refs/heads/master Commit: 4247e7df370418510e202e566e27e26b7939f5b9 Parents: 2d30a76 Author: Stephen Mallette <[email protected]> Authored: Tue Jul 18 16:14:52 2017 -0400 Committer: Stephen Mallette <[email protected]> Committed: Tue Jul 18 16:14:52 2017 -0400 ---------------------------------------------------------------------- CHANGELOG.asciidoc | 1 + .../jsr223/GremlinGroovyScriptEngine.java | 38 +++++++++++++------- 2 files changed, 27 insertions(+), 12 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/4247e7df/CHANGELOG.asciidoc ---------------------------------------------------------------------- diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 47668bb..176678c 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -26,6 +26,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima TinkerPop 3.3.0 (Release Date: NOT OFFICIALLY RELEASED YET) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +* Detected if type checking was required in `GremlinGroovyScriptEngine` and disabled related infrastructure if not. * Removed previously deprecated `DetachedEdge(Object,String,Map,Pair,Pair)` constructor. * Removed previously deprecated `Bindings` constructor. It is now a private constructor. * Removed previously deprecated `TraversalSource.withBindings()`. http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/4247e7df/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyScriptEngine.java ---------------------------------------------------------------------- diff --git a/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyScriptEngine.java b/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyScriptEngine.java index fe12b1c..bc46d63 100644 --- a/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyScriptEngine.java +++ b/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyScriptEngine.java @@ -30,6 +30,7 @@ import groovy.lang.MissingPropertyException; import groovy.lang.Script; import groovy.lang.Tuple; import org.apache.tinkerpop.gremlin.groovy.loaders.GremlinLoader; +import org.apache.tinkerpop.gremlin.jsr223.ConcurrentBindings; import org.apache.tinkerpop.gremlin.jsr223.CoreGremlinPlugin; import org.apache.tinkerpop.gremlin.jsr223.Customizer; import org.apache.tinkerpop.gremlin.jsr223.GremlinScriptContext; @@ -65,13 +66,11 @@ import java.lang.reflect.Method; import java.lang.reflect.Proxy; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.ServiceLoader; import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; @@ -226,6 +225,11 @@ public class GremlinGroovyScriptEngine extends GroovyScriptEngineImpl private final long expectedCompilationTime; /** + * There is no need to require type checking infrastructure if type checking is not enabled. + */ + private final boolean typeCheckingEnabled; + + /** * Creates a new instance using no {@link Customizer}. */ public GremlinGroovyScriptEngine() { @@ -233,6 +237,9 @@ public class GremlinGroovyScriptEngine extends GroovyScriptEngineImpl } public GremlinGroovyScriptEngine(final Customizer... customizers) { + // initialize the global scope in case this scriptengine was instantiated outside of the ScriptEngineManager + setBindings(new ConcurrentBindings(), ScriptContext.GLOBAL_SCOPE); + final List<Customizer> listOfCustomizers = new ArrayList<>(Arrays.asList(customizers)); // always need this plugin for a scriptengine to be "Gremlin-enabled" @@ -258,6 +265,9 @@ public class GremlinGroovyScriptEngine extends GroovyScriptEngineImpl expectedCompilationTime = compilationOptionsCustomizerProvider.isPresent() ? compilationOptionsCustomizerProvider.get().getExpectedCompilationTime() : 5000; + typeCheckingEnabled = listOfCustomizers.stream() + .anyMatch(p -> p instanceof TypeCheckedGroovyCustomizer || p instanceof CompileStaticGroovyCustomizer); + // determine if interpreter mode should be enabled interpreterModeEnabled = groovyCustomizers.stream() .anyMatch(p -> p.getClass().equals(InterpreterModeGroovyCustomizer.class)); @@ -683,16 +693,20 @@ public class GremlinGroovyScriptEngine extends GroovyScriptEngineImpl } private void registerBindingTypes(final ScriptContext context) { - final Map<String, ClassNode> variableTypes = new HashMap<>(); - clearVarTypes(); - - // use null for the classtype if the binding value itself is null - not fully sure if that is - // a sound way to deal with that. didn't see a class type for null - maybe it should just be - // unknown and be "Object". at least null is properly being accounted for now. - context.getBindings(ScriptContext.ENGINE_SCOPE).forEach((k, v) -> - variableTypes.put(k, null == v ? null : ClassHelper.make(v.getClass()))); - - COMPILE_OPTIONS.get().put(COMPILE_OPTIONS_VAR_TYPES, variableTypes); + if (typeCheckingEnabled) { + final Map<String, ClassNode> variableTypes = new HashMap<>(); + clearVarTypes(); + + // use null for the classtype if the binding value itself is null - not fully sure if that is + // a sound way to deal with that. didn't see a class type for null - maybe it should just be + // unknown and be "Object". at least null is properly being accounted for now. + context.getBindings(ScriptContext.GLOBAL_SCOPE).forEach((k, v) -> + variableTypes.put(k, null == v ? null : ClassHelper.make(v.getClass()))); + context.getBindings(ScriptContext.ENGINE_SCOPE).forEach((k, v) -> + variableTypes.put(k, null == v ? null : ClassHelper.make(v.getClass()))); + + COMPILE_OPTIONS.get().put(COMPILE_OPTIONS_VAR_TYPES, variableTypes); + } } private static void clearVarTypes() {
