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.

Sign up for GitHub

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

[CMS-307] #295

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

[CMS-307] #295

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5a6f3ae
go part cms-307
hdphuong Jan 31, 2023
f674bde
i don't remember what this change does but it looks legit lol
hdphuong Feb 26, 2023
48914e1
remove the wacky $ sign and have OperationType as a proper exported f…
hdphuong Feb 26, 2023
e976894
handle typescript opType
hdphuong Feb 26, 2023
67d1601
use enum
hdphuong Feb 27, 2023
818b1a8
Delete package-lock.json
hdphuong Feb 27, 2023
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
48 changes: 24 additions & 24 deletions backend/editor/OT/OTClient/operation.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ describe("String transformation functions", () => {

describe("insert insert", () => {
test("non overlap", () => {
const o1 = new StringOperation(1, 2, "1");
const o2 = new StringOperation(2, 3, "2");
const o1 = new StringOperation(1, 2, "1", "insert");
const o2 = new StringOperation(2, 3, "2", "insert");
expect(apply(o1, s)).toBe("a1bcde");
expect(apply(o2, s)).toBe("ab2cde");

Expand All @@ -25,17 +25,17 @@ describe("String transformation functions", () => {
});

test("same location", () => {
const o1 = new StringOperation(1, 2, "2");
const o2 = new StringOperation(1, 2, "1");
const o1 = new StringOperation(1, 2, "2", "insert");
const o2 = new StringOperation(1, 2, "1", "insert");
const [o1_t1, o2_t1] = o1.transformAgainst(o2);
const [o1_t2, o2_t2] = o2.transformAgainst(o1);
expect(apply(o1_t1, apply(o2_t1, s))).toBe("a12bcde");
expect(apply(o2_t2, apply(o1_t2, s))).toBe("a12bcde");
});

test("overlap", () => {
const o1 = new StringOperation(1, 3, "11");
const o2 = new StringOperation(2, 4, "22");
const o1 = new StringOperation(1, 3, "11", "insert");
const o2 = new StringOperation(2, 4, "22", "insert");
const [o1_t1, o2_t1] = o1.transformAgainst(o2);
const [o1_t2, o2_t2] = o2.transformAgainst(o1);
expect(apply(o1_t1, apply(o2_t1, s))).toBe("a11b22cde");
Expand All @@ -45,53 +45,53 @@ describe("String transformation functions", () => {

describe("insert delete", () => {
test("non overlap", () => {
const o1 = new StringOperation(1, 2, "1");
const o2 = new StringOperation(2, 3, "");
const o1 = new StringOperation(1, 2, "1", "insert");
const o2 = new StringOperation(2, 3, "", "delete");
const [o1_t1, o2_t1] = o1.transformAgainst(o2);
const [o1_t2, o2_t2] = o2.transformAgainst(o1);
expect(apply(o1_t1, apply(o2_t1, s))).toBe("a1bde");
expect(apply(o2_t2, apply(o1_t2, s))).toBe("a1bde");
});

test("same location", () => {
const o1 = new StringOperation(1, 2, "1");
const o2 = new StringOperation(0, 1, "");
const o1 = new StringOperation(1, 2, "1", "insert");
const o2 = new StringOperation(0, 1, "", "insert");
const [o1_t1, o2_t1] = o1.transformAgainst(o2);
const [o1_t2, o2_t2] = o2.transformAgainst(o1);
expect(apply(o1_t1, apply(o2_t1, s))).toBe("1bcde");
expect(apply(o2_t2, apply(o1_t2, s))).toBe("1bcde");
});

test("overlap insert before delete", () => {
const o1 = new StringOperation(1, 3, "12");
const o2 = new StringOperation(0, 1, "");
const o1 = new StringOperation(1, 3, "12", "insert");
const o2 = new StringOperation(0, 1, "", "delete");
const [o1_t1, o2_t1] = o1.transformAgainst(o2);
const [o1_t2, o2_t2] = o2.transformAgainst(o1);
expect(apply(o1_t1, apply(o2_t1, s))).toBe("12bcde");
expect(apply(o2_t2, apply(o1_t2, s))).toBe("12bcde");
});

test("overlap delete before insert", () => {
const o1 = new StringOperation(2, 3, "1");
const o2 = new StringOperation(0, 3, "");
const o1 = new StringOperation(2, 3, "1", "delete");
const o2 = new StringOperation(0, 3, "", "delete");
const [o1_t1, o2_t1] = o1.transformAgainst(o2);
const [o1_t2, o2_t2] = o2.transformAgainst(o1);
expect(apply(o1_t1, apply(o2_t1, s))).toBe("1de");
expect(apply(o2_t2, apply(o1_t2, s))).toBe("1de");
});

test("overlap less insert than delete", () => {
const o1 = new StringOperation(0, 1, "1");
const o2 = new StringOperation(0, 5, "");
const o1 = new StringOperation(0, 1, "1", "insert");
const o2 = new StringOperation(0, 5, "", "delete");
const [o1_t1, o2_t1] = o1.transformAgainst(o2);
const [o1_t2, o2_t2] = o2.transformAgainst(o1);
expect(apply(o1_t1, apply(o2_t1, s))).toBe("1");
expect(apply(o2_t2, apply(o1_t2, s))).toBe("1");
});

test("overlap more insert than delete", () => {
const o1 = new StringOperation(0, 5, "11111");
const o2 = new StringOperation(0, 1, "");
const o1 = new StringOperation(0, 5, "11111", "insert");
const o2 = new StringOperation(0, 1, "", "delete");
const [o1_t1, o2_t1] = o1.transformAgainst(o2);
const [o1_t2, o2_t2] = o2.transformAgainst(o1);
expect(apply(o1_t1, apply(o2_t1, s))).toBe("11111bcde");
Expand All @@ -101,26 +101,26 @@ describe("String transformation functions", () => {

describe("delete delete", () => {
test("non overlap", () => {
const o1 = new StringOperation(1, 2, "");
const o2 = new StringOperation(2, 3, "");
const o1 = new StringOperation(1, 2, "", "delete");
const o2 = new StringOperation(2, 3, "", "delete");
const [o1_t1, o2_t1] = o1.transformAgainst(o2);
const [o1_t2, o2_t2] = o2.transformAgainst(o1);
expect(apply(o1_t1, apply(o2_t1, s))).toBe("ade");
expect(apply(o2_t2, apply(o1_t2, s))).toBe("ade");
});

test("same operations", () => {
const o1 = new StringOperation(1, 2, "");
const o2 = new StringOperation(1, 2, "");
const o1 = new StringOperation(1, 2, "", "insert");
const o2 = new StringOperation(1, 2, "", "insert");
const [o1_t1, o2_t1] = o1.transformAgainst(o2);
const [o1_t2, o2_t2] = o2.transformAgainst(o1);
expect(apply(o1_t1, apply(o2_t1, s))).toBe("acde");
expect(apply(o2_t2, apply(o1_t2, s))).toBe("acde");
});

test("overlap", () => {
const o1 = new StringOperation(1, 3, "");
const o2 = new StringOperation(2, 3, "");
const o1 = new StringOperation(1, 3, "", "delete");
const o2 = new StringOperation(2, 3, "", "delete");
const [o1_t1, o2_t1] = o1.transformAgainst(o2);
const [o1_t2, o2_t2] = o2.transformAgainst(o1);
expect(apply(o1_t1, apply(o2_t1, s))).toBe("ade");
Expand Down
26 changes: 17 additions & 9 deletions backend/editor/OT/OTClient/operation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// operation is the atomic operation that is sent between clients and servers
export interface Operation {
path: number[];
opType: "insert" | "delete";

isNoOp: boolean;
atomicOp: AtomicOperation;
}
Expand All @@ -11,42 +9,50 @@ export interface Operation {
// TODO: in the future update object operation to strictly contain CMS operation data
export interface AtomicOperation {
type: string;
opType: "insert" | "delete";
transformAgainst: (op: AtomicOperation) => AtomicOperation[];
}

export class IntegerOperation implements AtomicOperation {
type = "integerOperation";
newValue: number;
constructor(newValue: number) {
opType: "insert" | "delete";
constructor(newValue: number, opType: "insert" | "delete") {
this.newValue = newValue;
this.opType = opType;
}

transformAgainst = (op: AtomicOperation): AtomicOperation[] => [this, op];
}

export class BooleanOperation implements AtomicOperation {
type = "booleanOperation";
newValue: boolean;
constructor(newValue: boolean) {
opType: "insert" | "delete";
constructor(newValue: boolean, opType: "insert" | "delete") {
Copy link
Member

Choose a reason for hiding this comment

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

There is a lot of unneeded duplication here, abstract this out into its own type:

type OperationType = "insert" | "delete";

export class BooleanOperation implements AtomicOperation {
  type = "booleanOperation";
  newValue: boolean;
  constructor(newValue: boolean) {
  opType: OperationType;
  constructor(newValue: boolean, opType: OperationType) {
       // ....
  }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

wait i thought that's what i'm supposed to do according to this jira ticket?

this.newValue = newValue;
this.opType = opType;
}
transformAgainst = (op: AtomicOperation): AtomicOperation[] => [this, op];
}

export class ObjectOperation implements AtomicOperation {
type = "objectOperation";
newValue: object;
constructor(newValue: object) {
opType: "insert" | "delete";
constructor(newValue: object, opType: "insert" | "delete") {
this.newValue = newValue;
this.opType = opType;
}
transformAgainst = (op: AtomicOperation): AtomicOperation[] => [this, op];
}

export class ArrayOperation implements AtomicOperation {
type = "arrayOperation";
newValue: object;
constructor(newValue: object) {
opType: "insert" | "delete";
constructor(newValue: object, opType: "insert" | "delete") {
this.newValue = newValue;
this.opType = opType;
}
transformAgainst = (op: AtomicOperation): AtomicOperation[] => [this, op];
}
Expand All @@ -56,11 +62,13 @@ export class StringOperation implements AtomicOperation {
start: number;
end: number;
newValue: string;
opType: "insert" | "delete";

constructor(start: number, end: number, newValue: string) {
constructor(start: number, end: number, newValue: string, opType: "insert" | "delete") {
this.start = start;
this.end = end;
this.newValue = newValue;
this.opType = opType;
}

transformAgainst = (op: AtomicOperation): AtomicOperation[] => {
Expand Down Expand Up @@ -169,11 +177,11 @@ const deleteDelete = (
export class NoOp implements AtomicOperation {
type = "noOp";
transformAgainst = (op: AtomicOperation): AtomicOperation[] => [this, op];
opType: "insert" | "delete" = "insert";
}

export const noOp: Operation = {
path: [],
opType: "insert",
isNoOp: true,
atomicOp: new NoOp(),
};
Expand Down
10 changes: 4 additions & 6 deletions backend/editor/OT/OTClient/transform.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,26 +8,24 @@ describe("Transform operation", () => {
test("two insert operations on same path result in a string transformation", () => {
const a: Operation = {
path: [0],
opType: "insert",
isNoOp: false,
atomicOp: new StringOperation(1, 2, "1"),
atomicOp: new StringOperation(1, 2, "1", "insert"),
};
const b: Operation = {
path: [0],
opType: "insert",
isNoOp: false,
atomicOp: new StringOperation(2, 3, "2"),
atomicOp: new StringOperation(2, 3, "2", "insert"),
};

// Apply transformation
let [transformedA, transformedB] = transform(a, b);

// Check other fields are constant
expect(transformedA.path).toBe(a.path);
expect(transformedA.opType).toBe(a.opType);
expect(transformedA.atomicOp.opType).toBe(a.atomicOp.opType);
expect(transformedA.isNoOp).toBe(a.isNoOp);
expect(transformedB.path).toBe(b.path);
expect(transformedB.opType).toBe(b.opType);
expect(transformedB.atomicOp.opType).toBe(b.atomicOp.opType);
expect(transformedB.isNoOp).toBe(b.isNoOp);

// Check that atomic operation was changed
Expand Down
6 changes: 3 additions & 3 deletions backend/editor/OT/OTClient/transform.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@ const transformPaths = (a: Operation, b: Operation): [number[], number[]] => {
const tp = transformationPoint(a.path, b.path);
if (!effectIndependent(a.path, b.path, tp)) {
switch (true) {
case a.opType === "insert" && b.opType === "insert":
case a.atomicOp.opType === "insert" && b.atomicOp.opType === "insert":
return transformInserts(a.path, b.path, tp);
case a.opType === "delete" && b.opType === "delete":
case a.atomicOp.opType === "delete" && b.atomicOp.opType === "delete":
return transformDeletes(a.path, b.path, tp);
case a.opType === "insert" && b.opType === "delete":
case a.atomicOp.opType === "insert" && b.atomicOp.opType === "delete":
return transformInsertDelete(a.path, b.path, tp);
default:
const result = transformInsertDelete(b.path, a.path, tp);
Expand Down
Loading