From 8dc852f816db9dac247859ef6a4bbc76c86da487 Mon Sep 17 00:00:00 2001 From: Steve Wilkes Date: Thu, 13 Sep 2018 10:58:52 +0100 Subject: [PATCH 1/5] Skipping target members lookup for enumerable members --- AgileMapper.UnitTests/WhenMappingToNewComplexTypes.cs | 9 +++++++++ AgileMapper/Members/MemberCache.cs | 5 +++++ 2 files changed, 14 insertions(+) diff --git a/AgileMapper.UnitTests/WhenMappingToNewComplexTypes.cs b/AgileMapper.UnitTests/WhenMappingToNewComplexTypes.cs index 09f11ab77..77ea99c05 100644 --- a/AgileMapper.UnitTests/WhenMappingToNewComplexTypes.cs +++ b/AgileMapper.UnitTests/WhenMappingToNewComplexTypes.cs @@ -1,5 +1,6 @@ namespace AgileObjects.AgileMapper.UnitTests { + using System.Security.Claims; using Common; using TestClasses; #if !NET35 @@ -105,6 +106,14 @@ public void ShouldMapToAGivenTypeObject() ((PublicField)result).Value.ShouldBe("kjubfelkjnds;lkmm"); } + // See https://github.com/agileobjects/AgileMapper/issues/97 + [Fact] + public void ShouldDeepCloneAClaim() + { + var claim = new Claim("test", "hello!"); + var cloned = Mapper.DeepClone(claim); + } + [Fact] public void ShouldHandleAnUnconstructableRootTargetType() { diff --git a/AgileMapper/Members/MemberCache.cs b/AgileMapper/Members/MemberCache.cs index 55e3c51b7..56d64e410 100644 --- a/AgileMapper/Members/MemberCache.cs +++ b/AgileMapper/Members/MemberCache.cs @@ -40,6 +40,11 @@ public IList GetTargetMembers(Type targetType) { return _membersCache.GetOrAdd(TypeKey.ForTargetMembers(targetType), key => { + if (key.Type.IsEnumerable()) + { + return Enumerable.EmptyArray; + } + var fields = GetFields(key.Type, All); var properties = GetProperties(key.Type, All); var methods = GetMethods(key.Type, OnlyCallableSetters, Member.SetMethod); From f27d43643f6ad3d60f260cec05fc5d053ef1af79 Mon Sep 17 00:00:00 2001 From: Steve Wilkes Date: Fri, 14 Sep 2018 09:01:14 +0100 Subject: [PATCH 2/5] Updating --- .../WhenMappingToNewComplexTypes.cs | 10 ++++++---- .../DictionaryMappingExpressionFactory.cs | 12 ++++++------ 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/AgileMapper.UnitTests/WhenMappingToNewComplexTypes.cs b/AgileMapper.UnitTests/WhenMappingToNewComplexTypes.cs index 77ea99c05..d365b37d9 100644 --- a/AgileMapper.UnitTests/WhenMappingToNewComplexTypes.cs +++ b/AgileMapper.UnitTests/WhenMappingToNewComplexTypes.cs @@ -1,6 +1,6 @@ namespace AgileObjects.AgileMapper.UnitTests { - using System.Security.Claims; + using System.Collections.Generic; using Common; using TestClasses; #if !NET35 @@ -108,10 +108,12 @@ public void ShouldMapToAGivenTypeObject() // See https://github.com/agileobjects/AgileMapper/issues/97 [Fact] - public void ShouldDeepCloneAClaim() + public void ShouldDeepCloneAReadOnlyDictionary() { - var claim = new Claim("test", "hello!"); - var cloned = Mapper.DeepClone(claim); + var source = new PublicReadOnlyProperty>( + new Dictionary()); + + var cloned = Mapper.DeepClone(source); } [Fact] diff --git a/AgileMapper/ObjectPopulation/DictionaryMappingExpressionFactory.cs b/AgileMapper/ObjectPopulation/DictionaryMappingExpressionFactory.cs index dc3937369..93a05d5d6 100644 --- a/AgileMapper/ObjectPopulation/DictionaryMappingExpressionFactory.cs +++ b/AgileMapper/ObjectPopulation/DictionaryMappingExpressionFactory.cs @@ -291,7 +291,7 @@ protected override IEnumerable GetObjectPopulation(MappingCreationCo { if (UseDictionaryCloneConstructor(sourceDictionaryMember, mapperData, out var cloneConstructor)) { - yield return GetClonedDictionaryAssignment(mapperData, cloneConstructor); + yield return GetClonedDictionaryAssignment(context.MappingData, cloneConstructor); yield break; } @@ -346,21 +346,21 @@ private static ConstructorInfo GetDictionaryCloneConstructor(ITypePair mapperDat (comparerProperty != null) ? 2 : 1); } - private static Expression GetClonedDictionaryAssignment(IMemberMapperData mapperData, ConstructorInfo cloneConstructor) + private static Expression GetClonedDictionaryAssignment(IObjectMappingData mappingData, ConstructorInfo cloneConstructor) { Expression cloneDictionary; if (cloneConstructor.GetParameters().Length == 1) { - cloneDictionary = Expression.New(cloneConstructor, mapperData.SourceObject); + cloneDictionary = Expression.New(cloneConstructor, mappingData.MapperData.SourceObject); } else { - var comparer = Expression.Property(mapperData.SourceObject, "Comparer"); - cloneDictionary = Expression.New(cloneConstructor, mapperData.SourceObject, comparer); + var comparer = Expression.Property(mappingData.MapperData.SourceObject, "Comparer"); + cloneDictionary = Expression.New(cloneConstructor, mappingData.MapperData.SourceObject, comparer); } - var assignment = mapperData.TargetInstance.AssignTo(cloneDictionary); + var assignment = GetDictionaryAssignment(cloneDictionary, mappingData); return assignment; } From 8882bc2cff81a4a5c448fbe7d2ebb29d85bf12dd Mon Sep 17 00:00:00 2001 From: Steve Wilkes Date: Fri, 14 Sep 2018 12:24:42 +0100 Subject: [PATCH 3/5] Short-circuiting GetTargetMembers checks --- .../WhenMappingToNewDictionaryMembers.cs | 10 +++ .../WhenMappingToNewComplexTypes.cs | 11 --- .../DictionaryMappingExpressionFactory.cs | 71 ++++++++++++------- .../ObjectPopulation/ObjectMapperData.cs | 7 ++ 4 files changed, 62 insertions(+), 37 deletions(-) diff --git a/AgileMapper.UnitTests/Dictionaries/WhenMappingToNewDictionaryMembers.cs b/AgileMapper.UnitTests/Dictionaries/WhenMappingToNewDictionaryMembers.cs index 52a91ba22..332ef6152 100644 --- a/AgileMapper.UnitTests/Dictionaries/WhenMappingToNewDictionaryMembers.cs +++ b/AgileMapper.UnitTests/Dictionaries/WhenMappingToNewDictionaryMembers.cs @@ -184,6 +184,16 @@ public void ShouldMapADictionaryObjectValuesToNewDictionaryObjectValues() result.Value["key2"].ShouldNotBeSameAs(source.Value["key2"]); } + // See https://github.com/agileobjects/AgileMapper/issues/97 + [Fact] + public void ShouldDeepCloneAReadOnlyDictionaryMember() + { + var source = new PublicReadOnlyProperty>( + new Dictionary()); + + var cloned = Mapper.DeepClone(source); + } + [Fact] public void ShouldFlattenAComplexTypeCollectionToANestedObjectDictionaryImplementation() { diff --git a/AgileMapper.UnitTests/WhenMappingToNewComplexTypes.cs b/AgileMapper.UnitTests/WhenMappingToNewComplexTypes.cs index d365b37d9..09f11ab77 100644 --- a/AgileMapper.UnitTests/WhenMappingToNewComplexTypes.cs +++ b/AgileMapper.UnitTests/WhenMappingToNewComplexTypes.cs @@ -1,6 +1,5 @@ namespace AgileObjects.AgileMapper.UnitTests { - using System.Collections.Generic; using Common; using TestClasses; #if !NET35 @@ -106,16 +105,6 @@ public void ShouldMapToAGivenTypeObject() ((PublicField)result).Value.ShouldBe("kjubfelkjnds;lkmm"); } - // See https://github.com/agileobjects/AgileMapper/issues/97 - [Fact] - public void ShouldDeepCloneAReadOnlyDictionary() - { - var source = new PublicReadOnlyProperty>( - new Dictionary()); - - var cloned = Mapper.DeepClone(source); - } - [Fact] public void ShouldHandleAnUnconstructableRootTargetType() { diff --git a/AgileMapper/ObjectPopulation/DictionaryMappingExpressionFactory.cs b/AgileMapper/ObjectPopulation/DictionaryMappingExpressionFactory.cs index 93a05d5d6..0857df2f6 100644 --- a/AgileMapper/ObjectPopulation/DictionaryMappingExpressionFactory.cs +++ b/AgileMapper/ObjectPopulation/DictionaryMappingExpressionFactory.cs @@ -277,40 +277,59 @@ protected override bool TargetCannotBeMapped(IObjectMappingData mappingData, out protected override IEnumerable GetObjectPopulation(MappingCreationContext context) { - var mapperData = context.MapperData; - - if (!mapperData.TargetMember.IsDictionary) + if (!context.MapperData.TargetMember.IsDictionary) { yield return GetDictionaryPopulation(context.MappingData); yield break; } - Func assignmentFactory; + var assignmentFactory = GetDictionaryAssignmentFactoryOrNull(context, out var useAssignmentOnly); + + if (useAssignmentOnly) + { + yield return assignmentFactory.Invoke(context.MappingData); + yield break; + } + + var population = GetDictionaryPopulation(context.MappingData); + var assignment = assignmentFactory?.Invoke(context.MappingData); + + yield return assignment; + yield return population; + } + + private static Func GetDictionaryAssignmentFactoryOrNull( + MappingCreationContext context, + out bool useAssignmentOnly) + { + if (context.MapperData.TargetMember.IsReadOnly) + { + useAssignmentOnly = false; + return null; + } + + var mapperData = context.MapperData; if (SourceMemberIsDictionary(mapperData, out var sourceDictionaryMember)) { if (UseDictionaryCloneConstructor(sourceDictionaryMember, mapperData, out var cloneConstructor)) { - yield return GetClonedDictionaryAssignment(context.MappingData, cloneConstructor); - yield break; + useAssignmentOnly = true; + return md => GetClonedDictionaryAssignment(md.MapperData, cloneConstructor); } - assignmentFactory = GetMappedDictionaryAssignment; - } - else if (context.InstantiateLocalVariable) - { - assignmentFactory = (dsm, md) => GetParameterlessDictionaryAssignment(md); + useAssignmentOnly = false; + return md => GetMappedDictionaryAssignment(sourceDictionaryMember, md); } - else + + useAssignmentOnly = false; + + if (context.InstantiateLocalVariable) { - assignmentFactory = null; + return GetParameterlessDictionaryAssignment; } - var population = GetDictionaryPopulation(context.MappingData); - var assignment = assignmentFactory?.Invoke(sourceDictionaryMember, context.MappingData); - - yield return assignment; - yield return population; + return null; } private static bool SourceMemberIsDictionary( @@ -328,9 +347,9 @@ private static bool UseDictionaryCloneConstructor( { cloneConstructor = null; - return mapperData.TargetMember.ElementType.IsSimple() && - (sourceDictionaryMember.Type == mapperData.TargetType) && - ((cloneConstructor = GetDictionaryCloneConstructor(mapperData)) != null); + return (sourceDictionaryMember.Type == mapperData.TargetType) && + mapperData.TargetMember.ElementType.IsSimple() && + ((cloneConstructor = GetDictionaryCloneConstructor(mapperData)) != null); } private static ConstructorInfo GetDictionaryCloneConstructor(ITypePair mapperData) @@ -346,21 +365,21 @@ private static ConstructorInfo GetDictionaryCloneConstructor(ITypePair mapperDat (comparerProperty != null) ? 2 : 1); } - private static Expression GetClonedDictionaryAssignment(IObjectMappingData mappingData, ConstructorInfo cloneConstructor) + private static Expression GetClonedDictionaryAssignment(IMemberMapperData mapperData, ConstructorInfo cloneConstructor) { Expression cloneDictionary; if (cloneConstructor.GetParameters().Length == 1) { - cloneDictionary = Expression.New(cloneConstructor, mappingData.MapperData.SourceObject); + cloneDictionary = Expression.New(cloneConstructor, mapperData.SourceObject); } else { - var comparer = Expression.Property(mappingData.MapperData.SourceObject, "Comparer"); - cloneDictionary = Expression.New(cloneConstructor, mappingData.MapperData.SourceObject, comparer); + var comparer = Expression.Property(mapperData.SourceObject, "Comparer"); + cloneDictionary = Expression.New(cloneConstructor, mapperData.SourceObject, comparer); } - var assignment = GetDictionaryAssignment(cloneDictionary, mappingData); + var assignment = mapperData.TargetInstance.AssignTo(cloneDictionary); return assignment; } diff --git a/AgileMapper/ObjectPopulation/ObjectMapperData.cs b/AgileMapper/ObjectPopulation/ObjectMapperData.cs index 6e3075a45..aaf8d1940 100644 --- a/AgileMapper/ObjectPopulation/ObjectMapperData.cs +++ b/AgileMapper/ObjectPopulation/ObjectMapperData.cs @@ -239,6 +239,13 @@ private static bool TypeHasACompatibleChildMember( checkedTypes.Add(parentType); + if (parentType.IsEnumerable()) + { + parentType = parentType.GetEnumerableElementType(); + + return !parentType.IsSimple() && TypeHasACompatibleChildMember(targetType, parentType, checkedTypes); + } + var childTargetMembers = GlobalContext.Instance.MemberCache.GetTargetMembers(parentType); foreach (var childMember in childTargetMembers) From 8a24e8d0937f85d61c62a3b0722bd8c80e870cbb Mon Sep 17 00:00:00 2001 From: Steve Wilkes Date: Fri, 14 Sep 2018 12:41:42 +0100 Subject: [PATCH 4/5] Updating test to cover mapping to a populated, readonly Dictionary member --- .../WhenMappingToNewDictionaryMembers.cs | 25 +++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/AgileMapper.UnitTests/Dictionaries/WhenMappingToNewDictionaryMembers.cs b/AgileMapper.UnitTests/Dictionaries/WhenMappingToNewDictionaryMembers.cs index 332ef6152..e12c31e8f 100644 --- a/AgileMapper.UnitTests/Dictionaries/WhenMappingToNewDictionaryMembers.cs +++ b/AgileMapper.UnitTests/Dictionaries/WhenMappingToNewDictionaryMembers.cs @@ -188,10 +188,14 @@ public void ShouldMapADictionaryObjectValuesToNewDictionaryObjectValues() [Fact] public void ShouldDeepCloneAReadOnlyDictionaryMember() { - var source = new PublicReadOnlyProperty>( - new Dictionary()); + var source = new Issue97.ReadonlyDictionary(); + + source.Dictionary["Test"] = "123"; var cloned = Mapper.DeepClone(source); + + cloned.Dictionary.ContainsKey("Test").ShouldBeTrue(); + cloned.Dictionary["Test"].ShouldBe("123"); } [Fact] @@ -244,5 +248,22 @@ public void ShouldFlattenAComplexTypeCollectionToANestedObjectDictionaryImplemen result.Value.ShouldNotContainKey("[1].Address.Line2"); } + + #region Helper Members + + private static class Issue97 + { + public class ReadonlyDictionary + { + public ReadonlyDictionary() + { + Dictionary = new Dictionary(); + } + + public IDictionary Dictionary { get; } + } + } + + #endregion } } From 4119d8f3c3aaeb99df6f9a71015d6471ad04415c Mon Sep 17 00:00:00 2001 From: Steve Wilkes Date: Fri, 14 Sep 2018 17:31:14 +0100 Subject: [PATCH 5/5] Stopping Dictionary from being created as fallback complex type data source / Extra dictionary mapping test coverage --- .../WhenMappingToNewDictionaryMembers.cs | 23 +++++++++++++++++++ .../Finders/SourceMemberDataSourceFinder.cs | 5 +++- .../Internal/EnumerableExtensions.cs | 3 +++ AgileMapper/Members/SourceMemberMatcher.cs | 1 - .../MappingDataCreationFactory.cs | 2 +- 5 files changed, 31 insertions(+), 3 deletions(-) diff --git a/AgileMapper.UnitTests/Dictionaries/WhenMappingToNewDictionaryMembers.cs b/AgileMapper.UnitTests/Dictionaries/WhenMappingToNewDictionaryMembers.cs index e12c31e8f..78e8b9664 100644 --- a/AgileMapper.UnitTests/Dictionaries/WhenMappingToNewDictionaryMembers.cs +++ b/AgileMapper.UnitTests/Dictionaries/WhenMappingToNewDictionaryMembers.cs @@ -198,6 +198,29 @@ public void ShouldDeepCloneAReadOnlyDictionaryMember() cloned.Dictionary["Test"].ShouldBe("123"); } + [Fact] + public void ShouldUseACloneConstructorToPopulateADictionaryConstructorParameter() + { + var source = new PublicReadOnlyProperty>( + new Dictionary { ["Test"] = "Hello!" }); + + var result = Mapper.Map(source).ToANew>>(); + + result.Value.ContainsKey("Test").ShouldBeTrue(); + result.Value["Test"].ShouldBe("Hello!"); + } + + [Fact] + public void ShouldNotCreateDictionaryAsFallbackComplexType() + { + var source = new PublicReadOnlyProperty>( + new Dictionary()); + + var cloned = Mapper.DeepClone(source); + + cloned.ShouldBeNull(); + } + [Fact] public void ShouldFlattenAComplexTypeCollectionToANestedObjectDictionaryImplementation() { diff --git a/AgileMapper/DataSources/Finders/SourceMemberDataSourceFinder.cs b/AgileMapper/DataSources/Finders/SourceMemberDataSourceFinder.cs index a6fda1c74..7a59d2630 100644 --- a/AgileMapper/DataSources/Finders/SourceMemberDataSourceFinder.cs +++ b/AgileMapper/DataSources/Finders/SourceMemberDataSourceFinder.cs @@ -22,7 +22,7 @@ public IEnumerable FindFor(DataSourceFindContext context) { if (context.DataSourceIndex == 0) { - if (targetMember.IsComplex && (targetMember.Type != typeof(object))) + if (UseFallbackComplexTypeMappingDataSource(targetMember)) { yield return new ComplexTypeMappingDataSource(context.DataSourceIndex, context.ChildMappingData); } @@ -61,5 +61,8 @@ private static IDataSource GetSourceMemberDataSourceOrNull(DataSourceFindContext return context.GetFinalDataSource(sourceMemberDataSource, contextMappingData); } + + private static bool UseFallbackComplexTypeMappingDataSource(QualifiedMember targetMember) + => targetMember.IsComplex && !targetMember.IsDictionary && (targetMember.Type != typeof(object)); } } \ No newline at end of file diff --git a/AgileMapper/Extensions/Internal/EnumerableExtensions.cs b/AgileMapper/Extensions/Internal/EnumerableExtensions.cs index e6bcba0fb..b106f25ac 100644 --- a/AgileMapper/Extensions/Internal/EnumerableExtensions.cs +++ b/AgileMapper/Extensions/Internal/EnumerableExtensions.cs @@ -43,6 +43,7 @@ public static void AddRange(this List items, IEnu [DebuggerStepThrough] public static T First(this IList items) => items[0]; + [DebuggerStepThrough] public static T First(this IList items, Func predicate) { if (TryFindMatch(items, predicate, out var match)) @@ -53,9 +54,11 @@ public static T First(this IList items, Func predicate) throw new InvalidOperationException("Sequence contains no matching element"); } + [DebuggerStepThrough] public static T FirstOrDefault(this IList items, Func predicate) => TryFindMatch(items, predicate, out var match) ? match : default(T); + [DebuggerStepThrough] public static bool TryFindMatch(this IList items, Func predicate, out T match) { for (int i = 0, n = items.Count; i < n; i++) diff --git a/AgileMapper/Members/SourceMemberMatcher.cs b/AgileMapper/Members/SourceMemberMatcher.cs index 49bc9010a..fa5e0a1d2 100644 --- a/AgileMapper/Members/SourceMemberMatcher.cs +++ b/AgileMapper/Members/SourceMemberMatcher.cs @@ -4,7 +4,6 @@ using System.Collections.Generic; using System.Linq; using Extensions; - using Extensions.Internal; internal static class SourceMemberMatcher { diff --git a/AgileMapper/ObjectPopulation/MappingDataCreationFactory.cs b/AgileMapper/ObjectPopulation/MappingDataCreationFactory.cs index 59fce1258..d7067d4f8 100644 --- a/AgileMapper/ObjectPopulation/MappingDataCreationFactory.cs +++ b/AgileMapper/ObjectPopulation/MappingDataCreationFactory.cs @@ -35,7 +35,7 @@ private static bool UseAsConversion(ObjectMapperData childMapperData, out Expres return false; } - //[DebuggerStepThrough] + [DebuggerStepThrough] public static Expression ForChild( MappingValues mappingValues, int dataSourceIndex,