refactor(redis): invoke redis_conn() method instead of cloning redis_conn property in StorageInterface (#1552)

Co-authored-by: jeeva <jeeva.ramu@codurance.com>
Co-authored-by: Sanchith Hegde <22217505+SanchithHegde@users.noreply.github.com>

crates/router/src/core/payment_methods/cards.rs

@@ -1713,7 +1713,12 @@ pub async fn list_customer_payment_method(
         };
         customer_pms.push(pma.to_owned());
 
-        let redis_conn = state.store.get_redis_conn();
+        let redis_conn = state
+            .store
+            .get_redis_conn()
+            .change_context(errors::ApiErrorResponse::InternalServerError)
+            .attach_printable("Failed to get redis connection")?;
+
         let key_for_hyperswitch_token = format!(
             "pm_token_{}_{}_hyperswitch",
             parent_payment_method_token, pma.payment_method

crates/router/src/core/payments.rs

@@ -922,7 +922,12 @@ async fn decide_payment_method_tokenize_action(
             }
         }
         Some(token) => {
-            let redis_conn = state.store.get_redis_conn();
+            let redis_conn = state
+                .store
+                .get_redis_conn()
+                .change_context(errors::ApiErrorResponse::InternalServerError)
+                .attach_printable("Failed to get redis connection")?;
+
             let key = format!(
                 "pm_token_{}_{}_{}",
                 token.to_owned(),

crates/router/src/core/payments/helpers.rs

@@ -1175,7 +1175,12 @@ pub async fn make_pm_data<'a, F: Clone, R>(
     let request = &payment_data.payment_method_data;
     let token = payment_data.token.clone();
     let hyperswitch_token = if let Some(token) = token {
-        let redis_conn = state.store.get_redis_conn();
+        let redis_conn = state
+            .store
+            .get_redis_conn()
+            .change_context(errors::ApiErrorResponse::InternalServerError)
+            .attach_printable("Failed to get redis connection")?;
+
         let key = format!(
             "pm_token_{}_{}_hyperswitch",
             token,

crates/router/src/core/payouts/helpers.rs

@@ -46,7 +46,12 @@ pub async fn make_payout_method_data<'a>(
             )
         );
 
-        let redis_conn = state.store.get_redis_conn();
+        let redis_conn = state
+            .store
+            .get_redis_conn()
+            .change_context(errors::ApiErrorResponse::InternalServerError)
+            .attach_printable("Failed to get redis connection")?;
+
         let hyperswitch_token_option = redis_conn
             .get_key::<Option<String>>(&key)
             .await

crates/router/src/db.rs

@@ -173,8 +173,13 @@ where
 }
 
 impl services::RedisConnInterface for MockDb {
-    fn get_redis_conn(&self) -> Arc<redis_interface::RedisConnectionPool> {
+    fn get_redis_conn(
-        self.redis.clone()
+        &self,
+    ) -> Result<
+        Arc<redis_interface::RedisConnectionPool>,
+        error_stack::Report<redis_interface::errors::RedisError>,
+    > {
+        Ok(self.redis.clone())
     }
 }
 

crates/router/src/db/api_keys.rs

@@ -470,7 +470,7 @@ mod tests {
     async fn test_api_keys_cache() {
         let db = MockDb::new(&Default::default()).await;
 
-        let redis_conn = db.get_redis_conn();
+        let redis_conn = db.get_redis_conn().unwrap();
         redis_conn
             .subscribe("hyperswitch_invalidate")
             .await

crates/router/src/db/cache.rs

@@ -1,5 +1,6 @@
 use common_utils::ext_traits::AsyncExt;
 use error_stack::ResultExt;
+use redis_interface::errors::RedisError;
 
 use super::StorageInterface;
 use crate::{
@@ -20,7 +21,12 @@ where
     Fut: futures::Future<Output = CustomResult<T, errors::StorageError>> + Send,
 {
     let type_name = std::any::type_name::<T>();
-    let redis = &store.get_redis_conn();
+    let redis = &store
+        .get_redis_conn()
+        .change_context(errors::StorageError::RedisError(
+            RedisError::RedisConnectionError.into(),
+        ))
+        .attach_printable("Failed to get redis connection")?;
     let redis_val = redis.get_and_deserialize_key::<T>(key, type_name).await;
     let get_data_set_redis = || async {
         let data = fun().await?;
@@ -76,8 +82,15 @@ where
 {
     let data = fun().await?;
     in_memory.async_map(|cache| cache.invalidate(key)).await;
-    store
+
+    let redis_conn = store
         .get_redis_conn()
+        .change_context(errors::StorageError::RedisError(
+            RedisError::RedisConnectionError.into(),
+        ))
+        .attach_printable("Failed to get redis connection")?;
+
+    redis_conn
         .delete_key(key)
         .await
         .change_context(errors::StorageError::KVError)?;
@@ -88,8 +101,14 @@ pub async fn publish_into_redact_channel<'a>(
     store: &dyn StorageInterface,
     key: cache::CacheKind<'a>,
 ) -> CustomResult<usize, errors::StorageError> {
-    store
+    let redis_conn = store
         .get_redis_conn()
+        .change_context(errors::StorageError::RedisError(
+            RedisError::RedisConnectionError.into(),
+        ))
+        .attach_printable("Failed to get redis connection")?;
+
+    redis_conn
         .publish(consts::PUB_SUB_CHANNEL, key)
         .await
         .change_context(errors::StorageError::KVError)

crates/router/src/db/merchant_connector_account.rs

@@ -689,7 +689,7 @@ mod merchant_connector_account_cache_tests {
     async fn test_connector_label_cache() {
         let db = MockDb::new(&Default::default()).await;
 
-        let redis_conn = db.get_redis_conn();
+        let redis_conn = db.get_redis_conn().unwrap();
         let master_key = db.get_master_key();
         redis_conn
             .subscribe("hyperswitch_invalidate")

crates/router/src/routes/dummy_connector/core.rs

@@ -91,7 +91,12 @@ pub async fn payment_complete(
         types::DummyConnectorStatus::Failed
     };
 
-    let redis_conn = state.store.get_redis_conn();
+    let redis_conn = state
+        .store
+        .get_redis_conn()
+        .change_context(errors::DummyConnectorErrors::InternalServerError)
+        .attach_printable("Failed to get redis connection")?;
+
     let _ = redis_conn.delete_key(req.attempt_id.as_str()).await;
 
     if let Ok(payment_data) = payment_data {
@@ -193,7 +198,11 @@ pub async fn refund_data(
     )
     .await;
 
-    let redis_conn = state.store.get_redis_conn();
+    let redis_conn = state
+        .store
+        .get_redis_conn()
+        .change_context(errors::DummyConnectorErrors::InternalServerError)
+        .attach_printable("Failed to get redis connection")?;
     let refund_data = redis_conn
         .get_and_deserialize_key::<types::DummyConnectorRefundResponse>(
             refund_id.as_str(),

crates/router/src/routes/dummy_connector/utils.rs

@@ -25,7 +25,11 @@ pub async fn store_data_in_redis(
     data: impl serde::Serialize + Debug,
     ttl: i64,
 ) -> types::DummyConnectorResult<()> {
-    let redis_conn = state.store.get_redis_conn();
+    let redis_conn = state
+        .store
+        .get_redis_conn()
+        .change_context(errors::DummyConnectorErrors::InternalServerError)
+        .attach_printable("Failed to get redis connection")?;
 
     redis_conn
         .serialize_and_set_key_with_expiry(&key, data, ttl)
@@ -39,7 +43,12 @@ pub async fn get_payment_data_from_payment_id(
     state: &AppState,
     payment_id: String,
 ) -> types::DummyConnectorResult<types::DummyConnectorPaymentData> {
-    let redis_conn = state.store.get_redis_conn();
+    let redis_conn = state
+        .store
+        .get_redis_conn()
+        .change_context(errors::DummyConnectorErrors::InternalServerError)
+        .attach_printable("Failed to get redis connection")?;
+
     redis_conn
         .get_and_deserialize_key::<types::DummyConnectorPaymentData>(
             payment_id.as_str(),
@@ -53,7 +62,12 @@ pub async fn get_payment_data_by_attempt_id(
     state: &AppState,
     attempt_id: String,
 ) -> types::DummyConnectorResult<types::DummyConnectorPaymentData> {
-    let redis_conn = state.store.get_redis_conn();
+    let redis_conn = state
+        .store
+        .get_redis_conn()
+        .change_context(errors::DummyConnectorErrors::InternalServerError)
+        .attach_printable("Failed to get redis connection")?;
+
     redis_conn
         .get_and_deserialize_key::<String>(attempt_id.as_str(), "String")
         .await

crates/router/src/services.rs

@@ -106,12 +106,22 @@ impl PubSubInterface for redis_interface::RedisConnectionPool {
 }
 
 pub trait RedisConnInterface {
-    fn get_redis_conn(&self) -> Arc<redis_interface::RedisConnectionPool>;
+    fn get_redis_conn(
+        &self,
+    ) -> common_utils::errors::CustomResult<
+        Arc<redis_interface::RedisConnectionPool>,
+        errors::RedisError,
+    >;
 }
 
 impl RedisConnInterface for Store {
-    fn get_redis_conn(&self) -> Arc<redis_interface::RedisConnectionPool> {
+    fn get_redis_conn(
-        self.redis_conn.clone()
+        &self,
+    ) -> common_utils::errors::CustomResult<
+        Arc<redis_interface::RedisConnectionPool>,
+        errors::RedisError,
+    > {
+        self.redis_conn()
     }
 }
 

crates/router/tests/cache.rs

@@ -19,6 +19,7 @@ async fn invalidate_existing_cache_success() {
     let _ = state
         .store
         .get_redis_conn()
+        .unwrap()
         .set_key(&cache_key.clone(), cache_key_value.clone())
         .await;
 

crates/router/tests/services.rs

@@ -0,0 +1,39 @@
+use std::sync::atomic;
+
+use router::{configs::settings::Settings, routes};
+
+mod utils;
+
+#[tokio::test]
+#[should_panic]
+async fn get_redis_conn_failure() {
+    // Arrange
+    utils::setup().await;
+    let (tx, _) = tokio::sync::oneshot::channel();
+    let state = routes::AppState::new(Settings::default(), tx).await;
+
+    let _ = state.store.get_redis_conn().map(|conn| {
+        conn.is_redis_available
+            .store(false, atomic::Ordering::SeqCst)
+    });
+
+    // Act
+    let _ = state.store.get_redis_conn();
+
+    // Assert
+    // based on #[should_panic] attribute
+}
+
+#[tokio::test]
+async fn get_redis_conn_success() {
+    // Arrange
+    utils::setup().await;
+    let (tx, _) = tokio::sync::oneshot::channel();
+    let state = routes::AppState::new(Settings::default(), tx).await;
+
+    // Act
+    let result = state.store.get_redis_conn();
+
+    // Assert
+    assert!(result.is_ok())
+}