This is an automated email from the ASF dual-hosted git repository. spmallette pushed a commit to branch TINKERPOP-3031 in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit d4892ada5432e9e850883aef6a76b78a81200840 Author: Stephen Mallette <[email protected]> AuthorDate: Tue Feb 13 13:30:13 2024 -0500 TINKERPOP-3031 Fixed bug in translation of g.tx() options --- CHANGELOG.asciidoc | 1 + .../gremlin/process/traversal/GraphOp.java | 7 +++++-- .../traversal/dsl/graph/GraphTraversalSource.java | 1 + .../traversal/translator/DotNetTranslator.java | 4 ++++ .../traversal/translator/GolangTranslator.java | 21 +++++++++++++-------- .../traversal/translator/GroovyTranslator.java | 5 ++++- .../traversal/translator/JavascriptTranslator.java | 4 ++++ .../traversal/translator/PythonTranslator.java | 6 +++++- .../process/traversal/util/BytecodeHelper.java | 1 + .../tinkerpop/gremlin/structure/Transaction.java | 14 ++++++++++++++ .../traversal/translator/DotNetTranslatorTest.java | 9 +++++++++ .../traversal/translator/GolangTranslatorTest.java | 9 +++++++++ .../traversal/translator/GroovyTranslatorTest.java | 9 +++++++++ .../translator/JavascriptTranslatorTest.java | 9 +++++++++ .../traversal/translator/PythonTranslatorTest.java | 9 +++++++++ 15 files changed, 97 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index e886f0a7ae..40fa474891 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -24,6 +24,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima === TinkerPop 3.6.7 (NOT OFFICIALLY RELEASED YET) * Fixed a bug in Gremlin.Net for .NET 8 that led to exceptions: `InvalidOperationException: Enumeration has not started. Call MoveNext.` +* Fixed bug in bytecode translation of `g.tx().commit()` and `g.tx().rollback()` in all languages. * Improved error message from `JavaTranslator` by including exception source. * Added missing `short` serialization (`gx:Int16`) to GraphSONV2 and GraphSONV3 in `gremlin-python` * Added tests for error handling for GLV's if tx.commit() is called remotely for graphs without transactions support. diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/GraphOp.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/GraphOp.java index 370009c193..ad47929414 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/GraphOp.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/GraphOp.java @@ -18,6 +18,9 @@ */ package org.apache.tinkerpop.gremlin.process.traversal; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; +import org.apache.tinkerpop.gremlin.structure.Transaction; + /** * A {@code GraphOp} or "graph operation" is a static {@link Bytecode} form that does not translate to a traversal * but instead refers to a specific function to perform on a graph instance. @@ -27,12 +30,12 @@ public enum GraphOp { /** * Commit a transaction. */ - TX_COMMIT(new Bytecode("tx", "commit")), + TX_COMMIT(new Bytecode(GraphTraversalSource.Symbols.tx, Transaction.Symbols.commit)), /** * Rollback a transaction. */ - TX_ROLLBACK(new Bytecode("tx", "rollback")); + TX_ROLLBACK(new Bytecode(GraphTraversalSource.Symbols.tx, Transaction.Symbols.rollback)); private final Bytecode bytecode; diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalSource.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalSource.java index c12bf41e46..a4bc9e33fa 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalSource.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalSource.java @@ -77,6 +77,7 @@ public class GraphTraversalSource implements TraversalSource { public static final String withBulk = "withBulk"; public static final String withPath = "withPath"; + public static final String tx = "tx"; } diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/translator/DotNetTranslator.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/translator/DotNetTranslator.java index 8e70b3000b..fbc2c1dc83 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/translator/DotNetTranslator.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/translator/DotNetTranslator.java @@ -33,6 +33,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.Traversal; import org.apache.tinkerpop.gremlin.process.traversal.TraversalSource; import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy; 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.strategy.TraversalStrategyProxy; import org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.PartitionStrategy; import org.apache.tinkerpop.gremlin.process.traversal.util.ConnectiveP; @@ -385,6 +386,9 @@ public final class DotNetTranslator implements Translator.ScriptTranslator { convertToScript(instArg); } script.append(","); + } else if (methodName.equals(GraphTraversalSource.Symbols.tx)) { + final String command = resolveSymbol(instruction.getArguments()[0].toString()); + script.append(").").append(command).append("()"); } else if (methodName.equals(GraphTraversal.Symbols.call)) { // call() // call(String) diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/translator/GolangTranslator.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/translator/GolangTranslator.java index 6aa6a006c1..8f9628bcb8 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/translator/GolangTranslator.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/translator/GolangTranslator.java @@ -26,10 +26,10 @@ import org.apache.tinkerpop.gremlin.process.traversal.P; import org.apache.tinkerpop.gremlin.process.traversal.Pick; import org.apache.tinkerpop.gremlin.process.traversal.SackFunctions; import org.apache.tinkerpop.gremlin.process.traversal.Script; -import org.apache.tinkerpop.gremlin.process.traversal.Text; import org.apache.tinkerpop.gremlin.process.traversal.TextP; import org.apache.tinkerpop.gremlin.process.traversal.Translator; import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; import org.apache.tinkerpop.gremlin.process.traversal.strategy.TraversalStrategyProxy; import org.apache.tinkerpop.gremlin.process.traversal.util.ConnectiveP; import org.apache.tinkerpop.gremlin.process.traversal.util.OrP; @@ -297,16 +297,21 @@ public final class GolangTranslator implements Translator.ScriptTranslator { final String methodName = instruction.getOperator(); final Object[] arguments = instruction.getArguments(); - script.append(".").append(resolveSymbol(methodName)).append("("); + if (methodName.equals(GraphTraversalSource.Symbols.tx)) { + final String command = resolveSymbol(arguments[0].toString()); + script.append(".").append(resolveSymbol(methodName)).append("().").append(resolveSymbol(command)).append("()"); + } else { + script.append(".").append(resolveSymbol(methodName)).append("("); - for (int i = 0; i < arguments.length; i++) { - convertToScript(arguments[i]); - if (i != arguments.length - 1) { - script.append(", "); + for (int i = 0; i < arguments.length; i++) { + convertToScript(arguments[i]); + if (i != arguments.length - 1) { + script.append(", "); + } } - } - script.append(")"); + script.append(")"); + } } return script; } diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/translator/GroovyTranslator.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/translator/GroovyTranslator.java index d87c841514..7d35e65857 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/translator/GroovyTranslator.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/translator/GroovyTranslator.java @@ -30,9 +30,9 @@ import org.apache.tinkerpop.gremlin.process.traversal.Script; import org.apache.tinkerpop.gremlin.process.traversal.TextP; import org.apache.tinkerpop.gremlin.process.traversal.Translator; 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.strategy.TraversalStrategyProxy; import org.apache.tinkerpop.gremlin.process.traversal.util.ConnectiveP; -import org.apache.tinkerpop.gremlin.process.traversal.util.OrP; import org.apache.tinkerpop.gremlin.structure.Edge; import org.apache.tinkerpop.gremlin.structure.Vertex; import org.apache.tinkerpop.gremlin.structure.VertexProperty; @@ -342,6 +342,9 @@ public final class GroovyTranslator implements Translator.ScriptTranslator { convertToScript(o); if (itty.hasNext()) script.append(","); } + } else if (methodName.equals(GraphTraversalSource.Symbols.tx)) { + final String command = instruction.getArguments()[0].toString(); + script.append(").").append(command).append("("); } else { final Iterator<Object> itty = Arrays.stream(instruction.getArguments()).iterator(); while (itty.hasNext()) { diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/translator/JavascriptTranslator.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/translator/JavascriptTranslator.java index c03fd01ebb..a81a3d5b40 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/translator/JavascriptTranslator.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/translator/JavascriptTranslator.java @@ -31,6 +31,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.TextP; import org.apache.tinkerpop.gremlin.process.traversal.Translator; import org.apache.tinkerpop.gremlin.process.traversal.TraversalSource; import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; import org.apache.tinkerpop.gremlin.process.traversal.strategy.TraversalStrategyProxy; import org.apache.tinkerpop.gremlin.process.traversal.util.ConnectiveP; import org.apache.tinkerpop.gremlin.process.traversal.util.OrP; @@ -322,6 +323,9 @@ public final class JavascriptTranslator implements Translator.ScriptTranslator { script.append(", (").append(castSecondArgTo).append(") "); convertToScript(instruction.getArguments()[1]); script.append(","); + } else if (methodName.equals(GraphTraversalSource.Symbols.tx)) { + final String command = instruction.getArguments()[0].toString(); + script.append(").").append(command).append("()"); } else { for (final Object object : instruction.getArguments()) { convertToScript(object); diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/translator/PythonTranslator.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/translator/PythonTranslator.java index 814cfcd9b3..659b90f44c 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/translator/PythonTranslator.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/translator/PythonTranslator.java @@ -32,6 +32,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.Translator; import org.apache.tinkerpop.gremlin.process.traversal.Traversal; import org.apache.tinkerpop.gremlin.process.traversal.TraversalSource; 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.strategy.TraversalStrategyProxy; import org.apache.tinkerpop.gremlin.process.traversal.util.ConnectiveP; import org.apache.tinkerpop.gremlin.process.traversal.util.OrP; @@ -308,7 +309,10 @@ public final class PythonTranslator implements Translator.ScriptTranslator { script.append("[0:").append(arguments[0].toString()).append("]"); else if (methodName.equals("values") && 1 == arguments.length && script.getScript().length() > 3 && !STEP_NAMES.contains(arguments[0].toString())) script.append(".").append(arguments[0].toString()); - else { + else if (methodName.equals(GraphTraversalSource.Symbols.tx)) { + final String command = instruction.getArguments()[0].toString(); + script.append(".").append(methodName).append("().").append(command).append("()"); + } else { script.append(".").append(resolveSymbol(methodName)).append("("); // python has trouble with java varargs...wrapping in collection seems to solve the problem diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/BytecodeHelper.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/BytecodeHelper.java index 06f819a628..7fa26c5f04 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/BytecodeHelper.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/BytecodeHelper.java @@ -272,6 +272,7 @@ public final class BytecodeHelper { put(TraversalSource.Symbols.withComputer, Collections.emptyList()); put(GraphTraversalSource.Symbols.withBulk, Collections.emptyList()); put(GraphTraversalSource.Symbols.withPath, Collections.emptyList()); + put(GraphTraversalSource.Symbols.tx, Collections.emptyList()); }}; byteCodeSymbolStepMap = Collections.unmodifiableMap(operationStepMap); } diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/Transaction.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/Transaction.java index d3a0eb60da..3b6bc74096 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/Transaction.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/Transaction.java @@ -36,6 +36,20 @@ import java.util.function.Consumer; */ public interface Transaction extends AutoCloseable { + //////////////// + + public static final class Symbols { + + private Symbols() { + // static fields only + } + + public static final String commit = "commit"; + public static final String rollback = "rollback"; + + } + + //////////////// /** * Opens a transaction. */ diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/translator/DotNetTranslatorTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/translator/DotNetTranslatorTest.java index 28256f79fe..f6c6b51c50 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/translator/DotNetTranslatorTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/translator/DotNetTranslatorTest.java @@ -19,6 +19,7 @@ package org.apache.tinkerpop.gremlin.process.traversal.translator; +import org.apache.tinkerpop.gremlin.process.traversal.GraphOp; import org.apache.tinkerpop.gremlin.process.traversal.Merge; import org.apache.tinkerpop.gremlin.process.traversal.Order; import org.apache.tinkerpop.gremlin.process.traversal.P; @@ -273,6 +274,14 @@ public class DotNetTranslatorTest { script4); } + @Test + public void shouldTranslateTx() { + String script = translator.translate(GraphOp.TX_COMMIT.getBytecode()).getScript(); + assertEquals("g.Tx().Commit()", script); + script = translator.translate(GraphOp.TX_ROLLBACK.getBytecode()).getScript(); + assertEquals("g.Tx().Rollback()", script); + } + private void assertTranslation(final String expectedTranslation, final Object... objs) { final String script = translator.translate(g.inject(objs).asAdmin().getBytecode()).getScript(); assertEquals(String.format("g.Inject(%s)", expectedTranslation), script); diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/translator/GolangTranslatorTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/translator/GolangTranslatorTest.java index cbc611f89b..fa81e35d5a 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/translator/GolangTranslatorTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/translator/GolangTranslatorTest.java @@ -21,6 +21,7 @@ package org.apache.tinkerpop.gremlin.process.traversal.translator; import org.apache.commons.text.StringEscapeUtils; import org.apache.tinkerpop.gremlin.process.traversal.Bytecode; +import org.apache.tinkerpop.gremlin.process.traversal.GraphOp; import org.apache.tinkerpop.gremlin.process.traversal.Translator; import org.apache.tinkerpop.gremlin.process.traversal.Traverser; import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; @@ -125,4 +126,12 @@ public class GolangTranslatorTest { assertEquals("g.Inject([]interface{}{[]interface{}{1, 2}, []interface{}{3, 4}})", translator.translate(g.inject(Arrays.asList(Arrays.asList(1,2),Arrays.asList(3,4))).asAdmin().getBytecode()).getScript()); } + + @Test + public void shouldTranslateTx() { + String script = translator.translate(GraphOp.TX_COMMIT.getBytecode()).getScript(); + assertEquals("g.Tx().Commit()", script); + script = translator.translate(GraphOp.TX_ROLLBACK.getBytecode()).getScript(); + assertEquals("g.Tx().Rollback()", script); + } } diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/translator/GroovyTranslatorTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/translator/GroovyTranslatorTest.java index 082c6cf353..575c7336b9 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/translator/GroovyTranslatorTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/translator/GroovyTranslatorTest.java @@ -19,6 +19,7 @@ package org.apache.tinkerpop.gremlin.process.traversal.translator; +import org.apache.tinkerpop.gremlin.process.traversal.GraphOp; import org.apache.tinkerpop.gremlin.process.traversal.Order; import org.apache.tinkerpop.gremlin.process.traversal.Pop; import org.apache.tinkerpop.gremlin.process.traversal.Scope; @@ -307,6 +308,14 @@ public class GroovyTranslatorTest { script4); } + @Test + public void shouldTranslateTx() { + String script = translator.translate(GraphOp.TX_COMMIT.getBytecode()).getScript(); + assertEquals("g.tx().commit()", script); + script = translator.translate(GraphOp.TX_ROLLBACK.getBytecode()).getScript(); + assertEquals("g.tx().rollback()", script); + } + private void assertTranslation(final String expectedTranslation, final Object... objs) { final String script = translator.translate(g.inject(objs)).getScript(); assertEquals(String.format("g.inject(%s)", expectedTranslation), script); diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/translator/JavascriptTranslatorTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/translator/JavascriptTranslatorTest.java index 98de2f988c..88bff57ee6 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/translator/JavascriptTranslatorTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/translator/JavascriptTranslatorTest.java @@ -19,6 +19,7 @@ package org.apache.tinkerpop.gremlin.process.traversal.translator; +import org.apache.tinkerpop.gremlin.process.traversal.GraphOp; import org.apache.tinkerpop.gremlin.process.traversal.Order; import org.apache.tinkerpop.gremlin.process.traversal.Pop; import org.apache.tinkerpop.gremlin.process.traversal.Scope; @@ -197,6 +198,14 @@ public class JavascriptTranslatorTest { script4); } + @Test + public void shouldTranslateTx() { + String script = translator.translate(GraphOp.TX_COMMIT.getBytecode()).getScript(); + assertEquals("g.tx().commit()", script); + script = translator.translate(GraphOp.TX_ROLLBACK.getBytecode()).getScript(); + assertEquals("g.tx().rollback()", script); + } + private void assertTranslation(final String expectedTranslation, final Object... objs) { final String script = translator.translate(g.inject(objs).asAdmin().getBytecode()).getScript(); assertEquals(String.format("g.inject(%s)", expectedTranslation), script); diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/translator/PythonTranslatorTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/translator/PythonTranslatorTest.java index 9fee3ed4a9..c2b4e39a2d 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/translator/PythonTranslatorTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/translator/PythonTranslatorTest.java @@ -19,6 +19,7 @@ package org.apache.tinkerpop.gremlin.process.traversal.translator; import org.apache.tinkerpop.gremlin.process.traversal.Bytecode; +import org.apache.tinkerpop.gremlin.process.traversal.GraphOp; import org.apache.tinkerpop.gremlin.process.traversal.TextP; import org.apache.tinkerpop.gremlin.process.traversal.Translator; import org.apache.tinkerpop.gremlin.process.traversal.Traverser; @@ -131,6 +132,14 @@ public class PythonTranslatorTest { assertEquals("g.V().range_(0,10).has('person','name','marko').limit(2).values('name')", gremlinAsPython); } + @Test + public void shouldTranslateTx() { + String script = translator.translate(GraphOp.TX_COMMIT.getBytecode()).getScript(); + assertEquals("g.tx().commit()", script); + script = translator.translate(GraphOp.TX_ROLLBACK.getBytecode()).getScript(); + assertEquals("g.tx().rollback()", script); + } + private void assertTranslation(final String expectedTranslation, final Object... objs) { final String script = translator.translate(g.inject(objs).asAdmin().getBytecode()).getScript(); assertEquals(String.format("g.inject(%s)", expectedTranslation), script);
