NH-4013 - Test case and fix for SQL-Server batcher failure after CloseCommands.

Author: fredericDelaporte

src/NHibernate.Test/ConnectionTest/BatcherFixture.cs

@@ -0,0 +1,45 @@
+using System;
+using System.Collections;
+using System.Collections.Generic;
+using NHibernate.Cfg;
+using NHibernate.Util;
+using NUnit.Framework;
+using Environment = NHibernate.Cfg.Environment;
+
+namespace NHibernate.Test.ConnectionTest
+{
+	[TestFixture]
+	public class BatcherFixture : ConnectionManagementTestCase
+	{
+		protected override void Configure(Configuration config)
+		{
+			base.Configure(config);
+			config.SetProperty(Environment.BatchSize, "10");
+		}
+
+		protected override ISession GetSessionUnderTest()
+			=> OpenSession();
+
+		protected override void OnTearDown()
+		{
+			using (var s = OpenSession())
+			{
+				s.CreateQuery("delete from System.Object").ExecuteUpdate();
+			}
+		}
+
+		[Test]
+		public void CanCloseCommandsAndUseBatcher()
+		{
+			using (var s = OpenSession())
+			{
+				// Need a generator strategy not causing insert at save.
+				var silly = new YetAnother { Name = "Silly" };
+				s.Save(silly);
+				s.GetSessionImplementation().ConnectionManager.Batcher.CloseCommands();
+				
+				Assert.DoesNotThrow(s.Flush, "Flush failure after closing commands.");
+			}
+		}
+	}
+}

src/NHibernate.Test/ConnectionTest/Silly.hbm.xml

@@ -16,4 +16,11 @@
 		</id>
 		<property name="Name"/>
 	</class>
+
+  <class name="YetAnother">
+    <id name="Id" type="long">
+      <generator class="hilo"/>
+    </id>
+    <property name="Name"/>
+  </class>
 </hibernate-mapping>

src/NHibernate.Test/ConnectionTest/YetAnother.cs

@@ -0,0 +1,12 @@
+using System;
+
+namespace NHibernate.Test.ConnectionTest
+{
+	[Serializable]
+	public class YetAnother
+	{
+		public virtual long Id { get; set; }
+
+		public virtual string Name { get; set; }
+	}
+}

src/NHibernate.Test/NHibernate.Test.csproj

@@ -202,8 +202,10 @@
     <Compile Include="CompositeId\Order.cs" />
     <Compile Include="CompositeId\Product.cs" />
     <Compile Include="ConnectionStringTest\NamedConnectionStringFixture.cs" />
+    <Compile Include="ConnectionTest\BatcherFixture.cs" />
     <Compile Include="ConnectionTest\AggressiveReleaseTest.cs" />
     <Compile Include="ConnectionTest\ConnectionManagementTestCase.cs" />
+    <Compile Include="ConnectionTest\YetAnother.cs" />
     <Compile Include="ConnectionTest\Other.cs" />
     <Compile Include="ConnectionTest\Silly.cs" />
     <Compile Include="ConnectionTest\MapBasedSessionContextFixture.cs" />

src/NHibernate/AdoNet/MySqlClientBatchingBatcher.cs

@@ -74,7 +74,6 @@ protected override void DoExecuteBatch(DbCommand ps)
 			if (Factory.Settings.SqlStatementLogger.IsDebugEnabled)
 			{
 				Factory.Settings.SqlStatementLogger.LogBatchCommand(currentBatchCommandsLog.ToString());
-				currentBatchCommandsLog = new StringBuilder().AppendLine("Batch commands:");
 			}
 
 			int rowsAffected;
@@ -89,28 +88,53 @@ protected override void DoExecuteBatch(DbCommand ps)
 
 			Expectations.VerifyOutcomeBatched(totalExpectedRowsAffected, rowsAffected);
 
-			currentBatch.Dispose();
-			totalExpectedRowsAffected = 0;
-			currentBatch = CreateConfiguredBatch();
+			ClearCurrentBatch();
 		}
 
 		private MySqlClientSqlCommandSet CreateConfiguredBatch()
 		{
 			return new MySqlClientSqlCommandSet(batchSize);
 		}
 
+		private void ClearCurrentBatch()
+		{
+			currentBatch.Dispose();
+			totalExpectedRowsAffected = 0;
+			currentBatch = CreateConfiguredBatch();
+
+			if (Factory.Settings.SqlStatementLogger.IsDebugEnabled)
+			{
+				currentBatchCommandsLog = new StringBuilder().AppendLine("Batch commands:");
+			}
+		}
+
 		public override void CloseCommands()
 		{
 			base.CloseCommands();
 
 			try
 			{
-				currentBatch.Dispose();
+				ClearCurrentBatch();
 			}
 			catch (Exception e)
 			{
-				// Prevent exceptions when closing the batch from hiding any original exception
+				// Prevent exceptions when clearing the batch from hiding any original exception
 				// (We do not know here if this batch closing occurs after a failure or not.)
+				Log.Warn("Exception clearing batch", e);
+			}
+		}
+
+		protected override void Dispose(bool isDisposing)
+		{
+			base.Dispose(isDisposing);
+			// Prevent exceptions when closing the batch from hiding any original exception
+			// (We do not know here if this batch closing occurs after a failure or not.)
+			try
+			{
+				currentBatch.Dispose();
+			}
+			catch (Exception e)
+			{
 				Log.Warn("Exception closing batcher", e);
 			}
 		}

src/NHibernate/AdoNet/SqlClientBatchingBatcher.cs

@@ -77,7 +77,6 @@ protected override void DoExecuteBatch(DbCommand ps)
 			if (Factory.Settings.SqlStatementLogger.IsDebugEnabled)
 			{
 				Factory.Settings.SqlStatementLogger.LogBatchCommand(_currentBatchCommandsLog.ToString());
-				_currentBatchCommandsLog = new StringBuilder().AppendLine("Batch commands:");
 			}
 
 			int rowsAffected;
@@ -92,9 +91,7 @@ protected override void DoExecuteBatch(DbCommand ps)
 
 			Expectations.VerifyOutcomeBatched(_totalExpectedRowsAffected, rowsAffected);
 
-			_currentBatch.Dispose();
-			_totalExpectedRowsAffected = 0;
-			_currentBatch = CreateConfiguredBatch();
+			ClearCurrentBatch();
 		}
 
 		private SqlClientSqlCommandSet CreateConfiguredBatch()
@@ -118,18 +115,45 @@ private SqlClientSqlCommandSet CreateConfiguredBatch()
 			return result;
 		}
 
+		private void ClearCurrentBatch()
+		{
+			_currentBatch.Dispose();
+			_totalExpectedRowsAffected = 0;
+			_currentBatch = CreateConfiguredBatch();
+
+			if (Factory.Settings.SqlStatementLogger.IsDebugEnabled)
+			{
+				_currentBatchCommandsLog = new StringBuilder().AppendLine("Batch commands:");
+			}
+		}
+
 		public override void CloseCommands()
 		{
 			base.CloseCommands();
 
+			// Prevent exceptions when closing the batch from hiding any original exception
+			// (We do not know here if this batch closing occurs after a failure or not.)
+			try
+			{
+				ClearCurrentBatch();
+			}
+			catch (Exception e)
+			{
+				Log.Warn("Exception clearing batch", e);
+			}
+		}
+
+		protected override void Dispose(bool isDisposing)
+		{
+			base.Dispose(isDisposing);
+			// Prevent exceptions when closing the batch from hiding any original exception
+			// (We do not know here if this batch closing occurs after a failure or not.)
 			try
 			{
 				_currentBatch.Dispose();
 			}
 			catch (Exception e)
 			{
-				// Prevent exceptions when closing the batch from hiding any original exception
-				// (We do not know here if this batch closing occurs after a failure or not.)
 				Log.Warn("Exception closing batcher", e);
 			}
 		}