feat: the duplicate resolver fetches a missing album track list from MusicBrainz to decide a wrong slot, five albums a pass (M498)
release / govulncheck (push) Successful in 14s
release / web (push) Successful in 1m16s
release / go (push) Successful in 1m33s
release / integration (push) Successful in 5m0s
release / android (push) Successful in 5m7s
release / Build signed APK (releases and dev) (push) Successful in 5m20s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Successful in 15s
release / Verify release artifacts (tag releases only) (push) Skipped
release / govulncheck (push) Successful in 14s
release / web (push) Successful in 1m16s
release / go (push) Successful in 1m33s
release / integration (push) Successful in 5m0s
release / android (push) Successful in 5m7s
release / Build signed APK (releases and dev) (push) Successful in 5m20s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Successful in 15s
release / Verify release artifacts (tag releases only) (push) Skipped
20 of the 34 groups left for review had no cached track list for their album: the catalogue only reads one when the wanted list or the importer needs it. The resolver now asks catalogue.RefreshReleaseGroup for an album it must decide and the catalogue has never read, through the shared MusicBrainz pacer, at most five a pass (hourly). An unreachable MusicBrainz stops the pass's fetching; a group read and found empty is not asked about again. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -363,10 +363,11 @@ func run() error {
|
||||
|
||||
// Duplicate resolver (M498, M502): classifies the sweep's groups and, with
|
||||
// the operator's auto-resolve on, merges the copies that are safe to merge.
|
||||
// Started after SetNotifier so its summaries reach admins. See
|
||||
// internal/library/duplicate_resolve.go.
|
||||
// Started after SetNotifier so its summaries reach admins. It asks the
|
||||
// catalogue for the track list of an album whose wrong slot it must
|
||||
// decide, a few a pass. See internal/library/duplicate_resolve.go.
|
||||
go library.NewDuplicateResolveWorker(pool, logger.With("component", "duplicate_resolve"),
|
||||
cfg.Storage.DataDir, fpSettings).Run(ctx)
|
||||
cfg.Storage.DataDir, fpSettings, cat.RefreshReleaseGroup).Run(ctx)
|
||||
|
||||
// Missing-file re-acquisition (milestone #290, M502 #5520). Wants back
|
||||
// albums whose files have been gone longer than the grace window; the
|
||||
|
||||
@@ -112,10 +112,24 @@ func mergePlan(g *resolveGroup) (ok bool, survivor int) {
|
||||
return false, -1
|
||||
}
|
||||
|
||||
// TrackListFetcher reads a release group's releases and track lists from
|
||||
// MusicBrainz into the catalogue (catalogue.Service.RefreshReleaseGroup).
|
||||
type TrackListFetcher func(ctx context.Context, releaseGroupMBID string) error
|
||||
|
||||
// ResolveDuplicates runs one pass. act is the operator's auto-resolve setting;
|
||||
// with it off, the pass only classifies.
|
||||
// with it off, the pass only classifies. It reads only track lists already in
|
||||
// the catalogue; ResolveDuplicatesFetching may also fetch missing ones.
|
||||
func ResolveDuplicates(
|
||||
ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, dataDir string, act bool,
|
||||
) (DuplicateResolveResult, error) {
|
||||
return ResolveDuplicatesFetching(ctx, pool, logger, dataDir, act, nil)
|
||||
}
|
||||
|
||||
// ResolveDuplicatesFetching is ResolveDuplicates that asks MusicBrainz, through
|
||||
// fetch, for the track list of an album whose wrong slot it must decide and
|
||||
// the catalogue has never read: a few per pass (trackListFetchesPerPass).
|
||||
func ResolveDuplicatesFetching(
|
||||
ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, dataDir string, act bool, fetch TrackListFetcher,
|
||||
) (DuplicateResolveResult, error) {
|
||||
if logger == nil {
|
||||
logger = slog.Default()
|
||||
@@ -182,7 +196,7 @@ func ResolveDuplicates(
|
||||
}
|
||||
|
||||
res.Dismissed = dismissDistinct(ctx, pool, logger, groups, merged)
|
||||
wrong, names := settleWrongSlots(ctx, pool, logger, groups, merged)
|
||||
wrong, names := settleWrongSlots(ctx, pool, logger, groups, merged, fetch)
|
||||
res.WrongFiles = wrong
|
||||
mergedNames = append(mergedNames, names...)
|
||||
|
||||
@@ -375,14 +389,18 @@ type DuplicateResolveWorker struct {
|
||||
logger *slog.Logger
|
||||
dataDir string
|
||||
settings *FingerprintSettingsService
|
||||
fetch TrackListFetcher
|
||||
tick time.Duration
|
||||
}
|
||||
|
||||
// NewDuplicateResolveWorker builds a worker with the production cadence.
|
||||
// NewDuplicateResolveWorker builds a worker with the production cadence. fetch
|
||||
// reads a missing track list from MusicBrainz; nil reads only what is held.
|
||||
func NewDuplicateResolveWorker(
|
||||
pool *pgxpool.Pool, logger *slog.Logger, dataDir string, settings *FingerprintSettingsService,
|
||||
pool *pgxpool.Pool, logger *slog.Logger, dataDir string, settings *FingerprintSettingsService, fetch TrackListFetcher,
|
||||
) *DuplicateResolveWorker {
|
||||
return &DuplicateResolveWorker{pool: pool, logger: logger, dataDir: dataDir, settings: settings, tick: duplicateResolveTick}
|
||||
return &DuplicateResolveWorker{
|
||||
pool: pool, logger: logger, dataDir: dataDir, settings: settings, fetch: fetch, tick: duplicateResolveTick,
|
||||
}
|
||||
}
|
||||
|
||||
// Run blocks until ctx is cancelled, running a pass at start and then each tick.
|
||||
@@ -418,7 +436,7 @@ func (w *DuplicateResolveWorker) tickOnce(ctx context.Context) {
|
||||
}
|
||||
return
|
||||
}
|
||||
res, err := ResolveDuplicates(ctx, w.pool, w.logger, w.dataDir, w.settings.Get().AutoResolve)
|
||||
res, err := ResolveDuplicatesFetching(ctx, w.pool, w.logger, w.dataDir, w.settings.Get().AutoResolve, w.fetch)
|
||||
if err != nil && ctx.Err() == nil {
|
||||
w.logger.Warn("duplicate resolve: pass failed", "err", err)
|
||||
}
|
||||
|
||||
@@ -405,6 +405,33 @@ func TestResolveDuplicates_RemovesACopyFromAnotherSongsSlot_Integration(t *testi
|
||||
}
|
||||
}
|
||||
|
||||
// An album whose track list the catalogue never read: the resolver asks for
|
||||
// it, once, and decides with what comes back.
|
||||
func TestResolveDuplicates_FetchesAMissingTrackList_Integration(t *testing.T) {
|
||||
f := newOneAlbumFixture(t, "acoustic", "Void", "Reach")
|
||||
rg := "rg-" + syncpkg.FormatUUID(f.album.ID)
|
||||
if _, err := f.pool.Exec(context.Background(),
|
||||
`UPDATE albums SET release_group_mbid = $2 WHERE id = $1`, f.album.ID, rg); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
calls := 0
|
||||
fetch := func(_ context.Context, asked string) error {
|
||||
calls++
|
||||
if asked != rg {
|
||||
t.Errorf("asked for %s, want %s", asked, rg)
|
||||
}
|
||||
seedTrackList(t, f.pool, f.album, map[string]int{"Void": 220500, "Reach": 281000})
|
||||
return nil
|
||||
}
|
||||
res, err := ResolveDuplicatesFetching(context.Background(), f.pool, nil, "", true, fetch)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if calls != 1 || res.WrongFiles != 1 || exists(f.bPath) {
|
||||
t.Fatalf("fetches %d, result %+v; want one fetch and Reach's copy removed", calls, res)
|
||||
}
|
||||
}
|
||||
|
||||
// Without a track list that decides, nothing is removed.
|
||||
func TestResolveDuplicates_LeavesAnUndecidedSlotForTheOperator_Integration(t *testing.T) {
|
||||
f := newOneAlbumFixture(t, "acoustic", "Void", "Reach")
|
||||
|
||||
@@ -13,6 +13,7 @@ import (
|
||||
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/audit"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/musicbrainz"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/notifications"
|
||||
syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync"
|
||||
)
|
||||
@@ -33,6 +34,10 @@ import (
|
||||
// Anything less certain stays with the operator.
|
||||
|
||||
const (
|
||||
// trackListFetchesPerPass is how many albums' track lists a pass may ask
|
||||
// MusicBrainz for, when the catalogue has never read them. The resolver
|
||||
// runs hourly, so a backlog drains over a few hours without a burst.
|
||||
trackListFetchesPerPass = 5
|
||||
// slotFit is how close a file must be to a slot's MusicBrainz length to be
|
||||
// that slot's song, and slotOther how far away every length of a slot's
|
||||
// title must be for the file to be another song's.
|
||||
@@ -143,9 +148,14 @@ func dismissDistinct(
|
||||
// to settled.
|
||||
func settleWrongSlots(
|
||||
ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, groups []*resolveGroup, settled map[pgtype.UUID]bool,
|
||||
fetch TrackListFetcher,
|
||||
) (int, []string) {
|
||||
q := dbq.New(pool)
|
||||
tracksByGroup := map[string][]dbq.MbReleaseTrack{}
|
||||
fetches := trackListFetchesPerPass
|
||||
if fetch == nil {
|
||||
fetches = 0
|
||||
}
|
||||
removed := 0
|
||||
var names []string
|
||||
for _, g := range groups {
|
||||
@@ -163,6 +173,18 @@ func settleWrongSlots(
|
||||
logger.Warn("duplicate resolve: reading the album's track list failed", "release_group", rg, "err", err)
|
||||
continue
|
||||
}
|
||||
if len(tracks) == 0 && fetches > 0 && neverRead(ctx, q, rg) {
|
||||
fetches--
|
||||
if err := fetch(ctx, rg); err != nil {
|
||||
logger.Info("duplicate resolve: fetching the album's track list failed", "release_group", rg, "err", err)
|
||||
if errors.Is(err, musicbrainz.ErrUnreachable) {
|
||||
fetches = 0
|
||||
}
|
||||
} else if tracks, err = q.ListMBReleaseTracksForGroup(ctx, rg); err != nil {
|
||||
logger.Warn("duplicate resolve: reading the album's track list failed", "release_group", rg, "err", err)
|
||||
continue
|
||||
}
|
||||
}
|
||||
tracksByGroup[rg] = tracks
|
||||
}
|
||||
copies := make([]slotCopy, len(g.members))
|
||||
@@ -187,6 +209,14 @@ func settleWrongSlots(
|
||||
return removed, names
|
||||
}
|
||||
|
||||
// neverRead reports whether the catalogue has never read a release group's
|
||||
// releases. A group read and found empty is not asked about again: the
|
||||
// catalogue's own refresh revisits it.
|
||||
func neverRead(ctx context.Context, q *dbq.Queries, rg string) bool {
|
||||
g, err := q.GetMBReleaseGroup(ctx, rg)
|
||||
return errors.Is(err, pgx.ErrNoRows) || (err == nil && !g.ReleasesRefreshedAt.Valid)
|
||||
}
|
||||
|
||||
// removeWrongSlots deletes each loser's file, then marks its track missing in
|
||||
// the same transaction that closes the group. The row is kept, with its likes
|
||||
// and plays: it is the slot, and a good copy fetched again takes it over. A
|
||||
|
||||
Reference in New Issue
Block a user