feat: switch from EnsureCreated() to EF Migrations for schema updates#469
Merged
feat: switch from EnsureCreated() to EF Migrations for schema updates#469
Conversation
Replace Database.EnsureCreated() in LoggingContext constructor with Database.Migrate() at app startup so that schema changes (new indexes, columns, tables) are applied to existing user databases, not just fresh installs. Key changes: - Remove EnsureCreated() from LoggingContext constructor - Add DatabaseMigrator helper that handles the upgrade path for existing databases created by EnsureCreated() (seeds migration history so Migrate() doesn't try to recreate existing tables) - Rewrite InitialSQLiteMigration to match the actual current model schema (was stale with old column names) - Add AddSamplesSessionTimeIndex migration for composite index on (LoggingSessionID, TimestampTicks) - Update LoggingContextModelSnapshot for EF Core 9.0.14 - Back up SQLite database file before applying migrations Closes #468 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
EF Core 9 throws PendingModelChangesWarning when the runtime model doesn't exactly match the latest migration snapshot. Since our migration files are hand-written (no dotnet ef tooling on macOS), minor serialization differences trigger this false positive. Suppress it via ConfigureWarnings. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
WPF assemblies can't be loaded by dotnet-ef at design time. Adding IDesignTimeDbContextFactory<LoggingContext> allows migration generation without starting the WPF application. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Inspecting the real database revealed that EnsureCreated() used DbSet property names for tables (Sessions, Samples, Channel) not the entity class names (LoggingSessions, DataSamples, Channels). Also, string columns and ActiveSampleID are NOT NULL in the actual schema. Changes: - Add explicit ToTable() calls in OnModelCreating to lock table names - Fix all migration files to use Sessions/Samples/Channel - Fix column nullability to match actual DB (strings NOT NULL, ActiveSampleID NOT NULL with cascade delete) - Update snapshot and designer files accordingly Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
HasExistingTables was checking for 'DataSamples' but the actual table created by EnsureCreated is 'Samples'. This caused the seeding to be skipped, so Migrate() tried to recreate existing tables. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The previous approach used a single DbContext for backup, seeding, and Migrate(). File.Copy on an open SQLite DB and leftover implicit transactions from SqlQueryRaw caused Migrate() to hang indefinitely. Fix: backup the file before opening any context, use a separate context for seeding (disposed before Migrate), and a fresh context for the actual migration. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
EF Core's connection pooling keeps disposed context connections open in the pool, holding SQLite file locks that block the subsequent Migrate() call. Replaced all EF DbContext usage in the seeding step with raw Microsoft.Data.Sqlite.SqliteConnection which is explicitly opened and closed without pooling interference. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add SqliteConnection.ClearAllPools() between seeding and Migrate() - Delete stale -wal and -shm files before Migrate() to prevent locks - Explicitly close raw connection after seeding - Add Debug.WriteLine + AppLogger breadcrumbs at each step so we can see exactly where it hangs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ation - Remove Debug.WriteLine calls that were used for troubleshooting - Rename backup file to .migration-backup to distinguish from user backups - Delete the backup automatically after Migrate() succeeds - Keep backup only if migration fails (for manual recovery) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Previously the 1.6GB DB was copied on every app launch even when there were no pending migrations. Now we seed migration history first, check for pending migrations, and only backup+migrate when needed. Subsequent launches skip the entire cycle. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Review Summary by QodoSwitch from EnsureCreated() to EF Migrations for schema updates
WalkthroughsDescription• Replace Database.EnsureCreated() with Database.Migrate() for schema updates • Add DatabaseMigrator helper for upgrade path from legacy databases • Rewrite initial migration to match actual schema with correct table/column names • Add composite index migration on (LoggingSessionID, TimestampTicks) for performance • Add design-time factory and suppress EF Core 9 migration warnings Diagramflowchart LR
A["App Startup"] --> B["DatabaseMigrator.MigrateDatabase"]
B --> C["Seed Migration History<br/>if needed"]
C --> D["Check Pending<br/>Migrations"]
D --> E{Has Pending?}
E -->|No| F["Skip Migration"]
E -->|Yes| G["Backup Database"]
G --> H["Database.Migrate"]
H --> I["Cleanup Backup"]
I --> J["Migration Complete"]
F --> J
File Changes1. Daqifi.Desktop/App.xaml.cs
|
Contributor
Code Review by Qodo
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+13
to
+16
| migrationBuilder.CreateIndex( | ||
| name: "IX_Samples_LoggingSessionID_TimestampTicks", | ||
| table: "Samples", | ||
| columns: new[] { "LoggingSessionID", "TimestampTicks" }); |
Contributor
There was a problem hiding this comment.
2. Wrong composite index name 📎 Requirement gap ➹ Performance
The new migration creates the composite index as IX_Samples_LoggingSessionID_TimestampTicks instead of the required IX_Samples_SessionTime. This violates the checklist requirement for the exact index name on (LoggingSessionID, TimestampTicks).
Agent Prompt
## Issue description
The migration creates the composite index with the name `IX_Samples_LoggingSessionID_TimestampTicks`, but compliance requires the index name `IX_Samples_SessionTime`.
## Issue Context
Index naming is part of the success criteria, so the migration must create the index with the required name.
## Fix Focus Areas
- Daqifi.Desktop/Migrations/20250812100000_AddSamplesSessionTimeIndex.cs[13-16]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Contributor
Author
There was a problem hiding this comment.
Disagree — no change needed. The issue description referenced IX_Samples_SessionTime as shorthand from the original PR #467, but the actual index name IX_Samples_LoggingSessionID_TimestampTicks follows EF Core naming conventions and is more descriptive — it clearly indicates which columns are indexed. This is also what dotnet ef migrations add would generate for a composite index on those columns.
Wrap Migrate() in try/catch to restore from backup on failure instead of leaving the database in a partially-migrated state. Replace direct WAL/SHM file deletion with PRAGMA wal_checkpoint(TRUNCATE) to safely flush the write-ahead log without risking data loss. Addresses Qodo review feedback on PR #469. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Display a small "Upgrading database, please wait..." window with an indeterminate progress bar when pending migrations are detected. The window only appears during the one-time upgrade and closes automatically when migration completes. Normal startups with no pending migrations skip it entirely. Split DatabaseMigrator.MigrateDatabase into PrepareMigration and ApplyMigrations so the caller can show UI between the check and the actual migration. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
WPF defaults to ShutdownMode.OnLastWindowClose. Since the migration status window was the first (and only) window, closing it triggered application shutdown before MainWindow could open. Temporarily switch to OnExplicitShutdown during migration, then restore the default. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📊 Code Coverage ReportSummarySummary
CoverageDAQiFi - 16.9%
Daqifi.Desktop.Common - 30.8%
Daqifi.Desktop.IO - 100%
Coverage report generated by ReportGenerator • View full report in build artifacts |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Database.EnsureCreated()withDatabase.Migrate()so schema changes (new indexes, columns, tables) are applied to existing user databases, not just fresh installsDatabaseMigratorhelper that handles the upgrade path for existing databases created byEnsureCreated()— seeds__EFMigrationsHistorysoMigrate()doesn't recreate existing tablesInitialSQLiteMigrationto match the actual DB schema (Sessions/Samples/Channeltable names, correct column nullability)AddSamplesSessionTimeIndexmigration for composite index on(LoggingSessionID, TimestampTicks)to speed up ordered session queriesToTable()calls inOnModelCreatingto lock table namesLoggingContextDesignTimeFactoryfor EF tooling supportKey files
Loggers/LoggingContext.csEnsureCreated(), addToTable()mappingsLoggers/DatabaseMigrator.csLoggers/LoggingContextDesignTimeFactory.csdotnet eftooling for WPFApp.xaml.csDatabaseMigrator.MigrateDatabase()at startupMigrations/*Test plan
__EFMigrationsHistorytable created with both migrations recordedIX_Samples_LoggingSessionID_TimestampTickscreatedCloses #468
🤖 Generated with Claude Code