Skip to content

Commit

Permalink
[commtest] Move backup client to shared
Browse files Browse the repository at this point in the history
Summary:
[ENG-5555](https://linear.app/comm/issue/ENG-5555) and [ENG-5554](https://linear.app/comm/issue/ENG-5554)

Move the backup client to shared so that it can be used on native. There are a few changes that I will annotate in inline comments. The total amount of moved logic is pretty small but if it's hard to review, let me know and I will split this into more diffs.

Test Plan:
- `cargo run` in backup and blob
- `yarn run-integration-tests backup`
- `yarn run-performance-tests blob`

Reviewers: bartek, varun, kamil

Reviewed By: bartek, varun

Subscribers: ashoat, tomek

Differential Revision: https://phab.comm.dev/D10260
  • Loading branch information
MichalGniadek committed Dec 19, 2023
1 parent a05bedf commit 1cadde3
Show file tree
Hide file tree
Showing 14 changed files with 2,325 additions and 236 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ shared/protos/build

# Shared libraries
shared/tunnelbroker-client/target
shared/backup_client/target

# Nix
result*
Expand Down
15 changes: 15 additions & 0 deletions services/commtest/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 services/commtest/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ license = "BSD-3-Clause"
[dependencies]
comm-opaque2 = { path = "../../shared/comm-opaque2" }
grpc_clients = { path = "../../shared/grpc_clients" }
backup_client = { path = "../../shared/backup_client" }
tonic = "0.8"
tokio = { version = "1.24", features = ["macros", "rt-multi-thread"] }
prost = "0.11"
Expand Down
9 changes: 0 additions & 9 deletions services/commtest/src/backup/backup_utils.rs

This file was deleted.

60 changes: 0 additions & 60 deletions services/commtest/src/backup/create_new_backup.rs

This file was deleted.

3 changes: 0 additions & 3 deletions services/commtest/src/backup/mod.rs

This file was deleted.

80 changes: 0 additions & 80 deletions services/commtest/src/backup/pull_backup.rs

This file was deleted.

1 change: 0 additions & 1 deletion services/commtest/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
pub mod backup;
pub mod blob;
pub mod constants;
pub mod identity;
Expand Down
2 changes: 2 additions & 0 deletions services/commtest/src/tools.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ pub enum Error {
JsonError(serde_json::error::Error),
#[display(...)]
FromUtf8Error(std::string::FromUtf8Error),
#[display(...)]
BackupClientError(backup_client::Error),
}

pub fn obtain_number_of_threads() -> usize {
Expand Down
81 changes: 35 additions & 46 deletions services/commtest/tests/backup_integration_test.rs
Original file line number Diff line number Diff line change
@@ -1,30 +1,26 @@
use backup_client::{
BackupClient, BackupData, BackupDescriptor, Error as BackupClientError,
RequestedData,
};
use bytesize::ByteSize;
use comm_lib::{auth::UserIdentity, backup::LatestBackupIDResponse};
use commtest::{
backup::{
backup_utils::BackupData,
create_new_backup,
pull_backup::{self, BackupDescriptor, RequestedData},
},
service_addr,
tools::{generate_stable_nbytes, Error},
};
use reqwest::StatusCode;

#[tokio::test]
async fn backup_integration_test() -> Result<(), Error> {
let url = reqwest::Url::try_from(service_addr::BACKUP_SERVICE_HTTP)
.expect("failed to parse backup service url");
let backup_client = BackupClient::new(service_addr::BACKUP_SERVICE_HTTP)?;

let backup_datas = [
BackupData {
backup_id: "b1".to_string(),
user_keys_hash: "kh1".to_string(),
user_keys: generate_stable_nbytes(
ByteSize::kib(4).as_u64() as usize,
Some(b'a'),
),
user_data_hash: "dh1".to_string(),
user_data: generate_stable_nbytes(
ByteSize::mib(4).as_u64() as usize,
Some(b'A'),
Expand All @@ -33,12 +29,10 @@ async fn backup_integration_test() -> Result<(), Error> {
},
BackupData {
backup_id: "b2".to_string(),
user_keys_hash: "kh2".to_string(),
user_keys: generate_stable_nbytes(
ByteSize::kib(4).as_u64() as usize,
Some(b'b'),
),
user_data_hash: "dh2".to_string(),
user_data: generate_stable_nbytes(
ByteSize::mib(4).as_u64() as usize,
Some(b'B'),
Expand All @@ -53,29 +47,27 @@ async fn backup_integration_test() -> Result<(), Error> {
device_id: "dummy device_id".to_string(),
};

create_new_backup::run(url.clone(), &user_identity, &backup_datas[0]).await?;
create_new_backup::run(url.clone(), &user_identity, &backup_datas[1]).await?;
backup_client
.upload_backup(&user_identity, backup_datas[0].clone())
.await?;
backup_client
.upload_backup(&user_identity, backup_datas[1].clone())
.await?;

// Test direct lookup
let second_backup_descriptor = BackupDescriptor::BackupID {
backup_id: backup_datas[1].backup_id.clone(),
user_identity: user_identity.clone(),
};

let user_keys = pull_backup::run(
url.clone(),
second_backup_descriptor.clone(),
RequestedData::UserKeys,
)
.await?;
let user_keys = backup_client
.download_backup_data(&second_backup_descriptor, RequestedData::UserKeys)
.await?;
assert_eq!(user_keys, backup_datas[1].user_keys);

let user_data = pull_backup::run(
url.clone(),
second_backup_descriptor.clone(),
RequestedData::UserData,
)
.await?;
let user_data = backup_client
.download_backup_data(&second_backup_descriptor, RequestedData::UserData)
.await?;
assert_eq!(user_data, backup_datas[1].user_data);

// Test latest backup lookup
Expand All @@ -84,22 +76,16 @@ async fn backup_integration_test() -> Result<(), Error> {
username: "1".to_string(),
};

let backup_id_response = pull_backup::run(
url.clone(),
latest_backup_descriptor.clone(),
RequestedData::BackupID,
)
.await?;
let backup_id_response = backup_client
.download_backup_data(&latest_backup_descriptor, RequestedData::BackupID)
.await?;
let response: LatestBackupIDResponse =
serde_json::from_slice(&backup_id_response)?;
assert_eq!(response.backup_id, backup_datas[1].backup_id);

let user_keys = pull_backup::run(
url.clone(),
latest_backup_descriptor.clone(),
RequestedData::UserKeys,
)
.await?;
let user_keys = backup_client
.download_backup_data(&latest_backup_descriptor, RequestedData::UserKeys)
.await?;
assert_eq!(user_keys, backup_datas[1].user_keys);

// Test cleanup
Expand All @@ -108,15 +94,18 @@ async fn backup_integration_test() -> Result<(), Error> {
user_identity: user_identity.clone(),
};

let response = pull_backup::run(
url.clone(),
first_backup_descriptor.clone(),
RequestedData::UserKeys,
)
.await;
assert!(
matches!(response, Err(Error::HttpStatus(StatusCode::NOT_FOUND))),
"First backup should have been removed, instead got response: {response:?}"
let response = backup_client
.download_backup_data(&first_backup_descriptor, RequestedData::UserKeys)
.await;

let Err(BackupClientError::ReqwestError(error)) = response else {
panic!("First backup should have been removed, instead got response: {response:?}");
};

assert_eq!(
error.status(),
Some(StatusCode::NOT_FOUND),
"Expected status 'not found'"
);

Ok(())
Expand Down
Loading

0 comments on commit 1cadde3

Please sign in to comment.