refactor(aggregator-client): rename client and error types to include Http

This makes clear that this client is Http focused only and can't be
extended as is to support other protocols.

Author: Alenar

internal/mithril-aggregator-client/src/builder.rs

@@ -8,9 +8,9 @@ use std::time::Duration;
 use mithril_common::StdResult;
 use mithril_common::api_version::APIVersionProvider;
 
-use crate::client::AggregatorClient;
+use crate::client::AggregatorHttpClient;
 
-/// A builder of [AggregatorClient]
+/// A builder of [AggregatorHttpClient]
 pub struct AggregatorClientBuilder {
     aggregator_url_result: reqwest::Result<Url>,
     api_version_provider: Option<Arc<APIVersionProvider>>,
@@ -68,8 +68,8 @@ impl AggregatorClientBuilder {
         self
     }
 
-    /// Returns an [AggregatorClient] based on the builder configuration
-    pub fn build(self) -> StdResult<AggregatorClient> {
+    /// Returns an [AggregatorHttpClient] based on the builder configuration
+    pub fn build(self) -> StdResult<AggregatorHttpClient> {
         let aggregator_endpoint =
             enforce_trailing_slash(self.aggregator_url_result.with_context(
                 || "Invalid aggregator endpoint, it must be a correctly formed url",
@@ -84,7 +84,7 @@ impl AggregatorClientBuilder {
                 .proxy(Proxy::all(relay_endpoint).with_context(|| "Relay proxy creation failed")?)
         }
 
-        Ok(AggregatorClient {
+        Ok(AggregatorHttpClient {
             aggregator_endpoint,
             api_version_provider,
             additional_headers: (&additional_headers)

internal/mithril-aggregator-client/src/client.rs

@@ -8,15 +8,15 @@ use std::time::Duration;
 use mithril_common::MITHRIL_API_VERSION_HEADER;
 use mithril_common::api_version::APIVersionProvider;
 
-use crate::AggregatorClientResult;
+use crate::AggregatorHttpClientResult;
 use crate::builder::AggregatorClientBuilder;
-use crate::error::AggregatorClientError;
+use crate::error::AggregatorHttpClientError;
 use crate::query::{AggregatorQuery, QueryContext, QueryMethod};
 
 const API_VERSION_MISMATCH_WARNING_MESSAGE: &str = "OpenAPI version may be incompatible, please update Mithril client library to the latest version.";
 
 /// A client to send HTTP requests to a Mithril Aggregator
-pub struct AggregatorClient {
+pub struct AggregatorHttpClient {
     pub(super) aggregator_endpoint: Url,
     pub(super) api_version_provider: Arc<APIVersionProvider>,
     pub(super) additional_headers: HeaderMap,
@@ -25,7 +25,7 @@ pub struct AggregatorClient {
     pub(super) logger: Logger,
 }
 
-impl AggregatorClient {
+impl AggregatorHttpClient {
     /// Creates a [AggregatorClientBuilder] to configure a `AggregatorClient`.
     //
     // This is the same as `AggregatorClient::builder()`.
@@ -34,7 +34,10 @@ impl AggregatorClient {
     }
 
     /// Send the given query to the Mithril Aggregator
-    pub async fn send<Q: AggregatorQuery>(&self, query: Q) -> AggregatorClientResult<Q::Response> {
+    pub async fn send<Q: AggregatorQuery>(
+        &self,
+        query: Q,
+    ) -> AggregatorHttpClientResult<Q::Response> {
         // Todo: error handling ? Reuse the version in `warn_if_api_version_mismatch` ?
         let current_api_version = self.api_version_provider.compute_current_version().unwrap();
         let mut request_builder = match Q::method() {
@@ -62,11 +65,13 @@ impl AggregatorClient {
                 };
                 query.handle_response(context).await
             }
-            Err(err) => Err(AggregatorClientError::RemoteServerUnreachable(anyhow!(err))),
+            Err(err) => Err(AggregatorHttpClientError::RemoteServerUnreachable(anyhow!(
+                err
+            ))),
         }
     }
 
-    fn join_aggregator_endpoint(&self, endpoint: &str) -> AggregatorClientResult<Url> {
+    fn join_aggregator_endpoint(&self, endpoint: &str) -> AggregatorHttpClientResult<Url> {
         self.aggregator_endpoint
             .join(endpoint)
             .with_context(|| {
@@ -75,7 +80,7 @@ impl AggregatorClient {
                     self.aggregator_endpoint
                 )
             })
-            .map_err(AggregatorClientError::InvalidEndpoint)
+            .map_err(AggregatorHttpClientError::InvalidEndpoint)
     }
 
     /// Check API version mismatch and log a warning if the aggregator's version is more recent.
@@ -141,13 +146,13 @@ mod tests {
         async fn handle_response(
             &self,
             context: QueryContext,
-        ) -> AggregatorClientResult<Self::Response> {
+        ) -> AggregatorHttpClientResult<Self::Response> {
             match context.response.status() {
                 StatusCode::OK => context
                     .response
                     .json::<TestResponse>()
                     .await
-                    .map_err(|err| AggregatorClientError::JsonParseFailed(anyhow!(err))),
+                    .map_err(|err| AggregatorHttpClientError::JsonParseFailed(anyhow!(err))),
                 _ => Err(context.unhandled_status_code().await),
             }
         }
@@ -192,7 +197,7 @@ mod tests {
         async fn handle_response(
             &self,
             context: QueryContext,
-        ) -> AggregatorClientResult<Self::Response> {
+        ) -> AggregatorHttpClientResult<Self::Response> {
             match context.response.status() {
                 StatusCode::CREATED => Ok(()),
                 _ => Err(context.unhandled_status_code().await),
@@ -293,7 +298,7 @@ mod tests {
         let error = client.send(TestGetQuery).await.expect_err("should not fail");
 
         assert!(
-            matches!(error, AggregatorClientError::RemoteServerUnreachable(_)),
+            matches!(error, AggregatorHttpClientError::RemoteServerUnreachable(_)),
             "unexpected error type: {error:?}"
         );
     }
@@ -337,7 +342,7 @@ mod tests {
             let aggregator_version = "2.0.0";
             let client_version = "1.0.0";
             let (logger, log_inspector) = TestLogger::memory();
-            let client = AggregatorClient::builder("http://whatever")
+            let client = AggregatorHttpClient::builder("http://whatever")
                 .with_logger(logger)
                 .with_api_version_provider(Arc::new(APIVersionProvider::new_with_default_version(
                     Version::parse(client_version).unwrap(),
@@ -361,7 +366,7 @@ mod tests {
         fn test_no_warning_logged_when_versions_match() {
             let version = "1.0.0";
             let (logger, log_inspector) = TestLogger::memory();
-            let client = AggregatorClient::builder("http://whatever")
+            let client = AggregatorHttpClient::builder("http://whatever")
                 .with_logger(logger)
                 .with_api_version_provider(Arc::new(APIVersionProvider::new_with_default_version(
                     Version::parse(version).unwrap(),
@@ -380,7 +385,7 @@ mod tests {
             let aggregator_version = "1.0.0";
             let client_version = "2.0.0";
             let (logger, log_inspector) = TestLogger::memory();
-            let client = AggregatorClient::builder("http://whatever")
+            let client = AggregatorHttpClient::builder("http://whatever")
                 .with_logger(logger)
                 .with_api_version_provider(Arc::new(APIVersionProvider::new_with_default_version(
                     Version::parse(client_version).unwrap(),
@@ -403,7 +408,7 @@ mod tests {
         #[test]
         fn test_does_not_log_or_fail_when_header_is_missing() {
             let (logger, log_inspector) = TestLogger::memory();
-            let client = AggregatorClient::builder("http://whatever")
+            let client = AggregatorHttpClient::builder("http://whatever")
                 .with_logger(logger)
                 .with_api_version_provider(Arc::new(APIVersionProvider::default()))
                 .build()
@@ -419,7 +424,7 @@ mod tests {
         #[test]
         fn test_does_not_log_or_fail_when_header_is_not_a_version() {
             let (logger, log_inspector) = TestLogger::memory();
-            let client = AggregatorClient::builder("http://whatever")
+            let client = AggregatorHttpClient::builder("http://whatever")
                 .with_logger(logger)
                 .with_api_version_provider(Arc::new(APIVersionProvider::default()))
                 .build()
@@ -435,7 +440,7 @@ mod tests {
         #[test]
         fn test_logs_error_when_client_version_cannot_be_computed() {
             let (logger, log_inspector) = TestLogger::memory();
-            let client = AggregatorClient::builder("http://whatever")
+            let client = AggregatorHttpClient::builder("http://whatever")
                 .with_logger(logger)
                 .with_api_version_provider(Arc::new(APIVersionProvider::new_failing()))
                 .build()

internal/mithril-aggregator-client/src/error.rs

@@ -9,7 +9,7 @@ use crate::JSON_CONTENT_TYPE;
 
 /// Error structure for the Aggregator Client.
 #[derive(Error, Debug)]
-pub enum AggregatorClientError {
+pub enum AggregatorHttpClientError {
     /// Error raised when querying the aggregator returned a 5XX error.
     #[error("Internal error of the Aggregator")]
     RemoteServerTechnical(#[source] StdError),
@@ -39,7 +39,7 @@ pub enum AggregatorClientError {
     RegistrationRoundNotYetOpened(#[source] StdError),
 }
 
-impl AggregatorClientError {
+impl AggregatorHttpClientError {
     /// Create an `AggregatorClientError` from a response.
     ///
     /// This method is meant to be used after handling domain-specific cases leaving only
@@ -133,12 +133,12 @@ mod tests {
     #[tokio::test]
     async fn test_4xx_errors_are_handled_as_remote_server_logical() {
         let response = build_text_response(StatusCode::BAD_REQUEST, "error text");
-        let handled_error = AggregatorClientError::from_response(response).await;
+        let handled_error = AggregatorHttpClientError::from_response(response).await;
 
         assert!(
             matches!(
                 handled_error,
-                AggregatorClientError::RemoteServerLogical(..)
+                AggregatorHttpClientError::RemoteServerLogical(..)
             ),
             "Expected error to be RemoteServerLogical\ngot '{handled_error:?}'",
         );
@@ -147,12 +147,12 @@ mod tests {
     #[tokio::test]
     async fn test_5xx_errors_are_handled_as_remote_server_technical() {
         let response = build_text_response(StatusCode::INTERNAL_SERVER_ERROR, "error text");
-        let handled_error = AggregatorClientError::from_response(response).await;
+        let handled_error = AggregatorHttpClientError::from_response(response).await;
 
         assert!(
             matches!(
                 handled_error,
-                AggregatorClientError::RemoteServerTechnical(..)
+                AggregatorHttpClientError::RemoteServerTechnical(..)
             ),
             "Expected error to be RemoteServerLogical\ngot '{handled_error:?}'",
         );
@@ -161,12 +161,12 @@ mod tests {
     #[tokio::test]
     async fn test_550_error_is_handled_as_registration_round_not_yet_opened() {
         let response = build_text_response(StatusCode::from_u16(550).unwrap(), "Not yet available");
-        let handled_error = AggregatorClientError::from_response(response).await;
+        let handled_error = AggregatorHttpClientError::from_response(response).await;
 
         assert!(
             matches!(
                 handled_error,
-                AggregatorClientError::RegistrationRoundNotYetOpened(..)
+                AggregatorHttpClientError::RegistrationRoundNotYetOpened(..)
             ),
             "Expected error to be RegistrationRoundNotYetOpened\ngot '{handled_error:?}'",
         );
@@ -176,12 +176,12 @@ mod tests {
     async fn test_non_4xx_or_5xx_errors_are_handled_as_unhandled_status_code_and_contains_response_text()
      {
         let response = build_text_response(StatusCode::OK, "ok text");
-        let handled_error = AggregatorClientError::from_response(response).await;
+        let handled_error = AggregatorHttpClientError::from_response(response).await;
 
         assert!(
             matches!(
                 handled_error,
-                AggregatorClientError::UnhandledStatusCode(..) if format!("{handled_error:?}").contains("ok text")
+                AggregatorHttpClientError::UnhandledStatusCode(..) if format!("{handled_error:?}").contains("ok text")
             ),
             "Expected error to be UnhandledStatusCode with 'ok text' in error text\ngot '{handled_error:?}'",
         );
@@ -193,7 +193,7 @@ mod tests {
         let response = build_text_response(StatusCode::EXPECTATION_FAILED, error_text);
 
         assert_error_text_contains!(
-            AggregatorClientError::get_root_cause(response).await,
+            AggregatorHttpClientError::get_root_cause(response).await,
             "expectation failed: An error occurred; please try again later."
         );
     }
@@ -205,7 +205,7 @@ mod tests {
         let response = build_json_response(StatusCode::BAD_REQUEST, &client_error);
 
         assert_error_text_contains!(
-            AggregatorClientError::get_root_cause(response).await,
+            AggregatorHttpClientError::get_root_cause(response).await,
             "bad request: label: message"
         );
     }
@@ -217,7 +217,7 @@ mod tests {
         let response = build_json_response(StatusCode::BAD_REQUEST, &server_error);
 
         assert_error_text_contains!(
-            AggregatorClientError::get_root_cause(response).await,
+            AggregatorHttpClientError::get_root_cause(response).await,
             "bad request: message"
         );
     }
@@ -230,7 +230,7 @@ mod tests {
         );
 
         assert_error_text_contains!(
-            AggregatorClientError::get_root_cause(response).await,
+            AggregatorHttpClientError::get_root_cause(response).await,
             r#"internal server error: {"first":"foreign","second":"unknown"}"#
         );
     }
@@ -244,7 +244,7 @@ mod tests {
             .unwrap()
             .into();
 
-        let root_cause = AggregatorClientError::get_root_cause(response).await;
+        let root_cause = AggregatorHttpClientError::get_root_cause(response).await;
 
         assert_error_text_contains!(root_cause, "bad request");
         assert!(

internal/mithril-aggregator-client/src/lib.rs

@@ -10,11 +10,11 @@ pub mod query;
 mod test;
 
 pub use builder::AggregatorClientBuilder;
-pub use client::AggregatorClient;
-pub use error::AggregatorClientError;
+pub use client::AggregatorHttpClient;
+pub use error::AggregatorHttpClientError;
 
 pub(crate) const JSON_CONTENT_TYPE: reqwest::header::HeaderValue =
     reqwest::header::HeaderValue::from_static("application/json");
 
 /// Aggregator-client result type
-pub type AggregatorClientResult<T> = Result<T, error::AggregatorClientError>;
+pub type AggregatorHttpClientResult<T> = Result<T, AggregatorHttpClientError>;

internal/mithril-aggregator-client/src/query/api.rs

@@ -2,8 +2,8 @@ use reqwest::Response;
 use serde::de::DeserializeOwned;
 use slog::Logger;
 
-use crate::AggregatorClientResult;
-use crate::error::AggregatorClientError;
+use crate::AggregatorHttpClientResult;
+use crate::error::AggregatorHttpClientError;
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum QueryMethod {
@@ -28,7 +28,7 @@ pub trait AggregatorQuery {
     async fn handle_response(
         &self,
         context: QueryContext,
-    ) -> AggregatorClientResult<Self::Response>;
+    ) -> AggregatorHttpClientResult<Self::Response>;
 }
 
 pub struct QueryContext {
@@ -37,7 +37,7 @@ pub struct QueryContext {
 }
 
 impl QueryContext {
-    pub async fn unhandled_status_code(self) -> AggregatorClientError {
-        AggregatorClientError::from_response(self.response).await
+    pub async fn unhandled_status_code(self) -> AggregatorHttpClientError {
+        AggregatorHttpClientError::from_response(self.response).await
     }
 }

internal/mithril-aggregator-client/src/query/get/get_certificate.rs

@@ -5,8 +5,8 @@ use slog::debug;
 
 use mithril_common::messages::CertificateMessage;
 
-use crate::AggregatorClientResult;
-use crate::error::AggregatorClientError;
+use crate::AggregatorHttpClientResult;
+use crate::error::AggregatorHttpClientError;
 use crate::query::{AggregatorQuery, QueryContext, QueryMethod};
 
 /// Query to get the details of a certificate
@@ -45,13 +45,13 @@ impl AggregatorQuery for GetCertificateQuery {
     async fn handle_response(
         &self,
         context: QueryContext,
-    ) -> AggregatorClientResult<Self::Response> {
+    ) -> AggregatorHttpClientResult<Self::Response> {
         debug!(context.logger, "Retrieve certificate details"; "certificate_hash" => %self.hash);
 
         match context.response.status() {
             StatusCode::OK => match context.response.json::<CertificateMessage>().await {
                 Ok(message) => Ok(Some(message)),
-                Err(err) => Err(AggregatorClientError::JsonParseFailed(anyhow!(err))),
+                Err(err) => Err(AggregatorHttpClientError::JsonParseFailed(anyhow!(err))),
             },
             StatusCode::NOT_FOUND => Ok(None),
             _ => Err(context.unhandled_status_code().await),
@@ -112,7 +112,7 @@ mod tests {
             .await
             .unwrap_err();
 
-        assert_error_matches!(error, AggregatorClientError::RemoteServerTechnical(_));
+        assert_error_matches!(error, AggregatorHttpClientError::RemoteServerTechnical(_));
     }
 
     #[tokio::test]
@@ -152,6 +152,6 @@ mod tests {
 
         let error = client.send(GetCertificateQuery::latest_genesis()).await.unwrap_err();
 
-        assert_error_matches!(error, AggregatorClientError::RemoteServerTechnical(_));
+        assert_error_matches!(error, AggregatorHttpClientError::RemoteServerTechnical(_));
     }
 }