Merge pull request #9563 from overleaf/em-tpds-merge-metadata

Return metadata from TPDS update endpoint in web

GitOrigin-RevId: 9154be67f7f975807c6e986a5d6fb66013c9a384

services/web/app/src/Features/Editor/EditorController.js

@@ -213,7 +213,7 @@ const EditorController = {
                 userId
               )
             }
-            callback()
+            callback(null, doc)
           }
         )
       }
@@ -273,7 +273,7 @@ const EditorController = {
               linkedFileData,
               userId
             )
-            callback()
+            callback(null, newFile)
           }
         )
       }

services/web/app/src/Features/Project/ProjectEntityMongoUpdateHandler.js

@@ -43,6 +43,7 @@ module.exports = {
     'project',
     'path',
     'newProject',
+    'newFileRef',
   ]),
   replaceDocWithFile: callbackify(replaceDocWithFile),
   replaceFileWithDoc: callbackify(replaceFileWithDoc),
@@ -188,7 +189,9 @@ async function replaceFileWithNew(projectId, fileId, newFileRef) {
       path,
     })
   }
-  return { oldFileRef: fileRef, project, path, newProject }
+  // Refresh newFileRef with the version returned from the database
+  newFileRef = ProjectLocator.findElementByMongoPath(newProject, path.mongo)
+  return { oldFileRef: fileRef, project, path, newProject, newFileRef }
 }
 
 async function replaceDocWithFile(projectId, docId, fileRef) {

services/web/app/src/Features/Project/ProjectEntityUpdateHandler.js

@@ -343,6 +343,7 @@ const ProjectEntityUpdateHandler = {
             if (err != null) {
               return callback(err)
             }
+            doc.rev = rev
             next(
               projectId,
               folderId,
@@ -576,7 +577,7 @@ const ProjectEntityUpdateHandler = {
       projectId,
       fileId,
       newFileRef,
-      (err, oldFileRef, project, path, newProject) => {
+      (err, oldFileRef, project, path, newProject, newFileRef) => {
         if (err != null) {
           return callback(err)
         }
@@ -597,18 +598,12 @@ const ProjectEntityUpdateHandler = {
           project.overleaf &&
           project.overleaf.history &&
           project.overleaf.history.id
-        // Increment the rev for an in-place update (with the same path) so the third-party-datastore
-        // knows this is a new file.
-        // Ideally we would get this from ProjectEntityMongoUpdateHandler.replaceFileWithNew
-        // but it returns the original oldFileRef (after incrementing the rev value in mongo),
-        // so we add 1 to the rev from that. This isn't atomic and relies on the lock
-        // but it is acceptable for now.
         TpdsUpdateSender.addFile(
           {
             projectId: project._id,
             fileId: newFileRef._id,
             path: path.fileSystem,
-            rev: oldFileRef.rev + 1,
+            rev: newFileRef.rev,
             projectName: project.name,
             folderId,
           },
@@ -624,7 +619,12 @@ const ProjectEntityUpdateHandler = {
               userId,
               { oldFiles, newFiles, newProject },
               source,
-              callback
+              err => {
+                if (err) {
+                  return callback(err)
+                }
+                callback(null, newFileRef)
+              }
             )
           }
         )
@@ -670,6 +670,7 @@ const ProjectEntityUpdateHandler = {
               if (err != null) {
                 return callback(err)
               }
+              doc.rev = rev
               ProjectEntityMongoUpdateHandler.replaceFileWithDoc(
                 projectId,
                 existingFile._id,
@@ -936,7 +937,7 @@ const ProjectEntityUpdateHandler = {
               fileStoreUrl,
               folderId,
               source,
-              err => {
+              (err, newFileRef) => {
                 if (err != null) {
                   return callback(err)
                 }

services/web/app/src/Features/Project/ProjectLocator.js

@@ -1,5 +1,6 @@
 const _ = require('underscore')
 const logger = require('@overleaf/logger')
+const OError = require('@overleaf/o-error')
 const async = require('async')
 const ProjectGetter = require('./ProjectGetter')
 const Errors = require('../Errors/Errors')
@@ -282,10 +283,31 @@ function getIndexOf(searchEntity, id) {
   }
 }
 
+/**
+ * Follow the given Mongo path (as returned by findElement) and return the
+ * entity at the end of it.
+ */
+function findElementByMongoPath(project, mongoPath) {
+  const components = mongoPath.split('.')
+  let node = project
+  for (const component of components) {
+    const key = Array.isArray(node) ? parseInt(component, 10) : component
+    node = node[key]
+    if (node == null) {
+      throw new OError('entity not found', {
+        projectId: project._id,
+        mongoPath,
+      })
+    }
+  }
+  return node
+}
+
 module.exports = {
   findElement,
   findElementByPath,
   findRootDoc,
+  findElementByMongoPath,
   promises: {
     findElement: promisifyMultiResult(findElement, [
       'element',

services/web/app/src/Features/ThirdPartyDataStore/TpdsController.js

@@ -9,17 +9,19 @@ const NotificationsBuilder = require('../Notifications/NotificationsBuilder')
 const SessionManager = require('../Authentication/SessionManager')
 const TpdsQueueManager = require('./TpdsQueueManager')
 
-// mergeUpdate and deleteUpdate are used by Dropbox, where the project is only passed as the name, as the
-// first part of the file path. They have to check the project exists, find it, and create it if not.
-// They also ignore 'noisy' files like .DS_Store, .gitignore, etc.
+// mergeUpdate and deleteUpdate are used by Dropbox, where the project is only
+// passed as the name, as the first part of the file path. They have to check
+// the project exists, find it, and create it if not.  They also ignore 'noisy'
+// files like .DS_Store, .gitignore, etc.
 
 async function mergeUpdate(req, res) {
   metrics.inc('tpds.merge-update')
   const { filePath, userId, projectName } = parseParams(req)
   const source = req.headers['x-sl-update-source'] || 'unknown'
 
+  let metadata
   try {
-    await TpdsUpdateHandler.promises.newUpdate(
+    metadata = await TpdsUpdateHandler.promises.newUpdate(
       userId,
       projectName,
       filePath,
@@ -45,7 +47,19 @@ async function mergeUpdate(req, res) {
     }
   }
 
-  res.sendStatus(200)
+  if (metadata == null) {
+    return res.json({ status: 'rejected' })
+  }
+
+  const payload = {
+    status: 'applied',
+    entityId: metadata.entityId.toString(),
+    entityType: metadata.entityType,
+  }
+  if (metadata.rev != null) {
+    payload.rev = metadata.rev
+  }
+  res.json(payload)
 }
 
 async function deleteUpdate(req, res) {
@@ -62,10 +76,11 @@ async function deleteUpdate(req, res) {
   res.sendStatus(200)
 }
 
-// updateProjectContents and deleteProjectContents are used by GitHub. The project_id is known so we
-// can skip right ahead to creating/updating/deleting the file. These methods will not ignore noisy
-// files like .DS_Store, .gitignore, etc because people are generally more explicit with the files they
-// want in git.
+// updateProjectContents and deleteProjectContents are used by GitHub. The
+// project_id is known so we can skip right ahead to creating/updating/deleting
+// the file. These methods will not ignore noisy files like .DS_Store,
+// .gitignore, etc because people are generally more explicit with the files
+// they want in git.
 
 async function updateProjectContents(req, res, next) {
   const projectId = req.params.project_id

services/web/app/src/Features/ThirdPartyDataStore/TpdsUpdateHandler.js

@@ -22,7 +22,7 @@ const rootDocResets = new BackgroundTaskTracker('root doc resets')
 async function newUpdate(userId, projectName, path, updateRequest, source) {
   const project = await getOrCreateProject(userId, projectName)
   if (project == null) {
-    return
+    return null
   }
 
   const projectIsOnCooldown =
@@ -33,16 +33,17 @@ async function newUpdate(userId, projectName, path, updateRequest, source) {
 
   const shouldIgnore = await FileTypeManager.promises.shouldIgnore(path)
   if (shouldIgnore) {
-    return
+    return null
   }
 
-  await UpdateMerger.promises.mergeUpdate(
+  const metadata = await UpdateMerger.promises.mergeUpdate(
     userId,
     project._id,
     path,
     updateRequest,
     source
   )
+  return metadata
 }
 
 async function deleteUpdate(userId, projectName, path, source) {

services/web/app/src/Features/ThirdPartyDataStore/UpdateMerger.js

@@ -13,7 +13,8 @@ async function mergeUpdate(userId, projectId, path, updateRequest, source) {
     updateRequest
   )
   try {
-    await _mergeUpdate(userId, projectId, path, fsPath, source)
+    const metadata = await _mergeUpdate(userId, projectId, path, fsPath, source)
+    return metadata
   } finally {
     try {
       await fsPromises.unlink(fsPath)
@@ -52,54 +53,37 @@ async function _determineFileType(projectId, path, fsPath) {
   // sync, so we'll treat non-utf8 files as binary
   const isBinary = binary || encoding !== 'utf-8'
 
-  // Existing | Update    | Action
-  // ---------|-----------|-------
-  // file     | isBinary  | existing-file
-  // file     | !isBinary | existing-file
-  // doc      | isBinary  | new-file, delete-existing-doc
-  // doc      | !isBinary | existing-doc
-  // null     | isBinary  | new-file
-  // null     | !isBinary | new-doc
+  // Existing | Update    | Resulting file type
+  // ---------|-----------|--------------------
+  // file     | isBinary  | file
+  // file     | !isBinary | file
+  // doc      | isBinary  | file
+  // doc      | !isBinary | doc
+  // null     | isBinary  | file
+  // null     | !isBinary | doc
 
   // if a binary file already exists, always keep it as a binary file
   // even if the update looks like a text file
   if (existingFileType === 'file') {
-    return { fileType: 'existing-file' }
-  }
-
-  // if there is an existing doc, keep it as a doc except when the
-  // incoming update is binary. In that case delete the doc and replace
-  // it with a new file.
-  if (existingFileType === 'doc') {
-    if (isBinary) {
-      return {
-        fileType: 'new-file',
-        deleteOriginalEntity: 'delete-existing-doc',
-      }
-    } else {
-      return { fileType: 'existing-doc' }
-    }
+    return 'file'
+  } else {
+    return isBinary ? 'file' : 'doc'
   }
-
-  // if there no existing file, create a file or doc as needed
-  return { fileType: isBinary ? 'new-file' : 'new-doc' }
 }
 
 async function _mergeUpdate(userId, projectId, path, fsPath, source) {
-  const { fileType, deleteOriginalEntity } = await _determineFileType(
-    projectId,
-    path,
-    fsPath
-  )
-
-  if (deleteOriginalEntity) {
-    await deleteUpdate(userId, projectId, path, source)
-  }
-
-  if (['existing-file', 'new-file'].includes(fileType)) {
-    await _processFile(projectId, fsPath, path, source, userId)
-  } else if (['existing-doc', 'new-doc'].includes(fileType)) {
-    await _processDoc(projectId, userId, fsPath, path, source)
+  const fileType = await _determineFileType(projectId, path, fsPath)
+
+  if (fileType === 'file') {
+    const file = await _processFile(projectId, fsPath, path, source, userId)
+    return { entityType: 'file', entityId: file._id, rev: file.rev }
+  } else if (fileType === 'doc') {
+    const doc = await _processDoc(projectId, userId, fsPath, path, source)
+    // The doc entry doesn't have a rev. Since the document is set in
+    // docupdater, it's possible that the next rev contains a merge of changes
+    // in Dropbox and changes from docupdater.
+    const metadata = { entityType: 'doc', entityId: doc._id, rev: doc.rev }
+    return metadata
   } else {
     throw new Error('unrecognized file')
   }
@@ -124,17 +108,18 @@ async function deleteUpdate(userId, projectId, path, source) {
 async function _processDoc(projectId, userId, fsPath, path, source) {
   const docLines = await _readFileIntoTextArray(fsPath)
   logger.debug({ docLines }, 'processing doc update from tpds')
-  await EditorController.promises.upsertDocWithPath(
+  const doc = await EditorController.promises.upsertDocWithPath(
     projectId,
     path,
     docLines,
     source,
     userId
   )
+  return doc
 }
 
 async function _processFile(projectId, fsPath, path, source, userId) {
-  await EditorController.promises.upsertFileWithPath(
+  const file = await EditorController.promises.upsertFileWithPath(
     projectId,
     path,
     fsPath,
@@ -142,6 +127,7 @@ async function _processFile(projectId, fsPath, path, source, userId) {
     source,
     userId
   )
+  return file
 }
 
 async function _readFileIntoTextArray(path) {

services/web/test/acceptance/src/TpdsUpdateTests.js

@@ -1,16 +1,3 @@
-/* eslint-disable
-    camelcase,
-    max-len,
-*/
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS101: Remove unnecessary use of Array.from
- * DS102: Remove unnecessary code created because of implicit returns
- * DS207: Consider shorter variations of null checks
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
 const { expect } = require('chai')
 const ProjectGetter = require('../../../app/src/Features/Project/ProjectGetter.js')
 const request = require('./helpers/request')
@@ -19,19 +6,19 @@ const User = require('./helpers/User')
 describe('TpdsUpdateTests', function () {
   beforeEach(function (done) {
     this.owner = new User()
-    return this.owner.login(error => {
-      if (error != null) {
+    this.owner.login(error => {
+      if (error) {
         throw error
       }
-      return this.owner.createProject(
+      this.owner.createProject(
         'test-project',
         { template: 'example' },
-        (error, project_id) => {
-          if (error != null) {
+        (error, projectId) => {
+          if (error) {
             throw error
           }
-          this.project_id = project_id
-          return done()
+          this.projectId = projectId
+          done()
         }
       )
     })
@@ -39,10 +26,10 @@ describe('TpdsUpdateTests', function () {
 
   describe('adding a file', function () {
     beforeEach(function (done) {
-      return request(
+      request(
         {
           method: 'POST',
-          url: `/project/${this.project_id}/contents/test.tex`,
+          url: `/project/${this.projectId}/contents/test.tex`,
           auth: {
             username: 'sharelatex',
             password: 'password',
@@ -51,34 +38,34 @@ describe('TpdsUpdateTests', function () {
           body: 'test one two',
         },
         (error, response, body) => {
-          if (error != null) {
+          if (error) {
             throw error
           }
           expect(response.statusCode).to.equal(200)
-          return done()
+          done()
         }
       )
     })
 
     it('should have added the file', function (done) {
-      return ProjectGetter.getProject(this.project_id, (error, project) => {
-        if (error != null) {
+      ProjectGetter.getProject(this.projectId, (error, project) => {
+        if (error) {
           throw error
         }
         const projectFolder = project.rootFolder[0]
         const file = projectFolder.docs.find(e => e.name === 'test.tex')
         expect(file).to.exist
-        return done()
+        done()
       })
     })
   })
 
   describe('deleting a file', function () {
     beforeEach(function (done) {
-      return request(
+      request(
         {
           method: 'DELETE',
-          url: `/project/${this.project_id}/contents/main.tex`,
+          url: `/project/${this.projectId}/contents/main.tex`,
           auth: {
             username: 'sharelatex',
             password: 'password',
@@ -86,34 +73,34 @@ describe('TpdsUpdateTests', function () {
           },
         },
         (error, response, body) => {
-          if (error != null) {
+          if (error) {
             throw error
           }
           expect(response.statusCode).to.equal(200)
-          return done()
+          done()
         }
       )
     })
 
     it('should have deleted the file', function (done) {
-      return ProjectGetter.getProject(this.project_id, (error, project) => {
-        if (error != null) {
+      ProjectGetter.getProject(this.projectId, (error, project) => {
+        if (error) {
           throw error
         }
         const projectFolder = project.rootFolder[0]
-        for (const doc of Array.from(projectFolder.docs)) {
+        for (const doc of projectFolder.docs) {
           if (doc.name === 'main.tex') {
             throw new Error('expected main.tex to have been deleted')
           }
         }
-        return done()
+        done()
       })
     })
   })
 
   describe('update a new file', function () {
     beforeEach(function (done) {
-      return request(
+      request(
         {
           method: 'POST',
           url: `/user/${this.owner._id}/update/test-project/other.tex`,
@@ -125,24 +112,29 @@ describe('TpdsUpdateTests', function () {
           body: 'test one two',
         },
         (error, response, body) => {
-          if (error != null) {
+          if (error) {
             throw error
           }
           expect(response.statusCode).to.equal(200)
-          return done()
+          const json = JSON.parse(response.body)
+          expect(json.status).to.equal('applied')
+          expect(json.entityType).to.equal('doc')
+          expect(json).to.have.property('entityId')
+          expect(json).to.have.property('rev')
+          done()
         }
       )
     })
 
     it('should have added the file', function (done) {
-      return ProjectGetter.getProject(this.project_id, (error, project) => {
-        if (error != null) {
+      ProjectGetter.getProject(this.projectId, (error, project) => {
+        if (error) {
           throw error
         }
         const projectFolder = project.rootFolder[0]
         const file = projectFolder.docs.find(e => e.name === 'other.tex')
         expect(file).to.exist
-        return done()
+        done()
       })
     })
   })
@@ -151,12 +143,12 @@ describe('TpdsUpdateTests', function () {
     beforeEach(function (done) {
       this.owner.request(
         {
-          url: `/Project/${this.project_id}/archive`,
+          url: `/Project/${this.projectId}/archive`,
           method: 'post',
         },
         (err, response, body) => {
           expect(err).to.not.exist
-          return request(
+          request(
             {
               method: 'POST',
               url: `/user/${this.owner._id}/update/test-project/test.tex`,
@@ -168,11 +160,13 @@ describe('TpdsUpdateTests', function () {
               body: 'test one two',
             },
             (error, response, body) => {
-              if (error != null) {
+              if (error) {
                 throw error
               }
               expect(response.statusCode).to.equal(200)
-              return done()
+              const json = JSON.parse(response.body)
+              expect(json.status).to.equal('rejected')
+              done()
             }
           )
         }
@@ -192,14 +186,14 @@ describe('TpdsUpdateTests', function () {
     })
 
     it('should not have added the file', function (done) {
-      return ProjectGetter.getProject(this.project_id, (error, project) => {
-        if (error != null) {
+      ProjectGetter.getProject(this.projectId, (error, project) => {
+        if (error) {
           throw error
         }
         const projectFolder = project.rootFolder[0]
         const file = projectFolder.docs.find(e => e.name === 'test.tex')
         expect(file).to.not.exist
-        return done()
+        done()
       })
     })
   })

services/web/test/unit/src/Project/ProjectEntityMongoUpdateHandlerTests.js

@@ -77,6 +77,7 @@ describe('ProjectEntityMongoUpdateHandler', function () {
       getAllEntitiesFromProject: sinon.stub(),
     }
     this.ProjectLocator = {
+      findElementByMongoPath: sinon.stub().throws(new Error('not found')),
       promises: {
         findElement: sinon.stub().rejects(new Error('not found')),
         findElementByPath: sinon.stub().rejects(new Error('not found')),
@@ -388,6 +389,9 @@ describe('ProjectEntityMongoUpdateHandler', function () {
         )
         .chain('exec')
         .resolves(this.project)
+      this.ProjectLocator.findElementByMongoPath
+        .withArgs(this.project, 'rootFolder.0.fileRefs.0')
+        .returns(newFile)
       await this.subject.promises.replaceFileWithNew(
         this.project._id,
         this.file._id,

services/web/test/unit/src/Project/ProjectEntityUpdateHandlerTests.js

@@ -34,7 +34,7 @@ describe('ProjectEntityUpdateHandler', function () {
         this.name = options.name
         this.lines = options.lines
         this._id = docId
-        this.rev = 0
+        this.rev = options.rev ?? 0
       }
     }
     this.FileModel = class File {
@@ -516,14 +516,15 @@ describe('ProjectEntityUpdateHandler', function () {
     describe('adding a doc', function () {
       beforeEach(function () {
         this.path = '/path/to/doc'
+        this.rev = 5
 
         this.newDoc = new this.DocModel({
           name: this.docName,
           lines: undefined,
           _id: docId,
-          rev: 0,
+          rev: this.rev,
         })
-        this.DocstoreManager.updateDoc.yields(null, false, (this.rev = 5))
+        this.DocstoreManager.updateDoc.yields(null, false, this.rev)
         this.TpdsUpdateSender.addDoc.yields()
         this.ProjectEntityMongoUpdateHandler.addDoc.yields(
           null,
@@ -555,18 +556,16 @@ describe('ProjectEntityUpdateHandler', function () {
             docLines: this.docLines.join('\n'),
           },
         ]
-        this.DocumentUpdaterHandler.updateProjectStructure
-          .calledWith(
-            projectId,
-            projectHistoryId,
-            userId,
-            {
-              newDocs,
-              newProject: this.project,
-            },
-            this.source
-          )
-          .should.equal(true)
+        this.DocumentUpdaterHandler.updateProjectStructure.should.have.been.calledWith(
+          projectId,
+          projectHistoryId,
+          userId,
+          {
+            newDocs,
+            newProject: this.project,
+          },
+          this.source
+        )
       })
     })
 
@@ -1020,6 +1019,7 @@ describe('ProjectEntityUpdateHandler', function () {
     describe('updating an existing file', function () {
       beforeEach(function () {
         this.existingFile = { _id: fileId, name: this.fileName, rev: 1 }
+        this.newFile = { _id: ObjectId(), name: this.fileName, rev: 3 }
         this.folder = { _id: folderId, fileRefs: [this.existingFile], docs: [] }
         this.ProjectLocator.findElement.yields(null, this.folder)
         this.newProject = 'new-project-stub'
@@ -1028,7 +1028,8 @@ describe('ProjectEntityUpdateHandler', function () {
           this.existingFile,
           this.project,
           { fileSystem: this.fileSystemPath },
-          this.newProject
+          this.newProject,
+          this.newFile
         )
         this.ProjectEntityUpdateHandler.upsertFile(
           projectId,
@@ -1065,8 +1066,8 @@ describe('ProjectEntityUpdateHandler', function () {
         this.TpdsUpdateSender.addFile.should.have.been.calledWith({
           projectId,
           projectName: this.project.name,
-          fileId: this.file._id,
-          rev: this.existingFile.rev + 1,
+          fileId: this.newFile._id,
+          rev: this.newFile.rev,
           path: this.fileSystemPath,
           folderId,
         })
@@ -1088,7 +1089,7 @@ describe('ProjectEntityUpdateHandler', function () {
         ]
         const newFiles = [
           {
-            file: this.file,
+            file: this.newFile,
             path: this.fileSystemPath,
             url: this.fileUrl,
           },

services/web/test/unit/src/Project/ProjectLocatorTests.js

@@ -557,4 +557,23 @@ describe('ProjectLocator', function () {
       })
     })
   })
+
+  describe('findElementByMongoPath', function () {
+    it('traverses the file tree like Mongo would do', function () {
+      const element = this.locator.findElementByMongoPath(
+        project,
+        'rootFolder.0.folders.1.folders.0.fileRefs.0'
+      )
+      expect(element).to.equal(subSubFile)
+    })
+
+    it('throws an error if no element is found', function () {
+      expect(() =>
+        this.locator.findElementByMongoPath(
+          project,
+          'rootolder.0.folders.0.folders.0.fileRefs.0'
+        )
+      ).to.throw
+    })
+  })
 })

services/web/test/unit/src/ThirdPartyDataStore/TpdsControllerTests.js

@@ -1,6 +1,7 @@
 const SandboxedModule = require('sandboxed-module')
 const sinon = require('sinon')
 const { expect } = require('chai')
+const { ObjectId } = require('mongodb')
 const Errors = require('../../../../app/src/Features/Errors/Errors')
 
 const MODULE_PATH =
@@ -8,15 +9,20 @@ const MODULE_PATH =
 
 describe('TpdsController', function () {
   beforeEach(function () {
+    this.metadata = {
+      entityId: ObjectId(),
+      entityType: 'doc',
+      rev: 2,
+    }
     this.TpdsUpdateHandler = {
       promises: {
-        newUpdate: sinon.stub().resolves(),
+        newUpdate: sinon.stub().resolves(this.metadata),
         deleteUpdate: sinon.stub().resolves(),
       },
     }
     this.UpdateMerger = {
       promises: {
-        mergeUpdate: sinon.stub().resolves(),
+        mergeUpdate: sinon.stub().resolves(this.file),
         deleteUpdate: sinon.stub().resolves(),
       },
     }
@@ -46,70 +52,65 @@ describe('TpdsController', function () {
   })
 
   describe('getting an update', function () {
-    it('should process the update with the update receiver', function (done) {
-      const path = '/projectName/here.txt'
-      const req = {
-        pause() {},
-        params: { 0: path, user_id: this.user_id },
-        session: {
-          destroy() {},
-        },
+    beforeEach(function () {
+      this.projectName = 'projectName'
+      this.path = '/here.txt'
+      this.req = {
+        params: { 0: `${this.projectName}${this.path}`, user_id: this.user_id },
         headers: {
           'x-sl-update-source': (this.source = 'dropbox'),
         },
       }
+    })
+
+    it('should process the update with the update receiver', function (done) {
       const res = {
-        sendStatus: () => {
+        json: payload => {
+          expect(payload).to.deep.equal({
+            status: 'applied',
+            entityId: this.metadata.entityId.toString(),
+            entityType: this.metadata.entityType,
+            rev: this.metadata.rev,
+          })
           this.TpdsUpdateHandler.promises.newUpdate
             .calledWith(
               this.user_id,
-              'projectName',
-              '/here.txt',
-              req,
+              this.projectName,
+              this.path,
+              this.req,
               this.source
             )
             .should.equal(true)
           done()
         },
       }
-      this.TpdsController.mergeUpdate(req, res)
+      this.TpdsController.mergeUpdate(this.req, res)
     })
 
-    it('should return a 500 error when the update receiver fails', function (done) {
-      const path = '/projectName/here.txt'
-      const req = {
-        pause() {},
-        params: { 0: path, user_id: this.user_id },
-        session: {
-          destroy() {},
-        },
-        headers: {
-          'x-sl-update-source': (this.source = 'dropbox'),
+    it('should indicate in the response when the update was rejected', function (done) {
+      this.TpdsUpdateHandler.promises.newUpdate.resolves(null)
+      const res = {
+        json: payload => {
+          expect(payload).to.deep.equal({ status: 'rejected' })
+          done()
         },
       }
+      this.TpdsController.mergeUpdate(this.req, res)
+    })
+
+    it('should return a 500 error when the update receiver fails', function (done) {
       this.TpdsUpdateHandler.promises.newUpdate.rejects(new Error())
       const res = {
-        sendStatus: sinon.stub(),
+        json: sinon.stub(),
       }
-      this.TpdsController.mergeUpdate(req, res, err => {
+      this.TpdsController.mergeUpdate(this.req, res, err => {
         expect(err).to.exist
-        expect(res.sendStatus).not.to.have.been.called
+        expect(res.json).not.to.have.been.called
         done()
       })
     })
 
     it('should return a 400 error when the project is too big', function (done) {
-      const path = '/projectName/here.txt'
-      const req = {
-        pause() {},
-        params: { 0: path, user_id: this.user_id, projectName: 'projectName' },
-        session: {
-          destroy() {},
-        },
-        headers: {
-          'x-sl-update-source': (this.source = 'dropbox'),
-        },
-      }
       this.TpdsUpdateHandler.promises.newUpdate.rejects({
         message: 'project_has_too_many_files',
       })
@@ -122,21 +123,10 @@ describe('TpdsController', function () {
           done()
         },
       }
-      this.TpdsController.mergeUpdate(req, res)
+      this.TpdsController.mergeUpdate(this.req, res)
     })
 
     it('should return a 429 error when the update receiver fails due to too many requests error', function (done) {
-      const path = '/projectName/here.txt'
-      const req = {
-        pause() {},
-        params: { 0: path, user_id: this.user_id },
-        session: {
-          destroy() {},
-        },
-        headers: {
-          'x-sl-update-source': (this.source = 'dropbox'),
-        },
-      }
       this.TpdsUpdateHandler.promises.newUpdate.rejects(
         new Errors.TooManyRequestsError('project on cooldown')
       )
@@ -146,7 +136,7 @@ describe('TpdsController', function () {
           done()
         },
       }
-      this.TpdsController.mergeUpdate(req, res)
+      this.TpdsController.mergeUpdate(this.req, res)
     })
   })
 

services/web/test/unit/src/ThirdPartyDataStore/UpdateMergerTests.js

@@ -2,6 +2,7 @@ const SandboxedModule = require('sandboxed-module')
 const sinon = require('sinon')
 const { expect } = require('chai')
 const BufferedStream = require('bufferedstream')
+const { ObjectId } = require('mongodb')
 
 const MODULE_PATH =
   '../../../../app/src/Features/ThirdPartyDataStore/UpdateMerger.js'
@@ -38,11 +39,21 @@ describe('UpdateMerger :', function () {
       readFile: sinon.stub().withArgs(this.fsPath).resolves(this.fileContents),
     }
 
+    this.doc = {
+      _id: ObjectId(),
+      rev: 2,
+    }
+
+    this.file = {
+      _id: ObjectId(),
+      rev: 6,
+    }
+
     this.EditorController = {
       promises: {
         deleteEntityWithPath: sinon.stub().resolves(),
-        upsertDocWithPath: sinon.stub().resolves(),
-        upsertFileWithPath: sinon.stub().resolves(),
+        upsertDocWithPath: sinon.stub().resolves(this.doc),
+        upsertFileWithPath: sinon.stub().resolves(this.file),
       },
     }
 
@@ -239,17 +250,6 @@ describe('UpdateMerger :', function () {
       expect(this.FileTypeManager.promises.getType).to.have.been.called
     })
 
-    it('should delete the existing doc', function () {
-      expect(
-        this.EditorController.promises.deleteEntityWithPath
-      ).to.have.been.calledWith(
-        this.projectId,
-        this.existingDocPath,
-        this.source,
-        this.userId
-      )
-    })
-
     it('should process update as file', function () {
       expect(
         this.EditorController.promises.upsertFileWithPath