From 52edad6f12d3ce21edca9d6a1094f0cb45eb340b Mon Sep 17 00:00:00 2001
From: Jakob Ackermann <jakob.ackermann@overleaf.com>
Date: Thu, 24 Oct 2024 13:43:25 +0200
Subject: [PATCH] Merge pull request #21322 from
 overleaf/jpa-skip-converted-cleanup

[filestore] remove unnecessary cleanup of converted files

GitOrigin-RevId: a46aaee473938209dc56d9fea90f96d09952ce47
---
 services/filestore/app/js/FileHandler.js      |  8 +--
 .../test/unit/js/FileHandlerTests.js          | 57 ++++++++++---------
 2 files changed, 34 insertions(+), 31 deletions(-)

diff --git a/services/filestore/app/js/FileHandler.js b/services/filestore/app/js/FileHandler.js
index 63928414b5..2c95003d62 100644
--- a/services/filestore/app/js/FileHandler.js
+++ b/services/filestore/app/js/FileHandler.js
@@ -49,9 +49,6 @@ async function insertFile(bucket, key, stream) {
       convertedKey,
     })
   }
-  if (Settings.enableConversions) {
-    await PersistorManager.deleteDirectory(bucket, convertedKey)
-  }
   await PersistorManager.sendStream(bucket, key, stream)
 }
 
@@ -65,7 +62,10 @@ async function deleteFile(bucket, key) {
     })
   }
   const jobs = [PersistorManager.deleteObject(bucket, key)]
-  if (Settings.enableConversions) {
+  if (
+    Settings.enableConversions &&
+    bucket === Settings.filestore.stores.template_files
+  ) {
     jobs.push(PersistorManager.deleteDirectory(bucket, convertedKey))
   }
   await Promise.all(jobs)
diff --git a/services/filestore/test/unit/js/FileHandlerTests.js b/services/filestore/test/unit/js/FileHandlerTests.js
index 1285051293..12a23667b4 100644
--- a/services/filestore/test/unit/js/FileHandlerTests.js
+++ b/services/filestore/test/unit/js/FileHandlerTests.js
@@ -67,7 +67,11 @@ describe('FileHandler', function () {
         compressPng: sinon.stub().resolves(),
       },
     }
-    Settings = {}
+    Settings = {
+      filestore: {
+        stores: { template_files: 'template_files', user_files: 'user_files' },
+      },
+    }
     fs = {
       createReadStream: sinon.stub().returns(readStream),
     }
@@ -133,23 +137,6 @@ describe('FileHandler', function () {
         done()
       })
     })
-
-    describe('when conversions are enabled', function () {
-      beforeEach(function () {
-        Settings.enableConversions = true
-      })
-
-      it('should delete the convertedKey folder', function (done) {
-        FileHandler.insertFile(bucket, key, stream, err => {
-          expect(err).not.to.exist
-          expect(PersistorManager.deleteDirectory).to.have.been.calledWith(
-            bucket,
-            convertedFolderKey
-          )
-          done()
-        })
-      })
-    })
   })
 
   describe('deleteFile', function () {
@@ -195,15 +182,31 @@ describe('FileHandler', function () {
         Settings.enableConversions = true
       })
 
-      it('should delete the convertedKey folder', function (done) {
-        FileHandler.deleteFile(bucket, key, err => {
-          expect(err).not.to.exist
-          expect(PersistorManager.deleteDirectory).to.have.been.calledWith(
-            bucket,
-            convertedFolderKey
-          )
-          done()
-        })
+      it('should delete the convertedKey folder for template files', function (done) {
+        FileHandler.deleteFile(
+          Settings.filestore.stores.template_files,
+          key,
+          err => {
+            expect(err).not.to.exist
+            expect(PersistorManager.deleteDirectory).to.have.been.calledWith(
+              Settings.filestore.stores.template_files,
+              convertedFolderKey
+            )
+            done()
+          }
+        )
+      })
+
+      it('should not delete the convertedKey folder for user files', function (done) {
+        FileHandler.deleteFile(
+          Settings.filestore.stores.user_files,
+          key,
+          err => {
+            expect(err).not.to.exist
+            expect(PersistorManager.deleteDirectory).to.not.have.been.called
+            done()
+          }
+        )
       })
     })
   })