Merge pull request #12986 from overleaf/jpa-docstore-archiving-disabled

[docstore] skip mongo/object-persistor calls when archiving is disabled

GitOrigin-RevId: 71bb7d77e987d6f32e37fd888311b6cc2a461170

services/docstore/app/js/DocArchiveManager.js

@@ -33,6 +33,10 @@ module.exports = {
 }
 
 async function archiveAllDocs(projectId) {
+  if (!_isArchivingEnabled()) {
+    return
+  }
+
   const docIds = await MongoManager.getNonArchivedProjectDocIds(projectId)
   await pMap(docIds, docId => archiveDoc(projectId, docId), {
     concurrency: PARALLEL_JOBS,
@@ -40,6 +44,10 @@ async function archiveAllDocs(projectId) {
 }
 
 async function archiveDoc(projectId, docId) {
+  if (!_isArchivingEnabled()) {
+    return
+  }
+
   const doc = await MongoManager.getDocForArchiving(projectId, docId)
 
   if (!doc) {
@@ -92,6 +100,10 @@ async function archiveDoc(projectId, docId) {
 }
 
 async function unArchiveAllDocs(projectId) {
+  if (!_isArchivingEnabled()) {
+    return
+  }
+
   while (true) {
     let docs
     if (Settings.docstore.keepSoftDeletedDocsArchived) {
@@ -150,6 +162,13 @@ async function unarchiveDoc(projectId, docId) {
     // The doc is already unarchived
     return
   }
+
+  if (!_isArchivingEnabled()) {
+    throw new Error(
+      'found archived doc, but archiving backend is not configured'
+    )
+  }
+
   const archivedDoc = await getDoc(projectId, docId)
   if (archivedDoc.rev == null) {
     // Older archived docs didn't have a rev. Assume that the rev of the

services/docstore/app/js/PersistorManager.js

@@ -4,6 +4,9 @@ const persistorSettings = settings.docstore
 persistorSettings.Metrics = require('@overleaf/metrics')
 
 const ObjectPersistor = require('@overleaf/object-persistor')
-const persistor = ObjectPersistor(persistorSettings)
+const AbstractPersistor = require('@overleaf/object-persistor/src/AbstractPersistor')
+const persistor = settings.docstore.backend
+  ? ObjectPersistor(persistorSettings)
+  : new AbstractPersistor()
 
 module.exports = persistor

services/docstore/config/settings.defaults.js

@@ -16,7 +16,7 @@ const Settings = {
     keepSoftDeletedDocsArchived:
       process.env.KEEP_SOFT_DELETED_DOCS_ARCHIVED === 'true',
 
-    backend: process.env.BACKEND || 's3',
+    backend: process.env.BACKEND,
     healthCheck: {
       project_id: process.env.HEALTH_CHECK_PROJECT_ID,
     },

services/docstore/test/unit/js/DocArchiveManagerTests.js

@@ -238,6 +238,17 @@ describe('DocArchiveManager', function () {
       )
     })
 
+    describe('when archiving is not configured', function () {
+      beforeEach(function () {
+        Settings.docstore.backend = undefined
+      })
+
+      it('should bail out early', async function () {
+        await DocArchiveManager.promises.archiveDoc(projectId, mongoDocs[0]._id)
+        expect(MongoManager.promises.getDocForArchiving).to.not.have.been.called
+      })
+    })
+
     describe('with null bytes in the result', function () {
       const _stringify = JSON.stringify
 
@@ -296,6 +307,26 @@ describe('DocArchiveManager', function () {
         ).to.have.been.calledWith(projectId, docId, archivedDoc)
       })
 
+      describe('when archiving is not configured', function () {
+        beforeEach(function () {
+          Settings.docstore.backend = undefined
+        })
+
+        it('should error out on archived doc', async function () {
+          await expect(
+            DocArchiveManager.promises.unarchiveDoc(projectId, docId)
+          ).to.eventually.be.rejected.and.match(
+            /found archived doc, but archiving backend is not configured/
+          )
+        })
+
+        it('should return early on non-archived doc', async function () {
+          MongoManager.promises.findDoc = sinon.stub().resolves({ rev })
+          await DocArchiveManager.promises.unarchiveDoc(projectId, docId)
+          expect(PersistorManager.getObjectMd5Hash).to.not.have.been.called
+        })
+      })
+
       describe('doc contents', function () {
         let archivedDoc
 
@@ -480,6 +511,18 @@ describe('DocArchiveManager', function () {
         MongoManager.promises.markDocAsArchived
       ).not.to.have.been.calledWith(projectId, mongoDocs[3]._id)
     })
+
+    describe('when archiving is not configured', function () {
+      beforeEach(function () {
+        Settings.docstore.backend = undefined
+      })
+
+      it('should bail out early', async function () {
+        await DocArchiveManager.promises.archiveDoc(projectId, mongoDocs[0]._id)
+        expect(MongoManager.promises.getNonArchivedProjectDocIds).to.not.have
+          .been.called
+      })
+    })
   })
 
   describe('unArchiveAllDocs', function () {
@@ -498,5 +541,17 @@ describe('DocArchiveManager', function () {
         )
       }
     })
+
+    describe('when archiving is not configured', function () {
+      beforeEach(function () {
+        Settings.docstore.backend = undefined
+      })
+
+      it('should bail out early', async function () {
+        await DocArchiveManager.promises.archiveDoc(projectId, mongoDocs[0]._id)
+        expect(MongoManager.promises.getNonDeletedArchivedProjectDocs).to.not
+          .have.been.called
+      })
+    })
   })
 })

services/docstore/test/unit/js/PersistorManagerTests.js

@@ -0,0 +1,53 @@
+const { expect } = require('chai')
+const modulePath = '../../../app/js/PersistorManager.js'
+const SandboxedModule = require('sandboxed-module')
+
+describe('PersistorManager', function () {
+  class FakePersistor {
+    async sendStream() {
+      return 'sent'
+    }
+  }
+
+  describe('configured', function () {
+    it('should return fake persistor', function () {
+      const Settings = {
+        docstore: {
+          backend: 'gcs',
+          bucket: 'wombat',
+        },
+      }
+      const PersistorManger = SandboxedModule.require(modulePath, {
+        requires: {
+          '@overleaf/settings': Settings,
+          '@overleaf/object-persistor': () => new FakePersistor(),
+        },
+      })
+
+      expect(PersistorManger).to.be.instanceof(FakePersistor)
+      expect(PersistorManger.sendStream()).to.eventually.equal('sent')
+    })
+  })
+
+  describe('not configured', function () {
+    it('should return abstract persistor', async function () {
+      const Settings = {
+        docstore: {
+          backend: undefined,
+          bucket: 'wombat',
+        },
+      }
+      const PersistorManger = SandboxedModule.require(modulePath, {
+        requires: {
+          '@overleaf/settings': Settings,
+          '@overleaf/object-persistor': () => new FakePersistor(),
+        },
+      })
+
+      expect(PersistorManger.constructor.name).to.equal('AbstractPersistor')
+      expect(PersistorManger.sendStream()).to.eventually.be.rejectedWith(
+        /method not implemented in persistor/
+      )
+    })
+  })
+})