Is CQRS/MediatR worth it when developing an ASP.NET application?

Question

I've been looking into CQRS/MediatR lately. But the more I drill down the less I like it. Perhaps I've misunderstood something/everything.

So it starts out awesome by claiming to reducing your controller to this

public async Task<ActionResult> Edit(Edit.Query query)
{
    var model = await _mediator.SendAsync(query);

    return View(model);
}

Which fits perfectly with the thin controller guideline. However it leaves out some pretty important details - error handling.

Lets look at the default Login action from a new MVC project

public async Task<IActionResult> Login(LoginViewModel model, string returnUrl = null)
{
    ViewData["ReturnUrl"] = returnUrl;
    if (ModelState.IsValid)
    {
        // This doesn't count login failures towards account lockout
        // To enable password failures to trigger account lockout, set lockoutOnFailure: true
        var result = await _signInManager.PasswordSignInAsync(model.Email, model.Password, model.RememberMe, lockoutOnFailure: false);
        if (result.Succeeded)
        {
            _logger.LogInformation(1, "User logged in.");
            return RedirectToLocal(returnUrl);
        }
        if (result.RequiresTwoFactor)
        {
            return RedirectToAction(nameof(SendCode), new { ReturnUrl = returnUrl, RememberMe = model.RememberMe });
        }
        if (result.IsLockedOut)
        {
            _logger.LogWarning(2, "User account locked out.");
            return View("Lockout");
        }
        else
        {
            ModelState.AddModelError(string.Empty, "Invalid login attempt.");
            return View(model);
        }
    }

    // If we got this far, something failed, redisplay form
    return View(model);
}

Converting that presents us with a bunch of real world problems. Remember the goal is to reduce it to

public async Task<IActionResult> Login(Login.Command command, string returnUrl = null)
{
    var model = await _mediator.SendAsync(command);

    return View(model);
}

One possible solution to this is to return an CommandResult<T> instead of a model and then handle the CommandResult in a post action filter. As discussed here.

One implementation of the CommandResult could be like this

public interface ICommandResult  
{
    bool IsSuccess { get; }
    bool IsFailure { get; }
    object Result { get; set; }
}

source

However that doesn't really solve our problem in the Login action, because there are multiple failure states. We could add these extra failure states to ICommandResult but that is a great start for a very bloated class/interface. One might say it doesn't comply with Single Responsibility (SRP).

Another problem is the returnUrl. We have this return RedirectToLocal(returnUrl); piece of code. Somehow we need to handle conditional arguments based on the success state of the command. While I think that could be done (I'm not sure if the ModelBinder can map FromBody and FromQuery (returnUrl is FromQuery) arguments to a single model). One can only wonder what kind of crazy scenarios could come down the road.

Model validation have also become more complex along with returning error messages. Take this as an example

else
{
    ModelState.AddModelError(string.Empty, "Invalid login attempt.");
    return View(model);
}

We attach an error message along with the model. This sort of thing cannot be done using an Exception strategy (as suggested here) because we need the model. Perhaps you can get the model from the Request but it would be a very involved process.

So all in all I'm having a hard time converting this "simple" action.

I'm looking for inputs. Am I totally in the wrong here?

Answer 1

I think you're expecting too much of the pattern you're using. CQRS is specifically designed to address the difference in model between query and commands to the database, and MediatR is just in-process messaging library. CQRS doesn't claim to eliminate the need for business logic like you're expecting them to. CQRS is a pattern for data access, but your problems are with presentation layer--redirects, views, controllers.

I think you may be mis-applying the CQRS pattern to authentication. With login it cannot be a modelled as a command in CQRS because

Commands: Change the state of a system but do not return a value
- Martin Fowler CommandQuerySeparation

In my opinion authentication is a poor domain for CQRS. With authentication you need strongly consistent, synchronous request-response flow so you can 1. check user's credentials 2. create a session for the user 3. handle any of the variety of edge cases that you've identified 4. immediately grant or deny user in response.

Is CQRS/MediatR worth it when developing an ASP.NET application?

CQRS is a pattern that has very specific uses. It's purpose is to model queries and commands instead of having a model for records as used in CRUD. As systems become more complex, the demands of views are often more complex than just showing a single record or a handful of records, and a query can better model the needs of the application. Similarly commands can represent changes to many records instead of CRUD which you change single records. Martin Fowler warns

Like any pattern, CQRS is useful in some places, but not in others. Many systems do fit a CRUD mental model, and so should be done in that style. CQRS is a significant mental leap for all concerned, so shouldn't be tackled unless the benefit is worth the jump. While I have come across successful uses of CQRS, so far the majority of cases I've run into have not been so good, with CQRS seen as a significant force for getting a software system into serious difficulties.
- Martin Fowler CQRS

So to answer your question CQRS should not be the first resort when designing an application when CRUD is suitable. Nothing in your question gave me the indication that you have a reason to use CQRS.

As for MediatR, it's an in-process messaging library, it aims to decouple requests from request handling. You must again decide if it will improve your design to use this library. I'm personally not an advocate of in-process messaging. Loose-coupling can be achieved in simpler ways than messaging, and I would recommend you start there.

Answer 2

CQRS is more of a data management thing rather than and doesn't tend to bleed too heavily into an application layer (or Domain if you prefer, as it tends to be most often used in DDD systems). Your MVC application, on the other hand, is a presentation layer application and should be fairly well separated from the query/persistence core of the CQRS.

Another thing worth noting (given your comparison of the default Login method and desire for thin controllers): I wouldn't exactly follow default ASP.NET templates/boilerplate code as being anything we should worry about for best practices.

I like thin controllers as well, because they're very easy to read. Each controller I have usually has a "service" object that it pairs with that essentially handles the logic required by the controller:

public async Task<IActionResult> Login(LoginViewModel model, string returnUrl = null) {

    var result = _service.Login(model);
    switch (result) {
        case result.lockout: return View("Lockout");
        case result.ok: return RedirectToLocal(returnUrl);
        default: return View("GeneralError");
    }
}

Still thin enough, but we've not really changed how the code works, just delegate the handling to the service method, which really serves no purpose other than making the controller actions easy to digest.

Bear in mind, this service class is still responsible for delegating the logic to the model/application as required, it's really just a slight extension of the controller to keep the code neat. The service methods are generally pretty short as well.

I'm not sure the mediator would be doing anything conceptually different than that: moving some basic controller logic out of the controller and into somewhere else to be processed.

(I hadn't heard of this MediatR before, and a quick look at the github page doesn't seem to indicate that it's anything groundbreaking - certainly not something like CQRS - in fact, it looks to be something like just another abstraction layer you can put in to complicate the code by means of making it look simplier, but that's just my initial take)

Answer 3

I highly recommend you view Jimmy Bogard's NDC presentation on his approach to modelling http requests https://www.youtube.com/watch?v=SUiWfhAhgQw

You will then get a clear idea of what Mediatr is used for.

Jimmy does not have a blind adherence to patterns and abstractions. He's very pragmatic. Mediatr does clean up controller actions. As for the exception handling, I push that into a parent class called something like Execute. So you end up with a very clean controller action.

Something like:

public bool Execute<T>(Func<T> messageFunction)
{
    try
    {
        messageFunction();

        return true;
    }
    catch (ValidationException exception)
    {
        Errors = string.Join(Environment.NewLine, exception.Errors.Select(e => e.ErrorMessage));
        Logger.LogException(exception, "ValidationException caught in SiteController");
    }
    catch (SiteException exception)
    {
        Errors = exception.Message;
        Logger.LogException(exception);
    }
    catch (DbEntityValidationException dbEntityValidationException)
    {
        // Retrieve the error messages as a list of strings.
        var errorMessages = dbEntityValidationException.EntityValidationErrors
                .SelectMany(x => x.ValidationErrors)
                .Select(x => x.ErrorMessage);

        // Join the list to a single string.
        var fullErrorMessage = string.Join("; ", errorMessages);

        // Combine the original exception message with the new one.
        var exceptionMessage = string.Concat(dbEntityValidationException.Message, " The validation errors are: ", fullErrorMessage);

        Logger.LogError(exceptionMessage);

        // Throw a new DbEntityValidationException with the improved exception message.
        throw new DbEntityValidationException(exceptionMessage, dbEntityValidationException.EntityValidationErrors);                
    }
    catch (Exception exception)
    {
        Errors = "An error has occurred.";
        Logger.LogException(exception, "Exception caught in SiteController.");
    }

    // used to indicate that any transaction which may be in progress needs to be rolled back for this request.
    HttpContext.Items[UiConstants.Error] = true;

    Response.StatusCode = (int)HttpStatusCode.InternalServerError; // fail

    return false;
}

Usage looks a bit like this:

[Route("api/licence")]
public IHttpActionResult Post(LicenceEditModel licenceEditModel)
{
    var updateLicenceCommand = new UpdateLicenceCommand { LicenceEditModel = licenceEditModel };
    int licenceId = -1;

    if (Execute(() => _mediator.Send(updateLicenceCommand)))
    {
        return JsonSuccess(licenceEditModel);
    }

    return JsonError(Errors);
}

Hope that helps.

Answer 4

Many people (I did it too) confuse pattern with a library. CQRS is a pattern but MediatR is a library that you can use to implement that pattern

You can use CQRS without MediatR or any in-process messaging library and you can use MediatR without CQRS:

public interface IProductsWriteService
{
    void CreateProduct(CreateProductCommand createProductCommand);
}

public interface IProductsReadService
{
    ProductDto QueryProduct(Guid guid);
}

CQS would look like this:

public interface IProductsService
{
    void CreateProduct(CreateProductCommand createProductCommand);
    ProductDto QueryProduct(Guid guid);
}

In fact, you don't have to name your input models "Commands" like above CreateProductCommand. And input of your queries "Queries". Command and queries are methods, not models.

CQRS is about segregation of responsibility (read methods must be in a separate place from write methods - isolated). It's an extension to CQS but the difference is in CQS you can put these methods in 1 class. (no responsibility segregation, just command-query separation). See separation vs segregation

From https://martinfowler.com/bliki/CQRS.html:

At its heart is the notion that you can use a different model to update information than the model you use to read information.

There's confusion in what it says, it's not about having a separate model for input and output, it's about separation of responsibility.

CQRS and id generation limitation

There's one limitation you will face when using CQRS or CQS

Technically in the original description commands shouldn't return any value (void) which I find stupid because there's no easy way to get generated id from a newly created object: https://stackoverflow.com/questions/4361889/how-to-get-id-in-create-when-applying-cqrs.

so you have to generate id each time yourself instead of letting the database do it.

---

If you want to learn more: https://cqrs.files.wordpress.com/2010/11/cqrs_documents.pdf