ipjohnson / Grace

Grace is a feature rich dependency injection container library
MIT License
336 stars 33 forks source link

How to register decorator classes? #149

Closed davidkeaveny closed 6 years ago

davidkeaveny commented 6 years ago

If I have an abstract base class CommandHandler<T>, and I want to register a decorator class LoggingCommandHandler<T> such that every class that implements the base class is resolved with the decorator class, how do I tell Grace to achieve such a thing?

How about if I want to add another decorator class ValidatingCommandHandler<T> and I want the order of the decorator classes to be preserved? i.e. always resolve as:

LoggingCommandHandler
--> ValidatingCommandHandler
----> ConcreteCommandHandler
ipjohnson commented 6 years ago

Hi @davidkeaveny

I took a quick look and out of the box you can't do what you want to do but I think it's possible to do it with a C# extension.

I'll write up an example of how to do it and post a gist of it.

I also think this is a worth feature request so I'll see about making it part of Grace officially so you don't have to do an extension.

ipjohnson commented 6 years ago

@davidkeaveny did the unit test for dectorators work for you? Also what are your thoughts on C# extension vs a member on the interface?

davidkeaveny commented 6 years ago

It works flawlessly 👍

Would the extension method basically replace the .When.MeetsCondition(new WhenDecoratingCondition(typeof(BaseCommand<>)))? It would certainly clean up the registration process a little more, but it's not essential.

davidkeaveny commented 6 years ago

Hmm, maybe I jumped the gun a little bit - the first decorator that I register works flawlessly (the LoggingCommandHandlerDecorator<> in my example) and all my command handlers are now logged; but when I then add the registration for another decorator (the ValidatingCommandHandlerDecorator<>) then Grace fails to resolve the type.

I've tried switching it round, and get the same result - register one decorator, all is good; register two decorators, all is bad.

The error that Grace throws is:

System.ArgumentNullException: Value cannot be null.
Parameter name: type
   at System.Reflection.IntrospectionExtensions.GetTypeInfo(Type type)
   at Grace.DependencyInjection.Impl.Expressions.ActivationExpressionBuilder.GetValueFromRequest(IInjectionScope scope, IActivationExpressionRequest request, Type activationType, Object key)
   at Grace.DependencyInjection.Impl.Expressions.ActivationExpressionBuilder.GetActivationExpression(IInjectionScope scope, IActivationExpressionRequest request)
   at Grace.DependencyInjection.Impl.Expressions.ConstructorExpressionCreator.GetParameterExpression(ParameterInfo parameter, ConstructorParameterInfo parameterInfo, IInjectionScope injectionScope, TypeActivationConfiguration configuration, IActivationExpressionRequest request)
   at Grace.DependencyInjection.Impl.Expressions.ConstructorExpressionCreator.GetParameterExpressionsForConstructor(IInjectionScope injectionScope, TypeActivationConfiguration configuration, IActivationExpressionRequest request, ConstructorInfo constructor)
   at Grace.DependencyInjection.Impl.Expressions.ConstructorExpressionCreator.CreateExpression(IInjectionScope scope, IActivationExpressionRequest request, TypeActivationConfiguration activationConfiguration)
   at Grace.DependencyInjection.Impl.Expressions.InstantiationExpressionCreator.CreateExpression(IInjectionScope scope, IActivationExpressionRequest request, TypeActivationConfiguration configuration)
   at Grace.DependencyInjection.Impl.Expressions.TypeExpressionBuilder.GetActivationExpression(IInjectionScope scope, IActivationExpressionRequest request, TypeActivationConfiguration activationConfiguration)
   at Grace.DependencyInjection.Impl.Expressions.DefaultStrategyExpressionBuilder.GetActivationExpression(IInjectionScope scope, IActivationExpressionRequest request, TypeActivationConfiguration activationConfiguration, ICompiledLifestyle lifestyle)
   at Grace.DependencyInjection.Impl.CompiledStrategies.GenericCompiledDecoratorStrategy.GetDecoratorActivationExpression(IInjectionScope scope, IActivationExpressionRequest request, ICompiledLifestyle lifestyle)
   at Grace.DependencyInjection.Impl.Expressions.DecoratorActivationPathNode.GetActivationExpression(IInjectionScope scope, IActivationExpressionRequest request)
   at Grace.DependencyInjection.Impl.Expressions.ActivationExpressionBuilder.CreateDecoratedActivationStrategy(IInjectionScope scope, IActivationExpressionRequest request, ICompiledExportStrategy strategy, List`1 decorators)
   at Grace.DependencyInjection.Impl.Expressions.ActivationExpressionBuilder.DecorateExportStrategy(IInjectionScope scope, IActivationExpressionRequest request, ICompiledExportStrategy strategy)
   at Grace.DependencyInjection.Impl.CompiledStrategies.CompiledExportStrategy.GetActivationExpression(IInjectionScope scope, IActivationExpressionRequest request)
   at Grace.DependencyInjection.Impl.CompiledStrategies.CompiledExportStrategy.GetActivationStrategyDelegate(IInjectionScope scope, IActivationStrategyCompiler compiler, Type activationType)
   at Grace.DependencyInjection.Impl.ActivationStrategyCompiler.LocateStrategyFromCollectionContainers(IInjectionScope scope, Type locateType, ActivationStrategyFilter consider, Object key, IInjectionContext injectionContext)
   at Grace.DependencyInjection.Impl.ActivationStrategyCompiler.FindDelegate(IInjectionScope scope, Type locateType, ActivationStrategyFilter consider, Object key, IInjectionContext injectionContext, Boolean checkMissing)
   at Grace.DependencyInjection.Impl.InjectionScope.InternalLocate(IExportLocatorScope scope, IDisposalScope disposalScope, Type type, ActivationStrategyFilter consider, Object key, IInjectionContext injectionContext, Boolean allowNull, Boolean isDynamic)
   at Grace.DependencyInjection.Impl.InjectionScope.Locate(Type type)
   at Grace.DependencyInjection.Impl.InjectionScope.Locate[T]()
ipjohnson commented 6 years ago

Hmm, I can see the piece of code that must have failed to end up in that situation (it's having an issue building the closed generic type for activation).

Would it be possible for you to create a small sample using your types that you want to decorate and the decorators that are failing? You can remove the logic from the classes as I think it's probably some corner case of generics that I'm missing but I'm not able to create it using my simple test cases.

davidkeaveny commented 6 years ago

I've prepared a repro - the bug appears if there are multiple decorators registered, so if you comment out one or other of the decorator registrations the test passes.

I also modified the decorators so that they had no other dependencies (apart from the ICommandHandler<TCommand> obviously) and then the bug disappears, even with both decorators registered.

cqrs-decorator.zip

ipjohnson commented 6 years ago

Thanks @davidkeaveny that was very helpful. I was able to replicate the problem and fix it. I should be able to find some time this weekend to look at the other generic issue you raised but I'm not sure what/if there is a fix for it.

Either way I should be able to get a new version out for you this weekend. In the mean time if you want you can try the nightly build at https://ci.appveyor.com/nuget/grace-master

ipjohnson commented 6 years ago

I've released version 6.3.3 to address this issue and I'm marking it closed

davidkeaveny commented 6 years ago

Looks great, @ipjohnson , thanks!