ipjohnson / Grace

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

StaticInjectionContext preservation #129

Closed darkcamper closed 6 years ago

darkcamper commented 6 years ago

Hi @ipjohnson , I'm back from my late summer vacation with a ton of pending work, and a question about Grace:

I have the following situation (simplified):

  1. Two classes A and B. A's constructor has a paremeter of type B (which might be decorated with SomeAtrribute)
  2. An ExportFactory that based on some stuff ends up doing a Locate<A>(withKey : someKey)
  3. An IActivationStrategyInspector which adds an EnrichmentDelegate to B's activation. The delegate says something in the lines of "if the target of activation has a SomeAttribute attached to it, perform some operations on the B instance being injected"

The problem is that the enrichment delegate of step 3 gets a StaticInjectionContext who doesn't have the original information (I know it's the correct behavior, be because i'm doing a scope.Locate at step 2).

So the question is: is there any way of preserving the original StaticInjectionContext in the call to Locate at step 2? If there is no way to do that, do you have any idea how could I get a simmilar result?

I've prepared a simple runnable example here

Thanks!

ipjohnson commented 6 years ago

@darkcamper at this point I don't know of a good way to link the two static context together automatically. What could be done is to stick the StaticInjectionContext instance into the Locate<OneDependency> call and then fetch it out of the injection context in the enrichment. It's definitely a little hacky.

cfg.ExportFactory((IExportLocatorScope sc, StaticInjectionContext ctx) =>
{
    //There would be some magic to get the key name...
   return sc.Locate<OneDependency>(new { StaticContext = ctx },  withKey: sc.ScopeName);
}).As<OneDependency>();

Then in DoSomethingToEnrich you can fetch the context out of the injection context like so

(StaticInjectionContext)injContext.GetExtraData("StaticContext");

I think the much more elegant solution will be to allow configuration of the ExportFactory to allow specifying a key

darkcamper commented 6 years ago

@ipjohnson I tried passing the original context inside the extraData parameter of Locate before, but then inside DoSomethingToEnrich, "injContext" is empty (it's not null but it contains no extradata at all). Is this the expected behavior?

I think the much more elegant solution will be to allow configuration of the ExportFactory to allow specifying a key

Wouldn't that be a bit against the idea of ExportFactory, that allows us to use almost any logic to decide how to activate the instance when it's requested?

ipjohnson commented 6 years ago

@darkcamper Hmmm that is not the expected behavior. I'll try and put together an example of what I'm thinking today.

As to your question about it going against the idea of ExportFactory. I see it a bit differently ExportFactory allows the developers to define logic any way you want, but it also allows the developer to specify what dependencies it needs to perform this logic.

In the example above you are saying for the ExportFactory logic to function it needs two dependencies the scope and the static injection context. You can just as easily add a third parameter to the method that requires interface IFoo and an implementation for IFoo will be located and passed in. At this point there is no control over which IFoo to use.

All that said the more I think about it the more I realize for your specific situation it actually won't work as the key you want to locate using isn't known until each request.

darkcamper commented 6 years ago

@ipjohnson

You can just as easily add a third parameter to the method that requires interface IFoo and an implementation for IFoo will be located and passed in. At this point there is no control over which IFoo to use.

Now I understand what you meant, and I fully agree.

All that said the more I think about it the more I realize for your specific situation it actually won't work as the key you want to locate using isn't known until each request.

That's true. I'm my case I have to choose the key in runtime because it's a workaround to avoid problems using mutiple Lifestyle.SingletonPerNamedScope for the same exported type (you gave me the tip and it's worked like a charm). But anyway you never know what kind of twisted logic are the users going to write in their factories.

ipjohnson commented 6 years ago

@darkcamper Ok I stepped through the code and figured out why the context is empty. It turns out I actually designed it that way.

My original thinking was that when requesting exports with Singleton, SingletonPerScope, and SingletonPerNamedScope no context data is passed through because you could create a condition where order of calls dictated how the export is constructed (i.e. conditional logic of ancestors could cause different exports to be used depending on order of calls).

Maybe it makes sense to off a configuration flag to allow injection context to be passed through.

ipjohnson commented 6 years ago

@darkcamper do you want me to add a flag for SingletonPerScope and SingletonPerNamedScope that let's you pass context in?

darkcamper commented 6 years ago

@ipjohnson Yes, please. I think that would be enough to solve this issue. Although sometime in the future I'd love to see a way to call Locate and preserve the scope (or just being able to ""forge"" a request) :smile:

ipjohnson commented 6 years ago

@darkcamper while I can see the desire to do that it's a harder task than one might think as the static contexts are created when the delegate is created and they are readonly by design. While Grace can imitate most of the behavior of a dynamic container it is at it's heart a compiled linq expression container.