diff --git a/internal/api/playlists.go b/internal/api/playlists.go index 6efba6ee..af262bd7 100644 --- a/internal/api/playlists.go +++ b/internal/api/playlists.go @@ -60,6 +60,11 @@ type playlistTrackView struct { DurationSec int32 `json:"duration_sec"` StreamURL *string `json:"stream_url"` AddedAt string `json:"added_at"` + // Unavailable marks an entry whose file is missing from disk (#2527). + // The track is still a real, known track — its history and likes are + // intact and the file may come back — so the entry keeps its place and + // its text; clients render it greyed out and skip over it on playback. + Unavailable bool `json:"unavailable"` } // playlistDetailView extends playlistRowView with the ordered track list. @@ -472,12 +477,20 @@ func playlistDetailToView(d *playlists.PlaylistDetail) playlistDetailView { AlbumTitle: t.AlbumTitle, DurationSec: t.DurationSec, AddedAt: formatTimestamp(t.AddedAt), + Unavailable: t.Unavailable, } if t.TrackID != nil { s := uuidToString(*t.TrackID) v.TrackID = &s - url := streamURL(*t.TrackID) - v.StreamURL = &url + // No stream URL for a missing file. The wire refuses to offer + // a URL that cannot serve rather than trusting every client to + // honour Unavailable — and `"stream_url": null` is a shape the + // clients already handle (the track-removed case), so an older + // build degrades to "present but not playable" on its own. + if !t.Unavailable { + url := streamURL(*t.TrackID) + v.StreamURL = &url + } } if t.AlbumID != nil { s := uuidToString(*t.AlbumID) diff --git a/internal/db/dbq/playlists.sql.go b/internal/db/dbq/playlists.sql.go index e4a9855e..ce928088 100644 --- a/internal/db/dbq/playlists.sql.go +++ b/internal/db/dbq/playlists.sql.go @@ -262,9 +262,10 @@ func (q *Queries) ListAllPlaylistTracksForCollage(ctx context.Context, arg ListA const listPlaylistTracks = `-- name: ListPlaylistTracks :many SELECT pt.playlist_id, pt.position, pt.track_id, pt.title, pt.artist_name, pt.album_title, pt.duration_sec, pt.added_at, pt.pick_kind, - t.id AS live_track_id, - albums.id AS album_id, - artists.id AS artist_id + t.id AS live_track_id, + t.missing_since AS missing_since, + albums.id AS album_id, + artists.id AS artist_id FROM playlist_tracks pt LEFT JOIN tracks t ON t.id = pt.track_id LEFT JOIN albums ON albums.id = t.album_id @@ -274,24 +275,31 @@ ORDER BY pt.position ` type ListPlaylistTracksRow struct { - PlaylistID pgtype.UUID - Position int32 - TrackID pgtype.UUID - Title string - ArtistName string - AlbumTitle string - DurationSec int32 - AddedAt pgtype.Timestamptz - PickKind *string - LiveTrackID pgtype.UUID - AlbumID pgtype.UUID - ArtistID pgtype.UUID + PlaylistID pgtype.UUID + Position int32 + TrackID pgtype.UUID + Title string + ArtistName string + AlbumTitle string + DurationSec int32 + AddedAt pgtype.Timestamptz + PickKind *string + LiveTrackID pgtype.UUID + MissingSince pgtype.Timestamptz + AlbumID pgtype.UUID + ArtistID pgtype.UUID } // Joined to tracks for the live track id (the service layer derives the // stream URL from it); LEFT JOIN preserves the row when track_id is NULL // (track was removed from the library). The denormalized snapshot fields // on playlist_tracks remain authoritative for title/artist/album text. +// +// Deliberately NOT filtered on missing_since (#2527), unlike every browse / +// discover / mix query. A playlist entry is something the user put here on +// purpose, so a missing file stays in the list and renders as a dead row +// rather than silently vanishing; missing_since rides along so the service +// layer can mark it unplayable. func (q *Queries) ListPlaylistTracks(ctx context.Context, playlistID pgtype.UUID) ([]ListPlaylistTracksRow, error) { rows, err := q.db.Query(ctx, listPlaylistTracks, playlistID) if err != nil { @@ -312,6 +320,7 @@ func (q *Queries) ListPlaylistTracks(ctx context.Context, playlistID pgtype.UUID &i.AddedAt, &i.PickKind, &i.LiveTrackID, + &i.MissingSince, &i.AlbumID, &i.ArtistID, ); err != nil { diff --git a/internal/db/queries/playlists.sql b/internal/db/queries/playlists.sql index 058a1ffa..fa414aaa 100644 --- a/internal/db/queries/playlists.sql +++ b/internal/db/queries/playlists.sql @@ -54,10 +54,17 @@ RETURNING id, cover_path; -- stream URL from it); LEFT JOIN preserves the row when track_id is NULL -- (track was removed from the library). The denormalized snapshot fields -- on playlist_tracks remain authoritative for title/artist/album text. +-- +-- Deliberately NOT filtered on missing_since (#2527), unlike every browse / +-- discover / mix query. A playlist entry is something the user put here on +-- purpose, so a missing file stays in the list and renders as a dead row +-- rather than silently vanishing; missing_since rides along so the service +-- layer can mark it unplayable. SELECT pt.*, - t.id AS live_track_id, - albums.id AS album_id, - artists.id AS artist_id + t.id AS live_track_id, + t.missing_since AS missing_since, + albums.id AS album_id, + artists.id AS artist_id FROM playlist_tracks pt LEFT JOIN tracks t ON t.id = pt.track_id LEFT JOIN albums ON albums.id = t.album_id diff --git a/internal/library/delete.go b/internal/library/delete.go index f496871e..81f5bd58 100644 --- a/internal/library/delete.go +++ b/internal/library/delete.go @@ -28,13 +28,22 @@ var ErrTrackNotFound = errors.New("library: track not found") // 3. Delete the tracks row. // // Order matters: file first, then DB. If the file delete fails (permission, -// I/O error), we leave the DB row alone so the admin can retry. The reverse -// failure mode — file gone, DB row still present — is currently NOT -// auto-reconciled (drift #572 audit found the misleading prior claim -// that a scan would clean it up — the scanner only walks + upserts; -// it does not enumerate orphan rows). An admin must re-trigger -// DeleteTrackFile or delete the row manually. A scanrun orphan-row -// sweep is tracked as future work in the audit queue. +// I/O error), we leave the DB row alone so the admin can retry. +// +// The reverse failure mode — file gone, DB row still present — IS reconciled +// now, and not by this function: the scan's reconcile pass stamps +// tracks.missing_since (#2523), every selection path filters on it, and a file +// that returns is un-marked or adopted at its new path (#2528). That is the +// normal life of a vanished file and it is deliberately non-destructive: the +// row, its play history and its likes survive, because a missing file is a +// track Minstrel still knows about (#2527). +// +// So this function is NOT the missing-file path. It is the explicit admin +// action "remove this recording from disk and from the library", and it is +// irreversible: tracks CASCADEs to play_events, general_likes_tracks, +// contextual_likes, track_tags and playback_errors. Reach for it when the +// operator means to destroy the record, never to tidy up a row whose file +// merely went away. func DeleteTrackFile(ctx context.Context, pool *pgxpool.Pool, trackID pgtype.UUID) error { q := dbq.New(pool) track, err := q.GetTrackByID(ctx, trackID) diff --git a/internal/library/watcher.go b/internal/library/watcher.go index 94a66900..dc6946c7 100644 --- a/internal/library/watcher.go +++ b/internal/library/watcher.go @@ -55,6 +55,14 @@ func classifyEvent(op fsnotify.Op, isDir, isAudio bool) watchAction { // It is recursive: a watch is added per directory, and new directories get a // watch as they appear. inotify watch-limit exhaustion on huge libraries is // logged, not fatal -- the periodic safety-net scan covers anything missed. +// +// "Covers anything missed" is true in two different ways, worth separating +// because the walk alone only ever gave one of them: the walk re-visits every +// path that EXISTS, which catches additions and edits, and the reconcile pass +// that runs with it compares the whole tracks table against what the walk saw, +// which is what catches removals (#2523). classifyEvent still ignores fsnotify +// removals by design, so a deleted file surfaces at the next scan, not the +// next inotify event. type Watcher struct { scanner *Scanner enricher *coverart.Enricher diff --git a/internal/playlists/service.go b/internal/playlists/service.go index f919abb8..b6ae36fe 100644 --- a/internal/playlists/service.go +++ b/internal/playlists/service.go @@ -110,6 +110,11 @@ type PlaylistTrack struct { AlbumTitle string DurationSec int32 AddedAt pgtype.Timestamptz + // Unavailable reports that the track still exists in the library but + // its file is currently missing from disk (tracks.missing_since is + // set, #2527). The entry keeps its place in the playlist — the user + // put it there — but nothing should try to play it. + Unavailable bool } // Create makes a new playlist owned by userID. @@ -173,6 +178,7 @@ func (s *Service) Get(ctx context.Context, callerID, playlistID pgtype.UUID) (*P AlbumTitle: t.AlbumTitle, DurationSec: t.DurationSec, AddedAt: t.AddedAt, + Unavailable: t.MissingSince.Valid, } // pt.track_id (snapshot FK, ON DELETE SET NULL) and the joined // live_track_id should agree on validity in normal operation —