Skip to content

Commit f96d1a2

Browse files
louis-zlouis-z
committed
Fix hooks
1 parent 78a5a70 commit f96d1a2

13 files changed

+115
-186
lines changed

‎NGitLab.Tests/Docker/GitLabTestContextRequestOptions.cs‎

Lines changed: 8 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -26,86 +26,44 @@ public GitLabTestContextRequestOptions()
2626
: base(retryCount: 0, retryInterval: TimeSpan.FromSeconds(1), isIncremental: true)
2727
{
2828
UserAgent = "NGitLab.Tests/1.0.0";
29-
MessageHandler = new TestHttpMessageHandler(this);
29+
MessageHook = new TestHttpMessageHandler(this);
3030
}
3131

32-
private sealed class TestHttpMessageHandler : IHttpMessageHandler
32+
private sealed class TestHttpMessageHandler : IHttpMessageHook
3333
{
3434
private readonly GitLabTestContextRequestOptions _options;
3535
private readonly HttpClient _httpClient;
36+
private HttpRequestMessage _currentRequest;
3637

3738
public TestHttpMessageHandler(GitLabTestContextRequestOptions options)
3839
{
3940
_options = options;
4041
_httpClient = HttpClientManager.GetOrCreateHttpClient(options);
4142
}
4243

43-
public HttpResponseMessage Send(HttpRequestMessage request)
44+
public void PrepareRequest(HttpRequestMessage request)
4445
{
46+
_currentRequest = request;
47+
4548
lock (_options._allRequests)
4649
{
4750
_options._allRequests.Add(request);
4851
}
4952

50-
HttpResponseMessage response = null;
51-
5253
// GitLab is unstable, so let's make sure we don't overload it with many concurrent requests
5354
s_semaphoreSlim.Wait();
54-
try
55-
{
56-
try
57-
{
58-
response = _httpClient.Send(request);
59-
}
60-
catch (HttpRequestException)
61-
{
62-
// Log the request even if it fails
63-
_options.LogRequest(request, response);
64-
throw;
65-
}
66-
67-
_options.LogRequest(request, response);
68-
}
69-
finally
70-
{
71-
s_semaphoreSlim.Release();
72-
}
73-
74-
return response;
7555
}
7656

77-
public async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken = default)
57+
public void ProcessResponse(HttpResponseMessage response)
7858
{
79-
lock (_options._allRequests)
80-
{
81-
_options._allRequests.Add(request);
82-
}
83-
84-
HttpResponseMessage response = null;
85-
86-
// GitLab is unstable, so let's make sure we don't overload it with many concurrent requests
87-
await s_semaphoreSlim.WaitAsync(cancellationToken).ConfigureAwait(false);
8859
try
8960
{
90-
try
91-
{
92-
response = await _httpClient.SendAsync(request, cancellationToken).ConfigureAwait(false);
93-
}
94-
catch (HttpRequestException)
95-
{
96-
// Log the request even if it fails
97-
await _options.LogRequestAsync(request, response).ConfigureAwait(false);
98-
throw;
99-
}
100-
101-
await _options.LogRequestAsync(request, response).ConfigureAwait(false);
61+
_options.LogRequest(_currentRequest, response);
10262
}
10363
finally
10464
{
10565
s_semaphoreSlim.Release();
10666
}
107-
108-
return response;
10967
}
11068
}
11169

‎NGitLab.Tests/HttpRequestorTests.cs‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
using System.Linq;
55
using System.Net;
66
using System.Net.Http;
7-
using System.Threading;
87
using System.Threading.Tasks;
98
using NGitLab.Http;
109
using NGitLab.Impl;
@@ -151,18 +150,18 @@ private sealed class MockRequestOptions : RequestOptions
151150

152151
public bool ShouldRetryCalled { get; set; }
153152

154-
public HashSet<HttpRequestMessage> HandledRequests { get; } = new();
153+
public HashSet<HttpRequestMessage> HandledRequests { get; } = [];
155154

156155
public MockRequestOptions()
157156
: base(retryCount: 0, retryInterval: TimeSpan.Zero)
158157
{
159-
MessageHandler = new MockHttpMessageHandler(this);
158+
MessageHook = new MockHttpMessageHook(this);
160159
}
161160

162161
public MockRequestOptions(int retryCount, TimeSpan retryInterval, bool isIncremental = true)
163162
: base(retryCount, retryInterval, isIncremental)
164163
{
165-
MessageHandler = new MockHttpMessageHandler(this);
164+
MessageHook = new MockHttpMessageHook(this);
166165
}
167166

168167
public override bool ShouldRetry(Exception ex, int retryNumber)
@@ -172,16 +171,16 @@ public override bool ShouldRetry(Exception ex, int retryNumber)
172171
return base.ShouldRetry(ex, retryNumber);
173172
}
174173

175-
private sealed class MockHttpMessageHandler : IHttpMessageHandler
174+
private sealed class MockHttpMessageHook : IHttpMessageHook
176175
{
177176
private readonly MockRequestOptions _options;
178177

179-
public MockHttpMessageHandler(MockRequestOptions options)
178+
public MockHttpMessageHook(MockRequestOptions options)
180179
{
181180
_options = options;
182181
}
183182

184-
public HttpResponseMessage Send(HttpRequestMessage request)
183+
public void PrepareRequest(HttpRequestMessage request)
185184
{
186185
if (request.Headers.TryGetValues("Sudo", out var sudoValues))
187186
{
@@ -192,9 +191,8 @@ public HttpResponseMessage Send(HttpRequestMessage request)
192191
throw new GitLabException { StatusCode = HttpStatusCode.InternalServerError };
193192
}
194193

195-
public Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken = default)
194+
public void ProcessResponse(HttpResponseMessage response)
196195
{
197-
return Task.FromResult(Send(request));
198196
}
199197
}
200198
}

‎NGitLab/Http/CustomHttpMessageHandler.cs‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
using System.Net.Http;
2+
using System.Threading;
3+
using System.Threading.Tasks;
4+
5+
namespace NGitLab.Http;
6+
7+
internal sealed class CustomHttpMessageHandler : DelegatingHandler
8+
{
9+
private readonly IHttpMessageHook _hook;
10+
11+
public CustomHttpMessageHandler(HttpMessageHandler innerHandler, IHttpMessageHook hook)
12+
{
13+
InnerHandler = innerHandler;
14+
_hook = hook;
15+
}
16+
17+
#if NET8_0_OR_GREATER
18+
protected override HttpResponseMessage Send(HttpRequestMessage request, CancellationToken cancellationToken)
19+
{
20+
_hook.PrepareRequest(request);
21+
var response = base.Send(request, cancellationToken);
22+
_hook.ProcessResponse(response);
23+
return response;
24+
}
25+
#endif
26+
27+
protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
28+
{
29+
_hook.PrepareRequest(request);
30+
var response = await base.SendAsync(request, cancellationToken).ConfigureAwait(false);
31+
_hook.ProcessResponse(response);
32+
return response;
33+
}
34+
}

‎NGitLab/Http/DefaultHttpMessageHandler.cs‎

Lines changed: 0 additions & 33 deletions
This file was deleted.

‎NGitLab/Http/HttpClientManager.cs‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ internal static class HttpClientManager
1818
/// </summary>
1919
private static readonly TimeSpan s_defaultHttpClientTimeout = TimeSpan.FromMinutes(5);
2020

21-
private static readonly Lazy<HttpClient> s_defaultClient = new(() => CreateHttpClient(proxy: null, timeout: null));
21+
private static readonly Lazy<HttpClient> s_defaultClient = new(() => CreateHttpClient(proxy: null, timeout: null, msgHook: null));
2222

2323
/// <summary>
2424
/// Gets the singleton HttpClient instance for default scenarios.
@@ -31,7 +31,7 @@ internal static class HttpClientManager
3131
/// <param name="proxy">Optional proxy configuration.</param>
3232
/// <param name="timeout">Optional timeout value.</param>
3333
/// <returns>A configured HttpClient instance.</returns>
34-
public static HttpClient CreateHttpClient(IWebProxy proxy, TimeSpan? timeout)
34+
private static HttpClient CreateHttpClient(IWebProxy proxy, TimeSpan? timeout, IHttpMessageHook msgHook)
3535
{
3636
var handler = new HttpClientHandler
3737
{
@@ -44,7 +44,17 @@ public static HttpClient CreateHttpClient(IWebProxy proxy, TimeSpan? timeout)
4444
handler.UseProxy = true;
4545
}
4646

47-
var client = new HttpClient(handler)
47+
HttpMessageHandler handlerChain;
48+
if (msgHook is null)
49+
{
50+
handlerChain = handler;
51+
}
52+
else
53+
{
54+
handlerChain = new CustomHttpMessageHandler(handler, msgHook);
55+
}
56+
57+
var client = new HttpClient(handlerChain)
4858
{
4959
Timeout = timeout ?? s_defaultHttpClientTimeout,
5060
};
@@ -60,18 +70,13 @@ public static HttpClient CreateHttpClient(IWebProxy proxy, TimeSpan? timeout)
6070
/// <returns>An HttpClient instance.</returns>
6171
public static HttpClient GetOrCreateHttpClient(RequestOptions options)
6272
{
63-
if (options.HttpClientFactory is not null)
64-
{
65-
return options.HttpClientFactory(options);
66-
}
67-
6873
// Use singleton if no custom proxy or timeout
69-
if (options.Proxy is null && !options.HttpClientTimeout.HasValue)
74+
if (options.Proxy is null && options.HttpClientTimeout is null && options.MessageHook is null)
7075
{
7176
return DefaultClient;
7277
}
7378

7479
// Create new instance for custom configuration
75-
return CreateHttpClient(options.Proxy, options.HttpClientTimeout);
80+
return CreateHttpClient(options.Proxy, options.HttpClientTimeout, options.MessageHook);
7681
}
7782
}

‎NGitLab/Http/IHttpMessageHandler.cs‎

Lines changed: 0 additions & 27 deletions
This file was deleted.

‎NGitLab/Http/IHttpMessageHook.cs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
using System.Net.Http;
2+
3+
namespace NGitLab.Http;
4+
5+
/// <summary>
6+
/// Provides an extensibility point for customizing HTTP request behavior.
7+
/// This interface replaces the obsolete virtual methods on <see cref="RequestOptions"/>.
8+
/// </summary>
9+
public interface IHttpMessageHook
10+
{
11+
void PrepareRequest(HttpRequestMessage request);
12+
13+
void ProcessResponse(HttpResponseMessage response);
14+
}

‎NGitLab/Impl/HttpRequestor.GitLabRequest.cs‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,19 +35,19 @@ private sealed class GitLabRequest
3535

3636
private readonly Dictionary<string, string> _headers = new(StringComparer.OrdinalIgnoreCase);
3737

38-
private readonly IHttpMessageHandler _messageHandler;
38+
private readonly HttpClient _httpClient;
3939
private readonly RequestOptions _options;
4040

4141
private bool HasOutput
4242
=> (Method == MethodType.Delete || Method == MethodType.Post || Method == MethodType.Put || Method == MethodType.Patch)
4343
&& Data != null;
4444

45-
public GitLabRequest(Uri url, MethodType method, object data, string apiToken, RequestOptions options, IHttpMessageHandler messageHandler)
45+
public GitLabRequest(Uri url, MethodType method, object data, string apiToken, RequestOptions options, HttpClient httpClient)
4646
{
4747
Method = method;
4848
Url = url;
4949
Data = data;
50-
_messageHandler = messageHandler;
50+
_httpClient = httpClient;
5151
_options = options ?? RequestOptions.Default;
5252

5353
// Add headers
@@ -112,8 +112,11 @@ private WebResponse GetResponseImpl(RequestOptions options)
112112
try
113113
{
114114
var request = CreateHttpRequestMessage();
115-
var response = _messageHandler.Send(request);
116-
115+
#if NET8_0_OR_GREATER
116+
var response = _httpClient.Send(request);
117+
#else
118+
var response = _httpClient.SendAsync(request).GetAwaiter().GetResult();
119+
#endif
117120
if (!response.IsSuccessStatusCode)
118121
{
119122
HandleHttpResponseError(response);
@@ -133,7 +136,7 @@ private async Task<WebResponse> GetResponseImplAsync(RequestOptions options, Can
133136
try
134137
{
135138
var request = CreateHttpRequestMessage();
136-
var response = await _messageHandler.SendAsync(request, cancellationToken).ConfigureAwait(false);
139+
var response = await _httpClient.SendAsync(request, cancellationToken).ConfigureAwait(false);
137140

138141
if (!response.IsSuccessStatusCode)
139142
{

0 commit comments

Comments
 (0)