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.

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