wrap in transaction

crates/service/web/storyteller_web/src/http_server/endpoints/folders/media_files/bulk_add_folder_media_files_handler.rs

@@ -3,8 +3,10 @@ use std::sync::Arc;
 
 use actix_web::web::{Json, Path};
 use actix_web::{web, HttpRequest};
-use log::warn;
-use sqlx::Acquire;
+use log::{error, warn};
+use sqlx::pool::PoolConnection;
+use sqlx::{Acquire, MySql, Transaction};
+use tokens::tokens::media_files::MediaFileToken;
 
 use artcraft_api_defs::folders::media_files::{
   BulkAddFolderMediaFilesRequest, BulkAddFolderMediaFilesSuccessResponse,
@@ -92,39 +94,10 @@ pub async fn bulk_add_folder_media_files_handler(
     CommonWebError::from_error(err)
   })?;
 
-  // Insert + recompute the denormalized `maybe_last_media_file_token_1..4`
-  // columns inside a single transaction so they stay in sync with
-  // membership. Empty `accepted` is a no-op — no membership change means
-  // no recompute needed.
+  // Empty `accepted` is a no-op — no membership change means no
+  // recompute needed and no transaction to open.
   if !accepted.is_empty() {
-    let mut tx = conn.begin().await.map_err(|err| {
-      warn!("Failed to begin transaction: {:?}", err);
-      CommonWebError::from_error(err)
-    })?;
-
-    bulk_insert_folder_media_files(BulkInsertFolderMediaFilesArgs {
-      folder_token: &path.folder_token,
-      media_file_tokens: &accepted,
-      mysql_executor: &mut *tx,
-      phantom: PhantomData,
-    }).await.map_err(|err| {
-      warn!("bulk_insert_folder_media_files failed: {:?}", err);
-      CommonWebError::from_error(err)
-    })?;
-
-    recompute_folder_last_media_files(RecomputeFolderLastMediaFilesArgs {
-      folder_token: &path.folder_token,
-      mysql_executor: &mut *tx,
-      phantom: PhantomData,
-    }).await.map_err(|err| {
-      warn!("recompute_folder_last_media_files failed: {:?}", err);
-      CommonWebError::from_error(err)
-    })?;
-
-    tx.commit().await.map_err(|err| {
-      warn!("Failed to commit bulk_add transaction: {:?}", err);
-      CommonWebError::from_error(err)
-    })?;
+    perform_atomic_add(&mut conn, &path.folder_token, &accepted).await?;
   }
 
   Ok(Json(BulkAddFolderMediaFilesSuccessResponse {
@@ -132,3 +105,68 @@ pub async fn bulk_add_folder_media_files_handler(
     accepted_media_file_tokens: accepted,
   }))
 }
+
+/// Open a transaction, run the insert + recompute as one unit, and
+/// commit on success. On any failure inside [`perform_add_work`] the
+/// transaction is explicitly rolled back before the original error is
+/// re-raised — sqlx would roll back on drop too, but doing it explicitly
+/// makes the failure path obvious and surfaces any rollback error in
+/// the log.
+async fn perform_atomic_add(
+  conn: &mut PoolConnection<MySql>,
+  folder_token: &FolderToken,
+  media_file_tokens: &[MediaFileToken],
+) -> Result<(), CommonWebError> {
+  let mut tx = conn.begin().await.map_err(|err| {
+    warn!("Failed to begin transaction: {:?}", err);
+    CommonWebError::from_error(err)
+  })?;
+
+  let work_result = perform_add_work(&mut tx, folder_token, media_file_tokens).await;
+
+  match work_result {
+    Ok(()) => {
+      tx.commit().await.map_err(|err| {
+        warn!("Failed to commit bulk_add transaction: {:?}", err);
+        CommonWebError::from_error(err)
+      })?;
+      Ok(())
+    }
+    Err(err) => {
+      if let Err(rollback_err) = tx.rollback().await {
+        error!(
+          "Rollback after bulk_add failure also failed: {:?} (original error: {:?})",
+          rollback_err, err,
+        );
+      }
+      Err(err)
+    }
+  }
+}
+
+async fn perform_add_work(
+  tx: &mut Transaction<'_, MySql>,
+  folder_token: &FolderToken,
+  media_file_tokens: &[MediaFileToken],
+) -> Result<(), CommonWebError> {
+  bulk_insert_folder_media_files(BulkInsertFolderMediaFilesArgs {
+    folder_token,
+    media_file_tokens,
+    mysql_executor: &mut **tx,
+    phantom: PhantomData,
+  }).await.map_err(|err| {
+    warn!("bulk_insert_folder_media_files failed: {:?}", err);
+    CommonWebError::from_error(err)
+  })?;
+
+  recompute_folder_last_media_files(RecomputeFolderLastMediaFilesArgs {
+    folder_token,
+    mysql_executor: &mut **tx,
+    phantom: PhantomData,
+  }).await.map_err(|err| {
+    warn!("recompute_folder_last_media_files failed: {:?}", err);
+    CommonWebError::from_error(err)
+  })?;
+
+  Ok(())
+}

crates/service/web/storyteller_web/src/http_server/endpoints/folders/media_files/bulk_remove_folder_media_files_handler.rs

@@ -3,7 +3,7 @@ use std::sync::Arc;
 
 use actix_web::web::{Json, Path};
 use actix_web::{web, HttpRequest};
-use log::warn;
+use log::{error, warn};
 
 use artcraft_api_defs::folders::media_files::{
   BulkRemoveFolderMediaFilesRequest, BulkRemoveFolderMediaFilesSuccessResponse,
@@ -18,8 +18,10 @@ use mysql_queries::queries::folders::media_files::bulk_delete_folder_media_files
 use mysql_queries::queries::folders::media_files::recompute_folder_last_media_files::{
   recompute_folder_last_media_files, RecomputeFolderLastMediaFilesArgs,
 };
-use sqlx::Acquire;
+use sqlx::pool::PoolConnection;
+use sqlx::{Acquire, MySql, Transaction};
 use tokens::tokens::folders::FolderToken;
+use tokens::tokens::media_files::MediaFileToken;
 
 use crate::http_server::common_responses::common_web_error::CommonWebError;
 use crate::http_server::web_utils::user_session::require_user_session_using_connection::require_user_session_using_connection;
@@ -78,9 +80,7 @@ pub async fn bulk_remove_folder_media_files_handler(
     return Err(CommonWebError::NotFound);
   }
 
-  // Delete + recompute the denormalized `maybe_last_media_file_token_1..4`
-  // columns inside a single transaction so they stay in sync with
-  // membership. Empty input short-circuits with zero work.
+  // Empty input short-circuits with zero work — no transaction needed.
   if request.media_file_tokens.is_empty() {
     return Ok(Json(BulkRemoveFolderMediaFilesSuccessResponse {
       success: true,
@@ -88,15 +88,65 @@ pub async fn bulk_remove_folder_media_files_handler(
     }));
   }
 
+  let removed_count = perform_atomic_remove(
+    &mut conn,
+    &path.folder_token,
+    &request.media_file_tokens,
+  ).await?;
+
+  Ok(Json(BulkRemoveFolderMediaFilesSuccessResponse {
+    success: true,
+    removed_count,
+  }))
+}
+
+/// Open a transaction, run the delete + recompute as one unit, and
+/// commit on success. On any failure inside [`perform_remove_work`] the
+/// transaction is explicitly rolled back before the original error is
+/// re-raised — sqlx would roll back on drop too, but doing it explicitly
+/// makes the failure path obvious and surfaces any rollback error in
+/// the log.
+async fn perform_atomic_remove(
+  conn: &mut PoolConnection<MySql>,
+  folder_token: &FolderToken,
+  media_file_tokens: &[MediaFileToken],
+) -> Result<u64, CommonWebError> {
   let mut tx = conn.begin().await.map_err(|err| {
     warn!("Failed to begin transaction: {:?}", err);
     CommonWebError::from_error(err)
   })?;
 
+  let work_result = perform_remove_work(&mut tx, folder_token, media_file_tokens).await;
+
+  match work_result {
+    Ok(removed_count) => {
+      tx.commit().await.map_err(|err| {
+        warn!("Failed to commit bulk_remove transaction: {:?}", err);
+        CommonWebError::from_error(err)
+      })?;
+      Ok(removed_count)
+    }
+    Err(err) => {
+      if let Err(rollback_err) = tx.rollback().await {
+        error!(
+          "Rollback after bulk_remove failure also failed: {:?} (original error: {:?})",
+          rollback_err, err,
+        );
+      }
+      Err(err)
+    }
+  }
+}
+
+async fn perform_remove_work(
+  tx: &mut Transaction<'_, MySql>,
+  folder_token: &FolderToken,
+  media_file_tokens: &[MediaFileToken],
+) -> Result<u64, CommonWebError> {
   let removed_count = bulk_delete_folder_media_files(BulkDeleteFolderMediaFilesArgs {
-    folder_token: &path.folder_token,
-    media_file_tokens: &request.media_file_tokens,
-    mysql_executor: &mut *tx,
+    folder_token,
+    media_file_tokens,
+    mysql_executor: &mut **tx,
     phantom: PhantomData,
   }).await.map_err(|err| {
     warn!("bulk_delete_folder_media_files failed: {:?}", err);
@@ -104,21 +154,13 @@ pub async fn bulk_remove_folder_media_files_handler(
   })?;
 
   recompute_folder_last_media_files(RecomputeFolderLastMediaFilesArgs {
-    folder_token: &path.folder_token,
-    mysql_executor: &mut *tx,
+    folder_token,
+    mysql_executor: &mut **tx,
     phantom: PhantomData,
   }).await.map_err(|err| {
     warn!("recompute_folder_last_media_files failed: {:?}", err);
     CommonWebError::from_error(err)
   })?;
 
-  tx.commit().await.map_err(|err| {
-    warn!("Failed to commit bulk_remove transaction: {:?}", err);
-    CommonWebError::from_error(err)
-  })?;
-
-  Ok(Json(BulkRemoveFolderMediaFilesSuccessResponse {
-    success: true,
-    removed_count,
-  }))
+  Ok(removed_count)
 }