Skip to content

Commit 515ff5b

Browse files
committed
Update conjoin() and asString() null handling.
The handling of null values in some recently implemented Steps were inconsistent. This addresses that issue by making conjoin() behave like concat() and asString() like the other String functions.
1 parent b5f420d commit 515ff5b

11 files changed

Lines changed: 29 additions & 20 deletions

File tree

docs/src/dev/provider/gremlin-semantics.asciidoc

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -632,8 +632,9 @@ incoming list traverser as string.
632632
*Arguments:*
633633
634634
* `scope` - Determines the type of traverser it operates on. Both scopes will operate on the level of individual traversers.
635-
The `global` scope will operate on individual traverser, casting all to string. The `local` scope will behave like `global`
636-
for everything except lists, where it will cast individual elements inside the list into string and return a list of string instead.
635+
The `global` scope will operate on individual traverser, casting all (except null) to string. The `local` scope will behave like
636+
`global` for everything except lists, where it will cast individual non-null elements inside the list into string and return a
637+
list of string instead.
637638
638639
Null values from the incoming traverser are not processed and remain as null when returned.
639640
@@ -1121,9 +1122,9 @@ None
11211122
11221123
*Considerations:*
11231124
1124-
Every element in the list (including nulls) is converted to a String. The delimiter is inserted between neighboring
1125-
elements to form the final result. This step only applies to list types which means that non-iterable types (including
1126-
null) will cause exceptions to be thrown.
1125+
Every element in the list (except null) is converted to a String. Null values are ignored. The delimiter is inserted
1126+
between neighboring elements to form the final result. This step only applies to list types which means that
1127+
non-iterable types (including null) will cause exceptions to be thrown.
11271128
11281129
*Exceptions*
11291130

docs/src/reference/the-traversal.asciidoc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -782,7 +782,7 @@ link:++https://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/gre
782782
[[asString-step]]
783783
=== AsString Step
784784
785-
The `asString()`-step (*map*) returns the value of incoming traverser as strings. Null values are returned as a string value "null".
785+
The `asString()`-step (*map*) returns the value of incoming traverser as strings. Null values are returned unchanged.
786786
787787
[gremlin-groovy,modern]
788788
----
@@ -1215,7 +1215,7 @@ link:++https://tinkerpop.apache.org/docs/x.y.z/dev/provider/#concat-step++[`Sema
12151215
The `conjoin()`-step (*map*) joins together the elements in the incoming list traverser together with the provided argument
12161216
as a delimiter. The resulting `String` is added to the Traversal Stream. This step only expects list data (array or
12171217
Iterable) in the incoming traverser and will throw an `IllegalArgumentException` if any other type is encountered
1218-
(including null). However, null is allowed as an element within the list and is converted to "null".
1218+
(including null). Null values are skipped and not included in the result.
12191219
12201220
[gremlin-groovy,modern]
12211221
----

gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AsStringGlobalStep.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727

2828
/**
2929
* Reference implementation for asString() step, a mid-traversal step which returns the incoming traverser value
30-
* as a string. Null values are returned as a string value "null".
30+
* as a string. Null values are returned unchanged.
3131
*
3232
* @author David Bechberger (http://bechberger.com)
3333
* @author Yang Xia (http://github.com/xiazcy)
@@ -40,7 +40,8 @@ public AsStringGlobalStep(final Traversal.Admin traversal) {
4040

4141
@Override
4242
protected E map(final Traverser.Admin<S> traverser) {
43-
return (E) String.valueOf(traverser.get());
43+
S traverserObject = traverser.get();
44+
return (E) ((traverserObject == null) ? null : String.valueOf(traverserObject));
4445
}
4546

4647
@Override

gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AsStringLocalStep.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131

3232
/**
3333
* Reference implementation for asString() step, a mid-traversal step which returns the incoming traverser value
34-
* as a string. Null values are returned as a string value "null".
34+
* as a string. Null values are returned unchanged.
3535
*
3636
* @author David Bechberger (http://bechberger.com)
3737
* @author Yang Xia (http://github.com/xiazcy)
@@ -54,7 +54,7 @@ protected E map(final Traverser.Admin<S> traverser) {
5454
final Iterator<E> iterator = IteratorUtils.asIterator(item);
5555
while (iterator.hasNext()) {
5656
final E i = iterator.next();
57-
resList.add(String.valueOf(i));
57+
resList.add((i == null) ? null : String.valueOf(i));
5858
}
5959
return (E) resList;
6060
} else {

gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/ConjoinStep.java

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,18 +48,22 @@ public ConjoinStep(final Traversal.Admin traversal, final String delimiter) {
4848
@Override
4949
protected String map(Traverser.Admin<S> traverser) {
5050
final Collection elements = convertTraverserToCollection(traverser);
51+
if (elements.isEmpty()) { return ""; }
5152

5253
final StringBuilder joinResult = new StringBuilder();
5354

5455
for (Object elem : elements) {
55-
joinResult.append(String.valueOf(elem)).append(delimiter);
56+
if (elem != null) {
57+
joinResult.append(String.valueOf(elem)).append(delimiter);
58+
}
5659
}
5760

5861
if (joinResult.length() != 0) {
5962
joinResult.delete(joinResult.length() - delimiter.length(), joinResult.length());
63+
return joinResult.toString();
64+
} else {
65+
return null;
6066
}
61-
62-
return joinResult.toString();
6367
}
6468

6569
@Override

gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AsStringGlobalStepTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public void testReturnTypes() {
4949
assertEquals("1", __.__(Arrays.asList(1, 2)).unfold().asString().next());
5050
assertArrayEquals(new String[]{"1", "2"}, __.inject(Arrays.asList(1, 2)).unfold().asString().toList().toArray());
5151

52-
assertEquals("null", __.__(null).asString().next());
52+
assertEquals(null, __.__(null).asString().next());
5353

5454
assertEquals("[1, 2]test", __.__(Arrays.asList(1, 2)).asString().concat("test").next());
5555
assertEquals("1test", __.__(Arrays.asList(1, 2)).unfold().asString().concat("test").next());

gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AsStringLocalStepTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public void testReturnTypes() {
4646
assertEquals("1", __.__(1).asString(Scope.local).next());
4747
assertArrayEquals(new String[]{"1", "2"}, __.inject(1, 2).asString(Scope.local).toList().toArray());
4848
assertArrayEquals(new String[]{}, ((List<?>) __.__(Collections.emptyList()).asString(Scope.local).next()).toArray());
49-
assertArrayEquals(new String[]{"1", "2", "null"}, ((List<?>) __.__(Arrays.asList(1, 2, null)).asString(Scope.local).next()).toArray());
49+
assertArrayEquals(new String[]{"1", "2", null}, ((List<?>) __.__(Arrays.asList(1, 2, null)).asString(Scope.local).next()).toArray());
5050

5151
assertArrayEquals(new String[]{"1test", "2test"},
5252
__.__(Arrays.asList(1, 2)).asString(Scope.local).unfold().concat("test").toList().toArray());

gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/ConjoinStepTest.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,14 @@
2323
import org.apache.tinkerpop.gremlin.process.traversal.step.StepTest;
2424
import org.junit.Test;
2525

26+
import java.util.Arrays;
2627
import java.util.Collections;
2728
import java.util.HashSet;
2829
import java.util.List;
2930
import java.util.Set;
3031

3132
import static org.junit.Assert.assertEquals;
33+
import static org.junit.Assert.assertNull;
3234
import static org.junit.Assert.assertTrue;
3335
import static org.junit.Assert.fail;
3436

@@ -49,6 +51,7 @@ public void testReturnTypes() {
4951
assertEquals("5AA8AA10", __.__(new long[] {5L, 8L, 10L}).conjoin("AA").next());
5052
assertEquals("715", __.__(1).constant(new Long[] {7L, 15L}).conjoin("").next());
5153
assertEquals("5.5,8.0,10.1", __.__(new double[] {5.5, 8.0, 10.1}).conjoin(",").next());
54+
assertNull(__.__(Arrays.asList(null, null)).conjoin(",").next());
5255

5356
final Set<Integer> set = new HashSet<>();
5457
set.add(10); set.add(11); set.add(12);

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ Feature: Step - asString()
7272
When iterated to list
7373
Then the result should be unordered
7474
| result |
75-
| str[null] |
75+
| null |
7676
| 1 |
7777

7878
@GraphComputerVerificationInjectionNotSupported
@@ -86,7 +86,7 @@ Feature: Step - asString()
8686
When iterated to list
8787
Then the result should be unordered
8888
| result |
89-
| l[1,str[null]] |
89+
| l[1,null] |
9090

9191
Scenario: g_V_valueMapXnameX_asString
9292
Given the modern graph

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ Feature: Step - conjoin()
139139
When iterated to list
140140
Then the result should be unordered
141141
| result |
142-
| str[axyznullxyzb] |
142+
| str[axyzb] |
143143

144144
@GraphComputerVerificationInjectionNotSupported
145145
Scenario: g_injectX3_threeX_conjoinX_X

0 commit comments

Comments
 (0)