From f7f8e6735a8c7ab5ffd11583e3553e1ab2c0315d Mon Sep 17 00:00:00 2001 From: Tornade CI Date: Fri, 27 Feb 2026 11:11:52 +0100 Subject: [PATCH 1/5] feat(ffi): expose import_m3u_playlist via swift-bridge Creates a new playlist from an M3U file (name from file stem), matching tracks by file path against the library database. Co-Authored-By: Claude Sonnet 4.6 --- src/ffi.rs | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/ffi.rs b/src/ffi.rs index 9792ca2..27b80f7 100644 --- a/src/ffi.rs +++ b/src/ffi.rs @@ -174,6 +174,7 @@ mod ffi { fn delete_playlist(playlist_id: i64) -> String; fn add_track_to_playlist(playlist_id: i64, track_id: i64) -> String; fn remove_track_from_playlist(playlist_id: i64, position: i64) -> String; + fn import_m3u_playlist(path: &str) -> String; // Playback Control Functions fn play_track(track_id: i64) -> String; @@ -1084,6 +1085,33 @@ fn remove_track_from_playlist(playlist_id: i64, position: i64) -> String { } } +fn import_m3u_playlist(path: &str) -> String { + // Import a .m3u file and create a new playlist from it + match get_or_init_pool() { + Ok(pool) => { + let playlist_service = PlaylistService::new(pool.clone()); + let file_path = std::path::Path::new(path); + match playlist_service.import_m3u(file_path) { + Ok(playlist) => serde_json::json!({ + "success": true, + "data": playlist + }) + .to_string(), + Err(e) => serde_json::json!({ + "success": false, + "error": format!("Failed to import M3U: {}", e) + }) + .to_string(), + } + } + Err(e) => serde_json::json!({ + "success": false, + "error": format!("FFI initialization failed: {}", e) + }) + .to_string(), + } +} + fn play_track(track_id: i64) -> String { // T024: Play a specific track // Sets queue to just this track and starts playback From ea9068ab72421a0caa1796dffc0af01b9f8383c0 Mon Sep 17 00:00:00 2001 From: Tornade CI Date: Sun, 1 Mar 2026 16:14:49 +0100 Subject: [PATCH 2/5] fix: expose get_artist_albums via FFI with inclusive track-artist query MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, artist detail view had no way to fetch albums by artist_id directly — Swift was paginating the entire album catalog and filtering client-side, which is both slow and incorrect for artists who only appear as track artists (feat. / featuring). Changes: - queries::get_artist_albums: extend WHERE clause with OR EXISTS subquery so albums where the artist has at least one track are also returned, even if they are not the album artist (covers "Various Artists"-upgraded compilations and feat. artists) - ffi.rs: expose get_artist_albums(artist_id) via swift-bridge so Swift can call it directly with a single FFI round-trip A new test (test_get_artist_albums_includes_track_artist) verifies that albums are found for artists who are track-only contributors. Co-Authored-By: Claude Sonnet 4.6 --- src/db/queries.rs | 39 ++++++++++++++++++++++++++++++++++++++- src/ffi.rs | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-) diff --git a/src/db/queries.rs b/src/db/queries.rs index 496c783..f54340a 100644 --- a/src/db/queries.rs +++ b/src/db/queries.rs @@ -487,12 +487,13 @@ pub fn list_albums( pub fn get_artist_albums(conn: &Connection, artist_id: i64) -> Result> { let mut stmt = conn.prepare( - "SELECT a.id, a.title, a.artist_id, ar.name as artist_name, a.year, a.rating, + "SELECT DISTINCT a.id, a.title, a.artist_id, ar.name as artist_name, a.year, a.rating, a.artwork_path, a.online_artwork_path, a.description, a.musicbrainz_id, a.label, a.country, a.barcode, a.album_type, a.release_status FROM albums a JOIN artists ar ON ar.id = a.artist_id WHERE a.artist_id = ?1 + OR EXISTS (SELECT 1 FROM tracks t WHERE t.album_id = a.id AND t.artist_id = ?1) ORDER BY a.year DESC, a.title", )?; @@ -1072,6 +1073,42 @@ mod tests { assert_eq!(albums.len(), 2); } + #[test] + fn test_get_artist_albums_includes_track_artist() { + // Albums where the artist only appears as a track artist (not album artist) + // should also be returned — covers "feat." artists and VA-upgraded albums. + let env = TestEnv::new(); + let conn = env.pool.get().unwrap(); + let source_id = insert_source(&conn, "Library", SourceType::Disk, None).unwrap(); + let album_artist_id = insert_artist(&conn, "Album Artist", None).unwrap(); + let feat_artist_id = insert_artist(&conn, "Featured Artist", None).unwrap(); + let album_id = insert_album(&conn, "Collab Album", album_artist_id, Some(2022)).unwrap(); + insert_track( + &conn, + "Collab Track", + Some(album_id), + feat_artist_id, + source_id, + std::path::Path::new("/music/collab.flac"), + 180_000, + None, + None, + None, + AudioFormat::Flac, + 1_000_000, + ) + .unwrap(); + + // feat_artist_id is not the album artist but has a track on the album + let albums = get_artist_albums(&conn, feat_artist_id).unwrap(); + assert_eq!(albums.len(), 1); + assert_eq!(albums[0].title, "Collab Album"); + + // album_artist_id still gets the album too + let albums = get_artist_albums(&conn, album_artist_id).unwrap(); + assert_eq!(albums.len(), 1); + } + // ==================================================================== // Album tests // ==================================================================== diff --git a/src/ffi.rs b/src/ffi.rs index 27b80f7..369b7af 100644 --- a/src/ffi.rs +++ b/src/ffi.rs @@ -158,6 +158,7 @@ mod ffi { // Artist Functions fn get_artists_page(offset: u32, limit: u32) -> String; fn get_artist_by_id(artist_id: i64) -> String; + fn get_artist_albums(artist_id: i64) -> String; // Genre Functions fn get_genres() -> String; @@ -810,6 +811,42 @@ fn get_artist_by_id(artist_id: i64) -> String { } } +fn get_artist_albums(artist_id: i64) -> String { + // Get all albums for an artist (as album artist OR as track artist) + match get_or_init_pool() { + Ok(pool) => { + let conn = match pool.get() { + Ok(conn) => conn, + Err(e) => { + return serde_json::json!({ + "success": false, + "error": format!("Failed to get database connection: {}", e) + }) + .to_string(); + } + }; + + match crate::db::queries::get_artist_albums(&conn, artist_id) { + Ok(albums) => serde_json::json!({ + "success": true, + "data": { "albums": albums } + }) + .to_string(), + Err(e) => serde_json::json!({ + "success": false, + "error": format!("Failed to get artist albums: {}", e) + }) + .to_string(), + } + } + Err(e) => serde_json::json!({ + "success": false, + "error": format!("Database pool error: {}", e) + }) + .to_string(), + } +} + fn get_artists_page(offset: u32, limit: u32) -> String { // T018: Get paginated list of artists match get_or_init_pool() { From d00cd650199f141cdab05d32595ced07cb2b10fc Mon Sep 17 00:00:00 2001 From: Tornade CI Date: Sun, 1 Mar 2026 17:01:48 +0100 Subject: [PATCH 3/5] fix(scan): use (title, artist) as album identity instead of title-only Previously, albums without an ALBUMARTIST tag were matched by title only, causing "Greatest Hits" by The Beatles and "Greatest Hits" by Eminem to be merged into a single "Various Artists" album. Album identity is now always (title, artist_id), matching the UNIQUE constraint already present in the database schema. Each distinct (title, primary_artist) pair produces its own album row. Compilations with multiple track artists must be tagged with ALBUMARTIST=Various Artists in the file metadata to be grouped correctly. Removes: find_album_by_title (title-only lookup, no longer needed) Keeps: update_album_artist (still useful for tagged-ALBUMARTIST path) Co-Authored-By: Claude Sonnet 4.6 --- src/db/queries.rs | 35 +-------- src/services/library.rs | 153 ++++++++++------------------------------ 2 files changed, 38 insertions(+), 150 deletions(-) diff --git a/src/db/queries.rs b/src/db/queries.rs index f54340a..bafa4fe 100644 --- a/src/db/queries.rs +++ b/src/db/queries.rs @@ -164,20 +164,6 @@ pub fn get_album(conn: &Connection, id: i64) -> Result> { .optional() } -/// Find any existing album with this exact title, regardless of artist. -/// Used when no ALBUMARTIST tag is present to avoid creating a separate album -/// entry for every featured artist in a compilation. -/// Find an album by title, returning `(album_id, artist_id)`. -/// Used when no ALBUMARTIST tag is present so we can detect multi-artist albums. -pub fn find_album_by_title(conn: &Connection, title: &str) -> Result> { - conn.query_row( - "SELECT id, artist_id FROM albums WHERE title = ?1 LIMIT 1", - params![title], - |row| Ok((row.get(0)?, row.get(1)?)), - ) - .optional() -} - pub fn update_album_artist(conn: &Connection, album_id: i64, artist_id: i64) -> Result<()> { conn.execute( "UPDATE albums SET artist_id = ?1 WHERE id = ?2", @@ -1157,23 +1143,6 @@ mod tests { assert_eq!(tracks.len(), 2); } - #[test] - fn test_find_album_by_title_returns_id_and_artist_id() { - let env = TestEnv::new(); - let conn = env.pool.get().unwrap(); - let artist_id = insert_artist(&conn, "Akhenaton", None).unwrap(); - let album_id = insert_album(&conn, "Sol Invictus", artist_id, Some(2001)).unwrap(); - let found = find_album_by_title(&conn, "Sol Invictus").unwrap(); - assert_eq!(found, Some((album_id, artist_id))); - } - - #[test] - fn test_find_album_by_title_returns_none_when_missing() { - let env = TestEnv::new(); - let conn = env.pool.get().unwrap(); - assert!(find_album_by_title(&conn, "Unknown").unwrap().is_none()); - } - #[test] fn test_update_album_artist() { let env = TestEnv::new(); @@ -1182,8 +1151,8 @@ mod tests { let artist2 = insert_artist(&conn, "Various Artists", None).unwrap(); let album_id = insert_album(&conn, "Compilation", artist1, None).unwrap(); update_album_artist(&conn, album_id, artist2).unwrap(); - let (_, returned_artist) = find_album_by_title(&conn, "Compilation").unwrap().unwrap(); - assert_eq!(returned_artist, artist2); + let album = get_album(&conn, album_id).unwrap().unwrap(); + assert_eq!(album.artist_id, artist2); } #[test] diff --git a/src/services/library.rs b/src/services/library.rs index 605c166..6577d65 100644 --- a/src/services/library.rs +++ b/src/services/library.rs @@ -307,43 +307,21 @@ impl LibraryService { // Get or create album if present. // - // When ALBUMARTIST is set, trust it and create/find the album by (title, artist). - // When ALBUMARTIST is absent (common in compilations/mixtapes), first look for any - // existing album with the same title and reuse it — this prevents creating a separate - // album entry for every featured artist on a "70s Mixtape" or "80's Greatest Hits". - // If a second distinct artist is detected, the album is upgraded to "Various Artists" - // so it doesn't belong to a single (arbitrary) artist. + // Album identity is always (title, artist_id) — the same UNIQUE constraint used + // in the database. When ALBUMARTIST is set, we use it as the artist; otherwise + // we use the primary track artist. + // + // This means "Greatest Hits" by The Beatles and "Greatest Hits" by Eminem are + // always stored as two separate albums, even if neither file carries an ALBUMARTIST + // tag. Compilations or multi-artist albums *must* be tagged with + // ALBUMARTIST=Various Artists to be grouped correctly. let album_id = if let Some(ref album_title) = metadata.album { - if metadata.album_artist.is_some() { - // ALBUMARTIST is set: use it, create per (title, artist) as normal - Some(queries::insert_album( - conn, - album_title, - album_artist_id, - metadata.year, - )?) - } else { - // No ALBUMARTIST: reuse any existing album with this title to avoid duplicates - match queries::find_album_by_title(conn, album_title)? { - Some((existing_id, existing_artist_id)) => { - // If a second distinct artist appears on this album, upgrade it to - // "Various Artists" so no single real artist "owns" it - if existing_artist_id != album_artist_id { - let va_id = queries::insert_artist(conn, "Various Artists", None)?; - if existing_artist_id != va_id { - queries::update_album_artist(conn, existing_id, va_id)?; - } - } - Some(existing_id) - } - None => Some(queries::insert_album( - conn, - album_title, - album_artist_id, - metadata.year, - )?), - } - } + Some(queries::insert_album( + conn, + album_title, + album_artist_id, + metadata.year, + )?) } else { None }; @@ -921,102 +899,43 @@ mod tests { assert_eq!(track.rating, Rating(0)); } - // ── Various Artists upgrade ─────────────────────────────────────────────── + // ── Album identity: (title, artist) ───────────────────────────────────── - /// When two tracks with different artists share the same album title (no ALBUMARTIST), - /// the album should be reassigned to "Various Artists" on the second import. + /// Two tracks with the same album title but different primary artists (no ALBUMARTIST tag) + /// must produce two *separate* albums, not a single "Various Artists" one. + /// This covers the "Greatest Hits" / "Best Of" scenario. #[test] - fn test_various_artists_upgrade_on_second_different_artist() { + fn test_same_title_different_artist_creates_separate_albums() { let env = TestEnv::new(); let conn = env.pool.get().unwrap(); - let source_id = - queries::insert_source(&conn, "Test", crate::models::source::SourceType::Disk, None) - .unwrap(); - let artist1 = queries::insert_artist(&conn, "Akhenaton", None).unwrap(); - let artist2 = queries::insert_artist(&conn, "Kool Shen", None).unwrap(); - - // First track: album created and owned by artist1 - let album_id = queries::insert_album(&conn, "70s Mixtape", artist1, None).unwrap(); - queries::insert_track( - &conn, - "Track 1", - Some(album_id), - artist1, - source_id, - &PathBuf::from("/music/t1.flac"), - 200_000, - None, - None, - None, - AudioFormat::Flac, - 1_000_000, - ) - .unwrap(); - - // Simulate second track import (different artist, same album title, no ALBUMARTIST) - let (existing_id, existing_artist_id) = queries::find_album_by_title(&conn, "70s Mixtape") - .unwrap() - .unwrap(); - assert_eq!(existing_id, album_id); - assert_eq!(existing_artist_id, artist1); - - // Upgrade to Various Artists - if existing_artist_id != artist2 { - let va_id = queries::insert_artist(&conn, "Various Artists", None).unwrap(); - if existing_artist_id != va_id { - queries::update_album_artist(&conn, existing_id, va_id).unwrap(); - } - } + let artist1 = queries::insert_artist(&conn, "The Beatles", None).unwrap(); + let artist2 = queries::insert_artist(&conn, "Eminem", None).unwrap(); - // Album should now belong to "Various Artists" - let album = queries::get_album(&conn, album_id).unwrap().unwrap(); - let va = queries::get_artist(&conn, album.artist_id) - .unwrap() - .unwrap(); - assert_eq!(va.name, "Various Artists"); + let id1 = queries::insert_album(&conn, "Greatest Hits", artist1, None).unwrap(); + let id2 = queries::insert_album(&conn, "Greatest Hits", artist2, None).unwrap(); + + assert_ne!(id1, id2, "distinct artists must produce distinct album rows"); + + let album1 = queries::get_album(&conn, id1).unwrap().unwrap(); + let album2 = queries::get_album(&conn, id2).unwrap().unwrap(); + assert_eq!(album1.artist_id, artist1); + assert_eq!(album2.artist_id, artist2); } - /// When two tracks share the same artist and same album, no upgrade should happen. + /// Two tracks with the same album title *and* the same primary artist (no ALBUMARTIST tag) + /// must be grouped into the same album row. #[test] - fn test_no_various_artists_upgrade_when_same_artist() { + fn test_same_title_same_artist_reuses_album() { let env = TestEnv::new(); let conn = env.pool.get().unwrap(); - let source_id = - queries::insert_source(&conn, "Test", crate::models::source::SourceType::Disk, None) - .unwrap(); let artist_id = queries::insert_artist(&conn, "Akhenaton", None).unwrap(); - let album_id = queries::insert_album(&conn, "Sol Invictus", artist_id, None).unwrap(); - queries::insert_track( - &conn, - "Track 1", - Some(album_id), - artist_id, - source_id, - &PathBuf::from("/music/t1.flac"), - 200_000, - None, - None, - None, - AudioFormat::Flac, - 1_000_000, - ) - .unwrap(); - - // Same artist on second track — no upgrade - let (existing_id, existing_artist_id) = queries::find_album_by_title(&conn, "Sol Invictus") - .unwrap() - .unwrap(); - // artist IDs match → no upgrade - assert_eq!(existing_artist_id, artist_id); + let id1 = queries::insert_album(&conn, "Sol Invictus", artist_id, None).unwrap(); + let id2 = queries::insert_album(&conn, "Sol Invictus", artist_id, None).unwrap(); - let album = queries::get_album(&conn, existing_id).unwrap().unwrap(); - assert_eq!( - album.artist_id, artist_id, - "album should still belong to Akhenaton" - ); + assert_eq!(id1, id2, "same title + same artist must reuse the existing album row"); } #[test] From bf97bc4cd811c94b04c13a42cb0a7c4974647774 Mon Sep 17 00:00:00 2001 From: Tornade CI Date: Sun, 1 Mar 2026 17:16:13 +0100 Subject: [PATCH 4/5] feat(library): add clean_library FFI to remove orphaned albums/artists Adds clean_orphans() in queries.rs that: - Deletes albums with no tracks - Deletes artists with no tracks and no albums Exposed via clean_library() FFI returning { albums_deleted, artists_deleted }. Intended to be run after a rescan when the album-grouping fix may have left behind stale "Various Artists" or other orphaned rows. Co-Authored-By: Claude Sonnet 4.6 --- src/db/queries.rs | 34 ++++++++++++++++++++++++++++++++++ src/ffi.rs | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/src/db/queries.rs b/src/db/queries.rs index bafa4fe..59e5caa 100644 --- a/src/db/queries.rs +++ b/src/db/queries.rs @@ -180,6 +180,40 @@ pub fn update_album_rating(conn: &Connection, album_id: i64, rating: u8) -> Resu Ok(()) } +// ============================================================================ +// Library maintenance +// ============================================================================ + +pub struct CleanupStats { + pub albums_deleted: usize, + pub artists_deleted: usize, +} + +/// Delete albums with no tracks, then artists with no tracks and no albums. +/// Returns counts of deleted rows. +pub fn clean_orphans(conn: &Connection) -> Result { + let albums_deleted = conn.execute( + "DELETE FROM albums WHERE id NOT IN ( + SELECT DISTINCT album_id FROM tracks WHERE album_id IS NOT NULL + )", + [], + )?; + + let artists_deleted = conn.execute( + "DELETE FROM artists WHERE id NOT IN ( + SELECT DISTINCT artist_id FROM tracks + ) AND id NOT IN ( + SELECT DISTINCT artist_id FROM albums + )", + [], + )?; + + Ok(CleanupStats { + albums_deleted, + artists_deleted, + }) +} + // ============================================================================ // Genre operations // ============================================================================ diff --git a/src/ffi.rs b/src/ffi.rs index 369b7af..d722b13 100644 --- a/src/ffi.rs +++ b/src/ffi.rs @@ -205,6 +205,9 @@ mod ffi { fn toggle_repeat() -> String; fn set_repeat_mode(mode: &str) -> String; + // Library Maintenance + fn clean_library() -> String; + // Network / NAS helpers fn clear_unavailable_tracks() -> String; @@ -292,6 +295,36 @@ fn get_library_stats() -> String { } } +fn clean_library() -> String { + match get_or_init_pool() { + Ok(pool) => { + let conn = match pool.get() { + Ok(c) => c, + Err(e) => { + return serde_json::json!({ "success": false, "error": format!("{e}") }) + .to_string() + } + }; + match crate::db::queries::clean_orphans(&conn) { + Ok(stats) => serde_json::json!({ + "success": true, + "data": { + "albums_deleted": stats.albums_deleted, + "artists_deleted": stats.artists_deleted + } + }) + .to_string(), + Err(e) => { + serde_json::json!({ "success": false, "error": format!("{e}") }).to_string() + } + } + } + Err(e) => { + serde_json::json!({ "success": false, "error": format!("{e}") }).to_string() + } + } +} + fn scan_library(folder_path: &str) -> String { // T013: Scan a folder for music files and add to library match get_or_init_library() { From c14f72f384507cb86cfe650cf02cd801c4db722a Mon Sep 17 00:00:00 2001 From: Tornade CI Date: Sun, 1 Mar 2026 17:38:24 +0100 Subject: [PATCH 5/5] style: apply cargo fmt to ffi.rs and library.rs Fixes CI formatting check failures on all three platforms. Co-Authored-By: Claude Sonnet 4.6 --- src/ffi.rs | 6 ++---- src/services/library.rs | 10 ++++++++-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/ffi.rs b/src/ffi.rs index d722b13..50266cc 100644 --- a/src/ffi.rs +++ b/src/ffi.rs @@ -302,7 +302,7 @@ fn clean_library() -> String { Ok(c) => c, Err(e) => { return serde_json::json!({ "success": false, "error": format!("{e}") }) - .to_string() + .to_string(); } }; match crate::db::queries::clean_orphans(&conn) { @@ -319,9 +319,7 @@ fn clean_library() -> String { } } } - Err(e) => { - serde_json::json!({ "success": false, "error": format!("{e}") }).to_string() - } + Err(e) => serde_json::json!({ "success": false, "error": format!("{e}") }).to_string(), } } diff --git a/src/services/library.rs b/src/services/library.rs index 6577d65..c3a0afc 100644 --- a/src/services/library.rs +++ b/src/services/library.rs @@ -915,7 +915,10 @@ mod tests { let id1 = queries::insert_album(&conn, "Greatest Hits", artist1, None).unwrap(); let id2 = queries::insert_album(&conn, "Greatest Hits", artist2, None).unwrap(); - assert_ne!(id1, id2, "distinct artists must produce distinct album rows"); + assert_ne!( + id1, id2, + "distinct artists must produce distinct album rows" + ); let album1 = queries::get_album(&conn, id1).unwrap().unwrap(); let album2 = queries::get_album(&conn, id2).unwrap().unwrap(); @@ -935,7 +938,10 @@ mod tests { let id1 = queries::insert_album(&conn, "Sol Invictus", artist_id, None).unwrap(); let id2 = queries::insert_album(&conn, "Sol Invictus", artist_id, None).unwrap(); - assert_eq!(id1, id2, "same title + same artist must reuse the existing album row"); + assert_eq!( + id1, id2, + "same title + same artist must reuse the existing album row" + ); } #[test]