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
