[docstore] remove fallback for update pipeline (#31791)

GitOrigin-RevId: b69a7b19b9ca09e2bb6404cb2ca941a43d8ae259

services/docstore/app/js/MongoManager.js

@@ -2,8 +2,6 @@ import mongodb from './mongodb.js'
 import Settings from '@overleaf/settings'
 import Errors from './Errors.js'
 import Metrics from '@overleaf/metrics'
-import logger from '@overleaf/logger'
-import _ from 'lodash'
 
 const { db, ObjectId, BSON } = mongodb
 
@@ -120,41 +118,17 @@ async function upsertIntoDocCollection(projectId, docId, previousRev, updates) {
     const pipeline = convertUpdateToPipeline(update)
     const payloadSize = BSON.calculateObjectSize(pipeline)
     Metrics.count('mongo_docs_write', payloadSize, 1, { method: 'update' })
-    const result = await db.docs.findOneAndUpdate(
+    const result = await db.docs.updateOne(
       {
         _id: new ObjectId(docId),
         project_id: new ObjectId(projectId),
         rev: previousRev,
       },
-      pipeline,
-      { returnDocument: 'after' }
+      pipeline
     )
-    if (!result) {
+    if (result.matchedCount !== 1) {
       throw new Errors.DocRevValueError()
     }
-    let fallbackUpdate = false
-    if (updates.lines && !_.isEqual(updates.lines, result.lines)) {
-      logger.warn({ projectId, docId }, 'lines are different after pipeline')
-      fallbackUpdate = true
-    }
-    if (updates.ranges && !_.isEqual(updates.ranges, result.ranges)) {
-      logger.warn({ projectId, docId }, 'ranges are different after pipeline')
-      fallbackUpdate = true
-    }
-    if (fallbackUpdate) {
-      update.$set.rev = previousRev + 2
-      const result = await db.docs.updateOne(
-        {
-          _id: new ObjectId(docId),
-          project_id: new ObjectId(projectId),
-          rev: previousRev + 1,
-        },
-        update
-      )
-      if (result.matchedCount !== 1) {
-        throw new Errors.DocRevValueError()
-      }
-    }
   } else {
     const payloadSize = BSON.calculateObjectSize(updates)
     Metrics.count('mongo_docs_write', payloadSize, 1, { method: 'insert' })
@@ -248,39 +222,13 @@ async function restoreArchivedDoc(projectId, docId, archivedDoc) {
   const pipeline = convertUpdateToPipeline(update)
   const payloadSize = BSON.calculateObjectSize(pipeline)
   Metrics.count('mongo_docs_write', payloadSize, 1, { method: 'restore' })
-  const result = await db.docs.findOneAndUpdate(query, pipeline, {
-    returnDocument: 'after',
-  })
-  if (!result) {
+  const result = await db.docs.updateOne(query, pipeline)
+  if (result.matchedCount !== 1) {
     throw new Errors.DocRevValueError('failed to unarchive doc', {
       docId,
       rev: archivedDoc.rev,
     })
   }
-  let fallbackUpdate = false
-  if (!_.isEqual(update.$set.lines, result.lines)) {
-    logger.warn(
-      { projectId, docId },
-      'lines are different after pipeline when unarchiving'
-    )
-    fallbackUpdate = true
-  }
-  if (!_.isEqual(update.$set.ranges, result.ranges)) {
-    logger.warn(
-      { projectId, docId },
-      'ranges are different after pipeline when unarchiving'
-    )
-    fallbackUpdate = true
-  }
-  if (fallbackUpdate) {
-    const result = await db.docs.updateOne(query, update)
-    if (result.matchedCount === 0) {
-      throw new Errors.DocRevValueError('failed to unarchive doc', {
-        docId,
-        rev: archivedDoc.rev,
-      })
-    }
-  }
 }
 
 async function getDocRev(docId) {

services/docstore/test/acceptance/js/UpdatingDocsTests.js

@@ -8,13 +8,13 @@ describe('Applying updates to a doc', function () {
   beforeEach(async function () {
     this.project_id = new ObjectId()
     this.doc_id = new ObjectId()
-    this.originalLines = ['original', 'lines']
-    this.newLines = ['new', 'lines']
+    this.originalLines = ['original', 'lines', '$1.00', '$foo']
+    this.newLines = ['new', 'lines', '$2.00', '$bar']
     this.originalRanges = {
       changes: [
         {
           id: new ObjectId().toString(),
-          op: { i: 'foo', p: 3 },
+          op: { i: '$foo', p: 3 },
           meta: {
             user_id: new ObjectId().toString(),
             ts: new Date().toString(),
@@ -26,7 +26,7 @@ describe('Applying updates to a doc', function () {
       changes: [
         {
           id: new ObjectId().toString(),
-          op: { i: 'bar', p: 6 },
+          op: { i: '$bar', p: 6 },
           meta: {
             user_id: new ObjectId().toString(),
             ts: new Date().toString(),

services/docstore/test/unit/js/MongoManager.test.js

@@ -224,9 +224,6 @@ describe('MongoManager', () => {
   describe('upsertIntoDocCollection', () => {
     beforeEach(ctx => {
       ctx.oldRev = 77
-      ctx.db.docs.findOneAndUpdate.resolves({
-        lines: ctx.lines,
-      })
     })
 
     it('should upsert the document', async ctx => {
@@ -237,55 +234,17 @@ describe('MongoManager', () => {
         { lines: ctx.lines }
       )
 
-      const args = ctx.db.docs.findOneAndUpdate.args[0]
-      assert.deepEqual(args[0], {
-        _id: new ObjectId(ctx.docId),
-        project_id: new ObjectId(ctx.projectId),
-        rev: ctx.oldRev,
-      })
-      assert.equal(args[1][0].$set.lines.$literal, ctx.lines)
-      assert.equal(args[1][1].$set.rev.$literal, ctx.oldRev + 1)
-    })
-
-    it('should fallback on mismatch', async ctx => {
-      ctx.db.docs.findOneAndUpdate.resolves({
-        lines: [],
-      })
-      await ctx.MongoManager.upsertIntoDocCollection(
-        ctx.projectId,
-        ctx.docId,
-        ctx.oldRev,
-        { lines: ctx.lines }
-      )
-
       const args = ctx.db.docs.updateOne.args[0]
       assert.deepEqual(args[0], {
         _id: new ObjectId(ctx.docId),
         project_id: new ObjectId(ctx.projectId),
-        rev: ctx.oldRev + 1,
+        rev: ctx.oldRev,
       })
-      assert.equal(args[1].$set.lines, ctx.lines)
-      assert.equal(args[1].$set.rev, ctx.oldRev + 2)
+      assert.equal(args[1][0].$set.lines.$literal, ctx.lines)
+      assert.equal(args[1][1].$set.rev.$literal, ctx.oldRev + 1)
     })
 
     it('should handle update error', async ctx => {
-      ctx.db.docs.findOneAndUpdate.rejects(ctx.stubbedErr)
-      await expect(
-        ctx.MongoManager.upsertIntoDocCollection(
-          ctx.projectId,
-          ctx.docId,
-          ctx.rev,
-          {
-            lines: ctx.lines,
-          }
-        )
-      ).to.be.rejectedWith(ctx.stubbedErr)
-    })
-
-    it('should handle update error of the fallback', async ctx => {
-      ctx.db.docs.findOneAndUpdate.resolves({
-        lines: [],
-      })
       ctx.db.docs.updateOne.rejects(ctx.stubbedErr)
       await expect(
         ctx.MongoManager.upsertIntoDocCollection(
@@ -398,38 +357,7 @@ describe('MongoManager', () => {
     })
 
     describe('complete doc', () => {
-      beforeEach(async ctx => {
-        ctx.db.docs.findOneAndUpdate.resolves({
-          lines: ctx.archivedDoc.lines,
-          ranges: ctx.archivedDoc.ranges,
-        })
-      })
-
       it('updates Mongo', async ctx => {
-        await ctx.MongoManager.restoreArchivedDoc(
-          ctx.projectId,
-          ctx.docId,
-          ctx.archivedDoc
-        )
-        expect(ctx.db.docs.findOneAndUpdate).to.have.been.calledWith(
-          {
-            _id: new ObjectId(ctx.docId),
-            project_id: new ObjectId(ctx.projectId),
-            rev: ctx.archivedDoc.rev,
-          },
-          [
-            { $set: { lines: { $literal: ctx.archivedDoc.lines } } },
-            { $set: { ranges: { $literal: ctx.archivedDoc.ranges } } },
-            { $unset: 'inS3' },
-          ]
-        )
-      })
-
-      it('updates Mongo on fallback', async ctx => {
-        ctx.db.docs.findOneAndUpdate.resolves({
-          lines: ['foo'],
-          ranges: ctx.archivedDoc.ranges,
-        })
         await ctx.MongoManager.restoreArchivedDoc(
           ctx.projectId,
           ctx.docId,
@@ -441,15 +369,11 @@ describe('MongoManager', () => {
             project_id: new ObjectId(ctx.projectId),
             rev: ctx.archivedDoc.rev,
           },
-          {
-            $set: {
-              lines: ctx.archivedDoc.lines,
-              ranges: ctx.archivedDoc.ranges,
-            },
-            $unset: {
-              inS3: true,
-            },
-          }
+          [
+            { $set: { lines: { $literal: ctx.archivedDoc.lines } } },
+            { $set: { ranges: { $literal: ctx.archivedDoc.ranges } } },
+            { $unset: 'inS3' },
+          ]
         )
       })
     })
@@ -465,7 +389,7 @@ describe('MongoManager', () => {
           ctx.docId,
           ctx.archivedDoc
         )
-        expect(ctx.db.docs.findOneAndUpdate).to.have.been.calledWith(
+        expect(ctx.db.docs.updateOne).to.have.been.calledWith(
           {
             _id: new ObjectId(ctx.docId),
             project_id: new ObjectId(ctx.projectId),
@@ -488,21 +412,6 @@ describe('MongoManager', () => {
 
     describe("when the update doesn't succeed", () => {
       it('throws a DocRevValueError', async ctx => {
-        ctx.db.docs.findOneAndUpdate.resolves(null)
-        await expect(
-          ctx.MongoManager.restoreArchivedDoc(
-            ctx.projectId,
-            ctx.docId,
-            ctx.archivedDoc
-          )
-        ).to.be.rejectedWith(Errors.DocRevValueError)
-      })
-
-      it('throws a DocRevValueError on fallback', async ctx => {
-        ctx.db.docs.findOneAndUpdate.resolves({
-          lines: ['foo'],
-          ranges: ctx.archivedDoc.ranges,
-        })
         ctx.db.docs.updateOne.resolves({ matchedCount: 0 })
         await expect(
           ctx.MongoManager.restoreArchivedDoc(