Fixing population of readonly dictionary members (#99)

* Skipping target members lookup for enumerable members

* Updating

* Short-circuiting GetTargetMembers checks

* Updating test to cover mapping to a populated, readonly Dictionary member

* Stopping Dictionary from being created as fallback complex type data source / Extra dictionary mapping test coverage

Author: SteveWilkes

AgileMapper.UnitTests/Dictionaries/WhenMappingToNewDictionaryMembers.cs

@@ -184,6 +184,43 @@ 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 Issue97.ReadonlyDictionary();
+
+            source.Dictionary["Test"] = "123";
+
+            var cloned = Mapper.DeepClone(source);
+
+            cloned.Dictionary.ContainsKey("Test").ShouldBeTrue();
+            cloned.Dictionary["Test"].ShouldBe("123");
+        }
+
+        [Fact]
+        public void ShouldUseACloneConstructorToPopulateADictionaryConstructorParameter()
+        {
+            var source = new PublicReadOnlyProperty<IDictionary<string, string>>(
+                new Dictionary<string, string> { ["Test"] = "Hello!" });
+
+            var result = Mapper.Map(source).ToANew<PublicCtor<IDictionary<string, string>>>();
+
+            result.Value.ContainsKey("Test").ShouldBeTrue();
+            result.Value["Test"].ShouldBe("Hello!");
+        }
+
+        [Fact]
+        public void ShouldNotCreateDictionaryAsFallbackComplexType()
+        {
+            var source = new PublicReadOnlyProperty<IDictionary<string, string>>(
+                new Dictionary<string, string>());
+
+            var cloned = Mapper.DeepClone(source);
+
+            cloned.ShouldBeNull();
+        }
+
         [Fact]
         public void ShouldFlattenAComplexTypeCollectionToANestedObjectDictionaryImplementation()
         {
@@ -234,5 +271,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<string, string>();
+                }
+
+                public IDictionary<string, string> Dictionary { get; }
+            }
+        }
+
+        #endregion
     }
 }

AgileMapper/DataSources/Finders/SourceMemberDataSourceFinder.cs

@@ -22,7 +22,7 @@ public IEnumerable<IDataSource> 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));
     }
 }

AgileMapper/Extensions/Internal/EnumerableExtensions.cs

@@ -43,6 +43,7 @@ public static void AddRange<TContained, TItem>(this List<TContained> items, IEnu
         [DebuggerStepThrough]
         public static T First<T>(this IList<T> items) => items[0];
 
+        [DebuggerStepThrough]
         public static T First<T>(this IList<T> items, Func<T, bool> predicate)
         {
             if (TryFindMatch(items, predicate, out var match))
@@ -53,9 +54,11 @@ public static T First<T>(this IList<T> items, Func<T, bool> predicate)
             throw new InvalidOperationException("Sequence contains no matching element");
         }
 
+        [DebuggerStepThrough]
         public static T FirstOrDefault<T>(this IList<T> items, Func<T, bool> predicate)
             => TryFindMatch(items, predicate, out var match) ? match : default(T);
 
+        [DebuggerStepThrough]
         public static bool TryFindMatch<T>(this IList<T> items, Func<T, bool> predicate, out T match)
         {
             for (int i = 0, n = items.Count; i < n; i++)

AgileMapper/Members/MemberCache.cs

@@ -40,6 +40,11 @@ public IList<Member> GetTargetMembers(Type targetType)
         {
             return _membersCache.GetOrAdd(TypeKey.ForTargetMembers(targetType), key =>
             {
+                if (key.Type.IsEnumerable())
+                {
+                    return Enumerable<Member>.EmptyArray;
+                }
+
                 var fields = GetFields(key.Type, All);
                 var properties = GetProperties(key.Type, All);
                 var methods = GetMethods(key.Type, OnlyCallableSetters, Member.SetMethod);

AgileMapper/Members/SourceMemberMatcher.cs

@@ -4,7 +4,6 @@
     using System.Collections.Generic;
     using System.Linq;
     using Extensions;
-    using Extensions.Internal;
 
     internal static class SourceMemberMatcher
     {

AgileMapper/ObjectPopulation/DictionaryMappingExpressionFactory.cs

@@ -277,40 +277,59 @@ protected override bool TargetCannotBeMapped(IObjectMappingData mappingData, out
 
         protected override IEnumerable<Expression> GetObjectPopulation(MappingCreationContext context)
         {
-            var mapperData = context.MapperData;
-
-            if (!mapperData.TargetMember.IsDictionary)
+            if (!context.MapperData.TargetMember.IsDictionary)
             {
                 yield return GetDictionaryPopulation(context.MappingData);
                 yield break;
             }
 
-            Func<DictionarySourceMember, IObjectMappingData, Expression> 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<IObjectMappingData, Expression> 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(mapperData, 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)

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,

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)