TINKERPOP-3247 Convert request bindings to gremlin-lang string format Moving parameters from binary-serialized maps to string representations makes the request side pure text, decoupling Gremlin language evolution from GraphBinary versioning. New types can be introduced in minor/patch versions without touching GraphBinary, eliminating the need for a major version bump across the ecosystem for every new request-side type. The asParameter() fallback is replaced with an unsupportedType flag that records the class name and falls back to toString(). A flag is used rather than throwing because embedded Traversals build GremlinLang as a side effect but never send it, so unknown types must not break execution. All other GLVs throw immediately since they have no embedded mode and the early throw gives better errors. Client APIs accept both map and string bindings (but not both at the same time) because users who use the Client directly with raw Gremlin strings shouldn't need to hand-craft gremlin-lang map literals. Mixing both throws immediately to prevent silent loss where one set of bindings would be discarded. Edge and VertexProperty tests that relied on the old asParameter fallback were removed because they aren't supported in gremlin-lang.
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 71a22eb..211e472 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc
@@ -31,6 +31,10 @@ * Changed `Client.stream()` in `gremlin-javascript` to return an `AsyncGenerator` for direct incremental consumption. * Removed `readable-stream` dependency from `gremlin-javascript`. * Added `Character`, `Binary`, `Duration` to the `gremlin-lang` grammar, GLVs, and Translators. +* Modified request parameters from `Map<String, Object>` to gremlin-lang compatible `String`. +* Modified HTTP API to expect gremlin-lang strings for parameters and update all GLVs to send requests in new format. +* Added string parameter parsing to `GremlinServer` to prevent traversal injection and excessive nesting depths. +* Modified all GLVs to detect unsupported types in `GremlinLang` and throw consistent error for that case. [[release-4-0-0-beta-2]] === TinkerPop 4.0.0-beta.2 (April 1, 2026)
diff --git a/docs/src/dev/provider/index.asciidoc b/docs/src/dev/provider/index.asciidoc index 87fc8c4..7654807 100644 --- a/docs/src/dev/provider/index.asciidoc +++ b/docs/src/dev/provider/index.asciidoc
@@ -1004,7 +1004,7 @@ { "gremlin": string, "timeoutMs": number, - "bindings": object, + "bindings": string, "g": string, "language" : string, "materializeProperties": string, @@ -1064,7 +1064,7 @@ |Key |Description |Value |Required |gremlin |The Gremlin query to execute. |String containing script |Yes |timeoutMs |The maximum time a query is allowed to execute in milliseconds. |Number between 0 and 2^31-1 |No -|bindings |A map used during query execution. Its usage depends on "language". For "gremlin-groovy", these are the variable bindings. For "gremlin-lang", these are the parameter bindings. |Object (Map) |No +|bindings |A gremlin-lang string that contains a map used during query execution. Its usage depends on "language". For "gremlin-groovy", these are the variable bindings. For "gremlin-lang", these are the parameter bindings. |Object (Map) |No |g |The name of the graph traversal source to which the query applies. Default: "g" |String containing traversal source name |No |language |The name of the ScriptEngine to use to parse the gremlin query. Default: "gremlin-lang" |String containing ScriptEngine name |No |materializeProperties |Whether to include all properties for results. One of "tokens" or "all". |String |No
diff --git a/docs/src/upgrade/release-4.x.x.asciidoc b/docs/src/upgrade/release-4.x.x.asciidoc index 77d1476..c60ca67 100644 --- a/docs/src/upgrade/release-4.x.x.asciidoc +++ b/docs/src/upgrade/release-4.x.x.asciidoc
@@ -138,6 +138,17 @@ See: link:https://issues.apache.org/jira/browse/TINKERPOP-3153[TINKERPOP-3153] +==== `gremlin-lang` based Parameters + +Bindings/parameters that are sent as part of the request are now `gremlin-lang` string maps rather than an actual Map +that would have been serialized based on the serializer used for the request. A side effect of this is that the map key +must be a valid Java identifier. This means that certain items can no longer be sent like `#jsr223` control flags that +were used for the `gremlin-groovy` ScriptEngine. Ensure that you are using valid Java identifiers (e.g. start with a +letter, no spaces, can't use reserved keywords, etc.) for keys. + +See: link:https://issues.apache.org/jira/browse/TINKERPOP-3247[TINKERPOP-3247] + + === Upgrading for Providers ==== Graph System Providers
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/language/grammar/GremlinQueryParser.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/language/grammar/GremlinQueryParser.java index d17fa5a..871233f 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/language/grammar/GremlinQueryParser.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/language/grammar/GremlinQueryParser.java
@@ -18,7 +18,6 @@ */ package org.apache.tinkerpop.gremlin.language.grammar; -import org.antlr.v4.runtime.CharStream; import org.antlr.v4.runtime.CharStreams; import org.antlr.v4.runtime.CommonTokenStream; import org.antlr.v4.runtime.atn.PredictionMode; @@ -26,6 +25,10 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.util.Collection; +import java.util.Map; +import javax.lang.model.SourceVersion; + /** * Parses Gremlin strings to an {@code Object}, typically to a {@link Traversal}. */ @@ -44,21 +47,15 @@ * Parse Gremlin string using a specified {@link GremlinAntlrToJava} object. */ public static Object parse(final String query, final GremlinVisitor<Object> visitor) { - final CharStream in = CharStreams.fromString(query); - final GremlinLexer lexer = new GremlinLexer(in); - lexer.removeErrorListeners(); - lexer.addErrorListener(errorListener); - + final GremlinLexer lexer = createLexer(query); final CommonTokenStream tokens = new CommonTokenStream(lexer); // Setup error handler on parser - final GremlinParser parser = new GremlinParser(tokens); + final GremlinParser parser = createParser(tokens); // SLL prediction mode is faster than the LL prediction mode when parsing the grammar, // but it does not cover parsing all types of input. We use the SLL by default, and fallback // to LL mode if fails to parse the query. parser.getInterpreter().setPredictionMode(PredictionMode.SLL); - parser.removeErrorListeners(); - parser.addErrorListener(errorListener); GremlinParser.QueryListContext queryContext; try { @@ -91,4 +88,94 @@ throw new GremlinParserException("Failed to interpret Gremlin query: " + ex.getMessage(), ex); } } + + /** + * Parses a gremlin-lang map literal string into a {@code Map<String, Object>} for use as parameters. + * <p> + * Uses {@link ParameterMapVisitor} to prevent traversal injection and validates that all keys are strings + * and no values contain traversals. + * + * @param parameterMapString the gremlin-lang map literal string (e.g. {@code [x:1,y:"marko"]}) or {@code null}/empty + * @return the parsed and validated parameter map + * @throws GremlinParserException if parsing fails or validation detects invalid content + */ + public static Map<String, Object> parseParameters(final String parameterMapString) { + if (parameterMapString == null || parameterMapString.isEmpty()) { + return Map.of(); + } + + final GremlinParser parser = createParser(parameterMapString); + final GremlinParser.GenericMapLiteralContext mapCtx = parser.genericMapLiteral(); + + final ParameterMapVisitor visitor = new ParameterMapVisitor(new GremlinAntlrToJava()); + final Map<Object, Object> rawMap = (Map<Object, Object>) visitor.visitGenericMapLiteral(mapCtx); + + if (rawMap == null) { + return Map.of(); + } + + for (final Map.Entry<?, ?> entry : rawMap.entrySet()) { + if (!(entry.getKey() instanceof String)) { + throw new GremlinParserException( + String.format("Parameter map keys must be String, found: %s", + entry.getKey() == null ? "null" : entry.getKey().getClass().getSimpleName())); + } + final String key = (String) entry.getKey(); + if (!SourceVersion.isIdentifier(key)) { + throw new GremlinParserException( + String.format("Parameter map key must be a valid identifier: %s", key)); + } + validateParameterValue(entry.getValue()); + } + + return (Map<String, Object>) (Map<?, ?>) rawMap; + } + + /** + * Recursively validates that a parameter value does not contain a {@link Traversal}. Nested validation is needed + * because steps like mergeV iterate map values, so a Traversal hiding inside a nested map or collection would still + * be dangerous. + */ + private static void validateParameterValue(final Object value) { + if (value instanceof Traversal) { + throw new GremlinParserException("Traversals are not allowed as parameter values"); + } + if (value instanceof Map) { + for (final Object v : ((Map<?, ?>) value).values()) { + validateParameterValue(v); + } + } + if (value instanceof Collection) { + for (final Object v : (Collection<?>) value) { + validateParameterValue(v); + } + } + } + + /** + * Creates a {@link GremlinParser} from the given input string. + */ + private static GremlinParser createParser(final String input) { + return createParser(new CommonTokenStream(createLexer(input))); + } + + /** + * Creates a {@link GremlinParser} from the given {@link GremlinLexer}. + */ + private static GremlinParser createParser(final CommonTokenStream tokens) { + final GremlinParser parser = new GremlinParser(tokens); + parser.removeErrorListeners(); + parser.addErrorListener(errorListener); + return parser; + } + + /** + * Creates a {@link GremlinLexer} from the given input string with error listeners configured. + */ + private static GremlinLexer createLexer(final String input) { + final GremlinLexer lexer = new GremlinLexer(CharStreams.fromString(input)); + lexer.removeErrorListeners(); + lexer.addErrorListener(errorListener); + return lexer; + } }
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/language/grammar/ParameterMapVisitor.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/language/grammar/ParameterMapVisitor.java new file mode 100644 index 0000000..b256693 --- /dev/null +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/language/grammar/ParameterMapVisitor.java
@@ -0,0 +1,102 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.tinkerpop.gremlin.language.grammar; + +/** + * A visitor for parsing parameter map strings that prevents traversal injection. + * <p> + * Extends {@link GenericLiteralVisitor} and overrides traversal-related visit methods + * to throw, ensuring that no traversal can be constructed or executed during the + * parameter map parsing walk. This is critical for security because + * {@code visitTerminatedTraversal} in the base class would execute the traversal + * immediately via {@link TraversalTerminalMethodVisitor}. + */ +public class ParameterMapVisitor extends GenericLiteralVisitor { + + private static final int DEFAULT_MAX_NESTING_DEPTH = 32; + + private final int maxNestingDepth; + private int currentNestingDepth = 0; + + public ParameterMapVisitor(final GremlinAntlrToJava antlr) { + this(antlr, DEFAULT_MAX_NESTING_DEPTH); + } + + public ParameterMapVisitor(final GremlinAntlrToJava antlr, final int maxNestingDepth) { + super(antlr); + this.maxNestingDepth = maxNestingDepth; + } + + /** + * Overridden to prevent nested traversal construction in parameter maps. + */ + @Override + public Object visitNestedTraversal(final GremlinParser.NestedTraversalContext ctx) { + throw new GremlinParserException("Traversals are not allowed in parameter maps"); + } + + /** + * Overridden to prevent terminated traversal execution in parameter maps. + * This is the critical override because the base class would execute the traversal + * immediately via {@link TraversalTerminalMethodVisitor}. + */ + @Override + public Object visitTerminatedTraversal(final GremlinParser.TerminatedTraversalContext ctx) { + throw new GremlinParserException("Traversals are not allowed in parameter maps"); + } + + @Override + public Object visitGenericMapLiteral(final GremlinParser.GenericMapLiteralContext ctx) { + currentNestingDepth++; + if (currentNestingDepth > maxNestingDepth) { + throw new GremlinParserException("Parameter map nesting depth exceeds maximum of " + maxNestingDepth); + } + try { + return super.visitGenericMapLiteral(ctx); + } finally { + currentNestingDepth--; + } + } + + @Override + public Object visitGenericCollectionLiteral(final GremlinParser.GenericCollectionLiteralContext ctx) { + currentNestingDepth++; + if (currentNestingDepth > maxNestingDepth) { + throw new GremlinParserException("Parameter map nesting depth exceeds maximum of " + maxNestingDepth); + } + try { + return super.visitGenericCollectionLiteral(ctx); + } finally { + currentNestingDepth--; + } + } + + @Override + public Object visitGenericSetLiteral(final GremlinParser.GenericSetLiteralContext ctx) { + currentNestingDepth++; + if (currentNestingDepth > maxNestingDepth) { + throw new GremlinParserException("Parameter map nesting depth exceeds maximum of " + maxNestingDepth); + } + try { + return super.visitGenericSetLiteral(ctx); + } finally { + currentNestingDepth--; + } + } +}
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/GremlinLang.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/GremlinLang.java index aa00fcc..08f1240 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/GremlinLang.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/GremlinLang.java
@@ -51,7 +51,6 @@ import java.util.Set; import java.util.UUID; import java.util.Base64; -import java.util.concurrent.atomic.AtomicInteger; import static org.apache.tinkerpop.gremlin.util.DatetimeHelper.format; import static org.apache.tinkerpop.gremlin.util.iterator.IteratorUtils.asIterator; @@ -66,7 +65,7 @@ private StringBuilder gremlin = new StringBuilder(); private Map<String, Object> parameters = new HashMap<>(); - private static final AtomicInteger paramCount = new AtomicInteger(0); + private String unsupportedType = ""; private List<OptionsStrategy> optionsStrategies = new ArrayList<>(); public GremlinLang() { @@ -208,6 +207,9 @@ if (arg instanceof GremlinLang || arg instanceof DefaultTraversal) { final GremlinLang gremlinLang = arg instanceof GremlinLang ? (GremlinLang) arg : ((DefaultTraversal) arg).getGremlinLang(); parameters.putAll(gremlinLang.getParameters()); + if (gremlinLang.containsUnsupportedTypes()) { + unsupportedType = gremlinLang.getUnsupportedType(); + } return gremlinLang.getGremlin("__"); } @@ -250,14 +252,8 @@ return ((Class) arg).getSimpleName(); } - return asParameter(arg); - } - - private String asParameter(final Object arg) { - final String paramName = String.format("_%d", paramCount.getAndIncrement()); - // todo: consider resetting paramCount when it's larger then 1_000_000 - parameters.put(paramName, arg); - return paramName; + unsupportedType = arg.getClass().getSimpleName(); + return arg.toString(); } private String asString(final Iterator itty) { @@ -378,6 +374,71 @@ } /** + * Returns the simple class name of the last type encountered by {@code argAsString()} that could not be + * represented as a gremlin-lang literal. An empty string means all types were supported. + * + * @return the simple class name of the unsupported type, or empty string if none + */ + public String getUnsupportedType() { + return unsupportedType; + } + + /** + * Returns {@code true} if {@code argAsString()} encountered at least one type that could not be represented + * as a gremlin-lang literal. In remote mode, the driver should check this before sending the request. + * <p> + * Note: this only covers types encountered while building the gremlin string. Named {@code GValue} parameters + * store their inner value directly in the parameter map without type-checking via {@code argAsString()}, so + * an unsupported type wrapped in a named {@code GValue} will not be detected by this method. Such cases are + * caught separately by {@link #convertParametersToString(Map)} when the parameter map is serialized. + * Unnamed {@code GValue} instances (null name) recurse into {@code argAsString()} and will set this flag. + * + * @return true if unsupported types were encountered in the gremlin string + */ + public boolean containsUnsupportedTypes() { + return !unsupportedType.isEmpty(); + } + + /** + * Serializes this instance's parameter map to a gremlin-lang map literal string. Keys are parameter names + * (identifiers), values are formatted using {@code argAsString()}. + * + * @return a gremlin-lang map literal string, e.g. {@code ["x":1,"y":"stephen"]} or {@code [:]} for empty + */ + public String getParametersAsString() { + return convertParametersToString(parameters); + } + + /** + * Converts an arbitrary parameter map to a gremlin-lang map literal string. Keys must be valid identifier strings. + * Values are formatted as gremlin-lang literals. + * + * @param params the parameter map to convert + * @return a gremlin-lang map literal string, e.g. {@code ["x":1,"y":"stephen"]} or {@code [:]} for empty + */ + public static String convertParametersToString(final Map<String, Object> params) { + if (params == null || params.isEmpty()) return "[:]"; + + final GremlinLang helper = new GremlinLang(); + final StringBuilder sb = new StringBuilder("["); + final Iterator<Map.Entry<String, Object>> itty = params.entrySet().iterator(); + while (itty.hasNext()) { + final Map.Entry<String, Object> entry = itty.next(); + sb.append(helper.argAsString(entry.getKey())).append(":").append(helper.argAsString(entry.getValue())); + if (itty.hasNext()) sb.append(","); + } + sb.append("]"); + + if (helper.containsUnsupportedTypes()) { + throw new IllegalArgumentException(String.format( + "Parameter map contains at least one type [%s] that cannot be represented as text.", + helper.getUnsupportedType())); + } + + return sb.toString(); + } + + /** * The alias to set. */ public void addG(final String g) { @@ -385,13 +446,6 @@ } /** - * Reset parameter naming counter. Mainly intended to make testing easier - */ - public void reset() { - paramCount.set(0); - } - - /** * Add a {@link TraversalSource} instruction to the GremlinLang. * * @param sourceName the traversal source method name (e.g. withSack()) @@ -510,6 +564,7 @@ clone.gremlin = new StringBuilder(gremlin.length()); clone.gremlin.append(gremlin); clone.optionsStrategies = new ArrayList<>(this.optionsStrategies); + clone.unsupportedType = this.unsupportedType; return clone; } catch (final CloneNotSupportedException e) { throw new IllegalStateException(e.getMessage(), e);
diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/language/grammar/ParameterMapVisitorTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/language/grammar/ParameterMapVisitorTest.java new file mode 100644 index 0000000..68c8d9b --- /dev/null +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/language/grammar/ParameterMapVisitorTest.java
@@ -0,0 +1,220 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.tinkerpop.gremlin.language.grammar; + +import org.junit.Test; + +import java.util.List; +import java.util.Map; +import java.util.UUID; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +/** + * Tests for parameter map parsing, validation, and security. + */ +public class ParameterMapVisitorTest { + + @Test + public void shouldParseEmptyMap() { + final Map<String, Object> result = GremlinQueryParser.parseParameters("[:]"); + assertNotNull(result); + assertTrue(result.isEmpty()); + } + + @Test + public void shouldParseNullInput() { + final Map<String, Object> result = GremlinQueryParser.parseParameters(null); + assertNotNull(result); + assertTrue(result.isEmpty()); + } + + @Test + public void shouldParseEmptyStringInput() { + final Map<String, Object> result = GremlinQueryParser.parseParameters(""); + assertNotNull(result); + assertTrue(result.isEmpty()); + } + + @Test + public void shouldParseSingleIntegerParameter() { + final Map<String, Object> result = GremlinQueryParser.parseParameters("[\"x\":1]"); + assertEquals(1, result.size()); + assertEquals(1, result.get("x")); + } + + @Test + public void shouldParseSingleStringParameter() { + final Map<String, Object> result = GremlinQueryParser.parseParameters("[\"name\":\"marko\"]"); + assertEquals(1, result.size()); + assertEquals("marko", result.get("name")); + } + + @Test + public void shouldParseSingleLongParameter() { + final Map<String, Object> result = GremlinQueryParser.parseParameters("[\"x\":1L]"); + assertEquals(1, result.size()); + assertEquals(1L, result.get("x")); + } + + @Test + public void shouldParseMultipleMixedParameters() { + final Map<String, Object> result = GremlinQueryParser.parseParameters("[\"x\":1,\"name\":\"marko\",\"flag\":true]"); + assertEquals(3, result.size()); + assertEquals(1, result.get("x")); + assertEquals("marko", result.get("name")); + assertEquals(true, result.get("flag")); + } + + @Test + public void shouldParseNullValue() { + final Map<String, Object> result = GremlinQueryParser.parseParameters("[\"x\":null]"); + assertEquals(1, result.size()); + assertNull(result.get("x")); + } + + @Test + public void shouldParseUuidValue() { + final Map<String, Object> result = GremlinQueryParser.parseParameters("[\"id\":UUID(\"bfa9bbe8-c3a3-4017-acc3-cd02dda55e3e\")]"); + assertEquals(1, result.size()); + assertEquals(UUID.fromString("bfa9bbe8-c3a3-4017-acc3-cd02dda55e3e"), result.get("id")); + } + + @Test + public void shouldParseNestedMapValue() { + final Map<String, Object> result = GremlinQueryParser.parseParameters("[\"m\":[\"name\":\"marko\"]]"); + assertEquals(1, result.size()); + assertTrue(result.get("m") instanceof Map); + assertEquals("marko", ((Map<?, ?>) result.get("m")).get("name")); + } + + @Test + public void shouldParseListValue() { + final Map<String, Object> result = GremlinQueryParser.parseParameters("[\"x\":[1,2,3]]"); + assertEquals(1, result.size()); + assertTrue(result.get("x") instanceof List); + assertEquals(3, ((List<?>) result.get("x")).size()); + } + + @Test + public void shouldParseUnicodeKey() { + final Map<String, Object> result = GremlinQueryParser.parseParameters("[\"caf\\u00E9\":1]"); + assertEquals(1, result.size()); + assertEquals(1, result.get("caf\u00e9")); + } + + @Test(expected = GremlinParserException.class) + public void shouldRejectMalformedInput() { + GremlinQueryParser.parseParameters("[\"x\":"); + } + + @Test(expected = GremlinParserException.class) + public void shouldRejectNumericKey() { + GremlinQueryParser.parseParameters("[1:\"value\"]"); + } + + @Test(expected = GremlinParserException.class) + public void shouldRejectEnumKey() { + GremlinQueryParser.parseParameters("[(T.id):\"value\"]"); + } + + @Test(expected = GremlinParserException.class) + public void shouldRejectNonIdentifierStringKey() { + GremlinQueryParser.parseParameters("[\"~id\":1]"); + } + + @Test(expected = GremlinParserException.class) + public void shouldRejectKeyWithSpaces() { + GremlinQueryParser.parseParameters("[\"my key\":1]"); + } + + @Test(expected = GremlinParserException.class) + public void shouldRejectEmptyStringKey() { + GremlinQueryParser.parseParameters("[\"\":1]"); + } + + @Test(expected = GremlinParserException.class) + public void shouldRejectExcessiveNestingDepth() { + final StringBuilder sb = new StringBuilder(); + for (int i = 0; i < 35; i++) { + sb.append("[\"a\":"); + } + sb.append("1"); + for (int i = 0; i < 35; i++) { + sb.append("]"); + } + GremlinQueryParser.parseParameters(sb.toString()); + } + + @Test(expected = GremlinParserException.class) + public void shouldRejectNestedTraversalInValue() { + GremlinQueryParser.parseParameters("[\"x\":__.out()]"); + } + + @Test(expected = GremlinParserException.class) + public void shouldRejectTerminatedTraversalInValue() { + GremlinQueryParser.parseParameters("[\"x\":g.V().drop().iterate()]"); + } + + @Test(expected = GremlinParserException.class) + public void shouldRejectTraversalInNestedList() { + GremlinQueryParser.parseParameters("[\"x\":[__.out()]]"); + } + + @Test(expected = GremlinParserException.class) + public void shouldRejectTraversalInNestedSet() { + GremlinQueryParser.parseParameters("[\"x\":{__.out()}]"); + } + + @Test(expected = GremlinParserException.class) + public void shouldRejectTraversalInNestedMapValue() { + GremlinQueryParser.parseParameters("[\"x\":[\"a\":__.out()]]"); + } + + @Test + public void shouldAcceptStringContainingGremlinLikeContent() { + final Map<String, Object> result = GremlinQueryParser.parseParameters("[\"x\":\"g.V().drop()\"]"); + assertEquals(1, result.size()); + assertEquals("g.V().drop()", result.get("x")); + } + + @Test + public void shouldAcceptStringContainingTraversalSyntax() { + final Map<String, Object> result = GremlinQueryParser.parseParameters("[\"x\":\"__.out().count()\"]"); + assertEquals(1, result.size()); + assertEquals("__.out().count()", result.get("x")); + } + + @Test + public void shouldParseUnquotedIdentifierKey() { + final Map<String, Object> result = GremlinQueryParser.parseParameters("[x:1]"); + assertEquals(1, result.size()); + assertEquals(1, result.get("x")); + } + + @Test + public void shouldParseKeywordAsMapKey() { + final Map<String, Object> result = GremlinQueryParser.parseParameters("[label:\"person\"]"); + assertEquals(1, result.size()); + assertEquals("person", result.get("label")); + } +}
diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/GremlinLangTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/GremlinLangTest.java index b7f9da3..57649b6 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/GremlinLangTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/GremlinLangTest.java
@@ -38,11 +38,14 @@ import java.math.BigDecimal; import java.math.BigInteger; import java.nio.ByteBuffer; +import java.time.Duration; import java.util.Arrays; import java.util.Collections; import java.util.Date; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.Map; import java.util.UUID; import static org.apache.tinkerpop.gremlin.process.traversal.AnonymousTraversalSource.traversal; @@ -52,6 +55,8 @@ import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.out; import static org.apache.tinkerpop.gremlin.util.CollectionUtil.asMap; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; @RunWith(Parameterized.class) public class GremlinLangTest { @@ -66,14 +71,14 @@ @Test public void doTest() { - final String gremlin = traversal.asAdmin().getGremlinLang().getGremlin(); - assertEquals(expected, gremlin); + final GremlinLang gremlinLang = traversal.asAdmin().getGremlinLang(); + assertEquals(expected, gremlinLang.getGremlin()); + assertFalse(gremlinLang.containsUnsupportedTypes()); + assertEquals("", gremlinLang.getUnsupportedType()); } private static GraphTraversalSource newG() { - final GraphTraversalSource g = traversal().with(EmptyGraph.instance()); - g.getGremlinLang().reset(); - return g; + return traversal().with(EmptyGraph.instance()); } @Parameterized.Parameters(name = "{0}") @@ -98,8 +103,6 @@ {g.V().has(T.label, "person"), "g.V().has(T.label,\"person\")"}, {g.addE("knows").from(new DetachedVertex(1, "test1", Collections.emptyList())).to(new DetachedVertex(6, "test2", Collections.emptyList())), "g.addE(\"knows\").from(__.V(1)).to(__.V(6))"}, - {newG().E(new ReferenceEdge(1, "test label", new ReferenceVertex(1, "v1"), new ReferenceVertex(1, "v1"))), - "g.E(_0)"}, {g.V().hasId(P.within(Collections.emptyList())).count(), "g.V().hasId(P.within([])).count()"}, {g.V(1).outE().has("weight", P.inside(0.0, 0.6)), "g.V(1).outE().has(\"weight\",P.gt(0.0D).and(P.lt(0.6D)))"}, {g.withSack(1.0, Operator.sum).V(1).local(__.out("knows").barrier(SackFunctions.Barrier.normSack)).in("knows").barrier().sack(), @@ -193,4 +196,404 @@ assertEquals("g.inject(Binary(\"AQID\"))", gremlin); } } + + public static class ParameterStringTests { + + @Test + public void shouldSerializeEmptyParameterMap() { + assertEquals("[:]", GremlinLang.convertParametersToString(new HashMap<>())); + assertEquals("[:]", GremlinLang.convertParametersToString(null)); + } + + @Test + public void shouldSerializeSingleStringParameter() { + final Map<String, Object> params = new HashMap<>(); + params.put("name", "marko"); + assertEquals("[\"name\":\"marko\"]", GremlinLang.convertParametersToString(params)); + } + + @Test + public void shouldSerializeSingleIntegerParameter() { + final Map<String, Object> params = new HashMap<>(); + params.put("x", 1); + assertEquals("[\"x\":1]", GremlinLang.convertParametersToString(params)); + } + + @Test + public void shouldSerializeLongParameter() { + final Map<String, Object> params = new HashMap<>(); + params.put("x", 1L); + assertEquals("[\"x\":1L]", GremlinLang.convertParametersToString(params)); + } + + @Test + public void shouldSerializeDoubleParameter() { + final Map<String, Object> params = new HashMap<>(); + params.put("x", 1.5D); + assertEquals("[\"x\":1.5D]", GremlinLang.convertParametersToString(params)); + } + + @Test + public void shouldSerializeBooleanParameter() { + final Map<String, Object> params = new HashMap<>(); + params.put("flag", true); + assertEquals("[\"flag\":true]", GremlinLang.convertParametersToString(params)); + } + + @Test + public void shouldSerializeNullParameter() { + final Map<String, Object> params = new HashMap<>(); + params.put("x", null); + assertEquals("[\"x\":null]", GremlinLang.convertParametersToString(params)); + } + + @Test + public void shouldSerializeUuidParameter() { + final Map<String, Object> params = new HashMap<>(); + final UUID uuid = UUID.fromString("bfa9bbe8-c3a3-4017-acc3-cd02dda55e3e"); + params.put("id", uuid); + assertEquals("[\"id\":UUID(\"bfa9bbe8-c3a3-4017-acc3-cd02dda55e3e\")]", GremlinLang.convertParametersToString(params)); + } + + @Test + public void shouldSerializeStringWithSpecialCharacters() { + final Map<String, Object> params = new HashMap<>(); + params.put("x", "hello \"world\""); + assertEquals("[\"x\":\"hello \\\"world\\\"\"]", GremlinLang.convertParametersToString(params)); + } + + @Test + public void shouldSerializeByteParameter() { + final Map<String, Object> params = new HashMap<>(); + params.put("x", (byte) 1); + assertEquals("[\"x\":1B]", GremlinLang.convertParametersToString(params)); + } + + @Test + public void shouldSerializeShortParameter() { + final Map<String, Object> params = new HashMap<>(); + params.put("x", (short) 1); + assertEquals("[\"x\":1S]", GremlinLang.convertParametersToString(params)); + } + + @Test + public void shouldSerializeBigIntegerParameter() { + final Map<String, Object> params = new HashMap<>(); + params.put("x", BigInteger.valueOf(123)); + assertEquals("[\"x\":123N]", GremlinLang.convertParametersToString(params)); + } + + @Test + public void shouldSerializeBigDecimalParameter() { + final Map<String, Object> params = new HashMap<>(); + params.put("x", BigDecimal.valueOf(1.5)); + assertEquals("[\"x\":1.5M]", GremlinLang.convertParametersToString(params)); + } + + @Test + public void shouldSerializeFloatParameter() { + final Map<String, Object> params = new HashMap<>(); + params.put("x", 1.5F); + assertEquals("[\"x\":1.5F]", GremlinLang.convertParametersToString(params)); + } + + @Test + public void shouldSerializeCharacterParameter() { + final Map<String, Object> params = new HashMap<>(); + params.put("x", 'a'); + assertEquals("[\"x\":\"a\"c]", GremlinLang.convertParametersToString(params)); + } + + @Test + public void shouldSerializeDurationParameter() { + final Map<String, Object> params = new HashMap<>(); + params.put("x", Duration.ofHours(2).plusMinutes(30)); + assertEquals("[\"x\":Duration(9000,0)]", GremlinLang.convertParametersToString(params)); + } + + @Test + public void shouldSerializeBinaryParameter() { + final Map<String, Object> params = new HashMap<>(); + params.put("x", ByteBuffer.wrap(new byte[]{1, 2, 3})); + assertEquals("[\"x\":Binary(\"AQID\")]", GremlinLang.convertParametersToString(params)); + } + + @Test + public void shouldSerializeEnumParameter() { + final Map<String, Object> params = new HashMap<>(); + params.put("x", T.id); + assertEquals("[\"x\":T.id]", GremlinLang.convertParametersToString(params)); + } + + @Test + public void shouldSerializeGetParametersAsString() { + final GraphTraversalSource g2 = newG(); + final String result = g2.V(GValue.of("x", 1)). + has("name", GValue.of("name", "marko")). + asAdmin(). + getGremlinLang(). + getParametersAsString(); + // parameters are in a HashMap so order may vary, just check structure + assertTrue(result.startsWith("[")); + assertTrue(result.endsWith("]")); + assertTrue(result, result.contains("\"x\":1")); + assertTrue(result, result.contains("\"name\":\"marko\"")); + } + + @Test + public void shouldSerializeOffsetDateTimeParameter() { + final Map<String, Object> params = new HashMap<>(); + params.put("x", DatetimeHelper.parse("2018-03-21T08:35:44.741Z")); + assertEquals("[\"x\":datetime(\"2018-03-21T08:35:44.741Z\")]", GremlinLang.convertParametersToString(params)); + } + + @Test + public void shouldSerializeDateParameter() { + final Map<String, Object> params = new HashMap<>(); + params.put("x", Date.from(DatetimeHelper.parse("2018-03-21T08:35:44.741Z").toInstant())); + assertEquals("[\"x\":datetime(\"2018-03-21T08:35:44.741Z\")]", GremlinLang.convertParametersToString(params)); + } + + @Test + public void shouldSerializeMultipleParameters() { + // use a LinkedHashMap to guarantee order + final Map<String, Object> params = new LinkedHashMap<>(); + params.put("x", 1); + params.put("name", "marko"); + params.put("flag", true); + assertEquals("[\"x\":1,\"name\":\"marko\",\"flag\":true]", GremlinLang.convertParametersToString(params)); + } + + @Test + public void shouldSerializeMapValuedParameter() { + final Map<String, Object> params = new LinkedHashMap<>(); + final Map<Object, Object> nested = new LinkedHashMap<>(); + nested.put(T.label, "person"); + nested.put("name", "marko"); + params.put("m", nested); + assertEquals("[\"m\":[(T.label):\"person\",\"name\":\"marko\"]]", GremlinLang.convertParametersToString(params)); + } + + @Test + public void shouldSerializeListValuedParameter() { + final Map<String, Object> params = new HashMap<>(); + params.put("x", Arrays.asList(1, 2, 3)); + assertEquals("[\"x\":[1,2,3]]", GremlinLang.convertParametersToString(params)); + } + + @Test + public void shouldSerializeSetValuedParameter() { + // use a LinkedHashSet to guarantee order + final java.util.Set<Integer> set = new java.util.LinkedHashSet<>(Arrays.asList(1, 2)); + final Map<String, Object> params = new HashMap<>(); + params.put("x", set); + assertEquals("[\"x\":{1,2}]", GremlinLang.convertParametersToString(params)); + } + + @Test + public void shouldSerializeStringWithBackslash() { + final Map<String, Object> params = new HashMap<>(); + params.put("x", "path\\to\\file"); + assertEquals("[\"x\":\"path\\\\to\\\\file\"]", GremlinLang.convertParametersToString(params)); + } + + @Test + public void shouldSerializeStringWithUnicode() { + final Map<String, Object> params = new HashMap<>(); + params.put("x", "caf\u00E9"); + assertEquals("[\"x\":\"caf\\u00E9\"]", GremlinLang.convertParametersToString(params)); + } + + @Test + public void shouldHandleEmptyStringKey() { + final Map<String, Object> params = new HashMap<>(); + params.put("", 1); + final String result = GremlinLang.convertParametersToString(params); + assertEquals("[\"\":1]", result); + } + + @Test + public void shouldHandleKeyWithSpaces() { + final Map<String, Object> params = new HashMap<>(); + params.put("my key", 1); + final String result = GremlinLang.convertParametersToString(params); + assertEquals("[\"my key\":1]", result); + } + + @Test + public void shouldHandleNullKey() { + final Map<String, Object> params = new HashMap<>(); + params.put(null, 1); + final String result = GremlinLang.convertParametersToString(params); + assertEquals("[null:1]", result); + } + + @Test + public void shouldSerializeUnicodeKey() { + final Map<String, Object> params = new HashMap<>(); + params.put("caf\u00e9", 1); + assertEquals("[\"caf\\u00E9\":1]", GremlinLang.convertParametersToString(params)); + } + } + + public static class UnsupportedTypeTests { + + /** + * A test-only type that will never have gremlin-lang literal support. + */ + private static class UnsupportedType { + @Override + public String toString() { + return "custom-unsupported"; + } + } + + @Test + public void shouldMarkUnsupportedTypeAndUseToString() { + final UnsupportedType custom = new UnsupportedType(); + final GremlinLang gremlinLang = g.inject(custom).asAdmin().getGremlinLang(); + assertTrue(gremlinLang.containsUnsupportedTypes()); + assertEquals("UnsupportedType", gremlinLang.getUnsupportedType()); + assertTrue(gremlinLang.getGremlin().contains(custom.toString())); + } + + @Test + public void shouldNotAddUnsupportedTypeToParameterMap() { + final GremlinLang gremlinLang = g.inject(new UnsupportedType()).asAdmin().getGremlinLang(); + // unsupported type is flagged but not stored in the parameter map + assertTrue(gremlinLang.containsUnsupportedTypes()); + assertTrue(gremlinLang.getParameters().isEmpty()); + } + + @Test + public void shouldRecordLastUnsupportedType() { + final GremlinLang gremlinLang = g.inject(new UnsupportedType(), new Thread()).asAdmin().getGremlinLang(); + assertTrue(gremlinLang.containsUnsupportedTypes()); + // last one wins + assertEquals("Thread", gremlinLang.getUnsupportedType()); + } + + @Test + public void shouldPropagateFlagFromChildTraversal() { + final GremlinLang gremlinLang = g.V().where(__.has("x", P.eq(new UnsupportedType()))).asAdmin().getGremlinLang(); + assertTrue(gremlinLang.containsUnsupportedTypes()); + assertEquals("UnsupportedType", gremlinLang.getUnsupportedType()); + } + + @Test + public void shouldNotPropagateFlagFromCleanChildTraversal() { + final GremlinLang gremlinLang = g.V().where(__.has("name", "marko")).asAdmin().getGremlinLang(); + assertFalse(gremlinLang.containsUnsupportedTypes()); + assertEquals("", gremlinLang.getUnsupportedType()); + } + + @Test + public void shouldDetectUnsupportedTypeInList() { + final GremlinLang gremlinLang = g.inject(Arrays.asList(1, new UnsupportedType())).asAdmin().getGremlinLang(); + assertTrue(gremlinLang.containsUnsupportedTypes()); + assertEquals("UnsupportedType", gremlinLang.getUnsupportedType()); + } + + @Test + public void shouldDetectUnsupportedTypeInMap() { + final Map<String, Object> map = new HashMap<>(); + map.put("key", new UnsupportedType()); + final GremlinLang gremlinLang = g.inject(map).asAdmin().getGremlinLang(); + assertTrue(gremlinLang.containsUnsupportedTypes()); + assertEquals("UnsupportedType", gremlinLang.getUnsupportedType()); + } + + @Test + public void shouldDetectUnsupportedTypeInSet() { + final java.util.Set<Object> set = new java.util.LinkedHashSet<>(); + set.add(1); + set.add(new UnsupportedType()); + final GremlinLang gremlinLang = g.inject(set).asAdmin().getGremlinLang(); + assertTrue(gremlinLang.containsUnsupportedTypes()); + assertEquals("UnsupportedType", gremlinLang.getUnsupportedType()); + } + + @Test + public void shouldDetectUnsupportedTypeInPredicate() { + final GremlinLang gremlinLang = g.V().has("x", P.eq(new UnsupportedType())).asAdmin().getGremlinLang(); + assertTrue(gremlinLang.containsUnsupportedTypes()); + assertEquals("UnsupportedType", gremlinLang.getUnsupportedType()); + } + + @Test + public void shouldPreserveUnsupportedTypeOnClone() { + final GremlinLang original = g.inject(new UnsupportedType()).asAdmin().getGremlinLang(); + final GremlinLang cloned = original.clone(); + assertTrue(cloned.containsUnsupportedTypes()); + assertEquals("UnsupportedType", cloned.getUnsupportedType()); + } + + @Test + public void shouldPreserveCleanStateOnClone() { + final GremlinLang original = g.V(1).asAdmin().getGremlinLang(); + final GremlinLang cloned = original.clone(); + assertFalse(cloned.containsUnsupportedTypes()); + assertEquals("", cloned.getUnsupportedType()); + } + + @Test + public void shouldStillAddGValueToParameterMap() { + final GremlinLang gremlinLang = g.V(GValue.of("myId", 1)).asAdmin().getGremlinLang(); + assertFalse(gremlinLang.containsUnsupportedTypes()); + assertEquals(1, gremlinLang.getParameters().size()); + assertEquals(1, gremlinLang.getParameters().get("myId")); + } + + @Test + public void shouldDetectUnsupportedTypeForReferenceEdge() { + final GremlinLang gremlinLang = newG().E( + new ReferenceEdge(1, "test label", new ReferenceVertex(1, "v1"), new ReferenceVertex(1, "v1"))) + .asAdmin().getGremlinLang(); + assertTrue(gremlinLang.containsUnsupportedTypes()); + assertEquals("ReferenceEdge", gremlinLang.getUnsupportedType()); + assertEquals("g.E(e[1][1-test label->1])", gremlinLang.getGremlin()); + } + + @Test + public void shouldDetectUnsupportedTypeInMapKey() { + final Map<Object, Object> map = new LinkedHashMap<>(); + map.put(new UnsupportedType(), "value"); + final GremlinLang gremlinLang = g.inject(map).asAdmin().getGremlinLang(); + assertTrue(gremlinLang.containsUnsupportedTypes()); + assertEquals("UnsupportedType", gremlinLang.getUnsupportedType()); + } + + @Test(expected = IllegalArgumentException.class) + public void shouldRejectUnsupportedTypeInConvertParametersToString() { + final Map<String, Object> params = new HashMap<>(); + params.put("x", new UnsupportedType()); + GremlinLang.convertParametersToString(params); + } + + @Test(expected = IllegalArgumentException.class) + public void shouldRejectUnsupportedTypeInGValueViaGetParametersAsString() { + final GremlinLang gremlinLang = g.V(GValue.of("x", new UnsupportedType())).asAdmin().getGremlinLang(); + // named GValue stores the raw value in the parameter map without type-checking, + // so containsUnsupportedTypes() does not detect it + assertFalse(gremlinLang.containsUnsupportedTypes()); + // the error is caught when the parameter map is serialized + gremlinLang.getParametersAsString(); + } + + @Test + public void shouldDetectUnsupportedTypeInUnnamedGValue() { + // unnamed GValue (null name) recurses into argAsString(gValue.get()), + // which hits the unsupported-type fallback and sets the flag + final GremlinLang gremlinLang = g.inject(GValue.of(null, new UnsupportedType())).asAdmin().getGremlinLang(); + assertTrue(gremlinLang.containsUnsupportedTypes()); + assertEquals("UnsupportedType", gremlinLang.getUnsupportedType()); + } + + @Test + public void shouldPersistFlagAfterSupportedTypeFollows() { + final GremlinLang gremlinLang = g.inject(new UnsupportedType(), "hello").asAdmin().getGremlinLang(); + assertTrue(gremlinLang.containsUnsupportedTypes()); + assertEquals("UnsupportedType", gremlinLang.getUnsupportedType()); + } + } }
diff --git a/gremlin-dotnet/src/Gremlin.Net/Driver/Messages/RequestMessage.cs b/gremlin-dotnet/src/Gremlin.Net/Driver/Messages/RequestMessage.cs index f599a53..2ebd02c 100644 --- a/gremlin-dotnet/src/Gremlin.Net/Driver/Messages/RequestMessage.cs +++ b/gremlin-dotnet/src/Gremlin.Net/Driver/Messages/RequestMessage.cs
@@ -23,6 +23,7 @@ using System; using System.Collections.Generic; +using Gremlin.Net.Process.Traversal; namespace Gremlin.Net.Driver.Messages { @@ -69,6 +70,7 @@ private readonly string _gremlin; private readonly Dictionary<string, object> _fields = new Dictionary<string, object>(); private readonly Dictionary<string, object> _bindings = new Dictionary<string, object>(); + private string? _bindingsString; internal Builder(string gremlin) { @@ -94,17 +96,23 @@ /// <returns>The <see cref="Builder" />.</returns> public Builder AddBinding(string key, object val) { + if (_bindingsString != null) + throw new InvalidOperationException("Cannot mix AddBinding() with AddBindingsString()."); _bindings[key] = val; return this; } /// <summary> - /// Adds multiple binding parameters. + /// Adds multiple binding parameters from a dictionary. The values will be + /// converted to a gremlin-lang string when the message is created. + /// Cannot be mixed with <see cref="AddBindingsString"/>. /// </summary> /// <param name="bindings">The bindings to add.</param> /// <returns>The <see cref="Builder" />.</returns> public Builder AddBindings(Dictionary<string, object> bindings) { + if (_bindingsString != null) + throw new InvalidOperationException("Cannot mix AddBindings() with AddBindingsString()."); foreach (var kvp in bindings) { _bindings[kvp.Key] = kvp.Value; @@ -113,6 +121,20 @@ } /// <summary> + /// Sets the bindings as a pre-serialized gremlin-lang map literal string. + /// Cannot be mixed with <see cref="AddBinding"/> or <see cref="AddBindings"/>. + /// </summary> + /// <param name="bindingsString">The gremlin-lang bindings string.</param> + /// <returns>The <see cref="Builder" />.</returns> + public Builder AddBindingsString(string bindingsString) + { + if (_bindings.Count > 0) + throw new InvalidOperationException("Cannot mix AddBindingsString() with AddBinding()/AddBindings()."); + _bindingsString = bindingsString; + return this; + } + + /// <summary> /// Adds a field to the request message. /// </summary> /// <param name="key">The field key.</param> @@ -203,7 +225,15 @@ /// <returns>The built <see cref="RequestMessage" />.</returns> public RequestMessage Create() { - _fields[Tokens.ArgsBindings] = _bindings; + // prefer pre-serialized bindings string over raw map + if (_bindingsString != null) + { + _fields[Tokens.ArgsBindings] = _bindingsString; + } + else if (_bindings.Count > 0) + { + _fields[Tokens.ArgsBindings] = GremlinLang.ConvertParametersToString(_bindings); + } return new RequestMessage(_gremlin, _fields); } }
diff --git a/gremlin-dotnet/src/Gremlin.Net/Driver/Remote/DriverRemoteConnection.cs b/gremlin-dotnet/src/Gremlin.Net/Driver/Remote/DriverRemoteConnection.cs index 81dd343..3340730 100644 --- a/gremlin-dotnet/src/Gremlin.Net/Driver/Remote/DriverRemoteConnection.cs +++ b/gremlin-dotnet/src/Gremlin.Net/Driver/Remote/DriverRemoteConnection.cs
@@ -112,8 +112,13 @@ gremlinLang.AddG(_traversalSource); var requestMsg = RequestMessage.Build(gremlinLang.GetGremlin()) - .AddG(_traversalSource) - .AddBindings(gremlinLang.Parameters); + .AddG(_traversalSource); + + var parametersString = gremlinLang.GetParametersAsString(); + if (parametersString != "[:]") + { + requestMsg.AddBindingsString(parametersString); + } foreach (var optionsStrategy in gremlinLang.OptionsStrategies) {
diff --git a/gremlin-dotnet/src/Gremlin.Net/Process/Traversal/GremlinLang.cs b/gremlin-dotnet/src/Gremlin.Net/Process/Traversal/GremlinLang.cs index 2dfdc02..21115a8 100644 --- a/gremlin-dotnet/src/Gremlin.Net/Process/Traversal/GremlinLang.cs +++ b/gremlin-dotnet/src/Gremlin.Net/Process/Traversal/GremlinLang.cs
@@ -27,7 +27,6 @@ using System.Globalization; using System.Numerics; using System.Text; -using System.Threading; using Gremlin.Net.Process.Traversal.Strategy; using Gremlin.Net.Process.Traversal.Strategy.Decoration; using Gremlin.Net.Structure; @@ -44,7 +43,6 @@ private StringBuilder _gremlin = new(); private Dictionary<string, object> _parameters = new(); - private static int _paramCount; private List<OptionsStrategy> _optionsStrategies = new(); /// <summary> @@ -124,6 +122,32 @@ public Dictionary<string, object> Parameters => _parameters; /// <summary> + /// Serializes this instance's parameter map to a gremlin-lang map literal string. + /// </summary> + public string GetParametersAsString() => ConvertParametersToString(_parameters); + + /// <summary> + /// Converts a parameter map to a gremlin-lang map literal string. + /// Throws ArgumentException if any value is an unsupported type. + /// </summary> + public static string ConvertParametersToString(Dictionary<string, object>? parameters) + { + if (parameters == null || parameters.Count == 0) return "[:]"; + + var helper = new GremlinLang(); + var sb = new StringBuilder("["); + var first = true; + foreach (var kvp in parameters) + { + if (!first) sb.Append(','); + sb.Append(helper.ArgAsString(kvp.Key)).Append(':').Append(helper.ArgAsString(kvp.Value)); + first = false; + } + sb.Append(']'); + return sb.ToString(); + } + + /// <summary> /// Gets the list of extracted OptionsStrategy instances. /// </summary> public List<OptionsStrategy> OptionsStrategies => _optionsStrategies; @@ -148,14 +172,6 @@ } } - /// <summary> - /// Resets the static parameter counter. Intended for test determinism. - /// </summary> - public static void ResetCounter() - { - Interlocked.Exchange(ref _paramCount, 0); - } - private void AddToGremlin(string name, object?[] arguments) { var flattenedArguments = FlattenArguments(arguments); @@ -332,14 +348,8 @@ if (arg is Type type) return type.Name; - return AsParameter(arg); - } - - private string AsParameter(object arg) - { - var paramName = $"_{Interlocked.Increment(ref _paramCount) - 1}"; - _parameters[paramName] = arg; - return paramName; + throw new ArgumentException( + $"GremlinLang contains at least one type [{arg.GetType().Name}] that cannot be represented as text."); } private string AsString(P p)
diff --git a/gremlin-dotnet/test/Gremlin.Net.UnitTest/Driver/DriverRemoteConnectionTests.cs b/gremlin-dotnet/test/Gremlin.Net.UnitTest/Driver/DriverRemoteConnectionTests.cs index 7a67ce6..2e83c11 100644 --- a/gremlin-dotnet/test/Gremlin.Net.UnitTest/Driver/DriverRemoteConnectionTests.cs +++ b/gremlin-dotnet/test/Gremlin.Net.UnitTest/Driver/DriverRemoteConnectionTests.cs
@@ -110,14 +110,14 @@ var connection = new DriverRemoteConnection(client, "g"); var gl = new GremlinLang(); - gl.Parameters["_0"] = 42; + gl.Parameters["x"] = 42; gl.AddStep("V", Array.Empty<object>()); await connection.SubmitAsync<object, object>(gl); Assert.NotNull(capturedRequest); - var bindings = (Dictionary<string, object>)capturedRequest!.Fields[Tokens.ArgsBindings]; - Assert.Equal(42, bindings["_0"]); + var bindingsString = (string)capturedRequest!.Fields[Tokens.ArgsBindings]; + Assert.Contains("\"x\":42", bindingsString); } [Fact]
diff --git a/gremlin-dotnet/test/Gremlin.Net.UnitTest/Driver/Messages/RequestMessageTests.cs b/gremlin-dotnet/test/Gremlin.Net.UnitTest/Driver/Messages/RequestMessageTests.cs index a1c734d..4d66984 100644 --- a/gremlin-dotnet/test/Gremlin.Net.UnitTest/Driver/Messages/RequestMessageTests.cs +++ b/gremlin-dotnet/test/Gremlin.Net.UnitTest/Driver/Messages/RequestMessageTests.cs
@@ -57,25 +57,25 @@ [Fact] public void ShouldSetBindings() { - var msg = RequestMessage.Build("g.V(_0)") - .AddBinding("_0", 1) + var msg = RequestMessage.Build("g.V(x)") + .AddBinding("x", 1) .Create(); - var bindings = (Dictionary<string, object>)msg.Fields[Tokens.ArgsBindings]; - Assert.Equal(1, bindings["_0"]); + var bindingsString = (string)msg.Fields[Tokens.ArgsBindings]; + Assert.Contains("\"x\":1", bindingsString); } [Fact] public void ShouldSetMultipleBindings() { - var bindings = new Dictionary<string, object> { { "_0", 1 }, { "_1", "test" } }; - var msg = RequestMessage.Build("g.V(_0).has(_1)") + var bindings = new Dictionary<string, object> { { "x", 1 }, { "name", "test" } }; + var msg = RequestMessage.Build("g.V(x).has(name)") .AddBindings(bindings) .Create(); - var resultBindings = (Dictionary<string, object>)msg.Fields[Tokens.ArgsBindings]; - Assert.Equal(1, resultBindings["_0"]); - Assert.Equal("test", resultBindings["_1"]); + var bindingsString = (string)msg.Fields[Tokens.ArgsBindings]; + Assert.Contains("\"x\":1", bindingsString); + Assert.Contains("\"name\":\"test\"", bindingsString); } [Fact] @@ -99,12 +99,21 @@ } [Fact] - public void ShouldIncludeEmptyBindingsWhenNoneAdded() + public void ShouldNotContainBindingsWhenNoneAdded() { var msg = RequestMessage.Build("g.V()").Create(); - var bindings = (Dictionary<string, object>)msg.Fields[Tokens.ArgsBindings]; - Assert.Empty(bindings); + Assert.False(msg.Fields.ContainsKey(Tokens.ArgsBindings)); + } + + [Fact] + public void ShouldSetBindingsString() + { + var msg = RequestMessage.Build("g.V(x)") + .AddBindingsString("[\"x\":1]") + .Create(); + + Assert.Equal("[\"x\":1]", msg.Fields[Tokens.ArgsBindings]); } } }
diff --git a/gremlin-dotnet/test/Gremlin.Net.UnitTest/Process/Traversal/GraphTraversalSourceTests.cs b/gremlin-dotnet/test/Gremlin.Net.UnitTest/Process/Traversal/GraphTraversalSourceTests.cs index 91370c6..51bb345 100644 --- a/gremlin-dotnet/test/Gremlin.Net.UnitTest/Process/Traversal/GraphTraversalSourceTests.cs +++ b/gremlin-dotnet/test/Gremlin.Net.UnitTest/Process/Traversal/GraphTraversalSourceTests.cs
@@ -55,7 +55,6 @@ [Fact] public void CloneShouldCreateIndependentGraphTraversal() { - GremlinLang.ResetCounter(); var g = AnonymousTraversalSource.Traversal().With(null); var original = g.V().Out("created"); var clone = original.Clone().Out("knows");
diff --git a/gremlin-dotnet/test/Gremlin.Net.UnitTest/Process/Traversal/GremlinLangTests.cs b/gremlin-dotnet/test/Gremlin.Net.UnitTest/Process/Traversal/GremlinLangTests.cs index f54f4e3..b4eeade 100644 --- a/gremlin-dotnet/test/Gremlin.Net.UnitTest/Process/Traversal/GremlinLangTests.cs +++ b/gremlin-dotnet/test/Gremlin.Net.UnitTest/Process/Traversal/GremlinLangTests.cs
@@ -38,7 +38,6 @@ public GremlinLangTests() { - GremlinLang.ResetCounter(); _g = new GraphTraversalSource(); } @@ -817,27 +816,8 @@ // --- Python-Only Tests 106-116 --- [Fact] - public void g_E_Edge_renders_as_parameter() - { - GremlinLang.ResetCounter(); - var edge = new Edge(2, new Vertex(1), "knows", new Vertex(3)); - var result = _g.E(edge).GremlinLang.GetGremlin(); - Assert.Equal("g.E(_0)", result); - } - - [Fact] - public void g_Inject_VertexProperty_renders_as_parameter() - { - GremlinLang.ResetCounter(); - var vp = new VertexProperty(3, "time", "18:00", null); - var result = _g.Inject((object)vp).GremlinLang.GetGremlin(); - Assert.Equal("g.inject(_0)", result); - } - - [Fact] public void g_Inject_dicts_with_enum_keys() { - GremlinLang.ResetCounter(); var dict1 = new Dictionary<object, object> { { "name", "java" } }; var dict2 = new Dictionary<object, object> { { T.Id, 0 } }; var dict3 = new Dictionary<object, object>(); @@ -1057,5 +1037,56 @@ Assert.Equal("g.inject(\"'\"c)", _g.Inject((object)'\'').GremlinLang.GetGremlin()); } + + [Fact] + public void Unsupported_type_throws_ArgumentException() + { + Assert.Throws<ArgumentException>(() => _g.Inject(new object()).GremlinLang.GetGremlin()); + } + + [Fact] + public void Unsupported_type_in_list_throws_ArgumentException() + { + Assert.Throws<ArgumentException>(() => + _g.Inject((object)new object[] { 1, new object() }).GremlinLang.GetGremlin()); + } + + [Fact] + public void ConvertParametersToString_empty_returns_empty_map() + { + Assert.Equal("[:]", GremlinLang.ConvertParametersToString(null)); + Assert.Equal("[:]", GremlinLang.ConvertParametersToString(new Dictionary<string, object>())); + } + + [Fact] + public void ConvertParametersToString_basic_parameters() + { + var parameters = new Dictionary<string, object> { { "x", 1 } }; + var result = GremlinLang.ConvertParametersToString(parameters); + Assert.Contains("\"x\":1", result); + } + + [Fact] + public void ConvertParametersToString_unsupported_type_throws() + { + var parameters = new Dictionary<string, object> { { "x", new object() } }; + Assert.Throws<ArgumentException>(() => GremlinLang.ConvertParametersToString(parameters)); + } + + [Fact] + public void ConvertParametersToString_nested_list() + { + var parameters = new Dictionary<string, object> { { "ids", new object[] { 1, 2, 3 } } }; + var result = GremlinLang.ConvertParametersToString(parameters); + Assert.Contains("\"ids\":[1,2,3]", result); + } + + [Fact] + public void ConvertParametersToString_escaped_string_value() + { + var parameters = new Dictionary<string, object> { { "name", "it's a \"test\"" } }; + var result = GremlinLang.ConvertParametersToString(parameters); + Assert.Contains("\"name\":", result); + } } }
diff --git a/gremlin-dotnet/test/Gremlin.Net.UnitTest/Process/Traversal/TraversalTests.cs b/gremlin-dotnet/test/Gremlin.Net.UnitTest/Process/Traversal/TraversalTests.cs index a158ed8..bf7c6c1 100644 --- a/gremlin-dotnet/test/Gremlin.Net.UnitTest/Process/Traversal/TraversalTests.cs +++ b/gremlin-dotnet/test/Gremlin.Net.UnitTest/Process/Traversal/TraversalTests.cs
@@ -184,7 +184,6 @@ [Fact] public void ShouldExtractIdFromVertex() { - GremlinLang.ResetCounter(); var g = AnonymousTraversalSource.Traversal().With(null); // Test basic V() step with mixed ID types
diff --git a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/RequestOptions.java b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/RequestOptions.java index c51f2b8..d51657f 100644 --- a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/RequestOptions.java +++ b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/RequestOptions.java
@@ -44,7 +44,7 @@ public static final RequestOptions EMPTY = RequestOptions.build().create(); private final String graphOrTraversalSource; - private final Map<String, Object> parameters; + private final String parameters; private final Integer batchSize; private final Long timeout; private final String language; @@ -54,7 +54,7 @@ private RequestOptions(final Builder builder) { this.graphOrTraversalSource = builder.graphOrTraversalSource; - this.parameters = builder.parameters; + this.parameters = builder.parametersString; this.batchSize = builder.batchSize; this.timeout = builder.timeout; this.language = builder.language; @@ -67,7 +67,7 @@ return Optional.ofNullable(graphOrTraversalSource); } - public Optional<Map<String, Object>> getParameters() { + public Optional<String> getParameters() { return Optional.ofNullable(parameters); } @@ -114,16 +114,17 @@ if (builder.bulkResults == null) builder.bulkResults(true); - final Map<String, Object> parameters = gremlinLang.getParameters(); - if (parameters != null && !parameters.isEmpty()) { - parameters.forEach(builder::addParameter); + final String parametersString = gremlinLang.getParametersAsString(); + if (!"[:]".equals(parametersString)) { + builder.addParametersString(parametersString); } return builder.create(); } public static final class Builder { private String graphOrTraversalSource = null; - private Map<String, Object> parameters = null; + private Map<String, Object> internalParameters = null; + private String parametersString = null; private Integer batchSize = null; private Long timeout = null; private String materializeProperties = null; @@ -139,7 +140,7 @@ public static Builder from(final RequestOptions options) { final Builder builder = build(); builder.graphOrTraversalSource = options.graphOrTraversalSource; - builder.parameters = options.parameters; + builder.parametersString = options.parameters; builder.batchSize = options.batchSize; builder.timeout = options.timeout; builder.materializeProperties = options.materializeProperties; @@ -158,11 +159,17 @@ } /** - * The parameters to pass on the request. + * Adds a parameter to the request. The parameter map is converted to a gremlin-lang map literal string when the + * request is built. This method accumulates parameters into an internal map which is converted on + * {@link #create()}. + * <p> + * Cannot be mixed with {@link #addParametersString(String)}. */ public Builder addParameter(final String name, final Object value) { - if (null == parameters) - parameters = new HashMap<>(); + if (parametersString != null) + throw new IllegalStateException("Cannot mix addParameter() with addParametersString()"); + + if (internalParameters == null) internalParameters = new HashMap<>(); if (ARGS_G.equals(name)) { this.graphOrTraversalSource = (String) value; @@ -172,7 +179,21 @@ this.language = (String) value; } - parameters.put(name, value); + internalParameters.put(name, value); + return this; + } + + /** + * Sets the parameters as a pre-formatted gremlin-lang map literal string. This allows users to pass parameters + * as a raw gremlin-lang string when using the Client API with a gremlin string. + * <p> + * Cannot be mixed with {@link #addParameter(String, Object)}. + */ + public Builder addParametersString(final String parametersString) { + if (internalParameters != null) + throw new IllegalStateException("Cannot mix addParametersString() with addParameter()"); + + this.parametersString = parametersString; return this; } @@ -228,6 +249,10 @@ } public RequestOptions create() { + // if parameters were added via addParameter(), convert the map to a string + if (internalParameters != null) { + parametersString = GremlinLang.convertParametersToString(internalParameters); + } return new RequestOptions(this); } }
diff --git a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/remote/DriverRemoteConnection.java b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/remote/DriverRemoteConnection.java index c909e48..baae303 100644 --- a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/remote/DriverRemoteConnection.java +++ b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/remote/DriverRemoteConnection.java
@@ -216,6 +216,12 @@ @Override public <E> CompletableFuture<RemoteTraversal<?, E>> submitAsync(final GremlinLang gremlinLang) throws RemoteConnectionException { + if (gremlinLang.containsUnsupportedTypes()) { + throw new IllegalArgumentException(String.format( + "GremlinLang contains at least one type [%s] that cannot be represented as text.", + gremlinLang.getUnsupportedType())); + } + try { gremlinLang.addG(remoteTraversalSourceName); return client.submitAsync(gremlinLang.getGremlin(), getRequestOptions(gremlinLang))
diff --git a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/remote/TransactionRemoteConnection.java b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/remote/TransactionRemoteConnection.java index 6ea9ce5..050ca00 100644 --- a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/remote/TransactionRemoteConnection.java +++ b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/remote/TransactionRemoteConnection.java
@@ -68,6 +68,12 @@ throw new RemoteConnectionException("Transaction is not open"); } + if (gremlinLang.containsUnsupportedTypes()) { + throw new IllegalArgumentException(String.format( + "GremlinLang contains at least one type [%s] that cannot be represented as text.", + gremlinLang.getUnsupportedType())); + } + try { // Synchronous submission through transaction final ResultSet rs = transaction.submit(gremlinLang.getGremlin(), getRequestOptions(gremlinLang));
diff --git a/gremlin-go/driver/client.go b/gremlin-go/driver/client.go index 4446350..5bee34e 100644 --- a/gremlin-go/driver/client.go +++ b/gremlin-go/driver/client.go
@@ -163,9 +163,12 @@ func (client *Client) submitGremlinLang(gremlinLang *GremlinLang) (ResultSet, error) { client.logHandler.logf(Debug, submitStartedString, *gremlinLang) requestOptionsBuilder := new(RequestOptionsBuilder) - if len(gremlinLang.GetParameters()) > 0 { - requestOptionsBuilder.SetBindings(gremlinLang.GetParameters()) + + parametersString := gremlinLang.GetParametersAsString() + if parametersString != "[:]" { + requestOptionsBuilder.SetBindingsString(parametersString) } + if len(gremlinLang.optionsStrategies) > 0 { requestOptionsBuilder = applyOptionsConfig(requestOptionsBuilder, gremlinLang.optionsStrategies[0].configuration) } @@ -183,12 +186,14 @@ func applyOptionsConfig(builder *RequestOptionsBuilder, config map[string]interface{}) *RequestOptionsBuilder { builderValue := reflect.ValueOf(builder) - // Map configuration keys to setter method names + // Map configuration keys to setter method names. + // "bindings" is intentionally excluded because bindings are handled separately + // via SetBindingsString in submitGremlinLang, and including it here would + // trigger the mutual exclusion panic between SetBindings and SetBindingsString. setterMap := map[string]string{ "evaluationTimeout": "SetEvaluationTimeout", "batchSize": "SetBatchSize", "userAgent": "SetUserAgent", - "bindings": "SetBindings", "materializeProperties": "SetMaterializeProperties", "bulkResults": "SetBulkResults", }
diff --git a/gremlin-go/driver/gremlinlang.go b/gremlin-go/driver/gremlinlang.go index 2362515..2625cb1 100644 --- a/gremlin-go/driver/gremlinlang.go +++ b/gremlin-go/driver/gremlinlang.go
@@ -28,7 +28,6 @@ "reflect" "strconv" "strings" - "sync/atomic" "time" "github.com/google/uuid" @@ -39,7 +38,6 @@ gremlin []string parameters map[string]interface{} optionsStrategies []*traversalStrategy - paramCount *atomic.Uint64 } // NewGremlinLang creates a new GremlinLang to be used in traversals. @@ -47,7 +45,6 @@ gremlin := make([]string, 0) parameters := make(map[string]interface{}) optionsStrategies := make([]*traversalStrategy, 0) - paramCount := atomic.Uint64{} if gl != nil { gremlin = make([]string, len(gl.gremlin)) copy(gremlin, gl.gremlin) @@ -59,15 +56,12 @@ optionsStrategies = make([]*traversalStrategy, len(gl.optionsStrategies)) copy(optionsStrategies, gl.optionsStrategies) - - paramCount.Store(gl.paramCount.Load()) } return &GremlinLang{ gremlin: gremlin, parameters: parameters, optionsStrategies: optionsStrategies, - paramCount: ¶mCount, } } @@ -175,7 +169,7 @@ return "NaN", nil } if math.IsInf(float64(v), 1) { - return "Infinity", nil + return "+Infinity", nil } if math.IsInf(float64(v), -1) { return "-Infinity", nil @@ -186,7 +180,7 @@ return "NaN", nil } if math.IsInf(v, 1) { - return "Infinity", nil + return "+Infinity", nil } if math.IsInf(v, -1) { return "-Infinity", nil @@ -296,7 +290,13 @@ case reflect.Slice: return gl.translateSlice(arg) default: - return gl.asParameter(arg), nil + // Panic rather than returning an error because unsupported types are programmer + // mistakes (passing a type the library can't serialize), not recoverable runtime + // conditions. This matches the existing GValue validation pattern in this file. + // An error-return approach was considered but every caller in the fluent traversal + // API (AddSource, WithSack, etc.) would need to propagate and immediately panic + // anyway, adding complexity with no practical benefit. + panic(fmt.Sprintf("GremlinLang contains at least one type [%s] that cannot be represented as text", reflect.TypeOf(arg).Name())) } } } @@ -435,13 +435,6 @@ return sb.String(), nil } -func (gl *GremlinLang) asParameter(arg interface{}) string { - paramName := fmt.Sprintf("_%d", gl.paramCount.Load()) - gl.paramCount.Add(1) - gl.parameters[paramName] = arg - return paramName -} - func (gl *GremlinLang) GetGremlin(arg ...string) string { var g string gremlin := strings.Join(gl.gremlin, "") @@ -461,12 +454,39 @@ return gl.parameters } -func (gl *GremlinLang) AddG(g string) { - gl.parameters["g"] = g +// GetParametersAsString serializes this instance's parameter map to a gremlin-lang map literal string. +func (gl *GremlinLang) GetParametersAsString() string { + return ConvertParametersToString(gl.parameters) } -func (gl *GremlinLang) Reset() { - gl.paramCount.Store(0) +// ConvertParametersToString converts a parameter map to a gremlin-lang map literal string. +// Panics if any value is an unsupported type. +func ConvertParametersToString(params map[string]interface{}) string { + if params == nil || len(params) == 0 { + return "[:]" + } + + helper := NewGremlinLang(nil) + sb := strings.Builder{} + sb.WriteString("[") + first := true + for k, v := range params { + if !first { + sb.WriteString(",") + } + ks, _ := helper.argAsString(k) + vs, _ := helper.argAsString(v) + sb.WriteString(ks) + sb.WriteString(":") + sb.WriteString(vs) + first = false + } + sb.WriteString("]") + return sb.String() +} + +func (gl *GremlinLang) AddG(g string) { + gl.parameters["g"] = g } func (gl *GremlinLang) AddSource(name string, arguments ...interface{}) {
diff --git a/gremlin-go/driver/gremlinlang_test.go b/gremlin-go/driver/gremlinlang_test.go index 9d3ee5b..719bd48 100644 --- a/gremlin-go/driver/gremlinlang_test.go +++ b/gremlin-go/driver/gremlinlang_test.go
@@ -20,8 +20,10 @@ package gremlingo import ( + "fmt" "math" "regexp" + "strings" "testing" "time" @@ -774,3 +776,85 @@ return params } + +func Test_UnsupportedType(t *testing.T) { + t.Run("should panic on unsupported type", func(t *testing.T) { + defer func() { + r := recover() + if r == nil { + t.Error("expected panic for unsupported type") + } + msg := fmt.Sprintf("%v", r) + if !strings.Contains(msg, "cannot be represented as text") { + t.Errorf("unexpected panic message: %v", msg) + } + }() + gl := NewGremlinLang(nil) + gl.argAsString(struct{}{}) + }) + + t.Run("should panic on unsupported type in slice", func(t *testing.T) { + defer func() { + if r := recover(); r == nil { + t.Error("expected panic for unsupported type in slice") + } + }() + gl := NewGremlinLang(nil) + gl.argAsString([]interface{}{1, struct{}{}}) + }) +} + +func Test_ConvertParametersToString(t *testing.T) { + t.Run("should convert empty map", func(t *testing.T) { + result := ConvertParametersToString(nil) + if result != "[:]" { + t.Errorf("expected [:], got %v", result) + } + }) + + t.Run("should convert basic parameters", func(t *testing.T) { + params := map[string]interface{}{"x": int32(1)} + result := ConvertParametersToString(params) + if !strings.Contains(result, "\"x\":1") { + t.Errorf("expected parameter string to contain \"x\":1, got %v", result) + } + }) + + t.Run("should panic on unsupported type in parameters", func(t *testing.T) { + defer func() { + if r := recover(); r == nil { + t.Error("expected panic for unsupported type in parameters") + } + }() + params := map[string]interface{}{"x": struct{}{}} + ConvertParametersToString(params) + }) + + t.Run("should convert nested list parameter", func(t *testing.T) { + params := map[string]interface{}{"ids": []interface{}{int32(1), int32(2), int32(3)}} + result := ConvertParametersToString(params) + if !strings.Contains(result, "\"ids\":[1,2,3]") { + t.Errorf("expected nested list in result, got %v", result) + } + }) + + t.Run("should panic on unsupported type in map value", func(t *testing.T) { + defer func() { + if r := recover(); r == nil { + t.Error("expected panic for unsupported type in map value") + } + }() + gl := NewGremlinLang(nil) + m := map[interface{}]interface{}{"key": struct{}{}} + gl.argAsString(m) + }) + + t.Run("should panic on unsupported type via RequestOptionsBuilder.Create()", func(t *testing.T) { + defer func() { + if r := recover(); r == nil { + t.Error("expected panic for unsupported type in bindings") + } + }() + new(RequestOptionsBuilder).SetBindings(map[string]interface{}{"x": struct{}{}}).Create() + }) +}
diff --git a/gremlin-go/driver/request.go b/gremlin-go/driver/request.go index df02ed4..915cc79 100644 --- a/gremlin-go/driver/request.go +++ b/gremlin-go/driver/request.go
@@ -53,8 +53,8 @@ "g": traversalSource, } - if requestOptions.bindings != nil { - newFields["bindings"] = requestOptions.bindings + if requestOptions.bindingsString != "" && requestOptions.bindingsString != "[:]" { + newFields["bindings"] = requestOptions.bindingsString } if requestOptions.evaluationTimeout != 0 {
diff --git a/gremlin-go/driver/requestOptions.go b/gremlin-go/driver/requestOptions.go index a622abc..81b2370 100644 --- a/gremlin-go/driver/requestOptions.go +++ b/gremlin-go/driver/requestOptions.go
@@ -23,7 +23,7 @@ evaluationTimeout int batchSize int userAgent string - bindings map[string]interface{} + bindingsString string materializeProperties string bulkResults *bool } @@ -33,6 +33,7 @@ batchSize int userAgent string bindings map[string]interface{} + bindingsString string materializeProperties string bulkResults *bool } @@ -53,10 +54,21 @@ } func (builder *RequestOptionsBuilder) SetBindings(bindings map[string]interface{}) *RequestOptionsBuilder { + if builder.bindingsString != "" { + panic("cannot mix SetBindings() with SetBindingsString()") + } builder.bindings = bindings return builder } +func (builder *RequestOptionsBuilder) SetBindingsString(bindingsString string) *RequestOptionsBuilder { + if builder.bindings != nil { + panic("cannot mix SetBindingsString() with SetBindings()") + } + builder.bindingsString = bindingsString + return builder +} + func (builder *RequestOptionsBuilder) SetMaterializeProperties(materializeProperties string) *RequestOptionsBuilder { builder.materializeProperties = materializeProperties return builder @@ -68,6 +80,9 @@ } func (builder *RequestOptionsBuilder) AddBinding(key string, binding interface{}) *RequestOptionsBuilder { + if builder.bindingsString != "" { + panic("cannot mix AddBinding() with SetBindingsString()") + } if builder.bindings == nil { builder.bindings = make(map[string]interface{}) } @@ -81,9 +96,15 @@ requestOptions.evaluationTimeout = builder.evaluationTimeout requestOptions.batchSize = builder.batchSize requestOptions.userAgent = builder.userAgent - requestOptions.bindings = builder.bindings requestOptions.materializeProperties = builder.materializeProperties requestOptions.bulkResults = builder.bulkResults + // convert map bindings to string at creation time, matching Java's RequestOptions.Builder.create() + if builder.bindingsString != "" { + requestOptions.bindingsString = builder.bindingsString + } else if builder.bindings != nil { + requestOptions.bindingsString = ConvertParametersToString(builder.bindings) + } + return *requestOptions }
diff --git a/gremlin-go/driver/requestOptions_test.go b/gremlin-go/driver/requestOptions_test.go index b715fe0..851e4d0 100644 --- a/gremlin-go/driver/requestOptions_test.go +++ b/gremlin-go/driver/requestOptions_test.go
@@ -43,31 +43,32 @@ assert.Equal(t, "TestMaterializeProperties", r.materializeProperties) }) t.Run("Test RequestOptionsBuilder with custom bindings", func(t *testing.T) { - bindings := map[string]interface{}{"x": 2, "y": 5} + bindings := map[string]interface{}{"x": int32(2), "y": int32(5)} r := new(RequestOptionsBuilder).SetBindings(bindings).Create() - assert.Equal(t, bindings, r.bindings) + assert.Contains(t, r.bindingsString, "\"x\":2") + assert.Contains(t, r.bindingsString, "\"y\":5") }) t.Run("Test RequestOptionsBuilder AddBinding() with no other bindings", func(t *testing.T) { - r := new(RequestOptionsBuilder).AddBinding("x", 2).AddBinding("y", 5).Create() - expectedBindings := map[string]interface{}{"x": 2, "y": 5} - assert.Equal(t, expectedBindings, r.bindings) + r := new(RequestOptionsBuilder).AddBinding("x", int32(2)).AddBinding("y", int32(5)).Create() + assert.Contains(t, r.bindingsString, "\"x\":2") + assert.Contains(t, r.bindingsString, "\"y\":5") }) t.Run("Test RequestOptionsBuilder AddBinding() overwriting existing key", func(t *testing.T) { - r := new(RequestOptionsBuilder).AddBinding("x", 2).AddBinding("x", 5).Create() - expectedBindings := map[string]interface{}{"x": 5} - assert.Equal(t, expectedBindings, r.bindings) + r := new(RequestOptionsBuilder).AddBinding("x", int32(2)).AddBinding("x", int32(5)).Create() + assert.Contains(t, r.bindingsString, "\"x\":5") }) t.Run("Test RequestOptionsBuilder AddBinding() with existing bindings", func(t *testing.T) { - bindings := map[string]interface{}{"x": 2, "y": 5} - r := new(RequestOptionsBuilder).SetBindings(bindings).AddBinding("z", 7).Create() - expectedBindings := map[string]interface{}{"x": 2, "y": 5, "z": 7} - assert.Equal(t, expectedBindings, r.bindings) + bindings := map[string]interface{}{"x": int32(2), "y": int32(5)} + r := new(RequestOptionsBuilder).SetBindings(bindings).AddBinding("z", int32(7)).Create() + assert.Contains(t, r.bindingsString, "\"x\":2") + assert.Contains(t, r.bindingsString, "\"y\":5") + assert.Contains(t, r.bindingsString, "\"z\":7") }) - t.Run("Test RequestOptionsBuilder SetBinding(...), SetBinding(nil), AddBinding(...)", func(t *testing.T) { - bindings := map[string]interface{}{"x": 2, "y": 5} - r := new(RequestOptionsBuilder).SetBindings(bindings). - SetBindings(nil).AddBinding("z", 7).Create() - expectedBindings := map[string]interface{}{"z": 7} - assert.Equal(t, expectedBindings, r.bindings) + t.Run("Test RequestOptionsBuilder SetBindings resets previous bindings", func(t *testing.T) { + bindings1 := map[string]interface{}{"x": int32(2)} + bindings2 := map[string]interface{}{"z": int32(7)} + r := new(RequestOptionsBuilder).SetBindings(bindings1).SetBindings(bindings2).Create() + assert.Contains(t, r.bindingsString, "\"z\":7") + assert.NotContains(t, r.bindingsString, "\"x\"") }) }
diff --git a/gremlin-go/driver/request_test.go b/gremlin-go/driver/request_test.go index f14f75a..2da846c 100644 --- a/gremlin-go/driver/request_test.go +++ b/gremlin-go/driver/request_test.go
@@ -57,4 +57,33 @@ new(RequestOptionsBuilder).SetBulkResults(true).Create()) assert.Equal(t, "true", r.Fields["bulkResults"]) }) + + t.Run("Test makeStringRequest() with string bindings", func(t *testing.T) { + r := MakeStringRequest("g.V(x)", "g", + new(RequestOptionsBuilder).SetBindingsString("[\"x\":1]").Create()) + assert.Equal(t, "[\"x\":1]", r.Fields["bindings"]) + }) + + t.Run("Test makeStringRequest() with map bindings converted to string", func(t *testing.T) { + r := MakeStringRequest("g.V(x)", "g", + new(RequestOptionsBuilder).SetBindings(map[string]interface{}{"x": int32(1)}).Create()) + assert.Contains(t, r.Fields["bindings"], "\"x\":1") + }) + + t.Run("Test RequestOptionsBuilder.Create() converts map bindings to string", func(t *testing.T) { + opts := new(RequestOptionsBuilder).SetBindings(map[string]interface{}{"x": int32(1)}).Create() + assert.NotEmpty(t, opts.bindingsString) + assert.Contains(t, opts.bindingsString, "\"x\":1") + }) + + t.Run("Test RequestOptionsBuilder rejects mixing SetBindings and SetBindingsString", func(t *testing.T) { + defer func() { + if r := recover(); r == nil { + t.Error("expected panic when mixing SetBindings and SetBindingsString") + } + }() + new(RequestOptionsBuilder). + SetBindings(map[string]interface{}{"x": int32(1)}). + SetBindingsString("[\"y\":2]") + }) }
diff --git a/gremlin-js/gremlin-javascript/lib/driver/client.ts b/gremlin-js/gremlin-javascript/lib/driver/client.ts index 9283a94..47acaa2 100644 --- a/gremlin-js/gremlin-javascript/lib/driver/client.ts +++ b/gremlin-js/gremlin-javascript/lib/driver/client.ts
@@ -100,10 +100,18 @@ requestBuilder.addLanguage(requestOptions.language); } if (requestOptions?.bindings) { - requestBuilder.addBindings(requestOptions.bindings); + if (typeof requestOptions.bindings === 'string') { + requestBuilder.addBindingsString(requestOptions.bindings); + } else { + requestBuilder.addBindings(requestOptions.bindings); + } } if (bindings) { - requestBuilder.addBindings(bindings); + if (typeof bindings === 'string') { + requestBuilder.addBindingsString(bindings); + } else { + requestBuilder.addBindings(bindings); + } } if (requestOptions?.materializeProperties) { requestBuilder.addMaterializeProperties(requestOptions.materializeProperties);
diff --git a/gremlin-js/gremlin-javascript/lib/driver/driver-remote-connection.ts b/gremlin-js/gremlin-javascript/lib/driver/driver-remote-connection.ts index 94ca4b3..d293462 100644 --- a/gremlin-js/gremlin-javascript/lib/driver/driver-remote-connection.ts +++ b/gremlin-js/gremlin-javascript/lib/driver/driver-remote-connection.ts
@@ -95,9 +95,9 @@ requestOptions.bulkResults = true; } - const params = gremlinLang.getParameters(); - if (params.size > 0) { - requestOptions.params = Object.fromEntries(params); + const parametersString = gremlinLang.getParametersAsString(); + if (parametersString !== '[:]') { + requestOptions.bindings = parametersString; } return { gremlin: gremlinLang.getGremlin(), requestOptions };
diff --git a/gremlin-js/gremlin-javascript/lib/driver/request-message.ts b/gremlin-js/gremlin-javascript/lib/driver/request-message.ts index 9360416..baaa8a2 100644 --- a/gremlin-js/gremlin-javascript/lib/driver/request-message.ts +++ b/gremlin-js/gremlin-javascript/lib/driver/request-message.ts
@@ -17,6 +17,8 @@ * under the License. */ +import GremlinLang from '../process/gremlin-lang.js'; + // Token constants const Tokens = { ARGS_LANGUAGE: 'language', @@ -109,6 +111,7 @@ export class Builder { private readonly gremlin: string; private readonly bindings = {}; + private bindingsString?: string; public language: string; public timeoutMs?: number; public g?: string; @@ -128,16 +131,24 @@ } addBinding(key: string, value: any): Builder { + if (this.bindingsString) throw new Error('Cannot mix addBinding() with addBindingsString().'); Object.assign(this.bindings, {[key]: value}) return this; } addBindings(otherBindings: object): Builder { if (!otherBindings) throw new Error('bindings argument cannot be null.'); + if (this.bindingsString) throw new Error('Cannot mix addBindings() with addBindingsString().'); Object.assign(this.bindings, otherBindings) return this; } + addBindingsString(bindingsString: string): Builder { + if (Object.keys(this.bindings).length > 0) throw new Error('Cannot mix addBindingsString() with addBinding()/addBindings().'); + this.bindingsString = bindingsString; + return this; + } + addG(g: string): Builder { if (!g) throw new Error('g argument cannot be null.'); this.g = g; @@ -175,12 +186,20 @@ * Create the request message given the settings provided to the Builder. */ create(): RequestMessage { + // mutual exclusion between addBindings/addBinding and addBindingsString is + // enforced at call time, so only one path can be set here + let bindings: string | undefined; + if (this.bindingsString) { + bindings = this.bindingsString; + } else if (Object.keys(this.bindings).length > 0) { + bindings = GremlinLang.convertParametersToString(new Map(Object.entries(this.bindings))); + } // @ts-ignore - accessing private constructor from Builder return new RequestMessage( this.gremlin, this.language || 'gremlin-lang', this.timeoutMs, - Object.keys(this.bindings).length > 0 ? this.bindings : undefined, + bindings, this.g, this.materializeProperties, this.bulkResults,
diff --git a/gremlin-js/gremlin-javascript/lib/process/gremlin-lang.ts b/gremlin-js/gremlin-javascript/lib/process/gremlin-lang.ts index 1830a7b..6d8c092 100644 --- a/gremlin-js/gremlin-javascript/lib/process/gremlin-lang.ts +++ b/gremlin-js/gremlin-javascript/lib/process/gremlin-lang.ts
@@ -152,7 +152,7 @@ if (entries.length === 0) return '[:]'; return '[' + entries.map(([k, v]) => `${this._argAsString(k)}:${this._argAsString(v)}`).join(',') + ']'; } - return String(arg); + throw new TypeError(`GremlinLang contains at least one type [${arg?.constructor?.name ?? typeof arg}] that cannot be represented as text.`); } addStep(name: string, args?: any[]): GremlinLang { @@ -193,4 +193,19 @@ } return prefix + this.gremlin; } + + getParametersAsString(): string { + return GremlinLang.convertParametersToString(this.parameters); + } + + static convertParametersToString(params: Map<string, any> | null): string { + if (params == null || params.size === 0) return '[:]'; + + const helper = new GremlinLang(); + const parts: string[] = []; + params.forEach((v, k) => { + parts.push(`${helper._argAsString(k)}:${helper._argAsString(v)}`); + }); + return '[' + parts.join(',') + ']'; + } } \ No newline at end of file
diff --git a/gremlin-js/gremlin-javascript/test/unit/gremlin-lang-test.js b/gremlin-js/gremlin-javascript/test/unit/gremlin-lang-test.js index be3a39c..40472c9 100644 --- a/gremlin-js/gremlin-javascript/test/unit/gremlin-lang-test.js +++ b/gremlin-js/gremlin-javascript/test/unit/gremlin-lang-test.js
@@ -435,4 +435,63 @@ assert.strictEqual(g.inject(view).getGremlinLang().getGremlin(), 'g.inject(Binary("AQID"))'); }); }); + + describe('Unsupported type tests', function () { + it('should throw TypeError for unsupported type', function () { + class UnsupportedType {} + assert.throws(() => { + g.inject(new UnsupportedType()).getGremlinLang().getGremlin(); + }, TypeError); + }); + + it('should throw TypeError for unsupported type in array', function () { + class UnsupportedType {} + assert.throws(() => { + g.inject([1, new UnsupportedType()]).getGremlinLang().getGremlin(); + }, TypeError); + }); + + it('should include type name in error message', function () { + class MyCustomType {} + assert.throws(() => { + g.inject(new MyCustomType()).getGremlinLang().getGremlin(); + }, /cannot be represented as text/); + }); + }); + + describe('convertParametersToString', function () { + it('should return [:] for empty map', function () { + assert.strictEqual(GremlinLang.convertParametersToString(null), '[:]'); + assert.strictEqual(GremlinLang.convertParametersToString(new Map()), '[:]'); + }); + + it('should convert basic parameters', function () { + const params = new Map([['x', 1]]); + const result = GremlinLang.convertParametersToString(params); + assert.ok(result.includes("'x':1")); + }); + + it('should throw TypeError for unsupported type in parameters', function () { + class UnsupportedType {} + const params = new Map([['x', new UnsupportedType()]]); + assert.throws(() => { + GremlinLang.convertParametersToString(params); + }, TypeError); + }); + + it('should convert nested list parameter', function () { + const params = new Map([['ids', [1, 2, 3]]]); + const result = GremlinLang.convertParametersToString(params); + assert.ok(result.includes("'ids':[1,2,3]")); + }); + + it('should convert multiple parameters', function () { + const params = new Map([['x', 1], ['name', 'marko']]); + const result = GremlinLang.convertParametersToString(params); + assert.ok(result.startsWith('[')); + assert.ok(result.endsWith(']')); + assert.ok(result.includes("'x':1")); + assert.ok(result.includes("'name':'marko'")); + }); + }); }); \ No newline at end of file
diff --git a/gremlin-python/src/main/python/gremlin_python/driver/client.py b/gremlin-python/src/main/python/gremlin_python/driver/client.py index 5399561..1ad25d9 100644 --- a/gremlin-python/src/main/python/gremlin_python/driver/client.py +++ b/gremlin-python/src/main/python/gremlin_python/driver/client.py
@@ -157,7 +157,11 @@ # TODO: bindings is now part of request_options, evaluate the need to keep it separate in python. # Note this bindings parameter only applies to string script submissions if isinstance(message, str) and bindings: - fields['bindings'] = bindings + from gremlin_python.process.traversal import GremlinLang + if isinstance(bindings, dict): + fields['bindings'] = GremlinLang.convert_parameters_to_string(bindings) + else: + fields['bindings'] = bindings if isinstance(message, str): log.debug("fields='%s', gremlin='%s'", str(fields), str(message)) @@ -168,14 +172,10 @@ message.fields.update({token: request_options[token] for token in request.Tokens if token in request_options and token != 'bindings'}) if 'bindings' in request_options: - if 'bindings' in message.fields: - message.fields['bindings'].update(request_options['bindings']) - else: - message.fields['bindings'] = request_options['bindings'] - if 'params' in request_options: - if 'bindings' in message.fields: - message.fields['bindings'].update(request_options['params']) - else: - message.fields['bindings'] = request_options['params'] + bindings_val = request_options['bindings'] + if isinstance(bindings_val, dict): + from gremlin_python.process.traversal import GremlinLang + bindings_val = GremlinLang.convert_parameters_to_string(bindings_val) + message.fields['bindings'] = bindings_val return conn.write(message)
diff --git a/gremlin-python/src/main/python/gremlin_python/driver/driver_remote_connection.py b/gremlin-python/src/main/python/gremlin_python/driver/driver_remote_connection.py index 0039a46..2393e72 100644 --- a/gremlin-python/src/main/python/gremlin_python/driver/driver_remote_connection.py +++ b/gremlin-python/src/main/python/gremlin_python/driver/driver_remote_connection.py
@@ -126,7 +126,9 @@ # request the server to bulk results by default when using drc through request options if 'bulkResults' not in request_options: request_options['bulkResults'] = True - if gremlin_lang.parameters is not None and len(gremlin_lang.parameters) > 0: - request_options["params"] = gremlin_lang.parameters + + parameters_string = gremlin_lang.get_parameters_as_string() + if parameters_string != '[:]': + request_options["bindings"] = parameters_string return request_options
diff --git a/gremlin-python/src/main/python/gremlin_python/process/traversal.py b/gremlin-python/src/main/python/gremlin_python/process/traversal.py index 4a531d5..76c9182 100644 --- a/gremlin-python/src/main/python/gremlin_python/process/traversal.py +++ b/gremlin-python/src/main/python/gremlin_python/process/traversal.py
@@ -814,13 +814,11 @@ self.gremlin = [] self.parameters = {} self.options_strategies = [] - self.param_count = AtomicInteger() if gremlin_lang is not None: self.gremlin = list(gremlin_lang.gremlin) self.parameters = dict(gremlin_lang.parameters) self.options_strategies = list(gremlin_lang.options_strategies) - self.param_count = gremlin_lang.param_count def _add_to_gremlin(self, string_name, *args): @@ -939,18 +937,17 @@ if isinstance(arg, list): return self._process_list(arg) - if hasattr(arg, '__class__'): - try: - return arg.__name__ - except AttributeError: - pass + # Strategy instances render as their name (e.g. "ReadOnlyStrategy") for withoutStrategies. + # Class objects render as their __name__ (e.g. strategy classes passed directly). + # These replace the old hasattr(arg, '__class__') check which was too broad since every + # Python object has __class__, making it a silent escape hatch for anything with __name__. + if isinstance(arg, TraversalStrategy): + return arg.strategy_name - return self._as_parameter(arg) + if isinstance(arg, type): + return arg.__name__ - def _as_parameter(self, arg): - param_name = f'_{self.param_count.get_and_increment()}' - self.parameters[param_name] = arg - return param_name + raise TypeError(f'GremlinLang contains at least one type [{type(arg).__name__}] that cannot be represented as text.') # Do special processing needed to format predicates that come in # such as "gt(a)" correctly. @@ -1029,12 +1026,27 @@ def get_parameters(self): return self.parameters + def get_parameters_as_string(self): + return GremlinLang.convert_parameters_to_string(self.parameters) + + @staticmethod + def convert_parameters_to_string(params): + """Converts a parameter map to a gremlin-lang map literal string. + + Raises TypeError if any value is an unsupported type. + """ + if params is None or len(params) == 0: + return '[:]' + + helper = GremlinLang() + parts = [] + for k, v in params.items(): + parts.append(f'{helper._arg_as_string(k)}:{helper._arg_as_string(v)}') + return '[' + ','.join(parts) + ']' + def add_g(self, g): self.parameters['g'] = g - def reset(self): - self.param_count.set(0) - def add_source(self, source_name, *args): if source_name == 'withStrategies' and len(args) != 0:
diff --git a/gremlin-python/src/main/python/tests/integration/driver/test_client.py b/gremlin-python/src/main/python/tests/integration/driver/test_client.py index 2c8c04c..a21590f 100644 --- a/gremlin-python/src/main/python/tests/integration/driver/test_client.py +++ b/gremlin-python/src/main/python/tests/integration/driver/test_client.py
@@ -261,7 +261,7 @@ g = GraphTraversalSource(Graph(), TraversalStrategies()) # Note that bindings for constructed traversals is done via Parameter only t = g.with_('language', 'gremlin-lang').V(GValue('x', [1, 2, 3])).count() - request_opts = {'language': 'gremlin-lang', 'params': {'x': [1, 2, 3]}} + request_opts = {'language': 'gremlin-lang', 'bindings': {'x': [1, 2, 3]}} message = create_basic_request_message(t) result_set = client.submit(message, request_options=request_opts) assert result_set.all().result()[0] == 3 @@ -269,10 +269,10 @@ result_set = client.submit('g.V(x).values("name")', request_options=request_opts) assert result_set.all().result()[0] == 'marko' # For script submission only, we can also add bindings to request options and they will be applied - request_opts['bindings'] = {'y': 4} - result_set = client.submit('g.V(y).values("name")', request_options=request_opts) + request_opts2 = {'language': 'gremlin-lang', 'bindings': {'y': 4}} + result_set = client.submit('g.V(y).values("name")', request_options=request_opts2) assert result_set.all().result()[0] == 'josh' - result_set = client.submit('g.V(z).values("name")', bindings={'z': 5}, request_options=request_opts) + result_set = client.submit('g.V(z).values("name")', bindings={'z': 5}) assert result_set.all().result()[0] == 'ripple'
diff --git a/gremlin-python/src/main/python/tests/unit/process/test_gremlin_lang.py b/gremlin-python/src/main/python/tests/unit/process/test_gremlin_lang.py index 0f1351e..edf0515 100644 --- a/gremlin-python/src/main/python/tests/unit/process/test_gremlin_lang.py +++ b/gremlin-python/src/main/python/tests/unit/process/test_gremlin_lang.py
@@ -25,7 +25,7 @@ from gremlin_python.process.traversal import within, eq, T, Order, Scope, Column, Operator, P, Pop, Cardinality, \ between, inside, WithOptions, ShortestPath, starting_with, ending_with, containing, gt, lte, GValue from gremlin_python.statics import SingleByte, short, long, bigint, BigDecimal, SingleChar -from gremlin_python.structure.graph import Graph, Vertex, Edge, VertexProperty +from gremlin_python.structure.graph import Graph, Vertex from gremlin_python.process.anonymous_traversal import traversal from gremlin_python.process.graph_traversal import __ from datetime import datetime, timezone, timedelta @@ -415,80 +415,66 @@ "g.V(0)"]) # 107 - outVertex = Vertex(0, "person") - inVertex = Vertex(1, "person") - edge = Edge(2, outVertex, "knows", inVertex) - tests.append([g.E(edge), - "g.E(_0)"]) - - # 108 - vp = VertexProperty(3, "time", "18:00", None) - tests.append([g.inject(vp), - "g.inject(_1)"]) - - # 109 tests.append([g.inject({'name': 'java'}, {T.id: 0}, {}, {'age': float(10), 'pos_inf': float("inf"), 'neg_inf': float("-inf"), 'nan': float("nan")}), "g.inject(['name':'java'],[(T.id):0],[:],['age':10.0D,'pos_inf':+Infinity,'neg_inf':-Infinity,'nan':NaN])"]) - # 110 + # 108 tests.append([g.inject(float(1)).is_(P.eq(1).or_(P.gt(2)).or_(P.lte(3)).or_(P.gte(4))), "g.inject(1.0D).is(eq(1).or(gt(2)).or(lte(3)).or(gte(4)))"]) - # 111 + # 109 tests.append([g.V().has_label('person').has('age', P.gt(10).or_(P.gte(11).and_(P.lt(20))).and_( P.lt(29).or_(P.eq(35)))).name, "g.V().hasLabel('person').has('age',gt(10).or(gte(11).and(lt(20))).and(lt(29).or(eq(35)))).values('name')"]) - # 112 + # 110 tests.append([g.inject(set('a')), "g.inject({'a'})"]) - # 113 + # 111 tests.append([g.merge_v(None), "g.mergeV(null)"]) - # 114 + # 112 tests.append([g.merge_e(None), "g.mergeE(null)"]) - # 115 + # 113 tests.append([g.add_v('\"test\"'), "g.addV('\"test\"')"]) - # 116 + # 114 tests.append([g.add_v('t\'"est'), "g.addV('t\\'\"est')"]) - # 117 + # 115 tests.append([g.inject(True, SingleByte(1), short(2), 3, long(4), float(5), float(6), bigint(7), BigDecimal(0, 8)), "g.inject(true,1B,2S,3,4L,5.0D,6.0D,7N,8M)"]) - #118 + # 116 tests.append([g.inject(uuid.UUID('9b8d8a9c-61c2-43e5-9cc8-c27b9261290e')), 'g.inject(UUID("9b8d8a9c-61c2-43e5-9cc8-c27b9261290e"))']) - # 119 + # 117 tests.append([g.add_v('test').property('date', datetime(2021, 2, 1, 9, 30, tzinfo=timezone.utc)), "g.addV('test').property('date',datetime(\"2021-02-01T09:30:00+00:00\"))"]) - - # 120 + + # 118 tests.append([g.add_v('test').property('date', datetime(2021, 2, 1, 9, 30, tzinfo=timezone(timedelta(hours=7)))), "g.addV('test').property('date',datetime(\"2021-02-01T09:30:00+07:00\"))"]) - - # 121 + + # 119 tests.append([g.add_v('test').property('date', datetime(2021, 2, 1, 9, 30, tzinfo=timezone(timedelta(hours=-5)))), "g.addV('test').property('date',datetime(\"2021-02-01T09:30:00-05:00\"))"]) - # Character - backslash (not in feature file due to Gherkin escaping issues) - # 122 + # 120 - Character backslash (not in feature file due to Gherkin escaping issues) tests.append([g.inject(SingleChar('\\')), "g.inject('\\\\'c)"]) - # Character - single quote (no feature equivalent) - # 123 + # 121 - Character single quote (no feature equivalent) tests.append([g.inject(SingleChar("'")), "g.inject(\"'\"c)"]) @@ -545,3 +531,51 @@ gremlin = g.inject(p).V(p).gremlin_lang assert 'g.inject(ids).V(ids)' == gremlin.get_gremlin() assert val == gremlin.get_parameters().get('ids') + + def test_unsupported_type_throws(self): + g = traversal().with_(None) + import pytest + with pytest.raises(TypeError, match='cannot be represented as text'): + g.inject(object()).gremlin_lang.get_gremlin() + + def test_unsupported_type_in_list_throws(self): + g = traversal().with_(None) + import pytest + with pytest.raises(TypeError, match='cannot be represented as text'): + g.inject([1, object()]).gremlin_lang.get_gremlin() + + def test_unsupported_type_in_dict_throws(self): + g = traversal().with_(None) + import pytest + with pytest.raises(TypeError, match='cannot be represented as text'): + g.inject({'key': object()}).gremlin_lang.get_gremlin() + + def test_unsupported_type_in_convert_parameters_to_string_throws(self): + from gremlin_python.process.traversal import GremlinLang + import pytest + with pytest.raises(TypeError, match='cannot be represented as text'): + GremlinLang.convert_parameters_to_string({'x': object()}) + + def test_convert_parameters_to_string_empty(self): + from gremlin_python.process.traversal import GremlinLang + assert '[:]' == GremlinLang.convert_parameters_to_string({}) + assert '[:]' == GremlinLang.convert_parameters_to_string(None) + + def test_convert_parameters_to_string_basic(self): + from gremlin_python.process.traversal import GremlinLang + result = GremlinLang.convert_parameters_to_string({'x': 1, 'name': 'marko'}) + assert result.startswith('[') + assert result.endswith(']') + assert "'x':1" in result + assert "'name':'marko'" in result + + def test_convert_parameters_to_string_nested_list(self): + from gremlin_python.process.traversal import GremlinLang + result = GremlinLang.convert_parameters_to_string({'ids': [1, 2, 3]}) + assert "'ids':[1,2,3]" in result + + def test_convert_parameters_to_string_escaped_string(self): + from gremlin_python.process.traversal import GremlinLang + result = GremlinLang.convert_parameters_to_string({'name': "it's a test"}) + assert "'name'" in result + assert "it" in result
diff --git a/gremlin-python/src/main/python/tests/unit/process/test_strategies.py b/gremlin-python/src/main/python/tests/unit/process/test_strategies.py index 0667f0d..324e125 100644 --- a/gremlin-python/src/main/python/tests/unit/process/test_strategies.py +++ b/gremlin-python/src/main/python/tests/unit/process/test_strategies.py
@@ -52,24 +52,21 @@ ## gremlin_script = g.without_strategies(ReadOnlyStrategy()).V().gremlin_lang gremlin_instr = gremlin_script.gremlin - gremlin_params = gremlin_script.parameters assert "withStrategies" in str(gremlin_script) assert "ReadOnlyStrategy" in str(gremlin_script) assert "IncidentToAdjacentStrategy" in str(gremlin_script) assert "withoutStrategies" in str(gremlin_script) assert "V()" in str(gremlin_script) assert "withStrategies(ReadOnlyStrategy,IncidentToAdjacentStrategy)" == gremlin_instr[1] - assert ReadOnlyStrategy() == gremlin_params["_0"] + assert "withoutStrategies(ReadOnlyStrategy)" in str(gremlin_script) ## gremlin_script = g.without_strategies(ReadOnlyStrategy(), LazyBarrierStrategy()).V().gremlin_lang - gremlin_params = gremlin_script.parameters assert "withStrategies" in str(gremlin_script) assert "ReadOnlyStrategy" in str(gremlin_script) assert "IncidentToAdjacentStrategy" in str(gremlin_script) assert "withoutStrategies" in str(gremlin_script) assert "V()" in str(gremlin_script) - assert ReadOnlyStrategy() == gremlin_params["_1"] - assert LazyBarrierStrategy() == gremlin_params["_2"] + assert "withoutStrategies(ReadOnlyStrategy,LazyBarrierStrategy)" in str(gremlin_script) ### g = traversal().with_(None) gremlin_script = g.with_("x", "test").with_("y").gremlin_lang
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/Context.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/Context.java index 06e2507..10148ab 100644 --- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/Context.java +++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/Context.java
@@ -30,7 +30,9 @@ import org.apache.tinkerpop.gremlin.util.Tokens; import org.apache.tinkerpop.gremlin.util.message.RequestMessage; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; @@ -57,6 +59,7 @@ private boolean timeoutExecutorGrabbed = false; private final Object timeoutExecutorLock = new Object(); private String transactionId; // initially null for non-transactional requests and begin() calls; set after transaction creation. + private Map<String, Object> parameters = new HashMap<>(); // only available after string parameters are parsed by grammar. public Context(final RequestMessage requestMessage, final ChannelHandlerContext ctx, final Settings settings, final GraphManager graphManager, @@ -129,6 +132,13 @@ this.transactionId = transactionId; } + public Map<String, Object> getParameters() { + return this.parameters; + } + + public void setParameters(final Map<String, Object> parameters) { + this.parameters = parameters; + } /** * Gets the current request to Gremlin Server.
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpGremlinEndpointHandler.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpGremlinEndpointHandler.java index fa34d4d..560c335 100644 --- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpGremlinEndpointHandler.java +++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpGremlinEndpointHandler.java
@@ -34,6 +34,7 @@ import org.apache.tinkerpop.gremlin.groovy.jsr223.TimedInterruptTimeoutException; import org.apache.tinkerpop.gremlin.jsr223.GremlinScriptEngine; import org.apache.tinkerpop.gremlin.language.grammar.GremlinParserException; +import org.apache.tinkerpop.gremlin.language.grammar.GremlinQueryParser; import org.apache.tinkerpop.gremlin.process.traversal.Failure; import org.apache.tinkerpop.gremlin.process.traversal.Operator; import org.apache.tinkerpop.gremlin.process.traversal.Order; @@ -76,7 +77,6 @@ import java.lang.reflect.UndeclaredThrowableException; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; import java.util.HashSet; import java.util.Iterator; import java.util.List; @@ -200,7 +200,7 @@ try { logger.debug("Processing request containing script [{}] and bindings of [{}] on {}", requestMessage.getFieldOrDefault(Tokens.ARGS_GREMLIN, ""), - requestMessage.getFieldOrDefault(Tokens.ARGS_BINDINGS, Collections.emptyMap()), + requestMessage.getFieldOrDefault(Tokens.ARGS_BINDINGS, "[:]"), Thread.currentThread().getName()); if (settings.enableAuditLog) { AuthenticatedUser user = ctx.channel().attr(StateKey.AUTHENTICATED_USER).get(); @@ -238,6 +238,33 @@ throw new ProcessingException(GremlinError.transactionalControlRequiresTransaction()); } + // Guard against bad parameters while trying to parse string-based bindings into a Map<String, Object> + if (requestMessage.optionalField(Tokens.ARGS_BINDINGS).isPresent()) { + Map<String, Object> bindings = null; + final String bindingsString = (String) requestMessage.getFields().get(Tokens.ARGS_BINDINGS); + try { + bindings = GremlinQueryParser.parseParameters(bindingsString); + } catch (GremlinParserException e) { + throw new ProcessingException(GremlinError.incorrectParameterFormat(bindingsString, e)); + } + + final String language = requestMessage.<String>optionalField(Tokens.ARGS_LANGUAGE).orElse("gremlin-lang"); + if ("gremlin-groovy".equals(language)) { + final Set<String> badBindings = IteratorUtils.set(IteratorUtils.<String>filter( + bindings.keySet().iterator(), + INVALID_BINDINGS_KEYS::contains)); + if (!badBindings.isEmpty()) { + throw new ProcessingException(GremlinError.binding(badBindings)); + } + } + + if (bindings.size() > settings.maxParameters) { + throw new ProcessingException(GremlinError.binding(bindings.size(), settings.maxParameters)); + } + + requestCtx.setParameters(bindings); + } + // Send back the 200 OK response header here since the response is always chunk transfer encoded. Any // failures that follow this will show up in the response body instead. sendHttpResponse(ctx, OK, createResponseHeaders(ctx, serializer, requestCtx).toArray(CharSequence[]::new)); @@ -371,23 +398,6 @@ private void iterateScriptEvalResult(final Context context, MessageSerializer<?> serializer, final RequestMessage message) throws ProcessingException, InterruptedException, ScriptException { - if (message.optionalField(Tokens.ARGS_BINDINGS).isPresent()) { - final Map bindings = (Map) message.getFields().get(Tokens.ARGS_BINDINGS); - if (IteratorUtils.anyMatch(bindings.keySet().iterator(), k -> null == k || !(k instanceof String))) { - throw new ProcessingException(GremlinError.binding()); - } - - final Set<String> badBindings = IteratorUtils.set(IteratorUtils.<String>filter(bindings.keySet().iterator(), INVALID_BINDINGS_KEYS::contains)); - if (!badBindings.isEmpty()) { - throw new ProcessingException(GremlinError.binding(badBindings)); - } - - // ignore control bindings that get passed in with the "#jsr223" prefix - those aren't used in compilation - if (IteratorUtils.count(IteratorUtils.filter(bindings.keySet().iterator(), k -> !k.toString().startsWith("#jsr223"))) > settings.maxParameters) { - throw new ProcessingException(GremlinError.binding(bindings.size(), settings.maxParameters)); - } - } - final Map<String, Object> args = message.getFields(); final String language = args.containsKey(Tokens.ARGS_LANGUAGE) ? (String) args.get(Tokens.ARGS_LANGUAGE) : "gremlin-lang"; final GremlinScriptEngine scriptEngine = gremlinExecutor.getScriptEngineManager().getEngineByName(language); @@ -507,7 +517,7 @@ final RequestMessage msg = ctx.getRequestMessage(); // add any bindings to override any other supplied - Optional.ofNullable((Map<String, Object>) msg.getFields().get(Tokens.ARGS_BINDINGS)).ifPresent(bindings::putAll); + bindings.putAll(ctx.getParameters()); if (msg.getFields().containsKey(Tokens.ARGS_G)) { final String aliased = msg.getField(Tokens.ARGS_G);
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpRequestMessageDecoder.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpRequestMessageDecoder.java index 4501697..dc5dfab 100644 --- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpRequestMessageDecoder.java +++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpRequestMessageDecoder.java
@@ -32,14 +32,10 @@ import io.netty.handler.codec.MessageToMessageDecoder; import org.apache.tinkerpop.shaded.jackson.databind.JsonNode; import org.apache.tinkerpop.shaded.jackson.databind.ObjectMapper; -import org.apache.tinkerpop.shaded.jackson.databind.node.ArrayNode; -import org.apache.tinkerpop.shaded.jackson.databind.node.ObjectNode; import org.javatuples.Pair; import java.io.IOException; -import java.util.ArrayList; import java.util.HashMap; -import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Optional; @@ -194,13 +190,7 @@ final RequestMessage.Builder builder = RequestMessage.build(scriptNode.asText()); final JsonNode bindingsNode = body.get(Tokens.ARGS_BINDINGS); - if (bindingsNode != null && !bindingsNode.isObject()) - throw new IllegalArgumentException("bindings must be a Map"); - - final Map<String, Object> bindings = new HashMap<>(); - if (bindingsNode != null) - bindingsNode.fields().forEachRemaining(kv -> bindings.put(kv.getKey(), fromJsonNode(kv.getValue()))); - builder.addBindings(bindings); + if (bindingsNode != null) builder.addBindings(bindingsNode.asText()); final JsonNode gNode = body.get(Tokens.ARGS_G); if (null != gNode) builder.addG(gNode.asText()); @@ -222,33 +212,4 @@ return builder.create(); } - - private Object fromJsonNode(final JsonNode node) { - if (node.isNull()) - return null; - else if (node.isObject()) { - final Map<String, Object> map = new HashMap<>(); - final ObjectNode objectNode = (ObjectNode) node; - final Iterator<String> iterator = objectNode.fieldNames(); - while (iterator.hasNext()) { - String key = iterator.next(); - map.put(key, fromJsonNode(objectNode.get(key))); - } - return map; - } else if (node.isArray()) { - final ArrayNode arrayNode = (ArrayNode) node; - final ArrayList<Object> array = new ArrayList<>(); - for (int i = 0; i < arrayNode.size(); i++) { - array.add(fromJsonNode(arrayNode.get(i))); - } - return array; - } else if (node.isFloatingPointNumber()) - return node.asDouble(); - else if (node.isIntegralNumber()) - return node.asLong(); - else if (node.isBoolean()) - return node.asBoolean(); - else - return node.asText(); - } }
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/util/GremlinError.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/util/GremlinError.java index 3014438..7a5b77f 100644 --- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/util/GremlinError.java +++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/util/GremlinError.java
@@ -69,6 +69,13 @@ return new GremlinError(HttpResponseStatus.BAD_REQUEST, message, "InvalidRequestException"); } + public static GremlinError incorrectParameterFormat(final String parameters, final GremlinParserException gpe) { + final String message = String.format( + "The provided parameter string \"%s\" could not be converted into a Map. %s", + parameters, gpe.getMessage()); + return new GremlinError(HttpResponseStatus.BAD_REQUEST, message, "InvalidRequestException"); + } + public static GremlinError binding(final Set<String> badBindings) { final String message = String.format("The message supplies one or more invalid parameters key of [%s] - these are reserved names.", badBindings);
diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverTransactionIntegrateTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverTransactionIntegrateTest.java index 9a88487..3cb37d0 100644 --- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverTransactionIntegrateTest.java +++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverTransactionIntegrateTest.java
@@ -25,6 +25,7 @@ import org.apache.tinkerpop.gremlin.driver.Result; import org.apache.tinkerpop.gremlin.driver.remote.DriverRemoteConnection; import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; +import org.apache.tinkerpop.gremlin.process.traversal.step.GValue; import org.apache.tinkerpop.gremlin.server.channel.HttpChannelizer; import org.apache.tinkerpop.gremlin.structure.Transaction; import org.apache.tinkerpop.gremlin.structure.Vertex; @@ -614,4 +615,36 @@ assertEquals(1L, g.V().hasLabel("val").count().next().longValue()); } + + @Test + public void shouldRejectTransactionWithUnsupportedType() throws Exception { + final GraphTraversalSource g = traversal().with(DriverRemoteConnection.using(cluster, GTX)); + final GraphTraversalSource gtx = g.tx().begin(); + try { + gtx.inject(new Thread()).iterate(); + fail("Expected IllegalArgumentException for unsupported type"); + } catch (IllegalArgumentException ex) { + assertThat(ex.getMessage(), containsString("at least one type")); + assertThat(ex.getMessage(), containsString("Thread")); + assertThat(ex.getMessage(), containsString("cannot be represented as text")); + } finally { + gtx.tx().rollback(); + } + } + + @Test + public void shouldRejectTransactionWithUnsupportedTypeInGValue() throws Exception { + final GraphTraversalSource g = traversal().with(DriverRemoteConnection.using(cluster, GTX)); + final GraphTraversalSource gtx = g.tx().begin(); + try { + gtx.V(GValue.of("x", new Thread())).iterate(); + fail("Expected exception for unsupported type in GValue parameter"); + } catch (Exception ex) { + final Throwable inner = ExceptionHelper.getRootCause(ex); + assertThat(inner.getMessage(), containsString("at least one type")); + assertThat(inner.getMessage(), containsString("cannot be represented as text")); + } finally { + gtx.tx().rollback(); + } + } }
diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerHttpIntegrateTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerHttpIntegrateTest.java index 7ce01ea..895b09b 100644 --- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerHttpIntegrateTest.java +++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerHttpIntegrateTest.java
@@ -329,7 +329,7 @@ public void should200OnPOSTWithGremlinQueryStringArgumentWithBindingsAndFunction() throws Exception { final CloseableHttpClient httpclient = HttpClients.createDefault(); final HttpPost httppost = new HttpPost(TestClientFactory.createURLString()); - httppost.setEntity(new StringEntity("{\"gremlin\":\"addItUp(Integer.parseInt(x),Integer.parseInt(y))\",\"language\":\"gremlin-groovy\",\"bindings\":{\"x\":\"10\", \"y\":\"10\"}}", Consts.UTF_8)); + httppost.setEntity(new StringEntity("{\"gremlin\":\"addItUp(Integer.parseInt(x),Integer.parseInt(y))\",\"language\":\"gremlin-groovy\",\"bindings\":\"[x:\\\"10\\\",y:\\\"10\\\"]\"}", Consts.UTF_8)); try (final CloseableHttpResponse response = httpclient.execute(httppost)) { assertEquals(200, response.getStatusLine().getStatusCode()); @@ -596,7 +596,7 @@ final CloseableHttpClient httpclient = HttpClients.createDefault(); final HttpPost httppost = new HttpPost(TestClientFactory.createURLString()); httppost.addHeader("Content-Type", "application/json"); - httppost.setEntity(new StringEntity("{\"gremlin\":\"x+y\", \"bindings\":{\"x\":10, \"y\":10}, \"language\":\"gremlin-groovy\"}", Consts.UTF_8)); + httppost.setEntity(new StringEntity("{\"gremlin\":\"x+y\", \"bindings\":\"[x:10,y:10]\", \"language\":\"gremlin-groovy\"}", Consts.UTF_8)); try (final CloseableHttpResponse response = httpclient.execute(httppost)) { assertEquals(200, response.getStatusLine().getStatusCode()); @@ -612,7 +612,7 @@ final CloseableHttpClient httpclient = HttpClients.createDefault(); final HttpPost httppost = new HttpPost(TestClientFactory.createURLString()); httppost.addHeader("Content-Type", "application/json"); - httppost.setEntity(new StringEntity("{\"gremlin\":\"g.inject(x)\", \"bindings\":{\"x\":10}, \"language\":\"gremlin-groovy\"}", Consts.UTF_8)); + httppost.setEntity(new StringEntity("{\"gremlin\":\"g.inject(x)\", \"bindings\":\"[x:10]\", \"language\":\"gremlin-groovy\"}", Consts.UTF_8)); try (final CloseableHttpResponse response = httpclient.execute(httppost)) { assertEquals(200, response.getStatusLine().getStatusCode()); @@ -628,7 +628,7 @@ final CloseableHttpClient httpclient = HttpClients.createDefault(); final HttpPost httppost = new HttpPost(TestClientFactory.createURLString()); httppost.addHeader("Content-Type", "application/json"); - httppost.setEntity(new StringEntity("{\"gremlin\":\"g.inject(x)\", \"bindings\":{\"x\":10.5}, \"language\":\"gremlin-groovy\"}", Consts.UTF_8)); + httppost.setEntity(new StringEntity("{\"gremlin\":\"g.inject(x)\", \"bindings\":\"[x:10.5]\", \"language\":\"gremlin-groovy\"}", Consts.UTF_8)); try (final CloseableHttpResponse response = httpclient.execute(httppost)) { assertEquals(200, response.getStatusLine().getStatusCode()); @@ -644,7 +644,7 @@ final CloseableHttpClient httpclient = HttpClients.createDefault(); final HttpPost httppost = new HttpPost(TestClientFactory.createURLString()); httppost.addHeader("Content-Type", "application/json"); - httppost.setEntity(new StringEntity("{\"gremlin\":\"g.V(1).out(x).values('name')\", \"bindings\":{\"x\":\"knows\"}, \"g\":\"gmodern\"}", Consts.UTF_8)); + httppost.setEntity(new StringEntity("{\"gremlin\":\"g.V(1).out(x).values('name')\", \"bindings\":\"[x:\\\"knows\\\"]\", \"g\":\"gmodern\"}", Consts.UTF_8)); try (final CloseableHttpResponse response = httpclient.execute(httppost)) { assertEquals(200, response.getStatusLine().getStatusCode()); @@ -660,7 +660,7 @@ final CloseableHttpClient httpclient = HttpClients.createDefault(); final HttpPost httppost = new HttpPost(TestClientFactory.createURLString()); httppost.addHeader("Content-Type", "application/json"); - httppost.setEntity(new StringEntity("{\"gremlin\":\"g.inject(x)\", \"bindings\":{\"x\":true}, \"language\":\"gremlin-groovy\"}", Consts.UTF_8)); + httppost.setEntity(new StringEntity("{\"gremlin\":\"g.inject(x)\", \"bindings\":\"[x:true]\", \"language\":\"gremlin-groovy\"}", Consts.UTF_8)); try (final CloseableHttpResponse response = httpclient.execute(httppost)) { assertEquals(200, response.getStatusLine().getStatusCode()); @@ -676,7 +676,7 @@ final CloseableHttpClient httpclient = HttpClients.createDefault(); final HttpPost httppost = new HttpPost(TestClientFactory.createURLString()); httppost.addHeader("Content-Type", "application/json"); - httppost.setEntity(new StringEntity("{\"gremlin\":\"g.inject(x)\", \"bindings\":{\"x\":null}, \"language\":\"gremlin-groovy\"}", Consts.UTF_8)); + httppost.setEntity(new StringEntity("{\"gremlin\":\"g.inject(x)\", \"bindings\":\"[x:null]\", \"language\":\"gremlin-groovy\"}", Consts.UTF_8)); try (final CloseableHttpResponse response = httpclient.execute(httppost)) { assertEquals(200, response.getStatusLine().getStatusCode()); @@ -692,7 +692,7 @@ final CloseableHttpClient httpclient = HttpClients.createDefault(); final HttpPost httppost = new HttpPost(TestClientFactory.createURLString()); httppost.addHeader("Content-Type", "application/json"); - httppost.setEntity(new StringEntity("{\"gremlin\":\"g.inject(x).unfold()\", \"bindings\":{\"x\":[1,2,3]}, \"language\":\"gremlin-groovy\"}", Consts.UTF_8)); + httppost.setEntity(new StringEntity("{\"gremlin\":\"g.inject(x).unfold()\", \"bindings\":\"[x:[1,2,3]]\", \"language\":\"gremlin-groovy\"}", Consts.UTF_8)); try (final CloseableHttpResponse response = httpclient.execute(httppost)) { assertEquals(200, response.getStatusLine().getStatusCode()); @@ -711,7 +711,7 @@ final CloseableHttpClient httpclient = HttpClients.createDefault(); final HttpPost httppost = new HttpPost(TestClientFactory.createURLString()); httppost.addHeader("Content-Type", "application/json"); - httppost.setEntity(new StringEntity("{\"gremlin\":\"g.inject(x)\", \"bindings\":{\"x\":{\"y\":1}}, \"language\":\"gremlin-groovy\"}", Consts.UTF_8)); + httppost.setEntity(new StringEntity("{\"gremlin\":\"g.inject(x)\", \"bindings\":\"[x:[\\\"y\\\":1]]\", \"language\":\"gremlin-groovy\"}", Consts.UTF_8)); try (final CloseableHttpResponse response = httpclient.execute(httppost)) { assertEquals(200, response.getStatusLine().getStatusCode()); @@ -1158,7 +1158,7 @@ public void should400OnPOSTWithInvalidRequestArgsWhenInvalidBindingsSupplied() throws Exception { final GraphSONMessageSerializerV4 serializer = new GraphSONMessageSerializerV4(); final ByteBuf serializedRequest = serializer.serializeRequestAsBinary( - RequestMessage.build("g.V(id)").addBinding("id", "1").create(), + RequestMessage.build("g.V(id)").addBindings("[id:1]").addLanguage("gremlin-groovy").create(), new UnpooledByteBufAllocator(false)); final CloseableHttpClient httpclient = HttpClients.createDefault(); @@ -1168,7 +1168,7 @@ httppost.setEntity(new ByteArrayEntity(serializedRequest.array())); try (final CloseableHttpResponse response = httpclient.execute(httppost)) { - assertEquals(200, response.getStatusLine().getStatusCode()); + assertEquals(400, response.getStatusLine().getStatusCode()); final String json = EntityUtils.toString(response.getEntity()); final JsonNode node = mapper.readTree(json); assertTrue(node.get("status").get("message").asText().contains("The message supplies one or more invalid parameters key"));
diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java index 9f56cb2..9b4030e 100644 --- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java +++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java
@@ -532,9 +532,9 @@ try (SimpleClient client = TestClientFactory.createSimpleHttpClient()) { final Map<String, Object> bindings = new HashMap<>(); - bindings.put("id", "123"); + bindings.put("id", "123"); // "id" is invalid for gremlin-groovy, but not gremlin-lang final RequestMessage request = RequestMessage.build("g.inject(1,2,3,4,5,6,7,8,9,0)") - .addBindings(bindings).create(); + .addBindings(bindings).addLanguage("gremlin-groovy").create(); final CountDownLatch latch = new CountDownLatch(1); final AtomicBoolean pass = new AtomicBoolean(false); client.submit(request, result -> { @@ -717,31 +717,6 @@ } } - @Test - public void shouldGarbageCollectPhantomButNotHard() throws Exception { - final Cluster cluster = TestClientFactory.open(); - final Client client = cluster.connect(); - - assertEquals(2, client.submit("addItUp(1,1)", groovyRequestOptions).all().join().get(0).getInt()); - assertEquals(0, client.submit("def subtract(x,y){x-y};subtract(1,1)", groovyRequestOptions).all().join().get(0).getInt()); - assertEquals(0, client.submit("subtract(1,1)", groovyRequestOptions).all().join().get(0).getInt()); - - RequestOptions options = RequestOptions.build() - .addParameter(GremlinGroovyScriptEngine.KEY_REFERENCE_TYPE, GremlinGroovyScriptEngine.REFERENCE_TYPE_PHANTOM) - .language("gremlin-groovy") - .create(); - assertEquals(4, client.submit("def multiply(x,y){x*y};multiply(2,2)", options).all().join().get(0).getInt()); - - try { - client.submit("multiply(2,2)", groovyRequestOptions).all().join().get(0).getInt(); - fail("Should throw an exception since reference is phantom."); - } catch (RuntimeException ignored) { - - } finally { - cluster.close(); - } - } - @SuppressWarnings("ThrowableResultOfMethodCallIgnored") @Test public void shouldBlockRequestWhenTooBig() throws Exception { @@ -1088,4 +1063,56 @@ assertEquals(new BigDecimal("1.0"), client.submit("g.inject(1.0)", gremlinGroovy).one().getObject()); assertEquals(new BigDecimal("-123.456"), client.submit("g.inject(-123.456)", gremlinGroovy).one().getObject()); } + + @Test + public void shouldSubmitWithStringBindingsViaRequestMessage() throws Exception { + try (SimpleClient client = TestClientFactory.createSimpleHttpClient()) { + final RequestMessage request = RequestMessage.build("g.V(x).out(y).values('name')") + .addBindings("[\"x\":1,\"y\":\"knows\"]").addG("gmodern").create(); + final List<ResponseMessage> responses = client.submit(request); + assertEquals(HttpResponseStatus.OK, responses.get(0).getStatus().getCode()); + assertEquals("vadas", responses.get(0).getResult().getData().get(0)); + } + } + + @Test + public void shouldRejectTraversalInjectionInStringBindings() throws Exception { + try (SimpleClient client = TestClientFactory.createSimpleHttpClient()) { + final RequestMessage request = RequestMessage.build("g.V(x)") + .addBindings("[x:__.V().drop()]").addG("gmodern").create(); + final List<ResponseMessage> responses = client.submit(request); + assertEquals(HttpResponseStatus.BAD_REQUEST, responses.get(0).getStatus().getCode()); + } + } + + @Test + public void shouldReturnUserFriendlyErrorMessageForMalformedParameterStrings() throws Exception { + final Cluster cluster = TestClientFactory.build().create(); + final Client client = cluster.connect(); + + // each entry is [malformed input, expected substring in error message] + final String[][] cases = { + {"[\"x\":", "could not be converted into a Map. Query parsing failed at"}, + {"not a map at all", "could not be converted into a Map. Query parsing failed at"}, + {"[\"x\":\"unclosed]", "could not be converted into a Map. Query parsing failed at"}, + {"[\"x\":,\"y\":1]", "could not be converted into a Map. Query parsing failed at"}, + {"[\"x\":__.V().drop()]", "Traversals are not allowed"}, + {"[\"~id\":1]", "must be a valid identifier"} + }; + + for (final String[] testCase : cases) { + final ResultSet result = client.submit( + "g.V(x)", RequestOptions.build().addParametersString(testCase[0]).create()); + try { + result.one(); + } catch (Exception e) { + assertTrue(ExceptionHelper.getRootCause(e) instanceof ResponseException); + final ResponseException re = (ResponseException) ExceptionHelper.getRootCause(e); + assertEquals(HttpResponseStatus.BAD_REQUEST, re.getResponseStatusCode()); + assertTrue(re.getMessage().contains(testCase[1])); + } + } + + cluster.close(); + } }
diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/HttpDriverIntegrateTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/HttpDriverIntegrateTest.java index f6452ab..b1f6423 100644 --- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/HttpDriverIntegrateTest.java +++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/HttpDriverIntegrateTest.java
@@ -30,6 +30,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.Merge; import org.apache.tinkerpop.gremlin.process.traversal.P; import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; +import org.apache.tinkerpop.gremlin.process.traversal.step.GValue; import org.apache.tinkerpop.gremlin.server.channel.HttpChannelizer; import org.apache.tinkerpop.gremlin.structure.VertexProperty; import org.apache.tinkerpop.gremlin.util.CollectionUtil; @@ -238,9 +239,8 @@ fail("Should have thrown exception over bad serialization"); } catch (Exception ex) { final Throwable inner = ExceptionHelper.getRootCause(ex); - assertThat(inner, instanceOf(ResponseException.class)); - assertEquals(HttpResponseStatus.BAD_REQUEST, ((ResponseException) inner).getResponseStatusCode()); - assertTrue(ex.getMessage().contains("An error occurred during serialization of this request")); + assertThat(inner, instanceOf(IllegalArgumentException.class)); + assertTrue(ex.getMessage().contains("Parameter map contains at least one type [Color] that cannot be represented as text")); } // should not die completely just because we had a bad serialization error. that kind of stuff happens @@ -556,6 +556,39 @@ } @Test + public void shouldRejectTraversalWithUnsupportedType() { + final Cluster cluster = TestClientFactory.build().create(); + try { + final GraphTraversalSource g = traversal().with(DriverRemoteConnection.using(cluster)); + g.inject(new Thread()).toList(); + fail("Expected IllegalArgumentException for unsupported type"); + } catch (IllegalArgumentException ex) { + assertThat(ex.getMessage(), containsString("at least one type")); + assertThat(ex.getMessage(), containsString("Thread")); + assertThat(ex.getMessage(), containsString("cannot be represented as text")); + } finally { + cluster.close(); + } + } + + @Test + public void shouldRejectTraversalWithUnsupportedTypeInGValue() { + final Cluster cluster = TestClientFactory.build().create(); + try { + final GraphTraversalSource g = traversal().with(DriverRemoteConnection.using(cluster)); + g.V(GValue.of("x", new Thread())).toList(); + fail("Expected exception for unsupported type in GValue parameter"); + } catch (Exception ex) { + final Throwable inner = ExceptionHelper.getRootCause(ex); + assertThat(inner, instanceOf(IllegalArgumentException.class)); + assertThat(inner.getMessage(), containsString("at least one type")); + assertThat(inner.getMessage(), containsString("cannot be represented as text")); + } finally { + cluster.close(); + } + } + + @Test public void shouldEvalInGremlinLang() { final Cluster cluster = TestClientFactory.build().create(); final Client client = cluster.connect();
diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/handler/HttpRequestMessageDecoderTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/handler/HttpRequestMessageDecoderTest.java index 7596698..6204506 100644 --- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/handler/HttpRequestMessageDecoderTest.java +++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/handler/HttpRequestMessageDecoderTest.java
@@ -147,7 +147,7 @@ final String gremlin = "g.V(x)"; final ByteBuf buffer = allocator.buffer(); buffer.writeCharSequence("{ \"gremlin\": \"" + gremlin + - "\", \"bindings\":{\"x\":\"2\"}" + + "\", \"bindings\":\"[\\\"x\\\":\\\"2\\\"]\"" + ", \"language\": \"gremlin-groovy\"}", CharsetUtil.UTF_8); @@ -163,7 +163,7 @@ final RequestMessage decodedRequestMessage = testChannel.readInbound(); assertEquals(gremlin, decodedRequestMessage.getGremlin()); assertEquals("gremlin-groovy", decodedRequestMessage.getField(Tokens.ARGS_LANGUAGE)); - assertEquals("2", ((Map)decodedRequestMessage.getField(Tokens.ARGS_BINDINGS)).get("x")); + assertEquals("[\"x\":\"2\"]", decodedRequestMessage.getField(Tokens.ARGS_BINDINGS)); } @Test @@ -314,7 +314,7 @@ final UUID rid = UUID.randomUUID(); final ByteBuf buffer = allocator.buffer(); buffer.writeCharSequence("{\"gremlin\":\"g.V().limit(2)\",\"batchSize\":\"10\",\"language\":\"gremlin-lang\"," + - "\"g\":\"gmodern\",\"bindings\":{\"x\":\"1\"},\"timeoutMs\":\"12\"," + + "\"g\":\"gmodern\",\"bindings\":\"[\\\"x\\\":1]\",\"timeoutMs\":\"12\"," + "\"materializeProperties\":\"" + Tokens.MATERIALIZE_PROPERTIES_TOKENS + "\"}", CharsetUtil.UTF_8); final HttpHeaders headers = new DefaultHttpHeaders(); @@ -331,8 +331,7 @@ assertEquals(10, (int) decodedRequest.getField(Tokens.ARGS_BATCH_SIZE)); assertEquals("gremlin-lang", decodedRequest.getField(Tokens.ARGS_LANGUAGE)); assertEquals("gmodern", decodedRequest.getField(Tokens.ARGS_G)); - assertEquals("1", ((Map) decodedRequest.getField(Tokens.ARGS_BINDINGS)).get("x")); - assertEquals(1, ((Map) decodedRequest.getField(Tokens.ARGS_BINDINGS)).size()); + assertEquals("[\"x\":1]", decodedRequest.getField(Tokens.ARGS_BINDINGS)); assertEquals(12, (long) decodedRequest.getField(Tokens.TIMEOUT_MS)); assertEquals(Tokens.MATERIALIZE_PROPERTIES_TOKENS, decodedRequest.getField(Tokens.ARGS_MATERIALIZE_PROPERTIES)); }
diff --git a/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/message/RequestMessage.java b/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/message/RequestMessage.java index f25aa37..bae392e 100644 --- a/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/message/RequestMessage.java +++ b/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/message/RequestMessage.java
@@ -18,6 +18,7 @@ */ package org.apache.tinkerpop.gremlin.util.message; +import org.apache.tinkerpop.gremlin.process.traversal.GremlinLang; import org.apache.tinkerpop.gremlin.util.Tokens; import java.util.Collections; @@ -76,18 +77,12 @@ public static Builder from(final RequestMessage msg) { final Builder builder = build(msg.gremlin); builder.fields.putAll(msg.getFields()); - if (msg.getFields().containsKey(Tokens.ARGS_BINDINGS)) { - builder.addBindings((Map<String, Object>) msg.getFields().get(Tokens.ARGS_BINDINGS)); - } return builder; } public static Builder from(final RequestMessage msg, final String gremlin) { final Builder builder = build(gremlin); builder.fields.putAll(msg.getFields()); - if (msg.getFields().containsKey(Tokens.ARGS_BINDINGS)) { - builder.addBindings((Map<String, Object>) msg.getFields().get(Tokens.ARGS_BINDINGS)); - } return builder; } @@ -108,7 +103,6 @@ */ public static final class Builder { private final String gremlin; - private final Map<String, Object> bindings = new HashMap<>(); private final Map<String, Object> fields = new HashMap<>(); // Only allow certain items to be added to prevent breaking changes. private Builder(final String gremlin) { @@ -121,14 +115,24 @@ return this; } - public Builder addBinding(final String key, final Object val) { - bindings.put(key, val); + /** + * Sets the bindings as a gremlin-lang map literal string. Calling this method multiple + * times will replace the previous bindings (last-one-wins). + */ + public Builder addBindings(final String bindingsString) { + Objects.requireNonNull(bindingsString, "bindings argument cannot be null."); + this.fields.put(Tokens.ARGS_BINDINGS, bindingsString); return this; } + /** + * Sets the bindings from a map by converting it to a gremlin-lang map literal string + * using {@link GremlinLang#convertParametersToString(Map)}. Calling this method multiple + * times will replace the previous bindings (last-one-wins). + */ public Builder addBindings(final Map<String, Object> otherBindings) { Objects.requireNonNull(otherBindings, "bindings argument cannot be null."); - this.bindings.putAll(otherBindings); + this.fields.put(Tokens.ARGS_BINDINGS, GremlinLang.convertParametersToString(otherBindings)); return this; } @@ -183,7 +187,6 @@ * Create the request message given the settings provided to the {@link Builder}. */ public RequestMessage create() { - this.fields.put(Tokens.ARGS_BINDINGS, bindings); return new RequestMessage(gremlin, fields); } }
diff --git a/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/ser/AbstractGraphSONMessageSerializerV4.java b/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/ser/AbstractGraphSONMessageSerializerV4.java index 4fe1be6..63c3893 100644 --- a/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/ser/AbstractGraphSONMessageSerializerV4.java +++ b/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/ser/AbstractGraphSONMessageSerializerV4.java
@@ -323,7 +323,7 @@ builder.addG(data.get(SerTokens.TOKEN_G).toString()); } if (data.containsKey(SerTokens.TOKEN_BINDINGS)) { - builder.addBindings((Map<String, Object>) data.get(SerTokens.TOKEN_BINDINGS)); + builder.addBindings(data.get(SerTokens.TOKEN_BINDINGS).toString()); } if (data.containsKey(Tokens.TIMEOUT_MS)) { // Can be int for untyped JSON and long for typed GraphSON.
diff --git a/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/ser/binary/RequestMessageSerializer.java b/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/ser/binary/RequestMessageSerializer.java index 25cc6ab..2f30d43 100644 --- a/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/ser/binary/RequestMessageSerializer.java +++ b/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/ser/binary/RequestMessageSerializer.java
@@ -59,7 +59,7 @@ builder.addG(fields.get(SerTokens.TOKEN_G).toString()); } if (fields.containsKey(SerTokens.TOKEN_BINDINGS)) { - builder.addBindings((Map<String, Object>) fields.get(SerTokens.TOKEN_BINDINGS)); + builder.addBindings(fields.get(SerTokens.TOKEN_BINDINGS).toString()); } if (fields.containsKey(Tokens.TIMEOUT_MS)) { builder.addTimeoutMillis(((Number) fields.get(Tokens.TIMEOUT_MS)).longValue());
diff --git a/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/util/message/RequestMessageTest.java b/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/util/message/RequestMessageTest.java index d5577a4..1337677 100644 --- a/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/util/message/RequestMessageTest.java +++ b/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/util/message/RequestMessageTest.java
@@ -43,19 +43,42 @@ bindings.put("a", "b"); bindings.put("g", "gmodern"); final RequestMessage msg = RequestMessage.build("gremlin").addBindings(bindings).create(); - assertEquals(bindings, msg.getField(Tokens.ARGS_BINDINGS)); + // bindings are now stored as a gremlin-lang map literal string + final String bindingsString = msg.getField(Tokens.ARGS_BINDINGS); + assertTrue(bindingsString.startsWith("[")); + assertTrue(bindingsString.endsWith("]")); + assertTrue(bindingsString.contains("\"a\":\"b\"")); + assertTrue(bindingsString.contains("\"g\":\"gmodern\"")); } @Test - public void shouldSetBindingsWithKeyValue() { - final Map<String, Object> bindings = new HashMap<>(); - bindings.put("a", "b"); - bindings.put("g", "gmodern"); + public void shouldSetBindingsWithString() { final RequestMessage msg = RequestMessage.build("gremlin") - .addBinding("a", "b") - .addBinding("g", "gmodern") + .addBindings("[a:\"b\",g:\"gmodern\"]") .create(); - assertEquals(bindings, msg.getField(Tokens.ARGS_BINDINGS)); + assertEquals("[a:\"b\",g:\"gmodern\"]", msg.getField(Tokens.ARGS_BINDINGS)); + } + + @Test + public void shouldNotContainBindingsWhenNoneSet() { + final RequestMessage msg = RequestMessage.build("g.V()").create(); + assertNull(msg.getField(Tokens.ARGS_BINDINGS)); + } + + @Test + public void shouldOverwriteBindingsOnMultipleCalls() { + final RequestMessage msg = RequestMessage.build("gremlin") + .addBindings("[x:1]") + .addBindings("[y:2]") + .create(); + assertEquals("[y:2]", msg.getField(Tokens.ARGS_BINDINGS)); + } + + @Test(expected = IllegalArgumentException.class) + public void shouldRejectUnsupportedTypeInBindingsMap() { + final Map<String, Object> bindings = new HashMap<>(); + bindings.put("x", new Thread()); + RequestMessage.build("g.V()").addBindings(bindings); } @Test @@ -111,7 +134,10 @@ final Map<String, Object> fields = msg.getFields(); assertEquals(g, fields.get(Tokens.ARGS_G)); assertEquals(lang, fields.get(Tokens.ARGS_LANGUAGE)); - assertEquals(bindings, fields.get(Tokens.ARGS_BINDINGS)); + // bindings are now a gremlin-lang string, not the original map + final String bindingsString = (String) fields.get(Tokens.ARGS_BINDINGS); + assertTrue(bindingsString.contains("\"b\":\"c\"")); + assertTrue(bindingsString.contains("\"g\":\"gmodern\"")); assertEquals(query, msg.getGremlin()); }
diff --git a/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/util/ser/binary/MessageSerializerTest.java b/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/util/ser/binary/MessageSerializerTest.java index 48fa31a..e8874bf 100644 --- a/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/util/ser/binary/MessageSerializerTest.java +++ b/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/util/ser/binary/MessageSerializerTest.java
@@ -81,7 +81,7 @@ .addTimeoutMillis(500) .addG("g1") .addLanguage("some-lang") - .addBinding("k", "v") + .addBindings("['k':'v']") .create(); final ByteBuf buffer = serializer.serializeRequestAsBinary(request, allocator);
diff --git a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphTest.java b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphTest.java index a9ecce5..1ef6bc4 100644 --- a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphTest.java +++ b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphTest.java
@@ -28,6 +28,7 @@ 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.dsl.graph.__; +import org.apache.tinkerpop.gremlin.process.traversal.step.GValue; import org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.IdentityRemovalStrategy; import org.apache.tinkerpop.gremlin.process.traversal.strategy.verification.ReservedKeysVerificationStrategy; import org.apache.tinkerpop.gremlin.process.traversal.util.Metrics; @@ -923,6 +924,31 @@ assertEquals(3, g.V(100, "1000", uuid).count().next().intValue()); } + @Test + public void shouldExecuteTraversalWithUnsupportedTypeInEmbeddedMode() { + final TinkerGraph graph = TinkerGraph.open(); + final GraphTraversalSource g = traversal().withEmbedded(graph); + + // use a type that has no gremlin-lang literal representation + final Object customValue = new Object(); + final List<Object> result = g.inject(customValue).toList(); + assertEquals(1, result.size()); + assertEquals(customValue, result.get(0)); + } + + @Test + public void shouldExecuteTraversalWithUnsupportedTypeInGValueInEmbeddedMode() { + final TinkerGraph graph = TinkerGraph.open(); + final GraphTraversalSource g = traversal().withEmbedded(graph); + graph.addVertex(T.id, 1, T.label, "person"); + + // named GValue with an unsupported type as the value works in embedded mode + // because the raw object is passed through bindings to the script engine + final Object customValue = 1; + final long count = g.V(GValue.of("myId", customValue)).count().next(); + assertEquals(1L, count); + } + /** * Coerces a {@code Color} to a {@link TinkerGraph} during serialization. Demonstrates how custom serializers * can be developed that can coerce one value to another during serialization.