RepeatEndStep has the same treatment as EndStep under ComputerAwareSt…

…ep in IdentityRemovalStrategy (#2879)

CHANGELOG.asciidoc

@@ -24,6 +24,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
 === TinkerPop 3.7.4 (NOT OFFICIALLY RELEASED YET)
 
 * Add log entry in WsAndHttpChannelizerHandler.
+* IdentityRemovalStrategy does not omit IdentityStep if only with RepeatEndStep under RepeatStep.
 
 [[release-3-7-3]]
 === TinkerPop 3.7.3 (October 23, 2024)

gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/IdentityRemovalStrategy.java

@@ -20,6 +20,7 @@
 
 import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
 import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy;
+import org.apache.tinkerpop.gremlin.process.traversal.step.branch.RepeatStep;
 import org.apache.tinkerpop.gremlin.process.traversal.step.sideEffect.IdentityStep;
 import org.apache.tinkerpop.gremlin.process.traversal.step.util.ComputerAwareStep;
 import org.apache.tinkerpop.gremlin.process.traversal.step.util.EmptyStep;
@@ -31,7 +32,16 @@
 /**
  * {@code IdentityRemovalStrategy} looks for {@link IdentityStep} instances and removes them.
  * If the identity step is labeled, its labels are added to the previous step.
- * If the identity step is labeled and it's the first step in the traversal, it stays.
+ * If the identity step is labeled, and it's the first step in the traversal, it stays.
+ * <p>
+ * Also for branch()/union() type steps an EndStep gets added which would lead to a traversal like:
+ * [UnionStep([[VertexStep(OUT,vertex), EndStep], [EndStep], [VertexStep(OUT,vertex), EndStep]])]
+ * if the identity() was removed. seems to make sense to account for that case so that the traversal gets to be:
+ * [UnionStep([[VertexStep(OUT,vertex), EndStep], [IdentityStep, EndStep], [VertexStep(OUT,vertex), EndStep]])]
+ * EndStep seems to just behave like an identity() in the above case, but perhaps it is more consistent
+ * to keep the identity() placeholder rather than a step that doesn't actually exist.
+ * Same applied to repeat() which would add RepeatEndStep, it's safe to keep RepeatStep([IdentityStep, RepeatEndStep]
+ * instead of leaving only RepeatEndStep.
  *
  * @author Marko A. Rodriguez (http://markorodriguez.com)
  * @example <pre>
@@ -58,15 +68,11 @@ public void apply(final Traversal.Admin<?, ?> traversal) {
             // moved to the previous step. if there is no previous step then this is a start of a labelled traversal
             // and is kept
             if (identityStep.getLabels().isEmpty() || !(identityStep.getPreviousStep() instanceof EmptyStep)) {
-
-                // for branch()/union() type steps an EndStep gets added which would lead to something like:
-                // [UnionStep([[VertexStep(OUT,vertex), EndStep], [EndStep], [VertexStep(OUT,vertex), EndStep]])]
-                // if the identity() was removed. seems to make sense to account for that case so that the traversal
-                // gets to be:
-                // [UnionStep([[VertexStep(OUT,vertex), EndStep], [IdentityStep, EndStep], [VertexStep(OUT,vertex), EndStep]])]
-                // EndStep seems to just behave like a identity() in the above case, but perhaps it is more consistent
-                // to keep the identity() placeholder rather than a step that doesn't actually exist
-                if (!(identityStep.getNextStep() instanceof ComputerAwareStep.EndStep && traversal.getSteps().size() == 2)) {
+                // For the EndStep and its variants, we maintain the IdentityStep if the removal would result in
+                // only the EndStep remaining under the traversal
+                final boolean isEndStep = identityStep.getNextStep() instanceof ComputerAwareStep.EndStep ||
+                                          identityStep.getNextStep() instanceof RepeatStep.RepeatEndStep;
+                if (!(isEndStep && traversal.getSteps().size() == 2)) {
                     TraversalHelper.copyLabels(identityStep, identityStep.getPreviousStep(), false);
                     traversal.removeStep(identityStep);
                 }

gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/IdentityRemovalStrategyTest.java

@@ -58,6 +58,8 @@ public static Iterable<Object[]> generateTestParameters() {
                 {__.match(__.as("a").out("knows").identity().as("b"),__.as("b").identity()).identity(), __.match(__.as("a").out("knows").as("b"),__.as("b"))},
                 {__.union(__.out().identity(), __.identity(), __.out()), __.union(__.out(), __.identity(), __.out())},
                 {__.choose(__.out().identity(), __.identity(), __.out("knows")), __.choose(__.out(), __.identity(), __.out("knows"))},
+                {__.repeat(__.identity()), __.repeat(__.identity())},
+                {__.repeat(__.out().identity()), __.repeat(__.out())},
                 {__.identity().out().identity(), __.out()},
                 {__.identity().as("a").out().identity(), __.identity().as("a").out()},
                 {__.identity().as("a").out().identity().as("b"), __.identity().as("a").out().as("b")},