Avoid some cases of Type -> string -> Type conversion in Mapping By Code (#2003)

Co-authored-by: Alexander Zaytsev <hazzik@gmail.com>

Author: 2 people

src/NHibernate.Test/TypesTest/TimestampTypeFixture.cs

@@ -24,10 +24,7 @@ public void ObsoleteMessage()
 					var log = spy.GetWholeLog();
 					Assert.That(
 						log,
-						Does.Contain("NHibernate.Type.TimestampType is obsolete. Please use DateTimeType instead.").IgnoreCase);
-					Assert.That(
-						log,
-						Does.Not.Contain($"{NHibernateUtil.Timestamp.Name} is obsolete. Please use DateTimeType instead.").IgnoreCase);
+						Does.Contain($"NHibernate.Type.TimestampType ({NHibernateUtil.Timestamp.Name}) is obsolete. Please use DateTimeType instead.").IgnoreCase);
 				}
 			}
 		}

src/NHibernate.Test/TypesTest/TypeFactoryFixture.cs

@@ -56,6 +56,7 @@ public void GetNullableGeneric()
 			//Assert.AreEqual(int64Type, TypeFactory.HeuristicType("Int64?"), "'Int64?' should return a NH Int64Type");
 
 			System.Type reflectedType = Util.ReflectHelper.ReflectedPropertyClass( typeof(GenericPropertyClass), "GenericInt64", "property" );
+			Assert.AreEqual( int64Type, TypeFactory.HeuristicType( reflectedType ), "using System.Type should return nh Int64Type" );
 			Assert.AreEqual( int64Type, TypeFactory.HeuristicType( reflectedType.AssemblyQualifiedName ), "using AQN should return nh Int64Type" );
 			Assert.AreEqual( int64Type, TypeFactory.HeuristicType( reflectedType.FullName ), "using FullName should return nh Int64Type" );
 
@@ -143,5 +144,19 @@ public void WhenUseNullableEnumThenReturnGenericEnumType()
 			var iType = TypeFactory.HeuristicType(typeof(MyEnum?).AssemblyQualifiedName);
 			Assert.That(iType, Is.TypeOf<EnumType<MyEnum>>());
 		}
+		
+		[Test]
+		public void WhenUseEnumTypeThenReturnGenericEnumType()
+		{
+			var iType = TypeFactory.HeuristicType(typeof (MyEnum));
+			Assert.That(iType, Is.TypeOf<EnumType<MyEnum>>());
+		}
+
+		[Test]
+		public void WhenUseNullableEnumTypeThenReturnGenericEnumType()
+		{
+			var iType = TypeFactory.HeuristicType(typeof(MyEnum?));
+			Assert.That(iType, Is.TypeOf<EnumType<MyEnum>>());
+		}
 	}
 }

src/NHibernate/Hql/Ast/ANTLR/Tree/JavaConstantNode.cs

@@ -70,7 +70,7 @@ private void ProcessText()
                 }
 
                 _constantValue = ReflectHelper.GetConstantValue(base.Text, _factory);
-                _heuristicType = TypeFactory.HeuristicType(_constantValue.GetType().AssemblyQualifiedName);
+                _heuristicType = TypeFactory.HeuristicType(_constantValue.GetType());
                 _processedText = true;
             }
         }

src/NHibernate/Impl/AbstractQueryImpl.cs

@@ -196,32 +196,22 @@ private IType GuessType(System.Type clazz)
 				throw new ArgumentNullException("clazz", "The IType can not be guessed for a null value.");
 			}
 
-			string typename = clazz.AssemblyQualifiedName;
-			IType type = TypeFactory.HeuristicType(typename);
-			bool serializable = (type != null && type is SerializableType);
-			if (type == null || serializable)
+			var type = TypeFactory.HeuristicType(clazz);
+			if (type == null || type is SerializableType)
 			{
-				try
+				if (session.Factory.TryGetEntityPersister(clazz.FullName) != null)
 				{
-					session.Factory.GetEntityPersister(clazz.FullName);
+					return NHibernateUtil.Entity(clazz);
 				}
-				catch (MappingException)
+
+				if (type == null)
 				{
-					if (serializable)
-					{
-						return type;
-					}
-					else
-					{
-						throw new HibernateException("Could not determine a type for class: " + typename);
-					}
+					throw new HibernateException(
+						"Could not determine a type for class: " + clazz.AssemblyQualifiedName);
 				}
-				return NHibernateUtil.Entity(clazz);
-			}
-			else
-			{
-				return type;
 			}
+
+			return type;
 		}
 
 		/// <summary>

src/NHibernate/Mapping/ByCode/Impl/TypeNameUtil.cs

@@ -8,7 +8,7 @@ public static class TypeNameUtil
 		public static string GetNhTypeName(this System.Type type)
 		{
 			string typeName;
-			IType nhType = TypeFactory.HeuristicType(type.AssemblyQualifiedName);
+			IType nhType = TypeFactory.HeuristicType(type);
 			if (nhType != null)
 			{
 				typeName = nhType.Name;
@@ -68,4 +68,4 @@ private static string GetTypeNameForMapping(System.Type type)
 			return type.Name;
 		}
 	}
-}
+}

src/NHibernate/Type/TypeFactory.cs

@@ -405,26 +405,11 @@ public static IType Basic(string name)
 		/// </remarks>
 		public static IType Basic(string name, IDictionary<string, string> parameters)
 		{
-			string typeName;
-
 			// Use the basic name (such as String or String(255)) to get the
-			// instance of the IType object.
-			IType returnType;
-			if (typeByTypeOfName.TryGetValue(name, out returnType))
-			{
-				if (_obsoleteMessageByAlias.TryGetValue(name, out string obsoleteMessage))
-					_log.Warn("{0} is obsolete. {1}", name, obsoleteMessage);
-
-				if (parameters?.Count > 0 && returnType is IParameterizedType)
-				{
-					// The type is parameterized, must apply the parameters to a new instance of the type.
-					// Some built-in types have internal default constructor like StringType, so we need to
-					// allow non-public constructors.
-					returnType = (IType) Activator.CreateInstance(returnType.GetType(), true);
-					InjectParameters(returnType, parameters);
-				}
+			// instance of the IType object.			
+			var returnType = GetBasicTypeByName(name, parameters);
+			if (returnType != null)
 				return returnType;
-			}
 
 			// if we get to here then the basic type with the length or precision/scale
 			// combination doesn't exists - so lets figure out which one we have and
@@ -442,7 +427,7 @@ public static IType Basic(string name, IDictionary<string, string> parameters)
 						"TypeClassification.PrecisionScale", name, "It is not a valid Precision/Scale name");
 				}
 
-				typeName = parsedName[0].Trim();
+				string typeName = parsedName[0].Trim();
 				byte precision = Byte.Parse(parsedName[1].Trim());
 				byte scale = Byte.Parse(parsedName[2].Trim());
 
@@ -459,7 +444,7 @@ public static IType Basic(string name, IDictionary<string, string> parameters)
 						"TypeClassification.LengthOrScale", name, "It is not a valid Length or Scale name");
 				}
 
-				typeName = parsedName[0].Trim();
+				string typeName = parsedName[0].Trim();
 				int length = Int32.Parse(parsedName[1].Trim());
 
 				returnType = BuiltInType(typeName, length);
@@ -478,6 +463,26 @@ public static IType Basic(string name, IDictionary<string, string> parameters)
 			return returnType;
 		}
 
+		private static IType GetBasicTypeByName(string name, IDictionary<string, string> parameters)
+		{
+			if (typeByTypeOfName.TryGetValue(name, out var returnType))
+			{
+				if (_obsoleteMessageByAlias.TryGetValue(name, out string obsoleteMessage))
+					_log.Warn("{0} is obsolete. {1}", name, obsoleteMessage);
+
+				if (parameters?.Count > 0 && returnType is IParameterizedType)
+				{
+					// The type is parameterized, must apply the parameters to a new instance of the type.
+					// Some built-in types have internal default constructor like StringType, so we need to
+					// allow non-public constructors.
+					returnType = (IType) Activator.CreateInstance(returnType.GetType(), true);
+					InjectParameters(returnType, parameters);
+				}
+				return returnType;
+			}
+			return null;
+		}
+
 		internal static IType BuiltInType(string typeName, int lengthOrScale)
 		{
 			GetNullableTypeWithLengthOrScale lengthOrScaleDelegate;
@@ -522,6 +527,23 @@ public static IType HeuristicType(string typeName)
 			return HeuristicType(typeName, null);
 		}
 
+		/// <summary>
+		/// Uses heuristics to deduce a NHibernate type given a string naming the
+		/// type.
+		/// </summary>
+		/// <param name="type"></param>
+		/// <returns>An instance of <c>NHibernate.Type.IType</c></returns>
+		/// <remarks>
+		/// We check to see if it implements IType, ICompositeUserType, IUserType, ILifecycle (Association), or
+		/// IPersistentEnum.  If none of those are implemented then we will serialize the Type to the
+		/// database using NHibernate.Type.SerializableType(typeName)
+		/// </remarks>
+		public static IType HeuristicType(System.Type type)
+		{
+			return GetBasicTypeByName(type.AssemblyQualifiedName, null)
+				?? GetBySystemType(type, null, null);
+		}
+
 		/// <summary>
 		/// Uses heuristics to deduce a NHibernate type given a string naming the type.
 		/// </summary>
@@ -532,7 +554,7 @@ public static IType HeuristicType(string typeName, IDictionary<string, string> p
 		{
 			return HeuristicType(typeName, parameters, null);
 		}
-		
+
 		/// <summary>
 		/// Uses heuristics to deduce a NHibernate type given a string naming the type.
 		/// </summary>
@@ -546,55 +568,71 @@ public static IType HeuristicType(string typeName, IDictionary<string, string> p
 
 			if (type != null)
 				return type;
-			
+
 			string[] parsedTypeName;
-			TypeClassification typeClassification = GetTypeClassification(typeName);
+			var typeClassification = GetTypeClassification(typeName);
 			if (typeClassification == TypeClassification.LengthOrScale)
+			{
 				parsedTypeName = typeName.Split(LengthSplit);
+				if (!int.TryParse(parsedTypeName[1], out int parsedLength))
+				{
+					throw new MappingException($"Could not parse length value '{parsedTypeName[1]}' as int for type '{typeName}'");
+				}
+				length = parsedLength;
+			}
 			else
 				parsedTypeName = typeClassification == TypeClassification.PrecisionScale ? typeName.Split(PrecisionScaleSplit) : new[] { typeName };
 
-
 			System.Type typeClass;
 			try
 			{
 				typeClass = ReflectHelper.ClassForName(parsedTypeName[0]); //typeName);
 			}
 			catch (Exception)
 			{
-				typeClass = null;
+				return null;
 			}
 
-			if (typeClass == null)
-				return null;
-				
+			return GetBySystemType(typeClass, parameters, length);
+		}
+
+		private static IType GetBySystemType(System.Type typeClass, IDictionary<string, string> parameters, int? length)
+		{
 			if (typeof(IType).IsAssignableFrom(typeClass))
 			{
 				try
 				{
-					type = (IType) Environment.ObjectsFactory.CreateInstance(typeClass);
+					var type = (IType) Environment.ObjectsFactory.CreateInstance(typeClass);
+					InjectParameters(type, parameters);
+
+					var obsolete = typeClass.GetCustomAttribute<ObsoleteAttribute>(false);
+					if (obsolete != null)
+					{
+						_log.Warn("{0} ({1}) is obsolete. {2}", typeClass.FullName, type.Name, obsolete.Message);
+					}
+
+					return type;
 				}
-				catch (Exception e)
+				catch (HibernateException)
 				{
-					throw new MappingException("Could not instantiate IType " + typeClass.Name + ": " + e, e);
+					throw;
 				}
-				InjectParameters(type, parameters);
-
-				var obsolete = typeClass.GetCustomAttribute<ObsoleteAttribute>(false);
-				if (obsolete != null)
+				catch (Exception e)
 				{
-					_log.Warn("{0} is obsolete. {1}", typeName, obsolete.Message);
+					throw new MappingException("Could not instantiate IType " + typeClass.Name + ": " + e, e);
 				}
-				return type;
 			}
+
 			if (typeof(ICompositeUserType).IsAssignableFrom(typeClass))
 			{
 				return new CompositeCustomType(typeClass, parameters);
 			}
+
 			if (typeof(IUserType).IsAssignableFrom(typeClass))
 			{
 				return new CustomType(typeClass, parameters);
 			}
+
 			if (typeof(ILifecycle).IsAssignableFrom(typeClass))
 			{
 				return NHibernateUtil.Entity(typeClass);
@@ -603,15 +641,12 @@ public static IType HeuristicType(string typeName, IDictionary<string, string> p
 			var unwrapped = typeClass.UnwrapIfNullable();
 			if (unwrapped.IsEnum)
 			{
-				return (IType) Activator.CreateInstance(typeof (EnumType<>).MakeGenericType(unwrapped));
+				return (IType) Activator.CreateInstance(typeof(EnumType<>).MakeGenericType(unwrapped));
 			}
 
 			if (!typeClass.IsSerializable)
 				return null;
 
-			if (typeClassification == TypeClassification.LengthOrScale)
-				return GetSerializableType(typeClass, Int32.Parse(parsedTypeName[1]));
-			
 			if (length.HasValue)
 				return GetSerializableType(typeClass, length.Value);
 
@@ -690,7 +725,7 @@ public static NullableType GetSerializableType(System.Type serializableType)
 			// So we should add the type with its other key in a later operation in order to ensure we cache the same
 			// instance for both keys.
 			var added = false;
-			var type = (NullableType)typeByTypeOfName.GetOrAdd(
+			var type = typeByTypeOfName.GetOrAdd(
 				key,
 				k =>
 				{
@@ -703,7 +738,7 @@ public static NullableType GetSerializableType(System.Type serializableType)
 				throw new HibernateException($"Another item with the key {type.Name} has already been added to typeByTypeOfName.");
 			}
 
-			return type;
+			return (NullableType) type;
 		}
 
 		public static NullableType GetSerializableType(System.Type serializableType, int length)

src/NHibernate/Util/ReflectHelper.cs

@@ -293,7 +293,7 @@ public static IType ReflectedPropertyType(System.Type theClass, string name, str
 
 			var heuristicClass = propertyClass.UnwrapIfNullable();
 
-			return TypeFactory.HeuristicType(heuristicClass.AssemblyQualifiedName);
+			return TypeFactory.HeuristicType(heuristicClass);
 		}
 
 		/// <summary>