From 1c85697eea367bfbfc87280fb9cd890e3c1e70c2 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Mon, 26 Feb 2024 08:43:13 +0100 Subject: [PATCH 01/29] Add a `validate_status` method --- mithril-common/src/test_utils/apispec.rs | 141 +++++++++++++++++++++++ 1 file changed, 141 insertions(+) diff --git a/mithril-common/src/test_utils/apispec.rs b/mithril-common/src/test_utils/apispec.rs index b370b1f7e4f..f001aa11f2e 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; +// TODO APISpec is used only for test in modules but we need to add this dependency to expose it. +// Can we use the feature "test_tools" ? 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; @@ -86,6 +89,50 @@ impl<'a> APISpec<'a> { self.validate_conformity(value, request_schema) } + /// Validates if the status is the expected one + pub fn validate_status( + &'a mut self, + response: &Response, + expected_status_code: &StatusCode, + ) -> Result<&mut 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(), + )); + } + + return Ok(self); + } + + /// Validates if a response is valid + pub fn validate_response_and_status( + &'a mut self, + response: &Response, + expected_status_code: &StatusCode, + ) -> Result<&mut APISpec, String> { + self.validate_status(response, expected_status_code) + .and_then(|s| s.validate_response(response)) + + // // Return ???? + // let result = self.validate_status(response, expected_status_code); + // // Recreate Err because it need a Result<&mut APISpec, String> when Result<&APISpec, String> is return + // // Can we do in a better way ? + // if let Err(err) = result { + // return Err(err); + // } + + // // 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(), + // // )); + // // } + + // return self.validate_response(response); + } /// Validates if a response is valid pub fn validate_response( &'a mut self, @@ -238,6 +285,100 @@ mod tests { .is_ok()); } + #[test] + fn test_apispec_should_fail_when_the_status_code_is_not_the_expected_one() { + // 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; + { + let mut api_spec = APISpec::from_file(&APISpec::get_defaut_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() + ) + ); + } + { + let mut api_spec = APISpec::from_file(&APISpec::get_defaut_spec_file()); + let result = api_spec + .method(Method::GET.as_str()) + .path("/certificate-pending") + .validate_request(&Null) + .unwrap() + .validate_response_and_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_apispec_should_be_ok_when_the_status_code_is_the_right_one() { + // 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; + + // Is it better to use unwrap to see the error message ? + let mut api_spec = APISpec::from_file(&APISpec::get_defaut_spec_file()); + let result = api_spec + .method(Method::GET.as_str()) + .path("/certificate-pending") + .validate_request(&Null) + .unwrap() + .validate_status(&response, &StatusCode::INTERNAL_SERVER_ERROR); + + // TODO it could be better to not have to unwrap + // We can call one function with a chain of validations + // We can return an ApiSpec with function is_ok and is_err. + + assert!(result.is_ok()); + // Can we put Debug on ApiSpec to uses expect_err ? + + // How to specify this is the response we expect ? Because it is not here + // Validate response check there is one response expected. + // We want to specify the one expected. + + let mut api_spec = APISpec::from_file(&APISpec::get_defaut_spec_file()); + let result = api_spec + .method(Method::GET.as_str()) + .path("/certificate-pending") + .validate_request(&Null) + .unwrap() + .validate_response_and_status(&response, &StatusCode::INTERNAL_SERVER_ERROR); + + assert!(result.is_ok()); + } + #[test] fn test_apispec_validate_errors() { // Route does not exist From 80d091358b7e16f293747df7e47d416e4c336fdf Mon Sep 17 00:00:00 2001 From: sfauvel Date: Mon, 26 Feb 2024 10:03:23 +0100 Subject: [PATCH 02/29] Create a trait RequestValidator --- mithril-common/src/test_utils/apispec.rs | 76 +++++++++++++++++++++++- 1 file changed, 74 insertions(+), 2 deletions(-) diff --git a/mithril-common/src/test_utils/apispec.rs b/mithril-common/src/test_utils/apispec.rs index f001aa11f2e..89f1bf08372 100644 --- a/mithril-common/src/test_utils/apispec.rs +++ b/mithril-common/src/test_utils/apispec.rs @@ -2,8 +2,6 @@ use glob::glob; use jsonschema::JSONSchema; -// TODO APISpec is used only for test in modules but we need to add this dependency to expose it. -// Can we use the feature "test_tools" ? use serde::Serialize; use serde_json::{json, Value, Value::Null}; use warp::http::Response; @@ -212,6 +210,17 @@ impl<'a> APISpec<'a> { } } + pub fn validate( + &'a mut self, + response: &Response, + validator: Box, + ) -> Result<&mut APISpec, String> { + match validator.validate(response) { + Ok(_) => Ok(self), + Err(e) => Err(e), + } + } + /// Get default spec file pub fn get_defaut_spec_file() -> String { "../openapi.yaml".to_string() @@ -235,6 +244,34 @@ impl<'a> APISpec<'a> { } } +pub trait RequestValidator { + fn validate(&self, response: &Response) -> Result<(), String>; +} + +struct StatusValidator { + expected_status_code: StatusCode, +} +impl RequestValidator for StatusValidator { + fn validate(&self, response: &Response) -> Result<(), String> { + if self.expected_status_code.as_u16() != response.status().as_u16() { + return Err(format!( + "expected status code {} but was {}", + self.expected_status_code.as_u16(), + response.status().as_u16(), + )); + } + + return Ok(()); + } +} + +fn request_validator(expected_status_code: StatusCode) -> Box { + let s = StatusValidator { + expected_status_code, + }; + Box::new(s) +} + #[cfg(test)] mod tests { use warp::http::Method; @@ -336,6 +373,41 @@ mod tests { } } + #[test] + fn test_apispec_should_be_ok_when_the_status_code_is_the_right_one_with_validator() { + // 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; + { + let mut api_spec = APISpec::from_file(&APISpec::get_defaut_spec_file()); + + let result = api_spec + .method(Method::GET.as_str()) + .path("/certificate-pending") + .validate( + &response, + request_validator(StatusCode::INTERNAL_SERVER_ERROR), + ); + + assert!(result.is_ok()); + } + { + let mut api_spec = APISpec::from_file(&APISpec::get_defaut_spec_file()); + let result = api_spec + .method(Method::GET.as_str()) + .path("/certificate-pending") + .validate(&response, request_validator(StatusCode::OK)); + + assert!(result.is_err()); + } + } + #[test] fn test_apispec_should_be_ok_when_the_status_code_is_the_right_one() { // Route exists and matches default status code From 4ed809170c935a70161502e874c11dee87f21e83 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Mon, 26 Feb 2024 11:29:12 +0100 Subject: [PATCH 03/29] Add a validation with a list of validator, remove mut on ApiSpec, and remove public attribute on validate methods --- mithril-common/src/test_utils/apispec.rs | 66 ++++++++++++++---------- 1 file changed, 40 insertions(+), 26 deletions(-) diff --git a/mithril-common/src/test_utils/apispec.rs b/mithril-common/src/test_utils/apispec.rs index 89f1bf08372..fa09c356464 100644 --- a/mithril-common/src/test_utils/apispec.rs +++ b/mithril-common/src/test_utils/apispec.rs @@ -73,10 +73,7 @@ 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(); @@ -88,11 +85,11 @@ impl<'a> APISpec<'a> { } /// Validates if the status is the expected one - pub fn validate_status( - &'a mut self, + fn validate_status( + &'a self, response: &Response, expected_status_code: &StatusCode, - ) -> Result<&mut APISpec, String> { + ) -> Result<&APISpec, String> { if expected_status_code.as_u16() != response.status().as_u16() { return Err(format!( "expected status code {} but was {}", @@ -105,11 +102,11 @@ impl<'a> APISpec<'a> { } /// Validates if a response is valid - pub fn validate_response_and_status( - &'a mut self, + fn validate_response_and_status( + &'a self, response: &Response, expected_status_code: &StatusCode, - ) -> Result<&mut APISpec, String> { + ) -> Result<&APISpec, String> { self.validate_status(response, expected_status_code) .and_then(|s| s.validate_response(response)) @@ -132,10 +129,7 @@ impl<'a> APISpec<'a> { // return self.validate_response(response); } /// Validates if a response is valid - pub fn validate_response( - &'a mut self, - response: &Response, - ) -> Result<&mut APISpec, String> { + fn validate_response(&'a self, response: &Response) -> Result<&APISpec, String> { let body = response.body(); let status = response.status(); @@ -171,7 +165,10 @@ impl<'a> APISpec<'a> { } } else { match &serde_json::from_slice(body) { - Ok(value) => self.validate_conformity(value, response_schema), + Ok(value) => match self.validate_conformity(value, response_schema) { + Ok(_) => Ok(self), + Err(e) => Err(e), + }, Err(_) => Err("non empty body expected".to_string()), } } @@ -181,11 +178,11 @@ impl<'a> APISpec<'a> { } /// Validates conformity of a value against a schema - pub fn validate_conformity( - &'a mut self, + fn validate_conformity( + &'a self, value: &Value, schema: &mut Value, - ) -> Result<&mut APISpec, String> { + ) -> Result<&APISpec, String> { match schema { Null => match value { Null => Ok(self), @@ -210,15 +207,17 @@ impl<'a> APISpec<'a> { } } - pub fn validate( + fn validate( &'a mut self, response: &Response, - validator: Box, + validators: Vec>, ) -> Result<&mut APISpec, String> { - match validator.validate(response) { - Ok(_) => Ok(self), - Err(e) => Err(e), + for validator in validators { + if let Err(e) = validator.validate(response) { + return Err(e); + } } + Ok(self) } /// Get default spec file @@ -265,7 +264,7 @@ impl RequestValidator for StatusValidator { } } -fn request_validator(expected_status_code: StatusCode) -> Box { +fn expected_status(expected_status_code: StatusCode) -> Box { let s = StatusValidator { expected_status_code, }; @@ -392,7 +391,7 @@ mod tests { .path("/certificate-pending") .validate( &response, - request_validator(StatusCode::INTERNAL_SERVER_ERROR), + vec![expected_status(StatusCode::INTERNAL_SERVER_ERROR)], ); assert!(result.is_ok()); @@ -402,7 +401,22 @@ mod tests { let result = api_spec .method(Method::GET.as_str()) .path("/certificate-pending") - .validate(&response, request_validator(StatusCode::OK)); + .validate(&response, vec![expected_status(StatusCode::OK)]); + + assert!(result.is_err()); + } + { + let mut api_spec = APISpec::from_file(&APISpec::get_defaut_spec_file()); + let result = api_spec + .method(Method::GET.as_str()) + .path("/certificate-pending") + .validate( + &response, + vec![ + expected_status(StatusCode::INTERNAL_SERVER_ERROR), + expected_status(StatusCode::OK), + ], + ); assert!(result.is_err()); } From 750704b11dbab27efff550aa866fbd5663fbc01c Mon Sep 17 00:00:00 2001 From: sfauvel Date: Mon, 26 Feb 2024 15:50:57 +0100 Subject: [PATCH 04/29] Create a verify_conformity_with_status --- mithril-common/src/test_utils/apispec.rs | 146 +++++++++++++++++++++++ 1 file changed, 146 insertions(+) diff --git a/mithril-common/src/test_utils/apispec.rs b/mithril-common/src/test_utils/apispec.rs index fa09c356464..6f62d5e1707 100644 --- a/mithril-common/src/test_utils/apispec.rs +++ b/mithril-common/src/test_utils/apispec.rs @@ -19,6 +19,56 @@ pub struct APISpec<'a> { } impl<'a> APISpec<'a> { + /// Verify conformity helper of API Specs + pub fn verify_conformity_with_status( + spec_files: Vec, + method: &str, + path: &str, + content_type: &str, + request_body: &impl Serialize, + response: &Response, + status_code: &StatusCode, + ) { + if let Err(e) = APISpec::verify_conformity_result_with_status( + spec_files, + method, + path, + content_type, + request_body, + response, + status_code, + ) { + panic!("{}", e); + } + } + + /// Verify conformity helper of API Specs + pub fn verify_conformity_result_with_status( + spec_files: Vec, + method: &str, + path: &str, + content_type: &str, + request_body: &impl Serialize, + response: &Response, + status_code: &StatusCode, + ) -> Result<(), String> { + for spec_file in spec_files { + if let Err(e) = APISpec::from_file(&spec_file) + .method(method) + .path(path) + .content_type(content_type) + .validate_request(request_body) + .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}, reason: {e}\nresponse: {response:#?}" + )); + } + } + Ok(()) + } + /// Verify conformity helper of API Specs pub fn verify_conformity( spec_files: Vec, @@ -516,4 +566,100 @@ mod tests { assert!(!spec_files.is_empty()); assert!(spec_files.contains(&APISpec::get_defaut_spec_file())) } + + #[test] + fn test_apispec_verify_conformity() { + // Route exists and does not expect request body, but expects response + APISpec::verify_conformity( + APISpec::get_all_spec_files(), + Method::GET.as_str(), + "/certificate-pending", + "application/json", + &Null, + &Response::::new(Bytes::from( + json!(CertificatePendingMessage::dummy()) + .to_string() + .into_bytes(), + )), + ); + } + + #[test] + #[should_panic] + fn test_apispec_verify_conformity_should_panic_with_bad_response() { + APISpec::verify_conformity( + APISpec::get_all_spec_files(), + Method::GET.as_str(), + "/certificate-pending", + "application/json", + &Null, + &Response::::new(Bytes::new()), + ); + } + + #[test] + fn test_apispec_verify_conformity_with_expected_status() { + APISpec::verify_conformity_with_status( + APISpec::get_all_spec_files(), + Method::GET.as_str(), + "/certificate-pending", + "application/json", + &Null, + &Response::::new(Bytes::from( + json!(CertificatePendingMessage::dummy()) + .to_string() + .into_bytes(), + )), + &StatusCode::OK, + ); + } + + #[test] + #[should_panic] + fn test_apispec_verify_conformity_with_non_expected_status() { + APISpec::verify_conformity_with_status( + APISpec::get_all_spec_files(), + Method::GET.as_str(), + "/certificate-pending", + "application/json", + &Null, + &Response::::new(Bytes::from( + json!(CertificatePendingMessage::dummy()) + .to_string() + .into_bytes(), + )), + &StatusCode::BAD_REQUEST, + ); + } + + #[test] + fn test_apispec_verify_conformity_result_with_non_expected_status() { + let response = Response::::new(Bytes::from( + json!(CertificatePendingMessage::dummy()) + .to_string() + .into_bytes(), + )); + + let spec_file = APISpec::get_defaut_spec_file(); + let result = APISpec::verify_conformity_result_with_status( + 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}, reason: {error_reason}\nresponse: {response:#?}" + ); + assert!(result.is_err()); + assert_eq!(result.err().unwrap().to_string(), error_message); + } } From 0fe3bf10652f6f5db1656c309d5f01227135dfb9 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Mon, 26 Feb 2024 16:05:21 +0100 Subject: [PATCH 05/29] Verify that we check with at least one api spec file --- mithril-common/src/test_utils/apispec.rs | 29 ++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/mithril-common/src/test_utils/apispec.rs b/mithril-common/src/test_utils/apispec.rs index 6f62d5e1707..5bc2a2eb651 100644 --- a/mithril-common/src/test_utils/apispec.rs +++ b/mithril-common/src/test_utils/apispec.rs @@ -52,6 +52,12 @@ impl<'a> APISpec<'a> { 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 { if let Err(e) = APISpec::from_file(&spec_file) .method(method) @@ -662,4 +668,27 @@ mod tests { assert!(result.is_err()); assert_eq!(result.err().unwrap().to_string(), error_message); } + + #[test] + fn test_apispec_verify_conformity_should_panic_when_no_spec_file() { + let result = APISpec::verify_conformity_result_with_status( + vec![], + Method::GET.as_str(), + "/certificate-pending", + "application/json", + &Null, + &Response::::new(Bytes::from( + json!(CertificatePendingMessage::dummy()) + .to_string() + .into_bytes(), + )), + &StatusCode::OK, + ); + + assert!(result.is_err()); + assert_eq!( + result.err().unwrap().to_string(), + "OpenAPI need a spec file to validate conformity. None were given." + ); + } } From 309ae583d808b35ff8c3b27b904131c20c895672 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Mon, 26 Feb 2024 16:36:19 +0100 Subject: [PATCH 06/29] Refactor and clean APISpec code --- mithril-common/src/test_utils/apispec.rs | 292 ++++------------------- 1 file changed, 44 insertions(+), 248 deletions(-) diff --git a/mithril-common/src/test_utils/apispec.rs b/mithril-common/src/test_utils/apispec.rs index 5bc2a2eb651..aefd6e0740c 100644 --- a/mithril-common/src/test_utils/apispec.rs +++ b/mithril-common/src/test_utils/apispec.rs @@ -28,29 +28,6 @@ impl<'a> APISpec<'a> { request_body: &impl Serialize, response: &Response, status_code: &StatusCode, - ) { - if let Err(e) = APISpec::verify_conformity_result_with_status( - spec_files, - method, - path, - content_type, - request_body, - response, - status_code, - ) { - panic!("{}", e); - } - } - - /// Verify conformity helper of API Specs - pub fn verify_conformity_result_with_status( - spec_files: Vec, - method: &str, - path: &str, - content_type: &str, - request_body: &impl Serialize, - response: &Response, - status_code: &StatusCode, ) -> Result<(), String> { if spec_files.is_empty() { return Err( @@ -154,36 +131,9 @@ impl<'a> APISpec<'a> { )); } - return Ok(self); + Ok(self) } - /// Validates if a response is valid - fn validate_response_and_status( - &'a self, - response: &Response, - expected_status_code: &StatusCode, - ) -> Result<&APISpec, String> { - self.validate_status(response, expected_status_code) - .and_then(|s| s.validate_response(response)) - - // // Return ???? - // let result = self.validate_status(response, expected_status_code); - // // Recreate Err because it need a Result<&mut APISpec, String> when Result<&APISpec, String> is return - // // Can we do in a better way ? - // if let Err(err) = result { - // return Err(err); - // } - - // // 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(), - // // )); - // // } - - // return self.validate_response(response); - } /// Validates if a response is valid fn validate_response(&'a self, response: &Response) -> Result<&APISpec, String> { let body = response.body(); @@ -221,10 +171,7 @@ impl<'a> APISpec<'a> { } } else { match &serde_json::from_slice(body) { - Ok(value) => match self.validate_conformity(value, response_schema) { - Ok(_) => Ok(self), - Err(e) => Err(e), - }, + Ok(value) => self.validate_conformity(value, response_schema), Err(_) => Err("non empty body expected".to_string()), } } @@ -263,21 +210,8 @@ impl<'a> APISpec<'a> { } } - fn validate( - &'a mut self, - response: &Response, - validators: Vec>, - ) -> Result<&mut APISpec, String> { - for validator in validators { - if let Err(e) = validator.validate(response) { - return Err(e); - } - } - Ok(self) - } - /// Get default spec file - pub fn get_defaut_spec_file() -> String { + pub fn get_default_spec_file() -> String { "../openapi.yaml".to_string() } @@ -299,34 +233,6 @@ impl<'a> APISpec<'a> { } } -pub trait RequestValidator { - fn validate(&self, response: &Response) -> Result<(), String>; -} - -struct StatusValidator { - expected_status_code: StatusCode, -} -impl RequestValidator for StatusValidator { - fn validate(&self, response: &Response) -> Result<(), String> { - if self.expected_status_code.as_u16() != response.status().as_u16() { - return Err(format!( - "expected status code {} but was {}", - self.expected_status_code.as_u16(), - response.status().as_u16(), - )); - } - - return Ok(()); - } -} - -fn expected_status(expected_status_code: StatusCode) -> Box { - let s = StatusValidator { - expected_status_code, - }; - Box::new(s) -} - #[cfg(test)] mod tests { use warp::http::Method; @@ -340,7 +246,7 @@ mod tests { #[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()) + assert!(APISpec::from_file(&APISpec::get_default_spec_file()) .method(Method::GET.as_str()) .path("/certificate-pending") .validate_request(&Null) @@ -353,7 +259,7 @@ mod tests { .is_ok()); // Route exists and expects request body, but does not expect response - assert!(APISpec::from_file(&APISpec::get_defaut_spec_file()) + assert!(APISpec::from_file(&APISpec::get_default_spec_file()) .method(Method::POST.as_str()) .path("/register-signer") .validate_request(&SignerMessagePart::dummy()) @@ -370,7 +276,7 @@ mod tests { .into_bytes(), )); *response.status_mut() = StatusCode::INTERNAL_SERVER_ERROR; - assert!(APISpec::from_file(&APISpec::get_defaut_spec_file()) + assert!(APISpec::from_file(&APISpec::get_default_spec_file()) .method(Method::POST.as_str()) .path("/register-signer") .validate_response(&response) @@ -388,94 +294,24 @@ mod tests { .into_bytes(), )); *response.status_mut() = StatusCode::INTERNAL_SERVER_ERROR; - { - let mut api_spec = APISpec::from_file(&APISpec::get_defaut_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() - ) - ); - } - { - let mut api_spec = APISpec::from_file(&APISpec::get_defaut_spec_file()); - let result = api_spec - .method(Method::GET.as_str()) - .path("/certificate-pending") - .validate_request(&Null) - .unwrap() - .validate_response_and_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_apispec_should_be_ok_when_the_status_code_is_the_right_one_with_validator() { - // 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; - { - let mut api_spec = APISpec::from_file(&APISpec::get_defaut_spec_file()); - - let result = api_spec - .method(Method::GET.as_str()) - .path("/certificate-pending") - .validate( - &response, - vec![expected_status(StatusCode::INTERNAL_SERVER_ERROR)], - ); - - assert!(result.is_ok()); - } - { - let mut api_spec = APISpec::from_file(&APISpec::get_defaut_spec_file()); - let result = api_spec - .method(Method::GET.as_str()) - .path("/certificate-pending") - .validate(&response, vec![expected_status(StatusCode::OK)]); - - assert!(result.is_err()); - } - { - let mut api_spec = APISpec::from_file(&APISpec::get_defaut_spec_file()); - let result = api_spec - .method(Method::GET.as_str()) - .path("/certificate-pending") - .validate( - &response, - vec![ - expected_status(StatusCode::INTERNAL_SERVER_ERROR), - expected_status(StatusCode::OK), - ], - ); - - assert!(result.is_err()); - } + 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] @@ -490,89 +326,60 @@ mod tests { )); *response.status_mut() = StatusCode::INTERNAL_SERVER_ERROR; - // Is it better to use unwrap to see the error message ? - let mut api_spec = APISpec::from_file(&APISpec::get_defaut_spec_file()); - let result = api_spec - .method(Method::GET.as_str()) - .path("/certificate-pending") - .validate_request(&Null) - .unwrap() - .validate_status(&response, &StatusCode::INTERNAL_SERVER_ERROR); - - // TODO it could be better to not have to unwrap - // We can call one function with a chain of validations - // We can return an ApiSpec with function is_ok and is_err. - - assert!(result.is_ok()); - // Can we put Debug on ApiSpec to uses expect_err ? - - // How to specify this is the response we expect ? Because it is not here - // Validate response check there is one response expected. - // We want to specify the one expected. - - let mut api_spec = APISpec::from_file(&APISpec::get_defaut_spec_file()); - let result = api_spec + APISpec::from_file(&APISpec::get_default_spec_file()) .method(Method::GET.as_str()) .path("/certificate-pending") .validate_request(&Null) .unwrap() - .validate_response_and_status(&response, &StatusCode::INTERNAL_SERVER_ERROR); - - assert!(result.is_ok()); + .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()) + assert!(APISpec::from_file(&APISpec::get_default_spec_file()) .method(Method::GET.as_str()) .path("/route-not-existing-in-openapi-spec") .validate_response(&Response::::new(Bytes::from_static(b"abcdefgh"))) .is_err()); // Route exists, but method does not - assert!(APISpec::from_file(&APISpec::get_defaut_spec_file()) + assert!(APISpec::from_file(&APISpec::get_default_spec_file()) .method(Method::OPTIONS.as_str()) .path("/certificate-pending") .validate_response(&Response::::new(Bytes::from_static(b"abcdefgh"))) .is_err()); // Route exists, but expects non empty reponse - assert!(APISpec::from_file(&APISpec::get_defaut_spec_file()) + assert!(APISpec::from_file(&APISpec::get_default_spec_file()) .method(Method::GET.as_str()) .path("/certificate-pending") .validate_response(&Response::::new(Bytes::new())) .is_err()); // Route exists, but expects empty reponse - assert!(APISpec::from_file(&APISpec::get_defaut_spec_file()) + assert!(APISpec::from_file(&APISpec::get_default_spec_file()) .method(Method::POST.as_str()) .path("/register-signer") .validate_response(&Response::::new(Bytes::from_static(b"abcdefgh"))) .is_err()); // Route exists, but does not expect request body - assert!(APISpec::from_file(&APISpec::get_defaut_spec_file()) + 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()) + 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_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())) - } - #[test] fn test_apispec_verify_conformity() { // Route exists and does not expect request body, but expects response @@ -604,7 +411,7 @@ mod tests { } #[test] - fn test_apispec_verify_conformity_with_expected_status() { + fn test_apispec_verify_conformity_with_expected_status() -> Result<(), String> { APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), Method::GET.as_str(), @@ -617,25 +424,7 @@ mod tests { .into_bytes(), )), &StatusCode::OK, - ); - } - - #[test] - #[should_panic] - fn test_apispec_verify_conformity_with_non_expected_status() { - APISpec::verify_conformity_with_status( - APISpec::get_all_spec_files(), - Method::GET.as_str(), - "/certificate-pending", - "application/json", - &Null, - &Response::::new(Bytes::from( - json!(CertificatePendingMessage::dummy()) - .to_string() - .into_bytes(), - )), - &StatusCode::BAD_REQUEST, - ); + ) } #[test] @@ -646,8 +435,8 @@ mod tests { .into_bytes(), )); - let spec_file = APISpec::get_defaut_spec_file(); - let result = APISpec::verify_conformity_result_with_status( + let spec_file = APISpec::get_default_spec_file(); + let result = APISpec::verify_conformity_with_status( vec![spec_file.clone()], Method::GET.as_str(), "/certificate-pending", @@ -671,7 +460,7 @@ mod tests { #[test] fn test_apispec_verify_conformity_should_panic_when_no_spec_file() { - let result = APISpec::verify_conformity_result_with_status( + let result = APISpec::verify_conformity_with_status( vec![], Method::GET.as_str(), "/certificate-pending", @@ -691,4 +480,11 @@ mod tests { "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_default_spec_file())) + } } From 264889ca051dd68bc7051f1a0460b486ef1ea5b0 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Mon, 26 Feb 2024 16:55:40 +0100 Subject: [PATCH 07/29] Split test on error in several tests --- mithril-common/src/test_utils/apispec.rs | 44 +++++++++++++----------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/mithril-common/src/test_utils/apispec.rs b/mithril-common/src/test_utils/apispec.rs index aefd6e0740c..f80d96b9b46 100644 --- a/mithril-common/src/test_utils/apispec.rs +++ b/mithril-common/src/test_utils/apispec.rs @@ -244,7 +244,7 @@ mod tests { use crate::test_utils::fake_data; #[test] - fn test_apispec_validate_ok() { + fn test_validate_ok() { // Route exists and does not expect request body, but expects response assert!(APISpec::from_file(&APISpec::get_default_spec_file()) .method(Method::GET.as_str()) @@ -284,7 +284,7 @@ mod tests { } #[test] - fn test_apispec_should_fail_when_the_status_code_is_not_the_expected_one() { + fn test_should_fail_when_the_status_code_is_not_the_expected_one() { // Route exists and matches default status code let mut response = Response::::new(Bytes::from( json!(&entities::InternalServerError::new( @@ -315,7 +315,7 @@ mod tests { } #[test] - fn test_apispec_should_be_ok_when_the_status_code_is_the_right_one() { + fn test_should_be_ok_when_the_status_code_is_the_right_one() { // Route exists and matches default status code let mut response = Response::::new(Bytes::from( json!(&entities::InternalServerError::new( @@ -336,43 +336,47 @@ mod tests { } #[test] - fn test_apispec_validate_errors() { - // Route does not exist + fn test_validate_returns_error_when_route_does_not_exist() { assert!(APISpec::from_file(&APISpec::get_default_spec_file()) .method(Method::GET.as_str()) .path("/route-not-existing-in-openapi-spec") .validate_response(&Response::::new(Bytes::from_static(b"abcdefgh"))) .is_err()); - - // Route exists, but method does not + } + #[test] + fn test_validate_returns_error_when_route_exists_but_method_does_not() { assert!(APISpec::from_file(&APISpec::get_default_spec_file()) .method(Method::OPTIONS.as_str()) .path("/certificate-pending") .validate_response(&Response::::new(Bytes::from_static(b"abcdefgh"))) .is_err()); - - // Route exists, but expects non empty reponse + } + #[test] + fn test_validate_returns_error_when_route_exists_but_expects_non_empty_response() { assert!(APISpec::from_file(&APISpec::get_default_spec_file()) .method(Method::GET.as_str()) .path("/certificate-pending") .validate_response(&Response::::new(Bytes::new())) .is_err()); - - // Route exists, but expects empty reponse + } + #[test] + fn test_validate_returns_error_when_route_exists_but_expects_empty_response() { assert!(APISpec::from_file(&APISpec::get_default_spec_file()) .method(Method::POST.as_str()) .path("/register-signer") .validate_response(&Response::::new(Bytes::from_static(b"abcdefgh"))) .is_err()); - - // Route exists, but does not expect request body + } + #[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 + } + #[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") @@ -381,7 +385,7 @@ mod tests { } #[test] - fn test_apispec_verify_conformity() { + fn test_verify_conformity() { // Route exists and does not expect request body, but expects response APISpec::verify_conformity( APISpec::get_all_spec_files(), @@ -399,7 +403,7 @@ mod tests { #[test] #[should_panic] - fn test_apispec_verify_conformity_should_panic_with_bad_response() { + fn test_verify_conformity_should_panic_with_bad_response() { APISpec::verify_conformity( APISpec::get_all_spec_files(), Method::GET.as_str(), @@ -411,7 +415,7 @@ mod tests { } #[test] - fn test_apispec_verify_conformity_with_expected_status() -> Result<(), String> { + fn test_verify_conformity_with_expected_status() -> Result<(), String> { APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), Method::GET.as_str(), @@ -428,7 +432,7 @@ mod tests { } #[test] - fn test_apispec_verify_conformity_result_with_non_expected_status() { + fn test_verify_conformity_with_non_expected_status_returns_error() { let response = Response::::new(Bytes::from( json!(CertificatePendingMessage::dummy()) .to_string() @@ -459,7 +463,7 @@ mod tests { } #[test] - fn test_apispec_verify_conformity_should_panic_when_no_spec_file() { + fn test_verify_conformity_when_no_spec_file_returns_error() { let result = APISpec::verify_conformity_with_status( vec![], Method::GET.as_str(), From 78c084f0c569407eceecb4c069cada6ef23a5a86 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Mon, 26 Feb 2024 17:07:38 +0100 Subject: [PATCH 08/29] Add a method to simplify response creation --- mithril-common/src/test_utils/apispec.rs | 50 ++++++------------------ 1 file changed, 13 insertions(+), 37 deletions(-) diff --git a/mithril-common/src/test_utils/apispec.rs b/mithril-common/src/test_utils/apispec.rs index f80d96b9b46..5998a3e90c4 100644 --- a/mithril-common/src/test_utils/apispec.rs +++ b/mithril-common/src/test_utils/apispec.rs @@ -243,6 +243,10 @@ mod tests { use crate::messages::{CertificatePendingMessage, SignerMessagePart}; use crate::test_utils::fake_data; + fn build_json_response(value: T) -> Response { + Response::::new(Bytes::from(json!(value).to_string().into_bytes())) + } + #[test] fn test_validate_ok() { // Route exists and does not expect request body, but expects response @@ -251,11 +255,7 @@ mod tests { .path("/certificate-pending") .validate_request(&Null) .unwrap() - .validate_response(&Response::::new(Bytes::from( - json!(CertificatePendingMessage::dummy()) - .to_string() - .into_bytes(), - ))) + .validate_response(&build_json_response(CertificatePendingMessage::dummy())) .is_ok()); // Route exists and expects request body, but does not expect response @@ -286,12 +286,8 @@ mod tests { #[test] fn test_should_fail_when_the_status_code_is_not_the_expected_one() { // 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(), + let mut response = build_json_response(entities::InternalServerError::new( + "an error occurred".to_string(), )); *response.status_mut() = StatusCode::INTERNAL_SERVER_ERROR; @@ -317,12 +313,8 @@ mod tests { #[test] fn test_should_be_ok_when_the_status_code_is_the_right_one() { // 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(), + let mut response = build_json_response(entities::InternalServerError::new( + "an error occurred".to_string(), )); *response.status_mut() = StatusCode::INTERNAL_SERVER_ERROR; @@ -393,11 +385,7 @@ mod tests { "/certificate-pending", "application/json", &Null, - &Response::::new(Bytes::from( - json!(CertificatePendingMessage::dummy()) - .to_string() - .into_bytes(), - )), + &build_json_response(CertificatePendingMessage::dummy()), ); } @@ -422,22 +410,14 @@ mod tests { "/certificate-pending", "application/json", &Null, - &Response::::new(Bytes::from( - json!(CertificatePendingMessage::dummy()) - .to_string() - .into_bytes(), - )), + &build_json_response(CertificatePendingMessage::dummy()), &StatusCode::OK, ) } #[test] fn test_verify_conformity_with_non_expected_status_returns_error() { - let response = Response::::new(Bytes::from( - json!(CertificatePendingMessage::dummy()) - .to_string() - .into_bytes(), - )); + let response = build_json_response(CertificatePendingMessage::dummy()); let spec_file = APISpec::get_default_spec_file(); let result = APISpec::verify_conformity_with_status( @@ -470,11 +450,7 @@ mod tests { "/certificate-pending", "application/json", &Null, - &Response::::new(Bytes::from( - json!(CertificatePendingMessage::dummy()) - .to_string() - .into_bytes(), - )), + &build_json_response(CertificatePendingMessage::dummy()), &StatusCode::OK, ); From b6f6024a9b2020a7e3ebf0d494fa894ea39a74cc Mon Sep 17 00:00:00 2001 From: sfauvel Date: Mon, 26 Feb 2024 18:09:18 +0100 Subject: [PATCH 09/29] Use conformity with status on proof_routes --- .../src/http_server/routes/proof_routes.rs | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/mithril-aggregator/src/http_server/routes/proof_routes.rs b/mithril-aggregator/src/http_server/routes/proof_routes.rs index 5357528bd36..348f69ed337 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::{ @@ -134,7 +137,7 @@ mod tests { } #[tokio::test] - async fn proof_cardano_transaction_ok() { + async fn proof_cardano_transaction_ok() -> Result<(), String> { let config = Configuration::new_sample(); let mut builder = DependenciesBuilder::new(config); let mut dependency_manager = builder.build_dependency_container().await.unwrap(); @@ -161,18 +164,19 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &Null, &response, - ); + &StatusCode::OK, + ) } #[tokio::test] - async fn proof_cardano_transaction_not_found() { + async fn proof_cardano_transaction_not_found() -> Result<(), String> { let config = Configuration::new_sample(); let mut builder = DependenciesBuilder::new(config); let dependency_manager = builder.build_dependency_container().await.unwrap(); @@ -188,18 +192,19 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &Null, &response, - ); + &StatusCode::NOT_FOUND, + ) } #[tokio::test] - async fn proof_cardano_transaction_ko() { + async fn proof_cardano_transaction_ko() -> Result<(), String> { let config = Configuration::new_sample(); let mut builder = DependenciesBuilder::new(config); let mut dependency_manager = builder.build_dependency_container().await.unwrap(); @@ -220,13 +225,14 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &Null, &response, - ); + &StatusCode::INTERNAL_SERVER_ERROR, + ) } } From f76e36c011979eabf5f9c31a9ffe5c6f5ad6761b Mon Sep 17 00:00:00 2001 From: sfauvel Date: Mon, 26 Feb 2024 18:13:50 +0100 Subject: [PATCH 10/29] Use conformity with status on root_routes --- mithril-aggregator/src/http_server/routes/root_routes.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/mithril-aggregator/src/http_server/routes/root_routes.rs b/mithril-aggregator/src/http_server/routes/root_routes.rs index dcd9b2892a7..d6a0085566c 100644 --- a/mithril-aggregator/src/http_server/routes/root_routes.rs +++ b/mithril-aggregator/src/http_server/routes/root_routes.rs @@ -91,6 +91,7 @@ mod tests { use warp::http::Method; use warp::test::request; use warp::Filter; + use zstd::stream::raw::Status; use super::*; @@ -108,7 +109,7 @@ mod tests { } #[tokio::test] - async fn test_root_route_ok() { + async fn test_root_route_ok() -> Result<(), String> { let method = Method::GET.as_str(); let path = "/"; let mut dependency_manager = initialize_dependencies().await; @@ -150,13 +151,14 @@ mod tests { } ); - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &Null, &response, - ); + &StatusCode::OK, + ) } } From 66af8bd41aeb608f6982b5637fd06b0b09edaca6 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Tue, 27 Feb 2024 17:59:54 +0100 Subject: [PATCH 11/29] Refactor APISpec and improve tests --- mithril-common/src/test_utils/apispec.rs | 222 ++++++++++++++++------- 1 file changed, 154 insertions(+), 68 deletions(-) diff --git a/mithril-common/src/test_utils/apispec.rs b/mithril-common/src/test_utils/apispec.rs index 5998a3e90c4..0426aa8c226 100644 --- a/mithril-common/src/test_utils/apispec.rs +++ b/mithril-common/src/test_utils/apispec.rs @@ -111,8 +111,8 @@ impl<'a> APISpec<'a> { 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 request_schema = + &self.openapi["paths"][path][method]["requestBody"]["content"][content_type]["schema"]; let value = &json!(&request_body); self.validate_conformity(value, request_schema) } @@ -147,52 +147,54 @@ 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 = &response_spec["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 - fn validate_conformity( - &'a self, - value: &Value, - schema: &mut Value, - ) -> Result<&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); @@ -243,22 +245,54 @@ mod tests { use crate::messages::{CertificatePendingMessage, SignerMessagePart}; use crate::test_utils::fake_data; - fn build_json_response(value: T) -> Response { - Response::::new(Bytes::from(json!(value).to_string().into_bytes())) + 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_validate_a_response_without_body() { + assert!(APISpec::from_file(&APISpec::get_default_spec_file()) + .method(Method::GET.as_str()) + .path("/certificate-pending") + .validate_request(&Null) + .unwrap() + .validate_response(&build_empty_response(204)) + .is_ok()); } #[test] - fn test_validate_ok() { - // Route exists and does not expect request body, but expects response + fn test_validate_ok_when_request_without_body_and_expects_response() { assert!(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(CertificatePendingMessage::dummy())) + .validate_response(&build_json_response( + 200, + CertificatePendingMessage::dummy() + )) .is_ok()); + } - // Route exists and expects request body, but does not expect response + #[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") @@ -266,16 +300,17 @@ mod tests { .unwrap() .validate_response(&Response::::new(Bytes::new())) .is_err()); + } + + #[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()), + ); - // 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_default_spec_file()) .method(Method::POST.as_str()) .path("/register-signer") @@ -285,11 +320,10 @@ mod tests { #[test] fn test_should_fail_when_the_status_code_is_not_the_expected_one() { - // Route exists and matches default status code - let mut response = build_json_response(entities::InternalServerError::new( - "an error occurred".to_string(), - )); - *response.status_mut() = StatusCode::INTERNAL_SERVER_ERROR; + 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 @@ -312,11 +346,10 @@ mod tests { #[test] fn test_should_be_ok_when_the_status_code_is_the_right_one() { - // Route exists and matches default status code - let mut response = build_json_response(entities::InternalServerError::new( - "an error occurred".to_string(), - )); - *response.status_mut() = StatusCode::INTERNAL_SERVER_ERROR; + 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()) @@ -329,36 +362,90 @@ mod tests { #[test] fn test_validate_returns_error_when_route_does_not_exist() { - assert!(APISpec::from_file(&APISpec::get_default_spec_file()) + 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()) + ); } + #[test] fn test_validate_returns_error_when_route_exists_but_method_does_not() { - assert!(APISpec::from_file(&APISpec::get_default_spec_file()) + 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")); + + 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() { - assert!(APISpec::from_file(&APISpec::get_default_spec_file()) + 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)); + + 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() { - assert!(APISpec::from_file(&APISpec::get_default_spec_file()) - .method(Method::POST.as_str()) - .path("/register-signer") - .validate_response(&Response::::new(Bytes::from_static(b"abcdefgh"))) - .is_err()); + { + 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()) + ); + } } + + #[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()) @@ -378,14 +465,13 @@ mod tests { #[test] fn test_verify_conformity() { - // Route exists and does not expect request body, but expects response APISpec::verify_conformity( APISpec::get_all_spec_files(), Method::GET.as_str(), "/certificate-pending", "application/json", &Null, - &build_json_response(CertificatePendingMessage::dummy()), + &build_json_response(200, CertificatePendingMessage::dummy()), ); } @@ -410,14 +496,14 @@ mod tests { "/certificate-pending", "application/json", &Null, - &build_json_response(CertificatePendingMessage::dummy()), + &build_json_response(200, CertificatePendingMessage::dummy()), &StatusCode::OK, ) } #[test] fn test_verify_conformity_with_non_expected_status_returns_error() { - let response = build_json_response(CertificatePendingMessage::dummy()); + let response = build_json_response(200, CertificatePendingMessage::dummy()); let spec_file = APISpec::get_default_spec_file(); let result = APISpec::verify_conformity_with_status( @@ -450,7 +536,7 @@ mod tests { "/certificate-pending", "application/json", &Null, - &build_json_response(CertificatePendingMessage::dummy()), + &build_json_response(200, CertificatePendingMessage::dummy()), &StatusCode::OK, ); From 9384737c9a03e9f2b9b765e55c8e3f7a69c5f470 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Wed, 28 Feb 2024 11:45:49 +0100 Subject: [PATCH 12/29] Improve fake aggregator tests --- .../src/application.rs | 234 +++++++++--------- .../mithril-aggregator-fake/src/error.rs | 14 +- .../mithril-aggregator-fake/src/handlers.rs | 16 +- 3 files changed, 132 insertions(+), 132 deletions(-) diff --git a/mithril-test-lab/mithril-aggregator-fake/src/application.rs b/mithril-test-lab/mithril-aggregator-fake/src/application.rs index 3fe10482be5..e561a57cd59 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,45 @@ 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() + // TODO call get_all_spec_files with a root_path (../..) + vec!["../../openapi.yaml".to_string()] + } + #[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(), + APISpec::verify_conformity_with_status( + 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 +173,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(); + let response = http_request(PORT, path).await; - assert_eq!(StatusCode::OK, response.status()); - - APISpec::verify_conformity( - APISpec::get_all_spec_files(), + APISpec::verify_conformity_with_status( + 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 +198,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(); + let response = http_request(PORT, path).await; - assert_eq!(StatusCode::OK, response.status()); - - APISpec::verify_conformity( - APISpec::get_all_spec_files(), + APISpec::verify_conformity_with_status( + 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 +223,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(); + let response = + http_request(PORT, &path.replace("{certificate_hash}", certificate_hash)).await; - assert_eq!(StatusCode::OK, response.status()); - - APISpec::verify_conformity( - APISpec::get_all_spec_files(), + APISpec::verify_conformity_with_status( + 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 +250,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_with_status( + get_spec_files(), + "GET", + path, + "application/json", + &Null, + &response, + &StatusCode::NOT_FOUND, + ) + .map_err(|e| anyhow!(e)) }); test(task, PORT).await; @@ -270,29 +276,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(), + APISpec::verify_conformity_with_status( + 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 +302,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_with_status( + get_spec_files(), + "GET", + path, + "application/json", + &Null, + &response, + &StatusCode::NOT_FOUND, + ) + .map_err(|e| anyhow!(e)) }); test(task, PORT).await; @@ -325,29 +328,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(); + let response = http_request(PORT, &path.replace("{hash}", hash)).await; - assert_eq!(StatusCode::OK, response.status()); - - APISpec::verify_conformity( - APISpec::get_all_spec_files(), + APISpec::verify_conformity_with_status( + 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 +354,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_with_status( + get_spec_files(), + "GET", + path, + "application/json", + &Null, + &response, + &StatusCode::NOT_FOUND, + ) + .map_err(|e| anyhow!(e)) }); test(task, PORT).await; @@ -377,26 +379,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(); + let response = http_request(PORT, path).await; - assert_eq!(StatusCode::OK, response.status()); - - APISpec::verify_conformity( - APISpec::get_all_spec_files(), + APISpec::verify_conformity_with_status( + 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; diff --git a/mithril-test-lab/mithril-aggregator-fake/src/error.rs b/mithril-test-lab/mithril-aggregator-fake/src/error.rs index 7089703d41e..c6a54e62785 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,7 +11,7 @@ pub enum AppError { /// Catching anyhow errors Internal(anyhow::Error), - /// Resource not found (specify what) + /// Resource not found (specify body) NotFound(String), } @@ -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(response_body) => (StatusCode::NOT_FOUND, response_body).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..5e27b2d8f6d 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("".to_string()) + }) } /// 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("".to_string()) + }) } /// 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("".to_string()) + }) } /// HTTP: return the list of certificates From 497d43240fe95305c9289b930c33dc9b08081599 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Wed, 28 Feb 2024 12:08:25 +0100 Subject: [PATCH 13/29] Improve http test adding expected status --- .../http_server/routes/signatures_routes.rs | 37 +++++++++++-------- .../http_server/routes/statistics_routes.rs | 11 ++++-- mithril-common/src/test_utils/apispec.rs | 2 +- 3 files changed, 30 insertions(+), 20 deletions(-) diff --git a/mithril-aggregator/src/http_server/routes/signatures_routes.rs b/mithril-aggregator/src/http_server/routes/signatures_routes.rs index 79ef15f0e53..cf1862a3d77 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::{ @@ -133,7 +133,7 @@ mod tests { } #[tokio::test] - async fn test_register_signatures_post_ok() { + async fn test_register_signatures_post_ok() -> Result<(), String> { let mut mock_certifier_service = MockCertifierService::new(); mock_certifier_service .expect_register_single_signature() @@ -153,18 +153,19 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &message, &response, - ); + &StatusCode::CREATED, + ) } #[tokio::test] - async fn test_register_signatures_post_ko_400() { + async fn test_register_signatures_post_ko_400() -> Result<(), String> { let mut mock_certifier_service = MockCertifierService::new(); mock_certifier_service .expect_register_single_signature() @@ -185,18 +186,19 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &message, &response, - ); + &StatusCode::BAD_REQUEST, + ) } #[tokio::test] - async fn test_register_signatures_post_ko_404() { + async fn test_register_signatures_post_ko_404() -> Result<(), String> { let signed_entity_type = SignedEntityType::dummy(); let message = RegisterSignatureMessage::dummy(); let mut mock_certifier_service = MockCertifierService::new(); @@ -218,18 +220,19 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &message, &response, - ); + &StatusCode::NOT_FOUND, + ) } #[tokio::test] - async fn test_register_signatures_post_ko_410() { + async fn test_register_signatures_post_ko_410() -> Result<(), String> { let signed_entity_type = SignedEntityType::dummy(); let message = RegisterSignatureMessage::dummy(); let mut mock_certifier_service = MockCertifierService::new(); @@ -251,18 +254,19 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &message, &response, - ); + &StatusCode::GONE, + ) } #[tokio::test] - async fn test_register_signatures_post_ko_500() { + async fn test_register_signatures_post_ko_500() -> Result<(), String> { let mut mock_certifier_service = MockCertifierService::new(); mock_certifier_service .expect_register_single_signature() @@ -282,13 +286,14 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &message, &response, - ); + &StatusCode::INTERNAL_SERVER_ERROR, + ) } } diff --git a/mithril-aggregator/src/http_server/routes/statistics_routes.rs b/mithril-aggregator/src/http_server/routes/statistics_routes.rs index 0b3e2652101..d238360ebd8 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, @@ -77,7 +80,7 @@ mod tests { } #[tokio::test] - async fn post_statistics_ok() { + async fn post_statistics_ok() -> Result<(), String> { let config = Configuration::new_sample(); let mut builder = DependenciesBuilder::new(config); let mut rx = builder.get_event_transmitter_receiver().await.unwrap(); @@ -94,15 +97,17 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + let result = APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &snapshot_download_message, &response, + &StatusCode::CREATED, ); let _ = rx.try_recv().unwrap(); + result } } diff --git a/mithril-common/src/test_utils/apispec.rs b/mithril-common/src/test_utils/apispec.rs index 0426aa8c226..87ae94aec4f 100644 --- a/mithril-common/src/test_utils/apispec.rs +++ b/mithril-common/src/test_utils/apispec.rs @@ -45,7 +45,7 @@ impl<'a> APISpec<'a> { .and_then(|api| api.validate_status(response, status_code)) { return Err(format!( - "OpenAPI invalid response in {spec_file}, reason: {e}\nresponse: {response:#?}" + "OpenAPI invalid response in {spec_file} on route {path}, reason: {e}\nresponse: {response:#?}" )); } } From 7e541c8d2e589b09c17887f60a74530c729d37f5 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Thu, 29 Feb 2024 10:25:08 +0100 Subject: [PATCH 14/29] Fix get ok epoch_routes test --- .../src/http_server/routes/epoch_routes.rs | 33 ++++++++++++------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/mithril-aggregator/src/http_server/routes/epoch_routes.rs b/mithril-aggregator/src/http_server/routes/epoch_routes.rs index e3ddefa593d..928dc34a457 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, @@ -83,10 +89,13 @@ mod tests { } #[tokio::test] - async fn test_epoch_settings_get_ok() { + async fn test_epoch_settings_get_ok() -> Result<(), String> { 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) @@ -94,18 +103,19 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &Null, &response, - ); + &StatusCode::OK, + ) } #[tokio::test] - async fn test_epoch_settings_get_ko_500() { + async fn test_epoch_settings_get_ko_500() -> Result<(), String> { let method = Method::GET.as_str(); let path = "/epoch-settings"; let dependency_manager = initialize_dependencies().await; @@ -116,13 +126,14 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &Null, &response, - ); + &StatusCode::INTERNAL_SERVER_ERROR, + ) } } From 8fecceee3a257fe0c3c19d9529633066b8aee18f Mon Sep 17 00:00:00 2001 From: sfauvel Date: Thu, 29 Feb 2024 12:19:14 +0100 Subject: [PATCH 15/29] Fix tests on certificate_routes --- .../http_server/routes/certificate_routes.rs | 203 ++++++++++++++---- 1 file changed, 164 insertions(+), 39 deletions(-) diff --git a/mithril-aggregator/src/http_server/routes/certificate_routes.rs b/mithril-aggregator/src/http_server/routes/certificate_routes.rs index d233785da4e..94c483173f7 100644 --- a/mithril-aggregator/src/http_server/routes/certificate_routes.rs +++ b/mithril-aggregator/src/http_server/routes/certificate_routes.rs @@ -121,16 +121,109 @@ mod handlers { #[cfg(test)] mod tests { use anyhow::anyhow; - use mithril_common::test_utils::{apispec::APISpec, fake_data}; + use async_trait::async_trait; + use mithril_common::{ + entities::CertificatePending, + test_utils::{apispec::APISpec, fake_data}, + }; + use mithril_persistence::store::adapter::AdapterError; + use mithril_persistence::store::adapter::StoreAdapter; 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::*; + /////////////// + + pub struct MockStoreAdapter {} + + #[async_trait] + impl StoreAdapter for MockStoreAdapter { + type Key = String; + type Record = CertificatePending; + + async fn store_record( + &mut self, + _key: &Self::Key, + _record: &Self::Record, + ) -> Result<(), AdapterError> { + Ok(()) + } + + async fn get_record(&self, _key: &Self::Key) -> Result, AdapterError> { + Err(AdapterError::GeneralError(anyhow!("Ca marche pô"))) + } + + async fn record_exists(&self, _key: &Self::Key) -> Result { + Ok(true) + } + + async fn get_last_n_records( + &self, + _how_many: usize, + ) -> Result, AdapterError> { + Ok(Vec::new()) + } + + async fn remove(&mut self, _key: &Self::Key) -> Result, AdapterError> { + Ok(None) + } + + async fn get_iter( + &self, + ) -> Result + '_>, AdapterError> { + Err(AdapterError::GeneralError(anyhow!(""))) + } + } + + ////////////// + // use mockall::mock; + + // mock! { + // pub StoreAdapterTotoImpl { } + + // #[async_trait] + // impl StoreAdapter for StoreAdapterTotoImpl where K: Sync + Send , R: Sync + Send { + // type Key = K; + // type Record = R; + + // /// Store the given `record`. + // async fn store_record( + // &mut self, + // key: &Self::Key, + // record: &Self::Record, + // ) -> Result<(), AdapterError>; + + // /// Get the record stored using the given `key`. + // async fn get_record(&self, key: &Self::Key) -> Result, AdapterError>; + + // /// Check if a record exist for the given `key`. + // async fn record_exists(&self, key: &Self::Key) -> Result; + + // /// Get the last `n` records in the store + // async fn get_last_n_records( + // &self, + // how_many: usize, + // ) -> Result, AdapterError>; + + // /// remove values from store + // /// + // /// if the value exists it is returned by the adapter otherwise None is returned + // async fn remove(&mut self, key: &Self::Key) -> Result, AdapterError>; + + // /// Get an iterator over the stored values, from the latest to the oldest. + // async fn get_iter(&self) -> Result + '_>, AdapterError>; + // } + // } + fn setup_router( dependency_manager: Arc, ) -> impl Filter + Clone { @@ -145,10 +238,23 @@ mod tests { } #[tokio::test] - async fn test_certificate_pending_get_ok() { + async fn test_certificate_pending_with_content_get_ok_200() -> Result<(), String> { 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) @@ -156,22 +262,22 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &Null, &response, - ); + &StatusCode::OK, + ) } #[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() -> Result<(), String> { let method = Method::GET.as_str(); let path = "/certificate-pending"; + let dependency_manager = initialize_dependencies().await; let response = request() .method(method) @@ -179,21 +285,26 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &Null, &response, - ); + &StatusCode::NO_CONTENT, + ) } #[tokio::test] - async fn test_certificate_pending_get_ko_500() { + async fn test_certificate_pending_get_ko_500() -> Result<(), String> { let method = Method::GET.as_str(); let path = "/certificate-pending"; - let dependency_manager = initialize_dependencies().await; + let mut dependency_manager = initialize_dependencies().await; + + let adapter = MockStoreAdapter {}; + let certificate_pending_store_store = CertificatePendingStore::new(Box::new(adapter)); + dependency_manager.certificate_pending_store = Arc::new(certificate_pending_store_store); let response = request() .method(method) @@ -201,18 +312,19 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &Null, &response, - ); + &StatusCode::INTERNAL_SERVER_ERROR, + ) } #[tokio::test] - async fn test_certificate_certificates_get_ok() { + async fn test_certificate_certificates_get_ok() -> Result<(), String> { let dependency_manager = initialize_dependencies().await; dependency_manager .certificate_repository @@ -229,24 +341,26 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &Null, &response, - ); + &StatusCode::OK, + ) } #[tokio::test] - async fn test_certificate_certificates_get_ko() { + async fn test_certificate_when_error_retrieving_certificates_returns_ko_500( + ) -> Result<(), String> { 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"; @@ -257,18 +371,19 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &Null, &response, - ); + &StatusCode::INTERNAL_SERVER_ERROR, + ) } #[tokio::test] - async fn test_certificate_certificate_hash_get_ok() { + async fn test_certificate_certificate_hash_get_ok() -> Result<(), String> { let dependency_manager = initialize_dependencies().await; dependency_manager .certificate_repository @@ -285,18 +400,21 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + println!("Response: {:?}", response); + + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &Null, &response, - ); + &StatusCode::OK, + ) } #[tokio::test] - async fn test_certificate_certificate_hash_get_ok_404() { + async fn test_certificate_certificate_hash_get_ok_404() -> Result<(), String> { let dependency_manager = initialize_dependencies().await; let method = Method::GET.as_str(); @@ -308,41 +426,48 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + println!("Response: {:?}", response); + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &Null, &response, - ); + &StatusCode::NOT_FOUND, + ) } #[tokio::test] - async fn test_certificate_certificate_hash_get_ko() { + async fn test_certificate_when_error_on_retrieving_certificate_hash_returns_ko_500( + ) -> Result<(), String> { 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; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &Null, &response, - ); + &StatusCode::INTERNAL_SERVER_ERROR, + ) } } From d9e5b2528b80243258fafc26840c3d01823737b4 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Thu, 29 Feb 2024 12:28:35 +0100 Subject: [PATCH 16/29] Fix expected return status code in test --- .../artifact_routes/cardano_transaction.rs | 40 ++++++++++-------- .../mithril_stake_distribution.rs | 41 +++++++++++-------- 2 files changed, 49 insertions(+), 32 deletions(-) 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..e96be61c2fe 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::*; @@ -120,7 +123,7 @@ pub mod tests { } #[tokio::test] - async fn test_cardano_transactions_get_ok() { + async fn test_cardano_transactions_get_ok() -> Result<(), String> { let signed_entity_records = create_signed_entities( SignedEntityType::CardanoTransactions(Beacon::default()), fake_data::cardano_transactions_snapshot(5), @@ -143,18 +146,19 @@ pub mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &Null, &response, - ); + &StatusCode::OK, + ) } #[tokio::test] - async fn test_cardano_transactions_get_ko() { + async fn test_cardano_transactions_get_ko() -> Result<(), String> { let mut mock_http_message_service = MockMessageService::new(); mock_http_message_service .expect_get_cardano_transaction_list_message() @@ -172,18 +176,19 @@ pub mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &Null, &response, - ); + &StatusCode::INTERNAL_SERVER_ERROR, + ) } #[tokio::test] - async fn test_cardano_transaction_get_ok() { + async fn test_cardano_transaction_get_ok() -> Result<(), String> { let signed_entity = create_signed_entities( SignedEntityType::CardanoTransactions(Beacon::default()), fake_data::cardano_transactions_snapshot(1), @@ -209,18 +214,19 @@ pub mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &Null, &response, - ); + &StatusCode::OK, + ) } #[tokio::test] - async fn test_cardano_transaction_ok_norecord() { + async fn test_cardano_transaction_return_404_not_found_when_no_record() -> Result<(), String> { let mut mock_http_message_service = MockMessageService::new(); mock_http_message_service .expect_get_cardano_transaction_message() @@ -238,18 +244,19 @@ pub mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &Null, &response, - ); + &StatusCode::NOT_FOUND, + ) } #[tokio::test] - async fn test_cardano_transaction_get_ko() { + async fn test_cardano_transaction_get_ko() -> Result<(), String> { let mut mock_http_message_service = MockMessageService::new(); mock_http_message_service .expect_get_cardano_transaction_message() @@ -267,13 +274,14 @@ pub mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &Null, &response, - ); + &StatusCode::INTERNAL_SERVER_ERROR, + ) } } 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..bb517bc295a 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::*; @@ -121,7 +124,7 @@ pub mod tests { } #[tokio::test] - async fn test_mithril_stake_distributions_get_ok() { + async fn test_mithril_stake_distributions_get_ok() -> Result<(), String> { let signed_entity_records = create_signed_entities( SignedEntityType::MithrilStakeDistribution(Epoch::default()), fake_data::mithril_stake_distributions(5), @@ -144,18 +147,19 @@ pub mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &Null, &response, - ); + &StatusCode::OK, + ) } #[tokio::test] - async fn test_mithril_stake_distributions_get_ko() { + async fn test_mithril_stake_distributions_get_ko() -> Result<(), String> { let mut mock_http_message_service = MockMessageService::new(); mock_http_message_service .expect_get_mithril_stake_distribution_list_message() @@ -173,18 +177,19 @@ pub mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &Null, &response, - ); + &StatusCode::INTERNAL_SERVER_ERROR, + ) } #[tokio::test] - async fn test_mithril_stake_distribution_get_ok() { + async fn test_mithril_stake_distribution_get_ok() -> Result<(), String> { let signed_entity = create_signed_entities( SignedEntityType::MithrilStakeDistribution(Epoch::default()), fake_data::mithril_stake_distributions(1), @@ -210,18 +215,20 @@ pub mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &Null, &response, - ); + &StatusCode::OK, + ) } #[tokio::test] - async fn test_mithril_stake_distribution_ok_norecord() { + async fn test_mithril_stake_distribution_returns_404_no_found_when_no_record( + ) -> Result<(), String> { let mut mock_http_message_service = MockMessageService::new(); mock_http_message_service .expect_get_mithril_stake_distribution_message() @@ -239,18 +246,19 @@ pub mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &Null, &response, - ); + &StatusCode::NOT_FOUND, + ) } #[tokio::test] - async fn test_mithril_stake_distribution_get_ko() { + async fn test_mithril_stake_distribution_get_ko() -> Result<(), String> { let mut mock_http_message_service = MockMessageService::new(); mock_http_message_service .expect_get_mithril_stake_distribution_message() @@ -268,13 +276,14 @@ pub mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &Null, &response, - ); + &StatusCode::INTERNAL_SERVER_ERROR, + ) } } From f75ccb289ad71bd154ec48a7414a2214abce4fca Mon Sep 17 00:00:00 2001 From: sfauvel Date: Thu, 29 Feb 2024 14:18:43 +0100 Subject: [PATCH 17/29] Adjust expected status code in tests --- .../routes/artifact_routes/snapshot.rs | 62 +++++++++------ .../src/http_server/routes/signer_routes.rs | 76 +++++++++++-------- 2 files changed, 82 insertions(+), 56 deletions(-) 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..9eb68c35387 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::*; @@ -252,7 +255,7 @@ mod tests { } #[tokio::test] - async fn test_snapshots_get_ok() { + async fn test_snapshots_get_ok() -> Result<(), String> { let signed_entities = create_signed_entities( SignedEntityType::CardanoImmutableFilesFull(Beacon::default()), fake_data::snapshots(5), @@ -275,18 +278,19 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &Null, &response, - ); + &StatusCode::OK, + ) } #[tokio::test] - async fn test_snapshots_get_ko() { + async fn test_snapshots_get_ko() -> Result<(), String> { let mut mock_http_message_service = MockMessageService::new(); mock_http_message_service .expect_get_snapshot_list_message() @@ -304,18 +308,19 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &Null, &response, - ); + &StatusCode::INTERNAL_SERVER_ERROR, + ) } #[tokio::test] - async fn test_snapshot_digest_get_ok() { + async fn test_snapshot_digest_get_ok() -> Result<(), String> { let signed_entity = create_signed_entities( SignedEntityType::CardanoImmutableFilesFull(Beacon::default()), fake_data::snapshots(1), @@ -341,18 +346,19 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &Null, &response, - ); + &StatusCode::OK, + ) } #[tokio::test] - async fn test_snapshot_digest_get_ok_nosnapshot() { + async fn test_snapshot_digest_returns_404_not_found_when_no_snapshot() -> Result<(), String> { let mut mock_http_message_service = MockMessageService::new(); mock_http_message_service .expect_get_snapshot_message() @@ -370,18 +376,19 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &Null, &response, - ); + &StatusCode::NOT_FOUND, + ) } #[tokio::test] - async fn test_snapshot_digest_get_ko() { + async fn test_snapshot_digest_get_ko() -> Result<(), String> { let mut mock_http_message_service = MockMessageService::new(); mock_http_message_service .expect_get_snapshot_message() @@ -399,18 +406,20 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &Null, &response, - ); + &StatusCode::INTERNAL_SERVER_ERROR, + ) } #[tokio::test] - async fn test_snapshot_download_get_ok() { + async fn test_snapshot_download_returns_302_found_when_the_snapshot_exists( + ) -> Result<(), String> { let signed_entity = create_signed_entities( SignedEntityType::CardanoImmutableFilesFull(Beacon::default()), fake_data::snapshots(1), @@ -435,18 +444,19 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/gzip", &Null, &response, - ); + &StatusCode::FOUND, + ) } #[tokio::test] - async fn test_snapshot_download_get_ok_nosnapshot() { + async fn test_snapshot_download_returns_404_not_found_when_no_snapshot() -> Result<(), String> { let mut mock_signed_entity_service = MockSignedEntityService::new(); mock_signed_entity_service .expect_get_signed_snapshot_by_id() @@ -464,18 +474,19 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/gzip", &Null, &response, - ); + &StatusCode::NOT_FOUND, + ) } #[tokio::test] - async fn test_snapshot_download_get_ko() { + async fn test_snapshot_download_get_ko() -> Result<(), String> { let mut mock_signed_entity_service = MockSignedEntityService::new(); mock_signed_entity_service .expect_get_signed_snapshot_by_id() @@ -493,13 +504,14 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &Null, &response, - ); + &StatusCode::INTERNAL_SERVER_ERROR, + ) } } diff --git a/mithril-aggregator/src/http_server/routes/signer_routes.rs b/mithril-aggregator/src/http_server/routes/signer_routes.rs index 3f9f988d353..39c402c5ab4 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::{ @@ -279,7 +282,7 @@ mod tests { } #[tokio::test] - async fn test_register_signer_post_ok() { + async fn test_register_signer_post_ok() -> Result<(), String> { let signer_with_stake = fake_data::signers_with_stakes(1).pop().unwrap(); let mut mock_signer_registerer = MockSignerRegisterer::new(); mock_signer_registerer @@ -303,18 +306,19 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &signer, &response, - ); + &StatusCode::CREATED, + ) } #[tokio::test] - async fn test_register_signer_post_ok_existing() { + async fn test_register_signer_post_ok_existing() -> Result<(), String> { let signer_with_stake = fake_data::signers_with_stakes(1).pop().unwrap(); let mut mock_signer_registerer = MockSignerRegisterer::new(); mock_signer_registerer @@ -342,18 +346,19 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &signer, &response, - ); + &StatusCode::CREATED, + ) } #[tokio::test] - async fn test_register_signer_post_ko_400() { + async fn test_register_signer_post_ko_400() -> Result<(), String> { let mut mock_signer_registerer = MockSignerRegisterer::new(); mock_signer_registerer .expect_register_signer() @@ -380,18 +385,19 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &signer, &response, - ); + &StatusCode::BAD_REQUEST, + ) } #[tokio::test] - async fn test_register_signer_post_ko_500() { + async fn test_register_signer_post_ko_500() -> Result<(), String> { let mut mock_signer_registerer = MockSignerRegisterer::new(); mock_signer_registerer .expect_register_signer() @@ -417,18 +423,19 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &signer, &response, - ); + &StatusCode::INTERNAL_SERVER_ERROR, + ) } #[tokio::test] - async fn test_register_signer_post_ko_503() { + async fn test_register_signer_post_ko_503() -> Result<(), String> { let mut mock_signer_registerer = MockSignerRegisterer::new(); mock_signer_registerer .expect_register_signer() @@ -450,14 +457,15 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &signer, &response, - ); + &StatusCode::SERVICE_UNAVAILABLE, + ) } #[tokio::test] @@ -489,7 +497,7 @@ mod tests { } #[tokio::test] - async fn test_registered_signers_get_ok() { + async fn test_registered_signers_get_ok() -> Result<(), String> { let mut mock_verification_key_store = MockVerificationKeyStorer::new(); mock_verification_key_store .expect_get_signers() @@ -507,18 +515,20 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, &format!("{base_path}/{{epoch}}"), "application/json", &Null, &response, - ); + &StatusCode::OK, + ) } #[tokio::test] - async fn test_registered_signers_get_ok_noregistration() { + async fn test_registered_signers_returns_404_not_found_when_no_registration( + ) -> Result<(), String> { let mut mock_verification_key_store = MockVerificationKeyStorer::new(); mock_verification_key_store .expect_get_signers() @@ -536,18 +546,19 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, &format!("{base_path}/{{epoch}}"), "application/json", &Null, &response, - ); + &StatusCode::NOT_FOUND, + ) } #[tokio::test] - async fn test_registered_signers_get_ko() { + async fn test_registered_signers_get_ko() -> Result<(), String> { let mut mock_verification_key_store = MockVerificationKeyStorer::new(); mock_verification_key_store .expect_get_signers() @@ -564,18 +575,19 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, &format!("{base_path}/{{epoch}}"), "application/json", &Null, &response, - ); + &StatusCode::INTERNAL_SERVER_ERROR, + ) } #[tokio::test] - async fn test_signers_tickers_get_ok() { + async fn test_signers_tickers_get_ok() -> Result<(), String> { let mut mock_signer_getter = MockSignerGetter::new(); mock_signer_getter .expect_get_all() @@ -610,18 +622,19 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &Null, &response, - ); + &StatusCode::OK, + ) } #[tokio::test] - async fn test_signers_tickers_get_ko() { + async fn test_signers_tickers_get_ko() -> Result<(), String> { let mut mock_signer_getter = MockSignerGetter::new(); mock_signer_getter .expect_get_all() @@ -639,13 +652,14 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( + APISpec::verify_conformity_with_status( APISpec::get_all_spec_files(), method, path, "application/json", &Null, &response, - ); + &StatusCode::INTERNAL_SERVER_ERROR, + ) } } From cd3e79c441a706e8d02ee81345cd1c0158224643 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Thu, 29 Feb 2024 16:24:30 +0100 Subject: [PATCH 18/29] Check that content_type exists in openapi.yaml --- mithril-common/src/test_utils/apispec.rs | 37 +++++++++++++++++++++--- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/mithril-common/src/test_utils/apispec.rs b/mithril-common/src/test_utils/apispec.rs index 87ae94aec4f..36647e3029b 100644 --- a/mithril-common/src/test_utils/apispec.rs +++ b/mithril-common/src/test_utils/apispec.rs @@ -67,10 +67,10 @@ impl<'a> APISpec<'a> { .path(path) .content_type(content_type) .validate_request(request_body) - .map_err(|e| panic!("OpenAPI invalid request in {spec_file}, reason: {e}\nresponse: {response:#?}")) + .map_err(|e| panic!("OpenAPI invalid request in {spec_file} on route {path}, reason: {e}\nresponse: {response:#?}")) .unwrap() .validate_response(response) - .map_err(|e| panic!("OpenAPI invalid response in {spec_file}, reason: {e}\nresponse: {response:#?}")) + .map_err(|e| panic!("OpenAPI invalid response in {spec_file} on route {path}, reason: {e}\nresponse: {response:#?}")) .unwrap(); } } @@ -163,7 +163,18 @@ impl<'a> APISpec<'a> { }; match response_spec { Some(response_spec) => { - let response_schema = &response_spec["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_schema.as_object() { Some(_) => Err("Non empty body expected".to_string()), @@ -463,6 +474,24 @@ mod tests { .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_verify_conformity() { APISpec::verify_conformity( @@ -522,7 +551,7 @@ mod tests { StatusCode::OK.as_u16() ); let error_message = format!( - "OpenAPI invalid response in {spec_file}, reason: {error_reason}\nresponse: {response:#?}" + "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); From 55bf32c80b6990a13ed6dbc6eab65eede4354ae9 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Thu, 29 Feb 2024 16:48:11 +0100 Subject: [PATCH 19/29] Fix download snapshot test --- .../routes/artifact_routes/snapshot.rs | 21 +++++++++---------- .../src/http_server/routes/root_routes.rs | 1 - 2 files changed, 10 insertions(+), 12 deletions(-) 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 9eb68c35387..5fe810c77ee 100644 --- a/mithril-aggregator/src/http_server/routes/artifact_routes/snapshot.rs +++ b/mithril-aggregator/src/http_server/routes/artifact_routes/snapshot.rs @@ -418,8 +418,7 @@ mod tests { } #[tokio::test] - async fn test_snapshot_download_returns_302_found_when_the_snapshot_exists( - ) -> Result<(), String> { + 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), @@ -444,15 +443,15 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( - APISpec::get_all_spec_files(), - method, - path, - "application/gzip", - &Null, - &response, - &StatusCode::FOUND, - ) + 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] diff --git a/mithril-aggregator/src/http_server/routes/root_routes.rs b/mithril-aggregator/src/http_server/routes/root_routes.rs index d6a0085566c..80f32546276 100644 --- a/mithril-aggregator/src/http_server/routes/root_routes.rs +++ b/mithril-aggregator/src/http_server/routes/root_routes.rs @@ -91,7 +91,6 @@ mod tests { use warp::http::Method; use warp::test::request; use warp::Filter; - use zstd::stream::raw::Status; use super::*; From 9d7fac7477c0d6b3983a9a3a56558ba0e0525588 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Thu, 29 Feb 2024 18:16:23 +0100 Subject: [PATCH 20/29] Remove parameters from query to search in openapi file --- mithril-common/src/test_utils/apispec.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/mithril-common/src/test_utils/apispec.rs b/mithril-common/src/test_utils/apispec.rs index 36647e3029b..56a0d019d82 100644 --- a/mithril-common/src/test_utils/apispec.rs +++ b/mithril-common/src/test_utils/apispec.rs @@ -106,6 +106,7 @@ impl<'a> APISpec<'a> { } /// Validates if a request is valid + // TODO should validate query parameters. fn validate_request(&'a self, request_body: &impl Serialize) -> Result<&APISpec, String> { let path = self.path.unwrap(); let method = self.method.unwrap().to_lowercase(); @@ -140,6 +141,7 @@ impl<'a> APISpec<'a> { 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(); @@ -492,6 +494,17 @@ mod tests { ); } + #[test] + fn test_validate_a_response_with_query_parameters() -> Result<(), String> { + 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| ()) + } + #[test] fn test_verify_conformity() { APISpec::verify_conformity( From e2809cd556668d31fd9a6d279f951abcef9f6944 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Thu, 29 Feb 2024 18:17:42 +0100 Subject: [PATCH 21/29] Align last added tests in fake aggregator with new verify conformity --- .../src/application.rs | 84 +++++++++---------- .../mithril-aggregator-fake/src/handlers.rs | 11 ++- 2 files changed, 49 insertions(+), 46 deletions(-) diff --git a/mithril-test-lab/mithril-aggregator-fake/src/application.rs b/mithril-test-lab/mithril-aggregator-fake/src/application.rs index e561a57cd59..0ee98888318 100644 --- a/mithril-test-lab/mithril-aggregator-fake/src/application.rs +++ b/mithril-test-lab/mithril-aggregator-fake/src/application.rs @@ -404,29 +404,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(), + APISpec::verify_conformity_with_status( + 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; @@ -436,20 +430,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(); - - assert_eq!(StatusCode::NOT_FOUND, response.status()); + let response = http_request(PORT, &path.replace("{hash}", hash)).await; - Ok(()) + APISpec::verify_conformity_with_status( + get_spec_files(), + "GET", + path, + "application/json", + &Null, + &response, + &StatusCode::NOT_FOUND, + ) + .map_err(|e| anyhow!(e)) }); test(task, PORT).await; @@ -459,29 +456,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(), + APISpec::verify_conformity_with_status( + 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; @@ -491,17 +482,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_with_status( + 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/handlers.rs b/mithril-test-lab/mithril-aggregator-fake/src/handlers.rs index 5e27b2d8f6d..487372c390a 100644 --- a/mithril-test-lab/mithril-aggregator-fake/src/handlers.rs +++ b/mithril-test-lab/mithril-aggregator-fake/src/handlers.rs @@ -155,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("".to_string()) + }) } #[derive(serde::Deserialize, Default)] @@ -176,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("".to_string()) }) } From 95e58ce90849cbbcec2b0a4357d387c59d6f31e2 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Fri, 1 Mar 2024 10:07:36 +0100 Subject: [PATCH 22/29] Replace old verify_conformity with the new one with status code --- .../artifact_routes/cardano_transaction.rs | 10 ++-- .../mithril_stake_distribution.rs | 10 ++-- .../routes/artifact_routes/snapshot.rs | 14 ++--- .../http_server/routes/certificate_routes.rs | 16 +++--- .../src/http_server/routes/epoch_routes.rs | 4 +- .../src/http_server/routes/proof_routes.rs | 6 +- .../src/http_server/routes/root_routes.rs | 2 +- .../http_server/routes/signatures_routes.rs | 10 ++-- .../src/http_server/routes/signer_routes.rs | 20 +++---- .../http_server/routes/statistics_routes.rs | 2 +- mithril-common/src/test_utils/apispec.rs | 56 ++----------------- .../src/application.rs | 28 +++++----- 12 files changed, 65 insertions(+), 113 deletions(-) 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 e96be61c2fe..92eb144ed40 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 @@ -146,7 +146,7 @@ pub mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, @@ -176,7 +176,7 @@ pub mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, @@ -214,7 +214,7 @@ pub mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, @@ -244,7 +244,7 @@ pub mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, @@ -274,7 +274,7 @@ pub mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, 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 bb517bc295a..ac74fe83c25 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 @@ -147,7 +147,7 @@ pub mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, @@ -177,7 +177,7 @@ pub mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, @@ -215,7 +215,7 @@ pub mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, @@ -246,7 +246,7 @@ pub mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, @@ -276,7 +276,7 @@ pub mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, 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 5fe810c77ee..6bfcdfa8243 100644 --- a/mithril-aggregator/src/http_server/routes/artifact_routes/snapshot.rs +++ b/mithril-aggregator/src/http_server/routes/artifact_routes/snapshot.rs @@ -278,7 +278,7 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, @@ -308,7 +308,7 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, @@ -346,7 +346,7 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, @@ -376,7 +376,7 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, @@ -406,7 +406,7 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, @@ -473,7 +473,7 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, @@ -503,7 +503,7 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, diff --git a/mithril-aggregator/src/http_server/routes/certificate_routes.rs b/mithril-aggregator/src/http_server/routes/certificate_routes.rs index 94c483173f7..161fe59adbf 100644 --- a/mithril-aggregator/src/http_server/routes/certificate_routes.rs +++ b/mithril-aggregator/src/http_server/routes/certificate_routes.rs @@ -262,7 +262,7 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, @@ -285,7 +285,7 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, @@ -312,7 +312,7 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, @@ -341,7 +341,7 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, @@ -371,7 +371,7 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, @@ -402,7 +402,7 @@ mod tests { println!("Response: {:?}", response); - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, @@ -427,7 +427,7 @@ mod tests { .await; println!("Response: {:?}", response); - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, @@ -460,7 +460,7 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, diff --git a/mithril-aggregator/src/http_server/routes/epoch_routes.rs b/mithril-aggregator/src/http_server/routes/epoch_routes.rs index 928dc34a457..2a32e870039 100644 --- a/mithril-aggregator/src/http_server/routes/epoch_routes.rs +++ b/mithril-aggregator/src/http_server/routes/epoch_routes.rs @@ -103,7 +103,7 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, @@ -126,7 +126,7 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, diff --git a/mithril-aggregator/src/http_server/routes/proof_routes.rs b/mithril-aggregator/src/http_server/routes/proof_routes.rs index 348f69ed337..a71c11f60e9 100644 --- a/mithril-aggregator/src/http_server/routes/proof_routes.rs +++ b/mithril-aggregator/src/http_server/routes/proof_routes.rs @@ -164,7 +164,7 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, @@ -192,7 +192,7 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, @@ -225,7 +225,7 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, diff --git a/mithril-aggregator/src/http_server/routes/root_routes.rs b/mithril-aggregator/src/http_server/routes/root_routes.rs index 80f32546276..37b3cc6a693 100644 --- a/mithril-aggregator/src/http_server/routes/root_routes.rs +++ b/mithril-aggregator/src/http_server/routes/root_routes.rs @@ -150,7 +150,7 @@ mod tests { } ); - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, diff --git a/mithril-aggregator/src/http_server/routes/signatures_routes.rs b/mithril-aggregator/src/http_server/routes/signatures_routes.rs index cf1862a3d77..a6a897d859d 100644 --- a/mithril-aggregator/src/http_server/routes/signatures_routes.rs +++ b/mithril-aggregator/src/http_server/routes/signatures_routes.rs @@ -153,7 +153,7 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, @@ -186,7 +186,7 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, @@ -220,7 +220,7 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, @@ -254,7 +254,7 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, @@ -286,7 +286,7 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, diff --git a/mithril-aggregator/src/http_server/routes/signer_routes.rs b/mithril-aggregator/src/http_server/routes/signer_routes.rs index 39c402c5ab4..291276114e3 100644 --- a/mithril-aggregator/src/http_server/routes/signer_routes.rs +++ b/mithril-aggregator/src/http_server/routes/signer_routes.rs @@ -306,7 +306,7 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, @@ -346,7 +346,7 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, @@ -385,7 +385,7 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, @@ -423,7 +423,7 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, @@ -457,7 +457,7 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, @@ -515,7 +515,7 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, &format!("{base_path}/{{epoch}}"), @@ -546,7 +546,7 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, &format!("{base_path}/{{epoch}}"), @@ -575,7 +575,7 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, &format!("{base_path}/{{epoch}}"), @@ -622,7 +622,7 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, @@ -652,7 +652,7 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, diff --git a/mithril-aggregator/src/http_server/routes/statistics_routes.rs b/mithril-aggregator/src/http_server/routes/statistics_routes.rs index d238360ebd8..e3a8dd64f33 100644 --- a/mithril-aggregator/src/http_server/routes/statistics_routes.rs +++ b/mithril-aggregator/src/http_server/routes/statistics_routes.rs @@ -97,7 +97,7 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - let result = APISpec::verify_conformity_with_status( + let result = APISpec::verify_conformity( APISpec::get_all_spec_files(), method, path, diff --git a/mithril-common/src/test_utils/apispec.rs b/mithril-common/src/test_utils/apispec.rs index 56a0d019d82..759dcf759ff 100644 --- a/mithril-common/src/test_utils/apispec.rs +++ b/mithril-common/src/test_utils/apispec.rs @@ -20,7 +20,7 @@ pub struct APISpec<'a> { impl<'a> APISpec<'a> { /// Verify conformity helper of API Specs - pub fn verify_conformity_with_status( + pub fn verify_conformity( spec_files: Vec, method: &str, path: &str, @@ -52,29 +52,6 @@ impl<'a> APISpec<'a> { Ok(()) } - /// Verify conformity helper of API Specs - pub fn verify_conformity( - spec_files: Vec, - method: &str, - path: &str, - content_type: &str, - request_body: &impl Serialize, - response: &Response, - ) { - for spec_file in spec_files { - 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} on route {path}, reason: {e}\nresponse: {response:#?}")) - .unwrap() - .validate_response(response) - .map_err(|e| panic!("OpenAPI invalid response in {spec_file} on route {path}, reason: {e}\nresponse: {response:#?}")) - .unwrap(); - } - } - /// APISpec factory from spec pub fn from_file(path: &str) -> APISpec<'a> { let yaml_spec = std::fs::read_to_string(path).unwrap(); @@ -505,34 +482,9 @@ mod tests { .map(|_apispec| ()) } - #[test] - fn test_verify_conformity() { - APISpec::verify_conformity( - APISpec::get_all_spec_files(), - Method::GET.as_str(), - "/certificate-pending", - "application/json", - &Null, - &build_json_response(200, CertificatePendingMessage::dummy()), - ); - } - - #[test] - #[should_panic] - fn test_verify_conformity_should_panic_with_bad_response() { - APISpec::verify_conformity( - APISpec::get_all_spec_files(), - Method::GET.as_str(), - "/certificate-pending", - "application/json", - &Null, - &Response::::new(Bytes::new()), - ); - } - #[test] fn test_verify_conformity_with_expected_status() -> Result<(), String> { - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( APISpec::get_all_spec_files(), Method::GET.as_str(), "/certificate-pending", @@ -548,7 +500,7 @@ mod tests { let response = build_json_response(200, CertificatePendingMessage::dummy()); let spec_file = APISpec::get_default_spec_file(); - let result = APISpec::verify_conformity_with_status( + let result = APISpec::verify_conformity( vec![spec_file.clone()], Method::GET.as_str(), "/certificate-pending", @@ -572,7 +524,7 @@ mod tests { #[test] fn test_verify_conformity_when_no_spec_file_returns_error() { - let result = APISpec::verify_conformity_with_status( + let result = APISpec::verify_conformity( vec![], Method::GET.as_str(), "/certificate-pending", diff --git a/mithril-test-lab/mithril-aggregator-fake/src/application.rs b/mithril-test-lab/mithril-aggregator-fake/src/application.rs index 0ee98888318..4f2ba6b7af7 100644 --- a/mithril-test-lab/mithril-aggregator-fake/src/application.rs +++ b/mithril-test-lab/mithril-aggregator-fake/src/application.rs @@ -154,7 +154,7 @@ mod tests { let path = "/certificates"; let response = http_request(PORT, path).await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( get_spec_files(), "GET", path, @@ -179,7 +179,7 @@ mod tests { let path = "/artifact/snapshots"; let response = http_request(PORT, path).await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( get_spec_files(), "GET", path, @@ -204,7 +204,7 @@ mod tests { let path = "/artifact/mithril-stake-distributions"; let response = http_request(PORT, path).await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( get_spec_files(), "GET", path, @@ -231,7 +231,7 @@ mod tests { let response = http_request(PORT, &path.replace("{certificate_hash}", certificate_hash)).await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( get_spec_files(), "GET", path, @@ -257,7 +257,7 @@ mod tests { let response = http_request(PORT, &path.replace("{certificate_hash}", "whatever")).await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( get_spec_files(), "GET", path, @@ -283,7 +283,7 @@ mod tests { let digest = default_values::snapshot_digests()[0]; let response = http_request(PORT, path.replace("{digest}", digest).as_str()).await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( get_spec_files(), "GET", path, @@ -309,7 +309,7 @@ mod tests { let digest = "whatever"; let response = http_request(PORT, &path.replace("{digest}", digest)).await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( get_spec_files(), "GET", path, @@ -335,7 +335,7 @@ mod tests { let hash = default_values::msd_hashes()[0]; let response = http_request(PORT, &path.replace("{hash}", hash)).await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( get_spec_files(), "GET", path, @@ -360,7 +360,7 @@ mod tests { let path = "/artifact/mithril-stake-distribution/{hash}"; let response = http_request(PORT, &path.replace("{hash}", "whatever")).await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( get_spec_files(), "GET", path, @@ -385,7 +385,7 @@ mod tests { let path = "/epoch-settings"; let response = http_request(PORT, path).await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( get_spec_files(), "GET", path, @@ -411,7 +411,7 @@ mod tests { let hash = default_values::ctx_snapshot_hashes()[0]; let response = http_request(PORT, &path.replace("{hash}", hash)).await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( get_spec_files(), "GET", path, @@ -437,7 +437,7 @@ mod tests { let hash = "whatever"; let response = http_request(PORT, &path.replace("{hash}", hash)).await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( get_spec_files(), "GET", path, @@ -463,7 +463,7 @@ mod tests { let hash = default_values::proof_transaction_hashes()[0]; let response = http_request(PORT, &path.replace("{hash}", hash)).await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( get_spec_files(), "GET", path, @@ -488,7 +488,7 @@ mod tests { let path = "/proof/cardano-transaction"; let response = http_request(PORT, &path).await; - APISpec::verify_conformity_with_status( + APISpec::verify_conformity( get_spec_files(), "GET", path, From ce844f1dcd6edc0748bb38f75758889f344ca6af Mon Sep 17 00:00:00 2001 From: sfauvel Date: Fri, 1 Mar 2024 13:02:21 +0100 Subject: [PATCH 23/29] Check query parameter name when there is one --- mithril-common/Cargo.toml | 4 +- mithril-common/src/test_utils/apispec.rs | 113 +++++++++++++++++- .../src/application.rs | 2 +- 3 files changed, 112 insertions(+), 7 deletions(-) diff --git a/mithril-common/Cargo.toml b/mithril-common/Cargo.toml index abf1658992c..cadcd708bbf 100644 --- a/mithril-common/Cargo.toml +++ b/mithril-common/Cargo.toml @@ -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", features = ["json"], optional = true } semver = "1.0.21" serde = { version = "1.0.196", features = ["derive"] } serde_bytes = "0.11.14" @@ -85,7 +86,6 @@ criterion = { version = "0.5.1", features = ["html_reports", "async_tokio"] } mockall = "0.12.1" pallas-crypto = "0.23.0" rand_core = { version = "0.6.4", features = ["getrandom"] } -reqwest = { version = "0.11.23", features = ["json"] } slog-async = "2.8.0" slog-scope = "4.4.0" slog-term = "2.9.0" @@ -119,7 +119,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 759dcf759ff..44529da46f2 100644 --- a/mithril-common/src/test_utils/apispec.rs +++ b/mithril-common/src/test_utils/apispec.rs @@ -2,8 +2,10 @@ 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; @@ -83,18 +85,46 @@ impl<'a> APISpec<'a> { } /// Validates if a request is valid - // TODO should validate query parameters. 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 = - &self.openapi["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) } + 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, @@ -225,6 +255,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; @@ -298,7 +347,7 @@ mod tests { // 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()), + entities::InternalServerError::new("an error occurred".to_string()), ); assert!(APISpec::from_file(&APISpec::get_default_spec_file()) @@ -482,6 +531,62 @@ mod tests { .map(|_apispec| ()) } + #[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() -> Result<(), String> { + 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| ()) + } + + #[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() -> Result<(), String> { APISpec::verify_conformity( diff --git a/mithril-test-lab/mithril-aggregator-fake/src/application.rs b/mithril-test-lab/mithril-aggregator-fake/src/application.rs index 4f2ba6b7af7..4fc0cc7ed56 100644 --- a/mithril-test-lab/mithril-aggregator-fake/src/application.rs +++ b/mithril-test-lab/mithril-aggregator-fake/src/application.rs @@ -486,7 +486,7 @@ mod tests { yield_now().await; let path = "/proof/cardano-transaction"; - let response = http_request(PORT, &path).await; + let response = http_request(PORT, path).await; APISpec::verify_conformity( get_spec_files(), From 18b27b7b58e27fe3a3b3b11d9ef3578a393fe4f1 Mon Sep 17 00:00:00 2001 From: Damien LACHAUME / PALO-IT Date: Mon, 4 Mar 2024 10:46:47 +0100 Subject: [PATCH 24/29] Rework `DumbStoreAdapter` after review --- .../src/store/adapter/dumb_adapter.rs | 33 ++++++- .../http_server/routes/certificate_routes.rs | 91 +------------------ 2 files changed, 32 insertions(+), 92 deletions(-) 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/src/http_server/routes/certificate_routes.rs b/mithril-aggregator/src/http_server/routes/certificate_routes.rs index 161fe59adbf..990a3c603c5 100644 --- a/mithril-aggregator/src/http_server/routes/certificate_routes.rs +++ b/mithril-aggregator/src/http_server/routes/certificate_routes.rs @@ -121,13 +121,11 @@ mod handlers { #[cfg(test)] mod tests { use anyhow::anyhow; - use async_trait::async_trait; use mithril_common::{ entities::CertificatePending, test_utils::{apispec::APISpec, fake_data}, }; - use mithril_persistence::store::adapter::AdapterError; - use mithril_persistence::store::adapter::StoreAdapter; + use mithril_persistence::store::adapter::DumbStoreAdapter; use serde_json::Value::Null; use warp::{ http::{Method, StatusCode}, @@ -141,89 +139,6 @@ mod tests { use super::*; - /////////////// - - pub struct MockStoreAdapter {} - - #[async_trait] - impl StoreAdapter for MockStoreAdapter { - type Key = String; - type Record = CertificatePending; - - async fn store_record( - &mut self, - _key: &Self::Key, - _record: &Self::Record, - ) -> Result<(), AdapterError> { - Ok(()) - } - - async fn get_record(&self, _key: &Self::Key) -> Result, AdapterError> { - Err(AdapterError::GeneralError(anyhow!("Ca marche pô"))) - } - - async fn record_exists(&self, _key: &Self::Key) -> Result { - Ok(true) - } - - async fn get_last_n_records( - &self, - _how_many: usize, - ) -> Result, AdapterError> { - Ok(Vec::new()) - } - - async fn remove(&mut self, _key: &Self::Key) -> Result, AdapterError> { - Ok(None) - } - - async fn get_iter( - &self, - ) -> Result + '_>, AdapterError> { - Err(AdapterError::GeneralError(anyhow!(""))) - } - } - - ////////////// - // use mockall::mock; - - // mock! { - // pub StoreAdapterTotoImpl { } - - // #[async_trait] - // impl StoreAdapter for StoreAdapterTotoImpl where K: Sync + Send , R: Sync + Send { - // type Key = K; - // type Record = R; - - // /// Store the given `record`. - // async fn store_record( - // &mut self, - // key: &Self::Key, - // record: &Self::Record, - // ) -> Result<(), AdapterError>; - - // /// Get the record stored using the given `key`. - // async fn get_record(&self, key: &Self::Key) -> Result, AdapterError>; - - // /// Check if a record exist for the given `key`. - // async fn record_exists(&self, key: &Self::Key) -> Result; - - // /// Get the last `n` records in the store - // async fn get_last_n_records( - // &self, - // how_many: usize, - // ) -> Result, AdapterError>; - - // /// remove values from store - // /// - // /// if the value exists it is returned by the adapter otherwise None is returned - // async fn remove(&mut self, key: &Self::Key) -> Result, AdapterError>; - - // /// Get an iterator over the stored values, from the latest to the oldest. - // async fn get_iter(&self) -> Result + '_>, AdapterError>; - // } - // } - fn setup_router( dependency_manager: Arc, ) -> impl Filter + Clone { @@ -302,8 +217,8 @@ mod tests { let path = "/certificate-pending"; let mut dependency_manager = initialize_dependencies().await; - let adapter = MockStoreAdapter {}; - let certificate_pending_store_store = CertificatePendingStore::new(Box::new(adapter)); + 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() From 2681733e2d70fdd530c88fc1ed81e8d2876aee40 Mon Sep 17 00:00:00 2001 From: Damien LACHAUME / PALO-IT Date: Mon, 4 Mar 2024 11:05:47 +0100 Subject: [PATCH 25/29] Replace test returned `Result` using `unwrap()` on `validate_conformity()` calls --- .../artifact_routes/cardano_transaction.rs | 15 ++++++--- .../mithril_stake_distribution.rs | 16 ++++++---- .../routes/artifact_routes/snapshot.rs | 21 ++++++++----- .../http_server/routes/certificate_routes.rs | 29 +++++++++-------- .../src/http_server/routes/epoch_routes.rs | 6 ++-- .../src/http_server/routes/proof_routes.rs | 9 ++++-- .../src/http_server/routes/root_routes.rs | 3 +- .../http_server/routes/signatures_routes.rs | 15 ++++++--- .../src/http_server/routes/signer_routes.rs | 31 ++++++++++++------- .../http_server/routes/statistics_routes.rs | 4 +-- mithril-common/src/test_utils/apispec.rs | 23 ++++++++------ 11 files changed, 107 insertions(+), 65 deletions(-) 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 92eb144ed40..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 @@ -123,7 +123,7 @@ pub mod tests { } #[tokio::test] - async fn test_cardano_transactions_get_ok() -> Result<(), String> { + async fn test_cardano_transactions_get_ok() { let signed_entity_records = create_signed_entities( SignedEntityType::CardanoTransactions(Beacon::default()), fake_data::cardano_transactions_snapshot(5), @@ -155,10 +155,11 @@ pub mod tests { &response, &StatusCode::OK, ) + .unwrap(); } #[tokio::test] - async fn test_cardano_transactions_get_ko() -> Result<(), String> { + async fn test_cardano_transactions_get_ko() { let mut mock_http_message_service = MockMessageService::new(); mock_http_message_service .expect_get_cardano_transaction_list_message() @@ -185,10 +186,11 @@ pub mod tests { &response, &StatusCode::INTERNAL_SERVER_ERROR, ) + .unwrap(); } #[tokio::test] - async fn test_cardano_transaction_get_ok() -> Result<(), String> { + async fn test_cardano_transaction_get_ok() { let signed_entity = create_signed_entities( SignedEntityType::CardanoTransactions(Beacon::default()), fake_data::cardano_transactions_snapshot(1), @@ -223,10 +225,11 @@ pub mod tests { &response, &StatusCode::OK, ) + .unwrap(); } #[tokio::test] - async fn test_cardano_transaction_return_404_not_found_when_no_record() -> Result<(), String> { + 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() @@ -253,10 +256,11 @@ pub mod tests { &response, &StatusCode::NOT_FOUND, ) + .unwrap(); } #[tokio::test] - async fn test_cardano_transaction_get_ko() -> Result<(), String> { + async fn test_cardano_transaction_get_ko() { let mut mock_http_message_service = MockMessageService::new(); mock_http_message_service .expect_get_cardano_transaction_message() @@ -283,5 +287,6 @@ pub mod tests { &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 ac74fe83c25..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 @@ -124,7 +124,7 @@ pub mod tests { } #[tokio::test] - async fn test_mithril_stake_distributions_get_ok() -> Result<(), String> { + async fn test_mithril_stake_distributions_get_ok() { let signed_entity_records = create_signed_entities( SignedEntityType::MithrilStakeDistribution(Epoch::default()), fake_data::mithril_stake_distributions(5), @@ -156,10 +156,11 @@ pub mod tests { &response, &StatusCode::OK, ) + .unwrap(); } #[tokio::test] - async fn test_mithril_stake_distributions_get_ko() -> Result<(), String> { + async fn test_mithril_stake_distributions_get_ko() { let mut mock_http_message_service = MockMessageService::new(); mock_http_message_service .expect_get_mithril_stake_distribution_list_message() @@ -186,10 +187,11 @@ pub mod tests { &response, &StatusCode::INTERNAL_SERVER_ERROR, ) + .unwrap(); } #[tokio::test] - async fn test_mithril_stake_distribution_get_ok() -> Result<(), String> { + async fn test_mithril_stake_distribution_get_ok() { let signed_entity = create_signed_entities( SignedEntityType::MithrilStakeDistribution(Epoch::default()), fake_data::mithril_stake_distributions(1), @@ -224,11 +226,11 @@ pub mod tests { &response, &StatusCode::OK, ) + .unwrap(); } #[tokio::test] - async fn test_mithril_stake_distribution_returns_404_no_found_when_no_record( - ) -> Result<(), String> { + 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() @@ -255,10 +257,11 @@ pub mod tests { &response, &StatusCode::NOT_FOUND, ) + .unwrap(); } #[tokio::test] - async fn test_mithril_stake_distribution_get_ko() -> Result<(), String> { + async fn test_mithril_stake_distribution_get_ko() { let mut mock_http_message_service = MockMessageService::new(); mock_http_message_service .expect_get_mithril_stake_distribution_message() @@ -285,5 +288,6 @@ pub mod tests { &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 6bfcdfa8243..7de2f848f3b 100644 --- a/mithril-aggregator/src/http_server/routes/artifact_routes/snapshot.rs +++ b/mithril-aggregator/src/http_server/routes/artifact_routes/snapshot.rs @@ -255,7 +255,7 @@ mod tests { } #[tokio::test] - async fn test_snapshots_get_ok() -> Result<(), String> { + async fn test_snapshots_get_ok() { let signed_entities = create_signed_entities( SignedEntityType::CardanoImmutableFilesFull(Beacon::default()), fake_data::snapshots(5), @@ -287,10 +287,11 @@ mod tests { &response, &StatusCode::OK, ) + .unwrap(); } #[tokio::test] - async fn test_snapshots_get_ko() -> Result<(), String> { + async fn test_snapshots_get_ko() { let mut mock_http_message_service = MockMessageService::new(); mock_http_message_service .expect_get_snapshot_list_message() @@ -317,10 +318,11 @@ mod tests { &response, &StatusCode::INTERNAL_SERVER_ERROR, ) + .unwrap(); } #[tokio::test] - async fn test_snapshot_digest_get_ok() -> Result<(), String> { + async fn test_snapshot_digest_get_ok() { let signed_entity = create_signed_entities( SignedEntityType::CardanoImmutableFilesFull(Beacon::default()), fake_data::snapshots(1), @@ -355,10 +357,11 @@ mod tests { &response, &StatusCode::OK, ) + .unwrap(); } #[tokio::test] - async fn test_snapshot_digest_returns_404_not_found_when_no_snapshot() -> Result<(), String> { + 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() @@ -385,10 +388,11 @@ mod tests { &response, &StatusCode::NOT_FOUND, ) + .unwrap(); } #[tokio::test] - async fn test_snapshot_digest_get_ko() -> Result<(), String> { + async fn test_snapshot_digest_get_ko() { let mut mock_http_message_service = MockMessageService::new(); mock_http_message_service .expect_get_snapshot_message() @@ -415,6 +419,7 @@ mod tests { &response, &StatusCode::INTERNAL_SERVER_ERROR, ) + .unwrap(); } #[tokio::test] @@ -455,7 +460,7 @@ mod tests { } #[tokio::test] - async fn test_snapshot_download_returns_404_not_found_when_no_snapshot() -> Result<(), String> { + 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() @@ -482,10 +487,11 @@ mod tests { &response, &StatusCode::NOT_FOUND, ) + .unwrap(); } #[tokio::test] - async fn test_snapshot_download_get_ko() -> Result<(), String> { + async fn test_snapshot_download_get_ko() { let mut mock_signed_entity_service = MockSignedEntityService::new(); mock_signed_entity_service .expect_get_signed_snapshot_by_id() @@ -512,5 +518,6 @@ mod tests { &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 990a3c603c5..cec2034b67a 100644 --- a/mithril-aggregator/src/http_server/routes/certificate_routes.rs +++ b/mithril-aggregator/src/http_server/routes/certificate_routes.rs @@ -153,7 +153,7 @@ mod tests { } #[tokio::test] - async fn test_certificate_pending_with_content_get_ok_200() -> Result<(), String> { + 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; @@ -186,10 +186,11 @@ mod tests { &response, &StatusCode::OK, ) + .unwrap(); } #[tokio::test] - async fn test_certificate_pending_without_content_get_ok_204() -> Result<(), String> { + 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; @@ -209,10 +210,11 @@ mod tests { &response, &StatusCode::NO_CONTENT, ) + .unwrap(); } #[tokio::test] - async fn test_certificate_pending_get_ko_500() -> Result<(), String> { + async fn test_certificate_pending_get_ko_500() { let method = Method::GET.as_str(); let path = "/certificate-pending"; let mut dependency_manager = initialize_dependencies().await; @@ -236,10 +238,11 @@ mod tests { &response, &StatusCode::INTERNAL_SERVER_ERROR, ) + .unwrap(); } #[tokio::test] - async fn test_certificate_certificates_get_ok() -> Result<(), String> { + async fn test_certificate_certificates_get_ok() { let dependency_manager = initialize_dependencies().await; dependency_manager .certificate_repository @@ -265,11 +268,11 @@ mod tests { &response, &StatusCode::OK, ) + .unwrap(); } #[tokio::test] - async fn test_certificate_when_error_retrieving_certificates_returns_ko_500( - ) -> Result<(), String> { + async fn test_certificate_when_error_retrieving_certificates_returns_ko_500() { let mut dependency_manager = initialize_dependencies().await; let mut message_service = MockMessageService::new(); message_service @@ -295,10 +298,11 @@ mod tests { &response, &StatusCode::INTERNAL_SERVER_ERROR, ) + .unwrap(); } #[tokio::test] - async fn test_certificate_certificate_hash_get_ok() -> Result<(), String> { + async fn test_certificate_certificate_hash_get_ok() { let dependency_manager = initialize_dependencies().await; dependency_manager .certificate_repository @@ -315,8 +319,6 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - println!("Response: {:?}", response); - APISpec::verify_conformity( APISpec::get_all_spec_files(), method, @@ -326,10 +328,11 @@ mod tests { &response, &StatusCode::OK, ) + .unwrap(); } #[tokio::test] - async fn test_certificate_certificate_hash_get_ok_404() -> Result<(), String> { + async fn test_certificate_certificate_hash_get_ok_404() { let dependency_manager = initialize_dependencies().await; let method = Method::GET.as_str(); @@ -341,7 +344,6 @@ mod tests { .reply(&setup_router(Arc::new(dependency_manager))) .await; - println!("Response: {:?}", response); APISpec::verify_conformity( APISpec::get_all_spec_files(), method, @@ -351,11 +353,11 @@ mod tests { &response, &StatusCode::NOT_FOUND, ) + .unwrap(); } #[tokio::test] - async fn test_certificate_when_error_on_retrieving_certificate_hash_returns_ko_500( - ) -> Result<(), String> { + async fn test_certificate_when_error_on_retrieving_certificate_hash_returns_ko_500() { let mut dependency_manager = initialize_dependencies().await; let mut message_service = MockMessageService::new(); message_service @@ -384,5 +386,6 @@ mod tests { &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 2a32e870039..a3176cf8271 100644 --- a/mithril-aggregator/src/http_server/routes/epoch_routes.rs +++ b/mithril-aggregator/src/http_server/routes/epoch_routes.rs @@ -89,7 +89,7 @@ mod tests { } #[tokio::test] - async fn test_epoch_settings_get_ok() -> Result<(), String> { + async fn test_epoch_settings_get_ok() { let method = Method::GET.as_str(); let path = "/epoch-settings"; let mut dependency_manager = initialize_dependencies().await; @@ -112,10 +112,11 @@ mod tests { &response, &StatusCode::OK, ) + .unwrap(); } #[tokio::test] - async fn test_epoch_settings_get_ko_500() -> Result<(), String> { + async fn test_epoch_settings_get_ko_500() { let method = Method::GET.as_str(); let path = "/epoch-settings"; let dependency_manager = initialize_dependencies().await; @@ -135,5 +136,6 @@ mod tests { &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 a71c11f60e9..1bdb6a8f326 100644 --- a/mithril-aggregator/src/http_server/routes/proof_routes.rs +++ b/mithril-aggregator/src/http_server/routes/proof_routes.rs @@ -137,7 +137,7 @@ mod tests { } #[tokio::test] - async fn proof_cardano_transaction_ok() -> Result<(), String> { + async fn proof_cardano_transaction_ok() { let config = Configuration::new_sample(); let mut builder = DependenciesBuilder::new(config); let mut dependency_manager = builder.build_dependency_container().await.unwrap(); @@ -173,10 +173,11 @@ mod tests { &response, &StatusCode::OK, ) + .unwrap(); } #[tokio::test] - async fn proof_cardano_transaction_not_found() -> Result<(), String> { + async fn proof_cardano_transaction_not_found() { let config = Configuration::new_sample(); let mut builder = DependenciesBuilder::new(config); let dependency_manager = builder.build_dependency_container().await.unwrap(); @@ -201,10 +202,11 @@ mod tests { &response, &StatusCode::NOT_FOUND, ) + .unwrap(); } #[tokio::test] - async fn proof_cardano_transaction_ko() -> Result<(), String> { + async fn proof_cardano_transaction_ko() { let config = Configuration::new_sample(); let mut builder = DependenciesBuilder::new(config); let mut dependency_manager = builder.build_dependency_container().await.unwrap(); @@ -234,5 +236,6 @@ mod tests { &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 37b3cc6a693..ad9f1862ec6 100644 --- a/mithril-aggregator/src/http_server/routes/root_routes.rs +++ b/mithril-aggregator/src/http_server/routes/root_routes.rs @@ -108,7 +108,7 @@ mod tests { } #[tokio::test] - async fn test_root_route_ok() -> Result<(), String> { + async fn test_root_route_ok() { let method = Method::GET.as_str(); let path = "/"; let mut dependency_manager = initialize_dependencies().await; @@ -159,5 +159,6 @@ mod tests { &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 a6a897d859d..8f8cdb69bd5 100644 --- a/mithril-aggregator/src/http_server/routes/signatures_routes.rs +++ b/mithril-aggregator/src/http_server/routes/signatures_routes.rs @@ -133,7 +133,7 @@ mod tests { } #[tokio::test] - async fn test_register_signatures_post_ok() -> Result<(), String> { + async fn test_register_signatures_post_ok() { let mut mock_certifier_service = MockCertifierService::new(); mock_certifier_service .expect_register_single_signature() @@ -162,10 +162,11 @@ mod tests { &response, &StatusCode::CREATED, ) + .unwrap(); } #[tokio::test] - async fn test_register_signatures_post_ko_400() -> Result<(), String> { + async fn test_register_signatures_post_ko_400() { let mut mock_certifier_service = MockCertifierService::new(); mock_certifier_service .expect_register_single_signature() @@ -195,10 +196,11 @@ mod tests { &response, &StatusCode::BAD_REQUEST, ) + .unwrap(); } #[tokio::test] - async fn test_register_signatures_post_ko_404() -> Result<(), String> { + async fn test_register_signatures_post_ko_404() { let signed_entity_type = SignedEntityType::dummy(); let message = RegisterSignatureMessage::dummy(); let mut mock_certifier_service = MockCertifierService::new(); @@ -229,10 +231,11 @@ mod tests { &response, &StatusCode::NOT_FOUND, ) + .unwrap(); } #[tokio::test] - async fn test_register_signatures_post_ko_410() -> Result<(), String> { + async fn test_register_signatures_post_ko_410() { let signed_entity_type = SignedEntityType::dummy(); let message = RegisterSignatureMessage::dummy(); let mut mock_certifier_service = MockCertifierService::new(); @@ -263,10 +266,11 @@ mod tests { &response, &StatusCode::GONE, ) + .unwrap(); } #[tokio::test] - async fn test_register_signatures_post_ko_500() -> Result<(), String> { + async fn test_register_signatures_post_ko_500() { let mut mock_certifier_service = MockCertifierService::new(); mock_certifier_service .expect_register_single_signature() @@ -295,5 +299,6 @@ mod tests { &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 291276114e3..40cf911bb85 100644 --- a/mithril-aggregator/src/http_server/routes/signer_routes.rs +++ b/mithril-aggregator/src/http_server/routes/signer_routes.rs @@ -282,7 +282,7 @@ mod tests { } #[tokio::test] - async fn test_register_signer_post_ok() -> Result<(), String> { + async fn test_register_signer_post_ok() { let signer_with_stake = fake_data::signers_with_stakes(1).pop().unwrap(); let mut mock_signer_registerer = MockSignerRegisterer::new(); mock_signer_registerer @@ -315,10 +315,11 @@ mod tests { &response, &StatusCode::CREATED, ) + .unwrap(); } #[tokio::test] - async fn test_register_signer_post_ok_existing() -> Result<(), String> { + async fn test_register_signer_post_ok_existing() { let signer_with_stake = fake_data::signers_with_stakes(1).pop().unwrap(); let mut mock_signer_registerer = MockSignerRegisterer::new(); mock_signer_registerer @@ -355,10 +356,11 @@ mod tests { &response, &StatusCode::CREATED, ) + .unwrap(); } #[tokio::test] - async fn test_register_signer_post_ko_400() -> Result<(), String> { + async fn test_register_signer_post_ko_400() { let mut mock_signer_registerer = MockSignerRegisterer::new(); mock_signer_registerer .expect_register_signer() @@ -394,10 +396,11 @@ mod tests { &response, &StatusCode::BAD_REQUEST, ) + .unwrap(); } #[tokio::test] - async fn test_register_signer_post_ko_500() -> Result<(), String> { + async fn test_register_signer_post_ko_500() { let mut mock_signer_registerer = MockSignerRegisterer::new(); mock_signer_registerer .expect_register_signer() @@ -432,10 +435,11 @@ mod tests { &response, &StatusCode::INTERNAL_SERVER_ERROR, ) + .unwrap(); } #[tokio::test] - async fn test_register_signer_post_ko_503() -> Result<(), String> { + async fn test_register_signer_post_ko_503() { let mut mock_signer_registerer = MockSignerRegisterer::new(); mock_signer_registerer .expect_register_signer() @@ -466,6 +470,7 @@ mod tests { &response, &StatusCode::SERVICE_UNAVAILABLE, ) + .unwrap(); } #[tokio::test] @@ -497,7 +502,7 @@ mod tests { } #[tokio::test] - async fn test_registered_signers_get_ok() -> Result<(), String> { + async fn test_registered_signers_get_ok() { let mut mock_verification_key_store = MockVerificationKeyStorer::new(); mock_verification_key_store .expect_get_signers() @@ -524,11 +529,11 @@ mod tests { &response, &StatusCode::OK, ) + .unwrap(); } #[tokio::test] - async fn test_registered_signers_returns_404_not_found_when_no_registration( - ) -> Result<(), String> { + 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() @@ -555,10 +560,11 @@ mod tests { &response, &StatusCode::NOT_FOUND, ) + .unwrap(); } #[tokio::test] - async fn test_registered_signers_get_ko() -> Result<(), String> { + async fn test_registered_signers_get_ko() { let mut mock_verification_key_store = MockVerificationKeyStorer::new(); mock_verification_key_store .expect_get_signers() @@ -584,10 +590,11 @@ mod tests { &response, &StatusCode::INTERNAL_SERVER_ERROR, ) + .unwrap(); } #[tokio::test] - async fn test_signers_tickers_get_ok() -> Result<(), String> { + async fn test_signers_tickers_get_ok() { let mut mock_signer_getter = MockSignerGetter::new(); mock_signer_getter .expect_get_all() @@ -631,10 +638,11 @@ mod tests { &response, &StatusCode::OK, ) + .unwrap(); } #[tokio::test] - async fn test_signers_tickers_get_ko() -> Result<(), String> { + async fn test_signers_tickers_get_ko() { let mut mock_signer_getter = MockSignerGetter::new(); mock_signer_getter .expect_get_all() @@ -661,5 +669,6 @@ mod tests { &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 e3a8dd64f33..4e9b7411f38 100644 --- a/mithril-aggregator/src/http_server/routes/statistics_routes.rs +++ b/mithril-aggregator/src/http_server/routes/statistics_routes.rs @@ -80,7 +80,7 @@ mod tests { } #[tokio::test] - async fn post_statistics_ok() -> Result<(), String> { + async fn post_statistics_ok() { let config = Configuration::new_sample(); let mut builder = DependenciesBuilder::new(config); let mut rx = builder.get_event_transmitter_receiver().await.unwrap(); @@ -108,6 +108,6 @@ mod tests { ); let _ = rx.try_recv().unwrap(); - result + result.unwrap(); } } diff --git a/mithril-common/src/test_utils/apispec.rs b/mithril-common/src/test_utils/apispec.rs index 44529da46f2..dcf5b6ba1c4 100644 --- a/mithril-common/src/test_utils/apispec.rs +++ b/mithril-common/src/test_utils/apispec.rs @@ -307,27 +307,27 @@ mod tests { #[test] fn test_validate_a_response_without_body() { - assert!(APISpec::from_file(&APISpec::get_default_spec_file()) + APISpec::from_file(&APISpec::get_default_spec_file()) .method(Method::GET.as_str()) .path("/certificate-pending") .validate_request(&Null) .unwrap() .validate_response(&build_empty_response(204)) - .is_ok()); + .unwrap(); } #[test] fn test_validate_ok_when_request_without_body_and_expects_response() { - assert!(APISpec::from_file(&APISpec::get_default_spec_file()) + 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() + CertificatePendingMessage::dummy(), )) - .is_ok()); + .unwrap(); } #[test] @@ -350,11 +350,11 @@ mod tests { entities::InternalServerError::new("an error occurred".to_string()), ); - assert!(APISpec::from_file(&APISpec::get_default_spec_file()) + APISpec::from_file(&APISpec::get_default_spec_file()) .method(Method::POST.as_str()) .path("/register-signer") .validate_response(&response) - .is_ok()); + .unwrap(); } #[test] @@ -521,7 +521,7 @@ mod tests { } #[test] - fn test_validate_a_response_with_query_parameters() -> Result<(), String> { + 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}") @@ -529,6 +529,7 @@ mod tests { .unwrap() .validate_response(&build_empty_response(404)) .map(|_apispec| ()) + .unwrap(); } #[test] @@ -562,7 +563,7 @@ mod tests { } #[test] - fn test_validate_query_parameters_with_correct_parameter_name() -> Result<(), String> { + 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( @@ -570,6 +571,7 @@ mod tests { &api_spec.openapi["paths"]["/proof/cardano-transaction"]["get"], ) .map(|_apispec| ()) + .unwrap() } #[test] @@ -588,7 +590,7 @@ mod tests { } #[test] - fn test_verify_conformity_with_expected_status() -> Result<(), String> { + fn test_verify_conformity_with_expected_status() { APISpec::verify_conformity( APISpec::get_all_spec_files(), Method::GET.as_str(), @@ -598,6 +600,7 @@ mod tests { &build_json_response(200, CertificatePendingMessage::dummy()), &StatusCode::OK, ) + .unwrap() } #[test] From c001854fdf0ba7bad0d7c7340f1c7a03d96c08fb Mon Sep 17 00:00:00 2001 From: Damien LACHAUME / PALO-IT Date: Mon, 4 Mar 2024 11:10:45 +0100 Subject: [PATCH 26/29] Add `get_all_spec_files_from` in `apispec` --- mithril-common/src/test_utils/apispec.rs | 7 ++++++- .../mithril-aggregator-fake/src/application.rs | 4 +--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/mithril-common/src/test_utils/apispec.rs b/mithril-common/src/test_utils/apispec.rs index dcf5b6ba1c4..7982b9b28b5 100644 --- a/mithril-common/src/test_utils/apispec.rs +++ b/mithril-common/src/test_utils/apispec.rs @@ -244,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); diff --git a/mithril-test-lab/mithril-aggregator-fake/src/application.rs b/mithril-test-lab/mithril-aggregator-fake/src/application.rs index 4fc0cc7ed56..e445595cbe7 100644 --- a/mithril-test-lab/mithril-aggregator-fake/src/application.rs +++ b/mithril-test-lab/mithril-aggregator-fake/src/application.rs @@ -139,9 +139,7 @@ mod tests { } fn get_spec_files() -> Vec { - // APISpec::get_all_spec_files() - // TODO call get_all_spec_files with a root_path (../..) - vec!["../../openapi.yaml".to_string()] + APISpec::get_all_spec_files_from("../..") } #[tokio::test] From e040b8775da4ae3e43ddec2c89ef9ad73892c1ec Mon Sep 17 00:00:00 2001 From: Damien LACHAUME / PALO-IT Date: Mon, 4 Mar 2024 12:05:27 +0100 Subject: [PATCH 27/29] Fix `reqwest` missing dependency for `test_http_server` --- mithril-common/Cargo.toml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mithril-common/Cargo.toml b/mithril-common/Cargo.toml index cadcd708bbf..8c2d4a0771f 100644 --- a/mithril-common/Cargo.toml +++ b/mithril-common/Cargo.toml @@ -47,7 +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", features = ["json"], optional = true } +reqwest = { version = "0.11.23", optional = true } semver = "1.0.21" serde = { version = "1.0.196", features = ["derive"] } serde_bytes = "0.11.14" @@ -86,6 +86,7 @@ criterion = { version = "0.5.1", features = ["html_reports", "async_tokio"] } mockall = "0.12.1" pallas-crypto = "0.23.0" rand_core = { version = "0.6.4", features = ["getrandom"] } +reqwest = { version = "0.11.23", features = ["json"] } slog-async = "2.8.0" slog-scope = "4.4.0" slog-term = "2.9.0" From 53dbc235f796998d7d1d002ab6ffbb755d9a9928 Mon Sep 17 00:00:00 2001 From: Damien LACHAUME / PALO-IT Date: Mon, 4 Mar 2024 12:14:52 +0100 Subject: [PATCH 28/29] Adapt `NotFound` error to match with the real aggregator behavior --- .../mithril-aggregator-fake/src/error.rs | 6 ++--- .../mithril-aggregator-fake/src/handlers.rs | 22 +++++++++---------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/mithril-test-lab/mithril-aggregator-fake/src/error.rs b/mithril-test-lab/mithril-aggregator-fake/src/error.rs index c6a54e62785..fbe7f95eb5b 100644 --- a/mithril-test-lab/mithril-aggregator-fake/src/error.rs +++ b/mithril-test-lab/mithril-aggregator-fake/src/error.rs @@ -11,8 +11,8 @@ pub enum AppError { /// Catching anyhow errors Internal(anyhow::Error), - /// Resource not found (specify body) - NotFound(String), + /// Resource not found + NotFound, } /// Tell axum how to convert `AppError` into a response. @@ -24,7 +24,7 @@ impl IntoResponse for AppError { (StatusCode::INTERNAL_SERVER_ERROR, format!("Error: {:?}", e)).into_response() } - Self::NotFound(response_body) => (StatusCode::NOT_FOUND, response_body).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 487372c390a..3a62acb7cea 100644 --- a/mithril-test-lab/mithril-aggregator-fake/src/handlers.rs +++ b/mithril-test-lab/mithril-aggregator-fake/src/handlers.rs @@ -74,7 +74,7 @@ pub async fn snapshot( .map(|s| s.into_response()) .ok_or_else(|| { debug!("snapshot digest={key} NOT FOUND."); - AppError::NotFound("".to_string()) + AppError::NotFound }) } @@ -107,7 +107,7 @@ pub async fn msd( .map(|s| s.into_response()) .ok_or_else(|| { debug!("mithril stake distribution epoch={key} NOT FOUND."); - AppError::NotFound("".to_string()) + AppError::NotFound }) } @@ -132,7 +132,7 @@ pub async fn certificate( .map(|s| s.into_response()) .ok_or_else(|| { debug!("certificate hash={key} NOT FOUND."); - AppError::NotFound("".to_string()) + AppError::NotFound }) } @@ -157,7 +157,7 @@ pub async fn ctx_snapshot( .map(|s| s.into_response()) .ok_or_else(|| { debug!("ctx snapshot hash={key} NOT FOUND."); - AppError::NotFound("".to_string()) + AppError::NotFound }) } @@ -183,7 +183,7 @@ pub async fn ctx_proof( "ctx proof ctx_hash={} NOT FOUND.", params.transaction_hashes ); - AppError::NotFound("".to_string()) + AppError::NotFound }) } @@ -227,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] @@ -251,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] @@ -275,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] @@ -299,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] @@ -322,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] @@ -339,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] From e8dcecfd1584a6fd1ddeb48c44a127fea0a1528e Mon Sep 17 00:00:00 2001 From: Damien LACHAUME / PALO-IT Date: Mon, 4 Mar 2024 12:16:37 +0100 Subject: [PATCH 29/29] Update crates versions --- Cargo.lock | 8 ++++---- internal/mithril-persistence/Cargo.toml | 2 +- mithril-aggregator/Cargo.toml | 2 +- mithril-common/Cargo.toml | 2 +- mithril-test-lab/mithril-aggregator-fake/Cargo.toml | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) 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/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-common/Cargo.toml b/mithril-common/Cargo.toml index 8c2d4a0771f..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 } 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 }