From 9155bb9e00a48d925f2309e3c4fcda9b54857ca8 Mon Sep 17 00:00:00 2001 From: Aleksandr Bogomiakov Date: Fri, 9 Oct 2026 15:34:20 +0100 Subject: [PATCH] Fixed release fed search regression --- Cargo.toml | 2 +- src/app/event.rs | 5 ++ src/app/mod.rs | 165 ++++++++++++++++++++++++++++++++++++++++++++++ src/app/state.rs | 144 ++++++++++++++++++++++++++++++++++++++++ src/app/update.rs | 60 ++++++++++------- src/ui/global.rs | 34 +++++++--- 6 files changed, 377 insertions(+), 33 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index e5e5668..2ccb72a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "furumi_tui" -version = "0.3.1" +version = "0.3.2" edition = "2024" rust-version = "1.97" description = "A federated P2P player for personal music libraries" diff --git a/src/app/event.rs b/src/app/event.rs index feeee90..e7fefcc 100644 --- a/src/app/event.rs +++ b/src/app/event.rs @@ -30,6 +30,11 @@ pub enum AppEvent { id: i64, result: Result, }, + /// Federated tracks that complete an already-open local release view. + ReleaseFederationLoaded { + id: i64, + result: Result, String>, + }, /// Live search results; `seq` drops responses that are already stale. SearchLoaded { seq: u64, diff --git a/src/app/mod.rs b/src/app/mod.rs index 42775e2..dbcf6e5 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -189,6 +189,126 @@ fn spawn_artist_federation_enrichment(runtime: &Runtime, id: i64, name: String) }); } +fn merge_release_federation_results( + title: &str, + releases: impl IntoIterator, +) -> Option { + let key = music_dht::normalize_name(title); + let mut merged = None; + for release in releases { + if music_dht::normalize_name(&release.title) != key { + continue; + } + let target = merged.get_or_insert_with(|| crate::federation::FedRelease { + title: release.title.clone(), + release_type: release.release_type.clone(), + year: release.year, + owners: Vec::new(), + cover_path: release.cover_path.clone(), + tracks: Vec::new(), + }); + if target.release_type.is_empty() { + target.release_type = release.release_type.clone(); + } + if target.year.is_none() { + target.year = release.year; + } + if target.cover_path.is_none() { + target.cover_path = release.cover_path.clone(); + } + for owner in release.owners { + if !target.owners.contains(&owner) { + target.owners.push(owner); + } + } + for track in release.tracks { + let existing = target.tracks.iter_mut().find(|existing| { + music_dht::normalize_name(&existing.title) + == music_dht::normalize_name(&track.title) + && (existing.track_number == track.track_number + || existing.track_number.is_none() + || track.track_number.is_none()) + }); + match existing { + Some(existing) => { + if existing.artists.is_empty() { + existing.artists = track.artists; + } + if existing.featured_artists.is_empty() { + existing.featured_artists = track.featured_artists; + } + if existing.track_number.is_none() { + existing.track_number = track.track_number; + } + if existing.disc_number.is_none() { + existing.disc_number = track.disc_number; + } + if existing.duration_seconds.is_none() { + existing.duration_seconds = track.duration_seconds; + } + if existing.content_id.is_none() { + existing.content_id = track.content_id; + } + for source in track.sources { + if !existing.sources.contains(&source) { + existing.sources.push(source); + } + } + } + None => target.tracks.push(track), + } + } + } + merged +} + +fn spawn_release_federation_enrichment( + runtime: &Runtime, + id: i64, + title: String, + artists: Vec, +) { + let fed = Arc::clone(&runtime.federation); + let tx = runtime.event_tx.clone(); + tokio::spawn(async move { + let requests = artists.into_iter().map(|artist| { + let fed = Arc::clone(&fed); + async move { fed.artist_card(&artist).await } + }); + let mut cards = Vec::new(); + let mut errors = Vec::new(); + for result in futures_util::future::join_all(requests).await { + match result { + Ok(card) => { + let own_owner = card.own_owner.clone(); + for mut release in card.releases { + // The local catalog is included in artist cards. This + // view only needs the missing network tracks; local + // rows are already supplied by the release query. + if let Some(own_owner) = own_owner.as_deref() { + release.owners.retain(|owner| owner != own_owner); + for track in &mut release.tracks { + track.sources.retain(|(owner, _)| owner != own_owner); + } + release.tracks.retain(|track| !track.sources.is_empty()); + } + cards.push(release); + } + } + Err(error) => errors.push(format!("{error:#}")), + } + } + let result = if let Some(release) = merge_release_federation_results(&title, cards) { + Ok(Some(release)) + } else if !errors.is_empty() { + Err(errors.join("; ")) + } else { + Ok(None) + }; + let _ = tx.send(AppEvent::ReleaseFederationLoaded { id, result }); + }); +} + fn maybe_enrich_open_artist(state: &mut AppState, runtime: &Runtime) { if !state.federation.settings.enabled || state.active_tab != state::Tab::Global { return; @@ -207,6 +327,38 @@ fn maybe_enrich_open_artist(state: &mut AppState, runtime: &Runtime) { spawn_artist_federation_enrichment(runtime, id, name); } +fn maybe_enrich_open_release(state: &mut AppState, runtime: &Runtime) { + if !state.federation.settings.enabled + || !state.global.filters.source_mode.includes_network() + || state.active_tab != state::Tab::Global + { + return; + } + let Some(state::GlobalView::Release { id, .. }) = state.global.stack.last().copied() else { + return; + }; + if state.release_fed_views.contains_key(&id) { + return; + } + let Some(state::Loadable::Ready(detail)) = state.release_views.get(&id) else { + return; + }; + let mut artists = Vec::new(); + for artist in &detail.artists { + if !artists.iter().any(|name: &String| { + music_dht::normalize_name(name) == music_dht::normalize_name(&artist.name) + }) { + artists.push(artist.name.clone()); + } + } + if artists.is_empty() { + return; + } + let title = detail.title.clone(); + state.release_fed_views.insert(id, state::Loadable::Loading); + spawn_release_federation_enrichment(runtime, id, title, artists); +} + fn spawn_content_id_backfill(runtime: &Runtime) { let library = Arc::clone(&runtime.library); let federation = Arc::clone(&runtime.federation); @@ -1011,6 +1163,7 @@ fn maintenance(state: &mut AppState, runtime: &mut Runtime) { maybe_refresh_network_library(state, runtime); maybe_fetch_network_artist_images(state, runtime); maybe_enrich_open_artist(state, runtime); + maybe_enrich_open_release(state, runtime); // Liked ids load once per session — markers are shown everywhere. if !state.likes_loaded { @@ -3591,6 +3744,17 @@ fn handle_app_event(state: &mut AppState, runtime: &mut Runtime, event: AppEvent state.artist_fed_views.insert(id, entry); state::restore_artist_cursor_anchor(state, id, cursor_anchor); } + AppEvent::ReleaseFederationLoaded { id, result } => { + let entry = match result { + Ok(Some(release)) => state::Loadable::Ready(release), + Ok(None) => state::Loadable::Ready(crate::federation::FedRelease::default()), + Err(message) => { + tracing::debug!(release = id, %message, "release federation enrichment failed"); + state::Loadable::Failed(message) + } + }; + state.release_fed_views.insert(id, entry); + } AppEvent::NetworkArtistCacheUpdated { source_id, count } => { tracing::debug!(source = %source_id, count, "network artist cache updated"); if state.active_tab == state::Tab::Global && state.global.stack.is_empty() { @@ -3714,6 +3878,7 @@ fn handle_app_event(state: &mut AppState, runtime: &mut Runtime, event: AppEvent } }; state.release_views.insert(id, entry); + maybe_enrich_open_release(state, runtime); // A Shift-J jump was waiting for this release: focus its track. if let Some((release_id, track_id)) = state.pending_release_focus && release_id == id diff --git a/src/app/state.rs b/src/app/state.rs index 82230b3..0b6cbfd 100644 --- a/src/app/state.rs +++ b/src/app/state.rs @@ -303,6 +303,79 @@ mod tests { assert_eq!(fed_release_display_order(&releases), vec![2, 0, 3, 1, 4]); } + #[test] + fn release_tracks_adds_remote_rows_without_duplicating_local_content() { + let local_content = + "b3:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string(); + let detail = ReleaseDetail { + id: 7, + title: "Album".to_string(), + release_type: "album".to_string(), + year: Some(2024), + cover_path: None, + artists: vec![crate::library::models::ArtistRef { + id: 3, + name: "Artist".to_string(), + }], + tracks: vec![TrackItem { + id: 1, + title: "Local".to_string(), + track_number: Some(1), + disc_number: None, + duration_seconds: 120.0, + artists: Vec::new(), + featured_artists: Vec::new(), + release_id: 7, + release_title: "Album".to_string(), + release_year: Some(2024), + file_path: "/music/local.mp3".to_string(), + content_id: Some(local_content.clone()), + cover_path: None, + audio_format: Some("mp3".to_string()), + audio_bitrate: Some(320), + audio_sample_rate: None, + audio_bit_depth: None, + file_size_bytes: None, + play_count: 0, + fed: None, + }], + }; + let mut state = AppState::default(); + state.release_fed_views.insert( + detail.id, + Loadable::Ready(crate::federation::FedRelease { + title: "Album".to_string(), + tracks: vec![ + crate::federation::FedCardTrack { + title: "Local".to_string(), + track_number: Some(1), + content_id: Some(local_content), + sources: vec![("peer".to_string(), "local-item".to_string())], + ..Default::default() + }, + crate::federation::FedCardTrack { + title: "Remote".to_string(), + track_number: Some(3), + sources: vec![("peer".to_string(), "remote-item".to_string())], + ..Default::default() + }, + ], + ..Default::default() + }), + ); + + let tracks = release_tracks(&state, detail.id, &detail); + + assert_eq!( + tracks + .iter() + .map(|track| track.title.as_str()) + .collect::>(), + vec!["Local", "Remote"] + ); + assert!(tracks[1].is_fed_pending()); + } + #[test] fn artist_merged_releases_marks_partially_local_federated_release() { let local_id = "b3:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; @@ -1604,6 +1677,8 @@ pub struct AppState { /// Federated card data that enriches a local artist page in-place. pub artist_fed_views: HashMap>, pub release_views: HashMap>, + /// Federated tracks that enrich an already-open local release view. + pub release_fed_views: HashMap>, pub playlists: PlaylistsTab, pub playlist_views: HashMap>, /// Liked local-library content ids, for the ♥ markers everywhere tracks are shown. @@ -1721,6 +1796,75 @@ pub fn artist_fed_card(state: &AppState, id: i64) -> Option<&crate::federation:: } } +/// The tracklist rendered and played by an opened release. When federation +/// has supplied a matching release, local rows remain authoritative and only +/// tracks that are not already present locally are appended. +pub fn release_tracks(state: &AppState, id: i64, detail: &ReleaseDetail) -> Vec { + let mut tracks = detail.tracks.clone(); + if !state.global.filters.source_mode.includes_network() { + return tracks; + } + let Some(Loadable::Ready(fed_release)) = state.release_fed_views.get(&id) else { + return tracks; + }; + for fed in &fed_release.tracks { + let Some((owner, item_id)) = fed.sources.first().cloned() else { + continue; + }; + let fed_content_id = fed + .content_id + .as_deref() + .and_then(music_dht::normalize_content_id); + let already_local = tracks.iter().any(|local| { + let same_content = fed_content_id.is_some() + && track_content_id(local).as_deref() == fed_content_id.as_deref(); + let same_position = local.track_number == fed.track_number + || local.track_number.is_none() + || fed.track_number.is_none(); + same_content + || (music_dht::normalize_name(&local.title) + == music_dht::normalize_name(&fed.title) + && same_position) + }); + if already_local { + continue; + } + let names = if fed.artists.is_empty() { + detail + .artists + .iter() + .map(|artist| artist.name.clone()) + .collect() + } else { + fed.artists.clone() + }; + let featured_artists = fed.featured_artists.clone(); + tracks.push(crate::federation::pending_track( + &crate::federation::FedTrack { + item_id, + owner, + own: false, + title: fed.title.clone(), + artist_names: names, + featured_artist_names: featured_artists, + year: fed_release.year.or(detail.year), + duration_seconds: fed.duration_seconds.map(|seconds| seconds.round() as i64), + content_id: fed.content_id.clone(), + release_title: Some(detail.title.clone()), + track_number: fed.track_number, + disc_number: fed.disc_number, + }, + )); + } + tracks.sort_by_key(|track| { + ( + track.disc_number.unwrap_or(1), + track.track_number.unwrap_or(i32::MAX), + ) + }); + tracks +} + #[derive(Debug, Clone, PartialEq, Eq)] pub enum ArtistCursorAnchor { TopTrack(i64), diff --git a/src/app/update.rs b/src/app/update.rs index 5c4ad38..2fab271 100644 --- a/src/app/update.rs +++ b/src/app/update.rs @@ -965,11 +965,10 @@ fn current_track_list_context(state: &AppState) -> Option<(TrackSelectionScope, _ => None, }, GlobalView::Release { id, cursor } => match state.release_views.get(id)? { - Loadable::Ready(detail) => Some(( - TrackSelectionScope::Release(*id), - *cursor, - detail.tracks.len(), - )), + Loadable::Ready(detail) => { + let tracks = crate::app::state::release_tracks(state, *id, detail); + Some((TrackSelectionScope::Release(*id), *cursor, tracks.len())) + } _ => None, }, GlobalView::Search { cursor } => { @@ -1021,7 +1020,7 @@ fn current_track_list_context(state: &AppState) -> Option<(TrackSelectionScope, } } -fn current_track_list(state: &AppState) -> Option<(TrackSelectionScope, usize, Vec<&TrackItem>)> { +fn current_track_list(state: &AppState) -> Option<(TrackSelectionScope, usize, Vec)> { match state.active_tab { Tab::Global => match state.global.stack.last()? { GlobalView::Artist { id, cursor } => match state.artist_views.get(id)? { @@ -1032,14 +1031,14 @@ fn current_track_list(state: &AppState) -> Option<(TrackSelectionScope, usize, V Some(( TrackSelectionScope::ArtistTop(*id), *cursor, - detail.top_tracks.iter().collect(), + detail.top_tracks.clone(), )) } else { let featured = cursor.checked_sub(tracks + releases)?; (featured < detail.featured_tracks.len()).then_some(( TrackSelectionScope::ArtistFeatured(*id), featured, - detail.featured_tracks.iter().collect(), + detail.featured_tracks.clone(), )) } } @@ -1049,7 +1048,7 @@ fn current_track_list(state: &AppState) -> Option<(TrackSelectionScope, usize, V Loadable::Ready(detail) => Some(( TrackSelectionScope::Release(*id), *cursor, - detail.tracks.iter().collect(), + crate::app::state::release_tracks(state, *id, detail), )), _ => None, }, @@ -1060,13 +1059,16 @@ fn current_track_list(state: &AppState) -> Option<(TrackSelectionScope, usize, V Some(( TrackSelectionScope::Playlist(opened.id), opened.cursor, - playlist_tracks(state, opened.id)?, + playlist_tracks(state, opened.id)? + .into_iter() + .cloned() + .collect(), )) } Tab::Queue => Some(( TrackSelectionScope::Queue, state.queue_tab.cursor, - state.player.queue.iter().collect(), + state.player.queue.clone(), )), Tab::Federation | Tab::Logs => None, } @@ -1104,7 +1106,7 @@ pub fn selected_tracks(state: &AppState) -> Vec { .unwrap_or_else(|| vec![cursor.min(tracks.len().saturating_sub(1))]); indices .into_iter() - .filter_map(|index| tracks.get(index).map(|track| (*track).clone())) + .filter_map(|index| tracks.get(index).cloned()) .collect() } @@ -1315,7 +1317,9 @@ pub fn selected_track(state: &AppState) -> Option { _ => None, }, GlobalView::Release { id, cursor } => match state.release_views.get(id)? { - Loadable::Ready(detail) => detail.tracks.get(*cursor).cloned(), + Loadable::Ready(detail) => crate::app::state::release_tracks(state, *id, detail) + .get(*cursor) + .cloned(), _ => None, }, GlobalView::Search { cursor } => { @@ -1456,11 +1460,12 @@ fn open_release_for_track(state: &mut AppState, track: &TrackItem) { } let release_id = track.release_id; let cursor = match state.release_views.get(&release_id) { - Some(Loadable::Ready(detail)) => detail - .tracks - .iter() - .position(|t| t.id == track.id) - .unwrap_or(0), + Some(Loadable::Ready(detail)) => { + crate::app::state::release_tracks(state, release_id, detail) + .iter() + .position(|t| t.id == track.id) + .unwrap_or(0) + } _ => { state.pending_release_focus = Some((release_id, track.id)); 0 @@ -1914,7 +1919,7 @@ fn move_selection(state: &mut AppState, dx: isize, dy: isize) { let Some(Loadable::Ready(detail)) = state.release_views.get(&id) else { return; }; - let total = detail.tracks.len() as isize; + let total = crate::app::state::release_tracks(state, id, detail).len() as isize; if total == 0 { return; } @@ -2045,7 +2050,7 @@ fn current_view_len(state: &AppState) -> usize { _ => 0, }, Some(GlobalView::Release { id, .. }) => match state.release_views.get(id) { - Some(Loadable::Ready(d)) => d.tracks.len(), + Some(Loadable::Ready(d)) => crate::app::state::release_tracks(state, *id, d).len(), _ => 0, }, Some(GlobalView::Search { .. }) => { @@ -2339,10 +2344,17 @@ fn select_current(state: &mut AppState) -> Option { _ => Outcome::Nothing, }, Some(GlobalView::Release { id, cursor }) => match state.release_views.get(&id) { - Some(Loadable::Ready(detail)) if !detail.tracks.is_empty() => Outcome::Play { - tracks: detail.tracks.clone(), - start: cursor.min(detail.tracks.len() - 1), - }, + Some(Loadable::Ready(detail)) => { + let tracks = crate::app::state::release_tracks(state, id, detail); + if tracks.is_empty() { + Outcome::Nothing + } else { + Outcome::Play { + start: cursor.min(tracks.len() - 1), + tracks, + } + } + } _ => Outcome::Nothing, }, Some(GlobalView::Search { cursor }) => { diff --git a/src/ui/global.rs b/src/ui/global.rs index 1cce883..8c3cd64 100644 --- a/src/ui/global.rs +++ b/src/ui/global.rs @@ -837,29 +837,47 @@ fn draw_release(frame: &mut Frame, area: Rect, state: &AppState, id: i64, cursor let artists: Vec<&str> = detail.artists.iter().map(|a| a.name.as_str()).collect(); let year = detail.year.map(|y| format!(" · {y}")).unwrap_or_default(); - let info = vec![ + let tracks = crate::app::state::release_tracks(state, id, detail); + let federation_line = match state.release_fed_views.get(&id) { + Some(Loadable::Loading) => Some(super::loading_line(state, "searching peers…")), + Some(Loadable::Ready(release)) if !release.tracks.is_empty() => Some(Line::styled( + format!("federation: +{} tracks", release.tracks.len()), + theme::dim(), + )), + Some(Loadable::Ready(_)) => Some(Line::styled( + "federation: no additional tracks", + theme::dim(), + )), + Some(Loadable::Failed(_)) => Some(Line::styled("federation: unavailable", theme::dim())), + _ if state.federation.settings.enabled + && state.global.filters.source_mode.includes_network() => + { + Some(Line::styled("federation: pending", theme::dim())) + } + _ => None, + }; + let mut info = vec![ Line::default(), Line::styled(detail.title.clone(), theme::header_for(state)), Line::raw(artists.join(", ")), Line::default(), Line::styled( - format!( - "{}{year} · {} tracks", - detail.release_type, - detail.tracks.len() - ), + format!("{}{year} · {} tracks", detail.release_type, tracks.len()), theme::dim(), ), ]; + if let Some(line) = federation_line { + info.push(line); + } frame.render_widget(Paragraph::new(info), info_area); // Track list with centered scrolling. let visible = usize::from(tracks_area.height.max(1)); - let total = detail.tracks.len(); + let total = tracks.len(); let first = cursor .saturating_sub(visible / 2) .min(total.saturating_sub(visible)); - for (offset, track) in detail.tracks.iter().enumerate().skip(first).take(visible) { + for (offset, track) in tracks.iter().enumerate().skip(first).take(visible) { let rect = Rect { x: tracks_area.x, y: tracks_area.y + (offset - first) as u16,