From 9ee0020e7436e23df9edb5dcd64107341e4e5e5c Mon Sep 17 00:00:00 2001 From: Roman Artiukhin Date: Fri, 14 Jun 2019 08:36:23 +0300 Subject: [PATCH 1/4] Skip null entities in bag --- .../Async/NHSpecificTest/GH1994/Fixture.cs | 17 +++++++++++++++++ .../Async/NHSpecificTest/NH750/Fixture.cs | 12 +++--------- .../NHSpecificTest/GH1994/Entity.cs | 1 + .../NHSpecificTest/GH1994/Fixture.cs | 17 +++++++++++++++++ .../NHSpecificTest/GH1994/Mappings.hbm.xml | 6 ++++++ .../NHSpecificTest/NH750/Fixture.cs | 12 +++--------- .../Collection/Generic/PersistentGenericBag.cs | 8 +++----- .../Collection/Generic/PersistentGenericBag.cs | 8 +++----- 8 files changed, 53 insertions(+), 28 deletions(-) diff --git a/src/NHibernate.Test/Async/NHSpecificTest/GH1994/Fixture.cs b/src/NHibernate.Test/Async/NHSpecificTest/GH1994/Fixture.cs index 89f287d5688..ff47bd2ad14 100644 --- a/src/NHibernate.Test/Async/NHSpecificTest/GH1994/Fixture.cs +++ b/src/NHibernate.Test/Async/NHSpecificTest/GH1994/Fixture.cs @@ -117,6 +117,23 @@ public async Task TestFilteredQueryOverAsync() } } + [Test] + public async Task TestFilteredBagQueryOverAsync() + { + using (var s = OpenSession()) + { + s.EnableFilter("deletedFilter").SetParameter("deletedParam", false); + + var query = await (s.QueryOver() + .Fetch(SelectMode.Fetch, x => x.DocumentsBag) + .TransformUsing(Transformers.DistinctRootEntity) + .ListAsync()); + + Assert.That(query.Count, Is.EqualTo(1), "filtered assets"); + Assert.That(query[0].DocumentsBag.Count, Is.EqualTo(1), "filtered asset documents"); + } + } + //NH-2991 [Test] public async Task TestQueryOverRestrictionWithClauseAsync() diff --git a/src/NHibernate.Test/Async/NHSpecificTest/NH750/Fixture.cs b/src/NHibernate.Test/Async/NHSpecificTest/NH750/Fixture.cs index 828332a5867..4aca125273a 100644 --- a/src/NHibernate.Test/Async/NHSpecificTest/NH750/Fixture.cs +++ b/src/NHibernate.Test/Async/NHSpecificTest/NH750/Fixture.cs @@ -66,7 +66,8 @@ public async Task DeviceOfDriveAsync() dv2 = (Device) await (s.LoadAsync(typeof(Device), dvSavedId[1])); } Assert.AreEqual(2, dv1.Drives.Count); - Assert.AreEqual(2, dv2.Drives.Count); + // Verify one is missing + Assert.AreEqual(1, dv2.Drives.Count); // Verify dv1 unchanged Assert.IsTrue(dv1.Drives.Contains(dr1)); Assert.IsTrue(dv1.Drives.Contains(dr2)); @@ -74,13 +75,6 @@ public async Task DeviceOfDriveAsync() // Verify dv2 Assert.IsTrue(dv2.Drives.Contains(dr1)); Assert.IsFalse(dv2.Drives.Contains(dr3)); - // Verify one null - int nullCount = 0; - for (int i = 0; i < dv2.Drives.Count; i++) - { - if (dv2.Drives[i] == null) nullCount++; - } - Assert.AreEqual(1, nullCount); } } -} \ No newline at end of file +} diff --git a/src/NHibernate.Test/NHSpecificTest/GH1994/Entity.cs b/src/NHibernate.Test/NHSpecificTest/GH1994/Entity.cs index 44f9719be0b..3d195379b5d 100644 --- a/src/NHibernate.Test/NHSpecificTest/GH1994/Entity.cs +++ b/src/NHibernate.Test/NHSpecificTest/GH1994/Entity.cs @@ -14,6 +14,7 @@ public class Asset : Base { public virtual ISet Documents { get; set; } = new HashSet(); public virtual ISet DocumentsFiltered { get; set; } = new HashSet(); + public virtual IList DocumentsBag { get; set; } = new List(); } public class Document : Base diff --git a/src/NHibernate.Test/NHSpecificTest/GH1994/Fixture.cs b/src/NHibernate.Test/NHSpecificTest/GH1994/Fixture.cs index e7d07216d2d..5dbe043ebfe 100644 --- a/src/NHibernate.Test/NHSpecificTest/GH1994/Fixture.cs +++ b/src/NHibernate.Test/NHSpecificTest/GH1994/Fixture.cs @@ -106,6 +106,23 @@ public void TestFilteredQueryOver() } } + [Test] + public void TestFilteredBagQueryOver() + { + using (var s = OpenSession()) + { + s.EnableFilter("deletedFilter").SetParameter("deletedParam", false); + + var query = s.QueryOver() + .Fetch(SelectMode.Fetch, x => x.DocumentsBag) + .TransformUsing(Transformers.DistinctRootEntity) + .List(); + + Assert.That(query.Count, Is.EqualTo(1), "filtered assets"); + Assert.That(query[0].DocumentsBag.Count, Is.EqualTo(1), "filtered asset documents"); + } + } + //NH-2991 [Test] public void TestQueryOverRestrictionWithClause() diff --git a/src/NHibernate.Test/NHSpecificTest/GH1994/Mappings.hbm.xml b/src/NHibernate.Test/NHSpecificTest/GH1994/Mappings.hbm.xml index 57a58acefa8..449a4b8bc97 100644 --- a/src/NHibernate.Test/NHSpecificTest/GH1994/Mappings.hbm.xml +++ b/src/NHibernate.Test/NHSpecificTest/GH1994/Mappings.hbm.xml @@ -18,6 +18,12 @@ + + + + + + diff --git a/src/NHibernate.Test/NHSpecificTest/NH750/Fixture.cs b/src/NHibernate.Test/NHSpecificTest/NH750/Fixture.cs index b3b38396591..d431b44e43a 100644 --- a/src/NHibernate.Test/NHSpecificTest/NH750/Fixture.cs +++ b/src/NHibernate.Test/NHSpecificTest/NH750/Fixture.cs @@ -55,7 +55,8 @@ public void DeviceOfDrive() dv2 = (Device) s.Load(typeof(Device), dvSavedId[1]); } Assert.AreEqual(2, dv1.Drives.Count); - Assert.AreEqual(2, dv2.Drives.Count); + // Verify one is missing + Assert.AreEqual(1, dv2.Drives.Count); // Verify dv1 unchanged Assert.IsTrue(dv1.Drives.Contains(dr1)); Assert.IsTrue(dv1.Drives.Contains(dr2)); @@ -63,13 +64,6 @@ public void DeviceOfDrive() // Verify dv2 Assert.IsTrue(dv2.Drives.Contains(dr1)); Assert.IsFalse(dv2.Drives.Contains(dr3)); - // Verify one null - int nullCount = 0; - for (int i = 0; i < dv2.Drives.Count; i++) - { - if (dv2.Drives[i] == null) nullCount++; - } - Assert.AreEqual(1, nullCount); } } -} \ No newline at end of file +} diff --git a/src/NHibernate/Async/Collection/Generic/PersistentGenericBag.cs b/src/NHibernate/Async/Collection/Generic/PersistentGenericBag.cs index fc1d89befe1..820d03c497d 100644 --- a/src/NHibernate/Async/Collection/Generic/PersistentGenericBag.cs +++ b/src/NHibernate/Async/Collection/Generic/PersistentGenericBag.cs @@ -154,11 +154,9 @@ public override async Task ReadFromAsync(DbDataReader reader, ICollectio // note that if we load this collection from a cartesian product // the multiplicity would be broken ... so use an idbag instead var element = await (role.ReadElementAsync(reader, owner, descriptor.SuffixedElementAliases, Session, cancellationToken)).ConfigureAwait(false); - // NH Different behavior : we don't check for null - // The NH-750 test show how checking for null we are ignoring the not-found tag and - // the DB may have some records ignored by NH. This issue may need some more deep consideration. - //if (element != null) - _gbag.Add((T) element); + + if (element != null) + _gbag.Add((T) element); return element; } } diff --git a/src/NHibernate/Collection/Generic/PersistentGenericBag.cs b/src/NHibernate/Collection/Generic/PersistentGenericBag.cs index 67564a5de3e..d58be9c9a94 100644 --- a/src/NHibernate/Collection/Generic/PersistentGenericBag.cs +++ b/src/NHibernate/Collection/Generic/PersistentGenericBag.cs @@ -448,11 +448,9 @@ public override object ReadFrom(DbDataReader reader, ICollectionPersister role, // note that if we load this collection from a cartesian product // the multiplicity would be broken ... so use an idbag instead var element = role.ReadElement(reader, owner, descriptor.SuffixedElementAliases, Session); - // NH Different behavior : we don't check for null - // The NH-750 test show how checking for null we are ignoring the not-found tag and - // the DB may have some records ignored by NH. This issue may need some more deep consideration. - //if (element != null) - _gbag.Add((T) element); + + if (element != null) + _gbag.Add((T) element); return element; } From 34dc4938b50ad888d485fedce45a35020685f9d2 Mon Sep 17 00:00:00 2001 From: Roman Artiukhin Date: Mon, 17 Jun 2019 17:17:48 +0300 Subject: [PATCH 2/4] Tests for flushing many-to-many collections with not-found="ignore" mapping --- .../Async/NHSpecificTest/NH750/Fixture.cs | 52 +++++++++++++++++++ .../NHSpecificTest/NH750/Fixture.cs | 52 +++++++++++++++++++ 2 files changed, 104 insertions(+) diff --git a/src/NHibernate.Test/Async/NHSpecificTest/NH750/Fixture.cs b/src/NHibernate.Test/Async/NHSpecificTest/NH750/Fixture.cs index 4aca125273a..c1aeb20d5fc 100644 --- a/src/NHibernate.Test/Async/NHSpecificTest/NH750/Fixture.cs +++ b/src/NHibernate.Test/Async/NHSpecificTest/NH750/Fixture.cs @@ -9,6 +9,7 @@ using System; +using NHibernate.Cfg; using NUnit.Framework; namespace NHibernate.Test.NHSpecificTest.NH750 @@ -27,6 +28,17 @@ protected override void OnTearDown() } } + protected override string CacheConcurrencyStrategy + { + get { return null; } + } + + protected override void Configure(Configuration configuration) + { + configuration.SetProperty(Cfg.Environment.UseSecondLevelCache, "false"); + base.Configure(configuration); + } + [Test] public async Task DeviceOfDriveAsync() { @@ -75,6 +87,46 @@ public async Task DeviceOfDriveAsync() // Verify dv2 Assert.IsTrue(dv2.Drives.Contains(dr1)); Assert.IsFalse(dv2.Drives.Contains(dr3)); + + //Make sure that flush didn't touch not-found="ignore" records for not modified collection + using (var s = Sfi.OpenSession()) + using (var t = s.BeginTransaction()) + { + dv2 = await (s.GetAsync(dv2.Id)); + await (s.FlushAsync()); + await (t.CommitAsync()); + } + + using (var s = Sfi.OpenSession()) + { + var realCound = await (s.CreateSQLQuery("select count(*) from DriveOfDevice where DeviceId = :id ") + .SetParameter("id", dv2.Id) + .UniqueResultAsync()); + dv2 = await (s.GetAsync(dv2.Id)); + + Assert.That(dv2.Drives.Count, Is.EqualTo(1), "not modified collection"); + Assert.That(realCound, Is.EqualTo(2), "not modified collection"); + } + + //Many-to-many clears collection and recreates it so not-found ignore records are lost + using (var s = Sfi.OpenSession()) + using (var t = s.BeginTransaction()) + { + dv2 = await (s.GetAsync(dv2.Id)); + dv2.Drives.Add(dr2); + await (t.CommitAsync()); + } + + using (var s = Sfi.OpenSession()) + { + var realCound = await (s.CreateSQLQuery("select count(*) from DriveOfDevice where DeviceId = :id ") + .SetParameter("id", dv2.Id) + .UniqueResultAsync()); + dv2 = await (s.GetAsync(dv2.Id)); + + Assert.That(dv2.Drives.Count, Is.EqualTo(2), "modified collection"); + Assert.That(realCound, Is.EqualTo(2), "modified collection"); + } } } } diff --git a/src/NHibernate.Test/NHSpecificTest/NH750/Fixture.cs b/src/NHibernate.Test/NHSpecificTest/NH750/Fixture.cs index d431b44e43a..c200ab445d2 100644 --- a/src/NHibernate.Test/NHSpecificTest/NH750/Fixture.cs +++ b/src/NHibernate.Test/NHSpecificTest/NH750/Fixture.cs @@ -1,4 +1,5 @@ using System; +using NHibernate.Cfg; using NUnit.Framework; namespace NHibernate.Test.NHSpecificTest.NH750 @@ -16,6 +17,17 @@ protected override void OnTearDown() } } + protected override string CacheConcurrencyStrategy + { + get { return null; } + } + + protected override void Configure(Configuration configuration) + { + configuration.SetProperty(Cfg.Environment.UseSecondLevelCache, "false"); + base.Configure(configuration); + } + [Test] public void DeviceOfDrive() { @@ -64,6 +76,46 @@ public void DeviceOfDrive() // Verify dv2 Assert.IsTrue(dv2.Drives.Contains(dr1)); Assert.IsFalse(dv2.Drives.Contains(dr3)); + + //Make sure that flush didn't touch not-found="ignore" records for not modified collection + using (var s = Sfi.OpenSession()) + using (var t = s.BeginTransaction()) + { + dv2 = s.Get(dv2.Id); + s.Flush(); + t.Commit(); + } + + using (var s = Sfi.OpenSession()) + { + var realCound = s.CreateSQLQuery("select count(*) from DriveOfDevice where DeviceId = :id ") + .SetParameter("id", dv2.Id) + .UniqueResult(); + dv2 = s.Get(dv2.Id); + + Assert.That(dv2.Drives.Count, Is.EqualTo(1), "not modified collection"); + Assert.That(realCound, Is.EqualTo(2), "not modified collection"); + } + + //Many-to-many clears collection and recreates it so not-found ignore records are lost + using (var s = Sfi.OpenSession()) + using (var t = s.BeginTransaction()) + { + dv2 = s.Get(dv2.Id); + dv2.Drives.Add(dr2); + t.Commit(); + } + + using (var s = Sfi.OpenSession()) + { + var realCound = s.CreateSQLQuery("select count(*) from DriveOfDevice where DeviceId = :id ") + .SetParameter("id", dv2.Id) + .UniqueResult(); + dv2 = s.Get(dv2.Id); + + Assert.That(dv2.Drives.Count, Is.EqualTo(2), "modified collection"); + Assert.That(realCound, Is.EqualTo(2), "modified collection"); + } } } } From 591229e0d8234f2792fe494196e4881d383d6931 Mon Sep 17 00:00:00 2001 From: Roman Artiukhin Date: Mon, 17 Jun 2019 18:31:50 +0300 Subject: [PATCH 3/4] Fix tests --- .../Async/NHSpecificTest/NH750/Fixture.cs | 31 +++++++++---------- .../NHSpecificTest/NH750/Fixture.cs | 31 +++++++++---------- 2 files changed, 28 insertions(+), 34 deletions(-) diff --git a/src/NHibernate.Test/Async/NHSpecificTest/NH750/Fixture.cs b/src/NHibernate.Test/Async/NHSpecificTest/NH750/Fixture.cs index c1aeb20d5fc..1e6514d9177 100644 --- a/src/NHibernate.Test/Async/NHSpecificTest/NH750/Fixture.cs +++ b/src/NHibernate.Test/Async/NHSpecificTest/NH750/Fixture.cs @@ -97,16 +97,7 @@ public async Task DeviceOfDriveAsync() await (t.CommitAsync()); } - using (var s = Sfi.OpenSession()) - { - var realCound = await (s.CreateSQLQuery("select count(*) from DriveOfDevice where DeviceId = :id ") - .SetParameter("id", dv2.Id) - .UniqueResultAsync()); - dv2 = await (s.GetAsync(dv2.Id)); - - Assert.That(dv2.Drives.Count, Is.EqualTo(1), "not modified collection"); - Assert.That(realCound, Is.EqualTo(2), "not modified collection"); - } + await (VerifyResultAsync( expectedInCollection:1, expectedInDb: 2, "not modified collection")); //Many-to-many clears collection and recreates it so not-found ignore records are lost using (var s = Sfi.OpenSession()) @@ -117,15 +108,21 @@ public async Task DeviceOfDriveAsync() await (t.CommitAsync()); } - using (var s = Sfi.OpenSession()) + await (VerifyResultAsync(2,2, "modified collection")); + + async Task VerifyResultAsync(int expectedInCollection, int expectedInDb, string msg) { - var realCound = await (s.CreateSQLQuery("select count(*) from DriveOfDevice where DeviceId = :id ") - .SetParameter("id", dv2.Id) - .UniqueResultAsync()); - dv2 = await (s.GetAsync(dv2.Id)); + using (var s = Sfi.OpenSession()) + { + var realCound = Convert.ToInt32( + await (s.CreateSQLQuery("select count(*) from DriveOfDevice where DeviceId = :id ") + .SetParameter("id", dv2.Id) + .UniqueResultAsync())); + dv2 = await (s.GetAsync(dv2.Id)); - Assert.That(dv2.Drives.Count, Is.EqualTo(2), "modified collection"); - Assert.That(realCound, Is.EqualTo(2), "modified collection"); + Assert.That(dv2.Drives.Count, Is.EqualTo(expectedInCollection), msg); + Assert.That(realCound, Is.EqualTo(expectedInDb), msg); + } } } } diff --git a/src/NHibernate.Test/NHSpecificTest/NH750/Fixture.cs b/src/NHibernate.Test/NHSpecificTest/NH750/Fixture.cs index c200ab445d2..97183476ae1 100644 --- a/src/NHibernate.Test/NHSpecificTest/NH750/Fixture.cs +++ b/src/NHibernate.Test/NHSpecificTest/NH750/Fixture.cs @@ -86,16 +86,7 @@ public void DeviceOfDrive() t.Commit(); } - using (var s = Sfi.OpenSession()) - { - var realCound = s.CreateSQLQuery("select count(*) from DriveOfDevice where DeviceId = :id ") - .SetParameter("id", dv2.Id) - .UniqueResult(); - dv2 = s.Get(dv2.Id); - - Assert.That(dv2.Drives.Count, Is.EqualTo(1), "not modified collection"); - Assert.That(realCound, Is.EqualTo(2), "not modified collection"); - } + VerifyResult( expectedInCollection:1, expectedInDb: 2, "not modified collection"); //Many-to-many clears collection and recreates it so not-found ignore records are lost using (var s = Sfi.OpenSession()) @@ -106,15 +97,21 @@ public void DeviceOfDrive() t.Commit(); } - using (var s = Sfi.OpenSession()) + VerifyResult(2,2, "modified collection"); + + void VerifyResult(int expectedInCollection, int expectedInDb, string msg) { - var realCound = s.CreateSQLQuery("select count(*) from DriveOfDevice where DeviceId = :id ") - .SetParameter("id", dv2.Id) - .UniqueResult(); - dv2 = s.Get(dv2.Id); + using (var s = Sfi.OpenSession()) + { + var realCound = Convert.ToInt32( + s.CreateSQLQuery("select count(*) from DriveOfDevice where DeviceId = :id ") + .SetParameter("id", dv2.Id) + .UniqueResult()); + dv2 = s.Get(dv2.Id); - Assert.That(dv2.Drives.Count, Is.EqualTo(2), "modified collection"); - Assert.That(realCound, Is.EqualTo(2), "modified collection"); + Assert.That(dv2.Drives.Count, Is.EqualTo(expectedInCollection), msg); + Assert.That(realCound, Is.EqualTo(expectedInDb), msg); + } } } } From 8b413e0b0aebc27d888f47e54732dcad795c1275 Mon Sep 17 00:00:00 2001 From: Roman Artiukhin Date: Mon, 17 Jun 2019 19:06:15 +0300 Subject: [PATCH 4/4] Fix build on CI --- src/NHibernate.Test/Async/NHSpecificTest/NH750/Fixture.cs | 4 ++-- src/NHibernate.Test/NHSpecificTest/NH750/Fixture.cs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/NHibernate.Test/Async/NHSpecificTest/NH750/Fixture.cs b/src/NHibernate.Test/Async/NHSpecificTest/NH750/Fixture.cs index 1e6514d9177..ebdf2b44c5f 100644 --- a/src/NHibernate.Test/Async/NHSpecificTest/NH750/Fixture.cs +++ b/src/NHibernate.Test/Async/NHSpecificTest/NH750/Fixture.cs @@ -97,7 +97,7 @@ public async Task DeviceOfDriveAsync() await (t.CommitAsync()); } - await (VerifyResultAsync( expectedInCollection:1, expectedInDb: 2, "not modified collection")); + await (VerifyResultAsync(expectedInCollection: 1, expectedInDb: 2, msg: "not modified collection")); //Many-to-many clears collection and recreates it so not-found ignore records are lost using (var s = Sfi.OpenSession()) @@ -108,7 +108,7 @@ public async Task DeviceOfDriveAsync() await (t.CommitAsync()); } - await (VerifyResultAsync(2,2, "modified collection")); + await (VerifyResultAsync(2,2, msg: "modified collection")); async Task VerifyResultAsync(int expectedInCollection, int expectedInDb, string msg) { diff --git a/src/NHibernate.Test/NHSpecificTest/NH750/Fixture.cs b/src/NHibernate.Test/NHSpecificTest/NH750/Fixture.cs index 97183476ae1..9ff71f07057 100644 --- a/src/NHibernate.Test/NHSpecificTest/NH750/Fixture.cs +++ b/src/NHibernate.Test/NHSpecificTest/NH750/Fixture.cs @@ -86,7 +86,7 @@ public void DeviceOfDrive() t.Commit(); } - VerifyResult( expectedInCollection:1, expectedInDb: 2, "not modified collection"); + VerifyResult(expectedInCollection: 1, expectedInDb: 2, msg: "not modified collection"); //Many-to-many clears collection and recreates it so not-found ignore records are lost using (var s = Sfi.OpenSession()) @@ -97,7 +97,7 @@ public void DeviceOfDrive() t.Commit(); } - VerifyResult(2,2, "modified collection"); + VerifyResult(2, 2, msg: "modified collection"); void VerifyResult(int expectedInCollection, int expectedInDb, string msg) {