diff --git a/src/AdoNetCore.AseClient/AseTransaction.cs b/src/AdoNetCore.AseClient/AseTransaction.cs index e01fa86..fd43483 100644 --- a/src/AdoNetCore.AseClient/AseTransaction.cs +++ b/src/AdoNetCore.AseClient/AseTransaction.cs @@ -141,16 +141,21 @@ public override void Commit() /// protected override void Dispose(bool disposing) { - base.Dispose(disposing); - if (_isDisposed) { return; } - Rollback(); + if (disposing) + { + _connection?.Dispose(); + } + Rollback(); _isDisposed = true; + + base.Dispose(disposing); + } internal bool IsDisposed => _isDisposed; @@ -165,7 +170,7 @@ public override void Rollback() throw new ObjectDisposedException(nameof(AseTransaction)); } - if (_complete) + if (_complete || _connection.State == ConnectionState.Closed || _connection.State == ConnectionState.Broken) { return; } diff --git a/test/AdoNetCore.AseClient.Tests/Unit/AseTransactionTests.cs b/test/AdoNetCore.AseClient.Tests/Unit/AseTransactionTests.cs index 6f9df0c..9197f22 100644 --- a/test/AdoNetCore.AseClient.Tests/Unit/AseTransactionTests.cs +++ b/test/AdoNetCore.AseClient.Tests/Unit/AseTransactionTests.cs @@ -1,4 +1,5 @@ -using System.Data; +using System; +using System.Data; using Moq; using NUnit.Framework; @@ -124,8 +125,8 @@ public void ExplicitRollback_WithValidTransaction_InteractsWithTheDbCommandCorre var t = new AseTransaction(mockConnection.Object, isolationLevel); t.Begin(); return t; - }); - + }); + mockConnection.Setup(x => x.State).Returns(() => { return ConnectionState.Open; }); mockConnection .SetupSequence(x => x.CreateCommand()) .Returns(mockCommandIsolationLevel.Object) @@ -193,6 +194,7 @@ public void ImplicitRollback_WithValidTransaction_InteractsWithTheDbCommandCorre return t; }); + mockConnection.Setup(x => x.State).Returns(() => { return ConnectionState.Open; }); mockConnection .SetupSequence(x => x.CreateCommand()) .Returns(mockCommandIsolationLevel.Object) @@ -222,16 +224,73 @@ public void ImplicitRollback_WithValidTransaction_InteractsWithTheDbCommandCorre mockCommandRollbackTransaction.Verify(); } + [Test] + public void ImplicitRollback_WithBrokenConnection_DoesNotThrowExceptionDuringDispose() + { + // Arrange + var mockConnection = new Mock(); + var isolationLevel = IsolationLevel.Serializable; + + var mockCommandIsolationLevel = new Mock(); + var mockCommandBeginTransaction = new Mock(); + var mockCommandRollbackTransaction = new Mock(); + + mockCommandIsolationLevel + .SetupAllProperties() + .Setup(x => x.ExecuteNonQuery()) + .Returns(0); + + mockCommandBeginTransaction + .SetupAllProperties() + .Setup(x => x.ExecuteNonQuery()) + .Returns(0); + + mockCommandRollbackTransaction + .SetupAllProperties() + .Setup(x => x.ExecuteNonQuery()) + .Throws( new InvalidOperationException("Cannot execute on a connection which is not open")); + + + mockConnection.Setup(x => x.State).Returns(() => { return ConnectionState.Broken; }); + mockConnection + .Setup(x => x.BeginTransaction(isolationLevel)) + .Returns(() => + { + // Simulate what AseConnection.BeginTransaction() does. + var t = new AseTransaction(mockConnection.Object, isolationLevel); + t.Begin(); + return t; + }); + + mockConnection + .SetupSequence(x => x.CreateCommand()) + .Returns(mockCommandIsolationLevel.Object) + .Returns(mockCommandBeginTransaction.Object) + .Returns(mockCommandRollbackTransaction.Object); + + + // Act + var connection = mockConnection.Object; + var transaction = connection.BeginTransaction(isolationLevel); + + Assert.DoesNotThrow(() => { transaction.Dispose(); }); + // Implicit rollback + } + + + [Test] public void RepeatedDisposal_DoesNotThrow() { // Arrange var mockConnection = new Mock(); + var isolationLevel = IsolationLevel.Serializable; var mockCommandIsolationLevel = new Mock(); var mockCommandBeginTransaction = new Mock(); var mockCommandRollbackTransaction = new Mock(); + mockCommandIsolationLevel .SetupAllProperties() @@ -271,6 +330,16 @@ public void RepeatedDisposal_DoesNotThrow() transaction.Dispose(); // Implicit rollback transaction.Dispose(); // Should do nothing + + mockConnection.Verify(t => t.Dispose(), Times.Once, "we called multiple time the dispose on the master"); + + //mockAseTransaction.Object. + //we tried to make this work but cannot work it as long as the class is sealed and it's sealed for performance constraint. + //if you want to verify it still, use var mockAseTransaction = new Mock(MockBehavior.Loose,mockConnection.Object, isolationLevel) { CallBase = true } + //mockAseTransaction.Verify(t => t.Rollback(), Times.Once, "we should have only called once the rollback"); + + + } } }