You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by dk...@apache.org on 2019/06/11 15:38:00 UTC

[tinkerpop] 01/02: TINKERPOP-1084 Allow predicates to be used as options in BranchSteps.

This is an automated email from the ASF dual-hosted git repository.

dkuppitz pushed a commit to branch TINKERPOP-1084
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git

commit 2bc081daf86548e245d2c918193581fb4c131411
Author: Daniel Kuppitz <da...@hotmail.com>
AuthorDate: Mon Jun 10 12:45:16 2019 -0700

    TINKERPOP-1084 Allow predicates to be used as options in BranchSteps.
---
 CHANGELOG.asciidoc                                 |   1 +
 .../process/traversal/step/branch/BranchStep.java  | 102 ++++++++++++++++-----
 .../process/traversal/step/ComplexTest.java        |  58 +++++++++---
 .../tinkergraph/structure/TinkerGraphPlayTest.java |  22 +++--
 4 files changed, 141 insertions(+), 42 deletions(-)

diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 03ba330..8b2cd62 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -30,6 +30,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
 * Added test infrastructure to check for storage iterator leak.
 * Fixed multiple iterator leaks in query processor.
 * Fixed bug in `MatchStep` where the correct was not properly determined.
+* Allow predicates to be used as options in BranchSteps.
 
 [[release-3-3-7]]
 === TinkerPop 3.3.7 (Release Date: May 28, 2019)
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/BranchStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/BranchStep.java
index e35d51e..efc43cc 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/BranchStep.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/BranchStep.java
@@ -38,18 +38,23 @@ import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
+import java.util.function.Predicate;
 import java.util.stream.Collectors;
 
 /**
  * @author Marko A. Rodriguez (http://markorodriguez.com)
+ * @author Daniel Kuppitz (http://gremlin.guru)
  */
 public class BranchStep<S, E, M> extends ComputerAwareStep<S, E> implements TraversalOptionParent<M, S, E> {
 
     protected Traversal.Admin<S, M> branchTraversal;
     protected Map<Object, List<Traversal.Admin<S, E>>> traversalOptions = new HashMap<>();
+
     private boolean first = true;
-    private boolean hasBarrier = false;
+    private boolean hasBarrier;
+    private boolean hasPredicateOptions;
 
     public BranchStep(final Traversal.Admin traversal) {
         super(traversal);
@@ -61,7 +66,8 @@ public class BranchStep<S, E, M> extends ComputerAwareStep<S, E> implements Trav
 
     @Override
     public void addGlobalChildOption(final M pickToken, final Traversal.Admin<S, E> traversalOption) {
-        final Object pickTokenKey = PickTokenKey.make(pickToken);
+        final Object pickTokenKey = makePickTokenKey(pickToken);
+        this.hasPredicateOptions |= pickTokenKey instanceof PredicateOption;
         if (this.traversalOptions.containsKey(pickTokenKey))
             this.traversalOptions.get(pickTokenKey).add(traversalOption);
         else
@@ -138,14 +144,13 @@ public class BranchStep<S, E, M> extends ComputerAwareStep<S, E> implements Trav
     private void applyCurrentTraverser(final Traverser.Admin<S> start) {
         // first get the value of the choice based on the current traverser and use that to select the right traversal
         // option to which that traverser should be routed
-        final Object choice = PickTokenKey.make(TraversalUtil.apply(start, this.branchTraversal));
-        final List<Traversal.Admin<S, E>> branch = this.traversalOptions.containsKey(choice) ?
-                this.traversalOptions.get(choice) : this.traversalOptions.get(Pick.none);
+        final Object choice = makePickTokenKey(TraversalUtil.apply(start, this.branchTraversal));
+        final List<Traversal.Admin<S, E>> branches = pickBranches(choice);
 
         // if a branch is identified, then split the traverser and add it to the start of the option so that when
         // that option is iterated (in the calling method) that value can be applied.
-        if (null != branch)
-            branch.forEach(traversal -> traversal.addStart(start.split()));
+        if (null != branches)
+            branches.forEach(traversal -> traversal.addStart(start.split()));
 
         if (choice != Pick.any) {
             final List<Traversal.Admin<S, E>> anyBranch = this.traversalOptions.get(Pick.any);
@@ -158,10 +163,10 @@ public class BranchStep<S, E, M> extends ComputerAwareStep<S, E> implements Trav
     protected Iterator<Traverser.Admin<E>> computerAlgorithm() {
         final List<Traverser.Admin<E>> ends = new ArrayList<>();
         final Traverser.Admin<S> start = this.starts.next();
-        final Object choice = PickTokenKey.make(TraversalUtil.apply(start, this.branchTraversal));
-        final List<Traversal.Admin<S, E>> branch = this.traversalOptions.containsKey(choice) ? this.traversalOptions.get(choice) : this.traversalOptions.get(Pick.none);
-        if (null != branch) {
-            branch.forEach(traversal -> {
+        final Object choice = makePickTokenKey(TraversalUtil.apply(start, this.branchTraversal));
+        final List<Traversal.Admin<S, E>> branches = pickBranches(choice);
+        if (null != branches) {
+            branches.forEach(traversal -> {
                 final Traverser.Admin<E> split = (Traverser.Admin<E>) start.split();
                 split.setStepId(traversal.getStartStep().getId());
                 //split.addLabels(this.labels);
@@ -182,6 +187,22 @@ public class BranchStep<S, E, M> extends ComputerAwareStep<S, E> implements Trav
         return ends.iterator();
     }
 
+    private List<Traversal.Admin<S, E>> pickBranches(final Object choice) {
+        final List<Traversal.Admin<S, E>> branches = new ArrayList<>();
+        if (this.hasPredicateOptions) {
+            for (final Map.Entry<Object, List<Traversal.Admin<S, E>>> e : this.traversalOptions.entrySet()) {
+                if (Objects.equals(e.getKey(), choice)) {
+                    branches.addAll(e.getValue());
+                }
+            }
+        } else {
+            if (this.traversalOptions.containsKey(choice)) {
+                branches.addAll(this.traversalOptions.get(choice));
+            }
+        }
+        return branches.isEmpty() ? this.traversalOptions.get(Pick.none) : branches;
+    }
+
     @Override
     public BranchStep<S, E, M> clone() {
         final BranchStep<S, E, M> clone = (BranchStep<S, E, M>) super.clone();
@@ -230,26 +251,30 @@ public class BranchStep<S, E, M> extends ComputerAwareStep<S, E> implements Trav
     }
 
     /**
-     * PickTokenKey is basically a wrapper for numbers that are used as a PickToken. This is
-     * required in order to treat equal numbers of different data types as a match.
+     * Converts numbers into {@link NumberOption} and predicates into {@link PredicateOption}.
+     */
+    private static Object makePickTokenKey(final Object o) {
+        return
+                o instanceof Number ? new NumberOption((Number) o) :
+                o instanceof Predicate ? new PredicateOption((Predicate) o) : o;
+    }
+
+    /**
+     * Wraps a single number and overrides equals/hashCode/compareTo in order to ignore the numeric data type.
      */
-    private static class PickTokenKey {
+    private static class NumberOption implements Comparable<Number> {
 
         final Number number;
 
-        private PickTokenKey(final Number number) {
+        NumberOption(final Number number) {
             this.number = number;
         }
 
-        static Object make(final Object value) {
-            return value instanceof Number ? new PickTokenKey((Number) value) : value;
-        }
-
         @Override
         public boolean equals(final Object o) {
             if (this == o) return true;
             if (o == null || getClass() != o.getClass()) return false;
-            final PickTokenKey other = (PickTokenKey) o;
+            final NumberOption other = (NumberOption) o;
             return 0 == NumberHelper.compare(number, other.number);
         }
 
@@ -262,5 +287,40 @@ public class BranchStep<S, E, M> extends ComputerAwareStep<S, E> implements Trav
         public String toString() {
             return number.toString();
         }
+
+        @Override
+        public int compareTo(final Number other) {
+            return NumberHelper.compare(this.number, other);
+        }
+    }
+
+    /**
+     * Wraps a single predicate and overrides equals/hashCode so that the predicate can be matched against a concrete value.
+     */
+    private static class PredicateOption {
+
+        final Predicate predicate;
+
+        PredicateOption(final Predicate predicate) {
+            this.predicate = predicate;
+        }
+
+        @SuppressWarnings({"EqualsWhichDoesntCheckParameterClass", "unchecked"})
+        @Override
+        public boolean equals(final Object o) {
+            if (o instanceof PredicateOption)
+                return ((PredicateOption) o).predicate.equals(predicate);
+            return predicate.test(o);
+        }
+
+        @Override
+        public int hashCode() {
+            return 0;
+        }
+
+        @Override
+        public String toString() {
+            return predicate.toString();
+        }
     }
-}
+}
\ No newline at end of file
diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/ComplexTest.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/ComplexTest.java
index 75dd0f5..a3bb1cd 100644
--- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/ComplexTest.java
+++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/ComplexTest.java
@@ -44,24 +44,14 @@ import java.util.Map;
 import static java.util.Collections.emptyList;
 import static org.apache.tinkerpop.gremlin.LoadGraphWith.GraphData.MODERN;
 import static org.apache.tinkerpop.gremlin.process.traversal.P.eq;
+import static org.apache.tinkerpop.gremlin.process.traversal.P.gte;
 import static org.apache.tinkerpop.gremlin.process.traversal.P.lt;
 import static org.apache.tinkerpop.gremlin.process.traversal.P.neq;
 import static org.apache.tinkerpop.gremlin.process.traversal.Pop.all;
 import static org.apache.tinkerpop.gremlin.process.traversal.Pop.first;
 import static org.apache.tinkerpop.gremlin.process.traversal.Pop.last;
 import static org.apache.tinkerpop.gremlin.process.traversal.Scope.local;
-import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.both;
-import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.constant;
-import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.count;
-import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.group;
-import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.in;
-import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.out;
-import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.outE;
-import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.project;
-import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.select;
-import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.unfold;
-import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.values;
-import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.where;
+import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.*;
 import static org.apache.tinkerpop.gremlin.structure.Column.keys;
 import static org.apache.tinkerpop.gremlin.structure.Column.values;
 import static org.junit.Assert.assertEquals;
@@ -86,6 +76,8 @@ public abstract class ComplexTest extends AbstractGremlinProcessTest {
 
     public abstract Traversal<Vertex, Map<String, List<String>>> getPlaylistPaths();
 
+    public abstract Traversal<Vertex, Map<String, List<String>>> getAgeComments();
+
     /**
      * Checks the result of both coworkerSummary tests, which is expected to look as follows:
      * <p>
@@ -243,6 +235,31 @@ public abstract class ComplexTest extends AbstractGremlinProcessTest {
         assertTrue(map.get("artists").contains("Grateful_Dead"));
     }
 
+    @Test
+    @LoadGraphWith(LoadGraphWith.GraphData.MODERN)
+    public void ageComments() {
+        final Traversal<Vertex, Map<String, List<String>>> traversal = getAgeComments();
+        printTraversalForm(traversal);
+        assertTrue(traversal.hasNext());
+        Map<String, List<String>> map = traversal.next();
+        assertEquals(4, map.size());
+        assertTrue(map.containsKey("marko"));
+        assertEquals(2, map.get("marko").size());
+        assertTrue(map.get("marko").contains("almost old"));
+        assertTrue(map.get("marko").contains("younger than peter"));
+        assertTrue(map.containsKey("vadas"));
+        assertEquals(2, map.get("vadas").size());
+        assertTrue(map.get("vadas").contains("pretty young"));
+        assertTrue(map.get("vadas").contains("younger than peter"));
+        assertTrue(map.containsKey("josh"));
+        assertEquals(2, map.get("josh").size());
+        assertTrue(map.get("josh").contains("younger than peter"));
+        assertTrue(map.get("josh").contains("pretty old"));
+        assertTrue(map.containsKey("peter"));
+        assertEquals(1, map.get("peter").size());
+        assertTrue(map.get("peter").contains("pretty old"));
+    }
+
     public static class Traversals extends ComplexTest {
 
         @Override
@@ -265,7 +282,7 @@ public abstract class ComplexTest extends AbstractGremlinProcessTest {
         public Traversal<Vertex, Map<String, Map<String, Map<String, Object>>>> getCoworkerSummary() {
             return g.V().hasLabel("person").filter(outE("created")).aggregate("p").as("p1").values("name").as("p1n")
                     .select("p").unfold().where(neq("p1")).as("p2").values("name").as("p2n").select("p2")
-                    .out("created").choose(in("created").where(eq("p1")), values("name"), constant(emptyList()))
+                    .out("created").choose(in("created").where(eq("p1")), __.values("name"), constant(emptyList()))
                     .<String, Map<String, Map<String, Object>>>group().by(select("p1n")).
                             by(group().by(select("p2n")).
                                     by(unfold().fold().project("numCoCreated", "coCreated").by(count(local)).by()));
@@ -323,6 +340,17 @@ public abstract class ComplexTest extends AbstractGremlinProcessTest {
                     by("name").
                     by(__.coalesce(out("sungBy", "writtenBy").dedup().values("name"), constant("Unknown")).fold());
         }
-    }
-}
 
+        @Override
+        public Traversal<Vertex, Map<String, List<String>>> getAgeComments() {
+            return g.V().hasLabel("person")
+                    .<String, List<String>> group()
+                        .by("name")
+                        .by(branch(__.values("age"))
+                                .option(29, constant("almost old"))
+                                .option(lt(29), constant("pretty young"))
+                                .option(lt(35), constant("younger than peter"))
+                                .option(gte(30), constant("pretty old")).fold());
+        }
+    }
+}
\ No newline at end of file
diff --git a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphPlayTest.java b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphPlayTest.java
index d4f7d5d..3b241bc 100644
--- a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphPlayTest.java
+++ b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphPlayTest.java
@@ -27,6 +27,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.Traverser;
 import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversal;
 import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource;
 import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__;
+import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalOptionParent;
 import org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.EarlyLimitStrategy;
 import org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.PathRetractionStrategy;
 import org.apache.tinkerpop.gremlin.structure.*;
@@ -45,6 +46,10 @@ import java.util.function.BiFunction;
 import java.util.function.Supplier;
 
 import static org.apache.tinkerpop.gremlin.process.traversal.Operator.sum;
+import static org.apache.tinkerpop.gremlin.process.traversal.P.between;
+import static org.apache.tinkerpop.gremlin.process.traversal.P.gt;
+import static org.apache.tinkerpop.gremlin.process.traversal.P.gte;
+import static org.apache.tinkerpop.gremlin.process.traversal.P.lt;
 import static org.apache.tinkerpop.gremlin.process.traversal.P.neq;
 import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.*;
 
@@ -124,12 +129,17 @@ public class TinkerGraphPlayTest {
     @Ignore
     public void testPlayDK() throws Exception {
 
-        final GraphTraversalSource g = TinkerFactory.createModern().traversal();
-        g.V().match(
-                __.as("b").out("created").as("c"),
-                __.as("a").hasLabel("person"),
-                __.as("b").hasLabel("person"),
-                __.as("a").out("knows").as("b")).forEachRemaining(System.out::println);
+        GraphTraversalSource g = TinkerFactory.createModern().traversal();
+        g./*withComputer().*/V().hasLabel("person")
+                .project("name", "age", "comments")
+                    .by("name")
+                    .by("age")
+                    .by(branch(values("age"))
+                            .option(29, constant("almost old"))
+                            .option(lt(29), constant("pretty young"))
+                            .option(lt(35), constant("younger than peter"))
+                            .option(gte(30), constant("pretty old")).fold())
+                .forEachRemaining(System.out::println);
     }
 
     @Test