dadhi / DryIoc

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

Throws when lazy resolve after explicit create using factory-func from within scope #508

Closed BinaryCraX closed 2 years ago

BinaryCraX commented 2 years ago

When lazy-resolving an service after it has been explicitly created using an factory-func from within an scope it will throw an UnableToResolveUnknownService exception for the parameter that was previously provided using the factory-func-call. Because the instance was already created inside the scope using the factory-func I wouldn't expect this to happen, because it doesn't need to be created at this point anymore. I would expect, that a call to Lazy.Value just resolves the previously created instance and returns it.

[TestFixture]
class Lazy_resolve_after_explicit_create_using_factory_func_with_scope
{
    [Test]
    public void Example()
    {
        var container = new Container();

        container.Register<CarFactory>(Reuse.Singleton);
        container.Register<Mercedes>(Reuse.Scoped);
        container.Register<Engine>(Reuse.Scoped);

        using (var scope = container.OpenScope())
        {
            var carFactory = scope.Resolve<CarFactory>();
            var car = carFactory.BuildCar();
        }
    }

    class CarFactory
    {
        private readonly Func<int, Engine> engineFactory;
        private readonly Func<Mercedes> mercedesFactory;

        public Mercedes BuildCar()
        {
            var engine = engineFactory(120);
            var mercedes = mercedesFactory();

            var sameEngineRetrievedUsingFactoryWithParam = mercedes.GetEngineUsingFuncWithParam(99); // works like expected: returns same instance of engine created earlier in mercedesFactory(Func<Mercedes>). So parameter isn't used, because instance was already present in scope (which is the behavior I expected).
            Assert.AreEqual(engine, sameEngineRetrievedUsingFactoryWithParam);

            var sameEngine = mercedes.GetEngine(); // crashes: UnableToResolveUnknownService Int32 as parameter "maxRpm"; Expected it to don't crash but return the same instance of Engine that was created earlier using the engineFactory (engine is already present in the scope, so it doesn't need to be created and therefore doesn't need the parameter).
            Assert.AreEqual(engine, sameEngine);

            return mercedes;
        }

        public CarFactory(Func<int, Engine> engineFactory, Func<Mercedes> mercedesFactory)
        {
            this.engineFactory = engineFactory;
            this.mercedesFactory = mercedesFactory;
        }
    }

    class Mercedes
    {
        private Lazy<Engine> getEngine;
        private Func<int, Engine> EngineFactory;

        public Mercedes(Lazy<Engine> engineLazy, Func<int, Engine> engineFactory)
        {
            this.getEngine = engineLazy;
            this.EngineFactory = engineFactory;
        }
        public Engine GetEngine() => getEngine.Value;
        public Engine GetEngineUsingFuncWithParam(int maxRpm) => EngineFactory(maxRpm);
    }

    class Engine
    {
        public Engine(int maxRpm)
        {
            MaxRpm = maxRpm;
        }

        public int MaxRpm { get; }
    }
}

Somehow when doing the same thing without an scope (but with singletons instead) everything works like expected.

[TestFixture]
class Lazy_resolve_after_explicit_create_using_factory_func_without_scope
{
    [Test]
    public void Example()
    {
        var container = new Container();

        container.Register<CarFactory>(Reuse.Singleton);
        container.Register<Mercedes>(Reuse.Singleton);
        container.Register<Engine>(Reuse.Singleton);

        var carFactory = container.Resolve<CarFactory>();
        var car = carFactory.BuildCar();
    }

    class CarFactory
    {
        private readonly Func<int, Engine> engineFactory;
        private readonly Func<Mercedes> mercedesFactory;

        public Mercedes BuildCar()
        {
            var engine = engineFactory(120);
            var mercedes = mercedesFactory();

            var sameEngineRetrievedUsingFactoryWithParam = mercedes.GetEngineUsingFuncWithParam(99); // works the same way like when scoped
            Assert.AreEqual(engine, sameEngineRetrievedUsingFactoryWithParam);

            var sameEngine = mercedes.GetEngine(); // somehow works when not scoped (but singleton)
            Assert.AreEqual(engine, sameEngine);

            return mercedes;
        }

        public CarFactory(Func<int, Engine> engineFactory, Func<Mercedes> mercedesFactory)
        {
            this.engineFactory = engineFactory;
            this.mercedesFactory = mercedesFactory;
        }
    }

    class Mercedes
    {
        private Lazy<Engine> getEngine;
        private Func<int, Engine> EngineFactory;

        public Mercedes(Lazy<Engine> engineLazy, Func<int, Engine> engineFactory)
        {
            this.getEngine = engineLazy;
            this.EngineFactory = engineFactory;
        }
        public Engine GetEngine() => getEngine.Value;
        public Engine GetEngineUsingFuncWithParam(int maxRpm) => EngineFactory(maxRpm);
    }

    class Engine
    {
        public Engine(int maxRpm)
        {
            MaxRpm = maxRpm;
        }

        public int MaxRpm { get; }
    }
}
dadhi commented 2 years ago

@BinaryCraX I get it. There was a similar case before...

In principle, what you are expecting is the temporal coupling - if you change the order of resolution, mercedes.GetEngine(); before the call to factory, it will fail. If you run it in parallel depends on how lucky are you.

In order to enable such scenario, I need to check the runtime state (what are already created instances in scope) during the graph generation. Then I won't be able to cache or manipulate the expression freely. It is just a one-of-spending for the specific runtime state.

So, I won't do that.

But if you really need it, you may change the code for that. Here is the modified test part.

            public Mercedes(Lazy<Engine> engineLazy, Func<int, Engine> engineFactory, IResolverContext rc)
            {
                this.getEngine = engineLazy;
                this.EngineFactory = n => {
                    var e = engineFactory(n);
                    rc.Use(e); // explicitly make the resolved instance available as runtime state
                    return e;
                };
            }
BinaryCraX commented 2 years ago

You are right, there is some temporal coupling code smell in the sample code.

I did some more experimenting and realized, that the problem isn't with the Lazy-wrapper. Boiled down example:

[TestFixture]
class Resolve_after_explicit_create_with_scope
{
    [Test]
    public void Example()
    {
        var container = new Container();

        container.Register<Engine>(Reuse.Scoped);

        using (var scope = container.OpenScope())
        {
            var engine = scope.Resolve<Engine>(new object[] { 120 });

            var sameEngine = scope.Resolve<Engine>(new object[] { 99 }); // expected to get the same instance of the engine (with maxRpm = 120). Works like expected.
            Assert.AreEqual(engine, sameEngine);

            var sameEngine2 = scope.Resolve<Engine>(); // expected to get the same instance of the engine. Doesn't work like expected: will crash because parameter can't be resolved.
            Assert.AreEqual(engine, sameEngine2);
        }
    }

    class Engine
    {
        public Engine(int maxRpm)
        {
            MaxRpm = maxRpm;
        }

        public int MaxRpm { get; }
    }
}



Sadly I can't remove the temporal coupling completly because in my use case I know a parameter only at runtime:

dadhi commented 2 years ago

@BinaryCraX

DryIoc has Use method to inject values to scopes at runtime. So instead of using Func try this:

using System;
using DryIoc;

public class Program
{
    public static void Main()
    {
        var container = new Container();

        container.Register<Engine>(Reuse.Scoped);

        using (var scope = container.OpenScope())
        {
        scope.Use(120); // set the runtime data to scope
            var engine = scope.Resolve<Engine>(); // new object[] { 120 }); - no need for passing args

            var sameEngine = scope.Resolve<Engine>(); //new object[] { 99 }); // expected to get the same instance of the engine (with maxRpm = 120). Works like expected.
            Console.WriteLine((engine == sameEngine) + ", maxRpm = " + engine.MaxRpm);

            var sameEngine2 = scope.Resolve<Engine>(); // expected to get the same instance of the engine. Doesn't work like expected: will crash because parameter can't be resolved.
            Console.WriteLine((engine == sameEngine2) + ", maxRpm = " + engine.MaxRpm);
        }       
    }

    class Engine
    {
        public Engine(int maxRpm)
        {
            MaxRpm = maxRpm;
        }

        public int MaxRpm { get; }
    }
}
BinaryCraX commented 2 years ago

Ok, I got it now. This is sufficient for my use case. Thank you very much for your support.