From 00b36c0e1369058eb7432a5752ac55c20b95030f Mon Sep 17 00:00:00 2001 From: Konstantin Fedorchenko Date: Wed, 29 Nov 2017 11:19:28 +0200 Subject: [PATCH 1/4] Join-fetching with property-ref results in n+1 select * Fixes #1226 (NH-2534) --- .../Async/NHSpecificTest/GH1226/Fixture.cs | 95 +++++++++++++++++++ .../NHSpecificTest/GH1226/Fixture.cs | 84 ++++++++++++++++ .../NHSpecificTest/GH1226/Mappings.hbm.xml | 18 ++++ .../NHSpecificTest/GH1226/Model.cs | 16 ++++ src/NHibernate/Async/Loader/Loader.cs | 2 + src/NHibernate/Loader/Loader.cs | 28 ++++++ 6 files changed, 243 insertions(+) create mode 100644 src/NHibernate.Test/Async/NHSpecificTest/GH1226/Fixture.cs create mode 100644 src/NHibernate.Test/NHSpecificTest/GH1226/Fixture.cs create mode 100644 src/NHibernate.Test/NHSpecificTest/GH1226/Mappings.hbm.xml create mode 100644 src/NHibernate.Test/NHSpecificTest/GH1226/Model.cs 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..04f76075288 --- /dev/null +++ b/src/NHibernate.Test/Async/NHSpecificTest/GH1226/Fixture.cs @@ -0,0 +1,95 @@ +//------------------------------------------------------------------------------ +// +// 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 System.Collections.Generic; +using System.Linq; +using NUnit.Framework; + +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(); + } + } + } + + [Test] + public async Task BankShouldBeJoinFetchedAsync() + { + using (var session = OpenSession()) + { + var wasStatisticsEnabled = session.SessionFactory.Statistics.IsStatisticsEnabled; + session.SessionFactory.Statistics.IsStatisticsEnabled = true; + + long statementCount; + + using (var tx = session.BeginTransaction()) + { + // Bug only occurs if the Banks are already in the session cache. + var preloadedBanks = await (session.CreateQuery("from Bank").ListAsync()); + + var countBeforeQuery = session.SessionFactory.Statistics.PrepareStatementCount; + + Console.WriteLine("Query: -------------------------------------------------------"); + + var accounts = await (session.CreateQuery("from Account a left join fetch a.Bank").ListAsync()); + IList associatedBanks = accounts.Select(x => x.Bank).ToList(); + + var countAfterQuery = session.SessionFactory.Statistics.PrepareStatementCount; + statementCount = countAfterQuery - countBeforeQuery; + + Console.WriteLine("End ----------------------------------------------------------"); + + await (tx.CommitAsync()); + } + + session.SessionFactory.Statistics.IsStatisticsEnabled = wasStatisticsEnabled; + + Assert.That(statementCount, Is.EqualTo(1)); + } + } + + protected override void OnTearDown() + { + base.OnTearDown(); + + using (var session = OpenSession()) + { + using (var tx = session.BeginTransaction()) + { + session.Delete("from Account"); + session.Delete("from Bank"); + 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..5d169497ada --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/GH1226/Fixture.cs @@ -0,0 +1,84 @@ +using System; +using System.Collections.Generic; +using System.Linq; +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(); + } + } + } + + [Test] + public void BankShouldBeJoinFetched() + { + using (var session = OpenSession()) + { + var wasStatisticsEnabled = session.SessionFactory.Statistics.IsStatisticsEnabled; + session.SessionFactory.Statistics.IsStatisticsEnabled = true; + + long statementCount; + + using (var tx = session.BeginTransaction()) + { + // Bug only occurs if the Banks are already in the session cache. + var preloadedBanks = session.CreateQuery("from Bank").List(); + + var countBeforeQuery = session.SessionFactory.Statistics.PrepareStatementCount; + + Console.WriteLine("Query: -------------------------------------------------------"); + + var accounts = session.CreateQuery("from Account a left join fetch a.Bank").List(); + IList associatedBanks = accounts.Select(x => x.Bank).ToList(); + + var countAfterQuery = session.SessionFactory.Statistics.PrepareStatementCount; + statementCount = countAfterQuery - countBeforeQuery; + + Console.WriteLine("End ----------------------------------------------------------"); + + tx.Commit(); + } + + session.SessionFactory.Statistics.IsStatisticsEnabled = wasStatisticsEnabled; + + Assert.That(statementCount, Is.EqualTo(1)); + } + } + + protected override void OnTearDown() + { + base.OnTearDown(); + + using (var session = OpenSession()) + { + using (var tx = session.BeginTransaction()) + { + session.Delete("from Account"); + session.Delete("from Bank"); + 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..d7dbb66166d 100644 --- a/src/NHibernate/Async/Loader/Loader.cs +++ b/src/NHibernate/Async/Loader/Loader.cs @@ -650,6 +650,8 @@ private async Task InstanceAlreadyLoadedAsync(DbDataReader rs, int i, IEntityPer entry.LockMode = lockMode; } } + + CacheByUniqueKey(i, persister, obj, session); } /// diff --git a/src/NHibernate/Loader/Loader.cs b/src/NHibernate/Loader/Loader.cs index 89c8271202d..86106055794 100644 --- a/src/NHibernate/Loader/Loader.cs +++ b/src/NHibernate/Loader/Loader.cs @@ -972,6 +972,34 @@ private void InstanceAlreadyLoaded(DbDataReader rs, int i, IEntityPersister pers entry.LockMode = lockMode; } } + + CacheByUniqueKey(i, persister, obj, session); + } + + private void CacheByUniqueKey(int i, IEntityPersister persister, object obj, ISessionImplementor session) + { + // #1226: If it is already loaded and can be loaded from an association with a property ref, make + // sure it is also cached by its unique key. + var ukName = OwnerAssociationTypes?[i]?.RHSUniqueKeyPropertyName; + if (ukName == null) + return; + var index = ((IUniqueKeyLoadable)persister).GetPropertyIndex(ukName); + var ukValue = persister.GetPropertyValue(obj, index); + // ukValue can be null for two reasons: + // - Entity currently loading and not yet fully hydrated. In such case, it has already been handled by + // InstanceNotYetLoaded 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); } /// From 912d21091682ee8a1b012c5c960d13cb2163e3ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= Date: Wed, 20 Dec 2017 13:40:51 +0100 Subject: [PATCH 2/4] Mutualize caching by unique key. --- src/NHibernate/Async/Loader/Loader.cs | 35 +++++------------ src/NHibernate/Loader/Loader.cs | 55 ++++++++++----------------- 2 files changed, 30 insertions(+), 60 deletions(-) diff --git a/src/NHibernate/Async/Loader/Loader.cs b/src/NHibernate/Async/Loader/Loader.cs index d7dbb66166d..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; @@ -650,8 +655,6 @@ private async Task InstanceAlreadyLoadedAsync(DbDataReader rs, int i, IEntityPer entry.LockMode = lockMode; } } - - CacheByUniqueKey(i, persister, obj, session); } /// @@ -726,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 86106055794..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; @@ -972,23 +977,25 @@ private void InstanceAlreadyLoaded(DbDataReader rs, int i, IEntityPersister pers entry.LockMode = lockMode; } } - - CacheByUniqueKey(i, persister, obj, session); } - private void CacheByUniqueKey(int i, IEntityPersister persister, object obj, ISessionImplementor session) + private void CacheByUniqueKey(int i, IEntityPersister persister, object obj, ISessionImplementor session, bool alreadyLoaded) { - // #1226: If it is already loaded and can be loaded from an association with a property ref, make - // sure it is also cached by its unique key. - var ukName = OwnerAssociationTypes?[i]?.RHSUniqueKeyPropertyName; + var ownerAssociationTypes = OwnerAssociationTypes; + if (ownerAssociationTypes == null) + return; + var ukName = ownerAssociationTypes[i]?.RHSUniqueKeyPropertyName; if (ukName == null) return; var index = ((IUniqueKeyLoadable)persister).GetPropertyIndex(ukName); - var ukValue = persister.GetPropertyValue(obj, index); + var ukValue = alreadyLoaded + ? persister.GetPropertyValue(obj, index) + : session.PersistenceContext.GetEntry(obj).LoadedState[index]; // ukValue can be null for two reasons: - // - Entity currently loading and not yet fully hydrated. In such case, it has already been handled by - // InstanceNotYetLoaded 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 + // - 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. @@ -1078,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); } From c62819643394ff7c87939f7fa3b7dbfb107797ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= Date: Wed, 20 Dec 2017 18:10:01 +0100 Subject: [PATCH 3/4] Tests adjustment and stale data case. --- .../Async/NHSpecificTest/GH1226/Fixture.cs | 98 +++++++++++++----- .../NHSpecificTest/GH1226/Fixture.cs | 99 ++++++++++++++----- 2 files changed, 144 insertions(+), 53 deletions(-) diff --git a/src/NHibernate.Test/Async/NHSpecificTest/GH1226/Fixture.cs b/src/NHibernate.Test/Async/NHSpecificTest/GH1226/Fixture.cs index 04f76075288..1cac6531c45 100644 --- a/src/NHibernate.Test/Async/NHSpecificTest/GH1226/Fixture.cs +++ b/src/NHibernate.Test/Async/NHSpecificTest/GH1226/Fixture.cs @@ -8,10 +8,11 @@ //------------------------------------------------------------------------------ -using System; -using System.Collections.Generic; using System.Linq; +using NHibernate.Engine; +using NHibernate.Persister.Entity; using NUnit.Framework; +using NHibernate.Linq; namespace NHibernate.Test.NHSpecificTest.GH1226 { @@ -39,41 +40,88 @@ protected override void OnSetUp() tx.Commit(); } } + Sfi.Statistics.IsStatisticsEnabled = true; } [Test] public async Task BankShouldBeJoinFetchedAsync() { using (var session = OpenSession()) + using (var tx = session.BeginTransaction()) { - var wasStatisticsEnabled = session.SessionFactory.Statistics.IsStatisticsEnabled; - session.SessionFactory.Statistics.IsStatisticsEnabled = true; + // Bug only occurs if the Banks are already in the session cache. + await (session.CreateQuery("from Bank").ListAsync()); - long statementCount; + var countBeforeQuery = Sfi.Statistics.PrepareStatementCount; - using (var tx = session.BeginTransaction()) - { - // Bug only occurs if the Banks are already in the session cache. - var preloadedBanks = await (session.CreateQuery("from Bank").ListAsync()); - - var countBeforeQuery = session.SessionFactory.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."); - Console.WriteLine("Query: -------------------------------------------------------"); + var countAfterQuery = Sfi.Statistics.PrepareStatementCount; + var statementCount = countAfterQuery - countBeforeQuery; - var accounts = await (session.CreateQuery("from Account a left join fetch a.Bank").ListAsync()); - IList associatedBanks = accounts.Select(x => x.Bank).ToList(); + await (tx.CommitAsync()); - var countAfterQuery = session.SessionFactory.Statistics.PrepareStatementCount; - statementCount = countAfterQuery - countBeforeQuery; - - Console.WriteLine("End ----------------------------------------------------------"); + 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()); } - session.SessionFactory.Statistics.IsStatisticsEnabled = wasStatisticsEnabled; + 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()); + } - Assert.That(statementCount, Is.EqualTo(1)); + // 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"); + } } } @@ -82,13 +130,11 @@ protected override void OnTearDown() base.OnTearDown(); using (var session = OpenSession()) + using (var tx = session.BeginTransaction()) { - using (var tx = session.BeginTransaction()) - { - session.Delete("from Account"); - session.Delete("from Bank"); - tx.Commit(); - } + 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 index 5d169497ada..1ef811be5ac 100644 --- a/src/NHibernate.Test/NHSpecificTest/GH1226/Fixture.cs +++ b/src/NHibernate.Test/NHSpecificTest/GH1226/Fixture.cs @@ -1,6 +1,6 @@ -using System; -using System.Collections.Generic; -using System.Linq; +using System.Linq; +using NHibernate.Engine; +using NHibernate.Persister.Entity; using NUnit.Framework; namespace NHibernate.Test.NHSpecificTest.GH1226 @@ -28,41 +28,88 @@ protected override void OnSetUp() tx.Commit(); } } + Sfi.Statistics.IsStatisticsEnabled = true; } [Test] public void BankShouldBeJoinFetched() { using (var session = OpenSession()) + using (var tx = session.BeginTransaction()) { - var wasStatisticsEnabled = session.SessionFactory.Statistics.IsStatisticsEnabled; - session.SessionFactory.Statistics.IsStatisticsEnabled = true; + // Bug only occurs if the Banks are already in the session cache. + session.CreateQuery("from Bank").List(); - long statementCount; + var countBeforeQuery = Sfi.Statistics.PrepareStatementCount; - using (var tx = session.BeginTransaction()) - { - // Bug only occurs if the Banks are already in the session cache. - var preloadedBanks = session.CreateQuery("from Bank").List(); - - var countBeforeQuery = session.SessionFactory.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."); - Console.WriteLine("Query: -------------------------------------------------------"); + var countAfterQuery = Sfi.Statistics.PrepareStatementCount; + var statementCount = countAfterQuery - countBeforeQuery; - var accounts = session.CreateQuery("from Account a left join fetch a.Bank").List(); - IList associatedBanks = accounts.Select(x => x.Bank).ToList(); + tx.Commit(); - var countAfterQuery = session.SessionFactory.Statistics.PrepareStatementCount; - statementCount = countAfterQuery - countBeforeQuery; - - Console.WriteLine("End ----------------------------------------------------------"); + 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(); } - session.SessionFactory.Statistics.IsStatisticsEnabled = wasStatisticsEnabled; + 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(); + } - Assert.That(statementCount, Is.EqualTo(1)); + // 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"); + } } } @@ -71,13 +118,11 @@ protected override void OnTearDown() base.OnTearDown(); using (var session = OpenSession()) + using (var tx = session.BeginTransaction()) { - using (var tx = session.BeginTransaction()) - { - session.Delete("from Account"); - session.Delete("from Bank"); - tx.Commit(); - } + session.CreateQuery("delete from Account").ExecuteUpdate(); + session.CreateQuery("delete from Bank").ExecuteUpdate(); + tx.Commit(); } } } From 7b74b9dd304699cc49ed4e4912a85e321aac8308 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= Date: Thu, 21 Dec 2017 15:14:22 +0100 Subject: [PATCH 4/4] Add nominal case test. --- .../Async/NHSpecificTest/GH1226/Fixture.cs | 25 ++++++++++++++++++- .../NHSpecificTest/GH1226/Fixture.cs | 25 ++++++++++++++++++- 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/src/NHibernate.Test/Async/NHSpecificTest/GH1226/Fixture.cs b/src/NHibernate.Test/Async/NHSpecificTest/GH1226/Fixture.cs index 1cac6531c45..91bc4287096 100644 --- a/src/NHibernate.Test/Async/NHSpecificTest/GH1226/Fixture.cs +++ b/src/NHibernate.Test/Async/NHSpecificTest/GH1226/Fixture.cs @@ -46,10 +46,33 @@ protected override void OnSetUp() [Test] public async Task BankShouldBeJoinFetchedAsync() { + // Simple case: nothing already in session. using (var session = OpenSession()) using (var tx = session.BeginTransaction()) { - // Bug only occurs if the Banks are already in the session cache. + 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; diff --git a/src/NHibernate.Test/NHSpecificTest/GH1226/Fixture.cs b/src/NHibernate.Test/NHSpecificTest/GH1226/Fixture.cs index 1ef811be5ac..a149b0cf45c 100644 --- a/src/NHibernate.Test/NHSpecificTest/GH1226/Fixture.cs +++ b/src/NHibernate.Test/NHSpecificTest/GH1226/Fixture.cs @@ -34,10 +34,33 @@ protected override void OnSetUp() [Test] public void BankShouldBeJoinFetched() { + // Simple case: nothing already in session. using (var session = OpenSession()) using (var tx = session.BeginTransaction()) { - // Bug only occurs if the Banks are already in the session cache. + 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;