From 02bc002d5812ae87af2d2cc7478f9f15430055c2 Mon Sep 17 00:00:00 2001 From: Sanchith Hegde Date: Fri, 25 Aug 2023 00:11:27 +0530 Subject: [PATCH 01/10] chore: address Rust 1.72 clippy lints --- .cargo/config.toml | 2 +- Cargo.lock | 40 ++++++++++++++++--- crates/router/src/connector/checkout.rs | 2 +- crates/router/src/connector/stripe.rs | 2 +- crates/router/src/core/errors.rs | 6 +-- .../router/src/core/payment_methods/cards.rs | 2 +- .../payments/operations/payment_status.rs | 10 +++-- crates/router/src/core/refunds.rs | 2 +- .../src/scheduler/workflows/payment_sync.rs | 2 +- crates/router/src/services/api/request.rs | 10 ++--- crates/router/tests/connectors/adyen.rs | 2 +- crates/router/tests/payments.rs | 2 +- .../test_utils/tests/connectors/selenium.rs | 2 +- 13 files changed, 54 insertions(+), 30 deletions(-) diff --git a/.cargo/config.toml b/.cargo/config.toml index 66da12a13b11..806b60a9a697 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -1,4 +1,4 @@ -[target.'cfg(all())'] +[build] rustflags = [ "-Funsafe_code", "-Wclippy::as_conversions", diff --git a/Cargo.lock b/Cargo.lock index ce46b6c6e95b..1eb250514f39 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1644,6 +1644,22 @@ dependencies = [ "parking_lot_core", ] +[[package]] +name = "data_models" +version = "0.1.0" +dependencies = [ + "api_models", + "async-trait", + "common_enums", + "common_utils", + "error-stack", + "serde", + "serde_json", + "strum 0.25.0", + "thiserror", + "time 0.3.22", +] + [[package]] name = "deadpool" version = "0.9.5" @@ -1824,9 +1840,9 @@ dependencies = [ [[package]] name = "dyn-clone" -version = "1.0.11" +version = "1.0.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "68b0cf012f1230e43cd00ebb729c6bb58707ecfa8ad08b52ef3a4ccd2697fc30" +checksum = "bbfc4744c1b8f2a09adc0e55242f60b1af195d88596bd8700be74418c056c555" [[package]] name = "either" @@ -2998,9 +3014,9 @@ dependencies = [ [[package]] name = "moka" -version = "0.11.2" +version = "0.11.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "206bf83f415b0579fd885fe0804eb828e727636657dc1bf73d80d2f1218e14a1" +checksum = "fa6e72583bf6830c956235bff0d5afec8cf2952f579ebad18ae7821a917d950f" dependencies = [ "async-io", "async-lock", @@ -3968,7 +3984,7 @@ dependencies = [ "clap", "common_utils", "config", - "crc32fast", + "data_models", "derive_deref", "diesel", "diesel_models", @@ -3988,7 +4004,6 @@ dependencies = [ "maud", "mimalloc", "mime", - "moka", "nanoid", "num_cpus", "once_cell", @@ -4588,11 +4603,24 @@ checksum = "6e63cff320ae2c57904679ba7cb63280a3dc4613885beafb148ee7bf9aa9042d" name = "storage_impl" version = "0.1.0" dependencies = [ + "api_models", "async-bb8-diesel", "async-trait", "bb8", + "common_utils", + "crc32fast", + "data_models", "diesel", + "diesel_models", + "dyn-clone", + "error-stack", + "futures", "masking", + "moka", + "once_cell", + "redis_interface", + "router_env", + "tokio", ] [[package]] diff --git a/crates/router/src/connector/checkout.rs b/crates/router/src/connector/checkout.rs index 3515533baa40..4f0bd152e234 100644 --- a/crates/router/src/connector/checkout.rs +++ b/crates/router/src/connector/checkout.rs @@ -848,7 +848,7 @@ impl api::FileUpload for Checkout { match purpose { api::FilePurpose::DisputeEvidence => { let supported_file_types = - vec!["image/jpeg", "image/jpg", "image/png", "application/pdf"]; + ["image/jpeg", "image/jpg", "image/png", "application/pdf"]; // 4 Megabytes (MB) if file_size > 4000000 { Err(errors::ConnectorError::FileValidationFailed { diff --git a/crates/router/src/connector/stripe.rs b/crates/router/src/connector/stripe.rs index f7dcebe10fa0..3e8c31ef6615 100644 --- a/crates/router/src/connector/stripe.rs +++ b/crates/router/src/connector/stripe.rs @@ -1311,7 +1311,7 @@ impl api::FileUpload for Stripe { ) -> CustomResult<(), errors::ConnectorError> { match purpose { api::FilePurpose::DisputeEvidence => { - let supported_file_types = vec!["image/jpeg", "image/png", "application/pdf"]; + let supported_file_types = ["image/jpeg", "image/png", "application/pdf"]; // 5 Megabytes (MB) if file_size > 5000000 { Err(errors::ConnectorError::FileValidationFailed { diff --git a/crates/router/src/core/errors.rs b/crates/router/src/core/errors.rs index 18191545ae0f..d298d69caf85 100644 --- a/crates/router/src/core/errors.rs +++ b/crates/router/src/core/errors.rs @@ -599,11 +599,7 @@ pub mod error_stack_parsing { attachments: current_error.attachments, }] .into_iter() - .chain( - Into::>::into(current_error.sources) - .0 - .into_iter(), - ) + .chain(Into::>::into(current_error.sources).0) }) .collect(); Self(multi_layered_errors) diff --git a/crates/router/src/core/payment_methods/cards.rs b/crates/router/src/core/payment_methods/cards.rs index 1028c37c25ab..6d623c339c0b 100644 --- a/crates/router/src/core/payment_methods/cards.rs +++ b/crates/router/src/core/payment_methods/cards.rs @@ -754,7 +754,7 @@ pub fn get_banks( fn get_val(str: String, val: &serde_json::Value) -> Option { str.split('.') - .fold(Some(val), |acc, x| acc.and_then(|v| v.get(x))) + .try_fold(val, |acc, x| acc.get(x)) .and_then(|v| v.as_str()) .map(|s| s.to_string()) } diff --git a/crates/router/src/core/payments/operations/payment_status.rs b/crates/router/src/core/payments/operations/payment_status.rs index 7960b4685965..732a0080a68f 100644 --- a/crates/router/src/core/payments/operations/payment_status.rs +++ b/crates/router/src/core/payments/operations/payment_status.rs @@ -415,7 +415,7 @@ pub async fn get_payment_intent_payment_attempt( merchant_id: &str, storage_scheme: enums::MerchantStorageScheme, ) -> RouterResult<(storage::PaymentIntent, storage::PaymentAttempt)> { - (|| async { + let get_pi_pa = || async { let (pi, pa); match payment_id { api_models::payments::PaymentIdType::PaymentIntentId(ref id) => { @@ -482,7 +482,9 @@ pub async fn get_payment_intent_payment_attempt( } } error_stack::Result::<_, errors::DataStorageError>::Ok((pi, pa)) - })() - .await - .to_not_found_response(errors::ApiErrorResponse::PaymentNotFound) + }; + + get_pi_pa() + .await + .to_not_found_response(errors::ApiErrorResponse::PaymentNotFound) } diff --git a/crates/router/src/core/refunds.rs b/crates/router/src/core/refunds.rs index db194ad8e6c7..41729ab51773 100644 --- a/crates/router/src/core/refunds.rs +++ b/crates/router/src/core/refunds.rs @@ -842,7 +842,7 @@ pub async fn sync_refund_with_gateway_workflow( }, ) .await?; - let terminal_status = vec![ + let terminal_status = [ enums::RefundStatus::Success, enums::RefundStatus::Failure, enums::RefundStatus::TransactionFailure, diff --git a/crates/router/src/scheduler/workflows/payment_sync.rs b/crates/router/src/scheduler/workflows/payment_sync.rs index 4aa5c9fd3409..db82019a524a 100644 --- a/crates/router/src/scheduler/workflows/payment_sync.rs +++ b/crates/router/src/scheduler/workflows/payment_sync.rs @@ -62,7 +62,7 @@ impl ProcessTrackerWorkflow for PaymentsSyncWorkflow { ) .await?; - let terminal_status = vec![ + let terminal_status = [ enums::AttemptStatus::RouterDeclined, enums::AttemptStatus::Charged, enums::AttemptStatus::AutoRefunded, diff --git a/crates/router/src/services/api/request.rs b/crates/router/src/services/api/request.rs index 6c96e336a101..2a9459b59302 100644 --- a/crates/router/src/services/api/request.rs +++ b/crates/router/src/services/api/request.rs @@ -275,8 +275,8 @@ impl HeaderExt for Headers { ) -> CustomResult { use reqwest::header::{HeaderMap, HeaderName, HeaderValue}; - self.into_iter().fold( - Ok(HeaderMap::new()), + self.into_iter().try_fold( + HeaderMap::new(), |mut header_map, (header_name, header_value)| { let header_name = HeaderName::from_str(&header_name) .into_report() @@ -285,10 +285,8 @@ impl HeaderExt for Headers { let header_value = HeaderValue::from_str(&header_value) .into_report() .change_context(errors::ApiClientError::HeaderMapConstructionFailed)?; - if let Ok(map) = header_map.as_mut() { - map.append(header_name, header_value); - } - header_map + header_map.append(header_name, header_value); + Ok(header_map) }, ) } diff --git a/crates/router/tests/connectors/adyen.rs b/crates/router/tests/connectors/adyen.rs index 68d641e5ec45..06f048bb7461 100644 --- a/crates/router/tests/connectors/adyen.rs +++ b/crates/router/tests/connectors/adyen.rs @@ -477,7 +477,7 @@ async fn should_fail_payment_for_invalid_exp_month() { ) .await .unwrap(); - let errors = vec!["The provided Expiry Date is not valid.: Expiry month should be between 1 and 12 inclusive: 20","Refused"]; + let errors = ["The provided Expiry Date is not valid.: Expiry month should be between 1 and 12 inclusive: 20","Refused"]; assert!(errors.contains(&response.response.unwrap_err().message.as_str())) } diff --git a/crates/router/tests/payments.rs b/crates/router/tests/payments.rs index b08a20e3ce1e..5e69f0105b28 100644 --- a/crates/router/tests/payments.rs +++ b/crates/router/tests/payments.rs @@ -226,7 +226,7 @@ async fn payments_todo() { let client = awc::Client::default(); let mut response; let mut response_body; - let _post_endpoints = vec!["123/update", "123/confirm", "cancel"]; + let _post_endpoints = ["123/update", "123/confirm", "cancel"]; let get_endpoints = vec!["list"]; for endpoint in get_endpoints { diff --git a/crates/test_utils/tests/connectors/selenium.rs b/crates/test_utils/tests/connectors/selenium.rs index 77bc7f45cbd3..370c1ffe9874 100644 --- a/crates/test_utils/tests/connectors/selenium.rs +++ b/crates/test_utils/tests/connectors/selenium.rs @@ -832,7 +832,7 @@ fn get_chrome_profile_path() -> Result { fp.join(&MAIN_SEPARATOR.to_string()) }) .unwrap(); - base_path.push_str(r#"/Library/Application\ Support/Google/Chrome/Default"#); //Issue: 1573 + base_path.push_str("/Library/Application\\ Support/Google/Chrome/Default"); //Issue: 1573 Ok(base_path) } From 69c0b8d530adb414f0c0d96bb4e11b761202c1d9 Mon Sep 17 00:00:00 2001 From: Sanchith Hegde Date: Fri, 25 Aug 2023 00:34:27 +0530 Subject: [PATCH 02/10] chore(openapi): update OpenAPI spec --- openapi/openapi_spec.json | 5 ----- 1 file changed, 5 deletions(-) diff --git a/openapi/openapi_spec.json b/openapi/openapi_spec.json index bf95efe64fd5..7a7bc9d8910a 100644 --- a/openapi/openapi_spec.json +++ b/openapi/openapi_spec.json @@ -10163,11 +10163,6 @@ "format": "int64", "description": "The total number of refunds in the list" }, - "total_count": { - "type": "integer", - "format": "int64", - "description": "The total number of refunds in the list" - }, "data": { "type": "array", "items": { From 4ee8efb8c60fa36fc246432de7fe494e73cce068 Mon Sep 17 00:00:00 2001 From: Sampras Lopes Date: Fri, 25 Aug 2023 02:04:51 +0530 Subject: [PATCH 03/10] chore(lints): fix clippy panic docs lints --- crates/data_models/src/errors.rs | 4 ++ crates/drainer/src/connection.rs | 4 ++ crates/drainer/src/services.rs | 1 + crates/router/src/connection.rs | 4 ++ crates/router/src/routes/app.rs | 14 +++-- crates/router/src/services.rs | 17 +++--- crates/storage_impl/src/database/store.rs | 36 +++++++----- crates/storage_impl/src/lib.rs | 70 +++++++++++++---------- crates/test_utils/src/connector_auth.rs | 6 ++ 9 files changed, 99 insertions(+), 57 deletions(-) diff --git a/crates/data_models/src/errors.rs b/crates/data_models/src/errors.rs index 263538847e01..4f8229ea0c9b 100644 --- a/crates/data_models/src/errors.rs +++ b/crates/data_models/src/errors.rs @@ -1,5 +1,9 @@ +pub type StorageResult = error_stack::Result; + #[derive(Debug, thiserror::Error)] pub enum StorageError { + #[error("Initialization Error")] + InitializationError, // TODO: deprecate this error type to use a domain error instead #[error("DatabaseError: {0:?}")] DatabaseError(String), diff --git a/crates/drainer/src/connection.rs b/crates/drainer/src/connection.rs index 429a80c0daa8..7b273244cbce 100644 --- a/crates/drainer/src/connection.rs +++ b/crates/drainer/src/connection.rs @@ -18,6 +18,10 @@ pub async fn redis_connection( .expect("Failed to create Redis connection Pool") } +// TODO: use stores defined in storage_impl instead +/// # Panics +/// +/// Will panic if could not create a db pool #[allow(clippy::expect_used)] pub async fn diesel_make_pg_pool( database: &Database, diff --git a/crates/drainer/src/services.rs b/crates/drainer/src/services.rs index 6edec31f26d7..5982bde45b7b 100644 --- a/crates/drainer/src/services.rs +++ b/crates/drainer/src/services.rs @@ -16,6 +16,7 @@ pub struct StoreConfig { } impl Store { + #[allow(clippy::expect_used)] pub async fn new(config: &crate::settings::Settings, test_transaction: bool) -> Self { Self { master_pool: diesel_make_pg_pool( diff --git a/crates/router/src/connection.rs b/crates/router/src/connection.rs index c5ca81004922..06536572c8fc 100644 --- a/crates/router/src/connection.rs +++ b/crates/router/src/connection.rs @@ -8,6 +8,10 @@ pub type PgPool = bb8::Pool>; pub type PgPooledConn = async_bb8_diesel::Connection; +/// +/// # Panics +/// +/// Panics if failed to create a redis pool #[allow(clippy::expect_used)] pub async fn redis_connection( conf: &crate::configs::settings::Settings, diff --git a/crates/router/src/routes/app.rs b/crates/router/src/routes/app.rs index 763f159e880e..8b61c94ea35b 100644 --- a/crates/router/src/routes/app.rs +++ b/crates/router/src/routes/app.rs @@ -59,6 +59,10 @@ impl AppStateInfo for AppState { } impl AppState { + /// # Panics + /// + /// Panics if Store can't be created or JWE decryption fails + #[allow(clippy::expect_used)] pub async fn with_storage( conf: settings::Settings, storage_impl: StorageImpl, @@ -68,14 +72,15 @@ impl AppState { let kms_client = kms::get_kms_client(&conf.kms).await; let testable = storage_impl == StorageImpl::PostgresqlTest; let store: Box = match storage_impl { - StorageImpl::Postgresql | StorageImpl::PostgresqlTest => { - Box::new(get_store(&conf, shut_down_signal, testable).await) - } + StorageImpl::Postgresql | StorageImpl::PostgresqlTest => Box::new( + get_store(&conf, shut_down_signal, testable) + .await + .expect("Failed to create store"), + ), StorageImpl::Mock => Box::new(MockDb::new(&conf).await), }; #[cfg(feature = "kms")] - #[allow(clippy::expect_used)] let kms_secrets = settings::ActiveKmsSecrets { jwekey: conf.jwekey.clone().into(), } @@ -84,7 +89,6 @@ impl AppState { .expect("Failed while performing KMS decryption"); #[cfg(feature = "email")] - #[allow(clippy::expect_used)] let email_client = Box::new(AwsSes::new(&conf.email).await); Self { flow_name: String::from("default"), diff --git a/crates/router/src/services.rs b/crates/router/src/services.rs index fd4a1667d16d..c8e4ff7a0db0 100644 --- a/crates/router/src/services.rs +++ b/crates/router/src/services.rs @@ -3,6 +3,7 @@ pub mod authentication; pub mod encryption; pub mod logger; +use data_models::errors::{StorageError, StorageResult}; use error_stack::{IntoReport, ResultExt}; #[cfg(feature = "kms")] use external_services::kms::{self, decrypt::KmsDecrypt}; @@ -31,29 +32,29 @@ pub async fn get_store( config: &settings::Settings, shut_down_signal: oneshot::Sender<()>, test_transaction: bool, -) -> Store { +) -> StorageResult { #[cfg(feature = "kms")] let kms_client = kms::get_kms_client(&config.kms).await; #[cfg(feature = "kms")] - #[allow(clippy::expect_used)] let master_config = config .master_database .clone() .decrypt_inner(kms_client) .await - .expect("Failed to decrypt master database config"); + .change_context(StorageError::InitializationError) + .attach_printable("Failed to decrypt master database config")?; #[cfg(not(feature = "kms"))] let master_config = config.master_database.clone().into(); #[cfg(all(feature = "olap", feature = "kms"))] - #[allow(clippy::expect_used)] let replica_config = config .replica_database .clone() .decrypt_inner(kms_client) .await - .expect("Failed to decrypt replica database config"); + .change_context(StorageError::InitializationError) + .attach_printable("Failed to decrypt replica database config")?; #[cfg(all(feature = "olap", not(feature = "kms")))] let replica_config = config.replica_database.clone().into(); @@ -70,7 +71,7 @@ pub async fn get_store( let conf = (master_config, replica_config); let store: RouterStore = if test_transaction { - RouterStore::test_store(conf, &config.redis, master_enc_key).await + RouterStore::test_store(conf, &config.redis, master_enc_key).await? } else { RouterStore::from_config( conf, @@ -79,7 +80,7 @@ pub async fn get_store( shut_down_signal, consts::PUB_SUB_CHANNEL, ) - .await + .await? }; #[cfg(feature = "kv_store")] @@ -89,7 +90,7 @@ pub async fn get_store( config.drainer.num_partitions, ); - store + Ok(store) } #[allow(clippy::expect_used)] diff --git a/crates/storage_impl/src/database/store.rs b/crates/storage_impl/src/database/store.rs index d2e18475eab3..f9a7450b1cd4 100644 --- a/crates/storage_impl/src/database/store.rs +++ b/crates/storage_impl/src/database/store.rs @@ -1,6 +1,8 @@ use async_bb8_diesel::{AsyncConnection, ConnectionError}; use bb8::CustomizeConnection; +use data_models::errors::{StorageError, StorageResult}; use diesel::PgConnection; +use error_stack::{IntoReport, ResultExt}; use masking::PeekInterface; use crate::config::Database; @@ -11,7 +13,7 @@ pub type PgPooledConn = async_bb8_diesel::Connection; #[async_trait::async_trait] pub trait DatabaseStore: Clone + Send + Sync { type Config: Send; - async fn new(config: Self::Config, test_transaction: bool) -> Self; + async fn new(config: Self::Config, test_transaction: bool) -> StorageResult; fn get_master_pool(&self) -> &PgPool; fn get_replica_pool(&self) -> &PgPool; } @@ -24,10 +26,10 @@ pub struct Store { #[async_trait::async_trait] impl DatabaseStore for Store { type Config = Database; - async fn new(config: Database, test_transaction: bool) -> Self { - Self { - master_pool: diesel_make_pg_pool(&config, test_transaction).await, - } + async fn new(config: Database, test_transaction: bool) -> StorageResult { + Ok(Self { + master_pool: diesel_make_pg_pool(&config, test_transaction).await?, + }) } fn get_master_pool(&self) -> &PgPool { @@ -48,14 +50,18 @@ pub struct ReplicaStore { #[async_trait::async_trait] impl DatabaseStore for ReplicaStore { type Config = (Database, Database); - async fn new(config: (Database, Database), test_transaction: bool) -> Self { + async fn new(config: (Database, Database), test_transaction: bool) -> StorageResult { let (master_config, replica_config) = config; - let master_pool = diesel_make_pg_pool(&master_config, test_transaction).await; - let replica_pool = diesel_make_pg_pool(&replica_config, test_transaction).await; - Self { + let master_pool = diesel_make_pg_pool(&master_config, test_transaction) + .await + .attach_printable("failed to create master pool")?; + let replica_pool = diesel_make_pg_pool(&replica_config, test_transaction) + .await + .attach_printable("failed to create replica pool")?; + Ok(Self { master_pool, replica_pool, - } + }) } fn get_master_pool(&self) -> &PgPool { @@ -67,8 +73,10 @@ impl DatabaseStore for ReplicaStore { } } -#[allow(clippy::expect_used)] -pub async fn diesel_make_pg_pool(database: &Database, test_transaction: bool) -> PgPool { +pub async fn diesel_make_pg_pool( + database: &Database, + test_transaction: bool, +) -> StorageResult { let database_url = format!( "postgres://{}:{}@{}:{}/{}", database.username, @@ -88,7 +96,9 @@ pub async fn diesel_make_pg_pool(database: &Database, test_transaction: bool) -> pool.build(manager) .await - .expect("Failed to create PostgreSQL connection pool") + .into_report() + .change_context(StorageError::InitializationError) + .attach_printable("Failed to create PostgreSQL connection pool") } #[derive(Debug)] diff --git a/crates/storage_impl/src/lib.rs b/crates/storage_impl/src/lib.rs index 88b817356441..818c17e004c1 100644 --- a/crates/storage_impl/src/lib.rs +++ b/crates/storage_impl/src/lib.rs @@ -1,5 +1,6 @@ use std::sync::Arc; +use data_models::errors::{StorageError, StorageResult}; use error_stack::ResultExt; use masking::StrongSecret; use redis::{kv_store::RedisConnInterface, RedisStore}; @@ -35,11 +36,13 @@ where tokio::sync::oneshot::Sender<()>, &'static str, ); - async fn new(config: Self::Config, test_transaction: bool) -> Self { + async fn new(config: Self::Config, test_transaction: bool) -> StorageResult { let (db_conf, cache_conf, encryption_key, cache_error_signal, inmemory_cache_stream) = config; if test_transaction { - Self::test_store(db_conf, &cache_conf, encryption_key).await + Self::test_store(db_conf, &cache_conf, encryption_key) + .await + .attach_printable("failed to create test router store") } else { Self::from_config( db_conf, @@ -49,6 +52,7 @@ where inmemory_cache_stream, ) .await + .attach_printable("failed to create store") } } fn get_master_pool(&self) -> &PgPool { @@ -74,46 +78,48 @@ impl RouterStore { encryption_key: StrongSecret>, cache_error_signal: tokio::sync::oneshot::Sender<()>, inmemory_cache_stream: &str, - ) -> Self { - // TODO: create an error enum and return proper error here - let db_store = T::new(db_conf, false).await; - #[allow(clippy::expect_used)] + ) -> StorageResult { + let db_store = T::new(db_conf, false).await?; let cache_store = RedisStore::new(cache_conf) .await - .expect("Failed to create cache store"); + .change_context(StorageError::InitializationError) + .attach_printable("Failed to create cache store")?; cache_store.set_error_callback(cache_error_signal); - #[allow(clippy::expect_used)] cache_store .subscribe_to_channel(inmemory_cache_stream) .await - .expect("Failed to subscribe to inmemory cache stream"); - Self { + .change_context(StorageError::InitializationError) + .attach_printable("Failed to subscribe to inmemory cache stream")?; + Ok(Self { db_store, cache_store, master_encryption_key: encryption_key, - } + }) } pub fn master_key(&self) -> &StrongSecret> { &self.master_encryption_key } + /// # Panics + /// + /// Will panic if `CONNECTOR_AUTH_FILE_PATH` is not set pub async fn test_store( db_conf: T::Config, cache_conf: &redis_interface::RedisSettings, encryption_key: StrongSecret>, - ) -> Self { + ) -> StorageResult { // TODO: create an error enum and return proper error here - let db_store = T::new(db_conf, true).await; - #[allow(clippy::expect_used)] + let db_store = T::new(db_conf, true).await?; let cache_store = RedisStore::new(cache_conf) .await - .expect("Failed to create cache store"); - Self { + .change_context(StorageError::InitializationError) + .attach_printable("failed to create redis cache")?; + Ok(Self { db_store, cache_store, master_encryption_key: encryption_key, - } + }) } } @@ -131,9 +137,13 @@ where T: DatabaseStore, { type Config = (RouterStore, String, u8); - async fn new(config: Self::Config, _test_transaction: bool) -> Self { + async fn new(config: Self::Config, _test_transaction: bool) -> StorageResult { let (router_store, drainer_stream_name, drainer_num_partitions) = config; - Self::from_store(router_store, drainer_stream_name, drainer_num_partitions) + Ok(Self::from_store( + router_store, + drainer_stream_name, + drainer_num_partitions, + )) } fn get_master_pool(&self) -> &PgPool { self.router_store.get_master_pool() @@ -224,28 +234,26 @@ impl DataModelExt for data_models::MerchantStorageScheme { pub(crate) fn diesel_error_to_data_error( diesel_error: &diesel_models::errors::DatabaseError, -) -> data_models::errors::StorageError { +) -> StorageError { match diesel_error { diesel_models::errors::DatabaseError::DatabaseConnectionError => { - data_models::errors::StorageError::DatabaseConnectionError + StorageError::DatabaseConnectionError } diesel_models::errors::DatabaseError::NotFound => { - data_models::errors::StorageError::ValueNotFound("Value not found".to_string()) - } - diesel_models::errors::DatabaseError::UniqueViolation => { - data_models::errors::StorageError::DuplicateValue { - entity: "entity ", - key: None, - } + StorageError::ValueNotFound("Value not found".to_string()) } + diesel_models::errors::DatabaseError::UniqueViolation => StorageError::DuplicateValue { + entity: "entity ", + key: None, + }, diesel_models::errors::DatabaseError::NoFieldsToUpdate => { - data_models::errors::StorageError::DatabaseError("No fields to update".to_string()) + StorageError::DatabaseError("No fields to update".to_string()) } diesel_models::errors::DatabaseError::QueryGenerationFailed => { - data_models::errors::StorageError::DatabaseError("Query generation failed".to_string()) + StorageError::DatabaseError("Query generation failed".to_string()) } diesel_models::errors::DatabaseError::Others => { - data_models::errors::StorageError::DatabaseError("Others".to_string()) + StorageError::DatabaseError("Others".to_string()) } } } diff --git a/crates/test_utils/src/connector_auth.rs b/crates/test_utils/src/connector_auth.rs index a6c8fccd2472..3a6d2c7d376f 100644 --- a/crates/test_utils/src/connector_auth.rs +++ b/crates/test_utils/src/connector_auth.rs @@ -70,6 +70,9 @@ impl Default for ConnectorAuthentication { #[allow(dead_code)] impl ConnectorAuthentication { + /// # Panics + /// + /// Will panic if `CONNECTOR_AUTH_FILE_PATH` env is not set #[allow(clippy::expect_used)] pub fn new() -> Self { // Do `export CONNECTOR_AUTH_FILE_PATH="/hyperswitch/crates/router/tests/connectors/sample_auth.toml"` @@ -99,6 +102,9 @@ impl ConnectorAuthenticationMap { &self.0 } + /// # Panics + /// + /// Will panic if `CONNECTOR_AUTH_FILE_PATH` env is not set #[allow(clippy::expect_used)] pub fn new() -> Self { // Do `export CONNECTOR_AUTH_FILE_PATH="/hyperswitch/crates/router/tests/connectors/sample_auth.toml"` From ee888565853faf234803f07e69e49e7dbb06b4cc Mon Sep 17 00:00:00 2001 From: Sampras Lopes Date: Fri, 25 Aug 2023 02:56:22 +0530 Subject: [PATCH 04/10] fix(lints): fix cargo hack --- crates/router/src/services.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/router/src/services.rs b/crates/router/src/services.rs index c8e4ff7a0db0..3688dfc42dab 100644 --- a/crates/router/src/services.rs +++ b/crates/router/src/services.rs @@ -3,7 +3,9 @@ pub mod authentication; pub mod encryption; pub mod logger; -use data_models::errors::{StorageError, StorageResult}; +#[cfg(feature = "kms")] +use data_models::errors::StorageError; +use data_models::errors::StorageResult; use error_stack::{IntoReport, ResultExt}; #[cfg(feature = "kms")] use external_services::kms::{self, decrypt::KmsDecrypt}; From 364fdcb8eef0face155f8f233a626a6fc35ec632 Mon Sep 17 00:00:00 2001 From: Sanchith Hegde Date: Fri, 25 Aug 2023 11:44:53 +0530 Subject: [PATCH 05/10] ci(CI-pr): use current stable release for testing --- .github/workflows/CI-pr.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/CI-pr.yml b/.github/workflows/CI-pr.yml index a51ef804daeb..61aaf5951f25 100644 --- a/.github/workflows/CI-pr.yml +++ b/.github/workflows/CI-pr.yml @@ -290,7 +290,7 @@ jobs: - name: Install Rust uses: dtolnay/rust-toolchain@master with: - toolchain: stable 2 weeks ago + toolchain: stable components: clippy - name: Install cargo-hack From 03570d84354d1c3652da71bc9502e0fa830acf0d Mon Sep 17 00:00:00 2001 From: Sampras Lopes Date: Fri, 25 Aug 2023 13:45:33 +0530 Subject: [PATCH 06/10] chore(lints): fix clippy lints --- crates/router/src/db/configs.rs | 9 ++++++--- crates/router/src/db/connector_response.rs | 9 ++++++--- crates/router/src/db/dispute.rs | 9 ++++++--- crates/router/src/db/events.rs | 9 ++++++--- crates/router/src/db/locker_mock_up.rs | 9 ++++++--- crates/router/src/db/mandate.rs | 9 ++++++--- crates/router/src/db/merchant_connector_account.rs | 7 +++++-- crates/router/src/db/payment_intent.rs | 8 ++++++-- crates/router/src/db/payment_method.rs | 9 ++++++--- crates/router/src/db/refund.rs | 8 ++++++-- 10 files changed, 59 insertions(+), 27 deletions(-) diff --git a/crates/router/src/db/configs.rs b/crates/router/src/db/configs.rs index ac78207811a6..f7bc8e699d33 100644 --- a/crates/router/src/db/configs.rs +++ b/crates/router/src/db/configs.rs @@ -1,5 +1,5 @@ use diesel_models::configs::ConfigUpdateInternal; -use error_stack::IntoReport; +use error_stack::{IntoReport, ResultExt}; use storage_impl::redis::{ cache::{CacheKind, CONFIG_CACHE}, kv_store::RedisConnInterface, @@ -126,8 +126,11 @@ impl ConfigInterface for MockDb { let mut configs = self.configs.lock().await; let config_new = storage::Config { - #[allow(clippy::as_conversions)] - id: configs.len() as i32, + id: configs + .len() + .try_into() + .into_report() + .change_context(errors::StorageError::MockDbError)?, key: config.key, config: config.config, }; diff --git a/crates/router/src/db/connector_response.rs b/crates/router/src/db/connector_response.rs index bf298fa4c0a9..6e5f22b4c8bc 100644 --- a/crates/router/src/db/connector_response.rs +++ b/crates/router/src/db/connector_response.rs @@ -1,4 +1,4 @@ -use error_stack::IntoReport; +use error_stack::{IntoReport, ResultExt}; use super::{MockDb, Store}; use crate::{ @@ -88,8 +88,11 @@ impl ConnectorResponseInterface for MockDb { ) -> CustomResult { let mut connector_response = self.connector_response.lock().await; let response = storage::ConnectorResponse { - #[allow(clippy::as_conversions)] - id: connector_response.len() as i32, + id: connector_response + .len() + .try_into() + .into_report() + .change_context(errors::StorageError::MockDbError)?, payment_id: new.payment_id, merchant_id: new.merchant_id, attempt_id: new.attempt_id, diff --git a/crates/router/src/db/dispute.rs b/crates/router/src/db/dispute.rs index 212e47a507d5..aa0c92bc77b8 100644 --- a/crates/router/src/db/dispute.rs +++ b/crates/router/src/db/dispute.rs @@ -1,4 +1,4 @@ -use error_stack::IntoReport; +use error_stack::{IntoReport, ResultExt}; use super::{MockDb, Store}; use crate::{ @@ -147,8 +147,11 @@ impl DisputeInterface for MockDb { let now = common_utils::date_time::now(); let new_dispute = storage::Dispute { - #[allow(clippy::as_conversions)] - id: locked_disputes.len() as i32, + id: locked_disputes + .len() + .try_into() + .into_report() + .change_context(errors::StorageError::MockDbError)?, dispute_id: dispute.dispute_id, amount: dispute.amount, currency: dispute.currency, diff --git a/crates/router/src/db/events.rs b/crates/router/src/db/events.rs index 5e2a4310eaa5..67819a9e92e6 100644 --- a/crates/router/src/db/events.rs +++ b/crates/router/src/db/events.rs @@ -1,4 +1,4 @@ -use error_stack::IntoReport; +use error_stack::{IntoReport, ResultExt}; use super::{MockDb, Store}; use crate::{ @@ -52,8 +52,11 @@ impl EventInterface for MockDb { let now = common_utils::date_time::now(); let stored_event = storage::Event { - #[allow(clippy::as_conversions)] - id: locked_events.len() as i32, + id: locked_events + .len() + .try_into() + .into_report() + .change_context(errors::StorageError::MockDbError)?, event_id: event.event_id, event_type: event.event_type, event_class: event.event_class, diff --git a/crates/router/src/db/locker_mock_up.rs b/crates/router/src/db/locker_mock_up.rs index 96c74d8b8dde..d476a26094ac 100644 --- a/crates/router/src/db/locker_mock_up.rs +++ b/crates/router/src/db/locker_mock_up.rs @@ -1,4 +1,4 @@ -use error_stack::IntoReport; +use error_stack::{IntoReport, ResultExt}; use super::{MockDb, Store}; use crate::{ @@ -84,8 +84,11 @@ impl LockerMockUpInterface for MockDb { } let created_locker = storage::LockerMockUp { - #[allow(clippy::as_conversions)] - id: locked_lockers.len() as i32, + id: locked_lockers + .len() + .try_into() + .into_report() + .change_context(errors::StorageError::MockDbError)?, card_id: new.card_id, external_id: new.external_id, card_fingerprint: new.card_fingerprint, diff --git a/crates/router/src/db/mandate.rs b/crates/router/src/db/mandate.rs index 02d04a040d5c..192b9c43482f 100644 --- a/crates/router/src/db/mandate.rs +++ b/crates/router/src/db/mandate.rs @@ -1,4 +1,4 @@ -use error_stack::IntoReport; +use error_stack::{IntoReport, ResultExt}; use super::{MockDb, Store}; use crate::{ @@ -221,8 +221,11 @@ impl MandateInterface for MockDb { ) -> CustomResult { let mut mandates = self.mandates.lock().await; let mandate = storage::Mandate { - #[allow(clippy::as_conversions)] - id: mandates.len() as i32, + id: mandates + .len() + .try_into() + .into_report() + .change_context(errors::StorageError::MockDbError)?, mandate_id: mandate_new.mandate_id.clone(), customer_id: mandate_new.customer_id, merchant_id: mandate_new.merchant_id, diff --git a/crates/router/src/db/merchant_connector_account.rs b/crates/router/src/db/merchant_connector_account.rs index 2654d1ceaf22..9863d93eea56 100644 --- a/crates/router/src/db/merchant_connector_account.rs +++ b/crates/router/src/db/merchant_connector_account.rs @@ -542,8 +542,11 @@ impl MerchantConnectorAccountInterface for MockDb { ) -> CustomResult { let mut accounts = self.merchant_connector_accounts.lock().await; let account = storage::MerchantConnectorAccount { - #[allow(clippy::as_conversions)] - id: accounts.len() as i32, + id: accounts + .len() + .try_into() + .into_report() + .change_context(errors::StorageError::MockDbError)?, merchant_id: t.merchant_id, connector_name: t.connector_name, connector_account_details: t.connector_account_details.into(), diff --git a/crates/router/src/db/payment_intent.rs b/crates/router/src/db/payment_intent.rs index dc1c30b3264e..33f561ed24d4 100644 --- a/crates/router/src/db/payment_intent.rs +++ b/crates/router/src/db/payment_intent.rs @@ -5,6 +5,7 @@ use data_models::payments::payment_intent::{ use data_models::payments::{ payment_attempt::PaymentAttempt, payment_intent::PaymentIntentFetchConstraints, }; +use error_stack::{IntoReport, ResultExt}; use super::MockDb; #[cfg(feature = "olap")] @@ -56,8 +57,11 @@ impl PaymentIntentInterface for MockDb { let mut payment_intents = self.payment_intents.lock().await; let time = common_utils::date_time::now(); let payment_intent = PaymentIntent { - #[allow(clippy::as_conversions)] - id: payment_intents.len() as i32, + id: payment_intents + .len() + .try_into() + .into_report() + .change_context(errors::DataStorageError::MockDbError)?, payment_id: new.payment_id, merchant_id: new.merchant_id, status: new.status, diff --git a/crates/router/src/db/payment_method.rs b/crates/router/src/db/payment_method.rs index 3f6dd60eb2e2..719d6afcd77e 100644 --- a/crates/router/src/db/payment_method.rs +++ b/crates/router/src/db/payment_method.rs @@ -1,5 +1,5 @@ use diesel_models::payment_method::PaymentMethodUpdateInternal; -use error_stack::IntoReport; +use error_stack::{IntoReport, ResultExt}; use super::{MockDb, Store}; use crate::{ @@ -134,8 +134,11 @@ impl PaymentMethodInterface for MockDb { let mut payment_methods = self.payment_methods.lock().await; let payment_method = storage::PaymentMethod { - #[allow(clippy::as_conversions)] - id: payment_methods.len() as i32, + id: payment_methods + .len() + .try_into() + .into_report() + .change_context(errors::StorageError::MockDbError)?, customer_id: payment_method_new.customer_id, merchant_id: payment_method_new.merchant_id, payment_method_id: payment_method_new.payment_method_id, diff --git a/crates/router/src/db/refund.rs b/crates/router/src/db/refund.rs index 4ef50d455272..cb7a46dfb450 100644 --- a/crates/router/src/db/refund.rs +++ b/crates/router/src/db/refund.rs @@ -2,6 +2,7 @@ use std::collections::HashSet; use diesel_models::{errors::DatabaseError, refund::RefundUpdateInternal}; +use error_stack::{IntoReport, ResultExt}; use super::MockDb; use crate::{ @@ -735,8 +736,11 @@ impl RefundInterface for MockDb { let current_time = common_utils::date_time::now(); let refund = storage_types::Refund { - #[allow(clippy::as_conversions)] - id: refunds.len() as i32, + id: refunds + .len() + .try_into() + .into_report() + .change_context(errors::StorageError::MockDbError)?, internal_reference_id: new.internal_reference_id, refund_id: new.refund_id, payment_id: new.payment_id, From 0d46c6a30ccecd09ef3f38a7f52191edb40cbc54 Mon Sep 17 00:00:00 2001 From: Sampras Lopes Date: Fri, 25 Aug 2023 14:22:57 +0530 Subject: [PATCH 07/10] revert ci to use 2 weeks stable --- .github/workflows/CI-pr.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/CI-pr.yml b/.github/workflows/CI-pr.yml index 61aaf5951f25..a51ef804daeb 100644 --- a/.github/workflows/CI-pr.yml +++ b/.github/workflows/CI-pr.yml @@ -290,7 +290,7 @@ jobs: - name: Install Rust uses: dtolnay/rust-toolchain@master with: - toolchain: stable + toolchain: stable 2 weeks ago components: clippy - name: Install cargo-hack From f258d11bf417cad7246a491069d2fe21cb8c5f01 Mon Sep 17 00:00:00 2001 From: Sampras Lopes Date: Fri, 25 Aug 2023 14:31:56 +0530 Subject: [PATCH 08/10] reverting `[target.'cfg(all())']` change --- .cargo/config.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.cargo/config.toml b/.cargo/config.toml index 806b60a9a697..66da12a13b11 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -1,4 +1,4 @@ -[build] +[target.'cfg(all())'] rustflags = [ "-Funsafe_code", "-Wclippy::as_conversions", From cbc51fa99553f2a7f1949a83daf22a28d8f5c4b9 Mon Sep 17 00:00:00 2001 From: Sanchith Hegde <22217505+SanchithHegde@users.noreply.github.com> Date: Fri, 25 Aug 2023 14:43:26 +0530 Subject: [PATCH 09/10] fix(selenium): use raw string literal without `#` --- crates/test_utils/tests/connectors/selenium.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/test_utils/tests/connectors/selenium.rs b/crates/test_utils/tests/connectors/selenium.rs index 370c1ffe9874..28cb5bad0ed3 100644 --- a/crates/test_utils/tests/connectors/selenium.rs +++ b/crates/test_utils/tests/connectors/selenium.rs @@ -832,7 +832,7 @@ fn get_chrome_profile_path() -> Result { fp.join(&MAIN_SEPARATOR.to_string()) }) .unwrap(); - base_path.push_str("/Library/Application\\ Support/Google/Chrome/Default"); //Issue: 1573 + base_path.push_str(r"/Library/Application\ Support/Google/Chrome/Default"); //Issue: 1573 Ok(base_path) } From 2616f0592ae703dc3125556ae9839aadfb960aa9 Mon Sep 17 00:00:00 2001 From: Sampras Lopes Date: Fri, 25 Aug 2023 14:47:03 +0530 Subject: [PATCH 10/10] chore(lints): remove unused lint suppression --- crates/drainer/src/services.rs | 1 - crates/router/src/routes/app.rs | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/drainer/src/services.rs b/crates/drainer/src/services.rs index 5982bde45b7b..6edec31f26d7 100644 --- a/crates/drainer/src/services.rs +++ b/crates/drainer/src/services.rs @@ -16,7 +16,6 @@ pub struct StoreConfig { } impl Store { - #[allow(clippy::expect_used)] pub async fn new(config: &crate::settings::Settings, test_transaction: bool) -> Self { Self { master_pool: diesel_make_pg_pool( diff --git a/crates/router/src/routes/app.rs b/crates/router/src/routes/app.rs index 8b61c94ea35b..eb1457753c44 100644 --- a/crates/router/src/routes/app.rs +++ b/crates/router/src/routes/app.rs @@ -62,7 +62,6 @@ impl AppState { /// # Panics /// /// Panics if Store can't be created or JWE decryption fails - #[allow(clippy::expect_used)] pub async fn with_storage( conf: settings::Settings, storage_impl: StorageImpl, @@ -73,6 +72,7 @@ impl AppState { let testable = storage_impl == StorageImpl::PostgresqlTest; let store: Box = match storage_impl { StorageImpl::Postgresql | StorageImpl::PostgresqlTest => Box::new( + #[allow(clippy::expect_used)] get_store(&conf, shut_down_signal, testable) .await .expect("Failed to create store"), @@ -81,6 +81,7 @@ impl AppState { }; #[cfg(feature = "kms")] + #[allow(clippy::expect_used)] let kms_secrets = settings::ActiveKmsSecrets { jwekey: conf.jwekey.clone().into(), }