Update FSPersistor deleteDirectory behaviour to match S3 and GCS

services/filestore/app/js/FSPersistor.js

@@ -1,7 +1,6 @@
 const fs = require('fs')
 const glob = require('glob')
 const path = require('path')
-const rimraf = require('rimraf')
 const Stream = require('stream')
 const { promisify, callbackify } = require('util')
 
@@ -14,7 +13,6 @@ const fsUnlink = promisify(fs.unlink)
 const fsOpen = promisify(fs.open)
 const fsStat = promisify(fs.stat)
 const fsGlob = promisify(glob)
-const rmrf = promisify(rimraf)
 
 const filterName = key => key.replace(/\//g, '_')
 
@@ -146,7 +144,9 @@ async function deleteDirectory(location, name) {
   const filteredName = filterName(name.replace(/\/$/, ''))
 
   try {
-    await rmrf(`${location}/${filteredName}`)
+    await Promise.all(
+      (await fsGlob(`${location}/${filteredName}*`)).map(file => fsUnlink(file))
+    )
   } catch (err) {
     throw PersistorHelper.wrapError(
       err,

services/filestore/package-lock.json

@@ -5059,11 +5059,6 @@
         }
       }
     },
-    "rimraf": {
-      "version": "2.2.8",
-      "resolved": "https://registry.npmjs.org/rimraf/-/rimraf-2.2.8.tgz",
-      "integrity": "sha512-R5KMKHnPAQaZMqLOsyuyUmcIjSeDm+73eoqQpaXA7AZ22BL+6C+1mcUscgOsNd8WVlJuvlgAPsegcx7pjlV0Dg=="
-    },
     "run-async": {
       "version": "2.4.0",
       "resolved": "https://registry.npmjs.org/run-async/-/run-async-2.4.0.tgz",

services/filestore/package.json

@@ -33,7 +33,6 @@
     "range-parser": "^1.0.2",
     "request": "^2.88.0",
     "request-promise-native": "^1.0.8",
-    "rimraf": "2.2.8",
     "settings-sharelatex": "^1.1.0",
     "stream-buffers": "~0.2.5",
     "stream-meter": "^1.0.4",

services/filestore/test/unit/js/FSPersistorTests.js

@@ -22,15 +22,7 @@ describe('FSPersistorTests', function() {
   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,
-    rimraf,
-    stream,
-    LocalFileWriter,
-    FSPersistor,
-    glob,
-    readStream,
-    crypto,
-    Hash
+  let fs, stream, LocalFileWriter, FSPersistor, glob, readStream, crypto, Hash
 
   beforeEach(function() {
     readStream = {
@@ -46,7 +38,6 @@ describe('FSPersistorTests', function() {
       stat: sinon.stub().yields(null, stat)
     }
     glob = sinon.stub().yields(null, globs)
-    rimraf = sinon.stub().yields()
     stream = { pipeline: sinon.stub().yields() }
     LocalFileWriter = {
       promises: {
@@ -68,7 +59,6 @@ describe('FSPersistorTests', function() {
         './Errors': Errors,
         fs,
         glob,
-        rimraf,
         stream,
         crypto,
         // imported by PersistorHelper but otherwise unused here
@@ -271,15 +261,22 @@ describe('FSPersistorTests', function() {
   })
 
   describe('deleteDirectory', function() {
-    it('Should call rmdir(rimraf) with correct options', async function() {
+    it('Should call glob with correct options', async function() {
       await FSPersistor.promises.deleteDirectory(location, files[0])
-      expect(rimraf).to.have.been.calledWith(
-        `${location}/${filteredFilenames[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() {
-      rimraf.yields(error)
+      glob.yields(error)
       await expect(
         FSPersistor.promises.deleteDirectory(location, files[0])
       ).to.eventually.be.rejected.and.have.property('cause', error)