From 50686090c8e4aa7a71a102ba9960db07f24a2e56 Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Fri, 6 Apr 2018 14:50:56 +0100 Subject: [PATCH 1/5] push doc pathname logic into DocController --- .../Documents/DocumentController.coffee | 29 +++++++++------ .../Project/ProjectEntityHandler.coffee | 10 +---- .../Documents/DocumentControllerTests.coffee | 20 ++++++---- .../Project/ProjectEntityHandlerTests.coffee | 37 ++++--------------- 4 files changed, 37 insertions(+), 59 deletions(-) diff --git a/services/web/app/coffee/Features/Documents/DocumentController.coffee b/services/web/app/coffee/Features/Documents/DocumentController.coffee index 3cd6f7c8b4..3c47edabc4 100644 --- a/services/web/app/coffee/Features/Documents/DocumentController.coffee +++ b/services/web/app/coffee/Features/Documents/DocumentController.coffee @@ -1,3 +1,4 @@ +ProjectLocator = require "../Project/ProjectLocator" ProjectEntityHandler = require "../Project/ProjectEntityHandler" ProjectEntityUpdateHandler = require "../Project/ProjectEntityUpdateHandler" logger = require("logger-sharelatex") @@ -8,21 +9,25 @@ module.exports = doc_id = req.params.doc_id plain = req?.query?.plain == 'true' logger.log doc_id:doc_id, project_id:project_id, "receiving get document request from api (docupdater)" - ProjectEntityHandler.getDoc project_id, doc_id, {pathname: true}, (error, lines, rev, version, ranges, pathname) -> + ProjectLocator.findElement {project_id: project_id, element_id: doc_id, type: 'doc'}, (error, doc, path) => if error? logger.err err:error, doc_id:doc_id, project_id:project_id, "error finding element for getDocument" return next(error) - if plain - res.type "text/plain" - res.send lines.join('\n') - else - res.type "json" - res.send JSON.stringify { - lines: lines - version: version - ranges: ranges - pathname: pathname - } + ProjectEntityHandler.getDoc project_id, doc_id, (error, lines, rev, version, ranges) -> + if error? + logger.err err:error, doc_id:doc_id, project_id:project_id, "error finding doc contents for getDocument" + return next(error) + if plain + res.type "text/plain" + res.send lines.join('\n') + else + res.type "json" + res.send JSON.stringify { + lines: lines + version: version + ranges: ranges + pathname: path.fileSystem + } setDocument: (req, res, next = (error) ->) -> project_id = req.params.Project_id diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index 85e41292da..5994521b8e 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -6,7 +6,6 @@ DocstoreManager = require "../Docstore/DocstoreManager" DocumentUpdaterHandler = require('../../Features/DocumentUpdater/DocumentUpdaterHandler') Errors = require '../Errors/Errors' Project = require('../../models/Project').Project -ProjectLocator = require('./ProjectLocator') ProjectGetter = require "./ProjectGetter" TpdsUpdateSender = require('../ThirdPartyDataStore/TpdsUpdateSender') @@ -105,14 +104,7 @@ module.exports = ProjectEntityHandler = self = callback = options options = {} - if options["pathname"] - delete options["pathname"] - ProjectLocator.findElement {project_id: project_id, element_id: doc_id, type: 'doc'}, (error, doc, path) => - return callback(error) if error? - DocstoreManager.getDoc project_id, doc_id, options, (error, lines, rev, version, ranges) => - callback(error, lines, rev, version, ranges, path.fileSystem) - else - DocstoreManager.getDoc project_id, doc_id, options, callback + DocstoreManager.getDoc project_id, doc_id, options, callback _getAllFolders: (project_id, callback) -> logger.log project_id:project_id, "getting all folders for project" diff --git a/services/web/test/unit/coffee/Documents/DocumentControllerTests.coffee b/services/web/test/unit/coffee/Documents/DocumentControllerTests.coffee index 30c12f48df..c373cb93f8 100644 --- a/services/web/test/unit/coffee/Documents/DocumentControllerTests.coffee +++ b/services/web/test/unit/coffee/Documents/DocumentControllerTests.coffee @@ -15,6 +15,7 @@ describe "DocumentController", -> "logger-sharelatex": log:-> err:-> + "../Project/ProjectLocator": @ProjectLocator = {} "../Project/ProjectEntityHandler": @ProjectEntityHandler = {} "../Project/ProjectEntityUpdateHandler": @ProjectEntityUpdateHandler = {} @res = new MockResponse() @@ -36,12 +37,19 @@ describe "DocumentController", -> describe "when the document exists", -> beforeEach -> - @ProjectEntityHandler.getDoc = sinon.stub().callsArgWith(3, null, @doc_lines, @rev, @version, @ranges, @pathname) + @doc = _id: @doc_id + @ProjectLocator.findElement = sinon.stub().callsArgWith(1, null, @doc, fileSystem: @pathname) + @ProjectEntityHandler.getDoc = sinon.stub().callsArgWith(2, null, @doc_lines, @rev, @version, @ranges) @DocumentController.getDocument(@req, @res, @next) - it "should get the document from Mongo", -> + it "should get the pathname of the document", -> + @ProjectLocator.findElement + .calledWith({project_id: @project_id, element_id: @doc_id, type: 'doc'}) + .should.equal true + + it "should get the document content", -> @ProjectEntityHandler.getDoc - .calledWith(@project_id, @doc_id, pathname: true) + .calledWith(@project_id, @doc_id) .should.equal true it "should return the document data to the client as JSON", -> @@ -54,7 +62,7 @@ describe "DocumentController", -> describe "when the document doesn't exist", -> beforeEach -> - @ProjectEntityHandler.getDoc = sinon.stub().callsArgWith(3, new Errors.NotFoundError("not found"), null) + @ProjectLocator.findElement = sinon.stub().callsArgWith(1, new Errors.NotFoundError("not found")) @DocumentController.getDocument(@req, @res, @next) it "should call next with the NotFoundError", -> @@ -94,7 +102,3 @@ describe "DocumentController", -> it "should call next with the NotFoundError", -> @next.calledWith(new Errors.NotFoundError("not found")) .should.equal true - - - - diff --git a/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee b/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee index 08f87b056f..77a4816f4c 100644 --- a/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee +++ b/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee @@ -241,35 +241,12 @@ describe 'ProjectEntityHandler', -> @ranges = {"mock": "ranges"} @DocstoreManager.getDoc = sinon.stub().callsArgWith(3, null, @lines, @rev, @version, @ranges) + @ProjectEntityHandler.getDoc project_id, doc_id, @callback - describe 'without pathname option', -> - beforeEach -> - @ProjectEntityHandler.getDoc project_id, doc_id, @callback + it "should call the docstore", -> + @DocstoreManager.getDoc + .calledWith(project_id, doc_id) + .should.equal true - it "should call the docstore", -> - @DocstoreManager.getDoc - .calledWith(project_id, doc_id) - .should.equal true - - it "should call the callback with the lines, version and rev", -> - @callback.calledWith(null, @lines, @rev, @version, @ranges).should.equal true - - describe 'with pathname option', -> - beforeEach -> - @project = 'a project' - @path = mongo: "mongo.path", fileSystem: "/file/system/path" - @ProjectLocator.findElement = sinon.stub().callsArgWith(1, null, {}, @path) - @ProjectEntityHandler.getDoc project_id, doc_id, {pathname: true}, @callback - - it "should call the project locator", -> - @ProjectLocator.findElement - .calledWith({project_id: project_id, element_id: doc_id, type: 'doc'}) - .should.equal true - - it "should call the docstore", -> - @DocstoreManager.getDoc - .calledWith(project_id, doc_id) - .should.equal true - - it "should return the pathname if option given", -> - @callback.calledWith(null, @lines, @rev, @version, @ranges, @path.fileSystem).should.equal true + it "should call the callback with the lines, version and rev", -> + @callback.calledWith(null, @lines, @rev, @version, @ranges).should.equal true From e5f3c472e3c2ceb1bd75b3e3563b93241fd3ffae Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Fri, 13 Apr 2018 13:43:21 +0100 Subject: [PATCH 2/5] return projectHistoryId in DocumentController.getDoc --- .../Documents/DocumentController.coffee | 41 ++++++----- .../Documents/DocumentControllerTests.coffee | 72 ++++++++++++++----- 2 files changed, 79 insertions(+), 34 deletions(-) diff --git a/services/web/app/coffee/Features/Documents/DocumentController.coffee b/services/web/app/coffee/Features/Documents/DocumentController.coffee index 3c47edabc4..9d147e8287 100644 --- a/services/web/app/coffee/Features/Documents/DocumentController.coffee +++ b/services/web/app/coffee/Features/Documents/DocumentController.coffee @@ -1,3 +1,4 @@ +ProjectGetter = require "../Project/ProjectGetter" ProjectLocator = require "../Project/ProjectLocator" ProjectEntityHandler = require "../Project/ProjectEntityHandler" ProjectEntityUpdateHandler = require "../Project/ProjectEntityUpdateHandler" @@ -9,25 +10,31 @@ module.exports = doc_id = req.params.doc_id plain = req?.query?.plain == 'true' logger.log doc_id:doc_id, project_id:project_id, "receiving get document request from api (docupdater)" - ProjectLocator.findElement {project_id: project_id, element_id: doc_id, type: 'doc'}, (error, doc, path) => - if error? - logger.err err:error, doc_id:doc_id, project_id:project_id, "error finding element for getDocument" - return next(error) - ProjectEntityHandler.getDoc project_id, doc_id, (error, lines, rev, version, ranges) -> + ProjectGetter.getProject project_id, rootFolder: true, overleaf: true, (error, project) -> + return next(error) if error? + return res.sendStatus(404) if !project? + ProjectLocator.findElement {project: project, element_id: doc_id, type: 'doc'}, (error, doc, path) -> if error? - logger.err err:error, doc_id:doc_id, project_id:project_id, "error finding doc contents for getDocument" + logger.err err:error, doc_id:doc_id, project_id:project_id, "error finding element for getDocument" return next(error) - if plain - res.type "text/plain" - res.send lines.join('\n') - else - res.type "json" - res.send JSON.stringify { - lines: lines - version: version - ranges: ranges - pathname: path.fileSystem - } + ProjectEntityHandler.getDoc project_id, doc_id, (error, lines, rev, version, ranges) -> + if error? + logger.err err:error, doc_id:doc_id, project_id:project_id, "error finding doc contents for getDocument" + return next(error) + if plain + res.type "text/plain" + res.send lines.join('\n') + + else + projectHistoryId = project?.overleaf?.history?.id + res.type "json" + res.send JSON.stringify { + lines: lines + version: version + ranges: ranges + pathname: path.fileSystem + projectHistoryId: projectHistoryId + } setDocument: (req, res, next = (error) ->) -> project_id = req.params.Project_id diff --git a/services/web/test/unit/coffee/Documents/DocumentControllerTests.coffee b/services/web/test/unit/coffee/Documents/DocumentControllerTests.coffee index c373cb93f8..6f4db7f8d5 100644 --- a/services/web/test/unit/coffee/Documents/DocumentControllerTests.coffee +++ b/services/web/test/unit/coffee/Documents/DocumentControllerTests.coffee @@ -15,6 +15,7 @@ describe "DocumentController", -> "logger-sharelatex": log:-> err:-> + "../Project/ProjectGetter": @ProjectGetter = {} "../Project/ProjectLocator": @ProjectLocator = {} "../Project/ProjectEntityHandler": @ProjectEntityHandler = {} "../Project/ProjectEntityUpdateHandler": @ProjectEntityUpdateHandler = {} @@ -35,39 +36,76 @@ describe "DocumentController", -> Project_id: @project_id doc_id: @doc_id - describe "when the document exists", -> + describe "when the project exists without project history enabled", -> + beforeEach -> + @project = _id: @project_id + @ProjectGetter.getProject = sinon.stub().callsArgWith(2, null, @project) + + describe "when the document exists", -> + beforeEach -> + @doc = _id: @doc_id + @ProjectLocator.findElement = sinon.stub().callsArgWith(1, null, @doc, fileSystem: @pathname) + @ProjectEntityHandler.getDoc = sinon.stub().callsArgWith(2, null, @doc_lines, @rev, @version, @ranges) + @DocumentController.getDocument(@req, @res, @next) + + it "should get the project", -> + @ProjectGetter.getProject + .calledWith(@project_id, rootFolder: true, overleaf: true) + .should.equal true + + it "should get the pathname of the document", -> + @ProjectLocator.findElement + .calledWith({project: @project, element_id: @doc_id, type: 'doc'}) + .should.equal true + + it "should get the document content", -> + @ProjectEntityHandler.getDoc + .calledWith(@project_id, @doc_id) + .should.equal true + + it "should return the document data to the client as JSON", -> + @res.type.should.equal "json" + @res.body.should.equal JSON.stringify + lines: @doc_lines + version: @version + ranges: @ranges + pathname: @pathname + + describe "when the document doesn't exist", -> + beforeEach -> + @ProjectLocator.findElement = sinon.stub().callsArgWith(1, new Errors.NotFoundError("not found")) + @DocumentController.getDocument(@req, @res, @next) + + it "should call next with the NotFoundError", -> + @next.calledWith(new Errors.NotFoundError("not found")) + .should.equal true + + describe "when project exists with project history enabled", -> beforeEach -> @doc = _id: @doc_id + @projectHistoryId = 1234 + @project = _id: @project_id, overleaf: history: id: @projectHistoryId + @ProjectGetter.getProject = sinon.stub().callsArgWith(2, null, @project) @ProjectLocator.findElement = sinon.stub().callsArgWith(1, null, @doc, fileSystem: @pathname) @ProjectEntityHandler.getDoc = sinon.stub().callsArgWith(2, null, @doc_lines, @rev, @version, @ranges) @DocumentController.getDocument(@req, @res, @next) - it "should get the pathname of the document", -> - @ProjectLocator.findElement - .calledWith({project_id: @project_id, element_id: @doc_id, type: 'doc'}) - .should.equal true - - it "should get the document content", -> - @ProjectEntityHandler.getDoc - .calledWith(@project_id, @doc_id) - .should.equal true - - it "should return the document data to the client as JSON", -> + it "should return the history id to the client as JSON", -> @res.type.should.equal "json" @res.body.should.equal JSON.stringify lines: @doc_lines version: @version ranges: @ranges pathname: @pathname + projectHistoryId: @projectHistoryId - describe "when the document doesn't exist", -> + describe "when the project does not exist", -> beforeEach -> - @ProjectLocator.findElement = sinon.stub().callsArgWith(1, new Errors.NotFoundError("not found")) + @ProjectGetter.getProject = sinon.stub().callsArgWith(2, null, null) @DocumentController.getDocument(@req, @res, @next) - it "should call next with the NotFoundError", -> - @next.calledWith(new Errors.NotFoundError("not found")) - .should.equal true + it "returns a 404", -> + @res.statusCode.should.equal 404 describe "setDocument", -> beforeEach -> From 3006d018d9740dfa9bd2f6602ca77e5f771ef2cb Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Fri, 13 Apr 2018 13:43:11 +0100 Subject: [PATCH 3/5] send projectHistoryId with project structure updates --- .../DocumentUpdaterHandler.coffee | 10 ++++- .../Project/ProjectEntityUpdateHandler.coffee | 36 +++++++++++------- .../DocumentUpdaterHandlerTests.coffee | 19 +++++----- ...rojectEntityMongoUpdateHandlerTests.coffee | 4 +- .../ProjectEntityUpdateHandlerTests.coffee | 38 ++++++++++--------- 5 files changed, 62 insertions(+), 45 deletions(-) diff --git a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee index 9ecff2185e..a30ed7bdef 100644 --- a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee +++ b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee @@ -129,7 +129,7 @@ module.exports = DocumentUpdaterHandler = method: "POST" }, project_id, "resync-project-history", callback - updateProjectStructure : (project_id, userId, changes, callback = (error) ->)-> + updateProjectStructure: (project_id, projectHistoryId, userId, changes, callback = (error) ->)-> return callback() if !settings.apis.project_history?.sendProjectStructureOps Project.findOne {_id: project_id}, {version:true}, (err, currentProject) -> @@ -144,7 +144,13 @@ module.exports = DocumentUpdaterHandler = logger.log {project_id}, "updating project structure in doc updater" DocumentUpdaterHandler._makeRequest { path: "/project/#{project_id}" - json: { docUpdates, fileUpdates, userId, version: currentProject.version } + json: { + docUpdates, + fileUpdates, + userId, + version: currentProject.version + projectHistoryId + } method: "POST" }, project_id, "update-project-structure", callback diff --git a/services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee index e3aea07456..788d1b40eb 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee @@ -48,6 +48,7 @@ module.exports = ProjectEntityUpdateHandler = self = # this doesn't need any locking because it's only called by ProjectDuplicator copyFileFromExistingProjectWithProject: (project, folder_id, originalProject_id, origonalFileRef, userId, callback = (error, fileRef, folder_id) ->)-> project_id = project._id + projectHistoryId = project.overleaf?.history?.id logger.log { project_id, folder_id, originalProject_id, origonalFileRef }, "copying file in s3 with project" return callback(err) if err? ProjectEntityMongoUpdateHandler._confirmFolder project, folder_id, (folder_id)=> @@ -72,7 +73,7 @@ module.exports = ProjectEntityUpdateHandler = self = path: result?.path?.fileSystem url: fileStoreUrl ] - DocumentUpdaterHandler.updateProjectStructure project_id, userId, {newFiles}, (error) -> + DocumentUpdaterHandler.updateProjectStructure project_id, projectHistoryId, userId, {newFiles}, (error) -> return callback(error) if error? callback null, fileRef, folder_id @@ -131,14 +132,15 @@ module.exports = ProjectEntityUpdateHandler = self = self.addDoc project_id, null, name, lines, callback addDoc: wrapWithLock (project_id, folder_id, docName, docLines, userId, callback = (error, doc, folder_id) ->)=> - self.addDocWithoutUpdatingHistory.withoutLock project_id, folder_id, docName, docLines, userId, (error, doc, folder_id, path) -> + self.addDocWithoutUpdatingHistory.withoutLock project_id, folder_id, docName, docLines, userId, (error, doc, folder_id, path, project) -> return callback(error) if error? + projectHistoryId = project.overleaf?.history?.id newDocs = [ doc: doc path: path docLines: docLines.join('\n') ] - DocumentUpdaterHandler.updateProjectStructure project_id, userId, {newDocs}, (error) -> + DocumentUpdaterHandler.updateProjectStructure project_id, projectHistoryId, userId, {newDocs}, (error) -> return callback(error) if error? callback null, doc, folder_id @@ -173,12 +175,13 @@ module.exports = ProjectEntityUpdateHandler = self = withLock: (project_id, folder_id, fileName, fsPath, linkedFileData, userId, fileRef, fileStoreUrl, callback = (error, fileRef, folder_id) ->)-> ProjectEntityUpdateHandler._addFileAndSendToTpds project_id, folder_id, fileName, fileRef, (err, result, project) -> return callback(err) if err? + projectHistoryId = project.overleaf?.history?.id newFiles = [ file: fileRef path: result?.path?.fileSystem url: fileStoreUrl ] - DocumentUpdaterHandler.updateProjectStructure project_id, userId, {newFiles}, (error) -> + DocumentUpdaterHandler.updateProjectStructure project_id, projectHistoryId, userId, {newFiles}, (error) -> return callback(error) if error? callback(null, fileRef, folder_id) @@ -205,9 +208,10 @@ module.exports = ProjectEntityUpdateHandler = self = path: path.fileSystem url: fileStoreUrl ] + projectHistoryId = project.overleaf?.history?.id TpdsUpdateSender.addFile {project_id:project._id, file_id:newFileRef._id, path:path.fileSystem, rev:newFileRef.rev+1, project_name:project.name}, (err) -> return callback(err) if err? - DocumentUpdaterHandler.updateProjectStructure project_id, userId, {oldFiles, newFiles}, callback + DocumentUpdaterHandler.updateProjectStructure project_id, projectHistoryId, userId, {oldFiles, newFiles}, callback addDocWithoutUpdatingHistory: wrapWithLock (project_id, folder_id, docName, docLines, userId, callback = (error, doc, folder_id) ->)=> # This method should never be called directly, except when importing a project @@ -232,7 +236,7 @@ module.exports = ProjectEntityUpdateHandler = self = rev: 0 }, (err) -> return callback(err) if err? - callback(null, doc, folder_id, result?.path?.fileSystem) + callback(null, doc, folder_id, result?.path?.fileSystem, project) addFileWithoutUpdatingHistory: wrapWithLock # This method should never be called directly, except when importing a project @@ -363,10 +367,11 @@ module.exports = ProjectEntityUpdateHandler = self = logger.err {err: "No entityType set", project_id, entity_id} return callback("No entityType set") entityType = entityType.toLowerCase() - ProjectEntityMongoUpdateHandler.moveEntity project_id, entity_id, destFolderId, entityType, (err, project_name, startPath, endPath, rev, changes) -> + ProjectEntityMongoUpdateHandler.moveEntity project_id, entity_id, destFolderId, entityType, (err, project, startPath, endPath, rev, changes) -> return callback(err) if err? - TpdsUpdateSender.moveEntity { project_id, project_name, startPath, endPath, rev } - DocumentUpdaterHandler.updateProjectStructure project_id, userId, changes, callback + projectHistoryId = project.overleaf?.history?.id + TpdsUpdateSender.moveEntity { project_id, project_name: project.name, startPath, endPath, rev } + DocumentUpdaterHandler.updateProjectStructure project_id, projectHistoryId, userId, changes, callback renameEntity: wrapWithLock (project_id, entity_id, entityType, newName, userId, callback)-> if not SafePath.isCleanFilename newName @@ -377,10 +382,11 @@ module.exports = ProjectEntityUpdateHandler = self = return callback("No entityType set") entityType = entityType.toLowerCase() - ProjectEntityMongoUpdateHandler.renameEntity project_id, entity_id, entityType, newName, (err, project_name, startPath, endPath, rev, changes) -> + ProjectEntityMongoUpdateHandler.renameEntity project_id, entity_id, entityType, newName, (err, project, startPath, endPath, rev, changes) -> return callback(err) if err? - TpdsUpdateSender.moveEntity({project_id, startPath, endPath, project_name, rev}) - DocumentUpdaterHandler.updateProjectStructure project_id, userId, changes, callback + projectHistoryId = project.overleaf?.history?.id + TpdsUpdateSender.moveEntity { project_id, project_name: project.name, startPath, endPath, rev } + DocumentUpdaterHandler.updateProjectStructure project_id, projectHistoryId, userId, changes, callback # This doesn't directly update project structure but we need to take the lock # to prevent anything else being queued before the resync update @@ -433,16 +439,18 @@ module.exports = ProjectEntityUpdateHandler = self = DocstoreManager.deleteDoc project_id, doc_id, (error) -> return callback(error) if error? changes = oldDocs: [ {doc, path} ] - DocumentUpdaterHandler.updateProjectStructure project_id, userId, changes, callback + projectHistoryId = project.overleaf?.history?.id + DocumentUpdaterHandler.updateProjectStructure project_id, projectHistoryId, userId, changes, callback _cleanUpFile: (project, file, path, userId, callback = (error) ->) -> ProjectEntityMongoUpdateHandler._insertDeletedFileReference project._id, file, (error) -> return callback(error) if error? project_id = project._id.toString() + projectHistoryId = project.overleaf?.history?.id changes = oldFiles: [ {file, path} ] # we are now keeping a copy of every file versio so we no longer delete # the file from the filestore - DocumentUpdaterHandler.updateProjectStructure project_id, userId, changes, callback + DocumentUpdaterHandler.updateProjectStructure project_id, projectHistoryId, userId, changes, callback _cleanUpFolder: (project, folder, folderPath, userId, callback = (error) ->) -> jobs = [] diff --git a/services/web/test/unit/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee b/services/web/test/unit/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee index 47010c2b9d..837a0f5b78 100644 --- a/services/web/test/unit/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee +++ b/services/web/test/unit/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee @@ -12,6 +12,7 @@ modulePath = path.join __dirname, '../../../../app/js/Features/DocumentUpdater/D describe 'DocumentUpdaterHandler', -> beforeEach -> @project_id = "project-id-923" + @projectHistoryId = "ol-project-id-1" @doc_id = "doc-id-394" @lines = ["one", "two", "three"] @version = 42 @@ -409,7 +410,7 @@ describe 'DocumentUpdaterHandler', -> describe "with project history disabled", -> beforeEach -> @settings.apis.project_history.sendProjectStructureOps = false - @handler.updateProjectStructure @project_id, @user_id, {}, @callback + @handler.updateProjectStructure @project_id, @projectHistoryId, @user_id, {}, @callback it 'does not make a web request', -> @request.called.should.equal false @@ -445,11 +446,11 @@ describe 'DocumentUpdaterHandler', -> newPathname: "/new_b" ] - @handler.updateProjectStructure @project_id, @user_id, @changes, () => + @handler.updateProjectStructure @project_id, @projectHistoryId, @user_id, @changes, () => @request.calledWith( url: @url, method: "POST" - json: {docUpdates, fileUpdates: [], userId: @user_id, @version} + json: {docUpdates, fileUpdates: [], userId: @user_id, @version, @projectHistoryId} ) .should.equal true done() @@ -468,11 +469,11 @@ describe 'DocumentUpdaterHandler', -> url: undefined ] - @handler.updateProjectStructure @project_id, @user_id, @changes, () => + @handler.updateProjectStructure @project_id, @projectHistoryId, @user_id, @changes, () => @request.calledWith( url: @url method: "POST" - json: {docUpdates, fileUpdates: [], userId: @user_id, @version} + json: {docUpdates, fileUpdates: [], userId: @user_id, @version, @projectHistoryId} ).should.equal true done() @@ -490,11 +491,11 @@ describe 'DocumentUpdaterHandler', -> docLines: undefined ] - @handler.updateProjectStructure @project_id, @user_id, @changes, () => + @handler.updateProjectStructure @project_id, @projectHistoryId, @user_id, @changes, () => @request.calledWith( url: @url method: "POST" - json: {docUpdates: [], fileUpdates, userId: @user_id, @version} + json: {docUpdates: [], fileUpdates, userId: @user_id, @version, @projectHistoryId} ).should.equal true done() @@ -511,10 +512,10 @@ describe 'DocumentUpdaterHandler', -> newPathname: '' ] - @handler.updateProjectStructure @project_id, @user_id, @changes, () => + @handler.updateProjectStructure @project_id, @projectHistoryId, @user_id, @changes, () => @request.calledWith( url: @url method: "POST" - json: {docUpdates, fileUpdates: [], userId: @user_id, @version} + json: {docUpdates, fileUpdates: [], userId: @user_id, @version, @projectHistoryId} ).should.equal true done() diff --git a/services/web/test/unit/coffee/Project/ProjectEntityMongoUpdateHandlerTests.coffee b/services/web/test/unit/coffee/Project/ProjectEntityMongoUpdateHandlerTests.coffee index 57911bd7d8..a79deb15a9 100644 --- a/services/web/test/unit/coffee/Project/ProjectEntityMongoUpdateHandlerTests.coffee +++ b/services/web/test/unit/coffee/Project/ProjectEntityMongoUpdateHandlerTests.coffee @@ -267,7 +267,7 @@ describe 'ProjectEntityMongoUpdateHandler', -> it "calls the callback", -> changes = { @oldDocs, @newDocs, @oldFiles, @newFiles } @callback.calledWith( - null, @project.name, @path.fileSystem, @pathAfterMove.fileSystem, @doc.rev, changes + null, @project, @path.fileSystem, @pathAfterMove.fileSystem, @doc.rev, changes ).should.equal true describe 'deleteEntity', -> @@ -349,7 +349,7 @@ describe 'ProjectEntityMongoUpdateHandler', -> it 'calls the callback', -> changes = { @oldDocs, @newDocs, @oldFiles, @newFiles } @callback.calledWith( - null, @project.name, '/old.tex', '/new.tex', @doc.rev, changes + null, @project, '/old.tex', '/new.tex', @doc.rev, changes ).should.equal true describe 'addFolder', -> diff --git a/services/web/test/unit/coffee/Project/ProjectEntityUpdateHandlerTests.coffee b/services/web/test/unit/coffee/Project/ProjectEntityUpdateHandlerTests.coffee index 09d0d274fb..d7fe7fc003 100644 --- a/services/web/test/unit/coffee/Project/ProjectEntityUpdateHandlerTests.coffee +++ b/services/web/test/unit/coffee/Project/ProjectEntityUpdateHandlerTests.coffee @@ -10,6 +10,7 @@ ObjectId = require("mongoose").Types.ObjectId describe 'ProjectEntityUpdateHandler', -> project_id = '4eecb1c1bffa66588e0000a1' + projectHistoryId = '123456' doc_id = '4eecb1c1bffa66588e0000a2' file_id = "4eecaffcbffa66588e000009" folder_id = "4eecaffcbffa66588e000008" @@ -18,7 +19,12 @@ describe 'ProjectEntityUpdateHandler', -> userId = 1234 beforeEach -> - @project = _id: project_id, name: 'project name' + @project = + _id: project_id, + name: 'project name' + overleaf: + history: + id: projectHistoryId @fileUrl = 'filestore.example.com/file' @FileStoreHandler = uploadFileFromDisk: sinon.stub().yields(null, @fileUrl) @@ -112,7 +118,7 @@ describe 'ProjectEntityUpdateHandler', -> newFile.url == @fileUrl @DocumentUpdaterHandler.updateProjectStructure - .calledWithMatch(project_id, userId, changesMatcher) + .calledWithMatch(project_id, projectHistoryId, userId, changesMatcher) .should.equal true describe 'updateDocLines', -> @@ -278,7 +284,7 @@ describe 'ProjectEntityUpdateHandler', -> @newDoc = _id: doc_id @ProjectEntityUpdateHandler.addDocWithoutUpdatingHistory = - withoutLock: sinon.stub().yields(null, @newDoc, folder_id, @path) + withoutLock: sinon.stub().yields(null, @newDoc, folder_id, @path, @project) @ProjectEntityUpdateHandler.addDoc project_id, folder_id, @docName, @docLines, userId, @callback it "creates the doc without history", () -> @@ -293,7 +299,7 @@ describe 'ProjectEntityUpdateHandler', -> docLines: @docLines.join('\n') ] @DocumentUpdaterHandler.updateProjectStructure - .calledWith(project_id, userId, {newDocs}) + .calledWith(project_id, projectHistoryId, userId, {newDocs}) .should.equal true describe 'addFile', -> @@ -336,7 +342,7 @@ describe 'ProjectEntityUpdateHandler', -> url: @fileUrl ] @DocumentUpdaterHandler.updateProjectStructure - .calledWith(project_id, userId, {newFiles}) + .calledWith(project_id, projectHistoryId, userId, {newFiles}) .should.equal true describe 'replaceFile', -> @@ -348,7 +354,6 @@ describe 'ProjectEntityUpdateHandler', -> @newFile = _id: new_file_id, name: "dummy-upload-filename", rev: 0 @oldFile = _id: file_id @path = "/path/to/file" - @project = _id: project_id, name: 'some project' @ProjectEntityMongoUpdateHandler._insertDeletedFileReference = sinon.stub().yields() @ProjectEntityMongoUpdateHandler.replaceFileWithNew = sinon.stub().yields(null, @oldFile, @project, fileSystem: @path) @ProjectEntityUpdateHandler.replaceFile project_id, file_id, @fileSystemPath, @linkedFileData, userId, @callback @@ -385,7 +390,7 @@ describe 'ProjectEntityUpdateHandler', -> url: @newFileUrl ] @DocumentUpdaterHandler.updateProjectStructure - .calledWith(project_id, userId, {oldFiles, newFiles}) + .calledWith(project_id, projectHistoryId, userId, {oldFiles, newFiles}) .should.equal true describe 'addDocWithoutUpdatingHistory', -> @@ -725,7 +730,7 @@ describe 'ProjectEntityUpdateHandler', -> @rev = 2 @changes = newDocs: ['old-doc'], newFiles: ['old-file'] @ProjectEntityMongoUpdateHandler.moveEntity = sinon.stub().yields( - null, @project_name, @startPath, @endPath, @rev, @changes + null, @project, @startPath, @endPath, @rev, @changes ) @TpdsUpdateSender.moveEntity = sinon.stub() @DocumentUpdaterHandler.updateProjectStructure = sinon.stub() @@ -744,7 +749,7 @@ describe 'ProjectEntityUpdateHandler', -> it 'sends the changes in project structure to the doc updater', -> @DocumentUpdaterHandler.updateProjectStructure - .calledWith(project_id, userId, @changes, @callback) + .calledWith(project_id, projectHistoryId, userId, @changes, @callback) .should.equal true describe "renameEntity", -> @@ -756,7 +761,7 @@ describe 'ProjectEntityUpdateHandler', -> @changes = newDocs: ['old-doc'], newFiles: ['old-file'] @newDocName = 'b.tex' @ProjectEntityMongoUpdateHandler.renameEntity = sinon.stub().yields( - null, @project_name, @startPath, @endPath, @rev, @changes + null, @project, @startPath, @endPath, @rev, @changes ) @TpdsUpdateSender.moveEntity = sinon.stub() @DocumentUpdaterHandler.updateProjectStructure = sinon.stub() @@ -775,7 +780,7 @@ describe 'ProjectEntityUpdateHandler', -> it 'sends the changes in project structure to the doc updater', -> @DocumentUpdaterHandler.updateProjectStructure - .calledWith(project_id, userId, @changes, @callback) + .calledWith(project_id, projectHistoryId, userId, @changes, @callback) .should.equal true describe "resyncProjectHistory", -> @@ -791,7 +796,7 @@ describe 'ProjectEntityUpdateHandler', -> describe "a project without project-history enabled", -> beforeEach -> - @project.ovreleaf = {} + @project.overleaf = {} @ProjectGetter.getProject = sinon.stub().yields(null, @project) @ProjectEntityUpdateHandler.resyncProjectHistory project_id, @callback @@ -802,7 +807,6 @@ describe 'ProjectEntityUpdateHandler', -> describe "a project with project-history enabled", -> beforeEach -> - @project.overleaf = history: id: 4 @ProjectGetter.getProject = sinon.stub().yields(null, @project) docs = [ doc: _id: doc_id @@ -840,7 +844,7 @@ describe 'ProjectEntityUpdateHandler', -> url: "www.filestore.test/#{project_id}/#{file_id}" ] @DocumentUpdaterHandler.resyncProjectHistory - .calledWith(project_id, docs, files) + .calledWith(project_id, projectHistoryId, docs, files) .should.equal true it 'calls the callback', -> @@ -874,7 +878,7 @@ describe 'ProjectEntityUpdateHandler', -> it "should should send the update to the doc updater", -> oldFiles = [ file: @entity, path: @path ] @DocumentUpdaterHandler.updateProjectStructure - .calledWith(project_id, userId, {oldFiles}) + .calledWith(project_id, projectHistoryId, userId, {oldFiles}) .should.equal true describe "a doc", -> @@ -924,8 +928,6 @@ describe 'ProjectEntityUpdateHandler', -> describe "_cleanUpDoc", -> beforeEach -> - @project = - _id: ObjectId(project_id) @doc = _id: ObjectId() name: "test.tex" @@ -963,7 +965,7 @@ describe 'ProjectEntityUpdateHandler', -> it "should should send the update to the doc updater", -> oldDocs = [ doc: @doc, path: @path ] @DocumentUpdaterHandler.updateProjectStructure - .calledWith(project_id, userId, {oldDocs}) + .calledWith(project_id, projectHistoryId, userId, {oldDocs}) .should.equal true it "should call the callback", -> From 9ef40902ca90e47d9b7939d8f256d5ef5b04e740 Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Wed, 11 Apr 2018 10:08:15 +0100 Subject: [PATCH 4/5] send projectHistoryId with resync requests --- .../Features/DocumentUpdater/DocumentUpdaterHandler.coffee | 4 ++-- .../Features/Project/ProjectEntityUpdateHandler.coffee | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee index a30ed7bdef..71e99cc67c 100644 --- a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee +++ b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee @@ -121,11 +121,11 @@ module.exports = DocumentUpdaterHandler = method: "DELETE" }, project_id, "delete-thread", callback - resyncProjectHistory: (project_id, docs, files, callback) -> + resyncProjectHistory: (project_id, projectHistoryId, docs, files, callback) -> logger.info {project_id, docs, files}, "resyncing project history in doc updater" DocumentUpdaterHandler._makeRequest { path: "/project/#{project_id}/history/resync" - json: { docs, files } + json: { docs, files, projectHistoryId } method: "POST" }, project_id, "resync-project-history", callback diff --git a/services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee index 788d1b40eb..8815cba100 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee @@ -394,7 +394,8 @@ module.exports = ProjectEntityUpdateHandler = self = ProjectGetter.getProject project_id, rootFolder: true, overleaf: true, (error, project) -> return callback(error) if error? - if !project?.overleaf?.history?.id? + projectHistoryId = project?.overleaf?.history?.id + if !projectHistoryId? error = new Errors.ProjectHistoryDisabledError("project history not enabled for #{project_id}") return callback(error) @@ -410,7 +411,8 @@ module.exports = ProjectEntityUpdateHandler = self = path: file.path url: FileStoreHandler._buildUrl(project_id, file.file._id) - DocumentUpdaterHandler.resyncProjectHistory project_id, docs, files, callback + DocumentUpdaterHandler.resyncProjectHistory project_id, projectHistoryId, docs, files, callback + _cleanUpEntity: (project, entity, entityType, path, userId, callback = (error) ->) -> if(entityType.indexOf("file") != -1) self._cleanUpFile project, entity, path, userId, callback From 7755a7cc1f7d96bc349f5c4a4f70b2f184c5d217 Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Fri, 13 Apr 2018 13:33:35 +0100 Subject: [PATCH 5/5] add overleaf to projections for project structure updates --- .../ProjectEntityMongoUpdateHandler.coffee | 20 +++++++++---------- ...rojectEntityMongoUpdateHandlerTests.coffee | 16 +++++++-------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityMongoUpdateHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityMongoUpdateHandler.coffee index 57f65a67dc..37f7f83ec1 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityMongoUpdateHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityMongoUpdateHandler.coffee @@ -31,7 +31,7 @@ module.exports = ProjectEntityMongoUpdateHandler = self = LOCK_NAMESPACE: LOCK_NAMESPACE addDoc: wrapWithLock (project_id, folder_id, doc, callback = (err, result) ->) -> - ProjectGetter.getProjectWithoutLock project_id, {rootFolder:true, name:true}, (err, project) -> + ProjectGetter.getProjectWithoutLock project_id, {rootFolder:true, name:true, overleaf:true}, (err, project) -> if err? logger.err project_id:project_id, err:err, "error getting project for add doc" return callback(err) @@ -40,7 +40,7 @@ module.exports = ProjectEntityMongoUpdateHandler = self = self._putElement project, folder_id, doc, "doc", callback addFile: wrapWithLock (project_id, folder_id, fileRef, callback = (error, result, project) ->)-> - ProjectGetter.getProjectWithoutLock project_id, {rootFolder:true, name:true}, (err, project) -> + ProjectGetter.getProjectWithoutLock project_id, {rootFolder:true, name:true, overleaf:true}, (err, project) -> if err? logger.err project_id:project_id, err:err, "error getting project for add file" return callback(err) @@ -49,7 +49,7 @@ module.exports = ProjectEntityMongoUpdateHandler = self = self._putElement project, folder_id, fileRef, "file", callback replaceFileWithNew: wrapWithLock (project_id, file_id, newFileRef, callback) -> - ProjectGetter.getProjectWithoutLock project_id, {rootFolder: true, name:true}, (err, project) -> + ProjectGetter.getProjectWithoutLock project_id, {rootFolder:true, name:true, overleaf:true}, (err, project) -> return callback(err) if err? ProjectLocator.findElement {project:project, element_id: file_id, type: 'file'}, (err, fileRef, path)=> return callback(err) if err? @@ -110,7 +110,7 @@ module.exports = ProjectEntityMongoUpdateHandler = self = callback null, folders, lastFolder moveEntity: wrapWithLock (project_id, entity_id, destFolderId, entityType, callback = (error) ->) -> - ProjectGetter.getProjectWithoutLock project_id, {rootFolder:true, name:true}, (err, project) -> + ProjectGetter.getProjectWithoutLock project_id, {rootFolder:true, name:true, overleaf:true}, (err, project) -> return callback(err) if err? ProjectLocator.findElement {project, element_id: entity_id, type: entityType}, (err, entity, entityPath)-> return callback(err) if err? @@ -127,10 +127,10 @@ module.exports = ProjectEntityMongoUpdateHandler = self = startPath = entityPath.fileSystem endPath = result.path.fileSystem changes = {oldDocs, newDocs, oldFiles, newFiles} - callback null, project.name, startPath, endPath, entity.rev, changes, callback + callback null, project, startPath, endPath, entity.rev, changes, callback deleteEntity: wrapWithLock (project_id, entity_id, entityType, callback) -> - ProjectGetter.getProjectWithoutLock project_id, {name:true, rootFolder:true}, (error, project) -> + ProjectGetter.getProjectWithoutLock project_id, {name:true, rootFolder:true, overleaf:true}, (error, project) -> return callback(error) if error? ProjectLocator.findElement {project: project, element_id: entity_id, type: entityType}, (error, entity, path) -> return callback(error) if error? @@ -139,7 +139,7 @@ module.exports = ProjectEntityMongoUpdateHandler = self = callback null, entity, path, project renameEntity: wrapWithLock (project_id, entity_id, entityType, newName, callback) -> - ProjectGetter.getProjectWithoutLock project_id, {rootFolder:true, name:true}, (error, project)=> + ProjectGetter.getProjectWithoutLock project_id, {rootFolder:true, name:true, overleaf:true}, (error, project)=> return callback(error) if error? ProjectEntityHandler.getAllEntitiesFromProject project, (error, oldDocs, oldFiles) => return callback(error) if error? @@ -161,10 +161,10 @@ module.exports = ProjectEntityMongoUpdateHandler = self = return callback(error) if error? startPath = entPath.fileSystem changes = {oldDocs, newDocs, oldFiles, newFiles} - callback null, project.name, startPath, endPath, entity.rev, changes, callback + callback null, project, startPath, endPath, entity.rev, changes, callback addFolder: wrapWithLock (project_id, parentFolder_id, folderName, callback) -> - ProjectGetter.getProjectWithoutLock project_id, {rootFolder:true, name:true}, (err, project) -> + ProjectGetter.getProjectWithoutLock project_id, {rootFolder:true, name:true, overleaf:true}, (err, project) -> if err? logger.err project_id:project_id, err:err, "error getting project for add folder" return callback(err) @@ -325,4 +325,4 @@ module.exports = ProjectEntityMongoUpdateHandler = self = deletedAt: new Date() } } - }, {}, callback \ No newline at end of file + }, {}, callback diff --git a/services/web/test/unit/coffee/Project/ProjectEntityMongoUpdateHandlerTests.coffee b/services/web/test/unit/coffee/Project/ProjectEntityMongoUpdateHandlerTests.coffee index a79deb15a9..41505a2176 100644 --- a/services/web/test/unit/coffee/Project/ProjectEntityMongoUpdateHandlerTests.coffee +++ b/services/web/test/unit/coffee/Project/ProjectEntityMongoUpdateHandlerTests.coffee @@ -57,7 +57,7 @@ describe 'ProjectEntityMongoUpdateHandler', -> it 'gets the project', -> @ProjectGetter.getProjectWithoutLock - .calledWith(project_id, {rootFolder:true, name: true}) + .calledWith(project_id, {rootFolder:true, name:true, overleaf:true}) .should.equal true it 'checks the folder exists', -> @@ -80,7 +80,7 @@ describe 'ProjectEntityMongoUpdateHandler', -> it 'gets the project', -> @ProjectGetter.getProjectWithoutLock - .calledWith(project_id, {rootFolder:true, name: true}) + .calledWith(project_id, {rootFolder:true, name:true, overleaf:true}) .should.equal true it 'checks the folder exists', -> @@ -106,7 +106,7 @@ describe 'ProjectEntityMongoUpdateHandler', -> it 'gets the project', -> @ProjectGetter.getProjectWithoutLock - .calledWith(project_id, {rootFolder:true, name: true}) + .calledWith(project_id, {rootFolder:true, name:true, overleaf:true}) .should.equal true it 'finds the existing element', -> @@ -241,7 +241,7 @@ describe 'ProjectEntityMongoUpdateHandler', -> it 'should get the project', -> @ProjectGetter.getProjectWithoutLock - .calledWith(project_id, {rootFolder:true, name:true}) + .calledWith(project_id, {rootFolder:true, name:true, overleaf:true}) .should.equal true it 'should find the doc to move', -> @@ -280,7 +280,7 @@ describe 'ProjectEntityMongoUpdateHandler', -> it "should get the project", -> @ProjectGetter.getProjectWithoutLock - .calledWith(project_id, {name:true, rootFolder:true}) + .calledWith(project_id, {rootFolder:true, name:true, overleaf:true}) .should.equal true it "should find the element", -> @@ -325,7 +325,7 @@ describe 'ProjectEntityMongoUpdateHandler', -> it 'should get the project', -> @ProjectGetter.getProjectWithoutLock - .calledWith(project_id, {rootFolder:true, name:true}) + .calledWith(project_id, {rootFolder:true, name:true, overleaf:true}) .should.equal true it 'should find the doc', -> @@ -363,7 +363,7 @@ describe 'ProjectEntityMongoUpdateHandler', -> it 'gets the project', -> @ProjectGetter.getProjectWithoutLock - .calledWith(project_id, {rootFolder:true, name: true}) + .calledWith(project_id, {rootFolder:true, name:true, overleaf:true}) .should.equal true it 'checks the parent folder exists', -> @@ -638,4 +638,4 @@ describe 'ProjectEntityMongoUpdateHandler', -> .should.equal true it "should call the callback", -> - @callback.called.should.equal true \ No newline at end of file + @callback.called.should.equal true