feat(library): fold a missing track into its on-disk replacement (M485 #5286 #5287)
release / web (push) Successful in 2m21s
release / go (push) Successful in 2m34s
release / govulncheck (push) Successful in 40s
release / integration (push) Successful in 6m17s
release / android (push) Successful in 6m36s
release / Build signed APK (releases and dev) (push) Successful in 6m6s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Successful in 1m19s
release / Verify release artifacts (tag releases only) (push) Skipped
release / web (push) Successful in 2m21s
release / go (push) Successful in 2m34s
release / govulncheck (push) Successful in 40s
release / integration (push) Successful in 6m17s
release / android (push) Successful in 6m36s
release / Build signed APK (releases and dev) (push) Successful in 6m6s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Successful in 1m19s
release / Verify release artifacts (tag releases only) (push) Skipped
A track marked missing whose replacement is already on disk under another row — on the operator's library 355 of 451 missing tracks, nearly all Lidarr mp3 -> flac upgrades — is folded into the replacement: likes, plays, playlist entries and tags move across and the missing row goes. Move adoption could not catch these: the replacements were re-encodes (no shared audio hash) with no recording MBID at import. Pairs (ListMissingTrackPairs): the same recording MBID within the album group, or the same album row and title ignoring case. Each side must have exactly one candidate; conflicting MBIDs refuse a pair. No duration or track position gate: on the 142 pairs known to be one recording, 18% differed by over 2s and the poorly tagged set is where numbering is broken (spike #5274). Each pair folds in its own transaction after locking both rows and checking the pair still holds. Runs automatically (operator, 2026-10-07) after a full scan, after a watcher batch that added or updated tracks, and after an AcoustID pass that matched any track. The first scan after deploy repairs the existing rows. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,153 @@
|
||||
// Code generated by sqlc. DO NOT EDIT.
|
||||
// versions:
|
||||
// sqlc v1.31.1
|
||||
// source: missing_pairs.sql
|
||||
|
||||
package dbq
|
||||
|
||||
import (
|
||||
"context"
|
||||
|
||||
"github.com/jackc/pgx/v5/pgtype"
|
||||
)
|
||||
|
||||
const listMissingTrackPairs = `-- name: ListMissingTrackPairs :many
|
||||
|
||||
WITH grp AS (
|
||||
SELECT id AS album_id, COALESCE(release_group_mbid, id::text) AS g
|
||||
FROM albums
|
||||
),
|
||||
cand AS (
|
||||
SELECT m.id AS missing_id, p.id AS present_id, 'mbid'::text AS rule
|
||||
FROM tracks m
|
||||
JOIN grp gm ON gm.album_id = m.album_id
|
||||
JOIN grp gp ON gp.g = gm.g
|
||||
JOIN tracks p ON p.album_id = gp.album_id
|
||||
AND p.missing_since IS NULL
|
||||
AND p.mbid = m.mbid
|
||||
WHERE m.missing_since IS NOT NULL
|
||||
AND m.mbid IS NOT NULL
|
||||
UNION ALL
|
||||
SELECT m.id, p.id, 'title'::text
|
||||
FROM tracks m
|
||||
JOIN tracks p ON p.album_id = m.album_id
|
||||
AND p.missing_since IS NULL
|
||||
AND lower(p.title) = lower(m.title)
|
||||
WHERE m.missing_since IS NOT NULL
|
||||
AND (m.mbid IS NULL OR p.mbid IS NULL OR m.mbid = p.mbid)
|
||||
),
|
||||
pairs AS (
|
||||
-- A pair both rules found is one candidate, credited to the stronger rule.
|
||||
SELECT missing_id, present_id, min(rule)::text AS rule
|
||||
FROM cand
|
||||
GROUP BY missing_id, present_id
|
||||
)
|
||||
SELECT pr.missing_id, pr.present_id, pr.rule,
|
||||
m.file_path AS missing_path, p.file_path AS present_path,
|
||||
(count(*) OVER (PARTITION BY pr.missing_id))::int AS missing_candidates,
|
||||
(count(*) OVER (PARTITION BY pr.present_id))::int AS present_candidates
|
||||
FROM pairs pr
|
||||
JOIN tracks m ON m.id = pr.missing_id
|
||||
JOIN tracks p ON p.id = pr.present_id
|
||||
ORDER BY m.file_path, p.file_path
|
||||
`
|
||||
|
||||
type ListMissingTrackPairsRow struct {
|
||||
MissingID pgtype.UUID
|
||||
PresentID pgtype.UUID
|
||||
Rule string
|
||||
MissingPath string
|
||||
PresentPath string
|
||||
MissingCandidates int32
|
||||
PresentCandidates int32
|
||||
}
|
||||
|
||||
// Missing-pair pass (M485). A track marked missing whose replacement is already
|
||||
// on disk under another row — most often a Lidarr quality upgrade, mp3 → flac at
|
||||
// a new path — is folded into that replacement, so its history moves across and
|
||||
// re-acquisition stops chasing music the library already has.
|
||||
//
|
||||
// Move adoption (#2528) cannot catch these: it links a new file to a missing row
|
||||
// at the moment the file is first inserted, and the replacement arrives as a
|
||||
// re-encode with no recording MBID, so it has nothing to match on. AcoustID may
|
||||
// give it an MBID later; by then both rows exist and only a fold can join them.
|
||||
// Every candidate (missing, present) pair, with how many candidates each side
|
||||
// has. Two rules, measured on the operator's library on 2026-10-07 (#5274):
|
||||
//
|
||||
// mbid the same recording MBID, within the album group (the album row, or
|
||||
// albums sharing a release group). 142 of 359 pairs.
|
||||
// title the same album row and the same title, ignoring case. 213 more. The
|
||||
// replacements are often not the original file — another edition,
|
||||
// re-encoded, untagged — so the metadata is what is left to go on.
|
||||
//
|
||||
// Deliberately NOT gated on duration or track position. On the 142 pairs known
|
||||
// to be one recording, 18% differed by over 2s (old mp3 durations are poor
|
||||
// estimates), and the poorly tagged set is exactly where numbering is broken.
|
||||
//
|
||||
// A pair whose MBIDs are both set and differ is not a title match: the tags say
|
||||
// they are different recordings. The caller folds a pair only when each side has
|
||||
// exactly one candidate; anything else is ambiguous and left alone.
|
||||
func (q *Queries) ListMissingTrackPairs(ctx context.Context) ([]ListMissingTrackPairsRow, error) {
|
||||
rows, err := q.db.Query(ctx, listMissingTrackPairs)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
defer rows.Close()
|
||||
var items []ListMissingTrackPairsRow
|
||||
for rows.Next() {
|
||||
var i ListMissingTrackPairsRow
|
||||
if err := rows.Scan(
|
||||
&i.MissingID,
|
||||
&i.PresentID,
|
||||
&i.Rule,
|
||||
&i.MissingPath,
|
||||
&i.PresentPath,
|
||||
&i.MissingCandidates,
|
||||
&i.PresentCandidates,
|
||||
); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
items = append(items, i)
|
||||
}
|
||||
if err := rows.Err(); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return items, nil
|
||||
}
|
||||
|
||||
const lockTracksForPairing = `-- name: LockTracksForPairing :many
|
||||
SELECT id, missing_since
|
||||
FROM tracks
|
||||
WHERE id = ANY($1::uuid[])
|
||||
ORDER BY id
|
||||
FOR UPDATE
|
||||
`
|
||||
|
||||
type LockTracksForPairingRow struct {
|
||||
ID pgtype.UUID
|
||||
MissingSince pgtype.Timestamptz
|
||||
}
|
||||
|
||||
// Locks both rows of a pair for the fold's transaction, in id order so two
|
||||
// passes cannot deadlock, and returns what the fold re-checks: the missing row
|
||||
// must still be missing and the present one still present. A scan may have
|
||||
// restored or adopted the first, or marked the second, since the pairs were listed.
|
||||
func (q *Queries) LockTracksForPairing(ctx context.Context, ids []pgtype.UUID) ([]LockTracksForPairingRow, error) {
|
||||
rows, err := q.db.Query(ctx, lockTracksForPairing, ids)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
defer rows.Close()
|
||||
var items []LockTracksForPairingRow
|
||||
for rows.Next() {
|
||||
var i LockTracksForPairingRow
|
||||
if err := rows.Scan(&i.ID, &i.MissingSince); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
items = append(items, i)
|
||||
}
|
||||
if err := rows.Err(); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return items, nil
|
||||
}
|
||||
@@ -0,0 +1,75 @@
|
||||
-- Missing-pair pass (M485). A track marked missing whose replacement is already
|
||||
-- on disk under another row — most often a Lidarr quality upgrade, mp3 → flac at
|
||||
-- a new path — is folded into that replacement, so its history moves across and
|
||||
-- re-acquisition stops chasing music the library already has.
|
||||
--
|
||||
-- Move adoption (#2528) cannot catch these: it links a new file to a missing row
|
||||
-- at the moment the file is first inserted, and the replacement arrives as a
|
||||
-- re-encode with no recording MBID, so it has nothing to match on. AcoustID may
|
||||
-- give it an MBID later; by then both rows exist and only a fold can join them.
|
||||
|
||||
-- name: ListMissingTrackPairs :many
|
||||
-- Every candidate (missing, present) pair, with how many candidates each side
|
||||
-- has. Two rules, measured on the operator's library on 2026-10-07 (#5274):
|
||||
--
|
||||
-- mbid the same recording MBID, within the album group (the album row, or
|
||||
-- albums sharing a release group). 142 of 359 pairs.
|
||||
-- title the same album row and the same title, ignoring case. 213 more. The
|
||||
-- replacements are often not the original file — another edition,
|
||||
-- re-encoded, untagged — so the metadata is what is left to go on.
|
||||
--
|
||||
-- Deliberately NOT gated on duration or track position. On the 142 pairs known
|
||||
-- to be one recording, 18% differed by over 2s (old mp3 durations are poor
|
||||
-- estimates), and the poorly tagged set is exactly where numbering is broken.
|
||||
--
|
||||
-- A pair whose MBIDs are both set and differ is not a title match: the tags say
|
||||
-- they are different recordings. The caller folds a pair only when each side has
|
||||
-- exactly one candidate; anything else is ambiguous and left alone.
|
||||
WITH grp AS (
|
||||
SELECT id AS album_id, COALESCE(release_group_mbid, id::text) AS g
|
||||
FROM albums
|
||||
),
|
||||
cand AS (
|
||||
SELECT m.id AS missing_id, p.id AS present_id, 'mbid'::text AS rule
|
||||
FROM tracks m
|
||||
JOIN grp gm ON gm.album_id = m.album_id
|
||||
JOIN grp gp ON gp.g = gm.g
|
||||
JOIN tracks p ON p.album_id = gp.album_id
|
||||
AND p.missing_since IS NULL
|
||||
AND p.mbid = m.mbid
|
||||
WHERE m.missing_since IS NOT NULL
|
||||
AND m.mbid IS NOT NULL
|
||||
UNION ALL
|
||||
SELECT m.id, p.id, 'title'::text
|
||||
FROM tracks m
|
||||
JOIN tracks p ON p.album_id = m.album_id
|
||||
AND p.missing_since IS NULL
|
||||
AND lower(p.title) = lower(m.title)
|
||||
WHERE m.missing_since IS NOT NULL
|
||||
AND (m.mbid IS NULL OR p.mbid IS NULL OR m.mbid = p.mbid)
|
||||
),
|
||||
pairs AS (
|
||||
-- A pair both rules found is one candidate, credited to the stronger rule.
|
||||
SELECT missing_id, present_id, min(rule)::text AS rule
|
||||
FROM cand
|
||||
GROUP BY missing_id, present_id
|
||||
)
|
||||
SELECT pr.missing_id, pr.present_id, pr.rule,
|
||||
m.file_path AS missing_path, p.file_path AS present_path,
|
||||
(count(*) OVER (PARTITION BY pr.missing_id))::int AS missing_candidates,
|
||||
(count(*) OVER (PARTITION BY pr.present_id))::int AS present_candidates
|
||||
FROM pairs pr
|
||||
JOIN tracks m ON m.id = pr.missing_id
|
||||
JOIN tracks p ON p.id = pr.present_id
|
||||
ORDER BY m.file_path, p.file_path;
|
||||
|
||||
-- name: LockTracksForPairing :many
|
||||
-- Locks both rows of a pair for the fold's transaction, in id order so two
|
||||
-- passes cannot deadlock, and returns what the fold re-checks: the missing row
|
||||
-- must still be missing and the present one still present. A scan may have
|
||||
-- restored or adopted the first, or marked the second, since the pairs were listed.
|
||||
SELECT id, missing_since
|
||||
FROM tracks
|
||||
WHERE id = ANY(sqlc.arg(ids)::uuid[])
|
||||
ORDER BY id
|
||||
FOR UPDATE;
|
||||
@@ -215,6 +215,14 @@ func (w *AcoustIDLookupWorker) runOnce(ctx context.Context) {
|
||||
"processed", res.Processed, "matched", res.Matched, "ambiguous", res.Ambiguous,
|
||||
"no_match", res.NoMatch, "failed", res.Failed, "inconclusive", res.Inconclusive)
|
||||
}
|
||||
// A recording MBID AcoustID just gave a replacement may be the one its
|
||||
// missing predecessor carried (M485): most of the operator's stale rows
|
||||
// became pairable exactly this way.
|
||||
if res.Matched > 0 && ctx.Err() == nil {
|
||||
if _, err := PairMissingTracks(ctx, w.pool, w.logger); err != nil && ctx.Err() == nil {
|
||||
w.logger.Warn("acoustid lookup: missing-pair pass failed", "err", err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func (w *AcoustIDLookupWorker) setStatus(f func(*AcoustIDLookupStatus)) {
|
||||
|
||||
@@ -0,0 +1,151 @@
|
||||
package library
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"log/slog"
|
||||
|
||||
"github.com/jackc/pgx/v5"
|
||||
"github.com/jackc/pgx/v5/pgtype"
|
||||
"github.com/jackc/pgx/v5/pgxpool"
|
||||
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
|
||||
)
|
||||
|
||||
// Missing-pair folding (M485).
|
||||
//
|
||||
// A track marked missing whose replacement already sits on disk under another
|
||||
// row is folded into that replacement: its likes, plays, playlist entries and
|
||||
// tags move across and the missing row goes. On the operator's library this was
|
||||
// 355 of 451 missing tracks (#5274), nearly all Lidarr quality upgrades — the
|
||||
// mp3 removed, a flac imported at a new path.
|
||||
//
|
||||
// Move adoption (moved.go) can't catch these. It links a new file to a missing
|
||||
// row when the file is first inserted, matching on the recording MBID or the
|
||||
// audio hash. A re-encode never shares the hash, and the replacements arrived
|
||||
// untagged, so there was nothing to match; AcoustID filled many of the MBIDs
|
||||
// weeks later, by which time both rows existed. Adoption keeps the old row and
|
||||
// re-points its path, which only works while the new file has no row of its own.
|
||||
// Here both do, so this folds instead, with the duplicate merge's row half.
|
||||
//
|
||||
// The rules are in ListMissingTrackPairs. Only a pair where each side has exactly
|
||||
// one candidate is folded: a wrong fold can't be undone, a skipped one is only a
|
||||
// row left in the missing list.
|
||||
//
|
||||
// Unlike the duplicate merge this runs without review (operator, 2026-10-07):
|
||||
// no file is touched, the missing row has nothing to play, and adoption, its
|
||||
// closest relative, is automatic too. Each fold is logged with both paths so a
|
||||
// wrong pair can be read back.
|
||||
|
||||
// PairResult tallies one pass.
|
||||
type PairResult struct {
|
||||
ByMbid int // folded on a shared recording MBID
|
||||
ByTitle int // folded on the same album and title
|
||||
Ambiguous int // missing rows with more than one candidate, or a candidate claimed twice
|
||||
Skipped int // the pair changed between listing and folding
|
||||
Failed int
|
||||
}
|
||||
|
||||
// Folded is how many missing rows the pass folded.
|
||||
func (r PairResult) Folded() int { return r.ByMbid + r.ByTitle }
|
||||
|
||||
// errPairChanged means a pair no longer holds: a scan restored or adopted the
|
||||
// missing row, or marked the present one, after the pairs were listed.
|
||||
var errPairChanged = errors.New("library: missing pair changed before it was folded")
|
||||
|
||||
// PairMissingTracks folds every unambiguous missing pair, one transaction each,
|
||||
// so a failed pair is logged and skipped rather than stopping the rest. It is
|
||||
// idempotent and cheap (one query over the missing rows), so callers run it
|
||||
// wherever a pair can newly appear: after a scan, a watcher batch, or an
|
||||
// AcoustID pass that set an MBID.
|
||||
func PairMissingTracks(ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger) (PairResult, error) {
|
||||
if logger == nil {
|
||||
logger = slog.Default()
|
||||
}
|
||||
var res PairResult
|
||||
pairs, err := dbq.New(pool).ListMissingTrackPairs(ctx)
|
||||
if err != nil {
|
||||
return res, fmt.Errorf("list missing pairs: %w", err)
|
||||
}
|
||||
|
||||
ambiguous := map[[16]byte]struct{}{}
|
||||
for _, p := range pairs {
|
||||
if err := ctx.Err(); err != nil {
|
||||
return res, err
|
||||
}
|
||||
if p.MissingCandidates != 1 || p.PresentCandidates != 1 {
|
||||
ambiguous[p.MissingID.Bytes] = struct{}{}
|
||||
continue
|
||||
}
|
||||
f, err := foldMissingPair(ctx, pool, p.MissingID, p.PresentID)
|
||||
switch {
|
||||
case errors.Is(err, errPairChanged):
|
||||
res.Skipped++
|
||||
continue
|
||||
case err != nil:
|
||||
if ctx.Err() != nil {
|
||||
return res, ctx.Err()
|
||||
}
|
||||
res.Failed++
|
||||
logger.Warn("missing pair: fold failed",
|
||||
"missing_path", p.MissingPath, "present_path", p.PresentPath, "err", err)
|
||||
continue
|
||||
}
|
||||
if p.Rule == "mbid" {
|
||||
res.ByMbid++
|
||||
} else {
|
||||
res.ByTitle++
|
||||
}
|
||||
logger.Info("missing pair: folded into its replacement",
|
||||
"rule", p.Rule,
|
||||
"missing_path", p.MissingPath, "present_path", p.PresentPath,
|
||||
"play_events", f.PlayEvents, "likes", len(f.Likers), "playlist_entries", f.PlaylistEntries)
|
||||
}
|
||||
res.Ambiguous = len(ambiguous)
|
||||
|
||||
if res.Folded() > 0 || res.Failed > 0 {
|
||||
logger.Info("missing pair: pass complete",
|
||||
"by_mbid", res.ByMbid, "by_title", res.ByTitle,
|
||||
"ambiguous", res.Ambiguous, "skipped", res.Skipped, "failed", res.Failed)
|
||||
}
|
||||
return res, nil
|
||||
}
|
||||
|
||||
// foldMissingPair folds one pair in its own transaction, after locking both rows
|
||||
// and checking the pair still holds.
|
||||
func foldMissingPair(ctx context.Context, pool *pgxpool.Pool, missingID, presentID pgtype.UUID) (foldResult, error) {
|
||||
var f foldResult
|
||||
err := pgx.BeginFunc(ctx, pool, func(tx pgx.Tx) error {
|
||||
tq := dbq.New(tx)
|
||||
rows, err := tq.LockTracksForPairing(ctx, []pgtype.UUID{missingID, presentID})
|
||||
if err != nil {
|
||||
return fmt.Errorf("lock pair: %w", err)
|
||||
}
|
||||
if !pairStillHolds(rows, missingID, presentID) {
|
||||
return errPairChanged
|
||||
}
|
||||
changes := mergeChanges{}
|
||||
f, err = foldTrackInto(ctx, tq, presentID, missingID, &changes)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
return changes.log(ctx, tx)
|
||||
})
|
||||
return f, err
|
||||
}
|
||||
|
||||
// pairStillHolds reports whether both rows still exist, the first still marked
|
||||
// missing and the second still present.
|
||||
func pairStillHolds(rows []dbq.LockTracksForPairingRow, missingID, presentID pgtype.UUID) bool {
|
||||
var missingOK, presentOK bool
|
||||
for _, r := range rows {
|
||||
switch r.ID.Bytes {
|
||||
case missingID.Bytes:
|
||||
missingOK = r.MissingSince.Valid
|
||||
case presentID.Bytes:
|
||||
presentOK = !r.MissingSince.Valid
|
||||
}
|
||||
}
|
||||
return missingOK && presentOK
|
||||
}
|
||||
@@ -0,0 +1,314 @@
|
||||
package library
|
||||
|
||||
import (
|
||||
"context"
|
||||
"crypto/sha256"
|
||||
"io"
|
||||
"log/slog"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/jackc/pgx/v5/pgtype"
|
||||
"github.com/jackc/pgx/v5/pgxpool"
|
||||
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/dbtest"
|
||||
syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync"
|
||||
)
|
||||
|
||||
func TestPairStillHolds(t *testing.T) {
|
||||
missing := pgtype.UUID{Bytes: [16]byte{1}, Valid: true}
|
||||
present := pgtype.UUID{Bytes: [16]byte{2}, Valid: true}
|
||||
marked := pgtype.Timestamptz{Time: time.Now(), Valid: true}
|
||||
row := func(id pgtype.UUID, missingSince pgtype.Timestamptz) dbq.LockTracksForPairingRow {
|
||||
return dbq.LockTracksForPairingRow{ID: id, MissingSince: missingSince}
|
||||
}
|
||||
cases := []struct {
|
||||
name string
|
||||
rows []dbq.LockTracksForPairingRow
|
||||
want bool
|
||||
}{
|
||||
{"still a pair", []dbq.LockTracksForPairingRow{row(missing, marked), row(present, pgtype.Timestamptz{})}, true},
|
||||
{"missing row restored", []dbq.LockTracksForPairingRow{row(missing, pgtype.Timestamptz{}), row(present, pgtype.Timestamptz{})}, false},
|
||||
{"present row marked", []dbq.LockTracksForPairingRow{row(missing, marked), row(present, marked)}, false},
|
||||
{"missing row gone", []dbq.LockTracksForPairingRow{row(present, pgtype.Timestamptz{})}, false},
|
||||
{"present row gone", []dbq.LockTracksForPairingRow{row(missing, marked)}, false},
|
||||
}
|
||||
for _, c := range cases {
|
||||
if got := pairStillHolds(c.rows, missing, present); got != c.want {
|
||||
t.Errorf("%s: pairStillHolds = %v, want %v", c.name, got, c.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// pairFixture seeds tracks straight into the tables: the pass reads rows, not
|
||||
// files, so no file needs to exist.
|
||||
type pairFixture struct {
|
||||
t *testing.T
|
||||
pool *pgxpool.Pool
|
||||
q *dbq.Queries
|
||||
artist dbq.Artist
|
||||
}
|
||||
|
||||
func (f pairFixture) exec(sql string, args ...any) {
|
||||
f.t.Helper()
|
||||
if _, err := f.pool.Exec(context.Background(), sql, args...); err != nil {
|
||||
f.t.Fatalf("exec %q: %v", sql, err)
|
||||
}
|
||||
}
|
||||
|
||||
func (f pairFixture) album(title, releaseGroup string) dbq.Album {
|
||||
f.t.Helper()
|
||||
a, err := f.q.UpsertAlbum(context.Background(), dbq.UpsertAlbumParams{
|
||||
Title: title, SortTitle: title, ArtistID: f.artist.ID,
|
||||
})
|
||||
if err != nil {
|
||||
f.t.Fatalf("album %s: %v", title, err)
|
||||
}
|
||||
if releaseGroup != "" {
|
||||
f.exec(`UPDATE albums SET release_group_mbid = $2 WHERE id = $1`, a.ID, releaseGroup)
|
||||
}
|
||||
return a
|
||||
}
|
||||
|
||||
// track adds a row; mbid "" leaves it without one, and missing marks it missing.
|
||||
func (f pairFixture) track(album dbq.Album, title, path, mbid string, missing bool) dbq.Track {
|
||||
f.t.Helper()
|
||||
tr, err := f.q.UpsertTrack(context.Background(), dbq.UpsertTrackParams{
|
||||
Title: title, AlbumID: album.ID, ArtistID: f.artist.ID,
|
||||
DurationMs: 200000, FilePath: path, FileSize: 100, FileFormat: filepath.Ext(path)[1:],
|
||||
})
|
||||
if err != nil {
|
||||
f.t.Fatalf("track %s: %v", path, err)
|
||||
}
|
||||
if mbid != "" {
|
||||
f.exec(`UPDATE tracks SET mbid = $2, mbid_source = 'tag' WHERE id = $1`, tr.ID, mbid)
|
||||
}
|
||||
if missing {
|
||||
f.exec(`UPDATE tracks SET missing_since = now() - interval '1 day' WHERE id = $1`, tr.ID)
|
||||
}
|
||||
return tr
|
||||
}
|
||||
|
||||
func (f pairFixture) exists(tr dbq.Track) bool {
|
||||
f.t.Helper()
|
||||
var n int
|
||||
if err := f.pool.QueryRow(context.Background(), `SELECT count(*) FROM tracks WHERE id = $1`, tr.ID).Scan(&n); err != nil {
|
||||
f.t.Fatalf("count: %v", err)
|
||||
}
|
||||
return n == 1
|
||||
}
|
||||
|
||||
func (f pairFixture) count(sql string, args ...any) int {
|
||||
f.t.Helper()
|
||||
var n int
|
||||
if err := f.pool.QueryRow(context.Background(), sql, args...).Scan(&n); err != nil {
|
||||
f.t.Fatalf("count %q: %v", sql, err)
|
||||
}
|
||||
return n
|
||||
}
|
||||
|
||||
// The M485 proof on rows: each rule folds the pair it should, history moves onto
|
||||
// the replacement, and every shape the pass must leave alone is left alone.
|
||||
func TestPairMissingTracks_Integration(t *testing.T) {
|
||||
pool := newPool(t)
|
||||
ctx := context.Background()
|
||||
q := dbq.New(pool)
|
||||
artist, err := q.UpsertArtist(ctx, dbq.UpsertArtistParams{Name: "Pair Artist", SortName: "Pair Artist"})
|
||||
if err != nil {
|
||||
t.Fatalf("artist: %v", err)
|
||||
}
|
||||
f := pairFixture{t: t, pool: pool, q: q, artist: artist}
|
||||
|
||||
albumA := f.album("Pair Album", "rg-pair")
|
||||
edition := f.album("Pair Album (Deluxe)", "rg-pair") // same release group as albumA
|
||||
other := f.album("Other Album", "rg-other")
|
||||
|
||||
// Rule title: the mp3 went, the flac came, no MBID on either, and the title's
|
||||
// case differs as it does between old and new tags.
|
||||
oldMp3 := f.track(albumA, "Song One", "/m/a/01 - Song One.mp3", "", true)
|
||||
newFlac := f.track(albumA, "song one", "/m/a/01 - song one.flac", "", false)
|
||||
|
||||
// Rule mbid: the same recording, retitled and filed under another edition of
|
||||
// the same release group.
|
||||
oldByMbid := f.track(albumA, "Old Name", "/m/a/02 - Old Name.mp3", "rec-2", true)
|
||||
newByMbid := f.track(edition, "New Name", "/m/d/02 - New Name.flac", "rec-2", false)
|
||||
|
||||
// Ambiguous, missing side: two present copies could be its replacement.
|
||||
twice := f.track(albumA, "Twice", "/m/a/03 - Twice.mp3", "", true)
|
||||
f.track(albumA, "Twice", "/m/a/03 - Twice.flac", "", false)
|
||||
f.track(albumA, "Twice", "/m/a/03 - Twice (live).flac", "", false)
|
||||
|
||||
// Ambiguous, present side: one present copy, two missing rows claiming it.
|
||||
echo1 := f.track(albumA, "Echo", "/m/a/04 - Echo.mp3", "", true)
|
||||
echo2 := f.track(albumA, "Echo", "/m/a/04 - Echo (1).mp3", "", true)
|
||||
f.track(albumA, "Echo", "/m/a/04 - Echo.flac", "", false)
|
||||
|
||||
// Conflicting MBIDs: the tags say these are different recordings.
|
||||
clash := f.track(albumA, "Clash", "/m/a/05 - Clash.mp3", "rec-x", true)
|
||||
f.track(albumA, "Clash", "/m/a/05 - Clash.flac", "rec-y", false)
|
||||
|
||||
// Negative controls across albums: a title match must stay inside one album
|
||||
// row, and an MBID match inside one release group.
|
||||
lonely := f.track(other, "Lonely", "/m/o/01 - Lonely.mp3", "", true)
|
||||
f.track(albumA, "Lonely", "/m/a/06 - Lonely.flac", "", false)
|
||||
cross := f.track(other, "Cross", "/m/o/02 - Cross.mp3", "rec-3", true)
|
||||
f.track(albumA, "Cross Over", "/m/a/07 - Cross Over.flac", "rec-3", false)
|
||||
|
||||
// History on the row being folded.
|
||||
user, err := q.CreateUser(ctx, dbq.CreateUserParams{
|
||||
Username: dbtest.TestUserPrefix + "pair-alice", PasswordHash: "x", ApiTokenHash: "pair-alice-token",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("user: %v", err)
|
||||
}
|
||||
f.exec(`INSERT INTO general_likes (user_id, track_id) VALUES ($1, $2)`, user.ID, oldMp3.ID)
|
||||
now := pgtype.Timestamptz{Time: time.Now(), Valid: true}
|
||||
session, err := q.InsertPlaySession(ctx, dbq.InsertPlaySessionParams{UserID: user.ID, StartedAt: now})
|
||||
if err != nil {
|
||||
t.Fatalf("session: %v", err)
|
||||
}
|
||||
for i := 0; i < 2; i++ {
|
||||
if _, err := q.InsertPlayEvent(ctx, dbq.InsertPlayEventParams{
|
||||
UserID: user.ID, TrackID: oldMp3.ID, SessionID: session.ID, StartedAt: now,
|
||||
}); err != nil {
|
||||
t.Fatalf("play event: %v", err)
|
||||
}
|
||||
}
|
||||
pl, err := q.CreatePlaylist(ctx, dbq.CreatePlaylistParams{UserID: user.ID, Name: "pair-mix"})
|
||||
if err != nil {
|
||||
t.Fatalf("playlist: %v", err)
|
||||
}
|
||||
if _, err := q.AppendPlaylistTrack(ctx, dbq.AppendPlaylistTrackParams{PlaylistID: pl.ID, TrackID: oldMp3.ID}); err != nil {
|
||||
t.Fatalf("playlist entry: %v", err)
|
||||
}
|
||||
|
||||
logger := slog.New(slog.NewTextHandler(io.Discard, nil))
|
||||
res, err := PairMissingTracks(ctx, pool, logger)
|
||||
if err != nil {
|
||||
t.Fatalf("PairMissingTracks: %v", err)
|
||||
}
|
||||
want := PairResult{ByMbid: 1, ByTitle: 1, Ambiguous: 3}
|
||||
if res != want {
|
||||
t.Errorf("result = %+v, want %+v", res, want)
|
||||
}
|
||||
|
||||
if f.exists(oldMp3) || f.exists(oldByMbid) {
|
||||
t.Error("a folded missing row is still in the table")
|
||||
}
|
||||
if !f.exists(newFlac) || !f.exists(newByMbid) {
|
||||
t.Fatal("a replacement row was deleted")
|
||||
}
|
||||
for _, tr := range []dbq.Track{twice, echo1, echo2, clash, lonely, cross} {
|
||||
if !f.exists(tr) {
|
||||
t.Errorf("%s was folded; it should have been left alone", tr.FilePath)
|
||||
}
|
||||
}
|
||||
|
||||
if n := f.count(`SELECT count(*) FROM general_likes WHERE track_id = $1`, newFlac.ID); n != 1 {
|
||||
t.Errorf("likes on the replacement = %d, want 1", n)
|
||||
}
|
||||
if n := f.count(`SELECT count(*) FROM play_events WHERE track_id = $1`, newFlac.ID); n != 2 {
|
||||
t.Errorf("plays on the replacement = %d, want 2", n)
|
||||
}
|
||||
if n := f.count(`SELECT count(*) FROM playlist_tracks WHERE playlist_id = $1 AND track_id = $2`, pl.ID, newFlac.ID); n != 1 {
|
||||
t.Errorf("playlist entries on the replacement = %d, want 1", n)
|
||||
}
|
||||
if n := f.count(`SELECT count(*) FROM library_changes WHERE entity_type = 'track' AND entity_id = $1 AND op = 'delete'`,
|
||||
syncpkg.FormatUUID(oldMp3.ID)); n != 1 {
|
||||
t.Errorf("track delete sync changes for the folded row = %d, want 1", n)
|
||||
}
|
||||
|
||||
// Idempotent: a second pass has nothing left to fold.
|
||||
again, err := PairMissingTracks(ctx, pool, logger)
|
||||
if err != nil {
|
||||
t.Fatalf("second pass: %v", err)
|
||||
}
|
||||
if again.Folded() != 0 || again.Failed != 0 {
|
||||
t.Errorf("second pass = %+v, want nothing folded", again)
|
||||
}
|
||||
}
|
||||
|
||||
// The case that made M485: a Lidarr quality upgrade. The mp3 is removed and a
|
||||
// re-encode of it arrives at a new path with no recording MBID, so move adoption
|
||||
// has nothing to match. The scan must still end with one row, carrying the old
|
||||
// row's history.
|
||||
func TestScanner_FoldsUpgradedFile_Integration(t *testing.T) {
|
||||
pool := newPool(t)
|
||||
ctx := context.Background()
|
||||
logger := slog.New(slog.NewTextHandler(io.Discard, nil))
|
||||
|
||||
root := t.TempDir()
|
||||
oldPath := filepath.Join(root, "artistU/albumU/01 - Upgraded.mp3")
|
||||
tags := map[string]string{"TIT2": "Upgraded", "TPE1": "Artist U", "TALB": "Album U", "TRCK": "1"}
|
||||
writeTestMP3(t, oldPath, tags)
|
||||
// Filler so one removal stays under the mark cap.
|
||||
for i := 1; i <= 7; i++ {
|
||||
writeTestMP3(t, filepath.Join(root, "artistU/albumU/filler", string(rune('a'+i))+".mp3"),
|
||||
map[string]string{"TIT2": "Filler " + string(rune('0'+i)), "TPE1": "Artist U", "TALB": "Album U"})
|
||||
}
|
||||
|
||||
scanner := New(pool, logger, []string{root}, nil)
|
||||
// A re-encode never shares the audio hash, so each file gets its own: the
|
||||
// synthetic files would otherwise hash alike and adopt by hash instead.
|
||||
scanner.fingerprint = func(_ context.Context, path string, _ fingerprintOptions) fingerprintResult {
|
||||
sum := sha256.Sum256([]byte(path))
|
||||
return fingerprintResult{streamSHA256: sum[:]}
|
||||
}
|
||||
if _, err := scanner.Scan(ctx, nil); err != nil {
|
||||
t.Fatalf("first scan: %v", err)
|
||||
}
|
||||
|
||||
q := dbq.New(pool)
|
||||
before, err := q.GetTrackByPath(ctx, oldPath)
|
||||
if err != nil {
|
||||
t.Fatalf("track not indexed on first scan: %v", err)
|
||||
}
|
||||
user, err := q.CreateUser(ctx, dbq.CreateUserParams{
|
||||
Username: dbtest.TestUserPrefix + "upgrade-bob", PasswordHash: "x", ApiTokenHash: "upgrade-bob-token",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("user: %v", err)
|
||||
}
|
||||
if _, err := pool.Exec(ctx, `INSERT INTO general_likes (user_id, track_id) VALUES ($1, $2)`, user.ID, before.ID); err != nil {
|
||||
t.Fatalf("like: %v", err)
|
||||
}
|
||||
|
||||
// The upgrade: the old file goes, the new one lands beside it.
|
||||
if err := os.Remove(oldPath); err != nil {
|
||||
t.Fatalf("remove: %v", err)
|
||||
}
|
||||
newPath := filepath.Join(root, "artistU/albumU/01 - Upgraded (lossless).mp3")
|
||||
writeTestMP3(t, newPath, tags)
|
||||
|
||||
if _, err := scanner.Scan(ctx, nil); err != nil {
|
||||
t.Fatalf("second scan: %v", err)
|
||||
}
|
||||
|
||||
after, err := q.GetTrackByPath(ctx, newPath)
|
||||
if err != nil {
|
||||
t.Fatalf("replacement not indexed: %v", err)
|
||||
}
|
||||
if after.ID == before.ID {
|
||||
t.Fatal("the replacement adopted the old row; this test needs the fold path, so adoption must not match")
|
||||
}
|
||||
if _, err := q.GetTrackByPath(ctx, oldPath); err == nil {
|
||||
t.Error("the old row is still there after the scan; it should have been folded")
|
||||
}
|
||||
var likes int
|
||||
if err := pool.QueryRow(ctx, `SELECT count(*) FROM general_likes WHERE track_id = $1`, after.ID).Scan(&likes); err != nil {
|
||||
t.Fatalf("likes: %v", err)
|
||||
}
|
||||
if likes != 1 {
|
||||
t.Errorf("likes on the replacement = %d, want 1", likes)
|
||||
}
|
||||
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 — an upgrade must not leave a second row", total)
|
||||
}
|
||||
}
|
||||
@@ -175,6 +175,14 @@ func (s *Scanner) Scan(ctx context.Context, progressCb func(Stats)) (Stats, erro
|
||||
if err := ctx.Err(); err != nil {
|
||||
return stats, err
|
||||
}
|
||||
|
||||
// PHASE 4 — fold missing rows into replacements this scan brought in or
|
||||
// that were already here (M485). After processing, so both this scan's
|
||||
// marks and its arrivals are in. Not fatal: an unfolded pair is only a row
|
||||
// left in the missing list, and the next scan tries again.
|
||||
if _, err := PairMissingTracks(ctx, s.pool, s.logger); err != nil && ctx.Err() == nil {
|
||||
s.logger.Warn("library scan: missing-pair pass failed", "err", err)
|
||||
}
|
||||
return stats, nil
|
||||
}
|
||||
|
||||
@@ -466,6 +474,11 @@ func (s *Scanner) ScanFiles(ctx context.Context, paths []string) ([]pgtype.UUID,
|
||||
"skipped", stats.Skipped,
|
||||
"errored", stats.Errored,
|
||||
)
|
||||
// A file the watcher just brought in may be the replacement for a row
|
||||
// an earlier scan marked missing (M485).
|
||||
if _, err := PairMissingTracks(ctx, s.pool, s.logger); err != nil && ctx.Err() == nil {
|
||||
s.logger.Warn("library watch: missing-pair pass failed", "err", err)
|
||||
}
|
||||
}
|
||||
return changed, nil
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user