2
\$\begingroup\$

I'm using the following as a wrapper for the HtptClient because it doesn't implement an interface -- which makes mocking more difficult. This logic also supports invalid and self-signed certs.

public class HttpClientWrapper : IHttpClient
{
    private readonly HttpClient _client;

    public Uri BaseAddress
    {
        get
        {
            return _client.BaseAddress;
        }

        set
        {
            _client.BaseAddress = value;
        }
    }

    public HttpRequestHeaders DefaultRequestHeaders
    {
        get
        {
            return _client.DefaultRequestHeaders;
        }
    }

    public HttpClientWrapper()
    {
        bool ignoreCertificateErrors = ConfigurationManager.AppSettings["IgnoreCertificateErrors"].ToBool();
        string environmentName = ConfigurationManager.AppSettings["EnvironmentName"];

        _client = new HttpClient();

        // If ignoreCertificateErrors config value is true, 
        // the setting ignores all Certificate validation errors in AppDomain, 
        // in every environment but PRODUCTION. 
        if (ignoreCertErrors && environmentName.ToUpper() != "PRODUCTION")
        {
            ServicePointManager.ServerCertificateValidationCallback = delegate { return true; };
        }
    }

    public Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, String userOrProcessName)
    {
        IUnityContainer container = UnityCommon.GetContainer();
        ILogService logService = container.Resolve<ILogService>();

        logService.Log(ApplicationLogTypes.Debug, JsonConvert.SerializeObject(request), userOrProcessName);

        return _client.SendAsync(request);
    }

    #region IDisposable Support
    private bool disposedValue = false; // To detect redundant calls

    protected virtual void Dispose(bool disposing)
    {
        if (!disposedValue)
        {
            if (disposing && _client != null)
            {
                _client.Dispose();
            }

            disposedValue = true;
        }
    }

    public void Dispose()
    {
        Dispose(true);
    }
    #endregion
}

using the following for interface:

public interface IHttpClient : IDisposable
{
    Uri BaseAddress { get; set; }

    HttpRequestHeaders DefaultRequestHeaders { get; }

    Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, String userOrProcessName);
}

Any suggestions?

\$\endgroup\$
0

1 Answer 1

1
\$\begingroup\$
bool ignoreCertificateErrors = ConfigurationManager.AppSettings["IgnoreCertificateErrors"].ToBool();
string environmentName = ConfigurationManager.AppSettings["EnvironmentName"];

These two lines I think shouldn't be there because with them, the wrapper is tigthly coupled to the app.config.

I see three possibilities how you can decouple the wrapper:

  • make both values constructor parameters if they shouldn't change
  • make them properties if they can
  • use the UnityCommon service locator that you use in the SendAsync method and let it resolve some ConfigProvider from which you can get both settings.

environmentName.ToUpper() != "PRODUCTION"

Usually in situations like this it's safer to use a case insensitive comparison

environmentName.Equals("PRODUCTION", StringComparison.OrdinalIgnoreCase)
\$\endgroup\$
4
  • \$\begingroup\$ better use string.Equals("PRODUCTION", environmentName, StringComparison.OrdinalIgnoreCase) to avoid NullPointerExceptions... \$\endgroup\$
    – Bernhard
    Commented Sep 10, 2018 at 13:51
  • \$\begingroup\$ @Bernhard In some cases yes but here the config value should be validated before it's used so I assume it's not null. \$\endgroup\$
    – t3chb0t
    Commented Sep 10, 2018 at 14:01
  • \$\begingroup\$ how do you exactly validate your null? you use null.ToUpper() or null.Equals() ?? \$\endgroup\$
    – Bernhard
    Commented Sep 25, 2018 at 13:35
  • \$\begingroup\$ @Bernhard there are many ways to do this but I think the simplest one is to use the exception expression like ConfigurationManager.AppSettings[key] ?? throw new .... \$\endgroup\$
    – t3chb0t
    Commented Sep 25, 2018 at 14:06

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.