fix: address code review findings

- Fix pagination to not trust MusicBrainz count for cutoff (prevents
  silent discography truncation / hidden missing releases)
- Add idx_external_releases_artist_id index for hot per-artist queries
- Dedupe cached_at layout constant in external_releases.go
This commit is contained in:
2026-07-19 21:13:24 +03:00
parent ab2af62cb0
commit e2de91c5d3
4 changed files with 21 additions and 13 deletions

View File

@@ -121,6 +121,10 @@ func (db *DB) migrate() error {
name: "006_add_secondary_types_to_external_releases", name: "006_add_secondary_types_to_external_releases",
sql: `ALTER TABLE external_releases ADD COLUMN secondary_types TEXT;`, sql: `ALTER TABLE external_releases ADD COLUMN secondary_types TEXT;`,
}, },
{
name: "007_index_external_releases_artist_id",
sql: `CREATE INDEX IF NOT EXISTS idx_external_releases_artist_id ON external_releases(artist_id);`,
},
} }
for _, m := range migrations { for _, m := range migrations {

View File

@@ -205,9 +205,10 @@ func TestMigrationTracking(t *testing.T) {
t.Fatalf("query migrations count: %v", err) t.Fatalf("query migrations count: %v", err)
} }
// We have 6 recorded migrations: artist_settings, external_releases, // We have 7 recorded migrations: artist_settings, external_releases,
// local_albums, notifications_sent, cached_at column, secondary_types column. // local_albums, notifications_sent, cached_at column, secondary_types
if count != 6 { // column, and the external_releases.artist_id index.
t.Errorf("expected 6 applied migrations, got %d", count) if count != 7 {
t.Errorf("expected 7 applied migrations, got %d", count)
} }
} }

View File

@@ -175,11 +175,10 @@ func GetExternalReleasesByArtistWithCache(db *DB, artistID string, ttl time.Dura
if ttl <= 0 { if ttl <= 0 {
return nil, nil return nil, nil
} }
// cached_at is stored in the "2006-01-02 15:04:05" UTC layout via // cached_at is stored in utcLayout via FormatCachedAt. Compare against an
// FormatCachedAt. Compare against an explicitly formatted cutoff string in // explicitly formatted cutoff string in the same layout so the
// the same layout so the lexicographic comparison is a valid time ordering. // lexicographic comparison is a valid time ordering.
const layout = "2006-01-02 15:04:05" cutoff := time.Now().UTC().Add(-ttl).Format(utcLayout)
cutoff := time.Now().UTC().Add(-ttl).Format(layout)
rows, err := db.Conn().Query( rows, err := db.Conn().Query(
"SELECT rgid, artist_id, title, type, release_date, is_ignored, cached_at, secondary_types FROM external_releases WHERE artist_id = ? AND cached_at >= ?", "SELECT rgid, artist_id, title, type, release_date, is_ignored, cached_at, secondary_types FROM external_releases WHERE artist_id = ? AND cached_at >= ?",
artistID, cutoff, artistID, cutoff,

View File

@@ -46,10 +46,14 @@ func (c *MusicBrainzClient) GetArtistReleaseGroups(ctx context.Context, artistMB
allGroups = append(allGroups, parsed.ReleaseGroups...) allGroups = append(allGroups, parsed.ReleaseGroups...)
// If we've fetched all results, we've reached the end. // Stop when a page is empty (no more results) or when the page
// Also break on empty page to prevent infinite loop if API // returned fewer items than the request limit — a reliable end-of-data
// returns fewer items than advertised by count. // signal. We intentionally do NOT trust parsed.Count for the cutoff:
if len(parsed.ReleaseGroups) == 0 || offset+len(parsed.ReleaseGroups) >= parsed.Count { // MusicBrainz occasionally reports an inaccurate count, which would
// prematurely truncate an artist's discography and hide missing
// releases. The empty-page check also prevents an infinite loop if the
// API keeps returning a non-empty page past the reported count.
if len(parsed.ReleaseGroups) == 0 || len(parsed.ReleaseGroups) < limit {
break break
} }