mirror of
https://github.com/veeso/termscp.git
synced 2026-04-01 07:42:17 -07:00
fix: replace recursive byte-counting with entry-based transfer progress (#395)
* fix: replace recursive byte-counting with entry-based transfer progress Replace the expensive recursive `get_total_transfer_size` pre-calculation with a lightweight entry-based counter (`TransferProgress`) for the overall progress bar. This avoids deep `list_dir` traversals before transfers begin, which could cause FTP idle-timeout disconnections on large directory trees. The per-file byte-level progress bar (`ProgressStates`) remains unchanged. Bytes are still tracked via `TransferStates::add_bytes` for notification threshold logic. Closes #384
This commit is contained in:
committed by
GitHub
parent
e8eed9f8d1
commit
34f1503391
@@ -9,14 +9,54 @@ use bytesize::ByteSize;
|
||||
|
||||
// -- States and progress
|
||||
|
||||
/// TransferStates contains the states related to the transfer process
|
||||
pub struct TransferStates {
|
||||
aborted: bool, // Describes whether the transfer process has been aborted
|
||||
pub full: ProgressStates, // full transfer states
|
||||
pub partial: ProgressStates, // Partial transfer states
|
||||
/// Tracks overall transfer progress as an entry counter (e.g. "3/12").
|
||||
///
|
||||
/// Unlike the byte-based `ProgressStates`, this counts top-level entries
|
||||
/// rather than bytes, avoiding the expensive recursive size pre-calculation
|
||||
/// that can cause FTP idle-timeout disconnections on large directory trees.
|
||||
#[derive(Default)]
|
||||
pub struct TransferProgress {
|
||||
completed: usize,
|
||||
total: usize,
|
||||
}
|
||||
|
||||
/// Progress states describes the states for the progress of a single transfer part
|
||||
impl fmt::Display for TransferProgress {
|
||||
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||
write!(f, "{}/{}", self.completed, self.total)
|
||||
}
|
||||
}
|
||||
|
||||
impl TransferProgress {
|
||||
/// Initialize progress with the total number of entries to transfer.
|
||||
pub fn init(&mut self, total: usize) {
|
||||
self.completed = 0;
|
||||
self.total = total;
|
||||
}
|
||||
|
||||
/// Mark one entry as completed.
|
||||
pub fn increment(&mut self) {
|
||||
self.completed += 1;
|
||||
}
|
||||
|
||||
/// Calculate progress in a range between 0.0 and 1.0.
|
||||
pub fn calc_progress(&self) -> f64 {
|
||||
if self.total == 0 {
|
||||
return 0.0;
|
||||
}
|
||||
let prog = self.completed as f64 / self.total as f64;
|
||||
prog.min(1.0)
|
||||
}
|
||||
}
|
||||
|
||||
/// Contains the states related to the transfer process.
|
||||
pub struct TransferStates {
|
||||
aborted: bool,
|
||||
pub full: TransferProgress,
|
||||
pub partial: ProgressStates,
|
||||
bytes_transferred: usize,
|
||||
}
|
||||
|
||||
/// Describes the states for the progress of a single file transfer.
|
||||
pub struct ProgressStates {
|
||||
started: Instant,
|
||||
total: usize,
|
||||
@@ -30,33 +70,40 @@ impl Default for TransferStates {
|
||||
}
|
||||
|
||||
impl TransferStates {
|
||||
/// Instantiates a new transfer states
|
||||
/// Instantiates a new transfer states.
|
||||
pub fn new() -> TransferStates {
|
||||
TransferStates {
|
||||
aborted: false,
|
||||
full: ProgressStates::default(),
|
||||
full: TransferProgress::default(),
|
||||
partial: ProgressStates::default(),
|
||||
bytes_transferred: 0,
|
||||
}
|
||||
}
|
||||
|
||||
/// Re-intiialize transfer states
|
||||
/// Re-initialize transfer states.
|
||||
pub fn reset(&mut self) {
|
||||
self.aborted = false;
|
||||
self.bytes_transferred = 0;
|
||||
}
|
||||
|
||||
/// Set aborted to true
|
||||
/// Set aborted to true.
|
||||
pub fn abort(&mut self) {
|
||||
self.aborted = true;
|
||||
}
|
||||
|
||||
/// Returns whether transfer has been aborted
|
||||
/// Returns whether transfer has been aborted.
|
||||
pub fn aborted(&self) -> bool {
|
||||
self.aborted
|
||||
}
|
||||
|
||||
/// Returns the size of the entire transfer
|
||||
/// Returns total bytes transferred (used for notification threshold).
|
||||
pub fn full_size(&self) -> usize {
|
||||
self.full.total
|
||||
self.bytes_transferred
|
||||
}
|
||||
|
||||
/// Accumulate transferred bytes for notification threshold tracking.
|
||||
pub fn add_bytes(&mut self, delta: usize) {
|
||||
self.bytes_transferred += delta;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -226,23 +273,46 @@ mod test {
|
||||
assert_eq!(states.calc_progress(), 0.0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_ui_activities_filetransfer_lib_transfer_progress() {
|
||||
let mut progress = TransferProgress::default();
|
||||
assert_eq!(progress.calc_progress(), 0.0);
|
||||
assert_eq!(progress.to_string(), "0/0");
|
||||
// Init with 4 entries
|
||||
progress.init(4);
|
||||
assert_eq!(progress.calc_progress(), 0.0);
|
||||
assert_eq!(progress.to_string(), "0/4");
|
||||
// Increment
|
||||
progress.increment();
|
||||
assert_eq!(progress.calc_progress(), 0.25);
|
||||
assert_eq!(progress.to_string(), "1/4");
|
||||
// Complete all
|
||||
progress.increment();
|
||||
progress.increment();
|
||||
progress.increment();
|
||||
assert_eq!(progress.calc_progress(), 1.0);
|
||||
assert_eq!(progress.to_string(), "4/4");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_ui_activities_filetransfer_lib_transfer_states() {
|
||||
let mut states: TransferStates = TransferStates::default();
|
||||
assert_eq!(states.aborted, false);
|
||||
assert_eq!(states.full.total, 0);
|
||||
assert_eq!(states.full.written, 0);
|
||||
assert!(states.full.started.elapsed().as_secs() < 5);
|
||||
assert!(!states.aborted());
|
||||
assert_eq!(states.partial.total, 0);
|
||||
assert_eq!(states.partial.written, 0);
|
||||
assert!(states.partial.started.elapsed().as_secs() < 5);
|
||||
// Aborted
|
||||
states.abort();
|
||||
assert_eq!(states.aborted(), true);
|
||||
assert!(states.aborted());
|
||||
states.reset();
|
||||
assert_eq!(states.aborted(), false);
|
||||
states.full.total = 1024;
|
||||
assert!(!states.aborted());
|
||||
// Bytes tracking
|
||||
states.add_bytes(512);
|
||||
states.add_bytes(512);
|
||||
assert_eq!(states.full_size(), 1024);
|
||||
// Reset clears bytes
|
||||
states.reset();
|
||||
assert_eq!(states.full_size(), 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -211,43 +211,6 @@ impl FileTransferActivity {
|
||||
}
|
||||
}
|
||||
|
||||
// -- transfer sizes --
|
||||
|
||||
/// Get total size of transfer for the specified side.
|
||||
pub(super) fn get_total_transfer_size(&mut self, entry: &File, local: bool) -> usize {
|
||||
self.mount_blocking_wait("Calculating transfer size…");
|
||||
|
||||
let sz = if entry.is_dir() {
|
||||
let list_result = if local {
|
||||
self.browser.local_pane_mut().fs.list_dir(entry.path())
|
||||
} else {
|
||||
self.browser.remote_pane_mut().fs.list_dir(entry.path())
|
||||
};
|
||||
match list_result {
|
||||
Ok(files) => files
|
||||
.iter()
|
||||
.map(|x| self.get_total_transfer_size(x, local))
|
||||
.sum(),
|
||||
Err(err) => {
|
||||
self.log(
|
||||
LogLevel::Error,
|
||||
format!(
|
||||
"Could not list directory {}: {}",
|
||||
entry.path().display(),
|
||||
err
|
||||
),
|
||||
);
|
||||
0
|
||||
}
|
||||
}
|
||||
} else {
|
||||
entry.metadata.size as usize
|
||||
};
|
||||
|
||||
self.umount_wait();
|
||||
sz
|
||||
}
|
||||
|
||||
// -- file changed --
|
||||
|
||||
/// Check whether a file has changed on the specified side, compared to the given metadata.
|
||||
|
||||
@@ -87,9 +87,8 @@ impl FileTransferActivity {
|
||||
) -> Result<(), String> {
|
||||
// Reset states
|
||||
self.transfer.reset();
|
||||
// Calculate total size of transfer
|
||||
let total_transfer_size: usize = file.metadata.size as usize;
|
||||
self.transfer.full.init(total_transfer_size);
|
||||
// Single file = 1 entry
|
||||
self.transfer.full.init(1);
|
||||
// Mount progress bar
|
||||
self.mount_progress_bar(format!("Uploading {}…", file.path.display()));
|
||||
// Get remote path
|
||||
@@ -102,54 +101,56 @@ impl FileTransferActivity {
|
||||
remote_path.push(remote_file_name);
|
||||
// Send
|
||||
let result = self.filetransfer_send_one(file, remote_path.as_path(), file_name);
|
||||
if result.is_ok() {
|
||||
self.transfer.full.increment();
|
||||
}
|
||||
// Umount progress bar
|
||||
self.umount_progress_bar();
|
||||
// Return result
|
||||
result.map_err(|x| x.to_string())
|
||||
}
|
||||
|
||||
/// Send a `TransferPayload` of type `Any`
|
||||
/// Send a `TransferPayload` of type `Any`.
|
||||
fn filetransfer_send_any(
|
||||
&mut self,
|
||||
entry: &File,
|
||||
curr_remote_path: &Path,
|
||||
dst_name: Option<String>,
|
||||
) -> Result<(), String> {
|
||||
// Reset states
|
||||
self.transfer.reset();
|
||||
// Calculate total size of transfer
|
||||
let total_transfer_size: usize = self.get_total_transfer_size(entry, true);
|
||||
self.transfer.full.init(total_transfer_size);
|
||||
// Mount progress bar
|
||||
if !entry.is_dir() {
|
||||
self.transfer.full.init(1);
|
||||
}
|
||||
self.mount_progress_bar(format!("Uploading {}…", entry.path().display()));
|
||||
// Send recurse
|
||||
let result = self.filetransfer_send_recurse(entry, curr_remote_path, dst_name);
|
||||
// Umount progress bar
|
||||
let result = self.filetransfer_send_recurse(entry, curr_remote_path, dst_name, true);
|
||||
self.umount_progress_bar();
|
||||
result
|
||||
}
|
||||
|
||||
/// Send transfer queue entries to remote
|
||||
/// Send transfer queue entries to remote.
|
||||
fn filetransfer_send_transfer_queue(
|
||||
&mut self,
|
||||
entries: &[(File, PathBuf)],
|
||||
) -> Result<(), String> {
|
||||
// Reset states
|
||||
self.transfer.reset();
|
||||
// Calculate total size of transfer
|
||||
let total_transfer_size: usize = entries
|
||||
.iter()
|
||||
.map(|(x, _)| self.get_total_transfer_size(x, true))
|
||||
.sum();
|
||||
self.transfer.full.init(total_transfer_size);
|
||||
// Total = number of queue entries
|
||||
self.transfer.full.init(entries.len());
|
||||
// Mount progress bar
|
||||
self.mount_progress_bar(format!("Uploading {} entries…", entries.len()));
|
||||
// Send recurse
|
||||
let result = entries
|
||||
.iter()
|
||||
.map(|(x, remote)| self.filetransfer_send_recurse(x, remote, None))
|
||||
.find(|x| x.is_err())
|
||||
.unwrap_or(Ok(()));
|
||||
// Send each entry
|
||||
let mut result = Ok(());
|
||||
for (entry, remote) in entries {
|
||||
if self.transfer.aborted() {
|
||||
break;
|
||||
}
|
||||
let r = self.filetransfer_send_recurse(entry, remote, None, false);
|
||||
if r.is_err() {
|
||||
result = r;
|
||||
break;
|
||||
}
|
||||
self.transfer.full.increment();
|
||||
}
|
||||
// Umount progress bar
|
||||
self.umount_progress_bar();
|
||||
result
|
||||
@@ -160,6 +161,7 @@ impl FileTransferActivity {
|
||||
entry: &File,
|
||||
curr_remote_path: &Path,
|
||||
dst_name: Option<String>,
|
||||
track_progress: bool,
|
||||
) -> Result<(), String> {
|
||||
// Write popup
|
||||
let file_name = entry.name();
|
||||
@@ -200,14 +202,17 @@ impl FileTransferActivity {
|
||||
// Get files in dir
|
||||
match self.browser.local_pane_mut().fs.list_dir(entry.path()) {
|
||||
Ok(entries) => {
|
||||
// Iterate over files
|
||||
if track_progress {
|
||||
self.transfer.full.init(entries.len());
|
||||
}
|
||||
for entry in entries.iter() {
|
||||
// If aborted; break
|
||||
if self.transfer.aborted() {
|
||||
break;
|
||||
}
|
||||
// Send entry; name is always None after first call
|
||||
self.filetransfer_send_recurse(entry, remote_path.as_path(), None)?
|
||||
self.filetransfer_send_recurse(entry, remote_path.as_path(), None, false)?;
|
||||
if track_progress {
|
||||
self.transfer.full.increment();
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
@@ -262,7 +267,12 @@ impl FileTransferActivity {
|
||||
}
|
||||
Err(err.to_string())
|
||||
}
|
||||
Ok(_) => Ok(()),
|
||||
Ok(_) => {
|
||||
if track_progress {
|
||||
self.transfer.full.increment();
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
};
|
||||
// Scan dir on remote
|
||||
@@ -302,7 +312,7 @@ impl FileTransferActivity {
|
||||
host_bridge.path().display()
|
||||
),
|
||||
);
|
||||
self.transfer.full.update_progress(metadata.size as usize);
|
||||
self.transfer.add_bytes(metadata.size as usize);
|
||||
return Ok(());
|
||||
}
|
||||
// Upload file
|
||||
@@ -393,7 +403,7 @@ impl FileTransferActivity {
|
||||
};
|
||||
// Increase progress
|
||||
self.transfer.partial.update_progress(delta);
|
||||
self.transfer.full.update_progress(delta);
|
||||
self.transfer.add_bytes(delta);
|
||||
// Draw only if a significant progress has been made (performance improvement)
|
||||
if last_progress_val < self.transfer.partial.calc_progress() - 0.01 {
|
||||
// Draw
|
||||
@@ -467,23 +477,19 @@ impl FileTransferActivity {
|
||||
|
||||
/// Recv fs entry from remote.
|
||||
/// If dst_name is Some, entry will be saved with a different name.
|
||||
/// If entry is a directory, this applies to directory only
|
||||
/// If entry is a directory, this applies to directory only.
|
||||
fn filetransfer_recv_any(
|
||||
&mut self,
|
||||
entry: &File,
|
||||
host_path: &Path,
|
||||
dst_name: Option<String>,
|
||||
) -> Result<(), String> {
|
||||
// Reset states
|
||||
self.transfer.reset();
|
||||
// Calculate total transfer size
|
||||
let total_transfer_size: usize = self.get_total_transfer_size(entry, false);
|
||||
self.transfer.full.init(total_transfer_size);
|
||||
// Mount progress bar
|
||||
if !entry.is_dir() {
|
||||
self.transfer.full.init(1);
|
||||
}
|
||||
self.mount_progress_bar(format!("Downloading {}…", entry.path().display()));
|
||||
// Receive
|
||||
let result = self.filetransfer_recv_recurse(entry, host_path, dst_name);
|
||||
// Umount progress bar
|
||||
let result = self.filetransfer_recv_recurse(entry, host_path, dst_name, true);
|
||||
self.umount_progress_bar();
|
||||
result
|
||||
}
|
||||
@@ -496,40 +502,45 @@ impl FileTransferActivity {
|
||||
) -> Result<(), String> {
|
||||
// Reset states
|
||||
self.transfer.reset();
|
||||
// Calculate total transfer size
|
||||
let total_transfer_size: usize = entry.metadata.size as usize;
|
||||
self.transfer.full.init(total_transfer_size);
|
||||
// Single file = 1 entry
|
||||
self.transfer.full.init(1);
|
||||
// Mount progress bar
|
||||
self.mount_progress_bar(format!("Downloading {}…", entry.path.display()));
|
||||
// Receive
|
||||
let result = self.filetransfer_recv_one(host_bridge_path, entry, entry.name());
|
||||
if result.is_ok() {
|
||||
self.transfer.full.increment();
|
||||
}
|
||||
// Umount progress bar
|
||||
self.umount_progress_bar();
|
||||
// Return result
|
||||
result.map_err(|x| x.to_string())
|
||||
}
|
||||
|
||||
/// Receive transfer queue from remote
|
||||
/// Receive transfer queue from remote.
|
||||
fn filetransfer_recv_transfer_queue(
|
||||
&mut self,
|
||||
entries: &[(File, PathBuf)],
|
||||
) -> Result<(), String> {
|
||||
// Reset states
|
||||
self.transfer.reset();
|
||||
// Calculate total size of transfer
|
||||
let total_transfer_size: usize = entries
|
||||
.iter()
|
||||
.map(|(x, _)| self.get_total_transfer_size(x, false))
|
||||
.sum();
|
||||
self.transfer.full.init(total_transfer_size);
|
||||
// Total = number of queue entries
|
||||
self.transfer.full.init(entries.len());
|
||||
// Mount progress bar
|
||||
self.mount_progress_bar(format!("Downloading {} entries…", entries.len()));
|
||||
// Send recurse
|
||||
let result = entries
|
||||
.iter()
|
||||
.map(|(x, path)| self.filetransfer_recv_recurse(x, path, None))
|
||||
.find(|x| x.is_err())
|
||||
.unwrap_or(Ok(()));
|
||||
// Receive each entry
|
||||
let mut result = Ok(());
|
||||
for (entry, path) in entries {
|
||||
if self.transfer.aborted() {
|
||||
break;
|
||||
}
|
||||
let r = self.filetransfer_recv_recurse(entry, path, None, false);
|
||||
if r.is_err() {
|
||||
result = r;
|
||||
break;
|
||||
}
|
||||
self.transfer.full.increment();
|
||||
}
|
||||
// Umount progress bar
|
||||
self.umount_progress_bar();
|
||||
result
|
||||
@@ -540,6 +551,7 @@ impl FileTransferActivity {
|
||||
entry: &File,
|
||||
host_bridge_path: &Path,
|
||||
dst_name: Option<String>,
|
||||
track_progress: bool,
|
||||
) -> Result<(), String> {
|
||||
// Write popup
|
||||
let file_name = entry.name();
|
||||
@@ -583,19 +595,22 @@ impl FileTransferActivity {
|
||||
// Get files in dir from remote
|
||||
match self.browser.remote_pane_mut().fs.list_dir(entry.path()) {
|
||||
Ok(entries) => {
|
||||
// Iterate over files
|
||||
if track_progress {
|
||||
self.transfer.full.init(entries.len());
|
||||
}
|
||||
for entry in entries.iter() {
|
||||
// If transfer has been aborted; break
|
||||
if self.transfer.aborted() {
|
||||
break;
|
||||
}
|
||||
// Receive entry; name is always None after first call
|
||||
// Local path becomes host_bridge_dir_path
|
||||
self.filetransfer_recv_recurse(
|
||||
entry,
|
||||
host_bridge_dir_path.as_path(),
|
||||
None,
|
||||
)?
|
||||
false,
|
||||
)?;
|
||||
if track_progress {
|
||||
self.transfer.full.increment();
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
@@ -672,6 +687,9 @@ impl FileTransferActivity {
|
||||
}
|
||||
Err(err.to_string())
|
||||
} else {
|
||||
if track_progress {
|
||||
self.transfer.full.increment();
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
};
|
||||
@@ -704,9 +722,7 @@ impl FileTransferActivity {
|
||||
remote.path().display()
|
||||
),
|
||||
);
|
||||
self.transfer
|
||||
.full
|
||||
.update_progress(remote.metadata().size as usize);
|
||||
self.transfer.add_bytes(remote.metadata().size as usize);
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
@@ -786,7 +802,7 @@ impl FileTransferActivity {
|
||||
};
|
||||
// Set progress
|
||||
self.transfer.partial.update_progress(delta);
|
||||
self.transfer.full.update_progress(delta);
|
||||
self.transfer.add_bytes(delta);
|
||||
// Draw only if a significant progress has been made (performance improvement)
|
||||
if last_progress_val < self.transfer.partial.calc_progress() - 0.01 {
|
||||
// Draw
|
||||
|
||||
Reference in New Issue
Block a user