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 2018/07/26 17:23:52 UTC

[45/50] tinkerpop git commit: Throw exception if withRemote() is called multiple times during configuration

Throw exception if withRemote() is called multiple times during configuration

This is a change in behavior. Prior to this we would have quietly closed the connection, but that's a little too sneaky. Also, the connection was being assigned to the current TraversalSource as well as the clone. Not sure why that was happening as the clone was really the only thing that would use it after the call to withRemote(), so the construction was also corrected. CTR


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

Branch: refs/heads/TINKERPOP-1913
Commit: 0bd9b5ae7a1bcfda27541399996a2eaf4e421908
Parents: 3be3550
Author: Stephen Mallette <sp...@genoprime.com>
Authored: Thu Jul 26 08:25:02 2018 -0400
Committer: Stephen Mallette <sp...@genoprime.com>
Committed: Thu Jul 26 08:25:02 2018 -0400

----------------------------------------------------------------------
 CHANGELOG.asciidoc                                |  1 +
 .../traversal/dsl/graph/GraphTraversalSource.java | 18 +++++++-----------
 .../dsl/graph/GraphTraversalSourceTest.java       |  9 ++-------
 3 files changed, 10 insertions(+), 18 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/0bd9b5ae/CHANGELOG.asciidoc
----------------------------------------------------------------------
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 0f2b18b..b6e85ce 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -28,6 +28,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
 * Fixed bug in Java driver where an disorderly shutdown of the server would cause the client to hang.
 * Added a dotnet template project that should make it easier to get started with Gremlin.Net.
 * Removed `ThreadInterruptCustomizerProvider` from documentation as a way to timeout.
+* Changed behavior of `withRemote()` if called multiple times so as to simply throw an exception and not perform the side-effect of auto-closing.
 * Added Docker images for Gremlin Console and Gremlin Server.
 * Fixed bug in `branch()` where reducing steps as options would produce incorrect results.
 * Removed recursive handling of streaming results from Gremlin-Python driver to avoid max recursion depth errors.

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/0bd9b5ae/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalSource.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalSource.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalSource.java
index baba19c..faf9459 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalSource.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalSource.java
@@ -302,19 +302,15 @@ public class GraphTraversalSource implements TraversalSource {
      */
     @Override
     public GraphTraversalSource withRemote(final RemoteConnection connection) {
-        try {
-            // check if someone called withRemote() more than once, so just release resources on the initial
-            // connection as you can't have more than one. maybe better to toss IllegalStateException??
-            if (this.connection != null)
-                this.connection.close();
-        } catch (Exception ignored) {
-            // not sure there's anything to do here
-        }
+        // check if someone called withRemote() more than once, so just release resources on the initial
+        // connection as you can't have more than one. maybe better to toss IllegalStateException??
+        if (this.connection != null)
+            throw new IllegalStateException(String.format("TraversalSource already configured with a RemoteConnection [%s]", connection));
 
-        this.connection = connection;
-        final TraversalSource clone = this.clone();
+        final GraphTraversalSource clone = this.clone();
+        clone.connection = connection;
         clone.getStrategies().addStrategies(new RemoteStrategy(connection));
-        return (GraphTraversalSource) clone;
+        return clone;
     }
 
     //// SPAWNS

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/0bd9b5ae/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalSourceTest.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalSourceTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalSourceTest.java
index 7e7d777..a645089 100644
--- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalSourceTest.java
+++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalSourceTest.java
@@ -47,16 +47,11 @@ public class GraphTraversalSourceTest {
         verify(mock, times(1)).close();
     }
 
-    @Test
+    @Test(expected = IllegalStateException.class)
     public void shouldNotAllowLeakRemoteConnectionsIfMultipleAreCreated() throws Exception {
-
         final RemoteConnection mock1 = mock(RemoteConnection.class);
         final RemoteConnection mock2 = mock(RemoteConnection.class);
-        final GraphTraversalSource g = EmptyGraph.instance().traversal().withRemote(mock1).withRemote(mock2);
-        g.close();
-
-        verify(mock1, times(1)).close();
-        verify(mock2, times(1)).close();
+        EmptyGraph.instance().traversal().withRemote(mock1).withRemote(mock2);
     }
 
     @Test