[BUG] Add Create Merchant and Create Merchant Key Store in a DB transaction

[dracarys18]

Currently MerchantAccount and MerchantKeyStore are getting inserted seperately. Since both of them are interdependent we need to make sure both are consistent with each other. If one of them fails other shouldn't get inserted.

We need to add this in a db_transaction

hyperswitch/crates/router/src/core/admin.rs

Lines 133 to 186 in 4805a94

	db.insert_merchant_key_store(key_store.clone(), &master_key.to_vec().into())
	.await
	.to_duplicate_response(errors::ApiErrorResponse::DuplicateMerchantAccount)?;
	let parent_merchant_id = get_parent_merchant(
	db,
	req.sub_merchants_enabled,
	req.parent_merchant_id,
	&key_store,
	)
	.await?;
	let merchant_account = async {
	Ok(domain::MerchantAccount {
	merchant_id: req.merchant_id,
	merchant_name: req
	.merchant_name
	.async_lift(|inner| domain_types::encrypt_optional(inner, &key))
	.await?,
	merchant_details: merchant_details
	.async_lift(|inner| domain_types::encrypt_optional(inner, &key))
	.await?,
	return_url: req.return_url.map(|a| a.to_string()),
	webhook_details,
	routing_algorithm: req.routing_algorithm,
	sub_merchants_enabled: req.sub_merchants_enabled,
	parent_merchant_id,
	enable_payment_response_hash,
	payment_response_hash_key,
	redirect_to_merchant_with_http_post: req
	.redirect_to_merchant_with_http_post
	.unwrap_or_default(),
	publishable_key,
	locker_id: req.locker_id,
	metadata: req.metadata,
	storage_scheme: diesel_models::enums::MerchantStorageScheme::PostgresOnly,
	primary_business_details,
	created_at: date_time::now(),
	modified_at: date_time::now(),
	frm_routing_algorithm: req.frm_routing_algorithm,
	intent_fulfillment_time: req.intent_fulfillment_time.map(i64::from),
	payout_routing_algorithm: req.payout_routing_algorithm,
	id: None,
	organization_id: req.organization_id,
	is_recon_enabled: false,
	})
	}
	.await
	.change_context(errors::ApiErrorResponse::InternalServerError)?;
	let merchant_account = db
	.insert_merchant(merchant_account, &key_store)
	.await
	.to_duplicate_response(errors::ApiErrorResponse::DuplicateMerchantAccount)?;

For reference of how to do it in a db transaction

  pool.transaction_async(|conn| async move {
        diesel::update(dsl::users)
            .filter(dsl::id.eq(0))
            .set(dsl::name.eq("Let's change the name again"))
            .execute_async(&conn)
            .await
            .map_err(|e| PoolError::Connection(e))
})

[artech-git]

Hi @dracarys18 gonna take a look into this issue, also how can you tell me how should the conn.transaction_async() be performed should it be done undercreate_merchant_account itself, or should I modify the previous method insert_merchant_key_store under the trait MerchantKeyStoreInterface

I was experimenting with the logic and made changes in the following sections

impl MerchantKeyStoreInterface for Store {

    async fn insert_merchant_and_key_store(
        &self,
        merchant_account: domain::MerchantAccount,
        merchant_key_store: domain::MerchantKeyStore,
        key: &Secret<Vec<u8>>,
    ) -> CustomResult<domain::MerchantAccount, errors::StorageError> {
        let conn = connection::pg_connection_write(self).await?;
        
        let val = conn.transaction_async(|e| async move {
    
            merchant_key_store
                .construct_new()
                .await
                .change_context(errors::StorageError::EncryptionError)?
                .insert(&e)
                .await
                .map_err(Into::into)
                .into_report()?
                .convert(key)
                .await
                .change_context(errors::StorageError::DecryptionError);

            merchant_account
                .construct_new()
                .await
                .change_context(errors::StorageError::EncryptionError)?
                .insert(&e)
                .await
                .map_err(Into::into)
                .into_report()?
                .convert(merchant_key_store.key.get_inner())
                .await
                .change_context(errors::StorageError::DecryptionError)
        
        }).await;

        val
    }

do let me know your thoughts about the approach

[dracarys18]

Hey @artech-git I would prefer keeping the Store interface seperate for MerchantKeyStore and MerchantAccount. i.e Store impl of MerchantKeystore will only insert into merchant_key_store table. So it would be better to do it in admin.rs as I mentioned in the issue.

[dracarys18]

Hey @artech-git , Please let me know if you need any help in this

[dracarys18]

Opening this issue for contributions because of inactivity.

[Azanul]

May I take this up?

[SanchithHegde]

Sure @Azanul, I'll assign this to you.

[Azanul]

To do this in admin.rs, I think we would need access to the connection to underlying Store.
Is it exposed via a trait/method? I couldn't find it.

[dracarys18]

Hey @Azanul in that case add it in MerchantAccountInterface in insert_merchant_account function. Since we don't want to insert MerchantAccount without inserting KeyStore.

[gorakhnathy7]

Closing this, as per inputs of @dracarys18