From 9357cba87074dc7c06cea904a224baa6210fb8d8 Mon Sep 17 00:00:00 2001 From: e11sy <130844513+e11sy@users.noreply.github.com> Date: Fri, 11 Apr 2025 03:09:48 +0200 Subject: [PATCH 01/15] imp(): implement all operations transformations --- .../src/CollaborationManager.spec.ts | 120 ++++++ .../src/CollaborationManager.ts | 88 +++-- .../src/Operation.spec.ts | 38 +- .../collaboration-manager/src/Operation.ts | 214 ++++++++++- .../src/OperationsBatch.spec.ts | 363 +++++++----------- .../src/OperationsBatch.ts | 145 +++---- .../src/UndoRedoManager.spec.ts | 150 ++++++++ .../src/UndoRedoManager.ts | 24 ++ packages/playground/src/App.vue | 3 +- packages/ui/src/Blocks/Blocks.ts | 4 + 10 files changed, 795 insertions(+), 354 deletions(-) diff --git a/packages/collaboration-manager/src/CollaborationManager.spec.ts b/packages/collaboration-manager/src/CollaborationManager.spec.ts index 81404897..e894976d 100644 --- a/packages/collaboration-manager/src/CollaborationManager.spec.ts +++ b/packages/collaboration-manager/src/CollaborationManager.spec.ts @@ -945,4 +945,124 @@ describe('CollaborationManager', () => { properties: {}, }); }); + + describe('remote operations', () => { + it('should transform current batch when remote operation arrives', () => { + const model = new EditorJSModel(userId, { identifier: documentId }); + model.initializeDocument({ + blocks: [{ + name: 'paragraph', + data: { + text: { + value: '', + $t: 't', + }, + }, + }], + }); + + const collaborationManager = new CollaborationManager(config as Required, model); + + // Create local operation + const localIndex = new IndexBuilder().addBlockIndex(0) + .addDataKey(createDataKey('text')) + .addTextRange([0, 4]) + .build(); + + const localOp = new Operation(OperationType.Insert, localIndex, { + payload: 'test', + }, userId); + + collaborationManager.applyOperation(localOp); + + // Apply remote operation + const remoteIndex = new IndexBuilder().addBlockIndex(0) + .addDataKey(createDataKey('text')) + .addTextRange([0, 5]) + .build(); + + const remoteOp = new Operation(OperationType.Insert, remoteIndex, { + payload: 'hello', + }, 'other-user'); + + collaborationManager.applyOperation(remoteOp); + + // Verify the operations were transformed correctly + expect(model.serialized).toStrictEqual({ + identifier: documentId, + blocks: [{ + name: 'paragraph', + tunes: {}, + data: { + text: { + $t: 't', + value: 'hellotest', + fragments: [], + }, + }, + }], + properties: {}, + }); + }); + + it('should clear current batch if not transformable with remote operation', () => { + const model = new EditorJSModel(userId, { identifier: documentId }); + model.initializeDocument({ + blocks: [{ + name: 'paragraph', + data: { + text: { + value: 'initial', + $t: 't', + }, + }, + }], + }); + + const collaborationManager = new CollaborationManager(config as Required, model); + + // Create local delete operation + const localIndex = new IndexBuilder().addBlockIndex(0) + .addDataKey(createDataKey('text')) + .addTextRange([0, 7]) + .build(); + + const localOp = new Operation(OperationType.Insert, localIndex, { + payload: 'initial', + }, userId); + + collaborationManager.applyOperation(localOp); + + // Apply conflicting remote operation + const remoteIndex = new IndexBuilder().addBlockIndex(0) + .addDataKey(createDataKey('text')) + .addTextRange([0, 7]) + .build(); + + const remoteOp = new Operation(OperationType.Delete, remoteIndex, { + payload: 'initial', + }, 'other-user'); + + collaborationManager.applyOperation(remoteOp); + + // Verify the current batch was cleared by checking undo doesn't restore text + collaborationManager.undo(); + + expect(model.serialized).toStrictEqual({ + identifier: documentId, + blocks: [{ + name: 'paragraph', + tunes: {}, + data: { + text: { + $t: 't', + value: '', + fragments: [], + }, + }, + }], + properties: {}, + }); + }); + }); }); diff --git a/packages/collaboration-manager/src/CollaborationManager.ts b/packages/collaboration-manager/src/CollaborationManager.ts index 7cc8f570..6acd1895 100644 --- a/packages/collaboration-manager/src/CollaborationManager.ts +++ b/packages/collaboration-manager/src/CollaborationManager.ts @@ -36,7 +36,7 @@ export class CollaborationManager { /** * Current operations batch */ - #currentBatch: OperationsBatch | null = null; + #currentBatch: OperationsBatch | null = null; /** * Editor's config @@ -91,7 +91,11 @@ export class CollaborationManager { * Undo last operation in the local stack */ public undo(): void { - this.#currentBatch?.terminate(); + if (this.#currentBatch !== null) { + this.#undoRedoManager.put(this.#currentBatch); + + this.#currentBatch = null; + } const operation = this.#undoRedoManager.undo(); @@ -99,10 +103,16 @@ export class CollaborationManager { return; } - // Disable event handling + // Disable handling this.#shouldHandleEvents = false; - this.applyOperation(operation); + if (operation instanceof OperationsBatch) { + operation.operations.forEach((op) => { + this.applyOperation(op); + }); + } else { + this.applyOperation(operation); + } // Re-enable event handling this.#shouldHandleEvents = true; @@ -112,7 +122,11 @@ export class CollaborationManager { * Redo last undone operation in the local stack */ public redo(): void { - this.#currentBatch?.terminate(); + if (this.#currentBatch !== null) { + this.#undoRedoManager.put(this.#currentBatch); + + this.#currentBatch = null; + } const operation = this.#undoRedoManager.redo(); @@ -123,7 +137,13 @@ export class CollaborationManager { // Disable event handling this.#shouldHandleEvents = false; - this.applyOperation(operation); + if (operation instanceof OperationsBatch) { + operation.operations.forEach((op) => { + this.applyOperation(op); + }); + } else { + this.applyOperation(operation); + } // Re-enable event handling this.#shouldHandleEvents = true; @@ -135,6 +155,10 @@ export class CollaborationManager { * @param operation - operation to apply */ public applyOperation(operation: Operation): void { + if (operation.type === OperationType.Neutral) { + return; + } + switch (operation.type) { case OperationType.Insert: this.#model.insertData(operation.userId, operation.index, operation.data.payload as string | BlockNodeSerialized[]); @@ -159,13 +183,8 @@ export class CollaborationManager { * @param e - event to handle */ #handleEvent(e: ModelEvents): void { - if (!this.#shouldHandleEvents) { - return; - } - let operation: Operation | null = null; - /** * @todo add all model events */ @@ -212,30 +231,47 @@ export class CollaborationManager { return; } + /** + * If operation is local, send it to the server + */ if (operation.userId === this.#config.userId) { void this.#client?.send(operation); } else { + /** + * If operation is remote, transform undo/redo stacks + */ + this.#undoRedoManager.transformUndoStack(operation); + this.#undoRedoManager.transformRedoStack(operation); + + /** + * If we got a new remote operation - transform current batch + * If batch is not transormable - clear it + */ + this.#currentBatch = this.#currentBatch?.transform(operation) ?? null; + return; } - const onBatchTermination = (batch: OperationsBatch, lastOp?: Operation): void => { - const effectiveOp = batch.getEffectiveOperation(); + if (!this.#shouldHandleEvents) { + return; + } - if (effectiveOp) { - this.#undoRedoManager.put(effectiveOp); - } + /** + * If there is no current batch, create a new one with current operation + */ + if (this.#currentBatch === null) { + this.#currentBatch = new OperationsBatch(operation); - /** - * lastOp is the operation on which the batch was terminated. - * So if there is one, we need to create a new batch - * - * lastOp could be null if the batch was terminated by time out - */ - this.#currentBatch = lastOp === undefined ? null : new OperationsBatch(onBatchTermination, lastOp); - }; + return; + } - if (this.#currentBatch === null) { - this.#currentBatch = new OperationsBatch(onBatchTermination, operation); + /** + * If current operation could not be added to the batch, then terminate current batch and create a new one with current operation + */ + if (!this.#currentBatch.canAdd(operation)) { + this.#undoRedoManager.put(this.#currentBatch); + + this.#currentBatch = new OperationsBatch(operation); return; } diff --git a/packages/collaboration-manager/src/Operation.spec.ts b/packages/collaboration-manager/src/Operation.spec.ts index f1f2455f..3c076c34 100644 --- a/packages/collaboration-manager/src/Operation.spec.ts +++ b/packages/collaboration-manager/src/Operation.spec.ts @@ -103,7 +103,7 @@ describe('Operation', () => { const localOp = createOperation(OperationType.Insert, 0, 'abc'); const transformedOp = receivedOp.transform(localOp); - expect(transformedOp.index.textRange).toEqual([6, 6]); + expect(transformedOp?.index.textRange).toEqual([6, 6]); }); it('should transform a received operation if it is at the same position as a local one', () => { @@ -111,7 +111,7 @@ describe('Operation', () => { const localOp = createOperation(OperationType.Insert, 0, 'def'); const transformedOp = receivedOp.transform(localOp); - expect(transformedOp.index.textRange).toEqual([3, 3]); + expect(transformedOp?.index.textRange).toEqual([3, 3]); }); it('should not change the text index if local op is a Block operation', () => { @@ -122,7 +122,7 @@ describe('Operation', () => { } ]); const transformedOp = receivedOp.transform(localOp); - expect(transformedOp.index.textRange).toEqual([0, 0]); + expect(transformedOp?.index.textRange).toEqual([0, 0]); }); it('should not change the operation if local op is a Block operation after a received one', () => { @@ -152,7 +152,7 @@ describe('Operation', () => { const transformedOp = receivedOp.transform(localOp); - expect(transformedOp.index.blockIndex).toEqual(2); + expect(transformedOp?.index.blockIndex).toEqual(2); }); it('should adjust the block index if local op is a Block operation at the same index as a received one', () => { @@ -167,7 +167,7 @@ describe('Operation', () => { const transformedOp = receivedOp.transform(localOp); - expect(transformedOp.index.blockIndex).toEqual(1); + expect(transformedOp?.index.blockIndex).toEqual(1); }); }); @@ -185,7 +185,7 @@ describe('Operation', () => { const localOp = createOperation(OperationType.Delete, 0, 'abc'); const transformedOp = receivedOp.transform(localOp); - expect(transformedOp.index.textRange).toEqual([0, 0]); + expect(transformedOp?.index.textRange).toEqual([0, 0]); }); it('should transform a received operation if it is at the same position as a local one', () => { @@ -193,7 +193,7 @@ describe('Operation', () => { const localOp = createOperation(OperationType.Delete, 3, 'def'); const transformedOp = receivedOp.transform(localOp); - expect(transformedOp.index.textRange).toEqual([0, 0]); + expect(transformedOp?.index.textRange).toEqual([0, 0]); }); it('should not change the text index if local op is a Block operation', () => { @@ -204,7 +204,7 @@ describe('Operation', () => { } ]); const transformedOp = receivedOp.transform(localOp); - expect(transformedOp.index.textRange).toEqual([1, 1]); + expect(transformedOp?.index.textRange).toEqual([1, 1]); }); it('should not change the text index if local op is a Block operation', () => { @@ -215,7 +215,7 @@ describe('Operation', () => { } ]); const transformedOp = receivedOp.transform(localOp); - expect(transformedOp.index.textRange).toEqual([0, 0]); + expect(transformedOp?.index.textRange).toEqual([0, 0]); }); it('should not change the operation if local op is a Block operation after a received one', () => { @@ -245,7 +245,7 @@ describe('Operation', () => { const transformedOp = receivedOp.transform(localOp); - expect(transformedOp.index.blockIndex).toEqual(0); + expect(transformedOp?.index.blockIndex).toEqual(0); }); it('should adjust the block index if local op is a Block operation at the same index as a received one', () => { @@ -260,7 +260,23 @@ describe('Operation', () => { const transformedOp = receivedOp.transform(localOp); - expect(transformedOp.index.blockIndex).toEqual(0); + expect(transformedOp?.index.blockIndex).toEqual(0); + }); + + it('should return null if the transformed text range becomes negative', () => { + const receivedOp = createOperation(OperationType.Delete, 1, 'abc'); + const localOp = createOperation(OperationType.Delete, 0, 'defghijk'); // Delete more characters than available + const transformedOp = receivedOp.transform(localOp); + + expect(transformedOp).toBeNull(); + }); + + it('should return null if the transformed text range becomes negative for Insert operation', () => { + const receivedOp = createOperation(OperationType.Insert, 1, 'abc'); + const localOp = createOperation(OperationType.Delete, 0, 'defghijk'); // Delete more characters than available + const transformedOp = receivedOp.transform(localOp); + + expect(transformedOp).toBeNull(); }); }); }); diff --git a/packages/collaboration-manager/src/Operation.ts b/packages/collaboration-manager/src/Operation.ts index 45c939b8..af092519 100644 --- a/packages/collaboration-manager/src/Operation.ts +++ b/packages/collaboration-manager/src/Operation.ts @@ -6,7 +6,8 @@ import { IndexBuilder, type Index, type BlockNodeSerialized } from '@editorjs/mo export enum OperationType { Insert = 'insert', Delete = 'delete', - Modify = 'modify' + Modify = 'modify', + Neutral = 'neutral', } /** @@ -38,6 +39,13 @@ export interface ModifyOperationData = Record { */ export type OperationTypeToData = T extends OperationType.Modify ? ModifyOperationData - : InsertOrDeleteOperationData; + : T extends OperationType.Neutral + ? NeutralOperationData + : InsertOrDeleteOperationData; /** * Helper type to get invert operation type @@ -82,7 +92,9 @@ export type InvertedOperationType = T extends Operation ? OperationType.Delete : T extends OperationType.Delete ? OperationType.Insert - : OperationType.Modify; + : T extends OperationType.Neutral + ? OperationType.Neutral + : OperationType.Modify; /** @@ -92,7 +104,7 @@ export class Operation { /** * Operation type */ - public type: T; + public type: T | OperationType.Neutral; /** * Index in the document model tree @@ -123,10 +135,10 @@ export class Operation { * @param userId - user identifier * @param rev - document revision */ - constructor(type: T, index: Index, data: OperationTypeToData, userId: string | number, rev?: number) { + constructor(type: T | OperationType.Neutral, index: Index, data: OperationTypeToData | OperationTypeToData, userId: string | number, rev?: number) { this.type = type; this.index = index; - this.data = data; + this.data = data as OperationTypeToData; this.userId = userId; this.rev = rev; } @@ -136,7 +148,7 @@ export class Operation { * * @param op - operation to copy */ - public static from(op: Operation): Operation; + public static from(op: Operation | Operation): Operation; /** * Creates an operation from another operation or serialized operation * @@ -198,10 +210,11 @@ export class Operation { /** * Transforms the operation against another operation + * If operation is not transformable returns null * * @param againstOp - operation to transform against */ - public transform(againstOp: Operation): Operation { + public transform(againstOp: Operation | Operation): Operation { /** * Do not transform operations if they are on different documents */ @@ -218,37 +231,194 @@ export class Operation { const newIndexBuilder = new IndexBuilder().from(this.index); - switch (againstOp.type) { - case OperationType.Insert: { - const payload = (againstOp as Operation).data.payload; + switch (true) { + /** + * If one of the operations is neutral, return this operation + */ + case (this.type === OperationType.Neutral || againstOp.type === OperationType.Neutral): { + return Operation.from(this); + } + /** + * Every operation against modify operation stays the same + */ + case (againstOp.type === OperationType.Modify): { + break; + } + case (this.type === OperationType.Insert && againstOp.type === OperationType.Insert): { + /** + * Update block index if againstOp is insert of a block + */ + if (againstOp.index.isBlockIndex) { + newIndexBuilder.addBlockIndex(this.index.blockIndex!++); + + break; + } + + /** + * Move current insert to the right on amount on chars, inserted by againstOp + */ + if (this.index.isTextIndex && againstOp.index.isTextIndex) { + const againstOpLength = againstOp.data.payload!.length; + + newIndexBuilder.addTextRange([this.index.textRange![0] + againstOpLength, this.index.textRange![1] + againstOpLength]) + + break; + } + } + case (this.type === OperationType.Insert && againstOp.type === OperationType.Delete): { + /** + * Decrease block index if againstOp is Delete block before current insert + */ + if (againstOp.index.isBlockIndex && this.index.blockIndex! > againstOp.index.blockIndex!) { + newIndexBuilder.addBlockIndex(this.index.blockIndex!--); + + break; + } + + if (this.index.isTextIndex && againstOp.index.isTextIndex) { + /** + * Deleted the range on the left of the current insert + */ + if (this.index.textRange![0] > againstOp.index.textRange![1]) { + newIndexBuilder.addTextRange([this.index.textRange![0] - againstOp.index.textRange![1], this.index.textRange![1] - againstOp.index.textRange![1]]); + + break; + } + + /** + * Deleted the range, then trying to insert new text inside of the deleted range + * Then insert should be done in the start of the deleted range + */ + if ((this.index.textRange![0] <= againstOp.index.textRange![0]) && (this.index.textRange![0] > againstOp.index.textRange![0])) { + newIndexBuilder.addTextRange([againstOp.index.textRange![0], againstOp.index.textRange![0]]); + } + } + break; + } + case (this.type === OperationType.Modify && againstOp.type === OperationType.Insert): { + /** + * Increase block index of the modify operation if againstOp insert a block before + */ if (againstOp.index.isBlockIndex) { - newIndexBuilder.addBlockIndex(this.index.blockIndex! + payload.length); + newIndexBuilder.addBlockIndex(this.index.blockIndex!++); break; } - newIndexBuilder.addTextRange([this.index.textRange![0] + payload.length, this.index.textRange![1] + payload.length]); + /** + * Extend modify operation range if againstOp insert a text inside of the modify bounds + */ + if (againstOp.index.textRange![0] < this.index.textRange![0] && againstOp.index.textRange![1] > this.index.textRange![0]) { + const againstOpLength = againstOp.index.textRange![1] - againstOp.index.textRange![0]; + newIndexBuilder.addTextRange([this.index.textRange![0], this.index.textRange![1] + againstOpLength]); + } break; } + case (this.type === OperationType.Modify && againstOp.type === OperationType.Delete): { + /** + * Decrease block index of the modify operation if againstOp delete a block before + */ + if (againstOp.index.isBlockIndex && this.index.blockIndex! > againstOp.index.blockIndex!) { + newIndexBuilder.addBlockIndex(this.index.blockIndex!--); - case OperationType.Delete: { - const payload = (againstOp as Operation).data.payload; + break; + } + + /** + * Make modify operation neutral if againstOp deleted a block, to apply modify to + */ + if (againstOp.index.isBlockIndex && this.index.blockIndex! === againstOp.index.blockIndex!) { + return new Operation(OperationType.Neutral, this.index, { payload: [] }, this.userId); + } + break; + } + case (this.type === OperationType.Delete && againstOp.type === OperationType.Insert): { + /** + * Increase block index if againstOp insert a block before + */ if (againstOp.index.isBlockIndex) { - newIndexBuilder.addBlockIndex(this.index.blockIndex! - payload.length); + newIndexBuilder.addBlockIndex(this.index.blockIndex!++); break; } - newIndexBuilder.addTextRange([this.index.textRange![0] - payload.length, this.index.textRange![1] - payload.length]); + if (this.index.isTextIndex && againstOp.index.isTextIndex) { + const againstOpLength = againstOp.data.payload?.length; + + /** + * Extend delete operation range if againstOp insert a text inside of the delete bounds + */ + if ((againstOp.index.textRange![0] > this.index.textRange![0]) && (againstOp.index.textRange![0] < this.index.textRange![1])) { + newIndexBuilder.addTextRange([this.index.textRange![0], this.index.textRange![1] + againstOpLength]); + break; + } + + /** + * Move deletion bounds to the right by amount of inserted text + */ + if (this.index.textRange![0] > againstOp.index.textRange![0]) { + newIndexBuilder.addTextRange([this.index.textRange![0] + againstOpLength, this.index.textRange![1] + againstOpLength]); + } + } break; } + case (this.type === OperationType.Delete && againstOp.type === OperationType.Delete): { + /** + * Decrease block index if againstOp delete a block before + */ + if (againstOp.index.isBlockIndex && this.index.blockIndex! > againstOp.index.blockIndex!) { + newIndexBuilder.addBlockIndex(this.index.blockIndex!--); - default: + break; + } + + if (this.index.isTextIndex && againstOp.index.isTextIndex) { + const againstOpLength = againstOp.index.textRange![1] - againstOp.index.textRange![0]; + + /** + * Move deletion bounds to the left by amount of deleted text + */ + if (this.index.textRange![0] > againstOp.index.textRange![1]) { + newIndexBuilder.addTextRange([this.index.textRange![0] - againstOpLength, this.index.textRange![1] - againstOpLength]); + + break; + } + + /** + * If operation tries to delete a range that is already deleted, return neutral operation + */ + if (this.index.textRange![0] >= againstOp.index.textRange![0] && this.index.textRange![1] <= againstOp.index.textRange![1]) { + return new Operation(OperationType.Neutral, this.index, { payload: [] }, this.userId); + } + + /** + * Remove part of the delete operation range if it is already deleted by againstOp + * Cover three possible overlaps + */ + if (this.index.textRange![0] > againstOp.index.textRange![0] && this.index.textRange![1] > againstOp.index.textRange![1]) { + newIndexBuilder.addTextRange([againstOp.index.textRange![1], this.index.textRange![1]]); + + break; + } + if (this.index.textRange![0] < againstOp.index.textRange![0] && this.index.textRange![1] < againstOp.index.textRange![1]) { + newIndexBuilder.addTextRange([this.index.textRange![0], againstOp.index.textRange![0]]); + + break; + } + if (this.index.textRange![0] < againstOp.index.textRange![0] && this.index.textRange![1] > againstOp.index.textRange![1]) { + newIndexBuilder.addTextRange([this.index.textRange![0], againstOp.index.textRange![1] - againstOpLength]); + + break; + } + } + } + default: { throw new Error('Unsupported operation type'); + } } const operation = Operation.from(this); @@ -289,4 +459,12 @@ export class Operation { return false; } + + // #removeOverlappingTextRange(range: TextRange, againstRange: TextRange): TextRange { + // if (range[0] <= againstRange[0] && range[1] >= againstRange[0]) { + // return [againstRange[0], range[1]]; + // } + + // return range; + // } } diff --git a/packages/collaboration-manager/src/OperationsBatch.spec.ts b/packages/collaboration-manager/src/OperationsBatch.spec.ts index 7902eea3..e6f9ecbd 100644 --- a/packages/collaboration-manager/src/OperationsBatch.spec.ts +++ b/packages/collaboration-manager/src/OperationsBatch.spec.ts @@ -1,7 +1,6 @@ import { createDataKey, IndexBuilder } from '@editorjs/model'; import { OperationsBatch } from './OperationsBatch.js'; -import { Operation, OperationType } from './Operation.js'; -import { jest } from '@jest/globals'; +import { Operation, OperationType, SerializedOperation } from './Operation.js'; const templateIndex = new IndexBuilder() .addBlockIndex(0) @@ -12,10 +11,6 @@ const templateIndex = new IndexBuilder() const userId = 'user'; describe('Batch', () => { - beforeAll(() => { - jest.useFakeTimers(); - }); - it('should add Insert operation to batch', () => { const op1 = new Operation(OperationType.Insert, templateIndex, { payload: 'a' }, userId); const op2 = new Operation( @@ -26,24 +21,14 @@ describe('Batch', () => { { payload: 'b' }, userId ); - const onTimeout = jest.fn(); - const batch = new OperationsBatch(onTimeout, op1); + const batch = new OperationsBatch(op1); batch.add(op2); - const effectiveOp = batch.getEffectiveOperation(); + const operations = batch.operations; - expect(effectiveOp).toEqual({ - type: OperationType.Insert, - index: new IndexBuilder() - .from(templateIndex) - .addTextRange([0, 1]) - .build(), - data: { payload: 'ab' }, - rev: undefined, - userId, - }); + expect(operations).toEqual([op1, op2]); }); it('should add Delete operation to batch', () => { @@ -56,239 +41,155 @@ describe('Batch', () => { { payload: 'b' }, userId ); - const onTimeout = jest.fn(); - - const batch = new OperationsBatch(onTimeout, op1); - - batch.add(op2); - - const effectiveOp = batch.getEffectiveOperation(); - - expect(effectiveOp).toEqual({ - type: OperationType.Delete, - index: new IndexBuilder() - .from(templateIndex) - .addTextRange([0, 1]) - .build(), - data: { payload: 'ab' }, - rev: undefined, - userId, - }); - }); - - it('should terminate the batch if the new operation is not text operation', () => { - const op1 = new Operation(OperationType.Delete, templateIndex, { payload: 'a' }, userId); - const op2 = new Operation( - OperationType.Delete, - new IndexBuilder().from(templateIndex) - .addDataKey(undefined) - .addTextRange(undefined) - .build(), - { - payload: [ - { - name: 'paragraph', - data: { text: '' }, - }, - ], - }, - userId - ); - - const onTimeout = jest.fn(); - - const batch = new OperationsBatch(onTimeout, op1); - - batch.add(op2); - - expect(onTimeout).toBeCalledWith(batch, op2); - }); - - it('should terminate the batch if operation in the batch is not text operation', () => { - const op1 = new Operation( - OperationType.Delete, - new IndexBuilder().from(templateIndex) - .addDataKey(undefined) - .addTextRange(undefined) - .build(), - { - payload: [ - { - name: 'paragraph', - data: { text: '' }, - }, - ], - }, - userId - ); - const op2 = new Operation(OperationType.Delete, templateIndex, { payload: 'a' }, userId); - - const onTimeout = jest.fn(); - const batch = new OperationsBatch(onTimeout, op1); + const batch = new OperationsBatch(op1); batch.add(op2); - expect(onTimeout).toBeCalledWith(batch, op2); - }); - - it('should terminate the batch if operation in the batch is Modify operation', () => { - const op1 = new Operation( - OperationType.Modify, - new IndexBuilder().from(templateIndex) - .build(), - { - payload: { - tool: 'bold', - }, - prevPayload: { - tool: 'bold', - }, - }, - userId - ); - const op2 = new Operation(OperationType.Delete, templateIndex, { payload: 'a' }, userId); - - const onTimeout = jest.fn(); - - const batch = new OperationsBatch(onTimeout, op1); - - batch.add(op2); + const operations = batch.operations; - expect(onTimeout).toBeCalledWith(batch, op2); + expect(operations).toEqual([op1, op2]); }); - it('should terminate the batch if the new operation is Modify operation', () => { - const op1 = new Operation(OperationType.Delete, templateIndex, { payload: 'a' }, userId); - const op2 = new Operation( - OperationType.Modify, - new IndexBuilder().from(templateIndex) - .build(), - { - payload: { - tool: 'bold', - }, - prevPayload: { - tool: 'bold', - }, - }, - userId - ); - - const onTimeout = jest.fn(); - - const batch = new OperationsBatch(onTimeout, op1); - - batch.add(op2); - - expect(onTimeout).toBeCalledWith(batch, op2); - }); - - it('should terminate the batch if operations are of different type', () => { - const op1 = new Operation(OperationType.Delete, templateIndex, { payload: 'a' }, userId); - const op2 = new Operation( - OperationType.Insert, - new IndexBuilder().from(templateIndex) - .addTextRange([1, 1]) - .build(), - { payload: 'b' }, - userId - ); - const onTimeout = jest.fn(); - - const batch = new OperationsBatch(onTimeout, op1); - - batch.add(op2); - - expect(onTimeout).toBeCalledWith(batch, op2); - }); - - it('should terminate the batch if operations block indexes are not the same', () => { - const op1 = new Operation(OperationType.Insert, templateIndex, { payload: 'a' }, userId); - const op2 = new Operation( - OperationType.Insert, - new IndexBuilder().from(templateIndex) - .addBlockIndex(1) - .addTextRange([1, 1]) - .build(), - { payload: 'b' }, - userId - ); - const onTimeout = jest.fn(); - - const batch = new OperationsBatch(onTimeout, op1); - - batch.add(op2); - - expect(onTimeout).toBeCalledWith(batch, op2); - }); - - it('should terminate the batch if operations data keys are not the same', () => { - const op1 = new Operation(OperationType.Insert, templateIndex, { payload: 'a' }, userId); - const op2 = new Operation( - OperationType.Insert, - new IndexBuilder().from(templateIndex) - .addDataKey(createDataKey('differentKey')) - .addTextRange([1, 1]) - .build(), - { payload: 'b' }, - userId - ); - const onTimeout = jest.fn(); + describe('from()', () => { + it('should create a new batch from an existing batch', () => { + const op1 = new Operation(OperationType.Insert, templateIndex, { payload: 'a' }, userId); + const op2 = new Operation( + OperationType.Insert, + new IndexBuilder().from(templateIndex).addTextRange([1, 1]).build(), + { payload: 'b' }, + userId + ); + const originalBatch = new OperationsBatch(op1); + originalBatch.add(op2); + + const newBatch = OperationsBatch.from(originalBatch); + + expect(newBatch.operations).toEqual(originalBatch.operations); + expect(newBatch).not.toBe(originalBatch); // Should be a new instance + }); - const batch = new OperationsBatch(onTimeout, op1); + it('should create a new batch from serialized operation', () => { + const serializedOp: SerializedOperation = new Operation(OperationType.Delete, templateIndex, { payload: 'a' }, userId).serialize(); - batch.add(op2); + const batch = OperationsBatch.from(serializedOp); - expect(onTimeout).toBeCalledWith(batch, op2); + expect(batch.operations[0].type).toBe(serializedOp.type); + expect(batch.operations[0].data).toEqual(serializedOp.data); + }); }); - it('should terminate the batch if operations index ranges are not adjacent', () => { - const op1 = new Operation(OperationType.Insert, templateIndex, { payload: 'a' }, userId); - const op2 = new Operation( - OperationType.Insert, - new IndexBuilder().from(templateIndex) - .addTextRange([2, 2]) - .build(), - { payload: 'b' }, - userId - ); - const onTimeout = jest.fn(); - - const batch = new OperationsBatch(onTimeout, op1); - - batch.add(op2); - - expect(onTimeout).toBeCalledWith(batch, op2); + describe('inverse()', () => { + it('should inverse all operations in the batch', () => { + const op1 = new Operation(OperationType.Insert, templateIndex, { payload: 'a' }, userId); + const op2 = new Operation( + OperationType.Insert, + new IndexBuilder().from(templateIndex).addTextRange([1, 1]).build(), + { payload: 'b' }, + userId + ); + const batch = new OperationsBatch(op1); + batch.add(op2); + + const inversedBatch = batch.inverse(); + + expect(inversedBatch.operations[0].type).toBe(OperationType.Delete); + expect(inversedBatch.operations[1].type).toBe(OperationType.Delete); + }); }); - it('should terminate the batch if timeout is exceeded', () => { - const op1 = new Operation(OperationType.Insert, templateIndex, { payload: 'a' }, userId); - - const onTimeout = jest.fn(); - - const batch = new OperationsBatch(onTimeout, op1); + describe('transform()', () => { + it('should transform operations against another operation', () => { + const op1 = new Operation(OperationType.Insert, templateIndex, { payload: 'a' }, userId); + const op2 = new Operation( + OperationType.Insert, + new IndexBuilder().from(templateIndex).addTextRange([1, 1]).build(), + { payload: 'b' }, + userId + ); + const batch = new OperationsBatch(op1); + batch.add(op2); + + const againstOp = new Operation( + OperationType.Insert, + new IndexBuilder().from(templateIndex).addTextRange([0, 0]).build(), + { payload: 'x' }, + 'other-user' + ); + + const transformedBatch = batch.transform(againstOp); + + expect(transformedBatch).not.toBeNull(); + expect(transformedBatch!.operations.length).toBe(2); + // Check if text ranges were shifted by 1 due to insertion + expect(transformedBatch!.operations[0].index.textRange![0]).toBe(1); + expect(transformedBatch!.operations[1].index.textRange![0]).toBe(2); + }); - // eslint-disable-next-line @typescript-eslint/no-magic-numbers - jest.advanceTimersByTime(1000); + it('should return null if no operations can be transformed', () => { + const op = new Operation(OperationType.Insert, templateIndex, { payload: 'a' }, userId); + const batch = new OperationsBatch(op); + + // An operation that would make transformation impossible + const againstOp = new Operation(OperationType.Delete, templateIndex, { payload: 'a' }, 'other-user'); - expect(onTimeout).toBeCalledWith(batch, undefined); - }); + const transformedBatch = batch.transform(againstOp); - it('should return null if there\'s no operations as effective operation in the batch', () => { - const onTimeout = jest.fn(); - const batch = new OperationsBatch(onTimeout); - - expect(batch.getEffectiveOperation()).toBeNull(); + expect(transformedBatch).toBeNull(); + }); }); - it('should return the only operation in the batch as effective operation', () => { - const op1 = new Operation(OperationType.Insert, templateIndex, { payload: 'a' }, userId); + describe('canAdd()', () => { + it('should return true for consecutive text operations of same type', () => { + const op1 = new Operation(OperationType.Insert, templateIndex, { payload: 'a' }, userId); + const op2 = new Operation( + OperationType.Insert, + new IndexBuilder().from(templateIndex).addTextRange([1, 1]).build(), + { payload: 'b' }, + userId + ); + const batch = new OperationsBatch(op1); + + expect(batch.canAdd(op2)).toBe(true); + }); - const onTimeout = jest.fn(); + it('should return false for non-consecutive text operations', () => { + const op1 = new Operation(OperationType.Insert, templateIndex, { payload: 'a' }, userId); + const op2 = new Operation( + OperationType.Insert, + new IndexBuilder().from(templateIndex).addTextRange([2, 2]).build(), + { payload: 'b' }, + userId + ); + const batch = new OperationsBatch(op1); + + expect(batch.canAdd(op2)).toBe(false); + }); - const batch = new OperationsBatch(onTimeout, op1); + it('should return false for different operation types', () => { + const op1 = new Operation(OperationType.Insert, templateIndex, { payload: 'a' }, userId); + const op2 = new Operation( + OperationType.Delete, + new IndexBuilder().from(templateIndex).addTextRange([1, 1]).build(), + { payload: 'b' }, + userId + ); + const batch = new OperationsBatch(op1); + + expect(batch.canAdd(op2)).toBe(false); + }); - expect(batch.getEffectiveOperation()).toEqual(op1); + it('should return false for modify operations', () => { + const op1 = new Operation(OperationType.Insert, templateIndex, { payload: 'a' }, userId); + const op2 = new Operation( + OperationType.Modify, + new IndexBuilder().from(templateIndex).addTextRange([1, 1]).build(), + { payload: { tool: 'bold' } }, + userId + ); + const batch = new OperationsBatch(op1); + + expect(batch.canAdd(op2)).toBe(false); + }); }); }); diff --git a/packages/collaboration-manager/src/OperationsBatch.ts b/packages/collaboration-manager/src/OperationsBatch.ts index 51661e59..d2544004 100644 --- a/packages/collaboration-manager/src/OperationsBatch.ts +++ b/packages/collaboration-manager/src/OperationsBatch.ts @@ -1,50 +1,28 @@ -import { IndexBuilder, type TextRange } from '@editorjs/model'; -import { Operation, OperationType } from './Operation.js'; +import { InvertedOperationType, Operation, OperationType, SerializedOperation } from './Operation.js'; /** * Batch debounce time */ const DEBOUNCE_TIMEOUT = 500; -/** - * Batch termination callback - * - * @param batch - terminated batch - * @param [lastOperation] - operation on which the batch was terminated - */ -type OnBatchTermination = (batch: OperationsBatch, lastOperation?: Operation) => void; - /** * Class to batch Text operations (maybe others in the future) for Undo/Redo purposes * * Operations are batched on timeout basis or if batch is terminated from the outside */ -export class OperationsBatch { +export class OperationsBatch extends Operation { /** * Array of operations to batch - * - * @private */ - #operations: Operation[] = []; - - /** - * Termination callback - */ - #onTermination: OnBatchTermination; - - /** - * Termination timeout - */ - #debounceTimer?: ReturnType; + operations: (Operation | Operation)[] = []; /** * Batch constructor function * - * @param onTermination - termination callback * @param firstOperation - first operation to add */ - constructor(onTermination: OnBatchTermination = () => {}, firstOperation?: Operation) { - this.#onTermination = onTermination; + constructor(firstOperation: Operation | Operation) { + super(firstOperation.type, firstOperation.index, firstOperation.data, firstOperation.userId, firstOperation.rev); if (firstOperation !== undefined) { this.add(firstOperation); @@ -53,63 +31,96 @@ export class OperationsBatch { /** * Adds an operation to the batch + * Make sure, that operation could be added to the batch * * @param op - operation to add */ - public add(op: Operation): void { - if (!this.#canAdd(op)) { - this.terminate(op); + public add(op: Operation | Operation): void { + this.operations.push(op); + } - return; - } + /** + * Create a new operation batch from an array of operations + * + * @param opBatch - operation batch to clone + */ + public static from(opBatch: OperationsBatch): OperationsBatch; - this.#operations.push(op); + /** + * Create a new operation batch from a serialized operation + * + * @param json - serialized operation + */ + public static from(json: SerializedOperation): OperationsBatch; - clearTimeout(this.#debounceTimer); - this.#debounceTimer = setTimeout(() => this.terminate(), DEBOUNCE_TIMEOUT); + /** + * Create a new operation batch from an operation batch or a serialized operation + * + * @param opBatchOrJSON - operation batch or serialized operation + */ + public static from(opBatchOrJSON: OperationsBatch | SerializedOperation): OperationsBatch { + if (opBatchOrJSON instanceof OperationsBatch) { + /** + * Every batch should have at least one operation + */ + const batch = new OperationsBatch(opBatchOrJSON.operations.shift()!); + + opBatchOrJSON.operations.forEach((op) => { + /** + * Deep clone operation to the new batch + */ + batch.add(Operation.from(op)); + }); + + return batch as OperationsBatch; + } else { + const batch = new OperationsBatch(Operation.from(opBatchOrJSON)); + + return batch; + } } - /** - * Returns and effective operations for all the operations in the batch + * Method that inverses all of the operations in the batch + * + * @returns new batch with inversed operations */ - public getEffectiveOperation(): Operation | null { - if (this.#operations.length === 0) { - return null; - } + public inverse(): OperationsBatch> { + /** + * Every batch should have at least one operation + */ + const newOperationsBatch = new OperationsBatch | OperationType.Neutral>(this.operations.pop()!.inverse()) + + while (this.operations.length > 0) { + const op = this.operations.pop()!.inverse(); - if (this.#operations.length === 1) { - return this.#operations[0]; + newOperationsBatch.add(op); } - const type = this.#operations[0].type; - const index = this.#operations[0].index; - - const range: TextRange = [ - this.#operations[0].index.textRange![0], - this.#operations[this.#operations.length - 1].index.textRange![1], - ]; - const payload = this.#operations.reduce((text, operation) => text + operation.data.payload, ''); - - return new Operation( - type, - new IndexBuilder().from(index) - .addTextRange(range) - .build(), - { payload }, - this.#operations[0].userId - ); + return newOperationsBatch as OperationsBatch>; } /** - * Terminates the batch, passes operation on which batch was terminated to the callback + * Method that transforms all of the operations in the batch against another operation * - * @param lastOp - operation on which the batch is terminated + * @param againstOp - operation to transform against + * @returns new batch with transformed operations */ - public terminate(lastOp?: Operation): void { - clearTimeout(this.#debounceTimer); + public transform(againstOp: Operation): OperationsBatch { + const transformedOp = this.operations.shift()!.transform(againstOp); + + const newOperationsBatch = new OperationsBatch(transformedOp); + + /** + * We either have a new operations batch or all operations were not transformable + */ + for (const op of this.operations) { + const transformedOp = op.transform(againstOp); + + newOperationsBatch.add(transformedOp); + } - this.#onTermination(this, lastOp); + return newOperationsBatch; } /** @@ -119,8 +130,8 @@ export class OperationsBatch { * * @param op - operation to check */ - #canAdd(op: Operation): boolean { - const lastOp = this.#operations[this.#operations.length - 1]; + canAdd(op: Operation): boolean { + const lastOp = this.operations[this.operations.length - 1]; if (lastOp === undefined) { return true; diff --git a/packages/collaboration-manager/src/UndoRedoManager.spec.ts b/packages/collaboration-manager/src/UndoRedoManager.spec.ts index ba95322b..cb9cfd84 100644 --- a/packages/collaboration-manager/src/UndoRedoManager.spec.ts +++ b/packages/collaboration-manager/src/UndoRedoManager.spec.ts @@ -5,6 +5,25 @@ import { UndoRedoManager } from './UndoRedoManager.js'; const userId = 'user'; +/** + * Helper function to create test operations + */ +function createOperation(index: number, text: string): Operation { + return new Operation( + OperationType.Insert, + new IndexBuilder() + .addBlockIndex(index) + .build(), + { + payload: [{ + name: 'paragraph', + data: { text }, + }], + }, + userId + ); +} + describe('UndoRedoManager', () => { it('should return inverted operation on undo', () => { const manager = new UndoRedoManager(); @@ -108,4 +127,135 @@ describe('UndoRedoManager', () => { expect(result).toBeUndefined(); }); + + describe('transform operations', () => { + it('should transform undo stack operations', () => { + const manager = new UndoRedoManager(); + const op1 = createOperation(0, 'first'); + const op2 = createOperation(1, 'second'); + + // Put operations in undo stack + manager.put(op1); + manager.put(op2); + + // Create operation that will affect the stack + const transformingOp = createOperation(0, 'transform'); + + // Mock transform method + const transformSpy = jest.spyOn(op2, 'transform').mockReturnValue(createOperation(2, 'transformed')); + + manager.transformUndoStack(transformingOp); + + expect(transformSpy).toHaveBeenCalledWith(transformingOp); + }); + + it('should transform redo stack operations', () => { + const manager = new UndoRedoManager(); + const op1 = createOperation(0, 'first'); + + // Put operation and undo it to move it to redo stack + manager.put(op1); + manager.undo(); + + // Create operation that will affect the stack + const transformingOp = createOperation(0, 'transform'); + + // Mock transform method + const transformSpy = jest.spyOn(op1, 'transform').mockReturnValue(createOperation(1, 'transformed')); + + manager.transformRedoStack(transformingOp); + + expect(transformSpy).toHaveBeenCalledWith(transformingOp); + }); + + it('should remove operations from stack if transform returns null', () => { + const manager = new UndoRedoManager(); + const op1 = createOperation(0, 'first'); + const op2 = createOperation(1, 'second'); + + manager.put(op1); + manager.put(op2); + + // Mock transform to return null + jest.spyOn(op2, 'transform').mockReturnValue(null); + + const transformingOp = createOperation(0, 'transform'); + manager.transformUndoStack(transformingOp); + + // Try to undo twice - second undo should return undefined since op2 was removed + expect(manager.undo()).toBeDefined(); + expect(manager.undo()).toBeUndefined(); + }); + + it('should handle empty stacks during transformation', () => { + const manager = new UndoRedoManager(); + const transformingOp = createOperation(0, 'transform'); + + // Should not throw when transforming empty stacks + expect(() => { + manager.transformUndoStack(transformingOp); + manager.transformRedoStack(transformingOp); + }).not.toThrow(); + }); + + it('should transform both undo and redo stacks when operations are undone', () => { + const manager = new UndoRedoManager(); + const op1 = createOperation(0, 'first'); + const op2 = createOperation(1, 'second'); + + // Put operations in undo stack + manager.put(op1); + manager.put(op2); + + // Undo op2 to move it to redo stack + manager.undo(); + + // Create operation that will affect both stacks + const transformingOp = createOperation(0, 'transform'); + + // Mock transform methods + const undoStackSpy = jest.spyOn(op1, 'transform').mockReturnValue(createOperation(1, 'transformed-undo')); + const redoStackSpy = jest.spyOn(op2, 'transform').mockReturnValue(createOperation(2, 'transformed-redo')); + + // Transform both stacks + manager.transformUndoStack(transformingOp); + manager.transformRedoStack(transformingOp); + + expect(undoStackSpy).toHaveBeenCalledWith(transformingOp); + expect(redoStackSpy).toHaveBeenCalledWith(transformingOp); + }); + + it('should maintain correct operation order after transforming both stacks', () => { + const manager = new UndoRedoManager(); + const op1 = createOperation(0, 'first'); + const op2 = createOperation(1, 'second'); + const op3 = createOperation(2, 'third'); + + // Setup initial state + manager.put(op1); + manager.put(op2); + manager.put(op3); + + // Move op3 and op2 to redo stack + manager.undo(); // undo op3 + manager.undo(); // undo op2 + + const transformingOp = createOperation(0, 'transform'); + + // Mock transforms to return operations with shifted indices + jest.spyOn(op1, 'transform').mockReturnValue(createOperation(1, 'transformed-1')); + jest.spyOn(op2, 'transform').mockReturnValue(createOperation(2, 'transformed-2')); + jest.spyOn(op3, 'transform').mockReturnValue(createOperation(3, 'transformed-3')); + + manager.transformUndoStack(transformingOp); + manager.transformRedoStack(transformingOp); + + // Verify operations can be redone in correct order + const redoOp1 = manager.redo(); + const redoOp2 = manager.redo(); + + expect(redoOp1?.index.toString()).toBe('2'); // transformed op2 + expect(redoOp2?.index.toString()).toBe('3'); // transformed op3 + }); + }); }); diff --git a/packages/collaboration-manager/src/UndoRedoManager.ts b/packages/collaboration-manager/src/UndoRedoManager.ts index 08c7cbac..0ab424e3 100644 --- a/packages/collaboration-manager/src/UndoRedoManager.ts +++ b/packages/collaboration-manager/src/UndoRedoManager.ts @@ -57,4 +57,28 @@ export class UndoRedoManager { this.#undoStack.push(operation); this.#redoStack = []; } + + public transformUndoStack(operation: Operation): void { + this.transformStack(operation, this.#undoStack); + } + + public transformRedoStack(operation: Operation): void { + this.transformStack(operation, this.#redoStack); + } + + public transformStack(operation: Operation, stack: Operation[]): void { + const transformed = stack.flatMap((op) => { + const transformedOp = op.transform(operation); + + if (transformedOp === null) { + return [] + } + + return [ transformedOp ]; + }) + + stack.length = 0; + stack.push(...transformed); + + } } diff --git a/packages/playground/src/App.vue b/packages/playground/src/App.vue index b1aca417..037fc876 100644 --- a/packages/playground/src/App.vue +++ b/packages/playground/src/App.vue @@ -29,7 +29,8 @@ onMounted(() => { holder: document.getElementById('editorjs') as HTMLElement, userId: userId, documentId: 'test', - // collaborationServer: 'ws://localhost:8080', + // collaborationServer: 'wss://lirili-larila.codex.so/', + collaborationServer: 'ws://localhost:8080', data: { blocks: [ { diff --git a/packages/ui/src/Blocks/Blocks.ts b/packages/ui/src/Blocks/Blocks.ts index cc8807bb..517d81fd 100644 --- a/packages/ui/src/Blocks/Blocks.ts +++ b/packages/ui/src/Blocks/Blocks.ts @@ -70,10 +70,14 @@ export class BlocksUI implements EditorjsPlugin { if (e.shiftKey) { this.#eventBus.dispatchEvent(new Event('core:redo')); + e.preventDefault(); + return; } this.#eventBus.dispatchEvent(new Event('core:undo')); + + e.preventDefault(); }); } From 7a5a0e44f5c4a12f21dcb36e317d506523316b5f Mon Sep 17 00:00:00 2001 From: e11sy <130844513+e11sy@users.noreply.github.com> Date: Sat, 12 Apr 2025 01:06:42 +0200 Subject: [PATCH 02/15] feat(collab): add operations transformer class --- ...Batch.spec.ts => BatchedOperation.spec.ts} | 2 +- ...OperationsBatch.ts => BatchedOperation.ts} | 0 .../src/CollaborationManager.ts | 2 +- .../collaboration-manager/src/Operation.ts | 245 +------------- .../src/OperationsTransformer.ts | 314 ++++++++++++++++++ 5 files changed, 323 insertions(+), 240 deletions(-) rename packages/collaboration-manager/src/{OperationsBatch.spec.ts => BatchedOperation.spec.ts} (99%) rename packages/collaboration-manager/src/{OperationsBatch.ts => BatchedOperation.ts} (100%) create mode 100644 packages/collaboration-manager/src/OperationsTransformer.ts diff --git a/packages/collaboration-manager/src/OperationsBatch.spec.ts b/packages/collaboration-manager/src/BatchedOperation.spec.ts similarity index 99% rename from packages/collaboration-manager/src/OperationsBatch.spec.ts rename to packages/collaboration-manager/src/BatchedOperation.spec.ts index e6f9ecbd..b577e5ec 100644 --- a/packages/collaboration-manager/src/OperationsBatch.spec.ts +++ b/packages/collaboration-manager/src/BatchedOperation.spec.ts @@ -1,5 +1,5 @@ import { createDataKey, IndexBuilder } from '@editorjs/model'; -import { OperationsBatch } from './OperationsBatch.js'; +import { OperationsBatch } from './BatchedOperation.js'; import { Operation, OperationType, SerializedOperation } from './Operation.js'; const templateIndex = new IndexBuilder() diff --git a/packages/collaboration-manager/src/OperationsBatch.ts b/packages/collaboration-manager/src/BatchedOperation.ts similarity index 100% rename from packages/collaboration-manager/src/OperationsBatch.ts rename to packages/collaboration-manager/src/BatchedOperation.ts diff --git a/packages/collaboration-manager/src/CollaborationManager.ts b/packages/collaboration-manager/src/CollaborationManager.ts index 6acd1895..e427fbbe 100644 --- a/packages/collaboration-manager/src/CollaborationManager.ts +++ b/packages/collaboration-manager/src/CollaborationManager.ts @@ -10,7 +10,7 @@ import { } from '@editorjs/model'; import type { CoreConfig } from '@editorjs/sdk'; import { OTClient } from './client/index.js'; -import { OperationsBatch } from './OperationsBatch.js'; +import { OperationsBatch } from './BatchedOperation.js'; import { type ModifyOperationData, Operation, OperationType } from './Operation.js'; import { UndoRedoManager } from './UndoRedoManager.js'; diff --git a/packages/collaboration-manager/src/Operation.ts b/packages/collaboration-manager/src/Operation.ts index af092519..ad1fcf10 100644 --- a/packages/collaboration-manager/src/Operation.ts +++ b/packages/collaboration-manager/src/Operation.ts @@ -1,4 +1,5 @@ import { IndexBuilder, type Index, type BlockNodeSerialized } from '@editorjs/model'; +import { OperationsTransformer } from './OperationsTransformer'; /** * Type of the operation @@ -126,6 +127,11 @@ export class Operation { */ public rev?: number; + /** + * Transformer for operations + */ + #transformer: OperationsTransformer = new OperationsTransformer(); + /** * Creates an instance of Operation * @@ -215,217 +221,7 @@ export class Operation { * @param againstOp - operation to transform against */ public transform(againstOp: Operation | Operation): Operation { - /** - * Do not transform operations if they are on different documents - */ - if (this.index.documentId !== againstOp.index.documentId) { - return this; - } - - /** - * Do not transform if the againstOp index is greater or if againstOp is Modify op - */ - if (!this.#shouldTransform(againstOp.index) || againstOp.type === OperationType.Modify) { - return this; - } - - const newIndexBuilder = new IndexBuilder().from(this.index); - - switch (true) { - /** - * If one of the operations is neutral, return this operation - */ - case (this.type === OperationType.Neutral || againstOp.type === OperationType.Neutral): { - return Operation.from(this); - } - /** - * Every operation against modify operation stays the same - */ - case (againstOp.type === OperationType.Modify): { - break; - } - case (this.type === OperationType.Insert && againstOp.type === OperationType.Insert): { - /** - * Update block index if againstOp is insert of a block - */ - if (againstOp.index.isBlockIndex) { - newIndexBuilder.addBlockIndex(this.index.blockIndex!++); - - break; - } - - /** - * Move current insert to the right on amount on chars, inserted by againstOp - */ - if (this.index.isTextIndex && againstOp.index.isTextIndex) { - const againstOpLength = againstOp.data.payload!.length; - - newIndexBuilder.addTextRange([this.index.textRange![0] + againstOpLength, this.index.textRange![1] + againstOpLength]) - - break; - } - } - case (this.type === OperationType.Insert && againstOp.type === OperationType.Delete): { - /** - * Decrease block index if againstOp is Delete block before current insert - */ - if (againstOp.index.isBlockIndex && this.index.blockIndex! > againstOp.index.blockIndex!) { - newIndexBuilder.addBlockIndex(this.index.blockIndex!--); - - break; - } - - if (this.index.isTextIndex && againstOp.index.isTextIndex) { - /** - * Deleted the range on the left of the current insert - */ - if (this.index.textRange![0] > againstOp.index.textRange![1]) { - newIndexBuilder.addTextRange([this.index.textRange![0] - againstOp.index.textRange![1], this.index.textRange![1] - againstOp.index.textRange![1]]); - - break; - } - - /** - * Deleted the range, then trying to insert new text inside of the deleted range - * Then insert should be done in the start of the deleted range - */ - if ((this.index.textRange![0] <= againstOp.index.textRange![0]) && (this.index.textRange![0] > againstOp.index.textRange![0])) { - newIndexBuilder.addTextRange([againstOp.index.textRange![0], againstOp.index.textRange![0]]); - } - } - - break; - } - case (this.type === OperationType.Modify && againstOp.type === OperationType.Insert): { - /** - * Increase block index of the modify operation if againstOp insert a block before - */ - if (againstOp.index.isBlockIndex) { - newIndexBuilder.addBlockIndex(this.index.blockIndex!++); - - break; - } - - /** - * Extend modify operation range if againstOp insert a text inside of the modify bounds - */ - if (againstOp.index.textRange![0] < this.index.textRange![0] && againstOp.index.textRange![1] > this.index.textRange![0]) { - const againstOpLength = againstOp.index.textRange![1] - againstOp.index.textRange![0]; - - newIndexBuilder.addTextRange([this.index.textRange![0], this.index.textRange![1] + againstOpLength]); - } - break; - } - case (this.type === OperationType.Modify && againstOp.type === OperationType.Delete): { - /** - * Decrease block index of the modify operation if againstOp delete a block before - */ - if (againstOp.index.isBlockIndex && this.index.blockIndex! > againstOp.index.blockIndex!) { - newIndexBuilder.addBlockIndex(this.index.blockIndex!--); - - break; - } - - /** - * Make modify operation neutral if againstOp deleted a block, to apply modify to - */ - if (againstOp.index.isBlockIndex && this.index.blockIndex! === againstOp.index.blockIndex!) { - return new Operation(OperationType.Neutral, this.index, { payload: [] }, this.userId); - - } - break; - } - case (this.type === OperationType.Delete && againstOp.type === OperationType.Insert): { - /** - * Increase block index if againstOp insert a block before - */ - if (againstOp.index.isBlockIndex) { - newIndexBuilder.addBlockIndex(this.index.blockIndex!++); - - break; - } - - if (this.index.isTextIndex && againstOp.index.isTextIndex) { - const againstOpLength = againstOp.data.payload?.length; - - /** - * Extend delete operation range if againstOp insert a text inside of the delete bounds - */ - if ((againstOp.index.textRange![0] > this.index.textRange![0]) && (againstOp.index.textRange![0] < this.index.textRange![1])) { - newIndexBuilder.addTextRange([this.index.textRange![0], this.index.textRange![1] + againstOpLength]); - - break; - } - - /** - * Move deletion bounds to the right by amount of inserted text - */ - if (this.index.textRange![0] > againstOp.index.textRange![0]) { - newIndexBuilder.addTextRange([this.index.textRange![0] + againstOpLength, this.index.textRange![1] + againstOpLength]); - } - } - break; - } - case (this.type === OperationType.Delete && againstOp.type === OperationType.Delete): { - /** - * Decrease block index if againstOp delete a block before - */ - if (againstOp.index.isBlockIndex && this.index.blockIndex! > againstOp.index.blockIndex!) { - newIndexBuilder.addBlockIndex(this.index.blockIndex!--); - - break; - } - - if (this.index.isTextIndex && againstOp.index.isTextIndex) { - const againstOpLength = againstOp.index.textRange![1] - againstOp.index.textRange![0]; - - /** - * Move deletion bounds to the left by amount of deleted text - */ - if (this.index.textRange![0] > againstOp.index.textRange![1]) { - newIndexBuilder.addTextRange([this.index.textRange![0] - againstOpLength, this.index.textRange![1] - againstOpLength]); - - break; - } - - /** - * If operation tries to delete a range that is already deleted, return neutral operation - */ - if (this.index.textRange![0] >= againstOp.index.textRange![0] && this.index.textRange![1] <= againstOp.index.textRange![1]) { - return new Operation(OperationType.Neutral, this.index, { payload: [] }, this.userId); - } - - /** - * Remove part of the delete operation range if it is already deleted by againstOp - * Cover three possible overlaps - */ - if (this.index.textRange![0] > againstOp.index.textRange![0] && this.index.textRange![1] > againstOp.index.textRange![1]) { - newIndexBuilder.addTextRange([againstOp.index.textRange![1], this.index.textRange![1]]); - - break; - } - if (this.index.textRange![0] < againstOp.index.textRange![0] && this.index.textRange![1] < againstOp.index.textRange![1]) { - newIndexBuilder.addTextRange([this.index.textRange![0], againstOp.index.textRange![0]]); - - break; - } - if (this.index.textRange![0] < againstOp.index.textRange![0] && this.index.textRange![1] > againstOp.index.textRange![1]) { - newIndexBuilder.addTextRange([this.index.textRange![0], againstOp.index.textRange![1] - againstOpLength]); - - break; - } - } - } - default: { - throw new Error('Unsupported operation type'); - } - } - - const operation = Operation.from(this); - - operation.index = newIndexBuilder.build(); - - return operation; + return this.#transformer.transform(this, againstOp); } /** @@ -440,31 +236,4 @@ export class Operation { rev: this.rev!, }; } - - /** - * Checks if operation needs to be transformed: - * 1. If relative operation (againstOp) happened in the block before or at the same index of the Block of _this_ operation - * 2. If relative operation happened in the same block and same data key and before the text range of _this_ operation - * - * @param indexToCompare - index of a relative operation - */ - #shouldTransform(indexToCompare: Index): boolean { - if (indexToCompare.isBlockIndex && this.index.blockIndex !== undefined) { - return indexToCompare.blockIndex! <= this.index.blockIndex; - } - - if (indexToCompare.isTextIndex && this.index.isTextIndex) { - return indexToCompare.dataKey === this.index.dataKey && indexToCompare.textRange![0] <= this.index.textRange![0]; - } - - return false; - } - - // #removeOverlappingTextRange(range: TextRange, againstRange: TextRange): TextRange { - // if (range[0] <= againstRange[0] && range[1] >= againstRange[0]) { - // return [againstRange[0], range[1]]; - // } - - // return range; - // } } diff --git a/packages/collaboration-manager/src/OperationsTransformer.ts b/packages/collaboration-manager/src/OperationsTransformer.ts new file mode 100644 index 00000000..471379fb --- /dev/null +++ b/packages/collaboration-manager/src/OperationsTransformer.ts @@ -0,0 +1,314 @@ +import { IndexBuilder } from "@editorjs/model"; +import { Operation, OperationType } from "./Operation"; + +/** + * Class that transforms operation against another operation + */ +export class OperationsTransformer { + constructor() {} + + public transform(operation: Operation, againstOp: Operation): Operation | Operation { + /** + * Do not transform operations if they are on different documents + */ + if (operation.index.documentId !== againstOp.index.documentId) { + return Operation.from(operation); + } + + return this.#determineTransformation(operation, againstOp); + } + + /** + * Method that desides what kind of transformation should be applied to the operation + * Cases: + * 1. Against operations is a block operation and current operation is also a block operation + * - check that againstOp affects current operation and transform against block operation + * + * 2. Against operation is a block operation and current operation is a text operation + * - same as above, check that againstOp affects current operation and transform against block operation + * + * 3. Against operation is a text operation and current operation is a block operation + * - text operation does not afftect block operation - so return copy of current operation + * + * 4. Against operation is a text operation and current operation is also a text operation + * - check that againstOp affects current operation and transform against text operation + * + * @param operation - operation to be transformed + * @param againstOp - operation against which the current operation should be transformed + * @returns new operation + */ + #determineTransformation(operation: Operation, againstOp: Operation): Operation | Operation { + const currentIndex = operation.index; + const againstIndex = againstOp.index; + + /** + * Cover 1 and 2 cases + * + * Check that againstOp is a block operation + */ + if (againstIndex.isBlockIndex && currentIndex.blockIndex !== undefined) { + /** + * Check that againstOp affects current operation + */ + if (againstIndex.blockIndex! <= currentIndex.blockIndex) { + return this.#transformAgainstBlockOperation(operation, againstOp); + } + } + + /** + * Cover 4 case + * + * Check that againstOp is a text operation and current operation is also a text operation + */ + if (againstIndex.isTextIndex && currentIndex.isTextIndex) { + /** + * Check that againstOp affects current operation (text operation on the same block and same input) + * and against op happened on the left side or has overlapping range + */ + if (currentIndex.dataKey === againstIndex.dataKey && currentIndex.blockIndex === againstIndex.blockIndex && againstIndex.textRange![0] <= currentIndex.textRange![0]) { + return this.#transformAgainstTextOperation(operation, againstOp); + } + } + + /** + * Cover 3 case + * + * Return copy of current operation + */ + return Operation.from(operation); + } + + /** + * Method that transforms operation against block operation + * + * Cases: + * 1. Against operation is an Insert operation + * - Increase block index of the current operation + * + * 2. Against operation is a Delete operation + * - Against operation deleted a block before the current operation + * - Decrease block index of the current operation + * - Against operation deleted exactly the block of the current operation + * - Return Neutral operation + * + * 3. Against operation is a Modify or Neutral operation + * - Modify and Neutral operations do not affect any operations so return copy of the current operation + * + * @param operation - Operation to be transformed + * @param againstOp - Operation against which the current operation should be transformed + * @returns New operation + */ + #transformAgainstBlockOperation(operation: Operation, againstOp: Operation): Operation | Operation { + const newIndexBuilder = new IndexBuilder().from(operation.index); + + /** + * Update the index of the current operation + */ + switch (againstOp.type) { + /** + * Cover case 1 + */ + case OperationType.Insert: + newIndexBuilder.addBlockIndex(operation.index.blockIndex! + 1); + break; + + /** + * Cover case 2 + */ + case OperationType.Delete: + if (operation.index.isBlockIndex) { + if (againstOp.index.blockIndex! < operation.index.blockIndex!) { + newIndexBuilder.addBlockIndex(operation.index.blockIndex! - 1); + } else { + return new Operation(OperationType.Neutral, newIndexBuilder.build(), { payload: [] }, operation.userId, operation.rev); + } + } + + break; + + /** + * Cover case 3 + */ + default: + return Operation.from(operation); + } + + /** + * Return new operation with the updated index + */ + return new Operation(operation.type, newIndexBuilder.build(), operation.data, operation.userId, operation.rev); + } + + /** + * Method that transforms operation against text operation + * + * Cases: + * 1. Current operation is a block operation + * - Text opearation cant affect block operation so return copy of the current one + * + * 2. Current operation is a text operation + * - Against operation is Insert + * - Transform current operation against textInsert + * - Against operation is Delete + * - Check that againstOp affects current operation and transform against text operation + * - Against operation is Modify or Neutral + * - Modify and Neutral operations do not affect any of the text operations so return copy of the current operation + * + * @param operation - Operation to be transformed + * @param againstOp - Operation against which the current operation should be transformed + * @returns New operation + */ + #transformAgainstTextOperation(operation: Operation, againstOp: Operation): Operation | Operation { + /** + * Cover case 1 + */ + if (operation.index.isBlockIndex) { + return Operation.from(operation); + } + + switch (againstOp.type) { + case OperationType.Insert: + return this.#transformAgainstTextInsert(operation, againstOp); + + case OperationType.Delete: + return this.#transformAgainstTextDelete(operation, againstOp); + + default: + return Operation.from(operation); + } + } + + /** + * Method that transforms operation against text insert operation happened on the left side of the current operation + * + * Cases: + * 1. Against operation is fully on the left of the current operation + * - Move text range of the current operation to the right by amount of inserted characters + * + * 2. Against operation is inside of the current operation text range + * - Move right bound of the current operation to the right by amount of inserted characters to include the inserted text + * + * @param operation - Operation to be transformed + * @param againstOp - Operation against which the current operation should be transformed + * @returns New operation + */ + #transformAgainstTextInsert(operation: Operation, againstOp: Operation): Operation | Operation { + const newIndexBuilder = new IndexBuilder().from(operation.index); + + const amountOfInsertedCharacters = againstOp.data.payload!.length; + const againstOpIsOnTheLeft = againstOp.index.textRange![0] < operation.index.textRange![0]; + const currentOpAgregatesAgainstOp = (operation.index.textRange![0] <= againstOp.index.textRange![0]) && (operation.index.textRange![1] >= againstOp.index.textRange![1]); + + /** + * Cover case 1 + */ + if (againstOpIsOnTheLeft) { + /** + * Move text index of the current operation to the right by amount of inserted characters + */ + newIndexBuilder.addTextRange([againstOp.index.textRange![0] + amountOfInsertedCharacters, againstOp.index.textRange![1] + amountOfInsertedCharacters]); + } + + /** + * Cover case 2 + */ + if (currentOpAgregatesAgainstOp) { + /** + * Move right bound of the current operation to the right by amount of inserted characters to include the inserted text + */ + newIndexBuilder.addTextRange([operation.index.textRange![0], operation.index.textRange![1] + amountOfInsertedCharacters]); + } + + return new Operation(operation.type, newIndexBuilder.build(), operation.data, operation.userId, operation.rev); + } + + /** + * Method that transforms operation against text delete operation + * + * Cases: + * 1. Delete range is fully on the left of the current operation + * - Move text range of the current operation to the left by amount of deleted characters + * + * 2. Delete range covers part of the current operation + * - Deleted left side of the current operation + * - Move left bound of the current operation to the start of the against Delete operation + * - Move right bound of the current operation to the left by (amount of deleted characters - amount of characters in the current operation that were deleted) + * - Deleted right side of the current operation + * - Move right bound of the current operation to the left by amount of deleted intersection + * + * 3. Delete range is inside of the current operation text range + * - Move right bound of the current operation to the left by amount of deleted characters + * + * 4. Delete range fully covers the current operation text rannge + * - Return Neutral operation + * + * @param operation - Operation to be transformed + * @param againstOp - Operation against which the current operation should be transformed + * @returns New operation + */ + #transformAgainstTextDelete(operation: Operation, againstOp: Operation): Operation | Operation { + const newIndexBuilder = new IndexBuilder().from(operation.index); + + const deletedAmount = againstOp.data.payload!.length; + + const deleteIsOnTheLeft = againstOp.index.textRange![1] < operation.index.textRange![0]; + + const deletedLeftSide = (againstOp.index.textRange![0] <= operation.index.textRange![0]) + && (againstOp.index.textRange![1] < operation.index.textRange![1]) + && (againstOp.index.textRange![1] > operation.index.textRange![0]); + + const deletedRightSide = (againstOp.index.textRange![0] > operation.index.textRange![0]) + && (againstOp.index.textRange![0] < operation.index.textRange![1]) + && (againstOp.index.textRange![1] <= operation.index.textRange![1]); + + const deletedInside = (againstOp.index.textRange![0] > operation.index.textRange![0]) + && (againstOp.index.textRange![1] < operation.index.textRange![1]); + + const deletedFull = (againstOp.index.textRange![0] <= operation.index.textRange![0]) + && (againstOp.index.textRange![1] >= operation.index.textRange![1]); + + /** + * Cover case 1 + */ + if (deleteIsOnTheLeft) { + newIndexBuilder.addTextRange([operation.index.textRange![0] - deletedAmount, operation.index.textRange![1] - deletedAmount]); + } + + /** + * Cover case 2.1 + */ + if (deletedLeftSide) { + const deletedFromCurrentOpRange = operation.index.textRange![0] - againstOp.index.textRange![1]; + + newIndexBuilder.addTextRange([againstOp.index.textRange![0], operation.index.textRange![1] - deletedFromCurrentOpRange]); + } + + /** + * Cover case 2.2 + */ + if (deletedRightSide) { + const deletedFromCurrentOpRange = operation.index.textRange![1] - againstOp.index.textRange![0]; + + newIndexBuilder.addTextRange([operation.index.textRange![0], operation.index.textRange![1] - deletedFromCurrentOpRange]); + } + + /** + * Cover case 3 + */ + if (deletedInside) { + newIndexBuilder.addTextRange([operation.index.textRange![0], operation.index.textRange![1] - deletedAmount]); + } + + /** + * Cover case 4 + */ + if (deletedFull) { + return new Operation(OperationType.Neutral, newIndexBuilder.build(), { payload: [] }, operation.userId, operation.rev); + } + + /** + * Return new operation with updated index + */ + return new Operation(operation.type, newIndexBuilder.build(), operation.data, operation.userId, operation.rev); + } +} \ No newline at end of file From 3a48624c67e66dc039963fff34552274ded5a9c8 Mon Sep 17 00:00:00 2001 From: e11sy <130844513+e11sy@users.noreply.github.com> Date: Tue, 15 Apr 2025 15:59:03 +0200 Subject: [PATCH 03/15] feat(collaboration-manager): determine intersection type util --- .../src/utils/getRangesIntersectionType.ts | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 packages/collaboration-manager/src/utils/getRangesIntersectionType.ts diff --git a/packages/collaboration-manager/src/utils/getRangesIntersectionType.ts b/packages/collaboration-manager/src/utils/getRangesIntersectionType.ts new file mode 100644 index 00000000..9f9a24e4 --- /dev/null +++ b/packages/collaboration-manager/src/utils/getRangesIntersectionType.ts @@ -0,0 +1,49 @@ +import { TextRange } from "@editorjs/model"; + +/** + * Represents the type of intersection between two text ranges. + */ +export enum RangeIntersectionType { + Left = 'left', + Right = 'right', + Includes = 'includes', + Included = 'included', + None = 'none', +} + +export function getRangesIntersectionType(range: TextRange, rangeToCompare: TextRange): RangeIntersectionType { + const [start, end] = range; + const [startToCompare, endToCompare] = rangeToCompare; + + /** + * Range is fully on the left or right of the range to compare + */ + if (end <= startToCompare || start >= endToCompare) { + return RangeIntersectionType.None; + } + + /** + * Range includes the range to compare + * If two ranges are the same, intersection type is "includes" + */ + if (start <= startToCompare && end >= endToCompare) { + return RangeIntersectionType.Includes; + } + + /** + * Range is included in the range to compare + */ + if (start > startToCompare && end < endToCompare) { + return RangeIntersectionType.Included; + } + + /** + * Right side of the range intersects with left side of the range to compare + * Cases with includes and included are handled before + */ + if (end > startToCompare && end < endToCompare) { + return RangeIntersectionType.Right; + } + + return RangeIntersectionType.Left; +} \ No newline at end of file From f2be325c1e82cc82842b0c3b39bf6dc78af722c7 Mon Sep 17 00:00:00 2001 From: e11sy <130844513+e11sy@users.noreply.github.com> Date: Tue, 15 Apr 2025 15:59:42 +0200 Subject: [PATCH 04/15] chore(): renaming --- .../src/BatchedOperation.ts | 34 +++++++++---------- .../src/CollaborationManager.ts | 15 ++++---- .../src/OperationsTransformer.ts | 4 +-- .../src/UndoRedoManager.ts | 17 +++++++--- 4 files changed, 38 insertions(+), 32 deletions(-) diff --git a/packages/collaboration-manager/src/BatchedOperation.ts b/packages/collaboration-manager/src/BatchedOperation.ts index d2544004..e0a2fd54 100644 --- a/packages/collaboration-manager/src/BatchedOperation.ts +++ b/packages/collaboration-manager/src/BatchedOperation.ts @@ -10,7 +10,7 @@ const DEBOUNCE_TIMEOUT = 500; * * Operations are batched on timeout basis or if batch is terminated from the outside */ -export class OperationsBatch extends Operation { +export class BatchedOperation extends Operation { /** * Array of operations to batch */ @@ -44,26 +44,26 @@ export class OperationsBatch extends Operation { * * @param opBatch - operation batch to clone */ - public static from(opBatch: OperationsBatch): OperationsBatch; + public static from(opBatch: BatchedOperation): BatchedOperation; /** * Create a new operation batch from a serialized operation * * @param json - serialized operation */ - public static from(json: SerializedOperation): OperationsBatch; + public static from(json: SerializedOperation): BatchedOperation; /** * Create a new operation batch from an operation batch or a serialized operation * * @param opBatchOrJSON - operation batch or serialized operation */ - public static from(opBatchOrJSON: OperationsBatch | SerializedOperation): OperationsBatch { - if (opBatchOrJSON instanceof OperationsBatch) { + public static from(opBatchOrJSON: BatchedOperation | SerializedOperation): BatchedOperation { + if (opBatchOrJSON instanceof BatchedOperation) { /** * Every batch should have at least one operation */ - const batch = new OperationsBatch(opBatchOrJSON.operations.shift()!); + const batch = new BatchedOperation(opBatchOrJSON.operations.shift()!); opBatchOrJSON.operations.forEach((op) => { /** @@ -72,9 +72,9 @@ export class OperationsBatch extends Operation { batch.add(Operation.from(op)); }); - return batch as OperationsBatch; + return batch as BatchedOperation; } else { - const batch = new OperationsBatch(Operation.from(opBatchOrJSON)); + const batch = new BatchedOperation(Operation.from(opBatchOrJSON)); return batch; } @@ -85,19 +85,19 @@ export class OperationsBatch extends Operation { * * @returns new batch with inversed operations */ - public inverse(): OperationsBatch> { + public inverse(): BatchedOperation> { /** * Every batch should have at least one operation */ - const newOperationsBatch = new OperationsBatch | OperationType.Neutral>(this.operations.pop()!.inverse()) + const newBatchedOperation = new BatchedOperation | OperationType.Neutral>(this.operations.pop()!.inverse()) while (this.operations.length > 0) { const op = this.operations.pop()!.inverse(); - newOperationsBatch.add(op); + newBatchedOperation.add(op); } - return newOperationsBatch as OperationsBatch>; + return newBatchedOperation as BatchedOperation>; } /** @@ -106,21 +106,21 @@ export class OperationsBatch extends Operation { * @param againstOp - operation to transform against * @returns new batch with transformed operations */ - public transform(againstOp: Operation): OperationsBatch { + public transform(againstOp: Operation): BatchedOperation { const transformedOp = this.operations.shift()!.transform(againstOp); - const newOperationsBatch = new OperationsBatch(transformedOp); - + const newBatchedOperation = new BatchedOperation(transformedOp); + /** * We either have a new operations batch or all operations were not transformable */ for (const op of this.operations) { const transformedOp = op.transform(againstOp); - newOperationsBatch.add(transformedOp); + newBatchedOperation.add(transformedOp); } - return newOperationsBatch; + return newBatchedOperation; } /** diff --git a/packages/collaboration-manager/src/CollaborationManager.ts b/packages/collaboration-manager/src/CollaborationManager.ts index e427fbbe..67599aae 100644 --- a/packages/collaboration-manager/src/CollaborationManager.ts +++ b/packages/collaboration-manager/src/CollaborationManager.ts @@ -10,7 +10,7 @@ import { } from '@editorjs/model'; import type { CoreConfig } from '@editorjs/sdk'; import { OTClient } from './client/index.js'; -import { OperationsBatch } from './BatchedOperation.js'; +import { BatchedOperation } from './BatchedOperation.js'; import { type ModifyOperationData, Operation, OperationType } from './Operation.js'; import { UndoRedoManager } from './UndoRedoManager.js'; @@ -36,7 +36,7 @@ export class CollaborationManager { /** * Current operations batch */ - #currentBatch: OperationsBatch | null = null; + #currentBatch: BatchedOperation | null = null; /** * Editor's config @@ -106,7 +106,7 @@ export class CollaborationManager { // Disable handling this.#shouldHandleEvents = false; - if (operation instanceof OperationsBatch) { + if (operation instanceof BatchedOperation) { operation.operations.forEach((op) => { this.applyOperation(op); }); @@ -137,7 +137,7 @@ export class CollaborationManager { // Disable event handling this.#shouldHandleEvents = false; - if (operation instanceof OperationsBatch) { + if (operation instanceof BatchedOperation) { operation.operations.forEach((op) => { this.applyOperation(op); }); @@ -240,8 +240,7 @@ export class CollaborationManager { /** * If operation is remote, transform undo/redo stacks */ - this.#undoRedoManager.transformUndoStack(operation); - this.#undoRedoManager.transformRedoStack(operation); + this.#undoRedoManager.transformStacks(operation); /** * If we got a new remote operation - transform current batch @@ -260,7 +259,7 @@ export class CollaborationManager { * If there is no current batch, create a new one with current operation */ if (this.#currentBatch === null) { - this.#currentBatch = new OperationsBatch(operation); + this.#currentBatch = new BatchedOperation(operation); return; } @@ -271,7 +270,7 @@ export class CollaborationManager { if (!this.#currentBatch.canAdd(operation)) { this.#undoRedoManager.put(this.#currentBatch); - this.#currentBatch = new OperationsBatch(operation); + this.#currentBatch = new BatchedOperation(operation); return; } diff --git a/packages/collaboration-manager/src/OperationsTransformer.ts b/packages/collaboration-manager/src/OperationsTransformer.ts index 471379fb..7b47c624 100644 --- a/packages/collaboration-manager/src/OperationsTransformer.ts +++ b/packages/collaboration-manager/src/OperationsTransformer.ts @@ -15,7 +15,7 @@ export class OperationsTransformer { return Operation.from(operation); } - return this.#determineTransformation(operation, againstOp); + return this.#applyTransformation(operation, againstOp); } /** @@ -37,7 +37,7 @@ export class OperationsTransformer { * @param againstOp - operation against which the current operation should be transformed * @returns new operation */ - #determineTransformation(operation: Operation, againstOp: Operation): Operation | Operation { + #applyTransformation(operation: Operation, againstOp: Operation): Operation | Operation { const currentIndex = operation.index; const againstIndex = againstOp.index; diff --git a/packages/collaboration-manager/src/UndoRedoManager.ts b/packages/collaboration-manager/src/UndoRedoManager.ts index 0ab424e3..8d0c802c 100644 --- a/packages/collaboration-manager/src/UndoRedoManager.ts +++ b/packages/collaboration-manager/src/UndoRedoManager.ts @@ -58,14 +58,22 @@ export class UndoRedoManager { this.#redoStack = []; } - public transformUndoStack(operation: Operation): void { + /** + * Transforms undo and redo stacks + * + * @param operation - operation to transform against + */ + public transformStacks(operation: Operation): void { this.transformStack(operation, this.#undoStack); - } - - public transformRedoStack(operation: Operation): void { this.transformStack(operation, this.#redoStack); } + /** + * Transforms passed operations stack against the operation + * + * @param operation - operation to transform against + * @param stack - stack to transform + */ public transformStack(operation: Operation, stack: Operation[]): void { const transformed = stack.flatMap((op) => { const transformedOp = op.transform(operation); @@ -79,6 +87,5 @@ export class UndoRedoManager { stack.length = 0; stack.push(...transformed); - } } From 2a339e13e42ed7a389545c28dc92c073817ef396 Mon Sep 17 00:00:00 2001 From: e11sy <130844513+e11sy@users.noreply.github.com> Date: Fri, 18 Apr 2025 20:52:48 +0300 Subject: [PATCH 05/15] imp(collaboration-manager): operationsTransformer improved --- .../src/OperationsTransformer.ts | 200 +++++++++--------- .../src/utils/getRangesIntersectionType.ts | 7 + 2 files changed, 103 insertions(+), 104 deletions(-) diff --git a/packages/collaboration-manager/src/OperationsTransformer.ts b/packages/collaboration-manager/src/OperationsTransformer.ts index 7b47c624..5bae21c1 100644 --- a/packages/collaboration-manager/src/OperationsTransformer.ts +++ b/packages/collaboration-manager/src/OperationsTransformer.ts @@ -1,5 +1,6 @@ import { IndexBuilder } from "@editorjs/model"; import { Operation, OperationType } from "./Operation"; +import { getRangesIntersectionType, RangeIntersectionType } from "./utils/getRangesIntersectionType"; /** * Class that transforms operation against another operation @@ -19,63 +20,40 @@ export class OperationsTransformer { } /** - * Method that desides what kind of transformation should be applied to the operation + * Method that returns new operation based on the type of againstOp index * Cases: - * 1. Against operations is a block operation and current operation is also a block operation - * - check that againstOp affects current operation and transform against block operation + * 1. Against operation is a block operation and current operation is also a block operation + * - check if againstOp affects current operation, update operation's block index * * 2. Against operation is a block operation and current operation is a text operation - * - same as above, check that againstOp affects current operation and transform against block operation + * - same as above, check if againstOp affects current operation and update operation's block index * * 3. Against operation is a text operation and current operation is a block operation * - text operation does not afftect block operation - so return copy of current operation * * 4. Against operation is a text operation and current operation is also a text operation - * - check that againstOp affects current operation and transform against text operation + * - check if againstOp affects current operation and update operation's text index * * @param operation - operation to be transformed * @param againstOp - operation against which the current operation should be transformed * @returns new operation */ #applyTransformation(operation: Operation, againstOp: Operation): Operation | Operation { - const currentIndex = operation.index; const againstIndex = againstOp.index; - /** - * Cover 1 and 2 cases - * - * Check that againstOp is a block operation - */ - if (againstIndex.isBlockIndex && currentIndex.blockIndex !== undefined) { - /** - * Check that againstOp affects current operation - */ - if (againstIndex.blockIndex! <= currentIndex.blockIndex) { - return this.#transformAgainstBlockOperation(operation, againstOp); - } - } + switch (true) { + case (againstIndex.isBlockIndex): + return this.#transformAgainstBlockOperation(operation, againstOp); + + case (againstIndex.isTextIndex): + return this.#transformAgainstTextOperation(operation, againstOp); - /** - * Cover 4 case - * - * Check that againstOp is a text operation and current operation is also a text operation - */ - if (againstIndex.isTextIndex && currentIndex.isTextIndex) { /** - * Check that againstOp affects current operation (text operation on the same block and same input) - * and against op happened on the left side or has overlapping range + * @todo Cover all index types */ - if (currentIndex.dataKey === againstIndex.dataKey && currentIndex.blockIndex === againstIndex.blockIndex && againstIndex.textRange![0] <= currentIndex.textRange![0]) { - return this.#transformAgainstTextOperation(operation, againstOp); - } + default: + throw new Error('Unsupported index type'); } - - /** - * Cover 3 case - * - * Return copy of current operation - */ - return Operation.from(operation); } /** @@ -100,7 +78,21 @@ export class OperationsTransformer { */ #transformAgainstBlockOperation(operation: Operation, againstOp: Operation): Operation | Operation { const newIndexBuilder = new IndexBuilder().from(operation.index); - + + /** + * If current operation has no block index, return copy of the current operation + */ + if (!operation.index.isBlockIndex) { + return Operation.from(operation); + } + + /** + * Check that againstOp affects current operation + */ + if (againstOp.index.blockIndex! <= operation.index.blockIndex!) { + return Operation.from(operation); + } + /** * Update the index of the current operation */ @@ -136,7 +128,11 @@ export class OperationsTransformer { /** * Return new operation with the updated index */ - return new Operation(operation.type, newIndexBuilder.build(), operation.data, operation.userId, operation.rev); + const newOp = Operation.from(operation); + + newOp.index = newIndexBuilder.build(); + + return newOp; } /** @@ -195,31 +191,36 @@ export class OperationsTransformer { #transformAgainstTextInsert(operation: Operation, againstOp: Operation): Operation | Operation { const newIndexBuilder = new IndexBuilder().from(operation.index); - const amountOfInsertedCharacters = againstOp.data.payload!.length; - const againstOpIsOnTheLeft = againstOp.index.textRange![0] < operation.index.textRange![0]; - const currentOpAgregatesAgainstOp = (operation.index.textRange![0] <= againstOp.index.textRange![0]) && (operation.index.textRange![1] >= againstOp.index.textRange![1]); + const insertedLength = againstOp.data.payload!.length; + + const index = operation.index; + const againstIndex = againstOp.index; /** - * Cover case 1 + * In this case, againstOp is insert operatioin, there would be only two possible intersections + * - None - inserted text is on the left side of the current operation + * - Includes - inserted text is inside of the current operation text range */ - if (againstOpIsOnTheLeft) { - /** - * Move text index of the current operation to the right by amount of inserted characters - */ - newIndexBuilder.addTextRange([againstOp.index.textRange![0] + amountOfInsertedCharacters, againstOp.index.textRange![1] + amountOfInsertedCharacters]); + const intersectionType = getRangesIntersectionType(index.textRange!, againstIndex.textRange!); + + switch (intersectionType) { + case (RangeIntersectionType.None): + newIndexBuilder.addTextRange([index.textRange![0] + insertedLength, index.textRange![1] + insertedLength]); + break; + + case (RangeIntersectionType.Includes): + newIndexBuilder.addTextRange([index.textRange![0], index.textRange![1] + insertedLength]); + break; } /** - * Cover case 2 + * Return new operation with the updated index */ - if (currentOpAgregatesAgainstOp) { - /** - * Move right bound of the current operation to the right by amount of inserted characters to include the inserted text - */ - newIndexBuilder.addTextRange([operation.index.textRange![0], operation.index.textRange![1] + amountOfInsertedCharacters]); - } + const newOp = Operation.from(operation); - return new Operation(operation.type, newIndexBuilder.build(), operation.data, operation.userId, operation.rev); + newOp.index = newIndexBuilder.build(); + + return newOp; } /** @@ -248,67 +249,58 @@ export class OperationsTransformer { */ #transformAgainstTextDelete(operation: Operation, againstOp: Operation): Operation | Operation { const newIndexBuilder = new IndexBuilder().from(operation.index); - const deletedAmount = againstOp.data.payload!.length; - const deleteIsOnTheLeft = againstOp.index.textRange![1] < operation.index.textRange![0]; - - const deletedLeftSide = (againstOp.index.textRange![0] <= operation.index.textRange![0]) - && (againstOp.index.textRange![1] < operation.index.textRange![1]) - && (againstOp.index.textRange![1] > operation.index.textRange![0]); - - const deletedRightSide = (againstOp.index.textRange![0] > operation.index.textRange![0]) - && (againstOp.index.textRange![0] < operation.index.textRange![1]) - && (againstOp.index.textRange![1] <= operation.index.textRange![1]); - - const deletedInside = (againstOp.index.textRange![0] > operation.index.textRange![0]) - && (againstOp.index.textRange![1] < operation.index.textRange![1]); + const index = operation.index; + const againstIndex = againstOp.index; + + const intersectionType = getRangesIntersectionType(index.textRange!, againstIndex.textRange!); - const deletedFull = (againstOp.index.textRange![0] <= operation.index.textRange![0]) - && (againstOp.index.textRange![1] >= operation.index.textRange![1]); + switch (intersectionType) { + /** + * Cover case 1 + */ + case (RangeIntersectionType.None): + newIndexBuilder.addTextRange([index.textRange![0] - deletedAmount, index.textRange![1] - deletedAmount]); + break; - /** - * Cover case 1 - */ - if (deleteIsOnTheLeft) { - newIndexBuilder.addTextRange([operation.index.textRange![0] - deletedAmount, operation.index.textRange![1] - deletedAmount]); - } + /** + * Cover case 2.1 + */ + case (RangeIntersectionType.Left): + newIndexBuilder.addTextRange([againstIndex.textRange![0], index.textRange![1] - deletedAmount]); + break; - /** - * Cover case 2.1 - */ - if (deletedLeftSide) { - const deletedFromCurrentOpRange = operation.index.textRange![0] - againstOp.index.textRange![1]; + /** + * Cover case 2.2 + */ + case (RangeIntersectionType.Right): + const overlapLength = index.textRange![1] - againstIndex.textRange![0]; - newIndexBuilder.addTextRange([againstOp.index.textRange![0], operation.index.textRange![1] - deletedFromCurrentOpRange]); - } + newIndexBuilder.addTextRange([index.textRange![0], index.textRange![1] - overlapLength]); + break; - /** - * Cover case 2.2 - */ - if (deletedRightSide) { - const deletedFromCurrentOpRange = operation.index.textRange![1] - againstOp.index.textRange![0]; + /** + * Cover case 3 + */ + case (RangeIntersectionType.Includes): + newIndexBuilder.addTextRange([index.textRange![0], index.textRange![1] - deletedAmount]); + break; - newIndexBuilder.addTextRange([operation.index.textRange![0], operation.index.textRange![1] - deletedFromCurrentOpRange]); + /** + * Cover case 4 + */ + case (RangeIntersectionType.Included): + return new Operation(OperationType.Neutral, newIndexBuilder.build(), { payload: [] }, operation.userId, operation.rev); } /** - * Cover case 3 + * Return new operation with updated index */ - if (deletedInside) { - newIndexBuilder.addTextRange([operation.index.textRange![0], operation.index.textRange![1] - deletedAmount]); - } + const newOp = Operation.from(operation); - /** - * Cover case 4 - */ - if (deletedFull) { - return new Operation(OperationType.Neutral, newIndexBuilder.build(), { payload: [] }, operation.userId, operation.rev); - } + newOp.index = newIndexBuilder.build(); - /** - * Return new operation with updated index - */ - return new Operation(operation.type, newIndexBuilder.build(), operation.data, operation.userId, operation.rev); + return newOp; } } \ No newline at end of file diff --git a/packages/collaboration-manager/src/utils/getRangesIntersectionType.ts b/packages/collaboration-manager/src/utils/getRangesIntersectionType.ts index 9f9a24e4..282fa3ee 100644 --- a/packages/collaboration-manager/src/utils/getRangesIntersectionType.ts +++ b/packages/collaboration-manager/src/utils/getRangesIntersectionType.ts @@ -11,6 +11,13 @@ export enum RangeIntersectionType { None = 'none', } +/** + * Returns the type of intersection between two text ranges + * + * @param range - Range to check + * @param rangeToCompare - Range to compare with + * @returns Type of intersection + */ export function getRangesIntersectionType(range: TextRange, rangeToCompare: TextRange): RangeIntersectionType { const [start, end] = range; const [startToCompare, endToCompare] = rangeToCompare; From 59e8c4f4e0e5cd76c30896dc6896c97adeea9d9c Mon Sep 17 00:00:00 2001 From: e11sy <130844513+e11sy@users.noreply.github.com> Date: Fri, 18 Apr 2025 21:45:00 +0300 Subject: [PATCH 06/15] imp(batchedOp): improvements --- .../src/BatchedOperation.ts | 46 ++++++------------- .../src/CollaborationManager.ts | 45 ++++++++++-------- .../src/OperationsTransformer.ts | 12 ++++- 3 files changed, 52 insertions(+), 51 deletions(-) diff --git a/packages/collaboration-manager/src/BatchedOperation.ts b/packages/collaboration-manager/src/BatchedOperation.ts index e0a2fd54..d3623f8a 100644 --- a/packages/collaboration-manager/src/BatchedOperation.ts +++ b/packages/collaboration-manager/src/BatchedOperation.ts @@ -1,16 +1,11 @@ -import { InvertedOperationType, Operation, OperationType, SerializedOperation } from './Operation.js'; - -/** - * Batch debounce time - */ -const DEBOUNCE_TIMEOUT = 500; +import { InvertedOperationType, Operation, OperationType, type SerializedOperation } from './Operation.js'; /** * Class to batch Text operations (maybe others in the future) for Undo/Redo purposes * * Operations are batched on timeout basis or if batch is terminated from the outside */ -export class BatchedOperation extends Operation { +export class BatchedOperation extends Operation { /** * Array of operations to batch */ @@ -29,16 +24,6 @@ export class BatchedOperation extends Operation { } } - /** - * Adds an operation to the batch - * Make sure, that operation could be added to the batch - * - * @param op - operation to add - */ - public add(op: Operation | Operation): void { - this.operations.push(op); - } - /** * Create a new operation batch from an array of operations * @@ -63,7 +48,7 @@ export class BatchedOperation extends Operation { /** * Every batch should have at least one operation */ - const batch = new BatchedOperation(opBatchOrJSON.operations.shift()!); + const batch = new BatchedOperation(Operation.from(opBatchOrJSON.operations.shift()!)); opBatchOrJSON.operations.forEach((op) => { /** @@ -80,6 +65,16 @@ export class BatchedOperation extends Operation { } } + /** + * Adds an operation to the batch + * Make sure, that operation could be added to the batch + * + * @param op - operation to add + */ + public add(op: Operation | Operation): void { + this.operations.push(op); + } + /** * Method that inverses all of the operations in the batch * @@ -91,11 +86,7 @@ export class BatchedOperation extends Operation { */ const newBatchedOperation = new BatchedOperation | OperationType.Neutral>(this.operations.pop()!.inverse()) - while (this.operations.length > 0) { - const op = this.operations.pop()!.inverse(); - - newBatchedOperation.add(op); - } + this.operations.toReversed().map(op => newBatchedOperation.add(op.inverse())); return newBatchedOperation as BatchedOperation>; } @@ -110,15 +101,8 @@ export class BatchedOperation extends Operation { const transformedOp = this.operations.shift()!.transform(againstOp); const newBatchedOperation = new BatchedOperation(transformedOp); - - /** - * We either have a new operations batch or all operations were not transformable - */ - for (const op of this.operations) { - const transformedOp = op.transform(againstOp); - newBatchedOperation.add(transformedOp); - } + this.operations.map(op => newBatchedOperation.add(op.transform(againstOp))); return newBatchedOperation; } diff --git a/packages/collaboration-manager/src/CollaborationManager.ts b/packages/collaboration-manager/src/CollaborationManager.ts index 67599aae..848b5735 100644 --- a/packages/collaboration-manager/src/CollaborationManager.ts +++ b/packages/collaboration-manager/src/CollaborationManager.ts @@ -91,11 +91,7 @@ export class CollaborationManager { * Undo last operation in the local stack */ public undo(): void { - if (this.#currentBatch !== null) { - this.#undoRedoManager.put(this.#currentBatch); - - this.#currentBatch = null; - } + this.#emptyBatch(); const operation = this.#undoRedoManager.undo(); @@ -106,13 +102,7 @@ export class CollaborationManager { // Disable handling this.#shouldHandleEvents = false; - if (operation instanceof BatchedOperation) { - operation.operations.forEach((op) => { - this.applyOperation(op); - }); - } else { - this.applyOperation(operation); - } + this.applyOperation(operation); // Re-enable event handling this.#shouldHandleEvents = true; @@ -122,11 +112,7 @@ export class CollaborationManager { * Redo last undone operation in the local stack */ public redo(): void { - if (this.#currentBatch !== null) { - this.#undoRedoManager.put(this.#currentBatch); - - this.#currentBatch = null; - } + this.#emptyBatch(); const operation = this.#undoRedoManager.redo(); @@ -154,7 +140,16 @@ export class CollaborationManager { * * @param operation - operation to apply */ - public applyOperation(operation: Operation): void { + public applyOperation(operation: Operation | BatchedOperation): void { + /** + * If operation is a batcher operation, apply all operations in the batch + */ + if (operation instanceof BatchedOperation) { + operation.operations.forEach(op => this.applyOperation(op)); + + return; + } + if (operation.type === OperationType.Neutral) { return; } @@ -244,7 +239,7 @@ export class CollaborationManager { /** * If we got a new remote operation - transform current batch - * If batch is not transormable - clear it + * If batch is empty leave it as it is */ this.#currentBatch = this.#currentBatch?.transform(operation) ?? null; @@ -266,6 +261,7 @@ export class CollaborationManager { /** * If current operation could not be added to the batch, then terminate current batch and create a new one with current operation + * @todo - add debounce timeout 500ms */ if (!this.#currentBatch.canAdd(operation)) { this.#undoRedoManager.put(this.#currentBatch); @@ -277,4 +273,15 @@ export class CollaborationManager { this.#currentBatch.add(operation); } + + /** + * Puts current batch to the undo stack and clears the batch + */ + #emptyBatch(): void { + if (this.#currentBatch !== null) { + this.#undoRedoManager.put(this.#currentBatch); + + this.#currentBatch = null; + } + } } diff --git a/packages/collaboration-manager/src/OperationsTransformer.ts b/packages/collaboration-manager/src/OperationsTransformer.ts index 5bae21c1..db1b2e8b 100644 --- a/packages/collaboration-manager/src/OperationsTransformer.ts +++ b/packages/collaboration-manager/src/OperationsTransformer.ts @@ -155,10 +155,20 @@ export class OperationsTransformer { * @returns New operation */ #transformAgainstTextOperation(operation: Operation, againstOp: Operation): Operation | Operation { + const index = operation.index; + const againstIndex = againstOp.index; + /** * Cover case 1 */ - if (operation.index.isBlockIndex) { + if (index.isBlockIndex) { + return Operation.from(operation); + } + + /** + * Check that againstOp affects current operation + */ + if (index.dataKey === againstIndex.dataKey && index.blockIndex === againstIndex.blockIndex && againstIndex.textRange![0] > index.textRange![1]) { return Operation.from(operation); } From bb68221c3c2ddd155f72b5968260cf4b2b6661c0 Mon Sep 17 00:00:00 2001 From: e11sy <130844513+e11sy@users.noreply.github.com> Date: Sat, 26 Apr 2025 22:20:34 +0300 Subject: [PATCH 07/15] test(collaboration-manager): add tests for transformations --- .../src/BatchedOperation.spec.ts | 87 ++++-- .../src/BatchedOperation.ts | 48 ++-- .../src/CollaborationManager.spec.ts | 36 +-- .../src/CollaborationManager.ts | 1 + .../src/Operation.spec.ts | 241 ---------------- .../collaboration-manager/src/Operation.ts | 4 +- .../src/OperationsTransformer.spec.ts | 260 ++++++++++++++++++ .../src/OperationsTransformer.ts | 87 +++--- .../src/UndoRedoManager.spec.ts | 102 ++----- .../src/UndoRedoManager.ts | 15 +- .../src/utils/getRangesIntersectionType.ts | 6 +- 11 files changed, 450 insertions(+), 437 deletions(-) create mode 100644 packages/collaboration-manager/src/OperationsTransformer.spec.ts diff --git a/packages/collaboration-manager/src/BatchedOperation.spec.ts b/packages/collaboration-manager/src/BatchedOperation.spec.ts index b577e5ec..0249b809 100644 --- a/packages/collaboration-manager/src/BatchedOperation.spec.ts +++ b/packages/collaboration-manager/src/BatchedOperation.spec.ts @@ -1,6 +1,7 @@ -import { createDataKey, IndexBuilder } from '@editorjs/model'; -import { OperationsBatch } from './BatchedOperation.js'; -import { Operation, OperationType, SerializedOperation } from './Operation.js'; +import { createDataKey, Index, IndexBuilder } from '@editorjs/model'; +import { BatchedOperation } from './BatchedOperation.js'; +import type { SerializedOperation } from './Operation.js'; +import { Operation, OperationType } from './Operation.js'; const templateIndex = new IndexBuilder() .addBlockIndex(0) @@ -22,7 +23,7 @@ describe('Batch', () => { userId ); - const batch = new OperationsBatch(op1); + const batch = new BatchedOperation(op1); batch.add(op2); @@ -42,7 +43,7 @@ describe('Batch', () => { userId ); - const batch = new OperationsBatch(op1); + const batch = new BatchedOperation(op1); batch.add(op2); @@ -56,23 +57,30 @@ describe('Batch', () => { const op1 = new Operation(OperationType.Insert, templateIndex, { payload: 'a' }, userId); const op2 = new Operation( OperationType.Insert, - new IndexBuilder().from(templateIndex).addTextRange([1, 1]).build(), + new IndexBuilder().from(templateIndex) + .addTextRange([1, 1]) + .build(), { payload: 'b' }, userId ); - const originalBatch = new OperationsBatch(op1); + const originalBatch = new BatchedOperation(op1); + originalBatch.add(op2); - const newBatch = OperationsBatch.from(originalBatch); + + const newBatch = BatchedOperation.from(originalBatch); + console.log('original operations', originalBatch.operations); + + console.log('new operations', newBatch.operations) - expect(newBatch.operations).toEqual(originalBatch.operations); + expect(newBatch.operations).toStrictEqual(originalBatch.operations); expect(newBatch).not.toBe(originalBatch); // Should be a new instance }); it('should create a new batch from serialized operation', () => { const serializedOp: SerializedOperation = new Operation(OperationType.Delete, templateIndex, { payload: 'a' }, userId).serialize(); - const batch = OperationsBatch.from(serializedOp); + const batch = BatchedOperation.from(serializedOp); expect(batch.operations[0].type).toBe(serializedOp.type); expect(batch.operations[0].data).toEqual(serializedOp.data); @@ -84,11 +92,14 @@ describe('Batch', () => { const op1 = new Operation(OperationType.Insert, templateIndex, { payload: 'a' }, userId); const op2 = new Operation( OperationType.Insert, - new IndexBuilder().from(templateIndex).addTextRange([1, 1]).build(), + new IndexBuilder().from(templateIndex) + .addTextRange([1, 1]) + .build(), { payload: 'b' }, userId ); - const batch = new OperationsBatch(op1); + const batch = new BatchedOperation(op1); + batch.add(op2); const inversedBatch = batch.inverse(); @@ -103,16 +114,21 @@ describe('Batch', () => { const op1 = new Operation(OperationType.Insert, templateIndex, { payload: 'a' }, userId); const op2 = new Operation( OperationType.Insert, - new IndexBuilder().from(templateIndex).addTextRange([1, 1]).build(), + new IndexBuilder().from(templateIndex) + .addTextRange([1, 1]) + .build(), { payload: 'b' }, userId ); - const batch = new OperationsBatch(op1); + const batch = new BatchedOperation(op1); + batch.add(op2); const againstOp = new Operation( OperationType.Insert, - new IndexBuilder().from(templateIndex).addTextRange([0, 0]).build(), + new IndexBuilder().from(templateIndex) + .addTextRange([0, 0]) + .build(), { payload: 'x' }, 'other-user' ); @@ -126,16 +142,23 @@ describe('Batch', () => { expect(transformedBatch!.operations[1].index.textRange![0]).toBe(2); }); - it('should return null if no operations can be transformed', () => { + it('should return batch with Neutral operations if no operations can be transformed', () => { const op = new Operation(OperationType.Insert, templateIndex, { payload: 'a' }, userId); - const batch = new OperationsBatch(op); - + const batch = new BatchedOperation(op); + + const deleteIndex = new IndexBuilder() + .from(templateIndex) + .addTextRange([0, 2]) + .build(); + // An operation that would make transformation impossible - const againstOp = new Operation(OperationType.Delete, templateIndex, { payload: 'a' }, 'other-user'); + const againstOp = new Operation(OperationType.Delete, deleteIndex, { payload: 'a' }, 'other-user'); const transformedBatch = batch.transform(againstOp); - expect(transformedBatch).toBeNull(); + const neutralOp = new Operation(OperationType.Neutral, templateIndex, { payload: 'a' }, userId) + + expect(transformedBatch.operations[0]).toEqual(neutralOp); }); }); @@ -144,11 +167,13 @@ describe('Batch', () => { const op1 = new Operation(OperationType.Insert, templateIndex, { payload: 'a' }, userId); const op2 = new Operation( OperationType.Insert, - new IndexBuilder().from(templateIndex).addTextRange([1, 1]).build(), + new IndexBuilder().from(templateIndex) + .addTextRange([1, 1]) + .build(), { payload: 'b' }, userId ); - const batch = new OperationsBatch(op1); + const batch = new BatchedOperation(op1); expect(batch.canAdd(op2)).toBe(true); }); @@ -157,11 +182,13 @@ describe('Batch', () => { const op1 = new Operation(OperationType.Insert, templateIndex, { payload: 'a' }, userId); const op2 = new Operation( OperationType.Insert, - new IndexBuilder().from(templateIndex).addTextRange([2, 2]).build(), + new IndexBuilder().from(templateIndex) + .addTextRange([2, 2]) + .build(), { payload: 'b' }, userId ); - const batch = new OperationsBatch(op1); + const batch = new BatchedOperation(op1); expect(batch.canAdd(op2)).toBe(false); }); @@ -170,11 +197,13 @@ describe('Batch', () => { const op1 = new Operation(OperationType.Insert, templateIndex, { payload: 'a' }, userId); const op2 = new Operation( OperationType.Delete, - new IndexBuilder().from(templateIndex).addTextRange([1, 1]).build(), + new IndexBuilder().from(templateIndex) + .addTextRange([1, 1]) + .build(), { payload: 'b' }, userId ); - const batch = new OperationsBatch(op1); + const batch = new BatchedOperation(op1); expect(batch.canAdd(op2)).toBe(false); }); @@ -183,11 +212,13 @@ describe('Batch', () => { const op1 = new Operation(OperationType.Insert, templateIndex, { payload: 'a' }, userId); const op2 = new Operation( OperationType.Modify, - new IndexBuilder().from(templateIndex).addTextRange([1, 1]).build(), + new IndexBuilder().from(templateIndex) + .addTextRange([1, 1]) + .build(), { payload: { tool: 'bold' } }, userId ); - const batch = new OperationsBatch(op1); + const batch = new BatchedOperation(op1); expect(batch.canAdd(op2)).toBe(false); }); diff --git a/packages/collaboration-manager/src/BatchedOperation.ts b/packages/collaboration-manager/src/BatchedOperation.ts index d3623f8a..c70972e2 100644 --- a/packages/collaboration-manager/src/BatchedOperation.ts +++ b/packages/collaboration-manager/src/BatchedOperation.ts @@ -1,15 +1,14 @@ -import { InvertedOperationType, Operation, OperationType, type SerializedOperation } from './Operation.js'; +import type { InvertedOperationType } from './Operation.js'; +import { Operation, OperationType, type SerializedOperation } from './Operation.js'; /** * Class to batch Text operations (maybe others in the future) for Undo/Redo purposes - * - * Operations are batched on timeout basis or if batch is terminated from the outside */ export class BatchedOperation extends Operation { /** * Array of operations to batch */ - operations: (Operation | Operation)[] = []; + public operations: (Operation | Operation)[] = []; /** * Batch constructor function @@ -26,42 +25,42 @@ export class BatchedOperation extends O /** * Create a new operation batch from an array of operations - * + * * @param opBatch - operation batch to clone - */ + */ public static from(opBatch: BatchedOperation): BatchedOperation; /** * Create a new operation batch from a serialized operation - * + * * @param json - serialized operation */ public static from(json: SerializedOperation): BatchedOperation; /** * Create a new operation batch from an operation batch or a serialized operation - * + * * @param opBatchOrJSON - operation batch or serialized operation - */ + */ public static from(opBatchOrJSON: BatchedOperation | SerializedOperation): BatchedOperation { if (opBatchOrJSON instanceof BatchedOperation) { /** * Every batch should have at least one operation */ - const batch = new BatchedOperation(Operation.from(opBatchOrJSON.operations.shift()!)); + const batch = new BatchedOperation(Operation.from(opBatchOrJSON.operations[0])); - opBatchOrJSON.operations.forEach((op) => { + opBatchOrJSON.operations.slice(1).forEach((op) => { /** * Deep clone operation to the new batch */ batch.add(Operation.from(op)); }); - + return batch as BatchedOperation; } else { const batch = new BatchedOperation(Operation.from(opBatchOrJSON)); - return batch; + return batch; } } @@ -78,15 +77,17 @@ export class BatchedOperation extends O /** * Method that inverses all of the operations in the batch * - * @returns new batch with inversed operations + * @returns {BatchedOperation>} new batch with inversed operations */ public inverse(): BatchedOperation> { + const lastOp = this.operations[this.operations.length - 1]; + /** * Every batch should have at least one operation */ - const newBatchedOperation = new BatchedOperation | OperationType.Neutral>(this.operations.pop()!.inverse()) + const newBatchedOperation = new BatchedOperation>(lastOp.inverse()); - this.operations.toReversed().map(op => newBatchedOperation.add(op.inverse())); + this.operations.toReversed().slice(1).map(op => newBatchedOperation.add(op.inverse())); return newBatchedOperation as BatchedOperation>; } @@ -95,14 +96,14 @@ export class BatchedOperation extends O * Method that transforms all of the operations in the batch against another operation * * @param againstOp - operation to transform against - * @returns new batch with transformed operations + * @returns {BatchedOperation} new batch with transformed operations */ public transform(againstOp: Operation): BatchedOperation { - const transformedOp = this.operations.shift()!.transform(againstOp); + const transformedOp = this.operations[0].transform(againstOp); const newBatchedOperation = new BatchedOperation(transformedOp); - this.operations.map(op => newBatchedOperation.add(op.transform(againstOp))); + this.operations.slice(1).map(op => newBatchedOperation.add(op.transform(againstOp))); return newBatchedOperation; } @@ -114,7 +115,14 @@ export class BatchedOperation extends O * * @param op - operation to check */ - canAdd(op: Operation): boolean { + public canAdd(op: Operation): boolean { + /** + * Can't add to batch insertion or deletion of several characters + */ + if (typeof op.data.payload === 'string' && op.data.payload?.length > 1) { + return false; + } + const lastOp = this.operations[this.operations.length - 1]; if (lastOp === undefined) { diff --git a/packages/collaboration-manager/src/CollaborationManager.spec.ts b/packages/collaboration-manager/src/CollaborationManager.spec.ts index e894976d..624e8bff 100644 --- a/packages/collaboration-manager/src/CollaborationManager.spec.ts +++ b/packages/collaboration-manager/src/CollaborationManager.spec.ts @@ -824,14 +824,14 @@ describe('CollaborationManager', () => { .addTextRange([0, 0]) .build(); const operation1 = new Operation(OperationType.Insert, index1, { - payload: 'te', + payload: 't', }, userId); const index2 = new IndexBuilder().from(index1) .addTextRange([1, 1]) .build(); const operation2 = new Operation(OperationType.Insert, index2, { - payload: 'st', + payload: 's', }, userId); collaborationManager.applyOperation(operation1); @@ -876,14 +876,14 @@ describe('CollaborationManager', () => { .addTextRange([0, 0]) .build(); const operation1 = new Operation(OperationType.Insert, index1, { - payload: 'te', + payload: 't', }, userId); const index2 = new IndexBuilder().from(index1) .addTextRange([1, 1]) .build(); const operation2 = new Operation(OperationType.Insert, index2, { - payload: 'st', + payload: 's', }, userId); collaborationManager.applyOperation(operation1); @@ -900,7 +900,7 @@ describe('CollaborationManager', () => { data: { text: { $t: 't', - value: 'test', + value: 'ts', fragments: [], }, }, @@ -949,8 +949,9 @@ describe('CollaborationManager', () => { describe('remote operations', () => { it('should transform current batch when remote operation arrives', () => { const model = new EditorJSModel(userId, { identifier: documentId }); + model.initializeDocument({ - blocks: [{ + blocks: [ { name: 'paragraph', data: { text: { @@ -958,7 +959,7 @@ describe('CollaborationManager', () => { $t: 't', }, }, - }], + } ], }); const collaborationManager = new CollaborationManager(config as Required, model); @@ -968,7 +969,7 @@ describe('CollaborationManager', () => { .addDataKey(createDataKey('text')) .addTextRange([0, 4]) .build(); - + const localOp = new Operation(OperationType.Insert, localIndex, { payload: 'test', }, userId); @@ -980,7 +981,7 @@ describe('CollaborationManager', () => { .addDataKey(createDataKey('text')) .addTextRange([0, 5]) .build(); - + const remoteOp = new Operation(OperationType.Insert, remoteIndex, { payload: 'hello', }, 'other-user'); @@ -990,7 +991,7 @@ describe('CollaborationManager', () => { // Verify the operations were transformed correctly expect(model.serialized).toStrictEqual({ identifier: documentId, - blocks: [{ + blocks: [ { name: 'paragraph', tunes: {}, data: { @@ -1000,15 +1001,16 @@ describe('CollaborationManager', () => { fragments: [], }, }, - }], + } ], properties: {}, }); }); it('should clear current batch if not transformable with remote operation', () => { const model = new EditorJSModel(userId, { identifier: documentId }); + model.initializeDocument({ - blocks: [{ + blocks: [ { name: 'paragraph', data: { text: { @@ -1016,7 +1018,7 @@ describe('CollaborationManager', () => { $t: 't', }, }, - }], + } ], }); const collaborationManager = new CollaborationManager(config as Required, model); @@ -1026,7 +1028,7 @@ describe('CollaborationManager', () => { .addDataKey(createDataKey('text')) .addTextRange([0, 7]) .build(); - + const localOp = new Operation(OperationType.Insert, localIndex, { payload: 'initial', }, userId); @@ -1038,7 +1040,7 @@ describe('CollaborationManager', () => { .addDataKey(createDataKey('text')) .addTextRange([0, 7]) .build(); - + const remoteOp = new Operation(OperationType.Delete, remoteIndex, { payload: 'initial', }, 'other-user'); @@ -1050,7 +1052,7 @@ describe('CollaborationManager', () => { expect(model.serialized).toStrictEqual({ identifier: documentId, - blocks: [{ + blocks: [ { name: 'paragraph', tunes: {}, data: { @@ -1060,7 +1062,7 @@ describe('CollaborationManager', () => { fragments: [], }, }, - }], + } ], properties: {}, }); }); diff --git a/packages/collaboration-manager/src/CollaborationManager.ts b/packages/collaboration-manager/src/CollaborationManager.ts index 848b5735..127af9d6 100644 --- a/packages/collaboration-manager/src/CollaborationManager.ts +++ b/packages/collaboration-manager/src/CollaborationManager.ts @@ -261,6 +261,7 @@ export class CollaborationManager { /** * If current operation could not be added to the batch, then terminate current batch and create a new one with current operation + * * @todo - add debounce timeout 500ms */ if (!this.#currentBatch.canAdd(operation)) { diff --git a/packages/collaboration-manager/src/Operation.spec.ts b/packages/collaboration-manager/src/Operation.spec.ts index 3c076c34..99ec161c 100644 --- a/packages/collaboration-manager/src/Operation.spec.ts +++ b/packages/collaboration-manager/src/Operation.spec.ts @@ -40,247 +40,6 @@ const createOperation = ( describe('Operation', () => { - describe('.transform()', () => { - it('should not change operation if document ids are different', () => { - const receivedOp = createOperation(OperationType.Insert, 0, 'abc'); - const localOp = createOperation(OperationType.Insert, 0, 'def'); - - localOp.index.documentId = 'document2' as DocumentIndex; - const transformedOp = receivedOp.transform(localOp); - - expect(transformedOp).toEqual(receivedOp); - }); - - it('should not change operation if data keys are different', () => { - const receivedOp = createOperation(OperationType.Insert, 0, 'abc'); - const localOp = createOperation(OperationType.Insert, 0, 'def'); - - localOp.index.dataKey = 'dataKey2' as DataKey; - - const transformedOp = receivedOp.transform(localOp); - - expect(transformedOp).toEqual(receivedOp); - }); - - it('should not change operation if operation is not Block or Text operation', () => { - const receivedOp = createOperation(OperationType.Insert, 0, 'abc'); - const localOp = createOperation(OperationType.Insert, 0, 'def'); - - localOp.index.textRange = undefined; - - const transformedOp = receivedOp.transform(localOp); - - expect(transformedOp).toEqual(receivedOp); - }); - - it('should throw an error if unsupported operation type is provided', () => { - const receivedOp = createOperation(OperationType.Insert, 0, 'def'); - // @ts-expect-error — for test purposes - const localOp = createOperation('unsupported', 0, 'def'); - - expect(() => receivedOp.transform(localOp)).toThrow('Unsupported operation type'); - }); - - it('should not transform relative to the Modify operation (as Modify operation doesn\'t change index)', () => { - const receivedOp = createOperation(OperationType.Insert, 0, 'abc'); - const localOp = createOperation(OperationType.Modify, 0, 'def'); - const transformedOp = receivedOp.transform(localOp); - - expect(transformedOp).toEqual(receivedOp); - }); - - describe('Transformation relative to Insert operation', () => { - it('should not change a received operation if it is before a local one', () => { - const receivedOp = createOperation(OperationType.Insert, 0, 'abc'); - const localOp = createOperation(OperationType.Insert, 3, 'def'); - const transformedOp = receivedOp.transform(localOp); - - expect(transformedOp).toEqual(receivedOp); - }); - - it('should transform an index for a received operation if it is after a local one', () => { - const receivedOp = createOperation(OperationType.Delete, 3, 'def'); - const localOp = createOperation(OperationType.Insert, 0, 'abc'); - const transformedOp = receivedOp.transform(localOp); - - expect(transformedOp?.index.textRange).toEqual([6, 6]); - }); - - it('should transform a received operation if it is at the same position as a local one', () => { - const receivedOp = createOperation(OperationType.Modify, 0, 'abc'); - const localOp = createOperation(OperationType.Insert, 0, 'def'); - const transformedOp = receivedOp.transform(localOp); - - expect(transformedOp?.index.textRange).toEqual([3, 3]); - }); - - it('should not change the text index if local op is a Block operation', () => { - const receivedOp = createOperation(OperationType.Modify, 0, 'abc'); - const localOp = createOperation(OperationType.Insert, 0, [ { - name: 'paragraph', - data: { text: 'hello' }, - } ]); - const transformedOp = receivedOp.transform(localOp); - - expect(transformedOp?.index.textRange).toEqual([0, 0]); - }); - - it('should not change the operation if local op is a Block operation after a received one', () => { - const receivedOp = createOperation(OperationType.Insert, 0, [ { - name: 'paragraph', - data: { text: 'abc' }, - } ]); - const localOp = createOperation(OperationType.Insert, 1, [ { - name: 'paragraph', - data: { text: 'hello' }, - } ]); - - const transformedOp = receivedOp.transform(localOp); - - expect(transformedOp).toEqual(receivedOp); - }); - - it('should adjust the block index if local op is a Block operation before a received one', () => { - const receivedOp = createOperation(OperationType.Insert, 1, [ { - name: 'paragraph', - data: { text: 'abc' }, - } ]); - const localOp = createOperation(OperationType.Insert, 0, [ { - name: 'paragraph', - data: { text: 'hello' }, - } ]); - - const transformedOp = receivedOp.transform(localOp); - - expect(transformedOp?.index.blockIndex).toEqual(2); - }); - - it('should adjust the block index if local op is a Block operation at the same index as a received one', () => { - const receivedOp = createOperation(OperationType.Insert, 0, [ { - name: 'paragraph', - data: { text: 'abc' }, - } ]); - const localOp = createOperation(OperationType.Insert, 0, [ { - name: 'paragraph', - data: { text: 'hello' }, - } ]); - - const transformedOp = receivedOp.transform(localOp); - - expect(transformedOp?.index.blockIndex).toEqual(1); - }); - }); - - describe('Transformation relative to Delete operation', () => { - it('should not change a received operation if it is before a local one', () => { - const receivedOp = createOperation(OperationType.Insert, 0, 'abc'); - const localOp = createOperation(OperationType.Delete, 3, 'def'); - const transformedOp = receivedOp.transform(localOp); - - expect(transformedOp).toEqual(receivedOp); - }); - - it('should transform an index for a received operation if it is after a local one', () => { - const receivedOp = createOperation(OperationType.Delete, 3, 'def'); - const localOp = createOperation(OperationType.Delete, 0, 'abc'); - const transformedOp = receivedOp.transform(localOp); - - expect(transformedOp?.index.textRange).toEqual([0, 0]); - }); - - it('should transform a received operation if it is at the same position as a local one', () => { - const receivedOp = createOperation(OperationType.Modify, 3, 'abc'); - const localOp = createOperation(OperationType.Delete, 3, 'def'); - const transformedOp = receivedOp.transform(localOp); - - expect(transformedOp?.index.textRange).toEqual([0, 0]); - }); - - it('should not change the text index if local op is a Block operation', () => { - const receivedOp = createOperation(OperationType.Modify, 1, 'abc'); - const localOp = createOperation(OperationType.Delete, 0, [ { - name: 'paragraph', - data: { text: 'hello' }, - } ]); - const transformedOp = receivedOp.transform(localOp); - - expect(transformedOp?.index.textRange).toEqual([1, 1]); - }); - - it('should not change the text index if local op is a Block operation', () => { - const receivedOp = createOperation(OperationType.Modify, 0, 'abc'); - const localOp = createOperation(OperationType.Insert, 0, [ { - name: 'paragraph', - data: { text: 'hello' }, - } ]); - const transformedOp = receivedOp.transform(localOp); - - expect(transformedOp?.index.textRange).toEqual([0, 0]); - }); - - it('should not change the operation if local op is a Block operation after a received one', () => { - const receivedOp = createOperation(OperationType.Insert, 0, [ { - name: 'paragraph', - data: { text: 'abc' }, - } ]); - const localOp = createOperation(OperationType.Delete, 1, [ { - name: 'paragraph', - data: { text: 'hello' }, - } ]); - - const transformedOp = receivedOp.transform(localOp); - - expect(transformedOp).toEqual(receivedOp); - }); - - it('should adjust the block index if local op is a Block operation before a received one', () => { - const receivedOp = createOperation(OperationType.Insert, 1, [ { - name: 'paragraph', - data: { text: 'abc' }, - } ]); - const localOp = createOperation(OperationType.Delete, 0, [ { - name: 'paragraph', - data: { text: 'hello' }, - } ]); - - const transformedOp = receivedOp.transform(localOp); - - expect(transformedOp?.index.blockIndex).toEqual(0); - }); - - it('should adjust the block index if local op is a Block operation at the same index as a received one', () => { - const receivedOp = createOperation(OperationType.Insert, 1, [ { - name: 'paragraph', - data: { text: 'abc' }, - } ]); - const localOp = createOperation(OperationType.Delete, 1, [ { - name: 'paragraph', - data: { text: 'hello' }, - } ]); - - const transformedOp = receivedOp.transform(localOp); - - expect(transformedOp?.index.blockIndex).toEqual(0); - }); - - it('should return null if the transformed text range becomes negative', () => { - const receivedOp = createOperation(OperationType.Delete, 1, 'abc'); - const localOp = createOperation(OperationType.Delete, 0, 'defghijk'); // Delete more characters than available - const transformedOp = receivedOp.transform(localOp); - - expect(transformedOp).toBeNull(); - }); - - it('should return null if the transformed text range becomes negative for Insert operation', () => { - const receivedOp = createOperation(OperationType.Insert, 1, 'abc'); - const localOp = createOperation(OperationType.Delete, 0, 'defghijk'); // Delete more characters than available - const transformedOp = receivedOp.transform(localOp); - - expect(transformedOp).toBeNull(); - }); - }); - }); - describe('.inverse()', () => { it('should change the type of Insert operation to Delete operation', () => { const op = createOperation(OperationType.Insert, 0, 'abc'); diff --git a/packages/collaboration-manager/src/Operation.ts b/packages/collaboration-manager/src/Operation.ts index ad1fcf10..8cf13efb 100644 --- a/packages/collaboration-manager/src/Operation.ts +++ b/packages/collaboration-manager/src/Operation.ts @@ -1,5 +1,5 @@ import { IndexBuilder, type Index, type BlockNodeSerialized } from '@editorjs/model'; -import { OperationsTransformer } from './OperationsTransformer'; +import { OperationsTransformer } from './OperationsTransformer.js'; /** * Type of the operation @@ -83,7 +83,7 @@ export interface SerializedOperation { export type OperationTypeToData = T extends OperationType.Modify ? ModifyOperationData : T extends OperationType.Neutral - ? NeutralOperationData + ? NeutralOperationData : InsertOrDeleteOperationData; /** diff --git a/packages/collaboration-manager/src/OperationsTransformer.spec.ts b/packages/collaboration-manager/src/OperationsTransformer.spec.ts new file mode 100644 index 00000000..284241f8 --- /dev/null +++ b/packages/collaboration-manager/src/OperationsTransformer.spec.ts @@ -0,0 +1,260 @@ +import { createDataKey, DocumentId, IndexBuilder } from '@editorjs/model'; +import { Operation, OperationType } from './Operation.js'; +import { OperationsTransformer } from './OperationsTransformer.js'; + +describe('OperationsTransformer', () => { + let transformer: OperationsTransformer; + + beforeEach(() => { + transformer = new OperationsTransformer(); + }); + + describe('transform', () => { + it('should not transform operations on different documents', () => { + const operation = new Operation( + OperationType.Insert, + new IndexBuilder().addDocumentId('doc1' as DocumentId).addBlockIndex(0).build(), + { payload: 'test' }, + 'user1', + 1 + ); + + const againstOp = new Operation( + OperationType.Insert, + new IndexBuilder().addDocumentId('doc2' as DocumentId).addBlockIndex(0).build(), + { payload: 'test' }, + 'user2', + 1 + ); + + const result = transformer.transform(operation, againstOp); + expect(result).toEqual(operation); + }); + + describe('Block operations transformation', () => { + describe('Against block operations', () => { + + it('should increase block index when transforming against Insert operation', () => { + const operation = new Operation( + OperationType.Insert, + new IndexBuilder().addDocumentId('doc1' as DocumentId).addBlockIndex(2).build(), + { payload: 'test' }, + 'user1', + 1 + ); + + const againstOp = new Operation( + OperationType.Insert, + new IndexBuilder().addDocumentId('doc1' as DocumentId).addBlockIndex(1).build(), + { payload: 'test' }, + 'user2', + 1 + ); + + const result = transformer.transform(operation, againstOp); + expect(result.index.blockIndex).toBe(3); + }); + + it('should decrease block index when transforming against Delete operation before current block', () => { + const operation = new Operation( + OperationType.Insert, + new IndexBuilder().addDocumentId('doc1' as DocumentId).addBlockIndex(2).build(), + { payload: 'test' }, + 'user1', + 1 + ); + + const againstOp = new Operation( + OperationType.Delete, + new IndexBuilder().addDocumentId('doc1' as DocumentId).addBlockIndex(1).build(), + { payload: 'test' }, + 'user2', + 1 + ); + + const result = transformer.transform(operation, againstOp); + expect(result.index.blockIndex).toBe(1); + }); + + it('should return Neutral operation when transforming against Delete operation of the same block', () => { + const operation = new Operation( + OperationType.Modify, + new IndexBuilder().addDocumentId('doc1' as DocumentId).addBlockIndex(1).build(), + { payload: 'test' }, + 'user1', + 1 + ); + + const againstOp = new Operation( + OperationType.Delete, + new IndexBuilder().addDocumentId('doc1' as DocumentId).addBlockIndex(1).build(), + { payload: 'test' }, + 'user2', + 1 + ); + + const result = transformer.transform(operation, againstOp); + expect(result.type).toBe(OperationType.Neutral); + }); + }); + + describe('Against text operations', () => { + it('should not transform block operation against text operation', () => { + const operation = new Operation( + OperationType.Insert, + new IndexBuilder().addDocumentId('doc1' as DocumentId).addBlockIndex(1).build(), + { payload: 'test' }, + 'user1', + 1 + ); + + const againstOp = new Operation( + OperationType.Insert, + new IndexBuilder() + .addDocumentId('doc1' as DocumentId) + .addBlockIndex(1) + .addDataKey(createDataKey('text')) + .addTextRange([0, 1]) + .build(), + { payload: 'a' }, + 'user2', + 1 + ); + + const result = transformer.transform(operation, againstOp); + expect(result).toEqual(operation); + }); + }); + }); + + describe('Text operations transformation', () => { + describe('Against text Insert operations', () => { + it('should shift right text range when Insert is before current operation', () => { + const operation = new Operation( + OperationType.Insert, + new IndexBuilder() + .addDocumentId('doc1' as DocumentId) + .addBlockIndex(1) + .addDataKey(createDataKey('text')) + .addTextRange([5, 8]) + .build(), + { payload: 'test' }, + 'user1', + 1 + ); + + const againstOp = new Operation( + OperationType.Insert, + new IndexBuilder() + .addDocumentId('doc1' as DocumentId) + .addBlockIndex(1) + .addDataKey(createDataKey('text')) + .addTextRange([2, 2]) + .build(), + { payload: 'abc' }, + 'user2', + 1 + ); + + const result = transformer.transform(operation, againstOp); + expect(result.index.textRange).toEqual([8, 11]); + }); + + it('should extend text range when Insert is inside current operation range', () => { + const operation = new Operation( + OperationType.Modify, + new IndexBuilder() + .addDocumentId('doc1' as DocumentId) + .addBlockIndex(1) + .addDataKey(createDataKey('text')) + .addTextRange([2, 8]) + .build(), + { payload: 'test' }, + 'user1', + 1 + ); + + const againstOp = new Operation( + OperationType.Insert, + new IndexBuilder() + .addDocumentId('doc1' as DocumentId) + .addBlockIndex(1) + .addDataKey(createDataKey('text')) + .addTextRange([4, 4]) + .build(), + { payload: 'abc' }, + 'user2', + 1 + ); + + const result = transformer.transform(operation, againstOp); + expect(result.index.textRange).toEqual([2, 11]); + }); + }); + + describe('Against text Delete operations', () => { + it('should shift text range left when Delete is before current operation', () => { + const operation = new Operation( + OperationType.Insert, + new IndexBuilder() + .addDocumentId('doc1' as DocumentId) + .addBlockIndex(1) + .addDataKey(createDataKey('text')) + .addTextRange([8, 10]) + .build(), + { payload: 'test' }, + 'user1', + 1 + ); + + const againstOp = new Operation( + OperationType.Delete, + new IndexBuilder() + .addDocumentId('doc1' as DocumentId) + .addBlockIndex(1) + .addDataKey(createDataKey('text')) + .addTextRange([2, 5]) + .build(), + { payload: 'abc' }, + 'user2', + 1 + ); + + const result = transformer.transform(operation, againstOp); + expect(result.index.textRange).toEqual([5, 7]); + }); + + it('should return Neutral operation when Delete fully covers current operation', () => { + const operation = new Operation( + OperationType.Modify, + new IndexBuilder() + .addDocumentId('doc1' as DocumentId) + .addBlockIndex(1) + .addDataKey(createDataKey('text')) + .addTextRange([3, 5]) + .build(), + { payload: 'test' }, + 'user1', + 1 + ); + + const againstOp = new Operation( + OperationType.Delete, + new IndexBuilder() + .addDocumentId('doc1' as DocumentId) + .addBlockIndex(1) + .addDataKey(createDataKey('text')) + .addTextRange([2, 8]) + .build(), + { payload: 'abcdef' }, + 'user2', + 1 + ); + + const result = transformer.transform(operation, againstOp); + expect(result.type).toBe(OperationType.Neutral); + }); + }); + }); + }); +}); \ No newline at end of file diff --git a/packages/collaboration-manager/src/OperationsTransformer.ts b/packages/collaboration-manager/src/OperationsTransformer.ts index db1b2e8b..c200f5d2 100644 --- a/packages/collaboration-manager/src/OperationsTransformer.ts +++ b/packages/collaboration-manager/src/OperationsTransformer.ts @@ -1,13 +1,18 @@ -import { IndexBuilder } from "@editorjs/model"; -import { Operation, OperationType } from "./Operation"; -import { getRangesIntersectionType, RangeIntersectionType } from "./utils/getRangesIntersectionType"; +import { IndexBuilder } from '@editorjs/model'; +import { Operation, OperationType } from './Operation.js'; +import { getRangesIntersectionType, RangeIntersectionType } from './utils/getRangesIntersectionType.js'; /** * Class that transforms operation against another operation */ export class OperationsTransformer { - constructor() {} - + /** + * Method that transforms operation against another operation + * + * @param operation - operation to be transformed + * @param againstOp - operation against which the current operation should be transformed + * @returns {Operation} new operation + */ public transform(operation: Operation, againstOp: Operation): Operation | Operation { /** * Do not transform operations if they are on different documents @@ -24,28 +29,30 @@ export class OperationsTransformer { * Cases: * 1. Against operation is a block operation and current operation is also a block operation * - check if againstOp affects current operation, update operation's block index - * + * * 2. Against operation is a block operation and current operation is a text operation * - same as above, check if againstOp affects current operation and update operation's block index - * + * * 3. Against operation is a text operation and current operation is a block operation * - text operation does not afftect block operation - so return copy of current operation - * + * * 4. Against operation is a text operation and current operation is also a text operation * - check if againstOp affects current operation and update operation's text index - * + * * @param operation - operation to be transformed * @param againstOp - operation against which the current operation should be transformed - * @returns new operation + * @returns {Operation} new operation */ #applyTransformation(operation: Operation, againstOp: Operation): Operation | Operation { const againstIndex = againstOp.index; switch (true) { case (againstIndex.isBlockIndex): - return this.#transformAgainstBlockOperation(operation, againstOp); + console.log('is block index') + return this.#transformAgainstBlockOperation(operation, againstOp); case (againstIndex.isTextIndex): + console.log('is text index') return this.#transformAgainstTextOperation(operation, againstOp); /** @@ -58,23 +65,23 @@ export class OperationsTransformer { /** * Method that transforms operation against block operation - * + * * Cases: - * 1. Against operation is an Insert operation + * 1. Against operation is an Insert operation * - Increase block index of the current operation - * - * 2. Against operation is a Delete operation + * + * 2. Against operation is a Delete operation * - Against operation deleted a block before the current operation * - Decrease block index of the current operation * - Against operation deleted exactly the block of the current operation * - Return Neutral operation - * - * 3. Against operation is a Modify or Neutral operation + * + * 3. Against operation is a Modify or Neutral operation * - Modify and Neutral operations do not affect any operations so return copy of the current operation - * + * * @param operation - Operation to be transformed * @param againstOp - Operation against which the current operation should be transformed - * @returns New operation + * @returns {Operation} new operation */ #transformAgainstBlockOperation(operation: Operation, againstOp: Operation): Operation | Operation { const newIndexBuilder = new IndexBuilder().from(operation.index); @@ -82,14 +89,14 @@ export class OperationsTransformer { /** * If current operation has no block index, return copy of the current operation */ - if (!operation.index.isBlockIndex) { + if (operation.index.blockIndex === undefined) { return Operation.from(operation); } /** * Check that againstOp affects current operation */ - if (againstOp.index.blockIndex! <= operation.index.blockIndex!) { + if (againstOp.index.blockIndex! > operation.index.blockIndex!) { return Operation.from(operation); } @@ -108,7 +115,7 @@ export class OperationsTransformer { * Cover case 2 */ case OperationType.Delete: - if (operation.index.isBlockIndex) { + if (operation.index.blockIndex !== undefined) { if (againstOp.index.blockIndex! < operation.index.blockIndex!) { newIndexBuilder.addBlockIndex(operation.index.blockIndex! - 1); } else { @@ -122,7 +129,7 @@ export class OperationsTransformer { * Cover case 3 */ default: - return Operation.from(operation); + return Operation.from(operation); } /** @@ -137,11 +144,11 @@ export class OperationsTransformer { /** * Method that transforms operation against text operation - * + * * Cases: * 1. Current operation is a block operation * - Text opearation cant affect block operation so return copy of the current one - * + * * 2. Current operation is a text operation * - Against operation is Insert * - Transform current operation against textInsert @@ -149,15 +156,15 @@ export class OperationsTransformer { * - Check that againstOp affects current operation and transform against text operation * - Against operation is Modify or Neutral * - Modify and Neutral operations do not affect any of the text operations so return copy of the current operation - * + * * @param operation - Operation to be transformed * @param againstOp - Operation against which the current operation should be transformed - * @returns New operation + * @returns {Operation} new operation */ #transformAgainstTextOperation(operation: Operation, againstOp: Operation): Operation | Operation { const index = operation.index; const againstIndex = againstOp.index; - + /** * Cover case 1 */ @@ -168,7 +175,7 @@ export class OperationsTransformer { /** * Check that againstOp affects current operation */ - if (index.dataKey === againstIndex.dataKey && index.blockIndex === againstIndex.blockIndex && againstIndex.textRange![0] > index.textRange![1]) { + if (index.dataKey === againstIndex.dataKey && index.blockIndex === againstIndex.blockIndex && againstIndex.textRange![0] >= index.textRange![1]) { return Operation.from(operation); } @@ -186,17 +193,17 @@ export class OperationsTransformer { /** * Method that transforms operation against text insert operation happened on the left side of the current operation - * + * * Cases: * 1. Against operation is fully on the left of the current operation * - Move text range of the current operation to the right by amount of inserted characters - * + * * 2. Against operation is inside of the current operation text range * - Move right bound of the current operation to the right by amount of inserted characters to include the inserted text - * + * * @param operation - Operation to be transformed * @param againstOp - Operation against which the current operation should be transformed - * @returns New operation + * @returns {Operation} new operation */ #transformAgainstTextInsert(operation: Operation, againstOp: Operation): Operation | Operation { const newIndexBuilder = new IndexBuilder().from(operation.index); @@ -235,27 +242,27 @@ export class OperationsTransformer { /** * Method that transforms operation against text delete operation - * + * * Cases: * 1. Delete range is fully on the left of the current operation * - Move text range of the current operation to the left by amount of deleted characters - * + * * 2. Delete range covers part of the current operation * - Deleted left side of the current operation * - Move left bound of the current operation to the start of the against Delete operation * - Move right bound of the current operation to the left by (amount of deleted characters - amount of characters in the current operation that were deleted) * - Deleted right side of the current operation * - Move right bound of the current operation to the left by amount of deleted intersection - * + * * 3. Delete range is inside of the current operation text range * - Move right bound of the current operation to the left by amount of deleted characters - * + * * 4. Delete range fully covers the current operation text rannge * - Return Neutral operation - * + * * @param operation - Operation to be transformed * @param againstOp - Operation against which the current operation should be transformed - * @returns New operation + * @returns {Operation} new operation */ #transformAgainstTextDelete(operation: Operation, againstOp: Operation): Operation | Operation { const newIndexBuilder = new IndexBuilder().from(operation.index); @@ -263,7 +270,7 @@ export class OperationsTransformer { const index = operation.index; const againstIndex = againstOp.index; - + const intersectionType = getRangesIntersectionType(index.textRange!, againstIndex.textRange!); switch (intersectionType) { diff --git a/packages/collaboration-manager/src/UndoRedoManager.spec.ts b/packages/collaboration-manager/src/UndoRedoManager.spec.ts index cb9cfd84..2c6e5a7b 100644 --- a/packages/collaboration-manager/src/UndoRedoManager.spec.ts +++ b/packages/collaboration-manager/src/UndoRedoManager.spec.ts @@ -2,23 +2,28 @@ import { IndexBuilder } from '@editorjs/model'; import { describe } from '@jest/globals'; import { Operation, OperationType } from './Operation.js'; import { UndoRedoManager } from './UndoRedoManager.js'; +import { jest } from '@jest/globals'; const userId = 'user'; /** * Helper function to create test operations + * + * @param index - block index of the operation + * @param text - text of the operation + * @param type - type of the operation */ -function createOperation(index: number, text: string): Operation { +function createOperation(index: number, text: string, type: OperationType = OperationType.Insert): Operation { return new Operation( - OperationType.Insert, + type, new IndexBuilder() .addBlockIndex(index) .build(), { - payload: [{ + payload: [ { name: 'paragraph', data: { text }, - }], + } ], }, userId ); @@ -133,7 +138,7 @@ describe('UndoRedoManager', () => { const manager = new UndoRedoManager(); const op1 = createOperation(0, 'first'); const op2 = createOperation(1, 'second'); - + // Put operations in undo stack manager.put(op1); manager.put(op2); @@ -143,119 +148,52 @@ describe('UndoRedoManager', () => { // Mock transform method const transformSpy = jest.spyOn(op2, 'transform').mockReturnValue(createOperation(2, 'transformed')); - - manager.transformUndoStack(transformingOp); - - expect(transformSpy).toHaveBeenCalledWith(transformingOp); - }); - - it('should transform redo stack operations', () => { - const manager = new UndoRedoManager(); - const op1 = createOperation(0, 'first'); - - // Put operation and undo it to move it to redo stack - manager.put(op1); - manager.undo(); - - // Create operation that will affect the stack - const transformingOp = createOperation(0, 'transform'); - // Mock transform method - const transformSpy = jest.spyOn(op1, 'transform').mockReturnValue(createOperation(1, 'transformed')); - - manager.transformRedoStack(transformingOp); + manager.transformStacks(transformingOp); expect(transformSpy).toHaveBeenCalledWith(transformingOp); }); - it('should remove operations from stack if transform returns null', () => { - const manager = new UndoRedoManager(); - const op1 = createOperation(0, 'first'); - const op2 = createOperation(1, 'second'); - - manager.put(op1); - manager.put(op2); - - // Mock transform to return null - jest.spyOn(op2, 'transform').mockReturnValue(null); - - const transformingOp = createOperation(0, 'transform'); - manager.transformUndoStack(transformingOp); - - // Try to undo twice - second undo should return undefined since op2 was removed - expect(manager.undo()).toBeDefined(); - expect(manager.undo()).toBeUndefined(); - }); - it('should handle empty stacks during transformation', () => { const manager = new UndoRedoManager(); const transformingOp = createOperation(0, 'transform'); // Should not throw when transforming empty stacks expect(() => { - manager.transformUndoStack(transformingOp); - manager.transformRedoStack(transformingOp); + manager.transformStacks(transformingOp); }).not.toThrow(); }); - it('should transform both undo and redo stacks when operations are undone', () => { - const manager = new UndoRedoManager(); - const op1 = createOperation(0, 'first'); - const op2 = createOperation(1, 'second'); - - // Put operations in undo stack - manager.put(op1); - manager.put(op2); - - // Undo op2 to move it to redo stack - manager.undo(); - - // Create operation that will affect both stacks - const transformingOp = createOperation(0, 'transform'); - - // Mock transform methods - const undoStackSpy = jest.spyOn(op1, 'transform').mockReturnValue(createOperation(1, 'transformed-undo')); - const redoStackSpy = jest.spyOn(op2, 'transform').mockReturnValue(createOperation(2, 'transformed-redo')); - - // Transform both stacks - manager.transformUndoStack(transformingOp); - manager.transformRedoStack(transformingOp); - - expect(undoStackSpy).toHaveBeenCalledWith(transformingOp); - expect(redoStackSpy).toHaveBeenCalledWith(transformingOp); - }); - it('should maintain correct operation order after transforming both stacks', () => { const manager = new UndoRedoManager(); const op1 = createOperation(0, 'first'); const op2 = createOperation(1, 'second'); const op3 = createOperation(2, 'third'); - + // Setup initial state manager.put(op1); manager.put(op2); manager.put(op3); - + // Move op3 and op2 to redo stack manager.undo(); // undo op3 manager.undo(); // undo op2 const transformingOp = createOperation(0, 'transform'); - + // Mock transforms to return operations with shifted indices jest.spyOn(op1, 'transform').mockReturnValue(createOperation(1, 'transformed-1')); jest.spyOn(op2, 'transform').mockReturnValue(createOperation(2, 'transformed-2')); jest.spyOn(op3, 'transform').mockReturnValue(createOperation(3, 'transformed-3')); - - manager.transformUndoStack(transformingOp); - manager.transformRedoStack(transformingOp); + + manager.transformStacks(transformingOp); // Verify operations can be redone in correct order const redoOp1 = manager.redo(); const redoOp2 = manager.redo(); - - expect(redoOp1?.index.toString()).toBe('2'); // transformed op2 - expect(redoOp2?.index.toString()).toBe('3'); // transformed op3 + + expect(redoOp1?.index.blockIndex?.toString()).toBe('2'); // transformed op2 + expect(redoOp2?.index.blockIndex?.toString()).toBe('3'); // transformed op3 }); }); }); diff --git a/packages/collaboration-manager/src/UndoRedoManager.ts b/packages/collaboration-manager/src/UndoRedoManager.ts index 8d0c802c..8a236f59 100644 --- a/packages/collaboration-manager/src/UndoRedoManager.ts +++ b/packages/collaboration-manager/src/UndoRedoManager.ts @@ -1,3 +1,4 @@ +import { BatchedOperation } from './BatchedOperation.js'; import type { Operation } from './Operation.js'; /** @@ -26,6 +27,8 @@ export class UndoRedoManager { const invertedOperation = operation.inverse(); + console.log('inverted operation', JSON.stringify(invertedOperation)); + this.#redoStack.push(invertedOperation); return invertedOperation; @@ -43,6 +46,7 @@ export class UndoRedoManager { const invertedOperation = operation.inverse(); + this.#undoStack.push(invertedOperation); return invertedOperation; @@ -60,30 +64,33 @@ export class UndoRedoManager { /** * Transforms undo and redo stacks - * + * * @param operation - operation to transform against */ public transformStacks(operation: Operation): void { + console.log('transform undo') this.transformStack(operation, this.#undoStack); + console.log('transform redo') this.transformStack(operation, this.#redoStack); } /** * Transforms passed operations stack against the operation - * + * * @param operation - operation to transform against * @param stack - stack to transform */ public transformStack(operation: Operation, stack: Operation[]): void { const transformed = stack.flatMap((op) => { + console.log('op in stack', op) const transformedOp = op.transform(operation); if (transformedOp === null) { - return [] + return []; } return [ transformedOp ]; - }) + }); stack.length = 0; stack.push(...transformed); diff --git a/packages/collaboration-manager/src/utils/getRangesIntersectionType.ts b/packages/collaboration-manager/src/utils/getRangesIntersectionType.ts index 282fa3ee..ff114f00 100644 --- a/packages/collaboration-manager/src/utils/getRangesIntersectionType.ts +++ b/packages/collaboration-manager/src/utils/getRangesIntersectionType.ts @@ -1,4 +1,4 @@ -import { TextRange } from "@editorjs/model"; +import type { TextRange } from '@editorjs/model'; /** * Represents the type of intersection between two text ranges. @@ -13,7 +13,7 @@ export enum RangeIntersectionType { /** * Returns the type of intersection between two text ranges - * + * * @param range - Range to check * @param rangeToCompare - Range to compare with * @returns Type of intersection @@ -35,7 +35,7 @@ export function getRangesIntersectionType(range: TextRange, rangeToCompare: Text */ if (start <= startToCompare && end >= endToCompare) { return RangeIntersectionType.Includes; - } + } /** * Range is included in the range to compare From fc106f603233b881ee61312e0b0db2a2e6e7da5e Mon Sep 17 00:00:00 2001 From: e11sy <130844513+e11sy@users.noreply.github.com> Date: Sat, 26 Apr 2025 22:27:30 +0300 Subject: [PATCH 08/15] chore(collaboration-manager): clean up --- .../src/BatchedOperation.spec.ts | 9 ++-- .../src/BatchedOperation.ts | 3 +- .../src/Operation.spec.ts | 2 +- .../src/OperationsTransformer.spec.ts | 52 ++++++++++++++----- .../src/OperationsTransformer.ts | 2 - .../src/UndoRedoManager.spec.ts | 3 +- .../src/UndoRedoManager.ts | 6 --- .../src/utils/getRangesIntersectionType.ts | 2 +- 8 files changed, 49 insertions(+), 30 deletions(-) diff --git a/packages/collaboration-manager/src/BatchedOperation.spec.ts b/packages/collaboration-manager/src/BatchedOperation.spec.ts index 0249b809..39abfaee 100644 --- a/packages/collaboration-manager/src/BatchedOperation.spec.ts +++ b/packages/collaboration-manager/src/BatchedOperation.spec.ts @@ -1,4 +1,4 @@ -import { createDataKey, Index, IndexBuilder } from '@editorjs/model'; +import { createDataKey, IndexBuilder } from '@editorjs/model'; import { BatchedOperation } from './BatchedOperation.js'; import type { SerializedOperation } from './Operation.js'; import { Operation, OperationType } from './Operation.js'; @@ -67,11 +67,8 @@ describe('Batch', () => { originalBatch.add(op2); - - const newBatch = BatchedOperation.from(originalBatch); - console.log('original operations', originalBatch.operations); - console.log('new operations', newBatch.operations) + const newBatch = BatchedOperation.from(originalBatch); expect(newBatch.operations).toStrictEqual(originalBatch.operations); expect(newBatch).not.toBe(originalBatch); // Should be a new instance @@ -156,7 +153,7 @@ describe('Batch', () => { const transformedBatch = batch.transform(againstOp); - const neutralOp = new Operation(OperationType.Neutral, templateIndex, { payload: 'a' }, userId) + const neutralOp = new Operation(OperationType.Neutral, templateIndex, { payload: 'a' }, userId); expect(transformedBatch.operations[0]).toEqual(neutralOp); }); diff --git a/packages/collaboration-manager/src/BatchedOperation.ts b/packages/collaboration-manager/src/BatchedOperation.ts index c70972e2..a6951b91 100644 --- a/packages/collaboration-manager/src/BatchedOperation.ts +++ b/packages/collaboration-manager/src/BatchedOperation.ts @@ -87,7 +87,8 @@ export class BatchedOperation extends O */ const newBatchedOperation = new BatchedOperation>(lastOp.inverse()); - this.operations.toReversed().slice(1).map(op => newBatchedOperation.add(op.inverse())); + this.operations.toReversed().slice(1) + .map(op => newBatchedOperation.add(op.inverse())); return newBatchedOperation as BatchedOperation>; } diff --git a/packages/collaboration-manager/src/Operation.spec.ts b/packages/collaboration-manager/src/Operation.spec.ts index 99ec161c..903a3bd2 100644 --- a/packages/collaboration-manager/src/Operation.spec.ts +++ b/packages/collaboration-manager/src/Operation.spec.ts @@ -1,5 +1,5 @@ /* eslint-disable @typescript-eslint/no-magic-numbers */ -import type { BlockNodeSerialized, DataKey, DocumentIndex } from '@editorjs/model'; +import type { BlockNodeSerialized, DataKey } from '@editorjs/model'; import { IndexBuilder } from '@editorjs/model'; import { describe } from '@jest/globals'; import { type InsertOrDeleteOperationData, type ModifyOperationData, Operation, OperationType } from './Operation.js'; diff --git a/packages/collaboration-manager/src/OperationsTransformer.spec.ts b/packages/collaboration-manager/src/OperationsTransformer.spec.ts index 284241f8..55b4154e 100644 --- a/packages/collaboration-manager/src/OperationsTransformer.spec.ts +++ b/packages/collaboration-manager/src/OperationsTransformer.spec.ts @@ -1,7 +1,9 @@ -import { createDataKey, DocumentId, IndexBuilder } from '@editorjs/model'; +import type { DocumentId } from '@editorjs/model'; +import { createDataKey, IndexBuilder } from '@editorjs/model'; import { Operation, OperationType } from './Operation.js'; import { OperationsTransformer } from './OperationsTransformer.js'; +/* eslint-disable @typescript-eslint/no-magic-numbers */ describe('OperationsTransformer', () => { let transformer: OperationsTransformer; @@ -13,7 +15,9 @@ describe('OperationsTransformer', () => { it('should not transform operations on different documents', () => { const operation = new Operation( OperationType.Insert, - new IndexBuilder().addDocumentId('doc1' as DocumentId).addBlockIndex(0).build(), + new IndexBuilder().addDocumentId('doc1' as DocumentId) + .addBlockIndex(0) + .build(), { payload: 'test' }, 'user1', 1 @@ -21,23 +25,27 @@ describe('OperationsTransformer', () => { const againstOp = new Operation( OperationType.Insert, - new IndexBuilder().addDocumentId('doc2' as DocumentId).addBlockIndex(0).build(), + new IndexBuilder().addDocumentId('doc2' as DocumentId) + .addBlockIndex(0) + .build(), { payload: 'test' }, 'user2', 1 ); const result = transformer.transform(operation, againstOp); + expect(result).toEqual(operation); }); describe('Block operations transformation', () => { describe('Against block operations', () => { - it('should increase block index when transforming against Insert operation', () => { const operation = new Operation( OperationType.Insert, - new IndexBuilder().addDocumentId('doc1' as DocumentId).addBlockIndex(2).build(), + new IndexBuilder().addDocumentId('doc1' as DocumentId) + .addBlockIndex(2) + .build(), { payload: 'test' }, 'user1', 1 @@ -45,20 +53,25 @@ describe('OperationsTransformer', () => { const againstOp = new Operation( OperationType.Insert, - new IndexBuilder().addDocumentId('doc1' as DocumentId).addBlockIndex(1).build(), + new IndexBuilder().addDocumentId('doc1' as DocumentId) + .addBlockIndex(1) + .build(), { payload: 'test' }, 'user2', 1 ); const result = transformer.transform(operation, againstOp); + expect(result.index.blockIndex).toBe(3); }); it('should decrease block index when transforming against Delete operation before current block', () => { const operation = new Operation( OperationType.Insert, - new IndexBuilder().addDocumentId('doc1' as DocumentId).addBlockIndex(2).build(), + new IndexBuilder().addDocumentId('doc1' as DocumentId) + .addBlockIndex(2) + .build(), { payload: 'test' }, 'user1', 1 @@ -66,20 +79,25 @@ describe('OperationsTransformer', () => { const againstOp = new Operation( OperationType.Delete, - new IndexBuilder().addDocumentId('doc1' as DocumentId).addBlockIndex(1).build(), + new IndexBuilder().addDocumentId('doc1' as DocumentId) + .addBlockIndex(1) + .build(), { payload: 'test' }, 'user2', 1 ); const result = transformer.transform(operation, againstOp); + expect(result.index.blockIndex).toBe(1); }); it('should return Neutral operation when transforming against Delete operation of the same block', () => { const operation = new Operation( OperationType.Modify, - new IndexBuilder().addDocumentId('doc1' as DocumentId).addBlockIndex(1).build(), + new IndexBuilder().addDocumentId('doc1' as DocumentId) + .addBlockIndex(1) + .build(), { payload: 'test' }, 'user1', 1 @@ -87,13 +105,16 @@ describe('OperationsTransformer', () => { const againstOp = new Operation( OperationType.Delete, - new IndexBuilder().addDocumentId('doc1' as DocumentId).addBlockIndex(1).build(), + new IndexBuilder().addDocumentId('doc1' as DocumentId) + .addBlockIndex(1) + .build(), { payload: 'test' }, 'user2', 1 ); const result = transformer.transform(operation, againstOp); + expect(result.type).toBe(OperationType.Neutral); }); }); @@ -102,7 +123,9 @@ describe('OperationsTransformer', () => { it('should not transform block operation against text operation', () => { const operation = new Operation( OperationType.Insert, - new IndexBuilder().addDocumentId('doc1' as DocumentId).addBlockIndex(1).build(), + new IndexBuilder().addDocumentId('doc1' as DocumentId) + .addBlockIndex(1) + .build(), { payload: 'test' }, 'user1', 1 @@ -122,6 +145,7 @@ describe('OperationsTransformer', () => { ); const result = transformer.transform(operation, againstOp); + expect(result).toEqual(operation); }); }); @@ -157,6 +181,7 @@ describe('OperationsTransformer', () => { ); const result = transformer.transform(operation, againstOp); + expect(result.index.textRange).toEqual([8, 11]); }); @@ -188,6 +213,7 @@ describe('OperationsTransformer', () => { ); const result = transformer.transform(operation, againstOp); + expect(result.index.textRange).toEqual([2, 11]); }); }); @@ -221,6 +247,7 @@ describe('OperationsTransformer', () => { ); const result = transformer.transform(operation, againstOp); + expect(result.index.textRange).toEqual([5, 7]); }); @@ -252,9 +279,10 @@ describe('OperationsTransformer', () => { ); const result = transformer.transform(operation, againstOp); + expect(result.type).toBe(OperationType.Neutral); }); }); }); }); -}); \ No newline at end of file +}); \ No newline at end of file diff --git a/packages/collaboration-manager/src/OperationsTransformer.ts b/packages/collaboration-manager/src/OperationsTransformer.ts index c200f5d2..144de9ea 100644 --- a/packages/collaboration-manager/src/OperationsTransformer.ts +++ b/packages/collaboration-manager/src/OperationsTransformer.ts @@ -48,11 +48,9 @@ export class OperationsTransformer { switch (true) { case (againstIndex.isBlockIndex): - console.log('is block index') return this.#transformAgainstBlockOperation(operation, againstOp); case (againstIndex.isTextIndex): - console.log('is text index') return this.#transformAgainstTextOperation(operation, againstOp); /** diff --git a/packages/collaboration-manager/src/UndoRedoManager.spec.ts b/packages/collaboration-manager/src/UndoRedoManager.spec.ts index 2c6e5a7b..bc697212 100644 --- a/packages/collaboration-manager/src/UndoRedoManager.spec.ts +++ b/packages/collaboration-manager/src/UndoRedoManager.spec.ts @@ -182,10 +182,11 @@ describe('UndoRedoManager', () => { const transformingOp = createOperation(0, 'transform'); // Mock transforms to return operations with shifted indices + /* eslint-disable @typescript-eslint/no-magic-numbers */ jest.spyOn(op1, 'transform').mockReturnValue(createOperation(1, 'transformed-1')); jest.spyOn(op2, 'transform').mockReturnValue(createOperation(2, 'transformed-2')); jest.spyOn(op3, 'transform').mockReturnValue(createOperation(3, 'transformed-3')); - + /* eslint-enable @typescript-eslint/no-magic-numbers */ manager.transformStacks(transformingOp); // Verify operations can be redone in correct order diff --git a/packages/collaboration-manager/src/UndoRedoManager.ts b/packages/collaboration-manager/src/UndoRedoManager.ts index 8a236f59..a4110ba0 100644 --- a/packages/collaboration-manager/src/UndoRedoManager.ts +++ b/packages/collaboration-manager/src/UndoRedoManager.ts @@ -1,4 +1,3 @@ -import { BatchedOperation } from './BatchedOperation.js'; import type { Operation } from './Operation.js'; /** @@ -27,8 +26,6 @@ export class UndoRedoManager { const invertedOperation = operation.inverse(); - console.log('inverted operation', JSON.stringify(invertedOperation)); - this.#redoStack.push(invertedOperation); return invertedOperation; @@ -68,9 +65,7 @@ export class UndoRedoManager { * @param operation - operation to transform against */ public transformStacks(operation: Operation): void { - console.log('transform undo') this.transformStack(operation, this.#undoStack); - console.log('transform redo') this.transformStack(operation, this.#redoStack); } @@ -82,7 +77,6 @@ export class UndoRedoManager { */ public transformStack(operation: Operation, stack: Operation[]): void { const transformed = stack.flatMap((op) => { - console.log('op in stack', op) const transformedOp = op.transform(operation); if (transformedOp === null) { diff --git a/packages/collaboration-manager/src/utils/getRangesIntersectionType.ts b/packages/collaboration-manager/src/utils/getRangesIntersectionType.ts index ff114f00..ac956714 100644 --- a/packages/collaboration-manager/src/utils/getRangesIntersectionType.ts +++ b/packages/collaboration-manager/src/utils/getRangesIntersectionType.ts @@ -16,7 +16,7 @@ export enum RangeIntersectionType { * * @param range - Range to check * @param rangeToCompare - Range to compare with - * @returns Type of intersection + * @returns {RangeIntersectionType} Type of intersection */ export function getRangesIntersectionType(range: TextRange, rangeToCompare: TextRange): RangeIntersectionType { const [start, end] = range; From 8e7f12b93d3a07cc7cf7d171557119637805ef78 Mon Sep 17 00:00:00 2001 From: e11sy <130844513+e11sy@users.noreply.github.com> Date: Sat, 26 Apr 2025 22:52:21 +0300 Subject: [PATCH 09/15] test(batchedOp): fix test indexes --- .../src/BatchedOperation.spec.ts | 121 +++++------------- 1 file changed, 29 insertions(+), 92 deletions(-) diff --git a/packages/collaboration-manager/src/BatchedOperation.spec.ts b/packages/collaboration-manager/src/BatchedOperation.spec.ts index 39abfaee..cfb847c3 100644 --- a/packages/collaboration-manager/src/BatchedOperation.spec.ts +++ b/packages/collaboration-manager/src/BatchedOperation.spec.ts @@ -1,27 +1,22 @@ -import { createDataKey, IndexBuilder } from '@editorjs/model'; +import { createDataKey, IndexBuilder, TextRange } from '@editorjs/model'; import { BatchedOperation } from './BatchedOperation.js'; import type { SerializedOperation } from './Operation.js'; import { Operation, OperationType } from './Operation.js'; -const templateIndex = new IndexBuilder() +const createIndexByRange = (range: TextRange) => new IndexBuilder() .addBlockIndex(0) .addDataKey(createDataKey('key')) - .addTextRange([0, 0]) + .addTextRange(range) .build(); +const templateIndex = createIndexByRange([0, 0]) + const userId = 'user'; describe('Batch', () => { it('should add Insert operation to batch', () => { const op1 = new Operation(OperationType.Insert, templateIndex, { payload: 'a' }, userId); - const op2 = new Operation( - OperationType.Insert, - new IndexBuilder().from(templateIndex) - .addTextRange([1, 1]) - .build(), - { payload: 'b' }, - userId - ); + const op2 = new Operation(OperationType.Insert, createIndexByRange([1, 1]), { payload: 'b' }, userId); const batch = new BatchedOperation(op1); @@ -34,14 +29,7 @@ describe('Batch', () => { it('should add Delete operation to batch', () => { const op1 = new Operation(OperationType.Delete, templateIndex, { payload: 'a' }, userId); - const op2 = new Operation( - OperationType.Delete, - new IndexBuilder().from(templateIndex) - .addTextRange([1, 1]) - .build(), - { payload: 'b' }, - userId - ); + const op2 = new Operation(OperationType.Delete, createIndexByRange([1, 1]), { payload: 'b' }, userId); const batch = new BatchedOperation(op1); @@ -55,14 +43,8 @@ describe('Batch', () => { describe('from()', () => { it('should create a new batch from an existing batch', () => { const op1 = new Operation(OperationType.Insert, templateIndex, { payload: 'a' }, userId); - const op2 = new Operation( - OperationType.Insert, - new IndexBuilder().from(templateIndex) - .addTextRange([1, 1]) - .build(), - { payload: 'b' }, - userId - ); + const op2 = new Operation(OperationType.Insert, createIndexByRange([1, 1]), { payload: 'b' }, userId); + const originalBatch = new BatchedOperation(op1); originalBatch.add(op2); @@ -87,14 +69,8 @@ describe('Batch', () => { describe('inverse()', () => { it('should inverse all operations in the batch', () => { const op1 = new Operation(OperationType.Insert, templateIndex, { payload: 'a' }, userId); - const op2 = new Operation( - OperationType.Insert, - new IndexBuilder().from(templateIndex) - .addTextRange([1, 1]) - .build(), - { payload: 'b' }, - userId - ); + const op2 = new Operation(OperationType.Insert, createIndexByRange([1, 1]), { payload: 'b' }, userId); + const batch = new BatchedOperation(op1); batch.add(op2); @@ -108,52 +84,37 @@ describe('Batch', () => { describe('transform()', () => { it('should transform operations against another operation', () => { - const op1 = new Operation(OperationType.Insert, templateIndex, { payload: 'a' }, userId); - const op2 = new Operation( - OperationType.Insert, - new IndexBuilder().from(templateIndex) - .addTextRange([1, 1]) - .build(), - { payload: 'b' }, - userId - ); + const op1 = new Operation(OperationType.Insert, createIndexByRange([1, 1]), { payload: 'a' }, userId); + const op2 = new Operation(OperationType.Insert, createIndexByRange([2, 2]), { payload: 'b' }, userId); + const batch = new BatchedOperation(op1); batch.add(op2); - const againstOp = new Operation( - OperationType.Insert, - new IndexBuilder().from(templateIndex) - .addTextRange([0, 0]) - .build(), - { payload: 'x' }, - 'other-user' - ); + const againstOp = new Operation(OperationType.Insert, createIndexByRange([0, 0]), { payload: 'x' }, 'other-user'); const transformedBatch = batch.transform(againstOp); expect(transformedBatch).not.toBeNull(); expect(transformedBatch!.operations.length).toBe(2); // Check if text ranges were shifted by 1 due to insertion - expect(transformedBatch!.operations[0].index.textRange![0]).toBe(1); - expect(transformedBatch!.operations[1].index.textRange![0]).toBe(2); + expect(transformedBatch!.operations[0].index.textRange![0]).toBe(2); + expect(transformedBatch!.operations[1].index.textRange![0]).toBe(3); }); it('should return batch with Neutral operations if no operations can be transformed', () => { - const op = new Operation(OperationType.Insert, templateIndex, { payload: 'a' }, userId); + const op = new Operation(OperationType.Insert, createIndexByRange([1, 1]), { payload: 'a' }, userId); + const batch = new BatchedOperation(op); - const deleteIndex = new IndexBuilder() - .from(templateIndex) - .addTextRange([0, 2]) - .build(); + const deleteIndex = createIndexByRange([0, 2]) // An operation that would make transformation impossible const againstOp = new Operation(OperationType.Delete, deleteIndex, { payload: 'a' }, 'other-user'); const transformedBatch = batch.transform(againstOp); - const neutralOp = new Operation(OperationType.Neutral, templateIndex, { payload: 'a' }, userId); + const neutralOp = new Operation(OperationType.Neutral, createIndexByRange([1, 1]), { payload: [] }, userId); expect(transformedBatch.operations[0]).toEqual(neutralOp); }); @@ -162,14 +123,8 @@ describe('Batch', () => { describe('canAdd()', () => { it('should return true for consecutive text operations of same type', () => { const op1 = new Operation(OperationType.Insert, templateIndex, { payload: 'a' }, userId); - const op2 = new Operation( - OperationType.Insert, - new IndexBuilder().from(templateIndex) - .addTextRange([1, 1]) - .build(), - { payload: 'b' }, - userId - ); + const op2 = new Operation(OperationType.Insert, createIndexByRange([1, 1]), { payload: 'b' }, userId); + const batch = new BatchedOperation(op1); expect(batch.canAdd(op2)).toBe(true); @@ -177,14 +132,8 @@ describe('Batch', () => { it('should return false for non-consecutive text operations', () => { const op1 = new Operation(OperationType.Insert, templateIndex, { payload: 'a' }, userId); - const op2 = new Operation( - OperationType.Insert, - new IndexBuilder().from(templateIndex) - .addTextRange([2, 2]) - .build(), - { payload: 'b' }, - userId - ); + const op2 = new Operation(OperationType.Insert, createIndexByRange([2, 2]), { payload: 'b' }, userId); + const batch = new BatchedOperation(op1); expect(batch.canAdd(op2)).toBe(false); @@ -192,14 +141,8 @@ describe('Batch', () => { it('should return false for different operation types', () => { const op1 = new Operation(OperationType.Insert, templateIndex, { payload: 'a' }, userId); - const op2 = new Operation( - OperationType.Delete, - new IndexBuilder().from(templateIndex) - .addTextRange([1, 1]) - .build(), - { payload: 'b' }, - userId - ); + const op2 = new Operation(OperationType.Delete, createIndexByRange([1, 1]), { payload: 'b' }, userId); + const batch = new BatchedOperation(op1); expect(batch.canAdd(op2)).toBe(false); @@ -207,14 +150,8 @@ describe('Batch', () => { it('should return false for modify operations', () => { const op1 = new Operation(OperationType.Insert, templateIndex, { payload: 'a' }, userId); - const op2 = new Operation( - OperationType.Modify, - new IndexBuilder().from(templateIndex) - .addTextRange([1, 1]) - .build(), - { payload: { tool: 'bold' } }, - userId - ); + const op2 = new Operation(OperationType.Modify, createIndexByRange([1, 1]), { payload: 'b' }, userId); + const batch = new BatchedOperation(op1); expect(batch.canAdd(op2)).toBe(false); From afd14a420e7311c175bcd4d51d125db5275ad8f5 Mon Sep 17 00:00:00 2001 From: e11sy <130844513+e11sy@users.noreply.github.com> Date: Sun, 27 Apr 2025 18:50:25 +0300 Subject: [PATCH 10/15] fix (OT-server): tests --- .../collaboration-manager/src/BatchedOperation.spec.ts | 10 ++++++---- .../src/CollaborationManager.spec.ts | 2 +- .../collaboration-manager/src/OperationsTransformer.ts | 6 +++++- .../src/utils/getRangesIntersectionType.ts | 4 ++-- 4 files changed, 14 insertions(+), 8 deletions(-) diff --git a/packages/collaboration-manager/src/BatchedOperation.spec.ts b/packages/collaboration-manager/src/BatchedOperation.spec.ts index cfb847c3..ed1b3e98 100644 --- a/packages/collaboration-manager/src/BatchedOperation.spec.ts +++ b/packages/collaboration-manager/src/BatchedOperation.spec.ts @@ -1,15 +1,15 @@ -import { createDataKey, IndexBuilder, TextRange } from '@editorjs/model'; +import { createDataKey, Index, IndexBuilder, type TextRange } from '@editorjs/model'; import { BatchedOperation } from './BatchedOperation.js'; import type { SerializedOperation } from './Operation.js'; import { Operation, OperationType } from './Operation.js'; -const createIndexByRange = (range: TextRange) => new IndexBuilder() +const createIndexByRange = (range: TextRange): Index => new IndexBuilder() .addBlockIndex(0) .addDataKey(createDataKey('key')) .addTextRange(range) .build(); -const templateIndex = createIndexByRange([0, 0]) +const templateIndex = createIndexByRange([0, 0]); const userId = 'user'; @@ -98,8 +98,10 @@ describe('Batch', () => { expect(transformedBatch).not.toBeNull(); expect(transformedBatch!.operations.length).toBe(2); // Check if text ranges were shifted by 1 due to insertion + /* eslint-disable @typescript-eslint-no-magic-numbers */ expect(transformedBatch!.operations[0].index.textRange![0]).toBe(2); expect(transformedBatch!.operations[1].index.textRange![0]).toBe(3); + /* eslint-enable @typescript-eslint-no-magic-numbers */ }); it('should return batch with Neutral operations if no operations can be transformed', () => { @@ -107,7 +109,7 @@ describe('Batch', () => { const batch = new BatchedOperation(op); - const deleteIndex = createIndexByRange([0, 2]) + const deleteIndex = createIndexByRange([0, 2]); // An operation that would make transformation impossible const againstOp = new Operation(OperationType.Delete, deleteIndex, { payload: 'a' }, 'other-user'); diff --git a/packages/collaboration-manager/src/CollaborationManager.spec.ts b/packages/collaboration-manager/src/CollaborationManager.spec.ts index 624e8bff..cfc1b43e 100644 --- a/packages/collaboration-manager/src/CollaborationManager.spec.ts +++ b/packages/collaboration-manager/src/CollaborationManager.spec.ts @@ -1026,7 +1026,7 @@ describe('CollaborationManager', () => { // Create local delete operation const localIndex = new IndexBuilder().addBlockIndex(0) .addDataKey(createDataKey('text')) - .addTextRange([0, 7]) + .addTextRange([0, 0]) .build(); const localOp = new Operation(OperationType.Insert, localIndex, { diff --git a/packages/collaboration-manager/src/OperationsTransformer.ts b/packages/collaboration-manager/src/OperationsTransformer.ts index 144de9ea..734f6f75 100644 --- a/packages/collaboration-manager/src/OperationsTransformer.ts +++ b/packages/collaboration-manager/src/OperationsTransformer.ts @@ -170,10 +170,13 @@ export class OperationsTransformer { return Operation.from(operation); } + const sameInput = index.dataKey === againstIndex.dataKey; + const sameBlock = index.blockIndex === againstIndex.blockIndex; + /** * Check that againstOp affects current operation */ - if (index.dataKey === againstIndex.dataKey && index.blockIndex === againstIndex.blockIndex && againstIndex.textRange![0] >= index.textRange![1]) { + if (sameInput && sameBlock && againstIndex.textRange![0] > index.textRange![1]) { return Operation.from(operation); } @@ -220,6 +223,7 @@ export class OperationsTransformer { switch (intersectionType) { case (RangeIntersectionType.None): + case (RangeIntersectionType.Left): newIndexBuilder.addTextRange([index.textRange![0] + insertedLength, index.textRange![1] + insertedLength]); break; diff --git a/packages/collaboration-manager/src/utils/getRangesIntersectionType.ts b/packages/collaboration-manager/src/utils/getRangesIntersectionType.ts index ac956714..26425bab 100644 --- a/packages/collaboration-manager/src/utils/getRangesIntersectionType.ts +++ b/packages/collaboration-manager/src/utils/getRangesIntersectionType.ts @@ -25,7 +25,7 @@ export function getRangesIntersectionType(range: TextRange, rangeToCompare: Text /** * Range is fully on the left or right of the range to compare */ - if (end <= startToCompare || start >= endToCompare) { + if (end < startToCompare || start > endToCompare) { return RangeIntersectionType.None; } @@ -33,7 +33,7 @@ export function getRangesIntersectionType(range: TextRange, rangeToCompare: Text * Range includes the range to compare * If two ranges are the same, intersection type is "includes" */ - if (start <= startToCompare && end >= endToCompare) { + if (start <= startToCompare && end >= endToCompare && start != end) { return RangeIntersectionType.Includes; } From 0093cea27628ad06979ee4e6a011f7cc1aeaf87e Mon Sep 17 00:00:00 2001 From: e11sy <130844513+e11sy@users.noreply.github.com> Date: Sun, 27 Apr 2025 18:51:28 +0300 Subject: [PATCH 11/15] chore(): lint fix --- .../collaboration-manager/src/BatchedOperation.spec.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/collaboration-manager/src/BatchedOperation.spec.ts b/packages/collaboration-manager/src/BatchedOperation.spec.ts index ed1b3e98..772e703d 100644 --- a/packages/collaboration-manager/src/BatchedOperation.spec.ts +++ b/packages/collaboration-manager/src/BatchedOperation.spec.ts @@ -1,4 +1,5 @@ -import { createDataKey, Index, IndexBuilder, type TextRange } from '@editorjs/model'; +import type { Index } from '@editorjs/model'; +import { createDataKey, IndexBuilder, type TextRange } from '@editorjs/model'; import { BatchedOperation } from './BatchedOperation.js'; import type { SerializedOperation } from './Operation.js'; import { Operation, OperationType } from './Operation.js'; @@ -98,10 +99,10 @@ describe('Batch', () => { expect(transformedBatch).not.toBeNull(); expect(transformedBatch!.operations.length).toBe(2); // Check if text ranges were shifted by 1 due to insertion - /* eslint-disable @typescript-eslint-no-magic-numbers */ + /* eslint-disable @typescript-eslint/no-magic-numbers */ expect(transformedBatch!.operations[0].index.textRange![0]).toBe(2); expect(transformedBatch!.operations[1].index.textRange![0]).toBe(3); - /* eslint-enable @typescript-eslint-no-magic-numbers */ + /* eslint-enable @typescript-eslint/no-magic-numbers */ }); it('should return batch with Neutral operations if no operations can be transformed', () => { From a8c2de8033dfea0815880bbf6ff69177580c5b9a Mon Sep 17 00:00:00 2001 From: e11sy <130844513+e11sy@users.noreply.github.com> Date: Sun, 15 Jun 2025 17:11:02 +0300 Subject: [PATCH 12/15] test: recover operation tests --- .../src/CollaborationManager.ts | 16 +- .../src/Operation.spec.ts | 233 +++++++++++++++++- .../src/OperationsTransformer.ts | 9 +- 3 files changed, 244 insertions(+), 14 deletions(-) diff --git a/packages/collaboration-manager/src/CollaborationManager.ts b/packages/collaboration-manager/src/CollaborationManager.ts index 127af9d6..f1cb00d0 100644 --- a/packages/collaboration-manager/src/CollaborationManager.ts +++ b/packages/collaboration-manager/src/CollaborationManager.ts @@ -91,7 +91,7 @@ export class CollaborationManager { * Undo last operation in the local stack */ public undo(): void { - this.#emptyBatch(); + this.#moveBatchToUndo(); const operation = this.#undoRedoManager.undo(); @@ -112,7 +112,7 @@ export class CollaborationManager { * Redo last undone operation in the local stack */ public redo(): void { - this.#emptyBatch(); + this.#moveBatchToUndo(); const operation = this.#undoRedoManager.redo(); @@ -123,13 +123,7 @@ export class CollaborationManager { // Disable event handling this.#shouldHandleEvents = false; - if (operation instanceof BatchedOperation) { - operation.operations.forEach((op) => { - this.applyOperation(op); - }); - } else { - this.applyOperation(operation); - } + this.applyOperation(operation); // Re-enable event handling this.#shouldHandleEvents = true; @@ -265,7 +259,7 @@ export class CollaborationManager { * @todo - add debounce timeout 500ms */ if (!this.#currentBatch.canAdd(operation)) { - this.#undoRedoManager.put(this.#currentBatch); + this.#moveBatchToUndo(); this.#currentBatch = new BatchedOperation(operation); @@ -278,7 +272,7 @@ export class CollaborationManager { /** * Puts current batch to the undo stack and clears the batch */ - #emptyBatch(): void { + #moveBatchToUndo(): void { if (this.#currentBatch !== null) { this.#undoRedoManager.put(this.#currentBatch); diff --git a/packages/collaboration-manager/src/Operation.spec.ts b/packages/collaboration-manager/src/Operation.spec.ts index 903a3bd2..fc8fb21b 100644 --- a/packages/collaboration-manager/src/Operation.spec.ts +++ b/packages/collaboration-manager/src/Operation.spec.ts @@ -1,5 +1,5 @@ /* eslint-disable @typescript-eslint/no-magic-numbers */ -import type { BlockNodeSerialized, DataKey } from '@editorjs/model'; +import type { BlockNodeSerialized, DataKey, DocumentIndex } from '@editorjs/model'; import { IndexBuilder } from '@editorjs/model'; import { describe } from '@jest/globals'; import { type InsertOrDeleteOperationData, type ModifyOperationData, Operation, OperationType } from './Operation.js'; @@ -10,7 +10,8 @@ const createOperation = ( // eslint-disable-next-line @typescript-eslint/no-explicit-any value: string | [ BlockNodeSerialized ] | Record, // eslint-disable-next-line @typescript-eslint/no-explicit-any - prevValue?: Record + prevValue?: Record, + endIndex?: number ): Operation => { const index = new IndexBuilder() .addBlockIndex(0); @@ -40,6 +41,234 @@ const createOperation = ( describe('Operation', () => { + describe('.transform()', () => { + it('should not change operation if document ids are different', () => { + const receivedOp = createOperation(OperationType.Insert, 0, 'abc'); + const localOp = createOperation(OperationType.Insert, 0, 'def'); + + localOp.index.documentId = 'document2' as DocumentIndex; + const transformedOp = receivedOp.transform(localOp); + + expect(transformedOp).toEqual(receivedOp); + }); + + it('should not change operation if data keys are different', () => { + const receivedOp = createOperation(OperationType.Insert, 0, 'abc'); + const localOp = createOperation(OperationType.Insert, 0, 'def'); + + localOp.index.dataKey = 'dataKey2' as DataKey; + + const transformedOp = receivedOp.transform(localOp); + + expect(transformedOp).toEqual(receivedOp); + }); + + it('should throw Unsupppoted index type error if op is not Block or Text operation', () => { + const receivedOp = createOperation(OperationType.Insert, 0, 'abc'); + const localOp = createOperation(OperationType.Insert, 0, 'def'); + + localOp.index.textRange = undefined; + + try { + receivedOp.transform(localOp); + } catch (e) { + expect(e).toBeInstanceOf(Error); + expect((e as Error).message).toContain('Unsupported index type'); + } + }); + + it('should throw an error if unsupported operation type is provided', () => { + const receivedOp = createOperation(OperationType.Insert, 0, 'def'); + // @ts-expect-error — for test purposes + const localOp = createOperation('unsupported', 0, 'def'); + + expect(() => receivedOp.transform(localOp)).toThrow('Unsupported operation type'); + }); + + it('should not transform relative to the Modify operation (as Modify operation doesn\'t change index)', () => { + const receivedOp = createOperation(OperationType.Insert, 0, 'abc'); + const localOp = createOperation(OperationType.Modify, 0, 'def'); + const transformedOp = receivedOp.transform(localOp); + + expect(transformedOp).toEqual(receivedOp); + }); + + describe('Transformation relative to Insert operation', () => { + it('should not change a received operation if it is before a local one', () => { + const receivedOp = createOperation(OperationType.Insert, 0, 'abc'); + const localOp = createOperation(OperationType.Insert, 3, 'def'); + const transformedOp = receivedOp.transform(localOp); + + expect(transformedOp).toEqual(receivedOp); + }); + + it('should transform an index for a received operation if it is after a local one', () => { + const receivedOp = createOperation(OperationType.Delete, 3, 'def'); + const localOp = createOperation(OperationType.Insert, 0, 'abc'); + const transformedOp = receivedOp.transform(localOp); + + expect(transformedOp.index.textRange).toEqual([6, 6]); + }); + + it('should transform a received operation if it is at the same position as a local one', () => { + const receivedOp = createOperation(OperationType.Modify, 0, 'abc'); + const localOp = createOperation(OperationType.Insert, 0, 'def'); + const transformedOp = receivedOp.transform(localOp); + + expect(transformedOp.index.textRange).toEqual([3, 3]); + }); + + it('should not change the text index if local op is a Block operation', () => { + const receivedOp = createOperation(OperationType.Modify, 0, 'abc'); + const localOp = createOperation(OperationType.Insert, 0, [ { + name: 'paragraph', + data: { text: 'hello' }, + } ]); + const transformedOp = receivedOp.transform(localOp); + + expect(transformedOp.index.textRange).toEqual([0, 0]); + }); + + it('should not change the operation if local op is a Block operation after a received one', () => { + const receivedOp = createOperation(OperationType.Insert, 0, [ { + name: 'paragraph', + data: { text: 'abc' }, + } ]); + const localOp = createOperation(OperationType.Insert, 1, [ { + name: 'paragraph', + data: { text: 'hello' }, + } ]); + + const transformedOp = receivedOp.transform(localOp); + + expect(transformedOp).toEqual(receivedOp); + }); + + it('should adjust the block index if local op is a Block operation before a received one', () => { + const receivedOp = createOperation(OperationType.Insert, 1, [ { + name: 'paragraph', + data: { text: 'abc' }, + } ]); + const localOp = createOperation(OperationType.Insert, 0, [ { + name: 'paragraph', + data: { text: 'hello' }, + } ]); + + const transformedOp = receivedOp.transform(localOp); + + expect(transformedOp.index.blockIndex).toEqual(2); + }); + + it('should adjust the block index if local op is a Block operation at the same index as a received one', () => { + const receivedOp = createOperation(OperationType.Insert, 0, [ { + name: 'paragraph', + data: { text: 'abc' }, + } ]); + const localOp = createOperation(OperationType.Insert, 0, [ { + name: 'paragraph', + data: { text: 'hello' }, + } ]); + + const transformedOp = receivedOp.transform(localOp); + + expect(transformedOp.index.blockIndex).toEqual(1); + }); + }); + + describe('Transformation relative to Delete operation', () => { + it('should not change a received operation if it is before a local one', () => { + const receivedOp = createOperation(OperationType.Insert, 0, 'abc'); + const localOp = createOperation(OperationType.Delete, 3, 'def'); + const transformedOp = receivedOp.transform(localOp); + + expect(transformedOp).toEqual(receivedOp); + }); + + it('should transform an index for a received operation if it is after a local one', () => { + const receivedOp = createOperation(OperationType.Delete, 3, 'def'); + const localOp = createOperation(OperationType.Delete, 0, 'abc'); + const transformedOp = receivedOp.transform(localOp); + + expect(transformedOp.index.textRange).toEqual([0, 0]); + }); + + it('should transform a received operation if it is at the same position as a local one', () => { + const receivedOp = createOperation(OperationType.Modify, 3, 'abc'); + const localOp = createOperation(OperationType.Delete, 0, 'def', undefined, 3); + const transformedOp = receivedOp.transform(localOp); + + expect(transformedOp.index.textRange).toEqual([0, 0]); + }); + + it('should not change the text index if local op is a Block operation', () => { + const receivedOp = createOperation(OperationType.Modify, 1, 'abc'); + const localOp = createOperation(OperationType.Delete, 0, [ { + name: 'paragraph', + data: { text: 'hello' }, + } ]); + const transformedOp = receivedOp.transform(localOp); + + expect(transformedOp.index.textRange).toEqual([1, 1]); + }); + + it('should not change the text index if local op is a Block operation', () => { + const receivedOp = createOperation(OperationType.Modify, 0, 'abc'); + const localOp = createOperation(OperationType.Insert, 0, [ { + name: 'paragraph', + data: { text: 'hello' }, + } ]); + const transformedOp = receivedOp.transform(localOp); + + expect(transformedOp.index.textRange).toEqual([0, 0]); + }); + + it('should not change the operation if local op is a Block operation after a received one', () => { + const receivedOp = createOperation(OperationType.Insert, 0, [ { + name: 'paragraph', + data: { text: 'abc' }, + } ]); + const localOp = createOperation(OperationType.Delete, 1, [ { + name: 'paragraph', + data: { text: 'hello' }, + } ]); + + const transformedOp = receivedOp.transform(localOp); + + expect(transformedOp).toEqual(receivedOp); + }); + + it('should adjust the block index if local op is a Block operation before a received one', () => { + const receivedOp = createOperation(OperationType.Insert, 1, [ { + name: 'paragraph', + data: { text: 'abc' }, + } ]); + const localOp = createOperation(OperationType.Delete, 0, [ { + name: 'paragraph', + data: { text: 'hello' }, + } ]); + + const transformedOp = receivedOp.transform(localOp); + + expect(transformedOp.index.blockIndex).toEqual(0); + }); + + it('should return Neutral operation if local op is a Block operation at the same index as a received one', () => { + const receivedOp = createOperation(OperationType.Insert, 1, [ { + name: 'paragraph', + data: { text: 'abc' }, + } ]); + const localOp = createOperation(OperationType.Delete, 1, [ { + name: 'paragraph', + data: { text: 'hello' }, + } ]); + + const transformedOp = receivedOp.transform(localOp); + + expect(transformedOp.type).toBe(OperationType.Neutral); + }); + }); + }); + describe('.inverse()', () => { it('should change the type of Insert operation to Delete operation', () => { const op = createOperation(OperationType.Insert, 0, 'abc'); diff --git a/packages/collaboration-manager/src/OperationsTransformer.ts b/packages/collaboration-manager/src/OperationsTransformer.ts index 734f6f75..e58b7a4c 100644 --- a/packages/collaboration-manager/src/OperationsTransformer.ts +++ b/packages/collaboration-manager/src/OperationsTransformer.ts @@ -20,6 +20,13 @@ export class OperationsTransformer { if (operation.index.documentId !== againstOp.index.documentId) { return Operation.from(operation); } + + /** + * Throw unsupported operation type error if operation type is not supported + */ + if (!Object.values(OperationType).includes(againstOp.type) || !Object.values(OperationType).includes(operation.type)) { + throw new Error('Unsupported operation type') + } return this.#applyTransformation(operation, againstOp); } @@ -176,7 +183,7 @@ export class OperationsTransformer { /** * Check that againstOp affects current operation */ - if (sameInput && sameBlock && againstIndex.textRange![0] > index.textRange![1]) { + if (!sameInput || !sameBlock || againstIndex.textRange![0] > index.textRange![1]) { return Operation.from(operation); } From 2647a7e76d29d1c3945e7de84cc2b652428e1910 Mon Sep 17 00:00:00 2001 From: e11sy <130844513+e11sy@users.noreply.github.com> Date: Sun, 15 Jun 2025 17:22:45 +0300 Subject: [PATCH 13/15] imp(UndoRedoManager): stacks transformation logic improved --- .../src/OperationsTransformer.ts | 14 +++----- .../src/UndoRedoManager.ts | 36 +++++++------------ 2 files changed, 18 insertions(+), 32 deletions(-) diff --git a/packages/collaboration-manager/src/OperationsTransformer.ts b/packages/collaboration-manager/src/OperationsTransformer.ts index e58b7a4c..fa40a5da 100644 --- a/packages/collaboration-manager/src/OperationsTransformer.ts +++ b/packages/collaboration-manager/src/OperationsTransformer.ts @@ -120,14 +120,12 @@ export class OperationsTransformer { * Cover case 2 */ case OperationType.Delete: - if (operation.index.blockIndex !== undefined) { - if (againstOp.index.blockIndex! < operation.index.blockIndex!) { - newIndexBuilder.addBlockIndex(operation.index.blockIndex! - 1); - } else { - return new Operation(OperationType.Neutral, newIndexBuilder.build(), { payload: [] }, operation.userId, operation.rev); - } + if (againstOp.index.blockIndex! >= operation.index.blockIndex!) { + return new Operation(OperationType.Neutral, newIndexBuilder.build(), { payload: [] }, operation.userId, operation.rev); } + newIndexBuilder.addBlockIndex(operation.index.blockIndex! - 1); + break; /** @@ -301,9 +299,7 @@ export class OperationsTransformer { * Cover case 2.2 */ case (RangeIntersectionType.Right): - const overlapLength = index.textRange![1] - againstIndex.textRange![0]; - - newIndexBuilder.addTextRange([index.textRange![0], index.textRange![1] - overlapLength]); + newIndexBuilder.addTextRange([index.textRange![0], againstIndex.textRange![0]]); break; /** diff --git a/packages/collaboration-manager/src/UndoRedoManager.ts b/packages/collaboration-manager/src/UndoRedoManager.ts index a4110ba0..de909c04 100644 --- a/packages/collaboration-manager/src/UndoRedoManager.ts +++ b/packages/collaboration-manager/src/UndoRedoManager.ts @@ -14,6 +14,17 @@ export class UndoRedoManager { */ #redoStack: Operation[] = []; + /** + * Transforms passed operations stack against the operation + * + * @param operation - operation to transform against + * @param stack - stack to transform + * @returns - new transformed list of operations + */ + private transformStack(operation: Operation, stack: Operation[]): Operation[] { + return stack.map((op: Operation) => op.transform(operation)); + } + /** * Returns operation to undo (if any) */ @@ -65,28 +76,7 @@ export class UndoRedoManager { * @param operation - operation to transform against */ public transformStacks(operation: Operation): void { - this.transformStack(operation, this.#undoStack); - this.transformStack(operation, this.#redoStack); - } - - /** - * Transforms passed operations stack against the operation - * - * @param operation - operation to transform against - * @param stack - stack to transform - */ - public transformStack(operation: Operation, stack: Operation[]): void { - const transformed = stack.flatMap((op) => { - const transformedOp = op.transform(operation); - - if (transformedOp === null) { - return []; - } - - return [ transformedOp ]; - }); - - stack.length = 0; - stack.push(...transformed); + this.#undoStack = this.transformStack(operation, this.#undoStack); + this.#redoStack = this.transformStack(operation, this.#redoStack); } } From 61d4dab4217141e719ccb65c74cb81501aace118 Mon Sep 17 00:00:00 2001 From: e11sy <130844513+e11sy@users.noreply.github.com> Date: Sun, 15 Jun 2025 17:59:44 +0300 Subject: [PATCH 14/15] feat: add debounce clear batch timer --- .../src/CollaborationManager.ts | 23 +++++++++++++++++-- .../src/UndoRedoManager.ts | 22 +++++++++--------- 2 files changed, 32 insertions(+), 13 deletions(-) diff --git a/packages/collaboration-manager/src/CollaborationManager.ts b/packages/collaboration-manager/src/CollaborationManager.ts index f1cb00d0..b0d918e6 100644 --- a/packages/collaboration-manager/src/CollaborationManager.ts +++ b/packages/collaboration-manager/src/CollaborationManager.ts @@ -14,6 +14,8 @@ import { BatchedOperation } from './BatchedOperation.js'; import { type ModifyOperationData, Operation, OperationType } from './Operation.js'; import { UndoRedoManager } from './UndoRedoManager.js'; +const DEBOUCE_TIMEOUT = 500; + /** * CollaborationManager listens to EditorJSModel events and applies operations */ @@ -48,6 +50,11 @@ export class CollaborationManager { */ #client: OTClient | null = null; + /** + * Debounce timer to move current batch to undo stack after a delay + */ + #debounceTimer?: ReturnType; + /** * Creates an instance of CollaborationManager * @@ -249,24 +256,25 @@ export class CollaborationManager { */ if (this.#currentBatch === null) { this.#currentBatch = new BatchedOperation(operation); + this.#debounce(); return; } /** * If current operation could not be added to the batch, then terminate current batch and create a new one with current operation - * - * @todo - add debounce timeout 500ms */ if (!this.#currentBatch.canAdd(operation)) { this.#moveBatchToUndo(); this.#currentBatch = new BatchedOperation(operation); + this.#debounce(); return; } this.#currentBatch.add(operation); + this.#debounce(); } /** @@ -279,4 +287,15 @@ export class CollaborationManager { this.#currentBatch = null; } } + + /** + * Debouneces timer of #moveBatchToUndo method + */ + #debounce(): void { + clearTimeout(this.#debounceTimer); + + this.#debounceTimer = setTimeout(() => { + this.#moveBatchToUndo(); + }, DEBOUCE_TIMEOUT); + } } diff --git a/packages/collaboration-manager/src/UndoRedoManager.ts b/packages/collaboration-manager/src/UndoRedoManager.ts index de909c04..54637ce4 100644 --- a/packages/collaboration-manager/src/UndoRedoManager.ts +++ b/packages/collaboration-manager/src/UndoRedoManager.ts @@ -14,17 +14,6 @@ export class UndoRedoManager { */ #redoStack: Operation[] = []; - /** - * Transforms passed operations stack against the operation - * - * @param operation - operation to transform against - * @param stack - stack to transform - * @returns - new transformed list of operations - */ - private transformStack(operation: Operation, stack: Operation[]): Operation[] { - return stack.map((op: Operation) => op.transform(operation)); - } - /** * Returns operation to undo (if any) */ @@ -79,4 +68,15 @@ export class UndoRedoManager { this.#undoStack = this.transformStack(operation, this.#undoStack); this.#redoStack = this.transformStack(operation, this.#redoStack); } + + /** + * Transforms passed operations stack against the operation + * + * @param operation - operation to transform against + * @param stack - stack to transform + * @returns - new transformed list of operations + */ + private transformStack(operation: Operation, stack: Operation[]): Operation[] { + return stack.map((op: Operation) => op.transform(operation)); + } } From a74d3937524f5603b4f02336d10395b2123e97d9 Mon Sep 17 00:00:00 2001 From: e11sy <130844513+e11sy@users.noreply.github.com> Date: Sun, 15 Jun 2025 18:03:22 +0300 Subject: [PATCH 15/15] fix: lint --- packages/collaboration-manager/src/Operation.spec.ts | 5 ++--- packages/collaboration-manager/src/OperationsTransformer.ts | 4 ++-- packages/collaboration-manager/src/UndoRedoManager.ts | 2 +- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/packages/collaboration-manager/src/Operation.spec.ts b/packages/collaboration-manager/src/Operation.spec.ts index fc8fb21b..50f21ead 100644 --- a/packages/collaboration-manager/src/Operation.spec.ts +++ b/packages/collaboration-manager/src/Operation.spec.ts @@ -10,8 +10,7 @@ const createOperation = ( // eslint-disable-next-line @typescript-eslint/no-explicit-any value: string | [ BlockNodeSerialized ] | Record, // eslint-disable-next-line @typescript-eslint/no-explicit-any - prevValue?: Record, - endIndex?: number + prevValue?: Record ): Operation => { const index = new IndexBuilder() .addBlockIndex(0); @@ -194,7 +193,7 @@ describe('Operation', () => { it('should transform a received operation if it is at the same position as a local one', () => { const receivedOp = createOperation(OperationType.Modify, 3, 'abc'); - const localOp = createOperation(OperationType.Delete, 0, 'def', undefined, 3); + const localOp = createOperation(OperationType.Delete, 0, 'def'); const transformedOp = receivedOp.transform(localOp); expect(transformedOp.index.textRange).toEqual([0, 0]); diff --git a/packages/collaboration-manager/src/OperationsTransformer.ts b/packages/collaboration-manager/src/OperationsTransformer.ts index fa40a5da..46ba29ae 100644 --- a/packages/collaboration-manager/src/OperationsTransformer.ts +++ b/packages/collaboration-manager/src/OperationsTransformer.ts @@ -20,12 +20,12 @@ export class OperationsTransformer { if (operation.index.documentId !== againstOp.index.documentId) { return Operation.from(operation); } - + /** * Throw unsupported operation type error if operation type is not supported */ if (!Object.values(OperationType).includes(againstOp.type) || !Object.values(OperationType).includes(operation.type)) { - throw new Error('Unsupported operation type') + throw new Error('Unsupported operation type'); } return this.#applyTransformation(operation, againstOp); diff --git a/packages/collaboration-manager/src/UndoRedoManager.ts b/packages/collaboration-manager/src/UndoRedoManager.ts index 54637ce4..93157808 100644 --- a/packages/collaboration-manager/src/UndoRedoManager.ts +++ b/packages/collaboration-manager/src/UndoRedoManager.ts @@ -74,7 +74,7 @@ export class UndoRedoManager { * * @param operation - operation to transform against * @param stack - stack to transform - * @returns - new transformed list of operations + * @returns {Operation[]} - new transformed list of operations */ private transformStack(operation: Operation, stack: Operation[]): Operation[] { return stack.map((op: Operation) => op.transform(operation));