feat(payment_methods): added kv support for payment_methods table #4311
Conversation
| impl PaymentMethodNew { | ||
| pub fn new_payment_method(&self) -> PaymentMethod{ | ||
| PaymentMethod{ | ||
| id: 0i32, | ||
| customer_id: self.customer_id.clone(), | ||
| merchant_id: self.merchant_id.clone(), | ||
| payment_method_id: self.payment_method_id.clone(), | ||
| locker_id: self.locker_id.clone(), | ||
| accepted_currency: self.accepted_currency.clone(), | ||
| scheme: self.scheme.clone(), | ||
| token: self.token.clone(), | ||
| cardholder_name: self.cardholder_name.clone(), | ||
| issuer_name: self.issuer_name.clone(), | ||
| issuer_country: self.issuer_country.clone(), | ||
| payer_country: self.payer_country.clone(), | ||
| is_stored: self.is_stored.clone(), | ||
| swift_code: self.swift_code.clone(), | ||
| direct_debit_token: self.direct_debit_token.clone(), | ||
| created_at: self.created_at.clone(), | ||
| last_modified: self.last_modified.clone(), | ||
| payment_method: self.payment_method.clone(), | ||
| payment_method_type: self.payment_method_type.clone(), | ||
| payment_method_issuer: self.payment_method_issuer.clone(), | ||
| payment_method_issuer_code: self.payment_method_issuer_code.clone(), | ||
| metadata: self.metadata.clone(), | ||
| payment_method_data: self.payment_method_data.clone(), | ||
| last_used_at: self.last_used_at.clone(), | ||
| connector_mandate_details: self.connector_mandate_details.clone(), | ||
| customer_acceptance: self.customer_acceptance.clone(), | ||
| status: self.status.clone(), | ||
|
|
||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
You can use impl From for this
|
|
||
| pub async fn update_with_payment_method_id_drainer( | ||
| self, | ||
| conn: &PgPooledConn, | ||
| payment_method: payment_method::PaymentMethodUpdateInternal, | ||
| ) -> StorageResult<Self> { | ||
| match generics::generic_update_with_unique_predicate_get_result::< | ||
| <Self as HasTable>::Table, | ||
| _, | ||
| _, | ||
| _, | ||
| >( | ||
| conn, | ||
| dsl::payment_method_id.eq(self.payment_method_id.to_owned()), | ||
| payment_method, | ||
| ) | ||
| .await | ||
| { | ||
| Err(error) => match error.current_context() { | ||
| errors::DatabaseError::NoFieldsToUpdate => Ok(self), | ||
| _ => Err(error), | ||
| }, | ||
| result => result, | ||
| } | ||
| } |
There was a problem hiding this comment.
takes PaymentMethodUpdateInternal instead of PaymentMethodUpdate
There was a problem hiding this comment.
We can modify the previous function to do the same
| let storage_scheme = match storage_scheme { | ||
| Some(v) => v, | ||
| None => { | ||
| let account = db.find_merchant_account_by_merchant_id(&merchant_id, &key_store) | ||
| .await | ||
| .to_not_found_response(errors::ApiErrorResponse::MerchantAccountNotFound)?; | ||
| account.storage_scheme | ||
| } | ||
| }; |
There was a problem hiding this comment.
Why is the redundant DB call find_merchant_account needed?
There was a problem hiding this comment.
in case calling function doesnt have merchant account, we need storage scheme
There was a problem hiding this comment.
No we have merchant_id getting passed here anyway cant we instead pass merchant_account here and use it.
| let account = state | ||
| .store | ||
| .find_merchant_account_by_merchant_id(&payment_intent.merchant_id, &merchant_key_store) | ||
| .await | ||
| .to_not_found_response(errors::ApiErrorResponse::MerchantAccountNotFound)?; |
There was a problem hiding this comment.
need storage_scheme
There was a problem hiding this comment.
You can pass it from the parent function
| }, | ||
| }; | ||
|
|
||
| match kv_wrapper::<diesel_models::PaymentMethod, _, _>( |
There was a problem hiding this comment.
Partiotionkey in kv_wrapper function is hardcoded to MerchantIdPaymentIdCombination. You can probably add MerchantIdCustomerId combination and avoid writing mid and cid prefixes manually.
| payment_method_update: storage::PaymentMethodUpdate, | ||
| storage_scheme: MerchantStorageScheme, | ||
| ) -> CustomResult<storage::PaymentMethod, errors::StorageError> { | ||
| let conn = connection::pg_connection_write(self).await?; |
There was a problem hiding this comment.
This connection is redundant when we are using KV we can move it undr PostgresOnly
| async fn delete_payment_method_by_merchant_id_payment_method_id( | ||
| &self, | ||
| merchant_id: &str, | ||
| payment_method_id: &str, | ||
| ) -> CustomResult<storage::PaymentMethod, errors::StorageError> { | ||
| let conn = connection::pg_connection_write(self).await?; | ||
| storage::PaymentMethod::delete_by_merchant_id_payment_method_id( | ||
| &conn, | ||
| merchant_id, | ||
| payment_method_id, | ||
| ) | ||
| .await | ||
| .map_err(|error| report!(errors::StorageError::from(error))) | ||
| } |
There was a problem hiding this comment.
Let's say when I do an insert_payment_method and data is not inserted into the drainer and immediately I do a delete this will cause error and data will still be in redis.
There was a problem hiding this comment.
yes, we have to start supporting delete in kv as well (this is the same case as find all as well right where we are eventually consistent)
| @@ -123,7 +425,7 @@ impl PaymentMethodInterface for Store { | |||
| ) -> CustomResult<storage::PaymentMethod, errors::StorageError> { | |||
| let conn = connection::pg_connection_write(self).await?; | |||
| payment_method | |||
| .update_with_payment_method_id(&conn, payment_method_update) | |||
| .update_with_payment_method_id(&conn, diesel_models::PaymentMethodUpdateInternal::from(payment_method_update)) | |||
There was a problem hiding this comment.
You can do payment_method_update.into()
| let merchant_id = payment_method.merchant_id.clone(); | ||
| let customer_id = payment_method.customer_id.clone(); | ||
| let key = PartitionKey::MerchantIdCustomerId {merchant_id : &merchant_id, customer_id : &customer_id}; | ||
| let key_str = format!("{}", key); |
There was a problem hiding this comment.
You dont have to do this you can just accept the enum in KVWrapper and while pushing to drainer it will use the Display implementation
There was a problem hiding this comment.
kv_wrapper still is accepting the enum, the rest of the functions are expecting &str, either in to_redis_failed_response or reverse lookup field
dracarys18
changed the title
| let merchant_id = this.merchant_id.clone(); | ||
| let payment_id = this.payment_id.clone(); |
There was a problem hiding this comment.
Seems like a redundant clone, We can just do &this.merchant_id below
There was a problem hiding this comment.
this is moved inside redis_entry which is needed by kv wrapper, couldnt find a way to match redis entry lifetime (since its not a reference)
| let merchant_id = new.merchant_id.clone(); | ||
| let payment_id = new.payment_id.clone(); |
| let merchant_id = this.merchant_id.clone(); | ||
| let payment_id = this.payment_id.clone(); |
| let merchant_id = new_payout_attempt.merchant_id.clone(); | ||
| let payout_attempt_id = new_payout_attempt.payout_id.clone(); |
akshay-97
force-pushed
the
payment_method_kv
branch
from
0127088 to
1697c02
Compare
dracarys18
added
A-core
akshay-97
force-pushed
the
payment_method_kv
branch
from
e672492 to
df15cef
Compare
SanchithHegde
removed
the
S-waiting-on-review
Type of Change
Description
Add kv support for payment_method table, so that writes and updates dont go to master db
Additional Changes
Motivation and Context
Since payment flows can write to payment method table, this entity needs to be supported in KV
How did you test it?
enable kv for test merchant and hit /payments/ (create) and payments/:id/confirm to test if payment method is
updated in kv, run the drainer to see if payment method is getting drained to db properly
create payment method with merchant redis_kv enabled
curl --location 'http://localhost:8080/payment_methods' \ --header 'Content-Type: application/json' \ --header 'Accept: application/json' \ --header 'api-key: ***' \ --data '{ "payment_method": "card", "payment_method_type": "credit", "payment_method_issuer": "Visa", "card": { "card_number": "4242424242424242", "card_exp_month": "10", "card_exp_year": "25", "card_holder_name": "John Doe" }, "customer_id": "***", "metadata": { "city": "NY", "unit": "245" } }'response:
{ "merchant_id": "merchant_1708344854", "customer_id": "cus_5BDVuipoqQvizwy8t5mu", "payment_method_id": "pm_8i5F6ovRxfj24y7qMF9Z", "payment_method": "card", "payment_method_type": "credit", "card": { "scheme": null, "issuer_country": null, "last4_digits": "4242", "expiry_month": "10", "expiry_year": "25", "card_token": null, "card_holder_name": "John Doe", "card_fingerprint": null, "nick_name": null, "card_network": null, "card_isin": "424242", "card_issuer": null, "card_type": null, "saved_to_locker": true }, "recurring_enabled": false, "installment_payment_enabled": false, "payment_experience": [ "redirect_to_url" ], "metadata": { "city": "NY", "unit": "245" }, "created": "2024-04-10T08:28:36.412Z", "last_used_at": "2024-04-10T08:28:36.412Z" }db query to check if drainer executed:
`select * from payment_methods where payment_method_id = 'pm_8i5F6ovRxfj24y7qMF9Z';
id | 1
customer_id | cus_5BDVuipoqQvizwy8t5mu
merchant_id | merchant_1708344854
payment_method_id | pm_8i5F6ovRxfj24y7qMF9Z
accepted_currency |
scheme |
token |
cardholder_name |
issuer_name |
issuer_country |
payer_country |
is_stored |
swift_code |
direct_debit_token |
created_at | 2024-04-10 08:28:36.418277
last_modified | 2024-04-10 08:28:36.418277
payment_method | card
payment_method_type | credit
payment_method_issuer | Visa
payment_method_issuer_code |
metadata | {"city":"NY","unit":"245"}
payment_method_data | \x7cf39208264281f7dd0c2cd5cc703043692702c01611ae80ba9fe6a31b71b50246ca6d014b5272e383b728be38ff33d50dd1bbd156d7fcf901870a3b84a3b099e3a754331a99349e2dee1020f24b7644e579aba3296e9f8aea5d0a4605bc2e8a49cd11901bf39c43eab28e930e5cb9063f530cea81ab388121f6b9b30f10d99965f49304ef10117b72aff72517754596d141e6ea03cc4265d3d5372c1bcf335514d492c92f79ed08e10b56e53786361125e286c4cf2e905858eb452872d7cc784f6286170ba1bacf3bd93d3d30a84ef9a678dd2030f19a17a2cd06218f837b61b24b09be00b5c51d1ff033520329be71bd7c3c95fe97ea77388ea3ed39ef11d4c04e544046bceec2bc538215
locker_id | card_0n0juVXDx9zcWtMLRAkf
last_used_at | 2024-04-10 08:28:36.418277
connector_mandate_details |
customer_acceptance |
status | active
network_transaction_id |
`
curl to list payment methods by customer
curl --location 'http://localhost:8080/customers/cus_5BDVuipoqQvizwy8t5mu/payment_methods' \ --header 'Accept: application/json' \ --header 'api-key:***'testing in sbx can be done similarly:
Checklist
cargo +nightly fmt --allcargo clippy