From 9d7f129b61f29f908d320d35232eb16c245718ee Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 31 May 2016 16:20:24 +0100 Subject: [PATCH 1/6] add per-user compilation in server when no user is defined, fallback to per-project compilation --- .../Features/Compile/ClsiManager.coffee | 38 +++++------ .../Features/Compile/CompileController.coffee | 64 ++++++++++++++----- .../Features/Compile/CompileManager.coffee | 11 ++-- services/web/app/coffee/router.coffee | 13 ++++ 4 files changed, 86 insertions(+), 40 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/ClsiManager.coffee b/services/web/app/coffee/Features/Compile/ClsiManager.coffee index f874573d6f..f04c568bd6 100755 --- a/services/web/app/coffee/Features/Compile/ClsiManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiManager.coffee @@ -11,11 +11,11 @@ ClsiCookieManager = require("./ClsiCookieManager") module.exports = ClsiManager = - sendRequest: (project_id, options = {}, callback = (error, success) ->) -> + sendRequest: (project_id, user_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) -> + ClsiManager._postToClsi project_id, user_id, req, options.compileGroup, (error, response) -> if error? logger.err err:error, project_id:project_id, "error sending request to clsi" return callback(error) @@ -27,14 +27,13 @@ module.exports = ClsiManager = outputFiles = ClsiManager._parseOutputFiles(project_id, response?.compile?.outputFiles) callback(null, response?.compile?.status, outputFiles, clsiServerId) - deleteAuxFiles: (project_id, options, callback = (error) ->) -> - compilerUrl = @_getCompilerUrl(options?.compileGroup) + deleteAuxFiles: (project_id, user_id, options, callback = (error) ->) -> + compilerUrl = @_getCompilerUrl(options?.compileGroup, project_id, user_id) opts = - url:"#{compilerUrl}/project/#{project_id}" + url:compilerUrl method:"DELETE" ClsiManager._makeRequest project_id, opts, callback - _makeRequest: (project_id, opts, callback)-> ClsiCookieManager.getCookieJar project_id, (err, jar)-> if err? @@ -51,14 +50,17 @@ module.exports = ClsiManager = return callback err, response, body + _getCompilerUrl: (compileGroup, project_id, user_id, action) -> + host = Settings.apis.clsi.url + path = "/project/#{project_id}" + path += "/user/#{user_id}" if user_id? + path += "/#{action}" if action? + return "#{host}#{path}" - _getCompilerUrl: (compileGroup) -> - return Settings.apis.clsi.url - - _postToClsi: (project_id, req, compileGroup, callback = (error, response) ->) -> - compilerUrl = Settings.apis.clsi.url - opts = - url: "#{compilerUrl}/project/#{project_id}/compile" + _postToClsi: (project_id, user_id, req, compileGroup, callback = (error, response) ->) -> + compileUrl = @_getCompilerUrl(compileGroup, project_id, user_id, "compile") + opts = + url: compileUrl json: req method: "POST" ClsiManager._makeRequest project_id, opts, (error, response, body) -> @@ -133,15 +135,15 @@ module.exports = ClsiManager = resources: resources } - wordCount: (project_id, file, options, callback = (error, response) ->) -> + wordCount: (project_id, user_id, file, options, callback = (error, response) ->) -> ClsiManager._buildRequest project_id, options, (error, req) -> - compilerUrl = ClsiManager._getCompilerUrl(options?.compileGroup) filename = file || req?.compile?.rootResourcePath - wordcount_url = "#{compilerUrl}/project/#{project_id}/wordcount?file=#{encodeURIComponent(filename)}" - if req.compile.options.imageName? - wordcount_url += "&image=#{encodeURIComponent(req.compile.options.imageName)}" + wordcount_url = ClsiManager._getCompilerUrl(options?.compileGroup, project_id, user_id, "wordcount") opts = url: wordcount_url + qs: + file: filename + image: req.compile.options.imageName method: "GET" ClsiManager._makeRequest project_id, opts, (error, response, body) -> return callback(error) if error? diff --git a/services/web/app/coffee/Features/Compile/CompileController.coffee b/services/web/app/coffee/Features/Compile/CompileController.coffee index b50c6ffd1a..830af98115 100755 --- a/services/web/app/coffee/Features/Compile/CompileController.coffee +++ b/services/web/app/coffee/Features/Compile/CompileController.coffee @@ -29,7 +29,9 @@ module.exports = CompileController = options.compiler = req.body.compiler if req.body?.draft options.draft = req.body.draft - logger.log {options, project_id}, "got compile request" + if req.query?.isolated is "true" + options.isolated = true + logger.log {options:options, project_id:project_id, user_id:user_id}, "got compile request" CompileManager.compile project_id, user_id, options, (error, status, outputFiles, clsiServerId, limits) -> return next(error) if error? res.contentType("application/json") @@ -40,6 +42,13 @@ module.exports = CompileController = clsiServerId:clsiServerId } + _compileAsUser: (req, callback) -> + isolated = req.query?.isolated is "true" + if isolated + AuthenticationController.getLoggedInUserId req, callback + else + callback() + downloadPdf: (req, res, next = (error) ->)-> Metrics.inc "pdf-downloads" project_id = req.params.Project_id @@ -76,28 +85,40 @@ module.exports = CompileController = deleteAuxFiles: (req, res, next) -> project_id = req.params.Project_id - CompileManager.deleteAuxFiles project_id, (error) -> + CompileController._compileAsUser req, (error, user_id) -> return next(error) if error? - res.sendStatus(200) + CompileManager.deleteAuxFiles project_id, user_id, (error) -> + return next(error) if error? + res.sendStatus(200) compileAndDownloadPdf: (req, res, next)-> project_id = req.params.project_id - CompileManager.compile project_id, null, {}, (err)-> - if err? - logger.err err:err, project_id:project_id, "something went wrong compile and downloading pdf" - res.sendStatus 500 - url = "/project/#{project_id}/output/output.pdf" - CompileController.proxyToClsi project_id, url, req, res, next + CompileController._compileAsUser req, (error, user_id) -> + return next(error) if error? + CompileManager.compile project_id, user_id, {}, (err)-> + if err? + logger.err err:err, project_id:project_id, "something went wrong compile and downloading pdf" + res.sendStatus 500 + url = "/project/#{project_id}/output/output.pdf" + CompileController.proxyToClsi project_id, user_id, url, req, res, next getFileFromClsi: (req, res, next = (error) ->) -> project_id = req.params.Project_id build = req.params.build - if build? + user = req.params.user + if user? and build? + url = "/project/#{project_id}/user/#{user}/build/#{build}/output/#{req.params.file}" + else if build? url = "/project/#{project_id}/build/#{build}/output/#{req.params.file}" else url = "/project/#{project_id}/output/#{req.params.file}" CompileController.proxyToClsi(project_id, url, req, res, next) + _getUrl: (project_id, user_id, action) -> + path = "/project/#{project_id}" + path += "/user/#{user_id}" if user_id? + return "#{path}/#{action}" + proxySyncPdf: (req, res, next = (error) ->) -> project_id = req.params.Project_id {page, h, v} = req.query @@ -107,8 +128,12 @@ module.exports = CompileController = return next(new Error("invalid h parameter")) if not v?.match(/^\d+\.\d+$/) return next(new Error("invalid v parameter")) - destination = {url: "/project/#{project_id}/sync/pdf", qs: {page, h, v}} - CompileController.proxyToClsi(project_id, destination, req, res, next) + url = CompileController._getUrl(project_id, user_id, "sync/pdf") + destination = {url: url, qs: {page, h, v}} + # whether this request is going to a per-user container + CompileController._compileAsUser req, (error, user_id) -> + return next(error) if error? + CompileController.proxyToClsi(project_id, destination, req, res, next) proxySyncCode: (req, res, next = (error) ->) -> project_id = req.params.Project_id @@ -119,8 +144,11 @@ module.exports = CompileController = return next(new Error("invalid line parameter")) if not column?.match(/^\d+$/) return next(new Error("invalid column parameter")) - destination = {url:"/project/#{project_id}/sync/code", qs: {file, line, column}} - CompileController.proxyToClsi(project_id, destination, req, res, next) + url = CompileController._getUrl(project_id, user_id, "sync/code") + destination = {url:url, qs: {file, line, column}} + CompileController._compileAsUser req, (error, user_id) -> + return next(error) if error? + CompileController.proxyToClsi(project_id, destination, req, res, next) proxyToClsi: (project_id, url, req, res, next = (error) ->) -> if req.query?.compileGroup @@ -168,7 +196,9 @@ module.exports = CompileController = wordCount: (req, res, next) -> project_id = req.params.Project_id file = req.query.file || false - CompileManager.wordCount project_id, file, (error, body) -> + CompileController._compileAsUser req, (error, user_id) -> return next(error) if error? - res.contentType("application/json") - res.send body + CompileManager.wordCount project_id, user_id, file, (error, body) -> + return next(error) if error? + res.contentType("application/json") + res.send body diff --git a/services/web/app/coffee/Features/Compile/CompileManager.coffee b/services/web/app/coffee/Features/Compile/CompileManager.coffee index 0d8d480b7b..b552b1a7c7 100755 --- a/services/web/app/coffee/Features/Compile/CompileManager.coffee +++ b/services/web/app/coffee/Features/Compile/CompileManager.coffee @@ -37,15 +37,16 @@ module.exports = CompileManager = return callback(error) if error? for key, value of limits options[key] = value - ClsiManager.sendRequest project_id, options, (error, status, outputFiles, clsiServerId) -> + user_id = undefined if not options.isolated + ClsiManager.sendRequest project_id, user_id, options, (error, status, outputFiles, clsiServerId) -> return callback(error) if error? logger.log files: outputFiles, "output files" callback(null, status, outputFiles, clsiServerId, limits) - deleteAuxFiles: (project_id, callback = (error) ->) -> + deleteAuxFiles: (project_id, user_id, callback = (error) ->) -> CompileManager.getProjectCompileLimits project_id, (error, limits) -> return callback(error) if error? - ClsiManager.deleteAuxFiles project_id, limits, callback + ClsiManager.deleteAuxFiles project_id, user_id, limits, callback getProjectCompileLimits: (project_id, callback = (error, limits) ->) -> Project.findById project_id, {owner_ref: 1}, (error, project) -> @@ -92,7 +93,7 @@ module.exports = CompileManager = else ProjectRootDocManager.setRootDocAutomatically project_id, callback - wordCount: (project_id, file, callback = (error) ->) -> + wordCount: (project_id, user_id, file, callback = (error) ->) -> CompileManager.getProjectCompileLimits project_id, (error, limits) -> return callback(error) if error? - ClsiManager.wordCount project_id, file, limits, callback + ClsiManager.wordCount project_id, user_id, file, limits, callback diff --git a/services/web/app/coffee/router.coffee b/services/web/app/coffee/router.coffee index 80c52c5a24..b281b019ff 100644 --- a/services/web/app/coffee/router.coffee +++ b/services/web/app/coffee/router.coffee @@ -124,6 +124,19 @@ module.exports = class Router next() ), AuthorizationMiddlewear.ensureUserCanReadProject, CompileController.getFileFromClsi + # direct url access to output files for a specific user and build (query string not required) + webRouter.get /^\/project\/([^\/]*)\/user\/([0-9a-f]+)\/build\/([0-9a-f-]+)\/output\/(.*)$/, + ((req, res, next) -> + params = + "Project_id": req.params[0] + "user": req.params[1] + "build": req.params[2] + "file": req.params[3] + req.params = params + next() + ), AuthorizationMiddlewear.ensureUserCanReadProject, CompileController.getFileFromClsi + + webRouter.delete "/project/:Project_id/output", AuthorizationMiddlewear.ensureUserCanReadProject, CompileController.deleteAuxFiles webRouter.get "/project/:Project_id/sync/code", AuthorizationMiddlewear.ensureUserCanReadProject, CompileController.proxySyncCode webRouter.get "/project/:Project_id/sync/pdf", AuthorizationMiddlewear.ensureUserCanReadProject, CompileController.proxySyncPdf From 272625fbcc22c322f65f46d49b2d3fc68af9322c Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 31 May 2016 16:20:46 +0100 Subject: [PATCH 2/6] add per-user compilation in client enabled only when query string includes isolated=true --- .../coffee/ide/pdf/controllers/PdfController.coffee | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee index 05d99124c7..7ed42b6e1f 100644 --- a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee +++ b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee @@ -5,6 +5,8 @@ define [ ], (App, LogParser, BibLogParser) -> App.controller "PdfController", ($scope, $http, ide, $modal, synctex, event_tracking, localStorage) -> + # enable per-user containers if querystring includes isolated=true + perUserCompile = window.location?.search?.match(/isolated=true/)? or undefined autoCompile = true # pdf.view = uncompiled | pdf | errors @@ -28,13 +30,16 @@ define [ sendCompileRequest = (options = {}) -> url = "/project/#{$scope.project_id}/compile" + params = {} if options.isAutoCompile - url += "?auto_compile=true" + params["auto_compile"]=true + if perUserCompile # send ?isolated=true for per-user compiles + params["isolated"] = true return $http.post url, { rootDoc_id: options.rootDocOverride_id or null draft: $scope.draft _csrf: window.csrfToken - } + }, {params: params} parseCompileResponse = (response) -> @@ -241,6 +246,7 @@ define [ method: "DELETE" params: clsiserverid:ide.clsiServerId + isolated: perUserCompile headers: "X-Csrf-Token": window.csrfToken } @@ -324,6 +330,7 @@ define [ line: row + 1 column: column clsiserverid:ide.clsiServerId + isolated: perUserCompile } }) .success (data) -> @@ -352,6 +359,7 @@ define [ h: position.offset.left.toFixed(2) v: position.offset.top.toFixed(2) clsiserverid:ide.clsiServerId + isolated: perUserCompile } }) .success (data) -> From ce1524cd07fe0b341364b0517080dee7df9607df Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 31 May 2016 16:21:07 +0100 Subject: [PATCH 3/6] remove unnecessary build id in query string this is now in the url --- .../web/public/coffee/ide/pdf/controllers/PdfController.coffee | 1 - 1 file changed, 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 7ed42b6e1f..a2aa5e27e5 100644 --- a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee +++ b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee @@ -132,7 +132,6 @@ define [ opts = method:"GET" params: - build:file.build clsiserverid:ide.clsiServerId if file.url? # FIXME clean this up when we have file.urls out consistently opts.url = file.url From 55a8e3cffe2a3a58f5e9df47212b618290706912 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 31 May 2016 16:54:28 +0100 Subject: [PATCH 4/6] extend tests for per-user compiles --- .../coffee/Compile/ClsiManagerTests.coffee | 44 +++++++++---------- .../Compile/CompileControllerTests.coffee | 16 ++++--- .../coffee/Compile/CompileManagerTests.coffee | 16 +++---- 3 files changed, 41 insertions(+), 35 deletions(-) diff --git a/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee b/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee index b5dce15e8e..50f598c96d 100644 --- a/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee @@ -28,6 +28,7 @@ describe "ClsiManager", -> "logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub(), warn: sinon.stub() } "request": @request = sinon.stub() @project_id = "project-id" + @user_id = "user-id" @callback = sinon.stub() describe "sendRequest", -> @@ -37,22 +38,22 @@ describe "ClsiManager", -> describe "with a successful compile", -> beforeEach -> - @ClsiManager._postToClsi = sinon.stub().callsArgWith(3, null, { + @ClsiManager._postToClsi = sinon.stub().callsArgWith(4, null, { compile: status: @status = "success" outputFiles: [{ - url: "#{@settings.apis.clsi.url}/project/#{@project_id}/build/1234/output/output.pdf" + url: "#{@settings.apis.clsi.url}/project/#{@project_id}/user/#{@user_id}/build/1234/output/output.pdf" path: "output.pdf" type: "pdf" build: 1234 },{ - url: "#{@settings.apis.clsi.url}/project/#{@project_id}/build/1234/output/output.log" + url: "#{@settings.apis.clsi.url}/project/#{@project_id}/user/#{@user_id}/build/1234/output/output.log" path: "output.log" type: "log" build: 1234 }] }) - @ClsiManager.sendRequest @project_id, {compileGroup:"standard"}, @callback + @ClsiManager.sendRequest @project_id, @user_id, {compileGroup:"standard"}, @callback it "should build the request", -> @ClsiManager._buildRequest @@ -61,17 +62,17 @@ describe "ClsiManager", -> it "should send the request to the CLSI", -> @ClsiManager._postToClsi - .calledWith(@project_id, @request, "standard") + .calledWith(@project_id, @user_id, @request, "standard") .should.equal true it "should call the callback with the status and output files", -> outputFiles = [{ - url: "/project/#{@project_id}/build/1234/output/output.pdf" + url: "/project/#{@project_id}/user/#{@user_id}/build/1234/output/output.pdf" path: "output.pdf" type: "pdf" build: 1234 },{ - url: "/project/#{@project_id}/build/1234/output/output.log" + url: "/project/#{@project_id}/user/#{@user_id}/build/1234/output/output.log" path: "output.log" type: "log" build: 1234 @@ -80,11 +81,11 @@ describe "ClsiManager", -> describe "with a failed compile", -> beforeEach -> - @ClsiManager._postToClsi = sinon.stub().callsArgWith(3, null, { + @ClsiManager._postToClsi = sinon.stub().callsArgWith(4, null, { compile: status: @status = "failure" }) - @ClsiManager.sendRequest @project_id, {}, @callback + @ClsiManager.sendRequest @project_id, @user_id, {}, @callback it "should call the callback with a failure statue", -> @callback.calledWith(null, @status).should.equal true @@ -95,11 +96,11 @@ describe "ClsiManager", -> describe "with the standard compileGroup", -> beforeEach -> - @ClsiManager.deleteAuxFiles @project_id, {compileGroup: "standard"}, @callback + @ClsiManager.deleteAuxFiles @project_id, @user_id, {compileGroup: "standard"}, @callback it "should call the delete method in the standard CLSI", -> @ClsiManager._makeRequest - .calledWith(@project_id, { method:"DELETE", url:"#{@settings.apis.clsi.url}/project/#{@project_id}"}) + .calledWith(@project_id, { method:"DELETE", url:"#{@settings.apis.clsi.url}/project/#{@project_id}/user/#{@user_id}"}) .should.equal true it "should call the callback", -> @@ -239,10 +240,10 @@ describe "ClsiManager", -> describe "successfully", -> beforeEach -> @ClsiManager._makeRequest = sinon.stub().callsArgWith(2, null, {statusCode: 204}, @body = { mock: "foo" }) - @ClsiManager._postToClsi @project_id, @req, "standard", @callback + @ClsiManager._postToClsi @project_id, @user_id, @req, "standard", @callback it 'should send the request to the CLSI', -> - url = "#{@settings.apis.clsi.url}/project/#{@project_id}/compile" + url = "#{@settings.apis.clsi.url}/project/#{@project_id}/user/#{@user_id}/compile" @ClsiManager._makeRequest.calledWith(@project_id, { method: "POST", url: url @@ -255,7 +256,7 @@ describe "ClsiManager", -> describe "when the CLSI returns an error", -> beforeEach -> @ClsiManager._makeRequest = sinon.stub().callsArgWith(2, null, {statusCode: 500}, @body = { mock: "foo" }) - @ClsiManager._postToClsi @project_id, @req, "standard", @callback + @ClsiManager._postToClsi @project_id, @user_id, @req, "standard", @callback 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 @@ -265,37 +266,36 @@ describe "ClsiManager", -> beforeEach -> @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" describe "with root file", -> beforeEach -> - @ClsiManager.wordCount @project_id, false, {}, @callback + @ClsiManager.wordCount @project_id, @user_id, false, {}, @callback it "should call wordCount with root file", -> @ClsiManager._makeRequest - .calledWith(@project_id, { method: "GET", url: "compiler.url/project/#{@project_id}/wordcount?file=rootfile.text" }) - .should.equal true + .calledWith(@project_id, {method: "GET", url: "http://clsi.example.com/project/#{@project_id}/user/#{@user_id}/wordcount", qs: {file: "rootfile.text",image:undefined}}) + .should.equal true it "should call the callback", -> @callback.called.should.equal true describe "with param file", -> beforeEach -> - @ClsiManager.wordCount @project_id, "main.tex", {}, @callback + @ClsiManager.wordCount @project_id, @user_id, "main.tex", {}, @callback it "should call wordCount with param file", -> @ClsiManager._makeRequest - .calledWith(@project_id, { method: "GET", url: "compiler.url/project/#{@project_id}/wordcount?file=main.tex" }) + .calledWith(@project_id, { method: "GET", url: "http://clsi.example.com/project/#{@project_id}/user/#{@user_id}/wordcount", qs:{file:"main.tex",image:undefined}}) .should.equal true describe "with image", -> beforeEach -> @req.compile.options.imageName = @image = "example.com/mock/image" - @ClsiManager.wordCount @project_id, "main.tex", {}, @callback + @ClsiManager.wordCount @project_id, @user_id, "main.tex", {}, @callback it "should call wordCount with file and image", -> @ClsiManager._makeRequest - .calledWith(@project_id, { method: "GET", url: "compiler.url/project/#{@project_id}/wordcount?file=main.tex&image=#{encodeURIComponent(@image)}" }) + .calledWith(@project_id, { method: "GET", url: "http://clsi.example.com/project/#{@project_id}/user/#{@user_id}/wordcount", qs:{file:"main.tex",image:@image}}) .should.equal true diff --git a/services/web/test/UnitTests/coffee/Compile/CompileControllerTests.coffee b/services/web/test/UnitTests/coffee/Compile/CompileControllerTests.coffee index 6bae5803c4..097a534262 100644 --- a/services/web/test/UnitTests/coffee/Compile/CompileControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Compile/CompileControllerTests.coffee @@ -45,13 +45,13 @@ describe "CompileController", -> @next = sinon.stub() @req = new MockRequest() @res = new MockResponse() + @AuthenticationController.getLoggedInUserId = sinon.stub().callsArgWith(1, null, @user_id = "mock-user-id") describe "compile", -> beforeEach -> @req.params = Project_id: @project_id @req.session = {} - @AuthenticationController.getLoggedInUserId = sinon.stub().callsArgWith(1, null, @user_id = "mock-user-id") @CompileManager.compile = sinon.stub().callsArgWith(3, null, @status = "success", @outputFiles = ["mock-output-files"]) describe "when not an auto compile", -> @@ -359,7 +359,7 @@ describe "CompileController", -> describe "deleteAuxFiles", -> beforeEach -> - @CompileManager.deleteAuxFiles = sinon.stub().callsArg(1) + @CompileManager.deleteAuxFiles = sinon.stub().callsArg(2) @req.params = Project_id: @project_id @res.sendStatus = sinon.stub() @@ -380,6 +380,8 @@ describe "CompileController", -> @req = params: project_id:@project_id + query: + isolated: "true" @CompileManager.compile.callsArgWith(3) @CompileController.proxyToClsi = sinon.stub() @res = @@ -392,21 +394,25 @@ describe "CompileController", -> it "should proxy the res to the clsi with correct url", (done)-> @CompileController.compileAndDownloadPdf @req, @res - @CompileController.proxyToClsi.calledWith(@project_id, "/project/#{@project_id}/output/output.pdf", @req, @res).should.equal true + sinon.assert.calledWith @CompileController.proxyToClsi, @project_id, @user_id, "/project/#{@project_id}/output/output.pdf", @req, @res + + @CompileController.proxyToClsi.calledWith(@project_id, @user_id, "/project/#{@project_id}/output/output.pdf", @req, @res).should.equal true done() describe "wordCount", -> beforeEach -> - @CompileManager.wordCount = sinon.stub().callsArgWith(2, null, {content:"body"}) + @CompileManager.wordCount = sinon.stub().callsArgWith(3, null, {content:"body"}) @req.params = Project_id: @project_id + @req.query = + isolated: "true" @res.send = sinon.stub() @res.contentType = sinon.stub() @CompileController.wordCount @req, @res, @next it "should proxy to the CLSI", -> @CompileManager.wordCount - .calledWith(@project_id, false) + .calledWith(@project_id, @user_id, false) .should.equal true it "should return a 200 and body", -> diff --git a/services/web/test/UnitTests/coffee/Compile/CompileManagerTests.coffee b/services/web/test/UnitTests/coffee/Compile/CompileManagerTests.coffee index 814b4f258b..82a4ab734b 100644 --- a/services/web/test/UnitTests/coffee/Compile/CompileManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Compile/CompileManagerTests.coffee @@ -42,7 +42,7 @@ describe "CompileManager", -> @CompileManager._ensureRootDocumentIsSet = sinon.stub().callsArgWith(1, null) @DocumentUpdaterHandler.flushProjectToMongo = sinon.stub().callsArgWith(1, null) @CompileManager.getProjectCompileLimits = sinon.stub().callsArgWith(1, null, @limits) - @ClsiManager.sendRequest = sinon.stub().callsArgWith(2, null, @status = "mock-status", @outputFiles = "mock output files", @output = "mock output") + @ClsiManager.sendRequest = sinon.stub().callsArgWith(3, null, @status = "mock-status", @outputFiles = "mock output files", @output = "mock output") describe "succesfully", -> beforeEach -> @@ -71,7 +71,7 @@ describe "CompileManager", -> it "should run the compile with the compile limits", -> @ClsiManager.sendRequest - .calledWith(@project_id, { + .calledWith(@project_id, undefined, { timeout: @limits.timeout }) .should.equal true @@ -135,8 +135,8 @@ describe "CompileManager", -> describe "deleteAuxFiles", -> beforeEach -> @CompileManager.getProjectCompileLimits = sinon.stub().callsArgWith 1, null, @limits = { compileGroup: "mock-compile-group" } - @ClsiManager.deleteAuxFiles = sinon.stub().callsArg(2) - @CompileManager.deleteAuxFiles @project_id, @callback + @ClsiManager.deleteAuxFiles = sinon.stub().callsArg(3) + @CompileManager.deleteAuxFiles @project_id, @user_id, @callback it "should look up the compile group to use", -> @CompileManager.getProjectCompileLimits @@ -145,7 +145,7 @@ describe "CompileManager", -> it "should delete the aux files", -> @ClsiManager.deleteAuxFiles - .calledWith(@project_id, @limits) + .calledWith(@project_id, @user_id, @limits) .should.equal true it "should call the callback", -> @@ -260,8 +260,8 @@ describe "CompileManager", -> describe "wordCount", -> beforeEach -> @CompileManager.getProjectCompileLimits = sinon.stub().callsArgWith 1, null, @limits = { compileGroup: "mock-compile-group" } - @ClsiManager.wordCount = sinon.stub().callsArg(3) - @CompileManager.wordCount @project_id, false, @callback + @ClsiManager.wordCount = sinon.stub().callsArg(4) + @CompileManager.wordCount @project_id, @user_id, false, @callback it "should look up the compile group to use", -> @CompileManager.getProjectCompileLimits @@ -270,7 +270,7 @@ describe "CompileManager", -> it "should call wordCount for project", -> @ClsiManager.wordCount - .calledWith(@project_id, false, @limits) + .calledWith(@project_id, @user_id, false, @limits) .should.equal true it "should call the callback", -> From 634c7745921fbd229e01066ca04e292da7274bba Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 2 Jun 2016 16:54:46 +0100 Subject: [PATCH 5/6] make code clearer when compilation is per-user --- .../web/app/coffee/Features/Compile/CompileManager.coffee | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/CompileManager.coffee b/services/web/app/coffee/Features/Compile/CompileManager.coffee index b552b1a7c7..08a4cdea3d 100755 --- a/services/web/app/coffee/Features/Compile/CompileManager.coffee +++ b/services/web/app/coffee/Features/Compile/CompileManager.coffee @@ -37,8 +37,9 @@ module.exports = CompileManager = return callback(error) if error? for key, value of limits options[key] = value - user_id = undefined if not options.isolated - ClsiManager.sendRequest project_id, user_id, options, (error, status, outputFiles, clsiServerId) -> + # only pass user_id down to clsi if this is a per-user compile + compileAsUser = if options.isolated then user_id else undefined + ClsiManager.sendRequest project_id, compileAsUser, options, (error, status, outputFiles, clsiServerId) -> return callback(error) if error? logger.log files: outputFiles, "output files" callback(null, status, outputFiles, clsiServerId, limits) From e6dcce21fdf0785bee861f6ee2d7495742ff7e20 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 2 Jun 2016 17:03:07 +0100 Subject: [PATCH 6/6] use build_id and user_id instead of build and user --- .../coffee/Features/Compile/CompileController.coffee | 12 ++++++------ services/web/app/coffee/router.coffee | 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/CompileController.coffee b/services/web/app/coffee/Features/Compile/CompileController.coffee index 830af98115..d29888d739 100755 --- a/services/web/app/coffee/Features/Compile/CompileController.coffee +++ b/services/web/app/coffee/Features/Compile/CompileController.coffee @@ -104,12 +104,12 @@ module.exports = CompileController = getFileFromClsi: (req, res, next = (error) ->) -> project_id = req.params.Project_id - build = req.params.build - user = req.params.user - if user? and build? - url = "/project/#{project_id}/user/#{user}/build/#{build}/output/#{req.params.file}" - else if build? - url = "/project/#{project_id}/build/#{build}/output/#{req.params.file}" + build_id = req.params.build_id + user_id = req.params.user_id + if user_id? and build_id? + url = "/project/#{project_id}/user/#{user_id}/build/#{build_id}/output/#{req.params.file}" + else if build_id? + url = "/project/#{project_id}/build/#{build_id}/output/#{req.params.file}" else url = "/project/#{project_id}/output/#{req.params.file}" CompileController.proxyToClsi(project_id, url, req, res, next) diff --git a/services/web/app/coffee/router.coffee b/services/web/app/coffee/router.coffee index b281b019ff..38d0413555 100644 --- a/services/web/app/coffee/router.coffee +++ b/services/web/app/coffee/router.coffee @@ -118,7 +118,7 @@ module.exports = class Router ((req, res, next) -> params = "Project_id": req.params[0] - "build": req.params[1] + "build_id": req.params[1] "file": req.params[2] req.params = params next() @@ -129,8 +129,8 @@ module.exports = class Router ((req, res, next) -> params = "Project_id": req.params[0] - "user": req.params[1] - "build": req.params[2] + "user_id": req.params[1] + "build_id": req.params[2] "file": req.params[3] req.params = params next()