From 586e129dbd71875e0e6fb55e058824458d11fee8 Mon Sep 17 00:00:00 2001 From: James Croft Date: Fri, 15 May 2026 12:15:39 +0100 Subject: [PATCH 1/5] fix(networking): dispose IDisposable objects across HTTP request types Add using declarations for HttpRequestMessage and HttpResponseMessage in all network request classes. Implement IDisposable on NetworkRequestManager to properly clean up Timer, and wrap CancellationTokenSource in using with synchronous task completion. Fix StreamWriter leak in StringExtensions.ToMemoryStreamAsync and avoid unprotected HttpClientHandler allocation in RetryDelegatingHandler constructor. --- .../Extensions/StringExtensions.cs | 10 +++++++--- .../Http/INetworkRequestManager.cs | 2 +- .../Http/NetworkRequestManager.cs | 20 ++++++++++++++++++- .../Requests/Json/JsonDeleteNetworkRequest.cs | 10 +++++----- .../Requests/Json/JsonGetNetworkRequest.cs | 10 +++++----- .../Requests/Json/JsonPatchNetworkRequest.cs | 10 +++++----- .../Requests/Json/JsonPostNetworkRequest.cs | 10 +++++----- .../Requests/Json/JsonPutNetworkRequest.cs | 10 +++++----- .../MultipartFormDataPostNetworkRequest.cs | 4 ++-- .../Streams/StreamGetNetworkRequest.cs | 10 +++++----- .../Http/RetryDelegatingHandler.cs | 4 +++- 11 files changed, 62 insertions(+), 38 deletions(-) diff --git a/src/MADE.Data.Converters/Extensions/StringExtensions.cs b/src/MADE.Data.Converters/Extensions/StringExtensions.cs index 9a00d896..ed1c9534 100644 --- a/src/MADE.Data.Converters/Extensions/StringExtensions.cs +++ b/src/MADE.Data.Converters/Extensions/StringExtensions.cs @@ -125,9 +125,13 @@ public static string FromBase64(this string base64Value, Encoding? encoding = de public static async Task ToMemoryStreamAsync(this string value, CancellationToken cancellationToken = default) { var stream = new MemoryStream(); - var writer = new StreamWriter(stream); - await writer.WriteAsync(value.AsMemory(), cancellationToken).ConfigureAwait(false); - await writer.FlushAsync(cancellationToken).ConfigureAwait(false); + var writer = new StreamWriter(stream, leaveOpen: true); + await using (writer.ConfigureAwait(false)) + { + await writer.WriteAsync(value.AsMemory(), cancellationToken).ConfigureAwait(false); + await writer.FlushAsync(cancellationToken).ConfigureAwait(false); + } + stream.Position = 0; return stream; } diff --git a/src/MADE.Networking/Http/INetworkRequestManager.cs b/src/MADE.Networking/Http/INetworkRequestManager.cs index 92a2133f..7051effc 100644 --- a/src/MADE.Networking/Http/INetworkRequestManager.cs +++ b/src/MADE.Networking/Http/INetworkRequestManager.cs @@ -10,7 +10,7 @@ namespace MADE.Networking.Http; /// /// Defines an interface for a network request manager. /// -public interface INetworkRequestManager +public interface INetworkRequestManager : IDisposable { /// /// Gets the current queue of network requests. diff --git a/src/MADE.Networking/Http/NetworkRequestManager.cs b/src/MADE.Networking/Http/NetworkRequestManager.cs index 8e7f7523..4baf8740 100644 --- a/src/MADE.Networking/Http/NetworkRequestManager.cs +++ b/src/MADE.Networking/Http/NetworkRequestManager.cs @@ -21,6 +21,7 @@ public sealed class NetworkRequestManager : INetworkRequestManager private readonly Timer processTimer; private bool isProcessingRequests; + private bool disposed; /// /// Initializes a new instance of the class. @@ -65,6 +66,21 @@ public void Stop() this.processTimer.Stop(); } + /// + public void Dispose() + { + if (this.disposed) + { + return; + } + + this.processTimer.Tick -= this.OnProcessTimerTick; + this.processTimer.Stop(); + this.processTimer.Dispose(); + this.disposed = true; + GC.SuppressFinalize(this); + } + /// /// Processes the current queue of network requests. /// @@ -79,7 +95,7 @@ public void ProcessCurrentQueue() try { - var cts = new CancellationTokenSource(); + using var cts = new CancellationTokenSource(); var requestTasks = new List(); var requestCallbacks = new List(); @@ -97,6 +113,8 @@ public void ProcessCurrentQueue() { requestTasks.Add(ExecuteRequestsAsync(this.CurrentQueue, container, cts.Token)); } + + Task.WhenAll(requestTasks).GetAwaiter().GetResult(); } finally { diff --git a/src/MADE.Networking/Http/Requests/Json/JsonDeleteNetworkRequest.cs b/src/MADE.Networking/Http/Requests/Json/JsonDeleteNetworkRequest.cs index e389d276..caff9ceb 100644 --- a/src/MADE.Networking/Http/Requests/Json/JsonDeleteNetworkRequest.cs +++ b/src/MADE.Networking/Http/Requests/Json/JsonDeleteNetworkRequest.cs @@ -101,7 +101,7 @@ private async Task GetJsonResponseAsync(CancellationToken cancellationTo } var uri = new Uri(this.Url); - var request = new HttpRequestMessage(HttpMethod.Delete, uri); + using var request = new HttpRequestMessage(HttpMethod.Delete, uri); if (this.Headers != null) { @@ -111,10 +111,10 @@ private async Task GetJsonResponseAsync(CancellationToken cancellationTo } } - HttpResponseMessage response = await this.client.SendAsync( - request, - HttpCompletionOption.ResponseHeadersRead, - cancellationToken).ConfigureAwait(false); + using HttpResponseMessage response = await this.client.SendAsync( + request, + HttpCompletionOption.ResponseHeadersRead, + cancellationToken).ConfigureAwait(false); response.EnsureSuccessStatusCode(); diff --git a/src/MADE.Networking/Http/Requests/Json/JsonGetNetworkRequest.cs b/src/MADE.Networking/Http/Requests/Json/JsonGetNetworkRequest.cs index 8e7650d2..dbcd38d5 100644 --- a/src/MADE.Networking/Http/Requests/Json/JsonGetNetworkRequest.cs +++ b/src/MADE.Networking/Http/Requests/Json/JsonGetNetworkRequest.cs @@ -101,7 +101,7 @@ private async Task GetJsonResponseAsync(CancellationToken cancellationTo } var uri = new Uri(this.Url); - var request = new HttpRequestMessage(HttpMethod.Get, uri); + using var request = new HttpRequestMessage(HttpMethod.Get, uri); if (this.Headers != null) { @@ -111,10 +111,10 @@ private async Task GetJsonResponseAsync(CancellationToken cancellationTo } } - HttpResponseMessage response = await this.client.SendAsync( - request, - HttpCompletionOption.ResponseHeadersRead, - cancellationToken).ConfigureAwait(false); + using HttpResponseMessage response = await this.client.SendAsync( + request, + HttpCompletionOption.ResponseHeadersRead, + cancellationToken).ConfigureAwait(false); response.EnsureSuccessStatusCode(); diff --git a/src/MADE.Networking/Http/Requests/Json/JsonPatchNetworkRequest.cs b/src/MADE.Networking/Http/Requests/Json/JsonPatchNetworkRequest.cs index f99ff5af..2dcb7370 100644 --- a/src/MADE.Networking/Http/Requests/Json/JsonPatchNetworkRequest.cs +++ b/src/MADE.Networking/Http/Requests/Json/JsonPatchNetworkRequest.cs @@ -133,7 +133,7 @@ private async Task GetJsonResponseAsync(CancellationToken cancellationTo var uri = new Uri(this.Url); - var request = new HttpRequestMessage + using var request = new HttpRequestMessage { Method = new HttpMethod("PATCH"), RequestUri = uri, @@ -148,10 +148,10 @@ private async Task GetJsonResponseAsync(CancellationToken cancellationTo } } - HttpResponseMessage response = await this.client.SendAsync( - request, - HttpCompletionOption.ResponseHeadersRead, - cancellationToken).ConfigureAwait(false); + using HttpResponseMessage response = await this.client.SendAsync( + request, + HttpCompletionOption.ResponseHeadersRead, + cancellationToken).ConfigureAwait(false); response.EnsureSuccessStatusCode(); diff --git a/src/MADE.Networking/Http/Requests/Json/JsonPostNetworkRequest.cs b/src/MADE.Networking/Http/Requests/Json/JsonPostNetworkRequest.cs index 11589e52..3031bac2 100644 --- a/src/MADE.Networking/Http/Requests/Json/JsonPostNetworkRequest.cs +++ b/src/MADE.Networking/Http/Requests/Json/JsonPostNetworkRequest.cs @@ -133,7 +133,7 @@ private async Task GetJsonResponseAsync(CancellationToken cancellationTo var uri = new Uri(this.Url); - var request = new HttpRequestMessage(HttpMethod.Post, uri) + using var request = new HttpRequestMessage(HttpMethod.Post, uri) { Content = new StringContent(this.Data, Encoding.UTF8, "application/json"), }; @@ -146,10 +146,10 @@ private async Task GetJsonResponseAsync(CancellationToken cancellationTo } } - HttpResponseMessage response = await this.client.SendAsync( - request, - HttpCompletionOption.ResponseHeadersRead, - cancellationToken).ConfigureAwait(false); + using HttpResponseMessage response = await this.client.SendAsync( + request, + HttpCompletionOption.ResponseHeadersRead, + cancellationToken).ConfigureAwait(false); response.EnsureSuccessStatusCode(); diff --git a/src/MADE.Networking/Http/Requests/Json/JsonPutNetworkRequest.cs b/src/MADE.Networking/Http/Requests/Json/JsonPutNetworkRequest.cs index 93eda6c6..0cc6b36d 100644 --- a/src/MADE.Networking/Http/Requests/Json/JsonPutNetworkRequest.cs +++ b/src/MADE.Networking/Http/Requests/Json/JsonPutNetworkRequest.cs @@ -129,7 +129,7 @@ private async Task GetJsonResponseAsync(CancellationToken cancellationTo var uri = new Uri(this.Url); - var request = new HttpRequestMessage(HttpMethod.Put, uri) + using var request = new HttpRequestMessage(HttpMethod.Put, uri) { Content = new StringContent(this.Data, Encoding.UTF8, "application/json"), }; @@ -142,10 +142,10 @@ private async Task GetJsonResponseAsync(CancellationToken cancellationTo } } - HttpResponseMessage response = await this.client.SendAsync( - request, - HttpCompletionOption.ResponseHeadersRead, - cancellationToken).ConfigureAwait(false); + using HttpResponseMessage response = await this.client.SendAsync( + request, + HttpCompletionOption.ResponseHeadersRead, + cancellationToken).ConfigureAwait(false); response.EnsureSuccessStatusCode(); diff --git a/src/MADE.Networking/Http/Requests/MultipartFormDataPostNetworkRequest.cs b/src/MADE.Networking/Http/Requests/MultipartFormDataPostNetworkRequest.cs index 1092b7fa..ccb7209f 100644 --- a/src/MADE.Networking/Http/Requests/MultipartFormDataPostNetworkRequest.cs +++ b/src/MADE.Networking/Http/Requests/MultipartFormDataPostNetworkRequest.cs @@ -120,7 +120,7 @@ private async Task PostAndGetJsonResponseAsync(CancellationToken cancell } var uri = new Uri(this.Url); - var request = new HttpRequestMessage(HttpMethod.Post, uri) { Content = this.Content }; + using var request = new HttpRequestMessage(HttpMethod.Post, uri) { Content = this.Content }; if (this.Headers != null) { @@ -130,7 +130,7 @@ private async Task PostAndGetJsonResponseAsync(CancellationToken cancell } } - HttpResponseMessage response = await this.client.SendAsync(request, cancellationToken).ConfigureAwait(false); + using HttpResponseMessage response = await this.client.SendAsync(request, cancellationToken).ConfigureAwait(false); response.EnsureSuccessStatusCode(); return await response.Content.ReadAsStringAsync(cancellationToken).ConfigureAwait(false); diff --git a/src/MADE.Networking/Http/Requests/Streams/StreamGetNetworkRequest.cs b/src/MADE.Networking/Http/Requests/Streams/StreamGetNetworkRequest.cs index 4a0a3590..8c4deaf2 100644 --- a/src/MADE.Networking/Http/Requests/Streams/StreamGetNetworkRequest.cs +++ b/src/MADE.Networking/Http/Requests/Streams/StreamGetNetworkRequest.cs @@ -98,7 +98,7 @@ private async Task GetStreamResponseAsync(CancellationToken cancellation } var uri = new Uri(this.Url); - var request = new HttpRequestMessage(HttpMethod.Get, uri); + using var request = new HttpRequestMessage(HttpMethod.Get, uri); if (this.Headers != null) { @@ -108,10 +108,10 @@ private async Task GetStreamResponseAsync(CancellationToken cancellation } } - HttpResponseMessage response = await this.client.SendAsync( - request, - HttpCompletionOption.ResponseHeadersRead, - cancellationToken).ConfigureAwait(false); + using HttpResponseMessage response = await this.client.SendAsync( + request, + HttpCompletionOption.ResponseHeadersRead, + cancellationToken).ConfigureAwait(false); response.EnsureSuccessStatusCode(); diff --git a/src/MADE.Networking/Http/RetryDelegatingHandler.cs b/src/MADE.Networking/Http/RetryDelegatingHandler.cs index a89db955..12e19dbe 100644 --- a/src/MADE.Networking/Http/RetryDelegatingHandler.cs +++ b/src/MADE.Networking/Http/RetryDelegatingHandler.cs @@ -34,8 +34,10 @@ public class RetryDelegatingHandler : DelegatingHandler /// The maximum number of retry attempts. Default is 3. /// The initial delay before the first retry. Default is 1 second. public RetryDelegatingHandler(int maxRetries = 3, TimeSpan? initialDelay = null) - : this(new HttpClientHandler(), maxRetries, initialDelay) { + this.InnerHandler = new HttpClientHandler(); + this.MaxRetries = maxRetries; + this.InitialDelay = initialDelay ?? TimeSpan.FromSeconds(1); } /// From 0a70727920c117b670c0353f36a5234bec76058d Mon Sep 17 00:00:00 2001 From: James Croft Date: Fri, 15 May 2026 12:19:55 +0100 Subject: [PATCH 2/5] refactor(networking): remove unnecessary intermediate list in ProcessCurrentQueue Dispatch tasks directly as items are dequeued from the queue instead of collecting into a separate requestCallbacks list first. --- src/MADE.Networking/Http/NetworkRequestManager.cs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/MADE.Networking/Http/NetworkRequestManager.cs b/src/MADE.Networking/Http/NetworkRequestManager.cs index 4baf8740..d54824b7 100644 --- a/src/MADE.Networking/Http/NetworkRequestManager.cs +++ b/src/MADE.Networking/Http/NetworkRequestManager.cs @@ -97,7 +97,6 @@ public void ProcessCurrentQueue() { using var cts = new CancellationTokenSource(); var requestTasks = new List(); - var requestCallbacks = new List(); while (this.CurrentQueue.Count > 0) { @@ -105,15 +104,10 @@ public void ProcessCurrentQueue() this.CurrentQueue.FirstOrDefault().Key, out NetworkRequestCallback request)) { - requestCallbacks.Add(request); + requestTasks.Add(ExecuteRequestsAsync(this.CurrentQueue, request, cts.Token)); } } - foreach (NetworkRequestCallback container in requestCallbacks) - { - requestTasks.Add(ExecuteRequestsAsync(this.CurrentQueue, container, cts.Token)); - } - Task.WhenAll(requestTasks).GetAwaiter().GetResult(); } finally From 9a5be19ccb39986e3b07cf71362ab1e3fd88c45d Mon Sep 17 00:00:00 2001 From: James Croft Date: Fri, 15 May 2026 14:28:30 +0100 Subject: [PATCH 3/5] fix: address PR review comments for code quality changes - Remove IDisposable from INetworkRequestManager interface to avoid breaking change for downstream implementers - Simplify StreamWriter async disposal in StringExtensions to fix compilation issue with ConfigureAwait - Add ArgumentOutOfRangeException validation for maxRetries in RetryDelegatingHandler constructors - Remove using on HttpRequestMessage in MultipartFormDataPostNetworkRequest to prevent disposing shared Content property --- src/MADE.Data.Converters/Extensions/StringExtensions.cs | 9 +++------ src/MADE.Networking/Http/INetworkRequestManager.cs | 3 +-- .../Http/Requests/MultipartFormDataPostNetworkRequest.cs | 2 +- src/MADE.Networking/Http/RetryDelegatingHandler.cs | 4 ++++ 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/MADE.Data.Converters/Extensions/StringExtensions.cs b/src/MADE.Data.Converters/Extensions/StringExtensions.cs index ed1c9534..f9a0109e 100644 --- a/src/MADE.Data.Converters/Extensions/StringExtensions.cs +++ b/src/MADE.Data.Converters/Extensions/StringExtensions.cs @@ -125,12 +125,9 @@ public static string FromBase64(this string base64Value, Encoding? encoding = de public static async Task ToMemoryStreamAsync(this string value, CancellationToken cancellationToken = default) { var stream = new MemoryStream(); - var writer = new StreamWriter(stream, leaveOpen: true); - await using (writer.ConfigureAwait(false)) - { - await writer.WriteAsync(value.AsMemory(), cancellationToken).ConfigureAwait(false); - await writer.FlushAsync(cancellationToken).ConfigureAwait(false); - } + await using var writer = new StreamWriter(stream, leaveOpen: true); + await writer.WriteAsync(value.AsMemory(), cancellationToken).ConfigureAwait(false); + await writer.FlushAsync(cancellationToken).ConfigureAwait(false); stream.Position = 0; return stream; diff --git a/src/MADE.Networking/Http/INetworkRequestManager.cs b/src/MADE.Networking/Http/INetworkRequestManager.cs index 7051effc..cb57f5c4 100644 --- a/src/MADE.Networking/Http/INetworkRequestManager.cs +++ b/src/MADE.Networking/Http/INetworkRequestManager.cs @@ -1,7 +1,6 @@ // MADE Apps licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using System; using System.Collections.Concurrent; using MADE.Networking.Http.Requests; @@ -10,7 +9,7 @@ namespace MADE.Networking.Http; /// /// Defines an interface for a network request manager. /// -public interface INetworkRequestManager : IDisposable +public interface INetworkRequestManager { /// /// Gets the current queue of network requests. diff --git a/src/MADE.Networking/Http/Requests/MultipartFormDataPostNetworkRequest.cs b/src/MADE.Networking/Http/Requests/MultipartFormDataPostNetworkRequest.cs index ccb7209f..bb0bb057 100644 --- a/src/MADE.Networking/Http/Requests/MultipartFormDataPostNetworkRequest.cs +++ b/src/MADE.Networking/Http/Requests/MultipartFormDataPostNetworkRequest.cs @@ -120,7 +120,7 @@ private async Task PostAndGetJsonResponseAsync(CancellationToken cancell } var uri = new Uri(this.Url); - using var request = new HttpRequestMessage(HttpMethod.Post, uri) { Content = this.Content }; + var request = new HttpRequestMessage(HttpMethod.Post, uri) { Content = this.Content }; if (this.Headers != null) { diff --git a/src/MADE.Networking/Http/RetryDelegatingHandler.cs b/src/MADE.Networking/Http/RetryDelegatingHandler.cs index 12e19dbe..f772526e 100644 --- a/src/MADE.Networking/Http/RetryDelegatingHandler.cs +++ b/src/MADE.Networking/Http/RetryDelegatingHandler.cs @@ -35,6 +35,8 @@ public class RetryDelegatingHandler : DelegatingHandler /// The initial delay before the first retry. Default is 1 second. public RetryDelegatingHandler(int maxRetries = 3, TimeSpan? initialDelay = null) { + ArgumentOutOfRangeException.ThrowIfNegative(maxRetries); + this.InnerHandler = new HttpClientHandler(); this.MaxRetries = maxRetries; this.InitialDelay = initialDelay ?? TimeSpan.FromSeconds(1); @@ -49,6 +51,8 @@ public RetryDelegatingHandler(int maxRetries = 3, TimeSpan? initialDelay = null) public RetryDelegatingHandler(HttpMessageHandler innerHandler, int maxRetries = 3, TimeSpan? initialDelay = null) : base(innerHandler) { + ArgumentOutOfRangeException.ThrowIfNegative(maxRetries); + this.MaxRetries = maxRetries; this.InitialDelay = initialDelay ?? TimeSpan.FromSeconds(1); } From 1eac035eda0f1f60f9a527c25c465089279bf10b Mon Sep 17 00:00:00 2001 From: James Croft Date: Fri, 15 May 2026 15:00:30 +0100 Subject: [PATCH 4/5] test: enhance network request tests with mock HTTP handlers --- .../Tests/JsonDeleteNetworkRequestTests.cs | 4 +++- .../Tests/JsonGetNetworkRequestTests.cs | 4 +++- .../Tests/JsonPatchNetworkRequestTests.cs | 7 +++++-- .../Tests/JsonPostNetworkRequestTests.cs | 7 +++++-- .../Tests/JsonPutNetworkRequestTests.cs | 7 +++++-- .../Tests/NetworkRequestFactoryTests.cs | 12 +++++++++--- .../Tests/NetworkRequestManagerTests.cs | 9 +++++++-- 7 files changed, 37 insertions(+), 13 deletions(-) diff --git a/tests/MADE.Networking.Tests/Tests/JsonDeleteNetworkRequestTests.cs b/tests/MADE.Networking.Tests/Tests/JsonDeleteNetworkRequestTests.cs index 449a9649..cfa04a27 100644 --- a/tests/MADE.Networking.Tests/Tests/JsonDeleteNetworkRequestTests.cs +++ b/tests/MADE.Networking.Tests/Tests/JsonDeleteNetworkRequestTests.cs @@ -21,7 +21,9 @@ public async Task ShouldReturnSuccessFromDeleteEndpointWithResponse() const bool queryValue = true; var requestUrl = $"https://httpbin.org/delete?{query}={queryValue}"; - var request = new JsonDeleteNetworkRequest(new HttpClient(), requestUrl); + var responseJson = $"{{\"args\":{{\"{query}\":\"{queryValue}\"}},\"url\":\"{requestUrl}\"}}"; + var mockHandler = new MockHttpMessageHandler(HttpStatusCode.OK, responseJson); + var request = new JsonDeleteNetworkRequest(new HttpClient(mockHandler), requestUrl); // Act var response = await request.ExecuteAsync(); diff --git a/tests/MADE.Networking.Tests/Tests/JsonGetNetworkRequestTests.cs b/tests/MADE.Networking.Tests/Tests/JsonGetNetworkRequestTests.cs index 8abb0a9d..09edad0f 100644 --- a/tests/MADE.Networking.Tests/Tests/JsonGetNetworkRequestTests.cs +++ b/tests/MADE.Networking.Tests/Tests/JsonGetNetworkRequestTests.cs @@ -21,7 +21,9 @@ public async Task ShouldReturnSuccessFromGetEndpointWithResponse() const bool queryValue = true; var requestUrl = $"https://httpbin.org/get?{query}={queryValue}"; - var request = new JsonGetNetworkRequest(new HttpClient(), requestUrl); + var responseJson = $"{{\"args\":{{\"{query}\":\"{queryValue}\"}},\"url\":\"{requestUrl}\"}}"; + var mockHandler = new MockHttpMessageHandler(HttpStatusCode.OK, responseJson); + var request = new JsonGetNetworkRequest(new HttpClient(mockHandler), requestUrl); // Act var response = await request.ExecuteAsync(); diff --git a/tests/MADE.Networking.Tests/Tests/JsonPatchNetworkRequestTests.cs b/tests/MADE.Networking.Tests/Tests/JsonPatchNetworkRequestTests.cs index f2f06ab1..7315602c 100644 --- a/tests/MADE.Networking.Tests/Tests/JsonPatchNetworkRequestTests.cs +++ b/tests/MADE.Networking.Tests/Tests/JsonPatchNetworkRequestTests.cs @@ -21,10 +21,13 @@ public async Task ShouldReturnSuccessFromPatchEndpointWithResponse() var requestData = new RequestData { Key = "test", Enabled = true }; const string requestUrl = "https://httpbin.org/patch"; + var serializedData = JsonSerializer.Serialize(requestData); + var responseJson = JsonSerializer.Serialize(new { data = serializedData, url = requestUrl }); + var mockHandler = new MockHttpMessageHandler(HttpStatusCode.OK, responseJson); var request = new JsonPatchNetworkRequest( - new HttpClient(), + new HttpClient(mockHandler), requestUrl, - JsonSerializer.Serialize(requestData)); + serializedData); // Act var response = await request.ExecuteAsync(); diff --git a/tests/MADE.Networking.Tests/Tests/JsonPostNetworkRequestTests.cs b/tests/MADE.Networking.Tests/Tests/JsonPostNetworkRequestTests.cs index 69af5c76..33f0d22a 100644 --- a/tests/MADE.Networking.Tests/Tests/JsonPostNetworkRequestTests.cs +++ b/tests/MADE.Networking.Tests/Tests/JsonPostNetworkRequestTests.cs @@ -21,10 +21,13 @@ public async Task ShouldReturnSuccessFromPostEndpointWithResponse() var requestData = new RequestData { Key = "test", Enabled = true }; const string requestUrl = "https://httpbin.org/post"; + var serializedData = JsonSerializer.Serialize(requestData); + var responseJson = JsonSerializer.Serialize(new { data = serializedData, url = requestUrl }); + var mockHandler = new MockHttpMessageHandler(HttpStatusCode.OK, responseJson); var request = new JsonPostNetworkRequest( - new HttpClient(), + new HttpClient(mockHandler), requestUrl, - JsonSerializer.Serialize(requestData)); + serializedData); // Act var response = await request.ExecuteAsync(); diff --git a/tests/MADE.Networking.Tests/Tests/JsonPutNetworkRequestTests.cs b/tests/MADE.Networking.Tests/Tests/JsonPutNetworkRequestTests.cs index be260964..12a2fee3 100644 --- a/tests/MADE.Networking.Tests/Tests/JsonPutNetworkRequestTests.cs +++ b/tests/MADE.Networking.Tests/Tests/JsonPutNetworkRequestTests.cs @@ -21,10 +21,13 @@ public async Task ShouldReturnSuccessFromPutEndpointWithResponse() var requestData = new RequestData { Key = "test", Enabled = true }; const string requestUrl = "https://httpbin.org/put"; + var serializedData = JsonSerializer.Serialize(requestData); + var responseJson = JsonSerializer.Serialize(new { data = serializedData, url = requestUrl }); + var mockHandler = new MockHttpMessageHandler(HttpStatusCode.OK, responseJson); var request = new JsonPutNetworkRequest( - new HttpClient(), + new HttpClient(mockHandler), requestUrl, - JsonSerializer.Serialize(requestData)); + serializedData); // Act var response = await request.ExecuteAsync(); diff --git a/tests/MADE.Networking.Tests/Tests/NetworkRequestFactoryTests.cs b/tests/MADE.Networking.Tests/Tests/NetworkRequestFactoryTests.cs index 68a2016b..1efe2a58 100644 --- a/tests/MADE.Networking.Tests/Tests/NetworkRequestFactoryTests.cs +++ b/tests/MADE.Networking.Tests/Tests/NetworkRequestFactoryTests.cs @@ -20,7 +20,9 @@ public class WhenCreatingRequests public async Task ShouldCreateGetRequest() { // Arrange - var factory = CreateFactory(); + var mockHandler = new MockHttpMessageHandler(HttpStatusCode.OK, + JsonSerializer.Serialize(new { url = "https://httpbin.org/get?key=value" })); + var factory = CreateFactoryWithMock(mockHandler); // Act var request = factory.Get("https://httpbin.org/get?key=value"); @@ -35,8 +37,10 @@ public async Task ShouldCreateGetRequest() public async Task ShouldCreatePostRequest() { // Arrange - var factory = CreateFactory(); var data = JsonSerializer.Serialize(new { key = "value" }); + var mockHandler = new MockHttpMessageHandler(HttpStatusCode.OK, + JsonSerializer.Serialize(new { data, url = "https://httpbin.org/post" })); + var factory = CreateFactoryWithMock(mockHandler); // Act var request = factory.Post("https://httpbin.org/post", data); @@ -51,7 +55,9 @@ public async Task ShouldCreatePostRequest() public async Task ShouldCreateDeleteRequest() { // Arrange - var factory = CreateFactory(); + var mockHandler = new MockHttpMessageHandler(HttpStatusCode.OK, + JsonSerializer.Serialize(new { url = "https://httpbin.org/delete" })); + var factory = CreateFactoryWithMock(mockHandler); // Act var request = factory.Delete("https://httpbin.org/delete"); diff --git a/tests/MADE.Networking.Tests/Tests/NetworkRequestManagerTests.cs b/tests/MADE.Networking.Tests/Tests/NetworkRequestManagerTests.cs index 9c275423..1556a296 100644 --- a/tests/MADE.Networking.Tests/Tests/NetworkRequestManagerTests.cs +++ b/tests/MADE.Networking.Tests/Tests/NetworkRequestManagerTests.cs @@ -1,5 +1,6 @@ using System; using System.Diagnostics.CodeAnalysis; +using System.Net; using System.Net.Http; using System.Threading; using MADE.Networking.Http; @@ -126,7 +127,9 @@ public void ShouldProcessQueue() const bool queryValue = true; var requestUrl = $"https://httpbin.org/get?{query}={queryValue}"; - var request = new JsonGetNetworkRequest(new HttpClient(), requestUrl); + var responseJson = $"{{\"args\":{{\"{query}\":\"{queryValue}\"}},\"url\":\"{requestUrl}\"}}"; + var mockHandler = new MockHttpMessageHandler(HttpStatusCode.OK, responseJson); + var request = new JsonGetNetworkRequest(new HttpClient(mockHandler), requestUrl); var manager = new NetworkRequestManager(); @@ -162,7 +165,9 @@ public void ShouldStopProcessingQueue() const bool queryValue = true; var requestUrl = $"https://httpbin.org/get?{query}={queryValue}"; - var request = new JsonGetNetworkRequest(new HttpClient(), requestUrl); + var responseJson = $"{{\"args\":{{\"{query}\":\"{queryValue}\"}},\"url\":\"{requestUrl}\"}}"; + var mockHandler = new MockHttpMessageHandler(HttpStatusCode.OK, responseJson); + var request = new JsonGetNetworkRequest(new HttpClient(mockHandler), requestUrl); var manager = new NetworkRequestManager(); From ebffedcf2a5a05a10374440f07ed97de7f7f5668 Mon Sep 17 00:00:00 2001 From: James Croft Date: Fri, 15 May 2026 15:35:05 +0100 Subject: [PATCH 5/5] refactor: implement IDisposable in NetworkRequestManager and enhance resource management --- src/MADE.Networking/Http/NetworkRequestManager.cs | 2 +- .../Http/Requests/MultipartFormDataPostNetworkRequest.cs | 3 ++- .../Http/Requests/Streams/StreamGetNetworkRequest.cs | 6 +++++- src/MADE.Networking/Http/RetryDelegatingHandler.cs | 2 ++ 4 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/MADE.Networking/Http/NetworkRequestManager.cs b/src/MADE.Networking/Http/NetworkRequestManager.cs index d54824b7..c869d245 100644 --- a/src/MADE.Networking/Http/NetworkRequestManager.cs +++ b/src/MADE.Networking/Http/NetworkRequestManager.cs @@ -16,7 +16,7 @@ namespace MADE.Networking.Http; /// /// Defines a manager for executing queued network requests. /// -public sealed class NetworkRequestManager : INetworkRequestManager +public sealed class NetworkRequestManager : INetworkRequestManager, IDisposable { private readonly Timer processTimer; diff --git a/src/MADE.Networking/Http/Requests/MultipartFormDataPostNetworkRequest.cs b/src/MADE.Networking/Http/Requests/MultipartFormDataPostNetworkRequest.cs index bb0bb057..01b12ea2 100644 --- a/src/MADE.Networking/Http/Requests/MultipartFormDataPostNetworkRequest.cs +++ b/src/MADE.Networking/Http/Requests/MultipartFormDataPostNetworkRequest.cs @@ -120,7 +120,7 @@ private async Task PostAndGetJsonResponseAsync(CancellationToken cancell } var uri = new Uri(this.Url); - var request = new HttpRequestMessage(HttpMethod.Post, uri) { Content = this.Content }; + using var request = new HttpRequestMessage(HttpMethod.Post, uri) { Content = this.Content }; if (this.Headers != null) { @@ -131,6 +131,7 @@ private async Task PostAndGetJsonResponseAsync(CancellationToken cancell } using HttpResponseMessage response = await this.client.SendAsync(request, cancellationToken).ConfigureAwait(false); + request.Content = null; // Detach shared content to prevent disposal cascade response.EnsureSuccessStatusCode(); return await response.Content.ReadAsStringAsync(cancellationToken).ConfigureAwait(false); diff --git a/src/MADE.Networking/Http/Requests/Streams/StreamGetNetworkRequest.cs b/src/MADE.Networking/Http/Requests/Streams/StreamGetNetworkRequest.cs index 8c4deaf2..3fcc644f 100644 --- a/src/MADE.Networking/Http/Requests/Streams/StreamGetNetworkRequest.cs +++ b/src/MADE.Networking/Http/Requests/Streams/StreamGetNetworkRequest.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.IO; using System.Net.Http; using System.Threading; using System.Threading.Tasks; @@ -115,6 +116,9 @@ private async Task GetStreamResponseAsync(CancellationToken cancellation response.EnsureSuccessStatusCode(); - return await response.Content.ReadAsStreamAsync().ConfigureAwait(false); + var memoryStream = new MemoryStream(); + await response.Content.CopyToAsync(memoryStream, cancellationToken).ConfigureAwait(false); + memoryStream.Position = 0; + return memoryStream; } } diff --git a/src/MADE.Networking/Http/RetryDelegatingHandler.cs b/src/MADE.Networking/Http/RetryDelegatingHandler.cs index f772526e..0bc5dd95 100644 --- a/src/MADE.Networking/Http/RetryDelegatingHandler.cs +++ b/src/MADE.Networking/Http/RetryDelegatingHandler.cs @@ -36,6 +36,7 @@ public class RetryDelegatingHandler : DelegatingHandler public RetryDelegatingHandler(int maxRetries = 3, TimeSpan? initialDelay = null) { ArgumentOutOfRangeException.ThrowIfNegative(maxRetries); + ArgumentOutOfRangeException.ThrowIfNegative(initialDelay?.TotalMilliseconds ?? 0); this.InnerHandler = new HttpClientHandler(); this.MaxRetries = maxRetries; @@ -52,6 +53,7 @@ public RetryDelegatingHandler(HttpMessageHandler innerHandler, int maxRetries = : base(innerHandler) { ArgumentOutOfRangeException.ThrowIfNegative(maxRetries); + ArgumentOutOfRangeException.ThrowIfNegative(initialDelay?.TotalMilliseconds ?? 0); this.MaxRetries = maxRetries; this.InitialDelay = initialDelay ?? TimeSpan.FromSeconds(1);