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/09/29 20:10:05 UTC

[26/50] tinkerpop git commit: I was led down a dumb hole with TinkerGraphStepStrategy --- FilterRankStrategy does the filter sorting. Dar. I rewrote TinkerGraphStepStrategyTest so its like the other SrategyTest classes. It doesn't need to be a suite and

I was led down a dumb hole with TinkerGraphStepStrategy --- FilterRankStrategy does the filter sorting. Dar. I rewrote TinkerGraphStepStrategyTest so its like the other SrategyTest classes. It doesn't need to be a suite and all that...Now things are much more obvious as we can run parallel strategies to see how things get compiled. Also, made FilterRankStrategy is bit quicker by bailing fast is the step is not a FilterStep or OrderGlobalStep. CTR as this is basically a revert from 1458 work.


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

Branch: refs/heads/TINKERPOP-1458
Commit: 24c14cf914fd5997160cba75cdcc479fe652e5fd
Parents: 4821dbc
Author: Marko A. Rodriguez <ok...@gmail.com>
Authored: Tue Sep 27 16:25:23 2016 -0600
Committer: Marko A. Rodriguez <ok...@gmail.com>
Committed: Tue Sep 27 16:25:23 2016 -0600

----------------------------------------------------------------------
 CHANGELOG.asciidoc                              |   1 -
 .../optimization/FilterRankingStrategy.java     |  42 +++--
 .../optimization/FilterRankingStrategyTest.java |  13 +-
 .../optimization/TinkerGraphStepStrategy.java   |  33 ++--
 .../strategy/TinkerGraphStrategySuite.java      |  44 -----
 .../strategy/TinkerGraphStrategyTest.java       |  32 ----
 .../TinkerGraphStepStrategyTest.java            | 160 +++++++++----------
 7 files changed, 117 insertions(+), 208 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/24c14cf9/CHANGELOG.asciidoc
----------------------------------------------------------------------
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 3fc278a..ff730f7 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -30,7 +30,6 @@ TinkerPop 3.2.3 (Release Date: NOT OFFICIALLY RELEASED YET)
 * Fixed a end-step label bug in `MatchPredicateStrategy`.
 * Fixed a bug in `MatchPredicateStrategy` where inlined traversals did not have strategies applied to it.
 * Fixed a bug in `RepeatUnrollStrategy` where inlined traversal did not have strategies applied to it.
-* `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*)

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/24c14cf9/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/FilterRankingStrategy.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/FilterRankingStrategy.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/FilterRankingStrategy.java
index eaa69bc..96843d9 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/FilterRankingStrategy.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/FilterRankingStrategy.java
@@ -23,7 +23,17 @@ import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
 import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy;
 import org.apache.tinkerpop.gremlin.process.traversal.step.LambdaHolder;
 import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalParent;
-import org.apache.tinkerpop.gremlin.process.traversal.step.filter.*;
+import org.apache.tinkerpop.gremlin.process.traversal.step.filter.AndStep;
+import org.apache.tinkerpop.gremlin.process.traversal.step.filter.CyclicPathStep;
+import org.apache.tinkerpop.gremlin.process.traversal.step.filter.DedupGlobalStep;
+import org.apache.tinkerpop.gremlin.process.traversal.step.filter.FilterStep;
+import org.apache.tinkerpop.gremlin.process.traversal.step.filter.HasStep;
+import org.apache.tinkerpop.gremlin.process.traversal.step.filter.IsStep;
+import org.apache.tinkerpop.gremlin.process.traversal.step.filter.OrStep;
+import org.apache.tinkerpop.gremlin.process.traversal.step.filter.SimplePathStep;
+import org.apache.tinkerpop.gremlin.process.traversal.step.filter.TraversalFilterStep;
+import org.apache.tinkerpop.gremlin.process.traversal.step.filter.WherePredicateStep;
+import org.apache.tinkerpop.gremlin.process.traversal.step.filter.WhereTraversalStep;
 import org.apache.tinkerpop.gremlin.process.traversal.step.map.OrderGlobalStep;
 import org.apache.tinkerpop.gremlin.process.traversal.strategy.AbstractTraversalStrategy;
 import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalHelper;
@@ -110,39 +120,41 @@ public final class FilterRankingStrategy extends AbstractTraversalStrategy<Trave
      * Ranks the given step. Steps with lower ranks can be moved in front of steps with higher ranks. 0 means that
      * the step has no rank and thus is not exchangeable with its neighbors.
      *
-     * @param step
+     * @param step the step to get a ranking for
      * @return The rank of the given step.
      */
     private static int getStepRank(final Step step) {
-        if (step instanceof IsStep) {
+        if (!(step instanceof FilterStep || step instanceof OrderGlobalStep))
+            return 0;
+        else if (step instanceof IsStep)
             return 1;
-        } else if (step instanceof HasStep) {
+        else if (step instanceof HasStep)
             return 2;
-        } else if (step instanceof WherePredicateStep) {
+        else if (step instanceof WherePredicateStep)
             return 3;
-        } else if (step instanceof SimplePathStep || step instanceof CyclicPathStep) {
+        else if (step instanceof SimplePathStep || step instanceof CyclicPathStep)
             return 4;
-        } else if (step instanceof TraversalFilterStep) {
+        else if (step instanceof TraversalFilterStep)
             return 5;
-        } else if (step instanceof WhereTraversalStep) {
+        else if (step instanceof WhereTraversalStep)
             return 6;
-        } else if (step instanceof OrStep) {
+        else if (step instanceof OrStep)
             return 7;
-        } else if (step instanceof AndStep) {
+        else if (step instanceof AndStep)
             return 8;
-        } else if (step instanceof DedupGlobalStep) {
+        else if (step instanceof DedupGlobalStep)
             return 9;
-        } else if (step instanceof OrderGlobalStep) {
+        else if (step instanceof OrderGlobalStep)
             return 10;
-        }
-        return 0;
+        else
+            return 0;
     }
 
     /**
      * If the given step has no child traversal that holds a lambda, then the actual rank determined by
      * {@link #getStepRank(Step)} is returned, otherwise 0.
      *
-     * @param step
+     * @param step the step to get a ranking for
      * @return The rank of the given step.
      */
     private static int rank(final Step step) {

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/24c14cf9/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/FilterRankingStrategyTest.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/FilterRankingStrategyTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/FilterRankingStrategyTest.java
index 71f5b4b..18ef6ca 100644
--- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/FilterRankingStrategyTest.java
+++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/FilterRankingStrategyTest.java
@@ -31,6 +31,7 @@ import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 
+import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.has;
 import static org.junit.Assert.assertEquals;
 
 /**
@@ -39,7 +40,6 @@ import static org.junit.Assert.assertEquals;
 @RunWith(Parameterized.class)
 public class FilterRankingStrategyTest {
 
-
     public static Iterable<Object[]> data() {
         return generateTestParameters();
     }
@@ -75,11 +75,12 @@ public class FilterRankingStrategyTest {
                 {__.identity().order().dedup(), __.dedup().order(), Collections.singletonList(IdentityRemovalStrategy.instance())},
                 {__.order().identity().dedup(), __.dedup().order(), Collections.singletonList(IdentityRemovalStrategy.instance())},
                 {__.order().out().dedup(), __.order().out().dedup(), Collections.emptyList()},
-                {__.has("value", 0).filter(__.out()).dedup(), __.has("value", 0).filter(__.out()).dedup(), Collections.emptyList()},
-                {__.dedup().filter(__.out()).has("value", 0), __.has("value", 0).filter(__.out()).dedup(), Collections.emptyList()},
-                {__.filter(__.out()).dedup().has("value", 0), __.has("value", 0).filter(__.out()).dedup(), Collections.emptyList()},
-                {__.has("value", 0).filter(__.out()).dedup(), __.has("value", 0).filter(__.out()).dedup(), Collections.emptyList()},
-                {__.has("value", 0).and(__.has("age"), __.has("name", "marko")).is(10), __.is(10).has("value", 0).has("name", "marko").has("age"), Collections.singletonList(InlineFilterStrategy.instance())},
+                {has("value", 0).filter(__.out()).dedup(), has("value", 0).filter(__.out()).dedup(), Collections.emptyList()},
+                {__.dedup().filter(__.out()).has("value", 0), has("value", 0).filter(__.out()).dedup(), Collections.emptyList()},
+                {__.filter(__.out()).dedup().has("value", 0), has("value", 0).filter(__.out()).dedup(), Collections.emptyList()},
+                {has("value", 0).filter(__.out()).dedup(), has("value", 0).filter(__.out()).dedup(), Collections.emptyList()},
+                {has("value", 0).or(has("name"), has("age")).has("value", 1).dedup(), has("value", 0).has("value", 1).or(has("name"), has("age")).dedup(), Collections.emptyList()},
+                {has("value", 0).and(has("age"), has("name", "marko")).is(10), __.is(10).has("value", 0).has("name", "marko").has("age"), Collections.singletonList(InlineFilterStrategy.instance())},
         });
     }
 }

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/24c14cf9/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 fc3748f..5ba68eb 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,11 +22,6 @@ 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.LambdaHolder;
-import org.apache.tinkerpop.gremlin.process.traversal.step.filter.DedupGlobalStep;
-import org.apache.tinkerpop.gremlin.process.traversal.step.filter.DropStep;
-import org.apache.tinkerpop.gremlin.process.traversal.step.filter.FilterStep;
-import org.apache.tinkerpop.gremlin.process.traversal.step.filter.RangeGlobalStep;
 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;
@@ -48,28 +43,20 @@ public final class TinkerGraphStepStrategy extends AbstractTraversalStrategy<Tra
         if (TraversalHelper.onGraphComputer(traversal))
             return;
 
-        TraversalHelper.getStepsOfClass(GraphStep.class, traversal).forEach(originalGraphStep -> {
+        for (final GraphStep originalGraphStep : TraversalHelper.getStepsOfClass(GraphStep.class, traversal)) {
             final TinkerGraphStep<?, ?> tinkerGraphStep = new TinkerGraphStep<>(originalGraphStep);
-            TraversalHelper.replaceStep(originalGraphStep, (Step) tinkerGraphStep, traversal);
-            Step<?, ?> lastNonHolderStep = tinkerGraphStep;
+            TraversalHelper.replaceStep(originalGraphStep, tinkerGraphStep, traversal);
             Step<?, ?> currentStep = tinkerGraphStep.getNextStep();
-            while (currentStep instanceof FilterStep &&
-                    !(currentStep instanceof DedupGlobalStep) &&
-                    !(currentStep instanceof RangeGlobalStep) &&
-                    !(currentStep instanceof DropStep) &&
-                    !(currentStep instanceof LambdaHolder)) {
-                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;
+            while (currentStep instanceof HasContainerHolder) {
+                for (final HasContainer hasContainer : ((HasContainerHolder) currentStep).getHasContainers()) {
+                    if (!GraphStep.processHasContainerIds(tinkerGraphStep, hasContainer))
+                        tinkerGraphStep.addHasContainer(hasContainer);
+                }
+                TraversalHelper.copyLabels(currentStep, tinkerGraphStep, false);
+                traversal.removeStep(currentStep);
                 currentStep = currentStep.getNextStep();
             }
-        });
+        }
     }
 
     public static TinkerGraphStepStrategy instance() {

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/24c14cf9/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/TinkerGraphStrategySuite.java
----------------------------------------------------------------------
diff --git a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/TinkerGraphStrategySuite.java b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/TinkerGraphStrategySuite.java
deleted file mode 100644
index 3982918..0000000
--- a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/TinkerGraphStrategySuite.java
+++ /dev/null
@@ -1,44 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-package org.apache.tinkerpop.gremlin.tinkergraph.process.traversal.strategy;
-
-import org.apache.tinkerpop.gremlin.AbstractGremlinSuite;
-import org.apache.tinkerpop.gremlin.process.traversal.TraversalEngine;
-import org.apache.tinkerpop.gremlin.tinkergraph.process.traversal.strategy.optimization.TinkerGraphStepStrategyTest;
-import org.junit.runners.model.InitializationError;
-import org.junit.runners.model.RunnerBuilder;
-
-/**
- * @author Daniel Kuppitz (http://gremlin.guru)
- */
-public class TinkerGraphStrategySuite extends AbstractGremlinSuite {
-
-    public TinkerGraphStrategySuite(final Class<?> klass, final RunnerBuilder builder) throws InitializationError {
-
-        super(klass, builder,
-                new Class<?>[]{
-                        TinkerGraphStepStrategyTest.class
-                }, new Class<?>[]{
-                        TinkerGraphStepStrategyTest.class
-                },
-                false,
-                TraversalEngine.Type.STANDARD);
-    }
-
-}
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/24c14cf9/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/TinkerGraphStrategyTest.java
----------------------------------------------------------------------
diff --git a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/TinkerGraphStrategyTest.java b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/TinkerGraphStrategyTest.java
deleted file mode 100644
index 7942626..0000000
--- a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/TinkerGraphStrategyTest.java
+++ /dev/null
@@ -1,32 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-package org.apache.tinkerpop.gremlin.tinkergraph.process.traversal.strategy;
-
-import org.apache.tinkerpop.gremlin.GraphProviderClass;
-import org.apache.tinkerpop.gremlin.tinkergraph.TinkerGraphProvider;
-import org.apache.tinkerpop.gremlin.tinkergraph.structure.TinkerGraph;
-import org.junit.runner.RunWith;
-
-/**
- * @author Daniel Kuppitz (http://gremlin.guru)
- */
-@RunWith(TinkerGraphStrategySuite.class)
-@GraphProviderClass(provider = TinkerGraphProvider.class, graph = TinkerGraph.class)
-public class TinkerGraphStrategyTest {
-}

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/24c14cf9/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 6e7a383..89ca7ba 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
@@ -18,112 +18,98 @@
  */
 package org.apache.tinkerpop.gremlin.tinkergraph.process.traversal.strategy.optimization;
 
-import org.apache.tinkerpop.gremlin.process.AbstractGremlinProcessTest;
-import org.apache.tinkerpop.gremlin.process.IgnoreEngine;
 import org.apache.tinkerpop.gremlin.process.traversal.P;
-import org.apache.tinkerpop.gremlin.process.traversal.TraversalEngine;
+import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
+import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategies;
+import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy;
+import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.DefaultGraphTraversal;
 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.process.traversal.dsl.graph.__;
+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.optimization.FilterRankingStrategy;
+import org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.InlineFilterStrategy;
+import org.apache.tinkerpop.gremlin.process.traversal.util.DefaultTraversalStrategies;
+import org.apache.tinkerpop.gremlin.structure.Vertex;
 import org.apache.tinkerpop.gremlin.tinkergraph.process.traversal.step.sideEffect.TinkerGraphStep;
+import org.apache.tinkerpop.gremlin.tinkergraph.structure.TinkerGraph;
 import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
 
 import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
 
+import static org.apache.tinkerpop.gremlin.process.traversal.P.eq;
+import static org.apache.tinkerpop.gremlin.process.traversal.P.gt;
+import static org.apache.tinkerpop.gremlin.process.traversal.P.lt;
 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)
  * @author Daniel Kuppitz (http://gremlin.guru)
  */
 
-public class TinkerGraphStepStrategyTest extends AbstractGremlinProcessTest {
+@RunWith(Parameterized.class)
+public class TinkerGraphStepStrategyTest {
+
+    @Parameterized.Parameter(value = 0)
+    public Traversal original;
+
+    @Parameterized.Parameter(value = 1)
+    public Traversal optimized;
+
+    @Parameterized.Parameter(value = 2)
+    public Collection<TraversalStrategy> otherStrategies;
 
     @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());
+    public void doTest() {
+        final TraversalStrategies strategies = new DefaultTraversalStrategies();
+        strategies.addStrategies(TinkerGraphStepStrategy.instance());
+        for (final TraversalStrategy strategy : this.otherStrategies) {
+            strategies.addStrategies(strategy);
+        }
+        this.original.asAdmin().setStrategies(strategies);
+        this.original.asAdmin().applyStrategies();
+        assertEquals(this.optimized, this.original);
     }
 
-    @Test
-    @IgnoreEngine(TraversalEngine.Type.COMPUTER)
-    public void shouldFoldInHasContainers() {
-        GraphTraversal.Admin traversal = g.V().has("name", "marko").asAdmin();
-        assertEquals(2, traversal.getSteps().size());
-        assertEquals(HasStep.class, traversal.getEndStep().getClass());
-        traversal.applyStrategies();
-        assertEquals(1, traversal.getSteps().size());
-        assertEquals(TinkerGraphStep.class, traversal.getStartStep().getClass());
-        assertEquals(TinkerGraphStep.class, traversal.getEndStep().getClass());
-        assertEquals(1, ((TinkerGraphStep) traversal.getStartStep()).getHasContainers().size());
-        assertEquals("name", ((TinkerGraphStep<?, ?>) traversal.getStartStep()).getHasContainers().get(0).getKey());
-        assertEquals("marko", ((TinkerGraphStep<?, ?>) traversal.getStartStep()).getHasContainers().get(0).getValue());
-        ////
-        traversal = g.V().has("name", "marko").has("age", P.gt(20)).asAdmin();
-        traversal.applyStrategies();
-        assertEquals(1, traversal.getSteps().size());
-        assertEquals(TinkerGraphStep.class, traversal.getStartStep().getClass());
-        assertEquals(2, ((TinkerGraphStep) traversal.getStartStep()).getHasContainers().size());
-        ////
-        traversal = g.V().has("name", "marko").out().has("name", "daniel").asAdmin();
-        traversal.applyStrategies();
-        assertEquals(3, traversal.getSteps().size());
-        assertEquals(TinkerGraphStep.class, traversal.getStartStep().getClass());
-        assertEquals(1, ((TinkerGraphStep) traversal.getStartStep()).getHasContainers().size());
-        assertEquals("name", ((TinkerGraphStep<?, ?>) traversal.getStartStep()).getHasContainers().get(0).getKey());
-        assertEquals("marko", ((TinkerGraphStep<?, ?>) traversal.getStartStep()).getHasContainers().get(0).getValue());
-        assertEquals(HasStep.class, traversal.getEndStep().getClass());
-        ////
-        traversal = g.V().has("name", "marko").out().V().has("name", "daniel").asAdmin();
-        traversal.applyStrategies();
-        assertEquals(3, traversal.getSteps().size());
-        assertEquals(TinkerGraphStep.class, traversal.getStartStep().getClass());
-        assertEquals(1, ((TinkerGraphStep) traversal.getStartStep()).getHasContainers().size());
-        assertEquals("name", ((TinkerGraphStep<?, ?>) traversal.getStartStep()).getHasContainers().get(0).getKey());
-        assertEquals("marko", ((TinkerGraphStep<?, ?>) traversal.getStartStep()).getHasContainers().get(0).getValue());
-        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(TinkerGraphStep.class, traversal.getEndStep().getClass());
+    private static GraphTraversal.Admin<?, ?> g_V(final Object... hasKeyValues) {
+        final GraphTraversal.Admin<?, ?> traversal = new DefaultGraphTraversal<>();
+        final TinkerGraphStep<Vertex, Vertex> graphStep = new TinkerGraphStep<>(new GraphStep<>(traversal, Vertex.class, true));
+        for (int i = 0; i < hasKeyValues.length; i = i + 2) {
+            graphStep.addHasContainer(new HasContainer((String) hasKeyValues[i], (P) hasKeyValues[i + 1]));
+        }
+        return traversal.addStep(graphStep);
     }
 
+    @Parameterized.Parameters(name = "{0}")
+    public static Iterable<Object[]> generateTestParameters() {
+        return Arrays.asList(new Object[][]{
+                {__.V().out(), g_V().out(), Collections.emptyList()},
+                {__.V().has("name", "marko").out(), g_V("name", eq("marko")).out(), Collections.emptyList()},
+                {__.V().has("name", "marko").has("age", gt(31).and(lt(10))).out(),
+                        g_V("name", eq("marko"), "age", gt(31), "age", lt(10)).out(), Collections.emptyList()},
+                {__.V().has("name", "marko").or(has("age"), has("age", gt(32))).has("lang", "java"),
+                        g_V("name", eq("marko"), "lang", eq("java")).or(has("age"), has("age", gt(32))), Collections.singletonList(FilterRankingStrategy.instance())},
+                {__.V().has("name", "marko").as("a").or(has("age"), has("age", gt(32))).has("lang", "java"),
+                        g_V("name", eq("marko")).as("a").or(has("age"), has("age", gt(32))).has("lang", "java"), Collections.emptyList()},
+                {__.V().has("name", "marko").as("a").or(has("age"), has("age", gt(32))).has("lang", "java"),
+                        g_V("name", eq("marko"), "lang", eq("java")).as("a").or(has("age"), has("age", gt(32))), Collections.singletonList(FilterRankingStrategy.instance())},
+                {__.V().dedup().has("name", "marko").or(has("age"), has("age", gt(32))).has("lang", "java"),
+                        g_V("name", eq("marko"), "lang", eq("java")).or(has("age"), has("age", gt(32))).dedup(), Collections.singletonList(FilterRankingStrategy.instance())},
+                {__.V().as("a").dedup().has("name", "marko").or(has("age"), has("age", gt(32))).has("lang", "java"),
+                        g_V("name", eq("marko"), "lang", eq("java")).as("a").or(has("age"), has("age", gt(32))).dedup(), Collections.singletonList(FilterRankingStrategy.instance())},
+                {__.V().as("a").has("name", "marko").as("b").or(has("age"), has("age", gt(32))).has("lang", "java"),
+                        g_V("name", eq("marko"), "lang", eq("java")).as("a", "b").or(has("age"), has("age", gt(32))), Collections.singletonList(FilterRankingStrategy.instance())},
+                {__.V().as("a").dedup().has("name", "marko").or(has("age"), has("age", gt(32))).filter(has("name", "bob")).has("lang", "java"),
+                        g_V("name", eq("marko"), "name", eq("bob"), "lang", eq("java")).as("a").or(has("age"), has("age", gt(32))).dedup(), Arrays.asList(InlineFilterStrategy.instance(), FilterRankingStrategy.instance())},
+                {__.V().as("a").dedup().has("name", "marko").or(has("age", 10), has("age", gt(32))).filter(has("name", "bob")).has("lang", "java"),
+                        g_V("name", eq("marko"), "name", eq("bob"), "lang", eq("java")).as("a").or(has("age", 10), has("age", gt(32))).dedup(), TraversalStrategies.GlobalCache.getStrategies(TinkerGraph.class).toList()},
+        });
+    }
 }
+
+