Monday, October 15, 2012

Commands with dependencies

Also read: Separating command data from logic and sending it on a bus

Yesterday I wrote about an architecture which limits abstractions by solely introducing commands and queries. I shared a dead simple variation of this pattern, the advantages I experienced, and how I could still unit test the controller if I wanted to.
At the end of that post I wondered how I would be able to test commands in isolation; suppose the implementation doesn't use a database this time, but a hairy, too low-level, third party webservice.

Right now, the input arguments are inserted via the constructor, but this leaves no room to inject dependencies. That is, if we don't want to do awkward stuff with our Dependency Injection framework, and don't want to resort to property- or method injection.
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
    }
}
One way to enable injecting dependencies could be to separate the command in two parts; the handler, and the data.
public interface ICommandHandler<in TCommandData> 
{
    void Execute(TCommandData commandData);
}
Now we could rewrite the WithdrawAmountCommand. One class contains and verifies the input data...
public class WithdrawAmountCommandData
{
    public WithdrawAmountCommandData(string username, int amount)
    {
        if (string.IsNullOrEmpty(username))
            throw new NullReferenceException("username");

        Username = username;
        Amount = amount;
    }

    public string Username { get; private set; }

    public int Amount { get; private set; }
}
...while the second class actually acts on the data.
public class WithdrawAmountCommandHandler : ICommandHandler<WithdrawAmountCommandData>
{
    private readonly IAccountWebservice _accountWebservice;

    public WithdrawAmountCommandHandler(
        IAccountWebservice accountWebservice)
    {
        _accountWebservice = accountWebservice;
    }

    public void Execute(WithdrawAmountCommandData data)
    {
        _accountWebservice.Invoke(data.Username, data.Amount);
    }
}
Testing commands in isolation is now straight-forward.
[TestMethod()]
public void Execute_should_invoke_webservice_with_correct_arguments()
{
    var accountWebService = new Mock<IAccountWebservice>();

    var command = new WithdrawAmountCommandHandler(accountWebService.Object); 
    command.Execute(new WithdrawAmountCommandData("JefClaes", 25));

    accountWebService.Verify(aws => aws.Invoke("JefClaes", 25));
}
Don't forget that the controller also needs to accommodate this change.
public class HomeController : Controller
{
    private readonly IQueryHandler _qryHandler;
    private readonly ICommandHandler<WithdrawAmountCommandData> _withdrawAmountCommandHandler; 

    public HomeController()
    {
        _qryHandler = new QueryHandler();
        _withdrawAmountCommandHandler = new WithdrawAmountCommandHandler(new AccountWebservice());
    }

    public HomeController(
        IQueryHandler qryHandler,
        ICommandHandler<WithdrawAmountCommandData> withdrawAmountCommandHandler)
    {
        _qryHandler = qryHandler;
        _withdrawAmountCommandHandler = withdrawAmountCommandHandler;
    }

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

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

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

        return RedirectToAction("Index");
    }
}
I'm not all that happy with how this approach will bloat my controller in the future to be honest. To smooth out this friction, I could - like in the previous post - introduce a dispatcher, which serves as an intermediary for the commandhandlers.

Integration testing wasn't an option in this scenario, so I had to introduce some extra abstractions just to facilitate unit testing. Although this particular variation still holds lots of advantages I talked about in my previous post, there's already a lot more boilerplate and some more complexity.

I'm going to give this topic some more thought. Care to share yours?

9 comments:

  1. if you use a "Dispatcher" or something alike, you don't need to worry about the specific dependencies. The only thing you're interested in is the fact that your command or query is correctly dispatched.

    You can use a base class for your controller tests to setup the Dispatcher dependency if you like.



    ReplyDelete
    Replies
    1. You're right of course. I first planned on writing an extra blogpost on that, but it actually fits better in this one.

      Delete
  2. I agree with using a Dispatcher to hide the Handlers from your Controller. Otherwise the Controllers could end up getting a lot of dependencies and isn't that what you want to avoid when limiting your abstractions?

    However, doesn't introducing Handlers bloat your solution? Every command now requires you to implement two classes that cannot live without each other. A command without a handler is never executed and a handler without a command is never called. A command can only be used by one type of handler and a handler can only consume one type of command.
    Is the only reason for splitting them not to do awkward stuff like property injection?

    ReplyDelete
    Replies
    1. Otherwise the Controllers could end up getting a lot of dependencies and isn't that what you want to avoid when limiting your abstractions? => Yes, in reality it still has lots of dependencies, but with a dispatcher they're easier to manage, I think.

      Is the only reason for splitting them not to do awkward stuff like property injection? => I'm afraid so. I don't really want to give up that discoverability that CDI brings.

      Delete
    2. While I agree that property injection is a bit too magical, I'm also not comfortable with splitting the Execute method is its own class. That just seems to add so much weight to the solution.

      Delete
    3. What would you eventually decide on then?

      Delete
    4. I'm leaning towards property injection. You can explain that once (if necessary in xml comments on the Handler/Dispatcher). Writing Command/Handler combo's you have to do again and again (and again).

      Delete
  3. Here's how we do it in MyGet.org:
    - Controllers send ICommand or IEvent implementations on an IBus
    - The IBus implementation dispatches these to IHandler or IHandler
    - Commands are always handled by 1 IHandler
    - Events can be handled my multiple IHandler

    This allows us to, when someone uploads a package, issue an AddPackageCommand which is handled by AddPackageCommandHandler. AddPackageCOmmandHandler then issues an PackageAddedEvent on the IBus.
    We then have a couple of handlers listening for that one:
    - UpdateStatistics (writing some stats to our datastore)
    - ActivityLogWriter (which writes several event types to an activity log)

    Commands and events only contain primitive types (no DI is needed there), our IBus implementation instantiates the handlers through a DI container which allows the handlers to use constructor parameter injection if needed.
    (speaking of DI: every handler runs in its own lifetime on thecontainer)

    ReplyDelete
    Replies
    1. Sounds good to me. I think you can't really comfortably avoid separating data and command when you have more things going on than just writing to one datastore.

      Delete