From 024663144317e7351a0f7f793cbe5b2045e4aa5a Mon Sep 17 00:00:00 2001 From: Michael Mazour Date: Wed, 13 Jun 2018 15:30:46 +0100 Subject: [PATCH] Add public API endpoints to reach CLSIs - `/api/clsi/compile/:submission_id` - `/api/clsi/compile/:submission_id/build/:build_id/output/:file` Also per review: - DRY up ClsiManager.sendRequestOnce and ClsiManager.sendExternalRequest - Include submission_id in a log message - Don't include timeout in limits when getting file --- .../Features/Compile/ClsiManager.coffee | 51 +++++++---- .../Features/Compile/CompileController.coffee | 33 +++++++ services/web/app/coffee/router.coffee | 16 ++++ .../acceptance/coffee/ApiClsiTests.coffee | 88 ++++++++++++++++++ .../coffee/helpers/MockClsiApi.coffee | 39 ++++++++ .../coffee/Compile/ClsiManagerTests.coffee | 87 ++++++++++++++++-- .../Compile/CompileControllerTests.coffee | 91 +++++++++++++++++++ 7 files changed, 378 insertions(+), 27 deletions(-) create mode 100644 services/web/test/acceptance/coffee/ApiClsiTests.coffee create mode 100644 services/web/test/acceptance/coffee/helpers/MockClsiApi.coffee diff --git a/services/web/app/coffee/Features/Compile/ClsiManager.coffee b/services/web/app/coffee/Features/Compile/ClsiManager.coffee index c27bc2fe90..e5158cdb9b 100755 --- a/services/web/app/coffee/Features/Compile/ClsiManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiManager.coffee @@ -14,6 +14,7 @@ async = require("async") ClsiFormatChecker = require("./ClsiFormatChecker") DocumentUpdaterHandler = require "../DocumentUpdater/DocumentUpdaterHandler" Metrics = require('metrics-sharelatex') +Errors = require ('../Errors/Errors') module.exports = ClsiManager = @@ -35,24 +36,16 @@ module.exports = ClsiManager = else return callback(error) logger.log project_id: project_id, "sending compile to CLSI" - ClsiFormatChecker.checkRecoursesForProblems req.compile?.resources, (err, validationProblems)-> - if err? - logger.err err, project_id, "could not check resources for potential problems before sending to clsi" - return callback(err) - if validationProblems? - logger.log project_id:project_id, validationProblems:validationProblems, "problems with users latex before compile was attempted" - return callback(null, "validation-problems", null, null, validationProblems) - 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) - logger.log project_id: project_id, outputFilesLength: response?.outputFiles?.length, status: response?.status, compile_status: response?.compile?.status, "received compile response from CLSI" - 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) - callback(null, response?.compile?.status, outputFiles, clsiServerId) + ClsiManager._sendBuiltRequest project_id, user_id, req, options, (error, status, result...) -> + return callback(error) if error? + callback(error, status, result...) + + # for public API requests where there is no project id + sendExternalRequest: (submission_id, clsi_request, options = {}, callback = (error, status, outputFiles, clsiServerId, validationProblems) ->) -> + logger.log submission_id: submission_id, "sending external compile to CLSI", clsi_request + ClsiManager._sendBuiltRequest submission_id, null, clsi_request, options, (error, status, result...) -> + return callback(error) if error? + callback(error, status, result...) stopCompile: (project_id, user_id, options, callback = (error) ->) -> compilerUrl = @_getCompilerUrl(options?.compileGroup, project_id, user_id, "compile/stop") @@ -74,6 +67,26 @@ module.exports = ClsiManager = return callback(error) if error? callback() + _sendBuiltRequest: (project_id, user_id, req, options = {}, callback = (error, status, outputFiles, clsiServerId, validationProblems) ->) -> + ClsiFormatChecker.checkRecoursesForProblems req.compile?.resources, (err, validationProblems)-> + if err? + logger.err err, project_id, "could not check resources for potential problems before sending to clsi" + return callback(err) + if validationProblems? + logger.log project_id:project_id, validationProblems:validationProblems, "problems with users latex before compile was attempted" + return callback(null, "validation-problems", null, null, validationProblems) + 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) + logger.log project_id: project_id, outputFilesLength: response?.outputFiles?.length, status: response?.status, compile_status: response?.compile?.status, "received compile response from CLSI" + 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) + callback(null, response?.compile?.status, outputFiles, clsiServerId) + _makeRequest: (project_id, opts, callback)-> ClsiCookieManager.getCookieJar project_id, (err, jar)-> if err? @@ -87,7 +100,7 @@ module.exports = ClsiManager = 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, project_id, user_id, action) -> diff --git a/services/web/app/coffee/Features/Compile/CompileController.coffee b/services/web/app/coffee/Features/Compile/CompileController.coffee index decfac515b..ee3e20fdc9 100755 --- a/services/web/app/coffee/Features/Compile/CompileController.coffee +++ b/services/web/app/coffee/Features/Compile/CompileController.coffee @@ -55,6 +55,33 @@ module.exports = CompileController = return next(error) if error? res.status(200).send() + # Used for submissions through the public API + compileSubmission: (req, res, next = (error) ->) -> + res.setTimeout(5 * 60 * 1000) + submission_id = req.params.submission_id + options = {} + if req.body?.rootResourcePath? + options.rootResourcePath = req.body.rootResourcePath + if req.body?.compiler + options.compiler = req.body.compiler + if req.body?.draft + options.draft = req.body.draft + if req.body?.check in ['validate', 'error', 'silent'] + options.check = req.body.check + options.compileGroup = req.body?.compileGroup || Settings.defaultFeatures.compileGroup + options.timeout = req.body?.timeout || Settings.defaultFeatures.compileTimeout + logger.log {options:options, submission_id:submission_id}, "got compileSubmission request" + ClsiManager.sendExternalRequest submission_id, req.body, options, (error, status, outputFiles, clsiServerId, validationProblems) -> + return next(error) if error? + logger.log {submission_id:submission_id, files:outputFiles}, "compileSubmission output files" + res.contentType("application/json") + res.status(200).send JSON.stringify { + status: status + outputFiles: outputFiles + clsiServerId: clsiServerId + validationProblems: validationProblems + } + _compileAsUser: (req, callback) -> # callback with user_id if per-user, undefined otherwise if not Settings.disablePerUserCompiles @@ -139,6 +166,12 @@ module.exports = CompileController = url = CompileController._getFileUrl project_id, user_id, req.params.build_id, req.params.file CompileController.proxyToClsi(project_id, url, req, res, next) + getFileFromClsiWithoutUser: (req, res, next = (error) ->) -> + submission_id = req.params.submission_id + url = CompileController._getFileUrl submission_id, null, req.params.build_id, req.params.file + limits = { compileGroup: req.body?.compileGroup || Settings.defaultFeatures.compileGroup } + CompileController.proxyToClsiWithLimits(submission_id, url, limits, req, res, next) + # compute a GET file url for a given project, user (optional), build (optional) and file _getFileUrl: (project_id, user_id, build_id, file) -> if user_id? and build_id? diff --git a/services/web/app/coffee/router.coffee b/services/web/app/coffee/router.coffee index 339b4abe1c..0abc170589 100644 --- a/services/web/app/coffee/router.coffee +++ b/services/web/app/coffee/router.coffee @@ -295,6 +295,22 @@ module.exports = class Router webRouter.post "/confirm-password", AuthenticationController.requireLogin(), SudoModeController.submitPassword + # New "api" endpoints. Started as a way for v1 to call over to v2 (for + # long-term features, as opposed to the nominally temporary ones in the + # overleaf-integration module), but may expand beyond that role. + publicApiRouter.post '/api/clsi/compile/:submission_id', AuthenticationController.httpAuth, CompileController.compileSubmission + publicApiRouter.get /^\/api\/clsi\/compile\/([^\/]*)\/build\/([0-9a-f-]+)\/output\/(.*)$/, + ((req, res, next) -> + params = + "submission_id": req.params[0] + "build_id": req.params[1] + "file": req.params[2] + req.params = params + next() + ), + AuthenticationController.httpAuth, + CompileController.getFileFromClsiWithoutUser + #Admin Stuff webRouter.get '/admin', AuthorizationMiddlewear.ensureUserIsSiteAdmin, AdminController.index webRouter.get '/admin/user', AuthorizationMiddlewear.ensureUserIsSiteAdmin, (req, res)-> res.redirect("/admin/register") #this gets removed by admin-panel addon diff --git a/services/web/test/acceptance/coffee/ApiClsiTests.coffee b/services/web/test/acceptance/coffee/ApiClsiTests.coffee new file mode 100644 index 0000000000..d19e11f933 --- /dev/null +++ b/services/web/test/acceptance/coffee/ApiClsiTests.coffee @@ -0,0 +1,88 @@ +expect = require("chai").expect +request = require './helpers/request' +Settings = require "settings-sharelatex" + +auth = new Buffer('sharelatex:password').toString("base64") +authed_request = request.defaults + headers: + Authorization: "Basic #{auth}" + + +describe 'ApiClsiTests', -> + describe 'compile', -> + before (done) -> + @compileSpec = + compile: + options: + compiler: 'pdflatex' + timeout: 60 + rootResourcePath: 'main.tex' + resources: [ + path: 'main/tex' + content: "\\documentclass{article}\n\\begin{document}\nHello World\n\\end{document}" + , + path: 'image.png' + url: 'www.example.com/image.png' + modified: 123456789 + ] + done() + + describe 'valid request', -> + it 'returns success and a list of output files', (done) -> + authed_request.post { + uri: '/api/clsi/compile/abcd' + json: @compileSpec + }, (error, response, body) -> + throw error if error? + expect(response.statusCode).to.equal 200 + expect(response.body).to.deep.equal { + status: 'success' + outputFiles: [ + path: 'project.pdf' + url: '/project/abcd/build/1234/output/project.pdf' + type: 'pdf' + build: 1234 + , + path: 'project.log' + url: '/project/abcd/build/1234/output/project.log' + type: 'log' + build: 1234 + ] + } + done() + + describe 'unauthorized', -> + it 'returns 401', (done) -> + request.post { + uri: '/api/clsi/compile/abcd' + json: @compileSpec + }, (error, response, body) -> + throw error if error? + expect(response.statusCode).to.equal 401 + expect(response.body).to.equal 'Unauthorized' + done() + + describe 'get output', -> + describe 'valid file', -> + it 'returns the file', (done) -> + authed_request.get '/api/clsi/compile/abcd/build/1234/output/project.pdf', (error, response, body) -> + throw error if error? + expect(response.statusCode).to.equal 200 + expect(response.body).to.equal 'mock-pdf' + done() + + describe 'invalid file', -> + it 'returns 404', (done) -> + authed_request.get '/api/clsi/compile/abcd/build/1234/output/project.aux', (error, response, body) -> + throw error if error? + expect(response.statusCode).to.equal 404 + expect(response.body).to.not.equal 'mock-pdf' + done() + + describe 'unauthorized', -> + it 'returns 401', (done) -> + request.get '/api/clsi/compile/abcd/build/1234/output/project.pdf', (error, response, body) -> + throw error if error? + expect(response.statusCode).to.equal 401 + expect(response.body).to.not.equal 'mock-pdf' + done() diff --git a/services/web/test/acceptance/coffee/helpers/MockClsiApi.coffee b/services/web/test/acceptance/coffee/helpers/MockClsiApi.coffee new file mode 100644 index 0000000000..2f9180960d --- /dev/null +++ b/services/web/test/acceptance/coffee/helpers/MockClsiApi.coffee @@ -0,0 +1,39 @@ +express = require("express") +app = express() + +module.exports = MockClsiApi = + run: () -> + app.post "/project/:project_id/compile", (req, res, next) => + res.status(200).send { + compile: + status: 'success' + error: null + outputFiles: [ + url: "/project/#{req.params.project_id}/build/1234/output/project.pdf" + path: 'project.pdf' + type: 'pdf' + build: 1234 + , + url: "/project/#{req.params.project_id}/build/1234/output/project.log" + path: 'project.log' + type: 'log' + build: 1234 + ] + } + + app.get "/project/:project_id/build/:build_id/output/*", (req, res, next) -> + filename = req.params[0] + if filename == 'project.pdf' + res.status(200).send 'mock-pdf' + else if filename == 'project.log' + res.status(200).send 'mock-log' + else + res.sendStatus(404) + + app.listen 3013, (error) -> + throw error if error? + .on "error", (error) -> + console.error "error starting MockClsiApi:", error.message + process.exit(1) + +MockClsiApi.run() diff --git a/services/web/test/unit/coffee/Compile/ClsiManagerTests.coffee b/services/web/test/unit/coffee/Compile/ClsiManagerTests.coffee index bd02af2f28..c8c39d9952 100644 --- a/services/web/test/unit/coffee/Compile/ClsiManagerTests.coffee +++ b/services/web/test/unit/coffee/Compile/ClsiManagerTests.coffee @@ -8,7 +8,7 @@ SandboxedModule = require('sandboxed-module') describe "ClsiManager", -> beforeEach -> @jar = {cookie:"stuff"} - @ClsiCookieManager = + @ClsiCookieManager = getCookieJar: sinon.stub().callsArgWith(1, null, @jar) setServerId: sinon.stub().callsArgWith(2) _getServerId:sinon.stub() @@ -99,8 +99,8 @@ describe "ClsiManager", -> status: @status = "failure" }) @ClsiManager.sendRequest @project_id, @user_id, {}, @callback - - it "should call the callback with a failure statue", -> + + it "should call the callback with a failure status", -> @callback.calledWith(null, @status).should.equal true describe "with a sync conflict", -> @@ -137,11 +137,82 @@ describe "ClsiManager", -> it "should call the callback with an error", -> @callback.calledWithExactly(new Error("failed")).should.equal true + describe "sendExternalRequest", -> + beforeEach -> + @submission_id = "submission-id" + @clsi_request = "mock-request" + @ClsiCookieManager._getServerId.callsArgWith(1, null, "clsi3") + + describe "with a successful compile", -> + beforeEach -> + @ClsiManager._postToClsi = sinon.stub().callsArgWith(4, null, { + compile: + status: @status = "success" + outputFiles: [{ + url: "#{@settings.apis.clsi.url}/project/#{@submission_id}/build/1234/output/output.pdf" + path: "output.pdf" + type: "pdf" + build: 1234 + },{ + url: "#{@settings.apis.clsi.url}/project/#{@submission_id}/build/1234/output/output.log" + path: "output.log" + type: "log" + build: 1234 + }] + }) + @ClsiManager.sendExternalRequest @submission_id, @clsi_request, {compileGroup:"standard"}, @callback + + it "should send the request to the CLSI", -> + @ClsiManager._postToClsi + .calledWith(@submission_id, null, @clsi_request, "standard") + .should.equal true + + it "should call the callback with the status and output files", -> + outputFiles = [{ + url: "/project/#{@submission_id}/build/1234/output/output.pdf" + path: "output.pdf" + type: "pdf" + build: 1234 + },{ + url: "/project/#{@submission_id}/build/1234/output/output.log" + path: "output.log" + type: "log" + build: 1234 + }] + @callback.calledWith(null, @status, outputFiles).should.equal true + + describe "with a failed compile", -> + beforeEach -> + @ClsiManager._postToClsi = sinon.stub().callsArgWith(4, null, { + compile: + status: @status = "failure" + }) + @ClsiManager.sendExternalRequest @submission_id, @clsi_request, {}, @callback + + it "should call the callback with a failure status", -> + @callback.calledWith(null, @status).should.equal true + + describe "when the resources fail the precompile check", -> + beforeEach -> + @ClsiFormatChecker.checkRecoursesForProblems = sinon.stub().callsArgWith(1, new Error("failed")) + @ClsiManager._postToClsi = sinon.stub().callsArgWith(4, null, { + compile: + status: @status = "failure" + }) + @ClsiManager.sendExternalRequest @submission_id, @clsi_request, {}, @callback + + it "should call the callback only once", -> + @callback.calledOnce.should.equal true + + it "should call the callback with an error", -> + @callback.calledWithExactly(new Error("failed")).should.equal true + + describe "deleteAuxFiles", -> beforeEach -> @ClsiManager._makeRequest = sinon.stub().callsArg(2) @DocumentUpdaterHandler.clearProjectState = sinon.stub().callsArg(1) - + describe "with the standard compileGroup", -> beforeEach -> @ClsiManager.deleteAuxFiles @project_id, @user_id, {compileGroup: "standard"}, @callback @@ -158,7 +229,7 @@ describe "ClsiManager", -> it "should call the callback", -> @callback.called.should.equal true - + describe "_buildRequest", -> beforeEach -> @@ -441,7 +512,7 @@ describe "ClsiManager", -> it "should call the callback", -> @callback.called.should.equal true - + describe "with param file", -> beforeEach -> @ClsiManager.wordCount @project_id, @user_id, "main.tex", {}, @callback @@ -450,7 +521,7 @@ describe "ClsiManager", -> @ClsiManager._makeRequest .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" @@ -468,7 +539,7 @@ describe "ClsiManager", -> beforeEach -> @response = {there:"something"} @request.callsArgWith(1, null, @response) - @opts = + @opts = method: "SOMETHIGN" url: "http://a place on the web" diff --git a/services/web/test/unit/coffee/Compile/CompileControllerTests.coffee b/services/web/test/unit/coffee/Compile/CompileControllerTests.coffee index 8a09ac866e..ced3260412 100644 --- a/services/web/test/unit/coffee/Compile/CompileControllerTests.coffee +++ b/services/web/test/unit/coffee/Compile/CompileControllerTests.coffee @@ -29,6 +29,9 @@ describe "CompileController", -> url: "clsi.example.com" clsi_priority: url: "clsi-priority.example.com" + defaultFeatures: + compileGroup: 'standard' + compileTimeout: 60 @jar = {cookie:"stuff"} @ClsiCookieManager = getCookieJar:sinon.stub().callsArgWith(1, null, @jar) @@ -109,6 +112,62 @@ describe "CompileController", -> .calledWith(@project_id, @user_id, { isAutoCompile: false, draft: true }) .should.equal true + describe "compileSubmission", -> + beforeEach -> + @submission_id = 'sub-1234' + @req.params = + submission_id: @submission_id + @req.body = {} + @ClsiManager.sendExternalRequest = sinon.stub() + .callsArgWith(3, null, @status = "success", @outputFiles = ["mock-output-files"], \ + @clsiServerId = "mock-server-id", @validationProblems = null) + + it "should set the content-type of the response to application/json", -> + @CompileController.compileSubmission @req, @res, @next + @res.contentType + .calledWith("application/json") + .should.equal true + + it "should send a successful response reporting the status and files", -> + @CompileController.compileSubmission @req, @res, @next + @res.statusCode.should.equal 200 + @res.body.should.equal JSON.stringify({ + status: @status + outputFiles: @outputFiles + clsiServerId: 'mock-server-id' + validationProblems: null + }) + + describe "with compileGroup and timeout", -> + beforeEach -> + @req.body = + compileGroup: 'special' + timeout: 600 + @CompileController.compileSubmission @req, @res, @next + + it "should use the supplied values", -> + @ClsiManager.sendExternalRequest + .calledWith(@submission_id, { compileGroup: 'special', timeout: 600 }, \ + { compileGroup: 'special', timeout: 600 }) + .should.equal true + + describe "with other supported options but not compileGroup and timeout", -> + beforeEach -> + @req.body = + rootResourcePath: 'main.tex' + compiler: 'lualatex' + draft: true + check: 'validate' + @CompileController.compileSubmission @req, @res, @next + + it "should use the other options but default values for compileGroup and timeout", -> + @ClsiManager.sendExternalRequest + .calledWith(@submission_id, \ + {rootResourcePath: 'main.tex', compiler: 'lualatex', draft: true, check: 'validate'}, \ + {rootResourcePath: 'main.tex', compiler: 'lualatex', draft: true, check: 'validate', \ + compileGroup: 'standard', timeout: 60}) + .should.equal true + describe "downloadPdf", -> beforeEach -> @req.params = @@ -167,6 +226,38 @@ describe "CompileController", -> done() @CompileController.downloadPdf @req, @res + describe "getFileFromClsiWithoutUser", -> + beforeEach -> + @submission_id = 'sub-1234' + @build_id = 123456 + @file = 'project.pdf' + @req.params = + submission_id: @submission_id + build_id: @build_id + file: @file + @req.body = {} + @expected_url = "/project/#{@submission_id}/build/#{@build_id}/output/#{@file}" + @CompileController.proxyToClsiWithLimits = sinon.stub() + + describe "without limits specified", -> + beforeEach -> + @CompileController.getFileFromClsiWithoutUser @req, @res, @next + + it "should proxy to CLSI with correct URL and default limits", -> + @CompileController.proxyToClsiWithLimits + .calledWith(@submission_id, @expected_url, {compileGroup: 'standard'}) + .should.equal true + + describe "with limits specified", -> + beforeEach -> + @req.body = {compileTimeout: 600, compileGroup: 'special'} + @CompileController.getFileFromClsiWithoutUser @req, @res, @next + + it "should proxy to CLSI with correct URL and specified limits", -> + @CompileController.proxyToClsiWithLimits + .calledWith(@submission_id, @expected_url, {compileGroup: 'special'}) + .should.equal true + describe "proxyToClsi", -> beforeEach -> @request.returns(@proxy = {