CTR update asString() semantics to throw IllegalArgumentException with null inputs for casting step consistency

Author: xiazcy

CHANGELOG.asciidoc

@@ -104,6 +104,7 @@ This release also includes changes from <<release-3-7-XXX, 3.7.XXX>>.
 * Introduced step interfaces for all parameterizable steps
 * Switched `gremlin-net` byte serializers to signed bytes (`sbyte`) to be consistent with IO doc
 * Removed auto-unfold of singleton collections from `range()`, `limit()`, and `tail()` local scope steps to improve consistency of output.
+* Updated `asString()` step to throw `IllegalArgumentException` with `null` inputs for casting step consistency
 * Renamed `MergeElementStep` to `MergeElementStep` as it is a base class to `mergeV()` and `mergeE()`.
 * Renamed `MergeStep` of `merge()` to `MergeElementStep` for consistency.
 

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

@@ -1026,7 +1026,7 @@ list of string instead.
 Null values from the incoming traverser are not processed and remain as null when returned.
 
 *Exceptions*
-None
+* If the incoming traverser is a `null` value then an `IllegalArgumentException` will be thrown.
 
 See: link:https://github.com/apache/tinkerpop/tree/x.y.z/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AsStringGlobalStep.java[source],
 link:https://github.com/apache/tinkerpop/tree/x.y.z/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AsStringLocalStep.java[source (local)],

docs/src/reference/the-traversal.asciidoc

@@ -786,7 +786,7 @@ link:++https://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/gre
 [[asString-step]]
 === AsString Step
 
-The `asString()`-step (*map*) returns the value of incoming traverser as strings. Null values are returned unchanged.
+The `asString()`-step (*map*) returns the value of incoming traverser as strings. Any `null` value will cause an `IllegalArgumentException`.
 
 [gremlin-groovy,modern]
 ----

docs/src/upgrade/release-3.8.x.asciidoc

@@ -272,6 +272,11 @@ g.inject("Hello").split("")
 
 See: link:https://issues.apache.org/jira/browse/TINKERPOP-3083[TINKERPOP-3083]
 
+==== asString() No Longer Allow Nulls
+The `asString()` step will no longer allow `null` input. An `IllegalArgumentException` will be thrown for consistency with all other parsing steps (i.e. `asDate()`, `asBool()`, `asNumber()`).
+
+See: link:https://lists.apache.org/thread/q76pgrvhprosb4lty63bnsnbw2ljyl7m[DISCUSS] thread
+
 ==== Javascript Set Deserialization
 
 Starting from this version, `gremlin-javascript` will deserialize `Set` data into a ECMAScript 2015 Set. Previously,

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

@@ -41,7 +41,9 @@ public AsStringGlobalStep(final Traversal.Admin traversal) {
     @Override
     protected E map(final Traverser.Admin<S> traverser) {
         S traverserObject = traverser.get();
-        return (E) ((traverserObject == null) ? null : String.valueOf(traverserObject));
+        if (traverserObject == null)
+            throw new IllegalArgumentException("Can't parse null as String.");
+        return (E) String.valueOf(traverserObject);
     }
 
     @Override

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

@@ -47,14 +47,15 @@ protected E map(final Traverser.Admin<S> traverser) {
         final S item = traverser.get();
 
         if (null == item) {
-            // we will pass null lists to next step
-            return null;
+            throw new IllegalArgumentException("Can't parse null as String.");
         } else if ((item instanceof Iterable) || (item instanceof Iterator) || item.getClass().isArray()) {
             final List<String> resList = new ArrayList<>();
             final Iterator<E> iterator = IteratorUtils.asIterator(item);
             while (iterator.hasNext()) {
                 final E i = iterator.next();
-                resList.add((i == null) ? null : String.valueOf(i));
+                if (i == null)
+                    throw new IllegalArgumentException("Can't parse null as String.");
+                resList.add(String.valueOf(i));
             }
             return (E) resList;
         } else {

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

@@ -49,8 +49,6 @@ public void testReturnTypes() {
         assertEquals("1", __.__(Arrays.asList(1, 2)).unfold().asString().next());
         assertArrayEquals(new String[]{"1", "2"}, __.inject(Arrays.asList(1, 2)).unfold().asString().toList().toArray());
 
-        assertEquals(null, __.__(null).asString().next());
-
         assertEquals("[1, 2]test", __.__(Arrays.asList(1, 2)).asString().concat("test").next());
         assertEquals("1test", __.__(Arrays.asList(1, 2)).unfold().asString().concat("test").next());
         assertArrayEquals(new String[]{"1test", "2test"},
@@ -59,4 +57,10 @@ public void testReturnTypes() {
                 __.__(Arrays.asList(1, 2)).unfold().asString().concat("test").fold().next().toArray());
     }
 
+
+    @Test(expected = IllegalArgumentException.class)
+    public void shouldThrowExceptionWhenNullInput() {
+        __.__(null).asString().next();
+    }
+
 }

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

@@ -46,10 +46,14 @@ public void testReturnTypes() {
         assertEquals("1", __.__(1).asString(Scope.local).next());
         assertArrayEquals(new String[]{"1", "2"}, __.inject(1, 2).asString(Scope.local).toList().toArray());
         assertArrayEquals(new String[]{}, ((List<?>) __.__(Collections.emptyList()).asString(Scope.local).next()).toArray());
-        assertArrayEquals(new String[]{"1", "2", null}, ((List<?>) __.__(Arrays.asList(1, 2, null)).asString(Scope.local).next()).toArray());
 
         assertArrayEquals(new String[]{"1test", "2test"},
                 __.__(Arrays.asList(1, 2)).asString(Scope.local).unfold().concat("test").toList().toArray());
     }
 
+    @Test(expected = IllegalArgumentException.class)
+    public void shouldThrowExceptionWhenNullInput() {
+        __.__(Arrays.asList(1, 2, null)).asString(Scope.local).next();
+    }
+
 }

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

@@ -1488,7 +1488,6 @@ private static IDictionary<string, List<Func<GraphTraversalSource, IDictionary<s
                {"g_V_valuesXnameX_order_fold_toLowerXlocalX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.AddV("person").Property("name", "MARKO").Property("age", 29).As("marko").AddV("person").Property("name", "VADAS").Property("age", 27).As("vadas").AddV("software").Property("name", "LOP").Property("lang", "java").As("lop").AddV("person").Property("name", "JOSH").Property("age", 32).As("josh").AddV("software").Property("name", "RIPPLE").Property("lang", "java").As("ripple").AddV("person").Property("name", "PETER").Property("age", 35).As("peter").AddE("knows").From("marko").To("vadas").Property("weight", 0.5d).AddE("knows").From("marko").To("josh").Property("weight", 1.0d).AddE("created").From("marko").To("lop").Property("weight", 0.4d).AddE("created").From("josh").To("ripple").Property("weight", 1.0d).AddE("created").From("josh").To("lop").Property("weight", 0.4d).AddE("created").From("peter").To("lop").Property("weight", 0.2d), (g,p) =>g.V().Values<object>("name").Order().Fold().ToLower<object>(Scope.Local)}}, 
                {"g_injectXfeature_test_nullX_toUpper", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.Inject<object>("feature", "tESt", null).ToUpper()}}, 
                {"g_injectXfeature_test_nullX_toUpperXlocalX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.Inject<object>(new List<object> { "feature", "tESt", null }).ToUpper<object>(Scope.Local)}}, 
-               {"g_injectXfeature_test_nullX_asString_toUpper", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.Inject<object>("feature", "tESt", null).AsString().ToUpper()}}, 
                {"g_injectXListXa_bXX_toUpper", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.Inject<object>(new List<object> { "a", "b" }).ToUpper()}}, 
                {"g_V_valuesXnameX_toUpper", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().Values<object>("name").ToUpper()}}, 
                {"g_V_valuesXnameX_toUpperXlocalX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().Values<object>("name").ToUpper<object>(Scope.Local)}}, 

gremlin-go/driver/cucumber/gremlin.go

@@ -1458,7 +1458,6 @@ var translationMap = map[string][]func(g *gremlingo.GraphTraversalSource, p map[
     "g_V_valuesXnameX_order_fold_toLowerXlocalX": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.AddV("person").Property("name", "MARKO").Property("age", 29).As("marko").AddV("person").Property("name", "VADAS").Property("age", 27).As("vadas").AddV("software").Property("name", "LOP").Property("lang", "java").As("lop").AddV("person").Property("name", "JOSH").Property("age", 32).As("josh").AddV("software").Property("name", "RIPPLE").Property("lang", "java").As("ripple").AddV("person").Property("name", "PETER").Property("age", 35).As("peter").AddE("knows").From("marko").To("vadas").Property("weight", 0.5).AddE("knows").From("marko").To("josh").Property("weight", 1.0).AddE("created").From("marko").To("lop").Property("weight", 0.4).AddE("created").From("josh").To("ripple").Property("weight", 1.0).AddE("created").From("josh").To("lop").Property("weight", 0.4).AddE("created").From("peter").To("lop").Property("weight", 0.2)}, func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V().Values("name").Order().Fold().ToLower(gremlingo.Scope.Local)}}, 
     "g_injectXfeature_test_nullX_toUpper": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.Inject("feature", "tESt", nil).ToUpper()}}, 
     "g_injectXfeature_test_nullX_toUpperXlocalX": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.Inject([]interface{}{"feature", "tESt", nil}).ToUpper(gremlingo.Scope.Local)}}, 
-    "g_injectXfeature_test_nullX_asString_toUpper": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.Inject("feature", "tESt", nil).AsString().ToUpper()}}, 
     "g_injectXListXa_bXX_toUpper": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.Inject([]interface{}{"a", "b"}).ToUpper()}}, 
     "g_V_valuesXnameX_toUpper": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V().Values("name").ToUpper()}}, 
     "g_V_valuesXnameX_toUpperXlocalX": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V().Values("name").ToUpper(gremlingo.Scope.Local)}},