Merge pull request #12229 from overleaf/jpa-reject-decrement-doc-version

[docstore] reject doc updates that decrement the doc version

GitOrigin-RevId: 533cc5ece8d5684c85b2f63fa2a093c68f6b5877

services/docstore/app.js

@@ -91,6 +91,8 @@ app.use(function (error, req, res, next) {
     return res.sendStatus(404)
   } else if (error instanceof Errors.DocModifiedError) {
     return res.sendStatus(409)
+  } else if (error instanceof Errors.DocVersionDecrementedError) {
+    return res.sendStatus(409)
   } else {
     return res.status(500).send('Oops, something went wrong')
   }

services/docstore/app/js/DocManager.js

@@ -263,6 +263,16 @@ module.exports = DocManager = {
           updateVersion = true
           updateRanges = true
         } else {
+          if (doc.version > version) {
+            // Reject update when the version was decremented.
+            // Potential reasons: racing flush, broken history.
+            return callback(
+              new Errors.DocVersionDecrementedError('rejecting stale update', {
+                updateVersion: version,
+                flushedVersion: doc.version,
+              })
+            )
+          }
           updateLines = !_.isEqual(doc.lines, lines)
           updateVersion = doc.version !== version
           updateRanges = RangeManager.shouldUpdateRanges(doc.ranges, ranges)

services/docstore/app/js/Errors.js

@@ -8,9 +8,12 @@ class DocModifiedError extends OError {}
 
 class DocRevValueError extends OError {}
 
+class DocVersionDecrementedError extends OError {}
+
 module.exports = {
   Md5MismatchError,
   DocModifiedError,
   DocRevValueError,
+  DocVersionDecrementedError,
   ...Errors,
 }

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

@@ -179,6 +179,43 @@ describe('Applying updates to a doc', function () {
     })
   })
 
+  describe('when the version was decremented', function () {
+    beforeEach(function (done) {
+      DocstoreClient.updateDoc(
+        this.project_id,
+        this.doc_id,
+        this.newLines,
+        this.version - 1,
+        this.newRanges,
+        (error, res, body) => {
+          if (error) return done(error)
+          this.res = res
+          this.body = body
+          done()
+        }
+      )
+    })
+
+    it('should return 409', function () {
+      this.res.statusCode.should.equal(409)
+    })
+
+    it('should not update the doc in the API', function (done) {
+      DocstoreClient.getDoc(
+        this.project_id,
+        this.doc_id,
+        {},
+        (error, res, doc) => {
+          if (error) return done(error)
+          doc.lines.should.deep.equal(this.originalLines)
+          doc.version.should.equal(this.version)
+          doc.ranges.should.deep.equal(this.originalRanges)
+          done()
+        }
+      )
+    })
+  })
+
   describe('when the ranges have changed', function () {
     beforeEach(function (done) {
       return DocstoreClient.updateDoc(

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

@@ -796,6 +796,26 @@ describe('DocManager', function () {
       })
     })
 
+    describe('when the version was decremented', function () {
+      beforeEach(function () {
+        this.DocManager._getDoc = sinon.stub().yields(null, this.doc)
+        this.DocManager.updateDoc(
+          this.project_id,
+          this.doc_id,
+          this.newDocLines,
+          this.version - 1,
+          this.originalRanges,
+          this.callback
+        )
+      })
+
+      it('should return an error', function () {
+        this.callback.should.have.been.calledWith(
+          sinon.match.instanceOf(Errors.DocVersionDecrementedError)
+        )
+      })
+    })
+
     describe('when the doc lines have not changed', function () {
       beforeEach(function () {
         this.DocManager._getDoc = sinon.stub().callsArgWith(3, null, this.doc)