From daf4f1ffd49a1b542e424ce68b47d53b3751d4e3 Mon Sep 17 00:00:00 2001 From: James Allen Date: Fri, 23 May 2014 13:54:20 +0100 Subject: [PATCH] Send content-length header when getting file --- .../app/coffee/FSPersistorManager.coffee | 13 +++++++------ .../app/coffee/FileController.coffee | 3 ++- .../filestore/app/coffee/FileHandler.coffee | 10 +++++----- .../app/coffee/S3PersistorManager.coffee | 4 ++-- .../coffee/FSPersistorManagerTests.coffee | 9 +++++---- .../unit/coffee/FileControllerTests.coffee | 5 +++-- .../coffee/S3PersistorManagerTests.coffee | 19 ++++++++++++++----- 7 files changed, 38 insertions(+), 25 deletions(-) diff --git a/services/filestore/app/coffee/FSPersistorManager.coffee b/services/filestore/app/coffee/FSPersistorManager.coffee index 032d4edb29..c86cd35727 100644 --- a/services/filestore/app/coffee/FSPersistorManager.coffee +++ b/services/filestore/app/coffee/FSPersistorManager.coffee @@ -28,12 +28,13 @@ module.exports = getFileStream: (location, name, callback = (err, res)->)-> filteredName = filterName name logger.log location:location, name:filteredName, "getting file" - sourceStream = fs.createReadStream "#{location}/#{filteredName}" - sourceStream.on 'error', (err) -> - logger.err err:err, location:location, name:name, "Error reading from file" - callback err - callback null,sourceStream - + path = "#{location}/#{filteredName}" + fs.stat path, (error, stat) -> + sourceStream = fs.createReadStream path + sourceStream.on 'error', (err) -> + logger.err err:err, location:location, name:name, "Error reading from file" + callback err + callback null, sourceStream, stat.size copyFile: (location, fromName, toName, callback = (err)->)-> filteredFromName=filterName fromName diff --git a/services/filestore/app/coffee/FileController.coffee b/services/filestore/app/coffee/FileController.coffee index 3b83e203fd..2d3792642e 100644 --- a/services/filestore/app/coffee/FileController.coffee +++ b/services/filestore/app/coffee/FileController.coffee @@ -12,7 +12,7 @@ module.exports = {key, bucket} = req {format, style} = req.query logger.log key:key, bucket:bucket, format:format, style:style, "reciving request to get file" - FileHandler.getFile bucket, key, {format:format,style:style}, (err, fileStream)-> + FileHandler.getFile bucket, key, {format:format,style:style}, (err, fileStream, size)-> if err? logger.err err:err, key:key, bucket:bucket, format:format, style:style, "problem getting file" if !res.finished and res?.send? @@ -22,6 +22,7 @@ module.exports = res.send 200 else logger.log key:key, bucket:bucket, format:format, style:style, "sending file to response" + res.header("Content-Length", size) fileStream.pipe res insertFile: (req, res)-> diff --git a/services/filestore/app/coffee/FileHandler.coffee b/services/filestore/app/coffee/FileHandler.coffee index aa9602b5a8..b2535e4488 100644 --- a/services/filestore/app/coffee/FileHandler.coffee +++ b/services/filestore/app/coffee/FileHandler.coffee @@ -22,20 +22,20 @@ module.exports = (done)-> PersistorManager.deleteFile bucket, convetedKey, done ], callback - getFile: (bucket, key, opts = {}, callback)-> + getFile: (bucket, key, opts = {}, callback = (err, fileStream, size) ->)-> logger.log bucket:bucket, key:key, opts:opts, "getting file" if !opts.format? and !opts.style? @_getStandardFile bucket, key, opts, callback else @_getConvertedFile bucket, key, opts, callback - _getStandardFile: (bucket, key, opts, callback)-> - PersistorManager.getFileStream bucket, key, (err, fileStream)-> + _getStandardFile: (bucket, key, opts, callback = (err, fileStream, size) ->)-> + PersistorManager.getFileStream bucket, key, (err, fileStream, size)-> if err? logger.err bucket:bucket, key:key, opts:opts, "error getting fileStream" - callback err, fileStream + callback err, fileStream, size - _getConvertedFile: (bucket, key, opts, callback)-> + _getConvertedFile: (bucket, key, opts, callback = (err, fileStream, size) ->)-> convetedKey = KeyBuilder.addCachingToKey(key, opts) PersistorManager.checkIfFileExists bucket, convetedKey, (err, exists)=> if exists diff --git a/services/filestore/app/coffee/S3PersistorManager.coffee b/services/filestore/app/coffee/S3PersistorManager.coffee index 432294ced6..97759e879b 100644 --- a/services/filestore/app/coffee/S3PersistorManager.coffee +++ b/services/filestore/app/coffee/S3PersistorManager.coffee @@ -67,7 +67,7 @@ module.exports = return callback(err) @sendFile bucketName, key, fsPath, callback - getFileStream: (bucketName, key, callback = (err, res)->)-> + getFileStream: (bucketName, key, callback = (err, res, size)->)-> logger.log bucketName:bucketName, key:key, "getting file from s3" s3Client = knox.createClient key: settings.filestore.s3.key @@ -76,7 +76,7 @@ module.exports = s3Stream = s3Client.get(key) s3Stream.end() s3Stream.on 'response', (res) -> - callback null, res + callback null, res, res.headers["content-length"] s3Stream.on 'error', (err) -> logger.err err:err, bucketName:bucketName, key:key, "error getting file stream from s3" callback err diff --git a/services/filestore/test/unit/coffee/FSPersistorManagerTests.coffee b/services/filestore/test/unit/coffee/FSPersistorManagerTests.coffee index bf5f08ea9d..757d1cc8df 100644 --- a/services/filestore/test/unit/coffee/FSPersistorManagerTests.coffee +++ b/services/filestore/test/unit/coffee/FSPersistorManagerTests.coffee @@ -66,11 +66,12 @@ describe "FSPersistorManagerTests", -> describe "getFileStream", -> it "should use correct file location", (done) -> - @Fs.createReadStream.returns( - on:-> - ) - @FSPersistorManager.getFileStream @location, @name1, (err,res)=> + @Fs.createReadStream.returns(@stream = on:->) + @Fs.stat = sinon.stub().callsArgWith(1, null, { size: @size = 42 }) + @FSPersistorManager.getFileStream @location, @name1, (err, res, size)=> @Fs.createReadStream.calledWith("#{@location}/#{@name1Filtered}").should.equal.true + res.should.equal @stream + size.should.equal @size done() describe "copyFile", -> diff --git a/services/filestore/test/unit/coffee/FileControllerTests.coffee b/services/filestore/test/unit/coffee/FileControllerTests.coffee index ecf067976f..d30d819f2a 100644 --- a/services/filestore/test/unit/coffee/FileControllerTests.coffee +++ b/services/filestore/test/unit/coffee/FileControllerTests.coffee @@ -43,15 +43,16 @@ describe "FileController", -> project_id:@project_id file_id:@file_id @res = - setHeader: -> + header: sinon.stub() @fileStream = {} describe "getFile", -> it "should pipe the stream", (done)-> - @FileHandler.getFile.callsArgWith(3, null, @fileStream) + @FileHandler.getFile.callsArgWith(3, null, @fileStream, @size = 42) @fileStream.pipe = (res)=> res.should.equal @res + res.header.calledWith("Content-Length", @size).should.equal true done() @controller.getFile @req, @res diff --git a/services/filestore/test/unit/coffee/S3PersistorManagerTests.coffee b/services/filestore/test/unit/coffee/S3PersistorManagerTests.coffee index fe70f1008d..efe142a6f8 100644 --- a/services/filestore/test/unit/coffee/S3PersistorManagerTests.coffee +++ b/services/filestore/test/unit/coffee/S3PersistorManagerTests.coffee @@ -45,13 +45,22 @@ describe "S3PersistorManagerTests", -> it "should use correct key", (done)-> - @stubbedKnoxClient.get.returns( - on:-> + @response = + headers: + "content-length": @size = 42 + + @stubbedKnoxClient.get.returns(@stream = + on: (e, callback) => + if e == "response" + callback(@response) end:-> + ) - @S3PersistorManager.getFileStream @bucketName, @key, @fsPath, (err)=> - @stubbedKnoxClient.get.calledWith(@key).should.equal true - done() + @S3PersistorManager.getFileStream @bucketName, @key, (err, res, size) => + res.should.equal @response + size.should.equal @size + @stubbedKnoxClient.get.calledWith(@key).should.equal true + done() describe "sendFile", ->