diff --git a/services/clsi/app.coffee b/services/clsi/app.coffee index 42c5c062dd..9083bb4bf3 100644 --- a/services/clsi/app.coffee +++ b/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}" diff --git a/services/clsi/app/coffee/CompileController.coffee b/services/clsi/app/coffee/CompileController.coffee index 29373c36db..71368be737 100644 --- a/services/clsi/app/coffee/CompileController.coffee +++ b/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 diff --git a/services/clsi/app/coffee/OutputFileOptimiser.coffee b/services/clsi/app/coffee/OutputFileOptimiser.coffee index d0c091d37b..f337a7a953 100644 --- a/services/clsi/app/coffee/OutputFileOptimiser.coffee +++ b/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) diff --git a/services/clsi/app/coffee/ProjectPersistenceManager.coffee b/services/clsi/app/coffee/ProjectPersistenceManager.coffee index 8b37947317..2e23d46cec 100644 --- a/services/clsi/app/coffee/ProjectPersistenceManager.coffee +++ b/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) - .success( - (project) -> + db.Project.findOrCreate(where: {project_id: project_id}) + .spread( + (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) - .success(() -> callback()) + db.Project.destroy(where: {project_id: project_id}) + .then(() -> callback()) .error callback _findExpiredProjectIds: (callback = (error, project_ids) ->) -> db.Project.findAll(where: ["lastAccessed < ?", new Date(Date.now() - ProjectPersistenceManager.EXPIRY_TIMEOUT)]) - .success( - (projects) -> - callback null, projects.map((project) -> project.project_id) - ) - .error callback + .then((projects) -> + callback null, projects.map((project) -> project.project_id) + ).error callback diff --git a/services/clsi/app/coffee/UrlCache.coffee b/services/clsi/app/coffee/UrlCache.coffee index 941bc9defd..535a70570c 100644 --- a/services/clsi/app/coffee/UrlCache.coffee +++ b/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() - readStream.pipe(writeStream) + writeStream.on "close", callbackOnce + 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) - .success( - (urlDetails) -> + db.UrlCache.findOrCreate(where: {url: url, project_id: project_id}) + .spread( + (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) - .success(() -> callback null) + db.UrlCache.destroy(where: {url: url, project_id: project_id}) + .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) ) diff --git a/services/clsi/app/coffee/UrlFetcher.coffee b/services/clsi/app/coffee/UrlFetcher.coffee index e14285d56e..201306c01d 100644 --- a/services/clsi/app/coffee/UrlFetcher.coffee +++ b/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) -> - _callback(error) - _callback = () -> + clearTimeout timeoutHandler if timeoutHandler? + _callback(error) + _callback = () -> - cleanUp = (error, callback) -> - if error? - logger.log filePath: filePath, "deleting file from cache due to error" - fs.unlink filePath, (err) -> - if err? - logger.err err: err, filePath: filePath, "error deleting file from cache" - callback(error) - else - callback() + timeoutHandler = setTimeout () -> + timeoutHandler = null + logger.error url:url, filePath: filePath, "Timed out downloading file to cache" + callbackOnce(new Error("Timed out downloading file to cache #{url}")) + # FIXME: maybe need to close fileStream here + , 3 * oneMinute - fileStream = fs.createWriteStream(filePath) - fileStream.on 'error', (error) -> - logger.error err: error, url:url, filePath: filePath, "error writing file into cache" - callbackOnce(error) + logger.log url:url, filePath: filePath, "started downloading url to cache" + urlStream = request.get({url: url, timeout: oneMinute}) + urlStream.pause() # stop data flowing until we are ready + + # 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() diff --git a/services/clsi/app/coffee/db.coffee b/services/clsi/app/coffee/db.coffee index 203915efa3..a72f61e8d0 100644 --- a/services/clsi/app/coffee/db.coffee +++ b/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() diff --git a/services/clsi/package.json b/services/clsi/package.json index 3d02800b2c..fc75cede50 100644 --- a/services/clsi/package.json +++ b/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", diff --git a/services/clsi/test/unit/coffee/CompileControllerTests.coffee b/services/clsi/test/unit/coffee/CompileControllerTests.coffee index bc2d988132..3298f472e6 100644 --- a/services/clsi/test/unit/coffee/CompileControllerTests.coffee +++ b/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" diff --git a/services/clsi/test/unit/coffee/UrlFetcherTests.coffee b/services/clsi/test/unit/coffee/UrlFetcherTests.coffee index 7d38aa667b..dd709ddeaf 100644 --- a/services/clsi/test/unit/coffee/UrlFetcherTests.coffee +++ b/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