dateo. Coding Blog

Coding, Tech and Developers Blog

csharp
asp.net
controllers
web api

Learn how to improve your ASP.NET Core controllers with these principles

Dennis Frühauff on October 17th, 2022

Using ASP.NET for creating APIs, and also consuming public endpoints for quite some time now, I have seen both good and bad examples. Sometimes, you can even tell from the responses you’ll get from an endpoint, that the underlying controller design is rubbish. But what does a healthy controller in ASP.NET Core look like? Google is actually not too generous with answers to this question. That’s why I’ve tried to compile a few guiding principles that reflect my personal opinion on the answer to that question. Many of them are also what you might consider best practices in the industry.


Introduction

Typically, ASP.NET controllers provide endpoints for an API that transports JSON data via HTTP. Many people equate this behavior with that of a REST API, but those are actually not the same. REST itself does not make any assumptions about the data structure or the transport technology in its specification. You might want to check out Wikipedia for that matter, or have a go at a decent talk of Spencer Schneidenbach at NDC.


Nonetheless, for the sake of simplicity, let’s assume that we are indeed talking about JSON via HTTP, implemented in ASP.NET Core.


Healthy API

Since we were talking about the API, let me share a few thoughts on how a well-defined API might look like. We all know the famous class Employee that is used through all ASP.NET tutorials out there. Here is what a bad example might look like:


[GET] /employee/getEmployees
[GET] /employee/getSingleEmployee/42

And right away, here is what I would want it to look like:


[GET] /v1/employees/
[GET] /v1/employees/42

So, there are a few things worth extracting from this. The first one is concerned with the versioning of APIs.


Version your API already in the URL, i.e., make the version information obvious.


Second, use plural nouns for your endpoints, and don’t get creative on the method names.


User plural nouns for endpoint names, such as /employees/. Keep it simple.


And third, let me share my assumptions about HTTP request methods when I see them:


  • [GET] Simply gets data. Don’t do manipulation/permutation of data within these requests.
  • [POST] Creates a new entry.
  • [PUT] Updates an entry. I will have to provide all the required properties. If not, they are set to null or its equivalent.
  • [PATCH] Updates an entry. I provide only the properties I want to update.
  • [DELETE] Deletes an entry.

Return values (status codes)

There are plenty of HTTP status codes and there sure as hell is a reason ASP.NET does not provide a shortcut method for every one of them.


Focus on a small set of well-defined HTTP status codes. Don’t reinvent the wheel.


Let’s have a look at common examples and what they usually mean to developers:


  • [200 OK] Everything is fine. GET request was good and here are your results.
  • [201 OK] Everything is fine. POST request was good.
  • [400 BR] Your input data was invalid, because (!) …
  • [404 NF] Could not find the resource.
  • [401 UA] I don’t know you.
  • [403 FB] I know you, but you don’t deserve to see this.
  • [500 IE] Something really bad happened.

Those are the ones you’d usually see. If you decide to use a few more, feel free, but please document them somewhere.


So, how do you return these? First of all, make sure, that you’re controllers return ActionResult, which is a wrapper for both status codes and your data.


[HttpGet]
public async Task<ActionResult> Get()
{
    var employees = await this.employeeService.GetEmployeesAsync();
    return Ok(employees);
}

[HttpPost]
public async Task<ActionResult> Post([FromBody] EmployeeDto employee)
{
    var validationResults = this.validator.Validate(employee).ToList();

    if (validationResults.Any())
    {
        var validationResponse = new ApiErrorValidationResponse(HttpStatusCode.BadRequest, validationResults);
        return BadRequest(validationResponse);
    }

    await this.employeeService.AddEmployeeAsync(employee);
    return Ok();
}

If your controller derives from ControllerBase there are a couple of shorthand functions to return proper responses, and you should use them.


Oh, and while we’re at it: Please, do stuff asynchronously, remember?


The controller does too much

So now somebody is consuming our API, we have a look at the controllers and we see it doing one or many of the following things: Validation, database access, applying business logic, error and exception handling, mapping, and returning values. Not a healthy thing to do in a controller.


Controllers are delegates. They tell other classes to do something with an information, and return their results to the consumer.


That being said, you should see to it, that your controllers really just dispatch work to other services. For example, input data validation can be done using framework methods, data retrieval should be done in a service using a repository, and mapping could be done in the exact same service already. Exception handling should definitely not be dealt with in a controller, but more on that later. Also, business logic does not belong in a controller either. So please keep your controllers clean of all that.


Validation

As far as validation goes, there are different options to reach similar outcomes.


  1. Use built-in framework validation:
using Microsoft.AspNetCore.Mvc.ModelBinding.Validation;

public class EmployeeDto
{
    [Required] public string FirstName { get; set; }
    [Required] public string LastName { get; set; }
}

This will work for very simple problems. DTOs will be automatically validated before they even hit your controller if it has the attribute [ApiController].


  1. Use extended framework validation:
public class EmployeeDto : IValidatableObject
{
    public string FirstName { get; set; }
    public string LastName { get; set; }
    
    public IEnumerable<ValidationResult> Validate(ValidationContext validationContext)
    {
        if (LastName.Contains("voldemort", StringComparison.OrdinalIgnoreCase))
        {
            yield return new ValidationResult("You may not speak his name.");
        }
    }
}

You can use this option for slightly more complex scenarios. But be aware that if you are using this extensively, your DTO will eventually know too much, i.e., it actually should not know how to validate itself. In order to separate these concerns, you might want to use a separate validator class.


  1. Use a separate validator class:
public class EmployeeValidator : IValidate<EmployeeDto>
{
    public IEnumerable<ValidationResult> Validate(EmployeeDto model)
    {
        if (model.FirstName.IsValid(
                o => !string.IsNullOrEmpty(o),
                o => !o.Contains('%')))
        {
            yield return new ValidationResult($"{nameof(model.FirstName)} is invalid.");
        }
    }
}

public static class Extensions
{
    public static bool IsValid<T>(this T @this, params Func<T, bool>[] validationFuncs)
    {
        return validationFuncs.All(o => o(@this));
    }
}

Here, I have created a separate class and a helpful extension to make the code more readable. The instance of this class can be passed into your controller via dependency injection to validate your DTOs before processing them:


[HttpPost]
public async Task<ActionResult> Post([FromBody] EmployeeDto employee)
{
    var validationResults = this.validator.Validate(employee).ToList();

    if (validationResults.Any())
    {
        var validationResponse = new ApiErrorValidationResponse(HttpStatusCode.BadRequest, validationResults);
        return BadRequest(validationResponse);
    }

    await this.employeeService.AddEmployeeAsync(employee);
    return Ok();
}

If you don’t want to reinvent the wheel here, you might want to have a look at the awesome NuGet package FluentValidation.


Error reporting

There is something about error reporting that makes it difficult for people to develop and get right. I tend to think about it like the frontend of exceptions. And (many) developers don’t like frontend, so they keep doing error reporting inherently wrong. But let’s first dive into another bad example:


[HttpPost]
public async Task<ActionResult> Post([FromBody] EmployeeDto employee)
{
    ...

    try
    {
        await employeeService.AddEmployeeAsync(employee);
        return Ok();
    }
    catch (InvalidOperationException e)
    {
        //do this
    }
    catch (OperationCanceledException e)
    {
        // do something different.
    }
}

This is classic control flow via exceptions, and there are a bunch of good articles out there that will explain to you why this is generally a bad idea if you did not know. Some might argue here, that you can get rid of this code in your controller by using a middleware approach that will take over the exception handling, but that will only move bad design somewhere else.


Also, throwing exceptions and catching them somewhere else is tremendously expensive in terms of performance. Nick Chapsas has a good video if you’re interested in how expensive that is.


One way of dealing with this is using dedicated result objects that represent the outcome of a specific operation, e.g., of a service method. You can either use libraries for this task, or simply create a set of classes that your controllers can return based on which you can redirect your program’s flow.


Make it easy for people to understand what is wrong.


So now that you’ve determined that something went wrong, you should your consumers let know about it, maybe even how to fix the issues. Good APIs make it easy for people to get things right. You could for example use a specific DTO for that:


public sealed class ApiResponseBase
{
    public string? Documentation { get; }
    public string? Message { get; }
    public HttpStatusCode StatusCode { get; }

    public ApiResponseBase(HttpStatusCode statusCode, string message? = null, string? documentation = null) {...}
}

See? That is very developer-friendly.


Data encapsulation

When you’re using the full feature set of Entity Framework and even letting the scaffolding tools generate controllers for you, your code will end up publicly shipping your entity model via your API. But a) you don’t want to make the internals of your resource structure visible to the world, and b) you don’t want to make people provide properties they are not interested in. Consequently, you are likely to need separate entity models for each operation.


Separate your API model from your entity model.


Let’s consider our famous employee. Internally, the entity class might look something like this:


public class Employee 
{
    public int Id { get; set; }
    public string FirstName { get; set; }
    public string LastName { get; set; }
    public string VerySensitiveInformation { get; set; }
}

See the VerySensitiveInformation? Might be a social security number or whatever private Information you’ll have to keep for your employees. In any case, you should not expose this information when delivering these entities via your API. You’ll need a DTO for that:


public class EmployeeResponse 
{
    public int Id { get; set; }
    public string FirstName { get; set; }
    public string LastName { get; set; }
}

Now, that’s better. The ID of the entity will be useful to request a single employee from your controller. But what if somebody wants to add an employee through your API? Another DTO…


public class EmployeeDto 
{
    public string FirstName { get; set; }
    public string LastName { get; set; }
}

In this way, you’ll only have to provide the really required properties when adding entities via the API. Our controller’s signature will then look something like this:


[HttpGet]
public Task<ActionResult<IEnumerable<EmployeeResponse>>> Get()

[HttpGet]
[Route("{id:int}")]
public Task<ActionResult<EmployeeResponse>> Get(int id)

[HttpPost]
public Task<ActionResult> Post([FromBody] EmployeeDto employee)

Some might say “That’s a lot of boilerplate code. And all that mapping code that is necessary for this to work!”. And yes, that’s what data encapsulation and protection are all about. And concerning the mapping: Be quiet and let Automapper or Mapster do the work for you.



So, that is my (probably not comprehensive) list of improvements you could make to your controllers. If you follow these guidelines, I suspect your API is user-friendly and easy to maintain as well. Let me know what you think – maybe you’ll have valuable additions to my collection.



Please share on social media, stay in touch via the contact form, and subscribe to our post newsletter!

Be the first to know when a new post was released

We don’t spam!
Read our Privacy Policy for more info.

We use cookies on our website to give you the most relevant experience by remembering your preferences and repeat visits. By clicking “Accept All”, you consent to the use of ALL the cookies. However, you may visit "Cookie Settings" to provide a controlled consent.