Fix collection filter on subclass columns (#3264)

Closes #3079

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();