From 4b543724661fab994b9305dbeac5cfa53b671f7b Mon Sep 17 00:00:00 2001 From: Xavier Trochu Date: Fri, 16 Jan 2015 11:04:15 +0100 Subject: [PATCH 01/14] miscellaneous cleanup _ Remove some references to scribtex _ Fix some issue when templates are unavailable --- services/filestore/.gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/services/filestore/.gitignore b/services/filestore/.gitignore index 7d881c3ca7..365bc9160d 100644 --- a/services/filestore/.gitignore +++ b/services/filestore/.gitignore @@ -40,6 +40,7 @@ test/IntergrationTests/js/* data/*/* app.js +cluster.js app/js/* test/IntergrationTests/js/* test/UnitTests/js/* From 566e69c6cce7769b21845cc37ff63eb9ddc467e7 Mon Sep 17 00:00:00 2001 From: Xavier Trochu Date: Tue, 21 Jul 2015 10:00:59 +0200 Subject: [PATCH 02/14] Make S3 Key/Secret pair optional. When using Instance Role in AWS, the credentials are unneeded. So make them optional to use. --- .../app/coffee/S3PersistorManager.coffee | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/services/filestore/app/coffee/S3PersistorManager.coffee b/services/filestore/app/coffee/S3PersistorManager.coffee index d5cb06074e..c0fab76747 100644 --- a/services/filestore/app/coffee/S3PersistorManager.coffee +++ b/services/filestore/app/coffee/S3PersistorManager.coffee @@ -16,8 +16,8 @@ thirtySeconds = 30 * 1000 buildDefaultOptions = (bucketName, method, key)-> return { aws: - key: settings.filestore.s3.key - secret: settings.filestore.s3.secret + key: settings.filestore.s3?.key + secret: settings.filestore.s3?.secret bucket: bucketName method: method timeout: thirtySeconds @@ -28,10 +28,10 @@ module.exports = sendFile: (bucketName, key, fsPath, callback)-> s3Client = knox.createClient - key: settings.filestore.s3.key - secret: settings.filestore.s3.secret + key: settings.filestore.s3?.key + secret: settings.filestore.s3?.secret bucket: bucketName - putEventEmiter = s3Client.putFile fsPath, key, (err, res)-> + putEventEmitter = s3Client.putFile fsPath, key, (err, res)-> if err? logger.err err:err, bucketName:bucketName, key:key, fsPath:fsPath,"something went wrong uploading file to s3" return callback(err) @@ -44,8 +44,8 @@ module.exports = LocalFileWriter.deleteFile fsPath, (err)-> logger.log res:res, bucketName:bucketName, key:key, fsPath:fsPath,"file uploaded to s3" callback(err) - putEventEmiter.on "error", (err)-> - logger.err err:err, bucketName:bucketName, key:key, fsPath:fsPath, "error emmited on put of file" + putEventEmitter.on "error", (err)-> + logger.err err:err, bucketName:bucketName, key:key, fsPath:fsPath, "error received uploading file to s3" callback err sendStream: (bucketName, key, readStream, callback)-> @@ -62,8 +62,8 @@ module.exports = callback = _.once callback logger.log bucketName:bucketName, key:key, "getting file from s3" s3Client = knox.createClient - key: settings.filestore.s3.key - secret: settings.filestore.s3.secret + key: settings.filestore.s3?.key + secret: settings.filestore.s3?.secret bucket: bucketName s3Stream = s3Client.get(key) s3Stream.end() @@ -76,12 +76,12 @@ module.exports = copyFile: (bucketName, sourceKey, destKey, callback)-> logger.log bucketName:bucketName, sourceKey:sourceKey, destKey:destKey, "copying file in s3" s3Client = knox.createClient - key: settings.filestore.s3.key - secret: settings.filestore.s3.secret + key: settings.filestore.s3?.key + secret: settings.filestore.s3?.secret bucket: bucketName s3Client.copyFile sourceKey, destKey, (err)-> if err? - logger.err err:err, bucketName:bucketName, sourceKey:sourceKey, destKey:destKey, "something went wrong copying file in aws" + logger.err err:err, bucketName:bucketName, sourceKey:sourceKey, destKey:destKey, "something went wrong copying file in s3" callback(err) deleteFile: (bucketName, key, callback)-> @@ -89,7 +89,7 @@ module.exports = options = buildDefaultOptions(bucketName, "delete", key) request options, (err, res)-> if err? - logger.err err:err, res:res, bucketName:bucketName, key:key, "something went wrong deleting file in aws" + logger.err err:err, res:res, bucketName:bucketName, key:key, "something went wrong deleting file in s3" callback(err) deleteDirectory: (bucketName, key, _callback)-> @@ -100,12 +100,12 @@ module.exports = logger.log key: key, bucketName: bucketName, "deleting directory" s3Client = knox.createClient - key: settings.filestore.s3.key - secret: settings.filestore.s3.secret + key: settings.filestore.s3?.key + secret: settings.filestore.s3?.secret bucket: bucketName s3Client.list prefix:key, (err, data)-> if err? - logger.err err:err, bucketName:bucketName, key:key, "something went wrong listing prefix in aws" + logger.err err:err, bucketName:bucketName, key:key, "something went wrong listing prefix in s3" return callback(err) keys = _.map data.Contents, (entry)-> return entry.Key @@ -116,13 +116,13 @@ module.exports = options = buildDefaultOptions(bucketName, "head", key) request options, (err, res)-> if err? - logger.err err:err, res:res, bucketName:bucketName, key:key, "something went wrong checking file in aws" + logger.err err:err, res:res, bucketName:bucketName, key:key, "something went wrong checking file in s3" return callback(err) if !res? logger.err err:err, res:res, bucketName:bucketName, key:key, "no response object returned when checking if file exists" err = new Error("no response from s3 #{bucketName} #{key}") return callback(err) exists = res.statusCode == 200 - logger.log bucketName:bucketName, key:key, exists:exists, "checked if file exsists in s3" + logger.log bucketName:bucketName, key:key, exists:exists, "checked if file exists in s3" callback(err, exists) From 3289451ecef2244cea81a2b3e9c3906d46849d00 Mon Sep 17 00:00:00 2001 From: Xavier Trochu Date: Wed, 22 Jul 2015 16:41:45 +0200 Subject: [PATCH 03/14] Revert "Make S3 Key/Secret pair optional." This reverts commit 65122e603099a4f9ef5c5186b7eb8832340a993f. knox requires the key and secret arguments. --- .../app/coffee/S3PersistorManager.coffee | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/services/filestore/app/coffee/S3PersistorManager.coffee b/services/filestore/app/coffee/S3PersistorManager.coffee index c0fab76747..d5cb06074e 100644 --- a/services/filestore/app/coffee/S3PersistorManager.coffee +++ b/services/filestore/app/coffee/S3PersistorManager.coffee @@ -16,8 +16,8 @@ thirtySeconds = 30 * 1000 buildDefaultOptions = (bucketName, method, key)-> return { aws: - key: settings.filestore.s3?.key - secret: settings.filestore.s3?.secret + key: settings.filestore.s3.key + secret: settings.filestore.s3.secret bucket: bucketName method: method timeout: thirtySeconds @@ -28,10 +28,10 @@ module.exports = sendFile: (bucketName, key, fsPath, callback)-> s3Client = knox.createClient - key: settings.filestore.s3?.key - secret: settings.filestore.s3?.secret + key: settings.filestore.s3.key + secret: settings.filestore.s3.secret bucket: bucketName - putEventEmitter = s3Client.putFile fsPath, key, (err, res)-> + putEventEmiter = s3Client.putFile fsPath, key, (err, res)-> if err? logger.err err:err, bucketName:bucketName, key:key, fsPath:fsPath,"something went wrong uploading file to s3" return callback(err) @@ -44,8 +44,8 @@ module.exports = LocalFileWriter.deleteFile fsPath, (err)-> logger.log res:res, bucketName:bucketName, key:key, fsPath:fsPath,"file uploaded to s3" callback(err) - putEventEmitter.on "error", (err)-> - logger.err err:err, bucketName:bucketName, key:key, fsPath:fsPath, "error received uploading file to s3" + putEventEmiter.on "error", (err)-> + logger.err err:err, bucketName:bucketName, key:key, fsPath:fsPath, "error emmited on put of file" callback err sendStream: (bucketName, key, readStream, callback)-> @@ -62,8 +62,8 @@ module.exports = callback = _.once callback logger.log bucketName:bucketName, key:key, "getting file from s3" s3Client = knox.createClient - key: settings.filestore.s3?.key - secret: settings.filestore.s3?.secret + key: settings.filestore.s3.key + secret: settings.filestore.s3.secret bucket: bucketName s3Stream = s3Client.get(key) s3Stream.end() @@ -76,12 +76,12 @@ module.exports = copyFile: (bucketName, sourceKey, destKey, callback)-> logger.log bucketName:bucketName, sourceKey:sourceKey, destKey:destKey, "copying file in s3" s3Client = knox.createClient - key: settings.filestore.s3?.key - secret: settings.filestore.s3?.secret + key: settings.filestore.s3.key + secret: settings.filestore.s3.secret bucket: bucketName s3Client.copyFile sourceKey, destKey, (err)-> if err? - logger.err err:err, bucketName:bucketName, sourceKey:sourceKey, destKey:destKey, "something went wrong copying file in s3" + logger.err err:err, bucketName:bucketName, sourceKey:sourceKey, destKey:destKey, "something went wrong copying file in aws" callback(err) deleteFile: (bucketName, key, callback)-> @@ -89,7 +89,7 @@ module.exports = options = buildDefaultOptions(bucketName, "delete", key) request options, (err, res)-> if err? - logger.err err:err, res:res, bucketName:bucketName, key:key, "something went wrong deleting file in s3" + logger.err err:err, res:res, bucketName:bucketName, key:key, "something went wrong deleting file in aws" callback(err) deleteDirectory: (bucketName, key, _callback)-> @@ -100,12 +100,12 @@ module.exports = logger.log key: key, bucketName: bucketName, "deleting directory" s3Client = knox.createClient - key: settings.filestore.s3?.key - secret: settings.filestore.s3?.secret + key: settings.filestore.s3.key + secret: settings.filestore.s3.secret bucket: bucketName s3Client.list prefix:key, (err, data)-> if err? - logger.err err:err, bucketName:bucketName, key:key, "something went wrong listing prefix in s3" + logger.err err:err, bucketName:bucketName, key:key, "something went wrong listing prefix in aws" return callback(err) keys = _.map data.Contents, (entry)-> return entry.Key @@ -116,13 +116,13 @@ module.exports = options = buildDefaultOptions(bucketName, "head", key) request options, (err, res)-> if err? - logger.err err:err, res:res, bucketName:bucketName, key:key, "something went wrong checking file in s3" + logger.err err:err, res:res, bucketName:bucketName, key:key, "something went wrong checking file in aws" return callback(err) if !res? logger.err err:err, res:res, bucketName:bucketName, key:key, "no response object returned when checking if file exists" err = new Error("no response from s3 #{bucketName} #{key}") return callback(err) exists = res.statusCode == 200 - logger.log bucketName:bucketName, key:key, exists:exists, "checked if file exists in s3" + logger.log bucketName:bucketName, key:key, exists:exists, "checked if file exsists in s3" callback(err, exists) From 41397821b15d2f371c9dca88de41f4bb3bcda171 Mon Sep 17 00:00:00 2001 From: Xavier Trochu Date: Wed, 22 Jul 2015 16:42:45 +0200 Subject: [PATCH 04/14] Add a manager using the aws-sdk library The knox library does not support the AWS Instance Role. So use the official AWS SDK to connect to S3 --- .../app/coffee/AWSSDKPersistorManager.coffee | 74 +++++++++++++++++++ .../app/coffee/PersistorManager.coffee | 2 + services/filestore/package.json | 1 + 3 files changed, 77 insertions(+) create mode 100644 services/filestore/app/coffee/AWSSDKPersistorManager.coffee diff --git a/services/filestore/app/coffee/AWSSDKPersistorManager.coffee b/services/filestore/app/coffee/AWSSDKPersistorManager.coffee new file mode 100644 index 0000000000..808b0b5219 --- /dev/null +++ b/services/filestore/app/coffee/AWSSDKPersistorManager.coffee @@ -0,0 +1,74 @@ +logger = require "logger-sharelatex" +aws = require "aws-sdk" +_ = require "underscore" +fs = require "fs" + +s3 = aws.S3() + +module.exports = + sendFile: (bucketName, key, fsPath, callback)-> + logger.log bucketName:bucketName, key, "send file data to s3" + stream = fs.createReadStream fsPath + s3.putObject Bucket: bucketName, Key: key, Body: stream, (err, data) -> + if err? + logger.err err: err, Bucket: bucketName, Key: key, "error sending file data to s3" + callback err + + sendStream: (bucketName, key, stream, callback)-> + logger.log bucketName:bucketName, key, "send file stream to s3" + s3.putObject Bucket: bucketName, Key: key, Body: stream, (err, data) -> + if err? + logger.err err: err, Bucket: bucketName, Key: key, "error sending file stream to s3" + callback err + + getFileStream: (bucketName, key, callback = (err, res)->)-> + logger.log bucketName:bucketName, key, "get file stream from s3" + callback = _.once callback + stream = s3.getObject(Bucket:bucketName, Key: key).createReadStream() + stream.on 'response', (res) -> + callback null, res + stream.on 'error', (err) -> + logger.err err:err, bucketName:bucketName, key:key, "error getting file stream from s3" + callback err + + copyFile: (bucketName, sourceKey, destKey, callback)-> + logger.log bucketName:bucketName, sourceKey:sourceKey, destKey: destKey, "copying file in s3" + source = bucketName + '/' + sourceKey + s3.copyObject {Bucket: bucketName, Key: destKey, CopySource: source}, (err) -> + if err? + logger.err err:err, bucketName:bucketName, sourceKey:sourceKey, destKey:destKey, "something went wrong copying file in s3" + callback err + + deleteFile: (bucketName, key, callback)-> + logger.log bucketName:bucketName, key:key, "delete file in s3" + s3.deleteObject {Bucket: bucketName, Key: key}, (err) -> + if err? + logger.err err:err, bucketName:bucketName, key:key, "something went wrong deleting file in s3" + callback err + + deleteDirectory: (bucketName, key, callback)-> + logger.log bucketName:bucketName, key:key, "delete directory in s3" + s3.listObjects {Bucket: bucketName, prefix: key}, (err, data) -> + if err? + logger.err err:err, bucketName:bucketName, key:key, "something went wrong listing prefix in s3" + return callback err + keys = _.map data.Contents, (entry)-> + Key: entry.Key + s3.deleteMultiple + Bucket: bucketName + Delete: + Objects: keys + Quiet: true + , (err) -> + if err? + logger.err err:err, bucketName:bucketName, key:key, "something went wrong deleting directory in s3" + callback err + + checkIfFileExists:(bucketName, key, callback)-> + logger.log bucketName:bucketName, key:key, "check file existence in s3" + s3.headObject {Bucket: bucketName, Key: key}, (err, data) -> + if err? + logger.err err:err, bucketName:bucketName, key:key, "something went wrong checking head in s3" + return callback err + callback null, data.ETag? + diff --git a/services/filestore/app/coffee/PersistorManager.coffee b/services/filestore/app/coffee/PersistorManager.coffee index 1dad923098..aa5c80599d 100644 --- a/services/filestore/app/coffee/PersistorManager.coffee +++ b/services/filestore/app/coffee/PersistorManager.coffee @@ -7,6 +7,8 @@ settings.filestore.backend ||= "s3" logger.log backend:settings.filestore.backend, "Loading backend" module.exports = switch settings.filestore.backend + when "aws-sdk" + require "./AWSSDKPersistorManager" when "s3" require("./S3PersistorManager") when "fs" diff --git a/services/filestore/package.json b/services/filestore/package.json index dd84289f30..441d4ad7e3 100644 --- a/services/filestore/package.json +++ b/services/filestore/package.json @@ -8,6 +8,7 @@ }, "dependencies": { "async": "~0.2.10", + "aws-sdk": "^2.1.39", "bunyan": "^1.3.5", "coffee-script": "~1.7.1", "express": "~3.4.8", From 7ef46a79a0cbd4823eb94c5fefad18bf79715154 Mon Sep 17 00:00:00 2001 From: Xavier Trochu Date: Fri, 24 Jul 2015 09:06:52 +0200 Subject: [PATCH 05/14] Fix aws-sdk persistor. Also fix some typos. --- .../app/coffee/AWSSDKPersistorManager.coffee | 22 +++++++----- .../app/coffee/FileController.coffee | 2 +- .../filestore/app/coffee/FileHandler.coffee | 36 +++++++++---------- 3 files changed, 32 insertions(+), 28 deletions(-) diff --git a/services/filestore/app/coffee/AWSSDKPersistorManager.coffee b/services/filestore/app/coffee/AWSSDKPersistorManager.coffee index 808b0b5219..311997ed03 100644 --- a/services/filestore/app/coffee/AWSSDKPersistorManager.coffee +++ b/services/filestore/app/coffee/AWSSDKPersistorManager.coffee @@ -7,7 +7,7 @@ s3 = aws.S3() module.exports = sendFile: (bucketName, key, fsPath, callback)-> - logger.log bucketName:bucketName, key, "send file data to s3" + logger.log bucketName:bucketName, key:key, "send file data to s3" stream = fs.createReadStream fsPath s3.putObject Bucket: bucketName, Key: key, Body: stream, (err, data) -> if err? @@ -15,18 +15,19 @@ module.exports = callback err sendStream: (bucketName, key, stream, callback)-> - logger.log bucketName:bucketName, key, "send file stream to s3" + logger.log bucketName:bucketName, key:key, "send file stream to s3" s3.putObject Bucket: bucketName, Key: key, Body: stream, (err, data) -> if err? logger.err err: err, Bucket: bucketName, Key: key, "error sending file stream to s3" callback err getFileStream: (bucketName, key, callback = (err, res)->)-> - logger.log bucketName:bucketName, key, "get file stream from s3" + logger.log bucketName:bucketName, key:key, "get file stream from s3" callback = _.once callback - stream = s3.getObject(Bucket:bucketName, Key: key).createReadStream() - stream.on 'response', (res) -> - callback null, res + request = s3.getObject(Bucket:bucketName, Key: key) + stream = request.createReadStream() + stream.on 'readable', () -> + callback null, stream stream.on 'error', (err) -> logger.err err:err, bucketName:bucketName, key:key, "error getting file stream from s3" callback err @@ -48,20 +49,23 @@ module.exports = deleteDirectory: (bucketName, key, callback)-> logger.log bucketName:bucketName, key:key, "delete directory in s3" - s3.listObjects {Bucket: bucketName, prefix: key}, (err, data) -> + s3.listObjects {Bucket: bucketName, Prefix: key}, (err, data) -> if err? logger.err err:err, bucketName:bucketName, key:key, "something went wrong listing prefix in s3" return callback err + if data.Contents.length == 0 + logger.log bucketName:bucketName, key:key, "the directory is empty" + return callback() keys = _.map data.Contents, (entry)-> Key: entry.Key - s3.deleteMultiple + s3.deleteObjects Bucket: bucketName Delete: Objects: keys Quiet: true , (err) -> if err? - logger.err err:err, bucketName:bucketName, key:key, "something went wrong deleting directory in s3" + logger.err err:err, bucketName:bucketName, key:keys, "something went wrong deleting directory in s3" callback err checkIfFileExists:(bucketName, key, callback)-> diff --git a/services/filestore/app/coffee/FileController.coffee b/services/filestore/app/coffee/FileController.coffee index 3b83e203fd..921faa6c6e 100644 --- a/services/filestore/app/coffee/FileController.coffee +++ b/services/filestore/app/coffee/FileController.coffee @@ -11,7 +11,7 @@ module.exports = metrics.inc "getFile" {key, bucket} = req {format, style} = req.query - logger.log key:key, bucket:bucket, format:format, style:style, "reciving request to get file" + logger.log key:key, bucket:bucket, format:format, style:style, "receiving request to get file" FileHandler.getFile bucket, key, {format:format,style:style}, (err, fileStream)-> if err? logger.err err:err, key:key, bucket:bucket, format:format, style:style, "problem getting file" diff --git a/services/filestore/app/coffee/FileHandler.coffee b/services/filestore/app/coffee/FileHandler.coffee index 51aec4bba6..ece26c6164 100644 --- a/services/filestore/app/coffee/FileHandler.coffee +++ b/services/filestore/app/coffee/FileHandler.coffee @@ -10,16 +10,16 @@ ImageOptimiser = require("./ImageOptimiser") module.exports = insertFile: (bucket, key, stream, callback)-> - convetedKey = KeyBuilder.getConvertedFolderKey(key) - PersistorManager.deleteDirectory bucket, convetedKey, (error) -> + convertedKey = KeyBuilder.getConvertedFolderKey key + PersistorManager.deleteDirectory bucket, convertedKey, (error) -> return callback(error) if error? PersistorManager.sendStream bucket, key, stream, callback deleteFile: (bucket, key, callback)-> - convetedKey = KeyBuilder.getConvertedFolderKey(key) + convertedKey = KeyBuilder.getConvertedFolderKey key async.parallel [ (done)-> PersistorManager.deleteFile bucket, key, done - (done)-> PersistorManager.deleteDirectory bucket, convetedKey, done + (done)-> PersistorManager.deleteDirectory bucket, convertedKey, done ], callback getFile: (bucket, key, opts = {}, callback)-> @@ -36,14 +36,14 @@ module.exports = callback err, fileStream _getConvertedFile: (bucket, key, opts, callback)-> - convetedKey = KeyBuilder.addCachingToKey(key, opts) - PersistorManager.checkIfFileExists bucket, convetedKey, (err, exists)=> + convertedKey = KeyBuilder.addCachingToKey key, opts + PersistorManager.checkIfFileExists bucket, convertedKey, (err, exists)=> if exists - PersistorManager.getFileStream bucket, convetedKey, callback + PersistorManager.getFileStream bucket, convertedKey, callback else - @_getConvertedFileAndCache bucket, key, convetedKey, opts, callback + @_getConvertedFileAndCache bucket, key, convertedKey, opts, callback - _getConvertedFileAndCache: (bucket, key, convetedKey, opts, callback)-> + _getConvertedFileAndCache: (bucket, key, convertedKey, opts, callback)-> self = @ convertedFsPath = "" async.series [ @@ -54,27 +54,27 @@ module.exports = (cb)-> ImageOptimiser.compressPng convertedFsPath, cb (cb)-> - PersistorManager.sendFile bucket, convetedKey, convertedFsPath, cb + PersistorManager.sendFile bucket, convertedKey, convertedFsPath, cb ], (err)-> if err? return callback(err) - PersistorManager.getFileStream bucket, convetedKey, callback + PersistorManager.getFileStream bucket, convertedKey, callback - _convertFile: (bucket, origonalKey, opts, callback)-> - @_writeS3FileToDisk bucket, origonalKey, (err, origonalFsPath)-> + _convertFile: (bucket, originalKey, opts, callback)-> + @_writeS3FileToDisk bucket, originalKey, (err, originalFsPath)-> done = (err, destPath)-> if err? - logger.err err:err, bucket:bucket, origonalKey:origonalKey, opts:opts, "error converting file" + logger.err err:err, bucket:bucket, originalKey:originalKey, opts:opts, "error converting file" return callback(err) - LocalFileWriter.deleteFile origonalFsPath, -> + LocalFileWriter.deleteFile originalFsPath, -> callback(err, destPath) if opts.format? - FileConverter.convert origonalFsPath, opts.format, done + FileConverter.convert originalFsPath, opts.format, done else if opts.style == "thumbnail" - FileConverter.thumbnail origonalFsPath, done + FileConverter.thumbnail originalFsPath, done else if opts.style == "preview" - FileConverter.preview origonalFsPath, done + FileConverter.preview originalFsPath, done else throw new Error("should have specified opts to convert file with #{JSON.stringify(opts)}") From f417b9b171ec7b48f183d2b827c7295665f0574a Mon Sep 17 00:00:00 2001 From: Xavier Trochu Date: Fri, 24 Jul 2015 15:48:46 +0200 Subject: [PATCH 06/14] Fix the AWS-SDK Persistor --- services/filestore/app/coffee/AWSSDKPersistorManager.coffee | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/services/filestore/app/coffee/AWSSDKPersistorManager.coffee b/services/filestore/app/coffee/AWSSDKPersistorManager.coffee index 311997ed03..e1474fa009 100644 --- a/services/filestore/app/coffee/AWSSDKPersistorManager.coffee +++ b/services/filestore/app/coffee/AWSSDKPersistorManager.coffee @@ -3,20 +3,20 @@ aws = require "aws-sdk" _ = require "underscore" fs = require "fs" -s3 = aws.S3() +s3 = new aws.S3() module.exports = sendFile: (bucketName, key, fsPath, callback)-> logger.log bucketName:bucketName, key:key, "send file data to s3" stream = fs.createReadStream fsPath - s3.putObject Bucket: bucketName, Key: key, Body: stream, (err, data) -> + s3.upload Bucket: bucketName, Key: key, Body: stream, (err, data) -> if err? logger.err err: err, Bucket: bucketName, Key: key, "error sending file data to s3" callback err sendStream: (bucketName, key, stream, callback)-> logger.log bucketName:bucketName, key:key, "send file stream to s3" - s3.putObject Bucket: bucketName, Key: key, Body: stream, (err, data) -> + s3.upload Bucket: bucketName, Key: key, Body: stream, (err, data) -> if err? logger.err err: err, Bucket: bucketName, Key: key, "error sending file stream to s3" callback err From 63aef6b832cf080ca8cc9d662eeff94f9418a60b Mon Sep 17 00:00:00 2001 From: Xavier Trochu Date: Thu, 5 Nov 2015 12:11:02 +0100 Subject: [PATCH 07/14] Fix postmerge issue --- services/filestore/app/coffee/FileHandler.coffee | 7 ------- 1 file changed, 7 deletions(-) diff --git a/services/filestore/app/coffee/FileHandler.coffee b/services/filestore/app/coffee/FileHandler.coffee index b11036465a..a0e80dde71 100644 --- a/services/filestore/app/coffee/FileHandler.coffee +++ b/services/filestore/app/coffee/FileHandler.coffee @@ -60,19 +60,12 @@ module.exports = ], (err)-> if err? return callback(err) -<<<<<<< HEAD PersistorManager.getFileStream bucket, convertedKey, callback _convertFile: (bucket, originalKey, opts, callback)-> @_writeS3FileToDisk bucket, originalKey, (err, originalFsPath)-> -======= - PersistorManager.getFileStream bucket, convetedKey, opts, callback - - _convertFile: (bucket, origonalKey, opts, callback)-> - @_writeS3FileToDisk bucket, origonalKey, opts, (err, origonalFsPath)-> if err? return callback(err) ->>>>>>> sharelatex/master done = (err, destPath)-> if err? logger.err err:err, bucket:bucket, originalKey:originalKey, opts:opts, "error converting file" From 26523b011a58288ef768d4536995b2b3a2797d92 Mon Sep 17 00:00:00 2001 From: Xavier Trochu Date: Mon, 16 Nov 2015 09:59:27 +0100 Subject: [PATCH 08/14] small fixes --- services/filestore/app/coffee/FSPersistorManager.coffee | 5 ++--- services/filestore/app/coffee/FileHandler.coffee | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/services/filestore/app/coffee/FSPersistorManager.coffee b/services/filestore/app/coffee/FSPersistorManager.coffee index 2ade1f3a5b..d1b72806cf 100644 --- a/services/filestore/app/coffee/FSPersistorManager.coffee +++ b/services/filestore/app/coffee/FSPersistorManager.coffee @@ -3,6 +3,7 @@ fs = require("fs") LocalFileWriter = require("./LocalFileWriter") Errors = require('./Errors') rimraf = require("rimraf") +_ = require "underscore" filterName = (key) -> return key.replace /\//g, "_" @@ -29,9 +30,7 @@ module.exports = # opts may be {start: Number, end: Number} getFileStream: (location, name, opts, _callback = (err, res)->) -> - callback = (args...) -> - _callback(args...) - _callback = () -> + callback = _.once _callback filteredName = filterName name logger.log location:location, name:filteredName, "getting file" sourceStream = fs.createReadStream "#{location}/#{filteredName}", opts diff --git a/services/filestore/app/coffee/FileHandler.coffee b/services/filestore/app/coffee/FileHandler.coffee index a0e80dde71..917df653db 100644 --- a/services/filestore/app/coffee/FileHandler.coffee +++ b/services/filestore/app/coffee/FileHandler.coffee @@ -63,7 +63,7 @@ module.exports = PersistorManager.getFileStream bucket, convertedKey, callback _convertFile: (bucket, originalKey, opts, callback)-> - @_writeS3FileToDisk bucket, originalKey, (err, originalFsPath)-> + @_writeS3FileToDisk bucket, originalKey, opts, (err, originalFsPath)-> if err? return callback(err) done = (err, destPath)-> From 476db58c3f77f6c2f9f179ba2ffed940f573c051 Mon Sep 17 00:00:00 2001 From: Xavier Trochu Date: Tue, 17 Nov 2015 12:52:25 +0100 Subject: [PATCH 09/14] Add the opts parameter to getFileStream and implement Range requests --- .../filestore/app/coffee/AWSSDKPersistorManager.coffee | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/services/filestore/app/coffee/AWSSDKPersistorManager.coffee b/services/filestore/app/coffee/AWSSDKPersistorManager.coffee index e1474fa009..13ba460ae3 100644 --- a/services/filestore/app/coffee/AWSSDKPersistorManager.coffee +++ b/services/filestore/app/coffee/AWSSDKPersistorManager.coffee @@ -21,10 +21,15 @@ module.exports = logger.err err: err, Bucket: bucketName, Key: key, "error sending file stream to s3" callback err - getFileStream: (bucketName, key, callback = (err, res)->)-> + getFileStream: (bucketName, key, opts, callback = (err, res)->)-> logger.log bucketName:bucketName, key:key, "get file stream from s3" callback = _.once callback - request = s3.getObject(Bucket:bucketName, Key: key) + params = + Bucket:bucketName + Key: key + if opts.start? and opts.end? + params['Range'] = "bytes=#{opts.start}-#{opts.end}" + request = s3.getObject params stream = request.createReadStream() stream.on 'readable', () -> callback null, stream From bfd41fdaf9c6996bc8b281a70be22da4403f4c63 Mon Sep 17 00:00:00 2001 From: Xavier Trochu Date: Fri, 20 Nov 2015 12:02:22 +0100 Subject: [PATCH 10/14] Add aws-sdk unit test. Fix Aws-Sdk persistor to return a correct error on file not found. Fix FileHandler after some change were lost on a previous merge --- .../app/coffee/AWSSDKPersistorManager.coffee | 3 + .../filestore/app/coffee/FileHandler.coffee | 9 +- .../coffee/AWSSDKPersistorManagerTests.coffee | 249 ++++++++++++++++++ 3 files changed, 256 insertions(+), 5 deletions(-) create mode 100644 services/filestore/test/unit/coffee/AWSSDKPersistorManagerTests.coffee diff --git a/services/filestore/app/coffee/AWSSDKPersistorManager.coffee b/services/filestore/app/coffee/AWSSDKPersistorManager.coffee index 13ba460ae3..b1465c126d 100644 --- a/services/filestore/app/coffee/AWSSDKPersistorManager.coffee +++ b/services/filestore/app/coffee/AWSSDKPersistorManager.coffee @@ -2,6 +2,7 @@ logger = require "logger-sharelatex" aws = require "aws-sdk" _ = require "underscore" fs = require "fs" +Errors = require("./Errors") s3 = new aws.S3() @@ -35,6 +36,8 @@ module.exports = callback null, stream stream.on 'error', (err) -> logger.err err:err, bucketName:bucketName, key:key, "error getting file stream from s3" + if err.code == 'NoSuchKey' + return callback new Errors.NotFoundError "File not found in S3: #{bucketName}:#{key}" callback err copyFile: (bucketName, sourceKey, destKey, callback)-> diff --git a/services/filestore/app/coffee/FileHandler.coffee b/services/filestore/app/coffee/FileHandler.coffee index 917df653db..a7f9ed84b7 100644 --- a/services/filestore/app/coffee/FileHandler.coffee +++ b/services/filestore/app/coffee/FileHandler.coffee @@ -41,16 +41,15 @@ module.exports = if err? return callback err if exists - PersistorManager.getFileStream bucket, convertedKey, callback + PersistorManager.getFileStream bucket, convertedKey, null, callback else @_getConvertedFileAndCache bucket, key, convertedKey, opts, callback _getConvertedFileAndCache: (bucket, key, convertedKey, opts, callback)-> - self = @ convertedFsPath = "" async.series [ - (cb)-> - self._convertFile bucket, key, opts, (err, fileSystemPath)-> + (cb) => + @_convertFile bucket, key, opts, (err, fileSystemPath) -> convertedFsPath = fileSystemPath cb err (cb)-> @@ -60,7 +59,7 @@ module.exports = ], (err)-> if err? return callback(err) - PersistorManager.getFileStream bucket, convertedKey, callback + PersistorManager.getFileStream bucket, convertedKey, opts, callback _convertFile: (bucket, originalKey, opts, callback)-> @_writeS3FileToDisk bucket, originalKey, opts, (err, originalFsPath)-> diff --git a/services/filestore/test/unit/coffee/AWSSDKPersistorManagerTests.coffee b/services/filestore/test/unit/coffee/AWSSDKPersistorManagerTests.coffee new file mode 100644 index 0000000000..a3ec3db5ba --- /dev/null +++ b/services/filestore/test/unit/coffee/AWSSDKPersistorManagerTests.coffee @@ -0,0 +1,249 @@ +sinon = require 'sinon' +chai = require 'chai' + +should = chai.should() +expect = chai.expect + +modulePath = "../../../app/js/AWSSDKPersistorManager.js" +SandboxedModule = require 'sandboxed-module' + +describe "AWSSDKPersistorManager", -> + beforeEach -> + @settings = + filestore: + backend: "aws-sdk" + @s3 = + upload: sinon.stub() + getObject: sinon.stub() + copyObject: sinon.stub() + deleteObject: sinon.stub() + listObjects: sinon.stub() + deleteObjects: sinon.stub() + headObject: sinon.stub() + @awssdk = + S3: sinon.stub().returns @s3 + + @requires = + "aws-sdk": @awssdk + "settings-sharelatex": @settings + "logger-sharelatex": + log:-> + err:-> + "fs": @fs = + createReadStream: sinon.stub() + "./Errors": @Errors = + NotFoundError: sinon.stub() + @key = "my/key" + @bucketName = "my-bucket" + @error = "my error" + @AWSSDKPersistorManager = SandboxedModule.require modulePath, requires: @requires + + describe "sendFile", -> + beforeEach -> + @stream = {} + @fsPath = "/usr/local/some/file" + @fs.createReadStream.returns @stream + + it "should put the file with s3.upload", (done) -> + @s3.upload.callsArgWith 1 + @AWSSDKPersistorManager.sendFile @bucketName, @key, @fsPath, (err) => + expect(err).to.not.be.ok + expect(@s3.upload.calledOnce, "called only once").to.be.true + expect((@s3.upload.calledWith Bucket: @bucketName, Key: @key, Body: @stream) + , "called with correct arguments").to.be.true + done() + + it "should dispatch the error from s3.upload", (done) -> + @s3.upload.callsArgWith 1, @error + @AWSSDKPersistorManager.sendFile @bucketName, @key, @fsPath, (err) => + expect(err).to.equal @error + done() + + + describe "sendStream", -> + beforeEach -> + @stream = {} + + it "should put the file with s3.upload", (done) -> + @s3.upload.callsArgWith 1 + @AWSSDKPersistorManager.sendStream @bucketName, @key, @stream, (err) => + expect(err).to.not.be.ok + expect(@s3.upload.calledOnce, "called only once").to.be.true + expect((@s3.upload.calledWith Bucket: @bucketName, Key: @key, Body: @stream), + "called with correct arguments").to.be.true + done() + + it "should dispatch the error from s3.upload", (done) -> + @s3.upload.callsArgWith 1, @error + @AWSSDKPersistorManager.sendStream @bucketName, @key, @stream, (err) => + expect(err).to.equal @error + done() + + describe "getFileStream", -> + beforeEach -> + @opts = {} + @stream = {} + @read_stream = + on: @read_stream_on = sinon.stub() + @object = + createReadStream: sinon.stub().returns @read_stream + @s3.getObject.returns @object + + it "should return a stream from s3.getObject", (done) -> + @read_stream_on.withArgs('readable').callsArgWith 1 + + @AWSSDKPersistorManager.getFileStream @bucketName, @key, @opts, (err, stream) => + expect(@read_stream_on.calledTwice) + expect(err).to.not.be.ok + expect(stream, "returned the stream").to.equal @read_stream + expect((@s3.getObject.calledWith Bucket: @bucketName, Key: @key), + "called with correct arguments").to.be.true + done() + + describe "with start and end options", -> + beforeEach -> + @opts = + start: 0 + end: 8 + it "should pass headers to the s3.GetObject", (done) -> + @read_stream_on.withArgs('readable').callsArgWith 1 + @AWSSDKPersistorManager.getFileStream @bucketName, @key, @opts, (err, stream) => + expect((@s3.getObject.calledWith Bucket: @bucketName, Key: @key, Range: 'bytes=0-8'), + "called with correct arguments").to.be.true + done() + + describe "error conditions", -> + describe "when the file doesn't exist", -> + beforeEach -> + @error = new Error() + @error.code = 'NoSuchKey' + it "should produce a NotFoundError", (done) -> + @read_stream_on.withArgs('error').callsArgWith 1, @error + @AWSSDKPersistorManager.getFileStream @bucketName, @key, @opts, (err, stream) => + expect(stream).to.not.be.ok + expect(err).to.be.ok + expect(err instanceof @Errors.NotFoundError, "error is a correct instance").to.equal true + done() + + describe "when there is some other error", -> + beforeEach -> + @error = new Error() + it "should dispatch the error from s3 object stream", (done) -> + @read_stream_on.withArgs('error').callsArgWith 1, @error + @AWSSDKPersistorManager.getFileStream @bucketName, @key, @opts, (err, stream) => + expect(stream).to.not.be.ok + expect(err).to.be.ok + expect(err).to.equal @error + done() + + describe "copyFile", -> + beforeEach -> + @destKey = "some/key" + @stream = {} + + it "should copy the file with s3.copyObject", (done) -> + @s3.copyObject.callsArgWith 1 + @AWSSDKPersistorManager.copyFile @bucketName, @key, @destKey, (err) => + expect(err).to.not.be.ok + expect(@s3.copyObject.calledOnce, "called only once").to.be.true + expect((@s3.copyObject.calledWith Bucket: @bucketName, Key: @destKey, CopySource: @bucketName + '/' + @key), + "called with correct arguments").to.be.true + done() + + it "should dispatch the error from s3.copyObject", (done) -> + @s3.copyObject.callsArgWith 1, @error + @AWSSDKPersistorManager.copyFile @bucketName, @key, @destKey, (err) => + expect(err).to.equal @error + done() + + describe "deleteFile", -> + it "should delete the file with s3.deleteObject", (done) -> + @s3.deleteObject.callsArgWith 1 + @AWSSDKPersistorManager.deleteFile @bucketName, @key, (err) => + expect(err).to.not.be.ok + expect(@s3.deleteObject.calledOnce, "called only once").to.be.true + expect((@s3.deleteObject.calledWith Bucket: @bucketName, Key: @key), + "called with correct arguments").to.be.true + done() + + it "should dispatch the error from s3.deleteObject", (done) -> + @s3.deleteObject.callsArgWith 1, @error + @AWSSDKPersistorManager.deleteFile @bucketName, @key, (err) => + expect(err).to.equal @error + done() + + describe "deleteDirectory", -> + + it "should list the directory content using s3.listObjects", (done) -> + @s3.listObjects.callsArgWith 1, null, Contents: [] + @AWSSDKPersistorManager.deleteDirectory @bucketName, @key, (err) => + expect(err).to.not.be.ok + expect(@s3.listObjects.calledOnce, "called only once").to.be.true + expect((@s3.listObjects.calledWith Bucket: @bucketName, Prefix: @key), + "called with correct arguments").to.be.true + done() + + it "should dispatch the error from s3.listObjects", (done) -> + @s3.listObjects.callsArgWith 1, @error + @AWSSDKPersistorManager.deleteDirectory @bucketName, @key, (err) => + expect(err).to.equal @error + done() + + describe "with directory content", -> + beforeEach -> + @fileList = [ + Key: 'foo' + , Key: 'bar' + , Key: 'baz' + ] + + it "should forward the file keys to s3.deleteObjects", (done) -> + @s3.listObjects.callsArgWith 1, null, Contents: @fileList + @s3.deleteObjects.callsArgWith 1 + @AWSSDKPersistorManager.deleteDirectory @bucketName, @key, (err) => + expect(err).to.not.be.ok + expect(@s3.deleteObjects.calledOnce, "called only once").to.be.true + expect((@s3.deleteObjects.calledWith + Bucket: @bucketName + Delete: + Quiet: true + Objects: @fileList), + "called with correct arguments").to.be.true + done() + + it "should dispatch the error from s3.deleteObjects", (done) -> + @s3.listObjects.callsArgWith 1, null, Contents: @fileList + @s3.deleteObjects.callsArgWith 1, @error + @AWSSDKPersistorManager.deleteDirectory @bucketName, @key, (err) => + expect(err).to.equal @error + done() + + + describe "checkIfFileExists", -> + + it "should check for the file with s3.headObject", (done) -> + @s3.headObject.callsArgWith 1, null, {} + @AWSSDKPersistorManager.checkIfFileExists @bucketName, @key, (err, exists) => + expect(err).to.not.be.ok + expect(@s3.headObject.calledOnce, "called only once").to.be.true + expect((@s3.headObject.calledWith Bucket: @bucketName, Key: @key), + "called with correct arguments").to.be.true + done() + + it "should return false on an inexistant file", (done) -> + @s3.headObject.callsArgWith 1, null, {} + @AWSSDKPersistorManager.checkIfFileExists @bucketName, @key, (err, exists) => + expect(exists).to.be.false + done() + + it "should return true on an existing file", (done) -> + @s3.headObject.callsArgWith 1, null, ETag: "etag" + @AWSSDKPersistorManager.checkIfFileExists @bucketName, @key, (err, exists) => + expect(exists).to.be.true + done() + + it "should dispatch the error from s3.headObject", (done) -> + @s3.headObject.callsArgWith 1, @error + @AWSSDKPersistorManager.checkIfFileExists @bucketName, @key, (err, exists) => + expect(err).to.equal @error + done() From 0219900933fa5b5a33bac7866b152760a10259e3 Mon Sep 17 00:00:00 2001 From: Xavier Trochu Date: Fri, 20 Nov 2015 12:07:04 +0100 Subject: [PATCH 11/14] Forward the options argument to getFileStream --- services/filestore/app/coffee/FileHandler.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/filestore/app/coffee/FileHandler.coffee b/services/filestore/app/coffee/FileHandler.coffee index a7f9ed84b7..f22285edae 100644 --- a/services/filestore/app/coffee/FileHandler.coffee +++ b/services/filestore/app/coffee/FileHandler.coffee @@ -41,7 +41,7 @@ module.exports = if err? return callback err if exists - PersistorManager.getFileStream bucket, convertedKey, null, callback + PersistorManager.getFileStream bucket, convertedKey, opts, callback else @_getConvertedFileAndCache bucket, key, convertedKey, opts, callback From b6486cb825676e1c0699dd32206b265de325dc87 Mon Sep 17 00:00:00 2001 From: Xavier Trochu Date: Fri, 20 Nov 2015 14:05:54 +0100 Subject: [PATCH 12/14] Cleanup the dependencies --- services/filestore/package.json | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/services/filestore/package.json b/services/filestore/package.json index 8e2dfd084b..184899250e 100644 --- a/services/filestore/package.json +++ b/services/filestore/package.json @@ -9,17 +9,11 @@ "dependencies": { "async": "~0.2.10", "aws-sdk": "^2.1.39", - "bunyan": "^1.3.5", "coffee-script": "~1.7.1", "express": "~3.4.8", - "grunt-bunyan": "^0.5.0", - "grunt-execute": "^0.2.2", - "grunt-mocha-test": "~0.8.2", "heapdump": "^0.3.2", "knox": "~0.9.1", "logger-sharelatex": "git+https://github.com/sharelatex/logger-sharelatex.git#v1.1.0", - "longjohn": "~0.2.2", - "lynx": "0.0.11", "metrics-sharelatex": "git+https://github.com/sharelatex/metrics-sharelatex.git#v1.3.0", "node-transloadit": "0.0.4", "node-uuid": "~1.4.1", @@ -37,7 +31,11 @@ "sinon": "", "chai": "", "sandboxed-module": "", + "bunyan": "^1.3.5", "grunt": "0.4.1", + "grunt-bunyan": "^0.5.0", + "grunt-execute": "^0.2.2", + "grunt-mocha-test": "~0.8.2", "grunt-contrib-requirejs": "0.4.1", "grunt-contrib-coffee": "0.7.0", "grunt-contrib-watch": "0.5.3", From 3b6270236c4fd67495b6308ebb87b468088e67af Mon Sep 17 00:00:00 2001 From: Xavier Trochu Date: Fri, 20 Nov 2015 14:38:23 +0100 Subject: [PATCH 13/14] Replace indentation from 2 space to tabs. --- .../app/coffee/AWSSDKPersistorManager.coffee | 142 +++--- .../coffee/AWSSDKPersistorManagerTests.coffee | 416 +++++++++--------- 2 files changed, 279 insertions(+), 279 deletions(-) diff --git a/services/filestore/app/coffee/AWSSDKPersistorManager.coffee b/services/filestore/app/coffee/AWSSDKPersistorManager.coffee index b1465c126d..50e15cfa27 100644 --- a/services/filestore/app/coffee/AWSSDKPersistorManager.coffee +++ b/services/filestore/app/coffee/AWSSDKPersistorManager.coffee @@ -2,85 +2,85 @@ logger = require "logger-sharelatex" aws = require "aws-sdk" _ = require "underscore" fs = require "fs" -Errors = require("./Errors") +Errors = require "./Errors" s3 = new aws.S3() module.exports = - sendFile: (bucketName, key, fsPath, callback)-> - logger.log bucketName:bucketName, key:key, "send file data to s3" - stream = fs.createReadStream fsPath - s3.upload Bucket: bucketName, Key: key, Body: stream, (err, data) -> - if err? - logger.err err: err, Bucket: bucketName, Key: key, "error sending file data to s3" - callback err + sendFile: (bucketName, key, fsPath, callback)-> + logger.log bucketName:bucketName, key:key, "send file data to s3" + stream = fs.createReadStream fsPath + s3.upload Bucket: bucketName, Key: key, Body: stream, (err, data) -> + if err? + logger.err err: err, Bucket: bucketName, Key: key, "error sending file data to s3" + callback err - sendStream: (bucketName, key, stream, callback)-> - logger.log bucketName:bucketName, key:key, "send file stream to s3" - s3.upload Bucket: bucketName, Key: key, Body: stream, (err, data) -> - if err? - logger.err err: err, Bucket: bucketName, Key: key, "error sending file stream to s3" - callback err + sendStream: (bucketName, key, stream, callback)-> + logger.log bucketName:bucketName, key:key, "send file stream to s3" + s3.upload Bucket: bucketName, Key: key, Body: stream, (err, data) -> + if err? + logger.err err: err, Bucket: bucketName, Key: key, "error sending file stream to s3" + callback err - getFileStream: (bucketName, key, opts, callback = (err, res)->)-> - logger.log bucketName:bucketName, key:key, "get file stream from s3" - callback = _.once callback - params = - Bucket:bucketName - Key: key - if opts.start? and opts.end? - params['Range'] = "bytes=#{opts.start}-#{opts.end}" - request = s3.getObject params - stream = request.createReadStream() - stream.on 'readable', () -> - callback null, stream - stream.on 'error', (err) -> - logger.err err:err, bucketName:bucketName, key:key, "error getting file stream from s3" - if err.code == 'NoSuchKey' - return callback new Errors.NotFoundError "File not found in S3: #{bucketName}:#{key}" - callback err + getFileStream: (bucketName, key, opts, callback = (err, res)->)-> + logger.log bucketName:bucketName, key:key, "get file stream from s3" + callback = _.once callback + params = + Bucket:bucketName + Key: key + if opts.start? and opts.end? + params['Range'] = "bytes=#{opts.start}-#{opts.end}" + request = s3.getObject params + stream = request.createReadStream() + stream.on 'readable', () -> + callback null, stream + stream.on 'error', (err) -> + logger.err err:err, bucketName:bucketName, key:key, "error getting file stream from s3" + if err.code == 'NoSuchKey' + return callback new Errors.NotFoundError "File not found in S3: #{bucketName}:#{key}" + callback err - copyFile: (bucketName, sourceKey, destKey, callback)-> - logger.log bucketName:bucketName, sourceKey:sourceKey, destKey: destKey, "copying file in s3" - source = bucketName + '/' + sourceKey - s3.copyObject {Bucket: bucketName, Key: destKey, CopySource: source}, (err) -> - if err? - logger.err err:err, bucketName:bucketName, sourceKey:sourceKey, destKey:destKey, "something went wrong copying file in s3" - callback err + copyFile: (bucketName, sourceKey, destKey, callback)-> + logger.log bucketName:bucketName, sourceKey:sourceKey, destKey: destKey, "copying file in s3" + source = bucketName + '/' + sourceKey + s3.copyObject {Bucket: bucketName, Key: destKey, CopySource: source}, (err) -> + if err? + logger.err err:err, bucketName:bucketName, sourceKey:sourceKey, destKey:destKey, "something went wrong copying file in s3" + callback err - deleteFile: (bucketName, key, callback)-> - logger.log bucketName:bucketName, key:key, "delete file in s3" - s3.deleteObject {Bucket: bucketName, Key: key}, (err) -> - if err? - logger.err err:err, bucketName:bucketName, key:key, "something went wrong deleting file in s3" - callback err + deleteFile: (bucketName, key, callback)-> + logger.log bucketName:bucketName, key:key, "delete file in s3" + s3.deleteObject {Bucket: bucketName, Key: key}, (err) -> + if err? + logger.err err:err, bucketName:bucketName, key:key, "something went wrong deleting file in s3" + callback err - deleteDirectory: (bucketName, key, callback)-> - logger.log bucketName:bucketName, key:key, "delete directory in s3" - s3.listObjects {Bucket: bucketName, Prefix: key}, (err, data) -> - if err? - logger.err err:err, bucketName:bucketName, key:key, "something went wrong listing prefix in s3" - return callback err - if data.Contents.length == 0 - logger.log bucketName:bucketName, key:key, "the directory is empty" - return callback() - keys = _.map data.Contents, (entry)-> - Key: entry.Key - s3.deleteObjects - Bucket: bucketName - Delete: - Objects: keys - Quiet: true - , (err) -> - if err? - logger.err err:err, bucketName:bucketName, key:keys, "something went wrong deleting directory in s3" - callback err + deleteDirectory: (bucketName, key, callback)-> + logger.log bucketName:bucketName, key:key, "delete directory in s3" + s3.listObjects {Bucket: bucketName, Prefix: key}, (err, data) -> + if err? + logger.err err:err, bucketName:bucketName, key:key, "something went wrong listing prefix in s3" + return callback err + if data.Contents.length == 0 + logger.log bucketName:bucketName, key:key, "the directory is empty" + return callback() + keys = _.map data.Contents, (entry)-> + Key: entry.Key + s3.deleteObjects + Bucket: bucketName + Delete: + Objects: keys + Quiet: true + , (err) -> + if err? + logger.err err:err, bucketName:bucketName, key:keys, "something went wrong deleting directory in s3" + callback err - checkIfFileExists:(bucketName, key, callback)-> - logger.log bucketName:bucketName, key:key, "check file existence in s3" - s3.headObject {Bucket: bucketName, Key: key}, (err, data) -> - if err? - logger.err err:err, bucketName:bucketName, key:key, "something went wrong checking head in s3" - return callback err - callback null, data.ETag? + checkIfFileExists:(bucketName, key, callback)-> + logger.log bucketName:bucketName, key:key, "check file existence in s3" + s3.headObject {Bucket: bucketName, Key: key}, (err, data) -> + if err? + logger.err err:err, bucketName:bucketName, key:key, "something went wrong checking head in s3" + return callback err + callback null, data.ETag? diff --git a/services/filestore/test/unit/coffee/AWSSDKPersistorManagerTests.coffee b/services/filestore/test/unit/coffee/AWSSDKPersistorManagerTests.coffee index a3ec3db5ba..0ca8c65ffc 100644 --- a/services/filestore/test/unit/coffee/AWSSDKPersistorManagerTests.coffee +++ b/services/filestore/test/unit/coffee/AWSSDKPersistorManagerTests.coffee @@ -8,242 +8,242 @@ modulePath = "../../../app/js/AWSSDKPersistorManager.js" SandboxedModule = require 'sandboxed-module' describe "AWSSDKPersistorManager", -> - beforeEach -> - @settings = - filestore: - backend: "aws-sdk" - @s3 = - upload: sinon.stub() - getObject: sinon.stub() - copyObject: sinon.stub() - deleteObject: sinon.stub() - listObjects: sinon.stub() - deleteObjects: sinon.stub() - headObject: sinon.stub() - @awssdk = - S3: sinon.stub().returns @s3 + beforeEach -> + @settings = + filestore: + backend: "aws-sdk" + @s3 = + upload: sinon.stub() + getObject: sinon.stub() + copyObject: sinon.stub() + deleteObject: sinon.stub() + listObjects: sinon.stub() + deleteObjects: sinon.stub() + headObject: sinon.stub() + @awssdk = + S3: sinon.stub().returns @s3 - @requires = - "aws-sdk": @awssdk - "settings-sharelatex": @settings - "logger-sharelatex": - log:-> - err:-> - "fs": @fs = - createReadStream: sinon.stub() - "./Errors": @Errors = - NotFoundError: sinon.stub() - @key = "my/key" - @bucketName = "my-bucket" - @error = "my error" - @AWSSDKPersistorManager = SandboxedModule.require modulePath, requires: @requires + @requires = + "aws-sdk": @awssdk + "settings-sharelatex": @settings + "logger-sharelatex": + log:-> + err:-> + "fs": @fs = + createReadStream: sinon.stub() + "./Errors": @Errors = + NotFoundError: sinon.stub() + @key = "my/key" + @bucketName = "my-bucket" + @error = "my error" + @AWSSDKPersistorManager = SandboxedModule.require modulePath, requires: @requires - describe "sendFile", -> - beforeEach -> - @stream = {} - @fsPath = "/usr/local/some/file" - @fs.createReadStream.returns @stream + describe "sendFile", -> + beforeEach -> + @stream = {} + @fsPath = "/usr/local/some/file" + @fs.createReadStream.returns @stream - it "should put the file with s3.upload", (done) -> - @s3.upload.callsArgWith 1 - @AWSSDKPersistorManager.sendFile @bucketName, @key, @fsPath, (err) => - expect(err).to.not.be.ok - expect(@s3.upload.calledOnce, "called only once").to.be.true - expect((@s3.upload.calledWith Bucket: @bucketName, Key: @key, Body: @stream) - , "called with correct arguments").to.be.true - done() + it "should put the file with s3.upload", (done) -> + @s3.upload.callsArgWith 1 + @AWSSDKPersistorManager.sendFile @bucketName, @key, @fsPath, (err) => + expect(err).to.not.be.ok + expect(@s3.upload.calledOnce, "called only once").to.be.true + expect((@s3.upload.calledWith Bucket: @bucketName, Key: @key, Body: @stream) + , "called with correct arguments").to.be.true + done() - it "should dispatch the error from s3.upload", (done) -> - @s3.upload.callsArgWith 1, @error - @AWSSDKPersistorManager.sendFile @bucketName, @key, @fsPath, (err) => - expect(err).to.equal @error - done() + it "should dispatch the error from s3.upload", (done) -> + @s3.upload.callsArgWith 1, @error + @AWSSDKPersistorManager.sendFile @bucketName, @key, @fsPath, (err) => + expect(err).to.equal @error + done() - describe "sendStream", -> - beforeEach -> - @stream = {} + describe "sendStream", -> + beforeEach -> + @stream = {} - it "should put the file with s3.upload", (done) -> - @s3.upload.callsArgWith 1 - @AWSSDKPersistorManager.sendStream @bucketName, @key, @stream, (err) => - expect(err).to.not.be.ok - expect(@s3.upload.calledOnce, "called only once").to.be.true - expect((@s3.upload.calledWith Bucket: @bucketName, Key: @key, Body: @stream), - "called with correct arguments").to.be.true - done() + it "should put the file with s3.upload", (done) -> + @s3.upload.callsArgWith 1 + @AWSSDKPersistorManager.sendStream @bucketName, @key, @stream, (err) => + expect(err).to.not.be.ok + expect(@s3.upload.calledOnce, "called only once").to.be.true + expect((@s3.upload.calledWith Bucket: @bucketName, Key: @key, Body: @stream), + "called with correct arguments").to.be.true + done() - it "should dispatch the error from s3.upload", (done) -> - @s3.upload.callsArgWith 1, @error - @AWSSDKPersistorManager.sendStream @bucketName, @key, @stream, (err) => - expect(err).to.equal @error - done() + it "should dispatch the error from s3.upload", (done) -> + @s3.upload.callsArgWith 1, @error + @AWSSDKPersistorManager.sendStream @bucketName, @key, @stream, (err) => + expect(err).to.equal @error + done() - describe "getFileStream", -> - beforeEach -> - @opts = {} - @stream = {} - @read_stream = - on: @read_stream_on = sinon.stub() - @object = - createReadStream: sinon.stub().returns @read_stream - @s3.getObject.returns @object + describe "getFileStream", -> + beforeEach -> + @opts = {} + @stream = {} + @read_stream = + on: @read_stream_on = sinon.stub() + @object = + createReadStream: sinon.stub().returns @read_stream + @s3.getObject.returns @object - it "should return a stream from s3.getObject", (done) -> - @read_stream_on.withArgs('readable').callsArgWith 1 + it "should return a stream from s3.getObject", (done) -> + @read_stream_on.withArgs('readable').callsArgWith 1 - @AWSSDKPersistorManager.getFileStream @bucketName, @key, @opts, (err, stream) => - expect(@read_stream_on.calledTwice) - expect(err).to.not.be.ok - expect(stream, "returned the stream").to.equal @read_stream - expect((@s3.getObject.calledWith Bucket: @bucketName, Key: @key), - "called with correct arguments").to.be.true - done() + @AWSSDKPersistorManager.getFileStream @bucketName, @key, @opts, (err, stream) => + expect(@read_stream_on.calledTwice) + expect(err).to.not.be.ok + expect(stream, "returned the stream").to.equal @read_stream + expect((@s3.getObject.calledWith Bucket: @bucketName, Key: @key), + "called with correct arguments").to.be.true + done() - describe "with start and end options", -> - beforeEach -> - @opts = - start: 0 - end: 8 - it "should pass headers to the s3.GetObject", (done) -> - @read_stream_on.withArgs('readable').callsArgWith 1 - @AWSSDKPersistorManager.getFileStream @bucketName, @key, @opts, (err, stream) => - expect((@s3.getObject.calledWith Bucket: @bucketName, Key: @key, Range: 'bytes=0-8'), - "called with correct arguments").to.be.true - done() + describe "with start and end options", -> + beforeEach -> + @opts = + start: 0 + end: 8 + it "should pass headers to the s3.GetObject", (done) -> + @read_stream_on.withArgs('readable').callsArgWith 1 + @AWSSDKPersistorManager.getFileStream @bucketName, @key, @opts, (err, stream) => + expect((@s3.getObject.calledWith Bucket: @bucketName, Key: @key, Range: 'bytes=0-8'), + "called with correct arguments").to.be.true + done() - describe "error conditions", -> - describe "when the file doesn't exist", -> - beforeEach -> - @error = new Error() - @error.code = 'NoSuchKey' - it "should produce a NotFoundError", (done) -> - @read_stream_on.withArgs('error').callsArgWith 1, @error - @AWSSDKPersistorManager.getFileStream @bucketName, @key, @opts, (err, stream) => - expect(stream).to.not.be.ok - expect(err).to.be.ok - expect(err instanceof @Errors.NotFoundError, "error is a correct instance").to.equal true - done() + describe "error conditions", -> + describe "when the file doesn't exist", -> + beforeEach -> + @error = new Error() + @error.code = 'NoSuchKey' + it "should produce a NotFoundError", (done) -> + @read_stream_on.withArgs('error').callsArgWith 1, @error + @AWSSDKPersistorManager.getFileStream @bucketName, @key, @opts, (err, stream) => + expect(stream).to.not.be.ok + expect(err).to.be.ok + expect(err instanceof @Errors.NotFoundError, "error is a correct instance").to.equal true + done() - describe "when there is some other error", -> - beforeEach -> - @error = new Error() - it "should dispatch the error from s3 object stream", (done) -> - @read_stream_on.withArgs('error').callsArgWith 1, @error - @AWSSDKPersistorManager.getFileStream @bucketName, @key, @opts, (err, stream) => - expect(stream).to.not.be.ok - expect(err).to.be.ok - expect(err).to.equal @error - done() + describe "when there is some other error", -> + beforeEach -> + @error = new Error() + it "should dispatch the error from s3 object stream", (done) -> + @read_stream_on.withArgs('error').callsArgWith 1, @error + @AWSSDKPersistorManager.getFileStream @bucketName, @key, @opts, (err, stream) => + expect(stream).to.not.be.ok + expect(err).to.be.ok + expect(err).to.equal @error + done() - describe "copyFile", -> - beforeEach -> - @destKey = "some/key" - @stream = {} + describe "copyFile", -> + beforeEach -> + @destKey = "some/key" + @stream = {} - it "should copy the file with s3.copyObject", (done) -> - @s3.copyObject.callsArgWith 1 - @AWSSDKPersistorManager.copyFile @bucketName, @key, @destKey, (err) => - expect(err).to.not.be.ok - expect(@s3.copyObject.calledOnce, "called only once").to.be.true - expect((@s3.copyObject.calledWith Bucket: @bucketName, Key: @destKey, CopySource: @bucketName + '/' + @key), - "called with correct arguments").to.be.true - done() + it "should copy the file with s3.copyObject", (done) -> + @s3.copyObject.callsArgWith 1 + @AWSSDKPersistorManager.copyFile @bucketName, @key, @destKey, (err) => + expect(err).to.not.be.ok + expect(@s3.copyObject.calledOnce, "called only once").to.be.true + expect((@s3.copyObject.calledWith Bucket: @bucketName, Key: @destKey, CopySource: @bucketName + '/' + @key), + "called with correct arguments").to.be.true + done() - it "should dispatch the error from s3.copyObject", (done) -> - @s3.copyObject.callsArgWith 1, @error - @AWSSDKPersistorManager.copyFile @bucketName, @key, @destKey, (err) => - expect(err).to.equal @error - done() + it "should dispatch the error from s3.copyObject", (done) -> + @s3.copyObject.callsArgWith 1, @error + @AWSSDKPersistorManager.copyFile @bucketName, @key, @destKey, (err) => + expect(err).to.equal @error + done() - describe "deleteFile", -> - it "should delete the file with s3.deleteObject", (done) -> - @s3.deleteObject.callsArgWith 1 - @AWSSDKPersistorManager.deleteFile @bucketName, @key, (err) => - expect(err).to.not.be.ok - expect(@s3.deleteObject.calledOnce, "called only once").to.be.true - expect((@s3.deleteObject.calledWith Bucket: @bucketName, Key: @key), - "called with correct arguments").to.be.true - done() + describe "deleteFile", -> + it "should delete the file with s3.deleteObject", (done) -> + @s3.deleteObject.callsArgWith 1 + @AWSSDKPersistorManager.deleteFile @bucketName, @key, (err) => + expect(err).to.not.be.ok + expect(@s3.deleteObject.calledOnce, "called only once").to.be.true + expect((@s3.deleteObject.calledWith Bucket: @bucketName, Key: @key), + "called with correct arguments").to.be.true + done() - it "should dispatch the error from s3.deleteObject", (done) -> - @s3.deleteObject.callsArgWith 1, @error - @AWSSDKPersistorManager.deleteFile @bucketName, @key, (err) => - expect(err).to.equal @error - done() + it "should dispatch the error from s3.deleteObject", (done) -> + @s3.deleteObject.callsArgWith 1, @error + @AWSSDKPersistorManager.deleteFile @bucketName, @key, (err) => + expect(err).to.equal @error + done() - describe "deleteDirectory", -> + describe "deleteDirectory", -> - it "should list the directory content using s3.listObjects", (done) -> - @s3.listObjects.callsArgWith 1, null, Contents: [] - @AWSSDKPersistorManager.deleteDirectory @bucketName, @key, (err) => - expect(err).to.not.be.ok - expect(@s3.listObjects.calledOnce, "called only once").to.be.true - expect((@s3.listObjects.calledWith Bucket: @bucketName, Prefix: @key), - "called with correct arguments").to.be.true - done() + it "should list the directory content using s3.listObjects", (done) -> + @s3.listObjects.callsArgWith 1, null, Contents: [] + @AWSSDKPersistorManager.deleteDirectory @bucketName, @key, (err) => + expect(err).to.not.be.ok + expect(@s3.listObjects.calledOnce, "called only once").to.be.true + expect((@s3.listObjects.calledWith Bucket: @bucketName, Prefix: @key), + "called with correct arguments").to.be.true + done() - it "should dispatch the error from s3.listObjects", (done) -> - @s3.listObjects.callsArgWith 1, @error - @AWSSDKPersistorManager.deleteDirectory @bucketName, @key, (err) => - expect(err).to.equal @error - done() + it "should dispatch the error from s3.listObjects", (done) -> + @s3.listObjects.callsArgWith 1, @error + @AWSSDKPersistorManager.deleteDirectory @bucketName, @key, (err) => + expect(err).to.equal @error + done() - describe "with directory content", -> - beforeEach -> - @fileList = [ - Key: 'foo' - , Key: 'bar' - , Key: 'baz' - ] + describe "with directory content", -> + beforeEach -> + @fileList = [ + Key: 'foo' + , Key: 'bar' + , Key: 'baz' + ] - it "should forward the file keys to s3.deleteObjects", (done) -> - @s3.listObjects.callsArgWith 1, null, Contents: @fileList - @s3.deleteObjects.callsArgWith 1 - @AWSSDKPersistorManager.deleteDirectory @bucketName, @key, (err) => - expect(err).to.not.be.ok - expect(@s3.deleteObjects.calledOnce, "called only once").to.be.true - expect((@s3.deleteObjects.calledWith - Bucket: @bucketName - Delete: - Quiet: true - Objects: @fileList), - "called with correct arguments").to.be.true - done() + it "should forward the file keys to s3.deleteObjects", (done) -> + @s3.listObjects.callsArgWith 1, null, Contents: @fileList + @s3.deleteObjects.callsArgWith 1 + @AWSSDKPersistorManager.deleteDirectory @bucketName, @key, (err) => + expect(err).to.not.be.ok + expect(@s3.deleteObjects.calledOnce, "called only once").to.be.true + expect((@s3.deleteObjects.calledWith + Bucket: @bucketName + Delete: + Quiet: true + Objects: @fileList), + "called with correct arguments").to.be.true + done() - it "should dispatch the error from s3.deleteObjects", (done) -> - @s3.listObjects.callsArgWith 1, null, Contents: @fileList - @s3.deleteObjects.callsArgWith 1, @error - @AWSSDKPersistorManager.deleteDirectory @bucketName, @key, (err) => - expect(err).to.equal @error - done() + it "should dispatch the error from s3.deleteObjects", (done) -> + @s3.listObjects.callsArgWith 1, null, Contents: @fileList + @s3.deleteObjects.callsArgWith 1, @error + @AWSSDKPersistorManager.deleteDirectory @bucketName, @key, (err) => + expect(err).to.equal @error + done() - describe "checkIfFileExists", -> + describe "checkIfFileExists", -> - it "should check for the file with s3.headObject", (done) -> - @s3.headObject.callsArgWith 1, null, {} - @AWSSDKPersistorManager.checkIfFileExists @bucketName, @key, (err, exists) => - expect(err).to.not.be.ok - expect(@s3.headObject.calledOnce, "called only once").to.be.true - expect((@s3.headObject.calledWith Bucket: @bucketName, Key: @key), - "called with correct arguments").to.be.true - done() + it "should check for the file with s3.headObject", (done) -> + @s3.headObject.callsArgWith 1, null, {} + @AWSSDKPersistorManager.checkIfFileExists @bucketName, @key, (err, exists) => + expect(err).to.not.be.ok + expect(@s3.headObject.calledOnce, "called only once").to.be.true + expect((@s3.headObject.calledWith Bucket: @bucketName, Key: @key), + "called with correct arguments").to.be.true + done() - it "should return false on an inexistant file", (done) -> - @s3.headObject.callsArgWith 1, null, {} - @AWSSDKPersistorManager.checkIfFileExists @bucketName, @key, (err, exists) => - expect(exists).to.be.false - done() + it "should return false on an inexistant file", (done) -> + @s3.headObject.callsArgWith 1, null, {} + @AWSSDKPersistorManager.checkIfFileExists @bucketName, @key, (err, exists) => + expect(exists).to.be.false + done() - it "should return true on an existing file", (done) -> - @s3.headObject.callsArgWith 1, null, ETag: "etag" - @AWSSDKPersistorManager.checkIfFileExists @bucketName, @key, (err, exists) => - expect(exists).to.be.true - done() + it "should return true on an existing file", (done) -> + @s3.headObject.callsArgWith 1, null, ETag: "etag" + @AWSSDKPersistorManager.checkIfFileExists @bucketName, @key, (err, exists) => + expect(exists).to.be.true + done() - it "should dispatch the error from s3.headObject", (done) -> - @s3.headObject.callsArgWith 1, @error - @AWSSDKPersistorManager.checkIfFileExists @bucketName, @key, (err, exists) => - expect(err).to.equal @error - done() + it "should dispatch the error from s3.headObject", (done) -> + @s3.headObject.callsArgWith 1, @error + @AWSSDKPersistorManager.checkIfFileExists @bucketName, @key, (err, exists) => + expect(err).to.equal @error + done() From 81a72033155df267646f657bbcbbff055be242c3 Mon Sep 17 00:00:00 2001 From: Xavier Trochu Date: Wed, 25 Nov 2015 15:40:35 +0100 Subject: [PATCH 14/14] Fix the checkIfFileExists API. --- services/filestore/app/coffee/AWSSDKPersistorManager.coffee | 1 + 1 file changed, 1 insertion(+) diff --git a/services/filestore/app/coffee/AWSSDKPersistorManager.coffee b/services/filestore/app/coffee/AWSSDKPersistorManager.coffee index 50e15cfa27..d0101b93a4 100644 --- a/services/filestore/app/coffee/AWSSDKPersistorManager.coffee +++ b/services/filestore/app/coffee/AWSSDKPersistorManager.coffee @@ -80,6 +80,7 @@ module.exports = logger.log bucketName:bucketName, key:key, "check file existence in s3" s3.headObject {Bucket: bucketName, Key: key}, (err, data) -> if err? + return (callback null, false) if err.code == 'NotFound' logger.err err:err, bucketName:bucketName, key:key, "something went wrong checking head in s3" return callback err callback null, data.ETag?