feat(api): use ApiClient trait in AppState (#2067)

crates/router/src/bin/scheduler.rs

@@ -1,10 +1,11 @@
 #![recursion_limit = "256"]
 use std::sync::Arc;
 
+use error_stack::ResultExt;
 use router::{
     configs::settings::{CmdLineConf, Settings},
     core::errors::{self, CustomResult},
-    logger, routes, scheduler,
+    logger, routes, scheduler, services,
 };
 use tokio::sync::{mpsc, oneshot};
 
@@ -19,9 +20,17 @@ async fn main() -> CustomResult<(), errors::ProcessTrackerError> {
     #[allow(clippy::expect_used)]
     let conf = Settings::with_config_path(cmd_line.config_path)
         .expect("Unable to construct application configuration");
+
+    let api_client = Box::new(
+        services::ProxyClient::new(
+            conf.proxy.clone(),
+            services::proxy_bypass_urls(&conf.locker),
+        )
+        .change_context(errors::ProcessTrackerError::ConfigurationError)?,
+    );
     // channel for listening to redis disconnect events
     let (redis_shutdown_signal_tx, redis_shutdown_signal_rx) = oneshot::channel();
-    let state = routes::AppState::new(conf, redis_shutdown_signal_tx).await;
+    let state = routes::AppState::new(conf, redis_shutdown_signal_tx, api_client).await;
     // channel to shutdown scheduler gracefully
     let (tx, rx) = mpsc::channel(1);
     tokio::spawn(router::receiver_for_error(

crates/router/src/core/errors.rs

@@ -194,6 +194,9 @@ pub enum ApplicationError {
 
     #[error("I/O: {0}")]
     IoError(std::io::Error),
+
+    #[error("Error while constructing api client: {0}")]
+    ApiClientError(ApiClientError),
 }
 
 impl From<MetricsError> for ApplicationError {
@@ -232,7 +235,8 @@ impl ResponseError for ApplicationError {
             Self::MetricsError(_)
             | Self::IoError(_)
             | Self::ConfigurationError(_)
-            | Self::InvalidConfigurationValueError(_) => StatusCode::INTERNAL_SERVER_ERROR,
+            | Self::InvalidConfigurationValueError(_)
+            | Self::ApiClientError(_) => StatusCode::INTERNAL_SERVER_ERROR,
         }
     }
 
@@ -248,7 +252,7 @@ pub fn http_not_implemented() -> actix_web::HttpResponse<BoxBody> {
     .error_response()
 }
 
-#[derive(Debug, thiserror::Error, PartialEq)]
+#[derive(Debug, thiserror::Error, PartialEq, Clone)]
 pub enum ApiClientError {
     #[error("Header map construction failed")]
     HeaderMapConstructionFailed,

crates/router/src/lib.rs

@@ -169,7 +169,16 @@ pub async fn start_server(conf: settings::Settings) -> ApplicationResult<Server>
     logger::debug!(startup_config=?conf);
     let server = conf.server.clone();
     let (tx, rx) = oneshot::channel();
-    let state = routes::AppState::new(conf, tx).await;
+    let api_client = Box::new(
+        services::ProxyClient::new(
+            conf.proxy.clone(),
+            services::proxy_bypass_urls(&conf.locker),
+        )
+        .map_err(|error| {
+            errors::ApplicationError::ApiClientError(error.current_context().clone())
+        })?,
+    );
+    let state = routes::AppState::new(conf, tx, api_client).await;
     let request_body_limit = server.request_body_limit;
     let server = actix_web::HttpServer::new(move || mk_app(state.clone(), request_body_limit))
         .bind((server.host.as_str(), server.port))?

crates/router/src/routes/app.rs

@@ -34,6 +34,7 @@ pub struct AppState {
     pub email_client: Box<dyn EmailClient>,
     #[cfg(feature = "kms")]
     pub kms_secrets: settings::ActiveKmsSecrets,
+    pub api_client: Box<dyn crate::services::ApiClient>,
 }
 
 pub trait AppStateInfo {
@@ -68,6 +69,7 @@ impl AppState {
         conf: settings::Settings,
         storage_impl: StorageImpl,
         shut_down_signal: oneshot::Sender<()>,
+        api_client: Box<dyn crate::services::ApiClient>,
     ) -> Self {
         #[cfg(feature = "kms")]
         let kms_client = kms::get_kms_client(&conf.kms).await;
@@ -101,11 +103,16 @@ impl AppState {
             email_client,
             #[cfg(feature = "kms")]
             kms_secrets,
+            api_client,
         }
     }
 
-    pub async fn new(conf: settings::Settings, shut_down_signal: oneshot::Sender<()>) -> Self {
+    pub async fn new(
-        Self::with_storage(conf, StorageImpl::Postgresql, shut_down_signal).await
+        conf: settings::Settings,
+        shut_down_signal: oneshot::Sender<()>,
+        api_client: Box<dyn crate::services::ApiClient>,
+    ) -> Self {
+        Self::with_storage(conf, StorageImpl::Postgresql, shut_down_signal, api_client).await
     }
 }
 

crates/router/src/services/api.rs

@@ -12,6 +12,7 @@ use std::{
 
 use actix_web::{body, HttpRequest, HttpResponse, Responder, ResponseError};
 use api_models::enums::CaptureMethod;
+pub use client::{proxy_bypass_urls, ApiClient, MockApiClient, ProxyClient};
 use common_utils::errors::ReportSwitchExt;
 use error_stack::{report, IntoReport, Report, ResultExt};
 use masking::{ExposeOptionInterface, PeekInterface};
@@ -451,10 +452,9 @@ pub async fn send_request(
     let should_bypass_proxy = url
         .as_str()
         .starts_with(&state.conf.connectors.dummyconnector.base_url)
-        || client::proxy_bypass_urls(&state.conf.locker).contains(&url.to_string());
+        || proxy_bypass_urls(&state.conf.locker).contains(&url.to_string());
     #[cfg(not(feature = "dummy_connector"))]
-    let should_bypass_proxy =
+    let should_bypass_proxy = proxy_bypass_urls(&state.conf.locker).contains(&url.to_string());
-        client::proxy_bypass_urls(&state.conf.locker).contains(&url.to_string());
     let client = client::create_client(
         &state.conf.proxy,
         should_bypass_proxy,

crates/router/src/services/api/client.rs

@@ -4,7 +4,7 @@ use error_stack::{IntoReport, ResultExt};
 use http::{HeaderValue, Method};
 use masking::PeekInterface;
 use once_cell::sync::OnceCell;
-use reqwest::{multipart::Form, IntoUrl};
+use reqwest::multipart::Form;
 
 use super::request::Maskable;
 use crate::{
@@ -106,7 +106,7 @@ pub(super) fn create_client(
     }
 }
 
-pub(super) fn proxy_bypass_urls(locker: &Locker) -> Vec<String> {
+pub fn proxy_bypass_urls(locker: &Locker) -> Vec<String> {
     let locker_host = locker.host.to_owned();
     let basilisk_host = locker.basilisk_host.to_owned();
     vec![
@@ -140,15 +140,11 @@ pub trait RequestBuilder: Send + Sync {
     >;
 }
 
-pub trait ApiClient
+pub trait ApiClient: dyn_clone::DynClone
 where
-    Self: Sized + Send + Sync,
+    Self: Send + Sync,
 {
-    fn new(
+    fn request(
-        proxy_config: Proxy,
-        whitelisted_urls: Vec<String>,
-    ) -> CustomResult<Self, ApiClientError>;
-    fn request<U: IntoUrl>(
         &self,
         method: Method,
         url: String,
@@ -162,6 +158,8 @@ where
     ) -> CustomResult<Box<dyn RequestBuilder>, ApiClientError>;
 }
 
+dyn_clone::clone_trait_object!(ApiClient);
+
 #[derive(Clone)]
 pub struct ProxyClient {
     proxy_client: reqwest::Client,
@@ -170,6 +168,45 @@ pub struct ProxyClient {
 }
 
 impl ProxyClient {
+    pub fn new(
+        proxy_config: Proxy,
+        whitelisted_urls: Vec<String>,
+    ) -> CustomResult<Self, ApiClientError> {
+        let non_proxy_client = reqwest::Client::builder()
+            .redirect(reqwest::redirect::Policy::none())
+            .build()
+            .into_report()
+            .change_context(ApiClientError::ClientConstructionFailed)?;
+
+        let mut proxy_builder =
+            reqwest::Client::builder().redirect(reqwest::redirect::Policy::none());
+
+        if let Some(url) = proxy_config.https_url.as_ref() {
+            proxy_builder = proxy_builder.proxy(
+                reqwest::Proxy::https(url)
+                    .into_report()
+                    .change_context(ApiClientError::InvalidProxyConfiguration)?,
+            );
+        }
+
+        if let Some(url) = proxy_config.http_url.as_ref() {
+            proxy_builder = proxy_builder.proxy(
+                reqwest::Proxy::http(url)
+                    .into_report()
+                    .change_context(ApiClientError::InvalidProxyConfiguration)?,
+            );
+        }
+
+        let proxy_client = proxy_builder
+            .build()
+            .into_report()
+            .change_context(ApiClientError::InvalidProxyConfiguration)?;
+        Ok(Self {
+            proxy_client,
+            non_proxy_client,
+            whitelisted_urls,
+        })
+    }
     fn get_reqwest_client(
         &self,
         base_url: String,
@@ -262,47 +299,7 @@ impl RequestBuilder for RouterRequestBuilder {
 // TODO: remove this when integrating this trait
 #[allow(dead_code)]
 impl ApiClient for ProxyClient {
-    fn new(
+    fn request(
-        proxy_config: Proxy,
-        whitelisted_urls: Vec<String>,
-    ) -> CustomResult<Self, ApiClientError> {
-        let non_proxy_client = reqwest::Client::builder()
-            .redirect(reqwest::redirect::Policy::none())
-            .build()
-            .into_report()
-            .change_context(ApiClientError::ClientConstructionFailed)?;
-
-        let mut proxy_builder =
-            reqwest::Client::builder().redirect(reqwest::redirect::Policy::none());
-
-        if let Some(url) = proxy_config.https_url.as_ref() {
-            proxy_builder = proxy_builder.proxy(
-                reqwest::Proxy::https(url)
-                    .into_report()
-                    .change_context(ApiClientError::InvalidProxyConfiguration)?,
-            );
-        }
-
-        if let Some(url) = proxy_config.http_url.as_ref() {
-            proxy_builder = proxy_builder.proxy(
-                reqwest::Proxy::http(url)
-                    .into_report()
-                    .change_context(ApiClientError::InvalidProxyConfiguration)?,
-            );
-        }
-
-        let proxy_client = proxy_builder
-            .build()
-            .into_report()
-            .change_context(ApiClientError::InvalidProxyConfiguration)?;
-        Ok(Self {
-            proxy_client,
-            non_proxy_client,
-            whitelisted_urls,
-        })
-    }
-
-    fn request<U: IntoUrl>(
         &self,
         method: Method,
         url: String,
@@ -325,3 +322,31 @@ impl ApiClient for ProxyClient {
         }))
     }
 }
+
+///
+/// Api client for testing sending request
+///
+#[derive(Clone)]
+pub struct MockApiClient;
+
+impl ApiClient for MockApiClient {
+    fn request(
+        &self,
+        _method: Method,
+        _url: String,
+    ) -> CustomResult<Box<dyn RequestBuilder>, ApiClientError> {
+        // [#2066]: Add Mock implementation for ApiClient
+        Err(ApiClientError::UnexpectedState.into())
+    }
+
+    fn request_with_certificate(
+        &self,
+        _method: Method,
+        _url: String,
+        _certificate: Option<String>,
+        _certificate_key: Option<String>,
+    ) -> CustomResult<Box<dyn RequestBuilder>, ApiClientError> {
+        // [#2066]: Add Mock implementation for ApiClient
+        Err(ApiClientError::UnexpectedState.into())
+    }
+}

crates/router/src/types/storage/payment_attempt.rs

@@ -74,7 +74,7 @@ mod tests {
     use crate::{
         configs::settings::Settings,
         db::StorageImpl,
-        routes,
+        routes, services,
         types::{self, storage::enums},
     };
 
@@ -83,7 +83,9 @@ mod tests {
     async fn test_payment_attempt_insert() {
         let conf = Settings::new().expect("invalid settings");
         let tx: oneshot::Sender<()> = oneshot::channel().0;
-        let state = routes::AppState::with_storage(conf, StorageImpl::PostgresqlTest, tx).await;
+        let api_client = Box::new(services::MockApiClient);
+        let state =
+            routes::AppState::with_storage(conf, StorageImpl::PostgresqlTest, tx, api_client).await;
 
         let payment_id = Uuid::new_v4().to_string();
         let current_time = common_utils::date_time::now();
@@ -113,7 +115,11 @@ mod tests {
         use crate::configs::settings::Settings;
         let conf = Settings::new().expect("invalid settings");
         let tx: oneshot::Sender<()> = oneshot::channel().0;
-        let state = routes::AppState::with_storage(conf, StorageImpl::PostgresqlTest, tx).await;
+
+        let api_client = Box::new(services::MockApiClient);
+
+        let state =
+            routes::AppState::with_storage(conf, StorageImpl::PostgresqlTest, tx, api_client).await;
 
         let current_time = common_utils::date_time::now();
         let payment_id = Uuid::new_v4().to_string();
@@ -160,7 +166,11 @@ mod tests {
         let conf = Settings::new().expect("invalid settings");
         let uuid = Uuid::new_v4().to_string();
         let tx: oneshot::Sender<()> = oneshot::channel().0;
-        let state = routes::AppState::with_storage(conf, StorageImpl::PostgresqlTest, tx).await;
+
+        let api_client = Box::new(services::MockApiClient);
+
+        let state =
+            routes::AppState::with_storage(conf, StorageImpl::PostgresqlTest, tx, api_client).await;
         let current_time = common_utils::date_time::now();
         let connector = types::Connector::DummyConnector1.to_string();
 

crates/router/tests/cache.rs

@@ -1,5 +1,5 @@
 #![allow(clippy::unwrap_used)]
-use router::{configs::settings::Settings, routes};
+use router::{configs::settings::Settings, routes, services};
 use storage_impl::redis::cache;
 
 mod utils;
@@ -9,7 +9,8 @@ async fn invalidate_existing_cache_success() {
     // Arrange
     utils::setup().await;
     let (tx, _) = tokio::sync::oneshot::channel();
-    let state = routes::AppState::new(Settings::default(), tx).await;
+    let state =
+        routes::AppState::new(Settings::default(), tx, Box::new(services::MockApiClient)).await;
 
     let cache_key = "cacheKey".to_string();
     let cache_key_value = "val".to_string();

crates/router/tests/connectors/aci.rs

@@ -154,7 +154,13 @@ fn construct_refund_router_data<F>() -> types::RefundsRouterData<F> {
 async fn payments_create_success() {
     let conf = Settings::new().unwrap();
     let tx: oneshot::Sender<()> = oneshot::channel().0;
-    let state = routes::AppState::with_storage(conf, StorageImpl::PostgresqlTest, tx).await;
+    let state = routes::AppState::with_storage(
+        conf,
+        StorageImpl::PostgresqlTest,
+        tx,
+        Box::new(services::MockApiClient),
+    )
+    .await;
 
     static CV: aci::Aci = aci::Aci;
     let connector = types::api::ConnectorData {
@@ -191,7 +197,13 @@ async fn payments_create_failure() {
         let conf = Settings::new().unwrap();
         static CV: aci::Aci = aci::Aci;
         let tx: oneshot::Sender<()> = oneshot::channel().0;
-        let state = routes::AppState::with_storage(conf, StorageImpl::PostgresqlTest, tx).await;
+        let state = routes::AppState::with_storage(
+            conf,
+            StorageImpl::PostgresqlTest,
+            tx,
+            Box::new(services::MockApiClient),
+        )
+        .await;
         let connector = types::api::ConnectorData {
             connector: Box::new(&CV),
             connector_name: types::Connector::Aci,
@@ -244,7 +256,13 @@ async fn refund_for_successful_payments() {
         get_token: types::api::GetToken::Connector,
     };
     let tx: oneshot::Sender<()> = oneshot::channel().0;
-    let state = routes::AppState::with_storage(conf, StorageImpl::PostgresqlTest, tx).await;
+    let state = routes::AppState::with_storage(
+        conf,
+        StorageImpl::PostgresqlTest,
+        tx,
+        Box::new(services::MockApiClient),
+    )
+    .await;
     let connector_integration: services::BoxedConnectorIntegration<
         '_,
         types::api::Authorize,
@@ -305,7 +323,13 @@ async fn refunds_create_failure() {
         get_token: types::api::GetToken::Connector,
     };
     let tx: oneshot::Sender<()> = oneshot::channel().0;
-    let state = routes::AppState::with_storage(conf, StorageImpl::PostgresqlTest, tx).await;
+    let state = routes::AppState::with_storage(
+        conf,
+        StorageImpl::PostgresqlTest,
+        tx,
+        Box::new(services::MockApiClient),
+    )
+    .await;
     let connector_integration: services::BoxedConnectorIntegration<
         '_,
         types::api::Execute,

crates/router/tests/connectors/utils.rs

@@ -68,6 +68,7 @@ pub trait ConnectorActions: Connector {
             Settings::new().unwrap(),
             StorageImpl::PostgresqlTest,
             tx,
+            Box::new(services::MockApiClient),
         )
         .await;
         integration.execute_pretasks(&mut request, &state).await?;
@@ -91,6 +92,7 @@ pub trait ConnectorActions: Connector {
             Settings::new().unwrap(),
             StorageImpl::PostgresqlTest,
             tx,
+            Box::new(services::MockApiClient),
         )
         .await;
         integration.execute_pretasks(&mut request, &state).await?;
@@ -114,6 +116,7 @@ pub trait ConnectorActions: Connector {
             Settings::new().unwrap(),
             StorageImpl::PostgresqlTest,
             tx,
+            Box::new(services::MockApiClient),
         )
         .await;
         integration.execute_pretasks(&mut request, &state).await?;
@@ -141,6 +144,7 @@ pub trait ConnectorActions: Connector {
             Settings::new().unwrap(),
             StorageImpl::PostgresqlTest,
             tx,
+            Box::new(services::MockApiClient),
         )
         .await;
         integration.execute_pretasks(&mut request, &state).await?;
@@ -551,6 +555,7 @@ pub trait ConnectorActions: Connector {
             Settings::new().unwrap(),
             StorageImpl::PostgresqlTest,
             tx,
+            Box::new(services::MockApiClient),
         )
         .await;
         connector_integration
@@ -589,6 +594,7 @@ pub trait ConnectorActions: Connector {
             Settings::new().unwrap(),
             StorageImpl::PostgresqlTest,
             tx,
+            Box::new(services::MockApiClient),
         )
         .await;
         connector_integration
@@ -628,6 +634,7 @@ pub trait ConnectorActions: Connector {
             Settings::new().unwrap(),
             StorageImpl::PostgresqlTest,
             tx,
+            Box::new(services::MockApiClient),
         )
         .await;
         connector_integration
@@ -667,6 +674,7 @@ pub trait ConnectorActions: Connector {
             Settings::new().unwrap(),
             StorageImpl::PostgresqlTest,
             tx,
+            Box::new(services::MockApiClient),
         )
         .await;
         connector_integration
@@ -750,6 +758,7 @@ pub trait ConnectorActions: Connector {
             Settings::new().unwrap(),
             StorageImpl::PostgresqlTest,
             tx,
+            Box::new(services::MockApiClient),
         )
         .await;
         connector_integration
@@ -777,7 +786,13 @@ async fn call_connector<
 ) -> Result<RouterData<T, Req, Resp>, Report<ConnectorError>> {
     let conf = Settings::new().unwrap();
     let tx: oneshot::Sender<()> = oneshot::channel().0;
-    let state = routes::AppState::with_storage(conf, StorageImpl::PostgresqlTest, tx).await;
+    let state = routes::AppState::with_storage(
+        conf,
+        StorageImpl::PostgresqlTest,
+        tx,
+        Box::new(services::MockApiClient),
+    )
+    .await;
     services::api::execute_connector_processing_step(
         &state,
         integration,

crates/router/tests/payments.rs

@@ -275,7 +275,13 @@ async fn payments_create_core() {
     use configs::settings::Settings;
     let conf = Settings::new().expect("invalid settings");
     let tx: oneshot::Sender<()> = oneshot::channel().0;
-    let state = routes::AppState::with_storage(conf, StorageImpl::PostgresqlTest, tx).await;
+    let state = routes::AppState::with_storage(
+        conf,
+        StorageImpl::PostgresqlTest,
+        tx,
+        Box::new(services::MockApiClient),
+    )
+    .await;
 
     let key_store = state
         .store
@@ -437,7 +443,13 @@ async fn payments_create_core_adyen_no_redirect() {
     use crate::configs::settings::Settings;
     let conf = Settings::new().expect("invalid settings");
     let tx: oneshot::Sender<()> = oneshot::channel().0;
-    let state = routes::AppState::with_storage(conf, StorageImpl::PostgresqlTest, tx).await;
+    let state = routes::AppState::with_storage(
+        conf,
+        StorageImpl::PostgresqlTest,
+        tx,
+        Box::new(services::MockApiClient),
+    )
+    .await;
 
     let customer_id = format!("cust_{}", Uuid::new_v4());
     let merchant_id = "arunraj".to_string();

crates/router/tests/payments2.rs

@@ -35,7 +35,13 @@ async fn payments_create_core() {
     use router::configs::settings::Settings;
     let conf = Settings::new().expect("invalid settings");
     let tx: oneshot::Sender<()> = oneshot::channel().0;
-    let state = routes::AppState::with_storage(conf, StorageImpl::PostgresqlTest, tx).await;
+    let state = routes::AppState::with_storage(
+        conf,
+        StorageImpl::PostgresqlTest,
+        tx,
+        Box::new(services::MockApiClient),
+    )
+    .await;
 
     let key_store = state
         .store
@@ -203,7 +209,13 @@ async fn payments_create_core_adyen_no_redirect() {
     use router::configs::settings::Settings;
     let conf = Settings::new().expect("invalid settings");
     let tx: oneshot::Sender<()> = oneshot::channel().0;
-    let state = routes::AppState::with_storage(conf, StorageImpl::PostgresqlTest, tx).await;
+    let state = routes::AppState::with_storage(
+        conf,
+        StorageImpl::PostgresqlTest,
+        tx,
+        Box::new(services::MockApiClient),
+    )
+    .await;
 
     let customer_id = format!("cust_{}", Uuid::new_v4());
     let merchant_id = "arunraj".to_string();

crates/router/tests/services.rs

@@ -1,6 +1,6 @@
 use std::sync::atomic;
 
-use router::{configs::settings::Settings, routes};
+use router::{configs::settings::Settings, routes, services};
 
 mod utils;
 
@@ -10,7 +10,8 @@ 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 =
+        routes::AppState::new(Settings::default(), tx, Box::new(services::MockApiClient)).await;
 
     let _ = state.store.get_redis_conn().map(|conn| {
         conn.is_redis_available
@@ -29,7 +30,8 @@ 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;
+    let state =
+        routes::AppState::new(Settings::default(), tx, Box::new(services::MockApiClient)).await;
 
     // Act
     let result = state.store.get_redis_conn();

crates/router/tests/utils.rs

@@ -11,7 +11,7 @@ use actix_web::{
     test::{call_and_read_body_json, TestRequest},
 };
 use derive_deref::Deref;
-use router::{configs::settings::Settings, routes::AppState};
+use router::{configs::settings::Settings, routes::AppState, services};
 use serde::{de::DeserializeOwned, Deserialize};
 use serde_json::{json, Value};
 use tokio::sync::{oneshot, OnceCell};
@@ -48,7 +48,13 @@ pub async fn mk_service(
         conf.connectors.stripe.base_url = url;
     }
     let tx: oneshot::Sender<()> = oneshot::channel().0;
-    let app_state = AppState::with_storage(conf, router::db::StorageImpl::Mock, tx).await;
+    let app_state = AppState::with_storage(
+        conf,
+        router::db::StorageImpl::Mock,
+        tx,
+        Box::new(services::MockApiClient),
+    )
+    .await;
     actix_web::test::init_service(router::mk_app(app_state, request_body_limit)).await
 }