TrimGlobalStep handles unicode characters as Ltrim and Rtrim

CHANGELOG.asciidoc

@@ -30,6 +30,7 @@ This release also includes changes from <<release-3-6-8, 3.6.8>>.
 * Deprecated public constructor for `SeedStrategy` in favor of builder pattern to be consistent with other strategies.
 * Allow specifying a customized Spark app name
 * CoinStep has a getter method for its probability field
+* Fixed so that TrimGlobalStep has the same character control handling as Ltrim and Rtrim
 
 [[release-3-7-2]]
 === TinkerPop 3.7.2 (April 8, 2024)

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

@@ -25,6 +25,8 @@
 import java.util.Collections;
 import java.util.Set;
 
+import static org.apache.tinkerpop.gremlin.process.traversal.step.util.StringLocalStep.getTrimmedString;
+
 /**
  * Reference implementation for lTrim() step, a mid-traversal step which returns a string with leading
  * whitespace removed. Null values are not processed and remain as null when returned.
@@ -49,15 +51,7 @@ protected E map(final Traverser.Admin<S> traverser) {
         }
 
         // we will pass null values to next step
-        if (null == item)
-                return null;
-
-        int i = 0;
-        while (i < ((String) item).length() && Character.isWhitespace(((String) item).charAt(i))) {
-            i++;
-        }
-
-        return (E) ((String) item).substring(i);
+        return (E) getTrimmedString((String) item, true, false);
     }
 
     @Override

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

@@ -51,18 +51,9 @@ public Set<TraverserRequirement> getRequirements() {
 
     @Override
     protected E applyStringOperation(String item) {
-        return (E) item.substring(getIdx(item));
+        return (E) getTrimmedString(item, true, false);
     }
 
     @Override
     public String getStepName() { return "lTrim(local)"; }
-
-    private int getIdx(final String str) {
-        int idx = 0;
-        while (idx < str.length() && Character.isWhitespace(str.charAt(idx))) {
-            idx++;
-        }
-        return idx;
-    }
-
 }

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

@@ -25,6 +25,8 @@
 import java.util.Collections;
 import java.util.Set;
 
+import static org.apache.tinkerpop.gremlin.process.traversal.step.util.StringLocalStep.getTrimmedString;
+
 /**
  * Reference implementation for rTrim() step, a mid-traversal step which a string with trailing
  * whitespace removed. Null values are not processed and remain as null when returned.
@@ -49,15 +51,7 @@ protected E map(final Traverser.Admin<S> traverser) {
         }
 
         // we will pass null values to next step
-        if (null == item)
-                return null;
-
-        int i = ((String) item).length() - 1;
-        while (i >= 0 && Character.isWhitespace(((String) item).charAt(i))) {
-            i--;
-        }
-
-        return (E) ((String) item).substring(0,i+1);
+        return (E) getTrimmedString((String) item, false, true);
     }
 
     @Override

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

@@ -37,18 +37,9 @@ public RTrimLocalStep(final Traversal.Admin traversal) {
 
     @Override
     protected E applyStringOperation(String item) {
-        return (E) item.substring(0,getEndIdx(item)+1);
+        return (E) getTrimmedString(item, false, true);
     }
 
     @Override
     public String getStepName() { return "rTrim(local)"; }
-
-    private int getEndIdx(final String str) {
-        int idx = str.length() - 1;
-        while (idx >= 0 && Character.isWhitespace(str.charAt(idx))) {
-            idx--;
-        }
-        return idx;
-    }
-
 }

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

@@ -25,6 +25,8 @@
 import java.util.Collections;
 import java.util.Set;
 
+import static org.apache.tinkerpop.gremlin.process.traversal.step.util.StringLocalStep.getTrimmedString;
+
 /**
  * Reference implementation for trim() step, a mid-traversal step which returns a string with leading and trailing
  * whitespace removed. Null values are not processed and remain as null when returned.
@@ -47,9 +49,7 @@ protected E map(final Traverser.Admin<S> traverser) {
             throw new IllegalArgumentException(
                     String.format("The trim() step can only take string as argument, encountered %s", item.getClass()));
         }
-
-        // we will pass null values to next step
-        return null == item? null : (E) ((String) item).trim();
+        return (E) getTrimmedString((String) item, true, true);
     }
 
     @Override

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

@@ -37,7 +37,7 @@ public TrimLocalStep(final Traversal.Admin traversal) {
 
     @Override
     protected E applyStringOperation(String item) {
-        return (E) item.trim();
+        return (E) getTrimmedString(item, true, true);
     }
 
     @Override

gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/StringLocalStep.java

@@ -72,6 +72,27 @@ public Set<TraverserRequirement> getRequirements() {
         return Collections.singleton(TraverserRequirement.OBJECT);
     }
 
+    public static String getTrimmedString(final String s, final boolean lTrim, final boolean rTrim) {
+        if (s == null) {
+            return null;
+        }
+
+        int start = 0;
+        if (lTrim) {
+            while (start < s.length() && Character.isWhitespace(s.charAt(start))) {
+                start++;
+            }
+        }
+
+        int end = s.length() - 1;
+        if (rTrim) {
+            while (end >= start && Character.isWhitespace(s.charAt(end))) {
+                end--;
+            }
+        }
+        return s.substring(start, end + 1);
+    }
+
     protected abstract E applyStringOperation(final String item);
 
     protected abstract String getStepName();

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

@@ -19,11 +19,12 @@
 Feature: Step - lTrim()
 
   @GraphComputerVerificationInjectionNotSupported
+  # This verifies both ASCII control space and ideographic space character \u3000 are property trimmed.
   Scenario: g_injectX__feature___test__nullX_lTrim
     Given the empty graph
     And the traversal of
       """
-      g.inject("  feature", " one test", null, "", " ").lTrim()
+      g.inject("  feature", " one test", null, "", " ", "　abc", "abc　", "　abc　", "　　").lTrim()
       """
     When iterated to list
     Then the result should be unordered
@@ -33,6 +34,23 @@ Feature: Step - lTrim()
       | null |
       | str[] |
       | str[] |
+      | str[abc] |
+      | str[abc　] |
+      | str[abc　] |
+      | str[] |
+
+  @GraphComputerVerificationInjectionNotSupported
+  # This verifies both ASCII control space and ideographic space character \u3000 are property trimmed.
+  Scenario: g_injectX__feature___test__nullX_lTrimXlocalX
+    Given the empty graph
+    And the traversal of
+      """
+      g.inject(["  feature  ", " one test ", null, "", " ", "　abc", "abc　", "　abc　", "　　"]).lTrim(Scope.local)
+      """
+    When iterated to list
+    Then the result should be unordered
+      | result |
+      | l[str[feature  ],str[one test ],null,str[],str[],str[abc],str[abc　],str[abc　],str[]] |
 
   @GraphComputerVerificationInjectionNotSupported
   Scenario: g_injectX__feature__X_lTrim

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

@@ -19,11 +19,12 @@
 Feature: Step - rTrim()
 
   @GraphComputerVerificationInjectionNotSupported
+  # This verifies both ASCII control space and ideographic space character \u3000 are property trimmed.
   Scenario: g_injectX__feature___test__nullX_rTrim
     Given the empty graph
     And the traversal of
       """
-      g.inject("feature  ", "one test ", null, "", " ").rTrim()
+      g.inject("feature  ", "one test ", null, "", " ", "　abc", "abc　", "　abc　", "　　").rTrim()
       """
     When iterated to list
     Then the result should be unordered
@@ -33,6 +34,23 @@ Feature: Step - rTrim()
       | null |
       | str[] |
       | str[] |
+      | str[　abc] |
+      | str[abc] |
+      | str[　abc] |
+      | str[] |
+
+  @GraphComputerVerificationInjectionNotSupported
+  # This verifies both ASCII control space and ideographic space character \u3000 are property trimmed.
+  Scenario: g_injectX__feature___test__nullX_rTrimXlocalX
+    Given the empty graph
+    And the traversal of
+      """
+      g.inject(["  feature  ", " one test ", null, "", " ", "　abc", "abc　", "　abc　", "　　"]).rTrim(Scope.local)
+      """
+    When iterated to list
+    Then the result should be unordered
+      | result |
+      | l[str[  feature],str[ one test],null,str[],str[],str[　abc],str[abc],str[　abc],str[]] |
 
   @GraphComputerVerificationInjectionNotSupported
   Scenario: g_injectX__feature__X_rTrim

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

@@ -19,11 +19,12 @@
 Feature: Step - trim()
 
   @GraphComputerVerificationInjectionNotSupported
+  # This verifies both ASCII control space and ideographic space character \u3000 are property trimmed.
   Scenario: g_injectX__feature___test__nullX_trim
     Given the empty graph
     And the traversal of
       """
-      g.inject("  feature  ", " one test ", null, "", " ").trim()
+      g.inject("  feature  ", " one test ", null, "", " ", "　abc", "abc　", "　abc　", "　　").trim()
       """
     When iterated to list
     Then the result should be unordered
@@ -33,6 +34,23 @@ Feature: Step - trim()
       | null |
       | str[] |
       | str[] |
+      | str[abc] |
+      | str[abc] |
+      | str[abc] |
+      | str[] |
+
+  @GraphComputerVerificationInjectionNotSupported
+  # This verifies both ASCII control space and ideographic space character \u3000 are property trimmed.
+  Scenario: g_injectX__feature___test__nullX_trimXlocalX
+    Given the empty graph
+    And the traversal of
+      """
+      g.inject(["  feature  ", " one test ", null, "", " ", "　abc", "abc　", "　abc　", "　　"]).trim(Scope.local)
+      """
+    When iterated to list
+    Then the result should be unordered
+      | result |
+      | l[str[feature],str[one test],null,str[],str[],str[abc],str[abc],str[abc],str[]] |
 
   @GraphComputerVerificationInjectionNotSupported
   Scenario: g_injectXListXa_bXX_trim