diff --git a/services/filestore/.gitignore b/services/filestore/.gitignore index 365bc9160d..d51931c976 100644 --- a/services/filestore/.gitignore +++ b/services/filestore/.gitignore @@ -55,6 +55,7 @@ public/stylesheets/mainStyle.css public/minjs/ test/unit/js/ test/acceptence/js +cluster.js user_files/* template_files/* diff --git a/services/filestore/.travis.yml b/services/filestore/.travis.yml index 6adc08643a..c9b4f1cf1a 100644 --- a/services/filestore/.travis.yml +++ b/services/filestore/.travis.yml @@ -10,13 +10,6 @@ install: - npm install - grunt install -before_script: - - grunt forever:app:start - script: - grunt test:unit - - grunt test:acceptance -services: - - redis-server - - mongodb diff --git a/services/filestore/app.coffee b/services/filestore/app.coffee index 9c744e75ab..5df7c76fc8 100644 --- a/services/filestore/app.coffee +++ b/services/filestore/app.coffee @@ -14,6 +14,7 @@ Metrics = require "metrics-sharelatex" Metrics.initialize("filestore") Metrics.open_sockets.monitor(logger) Metrics.event_loop?.monitor(logger) +Metrics.memory.monitor(logger) app.configure -> app.use express.bodyParser() @@ -85,7 +86,6 @@ app.post "/project/:project_id/public/:public_file_id", keyBuilder.publicFileKey app.put "/project/:project_id/public/:public_file_id", keyBuilder.publicFileKey, fileController.copyFile app.del "/project/:project_id/public/:public_file_id", keyBuilder.publicFileKey, fileController.deleteFile - app.get "/heapdump", (req, res)-> require('heapdump').writeSnapshot '/tmp/' + Date.now() + '.filestore.heapsnapshot', (err, filename)-> res.send filename @@ -140,3 +140,10 @@ server.listen port, host, -> process.on 'SIGTERM', () -> logger.log("filestore got SIGTERM, shutting down gracefully") beginShutdown() + +if global.gc? + gcTimer = setInterval () -> + global.gc() + logger.log process.memoryUsage(), "global.gc" + , 3 * oneMinute = 60 * 1000 + gcTimer.unref() diff --git a/services/filestore/app/coffee/Errors.coffee b/services/filestore/app/coffee/Errors.coffee new file mode 100644 index 0000000000..3bd9479abe --- /dev/null +++ b/services/filestore/app/coffee/Errors.coffee @@ -0,0 +1,9 @@ +NotFoundError = (message) -> + error = new Error(message) + error.name = "NotFoundError" + error.__proto__ = NotFoundError.prototype + return error +NotFoundError.prototype.__proto__ = Error.prototype + +module.exports = Errors = + NotFoundError: NotFoundError diff --git a/services/filestore/app/coffee/FSPersistorManager.coffee b/services/filestore/app/coffee/FSPersistorManager.coffee index c2f4564987..2ade1f3a5b 100644 --- a/services/filestore/app/coffee/FSPersistorManager.coffee +++ b/services/filestore/app/coffee/FSPersistorManager.coffee @@ -1,12 +1,12 @@ logger = require("logger-sharelatex") fs = require("fs") LocalFileWriter = require("./LocalFileWriter") +Errors = require('./Errors') rimraf = require("rimraf") -response = require ("response") filterName = (key) -> return key.replace /\//g, "_" - + module.exports = sendFile: ( location, target, source, callback = (err)->) -> @@ -27,19 +27,20 @@ module.exports = return callback err @sendFile location, target, fsPath, callback - getFileStream: (location, name, _callback = (err, res)->) -> + # opts may be {start: Number, end: Number} + getFileStream: (location, name, opts, _callback = (err, res)->) -> callback = (args...) -> _callback(args...) _callback = () -> filteredName = filterName name logger.log location:location, name:filteredName, "getting file" - sourceStream = fs.createReadStream "#{location}/#{filteredName}" + sourceStream = fs.createReadStream "#{location}/#{filteredName}", opts sourceStream.on 'error', (err) -> logger.err err:err, location:location, name:name, "Error reading from file" - if err.code = 'ENOENT' - callback null, response().html('NoSuchKey: file not found\n') + if err.code == 'ENOENT' + callback new Errors.NotFoundError(err.message), null else - callback err + callback err, null sourceStream.on 'readable', () -> # This can be called multiple times, but the callback wrapper # ensures the callback is only called once diff --git a/services/filestore/app/coffee/FileController.coffee b/services/filestore/app/coffee/FileController.coffee index 921faa6c6e..2b10419fb2 100644 --- a/services/filestore/app/coffee/FileController.coffee +++ b/services/filestore/app/coffee/FileController.coffee @@ -3,20 +3,37 @@ settings = require("settings-sharelatex") logger = require("logger-sharelatex") FileHandler = require("./FileHandler") metrics = require("metrics-sharelatex") -oneDayInSeconds = 60 * 60 * 24 +parseRange = require('range-parser') +Errors = require('./Errors') -module.exports = +oneDayInSeconds = 60 * 60 * 24 +maxSizeInBytes = 1024 * 1024 * 1024 # 1GB + +module.exports = FileController = getFile: (req, res)-> - metrics.inc "getFile" {key, bucket} = req {format, style} = req.query - 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)-> + options = { + key: key, + bucket: bucket, + format: format, + style: style, + } + metrics.inc "getFile" + logger.log key:key, bucket:bucket, format:format, style: style, "reciving request to get file" + if req.headers.range? + range = FileController._get_range(req.headers.range) + options.start = range.start + options.end = range.end + logger.log start: range.start, end: range.end, "getting range of bytes from file" + FileHandler.getFile bucket, key, options, (err, fileStream)-> if err? logger.err err:err, key:key, bucket:bucket, format:format, style:style, "problem getting file" - if !res.finished and res?.send? - res.send 500 + if err instanceof Errors.NotFoundError + return res.send 404 + if !res.finished and res?.send? + return res.send 500 else if req.query.cacheWarm logger.log key:key, bucket:bucket, format:format, style:style, "request is only for cache warm so not sending stream" res.send 200 @@ -29,6 +46,9 @@ module.exports = {key, bucket} = req logger.log key:key, bucket:bucket, "reciving request to insert file" FileHandler.insertFile bucket, key, req, (err)-> + if err? + logger.log err: err, key: key, bucket: bucket, "error inserting file" + res.send 500 res.send 200 copyFile: (req, res)-> @@ -38,7 +58,7 @@ module.exports = oldFile_id = req.body.source.file_id logger.log key:key, bucket:bucket, oldProject_id:oldProject_id, oldFile_id:oldFile_id, "reciving request to copy file" PersistorManager.copyFile bucket, "#{oldProject_id}/#{oldFile_id}", key, (err)-> - if err? + if err? logger.log err:err, oldProject_id:oldProject_id, oldFile_id:oldFile_id, "something went wrong copying file" res.send 500 else @@ -55,5 +75,10 @@ module.exports = else res.send 204 - - + _get_range: (header) -> + parsed = parseRange(maxSizeInBytes, header) + if parsed == -1 or parsed == -2 or parsed.type != 'bytes' + null + else + range = parsed[0] + {start: range.start, end: range.end} diff --git a/services/filestore/app/coffee/FileHandler.coffee b/services/filestore/app/coffee/FileHandler.coffee index ece26c6164..b11036465a 100644 --- a/services/filestore/app/coffee/FileHandler.coffee +++ b/services/filestore/app/coffee/FileHandler.coffee @@ -30,7 +30,7 @@ module.exports = @_getConvertedFile bucket, key, opts, callback _getStandardFile: (bucket, key, opts, callback)-> - PersistorManager.getFileStream bucket, key, (err, fileStream)-> + PersistorManager.getFileStream bucket, key, opts, (err, fileStream)-> if err? logger.err bucket:bucket, key:key, opts:opts, "error getting fileStream" callback err, fileStream @@ -38,6 +38,8 @@ module.exports = _getConvertedFile: (bucket, key, opts, callback)-> convertedKey = KeyBuilder.addCachingToKey key, opts PersistorManager.checkIfFileExists bucket, convertedKey, (err, exists)=> + if err? + return callback err if exists PersistorManager.getFileStream bucket, convertedKey, callback else @@ -58,10 +60,19 @@ 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" @@ -76,10 +87,11 @@ module.exports = else if opts.style == "preview" FileConverter.preview originalFsPath, done else - throw new Error("should have specified opts to convert file with #{JSON.stringify(opts)}") + return callback(new Error("should have specified opts to convert file with #{JSON.stringify(opts)}")) - _writeS3FileToDisk: (bucket, key, callback)-> - PersistorManager.getFileStream bucket, key, (err, fileStream)-> + _writeS3FileToDisk: (bucket, key, opts, callback)-> + PersistorManager.getFileStream bucket, key, opts, (err, fileStream)-> + if err? + return callback(err) LocalFileWriter.writeStream fileStream, key, callback - diff --git a/services/filestore/app/coffee/S3PersistorManager.coffee b/services/filestore/app/coffee/S3PersistorManager.coffee index d5cb06074e..15798185a3 100644 --- a/services/filestore/app/coffee/S3PersistorManager.coffee +++ b/services/filestore/app/coffee/S3PersistorManager.coffee @@ -9,6 +9,7 @@ fs = require("fs") knox = require("knox") path = require("path") LocalFileWriter = require("./LocalFileWriter") +Errors = require("./Errors") _ = require("underscore") thirtySeconds = 30 * 1000 @@ -57,17 +58,28 @@ module.exports = logger.err bucketName:bucketName, key:key, fsPath:fsPath, err:err, "something went wrong writing stream to disk" return callback(err) @sendFile bucketName, key, fsPath, callback - - getFileStream: (bucketName, key, callback = (err, res)->)-> + + # opts may be {start: Number, end: Number} + getFileStream: (bucketName, key, opts, callback = (err, res)->)-> + opts = opts || {} + headers = {} + if opts.start? and opts.end? + headers['Range'] = "bytes=#{opts.start}-#{opts.end}" 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 bucket: bucketName - s3Stream = s3Client.get(key) + s3Stream = s3Client.get(key, headers) s3Stream.end() s3Stream.on 'response', (res) -> + if res.statusCode == 404 + logger.log bucketName:bucketName, key:key, "file not found in s3" + return callback new Errors.NotFoundError("File not found in S3: #{bucketName}:#{key}"), null + if res.statusCode not in [200, 206] + logger.log bucketName:bucketName, key:key, "error getting file from s3: #{res.statusCode}" + return callback new Error("Got non-200 response from S3: #{res.statusCode}"), null callback null, res s3Stream.on 'error', (err) -> logger.err err:err, bucketName:bucketName, key:key, "error getting file stream from s3" @@ -125,4 +137,3 @@ module.exports = exists = res.statusCode == 200 logger.log bucketName:bucketName, key:key, exists:exists, "checked if file exsists in s3" callback(err, exists) - diff --git a/services/filestore/package.json b/services/filestore/package.json index 441d4ad7e3..8e2dfd084b 100644 --- a/services/filestore/package.json +++ b/services/filestore/package.json @@ -17,18 +17,19 @@ "grunt-mocha-test": "~0.8.2", "heapdump": "^0.3.2", "knox": "~0.9.1", - "logger-sharelatex": "git+https://github.com/sharelatex/logger-sharelatex.git#master", + "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#master", + "metrics-sharelatex": "git+https://github.com/sharelatex/metrics-sharelatex.git#v1.3.0", "node-transloadit": "0.0.4", "node-uuid": "~1.4.1", "pngcrush": "0.0.3", + "range-parser": "^1.0.2", "recluster": "^0.3.7", "request": "2.14.0", "response": "0.14.0", "rimraf": "2.2.8", - "settings-sharelatex": "git+https://github.com/sharelatex/settings-sharelatex.git#master", + "settings-sharelatex": "git+https://github.com/sharelatex/settings-sharelatex.git#v1.0.0", "stream-buffers": "~0.2.5", "underscore": "~1.5.2" }, diff --git a/services/filestore/test/acceptence/coffee/SendingFileTest.coffee b/services/filestore/test/acceptence/coffee/SendingFileTest.coffee index 11668f230e..2731e25565 100644 --- a/services/filestore/test/acceptence/coffee/SendingFileTest.coffee +++ b/services/filestore/test/acceptence/coffee/SendingFileTest.coffee @@ -32,7 +32,7 @@ describe "Filestore", -> it "should send a 200 for status endpoing", (done)-> - request "#{@filestoreUrl}/status", (err, response, body)-> + request "#{@filestoreUrl}/status", (err, response, body)-> response.statusCode.should.equal 200 body.indexOf("filestore").should.not.equal -1 body.indexOf("up").should.not.equal -1 @@ -50,18 +50,46 @@ describe "Filestore", -> writeStream.on "end", done fs.createReadStream(@localFileReadPath).pipe writeStream + it "should return 404 for a non-existant id", (done) -> + @timeout(1000 * 20) + options = + uri: @fileUrl + '___this_is_clearly_wrong___' + request.get options, (err, response, body) => + response.statusCode.should.equal 404 + done() + it "should be able get the file back", (done)-> @timeout(1000 * 10) request.get @fileUrl, (err, response, body)=> body.should.equal @constantFileContent done() + it "should be able to get back the first 8 bytes of the file", (done) -> + @timeout(1000 * 10) + options = + uri: @fileUrl + headers: + 'Range': 'bytes=0-8' + request.get options, (err, response, body)=> + body.should.equal 'hello wor' + done() + + it "should be able to get back bytes 4 through 10 of the file", (done) -> + @timeout(1000 * 10) + options = + uri: @fileUrl + headers: + 'Range': 'bytes=4-10' + request.get options, (err, response, body)=> + body.should.equal 'o world' + done() + it "should be able to delete the file", (done)-> @timeout(1000 * 20) request.del @fileUrl, (err, response, body)=> response.statusCode.should.equal 204 request.get @fileUrl, (err, response, body)=> - body.indexOf("NoSuchKey").should.not.equal -1 + response.statusCode.should.equal 404 done() it "should be able to copy files", (done)-> @@ -85,5 +113,59 @@ describe "Filestore", -> body.should.equal @constantFileContent done() + describe "with a pdf file", -> + beforeEach (done)-> + @timeout(1000 * 10) + @file_id = Math.random() + @fileUrl = "#{@filestoreUrl}/project/acceptence_tests/file/#{@file_id}" + @localFileReadPath = __dirname + '/../../fixtures/test.pdf' + writeStream = request.post(@fileUrl) + + writeStream.on "end", done + fs.createReadStream(@localFileReadPath).pipe writeStream + + it "should be able get the file back", (done)-> + @timeout(1000 * 10) + request.get @fileUrl, (err, response, body)=> + expect(body.substring(0, 8)).to.equal '%PDF-1.5' + done() + + describe "getting the preview image", -> + + beforeEach -> + @fileUrl = @fileUrl + '?style=preview' + + it "should not time out", (done) -> + @timeout(1000 * 20) + request.get @fileUrl, (err, response, body) => + expect(response).to.not.equal null + done() + + it "should respond with image data", (done) -> + # note: this test relies of the imagemagick conversion working + @timeout(1000 * 20) + request.get @fileUrl, (err, response, body) => + expect(response.statusCode).to.equal 200 + expect(body.length).to.be.greaterThan 400 + done() + + describe "warming the cache", -> + + beforeEach -> + @fileUrl = @fileUrl + '?style=preview&cacheWarm=true' + + it "should not time out", (done) -> + @timeout(1000 * 20) + request.get @fileUrl, (err, response, body) => + expect(response).to.not.equal null + done() + + it "should respond with only an 'OK'", (done) -> + # note: this test relies of the imagemagick conversion working + @timeout(1000 * 20) + request.get @fileUrl, (err, response, body) => + expect(response.statusCode).to.equal 200 + body.should.equal 'OK' + done() diff --git a/services/filestore/test/fixtures/test.pdf b/services/filestore/test/fixtures/test.pdf new file mode 100644 index 0000000000..b021cc12b3 Binary files /dev/null and b/services/filestore/test/fixtures/test.pdf differ diff --git a/services/filestore/test/unit/coffee/FSPersistorManagerTests.coffee b/services/filestore/test/unit/coffee/FSPersistorManagerTests.coffee index 786f6be323..75a4376b8c 100644 --- a/services/filestore/test/unit/coffee/FSPersistorManagerTests.coffee +++ b/services/filestore/test/unit/coffee/FSPersistorManagerTests.coffee @@ -29,6 +29,8 @@ describe "FSPersistorManagerTests", -> err:-> "response":response "rimraf":@Rimraf + "./Errors": @Errors = + NotFoundError: sinon.stub() @location = "/tmp" @name1 = "530f2407e7ef165704000007/530f838b46d9a9e859000008" @name1Filtered ="530f2407e7ef165704000007_530f838b46d9a9e859000008" @@ -69,14 +71,63 @@ describe "FSPersistorManagerTests", -> done() describe "getFileStream", -> + beforeEach -> + @opts = {} + it "should use correct file location", (done) -> - @Fs.createReadStream.returns( - on:-> - ) - @FSPersistorManager.getFileStream @location, @name1, (err,res)=> + @Fs.createReadStream.returns({on: ->}) + @FSPersistorManager.getFileStream @location, @name1, @opts, (err,res) => @Fs.createReadStream.calledWith("#{@location}/#{@name1Filtered}").should.equal.true done() + describe "with start and end options", -> + + beforeEach -> + @opts = {start: 0, end: 8} + + it 'should pass the options to createReadStream', (done) -> + @Fs.createReadStream.returns({on: ->}) + @FSPersistorManager.getFileStream @location, @name1, @opts, (err,res)=> + @Fs.createReadStream.calledWith("#{@location}/#{@name1Filtered}", @opts).should.equal true + done() + + describe "error conditions", -> + + beforeEach -> + @fakeCode = 'ENOENT' + @Fs.createReadStream.returns( + on: (key, callback) => + err = new Error() + err.code = @fakeCode + callback(err, null) + ) + + describe "when the file does not exist", -> + + beforeEach -> + @fakeCode = 'ENOENT' + + it "should give a NotFoundError", (done) -> + @FSPersistorManager.getFileStream @location, @name1, @opts, (err,res)=> + expect(res).to.equal null + expect(err).to.not.equal null + expect(err instanceof @Errors.NotFoundError).to.equal true + done() + + describe "when some other error happens", -> + + beforeEach -> + @fakeCode = 'SOMETHINGHORRIBLE' + + it "should give an Error", (done) -> + @FSPersistorManager.getFileStream @location, @name1, @opts, (err,res)=> + expect(res).to.equal null + expect(err).to.not.equal null + expect(err instanceof Error).to.equal true + done() + + + describe "copyFile", -> beforeEach -> @ReadStream= @@ -157,5 +208,3 @@ describe "FSPersistorManagerTests", -> @FSPersistorManager.checkIfFileExists @location, @name1, (err,exists) => exists.should.be.false done() - - diff --git a/services/filestore/test/unit/coffee/FileControllerTests.coffee b/services/filestore/test/unit/coffee/FileControllerTests.coffee index ecf067976f..1a2c3e81ea 100644 --- a/services/filestore/test/unit/coffee/FileControllerTests.coffee +++ b/services/filestore/test/unit/coffee/FileControllerTests.coffee @@ -42,6 +42,7 @@ describe "FileController", -> params: project_id:@project_id file_id:@file_id + headers: {} @res = setHeader: -> @fileStream = {} @@ -70,6 +71,19 @@ describe "FileController", -> done() @controller.getFile @req, @res + describe "with a 'Range' header set", -> + + beforeEach -> + @req.headers.range = 'bytes=0-8' + + it "should pass 'start' and 'end' options to FileHandler", (done) -> + @FileHandler.getFile.callsArgWith(3, null, @fileStream) + @fileStream.pipe = (res)=> + expect(@FileHandler.getFile.lastCall.args[2].start).to.equal 0 + expect(@FileHandler.getFile.lastCall.args[2].end).to.equal 8 + done() + @controller.getFile @req, @res + describe "insertFile", -> it "should send bucket name key and res to PersistorManager", (done)-> @@ -101,7 +115,7 @@ describe "FileController", -> @res.send = (code)=> code.should.equal 500 done() - @controller.copyFile @req, @res + @controller.copyFile @req, @res describe "delete file", -> @@ -119,3 +133,22 @@ describe "FileController", -> code.should.equal 500 done() @controller.deleteFile @req, @res + + describe "_get_range", -> + + it "should parse a valid Range header", (done) -> + result = @controller._get_range('bytes=0-200') + expect(result).to.not.equal null + expect(result.start).to.equal 0 + expect(result.end).to.equal 200 + done() + + it "should return null for an invalid Range header", (done) -> + result = @controller._get_range('wat') + expect(result).to.equal null + done() + + it "should return null for any type other than 'bytes'", (done) -> + result = @controller._get_range('carrots=0-200') + expect(result).to.equal null + done() diff --git a/services/filestore/test/unit/coffee/FileHandlerTests.coffee b/services/filestore/test/unit/coffee/FileHandlerTests.coffee index 2a2bf6e17d..e67a8d38b7 100644 --- a/services/filestore/test/unit/coffee/FileHandlerTests.coffee +++ b/services/filestore/test/unit/coffee/FileHandlerTests.coffee @@ -93,6 +93,13 @@ describe "FileHandler", -> @handler._getConvertedFile.called.should.equal false done() + it "should pass options to _getStandardFile", (done) -> + options = {start: 0, end: 8} + @handler.getFile @bucket, @key, options, => + expect(@handler._getStandardFile.lastCall.args[2].start).to.equal 0 + expect(@handler._getStandardFile.lastCall.args[2].end).to.equal 8 + done() + it "should call _getConvertedFile if a format is defined", (done)-> @handler.getFile @bucket, @key, format:"png", => @handler._getStandardFile.called.should.equal false @@ -104,7 +111,7 @@ describe "FileHandler", -> beforeEach -> @fileStream = {on:->} - @PersistorManager.getFileStream.callsArgWith(2, "err", @fileStream) + @PersistorManager.getFileStream.callsArgWith(3, "err", @fileStream) it "should get the stream", (done)-> @handler.getFile @bucket, @key, null, => @@ -117,11 +124,18 @@ describe "FileHandler", -> stream.should.equal @fileStream done() + it "should pass options to PersistorManager", (done) -> + @handler.getFile @bucket, @key, {start: 0, end: 8}, => + expect(@PersistorManager.getFileStream.lastCall.args[2].start).to.equal 0 + expect(@PersistorManager.getFileStream.lastCall.args[2].end).to.equal 8 + done() + + describe "_getConvertedFile", -> it "should getFileStream if it does exists", (done)-> @PersistorManager.checkIfFileExists.callsArgWith(2, null, true) - @PersistorManager.getFileStream.callsArgWith(2) + @PersistorManager.getFileStream.callsArgWith(3) @handler._getConvertedFile @bucket, @key, {}, => @PersistorManager.getFileStream.calledWith(@bucket).should.equal true done() @@ -138,7 +152,7 @@ describe "FileHandler", -> it "should _convertFile ", (done)-> @stubbedStream = {"something":"here"} @PersistorManager.sendFile = sinon.stub().callsArgWith(3) - @PersistorManager.getFileStream = sinon.stub().callsArgWith(2, null, @stubbedStream) + @PersistorManager.getFileStream = sinon.stub().callsArgWith(3, null, @stubbedStream) @convetedKey = @key+"converted" @handler._convertFile = sinon.stub().callsArgWith(3, null, @stubbedPath) @ImageOptimiser.compressPng = sinon.stub().callsArgWith(1) @@ -155,7 +169,7 @@ describe "FileHandler", -> @FileConverter.convert.callsArgWith(2, null, @formattedStubbedPath) @FileConverter.thumbnail.callsArgWith(1, null, @formattedStubbedPath) @FileConverter.preview.callsArgWith(1, null, @formattedStubbedPath) - @handler._writeS3FileToDisk = sinon.stub().callsArgWith(2, null, @stubbedPath) + @handler._writeS3FileToDisk = sinon.stub().callsArgWith(3, null, @stubbedPath) @LocalFileWriter.deleteFile.callsArgWith(1) it "should call thumbnail on the writer path if style was thumbnail was specified", (done)-> @@ -178,7 +192,3 @@ describe "FileHandler", -> @FileConverter.convert.calledWith(@stubbedPath, @format).should.equal true @LocalFileWriter.deleteFile.calledWith(@stubbedPath).should.equal true done() - - - - diff --git a/services/filestore/test/unit/coffee/S3PersistorManagerTests.coffee b/services/filestore/test/unit/coffee/S3PersistorManagerTests.coffee index fe70f1008d..78fec6cea5 100644 --- a/services/filestore/test/unit/coffee/S3PersistorManagerTests.coffee +++ b/services/filestore/test/unit/coffee/S3PersistorManagerTests.coffee @@ -35,6 +35,8 @@ describe "S3PersistorManagerTests", -> "logger-sharelatex": log:-> err:-> + "./Errors": @Errors = + NotFoundError: sinon.stub() @key = "my/key" @bucketName = "my-bucket" @error = "my errror" @@ -42,17 +44,77 @@ describe "S3PersistorManagerTests", -> describe "getFileStream", -> beforeEach -> @S3PersistorManager = SandboxedModule.require modulePath, requires: @requires - + @opts = {} it "should use correct key", (done)-> @stubbedKnoxClient.get.returns( on:-> end:-> ) - @S3PersistorManager.getFileStream @bucketName, @key, @fsPath, (err)=> + @S3PersistorManager.getFileStream @bucketName, @key, @opts, (err)=> # empty callback @stubbedKnoxClient.get.calledWith(@key).should.equal true done() + describe "with start and end options", -> + beforeEach -> + @opts = + start: 0 + end: 8 + it "should pass headers to the knox.Client.get()", (done) -> + @stubbedKnoxClient.get.returns( + on:-> + end:-> + ) + @S3PersistorManager.getFileStream @bucketName, @key, @opts, (err)=> # empty callback + @stubbedKnoxClient.get.calledWith(@key, {'Range': 'bytes=0-8'}).should.equal true + done() + + describe "error conditions", -> + + beforeEach -> + @fakeResponse = + statusCode: 500 + @stubbedKnoxClient.get.returns( + on: (key, callback) => + if key == 'response' + callback(@fakeResponse) + end: -> + ) + + describe "when the file doesn't exist", -> + + beforeEach -> + @fakeResponse = + statusCode: 404 + + it "should produce a NotFoundError", (done) -> + @S3PersistorManager.getFileStream @bucketName, @key, @opts, (err, stream)=> # empty callback + expect(stream).to.equal null + expect(err).to.not.equal null + expect(err instanceof @Errors.NotFoundError).to.equal true + done() + + it "should have bucket and key in the Error message", (done) -> + @S3PersistorManager.getFileStream @bucketName, @key, @opts, (err, stream)=> # empty callback + error_message = @Errors.NotFoundError.lastCall.args[0] + expect(error_message).to.not.equal null + error_message.should.match(new RegExp(".*#{@bucketName}.*")) + error_message.should.match(new RegExp(".*#{@key}.*")) + done() + + describe "when the S3 service produces an error", -> + beforeEach -> + @fakeResponse = + statusCode: 500 + + it "should produce an error", (done) -> + @S3PersistorManager.getFileStream @bucketName, @key, @opts, (err, stream)=> # empty callback + expect(stream).to.equal null + expect(err).to.not.equal null + expect(err instanceof Error).to.equal true + @Errors.NotFoundError.called.should.equal false + done() + describe "sendFile", -> beforeEach ->