diff --git a/src/NHibernate.Test/Async/NHSpecificTest/GH1226/Fixture.cs b/src/NHibernate.Test/Async/NHSpecificTest/GH1226/Fixture.cs new file mode 100644 index 00000000000..91bc4287096 --- /dev/null +++ b/src/NHibernate.Test/Async/NHSpecificTest/GH1226/Fixture.cs @@ -0,0 +1,164 @@ +//------------------------------------------------------------------------------ +// +// 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.Linq; +using NHibernate.Engine; +using NHibernate.Persister.Entity; +using NUnit.Framework; +using NHibernate.Linq; + +namespace NHibernate.Test.NHSpecificTest.GH1226 +{ + using System.Threading.Tasks; + [TestFixture] + public class FixtureAsync : BugTestCase + { + protected override void OnSetUp() + { + base.OnSetUp(); + + using (var session = OpenSession()) + { + using (var tx = session.BeginTransaction()) + { + var bank = new Bank { Code = "01234" }; + session.Save(bank); + + var account = new Account { Bank = bank }; + session.Save(account); + + var account2 = new Account { Bank = bank }; + session.Save(account2); + + tx.Commit(); + } + } + Sfi.Statistics.IsStatisticsEnabled = true; + } + + [Test] + public async Task BankShouldBeJoinFetchedAsync() + { + // Simple case: nothing already in session. + using (var session = OpenSession()) + using (var tx = session.BeginTransaction()) + { + var countBeforeQuery = Sfi.Statistics.PrepareStatementCount; + + var accounts = await (session.CreateQuery("from Account a left join fetch a.Bank").ListAsync()); + var associatedBanks = accounts.Select(x => x.Bank).ToList(); + Assert.That(associatedBanks, Has.All.Matches(NHibernateUtil.IsInitialized), + "One bank or more was lazily loaded."); + + var countAfterQuery = Sfi.Statistics.PrepareStatementCount; + var statementCount = countAfterQuery - countBeforeQuery; + + await (tx.CommitAsync()); + + Assert.That(statementCount, Is.EqualTo(1)); + } + } + + [Test] + public async Task InSessionBankShouldBeJoinFetchedAsync() + { + using (var session = OpenSession()) + using (var tx = session.BeginTransaction()) + { + // #1226 bug only occurs if the Banks are already in the session cache. + await (session.CreateQuery("from Bank").ListAsync()); + + var countBeforeQuery = Sfi.Statistics.PrepareStatementCount; + + var accounts = await (session.CreateQuery("from Account a left join fetch a.Bank").ListAsync()); + var associatedBanks = accounts.Select(x => x.Bank).ToList(); + Assert.That(associatedBanks, Has.All.Matches(NHibernateUtil.IsInitialized), + "One bank or more was lazily loaded."); + + var countAfterQuery = Sfi.Statistics.PrepareStatementCount; + var statementCount = countAfterQuery - countBeforeQuery; + + await (tx.CommitAsync()); + + Assert.That(statementCount, Is.EqualTo(1)); + } + } + + [Test] + public async Task AlteredBankShouldBeJoinFetchedAsync() + { + using (var s1 = OpenSession()) + { + using (var tx = s1.BeginTransaction()) + { + // Put them all in s1 cache. + await (s1.CreateQuery("from Bank").ListAsync()); + await (tx.CommitAsync()); + } + + string oldCode; + const string newCode = "12345"; + // Alter the bank code with another session. + using (var s2 = OpenSession()) + using (var tx2 = s2.BeginTransaction()) + { + var accounts = await (s2.Query().ToListAsync()); + foreach (var account in accounts) + account.Bank = null; + await (s2.FlushAsync()); + var bank = await (s2.Query().SingleAsync()); + oldCode = bank.Code; + bank.Code = newCode; + await (s2.FlushAsync()); + foreach (var account in accounts) + account.Bank = bank; + await (tx2.CommitAsync()); + } + + // Check querying them with s1 is still consistent + using (var tx = s1.BeginTransaction()) + { + var accounts = await (s1.CreateQuery("from Account a left join fetch a.Bank").ListAsync()); + var associatedBanks = accounts.Select(x => x.Bank).ToList(); + Assert.That(associatedBanks, Has.All.Not.Null, + "One bank or more failed loading."); + Assert.That(associatedBanks, Has.All.Matches(NHibernateUtil.IsInitialized), + "One bank or more was lazily loaded."); + Assert.That(associatedBanks, Has.All.Property(nameof(Bank.Code)).EqualTo(oldCode), + "One bank or more has no more the old code."); + + await (tx.CommitAsync()); + // Do not check statements count: we are in a special case defeating the eager fetching, because + // we have stale data in session for the bank code. + // But check that the new code, supposed to be unknown for the session, is not cached. + var persister = Sfi.GetEntityPersister(typeof(Bank).FullName); + var index = ((IUniqueKeyLoadable) persister).GetPropertyIndex(nameof(Bank.Code)); + var type = persister.PropertyTypes[index]; + var euk = new EntityUniqueKey(persister.EntityName, nameof(Bank.Code), newCode, type, Sfi); + Assert.That(s1.GetSessionImplementation().PersistenceContext.GetEntity(euk), + Is.Null, "Found a bank associated to the new code in s1"); + } + } + } + + protected override void OnTearDown() + { + base.OnTearDown(); + + using (var session = OpenSession()) + using (var tx = session.BeginTransaction()) + { + session.CreateQuery("delete from Account").ExecuteUpdate(); + session.CreateQuery("delete from Bank").ExecuteUpdate(); + tx.Commit(); + } + } + } +} diff --git a/src/NHibernate.Test/NHSpecificTest/GH1226/Fixture.cs b/src/NHibernate.Test/NHSpecificTest/GH1226/Fixture.cs new file mode 100644 index 00000000000..a149b0cf45c --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/GH1226/Fixture.cs @@ -0,0 +1,152 @@ +using System.Linq; +using NHibernate.Engine; +using NHibernate.Persister.Entity; +using NUnit.Framework; + +namespace NHibernate.Test.NHSpecificTest.GH1226 +{ + [TestFixture] + public class Fixture : BugTestCase + { + protected override void OnSetUp() + { + base.OnSetUp(); + + using (var session = OpenSession()) + { + using (var tx = session.BeginTransaction()) + { + var bank = new Bank { Code = "01234" }; + session.Save(bank); + + var account = new Account { Bank = bank }; + session.Save(account); + + var account2 = new Account { Bank = bank }; + session.Save(account2); + + tx.Commit(); + } + } + Sfi.Statistics.IsStatisticsEnabled = true; + } + + [Test] + public void BankShouldBeJoinFetched() + { + // Simple case: nothing already in session. + using (var session = OpenSession()) + using (var tx = session.BeginTransaction()) + { + var countBeforeQuery = Sfi.Statistics.PrepareStatementCount; + + var accounts = session.CreateQuery("from Account a left join fetch a.Bank").List(); + var associatedBanks = accounts.Select(x => x.Bank).ToList(); + Assert.That(associatedBanks, Has.All.Matches(NHibernateUtil.IsInitialized), + "One bank or more was lazily loaded."); + + var countAfterQuery = Sfi.Statistics.PrepareStatementCount; + var statementCount = countAfterQuery - countBeforeQuery; + + tx.Commit(); + + Assert.That(statementCount, Is.EqualTo(1)); + } + } + + [Test] + public void InSessionBankShouldBeJoinFetched() + { + using (var session = OpenSession()) + using (var tx = session.BeginTransaction()) + { + // #1226 bug only occurs if the Banks are already in the session cache. + session.CreateQuery("from Bank").List(); + + var countBeforeQuery = Sfi.Statistics.PrepareStatementCount; + + var accounts = session.CreateQuery("from Account a left join fetch a.Bank").List(); + var associatedBanks = accounts.Select(x => x.Bank).ToList(); + Assert.That(associatedBanks, Has.All.Matches(NHibernateUtil.IsInitialized), + "One bank or more was lazily loaded."); + + var countAfterQuery = Sfi.Statistics.PrepareStatementCount; + var statementCount = countAfterQuery - countBeforeQuery; + + tx.Commit(); + + Assert.That(statementCount, Is.EqualTo(1)); + } + } + + [Test] + public void AlteredBankShouldBeJoinFetched() + { + using (var s1 = OpenSession()) + { + using (var tx = s1.BeginTransaction()) + { + // Put them all in s1 cache. + s1.CreateQuery("from Bank").List(); + tx.Commit(); + } + + string oldCode; + const string newCode = "12345"; + // Alter the bank code with another session. + using (var s2 = OpenSession()) + using (var tx2 = s2.BeginTransaction()) + { + var accounts = s2.Query().ToList(); + foreach (var account in accounts) + account.Bank = null; + s2.Flush(); + var bank = s2.Query().Single(); + oldCode = bank.Code; + bank.Code = newCode; + s2.Flush(); + foreach (var account in accounts) + account.Bank = bank; + tx2.Commit(); + } + + // Check querying them with s1 is still consistent + using (var tx = s1.BeginTransaction()) + { + var accounts = s1.CreateQuery("from Account a left join fetch a.Bank").List(); + var associatedBanks = accounts.Select(x => x.Bank).ToList(); + Assert.That(associatedBanks, Has.All.Not.Null, + "One bank or more failed loading."); + Assert.That(associatedBanks, Has.All.Matches(NHibernateUtil.IsInitialized), + "One bank or more was lazily loaded."); + Assert.That(associatedBanks, Has.All.Property(nameof(Bank.Code)).EqualTo(oldCode), + "One bank or more has no more the old code."); + + tx.Commit(); + // Do not check statements count: we are in a special case defeating the eager fetching, because + // we have stale data in session for the bank code. + // But check that the new code, supposed to be unknown for the session, is not cached. + var persister = Sfi.GetEntityPersister(typeof(Bank).FullName); + var index = ((IUniqueKeyLoadable) persister).GetPropertyIndex(nameof(Bank.Code)); + var type = persister.PropertyTypes[index]; + var euk = new EntityUniqueKey(persister.EntityName, nameof(Bank.Code), newCode, type, Sfi); + Assert.That(s1.GetSessionImplementation().PersistenceContext.GetEntity(euk), + Is.Null, "Found a bank associated to the new code in s1"); + } + } + } + + protected override void OnTearDown() + { + base.OnTearDown(); + + using (var session = OpenSession()) + using (var tx = session.BeginTransaction()) + { + session.CreateQuery("delete from Account").ExecuteUpdate(); + session.CreateQuery("delete from Bank").ExecuteUpdate(); + tx.Commit(); + } + } + } +} diff --git a/src/NHibernate.Test/NHSpecificTest/GH1226/Mappings.hbm.xml b/src/NHibernate.Test/NHSpecificTest/GH1226/Mappings.hbm.xml new file mode 100644 index 00000000000..04d537158c2 --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/GH1226/Mappings.hbm.xml @@ -0,0 +1,18 @@ + + + + + + + + + + + + + + + + diff --git a/src/NHibernate.Test/NHSpecificTest/GH1226/Model.cs b/src/NHibernate.Test/NHSpecificTest/GH1226/Model.cs new file mode 100644 index 00000000000..9b527a7d51d --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/GH1226/Model.cs @@ -0,0 +1,16 @@ +using System; + +namespace NHibernate.Test.NHSpecificTest.GH1226 +{ + public class Account + { + public virtual Guid Id { get; set; } + public virtual Bank Bank { get; set; } + } + + public class Bank + { + public virtual Guid Id { get; set; } + public virtual string Code { get; set; } + } +} diff --git a/src/NHibernate/Async/Loader/Loader.cs b/src/NHibernate/Async/Loader/Loader.cs index da2bab6d5a6..70865f30988 100644 --- a/src/NHibernate/Async/Loader/Loader.cs +++ b/src/NHibernate/Async/Loader/Loader.cs @@ -591,7 +591,7 @@ private async Task GetRowAsync(DbDataReader rs, ILoadable[] persisters object obj = null; EntityKey key = keys[i]; - if (keys[i] == null) + if (key == null) { // do nothing /* TODO NH-1001 : if (persisters[i]...EntityType) is an OneToMany or a ManyToOne and @@ -603,17 +603,22 @@ private async Task GetRowAsync(DbDataReader rs, ILoadable[] persisters { //If the object is already loaded, return the loaded one obj = await (session.GetEntityUsingInterceptorAsync(key, cancellationToken)).ConfigureAwait(false); - if (obj != null) + var alreadyLoaded = obj != null; + var persister = persisters[i]; + if (alreadyLoaded) { //its already loaded so dont need to hydrate it - await (InstanceAlreadyLoadedAsync(rs, i, persisters[i], key, obj, lockModes[i], session, cancellationToken)).ConfigureAwait(false); + await (InstanceAlreadyLoadedAsync(rs, i, persister, key, obj, lockModes[i], session, cancellationToken)).ConfigureAwait(false); } else { obj = - await (InstanceNotYetLoadedAsync(rs, i, persisters[i], key, lockModes[i], descriptors[i].RowIdAlias, optionalObjectKey, + await (InstanceNotYetLoadedAsync(rs, i, persister, key, lockModes[i], descriptors[i].RowIdAlias, optionalObjectKey, optionalObject, hydratedObjects, session, cancellationToken)).ConfigureAwait(false); } + // #1226: Even if it is already loaded, if it can be loaded from an association with a property ref, make + // sure it is also cached by its unique key. + CacheByUniqueKey(i, persister, obj, session, alreadyLoaded); } rowResults[i] = obj; @@ -724,26 +729,6 @@ private async Task LoadFromResultSetAsync(DbDataReader rs, int i, object obj, st object rowId = persister.HasRowId ? rs[rowIdAlias] : null; - IAssociationType[] ownerAssociationTypes = OwnerAssociationTypes; - if (ownerAssociationTypes != null && ownerAssociationTypes[i] != null) - { - string ukName = ownerAssociationTypes[i].RHSUniqueKeyPropertyName; - if (ukName != null) - { - int index = ((IUniqueKeyLoadable)persister).GetPropertyIndex(ukName); - IType type = persister.PropertyTypes[index]; - - // polymorphism not really handled completely correctly, - // perhaps...well, actually its ok, assuming that the - // entity name used in the lookup is the same as the - // the one used here, which it will be - - EntityUniqueKey euk = - new EntityUniqueKey(rootPersister.EntityName, ukName, await (type.SemiResolveAsync(values[index], session, obj, cancellationToken)).ConfigureAwait(false), type, session.Factory); - session.PersistenceContext.AddEntity(euk, obj); - } - } - TwoPhaseLoad.PostHydrate(persister, id, values, rowId, obj, lockMode, !eagerPropertyFetch, session); } diff --git a/src/NHibernate/Loader/Loader.cs b/src/NHibernate/Loader/Loader.cs index 89c8271202d..d334a6c4a18 100644 --- a/src/NHibernate/Loader/Loader.cs +++ b/src/NHibernate/Loader/Loader.cs @@ -914,7 +914,7 @@ private object[] GetRow(DbDataReader rs, ILoadable[] persisters, EntityKey[] key object obj = null; EntityKey key = keys[i]; - if (keys[i] == null) + if (key == null) { // do nothing /* TODO NH-1001 : if (persisters[i]...EntityType) is an OneToMany or a ManyToOne and @@ -926,17 +926,22 @@ private object[] GetRow(DbDataReader rs, ILoadable[] persisters, EntityKey[] key { //If the object is already loaded, return the loaded one obj = session.GetEntityUsingInterceptor(key); - if (obj != null) + var alreadyLoaded = obj != null; + var persister = persisters[i]; + if (alreadyLoaded) { //its already loaded so dont need to hydrate it - InstanceAlreadyLoaded(rs, i, persisters[i], key, obj, lockModes[i], session); + InstanceAlreadyLoaded(rs, i, persister, key, obj, lockModes[i], session); } else { obj = - InstanceNotYetLoaded(rs, i, persisters[i], key, lockModes[i], descriptors[i].RowIdAlias, optionalObjectKey, + InstanceNotYetLoaded(rs, i, persister, key, lockModes[i], descriptors[i].RowIdAlias, optionalObjectKey, optionalObject, hydratedObjects, session); } + // #1226: Even if it is already loaded, if it can be loaded from an association with a property ref, make + // sure it is also cached by its unique key. + CacheByUniqueKey(i, persister, obj, session, alreadyLoaded); } rowResults[i] = obj; @@ -974,6 +979,36 @@ private void InstanceAlreadyLoaded(DbDataReader rs, int i, IEntityPersister pers } } + private void CacheByUniqueKey(int i, IEntityPersister persister, object obj, ISessionImplementor session, bool alreadyLoaded) + { + var ownerAssociationTypes = OwnerAssociationTypes; + if (ownerAssociationTypes == null) + return; + var ukName = ownerAssociationTypes[i]?.RHSUniqueKeyPropertyName; + if (ukName == null) + return; + var index = ((IUniqueKeyLoadable)persister).GetPropertyIndex(ukName); + var ukValue = alreadyLoaded + ? persister.GetPropertyValue(obj, index) + : session.PersistenceContext.GetEntry(obj).LoadedState[index]; + // ukValue can be null for two reasons: + // - Entity thought to be already loaded but indeed currently loading and not yet fully hydrated. + // In such case, it has already been handled by InstanceNotYetLoaded path on a previous row, + // there is nothing more to do. This case could also be detected with + // "session.PersistenceContext.GetEntry(obj).Status == Status.Loading", but since there + // is a second case, just test for ukValue null. + // - Entity association is unset in session but not yet persisted, autoflush disabled: ignore. We are + // already in an error case: querying entities changed in session without flushing them before querying. + // So here it gets loaded as if it were still associated, but we do not have the key anymore in session: + // we cannot cache it, so long for the additionnal round-trip this will cause. (Do not fallback on + // reading the key in rs, this is stale data in regard to the session state.) + if (ukValue == null) + return; + var type = persister.PropertyTypes[index]; + var euk = new EntityUniqueKey(persister.EntityName, ukName, ukValue, type, session.Factory); + session.PersistenceContext.AddEntity(euk, obj); + } + /// /// The entity instance is not in the session cache /// @@ -1050,26 +1085,6 @@ private void LoadFromResultSet(DbDataReader rs, int i, object obj, string instan object rowId = persister.HasRowId ? rs[rowIdAlias] : null; - IAssociationType[] ownerAssociationTypes = OwnerAssociationTypes; - if (ownerAssociationTypes != null && ownerAssociationTypes[i] != null) - { - string ukName = ownerAssociationTypes[i].RHSUniqueKeyPropertyName; - if (ukName != null) - { - int index = ((IUniqueKeyLoadable)persister).GetPropertyIndex(ukName); - IType type = persister.PropertyTypes[index]; - - // polymorphism not really handled completely correctly, - // perhaps...well, actually its ok, assuming that the - // entity name used in the lookup is the same as the - // the one used here, which it will be - - EntityUniqueKey euk = - new EntityUniqueKey(rootPersister.EntityName, ukName, type.SemiResolve(values[index], session, obj), type, session.Factory); - session.PersistenceContext.AddEntity(euk, obj); - } - } - TwoPhaseLoad.PostHydrate(persister, id, values, rowId, obj, lockMode, !eagerPropertyFetch, session); }