diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/PartitionStrategy.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/PartitionStrategy.java index e0920cf3ba7..d3d6147e10c 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/PartitionStrategy.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/PartitionStrategy.java @@ -126,7 +126,7 @@ public void apply(final Traversal.Admin traversal) { // writes. final Optional vertexFeatures; if (includeMetaProperties) { - final Graph graph = TraversalHelper.getRootTraversal(traversal).getGraph().orElseThrow( + final Graph graph = traversal.getGraph().orElseThrow( () -> new IllegalStateException("PartitionStrategy does not work with anonymous Traversals when includeMetaProperties is enabled")); final Graph.Features.VertexFeatures vf = graph.features().vertex(); final boolean supportsMetaProperties = vf.supportsMetaProperties(); diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversal.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversal.java index a6b77ea7d29..4f314dcf8b6 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversal.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversal.java @@ -149,7 +149,16 @@ public void applyStrategies() throws IllegalStateException { final Iterator> strategyIterator = this.strategies.iterator(); while (strategyIterator.hasNext()) { final TraversalStrategy strategy = strategyIterator.next(); - TraversalHelper.applyTraversalRecursively(strategy::apply, this); + TraversalHelper.applyTraversalRecursively(t -> { + strategy.apply(t); + + // after the strategy is applied, it may have modified the traversal where a new traversal object + // was added. if the strategy didn't set the Graph object it could leave that new traversal in a + // state where another strategy might fail if that dependency is not satisfied + TraversalHelper.applyTraversalRecursively(i -> { + if (hasGraph) i.setGraph(this.graph); + }, this); + }, this); } // don't need to re-apply strategies to "this" - leads to endless recursion in GraphComputer. diff --git a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphTest.java b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphTest.java index a6611bd3983..5c8654f9fc5 100644 --- a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphTest.java +++ b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphTest.java @@ -28,8 +28,13 @@ import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy; import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__; +import org.apache.tinkerpop.gremlin.process.traversal.lambda.AbstractLambdaTraversal; +import org.apache.tinkerpop.gremlin.process.traversal.strategy.AbstractTraversalStrategy; +import org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.CountStrategy; import org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.IdentityRemovalStrategy; +import org.apache.tinkerpop.gremlin.process.traversal.strategy.verification.ComputerVerificationStrategy; import org.apache.tinkerpop.gremlin.process.traversal.strategy.verification.ReservedKeysVerificationStrategy; +import org.apache.tinkerpop.gremlin.process.traversal.strategy.verification.VerificationException; import org.apache.tinkerpop.gremlin.process.traversal.util.Metrics; import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalHelper; import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalMetrics; @@ -51,6 +56,7 @@ import org.apache.tinkerpop.gremlin.structure.io.gryo.GryoMapper; import org.apache.tinkerpop.gremlin.structure.io.gryo.GryoVersion; import org.apache.tinkerpop.gremlin.structure.io.gryo.GryoWriter; +import org.apache.tinkerpop.gremlin.structure.util.empty.EmptyGraph; import org.apache.tinkerpop.gremlin.util.iterator.IteratorUtils; import org.apache.tinkerpop.shaded.jackson.databind.ObjectMapper; import org.apache.tinkerpop.shaded.kryo.ClassResolver; @@ -70,6 +76,7 @@ import java.io.InputStream; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -879,6 +886,28 @@ public void apply(final Traversal.Admin traversal) { assertEquals(3, traversal().withEmbedded(graph).V().hasLabel("person").count().next().intValue()); } + /** + * Basically just trying to validate through {@link CountStrategy} that a child traversal constructed there gets + * its {@link Graph} instance set. By using {@link AssertGraphStrategy} an exception can get triggered in betweeen + * strategy applications to validate that the {@code Graph} is assigned. + */ + @Test + public void shouldSetGraphBetweenStrategyApplicationsForNewChildTraversalsConstructedByStrategies() throws Exception { + final GraphTraversalSource g = TinkerGraph.open().traversal(); + g.addV("node").property(T.id, 1).as("1") + .addV("node").property(T.id, 2).as("2") + .addV("node").property(T.id, 3).as("3") + .addV("node").property(T.id, 4).as("4") + .addE("child").from("1").to("2") + .addE("child").from("2").to("3") + .addE("child").from("4").to("3").iterate(); + + // this just needs to not throw an exception for it to pass + g.withStrategies(AssertGraphStrategy.instance()).V(3).repeat(__.inE("child").outV().simplePath()) + .until(__.or(__.inE().count().is(0), __.loops().is(P.eq(2)))) + .path().count().next(); + } + /** * Coerces a {@code Color} to a {@link TinkerGraph} during serialization. Demonstrates how custom serializers * can be developed that can coerce one value to another during serialization. @@ -990,4 +1019,31 @@ public boolean requiresVersion(final Object version) { return false; } } + + /** + * Validates that a {@link Graph} is assigned to each {@link Traversal} if it is expected. + */ + public static class AssertGraphStrategy extends AbstractTraversalStrategy implements TraversalStrategy.VerificationStrategy { + + public static final AssertGraphStrategy INSTANCE = new AssertGraphStrategy(); + + private AssertGraphStrategy() {} + + @Override + public void apply(final Traversal.Admin traversal) { + if (!(traversal instanceof AbstractLambdaTraversal) && (!traversal.getGraph().isPresent() + || traversal.getGraph().get().equals(EmptyGraph.instance()))) + throw new VerificationException("Graph object not set on Traversal", traversal); + } + + @Override + public Set> applyPost() { + return Collections.singleton(ComputerVerificationStrategy.class); + } + + public static AssertGraphStrategy instance() { + return INSTANCE; + } + + } }