diff --git a/src/NHibernate.Test/Async/NHSpecificTest/GH1645/Fixture.cs b/src/NHibernate.Test/Async/NHSpecificTest/GH1645/Fixture.cs new file mode 100644 index 00000000000..017658be42a --- /dev/null +++ b/src/NHibernate.Test/Async/NHSpecificTest/GH1645/Fixture.cs @@ -0,0 +1,66 @@ +//------------------------------------------------------------------------------ +// +// This code was generated by AsyncGenerator. +// +// Changes to this file may cause incorrect behavior and will be lost if +// the code is regenerated. +// +//------------------------------------------------------------------------------ + + +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(_superParentId)); + Assert.That(() => NHibernateUtil.InitializeAsync(superParent), Throws.Nothing); + Assert.That(() => NHibernateUtil.InitializeAsync(superParent.Parent), Throws.Nothing); + } + } + } +} diff --git a/src/NHibernate.Test/NHSpecificTest/GH1645/EntityBase.cs b/src/NHibernate.Test/NHSpecificTest/GH1645/EntityBase.cs new file mode 100644 index 00000000000..a855e6b58de --- /dev/null +++ b/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(); + } + } +} diff --git a/src/NHibernate.Test/NHSpecificTest/GH1645/Fixture.cs b/src/NHibernate.Test/NHSpecificTest/GH1645/Fixture.cs new file mode 100644 index 00000000000..64a27ef63d6 --- /dev/null +++ b/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(_superParentId); + Assert.That(() => NHibernateUtil.Initialize(superParent), Throws.Nothing); + Assert.That(() => NHibernateUtil.Initialize(superParent.Parent), Throws.Nothing); + } + } + } +} diff --git a/src/NHibernate.Test/NHSpecificTest/GH1645/Mappings.hbm.xml b/src/NHibernate.Test/NHSpecificTest/GH1645/Mappings.hbm.xml new file mode 100644 index 00000000000..83fc1ab6f10 --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/GH1645/Mappings.hbm.xml @@ -0,0 +1,15 @@ + + + + + + + + + + + + + + diff --git a/src/NHibernate.Test/NHSpecificTest/GH1645/Parent.cs b/src/NHibernate.Test/NHSpecificTest/GH1645/Parent.cs new file mode 100644 index 00000000000..c9dc2342d28 --- /dev/null +++ b/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; } + } +} diff --git a/src/NHibernate.Test/NHSpecificTest/GH1645/SuperParent.cs b/src/NHibernate.Test/NHSpecificTest/GH1645/SuperParent.cs new file mode 100644 index 00000000000..ceeb4651785 --- /dev/null +++ b/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; } + } +} diff --git a/src/NHibernate/Async/Type/EntityType.cs b/src/NHibernate/Async/Type/EntityType.cs index 6e0e345b1bc..adef950ecdd 100644 --- a/src/NHibernate/Async/Type/EntityType.cs +++ b/src/NHibernate/Async/Type/EntityType.cs @@ -218,12 +218,19 @@ public async Task 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; diff --git a/src/NHibernate/Engine/EntityUniqueKey.cs b/src/NHibernate/Engine/EntityUniqueKey.cs index 8646295e02d..9affd5a664e 100644 --- a/src/NHibernate/Engine/EntityUniqueKey.cs +++ b/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() } } -} \ No newline at end of file +} diff --git a/src/NHibernate/Loader/Loader.cs b/src/NHibernate/Loader/Loader.cs index fb069e6eb98..bfe3cc92346 100644 --- a/src/NHibernate/Loader/Loader.cs +++ b/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); } diff --git a/src/NHibernate/Type/EntityType.cs b/src/NHibernate/Type/EntityType.cs index 5b1e94f1307..fd603e2acb4 100644 --- a/src/NHibernate/Type/EntityType.cs +++ b/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;