Skip to content

Commit

Permalink
wip
Browse files Browse the repository at this point in the history
  • Loading branch information
spmallette committed Jun 28, 2022
1 parent 19add74 commit 8a7049f
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ public void apply(final Traversal.Admin<?, ?> traversal) {
// writes.
final Optional<Graph.Features.VertexFeatures> 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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,16 @@ public void applyStrategies() throws IllegalStateException {
final Iterator<TraversalStrategy<?>> 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 comment has been minimized.

Copy link
@javeme

javeme Jun 30, 2022

Contributor

Since called setGraph() for every Traversal here, we don't need to call setGraph() again at line 166?

This comment has been minimized.

Copy link
@spmallette

spmallette Jun 30, 2022

Author Contributor

i'm wasn't entirely sure so i left it there out of caution. following the code paths though, i'd say it could probably be removed though.

}, this);
}, this);
}

// don't need to re-apply strategies to "this" - leads to endless recursion in GraphComputer.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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<TraversalStrategy.VerificationStrategy> 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<Class<? extends VerificationStrategy>> applyPost() {
return Collections.singleton(ComputerVerificationStrategy.class);
}

public static AssertGraphStrategy instance() {
return INSTANCE;
}

}
}

0 comments on commit 8a7049f

Please sign in to comment.