From b4e0756a90221d204a744ec4869de474e1a87a8c Mon Sep 17 00:00:00 2001 From: Connor Johnstone Date: Wed, 25 Mar 2026 14:32:17 -0400 Subject: [PATCH] proper fix plus delete artist actually removes files --- ...325_000019_allow_duplicate_artist_names.rs | 59 +++++++++++++++++++ src/migration/mod.rs | 2 + src/queries/artists.rs | 19 +++++- 3 files changed, 77 insertions(+), 3 deletions(-) create mode 100644 src/migration/m20260325_000019_allow_duplicate_artist_names.rs diff --git a/src/migration/m20260325_000019_allow_duplicate_artist_names.rs b/src/migration/m20260325_000019_allow_duplicate_artist_names.rs new file mode 100644 index 0000000..0923654 --- /dev/null +++ b/src/migration/m20260325_000019_allow_duplicate_artist_names.rs @@ -0,0 +1,59 @@ +use sea_orm_migration::prelude::*; + +#[derive(DeriveMigrationName)] +pub struct Migration; + +#[async_trait::async_trait] +impl MigrationTrait for Migration { + async fn up(&self, manager: &SchemaManager) -> Result<(), DbErr> { + // Drop the unique index on artist name — different artists can share a name + // (e.g., "Clara" the Italian singer and "Clara" the Brazilian singer) + manager + .drop_index( + Index::drop() + .name("idx_artists_name_unique") + .table(Artists::Table) + .to_owned(), + ) + .await?; + + // Replace with a non-unique index for lookup performance + manager + .create_index( + Index::create() + .name("idx_artists_name") + .table(Artists::Table) + .col(Artists::Name) + .to_owned(), + ) + .await + } + + async fn down(&self, manager: &SchemaManager) -> Result<(), DbErr> { + manager + .drop_index( + Index::drop() + .name("idx_artists_name") + .table(Artists::Table) + .to_owned(), + ) + .await?; + + manager + .create_index( + Index::create() + .name("idx_artists_name_unique") + .table(Artists::Table) + .col(Artists::Name) + .unique() + .to_owned(), + ) + .await + } +} + +#[derive(DeriveIden)] +enum Artists { + Table, + Name, +} diff --git a/src/migration/mod.rs b/src/migration/mod.rs index 9866cf0..67f568b 100644 --- a/src/migration/mod.rs +++ b/src/migration/mod.rs @@ -17,6 +17,7 @@ mod m20260320_000015_add_subsonic_password; mod m20260323_000016_remove_orphaned_artists; mod m20260323_000017_create_work_queue_and_scheduler; mod m20260324_000018_add_track_tagged; +mod m20260325_000019_allow_duplicate_artist_names; pub struct Migrator; @@ -41,6 +42,7 @@ impl MigratorTrait for Migrator { Box::new(m20260323_000016_remove_orphaned_artists::Migration), Box::new(m20260323_000017_create_work_queue_and_scheduler::Migration), Box::new(m20260324_000018_add_track_tagged::Migration), + Box::new(m20260325_000019_allow_duplicate_artist_names::Migration), ] } } diff --git a/src/queries/artists.rs b/src/queries/artists.rs index 5127a6b..3e9711d 100644 --- a/src/queries/artists.rs +++ b/src/queries/artists.rs @@ -21,13 +21,18 @@ pub async fn upsert( } if let Some(existing) = find_by_name(db, name).await? { - // Update musicbrainz_id if we have one now and didn't before if musicbrainz_id.is_some() && existing.musicbrainz_id.is_none() { + // We have an MBID now and the existing record doesn't — update it let mut active: ActiveModel = existing.into(); active.musicbrainz_id = Set(musicbrainz_id.map(String::from)); return Ok(active.update(db).await?); } - return Ok(existing); + if musicbrainz_id.is_none() || existing.musicbrainz_id.as_deref() == musicbrainz_id { + // No MBID provided, or MBIDs match — return existing + return Ok(existing); + } + // MBIDs differ — this is a different artist with the same name. + // Fall through to insert a new record. } // Try to insert — if we race with another task, catch the unique constraint @@ -48,7 +53,15 @@ pub async fn upsert( Err(DbErr::Exec(RuntimeErr::SqlxError(sqlx_err))) if sqlx_err.to_string().contains("UNIQUE constraint failed") => { - // Lost the race — another task inserted first, just look it up + // Lost the race on MBID unique constraint — look up by MBID first, then name + if let Some(mbid) = musicbrainz_id + && let Some(existing) = Artists::find() + .filter(artist::Column::MusicbrainzId.eq(mbid)) + .one(db) + .await? + { + return Ok(existing); + } find_by_name(db, name) .await? .ok_or_else(|| DbError::Other(format!("artist '{name}' vanished after conflict")))