Skip to content

Commit f1c0d13

Browse files
AlenarAlenar
committed
refactor(aggregator): use RouterConfig for config values in http server
Instead of relying on the `Configuration` struct that contains raw, unvalidated, data. This allow to simplify some tests since we don't have to provide for example string literals but directly the concrete type in some cases.
1 parent cb233ec commit f1c0d13

File tree

9 files changed

+132
-122
lines changed

9 files changed

+132
-122
lines changed

‎mithril-aggregator/src/dependency_injection/builder.rs‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1410,7 +1410,6 @@ impl DependenciesBuilder {
14101410
pub async fn build_dependency_container(&mut self) -> Result<DependencyContainer> {
14111411
let dependency_manager = DependencyContainer {
14121412
config: self.configuration.clone(),
1413-
allowed_discriminants: self.get_allowed_signed_entity_types_discriminants()?,
14141413
root_logger: self.root_logger(),
14151414
sqlite_connection: self.get_sqlite_connection().await?,
14161415
sqlite_connection_cardano_transaction_pool: self
@@ -1494,7 +1493,22 @@ impl DependenciesBuilder {
14941493
&mut self,
14951494
) -> Result<impl Filter<Extract = (impl warp::Reply,), Error = warp::Rejection> + Clone> {
14961495
let dependency_container = Arc::new(self.build_dependency_container().await?);
1497-
let router_state = RouterState::new(dependency_container.clone(), RouterConfig {});
1496+
let router_state = RouterState::new(
1497+
dependency_container.clone(),
1498+
RouterConfig {
1499+
network: self.configuration.get_network()?,
1500+
server_url: self.configuration.get_server_url(),
1501+
allowed_discriminants: self.get_allowed_signed_entity_types_discriminants()?,
1502+
cardano_transactions_prover_max_hashes_allowed_by_request: self
1503+
.configuration
1504+
.cardano_transactions_prover_max_hashes_allowed_by_request,
1505+
cardano_transactions_signing_config: self
1506+
.configuration
1507+
.cardano_transactions_signing_config
1508+
.clone(),
1509+
snapshot_directory: self.configuration.snapshot_directory.clone(),
1510+
},
1511+
);
14981512

14991513
Ok(router::routes(Arc::new(router_state)))
15001514
}

‎mithril-aggregator/src/dependency_injection/containers.rs‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use slog::Logger;
2-
use std::{collections::BTreeSet, sync::Arc};
2+
use std::sync::Arc;
33
use tokio::sync::RwLock;
44

55
use mithril_common::{
@@ -10,8 +10,8 @@ use mithril_common::{
1010
crypto_helper::ProtocolGenesisVerifier,
1111
digesters::{ImmutableDigester, ImmutableFileObserver},
1212
entities::{
13-
CardanoTransactionsSigningConfig, Epoch, ProtocolParameters, SignedEntityTypeDiscriminants,
14-
SignerWithStake, StakeDistribution,
13+
CardanoTransactionsSigningConfig, Epoch, ProtocolParameters, SignerWithStake,
14+
StakeDistribution,
1515
},
1616
era::{EraChecker, EraReader},
1717
signable_builder::SignableBuilderService,
@@ -52,9 +52,6 @@ pub struct DependencyContainer {
5252
/// Configuration structure.
5353
pub config: Configuration,
5454

55-
/// List of signed entity discriminants that are allowed to be processed
56-
pub allowed_discriminants: BTreeSet<SignedEntityTypeDiscriminants>,
57-
5855
/// Application root logger
5956
pub root_logger: Logger,
6057

‎mithril-aggregator/src/http_server/routes/artifact_routes/snapshot.rs‎

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -45,18 +45,20 @@ fn snapshot_download(
4545
warp::path!("artifact" / "snapshot" / String / "download")
4646
.and(warp::get().or(warp::head()).unify())
4747
.and(middlewares::with_logger(router_state))
48-
.and(middlewares::with_config(router_state))
48+
.and(middlewares::extract_config(router_state, |config| {
49+
config.server_url.clone()
50+
}))
4951
.and(middlewares::with_signed_entity_service(router_state))
5052
.and_then(handlers::snapshot_download)
5153
}
5254

5355
fn serve_snapshots_dir(
5456
router_state: &RouterState,
5557
) -> impl Filter<Extract = (impl warp::Reply,), Error = warp::Rejection> + Clone {
56-
let config = router_state.dependencies.config.clone();
57-
5858
warp::path("snapshot_download")
59-
.and(warp::fs::dir(config.snapshot_directory))
59+
.and(warp::fs::dir(
60+
router_state.configuration.snapshot_directory.clone(),
61+
))
6062
.and(middlewares::with_logger(router_state))
6163
.and(middlewares::with_signed_entity_service(router_state))
6264
.and_then(handlers::ensure_downloaded_file_is_a_snapshot)
@@ -94,8 +96,8 @@ mod handlers {
9496
use crate::http_server::routes::reply;
9597
use crate::http_server::SERVER_BASE_PATH;
9698
use crate::services::MessageService;
99+
use crate::services::SignedEntityService;
97100
use crate::MetricsService;
98-
use crate::{services::SignedEntityService, Configuration};
99101
use slog::{debug, warn, Logger};
100102
use std::convert::Infallible;
101103
use std::str::FromStr;
@@ -187,7 +189,7 @@ mod handlers {
187189
pub async fn snapshot_download(
188190
digest: String,
189191
logger: Logger,
190-
config: Configuration,
192+
server_url: String,
191193
signed_entity_service: Arc<dyn SignedEntityService>,
192194
) -> Result<impl warp::Reply, Infallible> {
193195
match signed_entity_service
@@ -204,12 +206,8 @@ mod handlers {
204206
snapshot.digest,
205207
snapshot.compression_algorithm.tar_file_extension()
206208
);
207-
let snapshot_uri = format!(
208-
"{}{}/snapshot_download/{}",
209-
config.get_server_url(),
210-
SERVER_BASE_PATH,
211-
filename
212-
);
209+
let snapshot_uri =
210+
format!("{server_url}{SERVER_BASE_PATH}/snapshot_download/{filename}",);
213211
let snapshot_uri = Uri::from_str(&snapshot_uri).unwrap();
214212

215213
Ok(Box::new(warp::redirect::found(snapshot_uri)) as Box<dyn warp::Reply>)
@@ -228,7 +226,9 @@ mod handlers {
228226

229227
#[cfg(test)]
230228
mod tests {
229+
use super::*;
231230
use crate::http_server::routes::artifact_routes::test_utils::*;
231+
use crate::http_server::routes::router::RouterConfig;
232232
use crate::{
233233
http_server::SERVER_BASE_PATH,
234234
initialize_dependencies,
@@ -248,8 +248,6 @@ mod tests {
248248
test::request,
249249
};
250250

251-
use super::*;
252-
253251
fn setup_router(
254252
state: RouterState,
255253
) -> impl Filter<Extract = (impl warp::Reply,), Error = warp::Rejection> + Clone {
@@ -488,22 +486,27 @@ mod tests {
488486

489487
let method = Method::GET.as_str();
490488
let path = "/artifact/snapshot/{digest}/download";
489+
let server_url = "https://1.2.3.4:5552/".to_string();
491490

492491
let response = request()
493492
.method(method)
494493
.path(&format!("/{SERVER_BASE_PATH}{path}"))
495-
.reply(&setup_router(RouterState::new_with_dummy_config(Arc::new(
496-
dependency_manager,
497-
))))
494+
.reply(&setup_router(RouterState::new(
495+
Arc::new(dependency_manager),
496+
RouterConfig {
497+
server_url: server_url.clone(),
498+
..RouterConfig::dummy()
499+
},
500+
)))
498501
.await;
499502

500503
assert_eq!(response.status(), StatusCode::FOUND);
501504
let location = std::str::from_utf8(response.headers()["location"].as_bytes())
502505
.unwrap()
503506
.to_string();
504507
assert!(
505-
location.contains(&format!("/{SERVER_BASE_PATH}/snapshot_download/{network}")),
506-
"Expected value '/{SERVER_BASE_PATH}/snapshot_download/testnet' not found in {location}",
508+
location.contains(&format!("{server_url}{SERVER_BASE_PATH}/snapshot_download/{network}")),
509+
"Expected value '{server_url}{SERVER_BASE_PATH}/snapshot_download/testnet' not found in {location}",
507510
);
508511
}
509512

‎mithril-aggregator/src/http_server/routes/epoch_routes.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,9 @@ fn epoch_settings(
2525
.and(warp::get())
2626
.and(middlewares::with_logger(router_state))
2727
.and(middlewares::with_epoch_service(router_state))
28-
.and(middlewares::with_allowed_signed_entity_type_discriminants(
29-
router_state,
30-
))
28+
.and(middlewares::extract_config(router_state, |config| {
29+
config.allowed_discriminants.clone()
30+
}))
3131
.and_then(handlers::epoch_settings)
3232
}
3333

‎mithril-aggregator/src/http_server/routes/middlewares.rs‎

Lines changed: 13 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,30 @@
11
use slog::{debug, Logger};
2-
use std::collections::BTreeSet;
32
use std::convert::Infallible;
43
use std::sync::Arc;
54
use warp::Filter;
65

76
use mithril_common::api_version::APIVersionProvider;
8-
use mithril_common::entities::SignedEntityTypeDiscriminants;
97

108
use crate::database::repository::SignerGetter;
119
use crate::dependency_injection::EpochServiceWrapper;
1210
use crate::event_store::{EventMessage, TransmitterService};
1311
use crate::http_server::routes::http_server_child_logger;
14-
use crate::http_server::routes::router::RouterState;
12+
use crate::http_server::routes::router::{RouterConfig, RouterState};
1513
use crate::services::{CertifierService, MessageService, ProverService, SignedEntityService};
1614
use crate::{
17-
CertificatePendingStore, Configuration, MetricsService, SignerRegisterer,
18-
SingleSignatureAuthenticator, VerificationKeyStorer,
15+
CertificatePendingStore, MetricsService, SignerRegisterer, SingleSignatureAuthenticator,
16+
VerificationKeyStorer,
1917
};
2018

19+
/// Extract a value from the configuration
20+
pub fn extract_config<D: Clone + Send>(
21+
state: &RouterState,
22+
extract: fn(&RouterConfig) -> D,
23+
) -> impl Filter<Extract = (D,), Error = Infallible> + Clone {
24+
let config_value = extract(&state.configuration);
25+
warp::any().map(move || config_value.clone())
26+
}
27+
2128
/// With logger middleware
2229
pub(crate) fn with_logger(
2330
router_state: &RouterState,
@@ -68,22 +75,6 @@ pub fn with_signer_getter(
6875
warp::any().map(move || signer_getter.clone())
6976
}
7077

71-
/// With config middleware
72-
pub fn with_config(
73-
router_state: &RouterState,
74-
) -> impl Filter<Extract = (Configuration,), Error = Infallible> + Clone {
75-
let config = router_state.dependencies.config.clone();
76-
warp::any().map(move || config.clone())
77-
}
78-
79-
/// With allowed signed entity discriminants middleware
80-
pub fn with_allowed_signed_entity_type_discriminants(
81-
router_state: &RouterState,
82-
) -> impl Filter<Extract = (BTreeSet<SignedEntityTypeDiscriminants>,), Error = Infallible> + Clone {
83-
let allowed_discriminants = router_state.dependencies.allowed_discriminants.clone();
84-
warp::any().map(move || allowed_discriminants.clone())
85-
}
86-
8778
/// With Event transmitter middleware
8879
pub fn with_event_transmitter(
8980
router_state: &RouterState,
@@ -177,8 +168,7 @@ pub mod validators {
177168
router_state: &RouterState,
178169
) -> impl Filter<Extract = (ProverTransactionsHashValidator,), Error = Infallible> + Clone {
179170
let max_hashes = router_state
180-
.dependencies
181-
.config
171+
.configuration
182172
.cardano_transactions_prover_max_hashes_allowed_by_request;
183173

184174
warp::any().map(move || ProverTransactionsHashValidator::new(max_hashes))

‎mithril-aggregator/src/http_server/routes/proof_routes.rs‎

Lines changed: 6 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -154,10 +154,7 @@ mod tests {
154154
test_utils::{apispec::APISpec, assert_equivalent, fake_data},
155155
};
156156

157-
use crate::{
158-
dependency_injection::DependenciesBuilder, http_server::SERVER_BASE_PATH,
159-
services::MockProverService, Configuration,
160-
};
157+
use crate::{http_server::SERVER_BASE_PATH, services::MockProverService};
161158
use crate::{initialize_dependencies, services::MockSignedEntityService};
162159

163160
use super::*;
@@ -250,9 +247,7 @@ mod tests {
250247

251248
#[tokio::test]
252249
async fn proof_cardano_transaction_ok() {
253-
let config = Configuration::new_sample();
254-
let mut builder = DependenciesBuilder::new_with_stdout_logger(config);
255-
let mut dependency_manager = builder.build_dependency_container().await.unwrap();
250+
let mut dependency_manager = initialize_dependencies().await;
256251
let mut mock_signed_entity_service = MockSignedEntityService::new();
257252
mock_signed_entity_service
258253
.expect_get_last_cardano_transaction_snapshot()
@@ -294,9 +289,7 @@ mod tests {
294289

295290
#[tokio::test]
296291
async fn proof_cardano_transaction_not_found() {
297-
let config = Configuration::new_sample();
298-
let mut builder = DependenciesBuilder::new_with_stdout_logger(config);
299-
let dependency_manager = builder.build_dependency_container().await.unwrap();
292+
let dependency_manager = initialize_dependencies().await;
300293

301294
let method = Method::GET.as_str();
302295
let path = "/proof/cardano-transaction";
@@ -327,9 +320,7 @@ mod tests {
327320

328321
#[tokio::test]
329322
async fn proof_cardano_transaction_ko() {
330-
let config = Configuration::new_sample();
331-
let mut builder = DependenciesBuilder::new_with_stdout_logger(config);
332-
let mut dependency_manager = builder.build_dependency_container().await.unwrap();
323+
let mut dependency_manager = initialize_dependencies().await;
333324
let mut mock_signed_entity_service = MockSignedEntityService::new();
334325
mock_signed_entity_service
335326
.expect_get_last_cardano_transaction_snapshot()
@@ -365,9 +356,7 @@ mod tests {
365356

366357
#[tokio::test]
367358
async fn proof_cardano_transaction_return_bad_request_with_invalid_hashes() {
368-
let config = Configuration::new_sample();
369-
let mut builder = DependenciesBuilder::new_with_stdout_logger(config);
370-
let dependency_manager = builder.build_dependency_container().await.unwrap();
359+
let dependency_manager = initialize_dependencies().await;
371360

372361
let method = Method::GET.as_str();
373362
let path = "/proof/cardano-transaction";
@@ -397,9 +386,7 @@ mod tests {
397386
#[tokio::test]
398387
async fn proof_cardano_transaction_route_deduplicate_hashes() {
399388
let tx = fake_data::transaction_hashes()[0].to_string();
400-
let config = Configuration::new_sample();
401-
let mut builder = DependenciesBuilder::new_with_stdout_logger(config);
402-
let mut dependency_manager = builder.build_dependency_container().await.unwrap();
389+
let mut dependency_manager = initialize_dependencies().await;
403390
let mut mock_signed_entity_service = MockSignedEntityService::new();
404391
mock_signed_entity_service
405392
.expect_get_last_cardano_transaction_snapshot()

0 commit comments

Comments
 (0)