Use MediatR for commanding

[xperiandri]

What do you think about using https://github.com/jbogard/MediatR for commanding as Jason proposes in his talk?
I succesfully implemented this approach on 2 projects in production.
And I can submit a PR.

[RehanSaeed]

I'm aware of MediatR and what it does. There are different ways to implement the command pattern, which is actually a very simple pattern to implement in theory. I think that it can be implemented very simply without another dependency as the API project template shows.

I will watch that video and get back to you.

Do you see some clear examples of where MediatR shows some real benefit?

There is also another question. Some people like to remove all ASP.NET Core related code from their commands e.g. remove all return new OkObjectResult() or code related to setting headers etc. and leaving all of that code in the Controller. In my experience, your command then has to return a lot of metadata about the result and you end up returning a lot of tuples. It also means that a lot of commands become one liners with lots of logic in the controllers. I've always felt that this is all a matter of taste but I've not always been certain I have it right. So far though, my approach seems to work and testing is not difficult.

[xperiandri]

The main benefit is you have only 2 dependencies in controller: IMediator and IAutoMapper
So that controller deals with HTTP stuff and commands/queries with handlers deal with business logic.
I separate commands and queries to different libraries not to have any dependencies between them at all.

It also means that a lot of commands become one-liners with lots of logic in the controllers.

I don't think that there are too many logic.
I usually put command, result and handler into a single file to have all the stuff together:

namespace Commands.Binder
{
    public class Create : IRequest<CreateResult>
    {
        public Guid Id { get; }
        public string Description { get; }
        public bool IsPublic { get; }
        public string Titles { get; }
        public string Sections { get; }
        public long OwnerId { get; }

        public Create(Guid id, string description, bool isPublic, string titles, string sections, long ownerId)
        {
            Id = id;
            Description = description;
            IsPublic = isPublic;
            Titles = titles;
            Sections = sections;
            OwnerId = ownerId;
        }
    }

    public struct CreateResult : IEquatable<CreateResult>
    {
        public Guid Id { get; }
        public DateTimeOffset CreatedAt { get; }

        public CreateResult(Guid id, DateTimeOffset createdAt)
        {
            Id = id;
            CreatedAt = createdAt;
        }

        #region IEquatable
        public override bool Equals(object obj)
        {
            if (obj is CreateResult)
            {
                return Equals((CreateResult)obj);
            }

            return base.Equals(obj);
        }

        public static bool operator ==(CreateResult first, CreateResult second)
        {
            return first.Equals(second);
        }

        public static bool operator !=(CreateResult first, CreateResult second)
        {
            return !(first == second);
        }

        public bool Equals(CreateResult other)
        {
            return Id.Equals(other.Id) && CreatedAt.Equals(other.CreatedAt);
        }

        public override int GetHashCode()
        {
            unchecked
            {
                var hashCode = 47;
                hashCode = (hashCode * 53) ^ EqualityComparer<Guid>.Default.GetHashCode(Id);
                hashCode = (hashCode * 53) ^ CreatedAt.GetHashCode();
                return hashCode;
            }
        }
        #endregion IEquatable
    }

    public class CreateHandler : DbContextHandler<RawCheckItDbContext, Create, CreateResult>
    {
        public CreateHandler(IMapper mapper, RawCheckItDbContext context) : base(mapper, context) { }

        public override async Task<CreateResult> Handle(Create request, CancellationToken cancellationToken)
        {
            var binder = mapper.Map<RawBinder>(request);
            context.RawBinders.Add(binder);
            context.Entry(binder)
                .Property(RawBinder.OwnerIdShadowPropertyName)
                .CurrentValue = request.OwnerId;
            await context.SaveChangesAsync(cancellationToken);
            return mapper.Map<CreateResult>(binder);
        }
    }
}

Then controller mthod will look like this:

[HttpPost]
[Route(Routes.Binders)]
public virtual async Task<ActionResult> Create(
    [ModelBinder(typeof(JsonNetModelBinder))] BinderCommandViewModel newBinder)
{
    if (!ModelState.IsValid)
        return BadRequest();

    if (string.IsNullOrWhiteSpace(newBinder.Description))
        return BadRequest("Empty description");

    var command = mapper.Map<Create>(newBinder);
    var result = await mediator.Send(command);
    Response.StatusCode = 201;
    return JsonNet(result);
}

[VictorioBerra]

@xperiandri whats a DbContextHandler

[RehanSaeed]

I watched the linked talk and here are some observations:

Cross Cutting Concerns

The main selling point of MediatR in my opinion is that it lets you implement cross cutting concerns like authentication, logging, validation etc. That's cool but standard ASP.NET Core action filters also let you do that easily. In the talk he implemented something to time the duration of a handlers execution time and log it. You can do that with an action filter easily. With Validation, ASP.NET Core already returns 400 if your model is invalid and with authentication, I'm not certain why you would want to do that instead of using authorization policies.

Exceptions for Flow Control

The guy in the talk was throwing an exception to signify that a resource could not be found. He then caught that exception and returned 404 Not Found. Using exceptions for flow control is bad practice and slow. He's the second guy I've seen do this with MediatR.

That is not to say that you have to use MediatR like this. One alternative is that the command handler could return a tuple as a result which determines the type of response to return e.g. return (isNotFound, isBadRequest, isConflict, value);, then in the controller action you have to have a bunch of if statements to return the correct response.

Alternatively, you can do what this project template does and return an IActionResult from your MediatR handler and just return that result in the controller, similar to what this project does. This means that the controller action methods are just one liners executing the command and returning the result.

Extra Classes

With MediatR you have to write a Command/Query class with the parameters that get passed to the Handler class. You might think that you could use the Command/Query in your controller action method but what if your action method accepts parameters from two or more of: [FromRoute], [FromQuery] or [FromHeader]? MediatR forces you to write additional view models for the action method too. So you end up with the following classes per action method:

• View Models for each [FromRoute], [FromQuery] or [FromHeader].
• Command/Query
• Handler

That's one extra class as compared to what this project does.

Extra Levels of Indirection

Imagine that you are looking in your controller at an action method and want to navigate to the MediatR handler. You have to hop to the command/query class and then find all usages on that to find the handler. It's an extra level if indirection that hides some things from you.

With this project, you see the IFooCommand in the controller action method and you know exactly where to go.

Extra Dependencies

I want to keep the number of dependencies low. Simply calling a command from a controller and injecting the command into the action method parameters directly using [FromServices] seems simple.

Benchmarks

Last time I checked, MediatR uses System.Activator.CreateInstance to create handlers. I'd like to see some benchmarks to see what impact it has.

Fluent Validation

This was also in the talk and I've been meaning to implement it for some time. I'd certainly take a PR for this.

These are just my opinions and other peoples may disagree. A lot of this starts to get a bit subjective and opinion based. I'd love to hear any arguments what I've written, maybe I've missed something crucial.

[xperiandri]

@VictorioBerra, just a helper class to inject dependencies

public abstract class DbContextHandler<TContext, TRequest> : IRequestHandler<TRequest>
    where TContext : DbContext
    where TRequest : IRequest<Unit>
{
    protected readonly IMapper mapper;
    protected readonly TContext context;

    public DbContextHandler(IMapper mapper, TContext context)
    {
        this.mapper = mapper;
        this.context = context;
    }

    public abstract Task<Unit> Handle(TRequest request, CancellationToken cancellationToken);
}

public abstract class DbContextHandler<TContext, TRequest, TResponse>
    : IRequestHandler<TRequest, TResponse>
    where TContext : DbContext
    where TRequest : IRequest<TResponse>
{
    protected readonly IMapper mapper;
    protected readonly TContext context;

    public DbContextHandler(IMapper mapper, TContext context)
    {
        this.mapper = mapper;
        this.context = context;
    }

    public abstract Task<TResponse> Handle(TRequest request, CancellationToken cancellationToken);
}

I would better get rid of it by matching handlers from static methods by signature. I research how to implement that with MediatR

[xperiandri]

Alternatively, you can do what this project template does and return an IActionResult from your MediatR handler and just return that result in the controller, similar to what this project does. This means that the controller action methods are just one liners executing the command and returning the result.

The basic idea is to decouple HTTP stuff from business logic.

That's one extra class as compared to what this project does.

I can't figure out how do you count that.
1 class for query/command, 1 class for viewmodel [FromBody], all the other stuff you inject directly into a command in controller method on command constructor.

Imagine that you are looking in your controller at an action method and want to navigate to the MediatR handler. You have to hop to the command/query class and then find all usages on that to find the handler. It's an extra level if indirection that hides some things from you.

That is why I put them all into a single file.
So that you F12 on command/query and you have all the stuff in a single file to work with.

I'm not a fun of file-per-class approach. I feel it like a workaround for dummies who don't know how to use Visual Studio features like Ctrl+,. I prefer to group interface with implementing class if possible and in case of MediatR to group command/query, result and handler together.

Last time I checked, MediatR uses System.Activator.CreateInstance to create handlers. I'd like to see some benchmarks to see what impact it has.

Now it caches them in a static dictionary.

[RehanSaeed]

The basic idea is to decouple HTTP stuff from business logic.

Thats a fair comment but there is a tradeoff. Decoupling or introducing an extra abstraction means writing more boilerplate code. You have to choose whether you think it's worth the extra effort or not. Even if you decide to go for decoupling, you can still do that without MediatR by simply calling a command in an action method that returns a result or error and then deciding what type of response to send in the action method. Very simple, perhaps I should implement this and see what it looks like. Perhaps we could also compare a MediatR PR.

I can't figure out how do you count that.
I think the handler is the extra class that you've missed.

That is why I put them all into a single file.
My StyleCop rules would not let me do that. This is a matter of personal preference, although I can see why your approach might be a good idea and why you've chosen to go down that route. Perhaps nested classes is the answer to avoid StyleCop warnings.

Now it caches them in a static dictionary.
Maybe I'll see if there are benchmarks.

[RehanSaeed]

This was a fun discussion :). This is one of those areas of software development where people have different preferences and there is probably no clear cut correct answer. Closing for now.