diff --git a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee index e340187dbb..9ecff2185e 100644 --- a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee +++ b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee @@ -174,6 +174,23 @@ module.exports = DocumentUpdaterHandler = oldEntitiesHash = _.indexBy oldEntities, (entity) -> entity[entityType]._id.toString() newEntitiesHash = _.indexBy newEntities, (entity) -> entity[entityType]._id.toString() + # Send deletes before adds (and renames) to keep a 1:1 mapping between + # paths and ids + # + # When a file is replaced, we first delete the old file and then add the + # new file. If the 'add' operation is sent to project history before the + # 'delete' then we would have two files with the same path at that point + # in time. + for id, oldEntity of oldEntitiesHash + newEntity = newEntitiesHash[id] + + if !newEntity? + # entity deleted + updates.push + id: id + pathname: oldEntity.path + newPathname: '' + for id, newEntity of newEntitiesHash oldEntity = oldEntitiesHash[id] @@ -191,16 +208,6 @@ module.exports = DocumentUpdaterHandler = pathname: oldEntity.path newPathname: newEntity.path - for id, oldEntity of oldEntitiesHash - newEntity = newEntitiesHash[id] - - if !newEntity? - # entity deleted - updates.push - id: id - pathname: oldEntity.path - newPathname: '' - updates PENDINGUPDATESKEY = "PendingUpdates" diff --git a/services/web/app/coffee/Features/Editor/EditorController.coffee b/services/web/app/coffee/Features/Editor/EditorController.coffee index 7294e88bea..846042312d 100644 --- a/services/web/app/coffee/Features/Editor/EditorController.coffee +++ b/services/web/app/coffee/Features/Editor/EditorController.coffee @@ -41,11 +41,13 @@ module.exports = EditorController = callback err, doc upsertFile: (project_id, folder_id, fileName, fsPath, linkedFileData, source, user_id, callback = (err, file) ->) -> - ProjectEntityUpdateHandler.upsertFile project_id, folder_id, fileName, fsPath, linkedFileData, user_id, (err, file, didAddFile) -> + ProjectEntityUpdateHandler.upsertFile project_id, folder_id, fileName, fsPath, linkedFileData, user_id, (err, newFile, didAddFile, existingFile) -> return callback(err) if err? - if didAddFile - EditorRealTimeController.emitToRoom project_id, 'reciveNewFile', folder_id, file, source, linkedFileData - callback null, file + if not didAddFile # replacement, so remove the existing file from the client + EditorRealTimeController.emitToRoom project_id, 'removeEntity', existingFile._id, source + # now add the new file on the client + EditorRealTimeController.emitToRoom project_id, 'reciveNewFile', folder_id, newFile, source, linkedFileData + callback null, newFile upsertDocWithPath: (project_id, elementPath, docLines, source, user_id, callback) -> ProjectEntityUpdateHandler.upsertDocWithPath project_id, elementPath, docLines, source, user_id, (err, doc, didAddNewDoc, newFolders, lastFolder) -> @@ -57,12 +59,14 @@ module.exports = EditorController = callback() upsertFileWithPath: (project_id, elementPath, fsPath, linkedFileData, source, user_id, callback) -> - ProjectEntityUpdateHandler.upsertFileWithPath project_id, elementPath, fsPath, linkedFileData, user_id, (err, file, didAddFile, newFolders, lastFolder) -> + ProjectEntityUpdateHandler.upsertFileWithPath project_id, elementPath, fsPath, linkedFileData, user_id, (err, newFile, didAddFile, existingFile, newFolders, lastFolder) -> return callback(err) if err? EditorController._notifyProjectUsersOfNewFolders project_id, newFolders, (err) -> return callback(err) if err? - if didAddFile - EditorRealTimeController.emitToRoom project_id, 'reciveNewFile', lastFolder._id, file, source, linkedFileData + if not didAddFile # replacement, so remove the existing file from the client + EditorRealTimeController.emitToRoom project_id, 'removeEntity', existingFile._id, source + # now add the new file on the client + EditorRealTimeController.emitToRoom project_id, 'reciveNewFile', lastFolder._id, newFile, source, linkedFileData callback() addFolder : (project_id, folder_id, folderName, source, callback = (error, folder)->)-> diff --git a/services/web/app/coffee/Features/Project/ProjectEntityMongoUpdateHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityMongoUpdateHandler.coffee index 63138d1ca0..57f65a67dc 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityMongoUpdateHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityMongoUpdateHandler.coffee @@ -48,28 +48,28 @@ module.exports = ProjectEntityMongoUpdateHandler = self = self._confirmFolder project, folder_id, (folder_id)-> self._putElement project, folder_id, fileRef, "file", callback - replaceFile: wrapWithLock (project_id, file_id, linkedFileData, callback) -> + replaceFileWithNew: wrapWithLock (project_id, file_id, newFileRef, callback) -> ProjectGetter.getProjectWithoutLock project_id, {rootFolder: true, name: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? - conditions = _id:project._id - inc = {} - inc["#{path.mongo}.rev"] = 1 - # currently we do not need to increment the project version number for changes that are replacements - # but when we make switch to having immutable files the replace operation will add a new file, and - # this will require a version increase. We will start incrementing the project version now as it does - # no harm and will help to test it. - inc['version'] = 1 - set = {} - set["#{path.mongo}.created"] = new Date() - set["#{path.mongo}.linkedFileData"] = linkedFileData - update = - "$inc": inc - "$set": set - Project.update conditions, update, {}, (err) -> + ProjectEntityMongoUpdateHandler._insertDeletedFileReference project_id, fileRef, (err) -> return callback(err) if err? - callback null, fileRef, project, path + conditions = _id:project._id + inc = {} + # increment the project structure version as we are adding a new file here + inc['version'] = 1 + set = {} + set["#{path.mongo}._id"] = newFileRef._id + set["#{path.mongo}.created"] = new Date() + set["#{path.mongo}.linkedFileData"] = newFileRef.linkedFileData + set["#{path.mongo}.rev"] = 1 + update = + "$inc": inc + "$set": set + Project.update conditions, update, {}, (err) -> + return callback(err) if err? + callback null, fileRef, project, path mkdirp: wrapWithLock (project_id, path, callback) -> folders = path.split('/') @@ -300,3 +300,29 @@ module.exports = ProjectEntityMongoUpdateHandler = self = if isNestedFolder return callback(new Errors.InvalidNameError("destination folder is a child folder of me")) callback() + + _insertDeletedDocReference: (project_id, doc, callback = (error) ->) -> + Project.update { + _id: project_id + }, { + $push: { + deletedDocs: { + _id: doc._id + name: doc.name + } + } + }, {}, callback + + _insertDeletedFileReference: (project_id, fileRef, callback = (error) ->) -> + Project.update { + _id: project_id + }, { + $push: { + deletedFiles: { + _id: fileRef._id + name: fileRef.name + linkedFileData: fileRef.linkedFileData + deletedAt: new Date() + } + } + }, {}, callback \ No newline at end of file diff --git a/services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee index ff09eda64d..e3aea07456 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee @@ -24,12 +24,25 @@ wrapWithLock = (methodWithoutLock) -> # This lock is used to make sure that the project structure updates are made # sequentially. In particular the updates must be made in mongo and sent to # the doc-updater in the same order. - methodWithLock = (project_id, args..., callback) -> - LockManager.runWithLock LOCK_NAMESPACE, project_id, - (cb) -> methodWithoutLock project_id, args..., cb - callback - methodWithLock.withoutLock = methodWithoutLock - methodWithLock + if typeof methodWithoutLock is 'function' + methodWithLock = (project_id, args..., callback) -> + LockManager.runWithLock LOCK_NAMESPACE, project_id, + (cb) -> methodWithoutLock project_id, args..., cb + callback + methodWithLock.withoutLock = methodWithoutLock + methodWithLock + else + # handle case with separate setup and locked stages + wrapWithSetup = methodWithoutLock.beforeLock # a function to set things up before the lock + mainTask = methodWithoutLock.withLock # function to execute inside the lock + methodWithLock = wrapWithSetup (project_id, args..., callback) -> + LockManager.runWithLock(LOCK_NAMESPACE, project_id, (cb) -> + mainTask(project_id, args..., cb) + callback) + methodWithLock.withoutLock = wrapWithSetup mainTask + methodWithLock.beforeLock = methodWithoutLock.beforeLock + methodWithLock.mainTask = methodWithoutLock.withLock + methodWithLock module.exports = ProjectEntityUpdateHandler = self = # this doesn't need any locking because it's only called by ProjectDuplicator @@ -129,31 +142,72 @@ module.exports = ProjectEntityUpdateHandler = self = return callback(error) if error? callback null, doc, folder_id - addFile: wrapWithLock (project_id, folder_id, fileName, fsPath, linkedFileData, userId, callback = (error, fileRef, folder_id) ->)-> - self.addFileWithoutUpdatingHistory.withoutLock project_id, folder_id, fileName, fsPath, linkedFileData, userId, (error, fileRef, folder_id, path, fileStoreUrl) -> - return callback(error) if error? - newFiles = [ - file: fileRef - path: path - url: fileStoreUrl - ] - DocumentUpdaterHandler.updateProjectStructure project_id, userId, {newFiles}, (error) -> - return callback(error) if error? - callback null, fileRef, folder_id + _uploadFile: (project_id, folder_id, fileName, fsPath, linkedFileData, userId, callback = (error, fileRef, fileStoreUrl) ->)-> + if not SafePath.isCleanFilename fileName + return callback new Errors.InvalidNameError("invalid element name") + fileRef = new File( + name: fileName + linkedFileData: linkedFileData + ) + FileStoreHandler.uploadFileFromDisk project_id, fileRef._id, fsPath, (err, fileStoreUrl)-> + if err? + logger.err err:err, project_id: project_id, folder_id: folder_id, file_name: fileName, fileRef:fileRef, "error uploading image to s3" + return callback(err) + callback(null, fileRef, fileStoreUrl) - replaceFile: wrapWithLock (project_id, file_id, fsPath, linkedFileData, userId, callback)-> - FileStoreHandler.uploadFileFromDisk project_id, file_id, fsPath, (err, fileStoreUrl)-> - return callback(err) if err? - ProjectEntityMongoUpdateHandler.replaceFile project_id, file_id, linkedFileData, (err, fileRef, project, path) -> + _addFileAndSendToTpds: (project_id, folder_id, fileName, fileRef, callback = (error) ->)-> + ProjectEntityMongoUpdateHandler.addFile project_id, folder_id, fileRef, (err, result, project) -> + if err? + logger.err err:err, project_id: project_id, folder_id: folder_id, file_name: fileName, fileRef:fileRef, "error adding file with project" + return callback(err) + TpdsUpdateSender.addFile {project_id:project_id, file_id:fileRef._id, path:result?.path?.fileSystem, project_name:project.name, rev:fileRef.rev}, (err) -> + return callback(err) if err? + callback(null, result, project) + + addFile: wrapWithLock + beforeLock: (next) -> + (project_id, folder_id, fileName, fsPath, linkedFileData, userId, callback) -> + ProjectEntityUpdateHandler._uploadFile project_id, folder_id, fileName, fsPath, linkedFileData, userId, (error, fileRef, fileStoreUrl) -> + return callback(error) if error? + next(project_id, folder_id, fileName, fsPath, linkedFileData, userId, fileRef, fileStoreUrl, callback) + 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? newFiles = [ file: fileRef + path: result?.path?.fileSystem + url: fileStoreUrl + ] + DocumentUpdaterHandler.updateProjectStructure project_id, userId, {newFiles}, (error) -> + return callback(error) if error? + callback(null, fileRef, folder_id) + + replaceFile: wrapWithLock + beforeLock: (next) -> + (project_id, file_id, fsPath, linkedFileData, userId, callback)-> + # create a new file + fileRef = new File( + name: "dummy-upload-filename" + linkedFileData: linkedFileData + ) + FileStoreHandler.uploadFileFromDisk project_id, fileRef._id, fsPath, (err, fileStoreUrl)-> + return callback(err) if err? + next project_id, file_id, fsPath, linkedFileData, userId, fileRef, fileStoreUrl, callback + withLock: (project_id, file_id, fsPath, linkedFileData, userId, newFileRef, fileStoreUrl, callback)-> + ProjectEntityMongoUpdateHandler.replaceFileWithNew project_id, file_id, newFileRef, (err, oldFileRef, project, path) -> + return callback(err) if err? + oldFiles = [ + file: oldFileRef + path: path.fileSystem + ] + newFiles = [ + file: newFileRef path: path.fileSystem url: fileStoreUrl ] - TpdsUpdateSender.addFile {project_id:project._id, file_id:fileRef._id, path:path.fileSystem, rev:fileRef.rev+1, project_name:project.name}, (err) -> + 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, {newFiles}, callback + DocumentUpdaterHandler.updateProjectStructure project_id, 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 @@ -180,29 +234,19 @@ module.exports = ProjectEntityUpdateHandler = self = return callback(err) if err? callback(null, doc, folder_id, result?.path?.fileSystem) - addFileWithoutUpdatingHistory: wrapWithLock (project_id, folder_id, fileName, fsPath, linkedFileData, userId, callback = (error, fileRef, folder_id, path, fileStoreUrl) ->)-> + addFileWithoutUpdatingHistory: wrapWithLock # This method should never be called directly, except when importing a project # from Overleaf. It skips sending updates to the project history, which will break # the history unless you are making sure it is updated in some other way. - - if not SafePath.isCleanFilename fileName - return callback new Errors.InvalidNameError("invalid element name") - - fileRef = new File( - name: fileName - linkedFileData: linkedFileData - ) - FileStoreHandler.uploadFileFromDisk project_id, fileRef._id, fsPath, (err, fileStoreUrl)-> - if err? - logger.err err:err, project_id: project_id, folder_id: folder_id, file_name: fileName, fileRef:fileRef, "error uploading image to s3" - return callback(err) - ProjectEntityMongoUpdateHandler.addFile project_id, folder_id, fileRef, (err, result, project) -> - if err? - logger.err err:err, project_id: project_id, folder_id: folder_id, file_name: fileName, fileRef:fileRef, "error adding file with project" - return callback(err) - TpdsUpdateSender.addFile {project_id:project_id, file_id:fileRef._id, path:result?.path?.fileSystem, project_name:project.name, rev:fileRef.rev}, (err) -> - return callback(err) if err? - callback(null, fileRef, folder_id, result?.path?.fileSystem, fileStoreUrl) + beforeLock: (next) -> + (project_id, folder_id, fileName, fsPath, linkedFileData, userId, callback) -> + ProjectEntityUpdateHandler._uploadFile project_id, folder_id, fileName, fsPath, linkedFileData, userId, (error, fileRef, fileStoreUrl) -> + return callback(error) if error? + next(project_id, folder_id, fileName, fsPath, linkedFileData, userId, fileRef, fileStoreUrl, callback) + withLock: (project_id, folder_id, fileName, fsPath, linkedFileData, userId, fileRef, fileStoreUrl, callback = (error, fileRef, folder_id, path, fileStoreUrl) ->)-> + ProjectEntityUpdateHandler._addFileAndSendToTpds project_id, folder_id, fileName, fileRef, (err, result, project) -> + return callback(err) if err? + callback(null, fileRef, folder_id, result?.path?.fileSystem, fileStoreUrl) upsertDoc: wrapWithLock (project_id, folder_id, docName, docLines, source, userId, callback = (err, doc, folder_id, isNewDoc)->)-> ProjectLocator.findElement project_id: project_id, element_id: folder_id, type: "folder", (error, folder) -> @@ -224,23 +268,36 @@ module.exports = ProjectEntityUpdateHandler = self = return callback(err) if err? callback null, doc, !existingDoc? - upsertFile: wrapWithLock (project_id, folder_id, fileName, fsPath, linkedFileData, userId, callback = (err, file, isNewFile)->)-> - ProjectLocator.findElement project_id: project_id, element_id: folder_id, type: "folder", (error, folder) -> - return callback(error) if error? - return callback(new Error("Couldn't find folder")) if !folder? - existingFile = null - for fileRef in folder.fileRefs - if fileRef.name == fileName - existingFile = fileRef - break - if existingFile? - self.replaceFile.withoutLock project_id, existingFile._id, fsPath, linkedFileData, userId, (err) -> + upsertFile: wrapWithLock + beforeLock: (next) -> + (project_id, folder_id, fileName, fsPath, linkedFileData, userId, callback)-> + # create a new file + fileRef = new File( + name: fileName + linkedFileData: linkedFileData + ) + FileStoreHandler.uploadFileFromDisk project_id, fileRef._id, fsPath, (err, fileStoreUrl)-> return callback(err) if err? - callback null, existingFile, !existingFile? - else - self.addFile.withoutLock project_id, folder_id, fileName, fsPath, linkedFileData, userId, (err, file) -> - return callback(err) if err? - callback null, file, !existingFile? + next(project_id, folder_id, fileName, fsPath, linkedFileData, userId, fileRef, fileStoreUrl, callback) + withLock: (project_id, folder_id, fileName, fsPath, linkedFileData, userId, newFileRef, fileStoreUrl, callback = (err, file, isNewFile, existingFile)->)-> + ProjectLocator.findElement project_id: project_id, element_id: folder_id, type: "folder", (error, folder) -> + return callback(error) if error? + return callback(new Error("Couldn't find folder")) if !folder? + existingFile = null + for fileRef in folder.fileRefs + if fileRef.name == fileName + existingFile = fileRef + break + if existingFile? + # this calls directly into the replaceFile main task (without the beforeLock part) + self.replaceFile.mainTask project_id, existingFile._id, fsPath, linkedFileData, userId, newFileRef, fileStoreUrl, (err) -> + return callback(err) if err? + callback null, newFileRef, !existingFile?, existingFile + else + # this calls directly into the addFile main task (without the beforeLock part) + self.addFile.mainTask project_id, folder_id, fileName, fsPath, linkedFileData, userId, newFileRef, fileStoreUrl, (err) -> + return callback(err) if err? + callback null, newFileRef, !existingFile?, existingFile upsertDocWithPath: wrapWithLock (project_id, elementPath, docLines, source, userId, callback) -> docName = path.basename(elementPath) @@ -251,14 +308,26 @@ module.exports = ProjectEntityUpdateHandler = self = return callback(err) if err? callback null, doc, isNewDoc, newFolders, folder - upsertFileWithPath: wrapWithLock (project_id, elementPath, fsPath, linkedFileData, userId, callback) -> - fileName = path.basename(elementPath) - folderPath = path.dirname(elementPath) - self.mkdirp.withoutLock project_id, folderPath, (err, newFolders, folder) -> - return callback(err) if err? - self.upsertFile.withoutLock project_id, folder._id, fileName, fsPath, linkedFileData, userId, (err, file, isNewFile) -> + upsertFileWithPath: wrapWithLock + beforeLock: (next) -> + (project_id, elementPath, fsPath, linkedFileData, userId, callback)-> + fileName = path.basename(elementPath) + folderPath = path.dirname(elementPath) + # create a new file + fileRef = new File( + name: fileName + linkedFileData: linkedFileData + ) + FileStoreHandler.uploadFileFromDisk project_id, fileRef._id, fsPath, (err, fileStoreUrl)-> + return callback(err) if err? + next project_id, folderPath, fileName, fsPath, linkedFileData, userId, fileRef, fileStoreUrl, callback + withLock: (project_id, folderPath, fileName, fsPath, linkedFileData, userId, fileRef, fileStoreUrl, callback) -> + self.mkdirp.withoutLock project_id, folderPath, (err, newFolders, folder) -> return callback(err) if err? - callback null, file, isNewFile, newFolders, folder + # this calls directly into the upsertFile main task (without the beforeLock part) + self.upsertFile.mainTask project_id, folder._id, fileName, fsPath, linkedFileData, userId, fileRef, fileStoreUrl, (err, newFile, isNewFile, existingFile) -> + return callback(err) if err? + callback null, newFile, isNewFile, existingFile, newFolders, folder deleteEntity: wrapWithLock (project_id, entity_id, entityType, userId, callback = (error) ->)-> logger.log entity_id:entity_id, entityType:entityType, project_id:project_id, "deleting project entity" @@ -335,7 +404,7 @@ 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, docs, files, callback _cleanUpEntity: (project, entity, entityType, path, userId, callback = (error) ->) -> if(entityType.indexOf("file") != -1) self._cleanUpFile project, entity, path, userId, callback @@ -357,7 +426,7 @@ module.exports = ProjectEntityUpdateHandler = self = unsetRootDocIfRequired (error) -> return callback(error) if error? - self._insertDeletedDocReference project._id, doc, (error) -> + ProjectEntityMongoUpdateHandler._insertDeletedDocReference project._id, doc, (error) -> return callback(error) if error? DocumentUpdaterHandler.deleteDoc project_id, doc_id, (error) -> return callback(error) if error? @@ -367,11 +436,12 @@ module.exports = ProjectEntityUpdateHandler = self = DocumentUpdaterHandler.updateProjectStructure project_id, userId, changes, callback _cleanUpFile: (project, file, path, userId, callback = (error) ->) -> - project_id = project._id.toString() - file_id = file._id.toString() - FileStoreHandler.deleteFile project_id, file_id, (error) -> + ProjectEntityMongoUpdateHandler._insertDeletedFileReference project._id, file, (error) -> return callback(error) if error? + project_id = project._id.toString() 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 _cleanUpFolder: (project, folder, folderPath, userId, callback = (error) ->) -> @@ -392,15 +462,3 @@ module.exports = ProjectEntityUpdateHandler = self = jobs.push (callback) -> self._cleanUpFolder project, childFolder, folderPath, userId, callback async.series jobs, callback - - _insertDeletedDocReference: (project_id, doc, callback = (error) ->) -> - Project.update { - _id: project_id - }, { - $push: { - deletedDocs: { - _id: doc._id - name: doc.name - } - } - }, {}, callback diff --git a/services/web/app/coffee/models/Project.coffee b/services/web/app/coffee/models/Project.coffee index fb0bc0fd6b..00c2f1be52 100644 --- a/services/web/app/coffee/models/Project.coffee +++ b/services/web/app/coffee/models/Project.coffee @@ -12,6 +12,10 @@ ObjectId = Schema.ObjectId DeletedDocSchema = new Schema name: String +DeletedFileSchema = new Schema + name: String + deletedAt: {type: Date} + ProjectSchema = new Schema name : {type:String, default:'new project'} lastUpdated : {type:Date, default: () -> new Date()} @@ -30,6 +34,7 @@ ProjectSchema = new Schema description : {type:String, default:''} archived : { type: Boolean } deletedDocs : [DeletedDocSchema] + deletedFiles : [DeletedFileSchema] imageName : { type: String } track_changes : { type: Object } tokens : diff --git a/services/web/test/acceptance/coffee/ProjectDuplicateNameTests.coffee b/services/web/test/acceptance/coffee/ProjectDuplicateNameTests.coffee index 5da94856fc..b4742eefec 100644 --- a/services/web/test/acceptance/coffee/ProjectDuplicateNameTests.coffee +++ b/services/web/test/acceptance/coffee/ProjectDuplicateNameTests.coffee @@ -186,10 +186,13 @@ describe "ProjectDuplicateNames", -> contentType: 'image/jpeg' , (err, res, body) => @body = body + # update the image id because we have replaced the file + @imageFile._id = @body.entity_id done() it "should succeed (overwriting the file)", -> expect(@body.success).to.equal true + # at this point the @imageFile._id has changed describe "for an existing folder", -> describe "trying to add a doc with the same name", -> diff --git a/services/web/test/acceptance/coffee/ProjectStructureTests.coffee b/services/web/test/acceptance/coffee/ProjectStructureTests.coffee index 51bd4a26c7..30fbccfb8d 100644 --- a/services/web/test/acceptance/coffee/ProjectStructureTests.coffee +++ b/services/web/test/acceptance/coffee/ProjectStructureTests.coffee @@ -56,7 +56,6 @@ describe "ProjectStructureChanges", -> describe "duplicating a project", -> before (done) -> MockDocUpdaterApi.clearProjectStructureUpdates() - console.log(example_project_id) @owner.request.post { uri: "/Project/#{example_project_id}/clone", json: @@ -219,11 +218,17 @@ describe "ProjectStructureChanges", -> if res.statusCode < 200 || res.statusCode >= 300 throw new Error("failed to upload file #{res.statusCode}") + example_file_id = JSON.parse(body).entity_id + {fileUpdates:updates, version} = MockDocUpdaterApi.getProjectStructureUpdates(example_project_id) - expect(updates.length).to.equal(1) + expect(updates.length).to.equal(2) update = updates[0] expect(update.userId).to.equal(@owner._id) expect(update.pathname).to.equal("/1pixel.png") + #expect(update.url).to.be.a('string'); + update = updates[1] + expect(update.userId).to.equal(@owner._id) + expect(update.pathname).to.equal("/1pixel.png") expect(update.url).to.be.a('string'); expect(version).to.equal(@project_0.version + 1) @@ -591,10 +596,14 @@ describe "ProjectStructureChanges", -> throw new Error("failed to upload file #{res.statusCode}") {fileUpdates:updates, version} = MockDocUpdaterApi.getProjectStructureUpdates(@tpds_project_id) - expect(updates.length).to.equal(1) + expect(updates.length).to.equal(2) update = updates[0] expect(update.userId).to.equal(@owner._id) expect(update.pathname).to.equal("/1pixel.png") + #expect(update.url).to.be.a('string'); + update = updates[1] + expect(update.userId).to.equal(@owner._id) + expect(update.pathname).to.equal("/1pixel.png") expect(update.url).to.be.a('string'); expect(version).to.equal(@project_0.version + 1) diff --git a/services/web/test/unit/coffee/Editor/EditorControllerTests.coffee b/services/web/test/unit/coffee/Editor/EditorControllerTests.coffee index 9e30ef3fd1..703a03d14f 100644 --- a/services/web/test/unit/coffee/Editor/EditorControllerTests.coffee +++ b/services/web/test/unit/coffee/Editor/EditorControllerTests.coffee @@ -20,6 +20,8 @@ describe "EditorController", -> @fsPath = "/folder/file.png" @linkedFileData = {provider: 'url'} + @newFile = _id: "new-file-id" + @folder_id = "123ksajdn" @folder = _id: @folder_id @folderName = "folder" @@ -107,7 +109,7 @@ describe "EditorController", -> describe 'upsertFile', -> beforeEach -> - @ProjectEntityUpdateHandler.upsertFile = sinon.stub().yields(null, @file, false) + @ProjectEntityUpdateHandler.upsertFile = sinon.stub().yields(null, @newFile, false, @file) @EditorController.upsertFile @project_id, @folder_id, @fileName, @fsPath, @linkedFileData, @source, @user_id, @callback it 'upserts the file using the project entity handler', -> @@ -116,7 +118,7 @@ describe "EditorController", -> .should.equal true it 'returns the file', -> - @callback.calledWith(null, @file).should.equal true + @callback.calledWith(null, @newFile).should.equal true describe 'file does not exist', -> beforeEach -> @@ -171,7 +173,7 @@ describe "EditorController", -> beforeEach -> @filePath = '/folder/file' - @ProjectEntityUpdateHandler.upsertFileWithPath = sinon.stub().yields(null, @file, false, [], @folder) + @ProjectEntityUpdateHandler.upsertFileWithPath = sinon.stub().yields(null, @newFile, false, @file, [], @folder) @EditorController.upsertFileWithPath @project_id, @filePath, @fsPath, @linkedFileData, @source, @user_id, @callback it 'upserts the file using the project entity handler', -> @@ -181,7 +183,7 @@ describe "EditorController", -> describe 'file does not exist', -> beforeEach -> - @ProjectEntityUpdateHandler.upsertFileWithPath = sinon.stub().yields(null, @file, true, [], @folder) + @ProjectEntityUpdateHandler.upsertFileWithPath = sinon.stub().yields(null, @file, true, undefined, [], @folder) @EditorController.upsertFileWithPath @project_id, @filePath, @fsPath, @linkedFileData, @source, @user_id, @callback it 'should send the update for the file out to users in the project', -> @@ -195,7 +197,7 @@ describe "EditorController", -> @folderA = { _id: 2, parentFolder_id: 1} @folderB = { _id: 3, parentFolder_id: 2} ] - @ProjectEntityUpdateHandler.upsertFileWithPath = sinon.stub().yields(null, @file, true, folders, @folderB) + @ProjectEntityUpdateHandler.upsertFileWithPath = sinon.stub().yields(null, @file, true, undefined, folders, @folderB) @EditorController.upsertFileWithPath @project_id, @filePath, @fsPath, @linkedFileData, @source, @user_id, @callback it 'should send the update for each folder to users in the project', -> diff --git a/services/web/test/unit/coffee/Project/ProjectEntityMongoUpdateHandlerTests.coffee b/services/web/test/unit/coffee/Project/ProjectEntityMongoUpdateHandlerTests.coffee index e05d5bc38a..57911bd7d8 100644 --- a/services/web/test/unit/coffee/Project/ProjectEntityMongoUpdateHandlerTests.coffee +++ b/services/web/test/unit/coffee/Project/ProjectEntityMongoUpdateHandlerTests.coffee @@ -93,33 +93,50 @@ describe 'ProjectEntityMongoUpdateHandler', -> .calledWith(@project, folder_id, @file, 'file', @callback) .should.equal true - describe 'replaceFile', -> + describe 'replaceFileWithNew', -> beforeEach -> @file = _id: file_id @path = mongo: 'file.png' - @linkedFileData = {provider: 'url'} + @newFile = _id: 'new-file-id' + @newFile.linkedFileData = @linkedFileData = {provider: 'url'} @ProjectLocator.findElement = sinon.stub().yields(null, @file, @path) @ProjectModel.update = sinon.stub().yields() - @subject.replaceFile project_id, file_id, @linkedFileData, @callback + @subject.replaceFileWithNew project_id, file_id, @newFile, @callback it 'gets the project', -> @ProjectGetter.getProjectWithoutLock .calledWith(project_id, {rootFolder:true, name: true}) .should.equal true - it 'finds the element', -> + it 'finds the existing element', -> @ProjectLocator.findElement .calledWith({ @project, element_id: file_id, type: 'file' }) .should.equal true - it 'increments the rev and sets the created_at', -> + it 'inserts a deletedFile reference for the old file', -> + @ProjectModel.update + .calledWith({ _id: project_id }, + { + $push: { + deletedFiles: { + _id: file_id + name: @file.name + linkedFileData: @file.linkedFileData + deletedAt: new Date() + } + } + } + ) + .should.equal true + + it 'increments the project version and sets the rev and created_at', -> @ProjectModel.update .calledWith( { _id: project_id }, { - '$inc': { 'file.png.rev': 1, 'version': 1 } - '$set': { 'file.png.created': new Date(), 'file.png.linkedFileData': @linkedFileData } + '$inc': { 'version': 1 } + '$set': { 'file.png._id': @newFile._id, 'file.png.created': new Date(), 'file.png.linkedFileData': @linkedFileData, 'file.png.rev': 1 } } {} ) @@ -596,3 +613,29 @@ describe 'ProjectEntityMongoUpdateHandler', -> folder = name: 'folder_name' @subject._checkValidMove @project, 'folder', folder, fileSystem: '/foo', @destFolder._id, (err) => expect(err).to.deep.equal new Errors.InvalidNameError("destination folder is a child folder of me") + + describe "_insertDeletedDocReference", -> + beforeEach -> + @doc = + _id: ObjectId() + name: "test.tex" + @callback = sinon.stub() + @ProjectModel.update = sinon.stub().yields() + @subject._insertDeletedDocReference project_id, @doc, @callback + + it "should insert the doc into deletedDocs", -> + @ProjectModel.update + .calledWith({ + _id: project_id + }, { + $push: { + deletedDocs: { + _id: @doc._id + name: @doc.name + } + } + }) + .should.equal true + + it "should call the callback", -> + @callback.called.should.equal true \ No newline at end of file diff --git a/services/web/test/unit/coffee/Project/ProjectEntityUpdateHandlerTests.coffee b/services/web/test/unit/coffee/Project/ProjectEntityUpdateHandlerTests.coffee index 9d3651f55d..09d0d274fb 100644 --- a/services/web/test/unit/coffee/Project/ProjectEntityUpdateHandlerTests.coffee +++ b/services/web/test/unit/coffee/Project/ProjectEntityUpdateHandlerTests.coffee @@ -14,6 +14,7 @@ describe 'ProjectEntityUpdateHandler', -> file_id = "4eecaffcbffa66588e000009" folder_id = "4eecaffcbffa66588e000008" rootFolderId = "4eecaffcbffa66588e000007" + new_file_id = "4eecaffcbffa66588e000099" userId = 1234 beforeEach -> @@ -31,7 +32,11 @@ describe 'ProjectEntityUpdateHandler', -> @FileModel = class File constructor:(options)-> {@name} = options - @._id = file_id + # use a new id for replacement files + if @name is 'dummy-upload-filename' + @._id = new_file_id + else + @._id = file_id @rev = 0 @docName = "doc-name" @@ -295,14 +300,33 @@ describe 'ProjectEntityUpdateHandler', -> beforeEach -> @path = "/path/to/file" - @newFile = _id: file_id - @ProjectEntityUpdateHandler.addFileWithoutUpdatingHistory = - withoutLock: sinon.stub().yields(null, @newFile, folder_id, @path, @fileUrl) - @ProjectEntityUpdateHandler.addFile project_id, folder_id, @docName, @fileSystemPath, @linkedFileData, userId, @callback + @newFile = {_id: file_id, rev: 0, name: @fileName} + @TpdsUpdateSender.addFile = sinon.stub().yields() + @ProjectEntityMongoUpdateHandler.addFile = sinon.stub().yields(null, {path: fileSystem: @path}, @project) + @ProjectEntityUpdateHandler.addFile project_id, folder_id, @fileName, @fileSystemPath, @linkedFileData, userId, @callback - it "creates the doc without history", () -> - @ProjectEntityUpdateHandler.addFileWithoutUpdatingHistory.withoutLock - .calledWith(project_id, folder_id, @docName, @fileSystemPath, @linkedFileData, userId) + it "updates the file in the filestore", () -> + @FileStoreHandler.uploadFileFromDisk + .calledWith(project_id, file_id, @fileSystemPath) + .should.equal true + + it "updates the file in mongo", () -> + fileMatcher = sinon.match (file) => + file.name == @fileName + + @ProjectEntityMongoUpdateHandler.addFile + .calledWithMatch(project_id, folder_id, fileMatcher) + .should.equal true + + it "notifies the tpds", () -> + @TpdsUpdateSender.addFile + .calledWith({ + project_id: project_id + project_name: @project.name + file_id: file_id + rev: 0 + path: @path + }) .should.equal true it "sends the change in project structure to the doc updater", () -> @@ -317,21 +341,26 @@ describe 'ProjectEntityUpdateHandler', -> describe 'replaceFile', -> beforeEach -> - @newFile = _id: file_id, rev: 0 + # replacement file now creates a new file object + @newFileUrl = "new-file-url" + @FileStoreHandler.uploadFileFromDisk = sinon.stub().yields(null, @newFileUrl) + + @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.replaceFile = sinon.stub().yields(null, @newFile, @project, fileSystem: @path) - + @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 it 'uploads a new version of the file', -> @FileStoreHandler.uploadFileFromDisk - .calledWith(project_id, file_id, @fileSystemPath) + .calledWith(project_id, new_file_id, @fileSystemPath) .should.equal true it 'replaces the file in mongo', -> - @ProjectEntityMongoUpdateHandler.replaceFile - .calledWith(project_id, file_id, @linkedFileData) + @ProjectEntityMongoUpdateHandler.replaceFileWithNew + .calledWith(project_id, file_id, @newFile) .should.equal true it 'notifies the tpds', -> @@ -339,20 +368,24 @@ describe 'ProjectEntityUpdateHandler', -> .calledWith({ project_id: project_id project_name: @project.name - file_id: file_id + file_id: new_file_id rev: @newFile.rev + 1 path: @path }) .should.equal true it 'updates the project structure in the doc updater', -> + oldFiles = [ + file: @oldFile + path: @path + ] newFiles = [ file: @newFile path: @path - url: @fileUrl + url: @newFileUrl ] @DocumentUpdaterHandler.updateProjectStructure - .calledWith(project_id, userId, {newFiles}) + .calledWith(project_id, userId, {oldFiles, newFiles}) .should.equal true describe 'addDocWithoutUpdatingHistory', -> @@ -511,12 +544,12 @@ describe 'ProjectEntityUpdateHandler', -> @existingFile = _id: file_id, name: @fileName @folder = _id: folder_id, fileRefs: [@existingFile] @ProjectLocator.findElement = sinon.stub().yields(null, @folder) - @ProjectEntityUpdateHandler.replaceFile = withoutLock: sinon.stub().yields(null, @newFile) + @ProjectEntityUpdateHandler.replaceFile = mainTask: sinon.stub().yields(null, @newFile) @ProjectEntityUpdateHandler.upsertFile project_id, folder_id, @fileName, @fileSystemPath, @linkedFileData, userId, @callback it 'replaces the file', -> - @ProjectEntityUpdateHandler.replaceFile.withoutLock + @ProjectEntityUpdateHandler.replaceFile.mainTask .calledWith(project_id, file_id, @fileSystemPath, @linkedFileData, userId) .should.equal true @@ -528,7 +561,7 @@ describe 'ProjectEntityUpdateHandler', -> @folder = _id: folder_id, fileRefs: [] @newFile = _id: file_id @ProjectLocator.findElement = sinon.stub().yields(null, @folder) - @ProjectEntityUpdateHandler.addFile = withoutLock: sinon.stub().yields(null, @newFile) + @ProjectEntityUpdateHandler.addFile = mainTask: sinon.stub().yields(null, @newFile) @ProjectEntityUpdateHandler.upsertFile project_id, folder_id, @fileName, @fileSystemPath, @linkedFileData, userId, @callback @@ -538,7 +571,7 @@ describe 'ProjectEntityUpdateHandler', -> .should.equal true it 'adds the file', -> - @ProjectEntityUpdateHandler.addFile.withoutLock + @ProjectEntityUpdateHandler.addFile.mainTask .calledWith(project_id, folder_id, @fileName, @fileSystemPath, @linkedFileData, userId) .should.equal true @@ -584,7 +617,7 @@ describe 'ProjectEntityUpdateHandler', -> @ProjectEntityUpdateHandler.mkdirp = withoutLock: sinon.stub().yields(null, @newFolders, @folder) @ProjectEntityUpdateHandler.upsertFile = - withoutLock: sinon.stub().yields(null, @file, @isNewFile) + mainTask: sinon.stub().yields(null, @file, @isNewFile) @ProjectEntityUpdateHandler.upsertFileWithPath project_id, @path, @fileSystemPath, @linkedFileData, userId, @callback @@ -594,13 +627,13 @@ describe 'ProjectEntityUpdateHandler', -> .should.equal true it 'upserts the file', -> - @ProjectEntityUpdateHandler.upsertFile.withoutLock + @ProjectEntityUpdateHandler.upsertFile.mainTask .calledWith(project_id, @folder._id, 'file.png', @fileSystemPath, @linkedFileData, userId) .should.equal true it 'calls the callback', -> @callback - .calledWith(null, @file, @isNewFile, @newFolders, @folder) + .calledWith(null, @file, @isNewFile, undefined, @newFolders, @folder) .should.equal true describe 'deleteEntity', -> @@ -819,6 +852,7 @@ describe 'ProjectEntityUpdateHandler', -> @FileStoreHandler.deleteFile = sinon.stub().yields() @DocumentUpdaterHandler.deleteDoc = sinon.stub().yields() @ProjectEntityUpdateHandler.unsetRootDoc = sinon.stub().yields() + @ProjectEntityMongoUpdateHandler._insertDeletedFileReference = sinon.stub().yields() describe "a file", -> beforeEach (done) -> @@ -826,8 +860,13 @@ describe 'ProjectEntityUpdateHandler', -> @entity = _id: @entity_id @ProjectEntityUpdateHandler._cleanUpEntity @project, @entity, 'file', @path, userId, done - it "should delete the file from FileStoreHandler", -> - @FileStoreHandler.deleteFile.calledWith(project_id, @entity_id).should.equal true + it "should insert the file into the deletedFiles array", -> + @ProjectEntityMongoUpdateHandler._insertDeletedFileReference + .calledWith(@project._id, @entity) + .should.equal true + + it "should not delete the file from FileStoreHandler", -> + @FileStoreHandler.deleteFile.calledWith(project_id, @entity_id).should.equal false it "should not attempt to delete from the document updater", -> @DocumentUpdaterHandler.deleteDoc.called.should.equal false @@ -892,7 +931,7 @@ describe 'ProjectEntityUpdateHandler', -> name: "test.tex" @path = "/path/to/doc" @ProjectEntityUpdateHandler.unsetRootDoc = sinon.stub().yields() - @ProjectEntityUpdateHandler._insertDeletedDocReference = sinon.stub().yields() + @ProjectEntityMongoUpdateHandler._insertDeletedDocReference = sinon.stub().yields() @DocumentUpdaterHandler.deleteDoc = sinon.stub().yields() @DocstoreManager.deleteDoc = sinon.stub().yields() @callback = sinon.stub() @@ -912,7 +951,7 @@ describe 'ProjectEntityUpdateHandler', -> .calledWith(project_id, @doc._id.toString()) it "should insert the doc into the deletedDocs array", -> - @ProjectEntityUpdateHandler._insertDeletedDocReference + @ProjectEntityMongoUpdateHandler._insertDeletedDocReference .calledWith(@project._id, @doc) .should.equal true @@ -941,28 +980,4 @@ describe 'ProjectEntityUpdateHandler', -> it "should call the callback", -> @callback.called.should.equal true - describe "_insertDeletedDocReference", -> - beforeEach -> - @doc = - _id: ObjectId() - name: "test.tex" - @callback = sinon.stub() - @ProjectModel.update = sinon.stub().yields() - @ProjectEntityUpdateHandler._insertDeletedDocReference project_id, @doc, @callback - - it "should insert the doc into deletedDocs", -> - @ProjectModel.update - .calledWith({ - _id: project_id - }, { - $push: { - deletedDocs: { - _id: @doc._id - name: @doc.name - } - } - }) - .should.equal true - - it "should call the callback", -> - @callback.called.should.equal true +