ipjohnson / Grace

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

ILocatorService.Locate using 'withKey' doesn't work for wrapped types. #185

Closed eglauko closed 6 years ago

eglauko commented 6 years ago

I have a class named OptionalWrapperStrategy (for Optional) extending BaseWrapperStrategy, and implementing IMissingExportStrategyProvider, IMissingDependencyExpressionProvider.

I can inject Optional with keys in properties, construtor parameters, and methods parameters.

If i try locate Optional using ILocatorService.Locate works fine, but if i try use 'withKey' an exception is thrown, and the methods of OptionalWrapperStrategy are not called.

ipjohnson commented 6 years ago

Could you include your optional wrapper implementation and I can take a look this evening?

eglauko commented 6 years ago

Optional.zip

ipjohnson commented 6 years ago

I've been able to replicate this problem in my environment so I can fix it. The one thing I will say is that this resolution won't be fast. Grace only caches delegates for non keyed exports, which wouldn't be a problem because it would be cached at the export itself but it's a wrapped/keyed export which won't be cached.

I can fix the problem for version 6.4.1 but I won't be fast. I think caching will need to be a different issue and I'd want to release version 6.5.0 for it as it's a major feature.

eglauko commented 6 years ago

Let me ask something: The IMissingDependencyExpressionProvider is used when i call the locate method?

My wrapper implements also the IMissingDependencyExpressionProvider, and I'm configuring this way:

var container = new DependencyInjectionContainer(GraceDynamicMethod.Configuration());
container.Configure(c =>
{
    var optionalStrategy = new OptionalWrapperStrategy(c.OwningScope);
    c.AddActivationStrategy(optionalStrategy);
    c.AddMissingDependencyExpressionProvider(optionalStrategy);
});

I realized that the IMissingDependencyExpressionProvider methods were not called from locate. Maybe using this strategy would solve the problem.

ipjohnson commented 6 years ago

@eglauko you are correct the IMissingDependencyExpressionProvider methods weren't being called because you were registering it as a strategy all that said I think you've stumbled on a bug that is going to be hard to fix without making a change to the IWrapperOrExportActivationStrategy interface.

After doing some digging the problem is going to affect all Wrapped/Keyed Locate requests. Essentially the GetActivationStrategyDelegate method needs to have key added to it but this will be a breaking change that I don't want to introduce in the 6.X series.

Now to the bad news I don't think I'm going to be able to start Grace 7 till next year because of a number of different family and work related obligations. This will most definitely make the list as well as caching keyed delegates but I just can't start the work now.

ipjohnson commented 6 years ago

@eglauko I think this solution will work for the moment and for 7 I'll remove the interface and add the object key parameter to IWrapperOrExportActivationStrategy

eglauko commented 6 years ago

Yes, I understood what you wanted to do and I created a solution to solve this problem without creating any impact on the current version.

I thought you could create another cache in the BaseExportLocatorScope class, similar to the ActivationDelegates cache, for delegates with type and key. And in the locate method of the InjectionScope class you could create and search for these keyed delegates.

eglauko commented 6 years ago

Could you release a preview version with this PR?

ipjohnson commented 6 years ago

I can do that this evening.

eglauko commented 6 years ago

100% of my tests passed. Super cool.

ipjohnson commented 6 years ago

I'm pushing out a release this evening for this. Glad it worked out