Merge pull request #2186 from aspnet/release/2.1

Fix not setting HttpConnection.ConnectionId (#2154)

Author: JamesNK

src/Microsoft.AspNetCore.Http.Connections.Client/HttpConnection.cs

@@ -68,8 +68,19 @@ public override IDuplexPipe Transport
         /// <inheritdoc />
         public override IFeatureCollection Features { get; } = new FeatureCollection();
 
-        /// <inheritdoc />
-        public override string ConnectionId { get; set; }
+        /// <summary>
+        /// Gets or sets the connection ID.
+        /// </summary>
+        /// <remarks>
+        /// The connection ID is set when the <see cref="HttpConnection"/> is started and should not be set by user code.
+        /// If the connection was created with <see cref="HttpConnectionOptions.SkipNegotiation"/> set to <c>true</c>
+        /// then the connection ID will be <c>null</c>.
+        /// </remarks>
+        public override string ConnectionId
+        {
+            get => _connectionId;
+            set => throw new InvalidOperationException("The ConnectionId is set internally and should not be set by user code.");
+        }
 
         /// <inheritdoc />
         public override IDictionary<object, object> Items { get; set; } = new ConnectionItems();

test/Microsoft.AspNetCore.SignalR.Client.Tests/HttpConnectionTests.Negotiate.cs

@@ -82,6 +82,42 @@ await WithConnectionAsync(
                 Assert.Equal(expectedNegotiate, await negotiateUrlTcs.Task.OrTimeout());
             }
 
+            [Fact]
+            public async Task NegotiateReturnedConenctionIdIsSetOnConnection()
+            {
+                string connectionId = null;
+
+                var testHttpHandler = new TestHttpMessageHandler(autoNegotiate: false);
+                testHttpHandler.OnNegotiate((request, cancellationToken) => ResponseUtils.CreateResponse(HttpStatusCode.OK,
+                    JsonConvert.SerializeObject(new
+                    {
+                        connectionId = "0rge0d00-0040-0030-0r00-000q00r00e00",
+                        availableTransports = new object[]
+                        {
+                            new
+                            {
+                                transport = "LongPolling",
+                                transferFormats = new[] { "Text" }
+                            },
+                        }
+                    })));
+                testHttpHandler.OnLongPoll(cancellationToken => ResponseUtils.CreateResponse(HttpStatusCode.NoContent));
+                testHttpHandler.OnLongPollDelete((token) => ResponseUtils.CreateResponse(HttpStatusCode.Accepted));
+
+                using (var noErrorScope = new VerifyNoErrorsScope())
+                {
+                    await WithConnectionAsync(
+                        CreateConnection(testHttpHandler, loggerFactory: noErrorScope.LoggerFactory),
+                        async (connection) =>
+                        {
+                            await connection.StartAsync(TransferFormat.Text).OrTimeout();
+                            connectionId = connection.ConnectionId;
+                        });
+                }
+
+                Assert.Equal("0rge0d00-0040-0030-0r00-000q00r00e00", connectionId);
+            }
+
             [Fact]
             public async Task NegotiateThatReturnsUrlGetFollowed()
             {

test/Microsoft.AspNetCore.SignalR.Client.Tests/HttpConnectionTests.cs

@@ -40,6 +40,14 @@ public void CannotCreateConnectionWithNullUrlOnOptions()
             Assert.Equal("httpConnectionOptions", exception.ParamName);
         }
 
+        [Fact]
+        public void CannotSetConnectionId()
+        {
+            var connection = new HttpConnection(new Uri("http://fakeuri.org/"));
+            var exception = Assert.Throws<InvalidOperationException>(() => connection.ConnectionId = "custom conneciton ID");
+            Assert.Equal("The ConnectionId is set internally and should not be set by user code.", exception.Message);
+        }
+
         [Fact]
         public async Task HttpOptionsSetOntoHttpClientHandler()
         {