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 2019/11/07 12:00:29 UTC

[tinkerpop] 01/01: TINKERPOP-2310 Traversal requires EmptyStep to be root level

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

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

commit a53b44fce2c9011e3fb95b16e5577d10dff6d7f8
Author: stephen <sp...@gmail.com>
AuthorDate: Thu Nov 7 06:54:26 2019 -0500

    TINKERPOP-2310 Traversal requires EmptyStep to be root level
---
 CHANGELOG.asciidoc                                          |  1 +
 docs/src/upgrade/release-3.5.x.asciidoc                     | 13 ++++++++++++-
 .../tinkerpop/gremlin/process/traversal/Traversal.java      | 13 ++++++-------
 .../gremlin/process/traversal/util/DefaultTraversal.java    |  6 +++---
 4 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 11a097b..571fa1f 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -31,6 +31,7 @@ This release also includes changes from <<release-3-4-3, 3.4.3>>.
 * Renamed `AggregateStep` to `AggregateGlobalStep`.
 * Renamed `SERVER_ERROR_SCRIPT_EVALUATION` to `SERVER_ERROR_EVALUATION` given that this response code applies to remote traversals as well as scripts.
 * Refactored `TraversalStrategies` to implement `Iterable`.
+* Refactored `Traversal` semantics to always expect `EmptyStep` as a parent if it is meant to be the root `Traversal`.
 * Configured GraphBinary as the default binary serialization format for the Java Driver.
 * Configured GraphSON 3.0 as the default text serialization format when no serializer can be determined.
 * Upgraded to Neo4j 3.4.11.
diff --git a/docs/src/upgrade/release-3.5.x.asciidoc b/docs/src/upgrade/release-3.5.x.asciidoc
index 7372107..7cf27a0 100644
--- a/docs/src/upgrade/release-3.5.x.asciidoc
+++ b/docs/src/upgrade/release-3.5.x.asciidoc
@@ -240,7 +240,18 @@ relied on the old ordering of application. It may also present an opportunity to
 written to gain some processing benefit to the new order. Please be sure to review any custom strategies carefully
 when upgrading to this version.
 
-See: link:https://issues.apache.org/jira/browse/TINKERPOP-1568[TINKERPOP-1568]
+As part of this change, there have been some adjustments to the `Traversal` and `Traversal.Admin` interfaces which have
+helped to clarify coding intent. There is now an `isRoot()` method which determines whether or not the traversal has a
+parent or not. Under revised semantics for 3.5.0, a traversal's parent must be an `EmptyStep` instance and should not
+be `null`. With this change, provider `TraversalStrategy` implementations should be reviewed to evaluate if `isRoot()`
+semantics cause any breaks in logic to existing code.
+
+In addition, `TraversalStrategies` now implements `Iterable` and exposes an `iterator()` method which may be preferred
+over the old `toList()` style construction for getting the list of configured strategies.
+
+See: link:https://issues.apache.org/jira/browse/TINKERPOP-1568[TINKERPOP-1568],
+link:https://issues.apache.org/jira/browse/TINKERPOP-2310[TINKERPOP-2310],
+link:https://issues.apache.org/jira/browse/TINKERPOP-2311[TINKERPOP-2311]
 
 ===== AbstractOpProcessor API Change
 
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Traversal.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Traversal.java
index 393c1d2..9bd876d 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Traversal.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Traversal.java
@@ -480,18 +480,17 @@ public interface Traversal<S, E> extends Iterator<E>, Serializable, Cloneable, A
         /**
          * Set the {@link org.apache.tinkerpop.gremlin.process.traversal.step.TraversalParent} {@link Step} that is
          * the parent of this traversal. Traversals can be nested and this is the means by which the traversal tree is
-         * connected.
+         * connected. If there is no parent, then it should be a {@link EmptyStep}.
          *
-         * @param step the traversal holder parent step
+         * @param step the traversal holder parent step or {@link EmptyStep} if it has no parent
          */
         public void setParent(final TraversalParent step);
 
         /**
-         * Get the {@link org.apache.tinkerpop.gremlin.process.traversal.step.TraversalParent} {@link Step} that is
-         * the parent of this traversal. Traversals can be nested and this is the means by which the traversal tree is
-         * walked.
+         * Get the {@link TraversalParent} {@link Step} that is the parent of this traversal. Traversals can be nested
+         * and this is the means by which the traversal tree is walked.
          *
-         * @return the traversal holder parent step
+         * @return the traversal holder parent step or {@link EmptyStep} if it has no parent.
          */
         public TraversalParent getParent();
 
@@ -499,7 +498,7 @@ public interface Traversal<S, E> extends Iterator<E>, Serializable, Cloneable, A
          * Determines if the traversal is at the root level.
          */
         public default boolean isRoot() {
-            return null == getParent() || getParent() instanceof EmptyStep;
+            return getParent() instanceof EmptyStep;
         }
 
         /**
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversal.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversal.java
index 78b49eb..d2c13ab 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversal.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversal.java
@@ -207,7 +207,7 @@ public class DefaultTraversal<S, E> implements Traversal.Admin<S, E> {
                 return this.finalEndStep.next();
             }
         } catch (final FastNoSuchElementException e) {
-            throw this.parent instanceof EmptyStep ? new NoSuchElementException() : e;
+            throw this.isRoot() ? new NoSuchElementException() : e;
         }
     }
 
@@ -230,7 +230,7 @@ public class DefaultTraversal<S, E> implements Traversal.Admin<S, E> {
             // and release the resources.
             CloseableIterator.closeIterator(this);
 
-            throw this.parent instanceof EmptyStep ? new NoSuchElementException() : e;
+            throw this.isRoot() ? new NoSuchElementException() : e;
         }
     }
 
@@ -347,7 +347,7 @@ public class DefaultTraversal<S, E> implements Traversal.Admin<S, E> {
 
     @Override
     public void setParent(final TraversalParent step) {
-        this.parent = step;
+        this.parent = null == step ? EmptyStep.instance() : step;
     }
 
     @Override