HttpClient wrapper for simple calls with optional cert
Clash Royale CLAN TAG#URR8PPP
.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;
up vote
1
down vote
favorite
Trying to ensure each controller contains it's own httpclient. The super user can change which endpoint is called from the site and some of those endpoints do not require a certificate, but some require their own certificate.
Want a safe call to this wrapper class (HttpClientProvider) as a static and in a static constructor for reuse. Any gotchas?
Will the httpclient still execute after not being used for a while?
Here is the Controller Call:
public class HomeController : Controller
private static readonly HttpClientProvider _clientProvider;
static HomeController()
_clientProvider = new HttpClientProvider();
public ActionResult Index()
var certPath = HostingEnvironment.MapPath("~/App_Data/cert.pfx");
var httpClient = _clientProvider.GetHttpClient(certPath);
var result = httpClient.GetAsync("www.example.com").Result;
return View();
Here is the Provider Class:
public class HttpClientProvider
private HttpClient _client;
private WebRequestHandler _clientHandler;
public HttpClient GetHttpClient()
SetUpHandler();
SetUpClient();
return _client;
public HttpClient GetHttpClient(string certPath, string password = "")
SetUpHandler();
SetUpClient();
SetUpCertificate(certPath, password);
return _client;
public HttpClient GetHttpClient(X509Certificate2 x509Cert)
SetUpHandler();
_clientHandler.ClientCertificates.Add(x509Cert);
SetUpClient();
return _client;
private void SetUpHandler()
if (_clientHandler == null)
_clientHandler = new WebRequestHandler();
else
_clientHandler.ClientCertificates.Clear();
private void SetUpCertificate(string certPath, string password = "")
if (certPath != null)
var absolutePath = HostingEnvironment.MapPath(certPath);
var cert = absolutePath != null
? new X509Certificate2(absolutePath, password, X509KeyStorageFlags.MachineKeySet)
: new X509Certificate2(certPath, password, X509KeyStorageFlags.MachineKeySet);
_clientHandler.ClientCertificates.Add(cert);
private void SetUpClient()
if (_client == null)
_client = new HttpClient(_clientHandler);
c# mvc thread-safety asp.net-mvc singleton
add a comment |Â
up vote
1
down vote
favorite
Trying to ensure each controller contains it's own httpclient. The super user can change which endpoint is called from the site and some of those endpoints do not require a certificate, but some require their own certificate.
Want a safe call to this wrapper class (HttpClientProvider) as a static and in a static constructor for reuse. Any gotchas?
Will the httpclient still execute after not being used for a while?
Here is the Controller Call:
public class HomeController : Controller
private static readonly HttpClientProvider _clientProvider;
static HomeController()
_clientProvider = new HttpClientProvider();
public ActionResult Index()
var certPath = HostingEnvironment.MapPath("~/App_Data/cert.pfx");
var httpClient = _clientProvider.GetHttpClient(certPath);
var result = httpClient.GetAsync("www.example.com").Result;
return View();
Here is the Provider Class:
public class HttpClientProvider
private HttpClient _client;
private WebRequestHandler _clientHandler;
public HttpClient GetHttpClient()
SetUpHandler();
SetUpClient();
return _client;
public HttpClient GetHttpClient(string certPath, string password = "")
SetUpHandler();
SetUpClient();
SetUpCertificate(certPath, password);
return _client;
public HttpClient GetHttpClient(X509Certificate2 x509Cert)
SetUpHandler();
_clientHandler.ClientCertificates.Add(x509Cert);
SetUpClient();
return _client;
private void SetUpHandler()
if (_clientHandler == null)
_clientHandler = new WebRequestHandler();
else
_clientHandler.ClientCertificates.Clear();
private void SetUpCertificate(string certPath, string password = "")
if (certPath != null)
var absolutePath = HostingEnvironment.MapPath(certPath);
var cert = absolutePath != null
? new X509Certificate2(absolutePath, password, X509KeyStorageFlags.MachineKeySet)
: new X509Certificate2(certPath, password, X509KeyStorageFlags.MachineKeySet);
_clientHandler.ClientCertificates.Add(cert);
private void SetUpClient()
if (_client == null)
_client = new HttpClient(_clientHandler);
c# mvc thread-safety asp.net-mvc singleton
client should have a backing abstraction and injected into controllers. clients should be kept based on domain being accessed. For example all calls tohttps://somehost
should be handled by the same client. That way if there needs to be any common headers set there would be no need to repeat them.
â Nkosi
Jan 13 at 2:17
so i should abstract away the client and inject that abstraction? can it be singleton per controller? would a key value pair work for selecting endpoint to httpclient in abstraction?
â asunrey
Jan 13 at 4:45
1
Take a look at this suggested answer on SO stackoverflow.com/a/48138819/5233410
â Nkosi
Jan 13 at 12:19
add a comment |Â
up vote
1
down vote
favorite
up vote
1
down vote
favorite
Trying to ensure each controller contains it's own httpclient. The super user can change which endpoint is called from the site and some of those endpoints do not require a certificate, but some require their own certificate.
Want a safe call to this wrapper class (HttpClientProvider) as a static and in a static constructor for reuse. Any gotchas?
Will the httpclient still execute after not being used for a while?
Here is the Controller Call:
public class HomeController : Controller
private static readonly HttpClientProvider _clientProvider;
static HomeController()
_clientProvider = new HttpClientProvider();
public ActionResult Index()
var certPath = HostingEnvironment.MapPath("~/App_Data/cert.pfx");
var httpClient = _clientProvider.GetHttpClient(certPath);
var result = httpClient.GetAsync("www.example.com").Result;
return View();
Here is the Provider Class:
public class HttpClientProvider
private HttpClient _client;
private WebRequestHandler _clientHandler;
public HttpClient GetHttpClient()
SetUpHandler();
SetUpClient();
return _client;
public HttpClient GetHttpClient(string certPath, string password = "")
SetUpHandler();
SetUpClient();
SetUpCertificate(certPath, password);
return _client;
public HttpClient GetHttpClient(X509Certificate2 x509Cert)
SetUpHandler();
_clientHandler.ClientCertificates.Add(x509Cert);
SetUpClient();
return _client;
private void SetUpHandler()
if (_clientHandler == null)
_clientHandler = new WebRequestHandler();
else
_clientHandler.ClientCertificates.Clear();
private void SetUpCertificate(string certPath, string password = "")
if (certPath != null)
var absolutePath = HostingEnvironment.MapPath(certPath);
var cert = absolutePath != null
? new X509Certificate2(absolutePath, password, X509KeyStorageFlags.MachineKeySet)
: new X509Certificate2(certPath, password, X509KeyStorageFlags.MachineKeySet);
_clientHandler.ClientCertificates.Add(cert);
private void SetUpClient()
if (_client == null)
_client = new HttpClient(_clientHandler);
c# mvc thread-safety asp.net-mvc singleton
Trying to ensure each controller contains it's own httpclient. The super user can change which endpoint is called from the site and some of those endpoints do not require a certificate, but some require their own certificate.
Want a safe call to this wrapper class (HttpClientProvider) as a static and in a static constructor for reuse. Any gotchas?
Will the httpclient still execute after not being used for a while?
Here is the Controller Call:
public class HomeController : Controller
private static readonly HttpClientProvider _clientProvider;
static HomeController()
_clientProvider = new HttpClientProvider();
public ActionResult Index()
var certPath = HostingEnvironment.MapPath("~/App_Data/cert.pfx");
var httpClient = _clientProvider.GetHttpClient(certPath);
var result = httpClient.GetAsync("www.example.com").Result;
return View();
Here is the Provider Class:
public class HttpClientProvider
private HttpClient _client;
private WebRequestHandler _clientHandler;
public HttpClient GetHttpClient()
SetUpHandler();
SetUpClient();
return _client;
public HttpClient GetHttpClient(string certPath, string password = "")
SetUpHandler();
SetUpClient();
SetUpCertificate(certPath, password);
return _client;
public HttpClient GetHttpClient(X509Certificate2 x509Cert)
SetUpHandler();
_clientHandler.ClientCertificates.Add(x509Cert);
SetUpClient();
return _client;
private void SetUpHandler()
if (_clientHandler == null)
_clientHandler = new WebRequestHandler();
else
_clientHandler.ClientCertificates.Clear();
private void SetUpCertificate(string certPath, string password = "")
if (certPath != null)
var absolutePath = HostingEnvironment.MapPath(certPath);
var cert = absolutePath != null
? new X509Certificate2(absolutePath, password, X509KeyStorageFlags.MachineKeySet)
: new X509Certificate2(certPath, password, X509KeyStorageFlags.MachineKeySet);
_clientHandler.ClientCertificates.Add(cert);
private void SetUpClient()
if (_client == null)
_client = new HttpClient(_clientHandler);
c# mvc thread-safety asp.net-mvc singleton
edited Jan 12 at 23:56
asked Jan 12 at 23:24
asunrey
1085
1085
client should have a backing abstraction and injected into controllers. clients should be kept based on domain being accessed. For example all calls tohttps://somehost
should be handled by the same client. That way if there needs to be any common headers set there would be no need to repeat them.
â Nkosi
Jan 13 at 2:17
so i should abstract away the client and inject that abstraction? can it be singleton per controller? would a key value pair work for selecting endpoint to httpclient in abstraction?
â asunrey
Jan 13 at 4:45
1
Take a look at this suggested answer on SO stackoverflow.com/a/48138819/5233410
â Nkosi
Jan 13 at 12:19
add a comment |Â
client should have a backing abstraction and injected into controllers. clients should be kept based on domain being accessed. For example all calls tohttps://somehost
should be handled by the same client. That way if there needs to be any common headers set there would be no need to repeat them.
â Nkosi
Jan 13 at 2:17
so i should abstract away the client and inject that abstraction? can it be singleton per controller? would a key value pair work for selecting endpoint to httpclient in abstraction?
â asunrey
Jan 13 at 4:45
1
Take a look at this suggested answer on SO stackoverflow.com/a/48138819/5233410
â Nkosi
Jan 13 at 12:19
client should have a backing abstraction and injected into controllers. clients should be kept based on domain being accessed. For example all calls to
https://somehost
should be handled by the same client. That way if there needs to be any common headers set there would be no need to repeat them.â Nkosi
Jan 13 at 2:17
client should have a backing abstraction and injected into controllers. clients should be kept based on domain being accessed. For example all calls to
https://somehost
should be handled by the same client. That way if there needs to be any common headers set there would be no need to repeat them.â Nkosi
Jan 13 at 2:17
so i should abstract away the client and inject that abstraction? can it be singleton per controller? would a key value pair work for selecting endpoint to httpclient in abstraction?
â asunrey
Jan 13 at 4:45
so i should abstract away the client and inject that abstraction? can it be singleton per controller? would a key value pair work for selecting endpoint to httpclient in abstraction?
â asunrey
Jan 13 at 4:45
1
1
Take a look at this suggested answer on SO stackoverflow.com/a/48138819/5233410
â Nkosi
Jan 13 at 12:19
Take a look at this suggested answer on SO stackoverflow.com/a/48138819/5233410
â Nkosi
Jan 13 at 12:19
add a comment |Â
1 Answer
1
active
oldest
votes
up vote
4
down vote
accepted
As is this does not work because multiple requests will concurrently operate on the same variables (instance variables and HttpClient internal state as it is being configured).
This is fairly easy to solve because there's a nice pattern for this. Your core problem is that you need to initialize one expensive object for each instance of a certain kind of key (here: HttpClient and certificate path).
I'm modifying some existing code to ensure that only one HttpClient per key is ever created:
public static class HttpClientManager
private static ConcurrentDictionary<string, Lazy<HttpClient>> _clients =
new ConcurrentDictionary<string, Lazy<HttpClient>>();
public static HttpClient Get(string certificatePath)
return _clients.GetOrAdd(certificatePath, _ =>
new Lazy<HttpClient>(CreateClient(certificatePath));
static HttpClient CreateClient(string certificatePath)
//TODO
You only need to fill in the TODO
.
This code is thread safe because the infrastructure takes care of only invoking your factory method once per unique certificatePath
. The factory will only use local state so that it is totally unconcerned with thread safety.
I'd avoid methods such as void SetUpXxx(...)
. These messods make use of side-effects for no reason. In particular, _clientHandler
seems to be used only as a way to thread this value through the various methods being called. It is often better to avoid passing data in such covert ways. Convert that field to a method parameter and explicitly pass that value around. I'd do the same thing for _client
itself. Then, you can keep the SetUpXxx
methods but they become static
. It will be explicit what data they consume and what they output.
This might seems like a subtle point but I found this kind of change helpful over and over again.
Thank you for the explanation. Did not know where to turn because I have little guidance and no mentors! So no need to use depency injection and inject the HttpManager class, just call it as a utility?
â asunrey
Jan 18 at 1:38
@asunrey Not sure I understand, but I'd pick the simplest solution that solves the problem. No need to overuse patterns. Maybe you had an OOP mindset in your solution. OOP has it's place but it's just another pattern like all others. OOP is not suitable as a framework for everything. Hope I understood your comment correctly and this helps.
â usr
Jan 23 at 12:05
add a comment |Â
1 Answer
1
active
oldest
votes
1 Answer
1
active
oldest
votes
active
oldest
votes
active
oldest
votes
up vote
4
down vote
accepted
As is this does not work because multiple requests will concurrently operate on the same variables (instance variables and HttpClient internal state as it is being configured).
This is fairly easy to solve because there's a nice pattern for this. Your core problem is that you need to initialize one expensive object for each instance of a certain kind of key (here: HttpClient and certificate path).
I'm modifying some existing code to ensure that only one HttpClient per key is ever created:
public static class HttpClientManager
private static ConcurrentDictionary<string, Lazy<HttpClient>> _clients =
new ConcurrentDictionary<string, Lazy<HttpClient>>();
public static HttpClient Get(string certificatePath)
return _clients.GetOrAdd(certificatePath, _ =>
new Lazy<HttpClient>(CreateClient(certificatePath));
static HttpClient CreateClient(string certificatePath)
//TODO
You only need to fill in the TODO
.
This code is thread safe because the infrastructure takes care of only invoking your factory method once per unique certificatePath
. The factory will only use local state so that it is totally unconcerned with thread safety.
I'd avoid methods such as void SetUpXxx(...)
. These messods make use of side-effects for no reason. In particular, _clientHandler
seems to be used only as a way to thread this value through the various methods being called. It is often better to avoid passing data in such covert ways. Convert that field to a method parameter and explicitly pass that value around. I'd do the same thing for _client
itself. Then, you can keep the SetUpXxx
methods but they become static
. It will be explicit what data they consume and what they output.
This might seems like a subtle point but I found this kind of change helpful over and over again.
Thank you for the explanation. Did not know where to turn because I have little guidance and no mentors! So no need to use depency injection and inject the HttpManager class, just call it as a utility?
â asunrey
Jan 18 at 1:38
@asunrey Not sure I understand, but I'd pick the simplest solution that solves the problem. No need to overuse patterns. Maybe you had an OOP mindset in your solution. OOP has it's place but it's just another pattern like all others. OOP is not suitable as a framework for everything. Hope I understood your comment correctly and this helps.
â usr
Jan 23 at 12:05
add a comment |Â
up vote
4
down vote
accepted
As is this does not work because multiple requests will concurrently operate on the same variables (instance variables and HttpClient internal state as it is being configured).
This is fairly easy to solve because there's a nice pattern for this. Your core problem is that you need to initialize one expensive object for each instance of a certain kind of key (here: HttpClient and certificate path).
I'm modifying some existing code to ensure that only one HttpClient per key is ever created:
public static class HttpClientManager
private static ConcurrentDictionary<string, Lazy<HttpClient>> _clients =
new ConcurrentDictionary<string, Lazy<HttpClient>>();
public static HttpClient Get(string certificatePath)
return _clients.GetOrAdd(certificatePath, _ =>
new Lazy<HttpClient>(CreateClient(certificatePath));
static HttpClient CreateClient(string certificatePath)
//TODO
You only need to fill in the TODO
.
This code is thread safe because the infrastructure takes care of only invoking your factory method once per unique certificatePath
. The factory will only use local state so that it is totally unconcerned with thread safety.
I'd avoid methods such as void SetUpXxx(...)
. These messods make use of side-effects for no reason. In particular, _clientHandler
seems to be used only as a way to thread this value through the various methods being called. It is often better to avoid passing data in such covert ways. Convert that field to a method parameter and explicitly pass that value around. I'd do the same thing for _client
itself. Then, you can keep the SetUpXxx
methods but they become static
. It will be explicit what data they consume and what they output.
This might seems like a subtle point but I found this kind of change helpful over and over again.
Thank you for the explanation. Did not know where to turn because I have little guidance and no mentors! So no need to use depency injection and inject the HttpManager class, just call it as a utility?
â asunrey
Jan 18 at 1:38
@asunrey Not sure I understand, but I'd pick the simplest solution that solves the problem. No need to overuse patterns. Maybe you had an OOP mindset in your solution. OOP has it's place but it's just another pattern like all others. OOP is not suitable as a framework for everything. Hope I understood your comment correctly and this helps.
â usr
Jan 23 at 12:05
add a comment |Â
up vote
4
down vote
accepted
up vote
4
down vote
accepted
As is this does not work because multiple requests will concurrently operate on the same variables (instance variables and HttpClient internal state as it is being configured).
This is fairly easy to solve because there's a nice pattern for this. Your core problem is that you need to initialize one expensive object for each instance of a certain kind of key (here: HttpClient and certificate path).
I'm modifying some existing code to ensure that only one HttpClient per key is ever created:
public static class HttpClientManager
private static ConcurrentDictionary<string, Lazy<HttpClient>> _clients =
new ConcurrentDictionary<string, Lazy<HttpClient>>();
public static HttpClient Get(string certificatePath)
return _clients.GetOrAdd(certificatePath, _ =>
new Lazy<HttpClient>(CreateClient(certificatePath));
static HttpClient CreateClient(string certificatePath)
//TODO
You only need to fill in the TODO
.
This code is thread safe because the infrastructure takes care of only invoking your factory method once per unique certificatePath
. The factory will only use local state so that it is totally unconcerned with thread safety.
I'd avoid methods such as void SetUpXxx(...)
. These messods make use of side-effects for no reason. In particular, _clientHandler
seems to be used only as a way to thread this value through the various methods being called. It is often better to avoid passing data in such covert ways. Convert that field to a method parameter and explicitly pass that value around. I'd do the same thing for _client
itself. Then, you can keep the SetUpXxx
methods but they become static
. It will be explicit what data they consume and what they output.
This might seems like a subtle point but I found this kind of change helpful over and over again.
As is this does not work because multiple requests will concurrently operate on the same variables (instance variables and HttpClient internal state as it is being configured).
This is fairly easy to solve because there's a nice pattern for this. Your core problem is that you need to initialize one expensive object for each instance of a certain kind of key (here: HttpClient and certificate path).
I'm modifying some existing code to ensure that only one HttpClient per key is ever created:
public static class HttpClientManager
private static ConcurrentDictionary<string, Lazy<HttpClient>> _clients =
new ConcurrentDictionary<string, Lazy<HttpClient>>();
public static HttpClient Get(string certificatePath)
return _clients.GetOrAdd(certificatePath, _ =>
new Lazy<HttpClient>(CreateClient(certificatePath));
static HttpClient CreateClient(string certificatePath)
//TODO
You only need to fill in the TODO
.
This code is thread safe because the infrastructure takes care of only invoking your factory method once per unique certificatePath
. The factory will only use local state so that it is totally unconcerned with thread safety.
I'd avoid methods such as void SetUpXxx(...)
. These messods make use of side-effects for no reason. In particular, _clientHandler
seems to be used only as a way to thread this value through the various methods being called. It is often better to avoid passing data in such covert ways. Convert that field to a method parameter and explicitly pass that value around. I'd do the same thing for _client
itself. Then, you can keep the SetUpXxx
methods but they become static
. It will be explicit what data they consume and what they output.
This might seems like a subtle point but I found this kind of change helpful over and over again.
answered Jan 15 at 12:11
usr
615310
615310
Thank you for the explanation. Did not know where to turn because I have little guidance and no mentors! So no need to use depency injection and inject the HttpManager class, just call it as a utility?
â asunrey
Jan 18 at 1:38
@asunrey Not sure I understand, but I'd pick the simplest solution that solves the problem. No need to overuse patterns. Maybe you had an OOP mindset in your solution. OOP has it's place but it's just another pattern like all others. OOP is not suitable as a framework for everything. Hope I understood your comment correctly and this helps.
â usr
Jan 23 at 12:05
add a comment |Â
Thank you for the explanation. Did not know where to turn because I have little guidance and no mentors! So no need to use depency injection and inject the HttpManager class, just call it as a utility?
â asunrey
Jan 18 at 1:38
@asunrey Not sure I understand, but I'd pick the simplest solution that solves the problem. No need to overuse patterns. Maybe you had an OOP mindset in your solution. OOP has it's place but it's just another pattern like all others. OOP is not suitable as a framework for everything. Hope I understood your comment correctly and this helps.
â usr
Jan 23 at 12:05
Thank you for the explanation. Did not know where to turn because I have little guidance and no mentors! So no need to use depency injection and inject the HttpManager class, just call it as a utility?
â asunrey
Jan 18 at 1:38
Thank you for the explanation. Did not know where to turn because I have little guidance and no mentors! So no need to use depency injection and inject the HttpManager class, just call it as a utility?
â asunrey
Jan 18 at 1:38
@asunrey Not sure I understand, but I'd pick the simplest solution that solves the problem. No need to overuse patterns. Maybe you had an OOP mindset in your solution. OOP has it's place but it's just another pattern like all others. OOP is not suitable as a framework for everything. Hope I understood your comment correctly and this helps.
â usr
Jan 23 at 12:05
@asunrey Not sure I understand, but I'd pick the simplest solution that solves the problem. No need to overuse patterns. Maybe you had an OOP mindset in your solution. OOP has it's place but it's just another pattern like all others. OOP is not suitable as a framework for everything. Hope I understood your comment correctly and this helps.
â usr
Jan 23 at 12:05
add a comment |Â
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f184999%2fhttpclient-wrapper-for-simple-calls-with-optional-cert%23new-answer', 'question_page');
);
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
client should have a backing abstraction and injected into controllers. clients should be kept based on domain being accessed. For example all calls to
https://somehost
should be handled by the same client. That way if there needs to be any common headers set there would be no need to repeat them.â Nkosi
Jan 13 at 2:17
so i should abstract away the client and inject that abstraction? can it be singleton per controller? would a key value pair work for selecting endpoint to httpclient in abstraction?
â asunrey
Jan 13 at 4:45
1
Take a look at this suggested answer on SO stackoverflow.com/a/48138819/5233410
â Nkosi
Jan 13 at 12:19