Fixed previous fixes to Issue #598

Fixed bug in JS and .NET clients where StateChanged events would be passed a copy of the new state in lieu of the old state.
SignalR · Oct 16, 2012 · 5d1e1ab · 5d1e1ab
1 parent b0b2e81
commit 5d1e1ab
Show file tree

Hide file tree

Showing 9 changed files with 53 additions and 13 deletions.
diff --git a/samples/Microsoft.AspNet.SignalR.Hosting.AspNet.Samples/Scripts/jquery.signalR.js b/samples/Microsoft.AspNet.SignalR.Hosting.AspNet.Samples/Scripts/jquery.signalR.js
@@ -74,7 +74,7 @@
         changeState = function (connection, expectedState, newState) {
             if (expectedState === connection.state) {
                 connection.state = newState;
-                $(connection).trigger(events.onStateChanged, [{ oldState: connection.state, newState: newState }]);
+                $(connection).trigger(events.onStateChanged, [{ oldState: expectedState, newState: newState }]);
                 return true;
             }
 
@@ -1184,6 +1184,9 @@
             url = transportLogic.getUrl(connection, this.name);
             url += "&frameId=" + frameId;
 
+            // Set body prior to setting URL to avoid caching issues.
+            $("body").append(frame);
+
             frame.prop("src", url);
             transportLogic.foreverFrame.connections[frameId] = connection;
 
@@ -1203,8 +1206,6 @@
                 connection.onSuccess = onSuccess;
             }
 
-            $("body").append(frame);
-
             // After connecting, if after the specified timeout there's no response stop the connection
             // and raise on failed
             // REVIEW: Why is connectTimeOut set here and never used again?

diff --git a/samples/Microsoft.AspNet.SignalR.Hosting.AspNet.Samples/Scripts/jquery.signalR.min.js b/samples/Microsoft.AspNet.SignalR.Hosting.AspNet.Samples/Scripts/jquery.signalR.min.js
diff --git a/samples/Microsoft.AspNet.SignalR.Hosting.AspNet45.Samples/Scripts/jquery.signalR.js b/samples/Microsoft.AspNet.SignalR.Hosting.AspNet45.Samples/Scripts/jquery.signalR.js
@@ -74,7 +74,7 @@
         changeState = function (connection, expectedState, newState) {
             if (expectedState === connection.state) {
                 connection.state = newState;
-                $(connection).trigger(events.onStateChanged, [{ oldState: connection.state, newState: newState }]);
+                $(connection).trigger(events.onStateChanged, [{ oldState: expectedState, newState: newState }]);
                 return true;
             }
 
@@ -1184,6 +1184,9 @@
             url = transportLogic.getUrl(connection, this.name);
             url += "&frameId=" + frameId;
 
+            // Set body prior to setting URL to avoid caching issues.
+            $("body").append(frame);
+
             frame.prop("src", url);
             transportLogic.foreverFrame.connections[frameId] = connection;
 
@@ -1203,8 +1206,6 @@
                 connection.onSuccess = onSuccess;
             }
 
-            $("body").append(frame);
-
             // After connecting, if after the specified timeout there's no response stop the connection
             // and raise on failed
             // REVIEW: Why is connectTimeOut set here and never used again?

diff --git a/samples/Microsoft.AspNet.SignalR.Hosting.AspNet45.Samples/Scripts/jquery.signalR.min.js b/samples/Microsoft.AspNet.SignalR.Hosting.AspNet45.Samples/Scripts/jquery.signalR.min.js
diff --git a/src/Microsoft.AspNet.SignalR.Client.JS/jquery.signalR.core.js b/src/Microsoft.AspNet.SignalR.Client.JS/jquery.signalR.core.js
@@ -73,7 +73,7 @@
         changeState = function (connection, expectedState, newState) {
             if (expectedState === connection.state) {
                 connection.state = newState;
-                $(connection).trigger(events.onStateChanged, [{ oldState: connection.state, newState: newState }]);
+                $(connection).trigger(events.onStateChanged, [{ oldState: expectedState, newState: newState }]);
                 return true;
             }
 

diff --git a/src/Microsoft.AspNet.SignalR.Client/Connection.cs b/src/Microsoft.AspNet.SignalR.Client/Connection.cs
@@ -162,10 +162,11 @@ private set
                 {
                     if (_state != value)
                     {
+                        var stateChange = new StateChange(oldState: _state, newState: value);
                         _state = value;
                         if (StateChanged != null)
                         {
-                            StateChanged(new StateChange(_state, value));
+                            StateChanged(stateChange);
                         }
                     }
                 }

diff --git a/tests/Microsoft.AspNet.SignalR.Client.JS.Tests/Tests/Connections/ConnectionStateFacts.js b/tests/Microsoft.AspNet.SignalR.Client.JS.Tests/Tests/Connections/ConnectionStateFacts.js
@@ -15,6 +15,13 @@ test("Changing State", function () {
     equal(con.changeState(con.fn, con.connectionState.disconnected, con.connectionState.connecting), true, "Changes state from disconnected to connecting.");
     equal(con.changeState(con.fn, con.connectionState.connected, con.connectionState.reconnecting), false, "Changing state from connected to connecting when state is connecting.");
 
+    con.fn.stateChanged(function (change) {
+        equal(change.oldState, con.connectionState.connecting, "Verifies that the proper old state is passed to the stateChanged event handler.");
+        equal(change.newState, con.connectionState.connected, "Verifies that the proper new state is passed to the stateChanged event handler.");
+        $(this).unbind(con.events.onStateChanged);
+    });
+    con.changeState(con.fn, con.connectionState.connecting, con.connectionState.connected);
+
     // Reset the connection state back to disconnected
-    con.changeState(con.fn, con.connectionState.connecting, con.connectionState.disconnected);
+    con.changeState(con.fn, con.connectionState.connected, con.connectionState.disconnected);
 });
diff --git a/tests/Microsoft.AspNet.SignalR.Tests/Client/ConnectionFacts.cs b/tests/Microsoft.AspNet.SignalR.Tests/Client/ConnectionFacts.cs
@@ -0,0 +1,29 @@
+using System;
+using Microsoft.AspNet.SignalR.Client;
+using Xunit;
+
+namespace Microsoft.AspNet.SignalR.Tests
+{
+    public class ConnectionFacts : IDisposable
+    {
+        [Fact]
+        public void ConnectionStateChangedEventIsCalledWithAppropriateArguments()
+        {
+            var connection = new Client.Connection("http://test");
+
+            connection.StateChanged += stateChange =>
+            {
+                Assert.Equal(ConnectionState.Disconnected, stateChange.OldState);
+                Assert.Equal(ConnectionState.Connecting, stateChange.NewState);
+            };
+
+            Assert.True(((Client.IConnection)connection).ChangeState(ConnectionState.Disconnected, ConnectionState.Connecting));
+        }
+
+        public void Dispose()
+        {
+            GC.Collect();
+            GC.WaitForPendingFinalizers();
+        }
+    }
+}
diff --git a/tests/Microsoft.AspNet.SignalR.Tests/Microsoft.AspNet.SignalR.Tests.csproj b/tests/Microsoft.AspNet.SignalR.Tests/Microsoft.AspNet.SignalR.Tests.csproj
@@ -1,4 +1,4 @@
-<?xml version="1.0" encoding="utf-8"?>
+<?xml version="1.0" encoding="utf-8"?>
 <Project ToolsVersion="4.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
   <PropertyGroup>
     <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
@@ -82,6 +82,7 @@
       <Link>Hubs\DemoHub.cs</Link>
     </Compile>
     <Compile Include="ChunkBufferFacts.cs" />
+    <Compile Include="Client\ConnectionFacts.cs" />
     <Compile Include="Infrastructure\CountDownRange.cs" />
     <Compile Include="Server\AckHandlerFacts.cs" />
     <Compile Include="Server\Hubs\HubAuthFacts.cs" />
@@ -133,4 +134,4 @@
   <Target Name="AfterBuild">
   </Target>
   -->
-</Project>
+</Project>