This project is read-only.

Trouble with Generic decorators ....

Coordinator
Dec 3, 2014 at 12:34 PM
Edited Dec 3, 2014 at 12:36 PM
I have a standard command:
sealed class ReassignWorkItemHandler : ICommandHandler<ReassignWorkItem>
{
    private readonly IServiceClient service;

    public ReassignWorkItemHandler(IServiceClient service)
    {
        this.service = service;
    }

    public void Execute(ReassignWorkItem command)
    {
        var request = new Request
        {
            //...
        };

        Response response = this.service.ReassignWorkItem(request);
    }
}
But I have a special case where a certain status cannot be reassigned: to get around this limitation I have to change the status, reassign and then change it back. So I made a second handler:
sealed class ReassignPendingDeliveryHandler : ICommandHandler<ReassignPendingDelivery>
{
    private readonly ICommandHandler<UpdateWorkItem> updateWorkItem;
    private readonly ICommandHandler<ReassignWorkItem> reassignWorkItem;

    public ReassignPendingDeliveryHandler(
        ICommandHandler<UpdateWorkItem> updateWorkItem,
        ICommandHandler<ReassignWorkItem> reassignWorkItem)
    {
        this.updateWorkItem = updateWorkItem;
        this.reassignWorkItem = reassignWorkItem;
    }

    public void Execute(ReassignPendingDelivery command)
    {
        this.updateWorkItem.SetStatus(
            command.WorkItemId, 
            WorkItemStatus.Active);

        this.reassignWorkItem.Execute(
            command.WorkItemId,
            command.UserName);

        this.updateWorkItem.SetStatus(
            command.WorkItemId, 
            WorkItemStatus.Pending);
    }
}
All good - looking again I realised that, "hey this should be a decorator":
sealed class ReassignPendingDeliveryDecorator : ICommandHandler<ReassignWorkItem>
{
    private readonly IQueryHandler<GetWorkItem, WorkItem> getWorkItem;
    private readonly ICommandHandler<ReassignWorkItem> decorated;
    private readonly ICommandHandler<UpdateWorkItem> updateWorkItem;

    public ReassignPendingDeliveryDecorator(
        IQueryHandler<GetWorkItem, WorkItem> getWorkItem,
        ICommandHandler<ReassignWorkItem> decorated,
        ICommandHandler<UpdateWorkItem> updateWorkItem)
    {
        this.getWorkItem = getWorkItem;
        this.updateWorkItem = updateWorkItem;
        this.decorated = decorated;
    }

    public void Execute(ReassignWorkItem command)
    {
        var status = this.getWorkItem.Execute(command.Id).status;

        if (status == WorkItemStatus.Pending)
        {
            this.updateWorkItem.SetStatus(command.Id, WorkItemStatus.Active);
        }

        this.decorated.Execute(command);

        if (status == WorkItemStatus.Pending)
        {
            this.updateWorkItem.SetStatus(command.Id, WorkItemStatus.Pending);
        }
    }
}
… but I can't do this because I need to use an additional ICommandHandler<> in a decorator of ICommandHandler<>
.
  1. Is there really no way for me to have this (it is a valid scenario)?
  2. The reported error is misleading
System.ArgumentException: For the container to be able to use ReassignPendingDeliveryDecorator as a decorator, its constructor must include a single parameter of type ICommandHandler<ReassignWorkItem> (or Func<ICommandHandler<ReassignWorkItem>>) - i.e. the type of the instance that is being decorated. The parameter type ICommandHandler<ReassignWorkItem> is defined multiple times in the constructor of class ReassignPendingDeliveryDecorator.
Parameter name: decoratorType
Coordinator
Dec 3, 2014 at 12:51 PM
Decorators in Simple Injector are currently limited in having only one dependency of the same generic-interface. In theory it would be possible to open this up and let Simple Injector use the normal cyclic dependency verification to find out whether the situation is actually valid or not. Currently however it can't do this so this is a technical limitation of Simple Injector 2.6. The only way around this is by wrapping the non-decorated handler in a different class and inject this.

Functionally however, I find it very disturbing that you are wrapping command handlers in a different command handler (or decorator). In my (limited) view on this design, I see commands to be the definition of a atomic operation. Letting go of this vision, will make it often much harder to correctly apply cross-cutting concerns, because most of the decorators should only be applied to the outer command handler (for instance when applying a transaction cross-cutting concern). Instead, I always advice to extract common functionality that multiple handlers share into a shared service. This effectively solves many problems, while still allowing to apply cross-cutting concerns on the PL/BL boundary.
Coordinator
Dec 3, 2014 at 3:52 PM
I use the Command pattern slightly differently to your original posts - my commands are more an individual strategy than a full atomic operation (transaction), they are more akin to a Query - they do one thing and I may need many for a transaction.

Where necessary I will have another higher level abstraction for transaction level aspects.

For this particular solution each Command is one or more SOAP calls and the services do not support "transactional" changes - one call, one change ...
Coordinator
Dec 5, 2014 at 4:37 PM
Edited Dec 11, 2014 at 6:59 PM
I've worked around it like this
public class CommandHandlerProxy<T> : ICommandHandler<T> where T : ICommand
{
    private readonly ICommandHandler<T> handler;

    public CommandHandlerProxy(ICommandHandler<T> handler)
    {
        this.handler = handler;
    }

    public void Execute(T command)
    {
        this.handler.Execute(command);
    }
}
sealed class ReassignPendingDeliveryDecorator : ICommandHandler<ReassignWorkItem>
{
    private readonly IQueryHandler<GetWorkItem, WorkItem> getWorkItem;
    private readonly ICommandHandler<ReassignWorkItem> decorated;
    private readonly ICommandHandler<UpdateWorkItem> updateWorkItem;

    public ReassignPendingDeliveryDecorator(
        IQueryHandler<GetWorkItem, WorkItem> getWorkItem,
        ICommandHandler<ReassignWorkItem> decorated,
        CommandHandlerProxy<UpdateWorkItem> updateWorkItem)
    {
    //...
Coordinator
Jan 15, 2015 at 6:12 PM
I pushed v2.7.1-alpha1 to NuGet. This build removes the limitation on the decorators. You might want to try it out.
Coordinator
Jan 15, 2015 at 6:35 PM
Cool, I'll try it tomorrow (it's a work related project)


Coordinator
Jan 17, 2015 at 11:19 AM
This discussion has been copied to a work item. Click here to go to the work item and continue the discussion.
Coordinator
Jan 19, 2015 at 10:46 AM
Hi, I can confirm this change works fine and means I no longer require the Proxy wrapper.
Coordinator
Jan 19, 2015 at 11:59 AM
The change for this has been checked-in. You can see the changes here.
Coordinator
Feb 2, 2015 at 8:13 PM
The RTM of v2.7.1 has been released. This release removes the limitation.
Marked as answer by dot_NET_Junkie on 2/2/2015 at 12:14 PM