Fixed an NPE around path and nested children with path processors. Added some TODO notes in PrunePathStrategyTest about why keepLabels are the way they are -- I believe that nesting is not sound and should be looked into more. And more tests added.
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/b45a957c Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/b45a957c Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/b45a957c Branch: refs/heads/TINKERPOP-1278 Commit: b45a957c52da9d4e3c180baaf7f16672e94d6abd Parents: 9d205f8 Author: Marko A. Rodriguez <okramma...@gmail.com> Authored: Fri Jul 8 16:56:48 2016 -0600 Committer: Marko A. Rodriguez <okramma...@gmail.com> Committed: Fri Jul 8 16:56:48 2016 -0600 ---------------------------------------------------------------------- .../strategy/optimization/PrunePathStrategy.java | 2 +- .../strategy/optimization/PrunePathStrategyTest.java | 11 ++++++++--- 2 files changed, 9 insertions(+), 4 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/b45a957c/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PrunePathStrategy.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PrunePathStrategy.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PrunePathStrategy.java index 7d6650f..358af7a 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PrunePathStrategy.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PrunePathStrategy.java @@ -90,7 +90,7 @@ public final class PrunePathStrategy extends AbstractTraversalStrategy<Traversal do { // if this is the top level traversal, propagate all nested labels // to previous PathProcess steps - if (step instanceof PathProcessor) { + if (step instanceof PathProcessor && null != ((PathProcessor) step).getKeepLabels()) { ((PathProcessor) step).getKeepLabels().addAll(referencedLabels); } step = step.getPreviousStep(); http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/b45a957c/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PrunePathStrategyTest.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PrunePathStrategyTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PrunePathStrategyTest.java index 9749971..72ee294 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PrunePathStrategyTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PrunePathStrategyTest.java @@ -73,7 +73,7 @@ public class PrunePathStrategyTest { currentTraversal.setStrategies(currentStrategies); currentTraversal.applyStrategies(); final List<Object> keepLabels = getKeepLabels(currentTraversal); - assertEquals(this.labels, keepLabels); + assertEquals(keepLabels, this.labels); if (null != optimized) assertEquals(currentTraversal, optimized); } @@ -84,9 +84,8 @@ public class PrunePathStrategyTest { for (Step step : traversal.getSteps()) { if (step instanceof PathProcessor) { final Set<String> keepers = ((PathProcessor) step).getKeepLabels(); - if (keepers != null) { + if (keepers != null) keepLabels.add(keepers); - } } if (step instanceof TraversalParent) { final TraversalParent parent = (TraversalParent) step; @@ -135,6 +134,12 @@ public class PrunePathStrategyTest { {__.V().as("a").out().as("b").where(P.gt("a")).out().out(), Arrays.asList(Collections.emptySet()), __.V().as("a").out().as("b").where(P.gt("a")).barrier(MAX_BARRIER_SIZE).out().out()}, {__.V().as("a").out().as("b").where(P.gt("a")).count(), Arrays.asList(Collections.emptySet()), __.V().as("a").out().as("b").where(P.gt("a")).count()}, {__.V().as("a").out().as("b").select("a").as("c").where(P.gt("b")).out(), Arrays.asList(Collections.singleton("b"), Collections.emptySet()), __.V().as("a").out().as("b").select("a").as("c").barrier(MAX_BARRIER_SIZE).where(P.gt("b")).barrier(MAX_BARRIER_SIZE).out()}, + // TODO: why is the local child preserving c and e? + {__.V().as("a").out().as("b").select("a").select("b").local(as("c").out().as("d", "e").select("c", "e").out().select("c")).out().select("c"), + Arrays.asList(new HashSet<>(Arrays.asList("b", "c", "e")), new HashSet<>(Arrays.asList("c", "e")), Arrays.asList(new HashSet<>(Arrays.asList("c", "e")), new HashSet<>(Arrays.asList("c", "e"))), Collections.emptySet()), null}, + // TODO: same as above but note how path() makes things react + {__.V().as("a").out().as("b").select("a").select("b").path().local(as("c").out().as("d", "e").select("c", "e").out().select("c")).out().select("c"), + Arrays.asList(Arrays.asList(new HashSet<>(Arrays.asList("c", "e")), new HashSet<>(Arrays.asList("c", "e")))), null}, }); } }
