From bc83fdcba8db9d929c76bccc52aa1cd8d75bc907 Mon Sep 17 00:00:00 2001 From: everpcpc Date: Tue, 15 Oct 2024 17:28:08 +0800 Subject: [PATCH 01/10] feat(core): abstract HttpFetch trait for raw http client --- core/src/raw/http_util/client.rs | 55 ++++++++++++++++++++++++-------- core/src/services/s3/backend.rs | 9 ++++-- core/src/services/s3/core.rs | 4 ++- 3 files changed, 50 insertions(+), 18 deletions(-) diff --git a/core/src/raw/http_util/client.rs b/core/src/raw/http_util/client.rs index 7d756030da0b..f7df25123aa6 100644 --- a/core/src/raw/http_util/client.rs +++ b/core/src/raw/http_util/client.rs @@ -20,6 +20,7 @@ use std::fmt::Formatter; use std::future; use std::mem; use std::str::FromStr; +use std::sync::Arc; use futures::TryStreamExt; use http::Request; @@ -34,7 +35,7 @@ use crate::*; /// HttpClient that used across opendal. #[derive(Clone)] pub struct HttpClient { - client: reqwest::Client, + fetcher: Arc, } /// We don't want users to know details about our clients. @@ -47,26 +48,24 @@ impl Debug for HttpClient { impl HttpClient { /// Create a new http client in async context. pub fn new() -> Result { - Self::build(reqwest::ClientBuilder::new()) + let fetcher = Arc::new(ReqwestHttpFetcher::new()); + Ok(Self { fetcher }) } /// Construct `Self` with given [`reqwest::Client`] - pub fn with(client: reqwest::Client) -> Self { - Self { client } + pub fn with(client: impl HttpFetch + 'static) -> Self { + let fetcher = Arc::new(client); + Self { fetcher } } /// Build a new http client in async context. + #[deprecated] pub fn build(builder: reqwest::ClientBuilder) -> Result { - Ok(Self { - client: builder.build().map_err(|err| { - Error::new(ErrorKind::Unexpected, "http client build failed").set_source(err) - })?, - }) - } - - /// Get the async client from http client. - pub fn client(&self) -> reqwest::Client { - self.client.clone() + let client = builder.build().map_err(|err| { + Error::new(ErrorKind::Unexpected, "http client build failed").set_source(err) + })?; + let fetcher = Arc::new(ReqwestHttpFetcher::with(client)); + Ok(Self { fetcher }) } /// Send a request in async way. @@ -78,6 +77,34 @@ impl HttpClient { /// Fetch a request in async way. pub async fn fetch(&self, req: Request) -> Result> { + self.fetcher.fetch(req).await + } +} + +#[async_trait::async_trait] +pub trait HttpFetch: Send + Sync { + /// Fetch a request in async way. + async fn fetch(&self, req: Request) -> Result>; +} + +#[derive(Clone)] +struct ReqwestHttpFetcher { + client: reqwest::Client, +} + +impl ReqwestHttpFetcher { + pub fn new() -> Self { + Self::with(reqwest::Client::new()) + } + + pub fn with(client: reqwest::Client) -> Self { + Self { client } + } +} + +#[async_trait::async_trait] +impl HttpFetch for ReqwestHttpFetcher { + async fn fetch(&self, req: Request) -> Result> { // Uri stores all string alike data in `Bytes` which means // the clone here is cheap. let uri = req.uri().clone(); diff --git a/core/src/services/s3/backend.rs b/core/src/services/s3/backend.rs index c441f8ac453a..83acea25758a 100644 --- a/core/src/services/s3/backend.rs +++ b/core/src/services/s3/backend.rs @@ -791,6 +791,8 @@ impl Builder for S3Builder { })? }; + let default_http_client = reqwest::Client::new(); + let mut loader: Option> = None; // If customized_credential_load is set, we will use it. if let Some(v) = self.customized_credential_load { @@ -800,7 +802,7 @@ impl Builder for S3Builder { // If role_arn is set, we must use AssumeRoleLoad. if let Some(role_arn) = self.config.role_arn { // use current env as source credential loader. - let default_loader = AwsDefaultLoader::new(client.client(), cfg.clone()); + let default_loader = AwsDefaultLoader::new(default_http_client.clone(), cfg.clone()); // Build the config for assume role. let mut assume_role_cfg = AwsConfig { @@ -817,7 +819,7 @@ impl Builder for S3Builder { } let assume_role_loader = AwsAssumeRoleLoader::new( - client.client(), + default_http_client.clone(), assume_role_cfg, Box::new(default_loader), ) @@ -835,7 +837,7 @@ impl Builder for S3Builder { let loader = match loader { Some(v) => v, None => { - let mut default_loader = AwsDefaultLoader::new(client.client(), cfg); + let mut default_loader = AwsDefaultLoader::new(default_http_client.clone(), cfg); if self.config.disable_ec2_metadata { default_loader = default_loader.with_disable_ec2_metadata(); } @@ -871,6 +873,7 @@ impl Builder for S3Builder { client, batch_max_operations, checksum_algorithm, + default_http_client, }), }) } diff --git a/core/src/services/s3/core.rs b/core/src/services/s3/core.rs index 66bcb14e2418..9717174db9be 100644 --- a/core/src/services/s3/core.rs +++ b/core/src/services/s3/core.rs @@ -97,6 +97,8 @@ pub struct S3Core { pub client: HttpClient, pub batch_max_operations: usize, pub checksum_algorithm: Option, + + pub(crate) default_http_client: reqwest::Client, } impl Debug for S3Core { @@ -114,7 +116,7 @@ impl S3Core { async fn load_credential(&self) -> Result> { let cred = self .loader - .load_credential(self.client.client()) + .load_credential(self.default_http_client.clone()) .await .map_err(new_request_credential_error)?; From efac5a3992828276a3abc4b1c77c601bef615098 Mon Sep 17 00:00:00 2001 From: everpcpc Date: Tue, 15 Oct 2024 17:58:16 +0800 Subject: [PATCH 02/10] z --- core/src/raw/http_util/client.rs | 4 ++++ core/src/raw/http_util/mod.rs | 1 + core/src/services/cos/backend.rs | 2 +- core/src/services/gcs/backend.rs | 2 +- core/src/services/oss/backend.rs | 2 +- core/src/services/s3/backend.rs | 11 +++++------ core/src/services/s3/core.rs | 4 +--- 7 files changed, 14 insertions(+), 12 deletions(-) diff --git a/core/src/raw/http_util/client.rs b/core/src/raw/http_util/client.rs index f7df25123aa6..3418a0ccfd14 100644 --- a/core/src/raw/http_util/client.rs +++ b/core/src/raw/http_util/client.rs @@ -25,6 +25,7 @@ use std::sync::Arc; use futures::TryStreamExt; use http::Request; use http::Response; +use once_cell::sync::Lazy; use raw::oio::Read; use super::parse_content_encoding; @@ -32,6 +33,9 @@ use super::parse_content_length; use super::HttpBody; use crate::*; +/// Http client used across opendal for loading credentials. +pub static CREDENTIAL_CLIENT: Lazy = Lazy::new(|| reqwest::Client::new()); + /// HttpClient that used across opendal. #[derive(Clone)] pub struct HttpClient { diff --git a/core/src/raw/http_util/mod.rs b/core/src/raw/http_util/mod.rs index 965ea0bd494f..b7ef1930930c 100644 --- a/core/src/raw/http_util/mod.rs +++ b/core/src/raw/http_util/mod.rs @@ -24,6 +24,7 @@ mod client; pub use client::HttpClient; +pub use client::CREDENTIAL_CLIENT; mod body; pub use body::HttpBody; diff --git a/core/src/services/cos/backend.rs b/core/src/services/cos/backend.rs index ff4db12ee420..9f8a41e326a4 100644 --- a/core/src/services/cos/backend.rs +++ b/core/src/services/cos/backend.rs @@ -205,7 +205,7 @@ impl Builder for CosBuilder { cfg.secret_key = Some(v); } - let cred_loader = TencentCosCredentialLoader::new(client.client(), cfg); + let cred_loader = TencentCosCredentialLoader::new(CREDENTIAL_CLIENT.clone(), cfg); let signer = TencentCosSigner::new(); diff --git a/core/src/services/gcs/backend.rs b/core/src/services/gcs/backend.rs index 74d837c4c85e..d6f55ff1a565 100644 --- a/core/src/services/gcs/backend.rs +++ b/core/src/services/gcs/backend.rs @@ -292,7 +292,7 @@ impl Builder for GcsBuilder { DEFAULT_GCS_SCOPE }; - let mut token_loader = GoogleTokenLoader::new(scope, client.client()); + let mut token_loader = GoogleTokenLoader::new(scope, CREDENTIAL_CLIENT.clone()); if let Some(account) = &self.config.service_account { token_loader = token_loader.with_service_account(account); } diff --git a/core/src/services/oss/backend.rs b/core/src/services/oss/backend.rs index 6b064d36821d..bb11520844c5 100644 --- a/core/src/services/oss/backend.rs +++ b/core/src/services/oss/backend.rs @@ -381,7 +381,7 @@ impl Builder for OssBuilder { })? }; - let loader = AliyunLoader::new(client.client(), cfg); + let loader = AliyunLoader::new(CREDENTIAL_CLIENT.clone(), cfg); let signer = AliyunOssSigner::new(bucket); diff --git a/core/src/services/s3/backend.rs b/core/src/services/s3/backend.rs index 83acea25758a..f3bec7a8dd0b 100644 --- a/core/src/services/s3/backend.rs +++ b/core/src/services/s3/backend.rs @@ -791,8 +791,6 @@ impl Builder for S3Builder { })? }; - let default_http_client = reqwest::Client::new(); - let mut loader: Option> = None; // If customized_credential_load is set, we will use it. if let Some(v) = self.customized_credential_load { @@ -802,7 +800,8 @@ impl Builder for S3Builder { // If role_arn is set, we must use AssumeRoleLoad. if let Some(role_arn) = self.config.role_arn { // use current env as source credential loader. - let default_loader = AwsDefaultLoader::new(default_http_client.clone(), cfg.clone()); + let default_loader = + AwsDefaultLoader::new(CREDENTIAL_CLIENT.clone().clone(), cfg.clone()); // Build the config for assume role. let mut assume_role_cfg = AwsConfig { @@ -819,7 +818,7 @@ impl Builder for S3Builder { } let assume_role_loader = AwsAssumeRoleLoader::new( - default_http_client.clone(), + CREDENTIAL_CLIENT.clone().clone(), assume_role_cfg, Box::new(default_loader), ) @@ -837,7 +836,8 @@ impl Builder for S3Builder { let loader = match loader { Some(v) => v, None => { - let mut default_loader = AwsDefaultLoader::new(default_http_client.clone(), cfg); + let mut default_loader = + AwsDefaultLoader::new(CREDENTIAL_CLIENT.clone().clone(), cfg); if self.config.disable_ec2_metadata { default_loader = default_loader.with_disable_ec2_metadata(); } @@ -873,7 +873,6 @@ impl Builder for S3Builder { client, batch_max_operations, checksum_algorithm, - default_http_client, }), }) } diff --git a/core/src/services/s3/core.rs b/core/src/services/s3/core.rs index 9717174db9be..bd3c3cec3f1a 100644 --- a/core/src/services/s3/core.rs +++ b/core/src/services/s3/core.rs @@ -97,8 +97,6 @@ pub struct S3Core { pub client: HttpClient, pub batch_max_operations: usize, pub checksum_algorithm: Option, - - pub(crate) default_http_client: reqwest::Client, } impl Debug for S3Core { @@ -116,7 +114,7 @@ impl S3Core { async fn load_credential(&self) -> Result> { let cred = self .loader - .load_credential(self.default_http_client.clone()) + .load_credential(CREDENTIAL_CLIENT.clone()) .await .map_err(new_request_credential_error)?; From b9f886f4647f3d4f04aa7749cce641ee26cfc360 Mon Sep 17 00:00:00 2001 From: everpcpc Date: Tue, 15 Oct 2024 18:03:34 +0800 Subject: [PATCH 03/10] z --- core/src/raw/http_util/client.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/raw/http_util/client.rs b/core/src/raw/http_util/client.rs index 3418a0ccfd14..1060c13e9110 100644 --- a/core/src/raw/http_util/client.rs +++ b/core/src/raw/http_util/client.rs @@ -34,7 +34,7 @@ use super::HttpBody; use crate::*; /// Http client used across opendal for loading credentials. -pub static CREDENTIAL_CLIENT: Lazy = Lazy::new(|| reqwest::Client::new()); +pub static CREDENTIAL_CLIENT: Lazy = Lazy::new(reqwest::Client::new); /// HttpClient that used across opendal. #[derive(Clone)] From 5c6d065f4cca5d7960c62316f0067244a3b06cc3 Mon Sep 17 00:00:00 2001 From: everpcpc Date: Wed, 16 Oct 2024 14:00:05 +0800 Subject: [PATCH 04/10] z --- core/src/raw/http_util/client.rs | 8 +++++--- core/src/raw/http_util/mod.rs | 2 +- core/src/services/cos/backend.rs | 2 +- core/src/services/gcs/backend.rs | 2 +- core/src/services/oss/backend.rs | 2 +- core/src/services/s3/backend.rs | 6 +++--- core/src/services/s3/core.rs | 2 +- 7 files changed, 13 insertions(+), 11 deletions(-) diff --git a/core/src/raw/http_util/client.rs b/core/src/raw/http_util/client.rs index 1060c13e9110..04bec3167360 100644 --- a/core/src/raw/http_util/client.rs +++ b/core/src/raw/http_util/client.rs @@ -34,7 +34,9 @@ use super::HttpBody; use crate::*; /// Http client used across opendal for loading credentials. -pub static CREDENTIAL_CLIENT: Lazy = Lazy::new(reqwest::Client::new); +/// This is merely a temporary solution because reqsign requires a reqwest client to be passed. +/// We will remove it after the next major version of reqsign, which will enable users to provide their own client. +pub static GLOBAL_REQWEST_CLIENT: Lazy = Lazy::new(reqwest::Client::new); /// HttpClient that used across opendal. #[derive(Clone)] @@ -57,7 +59,7 @@ impl HttpClient { } /// Construct `Self` with given [`reqwest::Client`] - pub fn with(client: impl HttpFetch + 'static) -> Self { + pub fn with(client: impl HttpFetch) -> Self { let fetcher = Arc::new(client); Self { fetcher } } @@ -86,7 +88,7 @@ impl HttpClient { } #[async_trait::async_trait] -pub trait HttpFetch: Send + Sync { +pub trait HttpFetch: Send + Sync + Unpin + 'static { /// Fetch a request in async way. async fn fetch(&self, req: Request) -> Result>; } diff --git a/core/src/raw/http_util/mod.rs b/core/src/raw/http_util/mod.rs index b7ef1930930c..e7ca1f550b94 100644 --- a/core/src/raw/http_util/mod.rs +++ b/core/src/raw/http_util/mod.rs @@ -24,7 +24,7 @@ mod client; pub use client::HttpClient; -pub use client::CREDENTIAL_CLIENT; +pub(crate) use client::GLOBAL_REQWEST_CLIENT; mod body; pub use body::HttpBody; diff --git a/core/src/services/cos/backend.rs b/core/src/services/cos/backend.rs index 9f8a41e326a4..6065dca95e5d 100644 --- a/core/src/services/cos/backend.rs +++ b/core/src/services/cos/backend.rs @@ -205,7 +205,7 @@ impl Builder for CosBuilder { cfg.secret_key = Some(v); } - let cred_loader = TencentCosCredentialLoader::new(CREDENTIAL_CLIENT.clone(), cfg); + let cred_loader = TencentCosCredentialLoader::new(GLOBAL_REQWEST_CLIENT.clone(), cfg); let signer = TencentCosSigner::new(); diff --git a/core/src/services/gcs/backend.rs b/core/src/services/gcs/backend.rs index d6f55ff1a565..4c7bc7eca69a 100644 --- a/core/src/services/gcs/backend.rs +++ b/core/src/services/gcs/backend.rs @@ -292,7 +292,7 @@ impl Builder for GcsBuilder { DEFAULT_GCS_SCOPE }; - let mut token_loader = GoogleTokenLoader::new(scope, CREDENTIAL_CLIENT.clone()); + let mut token_loader = GoogleTokenLoader::new(scope, GLOBAL_REQWEST_CLIENT.clone()); if let Some(account) = &self.config.service_account { token_loader = token_loader.with_service_account(account); } diff --git a/core/src/services/oss/backend.rs b/core/src/services/oss/backend.rs index bb11520844c5..7e2d67b3caf5 100644 --- a/core/src/services/oss/backend.rs +++ b/core/src/services/oss/backend.rs @@ -381,7 +381,7 @@ impl Builder for OssBuilder { })? }; - let loader = AliyunLoader::new(CREDENTIAL_CLIENT.clone(), cfg); + let loader = AliyunLoader::new(GLOBAL_REQWEST_CLIENT.clone(), cfg); let signer = AliyunOssSigner::new(bucket); diff --git a/core/src/services/s3/backend.rs b/core/src/services/s3/backend.rs index f3bec7a8dd0b..57b3a2f2fa46 100644 --- a/core/src/services/s3/backend.rs +++ b/core/src/services/s3/backend.rs @@ -801,7 +801,7 @@ impl Builder for S3Builder { if let Some(role_arn) = self.config.role_arn { // use current env as source credential loader. let default_loader = - AwsDefaultLoader::new(CREDENTIAL_CLIENT.clone().clone(), cfg.clone()); + AwsDefaultLoader::new(GLOBAL_REQWEST_CLIENT.clone().clone(), cfg.clone()); // Build the config for assume role. let mut assume_role_cfg = AwsConfig { @@ -818,7 +818,7 @@ impl Builder for S3Builder { } let assume_role_loader = AwsAssumeRoleLoader::new( - CREDENTIAL_CLIENT.clone().clone(), + GLOBAL_REQWEST_CLIENT.clone().clone(), assume_role_cfg, Box::new(default_loader), ) @@ -837,7 +837,7 @@ impl Builder for S3Builder { Some(v) => v, None => { let mut default_loader = - AwsDefaultLoader::new(CREDENTIAL_CLIENT.clone().clone(), cfg); + AwsDefaultLoader::new(GLOBAL_REQWEST_CLIENT.clone().clone(), cfg); if self.config.disable_ec2_metadata { default_loader = default_loader.with_disable_ec2_metadata(); } diff --git a/core/src/services/s3/core.rs b/core/src/services/s3/core.rs index bd3c3cec3f1a..79ba66239235 100644 --- a/core/src/services/s3/core.rs +++ b/core/src/services/s3/core.rs @@ -114,7 +114,7 @@ impl S3Core { async fn load_credential(&self) -> Result> { let cred = self .loader - .load_credential(CREDENTIAL_CLIENT.clone()) + .load_credential(GLOBAL_REQWEST_CLIENT.clone()) .await .map_err(new_request_credential_error)?; From bc21b1d074e32bde17b2a1077cfc5a96d0ee0075 Mon Sep 17 00:00:00 2001 From: everpcpc Date: Wed, 16 Oct 2024 14:01:07 +0800 Subject: [PATCH 05/10] z --- core/src/raw/http_util/client.rs | 22 +++------------------- 1 file changed, 3 insertions(+), 19 deletions(-) diff --git a/core/src/raw/http_util/client.rs b/core/src/raw/http_util/client.rs index 04bec3167360..17ad76f991c4 100644 --- a/core/src/raw/http_util/client.rs +++ b/core/src/raw/http_util/client.rs @@ -54,7 +54,7 @@ impl Debug for HttpClient { impl HttpClient { /// Create a new http client in async context. pub fn new() -> Result { - let fetcher = Arc::new(ReqwestHttpFetcher::new()); + let fetcher = Arc::new(reqwest::Client::new()); Ok(Self { fetcher }) } @@ -70,7 +70,7 @@ impl HttpClient { let client = builder.build().map_err(|err| { Error::new(ErrorKind::Unexpected, "http client build failed").set_source(err) })?; - let fetcher = Arc::new(ReqwestHttpFetcher::with(client)); + let fetcher = Arc::new(client); Ok(Self { fetcher }) } @@ -93,23 +93,8 @@ pub trait HttpFetch: Send + Sync + Unpin + 'static { async fn fetch(&self, req: Request) -> Result>; } -#[derive(Clone)] -struct ReqwestHttpFetcher { - client: reqwest::Client, -} - -impl ReqwestHttpFetcher { - pub fn new() -> Self { - Self::with(reqwest::Client::new()) - } - - pub fn with(client: reqwest::Client) -> Self { - Self { client } - } -} - #[async_trait::async_trait] -impl HttpFetch for ReqwestHttpFetcher { +impl HttpFetch for reqwest::Client { async fn fetch(&self, req: Request) -> Result> { // Uri stores all string alike data in `Bytes` which means // the clone here is cheap. @@ -119,7 +104,6 @@ impl HttpFetch for ReqwestHttpFetcher { let (parts, body) = req.into_parts(); let mut req_builder = self - .client .request( parts.method, reqwest::Url::from_str(&uri.to_string()).expect("input request url must be valid"), From 5c2d3c9454bc18c65852a20a99869251c30ba107 Mon Sep 17 00:00:00 2001 From: everpcpc Date: Wed, 16 Oct 2024 15:35:58 +0800 Subject: [PATCH 06/10] z --- core/src/raw/http_util/client.rs | 42 ++++++++++++++++++++++++++------ 1 file changed, 35 insertions(+), 7 deletions(-) diff --git a/core/src/raw/http_util/client.rs b/core/src/raw/http_util/client.rs index 17ad76f991c4..12d3d35f1c28 100644 --- a/core/src/raw/http_util/client.rs +++ b/core/src/raw/http_util/client.rs @@ -19,9 +19,11 @@ use std::fmt::Debug; use std::fmt::Formatter; use std::future; use std::mem; +use std::ops::Deref; use std::str::FromStr; use std::sync::Arc; +use futures::Future; use futures::TryStreamExt; use http::Request; use http::Response; @@ -31,6 +33,7 @@ use raw::oio::Read; use super::parse_content_encoding; use super::parse_content_length; use super::HttpBody; +use crate::raw::*; use crate::*; /// Http client used across opendal for loading credentials. @@ -38,10 +41,13 @@ use crate::*; /// We will remove it after the next major version of reqsign, which will enable users to provide their own client. pub static GLOBAL_REQWEST_CLIENT: Lazy = Lazy::new(reqwest::Client::new); +/// HttpFetcher is a type erased [`HttpFetch`]. +pub type HttpFetcher = Box; + /// HttpClient that used across opendal. #[derive(Clone)] pub struct HttpClient { - fetcher: Arc, + fetcher: Arc, } /// We don't want users to know details about our clients. @@ -54,13 +60,13 @@ impl Debug for HttpClient { impl HttpClient { /// Create a new http client in async context. pub fn new() -> Result { - let fetcher = Arc::new(reqwest::Client::new()); + let fetcher = Arc::new(Box::new(reqwest::Client::new()) as HttpFetcher); Ok(Self { fetcher }) } /// Construct `Self` with given [`reqwest::Client`] pub fn with(client: impl HttpFetch) -> Self { - let fetcher = Arc::new(client); + let fetcher = Arc::new(Box::new(client) as HttpFetcher); Self { fetcher } } @@ -70,7 +76,7 @@ impl HttpClient { let client = builder.build().map_err(|err| { Error::new(ErrorKind::Unexpected, "http client build failed").set_source(err) })?; - let fetcher = Arc::new(client); + let fetcher = Arc::new(Box::new(client) as HttpFetcher); Ok(Self { fetcher }) } @@ -87,13 +93,35 @@ impl HttpClient { } } -#[async_trait::async_trait] pub trait HttpFetch: Send + Sync + Unpin + 'static { /// Fetch a request in async way. - async fn fetch(&self, req: Request) -> Result>; + fn fetch( + &self, + req: Request, + ) -> impl Future>> + MaybeSend; +} + +/// HttpFetchDyn is the dyn version of [`HttpFetch`] +/// which make it possible to use as `Box` +pub trait HttpFetchDyn: Send + Sync + Unpin + 'static { + /// The dyn version of [`HttpFetch::fetch`]. + /// + /// This function returns a boxed future to make it object safe. + fn fetch_dyn(&self, req: Request) -> BoxedFuture>>; +} + +impl HttpFetchDyn for T { + fn fetch_dyn(&self, req: Request) -> BoxedFuture>> { + Box::pin(self.fetch(req)) + } +} + +impl HttpFetch for Box { + async fn fetch(&self, req: Request) -> Result> { + self.deref().fetch_dyn(req).await + } } -#[async_trait::async_trait] impl HttpFetch for reqwest::Client { async fn fetch(&self, req: Request) -> Result> { // Uri stores all string alike data in `Bytes` which means From 371fd0f3d407744f6f3f7406cbe30e85cbf67499 Mon Sep 17 00:00:00 2001 From: everpcpc Date: Wed, 16 Oct 2024 15:52:27 +0800 Subject: [PATCH 07/10] z --- core/src/raw/http_util/mod.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/src/raw/http_util/mod.rs b/core/src/raw/http_util/mod.rs index e7ca1f550b94..b95b70d9f915 100644 --- a/core/src/raw/http_util/mod.rs +++ b/core/src/raw/http_util/mod.rs @@ -24,6 +24,9 @@ mod client; pub use client::HttpClient; + +/// temporary client used by several features +#[allow(unused_imports)] pub(crate) use client::GLOBAL_REQWEST_CLIENT; mod body; From bb925626f7f0c4deee3f38ec79639d38f85237f3 Mon Sep 17 00:00:00 2001 From: everpcpc Date: Wed, 16 Oct 2024 15:57:15 +0800 Subject: [PATCH 08/10] z --- core/src/raw/http_util/client.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/src/raw/http_util/client.rs b/core/src/raw/http_util/client.rs index 12d3d35f1c28..8b970ad336ea 100644 --- a/core/src/raw/http_util/client.rs +++ b/core/src/raw/http_util/client.rs @@ -39,7 +39,8 @@ use crate::*; /// Http client used across opendal for loading credentials. /// This is merely a temporary solution because reqsign requires a reqwest client to be passed. /// We will remove it after the next major version of reqsign, which will enable users to provide their own client. -pub static GLOBAL_REQWEST_CLIENT: Lazy = Lazy::new(reqwest::Client::new); +#[allow(dead_code)] +pub(crate) static GLOBAL_REQWEST_CLIENT: Lazy = Lazy::new(reqwest::Client::new); /// HttpFetcher is a type erased [`HttpFetch`]. pub type HttpFetcher = Box; From bcaa642a2ce8535196efd6444e366af513652a57 Mon Sep 17 00:00:00 2001 From: everpcpc Date: Wed, 16 Oct 2024 16:39:12 +0800 Subject: [PATCH 09/10] z --- core/src/raw/http_util/client.rs | 17 ++++++++++------- core/src/raw/http_util/mod.rs | 1 + 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/core/src/raw/http_util/client.rs b/core/src/raw/http_util/client.rs index 8b970ad336ea..e4e74de92411 100644 --- a/core/src/raw/http_util/client.rs +++ b/core/src/raw/http_util/client.rs @@ -43,12 +43,12 @@ use crate::*; pub(crate) static GLOBAL_REQWEST_CLIENT: Lazy = Lazy::new(reqwest::Client::new); /// HttpFetcher is a type erased [`HttpFetch`]. -pub type HttpFetcher = Box; +pub type HttpFetcher = Arc; /// HttpClient that used across opendal. #[derive(Clone)] pub struct HttpClient { - fetcher: Arc, + fetcher: HttpFetcher, } /// We don't want users to know details about our clients. @@ -61,13 +61,13 @@ impl Debug for HttpClient { impl HttpClient { /// Create a new http client in async context. pub fn new() -> Result { - let fetcher = Arc::new(Box::new(reqwest::Client::new()) as HttpFetcher); + let fetcher = Arc::new(reqwest::Client::new()); Ok(Self { fetcher }) } /// Construct `Self` with given [`reqwest::Client`] pub fn with(client: impl HttpFetch) -> Self { - let fetcher = Arc::new(Box::new(client) as HttpFetcher); + let fetcher = Arc::new(client); Self { fetcher } } @@ -77,7 +77,7 @@ impl HttpClient { let client = builder.build().map_err(|err| { Error::new(ErrorKind::Unexpected, "http client build failed").set_source(err) })?; - let fetcher = Arc::new(Box::new(client) as HttpFetcher); + let fetcher = Arc::new(client); Ok(Self { fetcher }) } @@ -94,6 +94,8 @@ impl HttpClient { } } +/// HttpFetch is the trait to fetch a request in async way. +/// User should implement this trait to provide their own http client. pub trait HttpFetch: Send + Sync + Unpin + 'static { /// Fetch a request in async way. fn fetch( @@ -103,7 +105,8 @@ pub trait HttpFetch: Send + Sync + Unpin + 'static { } /// HttpFetchDyn is the dyn version of [`HttpFetch`] -/// which make it possible to use as `Box` +/// which make it possible to use as `Box`. +/// User should never implement this trait, but use `HttpFetch` instead. pub trait HttpFetchDyn: Send + Sync + Unpin + 'static { /// The dyn version of [`HttpFetch::fetch`]. /// @@ -117,7 +120,7 @@ impl HttpFetchDyn for T { } } -impl HttpFetch for Box { +impl HttpFetch for Arc { async fn fetch(&self, req: Request) -> Result> { self.deref().fetch_dyn(req).await } diff --git a/core/src/raw/http_util/mod.rs b/core/src/raw/http_util/mod.rs index b95b70d9f915..226fb17b7d47 100644 --- a/core/src/raw/http_util/mod.rs +++ b/core/src/raw/http_util/mod.rs @@ -24,6 +24,7 @@ mod client; pub use client::HttpClient; +pub use client::HttpFetch; /// temporary client used by several features #[allow(unused_imports)] From 86e2d628d64854c8d5b4c12d6e6e84d82a2598da Mon Sep 17 00:00:00 2001 From: everpcpc Date: Wed, 16 Oct 2024 16:39:42 +0800 Subject: [PATCH 10/10] z --- core/src/raw/http_util/client.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/raw/http_util/client.rs b/core/src/raw/http_util/client.rs index e4e74de92411..b9eedc2bcefd 100644 --- a/core/src/raw/http_util/client.rs +++ b/core/src/raw/http_util/client.rs @@ -105,7 +105,7 @@ pub trait HttpFetch: Send + Sync + Unpin + 'static { } /// HttpFetchDyn is the dyn version of [`HttpFetch`] -/// which make it possible to use as `Box`. +/// which make it possible to use as `Arc`. /// User should never implement this trait, but use `HttpFetch` instead. pub trait HttpFetchDyn: Send + Sync + Unpin + 'static { /// The dyn version of [`HttpFetch::fetch`].