From 28b17801a143a736215954198450b304a95ec602 Mon Sep 17 00:00:00 2001 From: Utkarsh Gupta Date: Wed, 31 Aug 2022 03:40:11 +0530 Subject: [PATCH 1/4] cluster_client: move code - move build method to builder - move open methods to the bottom --- redis/src/cluster_client.rs | 134 ++++++++++++++++++------------------ 1 file changed, 67 insertions(+), 67 deletions(-) diff --git a/redis/src/cluster_client.rs b/redis/src/cluster_client.rs index a6962f205..069b26531 100644 --- a/redis/src/cluster_client.rs +++ b/redis/src/cluster_client.rs @@ -4,7 +4,7 @@ use super::{ ConnectionAddr, ConnectionInfo, ErrorKind, IntoConnectionInfo, RedisError, RedisResult, }; -/// Used to configure and build a [ClusterClient](ClusterClient). +/// Used to configure and build a [`ClusterClient`]. pub struct ClusterClientBuilder { initial_nodes: RedisResult>, read_from_replicas: bool, @@ -26,16 +26,49 @@ impl ClusterClientBuilder { } } - /// Builds a [ClusterClient](ClusterClient). Despite the name, this does not actually open - /// a connection to Redis Cluster, but will perform some basic checks of the initial - /// nodes' URLs and passwords. - /// - /// # Errors - /// - /// Upon failure to parse initial nodes or if the initial nodes have different passwords, - /// an error is returned. - pub fn open(self) -> RedisResult { - ClusterClient::build(self) + fn build(self) -> RedisResult { + let initial_nodes = self.initial_nodes?; + let mut nodes = Vec::with_capacity(initial_nodes.len()); + let mut connection_info_password = None::; + let mut connection_info_username = None::; + + for (index, info) in initial_nodes.into_iter().enumerate() { + if let ConnectionAddr::Unix(_) = info.addr { + return Err(RedisError::from((ErrorKind::InvalidClientConfig, + "This library cannot use unix socket because Redis's cluster command returns only cluster's IP and port."))); + } + + if self.password.is_none() { + if index == 0 { + connection_info_password = info.redis.password.clone(); + } else if connection_info_password != info.redis.password { + return Err(RedisError::from(( + ErrorKind::InvalidClientConfig, + "Cannot use different password among initial nodes.", + ))); + } + } + + if self.username.is_none() { + if index == 0 { + connection_info_username = info.redis.username.clone(); + } else if connection_info_username != info.redis.username { + return Err(RedisError::from(( + ErrorKind::InvalidClientConfig, + "Cannot use different username among initial nodes.", + ))); + } + } + + nodes.push(info); + } + + Ok(ClusterClient { + initial_nodes: nodes, + read_from_replicas: self.read_from_replicas, + username: self.username.or(connection_info_username), + password: self.password.or(connection_info_password), + }) } /// Set password for new ClusterClient. @@ -59,6 +92,18 @@ impl ClusterClientBuilder { self } + /// Builds a [`ClusterClient`]. Despite the name, this does not actually open + /// a connection to Redis Cluster, but will perform some basic checks of the initial + /// nodes' URLs and passwords. + /// + /// # Errors + /// + /// Upon failure to parse initial nodes or if the initial nodes have different passwords, + /// an error is returned. + pub fn open(self) -> RedisResult { + self.build() + } + /// Use `read_from_replicas()`. #[deprecated(since = "0.22.0", note = "Use read_from_replicas()")] pub fn readonly(mut self, read_from_replicas: bool) -> ClusterClientBuilder { @@ -76,20 +121,8 @@ pub struct ClusterClient { } impl ClusterClient { - /// Create a [ClusterClient](ClusterClient) with the default configuration. Despite the name, - /// this does not actually open a connection to Redis Cluster, but only performs some basic - /// checks of the initial nodes' URLs and passwords. - /// - /// # Errors - /// - /// Upon failure to parse initial nodes or if the initial nodes have different passwords, - /// an error is returned. - pub fn open(initial_nodes: Vec) -> RedisResult { - ClusterClientBuilder::new(initial_nodes).open() - } - /// Opens connections to Redis Cluster nodes and returns a - /// [ClusterConnection](ClusterConnection). + /// [`ClusterConnection`]. /// /// # Errors /// @@ -103,49 +136,16 @@ impl ClusterClient { ) } - fn build(builder: ClusterClientBuilder) -> RedisResult { - let initial_nodes = builder.initial_nodes?; - let mut nodes = Vec::with_capacity(initial_nodes.len()); - let mut connection_info_password = None::; - let mut connection_info_username = None::; - - for (index, info) in initial_nodes.into_iter().enumerate() { - if let ConnectionAddr::Unix(_) = info.addr { - return Err(RedisError::from((ErrorKind::InvalidClientConfig, - "This library cannot use unix socket because Redis's cluster command returns only cluster's IP and port."))); - } - - if builder.password.is_none() { - if index == 0 { - connection_info_password = info.redis.password.clone(); - } else if connection_info_password != info.redis.password { - return Err(RedisError::from(( - ErrorKind::InvalidClientConfig, - "Cannot use different password among initial nodes.", - ))); - } - } - - if builder.username.is_none() { - if index == 0 { - connection_info_username = info.redis.username.clone(); - } else if connection_info_username != info.redis.username { - return Err(RedisError::from(( - ErrorKind::InvalidClientConfig, - "Cannot use different username among initial nodes.", - ))); - } - } - - nodes.push(info); - } - - Ok(ClusterClient { - initial_nodes: nodes, - read_from_replicas: builder.read_from_replicas, - username: builder.username.or(connection_info_username), - password: builder.password.or(connection_info_password), - }) + /// Create a [`ClusterClient`] with the default configuration. Despite the name, + /// this does not actually open a connection to Redis Cluster, but only performs some basic + /// checks of the initial nodes' URLs and passwords. + /// + /// # Errors + /// + /// Upon failure to parse initial nodes or if the initial nodes have different passwords, + /// an error is returned. + pub fn open(initial_nodes: Vec) -> RedisResult { + ClusterClientBuilder::new(initial_nodes).open() } } From 53e6c4baaf43c9e2c36002cb9eca3ff910f589b6 Mon Sep 17 00:00:00 2001 From: Utkarsh Gupta Date: Wed, 31 Aug 2022 03:46:02 +0530 Subject: [PATCH 2/4] cluster_client: implement proper builder pattern - implement proper builder pattern by adding `new` & `builder` methods to ClusterClient and `new` & `build` methods to ClusterClientBuilder - deprecate & redirect `open` methods for ClusterClient & ClusterClientBuilder --- redis/src/cluster.rs | 4 +- redis/src/cluster_client.rs | 72 +++++++++++++++++++++------------- redis/src/cluster_pipeline.rs | 4 +- redis/tests/support/cluster.rs | 2 +- 4 files changed, 50 insertions(+), 32 deletions(-) diff --git a/redis/src/cluster.rs b/redis/src/cluster.rs index 37a263786..20cd9f781 100644 --- a/redis/src/cluster.rs +++ b/redis/src/cluster.rs @@ -12,7 +12,7 @@ //! use redis::cluster::ClusterClient; //! //! let nodes = vec!["redis://127.0.0.1:6379/", "redis://127.0.0.1:6378/", "redis://127.0.0.1:6377/"]; -//! let client = ClusterClient::open(nodes).unwrap(); +//! let client = ClusterClient::new(nodes).unwrap(); //! let mut connection = client.get_connection().unwrap(); //! //! let _: () = connection.set("test", "test_data").unwrap(); @@ -27,7 +27,7 @@ //! use redis::cluster::{cluster_pipe, ClusterClient}; //! //! let nodes = vec!["redis://127.0.0.1:6379/", "redis://127.0.0.1:6378/", "redis://127.0.0.1:6377/"]; -//! let client = ClusterClient::open(nodes).unwrap(); +//! let client = ClusterClient::new(nodes).unwrap(); //! let mut connection = client.get_connection().unwrap(); //! //! let key = "test"; diff --git a/redis/src/cluster_client.rs b/redis/src/cluster_client.rs index 069b26531..085fc729a 100644 --- a/redis/src/cluster_client.rs +++ b/redis/src/cluster_client.rs @@ -13,7 +13,9 @@ pub struct ClusterClientBuilder { } impl ClusterClientBuilder { - /// Generate the base configuration for new Client. + /// Creates a new `ClusterClientBuilder` with the the provided initial_nodes. + /// + /// This is the same as `ClusterClient::builder(initial_nodes)`. pub fn new(initial_nodes: Vec) -> ClusterClientBuilder { ClusterClientBuilder { initial_nodes: initial_nodes @@ -26,8 +28,18 @@ impl ClusterClientBuilder { } } - fn build(self) -> RedisResult { + /// Creates a new [`ClusterClient`] with the parameters. + /// + /// This does not create connections to the Redis Cluster, but only performs some basic checks + /// on the initial nodes' URLs and passwords/usernames. + /// + /// # Errors + /// + /// Upon failure to parse initial nodes or if the initial nodes have different passwords or + /// usernames, an error is returned. + pub fn build(self) -> RedisResult { let initial_nodes = self.initial_nodes?; + let mut nodes = Vec::with_capacity(initial_nodes.len()); let mut connection_info_password = None::; let mut connection_info_username = None::; @@ -35,7 +47,7 @@ impl ClusterClientBuilder { for (index, info) in initial_nodes.into_iter().enumerate() { if let ConnectionAddr::Unix(_) = info.addr { return Err(RedisError::from((ErrorKind::InvalidClientConfig, - "This library cannot use unix socket because Redis's cluster command returns only cluster's IP and port."))); + "This library cannot use unix socket because Redis's cluster command returns only cluster's IP and port."))); } if self.password.is_none() { @@ -92,14 +104,8 @@ impl ClusterClientBuilder { self } - /// Builds a [`ClusterClient`]. Despite the name, this does not actually open - /// a connection to Redis Cluster, but will perform some basic checks of the initial - /// nodes' URLs and passwords. - /// - /// # Errors - /// - /// Upon failure to parse initial nodes or if the initial nodes have different passwords, - /// an error is returned. + /// Use `build()`. + #[deprecated(since = "0.22.0", note = "Use build()")] pub fn open(self) -> RedisResult { self.build() } @@ -121,6 +127,24 @@ pub struct ClusterClient { } impl ClusterClient { + /// Creates a `ClusterClient` with the default parameters. + /// + /// This does not create connections to the Redis Cluster, but only performs some basic checks + /// on the initial nodes' URLs and passwords/usernames. + /// + /// # Errors + /// + /// Upon failure to parse initial nodes or if the initial nodes have different passwords or + /// usernames, an error is returned. + pub fn new(initial_nodes: Vec) -> RedisResult { + ClusterClientBuilder::new(initial_nodes).build() + } + + /// Creates a [`ClusterClientBuilder`] with the the provided initial_nodes. + pub fn builder(initial_nodes: Vec) -> ClusterClientBuilder { + ClusterClientBuilder::new(initial_nodes) + } + /// Opens connections to Redis Cluster nodes and returns a /// [`ClusterConnection`]. /// @@ -136,22 +160,16 @@ impl ClusterClient { ) } - /// Create a [`ClusterClient`] with the default configuration. Despite the name, - /// this does not actually open a connection to Redis Cluster, but only performs some basic - /// checks of the initial nodes' URLs and passwords. - /// - /// # Errors - /// - /// Upon failure to parse initial nodes or if the initial nodes have different passwords, - /// an error is returned. + /// Use `new()`. + #[deprecated(since = "0.22.0", note = "Use new()")] pub fn open(initial_nodes: Vec) -> RedisResult { - ClusterClientBuilder::new(initial_nodes).open() + ClusterClient::new(initial_nodes) } } impl Clone for ClusterClient { fn clone(&self) -> ClusterClient { - ClusterClient::open(self.initial_nodes.clone()).unwrap() + ClusterClient::new(self.initial_nodes.clone()).unwrap() } } @@ -198,26 +216,26 @@ mod tests { #[test] fn give_no_password() { - let client = ClusterClient::open(get_connection_data()).unwrap(); + let client = ClusterClient::new(get_connection_data()).unwrap(); assert_eq!(client.password, None); } #[test] fn give_password_by_initial_nodes() { - let client = ClusterClient::open(get_connection_data_with_password()).unwrap(); + let client = ClusterClient::new(get_connection_data_with_password()).unwrap(); assert_eq!(client.password, Some("password".to_string())); } #[test] fn give_username_and_password_by_initial_nodes() { - let client = ClusterClient::open(get_connection_data_with_username_and_password()).unwrap(); + let client = ClusterClient::new(get_connection_data_with_username_and_password()).unwrap(); assert_eq!(client.password, Some("password".to_string())); assert_eq!(client.username, Some("user1".to_string())); } #[test] fn give_different_password_by_initial_nodes() { - let result = ClusterClient::open(vec![ + let result = ClusterClient::new(vec![ "redis://:password1@127.0.0.1:6379", "redis://:password2@127.0.0.1:6378", "redis://:password3@127.0.0.1:6377", @@ -227,7 +245,7 @@ mod tests { #[test] fn give_different_username_by_initial_nodes() { - let result = ClusterClient::open(vec![ + let result = ClusterClient::new(vec![ "redis://user1:password@127.0.0.1:6379", "redis://user2:password@127.0.0.1:6378", "redis://user1:password@127.0.0.1:6377", @@ -240,7 +258,7 @@ mod tests { let client = ClusterClientBuilder::new(get_connection_data_with_password()) .password("pass".to_string()) .username("user1".to_string()) - .open() + .build() .unwrap(); assert_eq!(client.password, Some("pass".to_string())); assert_eq!(client.username, Some("user1".to_string())); diff --git a/redis/src/cluster_pipeline.rs b/redis/src/cluster_pipeline.rs index 2a295a392..920d6962f 100644 --- a/redis/src/cluster_pipeline.rs +++ b/redis/src/cluster_pipeline.rs @@ -92,7 +92,7 @@ impl ClusterPipeline { /// /// ```rust,no_run /// # let nodes = vec!["redis://127.0.0.1:6379/"]; - /// # let client = redis::cluster::ClusterClient::open(nodes).unwrap(); + /// # let client = redis::cluster::ClusterClient::new(nodes).unwrap(); /// # let mut con = client.get_connection().unwrap(); /// let mut pipe = redis::cluster::cluster_pipe(); /// let (k1, k2) : (i32, i32) = pipe @@ -137,7 +137,7 @@ impl ClusterPipeline { /// /// ```rust,no_run /// # let nodes = vec!["redis://127.0.0.1:6379/"]; - /// # let client = redis::cluster::ClusterClient::open(nodes).unwrap(); + /// # let client = redis::cluster::ClusterClient::new(nodes).unwrap(); /// # let mut con = client.get_connection().unwrap(); /// let mut pipe = redis::cluster::cluster_pipe(); /// let _ : () = pipe.cmd("SET").arg("key_1").arg(42).ignore().query(&mut con).unwrap(); diff --git a/redis/tests/support/cluster.rs b/redis/tests/support/cluster.rs index 6093980fb..f0967d5ee 100644 --- a/redis/tests/support/cluster.rs +++ b/redis/tests/support/cluster.rs @@ -232,7 +232,7 @@ impl TestClusterContext { .collect(), ); builder = initializer(builder); - let client = builder.open().unwrap(); + let client = builder.build().unwrap(); TestClusterContext { cluster, client } } From 7bbac5fbf12d45aa9edd692a0c191c64e86d9c38 Mon Sep 17 00:00:00 2001 From: Utkarsh Gupta Date: Sun, 24 Jul 2022 21:04:41 +0530 Subject: [PATCH 3/4] cluster_client: use ClusterParams struct to pass params - this is to simplify passing multiple params to & inside ClusterConnection impl --- redis/src/cluster.rs | 19 +++--- redis/src/cluster_client.rs | 128 +++++++++++++++++------------------- 2 files changed, 68 insertions(+), 79 deletions(-) diff --git a/redis/src/cluster.rs b/redis/src/cluster.rs index 20cd9f781..aa9520548 100644 --- a/redis/src/cluster.rs +++ b/redis/src/cluster.rs @@ -55,6 +55,7 @@ use super::{ Cmd, Connection, ConnectionAddr, ConnectionInfo, ConnectionLike, ErrorKind, IntoConnectionInfo, RedisError, RedisResult, Value, }; +use crate::cluster_client::ClusterParams; pub use crate::cluster_client::{ClusterClient, ClusterClientBuilder}; use crate::cluster_pipeline::UNROUTABLE_ERROR; @@ -95,25 +96,23 @@ impl TlsMode { impl ClusterConnection { pub(crate) fn new( + cluster_params: ClusterParams, initial_nodes: Vec, - read_from_replicas: bool, - username: Option, - password: Option, ) -> RedisResult { let connections = Self::create_initial_connections( &initial_nodes, - read_from_replicas, - username.clone(), - password.clone(), + cluster_params.read_from_replicas, + cluster_params.username.clone(), + cluster_params.password.clone(), )?; let connection = ClusterConnection { connections: RefCell::new(connections), slots: RefCell::new(SlotMap::new()), auto_reconnect: RefCell::new(true), - read_from_replicas, - username, - password, + read_from_replicas: cluster_params.read_from_replicas, + username: cluster_params.username, + password: cluster_params.password, read_timeout: RefCell::new(None), write_timeout: RefCell::new(None), #[cfg(feature = "tls")] @@ -135,7 +134,7 @@ impl ClusterConnection { }, #[cfg(not(feature = "tls"))] tls: None, - initial_nodes, + initial_nodes: initial_nodes.to_vec(), }; connection.refresh_slots()?; diff --git a/redis/src/cluster_client.rs b/redis/src/cluster_client.rs index 085fc729a..b3c50f2d2 100644 --- a/redis/src/cluster_client.rs +++ b/redis/src/cluster_client.rs @@ -1,15 +1,19 @@ use crate::cluster::ClusterConnection; +use crate::connection::{ConnectionAddr, ConnectionInfo, IntoConnectionInfo}; +use crate::types::{ErrorKind, RedisError, RedisResult}; -use super::{ - ConnectionAddr, ConnectionInfo, ErrorKind, IntoConnectionInfo, RedisError, RedisResult, -}; +/// Redis cluster specific parameters. +#[derive(Default, Clone)] +pub(crate) struct ClusterParams { + pub(crate) password: Option, + pub(crate) username: Option, + pub(crate) read_from_replicas: bool, +} /// Used to configure and build a [`ClusterClient`]. pub struct ClusterClientBuilder { initial_nodes: RedisResult>, - read_from_replicas: bool, - username: Option, - password: Option, + cluster_params: ClusterParams, } impl ClusterClientBuilder { @@ -22,9 +26,7 @@ impl ClusterClientBuilder { .into_iter() .map(|x| x.into_connection_info()) .collect(), - read_from_replicas: false, - username: None, - password: None, + cluster_params: ClusterParams::default(), } } @@ -40,67 +42,68 @@ impl ClusterClientBuilder { pub fn build(self) -> RedisResult { let initial_nodes = self.initial_nodes?; - let mut nodes = Vec::with_capacity(initial_nodes.len()); - let mut connection_info_password = None::; - let mut connection_info_username = None::; + let mut cluster_params = self.cluster_params; + let password = if cluster_params.password.is_none() { + cluster_params.password = initial_nodes[0].redis.password.clone(); + &cluster_params.password + } else { + &None + }; + let username = if cluster_params.username.is_none() { + cluster_params.username = initial_nodes[0].redis.username.clone(); + &cluster_params.username + } else { + &None + }; - for (index, info) in initial_nodes.into_iter().enumerate() { - if let ConnectionAddr::Unix(_) = info.addr { + let mut nodes = Vec::with_capacity(initial_nodes.len()); + for node in initial_nodes { + if let ConnectionAddr::Unix(_) = node.addr { return Err(RedisError::from((ErrorKind::InvalidClientConfig, - "This library cannot use unix socket because Redis's cluster command returns only cluster's IP and port."))); + "This library cannot use unix socket because Redis's cluster command returns only cluster's IP and port."))); } - if self.password.is_none() { - if index == 0 { - connection_info_password = info.redis.password.clone(); - } else if connection_info_password != info.redis.password { - return Err(RedisError::from(( - ErrorKind::InvalidClientConfig, - "Cannot use different password among initial nodes.", - ))); - } + if password.is_some() && node.redis.password != *password { + return Err(RedisError::from(( + ErrorKind::InvalidClientConfig, + "Cannot use different password among initial nodes.", + ))); } - if self.username.is_none() { - if index == 0 { - connection_info_username = info.redis.username.clone(); - } else if connection_info_username != info.redis.username { - return Err(RedisError::from(( - ErrorKind::InvalidClientConfig, - "Cannot use different username among initial nodes.", - ))); - } + if username.is_some() && node.redis.username != *username { + return Err(RedisError::from(( + ErrorKind::InvalidClientConfig, + "Cannot use different username among initial nodes.", + ))); } - nodes.push(info); + nodes.push(node); } Ok(ClusterClient { initial_nodes: nodes, - read_from_replicas: self.read_from_replicas, - username: self.username.or(connection_info_username), - password: self.password.or(connection_info_password), + cluster_params, }) } - /// Set password for new ClusterClient. + /// Sets password for new ClusterClient. pub fn password(mut self, password: String) -> ClusterClientBuilder { - self.password = Some(password); + self.cluster_params.password = Some(password); self } - /// Set username for new ClusterClient. + /// Sets username for new ClusterClient. pub fn username(mut self, username: String) -> ClusterClientBuilder { - self.username = Some(username); + self.cluster_params.username = Some(username); self } - /// Enable read from replicas for new ClusterClient (default is false). + /// Enables read from replicas for new ClusterClient (default is false). /// /// If True, then read queries will go to the replica nodes & write queries will go to the /// primary nodes. If there are no replica nodes, then all queries will go to the primary nodes. pub fn read_from_replicas(mut self) -> ClusterClientBuilder { - self.read_from_replicas = true; + self.cluster_params.read_from_replicas = true; self } @@ -113,17 +116,16 @@ impl ClusterClientBuilder { /// Use `read_from_replicas()`. #[deprecated(since = "0.22.0", note = "Use read_from_replicas()")] pub fn readonly(mut self, read_from_replicas: bool) -> ClusterClientBuilder { - self.read_from_replicas = read_from_replicas; + self.cluster_params.read_from_replicas = read_from_replicas; self } } /// This is a Redis cluster client. +#[derive(Clone)] pub struct ClusterClient { initial_nodes: Vec, - read_from_replicas: bool, - username: Option, - password: Option, + cluster_params: ClusterParams, } impl ClusterClient { @@ -145,19 +147,14 @@ impl ClusterClient { ClusterClientBuilder::new(initial_nodes) } - /// Opens connections to Redis Cluster nodes and returns a + /// Creates new connections to Redis Cluster nodes and return a /// [`ClusterConnection`]. /// /// # Errors /// - /// An error is returned if there is a failure to open connections or to create slots. + /// An error is returned if there is a failure while creating connections or slots. pub fn get_connection(&self) -> RedisResult { - ClusterConnection::new( - self.initial_nodes.clone(), - self.read_from_replicas, - self.username.clone(), - self.password.clone(), - ) + ClusterConnection::new(self.cluster_params.clone(), self.initial_nodes.clone()) } /// Use `new()`. @@ -167,16 +164,9 @@ impl ClusterClient { } } -impl Clone for ClusterClient { - fn clone(&self) -> ClusterClient { - ClusterClient::new(self.initial_nodes.clone()).unwrap() - } -} - #[cfg(test)] mod tests { - use super::{ClusterClient, ClusterClientBuilder}; - use super::{ConnectionInfo, IntoConnectionInfo}; + use super::{ClusterClient, ClusterClientBuilder, ConnectionInfo, IntoConnectionInfo}; fn get_connection_data() -> Vec { vec![ @@ -217,20 +207,20 @@ mod tests { #[test] fn give_no_password() { let client = ClusterClient::new(get_connection_data()).unwrap(); - assert_eq!(client.password, None); + assert_eq!(client.cluster_params.password, None); } #[test] fn give_password_by_initial_nodes() { let client = ClusterClient::new(get_connection_data_with_password()).unwrap(); - assert_eq!(client.password, Some("password".to_string())); + assert_eq!(client.cluster_params.password, Some("password".to_string())); } #[test] fn give_username_and_password_by_initial_nodes() { let client = ClusterClient::new(get_connection_data_with_username_and_password()).unwrap(); - assert_eq!(client.password, Some("password".to_string())); - assert_eq!(client.username, Some("user1".to_string())); + assert_eq!(client.cluster_params.password, Some("password".to_string())); + assert_eq!(client.cluster_params.username, Some("user1".to_string())); } #[test] @@ -260,7 +250,7 @@ mod tests { .username("user1".to_string()) .build() .unwrap(); - assert_eq!(client.password, Some("pass".to_string())); - assert_eq!(client.username, Some("user1".to_string())); + assert_eq!(client.cluster_params.password, Some("pass".to_string())); + assert_eq!(client.cluster_params.username, Some("user1".to_string())); } } From f3fd7d2d5dbe4e0b09d4fb6ccd7d049dd137207a Mon Sep 17 00:00:00 2001 From: Utkarsh Gupta Date: Wed, 24 Aug 2022 12:38:53 +0530 Subject: [PATCH 4/4] cluster_client: add handling for empty initial_nodes --- redis/src/cluster_client.rs | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/redis/src/cluster_client.rs b/redis/src/cluster_client.rs index b3c50f2d2..f5815c885 100644 --- a/redis/src/cluster_client.rs +++ b/redis/src/cluster_client.rs @@ -42,15 +42,25 @@ impl ClusterClientBuilder { pub fn build(self) -> RedisResult { let initial_nodes = self.initial_nodes?; + let first_node = match initial_nodes.first() { + Some(node) => node, + None => { + return Err(RedisError::from(( + ErrorKind::InvalidClientConfig, + "Initial nodes can't be empty.", + ))) + } + }; + let mut cluster_params = self.cluster_params; let password = if cluster_params.password.is_none() { - cluster_params.password = initial_nodes[0].redis.password.clone(); + cluster_params.password = first_node.redis.password.clone(); &cluster_params.password } else { &None }; let username = if cluster_params.username.is_none() { - cluster_params.username = initial_nodes[0].redis.username.clone(); + cluster_params.username = first_node.redis.username.clone(); &cluster_params.username } else { &None @@ -253,4 +263,10 @@ mod tests { assert_eq!(client.cluster_params.password, Some("pass".to_string())); assert_eq!(client.cluster_params.username, Some("user1".to_string())); } + + #[test] + fn give_empty_initial_nodes() { + let client = ClusterClient::new(Vec::::new()); + assert!(client.is_err()) + } }