RavioliCompositionOverSpaghettiInheritance - refactoring usage of Web…

…SocketHandler on the client

WebSocketHandler class is used on the server and on the client. While on the server we just instantiate this type on the client the WebSocketTransport derives from it. This blocks creating a base class for client transport which would contain a common functionality (e.g. parsing the response). After this change WebSocketTransport no longer derives from the WebSocketHandler but instantiates it and uses it internally.

src/Microsoft.AspNet.SignalR.Client45/Microsoft.AspNet.SignalR.Client45.csproj

@@ -243,6 +243,7 @@
     </Compile>
     <Compile Include="Infrastructure\HttpClientException.cs" />
     <Compile Include="Properties\AssemblyInfo.cs" />
+    <Compile Include="Transports\WebSockets\ClientWebSocketHandler.cs" />
     <Compile Include="Transports\WebSockets\WebSocketWrapperRequest.cs" />
     <Compile Include="Transports\WebSocketTransport.cs" />
   </ItemGroup>

src/Microsoft.AspNet.SignalR.Client45/Transports/WebSocketTransport.cs

@@ -7,14 +7,15 @@
 using System.Threading.Tasks;
 using Microsoft.AspNet.SignalR.Client.Http;
 using Microsoft.AspNet.SignalR.Client.Infrastructure;
-using Microsoft.AspNet.SignalR.WebSockets;
+using Microsoft.AspNet.SignalR.Client.Transports.WebSockets;
 
 namespace Microsoft.AspNet.SignalR.Client.Transports
 {
-    public class WebSocketTransport : WebSocketHandler, IClientTransport
+    public class WebSocketTransport : IClientTransport
     {
         private readonly IHttpClient _client;
         private readonly TransportAbortHandler _abortHandler;
+        private readonly ClientWebSocketHandler _webSocketHandler;
         private CancellationToken _disconnectToken;
         private TransportInitializationHandler _initializeHandler;
         private WebSocketConnectionInfo _connectionInfo;
@@ -29,12 +30,19 @@ public WebSocketTransport()
         }
 
         public WebSocketTransport(IHttpClient client)
-            : base(maxIncomingMessageSize: null) // Disable max incoming message size on the client
         {
             _client = client;
             _disconnectToken = CancellationToken.None;
             _abortHandler = new TransportAbortHandler(client, Name);
             ReconnectDelay = TimeSpan.FromSeconds(2);
+            _webSocketHandler = new ClientWebSocketHandler(this);
+        }
+
+        // intended for testing
+        internal WebSocketTransport(ClientWebSocketHandler webSocketHandler)
+            : this()
+        {
+            _webSocketHandler = webSocketHandler;
         }
 
         /// <summary>
@@ -133,7 +141,7 @@ private async Task PerformConnect(bool reconnecting)
             CancellationToken token = linkedCts.Token;
 
             await _webSocket.ConnectAsync(builder.Uri, token);
-            await ProcessWebSocketRequestAsync(_webSocket, token);
+            await _webSocketHandler.ProcessWebSocketRequestAsync(_webSocket, token);
         }
 
         public void Abort(IConnection connection, TimeSpan timeout, string connectionData)
@@ -149,18 +157,19 @@ public Task Send(IConnection connection, string data, string connectionData)
             }
 
             // If we don't throw here when the WebSocket isn't open, WebSocketHander.SendAsync will noop.
-            if (WebSocket.State != WebSocketState.Open)
+            if (_webSocketHandler.WebSocket.State != WebSocketState.Open)
             {
                 // Make this a faulted task and trigger the OnError even to maintain consistency with the HttpBasedTransports
                 var ex = new InvalidOperationException(Resources.Error_DataCannotBeSentDuringWebSocketReconnect);
                 connection.OnError(ex);
                 return TaskAsyncHelper.FromError(ex);
             }
 
-            return SendAsync(data);
+            return _webSocketHandler.SendAsync(data);
         }
 
-        public override void OnMessage(string message)
+        // virtual for testing
+        internal virtual void OnMessage(string message)
         {
             _connectionInfo.Connection.Trace(TraceLevels.Messages, "WS: OnMessage({0})", message);
 
@@ -179,7 +188,8 @@ public override void OnMessage(string message)
             }
         }
 
-        public override void OnOpen()
+        // virtual for testing
+        internal virtual void OnOpen()
         {
             // This will noop if we're not in the reconnecting state
             if (_connectionInfo.Connection.ChangeState(ConnectionState.Reconnecting, ConnectionState.Connected))
@@ -188,7 +198,8 @@ public override void OnOpen()
             }
         }
 
-        public override void OnClose()
+        // virtual for testing
+        internal virtual void OnClose()
         {
             _connectionInfo.Connection.Trace(TraceLevels.Events, "WS: OnClose()");
 
@@ -232,9 +243,10 @@ private async void DoReconnect()
             }
         }
 
-        public override void OnError()
+        // virtual for testing
+        internal virtual void OnError(Exception error)
         {
-            _connectionInfo.Connection.OnError(Error);
+            _connectionInfo.Connection.OnError(error);
         }
 
         public void LostConnection(IConnection connection)

src/Microsoft.AspNet.SignalR.Client45/Transports/WebSockets/ClientWebSocketHandler.cs

@@ -0,0 +1,46 @@
+// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. See License.md in the project root for license information.
+
+using System.Diagnostics;
+using Microsoft.AspNet.SignalR.WebSockets;
+
+namespace Microsoft.AspNet.SignalR.Client.Transports.WebSockets
+{
+    internal class ClientWebSocketHandler : WebSocketHandler
+    {
+        private readonly WebSocketTransport _webSocketTransport;
+
+        public ClientWebSocketHandler(WebSocketTransport webSocketTransport)
+            : base(maxIncomingMessageSize: null)
+        {
+            Debug.Assert(webSocketTransport != null, "webSocketTransport is null");
+
+            _webSocketTransport = webSocketTransport;
+        }
+
+        // for mocking
+        internal ClientWebSocketHandler()
+            : base(maxIncomingMessageSize: null)
+        {
+        }
+
+        public override void OnMessage(string message)
+        {
+            _webSocketTransport.OnMessage(message);
+        }
+
+        public override void OnOpen()
+        {
+            _webSocketTransport.OnOpen();
+        }
+
+        public override void OnClose()
+        {
+            _webSocketTransport.OnClose();
+        }
+
+        public override void OnError()
+        {
+            _webSocketTransport.OnError(Error);
+        }
+    }
+}

tests/Microsoft.AspNet.SignalR.Client.Tests/Client/TransportFacts.cs

@@ -7,6 +7,7 @@
 using Microsoft.AspNet.SignalR.Client.Http;
 using Microsoft.AspNet.SignalR.Client.Infrastructure;
 using Microsoft.AspNet.SignalR.Client.Transports;
+using Microsoft.AspNet.SignalR.Client.Transports.WebSockets;
 using Moq;
 using Newtonsoft.Json;
 using Newtonsoft.Json.Linq;
@@ -244,13 +245,13 @@ public void WebSocketSendReturnsAFaultedTaskWhenNotConnected(WebSocketState stat
         {
             var mockConnection = new Mock<Client.IConnection>(MockBehavior.Strict);
             var mockWebSocket = new Mock<WebSocket>(MockBehavior.Strict);
+            var mockWebSocketHandler = new Mock<ClientWebSocketHandler>();
 
             mockWebSocket.SetupGet(ws => ws.State).Returns(state);
             mockConnection.Setup(c => c.OnError(It.IsAny<InvalidOperationException>()));
-
-            var wsTransport = new WebSocketTransport();
-
-            wsTransport.WebSocket = mockWebSocket.Object;
+            mockWebSocketHandler.Object.WebSocket = mockWebSocket.Object;
+
+            var wsTransport = new WebSocketTransport(mockWebSocketHandler.Object);
 
             var task = wsTransport.Send(mockConnection.Object, "", "");
 

tests/Microsoft.AspNet.SignalR.Client.Tests/Client/Transports/WebSockets/ClientWebSocketHandlerFacts.cs

@@ -0,0 +1,57 @@
+
+using System;
+using Moq;
+using Xunit;
+
+namespace Microsoft.AspNet.SignalR.Client.Transports.WebSockets
+{
+    public class ClientWebSocketHandlerFacts
+    {
+        [Fact]
+        public void OnOpenCallsIntoWebSocketTransportOnMessage()
+        {
+            var mockWebSocketTransport = new Mock<WebSocketTransport>();
+
+            var webSocketHandler = new ClientWebSocketHandler(mockWebSocketTransport.Object);
+            webSocketHandler.OnOpen();
+
+            mockWebSocketTransport.Verify(p => p.OnOpen(), Times.Once());
+        }
+
+        [Fact]
+        public void OnMessageCallsIntoWebSocketTransportOnMessage()
+        {
+            var mockWebSocketTransport = new Mock<WebSocketTransport>();
+
+            var webSocketHandler = new ClientWebSocketHandler(mockWebSocketTransport.Object);
+            webSocketHandler.OnMessage("msg");
+
+            mockWebSocketTransport.Verify(p => p.OnMessage("msg"), Times.Once());
+        }
+
+        [Fact]
+        public void OnCloseCallsIntoWebSocketTransportOnClose()
+        {
+            var mockWebSocketTransport = new Mock<WebSocketTransport>();
+
+            var webSocketHandler = new ClientWebSocketHandler(mockWebSocketTransport.Object);
+            webSocketHandler.OnClose();
+
+            mockWebSocketTransport.Verify(p => p.OnClose(), Times.Once());
+        }
+
+        [Fact]
+        public void OnErrorCallsIntoWebSocketTransportOnErrorAndPassesException()
+        {
+            var exception = new Exception();
+            var mockWebSocketTransport = new Mock<WebSocketTransport>();
+
+            var webSocketHandler =
+                new ClientWebSocketHandler(mockWebSocketTransport.Object) { Error = exception };
+
+            webSocketHandler.OnError();
+
+            mockWebSocketTransport.Verify(p => p.OnError(exception), Times.Once());
+        }
+    }
+}

tests/Microsoft.AspNet.SignalR.Client.Tests/Microsoft.AspNet.SignalR.Client.Tests.csproj

@@ -63,6 +63,7 @@
     <Compile Include="Client\Infrastructure\UrlBuilderFacts.cs" />
     <Compile Include="Client\KeepAliveFacts.cs" />
     <Compile Include="Client\Transports\TransportFacts.cs" />
+    <Compile Include="Client\Transports\WebSockets\ClientWebSocketHandlerFacts.cs" />
     <Compile Include="EventSourceStreamReaderFacts.cs" />
     <Compile Include="Properties\AssemblyInfo.cs" />
     <Compile Include="Client\TransportFacts.cs" />