refactor(v1v2): refactor database queries for v1 and v2 #7244

hrithikesh026 · 2025-02-11T11:53:19Z

Type of Change

Description

Other changes:

Introduced product type column in merchant_account
restructure payment_methods related v2 DB queries
drop auto increment id column from all possible v1 tables.
introduce varchar id column in merchant_account in v1.
justfile changes to run all v1 migrations first and then v2 migrations. (prefixed v2 migrations with 9 before copying to a common directory so that all v2 migration will run only after all v1 migrations are run).

Additional Changes

This PR modifies the API contract
This PR modifies the database schema
This PR modifies application configuration/environment variables

Motivation and Context

How did you test it?

Tested connector sanity.
No test cases.

Checklist

I formatted the code cargo +nightly fmt --all
I addressed lints thrown by cargo clippy
I reviewed the submitted code
I added unit tests for my changes where possible

…ueries_v1_v2

semanticdiff-com · 2025-02-11T11:53:21Z

Review changes with

Changed Files

File	Status
crates/diesel_models/src/payment_attempt.rs	78% smaller
crates/diesel_models/src/payment_method.rs	75% smaller
crates/diesel_models/src/business_profile.rs	67% smaller
crates/diesel_models/src/payment_intent.rs	60% smaller
crates/diesel_models/src/schema.rs	33% smaller
crates/router/src/db/configs.rs	24% smaller
crates/router/src/db/locker_mock_up.rs	15% smaller
crates/diesel_models/src/schema_v2.rs	4% smaller
crates/diesel_models/src/merchant_account.rs	3% smaller
crates/diesel_models/src/locker_mock_up.rs	1% smaller
crates/diesel_models/src/blocklist_lookup.rs	1% smaller
crates/diesel_models/src/configs.rs	1% smaller
crates/diesel_models/src/blocklist_fingerprint.rs	1% smaller
crates/common_enums/src/enums.rs	0% smaller
crates/common_enums/src/enums/accounts.rs	0% smaller
crates/diesel_models/drop_id.patch	Unsupported file format
crates/diesel_models/remove_id.patch	Unsupported file format
crates/diesel_models/src/merchant_connector_account.rs	0% smaller
crates/hyperswitch_domain_models/src/merchant_account.rs	0% smaller
crates/router/src/core/admin.rs	0% smaller
diesel.toml	Unsupported file format
justfile	Unsupported file format
migrations/2025-02-06-111828_drop_int_id_column_accross_database/down.sql	Unsupported file format
migrations/2025-02-06-111828_drop_int_id_column_accross_database/up.sql	Unsupported file format
migrations/2025-02-10-101701_recreate_varchar_id_column_for_merchant_account/down.sql	Unsupported file format
migrations/2025-02-10-101701_recreate_varchar_id_column_for_merchant_account/up.sql	Unsupported file format
migrations/2025-02-11-062329_add-product-type-column-merchant-account/down.sql	Unsupported file format
migrations/2025-02-11-062329_add-product-type-column-merchant-account/up.sql	Unsupported file format
v2_migrations/2024-08-23-112510_payment_methods_v2_db_changes/down.sql	Unsupported file format
v2_migrations/2024-08-23-112510_payment_methods_v2_db_changes/up.sql	Unsupported file format
v2_migrations/2024-08-28-081721_add_v2_columns/down.sql	Unsupported file format
v2_migrations/2024-08-28-081721_add_v2_columns/up.sql	Unsupported file format
v2_migrations/2024-08-28-081722_drop_not_null_constraints_on_v1_columns/down.sql	Unsupported file format
v2_migrations/2024-08-28-081722_drop_not_null_constraints_on_v1_columns/up.sql	Unsupported file format
v2_migrations/2024-08-28-081747_recreate_ids_for_v2/down.sql	Unsupported file format
v2_migrations/2024-08-28-081747_recreate_ids_for_v2/up.sql	Unsupported file format
v2_migrations/2024-08-28-081838_update_v2_primary_key_constraints/down.sql	Unsupported file format
v2_migrations/2024-08-28-081838_update_v2_primary_key_constraints/up.sql	Unsupported file format
v2_migrations/2024-08-28-081839_add_not_null_constraints_to_v2_columns/down.sql	Unsupported file format
v2_migrations/2024-08-28-081839_add_not_null_constraints_to_v2_columns/up.sql	Unsupported file format
v2_migrations/2025-01-13-081847_drop_v1_columns/down.sql	Unsupported file format
v2_migrations/2025-01-13-081847_drop_v1_columns/up.sql	Unsupported file format
v2_migrations/2025-01-13-081848_alter_column_types_for_v2/down.sql	Unsupported file format
v2_migrations/2025-01-13-081848_alter_column_types_for_v2/up.sql	Unsupported file format
v2_migrations/2025-01-13-181842_add_three_ds_decision_manager_config_in_profile/down.sql	Unsupported file format
v2_migrations/2025-01-13-181842_add_three_ds_decision_manager_config_in_profile/up.sql	Unsupported file format
v2_migrations/2025-01-29-103844_add_feature_metadata_in_merchant_connector_account/down.sql	Unsupported file format
v2_migrations/2025-01-29-103844_add_feature_metadata_in_merchant_connector_account/up.sql	Unsupported file format

jarnura · 2025-02-11T14:23:26Z

migrations/2025-02-06-111828_drop_int_id_column_accross_database/down.sql

+-- It must be ensured that the deployed version of the application includes the `id` column in any of its queries.
+-- Re-create the id column as this was used as the primary key with a different type
+------------------------ Merchant Account -----------------------
+ALTER TABLE merchant_account ADD COLUMN id SERIAL;


Can we make all these SERIAL to VARCHAR itself?

…ueries_v1_v2

Narayanbhat166 · 2025-02-17T06:24:44Z

crates/diesel_models/src/blocklist_fingerprint.rs

 pub struct BlocklistFingerprint {
-    #[serde(skip_serializing)]
-    pub id: i32,


we cannot remove id from here right? This will not be backwards compatible.

Yeah this will not be backwards compatible. Hence we need to go for a deployment with downtime. Instead of staggering.

Narayanbhat166 · 2025-02-17T06:28:43Z

crates/diesel_models/src/configs.rs

@@ -49,7 +47,6 @@ impl From<ConfigUpdate> for ConfigUpdateInternal {
 impl From<ConfigNew> for Config {
    fn from(config_new: ConfigNew) -> Self {
        Self {
-            id: 0i32,


we cannot remove this as well right? this will cause downtime

yes. We need to go with a downtime for this.

we can add this in the patch for now, anyways we will need to have some downtime when deploying for v2, let's please add this in patch for now

Narayanbhat166 · 2025-02-17T06:29:47Z

crates/diesel_models/src/merchant_account.rs

@@ -52,6 +52,8 @@ pub struct MerchantAccount {
    pub pm_collect_link_config: Option<serde_json::Value>,
    pub version: common_enums::ApiVersion,
    pub is_platform_account: bool,
+    pub id: Option<common_utils::id_type::MerchantId>,
+    pub product_type: Option<common_enums::MerchantProductType>,


can a merchant account be associated with only one product type? what if the merchant wants to use more than one product for the same merchant account?

That is not allowed. Once a merchant is assigned a product_type, it cannot be changed.

What I mean is, can a merchant be associated with more than one product type at moment, both orchestrator and recovery service. Should this be a vector / hashset instead of just one element?

No. One merchant can only belong to one product_type.

If the merchant wants to explore another product, they would need to create a new merchant_account.

This seems like a weird restriction to place though. I feel it's a valid use case to be able to use multiple products with the same merchant account.

Another question I have is about the API keys: is the merchant now expected to use different API keys in the same deployment of theirs, that uses our orchestration and vaulting products (or any two or more products for the matter)? Won't this become too much of a maintenance hassle for them?

Narayanbhat166 · 2025-02-17T06:32:40Z

crates/diesel_models/src/schema_v2.rs

@@ -5,7 +5,6 @@ diesel::table! {
    use crate::enums::diesel_exports::*;

    address (address_id) {
-        id -> Nullable<Int4>,


address table will not be present in v2, we can drop this

Will take this up in a separate pr since this is outside the scope of this pr.

Narayanbhat166 · 2025-02-17T07:27:46Z

crates/hyperswitch_domain_models/src/merchant_account.rs

@@ -764,6 +787,7 @@ impl super::behaviour::Conversion for MerchantAccount {
    async fn construct_new(self) -> CustomResult<Self::NewDstType, ValidationError> {
        let now = date_time::now();
        Ok(diesel_models::merchant_account::MerchantAccountNew {
+            id: None,


is this left None on purpose?

For now, we will only allow v1 application to retrive v2 merchants, Not the other way around

Chethan-rao · 2025-02-22T08:42:39Z

v2_migrations/2025-01-13-081847_drop_v1_columns/down.sql

+ALTER TABLE payment_methods
+    ADD COLUMN IF NOT EXISTS payment_method_id VARCHAR(64),
+    ADD COLUMN IF NOT EXISTS accepted_currency "Currency" [ ],
+    ADD COLUMN IF NOT EXISTS scheme VARCHAR(32),
+    ADD COLUMN IF NOT EXISTS token VARCHAR(128),
+    ADD COLUMN IF NOT EXISTS cardholder_name VARCHAR(255),
+    ADD COLUMN IF NOT EXISTS issuer_name VARCHAR(64),
+    ADD COLUMN IF NOT EXISTS issuer_country VARCHAR(64),
+    ADD COLUMN IF NOT EXISTS is_stored BOOLEAN,
+    ADD COLUMN IF NOT EXISTS direct_debit_token VARCHAR(128),
+    ADD COLUMN IF NOT EXISTS swift_code VARCHAR(32),
+    ADD COLUMN IF NOT EXISTS payment_method_issuer VARCHAR(128),
+    ADD COLUMN IF NOT EXISTS metadata JSON,
+    ADD COLUMN IF NOT EXISTS payment_method VARCHAR,
+    ADD COLUMN IF NOT EXISTS payment_method_type VARCHAR(64),
+    ADD COLUMN IF NOT EXISTS payment_method_issuer_code "PaymentMethodIssuerCode";


payer_country is not added back here while its being removed in up.sql?

…ueries_v1_v2

SanchithHegde

Just a few questions / nitpicks from my side. The nitpicks can be addressed separately, not a problem.

SanchithHegde · 2025-02-24T18:16:36Z

crates/common_enums/src/enums/accounts.rs

@@ -0,0 +1,28 @@
+use serde;


Nit: Unnecessary import, can be removed.

I am deriving serde::Deserialize and Serialize for enum ProductType

Yeah I'm aware, you can directly use it in the derive() line, you don't need the explicit use serde; import.

SanchithHegde · 2025-02-24T18:17:11Z

crates/common_enums/src/enums/accounts.rs

+#[derive(
+    Default,
+    Clone,
+    Debug,


Nit: This enum can also derive Copy.

SanchithHegde · 2025-02-24T18:22:30Z

crates/diesel_models/src/merchant_account.rs

@@ -52,6 +52,8 @@ pub struct MerchantAccount {
    pub pm_collect_link_config: Option<serde_json::Value>,
    pub version: common_enums::ApiVersion,
    pub is_platform_account: bool,
+    pub id: Option<common_utils::id_type::MerchantId>,
+    pub product_type: Option<common_enums::MerchantProductType>,


This seems like a weird restriction to place though. I feel it's a valid use case to be able to use multiple products with the same merchant account.

Another question I have is about the API keys: is the merchant now expected to use different API keys in the same deployment of theirs, that uses our orchestration and vaulting products (or any two or more products for the matter)? Won't this become too much of a maintenance hassle for them?

SanchithHegde · 2025-02-24T18:29:38Z

crates/hyperswitch_domain_models/src/merchant_account.rs

@@ -317,6 +324,7 @@ impl From<MerchantAccountUpdate> for MerchantAccountUpdateInternal {
                is_recon_enabled: None,
                recon_status: None,
                is_platform_account: None,
+                product_type: None,


Do we want to allow updating the product type once the merchant account has been created?

If the answer for the above question is yes, then I don't see the MerchantAccountUpdate enum modified? So as it stands, the field would never be updated?

Yes product_type will be populated only once after merchant account is created. Once it is populated, it cannot be modified.

That will be taken up later.

SanchithHegde · 2025-02-24T18:31:02Z

justfile

+    mkdir -p {{resultant_dir}}
+    cp -r {{v1_migration_dir}}/* {{resultant_dir}}/
+
+    # Prefix v2 migrations with 9


Is this being done so that v2 migrations always show up last in the combined list?

…ueries_v1_v2

hrithikesh026 added 8 commits February 6, 2025 12:38

refactor: restructure payment_methods_v2 migrations

d42cf45

fix: run v1 and v2 migrations separately

36d96f6

chore: drop unused integer id coloumn from tables

dcc0537

chore: remove id field from locker_mock_up

cbcb3d3

chore: introduce id varchar in merchant_account

fced36d

chore: add product_type column in merchant_account

8bb3a61

Merge remote-tracking branch 'origin/main' into refactor_id_related_q…

76fcb9c

…ueries_v1_v2

update schema_v2.rs

ab96aee

hrithikesh026 added S-waiting-on-review Status: This PR has been implemented and needs to be reviewed M-database-changes Metadata: This PR involves database schema changes labels Feb 11, 2025

hrithikesh026 self-assigned this Feb 11, 2025

hrithikesh026 requested review from a team as code owners February 11, 2025 11:53

Merge branch 'main' into refactor_id_related_queries_v1_v2

1800d55

jarnura requested changes Feb 11, 2025

View reviewed changes

hrithikesh026 added 5 commits February 12, 2025 12:15

update schema_v2.rs

35fbaa7

change v2 diesel model

c1f0fda

Merge remote-tracking branch 'origin/main' into refactor_id_related_q…

e4cfae4

…ueries_v1_v2

populate id in merchant_account while inserting

16c3b58

clone merchant_id

7bdcc46

hrithikesh026 linked an issue Feb 13, 2025 that may be closed by this pull request

[FEATURE] create accounts schema for accounts tables #7125

Open

2 tasks

hrithikesh026 added 2 commits February 13, 2025 17:01

make revert back id columns to varchar instead of serial

072e071

delete remove_id.patch

782a963

hrithikesh026 requested a review from jarnura February 14, 2025 05:24

jarnura previously approved these changes Feb 14, 2025

View reviewed changes

Narayanbhat166 reviewed Feb 17, 2025

View reviewed changes

Merge branch 'main' into refactor_id_related_queries_v1_v2

bafd03a

hrithikesh026 dismissed jarnura’s stale review via 35d7bcf February 20, 2025 14:15

hrithikesh026 requested review from jarnura and Narayanbhat166 February 21, 2025 05:41

Narayanbhat166 previously approved these changes Feb 21, 2025

View reviewed changes

hrithikesh026 added 2 commits February 21, 2025 12:31

introduce separate migration to alter type of v1 columns

854da0a

format v2 migrations

fbe3f1e

hrithikesh026 dismissed Narayanbhat166’s stale review via fbe3f1e February 21, 2025 07:06

Narayanbhat166 previously approved these changes Feb 21, 2025

View reviewed changes

hrithikesh026 requested a review from Chethan-rao February 21, 2025 10:09

Chethan-rao requested changes Feb 22, 2025

View reviewed changes

hrithikesh026 added 2 commits February 24, 2025 11:34

add payer country back in down.sql

c57e4ff

Merge remote-tracking branch 'origin/main' into refactor_id_related_q…

627ccf1

…ueries_v1_v2

hrithikesh026 dismissed Narayanbhat166’s stale review via 627ccf1 February 24, 2025 06:22

Narayanbhat166 previously approved these changes Feb 24, 2025

View reviewed changes

hrithikesh026 requested a review from Chethan-rao February 24, 2025 07:20

Chethan-rao previously approved these changes Feb 24, 2025

View reviewed changes

hrithikesh026 requested review from a team and removed request for a team February 24, 2025 10:51

jarnura previously approved these changes Feb 24, 2025

View reviewed changes

SanchithHegde reviewed Feb 24, 2025

View reviewed changes

Merge remote-tracking branch 'origin/main' into refactor_id_related_q…

0c27611

…ueries_v1_v2

hrithikesh026 dismissed stale reviews from jarnura, Chethan-rao, and Narayanbhat166 via 0c27611 February 25, 2025 06:38

jarnura previously approved these changes Feb 25, 2025

View reviewed changes

Chethan-rao previously approved these changes Feb 25, 2025

View reviewed changes

chore: add appropriate revert queries in down.sql

87db6fe

hrithikesh026 dismissed stale reviews from Chethan-rao and jarnura via 87db6fe February 25, 2025 11:37

jarnura approved these changes Feb 25, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(v1v2): refactor database queries for v1 and v2 #7244

refactor(v1v2): refactor database queries for v1 and v2 #7244

hrithikesh026 commented Feb 11, 2025 •

edited

Loading

semanticdiff-com bot commented Feb 11, 2025 •

edited

Loading

jarnura Feb 11, 2025

Narayanbhat166 Feb 17, 2025

hrithikesh026 Feb 18, 2025

Narayanbhat166 Feb 17, 2025

hrithikesh026 Feb 18, 2025

Narayanbhat166 Feb 18, 2025

Narayanbhat166 Feb 17, 2025

hrithikesh026 Feb 18, 2025

Narayanbhat166 Feb 18, 2025

hrithikesh026 Feb 18, 2025

hrithikesh026 Feb 18, 2025

SanchithHegde Feb 24, 2025

Narayanbhat166 Feb 17, 2025

hrithikesh026 Feb 18, 2025

Narayanbhat166 Feb 18, 2025

Narayanbhat166 Feb 17, 2025

hrithikesh026 Feb 18, 2025

Chethan-rao Feb 22, 2025

hrithikesh026 Feb 24, 2025

SanchithHegde left a comment

SanchithHegde Feb 24, 2025

hrithikesh026 Feb 25, 2025

SanchithHegde Feb 25, 2025

SanchithHegde Feb 24, 2025

SanchithHegde Feb 24, 2025

SanchithHegde Feb 24, 2025

hrithikesh026 Feb 25, 2025

SanchithHegde Feb 24, 2025

hrithikesh026 Feb 25, 2025

refactor(v1v2): refactor database queries for v1 and v2 #7244

Are you sure you want to change the base?

refactor(v1v2): refactor database queries for v1 and v2 #7244

Conversation

hrithikesh026 commented Feb 11, 2025 • edited Loading

Type of Change

Description

Additional Changes

Motivation and Context

How did you test it?

Checklist

semanticdiff-com bot commented Feb 11, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SanchithHegde left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hrithikesh026 commented Feb 11, 2025 •

edited

Loading

semanticdiff-com bot commented Feb 11, 2025 •

edited

Loading