Przeglądaj źródła

Look up compile group and features from project owner, not current user

James Allen 12 lat temu
rodzic
commit
175dfae085

+ 36 - 29
services/web/app/coffee/Features/Compile/CompileController.coffee

@@ -13,21 +13,25 @@ module.exports = CompileController =
 		res.setTimeout(5 * 60 * 1000)
 		project_id = req.params.Project_id
 		isAutoCompile = !!req.query?.auto_compile
-		settingsOverride = req.body?.settingsOverride ? {};
-		logger.log "root doc overriden" if settingsOverride.rootDoc_id?
 		AuthenticationController.getLoggedInUserId req, (error, user_id) ->
 			return next(error) if error?
-			UserGetter.getUser user_id, {"features.compileGroup":1, "features.compileTimeout":1}, (err, user)->
-				settingsOverride.timeout = user?.features?.compileTimeout || Settings.defaultFeatures.compileTimeout
-				settingsOverride.compiler = user?.features?.compileGroup || Settings.defaultFeatures.compileGroup
-				req.session.compileGroup = settingsOverride.compiler
-				CompileManager.compile project_id, user_id, { isAutoCompile, settingsOverride }, (error, status, outputFiles) ->
-					return next(error) if error?
-					res.contentType("application/json")
-					res.send 200, JSON.stringify {
-						status: status
-						outputFiles: outputFiles
-					}
+			options = {
+				isAutoCompile: isAutoCompile
+			}
+			if req.body?.rootDoc_id?
+				options.rootDoc_id = req.body.rootDoc_id
+			else if req.body?.settingsOverride?.rootDoc_id? # Can be removed after deploy
+				options.rootDoc_id = req.body.settingsOverride.rootDoc_id
+			if req.body?.compiler
+				options.compiler = req.body.compiler
+			logger.log {options, project_id}, "got compile request"
+			CompileManager.compile project_id, user_id, options, (error, status, outputFiles) ->
+				return next(error) if error?
+				res.contentType("application/json")
+				res.send 200, JSON.stringify {
+					status: status
+					outputFiles: outputFiles
+				}
 
 	downloadPdf: (req, res, next = (error) ->)->
 		Metrics.inc "pdf-downloads"
@@ -40,7 +44,7 @@ module.exports = CompileController =
 			else
 				logger.log project_id: project_id, "download pdf to embed in browser"
 				res.header('Content-Disposition', "filename=#{project.getSafeProjectName()}.pdf")
-			CompileController.proxyToClsi("/project/#{project_id}/output/output.pdf", req, res, next)
+			CompileController.proxyToClsi(project_id, "/project/#{project_id}/output/output.pdf", req, res, next)
 
 	deleteAuxFiles: (req, res, next) ->
 		project_id = req.params.Project_id
@@ -55,25 +59,28 @@ module.exports = CompileController =
 				logger.err err:err, project_id:project_id, "something went wrong compile and downloading pdf"
 				res.send 500
 			url = "/project/#{project_id}/output/output.pdf"
-			CompileController.proxyToClsi url, req, res, next
+			CompileController.proxyToClsi project_id, url, req, res, next
 
 	getFileFromClsi: (req, res, next = (error) ->) ->
-		CompileController.proxyToClsi("/project/#{req.params.Project_id}/output/#{req.params.file}", req, res, next)
+		project_id = req.params.Project_id
+		CompileController.proxyToClsi(project_id, "/project/#{project_id}/output/#{req.params.file}", req, res, next)
 
 	proxySync: (req, res, next = (error) ->) ->
-		CompileController.proxyToClsi(req.url, req, res, next)
+		CompileController.proxyToClsi(req.params.Project_id, req.url, req, res, next)
 
-	proxyToClsi: (url, req, res, next = (error) ->) ->
-		if req.session.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
-		proxy = request(url: url, method: req.method, timeout: oneMinute)
-		proxy.pipe(res)
-		proxy.on "error", (error) ->
-			logger.warn err: error, url: url, "CLSI proxy error"
+	proxyToClsi: (project_id, url, req, res, next = (error) ->) ->
+		CompileManager.getProjectCompileLimits project_id, (error, limits) ->
+			return next(error) if 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
+			proxy = request(url: url, method: req.method, timeout: oneMinute)
+			proxy.pipe(res)
+			proxy.on "error", (error) ->
+				logger.warn err: error, url: url, "CLSI proxy error"
 
 	

+ 19 - 5
services/web/app/coffee/Features/Compile/CompileManager.coffee

@@ -6,19 +6,20 @@ rclient = redis.createClient(Settings.redis.web)
 DocumentUpdaterHandler = require "../DocumentUpdater/DocumentUpdaterHandler"
 Project = require("../../models/Project").Project
 ProjectRootDocManager = require "../Project/ProjectRootDocManager"
+UserGetter = require "../User/UserGetter"
 ClsiManager = require "./ClsiManager"
 Metrics = require('../../infrastructure/Metrics')
 logger = require("logger-sharelatex")
 rateLimiter = require("../../infrastructure/RateLimiter")
 
 module.exports = CompileManager =
-	compile: (project_id, user_id, opt = {}, _callback = (error) ->) ->
+	compile: (project_id, user_id, options = {}, _callback = (error) ->) ->
 		timer = new Metrics.Timer("editor.compile")
 		callback = (args...) ->
 			timer.done()
 			_callback(args...)
 
-		@_checkIfAutoCompileLimitHasBeenHit opt.isAutoCompile, (err, canCompile)->
+		@_checkIfAutoCompileLimitHasBeenHit options.isAutoCompile, (err, canCompile)->
 			if !canCompile
 				return callback null, "autocompile-backoff", []
 			logger.log project_id: project_id, user_id: user_id, "compiling project"
@@ -31,11 +32,24 @@ module.exports = CompileManager =
 					return callback(error) if error?
 					DocumentUpdaterHandler.flushProjectToMongo project_id, (error) ->
 						return callback(error) if error?
-						ClsiManager.sendRequest project_id, opt.settingsOverride, (error, status, outputFiles) ->
+						CompileManager.getProjectCompileLimits project_id, (error, limits) ->
 							return callback(error) if error?
-							logger.log files: outputFiles, "output files"
-							callback(null, status, outputFiles)
+							for key, value of limits
+								options[key] = value
+							ClsiManager.sendRequest project_id, options, (error, status, outputFiles, output) ->
+								return callback(error) if error?
+								logger.log files: outputFiles, "output files"
+								callback(null, status, outputFiles, output)
 
+	getProjectCompileLimits: (project_id, callback = (error, limits) ->) ->
+		Project.findById project_id, {owner_ref: 1}, (error, project) ->
+			return callback(error) if error?
+			UserGetter.getUser project.owner_ref, {"features":1}, (err, owner)->
+				return callback(error) if error?
+				callback null, {
+					timeout: owner.features?.compileTimeout || Settings.defaultFeatures.compileTimeout
+					compileGroup: owner.features?.compileGroup || Settings.defaultFeatures.compileGroup
+				}
 
 	getLogLines: (project_id, callback)->
 		Metrics.inc "editor.raw-logs"

+ 1 - 2
services/web/public/coffee/ide/pdf/controllers/PdfController.coffee

@@ -15,8 +15,7 @@ define [
 			if options.isAutoCompile
 				url += "?auto_compile=true"
 			return $http.post url, {
-				settingsOverride:
-					rootDoc_id: options.rootDocOverride_id or null
+				rootDoc_id: options.rootDocOverride_id or null
 				_csrf: window.csrfToken
 			}
 

+ 12 - 23
services/web/test/UnitTests/coffee/Compile/CompileControllerTests.coffee

@@ -21,7 +21,7 @@ describe "CompileController", ->
 					clsi:
 						url: "clsi.example.com"
 					clsi_priority:
-						url: "clsi.example.com"
+						url: "clsi-priority.example.com"
 			"request": @request = sinon.stub()
 			"../../models/Project": Project: @Project = {}
 			"logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub() }
@@ -47,8 +47,7 @@ describe "CompileController", ->
 					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"])
-				@UserGetter.getUser.callsArgWith(2, null, @user)
+				@CompileManager.compile = sinon.stub().callsArgWith(3, null, @status = "success", @outputFiles = ["mock-output-files"], @output = "mock-output")
 				@CompileController.compile @req, @res, @next
 
 			it "should look up the user id", ->
@@ -58,7 +57,7 @@ describe "CompileController", ->
 
 			it "should do the compile without the auto compile flag", ->
 				@CompileManager.compile
-					.calledWith(@project_id, @user_id, { isAutoCompile: false, settingsOverride:{timeout:@user.features.compileTimeout, compiler:@user.features.compileGroup} })
+					.calledWith(@project_id, @user_id, { isAutoCompile: false })
 					.should.equal true
 
 			it "should set the content-type of the response to application/json", ->
@@ -73,16 +72,6 @@ describe "CompileController", ->
 					outputFiles: @outputFiles
 				})
 
-			it "should get the compile timeout from the users features",->
-				@UserGetter.getUser.args[0][0].should.equal @user_id
-				assert.deepEqual @UserGetter.getUser.args[0][1], {"features.compileGroup":1, "features.compileTimeout":1}
-
-			it "should put the compile group on the req", ->
-				@req.session.compileGroup.should.equal @user.features.compileGroup
-
-			it "should set the timeout", ->
-				assert @res.timout > 1000 * 60 * 3
-
 		describe "when an auto compile", ->
 			beforeEach ->
 				@req.params =
@@ -91,11 +80,12 @@ describe "CompileController", ->
 					auto_compile: "true"
 				@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"])
-				@UserGetter.getUser.callsArgWith(2, null, @user)
 				@CompileController.compile @req, @res, @next
 
 			it "should do the compile with the auto compile flag", ->
-				@CompileManager.compile.calledWith(@project_id, @user_id, { isAutoCompile: true, settingsOverride:{timeout:@user.features.compileTimeout, compiler:@user.features.compileGroup} }).should.equal true
+				@CompileManager.compile
+					.calledWith(@project_id, @user_id, { isAutoCompile: true })
+					.should.equal true
 
 	describe "downloadPdf", ->
 		beforeEach ->
@@ -134,7 +124,7 @@ describe "CompileController", ->
 
 			it "should proxy the PDF from the CLSI", ->
 				@CompileController.proxyToClsi
-					.calledWith("/project/#{@project_id}/output/output.pdf", @req, @res, @next)
+					.calledWith(@project_id, "/project/#{@project_id}/output/output.pdf", @req, @res, @next)
 					.should.equal true
 
 	describe "proxyToClsi", ->
@@ -152,8 +142,8 @@ describe "CompileController", ->
 		describe "user with standard priority", ->
 
 			beforeEach ->
-				@UserGetter.getUser.callsArgWith(2, null, @user)
-				@CompileController.proxyToClsi(@url = "/test", @req, @res, @next)	
+				@CompileManager.getProjectCompileLimits = sinon.stub().callsArgWith(1, null, {compileGroup: "standard"})
+				@CompileController.proxyToClsi(@project_id, @url = "/test", @req, @res, @next)	
 
 
 			it "should open a request to the CLSI", ->
@@ -176,9 +166,8 @@ describe "CompileController", ->
 		describe "user with priority compile", ->
 
 			beforeEach ->
-				@req.session.compileGroup = "priority"
-				@UserGetter.getUser.callsArgWith(2, null, @user)
-				@CompileController.proxyToClsi(@url = "/test", @req, @res, @next)	
+				@CompileManager.getProjectCompileLimits = sinon.stub().callsArgWith(1, null, {compileGroup: "priority"})
+				@CompileController.proxyToClsi(@project_id, @url = "/test", @req, @res, @next)	
 
 			it "should proxy to the priorty url if the user has the feature", ()->
 				@request
@@ -225,5 +214,5 @@ describe "CompileController", ->
 
 		it "should proxy the res to the clsi with correct url", (done)->
 			@CompileController.compileAndDownloadPdf @req, @res
-			@CompileController.proxyToClsi.calledWith("/project/#{@project_id}/output/output.pdf", @req, @res).should.equal true
+			@CompileController.proxyToClsi.calledWith(@project_id, "/project/#{@project_id}/output/output.pdf", @req, @res).should.equal true
 			done()

+ 46 - 5
services/web/test/UnitTests/coffee/Compile/CompileManagerTests.coffee

@@ -20,6 +20,7 @@ describe "CompileManager", ->
 			"../DocumentUpdater/DocumentUpdaterHandler": @DocumentUpdaterHandler = {}
 			"../Project/ProjectRootDocManager": @ProjectRootDocManager = {}
 			"../../models/Project": Project: @Project = {}
+			"../User/UserGetter": @UserGetter = {}
 			"./ClsiManager": @ClsiManager = {}
 			"../../infrastructure/RateLimiter": @ratelimiter
 			"../../infrastructure/Metrics": @Metrics =
@@ -30,13 +31,18 @@ describe "CompileManager", ->
 		@project_id = "mock-project-id-123"
 		@user_id = "mock-user-id-123"
 		@callback = sinon.stub()
+		@limits = {
+			timeout: 42
+		}
 
+	
 	describe "compile", ->
 		beforeEach ->
 			@CompileManager._checkIfRecentlyCompiled = sinon.stub().callsArgWith(2, null, false)
 			@CompileManager._ensureRootDocumentIsSet = sinon.stub().callsArgWith(1, null)
 			@DocumentUpdaterHandler.flushProjectToMongo = sinon.stub().callsArgWith(1, null)
-			@ClsiManager.sendRequest = sinon.stub().callsArgWith(2, null, @status = "mock-status")
+			@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")
 
 		describe "succesfully", ->
 			beforeEach ->
@@ -58,14 +64,21 @@ describe "CompileManager", ->
 					.calledWith(@project_id)
 					.should.equal true
 
-			it "should run the compile with the new compiler API", ->
-				@ClsiManager.sendRequest
+			it "should get the project compile limits", ->
+				@CompileManager.getProjectCompileLimits
 					.calledWith(@project_id)
 					.should.equal true
 
-			it "should call the callback", ->
+			it "should run the compile with the compile limits", ->
+				@ClsiManager.sendRequest
+					.calledWith(@project_id, {
+						timeout: @limits.timeout
+					})
+					.should.equal true
+
+			it "should call the callback with the output", ->
 				@callback
-					.calledWith(null, @status)
+					.calledWith(null, @status, @outputFiles, @output)
 					.should.equal true
 
 			it "should time the compile", ->
@@ -93,6 +106,34 @@ describe "CompileManager", ->
 				@CompileManager.compile @project_id, @user_id, {}, (err, status)->
 					status.should.equal "autocompile-backoff"
 					done()
+					
+	describe "getProjectCompileLimits", ->
+		beforeEach ->
+			@features = {
+				compileTimeout:   @timeout = 42
+				compileGroup:     @group = "priority"
+			}
+			@Project.findById = sinon.stub().callsArgWith(2, null, @project = { owner_ref: @owner_id = "owner-id-123" })
+			@UserGetter.getUser = sinon.stub().callsArgWith(2, null, @user = { features: @features })
+			@CompileManager.getProjectCompileLimits @project_id, @callback
+			
+		it "should look up the owner of the project", ->
+			@Project.findById
+				.calledWith(@project_id, { owner_ref: 1 })
+				.should.equal true
+				
+		it "should look up the owner's features", ->
+			@UserGetter.getUser
+				.calledWith(@project.owner_ref, { features: 1 })
+				.should.equal true
+				
+		it "should return the limits", ->
+			@callback
+				.calledWith(null, {
+					timeout:      @timeout
+					compileGroup: @group
+				})
+				.should.equal true
 
 	describe "getLogLines", ->
 		beforeEach ->