Sunday, October 14, 2012

Commands, queries and testing

Also read:
  1. Self-contained commands with dependencies
  2. Separating command data from logic and sending it on a bus
We need abstraction, but the amount of abstraction we really need depends, and should be assessed on a case-by-case basis. It seems advisable to grow abstractions, and to introduce them gradually.

That being said, in this post I want to talk about an architecture that tries to limit abstractions to solely commands and queries.

It all starts with two small pieces of infrastructure: a command and a query. A command performs an action, and can change state, while a query should only return data, and not alter any state; basic command and query separation.
public abstract class Command
{
    public abstract void Execute();
}

public abstract class Query<T>
{
    public abstract T Execute();
}
Now imagine, we are doing something with accounts, and we want to have a command that can withdraw money from an account, and a query that returns the total amount available on an account. The assumption is that we're only talking with a database.
public class WithdrawAmountCommand : Command
{
    public WithdrawAmountCommand(string user, int amount)
    {
        if (string.IsNullOrEmpty(user))
            throw new NullReferenceException("user");                
    
        User = user;
        Amount = amount;
    }

    public string User { get; private set; }

    public int Amount { get; private set; }

    public override void Execute()
    {
        // Implementation
    }
}
We inherit from the Command class, and pass in its input arguments via the constructor. We put our actual implementation in the Execute method override. For the query, we do something very similar.
public class TotalAmountQuery : Query<int>
{
    public TotalAmountQuery(string user)
    {
        if (string.IsNullOrEmpty(user))
            throw new NullReferenceException("user");
    
        User = user;
    }

    public string User { get; private set; }

    public override int Execute()
    {
        return 100; // Should be an actual implementation
    }
}
Now, if we would use this in ASP.NET MVC, we would end up with clean and compact controllers. Have a look at the action Foo, which first withdraws 25 euros, to then redirect to a warning action if the balance is negative, or to redirect back to the index action when it's positive. Very readable, right?
[HttpPost]
public ActionResult Foo(string user)
{
    new WithdrawAmountCommand(user, 25).Execute();

    var totalAmount = new TotalAmountQuery(user).Execute();

    if (totalAmount < 0)
        return RedirectToAction("Warning");

    return RedirectToAction("Index");
} 
There are several advantages I experienced while applying this pattern. First, there are less layers of abstraction, while not neglecting readability nor maintainability. In a more conservative approach, I would end up with an account repository, abstracting my data access, and probably also an account service, abstracting my business logic. This repository, and service, would have multiple methods, which probably solve related, yet different problems. I would have to wade through all the methods in these classes to find the operation that is relevant for me.

When you model each operation as a class on its own, you define it more explicitly. And you're able to adhere to the Single Responsibility principle even more. Practically, it's now also easier to locate an operation in the codebase by just looking at the solution explorer.

One last, but not unimportant, advantage I see, is that it gets easier to group queries and commands per functionality and context.

We could test the example above against the actual implementation, and I don't see anything wrong with that per se, but it isn't always that practical; the setup of your queries can be heavy and complex, performance of your tests might be suffering etc.. One technique would be to create an interface per query and command and inject them, but that seems cumbersome, and ceremony heavy, and that isn't something I'm willing to do.

Something in between could be introducing a Command- and QueryHandler.
public interface ICommandHandler
{
    void Execute(Command command);
}

public class CommandHandler : ICommandHandler
{
    public void Execute(Command command)
    {
        command.Execute();
    }
}

public interface IQueryHandler 
{
    T Execute<T>(Query<T> query);
}

public class QueryHandler : IQueryHandler
{
    public T Execute<T>(Query<T> query)
    {
        return query.Execute();
    }
}
These could be injected into the controller, and be used as an intermediary to execute queries and commands for us.
public class HomeController : Controller
{
    private readonly IQueryHandler _qryHandler;
    private readonly ICommandHandler _cmdHandler;

    public HomeController()
    {
        _qryHandler = new QueryHandler();
        _cmdHandler = new CommandHandler();
    }

    public HomeController(
        ICommandHandler cmdHandler, IQueryHandler qryHandler)
    {
        _cmdHandler = cmdHandler;
        _qryHandler = qryHandler;
    }

    [HttpPost]
    public ActionResult Foo(string user)
    {
        _cmdHandler.Execute(new WithdrawAmountCommand(user, 25));

        var totalAmount = _qryHandler.Execute<int>(new TotalAmountQuery(user));

        if (totalAmount < 0)
            return RedirectToAction("Warning");

        return RedirectToAction("Index");
    }
}
This technique would make it possible, and easy, to use stubs or mocks for queries and commands.
[TestMethod()]
public void Foo_should_withdraw_25_euros_from_the_account_of_jef_claes()
{
    var cmdHandler = new Mock<ICommandHandler>();
    var qryHandler = new Mock<IQueryHandler>();

    var controller = new HomeController(
        cmdHandler.Object, qryHandler.Object);

    controller.Foo("JefClaes");

    cmdHandler.Verify(
        h => h.Execute(It.Is<WithdrawAmountCommand>(
            c => c.Amount == 25 && c.User == "JefClaes")));
}

[TestMethod()]
public void Foo_should_redirect_to_a_warning_page_when_amount_negative()
{
    var cmdHandler = new Mock<ICommandHandler>();
    var qryHandler = new Mock<IQueryHandler>();

    qryHandler
        .Setup(h => h.Execute<int>(It.IsAny<TotalAmountQuery>()))
        .Returns(-1);

    var controller = new HomeController(cmdHandler.Object, qryHandler.Object);
    var result = (RedirectToRouteResult)controller.Foo("JefClaes");

    Assert.AreEqual("Warning", result.RouteValues["action"]);
}
I don't think this should be introduced as an application-wide thing necessarily, but it could only be used when testing the actual implementation becomes hard or awkward.

But what if I need to inject dependencies into my commands or queries for testing - there is more to it than a database? I gave that some thought as well, and it seems inevitable to introduce extra abstractions, and make changes to the concepts discussed above; I'll publish these tomorrow.

I've been entertaining lots of ideas on this topic lately, and I'm curious to hear yours.

20 comments:

  1. If we are talking about validation, then validation needs to be in the "Account" domain model - and not really in the controller although this is perhaps because it is just a dummy code.

    ReplyDelete
    Replies
    1. There isn't any validation in the controller, it's in the commands. At the current level of abstraction, I actually see the commands as part of the domain model. Makes sense?

      Delete
    2. Makes perfect sense. Since the commands are modelled after actions you make in the domain, they are part of the domain.

      Delete
  2. - Having the command be the executor works great for "smaller" projects/models. Yet, you won't be able to build a pipeline that easily (think decoration). The pipeline might involve UoW, performance, logging, security, batching and the like. Explicit handlers solve this. The other reason not to go the "execute on command/query" route is distribution (e.g. having them as DTOs).
    - I don't like having ICommandHandlers, IQueryHandlers inside my controller. I usually end up creating a small facade that increases readability and keeps the "Execute" semantics out of the controller. Personal preference, I admit.
    - Having commands and queries implement equality makes testing that much easier. Or use something like CompareObjects (http://comparenetobjects.codeplex.com/). It.IsAny() makes for low test quality, IMO.

    Looking forward to the next part ...

    ReplyDelete
  3. With your Command base class returning void on Execute, how do you handle creating new domain objects? It's most often necessary to know the id of the newly created object, to eg: redirect the user to the object's detail page.

    ReplyDelete
    Replies
    1. I think there are two options here:
      - A GUID
      - Passing in a reference, and changing that in the command (ex account.Id)

      Delete
  4. I spiked something similar a while back: https://github.com/Lodewijk-S/Sioen.Layerless/tree/master/Sioen.Layerless.Infrastructure/Command

    I used property injection to 'enrich' the commands inside the CommandExecutor (your CommandHandler). I would really like to use this in a real project to find out the limitations.

    ReplyDelete
    Replies
    1. I thought of that, but in general I have found property injection to be a PITA.

      Delete
  5. +1 on the pipeline argument. I'm currently looking at a plugin to record commands & queries to see what our testers are doing.

    I like the 'specific' handler approach that's used in the Agatha framework (http://davybrion.github.com/Agatha). Every command/query (request in Agatha) has it's own handler. To issue the request you use a RequestDispatcher which can be easily extended.



    ReplyDelete
    Replies
    1. That's the approach I used in the next post :)

      Delete
  6. You can also raise an even "MoneyWithdrawedEvent" that you will subscribe to get the id of whatever you want

    ReplyDelete
  7. Plus I think that the ICommandHandler and the IQueryExecuter are redundant...You will never create another executer. Make the execute method virtual so that you can mock it.

    ReplyDelete
    Replies
    1. Why not just mock it using the interfaces?

      Delete
  8. I wonder what the point is in injecting (either constructor or property) into your commands. A command or query should be a just a small DTO in my opinion. They shouldn't contain any methods or functionality.

    I mostly go with the following.

    You got a specific service within your application: Lets say AccountService. This service exposes in it's API the methods to "public GetTotalAmountResult GetTotalAmount(GetTotalAmountRequest request)" and "WithdrawAmount(WithdrawAmountCommand command)".

    This 'request' and 'command' then be processed by the AccountService implementation by calling to their appropriate handler (Command or Query).

    These handlers ( I rather call them 'Dispatchers' ) resolve 'QueryHandlers' or 'CommandHandlers' from your container. Those 'QueryHandlers' or 'CommandHandlers' then take in the command or request and process the provided data. On those 'QueryHandlers' or 'CommandHandlers' I would inject into the constructor whatever is necessary. Not on the command or query object itself.

    This setup will allow you to stub/mock anything in the process. We even extended this a bit more by pulling out the actually queries or commands from the handlers into separate objects, so they can to be tested. In case you won't some code to back this up, I'll be happy to create a small git for this.

    ReplyDelete
    Replies
    1. I think I took that approach in this post: http://www.jefclaes.be/2012/10/commands-with-dependencies.html

      Delete
    2. https://gist.github.com/4550550

      A gist to support my answer. See the difference. You utilize a "Command" but it also executes. I know that the command pattern tells us to do it like that. But in general, a command doesn't execute itself, something else executes the command. Just my opinion of course. Happy to hear your thoughts about it.

      Delete
    3. What's the reasoning wrapping everything behind a service again? Is that like the implementation of an endpoint?

      What's the _real_ advantage of separating commands and data if you're not using messaging, and only exceptionally need to have your dependencies injected?

      Delete
  9. I think in compare to your other post, that your commands/data/query are indeed in front of your service (AccountWebService in your other post). While mine are part of it.

    It just seemed a tad odd to send commands to a (web)service, taking off the passed data and pass it to a (web)service as parameters, while the (web)service just might as well take in the full command as a DTO and handle it on there own. Just seemed unnecessary wrapping and unwrapping of things.

    If you don't use messaging and you have a dependency to your service, why not use at the time of need in your frontend or the place where your commands are fired. It's just my opinion and i'm a big fan of keeping it simple.

    Or did I misinterpreted your question?

    ReplyDelete
    Replies
    1. Ah, I think I understand the confusion now. If you reread the post, you'll see that all logic I own (handlers, commands, queries) lives in the MVC app, not in webservices. My logic might invoke external services though.

      Delete