From 18560d862162290479ea34f78c63341286697517 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Wed, 20 Apr 2016 16:17:06 +0100 Subject: [PATCH] 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() + + + + + + + + + + + + + + + + + + + +