Wrapping IHttpActionResult - Generic solution

Question

I'd like to wrap IHttpActionResult because I need some extra data to be consumed by the client app.

My first approach was to create and return simple DTO, wrapping result object if succeeded:

Response DTO:

 public class Response<T>
{
    public string ErrorMessage { get; set; }
    public bool Success { get; set; }
    public string CodeStatus { get; set; }
    public T Result { get; set; }

    public Response(bool isSuccess, [Optional] T result, [Optional] string codeStatus, [Optional] string errorMessage)
    {
        Success = isSuccess;
        Result = result;
        CodeStatus = codeStatus;
        ErrorMessage = errorMessage;
    }
}

Controller:

    public IHttpActionResult Get(int id)
    {
        return BadRequest(new Response<MyObjectClass>(false, null,"Invalid Id",400));
        ...
        return Ok(new Response<MyObjectClass>(true, result);

    }

I've found it very ineffective way to deal with wrapping. I dont find it very elegant way. I've tried to figured out some generic solution and ended up with the following:

Example Controller Action:

    public IHttpActionResult GetById(int id)
    {
        var result = _someService.Get(id);

        if (result == null)
            return NotFound().WithError("Invalid Id");

        return Ok().WithSuccess(result);
    }

This still returns Response DTO.

I've wrapped IHttpActionResult to deal with creating Response DTO:

public class HttpActionResult : IHttpActionResult
{
    private readonly string _errorMessage;
    private readonly IHttpActionResult _innerResult;
    private readonly object _result;
    private readonly bool _isSuccess;

    public HttpActionResult(IHttpActionResult inner, bool isSuccess, object result,string errorMessage)
    {
        _errorMessage = errorMessage;
        _innerResult = inner;
        _result = result;
        _isSuccess = isSuccess;            
    }


    public async Task<HttpResponseMessage> ExecuteAsync(CancellationToken cancellationToken)
    {
        HttpResponseMessage response = await _innerResult.ExecuteAsync(cancellationToken);
        response.Content = new ObjectContent(typeof(Response), new Response(_isSuccess, _result, ((int)response.StatusCode).ToString(), _errorMessage), new JsonMediaTypeFormatter());
        return response;
    }
}

Finally I've added extension methods to IHttpActionResult to easier use in controller:

public static class IHttpActionResultExtensions
    {
        public static IHttpActionResult WithSuccess(this IHttpActionResult inner, object result = null, string message = null)
        {
            return new HttpActionResult(inner, true, result, message);
        }

        public static IHttpActionResult WithError(this IHttpActionResult inner,  string message = null)
        {
            return new HttpActionResult(inner, false,null, message);
        }
    }

What are the alternatives to deal with wrapping http messages in API Controller? What weak points do you see in my solution?

Answer 1

Avoid IHttpActionResult and use HttpResponseException with Business Entity as result type. As in your solution, you cannot write statically typed test cases.

For example,

protected void ThrowHttpError(HttpStatusCode statusCode, string message) 
{
     throw new HttpResponseException(
         new HttpResponseMessage(statusCode) {
            ReasonPhrase = message,
            // HTTP 2.0 ignores ReasonPhrase
            // so we send ReasonPhrase again in the Content
            Content = new StringContent(message)
     });
}


// some generic option...
protected void ThrowHttpError<T>(HttpStatusCode statusCode, T content) 
    where T:class
{
     throw new HttpResponseException(
         new HttpResponseMessage(statusCode) {
            ReasonPhrase = "Error",
            Content = JsonConvert.Serialize(content)
     });
}

Your methods,

public async Task<Product> Get(long id){

    var product = await context.Products
       .FirstOrDefaultAsync( x=> x.ProductID == id);

    if(product==null){
        ThrowHttpError(HttpStatusCode.NotFound, 
           $"Product not found for {id}");
    }

    if(product.RequiresValidation){

        // generic version....

        ThrowHttpError(HttpStatusCode.Conflict,
           new Product{
                ProductID = product.ProductID,
                ValidationRequestCode = product.ValidationRequestCode
        });
    }

    return product;
}

For further more, you can customise method ThrowHttpError to suite your needs. Best part is, it is still testable.

Answer 2

BTW, I see some weak points on your approach:

1. WebAPI is meant to be used to create RESTful Web services. Why are you trying to provide another layer of status and other details? HTTP is rich enough to cover these requirements. For example, you can use standard status codes and a subcode as follows: 500.1, 500.2.

2. Success or failure is easier to express with HTTP status codes. 2XX range for successful operations, and for an unsuccessful one you can use, for example, 400 (Bad Request). 401 for an unauthorized access... 500 for a server failure...

3. WebAPI already provides ModelState to let the framework build a response object. Use it and try to don't re-invent the wheel.

4. Again, keep it simple. Response entity goes on the response body. Success or failure is expressed by status codes. Details about a bad request are added to the ModelState dictionary. An error message should be set to the response's ReasonPhrase.

IHttpActionResult implementations are meant to transform your domain result into an HTTP response. That is, you're in the right track excepting when you try to return your response object as is. My advise is you should use your IHttpActionResult to set every detail on your own response object to standard HTTP semantics, and notify errors using ModelState out-of-the-box approach which works well.