From 3d039e163c7c2efcf6b35736a000591cd16ad4ec Mon Sep 17 00:00:00 2001 From: Quick <31828688+Quick104@users.noreply.github.com> Date: Tue, 28 Jul 2026 00:42:12 +0000 Subject: [PATCH] feat(settings): resolve catalog playback preferences through the contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit catalog/detail.go held the two hardest ladders in the codebase: subtitles resolved across four levels by hand, audio across three, each partially overriding the last through Has* flags. Both now call the canonical resolver, so the precedence lives in the manifest and this file cannot disagree with the contract about which override wins. Adding a scope is a manifest change rather than another branch here. The subtitle track signature stays on its specialized table — it identifies a concrete track rather than expressing a preference, so it is not a setting. Resolution keeps the memoization the old lookups had: the audio resolver still reads once per profile and once per library rather than once per file, which is what kept a many-track audiobook detail page fast. The test that guards it now counts resolver reads instead of GetProfile calls, since the guarantee is about scaling with file count rather than about which method does the reading. Four tests seeded the profile column directly. That column is a migration source now, not a read path, so they seed the canonical value instead — they were passing against storage nothing reads. Co-Authored-By: Claude Opus 5 (1M context) --- cmd/settingsgen/main.go | 7 + internal/catalog/detail.go | 141 +++++++++++++----- .../catalog/detail_audio_prefs_query_test.go | 20 ++- internal/catalog/detail_version_prefs_test.go | 48 +++--- 4 files changed, 154 insertions(+), 62 deletions(-) diff --git a/cmd/settingsgen/main.go b/cmd/settingsgen/main.go index 83b685b7..efcc2c9d 100644 --- a/cmd/settingsgen/main.go +++ b/cmd/settingsgen/main.go @@ -278,8 +278,15 @@ func generateKotlin(contract *settingscontract.Manifest, pkg string) ([]byte, er {"INT_KEYS", []settingscontract.ValueType{settingscontract.TypeInteger}}, {"DOUBLE_KEYS", []settingscontract.ValueType{settingscontract.TypeNumber}}, } { + // Remote only: these tables drive how a value read back from the + // server is parsed, and a client_local key never comes back from the + // server at all. Listing one would describe a wire format that has no + // wire. fmt.Fprintf(&out, "\n val %s: Set = setOf(\n", group.name) for _, def := range sortedDefinitions(contract) { + if !def.IsRemote() { + continue + } for _, want := range group.types { if def.ValueSchema.Type == want { fmt.Fprintf(&out, " %s,\n", screamingCase(def.Key)) diff --git a/internal/catalog/detail.go b/internal/catalog/detail.go index ab97aef9..1417117f 100644 --- a/internal/catalog/detail.go +++ b/internal/catalog/detail.go @@ -18,6 +18,9 @@ import ( "github.com/Silo-Server/silo-server/internal/models" "github.com/Silo-Server/silo-server/internal/overlays" "github.com/Silo-Server/silo-server/internal/playback" + "github.com/Silo-Server/silo-server/internal/settingscontract" + "github.com/Silo-Server/silo-server/internal/settingskeys" + "github.com/Silo-Server/silo-server/internal/settingsresolve" "github.com/Silo-Server/silo-server/internal/userstore" ) @@ -614,6 +617,10 @@ type DetailService struct { originalLangFn func(context.Context, string) string probeEnsurer PlaybackProbeEnsurer chapterThumbs ChapterThumbnailQueuer + + // resolver is built once on first use; see settingsResolver. + resolverOnce sync.Once + resolver *settingsresolve.Resolver } // NewDetailService creates a new DetailService. @@ -2612,6 +2619,18 @@ func (s *DetailService) newWatchDetail( } } +// effectiveSubtitleDefaults resolves the subtitle preferences that apply to one +// item, through the canonical resolver. +// +// This used to be four levels of hand-written precedence — profile columns, +// then a library preference row, then a series preference row, each partially +// overriding the last through Has* flags. The order lives in the manifest now +// (profile_series, profile_library, profile_device, profile, default), so this +// function and the contract cannot disagree about which override wins, and a +// new scope is a manifest change rather than another branch here. +// +// The track signature stays on its specialized table: it identifies a concrete +// track rather than expressing a preference, so it is not a setting. func (s *DetailService) effectiveSubtitleDefaults( ctx context.Context, filter AccessFilter, @@ -2629,44 +2648,50 @@ func (s *DetailService) effectiveSubtitleDefaults( return defaults } - if profile, err := store.GetProfile(ctx, filter.ProfileID); err == nil && profile != nil { - defaults.Language = profile.SubtitleLanguage - defaults.Mode = profile.SubtitleMode - defaults.ShowForced = profile.ShowForcedSubtitles - defaults.HasLanguage = true - defaults.HasMode = true - defaults.HasShowForced = true + rc := settingsresolve.Context{ProfileID: filter.ProfileID} + if libraryID := preferredPlayableLibraryID(files, filter.SelectedFileID); libraryID > 0 { + rc.LibraryIDs = []int{libraryID} + } + if seriesID != "" { + rc.SeriesIDs = []string{seriesID} } - if libraryID := preferredPlayableLibraryID(files, filter.SelectedFileID); libraryID > 0 { - if pref, err := store.GetLibraryPlaybackPreference(ctx, filter.ProfileID, libraryID); err == nil && pref != nil { - if pref.HasSubtitleLanguage { - defaults.Language = pref.SubtitleLanguage - defaults.HasLanguage = true - } - if pref.HasSubtitleMode { - defaults.Mode = pref.SubtitleMode - defaults.HasMode = true - } - if pref.HasShowForcedSubtitles { - defaults.ShowForced = pref.ShowForcedSubtitles - defaults.HasShowForced = true + resolved, err := s.settingsResolver().Resolve(ctx, store, rc, []string{ + settingskeys.PlaybackSubtitleLanguage, + settingskeys.PlaybackSubtitleMode, + settingskeys.PlaybackShowForcedSubtitles, + }, nil) + if err == nil { + for _, eff := range resolved { + // A value that resolved to the contract default is not a stored + // preference, and the callers distinguish the two through the Has* + // flags: an unset language must not read as "the user chose empty". + stored := eff.Source != settingscontract.ScopeDefault + switch eff.Key { + case settingskeys.PlaybackSubtitleLanguage: + var language string + if json.Unmarshal(eff.Value, &language) == nil && stored { + defaults.Language = language + defaults.HasLanguage = true + } + case settingskeys.PlaybackSubtitleMode: + var mode string + if json.Unmarshal(eff.Value, &mode) == nil && stored { + defaults.Mode = mode + defaults.HasMode = true + } + case settingskeys.PlaybackShowForcedSubtitles: + var forced bool + if json.Unmarshal(eff.Value, &forced) == nil && stored { + defaults.ShowForced = forced + defaults.HasShowForced = true + } } } } if seriesID != "" { if pref, err := store.GetSubtitlePreference(ctx, filter.ProfileID, seriesID); err == nil && pref != nil { - defaults.Language = pref.SubtitleLanguage - defaults.HasLanguage = true - if pref.SubtitleMode != "" { - defaults.Mode = pref.SubtitleMode - defaults.HasMode = true - } - if pref.HasShowForcedSubtitles { - defaults.ShowForced = pref.ShowForcedSubtitles - defaults.HasShowForced = true - } if pref.TrackSignature != nil && !pref.TrackSignature.IsZero() { defaults.TrackSignature = pref.TrackSignature } @@ -2676,6 +2701,23 @@ func (s *DetailService) effectiveSubtitleDefaults( return defaults } +// settingsResolver lazily builds the resolver over the embedded contract. +// +// The contract is validated at startup, so a load failure here is unreachable; +// returning a resolver with no contract makes Resolve error rather than panic, +// which degrades this to "no stored preferences" instead of failing the detail +// request. +func (s *DetailService) settingsResolver() *settingsresolve.Resolver { + s.resolverOnce.Do(func() { + contract, err := settingscontract.Load() + if err != nil { + return + } + s.resolver = settingsresolve.New(contract) + }) + return s.resolver +} + func (s *DetailService) effectiveVersionDefaults( ctx context.Context, filter AccessFilter, @@ -2810,28 +2852,53 @@ func (r *audioPrefResolver) audioPreference(ctx context.Context) *playback.Audio return &cp } +// profileLanguage is the audio language with no content context — the profile's +// own preference, resolved through the contract rather than read off the +// profile column it migrated from. func (r *audioPrefResolver) profileLanguage(ctx context.Context) string { if !r.profileDone { r.profileDone = true - if profile, profileErr := r.store.GetProfile(ctx, r.profileID); profileErr == nil && profile != nil { - r.profileLang = strings.TrimSpace(profile.Language) - } + r.profileLang = r.svc.resolvedAudioLanguage(ctx, r.store, settingsresolve.Context{ + ProfileID: r.profileID, + }) } return r.profileLang } +// libraryAudioLanguage resolves with a library in context, so a library +// override wins over the profile value and the resolver applies the same +// precedence the manifest declares. func (r *audioPrefResolver) libraryAudioLanguage(ctx context.Context, libraryID int) string { if lang, ok := r.libLang[libraryID]; ok { return lang } - lang := "" - if pref, prefErr := r.store.GetLibraryPlaybackPreference(ctx, r.profileID, libraryID); prefErr == nil && pref != nil { - lang = strings.TrimSpace(pref.AudioLanguage) - } + lang := r.svc.resolvedAudioLanguage(ctx, r.store, settingsresolve.Context{ + ProfileID: r.profileID, + LibraryIDs: []int{libraryID}, + }) r.libLang[libraryID] = lang return lang } +// resolvedAudioLanguage returns the effective playback.audio_language for one +// context, or "" when nothing is stored. +func (s *DetailService) resolvedAudioLanguage( + ctx context.Context, store userstore.UserStore, rc settingsresolve.Context, +) string { + resolved, err := s.settingsResolver().Resolve(ctx, store, rc, + []string{settingskeys.PlaybackAudioLanguage}, nil) + if err != nil || len(resolved) == 0 { + return "" + } + // The contract default is null, which means "no preference" — the caller + // treats "" the same way, so an unset language needs no special case. + var language string + if json.Unmarshal(resolved[0].Value, &language) != nil { + return "" + } + return strings.TrimSpace(language) +} + func (r *audioPrefResolver) originalLanguage(ctx context.Context) string { if !r.originalDone { r.originalDone = true diff --git a/internal/catalog/detail_audio_prefs_query_test.go b/internal/catalog/detail_audio_prefs_query_test.go index 168a858a..be0fd5a9 100644 --- a/internal/catalog/detail_audio_prefs_query_test.go +++ b/internal/catalog/detail_audio_prefs_query_test.go @@ -17,6 +17,7 @@ type countingUserStore struct { getProfile int32 getAudioPref int32 getLibraryPref int32 + resolveValues int32 } func (c *countingUserStore) GetProfile(ctx context.Context, id string) (*userstore.Profile, error) { @@ -24,6 +25,13 @@ func (c *countingUserStore) GetProfile(ctx context.Context, id string) (*usersto return c.UserStore.GetProfile(ctx, id) } +func (c *countingUserStore) ListSettingValuesForResolution( + ctx context.Context, query userstore.SettingResolutionQuery, +) ([]userstore.SettingValue, error) { + atomic.AddInt32(&c.resolveValues, 1) + return c.UserStore.ListSettingValuesForResolution(ctx, query) +} + func (c *countingUserStore) GetAudioPreference(ctx context.Context, profileID, seriesID string) (*userstore.AudioPreference, error) { atomic.AddInt32(&c.getAudioPref, 1) return c.UserStore.GetAudioPreference(ctx, profileID, seriesID) @@ -56,13 +64,15 @@ func TestBuildPlaybackInfo_AudioPrefLookupsDoNotScaleWithFileCount(t *testing.T) filter := AccessFilter{UserID: 1, ProfileID: "profile-1"} service.buildPlaybackInfo(context.Background(), files, filter, "book-1") - if got := atomic.LoadInt32(&counting.getProfile); got != 1 { - t.Fatalf("GetProfile called %d times for %d files; want 1 (lookup must not scale with file count)", got, fileCount) + // The audio language now resolves through the settings contract, which + // memoizes per profile and per library exactly as the profile-column + // lookups it replaced did. Two reads: one with no content context for the + // profile-level answer, one naming the folder's library. + if got := atomic.LoadInt32(&counting.resolveValues); got > 2 { + t.Fatalf("resolved settings %d times for %d files; want at most 2 "+ + "(lookup must not scale with file count)", got, fileCount) } if got := atomic.LoadInt32(&counting.getAudioPref); got != 1 { t.Fatalf("GetAudioPreference called %d times for %d files; want 1", got, fileCount) } - if got := atomic.LoadInt32(&counting.getLibraryPref); got != 1 { - t.Fatalf("GetLibraryPlaybackPreference called %d times for one folder; want 1", got) - } } diff --git a/internal/catalog/detail_version_prefs_test.go b/internal/catalog/detail_version_prefs_test.go index bda87185..b362c2ee 100644 --- a/internal/catalog/detail_version_prefs_test.go +++ b/internal/catalog/detail_version_prefs_test.go @@ -3,11 +3,14 @@ package catalog import ( "context" "database/sql" + "encoding/json" "reflect" "testing" "github.com/Silo-Server/silo-server/internal/models" "github.com/Silo-Server/silo-server/internal/playback" + "github.com/Silo-Server/silo-server/internal/settingscontract" + "github.com/Silo-Server/silo-server/internal/settingskeys" "github.com/Silo-Server/silo-server/internal/userdb" "github.com/Silo-Server/silo-server/internal/userstore" ) @@ -163,11 +166,7 @@ func TestSeriesFolderPathsFromFiles_PrefersObservedRootsAndDedupes(t *testing.T) func TestEffectiveAudioTrackIndex_PrefersSeriesAudioPreferenceOverLibraryAndProfile(t *testing.T) { store := newDetailTestStore(t) language := "en" - if err := store.UpdateProfile(context.Background(), "profile-1", userstore.UpdateProfileInput{ - Language: &language, - }); err != nil { - t.Fatalf("UpdateProfile: %v", err) - } + setProfileAudioLanguage(t, store, language) if err := store.UpsertLibraryPlaybackPreference(context.Background(), userstore.LibraryPlaybackPreference{ ProfileID: "profile-1", LibraryID: 12, @@ -206,11 +205,7 @@ func TestEffectiveAudioTrackIndex_PrefersSeriesAudioPreferenceOverLibraryAndProf func TestBuildPlaybackInfo_SetsEffectiveAudioLanguageFromOriginalWhenTrackLanguageMissing(t *testing.T) { store := newDetailTestStore(t) language := playback.OriginalLanguageSentinel - if err := store.UpdateProfile(context.Background(), "profile-1", userstore.UpdateProfileInput{ - Language: &language, - }); err != nil { - t.Fatalf("UpdateProfile: %v", err) - } + setProfileAudioLanguage(t, store, language) service := &DetailService{ originalLangFn: func(context.Context, string) string { @@ -426,11 +421,7 @@ func TestSortFileVersions_PrefersLargerFilesWithinQualityTier(t *testing.T) { func TestEffectiveAudioTrackIndex_ResolvesProfileOriginalWhenSeriesPreferenceFallsBack(t *testing.T) { store := newDetailTestStore(t) language := playback.OriginalLanguageSentinel - if err := store.UpdateProfile(context.Background(), "profile-1", userstore.UpdateProfileInput{ - Language: &language, - }); err != nil { - t.Fatalf("UpdateProfile: %v", err) - } + setProfileAudioLanguage(t, store, language) if err := store.SetAudioPreference(context.Background(), userstore.AudioPreference{ ProfileID: "profile-1", SeriesID: "series-1", @@ -466,11 +457,7 @@ func TestEffectiveAudioTrackIndex_ResolvesProfileOriginalWhenSeriesPreferenceFal func TestEffectiveAudioTrackIndex_KeepsProfileFallbackWhenLibraryOriginalIsUnresolved(t *testing.T) { store := newDetailTestStore(t) language := "en" - if err := store.UpdateProfile(context.Background(), "profile-1", userstore.UpdateProfileInput{ - Language: &language, - }); err != nil { - t.Fatalf("UpdateProfile: %v", err) - } + setProfileAudioLanguage(t, store, language) if err := store.UpsertLibraryPlaybackPreference(context.Background(), userstore.LibraryPlaybackPreference{ ProfileID: "profile-1", LibraryID: 12, @@ -501,3 +488,24 @@ func TestEffectiveAudioTrackIndex_KeepsProfileFallbackWhenLibraryOriginalIsUnres t.Fatalf("effectiveAudioTrackIndex() = %d, want 1", index) } } + +// setProfileAudioLanguage stores the profile's preferred audio language as a +// canonical setting value. +// +// The profile column these tests used to write is a migration source, not a +// read path: playback resolves playback.audio_language through the settings +// contract, so seeding the column would leave the resolver seeing nothing. +func setProfileAudioLanguage(t *testing.T, store userstore.UserStore, language string) { + t.Helper() + encoded, err := json.Marshal(language) + if err != nil { + t.Fatalf("encoding language: %v", err) + } + if _, err := store.UpsertSettingValue(context.Background(), userstore.SettingIdentity{ + Key: settingskeys.PlaybackAudioLanguage, + Scope: settingscontract.ScopeProfile, + ProfileID: "profile-1", + }, encoded); err != nil { + t.Fatalf("seeding %s: %v", settingskeys.PlaybackAudioLanguage, err) + } +}