Skip to content

Commit 3819f09

Browse files
bahusoidhazzik
authored andcommitted
Skip null entities when bag is populated (#2181)
1 parent 3041c49 commit 3819f09

File tree

8 files changed

+147
-24
lines changed

8 files changed

+147
-24
lines changed

src/NHibernate.Test/Async/NHSpecificTest/GH1994/Fixture.cs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,23 @@ public async Task TestFilteredQueryOverAsync()
117117
}
118118
}
119119

120+
[Test]
121+
public async Task TestFilteredBagQueryOverAsync()
122+
{
123+
using (var s = OpenSession())
124+
{
125+
s.EnableFilter("deletedFilter").SetParameter("deletedParam", false);
126+
127+
var query = await (s.QueryOver<Asset>()
128+
.Fetch(SelectMode.Fetch, x => x.DocumentsBag)
129+
.TransformUsing(Transformers.DistinctRootEntity)
130+
.ListAsync<Asset>());
131+
132+
Assert.That(query.Count, Is.EqualTo(1), "filtered assets");
133+
Assert.That(query[0].DocumentsBag.Count, Is.EqualTo(1), "filtered asset documents");
134+
}
135+
}
136+
120137
//NH-2991
121138
[Test]
122139
public async Task TestQueryOverRestrictionWithClauseAsync()

src/NHibernate.Test/Async/NHSpecificTest/NH750/Fixture.cs

Lines changed: 50 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010

1111
using System;
12+
using NHibernate.Cfg;
1213
using NUnit.Framework;
1314

1415
namespace NHibernate.Test.NHSpecificTest.NH750
@@ -27,6 +28,17 @@ protected override void OnTearDown()
2728
}
2829
}
2930

31+
protected override string CacheConcurrencyStrategy
32+
{
33+
get { return null; }
34+
}
35+
36+
protected override void Configure(Configuration configuration)
37+
{
38+
configuration.SetProperty(Cfg.Environment.UseSecondLevelCache, "false");
39+
base.Configure(configuration);
40+
}
41+
3042
[Test]
3143
public async Task DeviceOfDriveAsync()
3244
{
@@ -66,21 +78,52 @@ public async Task DeviceOfDriveAsync()
6678
dv2 = (Device) await (s.LoadAsync(typeof(Device), dvSavedId[1]));
6779
}
6880
Assert.AreEqual(2, dv1.Drives.Count);
69-
Assert.AreEqual(2, dv2.Drives.Count);
81+
// Verify one is missing
82+
Assert.AreEqual(1, dv2.Drives.Count);
7083
// Verify dv1 unchanged
7184
Assert.IsTrue(dv1.Drives.Contains(dr1));
7285
Assert.IsTrue(dv1.Drives.Contains(dr2));
7386

7487
// Verify dv2
7588
Assert.IsTrue(dv2.Drives.Contains(dr1));
7689
Assert.IsFalse(dv2.Drives.Contains(dr3));
77-
// Verify one null
78-
int nullCount = 0;
79-
for (int i = 0; i < dv2.Drives.Count; i++)
90+
91+
//Make sure that flush didn't touch not-found="ignore" records for not modified collection
92+
using (var s = Sfi.OpenSession())
93+
using (var t = s.BeginTransaction())
8094
{
81-
if (dv2.Drives[i] == null) nullCount++;
95+
dv2 = await (s.GetAsync<Device>(dv2.Id));
96+
await (s.FlushAsync());
97+
await (t.CommitAsync());
98+
}
99+
100+
await (VerifyResultAsync(expectedInCollection: 1, expectedInDb: 2, msg: "not modified collection"));
101+
102+
//Many-to-many clears collection and recreates it so not-found ignore records are lost
103+
using (var s = Sfi.OpenSession())
104+
using (var t = s.BeginTransaction())
105+
{
106+
dv2 = await (s.GetAsync<Device>(dv2.Id));
107+
dv2.Drives.Add(dr2);
108+
await (t.CommitAsync());
109+
}
110+
111+
await (VerifyResultAsync(2,2, msg: "modified collection"));
112+
113+
async Task VerifyResultAsync(int expectedInCollection, int expectedInDb, string msg)
114+
{
115+
using (var s = Sfi.OpenSession())
116+
{
117+
var realCound = Convert.ToInt32(
118+
await (s.CreateSQLQuery("select count(*) from DriveOfDevice where DeviceId = :id ")
119+
.SetParameter("id", dv2.Id)
120+
.UniqueResultAsync<object>()));
121+
dv2 = await (s.GetAsync<Device>(dv2.Id));
122+
123+
Assert.That(dv2.Drives.Count, Is.EqualTo(expectedInCollection), msg);
124+
Assert.That(realCound, Is.EqualTo(expectedInDb), msg);
125+
}
82126
}
83-
Assert.AreEqual(1, nullCount);
84127
}
85128
}
86-
}
129+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ public class Asset : Base
1414
{
1515
public virtual ISet<Document> Documents { get; set; } = new HashSet<Document>();
1616
public virtual ISet<Document> DocumentsFiltered { get; set; } = new HashSet<Document>();
17+
public virtual IList<Document> DocumentsBag { get; set; } = new List<Document>();
1718
}
1819

1920
public class Document : Base

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,23 @@ public void TestFilteredQueryOver()
106106
}
107107
}
108108

109+
[Test]
110+
public void TestFilteredBagQueryOver()
111+
{
112+
using (var s = OpenSession())
113+
{
114+
s.EnableFilter("deletedFilter").SetParameter("deletedParam", false);
115+
116+
var query = s.QueryOver<Asset>()
117+
.Fetch(SelectMode.Fetch, x => x.DocumentsBag)
118+
.TransformUsing(Transformers.DistinctRootEntity)
119+
.List<Asset>();
120+
121+
Assert.That(query.Count, Is.EqualTo(1), "filtered assets");
122+
Assert.That(query[0].DocumentsBag.Count, Is.EqualTo(1), "filtered asset documents");
123+
}
124+
}
125+
109126
//NH-2991
110127
[Test]
111128
public void TestQueryOverRestrictionWithClause()

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,12 @@
1818
<filter name="deletedFilter" condition="IsDeleted = :deletedParam"/>
1919
</many-to-many>
2020
</set>
21+
<bag name="DocumentsBag" table="asset_to_document" lazy="true" cascade="none">
22+
<key column="AssetId"/>
23+
<many-to-many class="Document" column="DocumentId">
24+
<filter name="deletedFilter" condition="IsDeleted = :deletedParam"/>
25+
</many-to-many>
26+
</bag>
2127
<set name="DocumentsFiltered" table="asset_to_document" lazy="true" cascade="none">
2228
<key column="AssetId"/>
2329
<many-to-many class="Document" column="DocumentId" where="IsDeleted = 0"/>

src/NHibernate.Test/NHSpecificTest/NH750/Fixture.cs

Lines changed: 50 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System;
2+
using NHibernate.Cfg;
23
using NUnit.Framework;
34

45
namespace NHibernate.Test.NHSpecificTest.NH750
@@ -16,6 +17,17 @@ protected override void OnTearDown()
1617
}
1718
}
1819

20+
protected override string CacheConcurrencyStrategy
21+
{
22+
get { return null; }
23+
}
24+
25+
protected override void Configure(Configuration configuration)
26+
{
27+
configuration.SetProperty(Cfg.Environment.UseSecondLevelCache, "false");
28+
base.Configure(configuration);
29+
}
30+
1931
[Test]
2032
public void DeviceOfDrive()
2133
{
@@ -55,21 +67,52 @@ public void DeviceOfDrive()
5567
dv2 = (Device) s.Load(typeof(Device), dvSavedId[1]);
5668
}
5769
Assert.AreEqual(2, dv1.Drives.Count);
58-
Assert.AreEqual(2, dv2.Drives.Count);
70+
// Verify one is missing
71+
Assert.AreEqual(1, dv2.Drives.Count);
5972
// Verify dv1 unchanged
6073
Assert.IsTrue(dv1.Drives.Contains(dr1));
6174
Assert.IsTrue(dv1.Drives.Contains(dr2));
6275

6376
// Verify dv2
6477
Assert.IsTrue(dv2.Drives.Contains(dr1));
6578
Assert.IsFalse(dv2.Drives.Contains(dr3));
66-
// Verify one null
67-
int nullCount = 0;
68-
for (int i = 0; i < dv2.Drives.Count; i++)
79+
80+
//Make sure that flush didn't touch not-found="ignore" records for not modified collection
81+
using (var s = Sfi.OpenSession())
82+
using (var t = s.BeginTransaction())
6983
{
70-
if (dv2.Drives[i] == null) nullCount++;
84+
dv2 = s.Get<Device>(dv2.Id);
85+
s.Flush();
86+
t.Commit();
87+
}
88+
89+
VerifyResult(expectedInCollection: 1, expectedInDb: 2, msg: "not modified collection");
90+
91+
//Many-to-many clears collection and recreates it so not-found ignore records are lost
92+
using (var s = Sfi.OpenSession())
93+
using (var t = s.BeginTransaction())
94+
{
95+
dv2 = s.Get<Device>(dv2.Id);
96+
dv2.Drives.Add(dr2);
97+
t.Commit();
98+
}
99+
100+
VerifyResult(2, 2, msg: "modified collection");
101+
102+
void VerifyResult(int expectedInCollection, int expectedInDb, string msg)
103+
{
104+
using (var s = Sfi.OpenSession())
105+
{
106+
var realCound = Convert.ToInt32(
107+
s.CreateSQLQuery("select count(*) from DriveOfDevice where DeviceId = :id ")
108+
.SetParameter("id", dv2.Id)
109+
.UniqueResult<object>());
110+
dv2 = s.Get<Device>(dv2.Id);
111+
112+
Assert.That(dv2.Drives.Count, Is.EqualTo(expectedInCollection), msg);
113+
Assert.That(realCound, Is.EqualTo(expectedInDb), msg);
114+
}
71115
}
72-
Assert.AreEqual(1, nullCount);
73116
}
74117
}
75-
}
118+
}

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

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -154,11 +154,9 @@ public override async Task<object> ReadFromAsync(DbDataReader reader, ICollectio
154154
// note that if we load this collection from a cartesian product
155155
// the multiplicity would be broken ... so use an idbag instead
156156
var element = await (role.ReadElementAsync(reader, owner, descriptor.SuffixedElementAliases, Session, cancellationToken)).ConfigureAwait(false);
157-
// NH Different behavior : we don't check for null
158-
// The NH-750 test show how checking for null we are ignoring the not-found tag and
159-
// the DB may have some records ignored by NH. This issue may need some more deep consideration.
160-
//if (element != null)
161-
_gbag.Add((T) element);
157+
158+
if (element != null)
159+
_gbag.Add((T) element);
162160
return element;
163161
}
164162
}

src/NHibernate/Collection/Generic/PersistentGenericBag.cs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -448,11 +448,9 @@ public override object ReadFrom(DbDataReader reader, ICollectionPersister role,
448448
// note that if we load this collection from a cartesian product
449449
// the multiplicity would be broken ... so use an idbag instead
450450
var element = role.ReadElement(reader, owner, descriptor.SuffixedElementAliases, Session);
451-
// NH Different behavior : we don't check for null
452-
// The NH-750 test show how checking for null we are ignoring the not-found tag and
453-
// the DB may have some records ignored by NH. This issue may need some more deep consideration.
454-
//if (element != null)
455-
_gbag.Add((T) element);
451+
452+
if (element != null)
453+
_gbag.Add((T) element);
456454
return element;
457455
}
458456

0 commit comments

Comments
 (0)