From 1c9f835b3f3494400de21a036cd96f827cc0967f Mon Sep 17 00:00:00 2001 From: dotnetjunkie Date: Wed, 1 May 2019 21:48:16 +0200 Subject: [PATCH] Fixed NullRefEx when decorating uncontrolled collections. Fixes #703. --- .../DecoratorCollectionTests.cs | 85 ++++++++++++++++++- ...ncontrolledServicesDecoratorInterceptor.cs | 20 +++-- src/SimpleInjector/StringResources.cs | 20 +++-- 3 files changed, 106 insertions(+), 19 deletions(-) diff --git a/src/SimpleInjector.Tests.Unit/DecoratorCollectionTests.cs b/src/SimpleInjector.Tests.Unit/DecoratorCollectionTests.cs index 8d39bd677..598a4be78 100644 --- a/src/SimpleInjector.Tests.Unit/DecoratorCollectionTests.cs +++ b/src/SimpleInjector.Tests.Unit/DecoratorCollectionTests.cs @@ -8,6 +8,7 @@ using System.Linq.Expressions; using Microsoft.VisualStudio.TestTools.UnitTesting; using SimpleInjector.Advanced; + using SimpleInjector.Lifestyles; /// /// This set of tests test whether individual items of registered collections are correctly decorated. @@ -1996,6 +1997,63 @@ public void GetAllInstances_ContainerUncontrolledDecoratorDependingOnDecoratorPr AssertThat.AreEqual(typeof(ICommandHandler), context.ImplementationType); } + // #703 + [TestMethod] + public void GetAllInstances_DecoratorTypeFactoryReturningProxyDecoratorWrappingUncontrolledCollection_ThrowsExpressiveException() + { + // Act + var container = new Container(); + + IEnumerable 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().ToArray(); + + // Assert + AssertThat.ThrowsWithExceptionMessageContains( + "It's impossible for the container to generate a Func 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 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().ToArray(); + + // Assert + AssertThat.ThrowsWithExceptionMessageContains( + $"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(); @@ -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).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", + 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).ToFriendlyName()), ex.Message); } @@ -2084,5 +2147,19 @@ public DeriveDecorator(IDerive decoratee) this.Decoratee = decoratee; } } + + public class PluginProxy : IPlugin + { + public PluginProxy(Func pluginFactory) + { + } + } + + public class PluginDecorator : IPlugin + { + public PluginDecorator(IPlugin plugin) + { + } + } } } \ No newline at end of file diff --git a/src/SimpleInjector/Decorators/ContainerUncontrolledServicesDecoratorInterceptor.cs b/src/SimpleInjector/Decorators/ContainerUncontrolledServicesDecoratorInterceptor.cs index cee40345e..baf8ed48e 100644 --- a/src/SimpleInjector/Decorators/ContainerUncontrolledServicesDecoratorInterceptor.cs +++ b/src/SimpleInjector/Decorators/ContainerUncontrolledServicesDecoratorInterceptor.cs @@ -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() { @@ -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)); } } diff --git a/src/SimpleInjector/StringResources.cs b/src/SimpleInjector/StringResources.cs index 8789ae08d..710236ca3 100644 --- a/src/SimpleInjector/StringResources.cs +++ b/src/SimpleInjector/StringResources.cs @@ -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. Either switch to one of the other {2} overloads, or don't use a decorator that " + - "depends on a Func 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());