diff --git a/services/filestore/app/js/FSPersistorManager.js b/services/filestore/app/js/FSPersistorManager.js index d11d839df7..1a9d2b824d 100644 --- a/services/filestore/app/js/FSPersistorManager.js +++ b/services/filestore/app/js/FSPersistorManager.js @@ -1,206 +1,229 @@ -/* eslint-disable - handle-callback-err, - no-unreachable, - node/no-deprecated-api, -*/ -// TODO: This file was created by bulk-decaffeinate. -// Fix any style issues and re-enable lint. -/* - * decaffeinate suggestions: - * DS102: Remove unnecessary code created because of implicit returns - * DS207: Consider shorter variations of null checks - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ -const logger = require('logger-sharelatex') const fs = require('fs') +const glob = require('glob') +const logger = require('logger-sharelatex') const path = require('path') -const LocalFileWriter = require('./LocalFileWriter') -const Errors = require('./Errors') const rimraf = require('rimraf') -const _ = require('underscore') +const Stream = require('stream') +const { promisify, callbackify } = require('util') + +const LocalFileWriter = require('./LocalFileWriter').promises +const { NotFoundError, ReadError, WriteError } = require('./Errors') + +const pipeline = promisify(Stream.pipeline) +const fsUnlink = promisify(fs.unlink) +const fsOpen = promisify(fs.open) +const fsStat = promisify(fs.stat) +const fsGlob = promisify(glob) +const rmrf = promisify(rimraf) const filterName = key => key.replace(/\//g, '_') -module.exports = { - sendFile(location, target, source, callback) { - if (callback == null) { - callback = function(err) {} - } - const filteredTarget = filterName(target) - logger.log({ location, target: filteredTarget, source }, 'sending file') - const done = _.once(function(err) { - if (err != null) { - logger.err( - { err, location, target: filteredTarget, source }, - 'Error on put of file' - ) - } - return callback(err) - }) - // actually copy the file (instead of moving it) to maintain consistent behaviour - // between the different implementations +async function sendFile(location, target, source) { + const filteredTarget = filterName(target) + logger.log({ location, target: filteredTarget, source }, 'sending file') + + // actually copy the file (instead of moving it) to maintain consistent behaviour + // between the different implementations + try { const sourceStream = fs.createReadStream(source) - sourceStream.on('error', done) const targetStream = fs.createWriteStream(`${location}/${filteredTarget}`) - targetStream.on('error', done) - targetStream.on('finish', () => done()) - return sourceStream.pipe(targetStream) - }, - - sendStream(location, target, sourceStream, callback) { - if (callback == null) { - callback = function(err) {} - } - logger.log({ location, target }, 'sending file stream') - sourceStream.on('error', err => - logger.err({ location, target, err: err('error on stream to send') }) + await pipeline(sourceStream, targetStream) + } catch (err) { + throw _wrapError( + err, + 'failed to copy the specified file', + { location, target, source }, + WriteError ) - return LocalFileWriter.writeStream(sourceStream, null, (err, fsPath) => { - if (err != null) { - logger.err( - { location, target, fsPath, err }, - 'something went wrong writing stream to disk' - ) - return callback(err) - } - return this.sendFile(location, target, fsPath, ( - err // delete the temporary file created above and return the original error - ) => LocalFileWriter.deleteFile(fsPath, () => callback(err))) - }) - }, - - // opts may be {start: Number, end: Number} - getFileStream(location, name, opts, callback) { - if (callback == null) { - callback = function(err, res) {} - } - const filteredName = filterName(name) - logger.log({ location, filteredName }, 'getting file') - return fs.open(`${location}/${filteredName}`, 'r', function(err, fd) { - if (err != null) { - logger.err( - { err, location, filteredName: name }, - 'Error reading from file' - ) - if (err.code === 'ENOENT') { - return callback(new Errors.NotFoundError(err.message), null) - } else { - return callback(err, null) - } - } - opts.fd = fd - const sourceStream = fs.createReadStream(null, opts) - return callback(null, sourceStream) - }) - }, - - getFileSize(location, filename, callback) { - const fullPath = path.join(location, filterName(filename)) - return fs.stat(fullPath, function(err, stats) { - if (err != null) { - if (err.code === 'ENOENT') { - logger.log({ location, filename }, 'file not found') - callback(new Errors.NotFoundError(err.message)) - } else { - logger.err({ err, location, filename }, 'failed to stat file') - callback(err) - } - return - } - return callback(null, stats.size) - }) - }, - - copyFile(location, fromName, toName, callback) { - if (callback == null) { - callback = function(err) {} - } - const filteredFromName = filterName(fromName) - const filteredToName = filterName(toName) - logger.log( - { location, fromName: filteredFromName, toName: filteredToName }, - 'copying file' - ) - const sourceStream = fs.createReadStream(`${location}/${filteredFromName}`) - sourceStream.on('error', function(err) { - logger.err( - { err, location, key: filteredFromName }, - 'Error reading from file' - ) - return callback(err) - }) - const targetStream = fs.createWriteStream(`${location}/${filteredToName}`) - targetStream.on('error', function(err) { - logger.err( - { err, location, key: filteredToName }, - 'Error writing to file' - ) - return callback(err) - }) - targetStream.on('finish', () => callback(null)) - return sourceStream.pipe(targetStream) - }, - - deleteFile(location, name, callback) { - const filteredName = filterName(name) - logger.log({ location, filteredName }, 'delete file') - return fs.unlink(`${location}/${filteredName}`, function(err) { - if (err != null) { - logger.err({ err, location, filteredName }, 'Error on delete.') - return callback(err) - } else { - return callback() - } - }) - }, - - deleteDirectory(location, name, callback) { - if (callback == null) { - callback = function(err) {} - } - const filteredName = filterName(name.replace(/\/$/, '')) - return rimraf(`${location}/${filteredName}`, function(err) { - if (err != null) { - logger.err({ err, location, filteredName }, 'Error on rimraf rmdir.') - return callback(err) - } else { - return callback() - } - }) - }, - - checkIfFileExists(location, name, callback) { - if (callback == null) { - callback = function(err, exists) {} - } - const filteredName = filterName(name) - logger.log({ location, filteredName }, 'checking if file exists') - return fs.exists(`${location}/${filteredName}`, function(exists) { - logger.log({ location, filteredName, exists }, 'checked if file exists') - return callback(null, exists) - }) - }, - - directorySize(location, name, callback) { - const filteredName = filterName(name.replace(/\/$/, '')) - logger.log({ location, filteredName }, 'get project size in file system') - return fs.readdir(`${location}/${filteredName}`, function(err, files) { - if (err != null) { - logger.err( - { err, location, filteredName }, - 'something went wrong listing prefix in aws' - ) - return callback(err) - } - let totalSize = 0 - _.each(files, function(entry) { - const fd = fs.openSync(`${location}/${filteredName}/${entry}`, 'r') - const fileStats = fs.fstatSync(fd) - totalSize += fileStats.size - return fs.closeSync(fd) - }) - logger.log({ totalSize }, 'total size', { files }) - return callback(null, totalSize) - }) + } +} + +async function sendStream(location, target, sourceStream) { + logger.log({ location, target }, 'sending file stream') + + const fsPath = await LocalFileWriter.writeStream(sourceStream) + + try { + await sendFile(location, target, fsPath) + } finally { + await LocalFileWriter.deleteFile(fsPath) + } +} + +// opts may be {start: Number, end: Number} +async function getFileStream(location, name, opts) { + const filteredName = filterName(name) + logger.log({ location, filteredName }, 'getting file') + + try { + opts.fd = await fsOpen(`${location}/${filteredName}`, 'r') + } catch (err) { + logger.err({ err, location, filteredName: name }, 'Error reading from file') + + throw _wrapError( + err, + 'failed to open file for streaming', + { location, filteredName, opts }, + ReadError + ) + } + + return fs.createReadStream(null, opts) +} + +async function getFileSize(location, filename) { + const fullPath = path.join(location, filterName(filename)) + + try { + const stat = await fsStat(fullPath) + return stat.size + } catch (err) { + logger.err({ err, location, filename }, 'failed to stat file') + + throw _wrapError( + err, + 'failed to stat file', + { location, filename }, + ReadError + ) + } +} + +async function copyFile(location, fromName, toName) { + const filteredFromName = filterName(fromName) + const filteredToName = filterName(toName) + logger.log({ location, filteredFromName, filteredToName }, 'copying file') + + try { + const sourceStream = fs.createReadStream(`${location}/${filteredFromName}`) + const targetStream = fs.createWriteStream(`${location}/${filteredToName}`) + await pipeline(sourceStream, targetStream) + } catch (err) { + throw _wrapError( + err, + 'failed to copy file', + { location, filteredFromName, filteredToName }, + WriteError + ) + } +} + +async function deleteFile(location, name) { + const filteredName = filterName(name) + logger.log({ location, filteredName }, 'delete file') + try { + await fsUnlink(`${location}/${filteredName}`) + } catch (err) { + throw _wrapError( + err, + 'failed to delete file', + { location, filteredName }, + WriteError + ) + } +} + +// this is only called internally for clean-up by `FileHandler` and isn't part of the external API +async function deleteDirectory(location, name) { + const filteredName = filterName(name.replace(/\/$/, '')) + + logger.log({ location, filteredName }, 'deleting directory') + + try { + await rmrf(`${location}/${filteredName}`) + } catch (err) { + throw _wrapError( + err, + 'failed to delete directory', + { location, filteredName }, + WriteError + ) + } +} + +async function checkIfFileExists(location, name) { + const filteredName = filterName(name) + try { + const stat = await fsStat(`${location}/${filteredName}`) + return !!stat + } catch (err) { + if (err.code === 'ENOENT') { + return false + } + throw _wrapError( + err, + 'failed to stat file', + { location, filteredName }, + ReadError + ) + } +} + +// note, does not recurse into subdirectories, as we use a flattened directory structure +async function directorySize(location, name) { + const filteredName = filterName(name.replace(/\/$/, '')) + let size = 0 + + try { + const files = await fsGlob(`${location}/${filteredName}_*`) + for (const file of files) { + try { + const stat = await fsStat(file) + if (stat.isFile()) { + size += stat.size + } + } catch (err) { + // ignore files that may have just been deleted + if (err.code !== 'ENOENT') { + throw err + } + } + } + } catch (err) { + throw _wrapError( + err, + 'failed to get directory size', + { location, name }, + ReadError + ) + } + + return size +} + +function _wrapError(error, message, params, ErrorType) { + if (error.code === 'ENOENT') { + return new NotFoundError({ + message: 'no such file or directory', + info: params + }).withCause(error) + } else { + return new ErrorType({ + message: message, + info: params + }).withCause(error) + } +} + +module.exports = { + sendFile: callbackify(sendFile), + sendStream: callbackify(sendStream), + getFileStream: callbackify(getFileStream), + getFileSize: callbackify(getFileSize), + copyFile: callbackify(copyFile), + deleteFile: callbackify(deleteFile), + deleteDirectory: callbackify(deleteDirectory), + checkIfFileExists: callbackify(checkIfFileExists), + directorySize: callbackify(directorySize), + promises: { + sendFile, + sendStream, + getFileStream, + getFileSize, + copyFile, + deleteFile, + deleteDirectory, + checkIfFileExists, + directorySize } } diff --git a/services/filestore/app/js/LocalFileWriter.js b/services/filestore/app/js/LocalFileWriter.js index 44f3f9433a..22957e15d1 100644 --- a/services/filestore/app/js/LocalFileWriter.js +++ b/services/filestore/app/js/LocalFileWriter.js @@ -32,6 +32,8 @@ async function writeStream(stream, key) { logger.log({ fsPath }, 'finished writing file locally') return fsPath } catch (err) { + await deleteFile(fsPath) + logger.err({ err, fsPath }, 'problem writing file locally') throw new WriteError({ message: 'problem writing file locally', @@ -45,7 +47,16 @@ async function deleteFile(fsPath) { return } logger.log({ fsPath }, 'removing local temp file') - await promisify(fs.unlink)(fsPath) + try { + await promisify(fs.unlink)(fsPath) + } catch (err) { + if (err.code !== 'ENOENT') { + throw new WriteError({ + message: 'failed to delete file', + info: { fsPath } + }).withCause(err) + } + } } function _getPath(key) { diff --git a/services/filestore/npm-shrinkwrap.json b/services/filestore/npm-shrinkwrap.json index fa498f3f1b..3ed1400a61 100644 --- a/services/filestore/npm-shrinkwrap.json +++ b/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" + } + } } } } diff --git a/services/filestore/package.json b/services/filestore/package.json index 9515c1850c..5d7c3e3ec1 100644 --- a/services/filestore/package.json +++ b/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", diff --git a/services/filestore/test/acceptance/js/FilestoreTests.js b/services/filestore/test/acceptance/js/FilestoreTests.js index 3315569a24..9260b1bd62 100644 --- a/services/filestore/test/acceptance/js/FilestoreTests.js +++ b/services/filestore/test/acceptance/js/FilestoreTests.js @@ -56,6 +56,7 @@ if (process.env.AWS_ACCESS_KEY_ID) { describe('Filestore', function() { this.timeout(1000 * 10) const filestoreUrl = `http://localhost:${Settings.internal.filestore.port}` + const directoryName = 'directory' // redefine the test suite for every available backend Object.keys(BackendSettings).forEach(backend => { @@ -113,11 +114,11 @@ describe('Filestore', function() { beforeEach(async function() { fileId = Math.random() - fileUrl = `${filestoreUrl}/project/acceptance_tests/file/${fileId}` + fileUrl = `${filestoreUrl}/project/acceptance_tests/file/${directoryName}%2F${fileId}` const writeStream = request.post(fileUrl) const readStream = fs.createReadStream(localFileReadPath) - // consume the result to ensure the http request has been fully processed + // hack to consume the result to ensure the http request has been fully processed const resultStream = fs.createWriteStream('/dev/null') await pipeline(readStream, writeStream, resultStream) }) @@ -176,14 +177,14 @@ describe('Filestore', function() { it('should be able to copy files', async function() { const newProjectID = 'acceptance_tests_copyied_project' const newFileId = Math.random() - const newFileUrl = `${filestoreUrl}/project/${newProjectID}/file/${newFileId}` + const newFileUrl = `${filestoreUrl}/project/${newProjectID}/file/${directoryName}%2F${newFileId}` const opts = { method: 'put', uri: newFileUrl, json: { source: { project_id: 'acceptance_tests', - file_id: fileId + file_id: `${directoryName}/${fileId}` } } } @@ -223,6 +224,70 @@ describe('Filestore', function() { } }) + describe('with multiple files', function() { + let fileIds, fileUrls, project + const directoryName = 'directory' + const localFileReadPaths = [ + '/tmp/filestore_acceptance_tests_file_read_1.txt', + '/tmp/filestore_acceptance_tests_file_read_2.txt' + ] + const constantFileContents = [ + [ + 'hello world', + `line 2 goes here ${Math.random()}`, + 'there are 3 lines in all' + ].join('\n'), + [ + `for reference: ${Math.random()}`, + 'cats are the best animals', + 'wombats are a close second' + ].join('\n') + ] + + before(async function() { + return Promise.all([ + fsWriteFile(localFileReadPaths[0], constantFileContents[0]), + fsWriteFile(localFileReadPaths[1], constantFileContents[1]) + ]) + }) + + beforeEach(async function() { + project = `acceptance_tests_${Math.random()}` + fileIds = [Math.random(), Math.random()] + fileUrls = [ + `${filestoreUrl}/project/${project}/file/${directoryName}%2F${fileIds[0]}`, + `${filestoreUrl}/project/${project}/file/${directoryName}%2F${fileIds[1]}` + ] + + const writeStreams = [ + request.post(fileUrls[0]), + request.post(fileUrls[1]) + ] + const readStreams = [ + fs.createReadStream(localFileReadPaths[0]), + fs.createReadStream(localFileReadPaths[1]) + ] + // hack to consume the result to ensure the http request has been fully processed + const resultStreams = [ + fs.createWriteStream('/dev/null'), + fs.createWriteStream('/dev/null') + ] + return Promise.all([ + pipeline(readStreams[0], writeStreams[0], resultStreams[0]), + pipeline(readStreams[1], writeStreams[1], resultStreams[1]) + ]) + }) + + it('should get the directory size', async function() { + const response = await rp.get( + `${filestoreUrl}/project/${project}/size` + ) + expect(parseInt(JSON.parse(response.body)['total bytes'])).to.equal( + constantFileContents[0].length + constantFileContents[1].length + ) + }) + }) + describe('with a pdf file', function() { let fileId, fileUrl, localFileSize const localFileReadPath = Path.resolve( @@ -232,7 +297,7 @@ describe('Filestore', function() { beforeEach(async function() { fileId = Math.random() - fileUrl = `${filestoreUrl}/project/acceptance_tests/file/${fileId}` + fileUrl = `${filestoreUrl}/project/acceptance_tests/file/${directoryName}%2F${fileId}` const stat = await fsStat(localFileReadPath) localFileSize = stat.size const writeStream = request.post(fileUrl) diff --git a/services/filestore/test/unit/js/FSPersistorManagerTests.js b/services/filestore/test/unit/js/FSPersistorManagerTests.js index 9e9018c17b..6847f5b8ef 100644 --- a/services/filestore/test/unit/js/FSPersistorManagerTests.js +++ b/services/filestore/test/unit/js/FSPersistorManagerTests.js @@ -1,502 +1,328 @@ -/* eslint-disable - handle-callback-err, - no-return-assign, - no-unused-vars, -*/ -// TODO: This file was created by bulk-decaffeinate. -// Fix any style issues and re-enable lint. -/* - * decaffeinate suggestions: - * DS102: Remove unnecessary code created because of implicit returns - * DS207: Consider shorter variations of null checks - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ -const { assert } = require('chai') const sinon = require('sinon') const chai = require('chai') -const { should } = chai const { expect } = chai -const modulePath = '../../../app/js/FSPersistorManager.js' const SandboxedModule = require('sandboxed-module') -const fs = require('fs') -const response = require('response') +const Errors = require('../../../app/js/Errors') + +chai.use(require('sinon-chai')) +chai.use(require('chai-as-promised')) + +const modulePath = '../../../app/js/FSPersistorManager.js' describe('FSPersistorManagerTests', function() { + const stat = { size: 4, isFile: sinon.stub().returns(true) } + const fd = 1234 + const readStream = 'readStream' + const writeStream = 'writeStream' + const remoteStream = 'remoteStream' + const tempFile = '/tmp/potato.txt' + const location = '/foo' + const error = new Error('guru meditation error') + + 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, FSPersistorManager, glob + beforeEach(function() { - this.Fs = { - rename: sinon.stub(), - createReadStream: sinon.stub(), - createWriteStream: sinon.stub(), - unlink: sinon.stub(), - rmdir: sinon.stub(), - exists: sinon.stub(), - readdir: sinon.stub(), - open: sinon.stub(), - openSync: sinon.stub(), - fstatSync: sinon.stub(), - closeSync: sinon.stub(), - stat: sinon.stub() + fs = { + createReadStream: sinon.stub().returns(readStream), + createWriteStream: sinon.stub().returns(writeStream), + unlink: sinon.stub().yields(), + open: sinon.stub().yields(null, fd), + stat: sinon.stub().yields(null, stat) } - this.Rimraf = sinon.stub() - this.LocalFileWriter = { - writeStream: sinon.stub(), - deleteFile: sinon.stub() + glob = sinon.stub().yields(null, globs) + rimraf = sinon.stub().yields() + stream = { pipeline: sinon.stub().yields() } + LocalFileWriter = { + promises: { + writeStream: sinon.stub().resolves(tempFile), + deleteFile: sinon.stub().resolves() + } } - this.requires = { - './LocalFileWriter': this.LocalFileWriter, - fs: this.Fs, - 'logger-sharelatex': { - log() {}, - err() {} + FSPersistorManager = SandboxedModule.require(modulePath, { + requires: { + './LocalFileWriter': LocalFileWriter, + 'logger-sharelatex': { + log() {}, + err() {} + }, + './Errors': Errors, + fs, + glob, + rimraf, + stream }, - response: response, - rimraf: this.Rimraf, - './Errors': (this.Errors = { NotFoundError: sinon.stub() }) - } - this.location = '/tmp' - this.name1 = '530f2407e7ef165704000007/530f838b46d9a9e859000008' - this.name1Filtered = '530f2407e7ef165704000007_530f838b46d9a9e859000008' - this.name2 = 'second_file' - this.error = 'error_message' - return (this.FSPersistorManager = SandboxedModule.require(modulePath, { - requires: this.requires - })) + globals: { console } + }) }) describe('sendFile', function() { - beforeEach(function() { - return (this.Fs.createReadStream = sinon.stub().returns({ - on() {}, - pipe() {} - })) + const localFilesystemPath = '/path/to/local/file' + it('should copy the file', async function() { + await FSPersistorManager.promises.sendFile( + location, + files[0], + localFilesystemPath + ) + expect(fs.createReadStream).to.have.been.calledWith(localFilesystemPath) + expect(fs.createWriteStream).to.have.been.calledWith( + `${location}/${filteredFilenames[0]}` + ) + expect(stream.pipeline).to.have.been.calledWith(readStream, writeStream) }) - it('should copy the file', function(done) { - this.Fs.createWriteStream = sinon.stub().returns({ - on(event, handler) { - if (event === 'finish') { - return process.nextTick(handler) - } - } - }) - return this.FSPersistorManager.sendFile( - this.location, - this.name1, - this.name2, - err => { - this.Fs.createReadStream.calledWith(this.name2).should.equal(true) - this.Fs.createWriteStream - .calledWith(`${this.location}/${this.name1Filtered}`) - .should.equal(true) - return done() - } - ) - }) - - return it('should return an error if the file cannot be stored', function(done) { - this.Fs.createWriteStream = sinon.stub().returns({ - on: (event, handler) => { - if (event === 'error') { - return process.nextTick(() => { - return handler(this.error) - }) - } - } - }) - return this.FSPersistorManager.sendFile( - this.location, - this.name1, - this.name2, - err => { - this.Fs.createReadStream.calledWith(this.name2).should.equal(true) - this.Fs.createWriteStream - .calledWith(`${this.location}/${this.name1Filtered}`) - .should.equal(true) - err.should.equal(this.error) - return done() - } - ) + 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], + localFilesystemPath + ) + ).to.eventually.be.rejected.and.have.property('cause', error) }) }) describe('sendStream', function() { - beforeEach(function() { - this.FSPersistorManager.sendFile = sinon.stub().callsArgWith(3) - this.LocalFileWriter.writeStream.callsArgWith(2, null, this.name1) - this.LocalFileWriter.deleteFile.callsArg(1) - return (this.SourceStream = { on() {} }) - }) - - it('should sent stream to LocalFileWriter', function(done) { - return this.FSPersistorManager.sendStream( - this.location, - this.name1, - this.SourceStream, - () => { - this.LocalFileWriter.writeStream - .calledWith(this.SourceStream) - .should.equal(true) - return done() - } + it('should send the stream to LocalFileWriter', async function() { + await FSPersistorManager.promises.sendStream( + location, + files[0], + remoteStream + ) + expect(LocalFileWriter.promises.writeStream).to.have.been.calledWith( + remoteStream ) }) - it('should return the error from LocalFileWriter', function(done) { - this.LocalFileWriter.writeStream.callsArgWith(2, this.error) - return this.FSPersistorManager.sendStream( - this.location, - this.name1, - this.SourceStream, - err => { - err.should.equal(this.error) - return done() - } + it('should delete the temporary file', async function() { + await FSPersistorManager.promises.sendStream( + location, + files[0], + remoteStream + ) + expect(LocalFileWriter.promises.deleteFile).to.have.been.calledWith( + tempFile ) }) - return it('should send the file to the filestore', function(done) { - this.LocalFileWriter.writeStream.callsArgWith(2) - return this.FSPersistorManager.sendStream( - this.location, - this.name1, - this.SourceStream, - err => { - this.FSPersistorManager.sendFile.called.should.equal(true) - return done() - } + it('should return the error from LocalFileWriter', async function() { + LocalFileWriter.promises.writeStream.rejects(error) + await expect( + FSPersistorManager.promises.sendStream(location, files[0], remoteStream) + ).to.eventually.be.rejectedWith(error) + }) + + it('should send the temporary file to the filestore', async function() { + await FSPersistorManager.promises.sendStream( + location, + files[0], + remoteStream ) + expect(fs.createReadStream).to.have.been.calledWith(tempFile) }) }) describe('getFileStream', function() { - beforeEach(function() { - return (this.opts = {}) - }) - - it('should use correct file location', function(done) { - this.FSPersistorManager.getFileStream( - this.location, - this.name1, - this.opts, - (err, res) => {} + it('should use correct file location', async function() { + await FSPersistorManager.promises.getFileStream(location, files[0], {}) + expect(fs.open).to.have.been.calledWith( + `${location}/${filteredFilenames[0]}` ) - this.Fs.open - .calledWith(`${this.location}/${this.name1Filtered}`) - .should.equal(true) - return done() }) - describe('with start and end options', function() { - beforeEach(function() { - this.fd = 2019 - this.opts_in = { start: 0, end: 8 } - this.opts = { start: 0, end: 8, fd: this.fd } - return this.Fs.open.callsArgWith(2, null, this.fd) + it('should pass the options to createReadStream', async function() { + await FSPersistorManager.promises.getFileStream(location, files[0], { + start: 0, + end: 8 }) - - return it('should pass the options to createReadStream', function(done) { - this.FSPersistorManager.getFileStream( - this.location, - this.name1, - this.opts_in, - (err, res) => {} - ) - this.Fs.createReadStream.calledWith(null, this.opts).should.equal(true) - return done() + expect(fs.createReadStream).to.have.been.calledWith(null, { + start: 0, + end: 8, + fd }) }) - return describe('error conditions', function() { - describe('when the file does not exist', function() { - beforeEach(function() { - this.fakeCode = 'ENOENT' - const err = new Error() - err.code = this.fakeCode - return this.Fs.open.callsArgWith(2, err, null) - }) + it('should give a NotFoundError if the file does not exist', async function() { + const err = new Error() + err.code = 'ENOENT' + fs.open.yields(err) - return it('should give a NotFoundError', function(done) { - return this.FSPersistorManager.getFileStream( - this.location, - this.name1, - this.opts, - (err, res) => { - expect(res).to.equal(null) - expect(err).to.not.equal(null) - expect(err instanceof this.Errors.NotFoundError).to.equal(true) - return done() - } - ) - }) - }) + await expect( + FSPersistorManager.promises.getFileStream(location, files[0], {}) + ) + .to.eventually.be.rejected.and.be.an.instanceOf(Errors.NotFoundError) + .and.have.property('cause', err) + }) - return describe('when some other error happens', function() { - beforeEach(function() { - this.fakeCode = 'SOMETHINGHORRIBLE' - const err = new Error() - err.code = this.fakeCode - return this.Fs.open.callsArgWith(2, err, null) - }) - - return it('should give an Error', function(done) { - return this.FSPersistorManager.getFileStream( - this.location, - this.name1, - this.opts, - (err, res) => { - expect(res).to.equal(null) - expect(err).to.not.equal(null) - expect(err instanceof Error).to.equal(true) - return done() - } - ) - }) - }) + it('should wrap any other error', async function() { + fs.open.yields(error) + await expect( + FSPersistorManager.promises.getFileStream(location, files[0], {}) + ) + .to.eventually.be.rejectedWith('failed to open file for streaming') + .and.be.an.instanceOf(Errors.ReadError) + .and.have.property('cause', error) }) }) describe('getFileSize', function() { - it('should return the file size', function(done) { - const expectedFileSize = 75382 - this.Fs.stat.yields(new Error('fs.stat got unexpected arguments')) - this.Fs.stat - .withArgs(`${this.location}/${this.name1Filtered}`) - .yields(null, { size: expectedFileSize }) + const badFilename = 'neenaw.tex' + const size = 65536 + const noentError = new Error('not found') + noentError.code = 'ENOENT' - return this.FSPersistorManager.getFileSize( - this.location, - this.name1, - (err, fileSize) => { - if (err != null) { - return done(err) - } - expect(fileSize).to.equal(expectedFileSize) - return done() - } - ) + beforeEach(function() { + fs.stat + .yields(error) + .withArgs(`${location}/${filteredFilenames[0]}`) + .yields(null, { size }) + .withArgs(`${location}/${badFilename}`) + .yields(noentError) }) - it('should throw a NotFoundError if the file does not exist', function(done) { - const error = new Error() - error.code = 'ENOENT' - this.Fs.stat.yields(error) - - return this.FSPersistorManager.getFileSize( - this.location, - this.name1, - (err, fileSize) => { - expect(err).to.be.instanceof(this.Errors.NotFoundError) - return done() - } - ) + it('should return the file size', async function() { + expect( + await FSPersistorManager.promises.getFileSize(location, files[0]) + ).to.equal(size) }) - return it('should rethrow any other error', function(done) { - const error = new Error() - this.Fs.stat.yields(error) + it('should throw a NotFoundError if the file does not exist', async function() { + await expect( + FSPersistorManager.promises.getFileSize(location, badFilename) + ).to.eventually.be.rejected.and.be.an.instanceOf(Errors.NotFoundError) + }) - return this.FSPersistorManager.getFileSize( - this.location, - this.name1, - (err, fileSize) => { - expect(err).to.equal(error) - return done() - } - ) + it('should wrap any other error', async function() { + await expect(FSPersistorManager.promises.getFileSize(location, 'raccoon')) + .to.eventually.be.rejected.and.be.an.instanceOf(Errors.ReadError) + .and.have.property('cause', error) }) }) describe('copyFile', function() { - beforeEach(function() { - this.ReadStream = { - on() {}, - pipe: sinon.stub() - } - this.WriteStream = { on() {} } - this.Fs.createReadStream.returns(this.ReadStream) - return this.Fs.createWriteStream.returns(this.WriteStream) + 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}/${filteredFilenames[0]}` + ) }) - it('Should open the source for reading', function(done) { - this.FSPersistorManager.copyFile( - this.location, - this.name1, - this.name2, - function() {} + 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}/${filteredFilenames[1]}` ) - this.Fs.createReadStream - .calledWith(`${this.location}/${this.name1Filtered}`) - .should.equal(true) - return done() }) - it('Should open the target for writing', function(done) { - this.FSPersistorManager.copyFile( - this.location, - this.name1, - this.name2, - function() {} - ) - this.Fs.createWriteStream - .calledWith(`${this.location}/${this.name2}`) - .should.equal(true) - return done() - }) - - return it('Should pipe the source to the target', function(done) { - this.FSPersistorManager.copyFile( - this.location, - this.name1, - this.name2, - function() {} - ) - this.ReadStream.pipe.calledWith(this.WriteStream).should.equal(true) - return done() + it('Should pipe the source to the target', async function() { + await FSPersistorManager.promises.copyFile(location, files[0], files[1]) + expect(stream.pipeline).to.have.been.calledWith(readStream, writeStream) }) }) describe('deleteFile', function() { - beforeEach(function() { - return this.Fs.unlink.callsArgWith(1, this.error) - }) - - it('Should call unlink with correct options', function(done) { - return this.FSPersistorManager.deleteFile( - this.location, - this.name1, - err => { - this.Fs.unlink - .calledWith(`${this.location}/${this.name1Filtered}`) - .should.equal(true) - return done() - } + it('Should call unlink with correct options', async function() { + await FSPersistorManager.promises.deleteFile(location, files[0]) + expect(fs.unlink).to.have.been.calledWith( + `${location}/${filteredFilenames[0]}` ) }) - return it('Should propogate the error', function(done) { - return this.FSPersistorManager.deleteFile( - this.location, - this.name1, - err => { - err.should.equal(this.error) - return done() - } - ) + it('Should propagate the error', async function() { + fs.unlink.yields(error) + await expect( + FSPersistorManager.promises.deleteFile(location, files[0]) + ).to.eventually.be.rejected.and.have.property('cause', error) }) }) describe('deleteDirectory', function() { - beforeEach(function() { - return this.Rimraf.callsArgWith(1, this.error) - }) - - it('Should call rmdir(rimraf) with correct options', function(done) { - return this.FSPersistorManager.deleteDirectory( - this.location, - this.name1, - err => { - this.Rimraf.calledWith( - `${this.location}/${this.name1Filtered}` - ).should.equal(true) - return done() - } + it('Should call rmdir(rimraf) with correct options', async function() { + await FSPersistorManager.promises.deleteDirectory(location, files[0]) + expect(rimraf).to.have.been.calledWith( + `${location}/${filteredFilenames[0]}` ) }) - return it('Should propogate the error', function(done) { - return this.FSPersistorManager.deleteDirectory( - this.location, - this.name1, - err => { - err.should.equal(this.error) - return done() - } - ) + it('Should propagate the error', async function() { + rimraf.yields(error) + await expect( + FSPersistorManager.promises.deleteDirectory(location, files[0]) + ).to.eventually.be.rejected.and.have.property('cause', error) }) }) describe('checkIfFileExists', function() { + const badFilename = 'pototo' + const noentError = new Error('not found') + noentError.code = 'ENOENT' + beforeEach(function() { - return this.Fs.exists.callsArgWith(1, true) + fs.stat + .yields(error) + .withArgs(`${location}/${filteredFilenames[0]}`) + .yields(null, {}) + .withArgs(`${location}/${badFilename}`) + .yields(noentError) }) - it('Should call exists with correct options', function(done) { - return this.FSPersistorManager.checkIfFileExists( - this.location, - this.name1, - exists => { - this.Fs.exists - .calledWith(`${this.location}/${this.name1Filtered}`) - .should.equal(true) - return done() - } + it('Should call stat with correct options', async function() { + await FSPersistorManager.promises.checkIfFileExists(location, files[0]) + expect(fs.stat).to.have.been.calledWith( + `${location}/${filteredFilenames[0]}` ) }) - // fs.exists simply returns false on any error, so... - it('should not return an error', function(done) { - return this.FSPersistorManager.checkIfFileExists( - this.location, - this.name1, - (err, exists) => { - expect(err).to.be.null - return done() - } - ) + it('Should return true for existing files', async function() { + expect( + await FSPersistorManager.promises.checkIfFileExists(location, files[0]) + ).to.equal(true) }) - it('Should return true for existing files', function(done) { - this.Fs.exists.callsArgWith(1, true) - return this.FSPersistorManager.checkIfFileExists( - this.location, - this.name1, - (err, exists) => { - exists.should.be.true - return done() - } - ) + it('Should return false for non-existing files', async function() { + expect( + await FSPersistorManager.promises.checkIfFileExists( + location, + badFilename + ) + ).to.equal(false) }) - return it('Should return false for non-existing files', function(done) { - this.Fs.exists.callsArgWith(1, false) - return this.FSPersistorManager.checkIfFileExists( - this.location, - this.name1, - (err, exists) => { - exists.should.be.false - return done() - } + it('should wrap the error if there is a problem', async function() { + await expect( + FSPersistorManager.promises.checkIfFileExists(location, 'llama') ) + .to.eventually.be.rejected.and.be.an.instanceOf(Errors.ReadError) + .and.have.property('cause', error) }) }) - return describe('directorySize', function() { - it('should propogate the error', function(done) { - this.Fs.readdir.callsArgWith(1, this.error) - return this.FSPersistorManager.directorySize( - this.location, - this.name1, - (err, totalsize) => { - err.should.equal(this.error) - return done() - } + describe('directorySize', function() { + it('should wrap the error', async function() { + glob.yields(error) + await expect( + 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: 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]}_*` ) }) - return it('should sum directory files size', function(done) { - this.Fs.readdir.callsArgWith(1, null, [ - { file1: 'file1' }, - { file2: 'file2' } - ]) - this.Fs.fstatSync.returns({ size: 1024 }) - return this.FSPersistorManager.directorySize( - this.location, - this.name1, - (err, totalsize) => { - expect(totalsize).to.equal(2048) - return done() - } - ) + it('should sum directory files size', async function() { + expect( + await FSPersistorManager.promises.directorySize(location, files[0]) + ).to.equal(stat.size * files.length) }) }) }) diff --git a/services/filestore/test/unit/js/LocalFileWriterTests.js b/services/filestore/test/unit/js/LocalFileWriterTests.js index 5d7008a91f..ad4d73bce6 100644 --- a/services/filestore/test/unit/js/LocalFileWriterTests.js +++ b/services/filestore/test/unit/js/LocalFileWriterTests.js @@ -49,6 +49,26 @@ describe('LocalFileWriter', function() { 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() { @@ -60,14 +80,6 @@ describe('LocalFileWriter', function() { }) }) - it('should not do anything if called with an empty path', function(done) { - fs.unlink = sinon.stub().yields(new Error('failed to reticulate splines')) - LocalFileWriter.deleteFile(fsPath, err => { - expect(err).to.exist - done() - }) - }) - it('should not call unlink with an empty path', function(done) { LocalFileWriter.deleteFile('', err => { expect(err).not.to.exist @@ -75,5 +87,25 @@ describe('LocalFileWriter', function() { 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() + }) + }) }) })