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 2018/03/08 22:20:24 UTC
tinkerpop git commit: TINKERPOP-1896 Fixed bug in lambda processing
for python
Repository: tinkerpop
Updated Branches:
refs/heads/TINKERPOP-1896 [created] 5c5df83e7
TINKERPOP-1896 Fixed bug in lambda processing for python
When a lambda is in a traversal, the bytecode gets pushed to a scriptengine for evaluation. For python that means, using the JythonTranslator. Prior to this change the JythonTranslator largely relied on the PythonTranslator to convert bytecode to Jython-syntaxed Gremlin. The problem there is that in the JythonScriptEngine, the Traversal bindings are Java objects and so if translated to pure Python syntax, traversal steps like as() become as_() and thus not part of the Java API that is expected - and then errors. Not sure why this didn't come up sooner, but it really only ends up being an issue if you are using bytecode, use gremlin server, use jython in gremlin server and then submit a traversal with a lambda - so perhaps that hasn't been a common combination.
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/5c5df83e
Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/5c5df83e
Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/5c5df83e
Branch: refs/heads/TINKERPOP-1896
Commit: 5c5df83e71ab0071c1d073a2ca68c5822ea9336d
Parents: 7bff988
Author: Stephen Mallette <sp...@genoprime.com>
Authored: Thu Mar 8 17:15:34 2018 -0500
Committer: Stephen Mallette <sp...@genoprime.com>
Committed: Thu Mar 8 17:15:34 2018 -0500
----------------------------------------------------------------------
CHANGELOG.asciidoc | 1 +
.../jsr223/GremlinJythonScriptEngine.java | 7 +--
.../gremlin/python/jsr223/JythonTranslator.java | 9 ++++
.../gremlin/python/jsr223/PythonTranslator.java | 19 +++++---
.../python/jsr223/JythonTranslatorTest.java | 47 +++++++++-----------
5 files changed, 44 insertions(+), 39 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/5c5df83e/CHANGELOG.asciidoc
----------------------------------------------------------------------
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index a369172..3b084b7 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -29,6 +29,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
* Fixed deserialization of `P.not()` for GraphSON.
* Bumped to Jackson 2.9.4.
* Added `idleConnectionTimeout` and `keepAliveInterval` to Gremlin Server that enables a "ping" and auto-close for seemingly dead clients.
+* Fixed a bug where lambdas in `gremlin-python` would trigger a failure if steps using python-only symbols were present (such as `as_()`).
* Fixed a bug in `NumberHelper` that led to wrong min/max results if numbers exceeded the Integer limits.
* Delayed setting of the request identifier until `RequestMessage` construction by the builder.
* Improved error messaging for failed serialization and deserialization of request/response messages.
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/5c5df83e/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 10f4790..a28a58f 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
@@ -41,11 +41,11 @@ import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
-import java.util.Map;
import java.util.stream.Collectors;
/**
* @author Marko A. Rodriguez (http://markorodriguez.com)
+ * @author Stephen Mallette (http://stephen.genoprime.com)
*/
public class GremlinJythonScriptEngine implements GremlinScriptEngine {
@@ -123,10 +123,7 @@ public class GremlinJythonScriptEngine implements GremlinScriptEngine {
// 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");
+ throw new IllegalArgumentException("The traversalSource cannot have the name " + 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);
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/5c5df83e/gremlin-python/src/main/java/org/apache/tinkerpop/gremlin/python/jsr223/JythonTranslator.java
----------------------------------------------------------------------
diff --git a/gremlin-python/src/main/java/org/apache/tinkerpop/gremlin/python/jsr223/JythonTranslator.java b/gremlin-python/src/main/java/org/apache/tinkerpop/gremlin/python/jsr223/JythonTranslator.java
index 0eb8961..f8a5bc3 100644
--- a/gremlin-python/src/main/java/org/apache/tinkerpop/gremlin/python/jsr223/JythonTranslator.java
+++ b/gremlin-python/src/main/java/org/apache/tinkerpop/gremlin/python/jsr223/JythonTranslator.java
@@ -23,6 +23,7 @@ import org.apache.tinkerpop.gremlin.util.function.Lambda;
/**
* @author Marko A. Rodriguez (http://markorodriguez.com)
+ * @author Stephen Mallette (http://stephen.genoprime.com)
*/
public final class JythonTranslator extends PythonTranslator {
@@ -54,4 +55,12 @@ public final class JythonTranslator extends PythonTranslator {
else
return "JythonUnknownArgLambda(" + lambdaString + ")";
}
+
+ @Override
+ protected String resolveSymbol(final String methodName) {
+ // since this is Jython we should expect the Gremlin to conform to the java classes to which the engine is
+ // bound - therefore, unlike the python engine which converts java names to python friendly ones, this
+ // jython one can just pass them through.
+ return methodName;
+ }
}
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/5c5df83e/gremlin-python/src/main/java/org/apache/tinkerpop/gremlin/python/jsr223/PythonTranslator.java
----------------------------------------------------------------------
diff --git a/gremlin-python/src/main/java/org/apache/tinkerpop/gremlin/python/jsr223/PythonTranslator.java b/gremlin-python/src/main/java/org/apache/tinkerpop/gremlin/python/jsr223/PythonTranslator.java
index 4af4578..7c74b85 100644
--- a/gremlin-python/src/main/java/org/apache/tinkerpop/gremlin/python/jsr223/PythonTranslator.java
+++ b/gremlin-python/src/main/java/org/apache/tinkerpop/gremlin/python/jsr223/PythonTranslator.java
@@ -56,6 +56,7 @@ import java.util.stream.Stream;
/**
* @author Marko A. Rodriguez (http://markorodriguez.com)
+ * @author Stephen Mallette (http://stephen.genoprime.com)
*/
public class PythonTranslator implements Translator.ScriptTranslator {
@@ -114,7 +115,7 @@ public class PythonTranslator implements Translator.ScriptTranslator {
instruction.getArguments()[0].toString().contains("TranslationStrategy"))
continue;
else if (0 == arguments.length)
- traversalScript.append(".").append(SymbolHelper.toPython(methodName)).append("()");
+ traversalScript.append(".").append(resolveSymbol(methodName)).append("()");
else if (methodName.equals("range") && 2 == arguments.length)
traversalScript.append("[").append(arguments[0]).append(":").append(arguments[1]).append("]");
else if (methodName.equals("limit") && 1 == arguments.length)
@@ -123,7 +124,7 @@ public class PythonTranslator implements Translator.ScriptTranslator {
traversalScript.append(".").append(arguments[0]);
else {
traversalScript.append(".");
- String temp = SymbolHelper.toPython(methodName) + "(";
+ String temp = resolveSymbol(methodName) + "(";
for (final Object object : arguments) {
temp = temp + convertToString(object) + ",";
}
@@ -131,7 +132,7 @@ public class PythonTranslator implements Translator.ScriptTranslator {
}
// clip off __.
if (this.importStatics && traversalScript.substring(0, 3).startsWith("__.")
- && !NO_STATIC.stream().filter(name -> traversalScript.substring(3).startsWith(SymbolHelper.toPython(name))).findAny().isPresent()) {
+ && !NO_STATIC.stream().filter(name -> traversalScript.substring(3).startsWith(resolveSymbol(name))).findAny().isPresent()) {
traversalScript.delete(0, 3);
}
}
@@ -183,13 +184,13 @@ public class PythonTranslator implements Translator.ScriptTranslator {
else if (object instanceof Class)
return ((Class) object).getCanonicalName();
else if (object instanceof VertexProperty.Cardinality)
- return "Cardinality." + SymbolHelper.toPython(object.toString());
+ return "Cardinality." + resolveSymbol(object.toString());
else if (object instanceof SackFunctions.Barrier)
- return "Barrier." + SymbolHelper.toPython(object.toString());
+ return "Barrier." + resolveSymbol(object.toString());
else if (object instanceof TraversalOptionParent.Pick)
- return "Pick." + SymbolHelper.toPython(object.toString());
+ return "Pick." + resolveSymbol(object.toString());
else if (object instanceof Enum)
- return convertStatic(((Enum) object).getDeclaringClass().getSimpleName() + ".") + SymbolHelper.toPython(object.toString());
+ return convertStatic(((Enum) object).getDeclaringClass().getSimpleName() + ".") + resolveSymbol(object.toString());
else if (object instanceof P)
return convertPToString((P) object, new StringBuilder()).toString();
else if (object instanceof Element) {
@@ -237,4 +238,8 @@ public class PythonTranslator implements Translator.ScriptTranslator {
return lambdaString.startsWith("lambda") ? lambdaString : "lambda " + lambdaString;
}
+ protected String resolveSymbol(final String methodName) {
+ return SymbolHelper.toPython(methodName);
+ }
+
}
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/5c5df83e/gremlin-python/src/test/java/org/apache/tinkerpop/gremlin/python/jsr223/JythonTranslatorTest.java
----------------------------------------------------------------------
diff --git a/gremlin-python/src/test/java/org/apache/tinkerpop/gremlin/python/jsr223/JythonTranslatorTest.java b/gremlin-python/src/test/java/org/apache/tinkerpop/gremlin/python/jsr223/JythonTranslatorTest.java
index 5165120..86e3b78 100644
--- a/gremlin-python/src/test/java/org/apache/tinkerpop/gremlin/python/jsr223/JythonTranslatorTest.java
+++ b/gremlin-python/src/test/java/org/apache/tinkerpop/gremlin/python/jsr223/JythonTranslatorTest.java
@@ -22,11 +22,11 @@ package org.apache.tinkerpop.gremlin.python.jsr223;
import org.apache.tinkerpop.gremlin.process.traversal.Traverser;
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.process.traversal.dsl.graph.__;
import org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.TranslationStrategy;
import org.apache.tinkerpop.gremlin.structure.Vertex;
import org.apache.tinkerpop.gremlin.tinkergraph.structure.TinkerFactory;
import org.apache.tinkerpop.gremlin.util.function.Lambda;
-import org.junit.Ignore;
import org.junit.Test;
import java.util.ArrayList;
@@ -37,39 +37,15 @@ import static org.junit.Assert.assertFalse;
/**
* @author Marko A. Rodriguez (http://markorodriguez.com)
+ * @author Stephen Mallette (http://stephen.genoprime.com)
*/
-@Ignore
public class JythonTranslatorTest {
- /*@Test
- public void shouldHandleStrategies() throws Exception {
- GraphTraversalSource g = TinkerFactory.createModern().traversal();
- g = g.withStrategies(new HashMap<String, Object>() {{
- put(SubgraphStrategy.STRATEGY, SubgraphStrategy.class.getCanonicalName());
- put(SubgraphStrategy.VERTICES, __.has("name", "marko"));
- }});
- final Bindings bindings = new SimpleBindings();
- bindings.put("g", g);
- //System.out.println(JythonTranslator.of("g").translate(g.V().values("name").asAdmin().getBytecode()));
- Traversal.Admin<Vertex, String> traversal = ((GremlinScriptEngine) ScriptEngineCache.get("gremlin-jython")).eval(g.V().values("name").asAdmin().getBytecode(), bindings);
- assertEquals("marko", traversal.next());
- assertFalse(traversal.hasNext());
- //
- g = g.withStrategies(new HashMap<String, Object>() {{
- put(SubgraphStrategy.STRATEGY, SubgraphStrategy.class.getCanonicalName());
- put(SubgraphStrategy.VERTICES, __.has("name", "marko"));
- }}, Collections.singletonMap(ReadOnlyStrategy.STRATEGY, ReadOnlyStrategy.class.getCanonicalName()));
- //System.out.println(JythonTranslator.of("g").translate(g.V().values("name").asAdmin().getBytecode()));
- traversal = ((GremlinScriptEngine) ScriptEngineCache.get("gremlin-jython")).eval(g.V().values("name").asAdmin().getBytecode(), bindings);
- assertEquals("marko", traversal.next());
- assertFalse(traversal.hasNext());
- }*/
-
@Test
public void shouldSupportStringSupplierLambdas() throws Exception {
GraphTraversalSource g = TinkerFactory.createModern().traversal();
g = g.withStrategies(new TranslationStrategy(g, JythonTranslator.of("g")));
- GraphTraversal.Admin<Vertex, Integer> t = g.withSideEffect("lengthSum", 0).withSack(1)
+ final GraphTraversal.Admin<Vertex, Integer> t = g.withSideEffect("lengthSum", 0).withSack(1)
.V()
.filter(Lambda.predicate("x : x.get().label() == 'person'"))
.flatMap(Lambda.function("lambda x : x.get().vertices(Direction.OUT)"))
@@ -110,4 +86,21 @@ public class JythonTranslatorTest {
public void shouldHaveValidToString() {
assertEquals("translator[h:gremlin-jython]", JythonTranslator.of("h").toString());
}
+
+ @Test
+ public void shouldTranslateToJythonAndNotPython() throws Exception {
+ // the jython translation bind "g" to java classes and thus does not require the strict python syntax which
+ // converts steps like as() to as_(). if those steps are converted then the traversal does not evaluate
+ // properly in the python engine. not much of an assertion here to worry about - just need to ensure that
+ // the traversal with such steps evaluates to success
+ GraphTraversalSource g = TinkerFactory.createModern().traversal();
+ g = g.withStrategies(new TranslationStrategy(g, JythonTranslator.of("g")));
+ final List<Object> o = g.V().has("name").
+ match(__.as("x").label().as("lbl"),
+ __.as("x").id().as("id")).
+ select("lbl", "id").
+ map(Lambda.function("lambda x: type(x.get())")).toList();
+
+ assertEquals(6, o.size());
+ }
}