perf(router): remove redundant DB call during merchant account update (#482)

crates/router/src/core/admin.rs

@@ -114,13 +114,6 @@ pub async fn merchant_account_update(
     merchant_id: &String,
     req: api::CreateMerchantAccount,
 ) -> RouterResponse<api::MerchantAccountResponse> {
-    let merchant_account = db
-        .find_merchant_account_by_merchant_id(merchant_id)
-        .await
-        .map_err(|error| {
-            error.to_not_found_response(errors::ApiErrorResponse::MerchantAccountNotFound)
-        })?;
-
     if &req.merchant_id != merchant_id {
         Err(report!(errors::ValidationError::IncorrectValueProvided {
             field_name: "parent_merchant_id"
@@ -167,10 +160,8 @@ pub async fn merchant_account_update(
 
         parent_merchant_id: get_parent_merchant(
             db,
-            req.sub_merchants_enabled
+            req.sub_merchants_enabled,
-                .or(merchant_account.sub_merchants_enabled),
+            req.parent_merchant_id,
-            req.parent_merchant_id
-                .or_else(|| merchant_account.parent_merchant_id.clone()),
         )
         .await?,
         enable_payment_response_hash: req.enable_payment_response_hash,
@@ -178,13 +169,12 @@ pub async fn merchant_account_update(
         redirect_to_merchant_with_http_post: req.redirect_to_merchant_with_http_post,
         locker_id: req.locker_id,
         metadata: req.metadata,
-        merchant_id: merchant_account.merchant_id.to_owned(),
         api_key: None,
         publishable_key: None,
     };
 
     let response = db
-        .update_merchant(merchant_account, updated_merchant_account)
+        .update_specific_fields_in_merchant(merchant_id, updated_merchant_account)
         .await
         .map_err(|error| {
             error.to_not_found_response(errors::ApiErrorResponse::MerchantAccountNotFound)
@@ -230,7 +220,9 @@ async fn get_parent_merchant(
                 })
                 .map(|id| validate_merchant_id(db, id).change_context(
                     errors::ApiErrorResponse::InvalidDataValue { field_name: "parent_merchant_id" }
-                ))?.await?.merchant_id
+                ))?
+                .await?
+                .merchant_id
             )
         }
         _ => None,

crates/router/src/db/merchant_account.rs

@@ -26,6 +26,12 @@ pub trait MerchantAccountInterface {
         merchant_account: storage::MerchantAccountUpdate,
     ) -> CustomResult<storage::MerchantAccount, errors::StorageError>;
 
+    async fn update_specific_fields_in_merchant(
+        &self,
+        merchant_id: &str,
+        merchant_account: storage::MerchantAccountUpdate,
+    ) -> CustomResult<storage::MerchantAccount, errors::StorageError>;
+
     async fn find_merchant_account_by_api_key(
         &self,
         api_key: &str,
@@ -79,6 +85,18 @@ impl MerchantAccountInterface for Store {
             .into_report()
     }
 
+    async fn update_specific_fields_in_merchant(
+        &self,
+        merchant_id: &str,
+        merchant_account: storage::MerchantAccountUpdate,
+    ) -> CustomResult<storage::MerchantAccount, errors::StorageError> {
+        let conn = pg_connection(&self.master_pool).await;
+        storage::MerchantAccount::update_with_specific_fields(&conn, merchant_id, merchant_account)
+            .await
+            .map_err(Into::into)
+            .into_report()
+    }
+
     async fn find_merchant_account_by_api_key(
         &self,
         api_key: &str,
@@ -175,6 +193,15 @@ impl MerchantAccountInterface for MockDb {
         Err(errors::StorageError::MockDbError)?
     }
 
+    async fn update_specific_fields_in_merchant(
+        &self,
+        _merchant_id: &str,
+        _merchant_account: storage::MerchantAccountUpdate,
+    ) -> CustomResult<storage::MerchantAccount, errors::StorageError> {
+        // [#TODO]: Implement function for `MockDb`
+        Err(errors::StorageError::MockDbError)?
+    }
+
     #[allow(clippy::panic)]
     async fn find_merchant_account_by_api_key(
         &self,

crates/storage_models/src/merchant_account.rs

@@ -48,7 +48,6 @@ pub struct MerchantAccountNew {
 #[derive(Debug)]
 pub enum MerchantAccountUpdate {
     Update {
-        merchant_id: String,
         merchant_name: Option<String>,
         api_key: Option<StrongSecret<String>>,
         merchant_details: Option<serde_json::Value>,
@@ -69,7 +68,6 @@ pub enum MerchantAccountUpdate {
 #[derive(Clone, Debug, Default, AsChangeset, router_derive::DebugAsDisplay)]
 #[diesel(table_name = merchant_account)]
 pub struct MerchantAccountUpdateInternal {
-    merchant_id: Option<String>,
     merchant_name: Option<String>,
     api_key: Option<StrongSecret<String>>,
     merchant_details: Option<serde_json::Value>,
@@ -90,7 +88,6 @@ impl From<MerchantAccountUpdate> for MerchantAccountUpdateInternal {
     fn from(merchant_account_update: MerchantAccountUpdate) -> Self {
         match merchant_account_update {
             MerchantAccountUpdate::Update {
-                merchant_id,
                 merchant_name,
                 api_key,
                 merchant_details,
@@ -106,7 +103,6 @@ impl From<MerchantAccountUpdate> for MerchantAccountUpdateInternal {
                 locker_id,
                 metadata,
             } => Self {
-                merchant_id: Some(merchant_id),
                 merchant_name,
                 api_key,
                 merchant_details,

crates/storage_models/src/query/generics.rs

@@ -95,20 +95,73 @@ where
         <<T as FilterDsl<P>>::Output as HasTable>::Table,
         <<T as FilterDsl<P>>::Output as IntoUpdateTarget>::WhereClause,
         <V as AsChangeset>::Changeset,
-    >: AsQuery + LoadQuery<'static, PgConnection, R> + QueryFragment<Pg> + Send,
+    >: AsQuery + LoadQuery<'static, PgConnection, R> + QueryFragment<Pg> + Send + Clone,
     R: Send + 'static,
+
+    // For cloning query (UpdateStatement)
+    <<T as FilterDsl<P>>::Output as HasTable>::Table: Clone,
+    <<T as FilterDsl<P>>::Output as IntoUpdateTarget>::WhereClause: Clone,
+    <V as AsChangeset>::Changeset: Clone,
+    <<<T as FilterDsl<P>>::Output as HasTable>::Table as QuerySource>::FromClause: Clone,
 {
     let debug_values = format!("{values:?}");
 
     let query = diesel::update(<T as HasTable>::table().filter(predicate)).set(values);
-    logger::debug!(query = %debug_query::<Pg, _>(&query).to_string());
 
-    query
+    match query.to_owned().get_results_async(conn).await {
-        .get_results_async(conn)
+        Ok(result) => {
+            logger::debug!(query = %debug_query::<Pg, _>(&query).to_string());
+            Ok(result)
+        }
+        Err(ConnectionError::Query(DieselError::QueryBuilderError(_))) => {
+            Err(report!(errors::DatabaseError::NoFieldsToUpdate))
+                .attach_printable_lazy(|| format!("Error while updating {debug_values}"))
+        }
+        Err(ConnectionError::Query(DieselError::NotFound)) => {
+            Err(report!(errors::DatabaseError::NotFound))
+                .attach_printable_lazy(|| format!("Error while updating {debug_values}"))
+        }
+        _ => Err(report!(errors::DatabaseError::Others))
+            .attach_printable_lazy(|| format!("Error while updating {debug_values}")),
+    }
+}
+
+#[instrument(level = "DEBUG", skip_all)]
+pub async fn generic_update_with_unique_predicate_get_result<T, V, P, R>(
+    conn: &PgPooledConn,
+    predicate: P,
+    values: V,
+) -> StorageResult<R>
+where
+    T: FilterDsl<P> + HasTable<Table = T> + Table + 'static,
+    V: AsChangeset<Target = <<T as FilterDsl<P>>::Output as HasTable>::Table> + Debug + 'static,
+    <T as FilterDsl<P>>::Output: IntoUpdateTarget + 'static,
+    UpdateStatement<
+        <<T as FilterDsl<P>>::Output as HasTable>::Table,
+        <<T as FilterDsl<P>>::Output as IntoUpdateTarget>::WhereClause,
+        <V as AsChangeset>::Changeset,
+    >: AsQuery + LoadQuery<'static, PgConnection, R> + QueryFragment<Pg> + Send,
+    R: Send + 'static,
+
+    // For cloning query (UpdateStatement)
+    <<T as FilterDsl<P>>::Output as HasTable>::Table: Clone,
+    <<T as FilterDsl<P>>::Output as IntoUpdateTarget>::WhereClause: Clone,
+    <V as AsChangeset>::Changeset: Clone,
+    <<<T as FilterDsl<P>>::Output as HasTable>::Table as QuerySource>::FromClause: Clone,
+{
+    generic_update_with_results::<<T as HasTable>::Table, _, _, _>(conn, predicate, values)
         .await
-        .into_report()
+        .map(|mut vec_r| {
-        .change_context(errors::DatabaseError::Others)
+            if vec_r.is_empty() {
-        .attach_printable_lazy(|| format!("Error while updating {debug_values}"))
+                Err(errors::DatabaseError::NotFound)
+            } else if vec_r.len() != 1 {
+                Err(errors::DatabaseError::Others)
+            } else {
+                vec_r.pop().ok_or(errors::DatabaseError::Others)
+            }
+            .into_report()
+            .attach_printable("Maybe not queried using a unique key")
+        })?
 }
 
 #[instrument(level = "DEBUG", skip_all)]
@@ -148,7 +201,8 @@ where
             Ok(result)
         }
         Err(ConnectionError::Query(DieselError::QueryBuilderError(_))) => {
-            generic_find_by_id_core::<T, _, _>(conn, id).await
+            Err(report!(errors::DatabaseError::NoFieldsToUpdate))
+                .attach_printable_lazy(|| format!("Error while updating by ID {debug_values}"))
         }
         Err(ConnectionError::Query(DieselError::NotFound)) => {
             Err(report!(errors::DatabaseError::NotFound))

crates/storage_models/src/query/merchant_account.rs

@@ -40,6 +40,24 @@ impl MerchantAccount {
         }
     }
 
+    pub async fn update_with_specific_fields(
+        conn: &PgPooledConn,
+        merchant_id: &str,
+        merchant_account: MerchantAccountUpdate,
+    ) -> StorageResult<Self> {
+        generics::generic_update_with_unique_predicate_get_result::<
+            <Self as HasTable>::Table,
+            _,
+            _,
+            _,
+        >(
+            conn,
+            dsl::merchant_id.eq(merchant_id.to_owned()),
+            MerchantAccountUpdateInternal::from(merchant_account),
+        )
+        .await
+    }
+
     pub async fn delete_by_merchant_id(
         conn: &PgPooledConn,
         merchant_id: &str,