diff --git a/src/EFCore.Relational/Infrastructure/EntityFrameworkRelationalServicesBuilder.cs b/src/EFCore.Relational/Infrastructure/EntityFrameworkRelationalServicesBuilder.cs index da72feae3c7..2a526d3dfc2 100644 --- a/src/EFCore.Relational/Infrastructure/EntityFrameworkRelationalServicesBuilder.cs +++ b/src/EFCore.Relational/Infrastructure/EntityFrameworkRelationalServicesBuilder.cs @@ -54,7 +54,7 @@ public static readonly IDictionary RelationalServi { typeof(ISqlGenerationHelper), new ServiceCharacteristics(ServiceLifetime.Singleton) }, { typeof(IRelationalAnnotationProvider), new ServiceCharacteristics(ServiceLifetime.Singleton) }, { typeof(IMigrationsAnnotationProvider), new ServiceCharacteristics(ServiceLifetime.Singleton) }, - { typeof(IMigrationCommandExecutor), new ServiceCharacteristics(ServiceLifetime.Singleton) }, + { typeof(IMigrationCommandExecutor), new ServiceCharacteristics(ServiceLifetime.Scoped) }, { typeof(IRelationalTypeMappingSource), new ServiceCharacteristics(ServiceLifetime.Singleton) }, { typeof(IUpdateSqlGenerator), new ServiceCharacteristics(ServiceLifetime.Singleton) }, { typeof(IRelationalTransactionFactory), new ServiceCharacteristics(ServiceLifetime.Singleton) }, diff --git a/src/EFCore.Relational/Migrations/IMigrationCommandExecutor.cs b/src/EFCore.Relational/Migrations/IMigrationCommandExecutor.cs index fe669e35e6c..7069ba9c045 100644 --- a/src/EFCore.Relational/Migrations/IMigrationCommandExecutor.cs +++ b/src/EFCore.Relational/Migrations/IMigrationCommandExecutor.cs @@ -8,9 +8,10 @@ namespace Microsoft.EntityFrameworkCore.Migrations; /// /// /// -/// The service lifetime is . This means a single instance -/// is used by many instances. The implementation must be thread-safe. -/// This service cannot depend on services registered as . +/// The service lifetime is . This means that each +/// instance will use its own instance of this service. +/// The implementation may depend on other services registered with any lifetime. +/// The implementation does not need to be thread-safe. /// /// /// See Database migrations for more information and examples. diff --git a/src/EFCore.Relational/Migrations/Internal/MigrationCommandExecutor.cs b/src/EFCore.Relational/Migrations/Internal/MigrationCommandExecutor.cs index 24546240a86..e545d4c13b5 100644 --- a/src/EFCore.Relational/Migrations/Internal/MigrationCommandExecutor.cs +++ b/src/EFCore.Relational/Migrations/Internal/MigrationCommandExecutor.cs @@ -13,6 +13,19 @@ namespace Microsoft.EntityFrameworkCore.Migrations.Internal; /// public class MigrationCommandExecutor : IMigrationCommandExecutor { + private readonly IExecutionStrategy _executionStrategy; + + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + public MigrationCommandExecutor(IExecutionStrategy executionStrategy) + { + _executionStrategy = executionStrategy; + } + /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in @@ -24,53 +37,70 @@ public virtual void ExecuteNonQuery( IRelationalConnection connection) { var userTransaction = connection.CurrentTransaction; - if (userTransaction is not null && migrationCommands.Any(x => x.TransactionSuppressed)) + if (userTransaction is not null + && (migrationCommands.Any(x => x.TransactionSuppressed) || _executionStrategy.RetriesOnFailure)) { throw new NotSupportedException(RelationalStrings.TransactionSuppressedMigrationInUserTransaction); } using (new TransactionScope(TransactionScopeOption.Suppress, TransactionScopeAsyncFlowOption.Enabled)) { - connection.Open(); + var parameters = new ExecuteParameters(migrationCommands.ToList(), connection); + if (userTransaction is null) + { + _executionStrategy.Execute(parameters, static (_, p) => Execute(p, beginTransaction: true), verifySucceeded: null); + } + else + { + Execute(parameters, beginTransaction: false); + } + } + } - try + private static bool Execute(ExecuteParameters parameters, bool beginTransaction) + { + var migrationCommands = parameters.MigrationCommands; + var connection = parameters.Connection; + IDbContextTransaction? transaction = null; + connection.Open(); + try + { + for (var i = parameters.CurrentCommandIndex; i < migrationCommands.Count; i++) { - IDbContextTransaction? transaction = null; + var command = migrationCommands[i]; + if (transaction == null + && !command.TransactionSuppressed + && beginTransaction) + { + transaction = connection.BeginTransaction(); + } - try + if (transaction != null + && command.TransactionSuppressed) { - foreach (var command in migrationCommands) - { - if (transaction == null - && !command.TransactionSuppressed - && userTransaction is null) - { - transaction = connection.BeginTransaction(); - } - - if (transaction != null - && command.TransactionSuppressed) - { - transaction.Commit(); - transaction.Dispose(); - transaction = null; - } - - command.ExecuteNonQuery(connection); - } - - transaction?.Commit(); + transaction.Commit(); + transaction.Dispose(); + transaction = null; + parameters.CurrentCommandIndex = i; } - finally + + command.ExecuteNonQuery(connection); + + if (transaction == null) { - transaction?.Dispose(); + parameters.CurrentCommandIndex = i + 1; } } - finally - { - connection.Close(); - } + + transaction?.Commit(); + } + finally + { + transaction?.Dispose(); + connection.Close(); } + + return true; } /// @@ -85,7 +115,8 @@ public virtual async Task ExecuteNonQueryAsync( CancellationToken cancellationToken = default) { var userTransaction = connection.CurrentTransaction; - if (userTransaction is not null && migrationCommands.Any(x => x.TransactionSuppressed)) + if (userTransaction is not null + && (migrationCommands.Any(x => x.TransactionSuppressed) || _executionStrategy.RetriesOnFailure)) { throw new NotSupportedException(RelationalStrings.TransactionSuppressedMigrationInUserTransaction); } @@ -93,57 +124,86 @@ public virtual async Task ExecuteNonQueryAsync( var transactionScope = new TransactionScope(TransactionScopeOption.Suppress, TransactionScopeAsyncFlowOption.Enabled); try { - await connection.OpenAsync(cancellationToken).ConfigureAwait(false); + var parameters = new ExecuteParameters(migrationCommands.ToList(), connection); + if (userTransaction is null) + { + await _executionStrategy.ExecuteAsync( + parameters, + static (_, p, ct) => ExecuteAsync(p, beginTransaction: true, ct), + verifySucceeded: null, + cancellationToken).ConfigureAwait(false); + } + else + { + await ExecuteAsync(parameters, beginTransaction: false, cancellationToken).ConfigureAwait(false); + } + + } + finally + { + await transactionScope.DisposeAsyncIfAvailable().ConfigureAwait(false); + } + } - try + private static async Task ExecuteAsync(ExecuteParameters parameters, bool beginTransaction, CancellationToken cancellationToken) + { + var migrationCommands = parameters.MigrationCommands; + var connection = parameters.Connection; + IDbContextTransaction? transaction = null; + await connection.OpenAsync(cancellationToken).ConfigureAwait(false); + try + { + for (var i = parameters.CurrentCommandIndex; i < migrationCommands.Count; i++) { - IDbContextTransaction? transaction = null; + var command = migrationCommands[i]; + if (transaction == null + && !command.TransactionSuppressed + && beginTransaction) + { + transaction = await connection.BeginTransactionAsync(cancellationToken) + .ConfigureAwait(false); + } - try + if (transaction != null + && command.TransactionSuppressed) { - foreach (var command in migrationCommands) - { - if (transaction == null - && !command.TransactionSuppressed - && userTransaction is null) - { - transaction = await connection.BeginTransactionAsync(cancellationToken) - .ConfigureAwait(false); - } - - if (transaction != null - && command.TransactionSuppressed) - { - await transaction.CommitAsync(cancellationToken).ConfigureAwait(false); - await transaction.DisposeAsync().ConfigureAwait(false); - transaction = null; - } - - await command.ExecuteNonQueryAsync(connection, cancellationToken: cancellationToken) - .ConfigureAwait(false); - } - - if (transaction != null) - { - await transaction.CommitAsync(cancellationToken).ConfigureAwait(false); - } + await transaction.CommitAsync(cancellationToken).ConfigureAwait(false); + await transaction.DisposeAsync().ConfigureAwait(false); + transaction = null; + parameters.CurrentCommandIndex = i; } - finally + + await command.ExecuteNonQueryAsync(connection, cancellationToken: cancellationToken) + .ConfigureAwait(false); + + if (transaction == null) { - if (transaction != null) - { - await transaction.DisposeAsync().ConfigureAwait(false); - } + parameters.CurrentCommandIndex = i + 1; } } - finally + + if (transaction != null) { - await connection.CloseAsync().ConfigureAwait(false); + await transaction.CommitAsync(cancellationToken).ConfigureAwait(false); } } finally { - await transactionScope.DisposeAsyncIfAvailable().ConfigureAwait(false); + if (transaction != null) + { + await transaction.DisposeAsync().ConfigureAwait(false); + } + + await connection.CloseAsync().ConfigureAwait(false); } + + return true; + } + + private sealed class ExecuteParameters(List migrationCommands, IRelationalConnection connection) + { + public int CurrentCommandIndex; + public List MigrationCommands { get; } = migrationCommands; + public IRelationalConnection Connection { get; } = connection; } } diff --git a/src/EFCore.Relational/Migrations/Internal/Migrator.cs b/src/EFCore.Relational/Migrations/Internal/Migrator.cs index 5c8a08093fe..fcd48552861 100644 --- a/src/EFCore.Relational/Migrations/Internal/Migrator.cs +++ b/src/EFCore.Relational/Migrations/Internal/Migrator.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using Microsoft.EntityFrameworkCore.Diagnostics.Internal; +using Microsoft.EntityFrameworkCore.Storage; namespace Microsoft.EntityFrameworkCore.Migrations.Internal; @@ -361,6 +362,7 @@ public virtual string GenerateScript( var migrationsToApply = migratorData.AppliedMigrations; var migrationsToRevert = migratorData.RevertedMigrations; var actualTargetMigration = migratorData.TargetMigration; + var transactionStarted = false; for (var i = 0; i < migrationsToRevert.Count; i++) { var migration = migrationsToRevert[i]; @@ -374,7 +376,9 @@ public virtual string GenerateScript( ? _historyRepository.GetBeginIfExistsScript(migration.GetId()) : null; - GenerateSqlScript(GenerateDownSql(migration, previousMigration, options), builder, _sqlGenerationHelper, noTransactions, idempotencyCondition, idempotencyEnd); + GenerateSqlScript( + GenerateDownSql(migration, previousMigration, options), + builder, _sqlGenerationHelper, ref transactionStarted, noTransactions, idempotencyCondition, idempotencyEnd); } foreach (var migration in migrationsToApply) @@ -385,7 +389,16 @@ public virtual string GenerateScript( ? _historyRepository.GetBeginIfNotExistsScript(migration.GetId()) : null; - GenerateSqlScript(GenerateUpSql(migration, options), builder, _sqlGenerationHelper, noTransactions, idempotencyCondition, idempotencyEnd); + GenerateSqlScript( + GenerateUpSql(migration, options), + builder, _sqlGenerationHelper, ref transactionStarted, noTransactions, idempotencyCondition, idempotencyEnd); + } + + if (!noTransactions && transactionStarted) + { + builder + .AppendLine(_sqlGenerationHelper.CommitTransactionStatement) + .Append(_sqlGenerationHelper.BatchTerminator); } return builder.ToString(); @@ -395,11 +408,11 @@ private static void GenerateSqlScript( IEnumerable commands, IndentedStringBuilder builder, ISqlGenerationHelper sqlGenerationHelper, + ref bool transactionStarted, bool noTransactions = false, string? idempotencyCondition = null, string? idempotencyEnd = null) { - var transactionStarted = false; foreach (var command in commands) { if (!noTransactions) @@ -439,13 +452,6 @@ private static void GenerateSqlScript( builder.Append(sqlGenerationHelper.BatchTerminator); } - - if (!noTransactions && transactionStarted) - { - builder - .AppendLine(sqlGenerationHelper.CommitTransactionStatement) - .Append(sqlGenerationHelper.BatchTerminator); - } } /// diff --git a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs index d94c8e9877f..f1bd4dba7ad 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs +++ b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs @@ -2036,7 +2036,7 @@ public static string TransactionAssociatedWithDifferentConnection => GetString("TransactionAssociatedWithDifferentConnection"); /// - /// User transaction is not supported with a TransactionSuppressed migrations. + /// User transaction is not supported with a TransactionSuppressed migrations or a retrying execution strategy. /// public static string TransactionSuppressedMigrationInUserTransaction => GetString("TransactionSuppressedMigrationInUserTransaction"); diff --git a/src/EFCore.Relational/Properties/RelationalStrings.resx b/src/EFCore.Relational/Properties/RelationalStrings.resx index f974ba7aa2c..c5f16c5539b 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.resx +++ b/src/EFCore.Relational/Properties/RelationalStrings.resx @@ -1205,7 +1205,7 @@ 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. + User transaction is not supported with a TransactionSuppressed migrations or a retrying execution strategy. Trigger '{trigger}' for table '{triggerTable}' is defined on entity type '{entityType}', which is mapped to table '{entityTable}'. See https://aka.ms/efcore-docs-triggers for more information on triggers. diff --git a/src/EFCore.Sqlite.Core/Extensions/SqliteServiceCollectionExtensions.cs b/src/EFCore.Sqlite.Core/Extensions/SqliteServiceCollectionExtensions.cs index c109fcdded7..20190cc57aa 100644 --- a/src/EFCore.Sqlite.Core/Extensions/SqliteServiceCollectionExtensions.cs +++ b/src/EFCore.Sqlite.Core/Extensions/SqliteServiceCollectionExtensions.cs @@ -117,9 +117,8 @@ public static IServiceCollection AddEntityFrameworkSqlite(this IServiceCollectio .TryAdd() .TryAdd() .TryAddProviderSpecificServices( - b => b.TryAddScoped()); - - builder.TryAddCoreServices(); + b => b.TryAddScoped()) + .TryAddCoreServices(); return serviceCollection; } diff --git a/test/EFCore.Design.Tests/Migrations/Design/MigrationScaffolderTest.cs b/test/EFCore.Design.Tests/Migrations/Design/MigrationScaffolderTest.cs index 8faf0632fc3..7cab26ed8f3 100644 --- a/test/EFCore.Design.Tests/Migrations/Design/MigrationScaffolderTest.cs +++ b/test/EFCore.Design.Tests/Migrations/Design/MigrationScaffolderTest.cs @@ -91,8 +91,7 @@ var migrationAssembly services.GetRequiredService()), idGenerator, new MigrationsCodeGeneratorSelector( - new[] - { + [ new CSharpMigrationsGenerator( new MigrationsCodeGeneratorDependencies( sqlServerTypeMappingSource, @@ -105,7 +104,7 @@ var migrationAssembly new CSharpSnapshotGenerator( new CSharpSnapshotGeneratorDependencies( code, sqlServerTypeMappingSource, sqlServerAnnotationCodeGenerator)))) - }), + ]), historyRepository, reporter, new MockProvider(), diff --git a/test/EFCore.Relational.Tests/Migrations/MigrationCommandExecutorTest.cs b/test/EFCore.Relational.Tests/Migrations/MigrationCommandExecutorTest.cs index 57ef0609153..630fa0d5269 100644 --- a/test/EFCore.Relational.Tests/Migrations/MigrationCommandExecutorTest.cs +++ b/test/EFCore.Relational.Tests/Migrations/MigrationCommandExecutorTest.cs @@ -22,7 +22,7 @@ public async Task Executes_migration_commands_in_same_transaction(bool async) new(CreateRelationalCommand(), null, logger), new(CreateRelationalCommand(), null, logger) }; - var migrationCommandExecutor = new MigrationCommandExecutor(); + var migrationCommandExecutor = CreateMigrationCommandExecutor(); if (async) { @@ -63,7 +63,7 @@ public async Task Executes_migration_commands_in_user_transaction(bool async) new(CreateRelationalCommand(), null, logger), new(CreateRelationalCommand(), null, logger) }; - var migrationCommandExecutor = new MigrationCommandExecutor(); + var migrationCommandExecutor = CreateMigrationCommandExecutor(); IDbContextTransaction tx; using (tx = fakeConnection.BeginTransaction()) @@ -113,7 +113,7 @@ public async Task Executes_transaction_suppressed_migration_commands_in_user_tra new(CreateRelationalCommand(), null, logger), new(CreateRelationalCommand(), null, logger, transactionSuppressed: true) }; - var migrationCommandExecutor = new MigrationCommandExecutor(); + var migrationCommandExecutor = CreateMigrationCommandExecutor(); IDbContextTransaction tx; using (tx = fakeConnection.BeginTransaction()) @@ -166,7 +166,7 @@ public async Task Executes_migration_commands_with_transaction_suppressed_outsid new(CreateRelationalCommand(), null, logger, transactionSuppressed: true) }; - var migrationCommandExecutor = new MigrationCommandExecutor(); + var migrationCommandExecutor = CreateMigrationCommandExecutor(); if (async) { @@ -201,7 +201,7 @@ public async Task Ends_transaction_when_transaction_is_suppressed(bool async) new(CreateRelationalCommand(), null, logger), new(CreateRelationalCommand(), null, logger, transactionSuppressed: true) }; - var migrationCommandExecutor = new MigrationCommandExecutor(); + var migrationCommandExecutor = CreateMigrationCommandExecutor(); if (async) { @@ -241,7 +241,7 @@ public async Task Begins_new_transaction_when_transaction_nolonger_suppressed(bo new(CreateRelationalCommand(), null, logger, transactionSuppressed: true), new(CreateRelationalCommand(), null, logger) }; - var migrationCommandExecutor = new MigrationCommandExecutor(); + var migrationCommandExecutor = CreateMigrationCommandExecutor(); if (async) { @@ -283,7 +283,7 @@ public async Task Executes_commands_in_order_regardless_of_transaction_suppressi new(CreateRelationalCommand(commandText: "Third"), null, logger) }; - var migrationCommandExecutor = new MigrationCommandExecutor(); + var migrationCommandExecutor = CreateMigrationCommandExecutor(); if (async) { @@ -353,19 +353,17 @@ public async Task Disposes_transaction_on_exception(bool async) var commandList = new List { new(CreateRelationalCommand(), null, logger) }; - var migrationCommandExecutor = new MigrationCommandExecutor(); + var migrationCommandExecutor = CreateMigrationCommandExecutor(); if (async) { - await Assert.ThrowsAsync( - async () - => await migrationCommandExecutor.ExecuteNonQueryAsync(commandList, fakeConnection)); + await Assert.ThrowsAsync(async () + => await migrationCommandExecutor.ExecuteNonQueryAsync(commandList, fakeConnection)); } else { - Assert.Throws( - () - => migrationCommandExecutor.ExecuteNonQuery(commandList, fakeConnection)); + Assert.Throws(() + => migrationCommandExecutor.ExecuteNonQuery(commandList, fakeConnection)); } Assert.Equal(1, fakeDbConnection.OpenCount); @@ -377,6 +375,9 @@ await Assert.ThrowsAsync( Assert.Equal(0, fakeDbConnection.DbTransactions[0].RollbackCount); } + private static IMigrationCommandExecutor CreateMigrationCommandExecutor() + => FakeRelationalTestHelpers.Instance.CreateContextServices().GetRequiredService(); + private const string ConnectionString = "Fake Connection String"; private static FakeRelationalConnection CreateConnection(IDbContextOptions options = null) diff --git a/test/EFCore.Relational.Tests/TestUtilities/FakeProvider/FakeRelationalOptionsExtension.cs b/test/EFCore.Relational.Tests/TestUtilities/FakeProvider/FakeRelationalOptionsExtension.cs index 940391a6704..c23460f4ea7 100644 --- a/test/EFCore.Relational.Tests/TestUtilities/FakeProvider/FakeRelationalOptionsExtension.cs +++ b/test/EFCore.Relational.Tests/TestUtilities/FakeProvider/FakeRelationalOptionsExtension.cs @@ -27,7 +27,7 @@ public override void ApplyServices(IServiceCollection services) public static IServiceCollection AddEntityFrameworkRelationalDatabase(IServiceCollection serviceCollection) { - var builder = new EntityFrameworkRelationalServicesBuilder(serviceCollection) + new EntityFrameworkRelationalServicesBuilder(serviceCollection) .TryAdd() .TryAdd>() .TryAdd() @@ -38,9 +38,8 @@ public static IServiceCollection AddEntityFrameworkRelationalDatabase(IServiceCo .TryAdd(_ => null) .TryAdd() .TryAdd() - .TryAdd(); - - builder.TryAddCoreServices(); + .TryAdd() + .TryAddCoreServices(); return serviceCollection; }