fix: address code review findings
- Fix artist-ID namespace mismatch in MusicBrainz provider: SyncArtistDiscography now stores the canonical Navidrome artist ID (artist_settings.id) as external_releases.artist_id instead of the MusicBrainz MBID. Previously the MBID was stored, which violated the FK to artist_settings and broke the scanner join (local_albums.artist_id is the Navidrome ID), causing every external release to be falsely reported as missing and the sync insert to fail at runtime. getArtistFilterOptions now also resolves by the Navidrome ID. - Resolve threshold in FindMissingReleases so the exported primitive honors the same zero-means-default contract as ScanArtist/ScanAll. - Remove dead maxLen==0 guard in scanner.Similarity. - Inline trivial buildPath helper; drop unused url import in client.go. - Replace hand-rolled itoa with strconv.Itoa in tests. - Rewrite SyncArtistDiscography tests to seed artist_settings with the Navidrome ID (tests previously seeded the MBID to mask the FK mismatch). - Fix TestFuzzySmoke to exercise the real dependency (fuzzy.LevenshteinDistance / scanner.Similarity) instead of an unused API. - Fix TestAppRun_ScanLogsMissingReleases to run the scan against a live context and assert the missing release is found. - Document cached_at column in Specification.md and note startup scan / required musicbrainz.user_agent in README. - Stop tracking .serena/ tooling config; add it to .gitignore.
This commit is contained in:
@@ -18,12 +18,18 @@ import (
|
||||
// 5. Within a transaction: delete old entries, then upsert each filtered release group.
|
||||
// 6. Return the list of external releases.
|
||||
//
|
||||
// artistID is the canonical artist key from artist_settings (the Navidrome
|
||||
// artist ID). It is stored as external_releases.artist_id so that the foreign
|
||||
// key to artist_settings and the scanner's join on ArtistID resolve correctly.
|
||||
// artistMBID is the MusicBrainz ID used only to query the MusicBrainz API.
|
||||
//
|
||||
// Context cancellation is checked before the API call and between each upsert
|
||||
// to allow graceful interruption.
|
||||
func SyncArtistDiscography(
|
||||
ctx context.Context,
|
||||
client *MusicBrainzClient,
|
||||
db *database.DB,
|
||||
artistID string,
|
||||
artistMBID string,
|
||||
ttl time.Duration,
|
||||
) ([]database.ExternalRelease, error) {
|
||||
@@ -33,7 +39,7 @@ func SyncArtistDiscography(
|
||||
}
|
||||
|
||||
// Step 1: Check cache.
|
||||
cachedReleases, err := GetCachedReleases(db, artistMBID, ttl)
|
||||
cachedReleases, err := GetCachedReleases(db, artistID, ttl)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("sync artist discography: cache check failed: %w", err)
|
||||
}
|
||||
@@ -53,7 +59,7 @@ func SyncArtistDiscography(
|
||||
}
|
||||
|
||||
// Step 4: Apply filtering with per-artist type preferences.
|
||||
opts, err := getArtistFilterOptions(db, artistMBID)
|
||||
opts, err := getArtistFilterOptions(db, artistID)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("sync artist discography: read artist filter options: %w", err)
|
||||
}
|
||||
@@ -69,7 +75,7 @@ func SyncArtistDiscography(
|
||||
|
||||
// Read existing ignore states before deleting to preserve user-set flags.
|
||||
ignoredMap := map[string]bool{}
|
||||
rows, err := tx.Query("SELECT rgid, is_ignored FROM external_releases WHERE artist_id = ?", artistMBID)
|
||||
rows, err := tx.Query("SELECT rgid, is_ignored FROM external_releases WHERE artist_id = ?", artistID)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("sync artist discography: query existing releases: %w", err)
|
||||
}
|
||||
@@ -89,11 +95,11 @@ func SyncArtistDiscography(
|
||||
// notifications_sent.rgid references external_releases.rgid.
|
||||
if _, err := tx.Exec(
|
||||
"DELETE FROM notifications_sent WHERE rgid IN (SELECT rgid FROM external_releases WHERE artist_id = ?)",
|
||||
artistMBID,
|
||||
artistID,
|
||||
); err != nil {
|
||||
return nil, fmt.Errorf("sync artist discography: delete old notifications: %w", err)
|
||||
}
|
||||
if _, err := tx.Exec("DELETE FROM external_releases WHERE artist_id = ?", artistMBID); err != nil {
|
||||
if _, err := tx.Exec("DELETE FROM external_releases WHERE artist_id = ?", artistID); err != nil {
|
||||
return nil, fmt.Errorf("sync artist discography: delete old releases: %w", err)
|
||||
}
|
||||
|
||||
@@ -104,7 +110,7 @@ func SyncArtistDiscography(
|
||||
return nil, fmt.Errorf("sync artist discography: %w", err)
|
||||
}
|
||||
|
||||
ext := rg.ToExternalRelease()
|
||||
ext := rg.ToExternalRelease(artistID)
|
||||
ext.CachedAt = now
|
||||
// Preserve user-set ignore flag from previous sync.
|
||||
if ignored, ok := ignoredMap[ext.RGID]; ok {
|
||||
|
||||
Reference in New Issue
Block a user