diff --git a/backend/app.go b/backend/app.go index 376f0ae..830a245 100644 --- a/backend/app.go +++ b/backend/app.go @@ -499,6 +499,10 @@ func (yj *YellowJacketApp) OnStartup(ctx context.Context) { PostRemove: yj.explore.InvalidateLibrarySync, }) + // A deleted playlist must not leave the queue's "Playing from" + // label pointing at it. + yj.playlist.SetOnPlaylistDeleted(yj.queue.DropSourceForPlaylist) + // Register playback finished handler to drive queue auto-advance. yj.player.SetPlaybackFinishedHandler(yj.queue.OnPlaybackFinished) @@ -775,6 +779,7 @@ func (yj *YellowJacketApp) startJanitor() { } yj.janitor.Register(maintenance.ExpiredHTTPCacheJob(yj.database)) + yj.janitor.Register(maintenance.StaleSearchClicksJob(yj.database)) yj.janitor.Register(maintenance.OrphanedCoverFilesJob( yj.database, coversDir, library.CoverArtFileSet, )) diff --git a/backend/database/lyrics_search.go b/backend/database/lyrics_search.go index a1f9ead..185f533 100644 --- a/backend/database/lyrics_search.go +++ b/backend/database/lyrics_search.go @@ -151,16 +151,28 @@ func (d *DB) SetLyrics(audioFileID int64, lyrics, source, recordingMBID string) return d.upsertLyricsIndex(audioFileID, lyrics) } -// upsertLyricsIndex refreshes a single file's entry in the contentless -// lyrics_index. contentless_delete=1 makes the DELETE valid; an empty -// lyrics string leaves the row deleted. -func (d *DB) upsertLyricsIndex(audioFileID int64, lyrics string) error { +// DeleteLyricsIndex removes one file's entry from the contentless +// lyrics_index. It is called wherever a file row is deleted — the +// `lyrics` table cascades with its file, but the FTS entry does not and +// would otherwise accumulate for the life of the install (#249). +func (d *DB) DeleteLyricsIndex(audioFileID int64) error { if _, err := d.db.ExecContext(d.Ctx, "DELETE FROM lyrics_index WHERE rowid = ?", audioFileID, ); err != nil { return fmt.Errorf("could not delete lyrics_index row: %w", err) } + return nil +} + +// upsertLyricsIndex refreshes a single file's entry in the contentless +// lyrics_index. contentless_delete=1 makes the DELETE valid; an empty +// lyrics string leaves the row deleted. +func (d *DB) upsertLyricsIndex(audioFileID int64, lyrics string) error { + if err := d.DeleteLyricsIndex(audioFileID); err != nil { + return err + } + if strings.TrimSpace(lyrics) == "" { return nil } diff --git a/backend/library/library.go b/backend/library/library.go index e161c0f..e30d8fa 100644 --- a/backend/library/library.go +++ b/backend/library/library.go @@ -968,7 +968,7 @@ func (l *Library) scanInternal( } } - // Remove from FTS5 search index. + // Remove from FTS5 search index and the lyrics index. if err := l.db.DeleteSearchIndex( audioFile.ID, ); err != nil { @@ -981,6 +981,18 @@ func (l *Library) scanInternal( metrics.addWarning(path, "orphan", err) } + if err := l.db.DeleteLyricsIndex( + audioFile.ID, + ); err != nil { + l.logger.Warn( + "failed to delete lyrics index entry for orphan", + "id", audioFile.ID, + "err", err, + ) + + metrics.addWarning(path, "orphan", err) + } + removed.Add(1) return true diff --git a/backend/library/remove_tracks.go b/backend/library/remove_tracks.go index a6e3098..c554225 100644 --- a/backend/library/remove_tracks.go +++ b/backend/library/remove_tracks.go @@ -127,6 +127,11 @@ func (l *Library) RemoveFromLibrary(filePaths []string) (*RemovalResult, error) l.logger.Warn("could not delete FTS entry for removed track", "path", row.FilePath, "id", row.ID, "err", err) } + + if err := l.db.DeleteLyricsIndex(row.ID); err != nil { + l.logger.Warn("could not delete lyrics index entry for removed track", + "path", row.FilePath, "id", row.ID, "err", err) + } } // Deleting an audio_files row cascades to queue_tracks, so the diff --git a/backend/maintenance/maintenance_test.go b/backend/maintenance/maintenance_test.go index f88d621..09d46f4 100644 --- a/backend/maintenance/maintenance_test.go +++ b/backend/maintenance/maintenance_test.go @@ -666,3 +666,52 @@ func TestExpiredHTTPCacheJob_TrimsToBudget(t *testing.T) { t.Errorf("kept %q, want the longest-lived row", kept) } } + +// TestStaleSearchClicksJob deletes only the clicks old enough to have +// left the retention window (#249). +func TestStaleSearchClicksJob(t *testing.T) { + t.Parallel() + + db := database.NewTestDB(t) + + count := func(mbid string) int { + t.Helper() + + var n int + if err := db.QueryRowWriter( + "SELECT COUNT(*) FROM search_clicks WHERE entity_mbid = ?", mbid, + ).Scan(&n); err != nil { + t.Fatalf("count %s: %v", mbid, err) + } + + return n + } + + seed := func(query, mbid, lastClicked string) { + t.Helper() + + if _, err := db.ExecContext( + `INSERT INTO search_clicks + (query, entity_mbid, entity_type, click_count, last_clicked) + VALUES (?, ?, 'recording', 1, ?)`, + query, mbid, lastClicked, + ); err != nil { + t.Fatalf("seed search_clicks: %v", err) + } + } + + seed("tide", "aaaa", "2024-01-01 00:00:00") // stale + seed("tide", "bbbb", "2999-01-01 00:00:00") // recent + + if _, err := StaleSearchClicksJob(db).Run(context.Background()); err != nil { + t.Fatalf("run job: %v", err) + } + + if n := count("bbbb"); n != 1 { + t.Errorf("recent click was deleted: %d rows, want 1", n) + } + + if n := count("aaaa"); n != 0 { + t.Errorf("stale click survived: %d rows, want 0", n) + } +} diff --git a/backend/maintenance/sweeps.go b/backend/maintenance/sweeps.go index 049eae4..9a69b56 100644 --- a/backend/maintenance/sweeps.go +++ b/backend/maintenance/sweeps.go @@ -628,3 +628,35 @@ func dirSize(dir string) (bytes, files int64) { return bytes, files } + +// searchClicksRetention is how long a search-click ranking signal stays +// useful. search_clicks is authored behavioural data — nothing that +// owns a row ever drops it — so age is the ceiling that keeps the table +// from growing without bound for the life of the install (#249). +const searchClicksRetention = "-180 days" + +// StaleSearchClicksJob deletes search-click ranking rows older than the +// retention window. Rows are small and the table grows slowly, so this +// runs daily and does almost nothing most runs. +func StaleSearchClicksJob(db *database.DB) Job { + return Job{ + Name: "search-clicks-sweep", + MinInterval: dailyInterval, + Run: func(_ context.Context) (Result, error) { + res, err := db.ExecContext( + `DELETE FROM search_clicks + WHERE last_clicked < datetime('now', ?)`, + searchClicksRetention, + ) + if err != nil { + return Result{}, fmt.Errorf( + "delete stale search_clicks rows: %w", err, + ) + } + + rows, _ := res.RowsAffected() + + return Result{RowsDeleted: rows}, nil + }, + } +} diff --git a/backend/playlist/playlist.go b/backend/playlist/playlist.go index fef27a6..126b531 100644 --- a/backend/playlist/playlist.go +++ b/backend/playlist/playlist.go @@ -134,6 +134,12 @@ type Service struct { libraryDir LibraryDirProvider favoritesConf FavoritesConfigProvider + // onDeleted, when set, is called after a playlist is deleted so + // cross-cutting state that points at it (the queue's "Playing + // from" label) can stop pointing at a playlist that no longer + // exists. Wired from app.go, like Library.SetRemovalHooks. + onDeleted func(playlistID int64) + // dataDirOverride, when non-empty, replaces the OS user data // directory as the base for the playlists folder. Set by tests to // keep M3U writes out of the real user data directory. @@ -166,6 +172,17 @@ func (s *Service) SetFavoritesConfig( s.favoritesConf = provider } +// SetOnPlaylistDeleted registers a callback invoked after a playlist is +// deleted, for cross-cutting invalidation. +// +//wails:ignore // internal wiring, not part of the app's IPC surface. +func (s *Service) SetOnPlaylistDeleted(onDeleted func(playlistID int64)) { + s.mu.Lock() + defer s.mu.Unlock() + + s.onDeleted = onDeleted +} + // ServiceStartup is v3's service lifecycle hook: it runs once the // runtime exists, and ctx is cancelled when the app shuts down. It // replaces v2's SetContext, which had to be called by hand from @@ -766,6 +783,17 @@ func (s *Service) DeletePlaylist(playlistID int64) error { s.emitEvent(events.PlaylistDeleted, playlistID) + // Cross-cutting invalidation: the queue's "Playing from" label may + // point at this playlist, and a link to a playlist that no longer + // exists is worse than none. + s.mu.Lock() + onDeleted := s.onDeleted + s.mu.Unlock() + + if onDeleted != nil { + onDeleted(playlistID) + } + // Recreate the default playlist if we just deleted it. if s.defaultPlaylistID() == playlistID { s.EnsureDefaultPlaylist() diff --git a/backend/queue/queue.go b/backend/queue/queue.go index 8a29c28..03b96eb 100644 --- a/backend/queue/queue.go +++ b/backend/queue/queue.go @@ -1581,6 +1581,21 @@ func (q *Queue) dropSource() { q.source = Source{} } +// DropSourceForPlaylist clears the queue's "Playing from" label when +// its source playlist is deleted. A link back to a playlist that no +// longer exists is worse than none, and the label otherwise survives +// the deletion until the next SetQueue (#249). +func (q *Queue) DropSourceForPlaylist(playlistID int64) { + q.mu.Lock() + defer q.mu.Unlock() + + if (q.source.Type == "playlist" || q.source.Type == "smartPlaylist") && + q.source.ID == playlistID { + q.dropSource() + q.persistState() + } +} + // commitMutation persists the current queue state after a mutation. // When reindex is true, track positions are renumbered first. // The caller must hold q.mu. diff --git a/backend/queue/queue_test.go b/backend/queue/queue_test.go index c062b6f..faaeae4 100644 --- a/backend/queue/queue_test.go +++ b/backend/queue/queue_test.go @@ -535,3 +535,35 @@ func TestCycleRepeat_CyclesThroughModes(t *testing.T) { t.Errorf("after third cycle: got %q, want %q", state.RepeatMode, RepeatOff) } } + +// TestDropSourceForPlaylist clears the "Playing from" label when the +// queue's source playlist is deleted, and leaves it alone otherwise +// (#249). +func TestDropSourceForPlaylist(t *testing.T) { + t.Parallel() + + q, db := setupTestQueue(t) + paths := seedAudioFiles(t, db, 2) + + q.SetQueue(paths, 0, false, Source{Type: "playlist", ID: 42, Label: "Road Trip"}) + q.DropSourceForPlaylist(42) + + if got := q.GetState().Source; got != (Source{}) { + t.Errorf("source = %+v, want empty after playlist 42 deleted", got) + } +} + +func TestDropSourceForPlaylistIgnoresOtherPlaylists(t *testing.T) { + t.Parallel() + + q, db := setupTestQueue(t) + paths := seedAudioFiles(t, db, 2) + + source := Source{Type: "smartPlaylist", ID: 42, Label: "Road Trip"} + q.SetQueue(paths, 0, false, source) + q.DropSourceForPlaylist(7) + + if got := q.GetState().Source; got != source { + t.Errorf("source = %+v, want %+v unchanged for a different playlist", got, source) + } +}