From 974ed50c14dbb899d5332354aa3f628269c4fd0c Mon Sep 17 00:00:00 2001 From: Kirill Kurdyukov Date: Wed, 25 Sep 2024 13:15:27 +0300 Subject: [PATCH] feat: an error happened in the transaction, allow one empty rollback (#190) --- CHANGELOG.md | 2 ++ src/Ydb.Sdk/src/Ado/YdbCommand.cs | 18 ++---------- src/Ydb.Sdk/src/Ado/YdbDataReader.cs | 2 +- src/Ydb.Sdk/src/Ado/YdbTransaction.cs | 29 +++++++++++++++++--- src/Ydb.Sdk/tests/Ado/YdbExceptionTests.cs | 10 ++++++- src/Ydb.Sdk/tests/Ado/YdbTransactionTests.cs | 25 +++++++++++++++-- 6 files changed, 62 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 001b7368..dfacab6a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,5 @@ +- If an error happened in the transaction, allow one empty rollback + ## v0.7.0 - Parsed @param then prepared for use with $ prefix (@p -> $p) - Fully integrated with Dapper diff --git a/src/Ydb.Sdk/src/Ado/YdbCommand.cs b/src/Ydb.Sdk/src/Ado/YdbCommand.cs index bbdbea9b..5feecda3 100644 --- a/src/Ydb.Sdk/src/Ado/YdbCommand.cs +++ b/src/Ydb.Sdk/src/Ado/YdbCommand.cs @@ -122,7 +122,7 @@ protected override DbTransaction? DbTransaction { if (value is null or YdbTransaction) { - _ydbTransaction = (YdbTransaction?)value; + Transaction = (YdbTransaction?)value; } else { @@ -132,21 +132,7 @@ protected override DbTransaction? DbTransaction } } - public new YdbTransaction? Transaction - { - get - { - if (_ydbTransaction?.Completed ?? false) - { - _ydbTransaction = null; - } - - return _ydbTransaction; - } - set => _ydbTransaction = value; - } - - private YdbTransaction? _ydbTransaction; + public new YdbTransaction? Transaction { get; set; } public override bool DesignTimeVisible { get; set; } diff --git a/src/Ydb.Sdk/src/Ado/YdbDataReader.cs b/src/Ydb.Sdk/src/Ado/YdbDataReader.cs index 5fc75321..f343e264 100644 --- a/src/Ydb.Sdk/src/Ado/YdbDataReader.cs +++ b/src/Ydb.Sdk/src/Ado/YdbDataReader.cs @@ -519,7 +519,7 @@ private void OnFailReadStream() ReaderState = State.Closed; if (_ydbTransaction != null) { - _ydbTransaction.Completed = true; + _ydbTransaction.Failed = true; } } diff --git a/src/Ydb.Sdk/src/Ado/YdbTransaction.cs b/src/Ydb.Sdk/src/Ado/YdbTransaction.cs index 3bc92c94..1d886355 100644 --- a/src/Ydb.Sdk/src/Ado/YdbTransaction.cs +++ b/src/Ydb.Sdk/src/Ado/YdbTransaction.cs @@ -9,12 +9,26 @@ public sealed class YdbTransaction : DbTransaction { private readonly TxMode _txMode; + private bool _failed; + internal string? TxId { get; set; } - internal bool Completed { get; set; } + internal bool Completed { get; private set; } + + internal bool Failed + { + private get => _failed; + set + { + _failed = value; + Completed = true; + } + } - internal TransactionControl TransactionControl => TxId == null - ? new TransactionControl { BeginTx = _txMode.TransactionSettings() } - : new TransactionControl { TxId = TxId }; + internal TransactionControl? TransactionControl => Completed + ? null + : TxId == null + ? new TransactionControl { BeginTx = _txMode.TransactionSettings() } + : new TransactionControl { TxId = TxId }; internal YdbTransaction(YdbConnection ydbConnection, TxMode txMode) { @@ -41,6 +55,13 @@ public override void Rollback() // TODO propagate cancellation token public override async Task RollbackAsync(CancellationToken cancellationToken = new()) { + if (Failed) + { + Failed = false; + + return; + } + await FinishTransaction(txId => DbConnection.Session.RollbackTransaction(txId)); } diff --git a/src/Ydb.Sdk/tests/Ado/YdbExceptionTests.cs b/src/Ydb.Sdk/tests/Ado/YdbExceptionTests.cs index 2f50c5e6..dff8f04c 100644 --- a/src/Ydb.Sdk/tests/Ado/YdbExceptionTests.cs +++ b/src/Ydb.Sdk/tests/Ado/YdbExceptionTests.cs @@ -21,7 +21,7 @@ public async Task IsTransient_WhenSchemaError_ReturnFalse() } [Fact] - public async Task IsTransient_WhenAborted_ReturnTrue() + public async Task IsTransient_WhenAborted_ReturnTrueAndMakeEmptyRollback() { var bankTable = $"Bank_{Utils.Net}"; await using var ydbConnection = new YdbConnection(); @@ -57,5 +57,13 @@ public async Task IsTransient_WhenAborted_ReturnTrue() { CommandText = $"DROP TABLE {bankTable}" }.ExecuteNonQueryAsync(); + Assert.Equal("This YdbTransaction has completed; it is no longer usable", + Assert.Throws(() => ydbCommand.Transaction.Commit()).Message); + + await ydbCommand.Transaction!.RollbackAsync(); + Assert.Equal("This YdbTransaction has completed; it is no longer usable", + Assert.Throws(() => ydbCommand.Transaction!.Commit()).Message); + Assert.Equal("This YdbTransaction has completed; it is no longer usable", + Assert.Throws(() => ydbCommand.Transaction!.Rollback()).Message); } } diff --git a/src/Ydb.Sdk/tests/Ado/YdbTransactionTests.cs b/src/Ydb.Sdk/tests/Ado/YdbTransactionTests.cs index 400f46d6..08aed205 100644 --- a/src/Ydb.Sdk/tests/Ado/YdbTransactionTests.cs +++ b/src/Ydb.Sdk/tests/Ado/YdbTransactionTests.cs @@ -97,7 +97,7 @@ public void Commit_WhenEmptyYdbCommand_DoNothing() } [Fact] - public void Commit_WhenDoubleCommit_ThrowException() + public void CommitAndRollback_WhenDoubleCommit_ThrowException() { using var connection = new YdbConnection(); connection.Open(); @@ -165,7 +165,7 @@ public void CommitAndRollback_WhenConnectionIsClosed_ThrowException() } [Fact] - public void Commit_WhenConnectionIsClosedAndTxDoesNotStarted_ThrowException() + public void CommitAndRollback_WhenConnectionIsClosedAndTxDoesNotStarted_ThrowException() { using var connection = new YdbConnection(); connection.Open(); @@ -179,6 +179,27 @@ public void Commit_WhenConnectionIsClosedAndTxDoesNotStarted_ThrowException() Assert.Throws(() => ydbTransaction.Rollback()).Message); } + [Fact] + public void CommitAndRollback_WhenTransactionIsFailed_ThrowException() + { + using var connection = new YdbConnection(); + connection.Open(); + + var ydbCommand = connection.CreateCommand(); + ydbCommand.Transaction = connection.BeginTransaction(); + ydbCommand.Transaction.Failed = true; + ydbCommand.Transaction.TxId = "no_tx"; + + Assert.Equal("This YdbTransaction has completed; it is no longer usable", + Assert.Throws(() => ydbCommand.Transaction.Commit()).Message); + + ydbCommand.Transaction.Rollback(); // Make completed + Assert.Equal("This YdbTransaction has completed; it is no longer usable", + Assert.Throws(() => ydbCommand.Transaction.Commit()).Message); + Assert.Equal("This YdbTransaction has completed; it is no longer usable", + Assert.Throws(() => ydbCommand.Transaction.Rollback()).Message); + } + public async Task InitializeAsync() { await using var connection = new YdbConnection();