Replace AsyncConnectionConfig with Manager builder pattern

crates/deadpool-redis/src/config.rs

@@ -1,10 +1,13 @@
-use std::{fmt, path::PathBuf};
+use std::{fmt, path::PathBuf, time::Duration};
 
 use redis::RedisError;
 #[cfg(feature = "serde")]
 use serde::{Deserialize, Serialize};
 
-use crate::{CreatePoolError, Pool, PoolBuilder, PoolConfig, RedisResult, Runtime};
+use crate::{
+    CreatePoolError, DEFAULT_CONNECTION_TIMEOUT, DEFAULT_RESPONSE_TIMEOUT, ManagerConfig, Pool,
+    PoolBuilder, PoolConfig, RedisResult, Runtime,
+};
 
 /// Configuration object.
 ///
@@ -17,6 +20,10 @@ use crate::{CreatePoolError, Pool, PoolBuilder, PoolConfig, RedisResult, Runtime
 /// REDIS__POOL__MAX_SIZE=16
 /// REDIS__POOL__TIMEOUTS__WAIT__SECS=2
 /// REDIS__POOL__TIMEOUTS__WAIT__NANOS=0
+/// REDIS__CONNECTION_TIMEOUT__SECS=1
+/// REDIS__CONNECTION_TIMEOUT__NANOS=0
+/// REDIS__RESPONSE_TIMEOUT__SECS=0
+/// REDIS__RESPONSE_TIMEOUT__NANOS=500000000
 /// ```
 /// ```rust
 /// #[derive(serde::Deserialize)]
@@ -47,6 +54,18 @@ pub struct Config {
 
     /// Pool configuration.
     pub pool: Option<PoolConfig>,
+
+    /// Connection timeout applied when creating new Redis connections.
+    ///
+    /// Set to `None` to disable the timeout. Defaults to 1 second.
+    #[cfg_attr(feature = "serde", serde(default = "default_connection_timeout"))]
+    pub connection_timeout: Option<Duration>,
+
+    /// Response timeout applied when waiting for Redis responses.
+    ///
+    /// Set to `None` to disable the timeout. Defaults to 500 milliseconds.
+    #[cfg_attr(feature = "serde", serde(default = "default_response_timeout"))]
+    pub response_timeout: Option<Duration>,
 }
 
 impl Config {
@@ -70,15 +89,30 @@ impl Config {
     /// See [`ConfigError`] for details.
     pub fn builder(&self) -> Result<PoolBuilder, ConfigError> {
         let manager = match (&self.url, &self.connection) {
-            (Some(url), None) => crate::Manager::new(url.as_str())?,
-            (None, Some(connection)) => crate::Manager::new(connection.clone())?,
-            (None, None) => crate::Manager::new(ConnectionInfo::default())?,
+            (Some(url), None) => self.build_manager(url.as_str())?,
+            (None, Some(connection)) => self.build_manager(connection.clone())?,
+            (None, None) => self.build_manager(ConnectionInfo::default())?,
             (Some(_), Some(_)) => return Err(ConfigError::UrlAndConnectionSpecified),
         };
+
         let pool_config = self.get_pool_config();
+
         Ok(Pool::builder(manager).config(pool_config))
     }
 
+    fn build_manager<T: redis::IntoConnectionInfo>(
+        &self,
+        params: T,
+    ) -> Result<crate::Manager, ConfigError> {
+        Ok(crate::Manager::new_with_config(
+            params,
+            ManagerConfig {
+                connection_timeout: self.connection_timeout,
+                response_timeout: self.response_timeout,
+            },
+        )?)
+    }
+
     /// Returns [`deadpool::managed::PoolConfig`] which can be used to construct
     /// a [`deadpool::managed::Pool`] instance.
     #[must_use]
@@ -93,7 +127,7 @@ impl Config {
         Config {
             url: Some(url.into()),
             connection: None,
-            pool: None,
+            ..Default::default()
         }
     }
 
@@ -104,9 +138,29 @@ impl Config {
         Config {
             url: None,
             connection: Some(connection_info.into()),
-            pool: None,
+            ..Default::default()
         }
     }
+
+    /// Sets the connection timeout.
+    ///
+    /// Pass `Some(duration)` to set a specific timeout, or `None` to
+    /// disable it.
+    #[must_use]
+    pub fn with_connection_timeout(mut self, timeout: Option<Duration>) -> Self {
+        self.connection_timeout = timeout;
+        self
+    }
+
+    /// Sets the response timeout.
+    ///
+    /// Pass `Some(duration)` to set a specific timeout, or `None` to
+    /// disable it.
+    #[must_use]
+    pub fn with_response_timeout(mut self, timeout: Option<Duration>) -> Self {
+        self.response_timeout = timeout;
+        self
+    }
 }
 
 impl Default for Config {
@@ -115,10 +169,22 @@ impl Default for Config {
             url: None,
             connection: Some(ConnectionInfo::default()),
             pool: None,
+            connection_timeout: DEFAULT_CONNECTION_TIMEOUT,
+            response_timeout: DEFAULT_RESPONSE_TIMEOUT,
         }
     }
 }
 
+#[cfg(feature = "serde")]
+fn default_connection_timeout() -> Option<Duration> {
+    DEFAULT_CONNECTION_TIMEOUT
+}
+
+#[cfg(feature = "serde")]
+fn default_response_timeout() -> Option<Duration> {
+    DEFAULT_RESPONSE_TIMEOUT
+}
+
 /// This is a 1:1 copy of the [`redis::ConnectionAddr`] enumeration (excluding `tls_params` since it is entirely opaque to consumers).
 ///
 /// This is duplicated here in order to add support for the

crates/deadpool-redis/src/lib.rs

@@ -31,8 +31,12 @@ pub mod sentinel;
 use std::{
     ops::{Deref, DerefMut},
     sync::atomic::{AtomicUsize, Ordering},
+    time::Duration,
 };
 
+const DEFAULT_CONNECTION_TIMEOUT: Option<Duration> = Some(Duration::from_secs(1));
+const DEFAULT_RESPONSE_TIMEOUT: Option<Duration> = Some(Duration::from_millis(500));
+
 use deadpool::managed;
 use redis::{
     AsyncConnectionConfig, Client, IntoConnectionInfo, RedisError, RedisResult,
@@ -142,6 +146,45 @@ impl std::fmt::Debug for Manager {
     }
 }
 
+/// Connection configuration for the [`Manager`].
+///
+/// # Example
+///
+/// ```rust
+/// use std::time::Duration;
+/// use deadpool_redis::{ManagerConfig, Manager};
+///
+/// let manager = Manager::new_with_config(
+///     "redis://127.0.0.1",
+///     ManagerConfig {
+///         connection_timeout: Some(Duration::from_secs(5)),
+///         response_timeout: None,
+///     },
+/// )
+/// .unwrap();
+/// ```
+#[derive(Clone, Copy, Debug)]
+pub struct ManagerConfig {
+    /// Timeout for establishing a connection.
+    ///
+    /// Set to `None` to disable. Defaults to 1 second.
+    pub connection_timeout: Option<Duration>,
+
+    /// Timeout for waiting for a response.
+    ///
+    /// Set to `None` to disable. Defaults to 500 milliseconds.
+    pub response_timeout: Option<Duration>,
+}
+
+impl Default for ManagerConfig {
+    fn default() -> Self {
+        Self {
+            connection_timeout: DEFAULT_CONNECTION_TIMEOUT,
+            response_timeout: DEFAULT_RESPONSE_TIMEOUT,
+        }
+    }
+}
+
 impl Manager {
     /// Creates a new [`Manager`] from the given `params`.
     ///
@@ -157,18 +200,19 @@ impl Manager {
     }
 
     /// Creates a new [`Manager`] from the given `params` and
-    /// [`AsyncConnectionConfig`].
-    ///
-    /// This allows configuring connection-level settings such as response timeouts and connection
-    /// timeouts.
+    /// [`ManagerConfig`].
     ///
     /// # Errors
     ///
     /// If establishing a new [`Client`] fails.
     pub fn new_with_config<T: IntoConnectionInfo>(
         params: T,
-        connection_config: AsyncConnectionConfig,
+        config: ManagerConfig,
     ) -> RedisResult<Self> {
+        let connection_config = AsyncConnectionConfig::new()
+            .set_connection_timeout(config.connection_timeout)
+            .set_response_timeout(config.response_timeout);
+
         Ok(Self {
             client: Client::open(params)?,
             connection_config: Some(connection_config),

crates/deadpool-redis/tests/redis_connection_config.rs

@@ -2,8 +2,8 @@
 
 use std::time::Duration;
 
-use deadpool_redis::{Manager, Pool, Runtime};
-use redis::{AsyncCommands, AsyncConnectionConfig};
+use deadpool_redis::Runtime;
+use redis::AsyncCommands;
 use serde::{Deserialize, Serialize};
 
 #[derive(Debug, Default, Deserialize, Serialize)]
@@ -27,63 +27,44 @@ fn redis_url() -> String {
     Config::from_env().redis.url.unwrap_or_default()
 }
 
-fn create_pool_with_config(connection_config: AsyncConnectionConfig) -> Pool {
-    let manager = Manager::new_with_config(redis_url(), connection_config).unwrap();
-    Pool::builder(manager)
-        .max_size(1)
-        .runtime(Runtime::Tokio1)
-        .build()
-        .unwrap()
-}
-
-fn create_pool_default() -> Pool {
-    let manager = Manager::new(redis_url()).unwrap();
-    Pool::builder(manager)
-        .max_size(1)
-        .runtime(Runtime::Tokio1)
-        .build()
-        .unwrap()
-}
-
-/// Verifies that `new_with_config` with `set_response_timeout(None)` allows commands that take
-/// longer than the default 500ms timeout.
-///
-/// Uses `BLPOP` on an empty list with a 1-second timeout. With the default `AsyncConnectionConfig`
-/// (500ms response timeout), this would fail. With `set_response_timeout(None)`, it waits the full
-/// second and returns nil.
+/// Verifies that disabling the response timeout allows commands that take longer than the
+/// default timeout. Uses `BLPOP` on an empty list with a 1-second server-side timeout.
 #[tokio::test]
-async fn test_response_timeout_can_be_disabled() {
-    let config = AsyncConnectionConfig::new().set_response_timeout(None);
-    let pool = create_pool_with_config(config);
+async fn test_response_timeout_disabled() {
+    let pool = deadpool_redis::Config::from_url(redis_url())
+        .with_response_timeout(None)
+        .create_pool(Some(Runtime::Tokio1))
+        .unwrap();
+
     let mut conn = pool.get().await.unwrap();
 
     let result: Option<(String, String)> = conn
-        .blpop("deadpool/nonexistent_timeout_test_key", 1.0)
+        .blpop("deadpool/test_timeout_disabled", 1.0)
         .await
         .unwrap();
+
     assert_eq!(result, None);
 }
 
-/// Verifies that the default `Manager::new` (without config) uses the redis crate's default
-/// timeouts, which causes blocking commands exceeding 500ms to fail.
+/// Verifies that setting an explicit response timeout causes commands exceeding it to fail.
 #[tokio::test]
-async fn test_default_manager_times_out_on_slow_commands() {
-    let pool = create_pool_default();
+async fn test_response_timeout_causes_timeout() {
+    let pool = deadpool_redis::Config::from_url(redis_url())
+        .with_response_timeout(Some(Duration::from_millis(100)))
+        .create_pool(Some(Runtime::Tokio1))
+        .unwrap();
+
     let mut conn = pool.get().await.unwrap();
 
     let start = std::time::Instant::now();
-    let result: Result<Option<(String, String)>, _> = conn
-        .blpop("deadpool/nonexistent_default_timeout_key", 1.0)
-        .await;
+    let result: Result<Option<(String, String)>, _> =
+        conn.blpop("deadpool/test_timeout_short", 1.0).await;
     let elapsed = start.elapsed();
 
+    assert!(result.is_err(), "expected timeout error");
     assert!(
-        result.is_err(),
-        "expected timeout error with default config"
-    );
-    assert!(
-        elapsed < Duration::from_millis(900),
-        "should have timed out before the 1s BLPOP completed, took {:?}",
+        elapsed < Duration::from_millis(500),
+        "should have timed out well before the 1s BLPOP, took {:?}",
         elapsed
     );
 }