Register decorator with another dependency of same generic type

I had to do some investigation in the code base to see what was going one. You might call this a glitch in Simple Injector's implementation, but it's IMO a fair trade off. Simple Injector's decorator sub system is based around the idea of working with open generic types and open generic decorators. The check it does upon decorator registration is to see if a decorator's constructor has only one decoratee. This check is done using the open generic abstraction to which the decorator must be applied; in your case ICommandHandler<T>. Since at that point only the generic ICommandHandler<T> is available, two constructor parameters match this type.

It is possible to improve these pre-condition checks, but this is actually quite nasty, while the usefulness of this feature is very limited. It's limited because it's only useful for non-generic decorators. Take a look at the following decorator for instance:

public class GenericDecorator<TCommand> : ICommandHandler<TCommand> {
    public GenericDecorator(
        ICommandHandler<TCommand> decoratee,
        ICommandHandler<LoggingCommand> dependency)
    {
    }
}

This decorator is generic and allows you to apply it to any decorator, which is much more useful. But what happens when you resolve an ICommandHandler<LoggingCommand>? That would cause a cyclic dependency graph and Simple Injector will (obviously) not be able to create that graph and will throw an exception. It must throw, since the decorator will in that case have two ICommandHandler<LoggingCommand> arguments. The first will be the decoratee and will be injected with your Logger, and the second will be a normal dependency and will be injected with a GenericDecorator<LoggingCommand>, which is recursive of course.

So I would argue that the problem is in your design. In general I advice against composing command handlers out of other command handlers. The ICommandHandler<T> should be the abstraction that lies on top of your business layer that defines how the presentation layer communicates with the business layer. It's not a mechanism for the business layer to use internally. If you start doing this, your dependency configuration becomes very complicated. Here's an example of a graph that uses DeadlockRetryCommandHandlerDecorator<T> and a TransactionCommandHandlerDecorator<T>:

new DeadlockRetryCommandHandlerDecorator<MessageCommand>(
    new TransactionCommandHandlerDecorator<MessageCommand>(
        new MessageSender()))

In this case the DeadlockRetryCommandHandlerDecorator<T> and a TransactionCommandHandlerDecorator<T> are applied to the MessageSender command handler. But look what happens we apply your MessageLogger decorator as well:

new DeadlockRetryCommandHandlerDecorator<MessageCommand>(
    new TransactionCommandHandlerDecorator<MessageCommand>(
        new MessageLogger(
            new MessageSender(),
            new DeadlockRetryCommandHandlerDecorator<MessageLogger>(
                new TransactionCommandHandlerDecorator<MessageLogger>(
                    new Logger())))))

Notice how there's a second DeadlockRetryCommandHandlerDecorator<T> and a second TransactionCommandHandlerDecorator<T> in the object graph. What does it mean to have a transaction in a transaction and have a nested deadlock retry (within a transaction). This can cause serious reliability problems in your application (since a database deadlock will cause your operation to continue in a transaction-less connection).

Although it is possible to create your decorators in such way that they are able to detect that they are nested to make them work correctly in case they're nested, this makes implementing them much harder and much more fragile. IMO that's a waste of your time.

So instead of allowing command handlers to be nested, let command handlers and command handler decorators depend upon other abstractions. In your case, the problem can be easily fixed by changing your decorator by letting it use an ILogger interface of some sort:

public class MessageLogger : ICommandHandler<MessageCommand> {
    private ICommandHandler<MessageCommand> innerHandler;
    private ILogger logger;

    public MessageLogger(
        ICommandHandler<MessageCommand> innerHandler, ILogger logger) {
        this.innerHandler = innerHandler;
        this.logger = logger;
    }

    public void Execute(MessageCommand command) {
        innerHandler.Execute(command);

        logger.Log(command.Message);
    }
}

You can still have an ICommandHandler<LogCommand> implementation in case the presentation layer needs to log directly, but in that case that implementation can simply depend on that ILogger as well:

public class LogCommandHandler : ICommandHandler<LogCommand> {
    private ILogger logger;

    public LogCommandHandler(ILogger logger) {
        this.logger = logger;
    }

    public void Execute(LogCommand command) {
        logger(string.Format("Message \"{0}\" sent at {1}",
            command.LogMessage, DateTime.Now));
    }
}

This is an edge case that you could possibly argue either way but the fact is that Simple Injector explicitly does not support what you are trying to do.

A decorator would normally be required to apply common logic across all (or some) of a particular abstraction, which in your example is ICommandHandler. In other words MessageLogger is designed to decorate ICommandHandler's and as it is a decorator of ICommandHandler's it can only take one ICommandHandler in it's constructor. In addition, allowing something like this would require reams of horrible circular checks that are best avoided with a cleaner design!

As such you would normally define a decorator with the same interface (and generic parameters) as the type it is decorating

public class MessageLogger<TCommand> : ICommandHandler<TCommand>
    where TCommand : <some criteria e.g. MessageCommand>
{
    //....
}

The first solution I can think of to mitigate your problem is create a mediator to remove the direct dependency:

public class LoggerMediator
{
    private readonly ICommandHandler<LogCommand> logger;

    public LoggerMediator(ICommandHandler<LogCommand> logger)
    {
        this.logger = logger;
    }

    public void Execute(LogCommand command)
    {
        this.logger.Execute(command);
    }
}

And change your MessageLogger to use the mediator.

public class MessageLogger<TCommand> : ICommandHandler<TCommand>
    where TCommand : MessageCommand
{
    private ICommandHandler<TCommand> innerHandler;
    private LoggerMediator logger;

    public MessageLogger(
        ICommandHandler<TCommand> innerHandler,
        LoggerMediator logger)
    {
        this.innerHandler = innerHandler;
        this.logger = logger;
    }

    public void Execute(TCommand command)
    {
        innerHandler.Execute(command);

        var logCommand = new LogCommand
        {
            LogMessage = command.Message,
            Time = DateTime.Now
        };
        logger.Execute(logCommand);
    }
}

BTW you can simplify your registrations like this

var container = new Container();
container.RegisterManyForOpenGeneric(
    typeof(ICommandHandler<>), 
    typeof(ICommandHandler<>).Assembly);
container.Register<LoggerMediator>();
container.RegisterDecorator(typeof(ICommandHandler<>), typeof(MessageLogger<>));
container.Verify();

---

UPDATE

Looking through my code base here I have found that I have had a similar requirement and I resolved it with one extra class - a generic command mediator:

public class CommandHandlerMediator<TCommand>
{
    private readonly ICommandHandler<TCommand> handler;

    public CommandHandlerMediator(ICommandHandler<TCommand> handler)
    {
        this.handler = handler;
    }

    public void Execute(TCommand command)
    {
        this.handler.Execute(command);
    }
}

registered like this:

container.RegisterOpenGeneric(
    typeof(CommandHandlerMediator<>), 
    typeof(CommandHandlerMediator<>));

and referenced like this:

public class MessageLogger<TCommand> : ICommandHandler<TCommand>
    where TCommand : <some criteria e.g. MessageCommand>
{
    private ICommandHandler<TCommand> decorated;
    private CommandHandlerMediator<LogCommand> logger;

    public MessageLogger(
        ICommandHandler<TCommand> decorated,
        CommandHandlerMediator<LogCommand> logger)
    {
        this.innerHandler = innerHandler;
        this.logger = logger;
    }

    //....

}

One new class and you're sorted for all of your handlers.