simpleinjector / SimpleInjector

An easy, flexible, and fast Dependency Injection library that promotes best practice to steer developers towards the pit of success.
https://simpleinjector.org
MIT License
1.21k stars 153 forks source link

DependencyMetadata.GetInstance() throws if registered with Flowing scope. #861

Closed henriblMSFT closed 4 years ago

henriblMSFT commented 4 years ago

DependencyMetadata.GetInstance() throws when attempting to resolve a Flowing scoped registration.

This happens because DependencyMetadata is always registered as a singleton and assume that scopes are always implicit. DependencyMetadata should be registered as Scoped if the InstanceProducer is a flowing scope.

Here's a test that exhibit this behavior:

[TestMethod]
public void DependencyMetadata_GetInstance_return_flowing_scoped_instance()
{
    var container = new Container()
    container.Options.DefaultScopedLifestyle = ScopedLifestyle.Flowing;

    container.Register<ILogger, NullLogger>(Lifestyle.Scoped);

    var scope = new Scope(container);
    var metadata = scope.GetInstance<DependencyMetadata<ILogger>>();
    metadata.GetInstance(); // Throws an exception
}
exception: 
    SimpleInjector.ActivationException: NullLogger is registered using the 'Scoped' lifestyle,
    but the instance is requested outside the context of an active (Scoped) scope.
    Please see https://simpleinjector.org/scoped for more information about how apply lifestyles and manage scopes.
  Stack Trace: 
    Scope.GetScopelessInstance(ScopedRegistration registration) line 458
    Scope.GetInstance[TImplementation](ScopedRegistration registration, Scope scope) line 317
    LazyScopedRegistration`1.GetInstance(Scope scope) line 51
    DynamicProducer3.Lambda(Object[] constants)
    <.ctor>b__0() line 204
    InstanceProducer.BuildAndReplaceInstanceCreatorAndCreateFirstInstance() line 613
    InstanceProducer.GetInstance() line 291
    DependencyMetadata`1.GetInstance() line 38
dotnetjunkie commented 4 years ago

Branch feature-861 created.

dotnetjunkie commented 4 years ago

This is not a bug, this is a feature 😇. Unfortunately, this feature is missing from v5.0 making it impossible to use the new DependencyMetadata<T> feature in combination with flowing scopes. I added this to the v5.1 milestone.

henriblMSFT commented 4 years ago

That was fast! Thank you!

dotnetjunkie commented 4 years ago

I'm no magician. I started working on this feature before responding to this issue. It was a few hours of work. Time well spent. Another nice improvement to the library. Thank you for reporting this (again).

dotnetjunkie commented 4 years ago

This feature is now part of the just-released v5.1.

ScarletKuro commented 2 years ago

Side note: as I understood, this was not merged to main and only works in 5.1 version.

dotnetjunkie commented 2 years ago

Whow shiii.... Thank's for reporting this @ScarletKuro. What this means is that this feature is part of v5.1, but is not part of v5.2 nor v5.3. This is a major error from my side.