diff --git a/internal/access/resolver_test.go b/internal/access/resolver_test.go index 5a6448ec..dfe700be 100644 --- a/internal/access/resolver_test.go +++ b/internal/access/resolver_test.go @@ -2,6 +2,7 @@ package access import ( "context" + "encoding/json" "errors" "testing" "time" @@ -271,6 +272,39 @@ func (s stubStore) UpsertLibraryPlaybackPreference(context.Context, userstore.Li func (s stubStore) DeleteLibraryPlaybackPreference(context.Context, string, int) error { panic("unused") } +func (s stubStore) GetSettingValue(context.Context, userstore.SettingIdentity) (*userstore.SettingValue, error) { + panic("unused") +} +func (s stubStore) ListSettingValuesForResolution(context.Context, userstore.SettingResolutionQuery) ([]userstore.SettingValue, error) { + panic("unused") +} +func (s stubStore) UpsertSettingValue(context.Context, userstore.SettingIdentity, json.RawMessage) (*userstore.SettingValue, error) { + panic("unused") +} +func (s stubStore) DeleteSettingValue(context.Context, userstore.SettingIdentity) (bool, error) { + panic("unused") +} +func (s stubStore) DeleteSettingValuesForProfile(context.Context, string) (int64, error) { + panic("unused") +} +func (s stubStore) DeleteSettingValuesForDevice(context.Context, string, string) (int64, error) { + panic("unused") +} +func (s stubStore) DeleteSettingValuesForLibrary(context.Context, int) (int64, error) { + panic("unused") +} +func (s stubStore) DeleteSettingValuesForSeries(context.Context, string) (int64, error) { + panic("unused") +} +func (s stubStore) GetSettingMutation(context.Context, string) (*userstore.SettingMutationRecord, error) { + panic("unused") +} +func (s stubStore) PutSettingMutation(context.Context, userstore.SettingMutationRecord) (userstore.SettingMutationRecord, bool, error) { + panic("unused") +} +func (s stubStore) DeleteExpiredSettingMutations(context.Context, time.Time) (int64, error) { + panic("unused") +} func TestResolver_UnrestrictedAccountRestrictedProfile(t *testing.T) { resolver := NewResolver( diff --git a/internal/jellycompat/content_direct_test.go b/internal/jellycompat/content_direct_test.go index 995efc89..dd371c2e 100644 --- a/internal/jellycompat/content_direct_test.go +++ b/internal/jellycompat/content_direct_test.go @@ -2,6 +2,7 @@ package jellycompat import ( "context" + "encoding/json" "fmt" "net/url" "slices" @@ -525,6 +526,39 @@ func (s *progressCountingStore) UpsertLibraryPlaybackPreference(context.Context, func (s *progressCountingStore) DeleteLibraryPlaybackPreference(context.Context, string, int) error { panic("unused") } +func (s *progressCountingStore) GetSettingValue(context.Context, userstore.SettingIdentity) (*userstore.SettingValue, error) { + panic("unused") +} +func (s *progressCountingStore) ListSettingValuesForResolution(context.Context, userstore.SettingResolutionQuery) ([]userstore.SettingValue, error) { + panic("unused") +} +func (s *progressCountingStore) UpsertSettingValue(context.Context, userstore.SettingIdentity, json.RawMessage) (*userstore.SettingValue, error) { + panic("unused") +} +func (s *progressCountingStore) DeleteSettingValue(context.Context, userstore.SettingIdentity) (bool, error) { + panic("unused") +} +func (s *progressCountingStore) DeleteSettingValuesForProfile(context.Context, string) (int64, error) { + panic("unused") +} +func (s *progressCountingStore) DeleteSettingValuesForDevice(context.Context, string, string) (int64, error) { + panic("unused") +} +func (s *progressCountingStore) DeleteSettingValuesForLibrary(context.Context, int) (int64, error) { + panic("unused") +} +func (s *progressCountingStore) DeleteSettingValuesForSeries(context.Context, string) (int64, error) { + panic("unused") +} +func (s *progressCountingStore) GetSettingMutation(context.Context, string) (*userstore.SettingMutationRecord, error) { + panic("unused") +} +func (s *progressCountingStore) PutSettingMutation(context.Context, userstore.SettingMutationRecord) (userstore.SettingMutationRecord, bool, error) { + panic("unused") +} +func (s *progressCountingStore) DeleteExpiredSettingMutations(context.Context, time.Time) (int64, error) { + panic("unused") +} // stubBrowseSource is a deterministic browseSource for testing // directContentService without a Postgres pool. diff --git a/internal/userdb/conformance_test.go b/internal/userdb/conformance_test.go index 0f1118cb..84530a9b 100644 --- a/internal/userdb/conformance_test.go +++ b/internal/userdb/conformance_test.go @@ -30,6 +30,14 @@ func TestSQLiteProgressSince(t *testing.T) { storetest.RunProgressSince(t, newConformanceStore) } +// TestSQLiteSettingValues runs the canonical settings-contract storage +// conformance tests against the per-user SQLite backend. The Postgres backend +// runs the same suite in internal/userstore/pgstore, which is what keeps the two +// from drifting on scope identity, partial uniqueness and delete behavior. +func TestSQLiteSettingValues(t *testing.T) { + storetest.RunSettingValues(t, newConformanceStore) +} + func TestSQLiteAddFavoriteAtReportsInsertion(t *testing.T) { ctx := context.Background() store := newConformanceStore(t) diff --git a/internal/userdb/migrate.go b/internal/userdb/migrate.go index 7d1435fc..e94a890e 100644 --- a/internal/userdb/migrate.go +++ b/internal/userdb/migrate.go @@ -5,7 +5,7 @@ import ( "fmt" ) -const schemaVersion = 13 +const schemaVersion = 14 func runMigrations(db *sql.DB) error { version, err := userVersion(db) @@ -133,9 +133,29 @@ func runMigrations(db *sql.DB) error { } } + if version < 14 { + if err := migrateToV14(tx); err != nil { + return err + } + if _, err := tx.Exec("PRAGMA user_version = 14"); err != nil { + return fmt.Errorf("setting sqlite user_version 14: %w", err) + } + } + return tx.Commit() } +// migrateToV14 adds the canonical settings contract tables. InitSchema already +// creates them with IF NOT EXISTS on every open, so this step is what records +// that an existing database has them — the same shape migrateToV6 used for +// series_playback_preferences. +func migrateToV14(tx *sql.Tx) error { + if _, err := tx.Exec(settingContractSchema); err != nil { + return fmt.Errorf("creating settings contract tables: %w", err) + } + return nil +} + // migrateToV13 replaces the v1 watch_progress stamp triggers with the current // bodies (CREATE TRIGGER IF NOT EXISTS never replaces an existing trigger, and // InitSchema runs before migrations, so this must drop AND recreate). v2 makes diff --git a/internal/userdb/profiles.go b/internal/userdb/profiles.go index 54f80b0f..7789d6a3 100644 --- a/internal/userdb/profiles.go +++ b/internal/userdb/profiles.go @@ -292,7 +292,9 @@ func DeleteProfile(db *sql.DB, id string) error { return fmt.Errorf("deleting collection visibility for profile %s: %w", id, err) } - // Cascade-delete related tables. + // Cascade-delete related tables. This database declares no foreign keys, so + // user_setting_values is listed here; account-scope rows carry a NULL + // profile_id and are untouched, matching the Postgres backend. cascadeTables := []string{ "favorites", "watchlist", @@ -301,6 +303,7 @@ func DeleteProfile(db *sql.DB, id string) error { "profile_allowed_libraries", "series_playback_preferences", "library_playback_preferences", + "user_setting_values", } for _, table := range cascadeTables { column := "profile_id" diff --git a/internal/userdb/schema.go b/internal/userdb/schema.go index 586afd5e..792f5cb0 100644 --- a/internal/userdb/schema.go +++ b/internal/userdb/schema.go @@ -255,6 +255,83 @@ CREATE INDEX IF NOT EXISTS idx_home_item_dismissals_lookup CREATE INDEX IF NOT EXISTS idx_hidden_history_items_lookup ON hidden_history_items(profile_id, hidden_before); +` + settingContractSchema + +// settingContractSchema is the per-user half of the canonical settings contract +// storage. It mirrors the PostgreSQL shape in +// migrations/sql/20260727010621_user_setting_values.sql with user_id omitted: +// this database is already scoped to one user. jsonb becomes TEXT plus a +// json_valid CHECK, bigserial becomes INTEGER PRIMARY KEY AUTOINCREMENT, and +// timestamptz becomes an RFC3339 TEXT column, matching the rest of this schema. +// +// This file declares no foreign keys, deliberately and consistently with every +// other table here, so deleting a profile, library, series or device removes +// these rows through the owning delete path rather than a cascade. The userstore +// conformance suite holds both backends to identical behavior there. +const settingContractSchema = ` +CREATE TABLE IF NOT EXISTS user_setting_values ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + key TEXT NOT NULL, + scope TEXT NOT NULL, + profile_id TEXT, + device_id TEXT, + library_id INTEGER, + series_id TEXT, + value TEXT NOT NULL CHECK (json_valid(value)), + revision INTEGER NOT NULL DEFAULT 1, + created_at TEXT NOT NULL, + updated_at TEXT NOT NULL, + CHECK (scope IN ('account', 'profile', 'profile_device', 'profile_library', 'profile_series')), + CHECK ( + (scope = 'account' AND profile_id IS NULL AND device_id IS NULL AND library_id IS NULL AND series_id IS NULL) OR + (scope = 'profile' AND profile_id IS NOT NULL AND device_id IS NULL AND library_id IS NULL AND series_id IS NULL) OR + (scope = 'profile_device' AND profile_id IS NOT NULL AND device_id IS NOT NULL AND library_id IS NULL AND series_id IS NULL) OR + (scope = 'profile_library' AND profile_id IS NOT NULL AND device_id IS NULL AND library_id IS NOT NULL AND series_id IS NULL) OR + (scope = 'profile_series' AND profile_id IS NOT NULL AND device_id IS NULL AND library_id IS NULL AND series_id IS NOT NULL) + ) +); + +CREATE UNIQUE INDEX IF NOT EXISTS user_setting_values_account_uq + ON user_setting_values (key) WHERE scope = 'account'; +CREATE UNIQUE INDEX IF NOT EXISTS user_setting_values_profile_uq + ON user_setting_values (profile_id, key) WHERE scope = 'profile'; +CREATE UNIQUE INDEX IF NOT EXISTS user_setting_values_profile_device_uq + ON user_setting_values (profile_id, device_id, key) WHERE scope = 'profile_device'; +CREATE UNIQUE INDEX IF NOT EXISTS user_setting_values_profile_library_uq + ON user_setting_values (profile_id, library_id, key) WHERE scope = 'profile_library'; +CREATE UNIQUE INDEX IF NOT EXISTS user_setting_values_profile_series_uq + ON user_setting_values (profile_id, series_id, key) WHERE scope = 'profile_series'; + +CREATE INDEX IF NOT EXISTS user_setting_values_resolution_idx + ON user_setting_values (profile_id, key, scope); +CREATE INDEX IF NOT EXISTS user_setting_values_series_idx + ON user_setting_values (profile_id, series_id); +CREATE INDEX IF NOT EXISTS user_setting_values_library_idx + ON user_setting_values (profile_id, library_id); + +CREATE TABLE IF NOT EXISTS user_setting_mutations ( + mutation_id TEXT PRIMARY KEY, + request_hash TEXT NOT NULL, + result TEXT NOT NULL CHECK (json_valid(result)), + created_at TEXT NOT NULL, + expires_at TEXT NOT NULL +); + +CREATE INDEX IF NOT EXISTS user_setting_mutations_expiry_idx + ON user_setting_mutations (expires_at); + +CREATE TABLE IF NOT EXISTS user_setting_migration_rejects ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + source_table TEXT NOT NULL, + source_key TEXT NOT NULL, + identity TEXT NOT NULL CHECK (json_valid(identity)), + value TEXT, + reason TEXT NOT NULL, + recorded_at TEXT NOT NULL +); + +CREATE INDEX IF NOT EXISTS user_setting_migration_rejects_source_idx + ON user_setting_migration_rejects (source_table); ` // InitSchema creates all tables in the given SQLite database. diff --git a/internal/userdb/setting_values.go b/internal/userdb/setting_values.go new file mode 100644 index 00000000..7c8fd71f --- /dev/null +++ b/internal/userdb/setting_values.go @@ -0,0 +1,394 @@ +package userdb + +import ( + "database/sql" + "encoding/json" + "fmt" + "strings" + "time" + + "github.com/Silo-Server/silo-server/internal/settingscontract" + "github.com/Silo-Server/silo-server/internal/userstore" +) + +// settingValueColumns is the projection every read shares, in the order +// scanSettingValue expects. +const settingValueColumns = `key, scope, profile_id, device_id, library_id, series_id, + value, revision, created_at, updated_at` + +// settingConflictTargets maps a scope to the partial unique index that enforces +// one explicit value per identity. SQLite requires an upsert against a partial +// index to repeat that index's WHERE clause, so the target carries it. +var settingConflictTargets = map[settingscontract.Scope]string{ + settingscontract.ScopeAccount: "(key) WHERE scope = 'account'", + settingscontract.ScopeProfile: "(profile_id, key) WHERE scope = 'profile'", + settingscontract.ScopeProfileDevice: "(profile_id, device_id, key) WHERE scope = 'profile_device'", + settingscontract.ScopeProfileLibrary: "(profile_id, library_id, key) WHERE scope = 'profile_library'", + settingscontract.ScopeProfileSeries: "(profile_id, series_id, key) WHERE scope = 'profile_series'", +} + +// settingIdentityPredicate returns the WHERE fragment and bind arguments that +// address exactly one row. Every scope compares only the columns it populates, +// so no clause ever has to reason about NULL equality. +func settingIdentityPredicate(id userstore.SettingIdentity) (string, []any) { + args := []any{id.Key, string(id.Scope)} + clause := "key = ? AND scope = ?" + switch id.Scope { + case settingscontract.ScopeProfile: + args = append(args, id.ProfileID) + clause += " AND profile_id = ?" + case settingscontract.ScopeProfileDevice: + args = append(args, id.ProfileID, id.DeviceID) + clause += " AND profile_id = ? AND device_id = ?" + case settingscontract.ScopeProfileLibrary: + args = append(args, id.ProfileID, id.LibraryID) + clause += " AND profile_id = ? AND library_id = ?" + case settingscontract.ScopeProfileSeries: + args = append(args, id.ProfileID, id.SeriesID) + clause += " AND profile_id = ? AND series_id = ?" + } + return clause, args +} + +// GetSettingValue returns the explicit value at exactly one scope, or nil when +// that identity is unset. +func GetSettingValue(db *sql.DB, id userstore.SettingIdentity) (*userstore.SettingValue, error) { + if err := id.Validate(); err != nil { + return nil, err + } + clause, args := settingIdentityPredicate(id) + row := db.QueryRow("SELECT "+settingValueColumns+" FROM user_setting_values WHERE "+clause, args...) + value, err := scanSettingValue(row) + if err == sql.ErrNoRows { + return nil, nil + } + if err != nil { + return nil, fmt.Errorf("getting setting value %q at %s: %w", id.Key, id.Scope, err) + } + return &value, nil +} + +// ListSettingValuesForResolution collects every candidate row for a resolution +// request in one query. The predicate covers all five scopes at once: ranking by +// each definition's resolution order happens in Go, so a four-scope chain still +// costs one round trip rather than four lookups per key. +func ListSettingValuesForResolution( + db *sql.DB, + query userstore.SettingResolutionQuery, +) ([]userstore.SettingValue, error) { + q := query.Normalized() + if len(q.Keys) == 0 { + return nil, nil + } + + args := make([]any, 0, len(q.Keys)+len(q.LibraryIDs)+len(q.SeriesIDs)+2) + for _, key := range q.Keys { + args = append(args, key) + } + args = append(args, q.ProfileID, q.DeviceID) + for _, libraryID := range q.LibraryIDs { + args = append(args, libraryID) + } + for _, seriesID := range q.SeriesIDs { + args = append(args, seriesID) + } + + libraryClause := "0" + if len(q.LibraryIDs) > 0 { + libraryClause = "scope = 'profile_library' AND library_id IN (" + placeholders(len(q.LibraryIDs)) + ")" + } + seriesClause := "0" + if len(q.SeriesIDs) > 0 { + seriesClause = "scope = 'profile_series' AND series_id IN (" + placeholders(len(q.SeriesIDs)) + ")" + } + + rows, err := db.Query(` + SELECT `+settingValueColumns+` + FROM user_setting_values + WHERE key IN (`+placeholders(len(q.Keys))+`) + AND ( + scope = 'account' + OR ( + profile_id = ? + AND ( + scope = 'profile' + OR (scope = 'profile_device' AND device_id = ?) + OR (`+libraryClause+`) + OR (`+seriesClause+`) + ) + ) + ) + ORDER BY key, scope, COALESCE(profile_id, ''), COALESCE(device_id, ''), + COALESCE(library_id, 0), COALESCE(series_id, '')`, + args..., + ) + if err != nil { + return nil, fmt.Errorf("listing setting values for resolution: %w", err) + } + defer rows.Close() + + var values []userstore.SettingValue + for rows.Next() { + value, err := scanSettingValue(rows) + if err != nil { + return nil, fmt.Errorf("scanning setting value: %w", err) + } + values = append(values, value) + } + return values, rows.Err() +} + +// UpsertSettingValue writes the explicit value at one scope and increments that +// row's revision. +func UpsertSettingValue( + db *sql.DB, + id userstore.SettingIdentity, + value json.RawMessage, +) (*userstore.SettingValue, error) { + if err := id.Validate(); err != nil { + return nil, err + } + if err := userstore.ValidateSettingValueJSON(value); err != nil { + return nil, err + } + target, ok := settingConflictTargets[id.Scope] + if !ok { + return nil, fmt.Errorf("%w: %q has no storage identity", userstore.ErrInvalidSettingIdentity, id.Scope) + } + + now := nowRFC3339() + row := db.QueryRow(fmt.Sprintf(` + INSERT INTO user_setting_values + (key, scope, profile_id, device_id, library_id, series_id, value, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?) + ON CONFLICT %s DO UPDATE SET + value = excluded.value, + revision = user_setting_values.revision + 1, + updated_at = excluded.updated_at + RETURNING %s`, target, settingValueColumns), + id.Key, string(id.Scope), + nullableText(id.ProfileID), nullableText(id.DeviceID), + nullableInt(id.LibraryID), nullableText(id.SeriesID), + string(value), now, now, + ) + stored, err := scanSettingValue(row) + if err != nil { + return nil, fmt.Errorf("upserting setting value %q at %s: %w", id.Key, id.Scope, err) + } + return &stored, nil +} + +// DeleteSettingValue removes the explicit value at one scope — the `unset` +// operation — and reports whether a row existed. +func DeleteSettingValue(db *sql.DB, id userstore.SettingIdentity) (bool, error) { + if err := id.Validate(); err != nil { + return false, err + } + clause, args := settingIdentityPredicate(id) + result, err := db.Exec("DELETE FROM user_setting_values WHERE "+clause, args...) + if err != nil { + return false, fmt.Errorf("deleting setting value %q at %s: %w", id.Key, id.Scope, err) + } + affected, err := result.RowsAffected() + if err != nil { + return false, fmt.Errorf("counting deleted setting value %q: %w", id.Key, err) + } + return affected > 0, nil +} + +// DeleteSettingValuesForProfile removes every profile-anchored value for one +// profile. Account-scope rows carry a NULL profile_id and survive, which is what +// deleting one household member out of an account has to mean. +func DeleteSettingValuesForProfile(db *sql.DB, profileID string) (int64, error) { + return execSettingValueDelete(db, + "DELETE FROM user_setting_values WHERE profile_id = ?", + fmt.Sprintf("profile %q", profileID), profileID) +} + +func DeleteSettingValuesForDevice(db *sql.DB, profileID, deviceID string) (int64, error) { + return execSettingValueDelete(db, + "DELETE FROM user_setting_values WHERE scope = 'profile_device' AND profile_id = ? AND device_id = ?", + fmt.Sprintf("device %q", deviceID), profileID, deviceID) +} + +func DeleteSettingValuesForLibrary(db *sql.DB, libraryID int) (int64, error) { + return execSettingValueDelete(db, + "DELETE FROM user_setting_values WHERE scope = 'profile_library' AND library_id = ?", + fmt.Sprintf("library %d", libraryID), libraryID) +} + +func DeleteSettingValuesForSeries(db *sql.DB, seriesID string) (int64, error) { + return execSettingValueDelete(db, + "DELETE FROM user_setting_values WHERE scope = 'profile_series' AND series_id = ?", + fmt.Sprintf("series %q", seriesID), seriesID) +} + +func execSettingValueDelete(db *sql.DB, query, subject string, args ...any) (int64, error) { + result, err := db.Exec(query, args...) + if err != nil { + return 0, fmt.Errorf("deleting setting values for %s: %w", subject, err) + } + affected, err := result.RowsAffected() + if err != nil { + return 0, fmt.Errorf("counting setting values deleted for %s: %w", subject, err) + } + return affected, nil +} + +// GetSettingMutation returns a recorded idempotency receipt, or nil. +func GetSettingMutation(db *sql.DB, mutationID string) (*userstore.SettingMutationRecord, error) { + row := db.QueryRow(` + SELECT mutation_id, request_hash, result, created_at, expires_at + FROM user_setting_mutations + WHERE mutation_id = ?`, mutationID) + record, err := scanSettingMutation(row) + if err == sql.ErrNoRows { + return nil, nil + } + if err != nil { + return nil, fmt.Errorf("getting setting mutation %q: %w", mutationID, err) + } + return &record, nil +} + +// PutSettingMutation never overwrites a receipt: DO NOTHING plus a second read +// keeps a replayed mutation_id answering with the result the first attempt +// produced, which is what makes a client's retry idempotent rather than a +// silent re-run. +func PutSettingMutation( + db *sql.DB, + record userstore.SettingMutationRecord, +) (userstore.SettingMutationRecord, bool, error) { + if err := record.Validate(); err != nil { + return userstore.SettingMutationRecord{}, false, err + } + + row := db.QueryRow(` + INSERT INTO user_setting_mutations (mutation_id, request_hash, result, created_at, expires_at) + VALUES (?, ?, ?, ?, ?) + ON CONFLICT (mutation_id) DO NOTHING + RETURNING mutation_id, request_hash, result, created_at, expires_at`, + record.MutationID, record.RequestHash, string(record.Result), + nowRFC3339(), record.ExpiresAt.UTC().Format(time.RFC3339), + ) + stored, err := scanSettingMutation(row) + if err == nil { + return stored, true, nil + } + if err != sql.ErrNoRows { + return userstore.SettingMutationRecord{}, false, fmt.Errorf("recording setting mutation %q: %w", record.MutationID, err) + } + + existing, err := GetSettingMutation(db, record.MutationID) + if err != nil { + return userstore.SettingMutationRecord{}, false, err + } + if existing == nil { + // The conflicting row was swept between the insert and this read; the + // caller can safely retry rather than receive a phantom conflict. + return userstore.SettingMutationRecord{}, false, fmt.Errorf( + "recording setting mutation %q: conflicting receipt disappeared", record.MutationID) + } + return *existing, false, nil +} + +// DeleteExpiredSettingMutations removes receipts that expired before the given +// instant and reports how many. +func DeleteExpiredSettingMutations(db *sql.DB, before time.Time) (int64, error) { + result, err := db.Exec( + "DELETE FROM user_setting_mutations WHERE expires_at <= ?", + before.UTC().Format(time.RFC3339), + ) + if err != nil { + return 0, fmt.Errorf("sweeping expired setting mutations: %w", err) + } + affected, err := result.RowsAffected() + if err != nil { + return 0, fmt.Errorf("counting swept setting mutations: %w", err) + } + return affected, nil +} + +// sqlRow is the subset of *sql.Row and *sql.Rows the scan helpers need. +type sqlRow interface { + Scan(dest ...any) error +} + +func scanSettingValue(row sqlRow) (userstore.SettingValue, error) { + var ( + value userstore.SettingValue + scope string + profileID sql.NullString + deviceID sql.NullString + libraryID sql.NullInt64 + seriesID sql.NullString + raw string + ) + if err := row.Scan( + &value.Key, &scope, &profileID, &deviceID, &libraryID, &seriesID, + &raw, &value.Revision, &value.CreatedAt, &value.UpdatedAt, + ); err != nil { + return userstore.SettingValue{}, err + } + value.Scope = settingscontract.Scope(scope) + value.ProfileID = profileID.String + value.DeviceID = deviceID.String + value.LibraryID = int(libraryID.Int64) + value.SeriesID = seriesID.String + value.Value = json.RawMessage(raw) + return value, nil +} + +func scanSettingMutation(row sqlRow) (userstore.SettingMutationRecord, error) { + var ( + record userstore.SettingMutationRecord + raw string + createdAt string + expiresAt string + ) + if err := row.Scan(&record.MutationID, &record.RequestHash, &raw, &createdAt, &expiresAt); err != nil { + return userstore.SettingMutationRecord{}, err + } + record.Result = json.RawMessage(raw) + var err error + if record.CreatedAt, err = parseRFC3339(createdAt); err != nil { + return userstore.SettingMutationRecord{}, fmt.Errorf("parsing created_at for mutation %q: %w", record.MutationID, err) + } + if record.ExpiresAt, err = parseRFC3339(expiresAt); err != nil { + return userstore.SettingMutationRecord{}, fmt.Errorf("parsing expires_at for mutation %q: %w", record.MutationID, err) + } + return record, nil +} + +func parseRFC3339(value string) (time.Time, error) { + parsed, err := time.Parse(time.RFC3339, value) + if err != nil { + return time.Time{}, err + } + return parsed.UTC(), nil +} + +func nowRFC3339() string { + return time.Now().UTC().Format(time.RFC3339) +} + +func nullableText(value string) any { + if value == "" { + return nil + } + return value +} + +func nullableInt(value int) any { + if value == 0 { + return nil + } + return value +} + +func placeholders(n int) string { + if n <= 0 { + return "" + } + return strings.TrimSuffix(strings.Repeat("?,", n), ",") +} diff --git a/internal/userdb/settings.go b/internal/userdb/settings.go index 83a16625..e241671a 100644 --- a/internal/userdb/settings.go +++ b/internal/userdb/settings.go @@ -166,12 +166,30 @@ func DeleteDeviceSetting(db *sql.DB, profileID, deviceID, key string) error { return nil } +// DeleteAllDeviceSettings clears everything one device holds for one profile. +// It is the forget-device path, so it drops the canonical profile_device values +// alongside the legacy string overrides: this database declares no foreign keys, +// so nothing else would. func DeleteAllDeviceSettings(db *sql.DB, profileID, deviceID string) error { - _, err := db.Exec("DELETE FROM user_device_settings WHERE profile_id = ? AND device_id = ?", profileID, deviceID) + tx, err := db.Begin() if err != nil { + return fmt.Errorf("beginning transaction to forget device %q: %w", deviceID, err) + } + defer tx.Rollback() //nolint:errcheck + + if _, err := tx.Exec( + "DELETE FROM user_device_settings WHERE profile_id = ? AND device_id = ?", + profileID, deviceID, + ); err != nil { return fmt.Errorf("deleting all device settings for device %q: %w", deviceID, err) } - return nil + if _, err := tx.Exec( + "DELETE FROM user_setting_values WHERE scope = 'profile_device' AND profile_id = ? AND device_id = ?", + profileID, deviceID, + ); err != nil { + return fmt.Errorf("deleting setting values for device %q: %w", deviceID, err) + } + return tx.Commit() } func DeleteDeviceSettingsByKey(db *sql.DB, key string) error { diff --git a/internal/userdb/sqlitestore.go b/internal/userdb/sqlitestore.go index 9c78c389..194272e6 100644 --- a/internal/userdb/sqlitestore.go +++ b/internal/userdb/sqlitestore.go @@ -3,6 +3,7 @@ package userdb import ( "context" "database/sql" + "encoding/json" "fmt" "strconv" "time" @@ -420,3 +421,49 @@ func (s *SQLiteUserStore) UpsertLibraryPlaybackPreference(_ context.Context, pre func (s *SQLiteUserStore) DeleteLibraryPlaybackPreference(_ context.Context, profileID string, libraryID int) error { return DeleteLibraryPlaybackPreference(s.db, profileID, libraryID) } + +// --- Canonical typed setting values --- + +func (s *SQLiteUserStore) GetSettingValue(_ context.Context, id userstore.SettingIdentity) (*userstore.SettingValue, error) { + return GetSettingValue(s.db, id) +} + +func (s *SQLiteUserStore) ListSettingValuesForResolution(_ context.Context, query userstore.SettingResolutionQuery) ([]userstore.SettingValue, error) { + return ListSettingValuesForResolution(s.db, query) +} + +func (s *SQLiteUserStore) UpsertSettingValue(_ context.Context, id userstore.SettingIdentity, value json.RawMessage) (*userstore.SettingValue, error) { + return UpsertSettingValue(s.db, id, value) +} + +func (s *SQLiteUserStore) DeleteSettingValue(_ context.Context, id userstore.SettingIdentity) (bool, error) { + return DeleteSettingValue(s.db, id) +} + +func (s *SQLiteUserStore) DeleteSettingValuesForProfile(_ context.Context, profileID string) (int64, error) { + return DeleteSettingValuesForProfile(s.db, profileID) +} + +func (s *SQLiteUserStore) DeleteSettingValuesForDevice(_ context.Context, profileID, deviceID string) (int64, error) { + return DeleteSettingValuesForDevice(s.db, profileID, deviceID) +} + +func (s *SQLiteUserStore) DeleteSettingValuesForLibrary(_ context.Context, libraryID int) (int64, error) { + return DeleteSettingValuesForLibrary(s.db, libraryID) +} + +func (s *SQLiteUserStore) DeleteSettingValuesForSeries(_ context.Context, seriesID string) (int64, error) { + return DeleteSettingValuesForSeries(s.db, seriesID) +} + +func (s *SQLiteUserStore) GetSettingMutation(_ context.Context, mutationID string) (*userstore.SettingMutationRecord, error) { + return GetSettingMutation(s.db, mutationID) +} + +func (s *SQLiteUserStore) PutSettingMutation(_ context.Context, record userstore.SettingMutationRecord) (userstore.SettingMutationRecord, bool, error) { + return PutSettingMutation(s.db, record) +} + +func (s *SQLiteUserStore) DeleteExpiredSettingMutations(_ context.Context, before time.Time) (int64, error) { + return DeleteExpiredSettingMutations(s.db, before) +} diff --git a/internal/userstore/pgstore/conformance_test.go b/internal/userstore/pgstore/conformance_test.go index 13d29c4c..c7727233 100644 --- a/internal/userstore/pgstore/conformance_test.go +++ b/internal/userstore/pgstore/conformance_test.go @@ -57,3 +57,46 @@ func TestPostgresProgressSince(t *testing.T) { return newStore(pool, userID) }) } + +// TestPostgresSettingValues runs the canonical settings-contract storage +// conformance tests against the Postgres backend. The per-user SQLite backend +// runs the same suite in internal/userdb, which is what keeps the two from +// drifting on scope identity, partial uniqueness and delete behavior. Skips +// unless SILO_TEST_DATABASE_URL is set and the migration is applied. +func TestPostgresSettingValues(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) + + var table *string + err = pool.QueryRow(ctx, `SELECT table_name FROM information_schema.tables + WHERE table_schema = 'public' AND table_name = 'user_setting_values'`).Scan(&table) + if errors.Is(err, pgx.ErrNoRows) || table == nil { + t.Skip("settings contract storage migration has not been applied") + } + if err != nil { + t.Fatalf("check migration: %v", err) + } + + storetest.RunSettingValues(t, func(t *testing.T) userstore.UserStore { + var userID int + if err := pool.QueryRow(ctx, + `INSERT INTO users (username, role) VALUES ($1, 'user') RETURNING id`, + fmt.Sprintf("conf-settings-%d", time.Now().UnixNano()), + ).Scan(&userID); err != nil { + t.Fatalf("seed user: %v", err) + } + // user_setting_values and user_setting_mutations cascade from users. + t.Cleanup(func() { + _, _ = pool.Exec(ctx, `DELETE FROM users WHERE id = $1`, userID) + }) + return newStore(pool, userID) + }) +} diff --git a/internal/userstore/pgstore/profiles.go b/internal/userstore/pgstore/profiles.go index 44e89fe2..fa872300 100644 --- a/internal/userstore/pgstore/profiles.go +++ b/internal/userstore/pgstore/profiles.go @@ -272,6 +272,10 @@ func (s *PostgresUserStore) DeleteProfile(ctx context.Context, id string) error return fmt.Errorf("deleting collection items for profile %s: %w", id, err) } + // user_setting_values is listed rather than left to its composite profile + // FK so both backends delete identically: the per-user SQLite store has no + // foreign keys at all. Account-scope rows carry a NULL profile_id and are + // untouched, which is what removing one household member has to mean. cascadeTables := []string{ "user_favorites", "user_watchlist", @@ -279,6 +283,7 @@ func (s *PostgresUserStore) DeleteProfile(ctx context.Context, id string) error "user_personal_collections", "user_series_playback_preferences", "user_library_playback_preferences", + "user_setting_values", } for _, table := range cascadeTables { if _, err := tx.Exec(ctx, fmt.Sprintf("DELETE FROM %s WHERE user_id = $1 AND profile_id = $2", table), s.userID, id); err != nil { diff --git a/internal/userstore/pgstore/setting_values.go b/internal/userstore/pgstore/setting_values.go new file mode 100644 index 00000000..9d0fe4be --- /dev/null +++ b/internal/userstore/pgstore/setting_values.go @@ -0,0 +1,366 @@ +package pgstore + +import ( + "context" + "encoding/json" + "fmt" + "time" + + "github.com/jackc/pgx/v5" + + "github.com/Silo-Server/silo-server/internal/settingscontract" + "github.com/Silo-Server/silo-server/internal/userstore" +) + +// settingValueColumns is the projection every read shares, in the order +// scanSettingValue expects. +const settingValueColumns = `key, scope, profile_id, device_id, library_id, series_id, + value, revision, created_at, updated_at` + +// settingConflictTargets maps a scope to the partial unique index that enforces +// one explicit value per identity. The upsert names the matching target so a +// repeated write updates its own row rather than inserting a duplicate. +var settingConflictTargets = map[settingscontract.Scope]string{ + settingscontract.ScopeAccount: "(user_id, key) WHERE scope = 'account'", + settingscontract.ScopeProfile: "(user_id, profile_id, key) WHERE scope = 'profile'", + settingscontract.ScopeProfileDevice: "(user_id, profile_id, device_id, key) WHERE scope = 'profile_device'", + settingscontract.ScopeProfileLibrary: "(user_id, profile_id, library_id, key) WHERE scope = 'profile_library'", + settingscontract.ScopeProfileSeries: "(user_id, profile_id, series_id, key) WHERE scope = 'profile_series'", +} + +// settingIdentityPredicate returns the WHERE fragment and bind arguments that +// address exactly one row. Every scope compares only the columns it populates, +// so no clause ever has to reason about NULL equality. +func settingIdentityPredicate(userID int, id userstore.SettingIdentity) (string, []any) { + args := []any{userID, id.Key, string(id.Scope)} + clause := "user_id = $1 AND key = $2 AND scope = $3" + switch id.Scope { + case settingscontract.ScopeProfile: + args = append(args, id.ProfileID) + clause += " AND profile_id = $4" + case settingscontract.ScopeProfileDevice: + args = append(args, id.ProfileID, id.DeviceID) + clause += " AND profile_id = $4 AND device_id = $5" + case settingscontract.ScopeProfileLibrary: + args = append(args, id.ProfileID, id.LibraryID) + clause += " AND profile_id = $4 AND library_id = $5" + case settingscontract.ScopeProfileSeries: + args = append(args, id.ProfileID, id.SeriesID) + clause += " AND profile_id = $4 AND series_id = $5" + } + return clause, args +} + +func (s *PostgresUserStore) GetSettingValue( + ctx context.Context, + id userstore.SettingIdentity, +) (*userstore.SettingValue, error) { + if err := id.Validate(); err != nil { + return nil, err + } + clause, args := settingIdentityPredicate(s.userID, id) + row := s.pool.QueryRow(ctx, + "SELECT "+settingValueColumns+" FROM user_setting_values WHERE "+clause, + args..., + ) + value, err := scanSettingValue(row) + if err == pgx.ErrNoRows { + return nil, nil + } + if err != nil { + return nil, fmt.Errorf("getting setting value %q at %s: %w", id.Key, id.Scope, err) + } + return &value, nil +} + +// ListSettingValuesForResolution collects every candidate row for a resolution +// request in one query. The predicate covers all five scopes at once: ranking by +// each definition's resolution order happens in Go, so a four-scope chain still +// costs one round trip and one index scan rather than four lookups per key. +func (s *PostgresUserStore) ListSettingValuesForResolution( + ctx context.Context, + query userstore.SettingResolutionQuery, +) ([]userstore.SettingValue, error) { + q := query.Normalized() + if len(q.Keys) == 0 { + return nil, nil + } + + rows, err := s.pool.Query(ctx, ` + SELECT `+settingValueColumns+` + FROM user_setting_values + WHERE user_id = $1 + AND key = ANY($2::text[]) + AND ( + scope = 'account' + OR ( + profile_id = $3 + AND ( + scope = 'profile' + OR (scope = 'profile_device' AND device_id = $4) + OR (scope = 'profile_library' AND library_id = ANY($5::int[])) + OR (scope = 'profile_series' AND series_id = ANY($6::text[])) + ) + ) + ) + ORDER BY key, scope, COALESCE(profile_id, ''), COALESCE(device_id, ''), + COALESCE(library_id, 0), COALESCE(series_id, '')`, + s.userID, q.Keys, q.ProfileID, q.DeviceID, q.LibraryIDs, q.SeriesIDs, + ) + if err != nil { + return nil, fmt.Errorf("listing setting values for resolution: %w", err) + } + defer rows.Close() + + var values []userstore.SettingValue + for rows.Next() { + value, err := scanSettingValue(rows) + if err != nil { + return nil, fmt.Errorf("scanning setting value: %w", err) + } + values = append(values, value) + } + return values, rows.Err() +} + +func (s *PostgresUserStore) UpsertSettingValue( + ctx context.Context, + id userstore.SettingIdentity, + value json.RawMessage, +) (*userstore.SettingValue, error) { + if err := id.Validate(); err != nil { + return nil, err + } + if err := userstore.ValidateSettingValueJSON(value); err != nil { + return nil, err + } + target, ok := settingConflictTargets[id.Scope] + if !ok { + return nil, fmt.Errorf("%w: %q has no storage identity", userstore.ErrInvalidSettingIdentity, id.Scope) + } + + row := s.pool.QueryRow(ctx, fmt.Sprintf(` + 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) + ON CONFLICT %s DO UPDATE SET + value = excluded.value, + revision = user_setting_values.revision + 1, + updated_at = now() + RETURNING %s`, target, settingValueColumns), + s.userID, id.Key, string(id.Scope), + nullableText(id.ProfileID), nullableText(id.DeviceID), + nullableInt(id.LibraryID), nullableText(id.SeriesID), + []byte(value), + ) + stored, err := scanSettingValue(row) + if err != nil { + return nil, fmt.Errorf("upserting setting value %q at %s: %w", id.Key, id.Scope, err) + } + return &stored, nil +} + +func (s *PostgresUserStore) DeleteSettingValue(ctx context.Context, id userstore.SettingIdentity) (bool, error) { + if err := id.Validate(); err != nil { + return false, err + } + clause, args := settingIdentityPredicate(s.userID, id) + tag, err := s.pool.Exec(ctx, "DELETE FROM user_setting_values WHERE "+clause, args...) + if err != nil { + return false, fmt.Errorf("deleting setting value %q at %s: %w", id.Key, id.Scope, err) + } + return tag.RowsAffected() > 0, nil +} + +// DeleteSettingValuesForProfile removes every profile-anchored value for one +// profile. Account-scope rows carry a NULL profile_id and survive, which is what +// deleting one household member out of an account has to mean. +func (s *PostgresUserStore) DeleteSettingValuesForProfile(ctx context.Context, profileID string) (int64, error) { + tag, err := s.pool.Exec(ctx, + "DELETE FROM user_setting_values WHERE user_id = $1 AND profile_id = $2", + s.userID, profileID, + ) + if err != nil { + return 0, fmt.Errorf("deleting setting values for profile %q: %w", profileID, err) + } + return tag.RowsAffected(), nil +} + +func (s *PostgresUserStore) DeleteSettingValuesForDevice(ctx context.Context, profileID, deviceID string) (int64, error) { + tag, err := s.pool.Exec(ctx, ` + DELETE FROM user_setting_values + WHERE user_id = $1 AND scope = 'profile_device' AND profile_id = $2 AND device_id = $3`, + s.userID, profileID, deviceID, + ) + if err != nil { + return 0, fmt.Errorf("deleting setting values for device %q: %w", deviceID, err) + } + return tag.RowsAffected(), nil +} + +func (s *PostgresUserStore) DeleteSettingValuesForLibrary(ctx context.Context, libraryID int) (int64, error) { + tag, err := s.pool.Exec(ctx, ` + DELETE FROM user_setting_values + WHERE user_id = $1 AND scope = 'profile_library' AND library_id = $2`, + s.userID, libraryID, + ) + if err != nil { + return 0, fmt.Errorf("deleting setting values for library %d: %w", libraryID, err) + } + return tag.RowsAffected(), nil +} + +func (s *PostgresUserStore) DeleteSettingValuesForSeries(ctx context.Context, seriesID string) (int64, error) { + tag, err := s.pool.Exec(ctx, ` + DELETE FROM user_setting_values + WHERE user_id = $1 AND scope = 'profile_series' AND series_id = $2`, + s.userID, seriesID, + ) + if err != nil { + return 0, fmt.Errorf("deleting setting values for series %q: %w", seriesID, err) + } + return tag.RowsAffected(), nil +} + +func (s *PostgresUserStore) GetSettingMutation( + ctx context.Context, + mutationID string, +) (*userstore.SettingMutationRecord, error) { + row := s.pool.QueryRow(ctx, ` + SELECT mutation_id, request_hash, result, created_at, expires_at + FROM user_setting_mutations + WHERE user_id = $1 AND mutation_id = $2`, + s.userID, mutationID, + ) + record, err := scanSettingMutation(row) + if err == pgx.ErrNoRows { + return nil, nil + } + if err != nil { + return nil, fmt.Errorf("getting setting mutation %q: %w", mutationID, err) + } + return &record, nil +} + +// PutSettingMutation never overwrites a receipt: DO NOTHING plus a second read +// keeps a replayed mutation_id answering with the result the first attempt +// produced, which is what makes a client's retry idempotent rather than a +// silent re-run. +func (s *PostgresUserStore) PutSettingMutation( + ctx context.Context, + record userstore.SettingMutationRecord, +) (userstore.SettingMutationRecord, bool, error) { + if err := record.Validate(); err != nil { + return userstore.SettingMutationRecord{}, false, err + } + + row := s.pool.QueryRow(ctx, ` + INSERT INTO user_setting_mutations (user_id, mutation_id, request_hash, result, expires_at) + VALUES ($1, $2, $3, $4, $5) + ON CONFLICT (user_id, mutation_id) DO NOTHING + RETURNING mutation_id, request_hash, result, created_at, expires_at`, + s.userID, record.MutationID, record.RequestHash, []byte(record.Result), record.ExpiresAt, + ) + stored, err := scanSettingMutation(row) + if err == nil { + return stored, true, nil + } + if err != pgx.ErrNoRows { + return userstore.SettingMutationRecord{}, false, fmt.Errorf("recording setting mutation %q: %w", record.MutationID, err) + } + + existing, err := s.GetSettingMutation(ctx, record.MutationID) + if err != nil { + return userstore.SettingMutationRecord{}, false, err + } + if existing == nil { + // The conflicting row was swept between the insert and this read; the + // caller can safely retry rather than receive a phantom conflict. + return userstore.SettingMutationRecord{}, false, fmt.Errorf( + "recording setting mutation %q: conflicting receipt disappeared", record.MutationID) + } + return *existing, false, nil +} + +func (s *PostgresUserStore) DeleteExpiredSettingMutations(ctx context.Context, before time.Time) (int64, error) { + tag, err := s.pool.Exec(ctx, + "DELETE FROM user_setting_mutations WHERE user_id = $1 AND expires_at <= $2", + s.userID, before, + ) + if err != nil { + return 0, fmt.Errorf("sweeping expired setting mutations: %w", err) + } + return tag.RowsAffected(), nil +} + +// pgxRow is the subset of pgx.Row and pgx.Rows the scan helpers need. +type pgxRow interface { + Scan(dest ...any) error +} + +func scanSettingValue(row pgxRow) (userstore.SettingValue, error) { + var ( + value userstore.SettingValue + scope string + profileID *string + deviceID *string + libraryID *int + seriesID *string + raw []byte + createdAt time.Time + updatedAt time.Time + ) + if err := row.Scan( + &value.Key, &scope, &profileID, &deviceID, &libraryID, &seriesID, + &raw, &value.Revision, &createdAt, &updatedAt, + ); err != nil { + return userstore.SettingValue{}, err + } + value.Scope = settingscontract.Scope(scope) + if profileID != nil { + value.ProfileID = *profileID + } + if deviceID != nil { + value.DeviceID = *deviceID + } + if libraryID != nil { + value.LibraryID = *libraryID + } + if seriesID != nil { + value.SeriesID = *seriesID + } + value.Value = json.RawMessage(raw) + value.CreatedAt = timeToString(createdAt) + value.UpdatedAt = timeToString(updatedAt) + return value, nil +} + +func scanSettingMutation(row pgxRow) (userstore.SettingMutationRecord, error) { + var ( + record userstore.SettingMutationRecord + raw []byte + ) + if err := row.Scan( + &record.MutationID, &record.RequestHash, &raw, &record.CreatedAt, &record.ExpiresAt, + ); err != nil { + return userstore.SettingMutationRecord{}, err + } + record.Result = json.RawMessage(raw) + record.CreatedAt = record.CreatedAt.UTC() + record.ExpiresAt = record.ExpiresAt.UTC() + return record, nil +} + +func nullableText(value string) *string { + if value == "" { + return nil + } + return &value +} + +func nullableInt(value int) *int { + if value == 0 { + return nil + } + return &value +} diff --git a/internal/userstore/pgstore/setting_values_test.go b/internal/userstore/pgstore/setting_values_test.go new file mode 100644 index 00000000..b3ff39e4 --- /dev/null +++ b/internal/userstore/pgstore/setting_values_test.go @@ -0,0 +1,140 @@ +package pgstore + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "os" + "sync" + "testing" + "time" + + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgxpool" + + "github.com/Silo-Server/silo-server/internal/settingscontract" + "github.com/Silo-Server/silo-server/internal/userstore" +) + +// countingQueryTracer records every statement pgx sends on behalf of a caller. +type countingQueryTracer struct { + mu sync.Mutex + queries []string +} + +func (c *countingQueryTracer) TraceQueryStart( + ctx context.Context, + _ *pgx.Conn, + data pgx.TraceQueryStartData, +) context.Context { + c.mu.Lock() + defer c.mu.Unlock() + c.queries = append(c.queries, data.SQL) + return ctx +} + +func (c *countingQueryTracer) TraceQueryEnd(context.Context, *pgx.Conn, pgx.TraceQueryEndData) {} + +func (c *countingQueryTracer) reset() { + c.mu.Lock() + defer c.mu.Unlock() + c.queries = nil +} + +func (c *countingQueryTracer) snapshot() []string { + c.mu.Lock() + defer c.mu.Unlock() + return append([]string(nil), c.queries...) +} + +// TestPostgresResolutionIssuesOneQuery pins the read path's normative rule: a +// batched resolution request costs one query no matter how many scopes, keys or +// content contexts it spans. Five sequential index lookups per key per item is a +// rejected implementation, and nothing else in the suite would notice the +// difference — the returned rows would be identical. +func TestPostgresResolutionIssuesOneQuery(t *testing.T) { + dsn := os.Getenv("SILO_TEST_DATABASE_URL") + if dsn == "" { + t.Skip("SILO_TEST_DATABASE_URL is not set") + } + ctx := context.Background() + + config, err := pgxpool.ParseConfig(dsn) + if err != nil { + t.Fatalf("parse test database url: %v", err) + } + tracer := &countingQueryTracer{} + config.ConnConfig.Tracer = tracer + // One connection keeps the trace deterministic: a second connection would + // replay session setup statements into the count. + config.MaxConns = 1 + + pool, err := pgxpool.NewWithConfig(ctx, config) + if err != nil { + t.Fatalf("connect test database: %v", err) + } + t.Cleanup(pool.Close) + + var table *string + err = pool.QueryRow(ctx, `SELECT table_name FROM information_schema.tables + WHERE table_schema = 'public' AND table_name = 'user_setting_values'`).Scan(&table) + if errors.Is(err, pgx.ErrNoRows) || table == nil { + t.Skip("settings contract storage migration has not been applied") + } + if err != nil { + t.Fatalf("check migration: %v", err) + } + + var userID int + if err := pool.QueryRow(ctx, + `INSERT INTO users (username, role) VALUES ($1, 'user') RETURNING id`, + fmt.Sprintf("conf-onequery-%d", time.Now().UnixNano()), + ).Scan(&userID); err != nil { + t.Fatalf("seed user: %v", err) + } + t.Cleanup(func() { + _, _ = pool.Exec(ctx, `DELETE FROM users WHERE id = $1`, userID) + }) + + store := newStore(pool, userID) + if err := store.CreateProfile(ctx, userstore.Profile{ID: "p1", Name: "Alice"}); err != nil { + t.Fatalf("CreateProfile: %v", err) + } + + const key = "playback.audio_language" + identities := []userstore.SettingIdentity{ + {Key: key, Scope: settingscontract.ScopeAccount}, + {Key: key, Scope: settingscontract.ScopeProfile, ProfileID: "p1"}, + {Key: key, Scope: settingscontract.ScopeProfileDevice, ProfileID: "p1", DeviceID: "apple-tv"}, + {Key: key, Scope: settingscontract.ScopeProfileLibrary, ProfileID: "p1", LibraryID: 42}, + {Key: key, Scope: settingscontract.ScopeProfileSeries, ProfileID: "p1", SeriesID: "s-1"}, + {Key: key, Scope: settingscontract.ScopeProfileSeries, ProfileID: "p1", SeriesID: "s-2"}, + {Key: key, Scope: settingscontract.ScopeProfileSeries, ProfileID: "p1", SeriesID: "s-3"}, + } + for _, id := range identities { + if _, err := store.UpsertSettingValue(ctx, id, json.RawMessage(`"en"`)); err != nil { + t.Fatalf("UpsertSettingValue(%+v): %v", id, err) + } + } + + tracer.reset() + rows, err := store.ListSettingValuesForResolution(ctx, userstore.SettingResolutionQuery{ + Keys: []string{key, "playback.subtitle_mode", "playback.subtitle_language"}, + ProfileID: "p1", + DeviceID: "apple-tv", + LibraryIDs: []int{42, 43}, + SeriesIDs: []string{"s-1", "s-2", "s-3"}, + }) + if err != nil { + t.Fatalf("ListSettingValuesForResolution: %v", err) + } + if len(rows) != len(identities) { + t.Fatalf("resolution returned %d rows, want %d", len(rows), len(identities)) + } + + issued := tracer.snapshot() + if len(issued) != 1 { + t.Fatalf("resolution issued %d queries, want 1:\n%v", len(issued), issued) + } +} diff --git a/internal/userstore/pgstore/settings.go b/internal/userstore/pgstore/settings.go index 55a35e86..38d7cfac 100644 --- a/internal/userstore/pgstore/settings.go +++ b/internal/userstore/pgstore/settings.go @@ -173,15 +173,31 @@ func (s *PostgresUserStore) DeleteDeviceSetting(ctx context.Context, profileID, return nil } +// DeleteAllDeviceSettings clears everything one device holds for one profile. +// It is the forget-device path, so it drops the canonical profile_device values +// alongside the legacy string overrides: device identity columns are not foreign +// keys in either backend, so nothing else would. func (s *PostgresUserStore) DeleteAllDeviceSettings(ctx context.Context, profileID, deviceID string) error { - _, err := s.pool.Exec(ctx, + tx, err := s.pool.Begin(ctx) + if err != nil { + return fmt.Errorf("beginning transaction to forget device %q: %w", deviceID, err) + } + defer tx.Rollback(ctx) //nolint:errcheck + + if _, err := tx.Exec(ctx, "DELETE FROM user_device_settings WHERE user_id = $1 AND profile_id = $2 AND device_id = $3", s.userID, profileID, deviceID, - ) - if err != nil { + ); err != nil { return fmt.Errorf("deleting all device settings for device %q: %w", deviceID, err) } - return nil + if _, err := tx.Exec(ctx, ` + DELETE FROM user_setting_values + WHERE user_id = $1 AND scope = 'profile_device' AND profile_id = $2 AND device_id = $3`, + s.userID, profileID, deviceID, + ); err != nil { + return fmt.Errorf("deleting setting values for device %q: %w", deviceID, err) + } + return tx.Commit(ctx) } func (s *PostgresUserStore) DeleteDeviceSettingsByKey(ctx context.Context, key string) error { diff --git a/internal/userstore/settingvalues.go b/internal/userstore/settingvalues.go new file mode 100644 index 00000000..768e7a83 --- /dev/null +++ b/internal/userstore/settingvalues.go @@ -0,0 +1,232 @@ +package userstore + +import ( + "encoding/json" + "errors" + "fmt" + "sort" + "strings" + "time" + + "github.com/Silo-Server/silo-server/internal/settingscontract" +) + +// ErrInvalidSettingIdentity is returned when a setting identity does not match +// the columns its scope requires. Both backends validate through +// SettingIdentity.Validate, so a request rejected by one is rejected by the +// other with the same reason. +var ErrInvalidSettingIdentity = errors.New("invalid setting identity") + +// ErrInvalidSettingValue is returned when a stored value is not well-formed +// JSON. The store checks only structural validity: whether a value satisfies its +// definition is settingscontract.ValidateValue's job, and that is the single +// validation path. +var ErrInvalidSettingValue = errors.New("invalid setting value") + +// SettingIdentity addresses exactly one canonical setting row: the key plus the +// context columns its scope requires. +// +// Only the fields belonging to Scope are meaningful; Validate enforces that and +// rejects anything else, so an identity that reaches SQL always matches the +// table's CHECK constraints. +type SettingIdentity struct { + Key string + Scope settingscontract.Scope + + ProfileID string + DeviceID string + LibraryID int + SeriesID string +} + +// Validate reports whether the identity is addressable. It mirrors the scope +// CHECK constraint on user_setting_values so an invalid identity is rejected +// before it reaches either backend rather than surfacing as a driver error whose +// text differs between them. +func (id SettingIdentity) Validate() error { + if strings.TrimSpace(id.Key) == "" { + return fmt.Errorf("%w: key is required", ErrInvalidSettingIdentity) + } + if !id.Scope.IsRemote() { + return fmt.Errorf("%w: %q is not a remote scope", ErrInvalidSettingIdentity, id.Scope) + } + + needProfile := id.Scope != settingscontract.ScopeAccount + if needProfile && strings.TrimSpace(id.ProfileID) == "" { + return fmt.Errorf("%w: scope %q requires a profile id", ErrInvalidSettingIdentity, id.Scope) + } + if !needProfile && id.ProfileID != "" { + return fmt.Errorf("%w: scope %q must not carry a profile id", ErrInvalidSettingIdentity, id.Scope) + } + + wantDevice := id.Scope == settingscontract.ScopeProfileDevice + if wantDevice && strings.TrimSpace(id.DeviceID) == "" { + return fmt.Errorf("%w: scope %q requires a device id", ErrInvalidSettingIdentity, id.Scope) + } + if !wantDevice && id.DeviceID != "" { + return fmt.Errorf("%w: scope %q must not carry a device id", ErrInvalidSettingIdentity, id.Scope) + } + + wantLibrary := id.Scope == settingscontract.ScopeProfileLibrary + if wantLibrary && id.LibraryID <= 0 { + return fmt.Errorf("%w: scope %q requires a library id", ErrInvalidSettingIdentity, id.Scope) + } + if !wantLibrary && id.LibraryID != 0 { + return fmt.Errorf("%w: scope %q must not carry a library id", ErrInvalidSettingIdentity, id.Scope) + } + + wantSeries := id.Scope == settingscontract.ScopeProfileSeries + if wantSeries && strings.TrimSpace(id.SeriesID) == "" { + return fmt.Errorf("%w: scope %q requires a series id", ErrInvalidSettingIdentity, id.Scope) + } + if !wantSeries && id.SeriesID != "" { + return fmt.Errorf("%w: scope %q must not carry a series id", ErrInvalidSettingIdentity, id.Scope) + } + return nil +} + +// SettingValue is one explicit value stored at one scope. Unset is the absence +// of a row, which is distinct from false, 0, "" and JSON null. +type SettingValue struct { + SettingIdentity + + // Value is the stored JSON. It is whatever settingscontract.NormalizeValue + // produced; the store neither interprets nor re-normalizes it. + Value json.RawMessage + // Revision increments on every write to this row. + Revision int64 + // CreatedAt and UpdatedAt are RFC3339 UTC timestamps. + CreatedAt string + UpdatedAt string +} + +// SettingResolutionQuery describes one resolution request: the keys to resolve +// and every identity they may resolve against. +// +// It is deliberately shaped for the batched read. A season view resolving n +// items passes every library and series id in one query and the resolver ranks +// the returned candidate rows by each definition's resolution order in Go. Five +// sequential index lookups per key per item is a rejected implementation. +type SettingResolutionQuery struct { + Keys []string + + // ProfileID drops every profile-anchored scope when empty, leaving only + // account-scope candidates. + ProfileID string + // DeviceID drops profile_device candidates when empty, which is what an + // unidentified client (jellycompat's DisplayPreferences seed) needs. + DeviceID string + // LibraryIDs and SeriesIDs carry the content contexts of a batch. Empty + // slices drop their scope from the candidate set. + LibraryIDs []int + SeriesIDs []string +} + +// Normalized returns the query with blanks removed and duplicates collapsed, in +// a stable order. Both backends bind the normalized form, so an empty or +// whitespace-only id never reaches SQL as a literal and the two backends issue +// the same predicate for the same request. +func (q SettingResolutionQuery) Normalized() SettingResolutionQuery { + return SettingResolutionQuery{ + Keys: compactStrings(q.Keys), + ProfileID: strings.TrimSpace(q.ProfileID), + DeviceID: strings.TrimSpace(q.DeviceID), + LibraryIDs: compactPositiveInts(q.LibraryIDs), + SeriesIDs: compactStrings(q.SeriesIDs), + } +} + +// SettingMutationRecord is the idempotency receipt for one mutation. +// +// The mutation endpoint treats a mutation_id as idempotent for at least 30 days: +// repeating the same id and body returns the prior Result, and reusing an id +// with different content is a mutation_id_conflict, which is what RequestHash +// distinguishes. +type SettingMutationRecord struct { + MutationID string + RequestHash string + // Result is the serialized per-mutation result returned to a repeat of the + // same request. + Result json.RawMessage + // CreatedAt is set by the store when the record is inserted. + CreatedAt time.Time + // ExpiresAt bounds retention. It is not self-enforcing: a sweeper deletes + // expired rows through DeleteExpiredSettingMutations. + ExpiresAt time.Time +} + +// Validate reports whether the record is storable. +func (r SettingMutationRecord) Validate() error { + if strings.TrimSpace(r.MutationID) == "" { + return fmt.Errorf("%w: mutation id is required", ErrInvalidSettingIdentity) + } + if strings.TrimSpace(r.RequestHash) == "" { + return fmt.Errorf("%w: request hash is required", ErrInvalidSettingIdentity) + } + if r.ExpiresAt.IsZero() { + return fmt.Errorf("%w: expires_at is required", ErrInvalidSettingIdentity) + } + return ValidateSettingValueJSON(r.Result) +} + +// ValidateSettingValueJSON checks that raw is a non-empty, well-formed JSON +// document. It is the only value check the store makes: the contract layer has +// already validated the value against its definition through +// settingscontract.NormalizeValue, and duplicating that here would be the second +// validator this contract exists to remove. +func ValidateSettingValueJSON(raw json.RawMessage) error { + if len(raw) == 0 { + return fmt.Errorf("%w: value is required", ErrInvalidSettingValue) + } + if !json.Valid(raw) { + return fmt.Errorf("%w: value is not well-formed JSON", ErrInvalidSettingValue) + } + return nil +} + +func compactStrings(values []string) []string { + if len(values) == 0 { + return nil + } + seen := make(map[string]struct{}, len(values)) + out := make([]string, 0, len(values)) + for _, value := range values { + trimmed := strings.TrimSpace(value) + if trimmed == "" { + continue + } + if _, dup := seen[trimmed]; dup { + continue + } + seen[trimmed] = struct{}{} + out = append(out, trimmed) + } + if len(out) == 0 { + return nil + } + sort.Strings(out) + return out +} + +func compactPositiveInts(values []int) []int { + if len(values) == 0 { + return nil + } + seen := make(map[int]struct{}, len(values)) + out := make([]int, 0, len(values)) + for _, value := range values { + if value <= 0 { + continue + } + if _, dup := seen[value]; dup { + continue + } + seen[value] = struct{}{} + out = append(out, value) + } + if len(out) == 0 { + return nil + } + sort.Ints(out) + return out +} diff --git a/internal/userstore/store.go b/internal/userstore/store.go index 61e40b7e..554f58ab 100644 --- a/internal/userstore/store.go +++ b/internal/userstore/store.go @@ -2,6 +2,7 @@ package userstore import ( "context" + "encoding/json" "errors" "time" ) @@ -130,6 +131,49 @@ type UserStore interface { ListLibraryPlaybackPreferences(ctx context.Context, profileID string) ([]LibraryPlaybackPreference, error) UpsertLibraryPlaybackPreference(ctx context.Context, pref LibraryPlaybackPreference) error DeleteLibraryPlaybackPreference(ctx context.Context, profileID string, libraryID int) error + + // Canonical typed setting values (contracts/settings/v1). + // + // These back the settings contract's storage layer. The manifest remains + // the schema; the store holds validated JSON keyed by scope identity, and + // knows nothing about definitions, defaults or resolution order. + + // GetSettingValue returns the explicit value at exactly one scope, or nil + // when that identity is unset. It does not resolve fallbacks. + GetSettingValue(ctx context.Context, id SettingIdentity) (*SettingValue, error) + // ListSettingValuesForResolution returns every candidate row for one + // resolution request in a single query, unranked. The resolver applies each + // definition's resolution order in Go; issuing one lookup per scope is a + // rejected implementation. + ListSettingValuesForResolution(ctx context.Context, query SettingResolutionQuery) ([]SettingValue, error) + // UpsertSettingValue writes the explicit value at one scope and increments + // that row's revision. Concurrent writes to one identity are + // last-write-wins in server receipt order; there is no compare-and-set + // precondition in v1. + UpsertSettingValue(ctx context.Context, id SettingIdentity, value json.RawMessage) (*SettingValue, error) + // DeleteSettingValue removes the explicit value at one scope — the `unset` + // operation — and reports whether a row existed. + DeleteSettingValue(ctx context.Context, id SettingIdentity) (bool, error) + + // The scoped deletes below are application-enforced cleanup for identities + // this table cannot reference: the per-user SQLite store declares no foreign + // keys, and libraries, series and devices are not FK targets in Postgres + // either. Each removes only the rows scoped to the named entity. + DeleteSettingValuesForProfile(ctx context.Context, profileID string) (int64, error) + DeleteSettingValuesForDevice(ctx context.Context, profileID, deviceID string) (int64, error) + DeleteSettingValuesForLibrary(ctx context.Context, libraryID int) (int64, error) + DeleteSettingValuesForSeries(ctx context.Context, seriesID string) (int64, error) + + // GetSettingMutation returns a recorded idempotency receipt, or nil. + GetSettingMutation(ctx context.Context, mutationID string) (*SettingMutationRecord, error) + // PutSettingMutation records a receipt without ever overwriting one. When + // the id is already recorded it returns the stored record with + // inserted=false, so the caller compares request hashes and answers + // already_applied or mutation_id_conflict. + PutSettingMutation(ctx context.Context, record SettingMutationRecord) (SettingMutationRecord, bool, error) + // DeleteExpiredSettingMutations removes receipts that expired before the + // given instant and reports how many. expires_at is not self-enforcing. + DeleteExpiredSettingMutations(ctx context.Context, before time.Time) (int64, error) } // DeviceRegistry is implemented by stores that track observed devices even diff --git a/internal/userstore/storetest/settingvalues.go b/internal/userstore/storetest/settingvalues.go new file mode 100644 index 00000000..987649fb --- /dev/null +++ b/internal/userstore/storetest/settingvalues.go @@ -0,0 +1,852 @@ +package storetest + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "reflect" + "sort" + "testing" + "time" + + "github.com/Silo-Server/silo-server/internal/settingscontract" + "github.com/Silo-Server/silo-server/internal/userstore" +) + +// RunSettingValues runs the canonical settings-contract storage conformance +// tests. It is exposed separately from RunSuite so each backend can pin this +// behavior on its own, which is what keeps the PostgreSQL and per-user SQLite +// stores from drifting on the table the whole contract rests on. +func RunSettingValues(t *testing.T, newStore func(t *testing.T) userstore.UserStore) { + t.Run("ExplicitValuesPerScope", func(t *testing.T) { + testSettingValueScopes(t, newStore) + }) + t.Run("UnsetIsNotFalsy", func(t *testing.T) { + testSettingValueUnsetIsNotFalsy(t, newStore) + }) + t.Run("RevisionIncrements", func(t *testing.T) { + testSettingValueRevisions(t, newStore) + }) + t.Run("PartialUniqueness", func(t *testing.T) { + testSettingValuePartialUniqueness(t, newStore) + }) + t.Run("IdentityValidation", func(t *testing.T) { + testSettingValueIdentityValidation(t, newStore) + }) + t.Run("ResolutionCandidates", func(t *testing.T) { + testSettingValueResolution(t, newStore) + }) + t.Run("DeletePaths", func(t *testing.T) { + testSettingValueDeletePaths(t, newStore) + }) + t.Run("MutationIdempotency", func(t *testing.T) { + testSettingMutationIdempotency(t, newStore) + }) +} + +const ( + audioKey = "playback.audio_language" + subtitleKey = "playback.subtitle_mode" +) + +// seedSettingProfiles creates the profiles every setting-value test addresses. +// The PostgreSQL table carries a composite profile FK, so a profile-anchored row +// cannot be written for a profile that does not exist. +func seedSettingProfiles(t *testing.T, ctx context.Context, store userstore.UserStore, ids ...string) { + t.Helper() + for _, id := range ids { + if err := store.CreateProfile(ctx, userstore.Profile{ID: id, Name: "Profile " + id}); err != nil { + t.Fatalf("CreateProfile(%s): %v", id, err) + } + } +} + +func accountID(key string) userstore.SettingIdentity { + return userstore.SettingIdentity{Key: key, Scope: settingscontract.ScopeAccount} +} + +func profileID(key, profile string) userstore.SettingIdentity { + return userstore.SettingIdentity{Key: key, Scope: settingscontract.ScopeProfile, ProfileID: profile} +} + +func deviceID(key, profile, device string) userstore.SettingIdentity { + return userstore.SettingIdentity{ + Key: key, Scope: settingscontract.ScopeProfileDevice, ProfileID: profile, DeviceID: device, + } +} + +func libraryID(key, profile string, library int) userstore.SettingIdentity { + return userstore.SettingIdentity{ + Key: key, Scope: settingscontract.ScopeProfileLibrary, ProfileID: profile, LibraryID: library, + } +} + +func seriesID(key, profile, series string) userstore.SettingIdentity { + return userstore.SettingIdentity{ + Key: key, Scope: settingscontract.ScopeProfileSeries, ProfileID: profile, SeriesID: series, + } +} + +func mustUpsert( + t *testing.T, + ctx context.Context, + store userstore.UserStore, + id userstore.SettingIdentity, + value string, +) userstore.SettingValue { + t.Helper() + stored, err := store.UpsertSettingValue(ctx, id, json.RawMessage(value)) + if err != nil { + t.Fatalf("UpsertSettingValue(%s at %s): %v", id.Key, id.Scope, err) + } + if stored == nil { + t.Fatalf("UpsertSettingValue(%s at %s) returned nil", id.Key, id.Scope) + } + if stored.SettingIdentity != id { + t.Fatalf("UpsertSettingValue echoed identity %+v, want %+v", stored.SettingIdentity, id) + } + if !jsonEqual(stored.Value, json.RawMessage(value)) { + t.Fatalf("UpsertSettingValue stored %s, want %s", stored.Value, value) + } + return *stored +} + +// testSettingValueScopes pins that every remote scope stores and reads back its +// own explicit value, that scopes do not read each other, and that an unset +// identity is nil rather than a zero value. +func testSettingValueScopes(t *testing.T, newStore func(t *testing.T) userstore.UserStore) { + ctx := context.Background() + store := newStore(t) + seedSettingProfiles(t, ctx, store, "p1") + + cases := []struct { + name string + id userstore.SettingIdentity + value string + }{ + {"account", accountID(audioKey), `"en"`}, + {"profile", profileID(audioKey, "p1"), `"fr"`}, + {"profile_device", deviceID(audioKey, "p1", "apple-tv"), `"de"`}, + {"profile_library", libraryID(audioKey, "p1", 42), `"es"`}, + {"profile_series", seriesID(audioKey, "p1", "series-1"), `"ja"`}, + } + + for _, tc := range cases { + missing, err := store.GetSettingValue(ctx, tc.id) + if err != nil { + t.Fatalf("GetSettingValue(%s, unset): %v", tc.name, err) + } + if missing != nil { + t.Fatalf("GetSettingValue(%s, unset) = %+v, want nil", tc.name, missing) + } + } + + for _, tc := range cases { + mustUpsert(t, ctx, store, tc.id, tc.value) + } + + for _, tc := range cases { + got, err := store.GetSettingValue(ctx, tc.id) + if err != nil { + t.Fatalf("GetSettingValue(%s): %v", tc.name, err) + } + if got == nil { + t.Fatalf("GetSettingValue(%s) = nil, want a stored value", tc.name) + } + if !jsonEqual(got.Value, json.RawMessage(tc.value)) { + t.Fatalf("GetSettingValue(%s) = %s, want %s", tc.name, got.Value, tc.value) + } + if got.Revision != 1 { + t.Fatalf("GetSettingValue(%s) revision = %d, want 1", tc.name, got.Revision) + } + if got.CreatedAt == "" || got.UpdatedAt == "" { + t.Fatalf("GetSettingValue(%s) timestamps = %q/%q, want both set", tc.name, got.CreatedAt, got.UpdatedAt) + } + if _, err := time.Parse(time.RFC3339, got.UpdatedAt); err != nil { + t.Fatalf("GetSettingValue(%s) updated_at %q is not RFC3339: %v", tc.name, got.UpdatedAt, err) + } + } + + // A different profile, device, library or series is a different identity and + // must not see the values above. + seedSettingProfiles(t, ctx, store, "p2") + for _, id := range []userstore.SettingIdentity{ + profileID(audioKey, "p2"), + deviceID(audioKey, "p1", "iphone"), + libraryID(audioKey, "p1", 43), + seriesID(audioKey, "p1", "series-2"), + } { + got, err := store.GetSettingValue(ctx, id) + if err != nil { + t.Fatalf("GetSettingValue(neighbor %+v): %v", id, err) + } + if got != nil { + t.Fatalf("GetSettingValue(neighbor %+v) = %+v, want nil", id, got) + } + } +} + +// testSettingValueUnsetIsNotFalsy pins the distinction the whole contract rests +// on: false, 0, "" and JSON null are stored values, and only deleting the row +// makes a setting unset. +func testSettingValueUnsetIsNotFalsy(t *testing.T, newStore func(t *testing.T) userstore.UserStore) { + ctx := context.Background() + store := newStore(t) + seedSettingProfiles(t, ctx, store, "p1") + + falsy := []struct { + key string + value string + }{ + {"playback.show_forced_subtitles", `false`}, + {"playback.next_up_prompt_seconds", `0`}, + {"catalog.metadata_language", `""`}, + {"playback.subtitle_language", `null`}, + } + + for _, tc := range falsy { + id := profileID(tc.key, "p1") + mustUpsert(t, ctx, store, id, tc.value) + + got, err := store.GetSettingValue(ctx, id) + if err != nil { + t.Fatalf("GetSettingValue(%s): %v", tc.key, err) + } + if got == nil { + t.Fatalf("GetSettingValue(%s) = nil; %s must be a stored value, not unset", tc.key, tc.value) + } + if !jsonEqual(got.Value, json.RawMessage(tc.value)) { + t.Fatalf("GetSettingValue(%s) = %s, want %s", tc.key, got.Value, tc.value) + } + + removed, err := store.DeleteSettingValue(ctx, id) + if err != nil { + t.Fatalf("DeleteSettingValue(%s): %v", tc.key, err) + } + if !removed { + t.Fatalf("DeleteSettingValue(%s) reported no row; %s was stored", tc.key, tc.value) + } + + got, err = store.GetSettingValue(ctx, id) + if err != nil { + t.Fatalf("GetSettingValue(%s, after unset): %v", tc.key, err) + } + if got != nil { + t.Fatalf("GetSettingValue(%s, after unset) = %+v, want nil", tc.key, got) + } + + removed, err = store.DeleteSettingValue(ctx, id) + if err != nil { + t.Fatalf("DeleteSettingValue(%s, repeat): %v", tc.key, err) + } + if removed { + t.Fatalf("DeleteSettingValue(%s, repeat) reported a row; the value was already unset", tc.key) + } + } +} + +// testSettingValueRevisions pins last-write-wins with a per-row revision: each +// write replaces the value and increments revision, and created_at is not +// rewritten. +func testSettingValueRevisions(t *testing.T, newStore func(t *testing.T) userstore.UserStore) { + ctx := context.Background() + store := newStore(t) + seedSettingProfiles(t, ctx, store, "p1") + + id := profileID(audioKey, "p1") + first := mustUpsert(t, ctx, store, id, `"en"`) + if first.Revision != 1 { + t.Fatalf("first write revision = %d, want 1", first.Revision) + } + + second := mustUpsert(t, ctx, store, id, `"ja"`) + if second.Revision != 2 { + t.Fatalf("second write revision = %d, want 2", second.Revision) + } + if second.CreatedAt != first.CreatedAt { + t.Fatalf("second write rewrote created_at %q -> %q", first.CreatedAt, second.CreatedAt) + } + + third := mustUpsert(t, ctx, store, id, `"de"`) + if third.Revision != 3 { + t.Fatalf("third write revision = %d, want 3", third.Revision) + } + + got, err := store.GetSettingValue(ctx, id) + if err != nil { + t.Fatalf("GetSettingValue: %v", err) + } + if got == nil || !jsonEqual(got.Value, json.RawMessage(`"de"`)) || got.Revision != 3 { + t.Fatalf("GetSettingValue = %+v, want the newest write at revision 3", got) + } + + // A re-set after an unset starts a fresh row rather than resurrecting the + // old revision counter. + if _, err := store.DeleteSettingValue(ctx, id); err != nil { + t.Fatalf("DeleteSettingValue: %v", err) + } + reset := mustUpsert(t, ctx, store, id, `"it"`) + if reset.Revision != 1 { + t.Fatalf("revision after unset/re-set = %d, want 1", reset.Revision) + } +} + +// testSettingValuePartialUniqueness pins the five partial unique indexes: one +// explicit value per identity, and identities that differ in any one context +// column coexist. +func testSettingValuePartialUniqueness(t *testing.T, newStore func(t *testing.T) userstore.UserStore) { + ctx := context.Background() + store := newStore(t) + seedSettingProfiles(t, ctx, store, "p1", "p2") + + identities := []userstore.SettingIdentity{ + accountID(audioKey), + profileID(audioKey, "p1"), + profileID(audioKey, "p2"), + deviceID(audioKey, "p1", "apple-tv"), + deviceID(audioKey, "p1", "iphone"), + deviceID(audioKey, "p2", "apple-tv"), + libraryID(audioKey, "p1", 1), + libraryID(audioKey, "p1", 2), + seriesID(audioKey, "p1", "s-1"), + seriesID(audioKey, "p1", "s-2"), + } + + // Two writes each: the second must update its own row, never insert a + // duplicate at the same identity. + for _, id := range identities { + mustUpsert(t, ctx, store, id, `"en"`) + mustUpsert(t, ctx, store, id, `"ja"`) + } + + rows, err := store.ListSettingValuesForResolution(ctx, userstore.SettingResolutionQuery{ + Keys: []string{audioKey}, + ProfileID: "p1", + DeviceID: "apple-tv", + LibraryIDs: []int{1, 2}, + SeriesIDs: []string{"s-1", "s-2"}, + }) + if err != nil { + t.Fatalf("ListSettingValuesForResolution: %v", err) + } + // p1's candidates: account, profile, one device, two libraries, two series. + want := []userstore.SettingIdentity{ + accountID(audioKey), + profileID(audioKey, "p1"), + deviceID(audioKey, "p1", "apple-tv"), + libraryID(audioKey, "p1", 1), + libraryID(audioKey, "p1", 2), + seriesID(audioKey, "p1", "s-1"), + seriesID(audioKey, "p1", "s-2"), + } + assertIdentitySet(t, rows, want) + for _, row := range rows { + if row.Revision != 2 { + t.Fatalf("identity %+v has revision %d, want 2 — the second write inserted a duplicate row", + row.SettingIdentity, row.Revision) + } + } +} + +// testSettingValueIdentityValidation pins that both backends reject the same +// malformed identities and values, with the same sentinel errors, before any SQL +// runs. A scope CHECK violation surfacing as a driver error would read +// differently in each backend. +func testSettingValueIdentityValidation(t *testing.T, newStore func(t *testing.T) userstore.UserStore) { + ctx := context.Background() + store := newStore(t) + seedSettingProfiles(t, ctx, store, "p1") + + invalid := []struct { + name string + id userstore.SettingIdentity + }{ + {"empty key", userstore.SettingIdentity{Scope: settingscontract.ScopeProfile, ProfileID: "p1"}}, + {"blank key", userstore.SettingIdentity{Key: " ", Scope: settingscontract.ScopeProfile, ProfileID: "p1"}}, + {"unknown scope", userstore.SettingIdentity{Key: audioKey, Scope: "wishful"}}, + {"client_local is not remote", userstore.SettingIdentity{Key: audioKey, Scope: settingscontract.ScopeClientLocal}}, + {"default is not remote", userstore.SettingIdentity{Key: audioKey, Scope: settingscontract.ScopeDefault}}, + {"profile scope without profile", userstore.SettingIdentity{Key: audioKey, Scope: settingscontract.ScopeProfile}}, + {"account scope with profile", userstore.SettingIdentity{ + Key: audioKey, Scope: settingscontract.ScopeAccount, ProfileID: "p1", + }}, + {"device scope without device", userstore.SettingIdentity{ + Key: audioKey, Scope: settingscontract.ScopeProfileDevice, ProfileID: "p1", + }}, + {"profile scope with device", userstore.SettingIdentity{ + Key: audioKey, Scope: settingscontract.ScopeProfile, ProfileID: "p1", DeviceID: "apple-tv", + }}, + {"library scope without library", userstore.SettingIdentity{ + Key: audioKey, Scope: settingscontract.ScopeProfileLibrary, ProfileID: "p1", + }}, + {"series scope with library", userstore.SettingIdentity{ + Key: audioKey, Scope: settingscontract.ScopeProfileSeries, ProfileID: "p1", SeriesID: "s-1", LibraryID: 4, + }}, + {"series scope without series", userstore.SettingIdentity{ + Key: audioKey, Scope: settingscontract.ScopeProfileSeries, ProfileID: "p1", + }}, + } + + for _, tc := range invalid { + if _, err := store.UpsertSettingValue(ctx, tc.id, json.RawMessage(`"en"`)); !errors.Is(err, userstore.ErrInvalidSettingIdentity) { + t.Fatalf("UpsertSettingValue(%s) error = %v, want ErrInvalidSettingIdentity", tc.name, err) + } + if _, err := store.GetSettingValue(ctx, tc.id); !errors.Is(err, userstore.ErrInvalidSettingIdentity) { + t.Fatalf("GetSettingValue(%s) error = %v, want ErrInvalidSettingIdentity", tc.name, err) + } + if _, err := store.DeleteSettingValue(ctx, tc.id); !errors.Is(err, userstore.ErrInvalidSettingIdentity) { + t.Fatalf("DeleteSettingValue(%s) error = %v, want ErrInvalidSettingIdentity", tc.name, err) + } + } + + valid := profileID(audioKey, "p1") + for _, tc := range []struct { + name string + value json.RawMessage + }{ + {"empty", nil}, + {"truncated object", json.RawMessage(`{"fontScale":`)}, + {"bare word", json.RawMessage(`nope`)}, + } { + if _, err := store.UpsertSettingValue(ctx, valid, tc.value); !errors.Is(err, userstore.ErrInvalidSettingValue) { + t.Fatalf("UpsertSettingValue(%s value) error = %v, want ErrInvalidSettingValue", tc.name, err) + } + } +} + +// testSettingValueResolution pins the read path's normative rule: one query +// returns every candidate row for a resolution request, unranked, and nothing +// belonging to another identity. Ranking is the resolver's job in Go. +func testSettingValueResolution(t *testing.T, newStore func(t *testing.T) userstore.UserStore) { + ctx := context.Background() + store := newStore(t) + seedSettingProfiles(t, ctx, store, "p1", "p2") + + mustUpsert(t, ctx, store, accountID(audioKey), `"en"`) + mustUpsert(t, ctx, store, profileID(audioKey, "p1"), `"fr"`) + mustUpsert(t, ctx, store, deviceID(audioKey, "p1", "apple-tv"), `"de"`) + mustUpsert(t, ctx, store, libraryID(audioKey, "p1", 42), `"es"`) + mustUpsert(t, ctx, store, seriesID(audioKey, "p1", "s-1"), `"ja"`) + mustUpsert(t, ctx, store, seriesID(audioKey, "p1", "s-2"), `"ko"`) + mustUpsert(t, ctx, store, profileID(subtitleKey, "p1"), `"forced"`) + + // Decoys: another profile, another device, another library, another series, + // and a key nobody asked for. + mustUpsert(t, ctx, store, profileID(audioKey, "p2"), `"pt"`) + mustUpsert(t, ctx, store, deviceID(audioKey, "p1", "iphone"), `"nl"`) + mustUpsert(t, ctx, store, libraryID(audioKey, "p1", 43), `"sv"`) + mustUpsert(t, ctx, store, seriesID(audioKey, "p1", "s-3"), `"da"`) + mustUpsert(t, ctx, store, profileID("ui.library_page_state", "p1"), `{"sort":"title"}`) + + // The batched shape: two content contexts resolved in one call. + rows, err := store.ListSettingValuesForResolution(ctx, userstore.SettingResolutionQuery{ + Keys: []string{audioKey, subtitleKey}, + ProfileID: "p1", + DeviceID: "apple-tv", + LibraryIDs: []int{42}, + SeriesIDs: []string{"s-1", "s-2"}, + }) + if err != nil { + t.Fatalf("ListSettingValuesForResolution: %v", err) + } + assertIdentitySet(t, rows, []userstore.SettingIdentity{ + accountID(audioKey), + profileID(audioKey, "p1"), + deviceID(audioKey, "p1", "apple-tv"), + libraryID(audioKey, "p1", 42), + seriesID(audioKey, "p1", "s-1"), + seriesID(audioKey, "p1", "s-2"), + profileID(subtitleKey, "p1"), + }) + + // A batch resolves the same rows n single-context calls would, which is what + // lets a list view make one round trip instead of one per item. + for _, series := range []string{"s-1", "s-2"} { + single, err := store.ListSettingValuesForResolution(ctx, userstore.SettingResolutionQuery{ + Keys: []string{audioKey}, + ProfileID: "p1", + DeviceID: "apple-tv", + SeriesIDs: []string{series}, + }) + if err != nil { + t.Fatalf("ListSettingValuesForResolution(%s): %v", series, err) + } + assertIdentitySet(t, single, []userstore.SettingIdentity{ + accountID(audioKey), + profileID(audioKey, "p1"), + deviceID(audioKey, "p1", "apple-tv"), + seriesID(audioKey, "p1", series), + }) + } + + // No device identity — an incognito window, or jellycompat's seed — drops + // profile_device candidates without touching the roaming ones. + noDevice, err := store.ListSettingValuesForResolution(ctx, userstore.SettingResolutionQuery{ + Keys: []string{audioKey}, + ProfileID: "p1", + }) + if err != nil { + t.Fatalf("ListSettingValuesForResolution(no device): %v", err) + } + assertIdentitySet(t, noDevice, []userstore.SettingIdentity{ + accountID(audioKey), + profileID(audioKey, "p1"), + }) + + // No profile at all leaves only account scope. + accountOnly, err := store.ListSettingValuesForResolution(ctx, userstore.SettingResolutionQuery{ + Keys: []string{audioKey}, + }) + if err != nil { + t.Fatalf("ListSettingValuesForResolution(account only): %v", err) + } + assertIdentitySet(t, accountOnly, []userstore.SettingIdentity{accountID(audioKey)}) + + // Blank and duplicate context ids are compacted rather than bound as + // literals, so they neither match a '' row nor multiply the result set. + dirty, err := store.ListSettingValuesForResolution(ctx, userstore.SettingResolutionQuery{ + Keys: []string{audioKey, "", audioKey, " "}, + ProfileID: "p1", + DeviceID: "apple-tv", + LibraryIDs: []int{42, 42, 0, -1}, + SeriesIDs: []string{"s-1", "s-1", "", " "}, + }) + if err != nil { + t.Fatalf("ListSettingValuesForResolution(dirty): %v", err) + } + assertIdentitySet(t, dirty, []userstore.SettingIdentity{ + accountID(audioKey), + profileID(audioKey, "p1"), + deviceID(audioKey, "p1", "apple-tv"), + libraryID(audioKey, "p1", 42), + seriesID(audioKey, "p1", "s-1"), + }) + + // No keys is not an error and is not "everything". + none, err := store.ListSettingValuesForResolution(ctx, userstore.SettingResolutionQuery{ProfileID: "p1"}) + if err != nil { + t.Fatalf("ListSettingValuesForResolution(no keys): %v", err) + } + if len(none) != 0 { + t.Fatalf("ListSettingValuesForResolution(no keys) = %d rows, want 0", len(none)) + } + + // An unknown key resolves to nothing rather than erroring; rejecting unknown + // keys is the contract layer's job, not the store's. + unknown, err := store.ListSettingValuesForResolution(ctx, userstore.SettingResolutionQuery{ + Keys: []string{"playback.not_a_setting"}, + ProfileID: "p1", + }) + if err != nil { + t.Fatalf("ListSettingValuesForResolution(unknown key): %v", err) + } + if len(unknown) != 0 { + t.Fatalf("ListSettingValuesForResolution(unknown key) = %d rows, want 0", len(unknown)) + } +} + +// testSettingValueDeletePaths pins the application-enforced delete behavior. +// Neither backend can inherit this from constraints: the per-user SQLite store +// declares no foreign keys, and library, series and device columns reference +// nothing in PostgreSQL either. +func testSettingValueDeletePaths(t *testing.T, newStore func(t *testing.T) userstore.UserStore) { + ctx := context.Background() + + // seed writes one value at every scope for two profiles, two devices, two + // libraries and two series, so each delete can be checked for over-reach. + seed := func(t *testing.T) (userstore.UserStore, []userstore.SettingIdentity) { + t.Helper() + store := newStore(t) + seedSettingProfiles(t, ctx, store, "p1", "p2") + identities := []userstore.SettingIdentity{ + accountID(audioKey), + profileID(audioKey, "p1"), + profileID(audioKey, "p2"), + deviceID(audioKey, "p1", "apple-tv"), + deviceID(audioKey, "p1", "iphone"), + deviceID(audioKey, "p2", "apple-tv"), + libraryID(audioKey, "p1", 1), + libraryID(audioKey, "p1", 2), + libraryID(audioKey, "p2", 1), + seriesID(audioKey, "p1", "s-1"), + seriesID(audioKey, "p1", "s-2"), + seriesID(audioKey, "p2", "s-1"), + } + for _, id := range identities { + mustUpsert(t, ctx, store, id, `"en"`) + } + return store, identities + } + + assertRemaining := func(t *testing.T, store userstore.UserStore, all, removed []userstore.SettingIdentity) { + t.Helper() + gone := make(map[userstore.SettingIdentity]struct{}, len(removed)) + for _, id := range removed { + gone[id] = struct{}{} + } + for _, id := range all { + got, err := store.GetSettingValue(ctx, id) + if err != nil { + t.Fatalf("GetSettingValue(%+v): %v", id, err) + } + _, shouldBeGone := gone[id] + if shouldBeGone && got != nil { + t.Fatalf("identity %+v survived a delete that owns it", id) + } + if !shouldBeGone && got == nil { + t.Fatalf("identity %+v was removed by a delete that does not own it", id) + } + } + } + + t.Run("Device", func(t *testing.T) { + store, all := seed(t) + removed, err := store.DeleteSettingValuesForDevice(ctx, "p1", "apple-tv") + if err != nil { + t.Fatalf("DeleteSettingValuesForDevice: %v", err) + } + if removed != 1 { + t.Fatalf("DeleteSettingValuesForDevice removed %d rows, want 1", removed) + } + assertRemaining(t, store, all, []userstore.SettingIdentity{deviceID(audioKey, "p1", "apple-tv")}) + }) + + t.Run("ForgetDeviceThroughDeviceSettings", func(t *testing.T) { + store, all := seed(t) + // DeleteAllDeviceSettings is the forget-device path: it must clear the + // canonical profile_device values alongside the legacy string overrides. + if err := store.SetDeviceSetting(ctx, userstore.DeviceSettingEntry{ + ProfileID: "p1", DeviceID: "apple-tv", Key: "player.playback_speed", Value: "1.25", + }); err != nil { + t.Fatalf("SetDeviceSetting: %v", err) + } + if err := store.DeleteAllDeviceSettings(ctx, "p1", "apple-tv"); err != nil { + t.Fatalf("DeleteAllDeviceSettings: %v", err) + } + legacy, err := store.GetDeviceSetting(ctx, "p1", "apple-tv", "player.playback_speed") + if err != nil { + t.Fatalf("GetDeviceSetting after forget: %v", err) + } + if legacy != nil { + t.Fatalf("GetDeviceSetting after forget = %+v, want nil", legacy) + } + assertRemaining(t, store, all, []userstore.SettingIdentity{deviceID(audioKey, "p1", "apple-tv")}) + }) + + t.Run("Library", func(t *testing.T) { + store, all := seed(t) + removed, err := store.DeleteSettingValuesForLibrary(ctx, 1) + if err != nil { + t.Fatalf("DeleteSettingValuesForLibrary: %v", err) + } + if removed != 2 { + t.Fatalf("DeleteSettingValuesForLibrary removed %d rows, want 2 (one per profile)", removed) + } + assertRemaining(t, store, all, []userstore.SettingIdentity{ + libraryID(audioKey, "p1", 1), + libraryID(audioKey, "p2", 1), + }) + }) + + t.Run("Series", func(t *testing.T) { + store, all := seed(t) + removed, err := store.DeleteSettingValuesForSeries(ctx, "s-1") + if err != nil { + t.Fatalf("DeleteSettingValuesForSeries: %v", err) + } + if removed != 2 { + t.Fatalf("DeleteSettingValuesForSeries removed %d rows, want 2 (one per profile)", removed) + } + assertRemaining(t, store, all, []userstore.SettingIdentity{ + seriesID(audioKey, "p1", "s-1"), + seriesID(audioKey, "p2", "s-1"), + }) + }) + + t.Run("Profile", func(t *testing.T) { + store, all := seed(t) + removed, err := store.DeleteSettingValuesForProfile(ctx, "p1") + if err != nil { + t.Fatalf("DeleteSettingValuesForProfile: %v", err) + } + if removed != 7 { + t.Fatalf("DeleteSettingValuesForProfile removed %d rows, want 7", removed) + } + assertRemaining(t, store, all, []userstore.SettingIdentity{ + profileID(audioKey, "p1"), + deviceID(audioKey, "p1", "apple-tv"), + deviceID(audioKey, "p1", "iphone"), + libraryID(audioKey, "p1", 1), + libraryID(audioKey, "p1", 2), + seriesID(audioKey, "p1", "s-1"), + seriesID(audioKey, "p1", "s-2"), + }) + }) + + t.Run("DeleteProfileCascades", func(t *testing.T) { + store, all := seed(t) + if err := store.DeleteProfile(ctx, "p1"); err != nil { + t.Fatalf("DeleteProfile: %v", err) + } + // Account scope belongs to the account, not to any one household member. + assertRemaining(t, store, all, []userstore.SettingIdentity{ + profileID(audioKey, "p1"), + deviceID(audioKey, "p1", "apple-tv"), + deviceID(audioKey, "p1", "iphone"), + libraryID(audioKey, "p1", 1), + libraryID(audioKey, "p1", 2), + seriesID(audioKey, "p1", "s-1"), + seriesID(audioKey, "p1", "s-2"), + }) + }) +} + +// testSettingMutationIdempotency pins the receipt storage behind +// mutation_id idempotency: a receipt is written once and never overwritten, a +// replay reads back the original result, and expired receipts are sweepable. +func testSettingMutationIdempotency(t *testing.T, newStore func(t *testing.T) userstore.UserStore) { + ctx := context.Background() + store := newStore(t) + + expires := time.Now().UTC().Add(30 * 24 * time.Hour).Truncate(time.Second) + record := userstore.SettingMutationRecord{ + MutationID: "8cc515ad-88c5-48f0-a6cc-44d0a870e32c", + RequestHash: "hash-a", + Result: json.RawMessage(`{"status":"applied"}`), + ExpiresAt: expires, + } + + missing, err := store.GetSettingMutation(ctx, record.MutationID) + if err != nil { + t.Fatalf("GetSettingMutation(unrecorded): %v", err) + } + if missing != nil { + t.Fatalf("GetSettingMutation(unrecorded) = %+v, want nil", missing) + } + + stored, inserted, err := store.PutSettingMutation(ctx, record) + if err != nil { + t.Fatalf("PutSettingMutation: %v", err) + } + if !inserted { + t.Fatal("PutSettingMutation reported no insertion for a new mutation id") + } + if stored.RequestHash != "hash-a" || !jsonEqual(stored.Result, record.Result) { + t.Fatalf("PutSettingMutation stored %+v, want the submitted receipt", stored) + } + if !stored.ExpiresAt.Equal(expires) { + t.Fatalf("PutSettingMutation expires_at = %s, want %s", stored.ExpiresAt, expires) + } + if stored.CreatedAt.IsZero() { + t.Fatal("PutSettingMutation left created_at zero") + } + + // A replay of the same id must read back the first result, whatever the + // second attempt carries: that is what makes a retry idempotent instead of a + // silent re-run, and what lets the caller answer mutation_id_conflict. + replay := record + replay.RequestHash = "hash-b" + replay.Result = json.RawMessage(`{"status":"invalid_value"}`) + existing, inserted, err := store.PutSettingMutation(ctx, replay) + if err != nil { + t.Fatalf("PutSettingMutation(replay): %v", err) + } + if inserted { + t.Fatal("PutSettingMutation(replay) reported an insertion; the receipt already existed") + } + if existing.RequestHash != "hash-a" { + t.Fatalf("PutSettingMutation(replay) request hash = %q, want the original hash-a", existing.RequestHash) + } + if !jsonEqual(existing.Result, record.Result) { + t.Fatalf("PutSettingMutation(replay) result = %s, want the original result", existing.Result) + } + + got, err := store.GetSettingMutation(ctx, record.MutationID) + if err != nil { + t.Fatalf("GetSettingMutation: %v", err) + } + if got == nil || got.RequestHash != "hash-a" { + t.Fatalf("GetSettingMutation = %+v, want the original receipt", got) + } + + // Expiry is not self-enforcing; the sweeper removes only what has expired. + expired := userstore.SettingMutationRecord{ + MutationID: "5ae96ffc-1077-4da8-8f64-a1ca9c3c72b8", + RequestHash: "hash-c", + Result: json.RawMessage(`{"status":"applied"}`), + ExpiresAt: time.Now().UTC().Add(-time.Hour), + } + if _, _, err := store.PutSettingMutation(ctx, expired); err != nil { + t.Fatalf("PutSettingMutation(expired): %v", err) + } + + swept, err := store.DeleteExpiredSettingMutations(ctx, time.Now().UTC()) + if err != nil { + t.Fatalf("DeleteExpiredSettingMutations: %v", err) + } + if swept != 1 { + t.Fatalf("DeleteExpiredSettingMutations swept %d rows, want 1", swept) + } + if got, err := store.GetSettingMutation(ctx, expired.MutationID); err != nil || got != nil { + t.Fatalf("GetSettingMutation(expired) = %+v (%v), want nil", got, err) + } + if got, err := store.GetSettingMutation(ctx, record.MutationID); err != nil || got == nil { + t.Fatalf("GetSettingMutation(live) = %+v (%v), want the unexpired receipt", got, err) + } + + invalid := []userstore.SettingMutationRecord{ + {RequestHash: "h", Result: json.RawMessage(`{}`), ExpiresAt: expires}, + {MutationID: "m", Result: json.RawMessage(`{}`), ExpiresAt: expires}, + {MutationID: "m", RequestHash: "h", Result: json.RawMessage(`{}`)}, + } + for i, rec := range invalid { + if _, _, err := store.PutSettingMutation(ctx, rec); !errors.Is(err, userstore.ErrInvalidSettingIdentity) { + t.Fatalf("PutSettingMutation(invalid %d) error = %v, want ErrInvalidSettingIdentity", i, err) + } + } + if _, _, err := store.PutSettingMutation(ctx, userstore.SettingMutationRecord{ + MutationID: "m", RequestHash: "h", ExpiresAt: expires, + }); !errors.Is(err, userstore.ErrInvalidSettingValue) { + t.Fatalf("PutSettingMutation(no result) error = %v, want ErrInvalidSettingValue", err) + } +} + +// assertIdentitySet compares the returned rows to the expected identities as a +// set. Row order is deliberately not asserted: the two backends sort text under +// different collations, and ranking is the resolver's job anyway. +func assertIdentitySet(t *testing.T, rows []userstore.SettingValue, want []userstore.SettingIdentity) { + t.Helper() + got := make([]string, 0, len(rows)) + for _, row := range rows { + got = append(got, identityToken(row.SettingIdentity)) + } + expected := make([]string, 0, len(want)) + for _, id := range want { + expected = append(expected, identityToken(id)) + } + sort.Strings(got) + sort.Strings(expected) + if !reflect.DeepEqual(got, expected) { + t.Fatalf("candidate identities =\n %v\nwant\n %v", got, expected) + } +} + +func identityToken(id userstore.SettingIdentity) string { + return fmt.Sprintf("%s|%s|%s|%s|%d|%s", + id.Key, id.Scope, id.ProfileID, id.DeviceID, id.LibraryID, id.SeriesID) +} + +// jsonEqual compares two JSON documents by value. PostgreSQL stores jsonb, which +// re-serializes objects in its own key order, so a byte comparison would report +// a difference between the backends that no client can observe. +func jsonEqual(a, b json.RawMessage) bool { + var left, right any + if err := json.Unmarshal(a, &left); err != nil { + return false + } + if err := json.Unmarshal(b, &right); err != nil { + return false + } + return reflect.DeepEqual(left, right) +} diff --git a/internal/userstore/storetest/suite.go b/internal/userstore/storetest/suite.go index 7f9b37c4..82afa3fc 100644 --- a/internal/userstore/storetest/suite.go +++ b/internal/userstore/storetest/suite.go @@ -77,6 +77,9 @@ func RunSuite(t *testing.T, newStore func(t *testing.T) userstore.UserStore) { t.Run("HomeDismissals", func(t *testing.T) { testHomeDismissals(t, newStore) }) + t.Run("SettingValues", func(t *testing.T) { + RunSettingValues(t, newStore) + }) } func testProfiles(t *testing.T, newStore func(t *testing.T) userstore.UserStore) { diff --git a/migrations/sql/20260727010621_user_setting_values.sql b/migrations/sql/20260727010621_user_setting_values.sql new file mode 100644 index 00000000..6cf92578 --- /dev/null +++ b/migrations/sql/20260727010621_user_setting_values.sql @@ -0,0 +1,112 @@ +-- Canonical storage for the cross-platform user settings contract. +-- +-- user_setting_values replaces the string-valued preference surfaces with one +-- typed table: the manifest in contracts/settings/v1 remains the schema, and +-- this table stores validated JSON plus the scope identity the value hangs off. +-- See docs/superpowers/specs/2026-07-10-cross-platform-user-settings-contract-design.md +-- ("Canonical storage"). +-- +-- Delete behavior is application-enforced. The two cascades below are the only +-- ones this schema can inherit — user ownership, and composite profile +-- ownership, which user_device_settings already carries. Library, series and +-- device identity columns reference nothing (libraries and series live in the +-- shared catalog, devices in user_devices, and the per-user SQLite store +-- declares no foreign keys at all), so the owning delete paths remove those +-- rows and the userstore conformance suite holds both backends to it. + +-- +goose Up +-- +goose StatementBegin +CREATE TABLE public.user_setting_values ( + id bigserial PRIMARY KEY, + user_id integer NOT NULL, + key text NOT NULL, + scope text NOT NULL, + profile_id text, + device_id text, + library_id integer, + series_id text, + value jsonb NOT NULL, + revision bigint NOT NULL DEFAULT 1, + created_at timestamptz NOT NULL DEFAULT now(), + updated_at timestamptz NOT NULL DEFAULT now(), + CONSTRAINT user_setting_values_user_id_fkey + FOREIGN KEY (user_id) REFERENCES public.users(id) ON DELETE CASCADE, + -- MATCH SIMPLE: account-scope rows carry a NULL profile_id and are exempt. + CONSTRAINT user_setting_values_profile_fkey + FOREIGN KEY (user_id, profile_id) REFERENCES public.user_profiles(user_id, id) ON DELETE CASCADE, + CONSTRAINT user_setting_values_scope_check + CHECK (scope IN ('account', 'profile', 'profile_device', 'profile_library', 'profile_series')), + CONSTRAINT user_setting_values_identity_check CHECK ( + (scope = 'account' AND profile_id IS NULL AND device_id IS NULL AND library_id IS NULL AND series_id IS NULL) OR + (scope = 'profile' AND profile_id IS NOT NULL AND device_id IS NULL AND library_id IS NULL AND series_id IS NULL) OR + (scope = 'profile_device' AND profile_id IS NOT NULL AND device_id IS NOT NULL AND library_id IS NULL AND series_id IS NULL) OR + (scope = 'profile_library' AND profile_id IS NOT NULL AND device_id IS NULL AND library_id IS NOT NULL AND series_id IS NULL) OR + (scope = 'profile_series' AND profile_id IS NOT NULL AND device_id IS NULL AND library_id IS NULL AND series_id IS NOT NULL) + ) +); + +-- One explicit value per identity. These exist for correctness, not for reads. +CREATE UNIQUE INDEX user_setting_values_account_uq + ON public.user_setting_values (user_id, key) WHERE scope = 'account'; +CREATE UNIQUE INDEX user_setting_values_profile_uq + ON public.user_setting_values (user_id, profile_id, key) WHERE scope = 'profile'; +CREATE UNIQUE INDEX user_setting_values_profile_device_uq + ON public.user_setting_values (user_id, profile_id, device_id, key) WHERE scope = 'profile_device'; +CREATE UNIQUE INDEX user_setting_values_profile_library_uq + ON public.user_setting_values (user_id, profile_id, library_id, key) WHERE scope = 'profile_library'; +CREATE UNIQUE INDEX user_setting_values_profile_series_uq + ON public.user_setting_values (user_id, profile_id, series_id, key) WHERE scope = 'profile_series'; + +-- The hot path: one query per resolution request collects every candidate row +-- for a key set at one identity, and the resolver ranks them in Go. +CREATE INDEX user_setting_values_resolution_idx + ON public.user_setting_values (user_id, profile_id, key, scope); +CREATE INDEX user_setting_values_series_idx + ON public.user_setting_values (user_id, profile_id, series_id); +CREATE INDEX user_setting_values_library_idx + ON public.user_setting_values (user_id, profile_id, library_id); + +-- Mutation idempotency. Rows expire after 30 days; expires_at is not +-- self-enforcing, so a sweeper deletes them on the decisionlog_cleanup pattern. +CREATE TABLE public.user_setting_mutations ( + user_id integer NOT NULL, + mutation_id text NOT NULL, + request_hash text NOT NULL, + result jsonb NOT NULL, + created_at timestamptz NOT NULL DEFAULT now(), + expires_at timestamptz NOT NULL, + CONSTRAINT user_setting_mutations_pkey PRIMARY KEY (user_id, mutation_id), + CONSTRAINT user_setting_mutations_user_id_fkey + FOREIGN KEY (user_id) REFERENCES public.users(id) ON DELETE CASCADE +); + +CREATE INDEX user_setting_mutations_expiry_idx + ON public.user_setting_mutations (expires_at); + +-- Inert audit table for the one-time migration. It has no runtime read/write +-- API and is not an extension bag: it retains unrecognized or invalid historical +-- rows for operator inspection instead of silently deleting them. Bounded by the +-- migration rather than by traffic, so no sweeper applies. +CREATE TABLE public.user_setting_migration_rejects ( + id bigserial PRIMARY KEY, + user_id integer NOT NULL, + source_table text NOT NULL, + source_key text NOT NULL, + identity jsonb NOT NULL, + value text, + reason text NOT NULL, + recorded_at timestamptz NOT NULL DEFAULT now(), + CONSTRAINT user_setting_migration_rejects_user_id_fkey + FOREIGN KEY (user_id) REFERENCES public.users(id) ON DELETE CASCADE +); + +CREATE INDEX user_setting_migration_rejects_user_idx + ON public.user_setting_migration_rejects (user_id, source_table); +-- +goose StatementEnd + +-- +goose Down +-- +goose StatementBegin +DROP TABLE IF EXISTS public.user_setting_migration_rejects; +DROP TABLE IF EXISTS public.user_setting_mutations; +DROP TABLE IF EXISTS public.user_setting_values; +-- +goose StatementEnd diff --git a/migrations/user_setting_values_test.go b/migrations/user_setting_values_test.go new file mode 100644 index 00000000..ea1c4cb8 --- /dev/null +++ b/migrations/user_setting_values_test.go @@ -0,0 +1,78 @@ +package migrations + +import ( + "strings" + "testing" +) + +// TestUserSettingValuesMigrationContract pins the parts of the canonical +// settings storage that the store code and the design both depend on: the scope +// CHECK constraints, the five partial unique indexes that enforce one explicit +// value per identity, and the covering indexes the one-query read path needs. +// A silent edit to any of them would not fail a store test until a duplicate row +// or a sequential-scan regression reached production. +func TestUserSettingValuesMigrationContract(t *testing.T) { + migration := readMigration(t, "sql/20260727010621_user_setting_values.sql") + + for _, want := range []string{ + "CREATE TABLE public.user_setting_values", + "value jsonb NOT NULL", + "revision bigint NOT NULL DEFAULT 1", + "CONSTRAINT user_setting_values_scope_check\n CHECK (scope IN ('account', 'profile', 'profile_device', 'profile_library', 'profile_series'))", + "(scope = 'account' AND profile_id IS NULL AND device_id IS NULL AND library_id IS NULL AND series_id IS NULL)", + "(scope = 'profile' AND profile_id IS NOT NULL AND device_id IS NULL AND library_id IS NULL AND series_id IS NULL)", + "(scope = 'profile_device' AND profile_id IS NOT NULL AND device_id IS NOT NULL AND library_id IS NULL AND series_id IS NULL)", + "(scope = 'profile_library' AND profile_id IS NOT NULL AND device_id IS NULL AND library_id IS NOT NULL AND series_id IS NULL)", + "(scope = 'profile_series' AND profile_id IS NOT NULL AND device_id IS NULL AND library_id IS NULL AND series_id IS NOT NULL)", + + // The cascades that exist today, and only those. + "CONSTRAINT user_setting_values_user_id_fkey\n FOREIGN KEY (user_id) REFERENCES public.users(id) ON DELETE CASCADE", + "CONSTRAINT user_setting_values_profile_fkey\n FOREIGN KEY (user_id, profile_id) REFERENCES public.user_profiles(user_id, id) ON DELETE CASCADE", + + // One explicit value per identity. + "CREATE UNIQUE INDEX user_setting_values_account_uq\n ON public.user_setting_values (user_id, key) WHERE scope = 'account'", + "CREATE UNIQUE INDEX user_setting_values_profile_uq\n ON public.user_setting_values (user_id, profile_id, key) WHERE scope = 'profile'", + "CREATE UNIQUE INDEX user_setting_values_profile_device_uq\n ON public.user_setting_values (user_id, profile_id, device_id, key) WHERE scope = 'profile_device'", + "CREATE UNIQUE INDEX user_setting_values_profile_library_uq\n ON public.user_setting_values (user_id, profile_id, library_id, key) WHERE scope = 'profile_library'", + "CREATE UNIQUE INDEX user_setting_values_profile_series_uq\n ON public.user_setting_values (user_id, profile_id, series_id, key) WHERE scope = 'profile_series'", + + // The hot read path. + "ON public.user_setting_values (user_id, profile_id, key, scope)", + "ON public.user_setting_values (user_id, profile_id, series_id)", + "ON public.user_setting_values (user_id, profile_id, library_id)", + + // Idempotency and the inert migration audit table. + "CREATE TABLE public.user_setting_mutations", + "CONSTRAINT user_setting_mutations_pkey PRIMARY KEY (user_id, mutation_id)", + "request_hash text NOT NULL", + "expires_at timestamptz NOT NULL", + "ON public.user_setting_mutations (expires_at)", + "CREATE TABLE public.user_setting_migration_rejects", + } { + if !strings.Contains(migration, want) { + t.Fatalf("migration missing %q", want) + } + } + + // Library, series and device identity columns must stay reference-free: the + // per-user SQLite store has no foreign keys at all, so inheriting cleanup + // from constraints here would let the two backends drift. + for _, forbidden := range []string{ + "REFERENCES public.library_folders", + "REFERENCES public.media_items", + "REFERENCES public.user_devices", + } { + if strings.Contains(migration, forbidden) { + t.Fatalf("migration must not add %q; delete behavior is application-enforced", forbidden) + } + } +} + +func readMigration(t *testing.T, path string) string { + t.Helper() + contents, err := FS.ReadFile(path) + if err != nil { + t.Fatalf("read migration %s: %v", path, err) + } + return string(contents) +}