Skip to content

Commit b6e298c

Browse files
author
MIchael Yarichuk
committed
code quality refactorings
1 parent 525b506 commit b6e298c

9 files changed

Lines changed: 125 additions & 79 deletions

‎Simple.HttpClientFactory.Tests/PollyHttpMessageHandlerTests.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
using Polly;
2-
using Simple.HttpClientFactory.Polly;
32
using System;
43
using System.Collections.Generic;
54
using System.Net;
65
using System.Net.Http;
76
using System.Text;
87
using System.Threading.Tasks;
8+
using Simple.HttpClientFactory.MessageHandlers;
99
using Xunit;
1010

1111
namespace Simple.HttpClientFactory.Tests
@@ -15,12 +15,12 @@ public class PollyHttpMessageHandlerTests
1515
{
1616
[Fact]
1717
public void Ctor_with_null_should_throw() =>
18-
Assert.Throws<ArgumentNullException>(() => new PolicyHttpMessageHandler(null));
18+
Assert.Throws<ArgumentNullException>(() => new PollyMessageMiddleware(null));
1919

2020
[Fact]
2121
public async Task Null_param_in_send_async_should_throw()
2222
{
23-
var middlewareHandler = new PolicyHttpMessageHandler(Policy<HttpResponseMessage>
23+
var middlewareHandler = new PollyMessageMiddleware(Policy<HttpResponseMessage>
2424
.Handle<HttpRequestException>()
2525
.OrResult(result => (int)result.StatusCode >= 500 || result.StatusCode == HttpStatusCode.RequestTimeout)
2626
.WaitAndRetryAsync(3, retryAttempt => TimeSpan.FromSeconds(1)));

‎Simple.HttpClientFactory/HttpClientBuilder.cs‎

Lines changed: 68 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
using Polly;
2-
using Simple.HttpClientFactory.Polly;
32
using System;
43
using System.Collections.Generic;
54
using System.Linq;
@@ -9,6 +8,7 @@
98
using System.Net.Security;
109
#endif
1110
using System.Security.Cryptography.X509Certificates;
11+
using Simple.HttpClientFactory.MessageHandlers;
1212

1313
namespace Simple.HttpClientFactory
1414
{
@@ -71,10 +71,13 @@ public IHttpClientBuilder WithTimeout(in TimeSpan timeout)
7171
public IHttpClientBuilder WithMessageExceptionHandler(
7272
Func<HttpRequestException, bool> exceptionHandlingPredicate,
7373
Func<HttpRequestException, Exception> exceptionHandler) =>
74-
WithMessageHandler(new MessageExceptionHandler(exceptionHandlingPredicate, exceptionHandler, null));
74+
WithMessageHandler(new ExceptionTranslatorRequestMiddleware(exceptionHandlingPredicate, exceptionHandler, null));
7575

76+
/// <exception cref="T:System.ArgumentNullException"><paramref name="handler"/> is <see langword="null"/></exception>
7677
public IHttpClientBuilder WithMessageHandler(DelegatingHandler handler)
7778
{
79+
if(handler == null)
80+
throw new ArgumentNullException(nameof(handler));
7881
if(_middlewareHandlers.Count > 0)
7982
_middlewareHandlers.Last().InnerHandler = handler;
8083
_middlewareHandlers.Add(handler);
@@ -94,7 +97,7 @@ public IHttpClientBuilder WithMessageHandlers(IEnumerable<DelegatingHandler> han
9497
//see https://github.com/dotnet/extensions/issues/1345#issuecomment-607490721
9598
public HttpClient Build(Action<SocketsHttpHandler> clientHandlerConfigurator = null)
9699
{
97-
PolicyHttpMessageHandler policyHandler = null;
100+
PollyMessageMiddleware rootPolicyHandler = null;
98101

99102
var clientHandler = new SocketsHttpHandler
100103
{
@@ -119,16 +122,16 @@ public HttpClient Build(Action<SocketsHttpHandler> clientHandlerConfigurator = n
119122

120123
for(int i = 0; i < _policies.Count; i++)
121124
{
122-
if(policyHandler == null)
123-
policyHandler = new PolicyHttpMessageHandler(_policies[i], clientHandler);
125+
if(rootPolicyHandler == null)
126+
rootPolicyHandler = new PollyMessageMiddleware(_policies[i], clientHandler);
124127
else
125128
{
126-
var @new = new PolicyHttpMessageHandler(_policies[i], policyHandler);
127-
policyHandler = @new;
129+
var @new = new PollyMessageMiddleware(_policies[i], rootPolicyHandler);
130+
rootPolicyHandler = @new;
128131
}
129132
}
130133

131-
var client = ConstructClientWithMiddleware(clientHandler, policyHandler);
134+
var client = ConstructClientWithMiddleware(clientHandler, rootPolicyHandler);
132135

133136
if(_timeout.HasValue)
134137
client.Timeout = _timeout.Value;
@@ -138,68 +141,100 @@ public HttpClient Build(Action<SocketsHttpHandler> clientHandlerConfigurator = n
138141

139142
#else
140143

144+
/// <exception cref="T:System.Exception">A <paramref name="clientHandlerConfigurator"/> throws an exception.</exception>
141145
public HttpClient Build(Action<HttpClientHandler> clientHandlerConfigurator = null)
142146
{
143147

144148
var clientHandler = new HttpClientHandlerEx();
145149

146150
if (_certificates.Count > 0)
147-
clientHandler.ClientCertificates.AddRange(_certificates.ToArray());
151+
{
152+
for (int i = 0; i < _certificates.Count; i++)
153+
clientHandler.ClientCertificates.Add(_certificates[i]);
154+
}
148155

149156
clientHandlerConfigurator?.Invoke(clientHandler);
150157

151-
PolicyHttpMessageHandler policyHandler = null;
158+
PollyMessageMiddleware rootPolicyHandler = null;
152159
for (int i = 0; i < _policies.Count; i++)
153160
{
154-
if (policyHandler == null)
155-
policyHandler = new PolicyHttpMessageHandler(_policies[i], clientHandler);
161+
if (rootPolicyHandler == null)
162+
rootPolicyHandler = new PollyMessageMiddleware(_policies[i], clientHandler);
156163
else
157164
{
158-
var @new = new PolicyHttpMessageHandler(_policies[i], policyHandler);
159-
policyHandler = @new;
165+
var @new = new PollyMessageMiddleware(_policies[i], rootPolicyHandler);
166+
rootPolicyHandler = @new;
160167
}
161168
}
162169

163-
var client = ConstructClientWithMiddleware(clientHandler, policyHandler);
170+
var client = ConstructClientWithMiddleware(clientHandler, rootPolicyHandler);
164171

165172
if (_timeout.HasValue)
166173
client.Timeout = _timeout.Value;
167174

168175
return client;
169176
}
170177
#endif
171-
private HttpClient ConstructClientWithMiddleware<TClientHandler>(TClientHandler clientHandler, PolicyHttpMessageHandler policyHandler)
178+
private HttpClient ConstructClientWithMiddleware<TClientHandler>(TClientHandler clientHandler, PollyMessageMiddleware rootPolicyHandler)
172179
where TClientHandler : HttpMessageHandler
173180
{
174181
HttpClient client;
175-
if (policyHandler != null)
182+
client = CreateClientInternal(clientHandler, rootPolicyHandler, _middlewareHandlers.LastOrDefault());
183+
184+
InitializeDefaultHeadersIfNeeded();
185+
186+
return client;
187+
188+
189+
void InitializeDefaultHeadersIfNeeded()
190+
{
191+
if (_defaultHeaders.Count > 0)
192+
{
193+
foreach (var header in _defaultHeaders)
194+
{
195+
if (!client.DefaultRequestHeaders.Contains(header.Key))
196+
client.DefaultRequestHeaders.Add(header.Key, header.Value);
197+
}
198+
}
199+
}
200+
}
201+
202+
private HttpClient CreateClientInternal<TClientHandler>(TClientHandler clientHandler,
203+
PollyMessageMiddleware rootPolicyHandler, DelegatingHandler lastMiddleware) where TClientHandler : HttpMessageHandler
204+
{
205+
HttpClient InitializeClientWithPoliciesAndMiddleware()
176206
{
207+
HttpClient client;
177208
if (_middlewareHandlers.Count > 0)
178209
{
179-
_middlewareHandlers.LastOrDefault().InnerHandler = policyHandler;
210+
if(lastMiddleware == null)
211+
throw new InvalidOperationException("One or more middleware handlers is null. This is not supposed to happen!");
212+
213+
lastMiddleware.InnerHandler = rootPolicyHandler;
180214
client = new HttpClient(_middlewareHandlers.FirstOrDefault(), true);
181215
}
182216
else
183-
client = new HttpClient(policyHandler, true);
184-
}
185-
else if (_middlewareHandlers.Count > 0)
186-
{
187-
_middlewareHandlers.LastOrDefault().InnerHandler = clientHandler;
188-
client = new HttpClient(_middlewareHandlers.FirstOrDefault(), true);
217+
client = new HttpClient(rootPolicyHandler, true);
218+
219+
return client;
189220
}
190-
else
191-
client = new HttpClient(clientHandler, true);
192221

193-
if(_defaultHeaders.Count > 0)
222+
HttpClient InitializeClientOnlyWithMiddleware()
194223
{
195-
foreach(var header in _defaultHeaders)
196-
{
197-
if(!client.DefaultRequestHeaders.Contains(header.Key))
198-
client.DefaultRequestHeaders.Add(header.Key, header.Value);
199-
}
224+
lastMiddleware.InnerHandler = clientHandler;
225+
var client = new HttpClient(_middlewareHandlers.FirstOrDefault(), true);
226+
return client;
200227
}
201228

202-
return client;
229+
HttpClient createdClient;
230+
if (rootPolicyHandler != null)
231+
createdClient = InitializeClientWithPoliciesAndMiddleware();
232+
else if (_middlewareHandlers.Count > 0)
233+
createdClient = InitializeClientOnlyWithMiddleware();
234+
else
235+
createdClient = new HttpClient(clientHandler, true);
236+
237+
return createdClient;
203238
}
204239
}
205240
}

‎Simple.HttpClientFactory/HttpMessageExtensions.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
using Polly;
2-
using System.Net.Http;
1+
using System.Net.Http;
2+
using Polly;
33

4-
namespace Simple.HttpClientFactory.Polly
4+
namespace Simple.HttpClientFactory
55
{
66
public static class HttpMessageExtensions
77
{

‎Simple.HttpClientFactory/IHttpClientBuilder.cs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,15 @@ public interface IHttpClientBuilder
4848
/// <summary>
4949
/// Add http message handler to processing pipeline
5050
/// </summary>
51+
/// <exception cref="T:System.ArgumentNullException"><paramref name="handler"/> is <see langword="null"/></exception>
5152
IHttpClientBuilder WithMessageHandler(DelegatingHandler handler);
5253

54+
/// <summary>
55+
/// Add multiple http message handlers to processing pipeline
56+
/// </summary>
57+
/// <exception cref="T:System.ArgumentNullException">One of items in <paramref name="handlers"/> is <see langword="null"/></exception>
58+
IHttpClientBuilder WithMessageHandlers(IEnumerable<DelegatingHandler> handlers);
59+
5360
/// <summary>
5461
/// Add exception handler to possibly transform thrown <see cref="HttpRequestException"/> into more user-friendly ones (or add more details)
5562
/// </summary>

Simple.HttpClientFactory/MessageHandlers/MessageExceptionHandler.cs renamed to Simple.HttpClientFactory/MessageHandlers/ExceptionTranslatorRequestMiddleware.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,17 @@
33
using System.Threading;
44
using System.Threading.Tasks;
55

6-
namespace Simple.HttpClientFactory
6+
namespace Simple.HttpClientFactory.MessageHandlers
77
{
8-
public class MessageExceptionHandler : DelegatingHandler
8+
public class ExceptionTranslatorRequestMiddleware : DelegatingHandler
99
{
1010
private readonly Func<HttpRequestException, bool> _exceptionHandlingPredicate;
1111
private readonly Func<HttpRequestException, Exception> _exceptionHandler;
1212

1313
public event EventHandler<HttpRequestException> RequestException;
1414
public event EventHandler<Exception> TransformedRequestException;
1515

16-
public MessageExceptionHandler(
16+
public ExceptionTranslatorRequestMiddleware(
1717
Func<HttpRequestException, bool> exceptionHandlingPredicate,
1818
Func<HttpRequestException, Exception> exceptionHandler, DelegatingHandler handler) : base(handler)
1919
{

‎Simple.HttpClientFactory/MessageHandlers/HttpClientHandlerEx.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
using System.Threading;
66
using System.Threading.Tasks;
77

8-
namespace Simple.HttpClientFactory
8+
namespace Simple.HttpClientFactory.MessageHandlers
99
{
1010
//credit: the idea for storing visited addresses in hash set is taken from https://github.com/NimaAra/Easy.Common/blob/master/Easy.Common/RestClient.cs#L570
1111
//note: Easy.Common is licensed with MIT License (https://github.com/NimaAra/Easy.Common/blob/master/LICENSE)

‎Simple.HttpClientFactory/MessageHandlers/MiddlewareMessageHandler.cs‎

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

‎Simple.HttpClientFactory/MessageHandlers/PollyHttpMessageHandler.cs‎

Lines changed: 31 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,35 @@
1-
using Polly;
2-
using System;
1+
using System;
32
using System.Net.Http;
43
using System.Threading;
54
using System.Threading.Tasks;
5+
using Polly;
66

7-
namespace Simple.HttpClientFactory.Polly
7+
namespace Simple.HttpClientFactory.MessageHandlers
88
{
9-
public class PolicyHttpMessageHandler : DelegatingHandler
9+
public class PollyMessageMiddleware : DelegatingHandler
1010
{
1111
private readonly IAsyncPolicy<HttpResponseMessage> _policy;
1212

1313
/// <summary>
14-
/// Creates a new <see cref="PolicyHttpMessageHandler"/>.
14+
/// Creates a new <see cref="PollyMessageMiddleware"/>.
1515
/// </summary>
1616
/// <param name="policy">The policy.</param>
1717
/// <param name="inner">The inner handler which is responsible for processing the HTTP response messages.</param>
18-
public PolicyHttpMessageHandler(IAsyncPolicy<HttpResponseMessage> policy, HttpMessageHandler inner) : base(inner) =>
18+
/// <exception cref="T:System.ArgumentNullException"><paramref name="policy"/> is <see langword="null"/></exception>
19+
public PollyMessageMiddleware(IAsyncPolicy<HttpResponseMessage> policy, HttpMessageHandler inner) : base(inner) =>
1920
_policy = policy ?? throw new ArgumentNullException(nameof(policy));
2021

2122
/// <summary>
22-
/// Creates a new <see cref="PolicyHttpMessageHandler"/>.
23+
/// Creates a new <see cref="PollyMessageMiddleware"/>.
2324
/// </summary>
2425
/// <param name="policy">The policy.</param>
25-
public PolicyHttpMessageHandler(IAsyncPolicy<HttpResponseMessage> policy) =>
26+
/// <exception cref="T:System.ArgumentNullException"><paramref name="policy"/> is <see langword="null"/></exception>
27+
public PollyMessageMiddleware(IAsyncPolicy<HttpResponseMessage> policy) =>
2628
_policy = policy ?? throw new ArgumentNullException(nameof(policy));
2729

2830

2931
/// <inheritdoc />
32+
/// <exception cref="T:System.ArgumentNullException"><paramref name="request"/> is <see langword="null"/></exception>
3033
protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
3134
{
3235
if (request == null)
@@ -35,32 +38,33 @@ protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage reques
3538
return SendAsyncInternal(request, cancellationToken);
3639
}
3740

38-
private async Task<HttpResponseMessage> SendAsyncInternal(HttpRequestMessage request, CancellationToken cancellationToken)
41+
private Task<HttpResponseMessage> SendAsyncInternal(HttpRequestMessage request, CancellationToken cancellationToken)
3942
{
4043
// Guarantee the existence of a context for every policy execution, but only create a new one if needed. This
4144
// allows later handlers to flow state if desired.
4245
var cleanUpContext = false;
43-
if (!request.TryGetPolicyExecutionContext(out var context))
44-
{
45-
context = new Context();
46-
request.SetPolicyExecutionContext(context);
47-
cleanUpContext = true;
48-
}
46+
var context = GetOrCreatePolicyExecutionContext(request, ref cleanUpContext);
4947

50-
HttpResponseMessage response;
51-
try
52-
{
53-
response = await _policy.ExecuteAsync(
54-
async (c, ct) => await base.SendAsync(request, cancellationToken),
55-
context, cancellationToken).ConfigureAwait(false);
56-
}
57-
finally
48+
return _policy.ExecuteAsync(
49+
async (c, ct) => await base.SendAsync(request, cancellationToken), context, cancellationToken)
50+
.ContinueWith(t =>
51+
{
52+
if(cleanUpContext)
53+
request.SetPolicyExecutionContext(null);
54+
return t.Result;
55+
}, cancellationToken);
56+
57+
Context GetOrCreatePolicyExecutionContext(HttpRequestMessage httpRequestMessage, ref bool b)
5858
{
59-
if (cleanUpContext)
60-
request.SetPolicyExecutionContext(null);
61-
}
59+
if (!httpRequestMessage.TryGetPolicyExecutionContext(out var fetchedContext))
60+
{
61+
fetchedContext = new Context();
62+
httpRequestMessage.SetPolicyExecutionContext(fetchedContext);
63+
b = true;
64+
}
6265

63-
return response;
66+
return fetchedContext;
67+
}
6468
}
6569
}
6670
}

‎Simple.HttpClientFactory/MessageHandlers/RequestMiddleware.cs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
using System.Net.Http;
2+
3+
namespace Simple.HttpClientFactory.MessageHandlers
4+
{
5+
internal class RequestMiddleware : DelegatingHandler
6+
{
7+
public RequestMiddleware(HttpMessageHandler innerHandler) : base(innerHandler){ }
8+
}
9+
}

0 commit comments

Comments
 (0)