From 711e95a82a33e9fcfbaf6cf98a678d2f1a05039c Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Tue, 4 Mar 2014 13:36:47 +0000 Subject: [PATCH] delete converted file when finished to clean up --- .../app/coffee/FileController.coffee | 1 - .../filestore/app/coffee/FileHandler.coffee | 38 +++++++++++-------- .../app/coffee/ImageOptimiser.coffee | 1 - .../app/coffee/LocalFileWriter.coffee | 1 + .../app/coffee/S3PersistorManager.coffee | 2 +- .../test/unit/coffee/FileHandlerTests.coffee | 11 +++++- 6 files changed, 34 insertions(+), 20 deletions(-) diff --git a/services/filestore/app/coffee/FileController.coffee b/services/filestore/app/coffee/FileController.coffee index 291e188da1..25c612d870 100644 --- a/services/filestore/app/coffee/FileController.coffee +++ b/services/filestore/app/coffee/FileController.coffee @@ -2,7 +2,6 @@ PersistorManager = require("./PersistorManager") settings = require("settings-sharelatex") logger = require("logger-sharelatex") FileHandler = require("./FileHandler") -LocalFileWriter = require("./LocalFileWriter") metrics = require("./metrics") oneDayInSeconds = 60 * 60 * 24 diff --git a/services/filestore/app/coffee/FileHandler.coffee b/services/filestore/app/coffee/FileHandler.coffee index ece883f772..aa9602b5a8 100644 --- a/services/filestore/app/coffee/FileHandler.coffee +++ b/services/filestore/app/coffee/FileHandler.coffee @@ -7,7 +7,6 @@ KeyBuilder = require("./KeyBuilder") async = require("async") ImageOptimiser = require("./ImageOptimiser") - module.exports = insertFile: (bucket, key, stream, callback)-> @@ -45,28 +44,37 @@ module.exports = @_getConvertedFileAndCache bucket, key, convetedKey, opts, callback _getConvertedFileAndCache: (bucket, key, convetedKey, opts, callback)-> - @_convertFile bucket, key, opts, (err, fsPath)-> + self = @ + convertedFsPath = "" + async.series [ + (cb)-> + self._convertFile bucket, key, opts, (err, fileSystemPath)-> + convertedFsPath = fileSystemPath + cb err + (cb)-> + ImageOptimiser.compressPng convertedFsPath, cb + (cb)-> + PersistorManager.sendFile bucket, convetedKey, convertedFsPath, cb + ], (err)-> if err? - logger.err err:err, fsPath:fsPath, bucket:bucket, key:key, opts:opts, "something went wrong with converting file" return callback(err) - ImageOptimiser.compressPng fsPath, (err)-> - if err? - logger.err err:err, fsPath:fsPath, bucket:bucket, key:key, opts:opts, "something went wrong optimising png file" - return callback(err) - PersistorManager.sendFile bucket, convetedKey, fsPath, (err)-> - if err? - logger.err err:err, bucket:bucket, key:key, convetedKey:convetedKey, opts:opts, "something went wrong sending the file" - return callback(err) - PersistorManager.getFileStream bucket, convetedKey, callback + PersistorManager.getFileStream bucket, convetedKey, callback _convertFile: (bucket, origonalKey, opts, callback)-> @_writeS3FileToDisk bucket, origonalKey, (err, origonalFsPath)-> + done = (err, destPath)-> + if err? + logger.err err:err, bucket:bucket, origonalKey:origonalKey, opts:opts, "error converting file" + return callback(err) + LocalFileWriter.deleteFile origonalFsPath, -> + callback(err, destPath) + if opts.format? - FileConverter.convert origonalFsPath, opts.format, callback + FileConverter.convert origonalFsPath, opts.format, done else if opts.style == "thumbnail" - FileConverter.thumbnail origonalFsPath, callback + FileConverter.thumbnail origonalFsPath, done else if opts.style == "preview" - FileConverter.preview origonalFsPath, callback + FileConverter.preview origonalFsPath, done else throw new Error("should have specified opts to convert file with #{JSON.stringify(opts)}") diff --git a/services/filestore/app/coffee/ImageOptimiser.coffee b/services/filestore/app/coffee/ImageOptimiser.coffee index dbddaa3205..be3fed1ca2 100644 --- a/services/filestore/app/coffee/ImageOptimiser.coffee +++ b/services/filestore/app/coffee/ImageOptimiser.coffee @@ -1,7 +1,6 @@ exec = require('child_process').exec logger = require("logger-sharelatex") - module.exports = compressPng: (localPath, callback)-> diff --git a/services/filestore/app/coffee/LocalFileWriter.coffee b/services/filestore/app/coffee/LocalFileWriter.coffee index 8bb8a6bc97..bf0c0fd82f 100644 --- a/services/filestore/app/coffee/LocalFileWriter.coffee +++ b/services/filestore/app/coffee/LocalFileWriter.coffee @@ -26,6 +26,7 @@ module.exports = callback err deleteFile: (fsPath, callback)-> + logger.log fsPath:fsPath, "removing local temp file" fs.unlink fsPath, callback _getPath : (key)-> diff --git a/services/filestore/app/coffee/S3PersistorManager.coffee b/services/filestore/app/coffee/S3PersistorManager.coffee index dd65b79abc..bfc8a89c6b 100644 --- a/services/filestore/app/coffee/S3PersistorManager.coffee +++ b/services/filestore/app/coffee/S3PersistorManager.coffee @@ -45,7 +45,7 @@ module.exports = return callback(err) if !res? logger.err err:err, res:res, bucketName:bucketName, key:key, fsPath:fsPath, "no response from s3 put file" - callback("no response from put file") + return callback("no response from put file") if res.statusCode != 200 logger.err bucketName:bucketName, key:key, fsPath:fsPath, "non 200 response from s3 putting file" return callback("non 200 response from s3 on put file") diff --git a/services/filestore/test/unit/coffee/FileHandlerTests.coffee b/services/filestore/test/unit/coffee/FileHandlerTests.coffee index c2bed383ab..b00f23af0e 100644 --- a/services/filestore/test/unit/coffee/FileHandlerTests.coffee +++ b/services/filestore/test/unit/coffee/FileHandlerTests.coffee @@ -22,6 +22,7 @@ describe "FileHandler", -> insertFile: sinon.stub() @LocalFileWriter = writeStream: sinon.stub() + deleteFile: sinon.stub() @FileConverter = convert: sinon.stub() thumbnail: sinon.stub() @@ -134,16 +135,18 @@ describe "FileHandler", -> describe "_getConvertedFileAndCache", -> it "should _convertFile ", (done)-> + @stubbedStream = {"something":"here"} @PersistorManager.sendFile = sinon.stub().callsArgWith(3) - @PersistorManager.getFileStream = sinon.stub().callsArgWith(2) + @PersistorManager.getFileStream = sinon.stub().callsArgWith(2, null, @stubbedStream) @convetedKey = @key+"converted" @handler._convertFile = sinon.stub().callsArgWith(3, null, @stubbedPath) @ImageOptimiser.compressPng = sinon.stub().callsArgWith(1) - @handler._getConvertedFileAndCache @bucket, @key, @convetedKey, {}, => + @handler._getConvertedFileAndCache @bucket, @key, @convetedKey, {}, (err, fsStream)=> @handler._convertFile.called.should.equal true @PersistorManager.sendFile.calledWith(@bucket, @convetedKey, @stubbedPath).should.equal true @PersistorManager.getFileStream.calledWith(@bucket, @convetedKey).should.equal true @ImageOptimiser.compressPng.calledWith(@stubbedPath).should.equal true + fsStream.should.equal @stubbedStream done() describe "_convertFile", -> @@ -152,23 +155,27 @@ describe "FileHandler", -> @FileConverter.thumbnail.callsArgWith(1, null, @formattedStubbedPath) @FileConverter.preview.callsArgWith(1, null, @formattedStubbedPath) @handler._writeS3FileToDisk = sinon.stub().callsArgWith(2, null, @stubbedPath) + @LocalFileWriter.deleteFile.callsArgWith(1) it "should call thumbnail on the writer path if style was thumbnail was specified", (done)-> @handler._convertFile @bucket, @key, style:"thumbnail", (err, path)=> path.should.equal @formattedStubbedPath @FileConverter.thumbnail.calledWith(@stubbedPath).should.equal true + @LocalFileWriter.deleteFile.calledWith(@stubbedPath).should.equal true done() it "should call preview on the writer path if style was preview was specified", (done)-> @handler._convertFile @bucket, @key, style:"preview", (err, path)=> path.should.equal @formattedStubbedPath @FileConverter.preview.calledWith(@stubbedPath).should.equal true + @LocalFileWriter.deleteFile.calledWith(@stubbedPath).should.equal true done() it "should call convert on the writer path if a format was specified", (done)-> @handler._convertFile @bucket, @key, format:@format, (err, path)=> path.should.equal @formattedStubbedPath @FileConverter.convert.calledWith(@stubbedPath, @format).should.equal true + @LocalFileWriter.deleteFile.calledWith(@stubbedPath).should.equal true done()