diff --git a/Cargo.lock b/Cargo.lock index 4174dbd54f4..370a63c73e5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3331,7 +3331,7 @@ dependencies = [ [[package]] name = "mithril-aggregator" -version = "0.4.43" +version = "0.4.44" dependencies = [ "anyhow", "async-trait", @@ -3373,7 +3373,7 @@ dependencies = [ [[package]] name = "mithril-aggregator-fake" -version = "0.2.2" +version = "0.2.3" dependencies = [ "anyhow", "axum", @@ -3483,7 +3483,7 @@ dependencies = [ [[package]] name = "mithril-common" -version = "0.3.11" +version = "0.3.12" dependencies = [ "anyhow", "async-trait", @@ -3581,7 +3581,7 @@ dependencies = [ [[package]] name = "mithril-persistence" -version = "0.1.1" +version = "0.1.2" dependencies = [ "anyhow", "async-trait", diff --git a/internal/mithril-persistence/Cargo.toml b/internal/mithril-persistence/Cargo.toml index 3f61ed972e7..7acfda82a38 100644 --- a/internal/mithril-persistence/Cargo.toml +++ b/internal/mithril-persistence/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mithril-persistence" -version = "0.1.1" +version = "0.1.2" description = "Common types, interfaces, and utilities to persist data for Mithril nodes." authors = { workspace = true } edition = { workspace = true } diff --git a/internal/mithril-persistence/src/store/adapter/dumb_adapter.rs b/internal/mithril-persistence/src/store/adapter/dumb_adapter.rs index c7a9aff4c43..a01148c0beb 100644 --- a/internal/mithril-persistence/src/store/adapter/dumb_adapter.rs +++ b/internal/mithril-persistence/src/store/adapter/dumb_adapter.rs @@ -1,10 +1,12 @@ use super::{AdapterError, StoreAdapter}; +use anyhow::anyhow; use async_trait::async_trait; /// A [StoreAdapter] that store one fixed data record, for testing purpose. pub struct DumbStoreAdapter { last_key: Option, last_value: Option, + error: Option, } impl DumbStoreAdapter { @@ -13,6 +15,15 @@ impl DumbStoreAdapter { Self { last_key: None, last_value: None, + error: None, + } + } + + /// DumbStoreAdapter factory that returns error when 'get_record' is called. + pub fn new_failing_adapter(error: &str) -> Self { + Self { + error: Some(error.to_string()), + ..Self::new() } } } @@ -47,10 +58,15 @@ where } async fn get_record(&self, key: &Self::Key) -> Result, AdapterError> { - if self.record_exists(key).await? { - Ok(self.last_value.as_ref().cloned()) - } else { - Ok(None) + match &self.error { + Some(error) => Err(AdapterError::GeneralError(anyhow!(error.clone()))), + None => { + if self.record_exists(key).await? { + Ok(self.last_value.as_ref().cloned()) + } else { + Ok(None) + } + } } } @@ -208,4 +224,13 @@ mod tests { assert_eq!(0, records.count()); } + + #[tokio::test] + async fn test_return_error_calling_get_record() { + let adapter: DumbStoreAdapter = + DumbStoreAdapter::new_failing_adapter("error"); + let result = adapter.get_record(&"key".to_string()).await; + + assert!(result.is_err()); + } } diff --git a/mithril-aggregator/Cargo.toml b/mithril-aggregator/Cargo.toml index e7135de36bb..ba4502dfb34 100644 --- a/mithril-aggregator/Cargo.toml +++ b/mithril-aggregator/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mithril-aggregator" -version = "0.4.43" +version = "0.4.44" description = "A Mithril Aggregator server" authors = { workspace = true } edition = { workspace = true } diff --git a/mithril-aggregator/src/http_server/routes/artifact_routes/cardano_transaction.rs b/mithril-aggregator/src/http_server/routes/artifact_routes/cardano_transaction.rs index 6069ef47e61..70b7086b8f8 100644 --- a/mithril-aggregator/src/http_server/routes/artifact_routes/cardano_transaction.rs +++ b/mithril-aggregator/src/http_server/routes/artifact_routes/cardano_transaction.rs @@ -102,7 +102,10 @@ pub mod tests { }; use mithril_persistence::sqlite::HydrationError; use serde_json::Value::Null; - use warp::{http::Method, test::request}; + use warp::{ + http::{Method, StatusCode}, + test::request, + }; use super::*; @@ -150,7 +153,9 @@ pub mod tests { "application/json", &Null, &response, - ); + &StatusCode::OK, + ) + .unwrap(); } #[tokio::test] @@ -179,7 +184,9 @@ pub mod tests { "application/json", &Null, &response, - ); + &StatusCode::INTERNAL_SERVER_ERROR, + ) + .unwrap(); } #[tokio::test] @@ -216,11 +223,13 @@ pub mod tests { "application/json", &Null, &response, - ); + &StatusCode::OK, + ) + .unwrap(); } #[tokio::test] - async fn test_cardano_transaction_ok_norecord() { + async fn test_cardano_transaction_return_404_not_found_when_no_record() { let mut mock_http_message_service = MockMessageService::new(); mock_http_message_service .expect_get_cardano_transaction_message() @@ -245,7 +254,9 @@ pub mod tests { "application/json", &Null, &response, - ); + &StatusCode::NOT_FOUND, + ) + .unwrap(); } #[tokio::test] @@ -274,6 +285,8 @@ pub mod tests { "application/json", &Null, &response, - ); + &StatusCode::INTERNAL_SERVER_ERROR, + ) + .unwrap(); } } diff --git a/mithril-aggregator/src/http_server/routes/artifact_routes/mithril_stake_distribution.rs b/mithril-aggregator/src/http_server/routes/artifact_routes/mithril_stake_distribution.rs index eaf5be7d26a..88c174ca94f 100644 --- a/mithril-aggregator/src/http_server/routes/artifact_routes/mithril_stake_distribution.rs +++ b/mithril-aggregator/src/http_server/routes/artifact_routes/mithril_stake_distribution.rs @@ -103,7 +103,10 @@ pub mod tests { }; use mithril_persistence::sqlite::HydrationError; use serde_json::Value::Null; - use warp::{http::Method, test::request}; + use warp::{ + http::{Method, StatusCode}, + test::request, + }; use super::*; @@ -151,7 +154,9 @@ pub mod tests { "application/json", &Null, &response, - ); + &StatusCode::OK, + ) + .unwrap(); } #[tokio::test] @@ -180,7 +185,9 @@ pub mod tests { "application/json", &Null, &response, - ); + &StatusCode::INTERNAL_SERVER_ERROR, + ) + .unwrap(); } #[tokio::test] @@ -217,11 +224,13 @@ pub mod tests { "application/json", &Null, &response, - ); + &StatusCode::OK, + ) + .unwrap(); } #[tokio::test] - async fn test_mithril_stake_distribution_ok_norecord() { + async fn test_mithril_stake_distribution_returns_404_no_found_when_no_record() { let mut mock_http_message_service = MockMessageService::new(); mock_http_message_service .expect_get_mithril_stake_distribution_message() @@ -246,7 +255,9 @@ pub mod tests { "application/json", &Null, &response, - ); + &StatusCode::NOT_FOUND, + ) + .unwrap(); } #[tokio::test] @@ -275,6 +286,8 @@ pub mod tests { "application/json", &Null, &response, - ); + &StatusCode::INTERNAL_SERVER_ERROR, + ) + .unwrap(); } } diff --git a/mithril-aggregator/src/http_server/routes/artifact_routes/snapshot.rs b/mithril-aggregator/src/http_server/routes/artifact_routes/snapshot.rs index fc320229d4b..7de2f848f3b 100644 --- a/mithril-aggregator/src/http_server/routes/artifact_routes/snapshot.rs +++ b/mithril-aggregator/src/http_server/routes/artifact_routes/snapshot.rs @@ -234,7 +234,10 @@ mod tests { }; use mithril_persistence::sqlite::HydrationError; use serde_json::Value::Null; - use warp::{http::Method, test::request}; + use warp::{ + http::{Method, StatusCode}, + test::request, + }; use super::*; @@ -282,7 +285,9 @@ mod tests { "application/json", &Null, &response, - ); + &StatusCode::OK, + ) + .unwrap(); } #[tokio::test] @@ -311,7 +316,9 @@ mod tests { "application/json", &Null, &response, - ); + &StatusCode::INTERNAL_SERVER_ERROR, + ) + .unwrap(); } #[tokio::test] @@ -348,11 +355,13 @@ mod tests { "application/json", &Null, &response, - ); + &StatusCode::OK, + ) + .unwrap(); } #[tokio::test] - async fn test_snapshot_digest_get_ok_nosnapshot() { + async fn test_snapshot_digest_returns_404_not_found_when_no_snapshot() { let mut mock_http_message_service = MockMessageService::new(); mock_http_message_service .expect_get_snapshot_message() @@ -377,7 +386,9 @@ mod tests { "application/json", &Null, &response, - ); + &StatusCode::NOT_FOUND, + ) + .unwrap(); } #[tokio::test] @@ -406,11 +417,13 @@ mod tests { "application/json", &Null, &response, - ); + &StatusCode::INTERNAL_SERVER_ERROR, + ) + .unwrap(); } #[tokio::test] - async fn test_snapshot_download_get_ok() { + async fn test_snapshot_local_download_returns_302_found_when_the_snapshot_exists() { let signed_entity = create_signed_entities( SignedEntityType::CardanoImmutableFilesFull(Beacon::default()), fake_data::snapshots(1), @@ -435,18 +448,19 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( - APISpec::get_all_spec_files(), - method, - path, - "application/gzip", - &Null, - &response, + assert_eq!(response.status(), StatusCode::FOUND); + let location = std::str::from_utf8(response.headers()["location"].as_bytes()) + .unwrap() + .to_string(); + assert!( + location.contains(&format!("/{SERVER_BASE_PATH}/snapshot_download/testnet")), + "Expected value '/{SERVER_BASE_PATH}/snapshot_download/testnet' not found in {}", + location ); } #[tokio::test] - async fn test_snapshot_download_get_ok_nosnapshot() { + async fn test_snapshot_download_returns_404_not_found_when_no_snapshot() { let mut mock_signed_entity_service = MockSignedEntityService::new(); mock_signed_entity_service .expect_get_signed_snapshot_by_id() @@ -471,7 +485,9 @@ mod tests { "application/gzip", &Null, &response, - ); + &StatusCode::NOT_FOUND, + ) + .unwrap(); } #[tokio::test] @@ -500,6 +516,8 @@ mod tests { "application/json", &Null, &response, - ); + &StatusCode::INTERNAL_SERVER_ERROR, + ) + .unwrap(); } } diff --git a/mithril-aggregator/src/http_server/routes/certificate_routes.rs b/mithril-aggregator/src/http_server/routes/certificate_routes.rs index d233785da4e..cec2034b67a 100644 --- a/mithril-aggregator/src/http_server/routes/certificate_routes.rs +++ b/mithril-aggregator/src/http_server/routes/certificate_routes.rs @@ -121,12 +121,20 @@ mod handlers { #[cfg(test)] mod tests { use anyhow::anyhow; - use mithril_common::test_utils::{apispec::APISpec, fake_data}; + use mithril_common::{ + entities::CertificatePending, + test_utils::{apispec::APISpec, fake_data}, + }; + use mithril_persistence::store::adapter::DumbStoreAdapter; use serde_json::Value::Null; - use warp::{http::Method, test::request}; + use warp::{ + http::{Method, StatusCode}, + test::request, + }; use crate::{ - http_server::SERVER_BASE_PATH, initialize_dependencies, services::MockCertifierService, + http_server::SERVER_BASE_PATH, initialize_dependencies, services::MockMessageService, + CertificatePendingStore, }; use super::*; @@ -145,10 +153,23 @@ mod tests { } #[tokio::test] - async fn test_certificate_pending_get_ok() { + async fn test_certificate_pending_with_content_get_ok_200() { let method = Method::GET.as_str(); let path = "/certificate-pending"; let dependency_manager = initialize_dependencies().await; + let certificate_pending = { + let mut signers = fake_data::signers(3); + signers[0].party_id = "1".to_string(); + CertificatePending { + signers, + ..fake_data::certificate_pending() + } + }; + dependency_manager + .certificate_pending_store + .save(certificate_pending) + .await + .unwrap(); let response = request() .method(method) @@ -163,15 +184,16 @@ mod tests { "application/json", &Null, &response, - ); + &StatusCode::OK, + ) + .unwrap(); } #[tokio::test] - async fn test_certificate_pending_get_ok_204() { - let dependency_manager = initialize_dependencies().await; - + async fn test_certificate_pending_without_content_get_ok_204() { let method = Method::GET.as_str(); let path = "/certificate-pending"; + let dependency_manager = initialize_dependencies().await; let response = request() .method(method) @@ -186,14 +208,20 @@ mod tests { "application/json", &Null, &response, - ); + &StatusCode::NO_CONTENT, + ) + .unwrap(); } #[tokio::test] async fn test_certificate_pending_get_ko_500() { let method = Method::GET.as_str(); let path = "/certificate-pending"; - let dependency_manager = initialize_dependencies().await; + let mut dependency_manager = initialize_dependencies().await; + + let certificate_pending_store_store = + CertificatePendingStore::new(Box::new(DumbStoreAdapter::new_failing_adapter("error"))); + dependency_manager.certificate_pending_store = Arc::new(certificate_pending_store_store); let response = request() .method(method) @@ -208,7 +236,9 @@ mod tests { "application/json", &Null, &response, - ); + &StatusCode::INTERNAL_SERVER_ERROR, + ) + .unwrap(); } #[tokio::test] @@ -236,17 +266,19 @@ mod tests { "application/json", &Null, &response, - ); + &StatusCode::OK, + ) + .unwrap(); } #[tokio::test] - async fn test_certificate_certificates_get_ko() { + async fn test_certificate_when_error_retrieving_certificates_returns_ko_500() { let mut dependency_manager = initialize_dependencies().await; - let mut certifier_service = MockCertifierService::new(); - certifier_service - .expect_get_latest_certificates() + let mut message_service = MockMessageService::new(); + message_service + .expect_get_certificate_list_message() .returning(|_| Err(anyhow!("an error"))); - dependency_manager.certifier_service = Arc::new(certifier_service); + dependency_manager.message_service = Arc::new(message_service); let method = Method::GET.as_str(); let path = "/certificates"; @@ -264,7 +296,9 @@ mod tests { "application/json", &Null, &response, - ); + &StatusCode::INTERNAL_SERVER_ERROR, + ) + .unwrap(); } #[tokio::test] @@ -292,7 +326,9 @@ mod tests { "application/json", &Null, &response, - ); + &StatusCode::OK, + ) + .unwrap(); } #[tokio::test] @@ -315,24 +351,29 @@ mod tests { "application/json", &Null, &response, - ); + &StatusCode::NOT_FOUND, + ) + .unwrap(); } #[tokio::test] - async fn test_certificate_certificate_hash_get_ko() { + async fn test_certificate_when_error_on_retrieving_certificate_hash_returns_ko_500() { let mut dependency_manager = initialize_dependencies().await; - let mut certifier_service = MockCertifierService::new(); - certifier_service - .expect_get_certificate_by_hash() + let mut message_service = MockMessageService::new(); + message_service + .expect_get_certificate_message() .returning(|_| Err(anyhow!("an error"))); - dependency_manager.certifier_service = Arc::new(certifier_service); + dependency_manager.message_service = Arc::new(message_service); let method = Method::GET.as_str(); let path = "/certificate/{certificate_hash}"; let response = request() .method(method) - .path(&format!("/{SERVER_BASE_PATH}{path}")) + .path(&format!( + "/{SERVER_BASE_PATH}{}", + path.replace("{certificate_hash}", "whatever") + )) .reply(&setup_router(Arc::new(dependency_manager))) .await; @@ -343,6 +384,8 @@ mod tests { "application/json", &Null, &response, - ); + &StatusCode::INTERNAL_SERVER_ERROR, + ) + .unwrap(); } } diff --git a/mithril-aggregator/src/http_server/routes/epoch_routes.rs b/mithril-aggregator/src/http_server/routes/epoch_routes.rs index e3ddefa593d..a3176cf8271 100644 --- a/mithril-aggregator/src/http_server/routes/epoch_routes.rs +++ b/mithril-aggregator/src/http_server/routes/epoch_routes.rs @@ -60,14 +60,20 @@ mod handlers { #[cfg(test)] mod tests { - use crate::http_server::SERVER_BASE_PATH; - use mithril_common::test_utils::apispec::APISpec; + use mithril_common::{ + entities::Epoch, + test_utils::{apispec::APISpec, MithrilFixtureBuilder}, + }; use serde_json::Value::Null; - use warp::http::Method; + use tokio::sync::RwLock; + use warp::http::{Method, StatusCode}; use warp::test::request; - use super::*; + use crate::http_server::SERVER_BASE_PATH; use crate::initialize_dependencies; + use crate::services::FakeEpochService; + + use super::*; fn setup_router( dependency_manager: Arc, @@ -86,7 +92,10 @@ mod tests { async fn test_epoch_settings_get_ok() { let method = Method::GET.as_str(); let path = "/epoch-settings"; - let dependency_manager = initialize_dependencies().await; + let mut dependency_manager = initialize_dependencies().await; + let fixture = MithrilFixtureBuilder::default().with_signers(5).build(); + let epoch_service = FakeEpochService::from_fixture(Epoch(5), &fixture); + dependency_manager.epoch_service = Arc::new(RwLock::new(epoch_service)); let response = request() .method(method) @@ -101,7 +110,9 @@ mod tests { "application/json", &Null, &response, - ); + &StatusCode::OK, + ) + .unwrap(); } #[tokio::test] @@ -123,6 +134,8 @@ mod tests { "application/json", &Null, &response, - ); + &StatusCode::INTERNAL_SERVER_ERROR, + ) + .unwrap(); } } diff --git a/mithril-aggregator/src/http_server/routes/proof_routes.rs b/mithril-aggregator/src/http_server/routes/proof_routes.rs index 5357528bd36..1bdb6a8f326 100644 --- a/mithril-aggregator/src/http_server/routes/proof_routes.rs +++ b/mithril-aggregator/src/http_server/routes/proof_routes.rs @@ -112,7 +112,10 @@ mod tests { CardanoTransactionsSetProof, CardanoTransactionsSnapshot, SignedEntity, }; use serde_json::Value::Null; - use warp::{http::Method, test::request}; + use warp::{ + http::{Method, StatusCode}, + test::request, + }; use crate::services::MockSignedEntityService; use crate::{ @@ -168,7 +171,9 @@ mod tests { "application/json", &Null, &response, - ); + &StatusCode::OK, + ) + .unwrap(); } #[tokio::test] @@ -195,7 +200,9 @@ mod tests { "application/json", &Null, &response, - ); + &StatusCode::NOT_FOUND, + ) + .unwrap(); } #[tokio::test] @@ -227,6 +234,8 @@ mod tests { "application/json", &Null, &response, - ); + &StatusCode::INTERNAL_SERVER_ERROR, + ) + .unwrap(); } } diff --git a/mithril-aggregator/src/http_server/routes/root_routes.rs b/mithril-aggregator/src/http_server/routes/root_routes.rs index dcd9b2892a7..ad9f1862ec6 100644 --- a/mithril-aggregator/src/http_server/routes/root_routes.rs +++ b/mithril-aggregator/src/http_server/routes/root_routes.rs @@ -157,6 +157,8 @@ mod tests { "application/json", &Null, &response, - ); + &StatusCode::OK, + ) + .unwrap(); } } diff --git a/mithril-aggregator/src/http_server/routes/signatures_routes.rs b/mithril-aggregator/src/http_server/routes/signatures_routes.rs index 79ef15f0e53..8f8cdb69bd5 100644 --- a/mithril-aggregator/src/http_server/routes/signatures_routes.rs +++ b/mithril-aggregator/src/http_server/routes/signatures_routes.rs @@ -103,7 +103,7 @@ mod handlers { #[cfg(test)] mod tests { use anyhow::anyhow; - use warp::http::Method; + use warp::http::{Method, StatusCode}; use warp::test::request; use mithril_common::{ @@ -160,7 +160,9 @@ mod tests { "application/json", &message, &response, - ); + &StatusCode::CREATED, + ) + .unwrap(); } #[tokio::test] @@ -192,7 +194,9 @@ mod tests { "application/json", &message, &response, - ); + &StatusCode::BAD_REQUEST, + ) + .unwrap(); } #[tokio::test] @@ -225,7 +229,9 @@ mod tests { "application/json", &message, &response, - ); + &StatusCode::NOT_FOUND, + ) + .unwrap(); } #[tokio::test] @@ -258,7 +264,9 @@ mod tests { "application/json", &message, &response, - ); + &StatusCode::GONE, + ) + .unwrap(); } #[tokio::test] @@ -289,6 +297,8 @@ mod tests { "application/json", &message, &response, - ); + &StatusCode::INTERNAL_SERVER_ERROR, + ) + .unwrap(); } } diff --git a/mithril-aggregator/src/http_server/routes/signer_routes.rs b/mithril-aggregator/src/http_server/routes/signer_routes.rs index 3f9f988d353..40cf911bb85 100644 --- a/mithril-aggregator/src/http_server/routes/signer_routes.rs +++ b/mithril-aggregator/src/http_server/routes/signer_routes.rs @@ -254,7 +254,10 @@ mod tests { use mithril_persistence::store::adapter::AdapterError; use mockall::predicate::eq; use serde_json::Value::Null; - use warp::{http::Method, test::request}; + use warp::{ + http::{Method, StatusCode}, + test::request, + }; use crate::database::provider::{MockSignerGetter, SignerRecord}; use crate::{ @@ -310,7 +313,9 @@ mod tests { "application/json", &signer, &response, - ); + &StatusCode::CREATED, + ) + .unwrap(); } #[tokio::test] @@ -349,7 +354,9 @@ mod tests { "application/json", &signer, &response, - ); + &StatusCode::CREATED, + ) + .unwrap(); } #[tokio::test] @@ -387,7 +394,9 @@ mod tests { "application/json", &signer, &response, - ); + &StatusCode::BAD_REQUEST, + ) + .unwrap(); } #[tokio::test] @@ -424,7 +433,9 @@ mod tests { "application/json", &signer, &response, - ); + &StatusCode::INTERNAL_SERVER_ERROR, + ) + .unwrap(); } #[tokio::test] @@ -457,7 +468,9 @@ mod tests { "application/json", &signer, &response, - ); + &StatusCode::SERVICE_UNAVAILABLE, + ) + .unwrap(); } #[tokio::test] @@ -514,11 +527,13 @@ mod tests { "application/json", &Null, &response, - ); + &StatusCode::OK, + ) + .unwrap(); } #[tokio::test] - async fn test_registered_signers_get_ok_noregistration() { + async fn test_registered_signers_returns_404_not_found_when_no_registration() { let mut mock_verification_key_store = MockVerificationKeyStorer::new(); mock_verification_key_store .expect_get_signers() @@ -543,7 +558,9 @@ mod tests { "application/json", &Null, &response, - ); + &StatusCode::NOT_FOUND, + ) + .unwrap(); } #[tokio::test] @@ -571,7 +588,9 @@ mod tests { "application/json", &Null, &response, - ); + &StatusCode::INTERNAL_SERVER_ERROR, + ) + .unwrap(); } #[tokio::test] @@ -617,7 +636,9 @@ mod tests { "application/json", &Null, &response, - ); + &StatusCode::OK, + ) + .unwrap(); } #[tokio::test] @@ -646,6 +667,8 @@ mod tests { "application/json", &Null, &response, - ); + &StatusCode::INTERNAL_SERVER_ERROR, + ) + .unwrap(); } } diff --git a/mithril-aggregator/src/http_server/routes/statistics_routes.rs b/mithril-aggregator/src/http_server/routes/statistics_routes.rs index 0b3e2652101..4e9b7411f38 100644 --- a/mithril-aggregator/src/http_server/routes/statistics_routes.rs +++ b/mithril-aggregator/src/http_server/routes/statistics_routes.rs @@ -57,7 +57,10 @@ mod tests { use mithril_common::messages::SnapshotDownloadMessage; use mithril_common::test_utils::apispec::APISpec; - use warp::{http::Method, test::request}; + use warp::{ + http::{Method, StatusCode}, + test::request, + }; use crate::{ dependency_injection::DependenciesBuilder, http_server::SERVER_BASE_PATH, Configuration, @@ -94,15 +97,17 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + let result = APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, "application/json", &snapshot_download_message, &response, + &StatusCode::CREATED, ); let _ = rx.try_recv().unwrap(); + result.unwrap(); } } diff --git a/mithril-common/Cargo.toml b/mithril-common/Cargo.toml index abf1658992c..693806b17f7 100644 --- a/mithril-common/Cargo.toml +++ b/mithril-common/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mithril-common" -version = "0.3.11" +version = "0.3.12" description = "Common types, interfaces, and utilities for Mithril nodes." authors = { workspace = true } edition = { workspace = true } @@ -47,6 +47,7 @@ pallas-traverse = { version = "0.23.0", optional = true } rand_chacha = "0.3.1" rand_core = "0.6.4" rayon = "1.8.1" +reqwest = { version = "0.11.23", optional = true } semver = "1.0.21" serde = { version = "1.0.196", features = ["derive"] } serde_bytes = "0.11.14" @@ -119,7 +120,7 @@ allow_skip_signer_certification = [] # Enable all tests tools test_tools = ["apispec", "test_http_server", "random"] # Enable tools to helps validate conformity to an OpenAPI specification -apispec = ["dep:glob", "dep:jsonschema", "dep:warp"] +apispec = ["dep:glob", "dep:jsonschema", "dep:warp", "dep:reqwest"] test_http_server = ["dep:warp"] [package.metadata.docs.rs] diff --git a/mithril-common/src/test_utils/apispec.rs b/mithril-common/src/test_utils/apispec.rs index b370b1f7e4f..7982b9b28b5 100644 --- a/mithril-common/src/test_utils/apispec.rs +++ b/mithril-common/src/test_utils/apispec.rs @@ -2,9 +2,12 @@ use glob::glob; use jsonschema::JSONSchema; +use reqwest::Url; use serde::Serialize; use serde_json::{json, Value, Value::Null}; + use warp::http::Response; +use warp::http::StatusCode; use warp::hyper::body::Bytes; use crate::era::SupportedEra; @@ -26,19 +29,29 @@ impl<'a> APISpec<'a> { content_type: &str, request_body: &impl Serialize, response: &Response, - ) { + status_code: &StatusCode, + ) -> Result<(), String> { + if spec_files.is_empty() { + return Err( + "OpenAPI need a spec file to validate conformity. None were given.".to_string(), + ); + } + for spec_file in spec_files { - APISpec::from_file(&spec_file) + if let Err(e) = APISpec::from_file(&spec_file) .method(method) .path(path) .content_type(content_type) .validate_request(request_body) - .map_err(|e| panic!("OpenAPI invalid request in {spec_file}, reason: {e}\nresponse: {response:#?}")) - .unwrap() - .validate_response(response) - .map_err(|e| panic!("OpenAPI invalid response in {spec_file}, reason: {e}\nresponse: {response:#?}")) - .unwrap(); + .and_then(|api| api.validate_response(response)) + .and_then(|api| api.validate_status(response, status_code)) + { + return Err(format!( + "OpenAPI invalid response in {spec_file} on route {path}, reason: {e}\nresponse: {response:#?}" + )); + } } + Ok(()) } /// APISpec factory from spec @@ -72,29 +85,70 @@ impl<'a> APISpec<'a> { } /// Validates if a request is valid - pub fn validate_request( - &'a mut self, - request_body: &impl Serialize, - ) -> Result<&mut APISpec, String> { + fn validate_request(&'a self, request_body: &impl Serialize) -> Result<&APISpec, String> { let path = self.path.unwrap(); let method = self.method.unwrap().to_lowercase(); let content_type = self.content_type.unwrap(); - let request_schema = &mut self.openapi.clone()["paths"][path][method]["requestBody"] - ["content"][content_type]["schema"]; + let openapi_path_entry = path.split('?').next().unwrap(); + let operation_object = &self.openapi["paths"][openapi_path_entry][method]; + + self.validate_query_parameters(path, operation_object)?; + + let request_schema = &operation_object["requestBody"]["content"][content_type]["schema"]; let value = &json!(&request_body); self.validate_conformity(value, request_schema) } - /// Validates if a response is valid - pub fn validate_response( - &'a mut self, + fn validate_query_parameters( + &'a self, + path: &str, + operation_object: &Value, + ) -> Result<&APISpec, String> { + let fake_base_url = "http://0.0.0.1"; + let url = Url::parse(&format!("{}{}", fake_base_url, path)).unwrap(); + + check_query_parameter_limitations(&url, operation_object); + + let mut query_pairs = url.query_pairs(); + if let Some(parameter) = query_pairs.next() { + let spec_parameter = &operation_object["parameters"][0]; + let spec_parameter_name = &spec_parameter["name"].as_str().unwrap(); + let spec_parameter_in = &spec_parameter["in"].as_str().unwrap(); + if spec_parameter_in.eq(&"query") && spec_parameter_name.eq(¶meter.0) { + Ok(self) + } else { + Err(format!("Unexpected query parameter '{}'", parameter.0)) + } + } else { + Ok(self) + } + } + + /// Validates if the status is the expected one + fn validate_status( + &'a self, response: &Response, - ) -> Result<&mut APISpec, String> { + expected_status_code: &StatusCode, + ) -> Result<&APISpec, String> { + if expected_status_code.as_u16() != response.status().as_u16() { + return Err(format!( + "expected status code {} but was {}", + expected_status_code.as_u16(), + response.status().as_u16(), + )); + } + + Ok(self) + } + + /// Validates if a response is valid + fn validate_response(&'a self, response: &Response) -> Result<&APISpec, String> { let body = response.body(); let status = response.status(); let path = self.path.unwrap(); + let path = path.split('?').next().unwrap(); let method = self.method.unwrap().to_lowercase(); let content_type = self.content_type.unwrap(); let mut openapi = self.openapi.clone(); @@ -102,52 +156,65 @@ impl<'a> APISpec<'a> { let response_spec = { match &mut openapi["paths"][path][&method]["responses"] { Null => None, - response_spec => { + responses_spec => { let status_code = status.as_str(); - if response_spec.as_object().unwrap().contains_key(status_code) { - Some(&response_spec[status_code]) + if responses_spec + .as_object() + .unwrap() + .contains_key(status_code) + { + Some(&responses_spec[status_code]) } else { - Some(&response_spec["default"]) + Some(&responses_spec["default"]) } } } }; - match response_spec { Some(response_spec) => { - let response_schema = &mut response_spec.clone()["content"][content_type]["schema"]; + let response_schema = match &response_spec["content"] { + Null => &Null, + content => { + if content[content_type] == Null { + return Err(format!( + "Expected content type '{}' but spec is '{}'", + content_type, response_spec["content"], + )); + } + &content[content_type]["schema"] + } + }; if body.is_empty() { - match response_spec.as_object() { - Some(_) => match response_schema.as_object() { - Some(_) => Err("non empty body expected".to_string()), - None => Ok(self), - }, - None => Err("empty body expected".to_string()), + match response_schema.as_object() { + Some(_) => Err("Non empty body expected".to_string()), + None => Ok(self), } } else { - match &serde_json::from_slice(body) { - Ok(value) => self.validate_conformity(value, response_schema), - Err(_) => Err("non empty body expected".to_string()), + match response_schema.as_object() { + Some(_) => match &serde_json::from_slice(body) { + Ok(value) => self.validate_conformity(value, response_schema), + Err(_) => Err(format!("Expected a valid json but got: {body:?}")), + }, + None => Err(format!("Expected empty body but got: {body:?}")), } } } - None => Err(format!("unmatched path and method: {path} {method}")), + None => Err(format!( + "Unmatched path and method: {path} {}", + method.to_uppercase() + )), } } /// Validates conformity of a value against a schema - pub fn validate_conformity( - &'a mut self, - value: &Value, - schema: &mut Value, - ) -> Result<&mut APISpec, String> { + fn validate_conformity(&'a self, value: &Value, schema: &Value) -> Result<&APISpec, String> { match schema { Null => match value { Null => Ok(self), _ => Err(format!("Expected nothing but got: {value:?}")), }, _ => { - let schema = &mut schema.as_object_mut().unwrap().clone(); + let mut schema = schema.as_object().unwrap().clone(); let components = self.openapi["components"].clone(); schema.insert(String::from("components"), components); @@ -166,7 +233,7 @@ impl<'a> APISpec<'a> { } /// Get default spec file - pub fn get_defaut_spec_file() -> String { + pub fn get_default_spec_file() -> String { "../openapi.yaml".to_string() } @@ -177,8 +244,13 @@ impl<'a> APISpec<'a> { /// Get all spec files pub fn get_all_spec_files() -> Vec { + APISpec::get_all_spec_files_from("..") + } + + /// Get all spec files in the directory + pub fn get_all_spec_files_from(root_path: &str) -> Vec { let mut open_api_spec_files = Vec::new(); - for entry in glob("../openapi*.yaml").unwrap() { + for entry in glob(&format!("{}/openapi*.yaml", root_path)).unwrap() { let entry_path = entry.unwrap().to_str().unwrap().to_string(); open_api_spec_files.push(entry_path.clone()); open_api_spec_files.push(entry_path); @@ -188,6 +260,25 @@ impl<'a> APISpec<'a> { } } +// TODO: For now, it verifies only one parameter, +// should verify with multiple query parameters using an openapi.yaml file for test. +fn check_query_parameter_limitations(url: &Url, operation_object: &Value) { + if url.query_pairs().count() >= 2 { + panic!("This method does not work with multiple parameters"); + } + + if let Some(parameters) = operation_object["parameters"].as_array() { + let len = parameters + .iter() + .filter(|p| p["in"].eq("query")) + .collect::>() + .len(); + if len >= 2 { + panic!("This method does not work with multiple parameters"); + } + } +} + #[cfg(test)] mod tests { use warp::http::Method; @@ -198,95 +289,375 @@ mod tests { use crate::messages::{CertificatePendingMessage, SignerMessagePart}; use crate::test_utils::fake_data; + fn build_empty_response(status_code: u16) -> Response { + Response::builder() + .status(status_code) + .body(Bytes::new()) + .unwrap() + } + + fn build_json_response(status_code: u16, value: T) -> Response { + Response::builder() + .status(status_code) + .body(Bytes::from(json!(value).to_string().into_bytes())) + .unwrap() + } + + fn build_response(status_code: u16, content: &'static [u8]) -> Response { + Response::builder() + .status(status_code) + .body(Bytes::from_static(content)) + .unwrap() + } + #[test] - fn test_apispec_validate_ok() { - // Route exists and does not expect request body, but expects response - assert!(APISpec::from_file(&APISpec::get_defaut_spec_file()) + fn test_validate_a_response_without_body() { + APISpec::from_file(&APISpec::get_default_spec_file()) .method(Method::GET.as_str()) .path("/certificate-pending") .validate_request(&Null) .unwrap() - .validate_response(&Response::::new(Bytes::from( - json!(CertificatePendingMessage::dummy()) - .to_string() - .into_bytes(), - ))) - .is_ok()); - - // Route exists and expects request body, but does not expect response - assert!(APISpec::from_file(&APISpec::get_defaut_spec_file()) + .validate_response(&build_empty_response(204)) + .unwrap(); + } + + #[test] + fn test_validate_ok_when_request_without_body_and_expects_response() { + APISpec::from_file(&APISpec::get_default_spec_file()) + .method(Method::GET.as_str()) + .path("/certificate-pending") + .validate_request(&Null) + .unwrap() + .validate_response(&build_json_response( + 200, + CertificatePendingMessage::dummy(), + )) + .unwrap(); + } + + #[test] + fn test_validate_ok_when_request_with_body_and_expects_no_response() { + assert!(APISpec::from_file(&APISpec::get_default_spec_file()) .method(Method::POST.as_str()) .path("/register-signer") .validate_request(&SignerMessagePart::dummy()) .unwrap() .validate_response(&Response::::new(Bytes::new())) .is_err()); + } - // Route exists and matches default status code - let mut response = Response::::new(Bytes::from( - json!(&entities::InternalServerError::new( - "an error occurred".to_string(), - )) - .to_string() - .into_bytes(), - )); - *response.status_mut() = StatusCode::INTERNAL_SERVER_ERROR; - assert!(APISpec::from_file(&APISpec::get_defaut_spec_file()) + #[test] + fn test_validate_ok_when_response_match_default_status_code() { + // INTERNAL_SERVER_ERROR(500) is not one of the defined status code + // for this route, so it's the default response spec that is used. + let response = build_json_response( + StatusCode::INTERNAL_SERVER_ERROR.into(), + entities::InternalServerError::new("an error occurred".to_string()), + ); + + APISpec::from_file(&APISpec::get_default_spec_file()) .method(Method::POST.as_str()) .path("/register-signer") .validate_response(&response) - .is_ok()); + .unwrap(); + } + + #[test] + fn test_should_fail_when_the_status_code_is_not_the_expected_one() { + let response = build_json_response( + StatusCode::INTERNAL_SERVER_ERROR.into(), + entities::InternalServerError::new("an error occurred".to_string()), + ); + + let mut api_spec = APISpec::from_file(&APISpec::get_default_spec_file()); + let result = api_spec + .method(Method::GET.as_str()) + .path("/certificate-pending") + .validate_request(&Null) + .unwrap() + .validate_status(&response, &StatusCode::OK); + + assert!(result.is_err()); + assert_eq!( + result.err().unwrap().to_string(), + format!( + "expected status code {} but was {}", + StatusCode::OK.as_u16(), + StatusCode::INTERNAL_SERVER_ERROR.as_u16() + ) + ); + } + + #[test] + fn test_should_be_ok_when_the_status_code_is_the_right_one() { + let response = build_json_response( + StatusCode::INTERNAL_SERVER_ERROR.into(), + entities::InternalServerError::new("an error occurred".to_string()), + ); + + APISpec::from_file(&APISpec::get_default_spec_file()) + .method(Method::GET.as_str()) + .path("/certificate-pending") + .validate_request(&Null) + .unwrap() + .validate_status(&response, &StatusCode::INTERNAL_SERVER_ERROR) + .unwrap(); } #[test] - fn test_apispec_validate_errors() { - // Route does not exist - assert!(APISpec::from_file(&APISpec::get_defaut_spec_file()) + fn test_validate_returns_error_when_route_does_not_exist() { + let mut api_spec = APISpec::from_file(&APISpec::get_default_spec_file()); + let result = api_spec .method(Method::GET.as_str()) .path("/route-not-existing-in-openapi-spec") - .validate_response(&Response::::new(Bytes::from_static(b"abcdefgh"))) - .is_err()); + .validate_response(&build_response(200, b"abcdefgh")); + + assert!(result.is_err()); + assert_eq!( + result.err(), + Some("Unmatched path and method: /route-not-existing-in-openapi-spec GET".to_string()) + ); + } - // Route exists, but method does not - assert!(APISpec::from_file(&APISpec::get_defaut_spec_file()) + #[test] + fn test_validate_returns_error_when_route_exists_but_method_does_not() { + let mut api_spec = APISpec::from_file(&APISpec::get_default_spec_file()); + let result = api_spec .method(Method::OPTIONS.as_str()) .path("/certificate-pending") - .validate_response(&Response::::new(Bytes::from_static(b"abcdefgh"))) - .is_err()); + .validate_response(&build_response(200, b"abcdefgh")); - // Route exists, but expects non empty reponse - assert!(APISpec::from_file(&APISpec::get_defaut_spec_file()) + assert!(result.is_err()); + assert_eq!( + result.err(), + Some("Unmatched path and method: /certificate-pending OPTIONS".to_string()) + ); + } + #[test] + fn test_validate_returns_error_when_route_exists_but_expects_non_empty_response() { + let mut api_spec = APISpec::from_file(&APISpec::get_default_spec_file()); + let result = api_spec .method(Method::GET.as_str()) .path("/certificate-pending") - .validate_response(&Response::::new(Bytes::new())) - .is_err()); + .validate_response(&build_empty_response(200)); - // Route exists, but expects empty reponse - assert!(APISpec::from_file(&APISpec::get_defaut_spec_file()) - .method(Method::POST.as_str()) - .path("/register-signer") - .validate_response(&Response::::new(Bytes::from_static(b"abcdefgh"))) - .is_err()); + assert!(result.is_err()); + assert_eq!(result.err(), Some("Non empty body expected".to_string())); + } + + #[test] + fn test_validate_returns_error_when_route_exists_but_expects_empty_response() { + { + let mut api_spec = APISpec::from_file(&APISpec::get_default_spec_file()); + let result = api_spec + .method(Method::POST.as_str()) + .path("/register-signer") + .validate_response(&build_response(201, b"abcdefgh")); + + assert!(result.is_err()); + assert_eq!( + result.err(), + Some("Expected empty body but got: b\"abcdefgh\"".to_string()) + ); + } + { + let mut api_spec = APISpec::from_file(&APISpec::get_default_spec_file()); + let result = api_spec + .method(Method::POST.as_str()) + .path("/register-signer") + .validate_response(&build_json_response(201, "something")); + + assert!(result.is_err()); + assert_eq!( + result.err(), + Some("Expected empty body but got: b\"\\\"something\\\"\"".to_string()) + ); + } + } - // Route exists, but does not expect request body - assert!(APISpec::from_file(&APISpec::get_defaut_spec_file()) + #[test] + fn test_validate_returns_error_when_json_is_not_valid() { + let mut api_spec = APISpec::from_file(&APISpec::get_default_spec_file()); + let result = api_spec + .method(Method::GET.as_str()) + .path("/certificate-pending") + .validate_request(&Null) + .unwrap() + .validate_response(&build_response(200, b"not a json")); + assert_eq!( + result.err(), + Some("Expected a valid json but got: b\"not a json\"".to_string()) + ); + } + + #[test] + fn test_validate_returns_errors_when_route_exists_but_does_not_expect_request_body() { + assert!(APISpec::from_file(&APISpec::get_default_spec_file()) .method(Method::GET.as_str()) .path("/certificate-pending") .validate_request(&fake_data::beacon()) .is_err()); - - // Route exists, but expects non empty request body - assert!(APISpec::from_file(&APISpec::get_defaut_spec_file()) + } + #[test] + fn test_validate_returns_error_when_route_exists_but_expects_non_empty_request_body() { + assert!(APISpec::from_file(&APISpec::get_default_spec_file()) .method(Method::POST.as_str()) .path("/register-signer") .validate_request(&Null) .is_err()); } + #[test] + fn test_validate_returns_error_when_content_type_does_not_exist() { + let mut api_spec = APISpec::from_file(&APISpec::get_default_spec_file()); + let result = api_spec + .method(Method::GET.as_str()) + .path("/certificate-pending") + .content_type("whatever") + .validate_request(&Null) + .unwrap() + .validate_response(&build_empty_response(200)); + + assert!(result.is_err()); + assert_eq!( + result.err().unwrap().to_string(), + "Expected content type 'whatever' but spec is '{\"application/json\":{\"schema\":{\"$ref\":\"#/components/schemas/CertificatePendingMessage\"}}}'", + ); + } + + #[test] + fn test_validate_a_response_with_query_parameters() { + APISpec::from_file(&APISpec::get_default_spec_file()) + .method(Method::GET.as_str()) + .path("/proof/cardano-transaction?transaction_hashes={hash}") + .validate_request(&Null) + .unwrap() + .validate_response(&build_empty_response(404)) + .map(|_apispec| ()) + .unwrap(); + } + + #[test] + fn test_validate_a_request_with_wrong_query_parameter_name() { + let mut api_spec = APISpec::from_file(&APISpec::get_default_spec_file()); + let result = api_spec + .method(Method::GET.as_str()) + .path("/proof/cardano-transaction?whatever=123") + .validate_request(&Null); + + assert!(result.is_err()); + assert_eq!( + result.err().unwrap().to_string(), + "Unexpected query parameter 'whatever'", + ); + } + + #[test] + fn test_validate_a_request_should_failed_when_query_parameter_is_in_path() { + let mut api_spec = APISpec::from_file(&APISpec::get_default_spec_file()); + let result = api_spec + .method(Method::GET.as_str()) + .path("/artifact/cardano-transaction/{hash}?hash=456") + .validate_request(&Null); + + assert!(result.is_err()); + assert_eq!( + result.err().unwrap().to_string(), + "Unexpected query parameter 'hash'", + ); + } + + #[test] + fn test_validate_query_parameters_with_correct_parameter_name() { + let api_spec = APISpec::from_file(&APISpec::get_default_spec_file()); + api_spec + .validate_query_parameters( + "/proof/cardano-transaction?transaction_hashes=123", + &api_spec.openapi["paths"]["/proof/cardano-transaction"]["get"], + ) + .map(|_apispec| ()) + .unwrap() + } + + #[test] + fn test_validate_query_parameters_with_wrong_query_parameter_name() { + let api_spec = APISpec::from_file(&APISpec::get_default_spec_file()); + let result = api_spec.validate_query_parameters( + "/proof/cardano-transaction?whatever=123", + &api_spec.openapi["paths"]["/proof/cardano-transaction"]["get"], + ); + + assert!(result.is_err()); + assert_eq!( + result.err().unwrap().to_string(), + "Unexpected query parameter 'whatever'", + ); + } + + #[test] + fn test_verify_conformity_with_expected_status() { + APISpec::verify_conformity( + APISpec::get_all_spec_files(), + Method::GET.as_str(), + "/certificate-pending", + "application/json", + &Null, + &build_json_response(200, CertificatePendingMessage::dummy()), + &StatusCode::OK, + ) + .unwrap() + } + + #[test] + fn test_verify_conformity_with_non_expected_status_returns_error() { + let response = build_json_response(200, CertificatePendingMessage::dummy()); + + let spec_file = APISpec::get_default_spec_file(); + let result = APISpec::verify_conformity( + vec![spec_file.clone()], + Method::GET.as_str(), + "/certificate-pending", + "application/json", + &Null, + &response, + &StatusCode::BAD_REQUEST, + ); + + let error_reason = format!( + "expected status code {} but was {}", + StatusCode::BAD_REQUEST.as_u16(), + StatusCode::OK.as_u16() + ); + let error_message = format!( + "OpenAPI invalid response in {spec_file} on route /certificate-pending, reason: {error_reason}\nresponse: {response:#?}" + ); + assert!(result.is_err()); + assert_eq!(result.err().unwrap().to_string(), error_message); + } + + #[test] + fn test_verify_conformity_when_no_spec_file_returns_error() { + let result = APISpec::verify_conformity( + vec![], + Method::GET.as_str(), + "/certificate-pending", + "application/json", + &Null, + &build_json_response(200, CertificatePendingMessage::dummy()), + &StatusCode::OK, + ); + + assert!(result.is_err()); + assert_eq!( + result.err().unwrap().to_string(), + "OpenAPI need a spec file to validate conformity. None were given." + ); + } + #[test] fn test_get_all_spec_files_not_empty() { let spec_files = APISpec::get_all_spec_files(); assert!(!spec_files.is_empty()); - assert!(spec_files.contains(&APISpec::get_defaut_spec_file())) + assert!(spec_files.contains(&APISpec::get_default_spec_file())) } } diff --git a/mithril-test-lab/mithril-aggregator-fake/Cargo.toml b/mithril-test-lab/mithril-aggregator-fake/Cargo.toml index d1c414ef021..dd4c612a299 100644 --- a/mithril-test-lab/mithril-aggregator-fake/Cargo.toml +++ b/mithril-test-lab/mithril-aggregator-fake/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mithril-aggregator-fake" -version = "0.2.2" +version = "0.2.3" description = "Mithril Fake Aggregator for client testing" authors = { workspace = true } documentation = { workspace = true } diff --git a/mithril-test-lab/mithril-aggregator-fake/src/application.rs b/mithril-test-lab/mithril-aggregator-fake/src/application.rs index 3fe10482be5..e445595cbe7 100644 --- a/mithril-test-lab/mithril-aggregator-fake/src/application.rs +++ b/mithril-test-lab/mithril-aggregator-fake/src/application.rs @@ -96,6 +96,7 @@ mod tests { time::sleep, }; use warp::http::{Response, StatusCode}; + use warp::hyper::body::Bytes; use mithril_common::test_utils::apispec::APISpec; @@ -124,30 +125,43 @@ mod tests { .unwrap(); } + async fn into_response(response: reqwest::Response) -> Response { + Response::builder() + .status(response.status()) + .body(response.bytes().await.unwrap()) + .unwrap() + } + + async fn http_request(port: u16, path: &str) -> Response { + let url = BASE_URL.replace("PORT", &port.to_string()); + let response = reqwest::get(&format!("{url}{path}")).await.unwrap(); + into_response(response).await + } + + fn get_spec_files() -> Vec { + APISpec::get_all_spec_files_from("../..") + } + #[tokio::test] async fn get_certificates() { const PORT: u16 = 3001; let task = tokio::spawn(async move { - // Yield back to Tokio's scheduler to ensure the web server is ready before going - // on. + // Yield back to Tokio's scheduler to ensure the web server is ready before going on. yield_now().await; let path = "/certificates"; - let url = BASE_URL.replace("PORT", &PORT.to_string()); - let response = reqwest::get(&format!("{url}{path}")).await.unwrap(); - - assert_eq!(StatusCode::OK, response.status()); + let response = http_request(PORT, path).await; APISpec::verify_conformity( - APISpec::get_all_spec_files(), + get_spec_files(), "GET", path, "application/json", &Null, - &Response::new(response.bytes().await.unwrap()), - ); - - Ok(()) + &response, + &StatusCode::OK, + ) + .map_err(|e| anyhow!(e)) }); test(task, PORT).await; @@ -157,26 +171,22 @@ mod tests { async fn get_snapshots() { const PORT: u16 = 3002; let task = tokio::spawn(async move { - // Yield back to Tokio's scheduler to ensure the web server is ready before going - // on. + // Yield back to Tokio's scheduler to ensure the web server is ready before going on. yield_now().await; let path = "/artifact/snapshots"; - let url = BASE_URL.replace("PORT", &PORT.to_string()); - let response = reqwest::get(&format!("{url}{path}")).await.unwrap(); - - assert_eq!(StatusCode::OK, response.status()); + let response = http_request(PORT, path).await; APISpec::verify_conformity( - APISpec::get_all_spec_files(), + get_spec_files(), "GET", path, "application/json", &Null, - &Response::new(response.bytes().await.unwrap()), - ); - - Ok(()) + &response, + &StatusCode::OK, + ) + .map_err(|e| anyhow!(e)) }); test(task, PORT).await; @@ -186,26 +196,22 @@ mod tests { async fn get_msds() { const PORT: u16 = 3003; let task = tokio::spawn(async move { - // Yield back to Tokio's scheduler to ensure the web server is ready before going - // on. + // Yield back to Tokio's scheduler to ensure the web server is ready before going on. yield_now().await; let path = "/artifact/mithril-stake-distributions"; - let url = BASE_URL.replace("PORT", &PORT.to_string()); - let response = reqwest::get(&format!("{url}{path}")).await.unwrap(); - - assert_eq!(StatusCode::OK, response.status()); + let response = http_request(PORT, path).await; APISpec::verify_conformity( - APISpec::get_all_spec_files(), + get_spec_files(), "GET", path, "application/json", &Null, - &Response::new(response.bytes().await.unwrap()), - ); - - Ok(()) + &response, + &StatusCode::OK, + ) + .map_err(|e| anyhow!(e)) }); test(task, PORT).await; @@ -215,32 +221,24 @@ mod tests { async fn get_certificate() { const PORT: u16 = 3004; let task = tokio::spawn(async move { - // Yield back to Tokio's scheduler to ensure the web server is ready before going - // on. + // Yield back to Tokio's scheduler to ensure the web server is ready before going on. yield_now().await; let path = "/certificate/{certificate_hash}"; - let url = BASE_URL.replace("PORT", &PORT.to_string()); let certificate_hash = default_values::certificate_hashes()[0]; - let response = reqwest::get(&format!( - "{url}{}", - path.replace("{certificate_hash}", certificate_hash) - )) - .await - .unwrap(); - - assert_eq!(StatusCode::OK, response.status()); + let response = + http_request(PORT, &path.replace("{certificate_hash}", certificate_hash)).await; APISpec::verify_conformity( - APISpec::get_all_spec_files(), + get_spec_files(), "GET", path, "application/json", &Null, - &Response::new(response.bytes().await.unwrap()), - ); - - Ok(()) + &response, + &StatusCode::OK, + ) + .map_err(|e| anyhow!(e)) }); test(task, PORT).await; @@ -250,17 +248,23 @@ mod tests { async fn get_no_certificate() { const PORT: u16 = 3005; let task = tokio::spawn(async move { - // Yield back to Tokio's scheduler to ensure the web server is ready before going - // on. + // Yield back to Tokio's scheduler to ensure the web server is ready before going on. yield_now().await; - let path = "/certificate/whatever"; - let url = BASE_URL.replace("PORT", &PORT.to_string()); - let response = reqwest::get(&format!("{url}{path}")).await.unwrap(); - - assert_eq!(StatusCode::NOT_FOUND, response.status()); + let path = "/certificate/{certificate_hash}"; + let response = + http_request(PORT, &path.replace("{certificate_hash}", "whatever")).await; - Ok(()) + APISpec::verify_conformity( + get_spec_files(), + "GET", + path, + "application/json", + &Null, + &response, + &StatusCode::NOT_FOUND, + ) + .map_err(|e| anyhow!(e)) }); test(task, PORT).await; @@ -270,29 +274,23 @@ mod tests { async fn get_snapshot() { const PORT: u16 = 3006; let task = tokio::spawn(async move { - // Yield back to Tokio's scheduler to ensure the web server is ready before going - // on. + // Yield back to Tokio's scheduler to ensure the web server is ready before going on. yield_now().await; let path = "/artifact/snapshot/{digest}"; let digest = default_values::snapshot_digests()[0]; - let url = BASE_URL.replace("PORT", &PORT.to_string()); - let response = reqwest::get(&format!("{url}{}", path.replace("{digest}", digest))) - .await - .unwrap(); - - assert_eq!(StatusCode::OK, response.status()); + let response = http_request(PORT, path.replace("{digest}", digest).as_str()).await; APISpec::verify_conformity( - APISpec::get_all_spec_files(), + get_spec_files(), "GET", path, "application/json", &Null, - &Response::new(response.bytes().await.unwrap()), - ); - - Ok(()) + &response, + &StatusCode::OK, + ) + .map_err(|e| anyhow!(e)) }); test(task, PORT).await; @@ -302,20 +300,23 @@ mod tests { async fn get_no_snapshot() { const PORT: u16 = 3007; let task = tokio::spawn(async move { - // Yield back to Tokio's scheduler to ensure the web server is ready before going - // on. + // Yield back to Tokio's scheduler to ensure the web server is ready before going on. yield_now().await; let path = "/artifact/snapshot/{digest}"; let digest = "whatever"; - let url = BASE_URL.replace("PORT", &PORT.to_string()); - let response = reqwest::get(&format!("{url}{}", path.replace("{digest}", digest))) - .await - .unwrap(); - - assert_eq!(StatusCode::NOT_FOUND, response.status()); + let response = http_request(PORT, &path.replace("{digest}", digest)).await; - Ok(()) + APISpec::verify_conformity( + get_spec_files(), + "GET", + path, + "application/json", + &Null, + &response, + &StatusCode::NOT_FOUND, + ) + .map_err(|e| anyhow!(e)) }); test(task, PORT).await; @@ -325,29 +326,23 @@ mod tests { async fn get_msd() { const PORT: u16 = 3008; let task = tokio::spawn(async move { - // Yield back to Tokio's scheduler to ensure the web server is ready before going - // on. + // Yield back to Tokio's scheduler to ensure the web server is ready before going on. yield_now().await; let path = "/artifact/mithril-stake-distribution/{hash}"; let hash = default_values::msd_hashes()[0]; - let url = BASE_URL.replace("PORT", &PORT.to_string()); - let response = reqwest::get(&format!("{url}{}", path.replace("{hash}", hash))) - .await - .unwrap(); - - assert_eq!(StatusCode::OK, response.status()); + let response = http_request(PORT, &path.replace("{hash}", hash)).await; APISpec::verify_conformity( - APISpec::get_all_spec_files(), + get_spec_files(), "GET", path, "application/json", &Null, - &Response::new(response.bytes().await.unwrap()), - ); - - Ok(()) + &response, + &StatusCode::OK, + ) + .map_err(|e| anyhow!(e)) }); test(task, PORT).await; @@ -357,17 +352,22 @@ mod tests { async fn get_no_msd() { const PORT: u16 = 3009; let task = tokio::spawn(async move { - // Yield back to Tokio's scheduler to ensure the web server is ready before going - // on. + // Yield back to Tokio's scheduler to ensure the web server is ready before going on. yield_now().await; - let path = "/artifact/mithril_stake_distribution/whatever"; - let url = BASE_URL.replace("PORT", &PORT.to_string()); - let response = reqwest::get(&format!("{url}{path}")).await.unwrap(); - - assert_eq!(StatusCode::NOT_FOUND, response.status()); + let path = "/artifact/mithril-stake-distribution/{hash}"; + let response = http_request(PORT, &path.replace("{hash}", "whatever")).await; - Ok(()) + APISpec::verify_conformity( + get_spec_files(), + "GET", + path, + "application/json", + &Null, + &response, + &StatusCode::NOT_FOUND, + ) + .map_err(|e| anyhow!(e)) }); test(task, PORT).await; @@ -377,26 +377,22 @@ mod tests { async fn get_epoch_settings() { const PORT: u16 = 3010; let task = tokio::spawn(async move { - // Yield back to Tokio's scheduler to ensure the web server is ready before going - // on. + // Yield back to Tokio's scheduler to ensure the web server is ready before going on. yield_now().await; let path = "/epoch-settings"; - let url = BASE_URL.replace("PORT", &PORT.to_string()); - let response = reqwest::get(&format!("{url}{path}")).await.unwrap(); - - assert_eq!(StatusCode::OK, response.status()); + let response = http_request(PORT, path).await; APISpec::verify_conformity( - APISpec::get_all_spec_files(), + get_spec_files(), "GET", path, "application/json", &Null, - &Response::new(response.bytes().await.unwrap()), - ); - - Ok(()) + &response, + &StatusCode::OK, + ) + .map_err(|e| anyhow!(e)) }); test(task, PORT).await; @@ -406,29 +402,23 @@ mod tests { async fn get_ctx_snapshot() { const PORT: u16 = 3011; let task = tokio::spawn(async move { - // Yield back to Tokio's scheduler to ensure the web server is ready before going - // on. + // Yield back to Tokio's scheduler to ensure the web server is ready before going on. yield_now().await; let path = "/artifact/cardano-transaction/{hash}"; let hash = default_values::ctx_snapshot_hashes()[0]; - let url = BASE_URL.replace("PORT", &PORT.to_string()); - let response = reqwest::get(&format!("{url}{}", path.replace("{hash}", hash))) - .await - .unwrap(); - - assert_eq!(StatusCode::OK, response.status()); + let response = http_request(PORT, &path.replace("{hash}", hash)).await; APISpec::verify_conformity( - APISpec::get_all_spec_files(), + get_spec_files(), "GET", path, "application/json", &Null, - &Response::new(response.bytes().await.unwrap()), - ); - - Ok(()) + &response, + &StatusCode::OK, + ) + .map_err(|e| anyhow!(e)) }); test(task, PORT).await; @@ -438,20 +428,23 @@ mod tests { async fn get_no_ctx_snapshot() { const PORT: u16 = 3012; let task = tokio::spawn(async move { - // Yield back to Tokio's scheduler to ensure the web server is ready before going - // on. + // Yield back to Tokio's scheduler to ensure the web server is ready before going on. yield_now().await; let path = "/artifact/cardano-transaction/{hash}"; let hash = "whatever"; - let url = BASE_URL.replace("PORT", &PORT.to_string()); - let response = reqwest::get(&format!("{url}{}", path.replace("{hash}", hash))) - .await - .unwrap(); + let response = http_request(PORT, &path.replace("{hash}", hash)).await; - assert_eq!(StatusCode::NOT_FOUND, response.status()); - - Ok(()) + APISpec::verify_conformity( + get_spec_files(), + "GET", + path, + "application/json", + &Null, + &response, + &StatusCode::NOT_FOUND, + ) + .map_err(|e| anyhow!(e)) }); test(task, PORT).await; @@ -461,29 +454,23 @@ mod tests { async fn get_ctx_proof() { const PORT: u16 = 3013; let task = tokio::spawn(async move { - // Yield back to Tokio's scheduler to ensure the web server is ready before going - // on. + // Yield back to Tokio's scheduler to ensure the web server is ready before going on. yield_now().await; let path = "/proof/cardano-transaction?transaction_hashes={hash}"; let hash = default_values::proof_transaction_hashes()[0]; - let url = BASE_URL.replace("PORT", &PORT.to_string()); - let response = reqwest::get(&format!("{url}{}", path.replace("{hash}", hash))) - .await - .unwrap(); - - assert_eq!(StatusCode::OK, response.status()); + let response = http_request(PORT, &path.replace("{hash}", hash)).await; APISpec::verify_conformity( - APISpec::get_all_spec_files(), + get_spec_files(), "GET", path, "application/json", &Null, - &Response::new(response.bytes().await.unwrap()), - ); - - Ok(()) + &response, + &StatusCode::OK, + ) + .map_err(|e| anyhow!(e)) }); test(task, PORT).await; @@ -493,17 +480,22 @@ mod tests { async fn get_no_ctx_proof() { const PORT: u16 = 3014; let task = tokio::spawn(async move { - // Yield back to Tokio's scheduler to ensure the web server is ready before going - // on. + // Yield back to Tokio's scheduler to ensure the web server is ready before going on. yield_now().await; let path = "/proof/cardano-transaction"; - let url = BASE_URL.replace("PORT", &PORT.to_string()); - let response = reqwest::get(&format!("{url}{}", path)).await.unwrap(); - - assert_eq!(StatusCode::NOT_FOUND, response.status()); + let response = http_request(PORT, path).await; - Ok(()) + APISpec::verify_conformity( + get_spec_files(), + "GET", + path, + "application/json", + &Null, + &response, + &StatusCode::NOT_FOUND, + ) + .map_err(|e| anyhow!(e)) }); test(task, PORT).await; diff --git a/mithril-test-lab/mithril-aggregator-fake/src/error.rs b/mithril-test-lab/mithril-aggregator-fake/src/error.rs index 7089703d41e..fbe7f95eb5b 100644 --- a/mithril-test-lab/mithril-aggregator-fake/src/error.rs +++ b/mithril-test-lab/mithril-aggregator-fake/src/error.rs @@ -3,7 +3,7 @@ use axum::{ http::{Response, StatusCode}, response::IntoResponse, }; -use tracing::{debug, error}; +use tracing::error; /// error that wraps `anyhow::Error`. #[derive(Debug)] @@ -11,8 +11,8 @@ pub enum AppError { /// Catching anyhow errors Internal(anyhow::Error), - /// Resource not found (specify what) - NotFound(String), + /// Resource not found + NotFound, } /// Tell axum how to convert `AppError` into a response. @@ -24,15 +24,7 @@ impl IntoResponse for AppError { (StatusCode::INTERNAL_SERVER_ERROR, format!("Error: {:?}", e)).into_response() } - Self::NotFound(resource) => { - debug!("{resource} NOT FOUND."); - - ( - StatusCode::NOT_FOUND, - format!("resource '{resource}' not found"), - ) - .into_response() - } + Self::NotFound => (StatusCode::NOT_FOUND).into_response(), } } } diff --git a/mithril-test-lab/mithril-aggregator-fake/src/handlers.rs b/mithril-test-lab/mithril-aggregator-fake/src/handlers.rs index b1e75b03b76..3a62acb7cea 100644 --- a/mithril-test-lab/mithril-aggregator-fake/src/handlers.rs +++ b/mithril-test-lab/mithril-aggregator-fake/src/handlers.rs @@ -15,6 +15,7 @@ use tower_http::{ trace::{DefaultMakeSpan, DefaultOnRequest, DefaultOnResponse, TraceLayer}, LatencyUnit, }; +use tracing::debug; use tracing::Level; use crate::shared_state::SharedState; @@ -71,7 +72,10 @@ pub async fn snapshot( .get_snapshot(&key) .await? .map(|s| s.into_response()) - .ok_or_else(|| AppError::NotFound(format!("snapshot digest={key}"))) + .ok_or_else(|| { + debug!("snapshot digest={key} NOT FOUND."); + AppError::NotFound + }) } /// HTTP: return the list of snapshots. @@ -101,7 +105,10 @@ pub async fn msd( .get_msd(&key) .await? .map(|s| s.into_response()) - .ok_or_else(|| AppError::NotFound(format!("mithril stake distribution epoch={key}"))) + .ok_or_else(|| { + debug!("mithril stake distribution epoch={key} NOT FOUND."); + AppError::NotFound + }) } /// HTTP: return the list of certificates @@ -123,7 +130,10 @@ pub async fn certificate( .get_certificate(&key) .await? .map(|s| s.into_response()) - .ok_or_else(|| AppError::NotFound(format!("certificate hash={key}"))) + .ok_or_else(|| { + debug!("certificate hash={key} NOT FOUND."); + AppError::NotFound + }) } /// HTTP: return the list of certificates @@ -145,7 +155,10 @@ pub async fn ctx_snapshot( .get_ctx_snapshot(&key) .await? .map(|s| s.into_response()) - .ok_or_else(|| AppError::NotFound(format!("ctx snapshot hash={key}"))) + .ok_or_else(|| { + debug!("ctx snapshot hash={key} NOT FOUND."); + AppError::NotFound + }) } #[derive(serde::Deserialize, Default)] @@ -166,7 +179,11 @@ pub async fn ctx_proof( .await? .map(|s| s.into_response()) .ok_or_else(|| { - AppError::NotFound(format!("ctx proof tx_hash={}", params.transaction_hashes)) + debug!( + "ctx proof ctx_hash={} NOT FOUND.", + params.transaction_hashes + ); + AppError::NotFound }) } @@ -210,7 +227,7 @@ mod tests { "The handler was expected to fail since the snapshot's digest does not exist.", ); - assert!(matches!(error, AppError::NotFound(_))); + assert!(matches!(error, AppError::NotFound)); } #[tokio::test] @@ -234,7 +251,7 @@ mod tests { .await .expect_err("The handler was expected to fail since the msd's hash does not exist."); - assert!(matches!(error, AppError::NotFound(_))); + assert!(matches!(error, AppError::NotFound)); } #[tokio::test] @@ -258,7 +275,7 @@ mod tests { "The handler was expected to fail since the certificate's hash does not exist.", ); - assert!(matches!(error, AppError::NotFound(_))); + assert!(matches!(error, AppError::NotFound)); } #[tokio::test] @@ -282,7 +299,7 @@ mod tests { "The handler was expected to fail since the ctx snapshot's hash does not exist.", ); - assert!(matches!(error, AppError::NotFound(_))); + assert!(matches!(error, AppError::NotFound)); } #[tokio::test] @@ -305,7 +322,7 @@ mod tests { .await .expect_err("The handler was expected to fail since no transaction hash was provided."); - assert!(matches!(error, AppError::NotFound(_))); + assert!(matches!(error, AppError::NotFound)); } #[tokio::test] @@ -322,7 +339,7 @@ mod tests { .await .expect_err("The handler was expected to fail since the ctx proof's hash does not exist."); - assert!(matches!(error, AppError::NotFound(_))); + assert!(matches!(error, AppError::NotFound)); } #[tokio::test]