From 83a192466849c5fd201296e7554644a025ced888 Mon Sep 17 00:00:00 2001 From: Narayan Bhat <48803246+Narayanbhat166@users.noreply.github.com> Date: Fri, 3 May 2024 13:50:17 +0530 Subject: [PATCH] fix(api_request): make `payment_method_data` as optional (#4527) Co-authored-by: hyperswitch-bot[bot] <148525504+hyperswitch-bot[bot]@users.noreply.github.com> --- crates/api_models/src/payments.rs | 91 ++++++++++++++----- .../stripe/payment_intents/types.rs | 4 +- .../stripe/setup_intents/types.rs | 4 +- crates/router/src/core/payments/helpers.rs | 38 +++++--- .../operations/payment_complete_authorize.rs | 4 +- .../payments/operations/payment_confirm.rs | 13 +-- .../payments/operations/payment_create.rs | 45 ++++++--- .../payments/operations/payment_update.rs | 6 +- crates/router/src/types/api/payments.rs | 2 +- crates/router/tests/payments.rs | 8 +- crates/router/tests/payments2.rs | 8 +- openapi/openapi_spec.json | 7 +- 12 files changed, 160 insertions(+), 70 deletions(-) diff --git a/crates/api_models/src/payments.rs b/crates/api_models/src/payments.rs index 3385b63369..b39355d2c9 100644 --- a/crates/api_models/src/payments.rs +++ b/crates/api_models/src/payments.rs @@ -1288,15 +1288,56 @@ mod payment_method_data_serde { OptionalPaymentMethod(serde_json::Value), } + // This struct is an intermediate representation + // This is required in order to catch deserialization errors when deserializing `payment_method_data` + // The #[serde(flatten)] attribute applied on `payment_method_data` discards + // any of the error when deserializing and deserializes to an option instead + #[derive(serde::Deserialize, Debug)] + struct __InnerPaymentMethodData { + billing: Option
, + #[serde(flatten)] + payment_method_data: Option, + } + let deserialize_to_inner = __Inner::deserialize(deserializer)?; + match deserialize_to_inner { __Inner::OptionalPaymentMethod(value) => { - let parsed_value = serde_json::from_value::(value) + let parsed_value = serde_json::from_value::<__InnerPaymentMethodData>(value) .map_err(|serde_json_error| { serde::de::Error::custom(serde_json_error.to_string()) })?; - Ok(Some(parsed_value)) + let payment_method_data = if let Some(payment_method_data_value) = + parsed_value.payment_method_data + { + // Even though no data is passed, the flatten serde_json::Value is deserialized as Some(Object {}) + if let serde_json::Value::Object(ref inner_map) = payment_method_data_value { + if inner_map.is_empty() { + None + } else { + Some( + serde_json::from_value::( + payment_method_data_value, + ) + .map_err(|serde_json_error| { + serde::de::Error::custom(serde_json_error.to_string()) + })?, + ) + } + } else { + Err(serde::de::Error::custom( + "Expected a map for payment_method_data", + ))? + } + } else { + None + }; + + Ok(Some(PaymentMethodDataRequest { + payment_method_data, + billing: parsed_value.billing, + })) } __Inner::RewardString(inner_string) => { let payment_method_data = match inner_string.as_str() { @@ -1305,7 +1346,7 @@ mod payment_method_data_serde { }; Ok(Some(PaymentMethodDataRequest { - payment_method_data, + payment_method_data: Some(payment_method_data), billing: None, })) } @@ -1320,21 +1361,29 @@ mod payment_method_data_serde { S: Serializer, { if let Some(payment_method_data_request) = payment_method_data_request { - match payment_method_data_request.payment_method_data { - PaymentMethodData::Reward => serializer.serialize_str("reward"), - PaymentMethodData::CardRedirect(_) - | PaymentMethodData::BankDebit(_) - | PaymentMethodData::BankRedirect(_) - | PaymentMethodData::BankTransfer(_) - | PaymentMethodData::CardToken(_) - | PaymentMethodData::Crypto(_) - | PaymentMethodData::GiftCard(_) - | PaymentMethodData::PayLater(_) - | PaymentMethodData::Upi(_) - | PaymentMethodData::Voucher(_) - | PaymentMethodData::Card(_) - | PaymentMethodData::MandatePayment - | PaymentMethodData::Wallet(_) => payment_method_data_request.serialize(serializer), + if let Some(payment_method_data) = + payment_method_data_request.payment_method_data.as_ref() + { + match payment_method_data { + PaymentMethodData::Reward => serializer.serialize_str("reward"), + PaymentMethodData::CardRedirect(_) + | PaymentMethodData::BankDebit(_) + | PaymentMethodData::BankRedirect(_) + | PaymentMethodData::BankTransfer(_) + | PaymentMethodData::CardToken(_) + | PaymentMethodData::Crypto(_) + | PaymentMethodData::GiftCard(_) + | PaymentMethodData::PayLater(_) + | PaymentMethodData::Upi(_) + | PaymentMethodData::Voucher(_) + | PaymentMethodData::Card(_) + | PaymentMethodData::MandatePayment + | PaymentMethodData::Wallet(_) => { + payment_method_data_request.serialize(serializer) + } + } + } else { + payment_method_data_request.serialize(serializer) } } else { serializer.serialize_none() @@ -1345,7 +1394,7 @@ mod payment_method_data_serde { #[derive(Debug, Clone, serde::Deserialize, serde::Serialize, ToSchema, Eq, PartialEq)] pub struct PaymentMethodDataRequest { #[serde(flatten)] - pub payment_method_data: PaymentMethodData, + pub payment_method_data: Option, /// billing details for the payment method. /// This billing details will be passed to the processor as billing address. /// If not passed, then payment.billing will be considered @@ -4715,7 +4764,7 @@ mod payments_request_api_contract { let payments_request = serde_json::from_str::(payments_request); assert!(payments_request.is_ok()); - if let PaymentMethodData::Card(card_data) = payments_request + if let Some(PaymentMethodData::Card(card_data)) = payments_request .unwrap() .payment_method_data .unwrap() @@ -4747,7 +4796,7 @@ mod payments_request_api_contract { .payment_method_data .unwrap() .payment_method_data, - PaymentMethodData::Reward + Some(PaymentMethodData::Reward) ); } } diff --git a/crates/router/src/compatibility/stripe/payment_intents/types.rs b/crates/router/src/compatibility/stripe/payment_intents/types.rs index 404d0e93db..eed80a1128 100644 --- a/crates/router/src/compatibility/stripe/payment_intents/types.rs +++ b/crates/router/src/compatibility/stripe/payment_intents/types.rs @@ -335,7 +335,9 @@ impl TryFrom for payments::PaymentsRequest { pmd.payment_method_details .as_ref() .map(|spmd| payments::PaymentMethodDataRequest { - payment_method_data: payments::PaymentMethodData::from(spmd.to_owned()), + payment_method_data: Some(payments::PaymentMethodData::from( + spmd.to_owned(), + )), billing: pmd.billing_details.clone().map(payments::Address::from), }) }), diff --git a/crates/router/src/compatibility/stripe/setup_intents/types.rs b/crates/router/src/compatibility/stripe/setup_intents/types.rs index d29bf235b5..03335d4272 100644 --- a/crates/router/src/compatibility/stripe/setup_intents/types.rs +++ b/crates/router/src/compatibility/stripe/setup_intents/types.rs @@ -246,7 +246,9 @@ impl TryFrom for payments::PaymentsRequest { pmd.payment_method_details .as_ref() .map(|spmd| payments::PaymentMethodDataRequest { - payment_method_data: payments::PaymentMethodData::from(spmd.to_owned()), + payment_method_data: Some(payments::PaymentMethodData::from( + spmd.to_owned(), + )), billing: pmd.billing_details.clone().map(payments::Address::from), }) }), diff --git a/crates/router/src/core/payments/helpers.rs b/crates/router/src/core/payments/helpers.rs index be04d2fb1a..702b63b5c0 100644 --- a/crates/router/src/core/payments/helpers.rs +++ b/crates/router/src/core/payments/helpers.rs @@ -1159,7 +1159,7 @@ pub fn verify_mandate_details_for_recurring_payments( #[instrument(skip_all)] pub fn payment_attempt_status_fsm( - payment_method_data: &Option, + payment_method_data: Option<&api::payments::PaymentMethodData>, confirm: Option, ) -> storage_enums::AttemptStatus { match payment_method_data { @@ -1172,7 +1172,7 @@ pub fn payment_attempt_status_fsm( } pub fn payment_intent_status_fsm( - payment_method_data: &Option, + payment_method_data: Option<&api::PaymentMethodData>, confirm: Option, ) -> storage_enums::IntentStatus { match payment_method_data { @@ -2129,8 +2129,14 @@ pub(crate) fn validate_amount_to_capture( pub(crate) fn validate_payment_method_fields_present( req: &api::PaymentsRequest, ) -> RouterResult<()> { + let payment_method_data = + req.payment_method_data + .as_ref() + .and_then(|request_payment_method_data| { + request_payment_method_data.payment_method_data.as_ref() + }); utils::when( - req.payment_method.is_none() && req.payment_method_data.is_some(), + req.payment_method.is_none() && payment_method_data.is_some(), || { Err(errors::ApiErrorResponse::MissingRequiredField { field_name: "payment_method", @@ -2152,7 +2158,7 @@ pub(crate) fn validate_payment_method_fields_present( utils::when( req.payment_method.is_some() - && req.payment_method_data.is_none() + && payment_method_data.is_none() && req.payment_token.is_none() && req.recurring_details.is_none(), || { @@ -2194,14 +2200,14 @@ pub(crate) fn validate_payment_method_fields_present( }; utils::when( - req.payment_method.is_some() && req.payment_method_data.is_some(), + req.payment_method.is_some() && payment_method_data.is_some(), || { - req.payment_method_data - .clone() - .map_or(Ok(()), |req_payment_method_data| { + payment_method_data + .cloned() + .map_or(Ok(()), |payment_method_data| { req.payment_method.map_or(Ok(()), |req_payment_method| { validate_payment_method_and_payment_method_data( - req_payment_method_data.payment_method_data, + payment_method_data, req_payment_method, ) }) @@ -3409,7 +3415,7 @@ impl AttemptType { // In case if fields are not overridden by the request then they contain the same data that was in the previous attempt provided it is populated in this function. #[inline(always)] fn make_new_payment_attempt( - payment_method_data: &Option, + payment_method_data: Option<&api_models::payments::PaymentMethodData>, old_payment_attempt: PaymentAttempt, new_attempt_count: i16, storage_scheme: enums::MerchantStorageScheme, @@ -3507,7 +3513,11 @@ impl AttemptType { let new_payment_attempt = db .insert_payment_attempt( Self::make_new_payment_attempt( - &request.payment_method_data, + request.payment_method_data.as_ref().and_then( + |request_payment_method_data| { + request_payment_method_data.payment_method_data.as_ref() + }, + ), fetched_payment_attempt, new_attempt_count, storage_scheme, @@ -3524,7 +3534,11 @@ impl AttemptType { fetched_payment_intent, storage::PaymentIntentUpdate::StatusAndAttemptUpdate { status: payment_intent_status_fsm( - &request.payment_method_data, + request.payment_method_data.as_ref().and_then( + |request_payment_method_data| { + request_payment_method_data.payment_method_data.as_ref() + }, + ), Some(true), ), active_attempt_id: new_payment_attempt.attempt_id.clone(), diff --git a/crates/router/src/core/payments/operations/payment_complete_authorize.rs b/crates/router/src/core/payments/operations/payment_complete_authorize.rs index d4dc52ce27..d31ec14a72 100644 --- a/crates/router/src/core/payments/operations/payment_complete_authorize.rs +++ b/crates/router/src/core/payments/operations/payment_complete_authorize.rs @@ -138,7 +138,7 @@ impl &request .payment_method_data .as_ref() - .map(|pmd| pmd.payment_method_data.clone()), + .and_then(|pmd| pmd.payment_method_data.clone()), &request.payment_method_type, &mandate_type, &token, @@ -284,7 +284,7 @@ impl payment_method_data: request .payment_method_data .as_ref() - .map(|pmd| pmd.payment_method_data.clone()), + .and_then(|pmd| pmd.payment_method_data.clone()), payment_method_info, force_sync: None, refunds: vec![], diff --git a/crates/router/src/core/payments/operations/payment_confirm.rs b/crates/router/src/core/payments/operations/payment_confirm.rs index 79680cb80c..125f79956a 100644 --- a/crates/router/src/core/payments/operations/payment_confirm.rs +++ b/crates/router/src/core/payments/operations/payment_confirm.rs @@ -341,7 +341,7 @@ impl request .payment_method_data .as_ref() - .map(|pmd| pmd.payment_method_data.clone()), + .and_then(|pmd| pmd.payment_method_data.clone()), )?; payment_attempt.browser_info = browser_info; @@ -412,7 +412,7 @@ impl let n_request_payment_method_data = request .payment_method_data .as_ref() - .map(|pmd| pmd.payment_method_data.clone()); + .and_then(|pmd| pmd.payment_method_data.clone()); let store = state.clone().store; @@ -524,7 +524,7 @@ impl &request .payment_method_data .as_ref() - .map(|pmd| pmd.payment_method_data.clone()), + .and_then(|pmd| pmd.payment_method_data.clone()), &request.payment_method_type, &mandate_type, &token, @@ -570,11 +570,12 @@ impl let payment_method_data_after_card_bin_call = request .payment_method_data .as_ref() + .and_then(|request_payment_method_data| { + request_payment_method_data.payment_method_data.as_ref() + }) .zip(additional_pm_data) .map(|(payment_method_data, additional_payment_data)| { - payment_method_data - .payment_method_data - .apply_additional_payment_data(additional_payment_data) + payment_method_data.apply_additional_payment_data(additional_payment_data) }); let authentication = payment_attempt.authentication_id.as_ref().async_map(|authentication_id| async move { state diff --git a/crates/router/src/core/payments/operations/payment_create.rs b/crates/router/src/core/payments/operations/payment_create.rs index 78162fbfa6..aaebccd880 100644 --- a/crates/router/src/core/payments/operations/payment_create.rs +++ b/crates/router/src/core/payments/operations/payment_create.rs @@ -396,11 +396,14 @@ impl let payment_method_data_after_card_bin_call = request .payment_method_data .as_ref() + .and_then(|payment_method_data_from_request| { + payment_method_data_from_request + .payment_method_data + .as_ref() + }) .zip(additional_payment_data) .map(|(payment_method_data, additional_payment_data)| { - payment_method_data - .payment_method_data - .apply_additional_payment_data(additional_payment_data) + payment_method_data.apply_additional_payment_data(additional_payment_data) }); let amount = payment_attempt.get_total_amount().into(); @@ -702,7 +705,7 @@ impl ValidateRequest ValidateRequest, )> { + let payment_method_data = + request + .payment_method_data + .as_ref() + .and_then(|payment_method_data_request| { + payment_method_data_request.payment_method_data.as_ref() + }); + let created_at @ modified_at @ last_synced = Some(common_utils::date_time::now()); - let status = - helpers::payment_attempt_status_fsm(&request.payment_method_data, request.confirm); + let status = helpers::payment_attempt_status_fsm(payment_method_data, request.confirm); let (amount, currency) = (money.0, Some(money.1)); let mut additional_pm_data = request .payment_method_data .as_ref() + .and_then(|payment_method_data_request| { + payment_method_data_request.payment_method_data.as_ref() + }) .async_map(|payment_method_data| async { - helpers::get_additional_payment_data( - &payment_method_data.payment_method_data, - &*state.store, - ) - .await + helpers::get_additional_payment_data(payment_method_data, &*state.store).await }) .await; @@ -944,8 +953,16 @@ impl PaymentCreate { session_expiry: PrimitiveDateTime, ) -> RouterResult { let created_at @ modified_at @ last_synced = Some(common_utils::date_time::now()); - let status = - helpers::payment_intent_status_fsm(&request.payment_method_data, request.confirm); + + let status = helpers::payment_intent_status_fsm( + request + .payment_method_data + .as_ref() + .and_then(|request_payment_method_data| { + request_payment_method_data.payment_method_data.as_ref() + }), + request.confirm, + ); let client_secret = crate::utils::generate_id(consts::ID_LENGTH, format!("{payment_id}_secret").as_str()); let (amount, currency) = (money.0, Some(money.1)); diff --git a/crates/router/src/core/payments/operations/payment_update.rs b/crates/router/src/core/payments/operations/payment_update.rs index ecf1fe998b..804d1dbc80 100644 --- a/crates/router/src/core/payments/operations/payment_update.rs +++ b/crates/router/src/core/payments/operations/payment_update.rs @@ -81,7 +81,7 @@ impl request .payment_method_data .as_ref() - .map(|pmd| pmd.payment_method_data.clone()), + .and_then(|pmd| pmd.payment_method_data.clone()), )?; helpers::validate_payment_status_against_not_allowed_statuses( @@ -265,7 +265,7 @@ impl &request .payment_method_data .as_ref() - .map(|pmd| pmd.payment_method_data.clone()), + .and_then(|pmd| pmd.payment_method_data.clone()), &request.payment_method_type, &mandate_type, &token, @@ -430,7 +430,7 @@ impl payment_method_data: request .payment_method_data .as_ref() - .map(|pmd| pmd.payment_method_data.clone()), + .and_then(|pmd| pmd.payment_method_data.clone()), payment_method_info, force_sync: None, refunds: vec![], diff --git a/crates/router/src/types/api/payments.rs b/crates/router/src/types/api/payments.rs index 4fc3fb4452..e36c68c27d 100644 --- a/crates/router/src/types/api/payments.rs +++ b/crates/router/src/types/api/payments.rs @@ -261,7 +261,7 @@ mod payments_test { PaymentsRequest { amount: Some(Amount::from(200)), payment_method_data: Some(PaymentMethodDataRequest { - payment_method_data: PaymentMethodData::Card(card()), + payment_method_data: Some(PaymentMethodData::Card(card())), billing: None, }), ..PaymentsRequest::default() diff --git a/crates/router/tests/payments.rs b/crates/router/tests/payments.rs index 7a990464da..646a11d4cd 100644 --- a/crates/router/tests/payments.rs +++ b/crates/router/tests/payments.rs @@ -317,7 +317,7 @@ async fn payments_create_core() { setup_future_usage: Some(api_enums::FutureUsage::OnSession), authentication_type: Some(api_enums::AuthenticationType::NoThreeDs), payment_method_data: Some(api::PaymentMethodDataRequest { - payment_method_data: api::PaymentMethodData::Card(api::Card { + payment_method_data: Some(api::PaymentMethodData::Card(api::Card { card_number: "4242424242424242".to_string().try_into().unwrap(), card_exp_month: "10".to_string().into(), card_exp_year: "35".to_string().into(), @@ -329,7 +329,7 @@ async fn payments_create_core() { card_issuing_country: None, bank_code: None, nick_name: Some(masking::Secret::new("nick_name".into())), - }), + })), billing: None, }), payment_method: Some(api_enums::PaymentMethod::Card), @@ -499,7 +499,7 @@ async fn payments_create_core_adyen_no_redirect() { setup_future_usage: Some(api_enums::FutureUsage::OnSession), authentication_type: Some(api_enums::AuthenticationType::NoThreeDs), payment_method_data: Some(api::PaymentMethodDataRequest { - payment_method_data: api::PaymentMethodData::Card(api::Card { + payment_method_data: Some(api::PaymentMethodData::Card(api::Card { card_number: "5555 3412 4444 1115".to_string().try_into().unwrap(), card_exp_month: "03".to_string().into(), card_exp_year: "2030".to_string().into(), @@ -511,7 +511,7 @@ async fn payments_create_core_adyen_no_redirect() { card_issuing_country: None, bank_code: None, nick_name: Some(masking::Secret::new("nick_name".into())), - }), + })), billing: None, }), payment_method: Some(api_enums::PaymentMethod::Card), diff --git a/crates/router/tests/payments2.rs b/crates/router/tests/payments2.rs index cce589fb03..9b622d11fd 100644 --- a/crates/router/tests/payments2.rs +++ b/crates/router/tests/payments2.rs @@ -77,7 +77,7 @@ async fn payments_create_core() { setup_future_usage: None, authentication_type: Some(api_enums::AuthenticationType::NoThreeDs), payment_method_data: Some(api::PaymentMethodDataRequest { - payment_method_data: api::PaymentMethodData::Card(api::Card { + payment_method_data: Some(api::PaymentMethodData::Card(api::Card { card_number: "4242424242424242".to_string().try_into().unwrap(), card_exp_month: "10".to_string().into(), card_exp_year: "35".to_string().into(), @@ -89,7 +89,7 @@ async fn payments_create_core() { card_issuing_country: None, bank_code: None, nick_name: Some(masking::Secret::new("nick_name".into())), - }), + })), billing: None, }), payment_method: Some(api_enums::PaymentMethod::Card), @@ -266,7 +266,7 @@ async fn payments_create_core_adyen_no_redirect() { setup_future_usage: Some(api_enums::FutureUsage::OffSession), authentication_type: Some(api_enums::AuthenticationType::NoThreeDs), payment_method_data: Some(api::PaymentMethodDataRequest { - payment_method_data: api::PaymentMethodData::Card(api::Card { + payment_method_data: Some(api::PaymentMethodData::Card(api::Card { card_number: "5555 3412 4444 1115".to_string().try_into().unwrap(), card_exp_month: "03".to_string().into(), card_exp_year: "2030".to_string().into(), @@ -278,7 +278,7 @@ async fn payments_create_core_adyen_no_redirect() { card_type: None, card_issuing_country: None, nick_name: Some(masking::Secret::new("nick_name".into())), - }), + })), billing: None, }), diff --git a/openapi/openapi_spec.json b/openapi/openapi_spec.json index d795387853..d278c50a38 100644 --- a/openapi/openapi_spec.json +++ b/openapi/openapi_spec.json @@ -12787,7 +12787,12 @@ "PaymentMethodDataRequest": { "allOf": [ { - "$ref": "#/components/schemas/PaymentMethodData" + "allOf": [ + { + "$ref": "#/components/schemas/PaymentMethodData" + } + ], + "nullable": true }, { "type": "object",