diff --git a/src/app/popup.rs b/src/app/popup.rs index 10e3e73..16b9d9d 100644 --- a/src/app/popup.rs +++ b/src/app/popup.rs @@ -792,15 +792,27 @@ pub(crate) fn spawn_add_target( let devices = Arc::clone(&runtime.devices); let tx = runtime.event_tx.clone(); let ids: Vec = tracks.iter().map(|t| t.id).filter(|id| *id >= 0).collect(); + let fed_tracks: Vec = tracks + .iter() + .filter(|track| track.is_fed_pending()) + .filter_map(|track| track.fed.clone()) + .collect(); tokio::task::spawn_blocking(move || { let result = library .add_tracks_to_playlist(playlist_id, &ids) + .and_then(|()| library.add_fed_tracks_to_playlist(playlist_id, &fed_tracks)) .map_err(|err| format!("{err:#}")); if result.is_ok() && let Err(err) = devices.record_playlist_tracks_added(playlist_id, &ids) { tracing::warn!(%err, playlist_id, "recording synced playlist add failed"); } + if result.is_ok() + && let Err(err) = + devices.record_playlist_fed_tracks_added(playlist_id, &fed_tracks) + { + tracing::warn!(%err, playlist_id, "recording synced federated playlist add failed"); + } let _ = tx.send(AppEvent::PlaylistTracksAdded { playlist_id, playlist_title, diff --git a/src/app/state.rs b/src/app/state.rs index f93e647..e157002 100644 --- a/src/app/state.rs +++ b/src/app/state.rs @@ -457,8 +457,9 @@ impl EditField { } } -/// What an add-to-playlist flow adds: local library tracks directly, or -/// federated tracks that are downloaded into the library first. +/// What an add-to-playlist flow adds: local library tracks directly (including +/// already-materialized federation placeholders), or federated search/card +/// tracks that are downloaded into the library first. #[derive(Debug, Clone)] pub enum PlaylistAddTarget { Local(Vec), diff --git a/src/devices.rs b/src/devices.rs index 60c9877..f473f49 100644 --- a/src/devices.rs +++ b/src/devices.rs @@ -1447,25 +1447,30 @@ impl DeviceSync { hlc_ms: i64, op_id: &str, ) -> Result { - let apply = { + let current = { let conn = lock(&self.conn); - let current: Option<(i64, String)> = conn - .query_row( - "SELECT hlc_ms, op_id - FROM sync_state_playlist_items - WHERE playlist_id = ?1 AND content_id = ?2", - params![playlist_id, content_id], - |row| Ok((row.get(0)?, row.get(1)?)), - ) - .optional()?; - current.as_ref().is_none_or(|(current_hlc, current_op)| { - (hlc_ms, op_id) > (*current_hlc, current_op.as_str()) - }) + conn.query_row( + "SELECT present, position, hlc_ms, op_id + FROM sync_state_playlist_items + WHERE playlist_id = ?1 AND content_id = ?2", + params![playlist_id, content_id], + |row| { + Ok(( + row.get::<_, i64>(0)? != 0, + row.get::<_, i64>(1)?, + row.get::<_, i64>(2)?, + row.get::<_, String>(3)?, + )) + }, + ) + .optional()? }; - if !apply { - return Ok(false); - } - { + let apply = current + .as_ref() + .is_none_or(|(_, _, current_hlc, current_op)| { + (hlc_ms, op_id) > (*current_hlc, current_op.as_str()) + }); + if apply { let conn = lock(&self.conn); conn.execute( "INSERT INTO sync_state_playlist_items @@ -1486,8 +1491,16 @@ impl DeviceSync { ], )?; } + let (effective_present, effective_position) = if apply { + (present, position) + } else { + current + .as_ref() + .map(|(present, position, _, _)| (*present, *position)) + .unwrap_or((present, position)) + }; let mut visible_changed = apply; - if present { + if effective_present { visible_changed |= self .library .add_content_id_to_synced_playlist(playlist_id, content_id)?; @@ -1495,10 +1508,16 @@ impl DeviceSync { visible_changed |= self.library.upsert_fed_playlist_track( playlist_id, &fed.to_fed_track(), - position, + effective_position, + )?; + } else if let Some(fed) = self.library.fed_like_by_content_id(content_id)? { + visible_changed |= self.library.upsert_fed_playlist_track( + playlist_id, + &fed, + effective_position, )?; } - } else { + } else if apply { self.library .remove_content_id_from_synced_playlist(playlist_id, content_id)?; } @@ -3258,4 +3277,45 @@ mod tests { ); assert_eq!(sync.library.playlist(playlist.id).unwrap().tracks.len(), 0); } + + #[test] + fn stale_synced_playlist_item_metadata_repairs_pending_fed_track() { + let sync = test_sync(); + let playlist = sync.library.create_playlist("Remote Mix").unwrap(); + let playlist_sync_id = sync.library.ensure_playlist_sync_id(playlist.id).unwrap(); + let content_id = format!("b3:{}", "e".repeat(64)); + let fed = test_fed_track(&content_id); + let synced = SyncedFedTrack::from_fed(&fed).unwrap(); + + assert!( + sync.apply_playlist_item_state( + &playlist_sync_id, + &content_id, + true, + 7, + None, + 10, + "dev_remote:2", + ) + .unwrap() + ); + assert_eq!(sync.library.playlist(playlist.id).unwrap().tracks.len(), 0); + + assert!( + sync.apply_playlist_item_state( + &playlist_sync_id, + &content_id, + true, + 7, + Some(&synced), + 10, + "dev_remote:2", + ) + .unwrap() + ); + let detail = sync.library.playlist(playlist.id).unwrap(); + assert_eq!(detail.tracks.len(), 1); + assert!(detail.tracks[0].is_fed_pending()); + assert_eq!(detail.tracks[0].title, fed.title); + } } diff --git a/src/library/mod.rs b/src/library/mod.rs index 81f1f5f..ae3b07b 100644 --- a/src/library/mod.rs +++ b/src/library/mod.rs @@ -898,6 +898,114 @@ impl Library { Ok(()) } + pub fn add_fed_tracks_to_playlist( + &self, + playlist_id: i64, + tracks: &[crate::federation::FedTrack], + ) -> Result<()> { + if tracks.is_empty() { + return Ok(()); + } + let mut conn = self.lock(); + let tx = conn.transaction()?; + let playlist_sync_id: Option = tx + .query_row( + "SELECT sync_id FROM playlists WHERE id = ?1", + [playlist_id], + |row| row.get::<_, Option>(0), + ) + .optional()? + .flatten(); + let playlist_sync_id = match playlist_sync_id { + Some(sync_id) => sync_id, + None => { + let title: String = tx.query_row( + "SELECT title FROM playlists WHERE id = ?1", + [playlist_id], + |row| row.get(0), + )?; + let sync_id = make_playlist_sync_id(playlist_id, &title); + tx.execute( + "UPDATE playlists SET sync_id = ?2 WHERE id = ?1", + params![playlist_id, sync_id], + )?; + sync_id + } + }; + let local_max: i64 = tx.query_row( + "SELECT COALESCE(MAX(position), -1) FROM playlist_tracks WHERE playlist_id = ?1", + [playlist_id], + |row| row.get(0), + )?; + let fed_max: i64 = tx.query_row( + "SELECT COALESCE(MAX(position), -1) FROM fed_playlist_tracks WHERE playlist_sync_id = ?1", + [&playlist_sync_id], + |row| row.get(0), + )?; + let mut next = local_max.max(fed_max) + 1; + for fed in tracks { + let Some(content_id) = fed + .content_id + .as_deref() + .and_then(music_dht::normalize_content_id) + else { + continue; + }; + let existing_position: Option = tx + .query_row( + "SELECT position FROM fed_playlist_tracks + WHERE playlist_sync_id = ?1 AND content_id = ?2", + params![playlist_sync_id, content_id], + |row| row.get(0), + ) + .optional()?; + let position = match existing_position { + Some(position) => position, + None => { + let position = next; + next += 1; + position + } + }; + tx.execute( + "INSERT INTO fed_playlist_tracks + (playlist_sync_id, item_id, owner, title, artist_names, + featured_artist_names, year, duration_seconds, content_id, + release_title, track_number, disc_number, position) + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13) + ON CONFLICT(playlist_sync_id, content_id) DO UPDATE SET + item_id = excluded.item_id, + owner = excluded.owner, + title = excluded.title, + artist_names = excluded.artist_names, + featured_artist_names = excluded.featured_artist_names, + year = excluded.year, + duration_seconds = excluded.duration_seconds, + release_title = excluded.release_title, + track_number = excluded.track_number, + disc_number = excluded.disc_number, + position = excluded.position", + params![ + playlist_sync_id, + fed.item_id, + fed.owner, + fed.title, + fed.artist_names.join("; "), + fed.featured_artist_names.join("; "), + fed.year, + fed.duration_seconds.map(|d| d as f64), + content_id, + fed.release_title, + fed.track_number, + fed.disc_number, + position, + ], + )?; + } + tx.commit()?; + Ok(()) + } + pub fn remove_tracks_from_playlist(&self, playlist_id: i64, track_ids: &[i64]) -> Result<()> { let conn = self.lock(); for &track_id in track_ids { @@ -1000,7 +1108,7 @@ impl Library { return Ok(None); }; let conn = self.lock(); - Ok(conn + let local_position = conn .query_row( "SELECT pt.position FROM playlist_tracks pt @@ -1010,6 +1118,30 @@ impl Library { params![playlist_id, content_id], |row| row.get(0), ) + .optional()?; + if local_position.is_some() { + return Ok(local_position); + } + let sync_id: Option = conn + .query_row( + "SELECT sync_id FROM playlists WHERE id = ?1", + [playlist_id], + |row| row.get::<_, Option>(0), + ) + .optional()? + .flatten(); + let Some(sync_id) = sync_id else { + return Ok(None); + }; + Ok(conn + .query_row( + "SELECT position + FROM fed_playlist_tracks + WHERE playlist_sync_id = ?1 AND content_id = ?2 + LIMIT 1", + params![sync_id, content_id], + |row| row.get(0), + ) .optional()?) } @@ -2263,6 +2395,47 @@ mod tests { ); } + #[test] + fn add_federated_pending_track_to_playlist_records_position() { + let lib = test_library(); + let local_id = add_track(&lib, "Local Song", "Artist", "Album"); + let playlist = lib.create_playlist("Remote Mix").unwrap(); + let content_id = format!("b3:{}", "b".repeat(64)); + let fed = crate::federation::FedTrack { + item_id: "fed_item_2".to_string(), + owner: "fed_owner_2".to_string(), + own: false, + title: "Remote Song".to_string(), + artist_names: vec!["Remote Artist".to_string()], + featured_artist_names: Vec::new(), + year: Some(2026), + duration_seconds: Some(123), + content_id: Some(content_id.clone()), + release_title: Some("Remote Release".to_string()), + track_number: Some(2), + disc_number: Some(1), + }; + + lib.add_tracks_to_playlist(playlist.id, &[local_id]) + .unwrap(); + lib.add_fed_tracks_to_playlist(playlist.id, std::slice::from_ref(&fed)) + .unwrap(); + + let position = lib + .playlist_content_position(playlist.id, &content_id) + .unwrap(); + assert_eq!(position, Some(1)); + let detail = lib.playlist(playlist.id).unwrap(); + assert_eq!( + detail + .tracks + .into_iter() + .map(|track| track.title) + .collect::>(), + vec!["Local Song", "Remote Song"] + ); + } + #[test] fn track_edit_relinks_artists() { let lib = test_library();