feat(lidarr): ask Lidarr for release groups; repair stored release-id requests (M483 #5244)
release / go (push) Successful in 2m33s
release / web (push) Successful in 1m41s
release / govulncheck (push) Successful in 22s
release / integration (push) Successful in 6m0s
release / android (push) Successful in 6m7s
release / Build signed APK (releases and dev) (push) Successful in 6m8s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Successful in 1m43s
release / Verify release artifacts (tag releases only) (push) Skipped
release / go (push) Successful in 2m33s
release / web (push) Successful in 1m41s
release / govulncheck (push) Successful in 22s
release / integration (push) Successful in 6m0s
release / android (push) Successful in 6m7s
release / Build signed APK (releases and dev) (push) Successful in 6m8s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Successful in 1m43s
release / Verify release artifacts (tag releases only) (push) Skipped
Lidarr's metadata is keyed by MusicBrainz release group, but re-acquisition requested albums by their release id, so every add came back "not found". - Sweeper requests an album by its tag-supplied release group, else the one MusicBrainz names (cached onto the album). An album MusicBrainz cannot name is skipped and counted, with no attempt spent. - Reconciler: an add refused as not found re-reads the request's album id as a release (library first, then MusicBrainz), rewrites the request to the group and adds again. This repairs the requests already stored. - Completion matches an album by release id or release group, and only once a track of it is on disk, so a re-acquisition request no longer completes against the row of the album it is trying to bring back. Closes #5241. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -13,6 +13,7 @@ import (
|
||||
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/lidarrrequests"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/tags"
|
||||
)
|
||||
|
||||
// requestCreator is the slice of lidarrrequests.Service the sweeper needs,
|
||||
@@ -37,6 +38,10 @@ type Sweeper struct {
|
||||
requests requestCreator
|
||||
logger *slog.Logger
|
||||
tick time.Duration
|
||||
// releaseGroup names the MusicBrainz release group of a release id, for an
|
||||
// album whose tags never carried one (#5241). Lidarr knows albums only by
|
||||
// release group. A field so tests can stand in for MusicBrainz.
|
||||
releaseGroup func(ctx context.Context, releaseMBID string) (string, error)
|
||||
}
|
||||
|
||||
// NewSweeper constructs a Sweeper. The tick is deliberately coarse: the
|
||||
@@ -49,11 +54,12 @@ func NewSweeper(
|
||||
logger *slog.Logger,
|
||||
) *Sweeper {
|
||||
return &Sweeper{
|
||||
pool: pool,
|
||||
settings: settings,
|
||||
requests: requests,
|
||||
logger: logger,
|
||||
tick: 1 * time.Hour,
|
||||
pool: pool,
|
||||
settings: settings,
|
||||
requests: requests,
|
||||
logger: logger,
|
||||
tick: 1 * time.Hour,
|
||||
releaseGroup: tags.ReleaseGroupForRelease,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -80,6 +86,10 @@ type PassResult struct {
|
||||
Approved int // of those, sent on to Lidarr
|
||||
GaveUp int // albums that spent their attempt budget
|
||||
Unnameable int64 // albums with missing files but no MBID to ask for
|
||||
// Unresolved: albums skipped this pass because MusicBrainz could not name
|
||||
// their release group (switched off, or no such release). Lidarr cannot be
|
||||
// asked for them, and no attempt is spent; the next pass tries again.
|
||||
Unresolved int
|
||||
}
|
||||
|
||||
// SweepOnce runs one pass. Exported so the admin surface can offer a "run
|
||||
@@ -160,11 +170,23 @@ func (s *Sweeper) attempt(
|
||||
return nil
|
||||
}
|
||||
|
||||
// Lidarr names albums by release group; albums.mbid is the release.
|
||||
group, err := s.albumReleaseGroup(ctx, q, album)
|
||||
if err != nil {
|
||||
if errors.Is(err, tags.ErrNotFound) {
|
||||
res.Unresolved++
|
||||
s.logger.Info("reacquisition: no MusicBrainz release group for album; skipped",
|
||||
"album", album.AlbumTitle, "release_mbid", *album.AlbumMbid)
|
||||
return nil
|
||||
}
|
||||
return fmt.Errorf("release group: %w", err)
|
||||
}
|
||||
|
||||
req, err := s.requests.Create(ctx, adminID, lidarrrequests.CreateParams{
|
||||
Kind: "album",
|
||||
LidarrArtistMBID: *album.ArtistMbid,
|
||||
ArtistName: album.ArtistName,
|
||||
LidarrAlbumMBID: *album.AlbumMbid,
|
||||
LidarrAlbumMBID: group,
|
||||
AlbumTitle: album.AlbumTitle,
|
||||
})
|
||||
if err != nil {
|
||||
@@ -212,10 +234,31 @@ func (s *Sweeper) attempt(
|
||||
return nil
|
||||
}
|
||||
|
||||
// albumReleaseGroup is the album's release group: the one its tags carried,
|
||||
// else MusicBrainz's answer for its release id, cached onto the album so the
|
||||
// next pass (and request completion) need not ask again. Returns
|
||||
// tags.ErrNotFound when MusicBrainz cannot name it.
|
||||
func (s *Sweeper) albumReleaseGroup(ctx context.Context, q *dbq.Queries, album dbq.ListAlbumsDueReacquisitionRow) (string, error) {
|
||||
if album.AlbumReleaseGroupMbid != nil && *album.AlbumReleaseGroupMbid != "" {
|
||||
return *album.AlbumReleaseGroupMbid, nil
|
||||
}
|
||||
group, err := s.releaseGroup(ctx, *album.AlbumMbid)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
if cerr := q.SetAlbumReleaseGroupMbidIfNull(ctx, dbq.SetAlbumReleaseGroupMbidIfNullParams{
|
||||
ID: album.AlbumID,
|
||||
ReleaseGroupMbid: &group,
|
||||
}); cerr != nil {
|
||||
s.logger.Warn("reacquisition: cache release group failed", "album", album.AlbumTitle, "err", cerr)
|
||||
}
|
||||
return group, nil
|
||||
}
|
||||
|
||||
func (s *Sweeper) logSummary(res PassResult) {
|
||||
// Silence when a pass did nothing at all — this runs hourly forever, and
|
||||
// an unconditional line would bury the passes that mattered.
|
||||
if res.Requested == 0 && res.Cleared == 0 && res.GaveUp == 0 {
|
||||
if res.Requested == 0 && res.Cleared == 0 && res.GaveUp == 0 && res.Unresolved == 0 {
|
||||
return
|
||||
}
|
||||
s.logger.Info("reacquisition: sweep",
|
||||
@@ -224,5 +267,6 @@ func (s *Sweeper) logSummary(res PassResult) {
|
||||
"gave_up", res.GaveUp,
|
||||
"cleared", res.Cleared,
|
||||
"unnameable", res.Unnameable,
|
||||
"unresolved", res.Unresolved,
|
||||
)
|
||||
}
|
||||
|
||||
@@ -0,0 +1,163 @@
|
||||
package reacquisition
|
||||
|
||||
import (
|
||||
"context"
|
||||
"io"
|
||||
"log/slog"
|
||||
"os"
|
||||
"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/dbtest"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/lidarrconfig"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/lidarrrequests"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/tags"
|
||||
)
|
||||
|
||||
func newSweepPool(t *testing.T) *pgxpool.Pool {
|
||||
t.Helper()
|
||||
if testing.Short() {
|
||||
t.Skip("skipping integration test in -short mode")
|
||||
}
|
||||
dsn := os.Getenv("MINSTREL_TEST_DATABASE_URL")
|
||||
if dsn == "" {
|
||||
t.Skip("MINSTREL_TEST_DATABASE_URL not set")
|
||||
}
|
||||
if err := db.Migrate(dsn, slog.New(slog.NewTextHandler(io.Discard, nil))); err != nil {
|
||||
t.Fatalf("migrate: %v", err)
|
||||
}
|
||||
pool, err := pgxpool.New(context.Background(), dsn)
|
||||
if err != nil {
|
||||
t.Fatalf("pool: %v", err)
|
||||
}
|
||||
t.Cleanup(pool.Close)
|
||||
dbtest.ResetDB(t, pool)
|
||||
if _, err := pool.Exec(context.Background(), "DELETE FROM lidarr_requests"); err != nil {
|
||||
t.Fatalf("reset requests: %v", err)
|
||||
}
|
||||
return pool
|
||||
}
|
||||
|
||||
// lostAlbum seeds an album whose only track has been missing for two days,
|
||||
// named by MusicBrainz release id and (when group is set) release group.
|
||||
func lostAlbum(t *testing.T, pool *pgxpool.Pool, title, release, group string) pgtype.UUID {
|
||||
t.Helper()
|
||||
ctx := context.Background()
|
||||
q := dbq.New(pool)
|
||||
artistMBID := "artist-" + release
|
||||
ar, err := q.UpsertArtist(ctx, dbq.UpsertArtistParams{Name: title + " Artist", SortName: title, Mbid: &artistMBID})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
params := dbq.UpsertAlbumParams{Title: title, SortTitle: title, ArtistID: ar.ID, Mbid: &release}
|
||||
if group != "" {
|
||||
params.ReleaseGroupMbid = &group
|
||||
}
|
||||
al, err := q.UpsertAlbum(ctx, params)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
tr, err := q.UpsertTrack(ctx, dbq.UpsertTrackParams{
|
||||
Title: "Lost", AlbumID: al.ID, ArtistID: ar.ID, DurationMs: 180000,
|
||||
FilePath: "/music/" + release + "/01.flac", FileSize: 1, FileFormat: "flac",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if _, err := pool.Exec(ctx, "UPDATE tracks SET missing_since = now() - interval '2 days' WHERE id = $1", tr.ID); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return al.ID
|
||||
}
|
||||
|
||||
// #5241: re-acquisition asks Lidarr for the album's release group, never its
|
||||
// release id. A tag-supplied group is used as is; otherwise MusicBrainz names
|
||||
// it and the answer is cached onto the album; an album MusicBrainz cannot
|
||||
// name is skipped without spending an attempt.
|
||||
func TestSweep_RequestsAlbumsByReleaseGroup_Integration(t *testing.T) {
|
||||
pool := newSweepPool(t)
|
||||
ctx := context.Background()
|
||||
logger := slog.New(slog.NewTextHandler(io.Discard, nil))
|
||||
q := dbq.New(pool)
|
||||
|
||||
if _, err := q.CreateUser(ctx, dbq.CreateUserParams{
|
||||
Username: dbtest.TestUserPrefix + "sweepadmin", PasswordHash: "x", ApiTokenHash: "x", IsAdmin: true,
|
||||
}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
settings, err := NewSettingsService(ctx, pool, logger)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
cfg := Defaults
|
||||
cfg.AutoApprove = false
|
||||
if _, err := settings.Set(ctx, cfg); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
tagged := lostAlbum(t, pool, "Tagged", "release-tagged", "group-tagged")
|
||||
resolved := lostAlbum(t, pool, "Resolved", "release-resolved", "")
|
||||
unknown := lostAlbum(t, pool, "Unknown", "release-unknown", "")
|
||||
|
||||
requests := lidarrrequests.NewService(pool, lidarrconfig.New(pool), nil, nil)
|
||||
s := NewSweeper(pool, settings, requests, logger)
|
||||
asked := map[string]int{}
|
||||
s.releaseGroup = func(_ context.Context, release string) (string, error) {
|
||||
asked[release]++
|
||||
if release == "release-resolved" {
|
||||
return "group-resolved", nil
|
||||
}
|
||||
return "", tags.ErrNotFound
|
||||
}
|
||||
if err := s.SweepOnce(ctx); err != nil {
|
||||
t.Fatalf("sweep: %v", err)
|
||||
}
|
||||
|
||||
requested := func(album pgtype.UUID) (string, bool) {
|
||||
t.Helper()
|
||||
var mbid *string
|
||||
err := pool.QueryRow(ctx, `
|
||||
SELECT lr.lidarr_album_mbid
|
||||
FROM missing_reacquisitions r
|
||||
JOIN lidarr_requests lr ON lr.id = r.last_request_id
|
||||
WHERE r.album_id = $1`, album).Scan(&mbid)
|
||||
if err != nil || mbid == nil {
|
||||
return "", false
|
||||
}
|
||||
return *mbid, true
|
||||
}
|
||||
if got, ok := requested(tagged); !ok || got != "group-tagged" {
|
||||
t.Errorf("tagged album requested as %q (ok=%v), want group-tagged", got, ok)
|
||||
}
|
||||
if asked["release-tagged"] != 0 {
|
||||
t.Error("MusicBrainz asked about an album whose tags named its group")
|
||||
}
|
||||
if got, ok := requested(resolved); !ok || got != "group-resolved" {
|
||||
t.Errorf("resolved album requested as %q (ok=%v), want group-resolved", got, ok)
|
||||
}
|
||||
var cached *string
|
||||
if err := pool.QueryRow(ctx, "SELECT release_group_mbid FROM albums WHERE id = $1", resolved).Scan(&cached); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if cached == nil || *cached != "group-resolved" {
|
||||
t.Errorf("resolved group not cached on the album: %v", cached)
|
||||
}
|
||||
var attempts int
|
||||
if err := pool.QueryRow(ctx, "SELECT count(*) FROM missing_reacquisitions WHERE album_id = $1", unknown).Scan(&attempts); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if attempts != 0 {
|
||||
t.Errorf("unresolvable album spent an attempt (%d state rows)", attempts)
|
||||
}
|
||||
var stray int
|
||||
if err := pool.QueryRow(ctx, "SELECT count(*) FROM lidarr_requests WHERE lidarr_album_mbid LIKE 'release-%'").Scan(&stray); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if stray != 0 {
|
||||
t.Errorf("%d request(s) named an album by its release id", stray)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user