Merge pull request #102 from overleaf/spd-integration-node12

Merge node 12 integration branch to master

services/filestore/.eslintrc

@@ -1,7 +1,7 @@
 // this file was auto-generated, do not edit it directly.
 // instead run bin/update_build_scripts from
 // https://github.com/sharelatex/sharelatex-dev-environment
-// Version: 1.3.5
+// Version: 1.3.6
 {
   "extends": [
     "standard",
@@ -23,8 +23,7 @@
   "rules": {
     // Swap the no-unused-expressions rule with a more chai-friendly one
     "no-unused-expressions": 0,
-    "chai-friendly/no-unused-expressions": "error",
-    "no-console": "error"
+    "chai-friendly/no-unused-expressions": "error"
   },
   "overrides": [
     {

services/filestore/.nvmrc

@@ -1 +1 @@
-10.19.0
+12.16.1

services/filestore/.prettierrc

@@ -1,7 +1,7 @@
 # This file was auto-generated, do not edit it directly.
 # Instead run bin/update_build_scripts from
 # https://github.com/sharelatex/sharelatex-dev-environment
-# Version: 1.3.5
+# Version: 1.3.6
 {
   "semi": false,
   "singleQuote": true

services/filestore/Dockerfile

@@ -1,9 +1,9 @@
 # This file was auto-generated, do not edit it directly.
 # Instead run bin/update_build_scripts from
 # https://github.com/sharelatex/sharelatex-dev-environment
-# Version: 1.3.5
+# Version: 1.3.6
 
-FROM node:10.19.0 as base
+FROM node:12.16.1 as base
 
 WORKDIR /app
 COPY install_deps.sh /app

services/filestore/Makefile

@@ -1,7 +1,7 @@
 # This file was auto-generated, do not edit it directly.
 # Instead run bin/update_build_scripts from
 # https://github.com/sharelatex/sharelatex-dev-environment
-# Version: 1.3.5
+# Version: 1.3.6
 
 BUILD_NUMBER ?= local
 BRANCH_NAME ?= $(shell git rev-parse --abbrev-ref HEAD)

services/filestore/README.md

@@ -10,7 +10,6 @@ filestore acts as a proxy between the CLSIs and (currently) Amazon S3 storage, p
 * `/project/:project_id/public/:public_file_id`
 * `/project/:project_id/size`
 * `/bucket/:bucket/key/*`
-* `/heapdump`
 * `/shutdown`
 * `/status` - returns `filestore sharelatex up` or `server is being shut down` (HTTP 500)
 * `/health_check` 

services/filestore/app.js

@@ -122,18 +122,6 @@ app.get(
   fileController.getFile
 )
 
-app.get('/heapdump', (req, res, next) =>
-  require('heapdump').writeSnapshot(
-    '/tmp/' + Date.now() + '.filestore.heapsnapshot',
-    (err, filename) => {
-      if (err) {
-        return next(err)
-      }
-      res.send(filename)
-    }
-  )
-)
-
 app.get('/status', function(req, res) {
   res.send('filestore sharelatex up')
 })

services/filestore/app/js/FileHandler.js

@@ -1,176 +1,153 @@
-const { promisify } = require('util')
+const { callbackify } = require('util')
 const fs = require('fs')
 const PersistorManager = require('./PersistorManager')
 const LocalFileWriter = require('./LocalFileWriter')
 const FileConverter = require('./FileConverter')
 const KeyBuilder = require('./KeyBuilder')
-const async = require('async')
 const ImageOptimiser = require('./ImageOptimiser')
 const { ConversionError } = require('./Errors')
 
 module.exports = {
-  insertFile,
-  deleteFile,
-  getFile,
-  getFileSize,
-  getDirectorySize,
+  insertFile: callbackify(insertFile),
+  deleteFile: callbackify(deleteFile),
+  getFile: callbackify(getFile),
+  getFileSize: callbackify(getFileSize),
+  getDirectorySize: callbackify(getDirectorySize),
   promises: {
-    getFile: promisify(getFile),
-    insertFile: promisify(insertFile),
-    deleteFile: promisify(deleteFile),
-    getFileSize: promisify(getFileSize),
-    getDirectorySize: promisify(getDirectorySize)
+    getFile,
+    insertFile,
+    deleteFile,
+    getFileSize,
+    getDirectorySize
   }
 }
 
-function insertFile(bucket, key, stream, callback) {
+async function insertFile(bucket, key, stream) {
   const convertedKey = KeyBuilder.getConvertedFolderKey(key)
-  PersistorManager.deleteDirectory(bucket, convertedKey, function(error) {
-    if (error) {
-      return callback(error)
-    }
-    PersistorManager.sendStream(bucket, key, stream, callback)
-  })
+  await PersistorManager.promises.deleteDirectory(bucket, convertedKey)
+  await PersistorManager.promises.sendStream(bucket, key, stream)
 }
 
-function deleteFile(bucket, key, callback) {
+async function deleteFile(bucket, key) {
   const convertedKey = KeyBuilder.getConvertedFolderKey(key)
-  async.parallel(
-    [
-      done => PersistorManager.deleteFile(bucket, key, done),
-      done => PersistorManager.deleteDirectory(bucket, convertedKey, done)
-    ],
-    callback
-  )
+  await Promise.all([
+    PersistorManager.promises.deleteFile(bucket, key),
+    PersistorManager.promises.deleteDirectory(bucket, convertedKey)
+  ])
 }
 
-function getFile(bucket, key, opts, callback) {
+async function getFile(bucket, key, opts) {
   opts = opts || {}
   if (!opts.format && !opts.style) {
-    PersistorManager.getFileStream(bucket, key, opts, callback)
+    return PersistorManager.promises.getFileStream(bucket, key, opts)
   } else {
-    _getConvertedFile(bucket, key, opts, callback)
+    return _getConvertedFile(bucket, key, opts)
   }
 }
 
-function getFileSize(bucket, key, callback) {
-  PersistorManager.getFileSize(bucket, key, callback)
+async function getFileSize(bucket, key) {
+  return PersistorManager.promises.getFileSize(bucket, key)
 }
 
-function getDirectorySize(bucket, projectId, callback) {
-  PersistorManager.directorySize(bucket, projectId, callback)
+async function getDirectorySize(bucket, projectId) {
+  return PersistorManager.promises.directorySize(bucket, projectId)
 }
 
-function _getConvertedFile(bucket, key, opts, callback) {
+async function _getConvertedFile(bucket, key, opts) {
   const convertedKey = KeyBuilder.addCachingToKey(key, opts)
-  PersistorManager.checkIfFileExists(bucket, convertedKey, (err, exists) => {
-    if (err) {
-      return callback(err)
-    }
-
-    if (exists) {
-      PersistorManager.getFileStream(bucket, convertedKey, opts, callback)
-    } else {
-      _getConvertedFileAndCache(bucket, key, convertedKey, opts, callback)
-    }
-  })
+  const exists = await PersistorManager.promises.checkIfFileExists(
+    bucket,
+    convertedKey
+  )
+  if (exists) {
+    return PersistorManager.promises.getFileStream(bucket, convertedKey, opts)
+  } else {
+    return _getConvertedFileAndCache(bucket, key, convertedKey, opts)
+  }
 }
 
-function _getConvertedFileAndCache(bucket, key, convertedKey, opts, callback) {
+async function _getConvertedFileAndCache(bucket, key, convertedKey, opts) {
   let convertedFsPath
-
-  async.series(
-    [
-      cb => {
-        _convertFile(bucket, key, opts, function(err, fileSystemPath) {
-          convertedFsPath = fileSystemPath
-          cb(err)
-        })
-      },
-      cb => ImageOptimiser.compressPng(convertedFsPath, cb),
-      cb => PersistorManager.sendFile(bucket, convertedKey, convertedFsPath, cb)
-    ],
-    function(err) {
-      if (err) {
-        LocalFileWriter.deleteFile(convertedFsPath, function() {})
-        return callback(
-          new ConversionError({
-            message: 'failed to convert file',
-            info: { opts, bucket, key, convertedKey }
-          }).withCause(err)
-        )
-      }
-      // Send back the converted file from the local copy to avoid problems
-      // with the file not being present in S3 yet.  As described in the
-      // documentation below, we have already made a 'HEAD' request in
-      // checkIfFileExists so we only have "eventual consistency" if we try
-      // to stream it from S3 here.  This was a cause of many 403 errors.
-      //
-      // "Amazon S3 provides read-after-write consistency for PUTS of new
-      // objects in your S3 bucket in all regions with one caveat. The
-      // caveat is that if you make a HEAD or GET request to the key name
-      // (to find if the object exists) before creating the object, Amazon
-      // S3 provides eventual consistency for read-after-write.""
-      // https://docs.aws.amazon.com/AmazonS3/latest/dev/Introduction.html#ConsistencyModel
-      const readStream = fs.createReadStream(convertedFsPath)
-      readStream.on('end', function() {
-        LocalFileWriter.deleteFile(convertedFsPath, function() {})
-      })
-      callback(null, readStream)
-    }
-  )
+  try {
+    convertedFsPath = await _convertFile(bucket, key, opts)
+    await ImageOptimiser.promises.compressPng(convertedFsPath)
+    await PersistorManager.promises.sendFile(
+      bucket,
+      convertedKey,
+      convertedFsPath
+    )
+  } catch (err) {
+    LocalFileWriter.deleteFile(convertedFsPath, () => {})
+    throw new ConversionError({
+      message: 'failed to convert file',
+      info: { opts, bucket, key, convertedKey }
+    }).withCause(err)
+  }
+  // Send back the converted file from the local copy to avoid problems
+  // with the file not being present in S3 yet.  As described in the
+  // documentation below, we have already made a 'HEAD' request in
+  // checkIfFileExists so we only have "eventual consistency" if we try
+  // to stream it from S3 here.  This was a cause of many 403 errors.
+  //
+  // "Amazon S3 provides read-after-write consistency for PUTS of new
+  // objects in your S3 bucket in all regions with one caveat. The
+  // caveat is that if you make a HEAD or GET request to the key name
+  // (to find if the object exists) before creating the object, Amazon
+  // S3 provides eventual consistency for read-after-write.""
+  // https://docs.aws.amazon.com/AmazonS3/latest/dev/Introduction.html#ConsistencyModel
+  const readStream = fs.createReadStream(convertedFsPath)
+  readStream.on('end', function() {
+    LocalFileWriter.deleteFile(convertedFsPath, function() {})
+  })
+  return readStream
 }
 
-function _convertFile(bucket, originalKey, opts, callback) {
-  _writeFileToDisk(bucket, originalKey, opts, function(err, originalFsPath) {
-    if (err) {
-      return callback(
-        new ConversionError({
-          message: 'unable to write file to disk',
-          info: { bucket, originalKey, opts }
-        }).withCause(err)
-      )
-    }
+async function _convertFile(bucket, originalKey, opts) {
+  let originalFsPath
+  try {
+    originalFsPath = await _writeFileToDisk(bucket, originalKey, opts)
+  } catch (err) {
+    throw new ConversionError({
+      message: 'unable to write file to disk',
+      info: { bucket, originalKey, opts }
+    }).withCause(err)
+  }
 
-    const done = function(err, destPath) {
-      if (err) {
-        return callback(
-          new ConversionError({
-            message: 'error converting file',
-            info: { bucket, originalKey, opts }
-          }).withCause(err)
-        )
+  let promise
+  if (opts.format) {
+    promise = FileConverter.promises.convert(originalFsPath, opts.format)
+  } else if (opts.style === 'thumbnail') {
+    promise = FileConverter.promises.thumbnail(originalFsPath)
+  } else if (opts.style === 'preview') {
+    promise = FileConverter.promises.preview(originalFsPath)
+  } else {
+    throw new ConversionError({
+      message: 'invalid file conversion options',
+      info: {
+        bucket,
+        originalKey,
+        opts
       }
-      LocalFileWriter.deleteFile(originalFsPath, function() {})
-      callback(err, destPath)
-    }
-
-    if (opts.format) {
-      FileConverter.convert(originalFsPath, opts.format, done)
-    } else if (opts.style === 'thumbnail') {
-      FileConverter.thumbnail(originalFsPath, done)
-    } else if (opts.style === 'preview') {
-      FileConverter.preview(originalFsPath, done)
-    } else {
-      callback(
-        new ConversionError({
-          message: 'invalid file conversion options',
-          info: {
-            bucket,
-            originalKey,
-            opts
-          }
-        })
-      )
-    }
-  })
+    })
+  }
+  let destPath
+  try {
+    destPath = await promise
+  } catch (err) {
+    throw new ConversionError({
+      message: 'error converting file',
+      info: { bucket, originalKey, opts }
+    }).withCause(err)
+  }
+  LocalFileWriter.deleteFile(originalFsPath, function() {})
+  return destPath
 }
 
-function _writeFileToDisk(bucket, key, opts, callback) {
-  PersistorManager.getFileStream(bucket, key, opts, function(err, fileStream) {
-    if (err) {
-      return callback(err)
-    }
-    LocalFileWriter.writeStream(fileStream, key, callback)
-  })
+async function _writeFileToDisk(bucket, key, opts) {
+  const fileStream = await PersistorManager.promises.getFileStream(
+    bucket,
+    key,
+    opts
+  )
+  return LocalFileWriter.promises.writeStream(fileStream, key)
 }

services/filestore/app/js/HealthCheckController.js

@@ -1,4 +1,4 @@
-const fs = require('fs-extra')
+const fs = require('fs')
 const path = require('path')
 const Settings = require('settings-sharelatex')
 const streamBuffers = require('stream-buffers')
@@ -6,7 +6,7 @@ const { promisify } = require('util')
 const Stream = require('stream')
 
 const pipeline = promisify(Stream.pipeline)
-const fsCopy = promisify(fs.copy)
+const fsCopy = promisify(fs.copyFile)
 const fsUnlink = promisify(fs.unlink)
 
 const { HealthCheckError } = require('./Errors')

services/filestore/app/js/SafeExec.js

@@ -1,4 +1,4 @@
-const _ = require('underscore')
+const lodashOnce = require('lodash.once')
 const childProcess = require('child_process')
 const Settings = require('settings-sharelatex')
 const { ConversionsDisabledError, FailedCommandError } = require('./Errors')
@@ -28,7 +28,7 @@ function safeExec(command, options, callback) {
 
   let killTimer
 
-  const cleanup = _.once(function(err) {
+  const cleanup = lodashOnce(function(err) {
     if (killTimer) {
       clearTimeout(killTimer)
     }

services/filestore/buildscript.txt

@@ -1,11 +1,11 @@
 filestore
---public-repo=True
---language=es
---env-add=ENABLE_CONVERSIONS="true",USE_PROM_METRICS="true",AWS_S3_USER_FILES_BUCKET_NAME=fake_user_files,AWS_S3_TEMPLATE_FILES_BUCKET_NAME=fake_template_files,AWS_S3_PUBLIC_FILES_BUCKET_NAME=fake_public_files
---node-version=10.19.0
 --acceptance-creds=
+--data-dirs=uploads,user_files,template_files
 --dependencies=s3
 --docker-repos=gcr.io/overleaf-ops
+--env-add=ENABLE_CONVERSIONS="true",USE_PROM_METRICS="true",AWS_S3_USER_FILES_BUCKET_NAME=fake_user_files,AWS_S3_TEMPLATE_FILES_BUCKET_NAME=fake_template_files,AWS_S3_PUBLIC_FILES_BUCKET_NAME=fake_public_files
 --env-pass-through=
---data-dirs=uploads,user_files,template_files
---script-version=1.3.5
+--language=es
+--node-version=12.16.1
+--public-repo=True
+--script-version=1.3.6

services/filestore/config/settings.defaults.coffee

@@ -38,6 +38,7 @@ settings =
 				key: process.env['AWS_ACCESS_KEY_ID']
 				secret: process.env['AWS_SECRET_ACCESS_KEY']
 				endpoint: process.env['AWS_S3_ENDPOINT']
+				pathStyle: process.env['AWS_S3_PATH_STYLE']
 				partSize: process.env['AWS_S3_PARTSIZE'] or (100 * 1024 * 1024)
 
 		stores:

services/filestore/docker-compose.ci.yml

@@ -1,7 +1,7 @@
 # This file was auto-generated, do not edit it directly.
 # Instead run bin/update_build_scripts from
 # https://github.com/sharelatex/sharelatex-dev-environment
-# Version: 1.3.5
+# Version: 1.3.6
 
 version: "2.3"
 

services/filestore/docker-compose.yml

@@ -1,7 +1,7 @@
 # This file was auto-generated, do not edit it directly.
 # Instead run bin/update_build_scripts from
 # https://github.com/sharelatex/sharelatex-dev-environment
-# Version: 1.3.5
+# Version: 1.3.6
 
 version: "2.3"
 

services/filestore/package-lock.json

@@ -1256,11 +1256,6 @@
       "integrity": "sha512-+Ryf6g3BKoRc7jfp7ad8tM4TtMiaWvbF/1/sQcZPkkS7ag3D5nMBCe2UfOTONtAkaG0tO0ij3C5Lwmf1EiyjHg==",
       "dev": true
     },
-    "async": {
-      "version": "0.2.10",
-      "resolved": "https://registry.npmjs.org/async/-/async-0.2.10.tgz",
-      "integrity": "sha512-eAkdoKxU6/LkKDBzLpT+t6Ff5EtfSF4wx1WfJiPEEV7WNLnDaRXk0oVysiEPm262roaachGexwUv94WhSgN5TQ=="
-    },
     "async-listener": {
       "version": "0.6.10",
       "resolved": "https://registry.npmjs.org/async-listener/-/async-listener-0.6.10.tgz",
@@ -1409,7 +1404,8 @@
     "browser-stdout": {
       "version": "1.3.1",
       "resolved": "https://registry.npmjs.org/browser-stdout/-/browser-stdout-1.3.1.tgz",
-      "integrity": "sha512-qhAVI1+Av2X7qelOfAIYwXONood6XlZE/fXaBSmW/T5SzLAmCgzi+eiWE7fUvbHaeNBQH13UftjpXxsfLkMpgw=="
+      "integrity": "sha512-qhAVI1+Av2X7qelOfAIYwXONood6XlZE/fXaBSmW/T5SzLAmCgzi+eiWE7fUvbHaeNBQH13UftjpXxsfLkMpgw==",
+      "dev": true
     },
     "buffer": {
       "version": "4.9.1",
@@ -1603,7 +1599,8 @@
     "commander": {
       "version": "2.15.1",
       "resolved": "https://registry.npmjs.org/commander/-/commander-2.15.1.tgz",
-      "integrity": "sha512-VlfT9F3V0v+jr4yxPc5gg9s62/fIVWsd2Bk2iD435um1NlGMYdVCq+MjcXnhYq2icNOizHr1kK+5TI6H0Hy0ag=="
+      "integrity": "sha512-VlfT9F3V0v+jr4yxPc5gg9s62/fIVWsd2Bk2iD435um1NlGMYdVCq+MjcXnhYq2icNOizHr1kK+5TI6H0Hy0ag==",
+      "dev": true
     },
     "common-tags": {
       "version": "1.8.0",
@@ -1765,7 +1762,8 @@
     "diff": {
       "version": "3.5.0",
       "resolved": "https://registry.npmjs.org/diff/-/diff-3.5.0.tgz",
-      "integrity": "sha512-A46qtFgd+g7pDZinpnwiRJtxbC1hpgf0uzP3iG89scHk0AUC7A1TGxf5OiiOUv/JMZR8GOt8hL900hV0bOy5xA=="
+      "integrity": "sha512-A46qtFgd+g7pDZinpnwiRJtxbC1hpgf0uzP3iG89scHk0AUC7A1TGxf5OiiOUv/JMZR8GOt8hL900hV0bOy5xA==",
+      "dev": true
     },
     "disrequire": {
       "version": "1.1.0",
@@ -1930,7 +1928,8 @@
     "escape-string-regexp": {
       "version": "1.0.5",
       "resolved": "https://registry.npmjs.org/escape-string-regexp/-/escape-string-regexp-1.0.5.tgz",
-      "integrity": "sha512-vbRorB5FUQWvla16U8R/qgaFIya2qGzwDrNmCZuYKrbdSUMG6I1ZCGQRefkRVhuOkIGVne7BQ35DSfo1qvJqFg=="
+      "integrity": "sha512-vbRorB5FUQWvla16U8R/qgaFIya2qGzwDrNmCZuYKrbdSUMG6I1ZCGQRefkRVhuOkIGVne7BQ35DSfo1qvJqFg==",
+      "dev": true
     },
     "eslint": {
       "version": "6.8.0",
@@ -2488,16 +2487,6 @@
       "resolved": "https://registry.npmjs.org/fresh/-/fresh-0.5.2.tgz",
       "integrity": "sha512-zJ2mQYM18rEFOudeV4GShTGIQ7RbzA7ozbU9I/XBpm7kqgMywgmylMwXHxZJmkVoYkna9d2pVXVXPdYTP9ej8Q=="
     },
-    "fs-extra": {
-      "version": "1.0.0",
-      "resolved": "https://registry.npmjs.org/fs-extra/-/fs-extra-1.0.0.tgz",
-      "integrity": "sha512-VerQV6vEKuhDWD2HGOybV6v5I73syoc/cXAbKlgTC7M/oFVEtklWlp9QH2Ijw3IaWDOQcMkldSPa7zXy79Z/UQ==",
-      "requires": {
-        "graceful-fs": "^4.1.2",
-        "jsonfile": "^2.1.0",
-        "klaw": "^1.0.0"
-      }
-    },
     "fs.realpath": {
       "version": "1.0.0",
       "resolved": "https://registry.npmjs.org/fs.realpath/-/fs.realpath-1.0.0.tgz",
@@ -2562,11 +2551,6 @@
         "assert-plus": "^1.0.0"
       }
     },
-    "gettemporaryfilepath": {
-      "version": "0.0.1",
-      "resolved": "https://registry.npmjs.org/gettemporaryfilepath/-/gettemporaryfilepath-0.0.1.tgz",
-      "integrity": "sha512-7avwQWP8MP42u7mtc+KjCRuUE3nafRJPuGaZaySD9NN1KEbfVTfSAywP4KOkK8gaxhdOxx11ZTWH28DwjAF70Q=="
-    },
     "glob": {
       "version": "7.1.6",
       "resolved": "https://registry.npmjs.org/glob/-/glob-7.1.6.tgz",
@@ -2644,12 +2628,14 @@
     "graceful-fs": {
       "version": "4.2.3",
       "resolved": "https://registry.npmjs.org/graceful-fs/-/graceful-fs-4.2.3.tgz",
-      "integrity": "sha512-a30VEBm4PEdx1dRB7MFK7BejejvCvBronbLjht+sHuGYj8PHs7M/5Z+rt5lw551vZ7yfTCj4Vuyy3mSJytDWRQ=="
+      "integrity": "sha512-a30VEBm4PEdx1dRB7MFK7BejejvCvBronbLjht+sHuGYj8PHs7M/5Z+rt5lw551vZ7yfTCj4Vuyy3mSJytDWRQ==",
+      "dev": true
     },
     "growl": {
       "version": "1.10.5",
       "resolved": "https://registry.npmjs.org/growl/-/growl-1.10.5.tgz",
-      "integrity": "sha512-qBr4OuELkhPenW6goKVXiv47US3clb3/IbuWF9KNKEijAy9oeHxU9IgzjvJhHkUzhaj7rOUD7+YGWqUjLp5oSA=="
+      "integrity": "sha512-qBr4OuELkhPenW6goKVXiv47US3clb3/IbuWF9KNKEijAy9oeHxU9IgzjvJhHkUzhaj7rOUD7+YGWqUjLp5oSA==",
+      "dev": true
     },
     "gtoken": {
       "version": "4.1.4",
@@ -2712,7 +2698,8 @@
     "has-flag": {
       "version": "3.0.0",
       "resolved": "https://registry.npmjs.org/has-flag/-/has-flag-3.0.0.tgz",
-      "integrity": "sha512-sKJf1+ceQBr4SMkvQnBDNDtf4TXpVhVGateu0t918bl30FnbE2m4vNLX+VWe/dpjlb+HugGYzW7uQXH98HPEYw=="
+      "integrity": "sha512-sKJf1+ceQBr4SMkvQnBDNDtf4TXpVhVGateu0t918bl30FnbE2m4vNLX+VWe/dpjlb+HugGYzW7uQXH98HPEYw==",
+      "dev": true
     },
     "has-symbols": {
       "version": "1.0.1",
@@ -2723,15 +2710,8 @@
     "he": {
       "version": "1.1.1",
       "resolved": "https://registry.npmjs.org/he/-/he-1.1.1.tgz",
-      "integrity": "sha512-z/GDPjlRMNOa2XJiB4em8wJpuuBfrFOlYKTZxtpkdr1uPdibHI8rYA3MY0KDObpVyaes0e/aunid/t88ZI2EKA=="
-    },
-    "heapdump": {
-      "version": "0.3.15",
-      "resolved": "https://registry.npmjs.org/heapdump/-/heapdump-0.3.15.tgz",
-      "integrity": "sha512-n8aSFscI9r3gfhOcAECAtXFaQ1uy4QSke6bnaL+iymYZ/dWs9cqDqHM+rALfsHUwukUbxsdlECZ0pKmJdQ/4OA==",
-      "requires": {
-        "nan": "^2.13.2"
-      }
+      "integrity": "sha512-z/GDPjlRMNOa2XJiB4em8wJpuuBfrFOlYKTZxtpkdr1uPdibHI8rYA3MY0KDObpVyaes0e/aunid/t88ZI2EKA==",
+      "dev": true
     },
     "hex2dec": {
       "version": "1.1.2",
@@ -3072,14 +3052,6 @@
       "resolved": "https://registry.npmjs.org/json-stringify-safe/-/json-stringify-safe-5.0.1.tgz",
       "integrity": "sha512-ZClg6AaYvamvYEE82d3Iyd3vSSIjQ+odgjaTzRuO3s7toCdFKczob2i0zCh7JE8kWn17yvAWhUVxvqGwUalsRA=="
     },
-    "jsonfile": {
-      "version": "2.4.0",
-      "resolved": "https://registry.npmjs.org/jsonfile/-/jsonfile-2.4.0.tgz",
-      "integrity": "sha512-PKllAqbgLgxHaj8TElYymKCAgrASebJrWpTnEkOaTowt23VKXXN0sUeriJ+eh7y6ufb/CC5ap11pz71/cM0hUw==",
-      "requires": {
-        "graceful-fs": "^4.1.6"
-      }
-    },
     "jsprim": {
       "version": "1.4.1",
       "resolved": "https://registry.npmjs.org/jsprim/-/jsprim-1.4.1.tgz",
@@ -3116,14 +3088,6 @@
         "safe-buffer": "^5.0.1"
       }
     },
-    "klaw": {
-      "version": "1.3.1",
-      "resolved": "https://registry.npmjs.org/klaw/-/klaw-1.3.1.tgz",
-      "integrity": "sha512-TED5xi9gGQjGpNnvRWknrwAB1eL5GciPfVFOt3Vk1OJCVDQbzuSfrF3hkUQKlsgKrG1F+0t5W0m+Fje1jIt8rw==",
-      "requires": {
-        "graceful-fs": "^4.1.9"
-      }
-    },
     "levn": {
       "version": "0.3.0",
       "resolved": "https://registry.npmjs.org/levn/-/levn-0.3.0.tgz",
@@ -3202,6 +3166,11 @@
       "integrity": "sha512-0KpjqXRVvrYyCsX1swR/XTK0va6VQkQM6MNo7PqW77ByjAhoARA8EfrP1N4+KlKj8YS0ZUCtRT/YUuhyYDujIQ==",
       "dev": true
     },
+    "lodash.once": {
+      "version": "4.1.1",
+      "resolved": "https://registry.npmjs.org/lodash.once/-/lodash.once-4.1.1.tgz",
+      "integrity": "sha512-Sb487aTOCr9drQVL8pIxOzVhafOjZN9UU54hiN8PU3uAiSV7lx1yYNpbNmex2PK6dSJoNTSJUUswT651yww3Mg=="
+    },
     "lodash.pickby": {
       "version": "4.6.0",
       "resolved": "https://registry.npmjs.org/lodash.pickby/-/lodash.pickby-4.6.0.tgz",
@@ -3428,9 +3397,9 @@
       "integrity": "sha512-iclAHeNqNm68zFtnZ0e+1L2yUIdvzNoauKU4WBA3VvH/vPFieF7qfRlwUZU+DA9P9bPXIS90ulxoUoCH23sV2w=="
     },
     "metrics-sharelatex": {
-      "version": "2.4.0",
-      "resolved": "https://registry.npmjs.org/metrics-sharelatex/-/metrics-sharelatex-2.4.0.tgz",
-      "integrity": "sha512-FbIRRhReVCEM4ETzh+qVMm3lP33zSSAdrHfSTtegkcB7GGi1kYs+Qt1/dXFawUA8pIZRQTtsfxiS1nZamiSwHg==",
+      "version": "2.5.0",
+      "resolved": "https://registry.npmjs.org/metrics-sharelatex/-/metrics-sharelatex-2.5.0.tgz",
+      "integrity": "sha512-JG4yBe5bEzUW5P//8aAUoexInPosPLOXxLS4AjGxMrP78BS5PSV7uVrY0Op6b6c7ZqKItHTtEjzsUfLRPGQ/sQ==",
       "requires": {
         "@google-cloud/debug-agent": "^3.0.0",
         "@google-cloud/profiler": "^0.2.3",
@@ -3440,13 +3409,6 @@
         "prom-client": "^11.1.3",
         "underscore": "~1.6.0",
         "yn": "^3.1.1"
-      },
-      "dependencies": {
-        "underscore": {
-          "version": "1.6.0",
-          "resolved": "https://registry.npmjs.org/underscore/-/underscore-1.6.0.tgz",
-          "integrity": "sha512-z4o1fvKUojIWh9XuaVLUDdf86RQiq13AC1dmHbTpoyuu+bquHms76v16CjycCbec87J7z0k//SiQVk0sMdFmpQ=="
-        }
       }
     },
     "mime": {
@@ -3498,6 +3460,7 @@
       "version": "5.2.0",
       "resolved": "https://registry.npmjs.org/mocha/-/mocha-5.2.0.tgz",
       "integrity": "sha512-2IUgKDhc3J7Uug+FxMXuqIyYzH7gJjXECKe/w43IGgQHTSj3InJi+yAA7T24L9bQMRKiUEHxEX37G5JpVUGLcQ==",
+      "dev": true,
       "requires": {
         "browser-stdout": "1.3.1",
         "commander": "2.15.1",
@@ -3516,6 +3479,7 @@
           "version": "3.1.0",
           "resolved": "https://registry.npmjs.org/debug/-/debug-3.1.0.tgz",
           "integrity": "sha512-OX8XqP7/1a9cqkxYw2yXss15f26NKWBpDXQd0/uK/KPqdQhxbPa994hnzjcE2VqQpDslf55723cKPUOGSmMY3g==",
+          "dev": true,
           "requires": {
             "ms": "2.0.0"
           }
@@ -3524,6 +3488,7 @@
           "version": "7.1.2",
           "resolved": "https://registry.npmjs.org/glob/-/glob-7.1.2.tgz",
           "integrity": "sha512-MJTUg1kjuLeQCJ+ccE4Vpa6kKVXkPYJ2mOCQyUuKLcLQsdrMCpBPUi8qVE6+YuaJkozeA9NusTAw3hLr8Xe5EQ==",
+          "dev": true,
           "requires": {
             "fs.realpath": "^1.0.0",
             "inflight": "^1.0.4",
@@ -3927,14 +3892,6 @@
         "find-up": "^2.1.0"
       }
     },
-    "pngcrush": {
-      "version": "0.0.3",
-      "resolved": "https://registry.npmjs.org/pngcrush/-/pngcrush-0.0.3.tgz",
-      "integrity": "sha512-RVaPWGv0PUUzGeSQJHH78rw2ks8NxKbFn8uENFM+/3bfsUs39MaFDG+eul5902gH97zZLQ0zd0h2yb0YBaMKDw==",
-      "requires": {
-        "gettemporaryfilepath": "=0.0.1"
-      }
-    },
     "prelude-ls": {
       "version": "1.1.2",
       "resolved": "https://registry.npmjs.org/prelude-ls/-/prelude-ls-1.1.2.tgz",
@@ -5305,6 +5262,7 @@
       "version": "5.4.0",
       "resolved": "https://registry.npmjs.org/supports-color/-/supports-color-5.4.0.tgz",
       "integrity": "sha512-zjaXglF5nnWpsq470jSv6P9DwPvgLkuapYmfDm3JWOm0vkNTVF2tI4UrN2r6jH1qM/uc/WtxYY1hYoA2dOKj5w==",
+      "dev": true,
       "requires": {
         "has-flag": "^3.0.0"
       }
@@ -5504,9 +5462,9 @@
       "dev": true
     },
     "underscore": {
-      "version": "1.5.2",
-      "resolved": "https://registry.npmjs.org/underscore/-/underscore-1.5.2.tgz",
-      "integrity": "sha512-yejOFsRnTJs0N9CK5Apzf6maDO2djxGoLLrlZlvGs2o9ZQuhIhDL18rtFyy4FBIbOkzA6+4hDgXbgz5EvDQCXQ=="
+      "version": "1.6.0",
+      "resolved": "https://registry.npmjs.org/underscore/-/underscore-1.6.0.tgz",
+      "integrity": "sha512-z4o1fvKUojIWh9XuaVLUDdf86RQiq13AC1dmHbTpoyuu+bquHms76v16CjycCbec87J7z0k//SiQVk0sMdFmpQ=="
     },
     "unpipe": {
       "version": "1.0.0",

services/filestore/package.json

@@ -14,33 +14,28 @@
     "start": "node $NODE_APP_OPTIONS app.js",
     "nodemon": "nodemon --config nodemon.json",
     "lint": "node_modules/.bin/eslint .",
-    "format": "node_modules/.bin/prettier-eslint '**/*.js' --list-different",
-    "format:fix": "node_modules/.bin/prettier-eslint '**/*.js' --write",
+    "format": "node_modules/.bin/prettier-eslint $PWD'/**/*.js' --list-different",
+    "format:fix": "node_modules/.bin/prettier-eslint $PWD'/**/*.js' --write",
     "test:acceptance:_run": "mocha --recursive --reporter spec --timeout 15000 --exit $@ test/acceptance/js",
     "test:unit:_run": "mocha --recursive --reporter spec $@ test/unit/js"
   },
   "dependencies": {
     "@overleaf/o-error": "^2.1.0",
-    "async": "~0.2.10",
     "aws-sdk": "^2.628.0",
     "body-parser": "^1.2.0",
     "express": "^4.2.0",
-    "fs-extra": "^1.0.0",
     "glob": "^7.1.6",
-    "heapdump": "^0.3.2",
+    "lodash.once": "^4.1.1",
     "logger-sharelatex": "^1.7.0",
-    "metrics-sharelatex": "^2.2.0",
-    "mocha": "5.2.0",
+    "metrics-sharelatex": "^2.5.0",
     "node-uuid": "~1.4.1",
-    "pngcrush": "0.0.3",
     "range-parser": "^1.0.2",
     "request": "^2.88.0",
     "request-promise-native": "^1.0.8",
     "rimraf": "2.2.8",
     "settings-sharelatex": "^1.1.0",
     "stream-buffers": "~0.2.5",
-    "stream-meter": "^1.0.4",
-    "underscore": "~1.5.2"
+    "stream-meter": "^1.0.4"
   },
   "devDependencies": {
     "babel-eslint": "^10.0.3",
@@ -59,6 +54,7 @@
     "eslint-plugin-prettier": "^3.1.2",
     "eslint-plugin-promise": "^4.2.1",
     "eslint-plugin-standard": "^4.0.1",
+    "mocha": "5.2.0",
     "prettier-eslint": "^9.0.1",
     "prettier-eslint-cli": "^5.0.0",
     "sandboxed-module": "2.0.3",

services/filestore/test/acceptance/js/FilestoreApp.js

@@ -4,11 +4,7 @@ const fs = require('fs')
 const Path = require('path')
 const { promisify } = require('util')
 const disrequire = require('disrequire')
-const rp = require('request-promise-native').defaults({
-  resolveWithFullResponse: true
-})
-
-const S3_TRIES = 30
+const AWS = require('aws-sdk')
 
 logger.logger.level('info')
 
@@ -66,6 +62,7 @@ class FilestoreApp {
   }
 
   async stop() {
+    if (!this.server) return
     const closeServer = promisify(this.server.close).bind(this.server)
     try {
       await closeServer()
@@ -80,21 +77,31 @@ class FilestoreApp {
       return
     }
 
-    let s3Available = false
+    const s3 = new AWS.S3({
+      accessKeyId: Settings.filestore.s3.key,
+      secretAccessKey: Settings.filestore.s3.secret,
+      endpoint: Settings.filestore.s3.endpoint,
+      s3ForcePathStyle: true,
+      signatureVersion: 'v4'
+    })
 
-    while (tries < S3_TRIES && !s3Available) {
+    while (true) {
       try {
-        const response = await rp.get(`${Settings.filestore.s3.endpoint}/`)
-        if ([200, 404].includes(response.statusCode)) {
-          s3Available = true
-        }
+        return await s3
+          .putObject({
+            Key: 'startup',
+            Body: '42',
+            Bucket: Settings.filestore.stores.user_files
+          })
+          .promise()
       } catch (err) {
         // swallow errors, as we may experience them until fake-s3 is running
-      } finally {
-        tries++
-        if (!s3Available) {
-          await sleep(1000)
+        if (tries === 9) {
+          // throw just before hitting the 10s test timeout
+          throw err
         }
+        tries++
+        await sleep(1000)
       }
     }
   }

services/filestore/test/acceptance/js/FilestoreTests.js

@@ -148,12 +148,8 @@ describe('Filestore', function() {
       })
 
       beforeEach(async function() {
-        // retrieve previous metrics from the app
         if (Settings.filestore.backend === 's3') {
-          ;[previousEgress, previousIngress] = await Promise.all([
-            getMetric(filestoreUrl, 's3_egress'),
-            getMetric(filestoreUrl, 's3_ingress')
-          ])
+          previousEgress = await getMetric(filestoreUrl, 's3_egress')
         }
         projectId = `acceptance_tests_${Math.random()}`
       })
@@ -195,6 +191,15 @@ describe('Filestore', function() {
           await pipeline(readStream, writeStream, resultStream)
         })
 
+        beforeEach(async function retrievePreviousIngressMetrics() {
+          // The upload request can bump the ingress metric.
+          // The content hash validation might require a full download
+          //  in case the ETag field of the upload response is not a md5 sum.
+          if (Settings.filestore.backend === 's3') {
+            previousIngress = await getMetric(filestoreUrl, 's3_ingress')
+          }
+        })
+
         it('should return 404 for a non-existant id', async function() {
           const options = { uri: fileUrl + '___this_is_clearly_wrong___' }
           await expect(
@@ -415,8 +420,8 @@ describe('Filestore', function() {
 
             const s3ClientSettings = {
               credentials: {
-                accessKeyId: 'fake',
-                secretAccessKey: 'fake'
+                accessKeyId: process.env.AWS_ACCESS_KEY_ID,
+                secretAccessKey: process.env.AWS_SECRET_ACCESS_KEY
               },
               endpoint: process.env.AWS_S3_ENDPOINT,
               sslEnabled: false,

services/filestore/test/unit/js/FileHandlerTests.js

@@ -4,6 +4,9 @@ const { expect } = chai
 const modulePath = '../../../app/js/FileHandler.js'
 const SandboxedModule = require('sandboxed-module')
 
+chai.use(require('sinon-chai'))
+chai.use(require('chai-as-promised'))
+
 describe('FileHandler', function() {
   let PersistorManager,
     LocalFileWriter,
@@ -32,29 +35,41 @@ describe('FileHandler', function() {
 
   beforeEach(function() {
     PersistorManager = {
-      getFileStream: sinon.stub().yields(null, sourceStream),
-      checkIfFileExists: sinon.stub().yields(),
-      deleteFile: sinon.stub().yields(),
-      deleteDirectory: sinon.stub().yields(),
-      sendStream: sinon.stub().yields(),
-      insertFile: sinon.stub().yields(),
-      sendFile: sinon.stub().yields(),
-      directorySize: sinon.stub().yields()
+      promises: {
+        getFileStream: sinon.stub().resolves(sourceStream),
+        checkIfFileExists: sinon.stub().resolves(),
+        deleteFile: sinon.stub().resolves(),
+        deleteDirectory: sinon.stub().resolves(),
+        sendStream: sinon.stub().resolves(),
+        insertFile: sinon.stub().resolves(),
+        sendFile: sinon.stub().resolves(),
+        directorySize: sinon.stub().resolves()
+      }
     }
     LocalFileWriter = {
-      writeStream: sinon.stub().yields(),
-      deleteFile: sinon.stub().yields()
+      // the callback style is used for detached cleanup calls
+      deleteFile: sinon.stub().yields(),
+      promises: {
+        writeStream: sinon.stub().resolves(),
+        deleteFile: sinon.stub().resolves()
+      }
     }
     FileConverter = {
-      convert: sinon.stub().yields(),
-      thumbnail: sinon.stub().yields(),
-      preview: sinon.stub().yields()
+      promises: {
+        convert: sinon.stub().resolves(),
+        thumbnail: sinon.stub().resolves(),
+        preview: sinon.stub().resolves()
+      }
     }
     KeyBuilder = {
       addCachingToKey: sinon.stub().returns(convertedKey),
       getConvertedFolderKey: sinon.stub().returns(convertedFolderKey)
     }
-    ImageOptimiser = { compressPng: sinon.stub().yields() }
+    ImageOptimiser = {
+      promises: {
+        compressPng: sinon.stub().resolves()
+      }
+    }
     fs = {
       createReadStream: sinon.stub().returns(readStream)
     }
@@ -79,7 +94,7 @@ describe('FileHandler', function() {
     it('should send file to the filestore', function(done) {
       FileHandler.insertFile(bucket, key, stream, err => {
         expect(err).not.to.exist
-        expect(PersistorManager.sendStream).to.have.been.calledWith(
+        expect(PersistorManager.promises.sendStream).to.have.been.calledWith(
           bucket,
           key,
           stream
@@ -91,10 +106,9 @@ describe('FileHandler', function() {
     it('should delete the convertedKey folder', function(done) {
       FileHandler.insertFile(bucket, key, stream, err => {
         expect(err).not.to.exist
-        expect(PersistorManager.deleteDirectory).to.have.been.calledWith(
-          bucket,
-          convertedFolderKey
-        )
+        expect(
+          PersistorManager.promises.deleteDirectory
+        ).to.have.been.calledWith(bucket, convertedFolderKey)
         done()
       })
     })
@@ -104,7 +118,10 @@ describe('FileHandler', function() {
     it('should tell the filestore manager to delete the file', function(done) {
       FileHandler.deleteFile(bucket, key, err => {
         expect(err).not.to.exist
-        expect(PersistorManager.deleteFile).to.have.been.calledWith(bucket, key)
+        expect(PersistorManager.promises.deleteFile).to.have.been.calledWith(
+          bucket,
+          key
+        )
         done()
       })
     })
@@ -112,10 +129,9 @@ describe('FileHandler', function() {
     it('should tell the filestore manager to delete the cached folder', function(done) {
       FileHandler.deleteFile(bucket, key, err => {
         expect(err).not.to.exist
-        expect(PersistorManager.deleteDirectory).to.have.been.calledWith(
-          bucket,
-          convertedFolderKey
-        )
+        expect(
+          PersistorManager.promises.deleteDirectory
+        ).to.have.been.calledWith(bucket, convertedFolderKey)
         done()
       })
     })
@@ -134,7 +150,7 @@ describe('FileHandler', function() {
       const options = { start: 0, end: 8 }
       FileHandler.getFile(bucket, key, options, err => {
         expect(err).not.to.exist
-        expect(PersistorManager.getFileStream).to.have.been.calledWith(
+        expect(PersistorManager.promises.getFileStream).to.have.been.calledWith(
           bucket,
           key,
           options
@@ -155,23 +171,27 @@ describe('FileHandler', function() {
         })
 
         it('should convert the file', function() {
-          expect(FileConverter.convert).to.have.been.called
-          expect(ImageOptimiser.compressPng).to.have.been.called
+          expect(FileConverter.promises.convert).to.have.been.called
+        })
+
+        it('should compress the converted file', function() {
+          expect(ImageOptimiser.promises.compressPng).to.have.been.called
         })
 
         it('should return the the converted stream', function() {
           expect(result.err).not.to.exist
           expect(result.stream).to.equal(readStream)
-          expect(PersistorManager.getFileStream).to.have.been.calledWith(
-            bucket,
-            key
-          )
+          expect(
+            PersistorManager.promises.getFileStream
+          ).to.have.been.calledWith(bucket, key)
         })
       })
 
       describe('when the file is cached', function() {
         beforeEach(function(done) {
-          PersistorManager.checkIfFileExists = sinon.stub().yields(null, true)
+          PersistorManager.promises.checkIfFileExists = sinon
+            .stub()
+            .resolves(true)
           FileHandler.getFile(bucket, key, { format: 'png' }, (err, stream) => {
             result = { err, stream }
             done()
@@ -179,17 +199,19 @@ describe('FileHandler', function() {
         })
 
         it('should not convert the file', function() {
-          expect(FileConverter.convert).not.to.have.been.called
-          expect(ImageOptimiser.compressPng).not.to.have.been.called
+          expect(FileConverter.promises.convert).not.to.have.been.called
+        })
+
+        it('should not compress the converted file again', function() {
+          expect(ImageOptimiser.promises.compressPng).not.to.have.been.called
         })
 
         it('should return the cached stream', function() {
           expect(result.err).not.to.exist
           expect(result.stream).to.equal(sourceStream)
-          expect(PersistorManager.getFileStream).to.have.been.calledWith(
-            bucket,
-            convertedKey
-          )
+          expect(
+            PersistorManager.promises.getFileStream
+          ).to.have.been.calledWith(bucket, convertedKey)
         })
       })
     })
@@ -198,8 +220,8 @@ describe('FileHandler', function() {
       it('generates a thumbnail when requested', function(done) {
         FileHandler.getFile(bucket, key, { style: 'thumbnail' }, err => {
           expect(err).not.to.exist
-          expect(FileConverter.thumbnail).to.have.been.called
-          expect(FileConverter.preview).not.to.have.been.called
+          expect(FileConverter.promises.thumbnail).to.have.been.called
+          expect(FileConverter.promises.preview).not.to.have.been.called
           done()
         })
       })
@@ -207,8 +229,8 @@ describe('FileHandler', function() {
       it('generates a preview when requested', function(done) {
         FileHandler.getFile(bucket, key, { style: 'preview' }, err => {
           expect(err).not.to.exist
-          expect(FileConverter.thumbnail).not.to.have.been.called
-          expect(FileConverter.preview).to.have.been.called
+          expect(FileConverter.promises.thumbnail).not.to.have.been.called
+          expect(FileConverter.promises.preview).to.have.been.called
           done()
         })
       })
@@ -219,7 +241,7 @@ describe('FileHandler', function() {
     it('should call the filestore manager to get directory size', function(done) {
       FileHandler.getDirectorySize(bucket, key, err => {
         expect(err).not.to.exist
-        expect(PersistorManager.directorySize).to.have.been.calledWith(
+        expect(PersistorManager.promises.directorySize).to.have.been.calledWith(
           bucket,
           key
         )

services/filestore/test/unit/js/ImageOptimiserTests.js

@@ -19,7 +19,10 @@ describe('ImageOptimiser', function() {
     ImageOptimiser = SandboxedModule.require(modulePath, {
       requires: {
         './SafeExec': SafeExec,
-        'logger-sharelatex': logger
+        'logger-sharelatex': logger,
+        'metrics-sharelatex': {
+          Timer: sinon.stub().returns({ done: sinon.stub() })
+        }
       }
     })
   })

services/filestore/test/unit/js/KeybuilderTests.js

@@ -2,12 +2,14 @@ const SandboxedModule = require('sandboxed-module')
 
 const modulePath = '../../../app/js/KeyBuilder.js'
 
-describe('LocalFileWriter', function() {
+describe('KeybuilderTests', function() {
   let KeyBuilder
   const key = 'wombat/potato'
 
   beforeEach(function() {
-    KeyBuilder = SandboxedModule.require(modulePath)
+    KeyBuilder = SandboxedModule.require(modulePath, {
+      requires: { 'settings-sharelatex': {} }
+    })
   })
 
   describe('cachedKey', function() {

services/filestore/test/unit/js/S3PersistorTests.js

@@ -158,7 +158,7 @@ describe('S3PersistorTests', function() {
         'metrics-sharelatex': Metrics,
         crypto
       },
-      globals: { console }
+      globals: { console, Buffer }
     })
   })
 

services/filestore/test/unit/js/SafeExecTests.js

@@ -12,6 +12,7 @@ describe('SafeExec', function() {
     options = { timeout: 10 * 1000, killSignal: 'SIGTERM' }
 
     safeExec = SandboxedModule.require(modulePath, {
+      globals: { process },
       requires: {
         'settings-sharelatex': settings
       }

services/filestore/test/unit/js/SettingsTests.js

@@ -13,7 +13,7 @@ describe('Settings', function() {
       }
       process.env.S3_BUCKET_CREDENTIALS = JSON.stringify(s3Settings)
       const settings = SandboxedModule.require('settings-sharelatex', {
-        globals: { console }
+        globals: { console, process }
       })
       expect(settings.filestore.s3BucketCreds).to.deep.equal(s3Settings)
     })