From 1437877b5ac4226d3b5a94cffb7ee28cd13c0aa8 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Wed, 1 Jun 2016 12:28:54 +0100 Subject: [PATCH 1/6] spike to check latex for basic errors before compile Aims to solve following problems which are currently not visible to user: - project is too big, which files are worst offenders? - when there are duplicate file names so an 'old' version keeps overrighting a new version - when a file has the same path as a folder which blows up clsi i.e. images/research images/research/1.png --- .../Features/Compile/ClsiManager.coffee | 69 ++++++++++++++ .../coffee/Compile/ClsiManagerTests.coffee | 94 +++++++++++++++++-- 2 files changed, 153 insertions(+), 10 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/ClsiManager.coffee b/services/web/app/coffee/Features/Compile/ClsiManager.coffee index 4a33a1a00c..db15483c9c 100755 --- a/services/web/app/coffee/Features/Compile/ClsiManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiManager.coffee @@ -7,6 +7,8 @@ ProjectEntityHandler = require("../Project/ProjectEntityHandler") logger = require "logger-sharelatex" url = require("url") ClsiCookieManager = require("./ClsiCookieManager") +_ = require("underscore") +async = require("async") module.exports = ClsiManager = @@ -153,3 +155,70 @@ module.exports = ClsiManager = logger.error err: error, project_id: project_id, "CLSI returned failure code" callback error, body + _checkRecoursesForErrors: (resources, callback)-> + jobs = + duplicatePaths: (cb)-> + ClsiManager._checkForFilesWithSameName resources, cb + + conflictedPaths: (cb)-> + ClsiManager._checkForConflictingPaths resources, cb + + sizeCheck: (cb)-> + ClsiManager._checkDocsAreUnderSizeLimit resources, cb + + async.series jobs, callback + + _checkForFilesWithSameName: (resources, callback)-> + paths = _.pluck(resources, 'path') + + duplicates = _.filter paths, (path)-> + return _.countBy(paths)[path] > 1 + + duplicates = _.uniq(duplicates) + + duplicateObjects = _.map duplicates, (dup)-> + path:dup + + callback(null, duplicateObjects) + + _checkForConflictingPaths: (resources, callback)-> + paths = _.pluck(resources, 'path') + + conflicts = _.filter paths, (path)-> + matchingPaths = _.filter paths, (checkPath)-> + return checkPath.indexOf(path+"/") != -1 + + return matchingPaths.length > 0 + + conflictObjects = _.map conflicts, (conflict)-> + path:conflict + + callback null, conflictObjects + + _checkDocsAreUnderSizeLimit: (resources, callback)-> + + FIVEMB = 1000 * 1000 * 5 + + totalSize = 0 + + sizedResources = _.map resources, (resource)-> + result = {path:resource.path} + if resource.content? + result.size = resource.content.join("").length + else + result.size = 0 + totalSize += result.size + return result + + tooLarge = totalSize > FIVEMB + + sizedResources = _.sortBy(sizedResources, "size").slice(10).reverse() + + callback(null, {resources:sizedResources, totalSize:totalSize, tooLarge:tooLarge}) + + + + + + + diff --git a/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee b/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee index 9889bcb87b..ef9c12f408 100644 --- a/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee @@ -320,16 +320,90 @@ describe "ClsiManager", -> - - - - - - - - - - + describe "_checkRecoursesForErrors", -> + + beforeEach -> + @resources = [{ + path: "main.tex" + content: ["stuff"] + }, { + path: "chapters/chapter1" + content: ["other stuff"] + }, { + path: "stuff/image/image.png" + url: "#{@settings.apis.filestore.url}/project/#{@project_id}/file/1234124321312" + modified: ["more stuff"] + }] + + it "should call _checkForFilesWithSameName and _checkForConflictingPaths", (done)-> + + @ClsiManager._checkForFilesWithSameName = sinon.stub().callsArgWith(1) + @ClsiManager._checkForConflictingPaths = sinon.stub().callsArgWith(1) + @ClsiManager._checkDocsAreUnderSizeLimit = sinon.stub().callsArgWith(1) + @ClsiManager._checkRecoursesForErrors @resources, => + @ClsiManager._checkForFilesWithSameName.called.should.equal true + @ClsiManager._checkForConflictingPaths.called.should.equal true + @ClsiManager._checkDocsAreUnderSizeLimit.called.should.equal true + done() + + describe "_checkForFilesWithSameName", -> + + it "should flag up 2 nested files with same path", (done)-> + + @resources.push({ + path: "chapters/chapter1" + url: "http://somwhere.com" + }) + + @ClsiManager._checkForFilesWithSameName @resources, (err, duplicateErrors)-> + duplicateErrors.length.should.equal 1 + duplicateErrors[0].path.should.equal "chapters/chapter1" + done() + + describe "_checkForConflictingPaths", -> + + it "should flag up when a nested file has folder with same subpath as file elsewhere", (done)-> + @resources.push({ + path: "stuff/image" + url: "http://somwhere.com" + }) + + @resources.push({ + path: "chapters/chapter1.tex" + content: ["other stuff"] + }) + + @resources.push({ + path: "chapters.tex" + content: ["other stuff"] + }) + + @ClsiManager._checkForConflictingPaths @resources, (err, conflictPathErrors)-> + conflictPathErrors.length.should.equal 1 + conflictPathErrors[0].path.should.equal "stuff/image" + done() + + + describe "_checkDocsAreUnderSizeLimit", -> + + it "should error when there is more than 2mb of data", (done)-> + + @resources.push({ + path: "massive.tex" + content: [require("crypto").randomBytes(1000 * 1000 * 5).toString("hex")] + }) + + while @resources.length < 20 + @resources.push({path:"chapters/chapter1.tex",url: "http://somwhere.com"}) + + @ClsiManager._checkDocsAreUnderSizeLimit @resources, (err, sizeError)-> + sizeError.tooLarge.should.equal true + sizeError.totalSize.should.equal 10000016 + sizeError.resources.length.should.equal 10 + sizeError.resources[0].path.should.equal "massive.tex" + sizeError.resources[0].size.should.equal 1000 * 1000 * 10 + done() + From 66ad587c9c981df085c771a80e4021e0e99d0e2c Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Wed, 1 Jun 2016 16:46:11 +0100 Subject: [PATCH 2/6] bump underscore to 1.8.3 --- services/web/package.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/services/web/package.json b/services/web/package.json index e34a564309..3f4f9e5c06 100644 --- a/services/web/package.json +++ b/services/web/package.json @@ -55,8 +55,7 @@ "sixpack-client": "^1.0.0", "temp": "^0.8.3", "translations-sharelatex": "git+https://github.com/sharelatex/translations-sharelatex.git#master", - "underscore": "1.6.0", - "underscore.string": "^3.0.2", + "underscore": "1.8.3", "v8-profiler": "^5.2.3", "xml2js": "0.2.0" }, From 8a5cb86c31dec3999fb28f5e47e520c1f0ed4100 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Wed, 1 Jun 2016 16:46:41 +0100 Subject: [PATCH 3/6] check _checkRecoursesForErrors before compile --- .../Features/Compile/ClsiManager.coffee | 54 ++++++++++++------- .../coffee/Compile/ClsiManagerTests.coffee | 26 ++++++--- 2 files changed, 54 insertions(+), 26 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/ClsiManager.coffee b/services/web/app/coffee/Features/Compile/ClsiManager.coffee index db15483c9c..18f313a368 100755 --- a/services/web/app/coffee/Features/Compile/ClsiManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiManager.coffee @@ -17,17 +17,24 @@ module.exports = ClsiManager = 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) -> - 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, "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._checkRecoursesForErrors req.compile.resources, (err, problems)-> + if err? + logger.err err, project_id, "could not check resources for potential problems before sending to clsi" + return callback(err) + if problems? + logger.log project_id:project_id, problems:problems, "problems with users latex before compile was attempted" + return callback(null, problems) + ClsiManager._postToClsi project_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, "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) deleteAuxFiles: (project_id, options, callback = (error) ->) -> compilerUrl = @_getCompilerUrl(options?.compileGroup) @@ -158,17 +165,25 @@ module.exports = ClsiManager = _checkRecoursesForErrors: (resources, callback)-> jobs = duplicatePaths: (cb)-> - ClsiManager._checkForFilesWithSameName resources, cb + ClsiManager._checkForDuplicatePaths resources, cb conflictedPaths: (cb)-> ClsiManager._checkForConflictingPaths resources, cb sizeCheck: (cb)-> ClsiManager._checkDocsAreUnderSizeLimit resources, cb - - async.series jobs, callback - _checkForFilesWithSameName: (resources, callback)-> + async.series jobs, (err, problems)-> + if err? + return callback(err) + problems = _.omit(problems, _.isEmpty) + if _.isEmpty(problems) + return callback() + else + callback(null, problems) + + + _checkForDuplicatePaths: (resources, callback)-> paths = _.pluck(resources, 'path') duplicates = _.filter paths, (path)-> @@ -211,10 +226,11 @@ module.exports = ClsiManager = return result tooLarge = totalSize > FIVEMB - - sizedResources = _.sortBy(sizedResources, "size").slice(10).reverse() - - callback(null, {resources:sizedResources, totalSize:totalSize, tooLarge:tooLarge}) + if !tooLarge + return callback() + else + sizedResources = _.sortBy(sizedResources, "size").slice(10).reverse() + return callback(null, {resources:sizedResources, totalSize:totalSize}) diff --git a/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee b/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee index ef9c12f408..b31fb59fb5 100644 --- a/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee @@ -335,18 +335,31 @@ describe "ClsiManager", -> modified: ["more stuff"] }] - it "should call _checkForFilesWithSameName and _checkForConflictingPaths", (done)-> + it "should call _checkForDuplicatePaths and _checkForConflictingPaths", (done)-> - @ClsiManager._checkForFilesWithSameName = sinon.stub().callsArgWith(1) + @ClsiManager._checkForDuplicatePaths = sinon.stub().callsArgWith(1) @ClsiManager._checkForConflictingPaths = sinon.stub().callsArgWith(1) @ClsiManager._checkDocsAreUnderSizeLimit = sinon.stub().callsArgWith(1) - @ClsiManager._checkRecoursesForErrors @resources, => - @ClsiManager._checkForFilesWithSameName.called.should.equal true + @ClsiManager._checkRecoursesForErrors @resources, (err, problems)=> + @ClsiManager._checkForDuplicatePaths.called.should.equal true @ClsiManager._checkForConflictingPaths.called.should.equal true @ClsiManager._checkDocsAreUnderSizeLimit.called.should.equal true + expect(problems).to.not.exist done() - describe "_checkForFilesWithSameName", -> + + it "should remove undefined errors", (done)-> + @ClsiManager._checkForDuplicatePaths = sinon.stub().callsArgWith(1, null, [path:"something/here"]) + @ClsiManager._checkForConflictingPaths = sinon.stub().callsArgWith(1, null, []) + @ClsiManager._checkDocsAreUnderSizeLimit = sinon.stub().callsArgWith(1, null) + @ClsiManager._checkRecoursesForErrors @resources, (err, problems)=> + problems.duplicatePaths[0].path.should.equal "something/here" + expect(problems.conflictedPaths).to.not.exist + expect(problems.sizeCheck).to.not.exist + + done() + + describe "_checkForDuplicatePaths", -> it "should flag up 2 nested files with same path", (done)-> @@ -355,7 +368,7 @@ describe "ClsiManager", -> url: "http://somwhere.com" }) - @ClsiManager._checkForFilesWithSameName @resources, (err, duplicateErrors)-> + @ClsiManager._checkForDuplicatePaths @resources, (err, duplicateErrors)-> duplicateErrors.length.should.equal 1 duplicateErrors[0].path.should.equal "chapters/chapter1" done() @@ -397,7 +410,6 @@ describe "ClsiManager", -> @resources.push({path:"chapters/chapter1.tex",url: "http://somwhere.com"}) @ClsiManager._checkDocsAreUnderSizeLimit @resources, (err, sizeError)-> - sizeError.tooLarge.should.equal true sizeError.totalSize.should.equal 10000016 sizeError.resources.length.should.equal 10 sizeError.resources[0].path.should.equal "massive.tex" From c284465ba5ccdd98942c1775b2cbf37e6c887a58 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Thu, 2 Jun 2016 13:09:11 +0100 Subject: [PATCH 4/6] added clsiformat checker, wired in --- .../Features/Compile/ClsiFormatChecker.coffee | 68 ++++++++ .../Features/Compile/ClsiManager.coffee | 88 +--------- .../Features/Compile/CompileController.coffee | 3 +- .../Features/Compile/CompileManager.coffee | 4 +- .../web/app/views/project/editor/pdf.jade | 21 +++ .../ide/pdf/controllers/PdfController.coffee | 6 +- .../public/stylesheets/app/editor/pdf.less | 2 +- .../Compile/ClsiFormatCheckerTests.coffee | 151 ++++++++++++++++++ .../coffee/Compile/ClsiManagerTests.coffee | 103 +----------- 9 files changed, 258 insertions(+), 188 deletions(-) create mode 100644 services/web/app/coffee/Features/Compile/ClsiFormatChecker.coffee create mode 100644 services/web/test/UnitTests/coffee/Compile/ClsiFormatCheckerTests.coffee diff --git a/services/web/app/coffee/Features/Compile/ClsiFormatChecker.coffee b/services/web/app/coffee/Features/Compile/ClsiFormatChecker.coffee new file mode 100644 index 0000000000..b285d0e6b7 --- /dev/null +++ b/services/web/app/coffee/Features/Compile/ClsiFormatChecker.coffee @@ -0,0 +1,68 @@ +_ = require("lodash") +async = require("async") + +module.exports = ClsiFormatChecker = + + checkRecoursesForProblems: (resources, callback)-> + jobs = + conflictedPaths: (cb)-> + ClsiFormatChecker._checkForConflictingPaths resources, cb + + sizeCheck: (cb)-> + ClsiFormatChecker._checkDocsAreUnderSizeLimit resources, cb + + async.series jobs, (err, problems)-> + if err? + return callback(err) + + problems = _.omitBy(problems, _.isEmpty) + + if _.isEmpty(problems) + return callback() + else + callback(null, problems) + + + _checkForConflictingPaths: (resources, callback)-> + paths = _.map(resources, 'path') + + conflicts = _.filter paths, (path)-> + matchingPaths = _.filter paths, (checkPath)-> + return checkPath.indexOf(path+"/") != -1 + + return matchingPaths.length > 0 + + conflictObjects = _.map conflicts, (conflict)-> + path:conflict + + callback null, conflictObjects + + _checkDocsAreUnderSizeLimit: (resources, callback)-> + + FIVEMB = 1000 * 1000 * 5 + + totalSize = 0 + + sizedResources = _.map resources, (resource)-> + result = {path:resource.path} + if resource.content? + result.size = resource.content.replace(/\n/g).length + result.kbSize = Math.ceil(result.size / 1000) + else + result.size = 0 + totalSize += result.size + return result + + tooLarge = totalSize > FIVEMB + if !tooLarge + return callback() + else + sizedResources = _.sortBy(sizedResources, "size").reverse().slice(0, 10) + return callback(null, {resources:sizedResources, totalSize:totalSize}) + + + + + + + diff --git a/services/web/app/coffee/Features/Compile/ClsiManager.coffee b/services/web/app/coffee/Features/Compile/ClsiManager.coffee index 18f313a368..6a40325e7b 100755 --- a/services/web/app/coffee/Features/Compile/ClsiManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiManager.coffee @@ -9,21 +9,21 @@ url = require("url") ClsiCookieManager = require("./ClsiCookieManager") _ = require("underscore") async = require("async") - +ClsiFormatChecker = require("./ClsiFormatChecker") module.exports = ClsiManager = - sendRequest: (project_id, options = {}, callback = (error, success) ->) -> + sendRequest: (project_id, options = {}, callback = (error, status, outputFiles, clsiServerId, validationProblems) ->) -> ClsiManager._buildRequest project_id, options, (error, req) -> return callback(error) if error? logger.log project_id: project_id, "sending compile to CLSI" - ClsiManager._checkRecoursesForErrors req.compile.resources, (err, problems)-> + 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 problems? - logger.log project_id:project_id, problems:problems, "problems with users latex before compile was attempted" - return callback(null, problems) + 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, req, options.compileGroup, (error, response) -> if error? logger.err err:error, project_id:project_id, "error sending request to clsi" @@ -162,79 +162,3 @@ module.exports = ClsiManager = logger.error err: error, project_id: project_id, "CLSI returned failure code" callback error, body - _checkRecoursesForErrors: (resources, callback)-> - jobs = - duplicatePaths: (cb)-> - ClsiManager._checkForDuplicatePaths resources, cb - - conflictedPaths: (cb)-> - ClsiManager._checkForConflictingPaths resources, cb - - sizeCheck: (cb)-> - ClsiManager._checkDocsAreUnderSizeLimit resources, cb - - async.series jobs, (err, problems)-> - if err? - return callback(err) - problems = _.omit(problems, _.isEmpty) - if _.isEmpty(problems) - return callback() - else - callback(null, problems) - - - _checkForDuplicatePaths: (resources, callback)-> - paths = _.pluck(resources, 'path') - - duplicates = _.filter paths, (path)-> - return _.countBy(paths)[path] > 1 - - duplicates = _.uniq(duplicates) - - duplicateObjects = _.map duplicates, (dup)-> - path:dup - - callback(null, duplicateObjects) - - _checkForConflictingPaths: (resources, callback)-> - paths = _.pluck(resources, 'path') - - conflicts = _.filter paths, (path)-> - matchingPaths = _.filter paths, (checkPath)-> - return checkPath.indexOf(path+"/") != -1 - - return matchingPaths.length > 0 - - conflictObjects = _.map conflicts, (conflict)-> - path:conflict - - callback null, conflictObjects - - _checkDocsAreUnderSizeLimit: (resources, callback)-> - - FIVEMB = 1000 * 1000 * 5 - - totalSize = 0 - - sizedResources = _.map resources, (resource)-> - result = {path:resource.path} - if resource.content? - result.size = resource.content.join("").length - else - result.size = 0 - totalSize += result.size - return result - - tooLarge = totalSize > FIVEMB - if !tooLarge - return callback() - else - sizedResources = _.sortBy(sizedResources, "size").slice(10).reverse() - return callback(null, {resources:sizedResources, totalSize:totalSize}) - - - - - - - diff --git a/services/web/app/coffee/Features/Compile/CompileController.coffee b/services/web/app/coffee/Features/Compile/CompileController.coffee index d961de554a..5e06af927f 100755 --- a/services/web/app/coffee/Features/Compile/CompileController.coffee +++ b/services/web/app/coffee/Features/Compile/CompileController.coffee @@ -30,7 +30,7 @@ module.exports = CompileController = if req.body?.draft options.draft = req.body.draft logger.log {options, project_id}, "got compile request" - CompileManager.compile project_id, user_id, options, (error, status, outputFiles, clsiServerId, limits) -> + CompileManager.compile project_id, user_id, options, (error, status, outputFiles, clsiServerId, limits, validationProblems) -> return next(error) if error? res.contentType("application/json") res.status(200).send JSON.stringify { @@ -38,6 +38,7 @@ module.exports = CompileController = outputFiles: outputFiles compileGroup: limits?.compileGroup clsiServerId:clsiServerId + validationProblems:validationProblems } downloadPdf: (req, res, next = (error) ->)-> diff --git a/services/web/app/coffee/Features/Compile/CompileManager.coffee b/services/web/app/coffee/Features/Compile/CompileManager.coffee index 0d8d480b7b..744d16364f 100755 --- a/services/web/app/coffee/Features/Compile/CompileManager.coffee +++ b/services/web/app/coffee/Features/Compile/CompileManager.coffee @@ -37,10 +37,10 @@ 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) -> + ClsiManager.sendRequest project_id, options, (error, status, outputFiles, clsiServerId, validationProblems) -> return callback(error) if error? logger.log files: outputFiles, "output files" - callback(null, status, outputFiles, clsiServerId, limits) + callback(null, status, outputFiles, clsiServerId, limits, validationProblems) deleteAuxFiles: (project_id, callback = (error) ->) -> CompileManager.getProjectCompileLimits project_id, (error, limits) -> diff --git a/services/web/app/views/project/editor/pdf.jade b/services/web/app/views/project/editor/pdf.jade index df97ff889e..f77753213a 100644 --- a/services/web/app/views/project/editor/pdf.jade +++ b/services/web/app/views/project/editor/pdf.jade @@ -154,6 +154,27 @@ div.full-size.pdf(ng-controller="PdfController") ng-if="settings.pdfViewer == 'native'" ) + .pdf-validation-problems(ng-switch-when="validation-problems") + + .alert.alert-danger(ng-show="pdf.validation.duplicatePaths") + strong #{translate("latex_error")} + span #{translate("duplicate_paths_found")} + + + .alert.alert-danger(ng-show="pdf.validation.sizeCheck") + strong #{translate("project_too_large")} + div #{translate("project_too_large_please_reduce")} + div + li(ng-repeat="entry in pdf.validation.sizeCheck.resources") {{ '/'+entry['path'] }} - {{entry['kbSize']}}kb + + .alert.alert-danger(ng-show="pdf.validation.conflictedPaths") + div + strong #{translate("conflicting_paths_found")} + div #{translate("following_paths_conflict")} + div + li(ng-repeat="entry in pdf.validation.conflictedPaths") {{ '/'+entry['path'] }} + + .pdf-errors(ng-switch-when="errors") .alert.alert-danger(ng-show="pdf.error") diff --git a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee index 4db21cd6f3..93447d22c9 100644 --- a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee +++ b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee @@ -42,11 +42,11 @@ define [ $scope.pdf.error = false $scope.pdf.timedout = false $scope.pdf.failure = false - $scope.pdf.projectTooLarge = false $scope.pdf.url = null $scope.pdf.clsiMaintenance = false $scope.pdf.tooRecentlyCompiled = false $scope.pdf.renderingError = false + $scope.pdf.projectTooLarge = false if response.status == "timedout" $scope.pdf.view = 'errors' @@ -60,13 +60,15 @@ define [ $scope.pdf.view = 'errors' $scope.pdf.failure = true $scope.shouldShowLogs = true - fetchLogs() else if response.status == 'clsi-maintenance' $scope.pdf.view = 'errors' $scope.pdf.clsiMaintenance = true else if response.status == "too-recently-compiled" $scope.pdf.view = 'errors' $scope.pdf.tooRecentlyCompiled = true + else if response.status == "validation-problems" + $scope.pdf.view = "validation-problems" + $scope.pdf.validation = response.validationProblems else if response.status == "success" $scope.pdf.view = 'pdf' $scope.shouldShowLogs = false diff --git a/services/web/public/stylesheets/app/editor/pdf.less b/services/web/public/stylesheets/app/editor/pdf.less index 353984de51..ed104a5c91 100644 --- a/services/web/public/stylesheets/app/editor/pdf.less +++ b/services/web/public/stylesheets/app/editor/pdf.less @@ -3,7 +3,7 @@ top: 58px; } -.pdf-logs, .pdf-errors, .pdf-uncompiled { +.pdf-logs, .pdf-errors, .pdf-uncompiled, .pdf-validation-problems{ padding: @line-height-computed / 2; } diff --git a/services/web/test/UnitTests/coffee/Compile/ClsiFormatCheckerTests.coffee b/services/web/test/UnitTests/coffee/Compile/ClsiFormatCheckerTests.coffee new file mode 100644 index 0000000000..d2f1c5ef3a --- /dev/null +++ b/services/web/test/UnitTests/coffee/Compile/ClsiFormatCheckerTests.coffee @@ -0,0 +1,151 @@ +sinon = require('sinon') +chai = require('chai') +should = chai.should() +expect = chai.expect +modulePath = "../../../../app/js/Features/Compile/ClsiFormatChecker.js" +SandboxedModule = require('sandboxed-module') + +describe "ClsiFormatChecker", -> + beforeEach -> + @ClsiFormatChecker = SandboxedModule.require modulePath, requires: + "settings-sharelatex": @settings ={} + "logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub(), warn: sinon.stub() } + @project_id = "project-id" + + + describe "checkRecoursesForProblems", -> + + beforeEach -> + @resources = [{ + path: "main.tex" + content: "stuff" + }, { + path: "chapters/chapter1" + content: "other stuff" + }, { + path: "stuff/image/image.png" + url: "http:somewhere.com/project/#{@project_id}/file/1234124321312" + modified: "more stuff" + }] + + it "should call _checkForDuplicatePaths and _checkForConflictingPaths", (done)-> + + @ClsiFormatChecker._checkForConflictingPaths = sinon.stub().callsArgWith(1, null) + @ClsiFormatChecker._checkDocsAreUnderSizeLimit = sinon.stub().callsArgWith(1) + @ClsiFormatChecker.checkRecoursesForProblems @resources, (err, problems)=> + @ClsiFormatChecker._checkForConflictingPaths.called.should.equal true + @ClsiFormatChecker._checkDocsAreUnderSizeLimit.called.should.equal true + done() + + it "should remove undefined errors", (done)-> + @ClsiFormatChecker._checkForConflictingPaths = sinon.stub().callsArgWith(1, null, []) + @ClsiFormatChecker._checkDocsAreUnderSizeLimit = sinon.stub().callsArgWith(1, null, {}) + @ClsiFormatChecker.checkRecoursesForProblems @resources, (err, problems)=> + expect(problems).to.not.exist + expect(problems).to.not.exist + done() + + it "should keep populated arrays", (done)-> + @ClsiFormatChecker._checkForConflictingPaths = sinon.stub().callsArgWith(1, null, [{path:"somewhere/main.tex"}]) + @ClsiFormatChecker._checkDocsAreUnderSizeLimit = sinon.stub().callsArgWith(1, null, {}) + @ClsiFormatChecker.checkRecoursesForProblems @resources, (err, problems)=> + problems.conflictedPaths[0].path.should.equal "somewhere/main.tex" + expect(problems.sizeCheck).to.not.exist + done() + + it "should keep populated object", (done)-> + @ClsiFormatChecker._checkForConflictingPaths = sinon.stub().callsArgWith(1, null, []) + @ClsiFormatChecker._checkDocsAreUnderSizeLimit = sinon.stub().callsArgWith(1, null, {resources:[{"a.tex"},{"b.tex"}], totalSize:1000000}) + @ClsiFormatChecker.checkRecoursesForProblems @resources, (err, problems)=> + problems.sizeCheck.resources.length.should.equal 2 + problems.sizeCheck.totalSize.should.equal 1000000 + expect(problems.conflictedPaths).to.not.exist + done() + + describe "_checkForConflictingPaths", -> + + beforeEach -> + + @resources.push({ + path: "chapters/chapter1.tex" + content: "other stuff" + }) + + @resources.push({ + path: "chapters.tex" + content: "other stuff" + }) + + it "should flag up when a nested file has folder with same subpath as file elsewhere", (done)-> + @resources.push({ + path: "stuff/image" + url: "http://somwhere.com" + }) + + @ClsiFormatChecker._checkForConflictingPaths @resources, (err, conflictPathErrors)-> + conflictPathErrors.length.should.equal 1 + conflictPathErrors[0].path.should.equal "stuff/image" + done() + + it "should flag up when a root level file has folder with same subpath as file elsewhere", (done)-> + @resources.push({ + path: "stuff" + content: "other stuff" + }) + + @ClsiFormatChecker._checkForConflictingPaths @resources, (err, conflictPathErrors)-> + conflictPathErrors.length.should.equal 1 + conflictPathErrors[0].path.should.equal "stuff" + done() + + it "should not flag up when the file is a substring of a path", (done)-> + @resources.push({ + path: "stuf" + content: "other stuff" + }) + + @ClsiFormatChecker._checkForConflictingPaths @resources, (err, conflictPathErrors)-> + conflictPathErrors.length.should.equal 0 + done() + + + describe "_checkDocsAreUnderSizeLimit", -> + + it "should error when there is more than 5mb of data", (done)-> + + @resources.push({ + path: "massive.tex" + content: require("crypto").randomBytes(1000 * 1000 * 5).toString("hex") + }) + + while @resources.length < 20 + @resources.push({path:"chapters/chapter1.tex",url: "http://somwhere.com"}) + + @ClsiFormatChecker._checkDocsAreUnderSizeLimit @resources, (err, sizeError)-> + sizeError.totalSize.should.equal 10000016 + sizeError.resources.length.should.equal 10 + sizeError.resources[0].path.should.equal "massive.tex" + sizeError.resources[0].size.should.equal 1000 * 1000 * 10 + done() + + + it "should return nothing when project is correct size", (done)-> + + @resources.push({ + path: "massive.tex" + content: require("crypto").randomBytes(1000 * 1000 * 1).toString("hex") + }) + + while @resources.length < 20 + @resources.push({path:"chapters/chapter1.tex",url: "http://somwhere.com"}) + + @ClsiFormatChecker._checkDocsAreUnderSizeLimit @resources, (err, sizeError)-> + expect(sizeError).to.not.exist + done() + + + + + + + diff --git a/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee b/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee index b31fb59fb5..211ef9b1f9 100644 --- a/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee @@ -12,6 +12,8 @@ describe "ClsiManager", -> getCookieJar: sinon.stub().callsArgWith(1, null, @jar) setServerId: sinon.stub().callsArgWith(2) _getServerId:sinon.stub() + @ClsiFormatChecker = + checkRecoursesForProblems:sinon.stub().callsArgWith(1) @ClsiManager = SandboxedModule.require modulePath, requires: "settings-sharelatex": @settings = apis: @@ -27,6 +29,7 @@ describe "ClsiManager", -> "./ClsiCookieManager": @ClsiCookieManager "logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub(), warn: sinon.stub() } "request": @request = sinon.stub() + "./ClsiFormatChecker": @ClsiFormatChecker @project_id = "project-id" @callback = sinon.stub() @@ -320,106 +323,6 @@ describe "ClsiManager", -> - describe "_checkRecoursesForErrors", -> - - beforeEach -> - @resources = [{ - path: "main.tex" - content: ["stuff"] - }, { - path: "chapters/chapter1" - content: ["other stuff"] - }, { - path: "stuff/image/image.png" - url: "#{@settings.apis.filestore.url}/project/#{@project_id}/file/1234124321312" - modified: ["more stuff"] - }] - - it "should call _checkForDuplicatePaths and _checkForConflictingPaths", (done)-> - - @ClsiManager._checkForDuplicatePaths = sinon.stub().callsArgWith(1) - @ClsiManager._checkForConflictingPaths = sinon.stub().callsArgWith(1) - @ClsiManager._checkDocsAreUnderSizeLimit = sinon.stub().callsArgWith(1) - @ClsiManager._checkRecoursesForErrors @resources, (err, problems)=> - @ClsiManager._checkForDuplicatePaths.called.should.equal true - @ClsiManager._checkForConflictingPaths.called.should.equal true - @ClsiManager._checkDocsAreUnderSizeLimit.called.should.equal true - expect(problems).to.not.exist - done() - - - it "should remove undefined errors", (done)-> - @ClsiManager._checkForDuplicatePaths = sinon.stub().callsArgWith(1, null, [path:"something/here"]) - @ClsiManager._checkForConflictingPaths = sinon.stub().callsArgWith(1, null, []) - @ClsiManager._checkDocsAreUnderSizeLimit = sinon.stub().callsArgWith(1, null) - @ClsiManager._checkRecoursesForErrors @resources, (err, problems)=> - problems.duplicatePaths[0].path.should.equal "something/here" - expect(problems.conflictedPaths).to.not.exist - expect(problems.sizeCheck).to.not.exist - - done() - - describe "_checkForDuplicatePaths", -> - - it "should flag up 2 nested files with same path", (done)-> - - @resources.push({ - path: "chapters/chapter1" - url: "http://somwhere.com" - }) - - @ClsiManager._checkForDuplicatePaths @resources, (err, duplicateErrors)-> - duplicateErrors.length.should.equal 1 - duplicateErrors[0].path.should.equal "chapters/chapter1" - done() - - describe "_checkForConflictingPaths", -> - - it "should flag up when a nested file has folder with same subpath as file elsewhere", (done)-> - @resources.push({ - path: "stuff/image" - url: "http://somwhere.com" - }) - - @resources.push({ - path: "chapters/chapter1.tex" - content: ["other stuff"] - }) - - @resources.push({ - path: "chapters.tex" - content: ["other stuff"] - }) - - @ClsiManager._checkForConflictingPaths @resources, (err, conflictPathErrors)-> - conflictPathErrors.length.should.equal 1 - conflictPathErrors[0].path.should.equal "stuff/image" - done() - - - describe "_checkDocsAreUnderSizeLimit", -> - - it "should error when there is more than 2mb of data", (done)-> - - @resources.push({ - path: "massive.tex" - content: [require("crypto").randomBytes(1000 * 1000 * 5).toString("hex")] - }) - - while @resources.length < 20 - @resources.push({path:"chapters/chapter1.tex",url: "http://somwhere.com"}) - - @ClsiManager._checkDocsAreUnderSizeLimit @resources, (err, sizeError)-> - sizeError.totalSize.should.equal 10000016 - sizeError.resources.length.should.equal 10 - sizeError.resources[0].path.should.equal "massive.tex" - sizeError.resources[0].size.should.equal 1000 * 1000 * 10 - done() - - - - - From 14cbf4f1c86d4423f41bed7ae4f79989cc9d29f6 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Thu, 2 Jun 2016 13:11:20 +0100 Subject: [PATCH 5/6] removed priority url from proxy to clsi --- .../Features/Compile/CompileController.coffee | 5 +-- .../Compile/CompileControllerTests.coffee | 42 ------------------- 2 files changed, 1 insertion(+), 46 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/CompileController.coffee b/services/web/app/coffee/Features/Compile/CompileController.coffee index 5e06af927f..ed6f5f56cd 100755 --- a/services/web/app/coffee/Features/Compile/CompileController.coffee +++ b/services/web/app/coffee/Features/Compile/CompileController.coffee @@ -115,10 +115,7 @@ module.exports = CompileController = if err? logger.err err:err, "error getting cookie jar for clsi request" return callback(err) - if limits.compileGroup == "priority" - compilerUrl = Settings.apis.clsi_priority.url - else - compilerUrl = Settings.apis.clsi.url + compilerUrl = Settings.apis.clsi.url url = "#{compilerUrl}#{url}" logger.log url: url, "proxying to CLSI" oneMinute = 60 * 1000 diff --git a/services/web/test/UnitTests/coffee/Compile/CompileControllerTests.coffee b/services/web/test/UnitTests/coffee/Compile/CompileControllerTests.coffee index 6bae5803c4..c724798a3e 100644 --- a/services/web/test/UnitTests/coffee/Compile/CompileControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Compile/CompileControllerTests.coffee @@ -206,16 +206,6 @@ describe "CompileController", -> @CompileManager.getProjectCompileLimits = sinon.stub().callsArgWith(1, null, {compileGroup: "priority"}) @CompileController.proxyToClsi(@project_id, @url = "/test", @req, @res, @next) - it "should proxy to the priority url if the user has the feature", ()-> - @request - .calledWith( - jar:@jar - method: @req.method - url: "#{@settings.apis.clsi_priority.url}#{@url}", - timeout: 60 * 1000 - ) - .should.equal true - describe "user with standard priority via query string", -> beforeEach -> @req.query = {compileGroup: 'standard'} @@ -239,20 +229,6 @@ describe "CompileController", -> it "should bind an error handle to the request proxy", -> @proxy.on.calledWith("error").should.equal true - describe "user with priority compile via query string", -> - beforeEach -> - @req.query = {compileGroup: 'priority'} - @CompileController.proxyToClsi(@project_id, @url = "/test", @req, @res, @next) - - it "should proxy to the priority url if the user has the feature", ()-> - @request - .calledWith( - jar:@jar - method: @req.method - url: "#{@settings.apis.clsi_priority.url}#{@url}", - timeout: 60 * 1000 - ) - .should.equal true describe "user with non-existent priority via query string", -> beforeEach -> @@ -316,25 +292,7 @@ describe "CompileController", -> it "should bind an error handle to the request proxy", -> @proxy.on.calledWith("error").should.equal true - describe "user with priority compile", -> - beforeEach -> - @CompileManager.getProjectCompileLimits = sinon.stub().callsArgWith(1, null, {compileGroup: "priority"}) - @CompileController.proxyToClsi(@project_id, @url = "/test", @req, @res, @next) - it "should proxy to the priority url if the user has the feature", ()-> - @request - .calledWith( - jar:@jar - method: @req.method - url: "#{@settings.apis.clsi_priority.url}#{@url}", - timeout: 60 * 1000 - headers: { - 'Range': '123-456' - 'If-Range': 'abcdef' - 'If-Modified-Since': 'Mon, 15 Dec 2014 15:23:56 GMT' - } - ) - .should.equal true describe "user with build parameter via query string", -> beforeEach -> From 8529cb50b621440dc781402f9666fad8616e7077 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Thu, 2 Jun 2016 13:18:07 +0100 Subject: [PATCH 6/6] rolled back underscore and added lodash in --- services/web/package.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/services/web/package.json b/services/web/package.json index 3f4f9e5c06..466782603d 100644 --- a/services/web/package.json +++ b/services/web/package.json @@ -29,6 +29,7 @@ "http-proxy": "^1.8.1", "jade": "~1.3.1", "ldapjs": "^1.0.0", + "lodash": "^4.13.1", "logger-sharelatex": "git+https://github.com/sharelatex/logger-sharelatex.git#v1.3.1", "lynx": "0.1.1", "marked": "^0.3.3", @@ -55,7 +56,7 @@ "sixpack-client": "^1.0.0", "temp": "^0.8.3", "translations-sharelatex": "git+https://github.com/sharelatex/translations-sharelatex.git#master", - "underscore": "1.8.3", + "underscore": "1.6.0", "v8-profiler": "^5.2.3", "xml2js": "0.2.0" },