Remove the RequestId from DefaultConnectionContext (#516)

* Remove the RequestId from DefaultConnectionContext
- Added a GetHttpContext() extension method on ConnectionContext
- Also fixed an issue not setting LastSeenUtc

src/Microsoft.AspNetCore.SignalR.Http/HubRouteBuilder.cs

@@ -1,4 +1,7 @@
-using Microsoft.AspNetCore.Sockets;
+// 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 Microsoft.AspNetCore.Sockets;
 
 namespace Microsoft.AspNetCore.SignalR
 {

src/Microsoft.AspNetCore.Sockets.Abstractions/DefaultConnectionContext.cs

@@ -20,15 +20,13 @@ namespace Microsoft.AspNetCore.Sockets
             Transport = transport;
             Application = application;
             ConnectionId = id;
+            LastSeenUtc = DateTime.UtcNow;
         }
 
         public CancellationTokenSource Cancellation { get; set; }
 
         public SemaphoreSlim Lock { get; } = new SemaphoreSlim(1, 1);
 
-        // REVIEW: This should only be on the Http implementation
-        public string RequestId { get; set; }
-
         public Task TransportTask { get; set; }
 
         public Task ApplicationTask { get; set; }
@@ -65,8 +63,6 @@ namespace Microsoft.AspNetCore.Sockets
                 {
                     Status = ConnectionStatus.Disposed;
 
-                    RequestId = null;
-
                     // If the application task is faulted, propagate the error to the transport
                     if (ApplicationTask?.IsFaulted == true)
                     {

src/Microsoft.AspNetCore.Sockets.Http/HttpConnectionContextExtensions.cs

@@ -0,0 +1,18 @@
+// 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;
+using System.Collections.Generic;
+using System.Text;
+using Microsoft.AspNetCore.Http;
+
+namespace Microsoft.AspNetCore.Sockets
+{
+    public static class HttpConnectionContextExtensions
+    {
+        public static HttpContext GetHttpContext(this ConnectionContext connection)
+        {
+            return connection.Metadata.Get<HttpContext>(ConnectionMetadataNames.HttpContext);
+        }
+    }
+}

src/Microsoft.AspNetCore.Sockets.Http/HttpConnectionDispatcher.cs

@@ -141,7 +141,7 @@ namespace Microsoft.AspNetCore.Sockets
 
                     if (connection.Status == DefaultConnectionContext.ConnectionStatus.Active)
                     {
-                        _logger.LogDebug("Connection {connectionId} is already active via {requestId}. Cancelling previous request.", connection.ConnectionId, connection.RequestId);
+                        _logger.LogDebug("Connection {connectionId} is already active via {requestId}. Cancelling previous request.", connection.ConnectionId, connection.GetHttpContext().TraceIdentifier);
 
                         using (connection.Cancellation)
                         {
@@ -158,20 +158,17 @@ namespace Microsoft.AspNetCore.Sockets
                                 // Should be a cancelled task
                             }
 
-                            _logger.LogDebug("Previous poll cancelled for {connectionId} on {requestId}.", connection.ConnectionId, connection.RequestId);
+                            _logger.LogDebug("Previous poll cancelled for {connectionId} on {requestId}.", connection.ConnectionId, connection.GetHttpContext().TraceIdentifier);
                         }
                     }
 
-                    // Mark the request identifier
-                    connection.RequestId = context.TraceIdentifier;
-
                     // Mark the connection as active
                     connection.Status = DefaultConnectionContext.ConnectionStatus.Active;
 
                     // Raise OnConnected for new connections only since polls happen all the time
                     if (connection.ApplicationTask == null)
                     {
-                        _logger.LogDebug("Establishing new connection: {connectionId} on {requestId}", connection.ConnectionId, connection.RequestId);
+                        _logger.LogDebug("Establishing new connection: {connectionId} on {requestId}", connection.ConnectionId, connection.GetHttpContext().TraceIdentifier);
 
                         connection.Metadata[ConnectionMetadataNames.Transport] = TransportType.LongPolling;
 
@@ -179,7 +176,7 @@ namespace Microsoft.AspNetCore.Sockets
                     }
                     else
                     {
-                        _logger.LogDebug("Resuming existing connection: {connectionId} on {requestId}", connection.ConnectionId, connection.RequestId);
+                        _logger.LogDebug("Resuming existing connection: {connectionId} on {requestId}", connection.ConnectionId, connection.GetHttpContext().TraceIdentifier);
                     }
 
                     var longPolling = new LongPollingTransport(connection.Application.Input, _loggerFactory);
@@ -241,7 +238,7 @@ namespace Microsoft.AspNetCore.Sockets
 
                             connection.Status = DefaultConnectionContext.ConnectionStatus.Inactive;
 
-                            connection.RequestId = null;
+                            connection.Metadata[ConnectionMetadataNames.HttpContext] = null;
 
                             // Dispose the cancellation token
                             connection.Cancellation.Dispose();
@@ -257,16 +254,6 @@ namespace Microsoft.AspNetCore.Sockets
             }
         }
 
-        private DefaultConnectionContext CreateConnection(HttpContext context)
-        {
-            var connection = _manager.CreateConnection();
-            var format = (string)context.Request.Query[ConnectionMetadataNames.Format];
-            connection.User = context.User;
-            connection.Metadata[ConnectionMetadataNames.HttpContext] = context;
-            connection.Metadata[ConnectionMetadataNames.Format] = string.IsNullOrEmpty(format) ? "json" : format;
-            return connection;
-        }
-
         private async Task DoPersistentConnection(SocketDelegate socketDelegate,
                                                   IHttpTransport transport,
                                                   HttpContext context,
@@ -288,7 +275,7 @@ namespace Microsoft.AspNetCore.Sockets
                 // There's already an active request
                 if (connection.Status == DefaultConnectionContext.ConnectionStatus.Active)
                 {
-                    _logger.LogDebug("Connection {connectionId} is already active via {requestId}.", connection.ConnectionId, connection.RequestId);
+                    _logger.LogDebug("Connection {connectionId} is already active via {requestId}.", connection.ConnectionId, connection.GetHttpContext().TraceIdentifier);
 
                     // Reject the request with a 409 conflict
                     context.Response.StatusCode = StatusCodes.Status409Conflict;
@@ -298,9 +285,6 @@ namespace Microsoft.AspNetCore.Sockets
                 // Mark the connection as active
                 connection.Status = DefaultConnectionContext.ConnectionStatus.Active;
 
-                // Store the request identifier
-                connection.RequestId = context.TraceIdentifier;
-
                 // Call into the end point passing the connection
                 connection.ApplicationTask = ExecuteApplication(socketDelegate, connection);
 
@@ -336,7 +320,7 @@ namespace Microsoft.AspNetCore.Sockets
             context.Response.ContentType = "text/plain";
 
             // Establish the connection
-            var connection = CreateConnection(context);
+            var connection = _manager.CreateConnection();
 
             // Get the bytes for the connection id
             var connectionIdBuffer = Encoding.UTF8.GetBytes(connection.ConnectionId);
@@ -407,8 +391,6 @@ namespace Microsoft.AspNetCore.Sockets
                 return false;
             }
 
-            connection.User = context.User;
-
             var transport = connection.Metadata.Get<TransportType?>(ConnectionMetadataNames.Transport);
 
             if (transport == null)
@@ -421,6 +403,13 @@ namespace Microsoft.AspNetCore.Sockets
                 await context.Response.WriteAsync("Cannot change transports mid-connection");
                 return false;
             }
+
+            // Setup the connection state from the http context
+            var format = (string)context.Request.Query[ConnectionMetadataNames.Format];
+            connection.User = context.User;
+            connection.Metadata[ConnectionMetadataNames.HttpContext] = context;
+            connection.Metadata[ConnectionMetadataNames.Format] = string.IsNullOrEmpty(format) ? "json" : format;
+
             return true;
         }
 
@@ -455,7 +444,7 @@ namespace Microsoft.AspNetCore.Sockets
             // There's no connection id so this is a brand new connection
             if (StringValues.IsNullOrEmpty(connectionId))
             {
-                connection = CreateConnection(context);
+                connection = _manager.CreateConnection();
             }
             else if (!_manager.TryGetConnection(connectionId, out connection))
             {

src/Microsoft.AspNetCore.Sockets.Http/SocketRouteBuilder.cs

@@ -1,4 +1,7 @@
-using System;
+// 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;
 using Microsoft.AspNetCore.Routing;
 
 namespace Microsoft.AspNetCore.Sockets

test/Microsoft.AspNetCore.Sockets.Tests/ConnectionManagerTests.cs

@@ -22,7 +22,7 @@ namespace Microsoft.AspNetCore.Sockets.Tests
             Assert.Null(connection.ApplicationTask);
             Assert.Null(connection.TransportTask);
             Assert.Null(connection.Cancellation);
-            Assert.Null(connection.RequestId);
+            Assert.NotEqual(default(DateTime), connection.LastSeenUtc);
             Assert.NotNull(connection.Transport);
         }
 

test/Microsoft.AspNetCore.Sockets.Tests/HttpConnectionDispatcherTests.cs

@@ -458,7 +458,7 @@ namespace Microsoft.AspNetCore.Sockets.Tests
             await task;
 
             Assert.Equal(DefaultConnectionContext.ConnectionStatus.Inactive, connection.Status);
-            Assert.Null(connection.RequestId);
+            Assert.Null(connection.GetHttpContext());
 
             Assert.Equal(StatusCodes.Status200OK, context.Response.StatusCode);
         }