From cfbd69b24064ade4ba9dbe1e3d672c5fa7a27c6f Mon Sep 17 00:00:00 2001 From: Quick <31828688+Quick104@users.noreply.github.com> Date: Mon, 27 Jul 2026 23:56:03 +0000 Subject: [PATCH] feat(settings): run the one-time migration on the Postgres backend MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The mirror of userdb V15, registered with goose as a Go migration rather than SQL: the conversion validates every value against its own definition and re-encodes it as typed JSON, and one legacy quality string becomes two rows — neither is expressible in SQL without duplicating the manifest. The rules stay in internal/settingsmigrate, so the two backends cannot disagree. RunTx, so the whole backfill lands in goose's transaction. The down migration empties the canonical tables; the legacy ones are never touched by the up, which is what keeps the cutover reversible until the follow-up migration drops the superseded columns. preferred_metadata_language is read here and only here — the column exists in this schema and not in SQLite's, so this is the sole source for catalog.metadata_language. Verified against a real Postgres: the full goose chain runs, 1080p-high decomposes to ("1080p", 10000), values land as typed jsonb rather than strings (jsonb_typeof reports number), rejects carry a queryable jsonb identity, and the composite profile foreign key refuses a row naming a profile that does not exist. Co-Authored-By: Claude Opus 5 (1M context) --- internal/database/migrate.go | 4 + internal/database/settings_backfill.go | 318 ++++++++++++++++++++ internal/database/settings_backfill_test.go | 202 +++++++++++++ 3 files changed, 524 insertions(+) create mode 100644 internal/database/settings_backfill.go create mode 100644 internal/database/settings_backfill_test.go diff --git a/internal/database/migrate.go b/internal/database/migrate.go index 473936bc..35cdddbc 100644 --- a/internal/database/migrate.go +++ b/internal/database/migrate.go @@ -132,6 +132,10 @@ func newMigrationProvider(pool *pgxpool.Pool, fsys fs.FS, dir string) (*goose.Pr goose.WithTableName(gooseVersionTable), goose.WithAllowOutofOrder(true), goose.WithSessionLocker(&legacyBootstrapLocker{delegate: locker}), + // The settings backfill is Go rather than SQL: it validates every value + // against the contract and re-encodes it as typed JSON, which SQL + // cannot do without duplicating the manifest. + goose.WithGoMigrations(settingsBackfillMigration()), ) if err != nil { _ = sqlDB.Close() diff --git a/internal/database/settings_backfill.go b/internal/database/settings_backfill.go new file mode 100644 index 00000000..2a61f3ff --- /dev/null +++ b/internal/database/settings_backfill.go @@ -0,0 +1,318 @@ +package database + +import ( + "context" + "database/sql" + "fmt" + + "github.com/pressly/goose/v3" + + "github.com/Silo-Server/silo-server/internal/settingscontract" + "github.com/Silo-Server/silo-server/internal/settingsmigrate" +) + +// settingsBackfillVersion is the timestamp version this backfill occupies. It +// sorts immediately after 20260727010621_user_setting_values.sql, which creates +// the tables this fills. +const settingsBackfillVersion int64 = 20260727010622 + +// settingsBackfillMigration is the one-time conversion of legacy settings +// storage into user_setting_values. +// +// A Go migration rather than SQL because the conversion is not expressible in +// SQL without duplicating the contract: every value has to be validated against +// its own definition and re-encoded as typed JSON, and a legacy quality string +// decomposes into two rows. Those rules live in internal/settingsmigrate so +// this and the SQLite backend cannot disagree; this file reads rows, hands them +// over, and writes what comes back. +// +// RunTx, so the whole backfill lands in goose's transaction — a partial +// migration is the one state neither an operator's backup nor a rollback +// covers. +func settingsBackfillMigration() *goose.Migration { + return goose.NewGoMigration( + settingsBackfillVersion, + &goose.GoFunc{RunTx: backfillSettingValues}, + &goose.GoFunc{RunTx: rollbackSettingValues}, + ) +} + +// backfillSettingValues converts every user's legacy settings. +func backfillSettingValues(ctx context.Context, tx *sql.Tx) error { + contract, err := settingscontract.Load() + if err != nil { + return fmt.Errorf("loading settings contract: %w", err) + } + planner := settingsmigrate.New(contract, settingscontract.ObjectSchemas()) + + userIDs, err := settingsBackfillUserIDs(ctx, tx) + if err != nil { + return err + } + + for _, userID := range userIDs { + input, err := readLegacySettingsForUser(ctx, tx, userID) + if err != nil { + return fmt.Errorf("reading legacy settings for user %d: %w", userID, err) + } + result := planner.Plan(input) + + for _, row := range result.Rows { + if _, err := tx.ExecContext(ctx, ` +INSERT INTO user_setting_values + (user_id, key, scope, profile_id, device_id, library_id, series_id, value) +VALUES ($1, $2, $3, $4, $5, $6, $7, $8::jsonb)`, + userID, row.Key, string(row.Scope), + nullText(row.ProfileID), nullText(row.DeviceID), + nullInt(row.LibraryID), nullText(row.SeriesID), + string(row.Value), + ); err != nil { + return fmt.Errorf("writing %s at %s for user %d: %w", + row.Key, row.Scope, userID, err) + } + } + + for _, reject := range result.Rejects { + if _, err := tx.ExecContext(ctx, ` +INSERT INTO user_setting_migration_rejects + (user_id, source_table, source_key, identity, value, reason) +VALUES ($1, $2, $3, $4::jsonb, $5, $6)`, + userID, reject.SourceTable, reject.SourceKey, + string(reject.Identity), reject.Value, reject.Reason, + ); err != nil { + return fmt.Errorf("recording reject %s for user %d: %w", + reject.SourceKey, userID, err) + } + } + } + + return nil +} + +// rollbackSettingValues empties the canonical tables. +// +// The legacy tables are never modified by the up migration, so undoing it is +// simply discarding what was derived from them. This is what makes the cutover +// reversible before the follow-up migration drops the legacy columns. +func rollbackSettingValues(ctx context.Context, tx *sql.Tx) error { + for _, table := range []string{ + "user_setting_values", + "user_setting_migration_rejects", + } { + if _, err := tx.ExecContext(ctx, "DELETE FROM "+table); err != nil { + return fmt.Errorf("clearing %s: %w", table, err) + } + } + return nil +} + +// settingsBackfillUserIDs returns every user with something to migrate. +// +// A user with no settings and no profiles produces nothing, so they are skipped +// rather than queried five times each. +func settingsBackfillUserIDs(ctx context.Context, tx *sql.Tx) ([]int, error) { + rows, err := tx.QueryContext(ctx, ` +SELECT id FROM users + WHERE EXISTS (SELECT 1 FROM user_profiles p WHERE p.user_id = users.id) + OR EXISTS (SELECT 1 FROM user_settings s WHERE s.user_id = users.id) + ORDER BY id`) + if err != nil { + return nil, fmt.Errorf("listing users: %w", err) + } + defer rows.Close() //nolint:errcheck // read-only iteration + + var ids []int + for rows.Next() { + var id int + if err := rows.Scan(&id); err != nil { + return nil, fmt.Errorf("scanning user id: %w", err) + } + ids = append(ids, id) + } + return ids, rows.Err() +} + +// readLegacySettingsForUser gathers one user's legacy rows. +func readLegacySettingsForUser( + ctx context.Context, tx *sql.Tx, userID int, +) (settingsmigrate.Input, error) { + var input settingsmigrate.Input + + // Profiles. preferred_metadata_language exists only in this schema — the + // SQLite profiles table never had the column — so this is the sole source + // for catalog.metadata_language. + if err := eachRow(ctx, tx, ` +SELECT id, quality_preference, language, subtitle_language, subtitle_mode, + show_forced_subtitles, preferred_metadata_language + FROM user_profiles WHERE user_id = $1`, + func(scan func(...any) error) error { + var profile settingsmigrate.LegacyProfile + var quality, language, subtitle, mode, metadata sql.NullString + var forced sql.NullBool + if err := scan(&profile.ID, &quality, &language, &subtitle, + &mode, &forced, &metadata); err != nil { + return err + } + profile.QualityPreference = nullableString(quality) + profile.Language = nullableString(language) + profile.SubtitleLanguage = nullableString(subtitle) + profile.SubtitleMode = nullableString(mode) + profile.ShowForcedSubtitles = nullableBool(forced) + profile.PreferredMetadataLanguage = nullableString(metadata) + input.Profiles = append(input.Profiles, profile) + return nil + }, userID); err != nil { + return input, fmt.Errorf("reading user_profiles: %w", err) + } + + if err := eachRow(ctx, tx, + `SELECT key, value FROM user_settings WHERE user_id = $1`, + func(scan func(...any) error) error { + var row settingsmigrate.LegacySetting + if err := scan(&row.Key, &row.Value); err != nil { + return err + } + input.Settings = append(input.Settings, row) + return nil + }, userID); err != nil { + return input, fmt.Errorf("reading user_settings: %w", err) + } + + if err := eachRow(ctx, tx, ` +SELECT profile_id, device_id, key, value + FROM user_device_settings WHERE user_id = $1`, + func(scan func(...any) error) error { + var row settingsmigrate.LegacyDeviceSetting + if err := scan(&row.ProfileID, &row.DeviceID, &row.Key, &row.Value); err != nil { + return err + } + input.DeviceSettings = append(input.DeviceSettings, row) + return nil + }, userID); err != nil { + return input, fmt.Errorf("reading user_device_settings: %w", err) + } + + // Subtitle and audio preferences are keyed alike, so they merge into one + // per-series record; converting them independently would produce two rows + // racing for the same identity. + bySeries := map[[2]string]*settingsmigrate.LegacySeriesPreference{} + seriesRecord := func(profileID, seriesID string) *settingsmigrate.LegacySeriesPreference { + key := [2]string{profileID, seriesID} + if existing, ok := bySeries[key]; ok { + return existing + } + record := &settingsmigrate.LegacySeriesPreference{ProfileID: profileID, SeriesID: seriesID} + bySeries[key] = record + return record + } + + if err := eachRow(ctx, tx, ` +SELECT profile_id, series_id, subtitle_language, subtitle_mode, show_forced_subtitles + FROM user_subtitle_preferences WHERE user_id = $1`, + func(scan func(...any) error) error { + var profileID, seriesID string + var language, mode sql.NullString + var forced sql.NullBool + if err := scan(&profileID, &seriesID, &language, &mode, &forced); err != nil { + return err + } + record := seriesRecord(profileID, seriesID) + record.SubtitleLanguage = nullableString(language) + record.SubtitleMode = nullableString(mode) + record.ShowForcedSubtitles = nullableBool(forced) + return nil + }, userID); err != nil { + return input, fmt.Errorf("reading user_subtitle_preferences: %w", err) + } + + if err := eachRow(ctx, tx, ` +SELECT profile_id, series_id, audio_language + FROM user_audio_preferences WHERE user_id = $1`, + func(scan func(...any) error) error { + var profileID, seriesID string + var language sql.NullString + if err := scan(&profileID, &seriesID, &language); err != nil { + return err + } + seriesRecord(profileID, seriesID).AudioLanguage = nullableString(language) + return nil + }, userID); err != nil { + return input, fmt.Errorf("reading user_audio_preferences: %w", err) + } + + for _, record := range bySeries { + input.SeriesPrefs = append(input.SeriesPrefs, *record) + } + + if err := eachRow(ctx, tx, ` +SELECT profile_id, library_id, audio_language, subtitle_language, subtitle_mode, + show_forced_subtitles + FROM user_library_playback_preferences WHERE user_id = $1`, + func(scan func(...any) error) error { + var row settingsmigrate.LegacyLibraryPreference + var audio, subtitle, mode sql.NullString + var forced sql.NullBool + if err := scan(&row.ProfileID, &row.LibraryID, + &audio, &subtitle, &mode, &forced); err != nil { + return err + } + row.AudioLanguage = nullableString(audio) + row.SubtitleLanguage = nullableString(subtitle) + row.SubtitleMode = nullableString(mode) + row.ShowForcedSubtitles = nullableBool(forced) + input.LibraryPrefs = append(input.LibraryPrefs, row) + return nil + }, userID); err != nil { + return input, fmt.Errorf("reading user_library_playback_preferences: %w", err) + } + + return input, nil +} + +func eachRow( + ctx context.Context, tx *sql.Tx, query string, + fn func(scan func(...any) error) error, args ...any, +) error { + rows, err := tx.QueryContext(ctx, query, args...) + if err != nil { + return err + } + defer rows.Close() //nolint:errcheck // read-only iteration + + for rows.Next() { + if err := fn(rows.Scan); err != nil { + return err + } + } + return rows.Err() +} + +func nullableString(value sql.NullString) *string { + if !value.Valid { + return nil + } + text := value.String + return &text +} + +func nullableBool(value sql.NullBool) *bool { + if !value.Valid { + return nil + } + flag := value.Bool + return &flag +} + +func nullText(value string) any { + if value == "" { + return nil + } + return value +} + +func nullInt(value int) any { + if value == 0 { + return nil + } + return value +} diff --git a/internal/database/settings_backfill_test.go b/internal/database/settings_backfill_test.go new file mode 100644 index 00000000..19da344b --- /dev/null +++ b/internal/database/settings_backfill_test.go @@ -0,0 +1,202 @@ +package database + +import ( + "context" + "encoding/json" + "os" + "testing" + + "github.com/jackc/pgx/v5/pgxpool" + "github.com/jackc/pgx/v5/stdlib" + + "github.com/Silo-Server/silo-server/migrations" +) + +// TestPostgresSettingsBackfill runs the real goose provider — every SQL +// migration plus the Go backfill — against a real database, then checks what +// landed. +// +// The planner's rules are unit-tested in internal/settingsmigrate. What this +// covers is everything only a live database can show: that the Go migration is +// registered and actually runs, that the rows satisfy the scope CHECK, the +// composite profile foreign key and the five partial unique indexes, and that +// jsonb accepts the values the planner encodes. +func TestPostgresSettingsBackfill(t *testing.T) { + dsn := os.Getenv("SILO_TEST_DATABASE_URL") + if dsn == "" { + t.Skip("SILO_TEST_DATABASE_URL is not set") + } + ctx := context.Background() + + pool, err := pgxpool.New(ctx, dsn) + if err != nil { + t.Fatalf("connect test database: %v", err) + } + t.Cleanup(pool.Close) + + // Seed legacy state, then run migrations over it. Ordering matters: the + // backfill has to find rows that predate it, which is the real upgrade. + if err := RunMigrations(ctx, pool, migrations.FS, "sql"); err != nil { + t.Fatalf("initial migration: %v", err) + } + seedLegacyPostgresSettings(ctx, t, pool) + + // Re-run the backfill against the seeded data. It is idempotent only under + // goose's version gate, so this exercises it directly. + sqlDB := stdlib.OpenDBFromPool(pool) + t.Cleanup(func() { _ = sqlDB.Close() }) + + tx, err := sqlDB.BeginTx(ctx, nil) + if err != nil { + t.Fatalf("begin: %v", err) + } + if err := backfillSettingValues(ctx, tx); err != nil { + t.Fatalf("backfillSettingValues: %v", err) + } + if err := tx.Commit(); err != nil { + t.Fatalf("commit: %v", err) + } + + t.Run("profile columns become profile-scope values", func(t *testing.T) { + var value string + err := pool.QueryRow(ctx, ` +SELECT value::text FROM user_setting_values + WHERE key = 'playback.audio_language' AND scope = 'profile' AND profile_id = 'mp1'`). + Scan(&value) + if err != nil { + t.Fatalf("reading migrated audio language: %v", err) + } + if value != `"ja"` { + t.Errorf("audio language = %s, want \"ja\"", value) + } + }) + + t.Run("metadata language migrates from the postgres-only column", func(t *testing.T) { + var value string + err := pool.QueryRow(ctx, ` +SELECT value::text FROM user_setting_values + WHERE key = 'catalog.metadata_language' AND scope = 'profile' AND profile_id = 'mp1'`). + Scan(&value) + if err != nil { + t.Fatalf("reading migrated metadata language: %v", err) + } + if value != `"fr"` { + t.Errorf("metadata language = %s, want \"fr\"", value) + } + }) + + t.Run("legacy quality decomposes into two axes", func(t *testing.T) { + var resolution, bitrate string + if err := pool.QueryRow(ctx, ` +SELECT value::text FROM user_setting_values + WHERE key = 'playback.preferred_quality' AND scope = 'profile_device' + AND profile_id = 'mp1' AND device_id = 'md1'`).Scan(&resolution); err != nil { + t.Fatalf("reading resolution: %v", err) + } + if err := pool.QueryRow(ctx, ` +SELECT value::text FROM user_setting_values + WHERE key = 'playback.max_bitrate_kbps' AND scope = 'profile_device' + AND profile_id = 'mp1' AND device_id = 'md1'`).Scan(&bitrate); err != nil { + t.Fatalf("reading bitrate: %v", err) + } + if resolution != `"1080p"` || bitrate != `10000` { + t.Errorf("decomposed to (%s, %s), want (\"1080p\", 10000)", resolution, bitrate) + } + }) + + t.Run("values are stored as typed jsonb, not strings", func(t *testing.T) { + var kind string + if err := pool.QueryRow(ctx, ` +SELECT jsonb_typeof(value) FROM user_setting_values + WHERE key = 'playback.max_bitrate_kbps' AND profile_id = 'mp1' AND device_id = 'md1'`). + Scan(&kind); err != nil { + t.Fatalf("reading jsonb type: %v", err) + } + if kind != "number" { + t.Errorf("bitrate stored as jsonb %s, want number", kind) + } + }) + + t.Run("rejects carry a queryable jsonb identity", func(t *testing.T) { + var identity, reason string + err := pool.QueryRow(ctx, ` +SELECT identity::text, reason FROM user_setting_migration_rejects + WHERE source_key = 'legacy.unknown.key' LIMIT 1`).Scan(&identity, &reason) + if err != nil { + t.Fatalf("the unknown key was dropped rather than recorded: %v", err) + } + var decoded map[string]any + if err := json.Unmarshal([]byte(identity), &decoded); err != nil { + t.Errorf("identity %q is not JSON: %v", identity, err) + } + if reason == "" { + t.Error("reject carries no reason") + } + }) + + t.Run("the composite profile foreign key holds", func(t *testing.T) { + // A profile-scope row naming a profile that does not exist must be + // refused, which is what keeps orphaned settings out after a profile is + // deleted. + _, err := pool.Exec(ctx, ` +INSERT INTO user_setting_values (user_id, key, scope, profile_id, value) +VALUES ((SELECT id FROM users WHERE username = 'migtest'), + 'playback.subtitle_mode', 'profile', 'no-such-profile', '"auto"'::jsonb)`) + if err == nil { + t.Error("a row for a nonexistent profile was accepted") + } + }) +} + +// seedLegacyPostgresSettings writes the pre-cutover rows a real install holds. +func seedLegacyPostgresSettings(ctx context.Context, t *testing.T, pool *pgxpool.Pool) { + t.Helper() + + var userID int + err := pool.QueryRow(ctx, ` +INSERT INTO users (username, email, password_hash, role) +VALUES ('migtest', 'migtest@example.com', 'x', 'user') +ON CONFLICT (username) DO UPDATE SET email = EXCLUDED.email +RETURNING id`).Scan(&userID) + if err != nil { + t.Fatalf("seeding user: %v", err) + } + t.Cleanup(func() { + _, _ = pool.Exec(context.Background(), `DELETE FROM users WHERE id = $1`, userID) + }) + + if _, err := pool.Exec(ctx, ` +INSERT INTO user_profiles + (user_id, id, name, quality_preference, language, subtitle_language, + subtitle_mode, show_forced_subtitles, preferred_metadata_language) +VALUES ($1, 'mp1', 'Migrate Me', '1080p', 'ja', 'en', 'always', false, 'fr') +ON CONFLICT (user_id, id) DO NOTHING`, userID); err != nil { + t.Fatalf("seeding profile: %v", err) + } + + if _, err := pool.Exec(ctx, ` +INSERT INTO user_device_settings (user_id, profile_id, device_id, key, value) +VALUES ($1, 'mp1', 'md1', 'playback.preferred_quality', '1080p-high') +ON CONFLICT (user_id, profile_id, device_id, key) DO UPDATE SET value = EXCLUDED.value`, + userID); err != nil { + t.Fatalf("seeding device setting: %v", err) + } + + if _, err := pool.Exec(ctx, ` +INSERT INTO user_settings (user_id, key, value) +VALUES ($1, 'ui_theme', 'cobalt-studio'), ($1, 'legacy.unknown.key', 'whatever') +ON CONFLICT (user_id, key) DO UPDATE SET value = EXCLUDED.value`, userID); err != nil { + t.Fatalf("seeding user settings: %v", err) + } + + // Clear anything a prior run left, so the assertions above see only this + // seed's conversions. + if _, err := pool.Exec(ctx, + `DELETE FROM user_setting_values WHERE user_id = $1`, userID); err != nil { + t.Fatalf("clearing prior values: %v", err) + } + if _, err := pool.Exec(ctx, + `DELETE FROM user_setting_migration_rejects WHERE user_id = $1`, userID); err != nil { + t.Fatalf("clearing prior rejects: %v", err) + } +}