From 9e78d7e6775bfdc03710149f154c59b83142de1c Mon Sep 17 00:00:00 2001 From: Roman Marusyk Date: Mon, 18 May 2020 23:41:29 +0300 Subject: [PATCH] Use existing transaction identifier in RelationalConnection.UseTransaction (#20844) * Use existing transaction identifier when UseTransaction of the RelationalConnection i called. #20828 * Add M prefix to XML comment to fix CS0419 * Implement interfaces in tests. Add M prefix to XML comment to fix CS0419 * Add [NotNull] attribute --- .../Diagnostics/DbTransactionInterceptor.cs | 12 ++--- .../Diagnostics/IDbTransactionInterceptor.cs | 12 ++--- .../RelationalDatabaseFacadeExtensions.cs | 44 +++++++++++++++---- .../Storage/IRelationalTransactionManager.cs | 21 +++++++++ .../Storage/RelationalConnection.cs | 27 ++++++++++-- .../RelationalConnectionTest.cs | 22 ++++++++++ .../RelationalEventIdTest.cs | 6 +++ .../Query/BadDataSqliteTest.cs | 6 +++ 8 files changed, 125 insertions(+), 25 deletions(-) diff --git a/src/EFCore.Relational/Diagnostics/DbTransactionInterceptor.cs b/src/EFCore.Relational/Diagnostics/DbTransactionInterceptor.cs index 635af1805c8..8f38a01d548 100644 --- a/src/EFCore.Relational/Diagnostics/DbTransactionInterceptor.cs +++ b/src/EFCore.Relational/Diagnostics/DbTransactionInterceptor.cs @@ -120,17 +120,17 @@ public virtual Task TransactionStartedAsync( /// /// - /// Called immediately after is called. + /// Called immediately after is called. /// /// /// The connection. /// Contextual information about connection and transaction. /// - /// The that was passed to . + /// The that was passed to . /// This value is typically used as the return value for the implementation of this method. /// /// - /// The value that will be used as the effective value passed to + /// The value that will be used as the effective value passed to /// A normal implementation of this method for any interceptor that is not attempting to change the result /// is to return the value passed in. /// @@ -142,19 +142,19 @@ public virtual DbTransaction TransactionUsed( /// /// - /// Called immediately after is called. + /// Called immediately after is called. /// /// /// The connection. /// Contextual information about connection and transaction. /// - /// The that was passed to . + /// The that was passed to . /// This value is typically used as the return value for the implementation of this method. /// /// The cancellation token. /// /// A containing the value that will be used as the effective value passed - /// to + /// to /// A normal implementation of this method for any interceptor that is not attempting to change the result /// is to return the value passed in, often using /// diff --git a/src/EFCore.Relational/Diagnostics/IDbTransactionInterceptor.cs b/src/EFCore.Relational/Diagnostics/IDbTransactionInterceptor.cs index d47b7e0edfb..7492644c909 100644 --- a/src/EFCore.Relational/Diagnostics/IDbTransactionInterceptor.cs +++ b/src/EFCore.Relational/Diagnostics/IDbTransactionInterceptor.cs @@ -137,17 +137,17 @@ Task TransactionStartedAsync( /// /// - /// Called immediately after is called. + /// Called immediately after is called. /// /// /// The connection. /// Contextual information about connection and transaction. /// - /// The that was passed to . + /// The that was passed to . /// This value is typically used as the return value for the implementation of this method. /// /// - /// The value that will be used as the effective value passed to + /// The value that will be used as the effective value passed to /// A normal implementation of this method for any interceptor that is not attempting to change the result /// is to return the value passed in. /// @@ -158,19 +158,19 @@ DbTransaction TransactionUsed( /// /// - /// Called immediately after is called. + /// Called immediately after is called. /// /// /// The connection. /// Contextual information about connection and transaction. /// - /// The that was passed to . + /// The that was passed to . /// This value is typically used as the return value for the implementation of this method. /// /// The cancellation token. /// /// A containing the value that will be used as the effective value passed - /// to + /// to /// A normal implementation of this method for any interceptor that is not attempting to change the result /// is to return the value passed in, often using /// diff --git a/src/EFCore.Relational/Extensions/RelationalDatabaseFacadeExtensions.cs b/src/EFCore.Relational/Extensions/RelationalDatabaseFacadeExtensions.cs index 625535816f0..3c2d71829cb 100644 --- a/src/EFCore.Relational/Extensions/RelationalDatabaseFacadeExtensions.cs +++ b/src/EFCore.Relational/Extensions/RelationalDatabaseFacadeExtensions.cs @@ -113,7 +113,7 @@ public static Task MigrateAsync( /// /// /// Note that this method does not start a transaction. To use this method with - /// a transaction, first call or . + /// a transaction, first call or . /// /// /// Note that the current is not used by this method @@ -151,7 +151,7 @@ public static int ExecuteSqlRaw( /// /// /// Note that this method does not start a transaction. To use this method with - /// a transaction, first call or . + /// a transaction, first call or . /// /// /// Note that the current is not used by this method @@ -182,7 +182,7 @@ public static int ExecuteSqlInterpolated( /// /// /// Note that this method does not start a transaction. To use this method with - /// a transaction, first call or . + /// a transaction, first call or . /// /// /// Note that the current is not used by this method @@ -244,7 +244,7 @@ public static int ExecuteSqlRaw( /// /// /// Note that this method does not start a transaction. To use this method with - /// a transaction, first call or . + /// a transaction, first call or . /// /// /// Note that the current is not used by this method @@ -279,7 +279,7 @@ public static Task ExecuteSqlInterpolatedAsync( /// /// /// Note that this method does not start a transaction. To use this method with - /// a transaction, first call or . + /// a transaction, first call or . /// /// /// Note that the current is not used by this method @@ -310,7 +310,7 @@ public static Task ExecuteSqlRawAsync( /// /// /// Note that this method does not start a transaction. To use this method with - /// a transaction, first call or . + /// a transaction, first call or . /// /// /// Note that the current is not used by this method @@ -350,7 +350,7 @@ public static Task ExecuteSqlRawAsync( /// /// /// Note that this method does not start a transaction. To use this method with - /// a transaction, first call or . + /// a transaction, first call or . /// /// /// Note that the current is not used by this method @@ -542,6 +542,17 @@ public static Task BeginTransactionAsync( /// A that encapsulates the given transaction. public static IDbContextTransaction UseTransaction( [NotNull] this DatabaseFacade databaseFacade, [CanBeNull] DbTransaction transaction) + => databaseFacade.UseTransaction(transaction, Guid.NewGuid()); + + /// + /// Sets the to be used by database operations on the . + /// + /// The for the context. + /// The to use. + /// The unique identifier for the transaction. + /// A that encapsulates the given transaction. + public static IDbContextTransaction UseTransaction( + [NotNull] this DatabaseFacade databaseFacade, [CanBeNull] DbTransaction transaction, Guid transactionId) { var transactionManager = GetTransactionManager(databaseFacade); @@ -550,7 +561,7 @@ public static IDbContextTransaction UseTransaction( throw new InvalidOperationException(RelationalStrings.RelationalNotInUse); } - return relationalTransactionManager.UseTransaction(transaction); + return relationalTransactionManager.UseTransaction(transaction, transactionId); } /// @@ -564,6 +575,21 @@ public static Task UseTransactionAsync( [NotNull] this DatabaseFacade databaseFacade, [CanBeNull] DbTransaction transaction, CancellationToken cancellationToken = default) + => databaseFacade.UseTransactionAsync(transaction, Guid.NewGuid(), cancellationToken); + + /// + /// Sets the to be used by database operations on the . + /// + /// The for the context. + /// The to use. + /// The unique identifier for the transaction. + /// A token to observe while waiting for the task to complete. + /// A containing the for the given transaction. + public static Task UseTransactionAsync( + [NotNull] this DatabaseFacade databaseFacade, + [CanBeNull] DbTransaction transaction, + Guid transactionId, + CancellationToken cancellationToken = default) { var transactionManager = GetTransactionManager(databaseFacade); @@ -572,7 +598,7 @@ public static Task UseTransactionAsync( throw new InvalidOperationException(RelationalStrings.RelationalNotInUse); } - return relationalTransactionManager.UseTransactionAsync(transaction, cancellationToken); + return relationalTransactionManager.UseTransactionAsync(transaction, transactionId, cancellationToken); } /// diff --git a/src/EFCore.Relational/Storage/IRelationalTransactionManager.cs b/src/EFCore.Relational/Storage/IRelationalTransactionManager.cs index 0c5da17a843..0af1e7b485d 100644 --- a/src/EFCore.Relational/Storage/IRelationalTransactionManager.cs +++ b/src/EFCore.Relational/Storage/IRelationalTransactionManager.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; using System.Data; using System.Data.Common; using System.Threading; @@ -53,10 +54,30 @@ public interface IRelationalTransactionManager : IDbContextTransactionManager /// Specifies an existing to be used for database operations. /// /// The transaction to be used. + /// The unique identifier for the transaction. + /// An instance of that wraps the provided transaction. + IDbContextTransaction UseTransaction([CanBeNull] DbTransaction transaction, Guid transactionId); + + /// + /// Specifies an existing to be used for database operations. + /// + /// The transaction to be used. + /// A to observe while waiting for the task to complete. + /// An instance of that wraps the provided transaction. + Task UseTransactionAsync( + [CanBeNull] DbTransaction transaction, + CancellationToken cancellationToken = default); + + /// + /// Specifies an existing to be used for database operations. + /// + /// The transaction to be used. + /// The unique identifier for the transaction. /// A to observe while waiting for the task to complete. /// An instance of that wraps the provided transaction. Task UseTransactionAsync( [CanBeNull] DbTransaction transaction, + Guid transactionId, CancellationToken cancellationToken = default); } } diff --git a/src/EFCore.Relational/Storage/RelationalConnection.cs b/src/EFCore.Relational/Storage/RelationalConnection.cs index 7ebcec6f835..a809b702c56 100644 --- a/src/EFCore.Relational/Storage/RelationalConnection.cs +++ b/src/EFCore.Relational/Storage/RelationalConnection.cs @@ -385,14 +385,22 @@ private IDbContextTransaction CreateRelationalTransaction(DbTransaction transact /// Specifies an existing to be used for database operations. /// /// The transaction to be used. + /// An instance of that wraps the provided transaction. public virtual IDbContextTransaction UseTransaction(DbTransaction transaction) + => UseTransaction(transaction, Guid.NewGuid()); + + /// + /// Specifies an existing to be used for database operations. + /// + /// The transaction to be used. + /// The unique identifier for the transaction. + /// An instance of that wraps the provided transaction. + public virtual IDbContextTransaction UseTransaction(DbTransaction transaction, Guid transactionId) { if (ShouldUseTransaction(transaction)) { Open(); - var transactionId = Guid.NewGuid(); - transaction = Dependencies.TransactionLogger.TransactionUsed( this, // ReSharper disable once AssignNullToNotNullAttribute @@ -412,16 +420,27 @@ public virtual IDbContextTransaction UseTransaction(DbTransaction transaction) /// The transaction to be used. /// A to observe while waiting for the task to complete. /// An instance of that wraps the provided transaction. + public virtual Task UseTransactionAsync( + DbTransaction transaction, + CancellationToken cancellationToken = default) + => UseTransactionAsync(transaction, Guid.NewGuid(), cancellationToken); + + /// + /// Specifies an existing to be used for database operations. + /// + /// The transaction to be used. + /// The unique identifier for the transaction. + /// A to observe while waiting for the task to complete. + /// An instance of that wraps the provided transaction. public virtual async Task UseTransactionAsync( DbTransaction transaction, + Guid transactionId, CancellationToken cancellationToken = default) { if (ShouldUseTransaction(transaction)) { await OpenAsync(cancellationToken); - var transactionId = Guid.NewGuid(); - transaction = await Dependencies.TransactionLogger.TransactionUsedAsync( this, // ReSharper disable once AssignNullToNotNullAttribute diff --git a/test/EFCore.Relational.Tests/RelationalConnectionTest.cs b/test/EFCore.Relational.Tests/RelationalConnectionTest.cs index aecb94365ea..c0571207c5c 100644 --- a/test/EFCore.Relational.Tests/RelationalConnectionTest.cs +++ b/test/EFCore.Relational.Tests/RelationalConnectionTest.cs @@ -735,6 +735,28 @@ public void Can_use_existing_transaction() Assert.Null(connection.CurrentTransaction); } + [ConditionalFact] + public void Can_use_existing_transaction_identifier() + { + var dbConnection = new FakeDbConnection("Database=FrodoLives"); + + var dbTransaction = dbConnection.BeginTransaction(IsolationLevel.Unspecified); + + using var connection = new FakeRelationalConnection( + CreateOptions(new FakeRelationalOptionsExtension().WithConnection(dbConnection))); + Assert.Null(connection.CurrentTransaction); + + var transactionId = Guid.NewGuid(); + + using (var transaction = connection.UseTransaction(dbTransaction, transactionId)) + { + Assert.Equal(dbTransaction, connection.CurrentTransaction.GetDbTransaction()); + Assert.Equal(transactionId, transaction.TransactionId); + } + + Assert.Null(connection.CurrentTransaction); + } + [ConditionalTheory] [InlineData(true)] [InlineData(false)] diff --git a/test/EFCore.Relational.Tests/RelationalEventIdTest.cs b/test/EFCore.Relational.Tests/RelationalEventIdTest.cs index 15bba5f126b..b690f3c57af 100644 --- a/test/EFCore.Relational.Tests/RelationalEventIdTest.cs +++ b/test/EFCore.Relational.Tests/RelationalEventIdTest.cs @@ -172,9 +172,15 @@ public Task OpenAsync(CancellationToken cancellationToken, bool errorsExpe public IDbContextTransaction UseTransaction(DbTransaction transaction) => throw new NotImplementedException(); + public IDbContextTransaction UseTransaction(DbTransaction transaction, Guid transactionId) => + throw new NotImplementedException(); + public Task UseTransactionAsync( DbTransaction transaction, CancellationToken cancellationToken = default) => throw new NotImplementedException(); + public Task UseTransactionAsync( + DbTransaction transaction, Guid transactionId, CancellationToken cancellationToken = default) => throw new NotImplementedException(); + public ValueTask DisposeAsync() => throw new NotImplementedException(); } diff --git a/test/EFCore.Sqlite.FunctionalTests/Query/BadDataSqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/Query/BadDataSqliteTest.cs index f839b8a69d4..24ed4e12a10 100644 --- a/test/EFCore.Sqlite.FunctionalTests/Query/BadDataSqliteTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/Query/BadDataSqliteTest.cs @@ -378,9 +378,15 @@ public Task BeginTransactionAsync( public IDbContextTransaction UseTransaction(DbTransaction transaction) => throw new NotImplementedException(); + public IDbContextTransaction UseTransaction(DbTransaction transaction, Guid transactionId) => + throw new NotImplementedException(); + public Task UseTransactionAsync( DbTransaction transaction, CancellationToken cancellationToken = default) => throw new NotImplementedException(); + public Task UseTransactionAsync( + DbTransaction transaction, Guid transactionId, CancellationToken cancellationToken = default) => throw new NotImplementedException(); + public void Dispose() { }