Fix collection filter on subclass columns (#3264)

Closes #3079

Author: bahusoid

src/NHibernate.Test/Async/SubclassFilterTest/JoinedSubclassFilterTest.cs

@@ -8,7 +8,6 @@
 //------------------------------------------------------------------------------
 
 
-using System;
 using System.Collections;
 using NUnit.Framework;
 using System.Linq;
@@ -17,29 +16,36 @@
 namespace NHibernate.Test.SubclassFilterTest
 {
 	using System.Threading.Tasks;
-	using System.Threading;
 	[TestFixture]
 	public class JoinedSubclassFilterTestAsync : TestCase
 	{
-		protected override string[] Mappings
+		protected override string[] Mappings => new[] {"SubclassFilterTest.joined-subclass.hbm.xml"};
+
+		protected override string MappingsAssembly => "NHibernate.Test";
+
+		protected override void OnSetUp()
 		{
-			get { return new string[] {"SubclassFilterTest.joined-subclass.hbm.xml"}; }
+			using var s = OpenSession();
+			using var t = s.BeginTransaction();
+			PrepareTestData(s);
+			t.Commit();
 		}
 
-		protected override string MappingsAssembly
+		protected override void OnTearDown()
 		{
-			get { return "NHibernate.Test"; }
+			using var s = OpenSession();
+			using var t = s.BeginTransaction();
+			s.Delete("from Customer c where c.ContactOwner is not null");
+			s.Delete("from Employee e where e.Manager is not null");
+			s.Delete("from Person");
+			t.Commit();
 		}
 
 		[Test]
 		public async Task FiltersWithSubclassAsync()
 		{
-			ISession s = OpenSession();
+			using var s = OpenSession();
 			s.EnableFilter("region").SetParameter("userRegion", "US");
-			ITransaction t = s.BeginTransaction();
-
-			await (PrepareTestDataAsync(s));
-			s.Clear();
 
 			IList results;
 
@@ -62,14 +68,6 @@ public async Task FiltersWithSubclassAsync()
 			}
 			s.Clear();
 
-			// TODO : currently impossible to define a collection-level filter w/
-			// joined-subclass elements that will filter based on a superclass
-			// column and function correctly in (theta only?) outer joins;
-			// this is consistent with the behaviour of a collection-level where.
-			// this might be one argument for "pulling" the attached class-level
-			// filters into collection assocations,
-			// although we'd need some way to apply the appropriate alias in that
-			// scenario.
 			results = (await (s.CreateQuery("from Person as p left join fetch p.Minions").ListAsync<Person>())).Distinct().ToList();
 			Assert.AreEqual(4, results.Count, "Incorrect qry result count");
 			foreach (Person p in results)
@@ -95,25 +93,12 @@ public async Task FiltersWithSubclassAsync()
 					break;
 				}
 			}
-
-			await (t.CommitAsync());
-			s.Close();
-
-			s = OpenSession();
-			t = s.BeginTransaction();
-			await (s.DeleteAsync("from Customer c where c.ContactOwner is not null"));
-			await (s.DeleteAsync("from Employee e where e.Manager is not null"));
-			await (s.DeleteAsync("from Person"));
-			await (t.CommitAsync());
-			s.Close();
 		}
 
 		[Test]
 		public async Task FilterCollectionWithSubclass1Async()
 		{
 			using var s = OpenSession();
-			using var t = s.BeginTransaction();
-			await (PrepareTestDataAsync(s));
 
 			s.EnableFilter("minionsWithManager");
 
@@ -122,22 +107,18 @@ public async Task FilterCollectionWithSubclass1Async()
 			Assert.That(employees[0].Minions.Count, Is.EqualTo(2));
 		}
 
-		[KnownBug("GH-3079: Collection filter on subclass columns")]
-		[Test]
+		[Test(Description = "GH-3079: Collection filter on subclass columns")]
 		public async Task FilterCollectionWithSubclass2Async()
 		{
 			using var s = OpenSession();
-			using var t = s.BeginTransaction();
-			await (PrepareTestDataAsync(s));
-
 			s.EnableFilter("minionsRegion").SetParameter("userRegion", "US");
 
 			var employees = await (s.Query<Employee>().Where(x => x.Minions.Any()).ToListAsync());
 			Assert.That(employees.Count, Is.EqualTo(1));
 			Assert.That(employees[0].Minions.Count, Is.EqualTo(1));
 		}
 
-		private static async Task PrepareTestDataAsync(ISession s, CancellationToken cancellationToken = default(CancellationToken))
+		private static void PrepareTestData(ISession s)
 		{
 			Employee john = new Employee("John Doe");
 			john.Company = ("JBoss");
@@ -170,11 +151,9 @@ public async Task FilterCollectionWithSubclass2Async()
 			ups.Company = ("UPS");
 			ups.Region = ("US");
 
-			await (s.SaveAsync(john, cancellationToken));
-			await (s.SaveAsync(cust, cancellationToken));
-			await (s.SaveAsync(ups, cancellationToken));
-
-			await (s.FlushAsync(cancellationToken));
+			s.Save(john);
+			s.Save(cust);
+			s.Save(ups);
 		}
 	}
 }

src/NHibernate.Test/SubclassFilterTest/JoinedSubclassFilterTest.cs

@@ -1,4 +1,3 @@
-using System;
 using System.Collections;
 using NUnit.Framework;
 using System.Linq;
@@ -8,25 +7,33 @@ namespace NHibernate.Test.SubclassFilterTest
 	[TestFixture]
 	public class JoinedSubclassFilterTest : TestCase
 	{
-		protected override string[] Mappings
+		protected override string[] Mappings => new[] {"SubclassFilterTest.joined-subclass.hbm.xml"};
+
+		protected override string MappingsAssembly => "NHibernate.Test";
+
+		protected override void OnSetUp()
 		{
-			get { return new string[] {"SubclassFilterTest.joined-subclass.hbm.xml"}; }
+			using var s = OpenSession();
+			using var t = s.BeginTransaction();
+			PrepareTestData(s);
+			t.Commit();
 		}
 
-		protected override string MappingsAssembly
+		protected override void OnTearDown()
 		{
-			get { return "NHibernate.Test"; }
+			using var s = OpenSession();
+			using var t = s.BeginTransaction();
+			s.Delete("from Customer c where c.ContactOwner is not null");
+			s.Delete("from Employee e where e.Manager is not null");
+			s.Delete("from Person");
+			t.Commit();
 		}
 
 		[Test]
 		public void FiltersWithSubclass()
 		{
-			ISession s = OpenSession();
+			using var s = OpenSession();
 			s.EnableFilter("region").SetParameter("userRegion", "US");
-			ITransaction t = s.BeginTransaction();
-
-			PrepareTestData(s);
-			s.Clear();
 
 			IList results;
 
@@ -49,14 +56,6 @@ public void FiltersWithSubclass()
 			}
 			s.Clear();
 
-			// TODO : currently impossible to define a collection-level filter w/
-			// joined-subclass elements that will filter based on a superclass
-			// column and function correctly in (theta only?) outer joins;
-			// this is consistent with the behaviour of a collection-level where.
-			// this might be one argument for "pulling" the attached class-level
-			// filters into collection assocations,
-			// although we'd need some way to apply the appropriate alias in that
-			// scenario.
 			results = s.CreateQuery("from Person as p left join fetch p.Minions").List<Person>().Distinct().ToList();
 			Assert.AreEqual(4, results.Count, "Incorrect qry result count");
 			foreach (Person p in results)
@@ -82,25 +81,12 @@ public void FiltersWithSubclass()
 					break;
 				}
 			}
-
-			t.Commit();
-			s.Close();
-
-			s = OpenSession();
-			t = s.BeginTransaction();
-			s.Delete("from Customer c where c.ContactOwner is not null");
-			s.Delete("from Employee e where e.Manager is not null");
-			s.Delete("from Person");
-			t.Commit();
-			s.Close();
 		}
 
 		[Test]
 		public void FilterCollectionWithSubclass1()
 		{
 			using var s = OpenSession();
-			using var t = s.BeginTransaction();
-			PrepareTestData(s);
 
 			s.EnableFilter("minionsWithManager");
 
@@ -109,14 +95,10 @@ public void FilterCollectionWithSubclass1()
 			Assert.That(employees[0].Minions.Count, Is.EqualTo(2));
 		}
 
-		[KnownBug("GH-3079: Collection filter on subclass columns")]
-		[Test]
+		[Test(Description = "GH-3079: Collection filter on subclass columns")]
 		public void FilterCollectionWithSubclass2()
 		{
 			using var s = OpenSession();
-			using var t = s.BeginTransaction();
-			PrepareTestData(s);
-
 			s.EnableFilter("minionsRegion").SetParameter("userRegion", "US");
 
 			var employees = s.Query<Employee>().Where(x => x.Minions.Any()).ToList();
@@ -160,8 +142,6 @@ private static void PrepareTestData(ISession s)
 			s.Save(john);
 			s.Save(cust);
 			s.Save(ups);
-
-			s.Flush();
 		}
 	}
 }

src/NHibernate/Persister/Collection/AbstractCollectionPersister.cs

@@ -1503,10 +1503,16 @@ protected virtual string FilterFragment(string alias)
 
 		public virtual string FilterFragment(string alias, IDictionary<string, IFilter> enabledFilters)
 		{
+			var filterFragment = FilterFragment(alias);
+			if (!filterHelper.IsAffectedBy(enabledFilters))
+				return filterFragment;
+
+			if (ElementType.IsEntityType && elementPersister is AbstractEntityPersister ep)
+				return ep.FilterFragment(filterHelper, alias, enabledFilters, filterFragment);
+
 			StringBuilder sessionFilterFragment = new StringBuilder();
 			filterHelper.Render(sessionFilterFragment, alias, enabledFilters);
-
-			return sessionFilterFragment.Append(FilterFragment(alias)).ToString();
+			return sessionFilterFragment.Append(filterFragment).ToString();
 		}
 
 		public string OneToManyFilterFragment(string alias)

src/NHibernate/Persister/Entity/AbstractEntityPersister.cs

@@ -3729,6 +3729,12 @@ public virtual string FilterFragment(string alias, IDictionary<string, IFilter>
 			if (!filterHelper.IsAffectedBy(enabledFilters))
 				return filterFragment;
 
+			return FilterFragment(filterHelper, alias, enabledFilters, filterFragment);
+		}
+
+		//TODO 6.0: Move to IEntityPersister and adjust usages accordingly
+		public virtual string FilterFragment(FilterHelper filterHelper, string alias, IDictionary<string, IFilter> enabledFilters, string filterFragment)
+		{
 			var sessionFilterFragment = new StringBuilder();
 			filterHelper.Render(sessionFilterFragment, GenerateFilterConditionAlias(alias), GetColumnsToTableAliasMap(alias), enabledFilters);
 			return sessionFilterFragment.Append(filterFragment).ToString();