From 2c57421e2149a729b4f9f9cb60b415c801ed106f Mon Sep 17 00:00:00 2001 From: Simon G Date: Mon, 5 Sep 2022 11:29:29 +0200 Subject: [PATCH] - fix passed null in factories - fix ctors without params not checking for remaining args --- .../IocValidator.cs | 2 +- .../Factories/FactoryHelper.cs | 35 ++++++++ .../Factories/TypedFactory.cs | 37 ++++++-- LightweightIocContainer/IocContainer.cs | 16 ++-- .../LightweightIocContainer.xml | 30 +++++++ LightweightIocContainer/NullParameter.cs | 22 +++++ .../IocValidatorTest.cs | 57 ++++++++++++- .../FluentFactoryRegistrationTest.cs | 84 ++++++++++++++++++- 8 files changed, 260 insertions(+), 23 deletions(-) create mode 100644 LightweightIocContainer/Factories/FactoryHelper.cs create mode 100644 LightweightIocContainer/NullParameter.cs diff --git a/LightweightIocContainer.Validation/IocValidator.cs b/LightweightIocContainer.Validation/IocValidator.cs index b32f778..df33fc2 100644 --- a/LightweightIocContainer.Validation/IocValidator.cs +++ b/LightweightIocContainer.Validation/IocValidator.cs @@ -96,7 +96,7 @@ public class IocValidator } catch (Exception) { - return null; + return new NullParameter(type); } } } \ No newline at end of file diff --git a/LightweightIocContainer/Factories/FactoryHelper.cs b/LightweightIocContainer/Factories/FactoryHelper.cs new file mode 100644 index 0000000..d62f1af --- /dev/null +++ b/LightweightIocContainer/Factories/FactoryHelper.cs @@ -0,0 +1,35 @@ +// Author: Gockner, Simon +// Created: 2022-09-02 +// Copyright(c) 2022 SimonG. All Rights Reserved. + +using System.Reflection; + +namespace LightweightIocContainer.Factories; + +/// +/// Helper class for the +/// +public class FactoryHelper +{ + /// + /// Convert `null` passed as argument to to handle it correctly + /// + /// The create method of the factory + /// The arguments passed to the create method + /// List of arguments with converted null + /// Wrong parameters passed + public object?[] ConvertPassedNull(MethodBase createMethod, params object?[] arguments) + { + if (!arguments.Any()) + return arguments; + + ParameterInfo[] parameters = createMethod.GetParameters(); + if (arguments.Length != parameters.Length) + throw new Exception("Wrong parameters passed"); + + for (int i = 0; i < arguments.Length; i++) + arguments[i] ??= new NullParameter(parameters[i].ParameterType); + + return arguments; + } +} \ No newline at end of file diff --git a/LightweightIocContainer/Factories/TypedFactory.cs b/LightweightIocContainer/Factories/TypedFactory.cs index 060e305..b0c534b 100644 --- a/LightweightIocContainer/Factories/TypedFactory.cs +++ b/LightweightIocContainer/Factories/TypedFactory.cs @@ -37,22 +37,28 @@ public class TypedFactory : TypedFactoryBase, ITypedFactory< private TFactory CreateFactory(IocContainer container) { Type factoryType = typeof(TFactory); - + AssemblyBuilder assemblyBuilder = AssemblyBuilder.DefineDynamicAssembly(new AssemblyName("Factory"), AssemblyBuilderAccess.Run); ModuleBuilder moduleBuilder = assemblyBuilder.DefineDynamicModule("Factory"); TypeBuilder typeBuilder = moduleBuilder.DefineType($"TypedFactory.{factoryType.Name}"); typeBuilder.AddInterfaceImplementation(factoryType); - //add `private readonly IIocContainer _container` field + //add `private readonly IocContainer _container` field FieldBuilder containerFieldBuilder = typeBuilder.DefineField("_container", typeof(IocContainer), FieldAttributes.Private | FieldAttributes.InitOnly); + + //add `private readonly FactoryHelper _helper` field + FieldBuilder helperFieldBuilder = typeBuilder.DefineField("_helper", typeof(FactoryHelper), FieldAttributes.Private | FieldAttributes.InitOnly); //add ctor - ConstructorBuilder constructorBuilder = typeBuilder.DefineConstructor(MethodAttributes.Public, CallingConventions.HasThis, new[] {typeof(IocContainer)}); + ConstructorBuilder constructorBuilder = typeBuilder.DefineConstructor(MethodAttributes.Public, CallingConventions.HasThis, new[] {typeof(IocContainer), typeof(FactoryHelper)}); ILGenerator constructorGenerator = constructorBuilder.GetILGenerator(); constructorGenerator.Emit(OpCodes.Ldarg_0); constructorGenerator.Emit(OpCodes.Ldarg_1); constructorGenerator.Emit(OpCodes.Stfld, containerFieldBuilder); //set `_container` field + constructorGenerator.Emit(OpCodes.Ldarg_0); + constructorGenerator.Emit(OpCodes.Ldarg_2); + constructorGenerator.Emit(OpCodes.Stfld, helperFieldBuilder); //set `_helper` field constructorGenerator.Emit(OpCodes.Ret); foreach (MethodInfo createMethod in CreateMethods) @@ -60,7 +66,9 @@ public class TypedFactory : TypedFactoryBase, ITypedFactory< //create a method that looks like this //public `createMethod.ReturnType` Create(`createMethod.GetParameters()`) //{ - // return IIocContainer.Resolve(`createMethod.ReturnType`, params); + // createMethod = MethodBase.GetCurrentMethod(); + // object[] args = _helper.ConvertPassedNull(createMethod, params); + // return IocContainer.Resolve(`createMethod.ReturnType`, args); //} ParameterInfo[] args = createMethod.GetParameters(); @@ -70,9 +78,16 @@ public class TypedFactory : TypedFactoryBase, ITypedFactory< typeBuilder.DefineMethodOverride(methodBuilder, createMethod); ILGenerator generator = methodBuilder.GetILGenerator(); + generator.DeclareLocal(typeof(MethodBase)); + generator.DeclareLocal(typeof(object[])); + generator.EmitCall(OpCodes.Call, typeof(MethodBase).GetMethod(nameof(MethodBase.GetCurrentMethod))!, null); + generator.Emit(OpCodes.Stloc_0); + generator.Emit(OpCodes.Ldarg_0); - generator.Emit(OpCodes.Ldfld, containerFieldBuilder); + generator.Emit(OpCodes.Ldfld, helperFieldBuilder); + + generator.Emit(OpCodes.Ldloc_0); if (args.Any()) { @@ -93,8 +108,14 @@ public class TypedFactory : TypedFactoryBase, ITypedFactory< MethodInfo emptyArray = typeof(Array).GetMethod(nameof(Array.Empty))!.MakeGenericMethod(typeof(object)); generator.EmitCall(OpCodes.Call, emptyArray, null); } + + generator.EmitCall(OpCodes.Call, typeof(FactoryHelper).GetMethod(nameof(FactoryHelper.ConvertPassedNull), new[] { typeof(MethodBase), typeof(object?[]) })!, null); + generator.Emit(OpCodes.Stloc_1); + generator.Emit(OpCodes.Ldarg_0); + generator.Emit(OpCodes.Ldfld, containerFieldBuilder); + generator.Emit(OpCodes.Ldloc_1); - generator.EmitCall(OpCodes.Call, typeof(IocContainer).GetMethod(nameof(IocContainer.FactoryResolve), new[] { typeof(object[]) })!.MakeGenericMethod(createMethod.ReturnType), null); + generator.EmitCall(OpCodes.Call, typeof(IocContainer).GetMethod(nameof(IocContainer.FactoryResolve), new[] { typeof(object?[]) })!.MakeGenericMethod(createMethod.ReturnType), null); generator.Emit(OpCodes.Castclass, createMethod.ReturnType); generator.Emit(OpCodes.Ret); } @@ -106,7 +127,7 @@ public class TypedFactory : TypedFactoryBase, ITypedFactory< //create a method that looks like this //public void ClearMultitonInstance() //{ - // IIocContainer.ClearMultitonInstances(); + // IocContainer.ClearMultitonInstances(); //} if (multitonClearMethod.IsGenericMethod) @@ -134,6 +155,6 @@ public class TypedFactory : TypedFactoryBase, ITypedFactory< } } - return Creator.CreateInstance(typeBuilder.CreateTypeInfo()!.AsType(), container); + return Creator.CreateInstance(typeBuilder.CreateTypeInfo()!.AsType(), container, new FactoryHelper()); } } \ No newline at end of file diff --git a/LightweightIocContainer/IocContainer.cs b/LightweightIocContainer/IocContainer.cs index 04c1a50..abb5a1a 100644 --- a/LightweightIocContainer/IocContainer.cs +++ b/LightweightIocContainer/IocContainer.cs @@ -117,7 +117,7 @@ public class IocContainer : IIocContainer, IIocResolver /// The given /// The constructor arguments /// An instance of the given - public T FactoryResolve(params object[] arguments) => ResolveInternal(arguments, null, true); + public T FactoryResolve(params object?[] arguments) => ResolveInternal(arguments, null, true); /// /// Gets an instance of a given registered @@ -127,7 +127,7 @@ public class IocContainer : IIocContainer, IIocResolver /// The current resolve stack /// True if resolve is called from factory, false (default) if not /// An instance of the given registered - private T ResolveInternal(object[]? arguments, List? resolveStack = null, bool isFactoryResolve = false) => ResolveInstance(TryResolve(arguments, resolveStack, isFactoryResolve)); + private T ResolveInternal(object?[]? arguments, List? resolveStack = null, bool isFactoryResolve = false) => ResolveInstance(TryResolve(arguments, resolveStack, isFactoryResolve)); /// /// Tries to resolve the given with the given arguments @@ -484,8 +484,6 @@ public class IocContainer : IIocContainer, IIocResolver private (bool result, List? parameters, List? exceptions) TryGetConstructorResolveStack(ConstructorInfo constructor, object?[]? arguments, List resolveStack) { List constructorParameters = constructor.GetParameters().ToList(); - if (!constructorParameters.Any()) - return (true, null, null); List exceptions = new(); List parameters = new(); @@ -500,10 +498,15 @@ public class IocContainer : IIocContainer, IIocResolver if (passedArguments != null) { fittingArgument = passedArguments.FirstOrGiven(a => - a?.GetType() == parameter.ParameterType || parameter.ParameterType.IsInstanceOfType(a)); - + a?.GetType() == parameter.ParameterType || + parameter.ParameterType.IsInstanceOfType(a) || + a is NullParameter nullParameter && parameter.ParameterType.IsAssignableFrom(nullParameter.ParameterType)); + if (fittingArgument is not InternalResolvePlaceholder) passedArguments.Remove(fittingArgument); + + if (fittingArgument is NullParameter) + fittingArgument = null; } if (fittingArgument is InternalResolvePlaceholder) @@ -537,7 +540,6 @@ public class IocContainer : IIocContainer, IIocResolver exceptions.Add(new ConstructorNotMatchingException(constructor, new Exception("Not all given arguments were used!"))); return (false, parameters, exceptions); - } /// diff --git a/LightweightIocContainer/LightweightIocContainer.xml b/LightweightIocContainer/LightweightIocContainer.xml index 9fee98c..60272ca 100644 --- a/LightweightIocContainer/LightweightIocContainer.xml +++ b/LightweightIocContainer/LightweightIocContainer.xml @@ -356,6 +356,20 @@ implementation for custom implemented factories + + + Helper class for the + + + + + Convert `null` passed as argument to to handle it correctly + + The create method of the factory + The arguments passed to the create method + List of arguments with converted null + Wrong parameters passed + Class to help implement an abstract typed factory @@ -1197,6 +1211,22 @@ A new instance gets created if the given scope has no created instance yet. Otherwise the already created instance is used. + + + Wrapper class to handle null passed as an argument correctly + + + + + Wrapper class to handle null passed as an argument correctly + + The of the parameter + + + + The of the parameter + + An to register multiple interfaces for on implementation type that implements them as a multiton diff --git a/LightweightIocContainer/NullParameter.cs b/LightweightIocContainer/NullParameter.cs new file mode 100644 index 0000000..dea128b --- /dev/null +++ b/LightweightIocContainer/NullParameter.cs @@ -0,0 +1,22 @@ +// Author: Gockner, Simon +// Created: 2022-09-02 +// Copyright(c) 2022 SimonG. All Rights Reserved. + +namespace LightweightIocContainer; + +/// +/// Wrapper class to handle null passed as an argument correctly +/// +public class NullParameter +{ + /// + /// Wrapper class to handle null passed as an argument correctly + /// + /// The of the parameter + public NullParameter(Type parameterType) => ParameterType = parameterType; + + /// + /// The of the parameter + /// + public Type ParameterType { get; } +} \ No newline at end of file diff --git a/Test.LightweightIocContainer.Validation/IocValidatorTest.cs b/Test.LightweightIocContainer.Validation/IocValidatorTest.cs index eb1923f..aa3363b 100644 --- a/Test.LightweightIocContainer.Validation/IocValidatorTest.cs +++ b/Test.LightweightIocContainer.Validation/IocValidatorTest.cs @@ -33,12 +33,21 @@ public class IocValidatorTest public Test(IParameter parameter) => parameter.Method(); } - private class TestViewModel : ITest + [UsedImplicitly] + private class TestViewModelIgnoreDesignTimeCtor : ITest { - public TestViewModel(IParameter parameter) => parameter.Method(); + public TestViewModelIgnoreDesignTimeCtor(IParameter parameter) => parameter.Method(); [IocIgnoreConstructor] - public TestViewModel() => throw new Exception(); + public TestViewModelIgnoreDesignTimeCtor() => throw new Exception(); + } + + [UsedImplicitly] + private class TestViewModelDontIgnoreDesignTimeCtor : ITest + { + public TestViewModelDontIgnoreDesignTimeCtor(string name, IParameter parameter) => parameter.Method(); + + public TestViewModelDontIgnoreDesignTimeCtor() => throw new Exception(); } private class Parameter : IParameter @@ -52,6 +61,12 @@ public class IocValidatorTest ITest Create(IParameter parameter); } + [UsedImplicitly] + public interface ITestMissingParamFactory + { + ITest Create(string name); + } + [UsedImplicitly] public interface IInvalidTestFactory { @@ -97,11 +112,20 @@ public class IocValidatorTest } } + private class TestInstallerInvalidFactoryParameterDesignTimeCtorNotIgnoredRegisteredWithFactory : IIocInstaller + { + public void Install(IRegistrationCollector registration) + { + registration.Add().WithFactory(); + registration.Add().WithFactory(); + } + } + private class TestInstallerInvalidFactoryViewModel : IIocInstaller { public void Install(IRegistrationCollector registration) { - registration.Add().WithFactory(); + registration.Add().WithFactory(); registration.Add().WithFactory(); } } @@ -211,6 +235,31 @@ public class IocValidatorTest Assert.IsInstanceOf(iTest2CtorNotMatchingException.InnerExceptions[0]); } + + [Test] + public void TestValidateViewModelWithInvalidParameterDesignTimeCtorNotIgnoredWithFactory() + { + IocContainer iocContainer = new(); + iocContainer.Install(new TestInstallerInvalidFactoryParameterDesignTimeCtorNotIgnoredRegisteredWithFactory()); + + IocValidator validator = new(iocContainer); + + AggregateException aggregateException = Assert.Throws(() => validator.Validate()); + + if (aggregateException?.InnerExceptions[0] is not NoMatchingConstructorFoundException noMatchingConstructorFoundException) + { + Assert.Fail($"First element of {nameof(aggregateException.InnerExceptions)} is not of type {nameof(NoMatchingConstructorFoundException)}."); + return; + } + + if (noMatchingConstructorFoundException.InnerExceptions[0] is not ConstructorNotMatchingException iTest2CtorNotMatchingException) + { + Assert.Fail($"First element of {nameof(noMatchingConstructorFoundException.InnerExceptions)} is not of type {nameof(ConstructorNotMatchingException)}."); + return; + } + + Assert.IsInstanceOf(iTest2CtorNotMatchingException.InnerExceptions[0]); + } [Test] public void TestValidateInvalidFactory() diff --git a/Test.LightweightIocContainer/FluentFactoryRegistrationTest.cs b/Test.LightweightIocContainer/FluentFactoryRegistrationTest.cs index 5932050..1ca6ba3 100644 --- a/Test.LightweightIocContainer/FluentFactoryRegistrationTest.cs +++ b/Test.LightweightIocContainer/FluentFactoryRegistrationTest.cs @@ -43,7 +43,36 @@ public class FluentFactoryRegistrationTest } } + + private class TestNull : ITest + { + + public TestNull(object obj, string content, string optional1, string optional2, ITestNullFactory testNullFactory) + { + Obj = obj; + Content = content; + Optional1 = optional1; + Optional2 = optional2; + TestNullFactory = testNullFactory; + } + public object Obj { get; } + public string Content { get; } + public string Optional1 { get; } + public string Optional2 { get; } + public ITestNullFactory TestNullFactory { get; } + } + + public interface ITestNullFactory + { + ITest Create(object obj, string content, string optional1, string optional2); + } + + public interface ITestDefaultFactory + { + ITest Create(object obj, string content, string optional1, string optional2, ITestNullFactory testNullFactory = null); + } + private interface ITestFactoryNoCreate { @@ -68,9 +97,9 @@ public class FluentFactoryRegistrationTest private class TestFactory : ITestFactory { public ITest Create() => new Test(); - public ITest Create(string name) => throw new System.NotImplementedException(); - public ITest CreateTest(string name = null) => throw new System.NotImplementedException(); - public ITest Create(byte id) => throw new System.NotImplementedException(); + public ITest Create(string name) => throw new NotImplementedException(); + public ITest CreateTest(string name = null) => throw new NotImplementedException(); + public ITest Create(byte id) => throw new NotImplementedException(); } [UsedImplicitly] @@ -210,6 +239,55 @@ public class FluentFactoryRegistrationTest Assert.IsInstanceOf(createdTest); } + + [Test] + public void TestPassingNullAsMiddleParameter() + { + _iocContainer.Register(r => r.Add().WithFactory()); + + ITestNullFactory testNullFactory = _iocContainer.Resolve(); + + object obj = new(); + string content = "TestContent"; + string optional2 = "optionalParameter2"; + + ITest createdTest = testNullFactory.Create(obj, content, null, optional2); + if (createdTest is not TestNull testNull) + { + Assert.Fail(); + return; + } + + Assert.AreSame(obj, testNull.Obj); + Assert.AreEqual(content, testNull.Content); + Assert.AreEqual(null, testNull.Optional1); + Assert.AreEqual(optional2, testNull.Optional2); + } + + [Test] + public void TestPassingNullAsDefaultParameter() + { + _iocContainer.Register(r => r.Add().WithFactory()); + + ITestDefaultFactory testDefaultFactory = _iocContainer.Resolve(); + + object obj = new(); + string content = "TestContent"; + string optional2 = "optionalParameter2"; + + ITest createdTest = testDefaultFactory.Create(obj, content, null, optional2); + if (createdTest is not TestNull testNull) + { + Assert.Fail(); + return; + } + + Assert.AreSame(obj, testNull.Obj); + Assert.AreEqual(content, testNull.Content); + Assert.AreEqual(null, testNull.Optional1); + Assert.AreEqual(optional2, testNull.Optional2); + Assert.AreEqual(null, testNull.TestNullFactory); + } [Test] public void TestResolveMultitonFromFactory()