Conditionally registering decorators

[alexmg]

From [email protected] on Google Group:

https://groups.google.com/forum/#!topic/autofac/svtRInh0my4

Originally, you helped me out with getting all the decorators attached using open generics here. https://groups.google.com/forum/#!searchin/autofac/brantley/autofac/lV2X4hR0mak/F5g4MfhUR08J
That has been working great and I modified the code so you could pass in a list of decorators to apply (decorators decorating decorators). For completeness here is that code (in case someone wants it):

    public static IRegistrationBuilder<TLimit, TScanningActivatorData, TRegistrationStyle> AsClosedTypesOfWithDecorators<TLimit, TScanningActivatorData, TRegistrationStyle>( 
        this IRegistrationBuilder<TLimit, TScanningActivatorData, TRegistrationStyle> registration, 
        ContainerBuilder builder, Type commandType, params Type[] decorators) where TScanningActivatorData : ScanningActivatorData 
    { 
        if (commandType == null) throw new ArgumentNullException(nameof(commandType)); 
        if (decorators == null || decorators.Length == 0) 
            return registration.AsClosedTypesOf(commandType); 

        string actualHandler = Guid.NewGuid().ToString(); 
        var returnValue = registration 
            .AsNamedClosedTypesOf(commandType, t => actualHandler); 

        string lastHandler = actualHandler; 
        var lastDecorator = decorators.Last(); 
        foreach (var decorator in decorators) //take all but last decorator...it cannot be keyed.. 
        { 
            var newKey = Guid.NewGuid().ToString(); 
            var decoratorRegistration = builder.RegisterGenericDecorator( 
                decorator, 
                commandType, lastHandler); 
            if (decorator != lastDecorator) //cannot key the last decorator... 
                decoratorRegistration.Keyed(newKey, commandType); 
            lastHandler = newKey; 
        } 

        return returnValue; 
    } 

Anyway, the big issue at hand is conditionally registering decorators. This is a shortcoming I have been working around for a while and last April you said may focus on it after DNX settle down (https://groups.google.com/forum/#!searchin/autofac/decorator/autofac/vSU6kx7XX2M/AxhpHqddGzkJ)

However, I now have a really wild situation that I cannot resolve.

1. I register all my query/commands with decorators using the above code. All good.
2. I hand register each query/command SECOND time which effectively hides the first registration. On this I use the non-open generic RegisterType<> and RegisterDecorator<> to filter out the decorators I do not want for these specific commands/queries.

This seems to work great. I open my web application and sure enough the new registration is being applied and there are no issues.

The issue is that if I do a WEBAPI request - it continues to use the original registration and not the new one!! I have pulled my hair out on this issue.

Any ideas?

[waynebrantley]

Glad you are interested in enhancing support for this! My most complex AutoFac code is around proper decorator support.

I ended up iterating through all my types to do the decorator registration.
With CQRS, you have a generic like IQuery and a handler like IQueryHandler<QT, T>.
The T is the type the query returns, QT is the IQuery. I am sure you are familiar with the pattern.

Here is an example issue. I create attribute, QueryCache and apply that attribute to all my IQuery<> definitions that I want Cached. I then create a Cache decorator to do the caching.

When it comes time to register, I want to do the following:

• Register every IQueryHandler<,> for each type of IQuery<>.
• Decorate every one of the handlers if the IQuery for the handler has QueryCache attribute.

Or I might use an interface. If I add the IUnitOfWork interface to a Command, then I want the command handler for that Command to be decorated only if the Command implements that interface.

And so on...What I did was

• go through all the types in the assembly that were was a closed type of IQuery<> and put in commands
• go through all types in the assembly that were closed type of IQueryHandler<> and put in handlers
• for each item in handlers, I located the query that was the generic for it from commands. While doing this location I threw exception if I could not find one. (making sure all CQRS was setup right). I then returned a new class to hold all the information (handler, command, hasCacheAttribute, hasUnitOfWork, etc). That left me with an array of objects that contained exactly what I need to do.
• for each possible combination of decorator possibilities, located objects in the final array above that meet those critera and register each one using the AsClosedTypesOfWithDcorators posted above.

It is a bunch of manual code. I am not sure how you would provide support for this. But, essentially I would want to see something like this:

builder.RegisterTypesWithDecorators(typeof(IQueryHandler<,>), (handler, genericArg1Type, genericArg2Type) => someCode();)

The someCode() would be able to look at the types that were provided to the handler above and determine exactly what decorators to apply. Here is an example of what that might look like.

builder.RegisterTypesWithDecorators(typeof(IQueryHandler<,>), (handler, genericArg1Type, genericArg2Type) => {
    if (genericArg1Type.GetCustomAttribute<QueryCacheAttribute>())
      AddDecoratorToHandler(handler, typeof(QueryHandlerCacheDecorator<,>);
    if (genericArg1Type.IsAssignableFrom((typeof(IUnitOfWork)))
      AddDecoratorToHandler(handler, typeof(QueryHandlerUnitOfWorkDecorator<,>);
});

The way decorators are applied now is a bit messy. You have to create a key for each level and then use that key for the next level....but you have to know in advance if there is a next level so you can not key it...or it will not work. Note in the code above, the need for the ability to just add a decorator without worrying about if it is the last decorator in the chain...or what the prior decorator was.

Another optional api...perhaps you register it like you do today...then the decorator is applied like:

 builder.RegisterGenericDecorator( typeof(QueryHandlerCacheDecorator<,>), typeof(IQueryHandler<,>), (handler, genericArg1Type, genericArg2Type) => FilterExpression());

With this you can register decorators on top of whatever decorator was there for everything that was originally registered as IQueryHandler<,> - but only if the FilterExpression() return true. This would allow me to easily pick which query gets this particular decorator.

Note genericArg1Type, genericArg2Type would probably have to be an array of types for the number of parameters in the open generic type...

[waynebrantley]

Reviewed my implementation in detail again. One of the biggest reasons I cannot make my custom registration code better is around the decorator keys.

If I could register the main handler for the query...not knowing if it will be decorated or not.
Then resolve the 'current outermost decorator/handler' for a query and 'add a decorator', not knowing if it will be decorated again or not.
etc.

This would make a big difference.

[waynebrantley]

Looking around, simple injector does exactly what is needed.
https://simpleinjector.readthedocs.org/en/latest/advanced.html#decorators

container.RegisterDecorator(
    typeof(ICommandHandler<>),
    typeof(AccessValidationCommandHandlerDecorator<>),
    context => !context.ImplementationType.Namespace.EndsWith("Admins"));

[waynebrantley]

Is there anyway in the current autofac, to register the open generic without a 'keyed registration', then when I want to decorate it - locate all those unkeyed registrations - key them, then register the decorator to that key. Then, if I wanted to decorate that decorator, I would again get all the 'unkeyed registrations' and key them, etc. That would be huge. (Just trying to find a solution that works with the current architecture)

[worldspawn]

I wonder if you could use resolved parameters to customise the decorated parameter to the decorator. http://docs.autofac.org/en/latest/faq/select-by-context.html#option-4-use-metadata

You could inspect the generic parameter of the type being decorated to see if it implemented a particular interface or whatever and provide a special service to handle that or another one if it didn't.

[waynebrantley]

No need to do that - pretty complex and runtime decision making.

Basically I just do my own reflection to workout all the types and relationships so I can decorate giving the autofac constraints. So, this can be done - but just not with the existing autofac api.

[worldspawn]

Yeah that would probably be easier on the brain :)

[waynebrantley]

@alexmg Any update on this now that DNX has settled down?

[worldspawn]

So @waynebrantley I've just come back to this :) Using your reflection solution are you stuck having to create a unique stack for every possible combination of decorators?

Say you had IFoo, IBar and IMoo and each of those interfaces had a corresponding decorator to do something would you have to create a decorator stack for foo, bar and moo. then foo + bar, foo + moo, bar + moo, foo + bar + moo etc? 🐙

[waynebrantley]

@worldspawn Yes..you are! It is due to the way they wire up the decorators. Hoping for a better solution from them...dont really want to switch DI as that is a big pain.

[worldspawn]

Yeah that's pretty god awful. It looks like this could be done by writing an IRegistrationSource. (http://docs.autofac.org/en/latest/advanced/registration-sources.html) I've been browsing the current implementation and it is using ParameterResolvers.

However its a pretty serious learning curve. The code is so heavily abstracted its extremely hard to see whats going on. Its going be one of those trial and error code sessions :D I ran into a wall pretty quickly where the current code gets the implementing type of the service type by reading the registrations (or something). Its all internal so I cant use it.

An extension method that accepted a list of decorator types and optionally a predicate (per type) and the base open generic type that is being decorated is what I'd like to see. I think it needs to be a single call because the paramter resolver will need to do something like check if the decorator is allowed and if not move on to the next decorator and inject that instead.

[snboisen]

Having to name decorated objects in Autofac is in my opinion its greatest shortcoming. It messes with proper modularization of the registration code and prevents easy testing of partial container setups.

It would be absolutely wonderful to have a proper decorator API, where we could register objects without caring about whether those objects will later be decorated. The same goes for decorators themselves, their registration should not depend on whether further decorators will be applied.

[waynebrantley]

@snboisen I agree, @alexmg Can you advise if this is on the roadmap?

[tillig]

We're pushing to enhance decorator syntax and features, which will hopefully resolve this issue. I've created a larger meta-issue for tracking that at #880. In the meantime, I'll close this specific issue and hopefully we can handle everything at once.