fix(maintenance): bound search_clicks and lyrics_index, clear stale queue source
Three unbounded or stale surfaces, each small on its own: - lyrics_index rows were never pruned on track removal, so the FTS index grew forever. Delete the entry where the library search FTS entry is already deleted, on the orphan and RemoveFromLibrary paths. - search_clicks had no ceiling; age out ranking rows after a retention window via a daily janitor job. - queue.source_* kept a "Playing from X" label after its playlist was deleted. Drop the source when the queue's own playlist goes, wired through a playlist-service hook like Library.SetRemovalHooks. Closes #249
This commit is contained in:
@@ -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,
|
||||
))
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user