Merge pull request #2849 from overleaf/em-convert-file-to-doc

Fix update order when converting a file to a doc

GitOrigin-RevId: a0c9488e3870cc972c21b40ff0e2577fcec40ee0

services/web/app/src/Features/DocumentUpdater/DocumentUpdaterHandler.js

@@ -1,445 +1,395 @@
-/* eslint-disable
-    camelcase,
-    handle-callback-err,
-    max-len,
-    no-unused-vars,
-*/
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS101: Remove unnecessary use of Array.from
- * DS102: Remove unnecessary code created because of implicit returns
- * DS103: Rewrite code to no longer use __guard__
- * DS207: Consider shorter variations of null checks
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
-let DocumentUpdaterHandler
-let request = require('request')
-request = request.defaults()
+const request = require('request').defaults()
 const settings = require('settings-sharelatex')
 const _ = require('underscore')
 const async = require('async')
 const logger = require('logger-sharelatex')
 const metrics = require('metrics-sharelatex')
-const { Project } = require('../../models/Project')
-const { promisifyAll } = require('../../util/promises')
+const { promisify } = require('util')
 
-module.exports = DocumentUpdaterHandler = {
-  flushProjectToMongo(project_id, callback) {
-    if (callback == null) {
-      callback = function(error) {}
-    }
-    return DocumentUpdaterHandler._makeRequest(
-      {
-        path: `/project/${project_id}/flush`,
-        method: 'POST'
-      },
-      project_id,
-      'flushing.mongo.project',
-      callback
-    )
-  },
+module.exports = {
+  flushProjectToMongo,
+  flushMultipleProjectsToMongo,
+  flushProjectToMongoAndDelete,
+  flushDocToMongo,
+  deleteDoc,
+  getDocument,
+  setDocument,
+  getProjectDocsIfMatch,
+  clearProjectState,
+  acceptChanges,
+  deleteThread,
+  resyncProjectHistory,
+  updateProjectStructure,
+  promises: {
+    flushProjectToMongo: promisify(flushProjectToMongo),
+    flushMultipleProjectsToMongo: promisify(flushMultipleProjectsToMongo),
+    flushProjectToMongoAndDelete: promisify(flushProjectToMongoAndDelete),
+    flushDocToMongo: promisify(flushDocToMongo),
+    deleteDoc: promisify(deleteDoc),
+    getDocument: promisify(getDocument),
+    setDocument: promisify(setDocument),
+    getProjectDocsIfMatch: promisify(getProjectDocsIfMatch),
+    clearProjectState: promisify(clearProjectState),
+    acceptChanges: promisify(acceptChanges),
+    deleteThread: promisify(deleteThread),
+    resyncProjectHistory: promisify(resyncProjectHistory),
+    updateProjectStructure: promisify(updateProjectStructure)
+  }
+}
 
-  flushMultipleProjectsToMongo(project_ids, callback) {
-    if (callback == null) {
-      callback = function(error) {}
-    }
-    const jobs = []
-    for (let project_id of Array.from(project_ids)) {
-      ;(project_id =>
-        jobs.push(callback =>
-          DocumentUpdaterHandler.flushProjectToMongo(project_id, callback)
-        ))(project_id)
-    }
-    return async.series(jobs, callback)
-  },
+function flushProjectToMongo(projectId, callback) {
+  _makeRequest(
+    {
+      path: `/project/${projectId}/flush`,
+      method: 'POST'
+    },
+    projectId,
+    'flushing.mongo.project',
+    callback
+  )
+}
 
-  flushProjectToMongoAndDelete(project_id, callback) {
-    if (callback == null) {
-      callback = function() {}
-    }
-    const timer = new metrics.Timer('delete.mongo.project')
-    const url = `${settings.apis.documentupdater.url}`
-    return DocumentUpdaterHandler._makeRequest(
-      {
-        path: `/project/${project_id}`,
-        method: 'DELETE'
-      },
-      project_id,
-      'flushing.mongo.project',
-      callback
-    )
-  },
+function flushMultipleProjectsToMongo(projectIds, callback) {
+  const jobs = projectIds.map(projectId => callback => {
+    flushProjectToMongo(projectId, callback)
+  })
+  async.series(jobs, callback)
+}
 
-  flushDocToMongo(project_id, doc_id, callback) {
-    if (callback == null) {
-      callback = function(error) {}
-    }
-    return DocumentUpdaterHandler._makeRequest(
-      {
-        path: `/project/${project_id}/doc/${doc_id}/flush`,
-        method: 'POST'
-      },
-      project_id,
-      'flushing.mongo.doc',
-      callback
-    )
-  },
+function flushProjectToMongoAndDelete(projectId, callback) {
+  _makeRequest(
+    {
+      path: `/project/${projectId}`,
+      method: 'DELETE'
+    },
+    projectId,
+    'flushing.mongo.project',
+    callback
+  )
+}
 
-  deleteDoc(project_id, doc_id, callback) {
-    if (callback == null) {
-      callback = function() {}
-    }
-    return DocumentUpdaterHandler._makeRequest(
-      {
-        path: `/project/${project_id}/doc/${doc_id}`,
-        method: 'DELETE'
-      },
-      project_id,
-      'delete.mongo.doc',
-      callback
-    )
-  },
+function flushDocToMongo(projectId, docId, callback) {
+  _makeRequest(
+    {
+      path: `/project/${projectId}/doc/${docId}/flush`,
+      method: 'POST'
+    },
+    projectId,
+    'flushing.mongo.doc',
+    callback
+  )
+}
 
-  getDocument(project_id, doc_id, fromVersion, callback) {
-    if (callback == null) {
-      callback = function(error, doclines, version, ranges, ops) {}
-    }
-    return DocumentUpdaterHandler._makeRequest(
-      {
-        path: `/project/${project_id}/doc/${doc_id}?fromVersion=${fromVersion}`,
-        json: true
-      },
-      project_id,
-      'get-document',
-      function(error, doc) {
-        if (error != null) {
-          return callback(error)
-        }
-        return callback(null, doc.lines, doc.version, doc.ranges, doc.ops)
-      }
-    )
-  },
+function deleteDoc(projectId, docId, callback) {
+  _makeRequest(
+    {
+      path: `/project/${projectId}/doc/${docId}`,
+      method: 'DELETE'
+    },
+    projectId,
+    'delete.mongo.doc',
+    callback
+  )
+}
 
-  setDocument(project_id, doc_id, user_id, docLines, source, callback) {
-    if (callback == null) {
-      callback = function(error) {}
-    }
-    return DocumentUpdaterHandler._makeRequest(
-      {
-        path: `/project/${project_id}/doc/${doc_id}`,
-        method: 'POST',
-        json: {
-          lines: docLines,
-          source,
-          user_id
-        }
-      },
-      project_id,
-      'set-document',
-      callback
-    )
-  },
-
-  getProjectDocsIfMatch(project_id, projectStateHash, callback) {
-    // If the project state hasn't changed, we can get all the latest
-    // docs from redis via the docupdater. Otherwise we will need to
-    // fall back to getting them from mongo.
-    if (callback == null) {
-      callback = function(error, docs) {}
-    }
-    const timer = new metrics.Timer('get-project-docs')
-    const url = `${
-      settings.apis.documentupdater.url
-    }/project/${project_id}/get_and_flush_if_old?state=${projectStateHash}`
-    return request.post(url, function(error, res, body) {
-      timer.done()
-      if (error != null) {
-        logger.warn(
-          { err: error, url, project_id },
-          'error getting project docs from doc updater'
-        )
+function getDocument(projectId, docId, fromVersion, callback) {
+  _makeRequest(
+    {
+      path: `/project/${projectId}/doc/${docId}?fromVersion=${fromVersion}`,
+      json: true
+    },
+    projectId,
+    'get-document',
+    function(error, doc) {
+      if (error) {
         return callback(error)
       }
-      if (res.statusCode === 409) {
-        // HTTP response code "409 Conflict"
-        // Docupdater has checked the projectStateHash and found that
-        // it has changed. This means that the docs currently in redis
-        // aren't the only change to the project and the full set of
-        // docs/files should be retreived from docstore/filestore
-        // instead.
-        return callback()
-      } else if (res.statusCode >= 200 && res.statusCode < 300) {
-        let docs
-        try {
-          docs = JSON.parse(body)
-        } catch (error1) {
-          error = error1
-          return callback(error)
-        }
-        return callback(null, docs)
-      } else {
-        logger.warn(
-          { project_id, url },
+      callback(null, doc.lines, doc.version, doc.ranges, doc.ops)
+    }
+  )
+}
+
+function setDocument(projectId, docId, userId, docLines, source, callback) {
+  _makeRequest(
+    {
+      path: `/project/${projectId}/doc/${docId}`,
+      method: 'POST',
+      json: {
+        lines: docLines,
+        source,
+        user_id: userId
+      }
+    },
+    projectId,
+    'set-document',
+    callback
+  )
+}
+
+function getProjectDocsIfMatch(projectId, projectStateHash, callback) {
+  // If the project state hasn't changed, we can get all the latest
+  // docs from redis via the docupdater. Otherwise we will need to
+  // fall back to getting them from mongo.
+  const timer = new metrics.Timer('get-project-docs')
+  const url = `${
+    settings.apis.documentupdater.url
+  }/project/${projectId}/get_and_flush_if_old?state=${projectStateHash}`
+  request.post(url, function(error, res, body) {
+    timer.done()
+    if (error) {
+      logger.warn(
+        { err: error, url, projectId },
+        'error getting project docs from doc updater'
+      )
+      return callback(error)
+    }
+    if (res.statusCode === 409) {
+      // HTTP response code "409 Conflict"
+      // Docupdater has checked the projectStateHash and found that
+      // it has changed. This means that the docs currently in redis
+      // aren't the only change to the project and the full set of
+      // docs/files should be retreived from docstore/filestore
+      // instead.
+      callback()
+    } else if (res.statusCode >= 200 && res.statusCode < 300) {
+      let docs
+      try {
+        docs = JSON.parse(body)
+      } catch (error1) {
+        error = error1
+        return callback(error)
+      }
+      callback(null, docs)
+    } else {
+      logger.warn(
+        { projectId, url },
+        `doc updater returned a non-success status code: ${res.statusCode}`
+      )
+      callback(
+        new Error(
           `doc updater returned a non-success status code: ${res.statusCode}`
         )
-        return callback(
-          new Error(
-            `doc updater returned a non-success status code: ${res.statusCode}`
-          )
-        )
-      }
-    })
-  },
-
-  clearProjectState(project_id, callback) {
-    if (callback == null) {
-      callback = function(error) {}
-    }
-    return DocumentUpdaterHandler._makeRequest(
-      {
-        path: `/project/${project_id}/clearState`,
-        method: 'POST'
-      },
-      project_id,
-      'clear-project-state',
-      callback
-    )
-  },
-
-  acceptChanges(project_id, doc_id, change_ids, callback) {
-    if (change_ids == null) {
-      change_ids = []
-    }
-    if (callback == null) {
-      callback = function(error) {}
-    }
-    return DocumentUpdaterHandler._makeRequest(
-      {
-        path: `/project/${project_id}/doc/${doc_id}/change/accept`,
-        json: {
-          change_ids
-        },
-        method: 'POST'
-      },
-      project_id,
-      'accept-changes',
-      callback
-    )
-  },
-
-  deleteThread(project_id, doc_id, thread_id, callback) {
-    if (callback == null) {
-      callback = function(error) {}
-    }
-    const timer = new metrics.Timer('delete-thread')
-    return DocumentUpdaterHandler._makeRequest(
-      {
-        path: `/project/${project_id}/doc/${doc_id}/comment/${thread_id}`,
-        method: 'DELETE'
-      },
-      project_id,
-      'delete-thread',
-      callback
-    )
-  },
-
-  resyncProjectHistory(project_id, projectHistoryId, docs, files, callback) {
-    return DocumentUpdaterHandler._makeRequest(
-      {
-        path: `/project/${project_id}/history/resync`,
-        json: { docs, files, projectHistoryId },
-        method: 'POST'
-      },
-      project_id,
-      'resync-project-history',
-      callback
-    )
-  },
-
-  updateProjectStructure(
-    project_id,
-    projectHistoryId,
-    userId,
-    changes,
-    callback
-  ) {
-    if (callback == null) {
-      callback = function(error) {}
-    }
-    if (
-      !(settings.apis.project_history != null
-        ? settings.apis.project_history.sendProjectStructureOps
-        : undefined)
-    ) {
-      return callback()
-    }
-
-    const docUpdates = DocumentUpdaterHandler._getUpdates(
-      'doc',
-      changes.oldDocs,
-      changes.newDocs
-    )
-    const fileUpdates = DocumentUpdaterHandler._getUpdates(
-      'file',
-      changes.oldFiles,
-      changes.newFiles
-    )
-    const projectVersion = __guard__(
-      changes != null ? changes.newProject : undefined,
-      x => x.version
-    )
-
-    if (docUpdates.length + fileUpdates.length < 1) {
-      return callback()
-    }
-
-    if (projectVersion == null) {
-      logger.warn(
-        { project_id, changes, projectVersion },
-        'did not receive project version in changes'
       )
-      return callback(new Error('did not receive project version in changes'))
     }
+  })
+}
 
-    return DocumentUpdaterHandler._makeRequest(
-      {
-        path: `/project/${project_id}`,
-        json: {
-          docUpdates,
-          fileUpdates,
-          userId,
-          version: projectVersion,
-          projectHistoryId
-        },
-        method: 'POST'
-      },
-      project_id,
-      'update-project-structure',
-      callback
-    )
-  },
+function clearProjectState(projectId, callback) {
+  _makeRequest(
+    {
+      path: `/project/${projectId}/clearState`,
+      method: 'POST'
+    },
+    projectId,
+    'clear-project-state',
+    callback
+  )
+}
 
-  _makeRequest(options, project_id, metricsKey, callback) {
-    const timer = new metrics.Timer(metricsKey)
-    return request(
-      {
-        url: `${settings.apis.documentupdater.url}${options.path}`,
-        json: options.json,
-        method: options.method || 'GET'
-      },
-      function(error, res, body) {
-        timer.done()
-        if (error != null) {
-          logger.warn(
-            { error, project_id },
-            'error making request to document updater'
-          )
-          return callback(error)
-        } else if (res.statusCode >= 200 && res.statusCode < 300) {
-          return callback(null, body)
-        } else {
-          error = new Error(
-            `document updater returned a failure status code: ${res.statusCode}`
-          )
-          logger.warn(
-            { error, project_id },
-            `document updater returned failure status code: ${res.statusCode}`
-          )
-          return callback(error)
-        }
-      }
-    )
-  },
+function acceptChanges(projectId, docId, changeIds, callback) {
+  _makeRequest(
+    {
+      path: `/project/${projectId}/doc/${docId}/change/accept`,
+      json: { change_ids: changeIds },
+      method: 'POST'
+    },
+    projectId,
+    'accept-changes',
+    callback
+  )
+}
 
-  _getUpdates(entityType, oldEntities, newEntities) {
-    let id, newEntity, oldEntity
-    if (!oldEntities) {
-      oldEntities = []
-    }
-    if (!newEntities) {
-      newEntities = []
-    }
-    const updates = []
+function deleteThread(projectId, docId, threadId, callback) {
+  _makeRequest(
+    {
+      path: `/project/${projectId}/doc/${docId}/comment/${threadId}`,
+      method: 'DELETE'
+    },
+    projectId,
+    'delete-thread',
+    callback
+  )
+}
 
-    const oldEntitiesHash = _.indexBy(oldEntities, entity =>
-      entity[entityType]._id.toString()
-    )
-    const newEntitiesHash = _.indexBy(newEntities, entity =>
-      entity[entityType]._id.toString()
-    )
+function resyncProjectHistory(
+  projectId,
+  projectHistoryId,
+  docs,
+  files,
+  callback
+) {
+  _makeRequest(
+    {
+      path: `/project/${projectId}/history/resync`,
+      json: { docs, files, projectHistoryId },
+      method: 'POST'
+    },
+    projectId,
+    'resync-project-history',
+    callback
+  )
+}
 
-    // Send deletes before adds (and renames) to keep a 1:1 mapping between
-    // paths and ids
-    //
-    // When a file is replaced, we first delete the old file and then add the
-    // new file. If the 'add' operation is sent to project history before the
-    // 'delete' then we would have two files with the same path at that point
-    // in time.
-    for (id in oldEntitiesHash) {
-      oldEntity = oldEntitiesHash[id]
-      newEntity = newEntitiesHash[id]
-
-      if (newEntity == null) {
-        // entity deleted
-        updates.push({
-          id,
-          pathname: oldEntity.path,
-          newPathname: ''
-        })
-      }
-    }
-
-    for (id in newEntitiesHash) {
-      newEntity = newEntitiesHash[id]
-      oldEntity = oldEntitiesHash[id]
-
-      if (oldEntity == null) {
-        // entity added
-        updates.push({
-          id,
-          pathname: newEntity.path,
-          docLines: newEntity.docLines,
-          url: newEntity.url,
-          hash: newEntity.file != null ? newEntity.file.hash : undefined
-        })
-      } else if (newEntity.path !== oldEntity.path) {
-        // entity renamed
-        updates.push({
-          id,
-          pathname: oldEntity.path,
-          newPathname: newEntity.path
-        })
-      }
-    }
-
-    return updates
+function updateProjectStructure(
+  projectId,
+  projectHistoryId,
+  userId,
+  changes,
+  callback
+) {
+  if (
+    settings.apis.project_history == null ||
+    !settings.apis.project_history.sendProjectStructureOps
+  ) {
+    return callback()
   }
-}
-module.exports.promises = promisifyAll(DocumentUpdaterHandler, {
-  without: ['_getUpdates']
-})
 
-const PENDINGUPDATESKEY = 'PendingUpdates'
-const DOCLINESKEY = 'doclines'
-const DOCIDSWITHPENDINGUPDATES = 'DocsWithPendingUpdates'
+  const {
+    deletes: docDeletes,
+    adds: docAdds,
+    renames: docRenames
+  } = _getUpdates('doc', changes.oldDocs, changes.newDocs)
+  const {
+    deletes: fileDeletes,
+    adds: fileAdds,
+    renames: fileRenames
+  } = _getUpdates('file', changes.oldFiles, changes.newFiles)
+  const updates = [].concat(
+    docDeletes,
+    fileDeletes,
+    docAdds,
+    fileAdds,
+    docRenames,
+    fileRenames
+  )
+  const projectVersion =
+    changes && changes.newProject && changes.newProject.version
 
-const keys = {
-  pendingUpdates(op) {
-    return `${PENDINGUPDATESKEY}:${op.doc_id}`
-  },
-  docsWithPendingUpdates: DOCIDSWITHPENDINGUPDATES,
-  docLines(op) {
-    return `${DOCLINESKEY}:${op.doc_id}`
-  },
-  combineProjectIdAndDocId(project_id, doc_id) {
-    return `${project_id}:${doc_id}`
+  if (updates.length < 1) {
+    return callback()
   }
+
+  if (projectVersion == null) {
+    logger.warn(
+      { projectId, changes, projectVersion },
+      'did not receive project version in changes'
+    )
+    return callback(new Error('did not receive project version in changes'))
+  }
+
+  _makeRequest(
+    {
+      path: `/project/${projectId}`,
+      json: {
+        updates,
+        userId,
+        version: projectVersion,
+        projectHistoryId
+      },
+      method: 'POST'
+    },
+    projectId,
+    'update-project-structure',
+    callback
+  )
 }
 
-function __guard__(value, transform) {
-  return typeof value !== 'undefined' && value !== null
-    ? transform(value)
-    : undefined
+function _makeRequest(options, projectId, metricsKey, callback) {
+  const timer = new metrics.Timer(metricsKey)
+  request(
+    {
+      url: `${settings.apis.documentupdater.url}${options.path}`,
+      json: options.json,
+      method: options.method || 'GET'
+    },
+    function(error, res, body) {
+      timer.done()
+      if (error) {
+        logger.warn(
+          { error, projectId },
+          'error making request to document updater'
+        )
+        callback(error)
+      } else if (res.statusCode >= 200 && res.statusCode < 300) {
+        callback(null, body)
+      } else {
+        error = new Error(
+          `document updater returned a failure status code: ${res.statusCode}`
+        )
+        logger.warn(
+          { error, projectId },
+          `document updater returned failure status code: ${res.statusCode}`
+        )
+        callback(error)
+      }
+    }
+  )
+}
+
+function _getUpdates(entityType, oldEntities, newEntities) {
+  if (!oldEntities) {
+    oldEntities = []
+  }
+  if (!newEntities) {
+    newEntities = []
+  }
+  const deletes = []
+  const adds = []
+  const renames = []
+
+  const oldEntitiesHash = _.indexBy(oldEntities, entity =>
+    entity[entityType]._id.toString()
+  )
+  const newEntitiesHash = _.indexBy(newEntities, entity =>
+    entity[entityType]._id.toString()
+  )
+
+  // Send deletes before adds (and renames) to keep a 1:1 mapping between
+  // paths and ids
+  //
+  // When a file is replaced, we first delete the old file and then add the
+  // new file. If the 'add' operation is sent to project history before the
+  // 'delete' then we would have two files with the same path at that point
+  // in time.
+  for (const id in oldEntitiesHash) {
+    const oldEntity = oldEntitiesHash[id]
+    const newEntity = newEntitiesHash[id]
+
+    if (newEntity == null) {
+      // entity deleted
+      deletes.push({
+        type: `rename-${entityType}`,
+        id,
+        pathname: oldEntity.path,
+        newPathname: ''
+      })
+    }
+  }
+
+  for (const id in newEntitiesHash) {
+    const newEntity = newEntitiesHash[id]
+    const oldEntity = oldEntitiesHash[id]
+
+    if (oldEntity == null) {
+      // entity added
+      adds.push({
+        type: `add-${entityType}`,
+        id,
+        pathname: newEntity.path,
+        docLines: newEntity.docLines,
+        url: newEntity.url,
+        hash: newEntity.file != null ? newEntity.file.hash : undefined
+      })
+    } else if (newEntity.path !== oldEntity.path) {
+      // entity renamed
+      renames.push({
+        type: `rename-${entityType}`,
+        id,
+        pathname: oldEntity.path,
+        newPathname: newEntity.path
+      })
+    }
+  }
+
+  return { deletes, adds, renames }
 }

services/web/app/src/Features/Project/ProjectEntityUpdateHandler.js

@@ -607,7 +607,7 @@ const ProjectEntityUpdateHandler = {
     }
     ProjectLocator.findElement(
       { project_id: projectId, element_id: folderId, type: 'folder' },
-      (error, folder, path) => {
+      (error, folder, folderPath) => {
         if (error != null) {
           return callback(error)
         }
@@ -620,6 +620,7 @@ const ProjectEntityUpdateHandler = {
         )
         if (existingFile) {
           const doc = new Doc({ name: docName })
+          const filePath = `${folderPath.fileSystem}/${existingFile.name}`
           DocstoreManager.updateDoc(
             projectId.toString(),
             doc._id.toString(),
@@ -642,7 +643,7 @@ const ProjectEntityUpdateHandler = {
                     {
                       project_id: projectId,
                       doc_id: doc._id,
-                      path: path.fileSystem,
+                      path: filePath,
                       project_name: project.name,
                       rev: existingFile.rev + 1
                     },
@@ -650,7 +651,6 @@ const ProjectEntityUpdateHandler = {
                       if (err) {
                         return callback(err)
                       }
-                      const docPath = path.fileSystem
                       const projectHistoryId =
                         project.overleaf &&
                         project.overleaf.history &&
@@ -658,14 +658,14 @@ const ProjectEntityUpdateHandler = {
                       const newDocs = [
                         {
                           doc,
-                          path: docPath,
+                          path: filePath,
                           docLines: docLines.join('\n')
                         }
                       ]
                       const oldFiles = [
                         {
                           file: existingFile,
-                          path: Path.join(path.fileSystem, existingFile.name)
+                          path: filePath
                         }
                       ]
                       DocumentUpdaterHandler.updateProjectStructure(