Explorar o código

Reconfigure internal CLSI call to understand more than just success/failure

James Allen %!s(int64=12) %!d(string=hai) anos
pai
achega
6c5a2c2740

+ 8 - 2
services/web/app/coffee/Features/Compile/ClsiManager.coffee

@@ -17,7 +17,7 @@ module.exports = ClsiManager =
 				logger.log project_id: project_id, response: response, "received compile response from CLSI"
 				logger.log project_id: project_id, response: response, "received compile response from CLSI"
 				callback(
 				callback(
 					null
 					null
-					(response?.compile?.status == "success")
+					response?.compile?.status
 					ClsiManager._parseOutputFiles(project_id, response?.compile?.outputFiles)
 					ClsiManager._parseOutputFiles(project_id, response?.compile?.outputFiles)
 				)
 				)
 
 
@@ -32,7 +32,13 @@ module.exports = ClsiManager =
 			json: req
 			json: req
 			jar:  false
 			jar:  false
 		}, (error, response, body) ->
 		}, (error, response, body) ->
-			callback error, body
+			return callback(error) if error?
+			if 200 <= response.statusCode < 300
+				callback null, body
+			else
+				error = new Error("CLSI returned non-success code: #{response.statusCode}")
+				logger.error err: error, project_id: project_id, "CLSI returned failure code"
+				callback error, body
 
 
 	_parseOutputFiles: (project_id, rawOutputFiles = []) ->
 	_parseOutputFiles: (project_id, rawOutputFiles = []) ->
 		outputFiles = []
 		outputFiles = []

+ 2 - 2
services/web/app/coffee/Features/Compile/CompileManager.coffee

@@ -31,10 +31,10 @@ module.exports = CompileManager =
 					return callback(error) if error?
 					return callback(error) if error?
 					DocumentUpdaterHandler.flushProjectToMongo project_id, (error) ->
 					DocumentUpdaterHandler.flushProjectToMongo project_id, (error) ->
 						return callback(error) if error?
 						return callback(error) if error?
-						ClsiManager.sendRequest project_id, (error, success, outputFiles) ->
+						ClsiManager.sendRequest project_id, (error, status, outputFiles) ->
 							return callback(error) if error?
 							return callback(error) if error?
 							logger.log files: outputFiles, "output files"
 							logger.log files: outputFiles, "output files"
-							callback(null, success, outputFiles)
+							callback(null, status, outputFiles)
 
 
 	getLogLines: (project_id, callback)->
 	getLogLines: (project_id, callback)->
 		Metrics.inc "editor.raw-logs"
 		Metrics.inc "editor.raw-logs"

+ 2 - 1
services/web/app/coffee/router.coffee

@@ -322,7 +322,8 @@ module.exports = class Router
 
 
 			client.on 'pdfProject', (opts, callback)->
 			client.on 'pdfProject', (opts, callback)->
 				AuthorizationManager.ensureClientCanViewProject client, (error, project_id) =>
 				AuthorizationManager.ensureClientCanViewProject client, (error, project_id) =>
-					CompileManager.compile(project_id, user._id, opts, callback)
+					CompileManager.compile project_id, user._id, opts, (error, status, outputFiles) ->
+						return callback error, status == "success", outputFiles
 
 
 			client.on 'enableversioningController', (callback)->
 			client.on 'enableversioningController', (callback)->
 				AuthorizationManager.ensureClientCanEditProject client, (error, project_id) =>
 				AuthorizationManager.ensureClientCanEditProject client, (error, project_id) =>

+ 34 - 5
services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee

@@ -17,7 +17,8 @@ describe "ClsiManager", ->
 						url: "http://clsi.example.com"
 						url: "http://clsi.example.com"
 			"../../models/Project": Project: @Project = {}
 			"../../models/Project": Project: @Project = {}
 			"../Project/ProjectEntityHandler": @ProjectEntityHandler = {}
 			"../Project/ProjectEntityHandler": @ProjectEntityHandler = {}
-			"logger-sharelatex": @logger = { log: sinon.stub() }
+			"logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub() }
+			"request": @request = {}
 		@project_id = "project-id"
 		@project_id = "project-id"
 		@callback = sinon.stub()
 		@callback = sinon.stub()
 
 
@@ -29,7 +30,7 @@ describe "ClsiManager", ->
 			beforeEach ->
 			beforeEach ->
 				@ClsiManager._postToClsi = sinon.stub().callsArgWith(2, null, {
 				@ClsiManager._postToClsi = sinon.stub().callsArgWith(2, null, {
 					compile:
 					compile:
-						status: "success"
+						status: @status = "success"
 						outputFiles: [{
 						outputFiles: [{
 							url: "#{@settings.apis.clsi.url}/project/#{@project_id}/output/output.pdf"
 							url: "#{@settings.apis.clsi.url}/project/#{@project_id}/output/output.pdf"
 							type: "pdf"
 							type: "pdf"
@@ -58,18 +59,18 @@ describe "ClsiManager", ->
 					path: "output.log"
 					path: "output.log"
 					type: "log"
 					type: "log"
 				}]
 				}]
-				@callback.calledWith(null, true, outputFiles).should.equal true
+				@callback.calledWith(null, @status, outputFiles).should.equal true
 
 
 		describe "with a failed compile", ->
 		describe "with a failed compile", ->
 			beforeEach ->
 			beforeEach ->
 				@ClsiManager._postToClsi = sinon.stub().callsArgWith(2, null, {
 				@ClsiManager._postToClsi = sinon.stub().callsArgWith(2, null, {
 					compile:
 					compile:
-						status: "failure"
+						status: @status = "failure"
 				})
 				})
 				@ClsiManager.sendRequest @project_id, @callback
 				@ClsiManager.sendRequest @project_id, @callback
 			
 			
 			it "should call the callback with a failure statue", ->
 			it "should call the callback with a failure statue", ->
-				@callback.calledWith(null, false).should.equal true
+				@callback.calledWith(null, @status).should.equal true
 
 
 	describe "_buildRequest", ->
 	describe "_buildRequest", ->
 		beforeEach ->
 		beforeEach ->
@@ -165,3 +166,31 @@ describe "ClsiManager", ->
 				expect(@error).to.exist
 				expect(@error).to.exist
 
 
 
 
+	describe '_postToClsi', ->
+		beforeEach ->
+			@req = { mock: "req" }
+
+		describe "successfully", ->
+			beforeEach ->
+				@request.post = sinon.stub().callsArgWith(1, null, {statusCode: 204}, @body = { mock: "foo" })
+				@ClsiManager._postToClsi @project_id, @req, @callback
+
+			it 'should send the request to the CLSI', ->
+				url = "#{@settings.apis.clsi.url}/project/#{@project_id}/compile"
+				@request.post.calledWith({
+					url: url
+					json: @req
+					jar: false
+				}).should.equal true
+
+			it "should call the callback with the body and no error", ->
+				@callback.calledWith(null, @body).should.equal true
+
+		describe "when the CLSI returns an error", ->
+			beforeEach ->
+				@request.post = sinon.stub().callsArgWith(1, null, {statusCode: 500}, @body = { mock: "foo" })
+				@ClsiManager._postToClsi @project_id, @req, @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
+

+ 0 - 2
services/web/test/UnitTests/coffee/Compile/CompileManagerTests.coffee

@@ -21,7 +21,6 @@ describe "CompileManager", ->
 			"../Project/ProjectRootDocManager": @ProjectRootDocManager = {}
 			"../Project/ProjectRootDocManager": @ProjectRootDocManager = {}
 			"../../models/Project": Project: @Project = {}
 			"../../models/Project": Project: @Project = {}
 			"./ClsiManager": @ClsiManager = {}
 			"./ClsiManager": @ClsiManager = {}
-			"../../managers/LatexManager": @LatexManager = {}
 			"../../infrastructure/RateLimiter": @ratelimiter
 			"../../infrastructure/RateLimiter": @ratelimiter
 			"../../infrastructure/Metrics": @Metrics =
 			"../../infrastructure/Metrics": @Metrics =
 				Timer: class Timer
 				Timer: class Timer
@@ -38,7 +37,6 @@ describe "CompileManager", ->
 			@CompileManager._ensureRootDocumentIsSet = sinon.stub().callsArgWith(1, null)
 			@CompileManager._ensureRootDocumentIsSet = sinon.stub().callsArgWith(1, null)
 			@DocumentUpdaterHandler.flushProjectToMongo = sinon.stub().callsArgWith(1, null)
 			@DocumentUpdaterHandler.flushProjectToMongo = sinon.stub().callsArgWith(1, null)
 			@ClsiManager.sendRequest = sinon.stub().callsArgWith(1, null, @status = "mock-status")
 			@ClsiManager.sendRequest = sinon.stub().callsArgWith(1, null, @status = "mock-status")
-			@LatexManager.compile = sinon.stub().callsArgWith(1, null, @status = "mock-status")
 
 
 		describe "succesfully", ->
 		describe "succesfully", ->
 			beforeEach ->
 			beforeEach ->