TINKERPOP-1832 - Fix bug where TraversalHelper.replaceStep sets the steps' previousStep incorrectly. It needs to remove the replaced step before inserting the new step.
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/8bc4b36f Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/8bc4b36f Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/8bc4b36f Branch: refs/heads/TINKERPOP-1827 Commit: 8bc4b36f52d12e37b7d8f37f7d2240caa233106f Parents: ec1cbda Author: pieter <pieter.mar...@riseup.net> Authored: Wed Nov 22 10:56:43 2017 +0200 Committer: pieter <pieter.mar...@riseup.net> Committed: Wed Nov 22 10:56:43 2017 +0200 ---------------------------------------------------------------------- .../process/traversal/util/TraversalHelper.java | 5 +++-- .../process/util/TraversalHelperTest.java | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/8bc4b36f/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/TraversalHelper.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/TraversalHelper.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/TraversalHelper.java index fd803e4..eda836a 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/TraversalHelper.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/TraversalHelper.java @@ -173,8 +173,9 @@ public final class TraversalHelper { * @param traversal the traversal on which the action will occur */ public static <S, E> void replaceStep(final Step<S, E> removeStep, final Step<S, E> insertStep, final Traversal.Admin<?, ?> traversal) { - traversal.addStep(stepIndex(removeStep, traversal), insertStep); - traversal.removeStep(removeStep); + final int i; + traversal.removeStep(i = stepIndex(removeStep, traversal)); + traversal.addStep(i, insertStep); } public static <S, E> Step<?, E> insertTraversal(final Step<?, S> previousStep, final Traversal.Admin<S, E> insertTraversal, final Traversal.Admin<?, ?> traversal) { http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/8bc4b36f/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/util/TraversalHelperTest.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/util/TraversalHelperTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/util/TraversalHelperTest.java index 23eaa39..df22cd2 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/util/TraversalHelperTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/util/TraversalHelperTest.java @@ -32,6 +32,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.step.filter.LambdaFilterSt import org.apache.tinkerpop.gremlin.process.traversal.step.filter.PathFilterStep; import org.apache.tinkerpop.gremlin.process.traversal.step.filter.TraversalFilterStep; import org.apache.tinkerpop.gremlin.process.traversal.step.filter.WhereTraversalStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.filter.NotStep; import org.apache.tinkerpop.gremlin.process.traversal.step.map.FlatMapStep; import org.apache.tinkerpop.gremlin.process.traversal.step.map.PropertiesStep; import org.apache.tinkerpop.gremlin.process.traversal.step.map.TraversalFlatMapStep; @@ -47,6 +48,8 @@ import org.apache.tinkerpop.gremlin.structure.util.empty.EmptyGraph; import org.junit.Test; import org.mockito.Mockito; +import java.util.List; +import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; @@ -66,6 +69,22 @@ import static org.junit.Assert.assertTrue; public class TraversalHelperTest { @Test + public void shouldSetPreviousStepToEmptyStep() { + final Traversal.Admin<?, ?> traversal = __.V().out().asAdmin(); + //transform the traversal to __.V().not(out()) + //the VertexStep's previousStep should be the EmptyStep + Optional<VertexStep> vertexStepOpt = TraversalHelper.getFirstStepOfAssignableClass(VertexStep.class, traversal); + assertTrue(vertexStepOpt.isPresent()); + Traversal.Admin<?,?> inner = __.start().asAdmin(); + inner.addStep(0, vertexStepOpt.get()); + TraversalHelper.replaceStep(vertexStepOpt.get(), new NotStep<>(__.identity().asAdmin(), inner), traversal); + List<VertexStep> vertexSteps = TraversalHelper.getStepsOfAssignableClassRecursively(VertexStep.class, traversal); + assertEquals(1, vertexSteps.size()); + VertexStep vertexStep = vertexSteps.get(0); + assertTrue("Expected the previousStep to be an EmptyStep, found instead " + vertexStep.getPreviousStep().toString(),vertexStep.getPreviousStep() == EmptyStep.instance()); + } + + @Test public void shouldIdentifyLocalChildren() { final Traversal.Admin<?, ?> localChild = __.as("x").select("a", "b").by("name").asAdmin(); new LocalStep<>(new DefaultTraversal(), localChild);