feat: cancel a download from the TUI, the CLI, and the daemon
Closes #54. A download can now be cancelled whether it is queued, in progress, or paused. Cancelled is a terminal status: the store refuses to overwrite it, the status channel relays nothing after it, the partial file is deleted, and the transfer thread releases its socket at its next one-second poll instead of the 30s read deadline. A queued download that is cancelled declines the peer's later offer with TransferResponse "Cancelled", and an inbound file connection for it is refused. A fresh download of the same file evicts the dead row so it is not shadowed, and a cancelled transfer is not re-queued after a restart. Surfaces: Client::cancel_download, download.cancel over RPC, `transfer cancel <user> <path>` on the CLI (which also stops an upload by that name), and x in the transfers pane, where d dismisses the row.
This commit is contained in:
@@ -1197,6 +1197,7 @@ fn main() {
|
||||
soulseek_rs::DownloadStatus::Queued => "Queued",
|
||||
soulseek_rs::DownloadStatus::InProgress { .. } => "InProgress",
|
||||
soulseek_rs::DownloadStatus::Completed => "Completed",
|
||||
soulseek_rs::DownloadStatus::Cancelled => "Cancelled",
|
||||
soulseek_rs::DownloadStatus::Failed(_) => "Failed",
|
||||
soulseek_rs::DownloadStatus::TimedOut => "TimedOut",
|
||||
soulseek_rs::DownloadStatus::Paused { .. } => "Paused",
|
||||
|
||||
@@ -3,6 +3,7 @@ use super::{
|
||||
Receiver, Result, RwLock, RwLockExt, Sender, error, info, mpsc,
|
||||
next_download_token,
|
||||
};
|
||||
use crate::peer::part_path_of;
|
||||
|
||||
impl Client {
|
||||
#[must_use]
|
||||
@@ -35,6 +36,24 @@ impl Client {
|
||||
}
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn cancel_download(&self, username: &str, filename: &str) -> bool {
|
||||
let cancelled = match self.context.write_safe() {
|
||||
Ok(mut ctx) => ctx.downloads.cancel_by_file(username, filename),
|
||||
Err(e) => {
|
||||
error!("[client] cancel_download: {}", e);
|
||||
None
|
||||
}
|
||||
};
|
||||
let Some(download) = cancelled else {
|
||||
return false;
|
||||
};
|
||||
if let Some(part) = part_path_of(&download) {
|
||||
let _ = std::fs::remove_file(part);
|
||||
}
|
||||
true
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn remove_queued_download(
|
||||
&self,
|
||||
|
||||
@@ -6,6 +6,7 @@ use super::{
|
||||
thread, trace, warn,
|
||||
};
|
||||
use crate::message::server::MessageFactory;
|
||||
use crate::peer::DownloadError;
|
||||
|
||||
const CONNECT_SWEEP_INTERVAL: Duration = Duration::from_secs(1);
|
||||
|
||||
@@ -148,6 +149,7 @@ impl Client {
|
||||
download.status,
|
||||
DownloadStatus::InProgress { .. }
|
||||
| DownloadStatus::Completed
|
||||
| DownloadStatus::Cancelled
|
||||
) {
|
||||
debug!(
|
||||
"[client] transfer token {} already claimed; \
|
||||
@@ -210,6 +212,7 @@ impl Client {
|
||||
download.size, filename
|
||||
);
|
||||
}
|
||||
Err(DownloadError::Cancelled) => {}
|
||||
Err(e) => {
|
||||
let reason = Some(e.to_string());
|
||||
let _ = download.sender.send(
|
||||
@@ -363,7 +366,14 @@ impl Client {
|
||||
}
|
||||
});
|
||||
|
||||
if let Some((old_token, download)) = download_to_update
|
||||
let cancelled = download_to_update
|
||||
.as_ref()
|
||||
.is_some_and(|(_, d)| {
|
||||
matches!(d.status, DownloadStatus::Cancelled)
|
||||
});
|
||||
if !cancelled
|
||||
&& let Some((old_token, download)) =
|
||||
download_to_update
|
||||
{
|
||||
trace!(
|
||||
"[client] UpdateDownloadTokens found {old_token}, transfer: {:?}",
|
||||
@@ -386,10 +396,16 @@ impl Client {
|
||||
let registry = context.peer_registry.clone();
|
||||
drop(context);
|
||||
if let Some(registry) = registry {
|
||||
let response =
|
||||
let response = if cancelled {
|
||||
MessageFactory::build_transfer_denial_message(
|
||||
transfer.token,
|
||||
"Cancelled",
|
||||
)
|
||||
} else {
|
||||
MessageFactory::build_transfer_response_message(
|
||||
transfer,
|
||||
);
|
||||
)
|
||||
};
|
||||
let _ = registry.send_to_peer(
|
||||
&username,
|
||||
PeerMessage::SendMessage(response),
|
||||
|
||||
@@ -623,6 +623,71 @@ fn a_replayed_transfer_response_does_not_start_a_second_transfer() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_cancelled_download_is_not_started_when_the_peer_allows_it() {
|
||||
let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap();
|
||||
listener.set_nonblocking(true).unwrap();
|
||||
let port = u32::from(listener.local_addr().unwrap().port());
|
||||
|
||||
let client = Client::new("u", "p");
|
||||
let (sender, _receiver) = mpsc::channel();
|
||||
client.context.write().unwrap().add_download(download(
|
||||
"peer",
|
||||
"f.mp3",
|
||||
9,
|
||||
DownloadStatus::Cancelled,
|
||||
sender,
|
||||
));
|
||||
|
||||
let (ops_tx, ops_rx) = mpsc::channel();
|
||||
Client::listen_to_client_operations(
|
||||
ops_rx,
|
||||
client.context,
|
||||
"u".to_string(),
|
||||
);
|
||||
let peer = Peer::new(
|
||||
"peer".to_string(),
|
||||
ConnectionType::F,
|
||||
"127.0.0.1".to_string(),
|
||||
port,
|
||||
None,
|
||||
0,
|
||||
0,
|
||||
0,
|
||||
);
|
||||
ops_tx
|
||||
.send(ClientOperation::DownloadFromPeer(9, peer, true))
|
||||
.unwrap();
|
||||
|
||||
thread::sleep(Duration::from_millis(500));
|
||||
assert!(
|
||||
matches!(listener.accept(), Err(ref e) if e.kind() == std::io::ErrorKind::WouldBlock),
|
||||
"a cancelled download must not dial the peer"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cancel_download_marks_the_store_and_answers_whether_it_matched() {
|
||||
let client = Client::new("u", "p");
|
||||
let (sender, receiver) = mpsc::channel();
|
||||
client.context.write().unwrap().add_download(download(
|
||||
"peer",
|
||||
"f.mp3",
|
||||
9,
|
||||
DownloadStatus::Queued,
|
||||
sender,
|
||||
));
|
||||
|
||||
assert!(client.cancel_download("peer", "f.mp3"));
|
||||
assert!(!client.cancel_download("peer", "f.mp3"));
|
||||
assert!(!client.cancel_download("peer", "other.mp3"));
|
||||
assert!(matches!(receiver.try_recv(), Ok(DownloadStatus::Cancelled)));
|
||||
assert!(matches!(
|
||||
client.get_all_downloads()[0].status,
|
||||
DownloadStatus::Cancelled
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn room_member_stats_are_returned_sorted_and_scoped_to_their_room() {
|
||||
use crate::types::{RoomUserStats, UserStatus};
|
||||
|
||||
@@ -12,6 +12,11 @@ impl DownloadStore {
|
||||
}
|
||||
|
||||
pub fn add(&mut self, download: Download) {
|
||||
self.downloads.retain(|d| {
|
||||
!(d.is_finished()
|
||||
&& d.username == download.username
|
||||
&& d.filename == download.filename)
|
||||
});
|
||||
self.downloads.push(download);
|
||||
}
|
||||
|
||||
@@ -48,10 +53,33 @@ impl DownloadStore {
|
||||
&self.downloads
|
||||
}
|
||||
|
||||
pub fn update_status(&mut self, token: u32, status: DownloadStatus) {
|
||||
if let Some(download) = self.get_by_token_mut(token) {
|
||||
download.status = status;
|
||||
pub fn update_status(
|
||||
&mut self,
|
||||
token: u32,
|
||||
status: DownloadStatus,
|
||||
) -> bool {
|
||||
let Some(download) = self.get_by_token_mut(token) else {
|
||||
return false;
|
||||
};
|
||||
if matches!(download.status, DownloadStatus::Cancelled) {
|
||||
return false;
|
||||
}
|
||||
download.status = status;
|
||||
true
|
||||
}
|
||||
|
||||
pub fn cancel_by_file(
|
||||
&mut self,
|
||||
username: &str,
|
||||
filename: &str,
|
||||
) -> Option<Download> {
|
||||
let download = self.get_by_file_mut(username, filename)?;
|
||||
if download.is_finished() {
|
||||
return None;
|
||||
}
|
||||
download.status = DownloadStatus::Cancelled;
|
||||
let _ = download.sender.send(DownloadStatus::Cancelled);
|
||||
Some(download.clone())
|
||||
}
|
||||
|
||||
pub fn update_queue_position(
|
||||
@@ -218,6 +246,91 @@ mod tests {
|
||||
assert!(!store.update_queue_position("other", "song.mp3", 1));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cancel_by_file_flips_an_unfinished_download_and_tells_its_channel() {
|
||||
let mut store = DownloadStore::new();
|
||||
let (tx, rx) = mpsc::channel();
|
||||
let mut queued = make_download(1, DownloadStatus::Queued);
|
||||
queued.sender = tx;
|
||||
store.add(queued);
|
||||
store.add(make_download(
|
||||
2,
|
||||
DownloadStatus::InProgress {
|
||||
bytes_downloaded: 5,
|
||||
total_bytes: 100,
|
||||
speed_bytes_per_sec: 1.0,
|
||||
},
|
||||
));
|
||||
store.add(make_download(
|
||||
3,
|
||||
DownloadStatus::Paused {
|
||||
bytes_downloaded: 5,
|
||||
total_bytes: 100,
|
||||
},
|
||||
));
|
||||
store.add(make_download(4, DownloadStatus::Completed));
|
||||
store.add(make_download(5, DownloadStatus::Failed(None)));
|
||||
|
||||
assert!(store.cancel_by_file("peer", "file-1.mp3").is_some());
|
||||
assert!(matches!(rx.try_recv(), Ok(DownloadStatus::Cancelled)));
|
||||
assert!(store.cancel_by_file("peer", "file-2.mp3").is_some());
|
||||
assert!(store.cancel_by_file("peer", "file-3.mp3").is_some());
|
||||
assert!(store.cancel_by_file("peer", "file-4.mp3").is_none());
|
||||
assert!(store.cancel_by_file("peer", "file-5.mp3").is_none());
|
||||
assert!(store.cancel_by_file("peer", "file-9.mp3").is_none());
|
||||
for token in 1..=3 {
|
||||
let download = store.get_by_token(token).unwrap();
|
||||
assert!(matches!(download.status, DownloadStatus::Cancelled));
|
||||
assert!(download.is_finished());
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_cancelled_download_stays_cancelled() {
|
||||
let mut store = DownloadStore::new();
|
||||
store.add(make_download(1, DownloadStatus::Queued));
|
||||
assert!(store.cancel_by_file("peer", "file-1.mp3").is_some());
|
||||
assert!(!store.update_status(
|
||||
1,
|
||||
DownloadStatus::InProgress {
|
||||
bytes_downloaded: 0,
|
||||
total_bytes: 100,
|
||||
speed_bytes_per_sec: 0.0,
|
||||
},
|
||||
));
|
||||
assert!(
|
||||
!store
|
||||
.update_status(1, DownloadStatus::Failed(Some("late".into())))
|
||||
);
|
||||
assert!(matches!(
|
||||
store.get_by_token(1).unwrap().status,
|
||||
DownloadStatus::Cancelled
|
||||
));
|
||||
assert!(store.cancel_by_file("peer", "file-1.mp3").is_none());
|
||||
assert!(!store.resume_by_file("peer", "file-1.mp3"));
|
||||
assert!(!store.pause_by_file("peer", "file-1.mp3"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn add_evicts_a_finished_entry_for_the_same_file() {
|
||||
let mut store = DownloadStore::new();
|
||||
store.add(make_download(1, DownloadStatus::Cancelled));
|
||||
store.add(make_download(2, DownloadStatus::Queued));
|
||||
let mut fresh = make_download(3, DownloadStatus::Queued);
|
||||
fresh.filename = "file-1.mp3".to_string();
|
||||
store.add(fresh);
|
||||
let mut again = make_download(4, DownloadStatus::Queued);
|
||||
again.filename = "file-2.mp3".to_string();
|
||||
store.add(again);
|
||||
|
||||
assert_eq!(store.tokens(), vec![2, 3, 4], "only the dead row goes");
|
||||
assert!(store.cancel_by_file("peer", "file-1.mp3").is_some());
|
||||
assert!(matches!(
|
||||
store.get_by_token(3).unwrap().status,
|
||||
DownloadStatus::Cancelled
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn pause_then_resume_in_progress_download() {
|
||||
let mut store = DownloadStore::new();
|
||||
|
||||
@@ -249,6 +249,16 @@ impl MessageFactory {
|
||||
.write_bool(true)
|
||||
.clone()
|
||||
}
|
||||
#[must_use]
|
||||
pub fn build_transfer_denial_message(token: u32, reason: &str) -> Message {
|
||||
Message::new()
|
||||
.write_int32(41)
|
||||
.write_int32(token)
|
||||
.write_bool(false)
|
||||
.write_string(reason)
|
||||
.clone()
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn build_pierce_firewall_message(token: u32) -> Message {
|
||||
Message::new()
|
||||
@@ -321,6 +331,18 @@ fn test_build_upload_transfer_request() {
|
||||
assert_eq!(transfer.size, 4096);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_transfer_denial_carries_the_token_and_the_reason() {
|
||||
let message =
|
||||
MessageFactory::build_transfer_denial_message(555, "Cancelled");
|
||||
let mut decoded = Message::new_with_data(message.get_buffer());
|
||||
assert_eq!(decoded.get_message_code_u32(), 41);
|
||||
decoded.set_pointer(8);
|
||||
assert_eq!(decoded.read_int32(), 555);
|
||||
assert!(!decoded.read_bool());
|
||||
assert_eq!(decoded.read_string(), "Cancelled");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_build_peer_init_message() {
|
||||
use crate::peer::ConnectionType;
|
||||
|
||||
@@ -14,6 +14,8 @@ use crate::types::{Download, DownloadStatus};
|
||||
use crate::utils::path::{PART_SUFFIX, expand_tilde};
|
||||
|
||||
const READ_BUFFER_SIZE: usize = 8192;
|
||||
const CANCEL_POLL: Duration = Duration::from_secs(1);
|
||||
const STALL_DEADLINE: Duration = Duration::from_secs(30);
|
||||
const PROGRESS_UPDATE_CHUNKS: usize = 15; // ~120KB (15 * 8192 bytes)
|
||||
|
||||
#[derive(Debug)]
|
||||
@@ -29,6 +31,7 @@ pub enum DownloadError {
|
||||
PathResolutionError(String),
|
||||
InvalidTokenBytes,
|
||||
LockPoisoned,
|
||||
Cancelled,
|
||||
IncompleteDownload { received: usize, expected: usize },
|
||||
}
|
||||
|
||||
@@ -56,6 +59,7 @@ impl std::fmt::Display for DownloadError {
|
||||
write!(f, "Invalid token bytes received")
|
||||
}
|
||||
Self::LockPoisoned => write!(f, "Lock poisoned"),
|
||||
Self::Cancelled => write!(f, "Download cancelled"),
|
||||
Self::IncompleteDownload { received, expected } => write!(
|
||||
f,
|
||||
"Incomplete download: received {received} of {expected} bytes"
|
||||
@@ -73,6 +77,16 @@ fn extract_filename_from_path(full_path: &str) -> &str {
|
||||
.unwrap_or(full_path)
|
||||
}
|
||||
|
||||
fn part_path(final_path: &str) -> String {
|
||||
format!("{final_path}{PART_SUFFIX}")
|
||||
}
|
||||
|
||||
pub fn part_path_of(download: &Download) -> Option<String> {
|
||||
resolve_download_path(download)
|
||||
.ok()
|
||||
.map(|path| part_path(&path))
|
||||
}
|
||||
|
||||
/// Where a download's bytes end up: its configured directory (falling back
|
||||
/// to the parent if it names a file) joined with the remote basename.
|
||||
///
|
||||
@@ -121,7 +135,7 @@ struct PartFile {
|
||||
impl PartFile {
|
||||
fn open(download: &Download) -> Result<Self, DownloadError> {
|
||||
let final_path = resolve_download_path(download)?;
|
||||
let path = PathBuf::from(format!("{final_path}{PART_SUFFIX}"));
|
||||
let path = PathBuf::from(part_path(&final_path));
|
||||
|
||||
if let Some(parent) = path.parent() {
|
||||
fs::create_dir_all(parent)
|
||||
@@ -164,6 +178,11 @@ impl PartFile {
|
||||
self.written >= self.expected
|
||||
}
|
||||
|
||||
fn discard(self) {
|
||||
drop(self.file);
|
||||
let _ = fs::remove_file(part_path(&self.final_path));
|
||||
}
|
||||
|
||||
/// A short transfer is a failure, and the `.part` stays put so the next
|
||||
/// attempt can resume it.
|
||||
fn finish(self) -> Result<String, DownloadError> {
|
||||
@@ -174,11 +193,8 @@ impl PartFile {
|
||||
});
|
||||
}
|
||||
drop(self.file);
|
||||
fs::rename(
|
||||
format!("{}{PART_SUFFIX}", self.final_path),
|
||||
&self.final_path,
|
||||
)
|
||||
.map_err(DownloadError::FileWriteError)?;
|
||||
fs::rename(part_path(&self.final_path), &self.final_path)
|
||||
.map_err(DownloadError::FileWriteError)?;
|
||||
Ok(self.final_path)
|
||||
}
|
||||
}
|
||||
@@ -245,7 +261,7 @@ impl DownloadPeer {
|
||||
stream: &TcpStream,
|
||||
) -> Result<(), DownloadError> {
|
||||
stream
|
||||
.set_read_timeout(Some(Duration::from_secs(30)))
|
||||
.set_read_timeout(Some(STALL_DEADLINE))
|
||||
.map_err(DownloadError::ConnectionFailed)?;
|
||||
stream
|
||||
.set_write_timeout(Some(Duration::from_secs(5)))
|
||||
@@ -289,6 +305,7 @@ impl DownloadPeer {
|
||||
stream: &mut TcpStream,
|
||||
client_context: &Arc<RwLock<ClientContext>>,
|
||||
) -> Result<(Download, PartFile), DownloadError> {
|
||||
Self::wait_while_paused(client_context, &download)?;
|
||||
let part = PartFile::open(&download)?;
|
||||
stream
|
||||
.write_all(&part.written.to_le_bytes())
|
||||
@@ -337,6 +354,10 @@ impl DownloadPeer {
|
||||
let mut read_buffer = [0u8; READ_BUFFER_SIZE];
|
||||
let mut chunk_counter = 0usize;
|
||||
let mut last_update_time = Instant::now();
|
||||
let mut last_data = Instant::now();
|
||||
stream
|
||||
.set_read_timeout(Some(CANCEL_POLL))
|
||||
.map_err(DownloadError::ConnectionFailed)?;
|
||||
|
||||
trace!(
|
||||
"[download_peer:{}] Starting to read data from peer",
|
||||
@@ -349,8 +370,19 @@ impl DownloadPeer {
|
||||
};
|
||||
|
||||
loop {
|
||||
if let Some((ref dl, _)) = transfer {
|
||||
Self::wait_while_paused(client_context, dl)?;
|
||||
if let Some((dl, _)) = &transfer {
|
||||
match Self::wait_while_paused(client_context, dl) {
|
||||
Ok(true) => last_data = Instant::now(),
|
||||
Ok(false) => {}
|
||||
Err(e) => {
|
||||
if matches!(e, DownloadError::Cancelled)
|
||||
&& let Some((_, part)) = transfer.take()
|
||||
{
|
||||
part.discard();
|
||||
}
|
||||
return Err(e);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
match stream.read(&mut read_buffer) {
|
||||
@@ -362,6 +394,7 @@ impl DownloadPeer {
|
||||
break;
|
||||
}
|
||||
Ok(bytes_read) => {
|
||||
last_data = Instant::now();
|
||||
let data = &read_buffer[..bytes_read];
|
||||
|
||||
if transfer.is_none() && !self.no_pierce {
|
||||
@@ -404,6 +437,11 @@ impl DownloadPeer {
|
||||
break;
|
||||
}
|
||||
}
|
||||
Err(e)
|
||||
if matches!(
|
||||
e.kind(),
|
||||
io::ErrorKind::WouldBlock | io::ErrorKind::TimedOut
|
||||
) && last_data.elapsed() < STALL_DEADLINE => {}
|
||||
Err(e) => {
|
||||
return Err(DownloadError::StreamReadError(e));
|
||||
}
|
||||
@@ -444,16 +482,21 @@ impl DownloadPeer {
|
||||
download: &Download,
|
||||
status: DownloadStatus,
|
||||
) {
|
||||
let _ = download.sender.send(status.clone());
|
||||
if let Ok(mut context) = client_context.write() {
|
||||
context.update_download_with_status(download.token, status);
|
||||
let applied = client_context.write().is_ok_and(|mut context| {
|
||||
context
|
||||
.downloads
|
||||
.update_status(download.token, status.clone())
|
||||
});
|
||||
if applied {
|
||||
let _ = download.sender.send(status);
|
||||
}
|
||||
}
|
||||
|
||||
fn wait_while_paused(
|
||||
client_context: &Arc<RwLock<ClientContext>>,
|
||||
download: &Download,
|
||||
) -> Result<(), DownloadError> {
|
||||
) -> Result<bool, DownloadError> {
|
||||
let mut waited = false;
|
||||
loop {
|
||||
let status = client_context
|
||||
.read()
|
||||
@@ -462,10 +505,14 @@ impl DownloadPeer {
|
||||
.map(|download| download.status.clone())
|
||||
.ok_or(DownloadError::TokenNotFound(download.token))?;
|
||||
|
||||
if matches!(status, DownloadStatus::Cancelled) {
|
||||
return Err(DownloadError::Cancelled);
|
||||
}
|
||||
if !matches!(status, DownloadStatus::Paused { .. }) {
|
||||
return Ok(());
|
||||
return Ok(waited);
|
||||
}
|
||||
|
||||
waited = true;
|
||||
thread::sleep(Duration::from_millis(200));
|
||||
}
|
||||
}
|
||||
@@ -514,8 +561,9 @@ mod tests {
|
||||
use super::{
|
||||
DownloadError, DownloadPeer, PartFile, extract_filename_from_path,
|
||||
};
|
||||
use crate::client::ClientContext;
|
||||
use crate::types::{Download, DownloadMetadata, DownloadStatus};
|
||||
use std::sync::mpsc;
|
||||
use std::sync::{Arc, RwLock, mpsc};
|
||||
|
||||
fn scratch_dir(name: &str) -> std::path::PathBuf {
|
||||
let dir = std::env::temp_dir()
|
||||
@@ -539,6 +587,117 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_discarded_part_file_is_removed_from_disk() {
|
||||
let dir = scratch_dir("discard");
|
||||
let mut part = PartFile::open(&download_into(&dir, 8)).unwrap();
|
||||
part.write(b"data").unwrap();
|
||||
let path = dir.join("song.mp3.part");
|
||||
assert!(path.exists());
|
||||
|
||||
part.discard();
|
||||
|
||||
assert!(!path.exists());
|
||||
assert!(!dir.join("song.mp3").exists());
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_pause_is_reported_so_the_stall_budget_restarts() {
|
||||
let dir = scratch_dir("paused");
|
||||
let mut download = download_into(&dir, 8);
|
||||
download.status = DownloadStatus::Paused {
|
||||
bytes_downloaded: 0,
|
||||
total_bytes: 8,
|
||||
};
|
||||
let context = Arc::new(RwLock::new(ClientContext::new()));
|
||||
context.write().unwrap().add_download(download.clone());
|
||||
let resumer = {
|
||||
let context = context.clone();
|
||||
std::thread::spawn(move || {
|
||||
std::thread::sleep(std::time::Duration::from_millis(300));
|
||||
assert!(
|
||||
context
|
||||
.write()
|
||||
.unwrap()
|
||||
.downloads
|
||||
.resume_by_file("peer", "song.mp3")
|
||||
);
|
||||
})
|
||||
};
|
||||
|
||||
assert!(matches!(
|
||||
DownloadPeer::wait_while_paused(&context, &download),
|
||||
Ok(true)
|
||||
));
|
||||
assert!(matches!(
|
||||
DownloadPeer::wait_while_paused(&context, &download),
|
||||
Ok(false)
|
||||
));
|
||||
let _ = resumer.join();
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_cancelled_download_stops_the_transfer_loop() {
|
||||
let dir = scratch_dir("cancelled");
|
||||
let mut download = download_into(&dir, 8);
|
||||
download.status = DownloadStatus::Cancelled;
|
||||
let context = Arc::new(RwLock::new(ClientContext::new()));
|
||||
context.write().unwrap().add_download(download.clone());
|
||||
|
||||
assert!(matches!(
|
||||
DownloadPeer::wait_while_paused(&context, &download),
|
||||
Err(DownloadError::Cancelled)
|
||||
));
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_status_after_cancel_is_not_relayed_on_the_channel() {
|
||||
let dir = scratch_dir("relay");
|
||||
let (tx, rx) = mpsc::channel();
|
||||
let mut download = download_into(&dir, 8);
|
||||
download.sender = tx;
|
||||
download.status = DownloadStatus::Cancelled;
|
||||
let context = Arc::new(RwLock::new(ClientContext::new()));
|
||||
context.write().unwrap().add_download(download.clone());
|
||||
|
||||
DownloadPeer::send_download_status(
|
||||
&context,
|
||||
&download,
|
||||
DownloadStatus::InProgress {
|
||||
bytes_downloaded: 4,
|
||||
total_bytes: 8,
|
||||
speed_bytes_per_sec: 1.0,
|
||||
},
|
||||
);
|
||||
|
||||
assert!(rx.try_recv().is_err(), "a cancelled row hears nothing more");
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn start_transfer_refuses_a_cancelled_download() {
|
||||
let dir = scratch_dir("refuse");
|
||||
let mut download = download_into(&dir, 8);
|
||||
download.status = DownloadStatus::Cancelled;
|
||||
let context = Arc::new(RwLock::new(ClientContext::new()));
|
||||
context.write().unwrap().add_download(download.clone());
|
||||
let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap();
|
||||
let mut stream =
|
||||
std::net::TcpStream::connect(listener.local_addr().unwrap())
|
||||
.unwrap();
|
||||
let _peer = listener.accept().unwrap();
|
||||
|
||||
let outcome =
|
||||
DownloadPeer::start_transfer(download, &mut stream, &context);
|
||||
|
||||
assert!(matches!(outcome, Err(DownloadError::Cancelled)));
|
||||
assert!(!dir.join("song.mp3.part").exists(), "no .part is opened");
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_directory_that_does_not_exist_yet_is_created_not_dodged() {
|
||||
let dir = scratch_dir("fresh").join("Soulseek");
|
||||
|
||||
@@ -8,7 +8,7 @@ use std::time::{Duration, Instant};
|
||||
use crate::client::{Client, ClientContext, ClientOperation};
|
||||
|
||||
use crate::message::{Message, MessageReader};
|
||||
use crate::peer::{ConnectionType, DownloadPeer, Peer};
|
||||
use crate::peer::{ConnectionType, DownloadError, DownloadPeer, Peer};
|
||||
use crate::types::Download;
|
||||
use crate::utils::lock::RwLockExt;
|
||||
use crate::utils::semaphore::{Permit, Semaphore};
|
||||
@@ -140,6 +140,7 @@ fn wait_for_download_registration(
|
||||
DownloadStatus::InProgress { .. }
|
||||
| DownloadStatus::Paused { .. }
|
||||
| DownloadStatus::Completed
|
||||
| DownloadStatus::Cancelled
|
||||
)
|
||||
})
|
||||
.cloned(),
|
||||
@@ -255,6 +256,7 @@ fn handle_file_connection(
|
||||
download.size, filename
|
||||
);
|
||||
}
|
||||
Err(DownloadError::Cancelled) => {}
|
||||
Err(e) => {
|
||||
error!(
|
||||
"Failed to download file from {}:{} (token: {}) - Error: {}",
|
||||
@@ -508,6 +510,23 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_cancelled_download_is_not_claimable() {
|
||||
let client_context = Arc::new(RwLock::new(ClientContext::new()));
|
||||
let mut download = registered_download(7);
|
||||
download.status = DownloadStatus::Cancelled;
|
||||
client_context.write().unwrap().add_download(download);
|
||||
|
||||
let claimed = wait_for_download_registration(
|
||||
&client_context,
|
||||
7,
|
||||
"peer",
|
||||
Instant::now() + Duration::from_millis(200),
|
||||
);
|
||||
|
||||
assert!(claimed.is_none(), "a cancelled transfer must not restart");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_download_registered_during_the_wait_is_found() {
|
||||
let client_context = Arc::new(RwLock::new(ClientContext::new()));
|
||||
|
||||
@@ -6,7 +6,8 @@ pub mod upload_peer;
|
||||
pub use crate::actor::peer_actor::{PeerActor, PeerMessage};
|
||||
pub use crate::actor::peer_registry::PeerRegistry;
|
||||
|
||||
pub use download_peer::DownloadPeer;
|
||||
pub(crate) use download_peer::part_path_of;
|
||||
pub use download_peer::{DownloadError, DownloadPeer};
|
||||
|
||||
use crate::message::Message;
|
||||
use core::fmt;
|
||||
|
||||
@@ -186,12 +186,7 @@ pub struct Download {
|
||||
impl Download {
|
||||
#[must_use]
|
||||
pub const fn is_finished(&self) -> bool {
|
||||
matches!(
|
||||
self.status,
|
||||
DownloadStatus::Completed
|
||||
| DownloadStatus::Failed(_)
|
||||
| DownloadStatus::TimedOut
|
||||
)
|
||||
self.status.is_terminal()
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
@@ -234,11 +229,25 @@ pub enum DownloadStatus {
|
||||
total_bytes: u64,
|
||||
},
|
||||
Completed,
|
||||
Cancelled,
|
||||
/// Failed, optionally with a human-readable reason.
|
||||
Failed(Option<String>),
|
||||
TimedOut,
|
||||
}
|
||||
|
||||
impl DownloadStatus {
|
||||
#[must_use]
|
||||
pub const fn is_terminal(&self) -> bool {
|
||||
matches!(
|
||||
self,
|
||||
Self::Completed
|
||||
| Self::Cancelled
|
||||
| Self::Failed(_)
|
||||
| Self::TimedOut
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/// Whether a user is reachable, as the server reports it.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]
|
||||
pub enum UserStatus {
|
||||
|
||||
@@ -600,6 +600,76 @@ fn run_mock_uploader_f_first(cfg: &MockUpload) -> std::io::Result<u64> {
|
||||
.map_err(|_| std::io::Error::other("F connection thread panicked"))?
|
||||
}
|
||||
|
||||
fn run_mock_slow_uploader(
|
||||
cfg: &MockUpload,
|
||||
stall_after: Option<usize>,
|
||||
) -> std::io::Result<usize> {
|
||||
let mut p = open_control_and_await_queue(cfg)?;
|
||||
p.write_all(&transfer_request(cfg).get_buffer())?;
|
||||
p.flush()?;
|
||||
expect_code(&mut p, 41, Duration::from_secs(10))?;
|
||||
|
||||
let mut f = connect_retry(&cfg.listen_addr, Duration::from_secs(5))?;
|
||||
f.set_read_timeout(Some(Duration::from_secs(10)))?;
|
||||
f.set_write_timeout(Some(Duration::from_secs(5)))?;
|
||||
let mut init = peer_init_bytes(&cfg.peer_username, "F", cfg.token);
|
||||
init.extend_from_slice(&cfg.token.to_le_bytes());
|
||||
f.write_all(&init)?;
|
||||
let mut start = [0u8; 8];
|
||||
f.read_exact(&mut start)?;
|
||||
|
||||
let mut sent = 0;
|
||||
for (n, chunk) in cfg.content.chunks(4096).enumerate() {
|
||||
if stall_after == Some(n) {
|
||||
f.set_read_timeout(Some(Duration::from_secs(15)))?;
|
||||
match f.read(&mut [0u8; 1]) {
|
||||
Ok(0) => {
|
||||
return Err(std::io::Error::other(
|
||||
"the downloader hung up",
|
||||
));
|
||||
}
|
||||
Ok(_) => {}
|
||||
Err(e)
|
||||
if matches!(
|
||||
e.kind(),
|
||||
std::io::ErrorKind::WouldBlock
|
||||
| std::io::ErrorKind::TimedOut
|
||||
) => {}
|
||||
Err(e) => return Err(e),
|
||||
}
|
||||
}
|
||||
f.write_all(chunk)?;
|
||||
f.flush()?;
|
||||
sent += chunk.len();
|
||||
std::thread::sleep(Duration::from_millis(20));
|
||||
}
|
||||
Ok(sent)
|
||||
}
|
||||
|
||||
fn run_mock_uploader_offering_after(
|
||||
cfg: &MockUpload,
|
||||
queued: &Sender<()>,
|
||||
go: &Receiver<()>,
|
||||
) -> std::io::Result<(bool, String)> {
|
||||
let mut p = open_control_and_await_queue(cfg)?;
|
||||
let _ = queued.send(());
|
||||
go.recv()
|
||||
.map_err(|_| std::io::Error::other("the test never said go"))?;
|
||||
|
||||
p.write_all(&transfer_request(cfg).get_buffer())?;
|
||||
p.flush()?;
|
||||
let mut msg = expect_code(&mut p, 41, Duration::from_secs(10))?;
|
||||
msg.set_pointer(8);
|
||||
let _token = msg.read_int32();
|
||||
let allowed = msg.read_bool();
|
||||
let reason = if allowed {
|
||||
String::new()
|
||||
} else {
|
||||
msg.read_string()
|
||||
};
|
||||
Ok((allowed, reason))
|
||||
}
|
||||
|
||||
fn run_refusing_uploader(
|
||||
cfg: &MockUpload,
|
||||
refusal: &Message,
|
||||
@@ -843,6 +913,174 @@ fn a_download_completes_when_the_f_connection_beats_token_registration() {
|
||||
);
|
||||
}
|
||||
|
||||
fn cancel_mid_transfer(peer_username: &str, stall_after: Option<usize>) {
|
||||
let server = server_or_skip!();
|
||||
|
||||
let listen_port = free_port().expect("free listen port");
|
||||
let mut client = Client::with_settings(server.listening_settings(
|
||||
&format!("{peer_username}_dl"),
|
||||
"pw",
|
||||
listen_port,
|
||||
));
|
||||
client.connect().expect("connect");
|
||||
assert!(client.login().expect("login"));
|
||||
|
||||
let filename = "cancel_me.mp3";
|
||||
let content: Vec<u8> = (0..400_000u32).map(|i| (i % 251) as u8).collect();
|
||||
let download_dir = unique_download_dir();
|
||||
let (ready_tx, ready_rx) = std::sync::mpsc::channel();
|
||||
let cfg = MockUpload {
|
||||
listen_addr: format!("127.0.0.1:{listen_port}"),
|
||||
peer_username: peer_username.to_string(),
|
||||
filename: filename.to_string(),
|
||||
content: content.clone(),
|
||||
token: 424_250_u32,
|
||||
ready: ready_tx,
|
||||
token_delay: Duration::ZERO,
|
||||
};
|
||||
let uploader =
|
||||
std::thread::spawn(move || run_mock_slow_uploader(&cfg, stall_after));
|
||||
ready_rx
|
||||
.recv_timeout(Duration::from_secs(5))
|
||||
.expect("mock uploader P connection");
|
||||
std::thread::sleep(Duration::from_millis(1500));
|
||||
|
||||
let (_download, status_rx) = client
|
||||
.download(
|
||||
filename.to_string(),
|
||||
peer_username.to_string(),
|
||||
content.len() as u64,
|
||||
download_dir.display().to_string(),
|
||||
)
|
||||
.expect("start download");
|
||||
|
||||
let part = download_dir.join(format!("{filename}.part"));
|
||||
assert!(
|
||||
wait_for(|| part.exists()),
|
||||
"the transfer should be streaming"
|
||||
);
|
||||
if stall_after.is_some() {
|
||||
std::thread::sleep(Duration::from_secs(2));
|
||||
}
|
||||
let cancelled_at = Instant::now();
|
||||
assert!(client.cancel_download(peer_username, filename));
|
||||
|
||||
let deadline = Instant::now() + Duration::from_secs(10);
|
||||
let mut cancelled = false;
|
||||
while Instant::now() < deadline {
|
||||
match status_rx.recv_timeout(Duration::from_millis(500)) {
|
||||
Ok(DownloadStatus::Cancelled) => {
|
||||
cancelled = true;
|
||||
break;
|
||||
}
|
||||
Ok(DownloadStatus::Completed | DownloadStatus::Failed(_)) => break,
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
assert!(cancelled, "the status channel should report the cancel");
|
||||
assert!(wait_for(|| !part.exists()), "the partial file should go");
|
||||
assert!(!download_dir.join(filename).exists());
|
||||
assert!(client.get_all_downloads().iter().any(|d| {
|
||||
d.filename == filename && matches!(d.status, DownloadStatus::Cancelled)
|
||||
}));
|
||||
|
||||
let outcome = uploader.join().expect("mock uploader thread");
|
||||
assert!(
|
||||
outcome.is_err(),
|
||||
"the uploader should see its connection drop, got {outcome:?}"
|
||||
);
|
||||
assert!(
|
||||
cancelled_at.elapsed() < Duration::from_secs(8),
|
||||
"the socket should be released within seconds, not at the read \
|
||||
deadline"
|
||||
);
|
||||
assert!(
|
||||
status_rx.try_recv().is_err(),
|
||||
"nothing follows Cancelled on the channel"
|
||||
);
|
||||
let _ = std::fs::remove_dir_all(&download_dir);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_cancelled_download_stops_and_drops_its_partial_file() {
|
||||
cancel_mid_transfer("e2e_cancel_peer", None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_cancelled_stalled_download_is_released_promptly() {
|
||||
cancel_mid_transfer("e2e_stalled_peer", Some(2));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_cancelled_queued_download_declines_the_peers_offer() {
|
||||
let server = server_or_skip!();
|
||||
|
||||
let listen_port = free_port().expect("free listen port");
|
||||
let mut client = Client::with_settings(server.listening_settings(
|
||||
"e2e_cancel_queued_dl",
|
||||
"pw",
|
||||
listen_port,
|
||||
));
|
||||
client.connect().expect("connect");
|
||||
assert!(client.login().expect("login"));
|
||||
|
||||
let filename = "never_starts.mp3";
|
||||
let (ready_tx, ready_rx) = std::sync::mpsc::channel();
|
||||
let (queued_tx, queued_rx) = std::sync::mpsc::channel();
|
||||
let (go_tx, go_rx) = std::sync::mpsc::channel();
|
||||
let cfg = MockUpload {
|
||||
listen_addr: format!("127.0.0.1:{listen_port}"),
|
||||
peer_username: "e2e_cancel_queued_peer".to_string(),
|
||||
filename: filename.to_string(),
|
||||
content: vec![0; 10],
|
||||
token: 424_251_u32,
|
||||
ready: ready_tx,
|
||||
token_delay: Duration::ZERO,
|
||||
};
|
||||
let uploader = std::thread::spawn(move || {
|
||||
run_mock_uploader_offering_after(&cfg, &queued_tx, &go_rx)
|
||||
});
|
||||
ready_rx
|
||||
.recv_timeout(Duration::from_secs(5))
|
||||
.expect("mock uploader P connection");
|
||||
std::thread::sleep(Duration::from_millis(1500));
|
||||
|
||||
let download_dir = unique_download_dir();
|
||||
let (_download, status_rx) = client
|
||||
.download(
|
||||
filename.to_string(),
|
||||
"e2e_cancel_queued_peer".to_string(),
|
||||
10,
|
||||
download_dir.display().to_string(),
|
||||
)
|
||||
.expect("start download");
|
||||
queued_rx
|
||||
.recv_timeout(Duration::from_secs(10))
|
||||
.expect("the peer should receive our queue request");
|
||||
|
||||
assert!(client.cancel_download("e2e_cancel_queued_peer", filename));
|
||||
assert!(matches!(
|
||||
status_rx.recv_timeout(Duration::from_secs(2)),
|
||||
Ok(DownloadStatus::Cancelled)
|
||||
));
|
||||
|
||||
go_tx.send(()).expect("mock uploader waiting");
|
||||
let (allowed, reason) = uploader
|
||||
.join()
|
||||
.expect("mock uploader thread")
|
||||
.expect("the peer should get an answer to its offer");
|
||||
assert!(
|
||||
!allowed,
|
||||
"a cancelled download must not accept the transfer"
|
||||
);
|
||||
assert_eq!(reason, "Cancelled");
|
||||
assert!(client.get_all_downloads().iter().any(|d| {
|
||||
d.filename == filename && matches!(d.status, DownloadStatus::Cancelled)
|
||||
}));
|
||||
assert!(!download_dir.join(format!("{filename}.part")).exists());
|
||||
let _ = std::fs::remove_dir_all(&download_dir);
|
||||
}
|
||||
|
||||
fn refusal_fails_the_download_quickly(
|
||||
peer_username: &str,
|
||||
build_refusal: impl FnOnce(&str) -> Message,
|
||||
|
||||
Reference in New Issue
Block a user