diff --git a/services/document-updater/app/js/ProjectManager.js b/services/document-updater/app/js/ProjectManager.js index df6be021ad..aad6077371 100644 --- a/services/document-updater/app/js/ProjectManager.js +++ b/services/document-updater/app/js/ProjectManager.js @@ -238,17 +238,48 @@ function updateProjectWithLocks( ) break case 'rename-doc': - DocumentManager.renameDocWithLock( - projectId, - update.id, - userId, - update, - projectHistoryId, - (error, count) => { - projectOpsLength = count - cb(error) - } - ) + if (!update.newPathname) { + // an empty newPathname signifies a delete, so there is no need to + // update the pathname in redis + ProjectHistoryRedisManager.queueRenameEntity( + projectId, + projectHistoryId, + 'doc', + update.id, + userId, + update, + (error, count) => { + projectOpsLength = count + cb(error) + } + ) + } else { + // rename the doc in redis before queuing the update + DocumentManager.renameDocWithLock( + projectId, + update.id, + userId, + update, + projectHistoryId, + error => { + if (error) { + return cb(error) + } + ProjectHistoryRedisManager.queueRenameEntity( + projectId, + projectHistoryId, + 'doc', + update.id, + userId, + update, + (error, count) => { + projectOpsLength = count + cb(error) + } + ) + } + ) + } break case 'add-file': ProjectHistoryRedisManager.queueAddEntity( diff --git a/services/document-updater/app/js/RedisManager.js b/services/document-updater/app/js/RedisManager.js index 0cf700d8d4..89346dfba9 100644 --- a/services/document-updater/app/js/RedisManager.js +++ b/services/document-updater/app/js/RedisManager.js @@ -593,36 +593,14 @@ module.exports = RedisManager = { if (error != null) { return callback(error) } - if (lines != null && version != null) { return rclient.set( keys.pathname({ doc_id }), update.newPathname, - function (error) { - if (error != null) { - return callback(error) - } - return ProjectHistoryRedisManager.queueRenameEntity( - project_id, - projectHistoryId, - 'doc', - doc_id, - user_id, - update, - callback - ) - } - ) - } else { - return ProjectHistoryRedisManager.queueRenameEntity( - project_id, - projectHistoryId, - 'doc', - doc_id, - user_id, - update, callback ) + } else { + return callback() } } ) diff --git a/services/document-updater/test/acceptance/js/ApplyingUpdatesToProjectStructureTests.js b/services/document-updater/test/acceptance/js/ApplyingUpdatesToProjectStructureTests.js index 6d4fc3c019..9e825a4f4b 100644 --- a/services/document-updater/test/acceptance/js/ApplyingUpdatesToProjectStructureTests.js +++ b/services/document-updater/test/acceptance/js/ApplyingUpdatesToProjectStructureTests.js @@ -69,6 +69,54 @@ describe("Applying updates to a project's structure", function () { }) }) + describe('deleting a file', function () { + before(function (done) { + this.project_id = DocUpdaterClient.randomId() + this.fileUpdate = { + type: 'rename-file', + id: DocUpdaterClient.randomId(), + pathname: '/file-path', + newPathname: '', + } + this.updates = [this.fileUpdate] + DocUpdaterClient.sendProjectUpdate( + this.project_id, + this.user_id, + this.updates, + this.version, + error => { + if (error) { + return done(error) + } + setTimeout(done, 200) + } + ) + }) + + it('should push the applied file renames to the project history api', function (done) { + rclientProjectHistory.lrange( + ProjectHistoryKeys.projectHistoryOps({ project_id: this.project_id }), + 0, + -1, + (error, updates) => { + if (error) { + return done(error) + } + + const update = JSON.parse(updates[0]) + update.file.should.equal(this.fileUpdate.id) + update.pathname.should.equal('/file-path') + update.new_pathname.should.equal('') + update.meta.user_id.should.equal(this.user_id) + update.meta.ts.should.be.a('string') + update.version.should.equal(`${this.version}.0`) + + done() + } + ) + }) + }) + describe('renaming a document', function () { before(function () { this.update = { @@ -288,6 +336,126 @@ describe("Applying updates to a project's structure", function () { }) }) + describe('deleting a document', function () { + before(function () { + this.update = { + type: 'rename-doc', + id: DocUpdaterClient.randomId(), + pathname: '/doc-path', + newPathname: '', + } + this.updates = [this.update] + }) + + describe('when the document is not loaded', function () { + before(function (done) { + this.project_id = DocUpdaterClient.randomId() + DocUpdaterClient.sendProjectUpdate( + this.project_id, + this.user_id, + this.updates, + this.version, + error => { + if (error) { + return done(error) + } + setTimeout(done, 200) + } + ) + }) + + it('should push the applied doc update to the project history api', function (done) { + rclientProjectHistory.lrange( + ProjectHistoryKeys.projectHistoryOps({ project_id: this.project_id }), + 0, + -1, + (error, updates) => { + if (error) { + return done(error) + } + + const update = JSON.parse(updates[0]) + update.doc.should.equal(this.update.id) + update.pathname.should.equal('/doc-path') + update.new_pathname.should.equal('') + update.meta.user_id.should.equal(this.user_id) + update.meta.ts.should.be.a('string') + update.version.should.equal(`${this.version}.0`) + + done() + } + ) + }) + }) + + describe('when the document is loaded', function () { + before(function (done) { + this.project_id = DocUpdaterClient.randomId() + MockWebApi.insertDoc(this.project_id, this.update.id, {}) + DocUpdaterClient.preloadDoc(this.project_id, this.update.id, error => { + if (error) { + return done(error) + } + sinon.spy(MockWebApi, 'getDocument') + DocUpdaterClient.sendProjectUpdate( + this.project_id, + this.user_id, + this.updates, + this.version, + error => { + if (error) { + return done(error) + } + setTimeout(done, 200) + } + ) + }) + }) + + after(function () { + MockWebApi.getDocument.restore() + }) + + it('should not modify the doc', function (done) { + DocUpdaterClient.getDoc( + this.project_id, + this.update.id, + (error, res, doc) => { + if (error) { + return done(error) + } + + doc.pathname.should.equal('/a/b/c.tex') // default pathname from MockWebApi + done() + } + ) + }) + + it('should push the applied doc update to the project history api', function (done) { + rclientProjectHistory.lrange( + ProjectHistoryKeys.projectHistoryOps({ project_id: this.project_id }), + 0, + -1, + (error, updates) => { + if (error) { + return done(error) + } + + const update = JSON.parse(updates[0]) + update.doc.should.equal(this.update.id) + update.pathname.should.equal('/doc-path') + update.new_pathname.should.equal('') + update.meta.user_id.should.equal(this.user_id) + update.meta.ts.should.be.a('string') + update.version.should.equal(`${this.version}.0`) + + done() + } + ) + }) + }) + }) + describe('adding a file', function () { before(function (done) { this.project_id = DocUpdaterClient.randomId() diff --git a/services/document-updater/test/unit/js/RedisManager/RedisManagerTests.js b/services/document-updater/test/unit/js/RedisManager/RedisManagerTests.js index 78341d6687..cedeb9a34f 100644 --- a/services/document-updater/test/unit/js/RedisManager/RedisManagerTests.js +++ b/services/document-updater/test/unit/js/RedisManager/RedisManagerTests.js @@ -1097,20 +1097,6 @@ describe('RedisManager', function () { .calledWith(`Pathname:${this.doc_id}`, this.newPathname) .should.equal(true) }) - - return it('should queue an update', function () { - return this.ProjectHistoryRedisManager.queueRenameEntity - .calledWithExactly( - this.project_id, - this.projectHistoryId, - 'doc', - this.doc_id, - this.userId, - this.update, - this.callback - ) - .should.equal(true) - }) }) describe('the document is not cached in redis', function () { @@ -1134,20 +1120,6 @@ describe('RedisManager', function () { it('does not update the cached pathname', function () { return this.rclient.set.called.should.equal(false) }) - - return it('should queue an update', function () { - return this.ProjectHistoryRedisManager.queueRenameEntity - .calledWithExactly( - this.project_id, - this.projectHistoryId, - 'doc', - this.doc_id, - this.userId, - this.update, - this.callback - ) - .should.equal(true) - }) }) return describe('getDocVersion', function () {