From 529cfeeff8590d0a3c387298498daac4edbe44db Mon Sep 17 00:00:00 2001 From: Mikael Mengistu Date: Sat, 15 Sep 2018 16:43:16 -0400 Subject: [PATCH 1/9] Java Client API review cleanup (#2956) --- .../microsoft/aspnet/signalr/CallbackMap.java | 2 +- .../aspnet/signalr/HubConnection.java | 36 +++++++++---------- .../microsoft/aspnet/signalr/HubProtocol.java | 1 - .../aspnet/signalr/JsonHubProtocol.java | 6 ++-- .../microsoft/aspnet/signalr/PingMessage.java | 12 +++++-- .../aspnet/signalr/TransferFormat.java | 4 +-- .../aspnet/signalr/JsonHubProtocolTest.java | 6 ++-- 7 files changed, 34 insertions(+), 33 deletions(-) diff --git a/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/CallbackMap.java b/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/CallbackMap.java index 528aa8d327..8dbd13488c 100644 --- a/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/CallbackMap.java +++ b/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/CallbackMap.java @@ -5,9 +5,9 @@ package com.microsoft.aspnet.signalr; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.concurrent.ConcurrentHashMap; -import java.util.Collections; class CallbackMap { private ConcurrentHashMap> handlers = new ConcurrentHashMap<>(); 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 361920280c..417cd3f743 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 @@ -249,6 +249,24 @@ public class HubConnection { transport.send(message); } + /** + * Removes all handlers associated with the method with the specified method name. + * + * @param name The name of the hub method from which handlers are being removed. + */ + public void remove(String name) { + handlers.remove(name); + logger.log(LogLevel.Trace, "Removing handlers for client method %s", name); + } + + public void onClosed(Consumer callback) { + if (onClosedCallbackList == null) { + onClosedCallbackList = new ArrayList<>(); + } + + onClosedCallbackList.add(callback); + } + /** * Registers a handler that will be invoked when the hub method with the specified method name is invoked. * @@ -515,24 +533,6 @@ public class HubConnection { return new Subscription(handlers, handler, target); } - /** - * Removes all handlers associated with the method with the specified method name. - * - * @param name The name of the hub method from which handlers are being removed. - */ - public void remove(String name) { - handlers.remove(name); - logger.log(LogLevel.Trace, "Removing handlers for client method %s", name); - } - - public void onClosed(Consumer callback) { - if (onClosedCallbackList == null) { - onClosedCallbackList = new ArrayList<>(); - } - - onClosedCallbackList.add(callback); - } - private class ConnectionState implements InvocationBinder { HubConnection connection; diff --git a/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/HubProtocol.java b/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/HubProtocol.java index 82244730fd..a22105fc91 100644 --- a/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/HubProtocol.java +++ b/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/HubProtocol.java @@ -3,7 +3,6 @@ package com.microsoft.aspnet.signalr; -import java.io.IOException; /** * A protocol abstraction for communicating with SignalR hubs. diff --git a/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/JsonHubProtocol.java b/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/JsonHubProtocol.java index 29ed3a5a89..bf04c52e83 100644 --- a/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/JsonHubProtocol.java +++ b/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/JsonHubProtocol.java @@ -3,7 +3,6 @@ package com.microsoft.aspnet.signalr; -import java.io.IOException; import java.io.StringReader; import java.util.ArrayList; import java.util.List; @@ -12,7 +11,6 @@ import com.google.gson.Gson; import com.google.gson.JsonArray; import com.google.gson.JsonParser; import com.google.gson.stream.JsonReader; -import com.google.gson.stream.JsonToken; class JsonHubProtocol implements HubProtocol { private final JsonParser jsonParser = new JsonParser(); @@ -31,7 +29,7 @@ class JsonHubProtocol implements HubProtocol { @Override public TransferFormat getTransferFormat() { - return TransferFormat.Text; + return TransferFormat.TEXT; } @Override @@ -120,7 +118,7 @@ class JsonHubProtocol implements HubProtocol { case CANCEL_INVOCATION: throw new UnsupportedOperationException(String.format("The message type %s is not supported yet.", messageType)); case PING: - hubMessages.add(new PingMessage()); + hubMessages.add(PingMessage.getInstance()); break; case CLOSE: if (error != null) { diff --git a/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/PingMessage.java b/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/PingMessage.java index 755f6fba88..e4e00fd365 100644 --- a/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/PingMessage.java +++ b/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/PingMessage.java @@ -3,12 +3,18 @@ package com.microsoft.aspnet.signalr; -class PingMessage extends HubMessage { +class PingMessage extends HubMessage +{ + private static PingMessage instance = new PingMessage(); - int type = HubMessageType.PING.value; + private PingMessage() + { + } + + public static PingMessage getInstance() {return instance;} @Override public HubMessageType getMessageType() { return HubMessageType.PING; } -} +} \ No newline at end of file diff --git a/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/TransferFormat.java b/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/TransferFormat.java index 32b8956bfb..cbda56e06f 100644 --- a/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/TransferFormat.java +++ b/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/TransferFormat.java @@ -4,6 +4,6 @@ package com.microsoft.aspnet.signalr; public enum TransferFormat { - Text, - Binary + TEXT, + BINARY } diff --git a/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/JsonHubProtocolTest.java b/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/JsonHubProtocolTest.java index 2c8f6e8d67..f36b45d81c 100644 --- a/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/JsonHubProtocolTest.java +++ b/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/JsonHubProtocolTest.java @@ -8,13 +8,11 @@ import static org.junit.Assert.*; import java.util.ArrayList; import java.util.Arrays; import java.util.List; -import java.util.concurrent.PriorityBlockingQueue; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; -import com.google.gson.JsonArray; public class JsonHubProtocolTest { private JsonHubProtocol jsonHubProtocol = new JsonHubProtocol(); @@ -31,7 +29,7 @@ public class JsonHubProtocolTest { @Test public void checkTransferFormat() { - assertEquals(TransferFormat.Text, jsonHubProtocol.getTransferFormat()); + assertEquals(TransferFormat.TEXT, jsonHubProtocol.getTransferFormat()); } @Test @@ -45,7 +43,7 @@ public class JsonHubProtocolTest { @Test public void parsePingMessage() throws Exception { String stringifiedMessage = "{\"type\":6}\u001E"; - TestBinder binder = new TestBinder(new PingMessage()); + TestBinder binder = new TestBinder(PingMessage.getInstance()); HubMessage[] messages = jsonHubProtocol.parseMessages(stringifiedMessage, binder); From 3f05f09fe462edc5a94f62e36bb6008ed592cd72 Mon Sep 17 00:00:00 2001 From: "ASP.NET CI" Date: Sun, 16 Sep 2018 12:28:05 -0700 Subject: [PATCH 2/9] Update dependencies.props [auto-updated: dependencies] --- build/dependencies.props | 98 ++++++++++++++++++++-------------------- korebuild-lock.txt | 4 +- 2 files changed, 51 insertions(+), 51 deletions(-) diff --git a/build/dependencies.props b/build/dependencies.props index 42296544c4..60617932b5 100644 --- a/build/dependencies.props +++ b/build/dependencies.props @@ -5,57 +5,57 @@ 0.10.13 3.1.0 - 2.2.0-preview3-35202 - 2.2.0-preview1-20180907.8 + 2.2.0-preview3-35252 + 2.2.0-preview1-20180911.1 1.7.3.4 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 4.5.0 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 2.2.0-preview2-26905-02 15.6.1 4.7.49 diff --git a/korebuild-lock.txt b/korebuild-lock.txt index 552300b0ce..1090ad6a92 100644 --- a/korebuild-lock.txt +++ b/korebuild-lock.txt @@ -1,2 +1,2 @@ -version:2.2.0-preview1-20180907.8 -commithash:078918eb5c1f176ee1da351c584fb4a4d7491aa0 +version:2.2.0-preview1-20180911.1 +commithash:ddfecdfc6e8e4859db5a0daea578070b862aac65 From bc148a07244a451e48b7e0efdf16db8f69f788ff Mon Sep 17 00:00:00 2001 From: BrennanConroy Date: Tue, 18 Sep 2018 08:36:17 -0700 Subject: [PATCH 3/9] Incorrect nameof usage (#2967) --- .../HubConnectionTests.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/Microsoft.AspNetCore.SignalR.Client.FunctionalTests/HubConnectionTests.cs b/test/Microsoft.AspNetCore.SignalR.Client.FunctionalTests/HubConnectionTests.cs index c8798cdac7..a4350d2c00 100644 --- a/test/Microsoft.AspNetCore.SignalR.Client.FunctionalTests/HubConnectionTests.cs +++ b/test/Microsoft.AspNetCore.SignalR.Client.FunctionalTests/HubConnectionTests.cs @@ -11,6 +11,7 @@ using System.Threading.Tasks; using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Http.Connections; using Microsoft.AspNetCore.Http.Connections.Client; +using Microsoft.AspNetCore.Http.Connections.Client.Internal; using Microsoft.AspNetCore.SignalR.Protocol; using Microsoft.AspNetCore.SignalR.Tests; using Microsoft.AspNetCore.Testing.xunit; @@ -398,10 +399,10 @@ namespace Microsoft.AspNetCore.SignalR.Client.FunctionalTests { bool ExpectedErrors(WriteContext writeContext) { - var firstLogCheck = (writeContext.LoggerName == nameof(Http.Connections.Client.Internal.ServerSentEventsTransport) || - writeContext.LoggerName == nameof(Http.Connections.Client.Internal.LongPollingTransport)) && + var firstLogCheck = (writeContext.LoggerName == typeof(ServerSentEventsTransport).FullName || + writeContext.LoggerName == typeof(LongPollingTransport).FullName) && writeContext.EventId.Name == "ErrorSending"; - var secondLogCheck = writeContext.LoggerName == nameof(Http.Connections.Client.HttpConnection) && + var secondLogCheck = writeContext.LoggerName == typeof(HttpConnection).FullName && writeContext.EventId.Name == "TransportThrewExceptionOnStop"; return firstLogCheck || secondLogCheck; } From 70ea1268a7236c3e5a2cab2a310088c10276e1e0 Mon Sep 17 00:00:00 2001 From: BrennanConroy Date: Tue, 18 Sep 2018 09:45:29 -0700 Subject: [PATCH 4/9] Use JUnit 5 for tests (#2968) --- clients/java/signalr/build.gradle | 8 +- .../aspnet/signalr/HandshakeProtocolTest.java | 4 +- .../aspnet/signalr/HubConnectionTest.java | 88 ++++++++----------- .../aspnet/signalr/HubExceptionTest.java | 4 +- .../aspnet/signalr/JsonHubProtocolTest.java | 29 +++--- .../aspnet/signalr/NegotiateResponseTest.java | 4 +- .../signalr/ResolveNegotiateUrlTest.java | 45 ++++------ .../signalr/WebSocketTransportTest.java | 14 +-- .../WebSocketTransportUrlFormatTest.java | 43 ++++----- 9 files changed, 99 insertions(+), 140 deletions(-) diff --git a/clients/java/signalr/build.gradle b/clients/java/signalr/build.gradle index b688ee869e..244695cedc 100644 --- a/clients/java/signalr/build.gradle +++ b/clients/java/signalr/build.gradle @@ -16,7 +16,9 @@ repositories { } dependencies { - testImplementation group: 'junit', name: 'junit', version: '4.12' + testImplementation 'org.junit.jupiter:junit-jupiter-api:5.3.1' + testCompile 'org.junit.jupiter:junit-jupiter-params:5.3.1' + testRuntime 'org.junit.jupiter:junit-jupiter-engine:5.3.1' implementation "org.java-websocket:Java-WebSocket:1.3.8" implementation 'com.google.code.gson:gson:2.8.5' implementation 'com.squareup.okhttp3:okhttp:3.11.0' @@ -41,6 +43,10 @@ spotless { } } +test { + useJUnitPlatform() +} + task sourceJar(type: Jar) { classifier "sources" from sourceSets.main.allJava diff --git a/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/HandshakeProtocolTest.java b/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/HandshakeProtocolTest.java index df168fefc8..ff2ea82184 100644 --- a/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/HandshakeProtocolTest.java +++ b/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/HandshakeProtocolTest.java @@ -3,9 +3,9 @@ package com.microsoft.aspnet.signalr; -import static org.junit.Assert.*; +import static org.junit.jupiter.api.Assertions.*; -import org.junit.Test; +import org.junit.jupiter.api.Test; public class HandshakeProtocolTest { 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 655cb11506..50531c8194 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 @@ -3,22 +3,17 @@ package com.microsoft.aspnet.signalr; -import static org.junit.Assert.*; +import static org.junit.jupiter.api.Assertions.*; import java.util.ArrayList; import java.util.concurrent.atomic.AtomicReference; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.ExpectedException; +import org.junit.jupiter.api.Test; public class HubConnectionTest { private static final String RECORD_SEPARATOR = "\u001e"; - @Rule - public ExpectedException exceptionRule = ExpectedException.none(); - @Test public void checkHubConnectionState() throws Exception { Transport mockTransport = new MockTransport(); @@ -47,14 +42,12 @@ public class HubConnectionTest { @Test public void hubConnectionReceiveHandshakeResponseWithError() throws Exception { - exceptionRule.expect(HubException.class); - exceptionRule.expectMessage("Requested protocol 'messagepack' is not available."); - MockTransport mockTransport = new MockTransport(); HubConnection hubConnection = new HubConnection("http://example.com", mockTransport, true); hubConnection.start(); - mockTransport.receiveMessage("{\"error\":\"Requested protocol 'messagepack' is not available.\"}" + RECORD_SEPARATOR); + Throwable exception = assertThrows(HubException.class, () -> mockTransport.receiveMessage("{\"error\":\"Requested protocol 'messagepack' is not available.\"}" + RECORD_SEPARATOR)); + assertEquals("Error in handshake Requested protocol 'messagepack' is not available.", exception.getMessage()); } @Test @@ -67,7 +60,7 @@ public class HubConnectionTest { hubConnection.on("inc", action); hubConnection.on("inc", action); - assertEquals(0.0, value.get(), 0); + assertEquals(Double.valueOf(0), value.get()); hubConnection.start(); @@ -80,7 +73,7 @@ public class HubConnectionTest { mockTransport.receiveMessage("{\"type\":1,\"target\":\"inc\",\"arguments\":[]}" + RECORD_SEPARATOR); // Confirming that our handler was called and that the counter property was incremented. - assertEquals(2, value.get(), 0); + assertEquals(Double.valueOf(2), value.get()); } @Test @@ -92,7 +85,7 @@ public class HubConnectionTest { hubConnection.on("inc", action); - assertEquals(0.0, value.get(), 0); + assertEquals(Double.valueOf(0), value.get()); hubConnection.start(); String message = mockTransport.getSentMessages()[0]; @@ -104,10 +97,10 @@ public class HubConnectionTest { mockTransport.receiveMessage("{\"type\":1,\"target\":\"inc\",\"arguments\":[]}" + RECORD_SEPARATOR); // Confirming that our handler was called and that the counter property was incremented. - assertEquals(1, value.get(), 0); + assertEquals(Double.valueOf(1), value.get()); hubConnection.remove("inc"); - assertEquals(1, value.get(), 0); + assertEquals(Double.valueOf(1), value.get()); } @Test @@ -120,7 +113,7 @@ public class HubConnectionTest { hubConnection.on("inc", action); hubConnection.remove("inc"); - assertEquals(0.0, value.get(), 0); + assertEquals(Double.valueOf(0), value.get()); hubConnection.start(); String message = mockTransport.getSentMessages()[0]; @@ -132,7 +125,7 @@ public class HubConnectionTest { mockTransport.receiveMessage("{\"type\":1,\"target\":\"inc\",\"arguments\":[]}" + RECORD_SEPARATOR); // Confirming that the handler was removed. - assertEquals(0.0, value.get(), 0); + assertEquals(Double.valueOf(0), value.get()); } @Test @@ -146,7 +139,7 @@ public class HubConnectionTest { hubConnection.on("inc", action); hubConnection.on("inc", secondAction); - assertEquals(0.0, value.get(), 0); + assertEquals(Double.valueOf(0), value.get()); hubConnection.start(); String message = mockTransport.getSentMessages()[0]; @@ -157,14 +150,14 @@ public class HubConnectionTest { mockTransport.receiveMessage("{}" + RECORD_SEPARATOR); mockTransport.receiveMessage("{\"type\":1,\"target\":\"inc\",\"arguments\":[]}" + RECORD_SEPARATOR); - assertEquals(3, value.get(), 0); + assertEquals(Double.valueOf(3), value.get()); hubConnection.remove("inc"); mockTransport.receiveMessage("{\"type\":1,\"target\":\"inc\",\"arguments\":[]}" + RECORD_SEPARATOR); // Confirm that another invocation doesn't change anything because the handlers have been removed. - assertEquals(3, value.get(), 0); + assertEquals(Double.valueOf(3), value.get()); } @Test @@ -176,7 +169,7 @@ public class HubConnectionTest { Subscription subscription = hubConnection.on("inc", action); - assertEquals(0.0, value.get(), 0); + assertEquals(Double.valueOf(0), value.get()); hubConnection.start(); String message = mockTransport.getSentMessages()[0]; @@ -188,7 +181,7 @@ public class HubConnectionTest { mockTransport.receiveMessage("{\"type\":1,\"target\":\"inc\",\"arguments\":[]}" + RECORD_SEPARATOR); // Confirming that our handler was called and that the counter property was incremented. - assertEquals(1, value.get(), 0); + assertEquals(Double.valueOf(1), value.get()); subscription.unsubscribe(); try { @@ -197,7 +190,7 @@ public class HubConnectionTest { assertEquals("There are no callbacks registered for the method 'inc'.", ex.getMessage()); } - assertEquals(1, value.get(), 0); + assertEquals(Double.valueOf(1), value.get()); } @Test @@ -209,7 +202,7 @@ public class HubConnectionTest { Subscription subscription = hubConnection.on("inc", action); - assertEquals(0.0, value.get(), 0); + assertEquals(Double.valueOf(0), value.get()); hubConnection.start(); String message = mockTransport.getSentMessages()[0]; @@ -221,7 +214,7 @@ public class HubConnectionTest { mockTransport.receiveMessage("{\"type\":1,\"target\":\"inc\",\"arguments\":[]}" + RECORD_SEPARATOR); // Confirming that our handler was called and that the counter property was incremented. - assertEquals(1, value.get(), 0); + assertEquals(Double.valueOf(1), value.get()); subscription.unsubscribe(); subscription.unsubscribe(); @@ -231,7 +224,7 @@ public class HubConnectionTest { assertEquals("There are no callbacks registered for the method 'inc'.", ex.getMessage()); } - assertEquals(1, value.get(), 0); + assertEquals(Double.valueOf(1), value.get()); } @Test @@ -245,7 +238,7 @@ public class HubConnectionTest { Subscription subscription = hubConnection.on("inc", action); Subscription secondSubscription = hubConnection.on("inc", secondAction); - assertEquals(0.0, value.get(), 0); + assertEquals(Double.valueOf(0), value.get()); hubConnection.start(); String message = mockTransport.getSentMessages()[0]; @@ -256,12 +249,12 @@ public class HubConnectionTest { mockTransport.receiveMessage("{}" + RECORD_SEPARATOR); mockTransport.receiveMessage("{\"type\":1,\"target\":\"inc\",\"arguments\":[]}" + RECORD_SEPARATOR); // Confirming that our handler was called and that the counter property was incremented. - assertEquals(3, value.get(), 0); + assertEquals(Double.valueOf(3), value.get()); // This removes the first handler so when "inc" is invoked secondAction should still run. subscription.unsubscribe(); mockTransport.receiveMessage("{\"type\":1,\"target\":\"inc\",\"arguments\":[]}" + RECORD_SEPARATOR); - assertEquals(5, value.get(), 0); + assertEquals(Double.valueOf(5), value.get()); } @Test @@ -274,7 +267,7 @@ public class HubConnectionTest { Subscription sub = hubConnection.on("inc", action); sub.unsubscribe(); - assertEquals(0.0, value.get(), 0); + assertEquals(Double.valueOf(0), value.get()); hubConnection.start(); mockTransport.receiveMessage("{}" + RECORD_SEPARATOR); @@ -286,7 +279,7 @@ public class HubConnectionTest { } // Confirming that the handler was removed. - assertEquals(0, value.get(), 0); + assertEquals(Double.valueOf(0), value.get()); } @Test @@ -300,25 +293,24 @@ public class HubConnectionTest { hubConnection.on("add", action, Double.class); hubConnection.on("add", action, Double.class); - assertEquals(0, value.get(), 0); + assertEquals(Double.valueOf(0), value.get()); hubConnection.start(); mockTransport.receiveMessage("{}" + RECORD_SEPARATOR); mockTransport.receiveMessage("{\"type\":1,\"target\":\"add\",\"arguments\":[12]}" + RECORD_SEPARATOR); hubConnection.send("add", 12); // Confirming that our handler was called and the correct message was passed in. - assertEquals(24, value.get(), 0); + assertEquals(Double.valueOf(24), value.get()); } - // We're using AtomicReference in the send tests instead of int here because Gson has trouble deserializing to Integer @Test public void sendWithNoParamsTriggersOnHandler() throws Exception { - AtomicReference value = new AtomicReference(0.0); + AtomicReference value = new AtomicReference<>(0); MockTransport mockTransport = new MockTransport(); HubConnection hubConnection = new HubConnection("http://example.com", mockTransport, true); hubConnection.on("inc", () ->{ - assertEquals(0.0, value.get(), 0); + assertEquals(Integer.valueOf(0), value.get()); value.getAndUpdate((val) -> val + 1); }); @@ -327,7 +319,7 @@ public class HubConnectionTest { mockTransport.receiveMessage("{\"type\":1,\"target\":\"inc\",\"arguments\":[]}" + RECORD_SEPARATOR); // Confirming that our handler was called and that the counter property was incremented. - assertEquals(1, value.get(), 0); + assertEquals(Integer.valueOf(1), value.get()); } @Test @@ -373,7 +365,7 @@ public class HubConnectionTest { // Confirming that our handler was called and the correct message was passed in. assertEquals("Hello World", value1.get()); - assertEquals(12, value2.get(), 0); + assertEquals(Double.valueOf(12), value2.get()); } @Test @@ -473,7 +465,7 @@ public class HubConnectionTest { assertEquals("B", value2.get()); assertEquals("C", value3.get()); assertTrue(value4.get()); - assertEquals(12, value5.get(), 0); + assertEquals(Double.valueOf(12), value5.get()); } @Test @@ -513,7 +505,7 @@ public class HubConnectionTest { assertEquals("B", value2.get()); assertEquals("C", value3.get()); assertTrue(value4.get()); - assertEquals(12, value5.get(), 0); + assertEquals(Double.valueOf(12), value5.get()); assertEquals("D", value6.get()); } @@ -557,7 +549,7 @@ public class HubConnectionTest { assertEquals("B", value2.get()); assertEquals("C", value3.get()); assertTrue(value4.get()); - assertEquals(12, value5.get(), 0); + assertEquals(Double.valueOf(12), value5.get()); assertEquals("D", value6.get()); assertEquals("E", value7.get()); } @@ -604,7 +596,7 @@ public class HubConnectionTest { assertEquals("B", value2.get()); assertEquals("C", value3.get()); assertTrue(value4.get()); - assertEquals(12, value5.get(), 0); + assertEquals(Double.valueOf(12), value5.get()); assertEquals("D", value6.get()); assertEquals("E", value7.get()); assertEquals("F", value8.get()); @@ -649,7 +641,7 @@ public class HubConnectionTest { HubConnection hubConnection = new HubConnection("http://example.com", mockTransport, true); hubConnection.on("inc", () ->{ - assertEquals(0.0, value.get(), 0); + assertEquals(Double.valueOf(0), value.get()); value.getAndUpdate((val) -> val + 1); }); @@ -661,7 +653,7 @@ public class HubConnectionTest { mockTransport.receiveMessage("{}" + RECORD_SEPARATOR + "{\"type\":1,\"target\":\"inc\",\"arguments\":[]}" + RECORD_SEPARATOR); // Confirming that our handler was called and that the counter property was incremented. - assertEquals(1, value.get(), 0); + assertEquals(Double.valueOf(1), value.get()); } @Test @@ -740,14 +732,12 @@ public class HubConnectionTest { @Test public void cannotSendBeforeStart() throws Exception { - exceptionRule.expect(HubException.class); - exceptionRule.expectMessage("The 'send' method cannot be called if the connection is not active"); - Transport mockTransport = new MockTransport(); HubConnection hubConnection = new HubConnection("http://example.com", mockTransport); assertEquals(HubConnectionState.DISCONNECTED, hubConnection.getConnectionState()); - hubConnection.send("inc"); + Throwable exception = assertThrows(HubException.class, () -> hubConnection.send("inc")); + assertEquals("The 'send' method cannot be called if the connection is not active", exception.getMessage()); } private class MockTransport implements Transport { diff --git a/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/HubExceptionTest.java b/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/HubExceptionTest.java index 9914fb4cd1..0e306e5890 100644 --- a/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/HubExceptionTest.java +++ b/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/HubExceptionTest.java @@ -3,9 +3,9 @@ package com.microsoft.aspnet.signalr; -import static org.junit.Assert.assertEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; -import org.junit.Test; +import org.junit.jupiter.api.Test; public class HubExceptionTest { @Test diff --git a/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/JsonHubProtocolTest.java b/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/JsonHubProtocolTest.java index f36b45d81c..ed9717f3af 100644 --- a/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/JsonHubProtocolTest.java +++ b/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/JsonHubProtocolTest.java @@ -3,15 +3,13 @@ package com.microsoft.aspnet.signalr; -import static org.junit.Assert.*; +import static org.junit.jupiter.api.Assertions.*; import java.util.ArrayList; import java.util.Arrays; import java.util.List; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.ExpectedException; +import org.junit.jupiter.api.Test; public class JsonHubProtocolTest { @@ -110,47 +108,40 @@ public class JsonHubProtocolTest { assertEquals(42, messageResult); } - @Rule - public ExpectedException exceptionRule = ExpectedException.none(); - @Test public void parseSingleUnsupportedStreamItemMessage() throws Exception { - exceptionRule.expect(UnsupportedOperationException.class); - exceptionRule.expectMessage("The message type STREAM_ITEM is not supported yet."); String stringifiedMessage = "{\"type\":2,\"Id\":1,\"Item\":42}\u001E"; TestBinder binder = new TestBinder(null); - HubMessage[] messages = jsonHubProtocol.parseMessages(stringifiedMessage, binder); + Throwable exception = assertThrows(UnsupportedOperationException.class, () -> jsonHubProtocol.parseMessages(stringifiedMessage, binder)); + assertEquals("The message type STREAM_ITEM is not supported yet.", exception.getMessage()); } @Test public void parseSingleUnsupportedStreamInvocationMessage() throws Exception { - exceptionRule.expect(UnsupportedOperationException.class); - exceptionRule.expectMessage("The message type STREAM_INVOCATION is not supported yet."); String stringifiedMessage = "{\"type\":4,\"Id\":1,\"target\":\"test\",\"arguments\":[42]}\u001E"; TestBinder binder = new TestBinder(new StreamInvocationMessage("1", "test", new Object[] { 42 })); - HubMessage[] messages = jsonHubProtocol.parseMessages(stringifiedMessage, binder); + Throwable exception = assertThrows(UnsupportedOperationException.class, () -> jsonHubProtocol.parseMessages(stringifiedMessage, binder)); + assertEquals("The message type STREAM_INVOCATION is not supported yet.", exception.getMessage()); } @Test public void parseSingleUnsupportedCancelInvocationMessage() throws Exception { - exceptionRule.expect(UnsupportedOperationException.class); - exceptionRule.expectMessage("The message type CANCEL_INVOCATION is not supported yet."); String stringifiedMessage = "{\"type\":5,\"invocationId\":123}\u001E"; TestBinder binder = new TestBinder(null); - HubMessage[] messages = jsonHubProtocol.parseMessages(stringifiedMessage, binder); + Throwable exception = assertThrows(UnsupportedOperationException.class, () -> jsonHubProtocol.parseMessages(stringifiedMessage, binder)); + assertEquals("The message type CANCEL_INVOCATION is not supported yet.", exception.getMessage()); } @Test public void parseSingleUnsupportedCompletionMessage() throws Exception { - exceptionRule.expect(UnsupportedOperationException.class); - exceptionRule.expectMessage("The message type COMPLETION is not supported yet."); String stringifiedMessage = "{\"type\":3,\"invocationId\":123}\u001E"; TestBinder binder = new TestBinder(null); - HubMessage[] messages = jsonHubProtocol.parseMessages(stringifiedMessage, binder); + Throwable exception = assertThrows(UnsupportedOperationException.class, () -> jsonHubProtocol.parseMessages(stringifiedMessage, binder)); + assertEquals("The message type COMPLETION is not supported yet.", exception.getMessage()); } @Test diff --git a/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/NegotiateResponseTest.java b/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/NegotiateResponseTest.java index c83e19eb01..7c2be3b9fc 100644 --- a/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/NegotiateResponseTest.java +++ b/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/NegotiateResponseTest.java @@ -3,9 +3,9 @@ package com.microsoft.aspnet.signalr; -import static org.junit.Assert.*; +import static org.junit.jupiter.api.Assertions.*; -import org.junit.Test; +import org.junit.jupiter.api.Test; public class NegotiateResponseTest { diff --git a/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/ResolveNegotiateUrlTest.java b/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/ResolveNegotiateUrlTest.java index 21daaae8bb..9f852c7725 100644 --- a/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/ResolveNegotiateUrlTest.java +++ b/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/ResolveNegotiateUrlTest.java @@ -3,39 +3,28 @@ package com.microsoft.aspnet.signalr; -import static org.junit.Assert.assertEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; -import java.util.Arrays; -import java.util.Collection; +import java.util.stream.Stream; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.Parameterized; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; - -@RunWith(Parameterized.class) public class ResolveNegotiateUrlTest { - private String url; - private String resolvedUrl; - - public ResolveNegotiateUrlTest(String url, String resolvedUrl) { - this.url = url; - this.resolvedUrl = resolvedUrl; + private static Stream protocols() { + return Stream.of( + Arguments.of("http://example.com/hub/", "http://example.com/hub/negotiate"), + Arguments.of("http://example.com/hub", "http://example.com/hub/negotiate"), + Arguments.of("http://example.com/endpoint?q=my/Data", "http://example.com/endpoint/negotiate?q=my/Data"), + Arguments.of("http://example.com/endpoint/?q=my/Data", "http://example.com/endpoint/negotiate?q=my/Data"), + Arguments.of("http://example.com/endpoint/path/more?q=my/Data", "http://example.com/endpoint/path/more/negotiate?q=my/Data")); } - @Parameterized.Parameters - public static Collection protocols() { - return Arrays.asList(new String[][]{ - {"http://example.com/hub/", "http://example.com/hub/negotiate"}, - {"http://example.com/hub", "http://example.com/hub/negotiate"}, - {"http://example.com/endpoint?q=my/Data", "http://example.com/endpoint/negotiate?q=my/Data"}, - {"http://example.com/endpoint/?q=my/Data", "http://example.com/endpoint/negotiate?q=my/Data"}, - {"http://example.com/endpoint/path/more?q=my/Data", "http://example.com/endpoint/path/more/negotiate?q=my/Data"},}); - } - - @Test - public void checkNegotiateUrl() { - String urlResult = Negotiate.resolveNegotiateUrl(this.url); - assertEquals(this.resolvedUrl, urlResult); + @ParameterizedTest + @MethodSource("protocols") + public void checkNegotiateUrl(String url, String resolvedUrl) { + String urlResult = Negotiate.resolveNegotiateUrl(url); + assertEquals(resolvedUrl, urlResult); } } \ No newline at end of file diff --git a/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/WebSocketTransportTest.java b/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/WebSocketTransportTest.java index a29246d812..dc16efa4a9 100644 --- a/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/WebSocketTransportTest.java +++ b/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/WebSocketTransportTest.java @@ -3,20 +3,14 @@ package com.microsoft.aspnet.signalr; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.ExpectedException; +import static org.junit.jupiter.api.Assertions.*; +import org.junit.jupiter.api.Test; public class WebSocketTransportTest { - - @Rule - public ExpectedException expectedEx = ExpectedException.none(); - @Test public void WebsocketThrowsIfItCantConnect() throws Exception { - expectedEx.expect(Exception.class); - expectedEx.expectMessage("There was an error starting the Websockets transport"); Transport transport = new WebSocketTransport("www.notarealurl12345.fake", new NullLogger()); - transport.start(); + Throwable exception = assertThrows(Exception.class, () -> transport.start()); + assertEquals("There was an error starting the Websockets transport.", exception.getMessage()); } } diff --git a/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/WebSocketTransportUrlFormatTest.java b/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/WebSocketTransportUrlFormatTest.java index 0e2a93284d..f7df0d956f 100644 --- a/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/WebSocketTransportUrlFormatTest.java +++ b/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/WebSocketTransportUrlFormatTest.java @@ -3,39 +3,28 @@ package com.microsoft.aspnet.signalr; -import static org.junit.Assert.*; +import static org.junit.jupiter.api.Assertions.*; import java.net.URISyntaxException; -import java.util.Arrays; -import java.util.Collection; +import java.util.stream.Stream; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.Parameterized; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; - -@RunWith(Parameterized.class) public class WebSocketTransportUrlFormatTest { - private String url; - private String expectedUrl; - - public WebSocketTransportUrlFormatTest(String url, String expectedProtocol) { - this.url = url; - this.expectedUrl = expectedProtocol; + private static Stream protocols() { + return Stream.of( + Arguments.of("http://example.com", "ws://example.com"), + Arguments.of("https://example.com", "wss://example.com"), + Arguments.of("ws://example.com", "ws://example.com"), + Arguments.of("wss://example.com", "wss://example.com")); } - @Parameterized.Parameters - public static Collection protocols() { - return Arrays.asList(new String[][]{ - {"http://example.com", "ws://example.com"}, - {"https://example.com", "wss://example.com"}, - {"ws://example.com", "ws://example.com"}, - {"wss://example.com", "wss://example.com"}}); - } - - @Test - public void checkWebsocketUrlProtocol() throws URISyntaxException { - WebSocketTransport webSocketTransport = new WebSocketTransport(this.url, new NullLogger()); - assertEquals(this.expectedUrl, webSocketTransport.getUrl().toString()); + @ParameterizedTest + @MethodSource("protocols") + public void checkWebsocketUrlProtocol(String url, String expectedUrl) throws URISyntaxException { + WebSocketTransport webSocketTransport = new WebSocketTransport(url, new NullLogger()); + assertEquals(expectedUrl, webSocketTransport.getUrl().toString()); } } \ No newline at end of file From f27df1d61e39d5c9995d65edd2212fa36e24ea94 Mon Sep 17 00:00:00 2001 From: Mikael Mengistu Date: Wed, 19 Sep 2018 10:14:35 -0700 Subject: [PATCH 5/9] Java Async APIs (#2971) --- .../aspnet/signalr/HubConnection.java | 101 +++++++++++------- .../microsoft/aspnet/signalr/Transport.java | 8 +- .../aspnet/signalr/WebSocketTransport.java | 40 ++++--- .../aspnet/signalr/HubConnectionTest.java | 12 ++- .../signalr/WebSocketTransportTest.java | 7 +- 5 files changed, 107 insertions(+), 61 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 417cd3f743..edcbf90558 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 @@ -3,10 +3,14 @@ package com.microsoft.aspnet.signalr; +import java.io.IOException; import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.function.Consumer; public class HubConnection { @@ -18,6 +22,7 @@ public class HubConnection { private Boolean handshakeReceived = false; private static final String RECORD_SEPARATOR = "\u001e"; private HubConnectionState hubConnectionState = HubConnectionState.DISCONNECTED; + private Lock hubConnectionStateLock = new ReentrantLock(); private Logger logger; private List> onClosedCallbackList; private boolean skipNegotiate = false; @@ -99,6 +104,29 @@ public class HubConnection { } } + private NegotiateResponse handleNegotiate() throws IOException { + accessToken = (negotiateResponse == null) ? null : negotiateResponse.getAccessToken(); + negotiateResponse = Negotiate.processNegotiate(url, accessToken); + + 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 = this.negotiateResponse.getRedirectUrl(); + } + + return negotiateResponse; + } + public HubConnection(String url, Transport transport, Logger logger) { this(url, transport, logger, false); } @@ -150,32 +178,15 @@ public class HubConnection { * * @throws Exception An error occurred while connecting. */ - public void start() throws Exception { + public CompletableFuture start() throws Exception { if (hubConnectionState != HubConnectionState.DISCONNECTED) { - return; + return CompletableFuture.completedFuture(null); } if (!skipNegotiate) { int negotiateAttempts = 0; do { accessToken = (negotiateResponse == null) ? null : negotiateResponse.getAccessToken(); - negotiateResponse = Negotiate.processNegotiate(url, accessToken); - - 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) { - url = this.negotiateResponse.getRedirectUrl(); - } - + negotiateResponse = handleNegotiate(); negotiateAttempts++; } while (negotiateResponse.getRedirectUrl() != null && negotiateAttempts < MAX_NEGOTIATE_ATTEMPTS); if (!negotiateResponse.getAvailableTransports().contains("WebSockets")) { @@ -189,32 +200,46 @@ public class HubConnection { } transport.setOnReceive(this.callback); - transport.start(); - String handshake = HandshakeProtocol.createHandshakeRequestMessage(new HandshakeRequestMessage(protocol.getName(), protocol.getVersion())); - transport.send(handshake); - hubConnectionState = HubConnectionState.CONNECTED; - connectionState = new ConnectionState(this); - logger.log(LogLevel.Information, "HubConnected started."); + return transport.start().thenCompose((future) -> { + String handshake = HandshakeProtocol.createHandshakeRequestMessage(new HandshakeRequestMessage(protocol.getName(), protocol.getVersion())); + return transport.send(handshake).thenRun(() -> { + hubConnectionStateLock.lock(); + try { + hubConnectionState = HubConnectionState.CONNECTED; + connectionState = new ConnectionState(this); + logger.log(LogLevel.Information, "HubConnected started."); + } finally { + hubConnectionStateLock.unlock(); + } + }); + }); + } /** * Stops a connection to the server. */ private void stop(String errorMessage) { - if (hubConnectionState == HubConnectionState.DISCONNECTED) { - return; + hubConnectionStateLock.lock(); + try { + if (hubConnectionState == HubConnectionState.DISCONNECTED) { + return; + } + + if (errorMessage != null) { + logger.log(LogLevel.Error, "HubConnection disconnected with an error %s.", errorMessage); + } else { + logger.log(LogLevel.Debug, "Stopping HubConnection."); + } + + transport.stop(); + hubConnectionState = HubConnectionState.DISCONNECTED; + connectionState = null; + logger.log(LogLevel.Information, "HubConnection stopped."); + } finally { + hubConnectionStateLock.unlock(); } - if (errorMessage != null) { - logger.log(LogLevel.Error, "HubConnection disconnected with an error %s.", errorMessage); - } else { - logger.log(LogLevel.Debug, "Stopping HubConnection."); - } - - transport.stop(); - hubConnectionState = HubConnectionState.DISCONNECTED; - connectionState = null; - logger.log(LogLevel.Information, "HubConnection stopped."); if (onClosedCallbackList != null) { HubException hubException = new HubException(errorMessage); for (Consumer callback : onClosedCallbackList) { diff --git a/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/Transport.java b/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/Transport.java index 79e093682c..eafb865465 100644 --- a/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/Transport.java +++ b/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/Transport.java @@ -3,10 +3,12 @@ package com.microsoft.aspnet.signalr; +import java.util.concurrent.CompletableFuture; + interface Transport { - void start() throws Exception; - void send(String message) throws Exception; + CompletableFuture start() throws Exception; + CompletableFuture send(String message); void setOnReceive(OnReceiveCallBack callback); void onReceive(String message) throws Exception; - void stop(); + CompletableFuture stop(); } diff --git a/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/WebSocketTransport.java b/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/WebSocketTransport.java index e20ef50ac8..91cd290898 100644 --- a/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/WebSocketTransport.java +++ b/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/WebSocketTransport.java @@ -6,6 +6,7 @@ package com.microsoft.aspnet.signalr; import java.net.URI; import java.net.URISyntaxException; import java.util.Map; +import java.util.concurrent.CompletableFuture; import org.java_websocket.client.WebSocketClient; import org.java_websocket.handshake.ServerHandshake; @@ -47,21 +48,28 @@ class WebSocketTransport implements Transport { } @Override - public void start() throws Exception { - logger.log(LogLevel.Debug, "Starting Websocket connection."); - webSocketClient = createWebSocket(headers); - - if (!webSocketClient.connectBlocking()) { - String errorMessage = "There was an error starting the Websockets transport."; - logger.log(LogLevel.Debug, errorMessage); - throw new Exception(errorMessage); - } - logger.log(LogLevel.Information, "WebSocket transport connected to: %s", webSocketClient.getURI()); + public CompletableFuture start() { + return CompletableFuture.runAsync(() -> { + logger.log(LogLevel.Debug, "Starting Websocket connection."); + webSocketClient = createWebSocket(headers); + try { + if (!webSocketClient.connectBlocking()) { + String errorMessage = "There was an error starting the Websockets transport."; + logger.log(LogLevel.Debug, errorMessage); + throw new RuntimeException(errorMessage); + } + } catch (InterruptedException e) { + String interruptedExMessage = "Connecting the Websockets transport was interrupted."; + logger.log(LogLevel.Debug, interruptedExMessage); + throw new RuntimeException(interruptedExMessage); + } + logger.log(LogLevel.Information, "WebSocket transport connected to: %s", webSocketClient.getURI()); + }); } @Override - public void send(String message) { - webSocketClient.send(message); + public CompletableFuture send(String message) { + return CompletableFuture.runAsync(() -> webSocketClient.send(message)); } @Override @@ -76,9 +84,11 @@ class WebSocketTransport implements Transport { } @Override - public void stop() { - webSocketClient.closeConnection(0, "HubConnection Stopped"); - logger.log(LogLevel.Information, "WebSocket connection stopped"); + public CompletableFuture stop() { + return CompletableFuture.runAsync(() -> { + webSocketClient.closeConnection(0, "HubConnection Stopped"); + logger.log(LogLevel.Information, "WebSocket connection stopped"); + }); } private WebSocketClient createWebSocket(Map headers) { 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 50531c8194..1371770950 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 @@ -6,6 +6,7 @@ package com.microsoft.aspnet.signalr; import static org.junit.jupiter.api.Assertions.*; import java.util.ArrayList; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.atomic.AtomicReference; import org.junit.jupiter.api.Test; @@ -745,11 +746,14 @@ public class HubConnectionTest { private ArrayList sentMessages = new ArrayList<>(); @Override - public void start() {} + public CompletableFuture start() { + return CompletableFuture.completedFuture(null); + } @Override - public void send(String message) { + public CompletableFuture send(String message) { sentMessages.add(message); + return CompletableFuture.completedFuture(null); } @Override @@ -763,7 +767,9 @@ public class HubConnectionTest { } @Override - public void stop() {} + public CompletableFuture stop() { + return CompletableFuture.completedFuture(null); + } public void receiveMessage(String message) throws Exception { this.onReceive(message); diff --git a/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/WebSocketTransportTest.java b/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/WebSocketTransportTest.java index dc16efa4a9..31116d4d65 100644 --- a/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/WebSocketTransportTest.java +++ b/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/WebSocketTransportTest.java @@ -4,13 +4,16 @@ package com.microsoft.aspnet.signalr; import static org.junit.jupiter.api.Assertions.*; + +import java.util.concurrent.TimeUnit; + import org.junit.jupiter.api.Test; public class WebSocketTransportTest { @Test public void WebsocketThrowsIfItCantConnect() throws Exception { Transport transport = new WebSocketTransport("www.notarealurl12345.fake", new NullLogger()); - Throwable exception = assertThrows(Exception.class, () -> transport.start()); - assertEquals("There was an error starting the Websockets transport.", exception.getMessage()); + Throwable exception = assertThrows(Exception.class, () -> transport.start().get(1,TimeUnit.SECONDS)); + assertEquals("There was an error starting the Websockets transport.", exception.getCause().getMessage()); } } From 8be051ce34319c9aac7cd55340c28642c25d0fbd Mon Sep 17 00:00:00 2001 From: BrennanConroy Date: Wed, 19 Sep 2018 13:39:34 -0700 Subject: [PATCH 6/9] Idempotentize AddSignalR (#2972) --- .../SignalRDependencyInjectionExtensions.cs | 2 +- .../SignalRDependencyInjectionExtensions.cs | 5 +++-- .../Microsoft.AspNetCore.SignalR.Tests/AddSignalRTests.cs | 8 ++++++++ 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.AspNetCore.SignalR.Core/SignalRDependencyInjectionExtensions.cs b/src/Microsoft.AspNetCore.SignalR.Core/SignalRDependencyInjectionExtensions.cs index 3271883a0a..efdeda8d18 100644 --- a/src/Microsoft.AspNetCore.SignalR.Core/SignalRDependencyInjectionExtensions.cs +++ b/src/Microsoft.AspNetCore.SignalR.Core/SignalRDependencyInjectionExtensions.cs @@ -20,7 +20,7 @@ namespace Microsoft.Extensions.DependencyInjection /// An that can be used to further configure the SignalR services. public static ISignalRServerBuilder AddSignalRCore(this IServiceCollection services) { - services.AddSingleton(); + services.TryAddSingleton(); services.TryAddSingleton(typeof(HubLifetimeManager<>), typeof(DefaultHubLifetimeManager<>)); services.TryAddSingleton(typeof(IHubProtocolResolver), typeof(DefaultHubProtocolResolver)); services.TryAddSingleton(typeof(IHubContext<>), typeof(HubContext<>)); diff --git a/src/Microsoft.AspNetCore.SignalR/SignalRDependencyInjectionExtensions.cs b/src/Microsoft.AspNetCore.SignalR/SignalRDependencyInjectionExtensions.cs index dfc3c9e644..8974bd094f 100644 --- a/src/Microsoft.AspNetCore.SignalR/SignalRDependencyInjectionExtensions.cs +++ b/src/Microsoft.AspNetCore.SignalR/SignalRDependencyInjectionExtensions.cs @@ -4,6 +4,7 @@ using System; using Microsoft.AspNetCore.SignalR; using Microsoft.AspNetCore.SignalR.Internal; +using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.Options; namespace Microsoft.Extensions.DependencyInjection @@ -35,8 +36,8 @@ namespace Microsoft.Extensions.DependencyInjection public static ISignalRServerBuilder AddSignalR(this IServiceCollection services) { services.AddConnections(); - services.AddSingleton(); - services.AddSingleton, HubOptionsSetup>(); + services.TryAddSingleton(); + services.TryAddEnumerable(ServiceDescriptor.Singleton, HubOptionsSetup>()); return services.AddSignalRCore(); } diff --git a/test/Microsoft.AspNetCore.SignalR.Tests/AddSignalRTests.cs b/test/Microsoft.AspNetCore.SignalR.Tests/AddSignalRTests.cs index 6d8360d745..f5e15fb0b0 100644 --- a/test/Microsoft.AspNetCore.SignalR.Tests/AddSignalRTests.cs +++ b/test/Microsoft.AspNetCore.SignalR.Tests/AddSignalRTests.cs @@ -4,8 +4,10 @@ using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; +using Microsoft.AspNetCore.SignalR.Internal; using Microsoft.AspNetCore.SignalR.Protocol; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; using Xunit; namespace Microsoft.AspNetCore.SignalR.Tests @@ -17,12 +19,16 @@ namespace Microsoft.AspNetCore.SignalR.Tests { var serviceCollection = new ServiceCollection(); + var markerService = new SignalRCoreMarkerService(); + serviceCollection.AddSingleton(markerService); serviceCollection.AddSingleton(); serviceCollection.AddSingleton(typeof(HubLifetimeManager<>), typeof(CustomHubLifetimeManager<>)); serviceCollection.AddSingleton(); serviceCollection.AddScoped(typeof(IHubActivator<>), typeof(CustomHubActivator<>)); serviceCollection.AddSingleton(typeof(IHubContext<>), typeof(CustomHubContext<>)); serviceCollection.AddSingleton(typeof(IHubContext<,>), typeof(CustomHubContext<,>)); + var hubOptions = new HubOptionsSetup(new List()); + serviceCollection.AddSingleton>(hubOptions); serviceCollection.AddSignalR(); var serviceProvider = serviceCollection.BuildServiceProvider(); @@ -33,6 +39,8 @@ namespace Microsoft.AspNetCore.SignalR.Tests Assert.IsType>(serviceProvider.GetRequiredService>()); Assert.IsType>(serviceProvider.GetRequiredService>()); Assert.IsType>(serviceProvider.GetRequiredService>()); + Assert.Equal(hubOptions, serviceProvider.GetRequiredService>()); + Assert.Equal(markerService, serviceProvider.GetRequiredService()); } [Fact] From 4b378692a4ea350091f1101e519169e28b8662b5 Mon Sep 17 00:00:00 2001 From: BrennanConroy Date: Wed, 19 Sep 2018 13:58:15 -0700 Subject: [PATCH 7/9] [Java] Add Invoke support (#2961) --- .../aspnet/signalr/CloseMessage.java | 2 +- .../aspnet/signalr/CompletionMessage.java | 38 +++++ .../aspnet/signalr/HubConnection.java | 139 ++++++++++++++++-- .../microsoft/aspnet/signalr/HubProtocol.java | 1 - .../aspnet/signalr/InvocationMessage.java | 13 +- .../aspnet/signalr/InvocationRequest.java | 45 ++++++ .../aspnet/signalr/JsonHubProtocol.java | 20 ++- .../signalr/StreamInvocationMessage.java | 3 +- .../aspnet/signalr/HubConnectionTest.java | 109 +++++++++++++- .../aspnet/signalr/JsonHubProtocolTest.java | 40 +++-- 10 files changed, 367 insertions(+), 43 deletions(-) create mode 100644 clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/CompletionMessage.java create mode 100644 clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/InvocationRequest.java diff --git a/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/CloseMessage.java b/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/CloseMessage.java index 920eb2eabc..1931f49ad4 100644 --- a/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/CloseMessage.java +++ b/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/CloseMessage.java @@ -4,7 +4,7 @@ package com.microsoft.aspnet.signalr; class CloseMessage extends HubMessage { - String error; + private String error; @Override public HubMessageType getMessageType() { diff --git a/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/CompletionMessage.java b/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/CompletionMessage.java new file mode 100644 index 0000000000..50a8e7f78a --- /dev/null +++ b/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/CompletionMessage.java @@ -0,0 +1,38 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +package com.microsoft.aspnet.signalr; + +class CompletionMessage extends HubMessage { + private int type = HubMessageType.COMPLETION.value; + private String invocationId; + private Object result; + private String error; + + public CompletionMessage(String invocationId, Object result, String error) { + if (error != null && result != null) + { + throw new IllegalArgumentException("Expected either 'error' or 'result' to be provided, but not both"); + } + this.invocationId = invocationId; + this.result = result; + this.error = error; + } + + public Object getResult() { + return result; + } + + public String getError() { + return error; + } + + public String getInvocationId() { + return invocationId; + } + + @Override + public HubMessageType getMessageType() { + return HubMessageType.values()[type - 1]; + } +} \ No newline at end of file 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 edcbf90558..06c3b34b55 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 @@ -9,6 +9,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; import java.util.function.Consumer; @@ -71,13 +72,13 @@ public class HubConnection { switch (message.getMessageType()) { case INVOCATION: InvocationMessage invocationMessage = (InvocationMessage) message; - List handlers = this.handlers.get(invocationMessage.target); + List handlers = this.handlers.get(invocationMessage.getTarget()); if (handlers != null) { for (InvocationHandler handler : handlers) { - handler.getAction().invoke(invocationMessage.arguments); + handler.getAction().invoke(invocationMessage.getArguments()); } } else { - logger.log(LogLevel.Warning, "Failed to find handler for %s method.", invocationMessage.target); + logger.log(LogLevel.Warning, "Failed to find handler for %s method.", invocationMessage.getMessageType()); } break; case CLOSE: @@ -88,10 +89,18 @@ public class HubConnection { case PING: // We don't need to do anything in the case of a ping message. break; + case COMPLETION: + CompletionMessage completionMessage = (CompletionMessage)message; + InvocationRequest irq = connectionState.tryRemoveInvocation(completionMessage.getInvocationId()); + if (irq == null) { + logger.log(LogLevel.Warning, "Dropped unsolicited Completion message for invocation '%s'.", completionMessage.getInvocationId()); + continue; + } + irq.complete(completionMessage); + break; case STREAM_INVOCATION: case STREAM_ITEM: case CANCEL_INVOCATION: - case COMPLETION: logger.log(LogLevel.Error, "This client does not support %s messages.", message.getMessageType()); throw new UnsupportedOperationException(String.format("The message type %s is not supported yet.", message.getMessageType())); @@ -220,6 +229,7 @@ public class HubConnection { * Stops a connection to the server. */ private void stop(String errorMessage) { + HubException hubException = null; hubConnectionStateLock.lock(); try { if (hubConnectionState == HubConnectionState.DISCONNECTED) { @@ -234,6 +244,11 @@ public class HubConnection { transport.stop(); hubConnectionState = HubConnectionState.DISCONNECTED; + + if (errorMessage != null) { + hubException = new HubException(errorMessage); + } + connectionState.cancelOutstandingInvocations(hubException); connectionState = null; logger.log(LogLevel.Information, "HubConnection stopped."); } finally { @@ -241,7 +256,6 @@ public class HubConnection { } if (onClosedCallbackList != null) { - HubException hubException = new HubException(errorMessage); for (Consumer callback : onClosedCallbackList) { callback.accept(hubException); } @@ -268,10 +282,49 @@ public class HubConnection { throw new HubException("The 'send' method cannot be called if the connection is not active"); } - InvocationMessage invocationMessage = new InvocationMessage(method, args); - String message = protocol.writeMessage(invocationMessage); - logger.log(LogLevel.Debug, "Sending message"); - transport.send(message); + InvocationMessage invocationMessage = new InvocationMessage(null, method, args); + sendHubMessage(invocationMessage); + } + + public CompletableFuture invoke(Class returnType, String method, Object... args) throws Exception { + String id = connectionState.getNextInvocationId(); + InvocationMessage invocationMessage = new InvocationMessage(id, method, args); + + CompletableFuture future = new CompletableFuture<>(); + InvocationRequest irq = new InvocationRequest(returnType, id); + connectionState.addInvocation(irq); + + // forward the invocation result or error to the user + // run continuations on a separate thread + CompletableFuture pendingCall = irq.getPendingCall(); + pendingCall.whenCompleteAsync((result, error) -> { + if (error == null) { + // Primitive types can't be cast with the Class cast function + if (returnType.isPrimitive()) { + future.complete((T)result); + } else { + future.complete(returnType.cast(result)); + } + } else { + future.completeExceptionally(error); + } + }); + + // Make sure the actual send is after setting up the future otherwise there is a race + // where the map doesn't have the future yet when the response is returned + sendHubMessage(invocationMessage); + + return future; + } + + private void sendHubMessage(HubMessage message) throws Exception { + String serializedMessage = protocol.writeMessage(message); + if (message.getMessageType() == HubMessageType.INVOCATION) { + logger.log(LogLevel.Debug, "Sending %d message '%s'.", message.getMessageType().value, ((InvocationMessage)message).getInvocationId()); + } else { + logger.log(LogLevel.Debug, "Sending %d message.", message.getMessageType().value); + } + transport.send(serializedMessage); } /** @@ -559,15 +612,79 @@ public class HubConnection { } private class ConnectionState implements InvocationBinder { - HubConnection connection; + private HubConnection connection; + private AtomicInteger nextId = new AtomicInteger(0); + private HashMap pendingInvocations = new HashMap<>(); + private Lock lock = new ReentrantLock(); public ConnectionState(HubConnection connection) { this.connection = connection; } + public String getNextInvocationId() { + int i = nextId.incrementAndGet(); + return Integer.toString(i); + } + + public void cancelOutstandingInvocations(Exception ex) { + lock.lock(); + try { + pendingInvocations.forEach((key, irq) -> { + if (ex == null) { + irq.cancel(); + } else { + irq.fail(ex); + } + }); + + pendingInvocations.clear(); + } finally { + lock.unlock(); + } + } + + public void addInvocation(InvocationRequest irq) { + lock.lock(); + try { + pendingInvocations.compute(irq.getInvocationId(), (key, value) -> { + if (value != null) { + // This should never happen + throw new IllegalStateException("Invocation Id is already used"); + } + + return irq; + }); + } finally { + lock.unlock(); + } + } + + public InvocationRequest getInvocation(String id) { + lock.lock(); + try { + return pendingInvocations.get(id); + } finally { + lock.unlock(); + } + } + + public InvocationRequest tryRemoveInvocation(String id) { + lock.lock(); + try { + return pendingInvocations.remove(id); + } finally { + lock.unlock(); + } + } + @Override public Class getReturnType(String invocationId) { - return null; + InvocationRequest irq = getInvocation(invocationId); + if (irq == null) { + return null; + } + + return irq.getReturnType(); } @Override diff --git a/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/HubProtocol.java b/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/HubProtocol.java index a22105fc91..a1d2ee4d92 100644 --- a/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/HubProtocol.java +++ b/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/HubProtocol.java @@ -3,7 +3,6 @@ package com.microsoft.aspnet.signalr; - /** * A protocol abstraction for communicating with SignalR hubs. */ diff --git a/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/InvocationMessage.java b/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/InvocationMessage.java index cde7f9feac..d7fe2e0f04 100644 --- a/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/InvocationMessage.java +++ b/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/InvocationMessage.java @@ -5,11 +5,12 @@ package com.microsoft.aspnet.signalr; class InvocationMessage extends HubMessage { int type = HubMessageType.INVOCATION.value; - String invocationId; - String target; - Object[] arguments; + protected String invocationId; + private String target; + private Object[] arguments; - public InvocationMessage(String target, Object[] args) { + public InvocationMessage(String invocationId, String target, Object[] args) { + this.invocationId = invocationId; this.target = target; this.arguments = args; } @@ -18,10 +19,6 @@ class InvocationMessage extends HubMessage { return invocationId; } - public void setInvocationId(String invocationId) { - this.invocationId = invocationId; - } - public String getTarget() { return target; } diff --git a/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/InvocationRequest.java b/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/InvocationRequest.java new file mode 100644 index 0000000000..5eae9374a2 --- /dev/null +++ b/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/InvocationRequest.java @@ -0,0 +1,45 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +package com.microsoft.aspnet.signalr; + +import java.util.concurrent.CompletableFuture; + +class InvocationRequest { + private Class returnType; + private CompletableFuture pendingCall = new CompletableFuture<>(); + private String invocationId; + + InvocationRequest(Class returnType, String invocationId) { + this.returnType = returnType; + this.invocationId = invocationId; + } + + public void complete(CompletionMessage completion) { + if (completion.getResult() != null) { + pendingCall.complete(completion.getResult()); + } else { + pendingCall.completeExceptionally(new HubException(completion.getError())); + } + } + + public void fail(Exception ex) { + pendingCall.completeExceptionally(ex); + } + + public void cancel() { + pendingCall.cancel(false); + } + + public CompletableFuture getPendingCall() { + return pendingCall; + } + + public Class getReturnType() { + return returnType; + } + + public String getInvocationId() { + return invocationId; + } +} \ No newline at end of file diff --git a/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/JsonHubProtocol.java b/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/JsonHubProtocol.java index bf04c52e83..4acc831226 100644 --- a/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/JsonHubProtocol.java +++ b/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/JsonHubProtocol.java @@ -9,6 +9,7 @@ import java.util.List; import com.google.gson.Gson; import com.google.gson.JsonArray; +import com.google.gson.JsonElement; import com.google.gson.JsonParser; import com.google.gson.stream.JsonReader; @@ -43,6 +44,8 @@ class JsonHubProtocol implements HubProtocol { String error = null; ArrayList arguments = null; JsonArray argumentsToken = null; + Object result = null; + JsonElement resultToken = null; JsonReader reader = new JsonReader(new StringReader(str)); reader.beginObject(); @@ -63,7 +66,11 @@ class JsonHubProtocol implements HubProtocol { error = reader.nextString(); break; case "result": - reader.skipValue(); + if (invocationId == null) { + resultToken = jsonParser.parse(reader); + } else { + result = gson.fromJson(reader, binder.getReturnType(invocationId)); + } break; case "item": reader.skipValue(); @@ -107,14 +114,19 @@ class JsonHubProtocol implements HubProtocol { } } if (arguments == null) { - hubMessages.add(new InvocationMessage(target, new Object[0])); + hubMessages.add(new InvocationMessage(invocationId, target, new Object[0])); } else { - hubMessages.add(new InvocationMessage(target, arguments.toArray())); + hubMessages.add(new InvocationMessage(invocationId, target, arguments.toArray())); } break; + case COMPLETION: + if (resultToken != null) { + result = gson.fromJson(resultToken, binder.getReturnType(invocationId)); + } + hubMessages.add(new CompletionMessage(invocationId, result, error)); + break; case STREAM_INVOCATION: case STREAM_ITEM: - case COMPLETION: case CANCEL_INVOCATION: throw new UnsupportedOperationException(String.format("The message type %s is not supported yet.", messageType)); case PING: diff --git a/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/StreamInvocationMessage.java b/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/StreamInvocationMessage.java index ab4f07983c..cf2d111a30 100644 --- a/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/StreamInvocationMessage.java +++ b/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/StreamInvocationMessage.java @@ -8,8 +8,7 @@ class StreamInvocationMessage extends InvocationMessage { int type = HubMessageType.STREAM_INVOCATION.value; public StreamInvocationMessage(String invocationId, String target, Object[] arguments) { - super(target, arguments); - this.invocationId = invocationId; + super(invocationId, target, arguments); } @Override 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 1371770950..47acc2178b 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 @@ -6,7 +6,9 @@ package com.microsoft.aspnet.signalr; import static org.junit.jupiter.api.Assertions.*; import java.util.ArrayList; +import java.util.concurrent.CancellationException; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; import org.junit.jupiter.api.Test; @@ -298,12 +300,117 @@ public class HubConnectionTest { hubConnection.start(); mockTransport.receiveMessage("{}" + RECORD_SEPARATOR); mockTransport.receiveMessage("{\"type\":1,\"target\":\"add\",\"arguments\":[12]}" + RECORD_SEPARATOR); - hubConnection.send("add", 12); // Confirming that our handler was called and the correct message was passed in. assertEquals(Double.valueOf(24), value.get()); } + @Test + public void invokeWaitsForCompletionMessage() throws Exception { + MockTransport mockTransport = new MockTransport(); + HubConnection hubConnection = new HubConnection("http://example.com", mockTransport, true); + + hubConnection.start(); + mockTransport.receiveMessage("{}" + RECORD_SEPARATOR); + + CompletableFuture result = hubConnection.invoke(Integer.class, "echo", "message"); + assertEquals("{\"type\":1,\"invocationId\":\"1\",\"target\":\"echo\",\"arguments\":[\"message\"]}" + RECORD_SEPARATOR, mockTransport.sentMessages.get(1)); + assertFalse(result.isDone()); + + mockTransport.receiveMessage("{\"type\":3,\"invocationId\":\"1\",\"result\":42}" + RECORD_SEPARATOR); + + assertEquals(Integer.valueOf(42), result.get(1000L, TimeUnit.MILLISECONDS)); + } + + @Test + public void multipleInvokesWaitForOwnCompletionMessage() throws Exception { + MockTransport mockTransport = new MockTransport(); + HubConnection hubConnection = new HubConnection("http://example.com", mockTransport, true); + + hubConnection.start(); + mockTransport.receiveMessage("{}" + RECORD_SEPARATOR); + + CompletableFuture result = hubConnection.invoke(Integer.class, "echo", "message"); + CompletableFuture result2 = hubConnection.invoke(String.class, "echo", "message"); + assertEquals("{\"type\":1,\"invocationId\":\"1\",\"target\":\"echo\",\"arguments\":[\"message\"]}" + RECORD_SEPARATOR, mockTransport.sentMessages.get(1)); + assertEquals("{\"type\":1,\"invocationId\":\"2\",\"target\":\"echo\",\"arguments\":[\"message\"]}" + RECORD_SEPARATOR, mockTransport.sentMessages.get(2)); + assertFalse(result.isDone()); + assertFalse(result2.isDone()); + + mockTransport.receiveMessage("{\"type\":3,\"invocationId\":\"2\",\"result\":\"message\"}" + RECORD_SEPARATOR); + assertEquals("message", result2.get(1000L, TimeUnit.MILLISECONDS)); + assertFalse(result.isDone()); + + mockTransport.receiveMessage("{\"type\":3,\"invocationId\":\"1\",\"result\":42}" + RECORD_SEPARATOR); + assertEquals(Integer.valueOf(42), result.get(1000L, TimeUnit.MILLISECONDS)); + } + + @Test + public void invokeWorksForPrimitiveTypes() throws Exception { + MockTransport mockTransport = new MockTransport(); + HubConnection hubConnection = new HubConnection("http://example.com", mockTransport, true); + + hubConnection.start(); + mockTransport.receiveMessage("{}" + RECORD_SEPARATOR); + + // int.class is a primitive type and since we use Class.cast to cast an Object to the expected return type + // which does not work for primitives we have to write special logic for that case. + CompletableFuture result = hubConnection.invoke(int.class, "echo", "message"); + assertFalse(result.isDone()); + + mockTransport.receiveMessage("{\"type\":3,\"invocationId\":\"1\",\"result\":42}" + RECORD_SEPARATOR); + + assertEquals(Integer.valueOf(42), result.get(1000L, TimeUnit.MILLISECONDS)); + } + + @Test + public void completionMessageCanHaveError() throws Exception { + MockTransport mockTransport = new MockTransport(); + HubConnection hubConnection = new HubConnection("http://example.com", mockTransport, true); + + hubConnection.start(); + mockTransport.receiveMessage("{}" + RECORD_SEPARATOR); + + CompletableFuture result = hubConnection.invoke(int.class, "echo", "message"); + assertFalse(result.isDone()); + + mockTransport.receiveMessage("{\"type\":3,\"invocationId\":\"1\",\"error\":\"There was an error\"}" + RECORD_SEPARATOR); + + String exceptionMessage = null; + try { + result.get(1000L, TimeUnit.MILLISECONDS); + assertFalse(true); + } catch (Exception ex) { + exceptionMessage = ex.getMessage(); + } + + assertEquals("com.microsoft.aspnet.signalr.HubException: There was an error", exceptionMessage); + } + + @Test + public void stopCancelsActiveInvokes() throws Exception { + MockTransport mockTransport = new MockTransport(); + HubConnection hubConnection = new HubConnection("http://example.com", mockTransport, true); + + hubConnection.start(); + mockTransport.receiveMessage("{}" + RECORD_SEPARATOR); + + CompletableFuture result = hubConnection.invoke(int.class, "echo", "message"); + assertFalse(result.isDone()); + + hubConnection.stop(); + + boolean hasException = false; + try { + result.get(1000L, TimeUnit.MILLISECONDS); + assertFalse(true); + } catch (CancellationException ex) { + hasException = true; + } + + assertTrue(hasException); + } + @Test public void sendWithNoParamsTriggersOnHandler() throws Exception { AtomicReference value = new AtomicReference<>(0); diff --git a/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/JsonHubProtocolTest.java b/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/JsonHubProtocolTest.java index ed9717f3af..08fe5c1065 100644 --- a/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/JsonHubProtocolTest.java +++ b/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/JsonHubProtocolTest.java @@ -32,7 +32,7 @@ public class JsonHubProtocolTest { @Test public void verifyWriteMessage() { - InvocationMessage invocationMessage = new InvocationMessage("test", new Object[] {"42"}); + InvocationMessage invocationMessage = new InvocationMessage(null, "test", new Object[] {"42"}); String result = jsonHubProtocol.writeMessage(invocationMessage); String expectedResult = "{\"type\":1,\"target\":\"test\",\"arguments\":[\"42\"]}\u001E"; assertEquals(expectedResult, result); @@ -89,7 +89,7 @@ public class JsonHubProtocolTest { @Test public void parseSingleMessage() throws Exception { String stringifiedMessage = "{\"type\":1,\"target\":\"test\",\"arguments\":[42]}\u001E"; - TestBinder binder = new TestBinder(new InvocationMessage("test", new Object[] { 42 })); + TestBinder binder = new TestBinder(new InvocationMessage("1", "test", new Object[] { 42 })); HubMessage[] messages = jsonHubProtocol.parseMessages(stringifiedMessage, binder); @@ -135,19 +135,10 @@ public class JsonHubProtocolTest { assertEquals("The message type CANCEL_INVOCATION is not supported yet.", exception.getMessage()); } - @Test - public void parseSingleUnsupportedCompletionMessage() throws Exception { - String stringifiedMessage = "{\"type\":3,\"invocationId\":123}\u001E"; - TestBinder binder = new TestBinder(null); - - Throwable exception = assertThrows(UnsupportedOperationException.class, () -> jsonHubProtocol.parseMessages(stringifiedMessage, binder)); - assertEquals("The message type COMPLETION is not supported yet.", exception.getMessage()); - } - @Test public void parseTwoMessages() throws Exception { String twoMessages = "{\"type\":1,\"target\":\"one\",\"arguments\":[42]}\u001E{\"type\":1,\"target\":\"two\",\"arguments\":[43]}\u001E"; - TestBinder binder = new TestBinder(new InvocationMessage("one", new Object[] { 42 })); + TestBinder binder = new TestBinder(new InvocationMessage("1", "one", new Object[] { 42 })); HubMessage[] messages = jsonHubProtocol.parseMessages(twoMessages, binder); assertEquals(2, messages.length); @@ -178,7 +169,7 @@ public class JsonHubProtocolTest { @Test public void parseSingleMessageMutipleArgs() throws Exception { String stringifiedMessage = "{\"type\":1,\"target\":\"test\",\"arguments\":[42, 24]}\u001E"; - TestBinder binder = new TestBinder(new InvocationMessage("test", new Object[] { 42, 24 })); + TestBinder binder = new TestBinder(new InvocationMessage("1", "test", new Object[] { 42, 24 })); HubMessage[] messages = jsonHubProtocol.parseMessages(stringifiedMessage, binder); @@ -197,7 +188,7 @@ public class JsonHubProtocolTest { @Test public void parseMessageWithOutOfOrderProperties() throws Exception { String stringifiedMessage = "{\"arguments\":[42, 24],\"type\":1,\"target\":\"test\"}\u001E"; - TestBinder binder = new TestBinder(new InvocationMessage("test", new Object[] { 42, 24 })); + TestBinder binder = new TestBinder(new InvocationMessage("1", "test", new Object[] { 42, 24 })); HubMessage[] messages = jsonHubProtocol.parseMessages(stringifiedMessage, binder); @@ -213,8 +204,24 @@ public class JsonHubProtocolTest { assertEquals(24, messageResult2); } + @Test + public void parseCompletionMessageWithOutOfOrderProperties() throws Exception { + String stringifiedMessage = "{\"type\":3,\"result\":42,\"invocationId\":\"1\"}\u001E"; + TestBinder binder = new TestBinder(new CompletionMessage("1", 42, null)); + + HubMessage[] messages = jsonHubProtocol.parseMessages(stringifiedMessage, binder); + + // We know it's only one message + assertEquals(HubMessageType.COMPLETION, messages[0].getMessageType()); + + CompletionMessage message = (CompletionMessage) messages[0]; + assertEquals(null, message.getError()); + assertEquals(42 , message.getResult()); + } + private class TestBinder implements InvocationBinder { private Class[] paramTypes = null; + private Class returnType = null; public TestBinder(HubMessage expectedMessage) { if (expectedMessage == null) { @@ -238,6 +245,9 @@ public class JsonHubProtocolTest { break; case STREAM_ITEM: break; + case COMPLETION: + returnType = ((CompletionMessage)expectedMessage).getResult().getClass(); + break; default: break; } @@ -245,7 +255,7 @@ public class JsonHubProtocolTest { @Override public Class getReturnType(String invocationId) { - return null; + return returnType; } @Override From 6ba5e87b4587657cef9bc916b9fe99d7589e2c1f Mon Sep 17 00:00:00 2001 From: BrennanConroy Date: Wed, 19 Sep 2018 15:21:07 -0700 Subject: [PATCH 8/9] Allow CancellationToken in streaming hub methods (#2818) --- .../Internal/DefaultHubDispatcher.cs | 57 +++++++++--- .../Internal/HubMethodDescriptor.cs | 25 +++++- .../CancellationTokenExtensions.cs | 21 +++++ .../HubConnectionHandlerTestUtils/Hubs.cs | 50 +++++++++++ .../HubConnectionHandlerTests.cs | 89 +++++++++++++++++++ 5 files changed, 226 insertions(+), 16 deletions(-) create mode 100644 test/Microsoft.AspNetCore.SignalR.Tests.Utils/CancellationTokenExtensions.cs diff --git a/src/Microsoft.AspNetCore.SignalR.Core/Internal/DefaultHubDispatcher.cs b/src/Microsoft.AspNetCore.SignalR.Core/Internal/DefaultHubDispatcher.cs index d26d10ac89..70ef9734bd 100644 --- a/src/Microsoft.AspNetCore.SignalR.Core/Internal/DefaultHubDispatcher.cs +++ b/src/Microsoft.AspNetCore.SignalR.Core/Internal/DefaultHubDispatcher.cs @@ -188,11 +188,45 @@ namespace Microsoft.AspNetCore.SignalR.Internal { InitializeHub(hub, connection); - var result = await ExecuteHubMethod(methodExecutor, hub, hubMethodInvocationMessage.Arguments); + CancellationTokenSource cts = null; + var arguments = hubMethodInvocationMessage.Arguments; + if (descriptor.HasSyntheticArguments) + { + // In order to add the synthetic arguments we need a new array because the invocation array is too small (it doesn't know about synthetic arguments) + arguments = new object[descriptor.OriginalParameterTypes.Count]; + + var hubInvocationArgumentPointer = 0; + for (var parameterPointer = 0; parameterPointer < arguments.Length; parameterPointer++) + { + if (hubMethodInvocationMessage.Arguments.Length > hubInvocationArgumentPointer && + hubMethodInvocationMessage.Arguments[hubInvocationArgumentPointer].GetType() == descriptor.OriginalParameterTypes[parameterPointer]) + { + // The types match so it isn't a synthetic argument, just copy it into the arguments array + arguments[parameterPointer] = hubMethodInvocationMessage.Arguments[hubInvocationArgumentPointer]; + hubInvocationArgumentPointer++; + } + else + { + // This is the only synthetic argument type we currently support + if (descriptor.OriginalParameterTypes[parameterPointer] == typeof(CancellationToken)) + { + cts = CancellationTokenSource.CreateLinkedTokenSource(connection.ConnectionAborted); + arguments[parameterPointer] = cts.Token; + } + else + { + // This should never happen + Debug.Assert(false, $"Failed to bind argument of type '{descriptor.OriginalParameterTypes[parameterPointer].Name}' for hub method '{methodExecutor.MethodInfo.Name}'."); + } + } + } + } + + var result = await ExecuteHubMethod(methodExecutor, hub, arguments); if (isStreamedInvocation) { - if (!TryGetStreamingEnumerator(connection, hubMethodInvocationMessage.InvocationId, descriptor, result, out var enumerator, out var streamCts)) + if (!TryGetStreamingEnumerator(connection, hubMethodInvocationMessage.InvocationId, descriptor, result, out var enumerator, ref cts)) { Log.InvalidReturnValueFromStreamingMethod(_logger, methodExecutor.MethodInfo.Name); @@ -204,7 +238,7 @@ namespace Microsoft.AspNetCore.SignalR.Internal disposeScope = false; Log.StreamingResult(_logger, hubMethodInvocationMessage.InvocationId, methodExecutor); // Fire-and-forget stream invocations, otherwise they would block other hub invocations from being able to run - _ = StreamResultsAsync(hubMethodInvocationMessage.InvocationId, connection, enumerator, scope, hubActivator, hub, streamCts); + _ = StreamResultsAsync(hubMethodInvocationMessage.InvocationId, connection, enumerator, scope, hubActivator, hub, cts); } // Non-empty/null InvocationId ==> Blocking invocation that needs a response else if (!string.IsNullOrEmpty(hubMethodInvocationMessage.InvocationId)) @@ -375,29 +409,24 @@ namespace Microsoft.AspNetCore.SignalR.Internal return true; } - private bool TryGetStreamingEnumerator(HubConnectionContext connection, string invocationId, HubMethodDescriptor hubMethodDescriptor, object result, out IAsyncEnumerator enumerator, out CancellationTokenSource streamCts) + private bool TryGetStreamingEnumerator(HubConnectionContext connection, string invocationId, HubMethodDescriptor hubMethodDescriptor, object result, out IAsyncEnumerator enumerator, ref CancellationTokenSource streamCts) { if (result != null) { if (hubMethodDescriptor.IsChannel) { - streamCts = CreateCancellation(); + if (streamCts == null) + { + streamCts = CancellationTokenSource.CreateLinkedTokenSource(connection.ConnectionAborted); + } + connection.ActiveRequestCancellationSources.TryAdd(invocationId, streamCts); enumerator = hubMethodDescriptor.FromChannel(result, streamCts.Token); return true; } } - streamCts = null; enumerator = null; return false; - - CancellationTokenSource CreateCancellation() - { - var userCts = new CancellationTokenSource(); - connection.ActiveRequestCancellationSources.TryAdd(invocationId, userCts); - - return CancellationTokenSource.CreateLinkedTokenSource(connection.ConnectionAborted, userCts.Token); - } } private void DiscoverHubMethods() diff --git a/src/Microsoft.AspNetCore.SignalR.Core/Internal/HubMethodDescriptor.cs b/src/Microsoft.AspNetCore.SignalR.Core/Internal/HubMethodDescriptor.cs index a15dce772e..6c7c895659 100644 --- a/src/Microsoft.AspNetCore.SignalR.Core/Internal/HubMethodDescriptor.cs +++ b/src/Microsoft.AspNetCore.SignalR.Core/Internal/HubMethodDescriptor.cs @@ -22,8 +22,6 @@ namespace Microsoft.AspNetCore.SignalR.Internal public HubMethodDescriptor(ObjectMethodExecutor methodExecutor, IEnumerable policies) { MethodExecutor = methodExecutor; - ParameterTypes = methodExecutor.MethodParameters.Select(p => p.ParameterType).ToArray(); - Policies = policies.ToArray(); NonAsyncReturnType = (MethodExecutor.IsMethodAsync) ? MethodExecutor.AsyncResultType @@ -34,6 +32,25 @@ namespace Microsoft.AspNetCore.SignalR.Internal IsChannel = true; StreamReturnType = channelItemType; } + + // Take out synthetic arguments that will be provided by the server, this list will be given to the protocol parsers + ParameterTypes = methodExecutor.MethodParameters.Where(p => + { + // Only streams can take CancellationTokens currently + if (IsStreamable && p.ParameterType == typeof(CancellationToken)) + { + HasSyntheticArguments = true; + return false; + } + return true; + }).Select(p => p.ParameterType).ToArray(); + + if (HasSyntheticArguments) + { + OriginalParameterTypes = methodExecutor.MethodParameters.Select(p => p.ParameterType).ToArray(); + } + + Policies = policies.ToArray(); } private Func> _convertToEnumerator; @@ -42,6 +59,8 @@ namespace Microsoft.AspNetCore.SignalR.Internal public IReadOnlyList ParameterTypes { get; } + public IReadOnlyList OriginalParameterTypes { get; } + public Type NonAsyncReturnType { get; } public bool IsChannel { get; } @@ -52,6 +71,8 @@ namespace Microsoft.AspNetCore.SignalR.Internal public IList Policies { get; } + public bool HasSyntheticArguments { get; private set; } + private static bool IsChannelType(Type type, out Type payloadType) { var channelType = type.AllBaseTypes().FirstOrDefault(t => t.IsGenericType && t.GetGenericTypeDefinition() == typeof(ChannelReader<>)); diff --git a/test/Microsoft.AspNetCore.SignalR.Tests.Utils/CancellationTokenExtensions.cs b/test/Microsoft.AspNetCore.SignalR.Tests.Utils/CancellationTokenExtensions.cs new file mode 100644 index 0000000000..fadb9626bf --- /dev/null +++ b/test/Microsoft.AspNetCore.SignalR.Tests.Utils/CancellationTokenExtensions.cs @@ -0,0 +1,21 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Threading; +using System.Threading.Tasks; + +namespace Microsoft.AspNetCore.SignalR.Tests +{ + public static class CancellationTokenExtensions + { + public static Task WaitForCancellationAsync(this CancellationToken token) + { + var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + token.Register((t) => + { + ((TaskCompletionSource)t).SetResult(null); + }, tcs); + return tcs.Task; + } + } +} diff --git a/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTestUtils/Hubs.cs b/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTestUtils/Hubs.cs index 7eb0aec498..37fc164c14 100644 --- a/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTestUtils/Hubs.cs +++ b/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTestUtils/Hubs.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Threading; using System.Threading.Channels; using System.Threading.Tasks; using Microsoft.AspNetCore.Authorization; @@ -165,6 +166,10 @@ namespace Microsoft.AspNetCore.SignalR.Tests return Clients.Caller.SendAsync("Send", new string('x', 3000), new SelfRef()); } + public void InvalidArgument(CancellationToken token) + { + } + private class SelfRef { public SelfRef() @@ -547,6 +552,51 @@ namespace Microsoft.AspNetCore.SignalR.Tests return Channel.CreateUnbounded().Reader; } + public ChannelReader CancelableStream(CancellationToken token) + { + var channel = Channel.CreateBounded(10); + + Task.Run(async () => + { + _tcsService.StartedMethod.SetResult(null); + await token.WaitForCancellationAsync(); + channel.Writer.TryComplete(); + _tcsService.EndMethod.SetResult(null); + }); + + return channel.Reader; + } + + public ChannelReader CancelableStream2(int ignore, int ignore2, CancellationToken token) + { + var channel = Channel.CreateBounded(10); + + Task.Run(async () => + { + _tcsService.StartedMethod.SetResult(null); + await token.WaitForCancellationAsync(); + channel.Writer.TryComplete(); + _tcsService.EndMethod.SetResult(null); + }); + + return channel.Reader; + } + + public ChannelReader CancelableStreamMiddle(int ignore, CancellationToken token, int ignore2) + { + var channel = Channel.CreateBounded(10); + + Task.Run(async () => + { + _tcsService.StartedMethod.SetResult(null); + await token.WaitForCancellationAsync(); + channel.Writer.TryComplete(); + _tcsService.EndMethod.SetResult(null); + }); + + return channel.Reader; + } + public int SimpleMethod() { return 21; diff --git a/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTests.cs b/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTests.cs index c637f9e131..c8d884ec5f 100644 --- a/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTests.cs +++ b/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTests.cs @@ -2381,6 +2381,95 @@ namespace Microsoft.AspNetCore.SignalR.Tests } } + [Theory] + [InlineData(nameof(LongRunningHub.CancelableStream))] + [InlineData(nameof(LongRunningHub.CancelableStream2), 1, 2)] + [InlineData(nameof(LongRunningHub.CancelableStreamMiddle), 1, 2)] + public async Task StreamHubMethodCanAcceptCancellationTokenAsArgumentAndBeTriggeredOnCancellation(string methodName, params object[] args) + { + var tcsService = new TcsService(); + var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(builder => + { + builder.AddSingleton(tcsService); + }); + var connectionHandler = serviceProvider.GetService>(); + + using (var client = new TestClient()) + { + var connectionHandlerTask = await client.ConnectAsync(connectionHandler).OrTimeout(); + + var streamInvocationId = await client.SendStreamInvocationAsync(methodName, args).OrTimeout(); + // Wait for the stream method to start + await tcsService.StartedMethod.Task.OrTimeout(); + + // Cancel the stream which should trigger the CancellationToken in the hub method + await client.SendHubMessageAsync(new CancelInvocationMessage(streamInvocationId)).OrTimeout(); + + var result = await client.ReadAsync().OrTimeout(); + + var simpleCompletion = Assert.IsType(result); + Assert.Null(simpleCompletion.Result); + + // CancellationToken passed to hub method will allow EndMethod to be triggered if it is canceled. + await tcsService.EndMethod.Task.OrTimeout(); + + // Shut down + client.Dispose(); + + await connectionHandlerTask.OrTimeout(); + } + } + + [Fact] + public async Task StreamHubMethodCanAcceptCancellationTokenAsArgumentAndBeTriggeredOnConnectionAborted() + { + var tcsService = new TcsService(); + var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(builder => + { + builder.AddSingleton(tcsService); + }); + var connectionHandler = serviceProvider.GetService>(); + + using (var client = new TestClient()) + { + var connectionHandlerTask = await client.ConnectAsync(connectionHandler).OrTimeout(); + + var streamInvocationId = await client.SendStreamInvocationAsync(nameof(LongRunningHub.CancelableStream)).OrTimeout(); + // Wait for the stream method to start + await tcsService.StartedMethod.Task.OrTimeout(); + + // Shut down the client which should trigger the CancellationToken in the hub method + client.Dispose(); + + // CancellationToken passed to hub method will allow EndMethod to be triggered if it is canceled. + await tcsService.EndMethod.Task.OrTimeout(); + + await connectionHandlerTask.OrTimeout(); + } + } + + [Fact] + public async Task InvokeHubMethodCannotAcceptCancellationTokenAsArgument() + { + var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(); + var connectionHandler = serviceProvider.GetService>(); + + using (var client = new TestClient()) + { + var connectionHandlerTask = await client.ConnectAsync(connectionHandler).OrTimeout(); + + var invocationId = await client.SendInvocationAsync(nameof(MethodHub.InvalidArgument)).OrTimeout(); + + var completion = Assert.IsType(await client.ReadAsync().OrTimeout()); + + Assert.Equal("Failed to invoke 'InvalidArgument' due to an error on the server.", completion.Error); + + client.Dispose(); + + await connectionHandlerTask.OrTimeout(); + } + } + private class CustomHubActivator : IHubActivator where THub : Hub { public int ReleaseCount; From f88b7ce0446b703deb401bd6fe26c52a30f60896 Mon Sep 17 00:00:00 2001 From: Mikael Mengistu Date: Thu, 20 Sep 2018 10:33:16 -0700 Subject: [PATCH 9/9] Move Java chat sample to samples package (#2974) --- .vscode/launch.json | 2 +- .../java/com/microsoft/aspnet/signalr/sample}/Chat.java | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) rename clients/java/signalr/src/{main/java/com/microsoft/aspnet/signalr => test/java/com/microsoft/aspnet/signalr/sample}/Chat.java (87%) diff --git a/.vscode/launch.json b/.vscode/launch.json index 06a988616d..8a35acc3ed 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -8,7 +8,7 @@ "cwd": "${workspaceFolder}/clients/java/", "console": "externalTerminal", "stopOnEntry": false, - "mainClass": "com.microsoft.aspnet.signalr.Chat", + "mainClass": "com.microsoft.aspnet.signalr.sample.Chat", "args": "" }, { diff --git a/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/Chat.java b/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/sample/Chat.java similarity index 87% rename from clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/Chat.java rename to clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/sample/Chat.java index c306c26eef..cec5fe913c 100644 --- a/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/Chat.java +++ b/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/sample/Chat.java @@ -1,10 +1,14 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -package com.microsoft.aspnet.signalr; +package com.microsoft.aspnet.signalr.sample; import java.util.Scanner; +import com.microsoft.aspnet.signalr.HubConnection; +import com.microsoft.aspnet.signalr.HubConnectionBuilder; +import com.microsoft.aspnet.signalr.LogLevel; + public class Chat { public static void main(String[] args) throws Exception { System.out.println("Enter the URL of the SignalR Chat you want to join");