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 bb8da31059e11010de71713740769640af67b7e7 Author: Cole Greer <[email protected]> AuthorDate: Wed Jun 10 11:22:50 2026 -0700 Simplify GValue name validation across Java and all GLVs Removes the leading-underscore name reservation from Java GValue and every GLV. That rule existed to avoid collisions with auto-generated _0/_1 parameter names, a fallback that has since been removed, so it no longer adds value (and '__' is just one of ~300 reserved grammar tokens, a grammar-level limitation rather than a client concern). Also drops the stricter ^[letter][letter|digit|_]*$ identifier-pattern enforcement the non-Java drivers had added on top of Java. The only client-side name checks retained are: GValues cannot be nested, and (for the non-Java drivers, where a null name is meaningless rather than a literal-boxing case) the name cannot be null. Tests updated to assert acceptance of the previously-rejected names. Assisted-by: Kiro:claude-opus-4.8 --- .../gremlin/process/traversal/step/GValue.java | 3 -- .../gremlin/process/traversal/step/GValueTest.java | 7 ++-- .../src/Gremlin.Net/Process/Traversal/GValue.cs | 19 +--------- .../Process/Traversal/GremlinLangTests.cs | 29 +++++++++------ gremlin-go/driver/gValue.go | 25 +------------ gremlin-go/driver/gValue_test.go | 23 +++++------- .../gremlin-javascript/lib/process/gvalue.ts | 11 ++---- .../test/unit/gremlin-lang-test.js | 30 ++++++++------- .../python/gremlin_python/process/traversal.py | 4 +- .../python/tests/unit/process/test_gremlin_lang.py | 43 +++++----------------- 10 files changed, 67 insertions(+), 127 deletions(-) diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/GValue.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/GValue.java index d45f0c073a..4e7ca2b7b1 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/GValue.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/GValue.java @@ -48,9 +48,6 @@ public class GValue<V> implements Serializable, Cloneable { } private GValue(final String name, final V value) { - if (name != null && name.startsWith("_")) { - throw new IllegalArgumentException(String.format("Invalid GValue name [%s]. Should not start with _.", name)); - } if (value instanceof GValue) { throw new IllegalArgumentException("GValues cannot be nested"); } diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/GValueTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/GValueTest.java index d1b84bd902..36bc28ed36 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/GValueTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/GValueTest.java @@ -59,9 +59,10 @@ public class GValueTest { GValue.of("x", gValue); } - @Test(expected = IllegalArgumentException.class) - public void shouldRejectVariableNamesStartingWithUnderscore() { - GValue.of("_invalid", "value"); + @Test + public void shouldAcceptVariableNamesStartingWithUnderscore() { + final GValue<String> gValue = GValue.of("_valid", "value"); + assertEquals("_valid", gValue.getName()); } @Test(expected = IllegalArgumentException.class) diff --git a/gremlin-dotnet/src/Gremlin.Net/Process/Traversal/GValue.cs b/gremlin-dotnet/src/Gremlin.Net/Process/Traversal/GValue.cs index 71505fe94b..c23a65b4ca 100644 --- a/gremlin-dotnet/src/Gremlin.Net/Process/Traversal/GValue.cs +++ b/gremlin-dotnet/src/Gremlin.Net/Process/Traversal/GValue.cs @@ -52,10 +52,10 @@ namespace Gremlin.Net.Process.Traversal /// <summary> /// Initializes a new instance of the <see cref="GValue{T}" /> class. /// </summary> - /// <param name="name">The parameter name. Must be a valid identifier, not null, and not start with underscore.</param> + /// <param name="name">The parameter name. Must not be null.</param> /// <param name="value">The parameter value.</param> /// <exception cref="ArgumentNullException">Thrown when <paramref name="name" /> is null.</exception> - /// <exception cref="ArgumentException">Thrown when <paramref name="name" /> is not a valid identifier.</exception> + /// <exception cref="ArgumentException">Thrown when value is a nested GValue.</exception> public GValue(string name, T value) { if (value is IGValue) @@ -64,21 +64,6 @@ namespace Gremlin.Net.Process.Traversal if (name == null) throw new ArgumentNullException(nameof(name), "The parameter name cannot be null."); - if (name.Length == 0) - throw new ArgumentException($"Invalid parameter name [{name}]."); - - if (name[0] == '_') - throw new ArgumentException($"Invalid GValue name {name}. Should not start with _."); - - if (!char.IsLetter(name[0])) - throw new ArgumentException($"Invalid parameter name [{name}]."); - - for (int i = 1; i < name.Length; i++) - { - if (!char.IsLetterOrDigit(name[i]) && name[i] != '_') - throw new ArgumentException($"Invalid parameter name [{name}]."); - } - Name = name; Value = value; } 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 00f27abffc..8b25863110 100644 --- a/gremlin-dotnet/test/Gremlin.Net.UnitTest/Process/Traversal/GremlinLangTests.cs +++ b/gremlin-dotnet/test/Gremlin.Net.UnitTest/Process/Traversal/GremlinLangTests.cs @@ -953,27 +953,30 @@ namespace Gremlin.Net.UnitTest.Process.Traversal } [Fact] - public void GValue_special_char_name_throws_ArgumentException() + public void GValue_special_char_name_accepted() { - Assert.Throws<ArgumentException>(() => new GValue<int>("\"", 1)); + var gval = new GValue<int>("\"", 1); + Assert.Equal("\"", gval.Name); } [Fact] - public void GValue_numeric_name_throws_ArgumentException() + public void GValue_numeric_name_accepted() { - Assert.Throws<ArgumentException>(() => new GValue<int>("1", 1)); + var gval = new GValue<int>("1", 1); + Assert.Equal("1", gval.Name); } [Fact] - public void GValue_invalid_identifier_name_throws_ArgumentException() + public void GValue_digit_start_name_accepted() { - Assert.Throws<ArgumentException>(() => new GValue<int>("1a", 1)); + var gval = new GValue<int>("1a", 1); + Assert.Equal("1a", gval.Name); } [Fact] - public void GValue_underscore_name_throws_ArgumentException() + public void GValue_underscore_name_accepted() { - Assert.Throws<ArgumentException>(() => new GValue<int>("_1", 1)); + Assert.Equal("_1", new GValue<int>("_1", 1).Name); } [Fact] @@ -1008,15 +1011,17 @@ namespace Gremlin.Net.UnitTest.Process.Traversal } [Fact] - public void GValue_empty_name_throws_ArgumentException() + public void GValue_empty_name_accepted() { - Assert.Throws<ArgumentException>(() => new GValue<int>("", 1)); + var gval = new GValue<int>("", 1); + Assert.Equal("", gval.Name); } [Fact] - public void GValue_mid_dollar_name_throws_ArgumentException() + public void GValue_mid_dollar_name_accepted() { - Assert.Throws<ArgumentException>(() => new GValue<int>("a$b", 1)); + var gval = new GValue<int>("a$b", 1); + Assert.Equal("a$b", gval.Name); } [Fact] diff --git a/gremlin-go/driver/gValue.go b/gremlin-go/driver/gValue.go index 79d3fa5d16..90992cb780 100644 --- a/gremlin-go/driver/gValue.go +++ b/gremlin-go/driver/gValue.go @@ -21,7 +21,6 @@ package gremlingo import ( "fmt" - "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,35 +30,15 @@ type GValue struct { value interface{} } -// 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 "_". +// NewGValue creates a new GValue to be used in traversals. GValues cannot be nested, which is +// the only restriction imposed on the name. func NewGValue(name string, value interface{}) GValue { if _, ok := value.(GValue); ok { panic("GValues cannot be nested") } - 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 diff --git a/gremlin-go/driver/gValue_test.go b/gremlin-go/driver/gValue_test.go index 6a014ceb57..90c74d4497 100644 --- a/gremlin-go/driver/gValue_test.go +++ b/gremlin-go/driver/gValue_test.go @@ -67,31 +67,28 @@ func TestGValue(t *testing.T) { assert.Panics(t, func() { g.Inject(param1).V(param2) }, "parameter with name ids already exists.") }) - t.Run("test invalid name that starts with _", func(t *testing.T) { - assert.Panics(t, func() { NewGValue("_ids", [2]int{1, 2}) }, - "invalid GValue name _ids. Should not start with _.") + t.Run("test name starting with _ accepted", func(t *testing.T) { + assert.NotPanics(t, func() { NewGValue("_ids", [2]int{1, 2}) }) }) - t.Run("test name is valid identifier", func(t *testing.T) { - assert.Panics(t, func() { NewGValue("1a", [2]int{1, 2}) }, - "invalid GValue name '1a'.") + t.Run("test name starting with digit accepted", func(t *testing.T) { + assert.NotPanics(t, func() { NewGValue("1a", [2]int{1, 2}) }) }) - 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 numeric name accepted", func(t *testing.T) { + assert.NotPanics(t, func() { NewGValue("1", [2]int{1, 2}) }) }) 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 empty-string name accepted", func(t *testing.T) { + assert.NotPanics(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 mid-string dollar sign accepted", func(t *testing.T) { + assert.NotPanics(t, func() { NewGValue("a$b", 1) }) }) t.Run("test unicode letter name accepted", func(t *testing.T) { diff --git a/gremlin-js/gremlin-javascript/lib/process/gvalue.ts b/gremlin-js/gremlin-javascript/lib/process/gvalue.ts index bbbb4f52f5..b7e9041c3f 100644 --- a/gremlin-js/gremlin-javascript/lib/process/gvalue.ts +++ b/gremlin-js/gremlin-javascript/lib/process/gvalue.ts @@ -17,22 +17,17 @@ * under the License. */ -const NAME_PATTERN = /^[\p{L}][\p{L}\p{Nd}_]*$/u; - export class GValue<T = any> { readonly name: string; readonly value: T; constructor(name: string, value: T) { - if (typeof name !== 'string' || name.length === 0) { - throw new Error(`Invalid GValue name: '${name}' - must be a non-empty string`); - } - if (!NAME_PATTERN.test(name)) { - throw new Error(`Invalid GValue name: '${name}' - must start with a Unicode letter followed by letters, digits, or underscores`); - } if (value instanceof GValue) { throw new Error('GValues cannot be nested'); } + if (name == null) { + throw new Error('GValue name cannot be null.'); + } this.name = name; this.value = value; } 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 a983b64d03..aae6f3ad04 100644 --- a/gremlin-js/gremlin-javascript/test/unit/gremlin-lang-test.js +++ b/gremlin-js/gremlin-javascript/test/unit/gremlin-lang-test.js @@ -647,24 +647,29 @@ describe('GremlinLang', function () { assert.strictEqual(new GValue('x', 0).isNull(), false); }); - it('should reject empty name', function () { - assert.throws(() => new GValue('', 1), /Invalid GValue name/); + it('should accept empty name', function () { + const gv = new GValue('', 1); + assert.strictEqual(gv.name, ''); }); - it('should reject name with $ character', function () { - assert.throws(() => new GValue('a$b', 1), /Invalid GValue name/); + it('should accept name with $ character', function () { + const gv = new GValue('a$b', 1); + assert.strictEqual(gv.name, 'a$b'); }); - it('should reject name starting with underscore', function () { - assert.throws(() => new GValue('_x', 1), /Invalid GValue name/); + it('should accept name starting with underscore', function () { + const gv = new GValue('_x', 1); + assert.strictEqual(gv.name, '_x'); }); - it('should reject numeric name', function () { - assert.throws(() => new GValue('1', 1), /Invalid GValue name/); + it('should accept numeric name', function () { + const gv = new GValue('1', 1); + assert.strictEqual(gv.name, '1'); }); - it('should reject name starting with digit', function () { - assert.throws(() => new GValue('1a', 1), /Invalid GValue name/); + it('should accept name starting with digit', function () { + const gv = new GValue('1a', 1); + assert.strictEqual(gv.name, '1a'); }); it('should accept name with mid-string underscore', function () { @@ -726,9 +731,8 @@ describe('GremlinLang', function () { assert.deepStrictEqual(gl.getParameters().get('vid4'), 4); }); - it('should reject non-string name', function () { - assert.throws(() => new GValue(123, 'v'), /Invalid GValue name/); - assert.throws(() => new GValue(null, 'v'), /Invalid GValue name/); + it('should reject null name', function () { + assert.throws(() => new GValue(null, 'v'), /GValue name cannot be null/); }); }); }); 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 7264307d4e..fdcff78584 100644 --- a/gremlin-python/src/main/python/gremlin_python/process/traversal.py +++ b/gremlin-python/src/main/python/gremlin_python/process/traversal.py @@ -1160,8 +1160,8 @@ class GValue: def __init__(self, name, value): if isinstance(value, GValue): raise Exception('GValues cannot be nested') - 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}.') + if name is None: + raise Exception('GValue name cannot be null.') self.name = name self.value = value 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 a733093373..f319311802 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 @@ -485,20 +485,15 @@ class TestGremlinLang(object): def test_gvalue_name_cannot_be_null(self): try: GValue(None, [1, 2, 3]) + assert False, 'expected exception for null name' except Exception as ex: - assert str(ex) == 'invalid GValue name None.' + assert str(ex) == 'GValue name cannot be null.' - 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_empty_string_accepted(self): + assert GValue('', [1, 2, 3]).get_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_mid_string_dollar_accepted(self): + assert GValue('a$b', [1, 2, 3]).get_name() == 'a$b' def test_gvalue_name_unicode_letter_accepted(self): g = traversal().with_(None) @@ -507,29 +502,11 @@ class TestGremlinLang(object): assert 'g.V(café)' == gremlin.get_gremlin() assert 42 == gremlin.get_parameters().get('café') - def test_gvalue_name_dont_need_escaping(self): - try: - GValue('"', [1, 2, 3]) - except Exception as ex: - assert str(ex) == 'invalid GValue name ".' - - def test_gvalue_is_not_number(self): - try: - GValue('1', [1, 2, 3]) - except Exception as ex: - assert str(ex) == 'invalid GValue name 1.' + def test_gvalue_name_numeric_start_accepted(self): + assert GValue('1a', 42).get_name() == '1a' - def test_gvalue_is_valid_identifier(self): - try: - GValue('1a', [1, 2, 3]) - except Exception as ex: - assert str(ex) == 'invalid GValue name 1a.' - - def test_gvalue_is_not_reserved(self): - try: - GValue('_1', [1, 2, 3]) - except Exception as ex: - assert str(ex) == 'invalid GValue name _1.' + def test_gvalue_underscore_name_accepted(self): + assert GValue('_1', [1, 2, 3]).get_name() == '_1' def test_gvalue_is_not_duplicate(self): g = traversal().with_(None)
