From 3e28eca26fd6b0f23b394d73778fcbc94f5a8d23 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Fri, 28 Jul 2017 15:01:05 +0100 Subject: [PATCH 01/35] move docupdater flush to point of use --- .../Features/Compile/ClsiManager.coffee | 73 ++++++++++--------- .../Features/Compile/CompileManager.coffee | 21 +++--- .../coffee/Compile/ClsiManagerTests.coffee | 7 ++ .../coffee/Compile/CompileManagerTests.coffee | 7 -- 4 files changed, 54 insertions(+), 54 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/ClsiManager.coffee b/services/web/app/coffee/Features/Compile/ClsiManager.coffee index 08ef23bcd8..35b49881ac 100755 --- a/services/web/app/coffee/Features/Compile/ClsiManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiManager.coffee @@ -10,6 +10,7 @@ ClsiCookieManager = require("./ClsiCookieManager") _ = require("underscore") async = require("async") ClsiFormatChecker = require("./ClsiFormatChecker") +DocumentUpdaterHandler = require "../DocumentUpdater/DocumentUpdaterHandler" module.exports = ClsiManager = @@ -109,48 +110,50 @@ module.exports = ClsiManager = if project.compiler not in ClsiManager.VALID_COMPILERS project.compiler = "pdflatex" - ProjectEntityHandler.getAllDocs project_id, (error, docs = {}) -> + DocumentUpdaterHandler.flushProjectToMongo project_id, (error) -> return callback(error) if error? - ProjectEntityHandler.getAllFiles project_id, (error, files = {}) -> + ProjectEntityHandler.getAllDocs project_id, (error, docs = {}) -> return callback(error) if error? + ProjectEntityHandler.getAllFiles project_id, (error, files = {}) -> + return callback(error) if error? - resources = [] - rootResourcePath = null - rootResourcePathOverride = null + resources = [] + rootResourcePath = null + rootResourcePathOverride = null - for path, doc of docs - path = path.replace(/^\//, "") # Remove leading / - resources.push - path: path - content: doc.lines.join("\n") - if project.rootDoc_id? and doc._id.toString() == project.rootDoc_id.toString() - rootResourcePath = path - if options.rootDoc_id? and doc._id.toString() == options.rootDoc_id.toString() - rootResourcePathOverride = path + for path, doc of docs + path = path.replace(/^\//, "") # Remove leading / + resources.push + path: path + content: doc.lines.join("\n") + if project.rootDoc_id? and doc._id.toString() == project.rootDoc_id.toString() + rootResourcePath = path + if options.rootDoc_id? and doc._id.toString() == options.rootDoc_id.toString() + rootResourcePathOverride = path - rootResourcePath = rootResourcePathOverride if rootResourcePathOverride? - if !rootResourcePath? - logger.warn {project_id}, "no root document found, setting to main.tex" - rootResourcePath = "main.tex" + rootResourcePath = rootResourcePathOverride if rootResourcePathOverride? + if !rootResourcePath? + logger.warn {project_id}, "no root document found, setting to main.tex" + rootResourcePath = "main.tex" - for path, file of files - path = path.replace(/^\//, "") # Remove leading / - resources.push - path: path - url: "#{Settings.apis.filestore.url}/project/#{project._id}/file/#{file._id}" - modified: file.created?.getTime() + for path, file of files + path = path.replace(/^\//, "") # Remove leading / + resources.push + path: path + url: "#{Settings.apis.filestore.url}/project/#{project._id}/file/#{file._id}" + modified: file.created?.getTime() - callback null, { - compile: - options: - compiler: project.compiler - timeout: options.timeout - imageName: project.imageName - draft: !!options.draft - check: options.check - rootResourcePath: rootResourcePath - resources: resources - } + callback null, { + compile: + options: + compiler: project.compiler + timeout: options.timeout + imageName: project.imageName + draft: !!options.draft + check: options.check + rootResourcePath: rootResourcePath + resources: resources + } wordCount: (project_id, user_id, file, options, callback = (error, response) ->) -> ClsiManager._buildRequest project_id, options, (error, req) -> diff --git a/services/web/app/coffee/Features/Compile/CompileManager.coffee b/services/web/app/coffee/Features/Compile/CompileManager.coffee index ad8a2459ec..61fe7038a6 100755 --- a/services/web/app/coffee/Features/Compile/CompileManager.coffee +++ b/services/web/app/coffee/Features/Compile/CompileManager.coffee @@ -1,7 +1,6 @@ Settings = require('settings-sharelatex') RedisWrapper = require("../../infrastructure/RedisWrapper") rclient = RedisWrapper.client("clsi_recently_compiled") -DocumentUpdaterHandler = require "../DocumentUpdater/DocumentUpdaterHandler" Project = require("../../models/Project").Project ProjectRootDocManager = require "../Project/ProjectRootDocManager" UserGetter = require "../User/UserGetter" @@ -31,19 +30,17 @@ module.exports = CompileManager = CompileManager._ensureRootDocumentIsSet project_id, (error) -> return callback(error) if error? - DocumentUpdaterHandler.flushProjectToMongo project_id, (error) -> + CompileManager.getProjectCompileLimits project_id, (error, limits) -> return callback(error) if error? - CompileManager.getProjectCompileLimits project_id, (error, limits) -> + for key, value of limits + options[key] = value + # only pass user_id down to clsi if this is a per-user compile + compileAsUser = if Settings.disablePerUserCompiles then undefined else user_id + ClsiManager.sendRequest project_id, compileAsUser, options, (error, status, outputFiles, clsiServerId, validationProblems) -> return callback(error) if error? - for key, value of limits - options[key] = value - # only pass user_id down to clsi if this is a per-user compile - compileAsUser = if Settings.disablePerUserCompiles then undefined else user_id - ClsiManager.sendRequest project_id, compileAsUser, options, (error, status, outputFiles, clsiServerId, validationProblems) -> - return callback(error) if error? - logger.log files: outputFiles, "output files" - callback(null, status, outputFiles, clsiServerId, limits, validationProblems) - + logger.log files: outputFiles, "output files" + callback(null, status, outputFiles, clsiServerId, limits, validationProblems) + stopCompile: (project_id, user_id, callback = (error) ->) -> CompileManager.getProjectCompileLimits project_id, (error, limits) -> diff --git a/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee b/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee index a3eeebcaaa..6bcde76444 100644 --- a/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee @@ -26,6 +26,7 @@ describe "ClsiManager", -> url: "https://clsipremium.example.com" "../../models/Project": Project: @Project = {} "../Project/ProjectEntityHandler": @ProjectEntityHandler = {} + "../DocumentUpdater/DocumentUpdaterHandler": @DocumentUpdaterHandler = {} "./ClsiCookieManager": @ClsiCookieManager "logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub(), warn: sinon.stub() } "request": @request = sinon.stub() @@ -144,6 +145,7 @@ describe "ClsiManager", -> @Project.findById = sinon.stub().callsArgWith(2, null, @project) @ProjectEntityHandler.getAllDocs = sinon.stub().callsArgWith(1, null, @docs) @ProjectEntityHandler.getAllFiles = sinon.stub().callsArgWith(1, null, @files) + @DocumentUpdaterHandler.flushProjectToMongo = sinon.stub().callsArgWith(1, null) describe "with a valid project", -> beforeEach (done) -> @@ -156,6 +158,11 @@ describe "ClsiManager", -> .calledWith(@project_id, {compiler:1, rootDoc_id: 1, imageName: 1}) .should.equal true + it "should flush the project to the database", -> + @DocumentUpdaterHandler.flushProjectToMongo + .calledWith(@project_id) + .should.equal true + it "should get all the docs", -> @ProjectEntityHandler.getAllDocs .calledWith(@project_id) diff --git a/services/web/test/UnitTests/coffee/Compile/CompileManagerTests.coffee b/services/web/test/UnitTests/coffee/Compile/CompileManagerTests.coffee index 3964acea41..21327a50c9 100644 --- a/services/web/test/UnitTests/coffee/Compile/CompileManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Compile/CompileManagerTests.coffee @@ -17,7 +17,6 @@ describe "CompileManager", -> redis: web: {host: "localhost", port: 42} "../../infrastructure/RedisWrapper": client: () => @rclient = { auth: () -> } - "../DocumentUpdater/DocumentUpdaterHandler": @DocumentUpdaterHandler = {} "../Project/ProjectRootDocManager": @ProjectRootDocManager = {} "../../models/Project": Project: @Project = {} "../User/UserGetter": @UserGetter = {} @@ -40,7 +39,6 @@ describe "CompileManager", -> beforeEach -> @CompileManager._checkIfRecentlyCompiled = sinon.stub().callsArgWith(2, null, false) @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(3, null, @status = "mock-status", @outputFiles = "mock output files", @output = "mock output") @@ -54,11 +52,6 @@ describe "CompileManager", -> .calledWith(@project_id, @user_id) .should.equal true - it "should flush the project to the database", -> - @DocumentUpdaterHandler.flushProjectToMongo - .calledWith(@project_id) - .should.equal true - it "should ensure that the root document is set", -> @CompileManager._ensureRootDocumentIsSet .calledWith(@project_id) From d66382382f477a92404246cff0eb1f2020b3f0ab Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Fri, 28 Jul 2017 15:17:10 +0100 Subject: [PATCH 02/35] split request to mongo into separate method --- .../Features/Compile/ClsiManager.coffee | 82 ++++++++++--------- 1 file changed, 44 insertions(+), 38 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/ClsiManager.coffee b/services/web/app/coffee/Features/Compile/ClsiManager.coffee index 35b49881ac..7caa9cc174 100755 --- a/services/web/app/coffee/Features/Compile/ClsiManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiManager.coffee @@ -110,50 +110,56 @@ module.exports = ClsiManager = if project.compiler not in ClsiManager.VALID_COMPILERS project.compiler = "pdflatex" - DocumentUpdaterHandler.flushProjectToMongo project_id, (error) -> + ClsiManager._getContentFromMongo project_id, (error, docs, files) -> return callback(error) if error? - ProjectEntityHandler.getAllDocs project_id, (error, docs = {}) -> - return callback(error) if error? - ProjectEntityHandler.getAllFiles project_id, (error, files = {}) -> - return callback(error) if error? + ClsiManager._finaliseRequest project_id, options, project, docs, files, callback - resources = [] - rootResourcePath = null - rootResourcePathOverride = null + _getContentFromMongo: (project_id, callback = (error, docs, files) ->) -> + DocumentUpdaterHandler.flushProjectToMongo project_id, (error) -> + return callback(error) if error? + ProjectEntityHandler.getAllDocs project_id, (error, docs = {}) -> + return callback(error) if error? + ProjectEntityHandler.getAllFiles project_id, (error, files = {}) -> + callback(null, docs, files) - for path, doc of docs - path = path.replace(/^\//, "") # Remove leading / - resources.push - path: path - content: doc.lines.join("\n") - if project.rootDoc_id? and doc._id.toString() == project.rootDoc_id.toString() - rootResourcePath = path - if options.rootDoc_id? and doc._id.toString() == options.rootDoc_id.toString() - rootResourcePathOverride = path + _finaliseRequest: (project_id, options, project, docs, files, callback = (error, params) -> ) -> + resources = [] + rootResourcePath = null + rootResourcePathOverride = null - rootResourcePath = rootResourcePathOverride if rootResourcePathOverride? - if !rootResourcePath? - logger.warn {project_id}, "no root document found, setting to main.tex" - rootResourcePath = "main.tex" + for path, doc of docs + path = path.replace(/^\//, "") # Remove leading / + resources.push + path: path + content: doc.lines.join("\n") + if project.rootDoc_id? and doc._id.toString() == project.rootDoc_id.toString() + rootResourcePath = path + if options.rootDoc_id? and doc._id.toString() == options.rootDoc_id.toString() + rootResourcePathOverride = path - for path, file of files - path = path.replace(/^\//, "") # Remove leading / - resources.push - path: path - url: "#{Settings.apis.filestore.url}/project/#{project._id}/file/#{file._id}" - modified: file.created?.getTime() + rootResourcePath = rootResourcePathOverride if rootResourcePathOverride? + if !rootResourcePath? + logger.warn {project_id}, "no root document found, setting to main.tex" + rootResourcePath = "main.tex" - callback null, { - compile: - options: - compiler: project.compiler - timeout: options.timeout - imageName: project.imageName - draft: !!options.draft - check: options.check - rootResourcePath: rootResourcePath - resources: resources - } + for path, file of files + path = path.replace(/^\//, "") # Remove leading / + resources.push + path: path + url: "#{Settings.apis.filestore.url}/project/#{project._id}/file/#{file._id}" + modified: file.created?.getTime() + + callback null, { + compile: + options: + compiler: project.compiler + timeout: options.timeout + imageName: project.imageName + draft: !!options.draft + check: options.check + rootResourcePath: rootResourcePath + resources: resources + } wordCount: (project_id, user_id, file, options, callback = (error, response) ->) -> ClsiManager._buildRequest project_id, options, (error, req) -> From 7a39eeb2ea27b38239160f1bf955c026bdc7f51e Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 1 Aug 2017 14:38:34 +0100 Subject: [PATCH 03/35] make request to docupdater for current docs --- .../DocumentUpdaterHandler.coffee | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee index f3cbe4be7f..b3247ba417 100644 --- a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee +++ b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee @@ -123,6 +123,29 @@ module.exports = DocumentUpdaterHandler = logger.error project_id:project_id, doc_id:doc_id, url: url, "doc updater returned a non-success status code: #{res.statusCode}" callback new Error("doc updater returned a non-success status code: #{res.statusCode}") + getProjectDocs: (project_id, callback = (error, docs) ->) -> + timer = new metrics.Timer("get-project-docs") + url = "#{settings.apis.documentupdater.url}/project/#{project_id}" + logger.log project_id:project_id, "getting project docs from document updater" + request.get url, (error, res, body)-> + timer.done() + if error? + logger.error err:error, url:url, project_id:project_id, "error getting project docs from doc updater" + return callback(error) + if res.statusCode >= 200 and res.statusCode < 300 + logger.log project_id:project_id, "got project docs from document document updater" + try + docs = JSON.parse(body) + for doc in docs or [] + doc.lines = JSON.parse(doc.lines) + catch error + return callback(error) + logger.log project_id: project_id, docs: docs, "RESULT" + callback null, docs + else + logger.error project_id:project_id, url: url, "doc updater returned a non-success status code: #{res.statusCode}" + callback new Error("doc updater returned a non-success status code: #{res.statusCode}") + acceptChanges: (project_id, doc_id, change_ids = [], callback = (error) ->) -> timer = new metrics.Timer("accept-changes") reqSettings = From 66cd6ada14e91e43667cbf01984a99431832d226 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 1 Aug 2017 14:39:02 +0100 Subject: [PATCH 04/35] allow querying folders from existing project avoid loading the project unnecessarily --- .../Project/ProjectEntityHandler.coffee | 26 +++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index 5868d6941a..bf4ac3d29f 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -31,8 +31,7 @@ module.exports = ProjectEntityHandler = ProjectGetter.getProjectWithoutDocLines project_id, (err, project) -> return callback(err) if err? return callback("no project") if !project? - processFolder "/", project.rootFolder[0] - callback null, folders + ProjectEntityHandler.getAllFoldersFromProject project, callback getAllDocs: (project_id, callback) -> logger.log project_id:project_id, "getting all docs for project" @@ -74,6 +73,29 @@ module.exports = ProjectEntityHandler = files[path.join(folderPath, file.name)] = file callback null, files + getAllFoldersFromProject: (project, callback) -> + folders = {} + processFolder = (basePath, folder) -> + folders[basePath] = folder + for childFolder in (folder.folders or []) + if childFolder.name? + processFolder path.join(basePath, childFolder.name), childFolder + + processFolder "/", project.rootFolder[0] + callback null, folders + + getAllDocPathsFromProject: (project, callback) -> + logger.log project:project, "getting all docs for project" + @getAllFoldersFromProject project, (err, folders = {}) -> + return callback(err) if err? + docPath = {} + for folderPath, folder of folders + for doc in (folder.docs or []) + console.log "PATH", path.join(folderPath, doc.name), doc._id, doc.name + docPath[doc._id] = path.join(folderPath, doc.name) + logger.log count:_.keys(docPath).length, project_id:project._id, "returning docPaths for project" + callback null, docPath + flushProjectToThirdPartyDataStore: (project_id, callback) -> self = @ logger.log project_id:project_id, "flushing project to tpds" From cf780fd8bb927731e6e8b955e4ae22f2371709e7 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 1 Aug 2017 14:39:57 +0100 Subject: [PATCH 05/35] start making requests incremental --- .../Features/Compile/ClsiManager.coffee | 67 +++++++++++++++++-- .../Features/Compile/ClsiStateManager.coffee | 41 ++++++++++++ 2 files changed, 101 insertions(+), 7 deletions(-) create mode 100644 services/web/app/coffee/Features/Compile/ClsiStateManager.coffee diff --git a/services/web/app/coffee/Features/Compile/ClsiManager.coffee b/services/web/app/coffee/Features/Compile/ClsiManager.coffee index 7caa9cc174..6f9b765554 100755 --- a/services/web/app/coffee/Features/Compile/ClsiManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiManager.coffee @@ -7,6 +7,7 @@ ProjectEntityHandler = require("../Project/ProjectEntityHandler") logger = require "logger-sharelatex" Url = require("url") ClsiCookieManager = require("./ClsiCookieManager") +ClsiStateManager = require("./ClsiStateManager") _ = require("underscore") async = require("async") ClsiFormatChecker = require("./ClsiFormatChecker") @@ -14,10 +15,20 @@ DocumentUpdaterHandler = require "../DocumentUpdater/DocumentUpdaterHandler" module.exports = ClsiManager = - sendRequest: (project_id, user_id, options = {}, callback = (error, status, outputFiles, clsiServerId, validationProblems) ->) -> - ClsiManager._buildRequest project_id, options, (error, req) -> + sendRequest: (project_id, user_id, options = {}, callback) -> + ClsiManager.sendRequestOnce project_id, user_id, _.clone(options), (error, status, result...) -> + return callback(error) if error? + if status is 'conflict' + options.state = "conflict" # will force full compile + ClsiManager.sendRequestOnce project_id, user_id, options, callback # try again + else + callback(error, status, result...) + + sendRequestOnce: (project_id, user_id, options = {}, callback = (error, status, outputFiles, clsiServerId, validationProblems) ->) -> + ClsiManager._buildRequest project_id, user_id, options, (error, req) -> return callback(error) if error? logger.log project_id: project_id, "sending compile to CLSI" + console.log "REQUEST", JSON.stringify(req, null, 2) 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" @@ -29,6 +40,9 @@ module.exports = ClsiManager = if error? logger.err err:error, project_id:project_id, "error sending request to clsi" return callback(error) + if response?.compile?.status is "conflict" + # FIXME try again without incremental option + console.log "CONFLICT TRY AGAIN" 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? @@ -86,6 +100,8 @@ module.exports = ClsiManager = callback null, body else if response.statusCode == 413 callback null, compile:status:"project-too-large" + else if response.statusCode == 409 + callback null, compile:status:"conflict" else error = new Error("CLSI returned non-success code: #{response.statusCode}") logger.error err: error, project_id: project_id, "CLSI returned failure code" @@ -102,17 +118,50 @@ module.exports = ClsiManager = return outputFiles VALID_COMPILERS: ["pdflatex", "latex", "xelatex", "lualatex"] - _buildRequest: (project_id, options={}, callback = (error, request) ->) -> - Project.findById project_id, {compiler: 1, rootDoc_id: 1, imageName: 1}, (error, project) -> + + _buildRequest: (project_id, user_id, options={}, callback = (error, request) ->) -> + Project.findById project_id, {}, (error, project) -> return callback(error) if error? return callback(new Errors.NotFoundError("project does not exist: #{project_id}")) if !project? - + console.log "PROJECT", project, JSON.stringify(project.rootFolder,null,2) if project.compiler not in ClsiManager.VALID_COMPILERS project.compiler = "pdflatex" - ClsiManager._getContentFromMongo project_id, (error, docs, files) -> + ClsiStateManager.checkState project_id, user_id, project, (error, stateOk, state) -> return callback(error) if error? - ClsiManager._finaliseRequest project_id, options, project, docs, files, callback + logger.log project_id: project_id, checkState: stateOk, "checked project state" + console.log "OPTIONS ARE", options + if stateOk and not options.state? # incremental + ClsiManager._getContentFromDocUpdater project_id, (error, docUpdaterDocs) -> + return callback(error) if error? + # make this incremental + ProjectEntityHandler.getAllDocPathsFromProject project, (error, docPath) -> + return callback(error) if error? + console.log "PATHS", docPath + console.log "DOCS", docUpdaterDocs + docs = {} + for doc in docUpdaterDocs or [] + path = docPath[doc._id] + docs[path] = doc + console.log "MAPPED DOCS", docs + options.incremental = state + ClsiManager._finaliseRequest project_id, options, project, docs, [], callback + else + ClsiManager._getContentFromMongo project_id, (error, docs, files) -> + return callback(error) if error? + console.log "DOCS", docs + # FIXME want to store state after project has been sent to clsi + ClsiStateManager.setState project_id, user_id, project, (error, state) -> + if error? + logger.err err:error, project_id:project_id, "error storing state in redis" + #return callback(error) + options.state = state + ClsiManager._finaliseRequest project_id, options, project, docs, files, callback + + _getContentFromDocUpdater: (project_id, callback = (error, docs) ->) -> + DocumentUpdaterHandler.getProjectDocs project_id, (error, docs) -> + return callback(error) if error? + callback(null, docs) _getContentFromMongo: (project_id, callback = (error, docs, files) ->) -> DocumentUpdaterHandler.flushProjectToMongo project_id, (error) -> @@ -120,6 +169,7 @@ module.exports = ClsiManager = ProjectEntityHandler.getAllDocs project_id, (error, docs = {}) -> return callback(error) if error? ProjectEntityHandler.getAllFiles project_id, (error, files = {}) -> + return callback(error) if error? callback(null, docs, files) _finaliseRequest: (project_id, options, project, docs, files, callback = (error, params) -> ) -> @@ -132,6 +182,7 @@ module.exports = ClsiManager = resources.push path: path content: doc.lines.join("\n") + rev: doc.rev if project.rootDoc_id? and doc._id.toString() == project.rootDoc_id.toString() rootResourcePath = path if options.rootDoc_id? and doc._id.toString() == options.rootDoc_id.toString() @@ -157,6 +208,8 @@ module.exports = ClsiManager = imageName: project.imageName draft: !!options.draft check: options.check + incremental: options.incremental + state: options.state rootResourcePath: rootResourcePath resources: resources } diff --git a/services/web/app/coffee/Features/Compile/ClsiStateManager.coffee b/services/web/app/coffee/Features/Compile/ClsiStateManager.coffee new file mode 100644 index 0000000000..f090ec6fc2 --- /dev/null +++ b/services/web/app/coffee/Features/Compile/ClsiStateManager.coffee @@ -0,0 +1,41 @@ +Settings = require "settings-sharelatex" +RedisWrapper = require("../../infrastructure/RedisWrapper") +rclient = RedisWrapper.client("clsi_state") +logger = require "logger-sharelatex" +crypto = require "crypto" + +buildKey = (project_id, user_id)-> + return "clsistate:#{project_id}-#{user_id}" # FIXME: should we cluster these on project?? + +buildState = (project) -> + JSON.stringify project + +clsiStateEnabled = Settings.clsiState + +OneHour = 3600 * 1000 + +module.exports = ClsiStateManager = + + checkState: (project_id, user_id, project, callback = (err, ok) ->) -> + newState = buildState(project) + @getState project_id, user_id, (err, oldState) -> + return callback(err) if err? + if newState is oldState + hash = crypto.createHash('sha1').update(newState, 'utf8').digest('hex') + callback(null,true,hash) + else + callback(null,false) + + getState: (project_id, user_id, callback = (err, state)->)-> + rclient.get buildKey(project_id, user_id), (err, state)-> + return callback(err) if err? + logger.log project_id: project_id, user_id: user_id, state: state, "got project state from redis" + return callback(null, state) + + setState: (project_id, user_id, project, callback = (err)->)-> + projectState = buildState project + logger.log project_id: project_id, user_id: user_id, projectState: projectState, "setting project state in redis" + rclient.set buildKey(project_id, user_id), projectState, "PX", OneHour, (err) -> + return callback(err) if err? + hash = crypto.createHash('sha1').update(projectState, 'utf8').digest('hex') + callback(null,hash) From 4d4cf4f6933720f6aaef035738670a27e5fac6cf Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Wed, 2 Aug 2017 16:25:47 +0100 Subject: [PATCH 06/35] project state can be stored per project there is no need to store it per project+user because it reflects the state of the project itself --- .../Features/Compile/ClsiManager.coffee | 8 ++++---- .../Features/Compile/ClsiStateManager.coffee | 20 +++++++++---------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/ClsiManager.coffee b/services/web/app/coffee/Features/Compile/ClsiManager.coffee index 6f9b765554..2adbbf2e53 100755 --- a/services/web/app/coffee/Features/Compile/ClsiManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiManager.coffee @@ -25,7 +25,7 @@ module.exports = ClsiManager = callback(error, status, result...) sendRequestOnce: (project_id, user_id, options = {}, callback = (error, status, outputFiles, clsiServerId, validationProblems) ->) -> - ClsiManager._buildRequest project_id, user_id, options, (error, req) -> + ClsiManager._buildRequest project_id, options, (error, req) -> return callback(error) if error? logger.log project_id: project_id, "sending compile to CLSI" console.log "REQUEST", JSON.stringify(req, null, 2) @@ -119,7 +119,7 @@ module.exports = ClsiManager = VALID_COMPILERS: ["pdflatex", "latex", "xelatex", "lualatex"] - _buildRequest: (project_id, user_id, options={}, callback = (error, request) ->) -> + _buildRequest: (project_id, options={}, callback = (error, request) ->) -> Project.findById project_id, {}, (error, project) -> return callback(error) if error? return callback(new Errors.NotFoundError("project does not exist: #{project_id}")) if !project? @@ -127,7 +127,7 @@ module.exports = ClsiManager = if project.compiler not in ClsiManager.VALID_COMPILERS project.compiler = "pdflatex" - ClsiStateManager.checkState project_id, user_id, project, (error, stateOk, state) -> + ClsiStateManager.checkState project_id, project, (error, stateOk, state) -> return callback(error) if error? logger.log project_id: project_id, checkState: stateOk, "checked project state" console.log "OPTIONS ARE", options @@ -151,7 +151,7 @@ module.exports = ClsiManager = return callback(error) if error? console.log "DOCS", docs # FIXME want to store state after project has been sent to clsi - ClsiStateManager.setState project_id, user_id, project, (error, state) -> + ClsiStateManager.setState project_id, project, (error, state) -> if error? logger.err err:error, project_id:project_id, "error storing state in redis" #return callback(error) diff --git a/services/web/app/coffee/Features/Compile/ClsiStateManager.coffee b/services/web/app/coffee/Features/Compile/ClsiStateManager.coffee index f090ec6fc2..9f4c558eb6 100644 --- a/services/web/app/coffee/Features/Compile/ClsiStateManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiStateManager.coffee @@ -4,8 +4,8 @@ rclient = RedisWrapper.client("clsi_state") logger = require "logger-sharelatex" crypto = require "crypto" -buildKey = (project_id, user_id)-> - return "clsistate:#{project_id}-#{user_id}" # FIXME: should we cluster these on project?? +buildKey = (project_id)-> + return "clsistate:#{project_id}" # FIXME: should we cluster these on project?? buildState = (project) -> JSON.stringify project @@ -16,9 +16,9 @@ OneHour = 3600 * 1000 module.exports = ClsiStateManager = - checkState: (project_id, user_id, project, callback = (err, ok) ->) -> + checkState: (project_id, project, callback = (err, ok) ->) -> newState = buildState(project) - @getState project_id, user_id, (err, oldState) -> + @getState project_id, (err, oldState) -> return callback(err) if err? if newState is oldState hash = crypto.createHash('sha1').update(newState, 'utf8').digest('hex') @@ -26,16 +26,16 @@ module.exports = ClsiStateManager = else callback(null,false) - getState: (project_id, user_id, callback = (err, state)->)-> - rclient.get buildKey(project_id, user_id), (err, state)-> + getState: (project_id, callback = (err, state)->)-> + rclient.get buildKey(project_id), (err, state)-> return callback(err) if err? - logger.log project_id: project_id, user_id: user_id, state: state, "got project state from redis" + logger.log project_id: project_id, state: state, "got project state from redis" return callback(null, state) - setState: (project_id, user_id, project, callback = (err)->)-> + setState: (project_id, project, callback = (err)->)-> projectState = buildState project - logger.log project_id: project_id, user_id: user_id, projectState: projectState, "setting project state in redis" - rclient.set buildKey(project_id, user_id), projectState, "PX", OneHour, (err) -> + logger.log project_id: project_id, projectState: projectState, "setting project state in redis" + rclient.set buildKey(project_id), projectState, "PX", OneHour, (err) -> return callback(err) if err? hash = crypto.createHash('sha1').update(projectState, 'utf8').digest('hex') callback(null,hash) From 17b1075dc99630bf4cdf66a8bdb3a5741c5bc9e3 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 3 Aug 2017 10:20:57 +0100 Subject: [PATCH 07/35] add rootFolder to attributes in Clsi request --- services/web/app/coffee/Features/Compile/ClsiManager.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/app/coffee/Features/Compile/ClsiManager.coffee b/services/web/app/coffee/Features/Compile/ClsiManager.coffee index 2adbbf2e53..2c3d478eef 100755 --- a/services/web/app/coffee/Features/Compile/ClsiManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiManager.coffee @@ -120,7 +120,7 @@ module.exports = ClsiManager = VALID_COMPILERS: ["pdflatex", "latex", "xelatex", "lualatex"] _buildRequest: (project_id, options={}, callback = (error, request) ->) -> - Project.findById project_id, {}, (error, project) -> + Project.findById project_id, {compiler: 1, rootDoc_id: 1, imageName: 1, rootFolder:1}, (error, project) -> return callback(error) if error? return callback(new Errors.NotFoundError("project does not exist: #{project_id}")) if !project? console.log "PROJECT", project, JSON.stringify(project.rootFolder,null,2) From a4117487e99922b1e46837324e3c552cc67ca51f Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 3 Aug 2017 10:22:11 +0100 Subject: [PATCH 08/35] switch from mongoose to mongojs in ClsiManager for efficiency --- services/web/app/coffee/Features/Compile/ClsiManager.coffee | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/services/web/app/coffee/Features/Compile/ClsiManager.coffee b/services/web/app/coffee/Features/Compile/ClsiManager.coffee index 2c3d478eef..77c2dc3dd8 100755 --- a/services/web/app/coffee/Features/Compile/ClsiManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiManager.coffee @@ -3,6 +3,7 @@ async = require "async" Settings = require "settings-sharelatex" request = require('request') Project = require("../../models/Project").Project +ProjectGetter = require("../Project/ProjectGetter") ProjectEntityHandler = require("../Project/ProjectEntityHandler") logger = require "logger-sharelatex" Url = require("url") @@ -120,7 +121,7 @@ module.exports = ClsiManager = VALID_COMPILERS: ["pdflatex", "latex", "xelatex", "lualatex"] _buildRequest: (project_id, options={}, callback = (error, request) ->) -> - Project.findById project_id, {compiler: 1, rootDoc_id: 1, imageName: 1, rootFolder:1}, (error, project) -> + ProjectGetter.getProject project_id, {compiler: 1, rootDoc_id: 1, imageName: 1, rootFolder:1}, (error, project) -> return callback(error) if error? return callback(new Errors.NotFoundError("project does not exist: #{project_id}")) if !project? console.log "PROJECT", project, JSON.stringify(project.rootFolder,null,2) From a955b8fcc9f360d312dd601efe242d442a970643 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 3 Aug 2017 10:30:50 +0100 Subject: [PATCH 09/35] remove unused inline function --- .../coffee/Features/Project/ProjectEntityHandler.coffee | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index bf4ac3d29f..88a8642358 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -20,14 +20,7 @@ CooldownManager = require '../Cooldown/CooldownManager' module.exports = ProjectEntityHandler = getAllFolders: (project_id, callback) -> - logger.log project_id:project_id, "getting all folders for project" - folders = {} - processFolder = (basePath, folder) -> - folders[basePath] = folder - for childFolder in (folder.folders or []) - if childFolder.name? - processFolder path.join(basePath, childFolder.name), childFolder - + logger.log project_id:project_id, "getting all folders for project" ProjectGetter.getProjectWithoutDocLines project_id, (err, project) -> return callback(err) if err? return callback("no project") if !project? From 0a859d3b33b1898d638bab100c05373d4e436b5c Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 3 Aug 2017 10:51:21 +0100 Subject: [PATCH 10/35] clean up state manager --- .../Features/Compile/ClsiManager.coffee | 9 +++-- .../Features/Compile/ClsiStateManager.coffee | 34 +++++++++++-------- 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/ClsiManager.coffee b/services/web/app/coffee/Features/Compile/ClsiManager.coffee index 77c2dc3dd8..2341129481 100755 --- a/services/web/app/coffee/Features/Compile/ClsiManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiManager.coffee @@ -128,10 +128,9 @@ module.exports = ClsiManager = if project.compiler not in ClsiManager.VALID_COMPILERS project.compiler = "pdflatex" - ClsiStateManager.checkState project_id, project, (error, stateOk, state) -> + ClsiStateManager.checkProjectStateMatch project_id, project, (error, stateOk, projectState) -> return callback(error) if error? logger.log project_id: project_id, checkState: stateOk, "checked project state" - console.log "OPTIONS ARE", options if stateOk and not options.state? # incremental ClsiManager._getContentFromDocUpdater project_id, (error, docUpdaterDocs) -> return callback(error) if error? @@ -145,18 +144,18 @@ module.exports = ClsiManager = path = docPath[doc._id] docs[path] = doc console.log "MAPPED DOCS", docs - options.incremental = state + options.incremental = projectState ClsiManager._finaliseRequest project_id, options, project, docs, [], callback else ClsiManager._getContentFromMongo project_id, (error, docs, files) -> return callback(error) if error? console.log "DOCS", docs # FIXME want to store state after project has been sent to clsi - ClsiStateManager.setState project_id, project, (error, state) -> + ClsiStateManager.setProjectState project_id, project, (error, projectState) -> if error? logger.err err:error, project_id:project_id, "error storing state in redis" #return callback(error) - options.state = state + options.state = projectState ClsiManager._finaliseRequest project_id, options, project, docs, files, callback _getContentFromDocUpdater: (project_id, callback = (error, docs) ->) -> diff --git a/services/web/app/coffee/Features/Compile/ClsiStateManager.coffee b/services/web/app/coffee/Features/Compile/ClsiStateManager.coffee index 9f4c558eb6..8347e9348b 100644 --- a/services/web/app/coffee/Features/Compile/ClsiStateManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiStateManager.coffee @@ -7,8 +7,19 @@ crypto = require "crypto" buildKey = (project_id)-> return "clsistate:#{project_id}" # FIXME: should we cluster these on project?? +# the "state" of a project is a hash of the relevant attributes in the +# project object in this case we only need the rootFolder. +# +# When it changes the full set of files on the CLSI will need to be +# updated. If it doesn't change then we can overwrite changed docs in +# place on the clsi, getting them from the docupdater. +# +# The docupdater is also responsible for unsetting the key in redis if +# it removes any documents from the doc updater. + buildState = (project) -> - JSON.stringify project + json = JSON.stringify(project.rootFolder) + return crypto.createHash('sha1').update(json, 'utf8').digest('hex') clsiStateEnabled = Settings.clsiState @@ -16,26 +27,19 @@ OneHour = 3600 * 1000 module.exports = ClsiStateManager = - checkState: (project_id, project, callback = (err, ok) ->) -> + checkProjectStateMatch: (project_id, project, callback = (err, ok) ->) -> newState = buildState(project) - @getState project_id, (err, oldState) -> + rclient.get buildKey(project_id), (err, oldState) -> return callback(err) if err? + logger.log project_id: project_id, new_state: newState, old_state: oldState, "got project state from redis" if newState is oldState - hash = crypto.createHash('sha1').update(newState, 'utf8').digest('hex') - callback(null,true,hash) + callback(null,true,oldState) else callback(null,false) - getState: (project_id, callback = (err, state)->)-> - rclient.get buildKey(project_id), (err, state)-> - return callback(err) if err? - logger.log project_id: project_id, state: state, "got project state from redis" - return callback(null, state) - - setState: (project_id, project, callback = (err)->)-> - projectState = buildState project + setProjectState: (project_id, project, callback = (err)->)-> + projectState = buildState(project) logger.log project_id: project_id, projectState: projectState, "setting project state in redis" rclient.set buildKey(project_id), projectState, "PX", OneHour, (err) -> return callback(err) if err? - hash = crypto.createHash('sha1').update(projectState, 'utf8').digest('hex') - callback(null,hash) + callback(null,projectState) From fb29ac3031be5dafef2850f4102ddeb4452bd6ef Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 3 Aug 2017 10:56:16 +0100 Subject: [PATCH 11/35] clean up logging --- .../app/coffee/Features/Compile/ClsiManager.coffee | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/ClsiManager.coffee b/services/web/app/coffee/Features/Compile/ClsiManager.coffee index 2341129481..6d8baefe5e 100755 --- a/services/web/app/coffee/Features/Compile/ClsiManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiManager.coffee @@ -29,7 +29,6 @@ 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" - console.log "REQUEST", JSON.stringify(req, null, 2) 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" @@ -41,10 +40,7 @@ module.exports = ClsiManager = if error? logger.err err:error, project_id:project_id, "error sending request to clsi" return callback(error) - if response?.compile?.status is "conflict" - # FIXME try again without incremental option - console.log "CONFLICT TRY AGAIN" - logger.log project_id: project_id, outputFilesLength: response?.outputFiles?.length, status: response?.status, "received compile response from CLSI" + 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" @@ -124,7 +120,6 @@ module.exports = ClsiManager = ProjectGetter.getProject project_id, {compiler: 1, rootDoc_id: 1, imageName: 1, rootFolder:1}, (error, project) -> return callback(error) if error? return callback(new Errors.NotFoundError("project does not exist: #{project_id}")) if !project? - console.log "PROJECT", project, JSON.stringify(project.rootFolder,null,2) if project.compiler not in ClsiManager.VALID_COMPILERS project.compiler = "pdflatex" @@ -137,20 +132,17 @@ module.exports = ClsiManager = # make this incremental ProjectEntityHandler.getAllDocPathsFromProject project, (error, docPath) -> return callback(error) if error? - console.log "PATHS", docPath - console.log "DOCS", docUpdaterDocs docs = {} for doc in docUpdaterDocs or [] path = docPath[doc._id] docs[path] = doc - console.log "MAPPED DOCS", docs options.incremental = projectState ClsiManager._finaliseRequest project_id, options, project, docs, [], callback else ClsiManager._getContentFromMongo project_id, (error, docs, files) -> return callback(error) if error? - console.log "DOCS", docs - # FIXME want to store state after project has been sent to clsi + # FIXME want to store state after project has been sent to + # clsi, but need to do it here. ClsiStateManager.setProjectState project_id, project, (error, projectState) -> if error? logger.err err:error, project_id:project_id, "error storing state in redis" From 5c02255e07ff740f1df2e6aeeeb19deddf292644 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 3 Aug 2017 11:44:10 +0100 Subject: [PATCH 12/35] use syncType and syncState for clsi state options --- .../app/coffee/Features/Compile/ClsiManager.coffee | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/ClsiManager.coffee b/services/web/app/coffee/Features/Compile/ClsiManager.coffee index 6d8baefe5e..32f3ce9f67 100755 --- a/services/web/app/coffee/Features/Compile/ClsiManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiManager.coffee @@ -20,7 +20,7 @@ module.exports = ClsiManager = ClsiManager.sendRequestOnce project_id, user_id, _.clone(options), (error, status, result...) -> return callback(error) if error? if status is 'conflict' - options.state = "conflict" # will force full compile + options.syncType = "full" # force full compile ClsiManager.sendRequestOnce project_id, user_id, options, callback # try again else callback(error, status, result...) @@ -126,7 +126,7 @@ module.exports = ClsiManager = ClsiStateManager.checkProjectStateMatch project_id, project, (error, stateOk, projectState) -> return callback(error) if error? logger.log project_id: project_id, checkState: stateOk, "checked project state" - if stateOk and not options.state? # incremental + if stateOk and options.syncType isnt "full" # incremental ClsiManager._getContentFromDocUpdater project_id, (error, docUpdaterDocs) -> return callback(error) if error? # make this incremental @@ -136,7 +136,8 @@ module.exports = ClsiManager = for doc in docUpdaterDocs or [] path = docPath[doc._id] docs[path] = doc - options.incremental = projectState + options.syncType = "incremental" + options.syncState = projectState ClsiManager._finaliseRequest project_id, options, project, docs, [], callback else ClsiManager._getContentFromMongo project_id, (error, docs, files) -> @@ -147,7 +148,8 @@ module.exports = ClsiManager = if error? logger.err err:error, project_id:project_id, "error storing state in redis" #return callback(error) - options.state = projectState + options.syncType = "full" + options.syncState = projectState ClsiManager._finaliseRequest project_id, options, project, docs, files, callback _getContentFromDocUpdater: (project_id, callback = (error, docs) ->) -> @@ -200,8 +202,8 @@ module.exports = ClsiManager = imageName: project.imageName draft: !!options.draft check: options.check - incremental: options.incremental - state: options.state + syncType: options.syncType + syncState: options.syncState rootResourcePath: rootResourcePath resources: resources } From f44b844d743ccf48c044fbb721040b560cb0c995 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 3 Aug 2017 12:08:11 +0100 Subject: [PATCH 13/35] refer to project state as projectStateHash --- .../app/coffee/Features/Compile/ClsiManager.coffee | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/ClsiManager.coffee b/services/web/app/coffee/Features/Compile/ClsiManager.coffee index 32f3ce9f67..4c543bcaaa 100755 --- a/services/web/app/coffee/Features/Compile/ClsiManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiManager.coffee @@ -123,13 +123,13 @@ module.exports = ClsiManager = if project.compiler not in ClsiManager.VALID_COMPILERS project.compiler = "pdflatex" - ClsiStateManager.checkProjectStateMatch project_id, project, (error, stateOk, projectState) -> + ClsiStateManager.checkProjectStateMatch project_id, project, (error, stateOk, projectStateHash) -> return callback(error) if error? logger.log project_id: project_id, checkState: stateOk, "checked project state" - if stateOk and options.syncType isnt "full" # incremental + # see if we can send an incremental update to the CLSI + if stateOk and options.syncType isnt "full" ClsiManager._getContentFromDocUpdater project_id, (error, docUpdaterDocs) -> return callback(error) if error? - # make this incremental ProjectEntityHandler.getAllDocPathsFromProject project, (error, docPath) -> return callback(error) if error? docs = {} @@ -137,19 +137,20 @@ module.exports = ClsiManager = path = docPath[doc._id] docs[path] = doc options.syncType = "incremental" - options.syncState = projectState + options.syncState = projectStateHash + # send new docs but not files as those are already on the clsi ClsiManager._finaliseRequest project_id, options, project, docs, [], callback else ClsiManager._getContentFromMongo project_id, (error, docs, files) -> return callback(error) if error? # FIXME want to store state after project has been sent to # clsi, but need to do it here. - ClsiStateManager.setProjectState project_id, project, (error, projectState) -> + ClsiStateManager.setProjectState project_id, project, (error, projectStateHash) -> if error? logger.err err:error, project_id:project_id, "error storing state in redis" #return callback(error) options.syncType = "full" - options.syncState = projectState + options.syncState = projectStateHash ClsiManager._finaliseRequest project_id, options, project, docs, files, callback _getContentFromDocUpdater: (project_id, callback = (error, docs) ->) -> From 6d331e8ffd13b781397e1ccf781d789be3cd4807 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 3 Aug 2017 12:10:23 +0100 Subject: [PATCH 14/35] use projectStateUnchanged instead of stateOk --- services/web/app/coffee/Features/Compile/ClsiManager.coffee | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/ClsiManager.coffee b/services/web/app/coffee/Features/Compile/ClsiManager.coffee index 4c543bcaaa..d14160b46d 100755 --- a/services/web/app/coffee/Features/Compile/ClsiManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiManager.coffee @@ -123,11 +123,11 @@ module.exports = ClsiManager = if project.compiler not in ClsiManager.VALID_COMPILERS project.compiler = "pdflatex" - ClsiStateManager.checkProjectStateMatch project_id, project, (error, stateOk, projectStateHash) -> + ClsiStateManager.checkProjectStateMatch project_id, project, (error, projectStateUnchanged, projectStateHash) -> return callback(error) if error? - logger.log project_id: project_id, checkState: stateOk, "checked project state" + logger.log project_id: project_id, projectStateUnchanged: projectStateUnchanged, "checked project state" # see if we can send an incremental update to the CLSI - if stateOk and options.syncType isnt "full" + if projectStateUnchanged and options.syncType isnt "full" ClsiManager._getContentFromDocUpdater project_id, (error, docUpdaterDocs) -> return callback(error) if error? ProjectEntityHandler.getAllDocPathsFromProject project, (error, docPath) -> From 38c879faf2bc17130ecffb20ddbfe480fc65c37e Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 3 Aug 2017 12:15:27 +0100 Subject: [PATCH 15/35] improve comment about ClsiStateManager hash --- .../coffee/Features/Compile/ClsiStateManager.coffee | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/ClsiStateManager.coffee b/services/web/app/coffee/Features/Compile/ClsiStateManager.coffee index 8347e9348b..6fc7b1dbf0 100644 --- a/services/web/app/coffee/Features/Compile/ClsiStateManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiStateManager.coffee @@ -7,12 +7,16 @@ crypto = require "crypto" buildKey = (project_id)-> return "clsistate:#{project_id}" # FIXME: should we cluster these on project?? -# the "state" of a project is a hash of the relevant attributes in the +# The "state" of a project is a hash of the relevant attributes in the # project object in this case we only need the rootFolder. # -# When it changes the full set of files on the CLSI will need to be -# updated. If it doesn't change then we can overwrite changed docs in -# place on the clsi, getting them from the docupdater. +# The idea is that it will change if any doc or file is +# created/renamed/deleted, and also if the content of any file (not +# doc) changes. +# +# When the hash changes the full set of files on the CLSI will need to +# be updated. If it doesn't change then we can overwrite changed docs +# in place on the clsi, getting them from the docupdater. # # The docupdater is also responsible for unsetting the key in redis if # it removes any documents from the doc updater. From 1321009fe1ee7f2ef7220716ea669ccabad9e73d Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 3 Aug 2017 14:40:46 +0100 Subject: [PATCH 16/35] update docupdater endpoint to /project/id/docs --- .../Features/DocumentUpdater/DocumentUpdaterHandler.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee index b3247ba417..50c263f938 100644 --- a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee +++ b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee @@ -125,7 +125,7 @@ module.exports = DocumentUpdaterHandler = getProjectDocs: (project_id, callback = (error, docs) ->) -> timer = new metrics.Timer("get-project-docs") - url = "#{settings.apis.documentupdater.url}/project/#{project_id}" + url = "#{settings.apis.documentupdater.url}/project/#{project_id}/doc" logger.log project_id:project_id, "getting project docs from document updater" request.get url, (error, res, body)-> timer.done() From 849e905efb1139e1e2b5f9fceb3354a051bdc5ce Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Mon, 7 Aug 2017 14:45:04 +0100 Subject: [PATCH 17/35] simplify incremental request to docupdater if project state hasn't changed, get the docs from the docupdater -- we check/set the hash and return the docs in a single request. Otherwise do a full request from mongo. --- .../Features/Compile/ClsiManager.coffee | 57 ++++++++++--------- .../Features/Compile/ClsiStateManager.coffee | 32 ++--------- .../DocumentUpdaterHandler.coffee | 9 +-- .../Project/ProjectEntityHandler.coffee | 1 - 4 files changed, 41 insertions(+), 58 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/ClsiManager.coffee b/services/web/app/coffee/Features/Compile/ClsiManager.coffee index d14160b46d..7ec267ede2 100755 --- a/services/web/app/coffee/Features/Compile/ClsiManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiManager.coffee @@ -123,35 +123,40 @@ module.exports = ClsiManager = if project.compiler not in ClsiManager.VALID_COMPILERS project.compiler = "pdflatex" - ClsiStateManager.checkProjectStateMatch project_id, project, (error, projectStateUnchanged, projectStateHash) -> + ClsiManager.getContentFromDocUpdaterIfMatch project_id, project, (error, projectStateHash, docUpdaterDocs) -> return callback(error) if error? - logger.log project_id: project_id, projectStateUnchanged: projectStateUnchanged, "checked project state" + logger.log project_id: project_id, projectStateHash: projectStateHash, docs: docUpdaterDocs?, "checked project state" # see if we can send an incremental update to the CLSI - if projectStateUnchanged and options.syncType isnt "full" - ClsiManager._getContentFromDocUpdater project_id, (error, docUpdaterDocs) -> - return callback(error) if error? - ProjectEntityHandler.getAllDocPathsFromProject project, (error, docPath) -> - return callback(error) if error? - docs = {} - for doc in docUpdaterDocs or [] - path = docPath[doc._id] - docs[path] = doc - options.syncType = "incremental" - options.syncState = projectStateHash - # send new docs but not files as those are already on the clsi - ClsiManager._finaliseRequest project_id, options, project, docs, [], callback + if docUpdaterDocs? and options.syncType isnt "full" + ClsiManager._buildRequestFromDocupdater project_id, options, project, projectStateHash, docUpdaterDocs, callback else - ClsiManager._getContentFromMongo project_id, (error, docs, files) -> - return callback(error) if error? - # FIXME want to store state after project has been sent to - # clsi, but need to do it here. - ClsiStateManager.setProjectState project_id, project, (error, projectStateHash) -> - if error? - logger.err err:error, project_id:project_id, "error storing state in redis" - #return callback(error) - options.syncType = "full" - options.syncState = projectStateHash - ClsiManager._finaliseRequest project_id, options, project, docs, files, callback + ClsiManager._buildRequestFromMongo project_id, options, project, projectStateHash, callback + + getContentFromDocUpdaterIfMatch: (project_id, project, callback = (error, projectStateHash, docs) ->) -> + ClsiStateManager.computeHash project, (error, projectStateHash) -> + return callback(error) if error? + DocumentUpdaterHandler.getProjectDocsIfMatch project_id, projectStateHash, (error, docs) -> + return callback(error) if error? + callback(null, projectStateHash, docs) + + _buildRequestFromDocupdater: (project_id, options, project, projectStateHash, docUpdaterDocs, callback = (error, request) ->) -> + ProjectEntityHandler.getAllDocPathsFromProject project, (error, docPath) -> + return callback(error) if error? + docs = {} + for doc in docUpdaterDocs or [] + path = docPath[doc._id] + docs[path] = doc + # send new docs but not files as those are already on the clsi + options.syncType = "incremental" + options.syncState = projectStateHash + ClsiManager._finaliseRequest project_id, options, project, docs, [], callback + + _buildRequestFromMongo: (project_id, options, project, projectStateHash, callback = (error, request) ->) -> + ClsiManager._getContentFromMongo project_id, (error, docs, files) -> + return callback(error) if error? + options.syncType = "full" + options.syncState = projectStateHash + ClsiManager._finaliseRequest project_id, options, project, docs, files, callback _getContentFromDocUpdater: (project_id, callback = (error, docs) ->) -> DocumentUpdaterHandler.getProjectDocs project_id, (error, docs) -> diff --git a/services/web/app/coffee/Features/Compile/ClsiStateManager.coffee b/services/web/app/coffee/Features/Compile/ClsiStateManager.coffee index 6fc7b1dbf0..715592e4dc 100644 --- a/services/web/app/coffee/Features/Compile/ClsiStateManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiStateManager.coffee @@ -1,12 +1,7 @@ Settings = require "settings-sharelatex" -RedisWrapper = require("../../infrastructure/RedisWrapper") -rclient = RedisWrapper.client("clsi_state") logger = require "logger-sharelatex" crypto = require "crypto" -buildKey = (project_id)-> - return "clsistate:#{project_id}" # FIXME: should we cluster these on project?? - # The "state" of a project is a hash of the relevant attributes in the # project object in this case we only need the rootFolder. # @@ -18,32 +13,15 @@ buildKey = (project_id)-> # be updated. If it doesn't change then we can overwrite changed docs # in place on the clsi, getting them from the docupdater. # -# The docupdater is also responsible for unsetting the key in redis if -# it removes any documents from the doc updater. +# The docupdater is responsible for setting the key in redis, and +# unsetting it if it removes any documents from the doc updater. buildState = (project) -> json = JSON.stringify(project.rootFolder) return crypto.createHash('sha1').update(json, 'utf8').digest('hex') -clsiStateEnabled = Settings.clsiState - -OneHour = 3600 * 1000 - module.exports = ClsiStateManager = - checkProjectStateMatch: (project_id, project, callback = (err, ok) ->) -> - newState = buildState(project) - rclient.get buildKey(project_id), (err, oldState) -> - return callback(err) if err? - logger.log project_id: project_id, new_state: newState, old_state: oldState, "got project state from redis" - if newState is oldState - callback(null,true,oldState) - else - callback(null,false) - - setProjectState: (project_id, project, callback = (err)->)-> - projectState = buildState(project) - logger.log project_id: project_id, projectState: projectState, "setting project state in redis" - rclient.set buildKey(project_id), projectState, "PX", OneHour, (err) -> - return callback(err) if err? - callback(null,projectState) + computeHash: (project, callback = (err, hash) ->) -> + hash = buildState(project) + callback(null, hash) diff --git a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee index 50c263f938..6064a41363 100644 --- a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee +++ b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee @@ -123,16 +123,18 @@ module.exports = DocumentUpdaterHandler = logger.error project_id:project_id, doc_id:doc_id, url: url, "doc updater returned a non-success status code: #{res.statusCode}" callback new Error("doc updater returned a non-success status code: #{res.statusCode}") - getProjectDocs: (project_id, callback = (error, docs) ->) -> + getProjectDocsIfMatch: (project_id, projectStateHash, callback = (error, docs) ->) -> timer = new metrics.Timer("get-project-docs") - url = "#{settings.apis.documentupdater.url}/project/#{project_id}/doc" + url = "#{settings.apis.documentupdater.url}/project/#{project_id}/doc?state=#{projectStateHash}" logger.log project_id:project_id, "getting project docs from document updater" request.get url, (error, res, body)-> timer.done() if error? logger.error err:error, url:url, project_id:project_id, "error getting project docs from doc updater" return callback(error) - if res.statusCode >= 200 and res.statusCode < 300 + if res.statusCode is 409 + return callback() + else if res.statusCode >= 200 and res.statusCode < 300 logger.log project_id:project_id, "got project docs from document document updater" try docs = JSON.parse(body) @@ -140,7 +142,6 @@ module.exports = DocumentUpdaterHandler = doc.lines = JSON.parse(doc.lines) catch error return callback(error) - logger.log project_id: project_id, docs: docs, "RESULT" callback null, docs else logger.error project_id:project_id, url: url, "doc updater returned a non-success status code: #{res.statusCode}" diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index 88a8642358..ca828ebf8d 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -84,7 +84,6 @@ module.exports = ProjectEntityHandler = docPath = {} for folderPath, folder of folders for doc in (folder.docs or []) - console.log "PATH", path.join(folderPath, doc.name), doc._id, doc.name docPath[doc._id] = path.join(folderPath, doc.name) logger.log count:_.keys(docPath).length, project_id:project._id, "returning docPaths for project" callback null, docPath From 31e71854a420b88c51c0a45dfe12cb7b0b90de53 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 8 Aug 2017 11:38:31 +0100 Subject: [PATCH 18/35] fix unit tests --- .../app/coffee/Features/Compile/ClsiManager.coffee | 1 - .../coffee/Compile/ClsiManagerTests.coffee | 14 +++++++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/ClsiManager.coffee b/services/web/app/coffee/Features/Compile/ClsiManager.coffee index 7ec267ede2..512bc54264 100755 --- a/services/web/app/coffee/Features/Compile/ClsiManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiManager.coffee @@ -182,7 +182,6 @@ module.exports = ClsiManager = resources.push path: path content: doc.lines.join("\n") - rev: doc.rev if project.rootDoc_id? and doc._id.toString() == project.rootDoc_id.toString() rootResourcePath = path if options.rootDoc_id? and doc._id.toString() == options.rootDoc_id.toString() diff --git a/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee b/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee index 6bcde76444..651cb1b328 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() + @ClsiStateManager = + computeHash: sinon.stub().callsArgWith(1, null, "01234567890abcdef") @ClsiFormatChecker = checkRecoursesForProblems:sinon.stub().callsArgWith(1) @ClsiManager = SandboxedModule.require modulePath, requires: @@ -26,8 +28,11 @@ describe "ClsiManager", -> url: "https://clsipremium.example.com" "../../models/Project": Project: @Project = {} "../Project/ProjectEntityHandler": @ProjectEntityHandler = {} - "../DocumentUpdater/DocumentUpdaterHandler": @DocumentUpdaterHandler = {} + "../Project/ProjectGetter": @ProjectGetter = {} + "../DocumentUpdater/DocumentUpdaterHandler": @DocumentUpdaterHandler = + getProjectDocsIfMatch: sinon.stub().callsArgWith(2,null,null) "./ClsiCookieManager": @ClsiCookieManager + "./ClsiStateManager": @ClsiStateManager "logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub(), warn: sinon.stub() } "request": @request = sinon.stub() "./ClsiFormatChecker": @ClsiFormatChecker @@ -145,6 +150,7 @@ describe "ClsiManager", -> @Project.findById = sinon.stub().callsArgWith(2, null, @project) @ProjectEntityHandler.getAllDocs = sinon.stub().callsArgWith(1, null, @docs) @ProjectEntityHandler.getAllFiles = sinon.stub().callsArgWith(1, null, @files) + @ProjectGetter.getProject = sinon.stub().callsArgWith(2, null, @project) @DocumentUpdaterHandler.flushProjectToMongo = sinon.stub().callsArgWith(1, null) describe "with a valid project", -> @@ -154,8 +160,8 @@ describe "ClsiManager", -> done() it "should get the project with the required fields", -> - @Project.findById - .calledWith(@project_id, {compiler:1, rootDoc_id: 1, imageName: 1}) + @ProjectGetter.getProject + .calledWith(@project_id, {compiler:1, rootDoc_id: 1, imageName: 1, rootFolder: 1}) .should.equal true it "should flush the project to the database", -> @@ -182,6 +188,8 @@ describe "ClsiManager", -> imageName: @image draft: false check: undefined + syncType: "full" + syncState: "01234567890abcdef" rootResourcePath: "main.tex" resources: [{ path: "main.tex" From 203e42fa4c2409ee5d7d40cae7b7041c42458413 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 8 Aug 2017 16:48:37 +0100 Subject: [PATCH 19/35] clean up options handling --- services/web/app/coffee/Features/Compile/ClsiManager.coffee | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/services/web/app/coffee/Features/Compile/ClsiManager.coffee b/services/web/app/coffee/Features/Compile/ClsiManager.coffee index 512bc54264..40617f23f8 100755 --- a/services/web/app/coffee/Features/Compile/ClsiManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiManager.coffee @@ -17,9 +17,10 @@ DocumentUpdaterHandler = require "../DocumentUpdater/DocumentUpdaterHandler" module.exports = ClsiManager = sendRequest: (project_id, user_id, options = {}, callback) -> - ClsiManager.sendRequestOnce project_id, user_id, _.clone(options), (error, status, result...) -> + ClsiManager.sendRequestOnce project_id, user_id, options, (error, status, result...) -> return callback(error) if error? if status is 'conflict' + options = _.clone(options) options.syncType = "full" # force full compile ClsiManager.sendRequestOnce project_id, user_id, options, callback # try again else @@ -147,6 +148,7 @@ module.exports = ClsiManager = path = docPath[doc._id] docs[path] = doc # send new docs but not files as those are already on the clsi + options = _.clone(options) options.syncType = "incremental" options.syncState = projectStateHash ClsiManager._finaliseRequest project_id, options, project, docs, [], callback @@ -154,6 +156,7 @@ module.exports = ClsiManager = _buildRequestFromMongo: (project_id, options, project, projectStateHash, callback = (error, request) ->) -> ClsiManager._getContentFromMongo project_id, (error, docs, files) -> return callback(error) if error? + options = _.clone(options) options.syncType = "full" options.syncState = projectStateHash ClsiManager._finaliseRequest project_id, options, project, docs, files, callback From 8aa77cec5e4da4e6093a9a688e28e5daae0e24e7 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 8 Aug 2017 16:48:47 +0100 Subject: [PATCH 20/35] provide fallback to normal compile method --- .../Features/Compile/ClsiManager.coffee | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/ClsiManager.coffee b/services/web/app/coffee/Features/Compile/ClsiManager.coffee index 40617f23f8..2f46a35d11 100755 --- a/services/web/app/coffee/Features/Compile/ClsiManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiManager.coffee @@ -124,14 +124,19 @@ module.exports = ClsiManager = if project.compiler not in ClsiManager.VALID_COMPILERS project.compiler = "pdflatex" - ClsiManager.getContentFromDocUpdaterIfMatch project_id, project, (error, projectStateHash, docUpdaterDocs) -> - return callback(error) if error? - logger.log project_id: project_id, projectStateHash: projectStateHash, docs: docUpdaterDocs?, "checked project state" - # see if we can send an incremental update to the CLSI - if docUpdaterDocs? and options.syncType isnt "full" - ClsiManager._buildRequestFromDocupdater project_id, options, project, projectStateHash, docUpdaterDocs, callback - else - ClsiManager._buildRequestFromMongo project_id, options, project, projectStateHash, callback + if options.syncType? # new way, either incremental or full + ClsiManager.getContentFromDocUpdaterIfMatch project_id, project, (error, projectStateHash, docUpdaterDocs) -> + return callback(error) if error? + logger.log project_id: project_id, projectStateHash: projectStateHash, docs: docUpdaterDocs?, "checked project state" + # see if we can send an incremental update to the CLSI + if docUpdaterDocs? and options.syncType isnt "full" + ClsiManager._buildRequestFromDocupdater project_id, options, project, projectStateHash, docUpdaterDocs, callback + else + ClsiManager._buildRequestFromMongo project_id, options, project, projectStateHash, callback + else # old way, always from mongo + ClsiManager._getContentFromMongo project_id, (error, docs, files) -> + return callback(error) if error? + ClsiManager._finaliseRequest project_id, options, project, docs, files, callback getContentFromDocUpdaterIfMatch: (project_id, project, callback = (error, projectStateHash, docs) ->) -> ClsiStateManager.computeHash project, (error, projectStateHash) -> From 97b129cbe35ccdd6b50aeb0688f0f4aa7b0c55da Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Wed, 9 Aug 2017 10:57:24 +0100 Subject: [PATCH 21/35] enable incremental compilation for beta users --- services/web/app/coffee/Features/Compile/ClsiManager.coffee | 2 +- .../web/app/coffee/Features/Compile/CompileController.coffee | 2 ++ services/web/app/views/beta_program/opt_in.pug | 2 +- .../web/public/coffee/ide/pdf/controllers/PdfController.coffee | 1 + 4 files changed, 5 insertions(+), 2 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/ClsiManager.coffee b/services/web/app/coffee/Features/Compile/ClsiManager.coffee index 2f46a35d11..e44f8c4d78 100755 --- a/services/web/app/coffee/Features/Compile/ClsiManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiManager.coffee @@ -124,7 +124,7 @@ module.exports = ClsiManager = if project.compiler not in ClsiManager.VALID_COMPILERS project.compiler = "pdflatex" - if options.syncType? # new way, either incremental or full + if options.incremental or options.syncType? # new way, either incremental or full ClsiManager.getContentFromDocUpdaterIfMatch project_id, project, (error, projectStateHash, docUpdaterDocs) -> return callback(error) if error? logger.log project_id: project_id, projectStateHash: projectStateHash, docs: docUpdaterDocs?, "checked project state" diff --git a/services/web/app/coffee/Features/Compile/CompileController.coffee b/services/web/app/coffee/Features/Compile/CompileController.coffee index 6d893f0dbb..98b1a69a7f 100755 --- a/services/web/app/coffee/Features/Compile/CompileController.coffee +++ b/services/web/app/coffee/Features/Compile/CompileController.coffee @@ -30,6 +30,8 @@ module.exports = CompileController = options.draft = req.body.draft if req.body?.check in ['validate', 'error', 'silent'] options.check = req.body.check + if req.body?.incremental + options.incremental = 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, validationProblems) -> return next(error) if error? diff --git a/services/web/app/views/beta_program/opt_in.pug b/services/web/app/views/beta_program/opt_in.pug index b9fb1062e7..7e6af48a00 100644 --- a/services/web/app/views/beta_program/opt_in.pug +++ b/services/web/app/views/beta_program/opt_in.pug @@ -18,7 +18,7 @@ block content | #{translate("beta_program_badge_description")} span.beta-feature-badge p.text-centered - strong We're not currently testing anything in beta, but keep checking back! + strong We're currently testing lower latency compilation features in beta. .row.text-centered .col-md-12 if user.betaProgram diff --git a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee index 675513e330..34b3b190e6 100644 --- a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee +++ b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee @@ -105,6 +105,7 @@ define [ rootDoc_id: options.rootDocOverride_id or null draft: $scope.draft check: checkType + incremental: window.user?.betaProgram _csrf: window.csrfToken }, {params: params} From 836219584cac6be94b9db1197f842f80da92bd2e Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Wed, 9 Aug 2017 11:31:10 +0100 Subject: [PATCH 22/35] fix tests for beta users --- .../web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee b/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee index 651cb1b328..0cca53d061 100644 --- a/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee @@ -188,8 +188,8 @@ describe "ClsiManager", -> imageName: @image draft: false check: undefined - syncType: "full" - syncState: "01234567890abcdef" + syncType: undefined # "full" + syncState: undefined # "01234567890abcdef" rootResourcePath: "main.tex" resources: [{ path: "main.tex" From 7eb1c019941e4731166d1ab62fd344e18e4e061b Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Wed, 9 Aug 2017 11:34:16 +0100 Subject: [PATCH 23/35] add metrics for incremental compiles --- .../web/app/coffee/Features/Compile/ClsiManager.coffee | 7 +++++++ .../test/UnitTests/coffee/Compile/ClsiManagerTests.coffee | 4 ++++ 2 files changed, 11 insertions(+) diff --git a/services/web/app/coffee/Features/Compile/ClsiManager.coffee b/services/web/app/coffee/Features/Compile/ClsiManager.coffee index e44f8c4d78..769e82a4c1 100755 --- a/services/web/app/coffee/Features/Compile/ClsiManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiManager.coffee @@ -13,6 +13,7 @@ _ = require("underscore") async = require("async") ClsiFormatChecker = require("./ClsiFormatChecker") DocumentUpdaterHandler = require "../DocumentUpdater/DocumentUpdaterHandler" +Metrics = require('metrics-sharelatex') module.exports = ClsiManager = @@ -125,16 +126,22 @@ module.exports = ClsiManager = project.compiler = "pdflatex" if options.incremental or options.syncType? # new way, either incremental or full + timer = new Metrics.Timer("editor.compile-getdocs-redis") ClsiManager.getContentFromDocUpdaterIfMatch project_id, project, (error, projectStateHash, docUpdaterDocs) -> + timer.done() return callback(error) if error? logger.log project_id: project_id, projectStateHash: projectStateHash, docs: docUpdaterDocs?, "checked project state" # see if we can send an incremental update to the CLSI if docUpdaterDocs? and options.syncType isnt "full" + Metrics.inc "compile-from-redis" ClsiManager._buildRequestFromDocupdater project_id, options, project, projectStateHash, docUpdaterDocs, callback else + Metrics.inc "compile-from-mongo" ClsiManager._buildRequestFromMongo project_id, options, project, projectStateHash, callback else # old way, always from mongo + timer = new Metrics.Timer("editor.compile-getdocs-mongo") ClsiManager._getContentFromMongo project_id, (error, docs, files) -> + timer.done() return callback(error) if error? ClsiManager._finaliseRequest project_id, options, project, docs, files, callback diff --git a/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee b/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee index 0cca53d061..6b4727f7a7 100644 --- a/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee @@ -36,6 +36,10 @@ describe "ClsiManager", -> "logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub(), warn: sinon.stub() } "request": @request = sinon.stub() "./ClsiFormatChecker": @ClsiFormatChecker + "metrics-sharelatex": @Metrics = + Timer: class Timer + done: sinon.stub() + inc: sinon.stub() @project_id = "project-id" @user_id = "user-id" @callback = sinon.stub() From ddecd26718a8a1b785eea092a4e8b771031266d7 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Wed, 9 Aug 2017 15:47:44 +0100 Subject: [PATCH 24/35] flush documents to mongo on incremental compiles --- .../web/app/coffee/Features/Compile/ClsiManager.coffee | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/ClsiManager.coffee b/services/web/app/coffee/Features/Compile/ClsiManager.coffee index 769e82a4c1..6b77ab3abf 100755 --- a/services/web/app/coffee/Features/Compile/ClsiManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiManager.coffee @@ -174,9 +174,14 @@ module.exports = ClsiManager = ClsiManager._finaliseRequest project_id, options, project, docs, files, callback _getContentFromDocUpdater: (project_id, callback = (error, docs) ->) -> - DocumentUpdaterHandler.getProjectDocs project_id, (error, docs) -> + # Workaround: for now, always flush project to mongo on compile + # until we have automatic periodic flushing on the docupdater + # side, to prevent documents staying in redis too long. + DocumentUpdaterHandler.flushProjectToMongo project_id, (error) -> return callback(error) if error? - callback(null, docs) + DocumentUpdaterHandler.getProjectDocs project_id, (error, docs) -> + return callback(error) if error? + callback(null, docs) _getContentFromMongo: (project_id, callback = (error, docs, files) ->) -> DocumentUpdaterHandler.flushProjectToMongo project_id, (error) -> From 836bddd91fee8974bba4be2264c851b742caa966 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Wed, 9 Aug 2017 16:00:11 +0100 Subject: [PATCH 25/35] comment about 409 code in DocumentUpdaterHandler --- .../DocumentUpdater/DocumentUpdaterHandler.coffee | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee index 6064a41363..e3666ad9ef 100644 --- a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee +++ b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee @@ -124,6 +124,9 @@ module.exports = DocumentUpdaterHandler = callback new Error("doc updater returned a non-success status code: #{res.statusCode}") getProjectDocsIfMatch: (project_id, projectStateHash, callback = (error, docs) ->) -> + # If the project state hasn't changed, we can get all the latest + # docs from redis via the docupdater. Otherwise we will need to + # fall back to getting them from mongo. timer = new metrics.Timer("get-project-docs") url = "#{settings.apis.documentupdater.url}/project/#{project_id}/doc?state=#{projectStateHash}" logger.log project_id:project_id, "getting project docs from document updater" @@ -132,7 +135,12 @@ module.exports = DocumentUpdaterHandler = if error? logger.error err:error, url:url, project_id:project_id, "error getting project docs from doc updater" return callback(error) - if res.statusCode is 409 + if res.statusCode is 409 # HTTP response code "409 Conflict" + # Docupdater has checked the projectStateHash and found that + # it has changed. This means that the docs currently in redis + # aren't the only change to the project and the full set of + # docs/files should be retreived from docstore/filestore + # instead. return callback() else if res.statusCode >= 200 and res.statusCode < 300 logger.log project_id:project_id, "got project docs from document document updater" From e2048e1ed5bccfe2c0aa670316f7e746c1210c87 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Wed, 9 Aug 2017 16:25:57 +0100 Subject: [PATCH 26/35] use incrementalCompilesEnabled as option name --- services/web/app/coffee/Features/Compile/ClsiManager.coffee | 2 +- .../web/app/coffee/Features/Compile/CompileController.coffee | 4 ++-- .../public/coffee/ide/pdf/controllers/PdfController.coffee | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/ClsiManager.coffee b/services/web/app/coffee/Features/Compile/ClsiManager.coffee index 6b77ab3abf..a61a30e88c 100755 --- a/services/web/app/coffee/Features/Compile/ClsiManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiManager.coffee @@ -125,7 +125,7 @@ module.exports = ClsiManager = if project.compiler not in ClsiManager.VALID_COMPILERS project.compiler = "pdflatex" - if options.incremental or options.syncType? # new way, either incremental or full + if options.incrementalCompilesEnabled or options.syncType? # new way, either incremental or full timer = new Metrics.Timer("editor.compile-getdocs-redis") ClsiManager.getContentFromDocUpdaterIfMatch project_id, project, (error, projectStateHash, docUpdaterDocs) -> timer.done() diff --git a/services/web/app/coffee/Features/Compile/CompileController.coffee b/services/web/app/coffee/Features/Compile/CompileController.coffee index 98b1a69a7f..9f056ddc6f 100755 --- a/services/web/app/coffee/Features/Compile/CompileController.coffee +++ b/services/web/app/coffee/Features/Compile/CompileController.coffee @@ -30,8 +30,8 @@ module.exports = CompileController = options.draft = req.body.draft if req.body?.check in ['validate', 'error', 'silent'] options.check = req.body.check - if req.body?.incremental - options.incremental = true + if req.body?.incrementalCompilesEnabled + options.incrementalCompilesEnabled = 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, validationProblems) -> return next(error) if error? diff --git a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee index 34b3b190e6..42c1cd1bc8 100644 --- a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee +++ b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee @@ -105,7 +105,7 @@ define [ rootDoc_id: options.rootDocOverride_id or null draft: $scope.draft check: checkType - incremental: window.user?.betaProgram + incrementalCompilesEnabled: window.user?.betaProgram _csrf: window.csrfToken }, {params: params} From 4789dd23ee885d9c4c24969d4904ac6025338d84 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Fri, 11 Aug 2017 16:57:23 +0100 Subject: [PATCH 27/35] docupdater will parse lines in getProjectDocs no need to do this in web now --- .../Features/DocumentUpdater/DocumentUpdaterHandler.coffee | 2 -- 1 file changed, 2 deletions(-) diff --git a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee index e3666ad9ef..6e013ef5f2 100644 --- a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee +++ b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee @@ -146,8 +146,6 @@ module.exports = DocumentUpdaterHandler = logger.log project_id:project_id, "got project docs from document document updater" try docs = JSON.parse(body) - for doc in docs or [] - doc.lines = JSON.parse(doc.lines) catch error return callback(error) callback null, docs From f9617034bea74742babf56d0679bc8b4a4da0581 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Mon, 14 Aug 2017 14:33:34 +0100 Subject: [PATCH 28/35] add unit test for getProjectDocsIfMatch --- .../DocumentUpdaterHandlerTests.coffee | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/services/web/test/UnitTests/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee b/services/web/test/UnitTests/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee index 748e796f4d..11838e1bdf 100644 --- a/services/web/test/UnitTests/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee @@ -252,6 +252,47 @@ describe 'DocumentUpdaterHandler', -> .calledWith(new Error("doc updater returned failure status code: 500")) .should.equal true + describe "getProjectDocsIfMatch", -> + beforeEach -> + @callback = sinon.stub() + @project_state_hash = "1234567890abcdef" + + describe "successfully", -> + beforeEach -> + @doc0 = + _id: @doc_id + lines: @lines + v: @version + @docs = [ @doc0, @doc0, @doc0 ] + @body = JSON.stringify @docs + @request.get = sinon.stub().callsArgWith(1, null, {statusCode: 200}, @body) + @handler.getProjectDocsIfMatch @project_id, @project_state_hash, @callback + + it 'should get the documenst from the document updater', -> + url = "#{@settings.apis.documentupdater.url}/project/#{@project_id}/doc?state=#{@project_state_hash}" + @request.get.calledWith(url).should.equal true + + it "should call the callback with the documents", -> + @callback.calledWithExactly(null, @docs).should.equal true + + describe "when the document updater API returns an error", -> + beforeEach -> + @request.get = sinon.stub().callsArgWith(1, @error = new Error("something went wrong"), null, null) + @handler.getProjectDocsIfMatch @project_id, @project_state_hash, @callback + + it "should return an error to the callback", -> + @callback.calledWith(@error).should.equal true + + describe "when the document updater returns a conflict error code", -> + beforeEach -> + @request.get = sinon.stub().callsArgWith(1, null, { statusCode: 409 }, "Conflict") + @handler.getProjectDocsIfMatch @project_id, @project_state_hash, @callback + + it "should return the callback with no documents", -> + @callback + .alwaysCalledWithExactly() + .should.equal true + describe "acceptChanges", -> beforeEach -> @change_id = "mock-change-id-1" From 0347abb13a6cfd973a36bb266a2639eb87195d72 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Mon, 14 Aug 2017 15:40:46 +0100 Subject: [PATCH 29/35] added unit tests for ProjectEntityHandler --- .../Project/ProjectEntityHandlerTests.coffee | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee b/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee index 187d59d03d..0ac40cd2c5 100644 --- a/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee @@ -781,6 +781,41 @@ describe 'ProjectEntityHandler', -> }) .should.equal true + describe "getAllFoldersFromProject", -> + beforeEach -> + @callback = sinon.stub() + @ProjectEntityHandler.getAllFoldersFromProject @project, @callback + + it "should call the callback with the folders", -> + @callback + .calledWith(null, { + "/": @project.rootFolder[0] + "/folder1": @folder1 + }) + .should.equal true + + describe "getAllDocPathsFromProject", -> + beforeEach -> + @docs = [{ + _id: @doc1._id + lines: @lines1 = ["one"] + rev: @rev1 = 1 + }, { + _id: @doc2._id + lines: @lines2 = ["two"] + rev: @rev2 = 2 + }] + @callback = sinon.stub() + @ProjectEntityHandler.getAllDocPathsFromProject @project, @callback + + it "should call the callback with the path for each doc_id", -> + @expected = {} + @expected[@doc1._id] = "/#{@doc1.name}" + @expected[@doc2._id] = "/folder1/#{@doc2.name}" + @callback + .calledWith(null, @expected) + .should.equal true + describe "flushProjectToThirdPartyDataStore", -> beforeEach (done) -> @project = { From 1179518f4e1f8a76809d01ca44fb53a5d347304e Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 15 Aug 2017 14:35:02 +0100 Subject: [PATCH 30/35] unit test for sync conflict --- .../coffee/Compile/ClsiManagerTests.coffee | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee b/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee index 6b4727f7a7..3799537328 100644 --- a/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee @@ -103,6 +103,25 @@ describe "ClsiManager", -> it "should call the callback with a failure statue", -> @callback.calledWith(null, @status).should.equal true + describe "with a sync conflict", -> + beforeEach -> + @ClsiManager.sendRequestOnce = sinon.stub() + @ClsiManager.sendRequestOnce.withArgs(@project_id, @user_id, {syncType:"full"}).callsArgWith(3, null, @status = "success") + @ClsiManager.sendRequestOnce.withArgs(@project_id, @user_id, {}).callsArgWith(3, null, "conflict") + @ClsiManager.sendRequest @project_id, @user_id, {}, @callback + + it "should call the sendRequestOnce method twice", -> + @ClsiManager.sendRequestOnce.calledTwice.should.equal true + + it "should call the sendRequestOnce method once with syncType:full", -> + @ClsiManager.sendRequestOnce.withArgs(@project_id, @user_id, {syncType:"full"}).calledOnce.should.equal true + + it "should call the sendRequestOnce method once without syncType:full", -> + @ClsiManager.sendRequestOnce.withArgs(@project_id, @user_id, {}).calledOnce.should.equal true + + it "should call the callback with a success status", -> + @callback.calledWith(null, @status, ).should.equal true + describe "deleteAuxFiles", -> beforeEach -> @ClsiManager._makeRequest = sinon.stub().callsArg(2) From 739445336f8773149de296ab25922695edb9c018 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 15 Aug 2017 15:59:44 +0100 Subject: [PATCH 31/35] remove unused code and fix flushing --- .../Features/Compile/ClsiManager.coffee | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/ClsiManager.coffee b/services/web/app/coffee/Features/Compile/ClsiManager.coffee index a61a30e88c..8046b78bd8 100755 --- a/services/web/app/coffee/Features/Compile/ClsiManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiManager.coffee @@ -133,8 +133,13 @@ module.exports = ClsiManager = logger.log project_id: project_id, projectStateHash: projectStateHash, docs: docUpdaterDocs?, "checked project state" # see if we can send an incremental update to the CLSI if docUpdaterDocs? and options.syncType isnt "full" - Metrics.inc "compile-from-redis" - ClsiManager._buildRequestFromDocupdater project_id, options, project, projectStateHash, docUpdaterDocs, callback + # Workaround: for now, always flush project to mongo on compile + # until we have automatic periodic flushing on the docupdater + # side, to prevent documents staying in redis too long. + DocumentUpdaterHandler.flushProjectToMongo project_id, (error) -> + return callback(error) if error? + Metrics.inc "compile-from-redis" + ClsiManager._buildRequestFromDocupdater project_id, options, project, projectStateHash, docUpdaterDocs, callback else Metrics.inc "compile-from-mongo" ClsiManager._buildRequestFromMongo project_id, options, project, projectStateHash, callback @@ -173,16 +178,6 @@ module.exports = ClsiManager = options.syncState = projectStateHash ClsiManager._finaliseRequest project_id, options, project, docs, files, callback - _getContentFromDocUpdater: (project_id, callback = (error, docs) ->) -> - # Workaround: for now, always flush project to mongo on compile - # until we have automatic periodic flushing on the docupdater - # side, to prevent documents staying in redis too long. - DocumentUpdaterHandler.flushProjectToMongo project_id, (error) -> - return callback(error) if error? - DocumentUpdaterHandler.getProjectDocs project_id, (error, docs) -> - return callback(error) if error? - callback(null, docs) - _getContentFromMongo: (project_id, callback = (error, docs, files) ->) -> DocumentUpdaterHandler.flushProjectToMongo project_id, (error) -> return callback(error) if error? From c91599bfeb3727ab9523a35450aaff69d26acb5e Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 15 Aug 2017 16:00:40 +0100 Subject: [PATCH 32/35] add unit test for incremental compile --- .../coffee/Compile/ClsiManagerTests.coffee | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee b/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee index 3799537328..48cc4a4c53 100644 --- a/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee @@ -227,6 +227,51 @@ describe "ClsiManager", -> }] ) + describe "with the incremental compile option", -> + beforeEach (done) -> + @ClsiStateManager.computeHash = sinon.stub().callsArgWith(1, null, @project_state_hash = "01234567890abcdef") + @DocumentUpdaterHandler.getProjectDocsIfMatch = sinon.stub().callsArgWith(2, null, [{_id:@doc_1._id, lines: @doc_1.lines, v: 123}]) + @ProjectEntityHandler.getAllDocPathsFromProject = sinon.stub().callsArgWith(1, null, {"mock-doc-id-1":"main.tex"}) + @ClsiManager._buildRequest @project_id, {timeout:100, incrementalCompilesEnabled:true}, (error, request) => + @request = request + done() + + it "should get the project with the required fields", -> + @ProjectGetter.getProject + .calledWith(@project_id, {compiler:1, rootDoc_id: 1, imageName: 1, rootFolder: 1}) + .should.equal true + + it "should flush the project to the database", -> + @DocumentUpdaterHandler.flushProjectToMongo + .calledWith(@project_id) + .should.equal true + + it "should get only the live docs from the docupdater", -> + @DocumentUpdaterHandler.getProjectDocsIfMatch + .calledWith(@project_id) + .should.equal true + + it "should not get any of the files", -> + @ProjectEntityHandler.getAllFiles + .called.should.equal false + + it "should build up the CLSI request", -> + expect(@request).to.deep.equal( + compile: + options: + compiler: @compiler + timeout : 100 + imageName: @image + draft: false + check: undefined + syncType: "incremental" + syncState: "01234567890abcdef" + rootResourcePath: "main.tex" + resources: [{ + path: "main.tex" + content: @doc_1.lines.join("\n") + }] + ) describe "when root doc override is valid", -> beforeEach (done) -> From 90ff58b8208c70911ceaf1788d75fd845163ed02 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Wed, 16 Aug 2017 10:49:29 +0100 Subject: [PATCH 33/35] compute project state hash from sorted docs/files --- .../Features/Compile/ClsiStateManager.coffee | 14 +++++++++----- .../Features/Project/ProjectEntityHandler.coffee | 15 +++++++++++++++ 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/ClsiStateManager.coffee b/services/web/app/coffee/Features/Compile/ClsiStateManager.coffee index 715592e4dc..d66894db75 100644 --- a/services/web/app/coffee/Features/Compile/ClsiStateManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiStateManager.coffee @@ -1,6 +1,7 @@ Settings = require "settings-sharelatex" logger = require "logger-sharelatex" crypto = require "crypto" +ProjectEntityHandler = require "../Project/ProjectEntityHandler" # The "state" of a project is a hash of the relevant attributes in the # project object in this case we only need the rootFolder. @@ -16,12 +17,15 @@ crypto = require "crypto" # The docupdater is responsible for setting the key in redis, and # unsetting it if it removes any documents from the doc updater. -buildState = (project) -> - json = JSON.stringify(project.rootFolder) - return crypto.createHash('sha1').update(json, 'utf8').digest('hex') +buildState = (s) -> + return crypto.createHash('sha1').update(s, 'utf8').digest('hex') module.exports = ClsiStateManager = computeHash: (project, callback = (err, hash) ->) -> - hash = buildState(project) - callback(null, hash) + ProjectEntityHandler.getAllEntitiesFromProject project, (err, docs, files) -> + fileList = ("#{f.file._id}:#{f.file.rev}:#{f.file.created}:#{f.path}" for f in files or []) + docList = ("#{d.doc._id}:#{d.path}" for d in docs or []) + sortedEntityList = [docList..., fileList...].sort() + hash = buildState(sortedEntityList.join("\n")) + callback(null, hash) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index ca828ebf8d..870e8a05b2 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -77,6 +77,21 @@ module.exports = ProjectEntityHandler = processFolder "/", project.rootFolder[0] callback null, folders + getAllEntitiesFromProject: (project, callback) -> + logger.log project:project, "getting all files for project" + @getAllFoldersFromProject project, (err, folders = {}) -> + return callback(err) if err? + docs = [] + files = [] + for folderPath, folder of folders + for doc in (folder.docs or []) + if doc? + docs.push({path: path.join(folderPath, doc.name), doc:doc}) + for file in (folder.fileRefs or []) + if file? + files.push({path: path.join(folderPath, file.name), file:file}) + callback null, docs, files + getAllDocPathsFromProject: (project, callback) -> logger.log project:project, "getting all docs for project" @getAllFoldersFromProject project, (err, folders = {}) -> From f4da089ee35051f268e993481832eb5cc755ca1e Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Wed, 16 Aug 2017 11:42:36 +0100 Subject: [PATCH 34/35] added unit tests for project state hash --- .../Compile/ClsiStateManagerTests.coffee | 148 ++++++++++++++++++ 1 file changed, 148 insertions(+) create mode 100644 services/web/test/UnitTests/coffee/Compile/ClsiStateManagerTests.coffee diff --git a/services/web/test/UnitTests/coffee/Compile/ClsiStateManagerTests.coffee b/services/web/test/UnitTests/coffee/Compile/ClsiStateManagerTests.coffee new file mode 100644 index 0000000000..24f0b847b8 --- /dev/null +++ b/services/web/test/UnitTests/coffee/Compile/ClsiStateManagerTests.coffee @@ -0,0 +1,148 @@ +sinon = require('sinon') +chai = require('chai') +should = chai.should() +expect = chai.expect +modulePath = "../../../../app/js/Features/Compile/ClsiStateManager.js" +SandboxedModule = require('sandboxed-module') + +describe "ClsiStateManager", -> + beforeEach -> + @ClsiStateManager = SandboxedModule.require modulePath, requires: + "settings-sharelatex": @settings = {} + "../Project/ProjectEntityHandler": @ProjectEntityHandler = {} + "logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub(), warn: sinon.stub() } + @project = "project" + @callback = sinon.stub() + + describe "computeHash", -> + beforeEach (done) -> + @docs = [ + {path: "/main.tex", doc: {_id: "doc-id-1"}} + {path: "/folder/sub.tex", doc: {_id: "doc-id-2"}} + ] + @files = [ + {path: "/figure.pdf", file: {_id: "file-id-1", rev: 123, created: "aaaaaa"}} + {path: "/folder/fig2.pdf", file: {_id: "file-id-2", rev: 456, created: "bbbbbb"}} + ] + @ProjectEntityHandler.getAllEntitiesFromProject = sinon.stub().callsArgWith(1, null, @docs, @files) + @ClsiStateManager.computeHash @project, (err, hash) => + @hash0 = hash + done() + + describe "with a sample project", -> + beforeEach -> + @ClsiStateManager.computeHash @project, @callback + + it "should call the callback with a hash value", -> + @callback + .calledWith(null, "9c2c2428e4147db63cacabf6f357af483af6551d") + .should.equal true + + describe "when the files and docs are in a different order", -> + beforeEach -> + [@docs[0], @docs[1]] = [@docs[1], @docs[0]] + [@files[0], @files[1]] = [@files[1], @files[0]] + @ClsiStateManager.computeHash @project, @callback + + it "should call the callback with the same hash value", -> + @callback + .calledWith(null, @hash0) + .should.equal true + + describe "when a doc is renamed", -> + beforeEach (done) -> + @docs[0].path = "/new.tex" + @ClsiStateManager.computeHash @project, (err, hash) => + @hash1 = hash + done() + + it "should call the callback with a different hash value", -> + @callback + .neverCalledWith(null, @hash0) + .should.equal true + + describe "when a file is renamed", -> + beforeEach (done) -> + @files[0].path = "/newfigure.pdf" + @ClsiStateManager.computeHash @project, (err, hash) => + @hash1 = hash + done() + + it "should call the callback with a different hash value", -> + @callback + .neverCalledWith(null, @hash0) + .should.equal true + + describe "when a doc is added", -> + beforeEach (done) -> + @docs.push { path: "/newdoc.tex", doc: {_id: "newdoc-id"}} + @ClsiStateManager.computeHash @project, (err, hash) => + @hash1 = hash + done() + + it "should call the callback with a different hash value", -> + @callback + .neverCalledWith(null, @hash0) + .should.equal true + + describe "when a file is added", -> + beforeEach (done) -> + @files.push { path: "/newfile.tex", file: {_id: "newfile-id", rev: 123}} + @ClsiStateManager.computeHash @project, (err, hash) => + @hash1 = hash + done() + + it "should call the callback with a different hash value", -> + @callback + .neverCalledWith(null, @hash0) + .should.equal true + + describe "when a doc is removed", -> + beforeEach (done) -> + @docs.pop() + @ClsiStateManager.computeHash @project, (err, hash) => + @hash1 = hash + done() + + it "should call the callback with a different hash value", -> + @callback + .neverCalledWith(null, @hash0) + .should.equal true + + describe "when a file is removed", -> + beforeEach (done) -> + @files.pop() + @ClsiStateManager.computeHash @project, (err, hash) => + @hash1 = hash + done() + + it "should call the callback with a different hash value", -> + @callback + .neverCalledWith(null, @hash0) + .should.equal true + + describe "when a file's revision is updated", -> + beforeEach (done) -> + @files[0].file.rev++ + @ClsiStateManager.computeHash @project, (err, hash) => + @hash1 = hash + done() + + it "should call the callback with a different hash value", -> + @callback + .neverCalledWith(null, @hash0) + .should.equal true + + + describe "when a file's date is updated", -> + beforeEach (done) -> + @files[0].file.created = "zzzzzz" + @ClsiStateManager.computeHash @project, (err, hash) => + @hash1 = hash + done() + + it "should call the callback with a different hash value", -> + @callback + .neverCalledWith(null, @hash0) + .should.equal true + From a569303b7e6abaf1e5695c199faa64060e790d75 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 17 Aug 2017 15:36:52 +0100 Subject: [PATCH 35/35] simplify unusual unit test --- .../test/UnitTests/coffee/Compile/ClsiManagerTests.coffee | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee b/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee index 48cc4a4c53..6979a45c69 100644 --- a/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Compile/ClsiManagerTests.coffee @@ -113,11 +113,11 @@ describe "ClsiManager", -> it "should call the sendRequestOnce method twice", -> @ClsiManager.sendRequestOnce.calledTwice.should.equal true - it "should call the sendRequestOnce method once with syncType:full", -> - @ClsiManager.sendRequestOnce.withArgs(@project_id, @user_id, {syncType:"full"}).calledOnce.should.equal true + it "should call the sendRequestOnce method with syncType:full", -> + @ClsiManager.sendRequestOnce.calledWith(@project_id, @user_id, {syncType:"full"}).should.equal true - it "should call the sendRequestOnce method once without syncType:full", -> - @ClsiManager.sendRequestOnce.withArgs(@project_id, @user_id, {}).calledOnce.should.equal true + it "should call the sendRequestOnce method without syncType:full", -> + @ClsiManager.sendRequestOnce.calledWith(@project_id, @user_id, {}).should.equal true it "should call the callback with a success status", -> @callback.calledWith(null, @status, ).should.equal true