ActiveCampaign
Non-static client factory
Microsoft advises that we should IHttpClientFactory to implement resilient HTTP requests (see docs). For this to happen, the HttpClient needs to be provided when creating instances of PostmarkClient or PostmarkAdminClient.
In this PR, the static client factory is replaced with an optional ISimpleHttpClient parameter. This works for scenarios where the injection of HttpClient instances is (and is not) preferred.
Hey @mburumaxwell sorry for the delay -- we'll take a look. Vlad no longer works at Wildbit.
@mburumaxwell is there a specific problem the current interface/design is causing for you that you are trying to solve with this PR?
While it is true that HttpClient should be re-used, and it is also true that MS docs currently suggest creating clients from IHttpClientFactory, I'm not sure this change is worth the risk imposed, and the testing effort that would be required.
@jchoca the intention is to inject the client using instances form IHttpClientFactory which brings a couple of benefits, including logging. I just thought it would be better to do a different PR for the dependency injection scenario.
@mburumaxwell - ClientFactory is already a static property, you are free to change it if you'd like to provide a different factory method for creating the client.
Unfortunately, that doesn't work well with a dependency injection.
It requires a lot of effort to setup get services.AddHttpClient<PostmarkClient>(...) working with it.
Fair enough, I understand your concern. However, conformance to a DI mechanism is not a strong enough reason to make this change.
@Archanian could you explain how you arrived at the decision for "not a strong enough reason"? For clarity purposes, what would break or not work as expected?