diff --git a/internal/library/duplicate_resolve.go b/internal/library/duplicate_resolve.go index b4e79da5..7344b969 100644 --- a/internal/library/duplicate_resolve.go +++ b/internal/library/duplicate_resolve.go @@ -668,12 +668,35 @@ func changeRelease( 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 { - return nil, "", err + // 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 diff --git a/internal/library/duplicate_resolve_test.go b/internal/library/duplicate_resolve_test.go index 20778fc6..12df7e89 100644 --- a/internal/library/duplicate_resolve_test.go +++ b/internal/library/duplicate_resolve_test.go @@ -3,6 +3,7 @@ package library import ( "context" "errors" + "fmt" "os" "path/filepath" "testing" @@ -83,6 +84,9 @@ type fakeLidarr struct { 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) { @@ -127,7 +131,7 @@ func (f *fakeLidarr) SetMonitoredRelease(_ context.Context, albumID, releaseID i for i := range f.releases { f.releases[i].Monitored = f.releases[i].ID == releaseID } - return nil + return f.setErr } // resolveFixture is the Humanz shape: one album, one song twice, both copies on @@ -565,3 +569,60 @@ func TestResolveDuplicates_SearchesForASoleCopyRip_Integration(t *testing.T) { 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) + } + 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)) + } + var audited int + if err := f.pool.QueryRow(context.Background(), + `SELECT count(*) FROM audit_log WHERE action = 'lidarr_release_change'`).Scan(&audited); err != nil { + t.Fatal(err) + } + if want := map[bool]int{true: 1, false: 0}[landed]; audited != want { + t.Errorf("audited %d release changes, want %d", audited, 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 +}