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-1274
Commit: 84f2d63d7b758c21bb22a6d771d57479fce3d06b
Parents: e3c5d8e
Author: Stephen Mallette <[email protected]>
Authored: Wed Jun 15 14:15:53 2016 -0400
Committer: Stephen Mallette <[email protected]>
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 {
