search

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.79k stars 3.19k forks source link

Non-deterministic user seed data in HasData() causes constant "model changed" warnings #35080

Open Drago95 opened 2 days ago

Drago95 commented 2 days ago

Repro Steps: 1) Create initial migration via command line (dotnet ef migrations add "AddDiaryEntryTable" --project .\DiaryApp\DiaryApp.csproj --context ApplicationDbContext) 2) Apply migration to database (dotnet ef database update --project .\DiaryApp\DiaryApp.csproj) 3) Uncomment OnModelCreating in ApplicationDbContext 4) Create a new migration (dotnet ef migrations add "AddedSeedingDataDiaryEntry" --project .\DiaryApp\DiaryApp.csproj --context ApplicationDbContext) 5) Try to apply the migration to database - get error

dotnet exec --depsfile "C:\Source\repos\Udemy\.NET 9\Diary\DiaryApp\bin\Debug\net9.0\DiaryApp.deps.json" --additionalprobingpath C:\Users\dan_fischer.CORP\.nuget\packages --additionalprobingpath "C:\Program Files (x86)\Microsoft Visual Studio\Shared\NuGetPack
ages" --runtimeconfig "C:\Source\repos\Udemy\.NET 9\Diary\DiaryApp\bin\Debug\net9.0\DiaryApp.runtimeconfig.json" C:\Users\dan_fischer.CORP\.dotnet\tools\.store\dotnet-ef\9.0.0-rc.2.24474.1\dotnet-ef\9.0.0-rc.2.24474.1\tools\net8.0\any\tools\netcoreapp2.0\any\
ef.dll database update --assembly "C:\Source\repos\Udemy\.NET 9\Diary\DiaryApp\bin\Debug\net9.0\DiaryApp.dll" --project "C:\Source\repos\Udemy\.NET 9\Diary\DiaryApp\DiaryApp.csproj" --startup-assembly "C:\Source\repos\Udemy\.NET 9\Diary\DiaryApp\bin\Debug\net
9.0\DiaryApp.dll" --startup-project "C:\Source\repos\Udemy\.NET 9\Diary\DiaryApp\DiaryApp.csproj" --project-dir "C:\Source\repos\Udemy\.NET 9\Diary\DiaryApp\\" --root-namespace DiaryApp --language C# --framework net9.0 --nullable --working-dir "C:\Source\repos\Udemy\.NET 9\Diary" --verbose
Using assembly 'DiaryApp'.
Using startup assembly 'DiaryApp'.
Using application base 'C:\Source\repos\Udemy\.NET 9\Diary\DiaryApp\bin\Debug\net9.0'.
Using working directory 'C:\Source\repos\Udemy\.NET 9\Diary\DiaryApp'.
Using root namespace 'DiaryApp'.
Using project directory 'C:\Source\repos\Udemy\.NET 9\Diary\DiaryApp\'.
Remaining arguments: .
Finding DbContext classes...
Finding IDesignTimeDbContextFactory implementations...
Finding DbContext classes in the project...
Found DbContext 'ApplicationDbContext'.
Finding application service provider in assembly 'DiaryApp'...
Finding Microsoft.Extensions.Hosting service provider...
Using environment 'Development'.
Using application service provider from Microsoft.Extensions.Hosting.
Using context 'ApplicationDbContext'.
Finding design-time services referenced by assembly 'DiaryApp'...
Finding design-time services referenced by assembly 'DiaryApp'...
No referenced design-time services were found.
Finding design-time services for provider 'Microsoft.EntityFrameworkCore.SqlServer'...
Using design-time services from provider 'Microsoft.EntityFrameworkCore.SqlServer'.
Finding IDesignTimeServices implementations in assembly 'DiaryApp'...
No design-time services were found.
System.InvalidOperationException: An error was generated for warning 'Microsoft.EntityFrameworkCore.Migrations.PendingModelChangesWarning': The model for context 'ApplicationDbContext' has pending changes. Add a new migration before updating the database. This exception can be suppressed or logged by passing event ID 'RelationalEventId.PendingModelChangesWarning' to the 'ConfigureWarnings' method in 'DbContext.OnConfiguring' or 'AddDbContext'.
   at Microsoft.EntityFrameworkCore.Diagnostics.EventDefinition`1.Log[TLoggerCategory](IDiagnosticsLogger`1 logger, TParam arg)
   at Microsoft.EntityFrameworkCore.Diagnostics.RelationalLoggerExtensions.PendingModelChangesWarning(IDiagnosticsLogger`1 diagnostics, Type contextType)
   at Microsoft.EntityFrameworkCore.Migrations.Internal.Migrator.Migrate(String targetMigration)
   at Microsoft.EntityFrameworkCore.Design.Internal.MigrationsOperations.UpdateDatabase(String targetMigration, String connectionString, String contextType)
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.OperationBase.Execute(Action action)
An error was generated for warning 'Microsoft.EntityFrameworkCore.Migrations.PendingModelChangesWarning': The model for context 'ApplicationDbContext' has pending changes. Add a new migration before updating the database. This exception can be suppressed or logged by passing event ID 'RelationalEventId.PendingModelChangesWarning' to the 'ConfigureWarnings' method in 'DbContext.OnConfiguring' or 'AddDbContext'.

Diary.zip

EF Core version: 9.0.0 Database provider: (e.g. Microsoft.EntityFrameworkCore.SqlServer) Target framework: (e.g. .NET 9.0) Operating system: Windows 11 23H2 (Build 22631.4460) IDE: JetBrains Rider 2024.2.7

ajcvickers commented 2 days ago

Note for team: I investigated this and it is not really a regression. It is one of those quite common model diffing cases where each time the model is compared it is still different even after applying the migration. Often this is a bug, but in this case it is not a bug because the model data contains DateTime.Now:

modelBuilder.Entity<DiaryEntry>().HasData(
    new DiaryEntry
    {
        Id = 1,
        Title = "Went Hiking",
        Content = "Went hiking with Joe!",
        Created = DateTime.Now
    },

We should be aware that this type of issue will now show up as a pending model changes error.

@Drago95 When EF adds a new migration, it compares the data in the old model to that in the current model. If it has changed, then the data is updated in the database. However, in your case the data will change every time EF looks at it, since DateTime.Now will return a different value each time it is run. The DateTime should instead be a value that will not change from run to run unless that is explicitly intended.

Drago95 commented 1 day ago

Thanks - understood.

roji commented 1 day ago

I'm assuming there's not much we can do here (except in theory generate the model twice and compare the resulting seeding values, so that we can error if they're different - would we go to the trouble)?

Adding a note to the docs in https://github.com/dotnet/EntityFramework.Docs/pull/4878.

ajcvickers commented 1 day ago

@roji I think this is by-design, but I added the label to check opinions from the team, and to raise awareness that we will now start seeing this kind of error for these kinds of issue.

roji commented 1 day ago

Yeah... As a limitation it feels like it would be nice to alert the user that they're doing something wrong, but I'm not sure it's worth investing in HasData() at this point to add this error/warning.

AndriySvyryd commented 21 hours ago

We would only be able to detect this using an analyzer.

roji commented 20 hours ago

We can indeed do an analyzer specifically for DateTime.Now - that doesn't seem too hard, but I'm also not sure I'd want us to invest the effort in HasData() at this point. It would also obviously be specific (there are any number of other non-deterministic functions). I'll bring to design to see if we want to keep it.