ipjohnson / Grace

Grace is a feature rich dependency injection container library
MIT License
336 stars 33 forks source link

Scoped ExportFactory doesn't use singleton pattern #274

Closed dtkujawski closed 3 years ago

dtkujawski commented 3 years ago

When I am using the main container, the following code works fine but doe not work when using a ChildScope. In this example I have a TOP class and a SUB class. The SUB class is an argument on the constructor of the TOP class. If I set SUB to a singleton, I expect that if I create multiple TOP instances, each should have the same instance of SUB in them.

Here are the simple classes

        public class Sub { }
        public class Top
        {
            public Sub Sub { get; }
            public Top(Sub sub) { Sub = sub; }
        }

Example code from the container, this works fine:

            var container = new DependencyInjectionContainer();
            container.Configure(c => c.Export<Top>());
            container.Configure(c => c.ExportFactory(() => new Sub()).Lifestyle.Singleton());

            Assert.Same(container.Locate<Sub>(), container.Locate<Sub>());          //passes
            Assert.Same(container.Locate<Top>().Sub, container.Locate<Top>().Sub);  //passes

However, when I use a ChildSope; it doesn't work for the TOP instances:

            var container = new DependencyInjectionContainer();
            container.Configure(c => c.Export<Top>());

            using (var scope = container.CreateChildScope(c => c.ExportFactory(() => new Sub()).Lifestyle.Singleton()))
            {
                Assert.Same(scope.Locate<Sub>(), scope.Locate<Sub>());          //passes
                Assert.Same(scope.Locate<Top>().Sub, scope.Locate<Top>().Sub);  //fails
            }
ipjohnson commented 3 years ago

I'll take a look later today, that seems like odd behavior considering the way singleton is implemented.

ipjohnson commented 3 years ago

So you're running into the same issue where the container is trying to auto resolve the Sub type. I'll be honest I try and avoid child containers as they are not performant at all in terms of a per-request solution and I don't solve tenancy issues though DI.

I've put together a couple tests showing how to solve this type of issue using a lifetime scope which is much faster.