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 14:47:21 UTC
[tinkerpop] branch TINKERPOP-1084 updated: TINKERPOP-1084 Allow
predicates and traversals to be used as options in `BranchStep`.
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
The following commit(s) were added to refs/heads/TINKERPOP-1084 by this push:
new 0398aaf TINKERPOP-1084 Allow predicates and traversals to be used as options in `BranchStep`.
0398aaf is described below
commit 0398aafdeddbd57296fe74a9c634b12e47972a89
Author: Daniel Kuppitz <da...@hotmail.com>
AuthorDate: Tue Jun 11 07:46:51 2019 -0700
TINKERPOP-1084 Allow predicates and traversals to be used as options in `BranchStep`.
---
CHANGELOG.asciidoc | 2 +-
.../traversal/lambda/PredicateTraversal.java | 68 +++++++++
.../process/traversal/step/branch/BranchStep.java | 163 +++++++--------------
.../process/traversal/step/branch/ChooseStep.java | 10 +-
.../process/traversal/step/branch/UnionStep.java | 2 +-
.../process/traversal/step/ComplexTest.java | 4 +-
.../tinkergraph/structure/TinkerGraphPlayTest.java | 8 +-
7 files changed, 136 insertions(+), 121 deletions(-)
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index d1b177f..f244cb1 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -26,7 +26,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
* Bump to Groovy 2.4.17.
* Bump to Jackson 2.9.9.
* Improved error messaging when an attempt is made to serialize multi-properties to GraphML.
-* Allow predicates to be used as options in BranchSteps.
+* Allow predicates and traversals to be used as options in `BranchStep`.
[[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/lambda/PredicateTraversal.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/PredicateTraversal.java
new file mode 100644
index 0000000..aa6dab0
--- /dev/null
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/PredicateTraversal.java
@@ -0,0 +1,68 @@
+/*
+ * 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.process.traversal.lambda;
+
+import org.apache.tinkerpop.gremlin.process.traversal.P;
+import org.apache.tinkerpop.gremlin.process.traversal.Traverser;
+import org.apache.tinkerpop.gremlin.process.traversal.util.FastNoSuchElementException;
+
+import java.util.function.Predicate;
+
+/**
+ * @author Marko A. Rodriguez (http://markorodriguez.com)
+ */
+public final class PredicateTraversal<S> extends AbstractLambdaTraversal<S, S> {
+
+ private final Predicate predicate;
+ private S s;
+ private boolean pass;
+
+ public PredicateTraversal(final Object value) {
+ this.predicate = value instanceof Predicate ? (Predicate) value : P.eq(value);
+ }
+
+ @Override
+ public S next() {
+ if (this.pass)
+ return this.s;
+ throw FastNoSuchElementException.instance();
+ }
+
+ @Override
+ public boolean hasNext() {
+ return this.pass;
+ }
+
+ @Override
+ public void addStart(final Traverser.Admin<S> start) {
+ //noinspection unchecked
+ this.pass = this.predicate.test(this.s = start.get());
+ }
+
+ @Override
+ public String toString() {
+ return "(" + this.predicate.toString() + ")";
+ }
+
+ @Override
+ public int hashCode() {
+ return this.getClass().hashCode() ^ this.predicate.hashCode();
+ }
+}
+
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 efc43cc..d92ce2d 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
@@ -18,8 +18,10 @@
*/
package org.apache.tinkerpop.gremlin.process.traversal.step.branch;
+import org.apache.tinkerpop.gremlin.process.traversal.P;
import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
import org.apache.tinkerpop.gremlin.process.traversal.Traverser;
+import org.apache.tinkerpop.gremlin.process.traversal.lambda.PredicateTraversal;
import org.apache.tinkerpop.gremlin.process.traversal.step.Barrier;
import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalOptionParent;
import org.apache.tinkerpop.gremlin.process.traversal.step.sideEffect.IdentityStep;
@@ -30,7 +32,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.util.FastNoSuchElementExce
import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalHelper;
import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalUtil;
import org.apache.tinkerpop.gremlin.structure.util.StringFactory;
-import org.apache.tinkerpop.gremlin.util.NumberHelper;
+import org.javatuples.Pair;
import java.util.ArrayList;
import java.util.Collections;
@@ -38,10 +40,10 @@ 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;
+import java.util.stream.Stream;
/**
* @author Marko A. Rodriguez (http://markorodriguez.com)
@@ -50,11 +52,11 @@ import java.util.stream.Collectors;
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<>();
+ protected Map<Pick, List<Traversal.Admin<S, E>>> traversalPickOptions = new HashMap<>();
+ protected List<Pair<Traversal.Admin, Traversal.Admin<S, E>>> traversalOptions = new ArrayList<>();
private boolean first = true;
private boolean hasBarrier;
- private boolean hasPredicateOptions;
public BranchStep(final Traversal.Admin traversal) {
super(traversal);
@@ -66,13 +68,20 @@ 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 = makePickTokenKey(pickToken);
- this.hasPredicateOptions |= pickTokenKey instanceof PredicateOption;
- if (this.traversalOptions.containsKey(pickTokenKey))
- this.traversalOptions.get(pickTokenKey).add(traversalOption);
- else
- this.traversalOptions.put(pickTokenKey, new ArrayList<>(Collections.singletonList(traversalOption)));
-
+ if (pickToken instanceof Pick) {
+ if (this.traversalPickOptions.containsKey(pickToken))
+ this.traversalPickOptions.get(pickToken).add(traversalOption);
+ else
+ this.traversalPickOptions.put((Pick) pickToken, new ArrayList<>(Collections.singletonList(traversalOption)));
+ } else {
+ final Traversal.Admin pickOptionTraversal;
+ if (pickToken instanceof Traversal) {
+ pickOptionTraversal = ((Traversal) pickToken).asAdmin();
+ } else {
+ pickOptionTraversal = new PredicateTraversal(pickToken);
+ }
+ this.traversalOptions.add(Pair.with(pickOptionTraversal, traversalOption));
+ }
// adding an IdentityStep acts as a placeholder when reducing barriers get in the way - see the
// standardAlgorithm() method for more information.
if (TraversalHelper.hasStepOfAssignableClass(ReducingBarrierStep.class, traversalOption))
@@ -91,9 +100,9 @@ public class BranchStep<S, E, M> extends ComputerAwareStep<S, E> implements Trav
@Override
public List<Traversal.Admin<S, E>> getGlobalChildren() {
- return Collections.unmodifiableList(this.traversalOptions.values().stream()
- .flatMap(List::stream)
- .collect(Collectors.toList()));
+ return Collections.unmodifiableList(Stream.concat(
+ this.traversalPickOptions.values().stream().flatMap(List::stream),
+ this.traversalOptions.stream().map(Pair::getValue1)).collect(Collectors.toList()));
}
@Override
@@ -113,11 +122,9 @@ public class BranchStep<S, E, M> extends ComputerAwareStep<S, E> implements Trav
// traverser as part of the condition for using that traversal option in the output. This is necessary
// because barriers like fold(), max(), etc. will always return true for hasNext() even if a traverser
// was not seeded in applyCurrentTraverser().
- for (final List<Traversal.Admin<S, E>> options : this.traversalOptions.values()) {
- for (final Traversal.Admin<S, E> option : options) {
- if (option.getStartStep().hasNext() && option.hasNext())
- return option.getEndStep();
- }
+ for (final Traversal.Admin<S, E> option : getGlobalChildren()) {
+ if (option.getStartStep().hasNext() && option.hasNext())
+ return option.getEndStep();
}
}
@@ -144,7 +151,7 @@ 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 = makePickTokenKey(TraversalUtil.apply(start, this.branchTraversal));
+ final Object choice = 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
@@ -153,7 +160,7 @@ public class BranchStep<S, E, M> extends ComputerAwareStep<S, E> implements Trav
branches.forEach(traversal -> traversal.addStart(start.split()));
if (choice != Pick.any) {
- final List<Traversal.Admin<S, E>> anyBranch = this.traversalOptions.get(Pick.any);
+ final List<Traversal.Admin<S, E>> anyBranch = this.traversalPickOptions.get(Pick.any);
if (null != anyBranch)
anyBranch.forEach(traversal -> traversal.addStart(start.split()));
}
@@ -163,7 +170,7 @@ 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 = makePickTokenKey(TraversalUtil.apply(start, this.branchTraversal));
+ final Object choice = TraversalUtil.apply(start, this.branchTraversal);
final List<Traversal.Admin<S, E>> branches = pickBranches(choice);
if (null != branches) {
branches.forEach(traversal -> {
@@ -174,7 +181,7 @@ public class BranchStep<S, E, M> extends ComputerAwareStep<S, E> implements Trav
});
}
if (choice != Pick.any) {
- final List<Traversal.Admin<S, E>> anyBranch = this.traversalOptions.get(Pick.any);
+ final List<Traversal.Admin<S, E>> anyBranch = this.traversalPickOptions.get(Pick.any);
if (null != anyBranch) {
anyBranch.forEach(traversal -> {
final Traverser.Admin<E> split = (Traverser.Admin<E>) start.split();
@@ -189,34 +196,37 @@ public class BranchStep<S, E, M> extends ComputerAwareStep<S, E> implements Trav
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());
- }
+ if (choice instanceof Pick) {
+ if (this.traversalPickOptions.containsKey(choice)) {
+ branches.addAll(this.traversalPickOptions.get(choice));
}
- } else {
- if (this.traversalOptions.containsKey(choice)) {
- branches.addAll(this.traversalOptions.get(choice));
+ }
+ for (final Pair<Traversal.Admin, Traversal.Admin<S, E>> p : this.traversalOptions) {
+ if (TraversalUtil.test(choice, p.getValue0())) {
+ branches.add(p.getValue1());
}
}
- return branches.isEmpty() ? this.traversalOptions.get(Pick.none) : branches;
+ return branches.isEmpty() ? this.traversalPickOptions.get(Pick.none) : branches;
}
@Override
public BranchStep<S, E, M> clone() {
final BranchStep<S, E, M> clone = (BranchStep<S, E, M>) super.clone();
- clone.traversalOptions = new HashMap<>(this.traversalOptions.size());
- for (final Map.Entry<Object, List<Traversal.Admin<S, E>>> entry : this.traversalOptions.entrySet()) {
+ clone.traversalPickOptions = new HashMap<>(this.traversalPickOptions.size());
+ clone.traversalOptions = new ArrayList<>(this.traversalOptions.size());
+ for (final Map.Entry<Pick, List<Traversal.Admin<S, E>>> entry : this.traversalPickOptions.entrySet()) {
final List<Traversal.Admin<S, E>> traversals = entry.getValue();
if (traversals.size() > 0) {
- final List<Traversal.Admin<S, E>> clonedTraversals = clone.traversalOptions.compute(entry.getKey(), (k, v) ->
+ final List<Traversal.Admin<S, E>> clonedTraversals = clone.traversalPickOptions.compute(entry.getKey(), (k, v) ->
(v == null) ? new ArrayList<>(traversals.size()) : v);
for (final Traversal.Admin<S, E> traversal : traversals) {
clonedTraversals.add(traversal.clone());
}
}
}
+ for (final Pair<Traversal.Admin, Traversal.Admin<S, E>> pair : this.traversalOptions) {
+ clone.traversalOptions.add(Pair.with(pair.getValue0().clone(), pair.getValue1().clone()));
+ }
clone.branchTraversal = this.branchTraversal.clone();
return clone;
}
@@ -225,12 +235,14 @@ public class BranchStep<S, E, M> extends ComputerAwareStep<S, E> implements Trav
public void setTraversal(final Traversal.Admin<?, ?> parentTraversal) {
super.setTraversal(parentTraversal);
this.integrateChild(this.branchTraversal);
- this.traversalOptions.values().stream().flatMap(List::stream).forEach(this::integrateChild);
+ this.getGlobalChildren().forEach(this::integrateChild);
}
@Override
public int hashCode() {
int result = super.hashCode();
+ if (this.traversalPickOptions != null)
+ result ^= this.traversalPickOptions.hashCode();
if (this.traversalOptions != null)
result ^= this.traversalOptions.hashCode();
if (this.branchTraversal != null)
@@ -240,7 +252,10 @@ public class BranchStep<S, E, M> extends ComputerAwareStep<S, E> implements Trav
@Override
public String toString() {
- return StringFactory.stepString(this, this.branchTraversal, this.traversalOptions);
+ final List<Pair> combinedOptions = Stream.concat(
+ this.traversalPickOptions.entrySet().stream().map(e -> Pair.with(e.getKey(), e.getValue())),
+ this.traversalOptions.stream()).collect(Collectors.toList());
+ return StringFactory.stepString(this, this.branchTraversal, combinedOptions);
}
@Override
@@ -249,78 +264,4 @@ public class BranchStep<S, E, M> extends ComputerAwareStep<S, E> implements Trav
this.getGlobalChildren().forEach(Traversal.Admin::reset);
this.first = true;
}
-
- /**
- * 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 NumberOption implements Comparable<Number> {
-
- final Number number;
-
- NumberOption(final Number number) {
- this.number = number;
- }
-
- @Override
- public boolean equals(final Object o) {
- if (this == o) return true;
- if (o == null || getClass() != o.getClass()) return false;
- final NumberOption other = (NumberOption) o;
- return 0 == NumberHelper.compare(number, other.number);
- }
-
- @Override
- public int hashCode() {
- return number.hashCode();
- }
-
- @Override
- 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-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/ChooseStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/ChooseStep.java
index e699f22..a804d8f 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/ChooseStep.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/ChooseStep.java
@@ -42,10 +42,12 @@ public final class ChooseStep<S, E, M> extends BranchStep<S, E, M> {
@Override
public void addGlobalChildOption(final M pickToken, final Traversal.Admin<S, E> traversalOption) {
- if (Pick.any.equals(pickToken))
- throw new IllegalArgumentException("Choose step can not have an any-option as only one option per traverser is allowed");
- if (this.traversalOptions.containsKey(pickToken))
- throw new IllegalArgumentException("Choose step can only have one traversal per pick token: " + pickToken);
+ if (pickToken instanceof Pick) {
+ if (Pick.any.equals(pickToken))
+ throw new IllegalArgumentException("Choose step can not have an any-option as only one option per traverser is allowed");
+ if (this.traversalPickOptions.containsKey(pickToken))
+ throw new IllegalArgumentException("Choose step can only have one traversal per pick token: " + pickToken);
+ }
super.addGlobalChildOption(pickToken, traversalOption);
}
}
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/UnionStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/UnionStep.java
index 85f3645..de70479 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/UnionStep.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/UnionStep.java
@@ -47,6 +47,6 @@ public final class UnionStep<S, E> extends BranchStep<S, E, TraversalOptionParen
@Override
public String toString() {
- return StringFactory.stepString(this, this.traversalOptions.getOrDefault(Pick.any, Collections.emptyList()));
+ return StringFactory.stepString(this, this.traversalPickOptions.getOrDefault(Pick.any, Collections.emptyList()));
}
}
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 a3bb1cd..592e945 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
@@ -252,9 +252,10 @@ public abstract class ComplexTest extends AbstractGremlinProcessTest {
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());
+ assertEquals(3, map.get("josh").size());
assertTrue(map.get("josh").contains("younger than peter"));
assertTrue(map.get("josh").contains("pretty old"));
+ assertTrue(map.get("josh").contains("looks like josh"));
assertTrue(map.containsKey("peter"));
assertEquals(1, map.get("peter").size());
assertTrue(map.get("peter").contains("pretty old"));
@@ -348,6 +349,7 @@ public abstract class ComplexTest extends AbstractGremlinProcessTest {
.by("name")
.by(branch(__.values("age"))
.option(29, constant("almost old"))
+ .option(__.is(32), constant("looks like josh"))
.option(lt(29), constant("pretty young"))
.option(lt(35), constant("younger than peter"))
.option(gte(30), constant("pretty old")).fold());
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 3b241bc..3216803 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
@@ -130,16 +130,18 @@ public class TinkerGraphPlayTest {
public void testPlayDK() throws Exception {
GraphTraversalSource g = TinkerFactory.createModern().traversal();
- g./*withComputer().*/V().hasLabel("person")
+ System.out.println(g./*withComputer().*/V().hasLabel("person")
.project("name", "age", "comments")
.by("name")
.by("age")
.by(branch(values("age"))
+ .option(TraversalOptionParent.Pick.any, constant("foo"))
.option(29, constant("almost old"))
+ .option(__.is(32), constant("looks like josh"))
.option(lt(29), constant("pretty young"))
.option(lt(35), constant("younger than peter"))
- .option(gte(30), constant("pretty old")).fold())
- .forEachRemaining(System.out::println);
+ .option(gte(30), constant("pretty old")).fold()).explain());
+ //.forEachRemaining(System.out::println);
}
@Test