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

alternate method of checking for cyclical dependencies allows for open generics #209

Merged
merged 4 commits into from
Aug 4, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
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
127 changes: 120 additions & 7 deletions src/Ninject.Test/Integration/CircularDependenciesTests.cs
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
namespace Ninject.Tests.Integration.CircularDependenciesTests
{
using System;
using System.Collections.Generic;
using System.Linq;
using FluentAssertions;
using Ninject.Activation;
using Ninject.Parameters;
using Ninject.Tests.Integration.StandardKernelTests;
using Xunit;

public class CircularDependenciesContext : IDisposable
Expand Down Expand Up @@ -82,8 +84,8 @@ public WhenDependenciesHaveThreeWayCircularReferenceBetweenConstructors()
[Fact]
public void DoesNotThrowExceptionIfHookIsCreated()
{
var request = new Request(typeof(ThreeWayConstructorFoo), null, Enumerable.Empty<IParameter>(), null, false, false);
var request = new Request(typeof(ThreeWayConstructorFoo), null, Enumerable.Empty<IParameter>(), null, false, false);
kernel.Resolve(request);
}

Expand Down Expand Up @@ -122,6 +124,55 @@ public void ScopeIsRespected()
}
}

public class WhenDependenciesHaveOpenGenericCircularReferenceBetweenConstructors : CircularDependenciesContext
{
public WhenDependenciesHaveOpenGenericCircularReferenceBetweenConstructors()
{
kernel.Bind(typeof(IOptions<>)).To(typeof(OptionsManager<>));

kernel.Bind<IConfigureOptions<ClassA>>().To<ConfigureA1>();
kernel.Bind<IConfigureOptions<ClassB>>().To<ConfigureB1>();
kernel.Bind<IConfigureOptions<ClassC>>().To<HasCircularDependency1>();
kernel.Bind<IConfigureOptions<ClassD>>().To<HasCircularDependency2>();

}

[Fact]
public void DoesNotThrowException()
{
kernel.Get<IOptions<ClassA>>();

}

[Fact]
public void DoesNotThrowException2()
{
var o = kernel.Get<HasOptionsPropertyInjected>();

}

[Fact]
public void DetectsCyclicDependenciesInPropertySetter()
{
Action act = () => kernel.Get<IOptions<ClassC>>();

act.ShouldThrow<ActivationException>();
}

[Fact]
public void DetectsCyclicDependenciesForGenericServiceRegisteredViaOpenGenericType2()
{
kernel.Bind(typeof(IGeneric<>)).To(typeof(GenericServiceWithGenericConstructor<>));

Action act = () => kernel.Get<IGeneric<int>>();

act.ShouldThrow<ActivationException>();
}

}



public class TwoWayConstructorFoo
{
public TwoWayConstructorFoo(TwoWayConstructorBar bar) { }
Expand All @@ -134,12 +185,14 @@ public TwoWayConstructorBar(TwoWayConstructorFoo foo) { }

public class TwoWayPropertyFoo
{
[Inject] public TwoWayPropertyBar Bar { get; set; }
[Inject]
public TwoWayPropertyBar Bar { get; set; }
}

public class TwoWayPropertyBar
{
[Inject] public TwoWayPropertyFoo Foo { get; set; }
[Inject]
public TwoWayPropertyFoo Foo { get; set; }
}

public class ThreeWayConstructorFoo
Expand All @@ -159,17 +212,77 @@ public ThreeWayConstructorBaz(TwoWayConstructorFoo foo) { }

public class ThreeWayPropertyFoo
{
[Inject] public ThreeWayPropertyBar Bar { get; set; }
[Inject]
public ThreeWayPropertyBar Bar { get; set; }
}

public class ThreeWayPropertyBar
{
[Inject] public ThreeWayPropertyBaz Baz { get; set; }
[Inject]
public ThreeWayPropertyBaz Baz { get; set; }
}

public class ThreeWayPropertyBaz
{
[Inject] public ThreeWayPropertyFoo Foo { get; set; }
[Inject]
public ThreeWayPropertyFoo Foo { get; set; }
}

public class GenericServiceWithGenericConstructor<T> : IGeneric<T>
{
public GenericServiceWithGenericConstructor(IGeneric<T> arg)
{
}
}

public interface IOptions<T>
{
}

public class OptionsManager<T> : IOptions<T>
{
public OptionsManager(IConfigureOptions<T> items)
{
}
}

public interface IConfigureOptions<T>
{
}

public class ConfigureA1 : IConfigureOptions<ClassA>
{
public ConfigureA1(IOptions<ClassB> bOptions)
{
}
}

public class ConfigureB1 : IConfigureOptions<ClassB>
{
}

public class HasOptionsPropertyInjected
{
[Inject]
public IOptions<ClassA> ClassAOptions { get; set; }
}

public class HasCircularDependency1 : IConfigureOptions<ClassC>
{
[Inject]
public IOptions<ClassD> ClassDOptions { get; set; }
}

public class HasCircularDependency2 : IConfigureOptions<ClassD>
{
public HasCircularDependency2(IOptions<ClassC> classCOptions) { }
}


public class ClassA { }
public class ClassB { }
public class ClassC { }
public class ClassD { }


}
69 changes: 44 additions & 25 deletions src/Ninject/Activation/Context.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,18 @@
//-------------------------------------------------------------------------------

namespace Ninject.Activation
{
using System;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;

using Ninject.Activation.Caching;
using Ninject.Infrastructure.Introspection;
using Ninject.Parameters;
using Ninject.Planning;
using Ninject.Planning.Bindings;

{
using System;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using Ninject.Activation.Caching;
using Ninject.Infrastructure.Introspection;
using Ninject.Parameters;
using Ninject.Planning;
using Ninject.Planning.Bindings;
using Planning.Targets;
/// <summary>
/// Contains information about the activation of a single instance.
/// </summary>
Expand Down Expand Up @@ -109,7 +109,7 @@ public IProvider GetProvider()
/// <inheritdoc />
public object Resolve()
{
if (this.Request.ActiveBindings.Contains(this.Binding))
if (IsCyclical(Request.ParentContext))
{
throw new ActivationException(ExceptionFormatter.CyclicalDependenciesDetected(this));
}
Expand All @@ -118,19 +118,19 @@ public object Resolve()
{
this.cachedScope = this.Request.GetScope() ?? this.Binding.GetScope(this);

if (this.cachedScope != null)
{
lock (this.cachedScope)
if (this.cachedScope != null)
{
return this.ResolveInternal(this.cachedScope);
}
lock (this.cachedScope)
{
return this.ResolveInternal(this.cachedScope);
}
}
else
{
return this.ResolveInternal(null);
}
}
else
{
return this.ResolveInternal(null);
}
}
finally
finally
{
this.cachedScope = null;
}
Expand Down Expand Up @@ -188,6 +188,25 @@ private object ResolveInternal(object scope)
this.Pipeline.Activate(this, reference);

return reference.Instance;
}
}

private bool IsCyclical(IContext targetContext)
{
if (targetContext == null)
return false;

if (targetContext.Request.Service == Request.Service)
{
if (!(Request.Target is PropertyTarget) || targetContext.GetScope() != this.GetScope() || this.GetScope() == null)
return true;
}

if (IsCyclical(targetContext.Request.ParentContext))
return true;

return false;
}


}
}
10 changes: 10 additions & 0 deletions src/Ninject/Infrastructure/Multimap.cs
Original file line number Diff line number Diff line change
Expand Up @@ -149,5 +149,15 @@ private ICollection<V> GetValues(K key)

return result;
}

public Multimap<K, V> Clone()
{
var map = new Multimap<K, V>();
foreach (var key in Keys)
map.items.Add(key, items[key].ToList());

return map;
}

}
}
3 changes: 1 addition & 2 deletions src/Ninject/KernelConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -227,8 +227,7 @@ public IReadonlyKernel BuildReadonlyKernel()

private Multimap<Type, IBinding> CloneBindings()
{
// Todo: Clone
return this.bindings;
return this.bindings.Clone();
}

/// <summary>
Expand Down