Skip to content

Commit

Permalink
ResolveUnregisteredType didn't cache registrations.
Browse files Browse the repository at this point in the history
ResolveUnregisteredType registrations didn't get cached. (fixes #113)
  • Loading branch information
dot_net_junkie committed Aug 31, 2015
1 parent bbe8825 commit d4c278e
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 19 deletions.
30 changes: 30 additions & 0 deletions SimpleInjector.NET.Tests.Unit/ResolveUnregisteredTypeEventTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -591,6 +591,36 @@ public void GetInstance_DependingOnAnArrayOfGenericElementsThatIsUnregistered_Th
AssertThat.ThrowsWithExceptionMessageContains<ActivationException>("Nullable<Int32>[]", action);
}

[TestMethod]
public void ResolveUnregisteredType_Always_IsExpectedToBeCached()
{
// Arrange
int callCount = 0;

var container = ContainerFactory.New();

container.Register<ComponentDependingOn<IUserRepository>>();
container.RegisterSingleton<ServiceDependingOn<IUserRepository>>();

var r = Lifestyle.Singleton.CreateRegistration<IUserRepository, SqlUserRepository>(container);

container.ResolveUnregisteredType += (s, e) =>
{
if (e.UnregisteredServiceType == typeof(IUserRepository))
{
callCount++;

e.Register(r);
}
};

// Act
container.Verify();

// Assert
Assert.AreEqual(1, callCount, "The result of ResolveUnregisteredType is expected to be cached.");
}

public class CompositeService<T> where T : struct
{
public CompositeService(Nullable<T>[] dependencies)
Expand Down
6 changes: 0 additions & 6 deletions SimpleInjector.NET/Container.Common.cs
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,6 @@ public partial class Container : IDisposable
private readonly Dictionary<Type, InstanceProducer> unregisteredConcreteTypeInstanceProducers =
new Dictionary<Type, InstanceProducer>();

// Cache for producers that are resolved as root type
// PERF: The rootProducerCache uses a special equality comparer that does a quicker lookup of types.
// PERF: This collection is updated by replacing the complete collection.
private Dictionary<Type, InstanceProducer> rootProducerCache =
new Dictionary<Type, InstanceProducer>(ReferenceEqualityComparer<Type>.Instance);

// Flag to signal that the container can't be altered by using any of the Register methods.
private bool locked;

Expand Down
72 changes: 60 additions & 12 deletions SimpleInjector.NET/Container.Resolving.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,16 @@ namespace SimpleInjector
#endif
public partial class Container : IServiceProvider
{
private readonly Dictionary<Type, InstanceProducer> emptyAndRedirectedCollectionRegistrationCache =
// Cache for producers that are resolved as root type
// PERF: The rootProducerCache uses a special equality comparer that does a quicker lookup of types.
// PERF: This collection is updated by replacing the complete collection.
private Dictionary<Type, InstanceProducer> rootProducerCache =
new Dictionary<Type, InstanceProducer>(ReferenceEqualityComparer<Type>.Instance);

private readonly Dictionary<Type, InstanceProducer> resolveUnregisteredTypeRegistrations =
new Dictionary<Type, InstanceProducer>();

private readonly Dictionary<Type, InstanceProducer> emptyAndRedirectedCollectionRegistrationCache =
new Dictionary<Type, InstanceProducer>();

/// <summary>Gets an instance of the given <typeparamref name="TService"/>.</summary>
Expand Down Expand Up @@ -230,7 +239,7 @@ internal InstanceProducer GetRootRegistrationNoAutoCreateConcretesAndIgnoreFailu
{
// Don't cache this root producer here, because this causes us to invalidly flag the registration
// is invalid.
return this.GetRegistration(serviceType, InjectionConsumerInfo.Root,
return this.GetRegistration(serviceType, InjectionConsumerInfo.Root,
throwOnFailure: false,
autoCreateConcreteTypes: false);
}
Expand Down Expand Up @@ -364,22 +373,61 @@ private InstanceProducer BuildInstanceProducerForType(Type serviceType, Injectio
private InstanceProducer TryBuildInstanceProducerThroughUnregisteredTypeResolution(Type serviceType,
InjectionConsumerInfo context)
{
var e = new UnregisteredTypeEventArgs(serviceType);
// Instead of wrapping the complete method in a lock, we lock inside the individual methods. We
// don't want to hold a lock while calling back into user code, because who knows what the user
// is doing there. We don't want a dead lock.
return this.TryGetInstanceProducerForUnregisteredTypeResolutionFromCache(serviceType)
?? this.TryGetInstanceProducerThroughResolveUnregisteredTypeEvent(serviceType);
}

if (this.resolveUnregisteredType != null)
private InstanceProducer TryGetInstanceProducerForUnregisteredTypeResolutionFromCache(Type serviceType)
{
lock (this.resolveUnregisteredTypeRegistrations)
{
this.resolveUnregisteredType(this, e);
return this.resolveUnregisteredTypeRegistrations.ContainsKey(serviceType)
? this.resolveUnregisteredTypeRegistrations[serviceType]
: null;
}
}

private InstanceProducer TryGetInstanceProducerThroughResolveUnregisteredTypeEvent(Type serviceType)
{
UnregisteredTypeEventArgs e = null;

if (e.Handled)
if (this.resolveUnregisteredType != null)
{
var registration = e.Registration ?? new ExpressionRegistration(e.Expression, this);
e = new UnregisteredTypeEventArgs(serviceType);

return new InstanceProducer(serviceType, registration);
this.resolveUnregisteredType(this, e);
}
else

return e != null && e.Handled
? TryGetProducerFromUnregisteredTypeResolutionCacheOrAdd(e)
: null;
}

private InstanceProducer TryGetProducerFromUnregisteredTypeResolutionCacheOrAdd(
UnregisteredTypeEventArgs e)
{
Type serviceType = e.UnregisteredServiceType;

var registration = e.Registration ?? new ExpressionRegistration(e.Expression, this);

lock (this.resolveUnregisteredTypeRegistrations)
{
return null;
if (this.resolveUnregisteredTypeRegistrations.ContainsKey(serviceType))
{
return this.resolveUnregisteredTypeRegistrations[serviceType];
}

// By creating the InstanceProducer after checking the dictionary, we prevent the producer
// from being created twice when multiple threads are running. Having the same duplicate
// producer can cause a torn lifestyle warning in the container.
var producer = new InstanceProducer(serviceType, registration);

this.resolveUnregisteredTypeRegistrations[serviceType] = producer;

return producer;
}
}

Expand Down Expand Up @@ -717,9 +765,9 @@ private void ThrowNotConstructableException(Type concreteType)

// Since we are at this point, we know the concreteType is NOT constructable.
this.Options.IsConstructableType(concreteType, concreteType, out exceptionMessage);

throw new ActivationException(
StringResources.ImplicitRegistrationCouldNotBeMadeForType(concreteType, this.HasRegistrations)
StringResources.ImplicitRegistrationCouldNotBeMadeForType(concreteType, this.HasRegistrations)
+ " " + exceptionMessage);
}
}
Expand Down
3 changes: 2 additions & 1 deletion changes.txt
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
Version [Version Number] ([Friendly Version Number]) [Release Date]


Version 3.0.4
Version 3.0.4 (v3.0.4 RTM) 2015-08-31

Bug fixes:
-[Core] Overriding a registration failed when done with same implementation, and registration of
conditional non-generic registrations was accidentally allowed. (fixes #114)
-[Core] ResolveUnregisteredType registrations didn't get cached. (fixes #113)


Version 3.0.3 (v3.0.3 RTM) 2015-08-27
Expand Down

0 comments on commit d4c278e

Please sign in to comment.