From 865566ad0c04b02e8345d9976a5811d161acc24e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn-Michael=20Miehe?= <40151420+ldericher@users.noreply.github.com> Date: Wed, 18 Jun 2025 23:40:27 +0000 Subject: [PATCH 1/3] [wip] implement handling "missing share" - rework `CacheFile` API --- src/appstate.rs | 29 ++++++++++++++---- src/cachefile.rs | 79 +++++++++++++++++++----------------------------- src/main.rs | 18 +++++++---- 3 files changed, 66 insertions(+), 60 deletions(-) diff --git a/src/appstate.rs b/src/appstate.rs index 8fb5252..68eff53 100644 --- a/src/appstate.rs +++ b/src/appstate.rs @@ -9,7 +9,7 @@ use indicatif::{ProgressBar, ProgressStyle}; use log::{debug, warn}; use crate::{ - cachefile::CacheFile, + cachefile::{CacheFile, FileState}, cli::Cli, file::{self, Chunk, FileTrait}, sharry::{self, Client}, @@ -153,14 +153,23 @@ impl AppState { Ok(self.is_done()) } - pub fn rewind(mut self) -> Option { - let Some(uploading) = self.inner.pop_file(&self.http) else { - warn!("rewind called on empty queue"); + pub fn rewind_chunk(mut self) -> Option { + let uploading = if let Some(state) = self.inner.pop_file() { + match state { + FileState::U(s) => s, + FileState::C(s) => {} + } + } else { + warn!("rewind_chunk called on empty queue"); return None; }; - let uploading = uploading.rewind()?; - self.inner.push_file(uploading); + let Some(FileState::U(uploading)) = self.inner.pop_file() else { + warn!("rewind_chunk called in invalid state"); + return None; + }; + + self.inner.push_file(FileState::U(uploading.rewind()?)); Some(self) } @@ -179,6 +188,14 @@ impl AppState { Some(self) } + pub fn rebuild_share(self, args: &Cli) -> sharry::Result { + let share_id = + self.http + .share_create(&args.get_uri(), &args.alias, args.get_share_request())?; + + Ok(Self::new(self.http, CacheFile::from_args(args, share_id))) + } + pub fn file_names(&self) -> Vec<&str> { self.inner.file_names() } diff --git a/src/cachefile.rs b/src/cachefile.rs index 44d07a0..022244e 100644 --- a/src/cachefile.rs +++ b/src/cachefile.rs @@ -14,34 +14,6 @@ use crate::{ sharry::{self, Client, Uri}, }; -#[derive(Serialize, Deserialize, Debug)] -enum FileState { - C(file::Checked), - U(file::Uploading), -} - -impl FileState { - fn file_name(&self) -> &str { - match self { - FileState::C(c) => c.get_name(), - FileState::U(u) => u.get_name(), - } - } - - fn start_upload( - self, - client: &impl sharry::Client, - uri: &sharry::Uri, - alias_id: &str, - share_id: &str, - ) -> sharry::Result { - match self { - FileState::C(checked) => checked.start_upload(client, uri, alias_id, share_id), - FileState::U(uploading) => Ok(uploading), - } - } -} - #[derive(Serialize, Deserialize, Debug)] pub struct CacheFile { #[serde(skip)] @@ -50,7 +22,9 @@ pub struct CacheFile { uri: Uri, alias_id: String, share_id: String, - files: VecDeque, + + uploading: Option, + files: VecDeque, } impl CacheFile { @@ -88,38 +62,47 @@ impl CacheFile { uri: args.get_uri(), alias_id: args.alias.clone(), share_id, - files: args.files.clone().into_iter().map(FileState::C).collect(), + uploading: None, + files: args.files.clone().into(), } } pub fn file_names(&self) -> Vec<&str> { - self.files.iter().map(FileState::file_name).collect() + let mut result = vec![]; + + if let Some(upl) = self.uploading.as_ref() { + result.push(upl.get_name()); + } + self.files.iter().map(|chk| result.push(chk.get_name())); + + result } - pub fn is_empty(&self) -> bool { - self.files.is_empty() - } + pub fn start_upload(&mut self, client: &impl Client) -> sharry::Result { + if self.uploading.is_some() { + return Ok(true); + } - pub fn pop_file(&mut self, client: &impl Client) -> Option { - if let Some(state) = self.files.pop_front() { - // HACK unwrap - // TODO somehow retry - Some( - state - .start_upload(client, &self.uri, &self.alias_id, &self.share_id) - .unwrap(), - ) + if let Some(chk) = self.files.pop_front() { + self.uploading = + Some(chk.start_upload(client, &self.uri, &self.alias_id, &self.share_id)?); + + Ok(true) } else { - None + Ok(false) } } - pub fn push_file(&mut self, file: file::Uploading) { - self.files.push_front(FileState::U(file)); + pub fn uploading(&mut self) -> &mut Option { + &mut self.uploading } - pub fn requeue_file(&mut self, file: file::Checked) { - self.files.push_back(FileState::C(file)); + pub fn abort_upload(&mut self) { + let Some(upl) = self.uploading.take() else { + panic!("abort called while not uploading"); + }; + + self.files.push_front(upl.abort()); } pub fn share_notify(&self, client: &impl Client) -> sharry::Result<()> { diff --git a/src/main.rs b/src/main.rs index de85892..9d3ef63 100644 --- a/src/main.rs +++ b/src/main.rs @@ -88,7 +88,6 @@ fn main() { match state.upload_chunk(&mut buffer) { Err(e) => { Log::handle(&e); - tries += 1; match e { ClientError::InvalidParameter(p) => match p { @@ -99,22 +98,29 @@ fn main() { Log::error("Failed to requeue file!"); }; - trace!("File {fid:?} requeued (tried: {tries})"); + trace!("File {fid:?} requeued"); state = s; } - // TODO Error 404: Share might have been deleted + // TODO Error 404: Share not found Parameter::ShareID(sid) => { - Log::error(format_args!("404 sid: {sid}")); + // requeue file + let Ok(s) = state.rebuild_share(&args) else { + Log::error("Failed to rebuild share!"); + }; + + trace!("Share {sid:?} rebuilt"); + state = s; } p => Log::error(format_args!("Unexpected {p}!")), }, _ => { // retry chunk - let Some(s) = state.rewind() else { + let Some(s) = state.rewind_chunk() else { Log::error("Failed to retry chunk!"); }; + tries += 1; - trace!("State rewound, retrying last chunk (tried: {tries})"); + trace!("State rewound, retrying last chunk (tries: {tries})"); state = s; } } From df055fc4e9619a241c89b4a3362bc7bf2452fbb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn-Michael=20Miehe?= <40151420+ldericher@users.noreply.github.com> Date: Thu, 19 Jun 2025 10:34:28 +0000 Subject: [PATCH 2/3] [wip] implement handling "missing share" - rework `CacheFile` API - fix `AppState::{next_chunk, is_done}` --- src/appstate.rs | 48 +++++++++++++++++++++--------------------------- src/cachefile.rs | 42 ++++++++++++++++++++++++++++++------------ 2 files changed, 51 insertions(+), 39 deletions(-) diff --git a/src/appstate.rs b/src/appstate.rs index 68eff53..29edeb9 100644 --- a/src/appstate.rs +++ b/src/appstate.rs @@ -9,7 +9,7 @@ use indicatif::{ProgressBar, ProgressStyle}; use log::{debug, warn}; use crate::{ - cachefile::{CacheFile, FileState}, + cachefile::CacheFile, cli::Cli, file::{self, Chunk, FileTrait}, sharry::{self, Client}, @@ -89,7 +89,7 @@ impl AppState { }) } - fn end_progressbar(&self, f: impl FnOnce(&ProgressBar)) { + fn drop_progressbar(&self, f: impl FnOnce(&ProgressBar)) { let mut slot = self.progress.borrow_mut(); if let Some(bar) = slot.as_ref() { f(bar); @@ -97,8 +97,8 @@ impl AppState { } } - fn next_chunk<'t>(&mut self, buffer: &'t mut [u8]) -> io::Result>> { - let Some(mut uploading) = self.inner.pop_file(&self.http) else { + fn next_chunk<'t>(&mut self, buffer: &'t mut [u8]) -> sharry::Result>> { + let Some(mut uploading) = self.inner.pop_uploading(&self.http)? else { self.inner .share_notify(&self.http) .unwrap_or_else(|e| warn!("Failed to notify the share: {e}")); @@ -109,38 +109,32 @@ impl AppState { self.get_or_create_progressbar(&uploading); - let chunk_res = uploading.read(buffer); - self.inner.push_file(uploading); + let chunk = { + let result = uploading.read(buffer); + self.inner.push_uploading(uploading); - let chunk = chunk_res?; + result? + }; debug!("{chunk:?}"); Ok(Some(chunk)) } fn is_done(&mut self) -> bool { - let Some(uploading) = self.inner.pop_file(&self.http) else { - return true; - }; + if let Some(path) = self.inner.check_eof() { + debug!("Finished {:?}!", path.display()); + self.drop_progressbar(ProgressBar::finish); + } else if let Some(uploading) = self.inner.peek_uploading() { + let bar = self.get_or_create_progressbar(uploading); + bar.set_position(uploading.get_offset()); + // BUG in `indicatif` crate? + // `set_position` does not force an immediate redraw, so we also call `inc_length` here + bar.inc_length(0); - match uploading.check_eof() { - Ok(uploading) => { - let bar = self.get_or_create_progressbar(&uploading); - bar.set_position(uploading.get_offset()); - // BUG in `indicatif` crate? - // `set_position` does not force an immediate redraw, so we also call `inc_length` here - bar.inc_length(0); - drop(bar); - - self.inner.push_file(uploading); - } - Err(path) => { - debug!("Finished {:?}!", path.display()); - self.end_progressbar(|pb| pb.finish()); - } + return false; } - self.inner.is_empty() + self.inner.queue_empty() } pub fn upload_chunk(&mut self, buffer: &mut [u8]) -> sharry::Result { @@ -183,7 +177,7 @@ impl AppState { let checked = uploading.abort(); self.inner.requeue_file(checked); - self.end_progressbar(|pb| pb.abandon()); + self.drop_progressbar(|pb| pb.abandon()); Some(self) } diff --git a/src/cachefile.rs b/src/cachefile.rs index 022244e..12d3478 100644 --- a/src/cachefile.rs +++ b/src/cachefile.rs @@ -78,23 +78,41 @@ impl CacheFile { result } - pub fn start_upload(&mut self, client: &impl Client) -> sharry::Result { - if self.uploading.is_some() { - return Ok(true); - } + pub fn queue_empty(&self) -> bool { + self.files.is_empty() + } - if let Some(chk) = self.files.pop_front() { - self.uploading = - Some(chk.start_upload(client, &self.uri, &self.alias_id, &self.share_id)?); - - Ok(true) + pub fn pop_uploading( + &mut self, + client: &impl Client, + ) -> sharry::Result> { + if let Some(upl) = self.uploading.take() { + Ok(Some(upl)) + } else if let Some(chk) = self.files.pop_front() { + let upl = chk.start_upload(client, &self.uri, &self.alias_id, &self.share_id)?; + Ok(Some(upl)) } else { - Ok(false) + Ok(None) } } - pub fn uploading(&mut self) -> &mut Option { - &mut self.uploading + pub fn push_uploading(&mut self, uploading: file::Uploading) { + self.uploading.replace(uploading); + } + + pub fn peek_uploading(&self) -> Option<&file::Uploading> { + self.uploading.as_ref() + } + + pub fn check_eof(&mut self) -> Option { + if let Some(upl) = self.uploading.take() { + match upl.check_eof() { + Ok(upl) => self.push_uploading(upl), + Err(p) => return Some(p), + } + } + + None } pub fn abort_upload(&mut self) { From 393feec12541692e84521e59c37a3185f6776b68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn-Michael=20Miehe?= <40151420+ldericher@users.noreply.github.com> Date: Tue, 24 Jun 2025 00:11:24 +0000 Subject: [PATCH 3/3] [wip] implement handling "missing share" - rework `CacheFile` API - fix `AppState` using new `CacheFile` - fix `main` using new `AppState` - logging enhancement --- src/appstate.rs | 112 +++++++++++++++++++---------------------------- src/cachefile.rs | 30 ++++++++----- src/impl_ureq.rs | 2 + src/main.rs | 37 +++++++--------- 4 files changed, 81 insertions(+), 100 deletions(-) diff --git a/src/appstate.rs b/src/appstate.rs index 29edeb9..2617861 100644 --- a/src/appstate.rs +++ b/src/appstate.rs @@ -1,8 +1,4 @@ -use std::{ - cell::{Ref, RefCell}, - fmt, io, - time::Duration, -}; +use std::{cell::RefCell, fmt, io, time::Duration}; use console::style; use indicatif::{ProgressBar, ProgressStyle}; @@ -11,7 +7,7 @@ use log::{debug, warn}; use crate::{ cachefile::CacheFile, cli::Cli, - file::{self, Chunk, FileTrait}, + file::{Chunk, FileTrait}, sharry::{self, Client}, }; @@ -61,10 +57,14 @@ impl AppState { Ok(Self::new(http, CacheFile::from_args(args, share_id))) } - fn get_or_create_progressbar(&self, uploading: &file::Uploading) -> Ref<'_, ProgressBar> { + fn with_progressbar(&self, drop_bar: bool, f: impl FnOnce(&ProgressBar)) { + let Some(upl) = self.inner.peek_uploading() else { + return; + }; + let mut slot = self.progress.borrow_mut(); if slot.is_none() { - let bar = ProgressBar::new(uploading.get_size()) + let bar = ProgressBar::no_length() .with_style( ProgressStyle::with_template(&format!( concat!( @@ -76,45 +76,45 @@ impl AppState { )) .expect("style template is not valid"), ) - .with_position(uploading.get_offset()) - .with_message(uploading.get_name().to_owned()); + .with_message(upl.get_name().to_owned()); bar.enable_steady_tick(Duration::from_millis(100)); *slot = Some(bar); } - drop(slot); - Ref::map(self.progress.borrow(), |opt| { - opt.as_ref().expect("somehow the slot holds None") - }) - } + let bar = slot.as_ref().expect("somehow the slot holds None"); - fn drop_progressbar(&self, f: impl FnOnce(&ProgressBar)) { - let mut slot = self.progress.borrow_mut(); - if let Some(bar) = slot.as_ref() { - f(bar); + bar.set_position(upl.get_offset()); + // BUG in `indicatif` crate? + // `set_position` does not force an immediate redraw, so we also call `inc_length` here + bar.set_length(upl.get_size()); + // bar.inc_length(0); + + f(bar); + + if drop_bar { *slot = None; } } - fn next_chunk<'t>(&mut self, buffer: &'t mut [u8]) -> sharry::Result>> { - let Some(mut uploading) = self.inner.pop_uploading(&self.http)? else { - self.inner - .share_notify(&self.http) - .unwrap_or_else(|e| warn!("Failed to notify the share: {e}")); + fn touch_progressbar(&self) { + self.with_progressbar(false, |_| ()); + } + fn next_chunk<'t>(&mut self, buffer: &'t mut [u8]) -> sharry::Result>> { + if self.inner.get_uploading(&self.http)?.is_none() { return Ok(None); - }; + } + + self.touch_progressbar(); + + let uploading = self + .inner + .get_uploading(&self.http)? + .expect("we just checked!"); debug!("{uploading:?}"); - self.get_or_create_progressbar(&uploading); - - let chunk = { - let result = uploading.read(buffer); - self.inner.push_uploading(uploading); - - result? - }; + let chunk = uploading.read(buffer)?; debug!("{chunk:?}"); Ok(Some(chunk)) @@ -123,13 +123,9 @@ impl AppState { fn is_done(&mut self) -> bool { if let Some(path) = self.inner.check_eof() { debug!("Finished {:?}!", path.display()); - self.drop_progressbar(ProgressBar::finish); - } else if let Some(uploading) = self.inner.peek_uploading() { - let bar = self.get_or_create_progressbar(uploading); - bar.set_position(uploading.get_offset()); - // BUG in `indicatif` crate? - // `set_position` does not force an immediate redraw, so we also call `inc_length` here - bar.inc_length(0); + self.with_progressbar(true, ProgressBar::finish); + } else if self.inner.peek_uploading().is_some() { + self.touch_progressbar(); return false; } @@ -139,6 +135,10 @@ impl AppState { pub fn upload_chunk(&mut self, buffer: &mut [u8]) -> sharry::Result { let Some(chunk) = self.next_chunk(buffer)? else { + self.inner + .share_notify(&self.http) + .unwrap_or_else(|e| warn!("Failed to notify the share: {e}")); + return Ok(true); }; @@ -148,38 +148,14 @@ impl AppState { } pub fn rewind_chunk(mut self) -> Option { - let uploading = if let Some(state) = self.inner.pop_file() { - match state { - FileState::U(s) => s, - FileState::C(s) => {} - } - } else { - warn!("rewind_chunk called on empty queue"); - return None; - }; - - let Some(FileState::U(uploading)) = self.inner.pop_file() else { - warn!("rewind_chunk called in invalid state"); - return None; - }; - - self.inner.push_file(FileState::U(uploading.rewind()?)); + self.inner = self.inner.rewind_chunk()?; Some(self) } - pub fn requeue_file(mut self) -> Option { - let Some(uploading) = self.inner.pop_file(&self.http) else { - warn!("requeue_file called on empty queue"); - return None; - }; - - let checked = uploading.abort(); - self.inner.requeue_file(checked); - - self.drop_progressbar(|pb| pb.abandon()); - - Some(self) + pub fn abort_upload(&mut self) { + self.inner.abort_upload(); + self.with_progressbar(true, ProgressBar::abandon); } pub fn rebuild_share(self, args: &Cli) -> sharry::Result { diff --git a/src/cachefile.rs b/src/cachefile.rs index 12d3478..0478355 100644 --- a/src/cachefile.rs +++ b/src/cachefile.rs @@ -82,24 +82,22 @@ impl CacheFile { self.files.is_empty() } - pub fn pop_uploading( + pub fn get_uploading( &mut self, client: &impl Client, - ) -> sharry::Result> { - if let Some(upl) = self.uploading.take() { - Ok(Some(upl)) + ) -> sharry::Result> { + if self.uploading.is_some() { + Ok(self.uploading.as_mut()) } else if let Some(chk) = self.files.pop_front() { let upl = chk.start_upload(client, &self.uri, &self.alias_id, &self.share_id)?; - Ok(Some(upl)) + self.uploading.replace(upl); + + Ok(self.uploading.as_mut()) } else { Ok(None) } } - pub fn push_uploading(&mut self, uploading: file::Uploading) { - self.uploading.replace(uploading); - } - pub fn peek_uploading(&self) -> Option<&file::Uploading> { self.uploading.as_ref() } @@ -107,7 +105,7 @@ impl CacheFile { pub fn check_eof(&mut self) -> Option { if let Some(upl) = self.uploading.take() { match upl.check_eof() { - Ok(upl) => self.push_uploading(upl), + Ok(upl) => self.uploading = Some(upl), Err(p) => return Some(p), } } @@ -115,9 +113,19 @@ impl CacheFile { None } + pub fn rewind_chunk(mut self) -> Option { + let Some(upl) = self.uploading.take() else { + panic!("rewind_chunk called while not uploading"); + }; + + self.uploading = Some(upl.rewind()?); + + Some(self) + } + pub fn abort_upload(&mut self) { let Some(upl) = self.uploading.take() else { - panic!("abort called while not uploading"); + panic!("abort_upload called while not uploading"); }; self.files.push_front(upl.abort()); diff --git a/src/impl_ureq.rs b/src/impl_ureq.rs index 87d4c12..d8c50fd 100644 --- a/src/impl_ureq.rs +++ b/src/impl_ureq.rs @@ -72,6 +72,8 @@ impl sharry::Client for ureq::Agent { debug!("{res:?}"); if res.success && (res.message == "Share created.") { + trace!("new share id: {:?}", res.id); + Ok(res.id) } else { Err(ClientError::response(format!("{res:?}"))) diff --git a/src/main.rs b/src/main.rs index 9d3ef63..cd711d6 100644 --- a/src/main.rs +++ b/src/main.rs @@ -89,40 +89,35 @@ fn main() { Err(e) => { Log::handle(&e); - match e { - ClientError::InvalidParameter(p) => match p { + if let ClientError::InvalidParameter(p) = e { + match p { // TODO Error 404: File not found Parameter::FileID(fid) => { - // requeue file - let Some(s) = state.requeue_file() else { - Log::error("Failed to requeue file!"); - }; + trace!("requeueing file {fid:?}"); - trace!("File {fid:?} requeued"); - state = s; + state.abort_upload(); } // TODO Error 404: Share not found Parameter::ShareID(sid) => { - // requeue file + trace!("rebuilding share {sid:?}"); + + // rebuild share let Ok(s) = state.rebuild_share(&args) else { Log::error("Failed to rebuild share!"); }; - - trace!("Share {sid:?} rebuilt"); state = s; } p => Log::error(format_args!("Unexpected {p}!")), - }, - _ => { - // retry chunk - let Some(s) = state.rewind_chunk() else { - Log::error("Failed to retry chunk!"); - }; - tries += 1; - - trace!("State rewound, retrying last chunk (tries: {tries})"); - state = s; } + } else { + // retry chunk + let Some(s) = state.rewind_chunk() else { + Log::error("Failed to retry chunk!"); + }; + tries += 1; + + trace!("State rewound, retrying last chunk (tries: {tries})"); + state = s; } } Ok(false) => {