Skip to content

Commit

Permalink
Fixed NullRefEx when decorating uncontrolled collections. Fixes #703.
Browse files Browse the repository at this point in the history
  • Loading branch information
dotnetjunkie committed May 1, 2019
1 parent 1069b10 commit 1c9f835
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 19 deletions.
85 changes: 81 additions & 4 deletions src/SimpleInjector.Tests.Unit/DecoratorCollectionTests.cs
Expand Up @@ -8,6 +8,7 @@
using System.Linq.Expressions;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using SimpleInjector.Advanced;
using SimpleInjector.Lifestyles;

/// <summary>
/// This set of tests test whether individual items of registered collections are correctly decorated.
Expand Down Expand Up @@ -1996,6 +1997,63 @@ public void GetAllInstances_ContainerUncontrolledDecoratorDependingOnDecoratorPr
AssertThat.AreEqual(typeof(ICommandHandler<RealCommand>), context.ImplementationType);
}

// #703
[TestMethod]
public void GetAllInstances_DecoratorTypeFactoryReturningProxyDecoratorWrappingUncontrolledCollection_ThrowsExpressiveException()
{
// Act
var container = new Container();

IEnumerable<IPlugin> collection = new[] { new PluginImpl() };

container.Collection.Register(containerUncontrolledCollection: collection);
container.RegisterDecorator(
typeof(IPlugin),
_ => typeof(PluginProxy), // factory returning (func-wrapping) proxy
Lifestyle.Transient,
_ => true);

// Act
Action action = () => container.GetAllInstances<IPlugin>().ToArray();

// Assert
AssertThat.ThrowsWithExceptionMessageContains<ActivationException>(
"It's impossible for the container to generate a Func<IPlugin> for injection into the " +
$"{typeof(PluginProxy).ToFriendlyName()} decorator that will be wrapped around instances " +
"of the collection of IPlugin instances",
action);
}

// #703
[TestMethod]
public void GetAllInstances_DecoratorTypeFactoryAndScopedLifestyleWrappingUncontrolledCollection_ThrowsExpressiveException()
{
// Act
string decoratorName = typeof(PluginDecorator).ToFriendlyName();
var container = new Container();
container.Options.DefaultScopedLifestyle = new AsyncScopedLifestyle();

IEnumerable<IPlugin> collection = new[] { new PluginImpl() };

container.Collection.Register(containerUncontrolledCollection: collection);

container.RegisterDecorator(
typeof(IPlugin),
_ => typeof(PluginDecorator), // using factory
Lifestyle.Scoped, // lifestyle other than singleton or transient
_ => true);

// Act
Action action = () => container.GetAllInstances<IPlugin>().ToArray();

// Assert
AssertThat.ThrowsWithExceptionMessageContains<ActivationException>(
$"You are trying to apply the {decoratorName} decorator with the 'Scoped' lifestyle to " +
"a collection of type IPlugin, but the registered collection is not controlled by the " +
"container.",
action);
}

private static KnownRelationship GetValidRelationship()
{
var container = new Container();
Expand All @@ -2016,13 +2074,18 @@ private static void
ex.Message);

AssertThat.StringContains(
"the registration hasn't been made using one of the Container.Collection.Register overloads that take " +
"a list of System.Type",
string.Format(
"the registration was made using either the " +
"Container.Collection.Register<{0}>(IEnumerable<{0}>) or " +
"Container.Collection.Register(Type, IEnumerable) overloads",
typeof(ICommandHandler<RealCommand>).ToFriendlyName()),
ex.Message);

AssertThat.StringContains(
"switch to one of the other Container.Collection.Register overloads, or don't use a decorator that " +
"depends on a Func<T>",
string.Format(
"switch to one of the other Container.Collection.Register overloads, " +
"or use a decorator that depends on {0} instead of Func<{0}>.",
typeof(ICommandHandler<RealCommand>).ToFriendlyName()),
ex.Message);
}

Expand Down Expand Up @@ -2084,5 +2147,19 @@ public DeriveDecorator(IDerive decoratee)
this.Decoratee = decoratee;
}
}

public class PluginProxy : IPlugin
{
public PluginProxy(Func<IPlugin> pluginFactory)
{
}
}

public class PluginDecorator : IPlugin
{
public PluginDecorator(IPlugin plugin)
{
}
}
}
}
Expand Up @@ -267,23 +267,27 @@ private OverriddenParameter[] CreateOverriddenParameters(Expression decorateeExp

private void ThrowWhenDecoratorNeedsAFunc()
{
bool needsADecorateeFactory = this.DecoratorNeedsADecorateeFactory();
Type decorateeFactoryType = this.GetDecorateeFactoryTypeOrNull();

if (needsADecorateeFactory)
if (decorateeFactoryType != null)
{
string message = StringResources.CantGenerateFuncForDecorator(
this.registeredServiceType, this.DecoratorTypeDefinition);
this.registeredServiceType,
decorateeFactoryType,
this.DecoratorTypeDefinition ?? this.decoratorType);

throw new ActivationException(message);
}
}

private bool DecoratorNeedsADecorateeFactory() => (
private Type GetDecorateeFactoryTypeOrNull() => (
from parameter in this.decoratorConstructor.GetParameters()
where DecoratorHelpers.IsScopelessDecorateeFactoryDependencyType(
parameter.ParameterType, this.registeredServiceType)
select parameter)
.Any();
|| DecoratorHelpers.IsScopeDecorateeFactoryDependencyParameter(
parameter.ParameterType, this.registeredServiceType)
select parameter.ParameterType)
.FirstOrDefault();

private void ThrownWhenLifestyleIsNotSupported()
{
Expand All @@ -297,7 +301,9 @@ private void ThrownWhenLifestyleIsNotSupported()
{
throw new NotSupportedException(
StringResources.CanNotDecorateContainerUncontrolledCollectionWithThisLifestyle(
this.DecoratorTypeDefinition, this.Lifestyle, this.registeredServiceType));
this.DecoratorTypeDefinition ?? this.decoratorType,
this.Lifestyle,
this.registeredServiceType));
}
}

Expand Down
20 changes: 12 additions & 8 deletions src/SimpleInjector/StringResources.cs
Expand Up @@ -546,18 +546,22 @@ internal static string TypeMustNotContainInvalidInjectionTarget(InjectionTargetI
CollectionsRegisterMethodName,
implementations.Select(TypeName).ToCommaSeparatedText());

internal static string CantGenerateFuncForDecorator(Type serviceType, Type decoratorType) =>
internal static string CantGenerateFuncForDecorator(
Type serviceType, Type decorateeFactoryType, Type decoratorType) =>
Format(
"It's impossible for the container to generate a Func<{0}> for injection into the {1} " +
"It's impossible for the container to generate a {3} for injection into the {1} " +
"decorator that will be wrapped around instances of the collection of {0} instances, " +
"because the registration hasn't been made using one of the {2} overloads that take a " +
"list of System.Type as serviceTypes. By passing in an IEnumerable<{0}>, it is impossible " +
"for the container to determine its lifestyle, which makes it impossible to generate a " +
"Func<T>. Either switch to one of the other {2} overloads, or don't use a decorator that " +
"depends on a Func<T> for injecting the decoratee.",
"because the registered collection is not controlled by the container. The collection is " +
"considered to be container-uncontrolled collection, because the registration was made " +
"using either the {2}<{0}>(IEnumerable<{0}>) or {2}(Type, IEnumerable) overloads. " +
"It is impossible for the container to determine its lifestyle of an element in a " +
"container-uncontrolled collections, which makes it impossible to generate a {3} for {1}. " +
"Either switch to one of the other {2} overloads, or use a decorator that depends on {0} " +
"instead of {3}.",
serviceType.TypeName(),
decoratorType.TypeName(),
CollectionsRegisterMethodName);
CollectionsRegisterMethodName,
decorateeFactoryType.TypeName());

internal static string SuppliedTypeIsNotAGenericType(Type type) =>
Format("The supplied type {0} is not a generic type.", type.TypeName());
Expand Down

0 comments on commit 1c9f835

Please sign in to comment.