From 2d0326b85afec33fef08d9027149d2f951e11fbb Mon Sep 17 00:00:00 2001
From: Elizabeth Paige Harper <foxcapade@gmail.com>
Date: Wed, 13 Nov 2024 12:59:37 -0500
Subject: [PATCH] rollback internal pg transaction on file download fail (#337)

---
 .../kotlin/vdi/component/db/cache/CacheDB.kt  |  5 +-
 lib/reconciler/build.gradle.kts               |  2 +-
 .../vdi/service/datasets/create-dataset.kt    | 52 +++++++++----------
 3 files changed, 29 insertions(+), 30 deletions(-)

diff --git a/lib/cache-db/src/main/kotlin/vdi/component/db/cache/CacheDB.kt b/lib/cache-db/src/main/kotlin/vdi/component/db/cache/CacheDB.kt
index a951fd57..2f39c26d 100644
--- a/lib/cache-db/src/main/kotlin/vdi/component/db/cache/CacheDB.kt
+++ b/lib/cache-db/src/main/kotlin/vdi/component/db/cache/CacheDB.kt
@@ -78,11 +78,10 @@ interface CacheDB {
   fun openTransaction(): vdi.component.db.cache.CacheDBTransaction
 }
 
-inline fun vdi.component.db.cache.CacheDB.withTransaction(fn: (vdi.component.db.cache.CacheDBTransaction) -> Unit) =
+inline fun <T> vdi.component.db.cache.CacheDB.withTransaction(fn: (vdi.component.db.cache.CacheDBTransaction) -> T) =
   openTransaction().use {
     try {
-      fn(it)
-      it.commit()
+      fn(it).apply { it.commit() }
     } catch (e: Throwable) {
       it.rollback()
       throw e
diff --git a/lib/reconciler/build.gradle.kts b/lib/reconciler/build.gradle.kts
index d19d345a..c8a2f998 100644
--- a/lib/reconciler/build.gradle.kts
+++ b/lib/reconciler/build.gradle.kts
@@ -26,7 +26,7 @@ dependencies {
 
   testImplementation(kotlin("test"))
   testImplementation("org.junit.jupiter:junit-jupiter-api:5.9.2")
-  testImplementation("org.mockito:mockito-core:5.2.0")
+  testImplementation("org.mockito:mockito-core:5.4.0")
   testRuntimeOnly("org.junit.jupiter:junit-jupiter-engine:5.9.2")
   testRuntimeOnly("org.apache.logging.log4j:log4j-slf4j-impl")
 }
diff --git a/service/rest-service/src/main/kotlin/org/veupathdb/service/vdi/service/datasets/create-dataset.kt b/service/rest-service/src/main/kotlin/org/veupathdb/service/vdi/service/datasets/create-dataset.kt
index 2c0a3cb9..5176d452 100644
--- a/service/rest-service/src/main/kotlin/org/veupathdb/service/vdi/service/datasets/create-dataset.kt
+++ b/service/rest-service/src/main/kotlin/org/veupathdb/service/vdi/service/datasets/create-dataset.kt
@@ -64,7 +64,7 @@ fun createDataset(
       throw BadRequestException("unrecognized target project")
   }
 
-  CacheDB().withTransaction {
+  val (tempDirectory, uploadFile) = CacheDB().withTransaction {
     it.tryInsertDataset(DatasetImpl(
       datasetID    = datasetID,
       typeName     = datasetMeta.type.name,
@@ -92,32 +92,32 @@ fun createDataset(
       dataUpdated   = OriginTimestamp,
       metaUpdated   = OriginTimestamp
     ))
-  }
 
-  // TODO: Post release!
-  //       The following call represents a 'unified' path for handling uploads
-  //       whether they are direct or via URL.  This leaves us open to proxy
-  //       timeouts in the case of URL uploads if the file transfer between the
-  //       VDI service and the remote server takes too long.
-  //
-  //       To handle this, the 'unified' path will need to be split into two
-  //       paths:
-  //       - Direct upload path
-  //       - URL upload path
-  //
-  //       For the direct upload path, the upload file will need to be copied to
-  //       a new temp directory (as the rest service thread will delete the
-  //       original upload file).  Then the new thread can be forked and the
-  //       file uploaded to MinIO.
-  //
-  //       For the URL upload path, the URL will need to be validated before
-  //       starting the new thread.  Then the new thread will be forked and the
-  //       target file downloaded into a temp directory in that thread before
-  //       being uploaded to MinIO (also in that forked thread).
-  val (tempDirectory, uploadFile) = try { entity.fetchDatasetFile() }
-  catch (e: Throwable) {
-    CacheDB().withTransaction { it.updateImportControl(datasetID, DatasetImportStatus.Failed) }
-    throw e
+    // TODO: Post release!
+    //       The following call represents a 'unified' path for handling uploads
+    //       whether they are direct or via URL.  This leaves us open to proxy
+    //       timeouts in the case of URL uploads if the file transfer between the
+    //       VDI service and the remote server takes too long.
+    //
+    //       To handle this, the 'unified' path will need to be split into two
+    //       paths:
+    //       - Direct upload path
+    //       - URL upload path
+    //
+    //       For the direct upload path, the upload file will need to be copied to
+    //       a new temp directory (as the rest service thread will delete the
+    //       original upload file).  Then the new thread can be forked and the
+    //       file uploaded to MinIO.
+    //
+    //       For the URL upload path, the URL will need to be validated before
+    //       starting the new thread.  Then the new thread will be forked and the
+    //       target file downloaded into a temp directory in that thread before
+    //       being uploaded to MinIO (also in that forked thread).
+    try { entity.fetchDatasetFile() }
+    catch (e: Throwable) {
+      CacheDB().withTransaction { it.updateImportControl(datasetID, DatasetImportStatus.Failed) }
+      throw e
+    }
   }
 
   Metrics.Upload.queueSize.inc()