Connected Devices: fixed rplaylist sync
This commit is contained in:
@@ -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<i64> = tracks.iter().map(|t| t.id).filter(|id| *id >= 0).collect();
|
||||
let fed_tracks: Vec<crate::federation::FedTrack> = 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,
|
||||
|
||||
+3
-2
@@ -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<TrackItem>),
|
||||
|
||||
+80
-20
@@ -1447,25 +1447,30 @@ impl DeviceSync {
|
||||
hlc_ms: i64,
|
||||
op_id: &str,
|
||||
) -> Result<bool> {
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
||||
+174
-1
@@ -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<String> = tx
|
||||
.query_row(
|
||||
"SELECT sync_id FROM playlists WHERE id = ?1",
|
||||
[playlist_id],
|
||||
|row| row.get::<_, Option<String>>(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<i64> = 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<String> = conn
|
||||
.query_row(
|
||||
"SELECT sync_id FROM playlists WHERE id = ?1",
|
||||
[playlist_id],
|
||||
|row| row.get::<_, Option<String>>(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<_>>(),
|
||||
vec!["Local Song", "Remote Song"]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn track_edit_relinks_artists() {
|
||||
let lib = test_library();
|
||||
|
||||
Reference in New Issue
Block a user