1

Closed

Incorrect behavior with overlapping registration of collections

description

Simple Injector contains several ways to register collections. The most common way is to call RegisterAll while supplying types, but other options are:
  • Calling RegisterAll while supplying a list of instances (singletons).
  • Calling RegisterAll while supplying an enumerable (container uncontrolled collection).
  • Calling RegisterSingle<IEnumerable<...>> while supplying an enumerable.
  • Calling Register<IEnumerable<...>>(() => ...) while supplying a delegate that returns an enumerable.
When (accidentally) mixing these calls, Simple Injector not always detects these duplicate registrations and sometimes allows these registrations to take place. Effect is sometimes that the original registration is completely replaced, while other times the registration is completely ignored. This violates the design principles and should be fixed.

The following tests demonstrate the problem.
[TestMethod]
public void RegisterAllClosedGeneric_CalledAfterRegisterForSameCollection_ThrowsAlreadyRegisteredException()
{
    // Arrange
    var container = new Container();

    container.Register<IEnumerable<IEventHandler<AuditableEvent>>>(() => null);

    // Act
    Action action = () => container.RegisterAll<IEventHandler<AuditableEvent>>(typeof(AuditableEventEventHandler));

    // Assert
    AssertThat.ThrowsWithExceptionMessageContains<InvalidOperationException>(
        "Type IEnumerable<IEventHandler<AuditableEvent>> has already been registered and the container.",
        action);
}

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

    var uncontrolledCollection = Enumerable.Empty<IEventHandler<AuditableEvent>>();

    container.RegisterAll<IEventHandler<AuditableEvent>>(uncontrolledCollection);

    // Act
    Action action = () => container.RegisterAll<IEventHandler<AuditableEvent>>(typeof(AuditableEventEventHandler));

    // Assert
    AssertThat.ThrowsWithExceptionMessageContains<InvalidOperationException>(
        "Type IEnumerable<IEventHandler<AuditableEvent>> has already been registered and the container.",
        action);
}

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

    var collection = new IEventHandler<AuditableEvent>[0];

    container.RegisterSingle<IEnumerable<IEventHandler<AuditableEvent>>>(collection);

    // Act
    Action action = 
        () => container.RegisterAll<IEventHandler<AuditableEvent>>(typeof(AuditableEventEventHandler));

    // Assert
    AssertThat.ThrowsWithExceptionMessageContains<InvalidOperationException>(
        "Type IEnumerable<IEventHandler<AuditableEvent>> has already been registered and the container.",
        action);
}

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

    container.RegisterAll<IEventHandler<AuditableEvent>>(typeof(AuditableEventEventHandler));

    // Act
    Action action = () => container.Register<IEnumerable<IEventHandler<AuditableEvent>>>(() => null);

    // Assert
    AssertThat.ThrowsWithExceptionMessageContains<InvalidOperationException>(
        "Type IEnumerable<IEventHandler<AuditableEvent>> has already been registered and the container.",
        action);
}

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

    container.RegisterAll<IEventHandler<AuditableEvent>>(new AuditableEventEventHandler());

    // Act
    Action action = () => container.Register<IEnumerable<IEventHandler<AuditableEvent>>>(() => null);

    // Assert
    AssertThat.ThrowsWithExceptionMessageContains<InvalidOperationException>(
        "Type IEnumerable<IEventHandler<AuditableEvent>> has already been registered and the container.",
        action);
}

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

    container.Register<IEnumerable<IEventHandler<AuditableEvent>>>(() => null);

    // Act
    Action action = () => container.RegisterAll<IEventHandler<AuditableEvent>>(new AuditableEventEventHandler());

    // Assert
    AssertThat.ThrowsWithExceptionMessageContains<InvalidOperationException>(
        "Type IEnumerable<IEventHandler<AuditableEvent>> has already been registered and the container.",
        action);
}

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

    container.Register<IEnumerable<IEventHandler<AuditableEvent>>>(() => null);

    // Act
    Action action = () => container.RegisterAll(typeof(IEventHandler<>), new[] 
    {
        typeof(AuditableEventEventHandler<>) 
    });

    // Assert
    AssertThat.ThrowsWithExceptionMessageContains<InvalidOperationException>(
        "Mixing calls to RegisterAll for the same open-generic service type is not supported.",
        action);
}

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

    container.RegisterAll<IEventHandler<AuditableEvent>>(new AuditableEventEventHandler());

    // Act
    Action action = () => container.RegisterAll<IEventHandler<AuditableEvent>>(new[]
    {
        typeof(AuditableEventEventHandler)
    });

    // Assert
    AssertThat.ThrowsWithExceptionMessageContains<InvalidOperationException>(
        "Type IEnumerable<IEventHandler<AuditableEvent>> has already been registered and the container.",
        action);
}
        
[TestMethod]
public void RegisterAllClosed_CalledAfterRegisterAllSingletonWithAllowOverridingRegistrations_CompletelyReplacesPrevious()
{
    // Arrange
    Type[] expectedHandlerTypes = new[] { typeof(AuditableEventEventHandler) };

    var container = new Container();

    container.Options.AllowOverridingRegistrations = true;

    container.RegisterAll<IEventHandler<AuditableEvent>>(new AuditableEventEventHandler<AuditableEvent>());

    // Act
    // Should Completely override the previous registration
    container.RegisterAll<IEventHandler<AuditableEvent>>(expectedHandlerTypes);

    // Assert
    var handlers = container.GetAllInstances<IEventHandler<AuditableEvent>>();

    Type[] actualHandlerTypes = handlers.Select(handler => handlers.GetType()).ToArray();

    Assert.IsTrue(expectedHandlerTypes.SequenceEqual(actualHandlerTypes),
        "The original registration was expected to be replaced completely. Actual: " +
        actualHandlerTypes.ToFriendlyNamesText());
}
Closed Apr 10, 2015 at 2:55 PM by dot_NET_Junkie

comments

dot_NET_Junkie wrote Feb 12, 2015 at 8:09 AM

A thorough rewrite of the RegisterAll implementation is needed to pull this off. I therefore assign this issue to the 2.8 release.

dot_NET_Junkie wrote Mar 15, 2015 at 3:03 PM

Fixed in changeset 1f5d8da1b86b59ee47e414d4dac61b58a3f29e35

dot_NET_Junkie wrote Mar 15, 2015 at 3:09 PM

The changeset only fixed part of the problem. The big rewrite is postponed to v3.0 and a new work item 21019 has been created to fix the remainder of the problems.