diff --git a/cmd/naviwatcher/main.go b/cmd/naviwatcher/main.go index 755d3a0..447b4d0 100644 --- a/cmd/naviwatcher/main.go +++ b/cmd/naviwatcher/main.go @@ -84,6 +84,9 @@ func NewApp(ctx context.Context, cfg *config.Config) (*App, error) { // Close cleans up all application resources in reverse order of initialization. func (a *App) Close() { + if a.mbClient != nil { + a.mbClient.Close() + } if a.db != nil { if err := a.db.Close(); err != nil { log.Printf("Error closing database: %v", err) diff --git a/internal/database/database.go b/internal/database/database.go index bda87c8..da3858a 100644 --- a/internal/database/database.go +++ b/internal/database/database.go @@ -15,7 +15,11 @@ type DB struct { // New opens a SQLite database at dbPath and runs schema migrations. func New(dbPath string) (*DB, error) { - conn, err := sql.Open("sqlite3", dbPath) + // The _foreign_keys=on DSN parameter enables foreign key enforcement on + // EVERY connection in the pool. A one-off "PRAGMA foreign_keys=ON" executed + // on the pooled *sql.DB only applies to the first connection and is lost on + // connections opened later by the pool, silently disabling the safety net. + conn, err := sql.Open("sqlite3", dbPath+"?_foreign_keys=on") if err != nil { return nil, fmt.Errorf("open database: %w", err) } @@ -26,12 +30,6 @@ func New(dbPath string) (*DB, error) { return nil, fmt.Errorf("set WAL mode: %w", err) } - // Enable foreign key enforcement. - if _, err := conn.Exec("PRAGMA foreign_keys=ON"); err != nil { - conn.Close() - return nil, fmt.Errorf("enable foreign keys: %w", err) - } - // Set busy timeout to handle concurrent write contention. if _, err := conn.Exec("PRAGMA busy_timeout=5000"); err != nil { conn.Close() diff --git a/internal/database/external_releases.go b/internal/database/external_releases.go index 753e566..9b9c4d1 100644 --- a/internal/database/external_releases.go +++ b/internal/database/external_releases.go @@ -119,28 +119,6 @@ func SetReleaseIgnored(db *DB, rgid string, ignored bool) error { return nil } -// DeleteExternalReleasesByArtist removes all external_release rows for a given artist_id. -func DeleteExternalReleasesByArtist(db *DB, artistID string) error { - _, err := db.Conn().Exec( - "DELETE FROM external_releases WHERE artist_id = ?", - artistID, - ) - if err != nil { - return fmt.Errorf("delete external releases by artist: %w", err) - } - return nil -} - -// CountExternalReleases returns the total number of external_release rows. -func CountExternalReleases(db *DB) (int, error) { - var count int - err := db.Conn().QueryRow("SELECT COUNT(*) FROM external_releases").Scan(&count) - if err != nil { - return 0, fmt.Errorf("count external releases: %w", err) - } - return count, nil -} - // GetExternalReleasesByArtistWithCache returns cached external_release rows for a given artist_id // that are within the specified TTL. func GetExternalReleasesByArtistWithCache(db *DB, artistID string, ttl time.Duration) ([]ExternalRelease, error) { diff --git a/internal/musicbrainz/api.go b/internal/musicbrainz/api.go index 2c65912..476c014 100644 --- a/internal/musicbrainz/api.go +++ b/internal/musicbrainz/api.go @@ -6,7 +6,6 @@ import ( "net/url" "naviwatcher/internal/database" - "naviwatcher/internal/normalize" ) // excludedStatuses contains release-group statuses that should be filtered out. @@ -112,20 +111,6 @@ func IsTypeIncluded(releaseType string) bool { return includedTypes[releaseType] } -// NormalizeString normalizes a string for fuzzy matching. -// It delegates to the shared normalize package; see normalize.NormalizeString -// for the full normalization contract. -func NormalizeString(s string) string { - return normalize.NormalizeString(s) -} - -// NormalizeArtistName normalizes an artist name for comparison. -// It delegates to the shared normalize package; see -// normalize.NormalizeArtistName for the full normalization contract. -func NormalizeArtistName(name string) string { - return normalize.NormalizeArtistName(name) -} - // ToExternalRelease converts a ReleaseGroup to an ExternalRelease for database // persistence. artistID is the canonical artist key from artist_settings (the // Navidrome artist ID), which is what external_releases.artist_id references and diff --git a/internal/musicbrainz/api_test.go b/internal/musicbrainz/api_test.go index 0f4a809..a3fac59 100644 --- a/internal/musicbrainz/api_test.go +++ b/internal/musicbrainz/api_test.go @@ -93,81 +93,6 @@ func TestFilterReleaseGroups_IgnoreCompilations(t *testing.T) { } } -// ---------- NormalizeString tests ---------- - -func TestNormalizeString_Basic(t *testing.T) { - tests := []struct { - input string - expected string - }{ - // Lowercase conversion - {"DARK SIDE OF THE MOON", "dark side of the moon"}, - // Special character removal - {"Dark Side of the Moon!", "dark side of the moon"}, - {"Dark-Side-of-the-Moon", "dark side of the moon"}, - {"Dark_Side_of_the_Moon", "dark side of the moon"}, - // Bracket removal - {"Dark Side of the Moon [Deluxe Edition]", "dark side of the moon"}, - {"Dark Side of the Moon [Remastered 2020]", "dark side of the moon"}, - {"Album [2023 Remix]", "album"}, - // Parenthesis removal - {"Dark Side of the Moon (Deluxe)", "dark side of the moon"}, - {"Album (Remastered)", "album"}, - // Year removal - {"Dark Side of the Moon 1973", "dark side of the moon"}, - {"Album 2020 Remastered", "album remastered"}, - // Space collapsing - {"Dark Side of the Moon", "dark side of the moon"}, - // Trim - {" Dark Side of the Moon ", "dark side of the moon"}, - // Combined - {"The Dark Side of the Moon [2011 Remaster] (Deluxe Edition)", "the dark side of the moon"}, - // Empty - {"", ""}, - // Only special chars - {"!@#$%^&*()", ""}, - // Digits that are not years should stay - {"30 Seconds to Mars", "30 seconds to mars"}, - {"1941 - The Greatest Hits", "the greatest hits"}, - } - - for _, tt := range tests { - t.Run(tt.input, func(t *testing.T) { - got := NormalizeString(tt.input) - if got != tt.expected { - t.Errorf("NormalizeString(%q) = %q, want %q", tt.input, got, tt.expected) - } - }) - } -} - -func TestNormalizeArtistName(t *testing.T) { - tests := []struct { - input string - expected string - }{ - {"Pink Floyd", "pink floyd"}, - {"The Beatles", "beatles"}, - {"A Perfect Circle", "perfect circle"}, - {"An Orchestra", "orchestra"}, - {" The Who ", "who"}, - {"THE WHO", "who"}, - // No stripping needed - {"Radiohead", "radiohead"}, - // Already stripped - {"Beatles", "beatles"}, - } - - for _, tt := range tests { - t.Run(tt.input, func(t *testing.T) { - got := NormalizeArtistName(tt.input) - if got != tt.expected { - t.Errorf("NormalizeArtistName(%q) = %q, want %q", tt.input, got, tt.expected) - } - }) - } -} - // ---------- ReleaseGroup.ToExternalRelease tests ---------- func TestReleaseGroup_ToExternalRelease(t *testing.T) { diff --git a/internal/musicbrainz/client.go b/internal/musicbrainz/client.go index 3198a3a..df2cecf 100644 --- a/internal/musicbrainz/client.go +++ b/internal/musicbrainz/client.go @@ -35,6 +35,12 @@ func NewClient(cfg config.MusicBrainzConfig) *MusicBrainzClient { } } +// Close releases resources held by the client, draining any idle keep-alive +// connections so they don't linger until garbage collection. +func (c *MusicBrainzClient) Close() { + c.httpClient.CloseIdleConnections() +} + // doGet performs a rate-limited HTTP GET request to the MusicBrainz API. // It blocks until the rate limiter allows the request, then sets the proper // User-Agent header and returns the response body. diff --git a/internal/normalize/normalize.go b/internal/normalize/normalize.go index 18fdb6f..2986ffe 100644 --- a/internal/normalize/normalize.go +++ b/internal/normalize/normalize.go @@ -18,6 +18,10 @@ var ( parenRe = regexp.MustCompile(`\([^)]*\)`) yearRe = regexp.MustCompile(`\b(1[0-9]{3}|2[0-9]{3})\b`) spaceRe = regexp.MustCompile(`\s+`) + // wordRe matches any alphabetic character. Used to decide whether a title + // that collapses entirely to a year actually had other words worth keeping + // (e.g. "1989 (Deluxe)") versus being a bare year title (e.g. "1989"). + wordRe = regexp.MustCompile(`[a-z]`) ) // NormalizeString normalizes a string for fuzzy matching by: @@ -28,6 +32,10 @@ var ( // - Collapsing multiple spaces into one // - Trimming leading/trailing whitespace func NormalizeString(s string) string { + // Capture the original input; used after stripping to tell a bare year + // title apart from a title that merely collapses to a year. + original := s + // Convert to lowercase s = strings.ToLower(s) @@ -37,11 +45,23 @@ func NormalizeString(s string) string { // Remove parenthesized content (e.g., (Deluxe), (Remastered)) s = parenRe.ReplaceAllString(s, "") - // Remove years (4-digit numbers between 1000-2999). If stripping the - // year would empty the entire string (e.g. an album literally titled - // "1989" or "2112"), keep the original form so the title can still match. + // Remove years (4-digit numbers between 1000-2999). If stripping the year + // would empty the entire string, we must decide what to keep: + // - A bare year title (e.g. "1989", "2112") has no other words, so keep + // the year so it can still match itself (the user owns that album). + // - A title that had OTHER words alongside the year (e.g. "1989 (Deluxe)") + // collapses to empty on purpose: it is a distinct release group that + // must NOT be considered already-present just because the user owns the + // standard "1989". Collapsing to empty makes it score 0.0 against a + // plain "1989", correctly reporting the reissue as missing. stripped := yearRe.ReplaceAllString(s, "") - if strings.TrimSpace(stripped) != "" { + if strings.TrimSpace(stripped) == "" { + if wordRe.MatchString(strings.ToLower(original)) { + s = "" + } else { + s = strings.TrimSpace(s) + } + } else { s = stripped } diff --git a/internal/normalize/normalize_test.go b/internal/normalize/normalize_test.go index 02992a8..010e382 100644 --- a/internal/normalize/normalize_test.go +++ b/internal/normalize/normalize_test.go @@ -39,6 +39,11 @@ func TestNormalizeString_Basic(t *testing.T) { // Year-only title is preserved (not collapsed to empty) so it can still match {"1989", "1989"}, {"2112", "2112"}, + // A year-plus-suffix title collapses to empty: it is a distinct release + // group (e.g. "1989 (Deluxe)") and must NOT match a bare "1989". + {"1989 (Deluxe)", ""}, + {"1989 [Deluxe Edition]", ""}, + {"2112 (Remastered)", ""}, } for _, tt := range tests { diff --git a/internal/scanner/scan.go b/internal/scanner/scan.go index f5bb872..18a874c 100644 --- a/internal/scanner/scan.go +++ b/internal/scanner/scan.go @@ -2,6 +2,7 @@ package scanner import ( "context" + "log" "naviwatcher/internal/database" ) @@ -55,9 +56,12 @@ func ScanAll(ctx context.Context, db *database.DB, threshold float64) ([]Missing if !s.Monitored { continue } + // A transient error for one artist must not abort the whole scan and + // take down the daemon; log it and continue with the remaining artists. missing, err := ScanArtist(ctx, db, s.ID, resolved) if err != nil { - return all, err + log.Printf("scan artist %s failed: %v", s.ID, err) + continue } all = append(all, missing...) } diff --git a/internal/scanner/scan_test.go b/internal/scanner/scan_test.go index c2710b8..b34e39e 100644 --- a/internal/scanner/scan_test.go +++ b/internal/scanner/scan_test.go @@ -139,6 +139,35 @@ func TestScanArtist_RemasteredVariantNotMissing(t *testing.T) { } } +func TestScanArtist_YearTitledAlbumReissueReportedMissing(t *testing.T) { + db := newTestDB(t) + defer db.Close() + + // Rush "2112" is a bare year-titled album; "2112 (Remastered)" is a + // distinct release group. Owning the standard 2112 must NOT count as owning + // the remastered reissue — the reissue should be reported missing. + seedArtist(t, db, "artist-1", "Rush") + seedLocalAlbum(t, db, "l1", "artist-1", "2112") + seedExternalRelease(t, db, "rg-standard", "artist-1", "2112", false) + seedExternalRelease(t, db, "rg-remaster", "artist-1", "2112 (Remastered)", 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["rg-standard"] { + t.Errorf("standard 2112 should match local copy, not be missing") + } + if !rgids["rg-remaster"] { + t.Errorf("2112 (Remastered) reissue should be reported missing, got %v", rgids) + } +} + func TestScanArtist_CtxCancelled(t *testing.T) { db := newTestDB(t) defer db.Close()