fix(lidarr): complete a request only when the album actually came back (#5263)
release / govulncheck (push) Failing after 33s
release / web (push) Successful in 1m21s
release / go (push) Successful in 1m44s
release / integration (push) Successful in 4m37s
release / android (push) Successful in 5m29s
release / Build signed APK (releases and dev) (push) Successful in 5m34s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Skipped
release / Verify release artifacts (tag releases only) (push) Skipped
release / govulncheck (push) Failing after 33s
release / web (push) Successful in 1m21s
release / go (push) Successful in 1m44s
release / integration (push) Successful in 4m37s
release / android (push) Successful in 5m29s
release / Build signed APK (releases and dev) (push) Successful in 5m34s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Skipped
release / Verify release artifacts (tag releases only) (push) Skipped
Re-acquisition targets albums with ANY track missing, and completion only asked for a track on disk, so the tracks that never left completed every re-acquisition request the moment Lidarr accepted the add (52 on the deploy, each ~150ms after its add). An album or track request now completes when an album named by its release or group has a track on disk AND either a track arrived after the request (a new album, or Lidarr fetching another release into its own row) or no track that was missing at the request is still missing. added_at is the arrival clock; updated_at moves on every tag re-read. Migration 0071 reopens completed album/track requests whose matched album fails that test, as approved with the match cleared; the Lidarr add stays confirmed, so nothing is re-sent. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -3,12 +3,17 @@ package lidarrrequests
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"io/fs"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"sync"
|
||||
"testing"
|
||||
|
||||
"github.com/jackc/pgx/v5/pgtype"
|
||||
"github.com/jackc/pgx/v5/pgxpool"
|
||||
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/db"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/lidarr"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/lidarrconfig"
|
||||
@@ -201,3 +206,168 @@ func TestReconciler_RepointsAReleaseIdRequestAtItsReleaseGroup(t *testing.T) {
|
||||
t.Errorf("MusicBrainz asked again on the next tick (%d -> %d)", before, asked)
|
||||
}
|
||||
}
|
||||
|
||||
// markMissing stamps a track missing since well before any request a test makes.
|
||||
func markMissing(t *testing.T, pool *pgxpool.Pool, trackID pgtype.UUID) {
|
||||
t.Helper()
|
||||
if _, err := pool.Exec(context.Background(),
|
||||
"UPDATE tracks SET missing_since = now() - interval '2 days' WHERE id = $1", trackID); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
|
||||
func requestStatus(t *testing.T, q *dbq.Queries, id pgtype.UUID) dbq.LidarrRequest {
|
||||
t.Helper()
|
||||
got, err := q.GetLidarrRequestByID(context.Background(), id)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return got
|
||||
}
|
||||
|
||||
// #5263: re-acquisition targets albums with SOME tracks missing. The tracks
|
||||
// that never left must not complete the request; it completes once the lost
|
||||
// track is back.
|
||||
func TestReconciler_PartlyMissingAlbumCompletesWhenTheLostTrackReturns(t *testing.T) {
|
||||
pool := newPool(t)
|
||||
q := dbq.New(pool)
|
||||
ctx := context.Background()
|
||||
enableLidarrForPool(t, pool)
|
||||
user := seedUser(t, pool)
|
||||
|
||||
const artistMBID, release = "rg-artist-partial", "rg-release-partial"
|
||||
artist := seedArtist(t, q, "Partial Artist", artistMBID)
|
||||
album := seedAlbum(t, q, artist.ID, "Partial Album", release)
|
||||
_ = seedTrack(t, q, album.ID, artist.ID, "Kept", "/music/rg-partial/01.flac")
|
||||
lost := seedTrack(t, q, album.ID, artist.ID, "Lost", "/music/rg-partial/02.flac")
|
||||
markMissing(t, pool, lost.ID)
|
||||
|
||||
req := seedApprovedRequestDirect(t, q, user, CreateParams{
|
||||
Kind: "album", LidarrArtistMBID: artistMBID, ArtistName: "Partial Artist",
|
||||
LidarrAlbumMBID: release, AlbumTitle: "Partial Album",
|
||||
})
|
||||
rec := NewReconciler(pool, lidarrconfig.New(pool), nil, newTestLogger(), nil)
|
||||
if err := rec.tickOnce(ctx); err != nil {
|
||||
t.Fatalf("tickOnce: %v", err)
|
||||
}
|
||||
if got := requestStatus(t, q, req.ID); got.Status != dbq.LidarrRequestStatusApproved {
|
||||
t.Fatalf("status = %v while a track is still missing, want approved", got.Status)
|
||||
}
|
||||
|
||||
if _, err := pool.Exec(ctx, "UPDATE tracks SET missing_since = NULL WHERE id = $1", lost.ID); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := rec.tickOnce(ctx); err != nil {
|
||||
t.Fatalf("second tick: %v", err)
|
||||
}
|
||||
got := requestStatus(t, q, req.ID)
|
||||
if got.Status != dbq.LidarrRequestStatusCompleted || got.MatchedAlbumID != album.ID {
|
||||
t.Errorf("status %v matched %v after the track returned, want completed on %v",
|
||||
got.Status, got.MatchedAlbumID, album.ID)
|
||||
}
|
||||
}
|
||||
|
||||
// Lidarr may fetch a different release of the group, which the scanner files
|
||||
// as its own album row. A track arriving there after the request completes it,
|
||||
// even though the old row's lost tracks stay missing.
|
||||
func TestReconciler_PartlyMissingAlbumCompletesOnAnotherReleaseArriving(t *testing.T) {
|
||||
pool := newPool(t)
|
||||
q := dbq.New(pool)
|
||||
ctx := context.Background()
|
||||
enableLidarrForPool(t, pool)
|
||||
user := seedUser(t, pool)
|
||||
|
||||
const artistMBID, oldRelease, newRelease, group = "rg-artist-edition", "rg-release-old", "rg-release-new", "rg-group-edition"
|
||||
artist := seedArtist(t, q, "Edition Artist", artistMBID)
|
||||
old := seedAlbum(t, q, artist.ID, "Edition Album", oldRelease)
|
||||
if err := q.SetAlbumReleaseGroupMbidIfNull(ctx, dbq.SetAlbumReleaseGroupMbidIfNullParams{
|
||||
ID: old.ID, ReleaseGroupMbid: nilableStr(group),
|
||||
}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
_ = seedTrack(t, q, old.ID, artist.ID, "Kept", "/music/rg-edition-old/01.flac")
|
||||
lost := seedTrack(t, q, old.ID, artist.ID, "Lost", "/music/rg-edition-old/02.flac")
|
||||
markMissing(t, pool, lost.ID)
|
||||
|
||||
req := seedApprovedRequestDirect(t, q, user, CreateParams{
|
||||
Kind: "album", LidarrArtistMBID: artistMBID, ArtistName: "Edition Artist",
|
||||
LidarrAlbumMBID: group, AlbumTitle: "Edition Album",
|
||||
})
|
||||
rec := NewReconciler(pool, lidarrconfig.New(pool), nil, newTestLogger(), nil)
|
||||
if err := rec.tickOnce(ctx); err != nil {
|
||||
t.Fatalf("tickOnce: %v", err)
|
||||
}
|
||||
if got := requestStatus(t, q, req.ID); got.Status != dbq.LidarrRequestStatusApproved {
|
||||
t.Fatalf("status = %v before anything arrived, want approved", got.Status)
|
||||
}
|
||||
|
||||
arrived := seedAlbum(t, q, artist.ID, "Edition Album (Deluxe)", newRelease)
|
||||
if err := q.SetAlbumReleaseGroupMbidIfNull(ctx, dbq.SetAlbumReleaseGroupMbidIfNullParams{
|
||||
ID: arrived.ID, ReleaseGroupMbid: nilableStr(group),
|
||||
}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
_ = seedTrack(t, q, arrived.ID, artist.ID, "Lost", "/music/rg-edition-new/02.flac")
|
||||
if err := rec.tickOnce(ctx); err != nil {
|
||||
t.Fatalf("second tick: %v", err)
|
||||
}
|
||||
got := requestStatus(t, q, req.ID)
|
||||
if got.Status != dbq.LidarrRequestStatusCompleted || got.MatchedAlbumID != arrived.ID {
|
||||
t.Errorf("status %v matched %v, want completed on the arrived release %v",
|
||||
got.Status, got.MatchedAlbumID, arrived.ID)
|
||||
}
|
||||
}
|
||||
|
||||
// Migration 0071 reopens requests the old check completed against a partly
|
||||
// missing album, and leaves a genuine completion alone.
|
||||
func TestMigration0071_ReopensFalseCompletions(t *testing.T) {
|
||||
pool := newPool(t)
|
||||
q := dbq.New(pool)
|
||||
ctx := context.Background()
|
||||
user := seedUser(t, pool)
|
||||
|
||||
artist := seedArtist(t, q, "Reopen Artist", "rg-artist-reopen")
|
||||
partial := seedAlbum(t, q, artist.ID, "Partial", "rg-release-reopen-partial")
|
||||
_ = seedTrack(t, q, partial.ID, artist.ID, "Kept", "/music/rg-reopen-partial/01.flac")
|
||||
lost := seedTrack(t, q, partial.ID, artist.ID, "Lost", "/music/rg-reopen-partial/02.flac")
|
||||
markMissing(t, pool, lost.ID)
|
||||
falseReq := seedApprovedRequestDirect(t, q, user, CreateParams{
|
||||
Kind: "album", LidarrArtistMBID: "rg-artist-reopen", ArtistName: "Reopen Artist",
|
||||
LidarrAlbumMBID: "rg-release-reopen-partial", AlbumTitle: "Partial",
|
||||
})
|
||||
|
||||
genuineReq := seedApprovedRequestDirect(t, q, user, CreateParams{
|
||||
Kind: "album", LidarrArtistMBID: "rg-artist-reopen", ArtistName: "Reopen Artist",
|
||||
LidarrAlbumMBID: "rg-release-reopen-new", AlbumTitle: "New",
|
||||
})
|
||||
arrived := seedAlbum(t, q, artist.ID, "New", "rg-release-reopen-new")
|
||||
_ = seedTrack(t, q, arrived.ID, artist.ID, "Fresh", "/music/rg-reopen-new/01.flac")
|
||||
|
||||
for _, c := range []struct {
|
||||
id pgtype.UUID
|
||||
album pgtype.UUID
|
||||
}{{falseReq.ID, partial.ID}, {genuineReq.ID, arrived.ID}} {
|
||||
if _, err := q.CompleteLidarrRequest(ctx, dbq.CompleteLidarrRequestParams{
|
||||
ID: c.id, MatchedAlbumID: c.album,
|
||||
}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
|
||||
up, err := fs.ReadFile(db.MigrationsFS(), "0071_reopen_false_reacquisition_completions.up.sql")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if _, err := pool.Exec(ctx, string(up)); err != nil {
|
||||
t.Fatalf("migration: %v", err)
|
||||
}
|
||||
|
||||
got := requestStatus(t, q, falseReq.ID)
|
||||
if got.Status != dbq.LidarrRequestStatusApproved || got.CompletedAt.Valid || got.MatchedAlbumID.Valid {
|
||||
t.Errorf("false completion: status %v completed_at %v matched %v, want approved and cleared",
|
||||
got.Status, got.CompletedAt, got.MatchedAlbumID)
|
||||
}
|
||||
if got := requestStatus(t, q, genuineReq.ID); got.Status != dbq.LidarrRequestStatusCompleted {
|
||||
t.Errorf("genuine completion reopened: status %v", got.Status)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user