Merge pull request #513 from waynesayonara/NH3912

NH-3912 Oracle IBatcher impl instance is not reusable after failed operation.

Author: oskarb

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,110 @@
+using System;
+using System.Linq;
+using NHibernate.AdoNet;
+using NHibernate.Cfg;
+using NHibernate.Cfg.MappingSchema;
+using NHibernate.Driver;
+using NHibernate.Engine;
+using NHibernate.Linq;
+using NHibernate.Mapping.ByCode;
+using NUnit.Framework;
+
+namespace NHibernate.Test.NHSpecificTest.NH3912
+{
+	public class ReusableBatcherFixture : TestCaseMappingByCode
+	{
+		protected override bool AppliesTo(ISessionFactoryImplementor factory)
+		{
+			var driver = factory.ConnectionProvider.Driver;
+			return driver is OracleDataClientDriver ||
+			       driver is OracleLiteDataClientDriver ||
+			       driver is OracleManagedDataClientDriver;
+		}
+
+		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 (var session = OpenSession())
+			using (var 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 (var session = OpenSession())
+			using (var 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 (var 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

@@ -733,6 +733,8 @@
     <Compile Include="NHSpecificTest\EntityWithUserTypeCanHaveLinqGenerators\IExample.cs" />
     <Compile Include="NHSpecificTest\NH2204\Model.cs" />
     <Compile Include="NHSpecificTest\NH2204\Fixture.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,27 @@ 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();
+					int rowsAffected;
+					try
+					{
+						rowsAffected = _currentBatch.ExecuteNonQuery();
+					}
+					catch (DbException e)
+					{
+						throw ADOExceptionHelper.Convert(Factory.SQLExceptionConverter, e, "could not execute batch command.");
+					}
+
+					Expectations.VerifyOutcomeBatched(_totalExpectedRowsAffected, rowsAffected);
 				}
-				catch (DbException e)
+				finally
 				{
-					throw ADOExceptionHelper.Convert(Factory.SQLExceptionConverter, e, "could not execute batch command.");
+					// Cleaning up even if batched outcome is invalid
+					_totalExpectedRowsAffected = 0;
+					_currentBatch = null;
+					_parameterValueListHashTable = null;
 				}
-
-				Expectations.VerifyOutcomeBatched(_totalExpectedRowsAffected, rowsAffected);
-
-				_totalExpectedRowsAffected = 0;
-				_currentBatch = null;
-				_parameterValueListHashTable = null;
 			}
 		}