Merge pull request #5826 from overleaf/bg-use-resync-path

[document-updater] use paths from project structure update in resync

GitOrigin-RevId: 6ed2a48da76553e2fe5d0d3ec8da535515a35110

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
     )
   },

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,

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)

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)
     })