TINKERPOP-1869 Allowed iterate() after profile() The iterate() method adds a NoneStep that was preventing traversals like g.V().profile().iterate(). Not the typical type of traversal you would do, but it seems like it should be allowed for consistency sake.
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/374b51a3 Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/374b51a3 Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/374b51a3 Branch: refs/heads/TINKERPOP-1869 Commit: 374b51a3fcb58ee6a7601b32d80ccd1e7a680f6a Parents: 8005cb3 Author: Stephen Mallette <sp...@genoprime.com> Authored: Tue Apr 24 19:07:28 2018 -0400 Committer: Stephen Mallette <sp...@genoprime.com> Committed: Tue Apr 24 19:07:28 2018 -0400 ---------------------------------------------------------------------- CHANGELOG.asciidoc | 1 + .../StandardVerificationStrategy.java | 13 ++++--- .../StandardVerificationStrategyTest.java | 41 +++++++++++++------- 3 files changed, 36 insertions(+), 19 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/374b51a3/CHANGELOG.asciidoc ---------------------------------------------------------------------- diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index aead6ef..5265bd4 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -27,6 +27,7 @@ This release also includes changes from <<release-3-2-9, 3.2.9>>. * Coerced `BulkSet` to `g:List` in GraphSON 3.0. * Deprecated `CredentialsGraph` DSL in favor of `CredentialsTraversalDsl` which uses the recommended method for Gremlin DSL development. +* Allowed `iterate()` to be called after `profile()`. [[release-3-3-2]] === TinkerPop 3.3.2 (Release Date: April 2, 2018) http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/374b51a3/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/verification/StandardVerificationStrategy.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/verification/StandardVerificationStrategy.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/verification/StandardVerificationStrategy.java index cad3b8e..0fa7f5a 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/verification/StandardVerificationStrategy.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/verification/StandardVerificationStrategy.java @@ -24,6 +24,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.Step; 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.filter.NoneStep; import org.apache.tinkerpop.gremlin.process.traversal.step.sideEffect.ProfileSideEffectStep; import org.apache.tinkerpop.gremlin.process.traversal.step.sideEffect.SideEffectCapStep; import org.apache.tinkerpop.gremlin.process.traversal.step.util.ReducingBarrierStep; @@ -63,15 +64,17 @@ public final class StandardVerificationStrategy extends AbstractTraversalStrateg throw new VerificationException("The parent of a reducing barrier can not be repeat()-step: " + step, traversal); } - // The ProfileSideEffectStep must be the last step, 2nd last step when accompanied by the cap step, - // or 3rd to last when the traversal ends with a RequirementsStep. + // The ProfileSideEffectStep must be one of the following + // (1) the last step + // (2) 2nd last step when accompanied by the cap step or none step (i.e. iterate() to nothing) + // (3) 3rd to last when the traversal ends with a RequirementsStep. final Step<?, ?> endStep = traversal.asAdmin().getEndStep(); if (TraversalHelper.hasStepOfClass(ProfileSideEffectStep.class, traversal) && !(endStep instanceof ProfileSideEffectStep || (endStep instanceof SideEffectCapStep && endStep.getPreviousStep() instanceof ProfileSideEffectStep) || - (endStep instanceof RequirementsStep && ( - endStep.getPreviousStep() instanceof SideEffectCapStep || - endStep.getPreviousStep() instanceof ProfileSideEffectStep)))) { + (endStep instanceof NoneStep && endStep.getPreviousStep() instanceof SideEffectCapStep && endStep.getPreviousStep().getPreviousStep() instanceof ProfileSideEffectStep) || + (endStep instanceof RequirementsStep && endStep.getPreviousStep() instanceof SideEffectCapStep && endStep.getPreviousStep().getPreviousStep() instanceof ProfileSideEffectStep) || + (endStep instanceof RequirementsStep && endStep.getPreviousStep() instanceof NoneStep && endStep.getPreviousStep().getPreviousStep() instanceof SideEffectCapStep && endStep.getPreviousStep().getPreviousStep().getPreviousStep() instanceof ProfileSideEffectStep))) { throw new VerificationException("When specified, the profile()-Step must be the last step or followed only by the cap()-step and/or requirements step.", traversal); } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/374b51a3/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/verification/StandardVerificationStrategyTest.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/verification/StandardVerificationStrategyTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/verification/StandardVerificationStrategyTest.java index aa64f68..ee96266 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/verification/StandardVerificationStrategyTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/verification/StandardVerificationStrategyTest.java @@ -21,16 +21,15 @@ package org.apache.tinkerpop.gremlin.process.traversal.strategy.verification; import org.apache.tinkerpop.gremlin.process.traversal.Traversal; import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategies; import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__; -import org.apache.tinkerpop.gremlin.process.traversal.step.util.RequirementsStep; +import org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.RequirementsStrategy; +import org.apache.tinkerpop.gremlin.process.traversal.traverser.TraverserRequirement; import org.apache.tinkerpop.gremlin.process.traversal.util.DefaultTraversalStrategies; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; import java.util.Arrays; -import java.util.Collections; -import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.__; import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.out; import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.repeat; import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.sum; @@ -39,12 +38,11 @@ import static org.junit.Assert.fail; /** * @author Marko A. Rodriguez (http://markorodriguez.com) + * @author Stephen Mallette (http://stephen.genoprime.com) */ @RunWith(Parameterized.class) public class StandardVerificationStrategyTest { - private static final RequirementsStep emptyRequirementStep = new RequirementsStep<>(null, Collections.EMPTY_SET); - @Parameterized.Parameters(name = "{0}") public static Iterable<Object[]> data() { return Arrays.asList(new Object[][]{ @@ -53,10 +51,10 @@ public class StandardVerificationStrategyTest { {"__.repeat(sum()).times(2)", repeat(sum()).times(2), false}, {"__.repeat(out().count())", repeat(out().count()), false}, // traversals that should pass verification - {"__.V().profile().requirementsStep()", - __.V().profile().asAdmin().addStep(emptyRequirementStep), true}, - {"__.V().profile('metrics').cap('metrics').requirementsStep()", - __.V().profile("metrics").asAdmin().addStep(emptyRequirementStep), true} + {"__.V().profile()", + __.V().profile(), true}, + {"__.V().profile('metrics').cap('metrics')", + __.V().profile("metrics").cap("metrics"), true} }); } @@ -71,19 +69,34 @@ public class StandardVerificationStrategyTest { @Test public void shouldBeVerified() { - final TraversalStrategies strategies = new DefaultTraversalStrategies(); - strategies.addStrategies(StandardVerificationStrategy.instance()); - traversal.asAdmin().setStrategies(strategies); + final Traversal copy = copyAndConfigureTraversal(traversal); if (legalTraversal) { - traversal.asAdmin().applyStrategies(); + copy.asAdmin().applyStrategies(); + + // try to also apply strategies with iterate() so that a NoneStep is added - for consistency sake we want + // to be able to run a profile and get no result back with this. + final Traversal forIteration = copyAndConfigureTraversal(traversal); + forIteration.iterate(); } else { try { - traversal.asAdmin().applyStrategies(); + copy.asAdmin().applyStrategies(); fail("The strategy should not allow traversal: " + this.traversal); } catch (IllegalStateException ise) { assertTrue(true); } } } + + private static Traversal copyAndConfigureTraversal(final Traversal traversalToCopy) { + final Traversal copy = traversalToCopy.asAdmin().clone(); + final TraversalStrategies strategies = new DefaultTraversalStrategies(); + strategies.addStrategies(StandardVerificationStrategy.instance()); + + // just add a junk requirement + RequirementsStrategy.addRequirements(strategies, TraverserRequirement.BULK); + + copy.asAdmin().setStrategies(strategies); + return copy; + } }