Răsfoiți Sursa

Migrate to new object-persistor module (#122)

* Migrate to new object-persistor module

* Support updated persistor module using ES6 classes

* Update object-persistor

* Upgrade new persistor code to OError3

* Update to latest minor release for outdated packages

* Update package hashes to sha512

* Point object-persistor at master branch
Simon Detheridge 6 ani în urmă
părinte
comite
6c853de5d5
28 a modificat fișierele cu 565 adăugiri și 4216 ștergeri
  1. 1 1
      services/filestore/.eslintrc
  2. 3 9
      services/filestore/app/js/Errors.js
  3. 0 247
      services/filestore/app/js/FSPersistor.js
  4. 9 16
      services/filestore/app/js/FileController.js
  5. 5 6
      services/filestore/app/js/FileConverter.js
  6. 28 37
      services/filestore/app/js/FileHandler.js
  7. 0 305
      services/filestore/app/js/GcsPersistor.js
  8. 1 1
      services/filestore/app/js/HealthCheckController.js
  9. 2 4
      services/filestore/app/js/LocalFileWriter.js
  10. 0 230
      services/filestore/app/js/MigrationPersistor.js
  11. 0 177
      services/filestore/app/js/PersistorHelper.js
  12. 5 34
      services/filestore/app/js/PersistorManager.js
  13. 0 367
      services/filestore/app/js/S3Persistor.js
  14. 412 209
      services/filestore/package-lock.json
  15. 18 17
      services/filestore/package.json
  16. 1 0
      services/filestore/test/acceptance/js/FilestoreApp.js
  17. 4 4
      services/filestore/test/acceptance/js/FilestoreTests.js
  18. 2 2
      services/filestore/test/acceptance/js/TestHelper.js
  19. 0 353
      services/filestore/test/unit/js/FSPersistorTests.js
  20. 15 9
      services/filestore/test/unit/js/FileControllerTests.js
  21. 5 1
      services/filestore/test/unit/js/FileConverterTests.js
  22. 44 40
      services/filestore/test/unit/js/FileHandlerTests.js
  23. 0 683
      services/filestore/test/unit/js/GcsPersistorTests.js
  24. 5 1
      services/filestore/test/unit/js/LocalFileWriterTests.js
  25. 0 519
      services/filestore/test/unit/js/MigrationPersistorTests.js
  26. 0 78
      services/filestore/test/unit/js/PersistorManagerTests.js
  27. 0 865
      services/filestore/test/unit/js/S3PersistorTests.js
  28. 5 1
      services/filestore/test/unit/js/SafeExecTests.js

+ 1 - 1
services/filestore/.eslintrc

@@ -8,7 +8,7 @@
     "prettier/standard"
   ],
   "parserOptions": {
-    "ecmaVersion": 2017
+    "ecmaVersion": 2018
   },
   "plugins": [
     "mocha",

+ 3 - 9
services/filestore/app/js/Errors.js

@@ -1,12 +1,9 @@
 const OError = require('@overleaf/o-error')
+const { Errors } = require('@overleaf/object-persistor')
 
-class NotFoundError extends OError {}
-class WriteError extends OError {}
-class ReadError extends OError {}
 class HealthCheckError extends OError {}
 class ConversionsDisabledError extends OError {}
 class ConversionError extends OError {}
-class SettingsError extends OError {}
 class TimeoutError extends OError {}
 class InvalidParametersError extends OError {}
 
@@ -23,14 +20,11 @@ class FailedCommandError extends OError {
 }
 
 module.exports = {
-  NotFoundError,
   FailedCommandError,
   ConversionsDisabledError,
-  WriteError,
-  ReadError,
   ConversionError,
   HealthCheckError,
-  SettingsError,
   TimeoutError,
-  InvalidParametersError
+  InvalidParametersError,
+  ...Errors
 }

+ 0 - 247
services/filestore/app/js/FSPersistor.js

@@ -1,247 +0,0 @@
-const fs = require('fs')
-const glob = require('glob')
-const path = require('path')
-const Stream = require('stream')
-const { promisify, callbackify } = require('util')
-
-const LocalFileWriter = require('./LocalFileWriter').promises
-const { NotFoundError, ReadError, WriteError } = require('./Errors')
-const PersistorHelper = require('./PersistorHelper')
-
-const pipeline = promisify(Stream.pipeline)
-const fsUnlink = promisify(fs.unlink)
-const fsOpen = promisify(fs.open)
-const fsStat = promisify(fs.stat)
-const fsGlob = promisify(glob)
-
-const filterName = key => key.replace(/\//g, '_')
-
-async function sendFile(location, target, source) {
-  const filteredTarget = filterName(target)
-
-  // actually copy the file (instead of moving it) to maintain consistent behaviour
-  // between the different implementations
-  try {
-    const sourceStream = fs.createReadStream(source)
-    const targetStream = fs.createWriteStream(`${location}/${filteredTarget}`)
-    await pipeline(sourceStream, targetStream)
-  } catch (err) {
-    throw PersistorHelper.wrapError(
-      err,
-      'failed to copy the specified file',
-      { location, target, source },
-      WriteError
-    )
-  }
-}
-
-async function sendStream(location, target, sourceStream, sourceMd5) {
-  const fsPath = await LocalFileWriter.writeStream(sourceStream)
-  if (!sourceMd5) {
-    sourceMd5 = await _getFileMd5HashForPath(fsPath)
-  }
-
-  try {
-    await sendFile(location, target, fsPath)
-    const destMd5 = await getFileMd5Hash(location, target)
-    if (sourceMd5 !== destMd5) {
-      await LocalFileWriter.deleteFile(`${location}/${filterName(target)}`)
-      throw new WriteError('md5 hash mismatch', {
-        sourceMd5,
-        destMd5,
-        location,
-        target
-      })
-    }
-  } finally {
-    await LocalFileWriter.deleteFile(fsPath)
-  }
-}
-
-// opts may be {start: Number, end: Number}
-async function getFileStream(location, name, opts) {
-  const filteredName = filterName(name)
-
-  try {
-    opts.fd = await fsOpen(`${location}/${filteredName}`, 'r')
-  } catch (err) {
-    throw PersistorHelper.wrapError(
-      err,
-      'failed to open file for streaming',
-      { location, filteredName, opts },
-      ReadError
-    )
-  }
-
-  return fs.createReadStream(null, opts)
-}
-
-async function getRedirectUrl() {
-  // not implemented
-  return null
-}
-
-async function getFileSize(location, filename) {
-  const fullPath = path.join(location, filterName(filename))
-
-  try {
-    const stat = await fsStat(fullPath)
-    return stat.size
-  } catch (err) {
-    throw PersistorHelper.wrapError(
-      err,
-      'failed to stat file',
-      { location, filename },
-      ReadError
-    )
-  }
-}
-
-async function getFileMd5Hash(location, filename) {
-  const fullPath = path.join(location, filterName(filename))
-  try {
-    return await _getFileMd5HashForPath(fullPath)
-  } catch (err) {
-    throw new ReadError('unable to get md5 hash from file', {
-      location,
-      filename
-    }).withCause(err)
-  }
-}
-
-async function copyFile(location, fromName, toName) {
-  const filteredFromName = filterName(fromName)
-  const filteredToName = filterName(toName)
-
-  try {
-    const sourceStream = fs.createReadStream(`${location}/${filteredFromName}`)
-    const targetStream = fs.createWriteStream(`${location}/${filteredToName}`)
-    await pipeline(sourceStream, targetStream)
-  } catch (err) {
-    throw PersistorHelper.wrapError(
-      err,
-      'failed to copy file',
-      { location, filteredFromName, filteredToName },
-      WriteError
-    )
-  }
-}
-
-async function deleteFile(location, name) {
-  const filteredName = filterName(name)
-  try {
-    await fsUnlink(`${location}/${filteredName}`)
-  } catch (err) {
-    const wrappedError = PersistorHelper.wrapError(
-      err,
-      'failed to delete file',
-      { location, filteredName },
-      WriteError
-    )
-    if (!(wrappedError instanceof NotFoundError)) {
-      // S3 doesn't give us a 404 when a file wasn't there to be deleted, so we
-      // should be consistent here as well
-      throw wrappedError
-    }
-  }
-}
-
-// this is only called internally for clean-up by `FileHandler` and isn't part of the external API
-async function deleteDirectory(location, name) {
-  const filteredName = filterName(name.replace(/\/$/, ''))
-
-  try {
-    await Promise.all(
-      (await fsGlob(`${location}/${filteredName}*`)).map(file => fsUnlink(file))
-    )
-  } catch (err) {
-    throw PersistorHelper.wrapError(
-      err,
-      'failed to delete directory',
-      { location, filteredName },
-      WriteError
-    )
-  }
-}
-
-async function checkIfFileExists(location, name) {
-  const filteredName = filterName(name)
-  try {
-    const stat = await fsStat(`${location}/${filteredName}`)
-    return !!stat
-  } catch (err) {
-    if (err.code === 'ENOENT') {
-      return false
-    }
-    throw PersistorHelper.wrapError(
-      err,
-      'failed to stat file',
-      { location, filteredName },
-      ReadError
-    )
-  }
-}
-
-// note, does not recurse into subdirectories, as we use a flattened directory structure
-async function directorySize(location, name) {
-  const filteredName = filterName(name.replace(/\/$/, ''))
-  let size = 0
-
-  try {
-    const files = await fsGlob(`${location}/${filteredName}_*`)
-    for (const file of files) {
-      try {
-        const stat = await fsStat(file)
-        if (stat.isFile()) {
-          size += stat.size
-        }
-      } catch (err) {
-        // ignore files that may have just been deleted
-        if (err.code !== 'ENOENT') {
-          throw err
-        }
-      }
-    }
-  } catch (err) {
-    throw PersistorHelper.wrapError(
-      err,
-      'failed to get directory size',
-      { location, name },
-      ReadError
-    )
-  }
-
-  return size
-}
-
-module.exports = {
-  sendFile: callbackify(sendFile),
-  sendStream: callbackify(sendStream),
-  getFileStream: callbackify(getFileStream),
-  getRedirectUrl: callbackify(getRedirectUrl),
-  getFileSize: callbackify(getFileSize),
-  getFileMd5Hash: callbackify(getFileMd5Hash),
-  copyFile: callbackify(copyFile),
-  deleteFile: callbackify(deleteFile),
-  deleteDirectory: callbackify(deleteDirectory),
-  checkIfFileExists: callbackify(checkIfFileExists),
-  directorySize: callbackify(directorySize),
-  promises: {
-    sendFile,
-    sendStream,
-    getFileStream,
-    getRedirectUrl,
-    getFileSize,
-    getFileMd5Hash,
-    copyFile,
-    deleteFile,
-    deleteDirectory,
-    checkIfFileExists,
-    directorySize
-  }
-}
-
-async function _getFileMd5HashForPath(fullPath) {
-  const stream = fs.createReadStream(fullPath)
-  return PersistorHelper.calculateStreamMd5(stream)
-}

+ 9 - 16
services/filestore/app/js/FileController.js

@@ -75,12 +75,11 @@ function getFile(req, res, next) {
           res.end()
         } else if (err) {
           next(
-            new Errors.ReadError('error transferring stream', {
-              bucket,
-              key,
-              format,
-              style
-            }).withCause(err)
+            new Errors.ReadError(
+              'error transferring stream',
+              { bucket, key, format, style },
+              err
+            )
           )
         }
       })
@@ -139,23 +138,17 @@ function copyFile(req, res, next) {
   })
   req.requestLogger.setMessage('copying file')
 
-  PersistorManager.copyFile(
-    bucket,
-    `${oldProjectId}/${oldFileId}`,
-    key,
-    function(err) {
+  PersistorManager.copyObject(bucket, `${oldProjectId}/${oldFileId}`, key)
+    .then(() => res.sendStatus(200))
+    .catch(err => {
       if (err) {
         if (err instanceof Errors.NotFoundError) {
           res.sendStatus(404)
         } else {
           next(err)
         }
-        return
       }
-
-      res.sendStatus(200)
-    }
-  )
+    })
 }
 
 function deleteFile(req, res, next) {

+ 5 - 6
services/filestore/app/js/FileConverter.js

@@ -86,12 +86,11 @@ async function _convert(sourcePath, requestedFormat, command) {
       timeout: FOURTY_SECONDS
     })
   } catch (err) {
-    throw new ConversionError('something went wrong converting file', {
-      stderr: err.stderr,
-      sourcePath,
-      requestedFormat,
-      destPath
-    }).withCause(err)
+    throw new ConversionError(
+      'something went wrong converting file',
+      { stderr: err.stderr, sourcePath, requestedFormat, destPath },
+      err
+    )
   }
 
   timer.done()

+ 28 - 37
services/filestore/app/js/FileHandler.js

@@ -37,9 +37,9 @@ async function insertFile(bucket, key, stream) {
     })
   }
   if (Settings.enableConversions) {
-    await PersistorManager.promises.deleteDirectory(bucket, convertedKey)
+    await PersistorManager.deleteDirectory(bucket, convertedKey)
   }
-  await PersistorManager.promises.sendStream(bucket, key, stream)
+  await PersistorManager.sendStream(bucket, key, stream)
 }
 
 async function deleteFile(bucket, key) {
@@ -51,9 +51,9 @@ async function deleteFile(bucket, key) {
       convertedKey
     })
   }
-  const jobs = [PersistorManager.promises.deleteFile(bucket, key)]
+  const jobs = [PersistorManager.deleteObject(bucket, key)]
   if (Settings.enableConversions) {
-    jobs.push(PersistorManager.promises.deleteDirectory(bucket, convertedKey))
+    jobs.push(PersistorManager.deleteDirectory(bucket, convertedKey))
   }
   await Promise.all(jobs)
 }
@@ -65,13 +65,13 @@ async function deleteProject(bucket, key) {
       key
     })
   }
-  await PersistorManager.promises.deleteDirectory(bucket, key)
+  await PersistorManager.deleteDirectory(bucket, key)
 }
 
 async function getFile(bucket, key, opts) {
   opts = opts || {}
   if (!opts.format && !opts.style) {
-    return PersistorManager.promises.getFileStream(bucket, key, opts)
+    return PersistorManager.getObjectStream(bucket, key, opts)
   } else {
     return _getConvertedFile(bucket, key, opts)
   }
@@ -89,28 +89,28 @@ async function getRedirectUrl(bucket, key, opts) {
     Object.values(Settings.filestore.stores).includes(bucket) &&
     Settings.filestore.allowRedirects
   ) {
-    return PersistorManager.promises.getRedirectUrl(bucket, key)
+    return PersistorManager.getRedirectUrl(bucket, key)
   }
 
   return null
 }
 
 async function getFileSize(bucket, key) {
-  return PersistorManager.promises.getFileSize(bucket, key)
+  return PersistorManager.getObjectSize(bucket, key)
 }
 
 async function getDirectorySize(bucket, projectId) {
-  return PersistorManager.promises.directorySize(bucket, projectId)
+  return PersistorManager.directorySize(bucket, projectId)
 }
 
 async function _getConvertedFile(bucket, key, opts) {
   const convertedKey = KeyBuilder.addCachingToKey(key, opts)
-  const exists = await PersistorManager.promises.checkIfFileExists(
+  const exists = await PersistorManager.checkIfObjectExists(
     bucket,
     convertedKey
   )
   if (exists) {
-    return PersistorManager.promises.getFileStream(bucket, convertedKey, opts)
+    return PersistorManager.getObjectStream(bucket, convertedKey, opts)
   } else {
     return _getConvertedFileAndCache(bucket, key, convertedKey, opts)
   }
@@ -121,19 +121,14 @@ async function _getConvertedFileAndCache(bucket, key, convertedKey, opts) {
   try {
     convertedFsPath = await _convertFile(bucket, key, opts)
     await ImageOptimiser.promises.compressPng(convertedFsPath)
-    await PersistorManager.promises.sendFile(
-      bucket,
-      convertedKey,
-      convertedFsPath
-    )
+    await PersistorManager.sendFile(bucket, convertedKey, convertedFsPath)
   } catch (err) {
     LocalFileWriter.deleteFile(convertedFsPath, () => {})
-    throw new ConversionError('failed to convert file', {
-      opts,
-      bucket,
-      key,
-      convertedKey
-    }).withCause(err)
+    throw new ConversionError(
+      'failed to convert file',
+      { opts, bucket, key, convertedKey },
+      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
@@ -159,11 +154,11 @@ async function _convertFile(bucket, originalKey, opts) {
   try {
     originalFsPath = await _writeFileToDisk(bucket, originalKey, opts)
   } catch (err) {
-    throw new ConversionError('unable to write file to disk', {
-      bucket,
-      originalKey,
-      opts
-    }).withCause(err)
+    throw new ConversionError(
+      'unable to write file to disk',
+      { bucket, originalKey, opts },
+      err
+    )
   }
 
   let promise
@@ -184,21 +179,17 @@ async function _convertFile(bucket, originalKey, opts) {
   try {
     destPath = await promise
   } catch (err) {
-    throw new ConversionError('error converting file', {
-      bucket,
-      originalKey,
-      opts
-    }).withCause(err)
+    throw new ConversionError(
+      'error converting file',
+      { bucket, originalKey, opts },
+      err
+    )
   }
   LocalFileWriter.deleteFile(originalFsPath, function() {})
   return destPath
 }
 
 async function _writeFileToDisk(bucket, key, opts) {
-  const fileStream = await PersistorManager.promises.getFileStream(
-    bucket,
-    key,
-    opts
-  )
+  const fileStream = await PersistorManager.getObjectStream(bucket, key, opts)
   return LocalFileWriter.promises.writeStream(fileStream, key)
 }

+ 0 - 305
services/filestore/app/js/GcsPersistor.js

@@ -1,305 +0,0 @@
-const settings = require('settings-sharelatex')
-const fs = require('fs')
-const { promisify } = require('util')
-const Stream = require('stream')
-const { Storage } = require('@google-cloud/storage')
-const { callbackify } = require('util')
-const { WriteError, ReadError, NotFoundError } = require('./Errors')
-const asyncPool = require('tiny-async-pool')
-const PersistorHelper = require('./PersistorHelper')
-
-const pipeline = promisify(Stream.pipeline)
-
-// endpoint settings will be null by default except for tests
-// that's OK - GCS uses the locally-configured service account by default
-const storage = new Storage(settings.filestore.gcs.endpoint)
-// workaround for broken uploads with custom endpoints:
-// https://github.com/googleapis/nodejs-storage/issues/898
-if (
-  settings.filestore.gcs.endpoint &&
-  settings.filestore.gcs.endpoint.apiEndpoint
-) {
-  storage.interceptors.push({
-    request: function(reqOpts) {
-      const url = new URL(reqOpts.uri)
-      url.host = settings.filestore.gcs.endpoint.apiEndpoint
-      if (settings.filestore.gcs.endpoint.apiScheme) {
-        url.protocol = settings.filestore.gcs.endpoint.apiScheme
-      }
-      reqOpts.uri = url.toString()
-      return reqOpts
-    }
-  })
-}
-
-const GcsPersistor = {
-  sendFile: callbackify(sendFile),
-  sendStream: callbackify(sendStream),
-  getFileStream: callbackify(getFileStream),
-  getRedirectUrl: callbackify(getRedirectUrl),
-  getFileMd5Hash: callbackify(getFileMd5Hash),
-  deleteDirectory: callbackify(deleteDirectory),
-  getFileSize: callbackify(getFileSize),
-  deleteFile: callbackify(deleteFile),
-  copyFile: callbackify(copyFile),
-  checkIfFileExists: callbackify(checkIfFileExists),
-  directorySize: callbackify(directorySize),
-  promises: {
-    sendFile,
-    sendStream,
-    getFileStream,
-    getRedirectUrl,
-    getFileMd5Hash,
-    deleteDirectory,
-    getFileSize,
-    deleteFile,
-    copyFile,
-    checkIfFileExists,
-    directorySize
-  }
-}
-
-module.exports = GcsPersistor
-
-async function sendFile(bucketName, key, fsPath) {
-  return sendStream(bucketName, key, fs.createReadStream(fsPath))
-}
-
-async function sendStream(bucketName, key, readStream, sourceMd5) {
-  try {
-    // egress from us to gcs
-    const observeOptions = { metric: 'gcs.egress' }
-
-    if (!sourceMd5) {
-      // if there is no supplied md5 hash, we calculate the hash as the data passes through
-      observeOptions.hash = 'md5'
-    }
-
-    const observer = new PersistorHelper.ObserverStream(observeOptions)
-
-    const writeOptions = {
-      // disabling of resumable uploads is recommended by Google:
-      resumable: false
-    }
-
-    if (sourceMd5) {
-      writeOptions.validation = 'md5'
-      writeOptions.metadata = {
-        md5Hash: PersistorHelper.hexToBase64(sourceMd5)
-      }
-    }
-
-    const uploadStream = storage
-      .bucket(bucketName)
-      .file(key)
-      .createWriteStream(writeOptions)
-
-    await pipeline(readStream, observer, uploadStream)
-
-    // if we didn't have an md5 hash, we should compare our computed one with Google's
-    // as we couldn't tell GCS about it beforehand
-    if (!sourceMd5) {
-      sourceMd5 = observer.getHash()
-      // throws on mismatch
-      await PersistorHelper.verifyMd5(GcsPersistor, bucketName, key, sourceMd5)
-    }
-  } catch (err) {
-    throw PersistorHelper.wrapError(
-      err,
-      'upload to GCS failed',
-      { bucketName, key },
-      WriteError
-    )
-  }
-}
-
-async function getFileStream(bucketName, key, _opts = {}) {
-  const opts = Object.assign({}, _opts)
-  const stream = storage
-    .bucket(bucketName)
-    .file(key)
-    .createReadStream(opts)
-
-  // ingress to us from gcs
-  const observer = new PersistorHelper.ObserverStream({
-    metric: 'gcs.ingress'
-  })
-
-  try {
-    // wait for the pipeline to be ready, to catch non-200s
-    await PersistorHelper.getReadyPipeline(stream, observer)
-    return observer
-  } catch (err) {
-    throw PersistorHelper.wrapError(
-      err,
-      'error reading file from GCS',
-      { bucketName, key, opts },
-      ReadError
-    )
-  }
-}
-
-async function getRedirectUrl(bucketName, key) {
-  try {
-    const [url] = await storage
-      .bucket(bucketName)
-      .file(key)
-      .getSignedUrl({
-        action: 'read',
-        expires: new Date().getTime() + settings.filestore.signedUrlExpiryInMs
-      })
-    return url
-  } catch (err) {
-    throw PersistorHelper.wrapError(
-      err,
-      'error generating signed url for GCS file',
-      { bucketName, key },
-      ReadError
-    )
-  }
-}
-
-async function getFileSize(bucketName, key) {
-  try {
-    const [metadata] = await storage
-      .bucket(bucketName)
-      .file(key)
-      .getMetadata()
-    return metadata.size
-  } catch (err) {
-    throw PersistorHelper.wrapError(
-      err,
-      'error getting size of GCS object',
-      { bucketName, key },
-      ReadError
-    )
-  }
-}
-
-async function getFileMd5Hash(bucketName, key) {
-  try {
-    const [metadata] = await storage
-      .bucket(bucketName)
-      .file(key)
-      .getMetadata()
-    return PersistorHelper.base64ToHex(metadata.md5Hash)
-  } catch (err) {
-    throw PersistorHelper.wrapError(
-      err,
-      'error getting hash of GCS object',
-      { bucketName, key },
-      ReadError
-    )
-  }
-}
-
-async function deleteFile(bucketName, key) {
-  try {
-    const file = storage.bucket(bucketName).file(key)
-
-    if (settings.filestore.gcs.deletedBucketSuffix) {
-      await file.copy(
-        storage
-          .bucket(`${bucketName}${settings.filestore.gcs.deletedBucketSuffix}`)
-          .file(`${key}-${new Date().toISOString()}`)
-      )
-    }
-    if (settings.filestore.gcs.unlockBeforeDelete) {
-      await file.setMetadata({ eventBasedHold: false })
-    }
-    await file.delete()
-  } catch (err) {
-    const error = PersistorHelper.wrapError(
-      err,
-      'error deleting GCS object',
-      { bucketName, key },
-      WriteError
-    )
-    if (!(error instanceof NotFoundError)) {
-      throw error
-    }
-  }
-}
-
-async function deleteDirectory(bucketName, key) {
-  try {
-    const [files] = await storage
-      .bucket(bucketName)
-      .getFiles({ directory: key })
-
-    await asyncPool(
-      settings.filestore.gcs.deleteConcurrency,
-      files,
-      async file => {
-        await deleteFile(bucketName, file.name)
-      }
-    )
-  } catch (err) {
-    const error = PersistorHelper.wrapError(
-      err,
-      'failed to delete directory in GCS',
-      { bucketName, key },
-      WriteError
-    )
-    if (error instanceof NotFoundError) {
-      return
-    }
-    throw error
-  }
-}
-
-async function directorySize(bucketName, key) {
-  let files
-
-  try {
-    const [response] = await storage
-      .bucket(bucketName)
-      .getFiles({ directory: key })
-    files = response
-  } catch (err) {
-    throw PersistorHelper.wrapError(
-      err,
-      'failed to list objects in GCS',
-      { bucketName, key },
-      ReadError
-    )
-  }
-
-  return files.reduce((acc, file) => Number(file.metadata.size) + acc, 0)
-}
-
-async function checkIfFileExists(bucketName, key) {
-  try {
-    const [response] = await storage
-      .bucket(bucketName)
-      .file(key)
-      .exists()
-    return response
-  } catch (err) {
-    throw PersistorHelper.wrapError(
-      err,
-      'error checking if file exists in GCS',
-      { bucketName, key },
-      ReadError
-    )
-  }
-}
-
-async function copyFile(bucketName, sourceKey, destKey) {
-  try {
-    const src = storage.bucket(bucketName).file(sourceKey)
-    const dest = storage.bucket(bucketName).file(destKey)
-    await src.copy(dest)
-  } catch (err) {
-    // fake-gcs-server has a bug that returns an invalid response when the file does not exist
-    if (err.message === 'Cannot parse response as JSON: not found\n') {
-      err.code = 404
-    }
-    throw PersistorHelper.wrapError(
-      err,
-      'failed to copy file in GCS',
-      { bucketName, sourceKey, destKey },
-      WriteError
-    )
-  }
-}

+ 1 - 1
services/filestore/app/js/HealthCheckController.js

@@ -31,7 +31,7 @@ async function checkCanGetFiles() {
   try {
     await pipeline(sourceStream, buffer)
   } catch (err) {
-    throw new HealthCheckError('failed to get health-check file').withCause(err)
+    throw new HealthCheckError('failed to get health-check file', {}, err)
   }
 
   if (!buffer.size()) {

+ 2 - 4
services/filestore/app/js/LocalFileWriter.js

@@ -30,9 +30,7 @@ async function writeStream(stream, key) {
   } catch (err) {
     await deleteFile(fsPath)
 
-    throw new WriteError('problem writing file locally', {
-      fsPath
-    }).withCause(err)
+    throw new WriteError('problem writing file locally', { fsPath }, err)
   }
 }
 
@@ -44,7 +42,7 @@ async function deleteFile(fsPath) {
     await promisify(fs.unlink)(fsPath)
   } catch (err) {
     if (err.code !== 'ENOENT') {
-      throw new WriteError('failed to delete file', { fsPath }).withCause(err)
+      throw new WriteError('failed to delete file', { fsPath }, err)
     }
   }
 }

+ 0 - 230
services/filestore/app/js/MigrationPersistor.js

@@ -1,230 +0,0 @@
-const metrics = require('metrics-sharelatex')
-const Settings = require('settings-sharelatex')
-const logger = require('logger-sharelatex')
-const Stream = require('stream')
-const { callbackify, promisify } = require('util')
-const { NotFoundError, WriteError } = require('./Errors')
-
-const pipeline = promisify(Stream.pipeline)
-
-// Persistor that wraps two other persistors. Talks to the 'primary' by default,
-// but will fall back to an older persistor in the case of a not-found error.
-// If `Settings.filestore.fallback.copyOnMiss` is set, this will copy files from the fallback
-// to the primary, in the event that they are missing.
-//
-// It is unlikely that the bucket/location name will be the same on the fallback
-// as the primary. The bucket names should be overridden in `Settings.filestore.fallback.buckets`
-// e.g.
-// Settings.filestore.fallback.buckets = {
-//   myBucketOnS3: 'myBucketOnGCS'
-// }
-
-module.exports = function(primary, fallback) {
-  function _wrapMethodOnBothPersistors(method) {
-    return async function(bucket, key, ...moreArgs) {
-      const fallbackBucket = _getFallbackBucket(bucket)
-
-      await Promise.all([
-        primary.promises[method](bucket, key, ...moreArgs),
-        fallback.promises[method](fallbackBucket, key, ...moreArgs)
-      ])
-    }
-  }
-
-  async function getFileStreamWithFallback(bucket, key, opts) {
-    const shouldCopy =
-      Settings.filestore.fallback.copyOnMiss && !opts.start && !opts.end
-
-    try {
-      return await primary.promises.getFileStream(bucket, key, opts)
-    } catch (err) {
-      if (err instanceof NotFoundError) {
-        const fallbackBucket = _getFallbackBucket(bucket)
-        const fallbackStream = await fallback.promises.getFileStream(
-          fallbackBucket,
-          key,
-          opts
-        )
-        // tee the stream to the client, and as a copy to the primary (if necessary)
-        // start listening on both straight away so that we don't consume bytes
-        // in one place before the other
-        const returnStream = new Stream.PassThrough()
-        pipeline(fallbackStream, returnStream)
-
-        if (shouldCopy) {
-          const copyStream = new Stream.PassThrough()
-          pipeline(fallbackStream, copyStream)
-
-          _copyStreamFromFallbackAndVerify(
-            copyStream,
-            fallbackBucket,
-            bucket,
-            key,
-            key
-          ).catch(() => {
-            // swallow errors, as this runs in the background and will log a warning
-          })
-        }
-        return returnStream
-      }
-      throw err
-    }
-  }
-
-  async function copyFileWithFallback(bucket, sourceKey, destKey) {
-    try {
-      return await primary.promises.copyFile(bucket, sourceKey, destKey)
-    } catch (err) {
-      if (err instanceof NotFoundError) {
-        const fallbackBucket = _getFallbackBucket(bucket)
-        const fallbackStream = await fallback.promises.getFileStream(
-          fallbackBucket,
-          sourceKey,
-          {}
-        )
-
-        const copyStream = new Stream.PassThrough()
-        pipeline(fallbackStream, copyStream)
-
-        if (Settings.filestore.fallback.copyOnMiss) {
-          const missStream = new Stream.PassThrough()
-          pipeline(fallbackStream, missStream)
-
-          // copy from sourceKey -> sourceKey
-          _copyStreamFromFallbackAndVerify(
-            missStream,
-            fallbackBucket,
-            bucket,
-            sourceKey,
-            sourceKey
-          ).then(() => {
-            // swallow errors, as this runs in the background and will log a warning
-          })
-        }
-        // copy from sourceKey -> destKey
-        return _copyStreamFromFallbackAndVerify(
-          copyStream,
-          fallbackBucket,
-          bucket,
-          sourceKey,
-          destKey
-        )
-      }
-      throw err
-    }
-  }
-
-  function _getFallbackBucket(bucket) {
-    return Settings.filestore.fallback.buckets[bucket] || bucket
-  }
-
-  function _wrapFallbackMethod(method) {
-    return async function(bucket, key, ...moreArgs) {
-      try {
-        return await primary.promises[method](bucket, key, ...moreArgs)
-      } catch (err) {
-        if (err instanceof NotFoundError) {
-          const fallbackBucket = _getFallbackBucket(bucket)
-          if (Settings.filestore.fallback.copyOnMiss) {
-            const fallbackStream = await fallback.promises.getFileStream(
-              fallbackBucket,
-              key,
-              {}
-            )
-            // run in background
-            _copyStreamFromFallbackAndVerify(
-              fallbackStream,
-              fallbackBucket,
-              bucket,
-              key,
-              key
-            ).catch(err => {
-              logger.warn({ err }, 'failed to copy file from fallback')
-            })
-          }
-          return fallback.promises[method](fallbackBucket, key, ...moreArgs)
-        }
-        throw err
-      }
-    }
-  }
-
-  async function _copyStreamFromFallbackAndVerify(
-    stream,
-    sourceBucket,
-    destBucket,
-    sourceKey,
-    destKey
-  ) {
-    try {
-      let sourceMd5
-      try {
-        sourceMd5 = await fallback.promises.getFileMd5Hash(
-          sourceBucket,
-          sourceKey
-        )
-      } catch (err) {
-        logger.warn(err, 'error getting md5 hash from fallback persistor')
-      }
-
-      await primary.promises.sendStream(destBucket, destKey, stream, sourceMd5)
-    } catch (err) {
-      const error = new WriteError(
-        'unable to copy file to destination persistor',
-        {
-          sourceBucket,
-          destBucket,
-          sourceKey,
-          destKey
-        }
-      ).withCause(err)
-      metrics.inc('fallback.copy.failure')
-
-      try {
-        await primary.promises.deleteFile(destBucket, destKey)
-      } catch (err) {
-        error.info.cleanupError = new WriteError(
-          'unable to clean up destination copy artifact',
-          {
-            destBucket,
-            destKey
-          }
-        ).withCause(err)
-      }
-
-      logger.warn({ error }, 'failed to copy file from fallback')
-      throw error
-    }
-  }
-
-  return {
-    primaryPersistor: primary,
-    fallbackPersistor: fallback,
-    sendFile: primary.sendFile,
-    sendStream: primary.sendStream,
-    getFileStream: callbackify(getFileStreamWithFallback),
-    getRedirectUrl: primary.getRedirectUrl,
-    getFileMd5Hash: callbackify(_wrapFallbackMethod('getFileMd5Hash')),
-    deleteDirectory: callbackify(
-      _wrapMethodOnBothPersistors('deleteDirectory')
-    ),
-    getFileSize: callbackify(_wrapFallbackMethod('getFileSize')),
-    deleteFile: callbackify(_wrapMethodOnBothPersistors('deleteFile')),
-    copyFile: callbackify(copyFileWithFallback),
-    checkIfFileExists: callbackify(_wrapFallbackMethod('checkIfFileExists')),
-    directorySize: callbackify(_wrapFallbackMethod('directorySize')),
-    promises: {
-      sendFile: primary.promises.sendFile,
-      sendStream: primary.promises.sendStream,
-      getFileStream: getFileStreamWithFallback,
-      getRedirectUrl: primary.promises.getRedirectUrl,
-      getFileMd5Hash: _wrapFallbackMethod('getFileMd5Hash'),
-      deleteDirectory: _wrapMethodOnBothPersistors('deleteDirectory'),
-      getFileSize: _wrapFallbackMethod('getFileSize'),
-      deleteFile: _wrapMethodOnBothPersistors('deleteFile'),
-      copyFile: copyFileWithFallback,
-      checkIfFileExists: _wrapFallbackMethod('checkIfFileExists'),
-      directorySize: _wrapFallbackMethod('directorySize')
-    }
-  }
-}

+ 0 - 177
services/filestore/app/js/PersistorHelper.js

@@ -1,177 +0,0 @@
-const crypto = require('crypto')
-const Stream = require('stream')
-const logger = require('logger-sharelatex')
-const metrics = require('metrics-sharelatex')
-const { WriteError, ReadError, NotFoundError } = require('./Errors')
-const { promisify } = require('util')
-
-const pipeline = promisify(Stream.pipeline)
-
-// Observes data that passes through and computes some metadata for it
-// - specifically, it computes the number of bytes transferred, and optionally
-//   computes a cryptographic hash based on the 'hash' option. e.g., pass
-//   { hash: 'md5' } to compute the md5 hash of the stream
-// - if 'metric' is supplied as an option, this metric will be incremented by
-//   the number of bytes transferred
-class ObserverStream extends Stream.Transform {
-  constructor(options) {
-    options.autoDestroy = true
-    super(options)
-
-    this.bytes = 0
-
-    if (options.hash) {
-      this.hash = crypto.createHash(options.hash)
-    }
-
-    if (options.metric) {
-      const onEnd = () => {
-        metrics.count(options.metric, this.bytes)
-      }
-      this.once('error', onEnd)
-      this.once('end', onEnd)
-    }
-  }
-
-  _transform(chunk, encoding, done) {
-    if (this.hash) {
-      this.hash.update(chunk)
-    }
-    this.bytes += chunk.length
-    this.push(chunk)
-    done()
-  }
-
-  getHash() {
-    return this.hash && this.hash.digest('hex')
-  }
-}
-
-module.exports = {
-  ObserverStream,
-  calculateStreamMd5,
-  verifyMd5,
-  getReadyPipeline,
-  wrapError,
-  hexToBase64,
-  base64ToHex
-}
-
-// returns a promise which resolves with the md5 hash of the stream
-// - consumes the stream
-function calculateStreamMd5(stream) {
-  const hash = crypto.createHash('md5')
-  hash.setEncoding('hex')
-
-  return pipeline(stream, hash).then(() => hash.read())
-}
-
-// verifies the md5 hash of a file against the supplied md5 or the one stored in
-// storage if not supplied - deletes the new file if the md5 does not match and
-// throws an error
-async function verifyMd5(persistor, bucket, key, sourceMd5, destMd5 = null) {
-  if (!destMd5) {
-    destMd5 = await persistor.promises.getFileMd5Hash(bucket, key)
-  }
-
-  if (sourceMd5 !== destMd5) {
-    try {
-      await persistor.promises.deleteFile(bucket, key)
-    } catch (err) {
-      logger.warn(err, 'error deleting file for invalid upload')
-    }
-
-    throw new WriteError('source and destination hashes do not match', {
-      sourceMd5,
-      destMd5,
-      bucket,
-      key
-    })
-  }
-}
-
-// resolves when a stream is 'readable', or rejects if the stream throws an error
-// before that happens - this lets us handle protocol-level errors before trying
-// to read them
-function getReadyPipeline(...streams) {
-  return new Promise((resolve, reject) => {
-    const lastStream = streams.slice(-1)[0]
-
-    // in case of error or stream close, we must ensure that we drain the
-    // previous stream so that it can clean up its socket (if it has one)
-    const drainPreviousStream = function(previousStream) {
-      // this stream is no longer reliable, so don't pipe anything more into it
-      previousStream.unpipe(this)
-      previousStream.resume()
-    }
-
-    // handler to resolve when either:
-    // - an error happens, or
-    // - the last stream in the chain is readable
-    // for example, in the case of a 4xx error an error will occur and the
-    // streams will not become readable
-    const handler = function(err) {
-      // remove handler from all streams because we don't want to do this on
-      // later errors
-      lastStream.removeListener('readable', handler)
-      for (const stream of streams) {
-        stream.removeListener('error', handler)
-      }
-
-      // return control to the caller
-      if (err) {
-        reject(
-          wrapError(err, 'error before stream became ready', {}, ReadError)
-        )
-      } else {
-        resolve(lastStream)
-      }
-    }
-
-    // ensure the handler fires when the last strem becomes readable
-    lastStream.on('readable', handler)
-
-    for (const stream of streams) {
-      // when a stream receives a pipe, set up the drain handler to drain the
-      // connection if an error occurs or the stream is closed
-      stream.on('pipe', previousStream => {
-        stream.on('error', x => {
-          drainPreviousStream(previousStream)
-        })
-        stream.on('close', () => {
-          drainPreviousStream(previousStream)
-        })
-      })
-      // add the handler function to resolve this method on error if we can't
-      // set up the pipeline
-      stream.on('error', handler)
-    }
-
-    // begin the pipeline
-    for (let index = 0; index < streams.length - 1; index++) {
-      streams[index].pipe(streams[index + 1])
-    }
-  })
-}
-
-function wrapError(error, message, params, ErrorType) {
-  if (
-    error instanceof NotFoundError ||
-    ['NoSuchKey', 'NotFound', 404, 'AccessDenied', 'ENOENT'].includes(
-      error.code
-    ) ||
-    (error.response && error.response.statusCode === 404)
-  ) {
-    return new NotFoundError('no such file', params).withCause(error)
-  } else {
-    return new ErrorType(message, params).withCause(error)
-  }
-}
-
-function base64ToHex(base64) {
-  return Buffer.from(base64, 'base64').toString('hex')
-}
-
-function hexToBase64(hex) {
-  return Buffer.from(hex, 'hex').toString('base64')
-}

+ 5 - 34
services/filestore/app/js/PersistorManager.js

@@ -1,39 +1,10 @@
 const settings = require('settings-sharelatex')
-const logger = require('logger-sharelatex')
 
-logger.log(
-  {
-    backend: settings.filestore.backend,
-    fallback: settings.filestore.fallback && settings.filestore.fallback.backend
-  },
-  'Loading backend'
-)
-if (!settings.filestore.backend) {
-  throw new Error('no backend specified - config incomplete')
-}
+const persistorSettings = settings.filestore
+persistorSettings.Metrics = require('metrics-sharelatex')
+persistorSettings.paths = settings.path
 
-function getPersistor(backend) {
-  switch (backend) {
-    case 'aws-sdk':
-    case 's3':
-      return require('./S3Persistor')
-    case 'fs':
-      return require('./FSPersistor')
-    case 'gcs':
-      return require('./GcsPersistor')
-    default:
-      throw new Error(`unknown filestore backend: ${backend}`)
-  }
-}
-
-let persistor = getPersistor(settings.filestore.backend)
-
-if (settings.filestore.fallback && settings.filestore.fallback.backend) {
-  const migrationPersistor = require('./MigrationPersistor')
-  persistor = migrationPersistor(
-    persistor,
-    getPersistor(settings.filestore.fallback.backend)
-  )
-}
+const ObjectPersistor = require('@overleaf/object-persistor')
+const persistor = ObjectPersistor(persistorSettings)
 
 module.exports = persistor

+ 0 - 367
services/filestore/app/js/S3Persistor.js

@@ -1,367 +0,0 @@
-const http = require('http')
-const https = require('https')
-http.globalAgent.maxSockets = 300
-https.globalAgent.maxSockets = 300
-
-const settings = require('settings-sharelatex')
-
-const PersistorHelper = require('./PersistorHelper')
-
-const fs = require('fs')
-const S3 = require('aws-sdk/clients/s3')
-const { URL } = require('url')
-const Stream = require('stream')
-const { promisify, callbackify } = require('util')
-const {
-  WriteError,
-  ReadError,
-  NotFoundError,
-  SettingsError
-} = require('./Errors')
-const pipeline = promisify(Stream.pipeline)
-
-const S3Persistor = {
-  sendFile: callbackify(sendFile),
-  sendStream: callbackify(sendStream),
-  getFileStream: callbackify(getFileStream),
-  getRedirectUrl: callbackify(getRedirectUrl),
-  getFileMd5Hash: callbackify(getFileMd5Hash),
-  deleteDirectory: callbackify(deleteDirectory),
-  getFileSize: callbackify(getFileSize),
-  deleteFile: callbackify(deleteFile),
-  copyFile: callbackify(copyFile),
-  checkIfFileExists: callbackify(checkIfFileExists),
-  directorySize: callbackify(directorySize),
-  promises: {
-    sendFile,
-    sendStream,
-    getFileStream,
-    getRedirectUrl,
-    getFileMd5Hash,
-    deleteDirectory,
-    getFileSize,
-    deleteFile,
-    copyFile,
-    checkIfFileExists,
-    directorySize
-  }
-}
-
-module.exports = S3Persistor
-
-async function sendFile(bucketName, key, fsPath) {
-  return sendStream(bucketName, key, fs.createReadStream(fsPath))
-}
-
-async function sendStream(bucketName, key, readStream, sourceMd5) {
-  try {
-    // egress from us to S3
-    const observeOptions = { metric: 's3.egress' }
-    let b64Hash
-
-    if (sourceMd5) {
-      b64Hash = PersistorHelper.hexToBase64(sourceMd5)
-    } else {
-      // if there is no supplied md5 hash, we calculate the hash as the data passes through
-      observeOptions.hash = 'md5'
-    }
-
-    const observer = new PersistorHelper.ObserverStream(observeOptions)
-    pipeline(readStream, observer)
-
-    // if we have an md5 hash, pass this to S3 to verify the upload
-    const uploadOptions = {
-      Bucket: bucketName,
-      Key: key,
-      Body: observer
-    }
-    if (b64Hash) {
-      uploadOptions.ContentMD5 = b64Hash
-    }
-
-    const response = await _getClientForBucket(bucketName)
-      .upload(uploadOptions, { partSize: settings.filestore.s3.partSize })
-      .promise()
-    let destMd5 = _md5FromResponse(response)
-    if (!destMd5) {
-      // the eTag isn't in md5 format so we need to calculate it ourselves
-      const verifyStream = await getFileStream(
-        response.Bucket,
-        response.Key,
-        {}
-      )
-      destMd5 = await PersistorHelper.calculateStreamMd5(verifyStream)
-    }
-
-    // if we didn't have an md5 hash, we should compare our computed one with S3's
-    // as we couldn't tell S3 about it beforehand
-    if (!sourceMd5) {
-      sourceMd5 = observer.getHash()
-      // throws on mismatch
-      await PersistorHelper.verifyMd5(
-        S3Persistor,
-        bucketName,
-        key,
-        sourceMd5,
-        destMd5
-      )
-    }
-  } catch (err) {
-    throw PersistorHelper.wrapError(
-      err,
-      'upload to S3 failed',
-      { bucketName, key },
-      WriteError
-    )
-  }
-}
-
-async function getFileStream(bucketName, key, opts) {
-  opts = opts || {}
-
-  const params = {
-    Bucket: bucketName,
-    Key: key
-  }
-  if (opts.start != null && opts.end != null) {
-    params.Range = `bytes=${opts.start}-${opts.end}`
-  }
-
-  const stream = _getClientForBucket(bucketName)
-    .getObject(params)
-    .createReadStream()
-
-  // ingress from S3 to us
-  const observer = new PersistorHelper.ObserverStream({ metric: 's3.ingress' })
-
-  try {
-    // wait for the pipeline to be ready, to catch non-200s
-    await PersistorHelper.getReadyPipeline(stream, observer)
-    return observer
-  } catch (err) {
-    throw PersistorHelper.wrapError(
-      err,
-      'error reading file from S3',
-      { bucketName, key, opts },
-      ReadError
-    )
-  }
-}
-
-async function getRedirectUrl() {
-  // not implemented
-  return null
-}
-
-async function deleteDirectory(bucketName, key) {
-  let response
-
-  try {
-    response = await _getClientForBucket(bucketName)
-      .listObjects({ Bucket: bucketName, Prefix: key })
-      .promise()
-  } catch (err) {
-    throw PersistorHelper.wrapError(
-      err,
-      'failed to list objects in S3',
-      { bucketName, key },
-      ReadError
-    )
-  }
-
-  const objects = response.Contents.map(item => ({ Key: item.Key }))
-  if (objects.length) {
-    try {
-      await _getClientForBucket(bucketName)
-        .deleteObjects({
-          Bucket: bucketName,
-          Delete: {
-            Objects: objects,
-            Quiet: true
-          }
-        })
-        .promise()
-    } catch (err) {
-      throw PersistorHelper.wrapError(
-        err,
-        'failed to delete objects in S3',
-        { bucketName, key },
-        WriteError
-      )
-    }
-  }
-}
-
-async function getFileSize(bucketName, key) {
-  try {
-    const response = await _getClientForBucket(bucketName)
-      .headObject({ Bucket: bucketName, Key: key })
-      .promise()
-    return response.ContentLength
-  } catch (err) {
-    throw PersistorHelper.wrapError(
-      err,
-      'error getting size of s3 object',
-      { bucketName, key },
-      ReadError
-    )
-  }
-}
-
-async function getFileMd5Hash(bucketName, key) {
-  try {
-    const response = await _getClientForBucket(bucketName)
-      .headObject({ Bucket: bucketName, Key: key })
-      .promise()
-    return _md5FromResponse(response)
-  } catch (err) {
-    throw PersistorHelper.wrapError(
-      err,
-      'error getting hash of s3 object',
-      { bucketName, key },
-      ReadError
-    )
-  }
-}
-
-async function deleteFile(bucketName, key) {
-  try {
-    await _getClientForBucket(bucketName)
-      .deleteObject({ Bucket: bucketName, Key: key })
-      .promise()
-  } catch (err) {
-    // s3 does not give us a NotFoundError here
-    throw PersistorHelper.wrapError(
-      err,
-      'failed to delete file in S3',
-      { bucketName, key },
-      WriteError
-    )
-  }
-}
-
-async function copyFile(bucketName, sourceKey, destKey) {
-  const params = {
-    Bucket: bucketName,
-    Key: destKey,
-    CopySource: `${bucketName}/${sourceKey}`
-  }
-  try {
-    await _getClientForBucket(bucketName)
-      .copyObject(params)
-      .promise()
-  } catch (err) {
-    throw PersistorHelper.wrapError(
-      err,
-      'failed to copy file in S3',
-      params,
-      WriteError
-    )
-  }
-}
-
-async function checkIfFileExists(bucketName, key) {
-  try {
-    await getFileSize(bucketName, key)
-    return true
-  } catch (err) {
-    if (err instanceof NotFoundError) {
-      return false
-    }
-    throw PersistorHelper.wrapError(
-      err,
-      'error checking whether S3 object exists',
-      { bucketName, key },
-      ReadError
-    )
-  }
-}
-
-async function directorySize(bucketName, key) {
-  try {
-    const response = await _getClientForBucket(bucketName)
-      .listObjects({ Bucket: bucketName, Prefix: key })
-      .promise()
-
-    return response.Contents.reduce((acc, item) => item.Size + acc, 0)
-  } catch (err) {
-    throw PersistorHelper.wrapError(
-      err,
-      'error getting directory size in S3',
-      { bucketName, key },
-      ReadError
-    )
-  }
-}
-
-const _clients = new Map()
-let _defaultClient
-
-function _getClientForBucket(bucket) {
-  if (_clients[bucket]) {
-    return _clients[bucket]
-  }
-
-  if (
-    settings.filestore.s3BucketCreds &&
-    settings.filestore.s3BucketCreds[bucket]
-  ) {
-    _clients[bucket] = new S3(
-      _buildClientOptions(settings.filestore.s3BucketCreds[bucket])
-    )
-    return _clients[bucket]
-  }
-
-  // no specific credentials for the bucket
-  if (_defaultClient) {
-    return _defaultClient
-  }
-
-  if (settings.filestore.s3.key) {
-    _defaultClient = new S3(_buildClientOptions())
-    return _defaultClient
-  }
-
-  throw new SettingsError(
-    'no bucket-specific or default credentials provided',
-    { bucket }
-  )
-}
-
-function _buildClientOptions(bucketCredentials) {
-  const options = {}
-
-  if (bucketCredentials) {
-    options.credentials = {
-      accessKeyId: bucketCredentials.auth_key,
-      secretAccessKey: bucketCredentials.auth_secret
-    }
-  } else {
-    options.credentials = {
-      accessKeyId: settings.filestore.s3.key,
-      secretAccessKey: settings.filestore.s3.secret
-    }
-  }
-
-  if (settings.filestore.s3.endpoint) {
-    const endpoint = new URL(settings.filestore.s3.endpoint)
-    options.endpoint = settings.filestore.s3.endpoint
-    options.sslEnabled = endpoint.protocol === 'https'
-  }
-
-  // path-style access is only used for acceptance tests
-  if (settings.filestore.s3.pathStyle) {
-    options.s3ForcePathStyle = true
-  }
-
-  return options
-}
-
-function _md5FromResponse(response) {
-  const md5 = (response.ETag || '').replace(/[ "]/g, '')
-  if (!md5.match(/^[a-f0-9]{32}$/)) {
-    return null
-  }
-
-  return md5
-}

Fișier diff suprimat deoarece este prea mare
+ 412 - 209
services/filestore/package-lock.json


+ 18 - 17
services/filestore/package.json

@@ -13,7 +13,7 @@
     "test:unit": "npm run test:unit:_run -- --grep=$MOCHA_GREP",
     "start": "node $NODE_APP_OPTIONS app.js",
     "nodemon": "nodemon --config nodemon.json",
-    "lint": "node_modules/.bin/eslint .",
+    "lint": "node_modules/.bin/eslint app test *.js",
     "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",
@@ -22,45 +22,46 @@
   "dependencies": {
     "@google-cloud/storage": "^4.7.0",
     "@overleaf/o-error": "^3.0.0",
-    "aws-sdk": "^2.648.0",
+    "@overleaf/object-persistor": "git+https://github.com/overleaf/object-persistor.git",
+    "aws-sdk": "^2.710.0",
     "body-parser": "^1.19.0",
     "express": "^4.17.1",
     "fast-crc32c": "^2.0.0",
     "glob": "^7.1.6",
     "lodash.once": "^4.1.1",
-    "logger-sharelatex": "2.1.1",
-    "metrics-sharelatex": "^2.6.2",
+    "logger-sharelatex": "^2.1.1",
+    "metrics-sharelatex": "^2.7.0",
     "node-uuid": "~1.4.8",
     "range-parser": "^1.2.1",
     "request": "^2.88.2",
     "request-promise-native": "^1.0.8",
     "settings-sharelatex": "^1.1.0",
-    "stream-buffers": "~0.2.5",
+    "stream-buffers": "~0.2.6",
     "tiny-async-pool": "^1.1.0"
   },
   "devDependencies": {
     "babel-eslint": "^10.1.0",
-    "bunyan": "^1.8.12",
+    "bunyan": "^1.8.14",
     "chai": "4.2.0",
     "chai-as-promised": "^7.1.1",
     "disrequire": "^1.1.0",
     "eslint": "^6.8.0",
-    "eslint-config-prettier": "^6.10.0",
-    "eslint-config-standard": "^14.1.0",
+    "eslint-config-prettier": "^6.11.0",
+    "eslint-config-standard": "^14.1.1",
     "eslint-plugin-chai-expect": "^2.1.0",
-    "eslint-plugin-chai-friendly": "^0.5.0",
-    "eslint-plugin-import": "^2.20.1",
+    "eslint-plugin-chai-friendly": "^0.6.0",
+    "eslint-plugin-import": "^2.22.0",
     "eslint-plugin-mocha": "^6.3.0",
-    "eslint-plugin-node": "^11.0.0",
-    "eslint-plugin-prettier": "^3.1.2",
+    "eslint-plugin-node": "^11.1.0",
+    "eslint-plugin-prettier": "^3.1.4",
     "eslint-plugin-promise": "^4.2.1",
     "eslint-plugin-standard": "^4.0.1",
-    "mocha": "7.1.1",
-    "mongodb": "^3.5.5",
-    "prettier-eslint": "^9.0.1",
+    "mocha": "7.2.0",
+    "mongodb": "^3.5.9",
+    "prettier-eslint": "^9.0.2",
     "prettier-eslint-cli": "^5.0.0",
-    "sandboxed-module": "2.0.3",
-    "sinon": "9.0.1",
+    "sandboxed-module": "2.0.4",
+    "sinon": "9.0.2",
     "sinon-chai": "^3.5.0",
     "streamifier": "^0.1.1",
     "timekeeper": "^2.2.0"

+ 1 - 0
services/filestore/test/acceptance/js/FilestoreApp.js

@@ -114,6 +114,7 @@ class FilestoreApp {
       disrequire(Path.resolve(__dirname, '../../../app/js', file))
     })
     disrequire(Path.resolve(__dirname, '../../../app'))
+    disrequire('@overleaf/object-persistor')
 
     return require('../../../app')
   }

+ 4 - 4
services/filestore/test/acceptance/js/FilestoreTests.js

@@ -590,7 +590,7 @@ describe('Filestore', function() {
 
             describe('when copyOnMiss is disabled', function() {
               beforeEach(function() {
-                Settings.filestore.fallback.copyOnMiss = false
+                app.persistor.settings.copyOnMiss = false
               })
 
               it('should fetch the file', async function() {
@@ -611,7 +611,7 @@ describe('Filestore', function() {
 
             describe('when copyOnMiss is enabled', function() {
               beforeEach(function() {
-                Settings.filestore.fallback.copyOnMiss = true
+                app.persistor.settings.copyOnMiss = true
               })
 
               it('should fetch the file', async function() {
@@ -656,7 +656,7 @@ describe('Filestore', function() {
 
               describe('when copyOnMiss is false', function() {
                 beforeEach(async function() {
-                  Settings.filestore.fallback.copyOnMiss = false
+                  app.persistor.settings.copyOnMiss = false
 
                   const response = await rp(opts)
                   expect(response.statusCode).to.equal(200)
@@ -702,7 +702,7 @@ describe('Filestore', function() {
 
               describe('when copyOnMiss is true', function() {
                 beforeEach(async function() {
-                  Settings.filestore.fallback.copyOnMiss = true
+                  app.persistor.settings.copyOnMiss = true
 
                   const response = await rp(opts)
                   expect(response.statusCode).to.equal(200)

+ 2 - 2
services/filestore/test/acceptance/js/TestHelper.js

@@ -34,11 +34,11 @@ function streamToString(stream) {
 
 async function uploadStringToPersistor(persistor, bucket, key, content) {
   const fileStream = streamifier.createReadStream(content)
-  await persistor.promises.sendStream(bucket, key, fileStream)
+  await persistor.sendStream(bucket, key, fileStream)
 }
 
 async function getStringFromPersistor(persistor, bucket, key) {
-  const stream = await persistor.promises.getFileStream(bucket, key, {})
+  const stream = await persistor.getObjectStream(bucket, key, {})
   return streamToString(stream)
 }
 

+ 0 - 353
services/filestore/test/unit/js/FSPersistorTests.js

@@ -1,353 +0,0 @@
-const sinon = require('sinon')
-const chai = require('chai')
-const { expect } = chai
-const SandboxedModule = require('sandboxed-module')
-const Errors = require('../../../app/js/Errors')
-const StreamModule = require('stream')
-
-chai.use(require('sinon-chai'))
-chai.use(require('chai-as-promised'))
-
-const modulePath = '../../../app/js/FSPersistor.js'
-
-describe('FSPersistorTests', function() {
-  const stat = { size: 4, isFile: sinon.stub().returns(true) }
-  const fd = 1234
-  const writeStream = 'writeStream'
-  const remoteStream = 'remoteStream'
-  const tempFile = '/tmp/potato.txt'
-  const location = '/foo'
-  const error = new Error('guru meditation error')
-  const md5 = 'ffffffff'
-
-  const files = ['animals/wombat.tex', 'vegetables/potato.tex']
-  const globs = [`${location}/${files[0]}`, `${location}/${files[1]}`]
-  const filteredFilenames = ['animals_wombat.tex', 'vegetables_potato.tex']
-  let fs, stream, LocalFileWriter, FSPersistor, glob, readStream, crypto, Hash
-
-  beforeEach(function() {
-    readStream = {
-      name: 'readStream',
-      on: sinon.stub().yields(),
-      pipe: sinon.stub()
-    }
-    fs = {
-      createReadStream: sinon.stub().returns(readStream),
-      createWriteStream: sinon.stub().returns(writeStream),
-      unlink: sinon.stub().yields(),
-      open: sinon.stub().yields(null, fd),
-      stat: sinon.stub().yields(null, stat)
-    }
-    glob = sinon.stub().yields(null, globs)
-    stream = {
-      pipeline: sinon.stub().yields(),
-      Transform: StreamModule.Transform
-    }
-    LocalFileWriter = {
-      promises: {
-        writeStream: sinon.stub().resolves(tempFile),
-        deleteFile: sinon.stub().resolves()
-      }
-    }
-    Hash = {
-      end: sinon.stub(),
-      read: sinon.stub().returns(md5),
-      digest: sinon.stub().returns(md5),
-      setEncoding: sinon.stub()
-    }
-    crypto = {
-      createHash: sinon.stub().returns(Hash)
-    }
-    FSPersistor = SandboxedModule.require(modulePath, {
-      requires: {
-        './LocalFileWriter': LocalFileWriter,
-        './Errors': Errors,
-        fs,
-        glob,
-        stream,
-        crypto,
-        // imported by PersistorHelper but otherwise unused here
-        'logger-sharelatex': {},
-        'metrics-sharelatex': {}
-      },
-      globals: { console }
-    })
-  })
-
-  describe('sendFile', function() {
-    const localFilesystemPath = '/path/to/local/file'
-    it('should copy the file', async function() {
-      await FSPersistor.promises.sendFile(
-        location,
-        files[0],
-        localFilesystemPath
-      )
-      expect(fs.createReadStream).to.have.been.calledWith(localFilesystemPath)
-      expect(fs.createWriteStream).to.have.been.calledWith(
-        `${location}/${filteredFilenames[0]}`
-      )
-      expect(stream.pipeline).to.have.been.calledWith(readStream, writeStream)
-    })
-
-    it('should return an error if the file cannot be stored', async function() {
-      stream.pipeline.yields(error)
-      await expect(
-        FSPersistor.promises.sendFile(location, files[0], localFilesystemPath)
-      ).to.eventually.be.rejected.and.have.property('cause', error)
-    })
-  })
-
-  describe('sendStream', function() {
-    it('should send the stream to LocalFileWriter', async function() {
-      await FSPersistor.promises.sendStream(location, files[0], remoteStream)
-      expect(LocalFileWriter.promises.writeStream).to.have.been.calledWith(
-        remoteStream
-      )
-    })
-
-    it('should delete the temporary file', async function() {
-      await FSPersistor.promises.sendStream(location, files[0], remoteStream)
-      expect(LocalFileWriter.promises.deleteFile).to.have.been.calledWith(
-        tempFile
-      )
-    })
-
-    it('should return the error from LocalFileWriter', async function() {
-      LocalFileWriter.promises.writeStream.rejects(error)
-      await expect(
-        FSPersistor.promises.sendStream(location, files[0], remoteStream)
-      ).to.eventually.be.rejectedWith(error)
-    })
-
-    it('should send the temporary file to the filestore', async function() {
-      await FSPersistor.promises.sendStream(location, files[0], remoteStream)
-      expect(fs.createReadStream).to.have.been.calledWith(tempFile)
-    })
-
-    describe('when the md5 hash does not match', function() {
-      it('should return a write error', async function() {
-        await expect(
-          FSPersistor.promises.sendStream(
-            location,
-            files[0],
-            remoteStream,
-            '00000000'
-          )
-        )
-          .to.eventually.be.rejected.and.be.an.instanceOf(Errors.WriteError)
-          .and.have.property('message', 'md5 hash mismatch')
-      })
-
-      it('deletes the copied file', async function() {
-        try {
-          await FSPersistor.promises.sendStream(
-            location,
-            files[0],
-            remoteStream,
-            '00000000'
-          )
-        } catch (_) {}
-        expect(LocalFileWriter.promises.deleteFile).to.have.been.calledWith(
-          `${location}/${filteredFilenames[0]}`
-        )
-      })
-    })
-  })
-
-  describe('getFileStream', function() {
-    it('should use correct file location', async function() {
-      await FSPersistor.promises.getFileStream(location, files[0], {})
-      expect(fs.open).to.have.been.calledWith(
-        `${location}/${filteredFilenames[0]}`
-      )
-    })
-
-    it('should pass the options to createReadStream', async function() {
-      await FSPersistor.promises.getFileStream(location, files[0], {
-        start: 0,
-        end: 8
-      })
-      expect(fs.createReadStream).to.have.been.calledWith(null, {
-        start: 0,
-        end: 8,
-        fd
-      })
-    })
-
-    it('should give a NotFoundError if the file does not exist', async function() {
-      const err = new Error()
-      err.code = 'ENOENT'
-      fs.open.yields(err)
-
-      await expect(FSPersistor.promises.getFileStream(location, files[0], {}))
-        .to.eventually.be.rejected.and.be.an.instanceOf(Errors.NotFoundError)
-        .and.have.property('cause', err)
-    })
-
-    it('should wrap any other error', async function() {
-      fs.open.yields(error)
-      await expect(FSPersistor.promises.getFileStream(location, files[0], {}))
-        .to.eventually.be.rejectedWith('failed to open file for streaming')
-        .and.be.an.instanceOf(Errors.ReadError)
-        .and.have.property('cause', error)
-    })
-  })
-
-  describe('getFileSize', function() {
-    const badFilename = 'neenaw.tex'
-    const size = 65536
-    const noentError = new Error('not found')
-    noentError.code = 'ENOENT'
-
-    beforeEach(function() {
-      fs.stat
-        .yields(error)
-        .withArgs(`${location}/${filteredFilenames[0]}`)
-        .yields(null, { size })
-        .withArgs(`${location}/${badFilename}`)
-        .yields(noentError)
-    })
-
-    it('should return the file size', async function() {
-      expect(
-        await FSPersistor.promises.getFileSize(location, files[0])
-      ).to.equal(size)
-    })
-
-    it('should throw a NotFoundError if the file does not exist', async function() {
-      await expect(
-        FSPersistor.promises.getFileSize(location, badFilename)
-      ).to.eventually.be.rejected.and.be.an.instanceOf(Errors.NotFoundError)
-    })
-
-    it('should wrap any other error', async function() {
-      await expect(FSPersistor.promises.getFileSize(location, 'raccoon'))
-        .to.eventually.be.rejected.and.be.an.instanceOf(Errors.ReadError)
-        .and.have.property('cause', error)
-    })
-  })
-
-  describe('copyFile', function() {
-    it('Should open the source for reading', async function() {
-      await FSPersistor.promises.copyFile(location, files[0], files[1])
-      expect(fs.createReadStream).to.have.been.calledWith(
-        `${location}/${filteredFilenames[0]}`
-      )
-    })
-
-    it('Should open the target for writing', async function() {
-      await FSPersistor.promises.copyFile(location, files[0], files[1])
-      expect(fs.createWriteStream).to.have.been.calledWith(
-        `${location}/${filteredFilenames[1]}`
-      )
-    })
-
-    it('Should pipe the source to the target', async function() {
-      await FSPersistor.promises.copyFile(location, files[0], files[1])
-      expect(stream.pipeline).to.have.been.calledWith(readStream, writeStream)
-    })
-  })
-
-  describe('deleteFile', function() {
-    it('Should call unlink with correct options', async function() {
-      await FSPersistor.promises.deleteFile(location, files[0])
-      expect(fs.unlink).to.have.been.calledWith(
-        `${location}/${filteredFilenames[0]}`
-      )
-    })
-
-    it('Should propagate the error', async function() {
-      fs.unlink.yields(error)
-      await expect(
-        FSPersistor.promises.deleteFile(location, files[0])
-      ).to.eventually.be.rejected.and.have.property('cause', error)
-    })
-  })
-
-  describe('deleteDirectory', function() {
-    it('Should call glob with correct options', async function() {
-      await FSPersistor.promises.deleteDirectory(location, files[0])
-      expect(glob).to.have.been.calledWith(
-        `${location}/${filteredFilenames[0]}*`
-      )
-    })
-
-    it('Should call unlink on the returned files', async function() {
-      await FSPersistor.promises.deleteDirectory(location, files[0])
-      for (const filename of globs) {
-        expect(fs.unlink).to.have.been.calledWith(filename)
-      }
-    })
-
-    it('Should propagate the error', async function() {
-      glob.yields(error)
-      await expect(
-        FSPersistor.promises.deleteDirectory(location, files[0])
-      ).to.eventually.be.rejected.and.have.property('cause', error)
-    })
-  })
-
-  describe('checkIfFileExists', function() {
-    const badFilename = 'pototo'
-    const noentError = new Error('not found')
-    noentError.code = 'ENOENT'
-
-    beforeEach(function() {
-      fs.stat
-        .yields(error)
-        .withArgs(`${location}/${filteredFilenames[0]}`)
-        .yields(null, {})
-        .withArgs(`${location}/${badFilename}`)
-        .yields(noentError)
-    })
-
-    it('Should call stat with correct options', async function() {
-      await FSPersistor.promises.checkIfFileExists(location, files[0])
-      expect(fs.stat).to.have.been.calledWith(
-        `${location}/${filteredFilenames[0]}`
-      )
-    })
-
-    it('Should return true for existing files', async function() {
-      expect(
-        await FSPersistor.promises.checkIfFileExists(location, files[0])
-      ).to.equal(true)
-    })
-
-    it('Should return false for non-existing files', async function() {
-      expect(
-        await FSPersistor.promises.checkIfFileExists(location, badFilename)
-      ).to.equal(false)
-    })
-
-    it('should wrap the error if there is a problem', async function() {
-      await expect(FSPersistor.promises.checkIfFileExists(location, 'llama'))
-        .to.eventually.be.rejected.and.be.an.instanceOf(Errors.ReadError)
-        .and.have.property('cause', error)
-    })
-  })
-
-  describe('directorySize', function() {
-    it('should wrap the error', async function() {
-      glob.yields(error)
-      await expect(FSPersistor.promises.directorySize(location, files[0]))
-        .to.eventually.be.rejected.and.be.an.instanceOf(Errors.ReadError)
-        .and.include({ cause: error })
-        .and.have.property('info')
-        .which.includes({ location, name: files[0] })
-    })
-
-    it('should filter the directory name', async function() {
-      await FSPersistor.promises.directorySize(location, files[0])
-      expect(glob).to.have.been.calledWith(
-        `${location}/${filteredFilenames[0]}_*`
-      )
-    })
-
-    it('should sum directory files size', async function() {
-      expect(
-        await FSPersistor.promises.directorySize(location, files[0])
-      ).to.equal(stat.size * files.length)
-    })
-  })
-})

+ 15 - 9
services/filestore/test/unit/js/FileControllerTests.js

@@ -32,8 +32,8 @@ describe('FileController', function() {
   beforeEach(function() {
     PersistorManager = {
       sendStream: sinon.stub().yields(),
-      copyFile: sinon.stub().yields(),
-      deleteFile: sinon.stub().yields()
+      copyObject: sinon.stub().resolves(),
+      deleteObject: sinon.stub().yields()
     }
 
     FileHandler = {
@@ -216,7 +216,9 @@ describe('FileController', function() {
     })
 
     it('should return a 404 is the file is not found', function(done) {
-      FileHandler.getFileSize.yields(new Errors.NotFoundError())
+      FileHandler.getFileSize.yields(
+        new Errors.NotFoundError({ message: 'not found', info: {} })
+      )
 
       res.sendStatus = code => {
         expect(code).to.equal(404)
@@ -262,7 +264,7 @@ describe('FileController', function() {
     it('should send bucket name and both keys to PersistorManager', function(done) {
       res.sendStatus = code => {
         code.should.equal(200)
-        expect(PersistorManager.copyFile).to.have.been.calledWith(
+        expect(PersistorManager.copyObject).to.have.been.calledWith(
           bucket,
           oldKey,
           key
@@ -273,7 +275,9 @@ describe('FileController', function() {
     })
 
     it('should send a 404 if the original file was not found', function(done) {
-      PersistorManager.copyFile.yields(new Errors.NotFoundError())
+      PersistorManager.copyObject.rejects(
+        new Errors.NotFoundError({ message: 'not found', info: {} })
+      )
       res.sendStatus = code => {
         code.should.equal(404)
         done()
@@ -281,10 +285,12 @@ describe('FileController', function() {
       FileController.copyFile(req, res, next)
     })
 
-    it('should send an error if there was an error', function() {
-      PersistorManager.copyFile.yields(error)
-      FileController.copyFile(req, res, next)
-      expect(next).to.have.been.calledWith(error)
+    it('should send an error if there was an error', function(done) {
+      PersistorManager.copyObject.rejects(error)
+      FileController.copyFile(req, res, err => {
+        expect(err).to.equal(error)
+        done()
+      })
     })
   })
 

+ 5 - 1
services/filestore/test/unit/js/FileConverterTests.js

@@ -2,6 +2,7 @@ const sinon = require('sinon')
 const chai = require('chai')
 const { expect } = chai
 const SandboxedModule = require('sandboxed-module')
+const { Errors } = require('@overleaf/object-persistor')
 
 const modulePath = '../../../app/js/FileConverter.js'
 
@@ -22,6 +23,8 @@ describe('FileConverter', function() {
       promises: sinon.stub().resolves(destPath)
     }
 
+    const ObjectPersistor = { Errors }
+
     FileConverter = SandboxedModule.require(modulePath, {
       requires: {
         './SafeExec': SafeExec,
@@ -29,7 +32,8 @@ describe('FileConverter', function() {
           inc: sinon.stub(),
           Timer: sinon.stub().returns({ done: sinon.stub() })
         },
-        'settings-sharelatex': Settings
+        'settings-sharelatex': Settings,
+        '@overleaf/object-persistor': ObjectPersistor
       }
     })
   })

+ 44 - 40
services/filestore/test/unit/js/FileHandlerTests.js

@@ -4,6 +4,7 @@ const { expect } = chai
 const modulePath = '../../../app/js/FileHandler.js'
 const SandboxedModule = require('sandboxed-module')
 const { ObjectId } = require('mongodb')
+const { Errors } = require('@overleaf/object-persistor')
 
 chai.use(require('sinon-chai'))
 chai.use(require('chai-as-promised'))
@@ -32,17 +33,15 @@ describe('FileHandler', function() {
 
   beforeEach(function() {
     PersistorManager = {
-      promises: {
-        getFileStream: sinon.stub().resolves(sourceStream),
-        getRedirectUrl: sinon.stub().resolves(redirectUrl),
-        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()
-      }
+      getObjectStream: sinon.stub().resolves(sourceStream),
+      getRedirectUrl: sinon.stub().resolves(redirectUrl),
+      checkIfObjectExists: sinon.stub().resolves(),
+      deleteObject: sinon.stub().resolves(),
+      deleteDirectory: sinon.stub().resolves(),
+      sendStream: sinon.stub().resolves(),
+      insertFile: sinon.stub().resolves(),
+      sendFile: sinon.stub().resolves(),
+      directorySize: sinon.stub().resolves()
     }
     LocalFileWriter = {
       // the callback style is used for detached cleanup calls
@@ -73,6 +72,8 @@ describe('FileHandler', function() {
       createReadStream: sinon.stub().returns(readStream)
     }
 
+    const ObjectPersistor = { Errors }
+
     FileHandler = SandboxedModule.require(modulePath, {
       requires: {
         './PersistorManager': PersistorManager,
@@ -81,6 +82,7 @@ describe('FileHandler', function() {
         './KeyBuilder': KeyBuilder,
         './ImageOptimiser': ImageOptimiser,
         'settings-sharelatex': Settings,
+        '@overleaf/object-persistor': ObjectPersistor,
         fs: fs
       },
       globals: { console }
@@ -93,7 +95,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.promises.sendStream).to.have.been.calledWith(
+        expect(PersistorManager.sendStream).to.have.been.calledWith(
           bucket,
           key,
           stream
@@ -105,8 +107,7 @@ describe('FileHandler', function() {
     it('should not make a delete request for the convertedKey folder', function(done) {
       FileHandler.insertFile(bucket, key, stream, err => {
         expect(err).not.to.exist
-        expect(PersistorManager.promises.deleteDirectory).not.to.have.been
-          .called
+        expect(PersistorManager.deleteDirectory).not.to.have.been.called
         done()
       })
     })
@@ -137,9 +138,10 @@ describe('FileHandler', function() {
       it('should delete the convertedKey folder', function(done) {
         FileHandler.insertFile(bucket, key, stream, err => {
           expect(err).not.to.exist
-          expect(
-            PersistorManager.promises.deleteDirectory
-          ).to.have.been.calledWith(bucket, convertedFolderKey)
+          expect(PersistorManager.deleteDirectory).to.have.been.calledWith(
+            bucket,
+            convertedFolderKey
+          )
           done()
         })
       })
@@ -150,7 +152,7 @@ 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.promises.deleteFile).to.have.been.calledWith(
+        expect(PersistorManager.deleteObject).to.have.been.calledWith(
           bucket,
           key
         )
@@ -161,8 +163,7 @@ describe('FileHandler', function() {
     it('should not tell the filestore manager to delete the cached folder', function(done) {
       FileHandler.deleteFile(bucket, key, err => {
         expect(err).not.to.exist
-        expect(PersistorManager.promises.deleteDirectory).not.to.have.been
-          .called
+        expect(PersistorManager.deleteDirectory).not.to.have.been.called
         done()
       })
     })
@@ -193,9 +194,10 @@ describe('FileHandler', function() {
       it('should delete the convertedKey folder', function(done) {
         FileHandler.deleteFile(bucket, key, err => {
           expect(err).not.to.exist
-          expect(
-            PersistorManager.promises.deleteDirectory
-          ).to.have.been.calledWith(bucket, convertedFolderKey)
+          expect(PersistorManager.deleteDirectory).to.have.been.calledWith(
+            bucket,
+            convertedFolderKey
+          )
           done()
         })
       })
@@ -206,9 +208,10 @@ describe('FileHandler', function() {
     it('should tell the filestore manager to delete the folder', function(done) {
       FileHandler.deleteProject(bucket, projectKey, err => {
         expect(err).not.to.exist
-        expect(
-          PersistorManager.promises.deleteDirectory
-        ).to.have.been.calledWith(bucket, projectKey)
+        expect(PersistorManager.deleteDirectory).to.have.been.calledWith(
+          bucket,
+          projectKey
+        )
         done()
       })
     })
@@ -234,7 +237,7 @@ describe('FileHandler', function() {
       const options = { start: 0, end: 8 }
       FileHandler.getFile(bucket, key, options, err => {
         expect(err).not.to.exist
-        expect(PersistorManager.promises.getFileStream).to.have.been.calledWith(
+        expect(PersistorManager.getObjectStream).to.have.been.calledWith(
           bucket,
           key,
           options
@@ -265,17 +268,16 @@ describe('FileHandler', function() {
         it('should return the the converted stream', function() {
           expect(result.err).not.to.exist
           expect(result.stream).to.equal(readStream)
-          expect(
-            PersistorManager.promises.getFileStream
-          ).to.have.been.calledWith(bucket, key)
+          expect(PersistorManager.getObjectStream).to.have.been.calledWith(
+            bucket,
+            key
+          )
         })
       })
 
       describe('when the file is cached', function() {
         beforeEach(function(done) {
-          PersistorManager.promises.checkIfFileExists = sinon
-            .stub()
-            .resolves(true)
+          PersistorManager.checkIfObjectExists = sinon.stub().resolves(true)
           FileHandler.getFile(bucket, key, { format: 'png' }, (err, stream) => {
             result = { err, stream }
             done()
@@ -293,9 +295,10 @@ describe('FileHandler', function() {
         it('should return the cached stream', function() {
           expect(result.err).not.to.exist
           expect(result.stream).to.equal(sourceStream)
-          expect(
-            PersistorManager.promises.getFileStream
-          ).to.have.been.calledWith(bucket, convertedKey)
+          expect(PersistorManager.getObjectStream).to.have.been.calledWith(
+            bucket,
+            convertedKey
+          )
         })
       })
     })
@@ -341,9 +344,10 @@ describe('FileHandler', function() {
 
     it('should call the persistor to get a redirect url', function(done) {
       FileHandler.getRedirectUrl(bucket, key, () => {
-        expect(
-          PersistorManager.promises.getRedirectUrl
-        ).to.have.been.calledWith(bucket, key)
+        expect(PersistorManager.getRedirectUrl).to.have.been.calledWith(
+          bucket,
+          key
+        )
         done()
       })
     })
@@ -383,7 +387,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.promises.directorySize).to.have.been.calledWith(
+        expect(PersistorManager.directorySize).to.have.been.calledWith(
           bucket,
           key
         )

+ 0 - 683
services/filestore/test/unit/js/GcsPersistorTests.js

@@ -1,683 +0,0 @@
-const sinon = require('sinon')
-const chai = require('chai')
-const { expect } = chai
-const modulePath = '../../../app/js/GcsPersistor.js'
-const SandboxedModule = require('sandboxed-module')
-const { ObjectId } = require('mongodb')
-const asyncPool = require('tiny-async-pool')
-
-const Errors = require('../../../app/js/Errors')
-
-describe('GcsPersistorTests', function() {
-  const filename = '/wombat/potato.tex'
-  const bucket = 'womBucket'
-  const key = 'monKey'
-  const destKey = 'donKey'
-  const genericError = new Error('guru meditation error')
-  const filesSize = 33
-  const md5 = 'ffffffff00000000ffffffff00000000'
-  const WriteStream = 'writeStream'
-  const redirectUrl = 'https://wombat.potato/giraffe'
-
-  let Metrics,
-    Logger,
-    Transform,
-    Storage,
-    Fs,
-    GcsNotFoundError,
-    ReadStream,
-    Stream,
-    GcsBucket,
-    GcsFile,
-    GcsPersistor,
-    FileNotFoundError,
-    Hash,
-    settings,
-    crypto,
-    files
-
-  beforeEach(function() {
-    settings = {
-      filestore: {
-        backend: 'gcs',
-        stores: {
-          user_files: 'user_files'
-        },
-        gcs: {
-          directoryKeyRegex: /^[0-9a-fA-F]{24}\/[0-9a-fA-F]{24}/
-        }
-      }
-    }
-
-    files = [
-      {
-        metadata: { size: 11, md5Hash: '/////wAAAAD/////AAAAAA==' },
-        delete: sinon.stub()
-      },
-      {
-        metadata: { size: 22, md5Hash: '/////wAAAAD/////AAAAAA==' },
-        delete: sinon.stub()
-      }
-    ]
-
-    ReadStream = {
-      pipe: sinon.stub().returns('readStream'),
-      on: sinon.stub(),
-      removeListener: sinon.stub()
-    }
-    ReadStream.on.withArgs('end').yields()
-    ReadStream.on.withArgs('pipe').yields({
-      unpipe: sinon.stub(),
-      resume: sinon.stub(),
-      on: sinon.stub()
-    })
-
-    Transform = class {
-      on(event, callback) {
-        if (event === 'readable') {
-          callback()
-        }
-      }
-
-      once() {}
-      removeListener() {}
-    }
-
-    Stream = {
-      pipeline: sinon.stub().yields(),
-      Transform: Transform
-    }
-
-    Metrics = {
-      count: sinon.stub()
-    }
-
-    GcsFile = {
-      delete: sinon.stub().resolves(),
-      createReadStream: sinon.stub().returns(ReadStream),
-      getMetadata: sinon.stub().resolves([files[0].metadata]),
-      createWriteStream: sinon.stub().returns(WriteStream),
-      copy: sinon.stub().resolves(),
-      exists: sinon.stub().resolves([true]),
-      getSignedUrl: sinon.stub().resolves([redirectUrl])
-    }
-
-    GcsBucket = {
-      file: sinon.stub().returns(GcsFile),
-      getFiles: sinon.stub().resolves([files])
-    }
-
-    Storage = class {
-      constructor() {
-        this.interceptors = []
-      }
-    }
-    Storage.prototype.bucket = sinon.stub().returns(GcsBucket)
-
-    GcsNotFoundError = new Error('File not found')
-    GcsNotFoundError.code = 404
-
-    Fs = {
-      createReadStream: sinon.stub().returns(ReadStream)
-    }
-
-    FileNotFoundError = new Error('File not found')
-    FileNotFoundError.code = 'ENOENT'
-
-    Hash = {
-      end: sinon.stub(),
-      read: sinon.stub().returns(md5),
-      digest: sinon.stub().returns(md5),
-      setEncoding: sinon.stub()
-    }
-    crypto = {
-      createHash: sinon.stub().returns(Hash)
-    }
-
-    Logger = {
-      warn: sinon.stub()
-    }
-
-    GcsPersistor = SandboxedModule.require(modulePath, {
-      requires: {
-        '@google-cloud/storage': { Storage },
-        'settings-sharelatex': settings,
-        'logger-sharelatex': Logger,
-        'tiny-async-pool': asyncPool,
-        './Errors': Errors,
-        fs: Fs,
-        stream: Stream,
-        'metrics-sharelatex': Metrics,
-        crypto
-      },
-      globals: { console, Buffer }
-    })
-  })
-
-  describe('getFileStream', function() {
-    describe('when called with valid parameters', function() {
-      let stream
-
-      beforeEach(async function() {
-        stream = await GcsPersistor.promises.getFileStream(bucket, key)
-      })
-
-      it('returns a metered stream', function() {
-        expect(stream).to.be.instanceOf(Transform)
-      })
-
-      it('fetches the right key from the right bucket', function() {
-        expect(Storage.prototype.bucket).to.have.been.calledWith(bucket)
-        expect(GcsBucket.file).to.have.been.calledWith(key)
-        expect(GcsFile.createReadStream).to.have.been.called
-      })
-
-      it('pipes the stream through the meter', function() {
-        expect(ReadStream.pipe).to.have.been.calledWith(
-          sinon.match.instanceOf(Transform)
-        )
-      })
-    })
-
-    describe('when called with a byte range', function() {
-      let stream
-
-      beforeEach(async function() {
-        stream = await GcsPersistor.promises.getFileStream(bucket, key, {
-          start: 5,
-          end: 10
-        })
-      })
-
-      it('returns a metered stream', function() {
-        expect(stream).to.be.instanceOf(Transform)
-      })
-
-      it('passes the byte range on to GCS', function() {
-        expect(GcsFile.createReadStream).to.have.been.calledWith({
-          start: 5,
-          end: 10
-        })
-      })
-    })
-
-    describe("when the file doesn't exist", function() {
-      let error, stream
-
-      beforeEach(async function() {
-        Transform.prototype.on = sinon.stub()
-        ReadStream.on.withArgs('error').yields(GcsNotFoundError)
-        try {
-          stream = await GcsPersistor.promises.getFileStream(bucket, key)
-        } catch (e) {
-          error = e
-        }
-      })
-
-      it('does not return a stream', function() {
-        expect(stream).not.to.exist
-      })
-
-      it('throws a NotFoundError', function() {
-        expect(error).to.be.an.instanceOf(Errors.NotFoundError)
-      })
-
-      it('wraps the error', function() {
-        expect(error.cause).to.exist
-      })
-
-      it('stores the bucket and key in the error', function() {
-        expect(error.info).to.include({ bucketName: bucket, key: key })
-      })
-    })
-
-    describe('when Gcs encounters an unkown error', function() {
-      let error, stream
-
-      beforeEach(async function() {
-        Transform.prototype.on = sinon.stub()
-        ReadStream.on.withArgs('error').yields(genericError)
-        try {
-          stream = await GcsPersistor.promises.getFileStream(bucket, key)
-        } catch (err) {
-          error = err
-        }
-      })
-
-      it('does not return a stream', function() {
-        expect(stream).not.to.exist
-      })
-
-      it('throws a ReadError', function() {
-        expect(error).to.be.an.instanceOf(Errors.ReadError)
-      })
-
-      it('wraps the error', function() {
-        expect(error.cause).to.exist
-      })
-
-      it('stores the bucket and key in the error', function() {
-        expect(error.info).to.include({ bucketName: bucket, key: key })
-      })
-    })
-  })
-
-  describe('getFile', function() {
-    let signedUrl
-
-    beforeEach(async function() {
-      signedUrl = await GcsPersistor.promises.getRedirectUrl(bucket, key)
-    })
-
-    it('should request a signed URL', function() {
-      expect(GcsFile.getSignedUrl).to.have.been.called
-    })
-
-    it('should return the url', function() {
-      expect(signedUrl).to.equal(redirectUrl)
-    })
-  })
-
-  describe('getFileSize', function() {
-    describe('when called with valid parameters', function() {
-      let size
-
-      beforeEach(async function() {
-        size = await GcsPersistor.promises.getFileSize(bucket, key)
-      })
-
-      it('should return the object size', function() {
-        expect(size).to.equal(files[0].metadata.size)
-      })
-
-      it('should pass the bucket and key to GCS', function() {
-        expect(Storage.prototype.bucket).to.have.been.calledWith(bucket)
-        expect(GcsBucket.file).to.have.been.calledWith(key)
-        expect(GcsFile.getMetadata).to.have.been.called
-      })
-    })
-
-    describe('when the object is not found', function() {
-      let error
-
-      beforeEach(async function() {
-        GcsFile.getMetadata = sinon.stub().rejects(GcsNotFoundError)
-        try {
-          await GcsPersistor.promises.getFileSize(bucket, key)
-        } catch (err) {
-          error = err
-        }
-      })
-
-      it('should return a NotFoundError', function() {
-        expect(error).to.be.an.instanceOf(Errors.NotFoundError)
-      })
-
-      it('should wrap the error', function() {
-        expect(error.cause).to.equal(GcsNotFoundError)
-      })
-    })
-
-    describe('when GCS returns an error', function() {
-      let error
-
-      beforeEach(async function() {
-        GcsFile.getMetadata = sinon.stub().rejects(genericError)
-        try {
-          await GcsPersistor.promises.getFileSize(bucket, key)
-        } catch (err) {
-          error = err
-        }
-      })
-
-      it('should return a ReadError', function() {
-        expect(error).to.be.an.instanceOf(Errors.ReadError)
-      })
-
-      it('should wrap the error', function() {
-        expect(error.cause).to.equal(genericError)
-      })
-    })
-  })
-
-  describe('sendStream', function() {
-    describe('with valid parameters', function() {
-      beforeEach(async function() {
-        return GcsPersistor.promises.sendStream(bucket, key, ReadStream)
-      })
-
-      it('should upload the stream', function() {
-        expect(Storage.prototype.bucket).to.have.been.calledWith(bucket)
-        expect(GcsBucket.file).to.have.been.calledWith(key)
-        expect(GcsFile.createWriteStream).to.have.been.called
-      })
-
-      it('should not try to create a resumable upload', function() {
-        expect(GcsFile.createWriteStream).to.have.been.calledWith({
-          resumable: false
-        })
-      })
-
-      it('should meter the stream and pass it to GCS', function() {
-        expect(Stream.pipeline).to.have.been.calledWith(
-          ReadStream,
-          sinon.match.instanceOf(Transform),
-          WriteStream
-        )
-      })
-
-      it('calculates the md5 hash of the file', function() {
-        expect(Hash.digest).to.have.been.called
-      })
-    })
-
-    describe('when a hash is supplied', function() {
-      beforeEach(async function() {
-        return GcsPersistor.promises.sendStream(
-          bucket,
-          key,
-          ReadStream,
-          'aaaaaaaabbbbbbbbaaaaaaaabbbbbbbb'
-        )
-      })
-
-      it('should not calculate the md5 hash of the file', function() {
-        expect(Hash.digest).not.to.have.been.called
-      })
-
-      it('sends the hash in base64', function() {
-        expect(GcsFile.createWriteStream).to.have.been.calledWith({
-          validation: 'md5',
-          metadata: {
-            md5Hash: 'qqqqqru7u7uqqqqqu7u7uw=='
-          },
-          resumable: false
-        })
-      })
-
-      it('does not fetch the md5 hash of the uploaded file', function() {
-        expect(GcsFile.getMetadata).not.to.have.been.called
-      })
-    })
-
-    describe('when the upload fails', function() {
-      let error
-      beforeEach(async function() {
-        Stream.pipeline
-          .withArgs(
-            ReadStream,
-            sinon.match.instanceOf(Transform),
-            WriteStream,
-            sinon.match.any
-          )
-          .yields(genericError)
-        try {
-          await GcsPersistor.promises.sendStream(bucket, key, ReadStream)
-        } catch (err) {
-          error = err
-        }
-      })
-
-      it('throws a WriteError', function() {
-        expect(error).to.be.an.instanceOf(Errors.WriteError)
-      })
-
-      it('wraps the error', function() {
-        expect(error.cause).to.equal(genericError)
-      })
-    })
-  })
-
-  describe('sendFile', function() {
-    describe('with valid parameters', function() {
-      beforeEach(async function() {
-        return GcsPersistor.promises.sendFile(bucket, key, filename)
-      })
-
-      it('should create a read stream for the file', function() {
-        expect(Fs.createReadStream).to.have.been.calledWith(filename)
-      })
-
-      it('should create a write stream', function() {
-        expect(Storage.prototype.bucket).to.have.been.calledWith(bucket)
-        expect(GcsBucket.file).to.have.been.calledWith(key)
-        expect(GcsFile.createWriteStream).to.have.been.called
-      })
-
-      it('should upload the stream via the meter', function() {
-        expect(Stream.pipeline).to.have.been.calledWith(
-          ReadStream,
-          sinon.match.instanceOf(Transform),
-          WriteStream
-        )
-      })
-    })
-  })
-
-  describe('copyFile', function() {
-    const destinationFile = 'destFile'
-
-    beforeEach(function() {
-      GcsBucket.file.withArgs(destKey).returns(destinationFile)
-    })
-
-    describe('with valid parameters', function() {
-      beforeEach(async function() {
-        return GcsPersistor.promises.copyFile(bucket, key, destKey)
-      })
-
-      it('should copy the object', function() {
-        expect(Storage.prototype.bucket).to.have.been.calledWith(bucket)
-        expect(GcsBucket.file).to.have.been.calledWith(key)
-        expect(GcsFile.copy).to.have.been.calledWith(destinationFile)
-      })
-    })
-
-    describe('when the file does not exist', function() {
-      let error
-
-      beforeEach(async function() {
-        GcsFile.copy = sinon.stub().rejects(GcsNotFoundError)
-        try {
-          await GcsPersistor.promises.copyFile(bucket, key, destKey)
-        } catch (err) {
-          error = err
-        }
-      })
-
-      it('should throw a NotFoundError', function() {
-        expect(error).to.be.an.instanceOf(Errors.NotFoundError)
-      })
-    })
-  })
-
-  describe('deleteFile', function() {
-    describe('with valid parameters', function() {
-      beforeEach(async function() {
-        return GcsPersistor.promises.deleteFile(bucket, key)
-      })
-
-      it('should delete the object', function() {
-        expect(Storage.prototype.bucket).to.have.been.calledWith(bucket)
-        expect(GcsBucket.file).to.have.been.calledWith(key)
-        expect(GcsFile.delete).to.have.been.called
-      })
-    })
-
-    describe('when the file does not exist', function() {
-      let error
-
-      beforeEach(async function() {
-        GcsFile.delete = sinon.stub().rejects(GcsNotFoundError)
-        try {
-          await GcsPersistor.promises.deleteFile(bucket, key)
-        } catch (err) {
-          error = err
-        }
-      })
-
-      it('should not throw an error', function() {
-        expect(error).not.to.exist
-      })
-    })
-  })
-
-  describe('deleteDirectory', function() {
-    const directoryName = `${ObjectId()}/${ObjectId()}`
-    describe('with valid parameters', function() {
-      beforeEach(async function() {
-        return GcsPersistor.promises.deleteDirectory(bucket, directoryName)
-      })
-
-      it('should list the objects in the directory', function() {
-        expect(Storage.prototype.bucket).to.have.been.calledWith(bucket)
-        expect(GcsBucket.getFiles).to.have.been.calledWith({
-          directory: directoryName
-        })
-      })
-
-      it('should delete the files', function() {
-        expect(GcsFile.delete).to.have.been.calledTwice
-      })
-    })
-
-    describe('when there is an error listing the objects', function() {
-      let error
-
-      beforeEach(async function() {
-        GcsBucket.getFiles = sinon.stub().rejects(genericError)
-        try {
-          await GcsPersistor.promises.deleteDirectory(bucket, directoryName)
-        } catch (err) {
-          error = err
-        }
-      })
-
-      it('should generate a WriteError', function() {
-        expect(error).to.be.an.instanceOf(Errors.WriteError)
-      })
-
-      it('should wrap the error', function() {
-        expect(error.cause).to.equal(genericError)
-      })
-    })
-  })
-
-  describe('directorySize', function() {
-    describe('with valid parameters', function() {
-      let size
-
-      beforeEach(async function() {
-        size = await GcsPersistor.promises.directorySize(bucket, key)
-      })
-
-      it('should list the objects in the directory', function() {
-        expect(Storage.prototype.bucket).to.have.been.calledWith(bucket)
-        expect(GcsBucket.getFiles).to.have.been.calledWith({ directory: key })
-      })
-
-      it('should return the directory size', function() {
-        expect(size).to.equal(filesSize)
-      })
-    })
-
-    describe('when there are no files', function() {
-      let size
-
-      beforeEach(async function() {
-        GcsBucket.getFiles.resolves([[]])
-        size = await GcsPersistor.promises.directorySize(bucket, key)
-      })
-
-      it('should list the objects in the directory', function() {
-        expect(Storage.prototype.bucket).to.have.been.calledWith(bucket)
-        expect(GcsBucket.getFiles).to.have.been.calledWith({ directory: key })
-      })
-
-      it('should return zero', function() {
-        expect(size).to.equal(0)
-      })
-    })
-
-    describe('when there is an error listing the objects', function() {
-      let error
-
-      beforeEach(async function() {
-        GcsBucket.getFiles.rejects(genericError)
-        try {
-          await GcsPersistor.promises.directorySize(bucket, key)
-        } catch (err) {
-          error = err
-        }
-      })
-
-      it('should generate a ReadError', function() {
-        expect(error).to.be.an.instanceOf(Errors.ReadError)
-      })
-
-      it('should wrap the error', function() {
-        expect(error.cause).to.equal(genericError)
-      })
-    })
-  })
-
-  describe('checkIfFileExists', function() {
-    describe('when the file exists', function() {
-      let exists
-
-      beforeEach(async function() {
-        exists = await GcsPersistor.promises.checkIfFileExists(bucket, key)
-      })
-
-      it('should ask the file if it exists', function() {
-        expect(Storage.prototype.bucket).to.have.been.calledWith(bucket)
-        expect(GcsBucket.file).to.have.been.calledWith(key)
-        expect(GcsFile.exists).to.have.been.called
-      })
-
-      it('should return that the file exists', function() {
-        expect(exists).to.equal(true)
-      })
-    })
-
-    describe('when the file does not exist', function() {
-      let exists
-
-      beforeEach(async function() {
-        GcsFile.exists = sinon.stub().resolves([false])
-        exists = await GcsPersistor.promises.checkIfFileExists(bucket, key)
-      })
-
-      it('should get the object header', function() {
-        expect(Storage.prototype.bucket).to.have.been.calledWith(bucket)
-        expect(GcsBucket.file).to.have.been.calledWith(key)
-        expect(GcsFile.exists).to.have.been.called
-      })
-
-      it('should return that the file does not exist', function() {
-        expect(exists).to.equal(false)
-      })
-    })
-
-    describe('when there is an error', function() {
-      let error
-
-      beforeEach(async function() {
-        GcsFile.exists = sinon.stub().rejects(genericError)
-        try {
-          await GcsPersistor.promises.checkIfFileExists(bucket, key)
-        } catch (err) {
-          error = err
-        }
-      })
-
-      it('should generate a ReadError', function() {
-        expect(error).to.be.an.instanceOf(Errors.ReadError)
-      })
-
-      it('should wrap the error', function() {
-        expect(error.cause).to.equal(genericError)
-      })
-    })
-  })
-})

+ 5 - 1
services/filestore/test/unit/js/LocalFileWriterTests.js

@@ -3,6 +3,7 @@ const chai = require('chai')
 const { expect } = chai
 const modulePath = '../../../app/js/LocalFileWriter.js'
 const SandboxedModule = require('sandboxed-module')
+const { Errors } = require('@overleaf/object-persistor')
 chai.use(require('sinon-chai'))
 
 describe('LocalFileWriter', function() {
@@ -22,6 +23,8 @@ describe('LocalFileWriter', function() {
       pipeline: sinon.stub().yields()
     }
 
+    const ObjectPersistor = { Errors }
+
     LocalFileWriter = SandboxedModule.require(modulePath, {
       requires: {
         fs,
@@ -30,7 +33,8 @@ describe('LocalFileWriter', function() {
         'metrics-sharelatex': {
           inc: sinon.stub(),
           Timer: sinon.stub().returns({ done: sinon.stub() })
-        }
+        },
+        '@overleaf/object-persistor': ObjectPersistor
       }
     })
   })

+ 0 - 519
services/filestore/test/unit/js/MigrationPersistorTests.js

@@ -1,519 +0,0 @@
-const sinon = require('sinon')
-const chai = require('chai')
-const { expect } = chai
-const modulePath = '../../../app/js/MigrationPersistor.js'
-const SandboxedModule = require('sandboxed-module')
-
-const Errors = require('../../../app/js/Errors')
-
-// Not all methods are tested here, but a method with each type of wrapping has
-// tests. Specifically, the following wrapping methods are tested here:
-// getFileStream: _wrapFallbackMethod
-// sendStream: forward-to-primary
-// deleteFile: _wrapMethodOnBothPersistors
-// copyFile: copyFileWithFallback
-
-describe('MigrationPersistorTests', function() {
-  const bucket = 'womBucket'
-  const fallbackBucket = 'bucKangaroo'
-  const key = 'monKey'
-  const destKey = 'donKey'
-  const genericError = new Error('guru meditation error')
-  const notFoundError = new Errors.NotFoundError('not found')
-  const size = 33
-  const md5 = 'ffffffff'
-
-  let Metrics,
-    Settings,
-    Logger,
-    Stream,
-    MigrationPersistor,
-    fileStream,
-    newPersistor
-
-  beforeEach(function() {
-    fileStream = {
-      name: 'fileStream',
-      on: sinon
-        .stub()
-        .withArgs('end')
-        .yields(),
-      pipe: sinon.stub()
-    }
-
-    newPersistor = function(hasFile) {
-      return {
-        promises: {
-          sendFile: sinon.stub().resolves(),
-          sendStream: sinon.stub().resolves(),
-          getFileStream: hasFile
-            ? sinon.stub().resolves(fileStream)
-            : sinon.stub().rejects(notFoundError),
-          deleteDirectory: sinon.stub().resolves(),
-          getFileSize: hasFile
-            ? sinon.stub().resolves(size)
-            : sinon.stub().rejects(notFoundError),
-          deleteFile: sinon.stub().resolves(),
-          copyFile: hasFile
-            ? sinon.stub().resolves()
-            : sinon.stub().rejects(notFoundError),
-          checkIfFileExists: sinon.stub().resolves(hasFile),
-          directorySize: hasFile
-            ? sinon.stub().resolves(size)
-            : sinon.stub().rejects(notFoundError),
-          getFileMd5Hash: hasFile
-            ? sinon.stub().resolves(md5)
-            : sinon.stub().rejects(notFoundError)
-        }
-      }
-    }
-
-    Settings = {
-      filestore: {
-        fallback: {
-          buckets: {
-            [bucket]: fallbackBucket
-          }
-        }
-      }
-    }
-
-    Metrics = {
-      inc: sinon.stub()
-    }
-
-    Stream = {
-      pipeline: sinon.stub().yields(),
-      PassThrough: sinon.stub()
-    }
-
-    Logger = {
-      warn: sinon.stub()
-    }
-
-    MigrationPersistor = SandboxedModule.require(modulePath, {
-      requires: {
-        'settings-sharelatex': Settings,
-        stream: Stream,
-        './Errors': Errors,
-        'metrics-sharelatex': Metrics,
-        'logger-sharelatex': Logger
-      },
-      globals: { console }
-    })
-  })
-
-  describe('getFileStream', function() {
-    const options = { wombat: 'potato' }
-    describe('when the primary persistor has the file', function() {
-      let primaryPersistor, fallbackPersistor, migrationPersistor, response
-      beforeEach(async function() {
-        primaryPersistor = newPersistor(true)
-        fallbackPersistor = newPersistor(false)
-        migrationPersistor = MigrationPersistor(
-          primaryPersistor,
-          fallbackPersistor
-        )
-        response = await migrationPersistor.promises.getFileStream(
-          bucket,
-          key,
-          options
-        )
-      })
-
-      it('should return the file stream', function() {
-        expect(response).to.equal(fileStream)
-      })
-
-      it('should fetch the file from the primary persistor, with the correct options', function() {
-        expect(
-          primaryPersistor.promises.getFileStream
-        ).to.have.been.calledWithExactly(bucket, key, options)
-      })
-
-      it('should not query the fallback persistor', function() {
-        expect(fallbackPersistor.promises.getFileStream).not.to.have.been.called
-      })
-    })
-
-    describe('when the fallback persistor has the file', function() {
-      let primaryPersistor, fallbackPersistor, migrationPersistor, response
-      beforeEach(async function() {
-        primaryPersistor = newPersistor(false)
-        fallbackPersistor = newPersistor(true)
-        migrationPersistor = MigrationPersistor(
-          primaryPersistor,
-          fallbackPersistor
-        )
-        response = await migrationPersistor.promises.getFileStream(
-          bucket,
-          key,
-          options
-        )
-      })
-
-      it('should return the file stream', function() {
-        expect(response).to.be.an.instanceOf(Stream.PassThrough)
-      })
-
-      it('should fetch the file from the primary persistor with the correct options', function() {
-        expect(
-          primaryPersistor.promises.getFileStream
-        ).to.have.been.calledWithExactly(bucket, key, options)
-      })
-
-      it('should fetch the file from the fallback persistor with the fallback bucket with the correct options', function() {
-        expect(
-          fallbackPersistor.promises.getFileStream
-        ).to.have.been.calledWithExactly(fallbackBucket, key, options)
-      })
-
-      it('should create one read stream', function() {
-        expect(fallbackPersistor.promises.getFileStream).to.have.been.calledOnce
-      })
-
-      it('should not send the file to the primary', function() {
-        expect(primaryPersistor.promises.sendStream).not.to.have.been.called
-      })
-    })
-
-    describe('when the file should be copied to the primary', function() {
-      let primaryPersistor,
-        fallbackPersistor,
-        migrationPersistor,
-        returnedStream
-      beforeEach(async function() {
-        primaryPersistor = newPersistor(false)
-        fallbackPersistor = newPersistor(true)
-        migrationPersistor = MigrationPersistor(
-          primaryPersistor,
-          fallbackPersistor
-        )
-        Settings.filestore.fallback.copyOnMiss = true
-        returnedStream = await migrationPersistor.promises.getFileStream(
-          bucket,
-          key,
-          options
-        )
-      })
-
-      it('should create one read stream', function() {
-        expect(fallbackPersistor.promises.getFileStream).to.have.been.calledOnce
-      })
-
-      it('should get the md5 hash from the source', function() {
-        expect(
-          fallbackPersistor.promises.getFileMd5Hash
-        ).to.have.been.calledWith(fallbackBucket, key)
-      })
-
-      it('should send a stream to the primary', function() {
-        expect(
-          primaryPersistor.promises.sendStream
-        ).to.have.been.calledWithExactly(
-          bucket,
-          key,
-          sinon.match.instanceOf(Stream.PassThrough),
-          md5
-        )
-      })
-
-      it('should send a stream to the client', function() {
-        expect(returnedStream).to.be.an.instanceOf(Stream.PassThrough)
-      })
-    })
-
-    describe('when neither persistor has the file', function() {
-      it('rejects with a NotFoundError', async function() {
-        const migrationPersistor = MigrationPersistor(
-          newPersistor(false),
-          newPersistor(false)
-        )
-        return expect(
-          migrationPersistor.promises.getFileStream(bucket, key)
-        ).to.eventually.be.rejected.and.be.an.instanceOf(Errors.NotFoundError)
-      })
-    })
-
-    describe('when the primary persistor throws an unexpected error', function() {
-      let primaryPersistor, fallbackPersistor, migrationPersistor, error
-      beforeEach(async function() {
-        primaryPersistor = newPersistor(false)
-        fallbackPersistor = newPersistor(true)
-        primaryPersistor.promises.getFileStream = sinon
-          .stub()
-          .rejects(genericError)
-        migrationPersistor = MigrationPersistor(
-          primaryPersistor,
-          fallbackPersistor
-        )
-        try {
-          await migrationPersistor.promises.getFileStream(bucket, key, options)
-        } catch (err) {
-          error = err
-        }
-      })
-
-      it('rejects with the error', function() {
-        expect(error).to.equal(genericError)
-      })
-
-      it('does not call the fallback', function() {
-        expect(fallbackPersistor.promises.getFileStream).not.to.have.been.called
-      })
-    })
-
-    describe('when the fallback persistor throws an unexpected error', function() {
-      let primaryPersistor, fallbackPersistor, migrationPersistor, error
-      beforeEach(async function() {
-        primaryPersistor = newPersistor(false)
-        fallbackPersistor = newPersistor(false)
-        fallbackPersistor.promises.getFileStream = sinon
-          .stub()
-          .rejects(genericError)
-        migrationPersistor = MigrationPersistor(
-          primaryPersistor,
-          fallbackPersistor
-        )
-        try {
-          await migrationPersistor.promises.getFileStream(bucket, key, options)
-        } catch (err) {
-          error = err
-        }
-      })
-
-      it('rejects with the error', function() {
-        expect(error).to.equal(genericError)
-      })
-
-      it('should have called the fallback', function() {
-        expect(
-          fallbackPersistor.promises.getFileStream
-        ).to.have.been.calledWith(fallbackBucket, key)
-      })
-    })
-  })
-
-  describe('sendStream', function() {
-    let primaryPersistor, fallbackPersistor, migrationPersistor
-    beforeEach(function() {
-      primaryPersistor = newPersistor(false)
-      fallbackPersistor = newPersistor(false)
-      migrationPersistor = MigrationPersistor(
-        primaryPersistor,
-        fallbackPersistor
-      )
-    })
-
-    describe('when it works', function() {
-      beforeEach(async function() {
-        return migrationPersistor.promises.sendStream(bucket, key, fileStream)
-      })
-
-      it('should send the file to the primary persistor', function() {
-        expect(
-          primaryPersistor.promises.sendStream
-        ).to.have.been.calledWithExactly(bucket, key, fileStream)
-      })
-
-      it('should not send the file to the fallback persistor', function() {
-        expect(fallbackPersistor.promises.sendStream).not.to.have.been.called
-      })
-    })
-
-    describe('when the primary persistor throws an error', function() {
-      it('returns the error', async function() {
-        primaryPersistor.promises.sendStream.rejects(notFoundError)
-        return expect(
-          migrationPersistor.promises.sendStream(bucket, key, fileStream)
-        ).to.eventually.be.rejected.and.be.an.instanceOf(Errors.NotFoundError)
-      })
-    })
-  })
-
-  describe('deleteFile', function() {
-    let primaryPersistor, fallbackPersistor, migrationPersistor
-    beforeEach(function() {
-      primaryPersistor = newPersistor(false)
-      fallbackPersistor = newPersistor(false)
-      migrationPersistor = MigrationPersistor(
-        primaryPersistor,
-        fallbackPersistor
-      )
-    })
-
-    describe('when it works', function() {
-      beforeEach(async function() {
-        return migrationPersistor.promises.deleteFile(bucket, key)
-      })
-
-      it('should delete the file from the primary', function() {
-        expect(
-          primaryPersistor.promises.deleteFile
-        ).to.have.been.calledWithExactly(bucket, key)
-      })
-
-      it('should delete the file from the fallback', function() {
-        expect(
-          fallbackPersistor.promises.deleteFile
-        ).to.have.been.calledWithExactly(fallbackBucket, key)
-      })
-    })
-
-    describe('when the primary persistor throws an error', function() {
-      let error
-      beforeEach(async function() {
-        primaryPersistor.promises.deleteFile.rejects(genericError)
-        try {
-          await migrationPersistor.promises.deleteFile(bucket, key)
-        } catch (err) {
-          error = err
-        }
-      })
-
-      it('should return the error', function() {
-        expect(error).to.equal(genericError)
-      })
-
-      it('should delete the file from the primary', function() {
-        expect(
-          primaryPersistor.promises.deleteFile
-        ).to.have.been.calledWithExactly(bucket, key)
-      })
-
-      it('should delete the file from the fallback', function() {
-        expect(
-          fallbackPersistor.promises.deleteFile
-        ).to.have.been.calledWithExactly(fallbackBucket, key)
-      })
-    })
-
-    describe('when the fallback persistor throws an error', function() {
-      let error
-      beforeEach(async function() {
-        fallbackPersistor.promises.deleteFile.rejects(genericError)
-        try {
-          await migrationPersistor.promises.deleteFile(bucket, key)
-        } catch (err) {
-          error = err
-        }
-      })
-
-      it('should return the error', function() {
-        expect(error).to.equal(genericError)
-      })
-
-      it('should delete the file from the primary', function() {
-        expect(
-          primaryPersistor.promises.deleteFile
-        ).to.have.been.calledWithExactly(bucket, key)
-      })
-
-      it('should delete the file from the fallback', function() {
-        expect(
-          fallbackPersistor.promises.deleteFile
-        ).to.have.been.calledWithExactly(fallbackBucket, key)
-      })
-    })
-  })
-
-  describe('copyFile', function() {
-    describe('when the file exists on the primary', function() {
-      let primaryPersistor, fallbackPersistor, migrationPersistor
-      beforeEach(async function() {
-        primaryPersistor = newPersistor(true)
-        fallbackPersistor = newPersistor(false)
-        migrationPersistor = MigrationPersistor(
-          primaryPersistor,
-          fallbackPersistor
-        )
-        return migrationPersistor.promises.copyFile(bucket, key, destKey)
-      })
-
-      it('should call copyFile to copy the file', function() {
-        expect(
-          primaryPersistor.promises.copyFile
-        ).to.have.been.calledWithExactly(bucket, key, destKey)
-      })
-
-      it('should not try to read from the fallback', function() {
-        expect(fallbackPersistor.promises.getFileStream).not.to.have.been.called
-      })
-    })
-
-    describe('when the file does not exist on the primary', function() {
-      let primaryPersistor, fallbackPersistor, migrationPersistor
-      beforeEach(async function() {
-        primaryPersistor = newPersistor(false)
-        fallbackPersistor = newPersistor(true)
-        migrationPersistor = MigrationPersistor(
-          primaryPersistor,
-          fallbackPersistor
-        )
-        return migrationPersistor.promises.copyFile(bucket, key, destKey)
-      })
-
-      it('should call copyFile to copy the file', function() {
-        expect(
-          primaryPersistor.promises.copyFile
-        ).to.have.been.calledWithExactly(bucket, key, destKey)
-      })
-
-      it('should fetch the file from the fallback', function() {
-        expect(
-          fallbackPersistor.promises.getFileStream
-        ).not.to.have.been.calledWithExactly(fallbackBucket, key)
-      })
-
-      it('should get the md5 hash from the source', function() {
-        expect(
-          fallbackPersistor.promises.getFileMd5Hash
-        ).to.have.been.calledWith(fallbackBucket, key)
-      })
-
-      it('should send the file to the primary', function() {
-        expect(
-          primaryPersistor.promises.sendStream
-        ).to.have.been.calledWithExactly(
-          bucket,
-          destKey,
-          sinon.match.instanceOf(Stream.PassThrough),
-          md5
-        )
-      })
-    })
-
-    describe('when the file does not exist on the fallback', function() {
-      let primaryPersistor, fallbackPersistor, migrationPersistor, error
-      beforeEach(async function() {
-        primaryPersistor = newPersistor(false)
-        fallbackPersistor = newPersistor(false)
-        migrationPersistor = MigrationPersistor(
-          primaryPersistor,
-          fallbackPersistor
-        )
-        try {
-          await migrationPersistor.promises.copyFile(bucket, key, destKey)
-        } catch (err) {
-          error = err
-        }
-      })
-
-      it('should call copyFile to copy the file', function() {
-        expect(
-          primaryPersistor.promises.copyFile
-        ).to.have.been.calledWithExactly(bucket, key, destKey)
-      })
-
-      it('should fetch the file from the fallback', function() {
-        expect(
-          fallbackPersistor.promises.getFileStream
-        ).not.to.have.been.calledWithExactly(fallbackBucket, key)
-      })
-
-      it('should return a not-found error', function() {
-        expect(error).to.be.an.instanceOf(Errors.NotFoundError)
-      })
-    })
-  })
-})

+ 0 - 78
services/filestore/test/unit/js/PersistorManagerTests.js

@@ -1,78 +0,0 @@
-const sinon = require('sinon')
-const chai = require('chai')
-const { expect } = chai
-const SandboxedModule = require('sandboxed-module')
-
-const modulePath = '../../../app/js/PersistorManager.js'
-
-describe('PersistorManager', function() {
-  let PersistorManager, FSPersistor, S3Persistor, settings, requires
-
-  beforeEach(function() {
-    FSPersistor = {
-      wrappedMethod: sinon.stub().returns('FSPersistor')
-    }
-    S3Persistor = {
-      wrappedMethod: sinon.stub().returns('S3Persistor')
-    }
-
-    settings = {
-      filestore: {}
-    }
-
-    requires = {
-      './S3Persistor': S3Persistor,
-      './FSPersistor': FSPersistor,
-      'settings-sharelatex': settings,
-      'logger-sharelatex': {
-        log() {},
-        err() {}
-      }
-    }
-  })
-
-  it('should implement the S3 wrapped method when S3 is configured', function() {
-    settings.filestore.backend = 's3'
-    PersistorManager = SandboxedModule.require(modulePath, { requires })
-
-    expect(PersistorManager).to.respondTo('wrappedMethod')
-    expect(PersistorManager.wrappedMethod()).to.equal('S3Persistor')
-  })
-
-  it("should implement the S3 wrapped method when 'aws-sdk' is configured", function() {
-    settings.filestore.backend = 'aws-sdk'
-    PersistorManager = SandboxedModule.require(modulePath, { requires })
-
-    expect(PersistorManager).to.respondTo('wrappedMethod')
-    expect(PersistorManager.wrappedMethod()).to.equal('S3Persistor')
-  })
-
-  it('should implement the FS wrapped method when FS is configured', function() {
-    settings.filestore.backend = 'fs'
-    PersistorManager = SandboxedModule.require(modulePath, { requires })
-
-    expect(PersistorManager).to.respondTo('wrappedMethod')
-    expect(PersistorManager.wrappedMethod()).to.equal('FSPersistor')
-  })
-
-  it('should throw an error when the backend is not configured', function() {
-    try {
-      SandboxedModule.require(modulePath, { requires })
-    } catch (err) {
-      expect(err.message).to.equal('no backend specified - config incomplete')
-      return
-    }
-    expect('should have caught an error').not.to.exist
-  })
-
-  it('should throw an error when the backend is unknown', function() {
-    settings.filestore.backend = 'magic'
-    try {
-      SandboxedModule.require(modulePath, { requires })
-    } catch (err) {
-      expect(err.message).to.equal('unknown filestore backend: magic')
-      return
-    }
-    expect('should have caught an error').not.to.exist
-  })
-})

+ 0 - 865
services/filestore/test/unit/js/S3PersistorTests.js

@@ -1,865 +0,0 @@
-const sinon = require('sinon')
-const chai = require('chai')
-const { expect } = chai
-const modulePath = '../../../app/js/S3Persistor.js'
-const SandboxedModule = require('sandboxed-module')
-
-const Errors = require('../../../app/js/Errors')
-
-describe('S3PersistorTests', function() {
-  const defaultS3Key = 'frog'
-  const defaultS3Secret = 'prince'
-  const defaultS3Credentials = {
-    credentials: {
-      accessKeyId: defaultS3Key,
-      secretAccessKey: defaultS3Secret
-    }
-  }
-  const filename = '/wombat/potato.tex'
-  const bucket = 'womBucket'
-  const key = 'monKey'
-  const destKey = 'donKey'
-  const objectSize = 5555
-  const genericError = new Error('guru meditation error')
-  const files = [
-    { Key: 'llama', Size: 11 },
-    { Key: 'hippo', Size: 22 }
-  ]
-  const filesSize = 33
-  const md5 = 'ffffffff00000000ffffffff00000000'
-
-  let Metrics,
-    Logger,
-    Transform,
-    S3,
-    Fs,
-    ReadStream,
-    Stream,
-    S3Persistor,
-    S3Client,
-    S3ReadStream,
-    S3NotFoundError,
-    S3AccessDeniedError,
-    FileNotFoundError,
-    EmptyPromise,
-    settings,
-    Hash,
-    crypto
-
-  beforeEach(function() {
-    settings = {
-      filestore: {
-        backend: 's3',
-        s3: {
-          secret: defaultS3Secret,
-          key: defaultS3Key,
-          partSize: 100 * 1024 * 1024
-        },
-        stores: {
-          user_files: 'sl_user_files'
-        }
-      }
-    }
-
-    Transform = class {
-      on(event, callback) {
-        if (event === 'readable') {
-          callback()
-        }
-      }
-
-      once() {}
-      removeListener() {}
-    }
-
-    Stream = {
-      pipeline: sinon.stub().yields(),
-      Transform: Transform
-    }
-
-    EmptyPromise = {
-      promise: sinon.stub().resolves()
-    }
-
-    Metrics = {
-      count: sinon.stub()
-    }
-
-    ReadStream = {
-      pipe: sinon.stub().returns('readStream'),
-      on: sinon.stub(),
-      removeListener: sinon.stub()
-    }
-    ReadStream.on.withArgs('end').yields()
-    ReadStream.on.withArgs('pipe').yields({
-      unpipe: sinon.stub(),
-      resume: sinon.stub()
-    })
-
-    FileNotFoundError = new Error('File not found')
-    FileNotFoundError.code = 'ENOENT'
-
-    Fs = {
-      createReadStream: sinon.stub().returns(ReadStream)
-    }
-
-    S3NotFoundError = new Error('not found')
-    S3NotFoundError.code = 'NoSuchKey'
-
-    S3AccessDeniedError = new Error('access denied')
-    S3AccessDeniedError.code = 'AccessDenied'
-
-    S3ReadStream = {
-      on: sinon.stub(),
-      pipe: sinon.stub(),
-      removeListener: sinon.stub()
-    }
-    S3ReadStream.on.withArgs('end').yields()
-    S3ReadStream.on.withArgs('pipe').yields({
-      unpipe: sinon.stub(),
-      resume: sinon.stub()
-    })
-    S3Client = {
-      getObject: sinon.stub().returns({
-        createReadStream: sinon.stub().returns(S3ReadStream)
-      }),
-      headObject: sinon.stub().returns({
-        promise: sinon.stub().resolves({
-          ContentLength: objectSize,
-          ETag: md5
-        })
-      }),
-      listObjects: sinon.stub().returns({
-        promise: sinon.stub().resolves({
-          Contents: files
-        })
-      }),
-      upload: sinon
-        .stub()
-        .returns({ promise: sinon.stub().resolves({ ETag: `"${md5}"` }) }),
-      copyObject: sinon.stub().returns(EmptyPromise),
-      deleteObject: sinon.stub().returns(EmptyPromise),
-      deleteObjects: sinon.stub().returns(EmptyPromise)
-    }
-    S3 = sinon.stub().returns(S3Client)
-
-    Hash = {
-      end: sinon.stub(),
-      read: sinon.stub().returns(md5),
-      digest: sinon.stub().returns(md5),
-      setEncoding: sinon.stub()
-    }
-    crypto = {
-      createHash: sinon.stub().returns(Hash)
-    }
-
-    Logger = {
-      warn: sinon.stub()
-    }
-
-    S3Persistor = SandboxedModule.require(modulePath, {
-      requires: {
-        'aws-sdk/clients/s3': S3,
-        'settings-sharelatex': settings,
-        'logger-sharelatex': Logger,
-        './Errors': Errors,
-        fs: Fs,
-        stream: Stream,
-        'metrics-sharelatex': Metrics,
-        crypto
-      },
-      globals: { console, Buffer }
-    })
-  })
-
-  describe('getFileStream', function() {
-    describe('when called with valid parameters', function() {
-      let stream
-
-      beforeEach(async function() {
-        stream = await S3Persistor.promises.getFileStream(bucket, key)
-      })
-
-      it('returns a metered stream', function() {
-        expect(stream).to.be.instanceOf(Transform)
-      })
-
-      it('sets the AWS client up with credentials from settings', function() {
-        expect(S3).to.have.been.calledWith(defaultS3Credentials)
-      })
-
-      it('fetches the right key from the right bucket', function() {
-        expect(S3Client.getObject).to.have.been.calledWith({
-          Bucket: bucket,
-          Key: key
-        })
-      })
-
-      it('pipes the stream through the meter', async function() {
-        expect(S3ReadStream.pipe).to.have.been.calledWith(
-          sinon.match.instanceOf(Transform)
-        )
-      })
-    })
-
-    describe('when called with a byte range', function() {
-      let stream
-
-      beforeEach(async function() {
-        stream = await S3Persistor.promises.getFileStream(bucket, key, {
-          start: 5,
-          end: 10
-        })
-      })
-
-      it('returns a metered stream', function() {
-        expect(stream).to.be.instanceOf(Stream.Transform)
-      })
-
-      it('passes the byte range on to S3', function() {
-        expect(S3Client.getObject).to.have.been.calledWith({
-          Bucket: bucket,
-          Key: key,
-          Range: 'bytes=5-10'
-        })
-      })
-    })
-
-    describe('when there are alternative credentials', function() {
-      let stream
-      const alternativeSecret = 'giraffe'
-      const alternativeKey = 'hippo'
-      const alternativeS3Credentials = {
-        credentials: {
-          accessKeyId: alternativeKey,
-          secretAccessKey: alternativeSecret
-        }
-      }
-
-      beforeEach(async function() {
-        settings.filestore.s3BucketCreds = {}
-        settings.filestore.s3BucketCreds[bucket] = {
-          auth_key: alternativeKey,
-          auth_secret: alternativeSecret
-        }
-
-        stream = await S3Persistor.promises.getFileStream(bucket, key)
-      })
-
-      it('returns a metered stream', function() {
-        expect(stream).to.be.instanceOf(Stream.Transform)
-      })
-
-      it('sets the AWS client up with the alternative credentials', function() {
-        expect(S3).to.have.been.calledWith(alternativeS3Credentials)
-      })
-
-      it('fetches the right key from the right bucket', function() {
-        expect(S3Client.getObject).to.have.been.calledWith({
-          Bucket: bucket,
-          Key: key
-        })
-      })
-
-      it('caches the credentials', async function() {
-        stream = await S3Persistor.promises.getFileStream(bucket, key)
-
-        expect(S3).to.have.been.calledOnceWith(alternativeS3Credentials)
-      })
-
-      it('uses the default credentials for an unknown bucket', async function() {
-        stream = await S3Persistor.promises.getFileStream('anotherBucket', key)
-
-        expect(S3).to.have.been.calledTwice
-        expect(S3.firstCall).to.have.been.calledWith(alternativeS3Credentials)
-        expect(S3.secondCall).to.have.been.calledWith(defaultS3Credentials)
-      })
-
-      it('caches the default credentials', async function() {
-        stream = await S3Persistor.promises.getFileStream('anotherBucket', key)
-        stream = await S3Persistor.promises.getFileStream('anotherBucket', key)
-
-        expect(S3).to.have.been.calledTwice
-        expect(S3.firstCall).to.have.been.calledWith(alternativeS3Credentials)
-        expect(S3.secondCall).to.have.been.calledWith(defaultS3Credentials)
-      })
-
-      it('throws an error if there are no credentials for the bucket', async function() {
-        delete settings.filestore.s3.key
-        delete settings.filestore.s3.secret
-
-        await expect(
-          S3Persistor.promises.getFileStream('anotherBucket', key)
-        ).to.eventually.be.rejected.and.be.an.instanceOf(Errors.SettingsError)
-      })
-    })
-
-    describe("when the file doesn't exist", function() {
-      let error, stream
-
-      beforeEach(async function() {
-        Transform.prototype.on = sinon.stub()
-        S3ReadStream.on.withArgs('error').yields(S3NotFoundError)
-        try {
-          stream = await S3Persistor.promises.getFileStream(bucket, key)
-        } catch (err) {
-          error = err
-        }
-      })
-
-      it('does not return a stream', function() {
-        expect(stream).not.to.exist
-      })
-
-      it('throws a NotFoundError', function() {
-        expect(error).to.be.an.instanceOf(Errors.NotFoundError)
-      })
-
-      it('wraps the error', function() {
-        expect(error.cause).to.exist
-      })
-
-      it('stores the bucket and key in the error', function() {
-        expect(error.info).to.include({ bucketName: bucket, key: key })
-      })
-    })
-
-    describe('when access to the file is denied', function() {
-      let error, stream
-
-      beforeEach(async function() {
-        Transform.prototype.on = sinon.stub()
-        S3ReadStream.on.withArgs('error').yields(S3AccessDeniedError)
-        try {
-          stream = await S3Persistor.promises.getFileStream(bucket, key)
-        } catch (err) {
-          error = err
-        }
-      })
-
-      it('does not return a stream', function() {
-        expect(stream).not.to.exist
-      })
-
-      it('throws a NotFoundError', function() {
-        expect(error).to.be.an.instanceOf(Errors.NotFoundError)
-      })
-
-      it('wraps the error', function() {
-        expect(error.cause).to.exist
-      })
-
-      it('stores the bucket and key in the error', function() {
-        expect(error.info).to.include({ bucketName: bucket, key: key })
-      })
-    })
-
-    describe('when S3 encounters an unkown error', function() {
-      let error, stream
-
-      beforeEach(async function() {
-        Transform.prototype.on = sinon.stub()
-        S3ReadStream.on.withArgs('error').yields(genericError)
-        try {
-          stream = await S3Persistor.promises.getFileStream(bucket, key)
-        } catch (err) {
-          error = err
-        }
-      })
-
-      it('does not return a stream', function() {
-        expect(stream).not.to.exist
-      })
-
-      it('throws a ReadError', function() {
-        expect(error).to.be.an.instanceOf(Errors.ReadError)
-      })
-
-      it('wraps the error', function() {
-        expect(error.cause).to.exist
-      })
-
-      it('stores the bucket and key in the error', function() {
-        expect(error.info).to.include({ bucketName: bucket, key: key })
-      })
-    })
-  })
-
-  describe('getFileSize', function() {
-    describe('when called with valid parameters', function() {
-      let size
-
-      beforeEach(async function() {
-        size = await S3Persistor.promises.getFileSize(bucket, key)
-      })
-
-      it('should return the object size', function() {
-        expect(size).to.equal(objectSize)
-      })
-
-      it('should pass the bucket and key to S3', function() {
-        expect(S3Client.headObject).to.have.been.calledWith({
-          Bucket: bucket,
-          Key: key
-        })
-      })
-    })
-
-    describe('when the object is not found', function() {
-      let error
-
-      beforeEach(async function() {
-        S3Client.headObject = sinon.stub().returns({
-          promise: sinon.stub().rejects(S3NotFoundError)
-        })
-        try {
-          await S3Persistor.promises.getFileSize(bucket, key)
-        } catch (err) {
-          error = err
-        }
-      })
-
-      it('should return a NotFoundError', function() {
-        expect(error).to.be.an.instanceOf(Errors.NotFoundError)
-      })
-
-      it('should wrap the error', function() {
-        expect(error.cause).to.equal(S3NotFoundError)
-      })
-    })
-
-    describe('when S3 returns an error', function() {
-      let error
-
-      beforeEach(async function() {
-        S3Client.headObject = sinon.stub().returns({
-          promise: sinon.stub().rejects(genericError)
-        })
-        try {
-          await S3Persistor.promises.getFileSize(bucket, key)
-        } catch (err) {
-          error = err
-        }
-      })
-
-      it('should return a ReadError', function() {
-        expect(error).to.be.an.instanceOf(Errors.ReadError)
-      })
-
-      it('should wrap the error', function() {
-        expect(error.cause).to.equal(genericError)
-      })
-    })
-  })
-
-  describe('sendStream', function() {
-    describe('with valid parameters', function() {
-      beforeEach(async function() {
-        return S3Persistor.promises.sendStream(bucket, key, ReadStream)
-      })
-
-      it('should upload the stream', function() {
-        expect(S3Client.upload).to.have.been.calledWith({
-          Bucket: bucket,
-          Key: key,
-          Body: sinon.match.instanceOf(Stream.Transform)
-        })
-      })
-
-      it('should upload files in a single part', function() {
-        expect(S3Client.upload).to.have.been.calledWith(sinon.match.any, {
-          partSize: 100 * 1024 * 1024
-        })
-      })
-
-      it('should meter the stream', function() {
-        expect(Stream.pipeline).to.have.been.calledWith(
-          ReadStream,
-          sinon.match.instanceOf(Stream.Transform)
-        )
-      })
-
-      it('calculates the md5 hash of the file', function() {
-        expect(Hash.digest).to.have.been.called
-      })
-    })
-
-    describe('when a hash is supploed', function() {
-      beforeEach(async function() {
-        return S3Persistor.promises.sendStream(
-          bucket,
-          key,
-          ReadStream,
-          'aaaaaaaabbbbbbbbaaaaaaaabbbbbbbb'
-        )
-      })
-
-      it('should not calculate the md5 hash of the file', function() {
-        expect(Hash.digest).not.to.have.been.called
-      })
-
-      it('sends the hash in base64', function() {
-        expect(S3Client.upload).to.have.been.calledWith({
-          Bucket: bucket,
-          Key: key,
-          Body: sinon.match.instanceOf(Transform),
-          ContentMD5: 'qqqqqru7u7uqqqqqu7u7uw=='
-        })
-      })
-
-      it('does not fetch the md5 hash of the uploaded file', function() {
-        expect(S3Client.headObject).not.to.have.been.called
-      })
-    })
-
-    describe('when the upload fails', function() {
-      let error
-      beforeEach(async function() {
-        S3Client.upload = sinon.stub().returns({
-          promise: sinon.stub().rejects(genericError)
-        })
-        try {
-          await S3Persistor.promises.sendStream(bucket, key, ReadStream)
-        } catch (err) {
-          error = err
-        }
-      })
-
-      it('throws a WriteError', function() {
-        expect(error).to.be.an.instanceOf(Errors.WriteError)
-      })
-    })
-
-    describe("when the etag isn't a valid md5 hash", function() {
-      beforeEach(async function() {
-        S3Client.upload = sinon.stub().returns({
-          promise: sinon.stub().resolves({
-            ETag: 'somethingthatisntanmd5',
-            Bucket: bucket,
-            Key: key
-          })
-        })
-
-        await S3Persistor.promises.sendStream(bucket, key, ReadStream)
-      })
-
-      it('should re-fetch the file to verify it', function() {
-        expect(S3Client.getObject).to.have.been.calledWith({
-          Bucket: bucket,
-          Key: key
-        })
-      })
-
-      it('should meter the download', function() {
-        expect(S3ReadStream.pipe).to.have.been.calledWith(
-          sinon.match.instanceOf(Stream.Transform)
-        )
-      })
-
-      it('should calculate the md5 hash from the file', function() {
-        expect(Hash.digest).to.have.been.called
-      })
-    })
-  })
-
-  describe('sendFile', function() {
-    describe('with valid parameters', function() {
-      beforeEach(async function() {
-        return S3Persistor.promises.sendFile(bucket, key, filename)
-      })
-
-      it('should create a read stream for the file', function() {
-        expect(Fs.createReadStream).to.have.been.calledWith(filename)
-      })
-
-      it('should upload the stream', function() {
-        expect(S3Client.upload).to.have.been.calledWith({
-          Bucket: bucket,
-          Key: key,
-          Body: sinon.match.instanceOf(Transform)
-        })
-      })
-    })
-  })
-
-  describe('copyFile', function() {
-    describe('with valid parameters', function() {
-      beforeEach(async function() {
-        return S3Persistor.promises.copyFile(bucket, key, destKey)
-      })
-
-      it('should copy the object', function() {
-        expect(S3Client.copyObject).to.have.been.calledWith({
-          Bucket: bucket,
-          Key: destKey,
-          CopySource: `${bucket}/${key}`
-        })
-      })
-    })
-
-    describe('when the file does not exist', function() {
-      let error
-
-      beforeEach(async function() {
-        S3Client.copyObject = sinon.stub().returns({
-          promise: sinon.stub().rejects(S3NotFoundError)
-        })
-        try {
-          await S3Persistor.promises.copyFile(bucket, key, destKey)
-        } catch (err) {
-          error = err
-        }
-      })
-
-      it('should throw a NotFoundError', function() {
-        expect(error).to.be.an.instanceOf(Errors.NotFoundError)
-      })
-    })
-  })
-
-  describe('deleteFile', function() {
-    describe('with valid parameters', function() {
-      beforeEach(async function() {
-        return S3Persistor.promises.deleteFile(bucket, key)
-      })
-
-      it('should delete the object', function() {
-        expect(S3Client.deleteObject).to.have.been.calledWith({
-          Bucket: bucket,
-          Key: key
-        })
-      })
-    })
-  })
-
-  describe('deleteDirectory', function() {
-    describe('with valid parameters', function() {
-      beforeEach(async function() {
-        return S3Persistor.promises.deleteDirectory(bucket, key)
-      })
-
-      it('should list the objects in the directory', function() {
-        expect(S3Client.listObjects).to.have.been.calledWith({
-          Bucket: bucket,
-          Prefix: key
-        })
-      })
-
-      it('should delete the objects using their keys', function() {
-        expect(S3Client.deleteObjects).to.have.been.calledWith({
-          Bucket: bucket,
-          Delete: {
-            Objects: [{ Key: 'llama' }, { Key: 'hippo' }],
-            Quiet: true
-          }
-        })
-      })
-    })
-
-    describe('when there are no files', function() {
-      beforeEach(async function() {
-        S3Client.listObjects = sinon
-          .stub()
-          .returns({ promise: sinon.stub().resolves({ Contents: [] }) })
-        return S3Persistor.promises.deleteDirectory(bucket, key)
-      })
-
-      it('should list the objects in the directory', function() {
-        expect(S3Client.listObjects).to.have.been.calledWith({
-          Bucket: bucket,
-          Prefix: key
-        })
-      })
-
-      it('should not try to delete any objects', function() {
-        expect(S3Client.deleteObjects).not.to.have.been.called
-      })
-    })
-
-    describe('when there is an error listing the objects', function() {
-      let error
-
-      beforeEach(async function() {
-        S3Client.listObjects = sinon
-          .stub()
-          .returns({ promise: sinon.stub().rejects(genericError) })
-        try {
-          await S3Persistor.promises.deleteDirectory(bucket, key)
-        } catch (err) {
-          error = err
-        }
-      })
-
-      it('should generate a ReadError', function() {
-        expect(error).to.be.an.instanceOf(Errors.ReadError)
-      })
-
-      it('should wrap the error', function() {
-        expect(error.cause).to.equal(genericError)
-      })
-
-      it('should not try to delete any objects', function() {
-        expect(S3Client.deleteObjects).not.to.have.been.called
-      })
-    })
-
-    describe('when there is an error deleting the objects', function() {
-      let error
-
-      beforeEach(async function() {
-        S3Client.deleteObjects = sinon
-          .stub()
-          .returns({ promise: sinon.stub().rejects(genericError) })
-        try {
-          await S3Persistor.promises.deleteDirectory(bucket, key)
-        } catch (err) {
-          error = err
-        }
-      })
-
-      it('should generate a WriteError', function() {
-        expect(error).to.be.an.instanceOf(Errors.WriteError)
-      })
-
-      it('should wrap the error', function() {
-        expect(error.cause).to.equal(genericError)
-      })
-    })
-  })
-
-  describe('directorySize', function() {
-    describe('with valid parameters', function() {
-      let size
-
-      beforeEach(async function() {
-        size = await S3Persistor.promises.directorySize(bucket, key)
-      })
-
-      it('should list the objects in the directory', function() {
-        expect(S3Client.listObjects).to.have.been.calledWith({
-          Bucket: bucket,
-          Prefix: key
-        })
-      })
-
-      it('should return the directory size', function() {
-        expect(size).to.equal(filesSize)
-      })
-    })
-
-    describe('when there are no files', function() {
-      let size
-
-      beforeEach(async function() {
-        S3Client.listObjects = sinon
-          .stub()
-          .returns({ promise: sinon.stub().resolves({ Contents: [] }) })
-        size = await S3Persistor.promises.directorySize(bucket, key)
-      })
-
-      it('should list the objects in the directory', function() {
-        expect(S3Client.listObjects).to.have.been.calledWith({
-          Bucket: bucket,
-          Prefix: key
-        })
-      })
-
-      it('should return zero', function() {
-        expect(size).to.equal(0)
-      })
-    })
-
-    describe('when there is an error listing the objects', function() {
-      let error
-
-      beforeEach(async function() {
-        S3Client.listObjects = sinon
-          .stub()
-          .returns({ promise: sinon.stub().rejects(genericError) })
-        try {
-          await S3Persistor.promises.directorySize(bucket, key)
-        } catch (err) {
-          error = err
-        }
-      })
-
-      it('should generate a ReadError', function() {
-        expect(error).to.be.an.instanceOf(Errors.ReadError)
-      })
-
-      it('should wrap the error', function() {
-        expect(error.cause).to.equal(genericError)
-      })
-    })
-  })
-
-  describe('checkIfFileExists', function() {
-    describe('when the file exists', function() {
-      let exists
-
-      beforeEach(async function() {
-        exists = await S3Persistor.promises.checkIfFileExists(bucket, key)
-      })
-
-      it('should get the object header', function() {
-        expect(S3Client.headObject).to.have.been.calledWith({
-          Bucket: bucket,
-          Key: key
-        })
-      })
-
-      it('should return that the file exists', function() {
-        expect(exists).to.equal(true)
-      })
-    })
-
-    describe('when the file does not exist', function() {
-      let exists
-
-      beforeEach(async function() {
-        S3Client.headObject = sinon
-          .stub()
-          .returns({ promise: sinon.stub().rejects(S3NotFoundError) })
-        exists = await S3Persistor.promises.checkIfFileExists(bucket, key)
-      })
-
-      it('should get the object header', function() {
-        expect(S3Client.headObject).to.have.been.calledWith({
-          Bucket: bucket,
-          Key: key
-        })
-      })
-
-      it('should return that the file does not exist', function() {
-        expect(exists).to.equal(false)
-      })
-    })
-
-    describe('when there is an error', function() {
-      let error
-
-      beforeEach(async function() {
-        S3Client.headObject = sinon
-          .stub()
-          .returns({ promise: sinon.stub().rejects(genericError) })
-        try {
-          await S3Persistor.promises.checkIfFileExists(bucket, key)
-        } catch (err) {
-          error = err
-        }
-      })
-
-      it('should generate a ReadError', function() {
-        expect(error).to.be.an.instanceOf(Errors.ReadError)
-      })
-
-      it('should wrap the upstream ReadError', function() {
-        expect(error.cause).to.be.an.instanceOf(Errors.ReadError)
-      })
-
-      it('should eventually wrap the error', function() {
-        expect(error.cause.cause).to.equal(genericError)
-      })
-    })
-  })
-})

+ 5 - 1
services/filestore/test/unit/js/SafeExecTests.js

@@ -2,6 +2,7 @@ const chai = require('chai')
 const should = chai.should()
 const { expect } = chai
 const modulePath = '../../../app/js/SafeExec'
+const { Errors } = require('@overleaf/object-persistor')
 const SandboxedModule = require('sandboxed-module')
 
 describe('SafeExec', function() {
@@ -11,10 +12,13 @@ describe('SafeExec', function() {
     settings = { enableConversions: true }
     options = { timeout: 10 * 1000, killSignal: 'SIGTERM' }
 
+    const ObjectPersistor = { Errors }
+
     safeExec = SandboxedModule.require(modulePath, {
       globals: { process },
       requires: {
-        'settings-sharelatex': settings
+        'settings-sharelatex': settings,
+        '@overleaf/object-persistor': ObjectPersistor
       }
     })
   })

Unele fișiere nu au fost afișate deoarece prea multe fișiere au fost modificate în acest diff