From ffa04c7b554fb4bd2a472e975914542346ea9dad Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Wed, 13 Apr 2016 15:38:04 +0100 Subject: [PATCH 01/18] add project url onto query string for compile hashing --- services/web/app/coffee/Features/Compile/ClsiManager.coffee | 2 ++ .../web/app/coffee/Features/Compile/CompileController.coffee | 1 + 2 files changed, 3 insertions(+) diff --git a/services/web/app/coffee/Features/Compile/ClsiManager.coffee b/services/web/app/coffee/Features/Compile/ClsiManager.coffee index 174ebe830b..0b8cf1b0b3 100755 --- a/services/web/app/coffee/Features/Compile/ClsiManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiManager.coffee @@ -37,6 +37,8 @@ module.exports = ClsiManager = url: "#{compilerUrl}/project/#{project_id}/compile" json: req jar: false + query: + project_id:project_id }, (error, response, body) -> return callback(error) if error? if 200 <= response.statusCode < 300 diff --git a/services/web/app/coffee/Features/Compile/CompileController.coffee b/services/web/app/coffee/Features/Compile/CompileController.coffee index 89fbc87e2e..54b3b1b598 100755 --- a/services/web/app/coffee/Features/Compile/CompileController.coffee +++ b/services/web/app/coffee/Features/Compile/CompileController.coffee @@ -127,6 +127,7 @@ module.exports = CompileController = for h, v of req.headers newHeaders[h] = req.headers[h] if h.match /^(If-|Range)/i options.headers = newHeaders + req.query.project_id = project_id proxy = request(options) proxy.pipe(res) proxy.on "error", (error) -> From b37595acf9d98f2de2ceb258bf1cf8077708a90f Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Tue, 19 Apr 2016 16:48:51 +0100 Subject: [PATCH 02/18] persist cookie in redis for compiles. --- .../Features/Compile/ClsiManager.coffee | 29 +++-- .../Compile/ClsiRequestManager.coffee | 60 ++++++++++ .../Features/Compile/CompileController.coffee | 55 +++++---- .../app/coffee/infrastructure/Server.coffee | 2 +- services/web/package.json | 1 + .../coffee/Compile/ClsiManagerTests.coffee | 44 +++---- .../Compile/ClsiRequestManagerTests.coffee | 111 ++++++++++++++++++ .../Compile/CompileControllerTests.coffee | 16 ++- 8 files changed, 262 insertions(+), 56 deletions(-) create mode 100644 services/web/app/coffee/Features/Compile/ClsiRequestManager.coffee create mode 100644 services/web/test/UnitTests/coffee/Compile/ClsiRequestManagerTests.coffee diff --git a/services/web/app/coffee/Features/Compile/ClsiManager.coffee b/services/web/app/coffee/Features/Compile/ClsiManager.coffee index 0b8cf1b0b3..6f4efabcf9 100755 --- a/services/web/app/coffee/Features/Compile/ClsiManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiManager.coffee @@ -6,8 +6,19 @@ Project = require("../../models/Project").Project ProjectEntityHandler = require("../Project/ProjectEntityHandler") logger = require "logger-sharelatex" url = require("url") +ClsiRequestManager = require("./ClsiRequestManager") + module.exports = ClsiManager = + + _makeRequest: (project_id, opts, callback)-> + ClsiRequestManager.getCookieJar project_id, (err, jar)-> + if err? + logger.err err:err, "error getting cookie jar for clsi request" + return callback(err) + opts.jar = jar + request opts, callback + sendRequest: (project_id, options = {}, callback = (error, success) ->) -> ClsiManager._buildRequest project_id, options, (error, req) -> return callback(error) if error? @@ -23,7 +34,10 @@ module.exports = ClsiManager = deleteAuxFiles: (project_id, options, callback = (error) ->) -> compilerUrl = @_getCompilerUrl(options?.compileGroup) - request.del "#{compilerUrl}/project/#{project_id}", callback + opts = + url:"#{compilerUrl}/project/#{project_id}" + method:"DELETE" + ClsiManager._makeRequest project_id, opts, callback _getCompilerUrl: (compileGroup) -> if compileGroup == "priority" @@ -33,13 +47,11 @@ module.exports = ClsiManager = _postToClsi: (project_id, req, compileGroup, callback = (error, response) ->) -> compilerUrl = @_getCompilerUrl(compileGroup) - request.post { + opts = url: "#{compilerUrl}/project/#{project_id}/compile" json: req - jar: false - query: - project_id:project_id - }, (error, response, body) -> + method: "POST" + ClsiManager._makeRequest project_id, opts, (error, response, body) -> return callback(error) if error? if 200 <= response.statusCode < 300 callback null, body @@ -117,9 +129,10 @@ module.exports = ClsiManager = wordcount_url = "#{compilerUrl}/project/#{project_id}/wordcount?file=#{encodeURIComponent(filename)}" if req.compile.options.imageName? wordcount_url += "&image=#{encodeURIComponent(req.compile.options.imageName)}" - request.get { + opts = url: wordcount_url - }, (error, response, body) -> + method: "GET" + ClsiManager._makeRequest project_id, opts, (error, response, body) -> return callback(error) if error? if 200 <= response.statusCode < 300 callback null, body diff --git a/services/web/app/coffee/Features/Compile/ClsiRequestManager.coffee b/services/web/app/coffee/Features/Compile/ClsiRequestManager.coffee new file mode 100644 index 0000000000..9de06c1538 --- /dev/null +++ b/services/web/app/coffee/Features/Compile/ClsiRequestManager.coffee @@ -0,0 +1,60 @@ +Settings = require "settings-sharelatex" +request = require('request') +redis = require("redis-sharelatex") +rclient = redis.createClient(Settings.redis.web) +cookie = require('cookie') + +buildKey = (project_id)-> + return "clsiserver:#{project_id}" + + +ONE_WEEK_IN_SECONDS = 60 * 60 * 24 * 7 + +module.exports = ClsiRequestManager = + + _getServerId : (project_id, callback = (err, serverId)->)-> + multi = rclient.multi() + multi.get buildKey(project_id) + multi.expire buildKey(project_id), ONE_WEEK_IN_SECONDS + multi.exec (err, results)-> + if err? + return callback(err) + serverId = results[0] + if serverId? + return callback(null, serverId) + else + return ClsiRequestManager._getServerIdViaRequest project_id, callback + + + _getServerIdViaRequest :(project_id, callback = (err, serverId)->)-> + url = "#{Settings.apis.clsi.url}/project/#{project_id}/status" + request.get url, (err, res, body)-> + if err? + logger.err err:err, project_id:project_id, "error getting initial server id for project" + return callback(err) + ClsiRequestManager.setServerId project_id, res, callback + + _parseServerIdFromResponse : (response)-> + console.log response.headers + cookies = cookie.parse(response.headers["set-cookie"]?[0] or "") + return cookies?.clsiserver + + setServerId: (project_id, response, callback = ->)-> + serverId = ClsiRequestManager._parseServerIdFromResponse(response) + multi = rclient.multi() + multi.set buildKey(project_id), serverId + multi.expire buildKey(project_id), ONE_WEEK_IN_SECONDS + multi.exec callback + + + getCookieJar: (project_id, opts, callback = (err, jar)->)-> + ClsiRequestManager._getServerId project_id, (err, serverId)=> + if err? + logger.err err:err, project_id:project_id, "error getting server id" + return callback(err) + cookie = request.cookie("clsiserver=#{serverId}") + jar = request.jar() + jar.setCookie cookie, Settings.apis.clsi.url + callback(null, jar) + + diff --git a/services/web/app/coffee/Features/Compile/CompileController.coffee b/services/web/app/coffee/Features/Compile/CompileController.coffee index 54b3b1b598..0e600dc3b0 100755 --- a/services/web/app/coffee/Features/Compile/CompileController.coffee +++ b/services/web/app/coffee/Features/Compile/CompileController.coffee @@ -8,6 +8,8 @@ Settings = require "settings-sharelatex" AuthenticationController = require "../Authentication/AuthenticationController" UserGetter = require "../User/UserGetter" RateLimiter = require("../../infrastructure/RateLimiter") +ClsiRequestManager = require("./ClsiRequestManager") + module.exports = CompileController = compile: (req, res, next = (error) ->) -> @@ -107,31 +109,34 @@ module.exports = CompileController = CompileController.proxyToClsiWithLimits(project_id, url, limits, req, res, next) proxyToClsiWithLimits: (project_id, url, limits, req, res, next = (error) ->) -> - if limits.compileGroup == "priority" - compilerUrl = Settings.apis.clsi_priority.url - else - compilerUrl = Settings.apis.clsi.url - url = "#{compilerUrl}#{url}" - logger.log url: url, "proxying to CLSI" - oneMinute = 60 * 1000 - # the base request - options = { url: url, method: req.method, timeout: oneMinute } - # if we have a build parameter, pass it through to the clsi - if req.query?.pdfng && req.query?.build? # only for new pdf viewer - options.qs = {} - options.qs.build = req.query.build - # if we are byte serving pdfs, pass through If-* and Range headers - # do not send any others, there's a proxying loop if Host: is passed! - if req.query?.pdfng - newHeaders = {} - for h, v of req.headers - newHeaders[h] = req.headers[h] if h.match /^(If-|Range)/i - options.headers = newHeaders - req.query.project_id = project_id - proxy = request(options) - proxy.pipe(res) - proxy.on "error", (error) -> - logger.warn err: error, url: url, "CLSI proxy error" + ClsiRequestManager.getCookieJar project_id, (err, jar)-> + if err? + logger.err err:err, "error getting cookie jar for clsi request" + return callback(err) + if limits.compileGroup == "priority" + compilerUrl = Settings.apis.clsi_priority.url + else + compilerUrl = Settings.apis.clsi.url + url = "#{compilerUrl}#{url}" + logger.log url: url, "proxying to CLSI" + oneMinute = 60 * 1000 + # the base request + options = { url: url, method: req.method, timeout: oneMinute, jar : jar } + # if we have a build parameter, pass it through to the clsi + if req.query?.pdfng && req.query?.build? # only for new pdf viewer + options.qs = {} + options.qs.build = req.query.build + # if we are byte serving pdfs, pass through If-* and Range headers + # do not send any others, there's a proxying loop if Host: is passed! + if req.query?.pdfng + newHeaders = {} + for h, v of req.headers + newHeaders[h] = req.headers[h] if h.match /^(If-|Range)/i + options.headers = newHeaders + proxy = request(options) + proxy.pipe(res) + proxy.on "error", (error) -> + logger.warn err: error, url: url, "CLSI proxy error" wordCount: (req, res, next) -> project_id = req.params.Project_id diff --git a/services/web/app/coffee/infrastructure/Server.coffee b/services/web/app/coffee/infrastructure/Server.coffee index fea8752bb2..11a3b5237e 100644 --- a/services/web/app/coffee/infrastructure/Server.coffee +++ b/services/web/app/coffee/infrastructure/Server.coffee @@ -125,7 +125,7 @@ apiRouter.get "/profile", (req, res) -> , time app.get "/heapdump", (req, res)-> - require('heapdump').writeSnapshot '/tmp/' + Date.now() + '.clsi.heapsnapshot', (err, filename)-> + require('heapdump').writeSnapshot '/tmp/' + Date.now() + '.web.heapsnapshot', (err, filename)-> res.send filename logger.info ("creating HTTP server").yellow diff --git a/services/web/package.json b/services/web/package.json index a2940f911f..33b6298fcc 100644 --- a/services/web/package.json +++ b/services/web/package.json @@ -18,6 +18,7 @@ "body-parser": "^1.13.1", "bufferedstream": "1.6.0", "connect-redis": "2.3.0", + "cookie": "^0.2.3", "cookie-parser": "1.3.5", "csurf": "^1.8.3", "dateformat": "1.0.4-1.2.3", diff --git a/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee b/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee index 3aa5b80528..00400542d8 100644 --- a/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee @@ -7,6 +7,9 @@ SandboxedModule = require('sandboxed-module') describe "ClsiManager", -> beforeEach -> + @jar = {cookie:"stuff"} + @ClsiRequestManager = + getCookieJar: sinon.stub().callsArgWith(1, null, @jar) @ClsiManager = SandboxedModule.require modulePath, requires: "settings-sharelatex": @settings = apis: @@ -19,8 +22,9 @@ describe "ClsiManager", -> url: "https://clsipremium.example.com" "../../models/Project": Project: @Project = {} "../Project/ProjectEntityHandler": @ProjectEntityHandler = {} + "./ClsiRequestManager": @ClsiRequestManager "logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub(), warn: sinon.stub() } - "request": @request = {} + "request": @request = sinon.stub() @project_id = "project-id" @callback = sinon.stub() @@ -80,15 +84,15 @@ describe "ClsiManager", -> describe "deleteAuxFiles", -> beforeEach -> - @request.del = sinon.stub().callsArg(1) + @ClsiManager._makeRequest = sinon.stub().callsArg(2) describe "with the standard compileGroup", -> beforeEach -> @ClsiManager.deleteAuxFiles @project_id, {compileGroup: "standard"}, @callback it "should call the delete method in the standard CLSI", -> - @request.del - .calledWith("#{@settings.apis.clsi.url}/project/#{@project_id}") + @ClsiManager._makeRequest + .calledWith(@project_id, { method:"DELETE", url:"#{@settings.apis.clsi.url}/project/#{@project_id}"}) .should.equal true it "should call the callback", -> @@ -99,8 +103,8 @@ describe "ClsiManager", -> @ClsiManager.deleteAuxFiles @project_id, {compileGroup: "priority"}, @callback it "should call the delete method in the CLSI", -> - @request.del - .calledWith("#{@settings.apis.clsi_priority.url}/project/#{@project_id}") + @ClsiManager._makeRequest + .calledWith(@project_id, { method:"DELETE", url:"#{@settings.apis.clsi_priority.url}/project/#{@project_id}"}) .should.equal true describe "_buildRequest", -> @@ -235,15 +239,15 @@ describe "ClsiManager", -> describe "successfully", -> beforeEach -> - @request.post = sinon.stub().callsArgWith(1, null, {statusCode: 204}, @body = { mock: "foo" }) + @ClsiManager._makeRequest = sinon.stub().callsArgWith(2, null, {statusCode: 204}, @body = { mock: "foo" }) @ClsiManager._postToClsi @project_id, @req, "standard", @callback it 'should send the request to the CLSI', -> url = "#{@settings.apis.clsi.url}/project/#{@project_id}/compile" - @request.post.calledWith({ + @ClsiManager._makeRequest.calledWith(@project_id, { + method: "POST", url: url json: @req - jar: false }).should.equal true it "should call the callback with the body and no error", -> @@ -251,7 +255,7 @@ describe "ClsiManager", -> describe "when the CLSI returns an error", -> beforeEach -> - @request.post = sinon.stub().callsArgWith(1, null, {statusCode: 500}, @body = { mock: "foo" }) + @ClsiManager._makeRequest = sinon.stub().callsArgWith(2, null, {statusCode: 500}, @body = { mock: "foo" }) @ClsiManager._postToClsi @project_id, @req, "standard", @callback it "should call the callback with the body and the error", -> @@ -259,20 +263,20 @@ describe "ClsiManager", -> describe "when the compiler is priority", -> beforeEach -> - @request.post = sinon.stub().callsArgWith(1, null, {statusCode: 500}, @body = { mock: "foo" }) + @ClsiManager._makeRequest = sinon.stub() @ClsiManager._postToClsi @project_id, @req, "priority", @callback it "should use the clsi_priority url", -> url = "#{@settings.apis.clsi_priority.url}/project/#{@project_id}/compile" - @request.post.calledWith({ + @ClsiManager._makeRequest.calledWith(@project_id, { + method: "POST", url: url json: @req - jar: false }).should.equal true describe "wordCount", -> beforeEach -> - @request.get = sinon.stub().callsArgWith(1, null, {statusCode: 200}, @body = { mock: "foo" }) + @ClsiManager._makeRequest = sinon.stub().callsArgWith(2, null, {statusCode: 200}, @body = { mock: "foo" }) @ClsiManager._buildRequest = sinon.stub().callsArgWith(2, null, @req = { compile: { rootResourcePath: "rootfile.text", options: {} } }) @ClsiManager._getCompilerUrl = sinon.stub().returns "compiler.url" @@ -281,8 +285,8 @@ describe "ClsiManager", -> @ClsiManager.wordCount @project_id, false, {}, @callback it "should call wordCount with root file", -> - @request.get - .calledWith({ url: "compiler.url/project/#{@project_id}/wordcount?file=rootfile.text" }) + @ClsiManager._makeRequest + .calledWith(@project_id, { method: "GET", url: "compiler.url/project/#{@project_id}/wordcount?file=rootfile.text" }) .should.equal true it "should call the callback", -> @@ -293,8 +297,8 @@ describe "ClsiManager", -> @ClsiManager.wordCount @project_id, "main.tex", {}, @callback it "should call wordCount with param file", -> - @request.get - .calledWith({ url: "compiler.url/project/#{@project_id}/wordcount?file=main.tex" }) + @ClsiManager._makeRequest + .calledWith(@project_id, { method: "GET", url: "compiler.url/project/#{@project_id}/wordcount?file=main.tex" }) .should.equal true describe "with image", -> @@ -303,6 +307,6 @@ describe "ClsiManager", -> @ClsiManager.wordCount @project_id, "main.tex", {}, @callback it "should call wordCount with file and image", -> - @request.get - .calledWith({ url: "compiler.url/project/#{@project_id}/wordcount?file=main.tex&image=#{encodeURIComponent(@image)}" }) + @ClsiManager._makeRequest + .calledWith(@project_id, { method: "GET", url: "compiler.url/project/#{@project_id}/wordcount?file=main.tex&image=#{encodeURIComponent(@image)}" }) .should.equal true diff --git a/services/web/test/UnitTests/coffee/Compile/ClsiRequestManagerTests.coffee b/services/web/test/UnitTests/coffee/Compile/ClsiRequestManagerTests.coffee new file mode 100644 index 0000000000..a88cef61c6 --- /dev/null +++ b/services/web/test/UnitTests/coffee/Compile/ClsiRequestManagerTests.coffee @@ -0,0 +1,111 @@ +sinon = require('sinon') +chai = require('chai') +should = chai.should() +expect = chai.expect +modulePath = "../../../../app/js/Features/Compile/ClsiRequestManager.js" +SandboxedModule = require('sandboxed-module') +realRequst = require("request") +describe "ClsiRequestManager", -> + beforeEach -> + @redisMulti = + set:sinon.stub() + get:sinon.stub() + expire:sinon.stub() + exec:sinon.stub() + self = @ + @project_id = "123423431321" + @request = + get: sinon.stub() + cookie:realRequst.cookie + jar: realRequst.jar + @ClsiRequestManager = SandboxedModule.require modulePath, requires: + "redis-sharelatex" : + createClient: => + auth:-> + multi: -> return self.redisMulti + "settings-sharelatex": @settings = + redis: + web:"redis.something" + apis: + clsi: + url: "http://clsi.example.com" + "request": @request + + "logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub(), warn: sinon.stub() } + + + + describe "getServerId", -> + + it "should call get for the key", (done)-> + @redisMulti.exec.callsArgWith(0, null, ["clsi-7"]) + @ClsiRequestManager._getServerId @project_id, (err, serverId)=> + @redisMulti.get.calledWith("clsiserver:#{@project_id}").should.equal true + serverId.should.equal "clsi-7" + done() + + it "should expire the key", (done)-> + @redisMulti.exec.callsArgWith(0, null, ["clsi-7"]) + @ClsiRequestManager._getServerId @project_id, (err, serverId)=> + @redisMulti.expire.calledWith("clsiserver:#{@project_id}", 60 * 60 * 24 * 7).should.equal true + done() + + it "should _getServerIdViaRequest if no key is found", (done)-> + @ClsiRequestManager._getServerIdViaRequest = sinon.stub().callsArgWith(1) + @redisMulti.exec.callsArgWith(0, null, []) + @ClsiRequestManager._getServerId @project_id, (err, serverId)=> + @ClsiRequestManager._getServerIdViaRequest.calledWith(@project_id).should.equal true + done() + + + describe "_getServerIdViaRequest", -> + + it "should make a request to the clsi", (done)-> + response = "some data" + @request.get.callsArgWith(1, null, response) + @ClsiRequestManager.setServerId = sinon.stub().callsArgWith(2) + @ClsiRequestManager._getServerIdViaRequest @project_id, (err, serverId)=> + args = @ClsiRequestManager.setServerId.args[0] + args[0].should.equal @project_id + args[1].should.deep.equal response + done() + + describe "setServerId", -> + + it "should set the server id with a ttl", (done)-> + @ClsiRequestManager._parseServerIdFromResponse = sinon.stub().returns("clsi-8") + response = "dsadsakj" + @redisMulti.exec.callsArgWith(0) + @ClsiRequestManager.setServerId @project_id, response, (err)=> + @redisMulti.set.calledWith("clsiserver:#{@project_id}", "clsi-8").should.equal true + @redisMulti.expire.calledWith("clsiserver:#{@project_id}", 60 * 60 * 24 * 7).should.equal true + done() + + + describe "getCookieJar", -> + + it "should return a jar with the cookie set populated from redis", (done)-> + @ClsiRequestManager._getServerId = sinon.stub().callsArgWith(1, null, "clsi-11") + opts = {} + @ClsiRequestManager.getCookieJar @project_id, opts, (err, jar)-> + jar._jar.store.idx["clsi.example.com"]["/"].clsiserver.key.should.equal "clsiserver" + jar._jar.store.idx["clsi.example.com"]["/"].clsiserver.value.should.equal "clsi-11" + done() + + + # describe "_parseServerIdFromResponse", -> + # it "take the cookie from the response", (done)-> + + # a.should.equal + + + + + + + + + + + + diff --git a/services/web/test/UnitTests/coffee/Compile/CompileControllerTests.coffee b/services/web/test/UnitTests/coffee/Compile/CompileControllerTests.coffee index 26cf0a2e2d..0827a6f206 100644 --- a/services/web/test/UnitTests/coffee/Compile/CompileControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Compile/CompileControllerTests.coffee @@ -22,6 +22,9 @@ describe "CompileController", -> url: "clsi.example.com" clsi_priority: url: "clsi-priority.example.com" + @jar = {cookie:"stuff"} + @ClsiRequestManager = + getCookieJar:sinon.stub().callsArgWith(1, null, @jar) @CompileController = SandboxedModule.require modulePath, requires: "settings-sharelatex": @settings "request": @request = sinon.stub() @@ -33,6 +36,7 @@ describe "CompileController", -> "./ClsiManager": @ClsiManager "../Authentication/AuthenticationController": @AuthenticationController = {} "../../infrastructure/RateLimiter":@RateLimiter + "./ClsiRequestManager":@ClsiRequestManager @project_id = "project-id" @user = features: @@ -182,6 +186,7 @@ describe "CompileController", -> it "should open a request to the CLSI", -> @request .calledWith( + jar:@jar method: @req.method url: "#{@settings.apis.clsi.url}#{@url}", timeout: 60 * 1000 @@ -204,6 +209,7 @@ describe "CompileController", -> it "should proxy to the priority url if the user has the feature", ()-> @request .calledWith( + jar:@jar method: @req.method url: "#{@settings.apis.clsi_priority.url}#{@url}", timeout: 60 * 1000 @@ -218,6 +224,7 @@ describe "CompileController", -> it "should open a request to the CLSI", -> @request .calledWith( + jar:@jar method: @req.method url: "#{@settings.apis.clsi.url}#{@url}", timeout: 60 * 1000 @@ -240,6 +247,7 @@ describe "CompileController", -> it "should proxy to the priority url if the user has the feature", ()-> @request .calledWith( + jar:@jar method: @req.method url: "#{@settings.apis.clsi_priority.url}#{@url}", timeout: 60 * 1000 @@ -254,6 +262,7 @@ describe "CompileController", -> it "should proxy to the standard url", ()-> @request .calledWith( + jar:@jar method: @req.method url: "#{@settings.apis.clsi.url}#{@url}", timeout: 60 * 1000 @@ -269,6 +278,7 @@ describe "CompileController", -> it "should proxy to the standard url without the build parameter", ()-> @request .calledWith( + jar:@jar method: @req.method url: "#{@settings.apis.clsi.url}#{@url}", timeout: 60 * 1000 @@ -286,6 +296,7 @@ describe "CompileController", -> it "should open a request to the CLSI", -> @request .calledWith( + jar:@jar method: @req.method url: "#{@settings.apis.clsi.url}#{@url}", timeout: 60 * 1000 @@ -313,6 +324,7 @@ describe "CompileController", -> it "should proxy to the priority url if the user has the feature", ()-> @request .calledWith( + jar:@jar method: @req.method url: "#{@settings.apis.clsi_priority.url}#{@url}", timeout: 60 * 1000 @@ -331,8 +343,8 @@ describe "CompileController", -> @CompileController.proxyToClsi(@project_id, @url = "/test", @req, @res, @next) it "should proxy to the standard url with the build parameter", ()-> - @request - .calledWith( + @request.calledWith( + jar:@jar method: @req.method qs: {build: 1234} url: "#{@settings.apis.clsi.url}#{@url}", From 4d54de8b9a55b7c14763b06b41662139a59fa908 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Wed, 20 Apr 2016 15:06:39 +0100 Subject: [PATCH 03/18] renamed ClsiRequestManager to ClsiCookieManager and improved logging --- ...anager.coffee => ClsiCookieManager.coffee} | 29 +++++++------ .../Features/Compile/ClsiManager.coffee | 24 ++++++----- .../Features/Compile/CompileController.coffee | 4 +- services/web/config/settings.defaults.coffee | 3 ++ ...s.coffee => ClsiCookieManagerTests.coffee} | 43 ++++++++----------- .../coffee/Compile/ClsiManagerTests.coffee | 4 +- .../Compile/CompileControllerTests.coffee | 4 +- 7 files changed, 56 insertions(+), 55 deletions(-) rename services/web/app/coffee/Features/Compile/{ClsiRequestManager.coffee => ClsiCookieManager.coffee} (59%) rename services/web/test/UnitTests/coffee/Compile/{ClsiRequestManagerTests.coffee => ClsiCookieManagerTests.coffee} (63%) diff --git a/services/web/app/coffee/Features/Compile/ClsiRequestManager.coffee b/services/web/app/coffee/Features/Compile/ClsiCookieManager.coffee similarity index 59% rename from services/web/app/coffee/Features/Compile/ClsiRequestManager.coffee rename to services/web/app/coffee/Features/Compile/ClsiCookieManager.coffee index 9de06c1538..e2b82e5d5d 100644 --- a/services/web/app/coffee/Features/Compile/ClsiRequestManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiCookieManager.coffee @@ -2,7 +2,8 @@ Settings = require "settings-sharelatex" request = require('request') redis = require("redis-sharelatex") rclient = redis.createClient(Settings.redis.web) -cookie = require('cookie') +Cookie = require('cookie') +logger = require "logger-sharelatex" buildKey = (project_id)-> return "clsiserver:#{project_id}" @@ -10,7 +11,7 @@ buildKey = (project_id)-> ONE_WEEK_IN_SECONDS = 60 * 60 * 24 * 7 -module.exports = ClsiRequestManager = +module.exports = ClsiCookieManager = _getServerId : (project_id, callback = (err, serverId)->)-> multi = rclient.multi() @@ -23,38 +24,40 @@ module.exports = ClsiRequestManager = if serverId? return callback(null, serverId) else - return ClsiRequestManager._getServerIdViaRequest project_id, callback + return ClsiCookieManager._populateServerIdViaRequest project_id, callback - _getServerIdViaRequest :(project_id, callback = (err, serverId)->)-> + _populateServerIdViaRequest :(project_id, callback = (err, serverId)->)-> url = "#{Settings.apis.clsi.url}/project/#{project_id}/status" request.get url, (err, res, body)-> if err? logger.err err:err, project_id:project_id, "error getting initial server id for project" return callback(err) - ClsiRequestManager.setServerId project_id, res, callback + ClsiCookieManager.setServerId project_id, res, (err)-> + if err? + logger.err err:err, project_id:project_id, "error setting server id via populate request" + callback(err) _parseServerIdFromResponse : (response)-> - console.log response.headers - cookies = cookie.parse(response.headers["set-cookie"]?[0] or "") - return cookies?.clsiserver + cookies = Cookie.parse(response.headers["set-cookie"]?[0] or "") + return cookies?[Settings.clsiCookieKey] setServerId: (project_id, response, callback = ->)-> - serverId = ClsiRequestManager._parseServerIdFromResponse(response) + serverId = ClsiCookieManager._parseServerIdFromResponse(response) multi = rclient.multi() multi.set buildKey(project_id), serverId multi.expire buildKey(project_id), ONE_WEEK_IN_SECONDS multi.exec callback - getCookieJar: (project_id, opts, callback = (err, jar)->)-> - ClsiRequestManager._getServerId project_id, (err, serverId)=> + getCookieJar: (project_id, callback = (err, jar)->)-> + ClsiCookieManager._getServerId project_id, (err, serverId)=> if err? logger.err err:err, project_id:project_id, "error getting server id" return callback(err) - cookie = request.cookie("clsiserver=#{serverId}") + serverCookie = request.cookie("clsiserver=#{serverId}") jar = request.jar() - jar.setCookie cookie, Settings.apis.clsi.url + jar.setCookie serverCookie, Settings.apis.clsi.url callback(null, jar) diff --git a/services/web/app/coffee/Features/Compile/ClsiManager.coffee b/services/web/app/coffee/Features/Compile/ClsiManager.coffee index 6f4efabcf9..0513fd4817 100755 --- a/services/web/app/coffee/Features/Compile/ClsiManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiManager.coffee @@ -6,25 +6,19 @@ Project = require("../../models/Project").Project ProjectEntityHandler = require("../Project/ProjectEntityHandler") logger = require "logger-sharelatex" url = require("url") -ClsiRequestManager = require("./ClsiRequestManager") +ClsiCookieManager = require("./ClsiCookieManager") module.exports = ClsiManager = - _makeRequest: (project_id, opts, callback)-> - ClsiRequestManager.getCookieJar project_id, (err, jar)-> - if err? - logger.err err:err, "error getting cookie jar for clsi request" - return callback(err) - opts.jar = jar - request opts, callback - sendRequest: (project_id, options = {}, callback = (error, success) ->) -> ClsiManager._buildRequest project_id, options, (error, req) -> return callback(error) if error? logger.log project_id: project_id, "sending compile to CLSI" ClsiManager._postToClsi project_id, req, options.compileGroup, (error, response) -> - return callback(error) if error? + if error? + logger.err err:error, project_id:project_id, "error sending request to clsi" + return callback(error) logger.log project_id: project_id, response: response, "received compile response from CLSI" callback( null @@ -39,6 +33,16 @@ module.exports = ClsiManager = method:"DELETE" ClsiManager._makeRequest project_id, opts, callback + + _makeRequest: (project_id, opts, callback)-> + ClsiCookieManager.getCookieJar project_id, (err, jar)-> + if err? + logger.err err:err, "error getting cookie jar for clsi request" + return callback(err) + opts.jar = jar + request opts, callback + + _getCompilerUrl: (compileGroup) -> if compileGroup == "priority" return Settings.apis.clsi_priority.url diff --git a/services/web/app/coffee/Features/Compile/CompileController.coffee b/services/web/app/coffee/Features/Compile/CompileController.coffee index 0e600dc3b0..934f9e97a4 100755 --- a/services/web/app/coffee/Features/Compile/CompileController.coffee +++ b/services/web/app/coffee/Features/Compile/CompileController.coffee @@ -8,7 +8,7 @@ Settings = require "settings-sharelatex" AuthenticationController = require "../Authentication/AuthenticationController" UserGetter = require "../User/UserGetter" RateLimiter = require("../../infrastructure/RateLimiter") -ClsiRequestManager = require("./ClsiRequestManager") +ClsiCookieManager = require("./ClsiCookieManager") module.exports = CompileController = @@ -109,7 +109,7 @@ module.exports = CompileController = CompileController.proxyToClsiWithLimits(project_id, url, limits, req, res, next) proxyToClsiWithLimits: (project_id, url, limits, req, res, next = (error) ->) -> - ClsiRequestManager.getCookieJar project_id, (err, jar)-> + ClsiCookieManager.getCookieJar project_id, (err, jar)-> if err? logger.err err:err, "error getting cookie jar for clsi request" return callback(err) diff --git a/services/web/config/settings.defaults.coffee b/services/web/config/settings.defaults.coffee index 2c16d41a52..5a1d207236 100644 --- a/services/web/config/settings.defaults.coffee +++ b/services/web/config/settings.defaults.coffee @@ -126,6 +126,9 @@ module.exports = # cookieDomain: ".sharelatex.dev" cookieName:"sharelatex.sid" + # this is only used if cookies are used for clsi backend + #clsiCookieKey: "clsiserver" + # Same, but with http auth credentials. httpAuthSiteUrl: 'http://#{httpAuthUser}:#{httpAuthPass}@localhost:3000' diff --git a/services/web/test/UnitTests/coffee/Compile/ClsiRequestManagerTests.coffee b/services/web/test/UnitTests/coffee/Compile/ClsiCookieManagerTests.coffee similarity index 63% rename from services/web/test/UnitTests/coffee/Compile/ClsiRequestManagerTests.coffee rename to services/web/test/UnitTests/coffee/Compile/ClsiCookieManagerTests.coffee index a88cef61c6..855a60b767 100644 --- a/services/web/test/UnitTests/coffee/Compile/ClsiRequestManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Compile/ClsiCookieManagerTests.coffee @@ -2,10 +2,10 @@ sinon = require('sinon') chai = require('chai') should = chai.should() expect = chai.expect -modulePath = "../../../../app/js/Features/Compile/ClsiRequestManager.js" +modulePath = "../../../../app/js/Features/Compile/ClsiCookieManager.js" SandboxedModule = require('sandboxed-module') realRequst = require("request") -describe "ClsiRequestManager", -> +describe "ClsiCookieManager", -> beforeEach -> @redisMulti = set:sinon.stub() @@ -18,7 +18,7 @@ describe "ClsiRequestManager", -> get: sinon.stub() cookie:realRequst.cookie jar: realRequst.jar - @ClsiRequestManager = SandboxedModule.require modulePath, requires: + @ClsiCookieManager = SandboxedModule.require modulePath, requires: "redis-sharelatex" : createClient: => auth:-> @@ -39,33 +39,33 @@ describe "ClsiRequestManager", -> it "should call get for the key", (done)-> @redisMulti.exec.callsArgWith(0, null, ["clsi-7"]) - @ClsiRequestManager._getServerId @project_id, (err, serverId)=> + @ClsiCookieManager._getServerId @project_id, (err, serverId)=> @redisMulti.get.calledWith("clsiserver:#{@project_id}").should.equal true serverId.should.equal "clsi-7" done() it "should expire the key", (done)-> @redisMulti.exec.callsArgWith(0, null, ["clsi-7"]) - @ClsiRequestManager._getServerId @project_id, (err, serverId)=> + @ClsiCookieManager._getServerId @project_id, (err, serverId)=> @redisMulti.expire.calledWith("clsiserver:#{@project_id}", 60 * 60 * 24 * 7).should.equal true done() - it "should _getServerIdViaRequest if no key is found", (done)-> - @ClsiRequestManager._getServerIdViaRequest = sinon.stub().callsArgWith(1) + it "should _populateServerIdViaRequest if no key is found", (done)-> + @ClsiCookieManager._populateServerIdViaRequest = sinon.stub().callsArgWith(1) @redisMulti.exec.callsArgWith(0, null, []) - @ClsiRequestManager._getServerId @project_id, (err, serverId)=> - @ClsiRequestManager._getServerIdViaRequest.calledWith(@project_id).should.equal true + @ClsiCookieManager._getServerId @project_id, (err, serverId)=> + @ClsiCookieManager._populateServerIdViaRequest.calledWith(@project_id).should.equal true done() - describe "_getServerIdViaRequest", -> + describe "_populateServerIdViaRequest", -> it "should make a request to the clsi", (done)-> response = "some data" @request.get.callsArgWith(1, null, response) - @ClsiRequestManager.setServerId = sinon.stub().callsArgWith(2) - @ClsiRequestManager._getServerIdViaRequest @project_id, (err, serverId)=> - args = @ClsiRequestManager.setServerId.args[0] + @ClsiCookieManager.setServerId = sinon.stub().callsArgWith(2) + @ClsiCookieManager._populateServerIdViaRequest @project_id, (err, serverId)=> + args = @ClsiCookieManager.setServerId.args[0] args[0].should.equal @project_id args[1].should.deep.equal response done() @@ -73,10 +73,10 @@ describe "ClsiRequestManager", -> describe "setServerId", -> it "should set the server id with a ttl", (done)-> - @ClsiRequestManager._parseServerIdFromResponse = sinon.stub().returns("clsi-8") + @ClsiCookieManager._parseServerIdFromResponse = sinon.stub().returns("clsi-8") response = "dsadsakj" @redisMulti.exec.callsArgWith(0) - @ClsiRequestManager.setServerId @project_id, response, (err)=> + @ClsiCookieManager.setServerId @project_id, response, (err)=> @redisMulti.set.calledWith("clsiserver:#{@project_id}", "clsi-8").should.equal true @redisMulti.expire.calledWith("clsiserver:#{@project_id}", 60 * 60 * 24 * 7).should.equal true done() @@ -85,23 +85,14 @@ describe "ClsiRequestManager", -> describe "getCookieJar", -> it "should return a jar with the cookie set populated from redis", (done)-> - @ClsiRequestManager._getServerId = sinon.stub().callsArgWith(1, null, "clsi-11") + @ClsiCookieManager._getServerId = sinon.stub().callsArgWith(1, null, "clsi-11") opts = {} - @ClsiRequestManager.getCookieJar @project_id, opts, (err, jar)-> + @ClsiCookieManager.getCookieJar @project_id, (err, jar)-> jar._jar.store.idx["clsi.example.com"]["/"].clsiserver.key.should.equal "clsiserver" jar._jar.store.idx["clsi.example.com"]["/"].clsiserver.value.should.equal "clsi-11" done() - # describe "_parseServerIdFromResponse", -> - # it "take the cookie from the response", (done)-> - - # a.should.equal - - - - - diff --git a/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee b/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee index 00400542d8..8f30634e9d 100644 --- a/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee @@ -8,7 +8,7 @@ SandboxedModule = require('sandboxed-module') describe "ClsiManager", -> beforeEach -> @jar = {cookie:"stuff"} - @ClsiRequestManager = + @ClsiCookieManager = getCookieJar: sinon.stub().callsArgWith(1, null, @jar) @ClsiManager = SandboxedModule.require modulePath, requires: "settings-sharelatex": @settings = @@ -22,7 +22,7 @@ describe "ClsiManager", -> url: "https://clsipremium.example.com" "../../models/Project": Project: @Project = {} "../Project/ProjectEntityHandler": @ProjectEntityHandler = {} - "./ClsiRequestManager": @ClsiRequestManager + "./ClsiCookieManager": @ClsiCookieManager "logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub(), warn: sinon.stub() } "request": @request = sinon.stub() @project_id = "project-id" diff --git a/services/web/test/UnitTests/coffee/Compile/CompileControllerTests.coffee b/services/web/test/UnitTests/coffee/Compile/CompileControllerTests.coffee index 0827a6f206..6bae5803c4 100644 --- a/services/web/test/UnitTests/coffee/Compile/CompileControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Compile/CompileControllerTests.coffee @@ -23,7 +23,7 @@ describe "CompileController", -> clsi_priority: url: "clsi-priority.example.com" @jar = {cookie:"stuff"} - @ClsiRequestManager = + @ClsiCookieManager = getCookieJar:sinon.stub().callsArgWith(1, null, @jar) @CompileController = SandboxedModule.require modulePath, requires: "settings-sharelatex": @settings @@ -36,7 +36,7 @@ describe "CompileController", -> "./ClsiManager": @ClsiManager "../Authentication/AuthenticationController": @AuthenticationController = {} "../../infrastructure/RateLimiter":@RateLimiter - "./ClsiRequestManager":@ClsiRequestManager + "./ClsiCookieManager":@ClsiCookieManager @project_id = "project-id" @user = features: From 18560d862162290479ea34f78c63341286697517 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Wed, 20 Apr 2016 16:17:06 +0100 Subject: [PATCH 04/18] set server cookie on every compile response and don't expire on get --- .../Features/Compile/ClsiCookieManager.coffee | 1 - .../Features/Compile/ClsiManager.coffee | 6 ++- .../Compile/ClsiCookieManagerTests.coffee | 6 --- .../coffee/Compile/ClsiManagerTests.coffee | 45 +++++++++++++++++++ 4 files changed, 50 insertions(+), 8 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/ClsiCookieManager.coffee b/services/web/app/coffee/Features/Compile/ClsiCookieManager.coffee index e2b82e5d5d..04332e685b 100644 --- a/services/web/app/coffee/Features/Compile/ClsiCookieManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiCookieManager.coffee @@ -16,7 +16,6 @@ module.exports = ClsiCookieManager = _getServerId : (project_id, callback = (err, serverId)->)-> multi = rclient.multi() multi.get buildKey(project_id) - multi.expire buildKey(project_id), ONE_WEEK_IN_SECONDS multi.exec (err, results)-> if err? return callback(err) diff --git a/services/web/app/coffee/Features/Compile/ClsiManager.coffee b/services/web/app/coffee/Features/Compile/ClsiManager.coffee index 0513fd4817..a0e03c2226 100755 --- a/services/web/app/coffee/Features/Compile/ClsiManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiManager.coffee @@ -40,7 +40,11 @@ module.exports = ClsiManager = logger.err err:err, "error getting cookie jar for clsi request" return callback(err) opts.jar = jar - request opts, callback + request opts, (err, response, body)-> + if err? + logger.err err:err, project_id:project_id, url:opts?.url, "error making request to clsi" + return callback(err) + ClsiCookieManager.setServerId project_id, response, callback _getCompilerUrl: (compileGroup) -> diff --git a/services/web/test/UnitTests/coffee/Compile/ClsiCookieManagerTests.coffee b/services/web/test/UnitTests/coffee/Compile/ClsiCookieManagerTests.coffee index 855a60b767..587e906db9 100644 --- a/services/web/test/UnitTests/coffee/Compile/ClsiCookieManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Compile/ClsiCookieManagerTests.coffee @@ -44,12 +44,6 @@ describe "ClsiCookieManager", -> serverId.should.equal "clsi-7" done() - it "should expire the key", (done)-> - @redisMulti.exec.callsArgWith(0, null, ["clsi-7"]) - @ClsiCookieManager._getServerId @project_id, (err, serverId)=> - @redisMulti.expire.calledWith("clsiserver:#{@project_id}", 60 * 60 * 24 * 7).should.equal true - done() - it "should _populateServerIdViaRequest if no key is found", (done)-> @ClsiCookieManager._populateServerIdViaRequest = sinon.stub().callsArgWith(1) @redisMulti.exec.callsArgWith(0, null, []) diff --git a/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee b/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee index 8f30634e9d..ba4f717e5f 100644 --- a/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee @@ -10,6 +10,7 @@ describe "ClsiManager", -> @jar = {cookie:"stuff"} @ClsiCookieManager = getCookieJar: sinon.stub().callsArgWith(1, null, @jar) + setServerId: sinon.stub().callsArgWith(2) @ClsiManager = SandboxedModule.require modulePath, requires: "settings-sharelatex": @settings = apis: @@ -310,3 +311,47 @@ describe "ClsiManager", -> @ClsiManager._makeRequest .calledWith(@project_id, { method: "GET", url: "compiler.url/project/#{@project_id}/wordcount?file=main.tex&image=#{encodeURIComponent(@image)}" }) .should.equal true + + + + describe "_makeRequest", -> + + beforeEach -> + @response = {there:"something"} + @request.callsArgWith(1, null, @response) + @opts = + method: "SOMETHIGN" + url: "http://a place on the web" + + it "should process a request with a cookie jar", (done)-> + @ClsiManager._makeRequest @project_id, @opts, => + args = @request.args[0] + args[0].method.should.equal @opts.method + args[0].url.should.equal @opts.url + args[0].jar.should.equal @jar + done() + + it "should set the cookie again on response as it might have changed", (done)-> + @ClsiManager._makeRequest @project_id, @opts, => + @ClsiCookieManager.setServerId.calledWith(@project_id, @response).should.equal true + done() + + + + + + + + + + + + + + + + + + + + From 616630200a35604f2104408126d2f935e14ce2a8 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Wed, 20 Apr 2016 17:00:17 +0100 Subject: [PATCH 05/18] improve send calls and return correct stuff from _makeRequest --- services/web/app/coffee/Features/Compile/ClsiManager.coffee | 6 +++++- .../app/coffee/Features/Compile/CompileController.coffee | 2 +- .../web/test/UnitTests/coffee/helpers/MockResponse.coffee | 4 ++++ 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/ClsiManager.coffee b/services/web/app/coffee/Features/Compile/ClsiManager.coffee index a0e03c2226..473df0f5cd 100755 --- a/services/web/app/coffee/Features/Compile/ClsiManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiManager.coffee @@ -44,7 +44,11 @@ module.exports = ClsiManager = if err? logger.err err:err, project_id:project_id, url:opts?.url, "error making request to clsi" return callback(err) - ClsiCookieManager.setServerId project_id, response, callback + ClsiCookieManager.setServerId project_id, response, (err)-> + if err? + logger.warn err:err, project_id:project_id, "error setting server id" + + return callback err, response, body _getCompilerUrl: (compileGroup) -> diff --git a/services/web/app/coffee/Features/Compile/CompileController.coffee b/services/web/app/coffee/Features/Compile/CompileController.coffee index 934f9e97a4..6ffca6affe 100755 --- a/services/web/app/coffee/Features/Compile/CompileController.coffee +++ b/services/web/app/coffee/Features/Compile/CompileController.coffee @@ -33,7 +33,7 @@ module.exports = CompileController = CompileManager.compile project_id, user_id, options, (error, status, outputFiles, output, limits) -> return next(error) if error? res.contentType("application/json") - res.send 200, JSON.stringify { + res.status(200).send JSON.stringify { status: status outputFiles: outputFiles compileGroup: limits?.compileGroup diff --git a/services/web/test/UnitTests/coffee/helpers/MockResponse.coffee b/services/web/test/UnitTests/coffee/helpers/MockResponse.coffee index a7182c9df0..5ed7c58522 100644 --- a/services/web/test/UnitTests/coffee/helpers/MockResponse.coffee +++ b/services/web/test/UnitTests/coffee/helpers/MockResponse.coffee @@ -63,6 +63,10 @@ class MockResponse @body = body if body @callback() if @callback? + status: (@statusCode)-> + return @ + + setHeader: (header, value) -> @headers[header] = value From 1ee94f9bf5095738638e0128653703c7ae0f649c Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Wed, 27 Apr 2016 16:20:10 +0100 Subject: [PATCH 06/18] return server id for set server id and _populateServerIdViaRequest --- .../Features/Compile/ClsiCookieManager.coffee | 9 +++--- .../Compile/ClsiCookieManagerTests.coffee | 28 ++++++++++++++----- 2 files changed, 26 insertions(+), 11 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/ClsiCookieManager.coffee b/services/web/app/coffee/Features/Compile/ClsiCookieManager.coffee index 04332e685b..ff42eb8820 100644 --- a/services/web/app/coffee/Features/Compile/ClsiCookieManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiCookieManager.coffee @@ -32,21 +32,22 @@ module.exports = ClsiCookieManager = if err? logger.err err:err, project_id:project_id, "error getting initial server id for project" return callback(err) - ClsiCookieManager.setServerId project_id, res, (err)-> + ClsiCookieManager.setServerId project_id, res, (err, serverId)-> if err? logger.err err:err, project_id:project_id, "error setting server id via populate request" - callback(err) + callback(err, serverId) _parseServerIdFromResponse : (response)-> cookies = Cookie.parse(response.headers["set-cookie"]?[0] or "") return cookies?[Settings.clsiCookieKey] - setServerId: (project_id, response, callback = ->)-> + setServerId: (project_id, response, callback = (err, serverId)->)-> serverId = ClsiCookieManager._parseServerIdFromResponse(response) multi = rclient.multi() multi.set buildKey(project_id), serverId multi.expire buildKey(project_id), ONE_WEEK_IN_SECONDS - multi.exec callback + multi.exec (err)-> + callback(err, serverId) getCookieJar: (project_id, callback = (err, jar)->)-> diff --git a/services/web/test/UnitTests/coffee/Compile/ClsiCookieManagerTests.coffee b/services/web/test/UnitTests/coffee/Compile/ClsiCookieManagerTests.coffee index 587e906db9..e4013ff06e 100644 --- a/services/web/test/UnitTests/coffee/Compile/ClsiCookieManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Compile/ClsiCookieManagerTests.coffee @@ -5,6 +5,7 @@ expect = chai.expect modulePath = "../../../../app/js/Features/Compile/ClsiCookieManager.js" SandboxedModule = require('sandboxed-module') realRequst = require("request") + describe "ClsiCookieManager", -> beforeEach -> @redisMulti = @@ -54,27 +55,40 @@ describe "ClsiCookieManager", -> describe "_populateServerIdViaRequest", -> + beforeEach -> + @response = "some data" + @request.get.callsArgWith(1, null, @response) + @ClsiCookieManager.setServerId = sinon.stub().callsArgWith(2, null, "clsi-9") + it "should make a request to the clsi", (done)-> - response = "some data" - @request.get.callsArgWith(1, null, response) - @ClsiCookieManager.setServerId = sinon.stub().callsArgWith(2) @ClsiCookieManager._populateServerIdViaRequest @project_id, (err, serverId)=> args = @ClsiCookieManager.setServerId.args[0] args[0].should.equal @project_id - args[1].should.deep.equal response + args[1].should.deep.equal @response + done() + + it "should return the server id", (done)-> + @ClsiCookieManager._populateServerIdViaRequest @project_id, (err, serverId)=> + serverId.should.equal "clsi-9" done() describe "setServerId", -> - it "should set the server id with a ttl", (done)-> + beforeEach -> + @response = "dsadsakj" @ClsiCookieManager._parseServerIdFromResponse = sinon.stub().returns("clsi-8") - response = "dsadsakj" @redisMulti.exec.callsArgWith(0) - @ClsiCookieManager.setServerId @project_id, response, (err)=> + + it "should set the server id with a ttl", (done)-> + @ClsiCookieManager.setServerId @project_id, @response, (err)=> @redisMulti.set.calledWith("clsiserver:#{@project_id}", "clsi-8").should.equal true @redisMulti.expire.calledWith("clsiserver:#{@project_id}", 60 * 60 * 24 * 7).should.equal true done() + it "should return the server id", (done)-> + @ClsiCookieManager.setServerId @project_id, @response, (err, serverId)=> + serverId.should.equal "clsi-8" + done() describe "getCookieJar", -> From 78b08060abf457c359eaf96c35005bf1bf091ecd Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Wed, 27 Apr 2016 16:56:21 +0100 Subject: [PATCH 07/18] redis get not multi used for _getServerId --- .../Features/Compile/ClsiCookieManager.coffee | 9 +++++---- .../coffee/Compile/ClsiCookieManagerTests.coffee | 16 ++++++++++------ 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/ClsiCookieManager.coffee b/services/web/app/coffee/Features/Compile/ClsiCookieManager.coffee index ff42eb8820..3c2fe8a879 100644 --- a/services/web/app/coffee/Features/Compile/ClsiCookieManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiCookieManager.coffee @@ -9,17 +9,16 @@ buildKey = (project_id)-> return "clsiserver:#{project_id}" +clsiCookiesEnabled = Settings.clsiCookieKey? and Settings.clsiCookieKey.length != 0 + ONE_WEEK_IN_SECONDS = 60 * 60 * 24 * 7 module.exports = ClsiCookieManager = _getServerId : (project_id, callback = (err, serverId)->)-> - multi = rclient.multi() - multi.get buildKey(project_id) - multi.exec (err, results)-> + rclient.get buildKey(project_id), (err, serverId)-> if err? return callback(err) - serverId = results[0] if serverId? return callback(null, serverId) else @@ -51,6 +50,8 @@ module.exports = ClsiCookieManager = getCookieJar: (project_id, callback = (err, jar)->)-> + # if !clsiCookiesEnabled + # return callback(null, request.jar()) ClsiCookieManager._getServerId project_id, (err, serverId)=> if err? logger.err err:err, project_id:project_id, "error getting server id" diff --git a/services/web/test/UnitTests/coffee/Compile/ClsiCookieManagerTests.coffee b/services/web/test/UnitTests/coffee/Compile/ClsiCookieManagerTests.coffee index e4013ff06e..576024e10e 100644 --- a/services/web/test/UnitTests/coffee/Compile/ClsiCookieManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Compile/ClsiCookieManagerTests.coffee @@ -8,12 +8,16 @@ realRequst = require("request") describe "ClsiCookieManager", -> beforeEach -> + self = @ @redisMulti = set:sinon.stub() get:sinon.stub() expire:sinon.stub() exec:sinon.stub() - self = @ + @redis = + auth:-> + get:sinon.stub() + multi: -> return self.redisMulti @project_id = "123423431321" @request = get: sinon.stub() @@ -22,14 +26,14 @@ describe "ClsiCookieManager", -> @ClsiCookieManager = SandboxedModule.require modulePath, requires: "redis-sharelatex" : createClient: => - auth:-> - multi: -> return self.redisMulti + @redis "settings-sharelatex": @settings = redis: web:"redis.something" apis: clsi: url: "http://clsi.example.com" + clsiCookieKey: "coooookie" "request": @request "logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub(), warn: sinon.stub() } @@ -39,15 +43,15 @@ describe "ClsiCookieManager", -> describe "getServerId", -> it "should call get for the key", (done)-> - @redisMulti.exec.callsArgWith(0, null, ["clsi-7"]) + @redis.get.callsArgWith(1, null, "clsi-7") @ClsiCookieManager._getServerId @project_id, (err, serverId)=> - @redisMulti.get.calledWith("clsiserver:#{@project_id}").should.equal true + @redis.get.calledWith("clsiserver:#{@project_id}").should.equal true serverId.should.equal "clsi-7" done() it "should _populateServerIdViaRequest if no key is found", (done)-> @ClsiCookieManager._populateServerIdViaRequest = sinon.stub().callsArgWith(1) - @redisMulti.exec.callsArgWith(0, null, []) + @redis.get.callsArgWith(1, null) @ClsiCookieManager._getServerId @project_id, (err, serverId)=> @ClsiCookieManager._populateServerIdViaRequest.calledWith(@project_id).should.equal true done() From b00bd5cd946b90c157d4e02b916798390ff879b1 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Wed, 27 Apr 2016 17:05:12 +0100 Subject: [PATCH 08/18] if clsi cookies are not enabled don't call redis, return empty --- .../Features/Compile/ClsiCookieManager.coffee | 7 ++-- .../Compile/ClsiCookieManagerTests.coffee | 42 +++++++++++++------ 2 files changed, 34 insertions(+), 15 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/ClsiCookieManager.coffee b/services/web/app/coffee/Features/Compile/ClsiCookieManager.coffee index 3c2fe8a879..ad8c908cee 100644 --- a/services/web/app/coffee/Features/Compile/ClsiCookieManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiCookieManager.coffee @@ -8,7 +8,6 @@ logger = require "logger-sharelatex" buildKey = (project_id)-> return "clsiserver:#{project_id}" - clsiCookiesEnabled = Settings.clsiCookieKey? and Settings.clsiCookieKey.length != 0 ONE_WEEK_IN_SECONDS = 60 * 60 * 24 * 7 @@ -41,6 +40,8 @@ module.exports = ClsiCookieManager = return cookies?[Settings.clsiCookieKey] setServerId: (project_id, response, callback = (err, serverId)->)-> + if !clsiCookiesEnabled + return callback() serverId = ClsiCookieManager._parseServerIdFromResponse(response) multi = rclient.multi() multi.set buildKey(project_id), serverId @@ -50,8 +51,8 @@ module.exports = ClsiCookieManager = getCookieJar: (project_id, callback = (err, jar)->)-> - # if !clsiCookiesEnabled - # return callback(null, request.jar()) + if !clsiCookiesEnabled + return callback(null, request.jar()) ClsiCookieManager._getServerId project_id, (err, serverId)=> if err? logger.err err:err, project_id:project_id, "error getting server id" diff --git a/services/web/test/UnitTests/coffee/Compile/ClsiCookieManagerTests.coffee b/services/web/test/UnitTests/coffee/Compile/ClsiCookieManagerTests.coffee index 576024e10e..28dad51d8b 100644 --- a/services/web/test/UnitTests/coffee/Compile/ClsiCookieManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Compile/ClsiCookieManagerTests.coffee @@ -1,5 +1,6 @@ sinon = require('sinon') chai = require('chai') +assert = chai.assert should = chai.should() expect = chai.expect modulePath = "../../../../app/js/Features/Compile/ClsiCookieManager.js" @@ -23,20 +24,22 @@ describe "ClsiCookieManager", -> get: sinon.stub() cookie:realRequst.cookie jar: realRequst.jar - @ClsiCookieManager = SandboxedModule.require modulePath, requires: + @settings = + redis: + web:"redis.something" + apis: + clsi: + url: "http://clsi.example.com" + clsiCookieKey: "coooookie" + @requires = "redis-sharelatex" : createClient: => @redis - "settings-sharelatex": @settings = - redis: - web:"redis.something" - apis: - clsi: - url: "http://clsi.example.com" - clsiCookieKey: "coooookie" + "settings-sharelatex": @settings "request": @request "logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub(), warn: sinon.stub() } + @ClsiCookieManager = SandboxedModule.require modulePath, requires:@requires @@ -94,21 +97,36 @@ describe "ClsiCookieManager", -> serverId.should.equal "clsi-8" done() + + it "should not set the server id if clsiCookies are not enabled", (done)-> + delete @settings.clsiCookieKey + @ClsiCookieManager = SandboxedModule.require modulePath, requires:@requires + @ClsiCookieManager.setServerId @project_id, @response, (err, serverId)=> + @redisMulti.exec.called.should.equal false + done() + describe "getCookieJar", -> - it "should return a jar with the cookie set populated from redis", (done)-> + beforeEach -> @ClsiCookieManager._getServerId = sinon.stub().callsArgWith(1, null, "clsi-11") - opts = {} + + it "should return a jar with the cookie set populated from redis", (done)-> @ClsiCookieManager.getCookieJar @project_id, (err, jar)-> jar._jar.store.idx["clsi.example.com"]["/"].clsiserver.key.should.equal "clsiserver" jar._jar.store.idx["clsi.example.com"]["/"].clsiserver.value.should.equal "clsi-11" done() + it "should return empty cookie jar if clsiCookies are not enabled", (done)-> + delete @settings.clsiCookieKey + @ClsiCookieManager = SandboxedModule.require modulePath, requires:@requires + @ClsiCookieManager.getCookieJar @project_id, (err, jar)-> + assert.deepEqual jar, realRequst.jar() + done() + + - - From ed4fdd48d771a746148963e3d069451d0db168e6 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Thu, 5 May 2016 16:50:18 +0100 Subject: [PATCH 09/18] clsi cookies are given an expire time via settings file --- .../web/app/coffee/Features/Compile/ClsiCookieManager.coffee | 3 +-- .../UnitTests/coffee/Compile/ClsiCookieManagerTests.coffee | 3 ++- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/ClsiCookieManager.coffee b/services/web/app/coffee/Features/Compile/ClsiCookieManager.coffee index ad8c908cee..9827e328b4 100644 --- a/services/web/app/coffee/Features/Compile/ClsiCookieManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiCookieManager.coffee @@ -10,7 +10,6 @@ buildKey = (project_id)-> clsiCookiesEnabled = Settings.clsiCookieKey? and Settings.clsiCookieKey.length != 0 -ONE_WEEK_IN_SECONDS = 60 * 60 * 24 * 7 module.exports = ClsiCookieManager = @@ -45,7 +44,7 @@ module.exports = ClsiCookieManager = serverId = ClsiCookieManager._parseServerIdFromResponse(response) multi = rclient.multi() multi.set buildKey(project_id), serverId - multi.expire buildKey(project_id), ONE_WEEK_IN_SECONDS + multi.expire buildKey(project_id), Settings.clsi_cookie_expire_length multi.exec (err)-> callback(err, serverId) diff --git a/services/web/test/UnitTests/coffee/Compile/ClsiCookieManagerTests.coffee b/services/web/test/UnitTests/coffee/Compile/ClsiCookieManagerTests.coffee index 28dad51d8b..f40078a1d2 100644 --- a/services/web/test/UnitTests/coffee/Compile/ClsiCookieManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Compile/ClsiCookieManagerTests.coffee @@ -30,6 +30,7 @@ describe "ClsiCookieManager", -> apis: clsi: url: "http://clsi.example.com" + clsi_cookie_expire_length: Math.random() clsiCookieKey: "coooookie" @requires = "redis-sharelatex" : @@ -89,7 +90,7 @@ describe "ClsiCookieManager", -> it "should set the server id with a ttl", (done)-> @ClsiCookieManager.setServerId @project_id, @response, (err)=> @redisMulti.set.calledWith("clsiserver:#{@project_id}", "clsi-8").should.equal true - @redisMulti.expire.calledWith("clsiserver:#{@project_id}", 60 * 60 * 24 * 7).should.equal true + @redisMulti.expire.calledWith("clsiserver:#{@project_id}", @settings.clsi_cookie_expire_length).should.equal true done() it "should return the server id", (done)-> From b8510301b603c9ddcc889801a52acaff34da2ee2 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Fri, 6 May 2016 12:19:22 +0100 Subject: [PATCH 10/18] expire cookie key from setting file length --- .../web/app/coffee/Features/Compile/ClsiCookieManager.coffee | 2 +- .../UnitTests/coffee/Compile/ClsiCookieManagerTests.coffee | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/ClsiCookieManager.coffee b/services/web/app/coffee/Features/Compile/ClsiCookieManager.coffee index 9827e328b4..81e2e633e6 100644 --- a/services/web/app/coffee/Features/Compile/ClsiCookieManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiCookieManager.coffee @@ -44,7 +44,7 @@ module.exports = ClsiCookieManager = serverId = ClsiCookieManager._parseServerIdFromResponse(response) multi = rclient.multi() multi.set buildKey(project_id), serverId - multi.expire buildKey(project_id), Settings.clsi_cookie_expire_length + multi.expire buildKey(project_id), Settings.clsi_cookie_expire_length_seconds multi.exec (err)-> callback(err, serverId) diff --git a/services/web/test/UnitTests/coffee/Compile/ClsiCookieManagerTests.coffee b/services/web/test/UnitTests/coffee/Compile/ClsiCookieManagerTests.coffee index f40078a1d2..1fc8716657 100644 --- a/services/web/test/UnitTests/coffee/Compile/ClsiCookieManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Compile/ClsiCookieManagerTests.coffee @@ -30,7 +30,7 @@ describe "ClsiCookieManager", -> apis: clsi: url: "http://clsi.example.com" - clsi_cookie_expire_length: Math.random() + clsi_cookie_expire_length_seconds: Math.random() clsiCookieKey: "coooookie" @requires = "redis-sharelatex" : @@ -90,7 +90,7 @@ describe "ClsiCookieManager", -> it "should set the server id with a ttl", (done)-> @ClsiCookieManager.setServerId @project_id, @response, (err)=> @redisMulti.set.calledWith("clsiserver:#{@project_id}", "clsi-8").should.equal true - @redisMulti.expire.calledWith("clsiserver:#{@project_id}", @settings.clsi_cookie_expire_length).should.equal true + @redisMulti.expire.calledWith("clsiserver:#{@project_id}", @settings.clsi_cookie_expire_length_seconds).should.equal true done() it "should return the server id", (done)-> From b0baea5073d061d761dd046ea966905a8af761e5 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Wed, 18 May 2016 10:09:22 +0100 Subject: [PATCH 11/18] add query string to end of project resources --- .../Features/Compile/ClsiManager.coffee | 22 +++++++++++++------ .../Features/Compile/CompileManager.coffee | 4 ++-- .../coffee/Compile/ClsiManagerTests.coffee | 2 ++ 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/ClsiManager.coffee b/services/web/app/coffee/Features/Compile/ClsiManager.coffee index 473df0f5cd..df7b5749cf 100755 --- a/services/web/app/coffee/Features/Compile/ClsiManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiManager.coffee @@ -20,11 +20,13 @@ module.exports = ClsiManager = logger.err err:error, project_id:project_id, "error sending request to clsi" return callback(error) logger.log project_id: project_id, response: response, "received compile response from CLSI" - callback( - null - response?.compile?.status - ClsiManager._parseOutputFiles(project_id, response?.compile?.outputFiles) - ) + ClsiCookieManager._getServerId project_id, (err, clsiServerId)-> + if err? + logger.err err:err, project_id:project_id, "error getting server id" + return callback(err) + outputFiles = ClsiManager._parseOutputFiles(project_id, response?.compile?.outputFiles, clsiServerId) + console.log outputFiles + callback(null, response?.compile?.status, outputFiles) deleteAuxFiles: (project_id, options, callback = (error) ->) -> compilerUrl = @_getCompilerUrl(options?.compileGroup) @@ -74,11 +76,17 @@ module.exports = ClsiManager = logger.error err: error, project_id: project_id, "CLSI returned failure code" callback error, body - _parseOutputFiles: (project_id, rawOutputFiles = []) -> + _parseOutputFiles: (project_id, rawOutputFiles = [], clsiServer) -> + # console.log rawOutputFiles outputFiles = [] for file in rawOutputFiles + console.log path + path = url.parse(file.url).path + path = path.replace("/project/#{project_id}/output/", "") + if clsiServer? + path = "#{path}?clsiserver=#{clsiServer}" outputFiles.push - path: url.parse(file.url).path.replace("/project/#{project_id}/output/", "") + path: path type: file.type build: file.build return outputFiles diff --git a/services/web/app/coffee/Features/Compile/CompileManager.coffee b/services/web/app/coffee/Features/Compile/CompileManager.coffee index c89e7107dd..8249352e5d 100755 --- a/services/web/app/coffee/Features/Compile/CompileManager.coffee +++ b/services/web/app/coffee/Features/Compile/CompileManager.coffee @@ -1,8 +1,6 @@ Settings = require('settings-sharelatex') - redis = require("redis-sharelatex") rclient = redis.createClient(Settings.redis.web) - DocumentUpdaterHandler = require "../DocumentUpdater/DocumentUpdaterHandler" Project = require("../../models/Project").Project ProjectRootDocManager = require "../Project/ProjectRootDocManager" @@ -13,6 +11,8 @@ logger = require("logger-sharelatex") rateLimiter = require("../../infrastructure/RateLimiter") module.exports = CompileManager = + + compile: (project_id, user_id, options = {}, _callback = (error) ->) -> timer = new Metrics.Timer("editor.compile") callback = (args...) -> diff --git a/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee b/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee index ba4f717e5f..89f312441d 100644 --- a/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee @@ -11,6 +11,7 @@ describe "ClsiManager", -> @ClsiCookieManager = getCookieJar: sinon.stub().callsArgWith(1, null, @jar) setServerId: sinon.stub().callsArgWith(2) + _getServerId:sinon.stub() @ClsiManager = SandboxedModule.require modulePath, requires: "settings-sharelatex": @settings = apis: @@ -32,6 +33,7 @@ describe "ClsiManager", -> describe "sendRequest", -> beforeEach -> @ClsiManager._buildRequest = sinon.stub().callsArgWith(2, null, @request = "mock-request") + @ClsiCookieManager._getServerId.callsArgWith(1, null, "clsi3") describe "with a successful compile", -> beforeEach -> From dba8d96d1143220613c79f7be928ccaa4feb5bb3 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Wed, 18 May 2016 12:50:50 +0100 Subject: [PATCH 12/18] pass clsiServerId to the client and use it as query stirng for requests --- .../Features/Compile/ClsiManager.coffee | 5 +--- .../Features/Compile/CompileController.coffee | 3 ++- .../Features/Compile/CompileManager.coffee | 4 ++-- .../web/app/views/project/editor/pdf.jade | 2 +- .../ide/pdf/controllers/PdfController.coffee | 24 ++++++++++++++++--- .../WordCountModalController.coffee | 8 +++++-- 6 files changed, 33 insertions(+), 13 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/ClsiManager.coffee b/services/web/app/coffee/Features/Compile/ClsiManager.coffee index df7b5749cf..3285026cfa 100755 --- a/services/web/app/coffee/Features/Compile/ClsiManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiManager.coffee @@ -25,8 +25,7 @@ module.exports = ClsiManager = logger.err err:err, project_id:project_id, "error getting server id" return callback(err) outputFiles = ClsiManager._parseOutputFiles(project_id, response?.compile?.outputFiles, clsiServerId) - console.log outputFiles - callback(null, response?.compile?.status, outputFiles) + callback(null, response?.compile?.status, outputFiles, clsiServerId) deleteAuxFiles: (project_id, options, callback = (error) ->) -> compilerUrl = @_getCompilerUrl(options?.compileGroup) @@ -83,8 +82,6 @@ module.exports = ClsiManager = console.log path path = url.parse(file.url).path path = path.replace("/project/#{project_id}/output/", "") - if clsiServer? - path = "#{path}?clsiserver=#{clsiServer}" outputFiles.push path: path type: file.type diff --git a/services/web/app/coffee/Features/Compile/CompileController.coffee b/services/web/app/coffee/Features/Compile/CompileController.coffee index 6ffca6affe..66f98c5974 100755 --- a/services/web/app/coffee/Features/Compile/CompileController.coffee +++ b/services/web/app/coffee/Features/Compile/CompileController.coffee @@ -30,13 +30,14 @@ module.exports = CompileController = if req.body?.draft options.draft = req.body.draft logger.log {options, project_id}, "got compile request" - CompileManager.compile project_id, user_id, options, (error, status, outputFiles, output, limits) -> + CompileManager.compile project_id, user_id, options, (error, status, outputFiles, clsiServerId, limits) -> return next(error) if error? res.contentType("application/json") res.status(200).send JSON.stringify { status: status outputFiles: outputFiles compileGroup: limits?.compileGroup + clsiServerId:clsiServerId } downloadPdf: (req, res, next = (error) ->)-> diff --git a/services/web/app/coffee/Features/Compile/CompileManager.coffee b/services/web/app/coffee/Features/Compile/CompileManager.coffee index 8249352e5d..0d8d480b7b 100755 --- a/services/web/app/coffee/Features/Compile/CompileManager.coffee +++ b/services/web/app/coffee/Features/Compile/CompileManager.coffee @@ -37,10 +37,10 @@ module.exports = CompileManager = return callback(error) if error? for key, value of limits options[key] = value - ClsiManager.sendRequest project_id, options, (error, status, outputFiles, output) -> + ClsiManager.sendRequest project_id, options, (error, status, outputFiles, clsiServerId) -> return callback(error) if error? logger.log files: outputFiles, "output files" - callback(null, status, outputFiles, output, limits) + callback(null, status, outputFiles, clsiServerId, limits) deleteAuxFiles: (project_id, callback = (error) ->) -> CompileManager.getProjectCompileLimits project_id, (error, limits) -> diff --git a/services/web/app/views/project/editor/pdf.jade b/services/web/app/views/project/editor/pdf.jade index f633060aaf..186391893f 100644 --- a/services/web/app/views/project/editor/pdf.jade +++ b/services/web/app/views/project/editor/pdf.jade @@ -121,7 +121,7 @@ div.full-size.pdf(ng-controller="PdfController") ul.dropdown-menu.dropdown-menu-right li(ng-repeat="file in pdf.outputFiles") a( - href="/project/{{project_id}}/output/{{file.path}}" + href="{{file.url}}" target="_blank" ) {{ file.name }} a.btn.btn-info.btn-sm(href, ng-click="toggleRawLog()") diff --git a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee index 5847b58e3e..82be226d42 100644 --- a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee +++ b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee @@ -37,6 +37,9 @@ define [ } parseCompileResponse = (response) -> + if response.clsiServerId? and response.clsiServerId != $scope.pdf.clsiServerId + ide.clsiServerId = response.clsiServerId + # Reset everything $scope.pdf.error = false $scope.pdf.timedout = false @@ -76,6 +79,8 @@ define [ if response.compileGroup? $scope.pdf.compileGroup = response.compileGroup $scope.pdf.url = $scope.pdf.url + "&compileGroup=#{$scope.pdf.compileGroup}" + if response.clsiServerId? + $scope.pdf.url = $scope.pdf.url + "&clsiserverid=#{response.clsiServerId}" # make a cache to look up files by name fileByPath = {} for file in response.outputFiles @@ -99,11 +104,19 @@ define [ file.name = "#{file.path.replace(/^output\./, "")} file" else file.name = file.path + file.url = "/project/#{project_id}/output/#{file.path}" + if response.clsiServerId? + file.url = file.url + "?clsiserverid=#{response.clsiServerId}" $scope.pdf.outputFiles.push file fetchLogs = (outputFile) -> - qs = if outputFile?.build? then "?build=#{outputFile.build}" else "" - $http.get "/project/#{$scope.project_id}/output/output.log" + qs + opts = + method:"GET" + url:"/project/#{$scope.project_id}/output/output.log" + params: + build:outputFile.build + clsiserverid:ide.clsiServerId + $http opts .success (log) -> #console.log ">>", log $scope.pdf.rawLog = log @@ -126,7 +139,8 @@ define [ text: entry.message } # Get the biber log and parse it too - $http.get "/project/#{$scope.project_id}/output/output.blg" + qs + opts.url = "/project/#{$scope.project_id}/output/output.blg" + $http opts .success (log) -> window._s = $scope biberLogEntries = BibLogParser.parse(log, {}) @@ -189,6 +203,8 @@ define [ $http { url: "/project/#{$scope.project_id}/output" method: "DELETE" + params: + clsiserverid:ide.clsiServerId headers: "X-Csrf-Token": window.csrfToken } @@ -271,6 +287,7 @@ define [ file: path line: row + 1 column: column + clsiserverid:ide.clsiServerId } }) .success (data) -> @@ -298,6 +315,7 @@ define [ page: position.page + 1 h: position.offset.left.toFixed(2) v: position.offset.top.toFixed(2) + clsiserverid:ide.clsiServerId } }) .success (data) -> diff --git a/services/web/public/coffee/ide/wordcount/controllers/WordCountModalController.coffee b/services/web/public/coffee/ide/wordcount/controllers/WordCountModalController.coffee index 3c69d3e276..e880a25eef 100644 --- a/services/web/public/coffee/ide/wordcount/controllers/WordCountModalController.coffee +++ b/services/web/public/coffee/ide/wordcount/controllers/WordCountModalController.coffee @@ -5,11 +5,15 @@ define [ $scope.status = loading:true - $http.get("/project/#{ide.project_id}/wordcount") + opts = + url:"/project/#{ide.project_id}/wordcount" + method:"GET" + params: + clsiserverid:ide.clsiServerId + $http opts .success (data) -> $scope.status.loading = false $scope.data = data.texcount - console.log $scope.data .error () -> $scope.status.error = true From d2cc75b73f5fd3f7f832a1c946aa9400239f0a6a Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Wed, 18 May 2016 14:38:17 +0100 Subject: [PATCH 13/18] changed $scope.pdf.clsiServerId to ide.clsiServerId --- .../web/public/coffee/ide/pdf/controllers/PdfController.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee index 82be226d42..32159750e5 100644 --- a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee +++ b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee @@ -37,7 +37,7 @@ define [ } parseCompileResponse = (response) -> - if response.clsiServerId? and response.clsiServerId != $scope.pdf.clsiServerId + if response.clsiServerId? and response.clsiServerId != ide.clsiServerId ide.clsiServerId = response.clsiServerId # Reset everything From 036b179ffe1473f9cf96f6168689d6707433a001 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Thu, 19 May 2016 13:28:20 +0100 Subject: [PATCH 14/18] put the clsiserverid onto qs object for correct server allocation --- .../ide/pdf/controllers/PdfController.coffee | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee index 478fed3ecc..0ce85686cf 100644 --- a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee +++ b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee @@ -36,9 +36,7 @@ define [ _csrf: window.csrfToken } - parseCompileResponse = (response) -> - if response.clsiServerId? and response.clsiServerId != ide.clsiServerId - ide.clsiServerId = response.clsiServerId + parseCompileResponse = (response) -> # Reset everything $scope.pdf.error = false @@ -72,14 +70,7 @@ define [ else if response.status == "success" $scope.pdf.view = 'pdf' $scope.shouldShowLogs = false - # define the base url - $scope.pdf.url = "/project/#{$scope.project_id}/output/output.pdf?cache_bust=#{Date.now()}" - # add a query string parameter for the compile group - if response.compileGroup? - $scope.pdf.compileGroup = response.compileGroup - $scope.pdf.url = $scope.pdf.url + "&compileGroup=#{$scope.pdf.compileGroup}" - if response.clsiServerId? - $scope.pdf.url = $scope.pdf.url + "&clsiserverid=#{response.clsiServerId}" + # make a cache to look up files by name fileByPath = {} for file in response.outputFiles @@ -98,6 +89,9 @@ define [ if response.compileGroup? $scope.pdf.compileGroup = response.compileGroup qs.compileGroup = "#{$scope.pdf.compileGroup}" + if response.clsiServerId? + qs.clsiserverid = response.clsiServerId + ide.clsiServerId = response.clsiServerId # convert the qs hash into a query string and append it qs_args = ("#{k}=#{v}" for k, v of qs) $scope.pdf.qs = if qs_args.length then "?" + qs_args.join("&") else "" From 789257fd4aa963b75a4950bec54f6f8647617511 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Thu, 19 May 2016 13:45:44 +0100 Subject: [PATCH 15/18] use cookie key when setting cookie for jar --- .../app/coffee/Features/Compile/ClsiCookieManager.coffee | 2 +- .../UnitTests/coffee/Compile/ClsiCookieManagerTests.coffee | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/ClsiCookieManager.coffee b/services/web/app/coffee/Features/Compile/ClsiCookieManager.coffee index 81e2e633e6..3372f9c871 100644 --- a/services/web/app/coffee/Features/Compile/ClsiCookieManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiCookieManager.coffee @@ -56,7 +56,7 @@ module.exports = ClsiCookieManager = if err? logger.err err:err, project_id:project_id, "error getting server id" return callback(err) - serverCookie = request.cookie("clsiserver=#{serverId}") + serverCookie = request.cookie("#{Settings.clsiCookieKey}=#{serverId}") jar = request.jar() jar.setCookie serverCookie, Settings.apis.clsi.url callback(null, jar) diff --git a/services/web/test/UnitTests/coffee/Compile/ClsiCookieManagerTests.coffee b/services/web/test/UnitTests/coffee/Compile/ClsiCookieManagerTests.coffee index 1fc8716657..c2feb2a2c2 100644 --- a/services/web/test/UnitTests/coffee/Compile/ClsiCookieManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Compile/ClsiCookieManagerTests.coffee @@ -112,9 +112,9 @@ describe "ClsiCookieManager", -> @ClsiCookieManager._getServerId = sinon.stub().callsArgWith(1, null, "clsi-11") it "should return a jar with the cookie set populated from redis", (done)-> - @ClsiCookieManager.getCookieJar @project_id, (err, jar)-> - jar._jar.store.idx["clsi.example.com"]["/"].clsiserver.key.should.equal "clsiserver" - jar._jar.store.idx["clsi.example.com"]["/"].clsiserver.value.should.equal "clsi-11" + @ClsiCookieManager.getCookieJar @project_id, (err, jar)=> + jar._jar.store.idx["clsi.example.com"]["/"][@settings.clsiCookieKey].key.should.equal + jar._jar.store.idx["clsi.example.com"]["/"][@settings.clsiCookieKey].value.should.equal "clsi-11" done() From f03a0766697d488e420e0faf649f2f1b9dd36a6e Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Thu, 19 May 2016 16:23:56 +0100 Subject: [PATCH 16/18] make cash bust add onto object not recreate it --- .../web/public/coffee/ide/pdf/controllers/PdfController.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee index 0ce85686cf..4c00ced0b8 100644 --- a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee +++ b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee @@ -84,7 +84,7 @@ define [ # no need to bust cache, build id is unique else $scope.pdf.url = "/project/#{$scope.project_id}/output/output.pdf" - qs = { cache_bust : "#{Date.now()}" } + qs.cache_bust = "#{Date.now()}" # add a query string parameter for the compile group if response.compileGroup? $scope.pdf.compileGroup = response.compileGroup From 61b9a683aa1a0ad0bd0cc3411519ea1ff169b32f Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Thu, 19 May 2016 16:55:58 +0100 Subject: [PATCH 17/18] put clsiCookie vals into subobject in settings --- .../Features/Compile/ClsiCookieManager.coffee | 8 ++++---- .../coffee/Compile/ClsiCookieManagerTests.coffee | 15 ++++++++------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/ClsiCookieManager.coffee b/services/web/app/coffee/Features/Compile/ClsiCookieManager.coffee index 3372f9c871..b6270cfce9 100644 --- a/services/web/app/coffee/Features/Compile/ClsiCookieManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiCookieManager.coffee @@ -8,7 +8,7 @@ logger = require "logger-sharelatex" buildKey = (project_id)-> return "clsiserver:#{project_id}" -clsiCookiesEnabled = Settings.clsiCookieKey? and Settings.clsiCookieKey.length != 0 +clsiCookiesEnabled = Settings.clsiCookie?.key? and Settings.clsiCookie.key.length != 0 module.exports = ClsiCookieManager = @@ -36,7 +36,7 @@ module.exports = ClsiCookieManager = _parseServerIdFromResponse : (response)-> cookies = Cookie.parse(response.headers["set-cookie"]?[0] or "") - return cookies?[Settings.clsiCookieKey] + return cookies?[Settings.clsiCookie.key] setServerId: (project_id, response, callback = (err, serverId)->)-> if !clsiCookiesEnabled @@ -44,7 +44,7 @@ module.exports = ClsiCookieManager = serverId = ClsiCookieManager._parseServerIdFromResponse(response) multi = rclient.multi() multi.set buildKey(project_id), serverId - multi.expire buildKey(project_id), Settings.clsi_cookie_expire_length_seconds + multi.expire buildKey(project_id), Settings.clsiCookie.ttl multi.exec (err)-> callback(err, serverId) @@ -56,7 +56,7 @@ module.exports = ClsiCookieManager = if err? logger.err err:err, project_id:project_id, "error getting server id" return callback(err) - serverCookie = request.cookie("#{Settings.clsiCookieKey}=#{serverId}") + serverCookie = request.cookie("#{Settings.clsiCookie.key}=#{serverId}") jar = request.jar() jar.setCookie serverCookie, Settings.apis.clsi.url callback(null, jar) diff --git a/services/web/test/UnitTests/coffee/Compile/ClsiCookieManagerTests.coffee b/services/web/test/UnitTests/coffee/Compile/ClsiCookieManagerTests.coffee index c2feb2a2c2..01d4ad0002 100644 --- a/services/web/test/UnitTests/coffee/Compile/ClsiCookieManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Compile/ClsiCookieManagerTests.coffee @@ -30,8 +30,9 @@ describe "ClsiCookieManager", -> apis: clsi: url: "http://clsi.example.com" - clsi_cookie_expire_length_seconds: Math.random() - clsiCookieKey: "coooookie" + clsiCookie: + ttl:Math.random() + key: "coooookie" @requires = "redis-sharelatex" : createClient: => @@ -90,7 +91,7 @@ describe "ClsiCookieManager", -> it "should set the server id with a ttl", (done)-> @ClsiCookieManager.setServerId @project_id, @response, (err)=> @redisMulti.set.calledWith("clsiserver:#{@project_id}", "clsi-8").should.equal true - @redisMulti.expire.calledWith("clsiserver:#{@project_id}", @settings.clsi_cookie_expire_length_seconds).should.equal true + @redisMulti.expire.calledWith("clsiserver:#{@project_id}", @settings.clsiCookie.ttl).should.equal true done() it "should return the server id", (done)-> @@ -100,7 +101,7 @@ describe "ClsiCookieManager", -> it "should not set the server id if clsiCookies are not enabled", (done)-> - delete @settings.clsiCookieKey + delete @settings.clsiCookie.key @ClsiCookieManager = SandboxedModule.require modulePath, requires:@requires @ClsiCookieManager.setServerId @project_id, @response, (err, serverId)=> @redisMulti.exec.called.should.equal false @@ -113,13 +114,13 @@ describe "ClsiCookieManager", -> it "should return a jar with the cookie set populated from redis", (done)-> @ClsiCookieManager.getCookieJar @project_id, (err, jar)=> - jar._jar.store.idx["clsi.example.com"]["/"][@settings.clsiCookieKey].key.should.equal - jar._jar.store.idx["clsi.example.com"]["/"][@settings.clsiCookieKey].value.should.equal "clsi-11" + jar._jar.store.idx["clsi.example.com"]["/"][@settings.clsiCookie.key].key.should.equal + jar._jar.store.idx["clsi.example.com"]["/"][@settings.clsiCookie.key].value.should.equal "clsi-11" done() it "should return empty cookie jar if clsiCookies are not enabled", (done)-> - delete @settings.clsiCookieKey + delete @settings.clsiCookie.key @ClsiCookieManager = SandboxedModule.require modulePath, requires:@requires @ClsiCookieManager.getCookieJar @project_id, (err, jar)-> assert.deepEqual jar, realRequst.jar() From c918028d14beac7343040460ed9fa947e6fd148e Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Fri, 20 May 2016 12:46:14 +0100 Subject: [PATCH 18/18] removed clsi priorty url --- .../Features/Compile/ClsiManager.coffee | 7 ++----- services/web/config/settings.defaults.coffee | 2 -- .../coffee/Compile/ClsiManagerTests.coffee | 20 ------------------- 3 files changed, 2 insertions(+), 27 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/ClsiManager.coffee b/services/web/app/coffee/Features/Compile/ClsiManager.coffee index 3285026cfa..ef347a17e0 100755 --- a/services/web/app/coffee/Features/Compile/ClsiManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiManager.coffee @@ -53,13 +53,10 @@ module.exports = ClsiManager = _getCompilerUrl: (compileGroup) -> - if compileGroup == "priority" - return Settings.apis.clsi_priority.url - else - return Settings.apis.clsi.url + return Settings.apis.clsi.url _postToClsi: (project_id, req, compileGroup, callback = (error, response) ->) -> - compilerUrl = @_getCompilerUrl(compileGroup) + compilerUrl = Settings.apis.clsi.url opts = url: "#{compilerUrl}/project/#{project_id}/compile" json: req diff --git a/services/web/config/settings.defaults.coffee b/services/web/config/settings.defaults.coffee index 026c451d0c..beb933e4c9 100644 --- a/services/web/config/settings.defaults.coffee +++ b/services/web/config/settings.defaults.coffee @@ -88,8 +88,6 @@ module.exports = url: "http://localhost:3009" clsi: url: "http://localhost:3013" - clsi_priority: - url: "http://localhost:3013" templates: url: "http://localhost:3007" githubSync: diff --git a/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee b/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee index 89f312441d..9889bcb87b 100644 --- a/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee @@ -101,14 +101,6 @@ describe "ClsiManager", -> it "should call the callback", -> @callback.called.should.equal true - describe "with the priority compileGroup", -> - beforeEach -> - @ClsiManager.deleteAuxFiles @project_id, {compileGroup: "priority"}, @callback - - it "should call the delete method in the CLSI", -> - @ClsiManager._makeRequest - .calledWith(@project_id, { method:"DELETE", url:"#{@settings.apis.clsi_priority.url}/project/#{@project_id}"}) - .should.equal true describe "_buildRequest", -> beforeEach -> @@ -264,18 +256,6 @@ describe "ClsiManager", -> it "should call the callback with the body and the error", -> @callback.calledWith(new Error("CLSI returned non-success code: 500"), @body).should.equal true - describe "when the compiler is priority", -> - beforeEach -> - @ClsiManager._makeRequest = sinon.stub() - @ClsiManager._postToClsi @project_id, @req, "priority", @callback - - it "should use the clsi_priority url", -> - url = "#{@settings.apis.clsi_priority.url}/project/#{@project_id}/compile" - @ClsiManager._makeRequest.calledWith(@project_id, { - method: "POST", - url: url - json: @req - }).should.equal true describe "wordCount", -> beforeEach ->