NH-3912 Batch operations with the same IBatcher instance fail on expected rows count after single failed operation.

Author: Maksim Sustretov

src/NHibernate.Test/NHSpecificTest/NH3912/BatcherLovingEntity.cs

@@ -0,0 +1,10 @@
+using System;
+
+namespace NHibernate.Test.NHSpecificTest.NH3912
+{
+	class BatcherLovingEntity
+	{
+		public virtual Guid Id { get; set; }
+		public virtual string Name { get; set; }
+	}
+}

src/NHibernate.Test/NHSpecificTest/NH3912/ReusableBatcherFixture.cs

@@ -0,0 +1,108 @@
+using System;
+using System.Linq;
+using NHibernate.AdoNet;
+using NHibernate.Cfg;
+using NHibernate.Cfg.MappingSchema;
+using NHibernate.Linq;
+using NHibernate.Mapping.ByCode;
+using NUnit.Framework;
+
+namespace NHibernate.Test.NHSpecificTest.NH3912
+{
+    /// <summary>
+    /// Fixture using 'by code' mappings
+    /// </summary>
+    /// <remarks>
+    /// This fixture is identical to <see cref="BatcherLovingEntity" /> except the <see cref="BatcherLovingEntity" /> mapping is performed
+    /// by code in the GetMappings method, and does not require the <c>Mappings.hbm.xml</c> file. Use this approach
+    /// if you prefer.
+    /// </remarks>
+    public class ReusableBatcherFixture : TestCaseMappingByCode
+	{
+		protected override HbmMapping GetMappings()
+		{
+			var mapper = new ModelMapper();
+			mapper.Class<BatcherLovingEntity>(rc =>
+			{
+				rc.Id(x => x.Id, m => m.Generator(Generators.GuidComb));
+				rc.Property(x => x.Name, m => m.Unique(true));
+			});
+
+			return mapper.CompileMappingForAllExplicitlyAddedEntities();
+		}
+
+		protected override void Configure(Configuration configuration)
+		{
+			base.Configure(configuration);
+			configuration.SetProperty(Cfg.Environment.BatchStrategy,
+									  typeof(OracleDataClientBatchingBatcherFactory).AssemblyQualifiedName);
+		}
+
+        protected override void OnSetUp()
+		{
+			using (ISession session = OpenSession())
+			using (ITransaction transaction = session.BeginTransaction())
+			{
+				var e1 = new BatcherLovingEntity { Name = "Bob" };
+				session.Save(e1);
+
+				var e2 = new BatcherLovingEntity { Name = "Sally" };
+				session.Save(e2);
+
+				session.Flush();
+				transaction.Commit();
+			}
+		}
+
+		protected override void OnTearDown()
+		{
+			using (ISession session = OpenSession())
+			using (ITransaction transaction = session.BeginTransaction())
+			{
+				session.Delete("from System.Object");
+
+				session.Flush();
+				transaction.Commit();
+			}
+		}
+
+        /// <summary> Batch operations with the same IBatcher instance fail on expect rows count after single failed operation.  </summary>
+		[Test]
+		public void Test_Batcher_Is_Reusable_After_Failed_Operation()
+		{
+		    using (ISession session = OpenSession())
+		    {
+		        try
+		        {
+                    using (session.BeginTransaction())
+                    {
+                        var valid = new BatcherLovingEntity { Name = "Bill" };
+                        session.Save(valid);
+
+                        Assert.That(() => session.Query<BatcherLovingEntity>().Count(x => x.Name == "Bob"), Is.EqualTo(1));
+                        var bob = new BatcherLovingEntity { Name = "Bob" };
+                        session.Save(bob);
+
+                        // Should fail on unique constraint violation
+                        // Expected behavior
+                        session.Flush();
+                    }
+		        }
+		        catch (Exception)
+		        {
+                    // Opening next transaction in the same session after rollback
+                    // to log the problem, for instance.
+                    // Executing in the same session with the same instance of IBatcher
+                    using (session.BeginTransaction())
+                    {
+                        // Inserting any valid entity will fail on expected rows count assert in batcher
+                        var e1 = new BatcherLovingEntity { Name = "Robert (because Bob already exists)" };
+                        session.Save(e1);
+                        // Batch update returned unexpected row count from update; actual row count: 1; expected: 2
+                        session.Flush();
+                    }
+		        }
+		    }
+		}
+	}
+}

src/NHibernate.Test/NHibernate.Test.csproj

@@ -723,6 +723,8 @@
     <Compile Include="NHSpecificTest\BagWithLazyExtraAndFilter\Fixture.cs" />
     <Compile Include="Linq\ByMethod\DistinctTests.cs" />
     <Compile Include="Component\Basic\ComponentWithUniqueConstraintTests.cs" />
+    <Compile Include="NHSpecificTest\NH3912\BatcherLovingEntity.cs" />
+    <Compile Include="NHSpecificTest\NH3912\ReusableBatcherFixture.cs" />
     <Compile Include="NHSpecificTest\NH3414\Entity.cs" />
     <Compile Include="NHSpecificTest\NH3414\FixtureByCode.cs" />
     <Compile Include="NHSpecificTest\NH2218\Fixture.cs" />

src/NHibernate/AdoNet/OracleDataClientBatchingBatcher.cs

@@ -125,21 +125,31 @@ protected override void DoExecuteBatch(IDbCommand ps)
 				// this value is not a part of the ADO.NET API.
 				// It's and ODP implementation, so it is being set by reflection
 				SetArrayBindCount(arraySize);
-				int rowsAffected;
-				try
-				{
-					rowsAffected = _currentBatch.ExecuteNonQuery();
-				}
-				catch (DbException e)
-				{
-					throw ADOExceptionHelper.Convert(Factory.SQLExceptionConverter, e, "could not execute batch command.");
-				}
-
-				Expectations.VerifyOutcomeBatched(_totalExpectedRowsAffected, rowsAffected);
-
-				_totalExpectedRowsAffected = 0;
-				_currentBatch = null;
-				_parameterValueListHashTable = null;
+			    try
+			    {
+                    int rowsAffected;
+                    try
+                    {
+                        rowsAffected = _currentBatch.ExecuteNonQuery();
+                    }
+                    catch (DbException e)
+                    {
+                        // Message from DbException could be useful
+                        var additionalMessage = string.IsNullOrEmpty(e.Message) ? "." : string.Concat(": ", e.Message);
+                        throw ADOExceptionHelper.Convert(
+                                        Factory.SQLExceptionConverter, e,
+                                        string.Format("could not execute batch command{0}", additionalMessage));
+                    }
+
+                    Expectations.VerifyOutcomeBatched(_totalExpectedRowsAffected, rowsAffected);
+			    }
+			    finally
+			    {
+                    // Cleaning up even if batched outcome is invalid
+                    _totalExpectedRowsAffected = 0;
+                    _currentBatch = null;
+                    _parameterValueListHashTable = null;
+			    }
 			}
 		}