dadhi / DryIoc

DryIoc is fast, small, full-featured IoC Container for .NET
MIT License
988 stars 122 forks source link

Property injection is not working correctly when appending a new implementation for multiple registrations of the same interface and injecting an enumerable of the interface. #535

Closed jbrookssmokeball closed 1 year ago

jbrookssmokeball commented 1 year ago

Property injection is not working correctly when appending a new implementation for multiple registrations of the same interface and injecting an enumerable of the interface.

It works fine if building the concrete type, but when injecting the enumerable of the interface it gives a null value on the second registered type.

For example you have the following interface and classes:

public interface IFoo
{
    string Test { get; set; }
}

public class Foo1 : IFoo
{
    public string Test { get; set; }
}

public class Foo2 : IFoo
{
    public string Test { get; set; }
}

Then when registering each implementation in the container twice (appending the implementation with a property selector the second time) only Foo1 will have the property set when resolving an IEnumerable of IFoo. Foo2 has null for the Test property and is a different instance to resolving Foo2 directly (which has the property set correctly) despite being registered as a singleton.

const string testFoo1 = "TF1";
const string testFoo2 = "TF2";

var container = new Container();

container.RegisterMany(new[] { typeof(Foo1), typeof(IFoo) }, typeof(Foo1), reuse: Reuse.Singleton);
var propertiesAndFieldsSelector = PropertiesAndFields.Of.Name(nameof(IFoo.Test), _ => testFoo1);
container.RegisterMany(new[] { typeof(Foo1), typeof(IFoo) }, typeof(Foo1), reuse: Reuse.Singleton, made: propertiesAndFieldsSelector, ifAlreadyRegistered: IfAlreadyRegistered.AppendNewImplementation);

container.RegisterMany(new[] { typeof(Foo2), typeof(IFoo) }, typeof(Foo2), reuse: Reuse.Singleton);
propertiesAndFieldsSelector = PropertiesAndFields.Of.Name(nameof(IFoo.Test), _ => testFoo2);
container.RegisterMany(new[] { typeof(Foo2), typeof(IFoo) }, typeof(Foo2), reuse: Reuse.Singleton, made: propertiesAndFieldsSelector, ifAlreadyRegistered: IfAlreadyRegistered.AppendNewImplementation);

var foo1 = container.Resolve<Foo1>();
var foo2 = container.Resolve<Foo2>();
var foos = container.Resolve<IEnumerable<IFoo>>();

Assert.AreEqual(testFoo1, foo1.Test);
Assert.AreEqual(testFoo2, foo2.Test);

foreach (var foo in foos)
{
    switch (foo)
    {
        case Foo1 foo1FromEnumerable:
            Assert.AreEqual(testFoo1, foo.Test);
            Assert.AreEqual(foo1, foo1FromEnumerable);
            break;
        case Foo2 foo2FromEnumerable:
            // This will fail as 'Test' will be null and 'foo2FromEnumerable' is a different instance to 'foo2'
            Assert.AreEqual(testFoo2, foo.Test);
            Assert.AreEqual(foo2, foo2FromEnumerable);
            break;
    }
}

Issue for PR https://github.com/dadhi/DryIoc/pull/534

dadhi commented 1 year ago

Yeah, there is some bug here. But I am not sure why do you do the registration twice, is it intended?

Anyway, if you comment the first registrations which do not do anything because they are overridden by the next, there will be no bug. I mean to comment those:

container.RegisterMany(new[] { typeof(Foo1), typeof(IFoo) }, typeof(Foo1), reuse: Reuse.Singleton);
container.RegisterMany(new[] { typeof(Foo2), typeof(IFoo) }, typeof(Foo2), reuse: Reuse.Singleton);
jbrookssmokeball commented 1 year ago

@dadhi yep you're right, it will work with just the second registration by itself. Found this bug when implementing DryIoc on a legacy DI abstraction which required me to register it a second time in order to get property injection so thought I'd raise it. I am unable to work around it using IfAlreadyRegistered.Replace unfortunately as this will replace the first registration for the interface too i.e. resolving an IEnumerable<IFoo> in this case will then only return Foo2.

dadhi commented 1 year ago

@jbrookssmokeball thanks for clarifying.