Merge pull request #14103 from overleaf/jpa-web-create-dump-once

[web] create the dump folder once at startup

GitOrigin-RevId: 0026ebe15a92f0d17f97966c89cb471b1282d061

services/web/app.js

@@ -44,6 +44,7 @@ const mongoose = require('./app/src/infrastructure/Mongoose')
 const {
   triggerGracefulShutdown,
 } = require('./app/src/infrastructure/GracefulShutdown')
+const FileWriter = require('./app/src/infrastructure/FileWriter')
 
 if (Settings.catchErrors) {
   process.removeAllListeners('uncaughtException')
@@ -51,6 +52,10 @@ if (Settings.catchErrors) {
     logger.error({ err: error }, 'uncaughtException')
   )
 }
+
+// Create ./data/dumpFolder if needed
+FileWriter.ensureDumpFolderExists()
+
 const port = Settings.port || Settings.internal.web.port || 3000
 const host = Settings.internal.web.host || 'localhost'
 if (!module.parent) {

services/web/app/src/Features/LinkedFiles/UrlAgent.js

@@ -84,7 +84,6 @@ function _getUrlStream(projectId, data, currentUserId, callback) {
   }
   url = UrlHelper.wrapUrlWithProxy(url)
   const readStream = request.get(url)
-  readStream.pause()
   callback(null, readStream)
 }
 

services/web/app/src/Features/Templates/TemplatesManager.js

@@ -4,7 +4,6 @@ const ProjectDetailsHandler = require('../Project/ProjectDetailsHandler')
 const ProjectOptionsHandler = require('../Project/ProjectOptionsHandler')
 const ProjectRootDocManager = require('../Project/ProjectRootDocManager')
 const ProjectUploadManager = require('../Uploads/ProjectUploadManager')
-const FileWriter = require('../../infrastructure/FileWriter')
 const async = require('async')
 const fs = require('fs')
 const util = require('util')
@@ -41,81 +40,76 @@ const TemplatesManager = {
       logger.warn({ err }, 'error getting zip from template API')
       return callback(err)
     })
-    FileWriter.ensureDumpFolderExists(function (err) {
-      if (err) {
-        return callback(err)
-      }
 
-      const projectName = ProjectDetailsHandler.fixProjectName(templateName)
-      const dumpPath = `${settings.path.dumpFolder}/${crypto.randomUUID()}`
-      const writeStream = fs.createWriteStream(dumpPath)
-      const attributes = {
-        fromV1TemplateId: templateId,
-        fromV1TemplateVersionId: templateVersionId,
-      }
-      writeStream.on('close', function () {
-        if (zipReq.response.statusCode !== 200) {
-          logger.warn(
-            { uri: zipUrl, statusCode: zipReq.response.statusCode },
-            'non-success code getting zip from template API'
-          )
-          return callback(new Error('get zip failed'))
-        }
-        ProjectUploadManager.createProjectFromZipArchiveWithName(
-          userId,
-          projectName,
-          dumpPath,
-          attributes,
-          function (err, project) {
-            if (err) {
-              OError.tag(err, 'problem building project from zip', {
-                zipReq,
-              })
-              return callback(err)
-            }
-            async.series(
-              [
-                cb => TemplatesManager._setCompiler(project._id, compiler, cb),
-                cb => TemplatesManager._setImage(project._id, imageName, cb),
-                cb => TemplatesManager._setMainFile(project._id, mainFile, cb),
-                cb =>
-                  TemplatesManager._setBrandVariationId(
-                    project._id,
-                    brandVariationId,
-                    cb
-                  ),
-              ],
-              function (err) {
-                if (err) {
-                  return callback(err)
-                }
-                fs.unlink(dumpPath, function (err) {
-                  if (err) {
-                    return logger.err({ err }, 'error unlinking template zip')
-                  }
-                })
-                const update = {
-                  fromV1TemplateId: templateId,
-                  fromV1TemplateVersionId: templateVersionId,
-                }
-                Project.updateOne(
-                  { _id: project._id },
-                  update,
-                  {},
-                  function (err) {
-                    if (err) {
-                      return callback(err)
-                    }
-                    callback(null, project)
-                  }
-                )
-              }
-            )
-          }
+    const projectName = ProjectDetailsHandler.fixProjectName(templateName)
+    const dumpPath = `${settings.path.dumpFolder}/${crypto.randomUUID()}`
+    const writeStream = fs.createWriteStream(dumpPath)
+    const attributes = {
+      fromV1TemplateId: templateId,
+      fromV1TemplateVersionId: templateVersionId,
+    }
+    writeStream.on('close', function () {
+      if (zipReq.response.statusCode !== 200) {
+        logger.warn(
+          { uri: zipUrl, statusCode: zipReq.response.statusCode },
+          'non-success code getting zip from template API'
         )
-      })
-      zipReq.pipe(writeStream)
+        return callback(new Error('get zip failed'))
+      }
+      ProjectUploadManager.createProjectFromZipArchiveWithName(
+        userId,
+        projectName,
+        dumpPath,
+        attributes,
+        function (err, project) {
+          if (err) {
+            OError.tag(err, 'problem building project from zip', {
+              zipReq,
+            })
+            return callback(err)
+          }
+          async.series(
+            [
+              cb => TemplatesManager._setCompiler(project._id, compiler, cb),
+              cb => TemplatesManager._setImage(project._id, imageName, cb),
+              cb => TemplatesManager._setMainFile(project._id, mainFile, cb),
+              cb =>
+                TemplatesManager._setBrandVariationId(
+                  project._id,
+                  brandVariationId,
+                  cb
+                ),
+            ],
+            function (err) {
+              if (err) {
+                return callback(err)
+              }
+              fs.unlink(dumpPath, function (err) {
+                if (err) {
+                  return logger.err({ err }, 'error unlinking template zip')
+                }
+              })
+              const update = {
+                fromV1TemplateId: templateId,
+                fromV1TemplateVersionId: templateVersionId,
+              }
+              Project.updateOne(
+                { _id: project._id },
+                update,
+                {},
+                function (err) {
+                  if (err) {
+                    return callback(err)
+                  }
+                  callback(null, project)
+                }
+              )
+            }
+          )
+        }
+      )
     })
+    zipReq.pipe(writeStream)
   },
 
   _setCompiler(projectId, compiler, callback) {

services/web/app/src/infrastructure/FileWriter.js

@@ -57,17 +57,8 @@ class SizeLimitedStream extends Transform {
 }
 
 const FileWriter = {
-  ensureDumpFolderExists(callback) {
-    if (callback == null) {
-      callback = function () {}
-    }
-    return fs.mkdir(Settings.path.dumpFolder, function (error) {
-      if (error != null && error.code !== 'EEXIST') {
-        // Ignore error about already existing
-        return callback(error)
-      }
-      return callback(null)
-    })
+  ensureDumpFolderExists() {
+    fs.mkdirSync(Settings.path.dumpFolder, { recursive: true })
   },
 
   writeLinesToDisk(identifier, lines, callback) {
@@ -81,20 +72,14 @@ const FileWriter = {
     if (callback == null) {
       callback = function () {}
     }
-    callback = _.once(callback)
     const fsPath = `${
       Settings.path.dumpFolder
     }/${identifier}_${crypto.randomUUID()}`
-    return FileWriter.ensureDumpFolderExists(function (error) {
+    return fs.writeFile(fsPath, content, function (error) {
       if (error != null) {
         return callback(error)
       }
-      return fs.writeFile(fsPath, content, function (error) {
-        if (error != null) {
-          return callback(error)
-        }
-        return callback(null, fsPath)
-      })
+      return callback(null, fsPath)
     })
   },
 
@@ -112,57 +97,47 @@ const FileWriter = {
       Settings.path.dumpFolder
     }/${identifier}_${crypto.randomUUID()}`
 
-    stream.pause()
+    const writeStream = fs.createWriteStream(fsPath)
+    const passThrough = new SizeLimitedStream({
+      maxSizeBytes: options.maxSizeBytes,
+    })
 
-    FileWriter.ensureDumpFolderExists(function (error) {
-      const writeStream = fs.createWriteStream(fsPath)
-
-      if (error != null) {
-        return callback(error)
-      }
-      stream.resume()
-
-      const passThrough = new SizeLimitedStream({
-        maxSizeBytes: options.maxSizeBytes,
+    // if writing fails, we want to consume the bytes from the source, to avoid leaks
+    for (const evt of ['error', 'close']) {
+      writeStream.on(evt, function () {
+        passThrough.unpipe(writeStream)
+        passThrough.resume()
       })
+    }
 
-      // if writing fails, we want to consume the bytes from the source, to avoid leaks
-      for (const evt of ['error', 'close']) {
-        writeStream.on(evt, function () {
-          passThrough.unpipe(writeStream)
-          passThrough.resume()
-        })
+    pipeline(stream, passThrough, writeStream, function (err) {
+      if (
+        options.maxSizeBytes &&
+        passThrough.bytes >= options.maxSizeBytes &&
+        !(err instanceof FileTooLargeError)
+      ) {
+        err = new FileTooLargeError({
+          message: 'stream size limit reached',
+          info: { size: passThrough.bytes },
+        }).withCause(err || {})
       }
-
-      pipeline(stream, passThrough, writeStream, function (err) {
-        if (
-          options.maxSizeBytes &&
-          passThrough.bytes >= options.maxSizeBytes &&
-          !(err instanceof FileTooLargeError)
-        ) {
-          err = new FileTooLargeError({
-            message: 'stream size limit reached',
-            info: { size: passThrough.bytes },
-          }).withCause(err || {})
-        }
-        if (err) {
-          OError.tag(
-            err,
-            '[writeStreamToDisk] something went wrong writing the stream to disk',
-            {
-              identifier,
-              fsPath,
-            }
-          )
-          return callback(err)
-        }
-
-        logger.debug(
-          { identifier, fsPath },
-          '[writeStreamToDisk] write stream finished'
+      if (err) {
+        OError.tag(
+          err,
+          '[writeStreamToDisk] something went wrong writing the stream to disk',
+          {
+            identifier,
+            fsPath,
+          }
         )
-        callback(null, fsPath)
-      })
+        return callback(err)
+      }
+
+      logger.debug(
+        { identifier, fsPath },
+        '[writeStreamToDisk] write stream finished'
+      )
+      callback(null, fsPath)
     })
   },
 
@@ -198,5 +173,7 @@ const FileWriter = {
 }
 
 module.exports = FileWriter
-module.exports.promises = promisifyAll(FileWriter)
+module.exports.promises = promisifyAll(FileWriter, {
+  without: ['ensureDumpFolderExists'],
+})
 module.exports.SizeLimitedStream = SizeLimitedStream

services/web/test/unit/src/Templates/TemplatesManagerTests.js

@@ -61,7 +61,6 @@ describe('TemplatesManager', function () {
       fixProjectName: sinon.stub().returns(this.templateName),
     }
     this.Project = { updateOne: sinon.stub().callsArgWith(3, null) }
-    this.FileWriter = { ensureDumpFolderExists: sinon.stub().callsArg(0) }
     this.FetchUtils = {
       fetchJson: sinon.stub(),
       RequestFailedError,
@@ -76,7 +75,6 @@ describe('TemplatesManager', function () {
         '../Authentication/SessionManager': (this.SessionManager = {
           getLoggedInUserId: sinon.stub(),
         }),
-        '../../infrastructure/FileWriter': this.FileWriter,
         '@overleaf/settings': {
           path: {
             dumpFolder: this.dumpFolder,
@@ -177,10 +175,6 @@ describe('TemplatesManager', function () {
           }
         )
       })
-
-      it('should ensure that the dump folder exists', function () {
-        return sinon.assert.called(this.FileWriter.ensureDumpFolderExists)
-      })
     })
 
     describe('when some options not set', function () {