Merge branch 'master' of https://github.com/sharelatex/clsi-sharelatex

services/clsi/app.coffee

@@ -84,7 +84,7 @@ if Settings.smokeTest
 
 app.get "/health_check", (req, res)->
 	res.contentType(resCacher?.setContentType)
-	res.send resCacher?.code, resCacher?.body
+	res.status(resCacher?.code).send(resCacher?.body)
 
 profiler = require "v8-profiler"
 app.get "/profile", (req, res) ->
@@ -101,7 +101,7 @@ app.get "/heapdump", (req, res)->
 
 app.use (error, req, res, next) ->
 	logger.error err: error, "server error"
-	res.send error?.statusCode || 500
+	res.sendStatus(error?.statusCode || 500)
 
 app.listen port = (Settings.internal?.clsi?.port or 3013), host = (Settings.internal?.clsi?.host or "localhost"), (error) ->
 	logger.info "CLSI starting up, listening on #{host}:#{port}"

services/clsi/app/coffee/CompileController.coffee

@@ -28,7 +28,7 @@ module.exports = CompileController =
 								status = "success"
 
 					timer.done()
-					res.send (code or 200), {
+					res.status(code or 200).send {
 						compile:
 							status: status
 							error:  error?.message or error
@@ -41,7 +41,7 @@ module.exports = CompileController =
 	clearCache: (req, res, next = (error) ->) ->
 		ProjectPersistenceManager.clearProject req.params.project_id, (error) ->
 			return next(error) if error?
-			res.send 204 # No content
+			res.sendStatus(204) # No content
 
 	syncFromCode: (req, res, next = (error) ->) ->
 		file   = req.query.file

services/clsi/app/coffee/OutputFileOptimiser.coffee

@@ -9,7 +9,7 @@ module.exports = OutputFileOptimiser =
 	optimiseFile: (src, dst, callback = (error) ->) ->
 		# check output file (src) and see if we can optimise it, storing
 		# the result in the build directory (dst)
-		if src.match(/\.pdf$/)
+		if src.match(/\/output\.pdf$/)
 			OutputFileOptimiser.optimisePDF src, dst, callback
 		else
 			callback (null)

services/clsi/app/coffee/ProjectPersistenceManager.coffee

@@ -8,11 +8,11 @@ module.exports = ProjectPersistenceManager =
 	EXPIRY_TIMEOUT: oneDay = 24 * 60 * 60 * 1000 #ms
 
 	markProjectAsJustAccessed: (project_id, callback = (error) ->) ->
-		db.Project.findOrCreate(project_id: project_id)
+		db.Project.findOrCreate(where: {project_id: project_id})
-			.success(
+			.spread(
-				(project) ->
+				(project, created) ->
 					project.updateAttributes(lastAccessed: new Date())
-						.success(() -> callback())
+						.then(() -> callback())
 						.error callback
 			)
 			.error callback
@@ -41,14 +41,12 @@ module.exports = ProjectPersistenceManager =
 					callback()
 
 	_clearProjectFromDatabase: (project_id, callback = (error) ->) ->
-		db.Project.destroy(project_id: project_id)
+		db.Project.destroy(where: {project_id: project_id})
-			.success(() -> callback())
+			.then(() -> callback())
 			.error callback
 
 	_findExpiredProjectIds: (callback = (error, project_ids) ->) ->
 		db.Project.findAll(where: ["lastAccessed < ?", new Date(Date.now() - ProjectPersistenceManager.EXPIRY_TIMEOUT)])
-			.success(
+			.then((projects) ->
-				(projects) ->
+				callback null, projects.map((project) -> project.project_id)
-					callback null, projects.map((project) -> project.project_id)
+			).error callback
-			)
-			.error callback

services/clsi/app/coffee/UrlCache.coffee

@@ -75,8 +75,9 @@ module.exports = UrlCache =
 		readStream = fs.createReadStream(from)
 		writeStream.on "error", callbackOnce
 		readStream.on "error", callbackOnce
-		writeStream.on "close", () -> callbackOnce()
+		writeStream.on "close", callbackOnce
-		readStream.pipe(writeStream)
+		writeStream.on "open", () ->
+			readStream.pipe(writeStream)
 
 	_clearUrlFromCache: (project_id, url, callback = (error) ->) ->
 		UrlCache._clearUrlDetails project_id, url, (error) ->
@@ -90,27 +91,27 @@ module.exports = UrlCache =
 
 	_findUrlDetails: (project_id, url, callback = (error, urlDetails) ->) ->
 		db.UrlCache.find(where: { url: url, project_id: project_id })
-			.success((urlDetails) -> callback null, urlDetails)
+			.then((urlDetails) -> callback null, urlDetails)
 			.error callback
 
 	_updateOrCreateUrlDetails: (project_id, url, lastModified, callback = (error) ->) ->
-		db.UrlCache.findOrCreate(url: url, project_id: project_id)
+		db.UrlCache.findOrCreate(where: {url: url, project_id: project_id})
-			.success(
+			.spread(
-				(urlDetails) ->
+				(urlDetails, created) ->
 					urlDetails.updateAttributes(lastModified: lastModified)
-						.success(() -> callback())
+						.then(() -> callback())
 						.error(callback)
 			)
 			.error callback
 
 	_clearUrlDetails: (project_id, url, callback = (error) ->) ->
-		db.UrlCache.destroy(url: url, project_id: project_id)
+		db.UrlCache.destroy(where: {url: url, project_id: project_id})
-			.success(() -> callback null)
+			.then(() -> callback null)
 			.error callback
 
 	_findAllUrlsInProject: (project_id, callback = (error, urls) ->) ->
 		db.UrlCache.findAll(where: { project_id: project_id })
-			.success(
+			.then(
 				(urlEntries) ->
 					callback null, urlEntries.map((entry) -> entry.url)
 			)

services/clsi/app/coffee/UrlFetcher.coffee

@@ -2,33 +2,55 @@ request = require("request").defaults(jar: false)
 fs = require("fs")
 logger = require "logger-sharelatex"
 
+oneMinute = 60 * 1000
+
 module.exports = UrlFetcher =
 	pipeUrlToFile: (url, filePath, _callback = (error) ->) ->
 		callbackOnce = (error) ->
-			cleanUp error, (error) ->
+			clearTimeout timeoutHandler if timeoutHandler?
-				_callback(error)
+			_callback(error)
-				_callback = () ->
+			_callback = () ->
 
-		cleanUp = (error, callback) ->
+		timeoutHandler = setTimeout () ->
-			if error?
+			timeoutHandler = null
-				logger.log filePath: filePath, "deleting file from cache due to error"
+			logger.error url:url, filePath: filePath, "Timed out downloading file to cache"
-				fs.unlink filePath, (err) ->
+			callbackOnce(new Error("Timed out downloading file to cache #{url}"))
-					if err?
+			# FIXME: maybe need to close fileStream here
-						logger.err err: err, filePath: filePath, "error deleting file from cache"
+		, 3 * oneMinute
-					callback(error)
-			else
-				callback()
 
-		fileStream = fs.createWriteStream(filePath)
+		logger.log url:url, filePath: filePath, "started downloading url to cache"
-		fileStream.on 'error', (error) ->
+		urlStream = request.get({url: url, timeout: oneMinute})
-			logger.error err: error, url:url, filePath: filePath, "error writing file into cache"
+		urlStream.pause() # stop data flowing until we are ready
-			callbackOnce(error)
+
+		# attach handlers before setting up pipes
+		urlStream.on "error", (error) ->
+			logger.error err: error, url:url, filePath: filePath, "error downloading url"
+			callbackOnce(error or new Error("Something went wrong downloading the URL #{url}"))
+
+		urlStream.on "end", () ->
+			logger.log url:url, filePath: filePath, "finished downloading file into cache"
 
-		logger.log url:url, filePath: filePath, "downloading url to cache"
-		urlStream = request.get(url)
 		urlStream.on "response", (res) ->
 			if res.statusCode >= 200 and res.statusCode < 300
+				fileStream = fs.createWriteStream(filePath)
+
+				# attach handlers before setting up pipes
+				fileStream.on 'error', (error) ->
+					logger.error err: error, url:url, filePath: filePath, "error writing file into cache"
+					fs.unlink filePath, (err) ->
+						if err?
+							logger.err err: err, filePath: filePath, "error deleting file from cache"
+						callbackOnce(error)
+
+				fileStream.on 'finish', () ->
+					logger.log url:url, filePath: filePath, "finished writing file into cache"
+					callbackOnce()
+
+				fileStream.on 'pipe', () ->
+					logger.log url:url, filePath: filePath, "piping into filestream"
+
 				urlStream.pipe(fileStream)
+				urlStream.resume() # now we are ready to handle the data
 			else
 				logger.error statusCode: res.statusCode, url:url, filePath: filePath, "unexpected status code downloading url to cache"
 				# https://nodejs.org/api/http.html#http_class_http_clientrequest
@@ -39,15 +61,5 @@ module.exports = UrlFetcher =
 				# method. Until the data is consumed, the 'end' event will not
 				# fire. Also, until the data is read it will consume memory
 				# that can eventually lead to a 'process out of memory' error.
-				urlStream.on 'data', () -> # discard the data
+				urlStream.resume() # discard the data
 				callbackOnce(new Error("URL returned non-success status code: #{res.statusCode} #{url}"))
-
-		urlStream.on "error", (error) ->
-			logger.error err: error, url:url, filePath: filePath, "error downloading url"
-			callbackOnce(error or new Error("Something went wrong downloading the URL #{url}"))
-
-		urlStream.on "end", () ->
-			# FIXME: what if we get an error writing the file?  Maybe we
-			# should be using the fileStream end event as the point of
-			# callback.
-			callbackOnce()

services/clsi/app/coffee/db.coffee

@@ -16,11 +16,20 @@ module.exports =
 		url: Sequelize.STRING
 		project_id: Sequelize.STRING
 		lastModified: Sequelize.DATE
+	}, {
+		indexes: [
+			{fields: ['url', 'project_id']},
+			{fields: ['project_id']}
+		]
 	})
 
 	Project: sequelize.define("Project", {
-		project_id: Sequelize.STRING
+		project_id: {type: Sequelize.STRING, primaryKey: true}
 		lastAccessed: Sequelize.DATE
+	}, {
+		indexes: [
+			{fields: ['lastAccessed']}
+		]
 	})
 
 	sync: () -> sequelize.sync()

services/clsi/package.json

@@ -11,12 +11,12 @@
     "async": "0.2.9",
     "lynx": "0.0.11",
     "mkdirp": "0.3.5",
-    "mysql": "2.0.0-alpha7",
+    "mysql": "2.6.2",
     "request": "~2.21.0",
     "logger-sharelatex": "git+https://github.com/sharelatex/logger-sharelatex.git#v1.0.0",
     "settings-sharelatex": "git+https://github.com/sharelatex/settings-sharelatex.git#v1.0.0",
     "metrics-sharelatex": "git+https://github.com/sharelatex/metrics-sharelatex.git",
-    "sequelize": "2.0.0-beta.2",
+    "sequelize": "^2.1.3",
     "wrench": "~1.5.4",
     "smoke-test-sharelatex": "git+https://github.com/sharelatex/smoke-test-sharelatex.git",
     "sqlite3": "~2.2.0",

services/clsi/test/unit/coffee/CompileControllerTests.coffee

@@ -44,6 +44,7 @@ describe "CompileController", ->
 			}]
 			@RequestParser.parse = sinon.stub().callsArgWith(1, null, @request)
 			@ProjectPersistenceManager.markProjectAsJustAccessed = sinon.stub().callsArg(1)
+			@res.status = sinon.stub().returnsThis()
 			@res.send = sinon.stub()
 
 		describe "successfully", ->
@@ -67,8 +68,9 @@ describe "CompileController", ->
 					.should.equal true
 
 			it "should return the JSON response", ->
+				@res.status.calledWith(200).should.equal true
 				@res.send
-					.calledWith(200,
+					.calledWith(
 						compile:
 							status: "success"
 							error: null
@@ -85,8 +87,9 @@ describe "CompileController", ->
 				@CompileController.compile @req, @res
 		
 			it "should return the JSON response with the error", ->
+				@res.status.calledWith(500).should.equal true
 				@res.send
-					.calledWith(500,
+					.calledWith(
 						compile:
 							status: "error"
 							error:  @message
@@ -102,8 +105,9 @@ describe "CompileController", ->
 				@CompileController.compile @req, @res
 		
 			it "should return the JSON response with the timeout status", ->
+				@res.status.calledWith(200).should.equal true
 				@res.send
-					.calledWith(200,
+					.calledWith(
 						compile:
 							status: "timedout"
 							error: @message
@@ -117,8 +121,9 @@ describe "CompileController", ->
 				@CompileController.compile @req, @res
 		
 			it "should return the JSON response with the failure status", ->
+				@res.status.calledWith(200).should.equal true
 				@res.send
-					.calledWith(200,
+					.calledWith(
 						compile:
 							error: null
 							status: "failure"

services/clsi/test/unit/coffee/UrlFetcherTests.coffee

@@ -22,25 +22,29 @@ describe "UrlFetcher", ->
 			@path = "/path/to/file/on/disk"
 			@request.get = sinon.stub().returns(@urlStream = new EventEmitter)
 			@urlStream.pipe = sinon.stub()
-			@fs.createWriteStream = sinon.stub().returns(@fileStream = { on: () -> })
+			@urlStream.pause = sinon.stub()
+			@urlStream.resume = sinon.stub()
+			@fs.createWriteStream = sinon.stub().returns(@fileStream = new EventEmitter)
 			@fs.unlink = (file, callback) -> callback()
 			@UrlFetcher.pipeUrlToFile(@url, @path, @callback)
 
 		it "should request the URL", ->
 			@request.get
-				.calledWith(@url)
+				.calledWith(sinon.match {"url": @url})
 				.should.equal true
 
-		it "should open the file for writing", ->
-			@fs.createWriteStream
-				.calledWith(@path)
-				.should.equal true
 
 		describe "successfully", ->
 			beforeEach ->
 				@res = statusCode: 200
 				@urlStream.emit "response", @res
 				@urlStream.emit "end"
+				@fileStream.emit "finish"
+
+			it "should open the file for writing", ->
+				@fs.createWriteStream
+					.calledWith(@path)
+					.should.equal true
 
 			it "should pipe the URL to the file", ->
 				@urlStream.pipe