Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Context-based injection with dependencies like HttpClient - general case #668

Open
voroninp opened this Issue Mar 12, 2019 · 3 comments

Comments

Projects
None yet
2 participants
@voroninp
Copy link

voroninp commented Mar 12, 2019

I think, HttpClient is a good example of situation, when we need multiple instances of same type, but with different initialisation:

A -> Dependency -> SettingsForA
and
B -> Dependency -> SettingsForB

The problem is that settings functionally depend on the type which is not the immediate consumer of the settings.

Does Simple Injector have any functionality for this scenario?

Something like:

  • Register dependency A, provided that Settings is a.
  • Register dependency B, provided that Settings is b.
@dotnetjunkie

This comment has been minimized.

Copy link
Collaborator

dotnetjunkie commented Mar 12, 2019

I think, HttpClient is a good example of situation, when we need multiple instances of same type

As you might know, I disagree with that.

Does Simple Injector have any functionality for this scenario?

I discussed multiple times with other contributors the addition of a feature that would allow making contextual registrations that change based on the consumer’s consumer. We eventually decided not to add such feature, as it can easily lead to hard find bugs. This is actually not a problem that would be specific to Simple Injector, but is a rather universal problem.

To simply demonstrate the problem, let’s assume that Simple Injector’s RegisterConditional would allow querying for the consumer’s consumer, as with your example:

container.Register<A>();
container.Register<B>();
container.Register<IDependency, Dependency>(Lifestyle.Singleton);
// Here we query for Consumer.Consumer, which is hypothetical and not 
// supported by Simple Injector.
container.RegisterConditional<ISettings, SettingsForA>(
    c => c.Consumer.Consumer.ImplementationType == typeof(A),
    Lifestyle.Singleton);
container.RegisterConditional<ISettings, SettingsForB>(
    c => c.Consumer.Consumer.ImplementationType == typeof(B),
    Lifestyle.Singleton);

At first sight this example might look like it should just work, but it is actually impossible for A to have a SettingsForA and B have a SettingsForB, while at the same time having Dependency registered as Singleton. To construct such graph, the guarantee of Dependency being a Singleton must be broken, because one single Dependency instance can’t depend on both SettingsForA and SettingsForB.

The only DI Container that I know does support this kind of registration is Ninject, but if you run the Ninject registrations, you’ll notice that it fails.

var kernel = new StandardKernel();

kernel.Bind<A>().ToSelf();
kernel.Bind<B>().ToSelf();
kernel.Bind<IDependency>().To<Dependency>().InSingletonScope();

kernel.Bind<ISettings>().To<SettingsForA>()
    .When(r => r.ParentRequest.ParentRequest.Service == typeof(A));
kernel.Bind<ISettings>().To<SettingsForB>()
    .When(r => r.ParentRequest.ParentRequest.Service == typeof(B));

// This will output “SettingsForA” twice.
Console.WriteLine(((Dependency)kernel.Get<A>().Dependency).Settings.GetType().Name); 
Console.WriteLine(((Dependency)kernel.Get<B>().Dependency).Settings.GetType().Name);

Of course, the use of the Singleton lifestyle causes an exaggeration of the fact, but you will easily find the same problem when using the Scoped lifestyle. Furthermore, as most DI Containers optimize object construction by emitting and caching functions, the first resolved ISettings implementation would be burned into the graph even when the Transient lifestyle is used (as is what would happen with Simple Injector).

To work around this, what you can do in Simple Injector is instead making the middle dependency conditional. For instance:

container.Register<A>();
container.Register<B>();

container.RegisterConditional<IDependency>(
    Lifestyle.Singleton.CreateRegistration(
        () => new Dependency(new SettingsForA()), container),
    c => c.Consumer.ImplementationType == typeof(A));

container.RegisterConditional<IDependency>(
    Lifestyle.Singleton.CreateRegistration(
        () => new Dependency(new SettingsForB()), container),
    c => c.Consumer.ImplementationType == typeof(B));

This does however still not prevent you to have Dependency as true Singleton, because this is simply impossible.

Downside of this approach is that you’ll lose the ability to auto-wire your dependencies. You can, however, allow Simple Injector to look further up the consumer chain with the use of generic types:

container.Register<A>();
container.Register<B>();

// Here you change "Dependency" to a generic Dependency<T>" (e.g. by defining
// a Dependency<T> that derives from Dependency and place that inside the 
// Composition Root).
container.RegisterConditional(
    typeof(IDependency),
    c => typeof(Dependency<>).MakeGenericType(c.Consumer.ImplementationType),
    Lifestyle.Singleton,
    c => true);

// We know Consumer is Dependency<T> and its T is either A or B.
container.RegisterConditional<ISettings, SettingsForA>(
    c => c.Consumer.ImplementationType.GetGenericArguments().Single() == typeof(A),
    Lifestyle.Singleton);

container.RegisterConditional<ISettings, SettingsForB>(
    c => c.Consumer.ImplementationType.GetGenericArguments().Single() == typeof(B),
    Lifestyle.Singleton);
@voroninp

This comment has been minimized.

Copy link
Author

voroninp commented Mar 13, 2019

the guarantee ... must be broken

Well, it just means that this set of registrations is fundamentally invalid. And the question is whether verification in principle can reveal all bad configurations to prevent the issue:

as it can easily lead to hard find bugs.

Even now it is invaluable to be able to verify lifestyles - I just trust SI. And I'd be happy if it could do more ;-)

I would not limit it the problem to consumer of consumer. Generally it is a functional dependency:
dependency_k = f(dependency_n), where n > k and counting is in direction from leaves to the roots.

The only way I currently see is to make dependency_k stateless regarding aspects of behaviour conditioned by dependency_n and require all data at the place where action is requested:

class DepK
{
    public DepK(CommonConfig cfg) {}
    public DoSomething(DepNBehaviourRelatedConfig cfg) {}
}

class DepN
{
   private DepNMinusOne dep;
    void DoIt()
   {
       ...
       dep.DoSomething(cfg);       
   }
}

As you may see in this case cfg must be carried through the whole chain of dependencies just to be passed to method DoSomething of DepK. I do not think this design is better than the initial one.

Here you say:

Downside of this approach is that you’ll lose the ability to auto-wire your dependencies.

That's indeed another question I intended to ask. Can we have the best of two worlds?

class DepConsumer
{
    public DepConsumer(DepA a, DepB b, ..., DepN n) {}
}
// Here I register DepConsumer and ask SI to provide all the dependencies for it
// except that one for parameter b, which I construct manually.
container.Register<DepConsumer>(overrides: new [] { new DepB(...)} ); 

Yes, the correctness cannot be checked at compile time, but container verification could signal about broken registration.

P.S. I tend to look at DI container similar to how Mark talks about object graph here. So I see nothing terrible in defining application logic through it.

@dotnetjunkie

This comment has been minimized.

Copy link
Collaborator

dotnetjunkie commented Mar 14, 2019

I just trust SI. And I'd be happy if it could do more ;-)

Of course. For Simple Injector, though, it is not only the verification. Because of the way that object graph composition is burned down into compiled and reused methods, this has to be broken open, as such thing will currently almost never yield the expected results. And even after such change, implementing the verification will actually be a lot of work as well.

And to be honest, this is an amount of added complexity that I don't think is worth it. The number of questions I had about doing this kind of stuff are few, and I have always been able to explain how to redesign a structure to be able to functionally achieve the same.

container.Register<DepConsumer>(overrides: new [] { new DepB(...)} );

This is how you would typically configure things in other containers, but Simple Injector turns this around. Instead of defining the condition on the consumer, you specify that the dependency is conditional. Results are the same.

As you may see in this case cfg must be carried through the whole chain of dependencies just to be passed to method DoSomething of DepK. I do not think this design is better than the initial one.

That can be very inefficient indeed. Perhaps I gave you the wrong impression, but I didn't mean you need to do this. As I described here, there are two ways of passing this kind of data along. First is, as you shown, passing runtime data through method calls of the API, and, second, retrieving runtime data from specific abstractions that allow resolving runtime data. To me it seems the second option is the better option in your case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.