From d195fb990488bdad83aaca13b2a305d1722b3e91 Mon Sep 17 00:00:00 2001
From: Nikolai Ovtsinnikov <nikolai@zone.ee>
Date: Mon, 23 Sep 2024 20:20:48 +0300
Subject: [PATCH] refactor code. Fix possible race condition

---
 lib/attachments/gridstore-storage.js | 54 +++++++++++++++++-----------
 1 file changed, 34 insertions(+), 20 deletions(-)

diff --git a/lib/attachments/gridstore-storage.js b/lib/attachments/gridstore-storage.js
index fa0be0ff..c9f8ff65 100644
--- a/lib/attachments/gridstore-storage.js
+++ b/lib/attachments/gridstore-storage.js
@@ -33,6 +33,23 @@ class GridstoreStorage {
         });
     }
 
+    updateFileWithContentHashMetadata(args, callback, hash, calculatedFileContentHash) {
+        this.gridfs.collection(this.bucketName + '.files').findOneAndUpdate(
+            {
+                _id: hash
+            },
+            {
+                $set: {
+                    'metadata.fileContentHash': calculatedFileContentHash
+                }
+            },
+            {
+                returnDocument: 'after'
+            },
+            () => callback(...args) // do not really care about error here. If error then highly likely the file has not been uploaded either
+        );
+    }
+
     async get(attachmentId) {
         let attachmentData = await this.gridfs.collection(this.bucketName + '.files').findOne({
             _id: attachmentId
@@ -131,6 +148,12 @@ class GridstoreStorage {
 
         let attachmentCallback = (...args) => {
             // store finished uploading, add the hash of the file contents to file metadata
+            let calculatedFileContentHash;
+
+            if (args.length > 2) {
+                calculatedFileContentHash = args[2];
+            }
+
             if (storeLock) {
                 log.silly('GridStore', '[%s] UNLOCK lock=%s status=%s', instance, lockId, storeLock.success ? 'locked' : 'empty');
                 if (storeLock.success) {
@@ -139,26 +162,11 @@ class GridstoreStorage {
                             // might be already finished if retrying after delay
                             return;
                         }
-                        if (args.length > 2) {
-                            const calculatedFileContentHash = args[2];
-
-                            if (calculatedFileContentHash) {
-                                this.gridfs.collection(this.bucketName + '.files').findOneAndUpdate(
-                                    {
-                                        _id: hash
-                                    },
-                                    {
-                                        $set: {
-                                            'metadata.fileContentHash': calculatedFileContentHash
-                                        }
-                                    },
-                                    {
-                                        returnDocument: 'after'
-                                    }
-                                );
-                            }
+                        if (calculatedFileContentHash) {
+                            // locked upload, new file
+                            this.updateFileWithContentHashMetadata(args, callback, hash, calculatedFileContentHash);
+                            return; // return from attachmentCallback. Top level callback will be ran after hash update
                         }
-                        callback(...args);
                     });
                     // unset variable to prevent double releasing
                     storeLock = false;
@@ -170,6 +178,11 @@ class GridstoreStorage {
                 // might be already finished if retrying after delay
                 return;
             }
+            if (calculatedFileContentHash) {
+                // no lock upload, new file
+                this.updateFileWithContentHashMetadata(args, callback, hash, calculatedFileContentHash);
+                return; // return from attachmentCallback. Top level callback will be ran after hash update
+            }
             callback(...args);
         };
 
@@ -308,7 +321,8 @@ class GridstoreStorage {
                         store.once('finish', () => attachmentCallback(null, id, fileHashCalculator.hash));
 
                         if (!metadata.decoded) {
-                            store.end(attachment.body);
+                            fileHashCalculator.pipe(store);
+                            fileHashCalculator.end(attachment.body);
                         } else {
                             let decoder = new libbase64.Decoder();
                             decoder.pipe(fileHashCalculator).pipe(store);