Feature/186 internal types (#187)

* Fixed performance issue where DefaultPropertyResolver threw a first chance exception when attempting to resolve the default value of System.String. * Updated DefaultTypeResolver to allow automatic resolution of internal types +semver: feature
roryprimrose · May 7, 2021 · 812c1ff · 812c1ff
1 parent 57feac1
commit 812c1ff
Show file tree

Hide file tree

Showing 10 changed files with 165 additions and 17 deletions.
diff --git a/ModelBuilder.UnitTests/DefaultTypeResolverTests.cs b/ModelBuilder.UnitTests/DefaultTypeResolverTests.cs
@@ -94,6 +94,20 @@ public void GetBuildTypeReturnsDerivedTypeMatchingOnAbstractBaseName()
             actual.Should().Be<SomeClass>();
         }
 
+        [Fact]
+        public void GetBuildTypeReturnsInternalType()
+        {
+            var configuration = new BuildConfiguration();
+
+            var sut = new DefaultTypeResolver();
+
+            var actual = sut.GetBuildType(configuration, typeof(IInternalItem));
+
+            _output.WriteLine(actual.FullName);
+
+            actual.Should().Be<InternalItem>();
+        }
+
         [Fact]
         public void GetBuildTypeReturnsOriginalTypeWhenNoBetterTypeFound()
         {
@@ -148,6 +162,20 @@ public void GetBuildTypeReturnsPossibleTypeFromInterface()
             actual.Should().Implement<ITestInterface>();
         }
 
+        [Fact]
+        public void GetBuildTypeReturnsPublicTypeOverInternalType()
+        {
+            var configuration = new BuildConfiguration();
+
+            var sut = new Wrapper(new[] {typeof(InternalNotPickedItem), typeof(PublicPickedItem)});
+
+            var actual = sut.GetBuildType(configuration, typeof(IPublicOverInternal));
+
+            _output.WriteLine(actual.FullName);
+
+            actual.Should().Be<PublicPickedItem>();
+        }
+
         [Fact]
         public void GetBuildTypeReturnsSourceWhenNoTypeMappingIsFound()
         {
@@ -254,6 +282,21 @@ public void GetBuildTypeThrowsExceptionWithNullSourceType()
 
             action.Should().Throw<ArgumentNullException>();
         }
+
+        private class Wrapper : DefaultTypeResolver
+        {
+            private readonly IEnumerable<Type> _types;
+
+            public Wrapper(IEnumerable<Type> types)
+            {
+                _types = types;
+            }
+
+            protected override IEnumerable<Type> GetTypesToEvaluate(Type requestedType)
+            {
+                return _types;
+            }
+        }
     }
 
     [SuppressMessage("Design", "CA1040:Avoid empty interfaces", Justification = "The interface is used for testing")]

diff --git a/ModelBuilder.UnitTests/Models/IInternalItem.cs b/ModelBuilder.UnitTests/Models/IInternalItem.cs
@@ -0,0 +1,11 @@
+namespace ModelBuilder.UnitTests.Models
+{
+    using System;
+
+    public interface IInternalItem
+    {
+        public string First { get; }
+        public bool Second { get; }
+        public Uri Third { get; }
+    }
+}
diff --git a/ModelBuilder.UnitTests/Models/IPublicOverInternal.cs b/ModelBuilder.UnitTests/Models/IPublicOverInternal.cs
@@ -0,0 +1,11 @@
+namespace ModelBuilder.UnitTests.Models
+{
+    using System;
+
+    public interface IPublicOverInternal
+    {
+        public string First { get; }
+        public bool Second { get; }
+        public Uri Third { get; }
+    }
+}
diff --git a/ModelBuilder.UnitTests/Models/InternalItem.cs b/ModelBuilder.UnitTests/Models/InternalItem.cs
@@ -0,0 +1,11 @@
+namespace ModelBuilder.UnitTests.Models
+{
+    using System;
+
+    internal class InternalItem : IInternalItem
+    {
+        public string First { get; set; } = string.Empty;
+        public bool Second { get; set; }
+        public Uri Third { get; set; } = new Uri("about:blank");
+    }
+}
diff --git a/ModelBuilder.UnitTests/Models/InternalNotPickedItem.cs b/ModelBuilder.UnitTests/Models/InternalNotPickedItem.cs
@@ -0,0 +1,11 @@
+namespace ModelBuilder.UnitTests.Models
+{
+    using System;
+
+    internal class InternalNotPickedItem : IPublicOverInternal
+    {
+        public string First { get; set; } = string.Empty;
+        public bool Second { get; set; }
+        public Uri Third { get; set; } = new Uri("about:blank");
+    }
+}
diff --git a/ModelBuilder.UnitTests/Models/PublicPickedItem.cs b/ModelBuilder.UnitTests/Models/PublicPickedItem.cs
@@ -0,0 +1,11 @@
+namespace ModelBuilder.UnitTests.Models
+{
+    using System;
+
+    public class PublicPickedItem : IPublicOverInternal
+    {
+        public string First { get; set; } = string.Empty;
+        public bool Second { get; set; }
+        public Uri Third { get; set; } = new Uri("about:blank");
+    }
+}
diff --git a/ModelBuilder.UnitTests/Scenarios/ScenarioTests.cs b/ModelBuilder.UnitTests/Scenarios/ScenarioTests.cs
@@ -108,6 +108,15 @@ public void CanCreateInstanceWithParameters()
             actual.Value.Should().NotBeEmpty();
         }
 
+        [Fact]
+        public void CanCreateInternalTypeFromPublicInterface()
+        {
+            var actual = Model.Create<IInternalItem>();
+
+            actual.Should().NotBeNull();
+            actual.Should().BeOfType<InternalItem>();
+        }
+
         [Fact]
         public void CanCreateWithMultipleNames()
         {
@@ -322,6 +331,15 @@ public void CreatePopulatesReadOnlyReferenceTypeProperties()
             actual.Unassigned.Should().BeNull();
         }
 
+        [Fact]
+        public void CreatePublicTypeInsteadOfInternalType()
+        {
+            var actual = Model.Create<IPublicOverInternal>();
+
+            actual.Should().NotBeNull();
+            actual.Should().BeOfType<PublicPickedItem>();
+        }
+
         [Fact]
         public void CreateReturnsGuid()
         {

diff --git a/ModelBuilder.sln.DotSettings b/ModelBuilder.sln.DotSettings
@@ -427,6 +427,7 @@
 	<s:String x:Key="/Default/CodeStyle/Naming/JavaScriptNaming/UserRules/=TS_005FPUBLIC_005FTYPE_005FFIELD/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="" Suffix="" Style="aaBb" /&gt;</s:String>
 	<s:String x:Key="/Default/CodeStyle/Naming/JavaScriptNaming/UserRules/=TS_005FPUBLIC_005FTYPE_005FMETHOD/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="" Suffix="" Style="aaBb" /&gt;</s:String>
 	<s:String x:Key="/Default/CodeStyle/Naming/JavaScriptNaming/UserRules/=TS_005FTYPE_005FPARAMETER/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="T" Suffix="" Style="AaBb" /&gt;</s:String>
+	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EdotCover_002EIde_002ECore_002EFilterManagement_002EModel_002ESolutionFilterSettingsManagerMigrateSettings/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ECSharpAttributeForSingleLineMethodUpgrade/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ECSharpKeepExistingMigration/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ECSharpPlaceEmbeddedOnSameLineMigration/@EntryIndexedValue">True</s:Boolean>

diff --git a/ModelBuilder/DefaultPropertyResolver.cs b/ModelBuilder/DefaultPropertyResolver.cs
@@ -14,8 +14,8 @@ namespace ModelBuilder
     /// </summary>
     public class DefaultPropertyResolver : IPropertyResolver
     {
-        private static readonly ConcurrentDictionary<Type, object> _defaultValues =
-            new ConcurrentDictionary<Type, object>();
+        private static readonly ConcurrentDictionary<Type, object?> _defaultValues =
+            new ConcurrentDictionary<Type, object?>();
 
         private static readonly ConcurrentDictionary<Type, IList<PropertyInfo>> _globalCache =
             new ConcurrentDictionary<Type, IList<PropertyInfo>>();
@@ -214,9 +214,9 @@ private static IEnumerable<PropertyInfo> CalculateOrderedProperties(IBuildConfig
             Type targetType)
         {
             return from x in targetType.GetProperties(BindingFlags.Instance | BindingFlags.Public)
-                where CanPopulate(x)
-                orderby GetMaximumOrderPriority(configuration, x) descending
-                select x;
+                   where CanPopulate(x)
+                   orderby GetMaximumOrderPriority(configuration, x) descending
+                   select x;
         }
 
         /// <summary>
@@ -262,7 +262,20 @@ private static bool CanPopulate(PropertyInfo propertyInfo)
         {
             try
             {
-                return _defaultValues.GetOrAdd(type, x => Activator.CreateInstance(x)!);
+                // Special case: System.String
+                // Attempting to use Activator.CreateInstance on string throws an exception which is poor for performance
+                if (type == typeof(string))
+                {
+                    return _defaultValues.GetOrAdd(type, x => null);
+                }
+
+                if (type.IsValueType == false)
+                {
+                    // This is a reference type which should also default to null
+                    return _defaultValues.GetOrAdd(type, x => null);
+                }
+
+                return _defaultValues.GetOrAdd(type, Activator.CreateInstance);
             }
             catch
             {
@@ -278,9 +291,9 @@ private static int GetMaximumOrderPriority(IBuildConfiguration configuration, Pr
             }
 
             var matchingRules = from x in configuration.ExecuteOrderRules
-                where x.IsMatch(property)
-                orderby x.Priority descending
-                select x;
+                                where x.IsMatch(property)
+                                orderby x.Priority descending
+                                select x;
 
             var matchingRule = matchingRules.FirstOrDefault();
 

diff --git a/ModelBuilder/DefaultTypeResolver.cs b/ModelBuilder/DefaultTypeResolver.cs
@@ -18,11 +18,9 @@ public class DefaultTypeResolver : ITypeResolver
         public Type GetBuildType(IBuildConfiguration configuration, Type requestedType)
         {
             configuration = configuration ?? throw new ArgumentNullException(nameof(configuration));
-
             requestedType = requestedType ?? throw new ArgumentNullException(nameof(requestedType));
 
-            var typeMappingRule = configuration.TypeMappingRules?.Where(x => x.SourceType == requestedType)
-                .FirstOrDefault();
+            var typeMappingRule = configuration.TypeMappingRules?.FirstOrDefault(x => x.SourceType == requestedType);
 
             if (typeMappingRule != null)
             {
@@ -48,7 +46,7 @@ public Type GetBuildType(IBuildConfiguration configuration, Type requestedType)
             return requestedType;
         }
 
-        private static Type AutoResolveBuildType(Type requestedType)
+        private Type AutoResolveBuildType(Type requestedType)
         {
             // Before search for matching types and finding a good match, check if this is an IEnumerable that would support a list type
             if (requestedType.IsInterface
@@ -90,10 +88,7 @@ private static Type AutoResolveBuildType(Type requestedType)
             }
 
             // Automatically resolve a derived type within the same assembly
-            var assemblyTypes = requestedType.GetTypeInfo().Assembly.GetTypes();
-            var possibleTypes = from x in assemblyTypes
-                where x.IsPublic && x.IsInterface == false && x.IsAbstract == false
-                select x;
+            var possibleTypes = GetTypesToEvaluate(requestedType);
 
             var matchingTypes = new List<Type>();
 
@@ -176,6 +171,15 @@ private static Type AutoResolveBuildType(Type requestedType)
                 }
             }
 
+            // First match to a public type
+            var publicType = matchingTypes.FirstOrDefault(x => x.IsPublic);
+
+            if (publicType != null)
+            {
+                return publicType;
+            }
+
+            // Match to any other type
             var matchingType = matchingTypes.FirstOrDefault();
 
             if (matchingType == null)
@@ -185,5 +189,19 @@ private static Type AutoResolveBuildType(Type requestedType)
 
             return matchingType;
         }
+
+        /// <summary>
+        /// Gets the types to evaluate for compatibility with the specified type.
+        /// </summary>
+        /// <param name="requestedType">The requested type.</param>
+        /// <returns>The set of types to evaluate.</returns>
+        protected virtual IEnumerable<Type> GetTypesToEvaluate(Type requestedType)
+        {
+            var assemblyTypes = requestedType.GetTypeInfo().Assembly.GetTypes();
+
+            return from x in assemblyTypes
+                where x.IsInterface == false && x.IsAbstract == false
+                select x;
+        }
     }
 }