changed optimisation to optipng and added timeouts to conversions

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)

services/filestore/app/coffee/ImageOptimiser.coffee

@@ -1,29 +1,15 @@
-PngCrush = require('pngcrush')
+exec = require('child_process').exec
-fs = require("fs")
 logger = require("logger-sharelatex")
 
 
 module.exports = 
 
 	compressPng: (localPath, callback)->
-		optimisedPath = "#{localPath}-optimised"
 		startTime = new Date()
-		logger.log localPath:localPath, optimisedPath:optimisedPath, "optimising png path"
+		logger.log localPath:localPath, "optimising png path"
-		readStream = fs.createReadStream(localPath)
+		args = "optipng #{localPath}"
-		writeStream = fs.createWriteStream(optimisedPath)
+		opts =
-		readStream.on "error", (err)->
+			timeout: 60 * 1000
-			logger.err err:err, localPath:localPath, "something went wrong getting read stream for compressPng"
+		exec args, opts, callback
-			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
 

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 

services/filestore/test/unit/coffee/ImageOptimiserTests.coffee

@@ -9,52 +9,31 @@ SandboxedModule = require('sandboxed-module')
 describe "ImageOptimiser", ->
 
 	beforeEach ->
-
+		@child_process =
-		@fs = 
+			exec : sinon.stub()
-			createReadStream:sinon.stub()
-			createWriteStream:sinon.stub()
-			rename:sinon.stub()
-		@pngcrush = class PngCrush
-			pipe:->
-			on: ->
 
 		@optimiser = SandboxedModule.require modulePath, requires:
-			"fs":@fs
+			'child_process': @child_process
-			"pngcrush":@pngcrush
 			"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()