Track identity was file_path, so a file that came back renamed or in a different directory looked like a deletion plus an unrelated new track: the old row kept the like and every play_event while a fresh zero-history row appeared, and nothing connected them. A liked song read as unliked, its play count reset, and Rediscover could offer it as a discovery — silently. Renumbering an album was enough, which is what happened to the operator's copy of Minutes to Midnight. Adoption re-points the existing row's file_path at the new location and clears its missing mark. The normal UpsertTrack then conflicts on file_path and updates THAT row, so the track id survives and likes, plays and playlist memberships travel with it — and clients see an update rather than a delete-and-create, so no cache churn either. Matching is MBID first (identifies the recording, so it survives a re-encode), then file_size + duration_ms for untagged files. Both fingerprint components must be non-zero: duration_ms is 0 when ffprobe failed, and matching 0 against 0 would pair up unrelated broken files. Only rows already marked missing are eligible — a row whose file is present elsewhere is a duplicate, not a move, and re-pointing it would corrupt the copy that still exists. An ambiguous match inserts fresh rather than adopting one arbitrarily: a fork is recoverable later, a wrong merge isn't. Scan is now three phases, and the order is the point. Adoption can only claim a row that is ALREADY marked missing, but reconcile previously ran after processing — so a rename performed while the server was down surfaced the deletion and the addition in the same scan, the new path inserted first, and the fork became permanent. Enumeration is therefore separated from processing so reconcile can run between them: walk (paths only, no tag reads or probes) -> reconcile -> process in walk order. Consequence worth knowing: when reconcile refuses (an absent root, or a reorganisation exceeding the 25% mark cap) adoption cannot fire and renamed files fork as before. That's the pre-#2528 behaviour rather than a new failure, and the warning now names it. The old outer walk-error branch was unreachable — the callback always returned nil, so WalkDir never surfaced an error — and verifyRootsPresent is the real protection, so enumerate counts walk errors instead of pretending to abort on them.
311 lines
10 KiB
Go
311 lines
10 KiB
Go
package library
|
|
|
|
import (
|
|
"bytes"
|
|
"context"
|
|
"encoding/binary"
|
|
"io"
|
|
"log/slog"
|
|
"os"
|
|
"path/filepath"
|
|
"testing"
|
|
|
|
"github.com/jackc/pgx/v5/pgxpool"
|
|
|
|
"git.fabledsword.com/bvandeusen/minstrel/internal/db"
|
|
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
|
|
)
|
|
|
|
func TestReleaseDateFromYear(t *testing.T) {
|
|
cases := []struct {
|
|
in int
|
|
ok bool
|
|
label string
|
|
}{
|
|
{0, false, "missing tag"},
|
|
{-1, false, "negative"},
|
|
{1, true, "lower bound"},
|
|
{1999, true, "ordinary"},
|
|
{9999, true, "upper bound"},
|
|
{10000, false, "five digits"},
|
|
{99999999, false, "tag corruption"},
|
|
}
|
|
for _, c := range cases {
|
|
got, ok := releaseDateFromYear(c.in)
|
|
if ok != c.ok {
|
|
t.Errorf("%s: ok = %v, want %v", c.label, ok, c.ok)
|
|
}
|
|
if ok && (!got.Valid || got.Time.Year() != c.in) {
|
|
t.Errorf("%s: date = %+v, want year=%d", c.label, got, c.in)
|
|
}
|
|
}
|
|
}
|
|
|
|
// TestScanner_Integration exercises walk → tag-parse → upsert → incremental
|
|
// skip against a real Postgres. Gated on MINSTREL_TEST_DATABASE_URL.
|
|
func TestScanner_Integration(t *testing.T) {
|
|
if testing.Short() {
|
|
t.Skip("skipping scanner integration in -short mode")
|
|
}
|
|
dsn := os.Getenv("MINSTREL_TEST_DATABASE_URL")
|
|
if dsn == "" {
|
|
t.Skip("MINSTREL_TEST_DATABASE_URL not set")
|
|
}
|
|
ctx := context.Background()
|
|
logger := slog.New(slog.NewTextHandler(io.Discard, nil))
|
|
|
|
if err := db.Migrate(dsn, logger); err != nil {
|
|
t.Fatalf("migrate: %v", err)
|
|
}
|
|
pool, err := pgxpool.New(ctx, dsn)
|
|
if err != nil {
|
|
t.Fatalf("pool: %v", err)
|
|
}
|
|
t.Cleanup(pool.Close)
|
|
|
|
if _, err := pool.Exec(ctx, "TRUNCATE tracks, albums, artists RESTART IDENTITY CASCADE"); err != nil {
|
|
t.Fatalf("truncate: %v", err)
|
|
}
|
|
|
|
root := t.TempDir()
|
|
writeTestMP3(t, filepath.Join(root, "artistX/albumA/01.mp3"), map[string]string{
|
|
"TIT2": "Track One", "TPE1": "Artist X", "TALB": "Album A", "TRCK": "1", "TYER": "2020",
|
|
})
|
|
writeTestMP3(t, filepath.Join(root, "artistX/albumA/02.mp3"), map[string]string{
|
|
"TIT2": "Track Two", "TPE1": "Artist X", "TALB": "Album A", "TRCK": "2", "TYER": "2020",
|
|
})
|
|
writeTestMP3(t, filepath.Join(root, "artistX/albumB/01.mp3"), map[string]string{
|
|
"TIT2": "B Song", "TPE1": "Artist X", "TALB": "Album B", "TRCK": "1",
|
|
})
|
|
writeTestMP3(t, filepath.Join(root, "artistY/01.mp3"), map[string]string{
|
|
"TIT2": "Solo", "TPE1": "The Artist Y", "TALB": "Y Album", "TRCK": "1",
|
|
})
|
|
|
|
scanner := New(pool, logger, []string{root})
|
|
stats, err := scanner.Scan(ctx, nil)
|
|
if err != nil {
|
|
t.Fatalf("first scan: %v", err)
|
|
}
|
|
if stats.Scanned != 4 || stats.Added != 4 {
|
|
t.Errorf("first scan stats = %+v, want Scanned=4 Added=4", stats)
|
|
}
|
|
|
|
q := dbq.New(pool)
|
|
artists, err := q.ListArtists(ctx)
|
|
if err != nil {
|
|
t.Fatalf("ListArtists: %v", err)
|
|
}
|
|
if len(artists) != 2 {
|
|
t.Fatalf("artist rows = %d (%+v), want 2", len(artists), artists)
|
|
}
|
|
// "The Artist Y" should sort under "Artist Y" (article stripped).
|
|
if artists[0].SortName != "Artist X" || artists[1].SortName != "Artist Y" {
|
|
t.Errorf("artist sort = [%q, %q], want [Artist X, Artist Y]", artists[0].SortName, artists[1].SortName)
|
|
}
|
|
|
|
// The synthetic MP3s carry only an ID3 tag — no decodable audio —
|
|
// so ffprobe yields duration 0. The scanner deliberately refuses to
|
|
// skip zero-duration rows (it re-runs them so a later scan can
|
|
// backfill duration once probing works), which is orthogonal to the
|
|
// mtime-based incremental-skip this test covers. Simulate a
|
|
// normally-probed library so the skip path is actually exercised;
|
|
// updated_at is left untouched (still ≥ file mtime).
|
|
if _, err := pool.Exec(ctx, "UPDATE tracks SET duration_ms = 1000 WHERE duration_ms = 0"); err != nil {
|
|
t.Fatalf("seed durations: %v", err)
|
|
}
|
|
|
|
stats2, err := scanner.Scan(ctx, nil)
|
|
if err != nil {
|
|
t.Fatalf("second scan: %v", err)
|
|
}
|
|
if stats2.Added != 0 || stats2.Updated != 0 || stats2.Skipped != 4 {
|
|
t.Errorf("incremental scan stats = %+v, want Skipped=4", stats2)
|
|
}
|
|
|
|
// ScanFiles (watcher-driven targeted scan): a newly-added file is picked
|
|
// up by path without walking the whole tree, and its album ID is returned
|
|
// so the watcher can enrich just that album. A non-audio path in the batch
|
|
// is silently ignored.
|
|
newFile := filepath.Join(root, "artistX/albumA/03.mp3")
|
|
writeTestMP3(t, newFile, map[string]string{
|
|
"TIT2": "Track Three", "TPE1": "Artist X", "TALB": "Album A", "TRCK": "3", "TYER": "2020",
|
|
})
|
|
notAudio := filepath.Join(root, "artistX/albumA/cover.txt")
|
|
if err := os.WriteFile(notAudio, []byte("nope"), 0o644); err != nil {
|
|
t.Fatalf("write non-audio: %v", err)
|
|
}
|
|
changed, err := scanner.ScanFiles(ctx, []string{newFile, notAudio})
|
|
if err != nil {
|
|
t.Fatalf("ScanFiles: %v", err)
|
|
}
|
|
if len(changed) != 1 || !changed[0].Valid {
|
|
t.Fatalf("ScanFiles changed albums = %+v, want exactly 1 valid", changed)
|
|
}
|
|
|
|
// The targeted scan upserts only the named file; the rest of the library
|
|
// is untouched (no full walk).
|
|
if _, err := q.GetTrackByPath(ctx, newFile); err != nil {
|
|
t.Fatalf("GetTrackByPath after ScanFiles: %v", err)
|
|
}
|
|
|
|
// Re-running ScanFiles on the now-unchanged file (duration probed) skips it,
|
|
// so no album is reported as changed.
|
|
if _, err := pool.Exec(ctx, "UPDATE tracks SET duration_ms = 1000 WHERE duration_ms = 0"); err != nil {
|
|
t.Fatalf("seed durations (2): %v", err)
|
|
}
|
|
changed2, err := scanner.ScanFiles(ctx, []string{newFile})
|
|
if err != nil {
|
|
t.Fatalf("ScanFiles (rescan): %v", err)
|
|
}
|
|
if len(changed2) != 0 {
|
|
t.Errorf("ScanFiles on unchanged file changed = %+v, want none", changed2)
|
|
}
|
|
}
|
|
|
|
// writeTestMP3 emits a file with only an ID3v2.3 tag payload plus a few
|
|
// bytes of MPEG-sync trailer. dhowden/tag reads tags without decoding audio,
|
|
// so this is enough for scanner round-trip tests without shipping binaries.
|
|
func writeTestMP3(t *testing.T, path string, frames map[string]string) {
|
|
t.Helper()
|
|
if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
var framesBuf bytes.Buffer
|
|
for id, value := range frames {
|
|
if len(id) != 4 {
|
|
t.Fatalf("frame id %q not 4 bytes", id)
|
|
}
|
|
payload := append([]byte{0x03}, []byte(value)...)
|
|
framesBuf.WriteString(id)
|
|
_ = binary.Write(&framesBuf, binary.BigEndian, uint32(len(payload)))
|
|
framesBuf.WriteByte(0x00)
|
|
framesBuf.WriteByte(0x00)
|
|
framesBuf.Write(payload)
|
|
}
|
|
total := framesBuf.Len()
|
|
header := []byte{
|
|
'I', 'D', '3', 0x03, 0x00, 0x00,
|
|
byte((total >> 21) & 0x7F),
|
|
byte((total >> 14) & 0x7F),
|
|
byte((total >> 7) & 0x7F),
|
|
byte(total & 0x7F),
|
|
}
|
|
f, err := os.Create(path)
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
defer func() { _ = f.Close() }()
|
|
if _, err := f.Write(header); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if _, err := f.Write(framesBuf.Bytes()); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if _, err := f.Write([]byte{0xFF, 0xFB, 0x90, 0x00}); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
}
|
|
|
|
// TestScanner_AdoptsMovedFile_Integration is the #2528 proof: a renamed file
|
|
// must keep its existing tracks row — same id, so likes, play history and
|
|
// playlist memberships travel with it — rather than forking into a marked ghost
|
|
// plus a fresh zero-history row.
|
|
//
|
|
// Uses the MBID path. The synthetic MP3s here carry no real audio, so ffprobe
|
|
// yields duration 0 and the size+duration fingerprint is deliberately unusable —
|
|
// which is why the recording MBID is the signal under test.
|
|
//
|
|
// Eight tracks with one rename keeps the marked fraction at 12.5%, under
|
|
// missingMarkMaxFraction. That is load-bearing: if the rename exceeded the cap,
|
|
// reconcile would refuse to mark, adoption could not fire, and the file would
|
|
// fork. See the "reconcile skipped" warning in Scan.
|
|
func TestScanner_AdoptsMovedFile_Integration(t *testing.T) {
|
|
if testing.Short() {
|
|
t.Skip("skipping scanner integration in -short mode")
|
|
}
|
|
dsn := os.Getenv("MINSTREL_TEST_DATABASE_URL")
|
|
if dsn == "" {
|
|
t.Skip("MINSTREL_TEST_DATABASE_URL not set")
|
|
}
|
|
ctx := context.Background()
|
|
logger := slog.New(slog.NewTextHandler(io.Discard, nil))
|
|
|
|
if err := db.Migrate(dsn, logger); err != nil {
|
|
t.Fatalf("migrate: %v", err)
|
|
}
|
|
pool, err := pgxpool.New(ctx, dsn)
|
|
if err != nil {
|
|
t.Fatalf("pool: %v", err)
|
|
}
|
|
t.Cleanup(pool.Close)
|
|
if _, err := pool.Exec(ctx, "TRUNCATE tracks, albums, artists RESTART IDENTITY CASCADE"); err != nil {
|
|
t.Fatalf("truncate: %v", err)
|
|
}
|
|
|
|
root := t.TempDir()
|
|
const movedMBID = "11111111-2222-3333-4444-555555555555"
|
|
movedFrom := filepath.Join(root, "artistM/albumM/04 - Bleed It Out.mp3")
|
|
writeTestMP3(t, movedFrom, map[string]string{
|
|
"TIT2": "Bleed It Out", "TPE1": "Artist M", "TALB": "Album M", "TRCK": "4",
|
|
// dhowden surfaces TXXX as a Comm whose Description is the Picard tag
|
|
// name; "MusicBrainz Track Id" is mbz.Recording.
|
|
"TXXX": "MusicBrainz Track Id\x00" + movedMBID,
|
|
})
|
|
// Filler so one rename stays under the mark cap.
|
|
for i := 1; i <= 7; i++ {
|
|
writeTestMP3(t, filepath.Join(root, "artistM/albumM/filler", string(rune('a'+i))+".mp3"),
|
|
map[string]string{
|
|
"TIT2": "Filler " + string(rune('0'+i)), "TPE1": "Artist M", "TALB": "Album M",
|
|
})
|
|
}
|
|
|
|
scanner := New(pool, logger, []string{root})
|
|
if _, err := scanner.Scan(ctx, nil); err != nil {
|
|
t.Fatalf("first scan: %v", err)
|
|
}
|
|
|
|
q := dbq.New(pool)
|
|
before, err := q.GetTrackByPath(ctx, movedFrom)
|
|
if err != nil {
|
|
t.Fatalf("track not indexed on first scan: %v", err)
|
|
}
|
|
if before.Mbid == nil || *before.Mbid != movedMBID {
|
|
t.Fatalf("recording mbid not stored: %v", before.Mbid)
|
|
}
|
|
|
|
// Renumber the file, exactly as a tag editor would.
|
|
movedTo := filepath.Join(root, "artistM/albumM/02 - Bleed It Out.mp3")
|
|
if err := os.Rename(movedFrom, movedTo); err != nil {
|
|
t.Fatalf("rename: %v", err)
|
|
}
|
|
|
|
if _, err := scanner.Scan(ctx, nil); err != nil {
|
|
t.Fatalf("second scan: %v", err)
|
|
}
|
|
|
|
after, err := q.GetTrackByPath(ctx, movedTo)
|
|
if err != nil {
|
|
t.Fatalf("track not found at its new path: %v", err)
|
|
}
|
|
if after.ID != before.ID {
|
|
t.Errorf("track id changed on rename: %v -> %v (history would be stranded)",
|
|
before.ID, after.ID)
|
|
}
|
|
if after.MissingSince.Valid {
|
|
t.Errorf("adopted row is still marked missing: %v", after.MissingSince)
|
|
}
|
|
|
|
// The old path must be gone entirely — not lingering as a marked ghost.
|
|
if _, err := q.GetTrackByPath(ctx, movedFrom); err == nil {
|
|
t.Error("old path still has a tracks row; the track forked instead of moving")
|
|
}
|
|
|
|
var total int
|
|
if err := pool.QueryRow(ctx, "SELECT count(*) FROM tracks").Scan(&total); err != nil {
|
|
t.Fatalf("count: %v", err)
|
|
}
|
|
if total != 8 {
|
|
t.Errorf("tracks = %d, want 8 — a rename must not add a row", total)
|
|
}
|
|
}
|