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

Iterated collections diagnostic warning invalidly goes off when calling Verify #755

Closed
dotnetjunkie opened this issue Oct 1, 2019 · 1 comment
Labels
Milestone

Comments

@dotnetjunkie
Copy link
Collaborator

@dotnetjunkie dotnetjunkie commented Oct 1, 2019

#553 added the ability to Simple Injector to detect whether collections were invalidly resolved from within constructors by the use of a diagnostic warning.

Unfortunately, when calling Container.Verify(), the detection invalidly goes off on collection types that are not streams, i.e. List<T> and T[]. In that case, the diagnostic message invalidly states:

ConsoleLogger is part of the collection of ILogger services that is injected into CaptivatingCompositeLogger<ILogger[]>. The problem in CaptivatingCompositeLogger<ILogger[]> is that instead of storing the injected collection of ILogger services in a private field and iterating over it at the point its instances are required, ConsoleLogger is being resolved (from the collection) during object construction. Resolving services from an injected collection during object construction (e.g. by calling collection.ToList() in the constructor) is not advised.

The addition of this message can cause confusion, because the message is incorrect.

Here are two tests that reproduce the issue:

[TestMethod]
public void Verify_SingletonThatIteratesListInCtorInjectedWithListWithTransient_ThrowsMismatchDetected()
{
    // Arrange
    Type dependencyType = typeof(List<ILogger>);

    var container = new Container();

    container.Collection.Append<ILogger, ConsoleLogger>(Lifestyle.Transient);

    container.RegisterSingleton(
        typeof(ILogger),
        typeof(CaptivatingCompositeLogger<>).MakeGenericType(dependencyType));

    // Act
    Action action = () => container.Verify();

    // Assert
    // No special communication about iterating during the collection: this can't be
    // detected as List<T> is not a stream and can't be intercepted.
    AssertThat.ThrowsWithExceptionMessageDoesNotContain<ActivationException>(
        "Resolving services from an injected collection during object construction",
        action);

    AssertThat.ThrowsWithExceptionMessageContains<ActivationException>(
        "lifestyle mismatch has been detected",
        action);
}

[TestMethod]
public void Verify_SingletonThatIteratesArrayInCtorInjectedWithArrayWithTransient_ThrowsMismatchDetected()
{
    // Arrange
    Type dependencyType = typeof(ILogger[]);

    var container = new Container();

    container.Collection.Append<ILogger, ConsoleLogger>(Lifestyle.Transient);

    container.RegisterSingleton(
        typeof(ILogger),
        typeof(CaptivatingCompositeLogger<>).MakeGenericType(dependencyType));

    // Act
    Action action = () => container.Verify();

    // Assert
    // No special communication about iterating during the collection: this can't be
    // detected as array is not a stream and can't be intercepted.
    AssertThat.ThrowsWithExceptionMessageDoesNotContain<ActivationException>(
        "Resolving services from an injected collection during object construction",
        action);
            
    AssertThat.ThrowsWithExceptionMessageContains<ActivationException>(
        "lifestyle mismatch has been detected",
        action);
}

These two tests can be added to LifestyleMismatchDueToIterationOfStreamsDuringConstructionTests.

@dotnetjunkie dotnetjunkie added this to the v4.8 milestone Oct 1, 2019
@dotnetjunkie dotnetjunkie modified the milestones: v4.8, v4.9, v4.8.1 Nov 17, 2019
@dotnetjunkie

This comment has been minimized.

Copy link
Collaborator Author

@dotnetjunkie dotnetjunkie commented Nov 24, 2019

bug-755 branch created.

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