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/11/03 11:50:38 UTC
[11/45] tinkerpop git commit: added OptionalStep to be used instead
of optional() being backed by ChooseStep. This fixes a side-effect issue.
added OptionalStep to be used instead of optional() being backed by ChooseStep. This fixes a side-effect issue.
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/40971c95
Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/40971c95
Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/40971c95
Branch: refs/heads/TINKERPOP-1420
Commit: 40971c95903077d38cb754843eb914b5548f5c1f
Parents: ce1099d
Author: Marko A. Rodriguez <ok...@gmail.com>
Authored: Mon Oct 31 12:03:33 2016 -0600
Committer: Marko A. Rodriguez <ok...@gmail.com>
Committed: Mon Oct 31 12:03:33 2016 -0600
----------------------------------------------------------------------
CHANGELOG.asciidoc | 1 +
docs/src/upgrade/release-3.3.x.asciidoc | 10 ++-
.../traversal/dsl/graph/GraphTraversal.java | 7 +-
.../traversal/step/branch/OptionalStep.java | 92 ++++++++++++++++++++
.../traversal/step/branch/OptionalStepTest.java | 49 +++++++++++
.../step/branch/GroovyOptionalTest.groovy | 4 +
.../traversal/step/branch/OptionalTest.java | 31 +++++--
7 files changed, 184 insertions(+), 10 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/40971c95/CHANGELOG.asciidoc
----------------------------------------------------------------------
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 344cb51..eb06f74 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -26,6 +26,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
TinkerPop 3.3.0 (Release Date: NOT OFFICIALLY RELEASED YET)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+* Added `OptionalStep` for use with `optional()` to better handle issues associated with branch side-effects.
* Removed all performance tests that were not part of `gremlin-benchmark`.
* Removed dependency on `junit-benchmarks` and it's related reference to `h2`.
* Moved the source for the "home page" into the repository under `/site` so that it easier to accept contributions.
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/40971c95/docs/src/upgrade/release-3.3.x.asciidoc
----------------------------------------------------------------------
diff --git a/docs/src/upgrade/release-3.3.x.asciidoc b/docs/src/upgrade/release-3.3.x.asciidoc
index 6a4496a..2937009 100644
--- a/docs/src/upgrade/release-3.3.x.asciidoc
+++ b/docs/src/upgrade/release-3.3.x.asciidoc
@@ -32,6 +32,14 @@ Please see the link:https://github.com/apache/tinkerpop/blob/3.3.3/CHANGELOG.asc
Upgrading for Users
~~~~~~~~~~~~~~~~~~~
+OptionalStep and Side-Effects
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+The `optional()`-step was previously implemented using `ChooseStep`. However, if the optional branch contained side-effects,
+then unexpected behaviors can emerge. Thus, a potential backwards compatibility issue arises is side-effects were being
+used in `optional()`. However, the behavior would be unpredictable so this backwards incompatibility is desirable.
+
+See link:https://issues.apache.org/jira/browse/TINKERPOP-1506[TINKERPOP-1506]
Upgrading for Providers
~~~~~~~~~~~~~~~~~~~~~~~
@@ -51,4 +59,4 @@ concerned with breaking changes related to the removal of:
Those graph providers who relied on these tests should simply remove them from their respective test suites.
-See: https://issues.apache.org/jira/browse/TINKERPOP-1235[TINKERPOP-1235]
+See: link:https://issues.apache.org/jira/browse/TINKERPOP-1235[TINKERPOP-1235]
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/40971c95/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java
index c8af8ca..dca3b7f 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java
@@ -42,6 +42,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalOptionParent
import org.apache.tinkerpop.gremlin.process.traversal.step.branch.BranchStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.branch.ChooseStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.branch.LocalStep;
+import org.apache.tinkerpop.gremlin.process.traversal.step.branch.OptionalStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.branch.RepeatStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.branch.UnionStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.filter.AndStep;
@@ -1306,17 +1307,17 @@ public interface GraphTraversal<S, E> extends Traversal<S, E> {
public default <E2> GraphTraversal<S, E2> optional(final Traversal<?, E2> optionalTraversal) {
this.asAdmin().getBytecode().addStep(Symbols.optional, optionalTraversal);
- return this.asAdmin().addStep(new ChooseStep<>(this.asAdmin(), (Traversal.Admin<E, ?>) optionalTraversal, (Traversal.Admin<E, E2>) optionalTraversal.asAdmin().clone(), (Traversal.Admin<E, E2>) __.<E2>identity()));
+ return this.asAdmin().addStep(new OptionalStep<>(this.asAdmin(), (Traversal.Admin<E2, E2>) optionalTraversal));
}
public default <E2> GraphTraversal<S, E2> union(final Traversal<?, E2>... unionTraversals) {
this.asAdmin().getBytecode().addStep(Symbols.union, unionTraversals);
- return this.asAdmin().addStep(new UnionStep(this.asAdmin(), Arrays.copyOf(unionTraversals, unionTraversals.length, Traversal.Admin[].class)));
+ return this.asAdmin().addStep(new UnionStep<>(this.asAdmin(), Arrays.copyOf(unionTraversals, unionTraversals.length, Traversal.Admin[].class)));
}
public default <E2> GraphTraversal<S, E2> coalesce(final Traversal<?, E2>... coalesceTraversals) {
this.asAdmin().getBytecode().addStep(Symbols.coalesce, coalesceTraversals);
- return this.asAdmin().addStep(new CoalesceStep(this.asAdmin(), Arrays.copyOf(coalesceTraversals, coalesceTraversals.length, Traversal.Admin[].class)));
+ return this.asAdmin().addStep(new CoalesceStep<>(this.asAdmin(), Arrays.copyOf(coalesceTraversals, coalesceTraversals.length, Traversal.Admin[].class)));
}
public default GraphTraversal<S, E> repeat(final Traversal<?, E> repeatTraversal) {
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/40971c95/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/OptionalStep.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/OptionalStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/OptionalStep.java
new file mode 100644
index 0000000..2db92da
--- /dev/null
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/OptionalStep.java
@@ -0,0 +1,92 @@
+/*
+ * 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.step.branch;
+
+import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
+import org.apache.tinkerpop.gremlin.process.traversal.Traverser;
+import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalParent;
+import org.apache.tinkerpop.gremlin.process.traversal.step.util.AbstractStep;
+import org.apache.tinkerpop.gremlin.process.traversal.traverser.TraverserRequirement;
+import org.apache.tinkerpop.gremlin.structure.util.StringFactory;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.NoSuchElementException;
+import java.util.Set;
+
+/**
+ * @author Marko A. Rodriguez (http://markorodriguez.com)
+ */
+public final class OptionalStep<S> extends AbstractStep<S, S> implements TraversalParent {
+
+ private Traversal.Admin<S, S> optionalTraversal;
+
+ public OptionalStep(final Traversal.Admin traversal, final Traversal.Admin<S, S> optionalTraversal) {
+ super(traversal);
+ this.optionalTraversal = optionalTraversal;
+ }
+
+ @Override
+ protected Traverser.Admin<S> processNextStart() throws NoSuchElementException {
+ if (this.optionalTraversal.hasNext())
+ return this.optionalTraversal.nextTraverser();
+ else {
+ final Traverser.Admin<S> traverser = this.starts.next();
+ this.optionalTraversal.addStart(traverser.split());
+ if (this.optionalTraversal.hasNext())
+ return this.optionalTraversal.nextTraverser();
+ else
+ return traverser;
+ }
+ }
+
+ @Override
+ public List<Traversal.Admin<S, S>> getLocalChildren() {
+ return Collections.singletonList(this.optionalTraversal);
+ }
+
+ @Override
+ public OptionalStep<S> clone() {
+ final OptionalStep<S> clone = (OptionalStep<S>) super.clone();
+ clone.optionalTraversal = this.optionalTraversal.clone();
+ return clone;
+ }
+
+ @Override
+ public void setTraversal(final Traversal.Admin<?, ?> parentTraversal) {
+ super.setTraversal(parentTraversal);
+ integrateChild(this.optionalTraversal);
+ }
+
+ @Override
+ public String toString() {
+ return StringFactory.stepString(this, this.optionalTraversal);
+ }
+
+ @Override
+ public int hashCode() {
+ return super.hashCode() ^ this.optionalTraversal.hashCode();
+ }
+
+ @Override
+ public Set<TraverserRequirement> getRequirements() {
+ return this.optionalTraversal.getTraverserRequirements();
+ }
+}
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/40971c95/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/OptionalStepTest.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/OptionalStepTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/OptionalStepTest.java
new file mode 100644
index 0000000..f40c5e8
--- /dev/null
+++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/OptionalStepTest.java
@@ -0,0 +1,49 @@
+/*
+ * 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.step.branch;
+
+import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
+import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__;
+import org.apache.tinkerpop.gremlin.process.traversal.step.StepTest;
+
+import java.util.Arrays;
+import java.util.List;
+
+import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.identity;
+import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.in;
+import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.out;
+
+/**
+ * @author Marko A. Rodriguez (http://markorodriguez.com)
+ */
+public class OptionalStepTest extends StepTest {
+
+ @Override
+ protected List<Traversal> getTraversals() {
+ return Arrays.asList(
+ __.optional(out()),
+ __.optional(out("knows")),
+ __.optional(in().out()),
+ __.optional(out("created")),
+ __.optional(in().in()),
+ __.optional(identity())
+ );
+ }
+}
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/40971c95/gremlin-groovy-test/src/main/groovy/org/apache/tinkerpop/gremlin/process/traversal/step/branch/GroovyOptionalTest.groovy
----------------------------------------------------------------------
diff --git a/gremlin-groovy-test/src/main/groovy/org/apache/tinkerpop/gremlin/process/traversal/step/branch/GroovyOptionalTest.groovy b/gremlin-groovy-test/src/main/groovy/org/apache/tinkerpop/gremlin/process/traversal/step/branch/GroovyOptionalTest.groovy
index d3f04cc..f463a5e 100644
--- a/gremlin-groovy-test/src/main/groovy/org/apache/tinkerpop/gremlin/process/traversal/step/branch/GroovyOptionalTest.groovy
+++ b/gremlin-groovy-test/src/main/groovy/org/apache/tinkerpop/gremlin/process/traversal/step/branch/GroovyOptionalTest.groovy
@@ -50,6 +50,10 @@ public abstract class GroovyOptionalTest {
new ScriptTraversal<>(g, "gremlin-groovy", "g.V.optional(out().optional(out())).path")
}
+ @Override
+ public Traversal<Vertex, String> get_g_VX1X_optionalXaddVXdogXX_label(Object v1Id) {
+ new ScriptTraversal<>(g, "gremlin-groovy", "g.V(v1Id).optional(addV('dog')).label", "v1Id", v1Id)
+ }
}
}
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/40971c95/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/OptionalTest.java
----------------------------------------------------------------------
diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/OptionalTest.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/OptionalTest.java
index 660af6c..cf252f1 100644
--- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/OptionalTest.java
+++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/OptionalTest.java
@@ -18,23 +18,26 @@
*/
package org.apache.tinkerpop.gremlin.process.traversal.step.branch;
+import org.apache.tinkerpop.gremlin.FeatureRequirement;
import org.apache.tinkerpop.gremlin.LoadGraphWith;
import org.apache.tinkerpop.gremlin.process.AbstractGremlinProcessTest;
import org.apache.tinkerpop.gremlin.process.GremlinProcessRunner;
-import org.apache.tinkerpop.gremlin.process.IgnoreEngine;
import org.apache.tinkerpop.gremlin.process.traversal.Path;
import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
-import org.apache.tinkerpop.gremlin.process.traversal.TraversalEngine;
+import org.apache.tinkerpop.gremlin.structure.Graph;
import org.apache.tinkerpop.gremlin.structure.Vertex;
+import org.apache.tinkerpop.gremlin.util.iterator.IteratorUtils;
import org.junit.Test;
import org.junit.runner.RunWith;
import java.util.Arrays;
+import java.util.Collections;
import java.util.List;
import java.util.Optional;
import java.util.function.Predicate;
import static org.apache.tinkerpop.gremlin.LoadGraphWith.GraphData.MODERN;
+import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.addV;
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.junit.Assert.assertEquals;
@@ -54,6 +57,8 @@ public abstract class OptionalTest extends AbstractGremlinProcessTest {
public abstract Traversal<Vertex, Path> get_g_V_optionalXout_optionalXoutXX_path();
+ public abstract Traversal<Vertex, String> get_g_VX1X_optionalXaddVXdogXX_label(Object v1Id);
+
@Test
@LoadGraphWith(MODERN)
public void g_VX2X_optionalXoutXknowsXX() {
@@ -122,27 +127,41 @@ public abstract class OptionalTest extends AbstractGremlinProcessTest {
assertTrue(paths.isEmpty());
}
+ @Test
+ @LoadGraphWith(MODERN)
+ @FeatureRequirement(featureClass = Graph.Features.VertexFeatures.class, feature = Graph.Features.VertexFeatures.FEATURE_ADD_VERTICES)
+ public void g_VX1X_optionalXaddVXdogXX_label() {
+ final Traversal<Vertex, String> traversal = get_g_VX1X_optionalXaddVXdogXX_label(convertToVertexId(this.graph, "marko"));
+ printTraversalForm(traversal);
+ checkResults(Collections.singletonList("dog"), traversal);
+ assertEquals(7, IteratorUtils.count(graph.vertices()));
+ }
+
public static class Traversals extends OptionalTest {
@Override
public Traversal<Vertex, Vertex> get_g_VX2X_optionalXoutXknowsXX(Object v2Id) {
- return this.g.V(v2Id).optional(out("knows"));
+ return g.V(v2Id).optional(out("knows"));
}
@Override
public Traversal<Vertex, Vertex> get_g_VX2X_optionalXinXknowsXX(Object v2Id) {
- return this.g.V(v2Id).optional(in("knows"));
+ return g.V(v2Id).optional(in("knows"));
}
@Override
public Traversal<Vertex, Path> get_g_V_hasLabelXpersonX_optionalXoutXknowsX_optionalXoutXcreatedXXX_path() {
- return this.g.V().hasLabel("person").optional(out("knows").optional(out("created"))).path();
+ return g.V().hasLabel("person").optional(out("knows").optional(out("created"))).path();
}
@Override
public Traversal<Vertex, Path> get_g_V_optionalXout_optionalXoutXX_path() {
- return this.g.V().optional(out().optional(out())).path();
+ return g.V().optional(out().optional(out())).path();
}
+ @Override
+ public Traversal<Vertex, String> get_g_VX1X_optionalXaddVXdogXX_label(Object v1Id) {
+ return g.V(v1Id).optional(addV("dog")).label();
+ }
}
}