diff --git a/internal/db/dbq/albums.sql.go b/internal/db/dbq/albums.sql.go index 98b07b4a..04ce65e6 100644 --- a/internal/db/dbq/albums.sql.go +++ b/internal/db/dbq/albums.sql.go @@ -552,6 +552,7 @@ SELECT a.id AS album_id, SELECT file_path FROM tracks WHERE album_id = a.id + AND missing_since IS NULL ORDER BY disc_number NULLS LAST, track_number NULLS LAST, id LIMIT 1 ) t ON true @@ -569,7 +570,9 @@ type ListAlbumsMissingMbidWithTrackRow struct { // One-shot MBID backfill: returns each album where mbid IS NULL alongside // one of its tracks' file_path so the worker can re-read tags. LIMIT -// supplied by caller for batching/progress purposes. +// supplied by caller for batching/progress purposes. The track read is +// one still on disk; an album with none left is skipped until a scan +// finds its files again (#5139). func (q *Queries) ListAlbumsMissingMbidWithTrack(ctx context.Context, limit int32) ([]ListAlbumsMissingMbidWithTrackRow, error) { rows, err := q.db.Query(ctx, listAlbumsMissingMbidWithTrack, limit) if err != nil { diff --git a/internal/db/dbq/tracks.sql.go b/internal/db/dbq/tracks.sql.go index 4cfe2964..638a5687 100644 --- a/internal/db/dbq/tracks.sql.go +++ b/internal/db/dbq/tracks.sql.go @@ -668,6 +668,7 @@ const listTracksMissingMbidWithPath = `-- name: ListTracksMissingMbidWithPath :m SELECT id, file_path FROM tracks WHERE mbid IS NULL + AND missing_since IS NULL ORDER BY id LIMIT $1 ` @@ -679,6 +680,8 @@ type ListTracksMissingMbidWithPathRow struct { // Track recording-MBID backfill: tracks with NULL mbid that still have // a file to re-read. $1 caps the batch (mirrors the album backfill). +// Missing tracks are skipped: there is no file, and opening it anyway +// logged a warning per track on every scan (#5139). func (q *Queries) ListTracksMissingMbidWithPath(ctx context.Context, limit int32) ([]ListTracksMissingMbidWithPathRow, error) { rows, err := q.db.Query(ctx, listTracksMissingMbidWithPath, limit) if err != nil { diff --git a/internal/db/queries/albums.sql b/internal/db/queries/albums.sql index 6f87dfc8..c9b88fa9 100644 --- a/internal/db/queries/albums.sql +++ b/internal/db/queries/albums.sql @@ -145,7 +145,9 @@ UPDATE albums -- name: ListAlbumsMissingMbidWithTrack :many -- One-shot MBID backfill: returns each album where mbid IS NULL alongside -- one of its tracks' file_path so the worker can re-read tags. LIMIT --- supplied by caller for batching/progress purposes. +-- supplied by caller for batching/progress purposes. The track read is +-- one still on disk; an album with none left is skipped until a scan +-- finds its files again (#5139). SELECT a.id AS album_id, a.artist_id AS artist_id, a.title AS title, @@ -155,6 +157,7 @@ SELECT a.id AS album_id, SELECT file_path FROM tracks WHERE album_id = a.id + AND missing_since IS NULL ORDER BY disc_number NULLS LAST, track_number NULLS LAST, id LIMIT 1 ) t ON true diff --git a/internal/db/queries/tracks.sql b/internal/db/queries/tracks.sql index e8075668..6d0b179e 100644 --- a/internal/db/queries/tracks.sql +++ b/internal/db/queries/tracks.sql @@ -26,9 +26,12 @@ RETURNING *; -- name: ListTracksMissingMbidWithPath :many -- Track recording-MBID backfill: tracks with NULL mbid that still have -- a file to re-read. $1 caps the batch (mirrors the album backfill). +-- Missing tracks are skipped: there is no file, and opening it anyway +-- logged a warning per track on every scan (#5139). SELECT id, file_path FROM tracks WHERE mbid IS NULL + AND missing_since IS NULL ORDER BY id LIMIT $1; diff --git a/internal/library/mbidbackfill_test.go b/internal/library/mbidbackfill_test.go new file mode 100644 index 00000000..a645abe4 --- /dev/null +++ b/internal/library/mbidbackfill_test.go @@ -0,0 +1,71 @@ +package library + +import ( + "bytes" + "context" + "log/slog" + "os" + "path/filepath" + "strings" + "testing" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) + +// The MBID backfills read tags from files, so a track whose file is gone has +// nothing to give them. Before #5139 they opened it anyway, and every scan +// logged an "open failed" warning per missing track. +func TestMBIDBackfill_SkipsMissingTracks_Integration(t *testing.T) { + pool := newPool(t) + ctx := context.Background() + q := dbq.New(pool) + dir := t.TempDir() + + // The album's first track is missing; its second is on disk, untagged. + gone, album, artist := seedTrack(t, pool, filepath.Join(dir, "gone.mp3")) + present := filepath.Join(dir, "present.mp3") + if err := os.WriteFile(present, []byte("not really audio"), 0o600); err != nil { + t.Fatal(err) + } + two := int32(2) + if _, err := q.UpsertTrack(ctx, dbq.UpsertTrackParams{ + Title: "Present", AlbumID: album.ID, ArtistID: artist.ID, TrackNumber: &two, + DurationMs: 1000, FilePath: present, FileSize: 100, FileFormat: "mp3", + }); err != nil { + t.Fatalf("track: %v", err) + } + if _, err := pool.Exec(ctx, "UPDATE tracks SET missing_since = now() WHERE id = $1", gone.ID); err != nil { + t.Fatalf("mark missing: %v", err) + } + + tracks, err := q.ListTracksMissingMbidWithPath(ctx, 100) + if err != nil { + t.Fatal(err) + } + if len(tracks) != 1 || tracks[0].FilePath != present { + t.Errorf("track backfill lists %+v, want only %s", tracks, present) + } + albums, err := q.ListAlbumsMissingMbidWithTrack(ctx, 100) + if err != nil { + t.Fatal(err) + } + if len(albums) != 1 || albums[0].TrackFilePath != present { + t.Errorf("album backfill reads %+v, want the album via %s", albums, present) + } + + // With every track gone, the album has no file to read and is skipped. + if _, err := pool.Exec(ctx, "UPDATE tracks SET missing_since = now() WHERE album_id = $1", album.ID); err != nil { + t.Fatalf("mark missing: %v", err) + } + var logs bytes.Buffer + logger := slog.New(slog.NewTextHandler(&logs, nil)) + if res, err := BackfillTrackMBIDs(ctx, pool, logger, -1, nil); err != nil || res.Processed != 0 { + t.Errorf("track backfill processed %d (err %v), want 0", res.Processed, err) + } + if res, err := BackfillMBIDs(ctx, pool, logger, -1, nil); err != nil || res.Processed != 0 { + t.Errorf("album backfill processed %d (err %v), want 0", res.Processed, err) + } + if strings.Contains(logs.String(), "open failed") { + t.Errorf("a backfill opened a missing file:\n%s", logs.String()) + } +}