Repository: tinkerpop Updated Branches: refs/heads/TINKERPOP-1139 a41971eca -> 84f2d63d7 (forced update)
Re-assigned step labels as appropriate in SubgraphStrategy. Step labels were not being re-written to replaced steps or to the subgraph filter steps and so traversals using .as('a') and the like were failing. Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/84f2d63d Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/84f2d63d Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/84f2d63d Branch: refs/heads/TINKERPOP-1139 Commit: 84f2d63d7b758c21bb22a6d771d57479fce3d06b Parents: e3c5d8e Author: Stephen Mallette <sp...@genoprime.com> Authored: Wed Jun 15 14:15:53 2016 -0400 Committer: Stephen Mallette <sp...@genoprime.com> Committed: Wed Jun 15 14:29:43 2016 -0400 ---------------------------------------------------------------------- CHANGELOG.asciidoc | 1 + .../strategy/decoration/SubgraphStrategy.java | 40 ++++++++++++++------ .../decoration/SubgraphStrategyProcessTest.java | 22 +++++++++++ 3 files changed, 51 insertions(+), 12 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/84f2d63d/CHANGELOG.asciidoc ---------------------------------------------------------------------- diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 7b5206b..a359eca 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -27,6 +27,7 @@ TinkerPop 3.1.3 (NOT OFFICIALLY RELEASED YET) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ * Avoid hamcrest conflict by using mockito-core instead of mockito-all dependency in `gremlin-test`. +* Fixed bug in `SubgraphStrategy` where step labels were not being propogated properly to new steps injected by the strategy. * Defaulted to `Edge.DEFAULT` if no edge label was supplied in GraphML. * Fixed bug in `IoGraphTest` causing IllegalArgumentException: URI is not hierarchical error for external graph implementations. * Fixed a bug where timeout functions provided to the `GremlinExecutor` were not executing in the same thread as the script evaluation. http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/84f2d63d/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/SubgraphStrategy.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/SubgraphStrategy.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/SubgraphStrategy.java index e2488fb..d328168 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/SubgraphStrategy.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/SubgraphStrategy.java @@ -34,6 +34,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.strategy.AbstractTraversal import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalHelper; import org.apache.tinkerpop.gremlin.structure.Direction; import org.apache.tinkerpop.gremlin.structure.Edge; +import org.apache.tinkerpop.gremlin.structure.Element; import org.apache.tinkerpop.gremlin.structure.Vertex; import java.util.ArrayList; @@ -86,7 +87,7 @@ public final class SubgraphStrategy extends AbstractTraversalStrategy<TraversalS vertexStepsToInsertFilterAfter.addAll(TraversalHelper.getStepsOfAssignableClass(AddVertexStartStep.class, traversal)); vertexStepsToInsertFilterAfter.addAll(graphSteps.stream().filter(GraphStep::returnsVertex).collect(Collectors.toList())); - vertexStepsToInsertFilterAfter.forEach(s -> TraversalHelper.insertAfterStep(new TraversalFilterStep<>(traversal, vertexCriterion.asAdmin().clone()), s, traversal)); + applyCriterion(vertexStepsToInsertFilterAfter, traversal, vertexCriterion.asAdmin()); } if (edgeCriterion != null) { @@ -95,7 +96,7 @@ public final class SubgraphStrategy extends AbstractTraversalStrategy<TraversalS edgeStepsToInsertFilterAfter.addAll(graphSteps.stream().filter(GraphStep::returnsEdge).collect(Collectors.toList())); edgeStepsToInsertFilterAfter.addAll(vertexSteps.stream().filter(VertexStep::returnsEdge).collect(Collectors.toList())); - edgeStepsToInsertFilterAfter.forEach(s -> TraversalHelper.insertAfterStep(new TraversalFilterStep<>(traversal, edgeCriterion.asAdmin().clone()), s, traversal)); + applyCriterion(edgeStepsToInsertFilterAfter, traversal, edgeCriterion.asAdmin()); } // explode g.V().out() to g.V().outE().inV() only if there is an edge predicate otherwise @@ -103,19 +104,19 @@ public final class SubgraphStrategy extends AbstractTraversalStrategy<TraversalS if (null == edgeCriterion) TraversalHelper.insertAfterStep(new TraversalFilterStep<>(traversal, vertexCriterion.asAdmin().clone()), s, traversal); else { - final VertexStep replacementVertexStep = new VertexStep(traversal, Edge.class, s.getDirection(), s.getEdgeLabels()); - Step intermediateFilterStep = null; - if (s.getDirection() == Direction.BOTH) - intermediateFilterStep = new EdgeOtherVertexStep(traversal); - else - intermediateFilterStep = new EdgeVertexStep(traversal, s.getDirection().opposite()); + final VertexStep someEStep = new VertexStep(traversal, Edge.class, s.getDirection(), s.getEdgeLabels()); + final Step someVStep = (s.getDirection() == Direction.BOTH) ? + new EdgeOtherVertexStep(traversal) : new EdgeVertexStep(traversal, s.getDirection().opposite()); - TraversalHelper.replaceStep(s, replacementVertexStep, traversal); - TraversalHelper.insertAfterStep(intermediateFilterStep, replacementVertexStep, traversal); - TraversalHelper.insertAfterStep(new TraversalFilterStep<>(traversal, edgeCriterion.asAdmin().clone()), replacementVertexStep, traversal); + // if s was labelled then propagate those labels to the new step that will return the vertex + transferLabels(s, someVStep); + + TraversalHelper.replaceStep(s, someEStep, traversal); + TraversalHelper.insertAfterStep(someVStep, someEStep, traversal); + TraversalHelper.insertAfterStep(new TraversalFilterStep<>(traversal, edgeCriterion.asAdmin().clone()), someEStep, traversal); if (vertexCriterion != null) - TraversalHelper.insertAfterStep(new TraversalFilterStep<>(traversal, vertexCriterion.asAdmin().clone()), intermediateFilterStep, traversal); + TraversalHelper.insertAfterStep(new TraversalFilterStep<>(traversal, vertexCriterion.asAdmin().clone()), someVStep, traversal); } }); } @@ -132,6 +133,21 @@ public final class SubgraphStrategy extends AbstractTraversalStrategy<TraversalS return new Builder(); } + private void applyCriterion(final List<Step> stepsToApplyCriterionAfter, final Traversal.Admin traversal, + final Traversal.Admin<? extends Element, ?> criterion) { + stepsToApplyCriterionAfter.forEach(s -> { + // re-assign the step label to the criterion because the label should apply seamlessly after the filter + final Step filter = new TraversalFilterStep<>(traversal, criterion.clone()); + transferLabels(s, filter); + TraversalHelper.insertAfterStep(filter, s, traversal); + }); + } + + private static void transferLabels(final Step from, final Step to) { + from.getLabels().forEach(label -> to.addLabel((String) label)); + to.getLabels().forEach(label -> from.removeLabel((String) label)); + } + public final static class Builder { private Traversal<Vertex, ?> vertexCriterion = null; http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/84f2d63d/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/SubgraphStrategyProcessTest.java ---------------------------------------------------------------------- diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/SubgraphStrategyProcessTest.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/SubgraphStrategyProcessTest.java index 11f96d0..81ea296 100644 --- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/SubgraphStrategyProcessTest.java +++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/SubgraphStrategyProcessTest.java @@ -32,11 +32,16 @@ import org.apache.tinkerpop.gremlin.structure.Vertex; import org.junit.Test; import org.junit.runner.RunWith; +import java.util.List; import java.util.NoSuchElementException; import static org.apache.tinkerpop.gremlin.LoadGraphWith.GraphData.MODERN; import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.bothE; import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.outE; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.core.IsCollectionContaining.hasItem; +import static org.hamcrest.core.IsCollectionContaining.hasItems; +import static org.hamcrest.core.IsNot.not; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -302,6 +307,23 @@ public class SubgraphStrategyProcessTest extends AbstractGremlinProcessTest { } } + @Test + @LoadGraphWith(MODERN) + @IgnoreEngine(TraversalEngine.Type.COMPUTER) + public void shouldFilterVertexCriterionAndKeepLabels() throws Exception { + // this will exclude "peter" + final Traversal<Vertex, ?> vertexCriterion = __.has("name", P.within("ripple", "josh", "marko")); + + final SubgraphStrategy strategy = SubgraphStrategy.build().vertexCriterion(vertexCriterion).create(); + final GraphTraversalSource sg = create(strategy); + + assertEquals(9, g.V().as("a").out().in().as("b").dedup("a", "b").count().next().intValue()); + assertEquals(2, sg.V().as("a").out().in().as("b").dedup("a", "b").count().next().intValue()); + + final List<Object> list = sg.V().as("a").out().in().as("b").dedup("a", "b").values("name").toList(); + assertThat(list, hasItems("marko", "josh")); + } + @Test(expected = NoSuchElementException.class) @LoadGraphWith(MODERN) public void shouldGetExcludedVertex() throws Exception {