Skip null entities when bag is populated

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<Asset>()
+				             .Fetch(SelectMode.Fetch, x => x.DocumentsBag)
+				             .TransformUsing(Transformers.DistinctRootEntity)
+				             .ListAsync<Asset>());
+
+				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()

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()
 		{
@@ -66,21 +78,52 @@ 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));
 
 			// 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++)
+
+			//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())
 			{
-				if (dv2.Drives[i] == null) nullCount++;
+				dv2 = await (s.GetAsync<Device>(dv2.Id));
+				await (s.FlushAsync());
+				await (t.CommitAsync());
+			}
+
+			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())
+			using (var t = s.BeginTransaction())
+			{
+				dv2 = await (s.GetAsync<Device>(dv2.Id));
+				dv2.Drives.Add(dr2);
+				await (t.CommitAsync());
+			}
+
+			await (VerifyResultAsync(2,2,  msg: "modified collection"));
+
+			async Task VerifyResultAsync(int expectedInCollection, int expectedInDb, string msg)
+			{
+				using (var s = Sfi.OpenSession())
+				{
+					var realCound = Convert.ToInt32(
+						await (s.CreateSQLQuery("select count(*) from DriveOfDevice where DeviceId = :id ")
+						.SetParameter("id", dv2.Id)
+						.UniqueResultAsync<object>()));
+					dv2 = await (s.GetAsync<Device>(dv2.Id));
+
+					Assert.That(dv2.Drives.Count, Is.EqualTo(expectedInCollection), msg);
+					Assert.That(realCound, Is.EqualTo(expectedInDb), msg);
+				}
 			}
-			Assert.AreEqual(1, nullCount);
 		}
 	}
-}
+}

src/NHibernate.Test/NHSpecificTest/GH1994/Entity.cs

@@ -14,6 +14,7 @@ public class Asset : Base
 	{
 		public virtual ISet<Document> Documents { get; set; } = new HashSet<Document>();
 		public virtual ISet<Document> DocumentsFiltered { get; set; } = new HashSet<Document>();
+		public virtual IList<Document> DocumentsBag { get; set; } = new List<Document>();
 	}
 
 	public class Document : Base

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<Asset>()
+				             .Fetch(SelectMode.Fetch, x => x.DocumentsBag)
+				             .TransformUsing(Transformers.DistinctRootEntity)
+				             .List<Asset>();
+
+				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()

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

@@ -18,6 +18,12 @@
 				<filter name="deletedFilter" condition="IsDeleted = :deletedParam"/>
 			</many-to-many>
 		</set>
+    <bag name="DocumentsBag" table="asset_to_document" lazy="true" cascade="none">
+      <key column="AssetId"/>
+      <many-to-many class="Document" column="DocumentId">
+        <filter name="deletedFilter" condition="IsDeleted = :deletedParam"/>
+      </many-to-many>
+    </bag>
 		<set name="DocumentsFiltered" table="asset_to_document" lazy="true" cascade="none">
 			<key column="AssetId"/>
 			<many-to-many class="Document" column="DocumentId" where="IsDeleted = 0"/>

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()
 		{
@@ -55,21 +67,52 @@ 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));
 
 			// 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++)
+
+			//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())
 			{
-				if (dv2.Drives[i] == null) nullCount++;
+				dv2 = s.Get<Device>(dv2.Id);
+				s.Flush();
+				t.Commit();
+			}
+
+			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())
+			using (var t = s.BeginTransaction())
+			{
+				dv2 = s.Get<Device>(dv2.Id);
+				dv2.Drives.Add(dr2);
+				t.Commit();
+			}
+
+			VerifyResult(2, 2, msg: "modified collection");
+
+			void VerifyResult(int expectedInCollection, int expectedInDb, string msg)
+			{
+				using (var s = Sfi.OpenSession())
+				{
+					var realCound = Convert.ToInt32(
+						s.CreateSQLQuery("select count(*) from DriveOfDevice where DeviceId = :id ")
+						.SetParameter("id", dv2.Id)
+						.UniqueResult<object>());
+					dv2 = s.Get<Device>(dv2.Id);
+
+					Assert.That(dv2.Drives.Count, Is.EqualTo(expectedInCollection), msg);
+					Assert.That(realCound, Is.EqualTo(expectedInDb), msg);
+				}
 			}
-			Assert.AreEqual(1, nullCount);
 		}
 	}
-}
+}

src/NHibernate/Async/Collection/Generic/PersistentGenericBag.cs

@@ -154,11 +154,9 @@ public override async Task<object> 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;
 		}
 	}

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;
 		}