Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CLI] Remove device data alongside the device #8605

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ testenv = []

[dependencies]
libparsec = { workspace = true }
libparsec_client = { workspace = true }
libparsec_client_connection = { workspace = true }
libparsec_platform_ipc = { workspace = true }

Expand Down
12 changes: 10 additions & 2 deletions cli/src/commands/device/remove.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Parsec Cloud (https://parsec.cloud) Copyright (c) BUSL-1.1 2016-present Scille SAS

use libparsec::AvailableDevice;
use libparsec_client::remove_device;

use crate::utils::*;

Expand All @@ -17,7 +18,14 @@ pub async fn main(args: Args) -> anyhow::Result<()> {
config_dir.display(),
);

let device = load_device_file(&config_dir, device).await?;
// FIXME: https://github.com/Scille/parsec-cloud/issues/8591
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get why the device deletion is related to the config

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I use the wrong link :x

Should be:

Suggested change
// FIXME: https://github.com/Scille/parsec-cloud/issues/8591
// FIXME: https://github.com/Scille/parsec-cloud/issues/8604

We need the configuration for deleting the data associated to the device (we need the value of data_base_dir).

Currently both the client and CLI use default value

// The client config should be loaded from a config file
let config = libparsec_client::ClientConfig::from(libparsec::ClientConfig {
config_dir,
..Default::default()
});

let device = load_device_file(&config.config_dir, device).await?;

let short_id = &device.device_id.hex()[..3];
let AvailableDevice {
Expand All @@ -36,7 +44,7 @@ pub async fn main(args: Args) -> anyhow::Result<()> {

match input.trim() {
"y" => {
std::fs::remove_file(&device.key_file_path)?;
remove_device(&config, &device).await?;
println!("The device has been removed");
}
_ => eprintln!("Operation cancelled"),
Expand Down
28 changes: 28 additions & 0 deletions libparsec/crates/client/src/device/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Parsec Cloud (https://parsec.cloud) Copyright (c) BUSL-1.1 2016-present Scille SAS

use libparsec_types::{anyhow, thiserror, AvailableDevice};

use crate::ClientConfig;

#[derive(Debug, thiserror::Error)]
pub enum RemoveDeviceError {
#[error("Failed to remove device: {}", .0)]
DeviceRemovalError(libparsec_platform_device_loader::RemoveDeviceError),
#[error("Failed to remove device data: {}", .0)]
DeviceDataRemovalError(anyhow::Error),
}

/// Remove device from existence.
/// That will also remove local associated data to the device (workspaces, certificates, etc).
pub async fn remove_device(
config: &ClientConfig,
device: &AvailableDevice,
) -> Result<(), RemoveDeviceError> {
libparsec_platform_device_loader::remove_device(&device.key_file_path)
.await
.map_err(RemoveDeviceError::DeviceRemovalError)?;
libparsec_platform_storage::remove_device_data(&config.data_base_dir, &device.device_id)
.await
.map_err(RemoveDeviceError::DeviceDataRemovalError)?;
Ok(())
}
3 changes: 3 additions & 0 deletions libparsec/crates/client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,19 @@
mod certif;
mod client;
mod config;
mod device;
mod event_bus;
mod invite;
mod monitors;
mod user;

// Workspaces can be started & accessed independently of each other, so we expose it directly
pub mod workspace;

// For clarity, user & certificate stuff are re-exposed through client
pub use certif::*;
pub use client::*;
pub use config::*;
pub use device::remove_device;
pub use event_bus::*;
pub use invite::*;
8 changes: 8 additions & 0 deletions libparsec/crates/platform_device_loader/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,3 +256,11 @@ pub enum ArchiveDeviceError {
}

pub use platform::archive_device;

#[derive(Debug, thiserror::Error)]
pub enum RemoveDeviceError {
#[error(transparent)]
Internal(#[from] anyhow::Error),
}

pub use platform::remove_device;
8 changes: 8 additions & 0 deletions libparsec/crates/platform_device_loader/src/native/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,14 @@ pub async fn archive_device(device_path: &Path) -> Result<(), crate::ArchiveDevi
.map_err(|e| crate::ArchiveDeviceError::Internal(e.into()))
}

pub async fn remove_device(device_path: &Path) -> Result<(), crate::RemoveDeviceError> {
log::debug!("Removing device {}", device_path.display());

tokio::fs::remove_file(device_path)
.await
.map_err(|e| crate::RemoveDeviceError::Internal(e.into()))
}

/*
* Recovery
*/
Expand Down
4 changes: 4 additions & 0 deletions libparsec/crates/platform_device_loader/src/web/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ pub async fn archive_device(_device_path: &Path) -> Result<(), crate::ArchiveDev
todo!()
}

pub async fn remove_device(_device_path: &Path) -> Result<(), crate::RemoveDeviceError> {
todo!()
}

/*
* Recovery
*/
Expand Down
27 changes: 27 additions & 0 deletions libparsec/crates/platform_device_loader/tests/remove.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Parsec Cloud (https://parsec.cloud) Copyright (c) BUSL-1.1 2016-present Scille SAS

// `allow-unwrap-in-test` don't behave as expected, see:
// https://github.com/rust-lang/rust-clippy/issues/11119
#![allow(clippy::unwrap_used)]

use libparsec_platform_device_loader::{remove_device, save_device};
use libparsec_tests_fixtures::{parsec_test, tmp_path, TestbedEnv, TmpPath};
use libparsec_types::DeviceAccessStrategy;

#[parsec_test(testbed = "minimal")]
async fn remove_ok(tmp_path: TmpPath, env: &TestbedEnv) {
// 1. Save device to filesystem.
let device = env.local_device("alice@dev1");
let key_file = tmp_path.join("alice.device");
let access = DeviceAccessStrategy::Password {
key_file: key_file.clone(),
password: "FooBar".to_owned().into(),
};
save_device(&tmp_path, &access, &device).await.unwrap();

// 2. Remove the device.
remove_device(&key_file).await.unwrap();

// 3. Check that the device as been removed.
assert!(!key_file.exists(), "Device file should have been removed");
}
1 change: 1 addition & 0 deletions libparsec/crates/platform_storage/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ pub(crate) use web as platform;
pub mod certificates;
pub mod user;
pub mod workspace;
pub use platform::cleanup::remove_device_data;

// Testbed integration is tested in the `libparsec_tests_fixture` crate.
#[cfg(feature = "test-with-testbed")]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -716,5 +716,5 @@ impl PlatformCertificatesStorage {
}

#[cfg(test)]
#[path = "../../tests/unit/native_sqlite_db_creation.rs"]
#[path = "../../tests/unit/native/sqlite_db_creation.rs"]
mod test;
18 changes: 18 additions & 0 deletions libparsec/crates/platform_storage/src/native/cleanup.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Parsec Cloud (https://parsec.cloud) Copyright (c) BUSL-1.1 2016-present Scille SAS

use std::path::Path;

use libparsec_types::{anyhow, DeviceID};

pub async fn remove_device_data(data_base_dir: &Path, device_id: &DeviceID) -> anyhow::Result<()> {
let path = data_base_dir.join(device_id.hex());
log::debug!("Removing device data at {}", path.display());

tokio::fs::remove_dir_all(&path)
.await
.map_err(anyhow::Error::from)
}

#[cfg(test)]
#[path = "../../tests/unit/native/cleanup.rs"]
mod test;
1 change: 1 addition & 0 deletions libparsec/crates/platform_storage/src/native/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Parsec Cloud (https://parsec.cloud) Copyright (c) BUSL-1.1 2016-present Scille SAS

pub(crate) mod certificates;
pub(crate) mod cleanup;
mod model;
pub(crate) mod user;
pub(crate) mod workspace;
Expand Down
12 changes: 12 additions & 0 deletions libparsec/crates/platform_storage/src/web/cleanup.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Parsec Cloud (https://parsec.cloud) Copyright (c) BUSL-1.1 2016-present Scille SAS

use std::path::Path;

use libparsec_types::{anyhow, DeviceID};

pub async fn remove_device_data(
_data_base_dir: &Path,
_device_id: &DeviceID,
) -> anyhow::Result<()> {
todo!()
}
1 change: 1 addition & 0 deletions libparsec/crates/platform_storage/src/web/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Parsec Cloud (https://parsec.cloud) Copyright (c) BUSL-1.1 2016-present Scille SAS

pub mod certificates;
pub(crate) mod cleanup;
mod db;
mod model;
pub mod user;
Expand Down
34 changes: 34 additions & 0 deletions libparsec/crates/platform_storage/tests/unit/native/cleanup.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// Parsec Cloud (https://parsec.cloud) Copyright (c) BUSL-1.1 2016-present Scille SAS

use libparsec_tests_fixtures::prelude::{parsec_test, tmp_path, TestbedEnv, TmpPath};
use libparsec_types::VlobID;

use crate::{
native::model::get_workspace_storage_db_relative_path, remove_device_data,
workspace::workspace_storage_non_speculative_init,
};

#[parsec_test(testbed = "minimal")]
async fn cleanup_device_data(tmp_path: TmpPath, env: &TestbedEnv) {
let realm_id = VlobID::from_hex("aa0000000000000000000000000000ff").unwrap();
let alice = env.local_device("alice@dev1");

// 1) Create some data associated with the device.
workspace_storage_non_speculative_init(&tmp_path, &alice, realm_id)
.await
.unwrap();

// 2) Check some data are written to the filesystem.
let wksp_file = get_workspace_storage_db_relative_path(&alice, realm_id);
let wksp_path = tmp_path.join(&wksp_file);
assert!(wksp_path.exists(), "wksp file does not exist");
let device_dir = tmp_path.join(alice.device_id.hex());
assert!(device_dir.exists(), "device dir does not exist");

// 3) Cleanup the device data.
remove_device_data(&tmp_path, &alice.device_id)
.await
.unwrap();
assert!(!wksp_file.exists(), "wksp file still exists");
assert!(!device_dir.exists(), "device dir still exists");
}
1 change: 1 addition & 0 deletions newsfragments/8601.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
On the CLI when removing a device (``device remove``), it will now also remove its associated data (workspaces, certificates, etc).
Loading