dotnet / efcore

EF Core is a modern object-database mapper for .NET. It supports LINQ queries, change tracking, updates, and schema migrations.
https://docs.microsoft.com/ef/
MIT License
13.82k stars 3.2k forks source link

Add safeguard for MigrateAsync with targetMigration #34819

Open Ben555555 opened 1 month ago

Ben555555 commented 1 month ago

We are using the following code to run each migration one by one, so we can better determine which migration failed:

                var pendingMigrations = await dbContext.Database.GetPendingMigrationsAsync(cancellationToken);

                var migrator = dbContext.Database.GetInfrastructure().GetRequiredService<IMigrator>();

                foreach (string migration in pendingMigrations)
                {
                    logger.LogInformation($"Applying migration '{migration}'...");

                    await migrator.MigrateAsync(migration, cancellationToken);
                }

Recently when we deployed an application we noticed an unfamiliar behaviour. A field disappeared and was gone after the migrations were executed. It turned out that for one migration, the Down-Migration was called, because that migration was already deployed and had a higher timestamp in the filename than the pending migrations. This happened because the migration was added on a branch that was going live, while on a dev branch already new migrations were added before. So when the first pending migration was executed via IMigrator.MigrateAsync, the down migration of that already deployed migration was called. As far as I understand this is to make sure that the migrations always run on the "correct" state of the database. In our case this caused an issue because the Down-Migration was called, but the Up-Migration was never called again. I assume this is because we use the migrator to explicitly execute migrations and it doesn't know when the last migration was done. I also assume this works correctly with the usual Database.MigrateAsync() method.

So my questions are:

Include provider and version information

EF Core version: Database provider: Microsoft.EntityFrameworkCore.SqlServer Target framework: .NET 8.0 Operating system: Windows 11 IDE: Visual Studio 2022

AndriySvyryd commented 1 month ago

Is it even safe to use IMigrator.MigrateAsync, when it can implicitly call down migrations but in the end will never call the Up migration again?

It isn't safe to do it like in the code snippet provided. MigrateAsync was designed to be called once per session, with the target migration you'd want as the final state for your database.

We can add a parameter that would make Migrate throw if any migration would be reverted or if a migration to be applied is older than the newest applied migration.

Also, there are already logging events for when each migration is applied.

Is there a way we can ensure that the Up-Migration is executed in the end?

MigrateAsync will only call Up or Down for any particular migration in a single call. The logic it uses is fairly straightforward:

Is this behaviour even documented somewhere? It took a lot of time to figure out why a field was deleted.

No, filed https://github.com/dotnet/EntityFramework.Docs/issues/4824