Skip to content

Commit

Permalink
refactor(router): flagged order_details validation to skip validation (
Browse files Browse the repository at this point in the history
…#3324)

Co-authored-by: Chethan Rao <[email protected]>
  • Loading branch information
sahkal and Chethan-rao authored Jan 11, 2024
1 parent 833ee62 commit b54603c
Show file tree
Hide file tree
Showing 29 changed files with 158 additions and 151 deletions.
3 changes: 1 addition & 2 deletions crates/common_utils/src/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,7 @@ impl RequestBuilder {
}

pub fn headers(mut self, headers: Vec<(String, Maskable<String>)>) -> Self {
let mut h = headers.into_iter().map(|(h, v)| (h, v));
self.headers.extend(&mut h);
self.headers.extend(headers);
self
}

Expand Down
2 changes: 1 addition & 1 deletion crates/kgraph_utils/src/mca.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ fn compile_request_pm_types(

let or_node_neighbor_id = if amount_nodes.len() == 1 {
amount_nodes
.get(0)
.first()
.copied()
.ok_or(KgraphError::IndexingError)?
} else {
Expand Down
1 change: 0 additions & 1 deletion crates/masking/src/diesel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
//! Diesel-related.
//!
pub use diesel::Expression;
use diesel::{
backend::Backend,
deserialize::{self, FromSql, Queryable},
Expand Down
2 changes: 1 addition & 1 deletion crates/masking/src/serde.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
//!
pub use erased_serde::Serialize as ErasedSerialize;
pub use serde::{de, ser, Deserialize, Serialize, Serializer};
pub use serde::{de, Deserialize, Serialize, Serializer};
use serde_json::{value::Serializer as JsonValueSerializer, Value};

use crate::{Secret, Strategy, StrongSecret, ZeroizableSecret};
Expand Down
2 changes: 1 addition & 1 deletion crates/redis_interface/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ pub use fred::interfaces::PubsubInterface;
use fred::{interfaces::ClientLike, prelude::EventInterface};
use router_env::logger;

pub use self::{commands::*, types::*};
pub use self::types::*;

pub struct RedisConnectionPool {
pub pool: fred::prelude::RedisPool,
Expand Down
2 changes: 1 addition & 1 deletion crates/router/src/configs/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,7 @@ impl From<Database> for storage_impl::config::Database {
dbname: val.dbname,
pool_size: val.pool_size,
connection_timeout: val.connection_timeout,
queue_strategy: val.queue_strategy.into(),
queue_strategy: val.queue_strategy,
min_idle: val.min_idle,
max_lifetime: val.max_lifetime,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -297,11 +297,11 @@ fn build_error_response<T>(

get_error_response(
response
.get(0)
.first()
.and_then(|err_details| err_details.extensions.as_ref())
.and_then(|extensions| extensions.legacy_code.clone()),
response
.get(0)
.first()
.map(|err_details| err_details.message.clone()),
reason,
http_code,
Expand Down
5 changes: 1 addition & 4 deletions crates/router/src/connector/checkout/transformers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1026,10 +1026,7 @@ impl utils::MultipleCaptureSyncResponse for Box<PaymentsResponse> {
self.status == CheckoutPaymentStatus::Captured
}
fn get_amount_captured(&self) -> Option<i64> {
match self.amount {
Some(amount) => amount.try_into().ok(),
None => None,
}
self.amount.map(Into::into)
}
}

Expand Down
2 changes: 1 addition & 1 deletion crates/router/src/connector/stripe/transformers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2336,7 +2336,7 @@ pub fn get_connector_metadata(
let next_action_response = next_action
.and_then(|next_action_response| match next_action_response {
StripeNextActionResponse::DisplayBankTransferInstructions(response) => {
let bank_instructions = response.financial_addresses.get(0);
let bank_instructions = response.financial_addresses.first();
let (sepa_bank_instructions, bacs_bank_instructions) =
bank_instructions.map_or((None, None), |financial_address| {
(
Expand Down
160 changes: 80 additions & 80 deletions crates/router/src/connector/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1508,86 +1508,6 @@ pub fn get_error_code_error_message_based_on_priority(
.cloned()
}

#[cfg(test)]
mod error_code_error_message_tests {
#![allow(clippy::unwrap_used)]
use super::*;

struct TestConnector;

impl ConnectorErrorTypeMapping for TestConnector {
fn get_connector_error_type(
&self,
error_code: String,
error_message: String,
) -> ConnectorErrorType {
match (error_code.as_str(), error_message.as_str()) {
("01", "INVALID_MERCHANT") => ConnectorErrorType::BusinessError,
("03", "INVALID_CVV") => ConnectorErrorType::UserError,
("04", "04") => ConnectorErrorType::TechnicalError,
_ => ConnectorErrorType::UnknownError,
}
}
}

#[test]
fn test_get_error_code_error_message_based_on_priority() {
let error_code_message_list_unknown = vec![
ErrorCodeAndMessage {
error_code: "01".to_string(),
error_message: "INVALID_MERCHANT".to_string(),
},
ErrorCodeAndMessage {
error_code: "05".to_string(),
error_message: "05".to_string(),
},
ErrorCodeAndMessage {
error_code: "03".to_string(),
error_message: "INVALID_CVV".to_string(),
},
ErrorCodeAndMessage {
error_code: "04".to_string(),
error_message: "04".to_string(),
},
];
let error_code_message_list_user = vec![
ErrorCodeAndMessage {
error_code: "01".to_string(),
error_message: "INVALID_MERCHANT".to_string(),
},
ErrorCodeAndMessage {
error_code: "03".to_string(),
error_message: "INVALID_CVV".to_string(),
},
];
let error_code_error_message_unknown = get_error_code_error_message_based_on_priority(
TestConnector,
error_code_message_list_unknown,
);
let error_code_error_message_user = get_error_code_error_message_based_on_priority(
TestConnector,
error_code_message_list_user,
);
let error_code_error_message_none =
get_error_code_error_message_based_on_priority(TestConnector, vec![]);
assert_eq!(
error_code_error_message_unknown,
Some(ErrorCodeAndMessage {
error_code: "05".to_string(),
error_message: "05".to_string(),
})
);
assert_eq!(
error_code_error_message_user,
Some(ErrorCodeAndMessage {
error_code: "03".to_string(),
error_message: "INVALID_CVV".to_string(),
})
);
assert_eq!(error_code_error_message_none, None);
}
}

pub trait MultipleCaptureSyncResponse {
fn get_connector_capture_id(&self) -> String;
fn get_capture_attempt_status(&self) -> enums::AttemptStatus;
Expand Down Expand Up @@ -1781,3 +1701,83 @@ impl FrmTransactionRouterDataRequest for fraud_check::FrmTransactionRouterData {
}
}
}

#[cfg(test)]
mod error_code_error_message_tests {
#![allow(clippy::unwrap_used)]
use super::*;

struct TestConnector;

impl ConnectorErrorTypeMapping for TestConnector {
fn get_connector_error_type(
&self,
error_code: String,
error_message: String,
) -> ConnectorErrorType {
match (error_code.as_str(), error_message.as_str()) {
("01", "INVALID_MERCHANT") => ConnectorErrorType::BusinessError,
("03", "INVALID_CVV") => ConnectorErrorType::UserError,
("04", "04") => ConnectorErrorType::TechnicalError,
_ => ConnectorErrorType::UnknownError,
}
}
}

#[test]
fn test_get_error_code_error_message_based_on_priority() {
let error_code_message_list_unknown = vec![
ErrorCodeAndMessage {
error_code: "01".to_string(),
error_message: "INVALID_MERCHANT".to_string(),
},
ErrorCodeAndMessage {
error_code: "05".to_string(),
error_message: "05".to_string(),
},
ErrorCodeAndMessage {
error_code: "03".to_string(),
error_message: "INVALID_CVV".to_string(),
},
ErrorCodeAndMessage {
error_code: "04".to_string(),
error_message: "04".to_string(),
},
];
let error_code_message_list_user = vec![
ErrorCodeAndMessage {
error_code: "01".to_string(),
error_message: "INVALID_MERCHANT".to_string(),
},
ErrorCodeAndMessage {
error_code: "03".to_string(),
error_message: "INVALID_CVV".to_string(),
},
];
let error_code_error_message_unknown = get_error_code_error_message_based_on_priority(
TestConnector,
error_code_message_list_unknown,
);
let error_code_error_message_user = get_error_code_error_message_based_on_priority(
TestConnector,
error_code_message_list_user,
);
let error_code_error_message_none =
get_error_code_error_message_based_on_priority(TestConnector, vec![]);
assert_eq!(
error_code_error_message_unknown,
Some(ErrorCodeAndMessage {
error_code: "05".to_string(),
error_message: "05".to_string(),
})
);
assert_eq!(
error_code_error_message_user,
Some(ErrorCodeAndMessage {
error_code: "03".to_string(),
error_message: "INVALID_CVV".to_string(),
})
);
assert_eq!(error_code_error_message_none, None);
}
}
4 changes: 2 additions & 2 deletions crates/router/src/connector/wise.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ impl ConnectorCommon for Wise {
let default_status = response.status.unwrap_or_default().to_string();
match response.errors {
Some(errs) => {
if let Some(e) = errs.get(0) {
if let Some(e) = errs.first() {
Ok(types::ErrorResponse {
status_code: res.status_code,
code: e.code.clone(),
Expand Down Expand Up @@ -301,7 +301,7 @@ impl services::ConnectorIntegration<api::PoCancel, types::PayoutsData, types::Pa
.change_context(errors::ConnectorError::ResponseDeserializationFailed)?;
let def_res = response.status.unwrap_or_default().to_string();
let errors = response.errors.unwrap_or_default();
let (code, message) = if let Some(e) = errors.get(0) {
let (code, message) = if let Some(e) = errors.first() {
(e.code.clone(), e.message.clone())
} else {
(def_res, response.message.unwrap_or_default())
Expand Down
10 changes: 5 additions & 5 deletions crates/router/src/core/fraud_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -306,21 +306,21 @@ where
// Panic Safety: we are first checking if the object is present... only if present, we try to fetch index 0
let frm_configs_object = FrmConfigsObject {
frm_enabled_gateway: filtered_frm_config
.get(0)
.first()
.and_then(|c| c.gateway),
frm_enabled_pm: filtered_payment_methods
.get(0)
.first()
.and_then(|pm| pm.payment_method),
frm_enabled_pm_type: filtered_payment_method_types
.get(0)
.first()
.and_then(|pmt| pmt.payment_method_type),
frm_action: filtered_payment_method_types
// .clone()
.get(0)
.first()
.map(|pmt| pmt.action.clone())
.unwrap_or(api_enums::FrmAction::ManualReview),
frm_preferred_flow_type: filtered_payment_method_types
.get(0)
.first()
.map(|pmt| pmt.flow.clone())
.unwrap_or(api_enums::FrmPreferredFlowTypes::Pre),
};
Expand Down
5 changes: 1 addition & 4 deletions crates/router/src/core/payments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2172,10 +2172,7 @@ pub async fn apply_filters_on_payments(
merchant.storage_scheme,
)
.await
.to_not_found_response(errors::ApiErrorResponse::PaymentNotFound)?
.into_iter()
.map(|(pi, pa)| (pi, pa))
.collect();
.to_not_found_response(errors::ApiErrorResponse::PaymentNotFound)?;

let data: Vec<api::PaymentsResponse> =
list.into_iter().map(ForeignFrom::foreign_from).collect();
Expand Down
23 changes: 14 additions & 9 deletions crates/router/src/core/payments/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3799,17 +3799,22 @@ pub async fn get_gsm_record(
pub fn validate_order_details_amount(
order_details: Vec<api_models::payments::OrderDetailsWithAmount>,
amount: i64,
should_validate: bool,
) -> Result<(), errors::ApiErrorResponse> {
let total_order_details_amount: i64 = order_details
.iter()
.map(|order| order.amount * i64::from(order.quantity))
.sum();
if should_validate {
let total_order_details_amount: i64 = order_details
.iter()
.map(|order| order.amount * i64::from(order.quantity))
.sum();

if total_order_details_amount != amount {
Err(errors::ApiErrorResponse::InvalidRequestData {
message: "Total sum of order details doesn't match amount in payment request"
.to_string(),
})
if total_order_details_amount != amount {
Err(errors::ApiErrorResponse::InvalidRequestData {
message: "Total sum of order details doesn't match amount in payment request"
.to_string(),
})
} else {
Ok(())
}
} else {
Ok(())
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ impl<F: Send + Clone, Ctx: PaymentMethodRetrieve>
helpers::validate_order_details_amount(
order_details.to_owned(),
payment_intent.amount,
false,
)?;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ impl<F: Send + Clone, Ctx: PaymentMethodRetrieve>
helpers::validate_order_details_amount(
order_details.to_owned(),
payment_intent.amount,
false,
)?;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ impl<F: Send + Clone, Ctx: PaymentMethodRetrieve>
helpers::validate_order_details_amount(
order_details.to_owned(),
payment_intent.amount,
false,
)?;
}

Expand Down
2 changes: 1 addition & 1 deletion crates/router/src/core/payments/transformers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,7 @@ where
if third_party_sdk_session_next_action(&payment_attempt, operation) {
next_action_response = Some(
api_models::payments::NextActionData::ThirdPartySdkSessionToken {
session_token: payment_data.sessions_token.get(0).cloned(),
session_token: payment_data.sessions_token.first().cloned(),
},
)
}
Expand Down
4 changes: 2 additions & 2 deletions crates/router/src/db/dispute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -572,7 +572,7 @@ mod tests {

assert_eq!(1, found_disputes.len());

assert_eq!(created_dispute, found_disputes.get(0).unwrap().clone());
assert_eq!(created_dispute, found_disputes.first().unwrap().clone());
}

#[tokio::test]
Expand Down Expand Up @@ -611,7 +611,7 @@ mod tests {

assert_eq!(1, found_disputes.len());

assert_eq!(created_dispute, found_disputes.get(0).unwrap().clone());
assert_eq!(created_dispute, found_disputes.first().unwrap().clone());
}

mod update_dispute {
Expand Down
Loading

0 comments on commit b54603c

Please sign in to comment.