This code simply sends a HttpRequest
message and, if it comes back Unauthorized, gets a new login and tries again.
This code is meant to deal with the moment a JSON Web Token goes out of date. In this system the client is notified of this by having its request become Unauthorized, it then picks up a new Token by logging on again. Hiding away that Unauthorized code, it continues to message.
Two things to note:
A HttpRequestMessage has to be recreated every time it is used.
I have a need to recreate the client to pick up the new JWT token
(Edit: Actually can just go in and change some properties for client, I assume it would be worth changing?)
Basically my question is: How unpleasant is my use of delegates and what lengths should I go to change this code, if any?
private async Task<HttpResponseMessage> SendToFooAsync(Func<HttpClient> client, Func<HttpRequestMessage> requestMessage)
{
var response = await client().SendAsync(requestMessage());
if (response.StatusCode == HttpStatusCode.Unauthorized)
{
//Re-authenticate, recreate client with new token, and retry
if (await authService.LoginAsync())
{
response = await client().SendAsync(requestMessage());
Log.Log("Unauthorized code received: Re-Authentication attempt succeeded");
}
else
{
Log.Log("Unauthorized code received: Authentication attempt has failed");
}
}
return response;
}
Simplest code that calls it:
HttpResponseMessage result = await SendToFooAsync(InitialiseHttpClient, () => InitialiseRequest(HttpMethod.Post, path));
Most complex code that calls it
Func<HttpClient> client = () =>
{
var httpClient = InitialiseHttpClient();
httpClient.DefaultRequestHeaders.Accept.Add("some json stuff");
return httpClient;
};
HttpResponseMessage result = await SendToFooAsync(client, () =>
{
var message = InitialiseRequest(HttpMethod.Post, path);
message.Content = new StringContent("some json stuff");
return message;
});
Where InitialiseRequest
and LoginAsync
are my own methods outside the scope of this review. Hopefully their use is obvious here.
-
\$\begingroup\$ BTW, the code you post here should be your real code (no "foo"s or "some stuff"s), because we want to be able to review all aspects of your code. I think it doesn't matter much here, but it's something to keep in mind for the next time. \$\endgroup\$svick– svick2014年08月02日 21:25:07 +00:00Commented Aug 2, 2014 at 21:25
1 Answer 1
How unpleasant is my use of delegates and what lengths should I go to change this code, if any?
If you want the caller to provide some small piece of code, then a delegate is exactly the right way to do it, it's not unpleasant at all.
But looking at your two usage samples, it looks like they have some code in common (calling InitialiseHttpClient
and InitialiseRequest
), so it might make sense to change the signature to something like:
private async Task<HttpResponseMessage> SendToFooAsync(
HttpMethod method, string uri,
Action<HttpClient> clientAction = null,
Action<HttpRequestMessage> requestMessageAction = null)
The two usage samples would then look like this:
SendToFooAsync(HttpMethod.Post, path);
SendToFooAsync(
HttpMethod.Post, path,
client => client.DefaultRequestHeaders.Accept.Add("some json stuff"),
message => message.Content = new StringContent("some json stuff"));
Also, I don't like the way you're naming your delegates. Func<HttpClient>
is not HttpClient
, so it shouldn't be called just client
. Something like clientFunc
would be more accurate.
-
\$\begingroup\$ normally when you have a Func you suffix the identifier with "Factory", i.e httpClientFactory for example \$\endgroup\$Ahmed Alejo– Ahmed Alejo2016年01月19日 17:00:27 +00:00Commented Jan 19, 2016 at 17:00