Merge pull request #15720 from overleaf/em-remove-docops-code

Remove code related to the docOps collection

GitOrigin-RevId: 38c1f752edda1cec4ed7051a0dba18b16ae4c754

services/docstore/app/js/DocManager.js

@@ -30,11 +30,6 @@ const DocManager = {
       return await DocManager._getDoc(projectId, docId, filter)
     }
 
-    if (filter.version && doc.version === undefined) {
-      const version = await MongoManager.promises.getDocVersion(docId)
-      doc.version = version
-    }
-
     return doc
   },
 
@@ -95,11 +90,7 @@ const DocManager = {
   // without unarchiving it (avoids unnecessary writes to mongo)
   async peekDoc(projectId, docId) {
     const doc = await DocManager._peekRawDoc(projectId, docId)
-    if (doc.version === undefined) {
-      const version = await MongoManager.promises.getDocVersion(docId)
-      await MongoManager.promises.checkRevUnchanged(doc)
-      doc.version = version
-    }
+    await MongoManager.promises.checkRevUnchanged(doc)
     return doc
   },
 

services/docstore/app/js/HealthChecker.js

@@ -61,7 +61,6 @@ module.exports = {
         })
       },
       cb => db.docs.deleteOne({ _id: docId, project_id: projectId }, cb),
-      cb => db.docOps.deleteOne({ doc_id: docId }, cb),
     ]
     return async.series(jobs, callback)
   },

services/docstore/app/js/MongoManager.js

@@ -5,20 +5,21 @@ const { callbackify } = require('util')
 
 const ARCHIVING_LOCK_DURATION_MS = Settings.archivingLockDurationMs
 
-async function findDoc(projectId, docId, filter) {
+async function findDoc(projectId, docId, projection) {
   const doc = await db.docs.findOne(
     {
       _id: new ObjectId(docId.toString()),
       project_id: new ObjectId(projectId.toString()),
     },
-    {
-      projection: filter,
-    }
+    { projection }
   )
+  if (doc && projection.version && !doc.version) {
+    doc.version = 0
+  }
   return doc
 }
 
-async function getProjectsDeletedDocs(projectId, filter) {
+async function getProjectsDeletedDocs(projectId, projection) {
   const docs = await db.docs
     .find(
       {
@@ -26,7 +27,7 @@ async function getProjectsDeletedDocs(projectId, filter) {
         deleted: true,
       },
       {
-        projection: filter,
+        projection,
         sort: { deletedAt: -1 },
         limit: Settings.max_deleted_docs,
       }
@@ -35,13 +36,13 @@ async function getProjectsDeletedDocs(projectId, filter) {
   return docs
 }
 
-async function getProjectsDocs(projectId, options, filter) {
+async function getProjectsDocs(projectId, options, projection) {
   const query = { project_id: new ObjectId(projectId.toString()) }
   if (!options.include_deleted) {
     query.deleted = { $ne: true }
   }
   const queryOptions = {
-    projection: filter,
+    projection,
   }
   if (options.limit) {
     queryOptions.limit = options.limit
@@ -203,18 +204,6 @@ async function restoreArchivedDoc(projectId, docId, archivedDoc) {
   }
 }
 
-async function getDocVersion(docId) {
-  const doc = await db.docOps.findOne(
-    { doc_id: new ObjectId(docId) },
-    {
-      projection: {
-        version: 1,
-      },
-    }
-  )
-  return (doc && doc.version) || 0
-}
-
 async function getDocRev(docId) {
   const doc = await db.docs.findOne(
     { _id: new ObjectId(docId.toString()) },
@@ -248,11 +237,6 @@ async function checkRevUnchanged(doc) {
 }
 
 async function destroyProject(projectId) {
-  const records = await db.docs
-    .find({ project_id: new ObjectId(projectId) }, { projection: { _id: 1 } })
-    .toArray()
-  const docIds = records.map(r => r._id)
-  await db.docOps.deleteMany({ doc_id: { $in: docIds } })
   await db.docs.deleteMany({ project_id: new ObjectId(projectId) })
 }
 
@@ -270,7 +254,6 @@ module.exports = {
   patchDoc: callbackify(patchDoc),
   getDocForArchiving: callbackify(getDocForArchiving),
   markDocAsArchived: callbackify(markDocAsArchived),
-  getDocVersion: callbackify(getDocVersion),
   checkRevUnchanged: callbackify(checkRevUnchanged),
   destroyProject: callbackify(destroyProject),
   promises: {
@@ -285,7 +268,6 @@ module.exports = {
     patchDoc,
     getDocForArchiving,
     markDocAsArchived,
-    getDocVersion,
     checkRevUnchanged,
     destroyProject,
   },

services/docstore/app/js/mongodb.js

@@ -7,7 +7,6 @@ const mongoDb = mongoClient.db()
 
 const db = {
   docs: mongoDb.collection('docs'),
-  docOps: mongoDb.collection('docOps'),
 }
 
 Metrics.mongodb.monitor(mongoClient)

services/docstore/test/acceptance/js/DeletingDocsTests.js

@@ -471,15 +471,7 @@ describe("Destroying a project's documents", function () {
 
   describe('when the doc exists', function () {
     beforeEach(function (done) {
-      db.docOps.insertOne(
-        { doc_id: new ObjectId(this.doc_id), version: 1 },
-        err => {
-          if (err) {
-            return done(err)
-          }
-          DocstoreClient.destroyAllDoc(this.project_id, done)
-        }
-      )
+      DocstoreClient.destroyAllDoc(this.project_id, done)
     })
 
     it('should remove the doc from the docs collection', function (done) {
@@ -489,14 +481,6 @@ describe("Destroying a project's documents", function () {
         done()
       })
     })
-
-    it('should remove the docOps from the docOps collection', function (done) {
-      db.docOps.find({ doc_id: this.doc_id }).toArray((err, docOps) => {
-        expect(err).not.to.exist
-        expect(docOps).to.deep.equal([])
-        done()
-      })
-    })
   })
 
   describe('when the doc is archived', function () {
@@ -517,14 +501,6 @@ describe("Destroying a project's documents", function () {
       })
     })
 
-    it('should remove the docOps from the docOps collection', function (done) {
-      db.docOps.find({ doc_id: this.doc_id }).toArray((err, docOps) => {
-        expect(err).not.to.exist
-        expect(docOps).to.deep.equal([])
-        done()
-      })
-    })
-
     it('should remove the doc contents from s3', function (done) {
       DocstoreClient.getS3Doc(this.project_id, this.doc_id, error => {
         expect(error).to.be.instanceOf(Errors.NotFoundError)

services/docstore/test/unit/js/DocManagerTests.js

@@ -16,7 +16,6 @@ describe('DocManager', function () {
     this.MongoManager = {
       promises: {
         findDoc: sinon.stub(),
-        getDocVersion: sinon.stub().resolves(this.version),
         getProjectsDocs: sinon.stub(),
         patchDoc: sinon.stub().resolves(),
         upsertIntoDocCollection: sinon.stub().resolves(),
@@ -120,6 +119,7 @@ describe('DocManager', function () {
         _id: this.doc_id,
         project_id: this.project_id,
         lines: ['mock-lines'],
+        version: this.version,
       }
     })
 
@@ -166,55 +166,12 @@ describe('DocManager', function () {
           .should.equal(true)
       })
 
-      it('should get the doc version from the docOps collection', function () {
-        this.MongoManager.promises.getDocVersion
-          .calledWith(this.doc_id)
-          .should.equal(true)
-      })
-
       it('should return the doc with the version', function () {
         this.result.lines.should.equal(this.doc.lines)
         this.result.version.should.equal(this.version)
       })
     })
 
-    describe('without the version filter', function () {
-      beforeEach(async function () {
-        this.MongoManager.promises.findDoc.resolves(this.doc)
-        await this.DocManager.promises._getDoc(this.project_id, this.doc_id, {
-          version: false,
-          inS3: true,
-        })
-      })
-
-      it('should not get the doc version from the docOps collection', function () {
-        this.MongoManager.promises.getDocVersion.called.should.equal(false)
-      })
-    })
-
-    describe('when the doc record already has a version', function () {
-      beforeEach(async function () {
-        this.docWithVersion = { ...this.doc, version: 109 }
-        this.MongoManager.promises.findDoc.resolves(this.docWithVersion)
-        this.result = await this.DocManager.promises._getDoc(
-          this.project_id,
-          this.doc_id,
-          {
-            version: true,
-            inS3: true,
-          }
-        )
-      })
-
-      it('should return that version', function () {
-        expect(this.result).to.deep.equal(this.docWithVersion)
-      })
-
-      it('should not fetch the doc version from the docOps collection', function () {
-        expect(this.MongoManager.promises.getDocVersion).not.to.have.been.called
-      })
-    })
-
     describe('when MongoManager.findDoc errors', function () {
       it('should return the error', async function () {
         this.MongoManager.promises.findDoc.rejects(this.stubbedError)
@@ -232,12 +189,14 @@ describe('DocManager', function () {
         this.doc = {
           _id: this.doc_id,
           project_id: this.project_id,
+          version: 2,
           inS3: true,
         }
         this.unarchivedDoc = {
           _id: this.doc_id,
           project_id: this.project_id,
           lines: ['mock-lines'],
+          version: 2,
           inS3: false,
         }
         this.MongoManager.promises.findDoc.resolves(this.doc)
@@ -271,7 +230,6 @@ describe('DocManager', function () {
       it('should return the doc', function () {
         expect(this.result).to.deep.equal({
           ...this.unarchivedDoc,
-          version: this.version,
         })
       })
     })

services/docstore/test/unit/js/MongoManagerTests.js

@@ -15,7 +15,6 @@ describe('MongoManager', function () {
         updateOne: sinon.stub().resolves({ matchedCount: 1 }),
         insertOne: sinon.stub().resolves(),
       },
-      docOps: {},
     }
     this.MongoManager = SandboxedModule.require(modulePath, {
       requires: {
@@ -281,19 +280,7 @@ describe('MongoManager', function () {
   describe('destroyProject', function () {
     beforeEach(async function () {
       this.projectId = new ObjectId()
-      this.docIds = [new ObjectId(), new ObjectId()]
       this.db.docs.deleteMany = sinon.stub().resolves()
-      this.db.docOps.deleteMany = sinon.stub().resolves()
-      this.db.docs.find = sinon
-        .stub()
-        .withArgs({ project_id: this.projectId })
-        .returns({
-          toArray: sinon.stub().resolves(
-            this.docIds.map(id => ({
-              _id: id,
-            }))
-          ),
-        })
       await this.MongoManager.promises.destroyProject(this.projectId)
     })
 
@@ -302,48 +289,6 @@ describe('MongoManager', function () {
         project_id: this.projectId,
       })
     })
-
-    it('should destroy the docOps', function () {
-      sinon.assert.calledWith(this.db.docOps.deleteMany, {
-        doc_id: { $in: this.docIds },
-      })
-    })
-  })
-
-  describe('getDocVersion', function () {
-    describe('when the doc exists', function () {
-      beforeEach(async function () {
-        this.doc = { version: (this.version = 42) }
-        this.db.docOps.findOne = sinon.stub().resolves(this.doc)
-        this.result = await this.MongoManager.promises.getDocVersion(this.docId)
-      })
-
-      it('should look for the doc in the database', function () {
-        this.db.docOps.findOne
-          .calledWith(
-            { doc_id: new ObjectId(this.docId) },
-            {
-              projection: { version: 1 },
-            }
-          )
-          .should.equal(true)
-      })
-
-      it('should return the version', function () {
-        expect(this.result).to.equal(this.version)
-      })
-    })
-
-    describe("when the doc doesn't exist", function () {
-      beforeEach(async function () {
-        this.db.docOps.findOne = sinon.stub().resolves(null)
-        this.result = await this.MongoManager.promises.getDocVersion(this.docId)
-      })
-
-      it('should return 0', function () {
-        expect(this.result).to.equal(0)
-      })
-    })
   })
 
   describe('checkRevUnchanged', function () {