diff --git a/crates/core/src/protocol.rs b/crates/core/src/protocol.rs index 2e6fee7..b5ac08d 100644 --- a/crates/core/src/protocol.rs +++ b/crates/core/src/protocol.rs @@ -37,7 +37,7 @@ pub enum Command { Pause { app: String, }, - /// Stop a running download, or throw away a paused one. + /// Stop a running download, or throw away a paused one or one kept after a failed install. Cancel { app: String, }, @@ -96,6 +96,9 @@ pub struct PausedDownload { pub const PAUSED: &str = "paused"; /// The `JobFinished` error a cancelled job ends with (not a failure). pub const CANCELLED: &str = "cancelled"; +/// The `JobFinished` error of an install whose system prompt (password, Windows Installer) the +/// user dismissed after the download finished (not a failure). +pub const INSTALL_CANCELLED: &str = "install cancelled"; /// Which service an app's release information came from (the footer names them). #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)] @@ -171,6 +174,12 @@ pub enum Event { app: String, download: Option, }, + /// Whether a finished download is waiting because its install step failed; `Install` checks + /// it again and goes straight to installing. + ReadyToInstall { + app: String, + ready: bool, + }, /// A job ended: `Ok` carries a short success message, `Err` what went wrong. JobFinished { app: String, diff --git a/crates/native/src/backend.rs b/crates/native/src/backend.rs index 9f2eea5..f665a5a 100644 --- a/crates/native/src/backend.rs +++ b/crates/native/src/backend.rs @@ -169,8 +169,9 @@ impl NativeBackend { let platform = self.platform; let options = self.options(); std::thread::spawn(move || { - // Leftovers from interrupted installs or older versions of the launcher. - http::clean_downloads(&paths.downloads(), Duration::from_secs(30 * 86_400)); + // Leftovers from interrupted installs or older versions of the launcher. A paused + // download stays a month; a finished one kept after a failed install, a week. + http::clean_downloads(&paths.downloads(), Duration::from_secs(30 * 86_400), Duration::from_secs(7 * 86_400)); let snapshot = crate::detect::Snapshot::take(); let manifest = shared.manifest.lock().unwrap_or_else(PoisonError::into_inner).clone(); for app in &SUITE { @@ -213,6 +214,7 @@ impl NativeBackend { } let shared = self.shared.clone(); let paths = self.paths.clone(); + let platform = self.platform; let pre = self.options().prereleases; std::thread::spawn(move || { shared.emit(Event::CheckStarted); @@ -268,6 +270,11 @@ impl NativeBackend { shared.loaded.store(true, Ordering::SeqCst); } } + // A newer release changes which file an install would use. + let options = shared.options.lock().unwrap_or_else(PoisonError::into_inner).clone(); + for app in &SUITE { + report_partial(app, &shared, &paths, platform, &options); + } let sources = sources.into_inner().unwrap_or_else(PoisonError::into_inner); let source_used = if sources.contains(&Source::Cache) { Source::Cache @@ -463,20 +470,28 @@ fn planned_asset( pick_asset(release, &stems, platform, &kinds).map(|(a, k)| (a.clone(), k)) } -/// Tells the UI whether `app` has a paused download waiting. +/// Tells the UI whether `app` has a paused download waiting, or a finished one to install. fn report_partial(app: &CraftApp, shared: &Shared, paths: &Paths, platform: Option, options: &BackendOptions) { - let download = partial_for(app, shared, paths, platform, options).map(|(asset, done)| PausedDownload { done, total: asset.size }); + let planned = planned_download(app, shared, paths, platform, options); + let download = planned.as_ref().and_then(|(dest, size)| Some(PausedDownload { done: http::partial_bytes(dest, *size)?, total: *size })); shared.emit(Event::Paused { app: app.id.to_string(), download }); + let ready = planned.is_some_and(|(dest, size)| http::finished(&dest, size)); + shared.emit(Event::ReadyToInstall { app: app.id.to_string(), ready }); } -/// The planned download of `app` and how many bytes of it are already on disk. -fn partial_for(app: &CraftApp, shared: &Shared, paths: &Paths, platform: Option, options: &BackendOptions) -> Option<(Asset, u64)> { +/// Where the planned download of `app` is kept, and its size. +fn planned_download( + app: &CraftApp, + shared: &Shared, + paths: &Paths, + platform: Option, + options: &BackendOptions, +) -> Option<(std::path::PathBuf, u64)> { let release = shared.latest.lock().unwrap_or_else(PoisonError::into_inner).get(app.id).cloned()?; let existing = shared.installed.lock().unwrap_or_else(PoisonError::into_inner).get(app.id).cloned(); let record = shared.manifest.lock().unwrap_or_else(PoisonError::into_inner).apps.get(app.id).cloned(); let (asset, _) = planned_asset(app, &release, platform?, options, existing.as_ref(), record.as_ref())?; - let done = http::partial_bytes(&install::download_path(paths, &asset), asset.size)?; - Some((asset, done)) + artcraft_launcher_core::release::valid_file_name(&asset.name).then(|| (install::download_path(paths, &asset), asset.size)) } /// How much the feed is trusted in one check, and how freely GitHub may be asked. @@ -686,14 +701,15 @@ impl Backend for NativeBackend { let running = self.shared.jobs.lock().unwrap_or_else(PoisonError::into_inner).get(&app).cloned(); match running { Some(stop) => stop.cancel(), - // A paused download: throw its partial file away. + // A paused download, or one kept after a failed install: throw the file away. None => { if let Some(def) = CraftApp::by_id(&app) - && let Some((asset, _)) = partial_for(def, &self.shared, &self.paths, self.platform, &self.options()) + && let Some((dest, size)) = planned_download(def, &self.shared, &self.paths, self.platform, &self.options()) { - http::discard_partial(&install::download_path(&self.paths, &asset), asset.size); + http::discard(&dest, size); } - self.shared.emit(Event::Paused { app, download: None }); + self.shared.emit(Event::Paused { app: app.clone(), download: None }); + self.shared.emit(Event::ReadyToInstall { app, ready: false }); } } } @@ -724,6 +740,11 @@ impl Backend for NativeBackend { *o = options; changed }; + // Another package format means another file to download. + let options = self.options(); + for app in &SUITE { + report_partial(app, &self.shared, &self.paths, self.platform, &options); + } if changed_pre { self.check(true); } @@ -945,4 +966,114 @@ mod tests { let (_, _, used) = latest_for(photo, Some(&feed), LOADED, &github, &fresh_shared, false, false, NOW + 2 * 86_400); assert_eq!(used, Some(Source::GitHubApi)); } + + const APP: &str = "photocraft"; + + /// A PhotoCraft release with one file per platform and format, so whichever this machine + /// installs is there. + fn release(version: &str) -> Release { + let suffixes = ["-portable.zip", ".tar.gz", ".msi", ".dmg", ".AppImage", ".deb", ".rpm", ".flatpak"]; + let tokens = ["windows-x64", "windows-x86", "windows-arm64", "macos-universal", "linux-x86_64", "linux-aarch64", "freebsd-x86_64"]; + let assets = tokens + .iter() + .flat_map(|t| suffixes.iter().map(move |s| format!("{APP}-{version}-{t}{s}"))) + .map(|name| Asset { url: format!("https://example.invalid/{name}"), name, size: 3, sha256: None }) + .collect(); + Release { assets, ..Release::default() } + } + + /// A backend that knows `release("0.5.0")` and keeps its files under `root`. + fn backend(root: &std::path::Path, platform: Platform, source: Option>) -> NativeBackend { + let shared = shared(); + shared.latest.lock().unwrap().insert(APP.to_string(), release("0.5.0")); + let shared = Arc::new(shared); + NativeBackend { shared, paths: Paths::under(root), http: None, source, platform: Some(platform), self_install: None } + } + + /// Reports again, and returns what the UI was last told about a kept download. + fn ready(backend: &mut NativeBackend) -> Option { + let app = CraftApp::by_id(APP).unwrap(); + report_partial(app, &backend.shared, &backend.paths, backend.platform, &BackendOptions::default()); + last_ready(backend.poll()) + } + + fn last_ready(events: Vec) -> Option { + events.into_iter().rev().find_map(|e| match e { + Event::ReadyToInstall { app, ready } if app == APP => Some(ready), + _ => None, + }) + } + + /// Puts the planned download on disk, as a failed install leaves it. + fn keep_file(backend: &NativeBackend) -> std::path::PathBuf { + let app = CraftApp::by_id(APP).unwrap(); + let (dest, size) = planned_download(app, &backend.shared, &backend.paths, backend.platform, &BackendOptions::default()).unwrap(); + std::fs::create_dir_all(backend.paths.downloads()).unwrap(); + std::fs::write(&dest, vec![0u8; size as usize]).unwrap(); + dest + } + + #[test] + fn a_kept_download_is_reported_and_can_be_thrown_away() { + let Some(platform) = native_platform() else { return }; + let dir = tempfile::tempdir().unwrap(); + let mut backend = backend(dir.path(), platform, None); + assert_eq!(ready(&mut backend), Some(false)); + let dest = keep_file(&backend); + assert_eq!(ready(&mut backend), Some(true)); + + backend.send(Command::Cancel { app: APP.to_string() }); + assert!(!dest.exists()); + assert_eq!(last_ready(backend.poll()), Some(false)); + } + + #[test] + fn changing_the_settings_reports_kept_downloads_again() { + let Some(platform) = native_platform() else { return }; + let dir = tempfile::tempdir().unwrap(); + let mut backend = backend(dir.path(), platform, None); + keep_file(&backend); + assert_eq!(ready(&mut backend), Some(true)); + // The planned file changed under the UI; only a new report can tell it. + backend.shared.latest.lock().unwrap().insert(APP.to_string(), release("0.6.0")); + backend.send(Command::Configure(BackendOptions::default())); + assert_eq!(last_ready(backend.poll()), Some(false)); + } + + #[test] + fn a_check_that_finds_a_newer_release_ends_the_retry_offer() { + struct Newer; + impl ReleaseSource for Newer { + fn latest(&self, repo: &str, _prereleases: bool) -> crate::source::Answer { + let found = CraftApp::by_id(APP).is_some_and(|a| a.repo == repo); + let latest = if found { LatestRelease::Found(release("0.6.0")) } else { LatestRelease::None }; + crate::source::Answer { latest, source: Source::GitHubApi } + } + fn cached(&self, _repo: &str, _prereleases: bool) -> Option { + None + } + fn rate_limited_until(&self) -> Option { + None + } + fn save(&self) {} + } + let Some(platform) = native_platform() else { return }; + let dir = tempfile::tempdir().unwrap(); + let mut backend = backend(dir.path(), platform, Some(Arc::new(Newer))); + // A check needs a client; with pre-releases on it asks only `Newer`, never ArtCraft's API. + backend.http = Some(Http::new().unwrap()); + backend.shared.options.lock().unwrap().prereleases = true; + keep_file(&backend); + assert_eq!(ready(&mut backend), Some(true)); + + backend.send(Command::CheckForUpdates { force: true }); + let mut events = Vec::new(); + let deadline = Instant::now() + Duration::from_secs(20); + while !events.iter().any(|e| matches!(e, Event::CheckFinished { .. })) { + assert!(Instant::now() < deadline, "the check never finished"); + std::thread::sleep(Duration::from_millis(10)); + events.extend(backend.poll()); + } + assert_eq!(last_ready(events), Some(false)); + } } diff --git a/crates/native/src/http.rs b/crates/native/src/http.rs index b5a75e9..29faba5 100644 --- a/crates/native/src/http.rs +++ b/crates/native/src/http.rs @@ -186,8 +186,9 @@ impl Http { std::fs::create_dir_all(dir).map_err(|e| format!("couldn't create {} ({e})", dir.display()))?; } let verified = if sha256.is_some() { Verified::Checksum } else { Verified::SizeOnly }; - // A finished copy from an earlier attempt that never got installed. - if dest.is_file() && matches_published(dest, size, sha256) { + // A finished copy from an earlier attempt that never got installed. Marking it as just + // used, before checking it, keeps `clean_downloads` away from it while it's installed. + if dest.is_file() && touch(dest) && matches_published(dest, size, sha256) { progress(Transfer::Bytes { done: size, total: size }); return Ok(verified); } @@ -402,10 +403,22 @@ pub fn partial_bytes(dest: &Path, size: u64) -> Option { std::fs::metadata(part_path(dest, size)).ok().map(|m| m.len()).filter(|n| *n > 0) } -/// Removes leftovers from the download folder: files in an old or unknown naming scheme, and -/// paused downloads untouched for `max_age` (they're for releases long since replaced). Files -/// written within the last minute are always kept, so a running download is never touched. -pub fn clean_downloads(dir: &Path, max_age: Duration) { +/// Sets a file's modification time to now. +fn touch(path: &Path) -> bool { + std::fs::File::options().write(true).open(path).and_then(|f| f.set_modified(std::time::SystemTime::now())).is_ok() +} + +/// Whether a finished download is waiting at `dest`. Judged by size alone: `download` checks it +/// against the published checksum before anything uses it. +pub fn finished(dest: &Path, size: u64) -> bool { + std::fs::metadata(dest).is_ok_and(|m| m.is_file() && m.len() == size) +} + +/// Removes leftovers from the download folder: partial files in an old naming scheme, paused +/// downloads untouched for `partial_max_age`, and finished ones (kept after a failed install) +/// untouched for `finished_max_age` (they're for releases long since replaced, or given up on). +/// Files written within the last minute are always kept, so a running download is never touched. +pub fn clean_downloads(dir: &Path, partial_max_age: Duration, finished_max_age: Duration) { let Ok(entries) = std::fs::read_dir(dir) else { return }; let now = std::time::SystemTime::now(); for entry in entries.flatten() { @@ -424,16 +437,19 @@ pub fn clean_downloads(dir: &Path, max_age: Duration) { .strip_suffix(".part") .and_then(|stem| stem.rsplit_once('.')) .is_some_and(|(_, size)| !size.is_empty() && size.bytes().all(|b| b.is_ascii_digit())); - if !current_part || age > max_age { + let legacy_part = name.ends_with(".part") && !current_part; + let max_age = if name.ends_with(".part") { partial_max_age } else { finished_max_age }; + if legacy_part || age > max_age { log::info!("removing stale download {}", path.display()); let _ = std::fs::remove_file(&path); } } } -/// Throws away a paused download. -pub fn discard_partial(dest: &Path, size: u64) { +/// Throws away a paused download, and a finished one kept for another try at installing. +pub fn discard(dest: &Path, size: u64) { let _ = std::fs::remove_file(part_path(dest, size)); + let _ = std::fs::remove_file(dest); } enum AttemptError { @@ -485,9 +501,12 @@ fn host(url: &str) -> &str { } #[cfg(test)] -mod tests { +pub(crate) mod tests { use super::*; + const DAYS_30: Duration = Duration::from_secs(30 * 86_400); + const DAYS_7: Duration = Duration::from_secs(7 * 86_400); + #[test] fn cleaning_keeps_current_partial_files_and_removes_leftovers() { let dir = tempfile::tempdir().unwrap(); @@ -503,13 +522,61 @@ mod tests { let finished = make("photocraft-0.5.0-macos-universal.dmg"); let fresh = dir.path().join("x.part"); std::fs::write(&fresh, b"x").unwrap(); - clean_downloads(dir.path(), Duration::from_secs(30 * 86_400)); - assert!(keep.exists() && fresh.exists()); - assert!(!legacy.exists() && !finished.exists()); - clean_downloads(dir.path(), Duration::from_secs(60)); + clean_downloads(dir.path(), DAYS_30, DAYS_7); + assert!(keep.exists() && fresh.exists() && finished.exists()); + assert!(!legacy.exists()); + // A finished file goes sooner than a paused one. + clean_downloads(dir.path(), DAYS_30, Duration::from_secs(60)); + assert!(keep.exists() && !finished.exists()); + clean_downloads(dir.path(), Duration::from_secs(60), DAYS_7); assert!(!keep.exists()); } + #[test] + fn a_kept_file_that_still_matches_is_not_fetched_again() { + let (body, sha) = body(); + let len = body.len() as u64; + let (url, requests) = serve(body.clone(), vec![Reply::Full, Reply::Full]); + let dir = tempfile::tempdir().unwrap(); + let dest = dir.path().join("file.bin"); + let http = Http::new().unwrap(); + http.download(&url, &dest, len, Some(&sha), &Stop::default(), &mut |_| {}).unwrap(); + assert!(finished(&dest, len) && !finished(&dest, 0)); + // Old enough to be cleaned up, were it not about to be used. + let old = std::time::SystemTime::now() - Duration::from_secs(40 * 86_400); + std::fs::File::options().write(true).open(&dest).unwrap().set_modified(old).unwrap(); + let mut last = (0, 0); + let again = http.download(&url, &dest, len, Some(&sha), &Stop::default(), &mut |t| { + if let Transfer::Bytes { done, total } = t { + last = (done, total); + } + }); + assert_eq!(again, Ok(Verified::Checksum)); + assert_eq!(last, (len, len)); + assert_eq!(requests.lock().unwrap().len(), 1); + // A cleanup while the installer runs leaves the reused file alone. + clean_downloads(dir.path(), DAYS_30, DAYS_7); + assert_eq!(std::fs::read(&dest).unwrap(), body); + discard(&dest, len); + assert!(!finished(&dest, len)); + } + + #[test] + fn a_kept_file_that_no_longer_matches_is_fetched_again() { + let (body, sha) = body(); + let len = body.len() as u64; + let (url, requests) = serve(body.clone(), vec![Reply::Full, Reply::Full]); + let dir = tempfile::tempdir().unwrap(); + let dest = dir.path().join("file.bin"); + let http = Http::new().unwrap(); + http.download(&url, &dest, len, Some(&sha), &Stop::default(), &mut |_| {}).unwrap(); + // The same size, so only the checksum can tell. + std::fs::write(&dest, vec![b'x'; body.len()]).unwrap(); + http.download(&url, &dest, len, Some(&sha), &Stop::default(), &mut |_| {}).unwrap(); + assert_eq!(std::fs::read(&dest).unwrap(), body); + assert_eq!(requests.lock().unwrap().len(), 2); + } + #[test] fn reads_content_ranges() { assert_eq!(parse_content_range("bytes 100-199/1000"), Some((100, Some(1000)))); @@ -524,7 +591,7 @@ mod tests { /// How the fake server answers one request. #[derive(Clone, Copy, Debug)] - enum Reply { + pub(crate) enum Reply { /// The whole file. Full, /// Promise the whole file, send `n` bytes, hang up. @@ -539,7 +606,7 @@ mod tests { /// Serves `body` over plain HTTP following `script`, one reply per connection; records the /// `Range` header each request carried. - fn serve(body: Vec, script: Vec) -> (String, Arc>>>) { + pub(crate) fn serve(body: Vec, script: Vec) -> (String, Arc>>>) { let listener = TcpListener::bind("127.0.0.1:0").unwrap(); let url = format!("http://{}/file.bin", listener.local_addr().unwrap()); let ranges = Arc::new(Mutex::new(Vec::new())); @@ -585,7 +652,7 @@ mod tests { (url, ranges) } - fn body() -> (Vec, String) { + pub(crate) fn body() -> (Vec, String) { let body: Vec = (0..700_000u32).map(|i| (i % 251) as u8).collect(); let sha = hex(&Sha256::digest(&body)); (body, sha) diff --git a/crates/native/src/install.rs b/crates/native/src/install.rs index f82f154..1e10e7c 100644 --- a/crates/native/src/install.rs +++ b/crates/native/src/install.rs @@ -14,7 +14,7 @@ use std::path::{Path, PathBuf}; -use artcraft_launcher_core::protocol::JobPhase; +use artcraft_launcher_core::protocol::{CANCELLED, INSTALL_CANCELLED, JobPhase}; use artcraft_launcher_core::release::{self, Asset, Release}; use artcraft_launcher_core::{CraftApp, InstallSource, Installed, PackageKind}; @@ -67,9 +67,18 @@ pub fn install( PackageKind::TarGz => freebsd::install_tree(app, &file, job.paths).map(|(bin, files, root)| Some(record(&version, kind, bin, files, root))), other => Err(format!("{} isn't supported on this system", other.label())), }; - // The download has served its purpose either way; a failed install starts fresh next time. - let _ = std::fs::remove_file(&file); - result.map(|r| (r, verified)) + if !keep_download(result.is_ok(), verified) { + let _ = std::fs::remove_file(&file); + } + // The system's own prompt was dismissed: the download itself wasn't cancelled. + result.map(|r| (r, verified)).map_err(|e| if e == CANCELLED { INSTALL_CANCELLED.to_string() } else { e }) +} + +/// Whether the download stays on disk once the install step has ended. A failed install keeps it: +/// the next try checks it against its published checksum again and skips the transfer. Without a +/// checksum there's nothing to check it against later, so it goes. +fn keep_download(installed: bool, verified: Verified) -> bool { + !installed && verified == Verified::Checksum } /// Where `asset` is downloaded to (and its partial file kept while unfinished). @@ -710,6 +719,51 @@ mod tests { assert_eq!(std::fs::read_to_string(dest.join("v")).unwrap(), "new"); } + #[test] + fn only_a_failed_install_of_a_checksummed_download_keeps_it() { + for (installed, verified, keep) in [ + (false, Verified::Checksum, true), + (false, Verified::SizeOnly, false), + (true, Verified::Checksum, false), + (true, Verified::SizeOnly, false), + ] { + assert_eq!(keep_download(installed, verified), keep, "installed: {installed}, {verified:?}"); + } + } + + #[test] + fn a_failed_install_keeps_the_download_for_the_next_try() { + use crate::http::tests::{Reply, body, serve}; + let app = CraftApp::by_id("wordcraft").unwrap(); + let (body, sha) = body(); + let (url, requests) = serve(body.clone(), vec![Reply::Full, Reply::Full]); + let asset = Asset { name: "wordcraft-0.3.0-test.pkg".into(), size: body.len() as u64, url, sha256: Some(sha) }; + let release = Release { assets: vec![asset.clone()], ..Release::default() }; + let dir = tempfile::tempdir().unwrap(); + let paths = Paths::under(dir.path()); + let http = Http::new().unwrap(); + // A format no build of the launcher installs here stands in for any failing install step. + let kind = if cfg!(windows) { PackageKind::Dmg } else { PackageKind::Msi }; + let mut phases = Vec::new(); + for _ in 0..2 { + let mut phase = |p: JobPhase| phases.push(p); + let mut job = Job { http: &http, paths: &paths, stop: &Stop::default(), phase: &mut phase }; + let err = install(app, &release, &asset, kind, None, &mut job).unwrap_err(); + assert!(err.contains("isn't supported"), "{err}"); + assert_eq!(std::fs::read(download_path(&paths, &asset)).unwrap(), body); + } + assert_eq!(requests.lock().unwrap().len(), 1, "the second try reuses the kept file"); + assert_eq!(phases.iter().filter(|p| **p == JobPhase::Installing).count(), 2); + + // Checked by size only: not kept. + let unchecked = Asset { name: "wordcraft-0.3.1-test.pkg".into(), sha256: None, ..asset }; + let mut phase = |_: JobPhase| {}; + let mut job = Job { http: &http, paths: &paths, stop: &Stop::default(), phase: &mut phase }; + install(app, &release, &unchecked, kind, None, &mut job).unwrap_err(); + assert!(!download_path(&paths, &unchecked).exists()); + assert_eq!(requests.lock().unwrap().len(), 2); + } + #[cfg(unix)] fn tarball(path: &Path, entries: &[&str]) { let f = std::fs::File::create(path).unwrap(); diff --git a/crates/ui-egui/src/app.rs b/crates/ui-egui/src/app.rs index 451841f..146d6db 100644 --- a/crates/ui-egui/src/app.rs +++ b/crates/ui-egui/src/app.rs @@ -2,7 +2,9 @@ use std::time::Duration; -use artcraft_launcher_core::protocol::{Backend, BackendOptions, CANCELLED, Capabilities, Command, Event, LatestRelease, PAUSED, Source}; +use artcraft_launcher_core::protocol::{ + Backend, BackendOptions, CANCELLED, Capabilities, Command, Event, INSTALL_CANCELLED, LatestRelease, PAUSED, Source, +}; use artcraft_launcher_core::release::Release; use artcraft_launcher_core::status::AppStatus; use artcraft_launcher_core::{Platform, SUITE}; @@ -267,6 +269,12 @@ impl LauncherUi { } false } + Event::ReadyToInstall { app, ready } => { + if let Some(e) = self.entry_mut(&app) { + e.ready = ready; + } + false + } Event::JobFinished { app, result } => { if app == artcraft_launcher_core::catalog::LAUNCHER_ID { self.launcher_job = None; @@ -274,11 +282,16 @@ impl LauncherUi { if let Some(e) = self.entry_mut(&app) { e.job = None; } + let kept = self.entry(&app).is_some_and(|e| e.ready); match result { Ok(msg) => self.toast(ctx, msg, false), // The paused card says it all. Err(e) if e == PAUSED => {} Err(e) if e == CANCELLED => self.toast(ctx, "Download cancelled", false), + Err(e) if e == INSTALL_CANCELLED && kept => { + self.toast(ctx, "Install cancelled. The download is kept for the next try.", false); + } + Err(e) if e == INSTALL_CANCELLED => self.toast(ctx, "Install cancelled", false), Err(e) => self.toast(ctx, e, true), } true diff --git a/crates/ui-egui/src/model.rs b/crates/ui-egui/src/model.rs index 24e84e9..1a8a34c 100644 --- a/crates/ui-egui/src/model.rs +++ b/crates/ui-egui/src/model.rs @@ -18,11 +18,13 @@ pub struct AppEntry { pub origin: Option, /// A download waiting to be resumed. pub paused: Option, + /// A finished download waiting for another try at installing it. + pub ready: bool, } impl AppEntry { pub fn new(app: &'static CraftApp) -> Self { - Self { app, latest: None, installed: None, detected: false, job: None, origin: None, paused: None } + Self { app, latest: None, installed: None, detected: false, job: None, origin: None, paused: None, ready: false } } pub fn release(&self) -> Option<&Release> { diff --git a/crates/ui-egui/src/views.rs b/crates/ui-egui/src/views.rs index cb4a9a8..59371e7 100644 --- a/crates/ui-egui/src/views.rs +++ b/crates/ui-egui/src/views.rs @@ -16,6 +16,8 @@ use crate::widgets::{self, Cell, Kind}; /// What the main button of an app does. enum Primary { Install, + /// The download is already here; only the install step is left. Carries the button's label. + Retry(String), Update(String), Open, Download(String), @@ -582,6 +584,8 @@ impl LauncherUi { }; } match status { + AppStatus::Available if entry.ready => Primary::Retry("Retry install".into()), + AppStatus::UpdateAvailable if entry.ready => Primary::Retry(format!("Retry update to {}", entry.latest_label().unwrap_or_default())), AppStatus::Available => Primary::Install, AppStatus::UpdateAvailable => Primary::Update(entry.latest_label().unwrap_or_default()), AppStatus::UpToDate | AppStatus::Installed => Primary::Open, @@ -674,6 +678,12 @@ impl LauncherUi { self.send(Command::Install { app: id.clone() }); } } + Primary::Retry(label) => { + let tip = "The download is still here. It's checked again, then installed."; + if widgets::button(ui, &label, Kind::Primary, main_w).on_hover_text(tip).clicked() { + self.send(Command::Install { app: id.clone() }); + } + } Primary::Update(v) => { if widgets::button(ui, &format!("Update to {v}"), Kind::Primary, main_w).clicked() { self.send(Command::Install { app: id.clone() }); @@ -709,6 +719,14 @@ impl LauncherUi { self.dialog = Some(Dialog::Notes(id.clone())); ui.close(); } + // Only where Install would download: an up-to-date app would be reinstalled. + let can_download_again = + manage && entry.ready && entry.paused.is_none() && matches!(status, AppStatus::Available | AppStatus::UpdateAvailable); + if can_download_again && ui.button("Download again").clicked() { + self.send(Command::Cancel { app: id.clone() }); + self.send(Command::Install { app: id.clone() }); + ui.close(); + } if manage && status.is_installed() { if ui.button("Open").clicked() { self.send(Command::Launch { app: id.clone() }); diff --git a/crates/ui-egui/tests/ui.rs b/crates/ui-egui/tests/ui.rs index ba72731..3c81746 100644 --- a/crates/ui-egui/tests/ui.rs +++ b/crates/ui-egui/tests/ui.rs @@ -6,7 +6,9 @@ use std::cell::{Cell, RefCell}; use std::rc::Rc; -use artcraft_launcher_core::protocol::{Backend, Capabilities, Command, Event, JobPhase, LatestRelease, Origin, PausedDownload, Source}; +use artcraft_launcher_core::protocol::{ + Backend, CANCELLED, Capabilities, Command, Event, INSTALL_CANCELLED, JobPhase, LatestRelease, Origin, PausedDownload, Source, +}; use artcraft_launcher_core::release::{Asset, Release}; use artcraft_launcher_core::status::InstallSource; use artcraft_launcher_core::{Arch, Installed, Os, Platform, SUITE, version}; @@ -101,6 +103,7 @@ fn events() -> Vec { ev.push(Event::Progress { app: "filmcraft".into(), phase: JobPhase::Downloading { done: 48 << 20, total: 131 << 20 } }); ev.push(Event::Paused { app: "deckcraft".into(), download: Some(PausedDownload { done: 30 << 20, total: 120 << 20 }) }); ev.push(Event::Progress { app: "gridcraft".into(), phase: JobPhase::Reconnecting { done: 10 << 20, total: 90 << 20 } }); + ev.push(Event::ReadyToInstall { app: "cadcraft".into(), ready: true }); ev.push(Event::CheckFinished { at: NOW - 180, source: Source::GitHubApi, rate_limited_until: None }); ev } @@ -110,9 +113,19 @@ fn harness(view: ViewMode, theme: egui::Theme, manage: bool) -> (Harness<'static } fn harness_at(view: ViewMode, theme: egui::Theme, manage: bool, scale: f32) -> (Harness<'static, Option>, Rc>>) { + harness_with(events(), view, theme, manage, scale) +} + +fn harness_with( + events: Vec, + view: ViewMode, + theme: egui::Theme, + manage: bool, + scale: f32, +) -> (Harness<'static, Option>, Rc>>) { let sent = Rc::new(RefCell::new(Vec::new())); let sent2 = sent.clone(); - let mut backend = Some(Fake { events: events(), sent: sent2, manage, clock: Rc::new(Cell::new(NOW)) }); + let mut backend = Some(Fake { events, sent: sent2, manage, clock: Rc::new(Cell::new(NOW)) }); let settings = Settings { view, notify: false, ..Settings::default() }; // No `.wgpu()`: it gives every test its own GPU device, and those segfault on parallel test threads (see above). // The default renderer creates one only when a test renders. @@ -222,6 +235,76 @@ fn downloads_can_be_paused_resumed_and_discarded() { assert!(sent.borrow().iter().any(|c| matches!(c, Command::Pause { .. }))); } +#[test] +fn a_kept_download_offers_retry_install_and_download_again() { + let (mut h, sent) = harness(ViewMode::Grid, egui::Theme::Dark, true); + h.run_steps(4); + if let Some(l) = h.state_mut().as_mut() { + l.settings.filter = artcraft_launcher_ui_egui::settings::Filter::NotInstalled; + } + h.run_steps(6); + // Below the fold in the restyled grid: bring the card on screen first. + h.get_by_label("Retry install").scroll_to_me(); + h.run_steps(4); + snapshot(&mut h, "grid-retry-install-dark"); + sent.borrow_mut().clear(); + let retry = h.get_by_label("Retry install"); + let row = retry.rect(); + retry.click(); + h.run_steps(2); + assert_eq!(*sent.borrow(), [Command::Install { app: "cadcraft".into() }]); + sent.borrow_mut().clear(); + // The … menu beside it. + let beside = |r: egui::Rect| row.y_range().contains(r.center().y) && (row.right()..row.right() + 60.0).contains(&r.left()); + h.get_all_by_label("…").find(|n| beside(n.rect())).unwrap().click(); + h.run_steps(3); + snapshot(&mut h, "grid-retry-install-menu-dark"); + h.get_by_label("Download again").click(); + h.run_steps(2); + assert_eq!(*sent.borrow(), [Command::Cancel { app: "cadcraft".into() }, Command::Install { app: "cadcraft".into() }]); + // Apps without a kept download don't offer it. + assert_eq!(h.query_all_by_label("Retry install").count(), 1); +} + +#[test] +fn a_dismissed_install_prompt_is_not_called_a_cancelled_download() { + let finished = |app: &str, why: &str| Event::JobFinished { app: app.into(), result: Err(why.into()) }; + // The password prompt was dismissed with the download kept (cadcraft) and without (pdfcraft); + // a download was cancelled (vectorcraft). + let mut ev = events(); + ev.push(finished("cadcraft", INSTALL_CANCELLED)); + ev.push(finished("pdfcraft", INSTALL_CANCELLED)); + ev.push(finished("vectorcraft", CANCELLED)); + let (mut h, _) = harness_with(ev, ViewMode::Grid, egui::Theme::Dark, true, 1.0); + h.run_steps(6); + snapshot(&mut h, "grid-install-cancelled-dark"); + for toast in ["Install cancelled. The download is kept for the next try.", "Install cancelled", "Download cancelled"] { + assert_eq!(h.query_all_by_label(toast).count(), 1, "{toast}"); + } +} + +#[test] +fn a_kept_update_names_its_version_and_an_up_to_date_app_is_not_downloaded_again() { + let mut ev = events(); + for app in ["effectcraft", "wordcraft"] { + ev.push(Event::ReadyToInstall { app: app.into(), ready: true }); + } + let (mut h, sent) = harness_with(ev, ViewMode::Table, egui::Theme::Dark, true, 1.0); + h.run_steps(8); + snapshot(&mut h, "table-retry-update-dark"); + sent.borrow_mut().clear(); + assert!(h.query_by_label("Update to 0.6.0").is_none()); + h.get_by_label("Retry update to 0.6.0").click(); + h.run_steps(2); + assert_eq!(*sent.borrow(), [Command::Install { app: "effectcraft".into() }]); + // WordCraft is up to date: its menu offers Reinstall, which uses the kept file. + let row = h.get_all_by_label("WordCraft").next().unwrap().rect().center().y; + h.get_all_by_label("…").find(|n| (n.rect().center().y - row).abs() < 25.0).unwrap().click(); + h.run_steps(3); + h.get_by_label("Reinstall"); + assert!(h.query_by_label("Download again").is_none()); +} + #[test] fn table_view_and_light_theme() { let (mut h, sent) = harness(ViewMode::Table, egui::Theme::Light, true);