diff --git a/services/docstore/app/js/MongoManager.js b/services/docstore/app/js/MongoManager.js index 9064294222..a6833c9a7f 100644 --- a/services/docstore/app/js/MongoManager.js +++ b/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) { diff --git a/services/docstore/test/acceptance/js/UpdatingDocsTests.js b/services/docstore/test/acceptance/js/UpdatingDocsTests.js index e46f346478..1fb190f2ea 100644 --- a/services/docstore/test/acceptance/js/UpdatingDocsTests.js +++ b/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(), diff --git a/services/docstore/test/unit/js/MongoManager.test.js b/services/docstore/test/unit/js/MongoManager.test.js index 3ad3f8c505..53ce0f4a0e 100644 --- a/services/docstore/test/unit/js/MongoManager.test.js +++ b/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(