Skip to content

Commit 352312a

Browse files
spmallettespmallette
committed
Add splitByElement() to Gherkin step definitions for nested collection parsing
Naive split(",") breaks on nested collection tokens like l[1,2,3] inside s[l[1,2,3],l[4,5,6]]. Adds a bracket-depth-aware splitByElement() helper to all Gherkin step definition files across Java and every GLV, so that commas inside nested brackets are not treated as top-level separators. Also adds two new Gherkin scenarios to Fold.feature that exercise nested list-of-list results (g.inject([1,2],[3,4]).fold()), along with the corresponding traversal bindings in each GLV's translation map. Java implementation follows PR #3216. (tinkerpop-mxr, tinkerpop-32y, tinkerpop-ghp, tinkerpop-2ur, tinkerpop-70k, tinkerpop-188, tinkerpop-93a)
1 parent 41a9008 commit 352312a

10 files changed

Lines changed: 184 additions & 22 deletions

File tree

‎gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/CommonSteps.cs‎

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -492,7 +492,39 @@ private static object ToNumber(string stringNumber, string graphName)
492492
{
493493
return new List<object?>(0);
494494
}
495-
return stringList.Split(',').Select(x => ParseValue(x, graphName)).ToList();
495+
return SplitByElement(stringList).Select(x => ParseValue(x, graphName)).ToList();
496+
}
497+
498+
private static List<string> SplitByElement(string s)
499+
{
500+
var result = new List<string>();
501+
var depth = 0;
502+
var current = new System.Text.StringBuilder();
503+
foreach (var c in s)
504+
{
505+
if (c == '[')
506+
{
507+
depth++;
508+
current.Append(c);
509+
}
510+
else if (c == ']')
511+
{
512+
depth--;
513+
current.Append(c);
514+
}
515+
else if (c == ',' && depth == 0)
516+
{
517+
result.Add(current.ToString().Trim());
518+
current.Clear();
519+
}
520+
else
521+
{
522+
current.Append(c);
523+
}
524+
}
525+
if (current.Length > 0)
526+
result.Add(current.ToString().Trim());
527+
return result;
496528
}
497529

498530
private static object ToDateTime(string date, string graphName)

‎gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/Gremlin.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1187,6 +1187,8 @@ private static IDictionary<string, List<Func<GraphTraversalSource, IDictionary<s
11871187
{"g_V_age_foldX0_plusX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().Values<object>("age").Fold<object>(0, Operator.Sum)}},
11881188
{"g_injectXa1_b2X_foldXm_addAllX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.Inject<object>(new Dictionary<object, object> {{ "a", 1 }}, new Dictionary<object, object> {{ "b", 2 }}).Fold<object>(new Dictionary<object, object> {}, Operator.AddAll)}},
11891189
{"g_injectXa1_b2_b4X_foldXm_addAllX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.Inject<object>(new Dictionary<object, object> {{ "a", 1 }}, new Dictionary<object, object> {{ "b", 2 }}, new Dictionary<object, object> {{ "b", 4 }}).Fold<object>(new Dictionary<object, object> {}, Operator.AddAll)}},
1190+
{"g_injectXlist1_list2X_fold", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.Inject<object>(new List<object> { 1, 2 }, new List<object> { 3, 4 }).Fold()}},
1191+
{"g_injectXlist1_list2_list3X_fold", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.Inject<object>(new List<object> { 1, 2 }, new List<object> { 3, 4 }, new List<object> { 5, 6 }).Fold()}},
11901192
{"g_VX1X_formatXstrX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().Has("name", "marko").Format("Hello world")}},
11911193
{"g_V_formatXstrX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().Format("%{name} is %{age} years old")}},
11921194
{"g_injectX1X_asXageX_V_formatXstrX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.Inject<object>(1).As("age").V().Format("%{name} is %{age} years old")}},

‎gremlin-go/driver/cucumber/cucumberSteps_test.go‎

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -256,14 +256,41 @@ func toPath(stringObjects, graphName string) interface{} {
256256
}
257257
}
258258

259+
// splitByElement splits a string on commas while respecting bracket nesting depth,
260+
// so that nested tokens like l[1,2,3] inside s[l[1,2,3],l[4,5,6]] are not split incorrectly.
261+
func splitByElement(s string) []string {
262+
var result []string
263+
depth := 0
264+
current := strings.Builder{}
265+
for _, c := range s {
266+
switch {
267+
case c == '[':
268+
depth++
269+
current.WriteRune(c)
270+
case c == ']':
271+
depth--
272+
current.WriteRune(c)
273+
case c == ',' && depth == 0:
274+
result = append(result, strings.TrimSpace(current.String()))
275+
current.Reset()
276+
default:
277+
current.WriteRune(c)
278+
}
279+
}
280+
if current.Len() > 0 {
281+
result = append(result, strings.TrimSpace(current.String()))
282+
}
283+
return result
284+
}
285+
259286
// Parse list.
260287
func toList(stringList, graphName string) interface{} {
261288
listVal := make([]interface{}, 0)
262289
if len(stringList) == 0 {
263290
return listVal
264291
}
265292

266-
for _, str := range strings.Split(stringList, ",") {
293+
for _, str := range splitByElement(stringList) {
267294
listVal = append(listVal, parseValue(str, graphName))
268295
}
269296
return listVal
@@ -275,7 +302,7 @@ func toSet(stringSet, graphName string) interface{} {
275302
if len(stringSet) == 0 {
276303
return setVal
277304
}
278-
for _, str := range strings.Split(stringSet, ",") {
305+
for _, str := range splitByElement(stringSet) {
279306
setVal.Add(parseValue(str, graphName))
280307
}
281308
return setVal

‎gremlin-go/driver/cucumber/gremlin.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1157,6 +1157,8 @@ var translationMap = map[string][]func(g *gremlingo.GraphTraversalSource, p map[
11571157
"g_V_age_foldX0_plusX": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V().Values("age").Fold(0, gremlingo.Operator.Sum)}},
11581158
"g_injectXa1_b2X_foldXm_addAllX": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.Inject(map[interface{}]interface{}{"a": 1 }, map[interface{}]interface{}{"b": 2 }).Fold(map[interface{}]interface{}{ }, gremlingo.Operator.AddAll)}},
11591159
"g_injectXa1_b2_b4X_foldXm_addAllX": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.Inject(map[interface{}]interface{}{"a": 1 }, map[interface{}]interface{}{"b": 2 }, map[interface{}]interface{}{"b": 4 }).Fold(map[interface{}]interface{}{ }, gremlingo.Operator.AddAll)}},
1160+
"g_injectXlist1_list2X_fold": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.Inject([]interface{}{1, 2}, []interface{}{3, 4}).Fold()}},
1161+
"g_injectXlist1_list2_list3X_fold": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.Inject([]interface{}{1, 2}, []interface{}{3, 4}, []interface{}{5, 6}).Fold()}},
11601162
"g_VX1X_formatXstrX": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V().Has("name", "marko").Format("Hello world")}},
11611163
"g_V_formatXstrX": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V().Format("%{name} is %{age} years old")}},
11621164
"g_injectX1X_asXageX_V_formatXstrX": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.Inject(1).As("age").V().Format("%{name} is %{age} years old")}},

‎gremlin-js/gremlin-javascript/test/cucumber/feature-steps.js‎

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -448,22 +448,47 @@ function toMerge(value) {
448448
return merge[value];
449449
}
450450

451+
function splitByElement(s) {
452+
let depth = 0;
453+
let current = '';
454+
const results = [];
455+
for (const c of s) {
456+
if (c === '[') {
457+
depth++;
458+
current += c;
459+
} else if (c === ']') {
460+
depth--;
461+
current += c;
462+
} else if (c === ',' && depth === 0) {
463+
results.push(current.trim());
464+
current = '';
465+
} else {
466+
current += c;
467+
}
468+
}
469+
if (current.length > 0) results.push(current.trim());
470+
return results;
471+
}
472+
451473
function toArray(stringList) {
452474
if (stringList === '') {
453475
return new Array(0);
454476
}
455-
return stringList.split(',').map(x => parseValue.call(this, x));
456-
}
457-
458-
function toMap(stringMap) {
459-
return parseMapValue.call(this, JSON.parse(stringMap));
477+
return splitByElement(stringList).map(x => parseValue.call(this, x));
460478
}
461479

462-
function toSet(stringSet) {
463-
if (stringSet === '') {
480+
function toSet(stringList) {
481+
if (stringList === '') {
464482
return new Set();
465483
}
466-
return new Set(stringSet.split(',').map(x => parseValue.call(this, x)));
484+
485+
const s = new Set();
486+
splitByElement(stringList).forEach(x => s.add(parseValue.call(this, x)));
487+
return s;
488+
}
489+
490+
function toMap(stringMap) {
491+
return parseMapValue.call(this, JSON.parse(stringMap));
467492
}
468493

469494
function parseMapValue(value) {

‎gremlin-js/gremlin-javascript/test/cucumber/gremlin.js‎

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎gremlin-python/src/main/python/tests/feature/feature_steps.py‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,27 @@ def unsupported_scenario(step, file):
324324
return
325325

326326

327+
def _split_by_element(s):
328+
depth = 0
329+
current = []
330+
results = []
331+
for c in s:
332+
if c == '[':
333+
depth += 1
334+
current.append(c)
335+
elif c == ']':
336+
depth -= 1
337+
current.append(c)
338+
elif c == ',' and depth == 0:
339+
results.append(''.join(current).strip())
340+
current = []
341+
else:
342+
current.append(c)
343+
if current:
344+
results.append(''.join(current).strip())
345+
return results
346+
347+
327348
def _convert(val, ctx):
328349
graph_name = ctx.graph_name
329350
if isinstance(val, dict): # convert dictionary keys/values
@@ -334,9 +355,9 @@ def _convert(val, ctx):
334355
n[tuple(k) if isinstance(k, (set, list)) else k] = _convert(value, ctx)
335356
return n
336357
elif isinstance(val, str) and re.match(r"^l\[.*\]$", val): # parse list
337-
return [] if val == "l[]" else list(map((lambda x: _convert(x, ctx)), val[2:-1].split(",")))
358+
return [] if val == "l[]" else list(map((lambda x: _convert(x, ctx)), _split_by_element(val[2:-1])))
338359
elif isinstance(val, str) and re.match(r"^s\[.*\]$", val): # parse set
339-
return set() if val == "s[]" else set(map((lambda x: _convert(x, ctx)), val[2:-1].split(",")))
360+
return set() if val == "s[]" else set(map((lambda x: _convert(x, ctx)), _split_by_element(val[2:-1])))
340361
elif isinstance(val, str) and re.match(r"^str\[.*\]$", val): # return string as is
341362
return val[4:-1]
342363
elif isinstance(val, str) and re.match(r"^dt\[.*\]$", val): # parse datetime

‎gremlin-python/src/main/python/tests/feature/gremlin.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1160,6 +1160,8 @@
11601160
'g_V_age_foldX0_plusX': [(lambda g:g.V().values('age').fold(0, Operator.sum_))],
11611161
'g_injectXa1_b2X_foldXm_addAllX': [(lambda g:g.inject({ 'a': 1 }, { 'b': 2 }).fold({ }, Operator.add_all))],
11621162
'g_injectXa1_b2_b4X_foldXm_addAllX': [(lambda g:g.inject({ 'a': 1 }, { 'b': 2 }, { 'b': 4 }).fold({ }, Operator.add_all))],
1163+
'g_injectXlist1_list2X_fold': [(lambda g:g.inject([1, 2], [3, 4]).fold())],
1164+
'g_injectXlist1_list2_list3X_fold': [(lambda g:g.inject([1, 2], [3, 4], [5, 6]).fold())],
11631165
'g_VX1X_formatXstrX': [(lambda g:g.V().has('name', 'marko').format_('Hello world'))],
11641166
'g_V_formatXstrX': [(lambda g:g.V().format_('%{name} is %{age} years old'))],
11651167
'g_injectX1X_asXageX_V_formatXstrX': [(lambda g:g.inject(1).as_('age').V().format_('%{name} is %{age} years old'))],

‎gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/features/StepDefinition.java‎

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,8 @@ public final class StepDefinition {
135135
}));
136136
add(Pair.with(Pattern.compile("l\\[\\]"), s -> "[]"));
137137
add(Pair.with(Pattern.compile("l\\[(.*)\\]"), s -> {
138-
final String[] items = s.split(",");
139-
final String listItems = Stream.of(items).map(String::trim).map(x -> convertToString(x)).collect(Collectors.joining(","));
138+
final List<String> items = splitByElement(s);
139+
final String listItems = items.stream().map(String::trim).map(x -> convertToString(x)).collect(Collectors.joining(","));
140140
return String.format("[%s]", listItems);
141141
}));
142142
add(Pair.with(Pattern.compile("s\\[\\]"), s -> "{}"));
@@ -147,8 +147,8 @@ public final class StepDefinition {
147147
}));
148148
add(Pair.with(Pattern.compile("s\\[\\]"), s -> String.format("{}")));
149149
add(Pair.with(Pattern.compile("s\\[(.*)\\]"), s -> {
150-
final String[] items = s.split(",");
151-
final String listItems = Stream.of(items).map(String::trim).map(x -> convertToString(x)).collect(Collectors.joining(","));
150+
final List<String> items = splitByElement(s);
151+
final String listItems = items.stream().map(String::trim).map(x -> convertToString(x)).collect(Collectors.joining(","));
152152
return String.format("{%s}", listItems);
153153
}));
154154
add(Pair.with(Pattern.compile("d\\[(NaN)\\]"), s -> "NaN"));
@@ -191,14 +191,14 @@ public final class StepDefinition {
191191

192192
add(Pair.with(Pattern.compile("l\\[\\]"), s -> new ArrayList<>()));
193193
add(Pair.with(Pattern.compile("l\\[(.*)\\]"), s -> {
194-
final String[] items = s.split(",");
195-
return Stream.of(items).map(String::trim).map(x -> convertToObject(x)).collect(Collectors.toList());
194+
final List<String> items = splitByElement(s);
195+
return items.stream().map(String::trim).map(x -> convertToObject(x)).collect(Collectors.toList());
196196
}));
197197

198198
add(Pair.with(Pattern.compile("s\\[\\]"), s -> new HashSet<>()));
199199
add(Pair.with(Pattern.compile("s\\[(.*)\\]"), s -> {
200-
final String[] items = s.split(",");
201-
return Stream.of(items).map(String::trim).map(x -> convertToObject(x)).collect(Collectors.toSet());
200+
final List<String> items = splitByElement(s);
201+
return items.stream().map(String::trim).map(x -> convertToObject(x)).collect(Collectors.toSet());
202202
}));
203203

204204
// return the string values as is, used to wrap results that may contain other regex patterns
@@ -713,6 +713,33 @@ private Object convertToObject(final Object pvalue) {
713713
return String.format("%s", v);
714714
}
715715

716+
/**
717+
* Splits a string on commas while respecting bracket nesting, so that nested collection tokens
718+
* like {@code l[1,2,3]} inside a set {@code s[l[1,2,3],l[4,5,6]]} are not incorrectly split.
719+
*/
720+
private static List<String> splitByElement(final String s) {
721+
final List<String> result = new ArrayList<>();
722+
int depth = 0;
723+
final StringBuilder current = new StringBuilder();
724+
for (int i = 0; i < s.length(); i++) {
725+
final char c = s.charAt(i);
726+
if (c == '[') {
727+
depth++;
728+
current.append(c);
729+
} else if (c == ']') {
730+
depth--;
731+
current.append(c);
732+
} else if (c == ',' && depth == 0) {
733+
result.add(current.toString());
734+
current.setLength(0);
735+
} else {
736+
current.append(c);
737+
}
738+
}
739+
if (current.length() > 0) result.add(current.toString());
740+
return result;
741+
}
742+
716743
private static Triplet<String,String,String> getEdgeTriplet(final String e) {
717744
final Matcher m = edgeTripletPattern.matcher(e);
718745
if (m.matches()) {

‎gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/map/Fold.feature‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,4 +81,26 @@ Feature: Step - fold()
8181
When iterated to list
8282
Then the result should be unordered
8383
| result |
84-
| m[{"a":"d[1].i", "b":"d[4].i"}] |
84+
| m[{"a":"d[1].i", "b":"d[4].i"}] |
85+
86+
Scenario: g_injectXlist1_list2X_fold
87+
Given the empty graph
88+
And the traversal of
89+
"""
90+
g.inject([1, 2], [3, 4]).fold()
91+
"""
92+
When iterated to list
93+
Then the result should be unordered
94+
| result |
95+
| l[l[d[1].i,d[2].i],l[d[3].i,d[4].i]] |
96+
97+
Scenario: g_injectXlist1_list2_list3X_fold
98+
Given the empty graph
99+
And the traversal of
100+
"""
101+
g.inject([1, 2], [3, 4], [5, 6]).fold()
102+
"""
103+
When iterated to list
104+
Then the result should be unordered
105+
| result |
106+
| l[l[d[1].i,d[2].i],l[d[3].i,d[4].i],l[d[5].i,d[6].i]] |

0 commit comments

Comments
 (0)