feat: Lidarr searches for a better copy of a rip that has none (M498 #5447)
release / govulncheck (push) Successful in 18s
release / web (push) Successful in 1m26s
release / go (push) Successful in 1m45s
release / integration (push) Successful in 5m6s
release / android (push) Successful in 5m58s
release / Build signed APK (releases and dev) (push) Successful in 6m17s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Successful in 26s
release / Verify release artifacts (tag releases only) (push) Skipped
release / govulncheck (push) Successful in 18s
release / web (push) Successful in 1m26s
release / go (push) Successful in 1m45s
release / integration (push) Successful in 5m6s
release / android (push) Successful in 5m58s
release / Build signed APK (releases and dev) (push) Successful in 6m17s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Successful in 26s
release / Verify release artifacts (tag releases only) (push) Skipped
The operator chose option 2: an album holding a video rip that is the only copy of its song is searched in Lidarr (AlbumSearch), at most once a week and ten albums a pass. Lidarr grabs only when the quality profile allows an upgrade, so nothing is removed and the rip stays held back either way. - lidarr.Client.SearchAlbums posts the AlbumSearch command. - ListAlbumsWithSoleCopyRips finds the albums: a held-back rip with no present clean copy sharing its song or its pending group, and not searched in the last week (audit action lidarr_rip_search). - Suspect sources shows when Lidarr last searched the track's album. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -50,6 +50,7 @@ type LidarrLibrary interface {
|
||||
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 (
|
||||
@@ -94,6 +95,9 @@ type DuplicateResolveResult struct {
|
||||
// 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.
|
||||
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
|
||||
@@ -265,6 +269,13 @@ func ResolveDuplicates(
|
||||
return res, err
|
||||
}
|
||||
|
||||
// Last, rips with no clean copy anywhere: ask Lidarr for something better.
|
||||
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{
|
||||
Count: int64(res.Merged + len(changes)),
|
||||
@@ -334,6 +345,61 @@ func linkSong(ctx context.Context, pool *pgxpool.Pool, ids []pgtype.UUID) (bool,
|
||||
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 {
|
||||
var groups []*resolveGroup
|
||||
for _, r := range rows {
|
||||
@@ -787,5 +853,5 @@ func (w *DuplicateResolveWorker) tickOnce(ctx context.Context) {
|
||||
w.logger.Info("duplicate resolve complete",
|
||||
"groups", res.Groups, "lidarr", res.LidarrConsulted, "merged", res.Merged,
|
||||
"merge_failed", res.MergeFailed, "release_changes", len(res.ReleaseChanges), "songs_linked", res.SongsLinked,
|
||||
"held_back", res.HeldBack, "released", res.Released)
|
||||
"held_back", res.HeldBack, "released", res.Released, "rip_searches", res.RipSearches)
|
||||
}
|
||||
|
||||
@@ -82,6 +82,7 @@ type fakeLidarr struct {
|
||||
releases []lidarr.AlbumRelease
|
||||
releaseTracks map[int][]lidarr.ReleaseTrack
|
||||
setCalls [][2]int
|
||||
searchCalls [][]int
|
||||
}
|
||||
|
||||
func (f *fakeLidarr) ListUnmappedTrackFiles(context.Context) ([]lidarr.TrackFile, error) {
|
||||
@@ -116,6 +117,11 @@ func (f *fakeLidarr) ListReleaseTracks(_ context.Context, releaseID int) ([]lida
|
||||
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 {
|
||||
@@ -502,3 +508,60 @@ func TestResolveDuplicates_HoldsBackRips_Integration(t *testing.T) {
|
||||
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")
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user