diff --git a/backend/internal/store/db.go b/backend/internal/store/db.go index 406a276..425a306 100644 --- a/backend/internal/store/db.go +++ b/backend/internal/store/db.go @@ -24,10 +24,7 @@ type DB struct { // Open opens (creating if needed) the SQLite database at path and applies // any migrations that haven't run yet. func Open(path string) (*DB, error) { - // Don't enable foreign keys in connection string; instead, enable them - // after migrations complete. This allows migrations to disable FK checks - // as needed for table rebuilds. - sqlDB, err := sql.Open("sqlite", path) + sqlDB, err := sql.Open("sqlite", path+"?_pragma=foreign_keys(1)") if err != nil { return nil, fmt.Errorf("open sqlite database: %w", err) } @@ -81,38 +78,62 @@ func (db *DB) migrate() error { if err != nil { return fmt.Errorf("read migration %s: %w", name, err) } + + // Migrations 0023, 0024, and 0025 rebuild tables that have incoming + // foreign keys (kind_assignments/workout_type_paces reference + // workout_kinds; sync_state is referenced by activities/sync_runs). + // These require temporary FK disable in autocommit mode (before the + // transaction begins) so the DROP TABLE succeeds; a mid-transaction + // PRAGMA is a no-op with modernc.org/sqlite. + tableRebuildMigrations := map[string]bool{ + "0023_profile_user_scoped.sql": true, + "0024_workout_kinds_user_scoped.sql": true, + "0025_sync_state_user_scoped.sql": true, + } + needsFKToggle := tableRebuildMigrations[name] + + if needsFKToggle { + if _, err := db.Exec(`PRAGMA foreign_keys = OFF`); err != nil { + return fmt.Errorf("disable foreign keys before migration %s: %w", name, err) + } + } + tx, err := db.Begin() if err != nil { + if needsFKToggle { + db.Exec(`PRAGMA foreign_keys = ON`) + } return fmt.Errorf("begin migration tx for %s: %w", name, err) } - // Disable foreign key enforcement for the migration so that table - // rebuilds (e.g. dropping a table with dependent foreign keys) can - // succeed. The migrations themselves use PRAGMA defer_foreign_keys - // when possible; this is a fallback for modernc.org/sqlite's - // limitations with that pragma in transactions. - if _, err := tx.Exec(`PRAGMA foreign_keys = OFF`); err != nil { - tx.Rollback() - return fmt.Errorf("disable foreign keys for migration %s: %w", name, err) - } + if _, err := tx.Exec(string(content)); err != nil { tx.Rollback() + if needsFKToggle { + db.Exec(`PRAGMA foreign_keys = ON`) + } return fmt.Errorf("apply migration %s: %w", name, err) } - if _, err := tx.Exec(`PRAGMA foreign_keys = ON`); err != nil { - tx.Rollback() - return fmt.Errorf("enable foreign keys after migration %s: %w", name, err) - } + if _, err := tx.Exec(`INSERT INTO schema_migrations (filename) VALUES (?)`, name); err != nil { tx.Rollback() + if needsFKToggle { + db.Exec(`PRAGMA foreign_keys = ON`) + } return fmt.Errorf("record migration %s: %w", name, err) } + if err := tx.Commit(); err != nil { + if needsFKToggle { + db.Exec(`PRAGMA foreign_keys = ON`) + } return fmt.Errorf("commit migration %s: %w", name, err) } - } - // Enable foreign key enforcement after all migrations are complete. - if _, err := db.Exec(`PRAGMA foreign_keys = ON`); err != nil { - return fmt.Errorf("enable foreign keys after migrations: %w", err) + + if needsFKToggle { + if _, err := db.Exec(`PRAGMA foreign_keys = ON`); err != nil { + return fmt.Errorf("enable foreign keys after migration %s: %w", name, err) + } + } } return nil } diff --git a/backend/internal/store/store_test.go b/backend/internal/store/store_test.go index 5222469..74a01dd 100644 --- a/backend/internal/store/store_test.go +++ b/backend/internal/store/store_test.go @@ -3,6 +3,7 @@ package store import ( "context" "path/filepath" + "strings" "testing" ) @@ -231,3 +232,71 @@ func TestUsersAndOwnershipSchema_AppliesCleanly(t *testing.T) { t.Fatal("expected a second profile row for the same user_id to violate UNIQUE(user_id)") } } + +func TestForeignKeyEnforcementPostMigration(t *testing.T) { + db := openTestDB(t) + ctx := context.Background() + + // Verify that FK enforcement is correctly active after migrations complete. + // This tests that the PRAGMA foreign_keys toggle in db.migrate() (for + // migrations 0023/0024/0025) is properly scoped and re-enabled after each + // table rebuild. + + // Insert an activity that we can reference. + activityID, err := db.UpsertActivity(ctx, Activity{ + GarminActivityID: 1001, + StartTimeUTC: "2026-07-11 05:00:00", + RawJSON: "{}", + }) + if err != nil { + t.Fatalf("UpsertActivity: %v", err) + } + + // Get the ID of one of the 8 migration-seeded workout_kinds. + var seedKindID int64 + if err := db.QueryRowContext(ctx, `SELECT id FROM workout_kinds LIMIT 1`).Scan(&seedKindID); err != nil { + t.Fatalf("query seeded workout_kind: %v", err) + } + + // Inserting a kind_assignment with a valid FK reference should succeed. + assignmentID, err := db.InsertKindAssignment(ctx, KindAssignment{ + ActivityID: activityID, + WorkoutKindID: &seedKindID, + AssignmentSource: AssignmentSourceManual, + Status: AssignmentStatusAssigned, + }) + if err != nil { + t.Fatalf("InsertKindAssignment with valid FK: %v", err) + } + if assignmentID == 0 { + t.Fatal("expected non-zero assignment ID") + } + + // Inserting a kind_assignment with a nonexistent workout_kind_id should + // be rejected by the FK constraint, proving FK enforcement is ON. + nonexistentKindID := int64(999999) + _, err = db.InsertKindAssignment(ctx, KindAssignment{ + ActivityID: activityID, + WorkoutKindID: &nonexistentKindID, + AssignmentSource: AssignmentSourceManual, + Status: AssignmentStatusAssigned, + }) + if err == nil { + t.Fatal("expected FK constraint violation for nonexistent workout_kind_id, but insert succeeded") + } + // The error message should mention FOREIGN KEY or similar constraint issue. + if !contains(err.Error(), "FOREIGN KEY", "constraint", "UNIQUE") { + t.Logf("FK error message: %v", err) + // Still pass, but log it; the exact error text varies by driver/context. + } +} + +func contains(s string, substrs ...string) bool { + lower := strings.ToLower(s) + for _, substr := range substrs { + if strings.Contains(lower, strings.ToLower(substr)) { + return true + } + } + return false +}