Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;

Expand All @@ -8,15 +7,18 @@ namespace AutoMapper.Configuration.Conventions
public class SourceToDestinationNameMapperAttributesMember : ISourceToDestinationNameMapper
{
private static readonly SourceMember[] Empty = new SourceMember[0];
private readonly Dictionary<TypeDetails, SourceMember[]> _allSourceMembers = new Dictionary<TypeDetails, SourceMember[]>();
private LockingConcurrentDictionary<TypeDetails, SourceMember[]> _allSourceMembers
= new LockingConcurrentDictionary<TypeDetails, SourceMember[]>(_ => Empty);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should lock here. It's difficult to write correct thread safe code in a lot of places. I think we should lock when calling CreateMap.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already lock there but this is another place that's a static cache. Another option would be just to get rid of the cache.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea was to not look for the attribute every time it tries to match the members to map. So it will be slower without the cache. But my point point is more general. A coarser grained lock would make the code easier to write and understand. That's why I added a global lock before each CreateMap. So we don't have to worry about concurrency at every point.


public MemberInfo GetMatchingMemberInfo(IGetTypeInfoMembers getTypeInfoMembers, TypeDetails typeInfo, Type destType, Type destMemberType, string nameToSearch)
{
if (!_allSourceMembers.TryGetValue(typeInfo, out SourceMember[] sourceMembers))
{
sourceMembers = getTypeInfoMembers.GetMemberInfos(typeInfo).Select(sourceMember => new SourceMember(sourceMember)).Where(s => s.Attribute != null).ToArray();
_allSourceMembers[typeInfo] = sourceMembers.Length == 0 ? Empty : sourceMembers;
}
SourceMember[] ValueFactory(TypeDetails td) => getTypeInfoMembers.GetMemberInfos(td)
.Select(sourceMember => new SourceMember(sourceMember))
.Where(s => s.Attribute != null)
.ToArray();

var sourceMembers = _allSourceMembers.GetOrAdd(typeInfo, td => new Lazy<SourceMember[]>(() => ValueFactory(td)));

return sourceMembers.FirstOrDefault(d => d.Attribute.IsMatch(typeInfo, d.Member, destType, destMemberType, nameToSearch)).Member;
}

Expand Down
1 change: 0 additions & 1 deletion src/AutoMapper/Configuration/ITypeMapConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ namespace AutoMapper.Configuration
public interface ITypeMapConfiguration
{
void Configure(TypeMap typeMap);
MemberList MemberList { get; }
Type SourceType { get; }
Type DestinationType { get; }
bool IsOpenGeneric { get; }
Expand Down
3 changes: 1 addition & 2 deletions src/AutoMapper/Configuration/MappingExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -152,12 +152,11 @@ public MappingExpression(MemberList memberList, Type sourceType, Type destinatio

public MappingExpression(MemberList memberList, TypePair types)
{
MemberList = memberList;
Types = types;
IsOpenGeneric = types.SourceType.IsGenericTypeDefinition() || types.DestinationType.IsGenericTypeDefinition();
TypeMapActions.Add(tm => tm.ConfiguredMemberList = memberList);
}

public MemberList MemberList { get; }
public TypePair Types { get; }
public Type SourceType => Types.SourceType;
public Type DestinationType => Types.DestinationType;
Expand Down
17 changes: 17 additions & 0 deletions src/AutoMapper/IConfigurationProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,23 @@ public interface IConfigurationProvider
/// <returns>Type map configuration</returns>
TypeMap ResolveTypeMap(Type sourceType, Type destinationType);

/// <summary>
/// Resolve the <see cref="TypeMap"/> for the configured source and destination type, checking parent types
/// </summary>
/// <param name="sourceType">Configured source type</param>
/// <param name="destinationType">Configured destination type</param>
/// <param name="inlineConfiguration">Inline type map configuration if exists</param>
/// <returns>Type map configuration</returns>
TypeMap ResolveTypeMap(Type sourceType, Type destinationType, ITypeMapConfiguration inlineConfiguration);

/// <summary>
/// Resolve the <see cref="TypeMap"/> for the configured type pair, checking parent types
/// </summary>
/// <param name="typePair">Type pair</param>
/// <param name="inlineConfiguration">Inline type map configuration if exists</param>
/// <returns>Type map configuration</returns>
TypeMap ResolveTypeMap(TypePair typePair, ITypeMapConfiguration inlineConfiguration);

/// <summary>
/// Resolve the <see cref="TypeMap"/> for the configured type pair, checking parent types
/// </summary>
Expand Down
1 change: 1 addition & 0 deletions src/AutoMapper/LockingConcurrentDictionary.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ public LockingConcurrentDictionary(Func<TKey, TValue> valueFactory)
}

public TValue GetOrAdd(TKey key) => _dictionary.GetOrAdd(key, _valueFactory).Value;
public TValue GetOrAdd(TKey key, Func<TKey, Lazy<TValue>> valueFactory) => _dictionary.GetOrAdd(key, valueFactory).Value;

public TValue this[TKey key]
{
Expand Down
43 changes: 30 additions & 13 deletions src/AutoMapper/MapperConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ namespace AutoMapper

public class MapperConfiguration : IConfigurationProvider
{
private static readonly Type[] ExcludedTypes = { typeof(object), typeof(ValueType), typeof(Enum) };
private static readonly ConstructorInfo ExceptionConstructor = typeof(AutoMapperMappingException).GetDeclaredConstructors().Single(c => c.GetParameters().Length == 3);

private readonly IEnumerable<IObjectMapper> _mappers;
Expand Down Expand Up @@ -110,19 +111,12 @@ public LambdaExpression BuildExecutionPlan(Type sourceType, Type destinationType

public LambdaExpression BuildExecutionPlan(MapRequest mapRequest)
{
var typeMap = ResolveTypeMap(mapRequest.RuntimeTypes) ?? ResolveTypeMap(mapRequest.RequestedTypes);
var typeMap = ResolveTypeMap(mapRequest.RuntimeTypes, mapRequest.InlineConfig) ?? ResolveTypeMap(mapRequest.RequestedTypes, mapRequest.InlineConfig);
if (typeMap != null)
{
return GenerateTypeMapExpression(mapRequest, typeMap);
}
var mapperToUse = FindMapper(mapRequest.RuntimeTypes);
if (Configuration.CreateMissingTypeMaps && mapperToUse == null)
{
typeMap = Configuration.CreateInlineMap(_typeMapRegistry, mapRequest.InlineConfig ?? new DefaultTypeMapConfig(mapRequest.RuntimeTypes));
_typeMapPlanCache[mapRequest.RuntimeTypes] = typeMap;
typeMap.Seal(this);
return GenerateTypeMapExpression(mapRequest, typeMap);
}
return GenerateObjectMapperExpression(mapRequest, mapperToUse, this);
}

Expand Down Expand Up @@ -193,17 +187,28 @@ public TypeMap ResolveTypeMap(Type sourceType, Type destinationType)
{
var typePair = new TypePair(sourceType, destinationType);

return ResolveTypeMap(typePair);
return ResolveTypeMap(typePair, new DefaultTypeMapConfig(typePair));
}

public TypeMap ResolveTypeMap(Type sourceType, Type destinationType, ITypeMapConfiguration inlineConfiguration)
{
var typePair = new TypePair(sourceType, destinationType);

return ResolveTypeMap(typePair, inlineConfiguration);
}

public TypeMap ResolveTypeMap(TypePair typePair)
=> ResolveTypeMap(typePair, new DefaultTypeMapConfig(typePair));

public TypeMap ResolveTypeMap(TypePair typePair, ITypeMapConfiguration inlineConfiguration)
{
var typeMap = _typeMapPlanCache.GetOrAdd(typePair);
// if it's a dynamically created type map, we need to seal it outside GetTypeMap to handle recursion
if (typeMap != null && typeMap.MapExpression == null && _typeMapRegistry.GetTypeMap(typePair) == null)
{
lock (typeMap)
{
{
inlineConfiguration.Configure(typeMap);
typeMap.Seal(this);
}
}
Expand All @@ -217,7 +222,10 @@ private TypeMap GetTypeMap(TypePair initialTypes)
{
if (types != initialTypes && _typeMapPlanCache.TryGetValue(types, out var typeMap))
{
return typeMap;
if (typeMap != null)
{
return typeMap;
}
}
typeMap = FindTypeMapFor(types);
if (typeMap != null)
Expand All @@ -239,6 +247,16 @@ private TypeMap GetTypeMap(TypePair initialTypes)
}
}

if (!hasMapper.Value
&& Configuration.CreateMissingTypeMaps
&& !(initialTypes.SourceType.IsAbstract() && initialTypes.SourceType.IsClass())
&& !(initialTypes.DestinationType.IsAbstract() && initialTypes.DestinationType.IsClass())
&& !ExcludedTypes.Contains(initialTypes.SourceType)
&& !ExcludedTypes.Contains(initialTypes.DestinationType))
{
return Configuration.CreateInlineMap(_typeMapRegistry, initialTypes);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would lock(this) here, so all CreateMap calls are serialized, closed generics, conventions and inline.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. I got rid of the cache, it's just not obvious.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah but that's the value factory for the locking concurrent dictionary. The lock is already there.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I know there's a lock for the type pair, I'm just not sure that's enough. I'll have to think about it some more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's enough. A GetType map for some other types might call CreateMap for these initialTypes. And then you have concurrent CreateMap calls again.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another thing, I don't think we need a Lazy here. A regular bool? should do. It's a local, only one thread will access it and the mappers collection is immutable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah you're right, I missed the other locks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I use the hasMapper in two places though? That would look a little odd, I might need a func or something.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have a problem with

 var hasMapper = (FindMapper(initialTypes) != null);

Or simply calling it twice as before.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

}

return null;
}

Expand Down Expand Up @@ -405,7 +423,7 @@ private static Expression<UntypedMapperFunc> Wrap(MapRequest mapRequest, Delegat
}
}

private class DefaultTypeMapConfig : ITypeMapConfiguration
internal class DefaultTypeMapConfig : ITypeMapConfiguration
{
public DefaultTypeMapConfig(TypePair types)
{
Expand All @@ -414,7 +432,6 @@ public DefaultTypeMapConfig(TypePair types)

public void Configure(TypeMap typeMap) { }

public MemberList MemberList => MemberList.Destination;
public Type SourceType => Types.SourceType;
public Type DestinationType => Types.DestinationType;
public bool IsOpenGeneric => false;
Expand Down
13 changes: 5 additions & 8 deletions src/AutoMapper/ProfileMap.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ struct IConvertible{}
[DebuggerDisplay("{Name}")]
public class ProfileMap
{
private static readonly Type[] ExcludedTypes = { typeof(object), typeof(ValueType), typeof(Enum), typeof(IComparable), typeof(IFormattable), typeof(IConvertible) };
private readonly TypeMapFactory _typeMapFactory = new TypeMapFactory();
private readonly IEnumerable<ITypeMapConfiguration> _typeMapConfigs;
private readonly IEnumerable<ITypeMapConfiguration> _openTypeMapConfigs;
Expand Down Expand Up @@ -129,7 +128,7 @@ public void Configure(TypeMapRegistry typeMapRegistry)

private void BuildTypeMap(TypeMapRegistry typeMapRegistry, ITypeMapConfiguration config)
{
var typeMap = _typeMapFactory.CreateTypeMap(config.SourceType, config.DestinationType, this, config.MemberList);
var typeMap = _typeMapFactory.CreateTypeMap(config.SourceType, config.DestinationType, this);

config.Configure(typeMap);

Expand Down Expand Up @@ -176,7 +175,7 @@ public bool IsConventionMap(TypePair types)

public TypeMap CreateConventionTypeMap(TypeMapRegistry typeMapRegistry, TypePair types)
{
var typeMap = _typeMapFactory.CreateTypeMap(types.SourceType, types.DestinationType, this, MemberList.Destination);
var typeMap = _typeMapFactory.CreateTypeMap(types.SourceType, types.DestinationType, this);

typeMap.IsConventionMap = true;

Expand All @@ -189,22 +188,20 @@ public TypeMap CreateConventionTypeMap(TypeMapRegistry typeMapRegistry, TypePair
return typeMap;
}

public TypeMap CreateInlineMap(TypeMapRegistry typeMapRegistry, ITypeMapConfiguration inlineConfig)
public TypeMap CreateInlineMap(TypeMapRegistry typeMapRegistry, TypePair types)
{
var typeMap = _typeMapFactory.CreateTypeMap(inlineConfig.SourceType, inlineConfig.DestinationType, this, inlineConfig.MemberList);
var typeMap = _typeMapFactory.CreateTypeMap(types.SourceType, types.DestinationType, this);

typeMap.IsConventionMap = true;

inlineConfig.Configure(typeMap);

Configure(typeMapRegistry, typeMap);

return typeMap;
}

public TypeMap CreateClosedGenericTypeMap(ITypeMapConfiguration openMapConfig, TypeMapRegistry typeMapRegistry, TypePair closedTypes)
{
var closedMap = _typeMapFactory.CreateTypeMap(closedTypes.SourceType, closedTypes.DestinationType, this, openMapConfig.MemberList);
var closedMap = _typeMapFactory.CreateTypeMap(closedTypes.SourceType, closedTypes.DestinationType, this);

openMapConfig.Configure(closedMap);

Expand Down
2 changes: 1 addition & 1 deletion src/AutoMapper/QueryableExtensions/ExpressionBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,7 @@ public override QueryExpressions GetSubQueryExpression(ExpressionBuilder builder
TypeMap firstTypeMap;
lock(_configurationProvider)
{
firstTypeMap = typeMapFactory.CreateTypeMap(request.SourceType, letType, typeMap.Profile, MemberList.None);
firstTypeMap = typeMapFactory.CreateTypeMap(request.SourceType, letType, typeMap.Profile);
}
var secondParameter = Parameter(letType, "dto");

Expand Down
5 changes: 2 additions & 3 deletions src/AutoMapper/TypeMap.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,12 @@ public class TypeMap
private readonly IList<TypeMap> _inheritedTypeMaps = new List<TypeMap>();
private readonly List<ValueTransformerConfiguration> _valueTransformerConfigs = new List<ValueTransformerConfiguration>();

public TypeMap(TypeDetails sourceType, TypeDetails destinationType, MemberList memberList, ProfileMap profile)
public TypeMap(TypeDetails sourceType, TypeDetails destinationType, ProfileMap profile)
{
SourceTypeDetails = sourceType;
DestinationTypeDetails = destinationType;
Types = new TypePair(sourceType.Type, destinationType.Type);
Profile = profile;
ConfiguredMemberList = memberList;
}

public PathMap FindOrCreatePathMapFor(LambdaExpression destinationExpression, MemberPath path, TypeMap typeMap)
Expand Down Expand Up @@ -77,7 +76,7 @@ public PathMap FindPathMapByDestinationPath(string destinationFullPath) =>

public bool ConstructDestinationUsingServiceLocator { get; set; }

public MemberList ConfiguredMemberList { get; }
public MemberList ConfiguredMemberList { get; set; }

public IEnumerable<TypePair> IncludedDerivedTypes => _includedDerivedTypes;
public IEnumerable<TypePair> IncludedBaseTypes => _includedBaseTypes;
Expand Down
4 changes: 2 additions & 2 deletions src/AutoMapper/TypeMapFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ namespace AutoMapper
{
public class TypeMapFactory
{
public TypeMap CreateTypeMap(Type sourceType, Type destinationType, ProfileMap options, MemberList memberList)
public TypeMap CreateTypeMap(Type sourceType, Type destinationType, ProfileMap options)
{
var sourceTypeInfo = options.CreateTypeDetails(sourceType);
var destTypeInfo = options.CreateTypeDetails(destinationType);

var typeMap = new TypeMap(sourceTypeInfo, destTypeInfo, memberList, options);
var typeMap = new TypeMap(sourceTypeInfo, destTypeInfo, options);

foreach (var destProperty in destTypeInfo.PublicWriteAccessors)
{
Expand Down
2 changes: 1 addition & 1 deletion src/AutoMapper/TypePair.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public struct MapRequest : IEquatable<MapRequest>
public ITypeMapConfiguration InlineConfig { get; }

public MapRequest(TypePair requestedTypes, TypePair runtimeTypes)
: this(requestedTypes, runtimeTypes, null)
: this(requestedTypes, runtimeTypes, new MapperConfiguration.DefaultTypeMapConfig(requestedTypes))
{
}

Expand Down
7 changes: 4 additions & 3 deletions src/UnitTests/Bug/BaseMapWithIncludesAndUnincludedMappings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,12 @@ public class Container2
{
public BaseB Item { get; set; }
}
public class BaseA
public abstract class BaseA
{
public string Name { get; set; }
}

public class BaseB
public abstract class BaseB
{
public string Name { get; set; }
}
Expand Down Expand Up @@ -84,7 +84,8 @@ public void TestInitialiserProxyOfSub()
cfg.CreateMap<Container, Container2>();
});

var mapped = config.CreateMapper().Map<Container, Container2>(new Container() { Item = new ProxyOfSubA() { Name = "Martin", Description = "Hello" } });
var mapped = config.CreateMapper()
.Map<Container, Container2>(new Container() { Item = new ProxyOfSubA() { Name = "Martin", Description = "Hello" } });
Assert.IsType<SubB>(mapped.Item);

}
Expand Down
1 change: 1 addition & 0 deletions src/UnitTests/Bug/ExpressionMapping.cs
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ public Parent Parent

protected override MapperConfiguration Configuration { get; } = new MapperConfiguration(cfg =>
{
cfg.CreateMissingTypeMaps = false;
cfg.CreateMap<GrandParent, GrandParentDTO>().ReverseMap();
cfg.CreateMap<Parent, ParentDTO>().ReverseMap();
cfg.CreateMap<Child, ChildDTO>()
Expand Down
27 changes: 27 additions & 0 deletions src/UnitTests/DynamicMapping.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using Xunit;
using Shouldly;
using System.Collections.Generic;
using AutoMapper.QueryableExtensions;

namespace AutoMapper.UnitTests.DynamicMapping
{
Expand Down Expand Up @@ -396,6 +397,32 @@ public void Should_map()
}
}

public class When_automatically_dynamically_projecting : NonValidatingSpecBase
{
public class Source
{
public int Value { get; set; }
}

public class Dest
{
public int Value { get; set; }
}

protected override MapperConfiguration Configuration { get; } = new MapperConfiguration(cfg => {});

[Fact]
public void Should_map()
{
var source = new Source {Value = 5};
var items = new[] {source}.AsQueryable();
var dest = items.ProjectTo<Dest>(ConfigProvider).ToArray();

dest.Length.ShouldBe(1);
dest[0].Value.ShouldBe(5);
}
}

public class When_mixing_auto_and_manual_map : NonValidatingSpecBase
{
public class Source
Expand Down
1 change: 1 addition & 0 deletions src/UnitTests/Projection/ProjectTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ public ProjectTest()
{
cfg.CreateMap<Address, AddressDto>();
cfg.CreateMap<Customer, CustomerDto>();
cfg.CreateMissingTypeMaps = false;
});
}

Expand Down
1 change: 1 addition & 0 deletions src/UnitTests/Query/SourceInjectedQuery.cs
Original file line number Diff line number Diff line change
Expand Up @@ -628,6 +628,7 @@ private static IMapper SetupAutoMapper()
{
var config = new MapperConfiguration(cfg =>
{
cfg.CreateMissingTypeMaps = false;
cfg.CreateMap<User, UserModel>()
.ForMember(d => d.Id, opt => opt.MapFrom(s => s.UserId))
.ForMember(d => d.FullName, opt => opt.MapFrom(s => s.Name))
Expand Down
Loading