From cc3d5041f59a5e9e3d1b644d629c8c3bb98bf715 Mon Sep 17 00:00:00 2001 From: Ruslan Fadeev Date: Tue, 24 Mar 2026 20:52:25 +0400 Subject: [PATCH] refactor: replace builder_opts with trait Builder --- src/aws/builder.rs | 29 ++++++++++++++++++++++++++++- src/azure/builder.rs | 29 ++++++++++++++++++++++++++++- src/builder.rs | 35 +++++++++++++++++++++++++++++++++++ src/gcp/builder.rs | 29 ++++++++++++++++++++++++++++- src/http/mod.rs | 25 +++++++++++++++++++++++++ src/lib.rs | 1 + src/parse.rs | 28 ++++++++-------------------- 7 files changed, 153 insertions(+), 23 deletions(-) create mode 100644 src/builder.rs diff --git a/src/aws/builder.rs b/src/aws/builder.rs index 1b9204c9..918722e5 100644 --- a/src/aws/builder.rs +++ b/src/aws/builder.rs @@ -24,9 +24,12 @@ use crate::aws::{ AmazonS3, AwsCredential, AwsCredentialProvider, Checksum, S3ConditionalPut, S3CopyIfNotExists, STORE, }; +use crate::builder::FromUrlBuilder; use crate::client::{HttpConnector, TokenCredentialProvider, http_connector}; use crate::config::ConfigValue; -use crate::{ClientConfigKey, ClientOptions, Result, RetryConfig, StaticCredentialProvider}; +use crate::{ + ClientConfigKey, ClientOptions, ObjectStore, Result, RetryConfig, StaticCredentialProvider, +}; use base64::Engine; use base64::prelude::BASE64_STANDARD; use itertools::Itertools; @@ -1257,6 +1260,30 @@ impl AmazonS3Builder { } } +impl FromUrlBuilder for AmazonS3Builder { + type ConfigKey = AmazonS3ConfigKey; + + fn builder_new() -> Self { + Self::new() + } + + fn builder_with_url(self, url: Url) -> Self { + self.with_url(url) + } + + fn builder_with_config(self, key: &str, value: String) -> Self { + if let Ok(key) = key.parse() { + self.with_config(key, value) + } else { + self + } + } + + fn builder_build(self) -> Result> { + Ok(Box::new(self.build()?) as _) + } +} + /// Extracts the AZ from a S3 Express One Zone bucket name /// /// diff --git a/src/azure/builder.rs b/src/azure/builder.rs index afd7a0ac..c8ea97a2 100644 --- a/src/azure/builder.rs +++ b/src/azure/builder.rs @@ -21,9 +21,12 @@ use crate::azure::credential::{ ImdsManagedIdentityProvider, WorkloadIdentityOAuthProvider, }; use crate::azure::{AzureCredential, AzureCredentialProvider, MicrosoftAzure, STORE}; +use crate::builder::FromUrlBuilder; use crate::client::{HttpConnector, TokenCredentialProvider, http_connector}; use crate::config::ConfigValue; -use crate::{ClientConfigKey, ClientOptions, Result, RetryConfig, StaticCredentialProvider}; +use crate::{ + ClientConfigKey, ClientOptions, ObjectStore, Result, RetryConfig, StaticCredentialProvider, +}; use percent_encoding::percent_decode_str; use serde::{Deserialize, Serialize}; use std::str::FromStr; @@ -1058,6 +1061,30 @@ impl MicrosoftAzureBuilder { } } +impl FromUrlBuilder for MicrosoftAzureBuilder { + type ConfigKey = AzureConfigKey; + + fn builder_new() -> Self { + Self::new() + } + + fn builder_with_url(self, url: Url) -> Self { + self.with_url(url) + } + + fn builder_with_config(self, key: &str, value: String) -> Self { + if let Ok(key) = key.parse() { + self.with_config(key, value) + } else { + self + } + } + + fn builder_build(self) -> Result> { + Ok(Box::new(self.build()?) as _) + } +} + /// Parses the contents of the environment variable `env_name` as a URL /// if present, otherwise falls back to default_url fn url_from_env(env_name: &str, default_url: &str) -> Result { diff --git a/src/builder.rs b/src/builder.rs new file mode 100644 index 00000000..bec22cfe --- /dev/null +++ b/src/builder.rs @@ -0,0 +1,35 @@ +use std::str::FromStr; + +use url::Url; + +use crate::ObjectStore; + +pub(crate) trait FromUrlBuilder { + type ConfigKey: FromStr; + + fn builder_new() -> Self; + + fn builder_with_url(self, url: Url) -> Self; + + fn builder_with_config(self, key: &str, value: String) -> Self; + + fn builder_build(self) -> crate::Result>; + + fn build_from_url_and_options( + url: &Url, + options: impl IntoIterator, + ) -> crate::Result> + where + Self: Sized, + K: AsRef, + V: Into, + { + let b = Self::builder_new().builder_with_url(url.clone()); + + let b = options.into_iter().fold(b, |builder, (key, value)| { + builder.builder_with_config(key.as_ref(), value.into()) + }); + + b.builder_build() + } +} diff --git a/src/gcp/builder.rs b/src/gcp/builder.rs index ece16ece..d77f668a 100644 --- a/src/gcp/builder.rs +++ b/src/gcp/builder.rs @@ -15,6 +15,7 @@ // specific language governing permissions and limitations // under the License. +use crate::builder::FromUrlBuilder; use crate::client::{HttpConnector, TokenCredentialProvider, http_connector}; use crate::config::ConfigValue; use crate::gcp::client::{GoogleCloudStorageClient, GoogleCloudStorageConfig}; @@ -26,7 +27,9 @@ use crate::gcp::{ GcpCredential, GcpCredentialProvider, GcpSigningCredential, GcpSigningCredentialProvider, GoogleCloudStorage, STORE, credential, }; -use crate::{ClientConfigKey, ClientOptions, Result, RetryConfig, StaticCredentialProvider}; +use crate::{ + ClientConfigKey, ClientOptions, ObjectStore, Result, RetryConfig, StaticCredentialProvider, +}; use serde::{Deserialize, Serialize}; use std::str::FromStr; use std::sync::Arc; @@ -638,6 +641,30 @@ impl GoogleCloudStorageBuilder { } } +impl FromUrlBuilder for GoogleCloudStorageBuilder { + type ConfigKey = GoogleConfigKey; + + fn builder_new() -> Self { + Self::new() + } + + fn builder_with_url(self, url: Url) -> Self { + self.with_url(url) + } + + fn builder_with_config(self, key: &str, value: String) -> Self { + if let Ok(key) = key.parse() { + self.with_config(key, value) + } else { + self + } + } + + fn builder_build(self) -> Result> { + Ok(Box::new(self.build()?) as _) + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/http/mod.rs b/src/http/mod.rs index 4d4081b8..3eb04749 100644 --- a/src/http/mod.rs +++ b/src/http/mod.rs @@ -39,6 +39,7 @@ use futures_util::{StreamExt, TryStreamExt}; use itertools::Itertools; use url::Url; +use crate::builder::FromUrlBuilder; use crate::client::get::GetClientExt; use crate::client::header::get_etag; use crate::client::{HttpConnector, http_connector}; @@ -290,6 +291,30 @@ impl HttpBuilder { } } +impl FromUrlBuilder for HttpBuilder { + type ConfigKey = ClientConfigKey; + + fn builder_new() -> Self { + Self::new() + } + + fn builder_with_url(self, url: Url) -> Self { + self.with_url(url) + } + + fn builder_with_config(self, key: &str, value: String) -> Self { + if let Ok(key) = key.parse() { + self.with_config(key, value) + } else { + self + } + } + + fn builder_build(self) -> Result> { + Ok(Box::new(self.build()?) as _) + } +} + #[cfg(test)] mod tests { use crate::integration::*; diff --git a/src/lib.rs b/src/lib.rs index d3ea9ee2..ba4fadd3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -563,6 +563,7 @@ pub mod signer; #[cfg(feature = "tokio")] pub mod throttle; +mod builder; #[cfg(feature = "cloud")] pub mod client; diff --git a/src/parse.rs b/src/parse.rs index b30fea70..7a8d1ac9 100644 --- a/src/parse.rs +++ b/src/parse.rs @@ -16,6 +16,7 @@ // under the License. use crate::ObjectStore; +use crate::builder::FromUrlBuilder; #[cfg(all(feature = "fs", not(target_arch = "wasm32")))] use crate::local::LocalFileSystem; use crate::memory::InMemory; @@ -139,20 +140,6 @@ impl ObjectStoreScheme { } } -#[cfg(feature = "cloud")] -macro_rules! builder_opts { - ($builder:ty, $url:expr, $options:expr) => {{ - let builder = $options.into_iter().fold( - <$builder>::new().with_url($url.to_string()), - |builder, (key, value)| match key.as_ref().to_ascii_lowercase().parse() { - Ok(k) => builder.with_config(k, value), - Err(_) => builder, - }, - ); - Box::new(builder.build()?) as _ - }}; -} - /// Create an [`ObjectStore`] based on the provided `url` /// /// Returns @@ -195,7 +182,6 @@ where { let _options = options; let (scheme, path) = ObjectStoreScheme::parse(url)?; - let path = Path::parse(path)?; let store = match scheme { #[cfg(all(feature = "fs", not(target_arch = "wasm32")))] @@ -203,20 +189,22 @@ where ObjectStoreScheme::Memory => Box::new(InMemory::new()) as _, #[cfg(feature = "aws")] ObjectStoreScheme::AmazonS3 => { - builder_opts!(crate::aws::AmazonS3Builder, url, _options) + crate::aws::AmazonS3Builder::build_from_url_and_options(url, _options)? } #[cfg(feature = "gcp")] ObjectStoreScheme::GoogleCloudStorage => { - builder_opts!(crate::gcp::GoogleCloudStorageBuilder, url, _options) + crate::gcp::GoogleCloudStorageBuilder::build_from_url_and_options(url, _options)? } #[cfg(feature = "azure")] ObjectStoreScheme::MicrosoftAzure => { - builder_opts!(crate::azure::MicrosoftAzureBuilder, url, _options) + crate::azure::MicrosoftAzureBuilder::build_from_url_and_options(url, _options)? } #[cfg(feature = "http")] ObjectStoreScheme::Http => { - let url = &url[..url::Position::BeforePath]; - builder_opts!(crate::http::HttpBuilder, url, _options) + let url: Url = url[..url::Position::BeforePath] + .parse() + .expect("URL with empty path and no query or fragment must still be a valid URL"); + crate::http::HttpBuilder::build_from_url_and_options(&url, _options)? } #[cfg(not(all( feature = "fs",