diff --git a/services/document-updater/app/js/DocumentManager.js b/services/document-updater/app/js/DocumentManager.js index cbedeea2a2..45d3aa80df 100644 --- a/services/document-updater/app/js/DocumentManager.js +++ b/services/document-updater/app/js/DocumentManager.js @@ -564,8 +564,8 @@ module.exports = DocumentManager = { ) }, - resyncDocContents(project_id, doc_id, callback) { - logger.debug({ project_id, doc_id }, 'start resyncing doc contents') + resyncDocContents(project_id, doc_id, path, callback) { + logger.debug({ project_id, doc_id, path }, 'start resyncing doc contents') return RedisManager.getDoc( project_id, doc_id, @@ -573,7 +573,11 @@ module.exports = DocumentManager = { if (error != null) { return callback(error) } - + // To avoid issues where the same doc_id appears with different paths, + // we use the path from the resyncProjectStructure update. If we used + // the path from the getDoc call to web then the two occurences of the + // doc_id would map to the same path, and this would be rejected by + // project-history as an unexpected resyncDocContent update. if (lines == null || version == null) { logger.debug( { project_id, doc_id }, @@ -604,7 +608,7 @@ module.exports = DocumentManager = { doc_id, lines, version, - pathname, + path, // use the path from the resyncProjectStructure update callback ) } @@ -620,7 +624,7 @@ module.exports = DocumentManager = { doc_id, lines, version, - pathname, + path, // use the path from the resyncProjectStructure update callback ) } @@ -768,7 +772,7 @@ module.exports = DocumentManager = { ) }, - resyncDocContentsWithLock(project_id, doc_id, callback) { + resyncDocContentsWithLock(project_id, doc_id, path, callback) { if (callback == null) { callback = function () {} } @@ -777,6 +781,7 @@ module.exports = DocumentManager = { DocumentManager.resyncDocContents, project_id, doc_id, + path, callback ) }, diff --git a/services/document-updater/app/js/HistoryManager.js b/services/document-updater/app/js/HistoryManager.js index a3e8f306d2..97b7c6de96 100644 --- a/services/document-updater/app/js/HistoryManager.js +++ b/services/document-updater/app/js/HistoryManager.js @@ -242,8 +242,14 @@ module.exports = HistoryManager = { return callback(error) } const DocumentManager = require('./DocumentManager') - const resyncDoc = (doc, cb) => - DocumentManager.resyncDocContentsWithLock(project_id, doc.doc, cb) + const resyncDoc = (doc, cb) => { + DocumentManager.resyncDocContentsWithLock( + project_id, + doc.doc, + doc.path, + cb + ) + } return async.eachLimit( docs, HistoryManager.MAX_PARALLEL_REQUESTS, diff --git a/services/document-updater/test/unit/js/DocumentManager/DocumentManagerTests.js b/services/document-updater/test/unit/js/DocumentManager/DocumentManagerTests.js index 9a3aa95c5e..f4203f5997 100644 --- a/services/document-updater/test/unit/js/DocumentManager/DocumentManagerTests.js +++ b/services/document-updater/test/unit/js/DocumentManager/DocumentManagerTests.js @@ -975,6 +975,7 @@ describe('DocumentManager', function () { describe('resyncDocContents', function () { describe('when doc is loaded in redis', function () { beforeEach(function () { + this.pathnameFromProjectStructureUpdate = '/foo/bar.tex' this.RedisManager.getDoc = sinon .stub() .callsArgWith( @@ -990,6 +991,7 @@ describe('DocumentManager', function () { this.DocumentManager.resyncDocContents( this.project_id, this.doc_id, + this.pathnameFromProjectStructureUpdate, this.callback ) }) @@ -1008,7 +1010,7 @@ describe('DocumentManager', function () { this.doc_id, this.lines, this.version, - this.pathname, + this.pathnameFromProjectStructureUpdate, this.callback ) .should.equal(true) @@ -1017,6 +1019,7 @@ describe('DocumentManager', function () { describe('when doc is not loaded in redis', function () { beforeEach(function () { + this.pathnameFromProjectStructureUpdate = '/foo/bar.tex' this.RedisManager.getDoc = sinon.stub().callsArgWith(2, null) this.PersistenceManager.getDoc = sinon .stub() @@ -1032,6 +1035,7 @@ describe('DocumentManager', function () { this.DocumentManager.resyncDocContents( this.project_id, this.doc_id, + this.pathnameFromProjectStructureUpdate, this.callback ) }) @@ -1056,7 +1060,7 @@ describe('DocumentManager', function () { this.doc_id, this.lines, this.version, - this.pathname, + this.pathnameFromProjectStructureUpdate, this.callback ) .should.equal(true) diff --git a/services/document-updater/test/unit/js/HistoryManager/HistoryManagerTests.js b/services/document-updater/test/unit/js/HistoryManager/HistoryManagerTests.js index c040d51300..ecc5aacf02 100644 --- a/services/document-updater/test/unit/js/HistoryManager/HistoryManagerTests.js +++ b/services/document-updater/test/unit/js/HistoryManager/HistoryManagerTests.js @@ -416,7 +416,7 @@ describe('HistoryManager', function () { it('should queue doc content reyncs', function () { return this.DocumentManager.resyncDocContentsWithLock - .calledWith(this.project_id, this.doc_id) + .calledWith(this.project_id, this.docs[0].doc, this.docs[0].path) .should.equal(true) })