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-1967
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

Reply via email to