diff --git a/backend/database/database.go b/backend/database/database.go index 05009d4..65b13b2 100644 --- a/backend/database/database.go +++ b/backend/database/database.go @@ -285,6 +285,18 @@ func runMigrations( } } + // Migration 5: rebuild release_groups with composite unique + // constraint on (name, album_artist_credit_id) instead of + // name alone, so albums with the same name by different + // artists are stored as separate rows. + if version < 5 { + if err := migration5ReleaseGroupCompositeUnique( + ctx, db, logger, + ); err != nil { + return err + } + } + return nil } @@ -458,6 +470,184 @@ func migration4TrackMetadataView( return nil } +// migration5ReleaseGroupCompositeUnique rebuilds the release_groups +// table with UNIQUE(name, album_artist_credit_id) instead of +// UNIQUE(name). SQLite cannot ALTER a UNIQUE constraint, so we +// must rebuild the table. +// +// SAFETY: Hand-crafted SQL for schema migration. +func migration5ReleaseGroupCompositeUnique( + ctx context.Context, + db *sql.DB, + logger *slog.Logger, +) error { + logger.Info( + "applying migration 5: release_groups composite unique constraint", + ) + + // Temporarily disable FK checks for table rebuild. + if _, err := db.ExecContext( + ctx, "PRAGMA foreign_keys = OFF", + ); err != nil { + return fmt.Errorf( + "migration 5: could not disable foreign keys: %w", + err, + ) + } + + // Drop the track_metadata VIEW that references release_groups + // so the table rebuild can proceed without SQLite complaining + // about a dangling VIEW reference. + if _, err := db.ExecContext( + ctx, "DROP VIEW IF EXISTS track_metadata", + ); err != nil { + return fmt.Errorf( + "migration 5: could not drop track_metadata VIEW: %w", + err, + ) + } + + // Create new table with composite unique constraint. + if _, err := db.ExecContext(ctx, ` + CREATE TABLE release_groups_new ( + id INTEGER PRIMARY KEY, + name TEXT NOT NULL, + cover_art_id INTEGER, + album_artist_credit_id INTEGER, + year INTEGER, + total_tracks INTEGER, + total_discs INTEGER, + FOREIGN KEY(cover_art_id) REFERENCES cover_art(id), + FOREIGN KEY(album_artist_credit_id) REFERENCES artist_credit(id), + UNIQUE(name, album_artist_credit_id) + ) + `); err != nil { + return fmt.Errorf( + "migration 5: could not create release_groups_new: %w", + err, + ) + } + + // Copy all data. + if _, err := db.ExecContext(ctx, ` + INSERT INTO release_groups_new + SELECT * FROM release_groups + `); err != nil { + return fmt.Errorf( + "migration 5: could not copy data: %w", err, + ) + } + + // Drop old table. + if _, err := db.ExecContext( + ctx, "DROP TABLE release_groups", + ); err != nil { + return fmt.Errorf( + "migration 5: could not drop old table: %w", err, + ) + } + + // Rename new table. + if _, err := db.ExecContext(ctx, ` + ALTER TABLE release_groups_new + RENAME TO release_groups + `); err != nil { + return fmt.Errorf( + "migration 5: could not rename table: %w", err, + ) + } + + // Recreate indexes. + if _, err := db.ExecContext(ctx, ` + CREATE INDEX IF NOT EXISTS idx_release_groups_cover_art_id + ON release_groups(cover_art_id) + `); err != nil { + return fmt.Errorf( + "migration 5: could not create cover_art_id index: %w", + err, + ) + } + + if _, err := db.ExecContext(ctx, ` + CREATE INDEX IF NOT EXISTS idx_release_groups_album_artist_credit_id + ON release_groups(album_artist_credit_id) + `); err != nil { + return fmt.Errorf( + "migration 5: could not create album_artist_credit_id index: %w", + err, + ) + } + + // Recreate the track_metadata VIEW that was dropped above. + // The definition must match the embedded schema file + // (sql/schemas/track_metadata_view.sql) exactly. + if _, err := db.ExecContext(ctx, ` + CREATE VIEW IF NOT EXISTS track_metadata AS + SELECT + af.id, + af.file_path, + af.length_milliseconds, + COALESCE(r.name, '') AS title, + COALESCE(ac.text, '') AS artist_name, + r.track_number, + r.disc_number, + COALESCE(rg.name, '') AS album, + CAST(COALESCE( + (SELECT GROUP_CONCAT(g.name, '||') + FROM recording_genres rg_sub + JOIN genres g ON rg_sub.genre_id = g.id + WHERE rg_sub.recording_id = r.id), + '' + ) AS TEXT) AS genre, + COALESCE(r.year, 0) AS year, + COALESCE(r.composer, '') AS composer, + COALESCE(ft.extension, '') AS file_type, + af.sample_rate, + af.bit_depth, + af.channels, + af.bitrate, + af.file_size + FROM audio_files af + LEFT JOIN recordings r ON af.recording_id = r.id + LEFT JOIN artist_credit ac ON r.artist_credit_id = ac.id + LEFT JOIN ( + SELECT recording_id, + MIN(release_group_id) AS release_group_id + FROM release_group_recordings + GROUP BY recording_id + ) rgr ON r.id = rgr.recording_id + LEFT JOIN release_groups rg ON rgr.release_group_id = rg.id + LEFT JOIN file_types ft ON af.file_type_id = ft.id + `); err != nil { + return fmt.Errorf( + "migration 5: could not recreate track_metadata VIEW: %w", + err, + ) + } + + // Re-enable FK checks. + if _, err := db.ExecContext( + ctx, "PRAGMA foreign_keys = ON", + ); err != nil { + return fmt.Errorf( + "migration 5: could not re-enable foreign keys: %w", + err, + ) + } + + if _, err := db.ExecContext( + ctx, "PRAGMA user_version = 5", + ); err != nil { + return fmt.Errorf( + "could not set user_version to 5: %w", err, + ) + } + + logger.Info("migration 5 complete") + + return nil +} + // isDuplicateColumnErr returns true when the error is SQLite's // "duplicate column name" error from an ALTER TABLE ADD COLUMN // on a column that already exists. diff --git a/backend/library/library.go b/backend/library/library.go index 1d7149b..63bb7bf 100644 --- a/backend/library/library.go +++ b/backend/library/library.go @@ -1261,8 +1261,18 @@ func (l *Library) resolveReleaseGroup( return sql.NullInt64{} } + // Build composite cache key: "albumName\x00artistCreditID" + // (or "albumName\x00-1" if no artist). This prevents albums + // with the same name by different artists from colliding. + artistID := int64(-1) + if albumArtistCreditID.Valid { + artistID = albumArtistCreditID.Int64 + } + + cacheKey := fmt.Sprintf("%s\x00%d", tags.Album, artistID) + // Check cache first. - if cached, ok := cache.releaseGroups[tags.Album]; ok { + if cached, ok := cache.releaseGroups[cacheKey]; ok { // If the cached release group lacks cover art and we now // have it, update it. if coverArtID.Valid && !cached.CoverArtID.Valid { @@ -1280,7 +1290,7 @@ func (l *Library) resolveReleaseGroup( ) } else { cached.CoverArtID = coverArtID - cache.releaseGroups[tags.Album] = cached + cache.releaseGroups[cacheKey] = cached } } @@ -1321,7 +1331,7 @@ func (l *Library) resolveReleaseGroup( } } - cache.releaseGroups[tags.Album] = rg + cache.releaseGroups[cacheKey] = rg return sql.NullInt64{Int64: rg.ID, Valid: true} } diff --git a/backend/library/scan_test.go b/backend/library/scan_test.go index d4de577..97ec792 100644 --- a/backend/library/scan_test.go +++ b/backend/library/scan_test.go @@ -533,7 +533,9 @@ func TestResolveReleaseGroup(t *testing.T) { } // Cover art should be updated on the cached release group. - cachedRG := cache.releaseGroups["A Night at the Opera"] + // Cache key is composite: "albumName\x00artistCreditID". + cacheKey := fmt.Sprintf("%s\x00%d", "A Night at the Opera", ac.ID) + cachedRG := cache.releaseGroups[cacheKey] if !cachedRG.CoverArtID.Valid { t.Error("expected CoverArtID to be set after update") } @@ -559,7 +561,8 @@ func TestResolveReleaseGroup_CacheHit(t *testing.T) { q := lib.db.Queries // Pre-populate cache with a known release group. - cache.releaseGroups["Cached Album"] = sqlcgen.ReleaseGroup{ + // Cache key is composite: "albumName\x00artistCreditID" (use -1 for no artist). + cache.releaseGroups[fmt.Sprintf("%s\x00%d", "Cached Album", int64(-1))] = sqlcgen.ReleaseGroup{ ID: 42, Name: "Cached Album", }