fix(quick-10): add migration 5 and fix entity cache for composite album key
- Migration 5 rebuilds release_groups with UNIQUE(name, album_artist_credit_id) - Drops and recreates track_metadata VIEW during table rebuild - Temporarily disables FK checks for safe table rebuild - Entity cache now keys by album name + artist credit ID - Update tests to use composite cache keys
This commit is contained in:
@@ -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
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -458,6 +470,184 @@ func migration4TrackMetadataView(
|
|||||||
return nil
|
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
|
// isDuplicateColumnErr returns true when the error is SQLite's
|
||||||
// "duplicate column name" error from an ALTER TABLE ADD COLUMN
|
// "duplicate column name" error from an ALTER TABLE ADD COLUMN
|
||||||
// on a column that already exists.
|
// on a column that already exists.
|
||||||
|
|||||||
@@ -1261,8 +1261,18 @@ func (l *Library) resolveReleaseGroup(
|
|||||||
return sql.NullInt64{}
|
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.
|
// 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
|
// If the cached release group lacks cover art and we now
|
||||||
// have it, update it.
|
// have it, update it.
|
||||||
if coverArtID.Valid && !cached.CoverArtID.Valid {
|
if coverArtID.Valid && !cached.CoverArtID.Valid {
|
||||||
@@ -1280,7 +1290,7 @@ func (l *Library) resolveReleaseGroup(
|
|||||||
)
|
)
|
||||||
} else {
|
} else {
|
||||||
cached.CoverArtID = coverArtID
|
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}
|
return sql.NullInt64{Int64: rg.ID, Valid: true}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -533,7 +533,9 @@ func TestResolveReleaseGroup(t *testing.T) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Cover art should be updated on the cached release group.
|
// 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 {
|
if !cachedRG.CoverArtID.Valid {
|
||||||
t.Error("expected CoverArtID to be set after update")
|
t.Error("expected CoverArtID to be set after update")
|
||||||
}
|
}
|
||||||
@@ -559,7 +561,8 @@ func TestResolveReleaseGroup_CacheHit(t *testing.T) {
|
|||||||
q := lib.db.Queries
|
q := lib.db.Queries
|
||||||
|
|
||||||
// Pre-populate cache with a known release group.
|
// 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,
|
ID: 42,
|
||||||
Name: "Cached Album",
|
Name: "Cached Album",
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user