diff --git a/services/document-updater/app/js/HttpController.js b/services/document-updater/app/js/HttpController.js index d2904eb898..5e47cf5bf1 100644 --- a/services/document-updater/app/js/HttpController.js +++ b/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 -} diff --git a/services/document-updater/test/acceptance/js/ApplyingUpdatesToProjectStructureTests.js b/services/document-updater/test/acceptance/js/ApplyingUpdatesToProjectStructureTests.js index 8be9291fdf..58fe5d13eb 100644 --- a/services/document-updater/test/acceptance/js/ApplyingUpdatesToProjectStructureTests.js +++ b/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) { diff --git a/services/document-updater/test/acceptance/js/helpers/DocUpdaterClient.js b/services/document-updater/test/acceptance/js/helpers/DocUpdaterClient.js index 10813e72fa..9e0ee6462f 100644 --- a/services/document-updater/test/acceptance/js/helpers/DocUpdaterClient.js +++ b/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) ) diff --git a/services/document-updater/test/unit/js/HttpController/HttpControllerTests.js b/services/document-updater/test/unit/js/HttpController/HttpControllerTests.js index 4d9790926a..07e9d93c9a 100644 --- a/services/document-updater/test/unit/js/HttpController/HttpControllerTests.js +++ b/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'