feat(peer): remove downloaded game files safely
Downloaded but uninstalled games can still occupy significant disk space. Add a separate removal path for that state instead of overloading uninstall, which is reserved for deleting only `local/` installs. The peer runtime now exposes `RemoveDownloadedGame` with matching lifecycle and active-operation events. The filesystem delete is intentionally strict: the id must be a catalog game and a single path component, the target must be a direct child of the configured game directory, the root must not be a symlink, it must have a regular root-level `version.ini`, and it must not contain `local/`, `.local.installing/`, or `.local.backup/`. Only then do we recursively remove the game root. The Tauri bridge exposes this as `remove_downloaded_game`, the frontend shows a matching danger action only for downloaded-but-uninstalled games, and a confirmation dialog warns that re-downloading can take a long time. Test Plan: - git diff --check - just fmt - RUSTC_WRAPPER= CARGO_BUILD_RUSTC_WRAPPER= just test - RUSTC_WRAPPER= CARGO_BUILD_RUSTC_WRAPPER= just clippy - RUSTC_WRAPPER= CARGO_BUILD_RUSTC_WRAPPER= just build Refs: user redesign nitpick about removing downloaded uninstalled games
This commit is contained in:
@@ -128,6 +128,11 @@ Reserved per-game paths:
|
||||
- `.lanspread_owned` inside `.local.*` directories proves Lanspread ownership
|
||||
when the current intent is `None`.
|
||||
|
||||
Downloaded-file removal is not an uninstall transaction. It removes the whole
|
||||
game root only for a catalog ID that is a single direct child of the configured
|
||||
game directory, has a regular root-level `version.ini`, and has no `local/`,
|
||||
`.local.installing/`, or `.local.backup/` path.
|
||||
|
||||
Recovery reads `.lanspread.json` and combines the recorded intent with the
|
||||
observed `local/`, `.local.installing/`, and `.local.backup/` state. Intent
|
||||
states `Installing`, `Updating`, and `Uninstalling` prove ownership of the
|
||||
|
||||
@@ -84,12 +84,17 @@ When the UI asks to download a game:
|
||||
|
||||
### Install Transactions
|
||||
|
||||
Install, update, uninstall, and startup recovery live under `src/install/`.
|
||||
Install, update, uninstall, downloaded-file removal, and startup recovery live
|
||||
under `src/install/`.
|
||||
Each game root has an atomic `.lanspread.json` intent log for install-side
|
||||
operations and uses Lanspread-owned `.local.installing/` and `.local.backup/`
|
||||
directories marked by `.lanspread_owned`. Startup recovery combines the recorded
|
||||
intent with the observed filesystem state and only deletes reserved directories
|
||||
when intent or marker ownership proves they belong to Lanspread.
|
||||
Downloaded-file removal is deliberately separate from uninstall: it only accepts
|
||||
catalog IDs that are direct children of the configured game directory, refuses
|
||||
installed or in-flight roots, and deletes the whole game root only after finding
|
||||
a regular root-level `version.ini` sentinel.
|
||||
|
||||
## Integration with `lanspread-tauri-deno-ts`
|
||||
|
||||
@@ -99,8 +104,9 @@ The Tauri application embeds this crate in
|
||||
- `LanSpreadState` holds onto the peer control channel, the latest aggregated
|
||||
`GameDB`, per-game operation state, the catalog set, and the user-selected
|
||||
game directory.
|
||||
- The Tauri commands (`request_games`, `install_game`, `update_game`, and
|
||||
`update_game_directory`) translate UI actions into `PeerCommand`s. In
|
||||
- The Tauri commands (`request_games`, `install_game`, `update_game`,
|
||||
`remove_downloaded_game`, and `update_game_directory`) translate UI actions
|
||||
into `PeerCommand`s. In
|
||||
particular, `update_game_directory` validates the filesystem path before
|
||||
storing it, loads the bundled catalog on first use, kicks off the peer runtime
|
||||
on demand, and mirrors the installed/uninstalled state into the UI-facing
|
||||
|
||||
@@ -24,6 +24,8 @@ pub enum OperationKind {
|
||||
Updating,
|
||||
/// Removing an existing `local/` install.
|
||||
Uninstalling,
|
||||
/// Removing downloaded archive files for an uninstalled game.
|
||||
RemovingDownload,
|
||||
}
|
||||
|
||||
/// Main context for the peer system.
|
||||
|
||||
@@ -59,6 +59,7 @@ fn active_operation_kind(operation: OperationKind) -> ActiveOperationKind {
|
||||
OperationKind::Installing => ActiveOperationKind::Installing,
|
||||
OperationKind::Updating => ActiveOperationKind::Updating,
|
||||
OperationKind::Uninstalling => ActiveOperationKind::Uninstalling,
|
||||
OperationKind::RemovingDownload => ActiveOperationKind::RemovingDownload,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -385,6 +385,18 @@ pub async fn handle_uninstall_game_command(
|
||||
});
|
||||
}
|
||||
|
||||
pub async fn handle_remove_downloaded_game_command(
|
||||
ctx: &Ctx,
|
||||
tx_notify_ui: &UnboundedSender<PeerEvent>,
|
||||
id: String,
|
||||
) {
|
||||
let ctx = ctx.clone();
|
||||
let tx_notify_ui = tx_notify_ui.clone();
|
||||
ctx.task_tracker.clone().spawn(async move {
|
||||
run_remove_downloaded_operation(&ctx, &tx_notify_ui, id).await;
|
||||
});
|
||||
}
|
||||
|
||||
fn spawn_install_operation(ctx: &Ctx, tx_notify_ui: &UnboundedSender<PeerEvent>, id: String) {
|
||||
let ctx = ctx.clone();
|
||||
let tx_notify_ui = tx_notify_ui.clone();
|
||||
@@ -560,6 +572,59 @@ async fn run_uninstall_operation(ctx: &Ctx, tx_notify_ui: &UnboundedSender<PeerE
|
||||
}
|
||||
}
|
||||
|
||||
async fn run_remove_downloaded_operation(
|
||||
ctx: &Ctx,
|
||||
tx_notify_ui: &UnboundedSender<PeerEvent>,
|
||||
id: String,
|
||||
) {
|
||||
if !catalog_contains(ctx, &id).await {
|
||||
log::warn!("Ignoring downloaded-file removal for non-catalog game {id}");
|
||||
return;
|
||||
}
|
||||
|
||||
if !begin_operation(ctx, tx_notify_ui, &id, OperationKind::RemovingDownload).await {
|
||||
log::warn!("Operation for {id} already in progress; ignoring downloaded-file removal");
|
||||
return;
|
||||
}
|
||||
|
||||
let game_dir = { ctx.game_dir.read().await.clone() };
|
||||
let operation_guard = OperationGuard::new(
|
||||
id.clone(),
|
||||
ctx.active_operations.clone(),
|
||||
tx_notify_ui.clone(),
|
||||
);
|
||||
let result = {
|
||||
events::send(
|
||||
tx_notify_ui,
|
||||
PeerEvent::RemoveDownloadedGameBegin { id: id.clone() },
|
||||
);
|
||||
|
||||
install::remove_downloaded(&game_dir, &id).await
|
||||
};
|
||||
end_operation(ctx, tx_notify_ui, &id).await;
|
||||
operation_guard.disarm();
|
||||
|
||||
match result {
|
||||
Ok(()) => {
|
||||
events::send(
|
||||
tx_notify_ui,
|
||||
PeerEvent::RemoveDownloadedGameFinished { id: id.clone() },
|
||||
);
|
||||
}
|
||||
Err(err) => {
|
||||
log::error!("Downloaded-file removal failed for {id}: {err}");
|
||||
events::send(
|
||||
tx_notify_ui,
|
||||
PeerEvent::RemoveDownloadedGameFailed { id: id.clone() },
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
if let Err(err) = refresh_local_game(ctx, tx_notify_ui, &id).await {
|
||||
log::error!("Failed to refresh local library after downloaded-file removal: {err}");
|
||||
}
|
||||
}
|
||||
|
||||
async fn begin_operation(
|
||||
ctx: &Ctx,
|
||||
tx_notify_ui: &UnboundedSender<PeerEvent>,
|
||||
@@ -1471,6 +1536,45 @@ mod tests {
|
||||
assert_local_update(recv_event(&mut rx).await, false, true);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn remove_downloaded_refreshes_settled_state_after_guard_release() {
|
||||
let temp = TempDir::new("lanspread-handler-remove-downloaded");
|
||||
let root = temp.game_root();
|
||||
write_file(&root.join("version.ini"), b"20250101");
|
||||
write_file(&root.join("game.eti"), b"archive");
|
||||
|
||||
let ctx = test_ctx(temp.path().to_path_buf());
|
||||
let (tx, mut rx) = mpsc::unbounded_channel();
|
||||
let catalog = ctx.catalog.read().await.clone();
|
||||
let scan = scan_local_library(temp.path(), &catalog)
|
||||
.await
|
||||
.expect("initial scan should succeed");
|
||||
update_and_announce_games(&ctx, &tx, scan).await;
|
||||
assert_local_update(recv_event(&mut rx).await, false, true);
|
||||
|
||||
run_remove_downloaded_operation(&ctx, &tx, "game".to_string()).await;
|
||||
|
||||
assert_active_update(
|
||||
recv_event(&mut rx).await,
|
||||
active_update("game", ActiveOperationKind::RemovingDownload),
|
||||
);
|
||||
assert!(matches!(
|
||||
recv_event(&mut rx).await,
|
||||
PeerEvent::RemoveDownloadedGameBegin { id } if id == "game"
|
||||
));
|
||||
assert_active_update(recv_event(&mut rx).await, Vec::new());
|
||||
assert!(matches!(
|
||||
recv_event(&mut rx).await,
|
||||
PeerEvent::RemoveDownloadedGameFinished { id } if id == "game"
|
||||
));
|
||||
let PeerEvent::LocalLibraryChanged { games } = recv_event(&mut rx).await else {
|
||||
panic!("expected LocalLibraryChanged");
|
||||
};
|
||||
assert!(games.is_empty());
|
||||
assert!(!root.exists());
|
||||
assert!(ctx.active_operations.read().await.is_empty());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn path_changing_set_game_dir_is_rejected_while_operations_are_active() {
|
||||
let current = TempDir::new("lanspread-handler-current-dir");
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
mod intent;
|
||||
mod remove;
|
||||
mod transaction;
|
||||
pub mod unpack;
|
||||
|
||||
pub use remove::remove_downloaded;
|
||||
pub use transaction::{install, recover_on_startup, uninstall, update};
|
||||
pub use unpack::{UnpackFuture, Unpacker};
|
||||
|
||||
@@ -0,0 +1,212 @@
|
||||
use std::{
|
||||
ffi::OsStr,
|
||||
io::ErrorKind,
|
||||
path::{Component, Path, PathBuf},
|
||||
};
|
||||
|
||||
use eyre::{WrapErr, bail};
|
||||
|
||||
const LOCAL_DIR: &str = "local";
|
||||
const INSTALLING_DIR: &str = ".local.installing";
|
||||
const BACKUP_DIR: &str = ".local.backup";
|
||||
const VERSION_INI: &str = "version.ini";
|
||||
|
||||
/// Remove the downloaded files for an uninstalled game root.
|
||||
///
|
||||
/// This is intentionally stricter than the scanner: callers must pass a catalog
|
||||
/// id that is a single path component, the target must be a direct child of the
|
||||
/// configured game directory, and the root must still look like a downloaded
|
||||
/// but uninstalled game immediately before recursive deletion.
|
||||
pub async fn remove_downloaded(game_dir: &Path, id: &str) -> eyre::Result<()> {
|
||||
validate_game_id(id)?;
|
||||
|
||||
let game_dir = canonical_game_dir(game_dir).await?;
|
||||
let game_root = game_dir.join(id);
|
||||
let Some(root_metadata) = symlink_metadata_if_exists(&game_root).await? else {
|
||||
return Ok(());
|
||||
};
|
||||
|
||||
if root_metadata.file_type().is_symlink() {
|
||||
bail!(
|
||||
"refusing to remove symlink game root {}",
|
||||
game_root.display()
|
||||
);
|
||||
}
|
||||
if !root_metadata.is_dir() {
|
||||
bail!(
|
||||
"refusing to remove non-directory game root {}",
|
||||
game_root.display()
|
||||
);
|
||||
}
|
||||
|
||||
let game_root = tokio::fs::canonicalize(&game_root)
|
||||
.await
|
||||
.wrap_err_with(|| format!("failed to canonicalize {}", game_root.display()))?;
|
||||
ensure_direct_child(&game_dir, &game_root, id)?;
|
||||
ensure_downloaded_uninstalled_root(&game_root).await?;
|
||||
|
||||
tokio::fs::remove_dir_all(&game_root)
|
||||
.await
|
||||
.wrap_err_with(|| format!("failed to remove downloaded game {}", game_root.display()))
|
||||
}
|
||||
|
||||
fn validate_game_id(id: &str) -> eyre::Result<()> {
|
||||
let mut components = Path::new(id).components();
|
||||
match (components.next(), components.next()) {
|
||||
(Some(Component::Normal(_)), None) => Ok(()),
|
||||
_ => bail!("refusing to remove invalid game id {id:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
async fn canonical_game_dir(game_dir: &Path) -> eyre::Result<PathBuf> {
|
||||
let game_dir = tokio::fs::canonicalize(game_dir)
|
||||
.await
|
||||
.wrap_err_with(|| format!("failed to canonicalize game dir {}", game_dir.display()))?;
|
||||
let metadata = tokio::fs::metadata(&game_dir).await?;
|
||||
if !metadata.is_dir() {
|
||||
bail!("game dir is not a directory: {}", game_dir.display());
|
||||
}
|
||||
Ok(game_dir)
|
||||
}
|
||||
|
||||
fn ensure_direct_child(game_dir: &Path, game_root: &Path, id: &str) -> eyre::Result<()> {
|
||||
if game_root == game_dir
|
||||
|| game_root.parent() != Some(game_dir)
|
||||
|| game_root.file_name() != Some(OsStr::new(id))
|
||||
{
|
||||
bail!(
|
||||
"refusing to remove game root outside direct game-dir child: {}",
|
||||
game_root.display()
|
||||
);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn ensure_downloaded_uninstalled_root(game_root: &Path) -> eyre::Result<()> {
|
||||
let version_path = game_root.join(VERSION_INI);
|
||||
let version_metadata = tokio::fs::symlink_metadata(&version_path)
|
||||
.await
|
||||
.wrap_err_with(|| format!("download sentinel is missing: {}", version_path.display()))?;
|
||||
if version_metadata.file_type().is_symlink() || !version_metadata.is_file() {
|
||||
bail!(
|
||||
"refusing to remove game without a regular version.ini sentinel: {}",
|
||||
game_root.display()
|
||||
);
|
||||
}
|
||||
|
||||
ensure_absent(&game_root.join(LOCAL_DIR), "local install").await?;
|
||||
ensure_absent(&game_root.join(INSTALLING_DIR), "install staging").await?;
|
||||
ensure_absent(&game_root.join(BACKUP_DIR), "install backup").await
|
||||
}
|
||||
|
||||
async fn ensure_absent(path: &Path, label: &str) -> eyre::Result<()> {
|
||||
if symlink_metadata_if_exists(path).await?.is_some() {
|
||||
bail!(
|
||||
"refusing to remove game root with {label}: {}",
|
||||
path.display()
|
||||
);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn symlink_metadata_if_exists(path: &Path) -> eyre::Result<Option<std::fs::Metadata>> {
|
||||
match tokio::fs::symlink_metadata(path).await {
|
||||
Ok(metadata) => Ok(Some(metadata)),
|
||||
Err(err) if err.kind() == ErrorKind::NotFound => Ok(None),
|
||||
Err(err) => Err(err.into()),
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use std::path::Path;
|
||||
|
||||
use super::*;
|
||||
use crate::test_support::TempDir;
|
||||
|
||||
fn write_file(path: &Path, bytes: &[u8]) {
|
||||
if let Some(parent) = path.parent() {
|
||||
std::fs::create_dir_all(parent).expect("parent dir should be created");
|
||||
}
|
||||
std::fs::write(path, bytes).expect("file should be written");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn remove_downloaded_deletes_only_requested_game_root() {
|
||||
let temp = TempDir::new("lanspread-remove-download");
|
||||
let root = temp.game_root();
|
||||
let sibling = temp.path().join("sibling");
|
||||
write_file(&root.join("version.ini"), b"20250101");
|
||||
write_file(&root.join("game.eti"), b"archive");
|
||||
write_file(&sibling.join("version.ini"), b"20250101");
|
||||
|
||||
remove_downloaded(temp.path(), "game")
|
||||
.await
|
||||
.expect("downloaded game root should be removed");
|
||||
|
||||
assert!(!root.exists());
|
||||
assert!(sibling.join("version.ini").is_file());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn remove_downloaded_refuses_installed_game() {
|
||||
let temp = TempDir::new("lanspread-remove-installed");
|
||||
let root = temp.game_root();
|
||||
write_file(&root.join("version.ini"), b"20250101");
|
||||
write_file(&root.join("local").join("payload.txt"), b"installed");
|
||||
|
||||
let err = remove_downloaded(temp.path(), "game")
|
||||
.await
|
||||
.expect_err("installed game must not be removed");
|
||||
|
||||
assert!(err.to_string().contains("local install"));
|
||||
assert!(root.join("version.ini").is_file());
|
||||
assert!(root.join("local").join("payload.txt").is_file());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn remove_downloaded_refuses_missing_download_sentinel() {
|
||||
let temp = TempDir::new("lanspread-remove-missing-sentinel");
|
||||
let root = temp.game_root();
|
||||
write_file(&root.join("game.eti"), b"archive");
|
||||
|
||||
let err = remove_downloaded(temp.path(), "game")
|
||||
.await
|
||||
.expect_err("undownloaded game root must not be removed");
|
||||
|
||||
assert!(err.to_string().contains("download sentinel is missing"));
|
||||
assert!(root.join("game.eti").is_file());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn remove_downloaded_rejects_path_traversal_id() {
|
||||
let temp = TempDir::new("lanspread-remove-traversal");
|
||||
let root = temp.game_root();
|
||||
write_file(&root.join("version.ini"), b"20250101");
|
||||
|
||||
let err = remove_downloaded(temp.path(), "../game")
|
||||
.await
|
||||
.expect_err("path traversal id must be rejected");
|
||||
|
||||
assert!(err.to_string().contains("invalid game id"));
|
||||
assert!(root.join("version.ini").is_file());
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[tokio::test]
|
||||
async fn remove_downloaded_refuses_symlink_game_root() {
|
||||
use std::os::unix::fs::symlink;
|
||||
|
||||
let temp = TempDir::new("lanspread-remove-symlink");
|
||||
let outside = temp.path().join("outside");
|
||||
write_file(&outside.join("version.ini"), b"20250101");
|
||||
symlink(&outside, temp.game_root()).expect("symlink should be created");
|
||||
|
||||
let err = remove_downloaded(temp.path(), "game")
|
||||
.await
|
||||
.expect_err("symlink game root must be rejected");
|
||||
|
||||
assert!(err.to_string().contains("symlink game root"));
|
||||
assert!(outside.join("version.ini").is_file());
|
||||
}
|
||||
}
|
||||
@@ -67,6 +67,7 @@ use crate::{
|
||||
handle_get_peer_count_command,
|
||||
handle_install_game_command,
|
||||
handle_list_games_command,
|
||||
handle_remove_downloaded_game_command,
|
||||
handle_set_game_dir_command,
|
||||
handle_uninstall_game_command,
|
||||
load_local_library,
|
||||
@@ -120,6 +121,12 @@ pub enum PeerEvent {
|
||||
UninstallGameFinished { id: String },
|
||||
/// Uninstall transaction has failed after rollback.
|
||||
UninstallGameFailed { id: String },
|
||||
/// Downloaded archive removal has started for an uninstalled game.
|
||||
RemoveDownloadedGameBegin { id: String },
|
||||
/// Downloaded archive removal has completed successfully.
|
||||
RemoveDownloadedGameFinished { id: String },
|
||||
/// Downloaded archive removal has failed before deleting the game root.
|
||||
RemoveDownloadedGameFailed { id: String },
|
||||
/// No peers have the requested game.
|
||||
NoPeersHaveGame { id: String },
|
||||
/// A peer has connected.
|
||||
@@ -187,6 +194,8 @@ pub enum ActiveOperationKind {
|
||||
Updating,
|
||||
/// Removing an existing `local/` install.
|
||||
Uninstalling,
|
||||
/// Removing downloaded archive files for an uninstalled game.
|
||||
RemovingDownload,
|
||||
}
|
||||
|
||||
/// Commands sent to the peer system from the UI.
|
||||
@@ -213,6 +222,8 @@ pub enum PeerCommand {
|
||||
InstallGame { id: String },
|
||||
/// Remove only the `local/` install for a game.
|
||||
UninstallGame { id: String },
|
||||
/// Remove downloaded archive files for an uninstalled game.
|
||||
RemoveDownloadedGame { id: String },
|
||||
/// Set the local game directory.
|
||||
SetGameDir(PathBuf),
|
||||
/// Request the current peer count.
|
||||
@@ -394,6 +405,9 @@ async fn handle_peer_commands(
|
||||
PeerCommand::UninstallGame { id } => {
|
||||
handle_uninstall_game_command(ctx, tx_notify_ui, id).await;
|
||||
}
|
||||
PeerCommand::RemoveDownloadedGame { id } => {
|
||||
handle_remove_downloaded_game_command(ctx, tx_notify_ui, id).await;
|
||||
}
|
||||
PeerCommand::SetGameDir(game_dir) => {
|
||||
handle_set_game_dir_command(ctx, tx_notify_ui, game_dir).await;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user