fix(maintenance): bound search_clicks and lyrics_index, clear stale queue source #253
@@ -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