feat(library): AcoustID lookup worker fills the MBIDs tags leave empty (M401 #3920 #3921)
release / web (push) Successful in 1m44s
release / go (push) Successful in 2m1s
release / govulncheck (push) Successful in 17s
release / integration (push) Successful in 5m22s
release / android (push) Successful in 5m48s
release / Build signed APK (releases and dev) (push) Successful in 5m53s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Successful in 2m6s
release / Verify release artifacts (tag releases only) (push) Skipped

Migration 0069 adds tracks.mbid_source (tag | acoustid), a lookup state per
track (matched | ambiguous | no_match | failed) and the acoustid_settings
row (off, no key, min score 0.85).

The file's tag outranks a lookup (D4). UpsertTrack keeps a looked-up id
through a re-read that finds no tag id and replaces it as soon as one
appears. SetTrackMbidFromAcoustID refuses to write over a tag id.

The worker fingerprints each untagged track with fpcalc's compressed
print, looks it up and writes an id only when D5 settles it: one
recording at or above the threshold, or one left after matching title and
length. Ambiguous and no-match results write nothing. A key AcoustID
refuses, or the service being unreachable, stops the pass and is reported
in the worker's status. It never counts as a verdict on a track.

A changed file drops its lookup in the scan. The re-lookup takes back an
id that no longer matches.

Admin API: GET /api/admin/library/acoustid (settings, status, coverage by
source), PUT …/acoustid-settings (write-only key), POST …/acoustid/run,
GET …/acoustid/unsettled.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
2026-10-06 23:26:03 -04:00
co-authored by Claude Opus 5.5
parent f13da62797
commit 3c575b137c
24 changed files with 1743 additions and 29 deletions
+454
View File
@@ -0,0 +1,454 @@
package library
import (
"context"
"errors"
"fmt"
"log/slog"
"strings"
"sync"
"time"
"unicode"
"github.com/jackc/pgx/v5"
"github.com/jackc/pgx/v5/pgtype"
"github.com/jackc/pgx/v5/pgxpool"
"git.fabledsword.com/bvandeusen/minstrel/internal/acoustid"
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
)
// AcoustID lookup worker (M401 #3921).
//
// A track whose tags carry no recording MBID is invisible to the ListenBrainz
// similarity arm: it is never a seed and never comes back as a result. This
// worker fingerprints such tracks and asks AcoustID which recording they are,
// writing an id only when the answer is unambiguous (D5). The file's own tag
// always outranks it (D4).
//
// It never gates anything else (rule 164). With no key, switched off or
// AcoustID unreachable, it idles and says why through Status; scans and
// playback do not notice.
// acoustIDLookupTick is how often the worker looks for work. A settings save
// starts a pass at once, so this only paces new tracks and retries after an
// outage.
const acoustIDLookupTick = 15 * time.Minute
// acoustIDLookupBatch is how many tracks one query hands the worker.
const acoustIDLookupBatch = 50
// acoustIDLookupConcurrency is how many files are fingerprinted at once. The
// client serialises the requests themselves to AcoustID's rate, so this only
// overlaps fpcalc's decodes; low for the reason the loudness backfill's is.
const acoustIDLookupConcurrency = 2
// acoustIDDurationToleranceSec is how far a recording's MusicBrainz length may
// be from the file's and still count as the same take when telling candidates
// apart. A release's length and a rip's differ by a second or two of
// silence; a radio edit or a live take differs by far more.
const acoustIDDurationToleranceSec = 3
// Lookup states, as migration 0069's CHECK has them.
const (
lookupMatched = "matched"
lookupAmbiguous = "ambiguous"
lookupNoMatch = "no_match"
lookupFailed = "failed"
)
// recordingLookup is the AcoustID client as the worker uses it.
type recordingLookup interface {
Lookup(ctx context.Context, apiKey, fingerprint string, durationSec int) ([]acoustid.Candidate, error)
}
// AcoustIDLookupStatus is what the admin card shows about the worker.
type AcoustIDLookupStatus struct {
Running bool
LastPassAt time.Time
// Problem says why the last pass stopped short, in words for the card:
// the key was refused, or AcoustID could not be reached. Empty when the
// last pass ran to the end.
Problem string
}
// AcoustIDLookupResult tallies one pass.
type AcoustIDLookupResult struct {
Processed int
Matched int
Ambiguous int
NoMatch int
Failed int
Inconclusive int // nothing stored; tried again on a later pass
}
func (r *AcoustIDLookupResult) add(state string) {
r.Processed++
switch state {
case lookupMatched:
r.Matched++
case lookupAmbiguous:
r.Ambiguous++
case lookupNoMatch:
r.NoMatch++
case lookupFailed:
r.Failed++
default:
r.Inconclusive++
}
}
// errPassStopped ends a pass early: every further lookup would fail the same
// way, so asking again would only spend the rate limit.
type errPassStopped struct{ reason string }
func (e errPassStopped) Error() string { return e.reason }
// AcoustIDLookupWorker fills recording MBIDs through AcoustID.
type AcoustIDLookupWorker struct {
pool *pgxpool.Pool
logger *slog.Logger
settings *AcoustIDSettingsService
client recordingLookup
tick time.Duration
batch int32
// fingerprint is a field so an integration test pins which tracks a pass
// touches, not what fpcalc prints.
fingerprint func(ctx context.Context, path string) (lookupFingerprint, error)
kick chan struct{}
mu sync.Mutex
status AcoustIDLookupStatus
}
// NewAcoustIDLookupWorker builds a worker with the production cadence.
func NewAcoustIDLookupWorker(
pool *pgxpool.Pool, logger *slog.Logger, settings *AcoustIDSettingsService, client recordingLookup,
) *AcoustIDLookupWorker {
return &AcoustIDLookupWorker{
pool: pool,
logger: logger,
settings: settings,
client: client,
tick: acoustIDLookupTick,
batch: acoustIDLookupBatch,
fingerprint: computeLookupFingerprint,
kick: make(chan struct{}, 1),
}
}
// Kick asks for a pass now, for the admin card's "look up now". A pass
// already running absorbs it.
func (w *AcoustIDLookupWorker) Kick() {
if w == nil {
return
}
select {
case w.kick <- struct{}{}:
default:
}
}
// Settings is the settings service the worker reads, for the admin API. A
// nil worker has none, which serves the defaults.
func (w *AcoustIDLookupWorker) Settings() *AcoustIDSettingsService {
if w == nil {
return nil
}
return w.settings
}
// Status reports the worker's state for the admin card.
func (w *AcoustIDLookupWorker) Status() AcoustIDLookupStatus {
if w == nil {
return AcoustIDLookupStatus{}
}
w.mu.Lock()
defer w.mu.Unlock()
return w.status
}
// Run blocks until ctx is cancelled: one pass at start, then one per tick, on
// a kick, and after every settings save.
func (w *AcoustIDLookupWorker) Run(ctx context.Context) {
w.runOnce(ctx)
t := time.NewTicker(w.tick)
defer t.Stop()
for {
select {
case <-ctx.Done():
return
case <-t.C:
case <-w.kick:
case <-w.settings.Changed():
}
w.runOnce(ctx)
}
}
// runOnce contains a pass so that nothing it does (an error, a panic) can stop
// the next one from starting (rule 157).
func (w *AcoustIDLookupWorker) runOnce(ctx context.Context) {
w.setStatus(func(s *AcoustIDLookupStatus) { s.Running = true })
problem := ""
defer func() {
if r := recover(); r != nil {
w.logger.Error("acoustid lookup: pass panicked", "panic", r)
problem = "The last lookup pass stopped on an internal error; see the server log."
}
w.setStatus(func(s *AcoustIDLookupStatus) {
s.Running, s.LastPassAt, s.Problem = false, time.Now(), problem
})
}()
res, err := w.pass(ctx)
var stopped errPassStopped
switch {
case errors.As(err, &stopped):
problem = stopped.reason
w.logger.Warn("acoustid lookup: pass stopped", "reason", stopped.reason, "processed", res.Processed)
case err != nil && ctx.Err() == nil:
problem = "The last lookup pass failed; see the server log."
w.logger.Warn("acoustid lookup: pass failed", "err", err, "processed", res.Processed)
}
if res.Processed > 0 {
w.logger.Info("acoustid lookup: pass complete",
"processed", res.Processed, "matched", res.Matched, "ambiguous", res.Ambiguous,
"no_match", res.NoMatch, "failed", res.Failed, "inconclusive", res.Inconclusive)
}
}
func (w *AcoustIDLookupWorker) setStatus(f func(*AcoustIDLookupStatus)) {
w.mu.Lock()
f(&w.status)
w.mu.Unlock()
}
// pass walks the queue once, keyset-paged on id so it ends even when every
// attempt is inconclusive. Settings are read before every batch, so switching
// the lookup off or changing the threshold applies at the next batch.
func (w *AcoustIDLookupWorker) pass(ctx context.Context) (AcoustIDLookupResult, error) {
q := dbq.New(w.pool)
var res AcoustIDLookupResult
after := pgtype.UUID{Valid: true}
for {
if err := ctx.Err(); err != nil {
return res, err
}
cfg := w.settings.Get()
if !cfg.Ready() {
return res, nil
}
rows, err := q.ListTracksNeedingAcoustIDLookup(ctx, dbq.ListTracksNeedingAcoustIDLookupParams{
AfterID: after,
BatchLimit: w.batch,
})
if err != nil {
return res, fmt.Errorf("list tracks needing a lookup: %w", err)
}
if len(rows) == 0 {
return res, nil
}
if err := w.lookupBatch(ctx, cfg, rows, &res); err != nil {
return res, err
}
after = rows[len(rows)-1].ID
}
}
// lookupBatch looks up one batch, acoustIDLookupConcurrency at a time. The
// first track that stops the pass stops the batch: the tracks already started
// finish, no new ones start.
func (w *AcoustIDLookupWorker) lookupBatch(
ctx context.Context, cfg AcoustIDSettings, rows []dbq.ListTracksNeedingAcoustIDLookupRow, res *AcoustIDLookupResult,
) error {
ctx, cancel := context.WithCancelCause(ctx)
defer cancel(nil)
var (
mu sync.Mutex
wg sync.WaitGroup
sem = make(chan struct{}, acoustIDLookupConcurrency)
)
for _, row := range rows {
if ctx.Err() != nil {
break
}
sem <- struct{}{}
// The slot may have come free because a lookup just stopped the pass.
if ctx.Err() != nil {
<-sem
break
}
wg.Add(1)
go func(row dbq.ListTracksNeedingAcoustIDLookupRow) {
defer wg.Done()
defer func() { <-sem }()
defer func() {
if r := recover(); r != nil {
w.logger.Error("acoustid lookup: track panicked", "path", row.FilePath, "panic", r)
}
}()
state, err := w.lookupTrack(ctx, cfg, row)
if err != nil {
cancel(err)
}
mu.Lock()
res.add(state)
mu.Unlock()
}(row)
}
wg.Wait()
if cause := context.Cause(ctx); cause != nil && !errors.Is(cause, context.Canceled) {
return cause
}
return nil
}
// lookupTrack fingerprints one file, asks AcoustID and stores the answer. It
// returns the state stored ("" when nothing was), and an errPassStopped when
// no further lookup can succeed this pass.
func (w *AcoustIDLookupWorker) lookupTrack(
ctx context.Context, cfg AcoustIDSettings, row dbq.ListTracksNeedingAcoustIDLookupRow,
) (string, error) {
fp, err := w.fingerprint(ctx, row.FilePath)
if err != nil {
if isInconclusive(err) {
return "", nil
}
// fpcalc ran and rejected the file: a verdict, settled until it changes.
return w.store(ctx, row, lookupDecision{state: lookupFailed, detail: err.Error()}), nil
}
cands, err := w.client.Lookup(ctx, cfg.APIKey, fp.fingerprint, fp.durationSec)
switch {
case err == nil:
case errors.Is(err, acoustid.ErrInvalidKey):
return "", errPassStopped{"AcoustID refused the API key. Check it in Settings."}
case errors.Is(err, acoustid.ErrUnavailable):
return "", errPassStopped{"AcoustID could not be reached; lookups resume on the next pass."}
case errors.Is(err, acoustid.ErrInvalidFingerprint):
return w.store(ctx, row, lookupDecision{state: lookupFailed, detail: err.Error()}), nil
default:
// The caller's cancellation, or a request this client built wrongly.
// Neither is a verdict on the track.
if ctx.Err() == nil {
w.logger.Warn("acoustid lookup: lookup failed", "path", row.FilePath, "err", err)
}
return "", nil
}
return w.store(ctx, row, chooseRecording(cands, cfg.MinScore, row.Title, row.DurationMs)), nil
}
// store records a decision and applies it to tracks.mbid in one transaction,
// so the gauge never shows a match whose id was not written. A failed write
// stores nothing and the track is tried again.
func (w *AcoustIDLookupWorker) store(ctx context.Context, row dbq.ListTracksNeedingAcoustIDLookupRow, d lookupDecision) string {
err := pgx.BeginFunc(ctx, w.pool, func(tx pgx.Tx) error {
q := dbq.New(tx)
params := dbq.RecordAcoustIDLookupParams{
TrackID: row.ID, State: d.state, Candidates: int32(d.candidates),
}
if d.candidates > 0 {
params.BestScore = &d.bestScore
}
if d.recording != "" {
params.RecordingMbid = &d.recording
}
if d.detail != "" {
params.Detail = &d.detail
}
if err := q.RecordAcoustIDLookup(ctx, params); err != nil {
return err
}
if d.state == lookupMatched {
_, err := q.SetTrackMbidFromAcoustID(ctx, dbq.SetTrackMbidFromAcoustIDParams{ID: row.ID, Mbid: &d.recording})
return err
}
// A re-lookup of changed bytes that no longer matches takes back the
// id the earlier lookup wrote.
return q.ClearTrackAcoustIDMbid(ctx, row.ID)
})
if err != nil {
if ctx.Err() == nil {
w.logger.Warn("acoustid lookup: storing the result failed", "path", row.FilePath, "err", err)
}
return ""
}
return d.state
}
// lookupDecision is what one lookup settles on.
type lookupDecision struct {
state string
recording string // set only when matched
bestScore float32
candidates int
detail string
}
// chooseRecording applies D5 to AcoustID's candidates (best score first):
// only recordings at or above minScore count, one of them is a match, and
// several are told apart by the track's title and length or not at all. A
// wrong MBID would feed similarity the wrong neighbours, which is worse than
// none, so anything left unresolved is ambiguous and writes nothing.
func chooseRecording(cands []acoustid.Candidate, minScore float32, title string, durationMs int32) lookupDecision {
d := lookupDecision{candidates: len(cands)}
if len(cands) > 0 {
d.bestScore = float32(cands[0].Score)
}
var above []acoustid.Candidate
for _, c := range cands {
if float32(c.Score) >= minScore {
above = append(above, c)
}
}
switch len(above) {
case 0:
d.state = lookupNoMatch
return d
case 1:
d.state, d.recording = lookupMatched, above[0].ID
return d
}
want := normalizeTitle(title)
var same []acoustid.Candidate
for _, c := range above {
if normalizeTitle(c.Title) == want && sameLength(c.DurationSec, durationMs) {
same = append(same, c)
}
}
if len(same) == 1 {
d.state, d.recording = lookupMatched, same[0].ID
return d
}
d.state = lookupAmbiguous
return d
}
// sameLength reports whether a recording's length (0 when MusicBrainz has
// none) fits the file's. An unknown length on either side tells nothing, so
// it does not rule a candidate out.
func sameLength(recordingSec int, fileMs int32) bool {
if recordingSec <= 0 || fileMs <= 0 {
return true
}
diff := recordingSec*1000 - int(fileMs)
return diff <= acoustIDDurationToleranceSec*1000 && diff >= -acoustIDDurationToleranceSec*1000
}
// normalizeTitle compares titles by their letters and digits alone, case
// folded, so "Don't Stop" and "Dont stop" match while "Song (Radio Edit)"
// and "Song" do not.
func normalizeTitle(s string) string {
var b strings.Builder
for _, r := range strings.ToLower(s) {
if unicode.IsLetter(r) || unicode.IsDigit(r) {
b.WriteRune(r)
}
}
return b.String()
}
// AcoustIDCoverage reports where the library's recording MBIDs came from and
// what the lookups found, for the admin gauge.
func AcoustIDCoverage(ctx context.Context, pool *pgxpool.Pool) (dbq.GetMbidCoverageRow, error) {
return dbq.New(pool).GetMbidCoverage(ctx)
}
+371
View File
@@ -0,0 +1,371 @@
package library
import (
"context"
"errors"
"fmt"
"io"
"log/slog"
"path/filepath"
"strings"
"sync"
"testing"
"time"
"github.com/jackc/pgx/v5/pgtype"
"github.com/jackc/pgx/v5/pgxpool"
"git.fabledsword.com/bvandeusen/minstrel/internal/acoustid"
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
"git.fabledsword.com/bvandeusen/minstrel/internal/dbtest"
)
func cand(id, title string, sec int, score float64) acoustid.Candidate {
return acoustid.Candidate{Recording: acoustid.Recording{ID: id, Title: title, DurationSec: sec}, Score: score}
}
// D5, as a table over what decides it: how many candidates clear the
// threshold, and whether title and length leave exactly one of them.
func TestChooseRecording(t *testing.T) {
const (
minScore = 0.85
title = "Don't Stop"
fileMs = 200_400
)
cases := []struct {
name string
cands []acoustid.Candidate
state string
rec string
}{
{"no candidates", nil, lookupNoMatch, ""},
{"all below the threshold", []acoustid.Candidate{cand("a", title, 200, 0.84)}, lookupNoMatch, ""},
{"one above the threshold", []acoustid.Candidate{cand("a", "Other title", 999, 0.9), cand("b", title, 200, 0.4)}, lookupMatched, "a"},
{"exactly at the threshold counts", []acoustid.Candidate{cand("a", title, 200, 0.85)}, lookupMatched, "a"},
{"several, one with the title and length",
[]acoustid.Candidate{cand("album", "Dont stop", 201, 0.95), cand("edit", "Don't Stop (Radio Edit)", 181, 0.95)},
lookupMatched, "album"},
{"several, title alike but only one length fits",
[]acoustid.Candidate{cand("live", title, 260, 0.9), cand("studio", title, 198, 0.9)},
lookupMatched, "studio"},
{"several with the same title and length",
[]acoustid.Candidate{cand("r1", title, 200, 0.95), cand("r2", title, 201, 0.95)},
lookupAmbiguous, ""},
{"several, none with the title",
[]acoustid.Candidate{cand("x", "Something", 200, 0.95), cand("y", "Else", 200, 0.95)},
lookupAmbiguous, ""},
// An unknown length rules nothing out, so it cannot break a tie on
// its own: both stay and the lookup is ambiguous.
{"unknown lengths do not disambiguate",
[]acoustid.Candidate{cand("r1", title, 0, 0.95), cand("r2", title, 0, 0.95)},
lookupAmbiguous, ""},
}
for _, c := range cases {
d := chooseRecording(c.cands, minScore, title, fileMs)
if d.state != c.state || d.recording != c.rec {
t.Errorf("%s: got %s %q, want %s %q", c.name, d.state, d.recording, c.state, c.rec)
}
if d.candidates != len(c.cands) {
t.Errorf("%s: candidates = %d, want %d", c.name, d.candidates, len(c.cands))
}
}
if d := chooseRecording([]acoustid.Candidate{cand("a", title, 200, 0.97)}, minScore, title, fileMs); d.bestScore != 0.97 {
t.Errorf("best score = %v, want 0.97", d.bestScore)
}
}
func TestNormalizeTitle(t *testing.T) {
if normalizeTitle("Don't Stop!") != normalizeTitle("dont stop") {
t.Error("punctuation and case should not separate titles")
}
if normalizeTitle("Song (Radio Edit)") == normalizeTitle("Song") {
t.Error("an edit's title must not match the original's")
}
if normalizeTitle("Ænima") != "ænima" {
t.Errorf("non-ASCII letters must survive: %q", normalizeTitle("Ænima"))
}
}
// fakeLookup answers by fingerprint. The worker passes each file's path
// through as its fingerprint (see testWorker), so a test keys answers by file.
type fakeLookup struct {
mu sync.Mutex
answers map[string][]acoustid.Candidate
err error
calls int
}
func (f *fakeLookup) Lookup(_ context.Context, _ string, fingerprint string, _ int) ([]acoustid.Candidate, error) {
f.mu.Lock()
defer f.mu.Unlock()
f.calls++
if f.err != nil {
return nil, f.err
}
return f.answers[fingerprint], nil
}
func testWorker(t *testing.T, pool *pgxpool.Pool, client recordingLookup) *AcoustIDLookupWorker {
t.Helper()
ctx := context.Background()
settings, err := NewAcoustIDSettingsService(ctx, pool)
if err != nil {
t.Fatal(err)
}
key := "test-key"
if _, err := settings.Set(ctx, AcoustIDSettingsUpdate{Enabled: true, MinScore: 0.85, APIKey: &key}); err != nil {
t.Fatal(err)
}
w := NewAcoustIDLookupWorker(pool, slog.New(slog.NewTextHandler(io.Discard, nil)), settings, client)
w.fingerprint = func(_ context.Context, path string) (lookupFingerprint, error) {
return lookupFingerprint{fingerprint: path, durationSec: 200}, nil
}
return w
}
func ptr(s string) *string { return &s }
// mbidOf reads a track's MBID and its source.
func mbidOf(t *testing.T, pool *pgxpool.Pool, id pgtype.UUID) (string, string) {
t.Helper()
var mbid, source *string
if err := pool.QueryRow(context.Background(),
`SELECT mbid, mbid_source FROM tracks WHERE id = $1`, id).Scan(&mbid, &source); err != nil {
t.Fatal(err)
}
deref := func(p *string) string {
if p == nil {
return ""
}
return *p
}
return deref(mbid), deref(source)
}
func addTrack(t *testing.T, q *dbq.Queries, album dbq.Album, artist dbq.Artist, title, path string, mbid *string) dbq.Track {
t.Helper()
tr, err := q.UpsertTrack(context.Background(), dbq.UpsertTrackParams{
Title: title, AlbumID: album.ID, ArtistID: artist.ID, DurationMs: 200_000,
FilePath: path, FileSize: 100, FileFormat: "mp3", Mbid: mbid,
})
if err != nil {
t.Fatalf("track %s: %v", title, err)
}
return tr
}
// D4: the file's tag outranks a lookup, in both directions. Falsified by
// restoring `mbid = EXCLUDED.mbid` in UpsertTrack: the first re-read below
// then erases the looked-up id.
func TestUpsertTrack_KeepsALookedUpMbidUntilATagCarriesOne_Integration(t *testing.T) {
pool := newPool(t)
ctx := context.Background()
q := dbq.New(pool)
dir := t.TempDir()
tr, album, artist := seedTrack(t, pool, filepath.Join(dir, "a.mp3"))
reread := func(mbid *string) {
t.Helper()
if _, err := q.UpsertTrack(ctx, dbq.UpsertTrackParams{
Title: tr.Title, AlbumID: album.ID, ArtistID: artist.ID, DurationMs: 1000,
FilePath: tr.FilePath, FileSize: 100, FileFormat: "mp3", Mbid: mbid,
}); err != nil {
t.Fatal(err)
}
}
if _, err := q.SetTrackMbidFromAcoustID(ctx, dbq.SetTrackMbidFromAcoustIDParams{ID: tr.ID, Mbid: ptr("rec-looked-up")}); err != nil {
t.Fatal(err)
}
reread(nil)
if m, s := mbidOf(t, pool, tr.ID); m != "rec-looked-up" || s != "acoustid" {
t.Errorf("after an untagged re-read: %q from %q, want the looked-up id kept", m, s)
}
reread(ptr("rec-from-tag"))
if m, s := mbidOf(t, pool, tr.ID); m != "rec-from-tag" || s != "tag" {
t.Errorf("after a tagged re-read: %q from %q, want the tag's id", m, s)
}
// A tag id is the file's to take away: removing the tag clears it, and a
// lookup cannot write over one.
if n, err := q.SetTrackMbidFromAcoustID(ctx, dbq.SetTrackMbidFromAcoustIDParams{ID: tr.ID, Mbid: ptr("rec-other")}); err != nil || n != 0 {
t.Errorf("a lookup wrote over a tag id: %d rows, %v", n, err)
}
reread(nil)
if m, s := mbidOf(t, pool, tr.ID); m != "" || s != "" {
t.Errorf("after the tag was removed: %q from %q, want none", m, s)
}
}
func TestAcoustIDLookup_FillsOnlyWhatItCanSettle_Integration(t *testing.T) {
pool := newPool(t)
ctx := context.Background()
q := dbq.New(pool)
dir := t.TempDir()
_, album, artist := seedTrack(t, pool, filepath.Join(dir, "seed.mp3"))
path := func(n string) string { return filepath.Join(dir, n+".mp3") }
matched := addTrack(t, q, album, artist, "Matched", path("matched"), nil)
ambiguous := addTrack(t, q, album, artist, "Twice", path("ambiguous"), nil)
none := addTrack(t, q, album, artist, "Unknown", path("none"), nil)
tagged := addTrack(t, q, album, artist, "Tagged", path("tagged"), ptr("rec-tag"))
fake := &fakeLookup{answers: map[string][]acoustid.Candidate{
path("matched"): {cand("rec-matched", "Matched", 200, 0.97)},
path("ambiguous"): {cand("rec-1", "Twice", 200, 0.95), cand("rec-2", "Twice", 200, 0.95)},
path("tagged"): {cand("rec-wrong", "Tagged", 200, 0.99)},
}}
w := testWorker(t, pool, fake)
res, err := w.pass(ctx)
if err != nil {
t.Fatal(err)
}
// seed.mp3 has no answer, so it settles as no_match beside "Unknown".
if res.Matched != 1 || res.Ambiguous != 1 || res.NoMatch != 2 {
t.Errorf("pass = %+v, want 1 matched, 1 ambiguous, 2 no match", res)
}
if m, s := mbidOf(t, pool, matched.ID); m != "rec-matched" || s != "acoustid" {
t.Errorf("matched track: %q from %q", m, s)
}
for _, tr := range []dbq.Track{ambiguous, none} {
if m, _ := mbidOf(t, pool, tr.ID); m != "" {
t.Errorf("%s got MBID %q from an unsettled lookup", tr.Title, m)
}
}
// The tagged track is never looked up, so the fake's wrong answer for it
// is never seen.
if m, s := mbidOf(t, pool, tagged.ID); m != "rec-tag" || s != "tag" {
t.Errorf("tagged track: %q from %q, want its tag untouched", m, s)
}
// Settled tracks are not looked up again.
before := fake.calls
if res, err := w.pass(ctx); err != nil || res.Processed != 0 || fake.calls != before {
t.Errorf("second pass = %+v (err %v, %d new calls), want nothing to do", res, err, fake.calls-before)
}
// The matched file changes and its new bytes no longer match: the scan
// drops the lookup, and the re-lookup takes the id back.
if err := q.DeleteAcoustIDLookup(ctx, matched.ID); err != nil {
t.Fatal(err)
}
fake.mu.Lock()
fake.answers[path("matched")] = nil
fake.mu.Unlock()
if _, err := w.pass(ctx); err != nil {
t.Fatal(err)
}
if m, s := mbidOf(t, pool, matched.ID); m != "" || s != "" {
t.Errorf("after a non-matching re-lookup: %q from %q, want the looked-up id taken back", m, s)
}
cov, err := AcoustIDCoverage(ctx, pool)
if err != nil {
t.Fatal(err)
}
if cov.Total != 5 || cov.FromTag != 1 || cov.FromAcoustid != 0 || cov.Ambiguous != 1 || cov.NoMatch != 3 || cov.Pending != 0 {
t.Errorf("coverage = %+v", cov)
}
}
func TestAcoustIDLookup_StopsWhenTheServiceCannotAnswer_Integration(t *testing.T) {
pool := newPool(t)
ctx := context.Background()
q := dbq.New(pool)
dir := t.TempDir()
_, album, artist := seedTrack(t, pool, filepath.Join(dir, "seed.mp3"))
for i := range 5 {
addTrack(t, q, album, artist, fmt.Sprint("T", i), filepath.Join(dir, fmt.Sprint(i, ".mp3")), nil)
}
for _, c := range []struct {
err error
problem string
}{
{fmt.Errorf("%w: invalid API key", acoustid.ErrInvalidKey), "refused the API key"},
{fmt.Errorf("%w: status 503", acoustid.ErrUnavailable), "could not be reached"},
} {
fake := &fakeLookup{err: c.err}
w := testWorker(t, pool, fake)
w.runOnce(ctx)
// Stopped at the first answer, not one failed call per track. Two may
// already be in flight at concurrency 2.
if fake.calls > acoustIDLookupConcurrency {
t.Errorf("%v: %d lookups made, want the pass to stop at the first", c.err, fake.calls)
}
if st := w.Status(); st.Running || !strings.Contains(st.Problem, c.problem) {
t.Errorf("%v: status = %+v, want a problem mentioning %q", c.err, st, c.problem)
}
var n int
if err := pool.QueryRow(ctx, `SELECT count(*) FROM track_acoustid_lookups`).Scan(&n); err != nil || n != 0 {
t.Errorf("%v: %d lookups stored (err %v); a service failure says nothing about a track", c.err, n, err)
}
}
}
// The point of the milestone (#3921): once a lookup fills an MBID, the
// similarity worker picks the track as a seed and can map ListenBrainz's
// answers back to it.
func TestAcoustIDLookup_FilledTrackReachesSimilarity_Integration(t *testing.T) {
pool := newPool(t)
ctx := context.Background()
q := dbq.New(pool)
dir := t.TempDir()
tr, _, _ := seedTrack(t, pool, filepath.Join(dir, "played.mp3"))
u, err := q.CreateUser(ctx, dbq.CreateUserParams{
Username: dbtest.TestUserPrefix + "acoustid", PasswordHash: "x", ApiTokenHash: "acoustid-token",
})
if err != nil {
t.Fatal(err)
}
now := pgtype.Timestamptz{Time: time.Now(), Valid: true}
session, err := q.InsertPlaySession(ctx, dbq.InsertPlaySessionParams{UserID: u.ID, StartedAt: now})
if err != nil {
t.Fatal(err)
}
if _, err := q.InsertPlayEvent(ctx, dbq.InsertPlayEventParams{
UserID: u.ID, TrackID: tr.ID, SessionID: session.ID, StartedAt: now,
}); err != nil {
t.Fatal(err)
}
seeds := func() int {
t.Helper()
rows, err := q.ListPlayedTracksNeedingSimilarity(ctx, 100)
if err != nil {
t.Fatal(err)
}
return len(rows)
}
if n := seeds(); n != 0 {
t.Fatalf("an untagged track is already a similarity seed (%d)", n)
}
fake := &fakeLookup{answers: map[string][]acoustid.Candidate{
tr.FilePath: {cand("rec-played", tr.Title, 1, 0.99)},
}}
if _, err := testWorker(t, pool, fake).pass(ctx); err != nil {
t.Fatal(err)
}
if n := seeds(); n != 1 {
t.Errorf("after the lookup, %d similarity seeds, want the played track", n)
}
got, err := q.GetTracksByMBIDs(ctx, []string{"rec-played"})
if err != nil || len(got) != 1 || got[0].ID != tr.ID {
t.Errorf("GetTracksByMBIDs = %+v (err %v), want the played track", got, err)
}
}
func TestAcoustIDLookup_IdleUntilReady(t *testing.T) {
var nilSvc *AcoustIDSettingsService
if nilSvc.Get().Ready() {
t.Error("the defaults are ready; the lookup must ship off")
}
for _, s := range []AcoustIDSettings{{Enabled: true}, {APIKey: "k"}} {
if s.Ready() {
t.Errorf("%+v reads as ready", s)
}
}
if _, err := nilSvc.Set(context.Background(), AcoustIDSettingsUpdate{MinScore: 0.2}); !errors.Is(err, ErrAcoustIDSettingOutOfRange) {
t.Errorf("min_score 0.2: err = %v, want out of range", err)
}
}
+132
View File
@@ -0,0 +1,132 @@
package library
import (
"context"
"errors"
"fmt"
"sync"
"time"
"github.com/jackc/pgx/v5/pgxpool"
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
)
// AcoustID lookup settings (M401 #3922). Rule 25: a database row, changed
// without a restart, shared by the worker and the admin API.
// AcoustIDSettings mirrors the acoustid_settings row.
type AcoustIDSettings struct {
Enabled bool
// APIKey is the operator's registered application key. Never sent back
// to the browser; the API reports only whether one is set.
APIKey string
// MinScore is the AcoustID score a recording needs to be considered.
MinScore float32
// UpdatedAt is set by the database.
UpdatedAt time.Time
}
// Ready reports whether the worker may call AcoustID at all.
func (s AcoustIDSettings) Ready() bool { return s.Enabled && s.APIKey != "" }
// DefaultAcoustIDSettings mirrors migration 0069's defaults: off, no key.
var DefaultAcoustIDSettings = AcoustIDSettings{MinScore: 0.85}
// The score threshold's bounds, as migration 0069's CHECK has them. Below 0.5
// AcoustID's own docs call a match doubtful; the floor keeps a typo in the
// field from filling the library with guesses.
const (
minAcoustIDScore = 0.5
maxAcoustIDScore = 1
)
// ErrAcoustIDSettingOutOfRange is returned by Set for a value the CHECK would
// reject, so the API answers 400 naming the field.
var ErrAcoustIDSettingOutOfRange = errors.New("acoustid setting out of range")
// AcoustIDSettingsUpdate is the admin save. A nil APIKey keeps the stored key;
// an empty one clears it.
type AcoustIDSettingsUpdate struct {
Enabled bool
MinScore float32
APIKey *string
}
// AcoustIDSettingsService caches the settings and owns their persistence.
type AcoustIDSettingsService struct {
pool *pgxpool.Pool
mu sync.RWMutex
cur AcoustIDSettings
// changed is signalled after every save, so the worker can start a pass
// at once instead of waiting out its tick.
changed chan struct{}
}
// NewAcoustIDSettingsService loads once and caches. It always returns a usable
// service, holding the defaults (off) when the load fails; the error says so.
func NewAcoustIDSettingsService(ctx context.Context, pool *pgxpool.Pool) (*AcoustIDSettingsService, error) {
s := &AcoustIDSettingsService{pool: pool, cur: DefaultAcoustIDSettings, changed: make(chan struct{}, 1)}
row, err := dbq.New(pool).GetAcoustIDSettings(ctx)
if err != nil {
return s, fmt.Errorf("acoustid settings: load: %w", err)
}
s.cur = acoustIDSettingsFromRow(row)
return s, nil
}
// Get returns the cached settings. A nil service answers with the defaults.
func (s *AcoustIDSettingsService) Get() AcoustIDSettings {
if s == nil {
return DefaultAcoustIDSettings
}
s.mu.RLock()
defer s.mu.RUnlock()
return s.cur
}
// Set validates, persists and re-caches.
func (s *AcoustIDSettingsService) Set(ctx context.Context, in AcoustIDSettingsUpdate) (AcoustIDSettings, error) {
if in.MinScore < minAcoustIDScore || in.MinScore > maxAcoustIDScore {
return AcoustIDSettings{}, fmt.Errorf("%w: min_score must be %.2f-%.2f",
ErrAcoustIDSettingOutOfRange, float32(minAcoustIDScore), float32(maxAcoustIDScore))
}
if s == nil {
return AcoustIDSettings{}, errors.New("acoustid settings: no settings service")
}
params := dbq.UpdateAcoustIDSettingsParams{Enabled: in.Enabled, MinScore: in.MinScore}
if in.APIKey != nil {
params.SetApiKey = true
params.ApiKey = *in.APIKey
}
row, err := dbq.New(s.pool).UpdateAcoustIDSettings(ctx, params)
if err != nil {
return AcoustIDSettings{}, fmt.Errorf("acoustid settings: save: %w", err)
}
out := acoustIDSettingsFromRow(row)
s.mu.Lock()
s.cur = out
s.mu.Unlock()
select {
case s.changed <- struct{}{}:
default:
}
return out, nil
}
// Changed is signalled after a save. A nil service never signals.
func (s *AcoustIDSettingsService) Changed() <-chan struct{} {
if s == nil {
return nil
}
return s.changed
}
func acoustIDSettingsFromRow(row dbq.AcoustidSetting) AcoustIDSettings {
out := AcoustIDSettings{Enabled: row.Enabled, MinScore: row.MinScore, UpdatedAt: row.UpdatedAt.Time}
if row.ApiKey != nil {
out.APIKey = *row.ApiKey
}
return out
}
+1 -1
View File
@@ -69,7 +69,7 @@ func newMergeFixture(t *testing.T) mergeFixture {
}
}
// Only the copy being removed carries a recording MBID.
mustExec(`UPDATE tracks SET mbid = 'rec-www' WHERE id = $1`, f.remove.ID)
mustExec(`UPDATE tracks SET mbid = 'rec-www', mbid_source = 'tag' WHERE id = $1`, f.remove.ID)
user := func(name string) dbq.User {
t.Helper()
+12
View File
@@ -270,6 +270,18 @@ func computeChromaprint(ctx context.Context, path string, lengthSec int32) ([]in
return parseFpcalcRaw(out)
}
// computeLookupFingerprint returns the compressed fingerprint and duration an
// AcoustID lookup needs. It fails as runFingerprintTool does, so a stall or a
// missing fpcalc reads as inconclusive (isInconclusive) and a file fpcalc
// rejects as a verdict.
func computeLookupFingerprint(ctx context.Context, path string) (lookupFingerprint, error) {
out, err := runFingerprintTool(ctx, "fpcalc", fpcalcLookupArgs(path))
if err != nil {
return lookupFingerprint{}, err
}
return parseFpcalcCompressed(out)
}
// runFingerprintTool runs one tool under fingerprintTimeout.
//
// Any non-zero exit is an error, and that deliberately includes fpcalc's exit 3:
+6
View File
@@ -409,6 +409,12 @@ func (s *Scanner) scanFile(
if err := q.DeleteTrackLoudness(ctx, track.ID); err != nil {
s.logger.Warn("loudness: clearing stale measurement failed", "path", path, "err", err)
}
// Likewise an AcoustID lookup (M401): the worker looks the new bytes
// up again. A looked-up MBID stays meanwhile and is taken back only if
// the new lookup does not match it.
if err := q.DeleteAcoustIDLookup(ctx, track.ID); err != nil {
s.logger.Warn("acoustid: clearing stale lookup failed", "path", path, "err", err)
}
}
if knownTrack {