From 3ae2814c277943b5fd363b7c8f5faceb1ec0dfe3 Mon Sep 17 00:00:00 2001 From: BrennanConroy Date: Tue, 2 Oct 2018 14:08:36 -0700 Subject: [PATCH] Make Java client more reusable (#3029) --- .../aspnet/signalr/HubConnection.java | 50 ++++++++----------- .../aspnet/signalr/HubConnectionTest.java | 39 ++++++++++++++- 2 files changed, 58 insertions(+), 31 deletions(-) diff --git a/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/HubConnection.java b/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/HubConnection.java index cc47949593..a0d76157be 100644 --- a/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/HubConnection.java +++ b/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/HubConnection.java @@ -16,7 +16,7 @@ import java.util.concurrent.locks.ReentrantLock; import java.util.function.Consumer; public class HubConnection { - private String url; + private String baseUrl; private Transport transport; private OnReceiveCallBack callback; private CallbackMap handlers = new CallbackMap(); @@ -41,7 +41,7 @@ public class HubConnection { throw new IllegalArgumentException("A valid url is required."); } - this.url = url; + this.baseUrl = url; this.protocol = new JsonHubProtocol(); if (logger != null) { @@ -126,7 +126,7 @@ public class HubConnection { }; } - private CompletableFuture handleNegotiate() throws IOException, InterruptedException, ExecutionException { + private CompletableFuture handleNegotiate(String url) throws IOException, InterruptedException, ExecutionException { HttpRequest request = new HttpRequest(); request.setHeaders(this.headers); @@ -142,22 +142,10 @@ public class HubConnection { throw new RuntimeException(negotiateResponse.getError()); } - if (negotiateResponse.getConnectionId() != null) { - if (url.contains("?")) { - url = url + "&id=" + negotiateResponse.getConnectionId(); - } else { - url = url + "?id=" + negotiateResponse.getConnectionId(); - } - } - if (negotiateResponse.getAccessToken() != null) { this.headers.put("Authorization", "Bearer " + negotiateResponse.getAccessToken()); } - if (negotiateResponse.getRedirectUrl() != null) { - this.url = negotiateResponse.getRedirectUrl(); - } - return CompletableFuture.completedFuture(negotiateResponse); }); } @@ -181,20 +169,14 @@ public class HubConnection { return CompletableFuture.completedFuture(null); } - CompletableFuture negotiate = null; + CompletableFuture negotiate = null; if (!skipNegotiate) { - negotiate = startNegotiate(0); + negotiate = startNegotiate(baseUrl, 0); } else { - negotiate = CompletableFuture.completedFuture(null); + negotiate = CompletableFuture.completedFuture(baseUrl); } - return negotiate.thenCompose((response) -> { - // If we didn't skip negotiate and got a null response then exit start because we - // are probably disconnected - if (response == null && !skipNegotiate) { - return CompletableFuture.completedFuture(null); - } - + return negotiate.thenCompose((url) -> { logger.log(LogLevel.Debug, "Starting HubConnection"); if (transport == null) { transport = new WebSocketTransport(url, headers, httpClient, logger); @@ -225,12 +207,12 @@ public class HubConnection { }); } - private CompletableFuture startNegotiate(int negotiateAttempts) throws IOException, InterruptedException, ExecutionException { + private CompletableFuture startNegotiate(String url, int negotiateAttempts) throws IOException, InterruptedException, ExecutionException { if (hubConnectionState != HubConnectionState.DISCONNECTED) { return CompletableFuture.completedFuture(null); } - return handleNegotiate().thenCompose((response) -> { + return handleNegotiate(url).thenCompose((response) -> { if (response.getRedirectUrl() != null && negotiateAttempts >= MAX_NEGOTIATE_ATTEMPTS) { throw new RuntimeException("Negotiate redirection limit exceeded."); } @@ -243,11 +225,21 @@ public class HubConnection { throw new RuntimeException(e); } } - return CompletableFuture.completedFuture(response); + + String finalUrl = url; + if (response.getConnectionId() != null) { + if (url.contains("?")) { + finalUrl = url + "&id=" + response.getConnectionId(); + } else { + finalUrl = url + "?id=" + response.getConnectionId(); + } + } + + return CompletableFuture.completedFuture(finalUrl); } try { - return startNegotiate(negotiateAttempts + 1); + return startNegotiate(response.getRedirectUrl(), negotiateAttempts + 1); } catch (IOException | InterruptedException | ExecutionException e) { throw new RuntimeException(e); } diff --git a/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/HubConnectionTest.java b/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/HubConnectionTest.java index f2321f6afa..9db3fca0a2 100644 --- a/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/HubConnectionTest.java +++ b/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/HubConnectionTest.java @@ -948,8 +948,7 @@ class HubConnectionTest { } @Test - public void negotiateRedirectIsFollowed() - throws InterruptedException, ExecutionException, TimeoutException, Exception { + public void negotiateRedirectIsFollowed() throws Exception { TestHttpClient client = new TestHttpClient().on("POST", "http://example.com/negotiate", (req) -> CompletableFuture.completedFuture(new HttpResponse(200, "", "{\"url\":\"http://testexample.com/\"}"))) .on("POST", "http://testexample.com/negotiate", @@ -966,4 +965,40 @@ class HubConnectionTest { assertEquals(HubConnectionState.CONNECTED, hubConnection.getConnectionState()); hubConnection.stop(); } + + @Test + public void hubConnectionCanBeStartedAfterBeingStopped() throws Exception { + MockTransport mockTransport = new MockTransport(); + HubConnection hubConnection = new HubConnection("http://example.com", mockTransport, new NullLogger(), true, new TestHttpClient()); + + hubConnection.start().get(1000, TimeUnit.MILLISECONDS); + assertEquals(HubConnectionState.CONNECTED, hubConnection.getConnectionState()); + hubConnection.stop().get(1000, TimeUnit.MILLISECONDS); + assertEquals(HubConnectionState.DISCONNECTED, hubConnection.getConnectionState()); + hubConnection.start().get(1000, TimeUnit.MILLISECONDS); + assertEquals(HubConnectionState.CONNECTED, hubConnection.getConnectionState()); + hubConnection.stop().get(1000, TimeUnit.MILLISECONDS); + assertEquals(HubConnectionState.DISCONNECTED, hubConnection.getConnectionState()); + } + + @Test + public void hubConnectionCanBeStartedAfterBeingStoppedAndRedirected() throws Exception { + MockTransport mockTransport = new MockTransport(); + TestHttpClient client = new TestHttpClient() + .on("POST", "http://example.com/negotiate", (req) -> CompletableFuture + .completedFuture(new HttpResponse(200, "", "{\"url\":\"http://testexample.com/\"}"))) + .on("POST", "http://testexample.com/negotiate", (req) -> CompletableFuture + .completedFuture(new HttpResponse(200, "", "{\"connectionId\":\"bVOiRPG8-6YiJ6d7ZcTOVQ\",\"" + + "availableTransports\":[{\"transport\":\"WebSockets\",\"transferFormats\":[\"Text\",\"Binary\"]}]}"))); + HubConnection hubConnection = new HubConnection("http://example.com", mockTransport, new NullLogger(), false, client); + + hubConnection.start().get(1000, TimeUnit.MILLISECONDS); + assertEquals(HubConnectionState.CONNECTED, hubConnection.getConnectionState()); + hubConnection.stop().get(1000, TimeUnit.MILLISECONDS); + assertEquals(HubConnectionState.DISCONNECTED, hubConnection.getConnectionState()); + hubConnection.start().get(1000, TimeUnit.MILLISECONDS); + assertEquals(HubConnectionState.CONNECTED, hubConnection.getConnectionState()); + hubConnection.stop().get(1000, TimeUnit.MILLISECONDS); + assertEquals(HubConnectionState.DISCONNECTED, hubConnection.getConnectionState()); + } } \ No newline at end of file