Merge pull request #1848 from lbargaoanu/RegistryMapper

Avoid static state; Mapper registry setup moved to mapper configuration instead of its own list

Author: jbogard

src/AutoMapper/Configuration/MapperConfigurationExpression.cs

@@ -13,6 +13,8 @@ public class MapperConfigurationExpression : Profile, IMapperConfigurationExpres
         public MapperConfigurationExpression() : base("")
         {
             IncludeSourceExtensionMethods(typeof(Enumerable));
+
+            Mappers = MapperRegistry.Mappers();
         }
 
         public IEnumerable<IProfileConfiguration> Profiles => _profiles;
@@ -21,6 +23,8 @@ public MapperConfigurationExpression() : base("")
         public void CreateProfile(string profileName, Action<IProfileExpression> config) 
             => AddProfile(new NamedProfile(profileName, config));
 
+        public IList<IObjectMapper> Mappers { get; }
+
         private class NamedProfile : Profile
         {
             public NamedProfile(string profileName, Action<IProfileExpression> config) : base(profileName, config)

src/AutoMapper/IMapperConfigurationExpression.cs

@@ -77,5 +77,10 @@ public interface IMapperConfigurationExpression : IProfileExpression
         /// <param name="profileName">Profile name, must be unique</param>
         /// <param name="config">Profile configuration</param>
         void CreateProfile(string profileName, Action<IProfileExpression> config);
+
+        /// <summary>
+        /// Object mappers
+        /// </summary>
+        IList<IObjectMapper> Mappers { get; }
     }
 }

src/AutoMapper/MapperConfiguration.cs

@@ -23,13 +23,8 @@ public class MapperConfiguration : IConfigurationProvider
         private readonly ConfigurationValidator _validator;
 
         public MapperConfiguration(MapperConfigurationExpression configurationExpression)
-            : this(configurationExpression, MapperRegistry.Mappers)
         {
-        }
-
-        public MapperConfiguration(MapperConfigurationExpression configurationExpression, IEnumerable<IObjectMapper> mappers)
-        {
-            _mappers = mappers;
+            _mappers = configurationExpression.Mappers.ToArray();
             _mapPlanCache = new LockingConcurrentDictionary<MapRequest, MapperFuncs>(CreateMapperFuncs);
             _validator = new ConfigurationValidator(this);
             ExpressionBuilder = new ExpressionBuilder(this);
@@ -43,12 +38,8 @@ public MapperConfiguration(MapperConfigurationExpression configurationExpression
             Seal();
         }
 
-        public MapperConfiguration(Action<IMapperConfigurationExpression> configure) : this(configure, MapperRegistry.Mappers)
-        {
-        }
-
-        public MapperConfiguration(Action<IMapperConfigurationExpression> configure, IEnumerable<IObjectMapper> mappers)
-            : this(Build(configure), mappers)
+        public MapperConfiguration(Action<IMapperConfigurationExpression> configure)
+            : this(Build(configure))
         {
         }
 

src/AutoMapper/Mappers/MapperRegistry.cs

@@ -2,9 +2,9 @@
 
 namespace AutoMapper.Mappers
 {
-    public static class MapperRegistry
+    internal static class MapperRegistry
     {
-        private static readonly IObjectMapper[] _initialMappers =
+        public static IList<IObjectMapper> Mappers() => new List<IObjectMapper>
         {
             new NullableSourceMapper(),
             new ExpressionMapper(), 
@@ -34,21 +34,5 @@ public static class MapperRegistry
             new FromDynamicMapper(),
             new ToDynamicMapper()
         };
-
-        private static readonly List<IObjectMapper> _mappers = new List<IObjectMapper>(_initialMappers);
-
-        /// <summary>
-        /// Extension point for modifying list of object mappers
-        /// </summary>
-        public static IList<IObjectMapper> Mappers => _mappers;
-
-        /// <summary>
-        /// Reset mapper registry to built-in values
-        /// </summary>
-        public static void Reset()
-        {
-            _mappers.Clear();
-            _mappers.AddRange(_initialMappers);
-        }
     }
 }

src/UnitTests/Bug/ConvertMapperThreading.cs

@@ -0,0 +1,47 @@
+using System;
+using Xunit;
+using AutoMapper;
+using System.Linq;
+using System.Threading.Tasks;
+using System.Diagnostics;
+using System.Runtime.ExceptionServices;
+
+namespace AutoMapper.UnitTests
+{
+    public class ConvertMapperThreading
+    {
+        class Source
+        {
+            public string Number { get; set; }
+        }
+
+        class Destination
+        {
+            public int Number { get; set; }
+        }
+
+        [Fact]
+        public void Should_work()
+        {
+            var tasks = Enumerable.Range(0, 5).Select(i => Task.Factory.StartNew(() =>
+            {
+                new MapperConfiguration(c => c.CreateMap<Source, Destination>());
+            })).ToArray();
+            try
+            {
+                Task.WaitAll(tasks);
+            }
+            catch(AggregateException ex)
+            {
+                ex.Handle(e =>
+                {
+                    if(e is InvalidOperationException)
+                    {
+                        ExceptionDispatchInfo.Capture(e).Throw();
+                    }
+                    return false;
+                });
+            }
+        }
+    }
+}

src/UnitTests/Mappers/CustomMapperTests.cs

@@ -10,15 +10,11 @@ namespace AutoMapper.UnitTests.Mappers
 {
     public class When_adding_a_custom_mapper : NonValidatingSpecBase
     {
-        public When_adding_a_custom_mapper()
-        {
-            MapperRegistry.Mappers.Insert(0, new TestObjectMapper());
-        }
-
         protected override MapperConfiguration Configuration { get; } = new MapperConfiguration(cfg =>
         {
             cfg.CreateMap<ClassA, ClassB>()
                 .ForMember(dest => dest.Destination, opt => opt.MapFrom(src => src.Source));
+            cfg.Mappers.Add(new TestObjectMapper());
         });
 
         [Fact]
@@ -78,7 +74,8 @@ public class When_adding_a_simple_custom_mapper : AutoMapperSpecBase
         {
             cfg.CreateMap<ClassA, ClassB>()
                 .ForMember(dest => dest.Destination, opt => opt.MapFrom(src => src.Source));
-        }, MapperRegistry.Mappers.Concat(new[] { new TestObjectMapper() }));
+            cfg.Mappers.Add(new TestObjectMapper());
+        });
 
         protected override void Because_of()
         {
@@ -162,7 +159,8 @@ public override string Map(object source, string destination, ResolutionContext
         protected override MapperConfiguration Configuration => new MapperConfiguration(cfg =>
         {
             cfg.CreateMap<Source, Destination>();
-        }, new[] { new EnumMapper() }.Concat(MapperRegistry.Mappers));
+            cfg.Mappers.Insert(0, new EnumMapper());
+        });
 
         protected override void Because_of()
         {

src/UnitTests/UnitTests.Net4.csproj

@@ -93,6 +93,7 @@
     <Compile Include="AssertionExtensions.cs" />
     <Compile Include="BasicFlattening.cs" />
     <Compile Include="Bug\ConventionCreateMapsWithCircularReference.cs" />
+    <Compile Include="Bug\ConvertMapperThreading.cs" />
     <Compile Include="Bug\DeepCloningBug.cs" />
     <Compile Include="Bug\ForAllMembersAndDoNotUseDestinationValue.cs" />
     <Compile Include="Bug\InterfaceMultipleInheritance.cs" />