From 112a8967a09af0f4a84f296367655c1073f67ba3 Mon Sep 17 00:00:00 2001 From: Nikita Kazmin Date: Sun, 9 May 2021 13:09:04 +0300 Subject: [PATCH 1/3] Allow migrations inside of a user transaction --- .../Internal/MigrationCommandExecutor.cs | 20 +++- .../MigrationCommandExecutorTest.cs | 100 ++++++++++++++++++ 2 files changed, 118 insertions(+), 2 deletions(-) diff --git a/src/EFCore.Relational/Migrations/Internal/MigrationCommandExecutor.cs b/src/EFCore.Relational/Migrations/Internal/MigrationCommandExecutor.cs index 4a8791293bb..b62ba1c66ea 100644 --- a/src/EFCore.Relational/Migrations/Internal/MigrationCommandExecutor.cs +++ b/src/EFCore.Relational/Migrations/Internal/MigrationCommandExecutor.cs @@ -1,7 +1,9 @@ // 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.Collections.Generic; +using System.Linq; using System.Threading; using System.Threading.Tasks; using System.Transactions; @@ -39,6 +41,12 @@ public virtual void ExecuteNonQuery( Check.NotNull(migrationCommands, nameof(migrationCommands)); Check.NotNull(connection, nameof(connection)); + var userTransaction = connection.CurrentTransaction; + if (userTransaction is not null && migrationCommands.Any(x => x.TransactionSuppressed)) + { + throw new NotSupportedException("User transaction is not supported with a TransactionSuppressed migrations"); + } + using (new TransactionScope(TransactionScopeOption.Suppress, TransactionScopeAsyncFlowOption.Enabled)) { connection.Open(); @@ -52,7 +60,8 @@ public virtual void ExecuteNonQuery( foreach (var command in migrationCommands) { if (transaction == null - && !command.TransactionSuppressed) + && !command.TransactionSuppressed + && userTransaction is null) { transaction = connection.BeginTransaction(); } @@ -96,6 +105,12 @@ public virtual async Task ExecuteNonQueryAsync( Check.NotNull(migrationCommands, nameof(migrationCommands)); Check.NotNull(connection, nameof(connection)); + var userTransaction = connection.CurrentTransaction; + if (userTransaction is not null && migrationCommands.Any(x => x.TransactionSuppressed)) + { + throw new NotSupportedException("User transaction is not supported with a TransactionSuppressed migrations"); + } + var transactionScope = new TransactionScope(TransactionScopeOption.Suppress, TransactionScopeAsyncFlowOption.Enabled); try { @@ -110,7 +125,8 @@ public virtual async Task ExecuteNonQueryAsync( foreach (var command in migrationCommands) { if (transaction == null - && !command.TransactionSuppressed) + && !command.TransactionSuppressed + && userTransaction is null) { transaction = await connection.BeginTransactionAsync(cancellationToken) .ConfigureAwait(false); diff --git a/test/EFCore.Relational.Tests/Migrations/MigrationCommandExecutorTest.cs b/test/EFCore.Relational.Tests/Migrations/MigrationCommandExecutorTest.cs index dc9384f150e..1b11124bf5b 100644 --- a/test/EFCore.Relational.Tests/Migrations/MigrationCommandExecutorTest.cs +++ b/test/EFCore.Relational.Tests/Migrations/MigrationCommandExecutorTest.cs @@ -58,6 +58,106 @@ public async Task Executes_migration_commands_in_same_transaction(bool async) fakeConnection.DbConnections[0].DbCommands[1].Transaction); } + [ConditionalTheory] + [InlineData(false)] + [InlineData(true)] + public async Task Executes_migration_commands_in_user_transaction(bool async) + { + var fakeConnection = CreateConnection(); + var logger = new FakeRelationalCommandDiagnosticsLogger(); + + var commandList = new List + { + new(CreateRelationalCommand(), null, logger), + new(CreateRelationalCommand(), null, logger) + }; + + var migrationCommandExecutor = new MigrationCommandExecutor(); + + IDbContextTransaction tx; + using (tx = fakeConnection.BeginTransaction()) + { + if (async) + { + await migrationCommandExecutor.ExecuteNonQueryAsync(commandList, fakeConnection); + } + else + { + migrationCommandExecutor.ExecuteNonQuery(commandList, fakeConnection); + } + + tx.Commit(); + } + + Assert.Equal(1, fakeConnection.DbConnections.Count); + Assert.Equal(1, fakeConnection.DbConnections[0].OpenCount); + Assert.Equal(1, fakeConnection.DbConnections[0].CloseCount); + + Assert.Equal(1, fakeConnection.DbConnections[0].DbTransactions.Count); + Assert.Equal(1, fakeConnection.DbConnections[0].DbTransactions[0].CommitCount); + Assert.Equal(0, fakeConnection.DbConnections[0].DbTransactions[0].RollbackCount); + + Assert.Equal(2, fakeConnection.DbConnections[0].DbCommands.Count); + Assert.Same( + fakeConnection.DbConnections[0].DbTransactions[0], + fakeConnection.DbConnections[0].DbCommands[0].Transaction); + Assert.Same( + fakeConnection.DbConnections[0].DbTransactions[0], + fakeConnection.DbConnections[0].DbCommands[1].Transaction); + Assert.Same( + tx.GetDbTransaction(), + fakeConnection.DbConnections[0].DbTransactions[0]); + } + + [ConditionalTheory] + [InlineData(false)] + [InlineData(true)] + public async Task Executes_transaction_suppressed_migration_commands_in_user_transaction(bool async) + { + var fakeConnection = CreateConnection(); + var logger = new FakeRelationalCommandDiagnosticsLogger(); + + var commandList = new List + { + new(CreateRelationalCommand(), null, logger), + new(CreateRelationalCommand(), null, logger, transactionSuppressed: true) + }; + + var migrationCommandExecutor = new MigrationCommandExecutor(); + + IDbContextTransaction tx; + using (tx = fakeConnection.BeginTransaction()) + { + if (async) + { + await Assert.ThrowsAsync( + async () + => await migrationCommandExecutor.ExecuteNonQueryAsync(commandList, fakeConnection)); + } + else + { + Assert.Throws( + () + => migrationCommandExecutor.ExecuteNonQuery(commandList, fakeConnection)); + } + + tx.Rollback(); + } + + Assert.Equal(1, fakeConnection.DbConnections.Count); + Assert.Equal(1, fakeConnection.DbConnections[0].OpenCount); + Assert.Equal(1, fakeConnection.DbConnections[0].CloseCount); + + Assert.Equal(1, fakeConnection.DbConnections[0].DbTransactions.Count); + Assert.Equal(0, fakeConnection.DbConnections[0].DbTransactions[0].CommitCount); + Assert.Equal(1, fakeConnection.DbConnections[0].DbTransactions[0].RollbackCount); + + Assert.Equal(0, fakeConnection.DbConnections[0].DbCommands.Count); + Assert.Same( + tx.GetDbTransaction(), + fakeConnection.DbConnections[0].DbTransactions[0]); + } + [ConditionalTheory] [InlineData(false)] [InlineData(true)] From 2e5805fcb4f08c58042231667086a375f10ab668 Mon Sep 17 00:00:00 2001 From: Nikita Kazmin Date: Tue, 11 May 2021 18:43:01 +0300 Subject: [PATCH 2/3] Made the exception message a relational string --- .../Migrations/Internal/MigrationCommandExecutor.cs | 5 +++-- .../Properties/RelationalStrings.Designer.cs | 6 ++++++ src/EFCore.Relational/Properties/RelationalStrings.resx | 3 +++ 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/EFCore.Relational/Migrations/Internal/MigrationCommandExecutor.cs b/src/EFCore.Relational/Migrations/Internal/MigrationCommandExecutor.cs index b62ba1c66ea..f074156d66e 100644 --- a/src/EFCore.Relational/Migrations/Internal/MigrationCommandExecutor.cs +++ b/src/EFCore.Relational/Migrations/Internal/MigrationCommandExecutor.cs @@ -7,6 +7,7 @@ using System.Threading; using System.Threading.Tasks; using System.Transactions; +using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.EntityFrameworkCore.Storage; using Microsoft.EntityFrameworkCore.Utilities; using Microsoft.Extensions.DependencyInjection; @@ -44,7 +45,7 @@ public virtual void ExecuteNonQuery( var userTransaction = connection.CurrentTransaction; if (userTransaction is not null && migrationCommands.Any(x => x.TransactionSuppressed)) { - throw new NotSupportedException("User transaction is not supported with a TransactionSuppressed migrations"); + throw new NotSupportedException(RelationalStrings.TransactionSuppressedMigrationInUserTransaction); } using (new TransactionScope(TransactionScopeOption.Suppress, TransactionScopeAsyncFlowOption.Enabled)) @@ -108,7 +109,7 @@ public virtual async Task ExecuteNonQueryAsync( var userTransaction = connection.CurrentTransaction; if (userTransaction is not null && migrationCommands.Any(x => x.TransactionSuppressed)) { - throw new NotSupportedException("User transaction is not supported with a TransactionSuppressed migrations"); + throw new NotSupportedException(RelationalStrings.TransactionSuppressedMigrationInUserTransaction); } var transactionScope = new TransactionScope(TransactionScopeOption.Suppress, TransactionScopeAsyncFlowOption.Enabled); diff --git a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs index ec06eac505e..42d721390be 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs +++ b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs @@ -1053,6 +1053,12 @@ public static string TransactionAlreadyStarted public static string TransactionAssociatedWithDifferentConnection => GetString("TransactionAssociatedWithDifferentConnection"); + /// + /// User transaction is not supported with a TransactionSuppressed migrations. + /// + public static string TransactionSuppressedMigrationInUserTransaction + => GetString("TransactionSuppressedMigrationInUserTransaction"); + /// /// Unable to bind '{memberType}.{member}' to an entity projection of '{entityType}'. /// diff --git a/src/EFCore.Relational/Properties/RelationalStrings.resx b/src/EFCore.Relational/Properties/RelationalStrings.resx index 30e53e273fd..7673a415acc 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.resx +++ b/src/EFCore.Relational/Properties/RelationalStrings.resx @@ -745,6 +745,9 @@ The specified transaction is not associated with the current connection. Only transactions associated with the current connection may be used. + + User transaction is not supported with a TransactionSuppressed migrations. + Unable to bind '{memberType}.{member}' to an entity projection of '{entityType}'. From 35427e04ccf291a2dd9148bb1dd91e210bb4b486 Mon Sep 17 00:00:00 2001 From: Nikita Kazmin Date: Tue, 11 May 2021 18:43:30 +0300 Subject: [PATCH 3/3] Modified the test to check for the message --- .../Migrations/MigrationCommandExecutorTest.cs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/test/EFCore.Relational.Tests/Migrations/MigrationCommandExecutorTest.cs b/test/EFCore.Relational.Tests/Migrations/MigrationCommandExecutorTest.cs index 1b11124bf5b..7bd475ab7d3 100644 --- a/test/EFCore.Relational.Tests/Migrations/MigrationCommandExecutorTest.cs +++ b/test/EFCore.Relational.Tests/Migrations/MigrationCommandExecutorTest.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Threading.Tasks; +using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.EntityFrameworkCore.Diagnostics.Internal; using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.EntityFrameworkCore.Migrations.Internal; @@ -130,15 +131,17 @@ public async Task Executes_transaction_suppressed_migration_commands_in_user_tra { if (async) { - await Assert.ThrowsAsync( + Assert.Equal(RelationalStrings.TransactionSuppressedMigrationInUserTransaction, + (await Assert.ThrowsAsync( async () - => await migrationCommandExecutor.ExecuteNonQueryAsync(commandList, fakeConnection)); + => await migrationCommandExecutor.ExecuteNonQueryAsync(commandList, fakeConnection))).Message); } else { - Assert.Throws( - () - => migrationCommandExecutor.ExecuteNonQuery(commandList, fakeConnection)); + Assert.Equal(RelationalStrings.TransactionSuppressedMigrationInUserTransaction, + Assert.Throws( + () + => migrationCommandExecutor.ExecuteNonQuery(commandList, fakeConnection)).Message); } tx.Rollback();