Implementing unified API data flow in ASP.NET Core

Question

I have the API project on ASP.NET Core, and it's quite annoying always write methods like this:

[HttpGet("{libraryId:int}")]
public async Task<IActionResult> Get(int libraryId)
{
 try
 {
 var libraryInfo = await _librariesService
 .GetLibraryInfo(libraryId);
 if (libraryInfo is null)
 {
 return NotFound();
 }
 return Ok(libraryInfo);
 }
 catch (Exception)
 {
 return StatusCode(StatusCodes.Status500InternalServerError);
 }
}

So, the flow is:

1. Validate arguments
2. (optional) Pre process data
3. Process data
4. (optional) Post process data
5. Return processed data with appropriate status code

Here my classes, which implement that flow:

public class ApiResponse
{
 public int StatusCode { get; set; }
 public List<CommonError> Errors { get; } = new List<CommonError>();
 public bool HasErrors => Errors.Count > 0;
 public bool ShouldSerializeErrors() => HasErrors;
 public ApiResponse AddErrors(IEnumerable<CommonError> errors)
 {
 Errors.AddRange(errors);
 return this;
 }
 public static ApiResponse Make() => new ApiResponse();
}
public class ApiResponse<T> : ApiResponse
{
 [JsonProperty(NullValueHandling = NullValueHandling.Ignore)]
 public T Data { get; set; }
 public static new ApiResponse<T> Make() => new ApiResponse<T>();
}
public class CommonError
{
 public string Key { get; private set; }
 public string Description { get; private set; }
 public CommonError(string key, string description)
 {
 Key = key;
 Description = description;
 }
}
public class CommonResult
{
 public IList<CommonError> Errors { get; } = new List<CommonError>();
 public bool HasErrors => Errors.Count > 0; 
 public bool Success => !HasErrors;
 public CommonResult AddError(CommonError error)
 {
 Errors.Add(error);
 return this;
 }
}
public class CommonResult<T> : CommonResult
{
 public T Data { get; set; }
}
public static class ApiResponseExtensions
{
 public static ApiResponse Validate<T>(this ApiResponse apiResponse, T argument, Func<T, CommonResult> validator)
 {
 var result = validator(argument);
 if (result.HasErrors)
 {
 apiResponse.StatusCode = StatusCodes.Status400BadRequest;
 apiResponse.AddErrors(result.Errors);
 }
 return apiResponse;
 }
 public static ApiResponse<T> Validate<A, T>(this ApiResponse<T> apiResponse, A argument, Func<A, CommonResult> validator)
 {
 var result = validator(argument);
 if (result.HasErrors)
 {
 apiResponse.StatusCode = StatusCodes.Status400BadRequest;
 apiResponse.AddErrors(result.Errors);
 }
 return apiResponse;
 }
 public static async Task<ApiResponse<T>> Process<T>(this ApiResponse<T> apiResponse, Func<Task<CommonResult<T>>> func)
 {
 if (apiResponse.HasErrors)
 {
 return apiResponse;
 }
 var processed = await func();
 if (processed.HasErrors)
 {
 apiResponse.StatusCode = StatusCodes.Status500InternalServerError;
 apiResponse.AddErrors(processed.Errors);
 }
 else
 {
 apiResponse.StatusCode = StatusCodes.Status200OK;
 apiResponse.Data = processed.Data;
 }
 return apiResponse;
 }
 public static async Task<ApiResponse<T>> SideProcesses<T>(this Task<ApiResponse<T>> apiResponse, Func<Task<CommonResult>>[] funcs)
 {
 var apiResponseAwaited = await apiResponse;
 if (apiResponseAwaited.HasErrors)
 {
 return apiResponseAwaited;
 }
 foreach (var func in funcs)
 {
 var processed = await func();
 if (processed.HasErrors)
 {
 apiResponseAwaited.AddErrors(processed.Errors);
 }
 }
 return apiResponseAwaited;
 }
 public static async Task<ApiResponse<T>> SideProcesses<T>(
 this Task<ApiResponse<T>> apiResponse, 
 Func<T, Task<CommonResult>>[] funcs)
 {
 var apiResponseAwaited = await apiResponse;
 if (apiResponseAwaited.HasErrors)
 {
 return apiResponseAwaited;
 }
 foreach (var func in funcs)
 {
 var processed = await func(apiResponseAwaited.Data);
 if (processed.HasErrors)
 {
 apiResponseAwaited.AddErrors(processed.Errors);
 }
 }
 return apiResponseAwaited;
 }
 public static async Task<IActionResult> ToResult<T>(this Task<ApiResponse<T>> apiResponse)
 {
 var apiResponseAwaited = await apiResponse;
 var result = new ObjectResult(apiResponseAwaited)
 {
 StatusCode = apiResponseAwaited.StatusCode
 };
 return result;
 }
}

Rewriting the GET library info method:

[HttpGet("{libraryId:int}")]
public async Task<IActionResult> Get(int libraryId)
{
 return await ApiResponse<LibraryInfo>.Make()
 .Validate(libraryId, IdValidator),
 .Process(() => _librariesService.GetLibraryInfo(libraryId))
 .ToResult();
}

This solution isn't perfect, so I have a couple of questions, how to best implement it.

Firstly, I don't like that the call order is not defined, and I can call Process before Validate, which is incorrect.

Secondly, where do I need to store already validated function arguments? Maybe have Dictionary<string, object> ValidatedArguments inside ApiResponse? Some better way?

Finally, what should I do with the status code? Functions in Services mustn't return it, because they don't know anything about an environment where they are used. Or maybe returning always 200 is enough, when there are special flag and list of errors inside response?

Answer 1

Regarding your specific points:

Firstly, I don't like that the call order is not defined, and I can call Process before Validate, which is incorrect.

You have two options to enforce this, and they each have their own advantages:

1. Use the compiler and type-checking to enforce that Validate must be called before Process.

   The idea here is to have Validate return a ProcessableResponse, which has the Process operation on it. Now the Validate cannot be called on a ProcessableResponse, and Process cannot be called on an ApiResponse. This has the advantage of enforcing the call at compile-time, though it has the disadvantage of more overhead (moving from one type to another) at runtime, and as such will likely have poorer performance.

2. Use a "Validation State" to verify that the validater was called first.

   The idea here is to have Validate alter state on the ApiResponse to something, you can use a three-state enum: enum ValidationState { NotValidate, Validating, ValidationComplete }, or just a bool Validated, something to indicate that Validate has been called. Then you can have it check that state and throw an appropriate exception in the Process method. The advantage to this is much higher performance in the valid cases, but the disadvantage is that it's run-time checking.

---

Secondly, where do I need to store already validated function arguments? Maybe have Dictionary<string, object> ValidatedArguments inside ApiResponse? Some better way?

From your comment you indicate that you want to force them to call .Validate before .Process, which I've somewhat covered above. In this case your .Validate is no longer a validation, but a preparation. You need a different API for that, which should have a Prepare method, that takes all the inputs and validates and stores them, then returns a PreparedApiResponse, which can then be Processed to get the actual result. Your Process would no longer take a Func<T>, but a Func<..., T>, which would have a parameter for each prepared parameter (passed as input to the function).

Another interesting concept would be to stream parameters in, to an extent. Consider the following method:

public ApiResponse AddParameter<T>(T value, Predicate<T> validator = null, string name)
{
 if ((validator?.Invoke(value) ?? true) == true)
 {
 ValidatedParameters.Add(name, (object)value);
 return this;
 }
 // ... Invalid parameter
}

Then in Process:

public static async Task<ApiResponse<T>> Process<T>(this ApiResponse<T> apiResponse, Func<ValidationParameters, Task<CommonResult<T>>> func)
{
 if (apiResponse.HasErrors)
 {
 return apiResponse;
 }
 var processed = await func(apiResponse.ValidatedParameters);
 if (processed.HasErrors)
 {
 apiResponse.StatusCode = StatusCodes.Status500InternalServerError;
 apiResponse.AddErrors(processed.Errors);
 }
 else
 {
 apiResponse.StatusCode = StatusCodes.Status200OK;
 apiResponse.Data = processed.Data;
 }
 return apiResponse;
}

This would turn your call into:

[HttpGet("{libraryId:int}")]
public async Task<IActionResult> Get(int libraryId)
{
 return await ApiResponse<LibraryInfo>
 .Make()
 .AddParameter(libraryId, IdValidator, nameof(libraryId)),
 .Process((args) => _librariesService.GetLibraryInfo((int)args["libraryId"]))
 .ToResult();
}

From here we can see that (int)args["libraryId"] is less-than-desirable, but you can clear that up with a new class or extension method Get<T>(string key).

This also satisfies your first point: you can force validation on the ApiResponse parameters before you process them.

---

Finally, what should I do with the status code? Functions in Services mustn't return it, because they don't know anything about an environment where they are used. Or maybe returning always 200 is enough, when there are special flag and list of errors inside response?

This is really an implementation detail, and your ApiResponse shouldn't handle that. You should have a converter that goes from ApiResponse to the status code, such that ApiResponse shouldn't care if it needs to return a 500, 404, 403, 200, etc. The ApiResponse is responsible for processing the API, there should be a separate converter to the status code.

As far as always returning 200 - don't do that. Return the appropriate status code for the result. If it was not found, don't return 200 with an error flag of 'Not Found', instead return a 404, you can still return the response as well, but you should always return the proper HTTP status code for the result.

• Yurii N.: About second question; when I'm doing this - .Process(() => _librariesService.GetLibraryInfo(libraryId)), I take libraryId which maybe! is not validated! How can I prevent this, with taking absolutely guaranteed validated argument? About third question: am I right that converter, which you meant, is my ToResult() method, which need to decide which code must be returned, depending on list of Errors?
• Der Kommissar: @YuriyN. Edit made for 2. Regarding 3: you can have a Status on the ApiResponse, but it should not be based on HTTP codes but on what it can do. (I.e. Status.NotFound, which the converter can translate to a HTTP404 Not Found.)
• Yurii N.: Now I understand, simply describes Status as NotFound, ValidationFailed, Conflict etc, and then translate them to HTTP status codes in ToResult() method.
• Der Kommissar: @YuriyN. Yep, the ApiResponse cares about what the ApiResponse does, it's not the job of that class to care about what HTTP does or what the API responder does. The converter cares about the ApiResponse -> HTTP Response map, and the Get method cares about the actual responding.
• Yurii N.: Still can't understand, Your Process would no longer take a Func<T>, but a Func<..., T>, which would have a parameter for each prepared parameter (passed as input to the function). does it mean that I need to implement up to 16 Func<...,T> like in already existed Func implementation? Could you please provide an example?

Answer 2

Your code could be reduced to just two lines:

[HttpGet("{libraryId:int}")]
public async Task<IActionResult> Get(int libraryId)
{
 var libraryInfo = await _librariesService.GetLibraryInfo(libraryId);
 return libraryInfo == null ? NotFound() : Ok(libraryInfo); 
}

How? You first need to implement an action-filter-attribute and decorate your controller with it (or just the action). I'm using this implementation:

public class ValidateModelAttribute : ActionFilterAttribute
{
 public override void OnActionExecuting(ActionExecutingContext context)
 {
 if (!context.ModelState.IsValid)
 {
 context.Result = new BadRequestObjectResult(context.ModelState);
 }
 }
}

It'll take care of validating the model and short-cutting the request if the model is invalid.

As far as the status-code 500 is concerned you should handle this one with a middleware, for example I use this pattern where in development I use the exception page and in the production code I let the middleware handle the exceptions. There's no need to put a try/catch in every action.

if (env.IsDevelopment())
{
 app.UseDeveloperExceptionPage();
}
else
{
 app.UseExceptionHandler(appBuilder =>
 {
 appBuilder.Run(async context =>
 {
 context.Response.StatusCode = 500;
 await context.Response.WriteAsync("An unhandeled fault happened.");
 });
 });
}

• Yurii N.: My code could be reduced for two lines only because of its simplicity, imagine more complex code and more complex flow, how it would be then?
• t3chb0t: @YuriyN. rarely there is anything more complex and if there is something then it probably does not belong to the action but to some other service/repository etc etc. If you have actions with so much logic then you're doing it wrong. But come back when you have something more complex to show, then we can talk. Currently your approach is not using what asp.net core offers you and you write things that are unnecessary work.
• Yurii N.: Ok, seems legit, but I still can't understand how can I validate a lot of input parameters with ModelState? Do you mean that for each action we need create DTO, like GetLibraryParameters?
• t3chb0t: @YuriyN. yes, usually it's better. You can do this for example for [FromBody] or [FromQuery]. With simple parameters this is not necessary as they are part of the route and if they were invalid the action wouldn't be hit and if their values are invalid but there is no such resource then 404 would be appropriate which is already handeled by the respository. Alternatively there is a large table of Route Constraints that you can apply to the route (mind the warning there).
• Yurii N.: Route constraints are not for validation, what should we do then with valid in context of query parameters, but invalid in context of business logic?