refactor(back): move S3 part of final approval code to last part of code

momentum-mod · Mar 5, 2024 · b250626 · b250626
1 parent 9d878ad
commit b250626
Showing 1 changed file with 48 additions and 44 deletions.
diff --git a/apps/backend/src/app/modules/maps/maps.service.ts b/apps/backend/src/app/modules/maps/maps.service.ts
@@ -1347,7 +1347,6 @@ export class MapsService {
       )
     });
 
-    // Copy final MapSubmissionVersion BSP and VMFs to maps/
     const {
       currentVersion: { id: currentVersionID, hash, hasVmf, zones: dbZones },
       versions
@@ -1357,56 +1356,13 @@ export class MapsService {
     });
     const zones = dbZones as unknown as MapZones; // TODO: #855
 
-    const [bspSuccess, vmfSuccess] = await parallel(
-      this.fileStoreService.copyFile(
-        submissionBspPath(currentVersionID),
-        approvedBspPath(map.name)
-      ),
-      hasVmf
-        ? this.fileStoreService.copyFile(
-            submissionVmfsPath(currentVersionID),
-            approvedVmfsPath(map.name)
-          )
-        : () => Promise.resolve(true)
-    );
-
-    if (!bspSuccess)
-      throw new InternalServerErrorException(
-        `BSP file for map submission version ${currentVersionID} not in object store`
-      );
-    if (!vmfSuccess)
-      throw new InternalServerErrorException(
-        `VMF file for map submission version ${currentVersionID} not in object store`
-      );
-
     // Set hash of MMap to final version BSP's hash, and hasVmf is just whether
     // final version had a VMF.
     await tx.mMap.update({
       where: { id: map.id },
       data: { hash, hasVmf, zones: dbZones } // TODO: e2e test zoines
     });
 
-    // Delete all the submission files - these would take up a LOT of space otherwise
-    await parallel(
-      this.fileStoreService
-        .deleteFiles(
-          versions.flatMap((v) => [
-            submissionBspPath(v.id),
-            submissionVmfsPath(v.id)
-          ])
-        )
-        .catch((error) => {
-          error.message = `Failed to delete map submission version file for ${map.name}: ${error.message}`;
-          throw new InternalServerErrorException(error);
-        }),
-      this.mapReviewService
-        .deleteAllReviewAssetsForMap(map.id)
-        .catch((error) => {
-          error.message = `Failed to delete map review files for ${map.name}: ${error.message}`;
-          throw new InternalServerErrorException(error);
-        })
-    );
-
     // Is it getting approved for first time?
     if (
       !map.submission.dates.some(
@@ -1454,6 +1410,54 @@ export class MapsService {
 
       this.checkZones(zones);
     }
+
+    // Do S3 stuff last in case something errors - this is well tested and
+    // *should* behave well in production, but it's still complex stuff and
+    // we can't rollback S3 operations like we can a Postgres transaction.
+
+    // Copy final MapSubmissionVersion BSP and VMFs to maps/
+    const [bspSuccess, vmfSuccess] = await parallel(
+      this.fileStoreService.copyFile(
+        submissionBspPath(currentVersionID),
+        approvedBspPath(map.name)
+      ),
+      hasVmf
+        ? this.fileStoreService.copyFile(
+            submissionVmfsPath(currentVersionID),
+            approvedVmfsPath(map.name)
+          )
+        : () => Promise.resolve(true)
+    );
+
+    if (!bspSuccess)
+      throw new InternalServerErrorException(
+        `BSP file for map submission version ${currentVersionID} not in object store`
+      );
+    if (!vmfSuccess)
+      throw new InternalServerErrorException(
+        `VMF file for map submission version ${currentVersionID} not in object store`
+      );
+
+    // Delete all the submission files - these would take up a LOT of space otherwise
+    await parallel(
+      this.fileStoreService
+        .deleteFiles(
+          versions.flatMap((v) => [
+            submissionBspPath(v.id),
+            submissionVmfsPath(v.id)
+          ])
+        )
+        .catch((error) => {
+          error.message = `Failed to delete map submission version file for ${map.name}: ${error.message}`;
+          throw new InternalServerErrorException(error);
+        }),
+      this.mapReviewService
+        .deleteAllReviewAssetsForMap(map.id)
+        .catch((error) => {
+          error.message = `Failed to delete map review files for ${map.name}: ${error.message}`;
+          throw new InternalServerErrorException(error);
+        })
+    );
   }
 
   //#endregion