Use 'glob' to determine 'directory' size.

Check for filtered directory paths in all tests.

services/filestore/app/js/FSPersistorManager.js

@@ -1,4 +1,5 @@
 const fs = require('fs')
+const glob = require('glob')
 const logger = require('logger-sharelatex')
 const path = require('path')
 const rimraf = require('rimraf')
@@ -12,7 +13,7 @@ const pipeline = promisify(Stream.pipeline)
 const fsUnlink = promisify(fs.unlink)
 const fsOpen = promisify(fs.open)
 const fsStat = promisify(fs.stat)
-const fsReaddir = promisify(fs.readdir)
+const fsGlob = promisify(glob)
 const rmrf = promisify(rimraf)
 
 const filterName = key => key.replace(/\//g, '_')
@@ -124,16 +125,25 @@ async function checkIfFileExists(location, name) {
   }
 }
 
-// note, does not recurse into subdirectories
+// 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 fsReaddir(`${location}/${filteredName}`)
+    const files = await fsGlob(`${location}/${filteredName}_*`)
     for (const file of files) {
-      const stat = await fsStat(`${location}/${filteredName}/${file}`)
-      size += stat.size
+      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 new ReadError({

services/filestore/npm-shrinkwrap.json

@@ -473,7 +473,7 @@
     "@sinonjs/text-encoding": {
       "version": "0.7.1",
       "resolved": "https://registry.npmjs.org/@sinonjs/text-encoding/-/text-encoding-0.7.1.tgz",
-      "integrity": "sha1-jaXGUwkVZT86Hzj9XxAdjD+AecU=",
+      "integrity": "sha512-+iTbntw2IZPb/anVDbypzfQa+ay64MW0Zo8aJ8gZPWMMK6/OubMVb6lUPMagqjOPnmtauXnFCACVl3O7ogjeqQ==",
       "dev": true
     },
     "@types/caseless": {
@@ -2189,14 +2189,14 @@
       "integrity": "sha1-uKLHAUu1zUFTTpg7XKFgo3RwhGk="
     },
     "glob": {
-      "version": "6.0.4",
-      "resolved": "https://registry.npmjs.org/glob/-/glob-6.0.4.tgz",
-      "integrity": "sha1-DwiGD2oVUSey+t1PnOJLGqtuTSI=",
-      "optional": true,
+      "version": "7.1.6",
+      "resolved": "https://registry.npmjs.org/glob/-/glob-7.1.6.tgz",
+      "integrity": "sha512-LwaxwyZ72Lk7vZINtNNrywX0ZuLyStrdDtabefZKAY5ZGJhVtgdznluResxNmPitE0SAO+O26sWTHeKSI2wMBA==",
       "requires": {
+        "fs.realpath": "^1.0.0",
         "inflight": "^1.0.4",
         "inherits": "2",
-        "minimatch": "2 || 3",
+        "minimatch": "^3.0.4",
         "once": "^1.3.0",
         "path-is-absolute": "^1.0.0"
       }
@@ -2671,7 +2671,7 @@
     "just-extend": {
       "version": "4.0.2",
       "resolved": "https://registry.npmjs.org/just-extend/-/just-extend-4.0.2.tgz",
-      "integrity": "sha1-8/R/ffyg+YnFVBCn68iFSwcQivw=",
+      "integrity": "sha512-FrLwOgm+iXrPV+5zDU6Jqu4gCRXbWEQg2O3SKONsWE4w7AXFRkryS53bpWdaL9cNol+AmR3AEYz6kn+o0fCPnw==",
       "dev": true
     },
     "jwa": {
@@ -3259,6 +3259,21 @@
           "optional": true,
           "requires": {
             "glob": "^6.0.1"
+          },
+          "dependencies": {
+            "glob": {
+              "version": "6.0.4",
+              "resolved": "https://registry.npmjs.org/glob/-/glob-6.0.4.tgz",
+              "integrity": "sha1-DwiGD2oVUSey+t1PnOJLGqtuTSI=",
+              "optional": true,
+              "requires": {
+                "inflight": "^1.0.4",
+                "inherits": "2",
+                "minimatch": "2 || 3",
+                "once": "^1.3.0",
+                "path-is-absolute": "^1.0.0"
+              }
+            }
           }
         }
       }

services/filestore/package.json

@@ -26,6 +26,7 @@
     "body-parser": "^1.2.0",
     "express": "^4.2.0",
     "fs-extra": "^1.0.0",
+    "glob": "^7.1.6",
     "heapdump": "^0.3.2",
     "knox": "~0.9.1",
     "logger-sharelatex": "^1.7.0",

services/filestore/test/unit/js/FSPersistorManagerTests.js

@@ -10,7 +10,7 @@ chai.use(require('chai-as-promised'))
 const modulePath = '../../../app/js/FSPersistorManager.js'
 
 describe('FSPersistorManagerTests', function() {
-  const stat = { size: 4 }
+  const stat = { size: 4, isFile: sinon.stub().returns(true) }
   const fd = 1234
   const readStream = 'readStream'
   const writeStream = 'writeStream'
@@ -19,8 +19,9 @@ describe('FSPersistorManagerTests', function() {
   const location = '/foo'
   const error = new Error('guru meditation error')
 
-  const files = ['wombat.txt', 'potato.tex']
-  let fs, rimraf, stream, LocalFileWriter, FSPersistorManager
+  const files = ['animals/wombat.tex', 'vegetables/potato.tex']
+  const filteredFilenames = ['animals_wombat.tex', 'vegetables_potato.tex']
+  let fs, rimraf, stream, LocalFileWriter, FSPersistorManager, glob
 
   beforeEach(function() {
     fs = {
@@ -28,9 +29,9 @@ describe('FSPersistorManagerTests', function() {
       createWriteStream: sinon.stub().returns(writeStream),
       unlink: sinon.stub().yields(),
       open: sinon.stub().yields(null, fd),
-      readdir: sinon.stub().yields(null, files),
       stat: sinon.stub().yields(null, stat)
     }
+    glob = sinon.stub().yields(null, files)
     rimraf = sinon.stub().yields()
     stream = { pipeline: sinon.stub().yields() }
     LocalFileWriter = {
@@ -42,25 +43,31 @@ describe('FSPersistorManagerTests', function() {
     FSPersistorManager = SandboxedModule.require(modulePath, {
       requires: {
         './LocalFileWriter': LocalFileWriter,
-        fs: fs,
         'logger-sharelatex': {
           log() {},
           err() {}
         },
-        rimraf: rimraf,
-        stream: stream,
-        './Errors': Errors
+        './Errors': Errors,
+        fs,
+        glob,
+        rimraf,
+        stream
       },
       globals: { console }
     })
   })
 
   describe('sendFile', function() {
+    const localFilesystemPath = '/path/to/local/file'
     it('should copy the file', async function() {
-      await FSPersistorManager.promises.sendFile(location, files[0], files[1])
-      expect(fs.createReadStream).to.have.been.calledWith(files[1])
+      await FSPersistorManager.promises.sendFile(
+        location,
+        files[0],
+        localFilesystemPath
+      )
+      expect(fs.createReadStream).to.have.been.calledWith(localFilesystemPath)
       expect(fs.createWriteStream).to.have.been.calledWith(
-        `${location}/${files[0]}`
+        `${location}/${filteredFilenames[0]}`
       )
       expect(stream.pipeline).to.have.been.calledWith(readStream, writeStream)
     })
@@ -68,7 +75,11 @@ describe('FSPersistorManagerTests', function() {
     it('should return an error if the file cannot be stored', async function() {
       stream.pipeline.yields(error)
       await expect(
-        FSPersistorManager.promises.sendFile(location, files[0], files[1])
+        FSPersistorManager.promises.sendFile(
+          location,
+          files[0],
+          localFilesystemPath
+        )
       ).to.eventually.be.rejectedWith(error)
     })
   })
@@ -114,16 +125,15 @@ describe('FSPersistorManagerTests', function() {
   })
 
   describe('getFileStream', function() {
-    const filename = 'wombat/potato'
-    const filteredFilename = 'wombat_potato'
-
     it('should use correct file location', async function() {
-      await FSPersistorManager.promises.getFileStream(location, filename, {})
-      expect(fs.open).to.have.been.calledWith(`${location}/${filteredFilename}`)
+      await FSPersistorManager.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 FSPersistorManager.promises.getFileStream(location, filename, {
+      await FSPersistorManager.promises.getFileStream(location, files[0], {
         start: 0,
         end: 8
       })
@@ -140,7 +150,7 @@ describe('FSPersistorManagerTests', function() {
       fs.open.yields(err)
 
       await expect(
-        FSPersistorManager.promises.getFileStream(location, filename, {})
+        FSPersistorManager.promises.getFileStream(location, files[0], {})
       )
         .to.eventually.be.rejectedWith('file not found')
         .and.be.an.instanceOf(Errors.NotFoundError)
@@ -149,7 +159,7 @@ describe('FSPersistorManagerTests', function() {
     it('should wrap any other error', async function() {
       fs.open.yields(error)
       await expect(
-        FSPersistorManager.promises.getFileStream(location, filename, {})
+        FSPersistorManager.promises.getFileStream(location, files[0], {})
       )
         .to.eventually.be.rejectedWith('failed to open file for streaming')
         .and.be.an.instanceOf(Errors.ReadError)
@@ -158,9 +168,7 @@ describe('FSPersistorManagerTests', function() {
   })
 
   describe('getFileSize', function() {
-    const filename = 'wombat/potato'
     const badFilename = 'neenaw.tex'
-    const filteredFilename = 'wombat_potato'
     const size = 65536
     const noentError = new Error('not found')
     noentError.code = 'ENOENT'
@@ -168,7 +176,7 @@ describe('FSPersistorManagerTests', function() {
     beforeEach(function() {
       fs.stat
         .yields(error)
-        .withArgs(`${location}/${filteredFilename}`)
+        .withArgs(`${location}/${filteredFilenames[0]}`)
         .yields(null, { size })
         .withArgs(`${location}/${badFilename}`)
         .yields(noentError)
@@ -176,7 +184,7 @@ describe('FSPersistorManagerTests', function() {
 
     it('should return the file size', async function() {
       expect(
-        await FSPersistorManager.promises.getFileSize(location, filename)
+        await FSPersistorManager.promises.getFileSize(location, files[0])
       ).to.equal(size)
     })
 
@@ -197,14 +205,14 @@ describe('FSPersistorManagerTests', function() {
     it('Should open the source for reading', async function() {
       await FSPersistorManager.promises.copyFile(location, files[0], files[1])
       expect(fs.createReadStream).to.have.been.calledWith(
-        `${location}/${files[0]}`
+        `${location}/${filteredFilenames[0]}`
       )
     })
 
     it('Should open the target for writing', async function() {
       await FSPersistorManager.promises.copyFile(location, files[0], files[1])
       expect(fs.createWriteStream).to.have.been.calledWith(
-        `${location}/${files[1]}`
+        `${location}/${filteredFilenames[1]}`
       )
     })
 
@@ -217,7 +225,9 @@ describe('FSPersistorManagerTests', function() {
   describe('deleteFile', function() {
     it('Should call unlink with correct options', async function() {
       await FSPersistorManager.promises.deleteFile(location, files[0])
-      expect(fs.unlink).to.have.been.calledWith(`${location}/${files[0]}`)
+      expect(fs.unlink).to.have.been.calledWith(
+        `${location}/${filteredFilenames[0]}`
+      )
     })
 
     it('Should propagate the error', async function() {
@@ -231,7 +241,9 @@ describe('FSPersistorManagerTests', function() {
   describe('deleteDirectory', function() {
     it('Should call rmdir(rimraf) with correct options', async function() {
       await FSPersistorManager.promises.deleteDirectory(location, files[0])
-      expect(rimraf).to.have.been.calledWith(`${location}/${files[0]}`)
+      expect(rimraf).to.have.been.calledWith(
+        `${location}/${filteredFilenames[0]}`
+      )
     })
 
     it('Should propagate the error', async function() {
@@ -243,28 +255,29 @@ describe('FSPersistorManagerTests', function() {
   })
 
   describe('checkIfFileExists', function() {
-    const filename = 'wombat'
-    const badFilename = 'potato'
+    const badFilename = 'pototo'
     const noentError = new Error('not found')
     noentError.code = 'ENOENT'
 
     beforeEach(function() {
       fs.stat
         .yields(error)
-        .withArgs(`${location}/${filename}`)
+        .withArgs(`${location}/${filteredFilenames[0]}`)
         .yields(null, {})
         .withArgs(`${location}/${badFilename}`)
         .yields(noentError)
     })
 
     it('Should call stat with correct options', async function() {
-      await FSPersistorManager.promises.checkIfFileExists(location, filename)
-      expect(fs.stat).to.have.been.calledWith(`${location}/${filename}`)
+      await FSPersistorManager.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 FSPersistorManager.promises.checkIfFileExists(location, filename)
+        await FSPersistorManager.promises.checkIfFileExists(location, files[0])
       ).to.equal(true)
     })
 
@@ -288,19 +301,26 @@ describe('FSPersistorManagerTests', function() {
 
   describe('directorySize', function() {
     it('should wrap the error', async function() {
-      fs.readdir.yields(error)
+      glob.yields(error)
       await expect(
-        FSPersistorManager.promises.directorySize(location, 'wombat')
+        FSPersistorManager.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: 'wombat' })
+        .which.includes({ location, name: files[0] })
+    })
+
+    it('should filter the directory name', async function() {
+      await FSPersistorManager.promises.directorySize(location, files[0])
+      expect(glob).to.have.been.calledWith(
+        `${location}/${filteredFilenames[0]}_*`
+      )
     })
 
     it('should sum directory files size', async function() {
       expect(
-        await FSPersistorManager.promises.directorySize(location, 'wombat')
+        await FSPersistorManager.promises.directorySize(location, files[0])
       ).to.equal(stat.size * files.length)
     })
   })