refactor(connector_customer): incorrect mapping of connector customer (#1275)

crates/router/src/core/payments.rs

@@ -122,7 +122,6 @@ where
 
     let updated_customer = call_create_connector_customer_if_required(
         state,
-        &payment_data.payment_attempt.connector.clone(),
         &customer,
         &merchant_account,
         &mut payment_data,
@@ -639,7 +638,6 @@ where
 
 pub async fn call_create_connector_customer_if_required<F, Req>(
     state: &AppState,
-    connector_name: &Option<String>,
     customer: &Option<domain::Customer>,
     merchant_account: &domain::MerchantAccount,
     payment_data: &mut PaymentData<F>,
@@ -658,17 +656,32 @@ where
     // To perform router related operation for PaymentResponse
     PaymentResponse: Operation<F, Req>,
 {
+    let connector_name = payment_data.payment_attempt.connector.clone();
+
     match connector_name {
         Some(connector_name) => {
             let connector = api::ConnectorData::get_connector_by_name(
                 &state.conf.connectors,
-                connector_name,
+                &connector_name,
                 api::GetToken::Connector,
             )?;
-            let (is_eligible, connector_customer_id, connector_customer_map) =
-                customers::should_call_connector_create_customer(state, &connector, customer)?;
 
-            if is_eligible {
+            let connector_label = helpers::get_connector_label(
+                payment_data.payment_intent.business_country,
+                &payment_data.payment_intent.business_label,
+                payment_data.payment_attempt.business_sub_label.as_ref(),
+                &connector_name,
+            );
+
+            let (should_call_connector, existing_connector_customer_id) =
+                customers::should_call_connector_create_customer(
+                    state,
+                    &connector,
+                    customer,
+                    &connector_label,
+                );
+
+            if should_call_connector {
                 // Create customer at connector and update the customer table to store this data
                 let router_data = payment_data
                     .construct_router_data(
@@ -679,15 +692,23 @@ where
                     )
                     .await?;
 
-                let (connector_customer, customer_update) = router_data
+                let connector_customer_id = router_data
-                    .create_connector_customer(state, &connector, connector_customer_map)
+                    .create_connector_customer(state, &connector)
                     .await?;
 
-                payment_data.connector_customer_id = connector_customer;
+                let customer_update = customers::update_connector_customer_in_customers(
+                    &connector_label,
+                    customer.as_ref(),
+                    &connector_customer_id,
+                )
+                .await;
+
+                payment_data.connector_customer_id = connector_customer_id;
                 Ok(customer_update)
             } else {
                 // Customer already created in previous calls use the same value, no need to update
-                payment_data.connector_customer_id = connector_customer_id;
+                payment_data.connector_customer_id =
+                    existing_connector_customer_id.map(ToOwned::to_owned);
                 Ok(None)
             }
         }

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

@@ -1,9 +1,8 @@
-use common_utils::ext_traits::ValueExt;
+use router_env::{instrument, tracing};
-use error_stack::{self, ResultExt};
 
 use crate::{
     core::{
-        errors::{self, ConnectorErrorExt, RouterResult},
+        errors::{ConnectorErrorExt, RouterResult},
         payments,
     },
     logger,
@@ -12,13 +11,13 @@ use crate::{
     types::{self, api, domain, storage},
 };
 
+#[instrument(skip_all)]
 pub async fn create_connector_customer<F: Clone, T: Clone>(
     state: &AppState,
     connector: &api::ConnectorData,
     router_data: &types::RouterData<F, T, types::PaymentsResponseData>,
     customer_request_data: types::ConnectorCustomerData,
-    connector_customer_map: Option<serde_json::Map<String, serde_json::Value>>,
+) -> RouterResult<Option<String>> {
-) -> RouterResult<(Option<String>, Option<storage::CustomerUpdate>)> {
     let connector_integration: services::BoxedConnectorIntegration<
         '_,
         api::CreateConnectorCustomer,
@@ -68,79 +67,75 @@ pub async fn create_connector_customer<F: Clone, T: Clone>(
             _ => None,
         },
         Err(err) => {
-            logger::debug!(payment_method_tokenization_error=?err);
+            logger::error!(create_connector_customer_error=?err);
             None
         }
     };
 
-    let update_customer = update_connector_customer_in_customers(
+    Ok(connector_customer_id)
-        connector,
-        connector_customer_map,
-        &connector_customer_id,
-    )
-    .await?;
-    Ok((connector_customer_id, update_customer))
 }
 
-type CreateCustomerCheck = (
+pub fn get_connector_customer_details_if_present<'a>(
-    bool,
+    customer: &'a domain::Customer,
-    Option<String>,
+    connector_name: &str,
-    Option<serde_json::Map<String, serde_json::Value>>,
+) -> Option<&'a str> {
-);
+    customer
-pub fn should_call_connector_create_customer(
+        .connector_customer
+        .as_ref()
+        .and_then(|connector_customer_value| connector_customer_value.get(connector_name))
+        .and_then(|connector_customer| connector_customer.as_str())
+}
+
+pub fn should_call_connector_create_customer<'a>(
     state: &AppState,
     connector: &api::ConnectorData,
-    customer: &Option<domain::Customer>,
+    customer: &'a Option<domain::Customer>,
-) -> RouterResult<CreateCustomerCheck> {
+    connector_label: &str,
-    let connector_name = connector.connector_name.to_string();
+) -> (bool, Option<&'a str>) {
     // Check if create customer is required for the connector
-    let connector_customer_filter = state
+    let connector_needs_customer = state
         .conf
         .connector_customer
         .connector_list
         .contains(&connector.connector_name);
 
-    if connector_customer_filter {
+    if connector_needs_customer {
-        match customer {
+        let connector_customer_details = customer.as_ref().and_then(|customer| {
-            Some(customer) => match &customer.connector_customer {
+            get_connector_customer_details_if_present(customer, connector_label)
-                Some(connector_customer) => {
-                    let connector_customer_map: serde_json::Map<String, serde_json::Value> =
-                        connector_customer
-                            .clone()
-                            .parse_value("Map<String, Value>")
-                            .change_context(errors::ApiErrorResponse::InternalServerError)
-                            .attach_printable("Failed to deserialize Value to CustomerConnector")?;
-                    let value = connector_customer_map.get(&connector_name); //Check if customer already created for this customer and for this connector
-                    Ok((
-                        value.is_none(),
-                        value.and_then(|val| val.as_str().map(|cust| cust.to_string())),
-                        Some(connector_customer_map),
-                    ))
-                }
-                None => Ok((true, None, None)),
-            },
-            None => Ok((false, None, None)),
-        }
-    } else {
-        Ok((false, None, None))
-    }
-}
-pub async fn update_connector_customer_in_customers(
-    connector: &api::ConnectorData,
-    connector_customer_map: Option<serde_json::Map<String, serde_json::Value>>,
-    connector_cust_id: &Option<String>,
-) -> RouterResult<Option<storage::CustomerUpdate>> {
-    let mut connector_customer = match connector_customer_map {
-        Some(cc) => cc,
-        None => serde_json::Map::new(),
-    };
-    connector_cust_id.clone().map(|cc| {
-        connector_customer.insert(
-            connector.connector_name.to_string(),
-            serde_json::Value::String(cc),
-        )
         });
-    Ok(Some(storage::CustomerUpdate::ConnectorCustomer {
+        let should_call_connector = connector_customer_details.is_none();
-        connector_customer: Some(serde_json::Value::Object(connector_customer)),
+        (should_call_connector, connector_customer_details)
-    }))
+    } else {
+        (false, None)
+    }
+}
+
+#[instrument]
+pub async fn update_connector_customer_in_customers(
+    connector_label: &str,
+    customer: Option<&domain::Customer>,
+    connector_customer_id: &Option<String>,
+) -> Option<storage::CustomerUpdate> {
+    let connector_customer_map = customer
+        .and_then(|customer| customer.connector_customer.as_ref())
+        .and_then(|connector_customer| connector_customer.as_object())
+        .map(ToOwned::to_owned)
+        .unwrap_or(serde_json::Map::new());
+
+    let updated_connector_customer_map =
+        connector_customer_id.as_ref().map(|connector_customer_id| {
+            let mut connector_customer_map = connector_customer_map;
+            let connector_customer_value =
+                serde_json::Value::String(connector_customer_id.to_string());
+            connector_customer_map.insert(connector_label.to_string(), connector_customer_value);
+            connector_customer_map
+        });
+
+    updated_connector_customer_map
+        .map(serde_json::Value::Object)
+        .map(
+            |connector_customer_value| storage::CustomerUpdate::ConnectorCustomer {
+                connector_customer: Some(connector_customer_value),
+            },
+        )
 }

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

@@ -16,7 +16,7 @@ use crate::{
     },
     routes::AppState,
     services,
-    types::{self, api, domain, storage},
+    types::{self, api, domain},
 };
 
 #[async_trait]
@@ -87,14 +87,13 @@ pub trait Feature<F, T> {
         &self,
         _state: &AppState,
         _connector: &api::ConnectorData,
-        _connector_customer_map: Option<serde_json::Map<String, serde_json::Value>>,
+    ) -> RouterResult<Option<String>>
-    ) -> RouterResult<(Option<String>, Option<storage::CustomerUpdate>)>
     where
         F: Clone,
         Self: Sized,
         dyn api::Connector: services::ConnectorIntegration<F, T, types::PaymentsResponseData>,
     {
-        Ok((None, None))
+        Ok(None)
     }
 }
 

crates/router/src/core/payments/flows/authorize_flow.rs

@@ -11,7 +11,7 @@ use crate::{
     logger,
     routes::{metrics, AppState},
     services,
-    types::{self, api, domain, storage},
+    types::{self, api, domain},
 };
 
 #[async_trait]
@@ -108,14 +108,12 @@ impl Feature<api::Authorize, types::PaymentsAuthorizeData> for types::PaymentsAu
         &self,
         state: &AppState,
         connector: &api::ConnectorData,
-        connector_customer_map: Option<serde_json::Map<String, serde_json::Value>>,
+    ) -> RouterResult<Option<String>> {
-    ) -> RouterResult<(Option<String>, Option<storage::CustomerUpdate>)> {
         customers::create_connector_customer(
             state,
             connector,
             self,
             types::ConnectorCustomerData::try_from(self)?,
-            connector_customer_map,
         )
         .await
     }

crates/router/src/core/payments/flows/verfiy_flow.rs

@@ -9,7 +9,7 @@ use crate::{
     },
     routes::AppState,
     services,
-    types::{self, api, domain, storage},
+    types::{self, api, domain},
 };
 
 #[async_trait]
@@ -84,14 +84,12 @@ impl Feature<api::Verify, types::VerifyRequestData> for types::VerifyRouterData
         &self,
         state: &AppState,
         connector: &api::ConnectorData,
-        connector_customer_map: Option<serde_json::Map<String, serde_json::Value>>,
+    ) -> RouterResult<Option<String>> {
-    ) -> RouterResult<(Option<String>, Option<storage::CustomerUpdate>)> {
         customers::create_connector_customer(
             state,
             connector,
             self,
             types::ConnectorCustomerData::try_from(self.request.to_owned())?,
-            connector_customer_map,
         )
         .await
     }