Просмотр исходного кода

Merge pull request #29841 from overleaf/ar-convert-filestore-to-esm

[filestore] convert to ES modules

GitOrigin-RevId: 404905973548bb6e437fff66b368e87be8249b73
Andrew Rumble 8 месяцев назад
Родитель
Сommit
90cf4b6a0a
39 измененных файлов с 1003 добавлено и 907 удалено
  1. 2 1
      package-lock.json
  2. 11 14
      services/filestore/app.js
  3. 7 6
      services/filestore/app/js/Errors.js
  4. 6 6
      services/filestore/app/js/FileController.js
  5. 8 7
      services/filestore/app/js/FileConverter.js
  6. 19 13
      services/filestore/app/js/FileHandler.js
  7. 6 6
      services/filestore/app/js/ImageOptimiser.js
  8. 3 3
      services/filestore/app/js/KeyBuilder.js
  9. 13 11
      services/filestore/app/js/LocalFileWriter.js
  10. 3 4
      services/filestore/app/js/PersistorManager.js
  11. 3 3
      services/filestore/app/js/RequestLogger.js
  12. 9 6
      services/filestore/app/js/SafeExec.js
  13. 3 6
      services/filestore/app/js/project_key.js
  14. 2 0
      services/filestore/buildscript.txt
  15. 0 0
      services/filestore/config/settings.defaults.cjs
  16. 2 0
      services/filestore/docker-compose.ci.yml
  17. 1 0
      services/filestore/docker-compose.yml
  18. 5 3
      services/filestore/package.json
  19. 6 6
      services/filestore/test/acceptance/js/FilestoreApp.js
  20. 45 39
      services/filestore/test/acceptance/js/FilestoreTests.js
  21. 10 9
      services/filestore/test/acceptance/js/TestConfig.js
  22. 5 6
      services/filestore/test/acceptance/js/TestHelper.js
  23. 6 37
      services/filestore/test/setup.js
  24. 92 69
      services/filestore/test/unit/js/FileController.test.js
  25. 45 36
      services/filestore/test/unit/js/FileConverter.test.js
  26. 283 0
      services/filestore/test/unit/js/FileHandler.test.js
  27. 0 298
      services/filestore/test/unit/js/FileHandlerTests.js
  28. 78 0
      services/filestore/test/unit/js/ImageOptimiser.test.js
  29. 0 67
      services/filestore/test/unit/js/ImageOptimiserTests.js
  30. 12 8
      services/filestore/test/unit/js/Keybuilder.test.js
  31. 115 0
      services/filestore/test/unit/js/LocalFileWriter.test.js
  32. 0 111
      services/filestore/test/unit/js/LocalFileWriterTests.js
  33. 114 0
      services/filestore/test/unit/js/SafeExec.test.js
  34. 0 110
      services/filestore/test/unit/js/SafeExecTests.js
  35. 24 0
      services/filestore/test/unit/js/Settings.test.js
  36. 0 21
      services/filestore/test/unit/js/SettingsTests.js
  37. 37 0
      services/filestore/test/unit/setup.js
  38. 3 1
      services/filestore/tsconfig.json
  39. 25 0
      services/filestore/vitest.config.unit.cjs

+ 2 - 1
package-lock.json

@@ -55300,7 +55300,8 @@
         "sinon": "9.0.2",
         "sinon-chai": "^3.7.0",
         "streamifier": "^0.1.1",
-        "typescript": "^5.0.4"
+        "typescript": "^5.0.4",
+        "vitest": "^3.2.4"
       }
     },
     "services/filestore/node_modules/diff": {

+ 11 - 14
services/filestore/app.js

@@ -1,20 +1,17 @@
 // Metrics must be initialized before importing anything else
-require('@overleaf/metrics/initialize')
+import '@overleaf/metrics/initialize.js'
 
-const Events = require('node:events')
-const Metrics = require('@overleaf/metrics')
+import Events from 'node:events'
+import Metrics from '@overleaf/metrics'
+import logger from '@overleaf/logger'
+import settings from '@overleaf/settings'
+import express from 'express'
+import fileController from './app/js/FileController.js'
+import keyBuilder from './app/js/KeyBuilder.js'
+import RequestLogger from './app/js/RequestLogger.js'
 
-const logger = require('@overleaf/logger')
 logger.initialize(process.env.METRICS_APP_NAME || 'filestore')
 
-const settings = require('@overleaf/settings')
-const express = require('express')
-
-const fileController = require('./app/js/FileController')
-const keyBuilder = require('./app/js/KeyBuilder')
-
-const RequestLogger = require('./app/js/RequestLogger')
-
 Events.setMaxListeners(20)
 
 const app = express()
@@ -106,7 +103,7 @@ const port = settings.internal.filestore.port || 3009
 const host = settings.internal.filestore.host || '0.0.0.0'
 
 let server = null
-if (!module.parent) {
+if (import.meta.main) {
   // Called directly
   server = app.listen(port, host, error => {
     if (error) {
@@ -157,4 +154,4 @@ function handleShutdownSignal(signal) {
 
 process.on('SIGTERM', handleShutdownSignal)
 
-module.exports = app
+export default app

+ 7 - 6
services/filestore/app/js/Errors.js

@@ -1,5 +1,6 @@
-const OError = require('@overleaf/o-error')
-const { Errors } = require('@overleaf/object-persistor')
+import OError from '@overleaf/o-error'
+
+import { Errors } from '@overleaf/object-persistor'
 
 class HealthCheckError extends OError {}
 class ConversionsDisabledError extends OError {}
@@ -19,12 +20,12 @@ class FailedCommandError extends OError {
   }
 }
 
-module.exports = {
-  FailedCommandError,
+export default {
+  ...Errors,
+  HealthCheckError,
   ConversionsDisabledError,
   ConversionError,
-  HealthCheckError,
   TimeoutError,
   InvalidParametersError,
-  ...Errors,
+  FailedCommandError,
 }

+ 6 - 6
services/filestore/app/js/FileController.js

@@ -1,12 +1,12 @@
-const FileHandler = require('./FileHandler')
-const metrics = require('@overleaf/metrics')
-const parseRange = require('range-parser')
-const Errors = require('./Errors')
-const { pipeline } = require('node:stream')
+import FileHandler from './FileHandler.js'
+import metrics from '@overleaf/metrics'
+import parseRange from 'range-parser'
+import Errors from './Errors.js'
+import { pipeline } from 'node:stream'
 
 const maxSizeInBytes = 1024 * 1024 * 1024 // 1GB
 
-module.exports = {
+export default {
   getFile,
   getFileHead,
   insertFile,

+ 8 - 7
services/filestore/app/js/FileConverter.js

@@ -1,15 +1,16 @@
-const metrics = require('@overleaf/metrics')
-const Settings = require('@overleaf/settings')
-const { callbackify } = require('node:util')
+import metrics from '@overleaf/metrics'
+import Settings from '@overleaf/settings'
+import { callbackify } from 'node:util'
+import SafeExec from './SafeExec.js'
+import Errors from './Errors.js'
 
-const safeExec = require('./SafeExec').promises
-const { ConversionError } = require('./Errors')
+const { ConversionError } = Errors
 
 const APPROVED_FORMATS = ['png']
 const FOURTY_SECONDS = 40 * 1000
 const KILL_SIGNAL = 'SIGTERM'
 
-module.exports = {
+export default {
   convert: callbackify(convert),
   thumbnail: callbackify(thumbnail),
   preview: callbackify(preview),
@@ -81,7 +82,7 @@ async function _convert(sourcePath, requestedFormat, command) {
   command = Settings.commands.convertCommandPrefix.concat(command)
 
   try {
-    await safeExec(command, {
+    await SafeExec.promises(command, {
       killSignal: KILL_SIGNAL,
       timeout: FOURTY_SECONDS,
     })

+ 19 - 13
services/filestore/app/js/FileHandler.js

@@ -1,15 +1,17 @@
-const Settings = require('@overleaf/settings')
-const { callbackify } = require('node:util')
-const fs = require('node:fs')
-let PersistorManager = require('./PersistorManager')
-const LocalFileWriter = require('./LocalFileWriter')
-const FileConverter = require('./FileConverter')
-const KeyBuilder = require('./KeyBuilder')
-const ImageOptimiser = require('./ImageOptimiser')
-const { ConversionError, InvalidParametersError } = require('./Errors')
-const metrics = require('@overleaf/metrics')
-
-module.exports = {
+import Settings from '@overleaf/settings'
+import { callbackify } from 'node:util'
+import fs from 'node:fs'
+import _PersistorManager from './PersistorManager.js'
+import LocalFileWriter from './LocalFileWriter.js'
+import FileConverter from './FileConverter.js'
+import KeyBuilder from './KeyBuilder.js'
+import ImageOptimiser from './ImageOptimiser.js'
+import Errors from './Errors.js'
+import metrics from '@overleaf/metrics'
+
+const { ConversionError, InvalidParametersError } = Errors
+
+const FileHandler = {
   insertFile: callbackify(insertFile),
   getFile: callbackify(getFile),
   getRedirectUrl: callbackify(getRedirectUrl),
@@ -22,8 +24,10 @@ module.exports = {
   },
 }
 
+let PersistorManager = _PersistorManager
+
 if (process.env.NODE_ENV === 'test') {
-  module.exports._TESTONLYSwapPersistorManager = _PersistorManager => {
+  FileHandler._TESTONLYSwapPersistorManager = _PersistorManager => {
     PersistorManager = _PersistorManager
   }
 }
@@ -183,3 +187,5 @@ async function _writeFileToDisk(bucket, key, opts) {
   const fileStream = await PersistorManager.getObjectStream(bucket, key, opts)
   return await LocalFileWriter.promises.writeStream(fileStream, key)
 }
+
+export default FileHandler

+ 6 - 6
services/filestore/app/js/ImageOptimiser.js

@@ -1,9 +1,9 @@
-const logger = require('@overleaf/logger')
-const metrics = require('@overleaf/metrics')
-const { callbackify } = require('node:util')
-const safeExec = require('./SafeExec').promises
+import logger from '@overleaf/logger'
+import metrics from '@overleaf/metrics'
+import { callbackify } from 'node:util'
+import SafeExec from './SafeExec.js'
 
-module.exports = {
+export default {
   compressPng: callbackify(compressPng),
   promises: {
     compressPng,
@@ -19,7 +19,7 @@ async function compressPng(localPath, callback) {
   }
 
   try {
-    await safeExec(args, opts)
+    await SafeExec.promises(args, opts)
     timer.done()
   } catch (err) {
     if (err.code === 'SIGKILL') {

+ 3 - 3
services/filestore/app/js/KeyBuilder.js

@@ -1,7 +1,7 @@
-const settings = require('@overleaf/settings')
-const projectKey = require('./project_key')
+import settings from '@overleaf/settings'
+import * as projectKey from './project_key.js'
 
-module.exports = {
+export default {
   getConvertedFolderKey,
   addCachingToKey,
   bucketFileKeyMiddleware,

+ 13 - 11
services/filestore/app/js/LocalFileWriter.js

@@ -1,13 +1,15 @@
-const fs = require('node:fs')
-const crypto = require('node:crypto')
-const path = require('node:path')
-const Stream = require('node:stream')
-const { callbackify, promisify } = require('node:util')
-const metrics = require('@overleaf/metrics')
-const Settings = require('@overleaf/settings')
-const { WriteError } = require('./Errors')
-
-module.exports = {
+import fs from 'node:fs'
+import crypto from 'node:crypto'
+import path from 'node:path'
+import Stream from 'node:stream'
+import { callbackify, promisify } from 'node:util'
+import metrics from '@overleaf/metrics'
+import Settings from '@overleaf/settings'
+import Errors from './Errors.js'
+
+const { WriteError } = Errors
+
+export default {
   promises: {
     writeStream,
     deleteFile,
@@ -39,7 +41,7 @@ async function deleteFile(fsPath) {
     return
   }
   try {
-    await promisify(fs.unlink)(fsPath)
+    await fs.promises.unlink(fsPath)
   } catch (err) {
     if (err.code !== 'ENOENT') {
       throw new WriteError('failed to delete file', { fsPath }, err)

+ 3 - 4
services/filestore/app/js/PersistorManager.js

@@ -1,9 +1,8 @@
-const settings = require('@overleaf/settings')
+import settings from '@overleaf/settings'
+import ObjectPersistor from '@overleaf/object-persistor'
 
 const persistorSettings = settings.filestore
 persistorSettings.paths = settings.path
-
-const ObjectPersistor = require('@overleaf/object-persistor')
 const persistor = ObjectPersistor(persistorSettings)
 
-module.exports = persistor
+export default persistor

+ 3 - 3
services/filestore/app/js/RequestLogger.js

@@ -1,5 +1,5 @@
-const logger = require('@overleaf/logger')
-const metrics = require('@overleaf/metrics')
+import logger from '@overleaf/logger'
+import metrics from '@overleaf/metrics'
 
 class RequestLogger {
   constructor() {
@@ -58,4 +58,4 @@ class RequestLogger {
   }
 }
 
-module.exports = RequestLogger
+export default RequestLogger

+ 9 - 6
services/filestore/app/js/SafeExec.js

@@ -1,7 +1,9 @@
-const lodashOnce = require('lodash.once')
-const childProcess = require('node:child_process')
-const Settings = require('@overleaf/settings')
-const { ConversionsDisabledError, FailedCommandError } = require('./Errors')
+import lodashOnce from 'lodash.once'
+import childProcess from 'node:child_process'
+import Settings from '@overleaf/settings'
+import Errors from './Errors.js'
+
+const { ConversionsDisabledError, FailedCommandError } = Errors
 
 // execute a command in the same way as 'exec' but with a timeout that
 // kills all child processes
@@ -9,8 +11,9 @@ const { ConversionsDisabledError, FailedCommandError } = require('./Errors')
 // we spawn the command with 'detached:true' to make a new process
 // group, then we can kill everything in that process group.
 
-module.exports = safeExec
-module.exports.promises = safeExecPromise
+export default safeExec
+
+safeExec.promises = safeExecPromise
 
 // options are {timeout:  number-of-milliseconds, killSignal: signal-name}
 function safeExec(command, options, callback) {

+ 3 - 6
services/filestore/app/js/project_key.js

@@ -1,23 +1,20 @@
 // Keep in sync with services/history-v1/storage/lib/project_key.js
-const path = require('node:path')
+import path from 'node:path'
 
 //
 // The advice in http://docs.aws.amazon.com/AmazonS3/latest/dev/
 // request-rate-perf-considerations.html is to avoid sequential key prefixes,
 // so we reverse the project ID part of the key as they suggest.
 //
-function format(projectId) {
+export function format(projectId) {
   const prefix = naiveReverse(pad(projectId))
   return path.join(prefix.slice(0, 3), prefix.slice(3, 6), prefix.slice(6))
 }
 
-function pad(number) {
+export function pad(number) {
   return (number || 0).toString().padStart(9, '0')
 }
 
 function naiveReverse(string) {
   return string.split('').reverse().join('')
 }
-
-exports.format = format
-exports.pad = pad

+ 2 - 0
services/filestore/buildscript.txt

@@ -8,4 +8,6 @@ filestore
 --pipeline-owner=🚉 Platform
 --public-repo=True
 --test-acceptance-shards=SHARD_01_,SHARD_02_,SHARD_03_
+--test-unit-vitest=True
+--tsconfig-extra-includes=vitest.config.unit.cjs,vitest.config.acceptance.cjs
 --use-large-ci-runner=True

+ 0 - 0
services/filestore/config/settings.defaults.js → services/filestore/config/settings.defaults.cjs


+ 2 - 0
services/filestore/docker-compose.ci.yml

@@ -11,12 +11,14 @@ services:
     user: node
     volumes:
       - ./reports:/overleaf/services/filestore/reports
+      - ../../tsconfig.backend.json:/overleaf/tsconfig.backend.json
     command: npm run test:unit:_run
     environment:
       CI:
       MONGO_CONNECTION_STRING: mongodb://mongo/test-overleaf
       NODE_ENV: test
       NODE_OPTIONS: "--unhandled-rejections=strict"
+      VITEST_NO_CACHE: true
 
 
   test_acceptance:

+ 1 - 0
services/filestore/docker-compose.yml

@@ -15,6 +15,7 @@ services:
       - .:/overleaf/services/filestore
       - ../../node_modules:/overleaf/node_modules
       - ../../libraries:/overleaf/libraries
+      - ../../tsconfig.backend.json:/overleaf/tsconfig.backend.json
     working_dir: /overleaf/services/filestore
     environment:
       MOCHA_GREP: ${MOCHA_GREP}

+ 5 - 3
services/filestore/package.json

@@ -3,18 +3,19 @@
   "description": "An API for CRUD operations on binary files stored in S3",
   "private": true,
   "main": "app.js",
+  "type": "module",
   "scripts": {
     "test:acceptance:run": "mocha --recursive --timeout 15000  $@ test/acceptance/js",
     "test:acceptance": "npm run test:acceptance:_run -- --grep=$MOCHA_GREP",
     "test:unit:run": "mocha --recursive $@ test/unit/js",
-    "test:unit": "npm run test:unit:_run -- --grep=$MOCHA_GREP",
+    "test:unit": "npm run test:unit:_run",
     "start": "node app.js",
     "nodemon": "node --watch app.js",
     "lint": "eslint --max-warnings 0 --format unix .",
     "format": "prettier --list-different $PWD/'**/{*.*js,*.ts}'",
     "format:fix": "prettier --write $PWD/'**/{*.*js,*.ts}'",
     "test:acceptance:_run": "mocha --recursive --timeout 15000 --exit $@ test/acceptance/js",
-    "test:unit:_run": "mocha --recursive --exit $@ test/unit/js",
+    "test:unit:_run": "vitest --config ./vitest.config.unit.cjs",
     "lint:fix": "eslint --fix .",
     "types:check": "tsc --noEmit"
   },
@@ -47,6 +48,7 @@
     "sinon": "9.0.2",
     "sinon-chai": "^3.7.0",
     "streamifier": "^0.1.1",
-    "typescript": "^5.0.4"
+    "typescript": "^5.0.4",
+    "vitest": "^3.2.4"
   }
 }

+ 6 - 6
services/filestore/test/acceptance/js/FilestoreApp.js

@@ -1,8 +1,8 @@
-const ObjectPersistor = require('@overleaf/object-persistor')
-const Settings = require('@overleaf/settings')
-const { promisify } = require('node:util')
-const App = require('../../../app')
-const FileHandler = require('../../../app/js/FileHandler')
+import ObjectPersistor from '@overleaf/object-persistor'
+import Settings from '@overleaf/settings'
+import { promisify } from 'node:util'
+import App from '../../../app.js'
+import FileHandler from '../../../app/js/FileHandler.js'
 
 class FilestoreApp {
   async runServer() {
@@ -39,4 +39,4 @@ class FilestoreApp {
   }
 }
 
-module.exports = FilestoreApp
+export default FilestoreApp

+ 45 - 39
services/filestore/test/acceptance/js/FilestoreTests.js

@@ -1,19 +1,48 @@
-const chai = require('chai')
+import chai from 'chai'
+import fs from 'node:fs'
+import Stream from 'node:stream'
+import Settings from '@overleaf/settings'
+import Path from 'node:path'
+import FilestoreApp from './FilestoreApp.js'
+import TestHelper from './TestHelper.js'
+import fetch from 'node-fetch'
+import { promisify } from 'node:util'
+import { Storage } from '@google-cloud/storage'
+import streamifier from 'streamifier'
+import { ObjectId } from 'mongodb'
+import { ListObjectsV2Command } from '@aws-sdk/client-s3'
+import ChildProcess from 'node:child_process'
+import chaiAsPromised from 'chai-as-promised'
+
+// store settings for multiple backends, so that we can test each one.
+// fs will always be available - add others if they are configured
+import TestConfig from './TestConfig.js'
+
+import {
+  AlreadyWrittenError,
+  NotFoundError,
+  NotImplementedError,
+  NoKEKMatchedError,
+} from '@overleaf/object-persistor/src/Errors.js'
+import {
+  PerProjectEncryptedS3Persistor,
+  RootKeyEncryptionKey,
+} from '@overleaf/object-persistor/src/PerProjectEncryptedS3Persistor.js'
+import { S3Persistor } from '@overleaf/object-persistor/src/S3Persistor.js'
+import crypto from 'node:crypto'
+import { WritableBuffer } from '@overleaf/stream-utils'
+import { gzipSync } from 'node:zlib'
+
 const { expect } = chai
-const fs = require('node:fs')
-const Stream = require('node:stream')
-const Settings = require('@overleaf/settings')
-const Path = require('node:path')
-const FilestoreApp = require('./FilestoreApp')
-const TestHelper = require('./TestHelper')
-const fetch = require('node-fetch')
-const { promisify } = require('node:util')
-const { Storage } = require('@google-cloud/storage')
-const streamifier = require('streamifier')
-chai.use(require('chai-as-promised'))
-const { ObjectId } = require('mongodb')
-const ChildProcess = require('node:child_process')
-const { ListObjectsV2Command } = require('@aws-sdk/client-s3')
+
+chai.use(chaiAsPromised)
+
+const {
+  BackendSettings,
+  s3Config,
+  s3SSECConfig,
+  AWS_S3_USER_FILES_STORAGE_CLASS,
+} = TestConfig
 
 const fsWriteFile = promisify(fs.writeFile)
 const fsStat = promisify(fs.stat)
@@ -30,29 +59,6 @@ process.on('unhandledRejection', e => {
   throw e
 })
 
-// store settings for multiple backends, so that we can test each one.
-// fs will always be available - add others if they are configured
-const {
-  BackendSettings,
-  s3Config,
-  s3SSECConfig,
-  AWS_S3_USER_FILES_STORAGE_CLASS,
-} = require('./TestConfig')
-const {
-  AlreadyWrittenError,
-  NotFoundError,
-  NotImplementedError,
-  NoKEKMatchedError,
-} = require('@overleaf/object-persistor/src/Errors')
-const {
-  PerProjectEncryptedS3Persistor,
-  RootKeyEncryptionKey,
-} = require('@overleaf/object-persistor/src/PerProjectEncryptedS3Persistor')
-const { S3Persistor } = require('@overleaf/object-persistor/src/S3Persistor')
-const crypto = require('node:crypto')
-const { WritableBuffer } = require('@overleaf/stream-utils')
-const { gzipSync } = require('node:zlib')
-
 describe('Filestore', function () {
   this.timeout(1000 * 10)
   const filestoreUrl = `http://127.0.0.1:${Settings.internal.filestore.port}`
@@ -899,7 +905,7 @@ describe('Filestore', function () {
       describe('with a pdf file', function () {
         let localFileSize
         const localFileReadPath = Path.resolve(
-          __dirname,
+          import.meta.dirname,
           '../../fixtures/test.pdf'
         )
 

+ 10 - 9
services/filestore/test/acceptance/js/TestConfig.js

@@ -1,9 +1,7 @@
-const fs = require('node:fs')
-const Path = require('node:path')
-const crypto = require('node:crypto')
-const {
-  RootKeyEncryptionKey,
-} = require('@overleaf/object-persistor/src/PerProjectEncryptedS3Persistor')
+import fs from 'node:fs'
+import Path from 'node:path'
+import crypto from 'node:crypto'
+import { RootKeyEncryptionKey } from '@overleaf/object-persistor/src/PerProjectEncryptedS3Persistor.js'
 
 const AWS_S3_USER_FILES_STORAGE_CLASS =
   process.env.AWS_S3_USER_FILES_STORAGE_CLASS
@@ -87,7 +85,10 @@ function gcsStores() {
 
 function fsStores() {
   return {
-    template_files: Path.resolve(__dirname, '../../../template_files'),
+    template_files: Path.resolve(
+      import.meta.dirname,
+      '../../../template_files'
+    ),
   }
 }
 
@@ -170,7 +171,7 @@ function checkForUnexpectedTestFile() {
     'TestConfig.js',
     'TestHelper.js',
   ]
-  for (const file of fs.readdirSync(__dirname).sort()) {
+  for (const file of fs.readdirSync(import.meta.dirname).sort()) {
     if (!awareOfSharding.includes(file)) {
       throw new Error(
         `Found new test file ${file}: All tests must be aware of the SHARD_ prefix.`
@@ -180,7 +181,7 @@ function checkForUnexpectedTestFile() {
 }
 checkForUnexpectedTestFile()
 
-module.exports = {
+export default {
   AWS_S3_USER_FILES_STORAGE_CLASS,
   BackendSettings,
   s3Config,

+ 5 - 6
services/filestore/test/acceptance/js/TestHelper.js

@@ -1,10 +1,9 @@
-const streamifier = require('streamifier')
-const fetch = require('node-fetch')
-const ObjectPersistor = require('@overleaf/object-persistor')
+import streamifier from 'streamifier'
+import fetch from 'node-fetch'
+import ObjectPersistor from '@overleaf/object-persistor'
+import { expect } from 'chai'
 
-const { expect } = require('chai')
-
-module.exports = {
+export default {
   uploadStringToPersistor,
   getStringFromPersistor,
   expectPersistorToHaveFile,

+ 6 - 37
services/filestore/test/setup.js

@@ -1,39 +1,8 @@
-const sinon = require('sinon')
-const SandboxedModule = require('sandboxed-module')
+import chai from 'chai'
+import mongodb from 'mongodb'
+import chaiAsPromised from 'chai-as-promised'
 
-// ensure every ObjectId has the id string as a property for correct comparisons
-require('mongodb').ObjectId.cacheHexString = true
-
-const sandbox = sinon.createSandbox()
-const stubs = {
-  logger: {
-    debug: sandbox.stub(),
-    log: sandbox.stub(),
-    info: sandbox.stub(),
-    warn: sandbox.stub(),
-    err: sandbox.stub(),
-    error: sandbox.stub(),
-    fatal: sandbox.stub(),
-  },
-}
-
-SandboxedModule.configure({
-  requires: {
-    '@overleaf/logger': stubs.logger,
-  },
-  sourceTransformers: {
-    removeNodePrefix: function (source) {
-      return source.replace(/require\(['"]node:/g, "require('")
-    },
-  },
-})
+chai.use(chaiAsPromised)
 
-exports.mochaHooks = {
-  beforeEach() {
-    this.logger = stubs.logger
-  },
-
-  afterEach() {
-    sandbox.reset()
-  },
-}
+// ensure every ObjectId has the id string as a property for correct comparisons
+mongodb.ObjectId.cacheHexString = true

+ 92 - 69
services/filestore/test/unit/js/FileControllerTests.js → services/filestore/test/unit/js/FileController.test.js

@@ -1,11 +1,10 @@
-const sinon = require('sinon')
-const chai = require('chai')
-const { expect } = chai
-const SandboxedModule = require('sandboxed-module')
-const Errors = require('../../../app/js/Errors')
+import sinon from 'sinon'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+import Errors from '../../../app/js/Errors.js'
+
 const modulePath = '../../../app/js/FileController.js'
 
-describe('FileController', function () {
+describe('FileController', () => {
   let FileHandler, LocalFileWriter, FileController, req, res, next, stream
   const settings = {
     s3: {
@@ -24,7 +23,7 @@ describe('FileController', function () {
   const key = `${projectId}/${fileId}`
   const error = new Error('incorrect utensil')
 
-  beforeEach(function () {
+  beforeEach(async () => {
     FileHandler = {
       getFile: sinon.stub().yields(null, fileStream),
       getFileSize: sinon.stub().yields(null, fileSize),
@@ -37,19 +36,31 @@ describe('FileController', function () {
       pipeline: sinon.stub(),
     }
 
-    FileController = SandboxedModule.require(modulePath, {
-      requires: {
-        './LocalFileWriter': LocalFileWriter,
-        './FileHandler': FileHandler,
-        './Errors': Errors,
-        stream,
-        '@overleaf/settings': settings,
-        '@overleaf/metrics': {
-          inc() {},
-        },
+    vi.doMock('../../../app/js/LocalFileWriter', () => ({
+      default: LocalFileWriter,
+    }))
+
+    vi.doMock('../../../app/js/FileHandler', () => ({
+      default: FileHandler,
+    }))
+
+    vi.doMock('../../../app/js/Errors', () => ({
+      default: Errors,
+    }))
+
+    vi.doMock('stream', () => stream)
+
+    vi.doMock('@overleaf/settings', () => ({
+      default: settings,
+    }))
+
+    vi.doMock('@overleaf/metrics', () => ({
+      default: {
+        inc() {},
       },
-      globals: { console },
-    })
+    }))
+
+    FileController = (await import(modulePath)).default
 
     req = {
       key,
@@ -76,76 +87,78 @@ describe('FileController', function () {
     next = sinon.stub()
   })
 
-  describe('getFile', function () {
-    it('should try and get a redirect url first', function () {
+  describe('getFile', () => {
+    it('should try and get a redirect url first', () => {
       FileController.getFile(req, res, next)
       expect(FileHandler.getRedirectUrl).to.have.been.calledWith(bucket, key)
     })
 
-    it('should pipe the stream', function () {
+    it('should pipe the stream', () => {
       FileController.getFile(req, res, next)
       expect(stream.pipeline).to.have.been.calledWith(fileStream, res)
     })
 
-    it('should send a 200 if the cacheWarm param is true', function (done) {
+    it('should send a 200 if the cacheWarm param is true', async () => {
       req.query.cacheWarm = true
-      res.sendStatus = statusCode => {
-        statusCode.should.equal(200)
-        done()
-      }
-      FileController.getFile(req, res, next)
+      await new Promise(resolve => {
+        res.sendStatus = statusCode => {
+          expect(statusCode).to.equal(200)
+          resolve()
+        }
+        FileController.getFile(req, res, next)
+      })
     })
 
-    it('should send an error if there is a problem', function () {
+    it('should send an error if there is a problem', () => {
       FileHandler.getFile.yields(error)
       FileController.getFile(req, res, next)
       expect(next).to.have.been.calledWith(error)
     })
 
-    describe('with a redirect url', function () {
+    describe('with a redirect url', () => {
       const redirectUrl = 'https://wombat.potato/giraffe'
 
-      beforeEach(function () {
+      beforeEach(() => {
         FileHandler.getRedirectUrl.yields(null, redirectUrl)
         res.redirect = sinon.stub()
       })
 
-      it('should redirect', function () {
+      it('should redirect', () => {
         FileController.getFile(req, res, next)
         expect(res.redirect).to.have.been.calledWith(redirectUrl)
       })
 
-      it('should not get a file stream', function () {
+      it('should not get a file stream', () => {
         FileController.getFile(req, res, next)
         expect(FileHandler.getFile).not.to.have.been.called
       })
 
-      describe('when there is an error getting the redirect url', function () {
-        beforeEach(function () {
+      describe('when there is an error getting the redirect url', () => {
+        beforeEach(() => {
           FileHandler.getRedirectUrl.yields(new Error('wombat herding error'))
         })
 
-        it('should not redirect', function () {
+        it('should not redirect', () => {
           FileController.getFile(req, res, next)
           expect(res.redirect).not.to.have.been.called
         })
 
-        it('should not return an error', function () {
+        it('should not return an error', () => {
           FileController.getFile(req, res, next)
           expect(next).not.to.have.been.called
         })
 
-        it('should proxy the file', function () {
+        it('should proxy the file', () => {
           FileController.getFile(req, res, next)
           expect(FileHandler.getFile).to.have.been.calledWith(bucket, key)
         })
       })
     })
 
-    describe('with a range header', function () {
+    describe('with a range header', () => {
       let expectedOptions
 
-      beforeEach(function () {
+      beforeEach(() => {
         expectedOptions = {
           bucket,
           key,
@@ -154,7 +167,7 @@ describe('FileController', function () {
         }
       })
 
-      it('should pass range options to FileHandler', function () {
+      it('should pass range options to FileHandler', () => {
         req.headers.range = 'bytes=0-8'
         expectedOptions.start = 0
         expectedOptions.end = 8
@@ -167,7 +180,7 @@ describe('FileController', function () {
         )
       })
 
-      it('should ignore an invalid range header', function () {
+      it('should ignore an invalid range header', () => {
         req.headers.range = 'potato'
         FileController.getFile(req, res, next)
         expect(FileHandler.getFile).to.have.been.calledWith(
@@ -177,7 +190,7 @@ describe('FileController', function () {
         )
       })
 
-      it("should ignore any type other than 'bytes'", function () {
+      it("should ignore any type other than 'bytes'", () => {
         req.headers.range = 'wombats=0-8'
         FileController.getFile(req, res, next)
         expect(FileHandler.getFile).to.have.been.calledWith(
@@ -189,31 +202,35 @@ describe('FileController', function () {
     })
   })
 
-  describe('getFileHead', function () {
-    it('should return the file size in a Content-Length header', function (done) {
-      res.end = () => {
-        expect(res.status).to.have.been.calledWith(200)
-        expect(res.set).to.have.been.calledWith('Content-Length', fileSize)
-        done()
-      }
+  describe('getFileHead', () => {
+    it('should return the file size in a Content-Length header', async () => {
+      await new Promise(resolve => {
+        res.end = () => {
+          expect(res.status).to.have.been.calledWith(200)
+          expect(res.set).to.have.been.calledWith('Content-Length', fileSize)
+          resolve()
+        }
 
-      FileController.getFileHead(req, res, next)
+        FileController.getFileHead(req, res, next)
+      })
     })
 
-    it('should return a 404 is the file is not found', function (done) {
-      FileHandler.getFileSize.yields(
-        new Errors.NotFoundError({ message: 'not found', info: {} })
-      )
+    it('should return a 404 is the file is not found', async () => {
+      await new Promise(resolve => {
+        FileHandler.getFileSize.yields(
+          new Errors.NotFoundError({ message: 'not found', info: {} })
+        )
 
-      res.sendStatus = code => {
-        expect(code).to.equal(404)
-        done()
-      }
+        res.sendStatus = code => {
+          expect(code).to.equal(404)
+          resolve()
+        }
 
-      FileController.getFileHead(req, res, next)
+        FileController.getFileHead(req, res, next)
+      })
     })
 
-    it('should send an error on internal errors', function () {
+    it('should send an error on internal errors', () => {
       FileHandler.getFileSize.yields(error)
 
       FileController.getFileHead(req, res, next)
@@ -221,14 +238,20 @@ describe('FileController', function () {
     })
   })
 
-  describe('insertFile', function () {
-    it('should send bucket name key and res to FileHandler', function (done) {
-      res.sendStatus = code => {
-        expect(FileHandler.insertFile).to.have.been.calledWith(bucket, key, req)
-        expect(code).to.equal(200)
-        done()
-      }
-      FileController.insertFile(req, res, next)
+  describe('insertFile', () => {
+    it('should send bucket name key and res to FileHandler', async () => {
+      await new Promise(resolve => {
+        res.sendStatus = code => {
+          expect(FileHandler.insertFile).to.have.been.calledWith(
+            bucket,
+            key,
+            req
+          )
+          expect(code).to.equal(200)
+          resolve()
+        }
+        FileController.insertFile(req, res, next)
+      })
     })
   })
 })

+ 45 - 36
services/filestore/test/unit/js/FileConverterTests.js → services/filestore/test/unit/js/FileConverter.test.js

@@ -1,12 +1,10 @@
-const sinon = require('sinon')
-const chai = require('chai')
-const { expect } = chai
-const SandboxedModule = require('sandboxed-module')
-const { Errors } = require('@overleaf/object-persistor')
+import sinon from 'sinon'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+import _ObjectPersistor, { Errors } from '@overleaf/object-persistor'
 
 const modulePath = '../../../app/js/FileConverter.js'
 
-describe('FileConverter', function () {
+describe('FileConverter', () => {
   let SafeExec, FileConverter
   const sourcePath = '/data/wombat.eps'
   const destPath = '/tmp/dest.png'
@@ -18,40 +16,53 @@ describe('FileConverter', function () {
     },
   }
 
-  beforeEach(function () {
+  beforeEach(async () => {
     SafeExec = {
       promises: sinon.stub().resolves(destPath),
     }
 
     const ObjectPersistor = { Errors }
 
-    FileConverter = SandboxedModule.require(modulePath, {
-      requires: {
-        './SafeExec': SafeExec,
-        '@overleaf/metrics': {
-          inc: sinon.stub(),
-          Timer: sinon.stub().returns({ done: sinon.stub() }),
-        },
-        '@overleaf/settings': Settings,
-        '@overleaf/object-persistor': ObjectPersistor,
+    vi.doMock('../../../app/js/SafeExec', () => ({
+      default: SafeExec,
+    }))
+
+    vi.doMock('@overleaf/metrics', () => ({
+      default: {
+        inc: sinon.stub(),
+        Timer: sinon.stub().returns({ done: sinon.stub() }),
       },
+    }))
+
+    vi.doMock('@overleaf/settings', async importOriginal => {
+      const originalModule = (await importOriginal()).default
+      return {
+        default: { ...originalModule, ...Settings },
+      }
     })
+
+    vi.doMock('@overleaf/object-persistor', () => ({
+      ...ObjectPersistor,
+      default: _ObjectPersistor,
+    }))
+
+    FileConverter = (await import(modulePath)).default
   })
 
-  describe('convert', function () {
-    it('should convert the source to the requested format', async function () {
+  describe('convert', () => {
+    it('should convert the source to the requested format', async () => {
       await FileConverter.promises.convert(sourcePath, format)
       const args = SafeExec.promises.args[0][0]
       expect(args).to.include(`${sourcePath}[0]`)
       expect(args).to.include(`${sourcePath}.${format}`)
     })
 
-    it('should return the dest path', async function () {
+    it('should return the dest path', async () => {
       const destPath = await FileConverter.promises.convert(sourcePath, format)
-      destPath.should.equal(`${sourcePath}.${format}`)
+      expect(destPath).to.equal(`${sourcePath}.${format}`)
     })
 
-    it('should wrap the error from convert', async function () {
+    it('should wrap the error from convert', async () => {
       SafeExec.promises.rejects(errorMessage)
       try {
         await FileConverter.promises.convert(sourcePath, format)
@@ -62,7 +73,7 @@ describe('FileConverter', function () {
       }
     })
 
-    it('should not accept an non approved format', async function () {
+    it('should not accept an non approved format', async () => {
       try {
         await FileConverter.promises.convert(sourcePath, 'potato')
         expect('error should have been thrown').not.to.exist
@@ -71,34 +82,32 @@ describe('FileConverter', function () {
       }
     })
 
-    it('should prefix the command with Settings.commands.convertCommandPrefix', async function () {
+    it('should prefix the command with Settings.commands.convertCommandPrefix', async () => {
       Settings.commands.convertCommandPrefix = ['nice']
       await FileConverter.promises.convert(sourcePath, format)
     })
 
-    it('should convert the file when called as a callback', function (done) {
-      FileConverter.convert(sourcePath, format, (err, destPath) => {
-        expect(err).not.to.exist
-        destPath.should.equal(`${sourcePath}.${format}`)
+    it('should convert the file when called as a callback', async () => {
+      const destPath = await FileConverter.promises.convert(sourcePath, format)
+
+      expect(destPath).to.equal(`${sourcePath}.${format}`)
 
-        const args = SafeExec.promises.args[0][0]
-        expect(args).to.include(`${sourcePath}[0]`)
-        expect(args).to.include(`${sourcePath}.${format}`)
-        done()
-      })
+      const args = SafeExec.promises.args[0][0]
+      expect(args).to.include(`${sourcePath}[0]`)
+      expect(args).to.include(`${sourcePath}.${format}`)
     })
   })
 
-  describe('thumbnail', function () {
-    it('should call converter resize with args', async function () {
+  describe('thumbnail', () => {
+    it('should call converter resize with args', async () => {
       await FileConverter.promises.thumbnail(sourcePath)
       const args = SafeExec.promises.args[0][0]
       expect(args).to.include(`${sourcePath}[0]`)
     })
   })
 
-  describe('preview', function () {
-    it('should call converter resize with args', async function () {
+  describe('preview', () => {
+    it('should call converter resize with args', async () => {
       await FileConverter.promises.preview(sourcePath)
       const args = SafeExec.promises.args[0][0]
       expect(args).to.include(`${sourcePath}[0]`)

+ 283 - 0
services/filestore/test/unit/js/FileHandler.test.js

@@ -0,0 +1,283 @@
+import sinon from 'sinon'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+import { ObjectId } from 'mongodb'
+import OriginalSettings from '@overleaf/settings'
+
+const modulePath = '../../../app/js/FileHandler.js'
+
+describe('FileHandler', () => {
+  let PersistorManager,
+    LocalFileWriter,
+    FileConverter,
+    KeyBuilder,
+    ImageOptimiser,
+    FileHandler,
+    Settings,
+    fs
+
+  const bucket = 'my_bucket'
+  const key = `${new ObjectId()}/${new ObjectId()}`
+  const convertedFolderKey = `${new ObjectId()}/${new ObjectId()}`
+  const sourceStream = 'sourceStream'
+  const convertedKey = 'convertedKey'
+  const redirectUrl = 'https://wombat.potato/giraffe'
+  const readStream = {
+    stream: 'readStream',
+    on: sinon.stub(),
+  }
+
+  beforeEach(async () => {
+    PersistorManager = {
+      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(),
+    }
+    LocalFileWriter = {
+      // the callback style is used for detached cleanup calls
+      deleteFile: sinon.stub().yields(),
+      promises: {
+        writeStream: sinon.stub().resolves(),
+        deleteFile: sinon.stub().resolves(),
+      },
+    }
+    FileConverter = {
+      promises: {
+        convert: sinon.stub().resolves(),
+        thumbnail: sinon.stub().resolves(),
+        preview: sinon.stub().resolves(),
+      },
+    }
+    KeyBuilder = {
+      addCachingToKey: sinon.stub().returns(convertedKey),
+      getConvertedFolderKey: sinon.stub().returns(convertedFolderKey),
+    }
+    ImageOptimiser = {
+      promises: {
+        compressPng: sinon.stub().resolves(),
+      },
+    }
+    Settings = {
+      ...OriginalSettings,
+      filestore: {
+        stores: {
+          ...(OriginalSettings.filestore?.stores ?? {}),
+          template_files: 'template_files',
+        },
+      },
+    }
+    fs = {
+      createReadStream: sinon.stub().returns(readStream),
+    }
+
+    vi.doMock('../../../app/js/PersistorManager', () => ({
+      default: PersistorManager,
+    }))
+
+    vi.doMock('../../../app/js/LocalFileWriter', () => ({
+      default: LocalFileWriter,
+    }))
+
+    vi.doMock('../../../app/js/FileConverter', () => ({
+      default: FileConverter,
+    }))
+
+    vi.doMock('../../../app/js/KeyBuilder', () => ({
+      default: KeyBuilder,
+    }))
+
+    vi.doMock('../../../app/js/ImageOptimiser', () => ({
+      default: ImageOptimiser,
+    }))
+
+    vi.doMock('@overleaf/settings', () => {
+      return {
+        default: Settings,
+      }
+    })
+
+    vi.doMock('@overleaf/metrics', () => ({
+      default: {
+        gauge: sinon.stub(),
+        Timer: sinon.stub().returns({ done: sinon.stub() }),
+      },
+    }))
+
+    vi.doMock('node:fs', () => ({
+      default: fs,
+    }))
+
+    FileHandler = (await import(modulePath)).default
+    FileHandler._TESTONLYSwapPersistorManager(PersistorManager)
+  })
+
+  describe('insertFile', () => {
+    const stream = 'stream'
+
+    it('should send file to the filestore', async () => {
+      await FileHandler.promises.insertFile(bucket, key, stream)
+      expect(PersistorManager.sendStream).to.have.been.calledWith(
+        bucket,
+        key,
+        stream
+      )
+    })
+
+    it('should not make a delete request for the convertedKey folder', async () => {
+      await FileHandler.promises.insertFile(bucket, key, stream)
+      expect(PersistorManager.deleteDirectory).not.to.have.been.called
+    })
+
+    it('should accept templates-api key format', async () => {
+      KeyBuilder.getConvertedFolderKey.returns(
+        '5ecba29f1a294e007d0bccb4/v/0/pdf'
+      )
+      await FileHandler.promises.insertFile(bucket, key, stream)
+    })
+
+    it('should throw an error when the key is in the wrong format', async () => {
+      KeyBuilder.getConvertedFolderKey.returns('wombat')
+      expect(FileHandler.promises.insertFile(bucket, key, stream)).to.be
+        .rejected
+    })
+  })
+
+  describe('getFile', () => {
+    it('should return the source stream no format or style are defined', async () => {
+      const stream = await FileHandler.promises.getFile(bucket, key, null)
+      expect(stream).to.equal(sourceStream)
+    })
+
+    it('should pass options through to PersistorManager', async () => {
+      const options = { start: 0, end: 8 }
+      await FileHandler.promises.getFile(bucket, key, options)
+      expect(PersistorManager.getObjectStream).to.have.been.calledWith(
+        bucket,
+        key,
+        options
+      )
+    })
+
+    describe('when a format is defined', () => {
+      let result
+
+      describe('when the file is not cached', () => {
+        beforeEach(async () => {
+          const stream = await FileHandler.promises.getFile(bucket, key, {
+            format: 'png',
+          })
+          result = { stream }
+        })
+
+        it('should convert the file', () => {
+          expect(FileConverter.promises.convert).to.have.been.called
+        })
+
+        it('should compress the converted file', () => {
+          expect(ImageOptimiser.promises.compressPng).to.have.been.called
+        })
+
+        it('should return the the converted stream', () => {
+          expect(result.stream).to.equal(readStream)
+          expect(PersistorManager.getObjectStream).to.have.been.calledWith(
+            bucket,
+            key
+          )
+        })
+      })
+
+      describe('when the file is cached', () => {
+        beforeEach(async () => {
+          PersistorManager.checkIfObjectExists = sinon.stub().resolves(true)
+          const stream = await FileHandler.promises.getFile(bucket, key, {
+            format: 'png',
+          })
+          result = { stream }
+        })
+
+        it('should not convert the file', () => {
+          expect(FileConverter.promises.convert).not.to.have.been.called
+        })
+
+        it('should not compress the converted file again', () => {
+          expect(ImageOptimiser.promises.compressPng).not.to.have.been.called
+        })
+
+        it('should return the cached stream', () => {
+          expect(result.stream).to.equal(sourceStream)
+          expect(PersistorManager.getObjectStream).to.have.been.calledWith(
+            bucket,
+            convertedKey
+          )
+        })
+      })
+    })
+
+    describe('when a style is defined', () => {
+      it('generates a thumbnail when requested', async () => {
+        await FileHandler.promises.getFile(bucket, key, { style: 'thumbnail' })
+        expect(FileConverter.promises.thumbnail).to.have.been.called
+        expect(FileConverter.promises.preview).not.to.have.been.called
+      })
+
+      it('generates a preview when requested', async () => {
+        await FileHandler.promises.getFile(bucket, key, { style: 'preview' })
+        expect(FileConverter.promises.thumbnail).not.to.have.been.called
+        expect(FileConverter.promises.preview).to.have.been.called
+      })
+    })
+  })
+
+  describe('getRedirectUrl', () => {
+    beforeEach(() => {
+      Settings.filestore = {
+        ...OriginalSettings.filestore,
+        allowRedirects: true,
+        stores: {
+          ...OriginalSettings.filestore.stores,
+          userFiles: bucket,
+        },
+      }
+    })
+
+    it('should return a redirect url', async () => {
+      const url = await FileHandler.promises.getRedirectUrl(bucket, key)
+      expect(url).to.equal(redirectUrl)
+    })
+
+    it('should call the persistor to get a redirect url', async () => {
+      await FileHandler.promises.getRedirectUrl(bucket, key)
+      expect(PersistorManager.getRedirectUrl).to.have.been.calledWith(
+        bucket,
+        key
+      )
+    })
+
+    it('should return null if options are supplied', async () => {
+      const url = await FileHandler.promises.getRedirectUrl(bucket, key, {
+        start: 100,
+        end: 200,
+      })
+      expect(url).to.be.null
+    })
+
+    it('should return null if the bucket is not one of the defined ones', async () => {
+      const url = await FileHandler.promises.getRedirectUrl(
+        'a_different_bucket',
+        key
+      )
+      expect(url).to.be.null
+    })
+
+    it('should return null if redirects are not enabled', async () => {
+      Settings.filestore.allowRedirects = false
+
+      const url = await FileHandler.promises.getRedirectUrl(bucket, key)
+      expect(url).to.be.null
+    })
+  })
+})

+ 0 - 298
services/filestore/test/unit/js/FileHandlerTests.js

@@ -1,298 +0,0 @@
-const sinon = require('sinon')
-const chai = require('chai')
-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'))
-
-describe('FileHandler', function () {
-  let PersistorManager,
-    LocalFileWriter,
-    FileConverter,
-    KeyBuilder,
-    ImageOptimiser,
-    FileHandler,
-    Settings,
-    fs
-
-  const bucket = 'my_bucket'
-  const key = `${new ObjectId()}/${new ObjectId()}`
-  const convertedFolderKey = `${new ObjectId()}/${new ObjectId()}`
-  const sourceStream = 'sourceStream'
-  const convertedKey = 'convertedKey'
-  const redirectUrl = 'https://wombat.potato/giraffe'
-  const readStream = {
-    stream: 'readStream',
-    on: sinon.stub(),
-  }
-
-  beforeEach(function () {
-    PersistorManager = {
-      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(),
-    }
-    LocalFileWriter = {
-      // the callback style is used for detached cleanup calls
-      deleteFile: sinon.stub().yields(),
-      promises: {
-        writeStream: sinon.stub().resolves(),
-        deleteFile: sinon.stub().resolves(),
-      },
-    }
-    FileConverter = {
-      promises: {
-        convert: sinon.stub().resolves(),
-        thumbnail: sinon.stub().resolves(),
-        preview: sinon.stub().resolves(),
-      },
-    }
-    KeyBuilder = {
-      addCachingToKey: sinon.stub().returns(convertedKey),
-      getConvertedFolderKey: sinon.stub().returns(convertedFolderKey),
-    }
-    ImageOptimiser = {
-      promises: {
-        compressPng: sinon.stub().resolves(),
-      },
-    }
-    Settings = {
-      filestore: {
-        stores: { template_files: 'template_files' },
-      },
-    }
-    fs = {
-      createReadStream: sinon.stub().returns(readStream),
-    }
-
-    const ObjectPersistor = { Errors }
-
-    FileHandler = SandboxedModule.require(modulePath, {
-      requires: {
-        './PersistorManager': PersistorManager,
-        './LocalFileWriter': LocalFileWriter,
-        './FileConverter': FileConverter,
-        './KeyBuilder': KeyBuilder,
-        './ImageOptimiser': ImageOptimiser,
-        '@overleaf/settings': Settings,
-        '@overleaf/object-persistor': ObjectPersistor,
-        '@overleaf/metrics': {
-          gauge: sinon.stub(),
-          Timer: sinon.stub().returns({ done: sinon.stub() }),
-        },
-        fs,
-      },
-      globals: { console, process },
-    })
-  })
-
-  describe('insertFile', function () {
-    const stream = 'stream'
-
-    it('should send file to the filestore', function (done) {
-      FileHandler.insertFile(bucket, key, stream, err => {
-        expect(err).not.to.exist
-        expect(PersistorManager.sendStream).to.have.been.calledWith(
-          bucket,
-          key,
-          stream
-        )
-        done()
-      })
-    })
-
-    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.deleteDirectory).not.to.have.been.called
-        done()
-      })
-    })
-
-    it('should accept templates-api key format', function (done) {
-      KeyBuilder.getConvertedFolderKey.returns(
-        '5ecba29f1a294e007d0bccb4/v/0/pdf'
-      )
-      FileHandler.insertFile(bucket, key, stream, err => {
-        expect(err).not.to.exist
-        done()
-      })
-    })
-
-    it('should throw an error when the key is in the wrong format', function (done) {
-      KeyBuilder.getConvertedFolderKey.returns('wombat')
-      FileHandler.insertFile(bucket, key, stream, err => {
-        expect(err).to.exist
-        done()
-      })
-    })
-  })
-
-  describe('getFile', function () {
-    it('should return the source stream no format or style are defined', function (done) {
-      FileHandler.getFile(bucket, key, null, (err, stream) => {
-        expect(err).not.to.exist
-        expect(stream).to.equal(sourceStream)
-        done()
-      })
-    })
-
-    it('should pass options through to PersistorManager', function (done) {
-      const options = { start: 0, end: 8 }
-      FileHandler.getFile(bucket, key, options, err => {
-        expect(err).not.to.exist
-        expect(PersistorManager.getObjectStream).to.have.been.calledWith(
-          bucket,
-          key,
-          options
-        )
-        done()
-      })
-    })
-
-    describe('when a format is defined', function () {
-      let result
-
-      describe('when the file is not cached', function () {
-        beforeEach(function (done) {
-          FileHandler.getFile(bucket, key, { format: 'png' }, (err, stream) => {
-            result = { err, stream }
-            done()
-          })
-        })
-
-        it('should convert the file', function () {
-          expect(FileConverter.promises.convert).to.have.been.called
-        })
-
-        it('should compress the converted file', function () {
-          expect(ImageOptimiser.promises.compressPng).to.have.been.called
-        })
-
-        it('should return the the converted stream', function () {
-          expect(result.err).not.to.exist
-          expect(result.stream).to.equal(readStream)
-          expect(PersistorManager.getObjectStream).to.have.been.calledWith(
-            bucket,
-            key
-          )
-        })
-      })
-
-      describe('when the file is cached', function () {
-        beforeEach(function (done) {
-          PersistorManager.checkIfObjectExists = sinon.stub().resolves(true)
-          FileHandler.getFile(bucket, key, { format: 'png' }, (err, stream) => {
-            result = { err, stream }
-            done()
-          })
-        })
-
-        it('should not convert the file', function () {
-          expect(FileConverter.promises.convert).not.to.have.been.called
-        })
-
-        it('should not compress the converted file again', function () {
-          expect(ImageOptimiser.promises.compressPng).not.to.have.been.called
-        })
-
-        it('should return the cached stream', function () {
-          expect(result.err).not.to.exist
-          expect(result.stream).to.equal(sourceStream)
-          expect(PersistorManager.getObjectStream).to.have.been.calledWith(
-            bucket,
-            convertedKey
-          )
-        })
-      })
-    })
-
-    describe('when a style is defined', function () {
-      it('generates a thumbnail when requested', function (done) {
-        FileHandler.getFile(bucket, key, { style: 'thumbnail' }, err => {
-          expect(err).not.to.exist
-          expect(FileConverter.promises.thumbnail).to.have.been.called
-          expect(FileConverter.promises.preview).not.to.have.been.called
-          done()
-        })
-      })
-
-      it('generates a preview when requested', function (done) {
-        FileHandler.getFile(bucket, key, { style: 'preview' }, err => {
-          expect(err).not.to.exist
-          expect(FileConverter.promises.thumbnail).not.to.have.been.called
-          expect(FileConverter.promises.preview).to.have.been.called
-          done()
-        })
-      })
-    })
-  })
-
-  describe('getRedirectUrl', function () {
-    beforeEach(function () {
-      Settings.filestore = {
-        allowRedirects: true,
-        stores: {
-          userFiles: bucket,
-        },
-      }
-    })
-
-    it('should return a redirect url', function (done) {
-      FileHandler.getRedirectUrl(bucket, key, (err, url) => {
-        expect(err).not.to.exist
-        expect(url).to.equal(redirectUrl)
-        done()
-      })
-    })
-
-    it('should call the persistor to get a redirect url', function (done) {
-      FileHandler.getRedirectUrl(bucket, key, () => {
-        expect(PersistorManager.getRedirectUrl).to.have.been.calledWith(
-          bucket,
-          key
-        )
-        done()
-      })
-    })
-
-    it('should return null if options are supplied', function (done) {
-      FileHandler.getRedirectUrl(
-        bucket,
-        key,
-        { start: 100, end: 200 },
-        (err, url) => {
-          expect(err).not.to.exist
-          expect(url).to.be.null
-          done()
-        }
-      )
-    })
-
-    it('should return null if the bucket is not one of the defined ones', function (done) {
-      FileHandler.getRedirectUrl('a_different_bucket', key, (err, url) => {
-        expect(err).not.to.exist
-        expect(url).to.be.null
-        done()
-      })
-    })
-
-    it('should return null if redirects are not enabled', function (done) {
-      Settings.filestore.allowRedirects = false
-      FileHandler.getRedirectUrl(bucket, key, (err, url) => {
-        expect(err).not.to.exist
-        expect(url).to.be.null
-        done()
-      })
-    })
-  })
-})

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

@@ -0,0 +1,78 @@
+import sinon from 'sinon'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+import Errors from '../../../app/js/Errors.js'
+
+const { FailedCommandError } = Errors
+
+const modulePath = '../../../app/js/ImageOptimiser.js'
+
+describe('ImageOptimiser', function () {
+  let ImageOptimiser, SafeExec
+  const sourcePath = '/wombat/potato.eps'
+
+  beforeEach(async function () {
+    SafeExec = {
+      promises: sinon.stub().resolves(),
+    }
+
+    vi.doMock('../../../app/js/SafeExec', () => ({
+      default: SafeExec,
+    }))
+
+    vi.doMock('@overleaf/metrics', () => ({
+      default: {
+        Timer: sinon.stub().returns({ done: sinon.stub() }),
+      },
+    }))
+
+    ImageOptimiser = (await import(modulePath)).default
+  })
+
+  describe('compressPng', function () {
+    it('should convert the file', async function () {
+      await new Promise(resolve => {
+        ImageOptimiser.compressPng(sourcePath, err => {
+          expect(err).not.to.exist
+          expect(SafeExec.promises).to.have.been.calledWith([
+            'optipng',
+            sourcePath,
+          ])
+          resolve()
+        })
+      })
+    })
+
+    it('should return the error', async function () {
+      await new Promise(resolve => {
+        SafeExec.promises.rejects('wombat herding failure')
+        ImageOptimiser.compressPng(sourcePath, err => {
+          expect(err.toString()).to.equal('wombat herding failure')
+          resolve()
+        })
+      })
+    })
+  })
+
+  describe('when optimiser is sigkilled', function () {
+    const expectedError = new FailedCommandError('', 'SIGKILL', '', '')
+    let error
+
+    beforeEach(async function () {
+      await new Promise(resolve => {
+        SafeExec.promises.rejects(expectedError)
+        ImageOptimiser.compressPng(sourcePath, err => {
+          error = err
+          resolve()
+        })
+      })
+    })
+
+    it('should not produce an error', function () {
+      expect(error).not.to.exist
+    })
+
+    it('should log a warning', function (ctx) {
+      expect(ctx.logger.warn).to.have.been.calledOnce
+    })
+  })
+})

+ 0 - 67
services/filestore/test/unit/js/ImageOptimiserTests.js

@@ -1,67 +0,0 @@
-const sinon = require('sinon')
-const chai = require('chai')
-const { expect } = chai
-const modulePath = '../../../app/js/ImageOptimiser.js'
-const { FailedCommandError } = require('../../../app/js/Errors')
-const SandboxedModule = require('sandboxed-module')
-
-describe('ImageOptimiser', function () {
-  let ImageOptimiser, SafeExec
-  const sourcePath = '/wombat/potato.eps'
-
-  beforeEach(function () {
-    SafeExec = {
-      promises: sinon.stub().resolves(),
-    }
-    ImageOptimiser = SandboxedModule.require(modulePath, {
-      requires: {
-        './SafeExec': SafeExec,
-        '@overleaf/metrics': {
-          Timer: sinon.stub().returns({ done: sinon.stub() }),
-        },
-      },
-    })
-  })
-
-  describe('compressPng', function () {
-    it('should convert the file', function (done) {
-      ImageOptimiser.compressPng(sourcePath, err => {
-        expect(err).not.to.exist
-        expect(SafeExec.promises).to.have.been.calledWith([
-          'optipng',
-          sourcePath,
-        ])
-        done()
-      })
-    })
-
-    it('should return the error', function (done) {
-      SafeExec.promises.rejects('wombat herding failure')
-      ImageOptimiser.compressPng(sourcePath, err => {
-        expect(err.toString()).to.equal('wombat herding failure')
-        done()
-      })
-    })
-  })
-
-  describe('when optimiser is sigkilled', function () {
-    const expectedError = new FailedCommandError('', 'SIGKILL', '', '')
-    let error
-
-    beforeEach(function (done) {
-      SafeExec.promises.rejects(expectedError)
-      ImageOptimiser.compressPng(sourcePath, err => {
-        error = err
-        done()
-      })
-    })
-
-    it('should not produce an error', function () {
-      expect(error).not.to.exist
-    })
-
-    it('should log a warning', function () {
-      expect(this.logger.warn).to.have.been.calledOnce
-    })
-  })
-})

+ 12 - 8
services/filestore/test/unit/js/KeybuilderTests.js → services/filestore/test/unit/js/Keybuilder.test.js

@@ -1,4 +1,4 @@
-const SandboxedModule = require('sandboxed-module')
+import { beforeEach, describe, expect, it, vi } from 'vitest'
 
 const modulePath = '../../../app/js/KeyBuilder.js'
 
@@ -6,23 +6,25 @@ describe('KeybuilderTests', function () {
   let KeyBuilder
   const key = 'wombat/potato'
 
-  beforeEach(function () {
-    KeyBuilder = SandboxedModule.require(modulePath, {
-      requires: { '@overleaf/settings': {} },
-    })
+  beforeEach(async function () {
+    vi.doMock('@overleaf/settings', () => ({
+      default: {},
+    }))
+
+    KeyBuilder = (await import(modulePath)).default
   })
 
   describe('cachedKey', function () {
     it('should add the format to the key', function () {
       const opts = { format: 'png' }
       const newKey = KeyBuilder.addCachingToKey(key, opts)
-      newKey.should.equal(`${key}-converted-cache/format-png`)
+      expect(newKey).to.equal(`${key}-converted-cache/format-png`)
     })
 
     it('should add the style to the key', function () {
       const opts = { style: 'thumbnail' }
       const newKey = KeyBuilder.addCachingToKey(key, opts)
-      newKey.should.equal(`${key}-converted-cache/style-thumbnail`)
+      expect(newKey).to.equal(`${key}-converted-cache/style-thumbnail`)
     })
 
     it('should add format first, then style', function () {
@@ -31,7 +33,9 @@ describe('KeybuilderTests', function () {
         format: 'png',
       }
       const newKey = KeyBuilder.addCachingToKey(key, opts)
-      newKey.should.equal(`${key}-converted-cache/format-png-style-thumbnail`)
+      expect(newKey).to.equal(
+        `${key}-converted-cache/format-png-style-thumbnail`
+      )
     })
   })
 })

+ 115 - 0
services/filestore/test/unit/js/LocalFileWriter.test.js

@@ -0,0 +1,115 @@
+import sinon from 'sinon'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+import { Errors } from '@overleaf/object-persistor'
+
+const modulePath = '../../../app/js/LocalFileWriter.js'
+
+describe('LocalFileWriter', function () {
+  const writeStream = 'writeStream'
+  const readStream = 'readStream'
+  const settings = { path: { uploadFolder: '/uploads' } }
+  const fsPath = '/uploads/wombat'
+  const filename = 'wombat'
+  let stream, fs, LocalFileWriter
+
+  beforeEach(async function () {
+    fs = {
+      createWriteStream: sinon.stub().returns(writeStream),
+      promises: {
+        unlink: sinon.stub().resolves(),
+      },
+    }
+    stream = {
+      pipeline: sinon.stub().yields(),
+    }
+
+    const ObjectPersistor = { Errors }
+
+    vi.doMock('fs', () => ({
+      default: fs,
+    }))
+
+    vi.doMock('stream', () => ({
+      default: stream,
+    }))
+
+    vi.doMock('@overleaf/settings', () => ({
+      default: settings,
+    }))
+
+    vi.doMock('@overleaf/metrics', () => ({
+      default: {
+        inc: sinon.stub(),
+        Timer: sinon.stub().returns({ done: sinon.stub() }),
+      },
+    }))
+
+    vi.doMock('@overleaf/object-persistor', () => ObjectPersistor)
+
+    LocalFileWriter = (await import(modulePath)).default
+  })
+
+  describe('writeStream', function () {
+    it('writes the stream to the upload folder', async function () {
+      await new Promise(resolve => {
+        LocalFileWriter.writeStream(readStream, filename, (err, path) => {
+          expect(err).not.to.exist
+          expect(fs.createWriteStream).to.have.been.calledWith(fsPath)
+          expect(stream.pipeline).to.have.been.calledWith(
+            readStream,
+            writeStream
+          )
+          expect(path).to.equal(fsPath)
+          resolve()
+        })
+      })
+    })
+
+    describe('when there is an error', function () {
+      const error = new Error('not enough ketchup')
+      beforeEach(function () {
+        stream.pipeline.yields(error)
+      })
+
+      it('should wrap the error', async function () {
+        await expect(
+          LocalFileWriter.promises.writeStream(readStream, filename)
+        ).to.be.rejected.and.eventually.have.property('cause', error)
+      })
+
+      it('should delete the temporary file', async function () {
+        await expect(LocalFileWriter.promises.writeStream(readStream, filename))
+          .to.be.rejected
+        expect(fs.promises.unlink).to.have.been.calledWith(fsPath)
+      })
+    })
+  })
+
+  describe('deleteFile', function () {
+    it('should unlink the file', async function () {
+      await LocalFileWriter.promises.deleteFile(fsPath)
+      expect(fs.promises.unlink).to.have.been.calledWith(fsPath)
+    })
+
+    it('should not call unlink with an empty path', async function () {
+      await LocalFileWriter.promises.deleteFile('')
+
+      expect(fs.promises.unlink).not.to.have.been.called
+    })
+
+    it('should not throw a error if the file does not exist', async function () {
+      const error = new Error('file not found')
+      error.code = 'ENOENT'
+      fs.promises.unlink = sinon.stub().rejects(error)
+      await LocalFileWriter.promises.deleteFile(fsPath)
+    })
+
+    it('should wrap the error', async function () {
+      const error = new Error('failed to reticulate splines')
+      fs.promises.unlink = sinon.stub().rejects(error)
+      await expect(
+        LocalFileWriter.promises.deleteFile(fsPath)
+      ).to.be.rejectedWith(Errors.WriteError)
+    })
+  })
+})

+ 0 - 111
services/filestore/test/unit/js/LocalFileWriterTests.js

@@ -1,111 +0,0 @@
-const sinon = require('sinon')
-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 () {
-  const writeStream = 'writeStream'
-  const readStream = 'readStream'
-  const settings = { path: { uploadFolder: '/uploads' } }
-  const fsPath = '/uploads/wombat'
-  const filename = 'wombat'
-  let stream, fs, LocalFileWriter
-
-  beforeEach(function () {
-    fs = {
-      createWriteStream: sinon.stub().returns(writeStream),
-      unlink: sinon.stub().yields(),
-    }
-    stream = {
-      pipeline: sinon.stub().yields(),
-    }
-
-    const ObjectPersistor = { Errors }
-
-    LocalFileWriter = SandboxedModule.require(modulePath, {
-      requires: {
-        fs,
-        stream,
-        '@overleaf/settings': settings,
-        '@overleaf/metrics': {
-          inc: sinon.stub(),
-          Timer: sinon.stub().returns({ done: sinon.stub() }),
-        },
-        '@overleaf/object-persistor': ObjectPersistor,
-      },
-    })
-  })
-
-  describe('writeStream', function () {
-    it('writes the stream to the upload folder', function (done) {
-      LocalFileWriter.writeStream(readStream, filename, (err, path) => {
-        expect(err).not.to.exist
-        expect(fs.createWriteStream).to.have.been.calledWith(fsPath)
-        expect(stream.pipeline).to.have.been.calledWith(readStream, writeStream)
-        expect(path).to.equal(fsPath)
-        done()
-      })
-    })
-
-    describe('when there is an error', function () {
-      const error = new Error('not enough ketchup')
-      beforeEach(function () {
-        stream.pipeline.yields(error)
-      })
-
-      it('should wrap the error', function () {
-        LocalFileWriter.writeStream(readStream, filename, err => {
-          expect(err).to.exist
-          expect(err.cause).to.equal(error)
-        })
-      })
-
-      it('should delete the temporary file', function () {
-        LocalFileWriter.writeStream(readStream, filename, () => {
-          expect(fs.unlink).to.have.been.calledWith(fsPath)
-        })
-      })
-    })
-  })
-
-  describe('deleteFile', function () {
-    it('should unlink the file', function (done) {
-      LocalFileWriter.deleteFile(fsPath, err => {
-        expect(err).not.to.exist
-        expect(fs.unlink).to.have.been.calledWith(fsPath)
-        done()
-      })
-    })
-
-    it('should not call unlink with an empty path', function (done) {
-      LocalFileWriter.deleteFile('', err => {
-        expect(err).not.to.exist
-        expect(fs.unlink).not.to.have.been.called
-        done()
-      })
-    })
-
-    it('should not throw a error if the file does not exist', function (done) {
-      const error = new Error('file not found')
-      error.code = 'ENOENT'
-      fs.unlink = sinon.stub().yields(error)
-      LocalFileWriter.deleteFile(fsPath, err => {
-        expect(err).not.to.exist
-        done()
-      })
-    })
-
-    it('should wrap the error', function (done) {
-      const error = new Error('failed to reticulate splines')
-      fs.unlink = sinon.stub().yields(error)
-      LocalFileWriter.deleteFile(fsPath, err => {
-        expect(err).to.exist
-        expect(err.cause).to.equal(error)
-        done()
-      })
-    })
-  })
-})

+ 114 - 0
services/filestore/test/unit/js/SafeExec.test.js

@@ -0,0 +1,114 @@
+import { beforeEach, chai, describe, expect, it, vi } from 'vitest'
+
+const should = chai.should()
+const modulePath = '../../../app/js/SafeExec.js'
+
+describe('SafeExec', function () {
+  let settings, options, safeExec
+
+  beforeEach(async function () {
+    settings = { enableConversions: true }
+    options = { timeout: 10 * 1000, killSignal: 'SIGTERM' }
+
+    vi.doMock('@overleaf/settings', () => ({
+      default: settings,
+    }))
+
+    safeExec = (await import(modulePath)).default
+  })
+
+  describe('safeExec', function () {
+    it('should execute a valid command', async function () {
+      await new Promise(resolve => {
+        safeExec(['/bin/echo', 'hello'], options, (err, stdout, stderr) => {
+          stdout.should.equal('hello\n')
+          stderr.should.equal('')
+          should.not.exist(err)
+          resolve()
+        })
+      })
+    })
+
+    it('should error when conversions are disabled', async function () {
+      await new Promise(resolve => {
+        settings.enableConversions = false
+        safeExec(['/bin/echo', 'hello'], options, err => {
+          expect(err).to.exist
+          resolve()
+        })
+      })
+    })
+
+    it('should execute a command with non-zero exit status', async function () {
+      await new Promise(resolve => {
+        safeExec(['/usr/bin/env', 'false'], options, err => {
+          expect(err).to.exist
+          expect(err.name).to.equal('FailedCommandError')
+          expect(err.code).to.equal(1)
+          expect(err.stdout).to.equal('')
+          expect(err.stderr).to.equal('')
+          resolve()
+        })
+      })
+    })
+
+    it('should handle an invalid command', async function () {
+      await new Promise(resolve => {
+        safeExec(['/bin/foobar'], options, err => {
+          err.code.should.equal('ENOENT')
+          resolve()
+        })
+      })
+    })
+
+    it('should handle a command that runs too long', async function () {
+      await new Promise(resolve => {
+        safeExec(
+          ['/bin/sleep', '10'],
+          { timeout: 500, killSignal: 'SIGTERM' },
+          err => {
+            expect(err).to.exist
+            expect(err.name).to.equal('FailedCommandError')
+            expect(err.code).to.equal('SIGTERM')
+            resolve()
+          }
+        )
+      })
+    })
+  })
+
+  describe('as a promise', function () {
+    beforeEach(function () {
+      safeExec = safeExec.promises
+    })
+
+    it('should execute a valid command', async function () {
+      const { stdout, stderr } = await safeExec(['/bin/echo', 'hello'], options)
+
+      stdout.should.equal('hello\n')
+      stderr.should.equal('')
+    })
+
+    it('should throw a ConversionsDisabledError when appropriate', async function () {
+      settings.enableConversions = false
+      try {
+        await safeExec(['/bin/echo', 'hello'], options)
+      } catch (err) {
+        expect(err.name).to.equal('ConversionsDisabledError')
+        return
+      }
+      expect('method did not throw an error').not.to.exist
+    })
+
+    it('should throw a FailedCommandError when appropriate', async function () {
+      try {
+        await safeExec(['/usr/bin/env', 'false'], options)
+      } catch (err) {
+        expect(err.name).to.equal('FailedCommandError')
+        expect(err.code).to.equal(1)
+        return
+      }
+      expect('method did not throw an error').not.to.exist
+    })
+  })
+})

+ 0 - 110
services/filestore/test/unit/js/SafeExecTests.js

@@ -1,110 +0,0 @@
-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 () {
-  let settings, options, safeExec
-
-  beforeEach(function () {
-    settings = { enableConversions: true }
-    options = { timeout: 10 * 1000, killSignal: 'SIGTERM' }
-
-    const ObjectPersistor = { Errors }
-
-    safeExec = SandboxedModule.require(modulePath, {
-      globals: { process },
-      requires: {
-        '@overleaf/settings': settings,
-        '@overleaf/object-persistor': ObjectPersistor,
-      },
-    })
-  })
-
-  describe('safeExec', function () {
-    it('should execute a valid command', function (done) {
-      safeExec(['/bin/echo', 'hello'], options, (err, stdout, stderr) => {
-        stdout.should.equal('hello\n')
-        stderr.should.equal('')
-        should.not.exist(err)
-        done()
-      })
-    })
-
-    it('should error when conversions are disabled', function (done) {
-      settings.enableConversions = false
-      safeExec(['/bin/echo', 'hello'], options, err => {
-        expect(err).to.exist
-        done()
-      })
-    })
-
-    it('should execute a command with non-zero exit status', function (done) {
-      safeExec(['/usr/bin/env', 'false'], options, err => {
-        expect(err).to.exist
-        expect(err.name).to.equal('FailedCommandError')
-        expect(err.code).to.equal(1)
-        expect(err.stdout).to.equal('')
-        expect(err.stderr).to.equal('')
-        done()
-      })
-    })
-
-    it('should handle an invalid command', function (done) {
-      safeExec(['/bin/foobar'], options, err => {
-        err.code.should.equal('ENOENT')
-        done()
-      })
-    })
-
-    it('should handle a command that runs too long', function (done) {
-      safeExec(
-        ['/bin/sleep', '10'],
-        { timeout: 500, killSignal: 'SIGTERM' },
-        err => {
-          expect(err).to.exist
-          expect(err.name).to.equal('FailedCommandError')
-          expect(err.code).to.equal('SIGTERM')
-          done()
-        }
-      )
-    })
-  })
-
-  describe('as a promise', function () {
-    beforeEach(function () {
-      safeExec = safeExec.promises
-    })
-
-    it('should execute a valid command', async function () {
-      const { stdout, stderr } = await safeExec(['/bin/echo', 'hello'], options)
-
-      stdout.should.equal('hello\n')
-      stderr.should.equal('')
-    })
-
-    it('should throw a ConversionsDisabledError when appropriate', async function () {
-      settings.enableConversions = false
-      try {
-        await safeExec(['/bin/echo', 'hello'], options)
-      } catch (err) {
-        expect(err.name).to.equal('ConversionsDisabledError')
-        return
-      }
-      expect('method did not throw an error').not.to.exist
-    })
-
-    it('should throw a FailedCommandError when appropriate', async function () {
-      try {
-        await safeExec(['/usr/bin/env', 'false'], options)
-      } catch (err) {
-        expect(err.name).to.equal('FailedCommandError')
-        expect(err.code).to.equal(1)
-        return
-      }
-      expect('method did not throw an error').not.to.exist
-    })
-  })
-})

+ 24 - 0
services/filestore/test/unit/js/Settings.test.js

@@ -0,0 +1,24 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+
+describe('Settings', function () {
+  describe('s3', function () {
+    const s3Settings = {
+      bucket1: {
+        auth_key: 'bucket1_key',
+        auth_secret: 'bucket1_secret',
+      },
+    }
+    afterEach(() => {
+      vi.unstubAllEnvs()
+    })
+
+    beforeEach(() => {
+      vi.stubEnv()
+      process.env.S3_BUCKET_CREDENTIALS = JSON.stringify(s3Settings)
+    })
+    it('should use JSONified env var if present', async function () {
+      const settings = (await import('@overleaf/settings')).default
+      expect(settings.filestore.s3.bucketCreds).to.deep.equal(s3Settings)
+    })
+  })
+})

+ 0 - 21
services/filestore/test/unit/js/SettingsTests.js

@@ -1,21 +0,0 @@
-const chai = require('chai')
-const { expect } = chai
-const SandboxedModule = require('sandboxed-module')
-
-describe('Settings', function () {
-  describe('s3', function () {
-    it('should use JSONified env var if present', function () {
-      const s3Settings = {
-        bucket1: {
-          auth_key: 'bucket1_key',
-          auth_secret: 'bucket1_secret',
-        },
-      }
-      process.env.S3_BUCKET_CREDENTIALS = JSON.stringify(s3Settings)
-      const settings = SandboxedModule.require('@overleaf/settings', {
-        globals: { console, process },
-      })
-      expect(settings.filestore.s3.bucketCreds).to.deep.equal(s3Settings)
-    })
-  })
-})

+ 37 - 0
services/filestore/test/unit/setup.js

@@ -0,0 +1,37 @@
+import { beforeEach, afterEach, chai, vi } from 'vitest'
+import sinon from 'sinon'
+import mongodb from 'mongodb'
+import sinonChai from 'sinon-chai'
+import chaiAsPromised from 'chai-as-promised'
+
+chai.use(sinonChai)
+chai.use(chaiAsPromised)
+
+// ensure every ObjectId has the id string as a property for correct comparisons
+mongodb.ObjectId.cacheHexString = true
+
+const sandbox = sinon.createSandbox()
+const stubs = {
+  logger: {
+    debug: sandbox.stub(),
+    log: sandbox.stub(),
+    info: sandbox.stub(),
+    warn: sandbox.stub(),
+    err: sandbox.stub(),
+    error: sandbox.stub(),
+    fatal: sandbox.stub(),
+  },
+}
+
+beforeEach(ctx => {
+  ctx.logger = stubs.logger
+  vi.doMock('@overleaf/logger', () => ({
+    default: ctx.logger,
+  }))
+})
+
+afterEach(() => {
+  sandbox.reset()
+  vi.restoreAllMocks()
+  vi.resetModules()
+})

+ 3 - 1
services/filestore/tsconfig.json

@@ -8,6 +8,8 @@
     "config/**/*",
     "scripts/**/*",
     "test/**/*",
-    "types"
+    "types",
+    "vitest.config.acceptance.cjs",
+    "vitest.config.unit.cjs"
   ]
 }

+ 25 - 0
services/filestore/vitest.config.unit.cjs

@@ -0,0 +1,25 @@
+const { defineConfig } = require('vitest/config')
+
+let reporterOptions = {}
+if (process.env.CI) {
+  reporterOptions = {
+    reporters: [
+      'default',
+      [
+        'junit',
+        {
+          classnameTemplate: `Unit tests.{filename}`,
+        },
+      ],
+    ],
+    outputFile: 'reports/junit-vitest-unit.xml',
+  }
+}
+module.exports = defineConfig({
+  test: {
+    include: ['test/unit/js/*.test.{js,ts}'],
+    setupFiles: ['./test/unit/setup.js'],
+    isolate: true,
+    ...reporterOptions,
+  },
+})