C# REST / Http Client Abstraction

Question

I have a REST library. These are the abstractions. Have I missed anything that a developer would need to use this in a dependency injection / IoC Container scenario? Anything that bothers you about this?

All Code Here

public interface IRestClient
{
 IRestHeadersCollection DefaultRequestHeaders { get; }
 TimeSpan Timeout { get; set; }
 Task DeleteAsync(Uri queryString);
 Task DeleteAsync(Uri queryString, CancellationToken cancellationToken);
 Task<RestResponse<TReturn>> GetAsync<TReturn>();
 Task<RestResponse<TReturn>> GetAsync<TReturn>(Uri queryString);
 Task<RestResponse<TReturn>> GetAsync<TReturn>(Uri queryString, CancellationToken cancellationToken);
 Task<RestResponse<TReturn>> PatchAsync<TReturn, TBody>(TBody body, Uri queryString);
 Task<RestResponse<TReturn>> PatchAsync<TReturn, TBody>(TBody body, Uri queryString, CancellationToken cancellationToken);
 Task<RestResponse<TReturn>> PostAsync<TReturn, TBody>(TBody body, Uri queryString);
 Task<RestResponse<TReturn>> PostAsync<TReturn, TBody>(TBody body, Uri queryString, CancellationToken cancellationToken);
 Task<RestResponse<TReturn>> PutAsync<TReturn, TBody>(TBody body, Uri queryString);
 Task<RestResponse<TReturn>> PutAsync<TReturn, TBody>(TBody body, Uri queryString, CancellationToken cancellationToken);
}
public interface IRestClientFactory
{
 IRestClient CreateRestClient(Uri baseUri);
 IRestClient CreateRestClient();
}
public interface IRestHeadersCollection : IEnumerable<KeyValuePair<string, IEnumerable<string>>>
{
 void Add(string name, string value);
 void Add(string name, IEnumerable<string> values);
 void Clear();
 IEnumerable<string> this[string name]
 {
 get;
 }
 bool Contains(string name);
}
public partial interface ISerializationAdapter
{
 /// <summary>
 /// Takes an object of Type T and converts it to binary data
 /// </summary>
 Task<byte[]> SerializeAsync<T>(T value);
 /// <summary>
 /// Takes binary data and converts it to an object of type T
 /// </summary>
 Task<T> DeserializeAsync<T>(byte[] data);
}
public interface ITracer
{
 void Trace(HttpVerb httpVerb, Uri baseUri, Uri queryString, byte[] body, TraceType traceType, int? httpStatusCode, IRestHeadersCollection restHeadersCollection);
}
public class RestResponse<TBody> : RestResponse
{
 public TBody Body { get; }
 public RestResponse(TBody body, IRestHeadersCollection restHeadersCollection, int statusCode, object underlyingResponse) : base(restHeadersCollection, statusCode, underlyingResponse)
 {
 Body = body;
 }
 #region Implicit Operator
 public static implicit operator TBody(RestResponse<TBody> readResult)
 {
 return readResult.Body;
 }
 public TBody ToTBody()
 {
 return Body;
 }
 #endregion
}
public class RestResponse
{
 #region Public Properties
 public int StatusCode { get; }
 public object UnderlyingResponse { get; }
 public IRestHeadersCollection Headers { get; }
 #endregion
 #region Constructor
 public RestResponse(IRestHeadersCollection restHeadersCollection, int statusCode, object underlyingResponse)
 {
 StatusCode = statusCode;
 UnderlyingResponse = underlyingResponse;
 Headers = restHeadersCollection;
 }
 #endregion
}

Originally I thought it might be useful to allow task async calls for serialization/deserialization but I've come to think that this is a bit redundant... Any thoughts on this?

• Nkosi: If already coupling to System.Net for HttpStatusCode, why not use HttpMethod instead of custom HttpVerb enum? If truly trying to abstract away then why not int? for status code?
• Nkosi: Looking at the client implementation you create a new instance of HttpClient if one is not provided. This can lead to socket exhaustion.
• Christian Findlay: @Nkosi nice find. Fixed your first point. Not sure I know what you mean by socket exhaustion... I just did a massive refactor and now the HttpClient has been changed to a HttpClientFactory so it can be implemented to create a new HttpClient in each case. However, the default factory is this and it probably suffers from the problem you mentioned: github.com/MelbourneDeveloper/RestClient.Net/blob/develop/…

Answer 1

IRestClient looks over-engineered. All of those helper members merge down to one common Call which can make the interface much simpler to define.

public interface IRestClient {
 IRestHeadersCollection DefaultRequestHeaders { get; }
 string DefaultContentType { get; set; }
 TimeSpan Timeout { get; set; }
 Task<RestResponse<TReturn>> SendAsync<TReturn>(RestRequest request, CancellationToken cancellationToken = default(CancellationToken));
}

The client now simplified to its core features.

The commonly used parameters can be aggregated into a class

public class RestRequest {
 public RestRequest() {
 }
 public RestRequest(IRestClient client) : this() {
 var contentType = client.DefaultContentType;
 if (!string.IsNullOrEmpty(contentType)) {
 ContentType = contentType;
 }
 var headers = client.DefaultRequestHeaders;
 if (headers != null) {
 foreach (var header in headers) {
 Headers.Add(header.Key, header.Value);
 }
 }
 }
 public IRestHeadersCollection Headers { get; } = new RestRequestHeaders();
 public Uri Resource { get; set; }
 public HttpVerb HttpVerb { get; set; } = HttpVerb.Get;
 public string ContentType { get; set; } = "application/json";
 public object Body { get; set; }
 class RestRequestHeaders : HttpHeaders, IRestHeadersCollection {
 public RestRequestHeaders() : base() { }
 public IEnumerable<string> this[string name] => GetValues(name) ?? Array.Empty<string>();
 }
}

Naming is important here. Change the name to resource from queryString, which means something different to how it was originally being used with regards to HTTP requests.

Parameter order should also be changed to be more consistent.

Your original call would have looked something like

...await client.PostAsync<ReturnObject, BodyObject>(body, "...");

with the URI parameter second. When making the request, the first argument should be the resource being called. Everything else can be built up from there

...await client.PostAsync<ReturnObject, BodyObject>("...", body);
...await client.PostAsync<ReturnObject, BodyObject>("...", body, token);
//...etc

The following extensions allow for the same functionality as before on the IRestClient abstraction

public static class RestClientExtensions {
 #region Delete
 public static Task DeleteAsync(this IRestClient client, string resource) {
 return client.DeleteAsync(new Uri(resource, UriKind.Relative));
 }
 public static Task DeleteAsync(this IRestClient client, Uri resource) {
 return client.DeleteAsync(resource, CancellationToken.None);
 }
 public static Task DeleteAsync(this IRestClient client, Uri resource, CancellationToken cancellationToken) {
 var request = new RestRequest(client) {
 Resource = resource,
 HttpVerb = HttpVerb.Delete
 };
 return client.SendAsync<object>(request, cancellationToken);
 }
 #endregion
 #region Get
 public static Task<RestResponse<TReturn>> GetAsync<TReturn>(this IRestClient client) {
 var request = new RestRequest(client) {
 HttpVerb = HttpVerb.Get
 };
 return client.SendAsync<TReturn>(request, CancellationToken.None);
 }
 public static Task<RestResponse<TResult>> GetAsync<TResult>(this IRestClient client, string resource) {
 try {
 return client.GetAsync<TResult>(new Uri(resource, UriKind.Relative));
 } catch (UriFormatException ufe) {
 if (ufe.Message == "A relative URI cannot be created because the 'uriString' parameter represents an absolute URI.") {
 throw new UriFormatException(Messages.ErrorMessageAbsoluteUriAsString, ufe);
 }
 throw;
 }
 }
 public static Task<RestResponse<TReturn>> GetAsync<TReturn>(this IRestClient client, Uri resource) {
 return client.GetAsync<TReturn>(resource, CancellationToken.None);
 }
 public static Task<RestResponse<TReturn>> GetAsync<TReturn>(this IRestClient client, Uri resource, CancellationToken cancellationToken) {
 var request = new RestRequest(client) {
 Resource = resource
 };
 return client.SendAsync<TReturn>(request, cancellationToken);
 }
 #endregion
 #region Patch
 public static Task<RestResponse<TReturn>> PatchAsync<TReturn, TBody>(this IRestClient client, Uri resource, TBody body) {
 return client.PatchAsync<TReturn, TBody>(resource, body, CancellationToken.None);
 }
 public static Task<RestResponse<TReturn>> PatchAsync<TReturn, TBody>(this IRestClient client, Uri resource, TBody body, CancellationToken cancellationToken) {
 var request = new RestRequest(client) {
 Resource = resource,
 HttpVerb = HttpVerb.Patch,
 Body = body,
 };
 return client.SendAsync<TReturn>(request, cancellationToken);
 }
 #endregion
 #region Post
 public static Task<RestResponse<TReturn>> PostAsync<TReturn, TBody>(this IRestClient client, TBody body, Uri querresourceString) {
 return client.PostAsync<TReturn, TBody>(body, querresourceString, CancellationToken.None);
 }
 public static Task<RestResponse<TReturn>> PostAsync<TReturn, TBody>(this IRestClient client, TBody body, Uri resource, CancellationToken cancellationToken) {
 var request = new RestRequest(client) {
 Resource = resource,
 HttpVerb = HttpVerb.Post,
 Body = body,
 };
 return client.SendAsync<TReturn>(request, cancellationToken);
 }
 #endregion
 #region Put
 public static Task<RestResponse<TReturn>> PutAsync<TReturn, TBody>(this IRestClient client, TBody body, Uri resource) {
 return client.PutAsync<TReturn, TBody>(body, resource, CancellationToken.None);
 }
 public static Task<RestResponse<TReturn>> PutAsync<TReturn, TBody>(this IRestClient client, TBody body, Uri resource, CancellationToken cancellationToken) {
 var request = new RestRequest(client) {
 Resource = resource,
 HttpVerb = HttpVerb.Put,
 Body = body,
 };
 return client.SendAsync<TReturn>(request, cancellationToken);
 }
 #endregion
}

• Christian Findlay: Wow. Thanks for the this detailed answer. I can definitely see the value here. I don't think I'm going to be able implement this for the V3, but I will digest this over time and see if it fits for V4.
• Christian Findlay: I think I might be able to take some of your naming advice for this version though.
• Nkosi: @MelbourneDeveloper yeah, the queryString variable name will cause confusion for those accustom to ?key=value representing query strings in a URI
• Christian Findlay: How about serialization? Do you think there's any point in making it task based?
• Nkosi: @MelbourneDeveloper my advice would be to experiment. Run some benchmarks for the synchronous and asynchronous and go with the better performing one. If the serialization is already wrapped in an async call it wont make much of a difference if the underlying serialization is actually synchronous.