From 9a069548e4e593923b40e9ab6f34f6fe0d6a13dc Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Thu, 18 Apr 2024 13:35:14 +0200 Subject: [PATCH 01/39] Adding accept header within NegotiateAsync() and default headers in CreateHttpClient() --- .../src/HttpConnection.cs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs b/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs index 06153ad8be64..9b4d0d4c81bc 100644 --- a/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs +++ b/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs @@ -18,6 +18,7 @@ using Microsoft.AspNetCore.Shared; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; +using System.Net.Http.Headers; namespace Microsoft.AspNetCore.Http.Connections.Client; @@ -464,11 +465,15 @@ private async Task NegotiateAsync(Uri url, HttpClient httpC using (var request = new HttpRequestMessage(HttpMethod.Post, uri)) { -#if NET5_0_OR_GREATER + #if NET5_0_OR_GREATER request.Options.Set(new HttpRequestOptionsKey("IsNegotiate"), true); -#else + #else request.Properties.Add("IsNegotiate", true); -#endif + #endif + + // Set the Accept header to "*/*" + request.Headers.Accept.Clear(); + request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("*/*")); // ResponseHeadersRead instructs SendAsync to return once headers are read // rather than buffer the entire response. This gives a small perf boost. @@ -477,9 +482,9 @@ private async Task NegotiateAsync(Uri url, HttpClient httpC using (var response = await httpClient.SendAsync(request, HttpCompletionOption.ResponseHeadersRead, cancellationToken).ConfigureAwait(false)) { response.EnsureSuccessStatusCode(); -#pragma warning disable CA2016 // Forward the 'CancellationToken' parameter to methods + #pragma warning disable CA2016 // Forward the 'CancellationToken' parameter to methods var responseBuffer = await response.Content.ReadAsByteArrayAsync().ConfigureAwait(false); -#pragma warning restore CA2016 // Forward the 'CancellationToken' parameter to methods + #pragma warning restore CA2016 // Forward the 'CancellationToken' parameter to methods var negotiateResponse = NegotiateProtocol.ParseResponse(responseBuffer); if (!string.IsNullOrEmpty(negotiateResponse.Error)) { @@ -625,6 +630,8 @@ private HttpClient CreateHttpClient() var httpClient = new HttpClient(httpMessageHandler); httpClient.Timeout = HttpClientTimeout; + httpClient.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("*/*")); + var userSetUserAgent = false; // Apply any headers configured on the HttpConnectionOptions From 98f859fc2d55b2ce1fd590940209a9222e7c44d2 Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Fri, 19 Apr 2024 11:19:30 +0200 Subject: [PATCH 02/39] Removing defaultRequestHeader for accept and focusing on setting the value on relevant call sites --- .../csharp/Http.Connections.Client/src/HttpConnection.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs b/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs index 9b4d0d4c81bc..009e72d7346d 100644 --- a/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs +++ b/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs @@ -630,8 +630,6 @@ private HttpClient CreateHttpClient() var httpClient = new HttpClient(httpMessageHandler); httpClient.Timeout = HttpClientTimeout; - httpClient.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("*/*")); - var userSetUserAgent = false; // Apply any headers configured on the HttpConnectionOptions From b4bebb5bdfe8aaa55a8528b2c90d9879d3a9eaae Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Fri, 19 Apr 2024 11:25:17 +0200 Subject: [PATCH 03/39] Removing the logic to clear accept headers before allocating the accept header in NegotiateAsync() --- .../clients/csharp/Http.Connections.Client/src/HttpConnection.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs b/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs index 009e72d7346d..35a89bcc5af1 100644 --- a/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs +++ b/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs @@ -472,7 +472,6 @@ private async Task NegotiateAsync(Uri url, HttpClient httpC #endif // Set the Accept header to "*/*" - request.Headers.Accept.Clear(); request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("*/*")); // ResponseHeadersRead instructs SendAsync to return once headers are read From 018350b28d9426bd37b1f88d7598d055301f775e Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Sun, 21 Apr 2024 11:45:43 +0200 Subject: [PATCH 04/39] Adding accept header to LongPolling Get request within the Poll() method --- .../src/Internal/LongPollingTransport.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/LongPollingTransport.cs b/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/LongPollingTransport.cs index 69e120b03a20..e703b4f4cc47 100644 --- a/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/LongPollingTransport.cs +++ b/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/LongPollingTransport.cs @@ -11,6 +11,7 @@ using Microsoft.AspNetCore.Connections; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; +using System.Net.Http.Headers; namespace Microsoft.AspNetCore.Http.Connections.Client.Internal; @@ -149,7 +150,7 @@ private async Task Poll(Uri pollUrl, CancellationToken cancellationToken) while (!cancellationToken.IsCancellationRequested) { var request = new HttpRequestMessage(HttpMethod.Get, pollUrl); - + request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("*/*")); HttpResponseMessage response; try @@ -228,6 +229,7 @@ private async Task SendDeleteRequest(Uri url) { Log.SendingDeleteRequest(_logger, url); var request = new HttpRequestMessage(HttpMethod.Delete, url); + request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("*/*")); var response = await _httpClient.SendAsync(request).ConfigureAwait(false); if (response.StatusCode == HttpStatusCode.NotFound) From a6fc3e5cb46be80745d2ff356bbc5ed43924c852 Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Sun, 21 Apr 2024 20:31:43 +0200 Subject: [PATCH 05/39] Adding the accept header to SendUtils call site. --- .../csharp/Http.Connections.Client/src/Internal/SendUtils.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/SendUtils.cs b/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/SendUtils.cs index 2ad92f7d279f..ed0b963e654a 100644 --- a/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/SendUtils.cs +++ b/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/SendUtils.cs @@ -10,6 +10,7 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.Extensions.Logging; +using System.Net.Http.Headers; namespace Microsoft.AspNetCore.Http.Connections.Client.Internal; @@ -40,7 +41,7 @@ public static async Task SendMessages(Uri sendUrl, IDuplexPipe application, Http // Send them in a single post var request = new HttpRequestMessage(HttpMethod.Post, sendUrl); - + request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("*/*")); request.Content = new ReadOnlySequenceContent(buffer); // ResponseHeadersRead instructs SendAsync to return once headers are read From 6d793dfbd7b9eec278f5eb7286c07a582b34d113 Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Sun, 21 Apr 2024 20:32:32 +0200 Subject: [PATCH 06/39] Added the accept header to the LongPolling callsite for the inital polling request. --- .../Http.Connections.Client/src/Internal/LongPollingTransport.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/LongPollingTransport.cs b/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/LongPollingTransport.cs index e703b4f4cc47..adf3fe2774d3 100644 --- a/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/LongPollingTransport.cs +++ b/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/LongPollingTransport.cs @@ -52,6 +52,7 @@ public async Task StartAsync(Uri url, TransferFormat transferFormat, Cancellatio // Make initial long polling request // Server uses first long polling request to finish initializing connection and it returns without data var request = new HttpRequestMessage(HttpMethod.Get, url); + request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("*/*")); using (var response = await _httpClient.SendAsync(request, cancellationToken).ConfigureAwait(false)) { response.EnsureSuccessStatusCode(); From 7576ac9371b97ab11f8dec4e704078580c10eeed Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Mon, 22 Apr 2024 10:48:31 +0200 Subject: [PATCH 07/39] Removing whitespaces --- .../Http.Connections.Client/src/HttpConnection.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs b/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs index 35a89bcc5af1..b48578c70144 100644 --- a/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs +++ b/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs @@ -465,11 +465,11 @@ private async Task NegotiateAsync(Uri url, HttpClient httpC using (var request = new HttpRequestMessage(HttpMethod.Post, uri)) { - #if NET5_0_OR_GREATER +#if NET5_0_OR_GREATER request.Options.Set(new HttpRequestOptionsKey("IsNegotiate"), true); - #else +#else request.Properties.Add("IsNegotiate", true); - #endif +#endif // Set the Accept header to "*/*" request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("*/*")); @@ -481,9 +481,9 @@ private async Task NegotiateAsync(Uri url, HttpClient httpC using (var response = await httpClient.SendAsync(request, HttpCompletionOption.ResponseHeadersRead, cancellationToken).ConfigureAwait(false)) { response.EnsureSuccessStatusCode(); - #pragma warning disable CA2016 // Forward the 'CancellationToken' parameter to methods +#pragma warning disable CA2016 // Forward the 'CancellationToken' parameter to methods var responseBuffer = await response.Content.ReadAsByteArrayAsync().ConfigureAwait(false); - #pragma warning restore CA2016 // Forward the 'CancellationToken' parameter to methods +#pragma warning restore CA2016 // Forward the 'CancellationToken' parameter to methods var negotiateResponse = NegotiateProtocol.ParseResponse(responseBuffer); if (!string.IsNullOrEmpty(negotiateResponse.Error)) { From 168a1085f70b397f84b629edab000ef4f62bcd56 Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Mon, 22 Apr 2024 11:10:23 +0200 Subject: [PATCH 08/39] Removing comment and fixing whitespace --- .../clients/csharp/Http.Connections.Client/src/HttpConnection.cs | 1 - .../Http.Connections.Client/src/Internal/LongPollingTransport.cs | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs b/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs index b48578c70144..5d9d62321197 100644 --- a/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs +++ b/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs @@ -471,7 +471,6 @@ private async Task NegotiateAsync(Uri url, HttpClient httpC request.Properties.Add("IsNegotiate", true); #endif - // Set the Accept header to "*/*" request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("*/*")); // ResponseHeadersRead instructs SendAsync to return once headers are read diff --git a/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/LongPollingTransport.cs b/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/LongPollingTransport.cs index adf3fe2774d3..32d7f5b7bb6e 100644 --- a/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/LongPollingTransport.cs +++ b/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/LongPollingTransport.cs @@ -53,6 +53,7 @@ public async Task StartAsync(Uri url, TransferFormat transferFormat, Cancellatio // Server uses first long polling request to finish initializing connection and it returns without data var request = new HttpRequestMessage(HttpMethod.Get, url); request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("*/*")); + using (var response = await _httpClient.SendAsync(request, cancellationToken).ConfigureAwait(false)) { response.EnsureSuccessStatusCode(); From c3704effb90140bb656a1b8ac9c31adf9f62656b Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Mon, 22 Apr 2024 11:12:00 +0200 Subject: [PATCH 09/39] Correcting indentations --- .../Http.Connections.Client/src/Internal/LongPollingTransport.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/LongPollingTransport.cs b/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/LongPollingTransport.cs index 32d7f5b7bb6e..0d14c4863d22 100644 --- a/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/LongPollingTransport.cs +++ b/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/LongPollingTransport.cs @@ -232,6 +232,7 @@ private async Task SendDeleteRequest(Uri url) Log.SendingDeleteRequest(_logger, url); var request = new HttpRequestMessage(HttpMethod.Delete, url); request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("*/*")); + var response = await _httpClient.SendAsync(request).ConfigureAwait(false); if (response.StatusCode == HttpStatusCode.NotFound) From e993ecc694e017eff5d557cc8fc274e88f1fa6e6 Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Mon, 22 Apr 2024 13:32:46 +0200 Subject: [PATCH 10/39] retrigger checks From a00806aba3155d5e102c3d3473d80aa7a8c3ab4f Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Mon, 22 Apr 2024 16:39:27 +0200 Subject: [PATCH 11/39] retrigger checks From baea8dc9540ffe21049c28b40480c45ef26dfc91 Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Mon, 22 Apr 2024 18:52:19 +0200 Subject: [PATCH 12/39] retrigger checks From 335c09e17b87620e91aba8ce3e74ca03b724bee5 Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Mon, 22 Apr 2024 21:01:39 +0200 Subject: [PATCH 13/39] retrigger checks From a0467c285152d290534b7be9368388afb8916b6d Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Wed, 24 Apr 2024 08:48:43 +0200 Subject: [PATCH 14/39] Removing default headers --- .../csharp/Http.Connections.Client/src/HttpConnection.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs b/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs index 5d9d62321197..8b53674d7879 100644 --- a/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs +++ b/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs @@ -471,8 +471,6 @@ private async Task NegotiateAsync(Uri url, HttpClient httpC request.Properties.Add("IsNegotiate", true); #endif - request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("*/*")); - // ResponseHeadersRead instructs SendAsync to return once headers are read // rather than buffer the entire response. This gives a small perf boost. // Note that it is important to dispose of the response when doing this to From 9c214bbd900ff419314a38464b08eeedda7bf9eb Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Wed, 24 Apr 2024 08:49:33 +0200 Subject: [PATCH 15/39] adding tests for LongPollingTransport to check accept headers are set accoringly --- .../UnitTests/LongPollingTransportTests.cs | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/src/SignalR/clients/csharp/Client/test/UnitTests/LongPollingTransportTests.cs b/src/SignalR/clients/csharp/Client/test/UnitTests/LongPollingTransportTests.cs index 79872a236ace..1c0d11747aad 100644 --- a/src/SignalR/clients/csharp/Client/test/UnitTests/LongPollingTransportTests.cs +++ b/src/SignalR/clients/csharp/Client/test/UnitTests/LongPollingTransportTests.cs @@ -18,6 +18,7 @@ using Moq; using Moq.Protected; using Xunit; +using System.Net.Http.Headers; namespace Microsoft.AspNetCore.SignalR.Client.Tests; @@ -692,4 +693,64 @@ public async Task SendsDeleteRequestWhenTransportCompleted() Assert.Equal(TestUri, deleteRequest.RequestUri); } } + + [Fact] + public async Task StartAsyncSetsAcceptHeaderCorrectly() + { + var mockHttpHandler = new Mock(); + mockHttpHandler.Protected() + .Setup>( + "SendAsync", + ItExpr.Is(req => req.Headers.Accept.Contains(new MediaTypeWithQualityHeaderValue("*/*"))), + ItExpr.IsAny()) + .ReturnsAsync(new HttpResponseMessage(HttpStatusCode.OK)) + .Verifiable("StartAsync did not set the Accept header correctly."); + + using (var httpClient = new HttpClient(mockHttpHandler.Object)) + { + var transport = new LongPollingTransport(httpClient, loggerFactory: LoggerFactory); + + await transport.StartAsync(TestUri, TransferFormat.Text); + await transport.StopAsync(); + } + + mockHttpHandler.Protected().Verify( + "SendAsync", + Times.Once(), // Ensure the method was called exactly once + ItExpr.Is(req => req.Headers.Accept.Contains(new MediaTypeWithQualityHeaderValue("*/*"))), + ItExpr.IsAny()); + } + + [Fact] + public async Task PollSetsAcceptHeaderCorrectly() + { + var mockHttpHandler = new Mock(); + mockHttpHandler.Protected() + .Setup>( + "SendAsync", + ItExpr.Is(req => req.Method == HttpMethod.Get && + req.Headers.Accept.Contains(new MediaTypeWithQualityHeaderValue("*/*"))), + ItExpr.IsAny()) + .ReturnsAsync(new HttpResponseMessage(HttpStatusCode.OK)) + .Verifiable("Poll did not set the Accept header correctly."); + + using (var httpClient = new HttpClient(mockHttpHandler.Object)) + { + var transport = new LongPollingTransport(httpClient, loggerFactory: LoggerFactory); + + // Start the transport to initialize and begin polling + await transport.StartAsync(TestUri, TransferFormat.Text); + // Wait a moment to allow the poll method to invoke + await Task.Delay(100); + await transport.StopAsync(); + } + + // Verify that the HTTP request sent by the Poll method had the correct Accept header + mockHttpHandler.Protected().Verify( + "SendAsync", + Times.AtLeastOnce(), // Ensure the Poll was called at least once + ItExpr.Is(req => req.Headers.Accept.Contains(new MediaTypeWithQualityHeaderValue("*/*"))), + ItExpr.IsAny()); + } + } From feb1330c56e031a2ff602d02adc042468a9d8d32 Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Wed, 24 Apr 2024 08:50:14 +0200 Subject: [PATCH 16/39] Adding tests for ServerSentEventsTransport to check accept headers are set accoringly --- .../ServerSentEventsTransportTests.cs | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/src/SignalR/clients/csharp/Client/test/UnitTests/ServerSentEventsTransportTests.cs b/src/SignalR/clients/csharp/Client/test/UnitTests/ServerSentEventsTransportTests.cs index ed40cf9055a3..b086d269ec8b 100644 --- a/src/SignalR/clients/csharp/Client/test/UnitTests/ServerSentEventsTransportTests.cs +++ b/src/SignalR/clients/csharp/Client/test/UnitTests/ServerSentEventsTransportTests.cs @@ -18,6 +18,7 @@ using Moq; using Moq.Protected; using Xunit; +using System.Net; namespace Microsoft.AspNetCore.SignalR.Client.Tests; @@ -409,4 +410,42 @@ public async Task SSETransportThrowsForInvalidTransferFormat(TransferFormat tran Assert.Equal("transferFormat", exception.ParamName); } } + + [Fact] + public async Task StartAsyncSetsCorrectAcceptHeaderForSSE() + { + var mockHttpHandler = new Mock(); + var startAsyncCalled = new TaskCompletionSource(); + + // Setup the mocked HttpMessageHandler + mockHttpHandler.Protected() + .Setup>( + "SendAsync", + ItExpr.Is(req => req.Headers.Accept.Contains(new MediaTypeWithQualityHeaderValue("text/event-stream"))), + ItExpr.IsAny()) + .ReturnsAsync(new HttpResponseMessage(HttpStatusCode.OK)) + .Callback(() => startAsyncCalled.SetResult(true)) + .Verifiable("Accept header for 'text/event-stream' was not set correctly."); + + using (var httpClient = new HttpClient(mockHttpHandler.Object)) + using (StartVerifiableLog()) + { + var sseTransport = new ServerSentEventsTransport(httpClient, loggerFactory: LoggerFactory); + + await sseTransport.StartAsync(new Uri("http://fakeuri.org"), TransferFormat.Text); + + // Ensure that StartAsync was called and completed before stopping the transport + await startAsyncCalled.Task; + + await sseTransport.StopAsync(); + } + + // Verify that the SendAsync method was called on the mock HttpMessageHandler + mockHttpHandler.Protected().Verify( + "SendAsync", + Times.Once(), + ItExpr.Is(req => req.Headers.Accept.Contains(new MediaTypeWithQualityHeaderValue("text/event-stream"))), + ItExpr.IsAny()); + } + } From 35614b78d41707e8be64490fcf3354803ad4067b Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Wed, 24 Apr 2024 12:44:16 +0200 Subject: [PATCH 17/39] Removing the Accept header addition for the SendDeleteRequestMethod() --- .../Http.Connections.Client/src/Internal/LongPollingTransport.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/LongPollingTransport.cs b/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/LongPollingTransport.cs index 0d14c4863d22..0f45bcfd9ae4 100644 --- a/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/LongPollingTransport.cs +++ b/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/LongPollingTransport.cs @@ -231,7 +231,6 @@ private async Task SendDeleteRequest(Uri url) { Log.SendingDeleteRequest(_logger, url); var request = new HttpRequestMessage(HttpMethod.Delete, url); - request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("*/*")); var response = await _httpClient.SendAsync(request).ConfigureAwait(false); From 39ddeb308ae0455a51119dfbde3536adaa590aae Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Mon, 29 Apr 2024 14:34:59 +0200 Subject: [PATCH 18/39] HttpConnection tests for NegotiateAsync --- .../test/UnitTests/HttpConnectionTests.cs | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.cs b/src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.cs index c7eb39857de6..ba3e0e34b030 100644 --- a/src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.cs +++ b/src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.cs @@ -17,6 +17,11 @@ using Microsoft.Extensions.Logging.Testing; using Moq; using Xunit; +using Moq.Protected; +using System.IO.Pipelines; +using System.Text; +using System.Net.Http.Headers; +using Microsoft.AspNetCore.Http.Connections.Client.Internal; namespace Microsoft.AspNetCore.SignalR.Client.Tests; @@ -157,4 +162,47 @@ await WithConnectionAsync( Assert.Equal("SendingHttpRequest", writeList[0].EventId.Name); Assert.Equal("UnsuccessfulHttpResponse", writeList[1].EventId.Name); } + + [Fact] + public async Task Negotiation_SendsCorrectHeaders() + { + try + { + var mockHandler = new Mock(); + var httpClient = new HttpClient(mockHandler.Object); + var options = new HttpConnectionOptions + { + Url = new Uri("http://fakeuri.org/"), + Transports = HttpTransportType.WebSockets, + SkipNegotiation = false + }; + + var loggerFactory = NullLoggerFactory.Instance; + var mockTransportFactory = new Mock(); + var mockTransport = new Mock(); + mockTransportFactory.Setup(x => x.CreateTransport(It.IsAny(), It.IsAny())) + .Returns(mockTransport.Object); + + var connection = new HttpConnection(options, loggerFactory, mockTransportFactory.Object, httpClient); + + mockHandler.Protected().Setup>( + "SendAsync", + ItExpr.IsAny(), + ItExpr.IsAny()) + .Callback((request, cancellationToken) => + { + Assert.Contains(new MediaTypeWithQualityHeaderValue("*/*"), request.Headers.Accept); + }) + .ReturnsAsync(new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent("{\"connectionId\":\"12345\",\"availableTransports\":[]}", Encoding.UTF8, "application/json") + }); + + await connection.StartAsync(); + } + catch (Exception ex) + { + Assert.False(true, $"Unexpected exception: {ex.Message}"); + } + } } From 6c5dd09274254384cf5c008e5f188fc3d972428e Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Mon, 29 Apr 2024 14:35:56 +0200 Subject: [PATCH 19/39] Better method naming --- .../clients/csharp/Client/test/UnitTests/HttpConnectionTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.cs b/src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.cs index ba3e0e34b030..542c2308dc7d 100644 --- a/src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.cs +++ b/src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.cs @@ -164,7 +164,7 @@ await WithConnectionAsync( } [Fact] - public async Task Negotiation_SendsCorrectHeaders() + public async Task NegotiationSendsCorrectAcceptHeader() { try { From 0204a272db0a7e766dc96519e4d2d837048cd93e Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Mon, 29 Apr 2024 14:36:17 +0200 Subject: [PATCH 20/39] Improving LongPollingTransportTests --- .../UnitTests/LongPollingTransportTests.cs | 61 +++++++------------ 1 file changed, 21 insertions(+), 40 deletions(-) diff --git a/src/SignalR/clients/csharp/Client/test/UnitTests/LongPollingTransportTests.cs b/src/SignalR/clients/csharp/Client/test/UnitTests/LongPollingTransportTests.cs index 1c0d11747aad..b4c32f310124 100644 --- a/src/SignalR/clients/csharp/Client/test/UnitTests/LongPollingTransportTests.cs +++ b/src/SignalR/clients/csharp/Client/test/UnitTests/LongPollingTransportTests.cs @@ -698,59 +698,40 @@ public async Task SendsDeleteRequestWhenTransportCompleted() public async Task StartAsyncSetsAcceptHeaderCorrectly() { var mockHttpHandler = new Mock(); + var responseTaskCompletionSource = new TaskCompletionSource(); + mockHttpHandler.Protected() - .Setup>( - "SendAsync", - ItExpr.Is(req => req.Headers.Accept.Contains(new MediaTypeWithQualityHeaderValue("*/*"))), - ItExpr.IsAny()) - .ReturnsAsync(new HttpResponseMessage(HttpStatusCode.OK)) - .Verifiable("StartAsync did not set the Accept header correctly."); + .Setup>("SendAsync", ItExpr.IsAny(), ItExpr.IsAny()) + .Returns((HttpRequestMessage request, CancellationToken cancellationToken) => + { + if (request.Headers.Accept?.Contains(new MediaTypeWithQualityHeaderValue("*/*")) == true) + { + responseTaskCompletionSource.SetResult(ResponseUtils.CreateResponse(HttpStatusCode.OK)); + } + else + { + responseTaskCompletionSource.SetResult(ResponseUtils.CreateResponse(HttpStatusCode.BadRequest)); + } + return responseTaskCompletionSource.Task; + }); using (var httpClient = new HttpClient(mockHttpHandler.Object)) { var transport = new LongPollingTransport(httpClient, loggerFactory: LoggerFactory); - await transport.StartAsync(TestUri, TransferFormat.Text); - await transport.StopAsync(); - } - - mockHttpHandler.Protected().Verify( - "SendAsync", - Times.Once(), // Ensure the method was called exactly once - ItExpr.Is(req => req.Headers.Accept.Contains(new MediaTypeWithQualityHeaderValue("*/*"))), - ItExpr.IsAny()); - } + await transport.StartAsync(TestUri, TransferFormat.Text).DefaultTimeout(); - [Fact] - public async Task PollSetsAcceptHeaderCorrectly() - { - var mockHttpHandler = new Mock(); - mockHttpHandler.Protected() - .Setup>( - "SendAsync", - ItExpr.Is(req => req.Method == HttpMethod.Get && - req.Headers.Accept.Contains(new MediaTypeWithQualityHeaderValue("*/*"))), - ItExpr.IsAny()) - .ReturnsAsync(new HttpResponseMessage(HttpStatusCode.OK)) - .Verifiable("Poll did not set the Accept header correctly."); + await transport.StopAsync(); - using (var httpClient = new HttpClient(mockHttpHandler.Object)) - { - var transport = new LongPollingTransport(httpClient, loggerFactory: LoggerFactory); + var response = await responseTaskCompletionSource.Task; + Assert.Equal(HttpStatusCode.OK, response.StatusCode); - // Start the transport to initialize and begin polling - await transport.StartAsync(TestUri, TransferFormat.Text); - // Wait a moment to allow the poll method to invoke - await Task.Delay(100); - await transport.StopAsync(); } - // Verify that the HTTP request sent by the Poll method had the correct Accept header mockHttpHandler.Protected().Verify( "SendAsync", - Times.AtLeastOnce(), // Ensure the Poll was called at least once - ItExpr.Is(req => req.Headers.Accept.Contains(new MediaTypeWithQualityHeaderValue("*/*"))), + Times.AtLeast(2), //ensure the mockHttpHandler was used twice in StartAsync() and Poll() + ItExpr.IsAny(), ItExpr.IsAny()); } - } From 0b828cafa667d1bd80d6a02514751a40047caa63 Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Mon, 29 Apr 2024 14:36:33 +0200 Subject: [PATCH 21/39] Creating accept header tests for the SendUtils --- .../Client/test/UnitTests/SendUtilsTests.cs | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 src/SignalR/clients/csharp/Client/test/UnitTests/SendUtilsTests.cs diff --git a/src/SignalR/clients/csharp/Client/test/UnitTests/SendUtilsTests.cs b/src/SignalR/clients/csharp/Client/test/UnitTests/SendUtilsTests.cs new file mode 100644 index 000000000000..8a4a764a4ae7 --- /dev/null +++ b/src/SignalR/clients/csharp/Client/test/UnitTests/SendUtilsTests.cs @@ -0,0 +1,56 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.IO.Pipelines; +using System.Net; +using System.Net.Http; +using System.Text; +using Microsoft.AspNetCore.Http.Connections.Client.Internal; +using Microsoft.AspNetCore.SignalR.Tests; +using Moq; +using Moq.Protected; +using System.Net.Http.Headers; +using Microsoft.AspNetCore.InternalTesting; + +namespace Microsoft.AspNetCore.SignalR.Client.Tests; +public partial class SendUtilsTests : VerifiableLoggedTest +{ + [Fact] + public async Task SendMessagesSetsCorrectAcceptHeader() + { + var mockHttpHandler = new Mock(); + var responseTaskCompletionSource = new TaskCompletionSource(); + + mockHttpHandler.Protected() + .Setup>("SendAsync", ItExpr.IsAny(), ItExpr.IsAny()) + .Returns((HttpRequestMessage request, CancellationToken cancellationToken) => + { + if (request.Headers.Accept?.Contains(new MediaTypeWithQualityHeaderValue("*/*")) == true) + { + responseTaskCompletionSource.SetResult(ResponseUtils.CreateResponse(HttpStatusCode.OK)); + } + else + { + responseTaskCompletionSource.SetResult(ResponseUtils.CreateResponse(HttpStatusCode.BadRequest)); + } + return responseTaskCompletionSource.Task; + }); + + using (var httpClient = new HttpClient(mockHttpHandler.Object)) + using (StartVerifiableLog()) + { + var pipe = new Pipe(); + var application = new DuplexPipe(pipe.Reader, pipe.Writer); + + // Simulate writing data to send + await application.Output.WriteAsync(Encoding.UTF8.GetBytes("Hello World")); + + application.Output.Complete(); + + await SendUtils.SendMessages(new Uri("http://fakeuri.org"), application, httpClient, logger: Logger).DefaultTimeout(); + + var response = await responseTaskCompletionSource.Task; + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + } + } +} From 69dae2a1da6a520897a0dc721780c24648e8851c Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Mon, 29 Apr 2024 14:36:55 +0200 Subject: [PATCH 22/39] Improving ServerSentEventTests for accept header --- .../ServerSentEventsTransportTests.cs | 39 +++++++++---------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/src/SignalR/clients/csharp/Client/test/UnitTests/ServerSentEventsTransportTests.cs b/src/SignalR/clients/csharp/Client/test/UnitTests/ServerSentEventsTransportTests.cs index b086d269ec8b..aee9591a9dce 100644 --- a/src/SignalR/clients/csharp/Client/test/UnitTests/ServerSentEventsTransportTests.cs +++ b/src/SignalR/clients/csharp/Client/test/UnitTests/ServerSentEventsTransportTests.cs @@ -415,37 +415,34 @@ public async Task SSETransportThrowsForInvalidTransferFormat(TransferFormat tran public async Task StartAsyncSetsCorrectAcceptHeaderForSSE() { var mockHttpHandler = new Mock(); - var startAsyncCalled = new TaskCompletionSource(); + var responseTaskCompletionSource = new TaskCompletionSource(); - // Setup the mocked HttpMessageHandler mockHttpHandler.Protected() - .Setup>( - "SendAsync", - ItExpr.Is(req => req.Headers.Accept.Contains(new MediaTypeWithQualityHeaderValue("text/event-stream"))), - ItExpr.IsAny()) - .ReturnsAsync(new HttpResponseMessage(HttpStatusCode.OK)) - .Callback(() => startAsyncCalled.SetResult(true)) - .Verifiable("Accept header for 'text/event-stream' was not set correctly."); + .Setup>("SendAsync", ItExpr.IsAny(), ItExpr.IsAny()) + .Returns((HttpRequestMessage request, CancellationToken cancellationToken) => + { + if (request.Headers.Accept?.Contains(new MediaTypeWithQualityHeaderValue("text/event-stream")) == true) + { + responseTaskCompletionSource.SetResult(new HttpResponseMessage(HttpStatusCode.OK)); + } + else + { + responseTaskCompletionSource.SetResult(new HttpResponseMessage(HttpStatusCode.NoContent)); + } + return responseTaskCompletionSource.Task; + }); using (var httpClient = new HttpClient(mockHttpHandler.Object)) using (StartVerifiableLog()) { var sseTransport = new ServerSentEventsTransport(httpClient, loggerFactory: LoggerFactory); - await sseTransport.StartAsync(new Uri("http://fakeuri.org"), TransferFormat.Text); - - // Ensure that StartAsync was called and completed before stopping the transport - await startAsyncCalled.Task; + await sseTransport.StartAsync(new Uri("http://fakeuri.org"), TransferFormat.Text).DefaultTimeout(); await sseTransport.StopAsync(); - } - // Verify that the SendAsync method was called on the mock HttpMessageHandler - mockHttpHandler.Protected().Verify( - "SendAsync", - Times.Once(), - ItExpr.Is(req => req.Headers.Accept.Contains(new MediaTypeWithQualityHeaderValue("text/event-stream"))), - ItExpr.IsAny()); + var response = await responseTaskCompletionSource.Task; + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + } } - } From bd2609f2fa7072bea39b0879f07111bdd25533d8 Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Thu, 2 May 2024 10:57:00 +0200 Subject: [PATCH 23/39] Adding an option parameter to HttpConnections contructor used for unit testing. Adding an optional HttpMessageHandler parameter for CreateHttpClient. --- .../csharp/Http.Connections.Client/src/HttpConnection.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs b/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs index 8b53674d7879..0e8410e47907 100644 --- a/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs +++ b/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs @@ -165,10 +165,11 @@ public HttpConnection(HttpConnectionOptions httpConnectionOptions, ILoggerFactor } // Used by unit tests - internal HttpConnection(HttpConnectionOptions httpConnectionOptions, ILoggerFactory loggerFactory, ITransportFactory transportFactory) + internal HttpConnection(HttpConnectionOptions httpConnectionOptions, ILoggerFactory loggerFactory, ITransportFactory transportFactory, HttpMessageHandler? messageHandler = null) : this(httpConnectionOptions, loggerFactory) { _transportFactory = transportFactory; + _httpClient = CreateHttpClient(messageHandler); } /// @@ -543,9 +544,9 @@ private async Task StartTransport(Uri connectUrl, HttpTransportType transportTyp Log.TransportStarted(_logger, transportType); } - private HttpClient CreateHttpClient() + private HttpClient CreateHttpClient(HttpMessageHandler? messageHandler = null) { - var httpClientHandler = new HttpClientHandler(); + var httpClientHandler = messageHandler as HttpClientHandler ?? new HttpClientHandler(); HttpMessageHandler httpMessageHandler = httpClientHandler; var isBrowser = OperatingSystem.IsBrowser(); From d570d0867ac6887634d081daaf051ceee795b55a Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Thu, 2 May 2024 10:58:00 +0200 Subject: [PATCH 24/39] Addition of Test method to ensure NegotiateAsync() appends the correct header to the Http Request --- .../test/UnitTests/HttpConnectionTests.cs | 62 +++++++++---------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.cs b/src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.cs index 542c2308dc7d..961e020db7b1 100644 --- a/src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.cs +++ b/src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.cs @@ -164,45 +164,45 @@ await WithConnectionAsync( } [Fact] - public async Task NegotiationSendsCorrectAcceptHeader() + public async Task NegotiateAsyncAppendsCorrectAcceptHeader() { - try - { - var mockHandler = new Mock(); - var httpClient = new HttpClient(mockHandler.Object); - var options = new HttpConnectionOptions - { - Url = new Uri("http://fakeuri.org/"), - Transports = HttpTransportType.WebSockets, - SkipNegotiation = false - }; + var handlerMock = new Mock(); + ITransport transport = null; + ITransportFactory transportFactory = null; - var loggerFactory = NullLoggerFactory.Instance; - var mockTransportFactory = new Mock(); - var mockTransport = new Mock(); - mockTransportFactory.Setup(x => x.CreateTransport(It.IsAny(), It.IsAny())) - .Returns(mockTransport.Object); + transportFactory = new TestTransportFactory(transport); - var connection = new HttpConnection(options, loggerFactory, mockTransportFactory.Object, httpClient); + var response = new HttpResponseMessage + { + StatusCode = HttpStatusCode.OK, + Content = new StringContent("{\"connectionId\":\"12345\",\"availableTransports\":[]}") + }; - mockHandler.Protected().Setup>( + handlerMock + .Protected() + .Setup>( "SendAsync", ItExpr.IsAny(), - ItExpr.IsAny()) - .Callback((request, cancellationToken) => + ItExpr.IsAny() + ) + .Callback((request, token) => { - Assert.Contains(new MediaTypeWithQualityHeaderValue("*/*"), request.Headers.Accept); + Assert.Equal("application/json", request.Headers.Accept.ToString()); }) - .ReturnsAsync(new HttpResponseMessage(HttpStatusCode.OK) - { - Content = new StringContent("{\"connectionId\":\"12345\",\"availableTransports\":[]}", Encoding.UTF8, "application/json") - }); + .ReturnsAsync(response) + .Verifiable(); - await connection.StartAsync(); - } - catch (Exception ex) - { - Assert.False(true, $"Unexpected exception: {ex.Message}"); - } + var url = new Uri("http://fakeurl/"); + var options = new HttpConnectionOptions { Url = url, Transports = HttpTransportType.WebSockets }; + var connection = new HttpConnection(options, NullLoggerFactory.Instance, transportFactory, handlerMock.Object); + + await connection.StartAsync().DefaultTimeout(); + + handlerMock.Protected().Verify( + "SendAsync", + Times.Once(), + ItExpr.IsAny(), + ItExpr.IsAny() + ); } } From 6f84149150b24f9a1a2f886691106aba8b18d04b Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Fri, 3 May 2024 10:31:54 +0200 Subject: [PATCH 25/39] Removing changes related to injecting httpMessageHandler --- .../csharp/Http.Connections.Client/src/HttpConnection.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs b/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs index 0e8410e47907..8b53674d7879 100644 --- a/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs +++ b/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs @@ -165,11 +165,10 @@ public HttpConnection(HttpConnectionOptions httpConnectionOptions, ILoggerFactor } // Used by unit tests - internal HttpConnection(HttpConnectionOptions httpConnectionOptions, ILoggerFactory loggerFactory, ITransportFactory transportFactory, HttpMessageHandler? messageHandler = null) + internal HttpConnection(HttpConnectionOptions httpConnectionOptions, ILoggerFactory loggerFactory, ITransportFactory transportFactory) : this(httpConnectionOptions, loggerFactory) { _transportFactory = transportFactory; - _httpClient = CreateHttpClient(messageHandler); } /// @@ -544,9 +543,9 @@ private async Task StartTransport(Uri connectUrl, HttpTransportType transportTyp Log.TransportStarted(_logger, transportType); } - private HttpClient CreateHttpClient(HttpMessageHandler? messageHandler = null) + private HttpClient CreateHttpClient() { - var httpClientHandler = messageHandler as HttpClientHandler ?? new HttpClientHandler(); + var httpClientHandler = new HttpClientHandler(); HttpMessageHandler httpMessageHandler = httpClientHandler; var isBrowser = OperatingSystem.IsBrowser(); From fe3a78a98f3711984e92846d3386e5e457fddfb0 Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Fri, 3 May 2024 11:46:55 +0200 Subject: [PATCH 26/39] Adjusting test to make use of httpmessageHandlerFactory and better practices --- .../test/UnitTests/HttpConnectionTests.cs | 69 ++++++++++--------- 1 file changed, 35 insertions(+), 34 deletions(-) diff --git a/src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.cs b/src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.cs index 961e020db7b1..4080e5fdc4a7 100644 --- a/src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.cs +++ b/src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.cs @@ -166,43 +166,44 @@ await WithConnectionAsync( [Fact] public async Task NegotiateAsyncAppendsCorrectAcceptHeader() { - var handlerMock = new Mock(); - ITransport transport = null; - ITransportFactory transportFactory = null; + var testHttpHandler = TestHttpMessageHandler.CreateDefault(); + + testHttpHandler.OnNegotiate((request, cancellationToken) => + { + Assert.Equal("application/json", request.Headers.Accept.ToString()); + return ResponseUtils.CreateResponse(HttpStatusCode.OK, "{}"); + }); + + var httpOptions = new HttpConnectionOptions(); + httpOptions.Url = new Uri("http://fakeurl.org/"); + httpOptions.HttpMessageHandlerFactory = inner => testHttpHandler; + httpOptions.SkipNegotiation = false; + httpOptions.Transports = HttpTransportType.WebSockets; - transportFactory = new TestTransportFactory(transport); + const string loggerName = "Microsoft.AspNetCore.Http.Connections.Client.Internal.LoggingHttpMessageHandler"; + var testSink = new TestSink(); + var logger = new TestLogger(loggerName, testSink, true); + + var mockLoggerFactory = new Mock(); + mockLoggerFactory + .Setup(m => m.CreateLogger(It.IsAny())) + .Returns((string categoryName) => (categoryName == loggerName) ? (ILogger)logger : NullLogger.Instance); - var response = new HttpResponseMessage + try { - StatusCode = HttpStatusCode.OK, - Content = new StringContent("{\"connectionId\":\"12345\",\"availableTransports\":[]}") - }; + await WithConnectionAsync( + CreateConnection(httpOptions, loggerFactory: mockLoggerFactory.Object), + async (connection) => + { + await connection.StartAsync().DefaultTimeout(); + }); + } + catch + { + // ignore connection error + } - handlerMock - .Protected() - .Setup>( - "SendAsync", - ItExpr.IsAny(), - ItExpr.IsAny() - ) - .Callback((request, token) => - { - Assert.Equal("application/json", request.Headers.Accept.ToString()); - }) - .ReturnsAsync(response) - .Verifiable(); - - var url = new Uri("http://fakeurl/"); - var options = new HttpConnectionOptions { Url = url, Transports = HttpTransportType.WebSockets }; - var connection = new HttpConnection(options, NullLoggerFactory.Instance, transportFactory, handlerMock.Object); - - await connection.StartAsync().DefaultTimeout(); - - handlerMock.Protected().Verify( - "SendAsync", - Times.Once(), - ItExpr.IsAny(), - ItExpr.IsAny() - ); + // Verify the Accept header was checked + Assert.Contains(testSink.Writes, w => w.EventId.Name == "SendingHttpRequest" && w.Message.Contains("application/json")); } } From 50afbcba173801e97e055c2ebdd9119b5f0001c0 Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Mon, 6 May 2024 15:27:04 +0200 Subject: [PATCH 27/39] Updating implementation for NegotiateAsyncAppendsCorrectAcceptHeader --- .../test/UnitTests/HttpConnectionTests.cs | 36 +++++++++---------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.cs b/src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.cs index 4080e5fdc4a7..730c01d818f0 100644 --- a/src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.cs +++ b/src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.cs @@ -167,32 +167,27 @@ await WithConnectionAsync( public async Task NegotiateAsyncAppendsCorrectAcceptHeader() { var testHttpHandler = TestHttpMessageHandler.CreateDefault(); + var negotiateUrlTcs = new TaskCompletionSource(); testHttpHandler.OnNegotiate((request, cancellationToken) => { - Assert.Equal("application/json", request.Headers.Accept.ToString()); - return ResponseUtils.CreateResponse(HttpStatusCode.OK, "{}"); + var headerFound = request.Headers.Accept?.Contains(new MediaTypeWithQualityHeaderValue("*/*")) == true; + negotiateUrlTcs.SetResult(headerFound); + return ResponseUtils.CreateResponse(HttpStatusCode.OK, ResponseUtils.CreateNegotiationContent()); }); - var httpOptions = new HttpConnectionOptions(); - httpOptions.Url = new Uri("http://fakeurl.org/"); - httpOptions.HttpMessageHandlerFactory = inner => testHttpHandler; - httpOptions.SkipNegotiation = false; - httpOptions.Transports = HttpTransportType.WebSockets; - - const string loggerName = "Microsoft.AspNetCore.Http.Connections.Client.Internal.LoggingHttpMessageHandler"; - var testSink = new TestSink(); - var logger = new TestLogger(loggerName, testSink, true); - - var mockLoggerFactory = new Mock(); - mockLoggerFactory - .Setup(m => m.CreateLogger(It.IsAny())) - .Returns((string categoryName) => (categoryName == loggerName) ? (ILogger)logger : NullLogger.Instance); + var httpOptions = new HttpConnectionOptions + { + Url = new Uri("http://fakeurl.org/"), + SkipNegotiation = false, + Transports = HttpTransportType.WebSockets, + HttpMessageHandlerFactory = inner => testHttpHandler + }; try { await WithConnectionAsync( - CreateConnection(httpOptions, loggerFactory: mockLoggerFactory.Object), + CreateConnection(httpOptions), async (connection) => { await connection.StartAsync().DefaultTimeout(); @@ -200,10 +195,11 @@ await WithConnectionAsync( } catch { - // ignore connection error + } - // Verify the Accept header was checked - Assert.Contains(testSink.Writes, w => w.EventId.Name == "SendingHttpRequest" && w.Message.Contains("application/json")); + Assert.True(negotiateUrlTcs.Task.IsCompleted); + var headerWasFound = await negotiateUrlTcs.Task; + Assert.True(headerWasFound); } } From 0aaf06f3b635b3e1c09ba84e9a6ad5bd148310e5 Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Mon, 6 May 2024 15:27:38 +0200 Subject: [PATCH 28/39] Uncommenting the addition of the accept header in NegotiateAsync() --- .../clients/csharp/Http.Connections.Client/src/HttpConnection.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs b/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs index 8b53674d7879..e193d346c037 100644 --- a/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs +++ b/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs @@ -470,6 +470,7 @@ private async Task NegotiateAsync(Uri url, HttpClient httpC #else request.Properties.Add("IsNegotiate", true); #endif + request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("*/*")); // ResponseHeadersRead instructs SendAsync to return once headers are read // rather than buffer the entire response. This gives a small perf boost. From 14387c72b2b8e1d7daec7f885af22eed53d642d9 Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Tue, 7 May 2024 09:14:33 +0200 Subject: [PATCH 29/39] Final implementation for checking the correct header is appended to NegotiateAsync --- .../csharp/Client/test/UnitTests/HttpConnectionTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.cs b/src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.cs index 730c01d818f0..8ec03522b821 100644 --- a/src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.cs +++ b/src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.cs @@ -166,7 +166,7 @@ await WithConnectionAsync( [Fact] public async Task NegotiateAsyncAppendsCorrectAcceptHeader() { - var testHttpHandler = TestHttpMessageHandler.CreateDefault(); + var testHttpHandler = new TestHttpMessageHandler(false); var negotiateUrlTcs = new TaskCompletionSource(); testHttpHandler.OnNegotiate((request, cancellationToken) => @@ -195,7 +195,7 @@ await WithConnectionAsync( } catch { - + // ignore connection error } Assert.True(negotiateUrlTcs.Task.IsCompleted); From acbea2667ae3300c6fc1e9ecd9413bd2a5e9af4a Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Tue, 7 May 2024 09:15:14 +0200 Subject: [PATCH 30/39] Ensuring the TaskCompletionSource is allocated to --- .../Client/test/UnitTests/ServerSentEventsTransportTests.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/SignalR/clients/csharp/Client/test/UnitTests/ServerSentEventsTransportTests.cs b/src/SignalR/clients/csharp/Client/test/UnitTests/ServerSentEventsTransportTests.cs index aee9591a9dce..f7c5a0049908 100644 --- a/src/SignalR/clients/csharp/Client/test/UnitTests/ServerSentEventsTransportTests.cs +++ b/src/SignalR/clients/csharp/Client/test/UnitTests/ServerSentEventsTransportTests.cs @@ -439,8 +439,9 @@ public async Task StartAsyncSetsCorrectAcceptHeaderForSSE() await sseTransport.StartAsync(new Uri("http://fakeuri.org"), TransferFormat.Text).DefaultTimeout(); - await sseTransport.StopAsync(); + await sseTransport.StopAsync().DefaultTimeout(); + Assert.True(responseTaskCompletionSource.Task.IsCompleted); var response = await responseTaskCompletionSource.Task; Assert.Equal(HttpStatusCode.OK, response.StatusCode); } From 4c8c275137401703a33c05e315df00097caadec0 Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Fri, 10 May 2024 10:19:40 +0200 Subject: [PATCH 31/39] Updating all TaskCompletionSources to run continuations asynchronously --- .../csharp/Client/test/UnitTests/HttpConnectionTests.cs | 4 ++-- .../csharp/Client/test/UnitTests/LongPollingTransportTests.cs | 2 +- .../clients/csharp/Client/test/UnitTests/SendUtilsTests.cs | 2 +- .../Client/test/UnitTests/ServerSentEventsTransportTests.cs | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.cs b/src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.cs index 8ec03522b821..4a99acd385e4 100644 --- a/src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.cs +++ b/src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.cs @@ -167,8 +167,8 @@ await WithConnectionAsync( public async Task NegotiateAsyncAppendsCorrectAcceptHeader() { var testHttpHandler = new TestHttpMessageHandler(false); - var negotiateUrlTcs = new TaskCompletionSource(); - + var negotiateUrlTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + testHttpHandler.OnNegotiate((request, cancellationToken) => { var headerFound = request.Headers.Accept?.Contains(new MediaTypeWithQualityHeaderValue("*/*")) == true; diff --git a/src/SignalR/clients/csharp/Client/test/UnitTests/LongPollingTransportTests.cs b/src/SignalR/clients/csharp/Client/test/UnitTests/LongPollingTransportTests.cs index b4c32f310124..d1420cf33fa2 100644 --- a/src/SignalR/clients/csharp/Client/test/UnitTests/LongPollingTransportTests.cs +++ b/src/SignalR/clients/csharp/Client/test/UnitTests/LongPollingTransportTests.cs @@ -698,7 +698,7 @@ public async Task SendsDeleteRequestWhenTransportCompleted() public async Task StartAsyncSetsAcceptHeaderCorrectly() { var mockHttpHandler = new Mock(); - var responseTaskCompletionSource = new TaskCompletionSource(); + var responseTaskCompletionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); mockHttpHandler.Protected() .Setup>("SendAsync", ItExpr.IsAny(), ItExpr.IsAny()) diff --git a/src/SignalR/clients/csharp/Client/test/UnitTests/SendUtilsTests.cs b/src/SignalR/clients/csharp/Client/test/UnitTests/SendUtilsTests.cs index 8a4a764a4ae7..bb277aa7271a 100644 --- a/src/SignalR/clients/csharp/Client/test/UnitTests/SendUtilsTests.cs +++ b/src/SignalR/clients/csharp/Client/test/UnitTests/SendUtilsTests.cs @@ -19,7 +19,7 @@ public partial class SendUtilsTests : VerifiableLoggedTest public async Task SendMessagesSetsCorrectAcceptHeader() { var mockHttpHandler = new Mock(); - var responseTaskCompletionSource = new TaskCompletionSource(); + var responseTaskCompletionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); mockHttpHandler.Protected() .Setup>("SendAsync", ItExpr.IsAny(), ItExpr.IsAny()) diff --git a/src/SignalR/clients/csharp/Client/test/UnitTests/ServerSentEventsTransportTests.cs b/src/SignalR/clients/csharp/Client/test/UnitTests/ServerSentEventsTransportTests.cs index f7c5a0049908..28bd88bea060 100644 --- a/src/SignalR/clients/csharp/Client/test/UnitTests/ServerSentEventsTransportTests.cs +++ b/src/SignalR/clients/csharp/Client/test/UnitTests/ServerSentEventsTransportTests.cs @@ -415,7 +415,7 @@ public async Task SSETransportThrowsForInvalidTransferFormat(TransferFormat tran public async Task StartAsyncSetsCorrectAcceptHeaderForSSE() { var mockHttpHandler = new Mock(); - var responseTaskCompletionSource = new TaskCompletionSource(); + var responseTaskCompletionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); mockHttpHandler.Protected() .Setup>("SendAsync", ItExpr.IsAny(), ItExpr.IsAny()) From 64b15b9c32066a4563fa4c6a78e6cfdf0ced4545 Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Fri, 10 May 2024 10:25:17 +0200 Subject: [PATCH 32/39] Sorting headers --- .../Client/test/UnitTests/HttpConnectionTests.cs | 10 +++++----- .../Client/test/UnitTests/LongPollingTransportTests.cs | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.cs b/src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.cs index 4a99acd385e4..23cd1c7d4910 100644 --- a/src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.cs +++ b/src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.cs @@ -2,26 +2,26 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.IO.Pipelines; using System.Linq; using System.Net; using System.Net.Http; +using System.Net.Http.Headers; using System.Security.Cryptography.X509Certificates; +using System.Text; 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.Tests; using Microsoft.AspNetCore.InternalTesting; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Logging.Testing; using Moq; -using Xunit; using Moq.Protected; -using System.IO.Pipelines; -using System.Text; -using System.Net.Http.Headers; -using Microsoft.AspNetCore.Http.Connections.Client.Internal; +using Xunit; namespace Microsoft.AspNetCore.SignalR.Client.Tests; diff --git a/src/SignalR/clients/csharp/Client/test/UnitTests/LongPollingTransportTests.cs b/src/SignalR/clients/csharp/Client/test/UnitTests/LongPollingTransportTests.cs index d1420cf33fa2..5043e07fbbf4 100644 --- a/src/SignalR/clients/csharp/Client/test/UnitTests/LongPollingTransportTests.cs +++ b/src/SignalR/clients/csharp/Client/test/UnitTests/LongPollingTransportTests.cs @@ -8,6 +8,7 @@ using System.Linq; using System.Net; using System.Net.Http; +using System.Net.Http.Headers; using System.Text; using System.Threading; using System.Threading.Tasks; @@ -18,7 +19,6 @@ using Moq; using Moq.Protected; using Xunit; -using System.Net.Http.Headers; namespace Microsoft.AspNetCore.SignalR.Client.Tests; From fb4d3e22ca9f01554a25d2c20fa30fe27fb8065b Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Tue, 14 May 2024 13:35:09 +0200 Subject: [PATCH 33/39] SendUtils test to use testHttpHandler instead of moq --- .../Client/test/UnitTests/SendUtilsTests.cs | 30 ++++++++----------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/src/SignalR/clients/csharp/Client/test/UnitTests/SendUtilsTests.cs b/src/SignalR/clients/csharp/Client/test/UnitTests/SendUtilsTests.cs index bb277aa7271a..3573749c9d48 100644 --- a/src/SignalR/clients/csharp/Client/test/UnitTests/SendUtilsTests.cs +++ b/src/SignalR/clients/csharp/Client/test/UnitTests/SendUtilsTests.cs @@ -18,33 +18,29 @@ public partial class SendUtilsTests : VerifiableLoggedTest [Fact] public async Task SendMessagesSetsCorrectAcceptHeader() { - var mockHttpHandler = new Mock(); + var testHttpHandler = new TestHttpMessageHandler(); var responseTaskCompletionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - mockHttpHandler.Protected() - .Setup>("SendAsync", ItExpr.IsAny(), ItExpr.IsAny()) - .Returns((HttpRequestMessage request, CancellationToken cancellationToken) => + testHttpHandler.OnRequest((request, next, cancellationToken) => + { + if (request.Headers.Accept?.Contains(new MediaTypeWithQualityHeaderValue("*/*")) == true) + { + responseTaskCompletionSource.SetResult(ResponseUtils.CreateResponse(HttpStatusCode.OK)); + } + else { - if (request.Headers.Accept?.Contains(new MediaTypeWithQualityHeaderValue("*/*")) == true) - { - responseTaskCompletionSource.SetResult(ResponseUtils.CreateResponse(HttpStatusCode.OK)); - } - else - { - responseTaskCompletionSource.SetResult(ResponseUtils.CreateResponse(HttpStatusCode.BadRequest)); - } - return responseTaskCompletionSource.Task; - }); + responseTaskCompletionSource.SetResult(ResponseUtils.CreateResponse(HttpStatusCode.BadRequest)); + } + return responseTaskCompletionSource.Task; + }); - using (var httpClient = new HttpClient(mockHttpHandler.Object)) - using (StartVerifiableLog()) + using (var httpClient = new HttpClient(testHttpHandler)) { var pipe = new Pipe(); var application = new DuplexPipe(pipe.Reader, pipe.Writer); // Simulate writing data to send await application.Output.WriteAsync(Encoding.UTF8.GetBytes("Hello World")); - application.Output.Complete(); await SendUtils.SendMessages(new Uri("http://fakeuri.org"), application, httpClient, logger: Logger).DefaultTimeout(); From a226948a1d4b8d3b02c528edc2cc21203efb8152 Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Tue, 14 May 2024 13:35:57 +0200 Subject: [PATCH 34/39] ServerSentEvents headers test to use testHttpHandler instead of moq --- .../ServerSentEventsTransportTests.cs | 32 +++++++++---------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/src/SignalR/clients/csharp/Client/test/UnitTests/ServerSentEventsTransportTests.cs b/src/SignalR/clients/csharp/Client/test/UnitTests/ServerSentEventsTransportTests.cs index 28bd88bea060..c54100b249e4 100644 --- a/src/SignalR/clients/csharp/Client/test/UnitTests/ServerSentEventsTransportTests.cs +++ b/src/SignalR/clients/csharp/Client/test/UnitTests/ServerSentEventsTransportTests.cs @@ -414,31 +414,29 @@ public async Task SSETransportThrowsForInvalidTransferFormat(TransferFormat tran [Fact] public async Task StartAsyncSetsCorrectAcceptHeaderForSSE() { - var mockHttpHandler = new Mock(); + var testHttpHandler = new TestHttpMessageHandler(); var responseTaskCompletionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - mockHttpHandler.Protected() - .Setup>("SendAsync", ItExpr.IsAny(), ItExpr.IsAny()) - .Returns((HttpRequestMessage request, CancellationToken cancellationToken) => + // Setting up the handler to check for 'text/event-stream' Accept header + testHttpHandler.OnRequest((request, next, cancellationToken) => + { + if (request.Headers.Accept?.Contains(new MediaTypeWithQualityHeaderValue("text/event-stream")) == true) { - if (request.Headers.Accept?.Contains(new MediaTypeWithQualityHeaderValue("text/event-stream")) == true) - { - responseTaskCompletionSource.SetResult(new HttpResponseMessage(HttpStatusCode.OK)); - } - else - { - responseTaskCompletionSource.SetResult(new HttpResponseMessage(HttpStatusCode.NoContent)); - } - return responseTaskCompletionSource.Task; - }); + responseTaskCompletionSource.SetResult(new HttpResponseMessage(HttpStatusCode.OK)); + } + else + { + responseTaskCompletionSource.SetResult(new HttpResponseMessage(HttpStatusCode.NoContent)); + } + return responseTaskCompletionSource.Task; + }); - using (var httpClient = new HttpClient(mockHttpHandler.Object)) - using (StartVerifiableLog()) + using (var httpClient = new HttpClient(testHttpHandler)) { var sseTransport = new ServerSentEventsTransport(httpClient, loggerFactory: LoggerFactory); + // Starting the SSE transport and verifying the outcome await sseTransport.StartAsync(new Uri("http://fakeuri.org"), TransferFormat.Text).DefaultTimeout(); - await sseTransport.StopAsync().DefaultTimeout(); Assert.True(responseTaskCompletionSource.Task.IsCompleted); From 1a5a21e560da226a1d9a083d6addb7f15ddb1e58 Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Tue, 14 May 2024 13:41:11 +0200 Subject: [PATCH 35/39] Improvement to PollRequestsContainCorrectAcceptHeader method to effectively handle the intial http request and the requests in Poll() --- .../UnitTests/LongPollingTransportTests.cs | 75 +++++++++++++------ 1 file changed, 54 insertions(+), 21 deletions(-) diff --git a/src/SignalR/clients/csharp/Client/test/UnitTests/LongPollingTransportTests.cs b/src/SignalR/clients/csharp/Client/test/UnitTests/LongPollingTransportTests.cs index 5043e07fbbf4..9f94a0cd515e 100644 --- a/src/SignalR/clients/csharp/Client/test/UnitTests/LongPollingTransportTests.cs +++ b/src/SignalR/clients/csharp/Client/test/UnitTests/LongPollingTransportTests.cs @@ -16,6 +16,7 @@ using Microsoft.AspNetCore.Http.Connections.Client.Internal; using Microsoft.AspNetCore.SignalR.Tests; using Microsoft.AspNetCore.InternalTesting; +using Microsoft.Extensions.Logging.Abstractions; using Moq; using Moq.Protected; using Xunit; @@ -695,43 +696,75 @@ public async Task SendsDeleteRequestWhenTransportCompleted() } [Fact] - public async Task StartAsyncSetsAcceptHeaderCorrectly() + public async Task PollRequestsContainCorrectAcceptHeader() { - var mockHttpHandler = new Mock(); - var responseTaskCompletionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var testHttpHandler = new TestHttpMessageHandler(); + var responseTaskCompletionSource = new TaskCompletionSource(); + var requestCount = 0; + var allHeadersCorrect = true; + var firstRequestReceived = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var secondRequestReceived = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + testHttpHandler.OnRequest(async (request, next, cancellationToken) => + { + if (request.Headers.Accept?.Contains(new MediaTypeWithQualityHeaderValue("*/*")) != true) + { + allHeadersCorrect = false; + } - mockHttpHandler.Protected() - .Setup>("SendAsync", ItExpr.IsAny(), ItExpr.IsAny()) - .Returns((HttpRequestMessage request, CancellationToken cancellationToken) => + requestCount++; + + if (requestCount == 1) { - if (request.Headers.Accept?.Contains(new MediaTypeWithQualityHeaderValue("*/*")) == true) + firstRequestReceived.SetResult(); + } + else if (requestCount == 2) + { + secondRequestReceived.SetResult(); + } + + if (requestCount >= 2) + { + if (allHeadersCorrect) { - responseTaskCompletionSource.SetResult(ResponseUtils.CreateResponse(HttpStatusCode.OK)); + responseTaskCompletionSource.TrySetResult(new HttpResponseMessage(HttpStatusCode.OK)); } else { - responseTaskCompletionSource.SetResult(ResponseUtils.CreateResponse(HttpStatusCode.BadRequest)); + responseTaskCompletionSource.TrySetResult(new HttpResponseMessage(HttpStatusCode.NoContent)); } - return responseTaskCompletionSource.Task; - }); + } - using (var httpClient = new HttpClient(mockHttpHandler.Object)) + return await Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK)); + }); + + using (var httpClient = new HttpClient(testHttpHandler)) { - var transport = new LongPollingTransport(httpClient, loggerFactory: LoggerFactory); + var loggerFactory = NullLoggerFactory.Instance; + var transport = new LongPollingTransport(httpClient, loggerFactory: loggerFactory); + + var cts = new CancellationTokenSource(); + var startTask = transport.StartAsync(new Uri("http://test.com"), TransferFormat.Text, cts.Token); + + await firstRequestReceived.Task; + + await Task.Delay(100); + + cts.Cancel(); - await transport.StartAsync(TestUri, TransferFormat.Text).DefaultTimeout(); + await secondRequestReceived.Task; await transport.StopAsync(); + if (!responseTaskCompletionSource.Task.IsCompleted) + { + responseTaskCompletionSource.TrySetResult(new HttpResponseMessage(HttpStatusCode.BadRequest)); + } + + Assert.True(responseTaskCompletionSource.Task.IsCompleted); var response = await responseTaskCompletionSource.Task; Assert.Equal(HttpStatusCode.OK, response.StatusCode); - } - - mockHttpHandler.Protected().Verify( - "SendAsync", - Times.AtLeast(2), //ensure the mockHttpHandler was used twice in StartAsync() and Poll() - ItExpr.IsAny(), - ItExpr.IsAny()); } + } From d4e3f0c3d6bb51eb714740fbf9aabd7bf4a92aa3 Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Wed, 15 May 2024 09:34:21 +0200 Subject: [PATCH 36/39] Adding default timeouts --- .../clients/csharp/Client/test/UnitTests/HttpConnectionTests.cs | 2 +- .../csharp/Client/test/UnitTests/LongPollingTransportTests.cs | 2 +- .../Client/test/UnitTests/ServerSentEventsTransportTests.cs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.cs b/src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.cs index 23cd1c7d4910..81c9e807698f 100644 --- a/src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.cs +++ b/src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.cs @@ -199,7 +199,7 @@ await WithConnectionAsync( } Assert.True(negotiateUrlTcs.Task.IsCompleted); - var headerWasFound = await negotiateUrlTcs.Task; + var headerWasFound = await negotiateUrlTcs.Task.DefaultTimeout(); Assert.True(headerWasFound); } } diff --git a/src/SignalR/clients/csharp/Client/test/UnitTests/LongPollingTransportTests.cs b/src/SignalR/clients/csharp/Client/test/UnitTests/LongPollingTransportTests.cs index 9f94a0cd515e..6630858e13e6 100644 --- a/src/SignalR/clients/csharp/Client/test/UnitTests/LongPollingTransportTests.cs +++ b/src/SignalR/clients/csharp/Client/test/UnitTests/LongPollingTransportTests.cs @@ -762,7 +762,7 @@ public async Task PollRequestsContainCorrectAcceptHeader() } Assert.True(responseTaskCompletionSource.Task.IsCompleted); - var response = await responseTaskCompletionSource.Task; + var response = await responseTaskCompletionSource.Task.DefaultTimeout(); Assert.Equal(HttpStatusCode.OK, response.StatusCode); } } diff --git a/src/SignalR/clients/csharp/Client/test/UnitTests/ServerSentEventsTransportTests.cs b/src/SignalR/clients/csharp/Client/test/UnitTests/ServerSentEventsTransportTests.cs index c54100b249e4..bb987398048a 100644 --- a/src/SignalR/clients/csharp/Client/test/UnitTests/ServerSentEventsTransportTests.cs +++ b/src/SignalR/clients/csharp/Client/test/UnitTests/ServerSentEventsTransportTests.cs @@ -440,7 +440,7 @@ public async Task StartAsyncSetsCorrectAcceptHeaderForSSE() await sseTransport.StopAsync().DefaultTimeout(); Assert.True(responseTaskCompletionSource.Task.IsCompleted); - var response = await responseTaskCompletionSource.Task; + var response = await responseTaskCompletionSource.Task.DefaultTimeout(); Assert.Equal(HttpStatusCode.OK, response.StatusCode); } } From 8df74b99ba9ed06c5f67a9a560f31a9ff1708830 Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Wed, 15 May 2024 09:36:05 +0200 Subject: [PATCH 37/39] Sorting using statements and removing Moq --- .../clients/csharp/Client/test/UnitTests/SendUtilsTests.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/SignalR/clients/csharp/Client/test/UnitTests/SendUtilsTests.cs b/src/SignalR/clients/csharp/Client/test/UnitTests/SendUtilsTests.cs index 3573749c9d48..5449de98c6a7 100644 --- a/src/SignalR/clients/csharp/Client/test/UnitTests/SendUtilsTests.cs +++ b/src/SignalR/clients/csharp/Client/test/UnitTests/SendUtilsTests.cs @@ -4,12 +4,10 @@ using System.IO.Pipelines; using System.Net; using System.Net.Http; +using System.Net.Http.Headers; using System.Text; using Microsoft.AspNetCore.Http.Connections.Client.Internal; using Microsoft.AspNetCore.SignalR.Tests; -using Moq; -using Moq.Protected; -using System.Net.Http.Headers; using Microsoft.AspNetCore.InternalTesting; namespace Microsoft.AspNetCore.SignalR.Client.Tests; From bf623c07a5b4c701d3692b50d46cc22cab632b0e Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Wed, 15 May 2024 09:42:29 +0200 Subject: [PATCH 38/39] Removing delay, only using secondRequestReceived, adding default timeouts and using testUri --- .../UnitTests/LongPollingTransportTests.cs | 24 +++---------------- 1 file changed, 3 insertions(+), 21 deletions(-) diff --git a/src/SignalR/clients/csharp/Client/test/UnitTests/LongPollingTransportTests.cs b/src/SignalR/clients/csharp/Client/test/UnitTests/LongPollingTransportTests.cs index 6630858e13e6..1e3d00cf3a37 100644 --- a/src/SignalR/clients/csharp/Client/test/UnitTests/LongPollingTransportTests.cs +++ b/src/SignalR/clients/csharp/Client/test/UnitTests/LongPollingTransportTests.cs @@ -702,7 +702,6 @@ public async Task PollRequestsContainCorrectAcceptHeader() var responseTaskCompletionSource = new TaskCompletionSource(); var requestCount = 0; var allHeadersCorrect = true; - var firstRequestReceived = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var secondRequestReceived = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); testHttpHandler.OnRequest(async (request, next, cancellationToken) => @@ -714,11 +713,7 @@ public async Task PollRequestsContainCorrectAcceptHeader() requestCount++; - if (requestCount == 1) - { - firstRequestReceived.SetResult(); - } - else if (requestCount == 2) + if (requestCount == 2) { secondRequestReceived.SetResult(); } @@ -743,28 +738,15 @@ public async Task PollRequestsContainCorrectAcceptHeader() var loggerFactory = NullLoggerFactory.Instance; var transport = new LongPollingTransport(httpClient, loggerFactory: loggerFactory); - var cts = new CancellationTokenSource(); - var startTask = transport.StartAsync(new Uri("http://test.com"), TransferFormat.Text, cts.Token); - - await firstRequestReceived.Task; - - await Task.Delay(100); - - cts.Cancel(); + var startTask = transport.StartAsync(TestUri, TransferFormat.Text); - await secondRequestReceived.Task; + await secondRequestReceived.Task.DefaultTimeout(); await transport.StopAsync(); - if (!responseTaskCompletionSource.Task.IsCompleted) - { - responseTaskCompletionSource.TrySetResult(new HttpResponseMessage(HttpStatusCode.BadRequest)); - } - Assert.True(responseTaskCompletionSource.Task.IsCompleted); var response = await responseTaskCompletionSource.Task.DefaultTimeout(); Assert.Equal(HttpStatusCode.OK, response.StatusCode); } } - } From fe978ae74fb95756a214bfea4e7c089b0f5b6ef1 Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Wed, 15 May 2024 12:29:39 +0200 Subject: [PATCH 39/39] Adding default time to CompletionSource --- .../clients/csharp/Client/test/UnitTests/SendUtilsTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SignalR/clients/csharp/Client/test/UnitTests/SendUtilsTests.cs b/src/SignalR/clients/csharp/Client/test/UnitTests/SendUtilsTests.cs index 5449de98c6a7..cb98da2eceb4 100644 --- a/src/SignalR/clients/csharp/Client/test/UnitTests/SendUtilsTests.cs +++ b/src/SignalR/clients/csharp/Client/test/UnitTests/SendUtilsTests.cs @@ -43,7 +43,7 @@ public async Task SendMessagesSetsCorrectAcceptHeader() await SendUtils.SendMessages(new Uri("http://fakeuri.org"), application, httpClient, logger: Logger).DefaultTimeout(); - var response = await responseTaskCompletionSource.Task; + var response = await responseTaskCompletionSource.Task.DefaultTimeout(); Assert.Equal(HttpStatusCode.OK, response.StatusCode); } }