From fd28751db0cf4f98f19bd8b9f77a9044aec2bdf1 Mon Sep 17 00:00:00 2001 From: Ingolf Becker Date: Mon, 17 Nov 2014 14:55:28 +0000 Subject: [PATCH 01/13] Change Login and Register methods to use POST rather than GET --- services/web/app/views/user/login.jade | 2 +- services/web/app/views/user/register.jade | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/services/web/app/views/user/login.jade b/services/web/app/views/user/login.jade index aa200d5db3..4d8848d8a6 100644 --- a/services/web/app/views/user/login.jade +++ b/services/web/app/views/user/login.jade @@ -8,7 +8,7 @@ block content .card .page-header h1 #{translate("log_in")} - form(async-form="login", name="loginForm", action='/login', ng-cloak) + form(async-form="login", name="loginForm", action='/login', method="POST", ng-cloak) input(name='_csrf', type='hidden', value=csrfToken) input(name='redir', type='hidden', value=redir) form-messages(for="loginForm") diff --git a/services/web/app/views/user/register.jade b/services/web/app/views/user/register.jade index 0b9010d0cc..89afcfb2e9 100644 --- a/services/web/app/views/user/register.jade +++ b/services/web/app/views/user/register.jade @@ -19,7 +19,7 @@ block content .card .page-header h1 #{translate("register")} - form(async-form="register", name="registerForm", action="/register", ng-cloak) + form(async-form="register", name="registerForm", action="/register", method="POST", ng-cloak) input(name='_csrf', type='hidden', value=csrfToken) input(name='redir', type='hidden', value=redir) form-messages(for="registerForm") From 9d71073a5c7c80db173b9cba88d0ca2770e19ddd Mon Sep 17 00:00:00 2001 From: Ingolf Becker Date: Mon, 17 Nov 2014 15:19:11 +0000 Subject: [PATCH 02/13] Added more POST declarations --- services/web/app/views/user/passwordReset.jade | 1 + services/web/app/views/user/setPassword.jade | 3 ++- services/web/app/views/user/settings.jade | 4 ++-- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/services/web/app/views/user/passwordReset.jade b/services/web/app/views/user/passwordReset.jade index 4db644d916..6516e2cc94 100644 --- a/services/web/app/views/user/passwordReset.jade +++ b/services/web/app/views/user/passwordReset.jade @@ -13,6 +13,7 @@ block content async-form="password-reset-request", name="passwordResetForm" action="/user/password/reset", + method="POST", ng-cloak ) input(type="hidden", name="_csrf", value=csrfToken) diff --git a/services/web/app/views/user/setPassword.jade b/services/web/app/views/user/setPassword.jade index 815f0030ca..c46659b54f 100644 --- a/services/web/app/views/user/setPassword.jade +++ b/services/web/app/views/user/setPassword.jade @@ -12,6 +12,7 @@ block content async-form="password-reset", name="passwordResetForm", action="/user/password/set", + method="POST", ng-cloak ) input(type="hidden", name="_csrf", value=csrfToken) @@ -41,4 +42,4 @@ block content button.btn.btn-primary( type='submit', ng-disabled="passwordResetForm.$invalid" - ) #{translate("set_new_password")} \ No newline at end of file + ) #{translate("set_new_password")} diff --git a/services/web/app/views/user/settings.jade b/services/web/app/views/user/settings.jade index 3cf36a5012..da63e8b19d 100644 --- a/services/web/app/views/user/settings.jade +++ b/services/web/app/views/user/settings.jade @@ -17,7 +17,7 @@ block content .row .col-md-5 h3 #{translate("update_account_info")} - form(async-form="settings", name="settingsForm", action="/user/settings", novalidate) + form(async-form="settings", name="settingsForm", method="POST", action="/user/settings", novalidate) input(type="hidden", name="_csrf", value=csrfToken) .form-group label(for='email') #{translate("email")} @@ -54,7 +54,7 @@ block content .col-md-5.col-md-offset-1 h3 #{translate("change_password")} - form(async-form="changepassword", name="changePasswordForm", action="/user/password/update", novalidate) + form(async-form="changepassword", name="changePasswordForm", action="/user/password/update", method="POST", novalidate) input(type="hidden", name="_csrf", value=csrfToken) .form-group label(for='currentPassword') #{translate("current_password")} From 5e570d52a0e52debcab5db37c0e0b8fff757ee5e Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Wed, 26 Nov 2014 17:19:21 +0000 Subject: [PATCH 03/13] modfied setRootDocAutomatically to work async was causing cpu to block for along time on big projects --- .../Project/ProjectRootDocManager.coffee | 29 ++++++++++++------- .../Project/ProjectRootDocManagerTests.coffee | 21 +++++++------- 2 files changed, 30 insertions(+), 20 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectRootDocManager.coffee b/services/web/app/coffee/Features/Project/ProjectRootDocManager.coffee index ca9dba02c6..579c12c742 100644 --- a/services/web/app/coffee/Features/Project/ProjectRootDocManager.coffee +++ b/services/web/app/coffee/Features/Project/ProjectRootDocManager.coffee @@ -1,18 +1,27 @@ ProjectEntityHandler = require "./ProjectEntityHandler" Path = require "path" +async = require("async") +_ = require("underscore") module.exports = ProjectRootDocManager = setRootDocAutomatically: (project_id, callback = (error) ->) -> ProjectEntityHandler.getAllDocs project_id, (error, docs) -> return callback(error) if error? - root_doc_id = null - for path, doc of docs - for line in doc.lines || [] - match = line.match /(.*)\\documentclass/ # no lookbehind in js regexp :( - if Path.extname(path).match(/\.R?tex$/) and match and !match[1].match /%/ - root_doc_id = doc._id - if root_doc_id? - ProjectEntityHandler.setRootDoc project_id, root_doc_id, callback - else - callback() + + + root_doc_id = null + jobs = _.map docs, (doc, path)-> + return (cb)-> + for line in doc.lines || [] + match = line.match /(.*)\\documentclass/ # no lookbehind in js regexp :( + isRootDoc = Path.extname(path).match(/\.R?tex$/) and match and !match[1].match /%/ + if isRootDoc + return cb(doc?._id) + else + return cb() + async.parallel jobs, (root_doc_id)-> + if root_doc_id? + ProjectEntityHandler.setRootDoc project_id, root_doc_id, callback + else + callback() diff --git a/services/web/test/UnitTests/coffee/Project/ProjectRootDocManagerTests.coffee b/services/web/test/UnitTests/coffee/Project/ProjectRootDocManagerTests.coffee index fbd12c6ed4..a155b1b388 100644 --- a/services/web/test/UnitTests/coffee/Project/ProjectRootDocManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Project/ProjectRootDocManagerTests.coffee @@ -14,7 +14,7 @@ describe 'ProjectRootDocManager', -> describe "setRootDocAutomatically", -> describe "when there is a suitable root doc", -> - beforeEach -> + beforeEach (done)-> @docs = "/chapter1.tex": _id: "doc-id-1" @@ -22,9 +22,16 @@ describe 'ProjectRootDocManager', -> "/main.tex": _id: "doc-id-2" lines: ["\\documentclass{article}", "\\input{chapter1}"] + "/nested/chapter1a.tex": + _id: "doc-id-3" + lines: ["Hello world"] + "/nested/chapter1b.tex": + _id: "doc-id-4" + lines: ["Hello world"] + @ProjectEntityHandler.getAllDocs = sinon.stub().callsArgWith(1, null, @docs) @ProjectEntityHandler.setRootDoc = sinon.stub().callsArgWith(2) - @ProjectRootDocManager.setRootDocAutomatically @project_id, @callback + @ProjectRootDocManager.setRootDocAutomatically @project_id, done it "should check the docs of the project", -> @ProjectEntityHandler.getAllDocs.calledWith(@project_id) @@ -34,9 +41,6 @@ describe 'ProjectRootDocManager', -> @ProjectEntityHandler.setRootDoc.calledWith(@project_id, "doc-id-2") .should.equal true - it "should call the callback", -> - @callback.called.should.equal true - describe "when the root doc is an Rtex file", -> beforeEach -> @docs = @@ -55,7 +59,7 @@ describe 'ProjectRootDocManager', -> .should.equal true describe "when there is no suitable root doc", -> - beforeEach -> + beforeEach (done)-> @docs = "/chapter1.tex": _id: "doc-id-1" @@ -65,11 +69,8 @@ describe 'ProjectRootDocManager', -> lines: ["%Example: \\documentclass{article}"] @ProjectEntityHandler.getAllDocs = sinon.stub().callsArgWith(1, null, @docs) @ProjectEntityHandler.setRootDoc = sinon.stub().callsArgWith(2) - @ProjectRootDocManager.setRootDocAutomatically @project_id, @callback + @ProjectRootDocManager.setRootDocAutomatically @project_id, done it "should not set the root doc to the doc containing a documentclass", -> @ProjectEntityHandler.setRootDoc.called.should.equal false - it "should call the callback", -> - @callback.called.should.equal true - From ca8a21c425d16ad73926eb3c93cf7b0007cb9b61 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Wed, 26 Nov 2014 21:53:57 +0000 Subject: [PATCH 04/13] change async to series no real gain from parallel, series might reduce the cpu load if it finds the doc early --- .../app/coffee/Features/Project/ProjectRootDocManager.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/app/coffee/Features/Project/ProjectRootDocManager.coffee b/services/web/app/coffee/Features/Project/ProjectRootDocManager.coffee index 579c12c742..98b5175c0d 100644 --- a/services/web/app/coffee/Features/Project/ProjectRootDocManager.coffee +++ b/services/web/app/coffee/Features/Project/ProjectRootDocManager.coffee @@ -19,7 +19,7 @@ module.exports = ProjectRootDocManager = return cb(doc?._id) else return cb() - async.parallel jobs, (root_doc_id)-> + async.series jobs, (root_doc_id)-> if root_doc_id? ProjectEntityHandler.setRootDoc project_id, root_doc_id, callback else From 280895bdf5df3de3990b447f563cb4475a7d387b Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Thu, 27 Nov 2014 10:46:52 +0000 Subject: [PATCH 05/13] added lock to update merger --- .../ThirdPartyDataStore/UpdateMerger.coffee | 45 ++++++++++------- .../UpdateMergerTests.coffee | 50 +++++++++++++------ 2 files changed, 61 insertions(+), 34 deletions(-) diff --git a/services/web/app/coffee/Features/ThirdPartyDataStore/UpdateMerger.coffee b/services/web/app/coffee/Features/ThirdPartyDataStore/UpdateMerger.coffee index 887168fc9e..d05443b6e9 100644 --- a/services/web/app/coffee/Features/ThirdPartyDataStore/UpdateMerger.coffee +++ b/services/web/app/coffee/Features/ThirdPartyDataStore/UpdateMerger.coffee @@ -6,26 +6,35 @@ Settings = require('settings-sharelatex') FileTypeManager = require('../Uploads/FileTypeManager') uuid = require('node-uuid') fs = require('fs') +LockManager = require("../../infrastructure/LockManager") + module.exports = mergeUpdate: (project_id, path, updateRequest, source, callback = (error) ->)-> self = @ logger.log project_id:project_id, path:path, "merging update from tpds" - projectLocator.findElementByPath project_id, path, (err, element)=> - # Returns an error if the element is not found - #return callback(err) if err? - logger.log project_id:project_id, path:path, "found element by path for merging update from tpds" - elementId = undefined - if element? - elementId = element._id - self.p.writeStreamToDisk project_id, elementId, updateRequest, (err, fsPath)-> - return callback(err) if err? - FileTypeManager.isBinary path, fsPath, (err, isFile)-> + LockManager.getLock project_id, (err)-> + if err? + logger.err project_id:project_id, "could not get lock for merge update" + return callback() + projectLocator.findElementByPath project_id, path, (err, element)=> + # Returns an error if the element is not found + #return callback(err) if err? + logger.log project_id:project_id, path:path, "found element by path for merging update from tpds" + elementId = undefined + if element? + elementId = element._id + self.p.writeStreamToDisk project_id, elementId, updateRequest, (err, fsPath)-> return callback(err) if err? - if isFile - self.p.processFile project_id, elementId, fsPath, path, source, callback #TODO clean up the stream written to disk here - else - self.p.processDoc project_id, elementId, fsPath, path, source, callback + FileTypeManager.isBinary path, fsPath, (err, isFile)-> + return callback(err) if err? + finish = (err)-> + LockManager.releaseLock project_id, -> + callback(err) + if isFile + self.p.processFile project_id, elementId, fsPath, path, source, finish #TODO clean up the stream written to disk here + else + self.p.processDoc project_id, elementId, fsPath, path, source, finish deleteUpdate: (project_id, path, source, callback)-> projectLocator.findElementByPath project_id, path, (err, element)-> @@ -38,7 +47,7 @@ module.exports = else if element.folders? type = 'folder' logger.log project_id:project_id, updateType:path, updateType:type, element:element, "processing update to delete entity from tpds" - editorController.deleteEntity project_id, element._id, type, source, (err)-> + editorController.deleteEntityWithoutLock project_id, element._id, type, source, (err)-> logger.log project_id:project_id, path:path, "finished processing update to delete entity from tpds" callback() @@ -55,7 +64,7 @@ module.exports = callback() else setupNewEntity project_id, path, (err, folder, fileName)-> - editorController.addDoc project_id, folder._id, fileName, docLines, source, (err)-> + editorController.addDocWithoutLock project_id, folder._id, fileName, docLines, source, (err)-> callback() processFile: (project_id, file_id, fsPath, path, source, callback)-> @@ -67,7 +76,7 @@ module.exports = if file_id? editorController.replaceFile project_id, file_id, fsPath, source, finish else - editorController.addFile project_id, folder._id, fileName, fsPath, source, finish + editorController.addFileWithoutLock project_id, folder._id, fileName, fsPath, source, finish writeStreamToDisk: (project_id, file_id, stream, callback = (err, fsPath)->)-> if !file_id? @@ -103,5 +112,5 @@ setupNewEntity = (project_id, path, callback)-> lastIndexOfSlash = path.lastIndexOf("/") fileName = path[lastIndexOfSlash+1 .. -1] folderPath = path[0 .. lastIndexOfSlash] - editorController.mkdirp project_id, folderPath, (err, newFolders, lastFolder)-> + editorController.mkdirpWithoutLock project_id, folderPath, (err, newFolders, lastFolder)-> callback err, lastFolder, fileName diff --git a/services/web/test/UnitTests/coffee/ThirdPartyDataStore/UpdateMergerTests.coffee b/services/web/test/UnitTests/coffee/ThirdPartyDataStore/UpdateMergerTests.coffee index 61ff8da023..4785a0517e 100644 --- a/services/web/test/UnitTests/coffee/ThirdPartyDataStore/UpdateMergerTests.coffee +++ b/services/web/test/UnitTests/coffee/ThirdPartyDataStore/UpdateMergerTests.coffee @@ -13,12 +13,16 @@ describe 'UpdateMerger :', -> @projectEntityHandler = {} @fs = {} @FileTypeManager = {} + @LockManager = + getLock:sinon.stub() + releaseLock:sinon.stub() @updateMerger = SandboxedModule.require modulePath, requires: '../Editor/EditorController': @editorController '../Project/ProjectLocator': @projectLocator '../Project/ProjectEntityHandler': @projectEntityHandler 'fs': @fs '../Uploads/FileTypeManager':@FileTypeManager + "../../infrastructure/LockManager": @LockManager 'logger-sharelatex': log: -> err: -> @@ -32,13 +36,17 @@ describe 'UpdateMerger :', -> @path = "/doc1" @fsPath = "file/system/path.tex" @updateMerger.p.writeStreamToDisk = sinon.stub().callsArgWith(3, null, @fsPath) + @LockManager.getLock.callsArgWith(1) + @LockManager.releaseLock.callsArgWith(1) @FileTypeManager.isBinary = sinon.stub() it 'should get the element id', (done)-> - @projectLocator.findElementByPath = sinon.spy() + @projectLocator.findElementByPath = sinon.stub().callsArgWith(2) + @FileTypeManager.isBinary.callsArgWith(2, null, false) + @updateMerger.p.processDoc = sinon.stub().callsArgWith(5) @updateMerger.mergeUpdate @project_id, @path, @update, @source, => - @projectLocator.findElementByPath.calledWith(@project_id, @path).should.equal true - done() + @projectLocator.findElementByPath.calledWith(@project_id, @path).should.equal true + done() it 'should process update as doc when it is a doc', (done)-> doc_id = "231312s" @@ -64,6 +72,16 @@ describe 'UpdateMerger :', -> @FileTypeManager.isBinary.calledWith(filePath, @fsPath).should.equal true done() + it "should get the lock", (done)-> + + @FileTypeManager.isBinary.callsArgWith(2, null, false) + @projectLocator.findElementByPath = (_, __, cb)->cb(null, {_id:"doc_id"}) + @updateMerger.p.processDoc = sinon.stub().callsArgWith(5) + + @updateMerger.mergeUpdate @project_id, @path, @update, @source, => + @LockManager.getLock.calledWith(@project_id).should.equal true + done() + describe 'processDoc :', (done)-> beforeEach -> @@ -87,9 +105,9 @@ describe 'UpdateMerger :', -> folder = {_id:"adslkjioj"} docName = "main.tex" path = "folder1/folder2/#{docName}" - @editorController.mkdirp = sinon.stub().withArgs(@project_id).callsArgWith(2, null, [folder], folder) - @editorController.addDoc = -> - mock = sinon.mock(@editorController).expects("addDoc").withArgs(@project_id, folder._id, docName, @splitDocLines, @source).callsArg(5) + @editorController.mkdirpWithoutLock = sinon.stub().withArgs(@project_id).callsArgWith(2, null, [folder], folder) + @editorController.addDocWithoutLock = -> + mock = sinon.mock(@editorController).expects("addDocWithoutLock").withArgs(@project_id, folder._id, docName, @splitDocLines, @source).callsArg(5) @update.write(@docLines) @update.end() @@ -106,22 +124,22 @@ describe 'UpdateMerger :', -> @folder = _id: @folder_id @fileName = "file.png" @fsPath = "fs/path.tex" - @editorController.addFile = sinon.stub().callsArg(5) + @editorController.addFileWithoutLock = sinon.stub().callsArg(5) @editorController.replaceFile = sinon.stub().callsArg(4) - @editorController.deleteEntity = sinon.stub() - @editorController.mkdirp = sinon.stub().withArgs(@project_id).callsArgWith(2, null, [@folder], @folder) + @editorController.deleteEntityWithoutLock = sinon.stub() + @editorController.mkdirpWithoutLock = sinon.stub().withArgs(@project_id).callsArgWith(2, null, [@folder], @folder) @updateMerger.p.writeStreamToDisk = sinon.stub().withArgs(@project_id, @file_id, @update).callsArgWith(3, null, @fsPath) it 'should replace file if the file already exists', (done)-> @updateMerger.p.processFile @project_id, @file_id, @fsPath, @path, @source, => - @editorController.addFile.called.should.equal false + @editorController.addFileWithoutLock.called.should.equal false @editorController.replaceFile.calledWith(@project_id, @file_id, @fsPath, @source).should.equal true done() it 'should call add file if the file does not exist', (done)-> @updateMerger.p.processFile @project_id, undefined, @fsPath, @path, @source, => - @editorController.mkdirp.calledWith(@project_id, "folder/").should.equal true - @editorController.addFile.calledWith(@project_id, @folder_id, @fileName, @fsPath, @source).should.equal true + @editorController.mkdirpWithoutLock.calledWith(@project_id, "folder/").should.equal true + @editorController.addFileWithoutLock.calledWith(@project_id, @folder_id, @fileName, @fsPath, @source).should.equal true @editorController.replaceFile.called.should.equal false done() @@ -129,7 +147,7 @@ describe 'UpdateMerger :', -> beforeEach -> @path = "folder/doc1" - @editorController.deleteEntity = -> + @editorController.deleteEntityWithoutLock = -> @entity_id = "entity_id_here" @entity = _id:@entity_id @projectLocator.findElementByPath = (project_id, path, cb)=> cb(null, @entity, @path) @@ -141,20 +159,20 @@ describe 'UpdateMerger :', -> it 'should delete the entity in the editor controller with type doc when entity has docLines array', (done)-> @entity.lines = [] - mock = sinon.mock(@editorController).expects("deleteEntity").withArgs(@project_id, @entity_id, "doc", @source).callsArg(4) + mock = sinon.mock(@editorController).expects("deleteEntityWithoutLock").withArgs(@project_id, @entity_id, "doc", @source).callsArg(4) @updateMerger.deleteUpdate @project_id, @path, @source, -> mock.verify() done() it 'should delete the entity in the editor controller with type folder when entity has folders array', (done)-> @entity.folders = [] - mock = sinon.mock(@editorController).expects("deleteEntity").withArgs(@project_id, @entity_id, "folder", @source).callsArg(4) + mock = sinon.mock(@editorController).expects("deleteEntityWithoutLock").withArgs(@project_id, @entity_id, "folder", @source).callsArg(4) @updateMerger.deleteUpdate @project_id, @path, @source, -> mock.verify() done() it 'should delete the entity in the editor controller with type file when entity has no interesting properties', (done)-> - mock = sinon.mock(@editorController).expects("deleteEntity").withArgs(@project_id, @entity_id, "file", @source).callsArg(4) + mock = sinon.mock(@editorController).expects("deleteEntityWithoutLock").withArgs(@project_id, @entity_id, "file", @source).callsArg(4) @updateMerger.deleteUpdate @project_id, @path, @source, -> mock.verify() done() From 63deb0a508210849442f957e5b9545d6dc1dd325 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Thu, 27 Nov 2014 11:46:17 +0000 Subject: [PATCH 06/13] Revert "added lock to update merger" This reverts commit 8cdac1d74fd63d6ef83ce1e60ba41b9195ed0cac. --- .../ThirdPartyDataStore/UpdateMerger.coffee | 45 +++++++---------- .../UpdateMergerTests.coffee | 50 ++++++------------- 2 files changed, 34 insertions(+), 61 deletions(-) diff --git a/services/web/app/coffee/Features/ThirdPartyDataStore/UpdateMerger.coffee b/services/web/app/coffee/Features/ThirdPartyDataStore/UpdateMerger.coffee index d05443b6e9..887168fc9e 100644 --- a/services/web/app/coffee/Features/ThirdPartyDataStore/UpdateMerger.coffee +++ b/services/web/app/coffee/Features/ThirdPartyDataStore/UpdateMerger.coffee @@ -6,35 +6,26 @@ Settings = require('settings-sharelatex') FileTypeManager = require('../Uploads/FileTypeManager') uuid = require('node-uuid') fs = require('fs') -LockManager = require("../../infrastructure/LockManager") - module.exports = mergeUpdate: (project_id, path, updateRequest, source, callback = (error) ->)-> self = @ logger.log project_id:project_id, path:path, "merging update from tpds" - LockManager.getLock project_id, (err)-> - if err? - logger.err project_id:project_id, "could not get lock for merge update" - return callback() - projectLocator.findElementByPath project_id, path, (err, element)=> - # Returns an error if the element is not found - #return callback(err) if err? - logger.log project_id:project_id, path:path, "found element by path for merging update from tpds" - elementId = undefined - if element? - elementId = element._id - self.p.writeStreamToDisk project_id, elementId, updateRequest, (err, fsPath)-> + projectLocator.findElementByPath project_id, path, (err, element)=> + # Returns an error if the element is not found + #return callback(err) if err? + logger.log project_id:project_id, path:path, "found element by path for merging update from tpds" + elementId = undefined + if element? + elementId = element._id + self.p.writeStreamToDisk project_id, elementId, updateRequest, (err, fsPath)-> + return callback(err) if err? + FileTypeManager.isBinary path, fsPath, (err, isFile)-> return callback(err) if err? - FileTypeManager.isBinary path, fsPath, (err, isFile)-> - return callback(err) if err? - finish = (err)-> - LockManager.releaseLock project_id, -> - callback(err) - if isFile - self.p.processFile project_id, elementId, fsPath, path, source, finish #TODO clean up the stream written to disk here - else - self.p.processDoc project_id, elementId, fsPath, path, source, finish + if isFile + self.p.processFile project_id, elementId, fsPath, path, source, callback #TODO clean up the stream written to disk here + else + self.p.processDoc project_id, elementId, fsPath, path, source, callback deleteUpdate: (project_id, path, source, callback)-> projectLocator.findElementByPath project_id, path, (err, element)-> @@ -47,7 +38,7 @@ module.exports = else if element.folders? type = 'folder' logger.log project_id:project_id, updateType:path, updateType:type, element:element, "processing update to delete entity from tpds" - editorController.deleteEntityWithoutLock project_id, element._id, type, source, (err)-> + editorController.deleteEntity project_id, element._id, type, source, (err)-> logger.log project_id:project_id, path:path, "finished processing update to delete entity from tpds" callback() @@ -64,7 +55,7 @@ module.exports = callback() else setupNewEntity project_id, path, (err, folder, fileName)-> - editorController.addDocWithoutLock project_id, folder._id, fileName, docLines, source, (err)-> + editorController.addDoc project_id, folder._id, fileName, docLines, source, (err)-> callback() processFile: (project_id, file_id, fsPath, path, source, callback)-> @@ -76,7 +67,7 @@ module.exports = if file_id? editorController.replaceFile project_id, file_id, fsPath, source, finish else - editorController.addFileWithoutLock project_id, folder._id, fileName, fsPath, source, finish + editorController.addFile project_id, folder._id, fileName, fsPath, source, finish writeStreamToDisk: (project_id, file_id, stream, callback = (err, fsPath)->)-> if !file_id? @@ -112,5 +103,5 @@ setupNewEntity = (project_id, path, callback)-> lastIndexOfSlash = path.lastIndexOf("/") fileName = path[lastIndexOfSlash+1 .. -1] folderPath = path[0 .. lastIndexOfSlash] - editorController.mkdirpWithoutLock project_id, folderPath, (err, newFolders, lastFolder)-> + editorController.mkdirp project_id, folderPath, (err, newFolders, lastFolder)-> callback err, lastFolder, fileName diff --git a/services/web/test/UnitTests/coffee/ThirdPartyDataStore/UpdateMergerTests.coffee b/services/web/test/UnitTests/coffee/ThirdPartyDataStore/UpdateMergerTests.coffee index 4785a0517e..61ff8da023 100644 --- a/services/web/test/UnitTests/coffee/ThirdPartyDataStore/UpdateMergerTests.coffee +++ b/services/web/test/UnitTests/coffee/ThirdPartyDataStore/UpdateMergerTests.coffee @@ -13,16 +13,12 @@ describe 'UpdateMerger :', -> @projectEntityHandler = {} @fs = {} @FileTypeManager = {} - @LockManager = - getLock:sinon.stub() - releaseLock:sinon.stub() @updateMerger = SandboxedModule.require modulePath, requires: '../Editor/EditorController': @editorController '../Project/ProjectLocator': @projectLocator '../Project/ProjectEntityHandler': @projectEntityHandler 'fs': @fs '../Uploads/FileTypeManager':@FileTypeManager - "../../infrastructure/LockManager": @LockManager 'logger-sharelatex': log: -> err: -> @@ -36,17 +32,13 @@ describe 'UpdateMerger :', -> @path = "/doc1" @fsPath = "file/system/path.tex" @updateMerger.p.writeStreamToDisk = sinon.stub().callsArgWith(3, null, @fsPath) - @LockManager.getLock.callsArgWith(1) - @LockManager.releaseLock.callsArgWith(1) @FileTypeManager.isBinary = sinon.stub() it 'should get the element id', (done)-> - @projectLocator.findElementByPath = sinon.stub().callsArgWith(2) - @FileTypeManager.isBinary.callsArgWith(2, null, false) - @updateMerger.p.processDoc = sinon.stub().callsArgWith(5) + @projectLocator.findElementByPath = sinon.spy() @updateMerger.mergeUpdate @project_id, @path, @update, @source, => - @projectLocator.findElementByPath.calledWith(@project_id, @path).should.equal true - done() + @projectLocator.findElementByPath.calledWith(@project_id, @path).should.equal true + done() it 'should process update as doc when it is a doc', (done)-> doc_id = "231312s" @@ -72,16 +64,6 @@ describe 'UpdateMerger :', -> @FileTypeManager.isBinary.calledWith(filePath, @fsPath).should.equal true done() - it "should get the lock", (done)-> - - @FileTypeManager.isBinary.callsArgWith(2, null, false) - @projectLocator.findElementByPath = (_, __, cb)->cb(null, {_id:"doc_id"}) - @updateMerger.p.processDoc = sinon.stub().callsArgWith(5) - - @updateMerger.mergeUpdate @project_id, @path, @update, @source, => - @LockManager.getLock.calledWith(@project_id).should.equal true - done() - describe 'processDoc :', (done)-> beforeEach -> @@ -105,9 +87,9 @@ describe 'UpdateMerger :', -> folder = {_id:"adslkjioj"} docName = "main.tex" path = "folder1/folder2/#{docName}" - @editorController.mkdirpWithoutLock = sinon.stub().withArgs(@project_id).callsArgWith(2, null, [folder], folder) - @editorController.addDocWithoutLock = -> - mock = sinon.mock(@editorController).expects("addDocWithoutLock").withArgs(@project_id, folder._id, docName, @splitDocLines, @source).callsArg(5) + @editorController.mkdirp = sinon.stub().withArgs(@project_id).callsArgWith(2, null, [folder], folder) + @editorController.addDoc = -> + mock = sinon.mock(@editorController).expects("addDoc").withArgs(@project_id, folder._id, docName, @splitDocLines, @source).callsArg(5) @update.write(@docLines) @update.end() @@ -124,22 +106,22 @@ describe 'UpdateMerger :', -> @folder = _id: @folder_id @fileName = "file.png" @fsPath = "fs/path.tex" - @editorController.addFileWithoutLock = sinon.stub().callsArg(5) + @editorController.addFile = sinon.stub().callsArg(5) @editorController.replaceFile = sinon.stub().callsArg(4) - @editorController.deleteEntityWithoutLock = sinon.stub() - @editorController.mkdirpWithoutLock = sinon.stub().withArgs(@project_id).callsArgWith(2, null, [@folder], @folder) + @editorController.deleteEntity = sinon.stub() + @editorController.mkdirp = sinon.stub().withArgs(@project_id).callsArgWith(2, null, [@folder], @folder) @updateMerger.p.writeStreamToDisk = sinon.stub().withArgs(@project_id, @file_id, @update).callsArgWith(3, null, @fsPath) it 'should replace file if the file already exists', (done)-> @updateMerger.p.processFile @project_id, @file_id, @fsPath, @path, @source, => - @editorController.addFileWithoutLock.called.should.equal false + @editorController.addFile.called.should.equal false @editorController.replaceFile.calledWith(@project_id, @file_id, @fsPath, @source).should.equal true done() it 'should call add file if the file does not exist', (done)-> @updateMerger.p.processFile @project_id, undefined, @fsPath, @path, @source, => - @editorController.mkdirpWithoutLock.calledWith(@project_id, "folder/").should.equal true - @editorController.addFileWithoutLock.calledWith(@project_id, @folder_id, @fileName, @fsPath, @source).should.equal true + @editorController.mkdirp.calledWith(@project_id, "folder/").should.equal true + @editorController.addFile.calledWith(@project_id, @folder_id, @fileName, @fsPath, @source).should.equal true @editorController.replaceFile.called.should.equal false done() @@ -147,7 +129,7 @@ describe 'UpdateMerger :', -> beforeEach -> @path = "folder/doc1" - @editorController.deleteEntityWithoutLock = -> + @editorController.deleteEntity = -> @entity_id = "entity_id_here" @entity = _id:@entity_id @projectLocator.findElementByPath = (project_id, path, cb)=> cb(null, @entity, @path) @@ -159,20 +141,20 @@ describe 'UpdateMerger :', -> it 'should delete the entity in the editor controller with type doc when entity has docLines array', (done)-> @entity.lines = [] - mock = sinon.mock(@editorController).expects("deleteEntityWithoutLock").withArgs(@project_id, @entity_id, "doc", @source).callsArg(4) + mock = sinon.mock(@editorController).expects("deleteEntity").withArgs(@project_id, @entity_id, "doc", @source).callsArg(4) @updateMerger.deleteUpdate @project_id, @path, @source, -> mock.verify() done() it 'should delete the entity in the editor controller with type folder when entity has folders array', (done)-> @entity.folders = [] - mock = sinon.mock(@editorController).expects("deleteEntityWithoutLock").withArgs(@project_id, @entity_id, "folder", @source).callsArg(4) + mock = sinon.mock(@editorController).expects("deleteEntity").withArgs(@project_id, @entity_id, "folder", @source).callsArg(4) @updateMerger.deleteUpdate @project_id, @path, @source, -> mock.verify() done() it 'should delete the entity in the editor controller with type file when entity has no interesting properties', (done)-> - mock = sinon.mock(@editorController).expects("deleteEntityWithoutLock").withArgs(@project_id, @entity_id, "file", @source).callsArg(4) + mock = sinon.mock(@editorController).expects("deleteEntity").withArgs(@project_id, @entity_id, "file", @source).callsArg(4) @updateMerger.deleteUpdate @project_id, @path, @source, -> mock.verify() done() From 0bc76b6d92f7bcca736e227f56c085dcff9c9dbf Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Thu, 27 Nov 2014 12:07:49 +0000 Subject: [PATCH 07/13] mkdir p changed so it does not get entire project, now without doc lines --- .../coffee/Features/Project/ProjectEntityHandler.coffee | 8 ++++---- .../coffee/Project/ProjectEntityHandlerTests.coffee | 1 + 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index 45d169e27b..732c0c88f4 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -195,13 +195,13 @@ module.exports = ProjectEntityHandler = tpdsUpdateSender.addFile {project_id:project._id, file_id:fileRef._id, path:result.path.fileSystem, rev:fileRef.rev, project_name:project.name}, (error) -> callback(error, fileRef, folder_id) - mkdirp: (project_or_id, path, callback = (err, newlyCreatedFolders, lastFolderInPath)->)-> + mkdirp: (project_id, path, callback = (err, newlyCreatedFolders, lastFolderInPath)->)-> self = @ folders = path.split('/') folders = _.select folders, (folder)-> return folder.length != 0 - Project.getProject project_or_id, "", (err, project)=> + ProjectGetter.getProjectWithoutDocLines project_id, (err, project)=> if path == '/' logger.log project_id: project._id, "mkdir is only trying to make path of / so sending back root folder" return callback(null, [], project.rootFolder[0]) @@ -214,10 +214,10 @@ module.exports = ProjectEntityHandler = if parentFolder? parentFolder_id = parentFolder._id builtUpPath = "#{builtUpPath}/#{folderName}" - projectLocator.findElementByPath project_or_id, builtUpPath, (err, foundFolder)=> + projectLocator.findElementByPath project_id, builtUpPath, (err, foundFolder)=> if !foundFolder? logger.log path:path, project_id:project._id, folderName:folderName, "making folder from mkdirp" - @addFolder project_or_id, parentFolder_id, folderName, (err, newFolder, parentFolder_id)-> + @addFolder project_id, parentFolder_id, folderName, (err, newFolder, parentFolder_id)-> newFolder.parentFolder_id = parentFolder_id previousFolders.push newFolder callback null, previousFolders diff --git a/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee b/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee index 29731de247..c25b126e82 100644 --- a/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee @@ -77,6 +77,7 @@ describe 'ProjectEntityHandler', -> @parentFolder_id = "1jnjknjk" @newFolder = {_id:"newFolder_id_here"} @lastFolder = {_id:"123das", folders:[]} + @ProjectGetter.getProjectWithoutDocLines = sinon.stub().callsArgWith(1, null, @project) @projectLocator.findElementByPath = (project_id, path, cb)=> @parentFolder = {_id:"parentFolder_id_here"} lastFolder = path.substring(path.lastIndexOf("/")) From fa755c65217c2be11fd4b12c1f043d0b0c7e7c5e Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Thu, 27 Nov 2014 15:42:37 +0000 Subject: [PATCH 08/13] added project to long check from clsi2 (bit of a punt) --- .../web/app/coffee/Features/Compile/ClsiManager.coffee | 2 ++ services/web/app/views/project/editor/pdf.jade | 8 ++++++++ .../coffee/ide/pdf/controllers/PdfController.coffee | 2 ++ 3 files changed, 12 insertions(+) diff --git a/services/web/app/coffee/Features/Compile/ClsiManager.coffee b/services/web/app/coffee/Features/Compile/ClsiManager.coffee index 59ce3b4724..1f6cec8fab 100755 --- a/services/web/app/coffee/Features/Compile/ClsiManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiManager.coffee @@ -42,6 +42,8 @@ module.exports = ClsiManager = return callback(error) if error? if 200 <= response.statusCode < 300 callback null, body + else if response.statusCode == 413 + callback null, "project-too-large" else error = new Error("CLSI returned non-success code: #{response.statusCode}") logger.error err: error, project_id: project_id, "CLSI returned failure code" diff --git a/services/web/app/views/project/editor/pdf.jade b/services/web/app/views/project/editor/pdf.jade index 6d59fd0584..1abcb75ae4 100644 --- a/services/web/app/views/project/editor/pdf.jade +++ b/services/web/app/views/project/editor/pdf.jade @@ -102,6 +102,14 @@ div.full-size.pdf(ng-controller="PdfController") ng-click="hello('compile-timeout')" ) #{translate("start_free_trial")} + .pdf-errors(ng-show="pdf.projectTooLarge") + .alert.alert-danger + strong #{translate("project_too_large")} + span #{translate("project_too_large_please_reduce")} + + + + .pdf-logs(ng-show="(pdf.view == 'logs' || pdf.failure) && !pdf.error && !pdf.timeout && !pdf.uncompiled") diff --git a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee index e5eaf6f352..d4e2c726b4 100644 --- a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee +++ b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee @@ -32,6 +32,8 @@ define [ $scope.pdf.timedout = true else if response.status == "autocompile-backoff" $scope.pdf.uncompiled = true + else if response.status == "project-too-large" + $scope.pdf.projectTooLarge = true else if response.status == "failure" $scope.pdf.failure = true fetchLogs() From 20a5ad6e2737c6d4f88159927fbb077d9bcb982d Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Thu, 27 Nov 2014 16:22:39 +0000 Subject: [PATCH 09/13] fixed where project to large code is passed from --- 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 1f6cec8fab..032d806665 100755 --- a/services/web/app/coffee/Features/Compile/ClsiManager.coffee +++ b/services/web/app/coffee/Features/Compile/ClsiManager.coffee @@ -43,7 +43,7 @@ module.exports = ClsiManager = if 200 <= response.statusCode < 300 callback null, body else if response.statusCode == 413 - callback null, "project-too-large" + callback null, compile:status:"project-too-large" else error = new Error("CLSI returned non-success code: #{response.statusCode}") logger.error err: error, project_id: project_id, "CLSI returned failure code" From dbd7b9582377967d205263e68d0c39d910f6fab5 Mon Sep 17 00:00:00 2001 From: James Allen Date: Fri, 28 Nov 2014 13:27:25 +0000 Subject: [PATCH 10/13] Close spell check menu on scroll so it doesn't appear in the wrong position --- .../spell-check/SpellCheckManager.coffee | 6 ++ .../spell-check/SpellingMenuView.coffee | 82 ------------------- 2 files changed, 6 insertions(+), 82 deletions(-) delete mode 100644 services/web/public/coffee/ide/editor/directives/aceEditor/spell-check/SpellingMenuView.coffee diff --git a/services/web/public/coffee/ide/editor/directives/aceEditor/spell-check/SpellCheckManager.coffee b/services/web/public/coffee/ide/editor/directives/aceEditor/spell-check/SpellCheckManager.coffee index 8b73ce6df9..bdddb2ab20 100644 --- a/services/web/public/coffee/ide/editor/directives/aceEditor/spell-check/SpellCheckManager.coffee +++ b/services/web/public/coffee/ide/editor/directives/aceEditor/spell-check/SpellCheckManager.coffee @@ -17,12 +17,18 @@ define [ onChange = (e) => @runCheckOnChange(e) + + onScroll = () => + @closeContextMenu() @editor.on "changeSession", (e) => @runSpellCheckSoon(200) e.oldSession?.getDocument().off "change", onChange e.session.getDocument().on "change", onChange + + e.oldSession?.off "changeScrollTop", onScroll + e.session.on "changeScrollTop", onScroll @$scope.spellingMenu = {left: '0px', top: '0px'} diff --git a/services/web/public/coffee/ide/editor/directives/aceEditor/spell-check/SpellingMenuView.coffee b/services/web/public/coffee/ide/editor/directives/aceEditor/spell-check/SpellingMenuView.coffee deleted file mode 100644 index 3ba9fd1d96..0000000000 --- a/services/web/public/coffee/ide/editor/directives/aceEditor/spell-check/SpellingMenuView.coffee +++ /dev/null @@ -1,82 +0,0 @@ -define [ - "libs/backbone" - "libs/mustache" -], () -> - SUGGESTIONS_TO_SHOW = 5 - - SpellingMenuView = Backbone.View.extend - templates: - menu: $("#spellingMenuTemplate").html() - entry: $("#spellingMenuEntryTemplate").html() - - events: - "click a#learnWord": -> - @trigger "click:learn", @_currentHighlight - @hide() - - initialize: (options) -> - @ide = options.ide - @ide.editor.getContainerElement().append @render().el - @ide.editor.on "click", () => @hide() - @ide.editor.on "scroll", () => @hide() - @ide.editor.on "update:doc", () => @hide() - @ide.editor.on "change:doc", () => @hide() - - render: () -> - @setElement($(@templates.menu)) - @$el.css "z-index" : 10000 - @$(".dropdown-toggle").dropdown() - @hide() - return @ - - showForHighlight: (highlight) -> - if @_currentHighlight? and highlight != @_currentHighlight - @_close() - - if !@_currentHighlight? - @_currentHighlight = highlight - @_setSuggestions(highlight) - position = @ide.editor.textToEditorCoordinates( - highlight.row - highlight.column + highlight.word.length - ) - @_position(position.x, position.y) - @_show() - - hideIfAppropriate: (cursorPosition) -> - if @_currentHighlight? - if !@_cursorCloseToHighlight(cursorPosition, @_currentHighlight) and !@_isOpen() - @hide() - - hide: () -> - delete @_currentHighlight - @_close() - @$el.hide() - - _setSuggestions: (highlight) -> - @$(".spelling-suggestion").remove() - divider = @$(".divider") - for suggestion in highlight.suggestions.slice(0, SUGGESTIONS_TO_SHOW) - do (suggestion) => - entry = $(Mustache.to_html(@templates.entry, word: suggestion)) - divider.before(entry) - entry.on "click", () => - @trigger "click:suggestion", suggestion, highlight - - _show: () -> @$el.show() - - _isOpen: () -> - @$(".dropdown-menu").is(":visible") - - _close: () -> - if @_isOpen() - @$el.dropdown("toggle") - - _cursorCloseToHighlight: (position, highlight) -> - position.row == highlight.row and - position.column >= highlight.column and - position.column <= highlight.column + highlight.word.length + 1 - - _position: (x,y) -> @$el.css left: x, top: y - - From 2c5f3c728ce1701eafa6f52d3ccf992e2db205d5 Mon Sep 17 00:00:00 2001 From: James Allen Date: Fri, 28 Nov 2014 13:58:27 +0000 Subject: [PATCH 11/13] Highlight \vref like \ref --- services/web/public/js/ace/mode-latex.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/public/js/ace/mode-latex.js b/services/web/public/js/ace/mode-latex.js index 84d5b8048b..886b5c058e 100644 --- a/services/web/public/js/ace/mode-latex.js +++ b/services/web/public/js/ace/mode-latex.js @@ -15,7 +15,7 @@ var LatexHighlightRules = function() { regex : "(\\\\(?:documentclass|usepackage))(?:(\\[)([^\\]]*)(\\]))?({)([^}]*)(})" }, { token : ["keyword","lparen", "variable.parameter", "rparen"], - regex : "(\\\\(?:label|ref|cite(?:[^{]*)))(?:({)([^}]*)(}))?" + regex : "(\\\\(?:label|v?ref|cite(?:[^{]*)))(?:({)([^}]*)(}))?" }, { token : ["storage.type", "lparen", "variable.parameter", "rparen"], regex : "(\\\\(?:begin|end))({)(\\w*)(})" From 175dfae0856759b6c9f3d6c7777a6db0426abd5b Mon Sep 17 00:00:00 2001 From: James Allen Date: Fri, 28 Nov 2014 14:26:21 +0000 Subject: [PATCH 12/13] Look up compile group and features from project owner, not current user --- .../Features/Compile/CompileController.coffee | 65 ++++++++++--------- .../Features/Compile/CompileManager.coffee | 24 +++++-- .../ide/pdf/controllers/PdfController.coffee | 3 +- .../Compile/CompileControllerTests.coffee | 35 ++++------ .../coffee/Compile/CompileManagerTests.coffee | 51 +++++++++++++-- 5 files changed, 114 insertions(+), 64 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/CompileController.coffee b/services/web/app/coffee/Features/Compile/CompileController.coffee index 387c9eb950..edb42f3e1f 100755 --- a/services/web/app/coffee/Features/Compile/CompileController.coffee +++ b/services/web/app/coffee/Features/Compile/CompileController.coffee @@ -13,21 +13,25 @@ module.exports = CompileController = res.setTimeout(5 * 60 * 1000) project_id = req.params.Project_id isAutoCompile = !!req.query?.auto_compile - settingsOverride = req.body?.settingsOverride ? {}; - logger.log "root doc overriden" if settingsOverride.rootDoc_id? AuthenticationController.getLoggedInUserId req, (error, user_id) -> return next(error) if error? - UserGetter.getUser user_id, {"features.compileGroup":1, "features.compileTimeout":1}, (err, user)-> - settingsOverride.timeout = user?.features?.compileTimeout || Settings.defaultFeatures.compileTimeout - settingsOverride.compiler = user?.features?.compileGroup || Settings.defaultFeatures.compileGroup - req.session.compileGroup = settingsOverride.compiler - CompileManager.compile project_id, user_id, { isAutoCompile, settingsOverride }, (error, status, outputFiles) -> - return next(error) if error? - res.contentType("application/json") - res.send 200, JSON.stringify { - status: status - outputFiles: outputFiles - } + options = { + isAutoCompile: isAutoCompile + } + if req.body?.rootDoc_id? + options.rootDoc_id = req.body.rootDoc_id + else if req.body?.settingsOverride?.rootDoc_id? # Can be removed after deploy + options.rootDoc_id = req.body.settingsOverride.rootDoc_id + if req.body?.compiler + options.compiler = req.body.compiler + logger.log {options, project_id}, "got compile request" + CompileManager.compile project_id, user_id, options, (error, status, outputFiles) -> + return next(error) if error? + res.contentType("application/json") + res.send 200, JSON.stringify { + status: status + outputFiles: outputFiles + } downloadPdf: (req, res, next = (error) ->)-> Metrics.inc "pdf-downloads" @@ -40,7 +44,7 @@ module.exports = CompileController = else logger.log project_id: project_id, "download pdf to embed in browser" res.header('Content-Disposition', "filename=#{project.getSafeProjectName()}.pdf") - CompileController.proxyToClsi("/project/#{project_id}/output/output.pdf", req, res, next) + CompileController.proxyToClsi(project_id, "/project/#{project_id}/output/output.pdf", req, res, next) deleteAuxFiles: (req, res, next) -> project_id = req.params.Project_id @@ -55,25 +59,28 @@ module.exports = CompileController = logger.err err:err, project_id:project_id, "something went wrong compile and downloading pdf" res.send 500 url = "/project/#{project_id}/output/output.pdf" - CompileController.proxyToClsi url, req, res, next + CompileController.proxyToClsi project_id, url, req, res, next getFileFromClsi: (req, res, next = (error) ->) -> - CompileController.proxyToClsi("/project/#{req.params.Project_id}/output/#{req.params.file}", req, res, next) + project_id = req.params.Project_id + CompileController.proxyToClsi(project_id, "/project/#{project_id}/output/#{req.params.file}", req, res, next) proxySync: (req, res, next = (error) ->) -> - CompileController.proxyToClsi(req.url, req, res, next) + CompileController.proxyToClsi(req.params.Project_id, req.url, req, res, next) - proxyToClsi: (url, req, res, next = (error) ->) -> - if req.session.compileGroup == "priority" - compilerUrl = Settings.apis.clsi_priority.url - else - compilerUrl = Settings.apis.clsi.url - url = "#{compilerUrl}#{url}" - logger.log url: url, "proxying to CLSI" - oneMinute = 60 * 1000 - proxy = request(url: url, method: req.method, timeout: oneMinute) - proxy.pipe(res) - proxy.on "error", (error) -> - logger.warn err: error, url: url, "CLSI proxy error" + proxyToClsi: (project_id, url, req, res, next = (error) ->) -> + CompileManager.getProjectCompileLimits project_id, (error, limits) -> + return next(error) if error? + if limits.compileGroup == "priority" + compilerUrl = Settings.apis.clsi_priority.url + else + compilerUrl = Settings.apis.clsi.url + url = "#{compilerUrl}#{url}" + logger.log url: url, "proxying to CLSI" + oneMinute = 60 * 1000 + proxy = request(url: url, method: req.method, timeout: oneMinute) + proxy.pipe(res) + proxy.on "error", (error) -> + logger.warn err: error, url: url, "CLSI proxy error" diff --git a/services/web/app/coffee/Features/Compile/CompileManager.coffee b/services/web/app/coffee/Features/Compile/CompileManager.coffee index 870ad14d6c..1835fa0c0c 100755 --- a/services/web/app/coffee/Features/Compile/CompileManager.coffee +++ b/services/web/app/coffee/Features/Compile/CompileManager.coffee @@ -6,19 +6,20 @@ rclient = redis.createClient(Settings.redis.web) DocumentUpdaterHandler = require "../DocumentUpdater/DocumentUpdaterHandler" Project = require("../../models/Project").Project ProjectRootDocManager = require "../Project/ProjectRootDocManager" +UserGetter = require "../User/UserGetter" ClsiManager = require "./ClsiManager" Metrics = require('../../infrastructure/Metrics') logger = require("logger-sharelatex") rateLimiter = require("../../infrastructure/RateLimiter") module.exports = CompileManager = - compile: (project_id, user_id, opt = {}, _callback = (error) ->) -> + compile: (project_id, user_id, options = {}, _callback = (error) ->) -> timer = new Metrics.Timer("editor.compile") callback = (args...) -> timer.done() _callback(args...) - @_checkIfAutoCompileLimitHasBeenHit opt.isAutoCompile, (err, canCompile)-> + @_checkIfAutoCompileLimitHasBeenHit options.isAutoCompile, (err, canCompile)-> if !canCompile return callback null, "autocompile-backoff", [] logger.log project_id: project_id, user_id: user_id, "compiling project" @@ -31,11 +32,24 @@ module.exports = CompileManager = return callback(error) if error? DocumentUpdaterHandler.flushProjectToMongo project_id, (error) -> return callback(error) if error? - ClsiManager.sendRequest project_id, opt.settingsOverride, (error, status, outputFiles) -> + CompileManager.getProjectCompileLimits project_id, (error, limits) -> return callback(error) if error? - logger.log files: outputFiles, "output files" - callback(null, status, outputFiles) + for key, value of limits + options[key] = value + ClsiManager.sendRequest project_id, options, (error, status, outputFiles, output) -> + return callback(error) if error? + logger.log files: outputFiles, "output files" + callback(null, status, outputFiles, output) + getProjectCompileLimits: (project_id, callback = (error, limits) ->) -> + Project.findById project_id, {owner_ref: 1}, (error, project) -> + return callback(error) if error? + UserGetter.getUser project.owner_ref, {"features":1}, (err, owner)-> + return callback(error) if error? + callback null, { + timeout: owner.features?.compileTimeout || Settings.defaultFeatures.compileTimeout + compileGroup: owner.features?.compileGroup || Settings.defaultFeatures.compileGroup + } getLogLines: (project_id, callback)-> Metrics.inc "editor.raw-logs" diff --git a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee index d4e2c726b4..3ab4687e8a 100644 --- a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee +++ b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee @@ -15,8 +15,7 @@ define [ if options.isAutoCompile url += "?auto_compile=true" return $http.post url, { - settingsOverride: - rootDoc_id: options.rootDocOverride_id or null + rootDoc_id: options.rootDocOverride_id or null _csrf: window.csrfToken } diff --git a/services/web/test/UnitTests/coffee/Compile/CompileControllerTests.coffee b/services/web/test/UnitTests/coffee/Compile/CompileControllerTests.coffee index 8d6925577b..f9fb7134b7 100644 --- a/services/web/test/UnitTests/coffee/Compile/CompileControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Compile/CompileControllerTests.coffee @@ -21,7 +21,7 @@ describe "CompileController", -> clsi: url: "clsi.example.com" clsi_priority: - url: "clsi.example.com" + url: "clsi-priority.example.com" "request": @request = sinon.stub() "../../models/Project": Project: @Project = {} "logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub() } @@ -47,8 +47,7 @@ describe "CompileController", -> Project_id: @project_id @req.session = {} @AuthenticationController.getLoggedInUserId = sinon.stub().callsArgWith(1, null, @user_id = "mock-user-id") - @CompileManager.compile = sinon.stub().callsArgWith(3, null, @status = "success", @outputFiles = ["mock-output-files"]) - @UserGetter.getUser.callsArgWith(2, null, @user) + @CompileManager.compile = sinon.stub().callsArgWith(3, null, @status = "success", @outputFiles = ["mock-output-files"], @output = "mock-output") @CompileController.compile @req, @res, @next it "should look up the user id", -> @@ -58,7 +57,7 @@ describe "CompileController", -> it "should do the compile without the auto compile flag", -> @CompileManager.compile - .calledWith(@project_id, @user_id, { isAutoCompile: false, settingsOverride:{timeout:@user.features.compileTimeout, compiler:@user.features.compileGroup} }) + .calledWith(@project_id, @user_id, { isAutoCompile: false }) .should.equal true it "should set the content-type of the response to application/json", -> @@ -73,16 +72,6 @@ describe "CompileController", -> outputFiles: @outputFiles }) - it "should get the compile timeout from the users features",-> - @UserGetter.getUser.args[0][0].should.equal @user_id - assert.deepEqual @UserGetter.getUser.args[0][1], {"features.compileGroup":1, "features.compileTimeout":1} - - it "should put the compile group on the req", -> - @req.session.compileGroup.should.equal @user.features.compileGroup - - it "should set the timeout", -> - assert @res.timout > 1000 * 60 * 3 - describe "when an auto compile", -> beforeEach -> @req.params = @@ -91,11 +80,12 @@ describe "CompileController", -> auto_compile: "true" @AuthenticationController.getLoggedInUserId = sinon.stub().callsArgWith(1, null, @user_id = "mock-user-id") @CompileManager.compile = sinon.stub().callsArgWith(3, null, @status = "success", @outputFiles = ["mock-output-files"]) - @UserGetter.getUser.callsArgWith(2, null, @user) @CompileController.compile @req, @res, @next it "should do the compile with the auto compile flag", -> - @CompileManager.compile.calledWith(@project_id, @user_id, { isAutoCompile: true, settingsOverride:{timeout:@user.features.compileTimeout, compiler:@user.features.compileGroup} }).should.equal true + @CompileManager.compile + .calledWith(@project_id, @user_id, { isAutoCompile: true }) + .should.equal true describe "downloadPdf", -> beforeEach -> @@ -134,7 +124,7 @@ describe "CompileController", -> it "should proxy the PDF from the CLSI", -> @CompileController.proxyToClsi - .calledWith("/project/#{@project_id}/output/output.pdf", @req, @res, @next) + .calledWith(@project_id, "/project/#{@project_id}/output/output.pdf", @req, @res, @next) .should.equal true describe "proxyToClsi", -> @@ -152,8 +142,8 @@ describe "CompileController", -> describe "user with standard priority", -> beforeEach -> - @UserGetter.getUser.callsArgWith(2, null, @user) - @CompileController.proxyToClsi(@url = "/test", @req, @res, @next) + @CompileManager.getProjectCompileLimits = sinon.stub().callsArgWith(1, null, {compileGroup: "standard"}) + @CompileController.proxyToClsi(@project_id, @url = "/test", @req, @res, @next) it "should open a request to the CLSI", -> @@ -176,9 +166,8 @@ describe "CompileController", -> describe "user with priority compile", -> beforeEach -> - @req.session.compileGroup = "priority" - @UserGetter.getUser.callsArgWith(2, null, @user) - @CompileController.proxyToClsi(@url = "/test", @req, @res, @next) + @CompileManager.getProjectCompileLimits = sinon.stub().callsArgWith(1, null, {compileGroup: "priority"}) + @CompileController.proxyToClsi(@project_id, @url = "/test", @req, @res, @next) it "should proxy to the priorty url if the user has the feature", ()-> @request @@ -225,5 +214,5 @@ describe "CompileController", -> it "should proxy the res to the clsi with correct url", (done)-> @CompileController.compileAndDownloadPdf @req, @res - @CompileController.proxyToClsi.calledWith("/project/#{@project_id}/output/output.pdf", @req, @res).should.equal true + @CompileController.proxyToClsi.calledWith(@project_id, "/project/#{@project_id}/output/output.pdf", @req, @res).should.equal true done() diff --git a/services/web/test/UnitTests/coffee/Compile/CompileManagerTests.coffee b/services/web/test/UnitTests/coffee/Compile/CompileManagerTests.coffee index 7e3a0b2f91..7e183e8050 100644 --- a/services/web/test/UnitTests/coffee/Compile/CompileManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Compile/CompileManagerTests.coffee @@ -20,6 +20,7 @@ describe "CompileManager", -> "../DocumentUpdater/DocumentUpdaterHandler": @DocumentUpdaterHandler = {} "../Project/ProjectRootDocManager": @ProjectRootDocManager = {} "../../models/Project": Project: @Project = {} + "../User/UserGetter": @UserGetter = {} "./ClsiManager": @ClsiManager = {} "../../infrastructure/RateLimiter": @ratelimiter "../../infrastructure/Metrics": @Metrics = @@ -30,13 +31,18 @@ describe "CompileManager", -> @project_id = "mock-project-id-123" @user_id = "mock-user-id-123" @callback = sinon.stub() + @limits = { + timeout: 42 + } + describe "compile", -> beforeEach -> @CompileManager._checkIfRecentlyCompiled = sinon.stub().callsArgWith(2, null, false) @CompileManager._ensureRootDocumentIsSet = sinon.stub().callsArgWith(1, null) @DocumentUpdaterHandler.flushProjectToMongo = sinon.stub().callsArgWith(1, null) - @ClsiManager.sendRequest = sinon.stub().callsArgWith(2, null, @status = "mock-status") + @CompileManager.getProjectCompileLimits = sinon.stub().callsArgWith(1, null, @limits) + @ClsiManager.sendRequest = sinon.stub().callsArgWith(2, null, @status = "mock-status", @outputFiles = "mock output files", @output = "mock output") describe "succesfully", -> beforeEach -> @@ -58,14 +64,21 @@ describe "CompileManager", -> .calledWith(@project_id) .should.equal true - it "should run the compile with the new compiler API", -> - @ClsiManager.sendRequest + it "should get the project compile limits", -> + @CompileManager.getProjectCompileLimits .calledWith(@project_id) .should.equal true - it "should call the callback", -> + it "should run the compile with the compile limits", -> + @ClsiManager.sendRequest + .calledWith(@project_id, { + timeout: @limits.timeout + }) + .should.equal true + + it "should call the callback with the output", -> @callback - .calledWith(null, @status) + .calledWith(null, @status, @outputFiles, @output) .should.equal true it "should time the compile", -> @@ -93,6 +106,34 @@ describe "CompileManager", -> @CompileManager.compile @project_id, @user_id, {}, (err, status)-> status.should.equal "autocompile-backoff" done() + + describe "getProjectCompileLimits", -> + beforeEach -> + @features = { + compileTimeout: @timeout = 42 + compileGroup: @group = "priority" + } + @Project.findById = sinon.stub().callsArgWith(2, null, @project = { owner_ref: @owner_id = "owner-id-123" }) + @UserGetter.getUser = sinon.stub().callsArgWith(2, null, @user = { features: @features }) + @CompileManager.getProjectCompileLimits @project_id, @callback + + it "should look up the owner of the project", -> + @Project.findById + .calledWith(@project_id, { owner_ref: 1 }) + .should.equal true + + it "should look up the owner's features", -> + @UserGetter.getUser + .calledWith(@project.owner_ref, { features: 1 }) + .should.equal true + + it "should return the limits", -> + @callback + .calledWith(null, { + timeout: @timeout + compileGroup: @group + }) + .should.equal true describe "getLogLines", -> beforeEach -> From a570d051324ebd09c3e22274a4e2dd22ee470593 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Mon, 1 Dec 2014 01:07:03 +0000 Subject: [PATCH 13/13] fixed bug with setting root doc which would fail if the document class isn't on the top line --- .../coffee/Features/Project/ProjectRootDocManager.coffee | 8 +++++--- .../coffee/Project/ProjectRootDocManagerTests.coffee | 4 ++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectRootDocManager.coffee b/services/web/app/coffee/Features/Project/ProjectRootDocManager.coffee index 98b5175c0d..da2743d661 100644 --- a/services/web/app/coffee/Features/Project/ProjectRootDocManager.coffee +++ b/services/web/app/coffee/Features/Project/ProjectRootDocManager.coffee @@ -5,6 +5,7 @@ _ = require("underscore") module.exports = ProjectRootDocManager = setRootDocAutomatically: (project_id, callback = (error) ->) -> + ProjectEntityHandler.getAllDocs project_id, (error, docs) -> return callback(error) if error? @@ -12,13 +13,14 @@ module.exports = ProjectRootDocManager = root_doc_id = null jobs = _.map docs, (doc, path)-> return (cb)-> + rootDocId = null for line in doc.lines || [] match = line.match /(.*)\\documentclass/ # no lookbehind in js regexp :( isRootDoc = Path.extname(path).match(/\.R?tex$/) and match and !match[1].match /%/ if isRootDoc - return cb(doc?._id) - else - return cb() + rootDocId = doc?._id + cb(rootDocId) + async.series jobs, (root_doc_id)-> if root_doc_id? ProjectEntityHandler.setRootDoc project_id, root_doc_id, callback diff --git a/services/web/test/UnitTests/coffee/Project/ProjectRootDocManagerTests.coffee b/services/web/test/UnitTests/coffee/Project/ProjectRootDocManagerTests.coffee index a155b1b388..c4b7d17d11 100644 --- a/services/web/test/UnitTests/coffee/Project/ProjectRootDocManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Project/ProjectRootDocManagerTests.coffee @@ -18,10 +18,10 @@ describe 'ProjectRootDocManager', -> @docs = "/chapter1.tex": _id: "doc-id-1" - lines: ["\\begin{document}", "Hello world", "\\end{document}"] + lines: ["something else","\\begin{document}", "Hello world", "\\end{document}"] "/main.tex": _id: "doc-id-2" - lines: ["\\documentclass{article}", "\\input{chapter1}"] + lines: ["different line","\\documentclass{article}", "\\input{chapter1}"] "/nested/chapter1a.tex": _id: "doc-id-3" lines: ["Hello world"]