feat: a wanted list — follow artists on release-type profiles; requests and re-acquisition become wants (M502 #5520)
release / govulncheck (push) Successful in 15s
release / web (push) Failing after 1m21s
release / go (push) Successful in 1m35s
release / integration (push) Failing after 4m57s
release / android (push) Successful in 5m11s
release / Build signed APK (releases and dev) (push) Successful in 5m28s
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) Successful in 15s
release / web (push) Failing after 1m21s
release / go (push) Successful in 1m35s
release / integration (push) Failing after 4m57s
release / android (push) Successful in 5m11s
release / Build signed APK (releases and dev) (push) Successful in 5m28s
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
Minstrel keeps its own list of what it is to get, keyed by MusicBrainz release group. A want comes from an approved request, a new release by a followed artist, or an album whose files went missing, and is held once the library has a full edition of it. - Migration 0081: release_type_profiles (Standard seeded: Album, EP, Single, studio only), follows, follow_seen_groups and wants. lidarr_requests becomes requests with plain *_mbid columns; approved album and track requests move onto the wanted list, approved artist requests become follows, and the library's album artists start out followed. missing_reacquisitions and the retry settings go. - internal/wanted: follows are checked against MusicBrainz discographies daily. The first check sets a baseline, and only releases after it are wanted unless the follow asked for what already exists. Profiles filter by primary and secondary type, so live recordings stay out unless a profile ticks Live. - internal/requests (was lidarrrequests): approving puts the request on the wanted list, and an artist request can be approved on a chosen profile. A request completes when its want is held. - The re-acquisition sweeper wants a missing album back instead of asking Lidarr; searching for it is the search schedule's (#5521). - Web: an admin Wanted page (Wanted, Missing files, Held, Following), a follow control on artist pages, a release-type profiles card on Integrations, "Approve on a profile" on Requests, and want state on Missing files. The artist detail API returns the artist's MBID. - Android: the request body sends artist_mbid, album_mbid and track_mbid. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -7,61 +7,38 @@ import (
|
||||
"log/slog"
|
||||
"time"
|
||||
|
||||
"github.com/google/uuid"
|
||||
"github.com/jackc/pgx/v5"
|
||||
"github.com/jackc/pgx/v5/pgtype"
|
||||
"github.com/jackc/pgx/v5/pgxpool"
|
||||
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/lidarrrequests"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/notifications"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/tags"
|
||||
)
|
||||
|
||||
// requestCreator is the slice of lidarrrequests.Service the sweeper needs,
|
||||
// narrowed to an interface so the pass can be tested without a Lidarr client
|
||||
// or an approval path that talks to one.
|
||||
type requestCreator interface {
|
||||
CreateTracked(ctx context.Context, userID pgtype.UUID, p lidarrrequests.CreateParams) (dbq.LidarrRequest, bool, error)
|
||||
Approve(ctx context.Context, requestID, adminID pgtype.UUID, ov lidarrrequests.ApproveOverrides) (dbq.LidarrRequest, error)
|
||||
}
|
||||
|
||||
// Sweeper periodically turns albums with long-missing files into Lidarr
|
||||
// requests (milestone #290).
|
||||
// Sweeper periodically wants back albums whose files have been missing past
|
||||
// the grace window (milestone #290, M502 #5520). Searching for them is the
|
||||
// search schedule's (#5521), which retries per want.
|
||||
//
|
||||
// Deliberately a periodic worker rather than a hook inside the scanner's
|
||||
// reconcile pass. Reconcile runs inside a scan and has no business deciding
|
||||
// to talk to a third-party service; it also re-runs often, which would make
|
||||
// "attempt once, then back off" awkward to express. A worker paces itself,
|
||||
// survives a restart, and retries without needing another scan.
|
||||
// reconcile pass: reconcile runs inside a scan and re-runs often, and a
|
||||
// filesystem lies transiently (an unmounted volume, a NAS mid-reboot) in ways
|
||||
// the grace window exists to ride out.
|
||||
type Sweeper struct {
|
||||
pool *pgxpool.Pool
|
||||
settings *SettingsService
|
||||
requests requestCreator
|
||||
logger *slog.Logger
|
||||
// notifier tells admins about a request the sweeper filed that still
|
||||
// needs their approval (M489). Nil: nobody is told.
|
||||
notifier *notifications.Notifier
|
||||
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.
|
||||
// album whose tags never carried one (#5241). Wants are kept 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
|
||||
// smallest meaningful backoff is measured in hours, so waking more often than
|
||||
// hourly would only re-read settings and find nothing due.
|
||||
func NewSweeper(
|
||||
pool *pgxpool.Pool,
|
||||
settings *SettingsService,
|
||||
requests requestCreator,
|
||||
logger *slog.Logger,
|
||||
) *Sweeper {
|
||||
// NewSweeper constructs a Sweeper. The tick is deliberately coarse: the grace
|
||||
// window is measured in hours.
|
||||
func NewSweeper(pool *pgxpool.Pool, settings *SettingsService, logger *slog.Logger) *Sweeper {
|
||||
return &Sweeper{
|
||||
pool: pool,
|
||||
settings: settings,
|
||||
requests: requests,
|
||||
logger: logger,
|
||||
tick: 1 * time.Hour,
|
||||
releaseGroup: tags.ReleaseGroupForRelease,
|
||||
@@ -86,100 +63,49 @@ func (s *Sweeper) Run(ctx context.Context) {
|
||||
|
||||
// PassResult reports what a single sweep did, for logging and tests.
|
||||
type PassResult struct {
|
||||
Cleared int64 // albums whose files came back; state dropped
|
||||
Requested int // requests created this pass
|
||||
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
|
||||
Wanted int // albums newly wanted, or wanted again after being held
|
||||
Unnameable int64 // albums with missing files but no MBID to want them by
|
||||
// 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.
|
||||
// their release group; the next pass tries again.
|
||||
Unresolved int
|
||||
}
|
||||
|
||||
// SetNotifier makes requests the sweeper files, and cannot approve itself,
|
||||
// reach the admins' notifications inbox (M489).
|
||||
func (s *Sweeper) SetNotifier(n *notifications.Notifier) { s.notifier = n }
|
||||
|
||||
// SweepOnce runs one pass. Exported so the admin surface can offer a "run
|
||||
// now" without waiting out the tick, and so tests drive it directly.
|
||||
// SweepOnce runs one pass. Exported so tests drive it directly.
|
||||
func (s *Sweeper) SweepOnce(ctx context.Context) error {
|
||||
cfg := s.settings.Get()
|
||||
if !cfg.Enabled {
|
||||
return nil
|
||||
}
|
||||
q := dbq.New(s.pool)
|
||||
|
||||
// Before selecting work: drop state for albums whose files came back, or
|
||||
// were adopted at a new path (#2528). Doing this first means a recovered
|
||||
// album cannot be picked in the same pass that would have retried it.
|
||||
cleared, err := q.ClearRecoveredReacquisitions(ctx)
|
||||
if err != nil {
|
||||
return fmt.Errorf("clear recovered: %w", err)
|
||||
}
|
||||
res := PassResult{Cleared: cleared}
|
||||
|
||||
var res PassResult
|
||||
// Counted, not acted on: an album MusicBrainz cannot name is not a
|
||||
// failure to retry, it is a permanent gap the operator should see.
|
||||
if n, cerr := q.CountAlbumsMissingWithoutMbid(ctx); cerr == nil {
|
||||
res.Unnameable = n
|
||||
}
|
||||
|
||||
due, err := q.ListAlbumsDueReacquisition(ctx, dbq.ListAlbumsDueReacquisitionParams{
|
||||
GraceHours: cfg.GraceHours,
|
||||
BackoffBaseHours: cfg.BackoffBaseHours,
|
||||
BackoffMaxHours: cfg.BackoffMaxHours,
|
||||
PageLimit: cfg.MaxPerPass,
|
||||
GraceHours: cfg.GraceHours,
|
||||
PageLimit: cfg.MaxPerPass,
|
||||
})
|
||||
if err != nil {
|
||||
return fmt.Errorf("list due: %w", err)
|
||||
}
|
||||
if len(due) == 0 {
|
||||
s.logSummary(res)
|
||||
return nil
|
||||
}
|
||||
|
||||
// One admin lookup per pass, not per album. A library with no admin at
|
||||
// all cannot own a request, so the pass stops rather than half-working.
|
||||
admin, err := q.GetOldestAdmin(ctx)
|
||||
if err != nil {
|
||||
if errors.Is(err, pgx.ErrNoRows) {
|
||||
s.logger.Warn("reacquisition: no admin account to own requests; skipping pass")
|
||||
return nil
|
||||
}
|
||||
return fmt.Errorf("owner lookup: %w", err)
|
||||
}
|
||||
|
||||
for _, album := range due {
|
||||
if err := s.attempt(ctx, q, cfg, admin.ID, album, &res); err != nil {
|
||||
// One album's failure must not abandon the rest of the pass: a
|
||||
// single unmatched MBID or a transient Lidarr error says nothing
|
||||
// about the next album in the list.
|
||||
s.logger.Warn("reacquisition: attempt failed",
|
||||
"album", album.AlbumTitle, "err", err)
|
||||
if err := s.want(ctx, q, album, &res); err != nil {
|
||||
// One album's failure must not abandon the rest of the pass.
|
||||
s.logger.Warn("reacquisition: want failed", "album", album.AlbumTitle, "err", err)
|
||||
}
|
||||
}
|
||||
s.logSummary(res)
|
||||
return nil
|
||||
}
|
||||
|
||||
// attempt creates (and optionally approves) the request for one album, then
|
||||
// records the attempt against its backoff budget.
|
||||
func (s *Sweeper) attempt(
|
||||
ctx context.Context,
|
||||
q *dbq.Queries,
|
||||
cfg Settings,
|
||||
adminID pgtype.UUID,
|
||||
album dbq.ListAlbumsDueReacquisitionRow,
|
||||
res *PassResult,
|
||||
) error {
|
||||
// The query already filters these out; belt and braces, because Create
|
||||
// would reject the request and burn an attempt for no reason.
|
||||
// want records one album as wanted under its release group.
|
||||
func (s *Sweeper) want(ctx context.Context, q *dbq.Queries, album dbq.ListAlbumsDueReacquisitionRow, res *PassResult) error {
|
||||
// The query already filters these out; belt and braces.
|
||||
if album.AlbumMbid == nil || album.ArtistMbid == nil {
|
||||
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) {
|
||||
@@ -190,77 +116,25 @@ func (s *Sweeper) attempt(
|
||||
}
|
||||
return fmt.Errorf("release group: %w", err)
|
||||
}
|
||||
|
||||
req, created, err := s.requests.CreateTracked(ctx, adminID, lidarrrequests.CreateParams{
|
||||
Kind: "album",
|
||||
LidarrArtistMBID: *album.ArtistMbid,
|
||||
ArtistName: album.ArtistName,
|
||||
LidarrAlbumMBID: group,
|
||||
AlbumTitle: album.AlbumTitle,
|
||||
_, err = q.WantMissingAlbum(ctx, dbq.WantMissingAlbumParams{
|
||||
ReleaseGroupMbid: group, ArtistMbid: *album.ArtistMbid, ArtistName: album.ArtistName,
|
||||
Title: album.AlbumTitle, AlbumID: album.AlbumID,
|
||||
})
|
||||
if errors.Is(err, pgx.ErrNoRows) {
|
||||
// Already wanted, open: by a request, a follow, or this sweeper.
|
||||
return nil
|
||||
}
|
||||
if err != nil {
|
||||
return fmt.Errorf("create request: %w", err)
|
||||
}
|
||||
res.Requested++
|
||||
|
||||
// Record the attempt even when Create deduped into somebody else's
|
||||
// existing request: the point of the counter is "how often have we gone
|
||||
// looking for this album", and a manual request in flight is a reason to
|
||||
// wait rather than to keep asking.
|
||||
row, err := q.RecordReacquisitionAttempt(ctx, dbq.RecordReacquisitionAttemptParams{
|
||||
AlbumID: album.AlbumID,
|
||||
LastRequestID: req.ID,
|
||||
})
|
||||
if err != nil {
|
||||
return fmt.Errorf("record attempt: %w", err)
|
||||
}
|
||||
|
||||
// A request the sweeper filed and could not approve waits on an admin.
|
||||
// One it deduped into was announced when it was first filed.
|
||||
awaitingAdmin := created
|
||||
if cfg.AutoApprove {
|
||||
_, aerr := s.requests.Approve(ctx, req.ID, adminID, lidarrrequests.ApproveOverrides{})
|
||||
switch {
|
||||
case aerr == nil:
|
||||
res.Approved++
|
||||
awaitingAdmin = false
|
||||
case errors.Is(aerr, lidarrrequests.ErrLidarrDisabled):
|
||||
// Leave it pending rather than treating it as a failure. The
|
||||
// request is still the right record of intent, and it becomes
|
||||
// actionable the moment Lidarr is configured.
|
||||
s.logger.Info("reacquisition: request left pending, Lidarr disabled",
|
||||
"album", album.AlbumTitle)
|
||||
case errors.Is(aerr, lidarrrequests.ErrNotPending):
|
||||
// Deduped onto a request somebody already approved. Nothing to do
|
||||
// and nothing wrong.
|
||||
default:
|
||||
return fmt.Errorf("approve: %w", aerr)
|
||||
}
|
||||
}
|
||||
|
||||
if awaitingAdmin {
|
||||
s.notifier.NotifyLogged(ctx, notifications.KindRequestPending, notifications.ToAdmins(pgtype.UUID{}),
|
||||
notifications.Payload{
|
||||
RequestID: uuid.UUID(req.ID.Bytes).String(),
|
||||
RequestKind: string(req.Kind),
|
||||
Name: lidarrrequests.DisplayName(req),
|
||||
Actor: "Re-acquisition",
|
||||
}.Map())
|
||||
}
|
||||
|
||||
if row.Attempts >= cfg.MaxAttempts {
|
||||
if err := q.MarkReacquisitionGaveUp(ctx, album.AlbumID); err != nil {
|
||||
return fmt.Errorf("mark gave up: %w", err)
|
||||
}
|
||||
res.GaveUp++
|
||||
return fmt.Errorf("want: %w", err)
|
||||
}
|
||||
res.Wanted++
|
||||
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.
|
||||
// next pass 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
|
||||
@@ -281,14 +155,11 @@ func (s *Sweeper) albumReleaseGroup(ctx context.Context, q *dbq.Queries, album d
|
||||
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 && res.Unresolved == 0 {
|
||||
if res.Wanted == 0 && res.Unresolved == 0 {
|
||||
return
|
||||
}
|
||||
s.logger.Info("reacquisition: sweep",
|
||||
"requested", res.Requested,
|
||||
"approved", res.Approved,
|
||||
"gave_up", res.GaveUp,
|
||||
"cleared", res.Cleared,
|
||||
"wanted", res.Wanted,
|
||||
"unnameable", res.Unnameable,
|
||||
"unresolved", res.Unresolved,
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user