From d0600a4b64c152eaad12ec85e047e686f458ce50 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Wed, 19 Feb 2014 13:02:53 +0000 Subject: [PATCH] changed optimisation to optipng and added timeouts to conversions --- .../filestore/app/coffee/FileConverter.coffee | 14 +++++-- .../app/coffee/ImageOptimiser.coffee | 28 ++++--------- .../unit/coffee/FileConverterTests.coffee | 12 +++--- .../unit/coffee/ImageOptimiserTests.coffee | 41 +++++-------------- 4 files changed, 34 insertions(+), 61 deletions(-) diff --git a/services/filestore/app/coffee/FileConverter.coffee b/services/filestore/app/coffee/FileConverter.coffee index ce62d32743..94323c0677 100644 --- a/services/filestore/app/coffee/FileConverter.coffee +++ b/services/filestore/app/coffee/FileConverter.coffee @@ -4,6 +4,8 @@ logger = require("logger-sharelatex") exec = require('child_process').exec approvedFormats = ["png"] +twoMinsInMs = 2 * (60 * 1000) + module.exports = convert: (sourcePath, requestedFormat, callback)-> @@ -15,7 +17,9 @@ module.exports = err = new Error("invalid format requested") return callback err args = "nice convert -flatten -density 300 #{sourcePath} #{destPath}" - exec args, (err, stdout, stderr)-> + opts = + timeout: twoMinsInMs + exec args, opts, (err, stdout, stderr)-> timer.done() callback(err, destPath) @@ -29,7 +33,9 @@ module.exports = width: 424 height: 300 args = "nice convert -flatten -background white -resize 260x -density 300 #{sourcePath} #{destPath}" - exec args, (err, stdout, stderr)-> + opts = + timeout: twoMinsInMs + exec args, opts,(err, stdout, stderr)-> callback(err, destPath) preview: (sourcePath, callback)-> @@ -42,5 +48,7 @@ module.exports = width: 600 height: 849 args = "nice convert -flatten -background white -resize 548x -density 300 #{sourcePath} #{destPath}" - exec args, (err, stdout, stderr)-> + opts = + timeout: twoMinsInMs + exec args, opts,(err, stdout, stderr)-> callback(err, destPath) diff --git a/services/filestore/app/coffee/ImageOptimiser.coffee b/services/filestore/app/coffee/ImageOptimiser.coffee index 8d5a9b8714..4c804cedc1 100644 --- a/services/filestore/app/coffee/ImageOptimiser.coffee +++ b/services/filestore/app/coffee/ImageOptimiser.coffee @@ -1,29 +1,15 @@ -PngCrush = require('pngcrush') -fs = require("fs") +exec = require('child_process').exec logger = require("logger-sharelatex") module.exports = compressPng: (localPath, callback)-> - optimisedPath = "#{localPath}-optimised" startTime = new Date() - logger.log localPath:localPath, optimisedPath:optimisedPath, "optimising png path" - readStream = fs.createReadStream(localPath) - writeStream = fs.createWriteStream(optimisedPath) - readStream.on "error", (err)-> - logger.err err:err, localPath:localPath, "something went wrong getting read stream for compressPng" - callback(err) - writeStream.on "error", (err)-> - logger.err err:err, localPath:localPath, "something went wrong getting write stream for compressPng" - callback(err) - myCrusher = new PngCrush() - myCrusher.on "error", (err)-> - logger.err err:err, localPath:localPath, "error compressing file" - callback err - readStream.pipe(myCrusher).pipe(writeStream) - writeStream.on "finish", -> - timeTaken = new Date() - startTime - logger.log localPath:localPath, timeTaken:timeTaken, "finished converting file" - fs.rename optimisedPath, localPath, callback + logger.log localPath:localPath, "optimising png path" + args = "optipng #{localPath}" + opts = + timeout: 60 * 1000 + exec args, opts, callback + diff --git a/services/filestore/test/unit/coffee/FileConverterTests.coffee b/services/filestore/test/unit/coffee/FileConverterTests.coffee index 41f9d5090d..b56cb55eb2 100644 --- a/services/filestore/test/unit/coffee/FileConverterTests.coffee +++ b/services/filestore/test/unit/coffee/FileConverterTests.coffee @@ -29,7 +29,7 @@ describe "FileConverter", -> describe "convert", -> it "should convert the source to the requested format", (done)-> - @child_process.exec.callsArgWith(1) + @child_process.exec.callsArgWith(2) @converter.convert @sourcePath, @format, (err)=> args = @child_process.exec.args[0][0] args.indexOf(@sourcePath).should.not.equal -1 @@ -37,26 +37,26 @@ describe "FileConverter", -> done() it "should return the dest path", (done)-> - @child_process.exec.callsArgWith(1) + @child_process.exec.callsArgWith(2) @converter.convert @sourcePath, @format, (err, destPath)=> destPath.should.equal "#{@sourcePath}.#{@format}" done() it "should return the error from convert", (done)-> - @child_process.exec.callsArgWith(1, @error) + @child_process.exec.callsArgWith(2, @error) @converter.convert @sourcePath, @format, (err)=> err.should.equal @error done() it "should not accapt an non aproved format", (done)-> - @child_process.exec.callsArgWith(1) + @child_process.exec.callsArgWith(2) @converter.convert @sourcePath, "ahhhhh", (err)=> expect(err).to.exist done() describe "thumbnail", -> it "should call easy image resize with args", (done)-> - @child_process.exec.callsArgWith(1) + @child_process.exec.callsArgWith(2) @converter.thumbnail @sourcePath, (err)=> args = @child_process.exec.args[0][0] args.indexOf(@sourcePath).should.not.equal -1 @@ -64,7 +64,7 @@ describe "FileConverter", -> describe "preview", -> it "should call easy image resize with args", (done)-> - @child_process.exec.callsArgWith(1) + @child_process.exec.callsArgWith(2) @converter.preview @sourcePath, (err)=> args = @child_process.exec.args[0][0] args.indexOf(@sourcePath).should.not.equal -1 diff --git a/services/filestore/test/unit/coffee/ImageOptimiserTests.coffee b/services/filestore/test/unit/coffee/ImageOptimiserTests.coffee index 4742d42840..80ca0c1d66 100644 --- a/services/filestore/test/unit/coffee/ImageOptimiserTests.coffee +++ b/services/filestore/test/unit/coffee/ImageOptimiserTests.coffee @@ -9,52 +9,31 @@ SandboxedModule = require('sandboxed-module') describe "ImageOptimiser", -> beforeEach -> - - @fs = - createReadStream:sinon.stub() - createWriteStream:sinon.stub() - rename:sinon.stub() - @pngcrush = class PngCrush - pipe:-> - on: -> + @child_process = + exec : sinon.stub() @optimiser = SandboxedModule.require modulePath, requires: - "fs":@fs - "pngcrush":@pngcrush + 'child_process': @child_process "logger-sharelatex": log:-> err:-> @sourcePath = "/this/path/here.eps" - @writeStream = - pipe:-> - on: (type, cb)-> - if type == "finish" - cb() - @sourceStream = - pipe:-> - return pipe:-> - on:-> @error = "Error" describe "compressPng", -> - beforeEach -> - @fs.createReadStream.returns(@sourceStream) - @fs.createWriteStream.returns(@writeStream) - @fs.rename.callsArgWith(2) - it "should get the file stream", (done)-> + it "convert the file", (done)-> + @child_process.exec.callsArgWith(2) @optimiser.compressPng @sourcePath, (err)=> - @fs.createReadStream.calledWith(@sourcePath).should.equal true + args = @child_process.exec.args[0][0] + args.should.equal "optipng #{@sourcePath}" done() - it "should create a compressed file stream", (done)-> - @optimiser.compressPng @sourcePath, (err)=> - @fs.createWriteStream.calledWith("#{@sourcePath}-optimised") - done() - it "should rename the file after completion", (done)-> + it "should return the errro the file", (done)-> + @child_process.exec.callsArgWith(2, @error) @optimiser.compressPng @sourcePath, (err)=> - @fs.rename.calledWith("#{@sourcePath}-optimised", @sourcePath).should.equal true + err.should.equal @error done() \ No newline at end of file