TinyIoC
TinyIoC copied to clipboard
grumpydev
Resolving open generics as singleton caches first request and returns for every other request
The title says it all.
NOTE: I am using ( and ) as angle brackets.
container.Register(typeof (IConfigurationProvider()), typeof (ConfigurationProvider()));
When I call Resolve, lets say, IConfigurationProvider(TestClass1), the result is as expected.
However, if I then call Resolve with IConfigurationProvider(TestClass2), I get a result of IConfigurationProvider(TestClass2).
I suspect it is because it is caching as singleton by default. I also suspect that changing the scope to multi-instance will resolve the issue. I need singleton instances for each requested generic type.
I suspect that the SingletonFactory should be storing a dictionary of types in case the singleton registration is an open generic.Regular types will store one item in the dictionary, where as open generic singletons will store an instance for each generic type.
Here is a failing unit test.
[TestFixture]
public class ContainerTests
{
[Test]
public void Can_store_open_generics_as_singleton()
{
// arrange
var container = new TinyIoCContainer();
// conventions say this will store as singleton by default
container.Register(typeof(IOpenGeneric<>), typeof(OpenGeneric<>));
// act
var type1 = container.Resolve<IOpenGeneric<Type1>>();
var type2 = container.Resolve<IOpenGeneric<Type2>>();
// assert
type1.ShouldNotBeNull();
type2.ShouldNotBeNull();
Assert.IsInstanceOf<OpenGeneric<Type1>>(type1);
Assert.IsInstanceOf<OpenGeneric<Type1>>(type2);
// arrange
type1.Value = 2;
type2.Value = 2;
// act
type1 = container.Resolve<IOpenGeneric<Type1>>();
type2 = container.Resolve<IOpenGeneric<Type2>>();
// assert
type1.Value.ShouldEqual(2);
type2.Value.ShouldEqual(2);
}
public interface IOpenGeneric<T>
{
int Value { get; set; }
}
public class OpenGeneric<T> : IOpenGeneric<T>
{
public OpenGeneric()
{
Value = 1;
}
public int Value{get;set;}
}
public class Type1
{
}
public class Type2
{
}
}
This resolve it. Should I make a pull request?
/// <summary>
/// A factory that lazy instantiates a type and always returns the same instance
/// </summary>
private class SingletonFactory : ObjectFactoryBase, IDisposable
{
private readonly Type registerType;
private readonly Type registerImplementation;
private readonly object SingletonLock = new object();
private Dictionary<int, object> _Current = new Dictionary<int, object>();
public SingletonFactory(Type registerType, Type registerImplementation)
{
//#if NETFX_CORE
// if (registerImplementation.GetTypeInfo().IsAbstract() || registerImplementation.GetTypeInfo().IsInterface())
//#else
if (registerImplementation.IsAbstract() || registerImplementation.IsInterface())
//#endif
throw new TinyIoCRegistrationTypeException(registerImplementation, "SingletonFactory");
if (!IsValidAssignment(registerType, registerImplementation))
throw new TinyIoCRegistrationTypeException(registerImplementation, "SingletonFactory");
this.registerType = registerType;
this.registerImplementation = registerImplementation;
}
public override Type CreatesType
{
get { return this.registerImplementation; }
}
public override object GetObject(Type requestedType, TinyIoCContainer container, NamedParameterOverloads parameters, ResolveOptions options)
{
if (parameters.Count != 0)
throw new ArgumentException("Cannot specify parameters for singleton types");
int hashCode = 0;
// if the registered type is an open generic....
if(registerType.IsGenericType && registerType.GetGenericArguments().All(x => x.FullName == null))
{
// then the requested type should have the parameters defined.
hashCode = requestedType.GetGenericArguments().Sum(x => x.GetHashCode());
}else
{
// just a regular old singleton registration with a single stored instance
hashCode = 0;
}
lock (SingletonLock)
{
if (!_Current.ContainsKey(hashCode))
_Current[hashCode] = container.ConstructType(requestedType, this.registerImplementation, Constructor, options);
return _Current[hashCode];
}
}
public override ObjectFactoryBase SingletonVariant
{
get
{
return this;
}
}
public override ObjectFactoryBase GetCustomObjectLifetimeVariant(ITinyIoCObjectLifetimeProvider lifetimeProvider, string errorString)
{
return new CustomObjectLifetimeFactory(this.registerType, this.registerImplementation, lifetimeProvider, errorString);
}
public override ObjectFactoryBase MultiInstanceVariant
{
get
{
return new MultiInstanceFactory(this.registerType, this.registerImplementation);
}
}
public override ObjectFactoryBase GetFactoryForChildContainer(Type type, TinyIoCContainer parent, TinyIoCContainer child)
{
// We make sure that the singleton is constructed before the child container takes the factory.
// Otherwise the results would vary depending on whether or not the parent container had resolved
// the type before the child container does.
GetObject(type, parent, NamedParameterOverloads.Default, ResolveOptions.Default);
return this;
}
public void Dispose()
{
if (_Current == null)
return;
foreach(var value in _Current.Values)
{
if(value is IDisposable)
(value as IDisposable).Dispose();
}
}
}
Hmmm.... I actually did pretty much the same thing yesterday because the issue came up on Stack Overflow. I think from a design perspective, it might be better to split this out into a GenericSingletonFactory. I would also rather assert on sameness for the second resolution attempt; the Value property seems a bit of a crutch. ;-)
Has this been resolved/ looked at? I don't see any pull requests. It's a pretty important feature...
@Worthaboutapig I couldn't find any pull requests or changes related to this either. It has apparently been fixed by at least @theonlylawislove and me, but as the project seemed to be all but abandoned, obviously nobody ever bothered with a pull request (at least that was my reason).
Its far from.abandoned, its in active use in countless projects and is the core of Nancy.
I am aware of its role in Nancy, but it looked rather dead around the time this issue was opened; there were lots of issues (like this one) and pull requests that never even got a reaction. (I even considered forking it, because it was important for my work at the time.)
I queried whether it had been fixed, as the current release doesn't resolve open generics correctly for me, so I wasn't convinced that @theonlylawislove had fixed it.
I just wanted clarification that it definitely should work...
@Worthaboutapig it should work yes, but it appears that it doesn't and as far as I can remember it's not something I've ever fixed.
@TeaDrivenDev : the main reason most PRs aren't brought in is because the next logical step for this, after the major perf work I did a while back, is to make it fully PCL, but that work has never been completed in the PR and I haven't had time yet to do it myself (PCLs aren't really something I've played much with).
Thanks. I'll try and take a look at it, though I moved on to the NInject plugin instead, as it worked ;) However, the simplicity of the TinyIoC defaults appeals to me.
Don't mean to be rude, but there's dozens of PRs outstanding already. I submitted a fairly simple one that I thought was pretty useful two years ago... and zilch.
If the existing requests are ignored, then why should people submit new ones?
Again, it's a great project and I don't particularly mean to bash the maintainers, but it's effectively dead- the last commit was in April; there's no concerted effort to keep development going or fix serious bugs.
@Worthaboutapig I was added as a contributor within the past year and have been resolving and responding to things as I get time... Before that I was in the same shoes as you... A lot of the current requests need a look over again but last time I did they were out dated and out of sync.
We have this problem in one of our projects. I tried the solution from @pauldotknopf and for us it works. Any plans to fix it in this repo @niemyjski? The proposed solution is maybe not perfect, but it works. And it will unblock some developers. Not sure if someone has the time to implement a better fix.
