diff --git a/libraries/overleaf-editor-core/index.js b/libraries/overleaf-editor-core/index.js index fc437c9d26..2e66720579 100644 --- a/libraries/overleaf-editor-core/index.js +++ b/libraries/overleaf-editor-core/index.js @@ -1,30 +1,61 @@ -exports.Author = require('./lib/author') -exports.AuthorList = require('./lib/author_list') -exports.Blob = require('./lib/blob') -exports.Change = require('./lib/change') -exports.ChangeRequest = require('./lib/change_request') -exports.ChangeNote = require('./lib/change_note') -exports.Chunk = require('./lib/chunk') -exports.ChunkResponse = require('./lib/chunk_response') -exports.File = require('./lib/file') -exports.FileMap = require('./lib/file_map') -exports.History = require('./lib/history') -exports.Label = require('./lib/label') -exports.AddFileOperation = require('./lib/operation/add_file_operation') -exports.MoveFileOperation = require('./lib/operation/move_file_operation') -exports.EditFileOperation = require('./lib/operation/edit_file_operation') -exports.EditNoOperation = require('./lib/operation/edit_no_operation') -exports.AddCommentOperation = require('./lib/operation/add_comment_operation') -exports.DeleteCommentOperation = require('./lib/operation/delete_comment_operation') -exports.SetFileMetadataOperation = require('./lib/operation/set_file_metadata_operation') -exports.NoOperation = require('./lib/operation/no_operation') -exports.Operation = require('./lib/operation') -exports.RestoreOrigin = require('./lib/origin/restore_origin') -exports.Origin = require('./lib/origin') -exports.OtClient = require('./lib/ot_client') -exports.TextOperation = require('./lib/operation/text_operation') -exports.EditOperation = require('./lib/operation/edit_operation') -exports.safePathname = require('./lib/safe_pathname') -exports.Snapshot = require('./lib/snapshot') -exports.util = require('./lib/util') -exports.V2DocVersions = require('./lib/v2_doc_versions') +const AddCommentOperation = require('./lib/operation/add_comment_operation') +const Author = require('./lib/author') +const AuthorList = require('./lib/author_list') +const Blob = require('./lib/blob') +const Change = require('./lib/change') +const ChangeRequest = require('./lib/change_request') +const ChangeNote = require('./lib/change_note') +const Chunk = require('./lib/chunk') +const ChunkResponse = require('./lib/chunk_response') +const DeleteCommentOperation = require('./lib/operation/delete_comment_operation') +const File = require('./lib/file') +const FileMap = require('./lib/file_map') +const History = require('./lib/history') +const Label = require('./lib/label') +const AddFileOperation = require('./lib/operation/add_file_operation') +const MoveFileOperation = require('./lib/operation/move_file_operation') +const EditFileOperation = require('./lib/operation/edit_file_operation') +const EditNoOperation = require('./lib/operation/edit_no_operation') +const SetFileMetadataOperation = require('./lib/operation/set_file_metadata_operation') +const NoOperation = require('./lib/operation/no_operation') +const Operation = require('./lib/operation') +const RestoreOrigin = require('./lib/origin/restore_origin') +const Origin = require('./lib/origin') +const OtClient = require('./lib/ot_client') +const TextOperation = require('./lib/operation/text_operation') +const EditOperation = require('./lib/operation/edit_operation') +const safePathname = require('./lib/safe_pathname') +const Snapshot = require('./lib/snapshot') +const util = require('./lib/util') +const V2DocVersions = require('./lib/v2_doc_versions') + +exports.AddCommentOperation = AddCommentOperation +exports.Author = Author +exports.AuthorList = AuthorList +exports.Blob = Blob +exports.Change = Change +exports.ChangeRequest = ChangeRequest +exports.ChangeNote = ChangeNote +exports.Chunk = Chunk +exports.ChunkResponse = ChunkResponse +exports.DeleteCommentOperation = DeleteCommentOperation +exports.File = File +exports.FileMap = FileMap +exports.History = History +exports.Label = Label +exports.AddFileOperation = AddFileOperation +exports.MoveFileOperation = MoveFileOperation +exports.EditFileOperation = EditFileOperation +exports.EditNoOperation = EditNoOperation +exports.SetFileMetadataOperation = SetFileMetadataOperation +exports.NoOperation = NoOperation +exports.Operation = Operation +exports.RestoreOrigin = RestoreOrigin +exports.Origin = Origin +exports.OtClient = OtClient +exports.TextOperation = TextOperation +exports.EditOperation = EditOperation +exports.safePathname = safePathname +exports.Snapshot = Snapshot +exports.util = util +exports.V2DocVersions = V2DocVersions diff --git a/libraries/overleaf-editor-core/lib/operation/index.js b/libraries/overleaf-editor-core/lib/operation/index.js index 916440518e..2826c58a13 100644 --- a/libraries/overleaf-editor-core/lib/operation/index.js +++ b/libraries/overleaf-editor-core/lib/operation/index.js @@ -29,16 +29,16 @@ class Operation { * @return {Operation} one of the subclasses */ static fromRaw(raw) { - if (Object.prototype.hasOwnProperty.call(raw, 'file')) { + if ('file' in raw) { return AddFileOperation.fromRaw(raw) } - if (Object.prototype.hasOwnProperty.call(raw, 'textOperation')) { + if ('textOperation' in raw || 'commentId' in raw) { return EditFileOperation.fromRaw(raw) } - if (Object.prototype.hasOwnProperty.call(raw, 'newPathname')) { + if ('newPathname' in raw) { return new MoveFileOperation(raw.pathname, raw.newPathname) } - if (Object.prototype.hasOwnProperty.call(raw, 'metadata')) { + if ('metadata' in raw) { return new SetFileMetadataOperation(raw.pathname, raw.metadata) } if (_.isEmpty(raw)) { diff --git a/libraries/overleaf-editor-core/test/delete_comment_operation.test.js b/libraries/overleaf-editor-core/test/delete_comment_operation.test.js index be5cc7f7cc..f47fbf78cd 100644 --- a/libraries/overleaf-editor-core/test/delete_comment_operation.test.js +++ b/libraries/overleaf-editor-core/test/delete_comment_operation.test.js @@ -32,9 +32,7 @@ describe('DeleteCommentOperation', function () { const fileData = new StringFileData('abc') const op = new DeleteCommentOperation('123') fileData.comments.add('123', new Comment([new Range(0, 1)])) - const invertedOp = /** @type {InstanceType} */ ( - op.invert(fileData) - ) + const invertedOp = /** @type {AddCommentOperation} */ (op.invert(fileData)) expect(invertedOp).to.be.instanceOf(AddCommentOperation) expect(invertedOp.commentId).to.equal('123') expect(invertedOp.comment).to.be.instanceOf(Comment) diff --git a/services/project-history/app/js/UpdateCompressor.js b/services/project-history/app/js/UpdateCompressor.js index 16f111e7ab..7d801a0cd7 100644 --- a/services/project-history/app/js/UpdateCompressor.js +++ b/services/project-history/app/js/UpdateCompressor.js @@ -36,6 +36,8 @@ const adjustLengthByOp = function (length, op) { return length + op.i.length } else if (op.d != null) { return length - op.d.length + } else if (op.c != null) { + return length } else { throw new OError('unexpected op type') } @@ -63,8 +65,7 @@ export function convertToSingleOpUpdates(updates) { splitUpdates.push(update) continue } - // Reject any non-insert or delete ops, i.e. comments - const ops = update.op.filter(o => o.i != null || o.d != null) + const ops = update.op let { doc_length: docLength } = update.meta for (const op of ops) { const splitUpdate = cloneWithOp(update, op) diff --git a/services/project-history/app/js/UpdateTranslator.js b/services/project-history/app/js/UpdateTranslator.js index 75d54fa082..5f46cd723f 100644 --- a/services/project-history/app/js/UpdateTranslator.js +++ b/services/project-history/app/js/UpdateTranslator.js @@ -1,30 +1,41 @@ +// @ts-check + import _ from 'lodash' import Core from 'overleaf-editor-core' import * as Errors from './Errors.js' import * as OperationsCompressor from './OperationsCompressor.js' -export function convertToChanges(projectId, updatesWithBlobs, callback) { - let changes - try { - // convert update to change - changes = updatesWithBlobs.map(update => - _convertToChange(projectId, update) - ) - } catch (error1) { - const error = error1 - if ( - error instanceof Errors.UpdateWithUnknownFormatError || - error instanceof Errors.UnexpectedOpTypeError - ) { - return callback(error) - } else { - throw error - } - } +/** + * @typedef {import('./types.ts').AddDocUpdate} AddDocUpdate + * @typedef {import('./types.ts').AddFileUpdate} AddFileUpdate + * @typedef {import('./types.ts').CommentOp} CommentOp + * @typedef {import('./types.ts').DeleteOp} DeleteOp + * @typedef {import('./types.ts').InsertOp} InsertOp + * @typedef {import('./types.ts').Op} Op + * @typedef {import('./types.ts').RenameUpdate} RenameUpdate + * @typedef {import('./types.ts').TextUpdate} TextUpdate + * @typedef {import('./types.ts').Update} Update + * @typedef {import('./types.ts').UpdateWithBlob} UpdateWithBlob + */ - callback(null, changes) +/** + * Convert updates into history changes + * + * @param {string} projectId + * @param {UpdateWithBlob[]} updatesWithBlobs + * @returns {Array} + */ +export function convertToChanges(projectId, updatesWithBlobs) { + return updatesWithBlobs.map(update => _convertToChange(projectId, update)) } +/** + * Convert an update into a history change + * + * @param {string} projectId + * @param {UpdateWithBlob} updateWithBlob + * @returns {Core.Change | null} + */ function _convertToChange(projectId, updateWithBlob) { let operations const { update } = updateWithBlob @@ -55,11 +66,12 @@ function _convertToChange(projectId, updateWithBlob) { let pathname = update.meta.pathname pathname = _convertPathname(pathname) - const builder = new TextOperationsBuilder(docLength, pathname) + const builder = new OperationsBuilder(docLength, pathname) // convert ops for (const op of update.op) { + // if this throws an exception it will be caught in convertToChanges builder.addOp(op) - } // if this throws an exception it will be caught in convertToChanges + } operations = builder.finish() // add doc version information if present if (update.v != null) { @@ -96,28 +108,62 @@ function _convertToChange(projectId, updateWithBlob) { } const change = Core.Change.fromRaw(rawChange) - change.operations = OperationsCompressor.compressOperations(change.operations) + if (change != null) { + change.operations = OperationsCompressor.compressOperations( + change.operations + ) + } return change } +/** + * @param {Update} update + * @returns {update is RenameUpdate} + */ function _isRenameUpdate(update) { - return update.new_pathname != null + return 'new_pathname' in update && update.new_pathname != null } +/** + * @param {Update} update + * @returns {update is AddDocUpdate} + */ function _isAddDocUpdate(update) { - return update.doc != null && update.docLines != null + return ( + 'doc' in update && + update.doc != null && + 'docLines' in update && + update.docLines != null + ) } +/** + * @param {Update} update + * @returns {update is AddFileUpdate} + */ function _isAddFileUpdate(update) { - return update.file != null && update.url != null + return ( + 'file' in update && + update.file != null && + 'url' in update && + update.url != null + ) } +/** + * @param {Update} update + * @returns {update is TextUpdate} + */ export function isTextUpdate(update) { return ( + 'doc' in update && update.doc != null && + 'op' in update && update.op != null && + 'pathname' in update.meta && update.meta.pathname != null && + 'doc_length' in update.meta && update.meta.doc_length != null ) } @@ -126,6 +172,10 @@ export function isProjectStructureUpdate(update) { return isAddUpdate(update) || _isRenameUpdate(update) } +/** + * @param {Update} update + * @returns {update is AddDocUpdate | AddFileUpdate} + */ export function isAddUpdate(update) { return _isAddDocUpdate(update) || _isAddFileUpdate(update) } @@ -152,20 +202,57 @@ export function _convertPathname(pathname) { return pathname } -class TextOperationsBuilder { +class OperationsBuilder { + /** + * @param {number} docLength + * @param {string} pathname + */ constructor(docLength, pathname) { + /** + * List of operations being built + */ this.operations = [] - this.currentOperation = [] + + /** + * Currently built text operation + * + * @type {(number | string)[]} + */ + this.textOperation = [] + + /** + * Cursor inside the current text operation + */ this.cursor = 0 + this.docLength = docLength this.pathname = pathname } + /** + * @param {Op} op + * @returns {void} + */ addOp(op) { - if (op.c != null) { - return // ignore comment op + if (isComment(op)) { + // Close the current text operation + this.pushTextOperation() + + // Add a comment operation + this.operations.push({ + pathname: this.pathname, + commentId: op.t, + ranges: [ + { + pos: op.p, + length: op.c.length, + }, + ], + }) + return } - if (op.i == null && op.d == null) { + + if (!isInsert(op) && !isDelete(op)) { throw new Errors.UnexpectedOpTypeError('unexpected op type', { op }) } @@ -175,7 +262,7 @@ class TextOperationsBuilder { const pos = Math.min(op.p, this.docLength) if (pos < this.cursor) { - this.pushCurrentOperation() + this.pushTextOperation() // At this point, this.cursor === 0 and we can continue } @@ -183,47 +270,72 @@ class TextOperationsBuilder { this.retain(pos - this.cursor) } - if (op.i != null) { + if (isInsert(op)) { this.insert(op.i) } - if (op.d != null) { + if (isDelete(op)) { this.delete(op.d.length) } } retain(length) { - this.currentOperation.push(length) + this.textOperation.push(length) this.cursor += length } insert(str) { - this.currentOperation.push(str) + this.textOperation.push(str) this.cursor += str.length this.docLength += str.length } delete(length) { - this.currentOperation.push(-length) + this.textOperation.push(-length) this.docLength -= length } - pushCurrentOperation() { - if (this.cursor < this.docLength) { - this.retain(this.docLength - this.cursor) - } - if (this.currentOperation.length > 0) { + pushTextOperation() { + if (this.textOperation.length > 0) + if (this.cursor < this.docLength) { + this.retain(this.docLength - this.cursor) + } + if (this.textOperation.length > 0) { this.operations.push({ pathname: this.pathname, - textOperation: this.currentOperation, + textOperation: this.textOperation, }) - this.currentOperation = [] + this.textOperation = [] } this.cursor = 0 } finish() { - this.pushCurrentOperation() + this.pushTextOperation() return this.operations } } + +/** + * @param {Op} op + * @returns {op is InsertOp} + */ +function isInsert(op) { + return 'i' in op && op.i != null +} + +/** + * @param {Op} op + * @returns {op is DeleteOp} + */ +function isDelete(op) { + return 'd' in op && op.d != null +} + +/** + * @param {Op} op + * @returns {op is CommentOp} + */ +function isComment(op) { + return 'c' in op && op.c != null && 't' in op && op.t != null +} diff --git a/services/project-history/app/js/UpdatesProcessor.js b/services/project-history/app/js/UpdatesProcessor.js index 06c71de69d..841ceba39b 100644 --- a/services/project-history/app/js/UpdatesProcessor.js +++ b/services/project-history/app/js/UpdatesProcessor.js @@ -408,15 +408,12 @@ function _processUpdates( ) }, (updatesWithBlobs, cb) => { - cb = profile.wrap('convertToChanges', cb) - UpdateTranslator.convertToChanges( + const changes = UpdateTranslator.convertToChanges( projectId, - updatesWithBlobs, - cb - ) - }, - (changes, cb) => { - changes = changes.map(change => change.toRaw()) + updatesWithBlobs + ).map(change => change.toRaw()) + profile.log('convertToChanges') + let change const numChanges = changes.length const byteLength = Buffer.byteLength( diff --git a/services/project-history/app/js/types.ts b/services/project-history/app/js/types.ts new file mode 100644 index 0000000000..e508f3cd35 --- /dev/null +++ b/services/project-history/app/js/types.ts @@ -0,0 +1,69 @@ +export type Update = TextUpdate | AddDocUpdate | AddFileUpdate | RenameUpdate + +export type UpdateMeta = { + user_id: string + ts: string + source?: string + type?: string + origin?: RawOrigin +} + +export type TextUpdate = { + doc: string + op: Op[] + v: number + meta: UpdateMeta & { + pathname: string + doc_length: number + } +} + +type ProjectUpdateBase = { + version: string + projectHistoryId: string + meta: UpdateMeta + doc: string +} + +export type AddDocUpdate = ProjectUpdateBase & { + pathname: string + docLines: string[] +} + +export type AddFileUpdate = ProjectUpdateBase & { + pathname: string + file: string + url: string +} + +export type RenameUpdate = ProjectUpdateBase & { + pathname: string + new_pathname: string +} + +export type Op = InsertOp | DeleteOp | CommentOp + +export type InsertOp = { + i: string + p: number +} + +export type DeleteOp = { + d: string + p: number +} + +export type CommentOp = { + c: string + p: number + t: string +} + +export type UpdateWithBlob = { + update: Update + blobHash: string +} + +export type RawOrigin = { + kind: string +} diff --git a/services/project-history/scripts/debug_translate_updates.js b/services/project-history/scripts/debug_translate_updates.js index cd6b324223..ce75df4a7a 100755 --- a/services/project-history/scripts/debug_translate_updates.js +++ b/services/project-history/scripts/debug_translate_updates.js @@ -36,16 +36,13 @@ function expandResyncProjectStructure(chunk, update) { function expandUpdates(updates) { const wrappedUpdates = updates.map(update => ({ update })) - UpdateTranslator.convertToChanges( - projectId, - wrappedUpdates, - (err, changes) => { - if (err != null) { - error(err) - } - console.log(JSON.stringify(changes, null, 2)) - } - ) + let changes + try { + changes = UpdateTranslator.convertToChanges(projectId, wrappedUpdates) + } catch (err) { + error(err) + } + console.log(JSON.stringify(changes, null, 2)) } if (updates[0].resyncProjectStructure) { diff --git a/services/project-history/test/acceptance/js/SendingUpdatesTests.js b/services/project-history/test/acceptance/js/SendingUpdatesTests.js index 5943096c98..ce1adf4958 100644 --- a/services/project-history/test/acceptance/js/SendingUpdatesTests.js +++ b/services/project-history/test/acceptance/js/SendingUpdatesTests.js @@ -73,19 +73,12 @@ function slRenameUpdate(historyId, doc, userId, ts, pathname, newPathname) { } } -function olTextUpdate(doc, userId, ts, textOperation, v) { +function olUpdate(doc, userId, ts, operations, v) { return { v2Authors: [userId], timestamp: ts.toJSON(), authors: [], - - operations: [ - { - pathname: doc.pathname.replace(/^\//, ''), // Strip leading / - textOperation, - }, - ], - + operations, v2DocVersions: { [doc.id]: { pathname: doc.pathname.replace(/^\//, ''), // Strip leading / @@ -95,28 +88,36 @@ function olTextUpdate(doc, userId, ts, textOperation, v) { } } -function olTextUpdates(doc, userId, ts, textOperations, v) { +function olTextOperation(doc, textOperation) { return { - v2Authors: [userId], - timestamp: ts.toJSON(), - authors: [], - - operations: textOperations.map(textOperation => ({ - // Strip leading / - pathname: doc.pathname.replace(/^\//, ''), - - textOperation, - })), - - v2DocVersions: { - [doc.id]: { - pathname: doc.pathname.replace(/^\//, ''), // Strip leading / - v: v || 1, - }, - }, + pathname: doc.pathname.replace(/^\//, ''), // Strip leading / + textOperation, } } +function olAddCommentOperation(doc, commentId, pos, length) { + return { + pathname: doc.pathname.replace(/^\//, ''), // Strip leading / + commentId, + ranges: [{ pos, length }], + resolved: false, + } +} + +function olTextUpdate(doc, userId, ts, textOperation, v) { + return olUpdate(doc, userId, ts, [olTextOperation(doc, textOperation)], v) +} + +function olTextUpdates(doc, userId, ts, textOperations, v) { + return olUpdate( + doc, + userId, + ts, + textOperations.map(textOperation => olTextOperation(doc, textOperation)), + v + ) +} + function olRenameUpdate(doc, userId, ts, pathname, newPathname) { return { v2Authors: [userId], @@ -594,11 +595,21 @@ describe('Sending Updates', function () { ) }) - it('should ignore comment ops', function (done) { + it('should handle comment ops', function (done) { const createChange = MockHistoryStore() .post(`/api/projects/${historyId}/legacy_changes`, body => { expect(body).to.deep.equal([ - olTextUpdate(this.doc, this.userId, this.timestamp, [3, '\nc', 2]), + olUpdate(this.doc, this.userId, this.timestamp, [ + olTextOperation(this.doc, [3, '\nc', 2]), + olAddCommentOperation(this.doc, 'comment-id-1', 3, 2), + ]), + olUpdate( + this.doc, + this.userId, + this.timestamp, + [olAddCommentOperation(this.doc, 'comment-id-2', 2, 1)], + 2 + ), ]) return true }) @@ -618,7 +629,7 @@ describe('Sending Updates', function () { this.timestamp, [ { p: 3, i: '\nc' }, - { p: 3, c: '\nc' }, + { p: 3, c: '\nc', t: 'comment-id-1' }, ] ), cb @@ -633,7 +644,7 @@ describe('Sending Updates', function () { this.userId, 2, this.timestamp, - [{ p: 2, c: 'b' }] + [{ p: 2, c: 'b', t: 'comment-id-2' }] ), cb ) diff --git a/services/project-history/test/unit/js/UpdateCompressor/UpdateCompressorTests.js b/services/project-history/test/unit/js/UpdateCompressor/UpdateCompressorTests.js index 9e4f94ff69..727d3fd935 100644 --- a/services/project-history/test/unit/js/UpdateCompressor/UpdateCompressorTests.js +++ b/services/project-history/test/unit/js/UpdateCompressor/UpdateCompressorTests.js @@ -65,7 +65,7 @@ describe('UpdateCompressor', function () { ).to.deep.equal([]) }) - it('should ignore comment ops', function () { + it('should not ignore comment ops', function () { expect( this.UpdateCompressor.convertToSingleOpUpdates([ { @@ -74,19 +74,24 @@ describe('UpdateCompressor', function () { (this.op2 = { p: 9, c: 'baz' }), (this.op3 = { p: 6, i: 'bar' }), ], - meta: { ts: this.ts1, user_id: this.user_id }, + meta: { ts: this.ts1, user_id: this.user_id, doc_length: 10 }, v: 42, }, ]) ).to.deep.equal([ { op: this.op1, - meta: { ts: this.ts1, user_id: this.user_id }, + meta: { ts: this.ts1, user_id: this.user_id, doc_length: 10 }, + v: 42, + }, + { + op: this.op2, + meta: { ts: this.ts1, user_id: this.user_id, doc_length: 13 }, v: 42, }, { op: this.op3, - meta: { ts: this.ts1, user_id: this.user_id }, + meta: { ts: this.ts1, user_id: this.user_id, doc_length: 13 }, v: 42, }, ]) diff --git a/services/project-history/test/unit/js/UpdateTranslator/UpdateTranslatorTests.js b/services/project-history/test/unit/js/UpdateTranslator/UpdateTranslatorTests.js index f18211b2d3..2ce7b279ef 100644 --- a/services/project-history/test/unit/js/UpdateTranslator/UpdateTranslatorTests.js +++ b/services/project-history/test/unit/js/UpdateTranslator/UpdateTranslatorTests.js @@ -22,7 +22,7 @@ describe('UpdateTranslator', function () { }) describe('convertToChanges', function () { - it('can translate doc additions', function (done) { + it('can translate doc additions', function () { const updates = [ { update: { @@ -37,35 +37,30 @@ describe('UpdateTranslator', function () { blobHash: this.mockBlobHash, }, ] - const assertion = (error, changes) => { - changes = changes.map(change => change.toRaw()) - expect(error).to.be.null - expect(changes).to.deep.equal([ - { - authors: [], - operations: [ - { - pathname: 'main.tex', - file: { - hash: this.mockBlobHash, - }, - }, - ], - v2Authors: [this.user_id], - timestamp: this.timestamp, - }, - ]) - done() - } - this.UpdateTranslator.convertToChanges( + const changes = this.UpdateTranslator.convertToChanges( this.project_id, - updates, - assertion - ) + updates + ).map(change => change.toRaw()) + + expect(changes).to.deep.equal([ + { + authors: [], + operations: [ + { + pathname: 'main.tex', + file: { + hash: this.mockBlobHash, + }, + }, + ], + v2Authors: [this.user_id], + timestamp: this.timestamp, + }, + ]) }) - it('can translate file additions', function (done) { + it('can translate file additions', function () { const updates = [ { update: { @@ -80,35 +75,30 @@ describe('UpdateTranslator', function () { blobHash: this.mockBlobHash, }, ] - const assertion = (error, changes) => { - changes = changes.map(change => change.toRaw()) - expect(error).to.be.null - expect(changes).to.deep.equal([ - { - authors: [], - operations: [ - { - pathname: 'test.png', - file: { - hash: this.mockBlobHash, - }, - }, - ], - v2Authors: [this.user_id], - timestamp: this.timestamp, - }, - ]) - done() - } - this.UpdateTranslator.convertToChanges( + const changes = this.UpdateTranslator.convertToChanges( this.project_id, - updates, - assertion - ) + updates + ).map(change => change.toRaw()) + + expect(changes).to.deep.equal([ + { + authors: [], + operations: [ + { + pathname: 'test.png', + file: { + hash: this.mockBlobHash, + }, + }, + ], + v2Authors: [this.user_id], + timestamp: this.timestamp, + }, + ]) }) - it('can translate doc renames', function (done) { + it('can translate doc renames', function () { const updates = [ { update: { @@ -122,33 +112,28 @@ describe('UpdateTranslator', function () { }, }, ] - const assertion = (error, changes) => { - changes = changes.map(change => change.toRaw()) - expect(error).to.be.null - expect(changes).to.deep.equal([ - { - authors: [], - operations: [ - { - pathname: 'main.tex', - newPathname: 'new_main.tex', - }, - ], - v2Authors: [this.user_id], - timestamp: this.timestamp, - }, - ]) - done() - } - this.UpdateTranslator.convertToChanges( + const changes = this.UpdateTranslator.convertToChanges( this.project_id, - updates, - assertion - ) + updates + ).map(change => change.toRaw()) + + expect(changes).to.deep.equal([ + { + authors: [], + operations: [ + { + pathname: 'main.tex', + newPathname: 'new_main.tex', + }, + ], + v2Authors: [this.user_id], + timestamp: this.timestamp, + }, + ]) }) - it('can translate file renames', function (done) { + it('can translate file renames', function () { const updates = [ { update: { @@ -162,33 +147,28 @@ describe('UpdateTranslator', function () { }, }, ] - const assertion = (error, changes) => { - changes = changes.map(change => change.toRaw()) - expect(error).to.be.null - expect(changes).to.deep.equal([ - { - authors: [], - operations: [ - { - pathname: 'test.png', - newPathname: 'new_test.png', - }, - ], - v2Authors: [this.user_id], - timestamp: this.timestamp, - }, - ]) - done() - } - this.UpdateTranslator.convertToChanges( + const changes = this.UpdateTranslator.convertToChanges( this.project_id, - updates, - assertion - ) + updates + ).map(change => change.toRaw()) + + expect(changes).to.deep.equal([ + { + authors: [], + operations: [ + { + pathname: 'test.png', + newPathname: 'new_test.png', + }, + ], + v2Authors: [this.user_id], + timestamp: this.timestamp, + }, + ]) }) - it('can translate multiple updates with the correct versions', function (done) { + it('can translate multiple updates with the correct versions', function () { const updates = [ { update: { @@ -215,48 +195,43 @@ describe('UpdateTranslator', function () { blobHash: this.mockBlobHash, }, ] - const assertion = (error, changes) => { - changes = changes.map(change => change.toRaw()) - expect(error).to.be.null - expect(changes).to.deep.equal([ - { - authors: [], - operations: [ - { - pathname: 'main.tex', - file: { - hash: this.mockBlobHash, - }, - }, - ], - v2Authors: [this.user_id], - timestamp: this.timestamp, - }, - { - authors: [], - operations: [ - { - pathname: 'test.png', - file: { - hash: this.mockBlobHash, - }, - }, - ], - v2Authors: [this.user_id], - timestamp: this.timestamp, - }, - ]) - done() - } - this.UpdateTranslator.convertToChanges( + const changes = this.UpdateTranslator.convertToChanges( this.project_id, - updates, - assertion - ) + updates + ).map(change => change.toRaw()) + + expect(changes).to.deep.equal([ + { + authors: [], + operations: [ + { + pathname: 'main.tex', + file: { + hash: this.mockBlobHash, + }, + }, + ], + v2Authors: [this.user_id], + timestamp: this.timestamp, + }, + { + authors: [], + operations: [ + { + pathname: 'test.png', + file: { + hash: this.mockBlobHash, + }, + }, + ], + v2Authors: [this.user_id], + timestamp: this.timestamp, + }, + ]) }) - it('returns an error if the update has an unknown format', function (done) { + it('returns an error if the update has an unknown format', function () { const updates = [ { update: { @@ -264,21 +239,12 @@ describe('UpdateTranslator', function () { }, }, ] - const assertion = (error, changes) => { - expect(error) - .to.exist.and.be.instanceof(Error) - .and.have.property('message', 'update with unknown format') - done() - } - - this.UpdateTranslator.convertToChanges( - this.project_id, - updates, - assertion - ) + expect(() => + this.UpdateTranslator.convertToChanges(this.project_id, updates) + ).to.throw('update with unknown format') }) - it('replaces backslashes with underscores in pathnames', function (done) { + it('replaces backslashes with underscores in pathnames', function () { const updates = [ { update: { @@ -292,33 +258,28 @@ describe('UpdateTranslator', function () { }, }, ] - const assertion = (error, changes) => { - changes = changes.map(change => change.toRaw()) - expect(error).to.be.null - expect(changes).to.deep.equal([ - { - authors: [], - operations: [ - { - pathname: '_main_foo.tex', - newPathname: '_new_main_foo_bar.tex', - }, - ], - v2Authors: [this.user_id], - timestamp: this.timestamp, - }, - ]) - done() - } - this.UpdateTranslator.convertToChanges( + const changes = this.UpdateTranslator.convertToChanges( this.project_id, - updates, - assertion - ) + updates + ).map(change => change.toRaw()) + + expect(changes).to.deep.equal([ + { + authors: [], + operations: [ + { + pathname: '_main_foo.tex', + newPathname: '_new_main_foo_bar.tex', + }, + ], + v2Authors: [this.user_id], + timestamp: this.timestamp, + }, + ]) }) - it('replaces leading asterisks with __ASTERISK__ in pathnames', function (done) { + it('replaces leading asterisks with __ASTERISK__ in pathnames', function () { const updates = [ { update: { @@ -333,35 +294,30 @@ describe('UpdateTranslator', function () { blobHash: this.mockBlobHash, }, ] - const assertion = (error, changes) => { - changes = changes.map(change => change.toRaw()) - expect(error).to.be.null - expect(changes).to.deep.equal([ - { - authors: [], - operations: [ - { - pathname: 'test__ASTERISK__test.png', - file: { - hash: this.mockBlobHash, - }, - }, - ], - v2Authors: [this.user_id], - timestamp: this.timestamp, - }, - ]) - done() - } - this.UpdateTranslator.convertToChanges( + const changes = this.UpdateTranslator.convertToChanges( this.project_id, - updates, - assertion - ) + updates + ).map(change => change.toRaw()) + + expect(changes).to.deep.equal([ + { + authors: [], + operations: [ + { + pathname: 'test__ASTERISK__test.png', + file: { + hash: this.mockBlobHash, + }, + }, + ], + v2Authors: [this.user_id], + timestamp: this.timestamp, + }, + ]) }) - it('replaces a leading space for top-level files with __SPACE__', function (done) { + it('replaces a leading space for top-level files with __SPACE__', function () { const updates = [ { update: { @@ -376,35 +332,30 @@ describe('UpdateTranslator', function () { blobHash: this.mockBlobHash, }, ] - const assertion = (error, changes) => { - changes = changes.map(change => change.toRaw()) - expect(error).to.be.null - expect(changes).to.deep.equal([ - { - authors: [], - operations: [ - { - pathname: '__SPACE__test.png', - file: { - hash: this.mockBlobHash, - }, - }, - ], - v2Authors: [this.user_id], - timestamp: this.timestamp, - }, - ]) - done() - } - this.UpdateTranslator.convertToChanges( + const changes = this.UpdateTranslator.convertToChanges( this.project_id, - updates, - assertion - ) + updates + ).map(change => change.toRaw()) + + expect(changes).to.deep.equal([ + { + authors: [], + operations: [ + { + pathname: '__SPACE__test.png', + file: { + hash: this.mockBlobHash, + }, + }, + ], + v2Authors: [this.user_id], + timestamp: this.timestamp, + }, + ]) }) - it('replaces leading spaces of files in subfolders with __SPACE__', function (done) { + it('replaces leading spaces of files in subfolders with __SPACE__', function () { const updates = [ { update: { @@ -419,35 +370,30 @@ describe('UpdateTranslator', function () { blobHash: this.mockBlobHash, }, ] - const assertion = (error, changes) => { - changes = changes.map(change => change.toRaw()) - expect(error).to.be.null - expect(changes).to.deep.equal([ - { - authors: [], - operations: [ - { - pathname: 'folder/__SPACE__test.png', - file: { - hash: this.mockBlobHash, - }, - }, - ], - v2Authors: [this.user_id], - timestamp: this.timestamp, - }, - ]) - done() - } - this.UpdateTranslator.convertToChanges( + const changes = this.UpdateTranslator.convertToChanges( this.project_id, - updates, - assertion - ) + updates + ).map(change => change.toRaw()) + + expect(changes).to.deep.equal([ + { + authors: [], + operations: [ + { + pathname: 'folder/__SPACE__test.png', + file: { + hash: this.mockBlobHash, + }, + }, + ], + v2Authors: [this.user_id], + timestamp: this.timestamp, + }, + ]) }) - it('sets a null author when user_id is "anonymous-user"', function (done) { + it('sets a null author when user_id is "anonymous-user"', function () { const updates = [ { update: { @@ -462,35 +408,30 @@ describe('UpdateTranslator', function () { blobHash: this.mockBlobHash, }, ] - const assertion = (error, changes) => { - changes = changes.map(change => change.toRaw()) - expect(error).to.be.null - expect(changes).to.deep.equal([ - { - authors: [], - operations: [ - { - pathname: 'main.tex', - file: { - hash: this.mockBlobHash, - }, - }, - ], - v2Authors: [null], - timestamp: this.timestamp, - }, - ]) - done() - } - this.UpdateTranslator.convertToChanges( + const changes = this.UpdateTranslator.convertToChanges( this.project_id, - updates, - assertion - ) + updates + ).map(change => change.toRaw()) + + expect(changes).to.deep.equal([ + { + authors: [], + operations: [ + { + pathname: 'main.tex', + file: { + hash: this.mockBlobHash, + }, + }, + ], + v2Authors: [null], + timestamp: this.timestamp, + }, + ]) }) - it('sets an empty array as author when there is no meta.user_id', function (done) { + it('sets an empty array as author when there is no meta.user_id', function () { const updates = [ { update: { @@ -504,36 +445,31 @@ describe('UpdateTranslator', function () { blobHash: this.mockBlobHash, }, ] - const assertion = (error, changes) => { - changes = changes.map(change => change.toRaw()) - expect(error).to.be.null - expect(changes).to.deep.equal([ - { - authors: [], - operations: [ - { - pathname: 'main.tex', - file: { - hash: this.mockBlobHash, - }, - }, - ], - v2Authors: [], - timestamp: this.timestamp, - }, - ]) - done() - } - this.UpdateTranslator.convertToChanges( + const changes = this.UpdateTranslator.convertToChanges( this.project_id, - updates, - assertion - ) + updates + ).map(change => change.toRaw()) + + expect(changes).to.deep.equal([ + { + authors: [], + operations: [ + { + pathname: 'main.tex', + file: { + hash: this.mockBlobHash, + }, + }, + ], + v2Authors: [], + timestamp: this.timestamp, + }, + ]) }) describe('text updates', function () { - it('can translate insertions', function (done) { + it('can translate insertions', function () { const updates = [ { update: { @@ -553,39 +489,34 @@ describe('UpdateTranslator', function () { }, }, ] - const assertion = (error, changes) => { - changes = changes.map(change => change.toRaw()) - expect(error).to.be.null - expect(changes).to.deep.equal([ - { - authors: [], - operations: [ - { - pathname: 'main.tex', - textOperation: [3, 'foo', 9, 'bar', 8], - }, - ], - v2Authors: [this.user_id], - timestamp: this.timestamp, - v2DocVersions: { - '59bfd450e3028c4d40a1e9ab': { - pathname: 'main.tex', - v: 0, - }, + + const changes = this.UpdateTranslator.convertToChanges( + this.project_id, + updates + ).map(change => change.toRaw()) + + expect(changes).to.deep.equal([ + { + authors: [], + operations: [ + { + pathname: 'main.tex', + textOperation: [3, 'foo', 9, 'bar', 8], + }, + ], + v2Authors: [this.user_id], + timestamp: this.timestamp, + v2DocVersions: { + '59bfd450e3028c4d40a1e9ab': { + pathname: 'main.tex', + v: 0, }, }, - ]) - done() - } - - this.UpdateTranslator.convertToChanges( - this.project_id, - updates, - assertion - ) + }, + ]) }) - it('can translate deletions', function (done) { + it('can translate deletions', function () { const updates = [ { update: { @@ -604,39 +535,34 @@ describe('UpdateTranslator', function () { }, }, ] - const assertion = (error, changes) => { - changes = changes.map(change => change.toRaw()) - expect(error).to.be.null - expect(changes).to.deep.equal([ - { - authors: [], - operations: [ - { - pathname: 'main.tex', - textOperation: [3, -2, 7, -3, 5], - }, - ], - v2Authors: [this.user_id], - timestamp: this.timestamp, - v2DocVersions: { - '59bfd450e3028c4d40a1e9ab': { - pathname: 'main.tex', - v: 0, - }, + + const changes = this.UpdateTranslator.convertToChanges( + this.project_id, + updates + ).map(change => change.toRaw()) + + expect(changes).to.deep.equal([ + { + authors: [], + operations: [ + { + pathname: 'main.tex', + textOperation: [3, -2, 7, -3, 5], + }, + ], + v2Authors: [this.user_id], + timestamp: this.timestamp, + v2DocVersions: { + '59bfd450e3028c4d40a1e9ab': { + pathname: 'main.tex', + v: 0, }, }, - ]) - done() - } - - this.UpdateTranslator.convertToChanges( - this.project_id, - updates, - assertion - ) + }, + ]) }) - it('can translate insertions at the start and end (with zero retained)', function (done) { + it('can translate insertions at the start and end (with zero retained)', function () { const updates = [ { update: { @@ -657,39 +583,34 @@ describe('UpdateTranslator', function () { }, }, ] - const assertion = (error, changes) => { - changes = changes.map(change => change.toRaw()) - expect(error).to.be.null - expect(changes).to.deep.equal([ - { - authors: [], - operations: [ - { - pathname: 'main.tex', - textOperation: [20], - }, - ], - v2Authors: [this.user_id], - timestamp: this.timestamp, - v2DocVersions: { - '59bfd450e3028c4d40a1e9ab': { - pathname: 'main.tex', - v: 0, - }, + + const changes = this.UpdateTranslator.convertToChanges( + this.project_id, + updates + ).map(change => change.toRaw()) + + expect(changes).to.deep.equal([ + { + authors: [], + operations: [ + { + pathname: 'main.tex', + textOperation: [20], + }, + ], + v2Authors: [this.user_id], + timestamp: this.timestamp, + v2DocVersions: { + '59bfd450e3028c4d40a1e9ab': { + pathname: 'main.tex', + v: 0, }, }, - ]) - done() - } - - this.UpdateTranslator.convertToChanges( - this.project_id, - updates, - assertion - ) + }, + ]) }) - it('can handle operations in non-linear offset order', function (done) { + it('can handle operations in non-linear offset order', function () { const updates = [ { update: { @@ -708,46 +629,43 @@ describe('UpdateTranslator', function () { }, }, ] - const assertion = (error, changes) => { - changes = changes.map(change => change.toRaw()) - expect(error).to.be.null - expect(changes).to.deep.equal([ - { - authors: [], - operations: [ - { - pathname: 'main.tex', - textOperation: [3, 'bar', 12, 'foo', 5], - }, - ], - v2Authors: [this.user_id], - timestamp: this.timestamp, - v2DocVersions: { - '59bfd450e3028c4d40a1e9ab': { - pathname: 'main.tex', - v: 0, - }, + + const changes = this.UpdateTranslator.convertToChanges( + this.project_id, + updates + ).map(change => change.toRaw()) + + expect(changes).to.deep.equal([ + { + authors: [], + operations: [ + { + pathname: 'main.tex', + textOperation: [3, 'bar', 12, 'foo', 5], + }, + ], + v2Authors: [this.user_id], + timestamp: this.timestamp, + v2DocVersions: { + '59bfd450e3028c4d40a1e9ab': { + pathname: 'main.tex', + v: 0, }, }, - ]) - done() - } - - this.UpdateTranslator.convertToChanges( - this.project_id, - updates, - assertion - ) + }, + ]) }) - it('can ignore comment ops', function (done) { + it('handles comment ops', function () { const updates = [ { update: { doc: this.doc_id, op: [ { p: 0, i: 'foo' }, - { p: 5, c: 'bar' }, + { p: 3, d: 'bar' }, + { p: 5, c: 'comment this', t: 'comment-id-1' }, + { p: 7, c: 'another comment', t: 'comment-id-2' }, { p: 10, i: 'baz' }, ], v: this.version, @@ -760,39 +678,50 @@ describe('UpdateTranslator', function () { }, }, ] - const assertion = (error, changes) => { - changes = changes.map(change => change.toRaw()) - expect(error).to.be.null - expect(changes).to.deep.equal([ - { - authors: [], - operations: [ - { - pathname: 'main.tex', - textOperation: ['foo', 7, 'baz', 13], - }, - ], - v2Authors: [this.user_id], - timestamp: this.timestamp, - v2DocVersions: { - '59bfd450e3028c4d40a1e9ab': { - pathname: 'main.tex', - v: 0, - }, + + const changes = this.UpdateTranslator.convertToChanges( + this.project_id, + updates + ).map(change => change.toRaw()) + + expect(changes).to.deep.equal([ + { + authors: [], + operations: [ + { + pathname: 'main.tex', + textOperation: ['foo', -3, 17], + }, + { + pathname: 'main.tex', + commentId: 'comment-id-1', + ranges: [{ pos: 5, length: 12 }], + resolved: false, + }, + { + pathname: 'main.tex', + commentId: 'comment-id-2', + ranges: [{ pos: 7, length: 15 }], + resolved: false, + }, + { + pathname: 'main.tex', + textOperation: [10, 'baz', 10], + }, + ], + v2Authors: [this.user_id], + timestamp: this.timestamp, + v2DocVersions: { + '59bfd450e3028c4d40a1e9ab': { + pathname: 'main.tex', + v: 0, }, }, - ]) - done() - } - - this.UpdateTranslator.convertToChanges( - this.project_id, - updates, - assertion - ) + }, + ]) }) - it('handles insertions after the end of the document', function (done) { + it('handles insertions after the end of the document', function () { const updates = [ { update: { @@ -808,39 +737,34 @@ describe('UpdateTranslator', function () { }, }, ] - const assertion = (error, changes) => { - changes = changes.map(change => change.toRaw()) - expect(error).to.be.null - expect(changes).to.deep.equal([ - { - authors: [], - operations: [ - { - pathname: 'main.tex', - textOperation: [2, '\\'], - }, - ], - v2Authors: [this.user_id], - timestamp: this.timestamp, - v2DocVersions: { - '59bfd450e3028c4d40a1e9ab': { - pathname: 'main.tex', - v: 0, - }, + + const changes = this.UpdateTranslator.convertToChanges( + this.project_id, + updates + ).map(change => change.toRaw()) + + expect(changes).to.deep.equal([ + { + authors: [], + operations: [ + { + pathname: 'main.tex', + textOperation: [2, '\\'], + }, + ], + v2Authors: [this.user_id], + timestamp: this.timestamp, + v2DocVersions: { + '59bfd450e3028c4d40a1e9ab': { + pathname: 'main.tex', + v: 0, }, }, - ]) - done() - } - - this.UpdateTranslator.convertToChanges( - this.project_id, - updates, - assertion - ) + }, + ]) }) - it('translates external source metadata into an origin', function (done) { + it('translates external source metadata into an origin', function () { const updates = [ { update: { @@ -858,40 +782,35 @@ describe('UpdateTranslator', function () { }, }, ] - const assertion = (error, changes) => { - changes = changes.map(change => change.toRaw()) - expect(error).to.be.null - expect(changes).to.deep.equal([ - { - authors: [], - operations: [ - { - pathname: 'main.tex', - textOperation: [3, 'foo', 17], - }, - ], - v2Authors: [this.user_id], - timestamp: this.timestamp, - v2DocVersions: { - '59bfd450e3028c4d40a1e9ab': { - pathname: 'main.tex', - v: 0, - }, - }, - origin: { kind: 'dropbox' }, - }, - ]) - done() - } - this.UpdateTranslator.convertToChanges( + const changes = this.UpdateTranslator.convertToChanges( this.project_id, - updates, - assertion - ) + updates + ).map(change => change.toRaw()) + + expect(changes).to.deep.equal([ + { + authors: [], + operations: [ + { + pathname: 'main.tex', + textOperation: [3, 'foo', 17], + }, + ], + v2Authors: [this.user_id], + timestamp: this.timestamp, + v2DocVersions: { + '59bfd450e3028c4d40a1e9ab': { + pathname: 'main.tex', + v: 0, + }, + }, + origin: { kind: 'dropbox' }, + }, + ]) }) - it('errors on unexpected ops', function (done) { + it('errors on unexpected ops', function () { const updates = [ { update: { @@ -907,16 +826,9 @@ describe('UpdateTranslator', function () { }, }, ] - const assertion = (error, changes) => { - expect(error.message).to.equal('unexpected op type') - done() - } - - this.UpdateTranslator.convertToChanges( - this.project_id, - updates, - assertion - ) + expect(() => { + this.UpdateTranslator.convertToChanges(this.project_id, updates) + }).to.throw('unexpected op type') }) }) }) diff --git a/services/project-history/test/unit/js/UpdatesManager/UpdatesProcessorTests.js b/services/project-history/test/unit/js/UpdatesManager/UpdatesProcessorTests.js index 04c24f8322..5118124f69 100644 --- a/services/project-history/test/unit/js/UpdatesManager/UpdatesProcessorTests.js +++ b/services/project-history/test/unit/js/UpdatesManager/UpdatesProcessorTests.js @@ -279,7 +279,7 @@ describe('UpdatesProcessor', function () { this.SyncManager.expandSyncUpdates.yields(null, this.expandedUpdates) this.UpdateCompressor.compressRawUpdates.returns(this.compressedUpdates) this.BlobManager.createBlobForUpdates.yields(null, this.updatesWithBlobs) - this.UpdateTranslator.convertToChanges.yields(null, this.changes) + this.UpdateTranslator.convertToChanges.returns(this.changes) this.UpdatesProcessor._processUpdates( this.project_id,