HttpClient wrapper for simple calls with optional cert

The name of the pictureThe name of the pictureThe name of the pictureClash Royale CLAN TAG#URR8PPP





.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;







up vote
1
down vote

favorite
2












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);









share|improve this question





















  • 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
















up vote
1
down vote

favorite
2












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);









share|improve this question





















  • 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












up vote
1
down vote

favorite
2









up vote
1
down vote

favorite
2






2





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);









share|improve this question













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);











share|improve this question












share|improve this question




share|improve this question








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 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
















  • 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















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










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.






share|improve this answer





















  • 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










Your Answer




StackExchange.ifUsing("editor", function ()
return StackExchange.using("mathjaxEditing", function ()
StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix)
StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
);
);
, "mathjax-editing");

StackExchange.ifUsing("editor", function ()
StackExchange.using("externalEditor", function ()
StackExchange.using("snippets", function ()
StackExchange.snippets.init();
);
);
, "code-snippets");

StackExchange.ready(function()
var channelOptions =
tags: "".split(" "),
id: "196"
;
initTagRenderer("".split(" "), "".split(" "), channelOptions);

StackExchange.using("externalEditor", function()
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled)
StackExchange.using("snippets", function()
createEditor();
);

else
createEditor();

);

function createEditor()
StackExchange.prepareEditor(
heartbeatType: 'answer',
convertImagesToLinks: false,
noModals: false,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
);



);








 

draft saved


draft discarded


















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






























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.






share|improve this answer





















  • 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














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.






share|improve this answer





















  • 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












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.






share|improve this answer













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.







share|improve this answer













share|improve this answer



share|improve this answer











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
















  • 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












 

draft saved


draft discarded


























 


draft saved


draft discarded














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













































































Popular posts from this blog

Chat program with C++ and SFML

Function to Return a JSON Like Objects Using VBA Collections and Arrays

Will my employers contract hold up in court?