You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by sp...@apache.org on 2016/06/15 18:29:56 UTC
tinkerpop git commit: Re-assigned step labels as appropriate in
SubgraphStrategy. [Forced Update!]
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 {