You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by sp...@apache.org on 2017/09/29 11:21:01 UTC
[1/2] tinkerpop git commit: TINKERPOP-1792 Fixed GremlinScriptEngine
bug in lambda processing
Repository: tinkerpop
Updated Branches:
refs/heads/tp32 1cd042b61 -> 29b6268d9
TINKERPOP-1792 Fixed GremlinScriptEngine bug in lambda processing
GremlinScriptEngine instances should now properly select the appropriate TraversalSource from bindings rather than select a random one. The TraversalSource is locally aliased to a "hidden" variable at the time of evaluation to avoid naming clashes with local variables defined in the bytecode itself. Added a number of validations to be sure that users get appropriate errors if they try to use the eval() method the wrong way. Included new tests to ensure that GremlinScriptEngine have the appropriate behavior.
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/30185646
Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/30185646
Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/30185646
Branch: refs/heads/tp32
Commit: 30185646379b051b81484dd75ad34490fa7461b8
Parents: a13f922
Author: Stephen Mallette <sp...@genoprime.com>
Authored: Tue Sep 26 11:31:31 2017 -0400
Committer: Stephen Mallette <sp...@genoprime.com>
Committed: Tue Sep 26 11:31:31 2017 -0400
----------------------------------------------------------------------
CHANGELOG.asciidoc | 2 +
.../gremlin/jsr223/GremlinScriptEngine.java | 35 +++++-
.../groovy/jsr223/GroovyTranslatorTest.java | 6 +-
.../gremlin/groovy/engine/GremlinExecutor.java | 23 ++++
.../gremlin/groovy/engine/ScriptEngines.java | 15 +++
.../jsr223/GremlinGroovyScriptEngine.java | 33 ++++--
.../jsr223/GremlinJythonScriptEngine.java | 34 ++++--
.../op/traversal/TraversalOpProcessor.java | 14 +--
.../jsr223/GremlinEnabledScriptEngineTest.java | 117 +++++++++++++++++++
.../gremlin/jsr223/MockGremlinScriptEngine.java | 6 +-
10 files changed, 248 insertions(+), 37 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/30185646/CHANGELOG.asciidoc
----------------------------------------------------------------------
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index b3a92e2..faa999d 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -26,6 +26,8 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
TinkerPop 3.2.7 (Release Date: NOT OFFICIALLY RELEASED YET)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+* Fixed a bug where bytecode containing lambdas would randomly select a traversal source from bindings.
+* Deprecated `GremlinScriptEngine.eval()` methods and replaced them with new overloads that include the specific `TraversalSource` to bind to.
* Added `GraphHelper.cloneElements(Graph original, Graph clone)` to the `gremlin-test` module to quickly clone a graph.
* Bump to GMavenPlus 1.6.
* Added better error message for illegal use of `repeat()`-step.
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/30185646/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/GremlinScriptEngine.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/GremlinScriptEngine.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/GremlinScriptEngine.java
index 69ae901..addca82 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/GremlinScriptEngine.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/GremlinScriptEngine.java
@@ -32,12 +32,18 @@ import javax.script.ScriptException;
* @author Stephen Mallette (http://stephen.genoprime.com)
*/
public interface GremlinScriptEngine extends ScriptEngine {
+ public static final String HIDDEN_G = "gremlinscriptengine__g";
+
@Override
public GremlinScriptEngineFactory getFactory();
/**
- * Evaluates {@link Traversal} {@link Bytecode}.
+ * Evaluates {@link Traversal} {@link Bytecode}. This method assumes that the traversal source to execute the
+ * bytecode against is in the global bindings and is named "g".
+ *
+ * @deprecated As of release 3.2.7, replaced by {@link #eval(Bytecode, String)}.
*/
+ @Deprecated
public default Traversal.Admin eval(final Bytecode bytecode) throws ScriptException {
final Bindings bindings = this.createBindings();
bindings.putAll(bytecode.getBindings());
@@ -46,7 +52,32 @@ public interface GremlinScriptEngine extends ScriptEngine {
/**
* Evaluates {@link Traversal} {@link Bytecode} with the specified {@code Bindings}. These {@code Bindings}
+ * supplied to this method will be merged with global engine bindings and override them where keys match. This
+ * method assumes that the traversal source to execute against is named "g".
+ *
+ * @deprecated As of release 3.2.7, replaced by {@link #eval(Bytecode, Bindings, String)}.
+ */
+ @Deprecated
+ public default Traversal.Admin eval(final Bytecode bytecode, final Bindings bindings) throws ScriptException {
+ return eval(bytecode, bindings, "g");
+ }
+
+ /**
+ * Evaluates {@link Traversal} {@link Bytecode} against a traversal source in the global bindings of the
+ * {@code ScriptEngine}.
+ *
+ * @param bytecode of the traversal to execute
+ * @param traversalSource to execute the bytecode against which should be in the available bindings.
+ */
+ public default Traversal.Admin eval(final Bytecode bytecode, final String traversalSource) throws ScriptException {
+ final Bindings bindings = this.createBindings();
+ bindings.putAll(bytecode.getBindings());
+ return eval(bytecode, bindings, traversalSource);
+ }
+
+ /**
+ * Evaluates {@link Traversal} {@link Bytecode} with the specified {@code Bindings}. These {@code Bindings}
* supplied to this method will be merged with global engine bindings and override them where keys match.
*/
- public Traversal.Admin eval(final Bytecode bytecode, final Bindings bindings) throws ScriptException;
+ public Traversal.Admin eval(final Bytecode bytecode, final Bindings bindings, final String traversalSource) throws ScriptException;
}
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/30185646/gremlin-groovy-test/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GroovyTranslatorTest.java
----------------------------------------------------------------------
diff --git a/gremlin-groovy-test/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GroovyTranslatorTest.java b/gremlin-groovy-test/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GroovyTranslatorTest.java
index 133646a..08451a4 100644
--- a/gremlin-groovy-test/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GroovyTranslatorTest.java
+++ b/gremlin-groovy-test/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GroovyTranslatorTest.java
@@ -59,17 +59,17 @@ public class GroovyTranslatorTest extends AbstractGremlinTest {
}})));
final Bindings bindings = new SimpleBindings();
bindings.put("g", g);
- Traversal.Admin<Vertex, Object> traversal = new GremlinGroovyScriptEngine().eval(g.V().values("name").asAdmin().getBytecode(), bindings);
+ Traversal.Admin<Vertex, Object> traversal = new GremlinGroovyScriptEngine().eval(g.V().values("name").asAdmin().getBytecode(), bindings, "g");
assertEquals("marko", traversal.next());
assertFalse(traversal.hasNext());
//
- traversal = new GremlinGroovyScriptEngine().eval(g.withoutStrategies(SubgraphStrategy.class).V().count().asAdmin().getBytecode(), bindings);
+ traversal = new GremlinGroovyScriptEngine().eval(g.withoutStrategies(SubgraphStrategy.class).V().count().asAdmin().getBytecode(), bindings, "g");
assertEquals(new Long(6), traversal.next());
assertFalse(traversal.hasNext());
//
traversal = new GremlinGroovyScriptEngine().eval(g.withStrategies(SubgraphStrategy.create(new MapConfiguration(new HashMap<String, Object>() {{
put(SubgraphStrategy.VERTICES, __.has("name", "marko"));
- }})), ReadOnlyStrategy.instance()).V().values("name").asAdmin().getBytecode(), bindings);
+ }})), ReadOnlyStrategy.instance()).V().values("name").asAdmin().getBytecode(), bindings, "g");
assertEquals("marko", traversal.next());
assertFalse(traversal.hasNext());
}
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/30185646/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/engine/GremlinExecutor.java
----------------------------------------------------------------------
diff --git a/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/engine/GremlinExecutor.java b/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/engine/GremlinExecutor.java
index fabc8cb..945719a 100644
--- a/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/engine/GremlinExecutor.java
+++ b/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/engine/GremlinExecutor.java
@@ -345,7 +345,10 @@ public class GremlinExecutor implements AutoCloseable {
/**
* Evaluates bytecode with bindings for a specific language into a {@link Traversal}.
+ *
+ * @deprecated As of release 3.2.7, replaced by {@link #eval(Bytecode, Bindings, String, String)}
*/
+ @Deprecated
public Traversal.Admin eval(final Bytecode bytecode, final Bindings boundVars, final String language) throws ScriptException {
final String lang = Optional.ofNullable(language).orElse("gremlin-groovy");
@@ -358,6 +361,26 @@ public class GremlinExecutor implements AutoCloseable {
}
/**
+ * Evaluates bytecode with bindings for a specific language into a {@link Traversal}.
+ *
+ * @param bytecode to execute as a traversal
+ * @param boundVars local bindings
+ * @param language the scripting language to use to process the bytecode
+ * @param traversalSource the specific traversal source to execute the bytecode against
+ */
+ public Traversal.Admin eval(final Bytecode bytecode, final Bindings boundVars, final String language, final String traversalSource) throws ScriptException {
+ final String lang = Optional.ofNullable(language).orElse("gremlin-groovy");
+
+ final Bindings bindings = new SimpleBindings();
+ bindings.putAll(globalBindings);
+ bindings.putAll(boundVars);
+
+ return useGremlinScriptEngineManager ?
+ gremlinScriptEngineManager.getEngineByName(lang).eval(bytecode, bindings, traversalSource) :
+ scriptEngines.eval(bytecode, bindings, lang, traversalSource);
+ }
+
+ /**
* @deprecated As of release 3.2.4, replaced by {@link #getScriptEngineManager()}.
*/
@Deprecated
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/30185646/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/engine/ScriptEngines.java
----------------------------------------------------------------------
diff --git a/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/engine/ScriptEngines.java b/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/engine/ScriptEngines.java
index 6dfd87a..8521812 100644
--- a/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/engine/ScriptEngines.java
+++ b/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/engine/ScriptEngines.java
@@ -94,6 +94,9 @@ public class ScriptEngines implements AutoCloseable {
this.initializer.accept(this);
}
+ /**
+ * @deprecated As of release 3.2.7, replaced by {@link #eval(Bytecode, Bindings, String, String)}.
+ */
public Traversal.Admin eval(final Bytecode bytecode, final Bindings bindings, final String language) throws ScriptException {
if (!scriptEngines.containsKey(language))
throw new IllegalArgumentException(String.format("Language [%s] not supported", language));
@@ -106,6 +109,18 @@ public class ScriptEngines implements AutoCloseable {
return engine.eval(bytecode, all);
}
+ public Traversal.Admin eval(final Bytecode bytecode, final Bindings bindings, final String language, final String traversalSource) throws ScriptException {
+ if (!scriptEngines.containsKey(language))
+ throw new IllegalArgumentException(String.format("Language [%s] not supported", language));
+
+ awaitControlOp();
+
+ final GremlinScriptEngine engine = scriptEngines.get(language);
+ final Bindings all = mergeBindings(bindings, engine);
+
+ return engine.eval(bytecode, all, traversalSource);
+ }
+
/**
* Evaluate a script with {@code Bindings} for a particular language.
*/
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/30185646/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 a74d4d9..381dd88 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
@@ -442,16 +442,29 @@ public class GremlinGroovyScriptEngine extends GroovyScriptEngineImpl
}
@Override
- public Traversal.Admin eval(final Bytecode bytecode, final Bindings bindings) throws ScriptException {
- bindings.putAll(bytecode.getBindings());
- String traversalSource = "g";
- for (final Map.Entry<String, Object> entry : bindings.entrySet()) {
- if (entry.getValue() instanceof TraversalSource) {
- traversalSource = entry.getKey();
- break;
- }
- }
- return (Traversal.Admin) this.eval(GroovyTranslator.of(traversalSource).translate(bytecode), bindings);
+ public Traversal.Admin eval(final Bytecode bytecode, final Bindings bindings, final String traversalSource) throws ScriptException {
+ // these validations occur before merging in bytecode bindings which will override existing ones. need to
+ // extract the named traversalsource prior to that happening so that bytecode bindings can share the same
+ // namespace as global bindings (e.g. traversalsources and graphs).
+ if (traversalSource.equals(HIDDEN_G))
+ throw new IllegalArgumentException("The traversalSource cannot have the name " + HIDDEN_G+ " - it is reserved");
+
+ if (bindings.containsKey(HIDDEN_G))
+ throw new IllegalArgumentException("Bindings cannot include " + HIDDEN_G + " - it is reserved");
+
+ if (!bindings.containsKey(traversalSource))
+ throw new IllegalArgumentException("The bindings available to the ScriptEngine do not contain a traversalSource named: " + traversalSource);
+
+ final Object b = bindings.get(traversalSource);
+ if (!(b instanceof TraversalSource))
+ throw new IllegalArgumentException(traversalSource + " is of type " + b.getClass().getSimpleName() + " and is not an instance of TraversalSource");
+
+ final Bindings inner = new SimpleBindings();
+ inner.putAll(bindings);
+ inner.putAll(bytecode.getBindings());
+ inner.put(HIDDEN_G, b);
+
+ return (Traversal.Admin) this.eval(GroovyTranslator.of(HIDDEN_G).translate(bytecode), inner);
}
/**
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/30185646/gremlin-python/src/main/java/org/apache/tinkerpop/gremlin/python/jsr223/GremlinJythonScriptEngine.java
----------------------------------------------------------------------
diff --git a/gremlin-python/src/main/java/org/apache/tinkerpop/gremlin/python/jsr223/GremlinJythonScriptEngine.java b/gremlin-python/src/main/java/org/apache/tinkerpop/gremlin/python/jsr223/GremlinJythonScriptEngine.java
index 04473c0..10f4790 100644
--- a/gremlin-python/src/main/java/org/apache/tinkerpop/gremlin/python/jsr223/GremlinJythonScriptEngine.java
+++ b/gremlin-python/src/main/java/org/apache/tinkerpop/gremlin/python/jsr223/GremlinJythonScriptEngine.java
@@ -35,6 +35,7 @@ import org.python.jsr223.PyScriptEngineFactory;
import javax.script.Bindings;
import javax.script.ScriptContext;
import javax.script.ScriptException;
+import javax.script.SimpleBindings;
import java.io.Reader;
import java.lang.reflect.Method;
import java.util.ArrayList;
@@ -117,16 +118,29 @@ public class GremlinJythonScriptEngine implements GremlinScriptEngine {
}
@Override
- public Traversal.Admin eval(final Bytecode bytecode, final Bindings bindings) throws ScriptException {
- bindings.putAll(bytecode.getBindings());
- String traversalSource = "g";
- for (final Map.Entry<String, Object> entry : bindings.entrySet()) {
- if (entry.getValue() instanceof TraversalSource) {
- traversalSource = entry.getKey();
- break;
- }
- }
- return (Traversal.Admin) this.eval(JythonTranslator.of(traversalSource).translate(bytecode), bindings);
+ public Traversal.Admin eval(final Bytecode bytecode, final Bindings bindings, final String traversalSource) throws ScriptException {
+ // these validations occur before merging in bytecode bindings which will override existing ones. need to
+ // extract the named traversalsource prior to that happening so that bytecode bindings can share the same
+ // namespace as global bindings (e.g. traversalsources and graphs).
+ if (traversalSource.equals(HIDDEN_G))
+ throw new IllegalArgumentException("The traversalSource cannot have the name " + HIDDEN_G+ " - it is reserved");
+
+ if (bindings.containsKey(HIDDEN_G))
+ throw new IllegalArgumentException("Bindings cannot include " + HIDDEN_G + " - it is reserved");
+
+ if (!bindings.containsKey(traversalSource))
+ throw new IllegalArgumentException("The bindings available to the ScriptEngine do not contain a traversalSource named: " + traversalSource);
+
+ final Object b = bindings.get(traversalSource);
+ if (!(b instanceof TraversalSource))
+ throw new IllegalArgumentException(traversalSource + " is of type " + b.getClass().getSimpleName() + " and is not an instance of TraversalSource");
+
+ final Bindings inner = new SimpleBindings();
+ inner.putAll(bindings);
+ inner.putAll(bytecode.getBindings());
+ inner.put(HIDDEN_G, b);
+
+ return (Traversal.Admin) this.eval(JythonTranslator.of(HIDDEN_G).translate(bytecode), inner);
}
@Override
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/30185646/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/traversal/TraversalOpProcessor.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/traversal/TraversalOpProcessor.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/traversal/TraversalOpProcessor.java
index fc3066a..b9705a6 100644
--- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/traversal/TraversalOpProcessor.java
+++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/traversal/TraversalOpProcessor.java
@@ -22,13 +22,11 @@ import com.codahale.metrics.Timer;
import com.github.benmanes.caffeine.cache.Cache;
import com.github.benmanes.caffeine.cache.Caffeine;
import io.netty.channel.ChannelHandlerContext;
-import org.apache.commons.lang.time.StopWatch;
import org.apache.tinkerpop.gremlin.driver.MessageSerializer;
import org.apache.tinkerpop.gremlin.driver.Tokens;
import org.apache.tinkerpop.gremlin.driver.message.RequestMessage;
import org.apache.tinkerpop.gremlin.driver.message.ResponseMessage;
import org.apache.tinkerpop.gremlin.driver.message.ResponseStatusCode;
-import org.apache.tinkerpop.gremlin.groovy.engine.ScriptEngines;
import org.apache.tinkerpop.gremlin.jsr223.JavaTranslator;
import org.apache.tinkerpop.gremlin.process.traversal.Bytecode;
import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
@@ -56,8 +54,8 @@ import org.apache.tinkerpop.shaded.jackson.databind.ObjectMapper;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import javax.script.Bindings;
import javax.script.SimpleBindings;
-import java.lang.ref.WeakReference;
import java.lang.reflect.UndeclaredThrowableException;
import java.util.ArrayList;
import java.util.Collections;
@@ -67,7 +65,6 @@ import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.UUID;
-import java.util.concurrent.CompletableFuture;
import java.util.concurrent.Future;
import java.util.concurrent.FutureTask;
import java.util.concurrent.TimeUnit;
@@ -118,6 +115,8 @@ public class TraversalOpProcessor extends AbstractOpProcessor {
protected static Cache<UUID, TraversalSideEffects> cache = null;
+ private static final Bindings EMPTY_BINDINGS = new SimpleBindings();
+
public TraversalOpProcessor() {
super(false);
}
@@ -366,11 +365,8 @@ public class TraversalOpProcessor extends AbstractOpProcessor {
final Optional<String> lambdaLanguage = BytecodeHelper.getLambdaLanguage(bytecode);
if (!lambdaLanguage.isPresent())
traversal = JavaTranslator.of(g).translate(bytecode);
- else {
- final SimpleBindings b = new SimpleBindings();
- b.put(Tokens.VAL_TRAVERSAL_SOURCE_ALIAS, g);
- traversal = context.getGremlinExecutor().eval(bytecode, b, lambdaLanguage.get());
- }
+ else
+ traversal = context.getGremlinExecutor().eval(bytecode, EMPTY_BINDINGS, lambdaLanguage.get(), traversalSourceName);
} catch (Exception ex) {
logger.error("Could not deserialize the Traversal instance", context);
throw new OpProcessorException("Could not deserialize the Traversal instance",
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/30185646/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/jsr223/GremlinEnabledScriptEngineTest.java
----------------------------------------------------------------------
diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/jsr223/GremlinEnabledScriptEngineTest.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/jsr223/GremlinEnabledScriptEngineTest.java
index 5a880f8..4569257 100644
--- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/jsr223/GremlinEnabledScriptEngineTest.java
+++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/jsr223/GremlinEnabledScriptEngineTest.java
@@ -18,19 +18,32 @@
*/
package org.apache.tinkerpop.gremlin.jsr223;
+import org.apache.tinkerpop.gremlin.process.traversal.Step;
+import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
+import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversal;
+import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource;
import org.apache.tinkerpop.gremlin.structure.Edge;
import org.apache.tinkerpop.gremlin.structure.Graph;
import org.apache.tinkerpop.gremlin.structure.Vertex;
import org.apache.tinkerpop.gremlin.structure.VertexProperty;
+import org.apache.tinkerpop.gremlin.structure.util.empty.EmptyGraph;
+import org.apache.tinkerpop.gremlin.util.function.Lambda;
import org.junit.Test;
+import javax.script.Bindings;
+import javax.script.SimpleBindings;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Optional;
import static org.apache.tinkerpop.gremlin.jsr223.GremlinScriptEngineSuite.ENGINE_TO_TEST;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.AnyOf.anyOf;
+import static org.hamcrest.core.CombinableMatcher.either;
+import static org.hamcrest.core.Is.is;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assume.assumeThat;
/**
* This is an important test case in that it validates that core features of {@code ScriptEngine} instances that claim
@@ -42,6 +55,92 @@ public class GremlinEnabledScriptEngineTest {
private static final GremlinScriptEngineManager manager = new DefaultGremlinScriptEngineManager();
@Test
+ public void shouldEvalBytecode() throws Exception {
+ final GremlinScriptEngine scriptEngine = manager.getEngineByName(ENGINE_TO_TEST);
+ final Graph graph = EmptyGraph.instance();
+ final GraphTraversalSource g = graph.traversal();
+
+ // purposefully use "x" to match the name of the traversal source binding for "x" below and
+ // thus tests the alias added for "x"
+ final GraphTraversal t = getTraversalWithLambda(g);
+
+ final Bindings bindings = new SimpleBindings();
+ bindings.put("x", g);
+
+ final Traversal evald = scriptEngine.eval(t.asAdmin().getBytecode(), bindings, "x");
+
+ assertTraversals(t, evald);
+
+ assertThat(manager.getBindings().containsKey(GremlinScriptEngine.HIDDEN_G), is(false));
+ }
+
+ @Test(expected = IllegalArgumentException.class)
+ public void shouldNotAllowBytecodeEvalWithAliasInBindings() throws Exception {
+ final GremlinScriptEngine scriptEngine = manager.getEngineByName(ENGINE_TO_TEST);
+ final Graph graph = EmptyGraph.instance();
+ final GraphTraversalSource g = graph.traversal();
+
+ // purposefully use "x" to match the name of the traversal source binding for "x" below and
+ // thus tests the alias added for "x"
+ final GraphTraversal t = getTraversalWithLambda(g);
+
+ final Bindings bindings = new SimpleBindings();
+ bindings.put("x", g);
+ bindings.put(GremlinScriptEngine.HIDDEN_G, g);
+
+ scriptEngine.eval(t.asAdmin().getBytecode(), bindings, "x");
+ }
+
+ @Test(expected = IllegalArgumentException.class)
+ public void shouldNotAllowBytecodeEvalWithAliasAsTraversalSource() throws Exception {
+ final GremlinScriptEngine scriptEngine = manager.getEngineByName(ENGINE_TO_TEST);
+ final Graph graph = EmptyGraph.instance();
+ final GraphTraversalSource g = graph.traversal();
+
+ // purposefully use "x" to match the name of the traversal source binding for "x" below and
+ // thus tests the alias added for "x"
+ final GraphTraversal t = getTraversalWithLambda(g);
+
+ final Bindings bindings = new SimpleBindings();
+ bindings.put("x", g);
+
+ scriptEngine.eval(t.asAdmin().getBytecode(), bindings, GremlinScriptEngine.HIDDEN_G);
+ }
+
+ @Test(expected = IllegalArgumentException.class)
+ public void shouldNotAllowBytecodeEvalWithMissingBinding() throws Exception {
+ final GremlinScriptEngine scriptEngine = manager.getEngineByName(ENGINE_TO_TEST);
+ final Graph graph = EmptyGraph.instance();
+ final GraphTraversalSource g = graph.traversal();
+
+ // purposefully use "x" to match the name of the traversal source binding for "x" below and
+ // thus tests the alias added for "x"
+ final GraphTraversal t = getTraversalWithLambda(g);
+
+ final Bindings bindings = new SimpleBindings();
+ bindings.put("z", g);
+
+ scriptEngine.eval(t.asAdmin().getBytecode(), bindings, "x");
+ }
+
+ @Test(expected = IllegalArgumentException.class)
+ public void shouldNotAllowBytecodeEvalWithInvalidBinding() throws Exception {
+ final GremlinScriptEngine scriptEngine = manager.getEngineByName(ENGINE_TO_TEST);
+ final Graph graph = EmptyGraph.instance();
+ final GraphTraversalSource g = graph.traversal();
+
+ // purposefully use "x" to match the name of the traversal source binding for "x" below and
+ // thus tests the alias added for "x"
+ final GraphTraversal t = getTraversalWithLambda(g);
+
+ final Bindings bindings = new SimpleBindings();
+ bindings.put("z", g);
+ bindings.put("x", "invalid-binding-for-x-given-x-should-be-traversal-source");
+
+ scriptEngine.eval(t.asAdmin().getBytecode(), bindings, "x");
+ }
+
+ @Test
public void shouldGetEngineByName() throws Exception {
final GremlinScriptEngine scriptEngine = manager.getEngineByName(ENGINE_TO_TEST);
assertEquals(ENGINE_TO_TEST, scriptEngine.getFactory().getEngineName());
@@ -89,4 +188,22 @@ public class GremlinEnabledScriptEngineTest {
.appliesTo(Collections.singletonList("fake-script-engine")).create());
assertEquals(0, mgr.getCustomizers(ENGINE_TO_TEST).size());
}
+
+ private static GraphTraversal<Vertex, Long> getTraversalWithLambda(final GraphTraversalSource g) {
+ assumeThat("This test is not enabled for this ScriptEngine: " + ENGINE_TO_TEST, ENGINE_TO_TEST,
+ anyOf(is("gremlin-python"), is("gremlin-jython"), is("gremlin-groovy")));
+ if (ENGINE_TO_TEST.equals("gremlin-groovy"))
+ return g.V().out("created").map(Lambda.function("{x -> x.get().values('name')}")).count();
+ else if (ENGINE_TO_TEST.equals("gremlin-python") || ENGINE_TO_TEST.equals("gremlin-jython"))
+ return g.V().out("created").map(Lambda.function("x : x.get().values('name')")).count();
+ else
+ throw new RuntimeException("The " + ENGINE_TO_TEST + " ScriptEngine is not supported by this test");
+ }
+
+ private static void assertTraversals(final GraphTraversal t, final Traversal evald) {
+ final List<Step> steps = t.asAdmin().getSteps();
+ for (int ix = 0; ix < steps.size(); ix++) {
+ assertEquals(steps.get(ix).getClass(), evald.asAdmin().getSteps().get(ix).getClass());
+ }
+ }
}
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/30185646/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/jsr223/MockGremlinScriptEngine.java
----------------------------------------------------------------------
diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/jsr223/MockGremlinScriptEngine.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/jsr223/MockGremlinScriptEngine.java
index ac0c94f..4380df7 100644
--- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/jsr223/MockGremlinScriptEngine.java
+++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/jsr223/MockGremlinScriptEngine.java
@@ -41,17 +41,17 @@ public class MockGremlinScriptEngine extends AbstractScriptEngine implements Gre
}
@Override
- public Traversal.Admin eval(final Bytecode bytecode, final Bindings bindings) throws ScriptException {
+ public Object eval(final String script, final ScriptContext context) throws ScriptException {
return null;
}
@Override
- public Object eval(final String script, final ScriptContext context) throws ScriptException {
+ public Object eval(final Reader reader, final ScriptContext context) throws ScriptException {
return null;
}
@Override
- public Object eval(final Reader reader, final ScriptContext context) throws ScriptException {
+ public Traversal.Admin eval(Bytecode bytecode, Bindings bindings, String traversalSource) throws ScriptException {
return null;
}
[2/2] tinkerpop git commit: Merge branch 'TINKERPOP-1792' into tp32
Posted by sp...@apache.org.
Merge branch 'TINKERPOP-1792' into tp32
Conflicts:
CHANGELOG.asciidoc
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/29b6268d
Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/29b6268d
Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/29b6268d
Branch: refs/heads/tp32
Commit: 29b6268d917a19ffd064f93bf2e257ab5a8a849d
Parents: 1cd042b 3018564
Author: Stephen Mallette <sp...@genoprime.com>
Authored: Fri Sep 29 06:57:14 2017 -0400
Committer: Stephen Mallette <sp...@genoprime.com>
Committed: Fri Sep 29 06:57:14 2017 -0400
----------------------------------------------------------------------
CHANGELOG.asciidoc | 2 +
.../gremlin/jsr223/GremlinScriptEngine.java | 35 +++++-
.../groovy/jsr223/GroovyTranslatorTest.java | 6 +-
.../gremlin/groovy/engine/GremlinExecutor.java | 23 ++++
.../gremlin/groovy/engine/ScriptEngines.java | 15 +++
.../jsr223/GremlinGroovyScriptEngine.java | 33 ++++--
.../jsr223/GremlinJythonScriptEngine.java | 34 ++++--
.../op/traversal/TraversalOpProcessor.java | 14 +--
.../jsr223/GremlinEnabledScriptEngineTest.java | 117 +++++++++++++++++++
.../gremlin/jsr223/MockGremlinScriptEngine.java | 6 +-
10 files changed, 248 insertions(+), 37 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/29b6268d/CHANGELOG.asciidoc
----------------------------------------------------------------------
diff --cc CHANGELOG.asciidoc
index e48dfb5,faa999d..9f98c08
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@@ -21,9 -23,11 +21,11 @@@ limitations under the License
image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/images/nine-inch-gremlins.png[width=185]
[[release-3-2-7]]
-TinkerPop 3.2.7 (Release Date: NOT OFFICIALLY RELEASED YET)
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+=== TinkerPop 3.2.7 (Release Date: NOT OFFICIALLY RELEASED YET)
+* `ReferenceVertex` was missing its `label()` string. `ReferenceElement` now supports all label handling.
+ * Fixed a bug where bytecode containing lambdas would randomly select a traversal source from bindings.
+ * Deprecated `GremlinScriptEngine.eval()` methods and replaced them with new overloads that include the specific `TraversalSource` to bind to.
* Added `GraphHelper.cloneElements(Graph original, Graph clone)` to the `gremlin-test` module to quickly clone a graph.
* Bump to GMavenPlus 1.6.
* Added better error message for illegal use of `repeat()`-step.