Skip to content

Commit cfabfa6

Browse files
dracarys18dracarys18
authored
fix: api lock on PaymentsCreate (#2916)
1 parent 922dc90 commit cfabfa6

File tree

7 files changed

+87
-62
lines changed

7 files changed

+87
-62
lines changed

‎crates/api_models/src/payments.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1679,6 +1679,20 @@ impl std::fmt::Display for PaymentIdType {
16791679
}
16801680
}
16811681

1682+
impl PaymentIdType {
1683+
pub fn and_then<F, E>(self, f: F) -> Result<Self, E>
1684+
where
1685+
F: FnOnce(String) -> Result<String, E>,
1686+
{
1687+
match self {
1688+
Self::PaymentIntentId(s) => f(s).map(Self::PaymentIntentId),
1689+
Self::ConnectorTransactionId(s) => f(s).map(Self::ConnectorTransactionId),
1690+
Self::PaymentAttemptId(s) => f(s).map(Self::PaymentAttemptId),
1691+
Self::PreprocessingId(s) => f(s).map(Self::PreprocessingId),
1692+
}
1693+
}
1694+
}
1695+
16821696
impl Default for PaymentIdType {
16831697
fn default() -> Self {
16841698
Self::PaymentIntentId(Default::default())

‎crates/router/src/core/payments/operations/payment_approve.rs

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use std::marker::PhantomData;
33
use api_models::enums::FrmSuggestion;
44
use async_trait::async_trait;
55
use data_models::mandates::MandateData;
6-
use error_stack::ResultExt;
6+
use error_stack::{report, IntoReport, ResultExt};
77
use router_derive::PaymentOperation;
88
use router_env::{instrument, tracing};
99

@@ -399,15 +399,6 @@ impl<F: Send + Clone, Ctx: PaymentMethodRetrieve> ValidateRequest<F, api::Paymen
399399
BoxedOperation<'b, F, api::PaymentsRequest, Ctx>,
400400
operations::ValidateResult<'a>,
401401
)> {
402-
let given_payment_id = match &request.payment_id {
403-
Some(id_type) => Some(
404-
id_type
405-
.get_payment_intent_id()
406-
.change_context(errors::ApiErrorResponse::PaymentNotFound)?,
407-
),
408-
None => None,
409-
};
410-
411402
let request_merchant_id = request.merchant_id.as_deref();
412403
helpers::validate_merchant_id(&merchant_account.merchant_id, request_merchant_id)
413404
.change_context(errors::ApiErrorResponse::InvalidDataFormat {
@@ -419,13 +410,18 @@ impl<F: Send + Clone, Ctx: PaymentMethodRetrieve> ValidateRequest<F, api::Paymen
419410

420411
let mandate_type =
421412
helpers::validate_mandate(request, payments::is_operation_confirm(self))?;
422-
let payment_id = core_utils::get_or_generate_id("payment_id", &given_payment_id, "pay")?;
413+
let payment_id = request
414+
.payment_id
415+
.clone()
416+
.ok_or(report!(errors::ApiErrorResponse::PaymentNotFound))?;
423417

424418
Ok((
425419
Box::new(self),
426420
operations::ValidateResult {
427421
merchant_id: &merchant_account.merchant_id,
428-
payment_id: api::PaymentIdType::PaymentIntentId(payment_id),
422+
payment_id: payment_id
423+
.and_then(|id| core_utils::validate_id(id, "payment_id"))
424+
.into_report()?,
429425
mandate_type,
430426
storage_scheme: merchant_account.storage_scheme,
431427
requeue: matches!(

‎crates/router/src/core/payments/operations/payment_complete_authorize.rs

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use std::marker::PhantomData;
22

33
use api_models::enums::FrmSuggestion;
44
use async_trait::async_trait;
5-
use error_stack::ResultExt;
5+
use error_stack::{report, IntoReport, ResultExt};
66
use router_derive::PaymentOperation;
77
use router_env::{instrument, tracing};
88

@@ -374,14 +374,10 @@ impl<F: Send + Clone, Ctx: PaymentMethodRetrieve> ValidateRequest<F, api::Paymen
374374
BoxedOperation<'b, F, api::PaymentsRequest, Ctx>,
375375
operations::ValidateResult<'a>,
376376
)> {
377-
let given_payment_id = match &request.payment_id {
378-
Some(id_type) => Some(
379-
id_type
380-
.get_payment_intent_id()
381-
.change_context(errors::ApiErrorResponse::PaymentNotFound)?,
382-
),
383-
None => None,
384-
};
377+
let payment_id = request
378+
.payment_id
379+
.clone()
380+
.ok_or(report!(errors::ApiErrorResponse::PaymentNotFound))?;
385381

386382
let request_merchant_id = request.merchant_id.as_deref();
387383
helpers::validate_merchant_id(&merchant_account.merchant_id, request_merchant_id)
@@ -394,13 +390,14 @@ impl<F: Send + Clone, Ctx: PaymentMethodRetrieve> ValidateRequest<F, api::Paymen
394390

395391
let mandate_type =
396392
helpers::validate_mandate(request, payments::is_operation_confirm(self))?;
397-
let payment_id = core_utils::get_or_generate_id("payment_id", &given_payment_id, "pay")?;
398393

399394
Ok((
400395
Box::new(self),
401396
operations::ValidateResult {
402397
merchant_id: &merchant_account.merchant_id,
403-
payment_id: api::PaymentIdType::PaymentIntentId(payment_id),
398+
payment_id: payment_id
399+
.and_then(|id| core_utils::validate_id(id, "payment_id"))
400+
.into_report()?,
404401
mandate_type,
405402
storage_scheme: merchant_account.storage_scheme,
406403
requeue: matches!(

‎crates/router/src/core/payments/operations/payment_confirm.rs

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use api_models::{
66
};
77
use async_trait::async_trait;
88
use common_utils::ext_traits::{AsyncExt, Encode};
9-
use error_stack::ResultExt;
9+
use error_stack::{report, IntoReport, ResultExt};
1010
use futures::FutureExt;
1111
use redis_interface::errors::RedisError;
1212
use router_derive::PaymentOperation;
@@ -19,7 +19,7 @@ use crate::{
1919
errors::{self, CustomResult, RouterResult, StorageErrorExt},
2020
payment_methods::PaymentMethodRetrieve,
2121
payments::{self, helpers, operations, CustomerDetails, PaymentAddress, PaymentData},
22-
utils::get_individual_surcharge_detail_from_redis,
22+
utils::{self as core_utils, get_individual_surcharge_detail_from_redis},
2323
},
2424
db::StorageInterface,
2525
routes::AppState,
@@ -820,14 +820,6 @@ impl<F: Send + Clone, Ctx: PaymentMethodRetrieve> ValidateRequest<F, api::Paymen
820820
operations::ValidateResult<'a>,
821821
)> {
822822
helpers::validate_customer_details_in_request(request)?;
823-
let given_payment_id = match &request.payment_id {
824-
Some(id_type) => Some(
825-
id_type
826-
.get_payment_intent_id()
827-
.change_context(errors::ApiErrorResponse::PaymentNotFound)?,
828-
),
829-
None => None,
830-
};
831823

832824
let request_merchant_id = request.merchant_id.as_deref();
833825
helpers::validate_merchant_id(&merchant_account.merchant_id, request_merchant_id)
@@ -840,14 +832,19 @@ impl<F: Send + Clone, Ctx: PaymentMethodRetrieve> ValidateRequest<F, api::Paymen
840832

841833
let mandate_type =
842834
helpers::validate_mandate(request, payments::is_operation_confirm(self))?;
843-
let payment_id =
844-
crate::core::utils::get_or_generate_id("payment_id", &given_payment_id, "pay")?;
835+
836+
let payment_id = request
837+
.payment_id
838+
.clone()
839+
.ok_or(report!(errors::ApiErrorResponse::PaymentNotFound))?;
845840

846841
Ok((
847842
Box::new(self),
848843
operations::ValidateResult {
849844
merchant_id: &merchant_account.merchant_id,
850-
payment_id: api::PaymentIdType::PaymentIntentId(payment_id),
845+
payment_id: payment_id
846+
.and_then(|id| core_utils::validate_id(id, "payment_id"))
847+
.into_report()?,
851848
mandate_type,
852849
storage_scheme: merchant_account.storage_scheme,
853850
requeue: matches!(

‎crates/router/src/core/payments/operations/payment_create.rs

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -529,14 +529,9 @@ impl<F: Send + Clone, Ctx: PaymentMethodRetrieve> ValidateRequest<F, api::Paymen
529529
)?;
530530
}
531531

532-
let given_payment_id = match &request.payment_id {
533-
Some(id_type) => Some(
534-
id_type
535-
.get_payment_intent_id()
536-
.change_context(errors::ApiErrorResponse::PaymentNotFound)?,
537-
),
538-
None => None,
539-
};
532+
let payment_id = request.payment_id.clone().ok_or(error_stack::report!(
533+
errors::ApiErrorResponse::PaymentNotFound
534+
))?;
540535

541536
let request_merchant_id = request.merchant_id.as_deref();
542537
helpers::validate_merchant_id(&merchant_account.merchant_id, request_merchant_id)
@@ -555,8 +550,6 @@ impl<F: Send + Clone, Ctx: PaymentMethodRetrieve> ValidateRequest<F, api::Paymen
555550

556551
helpers::validate_payment_method_fields_present(request)?;
557552

558-
let payment_id = core_utils::get_or_generate_id("payment_id", &given_payment_id, "pay")?;
559-
560553
let mandate_type =
561554
helpers::validate_mandate(request, payments::is_operation_confirm(self))?;
562555

@@ -583,7 +576,7 @@ impl<F: Send + Clone, Ctx: PaymentMethodRetrieve> ValidateRequest<F, api::Paymen
583576
Box::new(self),
584577
operations::ValidateResult {
585578
merchant_id: &merchant_account.merchant_id,
586-
payment_id: api::PaymentIdType::PaymentIntentId(payment_id),
579+
payment_id,
587580
mandate_type,
588581
storage_scheme: merchant_account.storage_scheme,
589582
requeue: matches!(

‎crates/router/src/core/payments/operations/payment_update.rs

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use std::marker::PhantomData;
33
use api_models::enums::FrmSuggestion;
44
use async_trait::async_trait;
55
use common_utils::ext_traits::{AsyncExt, Encode, ValueExt};
6-
use error_stack::ResultExt;
6+
use error_stack::{report, IntoReport, ResultExt};
77
use router_derive::PaymentOperation;
88
use router_env::{instrument, tracing};
99

@@ -607,14 +607,10 @@ impl<F: Send + Clone, Ctx: PaymentMethodRetrieve> ValidateRequest<F, api::Paymen
607607
operations::ValidateResult<'a>,
608608
)> {
609609
helpers::validate_customer_details_in_request(request)?;
610-
let given_payment_id = match &request.payment_id {
611-
Some(id_type) => Some(
612-
id_type
613-
.get_payment_intent_id()
614-
.change_context(errors::ApiErrorResponse::PaymentNotFound)?,
615-
),
616-
None => None,
617-
};
610+
let payment_id = request
611+
.payment_id
612+
.clone()
613+
.ok_or(report!(errors::ApiErrorResponse::PaymentNotFound))?;
618614

619615
let request_merchant_id = request.merchant_id.as_deref();
620616
helpers::validate_merchant_id(&merchant_account.merchant_id, request_merchant_id)
@@ -635,13 +631,14 @@ impl<F: Send + Clone, Ctx: PaymentMethodRetrieve> ValidateRequest<F, api::Paymen
635631
helpers::validate_payment_method_fields_present(request)?;
636632

637633
let mandate_type = helpers::validate_mandate(request, false)?;
638-
let payment_id = core_utils::get_or_generate_id("payment_id", &given_payment_id, "pay")?;
639634

640635
Ok((
641636
Box::new(self),
642637
operations::ValidateResult {
643638
merchant_id: &merchant_account.merchant_id,
644-
payment_id: api::PaymentIdType::PaymentIntentId(payment_id),
639+
payment_id: payment_id
640+
.and_then(|id| core_utils::validate_id(id, "payment_id"))
641+
.into_report()?,
645642
mandate_type,
646643
storage_scheme: merchant_account.storage_scheme,
647644
requeue: matches!(

‎crates/router/src/routes/payments.rs

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,16 @@ pub mod helpers;
33

44
use actix_web::{web, Responder};
55
use api_models::payments::HeaderPayload;
6-
use error_stack::report;
6+
use error_stack::{report, IntoReport};
77
use router_env::{env, instrument, tracing, types, Flow};
88

99
use crate::{
1010
self as app,
1111
core::{
12-
errors::http_not_implemented,
12+
errors::{self, http_not_implemented},
1313
payment_methods::{Oss, PaymentMethodRetrieve},
1414
payments::{self, PaymentRedirectFlow},
15+
utils as core_utils,
1516
},
1617
// openapi::examples::{
1718
// PAYMENTS_CREATE, PAYMENTS_CREATE_MINIMUM_FIELDS, PAYMENTS_CREATE_WITH_ADDRESS,
@@ -22,7 +23,10 @@ use crate::{
2223
routes::lock_utils,
2324
services::{api, authentication as auth},
2425
types::{
25-
api::{self as api_types, enums as api_enums, payments as payment_types},
26+
api::{
27+
self as api_types, enums as api_enums,
28+
payments::{self as payment_types, PaymentIdTypeExt},
29+
},
2630
domain,
2731
transformers::ForeignTryFrom,
2832
},
@@ -94,12 +98,16 @@ pub async fn payments_create(
9498
json_payload: web::Json<payment_types::PaymentsRequest>,
9599
) -> impl Responder {
96100
let flow = Flow::PaymentsCreate;
97-
let payload = json_payload.into_inner();
101+
let mut payload = json_payload.into_inner();
98102

99103
if let Some(api_enums::CaptureMethod::Scheduled) = payload.capture_method {
100104
return http_not_implemented();
101105
};
102106

107+
if let Err(err) = get_or_generate_payment_id(&mut payload) {
108+
return api::log_and_return_error_response(err);
109+
}
110+
103111
let locking_action = payload.get_locking_input(flow.clone());
104112

105113
Box::pin(api::server_wrap(
@@ -959,6 +967,29 @@ where
959967
}
960968
}
961969

970+
pub fn get_or_generate_payment_id(
971+
payload: &mut payment_types::PaymentsRequest,
972+
) -> errors::RouterResult<()> {
973+
let given_payment_id = payload
974+
.payment_id
975+
.clone()
976+
.map(|payment_id| {
977+
payment_id
978+
.get_payment_intent_id()
979+
.map_err(|err| err.change_context(errors::ApiErrorResponse::PaymentNotFound))
980+
})
981+
.transpose()?;
982+
983+
let payment_id =
984+
core_utils::get_or_generate_id("payment_id", &given_payment_id, "pay").into_report()?;
985+
986+
payload.payment_id = Some(api_models::payments::PaymentIdType::PaymentIntentId(
987+
payment_id,
988+
));
989+
990+
Ok(())
991+
}
992+
962993
impl GetLockingInput for payment_types::PaymentsRequest {
963994
fn get_locking_input<F>(&self, flow: F) -> api_locking::LockAction
964995
where

0 commit comments

Comments
 (0)