From ad0cb4c06a30ee26afe9bad274a0579e52edee63 Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Tue, 10 Dec 2024 10:40:39 +0100 Subject: [PATCH 01/13] refactor: rename `snapshot_uploaders` module to `file_uploaders` --- .../src/artifact_builder/cardano_immutable_files_full.rs | 4 ++-- mithril-aggregator/src/dependency_injection/containers.rs | 2 +- .../dumb_snapshot_uploader.rs | 0 .../local_snapshot_uploader.rs | 4 ++-- .../src/{snapshot_uploaders => file_uploaders}/mod.rs | 0 .../remote_snapshot_uploader.rs | 4 ++-- .../snapshot_uploader.rs | 0 mithril-aggregator/src/lib.rs | 8 ++++---- 8 files changed, 11 insertions(+), 11 deletions(-) rename mithril-aggregator/src/{snapshot_uploaders => file_uploaders}/dumb_snapshot_uploader.rs (100%) rename mithril-aggregator/src/{snapshot_uploaders => file_uploaders}/local_snapshot_uploader.rs (97%) rename mithril-aggregator/src/{snapshot_uploaders => file_uploaders}/mod.rs (100%) rename mithril-aggregator/src/{snapshot_uploaders => file_uploaders}/remote_snapshot_uploader.rs (97%) rename mithril-aggregator/src/{snapshot_uploaders => file_uploaders}/snapshot_uploader.rs (100%) diff --git a/mithril-aggregator/src/artifact_builder/cardano_immutable_files_full.rs b/mithril-aggregator/src/artifact_builder/cardano_immutable_files_full.rs index 293e3e8a57d..d1dd944e5a9 100644 --- a/mithril-aggregator/src/artifact_builder/cardano_immutable_files_full.rs +++ b/mithril-aggregator/src/artifact_builder/cardano_immutable_files_full.rs @@ -6,7 +6,7 @@ use std::sync::Arc; use thiserror::Error; use crate::{ - snapshot_uploaders::SnapshotLocation, snapshotter::OngoingSnapshot, SnapshotUploader, + file_uploaders::SnapshotLocation, snapshotter::OngoingSnapshot, SnapshotUploader, Snapshotter, }; @@ -174,7 +174,7 @@ mod tests { use mithril_common::{entities::CompressionAlgorithm, test_utils::fake_data}; use crate::{ - snapshot_uploaders::MockSnapshotUploader, test_tools::TestLogger, DumbSnapshotUploader, + file_uploaders::MockSnapshotUploader, test_tools::TestLogger, DumbSnapshotUploader, DumbSnapshotter, }; diff --git a/mithril-aggregator/src/dependency_injection/containers.rs b/mithril-aggregator/src/dependency_injection/containers.rs index e331d654c21..32b52532460 100644 --- a/mithril-aggregator/src/dependency_injection/containers.rs +++ b/mithril-aggregator/src/dependency_injection/containers.rs @@ -32,13 +32,13 @@ use crate::{ }, entities::AggregatorEpochSettings, event_store::{EventMessage, TransmitterService}, + file_uploaders::SnapshotUploader, multi_signer::MultiSigner, services::{ CertifierService, EpochService, MessageService, ProverService, SignedEntityService, StakeDistributionService, TransactionStore, UpkeepService, }, signer_registerer::SignerRecorder, - snapshot_uploaders::SnapshotUploader, store::CertificatePendingStorer, EpochSettingsStorer, MetricsService, SignerRegisterer, SignerRegistrationRoundOpener, SingleSignatureAuthenticator, Snapshotter, VerificationKeyStorer, diff --git a/mithril-aggregator/src/snapshot_uploaders/dumb_snapshot_uploader.rs b/mithril-aggregator/src/file_uploaders/dumb_snapshot_uploader.rs similarity index 100% rename from mithril-aggregator/src/snapshot_uploaders/dumb_snapshot_uploader.rs rename to mithril-aggregator/src/file_uploaders/dumb_snapshot_uploader.rs diff --git a/mithril-aggregator/src/snapshot_uploaders/local_snapshot_uploader.rs b/mithril-aggregator/src/file_uploaders/local_snapshot_uploader.rs similarity index 97% rename from mithril-aggregator/src/snapshot_uploaders/local_snapshot_uploader.rs rename to mithril-aggregator/src/file_uploaders/local_snapshot_uploader.rs index 461ac0dda4d..f984c571072 100644 --- a/mithril-aggregator/src/snapshot_uploaders/local_snapshot_uploader.rs +++ b/mithril-aggregator/src/file_uploaders/local_snapshot_uploader.rs @@ -6,8 +6,8 @@ use std::path::{Path, PathBuf}; use mithril_common::logging::LoggerExtensions; use mithril_common::StdResult; +use crate::file_uploaders::{SnapshotLocation, SnapshotUploader}; use crate::http_server; -use crate::snapshot_uploaders::{SnapshotLocation, SnapshotUploader}; use crate::tools; /// LocalSnapshotUploader is a snapshot uploader working using local files @@ -63,8 +63,8 @@ mod tests { use std::path::{Path, PathBuf}; use tempfile::tempdir; + use crate::file_uploaders::SnapshotUploader; use crate::http_server; - use crate::snapshot_uploaders::SnapshotUploader; use crate::test_tools::TestLogger; use super::LocalSnapshotUploader; diff --git a/mithril-aggregator/src/snapshot_uploaders/mod.rs b/mithril-aggregator/src/file_uploaders/mod.rs similarity index 100% rename from mithril-aggregator/src/snapshot_uploaders/mod.rs rename to mithril-aggregator/src/file_uploaders/mod.rs diff --git a/mithril-aggregator/src/snapshot_uploaders/remote_snapshot_uploader.rs b/mithril-aggregator/src/file_uploaders/remote_snapshot_uploader.rs similarity index 97% rename from mithril-aggregator/src/snapshot_uploaders/remote_snapshot_uploader.rs rename to mithril-aggregator/src/file_uploaders/remote_snapshot_uploader.rs index be0345bd1d3..7a061e982a7 100644 --- a/mithril-aggregator/src/snapshot_uploaders/remote_snapshot_uploader.rs +++ b/mithril-aggregator/src/file_uploaders/remote_snapshot_uploader.rs @@ -5,7 +5,7 @@ use std::path::Path; use mithril_common::logging::LoggerExtensions; use mithril_common::StdResult; -use crate::snapshot_uploaders::{SnapshotLocation, SnapshotUploader}; +use crate::file_uploaders::{SnapshotLocation, SnapshotUploader}; use crate::tools::RemoteFileUploader; /// GCPSnapshotUploader is a snapshot uploader working using Google Cloud Platform services @@ -61,7 +61,7 @@ mod tests { use anyhow::anyhow; use std::path::Path; - use crate::snapshot_uploaders::SnapshotUploader; + use crate::file_uploaders::SnapshotUploader; use crate::test_tools::TestLogger; use crate::tools::MockRemoteFileUploader; diff --git a/mithril-aggregator/src/snapshot_uploaders/snapshot_uploader.rs b/mithril-aggregator/src/file_uploaders/snapshot_uploader.rs similarity index 100% rename from mithril-aggregator/src/snapshot_uploaders/snapshot_uploader.rs rename to mithril-aggregator/src/file_uploaders/snapshot_uploader.rs diff --git a/mithril-aggregator/src/lib.rs b/mithril-aggregator/src/lib.rs index da8d6142161..6a56aff6a89 100644 --- a/mithril-aggregator/src/lib.rs +++ b/mithril-aggregator/src/lib.rs @@ -18,6 +18,7 @@ pub mod database; pub mod dependency_injection; pub mod entities; pub mod event_store; +mod file_uploaders; mod http_server; mod message_adapters; pub mod metrics; @@ -25,7 +26,6 @@ mod multi_signer; mod runtime; pub mod services; mod signer_registerer; -mod snapshot_uploaders; mod snapshotter; mod store; mod tools; @@ -38,6 +38,9 @@ pub use crate::configuration::{ pub use crate::multi_signer::{MultiSigner, MultiSignerImpl}; pub use commands::{CommandType, MainOpts}; pub use dependency_injection::DependencyContainer; +pub use file_uploaders::{ + DumbSnapshotUploader, LocalSnapshotUploader, RemoteSnapshotUploader, SnapshotUploader, +}; pub use message_adapters::{FromRegisterSignerAdapter, ToCertificatePendingMessageAdapter}; pub use metrics::*; pub use runtime::{ @@ -47,9 +50,6 @@ pub use signer_registerer::{ MithrilSignerRegisterer, SignerRecorder, SignerRegisterer, SignerRegistrationError, SignerRegistrationRound, SignerRegistrationRoundOpener, }; -pub use snapshot_uploaders::{ - DumbSnapshotUploader, LocalSnapshotUploader, RemoteSnapshotUploader, SnapshotUploader, -}; pub use snapshotter::{ CompressedArchiveSnapshotter, DumbSnapshotter, SnapshotError, Snapshotter, SnapshotterCompressionAlgorithm, From e58be848d96a229410e465832cad4aec960cdf92 Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Tue, 10 Dec 2024 10:43:39 +0100 Subject: [PATCH 02/13] refactor: rename `snapshot_uploader` module to `file_uploader` --- .../{snapshot_uploader.rs => file_uploader.rs} | 0 mithril-aggregator/src/file_uploaders/mod.rs | 8 ++++---- 2 files changed, 4 insertions(+), 4 deletions(-) rename mithril-aggregator/src/file_uploaders/{snapshot_uploader.rs => file_uploader.rs} (100%) diff --git a/mithril-aggregator/src/file_uploaders/snapshot_uploader.rs b/mithril-aggregator/src/file_uploaders/file_uploader.rs similarity index 100% rename from mithril-aggregator/src/file_uploaders/snapshot_uploader.rs rename to mithril-aggregator/src/file_uploaders/file_uploader.rs diff --git a/mithril-aggregator/src/file_uploaders/mod.rs b/mithril-aggregator/src/file_uploaders/mod.rs index 2b176a5ab05..0e760c67dc2 100644 --- a/mithril-aggregator/src/file_uploaders/mod.rs +++ b/mithril-aggregator/src/file_uploaders/mod.rs @@ -1,13 +1,13 @@ mod dumb_snapshot_uploader; +mod file_uploader; mod local_snapshot_uploader; mod remote_snapshot_uploader; -mod snapshot_uploader; pub use dumb_snapshot_uploader::*; +pub use file_uploader::SnapshotLocation; +pub use file_uploader::SnapshotUploader; pub use local_snapshot_uploader::LocalSnapshotUploader; pub use remote_snapshot_uploader::RemoteSnapshotUploader; -pub use snapshot_uploader::SnapshotLocation; -pub use snapshot_uploader::SnapshotUploader; #[cfg(test)] -pub use snapshot_uploader::MockSnapshotUploader; +pub use file_uploader::MockSnapshotUploader; From 171fb973cde64cad3ecac477e4863276fd8a427a Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Tue, 10 Dec 2024 10:52:01 +0100 Subject: [PATCH 03/13] refactor: rename `snapshot` to `file` in `file_uploader` module --- .../cardano_immutable_files_full.rs | 17 ++++++++--------- .../src/dependency_injection/builder.rs | 10 +++++----- .../src/dependency_injection/containers.rs | 4 ++-- .../file_uploaders/dumb_snapshot_uploader.rs | 8 ++++---- .../src/file_uploaders/file_uploader.rs | 10 +++++----- .../file_uploaders/local_snapshot_uploader.rs | 12 ++++++------ mithril-aggregator/src/file_uploaders/mod.rs | 6 +++--- .../file_uploaders/remote_snapshot_uploader.rs | 14 +++++++------- mithril-aggregator/src/lib.rs | 2 +- 9 files changed, 41 insertions(+), 42 deletions(-) diff --git a/mithril-aggregator/src/artifact_builder/cardano_immutable_files_full.rs b/mithril-aggregator/src/artifact_builder/cardano_immutable_files_full.rs index d1dd944e5a9..08ef75eb4e0 100644 --- a/mithril-aggregator/src/artifact_builder/cardano_immutable_files_full.rs +++ b/mithril-aggregator/src/artifact_builder/cardano_immutable_files_full.rs @@ -6,8 +6,7 @@ use std::sync::Arc; use thiserror::Error; use crate::{ - file_uploaders::SnapshotLocation, snapshotter::OngoingSnapshot, SnapshotUploader, - Snapshotter, + file_uploaders::FileLocation, snapshotter::OngoingSnapshot, FileUploader, Snapshotter, }; use super::ArtifactBuilder; @@ -33,7 +32,7 @@ pub struct CardanoImmutableFilesFullArtifactBuilder { cardano_network: CardanoNetwork, cardano_node_version: Version, snapshotter: Arc, - snapshot_uploader: Arc, + snapshot_uploader: Arc, compression_algorithm: CompressionAlgorithm, logger: Logger, } @@ -44,7 +43,7 @@ impl CardanoImmutableFilesFullArtifactBuilder { cardano_network: CardanoNetwork, cardano_node_version: &Version, snapshotter: Arc, - snapshot_uploader: Arc, + snapshot_uploader: Arc, compression_algorithm: CompressionAlgorithm, logger: Logger, ) -> Self { @@ -89,11 +88,11 @@ impl CardanoImmutableFilesFullArtifactBuilder { async fn upload_snapshot_archive( &self, ongoing_snapshot: &OngoingSnapshot, - ) -> StdResult> { + ) -> StdResult> { debug!(self.logger, ">> upload_snapshot_archive"); let location = self .snapshot_uploader - .upload_snapshot(ongoing_snapshot.get_file_path()) + .upload(ongoing_snapshot.get_file_path()) .await; if let Err(error) = tokio::fs::remove_file(ongoing_snapshot.get_file_path()).await { @@ -174,7 +173,7 @@ mod tests { use mithril_common::{entities::CompressionAlgorithm, test_utils::fake_data}; use crate::{ - file_uploaders::MockSnapshotUploader, test_tools::TestLogger, DumbSnapshotUploader, + file_uploaders::MockFileUploader, test_tools::TestLogger, DumbSnapshotUploader, DumbSnapshotter, }; @@ -325,9 +324,9 @@ mod tests { let file = NamedTempFile::new().unwrap(); let file_path = file.path(); let snapshot = OngoingSnapshot::new(file_path.to_path_buf(), 7331); - let mut snapshot_uploader = MockSnapshotUploader::new(); + let mut snapshot_uploader = MockFileUploader::new(); snapshot_uploader - .expect_upload_snapshot() + .expect_upload() .return_once(|_| Err(anyhow!("an error"))) .once(); diff --git a/mithril-aggregator/src/dependency_injection/builder.rs b/mithril-aggregator/src/dependency_injection/builder.rs index c09d6fb19ea..49636e5e132 100644 --- a/mithril-aggregator/src/dependency_injection/builder.rs +++ b/mithril-aggregator/src/dependency_injection/builder.rs @@ -78,7 +78,7 @@ use crate::{ AggregatorConfig, AggregatorRunner, AggregatorRuntime, CompressedArchiveSnapshotter, Configuration, DependencyContainer, DumbSnapshotUploader, DumbSnapshotter, EpochSettingsStorer, LocalSnapshotUploader, MetricsService, MithrilSignerRegisterer, MultiSigner, MultiSignerImpl, - RemoteSnapshotUploader, SingleSignatureAuthenticator, SnapshotUploader, SnapshotUploaderType, + RemoteSnapshotUploader, SingleSignatureAuthenticator, FileUploader, SnapshotUploaderType, Snapshotter, SnapshotterCompressionAlgorithm, VerificationKeyStorer, }; @@ -118,7 +118,7 @@ pub struct DependenciesBuilder { pub stake_store: Option>, /// Snapshot uploader service. - pub snapshot_uploader: Option>, + pub snapshot_uploader: Option>, /// Multisigner service. pub multi_signer: Option>, @@ -446,7 +446,7 @@ impl DependenciesBuilder { Ok(self.stake_store.as_ref().cloned().unwrap()) } - async fn build_snapshot_uploader(&mut self) -> Result> { + async fn build_snapshot_uploader(&mut self) -> Result> { let logger = self.root_logger(); if self.configuration.environment == ExecutionEnvironment::Production { match self.configuration.snapshot_uploader_type { @@ -479,8 +479,8 @@ impl DependenciesBuilder { } } - /// Get a [SnapshotUploader] - pub async fn get_snapshot_uploader(&mut self) -> Result> { + /// Get a [FileUploader] + pub async fn get_snapshot_uploader(&mut self) -> Result> { if self.snapshot_uploader.is_none() { self.snapshot_uploader = Some(self.build_snapshot_uploader().await?); } diff --git a/mithril-aggregator/src/dependency_injection/containers.rs b/mithril-aggregator/src/dependency_injection/containers.rs index 32b52532460..50de0d77a0f 100644 --- a/mithril-aggregator/src/dependency_injection/containers.rs +++ b/mithril-aggregator/src/dependency_injection/containers.rs @@ -32,7 +32,7 @@ use crate::{ }, entities::AggregatorEpochSettings, event_store::{EventMessage, TransmitterService}, - file_uploaders::SnapshotUploader, + file_uploaders::FileUploader, multi_signer::MultiSigner, services::{ CertifierService, EpochService, MessageService, ProverService, SignedEntityService, @@ -71,7 +71,7 @@ pub struct DependencyContainer { pub stake_store: Arc, /// Snapshot uploader service. - pub snapshot_uploader: Arc, + pub snapshot_uploader: Arc, /// Multisigner service. pub multi_signer: Arc, diff --git a/mithril-aggregator/src/file_uploaders/dumb_snapshot_uploader.rs b/mithril-aggregator/src/file_uploaders/dumb_snapshot_uploader.rs index f077514aed4..bb3c4a26aff 100644 --- a/mithril-aggregator/src/file_uploaders/dumb_snapshot_uploader.rs +++ b/mithril-aggregator/src/file_uploaders/dumb_snapshot_uploader.rs @@ -3,7 +3,7 @@ use async_trait::async_trait; use mithril_common::StdResult; use std::{path::Path, sync::RwLock}; -use super::{SnapshotLocation, SnapshotUploader}; +use super::{FileLocation, FileUploader}; /// Dummy uploader for test purposes. /// @@ -39,9 +39,9 @@ impl Default for DumbSnapshotUploader { } #[async_trait] -impl SnapshotUploader for DumbSnapshotUploader { +impl FileUploader for DumbSnapshotUploader { /// Upload a snapshot - async fn upload_snapshot(&self, snapshot_filepath: &Path) -> StdResult { + async fn upload(&self, snapshot_filepath: &Path) -> StdResult { let mut value = self .last_uploaded .write() @@ -66,7 +66,7 @@ mod tests { .expect("uploader should not fail") .is_none()); let res = uploader - .upload_snapshot(Path::new("/tmp/whatever")) + .upload(Path::new("/tmp/whatever")) .await .expect("uploading with a dumb uploader should not fail"); assert_eq!(res, "/tmp/whatever".to_string()); diff --git a/mithril-aggregator/src/file_uploaders/file_uploader.rs b/mithril-aggregator/src/file_uploaders/file_uploader.rs index d76b4a746e4..34eb21913ea 100644 --- a/mithril-aggregator/src/file_uploaders/file_uploader.rs +++ b/mithril-aggregator/src/file_uploaders/file_uploader.rs @@ -2,12 +2,12 @@ use async_trait::async_trait; use mithril_common::StdResult; use std::path::Path; -pub type SnapshotLocation = String; +pub type FileLocation = String; -/// SnapshotUploader represents a snapshot uploader interactor +/// FileUploader represents a file uploader interactor #[cfg_attr(test, mockall::automock)] #[async_trait] -pub trait SnapshotUploader: Sync + Send { - /// Upload a snapshot - async fn upload_snapshot(&self, snapshot_filepath: &Path) -> StdResult; +pub trait FileUploader: Sync + Send { + /// Upload a file + async fn upload(&self, filepath: &Path) -> StdResult; } diff --git a/mithril-aggregator/src/file_uploaders/local_snapshot_uploader.rs b/mithril-aggregator/src/file_uploaders/local_snapshot_uploader.rs index f984c571072..d9577e01b62 100644 --- a/mithril-aggregator/src/file_uploaders/local_snapshot_uploader.rs +++ b/mithril-aggregator/src/file_uploaders/local_snapshot_uploader.rs @@ -6,7 +6,7 @@ use std::path::{Path, PathBuf}; use mithril_common::logging::LoggerExtensions; use mithril_common::StdResult; -use crate::file_uploaders::{SnapshotLocation, SnapshotUploader}; +use crate::file_uploaders::{FileLocation, FileUploader}; use crate::http_server; use crate::tools; @@ -35,8 +35,8 @@ impl LocalSnapshotUploader { } #[async_trait] -impl SnapshotUploader for LocalSnapshotUploader { - async fn upload_snapshot(&self, snapshot_filepath: &Path) -> StdResult { +impl FileUploader for LocalSnapshotUploader { + async fn upload(&self, snapshot_filepath: &Path) -> StdResult { let archive_name = snapshot_filepath.file_name().unwrap().to_str().unwrap(); let target_path = &self.target_location.join(archive_name); tokio::fs::copy(snapshot_filepath, target_path) @@ -63,7 +63,7 @@ mod tests { use std::path::{Path, PathBuf}; use tempfile::tempdir; - use crate::file_uploaders::SnapshotUploader; + use crate::file_uploaders::FileUploader; use crate::http_server; use crate::test_tools::TestLogger; @@ -97,7 +97,7 @@ mod tests { let uploader = LocalSnapshotUploader::new(url, target_dir.path(), TestLogger::stdout()); let location = uploader - .upload_snapshot(&archive) + .upload(&archive) .await .expect("local upload should not fail"); @@ -115,7 +115,7 @@ mod tests { target_dir.path(), TestLogger::stdout(), ); - uploader.upload_snapshot(&archive).await.unwrap(); + uploader.upload(&archive).await.unwrap(); assert!(target_dir .path() diff --git a/mithril-aggregator/src/file_uploaders/mod.rs b/mithril-aggregator/src/file_uploaders/mod.rs index 0e760c67dc2..846d501e0dd 100644 --- a/mithril-aggregator/src/file_uploaders/mod.rs +++ b/mithril-aggregator/src/file_uploaders/mod.rs @@ -4,10 +4,10 @@ mod local_snapshot_uploader; mod remote_snapshot_uploader; pub use dumb_snapshot_uploader::*; -pub use file_uploader::SnapshotLocation; -pub use file_uploader::SnapshotUploader; +pub use file_uploader::FileLocation; +pub use file_uploader::FileUploader; pub use local_snapshot_uploader::LocalSnapshotUploader; pub use remote_snapshot_uploader::RemoteSnapshotUploader; #[cfg(test)] -pub use file_uploader::MockSnapshotUploader; +pub use file_uploader::MockFileUploader; diff --git a/mithril-aggregator/src/file_uploaders/remote_snapshot_uploader.rs b/mithril-aggregator/src/file_uploaders/remote_snapshot_uploader.rs index 7a061e982a7..7a6a5d144c7 100644 --- a/mithril-aggregator/src/file_uploaders/remote_snapshot_uploader.rs +++ b/mithril-aggregator/src/file_uploaders/remote_snapshot_uploader.rs @@ -5,7 +5,7 @@ use std::path::Path; use mithril_common::logging::LoggerExtensions; use mithril_common::StdResult; -use crate::file_uploaders::{SnapshotLocation, SnapshotUploader}; +use crate::file_uploaders::{FileLocation, FileUploader}; use crate::tools::RemoteFileUploader; /// GCPSnapshotUploader is a snapshot uploader working using Google Cloud Platform services @@ -36,8 +36,8 @@ impl RemoteSnapshotUploader { } #[async_trait] -impl SnapshotUploader for RemoteSnapshotUploader { - async fn upload_snapshot(&self, snapshot_filepath: &Path) -> StdResult { +impl FileUploader for RemoteSnapshotUploader { + async fn upload(&self, snapshot_filepath: &Path) -> StdResult { let archive_name = snapshot_filepath.file_name().unwrap().to_str().unwrap(); let location = if self.use_cdn_domain { format!("https://{}/{}", self.bucket, archive_name) @@ -61,7 +61,7 @@ mod tests { use anyhow::anyhow; use std::path::Path; - use crate::file_uploaders::SnapshotUploader; + use crate::file_uploaders::FileUploader; use crate::test_tools::TestLogger; use crate::tools::MockRemoteFileUploader; @@ -83,7 +83,7 @@ mod tests { "https://storage.googleapis.com/cardano-testnet/snapshot.xxx.tar.gz".to_string(); let location = snapshot_uploader - .upload_snapshot(snapshot_filepath) + .upload(snapshot_filepath) .await .expect("remote upload should not fail"); @@ -105,7 +105,7 @@ mod tests { let expected_location = "https://cdn.mithril.network/snapshot.xxx.tar.gz".to_string(); let location = snapshot_uploader - .upload_snapshot(snapshot_filepath) + .upload(snapshot_filepath) .await .expect("remote upload should not fail"); @@ -127,7 +127,7 @@ mod tests { let snapshot_filepath = Path::new("test/snapshot.xxx.tar.gz"); let result = snapshot_uploader - .upload_snapshot(snapshot_filepath) + .upload(snapshot_filepath) .await .expect_err("remote upload should fail"); assert_eq!("unexpected error".to_string(), result.to_string()); diff --git a/mithril-aggregator/src/lib.rs b/mithril-aggregator/src/lib.rs index 6a56aff6a89..76005aa73ae 100644 --- a/mithril-aggregator/src/lib.rs +++ b/mithril-aggregator/src/lib.rs @@ -39,7 +39,7 @@ pub use crate::multi_signer::{MultiSigner, MultiSignerImpl}; pub use commands::{CommandType, MainOpts}; pub use dependency_injection::DependencyContainer; pub use file_uploaders::{ - DumbSnapshotUploader, LocalSnapshotUploader, RemoteSnapshotUploader, SnapshotUploader, + DumbSnapshotUploader, FileUploader, LocalSnapshotUploader, RemoteSnapshotUploader, }; pub use message_adapters::{FromRegisterSignerAdapter, ToCertificatePendingMessageAdapter}; pub use metrics::*; From 9da14ea4220e04f1e250249062e87e44734b3bc3 Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Tue, 10 Dec 2024 10:58:57 +0100 Subject: [PATCH 04/13] refactor: rename uploader implementation modules for consistency --- ...mb_snapshot_uploader.rs => dumb_file_uploader.rs} | 0 ...l_snapshot_uploader.rs => local_file_uploader.rs} | 0 mithril-aggregator/src/file_uploaders/mod.rs | 12 ++++++------ ..._snapshot_uploader.rs => remote_file_uploader.rs} | 0 4 files changed, 6 insertions(+), 6 deletions(-) rename mithril-aggregator/src/file_uploaders/{dumb_snapshot_uploader.rs => dumb_file_uploader.rs} (100%) rename mithril-aggregator/src/file_uploaders/{local_snapshot_uploader.rs => local_file_uploader.rs} (100%) rename mithril-aggregator/src/file_uploaders/{remote_snapshot_uploader.rs => remote_file_uploader.rs} (100%) diff --git a/mithril-aggregator/src/file_uploaders/dumb_snapshot_uploader.rs b/mithril-aggregator/src/file_uploaders/dumb_file_uploader.rs similarity index 100% rename from mithril-aggregator/src/file_uploaders/dumb_snapshot_uploader.rs rename to mithril-aggregator/src/file_uploaders/dumb_file_uploader.rs diff --git a/mithril-aggregator/src/file_uploaders/local_snapshot_uploader.rs b/mithril-aggregator/src/file_uploaders/local_file_uploader.rs similarity index 100% rename from mithril-aggregator/src/file_uploaders/local_snapshot_uploader.rs rename to mithril-aggregator/src/file_uploaders/local_file_uploader.rs diff --git a/mithril-aggregator/src/file_uploaders/mod.rs b/mithril-aggregator/src/file_uploaders/mod.rs index 846d501e0dd..970324f86b2 100644 --- a/mithril-aggregator/src/file_uploaders/mod.rs +++ b/mithril-aggregator/src/file_uploaders/mod.rs @@ -1,13 +1,13 @@ -mod dumb_snapshot_uploader; +mod dumb_file_uploader; mod file_uploader; -mod local_snapshot_uploader; -mod remote_snapshot_uploader; +mod local_file_uploader; +mod remote_file_uploader; -pub use dumb_snapshot_uploader::*; +pub use dumb_file_uploader::*; pub use file_uploader::FileLocation; pub use file_uploader::FileUploader; -pub use local_snapshot_uploader::LocalSnapshotUploader; -pub use remote_snapshot_uploader::RemoteSnapshotUploader; +pub use local_file_uploader::LocalSnapshotUploader; +pub use remote_file_uploader::RemoteSnapshotUploader; #[cfg(test)] pub use file_uploader::MockFileUploader; diff --git a/mithril-aggregator/src/file_uploaders/remote_snapshot_uploader.rs b/mithril-aggregator/src/file_uploaders/remote_file_uploader.rs similarity index 100% rename from mithril-aggregator/src/file_uploaders/remote_snapshot_uploader.rs rename to mithril-aggregator/src/file_uploaders/remote_file_uploader.rs From e2cfd5f849c87191a3b7a9cc06071c974d531fdf Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Tue, 10 Dec 2024 11:04:26 +0100 Subject: [PATCH 05/13] refactor: rename `DumbSnapshotUploader` to `DumbFileUploader` and remove `snapshot` references --- .../cardano_immutable_files_full.rs | 11 +++++------ .../src/dependency_injection/builder.rs | 8 ++++---- .../src/file_uploaders/dumb_file_uploader.rs | 18 +++++++++--------- mithril-aggregator/src/lib.rs | 2 +- .../tests/test_extensions/runtime_tester.rs | 6 +++--- 5 files changed, 22 insertions(+), 23 deletions(-) diff --git a/mithril-aggregator/src/artifact_builder/cardano_immutable_files_full.rs b/mithril-aggregator/src/artifact_builder/cardano_immutable_files_full.rs index 08ef75eb4e0..ada08634768 100644 --- a/mithril-aggregator/src/artifact_builder/cardano_immutable_files_full.rs +++ b/mithril-aggregator/src/artifact_builder/cardano_immutable_files_full.rs @@ -173,8 +173,7 @@ mod tests { use mithril_common::{entities::CompressionAlgorithm, test_utils::fake_data}; use crate::{ - file_uploaders::MockFileUploader, test_tools::TestLogger, DumbSnapshotUploader, - DumbSnapshotter, + file_uploaders::MockFileUploader, test_tools::TestLogger, DumbFileUploader, DumbSnapshotter, }; use super::*; @@ -189,7 +188,7 @@ mod tests { .unwrap(); let dumb_snapshotter = Arc::new(DumbSnapshotter::new()); - let dumb_snapshot_uploader = Arc::new(DumbSnapshotUploader::new()); + let dumb_snapshot_uploader = Arc::new(DumbFileUploader::new()); let cardano_immutable_files_full_artifact_builder = CardanoImmutableFilesFullArtifactBuilder::new( @@ -236,7 +235,7 @@ mod tests { fake_data::network(), &Version::parse("1.0.0").unwrap(), Arc::new(DumbSnapshotter::new()), - Arc::new(DumbSnapshotUploader::new()), + Arc::new(DumbFileUploader::new()), CompressionAlgorithm::default(), TestLogger::stdout(), ); @@ -263,7 +262,7 @@ mod tests { network, &Version::parse("1.0.0").unwrap(), Arc::new(DumbSnapshotter::new()), - Arc::new(DumbSnapshotUploader::new()), + Arc::new(DumbFileUploader::new()), CompressionAlgorithm::Gzip, TestLogger::stdout(), ); @@ -292,7 +291,7 @@ mod tests { fake_data::network(), &Version::parse("1.0.0").unwrap(), Arc::new(DumbSnapshotter::new()), - Arc::new(DumbSnapshotUploader::new()), + Arc::new(DumbFileUploader::new()), algorithm, TestLogger::stdout(), ); diff --git a/mithril-aggregator/src/dependency_injection/builder.rs b/mithril-aggregator/src/dependency_injection/builder.rs index 49636e5e132..48fbf70a391 100644 --- a/mithril-aggregator/src/dependency_injection/builder.rs +++ b/mithril-aggregator/src/dependency_injection/builder.rs @@ -76,9 +76,9 @@ use crate::{ store::CertificatePendingStorer, tools::{CExplorerSignerRetriever, GcpFileUploader, GenesisToolsDependency, SignersImporter}, AggregatorConfig, AggregatorRunner, AggregatorRuntime, CompressedArchiveSnapshotter, - Configuration, DependencyContainer, DumbSnapshotUploader, DumbSnapshotter, EpochSettingsStorer, - LocalSnapshotUploader, MetricsService, MithrilSignerRegisterer, MultiSigner, MultiSignerImpl, - RemoteSnapshotUploader, SingleSignatureAuthenticator, FileUploader, SnapshotUploaderType, + Configuration, DependencyContainer, DumbFileUploader, DumbSnapshotter, EpochSettingsStorer, + FileUploader, LocalSnapshotUploader, MetricsService, MithrilSignerRegisterer, MultiSigner, + MultiSignerImpl, RemoteSnapshotUploader, SingleSignatureAuthenticator, SnapshotUploaderType, Snapshotter, SnapshotterCompressionAlgorithm, VerificationKeyStorer, }; @@ -475,7 +475,7 @@ impl DependenciesBuilder { ))), } } else { - Ok(Arc::new(DumbSnapshotUploader::new())) + Ok(Arc::new(DumbFileUploader::new())) } } diff --git a/mithril-aggregator/src/file_uploaders/dumb_file_uploader.rs b/mithril-aggregator/src/file_uploaders/dumb_file_uploader.rs index bb3c4a26aff..2383dc9bcc1 100644 --- a/mithril-aggregator/src/file_uploaders/dumb_file_uploader.rs +++ b/mithril-aggregator/src/file_uploaders/dumb_file_uploader.rs @@ -7,13 +7,13 @@ use super::{FileLocation, FileUploader}; /// Dummy uploader for test purposes. /// -/// It actually does NOT upload any snapshot but remembers the last snapshot it +/// It actually does NOT upload any file but remembers the last file it /// was asked to upload. This is intended to by used by integration tests. -pub struct DumbSnapshotUploader { +pub struct DumbFileUploader { last_uploaded: RwLock>, } -impl DumbSnapshotUploader { +impl DumbFileUploader { /// Create a new instance. pub fn new() -> Self { Self { @@ -32,22 +32,22 @@ impl DumbSnapshotUploader { } } -impl Default for DumbSnapshotUploader { +impl Default for DumbFileUploader { fn default() -> Self { Self::new() } } #[async_trait] -impl FileUploader for DumbSnapshotUploader { - /// Upload a snapshot - async fn upload(&self, snapshot_filepath: &Path) -> StdResult { +impl FileUploader for DumbFileUploader { + /// Upload a file + async fn upload(&self, filepath: &Path) -> StdResult { let mut value = self .last_uploaded .write() .map_err(|e| anyhow!("Error while saving filepath location: {e}"))?; - let location = snapshot_filepath.to_string_lossy().to_string(); + let location = filepath.to_string_lossy().to_string(); *value = Some(location.clone()); Ok(location) @@ -60,7 +60,7 @@ mod tests { #[tokio::test] async fn test_dumb_uploader() { - let uploader = DumbSnapshotUploader::new(); + let uploader = DumbFileUploader::new(); assert!(uploader .get_last_upload() .expect("uploader should not fail") diff --git a/mithril-aggregator/src/lib.rs b/mithril-aggregator/src/lib.rs index 76005aa73ae..c7749c51047 100644 --- a/mithril-aggregator/src/lib.rs +++ b/mithril-aggregator/src/lib.rs @@ -39,7 +39,7 @@ pub use crate::multi_signer::{MultiSigner, MultiSignerImpl}; pub use commands::{CommandType, MainOpts}; pub use dependency_injection::DependencyContainer; pub use file_uploaders::{ - DumbSnapshotUploader, FileUploader, LocalSnapshotUploader, RemoteSnapshotUploader, + DumbFileUploader, FileUploader, LocalSnapshotUploader, RemoteSnapshotUploader, }; pub use message_adapters::{FromRegisterSignerAdapter, ToCertificatePendingMessageAdapter}; pub use metrics::*; diff --git a/mithril-aggregator/tests/test_extensions/runtime_tester.rs b/mithril-aggregator/tests/test_extensions/runtime_tester.rs index b27f874f11e..c48548d75ce 100644 --- a/mithril-aggregator/tests/test_extensions/runtime_tester.rs +++ b/mithril-aggregator/tests/test_extensions/runtime_tester.rs @@ -7,7 +7,7 @@ use mithril_aggregator::{ database::{record::SignedEntityRecord, repository::OpenMessageRepository}, dependency_injection::DependenciesBuilder, event_store::EventMessage, - AggregatorRuntime, Configuration, DependencyContainer, DumbSnapshotUploader, DumbSnapshotter, + AggregatorRuntime, Configuration, DependencyContainer, DumbFileUploader, DumbSnapshotter, SignerRegistrationError, }; use mithril_common::{ @@ -100,7 +100,7 @@ macro_rules! assert_metrics_eq { pub struct RuntimeTester { pub network: String, - pub snapshot_uploader: Arc, + pub snapshot_uploader: Arc, pub chain_observer: Arc, pub immutable_file_observer: Arc, pub digester: Arc, @@ -130,7 +130,7 @@ impl RuntimeTester { let logger = build_logger(); let global_logger = slog_scope::set_global_logger(logger.clone()); let network = configuration.network.clone(); - let snapshot_uploader = Arc::new(DumbSnapshotUploader::new()); + let snapshot_uploader = Arc::new(DumbFileUploader::new()); let immutable_file_observer = Arc::new(DumbImmutableFileObserver::new()); immutable_file_observer .shall_return(Some(start_time_point.immutable_file_number)) From 3308c69b87473c3eb7992af9868439fc59b74877 Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Tue, 10 Dec 2024 11:47:53 +0100 Subject: [PATCH 06/13] refactor: rename `LocalSnapshotUploader` to `LocalFileUploader` and remove `snapshot` references MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Sébastien Fauvel --- .../src/dependency_injection/builder.rs | 4 +- .../src/file_uploaders/local_file_uploader.rs | 44 ++++++++++--------- mithril-aggregator/src/file_uploaders/mod.rs | 2 +- mithril-aggregator/src/lib.rs | 2 +- 4 files changed, 27 insertions(+), 25 deletions(-) diff --git a/mithril-aggregator/src/dependency_injection/builder.rs b/mithril-aggregator/src/dependency_injection/builder.rs index 48fbf70a391..f6a2a9a398f 100644 --- a/mithril-aggregator/src/dependency_injection/builder.rs +++ b/mithril-aggregator/src/dependency_injection/builder.rs @@ -77,7 +77,7 @@ use crate::{ tools::{CExplorerSignerRetriever, GcpFileUploader, GenesisToolsDependency, SignersImporter}, AggregatorConfig, AggregatorRunner, AggregatorRuntime, CompressedArchiveSnapshotter, Configuration, DependencyContainer, DumbFileUploader, DumbSnapshotter, EpochSettingsStorer, - FileUploader, LocalSnapshotUploader, MetricsService, MithrilSignerRegisterer, MultiSigner, + FileUploader, LocalFileUploader, MetricsService, MithrilSignerRegisterer, MultiSigner, MultiSignerImpl, RemoteSnapshotUploader, SingleSignatureAuthenticator, SnapshotUploaderType, Snapshotter, SnapshotterCompressionAlgorithm, VerificationKeyStorer, }; @@ -468,7 +468,7 @@ impl DependenciesBuilder { logger, ))) } - SnapshotUploaderType::Local => Ok(Arc::new(LocalSnapshotUploader::new( + SnapshotUploaderType::Local => Ok(Arc::new(LocalFileUploader::new( self.configuration.get_server_url(), &self.configuration.snapshot_directory, logger, diff --git a/mithril-aggregator/src/file_uploaders/local_file_uploader.rs b/mithril-aggregator/src/file_uploaders/local_file_uploader.rs index d9577e01b62..03bec153e75 100644 --- a/mithril-aggregator/src/file_uploaders/local_file_uploader.rs +++ b/mithril-aggregator/src/file_uploaders/local_file_uploader.rs @@ -10,24 +10,24 @@ use crate::file_uploaders::{FileLocation, FileUploader}; use crate::http_server; use crate::tools; -/// LocalSnapshotUploader is a snapshot uploader working using local files -pub struct LocalSnapshotUploader { - /// Snapshot server listening IP - snapshot_server_url: String, +/// LocalFileUploader is a file uploader working using local files +pub struct LocalFileUploader { + /// File server listening IP + file_server_url: String, - /// Target folder where to store snapshots archive + /// Target folder where to store files archive target_location: PathBuf, logger: Logger, } -impl LocalSnapshotUploader { - /// LocalSnapshotUploader factory - pub(crate) fn new(snapshot_server_url: String, target_location: &Path, logger: Logger) -> Self { +impl LocalFileUploader { + /// LocalFileUploader factory + pub(crate) fn new(file_server_url: String, target_location: &Path, logger: Logger) -> Self { let logger = logger.new_with_component_name::(); - debug!(logger, "New LocalSnapshotUploader created"; "snapshot_server_url" => &snapshot_server_url); + debug!(logger, "New LocalFileUploader created"; "file_server_url" => &file_server_url); Self { - snapshot_server_url, + file_server_url, target_location: target_location.to_path_buf(), logger, } @@ -35,23 +35,25 @@ impl LocalSnapshotUploader { } #[async_trait] -impl FileUploader for LocalSnapshotUploader { - async fn upload(&self, snapshot_filepath: &Path) -> StdResult { - let archive_name = snapshot_filepath.file_name().unwrap().to_str().unwrap(); +impl FileUploader for LocalFileUploader { + async fn upload(&self, filepath: &Path) -> StdResult { + let archive_name = filepath.file_name().unwrap().to_str().unwrap(); let target_path = &self.target_location.join(archive_name); - tokio::fs::copy(snapshot_filepath, target_path) + tokio::fs::copy(filepath, target_path) .await - .with_context(|| "Snapshot copy failure")?; + .with_context(|| "File copy failure")?; let digest = tools::extract_digest_from_path(Path::new(archive_name)); + let specific_route = "artifact/snapshot"; let location = format!( - "{}{}/artifact/snapshot/{}/download", - self.snapshot_server_url, + "{}{}/{}/{}/download", + self.file_server_url, http_server::SERVER_BASE_PATH, + specific_route, digest.unwrap() ); - debug!(self.logger, "Snapshot 'uploaded' to local storage"; "location" => &location); + debug!(self.logger, "File 'uploaded' to local storage"; "location" => &location); Ok(location) } } @@ -67,7 +69,7 @@ mod tests { use crate::http_server; use crate::test_tools::TestLogger; - use super::LocalSnapshotUploader; + use super::LocalFileUploader; fn create_fake_archive(dir: &Path, digest: &str) -> PathBuf { let file_path = dir.join(format!("test.{digest}.tar.gz")); @@ -94,7 +96,7 @@ mod tests { http_server::SERVER_BASE_PATH, &digest ); - let uploader = LocalSnapshotUploader::new(url, target_dir.path(), TestLogger::stdout()); + let uploader = LocalFileUploader::new(url, target_dir.path(), TestLogger::stdout()); let location = uploader .upload(&archive) @@ -110,7 +112,7 @@ mod tests { let target_dir = tempdir().unwrap(); let digest = "41e27b9ed5a32531b95b2b7ff3c0757591a06a337efaf19a524a998e348028e7"; let archive = create_fake_archive(source_dir.path(), digest); - let uploader = LocalSnapshotUploader::new( + let uploader = LocalFileUploader::new( "http://test.com:8080/".to_string(), target_dir.path(), TestLogger::stdout(), diff --git a/mithril-aggregator/src/file_uploaders/mod.rs b/mithril-aggregator/src/file_uploaders/mod.rs index 970324f86b2..764bcac2ff7 100644 --- a/mithril-aggregator/src/file_uploaders/mod.rs +++ b/mithril-aggregator/src/file_uploaders/mod.rs @@ -6,7 +6,7 @@ mod remote_file_uploader; pub use dumb_file_uploader::*; pub use file_uploader::FileLocation; pub use file_uploader::FileUploader; -pub use local_file_uploader::LocalSnapshotUploader; +pub use local_file_uploader::LocalFileUploader; pub use remote_file_uploader::RemoteSnapshotUploader; #[cfg(test)] diff --git a/mithril-aggregator/src/lib.rs b/mithril-aggregator/src/lib.rs index c7749c51047..aee76348939 100644 --- a/mithril-aggregator/src/lib.rs +++ b/mithril-aggregator/src/lib.rs @@ -39,7 +39,7 @@ pub use crate::multi_signer::{MultiSigner, MultiSignerImpl}; pub use commands::{CommandType, MainOpts}; pub use dependency_injection::DependencyContainer; pub use file_uploaders::{ - DumbFileUploader, FileUploader, LocalSnapshotUploader, RemoteSnapshotUploader, + DumbFileUploader, FileUploader, LocalFileUploader, RemoteSnapshotUploader, }; pub use message_adapters::{FromRegisterSignerAdapter, ToCertificatePendingMessageAdapter}; pub use metrics::*; From 95693bf9892c57535bac6f23968bd51eccd9c924 Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Tue, 10 Dec 2024 16:18:07 +0100 Subject: [PATCH 07/13] refactor: make `GcpFileUploader` implement `FileUploader` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Sébastien Fauvel --- .../src/dependency_injection/builder.rs | 8 +- .../file_uploaders/remote_file_uploader.rs | 97 +++++-------------- mithril-aggregator/src/tools/mod.rs | 5 +- .../src/tools/remote_file_uploader.rs | 72 +++++++++++--- 4 files changed, 91 insertions(+), 91 deletions(-) diff --git a/mithril-aggregator/src/dependency_injection/builder.rs b/mithril-aggregator/src/dependency_injection/builder.rs index f6a2a9a398f..1092d2d0321 100644 --- a/mithril-aggregator/src/dependency_injection/builder.rs +++ b/mithril-aggregator/src/dependency_injection/builder.rs @@ -462,9 +462,11 @@ impl DependenciesBuilder { })?; Ok(Arc::new(RemoteSnapshotUploader::new( - Box::new(GcpFileUploader::new(bucket.clone(), logger.clone())), - bucket, - self.configuration.snapshot_use_cdn_domain, + Box::new(GcpFileUploader::new( + bucket, + self.configuration.snapshot_use_cdn_domain, + logger.clone(), + )), logger, ))) } diff --git a/mithril-aggregator/src/file_uploaders/remote_file_uploader.rs b/mithril-aggregator/src/file_uploaders/remote_file_uploader.rs index 7a6a5d144c7..4e84da006fe 100644 --- a/mithril-aggregator/src/file_uploaders/remote_file_uploader.rs +++ b/mithril-aggregator/src/file_uploaders/remote_file_uploader.rs @@ -6,30 +6,20 @@ use mithril_common::logging::LoggerExtensions; use mithril_common::StdResult; use crate::file_uploaders::{FileLocation, FileUploader}; -use crate::tools::RemoteFileUploader; -/// GCPSnapshotUploader is a snapshot uploader working using Google Cloud Platform services +/// RemoteSnapshotUploader is a snapshot uploader working using Google Cloud Platform services pub struct RemoteSnapshotUploader { - bucket: String, - file_uploader: Box, - use_cdn_domain: bool, + file_uploader: Box, logger: Logger, } impl RemoteSnapshotUploader { - /// GCPSnapshotUploader factory - pub fn new( - file_uploader: Box, - bucket: String, - use_cdn_domain: bool, - logger: Logger, - ) -> Self { + /// RemoteSnapshotUploader factory + pub fn new(file_uploader: Box, logger: Logger) -> Self { let logger = logger.new_with_component_name::(); - debug!(logger, "New GCPSnapshotUploader created"); + debug!(logger, "New RemoteSnapshotUploader created"); Self { - bucket, file_uploader, - use_cdn_domain, logger, } } @@ -38,18 +28,7 @@ impl RemoteSnapshotUploader { #[async_trait] impl FileUploader for RemoteSnapshotUploader { async fn upload(&self, snapshot_filepath: &Path) -> StdResult { - let archive_name = snapshot_filepath.file_name().unwrap().to_str().unwrap(); - let location = if self.use_cdn_domain { - format!("https://{}/{}", self.bucket, archive_name) - } else { - format!( - "https://storage.googleapis.com/{}/{}", - self.bucket, archive_name - ) - }; - - debug!(self.logger, "Uploading snapshot to remote storage"; "location" => &location); - self.file_uploader.upload_file(snapshot_filepath).await?; + let location = self.file_uploader.upload(snapshot_filepath).await?; debug!(self.logger, "Snapshot upload to remote storage completed"; "location" => &location); Ok(location) @@ -59,53 +38,29 @@ impl FileUploader for RemoteSnapshotUploader { #[cfg(test)] mod tests { use anyhow::anyhow; + use mockall::predicate::eq; use std::path::Path; - use crate::file_uploaders::FileUploader; + use crate::file_uploaders::{FileUploader, MockFileUploader}; use crate::test_tools::TestLogger; - use crate::tools::MockRemoteFileUploader; use super::RemoteSnapshotUploader; #[tokio::test] - async fn test_upload_snapshot_not_using_cdn_domain_ok() { - let use_cdn_domain = false; - let mut file_uploader = MockRemoteFileUploader::new(); - file_uploader.expect_upload_file().returning(|_| Ok(())); - let snapshot_uploader = RemoteSnapshotUploader::new( - Box::new(file_uploader), - "cardano-testnet".to_string(), - use_cdn_domain, - TestLogger::stdout(), - ); - let snapshot_filepath = Path::new("test/snapshot.xxx.tar.gz"); - let expected_location = - "https://storage.googleapis.com/cardano-testnet/snapshot.xxx.tar.gz".to_string(); - - let location = snapshot_uploader - .upload(snapshot_filepath) - .await - .expect("remote upload should not fail"); - - assert_eq!(expected_location, location); - } - - #[tokio::test] - async fn test_upload_snapshot_using_cdn_domain_ok() { - let use_cdn_domain = true; - let mut file_uploader = MockRemoteFileUploader::new(); - file_uploader.expect_upload_file().returning(|_| Ok(())); - let snapshot_uploader = RemoteSnapshotUploader::new( - Box::new(file_uploader), - "cdn.mithril.network".to_string(), - use_cdn_domain, - TestLogger::stdout(), - ); - let snapshot_filepath = Path::new("test/snapshot.xxx.tar.gz"); + async fn upload_call_uploader_and_return_location() { + let mut file_uploader = MockFileUploader::new(); + file_uploader + .expect_upload() + .with(eq(Path::new("test/snapshot.xxx.tar.gz"))) + .times(1) + .returning(|_| Ok("https://cdn.mithril.network/snapshot.xxx.tar.gz".to_string())); + let snapshot_uploader = + RemoteSnapshotUploader::new(Box::new(file_uploader), TestLogger::stdout()); + let filepath = Path::new("test/snapshot.xxx.tar.gz"); let expected_location = "https://cdn.mithril.network/snapshot.xxx.tar.gz".to_string(); let location = snapshot_uploader - .upload(snapshot_filepath) + .upload(filepath) .await .expect("remote upload should not fail"); @@ -113,17 +68,13 @@ mod tests { } #[tokio::test] - async fn test_upload_snapshot_ko() { - let mut file_uploader = MockRemoteFileUploader::new(); + async fn upload_return_error_when_uploader_error() { + let mut file_uploader = MockFileUploader::new(); file_uploader - .expect_upload_file() + .expect_upload() .returning(|_| Err(anyhow!("unexpected error"))); - let snapshot_uploader = RemoteSnapshotUploader::new( - Box::new(file_uploader), - "".to_string(), - false, - TestLogger::stdout(), - ); + let snapshot_uploader = + RemoteSnapshotUploader::new(Box::new(file_uploader), TestLogger::stdout()); let snapshot_filepath = Path::new("test/snapshot.xxx.tar.gz"); let result = snapshot_uploader diff --git a/mithril-aggregator/src/tools/mod.rs b/mithril-aggregator/src/tools/mod.rs index 1df67dc565f..269a643aa4f 100644 --- a/mithril-aggregator/src/tools/mod.rs +++ b/mithril-aggregator/src/tools/mod.rs @@ -12,15 +12,12 @@ pub use certificates_hash_migrator::CertificatesHashMigrator; pub use digest_helpers::extract_digest_from_path; pub use era::EraTools; pub use genesis::{GenesisTools, GenesisToolsDependency}; -pub use remote_file_uploader::{GcpFileUploader, RemoteFileUploader}; +pub use remote_file_uploader::GcpFileUploader; pub use signer_importer::{ CExplorerSignerRetriever, SignersImporter, SignersImporterPersister, SignersImporterRetriever, }; pub use single_signature_authenticator::*; -#[cfg(test)] -pub use remote_file_uploader::MockRemoteFileUploader; - /// Downcast the error to the specified error type and check if the error satisfies the condition. pub(crate) fn downcast_check( error: &mithril_common::StdError, diff --git a/mithril-aggregator/src/tools/remote_file_uploader.rs b/mithril-aggregator/src/tools/remote_file_uploader.rs index 3d3ce9f7513..cfbe2e9e118 100644 --- a/mithril-aggregator/src/tools/remote_file_uploader.rs +++ b/mithril-aggregator/src/tools/remote_file_uploader.rs @@ -11,33 +11,41 @@ use tokio_util::{codec::BytesCodec, codec::FramedRead}; use mithril_common::logging::LoggerExtensions; use mithril_common::StdResult; -/// RemoteFileUploader represents a remote file uploader interactor -#[cfg_attr(test, mockall::automock)] -#[async_trait] -pub trait RemoteFileUploader: Sync + Send { - /// Upload a snapshot - async fn upload_file(&self, filepath: &Path) -> StdResult<()>; -} +use crate::file_uploaders::FileLocation; +use crate::FileUploader; /// GcpFileUploader represents a Google Cloud Platform file uploader interactor pub struct GcpFileUploader { bucket: String, + use_cdn_domain: bool, logger: Logger, } impl GcpFileUploader { /// GcpFileUploader factory - pub fn new(bucket: String, logger: Logger) -> Self { + pub fn new(bucket: String, use_cdn_domain: bool, logger: Logger) -> Self { Self { bucket, + use_cdn_domain, logger: logger.new_with_component_name::(), } } + + fn get_location(&self, filename: &str) -> String { + if self.use_cdn_domain { + format!("https://{}/{}", self.bucket, filename) + } else { + format!( + "https://storage.googleapis.com/{}/{}", + self.bucket, filename + ) + } + } } #[async_trait] -impl RemoteFileUploader for GcpFileUploader { - async fn upload_file(&self, filepath: &Path) -> StdResult<()> { +impl FileUploader for GcpFileUploader { + async fn upload(&self, filepath: &Path) -> StdResult { if env::var("GOOGLE_APPLICATION_CREDENTIALS_JSON").is_err() { return Err(anyhow!( "Missing GOOGLE_APPLICATION_CREDENTIALS_JSON environment variable".to_string() @@ -85,6 +93,48 @@ impl RemoteFileUploader for GcpFileUploader { info!(self.logger, "Updated acl for {filename}"); - Ok(()) + Ok(self.get_location(filename)) + } +} + +#[cfg(test)] +mod tests { + use crate::test_tools::TestLogger; + + use super::*; + + #[tokio::test] + async fn get_location_not_using_cdn_domain_return_google_api_uri() { + let use_cdn_domain = false; + + let file_uploader = GcpFileUploader::new( + "cardano-testnet".to_string(), + use_cdn_domain, + TestLogger::stdout(), + ); + let filename = "snapshot.xxx.tar.gz"; + let expected_location = + "https://storage.googleapis.com/cardano-testnet/snapshot.xxx.tar.gz".to_string(); + + let location = file_uploader.get_location(filename); + + assert_eq!(expected_location, location); + } + + #[tokio::test] + async fn get_location_using_cdn_domain_return_cdn_in_uri() { + let use_cdn_domain = true; + + let file_uploader = GcpFileUploader::new( + "cdn.mithril.network".to_string(), + use_cdn_domain, + TestLogger::stdout(), + ); + let filename = "snapshot.xxx.tar.gz"; + let expected_location = "https://cdn.mithril.network/snapshot.xxx.tar.gz".to_string(); + + let location = file_uploader.get_location(filename); + + assert_eq!(expected_location, location); } } From 8268e5feeeaabb1ee6fa8c9812740d190f49c71c Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Tue, 10 Dec 2024 16:23:20 +0100 Subject: [PATCH 08/13] refactor: rename `remote_file_uploader` to `gcp_file_uploader` --- .../tools/{remote_file_uploader.rs => gcp_file_uploader.rs} | 0 mithril-aggregator/src/tools/mod.rs | 4 ++-- 2 files changed, 2 insertions(+), 2 deletions(-) rename mithril-aggregator/src/tools/{remote_file_uploader.rs => gcp_file_uploader.rs} (100%) diff --git a/mithril-aggregator/src/tools/remote_file_uploader.rs b/mithril-aggregator/src/tools/gcp_file_uploader.rs similarity index 100% rename from mithril-aggregator/src/tools/remote_file_uploader.rs rename to mithril-aggregator/src/tools/gcp_file_uploader.rs diff --git a/mithril-aggregator/src/tools/mod.rs b/mithril-aggregator/src/tools/mod.rs index 269a643aa4f..e099821a6a3 100644 --- a/mithril-aggregator/src/tools/mod.rs +++ b/mithril-aggregator/src/tools/mod.rs @@ -1,18 +1,18 @@ mod certificates_hash_migrator; mod digest_helpers; mod era; +mod gcp_file_uploader; mod genesis; #[cfg(test)] pub mod mocks; -mod remote_file_uploader; mod signer_importer; mod single_signature_authenticator; pub use certificates_hash_migrator::CertificatesHashMigrator; pub use digest_helpers::extract_digest_from_path; pub use era::EraTools; +pub use gcp_file_uploader::GcpFileUploader; pub use genesis::{GenesisTools, GenesisToolsDependency}; -pub use remote_file_uploader::GcpFileUploader; pub use signer_importer::{ CExplorerSignerRetriever, SignersImporter, SignersImporterPersister, SignersImporterRetriever, }; From 424f6d3ec799958a082309ee467c0818cd0b3810 Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Tue, 10 Dec 2024 16:26:21 +0100 Subject: [PATCH 09/13] refactor: move `gcp_file_uploader` to `file_uploaders` module --- mithril-aggregator/src/dependency_injection/builder.rs | 3 ++- .../src/{tools => file_uploaders}/gcp_file_uploader.rs | 0 mithril-aggregator/src/file_uploaders/mod.rs | 2 ++ mithril-aggregator/src/tools/mod.rs | 2 -- 4 files changed, 4 insertions(+), 3 deletions(-) rename mithril-aggregator/src/{tools => file_uploaders}/gcp_file_uploader.rs (100%) diff --git a/mithril-aggregator/src/dependency_injection/builder.rs b/mithril-aggregator/src/dependency_injection/builder.rs index 1092d2d0321..8c3788a776f 100644 --- a/mithril-aggregator/src/dependency_injection/builder.rs +++ b/mithril-aggregator/src/dependency_injection/builder.rs @@ -64,6 +64,7 @@ use crate::{ }, entities::AggregatorEpochSettings, event_store::{EventMessage, EventStore, TransmitterService}, + file_uploaders::GcpFileUploader, http_server::routes::router::{self, RouterConfig, RouterState}, services::{ AggregatorSignableSeedBuilder, AggregatorUpkeepService, BufferedCertifierService, @@ -74,7 +75,7 @@ use crate::{ UpkeepService, UsageReporter, }, store::CertificatePendingStorer, - tools::{CExplorerSignerRetriever, GcpFileUploader, GenesisToolsDependency, SignersImporter}, + tools::{CExplorerSignerRetriever, GenesisToolsDependency, SignersImporter}, AggregatorConfig, AggregatorRunner, AggregatorRuntime, CompressedArchiveSnapshotter, Configuration, DependencyContainer, DumbFileUploader, DumbSnapshotter, EpochSettingsStorer, FileUploader, LocalFileUploader, MetricsService, MithrilSignerRegisterer, MultiSigner, diff --git a/mithril-aggregator/src/tools/gcp_file_uploader.rs b/mithril-aggregator/src/file_uploaders/gcp_file_uploader.rs similarity index 100% rename from mithril-aggregator/src/tools/gcp_file_uploader.rs rename to mithril-aggregator/src/file_uploaders/gcp_file_uploader.rs diff --git a/mithril-aggregator/src/file_uploaders/mod.rs b/mithril-aggregator/src/file_uploaders/mod.rs index 764bcac2ff7..4f8279843fd 100644 --- a/mithril-aggregator/src/file_uploaders/mod.rs +++ b/mithril-aggregator/src/file_uploaders/mod.rs @@ -1,11 +1,13 @@ mod dumb_file_uploader; mod file_uploader; +mod gcp_file_uploader; mod local_file_uploader; mod remote_file_uploader; pub use dumb_file_uploader::*; pub use file_uploader::FileLocation; pub use file_uploader::FileUploader; +pub use gcp_file_uploader::GcpFileUploader; pub use local_file_uploader::LocalFileUploader; pub use remote_file_uploader::RemoteSnapshotUploader; diff --git a/mithril-aggregator/src/tools/mod.rs b/mithril-aggregator/src/tools/mod.rs index e099821a6a3..bb13972d66e 100644 --- a/mithril-aggregator/src/tools/mod.rs +++ b/mithril-aggregator/src/tools/mod.rs @@ -1,7 +1,6 @@ mod certificates_hash_migrator; mod digest_helpers; mod era; -mod gcp_file_uploader; mod genesis; #[cfg(test)] pub mod mocks; @@ -11,7 +10,6 @@ mod single_signature_authenticator; pub use certificates_hash_migrator::CertificatesHashMigrator; pub use digest_helpers::extract_digest_from_path; pub use era::EraTools; -pub use gcp_file_uploader::GcpFileUploader; pub use genesis::{GenesisTools, GenesisToolsDependency}; pub use signer_importer::{ CExplorerSignerRetriever, SignersImporter, SignersImporterPersister, SignersImporterRetriever, From 0084fae7c34ee86a62b6a8415692c4411719032b Mon Sep 17 00:00:00 2001 From: sfauvel Date: Wed, 11 Dec 2024 14:26:02 +0100 Subject: [PATCH 10/13] refactor: Remobe `file` from file uploaders modules --- .../cardano_immutable_files_full.rs | 10 ++++---- .../src/dependency_injection/builder.rs | 16 ++++++------- ...dumb_file_uploader.rs => dumb_uploader.rs} | 10 ++++---- .../{gcp_file_uploader.rs => gcp_uploader.rs} | 14 +++++------ .../{file_uploader.rs => interface.rs} | 0 ...cal_file_uploader.rs => local_uploader.rs} | 18 +++++++------- mithril-aggregator/src/file_uploaders/mod.rs | 24 +++++++++---------- ...te_file_uploader.rs => remote_uploader.rs} | 20 +++++++--------- mithril-aggregator/src/lib.rs | 4 +--- .../tests/test_extensions/runtime_tester.rs | 6 ++--- 10 files changed, 59 insertions(+), 63 deletions(-) rename mithril-aggregator/src/file_uploaders/{dumb_file_uploader.rs => dumb_uploader.rs} (91%) rename mithril-aggregator/src/file_uploaders/{gcp_file_uploader.rs => gcp_uploader.rs} (92%) rename mithril-aggregator/src/file_uploaders/{file_uploader.rs => interface.rs} (100%) rename mithril-aggregator/src/file_uploaders/{local_file_uploader.rs => local_uploader.rs} (88%) rename mithril-aggregator/src/file_uploaders/{remote_file_uploader.rs => remote_uploader.rs} (79%) diff --git a/mithril-aggregator/src/artifact_builder/cardano_immutable_files_full.rs b/mithril-aggregator/src/artifact_builder/cardano_immutable_files_full.rs index ada08634768..61424f64103 100644 --- a/mithril-aggregator/src/artifact_builder/cardano_immutable_files_full.rs +++ b/mithril-aggregator/src/artifact_builder/cardano_immutable_files_full.rs @@ -173,7 +173,7 @@ mod tests { use mithril_common::{entities::CompressionAlgorithm, test_utils::fake_data}; use crate::{ - file_uploaders::MockFileUploader, test_tools::TestLogger, DumbFileUploader, DumbSnapshotter, + file_uploaders::MockFileUploader, test_tools::TestLogger, DumbUploader, DumbSnapshotter, }; use super::*; @@ -188,7 +188,7 @@ mod tests { .unwrap(); let dumb_snapshotter = Arc::new(DumbSnapshotter::new()); - let dumb_snapshot_uploader = Arc::new(DumbFileUploader::new()); + let dumb_snapshot_uploader = Arc::new(DumbUploader::new()); let cardano_immutable_files_full_artifact_builder = CardanoImmutableFilesFullArtifactBuilder::new( @@ -235,7 +235,7 @@ mod tests { fake_data::network(), &Version::parse("1.0.0").unwrap(), Arc::new(DumbSnapshotter::new()), - Arc::new(DumbFileUploader::new()), + Arc::new(DumbUploader::new()), CompressionAlgorithm::default(), TestLogger::stdout(), ); @@ -262,7 +262,7 @@ mod tests { network, &Version::parse("1.0.0").unwrap(), Arc::new(DumbSnapshotter::new()), - Arc::new(DumbFileUploader::new()), + Arc::new(DumbUploader::new()), CompressionAlgorithm::Gzip, TestLogger::stdout(), ); @@ -291,7 +291,7 @@ mod tests { fake_data::network(), &Version::parse("1.0.0").unwrap(), Arc::new(DumbSnapshotter::new()), - Arc::new(DumbFileUploader::new()), + Arc::new(DumbUploader::new()), algorithm, TestLogger::stdout(), ); diff --git a/mithril-aggregator/src/dependency_injection/builder.rs b/mithril-aggregator/src/dependency_injection/builder.rs index 8c3788a776f..71f5b63c365 100644 --- a/mithril-aggregator/src/dependency_injection/builder.rs +++ b/mithril-aggregator/src/dependency_injection/builder.rs @@ -64,7 +64,7 @@ use crate::{ }, entities::AggregatorEpochSettings, event_store::{EventMessage, EventStore, TransmitterService}, - file_uploaders::GcpFileUploader, + file_uploaders::GcpUploader, http_server::routes::router::{self, RouterConfig, RouterState}, services::{ AggregatorSignableSeedBuilder, AggregatorUpkeepService, BufferedCertifierService, @@ -77,9 +77,9 @@ use crate::{ store::CertificatePendingStorer, tools::{CExplorerSignerRetriever, GenesisToolsDependency, SignersImporter}, AggregatorConfig, AggregatorRunner, AggregatorRuntime, CompressedArchiveSnapshotter, - Configuration, DependencyContainer, DumbFileUploader, DumbSnapshotter, EpochSettingsStorer, - FileUploader, LocalFileUploader, MetricsService, MithrilSignerRegisterer, MultiSigner, - MultiSignerImpl, RemoteSnapshotUploader, SingleSignatureAuthenticator, SnapshotUploaderType, + Configuration, DependencyContainer, DumbSnapshotter, DumbUploader, EpochSettingsStorer, + FileUploader, LocalUploader, MetricsService, MithrilSignerRegisterer, MultiSigner, + MultiSignerImpl, RemoteUploader, SingleSignatureAuthenticator, SnapshotUploaderType, Snapshotter, SnapshotterCompressionAlgorithm, VerificationKeyStorer, }; @@ -462,8 +462,8 @@ impl DependenciesBuilder { ) })?; - Ok(Arc::new(RemoteSnapshotUploader::new( - Box::new(GcpFileUploader::new( + Ok(Arc::new(RemoteUploader::new( + Box::new(GcpUploader::new( bucket, self.configuration.snapshot_use_cdn_domain, logger.clone(), @@ -471,14 +471,14 @@ impl DependenciesBuilder { logger, ))) } - SnapshotUploaderType::Local => Ok(Arc::new(LocalFileUploader::new( + SnapshotUploaderType::Local => Ok(Arc::new(LocalUploader::new( self.configuration.get_server_url(), &self.configuration.snapshot_directory, logger, ))), } } else { - Ok(Arc::new(DumbFileUploader::new())) + Ok(Arc::new(DumbUploader::new())) } } diff --git a/mithril-aggregator/src/file_uploaders/dumb_file_uploader.rs b/mithril-aggregator/src/file_uploaders/dumb_uploader.rs similarity index 91% rename from mithril-aggregator/src/file_uploaders/dumb_file_uploader.rs rename to mithril-aggregator/src/file_uploaders/dumb_uploader.rs index 2383dc9bcc1..6f3ff18392a 100644 --- a/mithril-aggregator/src/file_uploaders/dumb_file_uploader.rs +++ b/mithril-aggregator/src/file_uploaders/dumb_uploader.rs @@ -9,11 +9,11 @@ use super::{FileLocation, FileUploader}; /// /// It actually does NOT upload any file but remembers the last file it /// was asked to upload. This is intended to by used by integration tests. -pub struct DumbFileUploader { +pub struct DumbUploader { last_uploaded: RwLock>, } -impl DumbFileUploader { +impl DumbUploader { /// Create a new instance. pub fn new() -> Self { Self { @@ -32,14 +32,14 @@ impl DumbFileUploader { } } -impl Default for DumbFileUploader { +impl Default for DumbUploader { fn default() -> Self { Self::new() } } #[async_trait] -impl FileUploader for DumbFileUploader { +impl FileUploader for DumbUploader { /// Upload a file async fn upload(&self, filepath: &Path) -> StdResult { let mut value = self @@ -60,7 +60,7 @@ mod tests { #[tokio::test] async fn test_dumb_uploader() { - let uploader = DumbFileUploader::new(); + let uploader = DumbUploader::new(); assert!(uploader .get_last_upload() .expect("uploader should not fail") diff --git a/mithril-aggregator/src/file_uploaders/gcp_file_uploader.rs b/mithril-aggregator/src/file_uploaders/gcp_uploader.rs similarity index 92% rename from mithril-aggregator/src/file_uploaders/gcp_file_uploader.rs rename to mithril-aggregator/src/file_uploaders/gcp_uploader.rs index cfbe2e9e118..2ffdd4c2572 100644 --- a/mithril-aggregator/src/file_uploaders/gcp_file_uploader.rs +++ b/mithril-aggregator/src/file_uploaders/gcp_uploader.rs @@ -14,15 +14,15 @@ use mithril_common::StdResult; use crate::file_uploaders::FileLocation; use crate::FileUploader; -/// GcpFileUploader represents a Google Cloud Platform file uploader interactor -pub struct GcpFileUploader { +/// GcpUploader represents a Google Cloud Platform file uploader interactor +pub struct GcpUploader { bucket: String, use_cdn_domain: bool, logger: Logger, } -impl GcpFileUploader { - /// GcpFileUploader factory +impl GcpUploader { + /// GcpUploader factory pub fn new(bucket: String, use_cdn_domain: bool, logger: Logger) -> Self { Self { bucket, @@ -44,7 +44,7 @@ impl GcpFileUploader { } #[async_trait] -impl FileUploader for GcpFileUploader { +impl FileUploader for GcpUploader { async fn upload(&self, filepath: &Path) -> StdResult { if env::var("GOOGLE_APPLICATION_CREDENTIALS_JSON").is_err() { return Err(anyhow!( @@ -107,7 +107,7 @@ mod tests { async fn get_location_not_using_cdn_domain_return_google_api_uri() { let use_cdn_domain = false; - let file_uploader = GcpFileUploader::new( + let file_uploader = GcpUploader::new( "cardano-testnet".to_string(), use_cdn_domain, TestLogger::stdout(), @@ -125,7 +125,7 @@ mod tests { async fn get_location_using_cdn_domain_return_cdn_in_uri() { let use_cdn_domain = true; - let file_uploader = GcpFileUploader::new( + let file_uploader = GcpUploader::new( "cdn.mithril.network".to_string(), use_cdn_domain, TestLogger::stdout(), diff --git a/mithril-aggregator/src/file_uploaders/file_uploader.rs b/mithril-aggregator/src/file_uploaders/interface.rs similarity index 100% rename from mithril-aggregator/src/file_uploaders/file_uploader.rs rename to mithril-aggregator/src/file_uploaders/interface.rs diff --git a/mithril-aggregator/src/file_uploaders/local_file_uploader.rs b/mithril-aggregator/src/file_uploaders/local_uploader.rs similarity index 88% rename from mithril-aggregator/src/file_uploaders/local_file_uploader.rs rename to mithril-aggregator/src/file_uploaders/local_uploader.rs index 03bec153e75..2d85692199c 100644 --- a/mithril-aggregator/src/file_uploaders/local_file_uploader.rs +++ b/mithril-aggregator/src/file_uploaders/local_uploader.rs @@ -10,8 +10,8 @@ use crate::file_uploaders::{FileLocation, FileUploader}; use crate::http_server; use crate::tools; -/// LocalFileUploader is a file uploader working using local files -pub struct LocalFileUploader { +/// LocalUploader is a file uploader working using local files +pub struct LocalUploader { /// File server listening IP file_server_url: String, @@ -21,11 +21,11 @@ pub struct LocalFileUploader { logger: Logger, } -impl LocalFileUploader { - /// LocalFileUploader factory +impl LocalUploader { + /// LocalUploader factory pub(crate) fn new(file_server_url: String, target_location: &Path, logger: Logger) -> Self { let logger = logger.new_with_component_name::(); - debug!(logger, "New LocalFileUploader created"; "file_server_url" => &file_server_url); + debug!(logger, "New LocalUploader created"; "file_server_url" => &file_server_url); Self { file_server_url, target_location: target_location.to_path_buf(), @@ -35,7 +35,7 @@ impl LocalFileUploader { } #[async_trait] -impl FileUploader for LocalFileUploader { +impl FileUploader for LocalUploader { async fn upload(&self, filepath: &Path) -> StdResult { let archive_name = filepath.file_name().unwrap().to_str().unwrap(); let target_path = &self.target_location.join(archive_name); @@ -69,7 +69,7 @@ mod tests { use crate::http_server; use crate::test_tools::TestLogger; - use super::LocalFileUploader; + use super::LocalUploader; fn create_fake_archive(dir: &Path, digest: &str) -> PathBuf { let file_path = dir.join(format!("test.{digest}.tar.gz")); @@ -96,7 +96,7 @@ mod tests { http_server::SERVER_BASE_PATH, &digest ); - let uploader = LocalFileUploader::new(url, target_dir.path(), TestLogger::stdout()); + let uploader = LocalUploader::new(url, target_dir.path(), TestLogger::stdout()); let location = uploader .upload(&archive) @@ -112,7 +112,7 @@ mod tests { let target_dir = tempdir().unwrap(); let digest = "41e27b9ed5a32531b95b2b7ff3c0757591a06a337efaf19a524a998e348028e7"; let archive = create_fake_archive(source_dir.path(), digest); - let uploader = LocalFileUploader::new( + let uploader = LocalUploader::new( "http://test.com:8080/".to_string(), target_dir.path(), TestLogger::stdout(), diff --git a/mithril-aggregator/src/file_uploaders/mod.rs b/mithril-aggregator/src/file_uploaders/mod.rs index 4f8279843fd..1148322dfd6 100644 --- a/mithril-aggregator/src/file_uploaders/mod.rs +++ b/mithril-aggregator/src/file_uploaders/mod.rs @@ -1,15 +1,15 @@ -mod dumb_file_uploader; -mod file_uploader; -mod gcp_file_uploader; -mod local_file_uploader; -mod remote_file_uploader; +mod dumb_uploader; +mod gcp_uploader; +mod interface; +mod local_uploader; +mod remote_uploader; -pub use dumb_file_uploader::*; -pub use file_uploader::FileLocation; -pub use file_uploader::FileUploader; -pub use gcp_file_uploader::GcpFileUploader; -pub use local_file_uploader::LocalFileUploader; -pub use remote_file_uploader::RemoteSnapshotUploader; +pub use dumb_uploader::*; +pub use gcp_uploader::GcpUploader; +pub use interface::FileLocation; +pub use interface::FileUploader; +pub use local_uploader::LocalUploader; +pub use remote_uploader::RemoteUploader; #[cfg(test)] -pub use file_uploader::MockFileUploader; +pub use interface::MockFileUploader; diff --git a/mithril-aggregator/src/file_uploaders/remote_file_uploader.rs b/mithril-aggregator/src/file_uploaders/remote_uploader.rs similarity index 79% rename from mithril-aggregator/src/file_uploaders/remote_file_uploader.rs rename to mithril-aggregator/src/file_uploaders/remote_uploader.rs index 4e84da006fe..f794107e2b1 100644 --- a/mithril-aggregator/src/file_uploaders/remote_file_uploader.rs +++ b/mithril-aggregator/src/file_uploaders/remote_uploader.rs @@ -7,17 +7,17 @@ use mithril_common::StdResult; use crate::file_uploaders::{FileLocation, FileUploader}; -/// RemoteSnapshotUploader is a snapshot uploader working using Google Cloud Platform services -pub struct RemoteSnapshotUploader { +/// RemoteUploader is a snapshot uploader working using Google Cloud Platform services +pub struct RemoteUploader { file_uploader: Box, logger: Logger, } -impl RemoteSnapshotUploader { - /// RemoteSnapshotUploader factory +impl RemoteUploader { + /// RemoteUploader factory pub fn new(file_uploader: Box, logger: Logger) -> Self { let logger = logger.new_with_component_name::(); - debug!(logger, "New RemoteSnapshotUploader created"); + debug!(logger, "New RemoteUploader created"); Self { file_uploader, logger, @@ -26,7 +26,7 @@ impl RemoteSnapshotUploader { } #[async_trait] -impl FileUploader for RemoteSnapshotUploader { +impl FileUploader for RemoteUploader { async fn upload(&self, snapshot_filepath: &Path) -> StdResult { let location = self.file_uploader.upload(snapshot_filepath).await?; debug!(self.logger, "Snapshot upload to remote storage completed"; "location" => &location); @@ -44,7 +44,7 @@ mod tests { use crate::file_uploaders::{FileUploader, MockFileUploader}; use crate::test_tools::TestLogger; - use super::RemoteSnapshotUploader; + use super::RemoteUploader; #[tokio::test] async fn upload_call_uploader_and_return_location() { @@ -54,8 +54,7 @@ mod tests { .with(eq(Path::new("test/snapshot.xxx.tar.gz"))) .times(1) .returning(|_| Ok("https://cdn.mithril.network/snapshot.xxx.tar.gz".to_string())); - let snapshot_uploader = - RemoteSnapshotUploader::new(Box::new(file_uploader), TestLogger::stdout()); + let snapshot_uploader = RemoteUploader::new(Box::new(file_uploader), TestLogger::stdout()); let filepath = Path::new("test/snapshot.xxx.tar.gz"); let expected_location = "https://cdn.mithril.network/snapshot.xxx.tar.gz".to_string(); @@ -73,8 +72,7 @@ mod tests { file_uploader .expect_upload() .returning(|_| Err(anyhow!("unexpected error"))); - let snapshot_uploader = - RemoteSnapshotUploader::new(Box::new(file_uploader), TestLogger::stdout()); + let snapshot_uploader = RemoteUploader::new(Box::new(file_uploader), TestLogger::stdout()); let snapshot_filepath = Path::new("test/snapshot.xxx.tar.gz"); let result = snapshot_uploader diff --git a/mithril-aggregator/src/lib.rs b/mithril-aggregator/src/lib.rs index aee76348939..b101ad9d6ac 100644 --- a/mithril-aggregator/src/lib.rs +++ b/mithril-aggregator/src/lib.rs @@ -38,9 +38,7 @@ pub use crate::configuration::{ pub use crate::multi_signer::{MultiSigner, MultiSignerImpl}; pub use commands::{CommandType, MainOpts}; pub use dependency_injection::DependencyContainer; -pub use file_uploaders::{ - DumbFileUploader, FileUploader, LocalFileUploader, RemoteSnapshotUploader, -}; +pub use file_uploaders::{DumbUploader, FileUploader, LocalUploader, RemoteUploader}; pub use message_adapters::{FromRegisterSignerAdapter, ToCertificatePendingMessageAdapter}; pub use metrics::*; pub use runtime::{ diff --git a/mithril-aggregator/tests/test_extensions/runtime_tester.rs b/mithril-aggregator/tests/test_extensions/runtime_tester.rs index c48548d75ce..4c707632a91 100644 --- a/mithril-aggregator/tests/test_extensions/runtime_tester.rs +++ b/mithril-aggregator/tests/test_extensions/runtime_tester.rs @@ -7,7 +7,7 @@ use mithril_aggregator::{ database::{record::SignedEntityRecord, repository::OpenMessageRepository}, dependency_injection::DependenciesBuilder, event_store::EventMessage, - AggregatorRuntime, Configuration, DependencyContainer, DumbFileUploader, DumbSnapshotter, + AggregatorRuntime, Configuration, DependencyContainer, DumbSnapshotter, DumbUploader, SignerRegistrationError, }; use mithril_common::{ @@ -100,7 +100,7 @@ macro_rules! assert_metrics_eq { pub struct RuntimeTester { pub network: String, - pub snapshot_uploader: Arc, + pub snapshot_uploader: Arc, pub chain_observer: Arc, pub immutable_file_observer: Arc, pub digester: Arc, @@ -130,7 +130,7 @@ impl RuntimeTester { let logger = build_logger(); let global_logger = slog_scope::set_global_logger(logger.clone()); let network = configuration.network.clone(); - let snapshot_uploader = Arc::new(DumbFileUploader::new()); + let snapshot_uploader = Arc::new(DumbUploader::new()); let immutable_file_observer = Arc::new(DumbImmutableFileObserver::new()); immutable_file_observer .shall_return(Some(start_time_point.immutable_file_number)) From 8d5e1c7c0502db6712038b64a467a39db395182d Mon Sep 17 00:00:00 2001 From: sfauvel Date: Wed, 11 Dec 2024 14:41:11 +0100 Subject: [PATCH 11/13] refactor: remove `RemoteUploader` --- .../src/dependency_injection/builder.rs | 15 ++-- mithril-aggregator/src/file_uploaders/mod.rs | 2 - .../src/file_uploaders/remote_uploader.rs | 84 ------------------- mithril-aggregator/src/lib.rs | 2 +- 4 files changed, 7 insertions(+), 96 deletions(-) delete mode 100644 mithril-aggregator/src/file_uploaders/remote_uploader.rs diff --git a/mithril-aggregator/src/dependency_injection/builder.rs b/mithril-aggregator/src/dependency_injection/builder.rs index 71f5b63c365..bf987fc4674 100644 --- a/mithril-aggregator/src/dependency_injection/builder.rs +++ b/mithril-aggregator/src/dependency_injection/builder.rs @@ -79,8 +79,8 @@ use crate::{ AggregatorConfig, AggregatorRunner, AggregatorRuntime, CompressedArchiveSnapshotter, Configuration, DependencyContainer, DumbSnapshotter, DumbUploader, EpochSettingsStorer, FileUploader, LocalUploader, MetricsService, MithrilSignerRegisterer, MultiSigner, - MultiSignerImpl, RemoteUploader, SingleSignatureAuthenticator, SnapshotUploaderType, - Snapshotter, SnapshotterCompressionAlgorithm, VerificationKeyStorer, + MultiSignerImpl, SingleSignatureAuthenticator, SnapshotUploaderType, Snapshotter, + SnapshotterCompressionAlgorithm, VerificationKeyStorer, }; const SQLITE_FILE: &str = "aggregator.sqlite3"; @@ -462,13 +462,10 @@ impl DependenciesBuilder { ) })?; - Ok(Arc::new(RemoteUploader::new( - Box::new(GcpUploader::new( - bucket, - self.configuration.snapshot_use_cdn_domain, - logger.clone(), - )), - logger, + Ok(Arc::new(GcpUploader::new( + bucket, + self.configuration.snapshot_use_cdn_domain, + logger.clone(), ))) } SnapshotUploaderType::Local => Ok(Arc::new(LocalUploader::new( diff --git a/mithril-aggregator/src/file_uploaders/mod.rs b/mithril-aggregator/src/file_uploaders/mod.rs index 1148322dfd6..8ec7ef0f704 100644 --- a/mithril-aggregator/src/file_uploaders/mod.rs +++ b/mithril-aggregator/src/file_uploaders/mod.rs @@ -2,14 +2,12 @@ mod dumb_uploader; mod gcp_uploader; mod interface; mod local_uploader; -mod remote_uploader; pub use dumb_uploader::*; pub use gcp_uploader::GcpUploader; pub use interface::FileLocation; pub use interface::FileUploader; pub use local_uploader::LocalUploader; -pub use remote_uploader::RemoteUploader; #[cfg(test)] pub use interface::MockFileUploader; diff --git a/mithril-aggregator/src/file_uploaders/remote_uploader.rs b/mithril-aggregator/src/file_uploaders/remote_uploader.rs deleted file mode 100644 index f794107e2b1..00000000000 --- a/mithril-aggregator/src/file_uploaders/remote_uploader.rs +++ /dev/null @@ -1,84 +0,0 @@ -use async_trait::async_trait; -use slog::{debug, Logger}; -use std::path::Path; - -use mithril_common::logging::LoggerExtensions; -use mithril_common::StdResult; - -use crate::file_uploaders::{FileLocation, FileUploader}; - -/// RemoteUploader is a snapshot uploader working using Google Cloud Platform services -pub struct RemoteUploader { - file_uploader: Box, - logger: Logger, -} - -impl RemoteUploader { - /// RemoteUploader factory - pub fn new(file_uploader: Box, logger: Logger) -> Self { - let logger = logger.new_with_component_name::(); - debug!(logger, "New RemoteUploader created"); - Self { - file_uploader, - logger, - } - } -} - -#[async_trait] -impl FileUploader for RemoteUploader { - async fn upload(&self, snapshot_filepath: &Path) -> StdResult { - let location = self.file_uploader.upload(snapshot_filepath).await?; - debug!(self.logger, "Snapshot upload to remote storage completed"; "location" => &location); - - Ok(location) - } -} - -#[cfg(test)] -mod tests { - use anyhow::anyhow; - use mockall::predicate::eq; - use std::path::Path; - - use crate::file_uploaders::{FileUploader, MockFileUploader}; - use crate::test_tools::TestLogger; - - use super::RemoteUploader; - - #[tokio::test] - async fn upload_call_uploader_and_return_location() { - let mut file_uploader = MockFileUploader::new(); - file_uploader - .expect_upload() - .with(eq(Path::new("test/snapshot.xxx.tar.gz"))) - .times(1) - .returning(|_| Ok("https://cdn.mithril.network/snapshot.xxx.tar.gz".to_string())); - let snapshot_uploader = RemoteUploader::new(Box::new(file_uploader), TestLogger::stdout()); - let filepath = Path::new("test/snapshot.xxx.tar.gz"); - let expected_location = "https://cdn.mithril.network/snapshot.xxx.tar.gz".to_string(); - - let location = snapshot_uploader - .upload(filepath) - .await - .expect("remote upload should not fail"); - - assert_eq!(expected_location, location); - } - - #[tokio::test] - async fn upload_return_error_when_uploader_error() { - let mut file_uploader = MockFileUploader::new(); - file_uploader - .expect_upload() - .returning(|_| Err(anyhow!("unexpected error"))); - let snapshot_uploader = RemoteUploader::new(Box::new(file_uploader), TestLogger::stdout()); - let snapshot_filepath = Path::new("test/snapshot.xxx.tar.gz"); - - let result = snapshot_uploader - .upload(snapshot_filepath) - .await - .expect_err("remote upload should fail"); - assert_eq!("unexpected error".to_string(), result.to_string()); - } -} diff --git a/mithril-aggregator/src/lib.rs b/mithril-aggregator/src/lib.rs index b101ad9d6ac..4d7c4c03d07 100644 --- a/mithril-aggregator/src/lib.rs +++ b/mithril-aggregator/src/lib.rs @@ -38,7 +38,7 @@ pub use crate::configuration::{ pub use crate::multi_signer::{MultiSigner, MultiSignerImpl}; pub use commands::{CommandType, MainOpts}; pub use dependency_injection::DependencyContainer; -pub use file_uploaders::{DumbUploader, FileUploader, LocalUploader, RemoteUploader}; +pub use file_uploaders::{DumbUploader, FileUploader, LocalUploader}; pub use message_adapters::{FromRegisterSignerAdapter, ToCertificatePendingMessageAdapter}; pub use metrics::*; pub use runtime::{ From 5904e295d78d519882901d69667dd0203718bf41 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Wed, 11 Dec 2024 15:58:13 +0100 Subject: [PATCH 12/13] refactor: Use a FileUri struct instead of a String for the location --- .../cardano_immutable_files_full.rs | 16 ++++++---- .../src/file_uploaders/dumb_uploader.rs | 16 +++++----- .../src/file_uploaders/gcp_uploader.rs | 30 +++++++++---------- .../src/file_uploaders/interface.rs | 12 ++++++-- .../src/file_uploaders/local_uploader.rs | 10 +++---- mithril-aggregator/src/file_uploaders/mod.rs | 2 +- 6 files changed, 48 insertions(+), 38 deletions(-) diff --git a/mithril-aggregator/src/artifact_builder/cardano_immutable_files_full.rs b/mithril-aggregator/src/artifact_builder/cardano_immutable_files_full.rs index 61424f64103..49a05488082 100644 --- a/mithril-aggregator/src/artifact_builder/cardano_immutable_files_full.rs +++ b/mithril-aggregator/src/artifact_builder/cardano_immutable_files_full.rs @@ -5,9 +5,7 @@ use slog::{debug, warn, Logger}; use std::sync::Arc; use thiserror::Error; -use crate::{ - file_uploaders::FileLocation, snapshotter::OngoingSnapshot, FileUploader, Snapshotter, -}; +use crate::{file_uploaders::FileUri, snapshotter::OngoingSnapshot, FileUploader, Snapshotter}; use super::ArtifactBuilder; use mithril_common::logging::LoggerExtensions; @@ -88,7 +86,7 @@ impl CardanoImmutableFilesFullArtifactBuilder { async fn upload_snapshot_archive( &self, ongoing_snapshot: &OngoingSnapshot, - ) -> StdResult> { + ) -> StdResult> { debug!(self.logger, ">> upload_snapshot_archive"); let location = self .snapshot_uploader @@ -157,7 +155,12 @@ impl ArtifactBuilder for CardanoImmutableFilesFullArt })?; let snapshot = self - .create_snapshot(beacon, &ongoing_snapshot, snapshot_digest, locations) + .create_snapshot( + beacon, + &ongoing_snapshot, + snapshot_digest, + locations.into_iter().map(Into::into).collect(), + ) .await?; Ok(snapshot) @@ -173,7 +176,7 @@ mod tests { use mithril_common::{entities::CompressionAlgorithm, test_utils::fake_data}; use crate::{ - file_uploaders::MockFileUploader, test_tools::TestLogger, DumbUploader, DumbSnapshotter, + file_uploaders::MockFileUploader, test_tools::TestLogger, DumbSnapshotter, DumbUploader, }; use super::*; @@ -211,6 +214,7 @@ mod tests { let remote_locations = vec![dumb_snapshot_uploader .get_last_upload() .unwrap() + .map(Into::into) .expect("A snapshot should have been 'uploaded'")]; let artifact_expected = Snapshot::new( snapshot_digest.to_owned(), diff --git a/mithril-aggregator/src/file_uploaders/dumb_uploader.rs b/mithril-aggregator/src/file_uploaders/dumb_uploader.rs index 6f3ff18392a..0ad7fef61b3 100644 --- a/mithril-aggregator/src/file_uploaders/dumb_uploader.rs +++ b/mithril-aggregator/src/file_uploaders/dumb_uploader.rs @@ -3,14 +3,14 @@ use async_trait::async_trait; use mithril_common::StdResult; use std::{path::Path, sync::RwLock}; -use super::{FileLocation, FileUploader}; +use crate::file_uploaders::{FileUploader, FileUri}; /// Dummy uploader for test purposes. /// /// It actually does NOT upload any file but remembers the last file it /// was asked to upload. This is intended to by used by integration tests. pub struct DumbUploader { - last_uploaded: RwLock>, + last_uploaded: RwLock>, } impl DumbUploader { @@ -22,13 +22,13 @@ impl DumbUploader { } /// Return the last upload that was triggered. - pub fn get_last_upload(&self) -> StdResult> { + pub fn get_last_upload(&self) -> StdResult> { let value = self .last_uploaded .read() .map_err(|e| anyhow!("Error while saving filepath location: {e}"))?; - Ok(value.as_ref().map(|v| v.to_string())) + Ok(value.as_ref().map(Clone::clone)) } } @@ -41,13 +41,13 @@ impl Default for DumbUploader { #[async_trait] impl FileUploader for DumbUploader { /// Upload a file - async fn upload(&self, filepath: &Path) -> StdResult { + async fn upload(&self, filepath: &Path) -> StdResult { let mut value = self .last_uploaded .write() .map_err(|e| anyhow!("Error while saving filepath location: {e}"))?; - let location = filepath.to_string_lossy().to_string(); + let location = FileUri(filepath.to_string_lossy().to_string()); *value = Some(location.clone()); Ok(location) @@ -69,9 +69,9 @@ mod tests { .upload(Path::new("/tmp/whatever")) .await .expect("uploading with a dumb uploader should not fail"); - assert_eq!(res, "/tmp/whatever".to_string()); + assert_eq!(res, FileUri("/tmp/whatever".to_string())); assert_eq!( - Some("/tmp/whatever".to_string()), + Some(FileUri("/tmp/whatever".to_string())), uploader .get_last_upload() .expect("getting dumb uploader last value after a fake download should not fail") diff --git a/mithril-aggregator/src/file_uploaders/gcp_uploader.rs b/mithril-aggregator/src/file_uploaders/gcp_uploader.rs index 2ffdd4c2572..a7f2824b2d1 100644 --- a/mithril-aggregator/src/file_uploaders/gcp_uploader.rs +++ b/mithril-aggregator/src/file_uploaders/gcp_uploader.rs @@ -6,13 +6,11 @@ use cloud_storage::{ }; use slog::{info, Logger}; use std::{env, path::Path}; -use tokio_util::{codec::BytesCodec, codec::FramedRead}; +use tokio_util::codec::{BytesCodec, FramedRead}; -use mithril_common::logging::LoggerExtensions; -use mithril_common::StdResult; +use mithril_common::{logging::LoggerExtensions, StdResult}; -use crate::file_uploaders::FileLocation; -use crate::FileUploader; +use crate::{file_uploaders::FileUri, FileUploader}; /// GcpUploader represents a Google Cloud Platform file uploader interactor pub struct GcpUploader { @@ -31,21 +29,21 @@ impl GcpUploader { } } - fn get_location(&self, filename: &str) -> String { - if self.use_cdn_domain { - format!("https://{}/{}", self.bucket, filename) - } else { - format!( - "https://storage.googleapis.com/{}/{}", - self.bucket, filename - ) + fn get_location(&self, filename: &str) -> FileUri { + let mut uri = vec![]; + if !self.use_cdn_domain { + uri.push("storage.googleapis.com"); } + uri.push(&self.bucket); + uri.push(filename); + + FileUri(format!("https://{}", uri.join("/"))) } } #[async_trait] impl FileUploader for GcpUploader { - async fn upload(&self, filepath: &Path) -> StdResult { + async fn upload(&self, filepath: &Path) -> StdResult { if env::var("GOOGLE_APPLICATION_CREDENTIALS_JSON").is_err() { return Err(anyhow!( "Missing GOOGLE_APPLICATION_CREDENTIALS_JSON environment variable".to_string() @@ -118,7 +116,7 @@ mod tests { let location = file_uploader.get_location(filename); - assert_eq!(expected_location, location); + assert_eq!(FileUri(expected_location), location); } #[tokio::test] @@ -135,6 +133,6 @@ mod tests { let location = file_uploader.get_location(filename); - assert_eq!(expected_location, location); + assert_eq!(FileUri(expected_location), location); } } diff --git a/mithril-aggregator/src/file_uploaders/interface.rs b/mithril-aggregator/src/file_uploaders/interface.rs index 34eb21913ea..a46334aa5b6 100644 --- a/mithril-aggregator/src/file_uploaders/interface.rs +++ b/mithril-aggregator/src/file_uploaders/interface.rs @@ -2,12 +2,20 @@ use async_trait::async_trait; use mithril_common::StdResult; use std::path::Path; -pub type FileLocation = String; +/// FileUri represents a file URI used to identify the file's location +#[derive(Debug, PartialEq, Clone)] +pub struct FileUri(pub String); + +impl From for String { + fn from(file_uri: FileUri) -> Self { + file_uri.0 + } +} /// FileUploader represents a file uploader interactor #[cfg_attr(test, mockall::automock)] #[async_trait] pub trait FileUploader: Sync + Send { /// Upload a file - async fn upload(&self, filepath: &Path) -> StdResult; + async fn upload(&self, filepath: &Path) -> StdResult; } diff --git a/mithril-aggregator/src/file_uploaders/local_uploader.rs b/mithril-aggregator/src/file_uploaders/local_uploader.rs index 2d85692199c..b6a2c7933f6 100644 --- a/mithril-aggregator/src/file_uploaders/local_uploader.rs +++ b/mithril-aggregator/src/file_uploaders/local_uploader.rs @@ -6,7 +6,7 @@ use std::path::{Path, PathBuf}; use mithril_common::logging::LoggerExtensions; use mithril_common::StdResult; -use crate::file_uploaders::{FileLocation, FileUploader}; +use crate::file_uploaders::{FileUploader, FileUri}; use crate::http_server; use crate::tools; @@ -36,7 +36,7 @@ impl LocalUploader { #[async_trait] impl FileUploader for LocalUploader { - async fn upload(&self, filepath: &Path) -> StdResult { + async fn upload(&self, filepath: &Path) -> StdResult { let archive_name = filepath.file_name().unwrap().to_str().unwrap(); let target_path = &self.target_location.join(archive_name); tokio::fs::copy(filepath, target_path) @@ -54,7 +54,7 @@ impl FileUploader for LocalUploader { ); debug!(self.logger, "File 'uploaded' to local storage"; "location" => &location); - Ok(location) + Ok(FileUri(location)) } } @@ -65,7 +65,7 @@ mod tests { use std::path::{Path, PathBuf}; use tempfile::tempdir; - use crate::file_uploaders::FileUploader; + use crate::file_uploaders::{FileUploader, FileUri}; use crate::http_server; use crate::test_tools::TestLogger; @@ -103,7 +103,7 @@ mod tests { .await .expect("local upload should not fail"); - assert_eq!(expected_location, location); + assert_eq!(FileUri(expected_location), location); } #[tokio::test] diff --git a/mithril-aggregator/src/file_uploaders/mod.rs b/mithril-aggregator/src/file_uploaders/mod.rs index 8ec7ef0f704..5a6e2a44e1e 100644 --- a/mithril-aggregator/src/file_uploaders/mod.rs +++ b/mithril-aggregator/src/file_uploaders/mod.rs @@ -5,8 +5,8 @@ mod local_uploader; pub use dumb_uploader::*; pub use gcp_uploader::GcpUploader; -pub use interface::FileLocation; pub use interface::FileUploader; +pub use interface::FileUri; pub use local_uploader::LocalUploader; #[cfg(test)] From 460c9afe8ac4c0ec2417df04880b363baa8072c2 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Wed, 11 Dec 2024 16:34:30 +0100 Subject: [PATCH 13/13] chore: upgrade crate versions * mithril-aggregator from `0.6.1` to `0.6.2` --- Cargo.lock | 2 +- mithril-aggregator/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3bfdfa77e21..40b6287013e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3586,7 +3586,7 @@ dependencies = [ [[package]] name = "mithril-aggregator" -version = "0.6.1" +version = "0.6.2" dependencies = [ "anyhow", "async-trait", diff --git a/mithril-aggregator/Cargo.toml b/mithril-aggregator/Cargo.toml index c44d6386400..48de3f7bc84 100644 --- a/mithril-aggregator/Cargo.toml +++ b/mithril-aggregator/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mithril-aggregator" -version = "0.6.1" +version = "0.6.2" description = "A Mithril Aggregator server" authors = { workspace = true } edition = { workspace = true }