You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by ok...@apache.org on 2016/09/27 18:46:13 UTC

[16/23] tinkerpop git commit: added propertyTraversal to toString() of PropertyMapStep (if it exists). Added TraversalHelper.copyLabels() which makes label copying between steps much easier and less error prone. TinkerGraphStepStrategy is smart about hop

added propertyTraversal to toString() of PropertyMapStep (if it exists). Added TraversalHelper.copyLabels() which makes label copying between steps much easier and less error prone. TinkerGraphStepStrategy is smart about hoping over FilterStepp in search of more HasContainer steps.


Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/de31baa4
Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/de31baa4
Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/de31baa4

Branch: refs/heads/master
Commit: de31baa49942293dff43154ecea4ead5b881b25a
Parents: d870372
Author: Marko A. Rodriguez <ok...@gmail.com>
Authored: Mon Sep 26 08:41:17 2016 -0600
Committer: Marko A. Rodriguez <ok...@gmail.com>
Committed: Tue Sep 27 12:45:49 2016 -0600

----------------------------------------------------------------------
 CHANGELOG.asciidoc                              |  2 +
 .../traversal/step/map/PropertyMapStep.java     |  4 +-
 .../strategy/decoration/SubgraphStrategy.java   | 32 ++++---------
 .../process/traversal/util/TraversalHelper.java |  8 ++++
 .../optimization/TinkerGraphStepStrategy.java   | 20 +++++---
 .../TinkerGraphStepStrategyTest.java            | 49 +++++++++++++++++++-
 6 files changed, 82 insertions(+), 33 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/de31baa4/CHANGELOG.asciidoc
----------------------------------------------------------------------
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index fb1160f..cdcad3a 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -26,6 +26,8 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
 TinkerPop 3.2.3 (Release Date: NOT OFFICIALLY RELEASED YET)
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
+* `TinkerGraphStepStrategy` is now smart about non-`HasContainer` filter steps.
+* Added `TraversalHelper.copyLabels()` for copying (or moving) labels form one step to another.
 * Added `TraversalHelper.applySingleLevelStrategies()` which will apply a subset of strategies but not walk the child tree.
 * Added the concept that hidden labels using during traversal compilation are removed at the end during `StandardVerificationStrategy`. (*breaking*)
 * Added `InlineFilterStrategy` which will determine if a `TraversalFilterStep` or `AndStep` children are filters and if so, inline them.

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/de31baa4/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/PropertyMapStep.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/PropertyMapStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/PropertyMapStep.java
index d10b12b..5ab3a17 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/PropertyMapStep.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/PropertyMapStep.java
@@ -113,7 +113,9 @@ public class PropertyMapStep<E> extends MapStep<Element, Map<String, E>> impleme
     }
 
     public String toString() {
-        return StringFactory.stepString(this, Arrays.asList(this.propertyKeys), this.returnType.name().toLowerCase());
+        return null != this.propertyTraversal ?
+                StringFactory.stepString(this, this.propertyTraversal, this.returnType.name().toLowerCase()) :
+                StringFactory.stepString(this, Arrays.asList(this.propertyKeys), this.returnType.name().toLowerCase());
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/de31baa4/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 6ff69b2..0a9cb15 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
@@ -25,7 +25,6 @@ import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__;
 import org.apache.tinkerpop.gremlin.process.traversal.lambda.ElementValueTraversal;
 import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalParent;
 import org.apache.tinkerpop.gremlin.process.traversal.step.filter.ClassFilterStep;
-import org.apache.tinkerpop.gremlin.process.traversal.step.filter.ConnectiveStep;
 import org.apache.tinkerpop.gremlin.process.traversal.step.filter.FilterStep;
 import org.apache.tinkerpop.gremlin.process.traversal.step.filter.OrStep;
 import org.apache.tinkerpop.gremlin.process.traversal.step.filter.TraversalFilterStep;
@@ -141,8 +140,10 @@ public final class SubgraphStrategy extends AbstractTraversalStrategy<TraversalS
     @Override
     public void apply(final Traversal.Admin<?, ?> traversal) {
         // do not apply subgraph strategy to already created subgraph filter branches (or else you get infinite recursion)
-        if (traversal.getStartStep().getLabels().contains(MARKER))
+        if (traversal.getStartStep().getLabels().contains(MARKER)) {
+            traversal.getStartStep().removeLabel(MARKER);
             return;
+        }
         //
         final List<GraphStep> graphSteps = TraversalHelper.getStepsOfAssignableClass(GraphStep.class, traversal);
         final List<VertexStep> vertexSteps = TraversalHelper.getStepsOfAssignableClass(VertexStep.class, traversal);
@@ -178,18 +179,12 @@ public final class SubgraphStrategy extends AbstractTraversalStrategy<TraversalS
 
                 TraversalHelper.replaceStep((Step<Vertex, Edge>) step, someEStep, traversal);
                 TraversalHelper.insertAfterStep(someVStep, someEStep, traversal);
-                // if step was labeled then propagate those labels to the new step that will return the vertex
-                for (final String label : step.getLabels()) {
-                    step.removeLabel(label);
-                    someVStep.addLabel(label);
-                }
+                TraversalHelper.copyLabels(step, someVStep, true);
 
                 if (null != this.edgeCriterion)
                     TraversalHelper.insertAfterStep(new TraversalFilterStep<>(traversal, this.edgeCriterion.clone()), someEStep, traversal);
                 if (null != this.vertexCriterion)
                     TraversalHelper.insertAfterStep(new TraversalFilterStep<>(traversal, this.vertexCriterion.clone()), someVStep, traversal);
-
-
             }
         }
 
@@ -243,26 +238,18 @@ public final class SubgraphStrategy extends AbstractTraversalStrategy<TraversalS
                         if ('v' == propertyType) {
                             final Traversal.Admin<?, ?> temp = nonCheckPropertyCriterion.clone();
                             TraversalHelper.insertTraversal((Step) step, temp, traversal);
-                            for (final String label : step.getLabels()) {
-                                step.removeLabel(label);
-                                temp.getEndStep().addLabel(label);
-                            }
+                            TraversalHelper.copyLabels(step, temp.getEndStep(), true);
                         } else {
                             final Step<?, ?> temp = checkPropertyCriterion.clone();
                             TraversalHelper.insertAfterStep(temp, (Step) step, traversal);
-                            for (final String label : step.getLabels()) {
-                                step.removeLabel(label);
-                                temp.addLabel(label);
-                            }
+                            TraversalHelper.copyLabels(step, temp, true);
                         }
                     } else {
                         // if the property step returns value, then replace it with a property step, append criterion, then append a value() step
                         final Step propertiesStep = new PropertiesStep(traversal, PropertyType.PROPERTY, ((PropertiesStep) step).getPropertyKeys());
                         TraversalHelper.replaceStep(step, propertiesStep, traversal);
                         final Step propertyValueStep = new PropertyValueStep(traversal);
-                        for (final String label : step.getLabels()) {
-                            propertyValueStep.addLabel(label);
-                        }
+                        TraversalHelper.copyLabels(step, propertyValueStep, false);
                         if ('v' == propertyType) {
                             TraversalHelper.insertAfterStep(propertyValueStep, propertiesStep, traversal);
                             TraversalHelper.insertTraversal(propertiesStep, nonCheckPropertyCriterion.clone(), traversal);
@@ -303,11 +290,8 @@ public final class SubgraphStrategy extends AbstractTraversalStrategy<TraversalS
         for (final Step<?, ?> step : stepsToApplyCriterionAfter) {
             // 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());
-            for (final String label : step.getLabels()) {
-                step.removeLabel(label);
-                filter.addLabel(label);
-            }
             TraversalHelper.insertAfterStep(filter, step, traversal);
+            TraversalHelper.copyLabels(step, filter, true);
         }
     }
 

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/de31baa4/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/TraversalHelper.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/TraversalHelper.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/TraversalHelper.java
index 054a1df..7564741 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/TraversalHelper.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/TraversalHelper.java
@@ -571,6 +571,14 @@ public final class TraversalHelper {
         }
     }
 
+    public static void copyLabels(final Step<?, ?> fromStep, final Step<?, ?> toStep, final boolean moveLabels) {
+        for (final String label : fromStep.getLabels()) {
+            toStep.addLabel(label);
+            if (moveLabels)
+                fromStep.removeLabel(label);
+        }
+    }
+
     public static boolean allStepsInstanceOf(final Traversal.Admin<?, ?> traversal, final Class<?> classToCheck, boolean checkIsInstanceOf) {
         for (final Step step : traversal.getSteps()) {
             boolean isInstance = classToCheck.isInstance(step);

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/de31baa4/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerGraphStepStrategy.java
----------------------------------------------------------------------
diff --git a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerGraphStepStrategy.java b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerGraphStepStrategy.java
index a5f258a..97d30e9 100644
--- a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerGraphStepStrategy.java
+++ b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerGraphStepStrategy.java
@@ -22,7 +22,9 @@ import org.apache.tinkerpop.gremlin.process.traversal.Step;
 import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
 import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy;
 import org.apache.tinkerpop.gremlin.process.traversal.step.HasContainerHolder;
+import org.apache.tinkerpop.gremlin.process.traversal.step.filter.FilterStep;
 import org.apache.tinkerpop.gremlin.process.traversal.step.map.GraphStep;
+import org.apache.tinkerpop.gremlin.process.traversal.step.util.HasContainer;
 import org.apache.tinkerpop.gremlin.process.traversal.strategy.AbstractTraversalStrategy;
 import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalHelper;
 import org.apache.tinkerpop.gremlin.tinkergraph.process.traversal.step.sideEffect.TinkerGraphStep;
@@ -45,14 +47,18 @@ public final class TinkerGraphStepStrategy extends AbstractTraversalStrategy<Tra
         TraversalHelper.getStepsOfClass(GraphStep.class, traversal).forEach(originalGraphStep -> {
             final TinkerGraphStep<?, ?> tinkerGraphStep = new TinkerGraphStep<>(originalGraphStep);
             TraversalHelper.replaceStep(originalGraphStep, (Step) tinkerGraphStep, traversal);
+            Step<?, ?> lastNonHolderStep = tinkerGraphStep;
             Step<?, ?> currentStep = tinkerGraphStep.getNextStep();
-            while (currentStep instanceof HasContainerHolder) {
-                ((HasContainerHolder) currentStep).getHasContainers().forEach(hasContainer -> {
-                    if (!GraphStep.processHasContainerIds(tinkerGraphStep, hasContainer))
-                        tinkerGraphStep.addHasContainer(hasContainer);
-                });
-                currentStep.getLabels().forEach(tinkerGraphStep::addLabel);
-                traversal.removeStep(currentStep);
+            while (currentStep instanceof HasContainerHolder || currentStep instanceof FilterStep) {
+                if (currentStep instanceof HasContainerHolder) {
+                    for (final HasContainer hasContainer : ((HasContainerHolder) currentStep).getHasContainers()) {
+                        if (!GraphStep.processHasContainerIds(tinkerGraphStep, hasContainer))
+                            tinkerGraphStep.addHasContainer(hasContainer);
+                    }
+                    TraversalHelper.copyLabels(currentStep, lastNonHolderStep, false);
+                    traversal.removeStep(currentStep);
+                } else
+                    lastNonHolderStep = currentStep;
                 currentStep = currentStep.getNextStep();
             }
         });

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/de31baa4/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerGraphStepStrategyTest.java
----------------------------------------------------------------------
diff --git a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerGraphStepStrategyTest.java b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerGraphStepStrategyTest.java
index 6e4850d..6e7a383 100644
--- a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerGraphStepStrategyTest.java
+++ b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerGraphStepStrategyTest.java
@@ -24,10 +24,18 @@ import org.apache.tinkerpop.gremlin.process.traversal.P;
 import org.apache.tinkerpop.gremlin.process.traversal.TraversalEngine;
 import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversal;
 import org.apache.tinkerpop.gremlin.process.traversal.step.filter.HasStep;
+import org.apache.tinkerpop.gremlin.process.traversal.step.filter.OrStep;
+import org.apache.tinkerpop.gremlin.process.traversal.step.filter.TraversalFilterStep;
+import org.apache.tinkerpop.gremlin.process.traversal.step.map.VertexStep;
 import org.apache.tinkerpop.gremlin.tinkergraph.process.traversal.step.sideEffect.TinkerGraphStep;
 import org.junit.Test;
 
+import java.util.Arrays;
+
+import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.has;
+import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.out;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 
 /**
  * @author Marko A. Rodriguez (http://markorodriguez.com)
@@ -38,6 +46,45 @@ public class TinkerGraphStepStrategyTest extends AbstractGremlinProcessTest {
 
     @Test
     @IgnoreEngine(TraversalEngine.Type.COMPUTER)
+    public void shouldSkipFilterSteps() {
+        GraphTraversal.Admin<?, ?> traversal = g.V().as("a").has("name", "marko").as("b").has("nothing").has("age", 32).as("c").asAdmin();
+        traversal.applyStrategies();
+        assertEquals(2, traversal.getSteps().size());
+        assertEquals(TinkerGraphStep.class, traversal.getStartStep().getClass());
+        assertEquals(TraversalFilterStep.class, traversal.getEndStep().getClass());
+        assertTrue(traversal.getStartStep().getLabels().containsAll(Arrays.asList("a", "b")));
+        assertTrue(traversal.getEndStep().getLabels().containsAll(Arrays.asList("c")));
+        assertEquals("name", ((TinkerGraphStep<?, ?>) traversal.getStartStep()).getHasContainers().get(0).getKey());
+        assertEquals("marko", ((TinkerGraphStep<?, ?>) traversal.getStartStep()).getHasContainers().get(0).getValue());
+        assertEquals("age", ((TinkerGraphStep<?, ?>) traversal.getStartStep()).getHasContainers().get(1).getKey());
+        assertEquals(32, ((TinkerGraphStep<?, ?>) traversal.getStartStep()).getHasContainers().get(1).getValue());
+        //
+        traversal = g.V().as("a").has("name", "marko").as("b").or(has("nothing"), out("something")).has("age", 32).as("c").asAdmin();
+        traversal.applyStrategies();
+        assertEquals(2, traversal.getSteps().size());
+        assertEquals(TinkerGraphStep.class, traversal.getStartStep().getClass());
+        assertEquals(OrStep.class, traversal.getEndStep().getClass());
+        assertTrue(traversal.getStartStep().getLabels().containsAll(Arrays.asList("a", "b")));
+        assertTrue(traversal.getEndStep().getLabels().containsAll(Arrays.asList("c")));
+        assertEquals("name", ((TinkerGraphStep<?, ?>) traversal.getStartStep()).getHasContainers().get(0).getKey());
+        assertEquals("marko", ((TinkerGraphStep<?, ?>) traversal.getStartStep()).getHasContainers().get(0).getValue());
+        assertEquals("age", ((TinkerGraphStep<?, ?>) traversal.getStartStep()).getHasContainers().get(1).getKey());
+        assertEquals(32, ((TinkerGraphStep<?, ?>) traversal.getStartStep()).getHasContainers().get(1).getValue());
+        //
+        traversal = g.V().as("a").has("name", "marko").as("b").out("something").has("age", 32).as("c").asAdmin();
+        traversal.applyStrategies();
+        assertEquals(3, traversal.getSteps().size());
+        assertEquals(TinkerGraphStep.class, traversal.getStartStep().getClass());
+        assertEquals(VertexStep.class, traversal.getSteps().get(1).getClass());
+        assertEquals(HasStep.class, traversal.getEndStep().getClass());
+        assertTrue(traversal.getStartStep().getLabels().containsAll(Arrays.asList("a", "b")));
+        assertTrue(traversal.getEndStep().getLabels().containsAll(Arrays.asList("c")));
+        assertEquals("name", ((TinkerGraphStep<?, ?>) traversal.getStartStep()).getHasContainers().get(0).getKey());
+        assertEquals("marko", ((TinkerGraphStep<?, ?>) traversal.getStartStep()).getHasContainers().get(0).getValue());
+    }
+
+    @Test
+    @IgnoreEngine(TraversalEngine.Type.COMPUTER)
     public void shouldFoldInHasContainers() {
         GraphTraversal.Admin traversal = g.V().has("name", "marko").asAdmin();
         assertEquals(2, traversal.getSteps().size());
@@ -75,7 +122,7 @@ public class TinkerGraphStepStrategyTest extends AbstractGremlinProcessTest {
         assertEquals(TinkerGraphStep.class, traversal.getSteps().get(2).getClass());
         assertEquals(1, ((TinkerGraphStep) traversal.getSteps().get(2)).getHasContainers().size());
         assertEquals("name", ((TinkerGraphStep<?, ?>) traversal.getSteps().get(2)).getHasContainers().get(0).getKey());
-        assertEquals("daniel", ((TinkerGraphStep<?,?>) traversal.getSteps().get(2)).getHasContainers().get(0).getValue());
+        assertEquals("daniel", ((TinkerGraphStep<?, ?>) traversal.getSteps().get(2)).getHasContainers().get(0).getValue());
         assertEquals(TinkerGraphStep.class, traversal.getEndStep().getClass());
     }