tinkerpop-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From spmalle...@apache.org
Subject [1/3] tinkerpop git commit: Fixed bug in script engine which wasn't registering globals
Date Tue, 18 Jul 2017 20:58:54 GMT
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 <spmva@genoprime.com>
Authored: Tue Jul 18 16:14:52 2017 -0400
Committer: Stephen Mallette <spmva@genoprime.com>
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() {


Mime
View raw message