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

Make sure that RegisterInstance is not managed by the container #213

Merged
merged 6 commits into from
May 3, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Make sure that RegisterInstance is not managed by the container
  • Loading branch information
hadashiA committed May 3, 2021
commit b29d5d16d8683b08521e0836f4ce3139c7382885
4 changes: 2 additions & 2 deletions VContainer/Assets/VContainer/Runtime/Container.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
using System;
using System.Collections.Generic;
using System.Threading;

namespace VContainer.Internal
{
Expand All @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
using System;
using System.Collections.Generic;
using UnityEngine;

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;
}
}
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions VContainer/Assets/VContainer/Tests/ScopedContainerTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ public void Inject()


[Test]
public void RegisterDisposableInstance()
public void InstanceRegistrationDoesNotDisposal()
{
var instance1 = new DisposableServiceA();

Expand All @@ -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);
}
}
}