Fix key type mismatch in EntityUniqueKey

src/NHibernate.Test/Async/NHSpecificTest/GH1645/Fixture.cs

@@ -0,0 +1,66 @@
+//------------------------------------------------------------------------------
+// <auto-generated>
+//     This code was generated by AsyncGenerator.
+//
+//     Changes to this file may cause incorrect behavior and will be lost if
+//     the code is regenerated.
+// </auto-generated>
+//------------------------------------------------------------------------------
+
+
+using System;
+using NUnit.Framework;
+
+namespace NHibernate.Test.NHSpecificTest.GH1645
+{
+	using System.Threading.Tasks;
+	[TestFixture]
+	public class FixtureAsync : BugTestCase
+	{
+		private Guid _superParentId;
+		private Guid _parentId;
+
+		protected override void OnSetUp()
+		{
+			using (var session = OpenSession())
+			using (var transaction = session.BeginTransaction())
+			{
+				var p = new Parent();
+				session.Save(p);
+				_parentId = p.Id;
+
+				_superParentId = (Guid) session.Save(new SuperParent { Parent = p });
+
+				transaction.Commit();
+			}
+		}
+
+		protected override void OnTearDown()
+		{
+			using (var session = OpenSession())
+			using (var transaction = session.BeginTransaction())
+			{
+				// The HQL delete does all the job inside the database without loading the entities, but it does
+				// not handle delete order for avoiding violating constraints if any. Use
+				// session.Delete("from System.Object");
+				// instead if in need of having NHbernate ordering the deletes, but this will cause
+				// loading the entities in the session.
+				session.CreateQuery("delete from System.Object").ExecuteUpdate();
+
+				transaction.Commit();
+			}
+		}
+
+		[Test]
+		public async Task SOEOnLoadAsync()
+		{
+			using (var session = OpenSession())
+			using (session.BeginTransaction())
+			{
+				var superParent = await (session.LoadAsync<SuperParent>(_superParentId));
+				Assert.That(() => NHibernateUtil.InitializeAsync(superParent), Throws.Nothing);
+				Assert.That(() => NHibernateUtil.InitializeAsync(superParent.Parent), Throws.Nothing);
+			}
+		}
+	}
+}

src/NHibernate.Test/NHSpecificTest/GH1645/EntityBase.cs

@@ -0,0 +1,67 @@
+using System;
+
+namespace NHibernate.Test.NHSpecificTest.GH1645
+{
+	public abstract class EntityBase
+	{
+		public virtual Guid Id { get; protected set; }
+
+		public static bool operator ==(EntityBase left, EntityBase right)
+		{
+			return Equals(left, right);
+		}
+
+		public static bool operator !=(EntityBase left, EntityBase right)
+		{
+			return !Equals(left, right);
+		}
+
+		public override bool Equals(object obj)
+		{
+			return Equals(obj as EntityBase);
+		}
+
+		public virtual bool Equals(EntityBase other)
+		{
+			if (other == null)
+			{
+				return false;
+			}
+
+			if (ReferenceEquals(this, other))
+			{
+				return true;
+			}
+
+			if (!IsTransient(this) && !IsTransient(other) && Equals(Id, other.Id))
+			{
+				var otherType = other.GetUnproxiedType();
+				var thisType = GetUnproxiedType();
+				return thisType.IsAssignableFrom(otherType) ||
+					otherType.IsAssignableFrom(thisType);
+			}
+
+			return false;
+		}
+
+		public override int GetHashCode()
+		{
+			if (Equals(Id, default(Guid)))
+			{
+				return base.GetHashCode();
+			}
+
+			return Id.GetHashCode();
+		}
+
+		private static bool IsTransient(EntityBase obj)
+		{
+			return obj != null && Equals(obj.Id, default(Guid));
+		}
+
+		private System.Type GetUnproxiedType()
+		{
+			return GetType();
+		}
+	}
+}

src/NHibernate.Test/NHSpecificTest/GH1645/Fixture.cs

@@ -0,0 +1,55 @@
+using System;
+using NUnit.Framework;
+
+namespace NHibernate.Test.NHSpecificTest.GH1645
+{
+	[TestFixture]
+	public class Fixture : BugTestCase
+	{
+		private Guid _superParentId;
+		private Guid _parentId;
+
+		protected override void OnSetUp()
+		{
+			using (var session = OpenSession())
+			using (var transaction = session.BeginTransaction())
+			{
+				var p = new Parent();
+				session.Save(p);
+				_parentId = p.Id;
+
+				_superParentId = (Guid) session.Save(new SuperParent { Parent = p });
+
+				transaction.Commit();
+			}
+		}
+
+		protected override void OnTearDown()
+		{
+			using (var session = OpenSession())
+			using (var transaction = session.BeginTransaction())
+			{
+				// The HQL delete does all the job inside the database without loading the entities, but it does
+				// not handle delete order for avoiding violating constraints if any. Use
+				// session.Delete("from System.Object");
+				// instead if in need of having NHbernate ordering the deletes, but this will cause
+				// loading the entities in the session.
+				session.CreateQuery("delete from System.Object").ExecuteUpdate();
+
+				transaction.Commit();
+			}
+		}
+
+		[Test]
+		public void SOEOnLoad()
+		{
+			using (var session = OpenSession())
+			using (session.BeginTransaction())
+			{
+				var superParent = session.Load<SuperParent>(_superParentId);
+				Assert.That(() => NHibernateUtil.Initialize(superParent), Throws.Nothing);
+				Assert.That(() => NHibernateUtil.Initialize(superParent.Parent), Throws.Nothing);
+			}
+		}
+	}
+}

src/NHibernate.Test/NHSpecificTest/GH1645/Mappings.hbm.xml

@@ -0,0 +1,15 @@
+<?xml version="1.0" encoding="utf-8" ?>
+<hibernate-mapping xmlns="urn:nhibernate-mapping-2.2" assembly="NHibernate.Test"
+                   namespace="NHibernate.Test.NHSpecificTest.GH1645">
+
+  <class name="SuperParent">
+    <id name="Id" generator="guid.comb"/>
+    <many-to-one name="Parent"/>
+  </class>
+
+  <class name="Parent">
+    <id name="Id" generator="guid.comb"/>
+    <one-to-one name="SuperParent" property-ref="Parent"/>
+  </class>
+
+</hibernate-mapping>

src/NHibernate.Test/NHSpecificTest/GH1645/Parent.cs

@@ -0,0 +1,7 @@
+namespace NHibernate.Test.NHSpecificTest.GH1645
+{
+	public class Parent : EntityBase
+	{
+		public virtual SuperParent SuperParent { get; set; }
+	}
+}

src/NHibernate.Test/NHSpecificTest/GH1645/SuperParent.cs

@@ -0,0 +1,7 @@
+namespace NHibernate.Test.NHSpecificTest.GH1645
+{
+    public class SuperParent : EntityBase
+    {
+        public virtual Parent Parent { get; set; }
+    }
+}

src/NHibernate/Async/Type/EntityType.cs

@@ -218,12 +218,19 @@ public async Task<object> LoadByUniqueKeyAsync(string entityName, string uniqueK
 
 			//TODO: implement caching?! proxies?!
 
+			var keyType = GetIdentifierOrUniqueKeyType(factory)
+				// EntityUniqueKey was doing this on the type. I suspect this was needed only for its usage in Loader,
+				// which can work with entities as keys not yet instanciated and just represented by their identifiers.
+				// But since removing this call from EntityUniqueKey is done for a patch and that the code path here has
+				// no known bugs with this GetSemiResolvedType, moving its call here for avoiding altering this code
+				// path. See GH1645.
+				.GetSemiResolvedType(factory);
 			EntityUniqueKey euk =
 				new EntityUniqueKey(
 					entityName,
 					uniqueKeyPropertyName,
 					key,
-					GetIdentifierOrUniqueKeyType(factory),
+					keyType,
 					session.Factory);
 
 			IPersistenceContext persistenceContext = session.PersistenceContext;

src/NHibernate/Engine/EntityUniqueKey.cs

@@ -20,12 +20,14 @@ public class EntityUniqueKey
 		private readonly IType keyType;
 		private readonly int hashCode;
 
+		// 6.0 TODO: rename semiResolvedKey as simply key. That is not the responsibility of this class to make any
+		// assumption on the key being semi-resolved or not, that is the responsibility of its callers.
 		public EntityUniqueKey(string entityName, string uniqueKeyName, object semiResolvedKey, IType keyType, ISessionFactoryImplementor factory)
 		{
 			if (string.IsNullOrEmpty(entityName))
 				throw new ArgumentNullException("entityName");
 			if (string.IsNullOrEmpty(uniqueKeyName))
-				throw new ArgumentNullException("entityName");
+				throw new ArgumentNullException("uniqueKeyName");
 			if (semiResolvedKey == null)
 				throw new ArgumentNullException("semiResolvedKey");
 			if (keyType == null)
@@ -34,7 +36,7 @@ public EntityUniqueKey(string entityName, string uniqueKeyName, object semiResol
 			this.entityName = entityName;
 			this.uniqueKeyName = uniqueKeyName;
 			key = semiResolvedKey;
-			this.keyType = keyType.GetSemiResolvedType(factory);
+			this.keyType = keyType;
 			hashCode = GenerateHashCode(factory);
 		}
 
@@ -88,4 +90,4 @@ public override string ToString()
 		}
 
 	}
-}
+}

src/NHibernate/Loader/Loader.cs

@@ -1014,6 +1014,8 @@ private void CacheByUniqueKey(int i, IEntityPersister persister, object obj, ISe
 			if (ukValue == null)
 				return;
 			var type = persister.PropertyTypes[index];
+			if (!alreadyLoaded)
+				type = type.GetSemiResolvedType(session.Factory);
 			var euk = new EntityUniqueKey(persister.EntityName, ukName, ukValue, type, session.Factory);
 			session.PersistenceContext.AddEntity(euk, obj);
 		}

src/NHibernate/Type/EntityType.cs

@@ -508,12 +508,19 @@ public object LoadByUniqueKey(string entityName, string uniqueKeyPropertyName, o
 
 			//TODO: implement caching?! proxies?!
 
+			var keyType = GetIdentifierOrUniqueKeyType(factory)
+				// EntityUniqueKey was doing this on the type. I suspect this was needed only for its usage in Loader,
+				// which can work with entities as keys not yet instanciated and just represented by their identifiers.
+				// But since removing this call from EntityUniqueKey is done for a patch and that the code path here has
+				// no known bugs with this GetSemiResolvedType, moving its call here for avoiding altering this code
+				// path. See GH1645.
+				.GetSemiResolvedType(factory);
 			EntityUniqueKey euk =
 				new EntityUniqueKey(
 					entityName,
 					uniqueKeyPropertyName,
 					key,
-					GetIdentifierOrUniqueKeyType(factory),
+					keyType,
 					session.Factory);
 
 			IPersistenceContext persistenceContext = session.PersistenceContext;