diff --git a/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterHandler.js b/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterHandler.js index 2f7bfeb996..082856052a 100644 --- a/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterHandler.js +++ b/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterHandler.js @@ -1,445 +1,395 @@ -/* eslint-disable - camelcase, - handle-callback-err, - max-len, - no-unused-vars, -*/ -// TODO: This file was created by bulk-decaffeinate. -// Fix any style issues and re-enable lint. -/* - * decaffeinate suggestions: - * DS101: Remove unnecessary use of Array.from - * DS102: Remove unnecessary code created because of implicit returns - * DS103: Rewrite code to no longer use __guard__ - * DS207: Consider shorter variations of null checks - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ -let DocumentUpdaterHandler -let request = require('request') -request = request.defaults() +const request = require('request').defaults() const settings = require('settings-sharelatex') const _ = require('underscore') const async = require('async') const logger = require('logger-sharelatex') const metrics = require('metrics-sharelatex') -const { Project } = require('../../models/Project') -const { promisifyAll } = require('../../util/promises') +const { promisify } = require('util') -module.exports = DocumentUpdaterHandler = { - flushProjectToMongo(project_id, callback) { - if (callback == null) { - callback = function(error) {} - } - return DocumentUpdaterHandler._makeRequest( - { - path: `/project/${project_id}/flush`, - method: 'POST' - }, - project_id, - 'flushing.mongo.project', - callback - ) - }, +module.exports = { + flushProjectToMongo, + flushMultipleProjectsToMongo, + flushProjectToMongoAndDelete, + flushDocToMongo, + deleteDoc, + getDocument, + setDocument, + getProjectDocsIfMatch, + clearProjectState, + acceptChanges, + deleteThread, + resyncProjectHistory, + updateProjectStructure, + promises: { + flushProjectToMongo: promisify(flushProjectToMongo), + flushMultipleProjectsToMongo: promisify(flushMultipleProjectsToMongo), + flushProjectToMongoAndDelete: promisify(flushProjectToMongoAndDelete), + flushDocToMongo: promisify(flushDocToMongo), + deleteDoc: promisify(deleteDoc), + getDocument: promisify(getDocument), + setDocument: promisify(setDocument), + getProjectDocsIfMatch: promisify(getProjectDocsIfMatch), + clearProjectState: promisify(clearProjectState), + acceptChanges: promisify(acceptChanges), + deleteThread: promisify(deleteThread), + resyncProjectHistory: promisify(resyncProjectHistory), + updateProjectStructure: promisify(updateProjectStructure) + } +} - flushMultipleProjectsToMongo(project_ids, callback) { - if (callback == null) { - callback = function(error) {} - } - const jobs = [] - for (let project_id of Array.from(project_ids)) { - ;(project_id => - jobs.push(callback => - DocumentUpdaterHandler.flushProjectToMongo(project_id, callback) - ))(project_id) - } - return async.series(jobs, callback) - }, +function flushProjectToMongo(projectId, callback) { + _makeRequest( + { + path: `/project/${projectId}/flush`, + method: 'POST' + }, + projectId, + 'flushing.mongo.project', + callback + ) +} - flushProjectToMongoAndDelete(project_id, callback) { - if (callback == null) { - callback = function() {} - } - const timer = new metrics.Timer('delete.mongo.project') - const url = `${settings.apis.documentupdater.url}` - return DocumentUpdaterHandler._makeRequest( - { - path: `/project/${project_id}`, - method: 'DELETE' - }, - project_id, - 'flushing.mongo.project', - callback - ) - }, +function flushMultipleProjectsToMongo(projectIds, callback) { + const jobs = projectIds.map(projectId => callback => { + flushProjectToMongo(projectId, callback) + }) + async.series(jobs, callback) +} - flushDocToMongo(project_id, doc_id, callback) { - if (callback == null) { - callback = function(error) {} - } - return DocumentUpdaterHandler._makeRequest( - { - path: `/project/${project_id}/doc/${doc_id}/flush`, - method: 'POST' - }, - project_id, - 'flushing.mongo.doc', - callback - ) - }, +function flushProjectToMongoAndDelete(projectId, callback) { + _makeRequest( + { + path: `/project/${projectId}`, + method: 'DELETE' + }, + projectId, + 'flushing.mongo.project', + callback + ) +} - deleteDoc(project_id, doc_id, callback) { - if (callback == null) { - callback = function() {} - } - return DocumentUpdaterHandler._makeRequest( - { - path: `/project/${project_id}/doc/${doc_id}`, - method: 'DELETE' - }, - project_id, - 'delete.mongo.doc', - callback - ) - }, +function flushDocToMongo(projectId, docId, callback) { + _makeRequest( + { + path: `/project/${projectId}/doc/${docId}/flush`, + method: 'POST' + }, + projectId, + 'flushing.mongo.doc', + callback + ) +} - getDocument(project_id, doc_id, fromVersion, callback) { - if (callback == null) { - callback = function(error, doclines, version, ranges, ops) {} - } - return DocumentUpdaterHandler._makeRequest( - { - path: `/project/${project_id}/doc/${doc_id}?fromVersion=${fromVersion}`, - json: true - }, - project_id, - 'get-document', - function(error, doc) { - if (error != null) { - return callback(error) - } - return callback(null, doc.lines, doc.version, doc.ranges, doc.ops) - } - ) - }, +function deleteDoc(projectId, docId, callback) { + _makeRequest( + { + path: `/project/${projectId}/doc/${docId}`, + method: 'DELETE' + }, + projectId, + 'delete.mongo.doc', + callback + ) +} - setDocument(project_id, doc_id, user_id, docLines, source, callback) { - if (callback == null) { - callback = function(error) {} - } - return DocumentUpdaterHandler._makeRequest( - { - path: `/project/${project_id}/doc/${doc_id}`, - method: 'POST', - json: { - lines: docLines, - source, - user_id - } - }, - project_id, - 'set-document', - callback - ) - }, - - getProjectDocsIfMatch(project_id, projectStateHash, callback) { - // If the project state hasn't changed, we can get all the latest - // docs from redis via the docupdater. Otherwise we will need to - // fall back to getting them from mongo. - if (callback == null) { - callback = function(error, docs) {} - } - const timer = new metrics.Timer('get-project-docs') - const url = `${ - settings.apis.documentupdater.url - }/project/${project_id}/get_and_flush_if_old?state=${projectStateHash}` - return request.post(url, function(error, res, body) { - timer.done() - if (error != null) { - logger.warn( - { err: error, url, project_id }, - 'error getting project docs from doc updater' - ) +function getDocument(projectId, docId, fromVersion, callback) { + _makeRequest( + { + path: `/project/${projectId}/doc/${docId}?fromVersion=${fromVersion}`, + json: true + }, + projectId, + 'get-document', + function(error, doc) { + if (error) { return callback(error) } - if (res.statusCode === 409) { - // HTTP response code "409 Conflict" - // Docupdater has checked the projectStateHash and found that - // it has changed. This means that the docs currently in redis - // aren't the only change to the project and the full set of - // docs/files should be retreived from docstore/filestore - // instead. - return callback() - } else if (res.statusCode >= 200 && res.statusCode < 300) { - let docs - try { - docs = JSON.parse(body) - } catch (error1) { - error = error1 - return callback(error) - } - return callback(null, docs) - } else { - logger.warn( - { project_id, url }, + callback(null, doc.lines, doc.version, doc.ranges, doc.ops) + } + ) +} + +function setDocument(projectId, docId, userId, docLines, source, callback) { + _makeRequest( + { + path: `/project/${projectId}/doc/${docId}`, + method: 'POST', + json: { + lines: docLines, + source, + user_id: userId + } + }, + projectId, + 'set-document', + callback + ) +} + +function getProjectDocsIfMatch(projectId, projectStateHash, callback) { + // If the project state hasn't changed, we can get all the latest + // docs from redis via the docupdater. Otherwise we will need to + // fall back to getting them from mongo. + const timer = new metrics.Timer('get-project-docs') + const url = `${ + settings.apis.documentupdater.url + }/project/${projectId}/get_and_flush_if_old?state=${projectStateHash}` + request.post(url, function(error, res, body) { + timer.done() + if (error) { + logger.warn( + { err: error, url, projectId }, + 'error getting project docs from doc updater' + ) + return callback(error) + } + if (res.statusCode === 409) { + // HTTP response code "409 Conflict" + // Docupdater has checked the projectStateHash and found that + // it has changed. This means that the docs currently in redis + // aren't the only change to the project and the full set of + // docs/files should be retreived from docstore/filestore + // instead. + callback() + } else if (res.statusCode >= 200 && res.statusCode < 300) { + let docs + try { + docs = JSON.parse(body) + } catch (error1) { + error = error1 + return callback(error) + } + callback(null, docs) + } else { + logger.warn( + { projectId, url }, + `doc updater returned a non-success status code: ${res.statusCode}` + ) + callback( + new Error( `doc updater returned a non-success status code: ${res.statusCode}` ) - return callback( - new Error( - `doc updater returned a non-success status code: ${res.statusCode}` - ) - ) - } - }) - }, - - clearProjectState(project_id, callback) { - if (callback == null) { - callback = function(error) {} - } - return DocumentUpdaterHandler._makeRequest( - { - path: `/project/${project_id}/clearState`, - method: 'POST' - }, - project_id, - 'clear-project-state', - callback - ) - }, - - acceptChanges(project_id, doc_id, change_ids, callback) { - if (change_ids == null) { - change_ids = [] - } - if (callback == null) { - callback = function(error) {} - } - return DocumentUpdaterHandler._makeRequest( - { - path: `/project/${project_id}/doc/${doc_id}/change/accept`, - json: { - change_ids - }, - method: 'POST' - }, - project_id, - 'accept-changes', - callback - ) - }, - - deleteThread(project_id, doc_id, thread_id, callback) { - if (callback == null) { - callback = function(error) {} - } - const timer = new metrics.Timer('delete-thread') - return DocumentUpdaterHandler._makeRequest( - { - path: `/project/${project_id}/doc/${doc_id}/comment/${thread_id}`, - method: 'DELETE' - }, - project_id, - 'delete-thread', - callback - ) - }, - - resyncProjectHistory(project_id, projectHistoryId, docs, files, callback) { - return DocumentUpdaterHandler._makeRequest( - { - path: `/project/${project_id}/history/resync`, - json: { docs, files, projectHistoryId }, - method: 'POST' - }, - project_id, - 'resync-project-history', - callback - ) - }, - - updateProjectStructure( - project_id, - projectHistoryId, - userId, - changes, - callback - ) { - if (callback == null) { - callback = function(error) {} - } - if ( - !(settings.apis.project_history != null - ? settings.apis.project_history.sendProjectStructureOps - : undefined) - ) { - return callback() - } - - const docUpdates = DocumentUpdaterHandler._getUpdates( - 'doc', - changes.oldDocs, - changes.newDocs - ) - const fileUpdates = DocumentUpdaterHandler._getUpdates( - 'file', - changes.oldFiles, - changes.newFiles - ) - const projectVersion = __guard__( - changes != null ? changes.newProject : undefined, - x => x.version - ) - - if (docUpdates.length + fileUpdates.length < 1) { - return callback() - } - - if (projectVersion == null) { - logger.warn( - { project_id, changes, projectVersion }, - 'did not receive project version in changes' ) - return callback(new Error('did not receive project version in changes')) } + }) +} - return DocumentUpdaterHandler._makeRequest( - { - path: `/project/${project_id}`, - json: { - docUpdates, - fileUpdates, - userId, - version: projectVersion, - projectHistoryId - }, - method: 'POST' - }, - project_id, - 'update-project-structure', - callback - ) - }, +function clearProjectState(projectId, callback) { + _makeRequest( + { + path: `/project/${projectId}/clearState`, + method: 'POST' + }, + projectId, + 'clear-project-state', + callback + ) +} - _makeRequest(options, project_id, metricsKey, callback) { - const timer = new metrics.Timer(metricsKey) - return request( - { - url: `${settings.apis.documentupdater.url}${options.path}`, - json: options.json, - method: options.method || 'GET' - }, - function(error, res, body) { - timer.done() - if (error != null) { - logger.warn( - { error, project_id }, - 'error making request to document updater' - ) - return callback(error) - } else if (res.statusCode >= 200 && res.statusCode < 300) { - return callback(null, body) - } else { - error = new Error( - `document updater returned a failure status code: ${res.statusCode}` - ) - logger.warn( - { error, project_id }, - `document updater returned failure status code: ${res.statusCode}` - ) - return callback(error) - } - } - ) - }, +function acceptChanges(projectId, docId, changeIds, callback) { + _makeRequest( + { + path: `/project/${projectId}/doc/${docId}/change/accept`, + json: { change_ids: changeIds }, + method: 'POST' + }, + projectId, + 'accept-changes', + callback + ) +} - _getUpdates(entityType, oldEntities, newEntities) { - let id, newEntity, oldEntity - if (!oldEntities) { - oldEntities = [] - } - if (!newEntities) { - newEntities = [] - } - const updates = [] +function deleteThread(projectId, docId, threadId, callback) { + _makeRequest( + { + path: `/project/${projectId}/doc/${docId}/comment/${threadId}`, + method: 'DELETE' + }, + projectId, + 'delete-thread', + callback + ) +} - const oldEntitiesHash = _.indexBy(oldEntities, entity => - entity[entityType]._id.toString() - ) - const newEntitiesHash = _.indexBy(newEntities, entity => - entity[entityType]._id.toString() - ) +function resyncProjectHistory( + projectId, + projectHistoryId, + docs, + files, + callback +) { + _makeRequest( + { + path: `/project/${projectId}/history/resync`, + json: { docs, files, projectHistoryId }, + method: 'POST' + }, + projectId, + 'resync-project-history', + callback + ) +} - // 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 in oldEntitiesHash) { - oldEntity = oldEntitiesHash[id] - newEntity = newEntitiesHash[id] - - if (newEntity == null) { - // entity deleted - updates.push({ - id, - pathname: oldEntity.path, - newPathname: '' - }) - } - } - - for (id in newEntitiesHash) { - newEntity = newEntitiesHash[id] - oldEntity = oldEntitiesHash[id] - - if (oldEntity == null) { - // entity added - updates.push({ - id, - pathname: newEntity.path, - docLines: newEntity.docLines, - url: newEntity.url, - hash: newEntity.file != null ? newEntity.file.hash : undefined - }) - } else if (newEntity.path !== oldEntity.path) { - // entity renamed - updates.push({ - id, - pathname: oldEntity.path, - newPathname: newEntity.path - }) - } - } - - return updates +function updateProjectStructure( + projectId, + projectHistoryId, + userId, + changes, + callback +) { + if ( + settings.apis.project_history == null || + !settings.apis.project_history.sendProjectStructureOps + ) { + return callback() } -} -module.exports.promises = promisifyAll(DocumentUpdaterHandler, { - without: ['_getUpdates'] -}) -const PENDINGUPDATESKEY = 'PendingUpdates' -const DOCLINESKEY = 'doclines' -const DOCIDSWITHPENDINGUPDATES = 'DocsWithPendingUpdates' + const { + deletes: docDeletes, + adds: docAdds, + renames: docRenames + } = _getUpdates('doc', changes.oldDocs, changes.newDocs) + const { + deletes: fileDeletes, + adds: fileAdds, + renames: fileRenames + } = _getUpdates('file', changes.oldFiles, changes.newFiles) + const updates = [].concat( + docDeletes, + fileDeletes, + docAdds, + fileAdds, + docRenames, + fileRenames + ) + const projectVersion = + changes && changes.newProject && changes.newProject.version -const keys = { - pendingUpdates(op) { - return `${PENDINGUPDATESKEY}:${op.doc_id}` - }, - docsWithPendingUpdates: DOCIDSWITHPENDINGUPDATES, - docLines(op) { - return `${DOCLINESKEY}:${op.doc_id}` - }, - combineProjectIdAndDocId(project_id, doc_id) { - return `${project_id}:${doc_id}` + if (updates.length < 1) { + return callback() } + + if (projectVersion == null) { + logger.warn( + { projectId, changes, projectVersion }, + 'did not receive project version in changes' + ) + return callback(new Error('did not receive project version in changes')) + } + + _makeRequest( + { + path: `/project/${projectId}`, + json: { + updates, + userId, + version: projectVersion, + projectHistoryId + }, + method: 'POST' + }, + projectId, + 'update-project-structure', + callback + ) } -function __guard__(value, transform) { - return typeof value !== 'undefined' && value !== null - ? transform(value) - : undefined +function _makeRequest(options, projectId, metricsKey, callback) { + const timer = new metrics.Timer(metricsKey) + request( + { + url: `${settings.apis.documentupdater.url}${options.path}`, + json: options.json, + method: options.method || 'GET' + }, + function(error, res, body) { + timer.done() + if (error) { + logger.warn( + { error, projectId }, + 'error making request to document updater' + ) + callback(error) + } else if (res.statusCode >= 200 && res.statusCode < 300) { + callback(null, body) + } else { + error = new Error( + `document updater returned a failure status code: ${res.statusCode}` + ) + logger.warn( + { error, projectId }, + `document updater returned failure status code: ${res.statusCode}` + ) + callback(error) + } + } + ) +} + +function _getUpdates(entityType, oldEntities, newEntities) { + if (!oldEntities) { + oldEntities = [] + } + if (!newEntities) { + newEntities = [] + } + const deletes = [] + const adds = [] + const renames = [] + + const oldEntitiesHash = _.indexBy(oldEntities, entity => + entity[entityType]._id.toString() + ) + const 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 (const id in oldEntitiesHash) { + const oldEntity = oldEntitiesHash[id] + const newEntity = newEntitiesHash[id] + + if (newEntity == null) { + // entity deleted + deletes.push({ + type: `rename-${entityType}`, + id, + pathname: oldEntity.path, + newPathname: '' + }) + } + } + + for (const id in newEntitiesHash) { + const newEntity = newEntitiesHash[id] + const oldEntity = oldEntitiesHash[id] + + if (oldEntity == null) { + // entity added + adds.push({ + type: `add-${entityType}`, + id, + pathname: newEntity.path, + docLines: newEntity.docLines, + url: newEntity.url, + hash: newEntity.file != null ? newEntity.file.hash : undefined + }) + } else if (newEntity.path !== oldEntity.path) { + // entity renamed + renames.push({ + type: `rename-${entityType}`, + id, + pathname: oldEntity.path, + newPathname: newEntity.path + }) + } + } + + return { deletes, adds, renames } } diff --git a/services/web/app/src/Features/Project/ProjectEntityUpdateHandler.js b/services/web/app/src/Features/Project/ProjectEntityUpdateHandler.js index 1a5d611ec1..3c9882b56c 100644 --- a/services/web/app/src/Features/Project/ProjectEntityUpdateHandler.js +++ b/services/web/app/src/Features/Project/ProjectEntityUpdateHandler.js @@ -607,7 +607,7 @@ const ProjectEntityUpdateHandler = { } ProjectLocator.findElement( { project_id: projectId, element_id: folderId, type: 'folder' }, - (error, folder, path) => { + (error, folder, folderPath) => { if (error != null) { return callback(error) } @@ -620,6 +620,7 @@ const ProjectEntityUpdateHandler = { ) if (existingFile) { const doc = new Doc({ name: docName }) + const filePath = `${folderPath.fileSystem}/${existingFile.name}` DocstoreManager.updateDoc( projectId.toString(), doc._id.toString(), @@ -642,7 +643,7 @@ const ProjectEntityUpdateHandler = { { project_id: projectId, doc_id: doc._id, - path: path.fileSystem, + path: filePath, project_name: project.name, rev: existingFile.rev + 1 }, @@ -650,7 +651,6 @@ const ProjectEntityUpdateHandler = { if (err) { return callback(err) } - const docPath = path.fileSystem const projectHistoryId = project.overleaf && project.overleaf.history && @@ -658,14 +658,14 @@ const ProjectEntityUpdateHandler = { const newDocs = [ { doc, - path: docPath, + path: filePath, docLines: docLines.join('\n') } ] const oldFiles = [ { file: existingFile, - path: Path.join(path.fileSystem, existingFile.name) + path: filePath } ] DocumentUpdaterHandler.updateProjectStructure( diff --git a/services/web/test/acceptance/src/ProjectStructureTests.js b/services/web/test/acceptance/src/ProjectStructureTests.js index 7dfe84ce69..937575c1bb 100644 --- a/services/web/test/acceptance/src/ProjectStructureTests.js +++ b/services/web/test/acceptance/src/ProjectStructureTests.js @@ -267,33 +267,24 @@ describe('ProjectStructureChanges', function() { }) }) - it('should version creating a doc', function() { - const { - docUpdates: updates, - version - } = MockDocUpdaterApi.getProjectStructureUpdates(exampleProjectId) - expect(updates.length).to.equal(2) - _.each(updates, update => { + it('should version creating a doc and a file', function() { + const { updates, version } = MockDocUpdaterApi.getProjectStructureUpdates( + exampleProjectId + ) + expect(updates.length).to.equal(3) + for (const update of updates.slice(0, 2)) { + expect(update.type).to.equal('add-doc') expect(update.userId).to.equal(owner._id) expect(update.docLines).to.be.a('string') - }) + } expect(_.where(updates, { pathname: '/main.tex' }).length).to.equal(1) expect(_.where(updates, { pathname: '/references.bib' }).length).to.equal( 1 ) - expect(version).to.equal(3) - }) - - it('should version creating a file', function() { - const { - fileUpdates: updates, - version - } = MockDocUpdaterApi.getProjectStructureUpdates(exampleProjectId) - expect(updates.length).to.equal(1) - const update = updates[0] - expect(update.userId).to.equal(owner._id) - expect(update.pathname).to.equal('/universe.jpg') - expect(update.url).to.be.a('string') + expect(updates[2].type).to.equal('add-file') + expect(updates[2].userId).to.equal(owner._id) + expect(updates[2].pathname).to.equal('/universe.jpg') + expect(updates[2].url).to.be.a('string') expect(version).to.equal(3) }) }) @@ -328,33 +319,24 @@ describe('ProjectStructureChanges', function() { }) }) - it('should version the docs created', function() { - const { - docUpdates: updates, - version - } = MockDocUpdaterApi.getProjectStructureUpdates(dupProjectId) - expect(updates.length).to.equal(2) - _.each(updates, update => { + it('should version the docs and files created', function() { + const { updates, version } = MockDocUpdaterApi.getProjectStructureUpdates( + dupProjectId + ) + expect(updates.length).to.equal(3) + for (const update of updates.slice(0, 2)) { + expect(update.type).to.equal('add-doc') expect(update.userId).to.equal(owner._id) expect(update.docLines).to.be.a('string') - }) + } expect(_.where(updates, { pathname: '/main.tex' }).length).to.equal(1) expect(_.where(updates, { pathname: '/references.bib' }).length).to.equal( 1 ) - expect(version).to.equal(1) - }) - - it('should version the files created', function() { - const { - fileUpdates: updates, - version - } = MockDocUpdaterApi.getProjectStructureUpdates(dupProjectId) - expect(updates.length).to.equal(1) - const update = updates[0] - expect(update.userId).to.equal(owner._id) - expect(update.pathname).to.equal('/universe.jpg') - expect(update.url).to.be.a('string') + expect(updates[2].type).to.equal('add-file') + expect(updates[2].userId).to.equal(owner._id) + expect(updates[2].pathname).to.equal('/universe.jpg') + expect(updates[2].url).to.be.a('string') expect(version).to.equal(1) }) }) @@ -382,11 +364,12 @@ describe('ProjectStructureChanges', function() { it('should version the doc added', function(done) { const { - docUpdates: updates, + updates, version: newVersion } = MockDocUpdaterApi.getProjectStructureUpdates(exampleProjectId) expect(updates.length).to.equal(1) const update = updates[0] + expect(update.type).to.equal('add-doc') expect(update.userId).to.equal(owner._id) expect(update.pathname).to.equal('/new.tex') expect(update.docLines).to.be.a('string') @@ -414,29 +397,19 @@ describe('ProjectStructureChanges', function() { }) }) - it('should version the docs created', function() { - const { - docUpdates: updates, - version - } = MockDocUpdaterApi.getProjectStructureUpdates(exampleProjectId) - expect(updates.length).to.equal(1) - const update = updates[0] - expect(update.userId).to.equal(owner._id) - expect(update.pathname).to.equal('/main.tex') - expect(update.docLines).to.equal('Test') - expect(version).to.equal(1) - }) - - it('should version the files created', function() { - const { - fileUpdates: updates, - version - } = MockDocUpdaterApi.getProjectStructureUpdates(exampleProjectId) - expect(updates.length).to.equal(1) - const update = updates[0] - expect(update.userId).to.equal(owner._id) - expect(update.pathname).to.equal('/1pixel.png') - expect(update.url).to.be.a('string') + it('should version the docs and files created', function() { + const { updates, version } = MockDocUpdaterApi.getProjectStructureUpdates( + exampleProjectId + ) + expect(updates.length).to.equal(2) + expect(updates[0].type).to.equal('add-doc') + expect(updates[0].userId).to.equal(owner._id) + expect(updates[0].pathname).to.equal('/main.tex') + expect(updates[0].docLines).to.equal('Test') + expect(updates[1].type).to.equal('add-file') + expect(updates[1].userId).to.equal(owner._id) + expect(updates[1].pathname).to.equal('/1pixel.png') + expect(updates[1].url).to.be.a('string') expect(version).to.equal(1) }) }) @@ -597,22 +570,22 @@ describe('ProjectStructureChanges', function() { }) describe('uploading a project with files in different encodings', function() { - let docUpdates + let updates beforeEach(function(done) { uploadExampleProject(owner, 'charsets/charsets.zip', (err, projectId) => { if (err) { return done(err) } - docUpdates = MockDocUpdaterApi.getProjectStructureUpdates(projectId) - .docUpdates + updates = MockDocUpdaterApi.getProjectStructureUpdates(projectId) + .updates done() }) }) it('should correctly parse windows-1252', function() { const update = _.find( - docUpdates, + updates, update => update.pathname === '/test-german-windows-1252.tex' ) expect(update.docLines).to.contain( @@ -622,7 +595,7 @@ describe('ProjectStructureChanges', function() { it('should correctly parse German utf8', function() { const update = _.find( - docUpdates, + updates, update => update.pathname === '/test-german-utf8x.tex' ) expect(update.docLines).to.contain( @@ -632,7 +605,7 @@ describe('ProjectStructureChanges', function() { it('should correctly parse little-endian utf16', function() { const update = _.find( - docUpdates, + updates, update => update.pathname === '/test-greek-utf16-le-bom.tex' ) expect(update.docLines).to.contain( @@ -642,7 +615,7 @@ describe('ProjectStructureChanges', function() { it('should correctly parse Greek utf8', function() { const update = _.find( - docUpdates, + updates, update => update.pathname === '/test-greek-utf8x.tex' ) expect(update.docLines).to.contain( @@ -675,12 +648,12 @@ describe('ProjectStructureChanges', function() { }) it('should version a newly uploaded file', function(done) { - const { - fileUpdates: updates, - version - } = MockDocUpdaterApi.getProjectStructureUpdates(exampleProjectId) + const { updates, version } = MockDocUpdaterApi.getProjectStructureUpdates( + exampleProjectId + ) expect(updates.length).to.equal(1) const update = updates[0] + expect(update.type).to.equal('add-file') expect(update.userId).to.equal(owner._id) expect(update.pathname).to.equal('/1pixel.png') expect(update.url).to.be.a('string') @@ -701,17 +674,18 @@ describe('ProjectStructureChanges', function() { 'image/png', () => { const { - fileUpdates: updates, + updates, version } = MockDocUpdaterApi.getProjectStructureUpdates(exampleProjectId) expect(updates.length).to.equal(2) - let update = updates[0] - expect(update.userId).to.equal(owner._id) - expect(update.pathname).to.equal('/1pixel.png') - 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(updates[0].type).to.equal('rename-file') + expect(updates[0].userId).to.equal(owner._id) + expect(updates[0].pathname).to.equal('/1pixel.png') + expect(updates[0].newPathname).to.equal('') + expect(updates[1].type).to.equal('add-file') + expect(updates[1].userId).to.equal(owner._id) + expect(updates[1].pathname).to.equal('/1pixel.png') + expect(updates[1].url).to.be.a('string') // two file uploads verifyVersionIncremented( @@ -778,11 +752,12 @@ describe('ProjectStructureChanges', function() { exampleFolderId, () => { const { - docUpdates: updates, + updates, version } = MockDocUpdaterApi.getProjectStructureUpdates(exampleProjectId) expect(updates.length).to.equal(1) const update = updates[0] + expect(update.type).to.equal('rename-doc') expect(update.userId).to.equal(owner._id) expect(update.pathname).to.equal('/new.tex') expect(update.newPathname).to.equal('/foo/new.tex') @@ -808,11 +783,12 @@ describe('ProjectStructureChanges', function() { exampleFolderId, () => { const { - fileUpdates: updates, + updates, version } = MockDocUpdaterApi.getProjectStructureUpdates(exampleProjectId) expect(updates.length).to.equal(1) const update = updates[0] + expect(update.type).to.equal('rename-file') expect(update.userId).to.equal(owner._id) expect(update.pathname).to.equal('/1pixel.png') expect(update.newPathname).to.equal('/foo/1pixel.png') @@ -860,13 +836,14 @@ describe('ProjectStructureChanges', function() { newFolderId, () => { const { - docUpdates: updates, + updates, version } = MockDocUpdaterApi.getProjectStructureUpdates( exampleProjectId ) expect(updates.length).to.equal(1) let update = updates[0] + expect(update.type).to.equal('rename-doc') expect(update.userId).to.equal(owner._id) expect(update.pathname).to.equal('/foo/new.tex') expect(update.newPathname).to.equal('/bar/foo/new.tex') @@ -946,11 +923,12 @@ describe('ProjectStructureChanges', function() { 'wombat.tex', () => { const { - docUpdates: updates, + updates, version } = MockDocUpdaterApi.getProjectStructureUpdates(exampleProjectId) expect(updates.length).to.equal(1) const update = updates[0] + expect(update.type).to.equal('rename-doc') expect(update.userId).to.equal(owner._id) expect(update.pathname).to.equal('/foo/new.tex') expect(update.newPathname).to.equal('/foo/wombat.tex') @@ -975,11 +953,12 @@ describe('ProjectStructureChanges', function() { 'potato.png', () => { const { - fileUpdates: updates, + updates, version } = MockDocUpdaterApi.getProjectStructureUpdates(exampleProjectId) expect(updates.length).to.equal(1) const update = updates[0] + expect(update.type).to.equal('rename-file') expect(update.userId).to.equal(owner._id) expect(update.pathname).to.equal('/foo/1pixel.png') expect(update.newPathname).to.equal('/foo/potato.png') @@ -1004,21 +983,18 @@ describe('ProjectStructureChanges', function() { 'giraffe', () => { const { - docUpdates, - fileUpdates, + updates, version } = MockDocUpdaterApi.getProjectStructureUpdates(exampleProjectId) - expect(docUpdates.length).to.equal(1) - const docUpdate = docUpdates[0] - expect(docUpdate.userId).to.equal(owner._id) - expect(docUpdate.pathname).to.equal('/foo/new.tex') - expect(docUpdate.newPathname).to.equal('/giraffe/new.tex') - - expect(fileUpdates.length).to.equal(1) - const fileUpdate = fileUpdates[0] - expect(fileUpdate.userId).to.equal(owner._id) - expect(fileUpdate.pathname).to.equal('/foo/1pixel.png') - expect(fileUpdate.newPathname).to.equal('/giraffe/1pixel.png') + expect(updates.length).to.equal(2) + expect(updates[0].type).to.equal('rename-doc') + expect(updates[0].userId).to.equal(owner._id) + expect(updates[0].pathname).to.equal('/foo/new.tex') + expect(updates[0].newPathname).to.equal('/giraffe/new.tex') + expect(updates[1].type).to.equal('rename-file') + expect(updates[1].userId).to.equal(owner._id) + expect(updates[1].pathname).to.equal('/foo/1pixel.png') + expect(updates[1].newPathname).to.equal('/giraffe/1pixel.png') verifyVersionIncremented( exampleProjectId, @@ -1078,21 +1054,18 @@ describe('ProjectStructureChanges', function() { it('should version deleting a folder', function(done) { deleteItem(owner, exampleProjectId, 'folder', exampleFolderId, () => { const { - docUpdates, - fileUpdates, + updates, version } = MockDocUpdaterApi.getProjectStructureUpdates(exampleProjectId) - expect(docUpdates.length).to.equal(1) - const docUpdate = docUpdates[0] - expect(docUpdate.userId).to.equal(owner._id) - expect(docUpdate.pathname).to.equal('/foo/new.tex') - expect(docUpdate.newPathname).to.equal('') - - expect(fileUpdates.length).to.equal(1) - const fileUpdate = fileUpdates[0] - expect(fileUpdate.userId).to.equal(owner._id) - expect(fileUpdate.pathname).to.equal('/foo/1pixel.png') - expect(fileUpdate.newPathname).to.equal('') + expect(updates.length).to.equal(2) + expect(updates[0].type).to.equal('rename-doc') + expect(updates[0].userId).to.equal(owner._id) + expect(updates[0].pathname).to.equal('/foo/new.tex') + expect(updates[0].newPathname).to.equal('') + expect(updates[1].type).to.equal('rename-file') + expect(updates[1].userId).to.equal(owner._id) + expect(updates[1].pathname).to.equal('/foo/1pixel.png') + expect(updates[1].newPathname).to.equal('') verifyVersionIncremented(exampleProjectId, oldVersion, version, 1, done) }) @@ -1250,11 +1223,12 @@ describe('ProjectStructureChanges', function() { } const { - docUpdates: updates, + updates, version } = MockDocUpdaterApi.getProjectStructureUpdates(exampleProjectId) expect(updates.length).to.equal(1) const update = updates[0] + expect(update.type).to.equal('add-doc') expect(update.userId).to.equal(owner._id) expect(update.pathname).to.equal('/test.tex') expect(update.docLines).to.equal('Test') @@ -1293,11 +1267,12 @@ describe('ProjectStructureChanges', function() { } const { - fileUpdates: updates, + updates, version } = MockDocUpdaterApi.getProjectStructureUpdates(exampleProjectId) expect(updates.length).to.equal(1) const update = updates[0] + expect(update.type).to.equal('add-file') expect(update.userId).to.equal(owner._id) expect(update.pathname).to.equal('/1pixel.png') expect(update.url).to.be.a('string') @@ -1352,18 +1327,18 @@ describe('ProjectStructureChanges', function() { } const { - fileUpdates: updates, + updates, version } = MockDocUpdaterApi.getProjectStructureUpdates(exampleProjectId) expect(updates.length).to.equal(2) - let 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(updates[0].type).to.equal('rename-file') + expect(updates[0].userId).to.equal(owner._id) + expect(updates[0].pathname).to.equal('/1pixel.png') + expect(updates[0].newPathname).to.equal('') + expect(updates[1].type).to.equal('add-file') + expect(updates[1].userId).to.equal(owner._id) + expect(updates[1].pathname).to.equal('/1pixel.png') + expect(updates[1].url).to.be.a('string') verifyVersionIncremented( exampleProjectId, @@ -1396,11 +1371,12 @@ describe('ProjectStructureChanges', function() { } const { - docUpdates: updates, + updates, version } = MockDocUpdaterApi.getProjectStructureUpdates(exampleProjectId) expect(updates.length).to.equal(1) const update = updates[0] + expect(update.type).to.equal('rename-doc') expect(update.userId).to.equal(owner._id) expect(update.pathname).to.equal('/new.tex') expect(update.newPathname).to.equal('') @@ -1442,11 +1418,12 @@ describe('ProjectStructureChanges', function() { 'test-greek-utf16-le-bom.tex', 'text/x-tex', () => { - const { - docUpdates: updates - } = MockDocUpdaterApi.getProjectStructureUpdates(exampleProjectId) + const { updates } = MockDocUpdaterApi.getProjectStructureUpdates( + exampleProjectId + ) expect(updates.length).to.equal(1) const update = updates[0] + expect(update.type).to.equal('add-doc') expect(update.pathname).to.equal('/test-greek-utf16-le-bom.tex') expect(update.docLines).to.contain( 'Η γρήγορη καστανή αλεπού πήδηξε χαλαρά πάνω από το σκυλί.' @@ -1465,11 +1442,12 @@ describe('ProjectStructureChanges', function() { 'test-german-windows-1252.tex', 'text/x-tex', () => { - const { - docUpdates: updates - } = MockDocUpdaterApi.getProjectStructureUpdates(exampleProjectId) + const { updates } = MockDocUpdaterApi.getProjectStructureUpdates( + exampleProjectId + ) expect(updates.length).to.equal(1) const update = updates[0] + expect(update.type).to.equal('add-doc') expect(update.pathname).to.equal('/test-german-windows-1252.tex') expect(update.docLines).to.contain( 'Der schnelle braune Fuchs sprang träge über den Hund.' diff --git a/services/web/test/acceptance/src/helpers/MockDocUpdaterApi.js b/services/web/test/acceptance/src/helpers/MockDocUpdaterApi.js index 728a955e22..5f552d330e 100644 --- a/services/web/test/acceptance/src/helpers/MockDocUpdaterApi.js +++ b/services/web/test/acceptance/src/helpers/MockDocUpdaterApi.js @@ -1,108 +1,76 @@ -/* eslint-disable - camelcase, - max-len, - no-return-assign, -*/ -// TODO: This file was created by bulk-decaffeinate. -// Fix any style issues and re-enable lint. -/* - * decaffeinate suggestions: - * DS101: Remove unnecessary use of Array.from - * DS102: Remove unnecessary code created because of implicit returns - * DS207: Consider shorter variations of null checks - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ -let MockDocUpdaterApi const express = require('express') const app = express() const bodyParser = require('body-parser') const jsonParser = bodyParser.json() -module.exports = MockDocUpdaterApi = { +const MockDocUpdaterApi = { updates: {}, clearProjectStructureUpdates() { - return (this.updates = {}) + this.updates = {} }, - getProjectStructureUpdates(project_id) { - return this.updates[project_id] || { docUpdates: [], fileUpdates: [] } + getProjectStructureUpdates(projectId) { + return this.updates[projectId] || { updates: [] } }, - addProjectStructureUpdates( - project_id, - userId, - docUpdates, - fileUpdates, - version - ) { - let update - if (!this.updates[project_id]) { - this.updates[project_id] = { docUpdates: [], fileUpdates: [] } + addProjectStructureUpdates(projectId, userId, updates, version) { + if (!this.updates[projectId]) { + this.updates[projectId] = { updates: [] } } - for (update of Array.from(docUpdates)) { + for (const update of updates) { update.userId = userId - this.updates[project_id].docUpdates.push(update) + this.updates[projectId].updates.push(update) } - for (update of Array.from(fileUpdates)) { - update.userId = userId - this.updates[project_id].fileUpdates.push(update) - } - - return (this.updates[project_id].version = version) + this.updates[projectId].version = version }, run() { - app.post('/project/:project_id/flush', (req, res, next) => { - return res.sendStatus(204) + app.post('/project/:projectId/flush', (req, res, next) => { + res.sendStatus(204) }) - app.post('/project/:project_id', jsonParser, (req, res, next) => { - const { project_id } = req.params - const { userId, docUpdates, fileUpdates, version } = req.body - this.addProjectStructureUpdates( - project_id, - userId, - docUpdates, - fileUpdates, - version - ) - return res.sendStatus(200) + app.post('/project/:projectId', jsonParser, (req, res, next) => { + const { projectId } = req.params + const { userId, updates, version } = req.body + this.addProjectStructureUpdates(projectId, userId, updates, version) + res.sendStatus(200) }) - app.post('/project/:project_id/doc/:doc_id', (req, res, next) => { - return res.sendStatus(204) + app.post('/project/:projectId/doc/:doc_id', (req, res, next) => { + res.sendStatus(204) }) - app.delete('/project/:project_id', (req, res) => { - return res.sendStatus(204) + app.delete('/project/:projectId', (req, res) => { + res.sendStatus(204) }) - app.post('/project/:project_id/doc/:doc_id/flush', (req, res, next) => { - return res.sendStatus(204) + app.post('/project/:projectId/doc/:doc_id/flush', (req, res, next) => { + res.sendStatus(204) }) - app.delete('/project/:project_id/doc/:doc_id', (req, res, next) => { - return res.sendStatus(204) + app.delete('/project/:projectId/doc/:doc_id', (req, res, next) => { + res.sendStatus(204) }) - app.post('/project/:project_id/history/resync', (req, res, next) => { - return res.sendStatus(204) + app.post('/project/:projectId/history/resync', (req, res, next) => { + res.sendStatus(204) }) - return app + app .listen(3003, error => { - if (error != null) { + if (error) { throw error } }) .on('error', error => { console.error('error starting MockDocUpdaterApi:', error.message) - return process.exit(1) + process.exit(1) }) } } MockDocUpdaterApi.run() +module.exports = MockDocUpdaterApi diff --git a/services/web/test/unit/src/DocumentUpdater/DocumentUpdaterHandlerTests.js b/services/web/test/unit/src/DocumentUpdater/DocumentUpdaterHandlerTests.js index 97a1a0b6dd..7853ad89a0 100644 --- a/services/web/test/unit/src/DocumentUpdater/DocumentUpdaterHandlerTests.js +++ b/services/web/test/unit/src/DocumentUpdater/DocumentUpdaterHandlerTests.js @@ -882,8 +882,9 @@ describe('DocumentUpdaterHandler', function() { newProject: { version: this.version } } - const docUpdates = [ + const updates = [ { + type: 'rename-doc', id: this.docIdB.toString(), pathname: '/old_b', newPathname: '/new_b' @@ -901,8 +902,7 @@ describe('DocumentUpdaterHandler', function() { url: this.url, method: 'POST', json: { - docUpdates, - fileUpdates: [], + updates, userId: this.user_id, version: this.version, projectHistoryId: this.projectHistoryId @@ -925,8 +925,9 @@ describe('DocumentUpdaterHandler', function() { newProject: { version: this.version } } - const docUpdates = [ + const updates = [ { + type: 'add-doc', id: this.docId.toString(), pathname: '/foo', docLines: 'a\nb', @@ -946,8 +947,7 @@ describe('DocumentUpdaterHandler', function() { url: this.url, method: 'POST', json: { - docUpdates, - fileUpdates: [], + updates, userId: this.user_id, version: this.version, projectHistoryId: this.projectHistoryId @@ -974,8 +974,9 @@ describe('DocumentUpdaterHandler', function() { newProject: { version: this.version } } - const fileUpdates = [ + const updates = [ { + type: 'add-file', id: this.fileId.toString(), pathname: '/bar', url: 'filestore.example.com/file', @@ -995,8 +996,7 @@ describe('DocumentUpdaterHandler', function() { url: this.url, method: 'POST', json: { - docUpdates: [], - fileUpdates, + updates, userId: this.user_id, version: this.version, projectHistoryId: this.projectHistoryId @@ -1019,8 +1019,9 @@ describe('DocumentUpdaterHandler', function() { newProject: { version: this.version } } - const docUpdates = [ + const updates = [ { + type: 'rename-doc', id: this.docId.toString(), pathname: '/foo', newPathname: '' @@ -1038,8 +1039,7 @@ describe('DocumentUpdaterHandler', function() { url: this.url, method: 'POST', json: { - docUpdates, - fileUpdates: [], + updates, userId: this.user_id, version: this.version, projectHistoryId: this.projectHistoryId @@ -1052,6 +1052,67 @@ describe('DocumentUpdaterHandler', function() { }) }) + describe('when a file is converted to a doc', function() { + it('should send the delete first', function(done) { + this.docId = new ObjectId() + this.fileId = new ObjectId() + this.changes = { + oldFiles: [ + { + path: '/foo.doc', + url: 'filestore.example.com/file', + file: { _id: this.fileId } + } + ], + newDocs: [ + { + path: '/foo.doc', + docLines: 'hello there', + doc: { _id: this.docId } + } + ], + newProject: { version: this.version } + } + + const updates = [ + { + type: 'rename-file', + id: this.fileId.toString(), + pathname: '/foo.doc', + newPathname: '' + }, + { + type: 'add-doc', + id: this.docId.toString(), + pathname: '/foo.doc', + docLines: 'hello there', + url: undefined, + hash: undefined + } + ] + + this.handler.updateProjectStructure( + this.project_id, + this.projectHistoryId, + this.user_id, + this.changes, + () => { + this.request.should.have.been.calledWith({ + url: this.url, + method: 'POST', + json: { + updates, + userId: this.user_id, + version: this.version, + projectHistoryId: this.projectHistoryId + } + }) + done() + } + ) + }) + }) + describe('when the project version is missing', function() { it('should call the callback with an error', function() { this.docId = new ObjectId() diff --git a/services/web/test/unit/src/Project/ProjectEntityUpdateHandlerTests.js b/services/web/test/unit/src/Project/ProjectEntityUpdateHandlerTests.js index 5c7ec60232..42d69c44de 100644 --- a/services/web/test/unit/src/Project/ProjectEntityUpdateHandlerTests.js +++ b/services/web/test/unit/src/Project/ProjectEntityUpdateHandlerTests.js @@ -880,10 +880,17 @@ describe('ProjectEntityUpdateHandler', function() { this.folder = { _id: folderId, docs: [], fileRefs: [this.existingFile] } this.newDoc = { _id: docId } this.docLines = ['line one', 'line two'] - this.path = 'path/to/file' - this.ProjectLocator.findElement.yields(null, this.folder, { - fileSystem: this.path - }) + this.folderPath = '/path/to/folder' + this.filePath = '/path/to/folder/foo.tex' + this.ProjectLocator.findElement + .withArgs({ + project_id: projectId, + element_id: this.folder._id, + type: 'folder' + }) + .yields(null, this.folder, { + fileSystem: this.folderPath + }) this.DocstoreManager.updateDoc.yields() this.ProjectEntityMongoUpdateHandler.replaceFileWithDoc.yields( null, @@ -924,7 +931,7 @@ describe('ProjectEntityUpdateHandler', function() { expect(this.TpdsUpdateSender.addDoc).to.have.been.calledWith({ project_id: projectId, doc_id: this.newDoc._id, - path: this.path, + path: this.filePath, project_name: this.newProject.name, rev: this.existingFile.rev + 1 }) @@ -934,13 +941,13 @@ describe('ProjectEntityUpdateHandler', function() { const oldFiles = [ { file: this.existingFile, - path: `${this.path}/foo.tex` + path: this.filePath } ] const newDocs = [ { doc: sinon.match(this.newDoc), - path: this.path, + path: this.filePath, docLines: this.docLines.join('\n') } ]