Skip to content

Commit edf9cd9

Browse files
committed
Fix #3725: DbBatchBatcher empty batch execution bug
Add empty batch validation to DoExecuteBatch() and DoExecuteBatchAsync() to prevent InvalidOperationException when ExecuteBatch is called with no commands. This matches the pattern used in GenericBatchingBatcher.
1 parent e15822d commit edf9cd9

File tree

7 files changed

+356
-0
lines changed

7 files changed

+356
-0
lines changed
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
//------------------------------------------------------------------------------
2+
// <auto-generated>
3+
// This code was generated by AsyncGenerator.
4+
//
5+
// Changes to this file may cause incorrect behavior and will be lost if
6+
// the code is regenerated.
7+
// </auto-generated>
8+
//------------------------------------------------------------------------------
9+
10+
11+
using System.Linq;
12+
using NUnit.Framework;
13+
using NHibernate.Linq;
14+
15+
namespace NHibernate.Test.NHSpecificTest.GH3725
16+
{
17+
using System.Threading.Tasks;
18+
[TestFixture]
19+
public class FixtureAsync : BugTestCase
20+
{
21+
protected override void OnSetUp()
22+
{
23+
using var session = OpenSession();
24+
using var transaction = session.BeginTransaction();
25+
26+
var e1 = new Entity { Name = "Test" };
27+
session.Save(e1);
28+
29+
transaction.Commit();
30+
}
31+
32+
protected override void OnTearDown()
33+
{
34+
using var session = OpenSession();
35+
using var transaction = session.BeginTransaction();
36+
37+
session.CreateQuery("delete from System.Object").ExecuteUpdate();
38+
39+
transaction.Commit();
40+
}
41+
42+
[Test]
43+
public async Task EmptyBatchShouldNotThrowExceptionAsync()
44+
{
45+
// This test verifies that DbBatchBatcher handles empty batches correctly
46+
// The bug occurred when ExecuteBatch was called on an empty batch,
47+
// causing InvalidOperationException: CommandText property has not been initialized
48+
49+
using var session = OpenSession();
50+
using var transaction = session.BeginTransaction();
51+
52+
// Load an entity - this prepares a SELECT command
53+
var entity = await (session.Query<Entity>().FirstOrDefaultAsync());
54+
Assert.That(entity, Is.Not.Null);
55+
56+
// Now prepare a batch command which triggers ExecuteBatch on any existing batch
57+
// If the previous command didn't add anything to the batch, this would fail
58+
// before the fix with InvalidOperationException
59+
var entity2 = await (session.Query<Entity>().FirstOrDefaultAsync());
60+
Assert.That(entity2, Is.Not.Null);
61+
62+
await (transaction.CommitAsync());
63+
}
64+
65+
[Test]
66+
public async Task FlushEmptyBatchShouldNotThrowExceptionAsync()
67+
{
68+
// Another scenario where empty batch execution can occur
69+
using var session = OpenSession();
70+
using var transaction = session.BeginTransaction();
71+
72+
// Query without any modifications
73+
var count = await (session.Query<Entity>().CountAsync());
74+
Assert.That(count, Is.EqualTo(1));
75+
76+
// Flush with no pending batch operations should not throw
77+
Assert.DoesNotThrowAsync(() => session.FlushAsync());
78+
79+
await (transaction.CommitAsync());
80+
}
81+
}
82+
}
Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
//------------------------------------------------------------------------------
2+
// <auto-generated>
3+
// This code was generated by AsyncGenerator.
4+
//
5+
// Changes to this file may cause incorrect behavior and will be lost if
6+
// the code is regenerated.
7+
// </auto-generated>
8+
//------------------------------------------------------------------------------
9+
10+
11+
using System.Linq;
12+
using NHibernate.Cfg.MappingSchema;
13+
using NHibernate.Mapping.ByCode;
14+
using NUnit.Framework;
15+
using NHibernate.Linq;
16+
17+
namespace NHibernate.Test.NHSpecificTest.GH3725
18+
{
19+
using System.Threading.Tasks;
20+
/// <summary>
21+
/// Fixture using 'by code' mappings
22+
/// </summary>
23+
/// <remarks>
24+
/// This fixture is identical to <see cref="FixtureAsync" /> except the <see cref="Entity" /> mapping is performed
25+
/// by code in the GetMappings method, and does not require the <c>Mappings.hbm.xml</c> file.
26+
/// </remarks>
27+
[TestFixture]
28+
public class FixtureByCodeAsync : TestCaseMappingByCode
29+
{
30+
protected override HbmMapping GetMappings()
31+
{
32+
var mapper = new ModelMapper();
33+
mapper.Class<Entity>(rc =>
34+
{
35+
rc.Id(x => x.Id, m => m.Generator(Generators.GuidComb));
36+
rc.Property(x => x.Name);
37+
});
38+
39+
return mapper.CompileMappingForAllExplicitlyAddedEntities();
40+
}
41+
42+
protected override void OnSetUp()
43+
{
44+
using var session = OpenSession();
45+
using var transaction = session.BeginTransaction();
46+
47+
var e1 = new Entity { Name = "Test" };
48+
session.Save(e1);
49+
50+
transaction.Commit();
51+
}
52+
53+
protected override void OnTearDown()
54+
{
55+
using var session = OpenSession();
56+
using var transaction = session.BeginTransaction();
57+
58+
session.CreateQuery("delete from System.Object").ExecuteUpdate();
59+
60+
transaction.Commit();
61+
}
62+
63+
[Test]
64+
public async Task EmptyBatchShouldNotThrowExceptionAsync()
65+
{
66+
using var session = OpenSession();
67+
using var transaction = session.BeginTransaction();
68+
69+
var entity = await (session.Query<Entity>().FirstOrDefaultAsync());
70+
Assert.That(entity, Is.Not.Null);
71+
72+
var entity2 = await (session.Query<Entity>().FirstOrDefaultAsync());
73+
Assert.That(entity2, Is.Not.Null);
74+
75+
await (transaction.CommitAsync());
76+
}
77+
78+
[Test]
79+
public async Task FlushEmptyBatchShouldNotThrowExceptionAsync()
80+
{
81+
using var session = OpenSession();
82+
using var transaction = session.BeginTransaction();
83+
84+
var count = await (session.Query<Entity>().CountAsync());
85+
Assert.That(count, Is.EqualTo(1));
86+
87+
Assert.DoesNotThrowAsync(() => session.FlushAsync());
88+
89+
await (transaction.CommitAsync());
90+
}
91+
}
92+
}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
using System;
2+
3+
namespace NHibernate.Test.NHSpecificTest.GH3725
4+
{
5+
class Entity
6+
{
7+
public virtual Guid Id { get; set; }
8+
public virtual string Name { get; set; }
9+
}
10+
}
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
using System.Linq;
2+
using NUnit.Framework;
3+
4+
namespace NHibernate.Test.NHSpecificTest.GH3725
5+
{
6+
[TestFixture]
7+
public class Fixture : BugTestCase
8+
{
9+
protected override void OnSetUp()
10+
{
11+
using var session = OpenSession();
12+
using var transaction = session.BeginTransaction();
13+
14+
var e1 = new Entity { Name = "Test" };
15+
session.Save(e1);
16+
17+
transaction.Commit();
18+
}
19+
20+
protected override void OnTearDown()
21+
{
22+
using var session = OpenSession();
23+
using var transaction = session.BeginTransaction();
24+
25+
session.CreateQuery("delete from System.Object").ExecuteUpdate();
26+
27+
transaction.Commit();
28+
}
29+
30+
[Test]
31+
public void EmptyBatchShouldNotThrowException()
32+
{
33+
// This test verifies that DbBatchBatcher handles empty batches correctly
34+
// The bug occurred when ExecuteBatch was called on an empty batch,
35+
// causing InvalidOperationException: CommandText property has not been initialized
36+
37+
using var session = OpenSession();
38+
using var transaction = session.BeginTransaction();
39+
40+
// Load an entity - this prepares a SELECT command
41+
var entity = session.Query<Entity>().FirstOrDefault();
42+
Assert.That(entity, Is.Not.Null);
43+
44+
// Now prepare a batch command which triggers ExecuteBatch on any existing batch
45+
// If the previous command didn't add anything to the batch, this would fail
46+
// before the fix with InvalidOperationException
47+
var entity2 = session.Query<Entity>().FirstOrDefault();
48+
Assert.That(entity2, Is.Not.Null);
49+
50+
transaction.Commit();
51+
}
52+
53+
[Test]
54+
public void FlushEmptyBatchShouldNotThrowException()
55+
{
56+
// Another scenario where empty batch execution can occur
57+
using var session = OpenSession();
58+
using var transaction = session.BeginTransaction();
59+
60+
// Query without any modifications
61+
var count = session.Query<Entity>().Count();
62+
Assert.That(count, Is.EqualTo(1));
63+
64+
// Flush with no pending batch operations should not throw
65+
Assert.DoesNotThrow(() => session.Flush());
66+
67+
transaction.Commit();
68+
}
69+
}
70+
}
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
using System.Linq;
2+
using NHibernate.Cfg.MappingSchema;
3+
using NHibernate.Mapping.ByCode;
4+
using NUnit.Framework;
5+
6+
namespace NHibernate.Test.NHSpecificTest.GH3725
7+
{
8+
/// <summary>
9+
/// Fixture using 'by code' mappings
10+
/// </summary>
11+
/// <remarks>
12+
/// This fixture is identical to <see cref="Fixture" /> except the <see cref="Entity" /> mapping is performed
13+
/// by code in the GetMappings method, and does not require the <c>Mappings.hbm.xml</c> file.
14+
/// </remarks>
15+
[TestFixture]
16+
public class FixtureByCode : TestCaseMappingByCode
17+
{
18+
protected override HbmMapping GetMappings()
19+
{
20+
var mapper = new ModelMapper();
21+
mapper.Class<Entity>(rc =>
22+
{
23+
rc.Id(x => x.Id, m => m.Generator(Generators.GuidComb));
24+
rc.Property(x => x.Name);
25+
});
26+
27+
return mapper.CompileMappingForAllExplicitlyAddedEntities();
28+
}
29+
30+
protected override void OnSetUp()
31+
{
32+
using var session = OpenSession();
33+
using var transaction = session.BeginTransaction();
34+
35+
var e1 = new Entity { Name = "Test" };
36+
session.Save(e1);
37+
38+
transaction.Commit();
39+
}
40+
41+
protected override void OnTearDown()
42+
{
43+
using var session = OpenSession();
44+
using var transaction = session.BeginTransaction();
45+
46+
session.CreateQuery("delete from System.Object").ExecuteUpdate();
47+
48+
transaction.Commit();
49+
}
50+
51+
[Test]
52+
public void EmptyBatchShouldNotThrowException()
53+
{
54+
using var session = OpenSession();
55+
using var transaction = session.BeginTransaction();
56+
57+
var entity = session.Query<Entity>().FirstOrDefault();
58+
Assert.That(entity, Is.Not.Null);
59+
60+
var entity2 = session.Query<Entity>().FirstOrDefault();
61+
Assert.That(entity2, Is.Not.Null);
62+
63+
transaction.Commit();
64+
}
65+
66+
[Test]
67+
public void FlushEmptyBatchShouldNotThrowException()
68+
{
69+
using var session = OpenSession();
70+
using var transaction = session.BeginTransaction();
71+
72+
var count = session.Query<Entity>().Count();
73+
Assert.That(count, Is.EqualTo(1));
74+
75+
Assert.DoesNotThrow(() => session.Flush());
76+
77+
transaction.Commit();
78+
}
79+
}
80+
}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?xml version="1.0" encoding="utf-8" ?>
2+
<hibernate-mapping xmlns="urn:nhibernate-mapping-2.2" assembly="NHibernate.Test"
3+
namespace="NHibernate.Test.NHSpecificTest.GH3725">
4+
5+
<class name="Entity">
6+
<id name="Id" generator="guid.comb"/>
7+
<property name="Name"/>
8+
</class>
9+
10+
</hibernate-mapping>

src/NHibernate/AdoNet/DbBatchBatcher.cs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,12 @@ public override Task AddToBatchAsync(IExpectation expectation, CancellationToken
115115

116116
protected override void DoExecuteBatch(DbCommand ps)
117117
{
118+
if (_currentBatch.BatchCommands.Count == 0)
119+
{
120+
Expectations.VerifyOutcomeBatched(_totalExpectedRowsAffected, 0, ps);
121+
return;
122+
}
123+
118124
try
119125
{
120126
Log.Debug("Executing batch");
@@ -145,6 +151,12 @@ protected override void DoExecuteBatch(DbCommand ps)
145151
protected override async Task DoExecuteBatchAsync(DbCommand ps, CancellationToken cancellationToken)
146152
{
147153
cancellationToken.ThrowIfCancellationRequested();
154+
if (_currentBatch.BatchCommands.Count == 0)
155+
{
156+
Expectations.VerifyOutcomeBatched(_totalExpectedRowsAffected, 0, ps);
157+
return;
158+
}
159+
148160
try
149161
{
150162
Log.Debug("Executing batch");

0 commit comments

Comments
 (0)