Browse Source

Merge pull request #24131 from overleaf/bg-disable-call-to-filestore-in-project-deleter

disable call to filestore in project deleter

GitOrigin-RevId: 07d9d3b33220e33abfa18a5f54e56e2b8d544e8d
Brian Gough 1 year ago
parent
commit
8a4c84e7dd

+ 3 - 0
services/web/app/src/Features/FileStore/FileStoreHandler.js

@@ -274,6 +274,9 @@ const FileStoreHandler = {
   },
 
   deleteProject(projectId, callback) {
+    if (!Features.hasFeature('filestore')) {
+      return callback() // if filestore is not in use, we don't need to delete anything
+    }
     request(
       {
         method: 'delete',

+ 8 - 4
services/web/test/acceptance/src/DeletionTests.mjs

@@ -391,7 +391,7 @@ describe('Deleting a project', function () {
         )
       })
 
-      it('Should destroy the files', function (done) {
+      it('Should destroy the files if filestore is in use', function (done) {
         expect(MockFilestoreApi.files[this.projectId.toString()]).to.exist
 
         request.post(
@@ -406,9 +406,13 @@ describe('Deleting a project', function () {
           (error, res) => {
             expect(error).not.to.exist
             expect(res.statusCode).to.equal(200)
-
-            expect(MockFilestoreApi.files[this.projectId.toString()]).not.to
-              .exist
+            if (Features.hasFeature('filestore')) {
+              expect(MockFilestoreApi.files[this.projectId.toString()]).not.to
+                .exist
+            } else {
+              // don't touch files in filestore if it's not in use
+              expect(MockFilestoreApi.files[this.projectId.toString()]).to.exist
+            }
             done()
           }
         )

+ 35 - 18
services/web/test/unit/src/FileStore/FileStoreHandlerTests.js

@@ -421,27 +421,44 @@ describe('FileStoreHandler', function () {
   })
 
   describe('deleteProject', function () {
-    it('should send a delete request to filestore api', function (done) {
-      const projectUrl = this.getProjectUrl(this.projectId)
-      this.request.callsArgWith(1, null)
+    describe('when filestore is enabled', function () {
+      beforeEach(function () {
+        this.Features.hasFeature.withArgs('filestore').returns(true)
+      })
+      it('should send a delete request to filestore api', function (done) {
+        const projectUrl = this.getProjectUrl(this.projectId)
+        this.request.callsArgWith(1, null)
+
+        this.handler.deleteProject(this.projectId, err => {
+          assert.equal(err, undefined)
+          this.request.args[0][0].method.should.equal('delete')
+          this.request.args[0][0].uri.should.equal(projectUrl)
+          done()
+        })
+      })
 
-      this.handler.deleteProject(this.projectId, err => {
-        assert.equal(err, undefined)
-        this.request.args[0][0].method.should.equal('delete')
-        this.request.args[0][0].uri.should.equal(projectUrl)
-        done()
+      it('should wrap the error if there is one', function (done) {
+        const error = new Error('my error')
+        this.request.callsArgWith(1, error)
+        this.handler.deleteProject(this.projectId, err => {
+          expect(OError.getFullStack(err)).to.match(
+            /something went wrong deleting a project in filestore/
+          )
+          expect(OError.getFullStack(err)).to.match(/my error/)
+          done()
+        })
       })
     })
-
-    it('should wrap the error if there is one', function (done) {
-      const error = new Error('my error')
-      this.request.callsArgWith(1, error)
-      this.handler.deleteProject(this.projectId, err => {
-        expect(OError.getFullStack(err)).to.match(
-          /something went wrong deleting a project in filestore/
-        )
-        expect(OError.getFullStack(err)).to.match(/my error/)
-        done()
+    describe('when filestore is disabled', function () {
+      beforeEach(function () {
+        this.Features.hasFeature.withArgs('filestore').returns(false)
+      })
+      it('should not send a delete request to filestore api', function (done) {
+        this.handler.deleteProject(this.projectId, err => {
+          assert.equal(err, undefined)
+          this.request.called.should.equal(false)
+          done()
+        })
       })
     })
   })