diff --git a/core/core/src/raw/time.rs b/core/core/src/raw/time.rs index f1a3e3c4dc6d..4ff95f4463a8 100644 --- a/core/core/src/raw/time.rs +++ b/core/core/src/raw/time.rs @@ -18,11 +18,12 @@ //! Time related utils. use crate::*; -use jiff::SignedDuration; + use std::fmt; use std::ops::{Add, AddAssign, Sub, SubAssign}; use std::str::FromStr; +pub use jiff::SignedDuration; pub use std::time::{Duration, UNIX_EPOCH}; #[cfg(not(all(target_arch = "wasm32", target_os = "unknown")))] pub use std::time::{Instant, SystemTime}; @@ -254,7 +255,6 @@ impl SubAssign for Timestamp { } } -/// Convert an unsigned [`Duration`] into a jiff [`SignedDuration`]. /// Parse a duration encoded either as ISO-8601 (e.g. `PT5M`) or friendly (e.g. `5m`). #[inline] pub fn signed_to_duration(value: &str) -> Result { @@ -262,7 +262,13 @@ pub fn signed_to_duration(value: &str) -> Result { Error::new(ErrorKind::ConfigInvalid, "failed to parse duration").set_source(err) })?; - Duration::try_from(signed).map_err(|err| { + signed_duration_to_duration(signed) +} + +/// Convert a jiff [`SignedDuration`] into an unsigned [`Duration`]. +#[inline] +pub fn signed_duration_to_duration(value: SignedDuration) -> Result { + Duration::try_from(value).map_err(|err| { Error::new( ErrorKind::ConfigInvalid, "duration must not be negative or overflow", @@ -304,4 +310,9 @@ mod tests { let v = Timestamp::parse_rfc2822(s).unwrap(); assert_eq!("Sat, 29 Oct 1994 19:43:31 GMT", v.format_http_date()); } + + #[test] + fn test_signed_duration_to_duration_rejects_negative_values() { + assert!(signed_duration_to_duration(SignedDuration::from_secs(-1)).is_err()); + } } diff --git a/core/services/cloudflare-kv/src/backend.rs b/core/services/cloudflare-kv/src/backend.rs index a900fb6036fc..bd536df6ab39 100644 --- a/core/services/cloudflare-kv/src/backend.rs +++ b/core/services/cloudflare-kv/src/backend.rs @@ -37,6 +37,7 @@ use super::writer::CloudflareWriter; #[derive(Default)] pub struct CloudflareKvBuilder { pub(super) config: CloudflareKvConfig, + pub(super) default_ttl: Option, } impl Debug for CloudflareKvBuilder { @@ -76,7 +77,7 @@ impl CloudflareKvBuilder { /// /// If set, we will specify `EX` for write operations. pub fn default_ttl(mut self, ttl: Duration) -> Self { - self.config.default_ttl = Some(ttl); + self.default_ttl = Some(ttl); self } @@ -96,6 +97,14 @@ impl Builder for CloudflareKvBuilder { type Config = CloudflareKvConfig; fn build(self) -> Result { + let default_ttl = match self.default_ttl { + Some(ttl) => Some(ttl), + None => self + .config + .default_ttl + .map(signed_duration_to_duration) + .transpose()?, + }; let api_token = match &self.config.api_token { Some(api_token) => format_authorization_by_bearer(api_token)?, None => { @@ -121,7 +130,7 @@ impl Builder for CloudflareKvBuilder { }; // Validate default TTL is at least 60 seconds if specified - if let Some(ttl) = self.config.default_ttl + if let Some(ttl) = default_ttl && ttl < Duration::from_secs(60) { return Err(Error::new( @@ -143,7 +152,7 @@ impl Builder for CloudflareKvBuilder { api_token, account_id, namespace_id, - expiration_ttl: self.config.default_ttl, + expiration_ttl: default_ttl, info: ServiceInfo::new(CLOUDFLARE_KV_SCHEME, &root, ""), capability: Capability { create_dir: true, diff --git a/core/services/cloudflare-kv/src/config.rs b/core/services/cloudflare-kv/src/config.rs index cad44d7ea3e3..7f5f2bae8f3d 100644 --- a/core/services/cloudflare-kv/src/config.rs +++ b/core/services/cloudflare-kv/src/config.rs @@ -34,7 +34,7 @@ pub struct CloudflareKvConfig { /// The namespace ID. Used as URI path parameter. pub namespace_id: Option, /// The default ttl for write operations. - pub default_ttl: Option, + pub default_ttl: Option, /// Root within this backend. pub root: Option, @@ -94,7 +94,10 @@ impl opendal_core::Configurator for CloudflareKvConfig { } fn into_builder(self) -> Self::Builder { - CloudflareKvBuilder { config: self } + CloudflareKvBuilder { + config: self, + default_ttl: None, + } } } @@ -125,4 +128,12 @@ mod tests { assert!(CloudflareKvConfig::from_uri(&uri).is_err()); } + + #[test] + fn from_iter_parses_default_ttl() { + let cfg = CloudflareKvConfig::from_iter([("default_ttl".to_string(), "PT1M".to_string())]) + .unwrap(); + + assert_eq!(cfg.default_ttl, Some(SignedDuration::from_mins(1))); + } } diff --git a/core/services/memcached/src/backend.rs b/core/services/memcached/src/backend.rs index 7d6e4a2d85b7..2a259892fe9b 100644 --- a/core/services/memcached/src/backend.rs +++ b/core/services/memcached/src/backend.rs @@ -34,6 +34,7 @@ use super::writer::MemcachedWriter; #[derive(Debug, Default)] pub struct MemcachedBuilder { pub(super) config: MemcachedConfig, + pub(super) default_ttl: Option, } impl MemcachedBuilder { @@ -74,7 +75,7 @@ impl MemcachedBuilder { /// Set the default ttl for memcached services. pub fn default_ttl(mut self, ttl: Duration) -> Self { - self.config.default_ttl = Some(ttl); + self.default_ttl = Some(ttl); self } @@ -97,6 +98,14 @@ impl Builder for MemcachedBuilder { type Config = MemcachedConfig; fn build(self) -> Result { + let default_ttl = match self.default_ttl { + Some(ttl) => Some(ttl), + None => self + .config + .default_ttl + .map(signed_duration_to_duration) + .transpose()?, + }; let endpoint_raw = self.config.endpoint.clone().ok_or_else(|| { Error::new(ErrorKind::ConfigInvalid, "endpoint is empty") .with_context("service", MEMCACHED_SCHEME) @@ -171,7 +180,7 @@ impl Builder for MemcachedBuilder { endpoint, self.config.username, self.config.password, - self.config.default_ttl, + default_ttl, self.config.connection_pool_max_size, )) .with_normalized_root(root)) diff --git a/core/services/memcached/src/config.rs b/core/services/memcached/src/config.rs index df8ab16b0c45..4cc2ad2b81b9 100644 --- a/core/services/memcached/src/config.rs +++ b/core/services/memcached/src/config.rs @@ -44,7 +44,7 @@ pub struct MemcachedConfig { /// Memcached password, optional. pub password: Option, /// The default ttl for put operations. - pub default_ttl: Option, + pub default_ttl: Option, /// The maximum number of connections allowed. /// /// default is 10 @@ -81,7 +81,10 @@ impl Configurator for MemcachedConfig { } fn into_builder(self) -> Self::Builder { - MemcachedBuilder { config: self } + MemcachedBuilder { + config: self, + default_ttl: None, + } } } @@ -101,4 +104,12 @@ mod tests { assert_eq!(cfg.root.as_deref(), Some("app/session")); Ok(()) } + + #[test] + fn from_iter_parses_default_ttl() -> Result<()> { + let cfg = MemcachedConfig::from_iter([("default_ttl".to_string(), "1500ms".to_string())])?; + + assert_eq!(cfg.default_ttl, Some(SignedDuration::from_millis(1500))); + Ok(()) + } } diff --git a/core/services/redis/src/backend.rs b/core/services/redis/src/backend.rs index 67aa3896d37c..a86665159ef5 100644 --- a/core/services/redis/src/backend.rs +++ b/core/services/redis/src/backend.rs @@ -44,6 +44,7 @@ const DEFAULT_REDIS_PORT: u16 = 6379; #[derive(Debug, Default)] pub struct RedisBuilder { pub(super) config: RedisConfig, + pub(super) default_ttl: Option, } impl RedisBuilder { @@ -108,7 +109,7 @@ impl RedisBuilder { /// /// If set, we will specify `EX` for write operations. pub fn default_ttl(mut self, ttl: Duration) -> Self { - self.config.default_ttl = Some(ttl); + self.default_ttl = Some(ttl); self } @@ -144,6 +145,14 @@ impl Builder for RedisBuilder { type Config = RedisConfig; fn build(self) -> Result { + let default_ttl = match self.default_ttl { + Some(ttl) => Some(ttl), + None => self + .config + .default_ttl + .map(signed_duration_to_duration) + .transpose()?, + }; let root = normalize_root( self.config .root @@ -170,7 +179,7 @@ impl Builder for RedisBuilder { endpoints, None, Some(client), - self.config.default_ttl, + default_ttl, self.config.connection_pool_max_size, )) .with_normalized_root(root)) @@ -194,7 +203,7 @@ impl Builder for RedisBuilder { endpoint, Some(client), None, - self.config.default_ttl, + default_ttl, self.config.connection_pool_max_size, )) .with_normalized_root(root)) diff --git a/core/services/redis/src/config.rs b/core/services/redis/src/config.rs index e617be4990a2..4f488dac8e9a 100644 --- a/core/services/redis/src/config.rs +++ b/core/services/redis/src/config.rs @@ -59,7 +59,7 @@ pub struct RedisConfig { /// default is db 0 pub db: i64, /// The default ttl for put operations. - pub default_ttl: Option, + pub default_ttl: Option, } impl Debug for RedisConfig { @@ -120,7 +120,10 @@ impl Configurator for RedisConfig { } fn into_builder(self) -> Self::Builder { - RedisBuilder { config: self } + RedisBuilder { + config: self, + default_ttl: None, + } } } @@ -156,6 +159,14 @@ mod tests { Ok(()) } + #[test] + fn from_iter_parses_default_ttl() -> Result<()> { + let cfg = RedisConfig::from_iter([("default_ttl".to_string(), "5s".to_string())])?; + + assert_eq!(cfg.default_ttl, Some(SignedDuration::from_secs(5))); + Ok(()) + } + #[test] fn test_redis_builder_interface() { // Test that RedisBuilder still works with the new implementation diff --git a/dev/src/generate/parser.rs b/dev/src/generate/parser.rs index f46eaf697bd3..df200daaa084 100644 --- a/dev/src/generate/parser.rs +++ b/dev/src/generate/parser.rs @@ -83,7 +83,7 @@ pub enum ConfigType { Bool, /// Mapping to rust's `String` String, - /// Mapping to rust's `Duration` + /// Mapping to rust's `Duration` and `SignedDuration` Duration, /// Mapping to rust's `usize` @@ -113,7 +113,7 @@ impl FromStr for ConfigType { Ok(match s { "bool" => ConfigType::Bool, "String" => ConfigType::String, - "Duration" => ConfigType::Duration, + "Duration" | "SignedDuration" => ConfigType::Duration, "usize" => ConfigType::Usize, "u64" => ConfigType::U64, @@ -514,6 +514,19 @@ mod tests { example: None, }, ), + ( + "default_ttl: Option", + Config { + name: "default_ttl".to_string(), + value: ConfigType::Duration, + optional: true, + deprecated: None, + comments: "".to_string(), + group: None, + default_value: None, + example: None, + }, + ), ]; for (input, expected) in cases { diff --git a/dev/src/generate/python.rs b/dev/src/generate/python.rs index b866349ffc5d..963048f2d0e8 100644 --- a/dev/src/generate/python.rs +++ b/dev/src/generate/python.rs @@ -264,8 +264,7 @@ fn is_path_like_field(name: &str) -> bool { fn make_config_field_type(field: ViaDeserialize) -> Result { Ok(match field.value { ConfigType::Bool => "bool".to_string(), - // Duration is typed `str` (a humantime string, e.g. "5s"). See #7887: - // core cannot yet deserialize Duration config fields from strings. + // Duration values cross the Python binding as strings, e.g. "5s". ConfigType::Duration => "str".to_string(), ConfigType::Usize | ConfigType::U64