From d03a0b36c754a2c93fdbb376e3b697bceb960555 Mon Sep 17 00:00:00 2001 From: Narayan Bhat <48803246+Narayanbhat166@users.noreply.github.com> Date: Thu, 29 Aug 2024 16:34:08 +0530 Subject: [PATCH] refactor(customers): add offset and limit to customers list (#5741) --- crates/api_models/src/customers.rs | 10 +++++ crates/api_models/src/events.rs | 4 +- crates/diesel_models/src/query/customers.rs | 10 ++++- crates/router/src/consts.rs | 2 + crates/router/src/core/customers.rs | 15 ++++++- crates/router/src/core/locker_migration.rs | 10 ++++- crates/router/src/db/customers.rs | 49 +++++++++++++++++---- crates/router/src/db/kafka_store.rs | 3 +- crates/router/src/routes/customers.rs | 12 +++-- crates/router/src/types/api/customers.rs | 3 +- 10 files changed, 100 insertions(+), 18 deletions(-) diff --git a/crates/api_models/src/customers.rs b/crates/api_models/src/customers.rs index 6a9f2b64791a..e689e66489c2 100644 --- a/crates/api_models/src/customers.rs +++ b/crates/api_models/src/customers.rs @@ -48,6 +48,16 @@ pub struct CustomerRequest { pub metadata: Option, } +#[derive(Debug, Default, Clone, Deserialize, Serialize, ToSchema)] +pub struct CustomerListRequest { + /// Offset + #[schema(example = 32)] + pub offset: Option, + /// Limit + #[schema(example = 32)] + pub limit: Option, +} + #[cfg(all(any(feature = "v1", feature = "v2"), not(feature = "customer_v2")))] impl CustomerRequest { pub fn get_merchant_reference_id(&self) -> Option { diff --git a/crates/api_models/src/events.rs b/crates/api_models/src/events.rs index a0808c0c24be..58727a46cf38 100644 --- a/crates/api_models/src/events.rs +++ b/crates/api_models/src/events.rs @@ -19,6 +19,7 @@ use common_utils::{ impl_api_event_type, }; +use crate::customers::CustomerListRequest; #[allow(unused_imports)] use crate::{ admin::*, @@ -131,7 +132,8 @@ impl_api_event_type!( GetDisputeMetricRequest, OrganizationResponse, OrganizationRequest, - OrganizationId + OrganizationId, + CustomerListRequest ) ); diff --git a/crates/diesel_models/src/query/customers.rs b/crates/diesel_models/src/query/customers.rs index 337679484c9c..e4e064e56486 100644 --- a/crates/diesel_models/src/query/customers.rs +++ b/crates/diesel_models/src/query/customers.rs @@ -23,6 +23,11 @@ impl CustomerNew { } } +pub struct CustomerListConstraints { + pub limit: i64, + pub offset: Option, +} + // #[cfg(all(feature = "v2", feature = "customer_v2"))] impl Customer { #[cfg(all(feature = "v2", feature = "customer_v2"))] @@ -57,12 +62,13 @@ impl Customer { pub async fn list_by_merchant_id( conn: &PgPooledConn, merchant_id: &id_type::MerchantId, + constraints: CustomerListConstraints, ) -> StorageResult> { generics::generic_filter::<::Table, _, _, _>( conn, dsl::merchant_id.eq(merchant_id.to_owned()), - None, - None, + Some(constraints.limit), + constraints.offset, Some(dsl::created_at), ) .await diff --git a/crates/router/src/consts.rs b/crates/router/src/consts.rs index 5a7070c2700e..d7afe8dafde5 100644 --- a/crates/router/src/consts.rs +++ b/crates/router/src/consts.rs @@ -32,6 +32,8 @@ pub const DEFAULT_SESSION_EXPIRY: i64 = 15 * 60; /// The length of a merchant fingerprint secret pub const FINGERPRINT_SECRET_LENGTH: usize = 64; +pub const DEFAULT_LIST_API_LIMIT: u16 = 10; + // String literals pub(crate) const UNSUPPORTED_ERROR_MESSAGE: &str = "Unsupported response type"; pub(crate) const LOW_BALANCE_ERROR_MESSAGE: &str = "Insufficient balance in the payment method"; diff --git a/crates/router/src/core/customers.rs b/crates/router/src/core/customers.rs index 4e45e48cfaad..9aad5061e371 100644 --- a/crates/router/src/core/customers.rs +++ b/crates/router/src/core/customers.rs @@ -497,11 +497,24 @@ pub async fn list_customers( merchant_id: id_type::MerchantId, _profile_id_list: Option>, key_store: domain::MerchantKeyStore, + request: customers::CustomerListRequest, ) -> errors::CustomerResponse> { let db = state.store.as_ref(); + let customer_list_constraints = crate::db::customers::CustomerListConstraints { + limit: request + .limit + .unwrap_or(crate::consts::DEFAULT_LIST_API_LIMIT), + offset: request.offset, + }; + let domain_customers = db - .list_customers_by_merchant_id(&(&state).into(), &merchant_id, &key_store) + .list_customers_by_merchant_id( + &(&state).into(), + &merchant_id, + &key_store, + customer_list_constraints, + ) .await .switch()?; diff --git a/crates/router/src/core/locker_migration.rs b/crates/router/src/core/locker_migration.rs index 9e633f224c04..470194c1ee25 100644 --- a/crates/router/src/core/locker_migration.rs +++ b/crates/router/src/core/locker_migration.rs @@ -30,6 +30,8 @@ pub async fn rust_locker_migration( state: SessionState, merchant_id: &id_type::MerchantId, ) -> CustomResult, errors::ApiErrorResponse> { + use crate::db::customers::CustomerListConstraints; + let db = state.store.as_ref(); let key_manager_state = &(&state).into(); let key_store = state @@ -48,8 +50,14 @@ pub async fn rust_locker_migration( .to_not_found_response(errors::ApiErrorResponse::MerchantAccountNotFound) .change_context(errors::ApiErrorResponse::InternalServerError)?; + // Handle cases where the number of customers is greater than the limit + let constraints = CustomerListConstraints { + limit: u16::MAX, + offset: None, + }; + let domain_customers = db - .list_customers_by_merchant_id(key_manager_state, merchant_id, &key_store) + .list_customers_by_merchant_id(key_manager_state, merchant_id, &key_store, constraints) .await .change_context(errors::ApiErrorResponse::InternalServerError)?; diff --git a/crates/router/src/db/customers.rs b/crates/router/src/db/customers.rs index f2688499cc8c..525f84b0f371 100644 --- a/crates/router/src/db/customers.rs +++ b/crates/router/src/db/customers.rs @@ -1,4 +1,5 @@ use common_utils::{ext_traits::AsyncExt, id_type, types::keymanager::KeyManagerState}; +use diesel_models::query::customers::CustomerListConstraints as DieselCustomerListConstraints; use error_stack::ResultExt; #[cfg(all(any(feature = "v1", feature = "v2"), not(feature = "customer_v2")))] use futures::future::try_join_all; @@ -18,6 +19,20 @@ use crate::{ }, }; +pub struct CustomerListConstraints { + pub limit: u16, + pub offset: Option, +} + +impl From for DieselCustomerListConstraints { + fn from(value: CustomerListConstraints) -> Self { + Self { + limit: i64::from(value.limit), + offset: value.offset.map(i64::from), + } + } +} + #[async_trait::async_trait] pub trait CustomerInterface where @@ -90,6 +105,7 @@ where state: &KeyManagerState, merchant_id: &id_type::MerchantId, key_store: &domain::MerchantKeyStore, + constraints: CustomerListConstraints, ) -> CustomResult, errors::StorageError>; async fn insert_customer( @@ -518,13 +534,20 @@ mod storage { state: &KeyManagerState, merchant_id: &id_type::MerchantId, key_store: &domain::MerchantKeyStore, + constraints: super::CustomerListConstraints, ) -> CustomResult, errors::StorageError> { let conn = connection::pg_connection_read(self).await?; - let encrypted_customers = - storage_types::Customer::list_by_merchant_id(&conn, merchant_id) - .await - .map_err(|error| report!(errors::StorageError::from(error)))?; + let customer_list_constraints = + diesel_models::query::customers::CustomerListConstraints::from(constraints); + + let encrypted_customers = storage_types::Customer::list_by_merchant_id( + &conn, + merchant_id, + customer_list_constraints, + ) + .await + .map_err(|error| report!(errors::StorageError::from(error)))?; let customers = try_join_all(encrypted_customers.into_iter().map( |encrypted_customer| async { @@ -1059,13 +1082,20 @@ mod storage { state: &KeyManagerState, merchant_id: &id_type::MerchantId, key_store: &domain::MerchantKeyStore, + constraints: super::CustomerListConstraints, ) -> CustomResult, errors::StorageError> { let conn = connection::pg_connection_read(self).await?; - let encrypted_customers = - storage_types::Customer::list_by_merchant_id(&conn, merchant_id) - .await - .map_err(|error| report!(errors::StorageError::from(error)))?; + let customer_list_constraints = + diesel_models::query::customers::CustomerListConstraints::from(constraints); + + let encrypted_customers = storage_types::Customer::list_by_merchant_id( + &conn, + merchant_id, + customer_list_constraints, + ) + .await + .map_err(|error| report!(errors::StorageError::from(error)))?; let customers = try_join_all(encrypted_customers.into_iter().map( |encrypted_customer| async { @@ -1250,6 +1280,7 @@ impl CustomerInterface for MockDb { state: &KeyManagerState, merchant_id: &id_type::MerchantId, key_store: &domain::MerchantKeyStore, + constraints: CustomerListConstraints, ) -> CustomResult, errors::StorageError> { let customers = self.customers.lock().await; @@ -1257,6 +1288,8 @@ impl CustomerInterface for MockDb { customers .iter() .filter(|customer| customer.merchant_id == *merchant_id) + .take(usize::from(constraints.limit)) + .skip(usize::try_from(constraints.offset.unwrap_or(0)).unwrap_or(0)) .map(|customer| async { customer .to_owned() diff --git a/crates/router/src/db/kafka_store.rs b/crates/router/src/db/kafka_store.rs index cb3f6db144e4..4f5fe169b5aa 100644 --- a/crates/router/src/db/kafka_store.rs +++ b/crates/router/src/db/kafka_store.rs @@ -449,9 +449,10 @@ impl CustomerInterface for KafkaStore { state: &KeyManagerState, merchant_id: &id_type::MerchantId, key_store: &domain::MerchantKeyStore, + constraints: super::customers::CustomerListConstraints, ) -> CustomResult, errors::StorageError> { self.diesel_store - .list_customers_by_merchant_id(state, merchant_id, key_store) + .list_customers_by_merchant_id(state, merchant_id, key_store, constraints) .await } diff --git a/crates/router/src/routes/customers.rs b/crates/router/src/routes/customers.rs index 20d6539bb076..ad74ec738266 100644 --- a/crates/router/src/routes/customers.rs +++ b/crates/router/src/routes/customers.rs @@ -112,20 +112,26 @@ pub async fn customers_retrieve( #[cfg(all(any(feature = "v1", feature = "v2"), not(feature = "customer_v2")))] #[instrument(skip_all, fields(flow = ?Flow::CustomersList))] -pub async fn customers_list(state: web::Data, req: HttpRequest) -> HttpResponse { +pub async fn customers_list( + state: web::Data, + req: HttpRequest, + query: web::Query, +) -> HttpResponse { let flow = Flow::CustomersList; + let payload = query.into_inner(); api::server_wrap( flow, state, &req, - (), - |state, auth, _, _| { + payload, + |state, auth, request, _| { list_customers( state, auth.merchant_account.get_id().to_owned(), None, auth.key_store, + request, ) }, auth::auth_type( diff --git a/crates/router/src/types/api/customers.rs b/crates/router/src/types/api/customers.rs index b72b364720dc..ac76960aab84 100644 --- a/crates/router/src/types/api/customers.rs +++ b/crates/router/src/types/api/customers.rs @@ -2,7 +2,8 @@ use api_models::customers; #[cfg(all(feature = "v2", feature = "customer_v2"))] pub use api_models::customers::GlobalId; pub use api_models::customers::{ - CustomerDeleteResponse, CustomerId, CustomerRequest, CustomerUpdateRequest, UpdateCustomerId, + CustomerDeleteResponse, CustomerId, CustomerListRequest, CustomerRequest, + CustomerUpdateRequest, UpdateCustomerId, }; #[cfg(all(feature = "v2", feature = "customer_v2"))] use hyperswitch_domain_models::customer;