Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement transformations #92

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .github/workflows/collaboration-manager.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,14 @@ jobs:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4

- run: yarn

- name: Build the package
uses: ./.github/actions/build
with:
package-name: '@editorjs/model'

- name: Run ESLint check
uses: ./.github/actions/lint
with:
Expand Down
164 changes: 164 additions & 0 deletions packages/collaboration-manager/src/Operation.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,164 @@
/* eslint-disable @typescript-eslint/no-magic-numbers */
import type { DataKey, DocumentIndex } from '@editorjs/model';
import { IndexBuilder } from '@editorjs/model';
import { Operation, OperationType } from './Operation.js';

describe('Operation', () => {
const createOperation = (type: OperationType, startIndex: number, value: string): Operation => {
return new Operation(
type,
new IndexBuilder()
.addBlockIndex(0)
.addDataKey('text' as DataKey)
.addTextRange([startIndex, startIndex])
.build(),
{
prevValue: type === OperationType.Delete ? value : '',
newValue: type === OperationType.Insert ? value : '',
Comment on lines +16 to +17
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pls remove this for Add/Remove opeeration, leave just data

}
);
};

describe('Insert vs Insert', () => {
test('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);
});

test('Should adjust an index for a received operation if it is after a local one', () => {
const receivedOp = createOperation(OperationType.Insert, 3, 'def');
const localOp = createOperation(OperationType.Insert, 0, 'abc');
const transformedOp = receivedOp.transform(localOp);

expect(transformedOp.index.textRange).toEqual([6, 6]);
});

test('Should not change a received operation if it is at the same position as a local one', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If position is the same, the index should change by the length of text in received operation

const receivedOp = createOperation(OperationType.Insert, 0, 'abc');
const localOp = createOperation(OperationType.Insert, 0, 'def');
const transformedOp = receivedOp.transform(localOp);

expect(transformedOp).toEqual(receivedOp);
});
});

describe('Delete vs Delete', () => {
test('Should not change a received operation if it is before a local one', () => {
const receivedOp = createOperation(OperationType.Delete, 0, 'abc');
const localOp = createOperation(OperationType.Delete, 3, 'def');
const transformedOp = receivedOp.transform(localOp);

expect(transformedOp).toEqual(receivedOp);
});

test('Should adjust 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]);
});

test('Should adjust an index for a received operation if it is at the same position as a local one', () => {
const receivedOp = createOperation(OperationType.Delete, 0, 'abc');
const localOp = createOperation(OperationType.Delete, 0, 'abc');
const transformedOp = receivedOp.transform(localOp);

expect(transformedOp.index.textRange).toEqual([0, 0]);
});
});

describe('Insert vs Delete', () => {
test('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);
});

test('Should adjust an index for a received operation if it is after a local one', () => {
const receivedOp = createOperation(OperationType.Insert, 6, 'ghi');
const localOp = createOperation(OperationType.Delete, 0, 'abc');
const transformedOp = receivedOp.transform(localOp);

expect(transformedOp.index.textRange).toEqual([3, 3]);
});

test('Should not change a received operation if it is at the same position as a local one', () => {
const receivedOp = createOperation(OperationType.Insert, 3, 'def');
const localOp = createOperation(OperationType.Delete, 3, 'ghi');
const transformedOp = receivedOp.transform(localOp);

expect(transformedOp).toEqual(receivedOp);
});
});

describe('Delete vs Insert', () => {
test('Should not change a received operation if it is before a local one', () => {
const receivedOp = createOperation(OperationType.Delete, 0, 'abc');
const localOp = createOperation(OperationType.Insert, 3, 'def');
const transformedOp = receivedOp.transform(localOp);

expect(transformedOp).toEqual(receivedOp);
});

test('Should adjust an index for a received operation if it is after a local one', () => {
const receivedOp = createOperation(OperationType.Delete, 6, 'ghi');
const localOp = createOperation(OperationType.Insert, 0, 'abc');
const transformedOp = receivedOp.transform(localOp);

expect(transformedOp.index.textRange).toEqual([9, 9]);
});

test('Should adjust an index for a received operation if it is at the same position as a local one', () => {
const receivedOp = createOperation(OperationType.Delete, 3, 'def');
const localOp = createOperation(OperationType.Insert, 3, 'ghi');
const transformedOp = receivedOp.transform(localOp);

expect(transformedOp.index.textRange).toEqual([6, 6]);
});
});

describe('Edge cases', () => {
test('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);
});

test('Should not change operation if blocks are different', () => {
const receivedOp = createOperation(OperationType.Insert, 0, 'abc');
const localOp = createOperation(OperationType.Insert, 0, 'def');

localOp.index.blockIndex = 1;

const transformedOp = receivedOp.transform(localOp);

expect(transformedOp).toEqual(receivedOp);
});

test('Should throw an error if unsupported index type is provided', () => {
const receivedOp = createOperation(OperationType.Insert, 0, 'def');

receivedOp.index.textRange = undefined;
const localOp = createOperation(OperationType.Insert, 0, 'def');

expect(() => receivedOp.transform(localOp)).toThrow('Unsupported index');
});

test('Should throw an error if unsupported operation type is provided', () => {
const receivedOp = createOperation(OperationType.Modify, 0, 'def');
const localOp = createOperation(OperationType.Insert, 0, 'def');

expect(() => receivedOp.transform(localOp)).toThrow('Unsupported operation type');
});
});
});
128 changes: 127 additions & 1 deletion packages/collaboration-manager/src/Operation.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { Index } from '@editorjs/model';
import { IndexBuilder, type Index } from '@editorjs/model';

/**
* Type of the operation
Expand Down Expand Up @@ -56,4 +56,130 @@ export class Operation {
this.index = index;
this.data = data;
}

/**
* Makes an inverse operation
*/
public inverse(): Operation {
const index = this.index;

switch (this.type) {
case OperationType.Insert:

const textRange = index.textRange;

if (textRange == undefined) {
throw new Error('Unsupported index');
}

const [ textRangeStart ] = textRange;

const newIndex = new IndexBuilder()
.from(index)
.addTextRange([textRangeStart, textRangeStart + this.data.newValue.length])
.build();

return new Operation(OperationType.Delete, newIndex, {
prevValue: this.data.newValue,
newValue: this.data.prevValue,
});
case OperationType.Delete:
return new Operation(OperationType.Insert, index, {
prevValue: this.data.newValue,
newValue: this.data.prevValue,
});
case OperationType.Modify:
return new Operation(OperationType.Modify, index, {
prevValue: this.data.newValue,
newValue: this.data.prevValue,
});
}
}

/**
* Transforms the operation against another operation
*
* @param againstOp - operation to transform against
*/
public transform(againstOp: Operation): Operation {
if (!this.index.isTextIndex || !againstOp.index.isTextIndex) {
throw new Error('Unsupported index');
}

if (this.type === OperationType.Modify || againstOp.type === OperationType.Modify) {
throw new Error('Unsupported operation type');
}

/**
* Do not transform operations if they are on different blocks or documents
*/
if (this.index.documentId !== againstOp.index.documentId || this.index.blockIndex !== againstOp.index.blockIndex) {
return this;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to consider block index for text operations, if block before was added or deleted, we need to update the index

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be done in a separate PR

}

const [ receivedStartIndex ] = this.index.textRange!;
const [ localStartIndex ] = againstOp.index.textRange!;

switch (true) {
case this.type === OperationType.Insert && againstOp.type === OperationType.Insert:
if (receivedStartIndex <= localStartIndex) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The type of the current operation doesn't matter here, index will be changed the same way for both insert and delte

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are only three cases here:

  1. If received operation is delete operatio before (index-wise) the current, we need to update the current one index shifting it left
  2. If received operation is insert operation before (index-wise) the current, we need to shift index right
  3. If operation of any type happened after (index-wise) the current, no need to do anything

return this;
} else {
return this.shiftOperation(againstOp.data.newValue.length);
}

case this.type === OperationType.Delete && againstOp.type === OperationType.Delete:
if (receivedStartIndex < localStartIndex) {
return this;
} else if (receivedStartIndex > localStartIndex) {
return this.shiftOperation(-againstOp.data.prevValue.length);
} else {
// If both delete at the same index, adjust the length of deletion
const minLength = Math.min(this.data.prevValue.length, againstOp.data.prevValue.length);

return new Operation(OperationType.Delete, this.index, {
prevValue: this.data.prevValue.slice(minLength),
newValue: '',
});
}

case this.type === OperationType.Insert && againstOp.type === OperationType.Delete:
if (receivedStartIndex <= localStartIndex) {
return this;
} else {
return this.shiftOperation(-againstOp.data.prevValue.length);
}

case this.type === OperationType.Delete && againstOp.type === OperationType.Insert:
if (receivedStartIndex < localStartIndex) {
return this;
} else {
return this.shiftOperation(againstOp.data.newValue.length);
}

default:
throw new Error('Unsupported operation type');
}
}

/**
* Shifts the operation by the given shift value (by adjusting the text range)
*
* @param shift - shift value
*/
private shiftOperation(shift: number): Operation {
if (!this.index.isTextIndex) {
throw new Error('Unsupported index');
}

const [ textRangeStart ] = this.index.textRange!;

return new Operation(
this.type,
new IndexBuilder().from(this.index)
.addTextRange([textRangeStart + shift, textRangeStart + shift])
.build(),
this.data
);
}
}
50 changes: 0 additions & 50 deletions packages/collaboration-manager/src/Transformer.ts

This file was deleted.

7 changes: 3 additions & 4 deletions packages/collaboration-manager/src/UndoRedoManager.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import type { Operation } from './Operation.js';
import { Transformer } from './Transformer.js';

/**
* Manages undo and redo operations
Expand All @@ -25,7 +24,7 @@ export class UndoRedoManager {
return;
}

const inversedOperation = Transformer.inverse(operation);
const inversedOperation = operation.inverse();

this.#redoStack.push(inversedOperation);

Expand All @@ -42,9 +41,9 @@ export class UndoRedoManager {
return;
}

const inversedOperation = Transformer.inverse(operation);
const inversedOperation = operation.inverse();

this.#undoStack.push(Transformer.inverse(inversedOperation));
this.#undoStack.push(operation);

return inversedOperation;
}
Expand Down
Loading