From c2da8c20957fd2f32a9b894fb89d655da556dbdd Mon Sep 17 00:00:00 2001 From: Vladimir Zagainov Date: Sun, 19 Jul 2026 18:19:57 +0300 Subject: [PATCH] feat: add DB-backed scanner entrypoint and compute-only main hook Implements ScanArtist/ScanAll in internal/scanner loading local/external releases via the database layer with ctx-cancellation checks, plus a compute-only run() hook that logs missing-release counts. Add table-driven tests using in-memory SQLite fixtures. --- cmd/naviwatcher/main.go | 23 ++- cmd/naviwatcher/main_test.go | 46 +++++ .../2026-07-19-scanner-engine-fuzzy-diff.md | 12 +- internal/scanner/scan.go | 66 ++++++ internal/scanner/scan_test.go | 195 ++++++++++++++++++ internal/scanner/scanner.go | 14 ++ 6 files changed, 348 insertions(+), 8 deletions(-) create mode 100644 internal/scanner/scan.go create mode 100644 internal/scanner/scan_test.go diff --git a/cmd/naviwatcher/main.go b/cmd/naviwatcher/main.go index 14e1f35..755d3a0 100644 --- a/cmd/naviwatcher/main.go +++ b/cmd/naviwatcher/main.go @@ -12,6 +12,7 @@ import ( "naviwatcher/internal/config" "naviwatcher/internal/database" "naviwatcher/internal/musicbrainz" + "naviwatcher/internal/scanner" ) // App holds all application dependencies for clean shutdown and testability. @@ -91,9 +92,27 @@ func (a *App) Close() { } func (a *App) run(ctx context.Context) error { + // Compute-only scanner hook: scan all monitored artists for missing + // releases and log the count. Notifier/Web UI are out of scope for this + // plan, so results are only logged. This call is non-blocking and + // goroutine-safe; it observes ctx cancellation and returns early. + missing, err := scanner.ScanAll(ctx, a.db, a.cfg.Scanner.FuzzyThreshold) + if err != nil { + if ctx.Err() != nil { + // Context cancelled (e.g. shutdown) — exit cleanly. + return nil + } + return fmt.Errorf("scan all: %w", err) + } + + log.Printf("Scan complete: %d missing release(s) across monitored artists", len(missing)) + for _, m := range missing { + log.Printf(" missing: artist=%s rgid=%s title=%q", m.ArtistID, m.RGID, m.Title) + } + // Main application loop — blocks until context is cancelled. - // Business logic (scanner, notifier, web server) will be wired into - // separate goroutines here in future tasks. + // Business logic (notifier, web server) will be wired into separate + // goroutines here in future tasks. <-ctx.Done() return nil } diff --git a/cmd/naviwatcher/main_test.go b/cmd/naviwatcher/main_test.go index 317c50b..20d0b0b 100644 --- a/cmd/naviwatcher/main_test.go +++ b/cmd/naviwatcher/main_test.go @@ -7,8 +7,54 @@ import ( "testing" "naviwatcher/internal/config" + "naviwatcher/internal/database" ) +func TestAppRun_ScanLogsMissingReleases(t *testing.T) { + // Verify the compute-only run() hook scans monitored artists and returns + // nil without starting notifier/web. Uses an in-memory DB with one + // monitored artist that has one missing release. + db, err := database.New(":memory:") + if err != nil { + t.Fatalf("database.New() error: %v", err) + } + defer db.Close() + + if err := database.SaveArtistSettings(db, &database.ArtistSettings{ + ID: "artist-1", + Name: "Pink Floyd", + Monitored: true, + }); err != nil { + t.Fatalf("seed artist: %v", err) + } + if err := database.SaveLocalAlbum(db, &database.LocalAlbum{ + ID: "l1", + ArtistID: "artist-1", + Title: "The Wall", + }); err != nil { + t.Fatalf("seed local album: %v", err) + } + if err := database.SaveExternalRelease(db, &database.ExternalRelease{ + RGID: "rg2", + ArtistID: "artist-1", + Title: "Animals", + }); err != nil { + t.Fatalf("seed external release: %v", err) + } + + app := &App{ + cfg: &config.Config{Scanner: config.ScannerConfig{FuzzyThreshold: 0.85}}, + db: db, + } + + ctx, cancel := context.WithCancel(context.Background()) + cancel() // cancel immediately so run() exits after scanning + + if err := app.run(ctx); err != nil { + t.Fatalf("app.run() returned error: %v", err) + } +} + func TestConfigIntegration(t *testing.T) { // Integration test: write a minimal valid config and load it via config.LoadConfig, // verifying the full path that main() uses. diff --git a/docs/plans/2026-07-19-scanner-engine-fuzzy-diff.md b/docs/plans/2026-07-19-scanner-engine-fuzzy-diff.md index bae0414..126cb80 100644 --- a/docs/plans/2026-07-19-scanner-engine-fuzzy-diff.md +++ b/docs/plans/2026-07-19-scanner-engine-fuzzy-diff.md @@ -76,12 +76,12 @@ - [x] run tests — must pass before task 5 ### Task 5: Wire a DB-backed scanner entrypoint + `main.go` hook (compute-only) -- [ ] add `func ScanArtist(ctx context.Context, db *database.DB, artistID string, threshold float64) ([]MissingRelease, error)` that loads local + external by artist via `database.GetLocalAlbumsByArtist` / `database.GetExternalReleasesByArtist` and calls `FindMissingReleases` -- [ ] add `func ScanAll(ctx context.Context, db *database.DB, threshold float64) ([]MissingRelease, error)` iterating monitored artists (reuse `database.GetAllArtistSettings`) with `ctx.Err()` checks between artists -- [ ] write tests `internal/scanner/scan_test.go` using `newTestDB(t)` + `seedArtist` + seeded `local_albums`/`external_releases` rows; assert missing set matches expectations; test ctx-cancellation returns early -- [ ] extend `cmd/naviwatcher/main.go` `App` struct + `NewApp` to construct the scanner (or keep stateless) and add a compute-only call in `run()` (e.g. log count of missing releases for monitored artists) without starting notifier/web — keep `run()` non-blocking / goroutine-safe per spec concurrency note -- [ ] write/extend `cmd/naviwatcher/main_test.go` if `App`/wiring changed -- [ ] run full test suite (`go test ./...`) and `go vet ./...` — must pass before final task +- [x] add `func ScanArtist(ctx context.Context, db *database.DB, artistID string, threshold float64) ([]MissingRelease, error)` that loads local + external by artist via `database.GetLocalAlbumsByArtist` / `database.GetExternalReleasesByArtist` and calls `FindMissingReleases` +- [x] add `func ScanAll(ctx context.Context, db *database.DB, threshold float64) ([]MissingRelease, error)` iterating monitored artists (reuse `database.GetAllArtistSettings`) with `ctx.Err()` checks between artists +- [x] write tests `internal/scanner/scan_test.go` using `newTestDB(t)` + `seedArtist` + seeded `local_albums`/`external_releases` rows; assert missing set matches expectations; test ctx-cancellation returns early +- [x] extend `cmd/naviwatcher/main.go` `App` struct + `NewApp` to construct the scanner (or keep stateless) and add a compute-only call in `run()` (e.g. log count of missing releases for monitored artists) without starting notifier/web — keep `run()` non-blocking / goroutine-safe per spec concurrency note +- [x] write/extend `cmd/naviwatcher/main_test.go` if `App`/wiring changed +- [x] run full test suite (`go test ./...`) and `go vet ./...` — must pass before final task ### Task 6: Verify acceptance criteria - [ ] verify `FindMissingReleases`/`ScanArtist`/`ScanAll` meet spec: normalization + 0.85 fuzzy threshold, ignore `IsIgnored`, per-artist scoping diff --git a/internal/scanner/scan.go b/internal/scanner/scan.go new file mode 100644 index 0000000..f5bb872 --- /dev/null +++ b/internal/scanner/scan.go @@ -0,0 +1,66 @@ +package scanner + +import ( + "context" + + "naviwatcher/internal/database" +) + +// ScanArtist loads the local albums and external releases for a single artist +// from the database and computes the list of missing releases. +// +// threshold is the fuzzy-similarity cutoff; pass 0 to use DefaultThreshold. +// The context is checked before querying the database; if it is already +// cancelled, no work is performed and the sentinel error ctx.Err() is returned. +func ScanArtist(ctx context.Context, db *database.DB, artistID string, threshold float64) ([]MissingRelease, error) { + if err := ctx.Err(); err != nil { + return nil, err + } + + local, err := database.GetLocalAlbumsByArtist(db, artistID) + if err != nil { + return nil, err + } + + external, err := database.GetExternalReleasesByArtist(db, artistID) + if err != nil { + return nil, err + } + + missing := FindMissingReleases(local, external, resolveThreshold(threshold)) + return missing, nil +} + +// ScanAll iterates over all monitored artists (those with Monitored == true) +// and computes the missing releases for each. Results are concatenated into a +// single slice across all artists. +// +// ctx.Err() is checked between artists; if cancellation occurs mid-iteration, +// scanning stops early and the accumulated results so far are returned along +// with the cancellation error. threshold follows the same contract as +// ScanArtist (0 → DefaultThreshold). +func ScanAll(ctx context.Context, db *database.DB, threshold float64) ([]MissingRelease, error) { + settings, err := database.GetAllArtistSettings(db) + if err != nil { + return nil, err + } + + resolved := resolveThreshold(threshold) + + var all []MissingRelease + for _, s := range settings { + if err := ctx.Err(); err != nil { + return all, err + } + if !s.Monitored { + continue + } + missing, err := ScanArtist(ctx, db, s.ID, resolved) + if err != nil { + return all, err + } + all = append(all, missing...) + } + + return all, nil +} diff --git a/internal/scanner/scan_test.go b/internal/scanner/scan_test.go new file mode 100644 index 0000000..eac7369 --- /dev/null +++ b/internal/scanner/scan_test.go @@ -0,0 +1,195 @@ +package scanner + +import ( + "context" + "testing" + + "naviwatcher/internal/database" +) + +// newTestDB creates an in-memory SQLite database with all migrations applied. +func newTestDB(t *testing.T) *database.DB { + t.Helper() + db, err := database.New(":memory:") + if err != nil { + t.Fatalf("database.New() error: %v", err) + } + return db +} + +// seedArtist inserts a minimal artist_settings row so FK constraints pass. +func seedArtist(t *testing.T, db *database.DB, id, name string) { + t.Helper() + if err := database.SaveArtistSettings(db, &database.ArtistSettings{ + ID: id, + Name: name, + Monitored: true, + }); err != nil { + t.Fatalf("seedArtist(%s) error: %v", id, err) + } +} + +// seedLocalAlbum inserts a local_albums row for an artist. +func seedLocalAlbum(t *testing.T, db *database.DB, id, artistID, title string) { + t.Helper() + if err := database.SaveLocalAlbum(db, &database.LocalAlbum{ + ID: id, + ArtistID: artistID, + Title: title, + }); err != nil { + t.Fatalf("seedLocalAlbum(%s) error: %v", id, err) + } +} + +// seedExternalRelease inserts an external_releases row for an artist. +func seedExternalRelease(t *testing.T, db *database.DB, rgid, artistID, title string, ignored bool) { + t.Helper() + if err := database.SaveExternalRelease(db, &database.ExternalRelease{ + RGID: rgid, + ArtistID: artistID, + Title: title, + IsIgnored: ignored, + }); err != nil { + t.Fatalf("seedExternalRelease(%s) error: %v", rgid, err) + } +} + +func TestScanArtist(t *testing.T) { + db := newTestDB(t) + defer db.Close() + + seedArtist(t, db, "artist-1", "Pink Floyd") + + // Local collection has "The Wall" but not "Animals". + seedLocalAlbum(t, db, "l1", "artist-1", "The Wall") + seedExternalRelease(t, db, "rg1", "artist-1", "The Wall", false) + seedExternalRelease(t, db, "rg2", "artist-1", "Animals", false) + + missing, err := ScanArtist(context.Background(), db, "artist-1", 0) + if err != nil { + t.Fatalf("ScanArtist() error: %v", err) + } + + rgids := map[string]bool{} + for _, m := range missing { + rgids[m.RGID] = true + } + if !rgids["rg2"] { + t.Errorf("expected rg2 (Animals) to be missing, got %v", rgids) + } + if rgids["rg1"] { + t.Errorf("did not expect rg1 (The Wall) to be missing, got %v", rgids) + } +} + +func TestScanArtist_IgnoredNotReported(t *testing.T) { + db := newTestDB(t) + defer db.Close() + + seedArtist(t, db, "artist-1", "Pink Floyd") + seedExternalRelease(t, db, "rg1", "artist-1", "Animals", true) + + missing, err := ScanArtist(context.Background(), db, "artist-1", 0.85) + if err != nil { + t.Fatalf("ScanArtist() error: %v", err) + } + if len(missing) != 0 { + t.Errorf("ignored release should not be reported, got %v", missing) + } +} + +func TestScanArtist_RemasteredVariantNotMissing(t *testing.T) { + db := newTestDB(t) + defer db.Close() + + seedArtist(t, db, "artist-1", "Pink Floyd") + seedLocalAlbum(t, db, "l1", "artist-1", "The Wall (Remastered)") + seedExternalRelease(t, db, "rg1", "artist-1", "The Wall", false) + + missing, err := ScanArtist(context.Background(), db, "artist-1", 0) + if err != nil { + t.Fatalf("ScanArtist() error: %v", err) + } + if len(missing) != 0 { + t.Errorf("remastered local should match external, got %v", missing) + } +} + +func TestScanArtist_CtxCancelled(t *testing.T) { + db := newTestDB(t) + defer db.Close() + seedArtist(t, db, "artist-1", "Pink Floyd") + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + if _, err := ScanArtist(ctx, db, "artist-1", 0.85); err == nil { + t.Fatal("expected error from cancelled context, got nil") + } +} + +func TestScanAll(t *testing.T) { + db := newTestDB(t) + defer db.Close() + + // Monitored artist with one missing release. + seedArtist(t, db, "artist-1", "Pink Floyd") + seedLocalAlbum(t, db, "l1", "artist-1", "The Wall") + seedExternalRelease(t, db, "rg1", "artist-1", "The Wall", false) + seedExternalRelease(t, db, "rg2", "artist-1", "Animals", false) + + // Unmonitored artist — must be skipped entirely. + seedArtistUnmonitored(t, db, "artist-2", "Other") + seedExternalRelease(t, db, "rg3", "artist-2", "Some Album", false) + + missing, err := ScanAll(context.Background(), db, 0) + if err != nil { + t.Fatalf("ScanAll() error: %v", err) + } + + rgids := map[string]bool{} + for _, m := range missing { + rgids[m.RGID] = true + } + if !rgids["rg2"] { + t.Errorf("expected rg2 (Animals) missing, got %v", rgids) + } + if rgids["rg1"] { + t.Errorf("did not expect rg1 (The Wall) missing, got %v", rgids) + } + if rgids["rg3"] { + t.Errorf("unmonitored artist's release must not be scanned, got %v", rgids) + } +} + +func TestScanAll_CtxCancelledMidIteration(t *testing.T) { + db := newTestDB(t) + defer db.Close() + + seedArtist(t, db, "artist-1", "Pink Floyd") + seedArtist(t, db, "artist-2", "Other") + + // Cancel before scanning starts. + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + missing, err := ScanAll(ctx, db, 0.85) + if err == nil { + t.Fatal("expected error from cancelled context, got nil") + } + if missing != nil { + t.Errorf("expected nil results on early cancellation, got %v", missing) + } +} + +// seedArtistUnmonitored inserts an artist_settings row with Monitored=false. +func seedArtistUnmonitored(t *testing.T, db *database.DB, id, name string) { + t.Helper() + if err := database.SaveArtistSettings(db, &database.ArtistSettings{ + ID: id, + Name: name, + Monitored: false, + }); err != nil { + t.Fatalf("seedArtistUnmonitored(%s) error: %v", id, err) + } +} diff --git a/internal/scanner/scanner.go b/internal/scanner/scanner.go index a1e12e2..f1ac132 100644 --- a/internal/scanner/scanner.go +++ b/internal/scanner/scanner.go @@ -10,6 +10,20 @@ import ( "naviwatcher/internal/normalize" ) +// DefaultThreshold is the fallback similarity threshold used when a caller +// passes threshold == 0. It matches config.Scanner.FuzzyThreshold default. +const DefaultThreshold = 0.85 + +// resolveThreshold returns the provided threshold, or DefaultThreshold when +// the caller passes zero (unset). This keeps the engine usable when config +// defaults are not threaded through explicitly. +func resolveThreshold(threshold float64) float64 { + if threshold == 0 { + return DefaultThreshold + } + return threshold +} + // Similarity returns a normalized similarity score in the range [0.0, 1.0] // between two strings. The strings are normalized first (lowercased, // bracketed/parenthesized content and years stripped, special characters