Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(router): flagged order_details validation to skip validation #3324

Merged
merged 4 commits into from
Jan 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading