From 46977209c1fe83b96e83c87bccbeea1a0eeea45f Mon Sep 17 00:00:00 2001 From: Andriy Svyryd Date: Wed, 10 Jul 2024 18:04:53 -0700 Subject: [PATCH] Execute migrations using the ExecutionStrategy Use a single transaction for all migrations in the script Fixes #17578 Fixes #22616 --- ...ntityFrameworkRelationalServicesBuilder.cs | 2 +- .../Migrations/IMigrationCommandExecutor.cs | 7 +- .../Internal/MigrationCommandExecutor.cs | 202 ++++++++---- .../Migrations/Internal/Migrator.cs | 26 +- .../Properties/RelationalStrings.Designer.cs | 2 +- .../Properties/RelationalStrings.resx | 2 +- .../SqliteServiceCollectionExtensions.cs | 5 +- ...ityFrameworkServiceCollectionExtensions.cs | 4 +- .../Design/MigrationScaffolderTest.cs | 5 +- .../MigrationCommandExecutorTest.cs | 39 ++- .../FakeRelationalOptionsExtension.cs | 7 +- .../MigrationsInfrastructureSqlServerTest.cs | 309 +++++++++++++----- .../TestSqlServerRetryingExecutionStrategy.cs | 3 +- .../MigrationsInfrastructureSqliteTest.cs | 28 -- 14 files changed, 419 insertions(+), 222 deletions(-) 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 b3ef67f076b..055fa992ac6 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; @@ -383,6 +384,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]; @@ -396,7 +398,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) @@ -407,7 +411,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(); @@ -417,11 +430,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) @@ -461,13 +474,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 5b6a4abfce9..76cb6123a68 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 4e725fbe83e..f94806c31ba 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.resx +++ b/src/EFCore.Relational/Properties/RelationalStrings.resx @@ -1209,7 +1209,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/src/EFCore/Extensions/EntityFrameworkServiceCollectionExtensions.cs b/src/EFCore/Extensions/EntityFrameworkServiceCollectionExtensions.cs index 1708ac969f7..ec8c8bd78f2 100644 --- a/src/EFCore/Extensions/EntityFrameworkServiceCollectionExtensions.cs +++ b/src/EFCore/Extensions/EntityFrameworkServiceCollectionExtensions.cs @@ -1041,7 +1041,7 @@ public static IServiceCollection AddPooledDbContextFactory /// /// /// , - /// , + /// , /// or /// /// must also be called for the specified configuration to take effect. @@ -1077,7 +1077,7 @@ public static IServiceCollection ConfigureDbContext /// /// /// , - /// , + /// , /// or /// /// must also be called for the specified configuration to take effect. 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..f428f1e65c9 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()) @@ -122,17 +122,15 @@ public async Task Executes_transaction_suppressed_migration_commands_in_user_tra { Assert.Equal( RelationalStrings.TransactionSuppressedMigrationInUserTransaction, - (await Assert.ThrowsAsync( - async () - => await migrationCommandExecutor.ExecuteNonQueryAsync(commandList, fakeConnection))).Message); + (await Assert.ThrowsAsync(async () + => await migrationCommandExecutor.ExecuteNonQueryAsync(commandList, fakeConnection))).Message); } else { Assert.Equal( RelationalStrings.TransactionSuppressedMigrationInUserTransaction, - Assert.Throws( - () - => migrationCommandExecutor.ExecuteNonQuery(commandList, fakeConnection)).Message); + Assert.Throws(() + => migrationCommandExecutor.ExecuteNonQuery(commandList, fakeConnection)).Message); } tx.Rollback(); @@ -166,7 +164,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 +199,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 +239,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 +281,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 +351,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 +373,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; } diff --git a/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsInfrastructureSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsInfrastructureSqlServerTest.cs index 195865951f0..bd54bca902e 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsInfrastructureSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsInfrastructureSqlServerTest.cs @@ -109,12 +109,6 @@ INSERT INTO [__EFMigrationsHistory] ([MigrationId], [ProductVersion]) VALUES (N'00000000000001_Migration1', N'7.0.0-test'); GO -COMMIT; -GO - -BEGIN TRANSACTION; -GO - EXEC sp_rename N'[Table1].[Foo]', N'Bar', 'COLUMN'; GO @@ -136,12 +130,6 @@ INSERT INTO [__EFMigrationsHistory] ([MigrationId], [ProductVersion]) INSERT INTO [__EFMigrationsHistory] ([MigrationId], [ProductVersion]) VALUES (N'00000000000003_Migration3', N'7.0.0-test'); -GO - -COMMIT; -GO - -BEGIN TRANSACTION; GO CREATE PROCEDURE [dbo].[GotoReproduction] @@ -170,12 +158,6 @@ INSERT INTO [__EFMigrationsHistory] ([MigrationId], [ProductVersion]) VALUES (N'00000000000004_Migration4', N'7.0.0-test'); GO -COMMIT; -GO - -BEGIN TRANSACTION; -GO - INSERT INTO Table1 (Id, Bar, Description) VALUES (-1, 3, 'Value With Empty Lines') @@ -185,12 +167,6 @@ INSERT INTO [__EFMigrationsHistory] ([MigrationId], [ProductVersion]) VALUES (N'00000000000005_Migration5', N'7.0.0-test'); GO -COMMIT; -GO - -BEGIN TRANSACTION; -GO - INSERT INTO Table1 (Id, Bar, Description) VALUES (-2, 4, 'GO Value With @@ -201,12 +177,6 @@ INSERT INTO [__EFMigrationsHistory] ([MigrationId], [ProductVersion]) VALUES (N'00000000000006_Migration6', N'7.0.0-test'); GO -COMMIT; -GO - -BEGIN TRANSACTION; -GO - INSERT INTO Table1 (Id, Bar, Description) VALUES (-3, 5, 'GO Value With @@ -433,12 +403,6 @@ INSERT INTO [__EFMigrationsHistory] ([MigrationId], [ProductVersion]) END; GO -COMMIT; -GO - -BEGIN TRANSACTION; -GO - IF NOT EXISTS ( SELECT * FROM [__EFMigrationsHistory] WHERE [MigrationId] = N'00000000000002_Migration2' @@ -492,12 +456,6 @@ INSERT INTO [__EFMigrationsHistory] ([MigrationId], [ProductVersion]) END; GO -COMMIT; -GO - -BEGIN TRANSACTION; -GO - IF NOT EXISTS ( SELECT * FROM [__EFMigrationsHistory] WHERE [MigrationId] = N'00000000000004_Migration4' @@ -536,12 +494,6 @@ INSERT INTO [__EFMigrationsHistory] ([MigrationId], [ProductVersion]) END; GO -COMMIT; -GO - -BEGIN TRANSACTION; -GO - IF NOT EXISTS ( SELECT * FROM [__EFMigrationsHistory] WHERE [MigrationId] = N'00000000000005_Migration5' @@ -563,12 +515,6 @@ INSERT INTO [__EFMigrationsHistory] ([MigrationId], [ProductVersion]) END; GO -COMMIT; -GO - -BEGIN TRANSACTION; -GO - IF NOT EXISTS ( SELECT * FROM [__EFMigrationsHistory] WHERE [MigrationId] = N'00000000000006_Migration6' @@ -591,12 +537,6 @@ INSERT INTO [__EFMigrationsHistory] ([MigrationId], [ProductVersion]) END; GO -COMMIT; -GO - -BEGIN TRANSACTION; -GO - IF NOT EXISTS ( SELECT * FROM [__EFMigrationsHistory] WHERE [MigrationId] = N'00000000000007_Migration7' @@ -860,12 +800,6 @@ DELETE FROM [__EFMigrationsHistory] WHERE [MigrationId] = N'00000000000002_Migration2'; GO -COMMIT; -GO - -BEGIN TRANSACTION; -GO - DROP TABLE [Table1]; GO @@ -910,12 +844,6 @@ DELETE FROM [__EFMigrationsHistory] END; GO -COMMIT; -GO - -BEGIN TRANSACTION; -GO - IF EXISTS ( SELECT * FROM [__EFMigrationsHistory] WHERE [MigrationId] = N'00000000000001_Migration1' @@ -1040,14 +968,204 @@ public async Task Empty_Migration_Creates_Database() Fixture.TestStore.AddProviderOptions( new DbContextOptionsBuilder().EnableServiceProviderCaching(false)) .ConfigureWarnings(e => e.Log(RelationalEventId.PendingModelChangesWarning)).Options); + + context.Database.EnsureDeleted(); + GiveMeSomeTime(context); + var creator = (SqlServerDatabaseCreator)context.GetService(); creator.RetryTimeout = TimeSpan.FromMinutes(10); - await context.Database.MigrateAsync(); + await context.Database.MigrateAsync(null, "Empty"); Assert.True(creator.Exists()); } + [ConditionalFact] + public void Non_transactional_migration_is_retried() + { + using var context = new BloggingContext( + Fixture.TestStore.AddProviderOptions( + new DbContextOptionsBuilder().EnableServiceProviderCaching(false)) + .ConfigureWarnings(e => e.Log(RelationalEventId.PendingModelChangesWarning)) + .UseLoggerFactory(Fixture.TestSqlLoggerFactory).Options); + + context.Database.EnsureDeleted(); + GiveMeSomeTime(context); + + Fixture.TestSqlLoggerFactory.Clear(); + + var creator = (SqlServerDatabaseCreator)context.GetService(); + creator.RetryTimeout = TimeSpan.FromMinutes(10); + + context.Database.Migrate(); + + Assert.Equal( + """ +CREATE DATABASE [MigrationsTest]; + +IF SERVERPROPERTY('EngineEdition') <> 5 +BEGIN + ALTER DATABASE [MigrationsTest] SET READ_COMMITTED_SNAPSHOT ON; +END; + +SELECT 1 + +@LockTimeout='?' (DbType = Double) + +DECLARE @result int; +EXEC @result = sp_getapplock @Resource = '__EFMigrationsLock', @LockOwner = 'Session', @LockMode = 'Exclusive', @LockTimeout = @LockTimeout; +SELECT @result + +SELECT OBJECT_ID(N'[__EFMigrationsHistory]'); + +CREATE TABLE [__EFMigrationsHistory] ( + [MigrationId] nvarchar(150) NOT NULL, + [ProductVersion] nvarchar(32) NOT NULL, + CONSTRAINT [PK___EFMigrationsHistory] PRIMARY KEY ([MigrationId]) +); + +SELECT OBJECT_ID(N'[__EFMigrationsHistory]'); + +SELECT [MigrationId], [ProductVersion] +FROM [__EFMigrationsHistory] +ORDER BY [MigrationId]; + +INSERT INTO [__EFMigrationsHistory] ([MigrationId], [ProductVersion]) +VALUES (N'00000000000000_Empty', N'9.0.0-dev'); + +--Before + +IF OBJECT_ID(N'Blogs', N'U') IS NULL +BEGIN + CREATE TABLE [Blogs] ( + [Id] int NOT NULL, + [Name] nvarchar(max) NOT NULL, + CONSTRAINT [PK_Blogs] PRIMARY KEY ([Id]) + ); + + THROW 65536, 'Test', 0; +END + +IF OBJECT_ID(N'Blogs', N'U') IS NULL +BEGIN + CREATE TABLE [Blogs] ( + [Id] int NOT NULL, + [Name] nvarchar(max) NOT NULL, + CONSTRAINT [PK_Blogs] PRIMARY KEY ([Id]) + ); + + THROW 65536, 'Test', 0; +END + +INSERT INTO [__EFMigrationsHistory] ([MigrationId], [ProductVersion]) +VALUES (N'00000000000001_Migration1', N'9.0.0-dev'); + +--After + +INSERT INTO [__EFMigrationsHistory] ([MigrationId], [ProductVersion]) +VALUES (N'00000000000002_Migration2', N'9.0.0-dev'); + +DECLARE @result int; +EXEC @result = sp_releaseapplock @Resource = '__EFMigrationsLock', @LockOwner = 'Session'; +SELECT @result +""", + Fixture.TestSqlLoggerFactory.Sql, + ignoreLineEndingDifferences: true); + } + + [ConditionalFact] + public async Task Non_transactional_migration_is_retried_async() + { + using var context = new BloggingContext( + Fixture.TestStore.AddProviderOptions( + new DbContextOptionsBuilder().EnableServiceProviderCaching(false)) + .ConfigureWarnings(e => e.Log(RelationalEventId.PendingModelChangesWarning)) + .UseLoggerFactory(Fixture.TestSqlLoggerFactory).Options); + + context.Database.EnsureDeleted(); + GiveMeSomeTime(context); + + Fixture.TestSqlLoggerFactory.Clear(); + + var creator = (SqlServerDatabaseCreator)context.GetService(); + creator.RetryTimeout = TimeSpan.FromMinutes(10); + + await context.Database.MigrateAsync(); + + Assert.Equal( + """ +CREATE DATABASE [MigrationsTest]; + +IF SERVERPROPERTY('EngineEdition') <> 5 +BEGIN + ALTER DATABASE [MigrationsTest] SET READ_COMMITTED_SNAPSHOT ON; +END; + +SELECT 1 + +@LockTimeout='?' (DbType = Double) + +DECLARE @result int; +EXEC @result = sp_getapplock @Resource = '__EFMigrationsLock', @LockOwner = 'Session', @LockMode = 'Exclusive', @LockTimeout = @LockTimeout; +SELECT @result + +SELECT OBJECT_ID(N'[__EFMigrationsHistory]'); + +CREATE TABLE [__EFMigrationsHistory] ( + [MigrationId] nvarchar(150) NOT NULL, + [ProductVersion] nvarchar(32) NOT NULL, + CONSTRAINT [PK___EFMigrationsHistory] PRIMARY KEY ([MigrationId]) +); + +SELECT OBJECT_ID(N'[__EFMigrationsHistory]'); + +SELECT [MigrationId], [ProductVersion] +FROM [__EFMigrationsHistory] +ORDER BY [MigrationId]; + +INSERT INTO [__EFMigrationsHistory] ([MigrationId], [ProductVersion]) +VALUES (N'00000000000000_Empty', N'9.0.0-dev'); + +--Before + +IF OBJECT_ID(N'Blogs', N'U') IS NULL +BEGIN + CREATE TABLE [Blogs] ( + [Id] int NOT NULL, + [Name] nvarchar(max) NOT NULL, + CONSTRAINT [PK_Blogs] PRIMARY KEY ([Id]) + ); + + THROW 65536, 'Test', 0; +END + +IF OBJECT_ID(N'Blogs', N'U') IS NULL +BEGIN + CREATE TABLE [Blogs] ( + [Id] int NOT NULL, + [Name] nvarchar(max) NOT NULL, + CONSTRAINT [PK_Blogs] PRIMARY KEY ([Id]) + ); + + THROW 65536, 'Test', 0; +END + +INSERT INTO [__EFMigrationsHistory] ([MigrationId], [ProductVersion]) +VALUES (N'00000000000001_Migration1', N'9.0.0-dev'); + +--After + +INSERT INTO [__EFMigrationsHistory] ([MigrationId], [ProductVersion]) +VALUES (N'00000000000002_Migration2', N'9.0.0-dev'); + +DECLARE @result int; +EXEC @result = sp_releaseapplock @Resource = '__EFMigrationsLock', @LockOwner = 'Session'; +SELECT @result +""", + Fixture.TestSqlLoggerFactory.Sql, + ignoreLineEndingDifferences: true); + } + private class BloggingContext(DbContextOptions options) : DbContext(options) { // ReSharper disable once UnusedMember.Local @@ -1066,13 +1184,53 @@ public class Blog [DbContext(typeof(BloggingContext))] [Migration("00000000000000_Empty")] - public class EmptyMigration : Migration + private class EmptyMigration : Migration { protected override void Up(MigrationBuilder migrationBuilder) { } } + [DbContext(typeof(BloggingContext))] + [Migration("00000000000001_Migration1")] + private class BloggingMigration1 : Migration + { + protected override void Up(MigrationBuilder migrationBuilder) + { + migrationBuilder.Sql("--Before", suppressTransaction: true); + migrationBuilder.Sql(""" +IF OBJECT_ID(N'Blogs', N'U') IS NULL +BEGIN + CREATE TABLE [Blogs] ( + [Id] int NOT NULL, + [Name] nvarchar(max) NOT NULL, + CONSTRAINT [PK_Blogs] PRIMARY KEY ([Id]) + ); + + THROW 65536, 'Test', 0; +END +""", suppressTransaction: true); + } + + protected override void Down(MigrationBuilder migrationBuilder) + { + } + } + + [DbContext(typeof(BloggingContext))] + [Migration("00000000000002_Migration2")] + private class BloggingMigration2 : Migration + { + protected override void Up(MigrationBuilder migrationBuilder) + { + migrationBuilder.Sql("--After"); + } + + protected override void Down(MigrationBuilder migrationBuilder) + { + } + } + public override void Can_diff_against_2_2_model() { using var context = new ModelSnapshot22.BloggingContext(); @@ -1925,6 +2083,9 @@ public override MigrationsContext CreateContext() .Options; return new MigrationsContext(options); } + + protected override bool ShouldLogCategory(string logCategory) + => base.ShouldLogCategory(logCategory); } } } diff --git a/test/EFCore.SqlServer.FunctionalTests/TestUtilities/TestSqlServerRetryingExecutionStrategy.cs b/test/EFCore.SqlServer.FunctionalTests/TestUtilities/TestSqlServerRetryingExecutionStrategy.cs index 3c9b11c92da..ec517ff802f 100644 --- a/test/EFCore.SqlServer.FunctionalTests/TestUtilities/TestSqlServerRetryingExecutionStrategy.cs +++ b/test/EFCore.SqlServer.FunctionalTests/TestUtilities/TestSqlServerRetryingExecutionStrategy.cs @@ -14,7 +14,8 @@ public class TestSqlServerRetryingExecutionStrategy : SqlServerRetryingExecution -1, // Physical connection is not usable -2, // Timeout 42008, // Mirroring (Only when a database is deleted and another one is created in fast succession) - 42019 // CREATE DATABASE operation failed + 42019, // CREATE DATABASE operation failed + 65536, // Used for testing ]; public TestSqlServerRetryingExecutionStrategy() diff --git a/test/EFCore.Sqlite.FunctionalTests/Migrations/MigrationsInfrastructureSqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/Migrations/MigrationsInfrastructureSqliteTest.cs index 54e0caea3a0..26799421608 100644 --- a/test/EFCore.Sqlite.FunctionalTests/Migrations/MigrationsInfrastructureSqliteTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/Migrations/MigrationsInfrastructureSqliteTest.cs @@ -68,33 +68,17 @@ public override void Can_generate_up_scripts() INSERT INTO "__EFMigrationsHistory" ("MigrationId", "ProductVersion") VALUES ('00000000000001_Migration1', '7.0.0-test'); -COMMIT; - -BEGIN TRANSACTION; - ALTER TABLE "Table1" RENAME COLUMN "Foo" TO "Bar"; INSERT INTO "__EFMigrationsHistory" ("MigrationId", "ProductVersion") VALUES ('00000000000002_Migration2', '7.0.0-test'); -COMMIT; - -BEGIN TRANSACTION; - INSERT INTO "__EFMigrationsHistory" ("MigrationId", "ProductVersion") VALUES ('00000000000003_Migration3', '7.0.0-test'); -COMMIT; - -BEGIN TRANSACTION; - INSERT INTO "__EFMigrationsHistory" ("MigrationId", "ProductVersion") VALUES ('00000000000004_Migration4', '7.0.0-test'); -COMMIT; - -BEGIN TRANSACTION; - INSERT INTO Table1 (Id, Bar, Description) VALUES (-1, 3, 'Value With Empty Lines') @@ -102,10 +86,6 @@ INSERT INTO Table1 (Id, Bar, Description) VALUES (-1, 3, 'Value With INSERT INTO "__EFMigrationsHistory" ("MigrationId", "ProductVersion") VALUES ('00000000000005_Migration5', '7.0.0-test'); -COMMIT; - -BEGIN TRANSACTION; - INSERT INTO Table1 (Id, Bar, Description) VALUES (-2, 4, 'GO Value With @@ -114,10 +94,6 @@ Value With INSERT INTO "__EFMigrationsHistory" ("MigrationId", "ProductVersion") VALUES ('00000000000006_Migration6', '7.0.0-test'); -COMMIT; - -BEGIN TRANSACTION; - INSERT INTO Table1 (Id, Bar, Description) VALUES (-3, 5, 'GO Value With @@ -261,10 +237,6 @@ public override void Can_generate_down_scripts() DELETE FROM "__EFMigrationsHistory" WHERE "MigrationId" = '00000000000002_Migration2'; -COMMIT; - -BEGIN TRANSACTION; - DROP TABLE "Table1"; DELETE FROM "__EFMigrationsHistory"