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.
152 lines
5.9 KiB
Go
152 lines
5.9 KiB
Go
package library
|
|
|
|
import (
|
|
"context"
|
|
|
|
"github.com/jackc/pgx/v5/pgtype"
|
|
|
|
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
|
|
syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync"
|
|
)
|
|
|
|
// Move detection (#2528).
|
|
//
|
|
// Track identity is file_path: UpsertTrack conflicts on it, and the reconcile
|
|
// pass in reconcile.go clears a missing mark when the walk sees that same path
|
|
// again. So a file that comes back exactly where it was restores cleanly, but a
|
|
// file that comes back RENAMED or in a different directory looked, to the
|
|
// scanner, like a deletion plus an unrelated new track:
|
|
//
|
|
// - the old row stayed marked missing, holding the like and every play_event
|
|
// - a fresh row appeared with no history
|
|
// - nothing connected them
|
|
//
|
|
// A liked song read as unliked after a retag, its play count reset to zero, and
|
|
// Rediscover could offer it as a discovery. All silently. Renumbering an album
|
|
// was enough to do it — which is exactly what happened on the operator's copy of
|
|
// Minutes to Midnight.
|
|
//
|
|
// The fix adopts the existing row rather than inserting: re-point its file_path
|
|
// at the new location and clear the mark. The caller's 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. Clients see an update
|
|
// rather than a delete-and-create, so no cache churn either.
|
|
//
|
|
// 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. That constraint is what makes this safe, and the
|
|
// marking added in #2523 is what makes it expressible.
|
|
|
|
// trackAdopter is the slice of dbq.Queries move detection needs, narrowed so the
|
|
// match/ambiguity logic can be tested against a fake.
|
|
type trackAdopter interface {
|
|
FindMissingTrackByMbid(ctx context.Context, mbid string) ([]dbq.FindMissingTrackByMbidRow, error)
|
|
FindMissingTrackByFingerprint(ctx context.Context, arg dbq.FindMissingTrackByFingerprintParams) ([]dbq.FindMissingTrackByFingerprintRow, error)
|
|
AdoptTrackPath(ctx context.Context, arg dbq.AdoptTrackPathParams) (int64, error)
|
|
}
|
|
|
|
// adoptMovedTrack looks for a missing row that is the same recording as the file
|
|
// at newPath and re-points it there. Reports whether a row was adopted.
|
|
//
|
|
// Never returns an error: failing to detect a move is a missed optimisation, not
|
|
// a broken scan. The caller carries on and inserts a fresh row, which is the
|
|
// pre-#2528 behaviour.
|
|
func (s *Scanner) adoptMovedTrack(
|
|
ctx context.Context, q trackAdopter, newPath string,
|
|
fileSize int64, durationMs int32, recordingMBID string,
|
|
) bool {
|
|
// MBID first. It identifies the recording rather than the bytes, so it
|
|
// survives a re-encode that the fingerprint cannot.
|
|
if recordingMBID != "" {
|
|
rows, err := q.FindMissingTrackByMbid(ctx, recordingMBID)
|
|
if err != nil {
|
|
s.logger.Warn("library scan: move lookup by mbid failed",
|
|
"path", newPath, "err", err)
|
|
} else if c, ok := s.uniqueMatch(rowsFromMbid(rows), newPath, "mbid"); ok {
|
|
return s.adopt(ctx, q, c, newPath, "mbid")
|
|
}
|
|
}
|
|
|
|
// Fingerprint fallback for untagged files. Both components must be real:
|
|
// duration_ms is 0 when ffprobe failed, and matching 0 against 0 would pair
|
|
// up unrelated broken files.
|
|
if fileSize > 0 && durationMs > 0 {
|
|
rows, err := q.FindMissingTrackByFingerprint(ctx, dbq.FindMissingTrackByFingerprintParams{
|
|
FileSize: fileSize,
|
|
DurationMs: durationMs,
|
|
})
|
|
if err != nil {
|
|
s.logger.Warn("library scan: move lookup by fingerprint failed",
|
|
"path", newPath, "err", err)
|
|
} else if c, ok := s.uniqueMatch(rowsFromFingerprint(rows), newPath, "fingerprint"); ok {
|
|
return s.adopt(ctx, q, c, newPath, "fingerprint")
|
|
}
|
|
}
|
|
|
|
return false
|
|
}
|
|
|
|
// candidate is the shared shape of both lookups, so uniqueMatch is written once.
|
|
type candidate struct {
|
|
id pgtype.UUID
|
|
filePath string
|
|
}
|
|
|
|
func rowsFromMbid(rows []dbq.FindMissingTrackByMbidRow) []candidate {
|
|
out := make([]candidate, 0, len(rows))
|
|
for _, r := range rows {
|
|
out = append(out, candidate{id: r.ID, filePath: r.FilePath})
|
|
}
|
|
return out
|
|
}
|
|
|
|
func rowsFromFingerprint(rows []dbq.FindMissingTrackByFingerprintRow) []candidate {
|
|
out := make([]candidate, 0, len(rows))
|
|
for _, r := range rows {
|
|
out = append(out, candidate{id: r.ID, filePath: r.FilePath})
|
|
}
|
|
return out
|
|
}
|
|
|
|
// uniqueMatch requires exactly one candidate. Adopting an arbitrary row out of
|
|
// several would attach this file's future history to a coin flip, which is worse
|
|
// than starting a fresh row — a fork is recoverable later, a wrong merge isn't.
|
|
// Libraries with genuine duplicates hit this, so it's logged rather than silent.
|
|
func (s *Scanner) uniqueMatch(
|
|
cands []candidate, newPath, via string,
|
|
) (candidate, bool) {
|
|
switch len(cands) {
|
|
case 0:
|
|
return candidate{}, false
|
|
case 1:
|
|
return cands[0], true
|
|
default:
|
|
s.logger.Info("library scan: ambiguous move match, inserting a new track instead",
|
|
"path", newPath, "via", via, "candidates", len(cands))
|
|
return candidate{}, false
|
|
}
|
|
}
|
|
|
|
func (s *Scanner) adopt(
|
|
ctx context.Context, q trackAdopter, c candidate, newPath, via string,
|
|
) bool {
|
|
n, err := q.AdoptTrackPath(ctx, dbq.AdoptTrackPathParams{ID: c.id, FilePath: newPath})
|
|
if err != nil {
|
|
// A unique violation on file_path means something else claimed this path
|
|
// first. Fall through to a normal insert rather than failing the file.
|
|
s.logger.Warn("library scan: adopting moved track failed",
|
|
"path", newPath, "via", via, "err", err)
|
|
return false
|
|
}
|
|
if n == 0 {
|
|
// Lost the race: another file adopted this row between lookup and
|
|
// update, so its mark was already cleared.
|
|
return false
|
|
}
|
|
// Logged with both paths: this is the operator's only window onto a
|
|
// reorganisation being understood as a move rather than a new track.
|
|
s.logger.Info("library scan: track moved, history preserved",
|
|
"from", c.filePath, "to", newPath, "via", via, "track_id", syncpkg.FormatUUID(c.id))
|
|
return true
|
|
}
|