Remove backwards-compat project update API

The project update endpoint accepted updates both in two array params:
docUpdates and fileUpdates, and in a single array: updates. This commit
removes the docUpdates/fileUpdates params now that web uses the updates
param.

services/document-updater/app/js/HttpController.js

@@ -325,28 +325,13 @@ function deleteComment(req, res, next) {
 function updateProject(req, res, next) {
   const timer = new Metrics.Timer('http.updateProject')
   const projectId = req.params.project_id
-  const {
-    projectHistoryId,
-    userId,
-    docUpdates,
-    fileUpdates,
-    updates,
-    version
-  } = req.body
-  logger.log(
-    { projectId, updates, docUpdates, fileUpdates, version },
-    'updating project via http'
-  )
-  const allUpdates = _mergeUpdates(
-    docUpdates || [],
-    fileUpdates || [],
-    updates || []
-  )
+  const { projectHistoryId, userId, updates = [], version } = req.body
+  logger.log({ projectId, updates, version }, 'updating project via http')
   ProjectManager.updateProjectWithLocks(
     projectId,
     projectHistoryId,
     userId,
-    allUpdates,
+    updates,
     version,
     (error) => {
       timer.done()
@@ -416,23 +401,3 @@ function flushQueuedProjects(req, res, next) {
     }
   })
 }
-
-/**
- * Merge updates from the previous project update interface (docUpdates +
- * fileUpdates) and the new update interface (updates).
- */
-function _mergeUpdates(docUpdates, fileUpdates, updates) {
-  const mergedUpdates = []
-  for (const update of docUpdates) {
-    const type = update.docLines != null ? 'add-doc' : 'rename-doc'
-    mergedUpdates.push({ type, ...update })
-  }
-  for (const update of fileUpdates) {
-    const type = update.url != null ? 'add-file' : 'rename-file'
-    mergedUpdates.push({ type, ...update })
-  }
-  for (const update of updates) {
-    mergedUpdates.push(update)
-  }
-  return mergedUpdates
-}

services/document-updater/test/acceptance/js/ApplyingUpdatesToProjectStructureTests.js

@@ -22,11 +22,12 @@ describe("Applying updates to a project's structure", function () {
     before(function (done) {
       this.project_id = DocUpdaterClient.randomId()
       this.fileUpdate = {
+        type: 'rename-file',
         id: DocUpdaterClient.randomId(),
         pathname: '/file-path',
         newPathname: '/new-file-path'
       }
-      this.fileUpdates = [this.fileUpdate]
+      this.updates = [this.fileUpdate]
       DocUpdaterApp.ensureRunning((error) => {
         if (error) {
           return done(error)
@@ -34,8 +35,7 @@ describe("Applying updates to a project's structure", function () {
         DocUpdaterClient.sendProjectUpdate(
           this.project_id,
           this.user_id,
-          [],
-          this.fileUpdates,
+          this.updates,
           this.version,
           (error) => {
             if (error) {
@@ -73,12 +73,13 @@ describe("Applying updates to a project's structure", function () {
 
   describe('renaming a document', function () {
     before(function () {
-      this.docUpdate = {
+      this.update = {
+        type: 'rename-doc',
         id: DocUpdaterClient.randomId(),
         pathname: '/doc-path',
         newPathname: '/new-doc-path'
       }
-      this.docUpdates = [this.docUpdate]
+      this.updates = [this.update]
     })
 
     describe('when the document is not loaded', function () {
@@ -87,8 +88,7 @@ describe("Applying updates to a project's structure", function () {
         DocUpdaterClient.sendProjectUpdate(
           this.project_id,
           this.user_id,
-          this.docUpdates,
-          [],
+          this.updates,
           this.version,
           (error) => {
             if (error) {
@@ -110,7 +110,7 @@ describe("Applying updates to a project's structure", function () {
             }
 
             const update = JSON.parse(updates[0])
-            update.doc.should.equal(this.docUpdate.id)
+            update.doc.should.equal(this.update.id)
             update.pathname.should.equal('/doc-path')
             update.new_pathname.should.equal('/new-doc-path')
             update.meta.user_id.should.equal(this.user_id)
@@ -126,10 +126,10 @@ describe("Applying updates to a project's structure", function () {
     describe('when the document is loaded', function () {
       before(function (done) {
         this.project_id = DocUpdaterClient.randomId()
-        MockWebApi.insertDoc(this.project_id, this.docUpdate.id, {})
+        MockWebApi.insertDoc(this.project_id, this.update.id, {})
         DocUpdaterClient.preloadDoc(
           this.project_id,
-          this.docUpdate.id,
+          this.update.id,
           (error) => {
             if (error) {
               return done(error)
@@ -138,8 +138,7 @@ describe("Applying updates to a project's structure", function () {
             DocUpdaterClient.sendProjectUpdate(
               this.project_id,
               this.user_id,
-              this.docUpdates,
-              [],
+              this.updates,
               this.version,
               (error) => {
                 if (error) {
@@ -159,12 +158,12 @@ describe("Applying updates to a project's structure", function () {
       it('should update the doc', function (done) {
         DocUpdaterClient.getDoc(
           this.project_id,
-          this.docUpdate.id,
+          this.update.id,
           (error, res, doc) => {
             if (error) {
               return done(error)
             }
-            doc.pathname.should.equal(this.docUpdate.newPathname)
+            doc.pathname.should.equal(this.update.newPathname)
             done()
           }
         )
@@ -181,7 +180,7 @@ describe("Applying updates to a project's structure", function () {
             }
 
             const update = JSON.parse(updates[0])
-            update.doc.should.equal(this.docUpdate.id)
+            update.doc.should.equal(this.update.id)
             update.pathname.should.equal('/doc-path')
             update.new_pathname.should.equal('/new-doc-path')
             update.meta.user_id.should.equal(this.user_id)
@@ -198,27 +197,35 @@ describe("Applying updates to a project's structure", function () {
   describe('renaming multiple documents and files', function () {
     before(function () {
       this.docUpdate0 = {
+        type: 'rename-doc',
         id: DocUpdaterClient.randomId(),
         pathname: '/doc-path0',
         newPathname: '/new-doc-path0'
       }
       this.docUpdate1 = {
+        type: 'rename-doc',
         id: DocUpdaterClient.randomId(),
         pathname: '/doc-path1',
         newPathname: '/new-doc-path1'
       }
-      this.docUpdates = [this.docUpdate0, this.docUpdate1]
       this.fileUpdate0 = {
+        type: 'rename-file',
         id: DocUpdaterClient.randomId(),
         pathname: '/file-path0',
         newPathname: '/new-file-path0'
       }
       this.fileUpdate1 = {
+        type: 'rename-file',
         id: DocUpdaterClient.randomId(),
         pathname: '/file-path1',
         newPathname: '/new-file-path1'
       }
-      this.fileUpdates = [this.fileUpdate0, this.fileUpdate1]
+      this.updates = [
+        this.docUpdate0,
+        this.docUpdate1,
+        this.fileUpdate0,
+        this.fileUpdate1
+      ]
     })
 
     describe('when the documents are not loaded', function () {
@@ -227,8 +234,7 @@ describe("Applying updates to a project's structure", function () {
         DocUpdaterClient.sendProjectUpdate(
           this.project_id,
           this.user_id,
-          this.docUpdates,
-          this.fileUpdates,
+          this.updates,
           this.version,
           (error) => {
             if (error) {
@@ -292,16 +298,16 @@ describe("Applying updates to a project's structure", function () {
     before(function (done) {
       this.project_id = DocUpdaterClient.randomId()
       this.fileUpdate = {
+        type: 'add-file',
         id: DocUpdaterClient.randomId(),
         pathname: '/file-path',
         url: 'filestore.example.com'
       }
-      this.fileUpdates = [this.fileUpdate]
+      this.updates = [this.fileUpdate]
       DocUpdaterClient.sendProjectUpdate(
         this.project_id,
         this.user_id,
-        [],
-        this.fileUpdates,
+        this.updates,
         this.version,
         (error) => {
           if (error) {
@@ -340,16 +346,16 @@ describe("Applying updates to a project's structure", function () {
     before(function (done) {
       this.project_id = DocUpdaterClient.randomId()
       this.docUpdate = {
+        type: 'add-doc',
         id: DocUpdaterClient.randomId(),
         pathname: '/file-path',
         docLines: 'a\nb'
       }
-      this.docUpdates = [this.docUpdate]
+      this.updates = [this.docUpdate]
       DocUpdaterClient.sendProjectUpdate(
         this.project_id,
         this.user_id,
-        this.docUpdates,
-        [],
+        this.updates,
         this.version,
         (error) => {
           if (error) {
@@ -394,6 +400,7 @@ describe("Applying updates to a project's structure", function () {
       for (let v = 0; v <= 599; v++) {
         // Should flush after 500 ops
         updates.push({
+          type: 'add-doc',
           id: DocUpdaterClient.randomId(),
           pathname: '/file-' + v,
           docLines: 'a\nb'
@@ -409,7 +416,6 @@ describe("Applying updates to a project's structure", function () {
         projectId,
         userId,
         updates.slice(0, 250),
-        [],
         this.version0,
         function (error) {
           if (error) {
@@ -419,7 +425,6 @@ describe("Applying updates to a project's structure", function () {
             projectId,
             userId,
             updates.slice(250),
-            [],
             this.version1,
             (error) => {
               if (error) {
@@ -454,6 +459,7 @@ describe("Applying updates to a project's structure", function () {
       for (let v = 0; v <= 42; v++) {
         // Should flush after 500 ops
         updates.push({
+          type: 'add-doc',
           id: DocUpdaterClient.randomId(),
           pathname: '/file-' + v,
           docLines: 'a\nb'
@@ -469,7 +475,6 @@ describe("Applying updates to a project's structure", function () {
         projectId,
         userId,
         updates.slice(0, 10),
-        [],
         this.version0,
         function (error) {
           if (error) {
@@ -479,7 +484,6 @@ describe("Applying updates to a project's structure", function () {
             projectId,
             userId,
             updates.slice(10),
-            [],
             this.version1,
             (error) => {
               if (error) {

services/document-updater/test/acceptance/js/helpers/DocUpdaterClient.js

@@ -185,18 +185,11 @@ module.exports = DocUpdaterClient = {
     )
   },
 
-  sendProjectUpdate(
-    projectId,
-    userId,
-    docUpdates,
-    fileUpdates,
-    version,
-    callback
-  ) {
+  sendProjectUpdate(projectId, userId, updates, version, callback) {
     request.post(
       {
         url: `http://localhost:3003/project/${projectId}`,
-        json: { userId, docUpdates, fileUpdates, version }
+        json: { userId, updates, version }
       },
       (error, res, body) => callback(error, res, body)
     )

services/document-updater/test/unit/js/HttpController/HttpControllerTests.js

@@ -809,92 +809,7 @@ describe('HttpController', function () {
     })
   })
 
-  describe('updateProject (split doc and file updates)', function () {
-    beforeEach(function () {
-      this.projectHistoryId = 'history-id-123'
-      this.userId = 'user-id-123'
-      this.docUpdates = [
-        { id: 1, pathname: 'thesis.tex', newPathname: 'book.tex' },
-        { id: 2, pathname: 'article.tex', docLines: 'hello' }
-      ]
-      this.fileUpdates = [
-        { id: 3, pathname: 'apple.png', newPathname: 'banana.png' },
-        { id: 4, url: 'filestore.example.com/4' }
-      ]
-      this.expectedUpdates = [
-        {
-          type: 'rename-doc',
-          id: 1,
-          pathname: 'thesis.tex',
-          newPathname: 'book.tex'
-        },
-        { type: 'add-doc', id: 2, pathname: 'article.tex', docLines: 'hello' },
-        {
-          type: 'rename-file',
-          id: 3,
-          pathname: 'apple.png',
-          newPathname: 'banana.png'
-        },
-        { type: 'add-file', id: 4, url: 'filestore.example.com/4' }
-      ]
-      this.version = 1234567
-      this.req = {
-        query: {},
-        body: {
-          projectHistoryId: this.projectHistoryId,
-          userId: this.userId,
-          docUpdates: this.docUpdates,
-          fileUpdates: this.fileUpdates,
-          version: this.version
-        },
-        params: {
-          project_id: this.project_id
-        }
-      }
-    })
-
-    describe('successfully', function () {
-      beforeEach(function () {
-        this.ProjectManager.updateProjectWithLocks = sinon.stub().yields()
-        this.HttpController.updateProject(this.req, this.res, this.next)
-      })
-
-      it('should accept the change', function () {
-        this.ProjectManager.updateProjectWithLocks
-          .calledWith(
-            this.project_id,
-            this.projectHistoryId,
-            this.userId,
-            this.expectedUpdates,
-            this.version
-          )
-          .should.equal(true)
-      })
-
-      it('should return a successful No Content response', function () {
-        this.res.sendStatus.calledWith(204).should.equal(true)
-      })
-
-      it('should time the request', function () {
-        this.Metrics.Timer.prototype.done.called.should.equal(true)
-      })
-    })
-
-    describe('when an errors occurs', function () {
-      beforeEach(function () {
-        this.ProjectManager.updateProjectWithLocks = sinon
-          .stub()
-          .yields(new Error('oops'))
-        this.HttpController.updateProject(this.req, this.res, this.next)
-      })
-
-      it('should call next with the error', function () {
-        this.next.calledWith(sinon.match.instanceOf(Error)).should.equal(true)
-      })
-    })
-  })
-
-  describe('updateProject (single updates parameter)', function () {
+  describe('updateProject', function () {
     beforeEach(function () {
       this.projectHistoryId = 'history-id-123'
       this.userId = 'user-id-123'