From 1413fee42864a2fed15fa8ec70fdcd860fe0b6e0 Mon Sep 17 00:00:00 2001 From: Jimmy Bogard Date: Tue, 5 Dec 2017 14:25:29 -0600 Subject: [PATCH 1/3] Making the delegate factory aware of creating certain collection types; remove collection mappers from assuming the destination type; fixes #2397; fixes #2435; fixes #2436; fixes #2449; fixes #2447 --- src/AutoMapper/Execution/DelegateFactory.cs | 24 +++++++++- .../CollectionMapperExpressionFactory.cs | 18 +++---- src/UnitTests/CollectionMapping.cs | 48 +++++++++++++++++++ src/UnitTests/Dictionaries.cs | 2 + 4 files changed, 83 insertions(+), 9 deletions(-) diff --git a/src/AutoMapper/Execution/DelegateFactory.cs b/src/AutoMapper/Execution/DelegateFactory.cs index b2bf7553eb..f3be598d4d 100644 --- a/src/AutoMapper/Execution/DelegateFactory.cs +++ b/src/AutoMapper/Execution/DelegateFactory.cs @@ -1,11 +1,15 @@ using System; +using System.Collections.Generic; using System.Linq; using System.Linq.Expressions; +using AutoMapper.Configuration; +using AutoMapper.Mappers.Internal; namespace AutoMapper.Execution { using static Expression; - using static ExpressionBuilder; + using static Internal.ExpressionFactory; + using static ElementTypeHelper; public static class DelegateFactory { @@ -44,6 +48,15 @@ public static Expression GenerateConstructorExpression(Type type) return Constant(null, typeof(string)); } + if (type.IsInterface()) + { + return type.ImplementsGenericInterface(typeof(IDictionary<,>)) + ? CreateCollection(type, typeof(Dictionary<,>)) + : (type.ImplementsGenericInterface(typeof(ICollection<>)) + ? CreateCollection(type, typeof(List<>)) + : InvalidType(type, $"Cannot create an instance of interface type {type}.")); + } + if (type.IsAbstract()) { return InvalidType(type, $"Cannot create an instance of abstract type {type}."); @@ -68,6 +81,15 @@ public static Expression GenerateConstructorExpression(Type type) return New(ctorWithOptionalArgs, args); } + private static Expression CreateCollection(Type type, Type collectionType) + { + var listType = collectionType.MakeGenericType(GetElementTypes(type, ElementTypeFlags.BreakKeyValuePair)); + if (type.IsAssignableFrom(listType)) + return ToType(New(listType), type); + + return InvalidType(type, $"Cannot create an instance of interface type {type}."); + } + private static Expression InvalidType(Type type, string message) { var ex = new ArgumentException(message, "type"); diff --git a/src/AutoMapper/Mappers/Internal/CollectionMapperExpressionFactory.cs b/src/AutoMapper/Mappers/Internal/CollectionMapperExpressionFactory.cs index d8b8458300..41778f8902 100644 --- a/src/AutoMapper/Mappers/Internal/CollectionMapperExpressionFactory.cs +++ b/src/AutoMapper/Mappers/Internal/CollectionMapperExpressionFactory.cs @@ -9,8 +9,9 @@ namespace AutoMapper.Mappers.Internal { using static Expression; - using static AutoMapper.Execution.ExpressionBuilder; + using static ExpressionBuilder; using static ExpressionFactory; + using static ElementTypeHelper; public static class CollectionMapperExpressionFactory { @@ -22,7 +23,7 @@ public static Expression MapCollectionExpression(IConfigurationProvider configur { var passedDestination = Variable(destExpression.Type, "passedDestination"); var newExpression = Variable(passedDestination.Type, "collectionDestination"); - var sourceElementType = ElementTypeHelper.GetElementType(sourceExpression.Type); + var sourceElementType = GetElementType(sourceExpression.Type); var itemExpr = mapItem(configurationProvider, profileMap, sourceExpression.Type, passedDestination.Type, contextExpression, out ParameterExpression itemParam); @@ -46,7 +47,8 @@ public static Expression MapCollectionExpression(IConfigurationProvider configur Assign(passedDestination, destExpression), assignNewExpression, Call(destination, clearMethod), - ToType(mapExpr, createInstance.Type) + mapExpr + //ToType(mapExpr, createInstance.Type) ); if (propertyMap != null) return checkNull; @@ -80,7 +82,7 @@ private static Expression NewExpr(this Type baseType, Type ifInterfaceType) { var newExpr = baseType.IsInterface() ? New( - ifInterfaceType.MakeGenericType(ElementTypeHelper.GetElementTypes(baseType, + ifInterfaceType.MakeGenericType(GetElementTypes(baseType, ElementTypeFlags.BreakKeyValuePair))) : DelegateFactory.GenerateConstructorExpression(baseType); return newExpr; @@ -88,8 +90,8 @@ private static Expression NewExpr(this Type baseType, Type ifInterfaceType) public static Expression MapItemExpr(IConfigurationProvider configurationProvider, ProfileMap profileMap, Type sourceType, Type destType, Expression contextParam, out ParameterExpression itemParam) { - var sourceElementType = ElementTypeHelper.GetElementType(sourceType); - var destElementType = ElementTypeHelper.GetElementType(destType); + var sourceElementType = GetElementType(sourceType); + var destElementType = GetElementType(destType); itemParam = Parameter(sourceElementType, "item"); var typePair = new TypePair(sourceElementType, destElementType); @@ -100,8 +102,8 @@ public static Expression MapItemExpr(IConfigurationProvider configurationProvide public static Expression MapKeyPairValueExpr(IConfigurationProvider configurationProvider, ProfileMap profileMap, Type sourceType, Type destType, Expression contextParam, out ParameterExpression itemParam) { - var sourceElementTypes = ElementTypeHelper.GetElementTypes(sourceType, ElementTypeFlags.BreakKeyValuePair); - var destElementTypes = ElementTypeHelper.GetElementTypes(destType, ElementTypeFlags.BreakKeyValuePair); + var sourceElementTypes = GetElementTypes(sourceType, ElementTypeFlags.BreakKeyValuePair); + var destElementTypes = GetElementTypes(destType, ElementTypeFlags.BreakKeyValuePair); var typePairKey = new TypePair(sourceElementTypes[0], destElementTypes[0]); var typePairValue = new TypePair(sourceElementTypes[1], destElementTypes[1]); diff --git a/src/UnitTests/CollectionMapping.cs b/src/UnitTests/CollectionMapping.cs index 4c41e6be53..a8705f91f0 100644 --- a/src/UnitTests/CollectionMapping.cs +++ b/src/UnitTests/CollectionMapping.cs @@ -616,6 +616,54 @@ public void Should_not_replace_destination_list() } } + public class When_mapping_from_ICollection_types_but_implementations_are_different : AutoMapperSpecBase + { + public class Source + { + public ICollection Items { get; set; } + + public class Item + { + public int Value { get; set; } + } + } + public class Dest + { + public ICollection Items { get; set; } = new HashSet(); + + public class Item + { + public int Value { get; set; } + } + } + + protected override MapperConfiguration Configuration { get; } = new MapperConfiguration(cfg => + { + cfg.CreateMap(); + cfg.CreateMap(); + }); + + [Fact] + public void Should_map_items() + { + var source = new Source + { + Items = new List + { + new Source.Item { Value = 5 } + } + }; + var dest = new Dest(); + + var plan = Mapper.ConfigurationProvider.FindTypeMapFor().MapExpression; + + Mapper.Map(source, dest); + + dest.Items.Count.ShouldBe(1); + dest.Items.First().Value.ShouldBe(5); + } + } + public class When_mapping_enumerable_to_array : AutoMapperSpecBase { public class Source diff --git a/src/UnitTests/Dictionaries.cs b/src/UnitTests/Dictionaries.cs index 60c217ff31..5a5349229a 100644 --- a/src/UnitTests/Dictionaries.cs +++ b/src/UnitTests/Dictionaries.cs @@ -192,6 +192,8 @@ protected override void Because_of() } }; + var plan = Configuration.FindTypeMapFor().MapExpression; + _result = Mapper.Map(foo1); } From a01ceeecac3f20937b1ad67a1cae5e31e9de63ed Mon Sep 17 00:00:00 2001 From: Jimmy Bogard Date: Tue, 5 Dec 2017 14:25:41 -0600 Subject: [PATCH 2/3] Cleanup --- .../Mappers/Internal/CollectionMapperExpressionFactory.cs | 6 ++---- src/UnitTests/CollectionMapping.cs | 2 -- src/UnitTests/Dictionaries.cs | 2 -- 3 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/AutoMapper/Mappers/Internal/CollectionMapperExpressionFactory.cs b/src/AutoMapper/Mappers/Internal/CollectionMapperExpressionFactory.cs index 41778f8902..1b02e60a8b 100644 --- a/src/AutoMapper/Mappers/Internal/CollectionMapperExpressionFactory.cs +++ b/src/AutoMapper/Mappers/Internal/CollectionMapperExpressionFactory.cs @@ -34,7 +34,7 @@ public static Expression MapCollectionExpression(IConfigurationProvider configur destinationCollectionType = typeof(IList); var addMethod = destinationCollectionType.GetDeclaredMethod("Add"); - Expression destination, createInstance, assignNewExpression; + Expression destination, assignNewExpression; UseDestinationValue(); @@ -48,7 +48,6 @@ public static Expression MapCollectionExpression(IConfigurationProvider configur assignNewExpression, Call(destination, clearMethod), mapExpr - //ToType(mapExpr, createInstance.Type) ); if (propertyMap != null) return checkNull; @@ -63,14 +62,13 @@ void UseDestinationValue() { if(propertyMap?.UseDestinationValue == true) { - createInstance = passedDestination; destination = passedDestination; assignNewExpression = Empty(); } else { destination = newExpression; - createInstance = passedDestination.Type.NewExpr(ifInterfaceType); + Expression createInstance = passedDestination.Type.NewExpr(ifInterfaceType); var isReadOnly = Property(ToType(passedDestination, destinationCollectionType), "IsReadOnly"); assignNewExpression = Assign(newExpression, Condition(OrElse(Equal(passedDestination, Constant(null)), isReadOnly), ToType(createInstance, passedDestination.Type), passedDestination)); diff --git a/src/UnitTests/CollectionMapping.cs b/src/UnitTests/CollectionMapping.cs index a8705f91f0..3dfe2d5c2e 100644 --- a/src/UnitTests/CollectionMapping.cs +++ b/src/UnitTests/CollectionMapping.cs @@ -655,8 +655,6 @@ public void Should_map_items() }; var dest = new Dest(); - var plan = Mapper.ConfigurationProvider.FindTypeMapFor().MapExpression; - Mapper.Map(source, dest); dest.Items.Count.ShouldBe(1); diff --git a/src/UnitTests/Dictionaries.cs b/src/UnitTests/Dictionaries.cs index 5a5349229a..60c217ff31 100644 --- a/src/UnitTests/Dictionaries.cs +++ b/src/UnitTests/Dictionaries.cs @@ -192,8 +192,6 @@ protected override void Because_of() } }; - var plan = Configuration.FindTypeMapFor().MapExpression; - _result = Mapper.Map(foo1); } From 6deb4c1382e449e56cde2410e9634407c739558b Mon Sep 17 00:00:00 2001 From: Jimmy Bogard Date: Tue, 5 Dec 2017 14:26:28 -0600 Subject: [PATCH 3/3] Minor version bump --- src/AutoMapper/AutoMapper.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/AutoMapper/AutoMapper.csproj b/src/AutoMapper/AutoMapper.csproj index 300d9e1a80..301a964444 100644 --- a/src/AutoMapper/AutoMapper.csproj +++ b/src/AutoMapper/AutoMapper.csproj @@ -3,7 +3,7 @@ A convention-based object-object mapper A convention-based object-object mapper. AutoMapper uses a fluent configuration API to define an object-object mapping strategy. AutoMapper uses a convention-based matching algorithm to match up source to destination values. Currently, AutoMapper is designed for model projection scenarios to flatten complex object models to DTOs and other simple objects, whose design is better suited for serialization, communication, messaging, or simply an anti-corruption layer between the domain and application layer. - 6.2.1 + 6.2.2 Jimmy Bogard netstandard1.3;netstandard1.1;net45;net40 $(NoWarn);1591