From 6ff735badb432064b0cfafc280f2043ae22d6c51 Mon Sep 17 00:00:00 2001
From: Bobby Wibowo <bobbywibowo98@gmail.com>
Date: Tue, 9 Aug 2022 17:18:56 +0700
Subject: [PATCH] feat: wrap add to album db query in transaction

additionally allow superadmins to arbitrarily add/remove files to/from
albums via manual API calls, instead of only allowing root user
---
 controllers/albumsController.js | 42 +++++++++++++++++++++------------
 controllers/utilsController.js  |  4 +++-
 2 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/controllers/albumsController.js b/controllers/albumsController.js
index 47d1a03..a12b71f 100644
--- a/controllers/albumsController.js
+++ b/controllers/albumsController.js
@@ -676,6 +676,8 @@ self.addFiles = async (req, res) => {
     throw new ClientError('No files specified.')
   }
 
+  const issuperadmin = perms.is(req.locals.user, 'superadmin')
+
   let albumid = parseInt(req.body.albumid)
   if (isNaN(albumid) || albumid < 0) {
     albumid = null
@@ -683,14 +685,16 @@ self.addFiles = async (req, res) => {
 
   const failed = []
   const albumids = []
+
+  // Wrap within a Promise then-async block for custom error handling
   return Promise.resolve().then(async () => {
     if (albumid !== null) {
       const album = await utils.db.table('albums')
         .where('id', albumid)
         .where(function () {
-          // Only allow "root" user to arbitrarily add/remove files to/from any albums
+          // Only allow superadmins to arbitrarily add/remove files to/from any albums
           // NOTE: Dashboard does not facilitate this, intended for manual API calls
-          if (req.locals.user.username !== 'root') {
+          if (!issuperadmin) {
             this.where('userid', req.locals.user.id)
           }
         })
@@ -700,31 +704,39 @@ self.addFiles = async (req, res) => {
         throw new ClientError('Album does not exist or it does not belong to the user.', { statusCode: 404 })
       }
 
+      // Insert this album's ID into "albumids" array to be updated later
       albumids.push(albumid)
     }
 
+    // Query all owned files matching submitted IDs
     const files = await utils.db.table('files')
       .whereIn('id', ids)
       .where('userid', req.locals.user.id)
 
+    // Push IDs not found in database into "failed" array
     failed.push(...ids.filter(id => !files.find(file => file.id === id)))
 
-    await utils.db.table('files')
-      .whereIn('id', files.map(file => file.id))
-      .update('albumid', albumid)
-    utils.invalidateStatsCache('albums')
+    await utils.db.transaction(async trx => {
+      // Update files' associated album IDs
+      await trx('files')
+        .whereIn('id', files.map(file => file.id))
+        .update('albumid', albumid)
+      utils.invalidateStatsCache('albums')
 
-    files.forEach(file => {
-      if (file.albumid && !albumids.includes(file.albumid)) {
-        albumids.push(file.albumid)
-      }
+      // Insert all previous albums' IDs into "albumids" array to be updated later
+      files.forEach(file => {
+        if (file.albumid && !albumids.includes(file.albumid)) {
+          albumids.push(file.albumid)
+        }
+      })
+
+      // Update all relevant albums' "editedAt" timestamp
+      await trx('albums')
+        .whereIn('id', albumids)
+        .update('editedAt', Math.floor(Date.now() / 1000))
+      utils.deleteStoredAlbumRenders(albumids)
     })
 
-    await utils.db.table('albums')
-      .whereIn('id', albumids)
-      .update('editedAt', Math.floor(Date.now() / 1000))
-    utils.deleteStoredAlbumRenders(albumids)
-
     return res.json({ success: true, failed })
   }).catch(error => {
     if (Array.isArray(failed) && (failed.length === ids.length)) {
diff --git a/controllers/utilsController.js b/controllers/utilsController.js
index 66083be..c5e59bc 100644
--- a/controllers/utilsController.js
+++ b/controllers/utilsController.js
@@ -628,6 +628,8 @@ self.bulkDeleteFromDb = async (field, values, user) => {
     const unlinkeds = []
     const albumids = []
 
+    // NOTE: Not wrapped within a Transaction because
+    // we cannot rollback files physically unlinked from the storage
     await Promise.all(chunks.map(async chunk => {
       const files = await self.db.table('files')
         .whereIn(field, chunk)
@@ -666,7 +668,7 @@ self.bulkDeleteFromDb = async (field, values, user) => {
         if (file.albumid && !albumids.includes(file.albumid)) {
           albumids.push(file.albumid)
         }
-        // Delete form Content-Disposition store if used
+        // Delete from Content-Disposition store if used
         if (self.contentDispositionStore) {
           self.contentDispositionStore.delete(file.name)
         }