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 154 forks source link

Container.GetRegistration<T>() returns null after calling container.Register<T>(); #909

Closed henriblMSFT closed 3 years ago

henriblMSFT commented 3 years ago

Describe the bug

calling container.GetRegistration<T>() immediately after calling container.Register<T>() returns null even though the registration is present

Expected behavior

container.GetRegistration<T>() should return an instance producer for the regisation.

An instance producer can be found using:

container.GetCurrentRegistrations().First(r => r.ServiceType == typeof(T));

Actual behavior

container.GetRegistration<T>() returns null;

To Reproduce

public class TestService {}
public void CanGetRegistrationAfterRegister()
{
    var container = new Container();
    container.Register<TestService>();
    var registration = container.GetRegistration<TestService>(); // returns null
    registration.Should().NotBeNull();
}

Additional context

SimpleInjector Version 5.3.0

dotnetjunkie commented 3 years ago

I'm unable to reproduce this issue. I added the following test to the Simple Injector test suite, but it succeeds:

public class TestService { }

[TestMethod]
public void CanGetRegistrationAfterRegister()
{
    var container = new Container();
    container.Register<TestService>();
    var registration = container.GetRegistration<TestService>();
    Assert.IsNotNull(registration);
}

When I debug through it, I see that the registration variable gets assigned with a valid registration; it is not null.

If the test you posted really returns null, something very weird is going on what I don't fully understand.

henriblMSFT commented 3 years ago

In an attempt to simplify the problem I removed the issue.

The problem occurs if you call GetRegistration<T>() before calling register. In that case the next call to GetRegistration<T>() is presumably cached and returns null:

I was originally trying to implement a TryRegister pattern like this but the second call to try register would fail, attempting to register a duplicate registration.

public static void TryRegister<T>(this Container container)
    where T : class
{
    if(container.GetRegistration<TestService>() == null)
        container.Register<T>();
}

Here's a test that actually exhibit this behavior:

public class TestService { }

[TestMethod]
public void CanGetRegistrationAfterRegister()
{
    var container = new Container();
    container.GetRegistration<TestService>();
    container.Register<TestService>();

    var registration = container.GetRegistration<TestService>();
    Assert.IsNotNull(registration); // Fails
}

This issue also affect the ability to resolve from the container

[TestMethod]
public void CanGetRegistrationAfterRegister()
{
    var container = new Container();
    container.GetRegistration<TestService>();
    container.Register<TestService>();

    container.GetInstance<TestService>(); // throws SimpleInjector.ActivationException
}
dotnetjunkie commented 3 years ago

In that case the next call to GetRegistration<T>() is presumably cached and returns null:

Shoot.... that's quite a bug. Thanks for taking the time to write the unit test. I will try to fix this in the next patch release.

dotnetjunkie commented 3 years ago

bug-909 branch created.

dotnetjunkie commented 3 years ago

Here's another failing test. Same problem, but this one is even worse:

[TestMethod]
public void GetService_WithGetRegistrationCalledBeforeRegistration_ReturnsThatService()
{
    // Arrange
    var container = new Container();

    container.GetRegistration<TestService>();
    container.Register<TestService>();

    // Act
    var service = ((IServiceProvider)provider).GetService(typeof(TestService));

    // Assert
    Assert.IsNotNull(service);
}
henriblMSFT commented 3 years ago

While our team doesn't use this settings what should be the behavior when the container is configured to automatically resolves unregistered concrete type?

Should GetRegistration<T>() dynamically create the registration or should it return null?

dotnetjunkie commented 3 years ago

Should GetRegistration<T>() dynamically create the registration or should it return null?

When the container is set with ResolveUnregisteredConcreteTypes to true, a call to GetRegistration for an unregistered concrete type will return its InstanceProducer—but only when all its dependencies are registered.

Example:

var container = new Container();
container.Options.ResolveUnregisteredConcreteTypes = true;
var registration = container.GetRegistration<TestService>();
Assert.IsNotNull(registration);
dotnetjunkie commented 3 years ago

Simple Injector v5.3.1 was pushed to NuGet. This patch release fixes this issue.