You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by co...@apache.org on 2026/06/11 00:22:26 UTC
(tinkerpop) 01/15: Align GValue support and validation across Python, .NET, and Go GLVs
This is an automated email from the ASF dual-hosted git repository.
Cole-Greer pushed a commit to branch GValueFollowupTP4
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit 6c7c780ec574cae5b27d19ed8322f1bb94a4d435
Author: Cole Greer <co...@colegreer.ca>
AuthorDate: Thu Jun 4 15:17:48 2026 -0700
Align GValue support and validation across Python, .NET, and Go GLVs
Brings the client-side GValue implementations into consistent behavior:
- Name validation: single shared predicate (non-empty; first char a Unicode
letter; remaining chars Unicode letter/digit/underscore), enforced in the
constructor (fail-fast). Mid-string underscores are now accepted in all GLVs;
no Gremlin-keyword check (reserved words fail server-side). Replaces Python's
str.isidentifier() and Go's token.IsIdentifier(), and relaxes the .NET rule.
- Duplicate-name detection now uses structural/value equality: new recursive
ValuesEqual helper in .NET GremlinLang; Go simplified to reflect.DeepEqual
(also removes a latent panic on non-comparable values); Python unchanged.
- String representation standardized on the Java "name=value" format
(.NET ToString, Python __repr__/__str__, Go String()).
- Added IsNull to the .NET GValue; removed a dead null-name branch in .NET
GremlinLang.
Tests:
- Added cross-GLV parity unit tests (mid-string underscore accepted, empty/$
rejected, Unicode accepted, accessors, null check, string representation,
and collection-valued duplicate equality).
- Corrected the stale Python integration-test assertion to expect the
'bindings' gremlin-lang string instead of a 'params' dict.
- Re-skipped the parameterized g.V(variable) integration test under
TINKERPOP-3126 (a separate, out-of-scope server-side parse limitation),
with an accurate skip reason.
---
.../src/Gremlin.Net/Process/Traversal/GValue.cs | 9 ++-
.../Gremlin.Net/Process/Traversal/GremlinLang.cs | 50 ++++++++++--
.../Process/Traversal/GremlinLangTests.cs | 93 ++++++++++++++++++++++
gremlin-go/driver/gValue.go | 29 ++++++-
gremlin-go/driver/gValue_test.go | 50 ++++++++++--
gremlin-go/driver/gremlinlang.go | 11 +--
.../python/gremlin_python/process/traversal.py | 15 ++--
.../driver/test_driver_remote_connection.py | 4 +-
.../python/tests/unit/process/test_gremlin_lang.py | 68 ++++++++++++----
9 files changed, 275 insertions(+), 54 deletions(-)
diff --git a/gremlin-dotnet/src/Gremlin.Net/Process/Traversal/GValue.cs b/gremlin-dotnet/src/Gremlin.Net/Process/Traversal/GValue.cs
index 84c4eb0402..97039fb229 100644
--- a/gremlin-dotnet/src/Gremlin.Net/Process/Traversal/GValue.cs
+++ b/gremlin-dotnet/src/Gremlin.Net/Process/Traversal/GValue.cs
@@ -72,7 +72,7 @@ namespace Gremlin.Net.Process.Traversal
for (int i = 1; i < name.Length; i++)
{
- if (!char.IsLetterOrDigit(name[i]))
+ if (!char.IsLetterOrDigit(name[i]) && name[i] != '_')
throw new ArgumentException($"Invalid parameter name [{name}].");
}
@@ -119,10 +119,15 @@ namespace Gremlin.Net.Process.Traversal
}
}
+ /// <summary>
+ /// Gets a value indicating whether the parameter value is null.
+ /// </summary>
+ public bool IsNull => Value == null;
+
/// <inheritdoc />
public override string ToString()
{
- return $"GValue({Name}, {Value})";
+ return $"{Name}={Value}";
}
}
}
diff --git a/gremlin-dotnet/src/Gremlin.Net/Process/Traversal/GremlinLang.cs b/gremlin-dotnet/src/Gremlin.Net/Process/Traversal/GremlinLang.cs
index 21115a8f8f..4767f90cfd 100644
--- a/gremlin-dotnet/src/Gremlin.Net/Process/Traversal/GremlinLang.cs
+++ b/gremlin-dotnet/src/Gremlin.Net/Process/Traversal/GremlinLang.cs
@@ -302,11 +302,6 @@ namespace Gremlin.Net.Process.Traversal
{
var key = gValue.Name;
- if (key == null)
- {
- return ArgAsString(gValue.ObjectValue);
- }
-
if (!IsValidIdentifier(key))
{
throw new ArgumentException($"Invalid parameter name [{key}].");
@@ -314,7 +309,7 @@ namespace Gremlin.Net.Process.Traversal
if (_parameters.ContainsKey(key))
{
- if (!Equals(_parameters[key], gValue.ObjectValue))
+ if (!ValuesEqual(_parameters[key], gValue.ObjectValue))
{
throw new ArgumentException($"Parameter with name [{key}] already defined.");
}
@@ -722,12 +717,53 @@ namespace Gremlin.Net.Process.Traversal
return false;
for (int i = 1; i < name.Length; i++)
{
- if (!char.IsLetterOrDigit(name[i]))
+ if (!char.IsLetterOrDigit(name[i]) && name[i] != '_')
return false;
}
return true;
}
+ private static bool ValuesEqual(object? a, object? b)
+ {
+ if (Equals(a, b)) return true;
+ if (a == null || b == null) return false;
+
+ if (a is IDictionary dictA && b is IDictionary dictB)
+ {
+ if (dictA.Count != dictB.Count) return false;
+ foreach (DictionaryEntry entry in dictA)
+ {
+ if (!dictB.Contains(entry.Key)) return false;
+ if (!ValuesEqual(entry.Value, dictB[entry.Key])) return false;
+ }
+ return true;
+ }
+
+ if (a is string || b is string) return false;
+
+ if (a is IEnumerable enumA && b is IEnumerable enumB)
+ {
+ var enumeratorA = enumA.GetEnumerator();
+ var enumeratorB = enumB.GetEnumerator();
+ try
+ {
+ while (enumeratorA.MoveNext())
+ {
+ if (!enumeratorB.MoveNext()) return false;
+ if (!ValuesEqual(enumeratorA.Current, enumeratorB.Current)) return false;
+ }
+ return !enumeratorB.MoveNext();
+ }
+ finally
+ {
+ if (enumeratorA is IDisposable dA) dA.Dispose();
+ if (enumeratorB is IDisposable dB) dB.Dispose();
+ }
+ }
+
+ return false;
+ }
+
/// <summary>
/// Creates a deep copy of this <see cref="GremlinLang" /> instance.
/// </summary>
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 b4eeade168..f7d4c5ee61 100644
--- a/gremlin-dotnet/test/Gremlin.Net.UnitTest/Process/Traversal/GremlinLangTests.cs
+++ b/gremlin-dotnet/test/Gremlin.Net.UnitTest/Process/Traversal/GremlinLangTests.cs
@@ -993,6 +993,81 @@ namespace Gremlin.Net.UnitTest.Process.Traversal
Assert.True(result.Parameters.ContainsKey("ids"));
}
+ [Fact]
+ public void GValue_mid_underscore_name_accepted()
+ {
+ var gval = new GValue<int>("a_b", 42);
+ var result = _g.Inject((object)gval).GremlinLang;
+ Assert.Equal("g.inject(a_b)", result.GetGremlin());
+ }
+
+ [Fact]
+ public void GValue_empty_name_throws_ArgumentException()
+ {
+ Assert.Throws<ArgumentException>(() => new GValue<int>("", 1));
+ }
+
+ [Fact]
+ public void GValue_mid_dollar_name_throws_ArgumentException()
+ {
+ Assert.Throws<ArgumentException>(() => new GValue<int>("a$b", 1));
+ }
+
+ [Fact]
+ public void GValue_unicode_letter_name_accepted()
+ {
+ var gval = new GValue<int>("caf\u00e9", 7);
+ var result = _g.Inject((object)gval).GremlinLang;
+ Assert.Equal("g.inject(caf\u00e9)", result.GetGremlin());
+ }
+
+ [Fact]
+ public void GValue_construction_and_accessors()
+ {
+ var gval = new GValue<string>("myName", "hello");
+ Assert.Equal("myName", gval.Name);
+ Assert.Equal("hello", gval.Value);
+ Assert.Equal("hello", gval.ObjectValue);
+ }
+
+ [Fact]
+ public void GValue_IsNull_true_when_null_value()
+ {
+ var gval = new GValue<string?>("x", null);
+ Assert.True(gval.IsNull);
+ }
+
+ [Fact]
+ public void GValue_IsNull_false_when_non_null_value()
+ {
+ var gval = new GValue<int>("x", 5);
+ Assert.False(gval.IsNull);
+ }
+
+ [Fact]
+ public void GValue_ToString_format()
+ {
+ var gval = new GValue<int>("myVar", 42);
+ Assert.Equal("myVar=42", gval.ToString());
+ }
+
+ [Fact]
+ public void GValue_duplicate_name_equal_list_values_allowed()
+ {
+ var gval1 = new GValue<List<int>>("ids", new List<int> { 1, 2, 3 });
+ var gval2 = new GValue<List<int>>("ids", new List<int> { 1, 2, 3 });
+ var result = _g.Inject((object)gval1).V(gval2).GremlinLang;
+ Assert.Equal("g.inject(ids).V(ids)", result.GetGremlin());
+ }
+
+ [Fact]
+ public void GValue_duplicate_name_different_list_values_throws()
+ {
+ var gval1 = new GValue<List<int>>("ids", new List<int> { 1, 2, 3 });
+ var gval2 = new GValue<List<int>>("ids", new List<int> { 4, 5, 6 });
+ Assert.Throws<ArgumentException>(() => _g.Inject((object)gval1).V(gval2));
+ }
+
// --- Cardinality with Map Tests ---
[Fact]
@@ -1088,5 +1163,23 @@ namespace Gremlin.Net.UnitTest.Process.Traversal
var result = GremlinLang.ConvertParametersToString(parameters);
Assert.Contains("\"name\":", result);
}
+
+ [Fact]
+ public void GValue_duplicate_name_equal_dictionary_values_allowed()
+ {
+ var gval1 = new GValue<Dictionary<string, int>>("m", new Dictionary<string, int> { { "a", 1 }, { "b", 2 } });
+ var gval2 = new GValue<Dictionary<string, int>>("m", new Dictionary<string, int> { { "a", 1 }, { "b", 2 } });
+ var result = _g.Inject((object)gval1).V(gval2).GremlinLang;
+ Assert.Equal("g.inject(m).V(m)", result.GetGremlin());
+ }
+
+ [Fact]
+ public void GValue_duplicate_name_equal_nested_collection_values_allowed()
+ {
+ var gval1 = new GValue<List<List<int>>>("n", new List<List<int>> { new List<int> { 1, 2 }, new List<int> { 3, 4 } });
+ var gval2 = new GValue<List<List<int>>>("n", new List<List<int>> { new List<int> { 1, 2 }, new List<int> { 3, 4 } });
+ var result = _g.Inject((object)gval1).V(gval2).GremlinLang;
+ Assert.Equal("g.inject(n).V(n)", result.GetGremlin());
+ }
}
}
diff --git a/gremlin-go/driver/gValue.go b/gremlin-go/driver/gValue.go
index f69ec2599c..32b260da98 100644
--- a/gremlin-go/driver/gValue.go
+++ b/gremlin-go/driver/gValue.go
@@ -21,7 +21,7 @@ package gremlingo
import (
"fmt"
- "strings"
+ "unicode"
)
// GValue is a variable or literal value that is used in a Traversal. It is composed of a key-value pair where the key
@@ -31,14 +31,32 @@ type GValue struct {
value interface{}
}
-// NewGValue creates a new GValue to be used in traversals. The GValue name cannot begin with "_".
+// NewGValue creates a new GValue to be used in traversals. The name must be non-empty, start with a
+// Unicode letter, and contain only Unicode letters, digits, or '_'. It cannot begin with "_".
func NewGValue(name string, value interface{}) GValue {
- if strings.HasPrefix(name, "_") {
+ runes := []rune(name)
+ if len(runes) > 0 && runes[0] == '_' {
panic(fmt.Sprintf("invalid GValue name '%v'. Should not start with _.", name))
}
+ if !isValidGValueName(name) {
+ panic(fmt.Sprintf("invalid GValue name '%v'.", name))
+ }
return GValue{name, value}
}
+func isValidGValueName(name string) bool {
+ runes := []rune(name)
+ if len(runes) == 0 || !unicode.IsLetter(runes[0]) {
+ return false
+ }
+ for _, r := range runes[1:] {
+ if !unicode.IsLetter(r) && !unicode.IsDigit(r) && r != '_' {
+ return false
+ }
+ }
+ return true
+}
+
// Name returns the name of the GValue.
func (gv GValue) Name() string {
return gv.name
@@ -53,3 +71,8 @@ func (gv GValue) IsNil() bool {
func (gv GValue) Value() interface{} {
return gv.value
}
+
+// String returns the string representation of the GValue in the format "name=value".
+func (gv GValue) String() string {
+ return fmt.Sprintf("%v=%v", gv.name, gv.value)
+}
diff --git a/gremlin-go/driver/gValue_test.go b/gremlin-go/driver/gValue_test.go
index 45b365262f..4f93f981b2 100644
--- a/gremlin-go/driver/gValue_test.go
+++ b/gremlin-go/driver/gValue_test.go
@@ -68,20 +68,54 @@ func TestGValue(t *testing.T) {
})
t.Run("test invalid name that starts with _", func(t *testing.T) {
- g := NewGraphTraversalSource(nil, nil)
- assert.Panics(t, func() { g.Inject(NewGValue("_ids", [2]int{1, 2})) },
- "invalid GValue name _1. Should not start with _.")
+ assert.Panics(t, func() { NewGValue("_ids", [2]int{1, 2}) },
+ "invalid GValue name _ids. Should not start with _.")
})
t.Run("test name is valid identifier", func(t *testing.T) {
- g := NewGraphTraversalSource(nil, nil)
- assert.Panics(t, func() { g.Inject(NewGValue("1a", [2]int{1, 2})) },
- "invalid parameter name '1a'")
+ assert.Panics(t, func() { NewGValue("1a", [2]int{1, 2}) },
+ "invalid GValue name '1a'.")
})
t.Run("test name is not a number", func(t *testing.T) {
+ assert.Panics(t, func() { NewGValue("1", [2]int{1, 2}) },
+ "invalid GValue name '1'.")
+ })
+
+ t.Run("test mid-string underscore name accepted", func(t *testing.T) {
+ assert.NotPanics(t, func() { NewGValue("a_b", 1) })
+ })
+
+ t.Run("test empty-string name rejected", func(t *testing.T) {
+ assert.Panics(t, func() { NewGValue("", 1) })
+ })
+
+ t.Run("test mid-string dollar sign rejected", func(t *testing.T) {
+ assert.Panics(t, func() { NewGValue("a$b", 1) })
+ })
+
+ t.Run("test unicode letter name accepted", func(t *testing.T) {
+ assert.NotPanics(t, func() { NewGValue("café", 1) })
+ })
+
+ t.Run("test IsNil returns true for nil value", func(t *testing.T) {
+ gv := NewGValue("x", nil)
+ assert.True(t, gv.IsNil())
+ })
+
+ t.Run("test String representation", func(t *testing.T) {
+ gv := NewGValue("x", 1)
+ assert.Equal(t, "x=1", gv.String())
+ })
+
+ t.Run("test Go keyword name accepted", func(t *testing.T) {
+ assert.NotPanics(t, func() { NewGValue("for", 1) })
+ })
+
+ t.Run("test distinct but equal slices allowed under same name", func(t *testing.T) {
g := NewGraphTraversalSource(nil, nil)
- assert.Panics(t, func() { g.Inject(NewGValue("1", [2]int{1, 2})) },
- "invalid parameter name '1'")
+ param1 := NewGValue("ids", []int{1, 2, 3})
+ param2 := NewGValue("ids", []int{1, 2, 3})
+ assert.NotPanics(t, func() { g.Inject(param1).V(param2) })
})
}
diff --git a/gremlin-go/driver/gremlinlang.go b/gremlin-go/driver/gremlinlang.go
index 2625cb10e6..8eb46509ab 100644
--- a/gremlin-go/driver/gremlinlang.go
+++ b/gremlin-go/driver/gremlinlang.go
@@ -22,7 +22,6 @@ package gremlingo
import (
"encoding/base64"
"fmt"
- "go/token"
"math"
"math/big"
"reflect"
@@ -246,17 +245,9 @@ func (gl *GremlinLang) argAsString(arg interface{}) (string, error) {
return v.GetGremlin("__"), nil
case GValue:
key := v.Name()
- if !token.IsIdentifier(key) {
- panic(fmt.Sprintf("invalid parameter name '%v'.", key))
- }
value := v.Value()
if val, ok := gl.parameters[key]; ok {
- if reflect.TypeOf(val).Kind() == reflect.Slice || reflect.TypeOf(value).Kind() == reflect.Slice ||
- reflect.TypeOf(val).Kind() == reflect.Map || reflect.TypeOf(value).Kind() == reflect.Map {
- if !reflect.DeepEqual(val, value) {
- panic(fmt.Sprintf("parameter with name '%v' already exists.", key))
- }
- } else if val != value {
+ if !reflect.DeepEqual(val, value) {
panic(fmt.Sprintf("parameter with name '%v' already exists.", key))
}
} else {
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 76c91822e7..e575f1e572 100644
--- a/gremlin-python/src/main/python/gremlin_python/process/traversal.py
+++ b/gremlin-python/src/main/python/gremlin_python/process/traversal.py
@@ -918,9 +918,6 @@ class GremlinLang(object):
if isinstance(arg, GValue):
key = arg.get_name()
- if not key.isidentifier():
- raise Exception(f'invalid parameter name {key}.')
-
if key in self.parameters:
if self.parameters[key] != arg.value:
raise Exception(f'parameter with name {key} already exists.')
@@ -1161,10 +1158,8 @@ class GremlinLang(object):
class GValue:
def __init__(self, name, value):
- if name is None:
- raise Exception("The parameter name cannot be None.")
- if name.startswith('_'):
- raise Exception(f'invalid GValue name {name}. Should not start with _.')
+ if not name or not name[0].isalpha() or not all(c.isalnum() or c == '_' for c in name[1:]):
+ raise Exception(f'invalid GValue name {name}.')
self.name = name
self.value = value
@@ -1177,6 +1172,12 @@ class GValue:
def get(self):
return self.value
+ def __repr__(self):
+ return f'{self.name}={self.value}'
+
+ def __str__(self):
+ return f'{self.name}={self.value}'
+
class CardinalityValue(GremlinLang):
def __init__(self, cardinality, val):
diff --git a/gremlin-python/src/main/python/tests/integration/driver/test_driver_remote_connection.py b/gremlin-python/src/main/python/tests/integration/driver/test_driver_remote_connection.py
index 604190b0c6..2891851148 100644
--- a/gremlin-python/src/main/python/tests/integration/driver/test_driver_remote_connection.py
+++ b/gremlin-python/src/main/python/tests/integration/driver/test_driver_remote_connection.py
@@ -52,7 +52,7 @@ class TestDriverRemoteConnection(object):
'bulkResults': True}
assert 6 == t.to_list()[0]
- @pytest.mark.skip(reason="investigate why 'ids' parameter name fails to parse in gremlin-lang")
+ @pytest.mark.skip(reason="TINKERPOP-3126: g.V() with a variable/parameter argument fails to parse in gremlin-lang")
def test_extract_request_options_with_params(self, remote_connection):
g = traversal().with_(remote_connection)
t = g.with_("evaluationTimeout",
@@ -62,7 +62,7 @@ class TestDriverRemoteConnection(object):
'evaluationTimeout': 1000,
'userAgent': 'test',
'bulkResults': True,
- 'params': {'ids': [1, 2, 3]}}
+ 'bindings': "['ids':[1,2,3]]"}
assert 3 == t.to_list()[0]
def test_traversals(self, remote_connection):
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 edf0515317..b3e7f4f09c 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
@@ -483,39 +483,53 @@ class TestGremlinLang(object):
assert gremlin_lang == tests[t][1]
def test_gvalue_name_cannot_be_null(self):
- g = traversal().with_(None)
try:
- g.V(GValue(None, [1, 2, 3]))
+ GValue(None, [1, 2, 3])
except Exception as ex:
- assert str(ex) == 'The parameter name cannot be None.'
+ assert str(ex) == 'invalid GValue name None.'
- def test_gvalue_name_dont_need_escaping(self):
+ def test_gvalue_name_empty_string_rejected(self):
+ try:
+ GValue('', [1, 2, 3])
+ except Exception as ex:
+ assert str(ex) == 'invalid GValue name .'
+
+ def test_gvalue_name_mid_string_dollar_rejected(self):
+ try:
+ GValue('a$b', [1, 2, 3])
+ except Exception as ex:
+ assert str(ex) == 'invalid GValue name a$b.'
+
+ def test_gvalue_name_unicode_letter_accepted(self):
g = traversal().with_(None)
+ p = GValue('café', 42)
+ gremlin = g.V(p).gremlin_lang
+ assert 'g.V(café)' == gremlin.get_gremlin()
+ assert 42 == gremlin.get_parameters().get('café')
+
+ def test_gvalue_name_dont_need_escaping(self):
try:
- g.V(GValue('\"', [1, 2, 3]))
+ GValue('"', [1, 2, 3])
except Exception as ex:
- assert str(ex) == 'invalid parameter name ".'
+ assert str(ex) == 'invalid GValue name ".'
def test_gvalue_is_not_number(self):
- g = traversal().with_(None)
try:
- g.V(GValue('1', [1, 2, 3]))
+ GValue('1', [1, 2, 3])
except Exception as ex:
- assert str(ex) == 'invalid parameter name 1.'
+ assert str(ex) == 'invalid GValue name 1.'
def test_gvalue_is_valid_identifier(self):
- g = traversal().with_(None)
try:
- g.V(GValue('1a', [1, 2, 3]))
+ GValue('1a', [1, 2, 3])
except Exception as ex:
- assert str(ex) == 'invalid parameter name 1a.'
+ assert str(ex) == 'invalid GValue name 1a.'
def test_gvalue_is_not_reserved(self):
- g = traversal().with_(None)
try:
- g.V(GValue('_1', [1, 2, 3]))
+ GValue('_1', [1, 2, 3])
except Exception as ex:
- assert str(ex) == 'invalid GValue name _1. Should not start with _.'
+ assert str(ex) == 'invalid GValue name _1.'
def test_gvalue_is_not_duplicate(self):
g = traversal().with_(None)
@@ -532,6 +546,30 @@ class TestGremlinLang(object):
assert 'g.inject(ids).V(ids)' == gremlin.get_gremlin()
assert val == gremlin.get_parameters().get('ids')
+ def test_gvalue_mid_string_underscore_accepted(self):
+ g = traversal().with_(None)
+ p = GValue('a_b', 42)
+ gremlin = g.V(p).gremlin_lang
+ assert 'g.V(a_b)' == gremlin.get_gremlin()
+ assert 42 == gremlin.get_parameters().get('a_b')
+
+ def test_gvalue_construction_and_accessors(self):
+ p = GValue('x', 1)
+ assert 'x' == p.get_name()
+ assert 1 == p.get()
+ assert p.is_null() is False
+
+ def test_gvalue_is_null(self):
+ p = GValue('n', None)
+ assert p.is_null() is True
+ q = GValue('m', 0)
+ assert q.is_null() is False
+
+ def test_gvalue_string_representation(self):
+ p = GValue('x', 1)
+ assert repr(p) == 'x=1'
+ assert str(p) == 'x=1'
+
def test_unsupported_type_throws(self):
g = traversal().with_(None)
import pytest