Merge pull request #213 from hadashiA/ku/register-instance-as-external

Make sure that RegisterInstance is not managed by the container

VContainer/Assets/VContainer/Runtime/Container.cs

@@ -107,7 +107,7 @@ public void Dispose()
         object CreateTrackedInstance(IRegistration registration)
         {
             var lazy = sharedInstances.GetOrAdd(registration, createInstance);
-            if (!lazy.IsValueCreated && lazy.Value is IDisposable disposable)
+            if (!lazy.IsValueCreated && lazy.Value is IDisposable disposable && !(registration is InstanceRegistration))
             {
                 disposables.Add(disposable);
             }
@@ -165,7 +165,7 @@ public object Resolve(IRegistration registration)
 
                 case Lifetime.Singleton:
                     var singleton = sharedInstances.GetOrAdd(registration, createInstance);
-                    if (!singleton.IsValueCreated && singleton.Value is IDisposable disposable)
+                    if (!singleton.IsValueCreated && singleton.Value is IDisposable disposable && !(registration is InstanceRegistration))
                     {
                         disposables.Add(disposable);
                     }

VContainer/Assets/VContainer/Runtime/Internal/InstanceRegistration.cs

@@ -1,6 +1,5 @@
 using System;
 using System.Collections.Generic;
-using System.Threading;
 
 namespace VContainer.Internal
 {
@@ -11,35 +10,19 @@ sealed class InstanceRegistration : IRegistration
         public Lifetime Lifetime { get; }
 
         readonly object implementationInstance;
-        readonly IInjector injector;
-        readonly IReadOnlyList<IInjectParameter> parameters;
-
-        long injected;
 
         public InstanceRegistration(
             object implementationInstance,
             Type implementationType,
             Lifetime lifetime,
-            IReadOnlyList<Type> interfaceTypes,
-            IReadOnlyList<IInjectParameter> parameters,
-            IInjector injector)
+            IReadOnlyList<Type> interfaceTypes)
         {
             ImplementationType = implementationType;
             Lifetime = lifetime;
             InterfaceTypes = interfaceTypes;
-
             this.implementationInstance = implementationInstance;
-            this.injector = injector;
-            this.parameters = parameters;
         }
 
-        public object SpawnInstance(IObjectResolver resolver)
-        {
-            if (Interlocked.CompareExchange(ref injected, 1, 0) == 0)
-            {
-                injector.Inject(implementationInstance, resolver, parameters);
-            }
-            return implementationInstance;
-        }
+        public object SpawnInstance(IObjectResolver resolver) => implementationInstance;
     }
 }

VContainer/Assets/VContainer/Runtime/Internal/InstanceRegistrationBuilder.cs

@@ -12,15 +12,11 @@ public InstanceRegistrationBuilder(object implementationInstance)
 
         public override IRegistration Build()
         {
-            var injector = InjectorCache.GetOrBuild(ImplementationType);
-
             return new InstanceRegistration(
                 implementationInstance,
                 ImplementationType,
                 Lifetime,
-                InterfaceTypes,
-                Parameters,
-                injector);
+                InterfaceTypes);
         }
     }
-}
+}

VContainer/Assets/VContainer/Runtime/Unity/ComponentRegistrationBuilder.cs

@@ -22,18 +22,30 @@ public Transform GetParent()
 
     public sealed class ComponentRegistrationBuilder : RegistrationBuilder
     {
-        readonly bool finding;
-        readonly Scene scene;
+        readonly object instance;
         readonly Component prefab;
         readonly string gameObjectName;
-        
+
         ComponentDestination destination;
+        Scene scene;
+
+        internal ComponentRegistrationBuilder(object instance)
+            : base(instance.GetType(), Lifetime.Singleton)
+        {
+            this.instance = instance;
+        }
+
+        internal ComponentRegistrationBuilder(in Scene scene, Type implementationType)
+            : base(implementationType, Lifetime.Scoped)
+        {
+            this.scene = scene;
+        }
 
         internal ComponentRegistrationBuilder(
             Component prefab,
             Type implementationType,
             Lifetime lifetime)
-            : this(false, default, implementationType, lifetime)
+            : base(implementationType, lifetime)
         {
             this.prefab = prefab;
         }
@@ -42,32 +54,25 @@ internal ComponentRegistrationBuilder(
             string gameObjectName,
             Type implementationType,
             Lifetime lifetime)
-            : this(false, default, implementationType, lifetime)
-        {
-            this.gameObjectName = gameObjectName;
-        }
-
-        internal ComponentRegistrationBuilder(Scene scene, Type implementationType)
-            : this(true, scene, implementationType, Lifetime.Scoped)
-        {
-        }
-
-        ComponentRegistrationBuilder(
-            bool finding,
-            Scene scene,
-            Type implementationType,
-            Lifetime lifetime)
             : base(implementationType, lifetime)
         {
-            this.finding = finding;
-            this.scene = scene;
+            this.gameObjectName = gameObjectName;
         }
 
         public override IRegistration Build()
         {
             var injector = InjectorCache.GetOrBuild(ImplementationType);
 
-            if (finding)
+            if (instance != null)
+            {
+                return new InstanceComponentRegistration(
+                    instance,
+                    ImplementationType,
+                    InterfaceTypes,
+                    Parameters,
+                    injector);
+            }
+            if (scene.IsValid())
             {
                 return new FindComponentRegistration(
                     ImplementationType,

VContainer/Assets/VContainer/Runtime/Unity/ContainerBuilderUnityExtensions.cs

@@ -108,13 +108,10 @@ public static void RegisterEntryPointExceptionHandler(
 
         public static RegistrationBuilder RegisterComponent<TInterface>(this IContainerBuilder builder, TInterface component)
         {
-            var registrationBuilder = builder.RegisterInstance(component).As(typeof(TInterface));
-            if (component is MonoBehaviour)
-            {
-                // Force inject execution
-                registrationBuilder
-                    .OnAfterBuild((registration, container) => registration.SpawnInstance(container));
-            }
+            var registrationBuilder = new ComponentRegistrationBuilder(component).As(typeof(TInterface));
+            // Force inject execution
+            registrationBuilder.OnAfterBuild((registration, container) => registration.SpawnInstance(container));
+            builder.Register(registrationBuilder);
             return registrationBuilder;
         }
 
@@ -211,9 +208,8 @@ public static RegistrationBuilder RegisterSystemFromWorld<T>(this IContainerBuil
             if (system is null)
                 throw new ArgumentException($"{typeof(T).FullName} is not in the world {world}");
 
-            return builder.RegisterInstance(system)
-                .As(typeof(ComponentSystemBase))
-                .AsSelf();
+            return builder.RegisterComponent(system)
+                .As(typeof(ComponentSystemBase), typeof(T));
         }
 
         // Use custom world

VContainer/Assets/VContainer/Runtime/Unity/FindComponentRegistration.cs

@@ -9,7 +9,7 @@ sealed class FindComponentRegistration : IRegistration
     {
         public Type ImplementationType { get; }
         public IReadOnlyList<Type> InterfaceTypes { get; }
-        public Lifetime Lifetime => Lifetime.Singleton;
+        public Lifetime Lifetime => Lifetime.Scoped;
 
         readonly IReadOnlyList<IInjectParameter> parameters;
         readonly IInjector injector;

VContainer/Assets/VContainer/Runtime/Unity/InstanceComponentRegistration.cs

@@ -0,0 +1,36 @@
+using System;
+using System.Collections.Generic;
+
+namespace VContainer.Unity
+{
+    sealed class InstanceComponentRegistration : IRegistration
+    {
+        public Type ImplementationType { get; }
+        public IReadOnlyList<Type> InterfaceTypes { get; }
+        public Lifetime Lifetime => Lifetime.Singleton;
+
+        readonly object instance;
+        readonly IReadOnlyList<IInjectParameter> parameters;
+        readonly IInjector injector;
+
+        public InstanceComponentRegistration(
+            object instance,
+            Type implementationType,
+            IReadOnlyList<Type> interfaceTypes,
+            IReadOnlyList<IInjectParameter> parameters,
+            IInjector injector)
+        {
+            ImplementationType = implementationType;
+            InterfaceTypes = interfaceTypes;
+            this.instance = instance;
+            this.parameters = parameters;
+            this.injector = injector;
+        }
+
+        public object SpawnInstance(IObjectResolver resolver)
+        {
+            injector.Inject(instance, resolver, parameters);
+            return instance;
+        }
+    }
+}

VContainer/Assets/VContainer/Tests/ScopedContainerTest.cs

@@ -217,7 +217,7 @@ public void Inject()
 
 
         [Test]
-        public void RegisterDisposableInstance()
+        public void InstanceRegistrationDoesNotDisposal()
         {
             var instance1 = new DisposableServiceA();
 
@@ -238,7 +238,7 @@ public void RegisterDisposableInstance()
             Assert.That(resolveFromParent.Disposed, Is.False);
 
             container.Dispose();
-            Assert.That(resolveFromParent.Disposed, Is.True);
+            Assert.That(resolveFromParent.Disposed, Is.False);
         }
     }
 }

VContainer/Assets/VContainer/Tests/Unity/EntryPointTest.cs

@@ -1,5 +1,6 @@
 using System.Collections;
 using NUnit.Framework;
+using UnityEngine;
 using UnityEngine.TestTools;
 using VContainer.Unity;
 
@@ -16,8 +17,6 @@ public IEnumerator Create()
                 builder.Register<DisposableServiceA>(Lifetime.Scoped);
             });
 
-            yield return null;
-            yield return null;
             yield return null;
 
             var entryPoint = lifetimeScope.Container.Resolve<SampleEntryPoint>();
@@ -28,6 +27,12 @@ public IEnumerator Create()
             Assert.That(entryPoint.StartCalled, Is.EqualTo(1));
             Assert.That(entryPoint.PostStartCalled, Is.EqualTo(1));
 
+            yield return new WaitForFixedUpdate();
+            yield return new WaitForFixedUpdate();
+
+            Assert.That(entryPoint.FixedTickCalls, Is.GreaterThan(0));
+            Assert.That(entryPoint.PostFixedTickCalls, Is.GreaterThan(0));
+
             var fixedTickCalls = entryPoint.FixedTickCalls;
             var postFixedTickCalls = entryPoint.PostFixedTickCalls;
             var tickCalls = entryPoint.TickCalls;
@@ -38,23 +43,16 @@ public IEnumerator Create()
             var disposable = lifetimeScope.Container.Resolve<DisposableServiceA>();
             lifetimeScope.Dispose();
 
-            Assert.That(fixedTickCalls, Is.GreaterThan(0));
-            Assert.That(postFixedTickCalls, Is.GreaterThan(0));
-            Assert.That(tickCalls, Is.GreaterThan(1));
-            Assert.That(postTickCalls, Is.GreaterThan(1));
-            Assert.That(lateTickCalls, Is.GreaterThan(1));
-            Assert.That(postLateTickCalls, Is.GreaterThan(1));
-
-            yield return null;
             yield return null;
+            yield return new WaitForFixedUpdate();
 
+            Assert.That(fixedTickCalls, Is.EqualTo(fixedTickCalls));
+            Assert.That(postFixedTickCalls, Is.EqualTo(postFixedTickCalls));
+            Assert.That(tickCalls, Is.EqualTo(tickCalls));
+            Assert.That(postTickCalls, Is.EqualTo(postTickCalls));
+            Assert.That(lateTickCalls, Is.EqualTo(lateTickCalls));
+            Assert.That(postLateTickCalls, Is.EqualTo(postLateTickCalls));
             Assert.That(disposable.Disposed, Is.True);
-            Assert.That(entryPoint.FixedTickCalls, Is.EqualTo(fixedTickCalls));
-            Assert.That(entryPoint.PostFixedTickCalls, Is.EqualTo(postFixedTickCalls));
-            Assert.That(entryPoint.TickCalls, Is.EqualTo(tickCalls));
-            Assert.That(entryPoint.PostTickCalls, Is.EqualTo(postTickCalls));
-            Assert.That(entryPoint.LateTickCalls, Is.EqualTo(lateTickCalls));
-            Assert.That(entryPoint.PostLateTickCalls, Is.EqualTo(postLateTickCalls));
         }
 
         [UnityTest]

VContainer/Assets/VContainer/Tests/Unity/UnityContainerBuilderTest.cs

@@ -41,25 +41,40 @@ public void RegisterEntryPointMultiple()
         public void RegisterComponent()
         {
             var component = new GameObject("SampleBehaviour").AddComponent<SampleMonoBehaviour>();
-            {
-                var builder = new ContainerBuilder();
-                builder.Register<ServiceA>(Lifetime.Transient);
-                builder.RegisterComponent(component);
+            var builder = new ContainerBuilder();
+            builder.Register<ServiceA>(Lifetime.Transient);
+            builder.RegisterComponent(component);
 
-                var container = builder.Build();
+            var container = builder.Build();
 
-                Assert.That(container.Resolve<SampleMonoBehaviour>(), Is.EqualTo(component));
-                Assert.That(container.Resolve<SampleMonoBehaviour>().ServiceA, Is.InstanceOf<ServiceA>());
-            }
-            {
-                var builder = new ContainerBuilder();
-                builder.Register<ServiceA>(Lifetime.Transient);
-                builder.RegisterComponent<IComponent>(component);
+            Assert.That(container.Resolve<SampleMonoBehaviour>(), Is.EqualTo(component));
+            Assert.That(container.Resolve<SampleMonoBehaviour>().ServiceA, Is.InstanceOf<ServiceA>());
+        }
 
-                var container = builder.Build();
+        [Test]
+        public void RegisterComponentAsInterface()
+        {
+            var component = new GameObject("SampleBehaviour").AddComponent<SampleMonoBehaviour>();
+            var builder = new ContainerBuilder();
+            builder.Register<ServiceA>(Lifetime.Transient);
+            builder.RegisterComponent<IComponent>(component);
 
-                Assert.That(container.Resolve<IComponent>(), Is.EqualTo(component));
-            }
+            var container = builder.Build();
+
+            Assert.That(container.Resolve<IComponent>(), Is.EqualTo(component));
+        }
+
+        [Test]
+        public void RegisterComponentWithAutoInjection()
+        {
+            var component = new GameObject("SampleBehaviour").AddComponent<SampleMonoBehaviour>();
+            var builder = new ContainerBuilder();
+            builder.Register<ServiceA>(Lifetime.Transient);
+            builder.RegisterComponent<IComponent>(component);
+
+            var container = builder.Build();
+
+            Assert.That(component.ServiceA, Is.InstanceOf<ServiceA>());
         }
 
         [Test]