feat: duplicates resolve themselves without Lidarr (M502 #5517)
release / govulncheck (push) Successful in 37s
release / web (push) Successful in 1m13s
release / go (push) Successful in 1m29s
release / integration (push) Successful in 4m46s
release / android (push) Successful in 5m36s
release / Build signed APK (releases and dev) (push) Successful in 5m54s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Successful in 6m48s
release / Verify release artifacts (tag releases only) (push) Skipped

Lidarr is off, and with it the resolver had stopped merging: it only
merged when Lidarr had said which copies it mapped. Minstrel is now the
only program writing the library, so the rule is about the copies:

- A song held twice on one album: the extra copies merge into the one
  whose tags fit best (then lossless, larger, older).
- Across releases or under another title: when every copy but one is a
  video rip (held back, or named like one and not marked fine), the rips
  merge into the one that is not.
- Clean copies on different releases are linked as one song, as before.
- Anything else waits for the operator.

Gone with Lidarr: release switches and their settle window, the
unmapped-file read, the rip search, the merge guard and its 409/503,
lidarr.Client's release methods, and duplicate_group_members.lidarr_state
(migration 0078). The duplicates page drops the per-copy Lidarr badge
and the blocked-merge notice; past release changes stay in its history.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
2026-10-09 20:12:49 -04:00
co-authored by Claude Opus 5.5
parent 8bd48b4302
commit 5306a261c1
27 changed files with 301 additions and 1743 deletions
+5 -11
View File
@@ -325,18 +325,12 @@ func run() error {
library.SetNotifier(notifier) library.SetNotifier(notifier)
go lidarrReconciler.Run(ctx) go lidarrReconciler.Run(ctx)
// Duplicate resolver (M498): classifies the sweep's groups and, with the // Duplicate resolver (M498, M502): classifies the sweep's groups and, with
// operator's auto-resolve on, merges copies Lidarr does not map and moves a // the operator's auto-resolve on, merges the copies that are safe to merge.
// Lidarr release that lists songs twice. Started after SetNotifier so its // Started after SetNotifier so its summaries reach admins. See
// summaries reach admins. See internal/library/duplicate_resolve.go. // internal/library/duplicate_resolve.go.
resolverLidarr := func() library.LidarrLibrary {
if c := lidarrClientFn(); c != nil {
return c
}
return nil // a nil *lidarr.Client must not become a non-nil interface
}
go library.NewDuplicateResolveWorker(pool, logger.With("component", "duplicate_resolve"), go library.NewDuplicateResolveWorker(pool, logger.With("component", "duplicate_resolve"),
cfg.Storage.DataDir, fpSettings, resolverLidarr).Run(ctx) cfg.Storage.DataDir, fpSettings).Run(ctx)
// Missing-file re-acquisition (milestone #290). Turns albums whose files // Missing-file re-acquisition (milestone #290). Turns albums whose files
// have been gone longer than the grace window into Lidarr requests, on an // have been gone longer than the grace window into Lidarr requests, on an
-16
View File
@@ -14,7 +14,6 @@ import (
"git.fabledsword.com/bvandeusen/minstrel/internal/audit" "git.fabledsword.com/bvandeusen/minstrel/internal/audit"
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
"git.fabledsword.com/bvandeusen/minstrel/internal/library" "git.fabledsword.com/bvandeusen/minstrel/internal/library"
"git.fabledsword.com/bvandeusen/minstrel/internal/tracks"
) )
// duplicateMemberView is one copy in a proposed duplicate group. LikeCount and // duplicateMemberView is one copy in a proposed duplicate group. LikeCount and
@@ -33,10 +32,6 @@ type duplicateMemberView struct {
AddedAt string `json:"added_at"` AddedAt string `json:"added_at"`
LikeCount int64 `json:"like_count"` LikeCount int64 `json:"like_count"`
PlayCount int64 `json:"play_count"` PlayCount int64 `json:"play_count"`
// LidarrState is Lidarr's view of the file as of the resolver's last pass
// (M498): "tracked" (mapped to a track, so it cannot be removed without
// Lidarr downloading it again), "unmapped", or null when not asked.
LidarrState *string `json:"lidarr_state"`
DiscNumber *int32 `json:"disc_number"` DiscNumber *int32 `json:"disc_number"`
TrackNumber *int32 `json:"track_number"` TrackNumber *int32 `json:"track_number"`
} }
@@ -291,13 +286,11 @@ func foldDuplicateGroups(rows []dbq.ListPendingDuplicateGroupMembersRow) []dupli
AddedAt: formatTimestamp(row.AddedAt), AddedAt: formatTimestamp(row.AddedAt),
LikeCount: row.LikeCount, LikeCount: row.LikeCount,
PlayCount: row.PlayCount, PlayCount: row.PlayCount,
LidarrState: row.LidarrState,
DiscNumber: row.DiscNumber, DiscNumber: row.DiscNumber,
TrackNumber: row.TrackNumber, TrackNumber: row.TrackNumber,
}) })
candidates[n] = append(candidates[n], library.SurvivorCandidate{ candidates[n] = append(candidates[n], library.SurvivorCandidate{
TrackID: trackID, FileFormat: row.FileFormat, FileSize: row.FileSize, AddedAt: row.AddedAt.Time, TrackID: trackID, FileFormat: row.FileFormat, FileSize: row.FileSize, AddedAt: row.AddedAt.Time,
LidarrTracked: row.LidarrState != nil && *row.LidarrState == "tracked",
TagFit: library.TagFitScore(row.TrackNumber != nil, row.PositionClash, row.FilePath, row.HasMbid), TagFit: library.TagFitScore(row.TrackNumber != nil, row.PositionClash, row.FilePath, row.HasMbid),
}) })
} }
@@ -398,10 +391,6 @@ const mergeRequestBodyLimit = 1 << 16
// Errors: // Errors:
// - 409 library_not_writable / 500 file_delete_failed when a file could not be // - 409 library_not_writable / 500 file_delete_failed when a file could not be
// removed — nothing was changed (fileRemoveAPIError) // removed — nothing was changed (fileRemoveAPIError)
// - 409 copy_tracked_by_lidarr when a copy to remove is one Lidarr maps to a
// track: removing it would make Lidarr download it again (M498)
// - 503 lidarr_unavailable when Lidarr is on but did not answer, so that
// cannot be checked
// - 404 duplicate_group_not_pending when the group was already resolved // - 404 duplicate_group_not_pending when the group was already resolved
// - 400 survivor_not_in_group, invalid_id, invalid_body // - 400 survivor_not_in_group, invalid_id, invalid_body
func (h *handlers) handleMergeDuplicateGroup(w http.ResponseWriter, r *http.Request) { func (h *handlers) handleMergeDuplicateGroup(w http.ResponseWriter, r *http.Request) {
@@ -439,11 +428,6 @@ func (h *handlers) handleMergeDuplicateGroup(w http.ResponseWriter, r *http.Requ
writeAdminJSONErr(w, http.StatusNotFound, "duplicate_group_not_pending") writeAdminJSONErr(w, http.StatusNotFound, "duplicate_group_not_pending")
case errors.Is(err, library.ErrSurvivorNotInGroup): case errors.Is(err, library.ErrSurvivorNotInGroup):
writeAdminJSONErr(w, http.StatusBadRequest, "survivor_not_in_group") writeAdminJSONErr(w, http.StatusBadRequest, "survivor_not_in_group")
case errors.Is(err, library.ErrCopyTrackedByLidarr):
writeAdminJSONErr(w, http.StatusConflict, "copy_tracked_by_lidarr")
case errors.Is(err, tracks.ErrLidarrUnavailable):
h.logger.Warn("admin: merge duplicate group: lidarr unavailable", "group_id", uuidToString(groupID), "err", err)
writeAdminJSONErr(w, http.StatusServiceUnavailable, "lidarr_unavailable")
default: default:
h.logger.Error("admin: merge duplicate group", "group_id", uuidToString(groupID), "err", err) h.logger.Error("admin: merge duplicate group", "group_id", uuidToString(groupID), "err", err)
writeAdminJSONErr(w, http.StatusInternalServerError, "server_error") writeAdminJSONErr(w, http.StatusInternalServerError, "server_error")
+9 -10
View File
@@ -65,27 +65,26 @@ func TestFoldDuplicateGroups(t *testing.T) {
} }
// M498: the proposal shown beside a group follows the resolver's rule. The copy // M498: the proposal shown beside a group follows the resolver's rule. The copy
// Lidarr tracks is proposed even when the other is lossless, because removing // whose tags fit the album is proposed even when the other is lossless.
// the tracked one would make Lidarr download it again. func TestFoldDuplicateGroups_ProposesTheCopyWhoseTagsFit(t *testing.T) {
func TestFoldDuplicateGroups_ProposesTheCopyLidarrTracks(t *testing.T) {
at := time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC) at := time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC)
tracked, unmapped := "tracked", "unmapped"
class := "same_release" class := "same_release"
three := int32(3)
rows := []dbq.ListPendingDuplicateGroupMembersRow{ rows := []dbq.ListPendingDuplicateGroupMembersRow{
{GroupID: dupUUID(3), Tier: "acoustic", DetectedAt: dupTS(at), TrackID: dupUUID(30), Class: &class, {GroupID: dupUUID(3), Tier: "acoustic", DetectedAt: dupTS(at), TrackID: dupUUID(30), Class: &class,
Title: "Song", FileFormat: "flac", FileSize: 30_000_000, AddedAt: dupTS(at), LidarrState: &unmapped}, Title: "Song", FileFormat: "flac", FileSize: 30_000_000, AddedAt: dupTS(at)},
{GroupID: dupUUID(3), Tier: "acoustic", DetectedAt: dupTS(at), TrackID: dupUUID(31), Class: &class, {GroupID: dupUUID(3), Tier: "acoustic", DetectedAt: dupTS(at), TrackID: dupUUID(31), Class: &class,
Title: "Song", FileFormat: "mp3", FileSize: 9_000_000, AddedAt: dupTS(at), LidarrState: &tracked}, Title: "Song", FileFormat: "mp3", FileSize: 9_000_000, AddedAt: dupTS(at), TrackNumber: &three, HasMbid: true},
} }
got := foldDuplicateGroups(rows) got := foldDuplicateGroups(rows)
if len(got) != 1 { if len(got) != 1 {
t.Fatalf("folded %d groups, want 1", len(got)) t.Fatalf("folded %d groups, want 1", len(got))
} }
g := got[0] g := got[0]
if g.SurvivorTrackID != uuidToString(dupUUID(31)) || g.SurvivorReason != "the copy Lidarr tracks" { if g.SurvivorTrackID != uuidToString(dupUUID(31)) || g.SurvivorReason != "tags fit the album" {
t.Errorf("survivor = (%s, %q), want the tracked mp3", g.SurvivorTrackID, g.SurvivorReason) t.Errorf("survivor = (%s, %q), want the tagged mp3", g.SurvivorTrackID, g.SurvivorReason)
} }
if g.Class == nil || *g.Class != "same_release" || g.Members[1].LidarrState == nil || *g.Members[1].LidarrState != "tracked" { if g.Class == nil || *g.Class != "same_release" {
t.Errorf("class or lidarr state not carried: %+v", g) t.Errorf("class not carried: %+v", g)
} }
} }
+6 -63
View File
@@ -210,32 +210,6 @@ func (q *Queries) GetLatestFingerprintComputedAt(ctx context.Context) (pgtype.Ti
return latest, err return latest, err
} }
const listAlbumPresentTrackTitles = `-- name: ListAlbumPresentTrackTitles :many
SELECT title FROM tracks WHERE album_id = $1 AND missing_since IS NULL
`
// The titles an album has on disk, for choosing the Lidarr release that covers
// them best (M498 #5437).
func (q *Queries) ListAlbumPresentTrackTitles(ctx context.Context, albumID pgtype.UUID) ([]string, error) {
rows, err := q.db.Query(ctx, listAlbumPresentTrackTitles, albumID)
if err != nil {
return nil, err
}
defer rows.Close()
var items []string
for rows.Next() {
var title string
if err := rows.Scan(&title); err != nil {
return nil, err
}
items = append(items, title)
}
if err := rows.Err(); err != nil {
return nil, err
}
return items, nil
}
const listDismissedDuplicateMemberSets = `-- name: ListDismissedDuplicateMemberSets :many const listDismissedDuplicateMemberSets = `-- name: ListDismissedDuplicateMemberSets :many
SELECT g.id, array_agg(m.track_id ORDER BY m.track_id)::uuid[] AS track_ids SELECT g.id, array_agg(m.track_id ORDER BY m.track_id)::uuid[] AS track_ids
FROM duplicate_groups g FROM duplicate_groups g
@@ -343,6 +317,7 @@ SELECT g.id AS group_id,
t.id AS track_id, t.id AS track_id,
t.album_id, t.album_id,
t.title, t.title,
t.source_verdict,
artists.name AS artist_name, artists.name AS artist_name,
t.file_path, t.file_path,
t.file_format, t.file_format,
@@ -351,8 +326,6 @@ SELECT g.id AS group_id,
t.disc_number, t.disc_number,
t.track_number, t.track_number,
(t.mbid IS NOT NULL)::boolean AS has_mbid, (t.mbid IS NOT NULL)::boolean AS has_mbid,
albums.release_group_mbid,
albums.title AS album_title,
EXISTS (SELECT 1 FROM tracks o EXISTS (SELECT 1 FROM tracks o
WHERE o.album_id = t.album_id AND o.id <> t.id AND o.missing_since IS NULL WHERE o.album_id = t.album_id AND o.id <> t.id AND o.missing_since IS NULL
AND t.track_number IS NOT NULL AND t.track_number IS NOT NULL
@@ -361,7 +334,6 @@ SELECT g.id AS group_id,
FROM duplicate_groups g FROM duplicate_groups g
JOIN duplicate_group_members m ON m.group_id = g.id JOIN duplicate_group_members m ON m.group_id = g.id
JOIN tracks t ON t.id = m.track_id JOIN tracks t ON t.id = m.track_id
JOIN albums ON albums.id = t.album_id
JOIN artists ON artists.id = t.artist_id JOIN artists ON artists.id = t.artist_id
WHERE g.status = 'pending' WHERE g.status = 'pending'
AND t.missing_since IS NULL AND t.missing_since IS NULL
@@ -374,6 +346,7 @@ type ListDuplicateGroupsForResolveRow struct {
TrackID pgtype.UUID TrackID pgtype.UUID
AlbumID pgtype.UUID AlbumID pgtype.UUID
Title string Title string
SourceVerdict *string
ArtistName string ArtistName string
FilePath string FilePath string
FileFormat string FileFormat string
@@ -382,8 +355,6 @@ type ListDuplicateGroupsForResolveRow struct {
DiscNumber *int32 DiscNumber *int32
TrackNumber *int32 TrackNumber *int32
HasMbid bool HasMbid bool
ReleaseGroupMbid *string
AlbumTitle string
PositionClash bool PositionClash bool
} }
@@ -391,7 +362,8 @@ type ListDuplicateGroupsForResolveRow struct {
// needs to classify the group and to choose the copy to keep. One row per copy, // needs to classify the group and to choose the copy to keep. One row per copy,
// grouped by group. position_clash is true when another present track on the // grouped by group. position_clash is true when another present track on the
// same album claims the same disc and track number: tags that do not fit the // same album claims the same disc and track number: tags that do not fit the
// album, which the survivor rule weighs against a copy. // album, which the survivor rule weighs against a copy. source_verdict says
// whether the copy is a video rip ('suspect') or one the operator kept ('fine').
func (q *Queries) ListDuplicateGroupsForResolve(ctx context.Context) ([]ListDuplicateGroupsForResolveRow, error) { func (q *Queries) ListDuplicateGroupsForResolve(ctx context.Context) ([]ListDuplicateGroupsForResolveRow, error) {
rows, err := q.db.Query(ctx, listDuplicateGroupsForResolve) rows, err := q.db.Query(ctx, listDuplicateGroupsForResolve)
if err != nil { if err != nil {
@@ -407,6 +379,7 @@ func (q *Queries) ListDuplicateGroupsForResolve(ctx context.Context) ([]ListDupl
&i.TrackID, &i.TrackID,
&i.AlbumID, &i.AlbumID,
&i.Title, &i.Title,
&i.SourceVerdict,
&i.ArtistName, &i.ArtistName,
&i.FilePath, &i.FilePath,
&i.FileFormat, &i.FileFormat,
@@ -415,8 +388,6 @@ func (q *Queries) ListDuplicateGroupsForResolve(ctx context.Context) ([]ListDupl
&i.DiscNumber, &i.DiscNumber,
&i.TrackNumber, &i.TrackNumber,
&i.HasMbid, &i.HasMbid,
&i.ReleaseGroupMbid,
&i.AlbumTitle,
&i.PositionClash, &i.PositionClash,
); err != nil { ); err != nil {
return nil, err return nil, err
@@ -487,7 +458,6 @@ SELECT p.id AS group_id,
p.detected_at, p.detected_at,
p.class, p.class,
p.resolve_note, p.resolve_note,
m.lidarr_state,
t.disc_number, t.disc_number,
t.track_number, t.track_number,
(t.mbid IS NOT NULL)::boolean AS has_mbid, (t.mbid IS NOT NULL)::boolean AS has_mbid,
@@ -529,7 +499,6 @@ type ListPendingDuplicateGroupMembersRow struct {
DetectedAt pgtype.Timestamptz DetectedAt pgtype.Timestamptz
Class *string Class *string
ResolveNote *string ResolveNote *string
LidarrState *string
DiscNumber *int32 DiscNumber *int32
TrackNumber *int32 TrackNumber *int32
HasMbid bool HasMbid bool
@@ -552,8 +521,7 @@ type ListPendingDuplicateGroupMembersRow struct {
// newest first, flattened to one row per member so the handler folds them // newest first, flattened to one row per member so the handler folds them
// without a query per group. What each copy carries — likes and plays from // without a query per group. What each copy carries — likes and plays from
// every user — is here because it is what the operator weighs when deciding // every user — is here because it is what the operator weighs when deciding
// which copy to keep; Lidarr's view of it, because a copy Lidarr tracks cannot // which copy to keep.
// be removed without Lidarr downloading it again.
func (q *Queries) ListPendingDuplicateGroupMembers(ctx context.Context, arg ListPendingDuplicateGroupMembersParams) ([]ListPendingDuplicateGroupMembersRow, error) { func (q *Queries) ListPendingDuplicateGroupMembers(ctx context.Context, arg ListPendingDuplicateGroupMembersParams) ([]ListPendingDuplicateGroupMembersRow, error) {
rows, err := q.db.Query(ctx, listPendingDuplicateGroupMembers, arg.View, arg.PageOffset, arg.PageLimit) rows, err := q.db.Query(ctx, listPendingDuplicateGroupMembers, arg.View, arg.PageOffset, arg.PageLimit)
if err != nil { if err != nil {
@@ -570,7 +538,6 @@ func (q *Queries) ListPendingDuplicateGroupMembers(ctx context.Context, arg List
&i.DetectedAt, &i.DetectedAt,
&i.Class, &i.Class,
&i.ResolveNote, &i.ResolveNote,
&i.LidarrState,
&i.DiscNumber, &i.DiscNumber,
&i.TrackNumber, &i.TrackNumber,
&i.HasMbid, &i.HasMbid,
@@ -632,30 +599,6 @@ func (q *Queries) SetDuplicateGroupClasses(ctx context.Context, arg SetDuplicate
return err return err
} }
const setDuplicateMemberLidarrStates = `-- name: SetDuplicateMemberLidarrStates :exec
UPDATE duplicate_group_members m
SET lidarr_state = NULLIF(v.state, '')
FROM (SELECT unnest($1::uuid[]) AS group_id,
unnest($2::uuid[]) AS track_id,
unnest($3::text[]) AS state) v
WHERE m.group_id = v.group_id
AND m.track_id = v.track_id
AND m.lidarr_state IS DISTINCT FROM NULLIF(v.state, '')
`
type SetDuplicateMemberLidarrStatesParams struct {
GroupIds []pgtype.UUID
TrackIds []pgtype.UUID
States []string
}
// Lidarr's view of each copy as of this pass; an empty state clears it (Lidarr
// not consulted). Unchanged rows are left alone.
func (q *Queries) SetDuplicateMemberLidarrStates(ctx context.Context, arg SetDuplicateMemberLidarrStatesParams) error {
_, err := q.db.Exec(ctx, setDuplicateMemberLidarrStates, arg.GroupIds, arg.TrackIds, arg.States)
return err
}
const startDuplicateSweep = `-- name: StartDuplicateSweep :one const startDuplicateSweep = `-- name: StartDuplicateSweep :one
INSERT INTO duplicate_sweeps DEFAULT VALUES RETURNING id, started_at INSERT INTO duplicate_sweeps DEFAULT VALUES RETURNING id, started_at
` `
-1
View File
@@ -339,7 +339,6 @@ type DuplicateGroup struct {
type DuplicateGroupMember struct { type DuplicateGroupMember struct {
GroupID pgtype.UUID GroupID pgtype.UUID
TrackID pgtype.UUID TrackID pgtype.UUID
LidarrState *string
} }
type DuplicateSweep struct { type DuplicateSweep struct {
-72
View File
@@ -405,78 +405,6 @@ func (q *Queries) GetTracksByIDs(ctx context.Context, dollar_1 []pgtype.UUID) ([
return items, nil return items, nil
} }
const listAlbumsWithSoleCopyRips = `-- name: ListAlbumsWithSoleCopyRips :many
SELECT a.id AS album_id,
a.release_group_mbid::text AS release_group_mbid,
a.title AS album_title,
ar.name AS artist_name,
count(*)::bigint AS rips
FROM tracks t
JOIN albums a ON a.id = t.album_id
JOIN artists ar ON ar.id = a.artist_id
WHERE t.source_verdict = 'suspect'
AND t.missing_since IS NULL
AND a.release_group_mbid IS NOT NULL
AND NOT EXISTS (
SELECT 1 FROM tracks c
WHERE c.song_key = t.song_key AND c.id <> t.id
AND c.missing_since IS NULL AND c.source_verdict IS DISTINCT FROM 'suspect')
AND NOT EXISTS (
SELECT 1 FROM duplicate_group_members m
JOIN duplicate_groups g ON g.id = m.group_id AND g.status = 'pending'
JOIN duplicate_group_members m2 ON m2.group_id = g.id AND m2.track_id <> t.id
JOIN tracks c ON c.id = m2.track_id
WHERE m.track_id = t.id
AND c.missing_since IS NULL AND c.source_verdict IS DISTINCT FROM 'suspect')
AND NOT EXISTS (
SELECT 1 FROM audit_log l
WHERE l.action = 'lidarr_rip_search'
AND l.metadata->>'album_id' = a.id::text
AND l.created_at > now() - interval '7 days')
GROUP BY a.id, a.release_group_mbid, a.title, ar.name
ORDER BY count(*) DESC, a.id
LIMIT $1
`
type ListAlbumsWithSoleCopyRipsRow struct {
AlbumID pgtype.UUID
ReleaseGroupMbid string
AlbumTitle string
ArtistName string
Rips int64
}
// Albums holding a held-back video rip that is the only copy of its song
// (#5447): no present copy that is not a rip shares its song, and no pending
// duplicate group pairs it with one. Lidarr counts such a track fulfilled, so
// the resolver asks it to search the album for a better release. Albums
// searched within the last week are left out, most rips first.
func (q *Queries) ListAlbumsWithSoleCopyRips(ctx context.Context, pageLimit int32) ([]ListAlbumsWithSoleCopyRipsRow, error) {
rows, err := q.db.Query(ctx, listAlbumsWithSoleCopyRips, pageLimit)
if err != nil {
return nil, err
}
defer rows.Close()
var items []ListAlbumsWithSoleCopyRipsRow
for rows.Next() {
var i ListAlbumsWithSoleCopyRipsRow
if err := rows.Scan(
&i.AlbumID,
&i.ReleaseGroupMbid,
&i.AlbumTitle,
&i.ArtistName,
&i.Rips,
); err != nil {
return nil, err
}
items = append(items, i)
}
if err := rows.Err(); err != nil {
return nil, err
}
return items, nil
}
const listArtistTracksForUser = `-- name: ListArtistTracksForUser :many const listArtistTracksForUser = `-- name: ListArtistTracksForUser :many
SELECT t.id, t.title, t.album_id, t.artist_id, t.track_number, t.disc_number, t.duration_ms, t.file_path, t.file_size, t.file_format, t.bitrate, t.mbid, t.genre, t.added_at, t.updated_at, t.tag_source, t.tag_sources_version, t.tag_read_version, t.missing_since, t.mbid_source, t.song_id, t.song_key, t.source_verdict, t.source_verdict_at, SELECT t.id, t.title, t.album_id, t.artist_id, t.track_number, t.disc_number, t.duration_ms, t.file_path, t.file_size, t.file_format, t.bitrate, t.mbid, t.genre, t.added_at, t.updated_at, t.tag_source, t.tag_sources_version, t.tag_read_version, t.missing_since, t.mbid_source, t.song_id, t.song_key, t.source_verdict, t.source_verdict_at,
albums.title AS album_title, albums.title AS album_title,
@@ -0,0 +1,3 @@
ALTER TABLE duplicate_group_members ADD COLUMN lidarr_state text;
ALTER TABLE duplicate_group_members ADD CONSTRAINT duplicate_group_members_lidarr_state_check
CHECK (lidarr_state IN ('tracked', 'unmapped'));
@@ -0,0 +1,9 @@
-- 0078_drop_duplicate_lidarr_state.up.sql — the duplicate resolver works
-- without Lidarr (Scribe milestone #502, #5517).
--
-- 0075 recorded Lidarr's view of each duplicate copy (tracked or unmapped)
-- because a copy Lidarr mapped could not be removed without Lidarr downloading
-- it again. Lidarr is gone: Minstrel is the only program writing the library,
-- so no copy is anyone else's to keep, and the column has nothing to say.
ALTER TABLE duplicate_group_members DROP CONSTRAINT IF EXISTS duplicate_group_members_lidarr_state_check;
ALTER TABLE duplicate_group_members DROP COLUMN IF EXISTS lidarr_state;
+4 -24
View File
@@ -131,8 +131,7 @@ SELECT count(*)::bigint
-- newest first, flattened to one row per member so the handler folds them -- newest first, flattened to one row per member so the handler folds them
-- without a query per group. What each copy carries — likes and plays from -- without a query per group. What each copy carries — likes and plays from
-- every user — is here because it is what the operator weighs when deciding -- every user — is here because it is what the operator weighs when deciding
-- which copy to keep; Lidarr's view of it, because a copy Lidarr tracks cannot -- which copy to keep.
-- be removed without Lidarr downloading it again.
WITH page AS ( WITH page AS (
SELECT g.id, g.tier, g.worst_bit_error_rate, g.detected_at, g.class, g.resolve_note SELECT g.id, g.tier, g.worst_bit_error_rate, g.detected_at, g.class, g.resolve_note
FROM duplicate_groups g FROM duplicate_groups g
@@ -151,7 +150,6 @@ SELECT p.id AS group_id,
p.detected_at, p.detected_at,
p.class, p.class,
p.resolve_note, p.resolve_note,
m.lidarr_state,
t.disc_number, t.disc_number,
t.track_number, t.track_number,
(t.mbid IS NOT NULL)::boolean AS has_mbid, (t.mbid IS NOT NULL)::boolean AS has_mbid,
@@ -201,12 +199,14 @@ SELECT count(*)::bigint
-- needs to classify the group and to choose the copy to keep. One row per copy, -- needs to classify the group and to choose the copy to keep. One row per copy,
-- grouped by group. position_clash is true when another present track on the -- grouped by group. position_clash is true when another present track on the
-- same album claims the same disc and track number: tags that do not fit the -- same album claims the same disc and track number: tags that do not fit the
-- album, which the survivor rule weighs against a copy. -- album, which the survivor rule weighs against a copy. source_verdict says
-- whether the copy is a video rip ('suspect') or one the operator kept ('fine').
SELECT g.id AS group_id, SELECT g.id AS group_id,
g.tier, g.tier,
t.id AS track_id, t.id AS track_id,
t.album_id, t.album_id,
t.title, t.title,
t.source_verdict,
artists.name AS artist_name, artists.name AS artist_name,
t.file_path, t.file_path,
t.file_format, t.file_format,
@@ -215,8 +215,6 @@ SELECT g.id AS group_id,
t.disc_number, t.disc_number,
t.track_number, t.track_number,
(t.mbid IS NOT NULL)::boolean AS has_mbid, (t.mbid IS NOT NULL)::boolean AS has_mbid,
albums.release_group_mbid,
albums.title AS album_title,
EXISTS (SELECT 1 FROM tracks o EXISTS (SELECT 1 FROM tracks o
WHERE o.album_id = t.album_id AND o.id <> t.id AND o.missing_since IS NULL WHERE o.album_id = t.album_id AND o.id <> t.id AND o.missing_since IS NULL
AND t.track_number IS NOT NULL AND t.track_number IS NOT NULL
@@ -225,7 +223,6 @@ SELECT g.id AS group_id,
FROM duplicate_groups g FROM duplicate_groups g
JOIN duplicate_group_members m ON m.group_id = g.id JOIN duplicate_group_members m ON m.group_id = g.id
JOIN tracks t ON t.id = m.track_id JOIN tracks t ON t.id = m.track_id
JOIN albums ON albums.id = t.album_id
JOIN artists ON artists.id = t.artist_id JOIN artists ON artists.id = t.artist_id
WHERE g.status = 'pending' WHERE g.status = 'pending'
AND t.missing_since IS NULL AND t.missing_since IS NULL
@@ -244,22 +241,5 @@ UPDATE duplicate_groups g
AND g.status = 'pending' AND g.status = 'pending'
AND (g.class IS DISTINCT FROM v.class OR g.resolve_note IS DISTINCT FROM NULLIF(v.note, '')); AND (g.class IS DISTINCT FROM v.class OR g.resolve_note IS DISTINCT FROM NULLIF(v.note, ''));
-- name: SetDuplicateMemberLidarrStates :exec
-- Lidarr's view of each copy as of this pass; an empty state clears it (Lidarr
-- not consulted). Unchanged rows are left alone.
UPDATE duplicate_group_members m
SET lidarr_state = NULLIF(v.state, '')
FROM (SELECT unnest(sqlc.arg(group_ids)::uuid[]) AS group_id,
unnest(sqlc.arg(track_ids)::uuid[]) AS track_id,
unnest(sqlc.arg(states)::text[]) AS state) v
WHERE m.group_id = v.group_id
AND m.track_id = v.track_id
AND m.lidarr_state IS DISTINCT FROM NULLIF(v.state, '');
-- name: MarkDuplicateGroupResolvedAutomatically :exec -- name: MarkDuplicateGroupResolvedAutomatically :exec
UPDATE duplicate_groups SET resolved_automatically = true WHERE id = sqlc.arg(id); UPDATE duplicate_groups SET resolved_automatically = true WHERE id = sqlc.arg(id);
-- name: ListAlbumPresentTrackTitles :many
-- The titles an album has on disk, for choosing the Lidarr release that covers
-- them best (M498 #5437).
SELECT title FROM tracks WHERE album_id = sqlc.arg(album_id) AND missing_since IS NULL;
-37
View File
@@ -334,40 +334,3 @@ UPDATE tracks
SET source_verdict = sqlc.arg(verdict)::text, source_verdict_at = now() SET source_verdict = sqlc.arg(verdict)::text, source_verdict_at = now()
WHERE id = sqlc.arg(id) WHERE id = sqlc.arg(id)
AND source_verdict IS NOT NULL; AND source_verdict IS NOT NULL;
-- name: ListAlbumsWithSoleCopyRips :many
-- Albums holding a held-back video rip that is the only copy of its song
-- (#5447): no present copy that is not a rip shares its song, and no pending
-- duplicate group pairs it with one. Lidarr counts such a track fulfilled, so
-- the resolver asks it to search the album for a better release. Albums
-- searched within the last week are left out, most rips first.
SELECT a.id AS album_id,
a.release_group_mbid::text AS release_group_mbid,
a.title AS album_title,
ar.name AS artist_name,
count(*)::bigint AS rips
FROM tracks t
JOIN albums a ON a.id = t.album_id
JOIN artists ar ON ar.id = a.artist_id
WHERE t.source_verdict = 'suspect'
AND t.missing_since IS NULL
AND a.release_group_mbid IS NOT NULL
AND NOT EXISTS (
SELECT 1 FROM tracks c
WHERE c.song_key = t.song_key AND c.id <> t.id
AND c.missing_since IS NULL AND c.source_verdict IS DISTINCT FROM 'suspect')
AND NOT EXISTS (
SELECT 1 FROM duplicate_group_members m
JOIN duplicate_groups g ON g.id = m.group_id AND g.status = 'pending'
JOIN duplicate_group_members m2 ON m2.group_id = g.id AND m2.track_id <> t.id
JOIN tracks c ON c.id = m2.track_id
WHERE m.track_id = t.id
AND c.missing_since IS NULL AND c.source_verdict IS DISTINCT FROM 'suspect')
AND NOT EXISTS (
SELECT 1 FROM audit_log l
WHERE l.action = 'lidarr_rip_search'
AND l.metadata->>'album_id' = a.id::text
AND l.created_at > now() - interval '7 days')
GROUP BY a.id, a.release_group_mbid, a.title, ar.name
ORDER BY count(*) DESC, a.id
LIMIT sqlc.arg(page_limit);
+3 -10
View File
@@ -13,11 +13,11 @@ import (
// 498's body) splits into four shapes: // 498's body) splits into four shapes:
// //
// - same_release: every copy is on one album. An album holds a song once, so // - same_release: every copy is on one album. An album holds a song once, so
// the extra copies are the ones to fold away — when Lidarr does not need // the extra copies are the ones to fold away.
// them (the resolver checks).
// - cross_release: the copies are on different albums under one title. A // - cross_release: the copies are on different albums under one title. A
// single and the album it came from, a deluxe edition and the standard one: // single and the album it came from, a deluxe edition and the standard one:
// each copy fulfils its own release in Lidarr, so none is deleted. // each copy belongs to its own release, so none is deleted (unless it is a
// video rip; the resolver decides).
// - mismatch: byte-identical audio on different albums under different // - mismatch: byte-identical audio on different albums under different
// titles. One file carries another song's tags — a wrong-file import. // titles. One file carries another song's tags — a wrong-file import.
// - review: anything else. The operator decides. // - review: anything else. The operator decides.
@@ -123,13 +123,6 @@ func MatchTitleKey(title, artist string) string {
return alnumKey(title) return alnumKey(title)
} }
// ReleaseTitleKey is the lighter comparison for asking whether one release
// lists a song twice: case, "&" and punctuation only. Every word stays, so a
// box set's "Song" and "Song (demo)" are two songs, as the release means them.
func ReleaseTitleKey(title string) string {
return alnumKey(title)
}
// alnumKey lowercases, reads "&" as "and", and keeps only letters and digits. // alnumKey lowercases, reads "&" as "and", and keeps only letters and digits.
func alnumKey(s string) string { func alnumKey(s string) string {
s = strings.ReplaceAll(strings.ToLower(s), "&", " and ") s = strings.ReplaceAll(strings.ToLower(s), "&", " and ")
@@ -59,13 +59,3 @@ func TestMatchTitleKey(t *testing.T) {
} }
} }
} }
// The release check keeps every word: a box set's demo is its own song.
func TestReleaseTitleKey(t *testing.T) {
if ReleaseTitleKey("Song (demo)") == ReleaseTitleKey("Song") {
t.Error("ReleaseTitleKey collapsed a demo into the song")
}
if ReleaseTitleKey("Hallelujah Money") != ReleaseTitleKey("hallelujah money!") {
t.Error("ReleaseTitleKey kept case or punctuation")
}
}
-28
View File
@@ -25,16 +25,6 @@ var ErrDuplicateGroupNotPending = errors.New("library: duplicate group is not pe
// ErrSurvivorNotInGroup means the copy chosen to keep is not a member of the group. // ErrSurvivorNotInGroup means the copy chosen to keep is not a member of the group.
var ErrSurvivorNotInGroup = errors.New("library: survivor is not a member of the group") var ErrSurvivorNotInGroup = errors.New("library: survivor is not a member of the group")
// ErrCopyTrackedByLidarr means a copy the merge would remove is one Lidarr maps
// to a track of the release it monitors (M498). Removing it would open a hole
// that Lidarr fills by downloading the song again, so the merge refuses and
// changes nothing.
var ErrCopyTrackedByLidarr = errors.New("library: a copy to remove is tracked by Lidarr")
// RemovableFunc reports whether the file at filePath may be removed. A merge
// asks it about every copy it would remove, before touching anything.
type RemovableFunc func(filePath string) bool
// MergedCopy is one copy a merge kept or removed. // MergedCopy is one copy a merge kept or removed.
type MergedCopy struct { type MergedCopy struct {
TrackID pgtype.UUID TrackID pgtype.UUID
@@ -85,17 +75,6 @@ type MergeResult struct {
func MergeDuplicateGroup( func MergeDuplicateGroup(
ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, dataDir string, ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, dataDir string,
groupID, survivorID pgtype.UUID, groupID, survivorID pgtype.UUID,
) (MergeResult, error) {
return MergeDuplicateGroupGuarded(ctx, pool, logger, dataDir, groupID, survivorID, nil)
}
// MergeDuplicateGroupGuarded is MergeDuplicateGroup with a check on what it may
// remove: when removable says no to any copy that would go, the merge returns
// ErrCopyTrackedByLidarr (wrapped with the path) and changes nothing. A nil
// removable allows every copy.
func MergeDuplicateGroupGuarded(
ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, dataDir string,
groupID, survivorID pgtype.UUID, removable RemovableFunc,
) (MergeResult, error) { ) (MergeResult, error) {
if logger == nil { if logger == nil {
logger = slog.Default() logger = slog.Default()
@@ -130,13 +109,6 @@ func MergeDuplicateGroupGuarded(
return MergeResult{}, err return MergeResult{}, err
} }
if removable != nil {
for _, l := range losers {
if !removable(l.FilePath) {
return MergeResult{}, fmt.Errorf("%w: %s", ErrCopyTrackedByLidarr, l.FilePath)
}
}
}
for _, l := range losers { for _, l := range losers {
if err := removeTrackFileOnDisk(l.FilePath); err != nil { if err := removeTrackFileOnDisk(l.FilePath); err != nil {
return MergeResult{}, err return MergeResult{}, err
+85 -548
View File
@@ -2,12 +2,9 @@ package library
import ( import (
"context" "context"
"encoding/json"
"errors" "errors"
"fmt" "fmt"
"log/slog" "log/slog"
"sort"
"strings"
"time" "time"
"github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5"
@@ -16,100 +13,46 @@ import (
"git.fabledsword.com/bvandeusen/minstrel/internal/audit" "git.fabledsword.com/bvandeusen/minstrel/internal/audit"
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
"git.fabledsword.com/bvandeusen/minstrel/internal/lidarr"
"git.fabledsword.com/bvandeusen/minstrel/internal/notifications" "git.fabledsword.com/bvandeusen/minstrel/internal/notifications"
syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync" syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync"
) )
// Duplicate resolver (M498 #5435, #5437). // Duplicate resolver (M498 #5435, #5437; without Lidarr since M502 #5517).
// //
// The sweep proposes duplicate groups; the resolver decides what each one is // The sweep proposes duplicate groups; the resolver decides what each one is
// and settles the ones that are safe to settle without asking. Safety is // and settles the ones that are safe to settle without asking. Minstrel is the
// Lidarr's to define: Lidarr maps one file to each track of the release it // only program writing the library, so no copy is kept because something else
// monitors, and re-downloads any mapped file that disappears. So: // depends on it. What decides is what the copies are:
// //
// - A copy Lidarr maps is never removed. // - On one album, a song is held once: the extra copies are merged into the
// - On one album, copies Lidarr does not map are merged into the copy it does // best by ProposeSurvivor.
// (or, when it maps none, into the best by ProposeSurvivor). // - Across releases, each copy belongs to its own release (a single and its
// - When Lidarr maps every copy on one album, the release it monitors lists // album): nothing is removed, and the copies are linked as one song.
// the song twice: the Humanz box set, 14 vinyl sides then the digital // - Except that a video rip is no release's copy. When every copy but one is
// medium. The resolver moves Lidarr to a release of that album that lists // a rip, across releases or under another title, the rips are merged into
// each song once. Lidarr then rescans the folder, maps one copy and leaves // the one that is not.
// the rest unmapped, and the next pass merges them. // - Anything else waits for the operator.
//
// The release change has a fixed point (lesson #4183): it happens only while
// the monitored release repeats a title, and the release it chooses repeats
// none, so the next pass finds nothing to change.
// LidarrLibrary is the part of Lidarr's API the resolver uses. *lidarr.Client
// satisfies it.
type LidarrLibrary interface {
ListUnmappedTrackFiles(ctx context.Context) ([]lidarr.TrackFile, error)
LookupAlbumByMBID(ctx context.Context, mbid string) (lidarr.LidarrAlbum, error)
ListAlbumTracks(ctx context.Context, albumID int) ([]lidarr.ReleaseTrack, error)
GetAlbumReleases(ctx context.Context, albumID int) ([]lidarr.AlbumRelease, error)
ListReleaseTracks(ctx context.Context, releaseID int) ([]lidarr.ReleaseTrack, error)
SetMonitoredRelease(ctx context.Context, albumID, releaseID int) error
SearchAlbums(ctx context.Context, albumIDs []int) error
}
const ( const (
// resolveMergeCap bounds the merges one pass makes. The first pass after // resolveMergeCap bounds the merges one pass makes. The first pass after
// deploy meets the whole backlog (433 groups measured on 2026-10-09); a // deploy meets the whole backlog; a cap spreads it over a few hours, each
// cap spreads it over a few hours, each pass short. // pass short.
resolveMergeCap = 200 resolveMergeCap = 200
// resolveReleaseCap bounds the Lidarr release changes one pass makes. Each
// makes Lidarr rescan an artist folder.
resolveReleaseCap = 10
// releaseChangeSettle is how long after a release change the resolver
// leaves that album's groups alone. Lidarr unmaps every file of the album
// and rescans; until the rescan lands, every copy reads as unmapped, and a
// merge then could remove the very file Lidarr is about to map.
releaseChangeSettle = 24 * time.Hour
// duplicateResolveTick is how often the worker runs a pass. // duplicateResolveTick is how often the worker runs a pass.
duplicateResolveTick = time.Hour duplicateResolveTick = time.Hour
) )
const (
lidarrStateTracked = "tracked"
lidarrStateUnmapped = "unmapped"
)
// ReleaseChange is one Lidarr release the resolver switched.
type ReleaseChange struct {
AlbumTitle string
ArtistName string
From, To lidarr.AlbumRelease
}
// DuplicateResolveResult tallies one pass. // DuplicateResolveResult tallies one pass.
type DuplicateResolveResult struct { type DuplicateResolveResult struct {
Groups int Groups int
Classes map[DuplicateClass]int Classes map[DuplicateClass]int
LidarrConsulted bool
Merged int Merged int
MergeFailed int MergeFailed int
ReleaseChanges []ReleaseChange
// SongsLinked counts cross-release groups newly linked as one song. // SongsLinked counts cross-release groups newly linked as one song.
SongsLinked int SongsLinked int
// HeldBack counts tracks newly held back from radio and the mixes as video // HeldBack counts tracks newly held back from radio and the mixes as video
// rips; Released counts held-back tracks whose name no longer says so. // rips; Released counts held-back tracks whose name no longer says so.
HeldBack, Released int64 HeldBack, Released int64
// RipSearches counts albums Lidarr was asked to search for a better copy
// of a rip that has no clean copy (#5447).
RipSearches int
}
// LidarrPathKey is the part of a path Minstrel and Lidarr agree on: the last
// three components (artist folder, album folder, file). Both see the library
// under their own mount; on the operator's deploy both happen to say /music,
// and the key keeps the match from depending on that.
func LidarrPathKey(p string) string {
parts := strings.Split(strings.Trim(p, "/"), "/")
if len(parts) > 3 {
parts = parts[len(parts)-3:]
}
return strings.Join(parts, "/")
} }
// resolveGroup is one pending group and its present copies. // resolveGroup is one pending group and its present copies.
@@ -119,53 +62,52 @@ type resolveGroup struct {
members []dbq.ListDuplicateGroupsForResolveRow members []dbq.ListDuplicateGroupsForResolveRow
class DuplicateClass class DuplicateClass
note string note string
states []string // per member: tracked, unmapped, or "" when Lidarr was not asked
} }
// autoMergeable says whether the resolver may merge the group on its own: when // isRip says whether a copy is a video rip: held back as one, or named like
// every copy it would remove is one Lidarr does not map (D-a rule 1). On one // one and not kept by the operator. The name is read as well as the verdict
// album that is any group with at most one mapped copy. Across releases each // because a pass reads the groups before it flags new rips.
// mapped copy fulfils its own release (D-b), so only a group with exactly one func isRip(m dbq.ListDuplicateGroupsForResolveRow) bool {
// mapped copy qualifies: the others fulfil nothing — a rip beside the clean if m.SourceVerdict != nil {
// copy on another release, or a wrong-file import Lidarr holds unmapped return *m.SourceVerdict == "suspect"
// (#5439) — and the mapped copy is the clear one to keep. }
func autoMergeable(g *resolveGroup) bool { return len(SourceMarkersFor(m.FilePath)) > 0
}
// lone returns the index of the group's only copy that is not a rip, or -1
// when there is none or more than one.
func (g *resolveGroup) lone() int {
at := -1
for i, m := range g.members {
if isRip(m) {
continue
}
if at >= 0 {
return -1
}
at = i
}
return at
}
// mergePlan says whether the resolver may merge the group on its own, and into
// which copy (an index into members; -1 means ProposeSurvivor decides).
func mergePlan(g *resolveGroup) (ok bool, survivor int) {
switch g.class { switch g.class {
case ClassSameRelease: case ClassSameRelease:
return g.tracked() <= 1 return true, -1
case ClassCrossRelease, ClassMismatch: case ClassCrossRelease, ClassMismatch:
return g.tracked() == 1 if i := g.lone(); i >= 0 {
default: return true, i
return false
} }
} }
return false, -1
}
// settling says whether any of the group's albums had its Lidarr release // ResolveDuplicates runs one pass. act is the operator's auto-resolve setting;
// changed recently, so Lidarr may still be remapping its files. // with it off, the pass only classifies.
func (g *resolveGroup) settling(albums map[string]bool) bool {
for _, m := range g.members {
if albums[syncpkg.FormatUUID(m.AlbumID)] {
return true
}
}
return false
}
func (g *resolveGroup) tracked() int {
n := 0
for _, s := range g.states {
if s == lidarrStateTracked {
n++
}
}
return n
}
// ResolveDuplicates runs one pass. lid is nil when Lidarr is disabled; act is
// the operator's auto-resolve setting. Without Lidarr, or with act off, the
// pass only classifies: nothing is removed when Lidarr cannot say what is safe.
func ResolveDuplicates( func ResolveDuplicates(
ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, dataDir string, lid LidarrLibrary, act bool, ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, dataDir string, act bool,
) (DuplicateResolveResult, error) { ) (DuplicateResolveResult, error) {
if logger == nil { if logger == nil {
logger = slog.Default() logger = slog.Default()
@@ -180,32 +122,10 @@ func ResolveDuplicates(
groups := foldResolveGroups(rows) groups := foldResolveGroups(rows)
res.Groups = len(groups) res.Groups = len(groups)
var unmapped map[string]bool
if lid != nil {
files, err := lid.ListUnmappedTrackFiles(ctx)
if err != nil {
logger.Warn("duplicate resolve: Lidarr's unmapped files unavailable; classifying only", "err", err)
} else {
unmapped = make(map[string]bool, len(files))
for _, f := range files {
unmapped[LidarrPathKey(f.Path)] = true
}
res.LidarrConsulted = true
}
}
for _, g := range groups { for _, g := range groups {
cm := make([]ClassifyMember, len(g.members)) cm := make([]ClassifyMember, len(g.members))
g.states = make([]string, len(g.members))
for i, m := range g.members { for i, m := range g.members {
cm[i] = ClassifyMember{AlbumID: syncpkg.FormatUUID(m.AlbumID), Title: m.Title, ArtistName: m.ArtistName} cm[i] = ClassifyMember{AlbumID: syncpkg.FormatUUID(m.AlbumID), Title: m.Title, ArtistName: m.ArtistName}
if res.LidarrConsulted {
if unmapped[LidarrPathKey(m.FilePath)] {
g.states[i] = lidarrStateUnmapped
} else {
g.states[i] = lidarrStateTracked
}
}
} }
g.class = ClassifyDuplicateGroup(g.tier, cm) g.class = ClassifyDuplicateGroup(g.tier, cm)
g.note = resolveNote(g) g.note = resolveNote(g)
@@ -219,31 +139,15 @@ func ResolveDuplicates(
return res, nil return res, nil
} }
// Holding rips back and linking songs remove nothing and need nothing from
// Lidarr.
if res.HeldBack, err = q.FlagSuspectSourceTracks(ctx, SuspectSourcePattern); err != nil { if res.HeldBack, err = q.FlagSuspectSourceTracks(ctx, SuspectSourcePattern); err != nil {
return res, fmt.Errorf("hold back video rips: %w", err) return res, fmt.Errorf("hold back video rips: %w", err)
} }
if res.Released, err = q.ClearStaleSuspectSourceTracks(ctx, SuspectSourcePattern); err != nil { if res.Released, err = q.ClearStaleSuspectSourceTracks(ctx, SuspectSourcePattern); err != nil {
return res, fmt.Errorf("release renamed tracks: %w", err) return res, fmt.Errorf("release renamed tracks: %w", err)
} }
linked, err := linkCrossRelease(ctx, pool, logger, groups)
res.SongsLinked = linked
if err != nil {
return res, err
}
if !res.LidarrConsulted { // Merges before links, so a rip merged away is not linked first.
return res, nil merged := map[pgtype.UUID]bool{}
}
settling, err := recentlyChangedAlbums(ctx, q, time.Now())
if err != nil {
return res, err
}
// Merges first, on the state Lidarr reported at the start of the pass.
removable := func(p string) bool { return unmapped[LidarrPathKey(p)] }
for _, g := range groups { for _, g := range groups {
if res.Merged >= resolveMergeCap { if res.Merged >= resolveMergeCap {
break break
@@ -251,35 +155,29 @@ func ResolveDuplicates(
if ctx.Err() != nil { if ctx.Err() != nil {
return res, ctx.Err() return res, ctx.Err()
} }
if !autoMergeable(g) || g.settling(settling) { ok, survivor := mergePlan(g)
if !ok {
continue continue
} }
if err := autoMerge(ctx, pool, logger, dataDir, g, removable); err != nil { if err := autoMerge(ctx, pool, logger, dataDir, g, survivor); err != nil {
res.MergeFailed++ res.MergeFailed++
logger.Warn("duplicate resolve: merge failed", "group_id", syncpkg.FormatUUID(g.id), "err", err) logger.Warn("duplicate resolve: merge failed", "group_id", syncpkg.FormatUUID(g.id), "err", err)
continue continue
} }
merged[g.id] = true
res.Merged++ res.Merged++
} }
// Then release changes, for albums where Lidarr maps every copy. linked, err := linkCrossRelease(ctx, pool, logger, groups, merged)
changes, err := changeRepeatingReleases(ctx, q, pool, logger, lid, groups, settling) res.SongsLinked = linked
res.ReleaseChanges = changes
if err != nil { if err != nil {
return res, err return res, err
} }
// Last, rips with no clean copy anywhere: ask Lidarr for something better. if res.Merged > 0 {
searched, err := searchSoleCopyRips(ctx, q, pool, logger, lid)
res.RipSearches = searched
if err != nil {
logger.Warn("duplicate resolve: rip search failed", "err", err)
}
if res.Merged > 0 || len(changes) > 0 {
notifyAdmins(ctx, notifications.KindDuplicatesResolved, notifications.Payload{ notifyAdmins(ctx, notifications.KindDuplicatesResolved, notifications.Payload{
Count: int64(res.Merged + len(changes)), Count: int64(res.Merged),
Detail: resolvedDetail(res.Merged, changes), Detail: fmt.Sprintf("Merged %d duplicate %s.", res.Merged, pluralWord(res.Merged, "copy", "copies")),
}) })
} }
return res, nil return res, nil
@@ -287,13 +185,15 @@ func ResolveDuplicates(
// linkCrossRelease links each cross-release group's copies as one song // linkCrossRelease links each cross-release group's copies as one song
// (#5438): a like on one is a like on all, and the Liked list, shuffle and the // (#5438): a like on one is a like on all, and the Liked list, shuffle and the
// mixes count the song once. Linking an already-linked group changes nothing, // mixes count the song once. A group the pass merged has nothing left to link. Linking an already-linked group changes nothing,
// so this runs every pass; likes are shared only when a link is new, since a // so this runs every pass; likes are shared only when a link is new, since a
// like made after it reaches every copy as it is made. // like made after it reaches every copy as it is made.
func linkCrossRelease(ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, groups []*resolveGroup) (int, error) { func linkCrossRelease(
ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, groups []*resolveGroup, merged map[pgtype.UUID]bool,
) (int, error) {
linked := 0 linked := 0
for _, g := range groups { for _, g := range groups {
if g.class != ClassCrossRelease { if g.class != ClassCrossRelease || merged[g.id] {
continue continue
} }
if ctx.Err() != nil { if ctx.Err() != nil {
@@ -345,61 +245,6 @@ func linkSong(ctx context.Context, pool *pgxpool.Pool, ids []pgtype.UUID) (bool,
return true, tx.Commit(ctx) return true, tx.Commit(ctx)
} }
// ripSearchCap bounds the albums one pass asks Lidarr to search, so the first
// pass after deploy does not queue the whole backlog of rips at once.
const ripSearchCap = 10
// searchSoleCopyRips asks Lidarr to search each album holding a video rip that
// is the only copy of its song (#5447, operator's choice: option 2). While
// Lidarr maps the rip it counts the track fulfilled, so nothing else asks for
// better. An album search grabs only when the quality profile allows an
// upgrade, so this removes nothing and may change nothing: the rip stays held
// back either way. An album is searched at most once a week (the query reads
// the audit log), and an album Lidarr does not hold is skipped.
func searchSoleCopyRips(
ctx context.Context, q *dbq.Queries, pool *pgxpool.Pool, logger *slog.Logger, lid LidarrLibrary,
) (int, error) {
albums, err := q.ListAlbumsWithSoleCopyRips(ctx, ripSearchCap)
if err != nil {
return 0, fmt.Errorf("list albums with sole-copy rips: %w", err)
}
type found struct {
row dbq.ListAlbumsWithSoleCopyRipsRow
lidarrID int
}
var targets []found
for _, a := range albums {
la, err := lid.LookupAlbumByMBID(ctx, a.ReleaseGroupMbid)
if errors.Is(err, lidarr.ErrNotFound) {
continue
}
if err != nil {
return 0, fmt.Errorf("look up %s: %w", a.ReleaseGroupMbid, err)
}
targets = append(targets, found{row: a, lidarrID: la.ID})
}
if len(targets) == 0 {
return 0, nil
}
ids := make([]int, len(targets))
for i, t := range targets {
ids[i] = t.lidarrID
}
if err := lid.SearchAlbums(ctx, ids); err != nil {
return 0, fmt.Errorf("search albums: %w", err)
}
for _, t := range targets {
audit.WriteOrLog(ctx, pool, logger, pgtype.UUID{}, pgtype.UUID{}, audit.ActionLidarrRipSearch, map[string]any{
"album_id": syncpkg.FormatUUID(t.row.AlbumID),
"album_title": t.row.AlbumTitle,
"artist_name": t.row.ArtistName,
"lidarr_album_id": t.lidarrID,
"rips": t.row.Rips,
})
}
return len(targets), nil
}
func foldResolveGroups(rows []dbq.ListDuplicateGroupsForResolveRow) []*resolveGroup { func foldResolveGroups(rows []dbq.ListDuplicateGroupsForResolveRow) []*resolveGroup {
var groups []*resolveGroup var groups []*resolveGroup
for _, r := range rows { for _, r := range rows {
@@ -419,16 +264,14 @@ func foldResolveGroups(rows []dbq.ListDuplicateGroupsForResolveRow) []*resolveGr
return out return out
} }
// resolveNote is what the report says beside a group the resolver leaves for // resolveNote is what the report says beside a group, when there is something
// the operator, when there is something to say. // to say.
func resolveNote(g *resolveGroup) string { func resolveNote(g *resolveGroup) string {
switch g.class { switch g.class {
case ClassSameRelease:
if g.tracked() > 1 {
return "Lidarr tracks every copy as its own track, so removing one would make Lidarr download it again."
}
case ClassCrossRelease: case ClassCrossRelease:
return "Each copy belongs to its own release, and Lidarr keeps each one. Nothing is removed; they count as one song." if g.lone() < 0 {
return "Each copy belongs to its own release, so none is removed; they count as one song."
}
case ClassMismatch: case ClassMismatch:
return "Identical audio filed under different titles: one file carries another song's tags." return "Identical audio filed under different titles: one file carries another song's tags."
} }
@@ -437,16 +280,10 @@ func resolveNote(g *resolveGroup) string {
func writeResolveVerdicts(ctx context.Context, q *dbq.Queries, groups []*resolveGroup) error { func writeResolveVerdicts(ctx context.Context, q *dbq.Queries, groups []*resolveGroup) error {
classes := dbq.SetDuplicateGroupClassesParams{} classes := dbq.SetDuplicateGroupClassesParams{}
states := dbq.SetDuplicateMemberLidarrStatesParams{}
for _, g := range groups { for _, g := range groups {
classes.GroupIds = append(classes.GroupIds, g.id) classes.GroupIds = append(classes.GroupIds, g.id)
classes.Classes = append(classes.Classes, string(g.class)) classes.Classes = append(classes.Classes, string(g.class))
classes.Notes = append(classes.Notes, g.note) classes.Notes = append(classes.Notes, g.note)
for i, m := range g.members {
states.GroupIds = append(states.GroupIds, g.id)
states.TrackIds = append(states.TrackIds, m.TrackID)
states.States = append(states.States, g.states[i])
}
} }
if len(classes.GroupIds) == 0 { if len(classes.GroupIds) == 0 {
return nil return nil
@@ -454,9 +291,6 @@ func writeResolveVerdicts(ctx context.Context, q *dbq.Queries, groups []*resolve
if err := q.SetDuplicateGroupClasses(ctx, classes); err != nil { if err := q.SetDuplicateGroupClasses(ctx, classes); err != nil {
return fmt.Errorf("write duplicate classes: %w", err) return fmt.Errorf("write duplicate classes: %w", err)
} }
if err := q.SetDuplicateMemberLidarrStates(ctx, states); err != nil {
return fmt.Errorf("write lidarr states: %w", err)
}
return nil return nil
} }
@@ -469,25 +303,27 @@ func survivorCandidates(g *resolveGroup) []SurvivorCandidate {
FileFormat: m.FileFormat, FileFormat: m.FileFormat,
FileSize: m.FileSize, FileSize: m.FileSize,
AddedAt: m.AddedAt.Time, AddedAt: m.AddedAt.Time,
LidarrTracked: g.states[i] == lidarrStateTracked,
TagFit: TagFitScore(m.TrackNumber != nil, m.PositionClash, m.FilePath, m.HasMbid), TagFit: TagFitScore(m.TrackNumber != nil, m.PositionClash, m.FilePath, m.HasMbid),
} }
} }
return cands return cands
} }
// autoMerge merges one group into the copy ProposeSurvivor picks, which is the // autoMerge merges one group into the copy at index survivor, or the copy
// copy Lidarr tracks when there is one. removable is the guard: a copy Lidarr // ProposeSurvivor picks when survivor is -1, and records the merge as the
// maps can never be among those removed, whatever the survivor rule said. // resolver's.
func autoMerge( func autoMerge(
ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, dataDir string, g *resolveGroup, removable RemovableFunc, ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, dataDir string, g *resolveGroup, survivor int,
) error { ) error {
survivorKey, reason := ProposeSurvivor(survivorCandidates(g)) survivorKey, reason := ProposeSurvivor(survivorCandidates(g))
if survivor >= 0 {
survivorKey, reason = syncpkg.FormatUUID(g.members[survivor].TrackID), "the copy that is not a video rip"
}
var survivorID pgtype.UUID var survivorID pgtype.UUID
if err := survivorID.Scan(survivorKey); err != nil { if err := survivorID.Scan(survivorKey); err != nil {
return fmt.Errorf("parse survivor id: %w", err) return fmt.Errorf("parse survivor id: %w", err)
} }
res, err := MergeDuplicateGroupGuarded(ctx, pool, logger, dataDir, g.id, survivorID, removable) res, err := MergeDuplicateGroup(ctx, pool, logger, dataDir, g.id, survivorID)
if err != nil { if err != nil {
return err return err
} }
@@ -515,294 +351,6 @@ func autoMerge(
return nil return nil
} }
// recentlyChangedAlbums is the Minstrel albums whose Lidarr release the
// resolver changed within releaseChangeSettle, read from the audit log so a
// restart does not forget them.
func recentlyChangedAlbums(ctx context.Context, q *dbq.Queries, now time.Time) (map[string]bool, error) {
rows, err := q.ListAuditLogByActions(ctx, dbq.ListAuditLogByActionsParams{
Actions: []string{string(audit.ActionLidarrReleaseChange)}, SystemOnly: true,
PageLimit: 200, PageOffset: 0,
})
if err != nil {
return nil, fmt.Errorf("read recent release changes: %w", err)
}
out := map[string]bool{}
for _, r := range rows {
if now.Sub(r.CreatedAt.Time) > releaseChangeSettle {
break // newest first
}
var meta struct {
AlbumID string `json:"album_id"`
}
if json.Unmarshal(r.Metadata, &meta) == nil && meta.AlbumID != "" {
out[meta.AlbumID] = true
}
}
return out, nil
}
// repeatingAlbum is a Minstrel album where Lidarr maps more than one copy of a
// song, and the groups that showed it.
type repeatingAlbum struct {
albumID pgtype.UUID
releaseGroupMbid string
title, artist string
groups []*resolveGroup
}
func changeRepeatingReleases(
ctx context.Context, q *dbq.Queries, pool *pgxpool.Pool, logger *slog.Logger,
lid LidarrLibrary, groups []*resolveGroup, settling map[string]bool,
) ([]ReleaseChange, error) {
albums := map[string]*repeatingAlbum{}
for _, g := range groups {
if g.class != ClassSameRelease || g.tracked() < 2 {
continue
}
m := g.members[0]
key := syncpkg.FormatUUID(m.AlbumID)
if settling[key] || m.ReleaseGroupMbid == nil || *m.ReleaseGroupMbid == "" {
continue
}
a := albums[key]
if a == nil {
a = &repeatingAlbum{albumID: m.AlbumID, releaseGroupMbid: *m.ReleaseGroupMbid, title: m.AlbumTitle, artist: m.ArtistName}
albums[key] = a
}
a.groups = append(a.groups, g)
}
keys := make([]string, 0, len(albums))
for k := range albums {
keys = append(keys, k)
}
sort.Strings(keys)
var changes []ReleaseChange
for _, k := range keys {
if len(changes) >= resolveReleaseCap {
break
}
if ctx.Err() != nil {
return changes, ctx.Err()
}
a := albums[k]
change, note, err := changeRelease(ctx, q, lid, a)
if err != nil {
logger.Warn("duplicate resolve: Lidarr release check failed", "album", a.title, "err", err)
continue
}
if change != nil {
changes = append(changes, *change)
audit.WriteOrLog(ctx, pool, logger, pgtype.UUID{}, pgtype.UUID{}, audit.ActionLidarrReleaseChange, map[string]any{
"automatic": true,
"album_id": k,
"album_title": a.title,
"artist_name": a.artist,
"from_release": releaseLabel(change.From),
"to_release": releaseLabel(change.To),
})
}
if note != "" {
if err := setGroupNotes(ctx, q, a.groups, note); err != nil {
return changes, err
}
}
}
return changes, nil
}
// changeRelease checks one album's monitored release and, when it lists a
// song twice, moves Lidarr to the release that lists each song once and best
// covers what is on disk. It returns the change made (nil when none) and the
// note for the album's groups.
func changeRelease(
ctx context.Context, q *dbq.Queries, lid LidarrLibrary, a *repeatingAlbum,
) (*ReleaseChange, string, error) {
la, err := lid.LookupAlbumByMBID(ctx, a.releaseGroupMbid)
if errors.Is(err, lidarr.ErrNotFound) {
return nil, "Lidarr does not hold this album, so its copies are left for you.", nil
}
if err != nil {
return nil, "", err
}
current, err := lid.ListAlbumTracks(ctx, la.ID)
if err != nil {
return nil, "", err
}
if !releaseRepeats(current) {
return nil, "Lidarr maps each copy to a different track of this album, so none can go without a download.", nil
}
releases, err := lid.GetAlbumReleases(ctx, la.ID)
if err != nil {
return nil, "", err
}
titles, err := q.ListAlbumPresentTrackTitles(ctx, a.albumID)
if err != nil {
return nil, "", fmt.Errorf("list album titles: %w", err)
}
onDisk := map[string]bool{}
for _, t := range titles {
if k := ReleaseTitleKey(t); k != "" {
onDisk[k] = true
}
}
var from lidarr.AlbumRelease
var candidates []releaseCandidate
for _, r := range releases {
if r.Monitored {
from = r
continue
}
tracks, err := lid.ListReleaseTracks(ctx, r.ID)
if err != nil {
return nil, "", err
}
if len(tracks) == 0 || releaseRepeats(tracks) {
continue
}
candidates = append(candidates, releaseCandidate{release: r, coverage: coverage(tracks, onDisk)})
}
best, ok := pickRelease(candidates, coverage(current, onDisk))
if !ok {
return nil, "Lidarr's release of this album lists songs twice, and no other release lists each once while keeping what is on disk.", nil
}
if err := lid.SetMonitoredRelease(ctx, la.ID, best.ID); err != nil {
// Lidarr's album update unlinks the album's files and queues the rescan
// before it answers, and on a large album that outlasts the client's
// timeout while the change still lands (Cracker Island on the first
// deploy pass). An unanswered PUT is read back: when the release did
// change, it is recorded like any other, so the album gets its settle
// window and the report shows it.
if !errors.Is(err, lidarr.ErrUnreachable) || !releaseMonitored(ctx, lid, la.ID, best.ID) {
return nil, "", err
}
}
note := fmt.Sprintf("Lidarr now monitors the %s release, which lists each song once. The extra copies are merged once Lidarr has rescanned.", releaseLabel(best))
return &ReleaseChange{AlbumTitle: a.title, ArtistName: a.artist, From: from, To: best}, note, nil
}
// releaseMonitored reads whether the album's monitored release is now
// releaseID. A failed read answers false: the change is not assumed.
func releaseMonitored(ctx context.Context, lid LidarrLibrary, albumID, releaseID int) bool {
releases, err := lid.GetAlbumReleases(ctx, albumID)
if err != nil {
return false
}
for _, r := range releases {
if r.ID == releaseID {
return r.Monitored
}
}
return false
}
type releaseCandidate struct {
release lidarr.AlbumRelease
coverage int
}
// pickRelease chooses among releases that list each song once: the one
// covering most of the titles on disk, then the fewest tracks (the plainest
// edition that holds them), then a digital one, then the lowest id so the
// choice is stable. It refuses a release covering fewer titles on disk than
// the current one does: the change must not drop a song Lidarr now keeps.
func pickRelease(cands []releaseCandidate, currentCoverage int) (lidarr.AlbumRelease, bool) {
if len(cands) == 0 {
return lidarr.AlbumRelease{}, false
}
sort.SliceStable(cands, func(i, j int) bool {
a, b := cands[i], cands[j]
if a.coverage != b.coverage {
return a.coverage > b.coverage
}
if a.release.TrackCount != b.release.TrackCount {
return a.release.TrackCount < b.release.TrackCount
}
if da, db := isDigital(a.release), isDigital(b.release); da != db {
return da
}
return a.release.ID < b.release.ID
})
if cands[0].coverage < currentCoverage {
return lidarr.AlbumRelease{}, false
}
return cands[0].release, true
}
func isDigital(r lidarr.AlbumRelease) bool {
return strings.Contains(strings.ToLower(r.Format), "digital")
}
// releaseRepeats reports whether a release lists one title more than once.
func releaseRepeats(tracks []lidarr.ReleaseTrack) bool {
seen := map[string]bool{}
for _, t := range tracks {
k := ReleaseTitleKey(t.Title)
if k == "" {
continue
}
if seen[k] {
return true
}
seen[k] = true
}
return false
}
// coverage counts the distinct titles on disk that the release lists.
func coverage(tracks []lidarr.ReleaseTrack, onDisk map[string]bool) int {
hit := map[string]bool{}
for _, t := range tracks {
if k := ReleaseTitleKey(t.Title); onDisk[k] {
hit[k] = true
}
}
return len(hit)
}
func releaseLabel(r lidarr.AlbumRelease) string {
label := r.Format
if label == "" {
label = r.Title
}
if r.Disambiguation != "" {
label = r.Disambiguation + ", " + label
}
return fmt.Sprintf("%s (%d tracks)", label, r.TrackCount)
}
func setGroupNotes(ctx context.Context, q *dbq.Queries, groups []*resolveGroup, note string) error {
p := dbq.SetDuplicateGroupClassesParams{}
for _, g := range groups {
g.note = note
p.GroupIds = append(p.GroupIds, g.id)
p.Classes = append(p.Classes, string(g.class))
p.Notes = append(p.Notes, note)
}
if err := q.SetDuplicateGroupClasses(ctx, p); err != nil {
return fmt.Errorf("write duplicate notes: %w", err)
}
return nil
}
func resolvedDetail(merged int, changes []ReleaseChange) string {
var parts []string
if merged > 0 {
parts = append(parts, fmt.Sprintf("Merged %d %s Lidarr does not need.", merged, pluralWord(merged, "copy", "copies")))
}
switch len(changes) {
case 0:
case 1:
c := changes[0]
parts = append(parts, fmt.Sprintf("Lidarr now monitors the %s release of %s by %s, which lists each song once.",
releaseLabel(c.To), c.AlbumTitle, c.ArtistName))
default:
parts = append(parts, fmt.Sprintf("Lidarr now monitors a release that lists each song once for %d albums.", len(changes)))
}
return strings.Join(parts, " ")
}
func pluralWord(n int, one, many string) string { func pluralWord(n int, one, many string) string {
if n == 1 { if n == 1 {
return one return one
@@ -816,20 +364,14 @@ type DuplicateResolveWorker struct {
logger *slog.Logger logger *slog.Logger
dataDir string dataDir string
settings *FingerprintSettingsService settings *FingerprintSettingsService
lidarr func() LidarrLibrary
tick time.Duration tick time.Duration
} }
// NewDuplicateResolveWorker builds a worker with the production cadence. // NewDuplicateResolveWorker builds a worker with the production cadence.
// lidarrFn is asked each pass, so a Lidarr setting saved in admin takes effect
// without a restart; it returns nil while Lidarr is disabled.
func NewDuplicateResolveWorker( func NewDuplicateResolveWorker(
pool *pgxpool.Pool, logger *slog.Logger, dataDir string, pool *pgxpool.Pool, logger *slog.Logger, dataDir string, settings *FingerprintSettingsService,
settings *FingerprintSettingsService, lidarrFn func() LidarrLibrary,
) *DuplicateResolveWorker { ) *DuplicateResolveWorker {
return &DuplicateResolveWorker{ return &DuplicateResolveWorker{pool: pool, logger: logger, dataDir: dataDir, settings: settings, tick: duplicateResolveTick}
pool: pool, logger: logger, dataDir: dataDir, settings: settings, lidarr: lidarrFn, tick: duplicateResolveTick,
}
} }
// Run blocks until ctx is cancelled, running a pass at start and then each tick. // Run blocks until ctx is cancelled, running a pass at start and then each tick.
@@ -865,16 +407,11 @@ func (w *DuplicateResolveWorker) tickOnce(ctx context.Context) {
} }
return return
} }
var lid LidarrLibrary res, err := ResolveDuplicates(ctx, w.pool, w.logger, w.dataDir, w.settings.Get().AutoResolve)
if w.lidarr != nil {
lid = w.lidarr()
}
res, err := ResolveDuplicates(ctx, w.pool, w.logger, w.dataDir, lid, w.settings.Get().AutoResolve)
if err != nil && ctx.Err() == nil { if err != nil && ctx.Err() == nil {
w.logger.Warn("duplicate resolve: pass failed", "err", err) w.logger.Warn("duplicate resolve: pass failed", "err", err)
} }
w.logger.Info("duplicate resolve complete", w.logger.Info("duplicate resolve complete",
"groups", res.Groups, "lidarr", res.LidarrConsulted, "merged", res.Merged, "groups", res.Groups, "merged", res.Merged, "merge_failed", res.MergeFailed,
"merge_failed", res.MergeFailed, "release_changes", len(res.ReleaseChanges), "songs_linked", res.SongsLinked, "songs_linked", res.SongsLinked, "held_back", res.HeldBack, "released", res.Released)
"held_back", res.HeldBack, "released", res.Released, "rip_searches", res.RipSearches)
} }
+86 -382
View File
@@ -2,8 +2,6 @@ package library
import ( import (
"context" "context"
"errors"
"fmt"
"os" "os"
"path/filepath" "path/filepath"
"testing" "testing"
@@ -12,128 +10,9 @@ import (
"github.com/jackc/pgx/v5/pgxpool" "github.com/jackc/pgx/v5/pgxpool"
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
"git.fabledsword.com/bvandeusen/minstrel/internal/lidarr"
syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync" syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync"
) )
func TestLidarrPathKey(t *testing.T) {
// Minstrel and Lidarr see the library under different mounts; the key is
// what they agree on.
a := LidarrPathKey("/music/Gorillaz/Humanz (2017)/Gorillaz - Humanz - 01 - Ascension.mp3")
b := LidarrPathKey("/data/media/music/Gorillaz/Humanz (2017)/Gorillaz - Humanz - 01 - Ascension.mp3")
if a != b || a != "Gorillaz/Humanz (2017)/Gorillaz - Humanz - 01 - Ascension.mp3" {
t.Errorf("keys differ or are wrong: %q, %q", a, b)
}
if LidarrPathKey("/music/Gorillaz/Humanz (2017)/a.mp3") == LidarrPathKey("/music/Gorillaz/Demon Days (2005)/a.mp3") {
t.Error("two albums' files share a key")
}
}
func rt(titles ...string) []lidarr.ReleaseTrack {
out := make([]lidarr.ReleaseTrack, len(titles))
for i, title := range titles {
out[i] = lidarr.ReleaseTrack{ID: i + 1, Title: title}
}
return out
}
func TestReleaseRepeats(t *testing.T) {
if !releaseRepeats(rt("Ascension", "Strobelite", "Ascension")) {
t.Error("the box set lists Ascension twice, but no repeat was found")
}
if releaseRepeats(rt("Charger", "Charger (demo)")) {
t.Error("a demo was read as a repeat of the song")
}
}
// The Humanz releases as Lidarr lists them (2026-10-09): the box set is
// monitored, and the 40-track digital release is the one that lists each song
// once and covers what is on disk.
func TestPickRelease(t *testing.T) {
digital40 := lidarr.AlbumRelease{ID: 10149, Format: "Digital Media", TrackCount: 40}
deluxe26 := lidarr.AlbumRelease{ID: 10150, Format: "CD", TrackCount: 26}
standard20 := lidarr.AlbumRelease{ID: 10151, Format: "Digital Media", TrackCount: 20}
got, ok := pickRelease([]releaseCandidate{
{release: standard20, coverage: 20}, {release: deluxe26, coverage: 26}, {release: digital40, coverage: 34},
}, 34)
if !ok || got.ID != digital40.ID {
t.Errorf("picked %+v (ok=%v), want the 40-track digital release", got, ok)
}
// Equal coverage: the plainer edition, then digital.
cd := lidarr.AlbumRelease{ID: 1, Format: "CD", TrackCount: 26}
web := lidarr.AlbumRelease{ID: 2, Format: "Digital Media", TrackCount: 26}
if got, _ := pickRelease([]releaseCandidate{{release: cd, coverage: 26}, {release: web, coverage: 26}}, 26); got.ID != web.ID {
t.Errorf("picked %+v, want the digital one of two equal releases", got)
}
// Never a release that keeps fewer of the songs on disk than the current one.
if got, ok := pickRelease([]releaseCandidate{{release: deluxe26, coverage: 26}}, 34); ok {
t.Errorf("picked %+v, which covers fewer songs on disk than the current release", got)
}
}
// fakeLidarr is one Lidarr album with several releases. Switching the
// monitored release changes what ListAlbumTracks answers, as in Lidarr.
type fakeLidarr struct {
unmapped []string
unmappedErr error
album lidarr.LidarrAlbum
releases []lidarr.AlbumRelease
releaseTracks map[int][]lidarr.ReleaseTrack
setCalls [][2]int
searchCalls [][]int
// setErr is what SetMonitoredRelease answers after applying the change:
// Lidarr timing out on a change that landed.
setErr error
}
func (f *fakeLidarr) ListUnmappedTrackFiles(context.Context) ([]lidarr.TrackFile, error) {
out := make([]lidarr.TrackFile, len(f.unmapped))
for i, p := range f.unmapped {
out[i] = lidarr.TrackFile{ID: i + 1, Path: p}
}
return out, f.unmappedErr
}
func (f *fakeLidarr) LookupAlbumByMBID(_ context.Context, mbid string) (lidarr.LidarrAlbum, error) {
if f.album.ForeignAlbumID != mbid {
return lidarr.LidarrAlbum{}, lidarr.ErrNotFound
}
return f.album, nil
}
func (f *fakeLidarr) ListAlbumTracks(context.Context, int) ([]lidarr.ReleaseTrack, error) {
for _, r := range f.releases {
if r.Monitored {
return f.releaseTracks[r.ID], nil
}
}
return nil, nil
}
func (f *fakeLidarr) GetAlbumReleases(context.Context, int) ([]lidarr.AlbumRelease, error) {
return append([]lidarr.AlbumRelease(nil), f.releases...), nil
}
func (f *fakeLidarr) ListReleaseTracks(_ context.Context, releaseID int) ([]lidarr.ReleaseTrack, error) {
return f.releaseTracks[releaseID], nil
}
func (f *fakeLidarr) SearchAlbums(_ context.Context, albumIDs []int) error {
f.searchCalls = append(f.searchCalls, albumIDs)
return nil
}
func (f *fakeLidarr) SetMonitoredRelease(_ context.Context, albumID, releaseID int) error {
f.setCalls = append(f.setCalls, [2]int{albumID, releaseID})
for i := range f.releases {
f.releases[i].Monitored = f.releases[i].ID == releaseID
}
return f.setErr
}
// resolveFixture is the Humanz shape: one album, one song twice, both copies on // resolveFixture is the Humanz shape: one album, one song twice, both copies on
// disk under an artist/album folder, in one pending exact-tier group. // disk under an artist/album folder, in one pending exact-tier group.
type resolveFixture struct { type resolveFixture struct {
@@ -178,7 +57,6 @@ func newResolveFixture(t *testing.T) resolveFixture {
} }
} }
exec(`UPDATE tracks SET title = 'Saturnz Barz' WHERE id = $1`, f.clean.ID) exec(`UPDATE tracks SET title = 'Saturnz Barz' WHERE id = $1`, f.clean.ID)
exec(`UPDATE albums SET release_group_mbid = 'rg-humanz' WHERE id = $1`, album.ID)
if err := pool.QueryRow(ctx, if err := pool.QueryRow(ctx,
`INSERT INTO duplicate_groups (member_key, tier) VALUES ('resolve-fixture', 'exact') RETURNING id`, `INSERT INTO duplicate_groups (member_key, tier) VALUES ('resolve-fixture', 'exact') RETURNING id`,
).Scan(&f.groupID); err != nil { ).Scan(&f.groupID); err != nil {
@@ -188,9 +66,6 @@ func newResolveFixture(t *testing.T) resolveFixture {
return f return f
} }
// lidarrPath is where Lidarr, mounted elsewhere, sees a fixture file.
func lidarrPath(p string) string { return "/music/" + LidarrPathKey(p) }
func (f resolveFixture) group(t *testing.T) (status string, class *string, note *string, auto bool) { func (f resolveFixture) group(t *testing.T) (status string, class *string, note *string, auto bool) {
t.Helper() t.Helper()
if err := f.pool.QueryRow(context.Background(), if err := f.pool.QueryRow(context.Background(),
@@ -203,18 +78,32 @@ func (f resolveFixture) group(t *testing.T) (status string, class *string, note
func exists(p string) bool { _, err := os.Stat(p); return err == nil } func exists(p string) bool { _, err := os.Stat(p); return err == nil }
// Lidarr maps the clean copy and holds the rip unmapped: the rip goes, the // autoMergeAudit reads the resolver's audit row for a group: how many there
// clean copy stays, and the merge is recorded as the resolver's. // are, and the reason the last one gave for its survivor.
func TestResolveDuplicates_MergesTheUnmappedCopy_Integration(t *testing.T) { func autoMergeAudit(t *testing.T, pool *pgxpool.Pool, groupID pgtype.UUID) (n int, reason string) {
f := newResolveFixture(t) t.Helper()
lid := &fakeLidarr{unmapped: []string{lidarrPath(f.ripPath)}} if err := pool.QueryRow(context.Background(),
`SELECT count(*), coalesce(max(metadata->>'survivor_reason'), '') FROM audit_log
WHERE action = 'duplicate_merge' AND actor_id IS NULL
AND metadata->>'group_id' = $1 AND (metadata->>'automatic')::boolean`,
syncpkg.FormatUUID(groupID)).Scan(&n, &reason); err != nil {
t.Fatal(err)
}
return n, reason
}
res, err := ResolveDuplicates(context.Background(), f.pool, nil, "", lid, true) // One album holding a song twice: the extra copy goes, the copy whose name and
// tags fit the album stays, and the merge is recorded as the resolver's. No
// Lidarr is asked (M502 #5517): Minstrel is the only program writing the
// library.
func TestResolveDuplicates_MergesASameReleaseCopy_Integration(t *testing.T) {
f := newResolveFixture(t)
res, err := ResolveDuplicates(context.Background(), f.pool, nil, "", true)
if err != nil { if err != nil {
t.Fatalf("resolve: %v", err) t.Fatalf("resolve: %v", err)
} }
if res.Merged != 1 || !res.LidarrConsulted { if res.Merged != 1 || res.Classes[ClassSameRelease] != 1 {
t.Fatalf("result = %+v, want one merge with Lidarr consulted", res) t.Fatalf("result = %+v, want the same-release group merged", res)
} }
if exists(f.ripPath) || !exists(f.cleanPath) { if exists(f.ripPath) || !exists(f.cleanPath) {
t.Errorf("rip exists=%v clean exists=%v; want only the clean copy left", exists(f.ripPath), exists(f.cleanPath)) t.Errorf("rip exists=%v clean exists=%v; want only the clean copy left", exists(f.ripPath), exists(f.cleanPath))
@@ -223,110 +112,15 @@ func TestResolveDuplicates_MergesTheUnmappedCopy_Integration(t *testing.T) {
if status != "merged" || class == nil || *class != string(ClassSameRelease) || !auto { if status != "merged" || class == nil || *class != string(ClassSameRelease) || !auto {
t.Errorf("group = (%s, %v, auto=%v), want merged same_release by the resolver", status, class, auto) t.Errorf("group = (%s, %v, auto=%v), want merged same_release by the resolver", status, class, auto)
} }
var audits int if n, _ := autoMergeAudit(t, f.pool, f.groupID); n != 1 {
if err := f.pool.QueryRow(context.Background(), t.Errorf("automatic merge audit rows = %d, want 1", n)
`SELECT count(*) FROM audit_log WHERE action = 'duplicate_merge' AND actor_id IS NULL
AND metadata->>'group_id' = $1 AND (metadata->>'automatic')::boolean`,
syncpkg.FormatUUID(f.groupID)).Scan(&audits); err != nil {
t.Fatal(err)
}
if audits != 1 {
t.Errorf("automatic merge audit rows = %d, want 1", audits)
} }
} }
// Lidarr maps both copies, and its release lists the song twice: nothing is // With auto-resolve off, the pass classifies and removes nothing.
// deleted, the release moves to the one listing each song once, and the next func TestResolveDuplicates_ClassifiesOnlyWithoutPermission_Integration(t *testing.T) {
// pass leaves it there (lesson #4183's fixed point).
func TestResolveDuplicates_ChangesARepeatingRelease_Integration(t *testing.T) {
f := newResolveFixture(t) f := newResolveFixture(t)
lid := &fakeLidarr{ res, err := ResolveDuplicates(context.Background(), f.pool, nil, "", false)
album: lidarr.LidarrAlbum{ID: 4723, ForeignAlbumID: "rg-humanz", Title: "Humanz"},
releases: []lidarr.AlbumRelease{
{ID: 10148, Format: "14x12\" Vinyl, Digital Media", TrackCount: 4, Monitored: true},
{ID: 10149, Format: "Digital Media", TrackCount: 2},
},
releaseTracks: map[int][]lidarr.ReleaseTrack{
10148: rt("Saturnz Barz", "Ascension", "Saturnz Barz", "Ascension"),
10149: rt("Saturnz Barz", "Ascension"),
},
}
res, err := ResolveDuplicates(context.Background(), f.pool, nil, "", lid, true)
if err != nil {
t.Fatalf("resolve: %v", err)
}
if res.Merged != 0 || !exists(f.ripPath) || !exists(f.cleanPath) {
t.Fatalf("merged %d; a copy Lidarr maps was touched", res.Merged)
}
if len(lid.setCalls) != 1 || lid.setCalls[0] != [2]int{4723, 10149} {
t.Fatalf("release changes = %v, want album 4723 to release 10149", lid.setCalls)
}
status, _, note, _ := f.group(t)
if status != "pending" || note == nil || *note == "" {
t.Errorf("group = (%s, note %v), want pending with a note saying what changed", status, note)
}
// The next pass: Lidarr has not rescanned yet, so both copies still read
// as tracked. The album is settling, so nothing changes.
if _, err := ResolveDuplicates(context.Background(), f.pool, nil, "", lid, true); err != nil {
t.Fatalf("second resolve: %v", err)
}
if len(lid.setCalls) != 1 {
t.Errorf("release changed again: %v", lid.setCalls)
}
// And once it has settled, the check itself finds nothing to do: the
// release it chose lists each song once.
change, _, err := changeRelease(context.Background(), dbq.New(f.pool), lid,
&repeatingAlbum{albumID: f.clean.AlbumID, releaseGroupMbid: "rg-humanz"})
if err != nil || change != nil || len(lid.setCalls) != 1 {
t.Errorf("after the change, changeRelease = (%+v, %v) with %d calls; want no change", change, err, len(lid.setCalls))
}
}
// After a release change Lidarr unmaps every file of the album until it has
// rescanned. A pass in that window must not read "every copy unmapped" as
// licence to merge: the copy it would remove may be the one Lidarr maps next.
func TestResolveDuplicates_LeavesAnAlbumAloneWhileLidarrRescans_Integration(t *testing.T) {
f := newResolveFixture(t)
lid := &fakeLidarr{
album: lidarr.LidarrAlbum{ID: 4723, ForeignAlbumID: "rg-humanz"},
releases: []lidarr.AlbumRelease{
{ID: 10148, TrackCount: 2, Monitored: true}, {ID: 10149, TrackCount: 1},
},
releaseTracks: map[int][]lidarr.ReleaseTrack{10148: rt("Saturnz Barz", "Saturnz Barz"), 10149: rt("Saturnz Barz")},
}
if _, err := ResolveDuplicates(context.Background(), f.pool, nil, "", lid, true); err != nil {
t.Fatal(err)
}
lid.unmapped = []string{lidarrPath(f.cleanPath), lidarrPath(f.ripPath)}
res, err := ResolveDuplicates(context.Background(), f.pool, nil, "", lid, true)
if err != nil {
t.Fatal(err)
}
if res.Merged != 0 || !exists(f.ripPath) || !exists(f.cleanPath) {
t.Errorf("merged %d during the rescan window", res.Merged)
}
}
// Without Lidarr, or with auto-resolve off, the pass classifies and removes
// nothing.
func TestResolveDuplicates_ClassifiesOnlyWithoutLidarrOrPermission_Integration(t *testing.T) {
for name, run := range map[string]func(resolveFixture) (DuplicateResolveResult, error){
"lidarr disabled": func(f resolveFixture) (DuplicateResolveResult, error) {
return ResolveDuplicates(context.Background(), f.pool, nil, "", nil, true)
},
"lidarr unreachable": func(f resolveFixture) (DuplicateResolveResult, error) {
return ResolveDuplicates(context.Background(), f.pool, nil, "", &fakeLidarr{unmappedErr: errors.New("timeout")}, true)
},
"auto-resolve off": func(f resolveFixture) (DuplicateResolveResult, error) {
lid := &fakeLidarr{unmapped: []string{lidarrPath(f.ripPath)}}
return ResolveDuplicates(context.Background(), f.pool, nil, "", lid, false)
},
} {
t.Run(name, func(t *testing.T) {
f := newResolveFixture(t)
res, err := run(f)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
@@ -337,25 +131,6 @@ func TestResolveDuplicates_ClassifiesOnlyWithoutLidarrOrPermission_Integration(t
if status != "pending" || class == nil || *class != string(ClassSameRelease) { if status != "pending" || class == nil || *class != string(ClassSameRelease) {
t.Errorf("group = (%s, %v), want pending and classified", status, class) t.Errorf("group = (%s, %v), want pending and classified", status, class)
} }
})
}
}
// The guard itself: a merge asked to remove a copy the check refuses changes
// nothing, file or row.
func TestMergeDuplicateGroupGuarded_RefusesATrackedCopy_Integration(t *testing.T) {
f := newResolveFixture(t)
refuse := func(p string) bool { return p != f.ripPath }
_, err := MergeDuplicateGroupGuarded(context.Background(), f.pool, nil, "", f.groupID, f.clean.ID, refuse)
if !errors.Is(err, ErrCopyTrackedByLidarr) {
t.Fatalf("err = %v, want ErrCopyTrackedByLidarr", err)
}
if !exists(f.ripPath) {
t.Error("the refused copy's file was removed")
}
if status, _, _, _ := f.group(t); status != "pending" {
t.Errorf("group status = %s, want pending", status)
}
} }
// crossReleaseRipFixture is a video rip on one release with the clean copy on // crossReleaseRipFixture is a video rip on one release with the clean copy on
@@ -416,12 +191,12 @@ func newCrossReleaseRipFixture(t *testing.T) crossReleaseRipFixture {
return f return f
} }
// The rip fulfils nothing in Lidarr while the clean copy on the album does: // A video rip on one release with the clean copy on another: the rip is no
// the rip goes and the album copy stays (#5439, D-a rule 1). // release's copy, so it goes and the album cut stays (#5439, now without
func TestResolveDuplicates_MergesAnUnmappedRipAcrossReleases_Integration(t *testing.T) { // Lidarr).
func TestResolveDuplicates_MergesARipAcrossReleases_Integration(t *testing.T) {
f := newCrossReleaseRipFixture(t) f := newCrossReleaseRipFixture(t)
lid := &fakeLidarr{unmapped: []string{lidarrPath(f.ripPath)}} res, err := ResolveDuplicates(context.Background(), f.pool, nil, "", true)
res, err := ResolveDuplicates(context.Background(), f.pool, nil, "", lid, true)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
@@ -431,13 +206,31 @@ func TestResolveDuplicates_MergesAnUnmappedRipAcrossReleases_Integration(t *test
if exists(f.ripPath) || !exists(f.cleanPath) { if exists(f.ripPath) || !exists(f.cleanPath) {
t.Errorf("rip exists %v, clean exists %v; want only the clean copy", exists(f.ripPath), exists(f.cleanPath)) t.Errorf("rip exists %v, clean exists %v; want only the clean copy", exists(f.ripPath), exists(f.cleanPath))
} }
if _, reason := autoMergeAudit(t, f.pool, f.groupID); reason != "the copy that is not a video rip" {
t.Errorf("survivor reason %q, want the rip rule", reason)
}
} }
// When Lidarr maps both copies, each fulfils its own release: nothing is // Copies on two releases that are both clean each belong to their release:
// removed, and the copies are linked as one song instead. // nothing is removed, and they are linked as one song. A rip the operator
func TestResolveDuplicates_KeepsCopiesLidarrMapsOnTwoReleases_Integration(t *testing.T) { // marked fine counts as clean.
func TestResolveDuplicates_KeepsCleanCopiesOnTwoReleases_Integration(t *testing.T) {
for name, prepare := range map[string]func(f crossReleaseRipFixture) (string, []any){
"renamed": func(f crossReleaseRipFixture) (string, []any) {
return `UPDATE tracks SET title = 'Feel Good Inc.', file_path = $2 WHERE id = $1`,
[]any{f.rip.ID, filepath.Join(filepath.Dir(f.ripPath), "01 - Feel Good Inc.mp3")}
},
"kept as fine": func(f crossReleaseRipFixture) (string, []any) {
return `UPDATE tracks SET source_verdict = 'fine' WHERE id = $1`, []any{f.rip.ID}
},
} {
t.Run(name, func(t *testing.T) {
f := newCrossReleaseRipFixture(t) f := newCrossReleaseRipFixture(t)
res, err := ResolveDuplicates(context.Background(), f.pool, nil, "", &fakeLidarr{}, true) sql, args := prepare(f)
if _, err := f.pool.Exec(context.Background(), sql, args...); err != nil {
t.Fatal(err)
}
res, err := ResolveDuplicates(context.Background(), f.pool, nil, "", true)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
@@ -447,6 +240,32 @@ func TestResolveDuplicates_KeepsCopiesLidarrMapsOnTwoReleases_Integration(t *tes
if res.SongsLinked != 1 { if res.SongsLinked != 1 {
t.Errorf("linked %d, want the pair linked as one song", res.SongsLinked) t.Errorf("linked %d, want the pair linked as one song", res.SongsLinked)
} }
})
}
}
// Identical audio under two titles on two albums, neither a rip: which tags
// are wrong is the operator's call, so nothing is merged or linked.
func TestResolveDuplicates_LeavesAMismatchForTheOperator_Integration(t *testing.T) {
f := newCrossReleaseRipFixture(t)
ctx := context.Background()
if _, err := f.pool.Exec(ctx, `UPDATE duplicate_groups SET tier = 'exact' WHERE id = $1`, f.groupID); err != nil {
t.Fatal(err)
}
if _, err := f.pool.Exec(ctx, `UPDATE tracks SET title = 'DARE', file_path = $2 WHERE id = $1`,
f.rip.ID, filepath.Join(filepath.Dir(f.ripPath), "01 - DARE.mp3")); err != nil {
t.Fatal(err)
}
res, err := ResolveDuplicates(ctx, f.pool, nil, "", true)
if err != nil {
t.Fatal(err)
}
if res.Classes[ClassMismatch] != 1 || res.Merged != 0 || res.SongsLinked != 0 {
t.Fatalf("classes %v, merged %d, linked %d; want a mismatch left alone", res.Classes, res.Merged, res.SongsLinked)
}
if status, _, note, _ := (resolveFixture{pool: f.pool, groupID: f.groupID}).group(t); status != "pending" || note == nil || *note == "" {
t.Errorf("group = (%s, note %v), want pending with a note", status, note)
}
} }
// A rip is held back from radio and the mixes; the operator's "fine" sticks // A rip is held back from radio and the mixes; the operator's "fine" sticks
@@ -455,6 +274,11 @@ func TestResolveDuplicates_HoldsBackRips_Integration(t *testing.T) {
f := newCrossReleaseRipFixture(t) f := newCrossReleaseRipFixture(t)
ctx := context.Background() ctx := context.Background()
q := dbq.New(f.pool) q := dbq.New(f.pool)
// Without its group the rip has no clean copy to merge into, so it stays
// on disk and only its verdict changes.
if _, err := f.pool.Exec(ctx, `DELETE FROM duplicate_groups WHERE id = $1`, f.groupID); err != nil {
t.Fatal(err)
}
verdict := func(id pgtype.UUID) *string { verdict := func(id pgtype.UUID) *string {
t.Helper() t.Helper()
var v *string var v *string
@@ -465,7 +289,7 @@ func TestResolveDuplicates_HoldsBackRips_Integration(t *testing.T) {
} }
resolve := func() DuplicateResolveResult { resolve := func() DuplicateResolveResult {
t.Helper() t.Helper()
res, err := ResolveDuplicates(ctx, f.pool, nil, "", &fakeLidarr{}, true) res, err := ResolveDuplicates(ctx, f.pool, nil, "", true)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
@@ -512,123 +336,3 @@ func TestResolveDuplicates_HoldsBackRips_Integration(t *testing.T) {
t.Errorf("set on an unflagged track: %d, %v; want 0 rows", n, err) t.Errorf("set on an unflagged track: %d, %v; want 0 rows", n, err)
} }
} }
// A rip that is the only copy of its song: Lidarr is asked to search its album
// for a better release (#5447), once a week at most, and the rip is not touched.
// A rip with a clean copy elsewhere is not searched for.
func TestResolveDuplicates_SearchesForASoleCopyRip_Integration(t *testing.T) {
f := newCrossReleaseRipFixture(t)
ctx := context.Background()
exec := func(sql string, args ...any) {
t.Helper()
if _, err := f.pool.Exec(ctx, sql, args...); err != nil {
t.Fatalf("exec %q: %v", sql, err)
}
}
// The single's rip has a clean copy on the album: never searched for.
exec(`UPDATE albums SET release_group_mbid = 'rg-single' WHERE id = $1`, f.rip.AlbumID)
lid := &fakeLidarr{album: lidarr.LidarrAlbum{ID: 77, ForeignAlbumID: "rg-single"}}
if _, err := ResolveDuplicates(ctx, f.pool, nil, "", lid, true); err != nil {
t.Fatal(err)
}
if len(lid.searchCalls) != 0 {
t.Fatalf("searched %v while a clean copy exists", lid.searchCalls)
}
// Without the group and the song link, the rip is the only copy.
exec(`DELETE FROM duplicate_groups WHERE id = $1`, f.groupID)
exec(`UPDATE tracks SET song_id = NULL WHERE id IN ($1, $2)`, f.rip.ID, f.clean.ID)
res, err := ResolveDuplicates(ctx, f.pool, nil, "", lid, true)
if err != nil {
t.Fatal(err)
}
if res.RipSearches != 1 || len(lid.searchCalls) != 1 || len(lid.searchCalls[0]) != 1 || lid.searchCalls[0][0] != 77 {
t.Fatalf("rip searches %d, calls %v; want album 77 searched once", res.RipSearches, lid.searchCalls)
}
if !exists(f.ripPath) {
t.Error("the rip was removed; a search must not touch it")
}
// The next pass, inside the week, leaves the album alone.
if res, err := ResolveDuplicates(ctx, f.pool, nil, "", lid, true); err != nil || res.RipSearches != 0 {
t.Errorf("second pass searched %d (%v), want 0", res.RipSearches, err)
}
var searchedAt pgtype.Timestamptz
rows, err := dbq.New(f.pool).ListSuspectSourceTracks(ctx, dbq.ListSuspectSourceTracksParams{
Pattern: SuspectSourcePattern, PageLimit: 10,
})
if err != nil {
t.Fatal(err)
}
for _, r := range rows {
if r.ID == f.rip.ID {
searchedAt = r.SearchedAt
}
}
if !searchedAt.Valid {
t.Error("the suspect-sources report doesn't show the search")
}
}
// Lidarr applies the release change but the PUT times out (Cracker Island on
// the first deploy pass). The change is read back and recorded, so the album
// settles; when it did not land, the error stands and nothing is recorded.
func TestResolveDuplicates_RecordsAReleaseChangeLidarrDidNotAnswer_Integration(t *testing.T) {
for name, landed := range map[string]bool{"landed": true, "did not land": false} {
t.Run(name, func(t *testing.T) {
f := newResolveFixture(t)
lid := &fakeLidarr{
album: lidarr.LidarrAlbum{ID: 4706, ForeignAlbumID: "rg-humanz", Title: "Cracker Island"},
releases: []lidarr.AlbumRelease{
{ID: 1, Format: "Digital Media", TrackCount: 4, Monitored: true},
{ID: 2, Format: "Digital Media", TrackCount: 2},
},
releaseTracks: map[int][]lidarr.ReleaseTrack{
1: rt("Saturnz Barz", "Ascension", "Saturnz Barz", "Ascension"),
2: rt("Saturnz Barz", "Ascension"),
},
setErr: fmt.Errorf("%w: context deadline exceeded", lidarr.ErrUnreachable),
}
if !landed {
lid.setErr = fmt.Errorf("%w: connection refused", lidarr.ErrUnreachable)
}
// ResetDB keeps audit_log, so other tests' release changes are
// still in it: count what this pass adds.
audited := func() (n int) {
t.Helper()
if err := f.pool.QueryRow(context.Background(),
`SELECT count(*) FROM audit_log WHERE action = 'lidarr_release_change'`).Scan(&n); err != nil {
t.Fatal(err)
}
return n
}
before := audited()
res, err := ResolveDuplicates(context.Background(), f.pool, nil, "", &undoingLidarr{fakeLidarr: lid, undo: !landed}, true)
if landed {
if err != nil || len(res.ReleaseChanges) != 1 {
t.Fatalf("changes %d (%v), want the landed change recorded", len(res.ReleaseChanges), err)
}
} else if len(res.ReleaseChanges) != 0 {
t.Fatalf("recorded %d changes that did not land", len(res.ReleaseChanges))
}
if got, want := audited()-before, map[bool]int{true: 1, false: 0}[landed]; got != want {
t.Errorf("audited %d release changes, want %d", got, want)
}
})
}
}
// undoingLidarr is a Lidarr whose release change fails without landing when
// undo is set: the PUT never reached it.
type undoingLidarr struct {
*fakeLidarr
undo bool
}
func (u *undoingLidarr) SetMonitoredRelease(ctx context.Context, albumID, releaseID int) error {
if !u.undo {
return u.fakeLidarr.SetMonitoredRelease(ctx, albumID, releaseID)
}
u.setCalls = append(u.setCalls, [2]int{albumID, releaseID})
return u.setErr
}
+3 -12
View File
@@ -14,10 +14,6 @@ type SurvivorCandidate struct {
FileSize int64 FileSize int64
AddedAt time.Time AddedAt time.Time
// LidarrTracked: Lidarr maps this file to a track of the release it
// monitors. Removing it opens a hole Lidarr downloads again (M498), so a
// tracked copy outranks everything else. False when Lidarr was not asked.
LidarrTracked bool
// TagFit is TagFitScore: how well the copy's tags and name fit the album. // TagFit is TagFitScore: how well the copy's tags and name fit the album.
TagFit int TagFit int
} }
@@ -56,9 +52,9 @@ var losslessFormats = map[string]bool{"flac": true, "wav": true}
// the report shows it and the merge (#3911) lets the operator choose another. // the report shows it and the merge (#3911) lets the operator choose another.
// //
// In order: // In order:
// 0. the copy Lidarr maps, then the copy whose tags fit the album best // 0. the copy whose tags fit the album best (TagFitScore). It is zero for
// (TagFitScore). Both are zero for every copy when the caller does not // every copy when the caller does not know it, and the order falls through
// know them, and the order falls through to the quality rules below // to the quality rules below
// 1. lossless over lossy — the one difference no later step can recover // 1. lossless over lossy — the one difference no later step can recover
// 2. the larger file — for one recording at one duration that is the higher // 2. the larger file — for one recording at one duration that is the higher
// bitrate. The scanner does not record bitrate (tracks.bitrate is never // bitrate. The scanner does not record bitrate (tracks.bitrate is never
@@ -81,8 +77,6 @@ func ProposeSurvivor(cands []SurvivorCandidate) (trackID, reason string) {
// runner-up — the rule that actually decided, not every rule it passed. // runner-up — the rule that actually decided, not every rule it passed.
next := ranked[1] next := ranked[1]
switch { switch {
case best.LidarrTracked != next.LidarrTracked:
return best.TrackID, "the copy Lidarr tracks"
case best.TagFit != next.TagFit: case best.TagFit != next.TagFit:
return best.TrackID, "tags fit the album" return best.TrackID, "tags fit the album"
case isLossless(best) != isLossless(next): case isLossless(best) != isLossless(next):
@@ -97,9 +91,6 @@ func ProposeSurvivor(cands []SurvivorCandidate) (trackID, reason string) {
} }
func survivorBefore(a, b SurvivorCandidate) bool { func survivorBefore(a, b SurvivorCandidate) bool {
if a.LidarrTracked != b.LidarrTracked {
return a.LidarrTracked
}
if a.TagFit != b.TagFit { if a.TagFit != b.TagFit {
return a.TagFit > b.TagFit return a.TagFit > b.TagFit
} }
+8 -9
View File
@@ -90,11 +90,10 @@ func TestProposeSurvivor_ReasonIsTheDecidingRule(t *testing.T) {
} }
} }
// M498 #5436: the copy Lidarr maps outranks everything, then the copy whose // M498 #5436: the copy whose tags fit the album outranks quality, which decides
// tags fit the album; quality decides only between copies equal on both. Each // only between copies whose tags fit equally. Each case is built so the quality
// case is built so the older rules would choose the other copy, so a reorder // rules would choose the other copy, so a reorder that drops tag fit fails.
// that drops the new rules fails. func TestProposeSurvivor_TagFitFirst(t *testing.T) {
func TestProposeSurvivor_LidarrThenTagFit(t *testing.T) {
at := time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC) at := time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC)
cases := []struct { cases := []struct {
name string name string
@@ -103,12 +102,12 @@ func TestProposeSurvivor_LidarrThenTagFit(t *testing.T) {
wantReason string wantReason string
}{ }{
{ {
name: "tracked beats lossless and better tags", name: "tag fit beats lossless",
cands: []SurvivorCandidate{ cands: []SurvivorCandidate{
{TrackID: "flac", FileFormat: "flac", FileSize: 40_000_000, AddedAt: at, TagFit: 3}, {TrackID: "flac", FileFormat: "flac", FileSize: 40_000_000, AddedAt: at, TagFit: 1},
{TrackID: "tracked", FileFormat: "mp3", FileSize: 5_000_000, AddedAt: at, TagFit: 1, LidarrTracked: true}, {TrackID: "fits", FileFormat: "mp3", FileSize: 5_000_000, AddedAt: at, TagFit: 3},
}, },
wantID: "tracked", wantReason: "the copy Lidarr tracks", wantID: "fits", wantReason: "tags fit the album",
}, },
{ {
// The Humanz shape: two rips a few bytes apart, one carrying a // The Humanz shape: two rips a few bytes apart, one carrying a
+3 -3
View File
@@ -78,7 +78,7 @@ func TestCrossReleaseCopiesCountAsOneSong_Integration(t *testing.T) {
q := dbq.New(pool) q := dbq.New(pool)
f := newSongLinkFixture(t, pool) f := newSongLinkFixture(t, pool)
res, err := ResolveDuplicates(ctx, pool, nil, "", nil, true) res, err := ResolveDuplicates(ctx, pool, nil, "", true)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
@@ -118,7 +118,7 @@ func TestCrossReleaseCopiesCountAsOneSong_Integration(t *testing.T) {
} }
// A second pass changes nothing. // A second pass changes nothing.
if res, err := ResolveDuplicates(ctx, pool, nil, "", nil, true); err != nil || res.SongsLinked != 0 { if res, err := ResolveDuplicates(ctx, pool, nil, "", true); err != nil || res.SongsLinked != 0 {
t.Errorf("second pass linked %d (%v), want 0", res.SongsLinked, err) t.Errorf("second pass linked %d (%v), want 0", res.SongsLinked, err)
} }
@@ -155,7 +155,7 @@ func TestCrossReleaseLinkNeedsAutoResolve_Integration(t *testing.T) {
ctx := context.Background() ctx := context.Background()
q := dbq.New(pool) q := dbq.New(pool)
f := newSongLinkFixture(t, pool) f := newSongLinkFixture(t, pool)
if _, err := ResolveDuplicates(ctx, pool, nil, "", nil, false); err != nil { if _, err := ResolveDuplicates(ctx, pool, nil, "", false); err != nil {
t.Fatal(err) t.Fatal(err)
} }
keys := songKeys(t, q, f.single.ID, f.albumCut.ID) keys := songKeys(t, q, f.single.ID, f.albumCut.ID)
-175
View File
@@ -1,175 +0,0 @@
package lidarr
import (
"context"
"encoding/json"
"fmt"
"net/url"
"strconv"
)
// What Lidarr holds on disk and how it maps it (Scribe milestone #498).
//
// Lidarr maps one file to each track of the ONE release it monitors per album.
// A file it holds but maps to no track is "unmapped": removing it opens no hole.
// Removing a mapped file does, and Lidarr fills the hole by downloading again —
// which is why Minstrel's duplicate resolver asks before it deletes anything.
// TrackFile is a file Lidarr holds on disk.
type TrackFile struct {
ID int `json:"id"`
Path string `json:"path"`
AlbumID int `json:"albumId"`
}
// ListUnmappedTrackFiles hits GET /api/v1/trackfile?unmapped=true: files under
// Lidarr's root folders that it has scanned but matched to no track.
func (c *Client) ListUnmappedTrackFiles(ctx context.Context) ([]TrackFile, error) {
resp, err := c.get(ctx, "/api/v1/trackfile", url.Values{"unmapped": {"true"}})
if err != nil {
return nil, err
}
defer func() { _ = resp.Body.Close() }()
var out []TrackFile
if err := json.NewDecoder(resp.Body).Decode(&out); err != nil {
return nil, fmt.Errorf("%w: decode unmapped track files: %v", ErrInvalidPayload, err)
}
return out, nil
}
// AlbumRelease is one edition of an album as Lidarr knows it: the 14×12" vinyl
// box set, the 2×CD deluxe, the digital standard. Exactly one is Monitored.
type AlbumRelease struct {
ID int `json:"id"`
Title string `json:"title"`
Disambiguation string `json:"disambiguation"`
Format string `json:"format"`
TrackCount int `json:"trackCount"`
MediumCount int `json:"mediumCount"`
Monitored bool `json:"monitored"`
}
// ReleaseTrack is one track of a release.
type ReleaseTrack struct {
ID int `json:"id"`
Title string `json:"title"`
MediumNumber int `json:"mediumNumber"`
TrackFileID int `json:"trackFileId"`
}
// GetAlbumReleases hits GET /api/v1/album/{id} and returns the album's releases.
func (c *Client) GetAlbumReleases(ctx context.Context, albumID int) ([]AlbumRelease, error) {
resp, err := c.get(ctx, "/api/v1/album/"+strconv.Itoa(albumID), nil)
if err != nil {
return nil, err
}
defer func() { _ = resp.Body.Close() }()
var album struct {
Releases []AlbumRelease `json:"releases"`
}
if err := json.NewDecoder(resp.Body).Decode(&album); err != nil {
return nil, fmt.Errorf("%w: decode album: %v", ErrInvalidPayload, err)
}
return album.Releases, nil
}
// ListAlbumTracks hits GET /api/v1/track?albumId= : the tracks of the release
// Lidarr monitors for that album.
func (c *Client) ListAlbumTracks(ctx context.Context, albumID int) ([]ReleaseTrack, error) {
return c.listTracks(ctx, url.Values{"albumId": {strconv.Itoa(albumID)}})
}
// ListReleaseTracks hits GET /api/v1/track?albumReleaseId= : the tracks of one
// release, monitored or not.
func (c *Client) ListReleaseTracks(ctx context.Context, releaseID int) ([]ReleaseTrack, error) {
return c.listTracks(ctx, url.Values{"albumReleaseId": {strconv.Itoa(releaseID)}})
}
func (c *Client) listTracks(ctx context.Context, q url.Values) ([]ReleaseTrack, error) {
resp, err := c.get(ctx, "/api/v1/track", q)
if err != nil {
return nil, err
}
defer func() { _ = resp.Body.Close() }()
var out []ReleaseTrack
if err := json.NewDecoder(resp.Body).Decode(&out); err != nil {
return nil, fmt.Errorf("%w: decode tracks: %v", ErrInvalidPayload, err)
}
return out, nil
}
// SetMonitoredRelease makes releaseID the album's monitored release.
//
// It is the round trip Lidarr's own "edit album" makes: GET the album resource,
// flip `monitored` on its releases, PUT the whole resource back. Lidarr's
// UpdateAlbum publishes AlbumEditedEvent, and when the monitored release changed,
// AlbumEditedService unlinks the album's track files and queues
// RescanFoldersCommand for the artist's folder, which maps the files on disk to
// the new release's tracks. Files the new release has no track for come out of
// that rescan unmapped. (Lidarr source, develop: Music/Services/
// AlbumEditedService.cs and AlbumService.UpdateAlbum.)
//
// The resource is kept as raw JSON so every field Lidarr sent goes back
// unchanged; only the releases' monitored flags differ. Returns ErrNotFound when
// the album has no release with that id.
func (c *Client) SetMonitoredRelease(ctx context.Context, albumID, releaseID int) error {
path := "/api/v1/album/" + strconv.Itoa(albumID)
resp, err := c.get(ctx, path, nil)
if err != nil {
return err
}
var album map[string]any
err = json.NewDecoder(resp.Body).Decode(&album)
_ = resp.Body.Close()
if err != nil {
return fmt.Errorf("%w: decode album: %v", ErrInvalidPayload, err)
}
releases, _ := album["releases"].([]any)
found := false
for _, r := range releases {
rel, ok := r.(map[string]any)
if !ok {
continue
}
id, _ := rel["id"].(float64)
match := int(id) == releaseID
rel["monitored"] = match
found = found || match
}
if !found {
return fmt.Errorf("%w: release %d on album %d", ErrNotFound, releaseID, albumID)
}
body, err := json.Marshal(album)
if err != nil {
return fmt.Errorf("%w: marshal album: %v", ErrInvalidPayload, err)
}
putResp, err := c.put(ctx, path, body)
if err != nil {
return err
}
_ = putResp.Body.Close()
return nil
}
// SearchAlbums asks Lidarr to search its indexers for the albums
// (POST /api/v1/command, AlbumSearch). Lidarr grabs a release only when it
// beats the files the album already has under the quality profile, so for an
// album whose tracks are all present this is an upgrade search: harmless when
// nothing better exists. The command is queued; the call does not wait for it.
func (c *Client) SearchAlbums(ctx context.Context, albumIDs []int) error {
if len(albumIDs) == 0 {
return nil
}
body, err := json.Marshal(map[string]any{"name": "AlbumSearch", "albumIds": albumIDs})
if err != nil {
return fmt.Errorf("%w: marshal command: %v", ErrInvalidPayload, err)
}
resp, err := c.post(ctx, "/api/v1/command", body)
if err != nil {
return err
}
_ = resp.Body.Close()
return nil
}
-138
View File
@@ -1,138 +0,0 @@
package lidarr
import (
"context"
"encoding/json"
"errors"
"io"
"net/http"
"testing"
)
func TestListUnmappedTrackFiles(t *testing.T) {
c, srv := newTestClient(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path != "/api/v1/trackfile" || r.URL.Query().Get("unmapped") != "true" {
t.Errorf("request = %s?%s, want /api/v1/trackfile?unmapped=true", r.URL.Path, r.URL.RawQuery)
}
_, _ = w.Write([]byte(`[{"id":7,"path":"/music/A/B (2017)/A - B - 02 - X.mp3","albumId":0}]`))
})
defer srv.Close()
got, err := c.ListUnmappedTrackFiles(context.Background())
if err != nil {
t.Fatalf("ListUnmappedTrackFiles: %v", err)
}
if len(got) != 1 || got[0].ID != 7 || got[0].Path != "/music/A/B (2017)/A - B - 02 - X.mp3" {
t.Errorf("got %+v", got)
}
}
func TestReleaseReads(t *testing.T) {
c, srv := newTestClient(func(w http.ResponseWriter, r *http.Request) {
switch {
case r.URL.Path == "/api/v1/album/4723":
_, _ = w.Write([]byte(`{"id":4723,"releases":[
{"id":10148,"title":"Humanz","format":"14x12\" Vinyl, Digital Media","trackCount":68,"mediumCount":15,"monitored":true},
{"id":10149,"title":"Humanz","format":"Digital Media","trackCount":40,"mediumCount":1,"monitored":false}]}`))
case r.URL.Path == "/api/v1/track" && r.URL.Query().Get("albumId") == "4723":
_, _ = w.Write([]byte(`[{"id":1,"title":"Ascension","mediumNumber":1,"trackFileId":9}]`))
case r.URL.Path == "/api/v1/track" && r.URL.Query().Get("albumReleaseId") == "10149":
_, _ = w.Write([]byte(`[{"id":2,"title":"Ascension","mediumNumber":1,"trackFileId":0}]`))
default:
t.Errorf("unexpected request %s?%s", r.URL.Path, r.URL.RawQuery)
w.WriteHeader(http.StatusNotFound)
}
})
defer srv.Close()
ctx := context.Background()
rels, err := c.GetAlbumReleases(ctx, 4723)
if err != nil {
t.Fatalf("GetAlbumReleases: %v", err)
}
if len(rels) != 2 || !rels[0].Monitored || rels[0].MediumCount != 15 || rels[1].TrackCount != 40 {
t.Errorf("releases = %+v", rels)
}
tr, err := c.ListAlbumTracks(ctx, 4723)
if err != nil || len(tr) != 1 || tr[0].TrackFileID != 9 {
t.Errorf("ListAlbumTracks = %+v, %v", tr, err)
}
tr, err = c.ListReleaseTracks(ctx, 10149)
if err != nil || len(tr) != 1 || tr[0].Title != "Ascension" {
t.Errorf("ListReleaseTracks = %+v, %v", tr, err)
}
}
// The PUT carries the whole resource back, with only the releases' monitored
// flags changed: exactly the chosen release is monitored, and a field Minstrel
// does not model survives the round trip.
func TestSetMonitoredRelease_RoundTripsTheResource(t *testing.T) {
var put map[string]any
c, srv := newTestClient(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path != "/api/v1/album/4723" {
t.Errorf("path = %q", r.URL.Path)
}
switch r.Method {
case http.MethodGet:
_, _ = w.Write([]byte(`{"id":4723,"anyReleaseOk":false,"secondaryTypes":["Studio"],"releases":[
{"id":10148,"monitored":true},{"id":10149,"monitored":false}]}`))
case http.MethodPut:
b, _ := io.ReadAll(r.Body)
if err := json.Unmarshal(b, &put); err != nil {
t.Errorf("PUT body: %v", err)
}
w.WriteHeader(http.StatusAccepted)
}
})
defer srv.Close()
if err := c.SetMonitoredRelease(context.Background(), 4723, 10149); err != nil {
t.Fatalf("SetMonitoredRelease: %v", err)
}
rels := put["releases"].([]any)
if m := rels[0].(map[string]any)["monitored"]; m != false {
t.Errorf("old release monitored = %v, want false", m)
}
if m := rels[1].(map[string]any)["monitored"]; m != true {
t.Errorf("new release monitored = %v, want true", m)
}
if st, _ := put["secondaryTypes"].([]any); len(st) != 1 || st[0] != "Studio" {
t.Errorf("secondaryTypes = %v, want it carried through unchanged", put["secondaryTypes"])
}
}
func TestSetMonitoredRelease_UnknownReleaseSendsNothing(t *testing.T) {
c, srv := newTestClient(func(w http.ResponseWriter, r *http.Request) {
if r.Method == http.MethodPut {
t.Error("PUT sent for a release the album does not have")
}
_, _ = w.Write([]byte(`{"id":4723,"releases":[{"id":10148,"monitored":true}]}`))
})
defer srv.Close()
if err := c.SetMonitoredRelease(context.Background(), 4723, 999); !errors.Is(err, ErrNotFound) {
t.Errorf("err = %v, want ErrNotFound", err)
}
}
func TestSearchAlbums_PostsTheCommand(t *testing.T) {
var got map[string]any
c, srv := newTestClient(func(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodPost || r.URL.Path != "/api/v1/command" {
t.Errorf("request = %s %s, want POST /api/v1/command", r.Method, r.URL.Path)
}
b, _ := io.ReadAll(r.Body)
_ = json.Unmarshal(b, &got)
w.WriteHeader(http.StatusCreated)
_, _ = w.Write([]byte(`{"id":1,"name":"AlbumSearch"}`))
})
defer srv.Close()
if err := c.SearchAlbums(context.Background(), []int{4723, 12}); err != nil {
t.Fatalf("SearchAlbums: %v", err)
}
ids, _ := got["albumIds"].([]any)
if got["name"] != "AlbumSearch" || len(ids) != 2 || ids[0] != float64(4723) || ids[1] != float64(12) {
t.Errorf("command = %v", got)
}
}
-11
View File
@@ -57,17 +57,6 @@ func (a lidarrUnmonitorAdapter) UnmonitorTrack(ctx context.Context, trackMbid, a
return c.UnmonitorTrack(ctx, trackMbid, albumMbid) return c.UnmonitorTrack(ctx, trackMbid, albumMbid)
} }
// ListUnmappedTrackFiles lets the duplicate merge refuse to remove a copy
// Lidarr maps (M498). tracks.ErrLidarrDisabled tells it there is no Lidarr to
// protect.
func (a lidarrUnmonitorAdapter) ListUnmappedTrackFiles(ctx context.Context) ([]lidarr.TrackFile, error) {
c := a.fn()
if c == nil {
return nil, tracks.ErrLidarrDisabled
}
return c.ListUnmappedTrackFiles(ctx)
}
// ScanTrigger is the subset of the scanner the HTTP handler needs. Kept as an // ScanTrigger is the subset of the scanner the HTTP handler needs. Kept as an
// interface so tests can stub it without touching the DB. The progressCb // interface so tests can stub it without touching the DB. The progressCb
// parameter (added in m7-scan-progress) lets the orchestrator drive partial- // parameter (added in m7-scan-progress) lets the orchestrator drive partial-
+2 -48
View File
@@ -31,7 +31,6 @@ import (
"git.fabledsword.com/bvandeusen/minstrel/internal/audit" "git.fabledsword.com/bvandeusen/minstrel/internal/audit"
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
"git.fabledsword.com/bvandeusen/minstrel/internal/library" "git.fabledsword.com/bvandeusen/minstrel/internal/library"
"git.fabledsword.com/bvandeusen/minstrel/internal/lidarr"
syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync" syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync"
) )
@@ -41,22 +40,6 @@ import (
// same pointer. // same pointer.
var ErrNotFound = apierror.ErrNotFound var ErrNotFound = apierror.ErrNotFound
// ErrLidarrDisabled is what a LidarrUnmonitorer that also lists Lidarr's
// unmapped files returns when Lidarr is turned off: there is then no Lidarr
// record a removal could break, and a merge goes ahead unguarded.
var ErrLidarrDisabled = errors.New("tracks: lidarr disabled")
// ErrLidarrUnavailable means Lidarr is on but did not answer, so a merge cannot
// confirm the copies it would remove are ones Lidarr does not need (M498), and
// refuses rather than guess.
var ErrLidarrUnavailable = errors.New("tracks: lidarr did not answer")
// lidarrFileLister is the optional part of the Lidarr dependency a merge uses to
// learn which files Lidarr holds without mapping to any track.
type lidarrFileLister interface {
ListUnmappedTrackFiles(ctx context.Context) ([]lidarr.TrackFile, error)
}
// LidarrUnmonitorer is the subset of *lidarr.Client RemoveTrack uses. // LidarrUnmonitorer is the subset of *lidarr.Client RemoveTrack uses.
// Defined as an interface so tests can stub without spinning up a Lidarr // Defined as an interface so tests can stub without spinning up a Lidarr
// httptest server. UnmonitorTrack failures are non-fatal at the service // httptest server. UnmonitorTrack failures are non-fatal at the service
@@ -168,9 +151,7 @@ func (s *Service) RemoveTrack(
} }
// MergeDuplicates merges a duplicate group into the copy to keep // MergeDuplicates merges a duplicate group into the copy to keep
// (library.MergeDuplicateGroupGuarded, refusing to remove a copy Lidarr maps: // (library.MergeDuplicateGroup), then — when asked — tells Lidarr to stop
// library.ErrCopyTrackedByLidarr, or ErrLidarrUnavailable when Lidarr cannot
// say), then — when asked — tells Lidarr to stop
// monitoring the removed copies so it does not download them again, and records // monitoring the removed copies so it does not download them again, and records
// the merge in the audit log. // the merge in the audit log.
// //
@@ -180,11 +161,7 @@ func (s *Service) RemoveTrack(
func (s *Service) MergeDuplicates( func (s *Service) MergeDuplicates(
ctx context.Context, groupID, survivorID, actorID pgtype.UUID, unmonitor bool, ctx context.Context, groupID, survivorID, actorID pgtype.UUID, unmonitor bool,
) (res library.MergeResult, lidarrUnmonitorFailed bool, err error) { ) (res library.MergeResult, lidarrUnmonitorFailed bool, err error) {
removable, err := s.lidarrRemovable(ctx) res, err = library.MergeDuplicateGroup(ctx, s.pool, s.logger, s.dataDir, groupID, survivorID)
if err != nil {
return res, false, err
}
res, err = library.MergeDuplicateGroupGuarded(ctx, s.pool, s.logger, s.dataDir, groupID, survivorID, removable)
if err != nil { if err != nil {
return res, false, err return res, false, err
} }
@@ -226,29 +203,6 @@ func (s *Service) MergeDuplicates(
return res, lidarrUnmonitorFailed, nil return res, lidarrUnmonitorFailed, nil
} }
// lidarrRemovable is the merge's guard (M498): only a copy Lidarr holds without
// mapping it to a track may be removed, because removing a mapped one makes
// Lidarr download it again. Nil, allowing everything, when Lidarr is disabled
// or the dependency cannot list files.
func (s *Service) lidarrRemovable(ctx context.Context) (library.RemovableFunc, error) {
lister, ok := s.lidarr.(lidarrFileLister)
if !ok {
return nil, nil
}
files, err := lister.ListUnmappedTrackFiles(ctx)
if errors.Is(err, ErrLidarrDisabled) {
return nil, nil
}
if err != nil {
return nil, fmt.Errorf("%w: %v", ErrLidarrUnavailable, err)
}
unmapped := make(map[string]bool, len(files))
for _, f := range files {
unmapped[library.LidarrPathKey(f.Path)] = true
}
return func(p string) bool { return unmapped[library.LidarrPathKey(p)] }, nil
}
// sameLidarrTrack reports whether two copies are the same Lidarr track: one // sameLidarrTrack reports whether two copies are the same Lidarr track: one
// recording on one album. Lidarr monitors per album track, so when the removed // recording on one album. Lidarr monitors per album track, so when the removed
// copy is a second file of the kept copy's own album track — the #3885 case — // copy is a second file of the kept copy's own album track — the #3885 case —
-4
View File
@@ -479,10 +479,6 @@ export type AdminDuplicateMember = {
added_at: string; added_at: string;
like_count: number; like_count: number;
play_count: number; play_count: number;
// Lidarr's view of the file as of the resolver's last pass (M498): tracked
// means Lidarr maps it to a track, so removing it would make Lidarr download
// it again. null when Lidarr wasn't asked.
lidarr_state: 'tracked' | 'unmapped' | null;
disc_number: number | null; disc_number: number | null;
track_number: number | null; track_number: number | null;
}; };
@@ -120,10 +120,9 @@
<span> <span>
<span class="text-sm text-text-primary">Resolve duplicates automatically</span> <span class="text-sm text-text-primary">Resolve duplicates automatically</span>
<span class="block text-xs text-text-secondary"> <span class="block text-xs text-text-secondary">
Merges copies Lidarr doesn't use into the one it does, and moves a Lidarr album off a Merges a song held twice on one album, and a video rip with a clean copy elsewhere, into
release that lists songs twice. A song kept on several releases counts as one song, and the copy that fits best. A song kept on several releases counts as one song, and video
video rips are kept out of radio and the mixes. A copy Lidarr uses is never removed. rips are kept out of radio and the mixes. Off, every group waits here for you.
Off, every group waits here for you.
</span> </span>
</span> </span>
</label> </label>
-2
View File
@@ -46,8 +46,6 @@
"sweep_in_progress": "A duplicate sweep is already running.", "sweep_in_progress": "A duplicate sweep is already running.",
"duplicate_group_not_pending": "That group has already been resolved.", "duplicate_group_not_pending": "That group has already been resolved.",
"survivor_not_in_group": "That copy isn't part of this group any more.", "survivor_not_in_group": "That copy isn't part of this group any more.",
"copy_tracked_by_lidarr": "Lidarr uses a copy this would remove, and would download it again. Keep that copy instead.",
"lidarr_unavailable": "Lidarr didn't answer, so it isn't safe to remove a copy yet. Try again shortly.",
"suspect_track_not_found": "That track isn't flagged any more. Refresh the list.", "suspect_track_not_found": "That track isn't flagged any more. Refresh the list.",
"invalid_verdict": "That isn't a choice for a flagged track.", "invalid_verdict": "That isn't a choice for a flagged track.",
"invalid_setting": "That setting is out of range.", "invalid_setting": "That setting is out of range.",
+11 -44
View File
@@ -23,17 +23,17 @@
import TabStrip from '#lib/components/TabStrip.svelte'; import TabStrip from '#lib/components/TabStrip.svelte';
// Tracks the duplicate sweep believes hold one recording (#3912). The // Tracks the duplicate sweep believes hold one recording (#3912). The
// resolver (M498) settles what is safe on its own: copies Lidarr doesn't use // resolver (M498, M502) settles what is safe on its own: a song held twice on
// are merged into the one it does, and a Lidarr release that lists songs // one album, and a video rip with a clean copy elsewhere. What is left comes
// twice is changed. What is left comes in two tabs: groups that need the // in two tabs: groups that need the operator, and the same song on different
// operator, and the same song on different releases, which is kept on // releases, which is kept on purpose. A third tab lists what the resolver
// purpose. A third tab lists what the resolver did. // did.
// //
// Dismissing a group says "these are not duplicates", and the sweep will not // Dismissing a group says "these are not duplicates", and the sweep will not
// propose that set again. On the cross-release tab it reads "Not the same // propose that set again. On the cross-release tab it reads "Not the same
// song" and also undoes the link that made the copies one song (#5438). Merging (#3911) keeps one copy, moves the others' // song" and also undoes the link that made the copies one song (#5438). Merging (#3911) keeps one copy, moves the others'
// likes, plays and playlist entries onto it, and deletes their files, so it // likes, plays and playlist entries onto it, and deletes their files, so it
// asks twice. The server refuses to remove a copy Lidarr uses. // asks twice.
type Tab = DuplicatesView | 'resolved'; type Tab = DuplicatesView | 'resolved';
const fromUrl = page.url.searchParams.get('view'); const fromUrl = page.url.searchParams.get('view');
@@ -108,13 +108,6 @@
return keepChoice[group.id] ?? group.survivor_track_id; return keepChoice[group.id] ?? group.survivor_track_id;
} }
// Copies the merge would remove that Lidarr uses. The server refuses these,
// so the confirm says so before the click rather than after.
function trackedLosers(group: AdminDuplicateGroup): AdminDuplicateMember[] {
const keep = keeperOf(group);
return group.members.filter((m) => m.track_id !== keep && m.lidarr_state === 'tracked');
}
function fileCountLabel(n: number): string { function fileCountLabel(n: number): string {
return n === 1 ? '1 file' : `${n} files`; return n === 1 ? '1 file' : `${n} files`;
} }
@@ -223,8 +216,8 @@
</button> </button>
</div> </div>
<p class="text-text-secondary"> <p class="text-text-secondary">
Tracks that hold the same recording more than once. Copies Lidarr doesn't use are merged on Tracks that hold the same recording more than once. A song held twice on one album is merged on
their own, and a copy Lidarr uses is never removed. What's left here needs you. its own, and so is a video rip with a clean copy elsewhere. What's left here needs you.
</p> </p>
</header> </header>
@@ -265,7 +258,7 @@
<Copy size={28} strokeWidth={1} class="mx-auto text-text-muted" /> <Copy size={28} strokeWidth={1} class="mx-auto text-text-muted" />
<p class="mt-3 text-text-primary">Nothing resolved automatically yet.</p> <p class="mt-3 text-text-primary">Nothing resolved automatically yet.</p>
<p class="mt-1 text-sm text-text-secondary"> <p class="mt-1 text-sm text-text-secondary">
Merges and Lidarr release changes made on their own are listed here. Merges made on their own are listed here.
</p> </p>
</div> </div>
{:else} {:else}
@@ -279,7 +272,7 @@
{#if item.details.survivor_reason} {#if item.details.survivor_reason}
<span class="text-text-secondary">({item.details.survivor_reason})</span> <span class="text-text-secondary">({item.details.survivor_reason})</span>
{/if} {/if}
and removed {fileCountLabel(removed.length)} Lidarr doesn't use. and removed {fileCountLabel(removed.length)}.
</p> </p>
{#each removed as r (r.track_id)} {#each removed as r (r.track_id)}
<p class="truncate font-mono text-xs text-text-muted" title={r.file_path}>{r.file_path}</p> <p class="truncate font-mono text-xs text-text-muted" title={r.file_path}>{r.file_path}</p>
@@ -354,7 +347,6 @@
<ul class="space-y-4"> <ul class="space-y-4">
{#each groups as group (group.id)} {#each groups as group (group.id)}
{@const titles = differingTitles(group)} {@const titles = differingTitles(group)}
{@const blocked = trackedLosers(group)}
<li class="overflow-hidden rounded-lg border border-border bg-surface" data-testid="duplicate-group"> <li class="overflow-hidden rounded-lg border border-border bg-surface" data-testid="duplicate-group">
<div class="flex items-center justify-between gap-4 border-b border-border px-4 py-3"> <div class="flex items-center justify-between gap-4 border-b border-border px-4 py-3">
<div class="min-w-0"> <div class="min-w-0">
@@ -373,7 +365,7 @@
<button <button
type="button" type="button"
onclick={() => onMerge(group)} onclick={() => onMerge(group)}
disabled={merging === group.id || (confirmingMerge === group.id && blocked.length > 0)} disabled={merging === group.id}
class="rounded-md px-3 py-1.5 text-sm disabled:opacity-50 {confirmingMerge === group.id class="rounded-md px-3 py-1.5 text-sm disabled:opacity-50 {confirmingMerge === group.id
? 'bg-action-destructive text-action-fg hover:opacity-90' ? 'bg-action-destructive text-action-fg hover:opacity-90'
: 'border border-border text-text-primary hover:bg-surface-hover'}" : 'border border-border text-text-primary hover:bg-surface-hover'}"
@@ -416,13 +408,6 @@
{#if confirmingMerge === group.id} {#if confirmingMerge === group.id}
<!-- What the second click will do, said plainly before it is done. --> <!-- What the second click will do, said plainly before it is done. -->
<div class="space-y-2 border-b border-border bg-surface-hover px-4 py-3 text-sm" data-testid="merge-confirm"> <div class="space-y-2 border-b border-border bg-surface-hover px-4 py-3 text-sm" data-testid="merge-confirm">
{#if blocked.length > 0}
<p class="text-error-fg" data-testid="merge-blocked">
Lidarr uses {blocked.length === 1 ? 'a copy' : 'copies'} this would remove, and would
download {blocked.length === 1 ? 'it' : 'them'} again. Keep the copy Lidarr uses, or
leave this group as it is.
</p>
{:else}
<p class="text-text-primary"> <p class="text-text-primary">
The copy marked Keep stays. The other {fileCountLabel(group.members.length - 1)} will be The copy marked Keep stays. The other {fileCountLabel(group.members.length - 1)} will be
deleted from disk, and their likes, plays and playlist entries move to the copy kept. deleted from disk, and their likes, plays and playlist entries move to the copy kept.
@@ -435,7 +420,6 @@
/> />
Tell Lidarr to stop monitoring the removed copies, so it doesn't download them again Tell Lidarr to stop monitoring the removed copies, so it doesn't download them again
</label> </label>
{/if}
<button <button
type="button" type="button"
class="text-xs text-text-secondary underline hover:text-text-primary" class="text-xs text-text-secondary underline hover:text-text-primary"
@@ -472,23 +456,6 @@
Keep · {group.survivor_reason} Keep · {group.survivor_reason}
</span> </span>
{/if} {/if}
{#if m.lidarr_state === 'tracked'}
<span
class="shrink-0 rounded-full border border-border px-2 py-0.5 text-xs text-text-secondary"
data-testid="lidarr-state"
title="Lidarr maps this file to a track. Removing it would make Lidarr download it again."
>
Lidarr uses this
</span>
{:else if m.lidarr_state === 'unmapped'}
<span
class="shrink-0 rounded-full border border-border px-2 py-0.5 text-xs text-text-muted"
data-testid="lidarr-state"
title="Lidarr holds this file but maps it to no track. Removing it opens no gap."
>
Lidarr doesn't use this
</span>
{/if}
</div> </div>
<div class="truncate text-xs text-text-secondary"> <div class="truncate text-xs text-text-secondary">
{m.artist_name} · {m.album_title}{position(m) ? ` · ${position(m)}` : ''} {m.artist_name} · {m.album_title}{position(m) ? ` · ${position(m)}` : ''}
@@ -66,7 +66,6 @@ function member(id: string, extra: Partial<AdminDuplicatesResponse['groups'][num
added_at: ago(500 * HOUR), added_at: ago(500 * HOUR),
like_count: 0, like_count: 0,
play_count: 0, play_count: 0,
lidarr_state: null,
disc_number: null, disc_number: null,
track_number: null, track_number: null,
...extra ...extra
@@ -332,7 +331,9 @@ describe('admin duplicates', () => {
expect(dismissDuplicateGroup).toHaveBeenCalledWith('g-1'); expect(dismissDuplicateGroup).toHaveBeenCalledWith('g-1');
}); });
test('Resolved automatically lists merges and release changes', async () => { // Release changes stopped with Lidarr (M502), but the ones made before stay
// in the history.
test('Resolved automatically lists merges and past release changes', async () => {
renderWith(response(), [ renderWith(response(), [
{ {
id: 'a-1', id: 'a-1',
@@ -340,7 +341,7 @@ describe('admin duplicates', () => {
created_at: ago(HOUR), created_at: ago(HOUR),
details: { details: {
survivor_path: '/music/Gorillaz/Humanz (2017)/03 - Saturnz Barz.mp3', survivor_path: '/music/Gorillaz/Humanz (2017)/03 - Saturnz Barz.mp3',
survivor_reason: 'the copy Lidarr tracks', survivor_reason: 'the copy that is not a video rip',
removed: [{ track_id: 't-2', file_path: '/music/Gorillaz/Humanz (2017)/03 - Saturnz Barz (Official Video).mp3' }] removed: [{ track_id: 't-2', file_path: '/music/Gorillaz/Humanz (2017)/03 - Saturnz Barz (Official Video).mp3' }]
} }
}, },
@@ -358,47 +359,26 @@ describe('admin duplicates', () => {
]); ]);
await fireEvent.click(screen.getByRole('tab', { name: /resolved automatically/i })); await fireEvent.click(screen.getByRole('tab', { name: /resolved automatically/i }));
const items = screen.getAllByTestId('resolved-item').map(text); const items = screen.getAllByTestId('resolved-item').map(text);
expect(items[0]).toContain('Kept 03 - Saturnz Barz.mp3 (the copy Lidarr tracks) and removed 1 file'); expect(items[0]).toContain('Kept 03 - Saturnz Barz.mp3 (the copy that is not a video rip) and removed 1 file.');
expect(items[1]).toContain('Lidarr now monitors Digital Media (40 tracks) of Humanz by Gorillaz'); expect(items[1]).toContain('Lidarr now monitors Digital Media (40 tracks) of Humanz by Gorillaz');
expect(screen.queryByTestId('duplicate-group')).toBeNull(); expect(screen.queryByTestId('duplicate-group')).toBeNull();
}); });
}); });
describe("Lidarr's view of each copy", () => { describe("the resolver's note", () => {
function tracked() { test('shows above the group it explains', () => {
const r = response(); const r = response();
r.groups[0] = { r.groups[0] = { ...r.groups[0], resolve_note: 'Identical audio filed under different titles.' };
...r.groups[0], renderWith(r);
resolve_note: 'Lidarr tracks every copy as its own track.', expect(text(screen.getByTestId('resolve-note'))).toBe('Identical audio filed under different titles.');
members: [member('www-01', { lidarr_state: 'tracked' }), member('www-02', { lidarr_state: 'unmapped' })]
};
return r;
}
test('each copy says whether Lidarr uses it, and the resolver note shows', () => {
renderWith(tracked());
expect(screen.getAllByTestId('lidarr-state').map(text)).toEqual([
'Lidarr uses this',
"Lidarr doesn't use this"
]);
expect(text(screen.getByTestId('resolve-note'))).toBe('Lidarr tracks every copy as its own track.');
}); });
// Keeping the unmapped copy would remove the one Lidarr uses. The server test('a merge confirms and sends the copy chosen to keep', async () => {
// refuses that; the confirm says so first and offers no button to press. renderWith(response());
test('a merge that would remove the copy Lidarr uses is blocked before it is sent', async () => {
renderWith(tracked());
await fireEvent.click(screen.getAllByRole('radio')[1]); await fireEvent.click(screen.getAllByRole('radio')[1]);
await fireEvent.click(screen.getByRole('button', { name: 'Merge…' })); await fireEvent.click(screen.getByRole('button', { name: 'Merge…' }));
expect(text(screen.getByTestId('merge-blocked'))).toContain('Lidarr uses a copy this would remove');
const confirm = screen.getByRole('button', { name: 'Remove 1 file and merge' }) as HTMLButtonElement;
expect(confirm.disabled).toBe(true);
// Choosing the copy Lidarr uses clears it.
await fireEvent.click(screen.getAllByRole('radio')[0]);
expect(screen.queryByTestId('merge-blocked')).toBeNull();
await fireEvent.click(screen.getByRole('button', { name: 'Remove 1 file and merge' })); await fireEvent.click(screen.getByRole('button', { name: 'Remove 1 file and merge' }));
expect(mergeDuplicateGroup).toHaveBeenCalledWith('g-1', { survivor_track_id: 'www-01', unmonitor: false }); expect(mergeDuplicateGroup).toHaveBeenCalledWith('g-1', { survivor_track_id: 'www-02', unmonitor: false });
}); });
}); });
}); });