From 74b3cb1c5884acbeb7eb4b9e0d87933e04d14ad3 Mon Sep 17 00:00:00 2001 From: Kingtous Date: Fri, 29 Apr 2022 00:42:06 +0800 Subject: [PATCH] fix: skip issue --- libs/hbb_common/src/fs.rs | 19 ++-- src/server/connection.rs | 2 +- src/ui/cm.rs | 37 ++++--- src/ui/remote.rs | 207 ++++++++++++++++++++++---------------- 4 files changed, 158 insertions(+), 107 deletions(-) diff --git a/libs/hbb_common/src/fs.rs b/libs/hbb_common/src/fs.rs index 7180cbdd7..9a040a00c 100644 --- a/libs/hbb_common/src/fs.rs +++ b/libs/hbb_common/src/fs.rs @@ -191,6 +191,7 @@ pub fn is_file_exists(file_path: &str) -> bool { return Path::new(file_path).exists(); } +#[inline] pub fn can_enable_overwrite_detection(version: i64) -> bool { version >= get_version_number("1.2.0") } @@ -525,14 +526,14 @@ impl TransferJob { match r.union { Some(file_transfer_send_confirm_request::Union::skip(s)) => { if s { - log::info!("skip file id:{}, file_num:{}", r.id, r.file_num); + log::debug!("skip file id:{}, file_num:{}", r.id, r.file_num); self.skip_current_file(); } else { self.set_file_confirmed(true); } } Some(file_transfer_send_confirm_request::Union::offset_blk(offset)) => { - log::info!("file confirmed"); + log::debug!("file confirmed"); self.set_file_confirmed(true); } _ => {} @@ -702,11 +703,17 @@ pub fn create_dir(dir: &str) -> ResultType<()> { Ok(()) } +pub enum DigestCheckResult { + IsSame, + NeedConfirm(FileTransferDigest), + NoSuchFile, +} + #[inline] pub fn is_write_need_confirmation( file_path: &str, digest: &FileTransferDigest, -) -> ResultType> { +) -> ResultType { let path = Path::new(file_path); if path.exists() && path.is_file() { let metadata = std::fs::metadata(path)?; @@ -722,9 +729,9 @@ pub fn is_write_need_confirmation( metadata.len() ); if remote_mt == local_mt && digest.file_size == metadata.len() { - return Ok(None); + return Ok(DigestCheckResult::IsSame); } - Ok(Some(FileTransferDigest { + Ok(DigestCheckResult::NeedConfirm(FileTransferDigest { id: digest.id, file_num: digest.file_num, last_edit_timestamp: local_mt.as_secs(), @@ -732,6 +739,6 @@ pub fn is_write_need_confirmation( ..Default::default() })) } else { - Ok(None) + Ok(DigestCheckResult::NoSuchFile) } } diff --git a/src/server/connection.rs b/src/server/connection.rs index 5ecee471c..b0f651884 100644 --- a/src/server/connection.rs +++ b/src/server/connection.rs @@ -275,7 +275,7 @@ impl Connection { } } ipc::Data::RawMessage(bytes) => { - conn.stream.send_raw(bytes).await; + allow_err!(conn.stream.send_raw(bytes).await); } #[cfg(windows)] ipc::Data::ClipbaordFile(_clip) => { diff --git a/src/ui/cm.rs b/src/ui/cm.rs index 444e26529..7eb99a4c2 100644 --- a/src/ui/cm.rs +++ b/src/ui/cm.rs @@ -6,6 +6,7 @@ use clipboard::{ }; use hbb_common::fs::{ can_enable_overwrite_detection, get_string, is_write_need_confirmation, new_send_confirm, + DigestCheckResult, }; use hbb_common::log::log; use hbb_common::{ @@ -211,23 +212,29 @@ impl ConnectionManager { if let Some(file) = job.files().get(file_num as usize) { let path = get_string(&job.join(&file.name)); match is_write_need_confirmation(&path, &digest) { - Ok(digest) => { - if let Some(mut digest) = digest { - // upload to server, but server has the same file, request - digest.is_upload = is_upload; - log::info!( + Ok(digest_result) => { + match digest_result { + DigestCheckResult::IsSame => { + req.set_skip(true); + let msg_out = new_send_confirm(req); + Self::send(msg_out, conn).await; + } + DigestCheckResult::NeedConfirm(mut digest) => { + // upload to server, but server has the same file, request + digest.is_upload = is_upload; + log::info!( "server has the same file, send server digest to local" ); - let mut msg_out = Message::new(); - let mut fr = FileResponse::new(); - fr.set_digest(digest); - msg_out.set_file_response(fr); - Self::send(msg_out, conn).await; - } else { - log::info!("skip job {}, file_num {}", id, file_num); - req.set_skip(true); - let msg_out = new_send_confirm(req); - Self::send(msg_out, conn).await; + let mut msg_out = Message::new(); + let mut fr = FileResponse::new(); + fr.set_digest(digest); + msg_out.set_file_response(fr); + Self::send(msg_out, conn).await; + } + DigestCheckResult::NoSuchFile => { + let msg_out = new_send_confirm(req); + Self::send(msg_out, conn).await; + } } } Err(err) => { diff --git a/src/ui/remote.rs b/src/ui/remote.rs index 8a1492393..f53f121aa 100644 --- a/src/ui/remote.rs +++ b/src/ui/remote.rs @@ -12,7 +12,7 @@ use clipboard::{ }; use enigo::{self, Enigo, KeyboardControllable}; use hbb_common::fs::{ - can_enable_overwrite_detection, get_string, is_file_exists, new_send_confirm, + can_enable_overwrite_detection, get_string, is_file_exists, new_send_confirm, DigestCheckResult, }; use hbb_common::log::log; use hbb_common::{ @@ -1849,104 +1849,141 @@ impl Remote { } } } - Some(message::Union::file_response(fr)) => match fr.union { - Some(file_response::Union::dir(fd)) => { - let entries = fd.entries.to_vec(); - let mut m = make_fd(fd.id, &entries, fd.id > 0); - if fd.id <= 0 { - m.set_item("path", fd.path); - } - self.handler.call("updateFolderFiles", &make_args!(m)); - if let Some(job) = fs::get_job(fd.id, &mut self.write_jobs) { - job.set_files(entries); - } else if let Some(job) = self.remove_jobs.get_mut(&fd.id) { - job.files = entries; - } - } - Some(file_response::Union::digest(digest)) => { - if digest.is_upload { - if let Some(job) = fs::get_job(digest.id, &mut self.read_jobs) { - if let Some(file) = job.files().get(digest.file_num as usize) { - let read_path = get_string(&job.join(&file.name)); - self.handler.call( - "overrideFileConfirm", - &make_args!(digest.id, digest.file_num, read_path, true), - ); - } + Some(message::Union::file_response(fr)) => { + match fr.union { + Some(file_response::Union::dir(fd)) => { + let entries = fd.entries.to_vec(); + let mut m = make_fd(fd.id, &entries, fd.id > 0); + if fd.id <= 0 { + m.set_item("path", fd.path); } - } else { - if let Some(job) = fs::get_job(digest.id, &mut self.write_jobs) { - if let Some(file) = job.files().get(digest.file_num as usize) { - let write_path = get_string(&job.join(&file.name)); - let overwrite_strategy = job.default_overwrite_strategy(); - match fs::is_write_need_confirmation(&write_path, &digest) { - Ok(res) => { - if res.is_some() { - // need confirm - if overwrite_strategy.is_none() { - self.handler.call( - "overrideFileConfirm", - &make_args!( - digest.id, - digest.file_num, - write_path, - false - ), - ); - } else { - let msg = new_send_confirm( - FileTransferSendConfirmRequest { - id: digest.id, - file_num: digest.file_num, - union: if overwrite_strategy.unwrap() { - Some(file_transfer_send_confirm_request::Union::offset_blk(0)) - } else { - Some(file_transfer_send_confirm_request::Union::skip(true)) - }, - ..Default::default() - }, - ); - allow_err!(peer.send(&msg).await); - } - } else { - let msg= new_send_confirm(FileTransferSendConfirmRequest { + self.handler.call("updateFolderFiles", &make_args!(m)); + if let Some(job) = fs::get_job(fd.id, &mut self.write_jobs) { + job.set_files(entries); + } else if let Some(job) = self.remove_jobs.get_mut(&fd.id) { + job.files = entries; + } + } + Some(file_response::Union::digest(digest)) => { + if digest.is_upload { + if let Some(job) = fs::get_job(digest.id, &mut self.read_jobs) { + if let Some(file) = job.files().get(digest.file_num as usize) { + let read_path = get_string(&job.join(&file.name)); + let overwrite_strategy = job.default_overwrite_strategy(); + if let Some(overwrite) = overwrite_strategy { + let req = FileTransferSendConfirmRequest { id: digest.id, file_num: digest.file_num, - union: Some(file_transfer_send_confirm_request::Union::skip(true)), + union: Some(if overwrite { + file_transfer_send_confirm_request::Union::offset_blk(0) + } else { + file_transfer_send_confirm_request::Union::skip( + true, + ) + }), ..Default::default() - }); - allow_err!(peer.send(&msg).await); - } + }; + job.confirm(&req); + let msg = new_send_confirm(req); + allow_err!(peer.send(&msg).await); + } else { + self.handler.call( + "overrideFileConfirm", + &make_args!( + digest.id, + digest.file_num, + read_path, + true + ), + ); } - Err(err) => { - println!("error recving digest: {}", err); + } + } + } else { + if let Some(job) = fs::get_job(digest.id, &mut self.write_jobs) { + if let Some(file) = job.files().get(digest.file_num as usize) { + let write_path = get_string(&job.join(&file.name)); + let overwrite_strategy = job.default_overwrite_strategy(); + match fs::is_write_need_confirmation(&write_path, &digest) { + Ok(res) => match res { + DigestCheckResult::IsSame => { + let msg= new_send_confirm(FileTransferSendConfirmRequest { + id: digest.id, + file_num: digest.file_num, + union: Some(file_transfer_send_confirm_request::Union::skip(true)), + ..Default::default() + }); + allow_err!(peer.send(&msg).await); + } + DigestCheckResult::NeedConfirm(digest) => { + if let Some(overwrite) = overwrite_strategy { + let msg = new_send_confirm( + FileTransferSendConfirmRequest { + id: digest.id, + file_num: digest.file_num, + union: Some(if overwrite { + file_transfer_send_confirm_request::Union::offset_blk(0) + } else { + file_transfer_send_confirm_request::Union::skip(true) + }), + ..Default::default() + }, + ); + allow_err!(peer.send(&msg).await); + } else { + self.handler.call( + "overrideFileConfirm", + &make_args!( + digest.id, + digest.file_num, + write_path, + false + ), + ); + } + } + DigestCheckResult::NoSuchFile => { + let msg = new_send_confirm( + FileTransferSendConfirmRequest { + id: digest.id, + file_num: digest.file_num, + union: Some(file_transfer_send_confirm_request::Union::offset_blk(0)), + ..Default::default() + }, + ); + allow_err!(peer.send(&msg).await); + } + }, + Err(err) => { + println!("error recving digest: {}", err); + } } } } } } - } - Some(file_response::Union::block(block)) => { - log::info!("file response block, file num: {}", block.file_num); - if let Some(job) = fs::get_job(block.id, &mut self.write_jobs) { - if let Err(_err) = job.write(block, None).await { - // to-do: add "skip" for writing job + Some(file_response::Union::block(block)) => { + log::info!("file response block, file num: {}", block.file_num); + if let Some(job) = fs::get_job(block.id, &mut self.write_jobs) { + if let Err(_err) = job.write(block, None).await { + // to-do: add "skip" for writing job + } + self.update_jobs_status(); } - self.update_jobs_status(); } - } - Some(file_response::Union::done(d)) => { - if let Some(job) = fs::get_job(d.id, &mut self.write_jobs) { - job.modify_time(); - fs::remove_job(d.id, &mut self.write_jobs); + Some(file_response::Union::done(d)) => { + if let Some(job) = fs::get_job(d.id, &mut self.write_jobs) { + job.modify_time(); + fs::remove_job(d.id, &mut self.write_jobs); + } + self.handle_job_status(d.id, d.file_num, None); } - self.handle_job_status(d.id, d.file_num, None); + Some(file_response::Union::error(e)) => { + self.handle_job_status(e.id, e.file_num, Some(e.error)); + } + _ => {} } - Some(file_response::Union::error(e)) => { - self.handle_job_status(e.id, e.file_num, Some(e.error)); - } - _ => {} - }, + } Some(message::Union::misc(misc)) => match misc.union { Some(misc::Union::audio_format(f)) => { self.audio_sender.send(MediaData::AudioFormat(f)).ok();