Skip to content

Commit

Permalink
Bug fixes with AllowOverridingRegistrations.
Browse files Browse the repository at this point in the history 
Overriding a registration failed when done with same implementation, and
registration of conditional non-generic registrations was accidentally
allowed. (fixes #114)
  • Loading branch information
dot_net_junkie committed Aug 31, 2015
1 parent 8bc1974 commit bbe8825
Show file tree
Hide file tree
Showing 6 changed files with 190 additions and 38 deletions.
114 changes: 114 additions & 0 deletions SimpleInjector.NET.Tests.Unit/ContainerOptionsTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,120 @@ public void AllowOverridingRegistrations_SetToTrue_ContainerDoesAllowOverringCol
AssertThat.IsInstanceOfType(typeof(InMemoryUserRepository), instance);
}


[TestMethod]
public void AllowOverridingRegistrations_SetToTrueWhileOverridingRegistrationWithSameImplementation_Succeeds()
{
// Arrange
var container = new Container();

container.Register(typeof(ILogger), typeof(NullLogger));
container.Options.AllowOverridingRegistrations = true;

// Act
container.Register(typeof(ILogger), typeof(NullLogger));
}

[TestMethod]
public void RegisterConditional_AllowOverridingRegistrationsSetTrueAndOverlappingConditionalExists_ThrowsException()
{
// Arrange
var container = new Container();

container.RegisterConditional(typeof(ILogger), typeof(NullLogger), c => true);
container.Options.AllowOverridingRegistrations = true;

// Act
Action action = () => container.RegisterConditional(typeof(ILogger), typeof(ConsoleLogger), c => true);

// Assert
AssertThat.ThrowsWithExceptionMessageContains<NotSupportedException>(
"making of conditional registrations is not supported when AllowOverridingRegistrations is set",
action);
}

[TestMethod]
public void AllowOverridingRegistrations_SetToTrueWhileOverridingRegistrationWithDifferentGenericImplementation_ResolvesNewType()
{
// Arrange
var container = new Container();

container.Register(typeof(ICommandHandler<>), typeof(NullCommandHandler<>));
container.Options.AllowOverridingRegistrations = true;

container.Register(typeof(ICommandHandler<>), typeof(DefaultCommandHandler<>));

// Act
var instance = container.GetInstance<ICommandHandler<int>>();

// Assert
AssertThat.IsInstanceOfType(typeof(DefaultCommandHandler<int>), instance);
}

[TestMethod]
public void AllowOverridingRegistrations_SetToTrueWhileOverridingRegistrationWithSameGenericImplementation_Succeeds()
{
// Arrange
var container = new Container();

container.Register(typeof(ICommandHandler<>), typeof(NullCommandHandler<>));
container.Options.AllowOverridingRegistrations = true;

container.Register(typeof(ICommandHandler<>), typeof(NullCommandHandler<>));

// Act
container.GetInstance<ICommandHandler<int>>();
}

[TestMethod]
public void AllowOverridingRegistrations_SetToFalseWhileOverridingRegistrationWithSameGenericImplementation_Succeeds()
{
// Arrange
var container = new Container();

container.Register(typeof(ICommandHandler<>), typeof(NullCommandHandler<>));
container.Options.AllowOverridingRegistrations = false;

// Act
Action action = () => container.Register(typeof(ICommandHandler<>), typeof(NullCommandHandler<>));

// Assert
AssertThat.Throws<InvalidOperationException>(action);
}

[TestMethod]
public void AllowOverridingRegistrations_SetToTrueWhileOverridingGenericTypeWithoutConstraints_SuccessfullyReplacesTheOld()
{
// Arrange
var container = new Container();

container.Register(typeof(ICommandHandler<>), typeof(NullCommandHandler<>));
container.Options.AllowOverridingRegistrations = true;
container.Register(typeof(ICommandHandler<>), typeof(DefaultCommandHandler<>));

// Act
var instance = container.GetInstance<ICommandHandler<int>>();

// Assert
AssertThat.IsInstanceOfType(typeof(DefaultCommandHandler<int>), instance);
}

[TestMethod]
public void Verify_SingletonRegistrationOverriddenWithExactSameImplementation_DoesNotCauseTornLifestyleError()
{
// Arrange
var container = new Container();

container.Register(typeof(ILogger), typeof(NullLogger), Lifestyle.Singleton);
container.Options.AllowOverridingRegistrations = true;

container.Register(typeof(ILogger), typeof(NullLogger), Lifestyle.Singleton);

// Act
container.Verify(VerificationOption.VerifyAndDiagnose);
}


[TestMethod]
public void ConstructorResolutionBehavior_ChangedBeforeAnyRegistrations_ChangesThePropertyToTheSetInstance()
{
Expand Down
4 changes: 2 additions & 2 deletions SimpleInjector.NET/Container.Common.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,15 +113,15 @@ public Container()
{
this.containerId = Interlocked.Increment(ref counter);

this.RegisterSingleton<Container>(this);

this.Options = new ContainerOptions(this)
{
EnableDynamicAssemblyCompilation =
this.containerId < MaximumNumberOfContainersWithDynamicallyCreatedAssemblies
};

this.SelectionBasedLifestyle = new LifestyleSelectionBehaviorProxyLifestyle(this.Options);

this.RegisterSingleton(this);
}

// Wrapper for instance initializer delegates
Expand Down
25 changes: 19 additions & 6 deletions SimpleInjector.NET/Internals/GenericRegistrationEntry.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,25 +74,32 @@ public void AddGeneric(Type serviceType, Type implementationType,
Lifestyle lifestyle, Predicate<PredicateContext> predicate)
{
this.container.ThrowWhenContainerIsLocked();
this.ThrowWhenConditionalIsRegisteredInOverridingMode(predicate);

var provider = new OpenGenericToInstanceProducerProvider(
serviceType, implementationType, lifestyle, predicate, this.container);

this.ThrowWhenConditionalIsRegisteredInOverridingMode(provider);

this.ThrowWhenProviderToRegisterOverlapsWithExistingProvider(provider);

if (provider.AppliesToAllClosedServiceTypes && this.container.Options.AllowOverridingRegistrations)
{
this.providers.RemoveAll(p => p.AppliesToAllClosedServiceTypes);
}

this.providers.Add(provider);
}

public void Add(Type serviceType, Func<TypeFactoryContext, Type> implementationTypeFactory,
Lifestyle lifestyle, Predicate<PredicateContext> predicate)
{
this.container.ThrowWhenContainerIsLocked();
this.ThrowWhenConditionalIsRegisteredInOverridingMode(predicate);

var provider = new OpenGenericToInstanceProducerProvider(
serviceType, implementationTypeFactory, lifestyle, predicate, this.container);

this.ThrowWhenConditionalIsRegisteredInOverridingMode(provider);

this.ThrowWhenProviderToRegisterOverlapsWithExistingProvider(provider);

this.providers.Add(provider);
Expand Down Expand Up @@ -147,9 +154,10 @@ where provider.OverlapsWith(producer.ServiceType)
}
}

private void ThrowWhenConditionalIsRegisteredInOverridingMode(Predicate<PredicateContext> predicate)
private void ThrowWhenConditionalIsRegisteredInOverridingMode(
OpenGenericToInstanceProducerProvider provider)
{
if (predicate != null && this.container.Options.AllowOverridingRegistrations)
if (!provider.AppliesToAllClosedServiceTypes && this.container.Options.AllowOverridingRegistrations)
{
throw new NotSupportedException(
StringResources.MakingConditionalRegistrationsInOverridingModeIsNotSupported());
Expand All @@ -162,11 +170,16 @@ private void ThrowWhenProviderToRegisterOverlapsWithExistingProvider(
bool providerToRegisterIsSuperset =
providerToRegister.AppliesToAllClosedServiceTypes && this.providers.Any();

bool isReplacement = providerToRegister.AppliesToAllClosedServiceTypes
&& this.container.Options.AllowOverridingRegistrations;

// A provider is a superset of the providerToRegister when it can be applied to ALL generic
// types that the providerToRegister can be applied to as well.
var supersetProviders = this.GetSupersetProvidersFor(providerToRegister.ImplementationType);

if (providerToRegisterIsSuperset || supersetProviders.Any())
bool overlaps = providerToRegisterIsSuperset || supersetProviders.Any();

if (!isReplacement && overlaps)
{
var overlappingProvider = supersetProviders.FirstOrDefault() ?? this.providers.First();

Expand All @@ -188,7 +201,7 @@ private InvalidOperationException GetAnOverlappingGenericRegistrationExistsExcep
IProducerProvider providerToRegister, IProducerProvider overlappingProvider)
{
return new InvalidOperationException(
StringResources.AnOverlappingGenericRegistrationExists(
StringResources.AnOverlappingRegistrationExists(
providerToRegister.ServiceType,
overlappingProvider.ImplementationType,
overlappingProvider.IsConditional,
Expand Down
74 changes: 46 additions & 28 deletions SimpleInjector.NET/Internals/NonGenericRegistrationEntry.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,10 @@ public void Add(InstanceProducer producer)
{
this.container.ThrowWhenContainerIsLocked();
this.ThrowWhenConditionalAndUnconditionalAreMixed(producer);
this.ThrowWhenConditionalIsRegisteredInOverridingMode(producer);

this.ThrowWhenTypeAlreadyRegistered(producer);
this.ThrowWhenProviderToRegisterOverlapsWithExistingProvider(producer);
this.ThrowWhenIdenticalImplementationIsAlreadyRegistered(producer);

if (producer.IsUnconditional)
{
Expand All @@ -66,33 +67,6 @@ public void Add(InstanceProducer producer)
this.providers.Add(new SingleInstanceProducerProvider(producer));
}

private void ThrowWhenProviderToRegisterOverlapsWithExistingProvider(
InstanceProducer producerToRegister)
{
// A provider is a superset of the providerToRegister when it can be applied to ALL generic
// types that the providerToRegister can be applied to as well.
var overlappingProducers =
from producer in this.CurrentProducers
where producer.ImplementationType != null
where !producer.Registration.WrapsInstanceCreationDelegate
where !producerToRegister.Registration.WrapsInstanceCreationDelegate
where producer.ImplementationType == producerToRegister.ImplementationType
select producer;

if (overlappingProducers.Any())
{
var overlappingProducer = overlappingProducers.FirstOrDefault();

throw new InvalidOperationException(
StringResources.AnOverlappingGenericRegistrationExists(
producerToRegister.ServiceType,
overlappingProducer.ImplementationType,
overlappingProducer.IsConditional,
producerToRegister.ImplementationType,
producerToRegister.IsConditional));
}
}

public void Add(Type serviceType, Func<TypeFactoryContext, Type> implementationTypeFactory,
Lifestyle lifestyle, Predicate<PredicateContext> predicate)
{
Expand Down Expand Up @@ -153,6 +127,41 @@ private void ThrowWhenTypeAlreadyRegistered(InstanceProducer producer)
}
}

private void ThrowWhenIdenticalImplementationIsAlreadyRegistered(
InstanceProducer producerToRegister)
{
// A provider overlaps the providerToRegister when it can be applied to ALL generic
// types that the providerToRegister can be applied to as well.
var overlappingProducers = this.GetOverlappingProducers(producerToRegister);

bool isReplacement =
producerToRegister.IsUnconditional && this.container.Options.AllowOverridingRegistrations;

if (!isReplacement && overlappingProducers.Any())
{
var overlappingProducer = overlappingProducers.FirstOrDefault();

throw new InvalidOperationException(
StringResources.AnOverlappingRegistrationExists(
producerToRegister.ServiceType,
overlappingProducer.ImplementationType,
overlappingProducer.IsConditional,
producerToRegister.ImplementationType,
producerToRegister.IsConditional));
}
}

private IEnumerable<InstanceProducer> GetOverlappingProducers(InstanceProducer producerToRegister)
{
return
from producer in this.CurrentProducers
where producer.ImplementationType != null
where !producer.Registration.WrapsInstanceCreationDelegate
where !producerToRegister.Registration.WrapsInstanceCreationDelegate
where producer.ImplementationType == producerToRegister.ImplementationType
select producer;
}

private ActivationException ThrowMultipleApplicableRegistrationsFound(
InstanceProducer[] instanceProducers)
{
Expand All @@ -171,6 +180,15 @@ private void ThrowWhenConditionalAndUnconditionalAreMixed(InstanceProducer produ
this.ThrowWhenNonGenericTypeAlreadyRegisteredAsConditionalRegistration(producer);
}

private void ThrowWhenConditionalIsRegisteredInOverridingMode(InstanceProducer producer)
{
if (producer.IsConditional && this.container.Options.AllowOverridingRegistrations)
{
throw new NotSupportedException(
StringResources.MakingConditionalRegistrationsInOverridingModeIsNotSupported());
}
}

private void ThrowWhenNonGenericTypeAlreadyRegisteredAsUnconditionalRegistration(
InstanceProducer producer)
{
Expand Down
2 changes: 1 addition & 1 deletion SimpleInjector.NET/StringResources.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,7 @@ internal static string RegistrationForClosedServiceTypeOverlapsWithOpenGenericRe
Helpers.ToCSharpFriendlyName(overlappingGenericImplementationType),
nameof(Container.RegisterConditional));

internal static string AnOverlappingGenericRegistrationExists(Type openGenericServiceType,
internal static string AnOverlappingRegistrationExists(Type openGenericServiceType,
Type overlappingImplementationType, bool isExistingRegistrationConditional,
Type implementationTypeOfNewRegistration, bool isNewRegistrationConditional)
{
Expand Down
9 changes: 8 additions & 1 deletion changes.txt
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@
Version [Version Number] ([Friendly Version Number]) [Release Date]


Version 3.0.3
Version 3.0.4

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


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

Bug fixes:
-[Core] Container.Verify() does not dispose created instances, while running in an active scope.
Expand Down

0 comments on commit bbe8825

Please sign in to comment.