Pass Through Web API calls

Question

Our application architecture consists of:

• SPA JavaScript calls a set of Web API endpoints
• The Controllers then call an Entity specific Service
• Entity specific Service calls a Pass Through Service
• Pass Through Service calls an API Service that calls external APIs using the HttpClient library via a wrapper.
• The response is "passed through" to the originating client

Controller:

[RoutePrefix(WebApiConfig.UrlPrefix)]
public class EntityController : ApiController
{
    [Route("entity/next")]
    public async Task<IHttpActionResult> PostPassthru(HttpRequestMessage requestMessage)
    {
        var entityApiService = new EntityApiService>();
        HttpResponseMessage response;
        string pathQuery;

        pathQuery = Common.StripPrefix(WebApiConfig.UrlPrefix, requestMessage.RequestUri.PathAndQuery, true);
        response = await EntityApiService.CallApiAsync(pathQuery, HttpMethod.Post, requestMessage.Content, requestMessage.Headers);

        return ResponseMessage(response);
    }
}

EntityApiService:

public class EntityApiService : IEntityApiService
{
    PassThroughApiService deafultApiService;

    public string API_NAME
    {
        get { return ApiNames.Entity; }
    }

    public EntityApiService()
    {
        deafultApiService = new PassThroughApiService(API_NAME);
    }

    public Task<HttpResponseMessage> GetAsync(string pathAndQuery, HttpRequestHeaders requestHeaders)
    {
        return deafultApiService.GetAsync(pathAndQuery, requestHeaders);
    }


    public async Task<HttpResponseMessage> CallApiAsync(string pathAndQuery, HttpMethod method, HttpContent content, HttpRequestHeaders requestHeaders, bool addJsonMimeRequest = true)
    {
        return await deafultApiService.CallApiAsync(pathAndQuery.ApplyPrefix(ApiPrefix), method, content, requestHeaders, addJsonMimeRequest);
    }
}

PassThroughApiService:

public class PassThroughApiService : IApiService
{
    private string _apiName;

    public string API_NAME { get { return _apiName; } }

    public PassThroughApiService(string apiName)
    {
        this._apiName = apiName;
    }

    public async Task<HttpResponseMessage> CallApiAsync(string pathAndQuery, HttpMethod method, HttpContent content, HttpRequestHeaders requestHeaders, bool addJsonMimeRequest = true)
    {
        HttpResponseMessage response;

        if (content != null && addJsonMimeRequest)
            content.Headers.ContentType = new MediaTypeWithQualityHeaderValue("application/json");

        var apiService = New ApiService();

        response = await apiService.CallApiAsync(API_NAME, pathAndQuery, method, content, requestHeaders, true);


        return response;
    }

    public Task<HttpResponseMessage> GetAsync(string pathAndQuery, HttpRequestHeaders requestHeaders)
    {
        return this.CallApiAsync(pathAndQuery, HttpMethod.Get, null, requestHeaders);
    }
}

---

ApiService:

public class ApiService : IApiService
{
    // Do not use a USING statement and do not call Dispose, see wrapper comments for details
    private static IHttpClient _httpClient;

    static ApiService()
    {
        _httpClient = new HttpClientWrapper();
    }

    public Uri BuildUri(IApiMap apiMap, string pathAndQuery)
    {
        Uri uri;
        Uri baseUri;

        try
        {
            baseUri = new Uri(apiMap.ApiUri);
        }
        catch (Exception ex)
        {
            throw new Exception("Base URI mapped from API MAP was invalid.", ex);
        }

        try
        {
            uri = new Uri(baseUri, pathAndQuery);
        }
        catch (Exception ex)
        {
            throw new Exception("PathAndQuery value was invalid.", ex);
        }

        return uri;
    }

    public Task<HttpResponseMessage> CallApiAsync(string apiName, string pathAndQuery, HttpMethod method, HttpContent content, HttpRequestHeaders requestHeaders, bool addJsonMimeAccept = true)
    {
        ApiMapService apiMapService = new ApiMapService();
        IApiMap apiMap;
        Uri uri;

        if (String.IsNullOrWhiteSpace(apiName))
            throw new ArgumentNullException("apiName", "ApiName cannot be null or empty.");

        if (String.IsNullOrWhiteSpace(pathAndQuery))
            throw new ArgumentNullException("pathAndQuery", "PathAndQuery cannot be null.");

        if (method == null)
            throw new ArgumentNullException("method", "Method cannot be null.");

        apiMap = apiMapService.GetByName(apiName);

        if (apiMap == null)
            throw new Exception("The ApiName value provided did not resolve to a record.");

        uri = BuildUri(apiMap, pathAndQuery);

        return CallApiAsync(uri, method, content, requestHeaders, addJsonMimeAccept);
    }

    public Task<HttpResponseMessage> CallApiAsync(Uri uri, HttpMethod method, HttpContent content, HttpRequestHeaders requestHeaders, bool addJsonMimeAccept = true)
    {
        HttpRequestMessage request;

        if (uri == null)
            throw new ArgumentNullException("uri");

        if (method == null)
            throw new ArgumentNullException("method");

        request = new HttpRequestMessage(method, uri.AbsoluteUri);

        if (addJsonMimeAccept)
            request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));

        if (content != null)
            request.Content = content;

        if (requestHeaders != null)
        {
            foreach (var item in requestHeaders.Where(header => header.Key.Contains(HeaderPrefix)))
            {
                request.Headers.Add(item.Key.Replace(HeaderPrefix, ""), item.Value);
            }
        }

        return _httpClient.SendAsync(request);
    }
}

HttpClientWrapper

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)
    {
        return _client.SendAsync(request);
    }

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

Any suggestions to improve?

Answer 1

A small contribution, may I suggest a couple improvements here?

public class EntityApiService : IEntityApiService
{
    PassThroughApiService deafultApiService;

    public string API_NAME
    {
        get { return ApiNames.Entity; }
    }

First, would be to replace PassThroughApiService with the IApiService along with maybe a spelling correction of deafultApiService to defaultApiService or better yet, _apiService.

Second, would be to use Pascal Case for your properties, so that means API_NAME would be ApiName

The improvements applied to the above code block would be:

public class EntityApiService : IEntityApiService
{
    private IApiService _apiService;

    public string ApiName
    {
        get { return ApiNames.Entity; }
    }

---

I also see that you could make some improvements here:

if (String.IsNullOrWhiteSpace(apiName))
    throw new ArgumentNullException("apiName", "ApiName cannot be null or empty.");

        if (String.IsNullOrWhiteSpace(pathAndQuery))
    throw new ArgumentNullException("pathAndQuery", "PathAndQuery cannot be null.");

        if (method == null)
    throw new ArgumentNullException("method", "Method cannot be null.");

You should use nameof() instead of manually putting in the parameter name, as this will make for easier refactoring and maintainability in the future. In addition, I feel that it's redundant to place the message of repeating "{var} cannot be null." when the Exception type is already obvious.

The improvement would be:

if (String.IsNullOrWhiteSpace(apiName))
    throw new ArgumentNullException(nameof(apiName));
if (String.IsNullOrWhiteSpace(pathAndQuery)) 
    throw new ArgumentNullException(nameof(pathAndQuery));
if (method == null)
    throw new ArgumentNullException(nameof(method));

Answer 2

    var entityApiService = new EntityApiService>();
    HttpResponseMessage response;
    string pathQuery;

    pathQuery = Common.StripPrefix(WebApiConfig.UrlPrefix, requestMessage.RequestUri.PathAndQuery, true);
    response = await EntityApiService.CallApiAsync(pathQuery, HttpMethod.Post, requestMessage.Content, requestMessage.Headers);

The usage of var or full type name isn't consistand in your code. I suggest picking one and stick to it. You also don't need to declare the variables first and then assign values to them. Why not do everything in one line? This also lacks consistency because the first line does that and the other two not.

---

Common.StripPrefix

Common is not a good name for a class and it probably contains many unrelated methods. Try to group them in classes that have more concrete names like here the name Url would be great or even UrlHelper but at least something more specific. If I see Common.StripPrefix I think: StripPrefix from what? however Url.StripPrefix clearly suggests what's going on.

---

PassThroughApiService deafultApiService;

It's a good idea to declare fields like this that do/should not change as readonly.

---

public class ApiService : IApiService

I'm not fond of this class name because it's virtually the same as the interface which is still very abstract. This means it should either provide a default implementation for an IApiService and be an abstract class or if it is something concrete that its name should reflect this like you did with the EntityController derived from the ApiController where you replaced the API with Entity.