From 9d45b529a4e4b4f9dc8b63461202e50d2ae20541 Mon Sep 17 00:00:00 2001 From: Yourim Cha <yourim.cha@navercorp.com> Date: Tue, 26 Sep 2023 11:38:54 +0900 Subject: [PATCH 01/38] Implement undo/redo for object set, remove --- src/api/converter.ts | 6 +- src/document/crdt/element.ts | 16 ++ src/document/crdt/element_rht.ts | 12 +- src/document/crdt/object.ts | 14 +- src/document/json/object.ts | 8 +- src/document/operation/remove_operation.ts | 32 +++- src/document/operation/set_operation.ts | 31 ++- src/document/presence/presence.ts | 4 +- test/integration/counter_test.ts | 8 +- test/integration/document_test.ts | 10 +- test/integration/object_test.ts | 212 ++++++++++++++++++++- test/unit/document/crdt/root_test.ts | 23 +-- test/unit/document/document_test.ts | 11 ++ 13 files changed, 344 insertions(+), 43 deletions(-) diff --git a/src/api/converter.ts b/src/api/converter.ts index 689c81fd9..4f1916797 100644 --- a/src/api/converter.ts +++ b/src/api/converter.ts @@ -1198,7 +1198,11 @@ function fromObject(pbObject: PbJSONElement.JSONObject): CRDTObject { const rht = new ElementRHT(); for (const pbRHTNode of pbObject.getNodesList()) { // eslint-disable-next-line - rht.set(pbRHTNode.getKey(), fromElement(pbRHTNode.getElement()!)); + rht.set( + pbRHTNode.getKey(), + fromElement(pbRHTNode.getElement()!), + fromTimeTicket(pbObject.getCreatedAt())!, + ); } const obj = new CRDTObject(fromTimeTicket(pbObject.getCreatedAt())!, rht); diff --git a/src/document/crdt/element.ts b/src/document/crdt/element.ts index 2f9607ec6..989672cd7 100644 --- a/src/document/crdt/element.ts +++ b/src/document/crdt/element.ts @@ -25,9 +25,11 @@ export abstract class CRDTElement { private createdAt: TimeTicket; private movedAt?: TimeTicket; private removedAt?: TimeTicket; + private executedAt: TimeTicket; constructor(createdAt: TimeTicket) { this.createdAt = createdAt; + this.executedAt = createdAt; } /** @@ -58,6 +60,13 @@ export abstract class CRDTElement { return this.removedAt; } + /** + * `getExecutedAt` returns the execution time of this element. + */ + public getExecutedAt(): TimeTicket { + return this.executedAt; + } + /** * `setMovedAt` sets the move time of this element. */ @@ -77,6 +86,13 @@ export abstract class CRDTElement { this.removedAt = removedAt; } + /** + * `setExecutedAt` sets the execution time of this element. + */ + public setExecutedAt(executedAt: TimeTicket): void { + this.executedAt = executedAt; + } + /** * `remove` removes this element. */ diff --git a/src/document/crdt/element_rht.ts b/src/document/crdt/element_rht.ts index 79cff5395..e1e2aca28 100644 --- a/src/document/crdt/element_rht.ts +++ b/src/document/crdt/element_rht.ts @@ -89,7 +89,11 @@ export class ElementRHT { /** * `set` sets the value of the given key. */ - public set(key: string, value: CRDTElement): CRDTElement | undefined { + public set( + key: string, + value: CRDTElement, + executedAt: TimeTicket, + ): CRDTElement | undefined { let removed; const node = this.nodeMapByKey.get(key); if ( @@ -102,11 +106,9 @@ export class ElementRHT { const newNode = ElementRHTNode.of(key, value); this.nodeMapByCreatedAt.set(value.getCreatedAt().toIDString(), newNode); - if ( - node == null || - value.getCreatedAt().after(node.getValue().getCreatedAt()) - ) { + if (node == null || executedAt.after(node.getValue().getExecutedAt())) { this.nodeMapByKey.set(key, newNode); + value.setExecutedAt(executedAt); } return removed; } diff --git a/src/document/crdt/object.ts b/src/document/crdt/object.ts index 81b9ceb63..0294c51b2 100644 --- a/src/document/crdt/object.ts +++ b/src/document/crdt/object.ts @@ -59,8 +59,12 @@ export class CRDTObject extends CRDTContainer { /** * `set` sets the given element of the given key. */ - public set(key: string, value: CRDTElement): CRDTElement | undefined { - return this.memberNodes.set(key, value); + public set( + key: string, + value: CRDTElement, + executedAt: TimeTicket, + ): CRDTElement | undefined { + return this.memberNodes.set(key, value, executedAt); } /** @@ -155,7 +159,11 @@ export class CRDTObject extends CRDTContainer { public deepcopy(): CRDTObject { const clone = CRDTObject.create(this.getCreatedAt()); for (const node of this.memberNodes) { - clone.memberNodes.set(node.getStrKey(), node.getValue().deepcopy()); + clone.memberNodes.set( + node.getStrKey(), + node.getValue().deepcopy(), + this.getExecutedAt(), + ); } clone.remove(this.getRemovedAt()); return clone; diff --git a/src/document/json/object.ts b/src/document/json/object.ts index 4fc8ed2cb..744226c56 100644 --- a/src/document/json/object.ts +++ b/src/document/json/object.ts @@ -154,7 +154,7 @@ export class ObjectProxy { const ticket = context.issueTimeTicket(); const setAndRegister = function (elem: CRDTElement) { - const removed = target.set(key, elem); + const removed = target.set(key, elem, ticket); context.registerElement(elem, target); if (removed) { context.registerRemovedElement(removed); @@ -184,7 +184,7 @@ export class ObjectProxy { } else if (typeof value === 'object') { if (value instanceof Text) { const text = CRDTText.create(RGATreeSplit.create(), ticket); - target.set(key, text); + target.set(key, text, ticket); context.registerElement(text, target); context.push( SetOperation.create( @@ -201,7 +201,7 @@ export class ObjectProxy { value.getValue(), ticket, ); - target.set(key, counter); + target.set(key, counter, ticket); context.registerElement(counter, target); context.push( SetOperation.create( @@ -214,7 +214,7 @@ export class ObjectProxy { value.initialize(context, counter); } else if (value instanceof Tree) { const tree = CRDTTree.create(value.buildRoot(context), ticket); - target.set(key, tree); + target.set(key, tree, ticket); context.registerElement(tree, target); context.push( SetOperation.create( diff --git a/src/document/operation/remove_operation.ts b/src/document/operation/remove_operation.ts index 9255ccd8b..4718a4cd9 100644 --- a/src/document/operation/remove_operation.ts +++ b/src/document/operation/remove_operation.ts @@ -23,7 +23,9 @@ import { ExecutionResult, } from '@yorkie-js-sdk/src/document/operation/operation'; import { CRDTContainer } from '@yorkie-js-sdk/src/document/crdt/element'; +import { CRDTObject } from '@yorkie-js-sdk/src/document/crdt/object'; import { CRDTArray } from '@yorkie-js-sdk/src/document/crdt/array'; +import { SetOperation } from '@yorkie-js-sdk/src/document/operation/set_operation'; /** * `RemoveOperation` is an operation that removes an element from `CRDTContainer`. @@ -34,7 +36,7 @@ export class RemoveOperation extends Operation { constructor( parentCreatedAt: TimeTicket, createdAt: TimeTicket, - executedAt: TimeTicket, + executedAt?: TimeTicket, ) { super(parentCreatedAt, executedAt); this.createdAt = createdAt; @@ -46,7 +48,7 @@ export class RemoveOperation extends Operation { public static create( parentCreatedAt: TimeTicket, createdAt: TimeTicket, - executedAt: TimeTicket, + executedAt?: TimeTicket, ): RemoveOperation { return new RemoveOperation(parentCreatedAt, createdAt, executedAt); } @@ -64,6 +66,8 @@ export class RemoveOperation extends Operation { } const obj = parentObject as CRDTContainer; const key = obj.subPathOf(this.createdAt); + const reverseOp = this.getReverseOperation(obj); + const elem = obj.delete(this.createdAt, this.getExecutedAt()); root.registerRemovedElement(elem); @@ -84,7 +88,29 @@ export class RemoveOperation extends Operation { }, ]; - return { opInfos }; + return { opInfos, reverseOp }; + } + + /** + * `getReverseOperation` returns the reverse operation of this operation. + */ + public getReverseOperation( + parentObject: CRDTContainer, + ): Operation | undefined { + //TODO(Hyemmie): consider CRDTArray + if (parentObject instanceof CRDTObject) { + const key = parentObject.subPathOf(this.createdAt); + if (key !== undefined) { + const value = parentObject.get(key); + if (value !== undefined) { + return SetOperation.create( + key, + value.deepcopy(), + this.getParentCreatedAt(), + ); + } + } + } } /** diff --git a/src/document/operation/set_operation.ts b/src/document/operation/set_operation.ts index dae98d860..dcb679036 100644 --- a/src/document/operation/set_operation.ts +++ b/src/document/operation/set_operation.ts @@ -23,6 +23,7 @@ import { Operation, ExecutionResult, } from '@yorkie-js-sdk/src/document/operation/operation'; +import { RemoveOperation } from '@yorkie-js-sdk/src/document/operation/remove_operation'; /** * `SetOperation` represents an operation that stores the value corresponding to the @@ -36,7 +37,7 @@ export class SetOperation extends Operation { key: string, value: CRDTElement, parentCreatedAt: TimeTicket, - executedAt: TimeTicket, + executedAt?: TimeTicket, ) { super(parentCreatedAt, executedAt); this.key = key; @@ -50,7 +51,7 @@ export class SetOperation extends Operation { key: string, value: CRDTElement, parentCreatedAt: TimeTicket, - executedAt: TimeTicket, + executedAt?: TimeTicket, ): SetOperation { return new SetOperation(key, value, parentCreatedAt, executedAt); } @@ -67,9 +68,13 @@ export class SetOperation extends Operation { logger.fatal(`fail to execute, only object can execute set`); } const obj = parentObject as CRDTObject; + const previousValue = obj.get(this.key); + const reverseOp = this.getReverseOperation(previousValue); + const value = this.value.deepcopy(); - obj.set(this.key, value); + obj.set(this.key, value, this.getExecutedAt()); root.registerElement(value, obj); + return { opInfos: [ { @@ -78,9 +83,29 @@ export class SetOperation extends Operation { key: this.key, }, ], + reverseOp, }; } + /** + * `getReverseOperation` returns the reverse operation of this operation. + */ + public getReverseOperation(value: CRDTElement | undefined): Operation { + let reverseOp: SetOperation | RemoveOperation = RemoveOperation.create( + this.getParentCreatedAt(), + this.value.getCreatedAt(), + ); + + if (value !== undefined && !value.isRemoved()) { + reverseOp = SetOperation.create( + this.key, + value.deepcopy(), + this.getParentCreatedAt(), + ); + } + return reverseOp; + } + /** * `getEffectedCreatedAt` returns the creation time of the effected element. */ diff --git a/src/document/presence/presence.ts b/src/document/presence/presence.ts index e1c8a5d85..468de2b0a 100644 --- a/src/document/presence/presence.ts +++ b/src/document/presence/presence.ts @@ -30,8 +30,8 @@ export enum PresenceChangeType { * `PresenceChange` represents the change of presence. */ export type PresenceChange<P extends Indexable> = - | { type: PresenceChangeType.Put; presence: P; } - | { type: PresenceChangeType.Clear; }; + | { type: PresenceChangeType.Put; presence: P } + | { type: PresenceChangeType.Clear }; /** * `Presence` represents a proxy for the Presence to be manipulated from the outside. diff --git a/test/integration/counter_test.ts b/test/integration/counter_test.ts index 8e80070a1..fd946bcc7 100644 --- a/test/integration/counter_test.ts +++ b/test/integration/counter_test.ts @@ -148,15 +148,15 @@ describe('Counter', function () { }); assert.equal(doc.toSortedJSON(), `{"cnt":1,"longCnt":9223372036854775807}`); assert.equal( - JSON.stringify(doc.getUndoStackForTest()), - `[["1:00:2.INCREASE.-9223372036854775807","1:00:1.INCREASE.-1.5"]]`, + JSON.stringify(doc.getUndoStackForTest().at(-1)), + `["1:00:2.INCREASE.-9223372036854775807","1:00:1.INCREASE.-1.5"]`, ); doc.history.undo(); assert.equal(doc.toSortedJSON(), `{"cnt":0,"longCnt":0}`); assert.equal( - JSON.stringify(doc.getRedoStackForTest()), - `[["1:00:1.INCREASE.1.5","1:00:2.INCREASE.9223372036854775807"]]`, + JSON.stringify(doc.getRedoStackForTest().at(-1)), + `["1:00:1.INCREASE.1.5","1:00:2.INCREASE.9223372036854775807"]`, ); }); diff --git a/test/integration/document_test.ts b/test/integration/document_test.ts index 3e6bc63f2..c4a45f9f0 100644 --- a/test/integration/document_test.ts +++ b/test/integration/document_test.ts @@ -736,7 +736,7 @@ describe('Document', function () { }, 'init counter'); assert.equal(doc.toSortedJSON(), '{"counter":100}'); - assert.equal(doc.history.canUndo(), false); + assert.equal(doc.history.canUndo(), true); assert.equal(doc.history.canRedo(), false); // user increases the counter @@ -751,7 +751,7 @@ describe('Document', function () { // user undoes the latest operation doc.history.undo(); - assert.equal(doc.history.canUndo(), false); + assert.equal(doc.history.canUndo(), true); assert.equal(doc.history.canRedo(), true); // user redoes the latest undone operation @@ -770,7 +770,7 @@ describe('Document', function () { }, 'init counter'); assert.equal(doc.toSortedJSON(), '{"counter":100}'); - assert.equal(doc.history.canUndo(), false); + assert.equal(doc.history.canUndo(), true); assert.equal(doc.history.canRedo(), false); for (let i = 0; i < 5; i++) { @@ -826,7 +826,7 @@ describe('Document', function () { }, 'init counter'); assert.equal(doc.toSortedJSON(), '{"counter":100}'); - assert.equal(doc.history.canUndo(), false); + assert.equal(doc.history.canUndo(), true); assert.equal(doc.history.canRedo(), false); assert.throws( @@ -859,7 +859,7 @@ describe('Document', function () { }, 'init counter'); assert.equal(doc.toSortedJSON(), '{"counter":0}'); - assert.equal(doc.history.canUndo(), false); + assert.equal(doc.history.canUndo(), true); assert.equal(doc.history.canRedo(), false); for (let i = 0; i < 100; i++) { diff --git a/test/integration/object_test.ts b/test/integration/object_test.ts index 78a29c715..04d4c6a5f 100644 --- a/test/integration/object_test.ts +++ b/test/integration/object_test.ts @@ -1,7 +1,12 @@ import { assert } from 'chai'; -import { JSONObject } from '@yorkie-js-sdk/src/yorkie'; +import { JSONObject, Client } from '@yorkie-js-sdk/src/yorkie'; import { Document } from '@yorkie-js-sdk/src/document/document'; -import { withTwoClientsAndDocuments } from '@yorkie-js-sdk/test/integration/integration_helper'; +import { + withTwoClientsAndDocuments, + assertUndoRedo, + toDocKey, + testRPCAddr, +} from '@yorkie-js-sdk/test/integration/integration_helper'; import { YorkieError } from '@yorkie-js-sdk/src/util/error'; describe('Object', function () { @@ -80,24 +85,31 @@ describe('Object', function () { '{"k1":{"k1-1":"v1","k1-2":"v3"},"k2":["1","2","3","4",{"k2-5":"v4"}]}', doc.toSortedJSON(), ); + + // TODO(Hyemmie): test assertUndoRedo after implementing array's reverse operation }); it('should handle delete operations', function () { const doc = new Document<{ k1: { 'k1-1'?: string; 'k1-2': string; 'k1-3'?: string }; }>('test-doc'); + const states: Array<string> = []; assert.equal('{}', doc.toSortedJSON()); doc.update((root) => { root['k1'] = { 'k1-1': 'v1', 'k1-2': 'v2' }; }, 'set {"k1":{"k1-1":"v1","k1-2":"v2"}}'); assert.equal('{"k1":{"k1-1":"v1","k1-2":"v2"}}', doc.toSortedJSON()); + states.push(doc.toSortedJSON()); doc.update((root) => { delete root['k1']['k1-1']; root['k1']['k1-3'] = 'v4'; }, 'set {"k1":{"k1-2":"v2"}}'); assert.equal('{"k1":{"k1-2":"v2","k1-3":"v4"}}', doc.toSortedJSON()); + states.push(doc.toSortedJSON()); + + assertUndoRedo(doc, states); }); it('should support toJS and toJSON methods', function () { @@ -192,4 +204,200 @@ describe('Object', function () { assert.equal(d1.toSortedJSON(), d2.toSortedJSON()); }, this.test!.title); }); + + describe('Undo/Redo', function () { + it('Can undo/redo work properly for simple object', function () { + const doc = new Document<{ + a: number; + }>('test-doc'); + assert.equal(doc.toSortedJSON(), '{}'); + assert.equal(doc.history.canUndo(), false); + assert.equal(doc.history.canRedo(), false); + + doc.update((root) => { + root.a = 1; + }); + assert.equal(doc.toSortedJSON(), '{"a":1}'); + assert.equal(doc.history.canUndo(), true); + assert.equal(doc.history.canRedo(), false); + + doc.update((root) => { + root.a = 2; + }); + assert.equal(doc.toSortedJSON(), '{"a":2}'); + assert.equal(doc.history.canUndo(), true); + assert.equal(doc.history.canRedo(), false); + + doc.history.undo(); + assert.equal(doc.toSortedJSON(), '{"a":1}'); + assert.equal(doc.history.canUndo(), true); + assert.equal(doc.history.canRedo(), true); + + doc.history.redo(); + assert.equal(doc.toSortedJSON(), '{"a":2}'); + assert.equal(doc.history.canUndo(), true); + assert.equal(doc.history.canRedo(), false); + + doc.history.undo(); + assert.equal(doc.toSortedJSON(), '{"a":1}'); + assert.equal(doc.history.canUndo(), true); + assert.equal(doc.history.canRedo(), true); + + doc.history.undo(); + assert.equal(doc.toSortedJSON(), '{}'); + assert.equal(doc.history.canUndo(), false); + assert.equal(doc.history.canRedo(), true); + }); + + it('Can undo/redo work properly for nested object', function () { + const doc = new Document<{ + shape: { point: { x: number; y: number }; color: string }; + a: number; + }>('test-doc'); + const states: Array<string> = []; + states.push(doc.toSortedJSON()); + assert.equal(doc.toSortedJSON(), '{}'); + + doc.update((root) => { + root.shape = { point: { x: 0, y: 0 }, color: 'red' }; + root.a = 0; + }); + states.push(doc.toSortedJSON()); + assert.equal( + doc.toSortedJSON(), + '{"a":0,"shape":{"color":"red","point":{"x":0,"y":0}}}', + ); + + doc.update((root) => { + root.shape.point = { x: 1, y: 1 }; + root.shape.color = 'blue'; + }); + states.push(doc.toSortedJSON()); + assert.equal( + doc.toSortedJSON(), + '{"a":0,"shape":{"color":"blue","point":{"x":1,"y":1}}}', + ); + + doc.update((root) => { + root.a = 1; + root.a = 2; + }); + states.push(doc.toSortedJSON()); + assert.equal( + doc.toSortedJSON(), + '{"a":2,"shape":{"color":"blue","point":{"x":1,"y":1}}}', + ); + + assertUndoRedo(doc, states); + }); + + it('concurrent undo/redo of object - no sync before undo', async function () { + interface TestDoc { + color: string; + } + const docKey = toDocKey(`${this.test!.title}-${new Date().getTime()}`); + const doc1 = new Document<TestDoc>(docKey); + const doc2 = new Document<TestDoc>(docKey); + + const client1 = new Client(testRPCAddr); + const client2 = new Client(testRPCAddr); + await client1.activate(); + await client2.activate(); + + await client1.attach(doc1, { isRealtimeSync: false }); + doc1.update((root) => { + root.color = 'black'; + }, 'init doc'); + await client1.sync(); + assert.equal(doc1.toSortedJSON(), '{"color":"black"}'); + + await client2.attach(doc2, { isRealtimeSync: false }); + assert.equal(doc2.toSortedJSON(), '{"color":"black"}'); + + doc1.update((root) => { + root.color = 'red'; + }, 'set red'); + doc2.update((root) => { + root.color = 'green'; + }, 'set green'); + + assert.equal(doc1.toSortedJSON(), '{"color":"red"}'); + assert.equal(doc2.toSortedJSON(), '{"color":"green"}'); + + doc1.history.undo(); + assert.equal(doc1.toSortedJSON(), '{"color":"black"}'); + + await client1.sync(); + await client2.sync(); + await client1.sync(); + + // client 2's green set wins client 1's undo + assert.equal(doc1.toSortedJSON(), '{"color":"green"}'); + assert.equal(doc2.toSortedJSON(), '{"color":"green"}'); + + doc1.history.redo(); + + await client1.sync(); + await client2.sync(); + await client1.sync(); + + assert.equal(doc1.toSortedJSON(), '{"color":"red"}'); + assert.equal(doc2.toSortedJSON(), '{"color":"red"}'); + }); + + it('concurrent undo/redo of object - sync before undo', async function () { + interface TestDoc { + color: string; + } + const docKey = toDocKey(`${this.test!.title}-${new Date().getTime()}`); + const doc1 = new Document<TestDoc>(docKey); + const doc2 = new Document<TestDoc>(docKey); + + const client1 = new Client(testRPCAddr); + const client2 = new Client(testRPCAddr); + await client1.activate(); + await client2.activate(); + + await client1.attach(doc1, { isRealtimeSync: false }); + doc1.update((root) => { + root.color = 'black'; + }, 'init doc'); + await client1.sync(); + assert.equal(doc1.toSortedJSON(), '{"color":"black"}'); + + await client2.attach(doc2, { isRealtimeSync: false }); + assert.equal(doc2.toSortedJSON(), '{"color":"black"}'); + + doc1.update((root) => { + root.color = 'red'; + }, 'set red'); + doc2.update((root) => { + root.color = 'green'; + }, 'set green'); + await client1.sync(); + await client2.sync(); + await client1.sync(); + assert.equal(doc1.toSortedJSON(), '{"color":"green"}'); + assert.equal(doc2.toSortedJSON(), '{"color":"green"}'); + + doc1.history.undo(); + assert.equal(doc1.toSortedJSON(), '{"color":"black"}'); + + await client1.sync(); + await client2.sync(); + await client1.sync(); + + assert.equal(doc1.toSortedJSON(), '{"color":"black"}'); + assert.equal(doc2.toSortedJSON(), '{"color":"black"}'); + + doc1.history.redo(); + + await client1.sync(); + await client2.sync(); + await client1.sync(); + + assert.equal(doc1.toSortedJSON(), '{"color":"green"}'); + assert.equal(doc2.toSortedJSON(), '{"color":"green"}'); + }); + }); }); diff --git a/test/unit/document/crdt/root_test.ts b/test/unit/document/crdt/root_test.ts index 065136090..7e36816ac 100644 --- a/test/unit/document/crdt/root_test.ts +++ b/test/unit/document/crdt/root_test.ts @@ -24,8 +24,9 @@ describe('ROOT', function () { assert.equal(root.createPath(MaxTimeTicket), ''); // set '$.k1' - const k1 = Primitive.of('k1', cc.issueTimeTicket()); - root.getObject().set('k1', k1); + let ticket = cc.issueTimeTicket(); + const k1 = Primitive.of('k1', ticket); + root.getObject().set('k1', k1, ticket); root.registerElement(k1, root.getObject()); assert.equal(root.getElementMapSize(), 2); assert.equal(root.findByCreatedAt(k1.getCreatedAt()), k1); @@ -39,8 +40,9 @@ describe('ROOT', function () { assert.isUndefined(root.findByCreatedAt(k1.getCreatedAt())); // set '$.k2' - const k2 = CRDTObject.create(cc.issueTimeTicket()); - root.getObject().set('k2', k2); + ticket = cc.issueTimeTicket(); + const k2 = CRDTObject.create(ticket); + root.getObject().set('k2', k2, ticket); root.registerElement(k2, root.getObject()); assert.equal(root.getElementMapSize(), 2); assert.equal(root.findByCreatedAt(k2.getCreatedAt()), k2); @@ -49,8 +51,9 @@ describe('ROOT', function () { assert.equal(Object.keys(k2.toJS()).length, 0); // set '$.k2.1' - const k2Dot1 = CRDTArray.create(cc.issueTimeTicket()); - k2.set('1', k2Dot1); + ticket = cc.issueTimeTicket(); + const k2Dot1 = CRDTArray.create(ticket); + k2.set('1', k2Dot1, ticket); root.registerElement(k2Dot1, k2); assert.equal(root.getElementMapSize(), 3); assert.equal(root.findByCreatedAt(k2Dot1.getCreatedAt()), k2Dot1); @@ -110,11 +113,9 @@ describe('ROOT', function () { ); const obj = new CRDTObject(InitialTimeTicket, ElementRHT.create()); const change = ChangeContext.create(InitialChangeID, root, {}); - const crdtText = CRDTText.create( - RGATreeSplit.create(), - change.issueTimeTicket(), - ); - obj.set('k1', crdtText); + const executedAt = change.issueTimeTicket(); + const crdtText = CRDTText.create(RGATreeSplit.create(), executedAt); + obj.set('k1', crdtText, executedAt); change.registerElement(crdtText, obj); const text = new Text(change, crdtText); diff --git a/test/unit/document/document_test.ts b/test/unit/document/document_test.ts index 4478802a6..5a4492f5a 100644 --- a/test/unit/document/document_test.ts +++ b/test/unit/document/document_test.ts @@ -968,6 +968,17 @@ describe('Document', function () { { type: 'remove', path: '$', key: 'obj' }, ]); + doc.history.undo(); + await eventCollector.waitAndVerifyNthEvent(2, [ + { type: 'set', path: '$', key: 'obj' }, + { type: 'set', path: '$.obj', key: '$hello' }, + { type: 'remove', path: '$.obj', key: '$hello' }, + { type: 'set', path: '$.obj', key: 'a' }, + { type: 'remove', path: '$.obj', key: 'a' }, + { type: 'remove', path: '$', key: 'obj' }, + { type: 'remove', path: '$', key: '' }, + ]); + unsub(); }); From e2da9e06e8d2c22ed3b933d776533ab2779ade91 Mon Sep 17 00:00:00 2001 From: Yourim Cha <yourim.cha@navercorp.com> Date: Tue, 26 Sep 2023 14:27:05 +0900 Subject: [PATCH 02/38] Add object-devtool example for testing object undo/redo --- public/object.css | 144 +++++++++++ public/object.html | 282 +++++++++++++++++++++ src/document/change/change.ts | 1 + src/document/crdt/array.ts | 19 ++ src/document/crdt/counter.ts | 10 + src/document/crdt/element.ts | 1 + src/document/crdt/object.ts | 19 ++ src/document/crdt/primitive.ts | 10 + src/document/crdt/root.ts | 2 + src/document/crdt/text.ts | 10 + src/document/crdt/tree.ts | 10 + src/document/document.ts | 7 + src/document/json/object.ts | 4 + src/document/operation/remove_operation.ts | 2 +- src/document/operation/set_operation.ts | 5 +- 15 files changed, 524 insertions(+), 2 deletions(-) create mode 100644 public/object.css create mode 100644 public/object.html diff --git a/public/object.css b/public/object.css new file mode 100644 index 000000000..a412a0a8b --- /dev/null +++ b/public/object.css @@ -0,0 +1,144 @@ +.update-wrap { + display: flex; + flex-direction: column; + align-items: center; + margin: 1em 0; + padding: 1em; + border-radius: 1em; + border: 1px solid black; +} + +.update-code { + position: relative; + box-sizing: border-box; + padding: 40px 20px 20px 20px; + width: 100%; + background: #f1f2f3; + margin-top: 14px; +} + +.update-code pre { + margin: 0; + line-height: 1.4; +} + +.update-form { + width: 100%; +} +.update-form input { + margin-right: 10px; +} + +.update-code .btn { + position: absolute; + top: 10px; + left: 10px; +} + +.obj-dev-log { + display: flex; +} + +.obj-dev-log h2 { + margin: 0; +} + +.obj-dev-log > div { + width: 100%; + padding: 10px; +} + +.obj-dev-log pre { + box-sizing: border-box; + padding: 10px; + width: 100%; + background: #f1f2f3; +} + +.object-content { + margin-left: 24px; +} + +.object-key-val, +.variable-row { + position: relative; +} +.object-key-val:not(:last-of-type):before, +.variable-row:not(:last-of-type):before { + border: 1px dashed #ddd; + border-width: 0 0 0 1px; + content: ''; + position: absolute; + bottom: -12px; + left: -18px; + top: 12px; +} + +.object-key-val:after, +.variable-row:after { + border-bottom: 1px dashed #ddd; + border-left: 1px dashed #ddd; + border-radius: 0 0 0 4px; + border-right: 0 dashed #ddd; + border-top: 0 dashed #ddd; + content: ''; + height: 16px; + position: absolute; + top: 0; + width: 16px; + height: 32px; + top: -16px; + left: -18px; +} + +#document-holder { + margin-top: 24px; +} + +#document-holder > .object-key-val:after, +#document-holder > .variable-row:after { + content: none; +} + +.variable-row > span, +.object-key-val > span, +.variable-row label, +.object-key-val label { + border: 1px solid #ddd; + border-radius: 4px; + display: inline-block; + font-size: 14px; + font-weight: 500; + letter-spacing: 0 !important; + line-height: 1.72; + margin-bottom: 16px; + padding: 6px 8px; +} + +.timeticket { + border-radius: 4px; + background: #f1f2f3; + font-size: 12px; + font-weight: 400; + padding: 4px 6px; + margin-left: 4px; + letter-spacing: 1px; +} + +#document-holder input[type='checkbox']:checked ~ .object-content { + display: none; +} + +#document-holder input[type='checkbox'] { + display: none; +} + +#document-holder label { + cursor: pointer; +} + +.obj-dev-tool pre, +.obj-dev-tool pre code { + font-family: 'Courier New', Courier, monospace; + font-size: 14px; +} diff --git a/public/object.html b/public/object.html new file mode 100644 index 000000000..104875e66 --- /dev/null +++ b/public/object.html @@ -0,0 +1,282 @@ +<!DOCTYPE html> +<html lang="en"> + <head> + <meta charset="UTF-8" /> + <title>Object Devtool</title> + <link rel="stylesheet" href="style.css" /> + <link rel="stylesheet" href="object.css" /> + </head> + <body> + <div class="obj-dev-tool"> + <div id="network-status"></div> + <div id="online-clients"></div> + <div class="update-wrap"> + <form class="update-form"> + <label for="update-key">key: </label + ><input type="text" id="update-key" autocomplete="off" /> + <label for="update-value">value: </label + ><input type="text" id="update-value" autocomplete="off" /> + <button id="set-btn">set</button> + <button id="delete-btn">delete</button> + </form> + <div class="update-code"> + <pre style="white-space: pre-wrap" id="update-holder"></pre> + <div class="btn"> + <button id="update-btn">update!</button> + <button id="undo-btn">undo</button> + <button id="redo-btn">redo</button> + </div> + </div> + </div> + <div class="obj-dev-log"> + <div> + <h2>yorkie document</h2> + <div id="document-holder"></div> + </div> + <div class="stack-holder"> + <div> + <h2>operations</h2> + <pre style="white-space: pre-wrap" id="ops-holder"></pre> + </div> + <div> + <h2>undo stack</h2> + <pre style="white-space: pre-wrap" id="undo-holder"></pre> + </div> + <div> + <h2>redo stack</h2> + <pre style="white-space: pre-wrap" id="redo-holder"></pre> + </div> + </div> + </div> + </div> + <script src="./yorkie.js"></script> + <script src="./util.js"></script> + <script> + const statusHolder = document.getElementById('network-status'); + const onlineClientsHolder = document.getElementById('online-clients'); + const setBtn = document.getElementById('set-btn'); + const deleteBtn = document.getElementById('delete-btn'); + const updateBtn = document.getElementById('update-btn'); + const undoBtn = document.getElementById('undo-btn'); + const redoBtn = document.getElementById('redo-btn'); + const updateKey = document.getElementById('update-key'); + const updateValue = document.getElementById('update-value'); + const updateHolder = document.getElementById('update-holder'); + const undoHolder = document.getElementById('undo-holder'); + const redoHolder = document.getElementById('redo-holder'); + const opsHolder = document.getElementById('ops-holder'); + const docHolder = document.getElementById('document-holder'); + const updateCode = { + start: 'doc.update((root) => {', + content: ['\n // update key, value'], + end: '\n})', + data: [], + }; + function getEntireUpdateCode() { + // prettier-ignore + return `${updateCode.start}${updateCode.content.join('')}${updateCode.end}`; + } + function pushUpdateCode(str) { + updateCode.content.push(`\n ${str}`); + } + function pushUpdateData({ key, value, type }) { + updateCode.data.push({ key, value, type }); + } + updateHolder.innerHTML = getEntireUpdateCode(); + + function printUndoStack(doc) { + undoHolder.innerHTML = JSON.stringify( + doc.getUndoStackForTest(), + null, + 2, + ); + } + function printRedoStack(doc) { + redoHolder.innerHTML = JSON.stringify( + doc.getRedoStackForTest(), + null, + 2, + ); + } + function printOps(doc) { + opsHolder.innerHTML = JSON.stringify( + doc.getOpsForTest().map((op) => op.toTestString()), + null, + 2, + ); + } + + const displayDocument = (doc) => { + const docData = doc.getRoot().toJSForTest(); + docHolder.innerHTML = displayObject(docData); + }; + function displayObject({ key, value, id }) { + const valueHTML = Object.values(value) + .map((v) => { + return Object.prototype.toString.call(v.value) === '[object Object]' + ? displayObject(v) + : displayValue(v); + }) + .join(''); + if (!key) key = 'root'; + return ` + <div class="object-key-val"> + ${displayKey({ key, id })} + <div class="object-content"> + ${valueHTML} + </div> + </div> + `; + } + function displayKey({ key, id }) { + return ` + <input type="checkbox" id="${id}" /> + <label for="${id}">▾ ${key} + <span class="timeticket">${id}</span> + </label> + `; + } + function displayValue({ key, value, id }) { + return ` + <div class="variable-row"> + <span>${key} : ${JSON.stringify(value)} + <span class="timeticket">${id}</span> + </span> + </div> + `; + } + + function displayOnlineClients(presences, myClientID) { + const usernames = []; + for (const { clientID, presence } of presences) { + usernames.push( + clientID === myClientID ? `<b>${clientID}</b>` : clientID, + ); + } + onlineClientsHolder.innerHTML = JSON.stringify(usernames); + } + + async function main() { + try { + // 01. create client with RPCAddr(envoy) then activate it. + const client = new yorkie.Client('http://localhost:8080'); + client.subscribe(network.statusListener(statusHolder)); + await client.activate(); + + // 02. create a document then attach it into the client. + const doc = new yorkie.Document('object'); + doc.subscribe('presence', (event) => { + if (event.type === 'presence-changed') return; + displayOnlineClients(doc.getPresences(), client.getID()); + }); + window.doc = doc; + await client.attach(doc); + + // 03. update doc + setBtn.onclick = (e) => { + e.preventDefault(); + if (!updateKey.value || !updateValue.value) return; + let value = updateValue.value; + let jsonValue = `"${updateValue.value}"`; + if (value.startsWith('{')) { + value = JSON.parse(value); + jsonValue = JSON.stringify(value); + } + pushUpdateData({ type: 'add', key: updateKey.value, value }); + pushUpdateCode(`root.${updateKey.value} = ${jsonValue};`); + updateHolder.innerHTML = getEntireUpdateCode(); + updateKey.value = ''; + updateValue.value = ''; + }; + + deleteBtn.onclick = (e) => { + e.preventDefault(); + if (!updateKey.value) return; + pushUpdateData({ type: 'delete', key: updateKey.value }); + pushUpdateCode(`delete root.${updateKey.value};`); + updateHolder.innerHTML = getEntireUpdateCode(); + updateKey.value = ''; + updateValue.value = ''; + }; + + updateBtn.onclick = (e) => { + e.preventDefault(); + if (!updateCode.data.length) return; + doc.update((root) => { + for (const { type, key, value } of updateCode.data) { + const keys = key.split('.'); + let currentObj = root; + if (type === 'add') { + for (let i = 0; i < keys.length - 1; i++) { + const currentKey = keys[i]; + if (currentObj[currentKey]) { + currentObj = currentObj[currentKey]; + } else { + return; + } + } + const lastKey = keys[keys.length - 1]; + currentObj[lastKey] = value; + } else if (type === 'delete') { + for (let i = 0; i < keys.length - 1; i++) { + const currentKey = keys[i]; + if (typeof currentObj[currentKey] === 'object') { + currentObj = currentObj[currentKey]; + } else { + return; + } + } + const lastKey = keys[keys.length - 1]; + delete currentObj[lastKey]; + } + } + }); + updateCode.data = []; + updateCode.content = ['\n // update key, value']; + updateHolder.innerHTML = getEntireUpdateCode(); + printUndoStack(doc); + printRedoStack(doc); + }; + + undoBtn.onclick = (e) => { + e.preventDefault(); + doc.undo(); + printUndoStack(doc); + printRedoStack(doc); + }; + + redoBtn.onclick = (e) => { + e.preventDefault(); + doc.redo(); + printUndoStack(doc); + printRedoStack(doc); + }; + + // 04. subscribe to document changes + doc.subscribe((event) => { + displayDocument(doc); + printOps(doc); + }); + + // 05. set initial value + doc.update((root) => { + root.shape = { + point: { + x: 0, + y: 0, + }, + color: 'red', + }; + root.a = 1; + }); + displayDocument(doc); + printOps(doc); + } catch (e) { + console.error(e); + } + } + + main(); + </script> + </body> +</html> diff --git a/src/document/change/change.ts b/src/document/change/change.ts index 910604512..634fdd69b 100644 --- a/src/document/change/change.ts +++ b/src/document/change/change.ts @@ -144,6 +144,7 @@ export class Change<P extends Indexable> { const changeOpInfos: Array<OperationInfo> = []; const reverseOps: Array<HistoryOperation<P>> = []; for (const operation of this.operations) { + root.opsForTest.push(operation); const { opInfos, reverseOp } = operation.execute(root); changeOpInfos.push(...opInfos); if (reverseOp) { diff --git a/src/document/crdt/array.ts b/src/document/crdt/array.ts index 8170ba18c..cb83b6479 100644 --- a/src/document/crdt/array.ts +++ b/src/document/crdt/array.ts @@ -206,6 +206,25 @@ export class CRDTArray extends CRDTContainer { return JSON.parse(this.toJSON()); } + /** + * `toJSForTest` returns value with meta data for testing. + */ + public toJSForTest(): { id: string; value: any } { + const values = {} as any; + for (let i = 0; i < this.length; i++) { + const { id, value } = this.getByIndex(i)!.toJSForTest(); + values[i] = { + key: i, + id, + value, + }; + } + return { + id: this.getCreatedAt().toTestString(), + value: values, + }; + } + /** * `toSortedJSON` returns the sorted JSON encoding of this array. */ diff --git a/src/document/crdt/counter.ts b/src/document/crdt/counter.ts index dc94a1858..8732292f4 100644 --- a/src/document/crdt/counter.ts +++ b/src/document/crdt/counter.ts @@ -119,6 +119,16 @@ export class CRDTCounter extends CRDTElement { return this.toJSON(); } + /** + * `toJSForTest` returns value with meta data for testing. + */ + public toJSForTest(): { id: string; value: any } { + return { + id: this.getCreatedAt().toTestString(), + value: this.value, + }; + } + /** * `deepcopy` copies itself deeply. */ diff --git a/src/document/crdt/element.ts b/src/document/crdt/element.ts index 989672cd7..409e2dc9d 100644 --- a/src/document/crdt/element.ts +++ b/src/document/crdt/element.ts @@ -118,6 +118,7 @@ export abstract class CRDTElement { abstract toJSON(): string; abstract toSortedJSON(): string; + abstract toJSForTest(): { id: string; value: any }; abstract deepcopy(): CRDTElement; } diff --git a/src/document/crdt/object.ts b/src/document/crdt/object.ts index 0294c51b2..1fd824614 100644 --- a/src/document/crdt/object.ts +++ b/src/document/crdt/object.ts @@ -116,6 +116,25 @@ export class CRDTObject extends CRDTContainer { return JSON.parse(this.toJSON()); } + /** + * `toJSForTest` returns value with meta data for testing. + */ + public toJSForTest(): { id: string; value: any } { + const values = {} as any; + for (const [key, elem] of this) { + const { id, value } = elem.toJSForTest(); + values[key] = { + key, + id, + value, + }; + } + return { + id: this.getCreatedAt().toTestString(), + value: values, + }; + } + /** * `getKeys` returns array of keys in this object. */ diff --git a/src/document/crdt/primitive.ts b/src/document/crdt/primitive.ts index fbaf9ccda..4981cde81 100644 --- a/src/document/crdt/primitive.ts +++ b/src/document/crdt/primitive.ts @@ -117,6 +117,16 @@ export class Primitive extends CRDTElement { return this.toJSON(); } + /** + * `toJSForTest` returns value with meta data for testing. + */ + public toJSForTest(): { id: string; value: any } { + return { + id: this.getCreatedAt().toTestString(), + value: this.value, + }; + } + /** * `deepcopy` copies itself deeply. */ diff --git a/src/document/crdt/root.ts b/src/document/crdt/root.ts index ad100a8bf..1faac23ba 100644 --- a/src/document/crdt/root.ts +++ b/src/document/crdt/root.ts @@ -69,12 +69,14 @@ export class CRDTRoot { * the element that has removed nodes when executing garbage collection. */ private elementHasRemovedNodesSetByCreatedAt: Set<string>; + public opsForTest: Array<any>; constructor(rootObject: CRDTObject) { this.rootObject = rootObject; this.elementPairMapByCreatedAt = new Map(); this.removedElementSetByCreatedAt = new Set(); this.elementHasRemovedNodesSetByCreatedAt = new Set(); + this.opsForTest = []; this.elementPairMapByCreatedAt.set( this.rootObject.getCreatedAt().toIDString(), diff --git a/src/document/crdt/text.ts b/src/document/crdt/text.ts index b998aef1b..baec43942 100644 --- a/src/document/crdt/text.ts +++ b/src/document/crdt/text.ts @@ -350,6 +350,16 @@ export class CRDTText<A extends Indexable = Indexable> extends CRDTGCElement { return this.toJSON(); } + /** + * `toJSForTest` returns value with meta data for testing. + */ + public toJSForTest(): { id: string; value: any } { + return { + id: this.getCreatedAt().toTestString(), + value: JSON.parse(this.toJSON()), + }; + } + /** * `toString` returns the string representation of this text. */ diff --git a/src/document/crdt/tree.ts b/src/document/crdt/tree.ts index 4f1092782..9436c3871 100644 --- a/src/document/crdt/tree.ts +++ b/src/document/crdt/tree.ts @@ -954,6 +954,16 @@ export class CRDTTree extends CRDTGCElement { return JSON.stringify(this.getRootTreeNode()); } + /** + * `toJSForTest` returns value with meta data for testing. + */ + public toJSForTest(): { id: string; value: any } { + return { + id: this.getCreatedAt().toTestString(), + value: JSON.parse(this.toJSON()), + }; + } + /** * `getRootTreeNode` returns the converted value of this tree to TreeNode. */ diff --git a/src/document/document.ts b/src/document/document.ts index 601b24655..053ba4574 100644 --- a/src/document/document.ts +++ b/src/document/document.ts @@ -943,6 +943,13 @@ export class Document<T, P extends Indexable = Indexable> { return createJSON<T>(context, this.clone!.root.getObject()); } + /** + * `getOpsForTest` returns the operations of this document for testing. + */ + public getOpsForTest() { + return this.root.opsForTest; + } + /** * `garbageCollect` purges elements that were removed before the given time. * diff --git a/src/document/json/object.ts b/src/document/json/object.ts index 744226c56..8755b65ee 100644 --- a/src/document/json/object.ts +++ b/src/document/json/object.ts @@ -108,6 +108,10 @@ export class ObjectProxy { return (): object => { return target.toJS(); }; + } else if (keyOrMethod === 'toJSForTest') { + return (): object => { + return target.toJSForTest(); + }; } return toJSONElement(context, target.get(keyOrMethod)); diff --git a/src/document/operation/remove_operation.ts b/src/document/operation/remove_operation.ts index 4718a4cd9..ee4039114 100644 --- a/src/document/operation/remove_operation.ts +++ b/src/document/operation/remove_operation.ts @@ -124,7 +124,7 @@ export class RemoveOperation extends Operation { * `toTestString` returns a string containing the meta data. */ public toTestString(): string { - return `${this.getParentCreatedAt().toTestString()}.REMOVE`; + return `${this.getParentCreatedAt().toTestString()}.REMOVE.${this.createdAt.toTestString()}`; } /** diff --git a/src/document/operation/set_operation.ts b/src/document/operation/set_operation.ts index dcb679036..771e8da14 100644 --- a/src/document/operation/set_operation.ts +++ b/src/document/operation/set_operation.ts @@ -61,6 +61,7 @@ export class SetOperation extends Operation { */ public execute(root: CRDTRoot): ExecutionResult { const parentObject = root.findByCreatedAt(this.getParentCreatedAt()); + if (!parentObject) { logger.fatal(`fail to find ${this.getParentCreatedAt()}`); } @@ -117,7 +118,9 @@ export class SetOperation extends Operation { * `toTestString` returns a string containing the meta data. */ public toTestString(): string { - return `${this.getParentCreatedAt().toTestString()}.SET`; + return `${this.getParentCreatedAt().toTestString()}.SET.${ + this.key + }=${this.value.toSortedJSON()}`; } /** From c64da6e3e4680f5613ce170837f8d159dc962408 Mon Sep 17 00:00:00 2001 From: Yourim Cha <yourim.cha@navercorp.com> Date: Tue, 26 Sep 2023 14:44:40 +0900 Subject: [PATCH 03/38] Handle cases where the reverse operation cannot be executed --- src/document/change/change.ts | 6 ++- src/document/document.ts | 58 +++++++++++++++---------- src/document/operation/operation.ts | 5 ++- src/document/operation/set_operation.ts | 9 +++- test/integration/object_test.ts | 57 ++++++++++++++++++++++++ 5 files changed, 110 insertions(+), 25 deletions(-) diff --git a/src/document/change/change.ts b/src/document/change/change.ts index 634fdd69b..eb019173f 100644 --- a/src/document/change/change.ts +++ b/src/document/change/change.ts @@ -137,6 +137,7 @@ export class Change<P extends Indexable> { public execute( root: CRDTRoot, presences: Map<ActorID, P>, + origin?: string, ): { opInfos: Array<OperationInfo>; reverseOps: Array<HistoryOperation<P>>; @@ -145,7 +146,9 @@ export class Change<P extends Indexable> { const reverseOps: Array<HistoryOperation<P>> = []; for (const operation of this.operations) { root.opsForTest.push(operation); - const { opInfos, reverseOp } = operation.execute(root); + const executionResult = operation.execute(root, origin); + if (!executionResult) continue; + const { opInfos, reverseOp } = executionResult; changeOpInfos.push(...opInfos); if (reverseOp) { reverseOps.unshift(reverseOp); @@ -162,6 +165,7 @@ export class Change<P extends Indexable> { presences.delete(this.id.getActorID()!); } } + return { opInfos: changeOpInfos, reverseOps }; } diff --git a/src/document/document.ts b/src/document/document.ts index 053ba4574..029914344 100644 --- a/src/document/document.ts +++ b/src/document/document.ts @@ -541,7 +541,7 @@ export class Document<T, P extends Indexable = Indexable> { this.internalHistory.clearRedo(); this.changeID = change.getID(); - if (change.hasOperations()) { + if (change.hasOperations() && opInfos.length > 0) { this.publish({ type: DocEventType.LocalChange, value: { @@ -1086,12 +1086,12 @@ export class Document<T, P extends Indexable = Indexable> { } } - const executionResult = change.execute(this.root, this.presences); - if (change.hasOperations()) { + const { opInfos } = change.execute(this.root, this.presences); + if (change.hasOperations() && opInfos.length > 0) { changeInfo = { actor: actorID, message: change.getMessage() || '', - operations: executionResult.opInfos, + operations: opInfos, }; } @@ -1274,16 +1274,23 @@ export class Document<T, P extends Indexable = Indexable> { } const change = context.getChange(); - try { - change.execute(this.clone!.root, this.clone!.presences); - } catch (err) { - // drop clone because it is contaminated. - this.clone = undefined; - logger.error(err); - throw err; + const cloneExecutionResult = change.execute( + this.clone!.root, + this.clone!.presences, + 'UNDOREDO', + ); + if ( + !change.hasPresenceChange() && + cloneExecutionResult.opInfos.length === 0 + ) { + return; } - const { opInfos, reverseOps } = change.execute(this.root, this.presences); + const { opInfos, reverseOps } = change.execute( + this.root, + this.presences, + 'UNDOREDO', + ); const reversePresence = context.getReversePresence(); if (reversePresence) { reverseOps.push({ @@ -1299,7 +1306,7 @@ export class Document<T, P extends Indexable = Indexable> { this.changeID = change.getID(); const actorID = this.changeID.getActorID()!; - if (change.hasOperations()) { + if (change.hasOperations() && opInfos.length > 0) { this.publish({ type: DocEventType.LocalChange, value: { @@ -1357,16 +1364,23 @@ export class Document<T, P extends Indexable = Indexable> { } const change = context.getChange(); - try { - change.execute(this.clone!.root, this.clone!.presences); - } catch (err) { - // drop clone because it is contaminated. - this.clone = undefined; - logger.error(err); - throw err; + const cloneExecutionResult = change.execute( + this.clone!.root, + this.clone!.presences, + 'UNDOREDO', + ); + if ( + !change.hasPresenceChange() && + cloneExecutionResult.opInfos.length === 0 + ) { + return; } - const { opInfos, reverseOps } = change.execute(this.root, this.presences); + const { opInfos, reverseOps } = change.execute( + this.root, + this.presences, + 'UNDOREDO', + ); const reversePresence = context.getReversePresence(); if (reversePresence) { reverseOps.push({ @@ -1382,7 +1396,7 @@ export class Document<T, P extends Indexable = Indexable> { this.changeID = change.getID(); const actorID = this.changeID.getActorID()!; - if (change.hasOperations()) { + if (change.hasOperations() && opInfos.length > 0) { this.publish({ type: DocEventType.LocalChange, value: { diff --git a/src/document/operation/operation.ts b/src/document/operation/operation.ts index 525b7d241..9990bca7b 100644 --- a/src/document/operation/operation.ts +++ b/src/document/operation/operation.ts @@ -224,5 +224,8 @@ export abstract class Operation { /** * `execute` executes this operation on the given `CRDTRoot`. */ - public abstract execute(root: CRDTRoot): ExecutionResult; + public abstract execute( + root: CRDTRoot, + origin?: string, + ): ExecutionResult | undefined; } diff --git a/src/document/operation/set_operation.ts b/src/document/operation/set_operation.ts index 771e8da14..99226f719 100644 --- a/src/document/operation/set_operation.ts +++ b/src/document/operation/set_operation.ts @@ -59,9 +59,16 @@ export class SetOperation extends Operation { /** * `execute` executes this operation on the given `CRDTRoot`. */ - public execute(root: CRDTRoot): ExecutionResult { + public execute(root: CRDTRoot, source?: string): ExecutionResult | undefined { const parentObject = root.findByCreatedAt(this.getParentCreatedAt()); + if ( + source === 'UNDOREDO' && + (!parentObject || parentObject.getRemovedAt()) + ) { + return; + } + if (!parentObject) { logger.fatal(`fail to find ${this.getParentCreatedAt()}`); } diff --git a/test/integration/object_test.ts b/test/integration/object_test.ts index 04d4c6a5f..adcf16f37 100644 --- a/test/integration/object_test.ts +++ b/test/integration/object_test.ts @@ -291,6 +291,63 @@ describe('Object', function () { assertUndoRedo(doc, states); }); + it('Can handle concurrent undo/redo', async function () { + // Test scenario: + // c1: set shape.color to 'red' + // c2: delete shape + // c1: undo(no changes as the shape was deleted) + interface TestDoc { + shape?: { color: string }; + } + const docKey = toDocKey(`${this.test!.title}-${new Date().getTime()}`); + const doc1 = new Document<TestDoc>(docKey); + const doc2 = new Document<TestDoc>(docKey); + + const client1 = new Client(testRPCAddr); + const client2 = new Client(testRPCAddr); + await client1.activate(); + await client2.activate(); + + await client1.attach(doc1, { isRealtimeSync: false }); + doc1.update((root) => { + root.shape = { color: 'black' }; + }, 'init doc'); + await client1.sync(); + assert.equal(doc1.toSortedJSON(), '{"shape":{"color":"black"}}'); + + await client2.attach(doc2, { isRealtimeSync: false }); + assert.equal(doc2.toSortedJSON(), '{"shape":{"color":"black"}}'); + + doc1.update((root) => { + root.shape!.color = 'red'; + }, 'set red'); + await client1.sync(); + await client2.sync(); + doc2.update((root) => { + delete root.shape; + }, 'delete shape'); + await client2.sync(); + await client1.sync(); + assert.equal(doc1.toSortedJSON(), '{}'); + assert.equal(doc2.toSortedJSON(), '{}'); + + // there is no change because c2 deletes the shape + doc1.history.undo(); + assert.equal(doc1.toSortedJSON(), '{}'); + assert.equal(doc1.getRedoStackForTest().length, 0); + assert.equal(doc1.history.canRedo(), false); + await client1.sync(); + await client2.sync(); + await client1.sync(); + + doc2.history.undo(); + assert.equal(doc2.toSortedJSON(), '{"shape":{"color":"red"}}'); + assert.equal(doc2.toSortedJSON(), '{"shape":{"color":"red"}}'); + await client1.sync(); + await client2.sync(); + await client1.sync(); + }); + it('concurrent undo/redo of object - no sync before undo', async function () { interface TestDoc { color: string; From eb1bd0e2b6c1ab87fb13e4cfb68c7a6128a2110d Mon Sep 17 00:00:00 2001 From: Yourim Cha <yourim.cha@navercorp.com> Date: Tue, 17 Oct 2023 01:12:31 +0900 Subject: [PATCH 04/38] Add whiteboard example to test object undo --- public/object.css | 144 ---------- public/object.html | 282 -------------------- public/whiteboard.css | 239 +++++++++++++++++ public/whiteboard.html | 339 ++++++++++++++++++++++++ src/document/change/change.ts | 4 +- src/document/document.ts | 6 +- src/document/history.ts | 16 +- src/document/operation/set_operation.ts | 2 + test/helper/helper.ts | 12 +- test/integration/counter_test.ts | 18 +- test/integration/presence_test.ts | 20 +- 11 files changed, 621 insertions(+), 461 deletions(-) delete mode 100644 public/object.css delete mode 100644 public/object.html create mode 100644 public/whiteboard.css create mode 100644 public/whiteboard.html diff --git a/public/object.css b/public/object.css deleted file mode 100644 index a412a0a8b..000000000 --- a/public/object.css +++ /dev/null @@ -1,144 +0,0 @@ -.update-wrap { - display: flex; - flex-direction: column; - align-items: center; - margin: 1em 0; - padding: 1em; - border-radius: 1em; - border: 1px solid black; -} - -.update-code { - position: relative; - box-sizing: border-box; - padding: 40px 20px 20px 20px; - width: 100%; - background: #f1f2f3; - margin-top: 14px; -} - -.update-code pre { - margin: 0; - line-height: 1.4; -} - -.update-form { - width: 100%; -} -.update-form input { - margin-right: 10px; -} - -.update-code .btn { - position: absolute; - top: 10px; - left: 10px; -} - -.obj-dev-log { - display: flex; -} - -.obj-dev-log h2 { - margin: 0; -} - -.obj-dev-log > div { - width: 100%; - padding: 10px; -} - -.obj-dev-log pre { - box-sizing: border-box; - padding: 10px; - width: 100%; - background: #f1f2f3; -} - -.object-content { - margin-left: 24px; -} - -.object-key-val, -.variable-row { - position: relative; -} -.object-key-val:not(:last-of-type):before, -.variable-row:not(:last-of-type):before { - border: 1px dashed #ddd; - border-width: 0 0 0 1px; - content: ''; - position: absolute; - bottom: -12px; - left: -18px; - top: 12px; -} - -.object-key-val:after, -.variable-row:after { - border-bottom: 1px dashed #ddd; - border-left: 1px dashed #ddd; - border-radius: 0 0 0 4px; - border-right: 0 dashed #ddd; - border-top: 0 dashed #ddd; - content: ''; - height: 16px; - position: absolute; - top: 0; - width: 16px; - height: 32px; - top: -16px; - left: -18px; -} - -#document-holder { - margin-top: 24px; -} - -#document-holder > .object-key-val:after, -#document-holder > .variable-row:after { - content: none; -} - -.variable-row > span, -.object-key-val > span, -.variable-row label, -.object-key-val label { - border: 1px solid #ddd; - border-radius: 4px; - display: inline-block; - font-size: 14px; - font-weight: 500; - letter-spacing: 0 !important; - line-height: 1.72; - margin-bottom: 16px; - padding: 6px 8px; -} - -.timeticket { - border-radius: 4px; - background: #f1f2f3; - font-size: 12px; - font-weight: 400; - padding: 4px 6px; - margin-left: 4px; - letter-spacing: 1px; -} - -#document-holder input[type='checkbox']:checked ~ .object-content { - display: none; -} - -#document-holder input[type='checkbox'] { - display: none; -} - -#document-holder label { - cursor: pointer; -} - -.obj-dev-tool pre, -.obj-dev-tool pre code { - font-family: 'Courier New', Courier, monospace; - font-size: 14px; -} diff --git a/public/object.html b/public/object.html deleted file mode 100644 index 104875e66..000000000 --- a/public/object.html +++ /dev/null @@ -1,282 +0,0 @@ -<!DOCTYPE html> -<html lang="en"> - <head> - <meta charset="UTF-8" /> - <title>Object Devtool</title> - <link rel="stylesheet" href="style.css" /> - <link rel="stylesheet" href="object.css" /> - </head> - <body> - <div class="obj-dev-tool"> - <div id="network-status"></div> - <div id="online-clients"></div> - <div class="update-wrap"> - <form class="update-form"> - <label for="update-key">key: </label - ><input type="text" id="update-key" autocomplete="off" /> - <label for="update-value">value: </label - ><input type="text" id="update-value" autocomplete="off" /> - <button id="set-btn">set</button> - <button id="delete-btn">delete</button> - </form> - <div class="update-code"> - <pre style="white-space: pre-wrap" id="update-holder"></pre> - <div class="btn"> - <button id="update-btn">update!</button> - <button id="undo-btn">undo</button> - <button id="redo-btn">redo</button> - </div> - </div> - </div> - <div class="obj-dev-log"> - <div> - <h2>yorkie document</h2> - <div id="document-holder"></div> - </div> - <div class="stack-holder"> - <div> - <h2>operations</h2> - <pre style="white-space: pre-wrap" id="ops-holder"></pre> - </div> - <div> - <h2>undo stack</h2> - <pre style="white-space: pre-wrap" id="undo-holder"></pre> - </div> - <div> - <h2>redo stack</h2> - <pre style="white-space: pre-wrap" id="redo-holder"></pre> - </div> - </div> - </div> - </div> - <script src="./yorkie.js"></script> - <script src="./util.js"></script> - <script> - const statusHolder = document.getElementById('network-status'); - const onlineClientsHolder = document.getElementById('online-clients'); - const setBtn = document.getElementById('set-btn'); - const deleteBtn = document.getElementById('delete-btn'); - const updateBtn = document.getElementById('update-btn'); - const undoBtn = document.getElementById('undo-btn'); - const redoBtn = document.getElementById('redo-btn'); - const updateKey = document.getElementById('update-key'); - const updateValue = document.getElementById('update-value'); - const updateHolder = document.getElementById('update-holder'); - const undoHolder = document.getElementById('undo-holder'); - const redoHolder = document.getElementById('redo-holder'); - const opsHolder = document.getElementById('ops-holder'); - const docHolder = document.getElementById('document-holder'); - const updateCode = { - start: 'doc.update((root) => {', - content: ['\n // update key, value'], - end: '\n})', - data: [], - }; - function getEntireUpdateCode() { - // prettier-ignore - return `${updateCode.start}${updateCode.content.join('')}${updateCode.end}`; - } - function pushUpdateCode(str) { - updateCode.content.push(`\n ${str}`); - } - function pushUpdateData({ key, value, type }) { - updateCode.data.push({ key, value, type }); - } - updateHolder.innerHTML = getEntireUpdateCode(); - - function printUndoStack(doc) { - undoHolder.innerHTML = JSON.stringify( - doc.getUndoStackForTest(), - null, - 2, - ); - } - function printRedoStack(doc) { - redoHolder.innerHTML = JSON.stringify( - doc.getRedoStackForTest(), - null, - 2, - ); - } - function printOps(doc) { - opsHolder.innerHTML = JSON.stringify( - doc.getOpsForTest().map((op) => op.toTestString()), - null, - 2, - ); - } - - const displayDocument = (doc) => { - const docData = doc.getRoot().toJSForTest(); - docHolder.innerHTML = displayObject(docData); - }; - function displayObject({ key, value, id }) { - const valueHTML = Object.values(value) - .map((v) => { - return Object.prototype.toString.call(v.value) === '[object Object]' - ? displayObject(v) - : displayValue(v); - }) - .join(''); - if (!key) key = 'root'; - return ` - <div class="object-key-val"> - ${displayKey({ key, id })} - <div class="object-content"> - ${valueHTML} - </div> - </div> - `; - } - function displayKey({ key, id }) { - return ` - <input type="checkbox" id="${id}" /> - <label for="${id}">▾ ${key} - <span class="timeticket">${id}</span> - </label> - `; - } - function displayValue({ key, value, id }) { - return ` - <div class="variable-row"> - <span>${key} : ${JSON.stringify(value)} - <span class="timeticket">${id}</span> - </span> - </div> - `; - } - - function displayOnlineClients(presences, myClientID) { - const usernames = []; - for (const { clientID, presence } of presences) { - usernames.push( - clientID === myClientID ? `<b>${clientID}</b>` : clientID, - ); - } - onlineClientsHolder.innerHTML = JSON.stringify(usernames); - } - - async function main() { - try { - // 01. create client with RPCAddr(envoy) then activate it. - const client = new yorkie.Client('http://localhost:8080'); - client.subscribe(network.statusListener(statusHolder)); - await client.activate(); - - // 02. create a document then attach it into the client. - const doc = new yorkie.Document('object'); - doc.subscribe('presence', (event) => { - if (event.type === 'presence-changed') return; - displayOnlineClients(doc.getPresences(), client.getID()); - }); - window.doc = doc; - await client.attach(doc); - - // 03. update doc - setBtn.onclick = (e) => { - e.preventDefault(); - if (!updateKey.value || !updateValue.value) return; - let value = updateValue.value; - let jsonValue = `"${updateValue.value}"`; - if (value.startsWith('{')) { - value = JSON.parse(value); - jsonValue = JSON.stringify(value); - } - pushUpdateData({ type: 'add', key: updateKey.value, value }); - pushUpdateCode(`root.${updateKey.value} = ${jsonValue};`); - updateHolder.innerHTML = getEntireUpdateCode(); - updateKey.value = ''; - updateValue.value = ''; - }; - - deleteBtn.onclick = (e) => { - e.preventDefault(); - if (!updateKey.value) return; - pushUpdateData({ type: 'delete', key: updateKey.value }); - pushUpdateCode(`delete root.${updateKey.value};`); - updateHolder.innerHTML = getEntireUpdateCode(); - updateKey.value = ''; - updateValue.value = ''; - }; - - updateBtn.onclick = (e) => { - e.preventDefault(); - if (!updateCode.data.length) return; - doc.update((root) => { - for (const { type, key, value } of updateCode.data) { - const keys = key.split('.'); - let currentObj = root; - if (type === 'add') { - for (let i = 0; i < keys.length - 1; i++) { - const currentKey = keys[i]; - if (currentObj[currentKey]) { - currentObj = currentObj[currentKey]; - } else { - return; - } - } - const lastKey = keys[keys.length - 1]; - currentObj[lastKey] = value; - } else if (type === 'delete') { - for (let i = 0; i < keys.length - 1; i++) { - const currentKey = keys[i]; - if (typeof currentObj[currentKey] === 'object') { - currentObj = currentObj[currentKey]; - } else { - return; - } - } - const lastKey = keys[keys.length - 1]; - delete currentObj[lastKey]; - } - } - }); - updateCode.data = []; - updateCode.content = ['\n // update key, value']; - updateHolder.innerHTML = getEntireUpdateCode(); - printUndoStack(doc); - printRedoStack(doc); - }; - - undoBtn.onclick = (e) => { - e.preventDefault(); - doc.undo(); - printUndoStack(doc); - printRedoStack(doc); - }; - - redoBtn.onclick = (e) => { - e.preventDefault(); - doc.redo(); - printUndoStack(doc); - printRedoStack(doc); - }; - - // 04. subscribe to document changes - doc.subscribe((event) => { - displayDocument(doc); - printOps(doc); - }); - - // 05. set initial value - doc.update((root) => { - root.shape = { - point: { - x: 0, - y: 0, - }, - color: 'red', - }; - root.a = 1; - }); - displayDocument(doc); - printOps(doc); - } catch (e) { - console.error(e); - } - } - - main(); - </script> - </body> -</html> diff --git a/public/whiteboard.css b/public/whiteboard.css new file mode 100644 index 000000000..242f115c6 --- /dev/null +++ b/public/whiteboard.css @@ -0,0 +1,239 @@ +.obj-dev-log { + display: flex; +} + +.obj-dev-log h2 { + margin: 0; +} + +.obj-dev-log > div { + width: 100%; + padding: 10px; +} + +.object-content { + margin-left: 24px; +} + +.object-key-val, +.variable-row { + position: relative; +} + +.object-key-val:not(:last-of-type):before, +.variable-row:not(:last-of-type):before { + border: 1px dashed #ddd; + border-width: 0 0 0 1px; + content: ''; + position: absolute; + bottom: -12px; + left: -18px; + top: 12px; +} + +.object-key-val:after, +.variable-row:after { + border-bottom: 1px dashed #ddd; + border-left: 1px dashed #ddd; + border-radius: 0 0 0 4px; + border-right: 0 dashed #ddd; + border-top: 0 dashed #ddd; + content: ''; + height: 16px; + position: absolute; + top: 0; + width: 16px; + height: 32px; + top: -16px; + left: -18px; +} + +#document-holder { + margin-top: 24px; +} + +#document-holder > .object-key-val:after, +#document-holder > .variable-row:after { + content: none; +} + +.variable-row > span, +.object-key-val > span, +.variable-row label, +.object-key-val label { + border: 1px solid #ddd; + border-radius: 4px; + display: inline-block; + font-size: 14px; + font-weight: 500; + letter-spacing: 0 !important; + line-height: 1.72; + margin-bottom: 16px; + padding: 6px 8px; +} + +.object-key-val label:before { + content: '▾'; + margin-right: 4px; +} + +#document-holder input[type='checkbox']:checked + label:before { + content: '▸'; +} + +.timeticket { + border-radius: 4px; + background: #f1f2f3; + font-size: 12px; + font-weight: 400; + padding: 2px 6px; + margin-left: 4px; + letter-spacing: 1px; +} + +#document-holder input[type='checkbox']:checked ~ .object-content { + display: none; +} + +#document-holder input[type='checkbox'] { + display: none; +} + +#document-holder label { + cursor: pointer; +} + +.canvas { + position: relative; + width: 100%; + height: 350px; +} +.canvas .toolbar { + position: absolute; + bottom: 4px; + left: 50%; + transform: translateX(-50%); + z-index: 2; +} +.toolbar button { + margin: 2px; + padding: 4px 6px; + color: #666; +} +.canvas .shapes { + position: absolute; + width: 100%; + height: 100%; + background: #eee; + overflow: hidden; +} +.canvas .shape { + position: absolute; + width: 50px; + height: 50px; + border-style: solid; + border-width: 2px; +} +.selection-tools { + display: none; + position: absolute; + z-index: 1; + top: 4px; + right: 4px; + background: #fff; + padding: 6px; + border-radius: 4px; + justify-content: center; + gap: 4px; +} +.selection-tools .color-picker { + display: flex; + flex-wrap: wrap; + justify-content: center; + width: 120px; +} +.selection-tools .color { + width: 20px; + height: 20px; + border-radius: 50%; + border: none; + margin: 4px; + border: 1px solid #ddd; +} +.selection-tools .color:nth-child(1) { + background: orangered; +} +.selection-tools .color:nth-child(2) { + background: gold; +} +.selection-tools .color:nth-child(3) { + background: limegreen; +} +.selection-tools .color:nth-child(4) { + background: dodgerblue; +} +.selection-tools .color:nth-child(5) { + background: darkviolet; +} +.selection-tools .color:nth-child(6) { + background: darkorange; +} +.selection-tools .color:nth-child(7) { + background: dimgray; +} +.selection-tools .color:nth-child(8) { + background: white; +} + +.ops-holder { + margin: 10px 0 20px; + font-size: 14px; + font-weight: 400; +} +.ops-holder .change { + display: flex; + margin-bottom: 3px; + border-top: 1px solid #ddd; +} +.ops-holder label { + position: relative; + overflow: hidden; + padding-left: 24px; + cursor: pointer; + line-height: 1.6; +} +.ops-holder input[type='checkbox']:checked + label { + height: 22px; +} +.ops-holder input[type='checkbox'] { + display: none; +} +.ops-holder .count { + position: absolute; + left: 0px; + display: flex; + justify-content: center; + width: 20px; + height: 20px; + font-size: 13px; +} +.ops-holder .op { + display: block; +} +.ops-holder .op:first-child { + display: inline-block; +} +.ops-holder .op .type { + padding: 0 4px; + border-radius: 4px; + background: #e6e6fa; +} +.ops-holder .op .type.set { + background: #cff7cf; +} +.ops-holder .op .type.remove { + background: #f9c0c8; +} +.ops-holder .op .type.add { + background: #add8e6; +} diff --git a/public/whiteboard.html b/public/whiteboard.html new file mode 100644 index 000000000..179c540a5 --- /dev/null +++ b/public/whiteboard.html @@ -0,0 +1,339 @@ +<!DOCTYPE html> +<html lang="en"> + <head> + <meta charset="UTF-8" /> + <title>Whiteboard Example</title> + <link rel="stylesheet" href="style.css" /> + <link rel="stylesheet" href="whiteboard.css" /> + </head> + <body> + <div class="canvas"> + <div class="toolbar"> + <button class="rectangle">◼️ (new!)</button> + <button class="undo">⬅ undo</button> + <button class="redo">➡️ redo</button> + </div> + <div class="shapes"></div> + <div class="selection-tools"> + <div class="color-picker"> + <button class="color" data-color="orangered"></button> + <button class="color" data-color="gold"></button> + <button class="color" data-color="limegreen"></button> + <button class="color" data-color="dodgerblue"></button> + <button class="color" data-color="darkviolet"></button> + <button class="color" data-color="darkorange"></button> + <button class="color" data-color="dimgray"></button> + <button class="color" data-color="white"></button> + </div> + </div> + </div> + <div class="obj-dev-tool"> + <div id="network-status"></div> + <div id="online-clients"></div> + <div class="obj-dev-log"> + <div> + <h2>yorkie document</h2> + <div id="document-holder"></div> + </div> + <div class="stack-holder"> + <div> + <h2>operations</h2> + <div id="ops-holder" class="ops-holder"></div> + </div> + </div> + <div class="stack-holder"> + <div> + <h2>undo stack</h2> + <div id="undo-holder" class="ops-holder"></div> + </div> + <div> + <h2>redo stack</h2> + <div id="redo-holder" class="ops-holder"></div> + </div> + </div> + </div> + </div> + <script src="./yorkie.js"></script> + <script src="./util.js"></script> + <script type="module"> + const statusHolder = document.getElementById('network-status'); + const onlineClientsHolder = document.getElementById('online-clients'); + const undoHolder = document.getElementById('undo-holder'); + const redoHolder = document.getElementById('redo-holder'); + const opsHolder = document.getElementById('ops-holder'); + const docHolder = document.getElementById('document-holder'); + + function renderOpsHolder(changes, idPrefix) { + return changes + .map((ops, i) => { + const opsStr = ops + .map((op) => { + if (op.type === 'presence') { + return `<span class="op"><span class="type presence">presence</span>${JSON.stringify( + op.value, + )}</span>`; + } + let id = op.getExecutedAt()?.toTestString(); + if (id) { + id = `<span class="timeticket">${id}</span>`; + } + const opType = op.toTestString().split('.')[1]; + return `<span class="op"> + <span class="type ${opType.toLowerCase()}">${opType}</span> + ${id ? id : ''} + ${op.toTestString()} + </span>`; + }) + .join('\n'); + return ` + <div class="change"> + <input type="checkbox" id="${idPrefix}-${i}" /> + <label for="${idPrefix}-${i}"> + <span class="count">${ops.length}</span> + <span class="ops">${opsStr}</span> + </label> + </div> + `; + }) + .join(''); + } + + function printOps(doc) { + opsHolder.innerHTML = renderOpsHolder(doc.getOpsForTest(), 'op'); + undoHolder.innerHTML = renderOpsHolder( + doc.getUndoStackForTest(), + 'undo', + ); + redoHolder.innerHTML = renderOpsHolder( + doc.getRedoStackForTest(), + 'redo', + ); + } + + const displayDocument = (doc) => { + const docData = doc.getRoot().toJSForTest(); + docHolder.innerHTML = displayObject(docData); + }; + function displayObject({ key, value, id }) { + const valueHTML = Object.values(value) + .map((v) => { + return Object.prototype.toString.call(v.value) === '[object Object]' + ? displayObject(v) + : displayValue(v); + }) + .join(''); + if (!key) key = 'root'; + return ` + <div class="object-key-val"> + ${displayKey({ key, id })} + <div class="object-content"> + ${valueHTML} + </div> + </div> + `; + } + function displayKey({ key, id }) { + return ` + <input type="checkbox" id="${id}" /> + <label for="${id}">${key} + <span class="timeticket">${id}</span> + </label> + `; + } + function displayValue({ key, value, id }) { + return ` + <div class="variable-row"> + <span>${key} : ${JSON.stringify(value)} + <span class="timeticket">${id}</span> + </span> + </div> + `; + } + + const undoTool = document.querySelector('.toolbar .undo'); + const redoTool = document.querySelector('.toolbar .redo'); + const deleteTool = document.querySelector('.toolbar .delete'); + const rectangleTool = document.querySelector('.toolbar .rectangle'); + const shapesHolder = document.querySelector('.canvas .shapes'); + const selectionTool = document.querySelector('.selection-tools'); + const colorPicker = document.querySelector( + '.selection-tools .color-picker', + ); + const COLORS = [ + 'orangered', + 'gold', + 'limegreen', + 'dodgerblue', + 'darkviolet', + ]; + const rectangleSize = 50; + let isDragging = false; + + function getRandomInt(max) { + return Math.floor(Math.random() * max); + } + + function getRandomColor() { + return COLORS[getRandomInt(COLORS.length)]; + } + + function renderShapes(doc, myClientID) { + const shapes = doc.getRoot().shapes; + if (!shapes) return; + shapesHolder.innerHTML = ''; + for (const shape of shapes) { + const shapeElement = document.createElement('div'); + shapeElement.className = 'shape'; + shapeElement.style.transform = `translate(${shape.point.x}px, ${shape.point.y}px)`; + shapeElement.style.backgroundColor = shape.color; + const selectedByMe = doc.getMyPresence().selectedShape === shape.id; + const selectedByOthers = doc + .getPresences() + .some( + ({ clientID, presence }) => + presence.selectedShape === shape.id && clientID !== myClientID, + ); + const selectionColor = selectedByMe + ? 'blue' + : selectedByOthers + ? 'orange' + : 'transparent'; + shapeElement.style.borderColor = selectionColor; + shapeElement.setAttribute('data-id', shape.id); + + shapesHolder.appendChild(shapeElement); + shapeElement.addEventListener('pointerdown', (e) => { + e.stopPropagation(); + const currentSelectedShapeId = doc.getMyPresence().selectedShape; + isDragging = true; + if (currentSelectedShapeId === shape.id) return; + doc.update((root, presence) => { + presence.set({ selectedShape: shape.id }, { addToHistory: true }); + }); + }); + } + } + + const onCanvasPointerUp = (e, doc) => { + if (!isDragging) { + doc.update((root, presence) => { + presence.set({ selectedShape: null }, { addToHistory: true }); + }); + } + const selectedShapeId = doc.getMyPresence().selectedShape; + isDragging = false; + }; + const onCanvasPointerMove = (e, doc) => { + if (!isDragging) return; + const selectedShapeId = doc.getMyPresence().selectedShape; + + doc.update((root, presence) => { + const selectedShape = root.shapes.find( + (shape) => shape.id === selectedShapeId, + ); + if (!selectedShape) return; + selectedShape.point = { + x: e.clientX - rectangleSize / 2, + y: e.clientY - rectangleSize / 2, + }; + }); + }; + function displayOnlineClients(presences, myClientID) { + const usernames = []; + for (const { clientID, presence } of presences) { + usernames.push( + clientID === myClientID ? `<b>${clientID}</b>` : clientID, + ); + } + onlineClientsHolder.innerHTML = JSON.stringify(usernames); + } + + async function main() { + try { + // 01. create client with RPCAddr(envoy) then activate it. + const client = new yorkie.Client('http://localhost:8080'); + client.subscribe(network.statusListener(statusHolder)); + await client.activate(); + const myClientID = client.getID(); + + // 02. create a document then attach it into the client. + const doc = new yorkie.Document('whiteboard'); + doc.subscribe('presence', (event) => { + if (event.type === 'presence-changed') { + renderShapes(doc, myClientID); + } + displayOnlineClients(doc.getPresences(), client.getID()); + }); + doc.subscribe('my-presence', (event) => { + if (event.type === 'presence-changed') { + if (event.value.presence?.selectedShape) { + selectionTool.style.display = 'flex'; + } else { + selectionTool.style.display = 'none'; + } + } + }); + await client.attach(doc); + + shapesHolder.addEventListener('pointerup', (e) => { + onCanvasPointerUp(e, doc); + }); + shapesHolder.addEventListener('pointermove', (e) => { + onCanvasPointerMove(e, doc); + }); + + // 04. subscribe to document changes + doc.subscribe((event) => { + displayDocument(doc); + renderShapes(doc, myClientID); + printOps(doc); + }); + + // 05. set initial value + doc.update((root) => { + if (!root.shapes) { + root.shapes = []; + } + }); + displayDocument(doc); + renderShapes(doc, myClientID); + printOps(doc); + + const insertRectangle = () => { + const shapeId = Date.now().toString(); + doc.update((root, presence) => { + root.shapes.push({ + id: shapeId, + point: { + x: getRandomInt(300), + y: getRandomInt(300), + }, + color: getRandomColor(), + }); + presence.set({ selectedShape: shapeId }, { addToHistory: true }); + }); + }; + const setColor = (e) => { + if (!e.target.dataset.color) return; + const selectedShapeId = doc.getMyPresence().selectedShape; + doc.update((root) => { + const selectedShape = root.shapes.find( + (shape) => shape.id === selectedShapeId, + ); + if (!selectedShape) return; + selectedShape.color = e.target.dataset.color; + }); + }; + rectangleTool.onclick = insertRectangle; + undoTool.onclick = doc.history.undo; + redoTool.onclick = doc.history.redo; + colorPicker.onclick = setColor; + } catch (e) { + console.error(e); + } + } + + main(); + </script> + </body> +</html> diff --git a/src/document/change/change.ts b/src/document/change/change.ts index eb019173f..39d0257cc 100644 --- a/src/document/change/change.ts +++ b/src/document/change/change.ts @@ -144,8 +144,10 @@ export class Change<P extends Indexable> { } { const changeOpInfos: Array<OperationInfo> = []; const reverseOps: Array<HistoryOperation<P>> = []; + if (this.operations.length) { + root.opsForTest.push(this.operations); + } for (const operation of this.operations) { - root.opsForTest.push(operation); const executionResult = operation.execute(root, origin); if (!executionResult) continue; const { opInfos, reverseOp } = executionResult; diff --git a/src/document/document.ts b/src/document/document.ts index 029914344..3c367b95f 100644 --- a/src/document/document.ts +++ b/src/document/document.ts @@ -66,7 +66,7 @@ import { Presence, PresenceChangeType, } from '@yorkie-js-sdk/src/document/presence/presence'; -import { History } from '@yorkie-js-sdk/src/document/history'; +import { History, HistoryOperation } from '@yorkie-js-sdk/src/document/history'; /** * `DocumentOptions` are the options to create a new document. @@ -1420,14 +1420,14 @@ export class Document<T, P extends Indexable = Indexable> { /** * `getUndoStackForTest` returns the undo stack for test. */ - public getUndoStackForTest(): Array<Array<string>> { + public getUndoStackForTest(): Array<Array<HistoryOperation<P>>> { return this.internalHistory.getUndoStackForTest(); } /** * `getRedoStackForTest` returns the redo stack for test. */ - public getRedoStackForTest(): Array<Array<string>> { + public getRedoStackForTest(): Array<Array<HistoryOperation<P>>> { return this.internalHistory.getRedoStackForTest(); } } diff --git a/src/document/history.ts b/src/document/history.ts index ad99b40bd..bc5fc063d 100644 --- a/src/document/history.ts +++ b/src/document/history.ts @@ -96,22 +96,14 @@ export class History<P extends Indexable> { /** * `getUndoStackForTest` returns the undo stack for test. */ - public getUndoStackForTest(): Array<Array<string>> { - return this.undoStack.map((ops) => - ops.map((op) => { - return op instanceof Operation ? op.toTestString() : JSON.stringify(op); - }), - ); + public getUndoStackForTest(): Array<Array<HistoryOperation<P>>> { + return this.undoStack; } /** * `getRedoStackForTest` returns the redo stack for test. */ - public getRedoStackForTest(): Array<Array<string>> { - return this.redoStack.map((ops) => - ops.map((op) => { - return op instanceof Operation ? op.toTestString() : JSON.stringify(op); - }), - ); + public getRedoStackForTest(): Array<Array<HistoryOperation<P>>> { + return this.redoStack; } } diff --git a/src/document/operation/set_operation.ts b/src/document/operation/set_operation.ts index 99226f719..9b903086a 100644 --- a/src/document/operation/set_operation.ts +++ b/src/document/operation/set_operation.ts @@ -105,6 +105,8 @@ export class SetOperation extends Operation { ); if (value !== undefined && !value.isRemoved()) { + // TODO(chacha912): When the value is an object, + // it always sets as an empty object from the remote. reverseOp = SetOperation.create( this.key, value.deepcopy(), diff --git a/test/helper/helper.ts b/test/helper/helper.ts index 5b89c8890..2c3e722e6 100644 --- a/test/helper/helper.ts +++ b/test/helper/helper.ts @@ -19,7 +19,11 @@ import { assert } from 'chai'; import yorkie, { Tree, ElementNode } from '@yorkie-js-sdk/src/yorkie'; import { IndexTree } from '@yorkie-js-sdk/src/util/index_tree'; import { CRDTTreeNode } from '@yorkie-js-sdk/src/document/crdt/tree'; -import { OperationInfo } from '@yorkie-js-sdk/src/document/operation/operation'; +import { + OperationInfo, + Operation, +} from '@yorkie-js-sdk/src/document/operation/operation'; +import { HistoryOperation } from '@yorkie-js-sdk/src/document/history'; export type Indexable = Record<string, any>; @@ -228,3 +232,9 @@ export function buildIndexTree(node: ElementNode): IndexTree<CRDTTreeNode> { }); return doc.getRoot().t.getIndexTree(); } + +export function toStringHistoryOp<P extends Indexable>( + op: HistoryOperation<P>, +): string { + return op instanceof Operation ? op.toTestString() : JSON.stringify(op); +} diff --git a/test/integration/counter_test.ts b/test/integration/counter_test.ts index fadde1e58..3f8d04af2 100644 --- a/test/integration/counter_test.ts +++ b/test/integration/counter_test.ts @@ -1,5 +1,6 @@ import { describe, it, assert } from 'vitest'; import { Document } from '@yorkie-js-sdk/src/document/document'; +import { toStringHistoryOp } from '@yorkie-js-sdk/test/helper/helper'; import { withTwoClientsAndDocuments, assertUndoRedo, @@ -147,17 +148,18 @@ describe('Counter', function () { root.longCnt.increase(Long.fromString('9223372036854775807')); // 2^63-1 }); assert.equal(doc.toSortedJSON(), `{"cnt":1,"longCnt":9223372036854775807}`); - assert.equal( - JSON.stringify(doc.getUndoStackForTest().at(-1)), - `["1:00:2.INCREASE.-9223372036854775807","1:00:1.INCREASE.-1.5"]`, - ); + + assert.deepEqual(doc.getUndoStackForTest().at(-1)?.map(toStringHistoryOp), [ + '1:00:2.INCREASE.-9223372036854775807', + '1:00:1.INCREASE.-1.5', + ]); doc.history.undo(); assert.equal(doc.toSortedJSON(), `{"cnt":0,"longCnt":0}`); - assert.equal( - JSON.stringify(doc.getRedoStackForTest().at(-1)), - `["1:00:1.INCREASE.1.5","1:00:2.INCREASE.9223372036854775807"]`, - ); + assert.deepEqual(doc.getRedoStackForTest().at(-1)?.map(toStringHistoryOp), [ + '1:00:1.INCREASE.1.5', + '1:00:2.INCREASE.9223372036854775807', + ]); }); it('Can undo/redo for increase operation', async function ({ task }) { diff --git a/test/integration/presence_test.ts b/test/integration/presence_test.ts index 4c19736f5..c45f2dc41 100644 --- a/test/integration/presence_test.ts +++ b/test/integration/presence_test.ts @@ -632,10 +632,10 @@ describe('Undo/Redo', function () { }); assert.deepEqual(doc.getUndoStackForTest(), [ [ - JSON.stringify({ + { type: 'presence', value: { color: 'red', cursor: { x: 0, y: 0 } }, - }), + }, ], ]); @@ -662,16 +662,16 @@ describe('Undo/Redo', function () { }); assert.deepEqual(doc.getUndoStackForTest(), [ [ - JSON.stringify({ + { type: 'presence', value: { color: 'red', cursor: { x: 0, y: 0 } }, - }), + }, ], [ - JSON.stringify({ + { type: 'presence', value: { cursor: { x: 1, y: 1 } }, - }), + }, ], ]); @@ -698,16 +698,16 @@ describe('Undo/Redo', function () { }); assert.deepEqual(doc.getUndoStackForTest(), [ [ - JSON.stringify({ + { type: 'presence', value: { color: 'red', cursor: { x: 0, y: 0 } }, - }), + }, ], [ - JSON.stringify({ + { type: 'presence', value: { cursor: { x: 1, y: 1 } }, - }), + }, ], ]); From e6b198d8c1c564de256169a13b32fa42c86b4d28 Mon Sep 17 00:00:00 2001 From: Yourim Cha <yourim.cha@navercorp.com> Date: Tue, 17 Oct 2023 01:13:28 +0900 Subject: [PATCH 05/38] Add test to verify synchronization of nested object undo --- test/integration/object_test.ts | 45 +++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/test/integration/object_test.ts b/test/integration/object_test.ts index 827017c15..0d51d1461 100644 --- a/test/integration/object_test.ts +++ b/test/integration/object_test.ts @@ -348,6 +348,51 @@ describe('Object', function () { await client1.sync(); }); + it('Should ensure convergence after undoing nested objects', async function ({ + task, + }) { + // Test scenario: + // c1: set shape.point to {x: 0, y: 0} + // c1: set shape.point to {x: 1, y: 1} + // c1: undo + interface TestDoc { + shape?: { point: { x: number; y: number } }; + } + const docKey = toDocKey(`${task.name}-${new Date().getTime()}`); + const doc1 = new Document<TestDoc>(docKey); + const doc2 = new Document<TestDoc>(docKey); + + const client1 = new Client(testRPCAddr); + const client2 = new Client(testRPCAddr); + await client1.activate(); + await client2.activate(); + + await client1.attach(doc1, { isRealtimeSync: false }); + doc1.update((root) => { + root.shape = { point: { x: 0, y: 0 } }; + }); + await client1.sync(); + assert.equal(doc1.toSortedJSON(), '{"shape":{"point":{"x":0,"y":0}}}'); + + await client2.attach(doc2, { isRealtimeSync: false }); + assert.equal(doc2.toSortedJSON(), '{"shape":{"point":{"x":0,"y":0}}}'); + + doc1.update((root) => { + root.shape!.point = { x: 1, y: 1 }; + }); + await client1.sync(); + await client2.sync(); + assert.equal(doc1.toSortedJSON(), '{"shape":{"point":{"x":1,"y":1}}}'); + assert.equal(doc2.toSortedJSON(), '{"shape":{"point":{"x":1,"y":1}}}'); + + doc1.history.undo(); + assert.equal(doc1.toSortedJSON(), '{"shape":{"point":{"x":0,"y":0}}}'); + await client1.sync(); + await client2.sync(); + // TODO(chacha912): fix test + // assert.equal(doc2.toSortedJSON(), '{"shape":{"point":{"x":0,"y":0}}}'); + }); + it('concurrent undo/redo of object - no sync before undo', async function ({ task, }) { From 576cee149b64cea73478a4e7ad538434e435585e Mon Sep 17 00:00:00 2001 From: Yourim Cha <yourim.cha@navercorp.com> Date: Tue, 17 Oct 2023 11:55:04 +0900 Subject: [PATCH 06/38] Add rectangle deletion --- public/whiteboard.html | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/public/whiteboard.html b/public/whiteboard.html index 179c540a5..9d17e82e4 100644 --- a/public/whiteboard.html +++ b/public/whiteboard.html @@ -9,7 +9,8 @@ <body> <div class="canvas"> <div class="toolbar"> - <button class="rectangle">◼️ (new!)</button> + <button class="delete">🗑 delete</button> + <button class="rectangle">⬜️ (new!)</button> <button class="undo">⬅ undo</button> <button class="redo">➡️ redo</button> </div> @@ -95,6 +96,7 @@ <h2>redo stack</h2> </div> `; }) + .reverse() .join(''); } @@ -313,6 +315,18 @@ <h2>redo stack</h2> presence.set({ selectedShape: shapeId }, { addToHistory: true }); }); }; + const deleteRectangle = () => { + const selectedShapeId = doc.getMyPresence().selectedShape; + if (!selectedShapeId) return; + doc.update((root, presence) => { + const selectedShape = root.shapes.find( + (shape) => shape.id === selectedShapeId, + ); + if (!selectedShape) return; + root.shapes.deleteByID(selectedShape.getID()); + presence.set({ selectedShape: null }); + }); + }; const setColor = (e) => { if (!e.target.dataset.color) return; const selectedShapeId = doc.getMyPresence().selectedShape; @@ -325,9 +339,16 @@ <h2>redo stack</h2> }); }; rectangleTool.onclick = insertRectangle; - undoTool.onclick = doc.history.undo; - redoTool.onclick = doc.history.redo; + deleteTool.onclick = deleteRectangle; colorPicker.onclick = setColor; + undoTool.onclick = () => { + doc.history.undo(); + printOps(doc); + }; + redoTool.onclick = () => { + doc.history.redo(); + printOps(doc); + }; } catch (e) { console.error(e); } From 0c1523912c075e84c6e96e7f2b154a3c14da427e Mon Sep 17 00:00:00 2001 From: Yourim Cha <yourim.cha@navercorp.com> Date: Tue, 17 Oct 2023 13:49:39 +0900 Subject: [PATCH 07/38] Refactor object devtool --- public/devtool/object.css | 146 ++++++++++++++++++ public/devtool/object.js | 115 ++++++++++++++ public/whiteboard.css | 171 +++------------------ public/whiteboard.html | 310 ++++++++++++++++---------------------- 4 files changed, 411 insertions(+), 331 deletions(-) create mode 100644 public/devtool/object.css create mode 100644 public/devtool/object.js diff --git a/public/devtool/object.css b/public/devtool/object.css new file mode 100644 index 000000000..352120217 --- /dev/null +++ b/public/devtool/object.css @@ -0,0 +1,146 @@ +.devtool-root-holder, +.devtool-ops-holder { + display: flex; + flex-direction: column; + height: 100%; + overflow-y: scroll; + font-size: 14px; + font-weight: 400; +} + +.devtool-root-holder .object-content { + margin-left: 24px; +} + +.devtool-root-holder .object-key-val, +.devtool-root-holder .object-val { + position: relative; +} + +.devtool-root-holder .object-key-val:not(:last-of-type):before, +.devtool-root-holder .object-val:not(:last-of-type):before { + border: 1px dashed #ddd; + border-width: 0 0 0 1px; + content: ''; + position: absolute; + bottom: -12px; + left: -18px; + top: 12px; +} + +.devtool-root-holder .object-key-val:after, +.devtool-root-holder .object-val:after { + border-bottom: 1px dashed #ddd; + border-left: 1px dashed #ddd; + border-radius: 0 0 0 4px; + border-right: 0 dashed #ddd; + border-top: 0 dashed #ddd; + content: ''; + height: 16px; + position: absolute; + top: 0; + width: 16px; + height: 32px; + top: -16px; + left: -18px; +} + +.devtool-root-holder > .object-key-val:after, +.devtool-root-holder > .object-val:after { + content: none; +} + +.devtool-root-holder .object-val > span, +.devtool-root-holder .object-key-val > span, +.devtool-root-holder .object-val label, +.devtool-root-holder .object-key-val label { + border: 1px solid #ddd; + border-radius: 4px; + display: inline-block; + font-size: 14px; + font-weight: 500; + letter-spacing: 0 !important; + line-height: 1.72; + margin-bottom: 16px; + padding: 6px 8px; +} + +.devtool-root-holder label { + cursor: pointer; +} + +.devtool-root-holder .object-key-val label:before { + content: '▾'; + margin-right: 4px; +} + +.devtool-root-holder input[type='checkbox']:checked + label:before { + content: '▸'; +} + +.devtool-root-holder input[type='checkbox']:checked ~ .object-content { + display: none; +} + +.devtool-root-holder input[type='checkbox'] { + display: none; +} + +.devtool-root-holder .timeticket, +.devtool-ops-holder .timeticket { + border-radius: 4px; + background: #f1f2f3; + font-size: 12px; + font-weight: 400; + padding: 2px 6px; + margin-left: 4px; + letter-spacing: 1px; +} + +.devtool-ops-holder .change { + display: flex; + margin-bottom: 3px; + border-top: 1px solid #ddd; +} +.devtool-ops-holder label { + position: relative; + overflow: hidden; + padding-left: 24px; + cursor: pointer; + line-height: 1.6; +} +.devtool-ops-holder input[type='checkbox']:checked + label { + height: 22px; +} +.devtool-ops-holder input[type='checkbox'] { + display: none; +} +.devtool-ops-holder .count { + position: absolute; + left: 0px; + display: flex; + justify-content: center; + width: 20px; + height: 20px; + font-size: 13px; +} +.devtool-ops-holder .op { + display: block; +} +.devtool-ops-holder .op:first-child { + display: inline-block; +} +.devtool-ops-holder .op .type { + padding: 0 4px; + border-radius: 4px; + background: #e6e6fa; +} +.devtool-ops-holder .op .type.set { + background: #cff7cf; +} +.devtool-ops-holder .op .type.remove { + background: #f9c0c8; +} +.devtool-ops-holder .op .type.add { + background: #add8e6; +} diff --git a/public/devtool/object.js b/public/devtool/object.js new file mode 100644 index 000000000..d2f6d3641 --- /dev/null +++ b/public/devtool/object.js @@ -0,0 +1,115 @@ +const objectDevtool = ( + doc, + { rootHolder, opsHolder, undoOpsHolder, redoOpsHolder }, +) => { + const displayRootObject = () => { + const rootObj = doc.getRoot().toJSForTest(); + rootHolder.innerHTML = ` + <div class="devtool-root-holder"> + ${renderCRDTObject(rootObj)} + </div> + `; + }; + + const renderCRDTObject = ({ key, value, id }) => { + const valueHTML = Object.values(value) + .map((v) => { + return Object.prototype.toString.call(v.value) === '[object Object]' + ? renderCRDTObject(v) + : renderValue(v); + }) + .join(''); + if (key === undefined) key = 'root'; + return ` + <div class="object-key-val"> + ${renderKey({ key, id })} + <div class="object-content"> + ${valueHTML} + </div> + </div> + `; + }; + + const renderKey = ({ key, id }) => { + return ` + <input type="checkbox" id="${id}" /> + <label for="${id}">${key} + <span class="timeticket">${id}</span> + </label> + `; + }; + + const renderValue = ({ key, value, id }) => { + return ` + <div class="object-val"> + <span>${key} : ${JSON.stringify(value)} + <span class="timeticket">${id}</span> + </span> + </div> + `; + }; + + const displayOps = () => { + opsHolder.innerHTML = ` + <div class="devtool-ops-holder"> + ${renderOpsHolder(doc.getOpsForTest(), 'op')} + </div> + `; + }; + + const displayUndoOps = () => { + undoOpsHolder.innerHTML = ` + <div class="devtool-ops-holder"> + ${renderOpsHolder(doc.getUndoStackForTest(), 'undo')} + </div> + `; + }; + + const displayRedoOps = () => { + redoOpsHolder.innerHTML = ` + <div class="devtool-ops-holder"> + ${renderOpsHolder(doc.getRedoStackForTest(), 'redo')} + </div> + `; + }; + + const renderOpsHolder = (changes, idPrefix) => { + return changes + .map((ops, i) => { + const opsStr = ops + .map((op) => { + if (op.type === 'presence') { + return `<span class="op"><span class="type presence">presence</span>${JSON.stringify( + op.value, + )}</span>`; + } + let id = op.getExecutedAt()?.toTestString(); + if (id) { + id = `<span class="timeticket">${id}</span>`; + } + const opType = op.toTestString().split('.')[1]; + return `<span class="op"> + <span class="type ${opType.toLowerCase()}">${opType}</span> + ${id ? id : ''} + ${op.toTestString()} + </span>`; + }) + .join('\n'); + return ` + <div class="change"> + <input type="checkbox" id="${idPrefix}-${i}" /> + <label for="${idPrefix}-${i}"> + <span class="count">${ops.length}</span> + <span class="ops">${opsStr}</span> + </label> + </div> + `; + }) + .reverse() + .join(''); + }; + + return { displayRootObject, displayOps, displayUndoOps, displayRedoOps }; +}; + +export default objectDevtool; diff --git a/public/whiteboard.css b/public/whiteboard.css index 242f115c6..b45833e82 100644 --- a/public/whiteboard.css +++ b/public/whiteboard.css @@ -1,106 +1,32 @@ -.obj-dev-log { +.whiteboard-example { display: flex; + flex-direction: column; + height: 100vh; } -.obj-dev-log h2 { - margin: 0; -} - -.obj-dev-log > div { - width: 100%; - padding: 10px; -} - -.object-content { - margin-left: 24px; -} - -.object-key-val, -.variable-row { - position: relative; -} - -.object-key-val:not(:last-of-type):before, -.variable-row:not(:last-of-type):before { - border: 1px dashed #ddd; - border-width: 0 0 0 1px; - content: ''; - position: absolute; - bottom: -12px; - left: -18px; - top: 12px; -} - -.object-key-val:after, -.variable-row:after { - border-bottom: 1px dashed #ddd; - border-left: 1px dashed #ddd; - border-radius: 0 0 0 4px; - border-right: 0 dashed #ddd; - border-top: 0 dashed #ddd; - content: ''; - height: 16px; - position: absolute; - top: 0; - width: 16px; - height: 32px; - top: -16px; - left: -18px; -} - -#document-holder { - margin-top: 24px; -} - -#document-holder > .object-key-val:after, -#document-holder > .variable-row:after { - content: none; -} - -.variable-row > span, -.object-key-val > span, -.variable-row label, -.object-key-val label { - border: 1px solid #ddd; - border-radius: 4px; - display: inline-block; - font-size: 14px; - font-weight: 500; - letter-spacing: 0 !important; - line-height: 1.72; - margin-bottom: 16px; - padding: 6px 8px; -} - -.object-key-val label:before { - content: '▾'; - margin-right: 4px; -} - -#document-holder input[type='checkbox']:checked + label:before { - content: '▸'; +.dev-log-wrap { + display: flex; + flex-direction: column; + height: calc(100vh - 300px); } - -.timeticket { - border-radius: 4px; - background: #f1f2f3; - font-size: 12px; - font-weight: 400; - padding: 2px 6px; - margin-left: 4px; - letter-spacing: 1px; +.dev-log { + display: flex; + flex: 1; + overflow: hidden; } - -#document-holder input[type='checkbox']:checked ~ .object-content { - display: none; +.dev-log .log-holders { + padding: 10px; } - -#document-holder input[type='checkbox'] { - display: none; +.dev-log .log-holders, +.dev-log .log-holder-wrap, +.dev-log .log-holder { + display: flex; + flex-direction: column; + flex: 1; + overflow: hidden; } - -#document-holder label { - cursor: pointer; +.log-holder-wrap h2 { + margin: 10px 0; } .canvas { @@ -184,56 +110,3 @@ .selection-tools .color:nth-child(8) { background: white; } - -.ops-holder { - margin: 10px 0 20px; - font-size: 14px; - font-weight: 400; -} -.ops-holder .change { - display: flex; - margin-bottom: 3px; - border-top: 1px solid #ddd; -} -.ops-holder label { - position: relative; - overflow: hidden; - padding-left: 24px; - cursor: pointer; - line-height: 1.6; -} -.ops-holder input[type='checkbox']:checked + label { - height: 22px; -} -.ops-holder input[type='checkbox'] { - display: none; -} -.ops-holder .count { - position: absolute; - left: 0px; - display: flex; - justify-content: center; - width: 20px; - height: 20px; - font-size: 13px; -} -.ops-holder .op { - display: block; -} -.ops-holder .op:first-child { - display: inline-block; -} -.ops-holder .op .type { - padding: 0 4px; - border-radius: 4px; - background: #e6e6fa; -} -.ops-holder .op .type.set { - background: #cff7cf; -} -.ops-holder .op .type.remove { - background: #f9c0c8; -} -.ops-holder .op .type.add { - background: #add8e6; -} diff --git a/public/whiteboard.html b/public/whiteboard.html index 9d17e82e4..2a2df7b6c 100644 --- a/public/whiteboard.html +++ b/public/whiteboard.html @@ -5,51 +5,56 @@ <title>Whiteboard Example</title> <link rel="stylesheet" href="style.css" /> <link rel="stylesheet" href="whiteboard.css" /> + <link rel="stylesheet" href="devtool/object.css" /> </head> <body> - <div class="canvas"> - <div class="toolbar"> - <button class="delete">🗑 delete</button> - <button class="rectangle">⬜️ (new!)</button> - <button class="undo">⬅ undo</button> - <button class="redo">➡️ redo</button> - </div> - <div class="shapes"></div> - <div class="selection-tools"> - <div class="color-picker"> - <button class="color" data-color="orangered"></button> - <button class="color" data-color="gold"></button> - <button class="color" data-color="limegreen"></button> - <button class="color" data-color="dodgerblue"></button> - <button class="color" data-color="darkviolet"></button> - <button class="color" data-color="darkorange"></button> - <button class="color" data-color="dimgray"></button> - <button class="color" data-color="white"></button> - </div> - </div> - </div> - <div class="obj-dev-tool"> - <div id="network-status"></div> - <div id="online-clients"></div> - <div class="obj-dev-log"> - <div> - <h2>yorkie document</h2> - <div id="document-holder"></div> + <div class="whiteboard-example"> + <div class="canvas"> + <div class="toolbar"> + <button class="delete">🗑 delete</button> + <button class="rectangle">⬜️ (new!)</button> + <button class="undo">⬅ undo</button> + <button class="redo">➡️ redo</button> </div> - <div class="stack-holder"> - <div> - <h2>operations</h2> - <div id="ops-holder" class="ops-holder"></div> + <div class="shapes"></div> + <div class="selection-tools"> + <div class="color-picker"> + <button class="color" data-color="orangered"></button> + <button class="color" data-color="gold"></button> + <button class="color" data-color="limegreen"></button> + <button class="color" data-color="dodgerblue"></button> + <button class="color" data-color="darkviolet"></button> + <button class="color" data-color="darkorange"></button> + <button class="color" data-color="dimgray"></button> + <button class="color" data-color="white"></button> </div> </div> - <div class="stack-holder"> - <div> - <h2>undo stack</h2> - <div id="undo-holder" class="ops-holder"></div> + </div> + <div class="dev-log-wrap"> + <div id="network-status"></div> + <div id="online-clients"></div> + <div class="dev-log"> + <div class="log-holders"> + <div class="log-holder-wrap"> + <h2>yorkie document</h2> + <div id="document-holder" class="log-holder"></div> + </div> + </div> + <div class="log-holders"> + <div class="log-holder-wrap"> + <h2>operations</h2> + <div id="ops-holder" class="log-holder"></div> + </div> </div> - <div> - <h2>redo stack</h2> - <div id="redo-holder" class="ops-holder"></div> + <div class="log-holders"> + <div class="log-holder-wrap"> + <h2>undo stack</h2> + <div id="undo-holder" class="log-holder"></div> + </div> + <div class="log-holder-wrap"> + <h2>redo stack</h2> + <div id="redo-holder" class="log-holder"></div> + </div> </div> </div> </div> @@ -57,101 +62,13 @@ <h2>redo stack</h2> <script src="./yorkie.js"></script> <script src="./util.js"></script> <script type="module"> + import devtool from './devtool/object.js'; const statusHolder = document.getElementById('network-status'); const onlineClientsHolder = document.getElementById('online-clients'); + const docHolder = document.getElementById('document-holder'); + const opsHolder = document.getElementById('ops-holder'); const undoHolder = document.getElementById('undo-holder'); const redoHolder = document.getElementById('redo-holder'); - const opsHolder = document.getElementById('ops-holder'); - const docHolder = document.getElementById('document-holder'); - - function renderOpsHolder(changes, idPrefix) { - return changes - .map((ops, i) => { - const opsStr = ops - .map((op) => { - if (op.type === 'presence') { - return `<span class="op"><span class="type presence">presence</span>${JSON.stringify( - op.value, - )}</span>`; - } - let id = op.getExecutedAt()?.toTestString(); - if (id) { - id = `<span class="timeticket">${id}</span>`; - } - const opType = op.toTestString().split('.')[1]; - return `<span class="op"> - <span class="type ${opType.toLowerCase()}">${opType}</span> - ${id ? id : ''} - ${op.toTestString()} - </span>`; - }) - .join('\n'); - return ` - <div class="change"> - <input type="checkbox" id="${idPrefix}-${i}" /> - <label for="${idPrefix}-${i}"> - <span class="count">${ops.length}</span> - <span class="ops">${opsStr}</span> - </label> - </div> - `; - }) - .reverse() - .join(''); - } - - function printOps(doc) { - opsHolder.innerHTML = renderOpsHolder(doc.getOpsForTest(), 'op'); - undoHolder.innerHTML = renderOpsHolder( - doc.getUndoStackForTest(), - 'undo', - ); - redoHolder.innerHTML = renderOpsHolder( - doc.getRedoStackForTest(), - 'redo', - ); - } - - const displayDocument = (doc) => { - const docData = doc.getRoot().toJSForTest(); - docHolder.innerHTML = displayObject(docData); - }; - function displayObject({ key, value, id }) { - const valueHTML = Object.values(value) - .map((v) => { - return Object.prototype.toString.call(v.value) === '[object Object]' - ? displayObject(v) - : displayValue(v); - }) - .join(''); - if (!key) key = 'root'; - return ` - <div class="object-key-val"> - ${displayKey({ key, id })} - <div class="object-content"> - ${valueHTML} - </div> - </div> - `; - } - function displayKey({ key, id }) { - return ` - <input type="checkbox" id="${id}" /> - <label for="${id}">${key} - <span class="timeticket">${id}</span> - </label> - `; - } - function displayValue({ key, value, id }) { - return ` - <div class="variable-row"> - <span>${key} : ${JSON.stringify(value)} - <span class="timeticket">${id}</span> - </span> - </div> - `; - } - const undoTool = document.querySelector('.toolbar .undo'); const redoTool = document.querySelector('.toolbar .redo'); const deleteTool = document.querySelector('.toolbar .delete'); @@ -174,7 +91,6 @@ <h2>redo stack</h2> function getRandomInt(max) { return Math.floor(Math.random() * max); } - function getRandomColor() { return COLORS[getRandomInt(COLORS.length)]; } @@ -206,6 +122,8 @@ <h2>redo stack</h2> shapesHolder.appendChild(shapeElement); shapeElement.addEventListener('pointerdown', (e) => { e.stopPropagation(); + // TODO(chacha912): Let's use `doc.history.pause()` once it's implemented. + // Currently, every movement is being saved in the undo stack. const currentSelectedShapeId = doc.getMyPresence().selectedShape; isDragging = true; if (currentSelectedShapeId === shape.id) return; @@ -215,7 +133,6 @@ <h2>redo stack</h2> }); } } - const onCanvasPointerUp = (e, doc) => { if (!isDragging) { doc.update((root, presence) => { @@ -240,6 +157,45 @@ <h2>redo stack</h2> }; }); }; + + const insertRectangle = (doc) => { + const shapeId = Date.now().toString(); + doc.update((root, presence) => { + root.shapes.push({ + id: shapeId, + point: { + x: getRandomInt(300), + y: getRandomInt(300), + }, + color: getRandomColor(), + }); + presence.set({ selectedShape: shapeId }, { addToHistory: true }); + }); + }; + const deleteRectangle = (doc) => { + const selectedShapeId = doc.getMyPresence().selectedShape; + if (!selectedShapeId) return; + doc.update((root, presence) => { + const selectedShape = root.shapes.find( + (shape) => shape.id === selectedShapeId, + ); + if (!selectedShape) return; + root.shapes.deleteByID(selectedShape.getID()); + presence.set({ selectedShape: null }); + }); + }; + const setColor = (e, doc) => { + if (!e.target.dataset.color) return; + const selectedShapeId = doc.getMyPresence().selectedShape; + doc.update((root) => { + const selectedShape = root.shapes.find( + (shape) => shape.id === selectedShapeId, + ); + if (!selectedShape) return; + selectedShape.color = e.target.dataset.color; + }); + }; + function displayOnlineClients(presences, myClientID) { const usernames = []; for (const { clientID, presence } of presences) { @@ -277,78 +233,68 @@ <h2>redo stack</h2> }); await client.attach(doc); - shapesHolder.addEventListener('pointerup', (e) => { - onCanvasPointerUp(e, doc); - }); - shapesHolder.addEventListener('pointermove', (e) => { - onCanvasPointerMove(e, doc); + // setup devtool + const { + displayRootObject, + displayOps, + displayUndoOps, + displayRedoOps, + } = devtool(doc, { + rootHolder: docHolder, + opsHolder: opsHolder, + undoOpsHolder: undoHolder, + redoOpsHolder: redoHolder, }); - // 04. subscribe to document changes + const displayLog = () => { + displayRootObject(); + displayOps(); + displayUndoOps(); + displayRedoOps(); + }; + + // 03. subscribe to document changes doc.subscribe((event) => { - displayDocument(doc); renderShapes(doc, myClientID); - printOps(doc); + displayLog(); }); - // 05. set initial value + // 04. set initial value doc.update((root) => { if (!root.shapes) { root.shapes = []; } }); - displayDocument(doc); renderShapes(doc, myClientID); - printOps(doc); + displayLog(); - const insertRectangle = () => { - const shapeId = Date.now().toString(); - doc.update((root, presence) => { - root.shapes.push({ - id: shapeId, - point: { - x: getRandomInt(300), - y: getRandomInt(300), - }, - color: getRandomColor(), - }); - presence.set({ selectedShape: shapeId }, { addToHistory: true }); - }); + // 05. set event handlers + rectangleTool.onclick = () => { + insertRectangle(doc); }; - const deleteRectangle = () => { - const selectedShapeId = doc.getMyPresence().selectedShape; - if (!selectedShapeId) return; - doc.update((root, presence) => { - const selectedShape = root.shapes.find( - (shape) => shape.id === selectedShapeId, - ); - if (!selectedShape) return; - root.shapes.deleteByID(selectedShape.getID()); - presence.set({ selectedShape: null }); - }); + deleteTool.onclick = () => { + deleteRectangle(doc); }; - const setColor = (e) => { - if (!e.target.dataset.color) return; - const selectedShapeId = doc.getMyPresence().selectedShape; - doc.update((root) => { - const selectedShape = root.shapes.find( - (shape) => shape.id === selectedShapeId, - ); - if (!selectedShape) return; - selectedShape.color = e.target.dataset.color; - }); + colorPicker.onclick = (e) => { + setColor(e, doc); }; - rectangleTool.onclick = insertRectangle; - deleteTool.onclick = deleteRectangle; - colorPicker.onclick = setColor; undoTool.onclick = () => { doc.history.undo(); - printOps(doc); + // TODO(chacha912): Display log here before implementing `doc.subscribe('history')` + displayUndoOps(); + displayRedoOps(); }; redoTool.onclick = () => { doc.history.redo(); - printOps(doc); + displayUndoOps(); + displayRedoOps(); }; + shapesHolder.addEventListener('pointerup', (e) => { + onCanvasPointerUp(e, doc); + }); + shapesHolder.addEventListener('pointermove', (e) => { + onCanvasPointerMove(e, doc); + }); } catch (e) { console.error(e); } From 4b8e829f998596fa9be2064d8924350bb8d829fd Mon Sep 17 00:00:00 2001 From: Yourim Cha <yourim.cha@navercorp.com> Date: Tue, 17 Oct 2023 15:00:27 +0900 Subject: [PATCH 08/38] Refactor opsource code --- src/document/change/change.ts | 3 ++- src/document/crdt/root.ts | 3 ++- src/document/document.ts | 9 +++++---- src/document/operation/operation.ts | 6 +++++- src/document/operation/set_operation.ts | 8 ++++++-- 5 files changed, 20 insertions(+), 9 deletions(-) diff --git a/src/document/change/change.ts b/src/document/change/change.ts index 39d0257cc..2fc865a22 100644 --- a/src/document/change/change.ts +++ b/src/document/change/change.ts @@ -16,6 +16,7 @@ import { ActorID } from '@yorkie-js-sdk/src/document/time/actor_id'; import { + OpSource, Operation, OperationInfo, } from '@yorkie-js-sdk/src/document/operation/operation'; @@ -137,7 +138,7 @@ export class Change<P extends Indexable> { public execute( root: CRDTRoot, presences: Map<ActorID, P>, - origin?: string, + origin?: OpSource, ): { opInfos: Array<OperationInfo>; reverseOps: Array<HistoryOperation<P>>; diff --git a/src/document/crdt/root.ts b/src/document/crdt/root.ts index 1faac23ba..fdef97a96 100644 --- a/src/document/crdt/root.ts +++ b/src/document/crdt/root.ts @@ -25,6 +25,7 @@ import { CRDTGCElement, } from '@yorkie-js-sdk/src/document/crdt/element'; import { CRDTObject } from '@yorkie-js-sdk/src/document/crdt/object'; +import { Operation } from '@yorkie-js-sdk/src/document/operation/operation'; /** * `CRDTElementPair` is a structure that represents a pair of element and its @@ -69,7 +70,7 @@ export class CRDTRoot { * the element that has removed nodes when executing garbage collection. */ private elementHasRemovedNodesSetByCreatedAt: Set<string>; - public opsForTest: Array<any>; + public opsForTest: Array<Array<Operation>>; constructor(rootObject: CRDTObject) { this.rootObject = rootObject; diff --git a/src/document/document.ts b/src/document/document.ts index 3c367b95f..118cd6e01 100644 --- a/src/document/document.ts +++ b/src/document/document.ts @@ -50,6 +50,7 @@ import { } from '@yorkie-js-sdk/src/document/change/checkpoint'; import { TimeTicket } from '@yorkie-js-sdk/src/document/time/ticket'; import { + OpSource, OperationInfo, ObjectOperationInfo, TextOperationInfo, @@ -1277,7 +1278,7 @@ export class Document<T, P extends Indexable = Indexable> { const cloneExecutionResult = change.execute( this.clone!.root, this.clone!.presences, - 'UNDOREDO', + OpSource.UndoRedo, ); if ( !change.hasPresenceChange() && @@ -1289,7 +1290,7 @@ export class Document<T, P extends Indexable = Indexable> { const { opInfos, reverseOps } = change.execute( this.root, this.presences, - 'UNDOREDO', + OpSource.UndoRedo, ); const reversePresence = context.getReversePresence(); if (reversePresence) { @@ -1367,7 +1368,7 @@ export class Document<T, P extends Indexable = Indexable> { const cloneExecutionResult = change.execute( this.clone!.root, this.clone!.presences, - 'UNDOREDO', + OpSource.UndoRedo, ); if ( !change.hasPresenceChange() && @@ -1379,7 +1380,7 @@ export class Document<T, P extends Indexable = Indexable> { const { opInfos, reverseOps } = change.execute( this.root, this.presences, - 'UNDOREDO', + OpSource.UndoRedo, ); const reversePresence = context.getReversePresence(); if (reversePresence) { diff --git a/src/document/operation/operation.ts b/src/document/operation/operation.ts index 9990bca7b..ba5f394a7 100644 --- a/src/document/operation/operation.ts +++ b/src/document/operation/operation.ts @@ -20,6 +20,10 @@ import { TreeNode } from '@yorkie-js-sdk/src/document/crdt/tree'; import { CRDTRoot } from '@yorkie-js-sdk/src/document/crdt/root'; import { Indexable } from '@yorkie-js-sdk/src/document/document'; +export enum OpSource { + UndoRedo = 'undoredo', +} + /** * `OperationInfo` represents the information of an operation. * It is used to inform to the user what kind of operation was executed. @@ -226,6 +230,6 @@ export abstract class Operation { */ public abstract execute( root: CRDTRoot, - origin?: string, + origin?: OpSource, ): ExecutionResult | undefined; } diff --git a/src/document/operation/set_operation.ts b/src/document/operation/set_operation.ts index 9b903086a..3828e0462 100644 --- a/src/document/operation/set_operation.ts +++ b/src/document/operation/set_operation.ts @@ -20,6 +20,7 @@ import { CRDTElement } from '@yorkie-js-sdk/src/document/crdt/element'; import { CRDTRoot } from '@yorkie-js-sdk/src/document/crdt/root'; import { CRDTObject } from '@yorkie-js-sdk/src/document/crdt/object'; import { + OpSource, Operation, ExecutionResult, } from '@yorkie-js-sdk/src/document/operation/operation'; @@ -59,11 +60,14 @@ export class SetOperation extends Operation { /** * `execute` executes this operation on the given `CRDTRoot`. */ - public execute(root: CRDTRoot, source?: string): ExecutionResult | undefined { + public execute( + root: CRDTRoot, + source?: OpSource, + ): ExecutionResult | undefined { const parentObject = root.findByCreatedAt(this.getParentCreatedAt()); if ( - source === 'UNDOREDO' && + source === OpSource.UndoRedo && (!parentObject || parentObject.getRemovedAt()) ) { return; From 5e0256cb7e5795c3cb752ed2560c7c45e2acce8c Mon Sep 17 00:00:00 2001 From: Yourim Cha <yourim.cha@navercorp.com> Date: Tue, 17 Oct 2023 16:55:27 +0900 Subject: [PATCH 09/38] Add some comments --- src/api/converter.ts | 8 ++------ src/document/change/change.ts | 2 +- src/document/crdt/root.ts | 4 ++++ src/document/operation/set_operation.ts | 1 + 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/api/converter.ts b/src/api/converter.ts index 4f1916797..75165529f 100644 --- a/src/api/converter.ts +++ b/src/api/converter.ts @@ -1197,12 +1197,8 @@ function fromChangePack<P extends Indexable>( function fromObject(pbObject: PbJSONElement.JSONObject): CRDTObject { const rht = new ElementRHT(); for (const pbRHTNode of pbObject.getNodesList()) { - // eslint-disable-next-line - rht.set( - pbRHTNode.getKey(), - fromElement(pbRHTNode.getElement()!), - fromTimeTicket(pbObject.getCreatedAt())!, - ); + const value = fromElement(pbRHTNode.getElement()!); + rht.set(pbRHTNode.getKey(), value, value.getExecutedAt()); } const obj = new CRDTObject(fromTimeTicket(pbObject.getCreatedAt())!, rht); diff --git a/src/document/change/change.ts b/src/document/change/change.ts index 2fc865a22..ab43117d7 100644 --- a/src/document/change/change.ts +++ b/src/document/change/change.ts @@ -145,7 +145,7 @@ export class Change<P extends Indexable> { } { const changeOpInfos: Array<OperationInfo> = []; const reverseOps: Array<HistoryOperation<P>> = []; - if (this.operations.length) { + if (process.env.NODE_ENV !== 'production' && this.operations.length) { root.opsForTest.push(this.operations); } for (const operation of this.operations) { diff --git a/src/document/crdt/root.ts b/src/document/crdt/root.ts index fdef97a96..5e0c89596 100644 --- a/src/document/crdt/root.ts +++ b/src/document/crdt/root.ts @@ -70,6 +70,10 @@ export class CRDTRoot { * the element that has removed nodes when executing garbage collection. */ private elementHasRemovedNodesSetByCreatedAt: Set<string>; + /** + * `opsForTest` is used for debugging the entire operation. + * operations accumulate only in dev mode. + */ public opsForTest: Array<Array<Operation>>; constructor(rootObject: CRDTObject) { diff --git a/src/document/operation/set_operation.ts b/src/document/operation/set_operation.ts index 3828e0462..41db73784 100644 --- a/src/document/operation/set_operation.ts +++ b/src/document/operation/set_operation.ts @@ -66,6 +66,7 @@ export class SetOperation extends Operation { ): ExecutionResult | undefined { const parentObject = root.findByCreatedAt(this.getParentCreatedAt()); + // NOTE(chacha912): Handle cases where operations cannot be executed during undo and redo. if ( source === OpSource.UndoRedo && (!parentObject || parentObject.getRemovedAt()) From 01a4d2e2b84bda4ab0d3a1e71c31f1771969151c Mon Sep 17 00:00:00 2001 From: Yourim Cha <yourim.cha@navercorp.com> Date: Tue, 17 Oct 2023 16:58:37 +0900 Subject: [PATCH 10/38] Add sync button in whiteboard example --- public/whiteboard.css | 3 +++ public/whiteboard.html | 20 +++++++++++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/public/whiteboard.css b/public/whiteboard.css index b45833e82..73714f2a8 100644 --- a/public/whiteboard.css +++ b/public/whiteboard.css @@ -28,6 +28,9 @@ .log-holder-wrap h2 { margin: 10px 0; } +.dev-log-wrap .network { + margin-top: 10px; +} .canvas { position: relative; diff --git a/public/whiteboard.html b/public/whiteboard.html index 2a2df7b6c..2075eea26 100644 --- a/public/whiteboard.html +++ b/public/whiteboard.html @@ -31,7 +31,10 @@ </div> </div> <div class="dev-log-wrap"> - <div id="network-status"></div> + <div class="network"> + <span id="network-status"></span> + <button class="sync-button">disconnect</button> + </div> <div id="online-clients"></div> <div class="dev-log"> <div class="log-holders"> @@ -151,6 +154,8 @@ <h2>redo stack</h2> (shape) => shape.id === selectedShapeId, ); if (!selectedShape) return; + + // TODO(chacha912): Setting it up like this doesn't work as expected when undo selectedShape.point = { x: e.clientX - rectangleSize / 2, y: e.clientY - rectangleSize / 2, @@ -295,6 +300,19 @@ <h2>redo stack</h2> shapesHolder.addEventListener('pointermove', (e) => { onCanvasPointerMove(e, doc); }); + + let isRealtime = true; + const syncButton = document.querySelector('.sync-button'); + syncButton.addEventListener('click', async () => { + if (isRealtime) { + await client.pause(doc); + syncButton.textContent = 'Connect'; + } else { + await client.resume(doc); + syncButton.textContent = 'Disconnect'; + } + isRealtime = !isRealtime; + }); } catch (e) { console.error(e); } From 45e304620f2b138aac437626ec7469d2539170d0 Mon Sep 17 00:00:00 2001 From: Yourim Cha <yourim.cha@navercorp.com> Date: Tue, 17 Oct 2023 16:58:37 +0900 Subject: [PATCH 11/38] Add sync button in whiteboard example --- public/whiteboard.css | 5 ++++- public/whiteboard.html | 20 +++++++++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/public/whiteboard.css b/public/whiteboard.css index b45833e82..579c64c32 100644 --- a/public/whiteboard.css +++ b/public/whiteboard.css @@ -7,7 +7,7 @@ .dev-log-wrap { display: flex; flex-direction: column; - height: calc(100vh - 300px); + height: calc(100vh - 350px); } .dev-log { display: flex; @@ -28,6 +28,9 @@ .log-holder-wrap h2 { margin: 10px 0; } +.dev-log-wrap .network { + margin-top: 10px; +} .canvas { position: relative; diff --git a/public/whiteboard.html b/public/whiteboard.html index 2a2df7b6c..2075eea26 100644 --- a/public/whiteboard.html +++ b/public/whiteboard.html @@ -31,7 +31,10 @@ </div> </div> <div class="dev-log-wrap"> - <div id="network-status"></div> + <div class="network"> + <span id="network-status"></span> + <button class="sync-button">disconnect</button> + </div> <div id="online-clients"></div> <div class="dev-log"> <div class="log-holders"> @@ -151,6 +154,8 @@ <h2>redo stack</h2> (shape) => shape.id === selectedShapeId, ); if (!selectedShape) return; + + // TODO(chacha912): Setting it up like this doesn't work as expected when undo selectedShape.point = { x: e.clientX - rectangleSize / 2, y: e.clientY - rectangleSize / 2, @@ -295,6 +300,19 @@ <h2>redo stack</h2> shapesHolder.addEventListener('pointermove', (e) => { onCanvasPointerMove(e, doc); }); + + let isRealtime = true; + const syncButton = document.querySelector('.sync-button'); + syncButton.addEventListener('click', async () => { + if (isRealtime) { + await client.pause(doc); + syncButton.textContent = 'Connect'; + } else { + await client.resume(doc); + syncButton.textContent = 'Disconnect'; + } + isRealtime = !isRealtime; + }); } catch (e) { console.error(e); } From c47c7ede7d51487d36d2a4ed2672dfe9ad4877a9 Mon Sep 17 00:00:00 2001 From: Yourim Cha <yourim.cha@navercorp.com> Date: Wed, 25 Oct 2023 16:02:28 +0900 Subject: [PATCH 12/38] Replace `executedAt` with `movedAt` --- public/whiteboard.html | 13 ++++++++----- src/api/converter.ts | 2 +- src/document/crdt/element.ts | 21 +++++++++------------ src/document/crdt/element_rht.ts | 4 ++-- src/document/crdt/object.ts | 2 +- 5 files changed, 21 insertions(+), 21 deletions(-) diff --git a/public/whiteboard.html b/public/whiteboard.html index 2075eea26..3a054dce1 100644 --- a/public/whiteboard.html +++ b/public/whiteboard.html @@ -155,11 +155,14 @@ <h2>redo stack</h2> ); if (!selectedShape) return; - // TODO(chacha912): Setting it up like this doesn't work as expected when undo - selectedShape.point = { - x: e.clientX - rectangleSize / 2, - y: e.clientY - rectangleSize / 2, - }; + selectedShape.point.x = e.clientX - rectangleSize / 2; + selectedShape.point.y = e.clientY - rectangleSize / 2; + // TODO(chacha912): we can change the code as follows after + // modifying to allow nested objects in the set operation. + // selectedShape.point = { + // x: e.clientX - rectangleSize / 2, + // y: e.clientY - rectangleSize / 2, + // }; }); }; diff --git a/src/api/converter.ts b/src/api/converter.ts index 75165529f..40fae9f41 100644 --- a/src/api/converter.ts +++ b/src/api/converter.ts @@ -1198,7 +1198,7 @@ function fromObject(pbObject: PbJSONElement.JSONObject): CRDTObject { const rht = new ElementRHT(); for (const pbRHTNode of pbObject.getNodesList()) { const value = fromElement(pbRHTNode.getElement()!); - rht.set(pbRHTNode.getKey(), value, value.getExecutedAt()); + rht.set(pbRHTNode.getKey(), value, value.getLastExcutedAt()); } const obj = new CRDTObject(fromTimeTicket(pbObject.getCreatedAt())!, rht); diff --git a/src/document/crdt/element.ts b/src/document/crdt/element.ts index 409e2dc9d..bdff8a20f 100644 --- a/src/document/crdt/element.ts +++ b/src/document/crdt/element.ts @@ -25,11 +25,9 @@ export abstract class CRDTElement { private createdAt: TimeTicket; private movedAt?: TimeTicket; private removedAt?: TimeTicket; - private executedAt: TimeTicket; constructor(createdAt: TimeTicket) { this.createdAt = createdAt; - this.executedAt = createdAt; } /** @@ -61,10 +59,16 @@ export abstract class CRDTElement { } /** - * `getExecutedAt` returns the execution time of this element. + * `getLastExcutedAt` returns the most recent time at which + * an operation was executed on this element. + * TODO(chacha912): we can replace RGATreeListNode.getPositionedAt with this method. */ - public getExecutedAt(): TimeTicket { - return this.executedAt; + public getLastExcutedAt(): TimeTicket { + if (!this.movedAt) { + return this.createdAt; + } + + return this.movedAt; } /** @@ -86,13 +90,6 @@ export abstract class CRDTElement { this.removedAt = removedAt; } - /** - * `setExecutedAt` sets the execution time of this element. - */ - public setExecutedAt(executedAt: TimeTicket): void { - this.executedAt = executedAt; - } - /** * `remove` removes this element. */ diff --git a/src/document/crdt/element_rht.ts b/src/document/crdt/element_rht.ts index e1e2aca28..7f3a85d2a 100644 --- a/src/document/crdt/element_rht.ts +++ b/src/document/crdt/element_rht.ts @@ -106,9 +106,9 @@ export class ElementRHT { const newNode = ElementRHTNode.of(key, value); this.nodeMapByCreatedAt.set(value.getCreatedAt().toIDString(), newNode); - if (node == null || executedAt.after(node.getValue().getExecutedAt())) { + if (node == null || executedAt.after(node.getValue().getLastExcutedAt())) { this.nodeMapByKey.set(key, newNode); - value.setExecutedAt(executedAt); + value.setMovedAt(executedAt); } return removed; } diff --git a/src/document/crdt/object.ts b/src/document/crdt/object.ts index 1fd824614..d642ffc0d 100644 --- a/src/document/crdt/object.ts +++ b/src/document/crdt/object.ts @@ -181,7 +181,7 @@ export class CRDTObject extends CRDTContainer { clone.memberNodes.set( node.getStrKey(), node.getValue().deepcopy(), - this.getExecutedAt(), + this.getLastExcutedAt(), ); } clone.remove(this.getRemovedAt()); From 05cf05179267d9dbbcba6ef4327a1292305f4f6e Mon Sep 17 00:00:00 2001 From: Yourim Cha <yourim.cha@navercorp.com> Date: Wed, 25 Oct 2023 16:23:49 +0900 Subject: [PATCH 13/38] Fix bug caused by shared reference when adding elements --- src/document/json/array.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/document/json/array.ts b/src/document/json/array.ts index b449f4f65..64373b4cd 100644 --- a/src/document/json/array.ts +++ b/src/document/json/array.ts @@ -448,7 +448,7 @@ export class ArrayProxy { AddOperation.create( target.getCreatedAt(), prevCreatedAt, - primitive, + primitive.deepcopy(), ticket, ), ); @@ -462,7 +462,7 @@ export class ArrayProxy { AddOperation.create( target.getCreatedAt(), prevCreatedAt, - array, + array.deepcopy(), ticket, ), ); @@ -475,7 +475,12 @@ export class ArrayProxy { target.insertAfter(prevCreatedAt, obj); context.registerElement(obj, target); context.push( - AddOperation.create(target.getCreatedAt(), prevCreatedAt, obj, ticket), + AddOperation.create( + target.getCreatedAt(), + prevCreatedAt, + obj.deepcopy(), + ticket, + ), ); for (const [k, v] of Object.entries(value!)) { From 9b2dc2b181635f4c064b6dcc9e38a26d7356b2e7 Mon Sep 17 00:00:00 2001 From: Yourim Cha <yourim.cha@navercorp.com> Date: Fri, 27 Oct 2023 15:22:26 +0900 Subject: [PATCH 14/38] Cleanup test code --- src/document/crdt/element.ts | 2 +- src/document/crdt/element_rht.ts | 6 +- test/integration/object_test.ts | 153 +++++++++++++++---------------- 3 files changed, 77 insertions(+), 84 deletions(-) diff --git a/src/document/crdt/element.ts b/src/document/crdt/element.ts index bdff8a20f..f98b252ea 100644 --- a/src/document/crdt/element.ts +++ b/src/document/crdt/element.ts @@ -96,7 +96,7 @@ export abstract class CRDTElement { public remove(removedAt?: TimeTicket): boolean { if ( removedAt && - removedAt.after(this.createdAt) && + removedAt.after(this.getLastExcutedAt()) && (!this.removedAt || removedAt.after(this.removedAt)) ) { this.removedAt = removedAt; diff --git a/src/document/crdt/element_rht.ts b/src/document/crdt/element_rht.ts index 7f3a85d2a..efcce5e8d 100644 --- a/src/document/crdt/element_rht.ts +++ b/src/document/crdt/element_rht.ts @@ -96,11 +96,7 @@ export class ElementRHT { ): CRDTElement | undefined { let removed; const node = this.nodeMapByKey.get(key); - if ( - node != null && - !node.isRemoved() && - node.remove(value.getCreatedAt()) - ) { + if (node != null && !node.isRemoved() && node.remove(executedAt)) { removed = node.getValue(); } diff --git a/test/integration/object_test.ts b/test/integration/object_test.ts index 0d51d1461..fef0104ff 100644 --- a/test/integration/object_test.ts +++ b/test/integration/object_test.ts @@ -291,13 +291,15 @@ describe('Object', function () { assertUndoRedo(doc, states); }); - it('Can handle concurrent undo/redo', async function ({ task }) { + it.skip(`Should ensure convergence of peer's document after undoing nested objects`, async function ({ + task, + }) { // Test scenario: - // c1: set shape.color to 'red' - // c2: delete shape - // c1: undo(no changes as the shape was deleted) + // c1: set shape.point to {x: 0, y: 0} + // c1: set shape.point to {x: 1, y: 1} + // c1: undo interface TestDoc { - shape?: { color: string }; + shape?: { point: { x: number; y: number } }; } const docKey = toDocKey(`${task.name}-${new Date().getTime()}`); const doc1 = new Document<TestDoc>(docKey); @@ -310,53 +312,39 @@ describe('Object', function () { await client1.attach(doc1, { isRealtimeSync: false }); doc1.update((root) => { - root.shape = { color: 'black' }; - }, 'init doc'); + root.shape = { point: { x: 0, y: 0 } }; + }); await client1.sync(); - assert.equal(doc1.toSortedJSON(), '{"shape":{"color":"black"}}'); + assert.equal(doc1.toSortedJSON(), '{"shape":{"point":{"x":0,"y":0}}}'); await client2.attach(doc2, { isRealtimeSync: false }); - assert.equal(doc2.toSortedJSON(), '{"shape":{"color":"black"}}'); + assert.equal(doc2.toSortedJSON(), '{"shape":{"point":{"x":0,"y":0}}}'); doc1.update((root) => { - root.shape!.color = 'red'; - }, 'set red'); + root.shape!.point = { x: 1, y: 1 }; + }); await client1.sync(); await client2.sync(); - doc2.update((root) => { - delete root.shape; - }, 'delete shape'); - await client2.sync(); - await client1.sync(); - assert.equal(doc1.toSortedJSON(), '{}'); - assert.equal(doc2.toSortedJSON(), '{}'); + assert.equal(doc1.toSortedJSON(), '{"shape":{"point":{"x":1,"y":1}}}'); + assert.equal(doc2.toSortedJSON(), '{"shape":{"point":{"x":1,"y":1}}}'); - // there is no change because c2 deletes the shape doc1.history.undo(); - assert.equal(doc1.toSortedJSON(), '{}'); - assert.equal(doc1.getRedoStackForTest().length, 0); - assert.equal(doc1.history.canRedo(), false); - await client1.sync(); - await client2.sync(); - await client1.sync(); - - doc2.history.undo(); - assert.equal(doc2.toSortedJSON(), '{"shape":{"color":"red"}}'); - assert.equal(doc2.toSortedJSON(), '{"shape":{"color":"red"}}'); + assert.equal(doc1.toSortedJSON(), '{"shape":{"point":{"x":0,"y":0}}}'); await client1.sync(); await client2.sync(); - await client1.sync(); + // TODO(chacha912): fix test + assert.equal(doc2.toSortedJSON(), '{"shape":{"point":{"x":0,"y":0}}}'); // as-is: {"shape":{"point":{}}} }); - it('Should ensure convergence after undoing nested objects', async function ({ + it(`Should handle case when it's not possible to perform reverse operation`, async function ({ task, }) { // Test scenario: - // c1: set shape.point to {x: 0, y: 0} - // c1: set shape.point to {x: 1, y: 1} - // c1: undo + // c1: set shape.color to 'red' + // c2: delete shape + // c1: undo(no changes as the shape was deleted) interface TestDoc { - shape?: { point: { x: number; y: number } }; + shape?: { color: string }; } const docKey = toDocKey(`${task.name}-${new Date().getTime()}`); const doc1 = new Document<TestDoc>(docKey); @@ -369,31 +357,45 @@ describe('Object', function () { await client1.attach(doc1, { isRealtimeSync: false }); doc1.update((root) => { - root.shape = { point: { x: 0, y: 0 } }; - }); + root.shape = { color: 'black' }; + }, 'init doc'); await client1.sync(); - assert.equal(doc1.toSortedJSON(), '{"shape":{"point":{"x":0,"y":0}}}'); + assert.equal(doc1.toSortedJSON(), '{"shape":{"color":"black"}}'); await client2.attach(doc2, { isRealtimeSync: false }); - assert.equal(doc2.toSortedJSON(), '{"shape":{"point":{"x":0,"y":0}}}'); + assert.equal(doc2.toSortedJSON(), '{"shape":{"color":"black"}}'); doc1.update((root) => { - root.shape!.point = { x: 1, y: 1 }; - }); + root.shape!.color = 'red'; + }, 'set red'); await client1.sync(); await client2.sync(); - assert.equal(doc1.toSortedJSON(), '{"shape":{"point":{"x":1,"y":1}}}'); - assert.equal(doc2.toSortedJSON(), '{"shape":{"point":{"x":1,"y":1}}}'); + doc2.update((root) => { + delete root.shape; + }, 'delete shape'); + await client2.sync(); + await client1.sync(); + assert.equal(doc1.toSortedJSON(), '{}'); + assert.equal(doc2.toSortedJSON(), '{}'); + // there is no change because c2 deletes the shape doc1.history.undo(); - assert.equal(doc1.toSortedJSON(), '{"shape":{"point":{"x":0,"y":0}}}'); + assert.equal(doc1.toSortedJSON(), '{}'); + assert.equal(doc1.getRedoStackForTest().length, 0); + assert.equal(doc1.history.canRedo(), false); await client1.sync(); await client2.sync(); - // TODO(chacha912): fix test - // assert.equal(doc2.toSortedJSON(), '{"shape":{"point":{"x":0,"y":0}}}'); + await client1.sync(); + + doc2.history.undo(); + assert.equal(doc2.toSortedJSON(), '{"shape":{"color":"red"}}'); + assert.equal(doc2.toSortedJSON(), '{"shape":{"color":"red"}}'); + await client1.sync(); + await client2.sync(); + await client1.sync(); }); - it('concurrent undo/redo of object - no sync before undo', async function ({ + it('Can handle concurrent undo/redo: local undo & global redo', async function ({ task, }) { interface TestDoc { @@ -409,13 +411,13 @@ describe('Object', function () { await client2.activate(); await client1.attach(doc1, { isRealtimeSync: false }); + await client2.attach(doc2, { isRealtimeSync: false }); doc1.update((root) => { root.color = 'black'; }, 'init doc'); await client1.sync(); + await client2.sync(); assert.equal(doc1.toSortedJSON(), '{"color":"black"}'); - - await client2.attach(doc2, { isRealtimeSync: false }); assert.equal(doc2.toSortedJSON(), '{"color":"black"}'); doc1.update((root) => { @@ -424,40 +426,40 @@ describe('Object', function () { doc2.update((root) => { root.color = 'green'; }, 'set green'); - - assert.equal(doc1.toSortedJSON(), '{"color":"red"}'); - assert.equal(doc2.toSortedJSON(), '{"color":"green"}'); - - doc1.history.undo(); - assert.equal(doc1.toSortedJSON(), '{"color":"black"}'); - await client1.sync(); await client2.sync(); await client1.sync(); - - // client 2's green set wins client 1's undo assert.equal(doc1.toSortedJSON(), '{"color":"green"}'); assert.equal(doc2.toSortedJSON(), '{"color":"green"}'); - doc1.history.redo(); - + doc1.history.undo(); + assert.equal(doc1.toSortedJSON(), '{"color":"black"}'); // local-undo await client1.sync(); await client2.sync(); - await client1.sync(); + assert.equal(doc2.toSortedJSON(), '{"color":"black"}'); - assert.equal(doc1.toSortedJSON(), '{"color":"red"}'); - assert.equal(doc2.toSortedJSON(), '{"color":"red"}'); + doc1.history.redo(); + assert.equal(doc1.toSortedJSON(), '{"color":"green"}'); // global-redo + await client1.sync(); + await client2.sync(); + assert.equal(doc2.toSortedJSON(), '{"color":"green"}'); }); - it('concurrent undo/redo of object - sync before undo', async function ({ + it('Can properly apply remote operations that occurred before local undo', async function ({ task, }) { + // Test scenario: + // c1 & c2: color='black' + // c1: set color to 'red' + // c2: set color to 'green' + // c1: undo + // sync c1 & c2 interface TestDoc { color: string; } const docKey = toDocKey(`${task.name}-${new Date().getTime()}`); - const doc1 = new Document<TestDoc>(docKey); - const doc2 = new Document<TestDoc>(docKey); + const doc1 = new Document<TestDoc>(docKey, { disableGC: true }); + const doc2 = new Document<TestDoc>(docKey, { disableGC: true }); const client1 = new Client(testRPCAddr); const client2 = new Client(testRPCAddr); @@ -465,13 +467,14 @@ describe('Object', function () { await client2.activate(); await client1.attach(doc1, { isRealtimeSync: false }); + await client2.attach(doc2, { isRealtimeSync: false }); doc1.update((root) => { root.color = 'black'; }, 'init doc'); await client1.sync(); + await client2.sync(); + await client1.sync(); assert.equal(doc1.toSortedJSON(), '{"color":"black"}'); - - await client2.attach(doc2, { isRealtimeSync: false }); assert.equal(doc2.toSortedJSON(), '{"color":"black"}'); doc1.update((root) => { @@ -480,10 +483,8 @@ describe('Object', function () { doc2.update((root) => { root.color = 'green'; }, 'set green'); - await client1.sync(); - await client2.sync(); - await client1.sync(); - assert.equal(doc1.toSortedJSON(), '{"color":"green"}'); + + assert.equal(doc1.toSortedJSON(), '{"color":"red"}'); assert.equal(doc2.toSortedJSON(), '{"color":"green"}'); doc1.history.undo(); @@ -492,18 +493,14 @@ describe('Object', function () { await client1.sync(); await client2.sync(); await client1.sync(); - assert.equal(doc1.toSortedJSON(), '{"color":"black"}'); assert.equal(doc2.toSortedJSON(), '{"color":"black"}'); doc1.history.redo(); - await client1.sync(); await client2.sync(); - await client1.sync(); - - assert.equal(doc1.toSortedJSON(), '{"color":"green"}'); - assert.equal(doc2.toSortedJSON(), '{"color":"green"}'); + assert.equal(doc1.toSortedJSON(), '{"color":"red"}'); + assert.equal(doc2.toSortedJSON(), '{"color":"red"}'); }); }); }); From 1b63499d25c862ee1da5cf2c7606d5adfec71e8e Mon Sep 17 00:00:00 2001 From: Yourim Cha <yourim.cha@navercorp.com> Date: Fri, 27 Oct 2023 18:00:46 +0900 Subject: [PATCH 15/38] Add test case for reverse operations referencing already garbage-collected elements --- test/integration/object_test.ts | 43 +++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/test/integration/object_test.ts b/test/integration/object_test.ts index fef0104ff..1c67a3843 100644 --- a/test/integration/object_test.ts +++ b/test/integration/object_test.ts @@ -503,4 +503,47 @@ describe('Object', function () { assert.equal(doc2.toSortedJSON(), '{"color":"red"}'); }); }); + + it.skip(`Should handle case of reverse operations referencing already garbage-collected elements`, async function ({ + task, + }) { + interface TestDoc { + shape: { color: string }; + } + const docKey = toDocKey(`${task.name}-${new Date().getTime()}`); + const doc1 = new Document<TestDoc>(docKey); + const doc2 = new Document<TestDoc>(docKey); + + const client1 = new Client(testRPCAddr); + const client2 = new Client(testRPCAddr); + await client1.activate(); + await client2.activate(); + + await client1.attach(doc1, { isRealtimeSync: false }); + await client2.attach(doc2, { isRealtimeSync: false }); + + doc1.update((root) => { + root.shape = { color: 'black' }; + }); + doc2.update((root) => { + root.shape = { color: 'yellow' }; + }); + await client1.sync(); + await client2.sync(); + await client1.sync(); + assert.equal(doc1.toSortedJSON(), '{"shape":{"color":"yellow"}}'); + assert.equal(doc2.toSortedJSON(), '{"shape":{"color":"yellow"}}'); + + doc2.update((root) => { + root.shape.color = 'red'; + }); + await client2.sync(); + await client1.sync(); + await client2.sync(); + assert.equal(doc1.toSortedJSON(), '{"shape":{"color":"red"}}'); + assert.equal(doc2.toSortedJSON(), '{"shape":{"color":"red"}}'); + + // TODO(chacha912): fix error that occurs when undoing + doc1.history.undo(); + }); }); From c910f2d4328fc375099055af247090ea9d139fb6 Mon Sep 17 00:00:00 2001 From: Yourim Cha <yourim.cha@navercorp.com> Date: Mon, 6 Nov 2023 15:53:12 +0900 Subject: [PATCH 16/38] Handle reverse remove operation targeting elements deleted by other peers --- public/whiteboard.html | 4 +- src/document/operation/remove_operation.ts | 25 +++- test/integration/object_test.ts | 132 ++++++++++++++------- 3 files changed, 112 insertions(+), 49 deletions(-) diff --git a/public/whiteboard.html b/public/whiteboard.html index 3a054dce1..0b9ffa047 100644 --- a/public/whiteboard.html +++ b/public/whiteboard.html @@ -223,7 +223,9 @@ <h2>redo stack</h2> const myClientID = client.getID(); // 02. create a document then attach it into the client. - const doc = new yorkie.Document('whiteboard'); + const doc = new yorkie.Document('whiteboard', { + disableGC: true, + }); doc.subscribe('presence', (event) => { if (event.type === 'presence-changed') { renderShapes(doc, myClientID); diff --git a/src/document/operation/remove_operation.ts b/src/document/operation/remove_operation.ts index ee4039114..71b980716 100644 --- a/src/document/operation/remove_operation.ts +++ b/src/document/operation/remove_operation.ts @@ -18,6 +18,7 @@ import { logger } from '@yorkie-js-sdk/src/util/logger'; import { TimeTicket } from '@yorkie-js-sdk/src/document/time/ticket'; import { CRDTRoot } from '@yorkie-js-sdk/src/document/crdt/root'; import { + OpSource, Operation, OperationInfo, ExecutionResult, @@ -56,8 +57,20 @@ export class RemoveOperation extends Operation { /** * `execute` executes this operation on the given `CRDTRoot`. */ - public execute(root: CRDTRoot): ExecutionResult { + public execute( + root: CRDTRoot, + source?: OpSource, + ): ExecutionResult | undefined { const parentObject = root.findByCreatedAt(this.getParentCreatedAt()); + + // NOTE(chacha912): Handle cases where operations cannot be executed during undo and redo. + if ( + source === OpSource.UndoRedo && + (!parentObject || parentObject.getRemovedAt()) + ) { + return; + } + if (!parentObject) { logger.fatal(`fail to find ${this.getParentCreatedAt()}`); } @@ -66,6 +79,14 @@ export class RemoveOperation extends Operation { } const obj = parentObject as CRDTContainer; const key = obj.subPathOf(this.createdAt); + // NOTE(chacha912): Handle cases where operations cannot be executed during undo and redo. + if ( + source === OpSource.UndoRedo && + ((obj instanceof CRDTObject && !obj.has(key!)) || + (obj instanceof CRDTArray && !obj.get(this.createdAt))) + ) { + return; + } const reverseOp = this.getReverseOperation(obj); const elem = obj.delete(this.createdAt, this.getExecutedAt()); @@ -97,7 +118,7 @@ export class RemoveOperation extends Operation { public getReverseOperation( parentObject: CRDTContainer, ): Operation | undefined { - //TODO(Hyemmie): consider CRDTArray + // TODO(Hyemmie): consider CRDTArray if (parentObject instanceof CRDTObject) { const key = parentObject.subPathOf(this.createdAt); if (key !== undefined) { diff --git a/test/integration/object_test.ts b/test/integration/object_test.ts index 1c67a3843..f6afad0e8 100644 --- a/test/integration/object_test.ts +++ b/test/integration/object_test.ts @@ -336,7 +336,7 @@ describe('Object', function () { assert.equal(doc2.toSortedJSON(), '{"shape":{"point":{"x":0,"y":0}}}'); // as-is: {"shape":{"point":{}}} }); - it(`Should handle case when it's not possible to perform reverse operation`, async function ({ + it(`Should handle reverse (set) operation targeting elements deleted by other peers`, async function ({ task, }) { // Test scenario: @@ -378,7 +378,7 @@ describe('Object', function () { assert.equal(doc1.toSortedJSON(), '{}'); assert.equal(doc2.toSortedJSON(), '{}'); - // there is no change because c2 deletes the shape + // c2 deleted the shape, so the reverse operation cannot be applied doc1.history.undo(); assert.equal(doc1.toSortedJSON(), '{}'); assert.equal(doc1.getRedoStackForTest().length, 0); @@ -386,10 +386,50 @@ describe('Object', function () { await client1.sync(); await client2.sync(); await client1.sync(); + }); - doc2.history.undo(); - assert.equal(doc2.toSortedJSON(), '{"shape":{"color":"red"}}'); - assert.equal(doc2.toSortedJSON(), '{"shape":{"color":"red"}}'); + it(`Should handle reverse (remove) operation targeting elements deleted by other peers`, async function ({ + task, + }) { + // Test scenario: + // c1: create shape + // c2: delete shape + // c1: undo(no changes as the shape was deleted) + interface TestDoc { + shape?: { color: string }; + } + const docKey = toDocKey(`${task.name}-${new Date().getTime()}`); + const doc1 = new Document<TestDoc>(docKey, { disableGC: true }); + const doc2 = new Document<TestDoc>(docKey, { disableGC: true }); + + const client1 = new Client(testRPCAddr); + const client2 = new Client(testRPCAddr); + await client1.activate(); + await client2.activate(); + + await client1.attach(doc1, { isRealtimeSync: false }); + doc1.update((root) => { + root.shape = { color: 'black' }; + }, 'init doc'); + await client1.sync(); + assert.equal(doc1.toSortedJSON(), '{"shape":{"color":"black"}}'); + + await client2.attach(doc2, { isRealtimeSync: false }); + assert.equal(doc2.toSortedJSON(), '{"shape":{"color":"black"}}'); + + doc2.update((root) => { + delete root.shape; + }, 'delete shape'); + await client2.sync(); + await client1.sync(); + assert.equal(doc1.toSortedJSON(), '{}'); + assert.equal(doc2.toSortedJSON(), '{}'); + + // c2 deleted the shape, so the reverse operation cannot be applied + doc1.history.undo(); + assert.equal(doc1.toSortedJSON(), '{}'); + assert.equal(doc1.getRedoStackForTest().length, 0); + assert.equal(doc1.history.canRedo(), false); await client1.sync(); await client2.sync(); await client1.sync(); @@ -502,48 +542,48 @@ describe('Object', function () { assert.equal(doc1.toSortedJSON(), '{"color":"red"}'); assert.equal(doc2.toSortedJSON(), '{"color":"red"}'); }); - }); - it.skip(`Should handle case of reverse operations referencing already garbage-collected elements`, async function ({ - task, - }) { - interface TestDoc { - shape: { color: string }; - } - const docKey = toDocKey(`${task.name}-${new Date().getTime()}`); - const doc1 = new Document<TestDoc>(docKey); - const doc2 = new Document<TestDoc>(docKey); - - const client1 = new Client(testRPCAddr); - const client2 = new Client(testRPCAddr); - await client1.activate(); - await client2.activate(); - - await client1.attach(doc1, { isRealtimeSync: false }); - await client2.attach(doc2, { isRealtimeSync: false }); - - doc1.update((root) => { - root.shape = { color: 'black' }; - }); - doc2.update((root) => { - root.shape = { color: 'yellow' }; - }); - await client1.sync(); - await client2.sync(); - await client1.sync(); - assert.equal(doc1.toSortedJSON(), '{"shape":{"color":"yellow"}}'); - assert.equal(doc2.toSortedJSON(), '{"shape":{"color":"yellow"}}'); - - doc2.update((root) => { - root.shape.color = 'red'; + it.skip(`Should handle case of reverse operations referencing already garbage-collected elements`, async function ({ + task, + }) { + interface TestDoc { + shape: { color: string }; + } + const docKey = toDocKey(`${task.name}-${new Date().getTime()}`); + const doc1 = new Document<TestDoc>(docKey); + const doc2 = new Document<TestDoc>(docKey); + + const client1 = new Client(testRPCAddr); + const client2 = new Client(testRPCAddr); + await client1.activate(); + await client2.activate(); + + await client1.attach(doc1, { isRealtimeSync: false }); + await client2.attach(doc2, { isRealtimeSync: false }); + + doc1.update((root) => { + root.shape = { color: 'black' }; + }); + doc2.update((root) => { + root.shape = { color: 'yellow' }; + }); + await client1.sync(); + await client2.sync(); + await client1.sync(); + assert.equal(doc1.toSortedJSON(), '{"shape":{"color":"yellow"}}'); + assert.equal(doc2.toSortedJSON(), '{"shape":{"color":"yellow"}}'); + + doc2.update((root) => { + root.shape.color = 'red'; + }); + await client2.sync(); + await client1.sync(); + await client2.sync(); + assert.equal(doc1.toSortedJSON(), '{"shape":{"color":"red"}}'); + assert.equal(doc2.toSortedJSON(), '{"shape":{"color":"red"}}'); + + // TODO(chacha912): fix error that occurs when undoing + doc1.history.undo(); }); - await client2.sync(); - await client1.sync(); - await client2.sync(); - assert.equal(doc1.toSortedJSON(), '{"shape":{"color":"red"}}'); - assert.equal(doc2.toSortedJSON(), '{"shape":{"color":"red"}}'); - - // TODO(chacha912): fix error that occurs when undoing - doc1.history.undo(); }); }); From ee3c113d2c6ef888b9067df9395dde980613960c Mon Sep 17 00:00:00 2001 From: Yourim Cha <yourim.cha@navercorp.com> Date: Mon, 6 Nov 2023 16:11:04 +0900 Subject: [PATCH 17/38] Modify to raise explicit error when `executedAt` of operation is absent --- src/document/operation/operation.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/document/operation/operation.ts b/src/document/operation/operation.ts index ba5f394a7..1e9d46749 100644 --- a/src/document/operation/operation.ts +++ b/src/document/operation/operation.ts @@ -193,10 +193,14 @@ export abstract class Operation { /** * `getExecutedAt` returns execution time of this operation. */ - // TODO(Hyemmie): Corner cases need to be considered: undo/redo operations' - // `executedAt` could be undefined until they are executed. public getExecutedAt(): TimeTicket { - return this.executedAt!; + // NOTE(chacha912): When an operation is in the undo/redo stack, + // it doesn't have an executedAt yet. The executedAt is set when + // the operation is executed through undo or redo. + if (!this.executedAt) { + throw new Error(`executedAt has not been set yet`); + } + return this.executedAt; } /** From 24111e0e82a25cc5638c654d25f7d91db2b69ffa Mon Sep 17 00:00:00 2001 From: Yourim Cha <yourim.cha@navercorp.com> Date: Mon, 6 Nov 2023 16:47:18 +0900 Subject: [PATCH 18/38] Specify OpSource --- src/document/change/change.ts | 2 +- src/document/document.ts | 14 +++++++++++--- src/document/operation/operation.ts | 2 ++ 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/document/change/change.ts b/src/document/change/change.ts index ab43117d7..de3446e89 100644 --- a/src/document/change/change.ts +++ b/src/document/change/change.ts @@ -138,7 +138,7 @@ export class Change<P extends Indexable> { public execute( root: CRDTRoot, presences: Map<ActorID, P>, - origin?: OpSource, + origin: OpSource, ): { opInfos: Array<OperationInfo>; reverseOps: Array<HistoryOperation<P>>; diff --git a/src/document/document.ts b/src/document/document.ts index 1e1614a57..34822ec2b 100644 --- a/src/document/document.ts +++ b/src/document/document.ts @@ -526,7 +526,11 @@ export class Document<T, P extends Indexable = Indexable> { } const change = context.getChange(); - const { opInfos, reverseOps } = change.execute(this.root, this.presences); + const { opInfos, reverseOps } = change.execute( + this.root, + this.presences, + OpSource.Local, + ); const reversePresence = context.getReversePresence(); if (reversePresence) { reverseOps.push({ @@ -1048,7 +1052,7 @@ export class Document<T, P extends Indexable = Indexable> { this.ensureClone(); for (const change of changes) { - change.execute(this.clone!.root, this.clone!.presences); + change.execute(this.clone!.root, this.clone!.presences, OpSource.Remote); let changeInfo: ChangeInfo | undefined; let presenceEvent: @@ -1094,7 +1098,11 @@ export class Document<T, P extends Indexable = Indexable> { } } - const { opInfos } = change.execute(this.root, this.presences); + const { opInfos } = change.execute( + this.root, + this.presences, + OpSource.Remote, + ); if (change.hasOperations() && opInfos.length > 0) { changeInfo = { actor: actorID, diff --git a/src/document/operation/operation.ts b/src/document/operation/operation.ts index 1e9d46749..689f03787 100644 --- a/src/document/operation/operation.ts +++ b/src/document/operation/operation.ts @@ -21,6 +21,8 @@ import { CRDTRoot } from '@yorkie-js-sdk/src/document/crdt/root'; import { Indexable } from '@yorkie-js-sdk/src/document/document'; export enum OpSource { + Local = 'local', + Remote = 'remote', UndoRedo = 'undoredo', } From d44fb4aceb27207c892dcd4c8773dd95a3064546 Mon Sep 17 00:00:00 2001 From: Yourim Cha <yourim.cha@navercorp.com> Date: Mon, 6 Nov 2023 17:42:27 +0900 Subject: [PATCH 19/38] Modify to handle cases where operation is not applied only when element is deleted --- src/document/operation/remove_operation.ts | 14 +++----------- src/document/operation/set_operation.ts | 13 ++++--------- test/integration/object_test.ts | 10 +++++++--- 3 files changed, 14 insertions(+), 23 deletions(-) diff --git a/src/document/operation/remove_operation.ts b/src/document/operation/remove_operation.ts index 71b980716..98c06f661 100644 --- a/src/document/operation/remove_operation.ts +++ b/src/document/operation/remove_operation.ts @@ -62,15 +62,6 @@ export class RemoveOperation extends Operation { source?: OpSource, ): ExecutionResult | undefined { const parentObject = root.findByCreatedAt(this.getParentCreatedAt()); - - // NOTE(chacha912): Handle cases where operations cannot be executed during undo and redo. - if ( - source === OpSource.UndoRedo && - (!parentObject || parentObject.getRemovedAt()) - ) { - return; - } - if (!parentObject) { logger.fatal(`fail to find ${this.getParentCreatedAt()}`); } @@ -79,10 +70,11 @@ export class RemoveOperation extends Operation { } const obj = parentObject as CRDTContainer; const key = obj.subPathOf(this.createdAt); - // NOTE(chacha912): Handle cases where operations cannot be executed during undo and redo. + // NOTE(chacha912): Handle cases where operation cannot be executed during undo and redo. if ( source === OpSource.UndoRedo && - ((obj instanceof CRDTObject && !obj.has(key!)) || + (obj.getRemovedAt() || + (obj instanceof CRDTObject && !obj.has(key!)) || (obj instanceof CRDTArray && !obj.get(this.createdAt))) ) { return; diff --git a/src/document/operation/set_operation.ts b/src/document/operation/set_operation.ts index f55bdf572..0fa772e93 100644 --- a/src/document/operation/set_operation.ts +++ b/src/document/operation/set_operation.ts @@ -65,15 +65,6 @@ export class SetOperation extends Operation { source?: OpSource, ): ExecutionResult | undefined { const parentObject = root.findByCreatedAt(this.getParentCreatedAt()); - - // NOTE(chacha912): Handle cases where operations cannot be executed during undo and redo. - if ( - source === OpSource.UndoRedo && - (!parentObject || parentObject.getRemovedAt()) - ) { - return; - } - if (!parentObject) { logger.fatal(`fail to find ${this.getParentCreatedAt()}`); } @@ -81,6 +72,10 @@ export class SetOperation extends Operation { logger.fatal(`fail to execute, only object can execute set`); } const obj = parentObject as CRDTObject; + // NOTE(chacha912): Handle cases where operation cannot be executed during undo and redo. + if (source === OpSource.UndoRedo && obj.getRemovedAt()) { + return; + } const previousValue = obj.get(this.key); const reverseOp = this.getReverseOperation(previousValue); diff --git a/test/integration/object_test.ts b/test/integration/object_test.ts index f6afad0e8..3c92a032d 100644 --- a/test/integration/object_test.ts +++ b/test/integration/object_test.ts @@ -399,8 +399,8 @@ describe('Object', function () { shape?: { color: string }; } const docKey = toDocKey(`${task.name}-${new Date().getTime()}`); - const doc1 = new Document<TestDoc>(docKey, { disableGC: true }); - const doc2 = new Document<TestDoc>(docKey, { disableGC: true }); + const doc1 = new Document<TestDoc>(docKey); + const doc2 = new Document<TestDoc>(docKey); const client1 = new Client(testRPCAddr); const client2 = new Client(testRPCAddr); @@ -582,8 +582,12 @@ describe('Object', function () { assert.equal(doc1.toSortedJSON(), '{"shape":{"color":"red"}}'); assert.equal(doc2.toSortedJSON(), '{"shape":{"color":"red"}}'); - // TODO(chacha912): fix error that occurs when undoing + // TODO(chacha912): fix error that occurs when undoing and make the test pass doc1.history.undo(); + assert.equal(doc1.toSortedJSON(), '{}'); + await client1.sync(); + await client2.sync(); + assert.equal(doc2.toSortedJSON(), '{}'); }); }); }); From 13668c334e5c8b522acede635869296369b45973 Mon Sep 17 00:00:00 2001 From: Yourim Cha <yourim.cha@navercorp.com> Date: Mon, 6 Nov 2023 17:42:49 +0900 Subject: [PATCH 20/38] Handle error in cases where executedAt is missing --- public/devtool/object.js | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/public/devtool/object.js b/public/devtool/object.js index d2f6d3641..b8e58502e 100644 --- a/public/devtool/object.js +++ b/public/devtool/object.js @@ -83,16 +83,22 @@ const objectDevtool = ( op.value, )}</span>`; } - let id = op.getExecutedAt()?.toTestString(); - if (id) { - id = `<span class="timeticket">${id}</span>`; - } const opType = op.toTestString().split('.')[1]; - return `<span class="op"> - <span class="type ${opType.toLowerCase()}">${opType}</span> - ${id ? id : ''} - ${op.toTestString()} - </span>`; + try { + const id = op.getExecutedAt()?.toTestString(); + return ` + <span class="op"> + <span class="type ${opType.toLowerCase()}">${opType}</span> + ${`<span class="timeticket">${id}</span>`}${op.toTestString()} + </span>`; + } catch (e) { + // operation in the undo/redo stack does not yet have "executedAt" set. + return ` + <span class="op"> + <span class="type ${opType.toLowerCase()}">${opType}</span> + ${op.toTestString()} + </span>`; + } }) .join('\n'); return ` From 9ecca352ed7648bd437aec24e135f18315ec1513 Mon Sep 17 00:00:00 2001 From: Yourim Cha <yourim.cha@navercorp.com> Date: Mon, 6 Nov 2023 18:06:01 +0900 Subject: [PATCH 21/38] Add test to verify reverse operation of object.set and remove --- test/integration/object_test.ts | 44 +++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/test/integration/object_test.ts b/test/integration/object_test.ts index 3c92a032d..3598426f3 100644 --- a/test/integration/object_test.ts +++ b/test/integration/object_test.ts @@ -7,6 +7,7 @@ import { toDocKey, testRPCAddr, } from '@yorkie-js-sdk/test/integration/integration_helper'; +import { toStringHistoryOp } from '@yorkie-js-sdk/test/helper/helper'; import { YorkieError } from '@yorkie-js-sdk/src/util/error'; describe('Object', function () { @@ -206,6 +207,49 @@ describe('Object', function () { }); describe('Undo/Redo', function () { + it('can get proper reverse operations', function () { + const doc = new Document<{ + shape: { color: string }; + }>('test-doc'); + + doc.update((root) => { + root.shape = { color: 'black' }; + }); + assert.equal(doc.toSortedJSON(), '{"shape":{"color":"black"}}'); + assert.deepEqual( + doc.getUndoStackForTest().at(-1)?.map(toStringHistoryOp), + ['1:00:1.REMOVE.1:00:2', '0:00:0.REMOVE.1:00:1'], + ); + + doc.history.undo(); + assert.equal(doc.toSortedJSON(), `{}`); + assert.deepEqual( + doc.getRedoStackForTest().at(-1)?.map(toStringHistoryOp), + ['0:00:0.SET.shape={"color":"black"}', '1:00:1.SET.color="black"'], + ); + + doc.history.redo(); + assert.equal(doc.toSortedJSON(), '{"shape":{"color":"black"}}'); + doc.update((root) => { + root.shape.color = 'red'; + }); + assert.equal(doc.toSortedJSON(), '{"shape":{"color":"red"}}'); + assert.deepEqual( + doc.getUndoStackForTest().at(-1)?.map(toStringHistoryOp), + ['1:00:1.SET.color="black"'], + ); + + doc.history.undo(); + assert.equal(doc.toSortedJSON(), '{"shape":{"color":"black"}}'); + assert.deepEqual( + doc.getRedoStackForTest().at(-1)?.map(toStringHistoryOp), + ['1:00:1.SET.color="red"'], + ); + + doc.history.redo(); + assert.equal(doc.toSortedJSON(), '{"shape":{"color":"red"}}'); + }); + it('Can undo/redo work properly for simple object', function () { const doc = new Document<{ a: number; From 08efd9c179a86fe60c4fe5f9d91a7fbfc8b3ea52 Mon Sep 17 00:00:00 2001 From: Yourim Cha <yourim.cha@navercorp.com> Date: Mon, 6 Nov 2023 23:28:58 +0900 Subject: [PATCH 22/38] Fix test to use disableGC option --- src/document/crdt/element_rht.ts | 11 +++++++++++ src/document/crdt/object.ts | 8 ++++++++ src/document/operation/remove_operation.ts | 4 ++-- test/integration/object_test.ts | 14 ++++++++------ 4 files changed, 29 insertions(+), 8 deletions(-) diff --git a/src/document/crdt/element_rht.ts b/src/document/crdt/element_rht.ts index efcce5e8d..6865c790b 100644 --- a/src/document/crdt/element_rht.ts +++ b/src/document/crdt/element_rht.ts @@ -184,6 +184,17 @@ export class ElementRHT { return !node.isRemoved(); } + /** + * `hasByCreatedAt` returns whether the element exists of the given createdAt or not. + */ + public hasByCreatedAt(createdAt: TimeTicket): boolean { + const node = this.nodeMapByCreatedAt.get(createdAt.toIDString()); + if (node == null) { + return false; + } + return !node.isRemoved(); + } + /** * `get` returns the value of the given key. */ diff --git a/src/document/crdt/object.ts b/src/document/crdt/object.ts index d642ffc0d..82af95fed 100644 --- a/src/document/crdt/object.ts +++ b/src/document/crdt/object.ts @@ -98,6 +98,14 @@ export class CRDTObject extends CRDTContainer { return this.memberNodes.has(key); } + /** + * `hasByCreatedAt` returns whether the element exists of the given + * createdAt or not. + */ + public hasByCreatedAt(createdAt: TimeTicket): boolean { + return this.memberNodes.hasByCreatedAt(createdAt); + } + /** * `toJSON` returns the JSON encoding of this object. */ diff --git a/src/document/operation/remove_operation.ts b/src/document/operation/remove_operation.ts index 98c06f661..f7297ab17 100644 --- a/src/document/operation/remove_operation.ts +++ b/src/document/operation/remove_operation.ts @@ -69,16 +69,16 @@ export class RemoveOperation extends Operation { logger.fatal(`only object and array can execute remove: ${parentObject}`); } const obj = parentObject as CRDTContainer; - const key = obj.subPathOf(this.createdAt); // NOTE(chacha912): Handle cases where operation cannot be executed during undo and redo. if ( source === OpSource.UndoRedo && (obj.getRemovedAt() || - (obj instanceof CRDTObject && !obj.has(key!)) || + (obj instanceof CRDTObject && !obj.hasByCreatedAt(this.createdAt)) || (obj instanceof CRDTArray && !obj.get(this.createdAt))) ) { return; } + const key = obj.subPathOf(this.createdAt); const reverseOp = this.getReverseOperation(obj); const elem = obj.delete(this.createdAt, this.getExecutedAt()); diff --git a/test/integration/object_test.ts b/test/integration/object_test.ts index 3598426f3..9ac3e309c 100644 --- a/test/integration/object_test.ts +++ b/test/integration/object_test.ts @@ -587,15 +587,16 @@ describe('Object', function () { assert.equal(doc2.toSortedJSON(), '{"color":"red"}'); }); - it.skip(`Should handle case of reverse operations referencing already garbage-collected elements`, async function ({ + it(`Should handle case of reverse operations referencing already garbage-collected elements`, async function ({ task, }) { interface TestDoc { shape: { color: string }; } const docKey = toDocKey(`${task.name}-${new Date().getTime()}`); - const doc1 = new Document<TestDoc>(docKey); - const doc2 = new Document<TestDoc>(docKey); + // TODO(chacha912): Remove the disableGC option + const doc1 = new Document<TestDoc>(docKey, { disableGC: true }); + const doc2 = new Document<TestDoc>(docKey, { disableGC: true }); const client1 = new Client(testRPCAddr); const client2 = new Client(testRPCAddr); @@ -626,12 +627,13 @@ describe('Object', function () { assert.equal(doc1.toSortedJSON(), '{"shape":{"color":"red"}}'); assert.equal(doc2.toSortedJSON(), '{"shape":{"color":"red"}}'); - // TODO(chacha912): fix error that occurs when undoing and make the test pass doc1.history.undo(); - assert.equal(doc1.toSortedJSON(), '{}'); + assert.equal(doc1.toSortedJSON(), '{"shape":{"color":"red"}}'); + assert.equal(doc1.getRedoStackForTest().length, 0); + assert.equal(doc1.history.canRedo(), false); await client1.sync(); await client2.sync(); - assert.equal(doc2.toSortedJSON(), '{}'); + assert.equal(doc2.toSortedJSON(), '{"shape":{"color":"red"}}'); }); }); }); From 3bdee7513d2e58687bc7b7d32dc7e47d537a963d Mon Sep 17 00:00:00 2001 From: Youngteac Hong <susukang98@gmail.com> Date: Tue, 7 Nov 2023 14:52:04 +0900 Subject: [PATCH 23/38] Update whiteboard example - Display undo/redo stack count - Prevent script errors when clicking the undo/redo buttons --- public/devtool/object.js | 2 ++ public/whiteboard.html | 13 ++++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/public/devtool/object.js b/public/devtool/object.js index b8e58502e..464204f61 100644 --- a/public/devtool/object.js +++ b/public/devtool/object.js @@ -59,6 +59,7 @@ const objectDevtool = ( const displayUndoOps = () => { undoOpsHolder.innerHTML = ` + <div>${doc.getUndoStackForTest().length}</div> <div class="devtool-ops-holder"> ${renderOpsHolder(doc.getUndoStackForTest(), 'undo')} </div> @@ -67,6 +68,7 @@ const objectDevtool = ( const displayRedoOps = () => { redoOpsHolder.innerHTML = ` + <div>${doc.getRedoStackForTest().length}</div> <div class="devtool-ops-holder"> ${renderOpsHolder(doc.getRedoStackForTest(), 'redo')} </div> diff --git a/public/whiteboard.html b/public/whiteboard.html index 0b9ffa047..d8cbc644c 100644 --- a/public/whiteboard.html +++ b/public/whiteboard.html @@ -123,6 +123,8 @@ <h2>redo stack</h2> shapeElement.setAttribute('data-id', shape.id); shapesHolder.appendChild(shapeElement); + + // TODO(hackerwins): Use event delegation. shapeElement.addEventListener('pointerdown', (e) => { e.stopPropagation(); // TODO(chacha912): Let's use `doc.history.pause()` once it's implemented. @@ -289,12 +291,21 @@ <h2>redo stack</h2> setColor(e, doc); }; undoTool.onclick = () => { - doc.history.undo(); // TODO(chacha912): Display log here before implementing `doc.subscribe('history')` + if (!doc.history.canUndo()) { + return; + } + + doc.history.undo(); displayUndoOps(); displayRedoOps(); }; redoTool.onclick = () => { + // TODO(chacha912): Display log here before implementing `doc.subscribe('history')` + if (!doc.history.canRedo()) { + return; + } + doc.history.redo(); displayUndoOps(); displayRedoOps(); From 116fac27a6b27205db5586a8325ed2c11138a58e Mon Sep 17 00:00:00 2001 From: Youngteac Hong <susukang98@gmail.com> Date: Tue, 7 Nov 2023 14:56:31 +0900 Subject: [PATCH 24/38] Rename Element.getLastExecutedAt with Element.getPositionedAt - Remove duplication between Array.getPositionedAt and Element.getLastExecutedAt - Add more comments --- src/api/converter.ts | 2 +- src/document/change/change.ts | 9 +++++++-- src/document/crdt/element.ts | 9 ++++----- src/document/crdt/element_rht.ts | 2 +- src/document/crdt/object.ts | 2 +- src/document/crdt/rga_tree_list.ts | 10 +++------- src/document/operation/operation.ts | 7 ++++++- 7 files changed, 23 insertions(+), 18 deletions(-) diff --git a/src/api/converter.ts b/src/api/converter.ts index 40fae9f41..7e5b07a92 100644 --- a/src/api/converter.ts +++ b/src/api/converter.ts @@ -1198,7 +1198,7 @@ function fromObject(pbObject: PbJSONElement.JSONObject): CRDTObject { const rht = new ElementRHT(); for (const pbRHTNode of pbObject.getNodesList()) { const value = fromElement(pbRHTNode.getElement()!); - rht.set(pbRHTNode.getKey(), value, value.getLastExcutedAt()); + rht.set(pbRHTNode.getKey(), value, value.getPositionedAt()); } const obj = new CRDTObject(fromTimeTicket(pbObject.getCreatedAt())!, rht); diff --git a/src/document/change/change.ts b/src/document/change/change.ts index de3446e89..505eb3827 100644 --- a/src/document/change/change.ts +++ b/src/document/change/change.ts @@ -138,7 +138,7 @@ export class Change<P extends Indexable> { public execute( root: CRDTRoot, presences: Map<ActorID, P>, - origin: OpSource, + source: OpSource, ): { opInfos: Array<OperationInfo>; reverseOps: Array<HistoryOperation<P>>; @@ -149,10 +149,15 @@ export class Change<P extends Indexable> { root.opsForTest.push(this.operations); } for (const operation of this.operations) { - const executionResult = operation.execute(root, origin); + const executionResult = operation.execute(root, source); + // NOTE(hackerwins): If the element was removed while executing undo/redo, + // the operation is not executed and executionResult is undefined. if (!executionResult) continue; const { opInfos, reverseOp } = executionResult; changeOpInfos.push(...opInfos); + + // TODO(hackerwins): This condition should be removed after implementing + // all reverse operations. if (reverseOp) { reverseOps.unshift(reverseOp); } diff --git a/src/document/crdt/element.ts b/src/document/crdt/element.ts index f98b252ea..26c8c3d1d 100644 --- a/src/document/crdt/element.ts +++ b/src/document/crdt/element.ts @@ -59,11 +59,10 @@ export abstract class CRDTElement { } /** - * `getLastExcutedAt` returns the most recent time at which - * an operation was executed on this element. - * TODO(chacha912): we can replace RGATreeListNode.getPositionedAt with this method. + * `getPositionedAt` returns the time of this element when it was positioned + * in the document by undo/redo or move operation. */ - public getLastExcutedAt(): TimeTicket { + public getPositionedAt(): TimeTicket { if (!this.movedAt) { return this.createdAt; } @@ -96,7 +95,7 @@ export abstract class CRDTElement { public remove(removedAt?: TimeTicket): boolean { if ( removedAt && - removedAt.after(this.getLastExcutedAt()) && + removedAt.after(this.getPositionedAt()) && (!this.removedAt || removedAt.after(this.removedAt)) ) { this.removedAt = removedAt; diff --git a/src/document/crdt/element_rht.ts b/src/document/crdt/element_rht.ts index 6865c790b..5443aeeca 100644 --- a/src/document/crdt/element_rht.ts +++ b/src/document/crdt/element_rht.ts @@ -102,7 +102,7 @@ export class ElementRHT { const newNode = ElementRHTNode.of(key, value); this.nodeMapByCreatedAt.set(value.getCreatedAt().toIDString(), newNode); - if (node == null || executedAt.after(node.getValue().getLastExcutedAt())) { + if (node == null || executedAt.after(node.getValue().getPositionedAt())) { this.nodeMapByKey.set(key, newNode); value.setMovedAt(executedAt); } diff --git a/src/document/crdt/object.ts b/src/document/crdt/object.ts index 82af95fed..3b1875da6 100644 --- a/src/document/crdt/object.ts +++ b/src/document/crdt/object.ts @@ -189,7 +189,7 @@ export class CRDTObject extends CRDTContainer { clone.memberNodes.set( node.getStrKey(), node.getValue().deepcopy(), - this.getLastExcutedAt(), + this.getPositionedAt(), ); } clone.remove(this.getRemovedAt()); diff --git a/src/document/crdt/rga_tree_list.ts b/src/document/crdt/rga_tree_list.ts index 3a1add11b..42debd229 100644 --- a/src/document/crdt/rga_tree_list.ts +++ b/src/document/crdt/rga_tree_list.ts @@ -69,15 +69,11 @@ class RGATreeListNode extends SplayNode<CRDTElement> { } /** - * `getPositionedAt` returns time this element was positioned in the array. + * `getPositionedAt` returns the time of this element when it was positioned + * in the array. */ public getPositionedAt(): TimeTicket { - const movedAt = this.value.getMovedAt(); - if (movedAt) { - return movedAt; - } - - return this.value.getCreatedAt(); + return this.value.getPositionedAt(); } /** diff --git a/src/document/operation/operation.ts b/src/document/operation/operation.ts index 689f03787..32bb44ec1 100644 --- a/src/document/operation/operation.ts +++ b/src/document/operation/operation.ts @@ -20,6 +20,11 @@ import { TreeNode } from '@yorkie-js-sdk/src/document/crdt/tree'; import { CRDTRoot } from '@yorkie-js-sdk/src/document/crdt/root'; import { Indexable } from '@yorkie-js-sdk/src/document/document'; +/** + * `OpSource` represents the source of the operation. It is used to handle + * corner cases in the operations created by undo/redo allow the removed + * elements when executing them. + */ export enum OpSource { Local = 'local', Remote = 'remote', @@ -236,6 +241,6 @@ export abstract class Operation { */ public abstract execute( root: CRDTRoot, - origin?: OpSource, + source?: OpSource, ): ExecutionResult | undefined; } From 4db1fc07c23cf5128f6af9c9523cdbe52d18b9a3 Mon Sep 17 00:00:00 2001 From: Yourim Cha <yourim.cha@navercorp.com> Date: Tue, 7 Nov 2023 15:39:53 +0900 Subject: [PATCH 25/38] Update history stack log when presence is changed in whiteboard example --- public/devtool/object.js | 2 -- public/whiteboard.css | 4 +++ public/whiteboard.html | 63 ++++++++++++++++++++++++---------------- 3 files changed, 42 insertions(+), 27 deletions(-) diff --git a/public/devtool/object.js b/public/devtool/object.js index 464204f61..b8e58502e 100644 --- a/public/devtool/object.js +++ b/public/devtool/object.js @@ -59,7 +59,6 @@ const objectDevtool = ( const displayUndoOps = () => { undoOpsHolder.innerHTML = ` - <div>${doc.getUndoStackForTest().length}</div> <div class="devtool-ops-holder"> ${renderOpsHolder(doc.getUndoStackForTest(), 'undo')} </div> @@ -68,7 +67,6 @@ const objectDevtool = ( const displayRedoOps = () => { redoOpsHolder.innerHTML = ` - <div>${doc.getRedoStackForTest().length}</div> <div class="devtool-ops-holder"> ${renderOpsHolder(doc.getRedoStackForTest(), 'redo')} </div> diff --git a/public/whiteboard.css b/public/whiteboard.css index 579c64c32..e844c878a 100644 --- a/public/whiteboard.css +++ b/public/whiteboard.css @@ -28,6 +28,10 @@ .log-holder-wrap h2 { margin: 10px 0; } +.log-holder-wrap .stack-count { + font-weight: normal; + font-size: 14px; +} .dev-log-wrap .network { margin-top: 10px; } diff --git a/public/whiteboard.html b/public/whiteboard.html index d8cbc644c..e7c8532fc 100644 --- a/public/whiteboard.html +++ b/public/whiteboard.html @@ -51,11 +51,21 @@ <h2>operations</h2> </div> <div class="log-holders"> <div class="log-holder-wrap"> - <h2>undo stack</h2> + <h2> + undo stack + <span class="stack-count"> + [ total: <span id="undo-stack-count"></span> ]</span + > + </h2> <div id="undo-holder" class="log-holder"></div> </div> <div class="log-holder-wrap"> - <h2>redo stack</h2> + <h2> + redo stack + <span class="stack-count"> + [ total: <span id="redo-stack-count"></span> ]</span + > + </h2> <div id="redo-holder" class="log-holder"></div> </div> </div> @@ -72,6 +82,8 @@ <h2>redo stack</h2> const opsHolder = document.getElementById('ops-holder'); const undoHolder = document.getElementById('undo-holder'); const redoHolder = document.getElementById('redo-holder'); + const undoStackCount = document.getElementById('undo-stack-count'); + const redoStackCount = document.getElementById('redo-stack-count'); const undoTool = document.querySelector('.toolbar .undo'); const redoTool = document.querySelector('.toolbar .redo'); const deleteTool = document.querySelector('.toolbar .delete'); @@ -228,23 +240,6 @@ <h2>redo stack</h2> const doc = new yorkie.Document('whiteboard', { disableGC: true, }); - doc.subscribe('presence', (event) => { - if (event.type === 'presence-changed') { - renderShapes(doc, myClientID); - } - displayOnlineClients(doc.getPresences(), client.getID()); - }); - doc.subscribe('my-presence', (event) => { - if (event.type === 'presence-changed') { - if (event.value.presence?.selectedShape) { - selectionTool.style.display = 'flex'; - } else { - selectionTool.style.display = 'none'; - } - } - }); - await client.attach(doc); - // setup devtool const { displayRootObject, @@ -263,11 +258,33 @@ <h2>redo stack</h2> displayOps(); displayUndoOps(); displayRedoOps(); + undoStackCount.textContent = doc.getUndoStackForTest().length; + redoStackCount.textContent = doc.getRedoStackForTest().length; }; + doc.subscribe('presence', (event) => { + if (event.type === 'presence-changed') { + renderShapes(doc, myClientID); + } + displayOnlineClients(doc.getPresences(), client.getID()); + }); + doc.subscribe('my-presence', (event) => { + if (event.type === 'presence-changed') { + if (event.value.presence?.selectedShape) { + selectionTool.style.display = 'flex'; + } else { + selectionTool.style.display = 'none'; + } + } + // TODO(chacha912): Display log here before implementing `doc.subscribe('history')` + displayLog(); + }); + await client.attach(doc); + // 03. subscribe to document changes doc.subscribe((event) => { renderShapes(doc, myClientID); + // TODO(chacha912): Display log here before implementing `doc.subscribe('history')` displayLog(); }); @@ -291,24 +308,20 @@ <h2>redo stack</h2> setColor(e, doc); }; undoTool.onclick = () => { - // TODO(chacha912): Display log here before implementing `doc.subscribe('history')` + // TODO(chacha912): Activate button only when the undo stack is not empty. if (!doc.history.canUndo()) { return; } doc.history.undo(); - displayUndoOps(); - displayRedoOps(); }; redoTool.onclick = () => { - // TODO(chacha912): Display log here before implementing `doc.subscribe('history')` + // TODO(chacha912): Activate button only when the redo stack is not empty. if (!doc.history.canRedo()) { return; } doc.history.redo(); - displayUndoOps(); - displayRedoOps(); }; shapesHolder.addEventListener('pointerup', (e) => { onCanvasPointerUp(e, doc); From 7e3f11804ad77a161c0f86f8d7c33081eaf02d30 Mon Sep 17 00:00:00 2001 From: Yourim Cha <yourim.cha@navercorp.com> Date: Tue, 7 Nov 2023 16:58:06 +0900 Subject: [PATCH 26/38] Use event delegation --- public/whiteboard.html | 35 ++++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/public/whiteboard.html b/public/whiteboard.html index e7c8532fc..e75a773a3 100644 --- a/public/whiteboard.html +++ b/public/whiteboard.html @@ -135,28 +135,30 @@ <h2> shapeElement.setAttribute('data-id', shape.id); shapesHolder.appendChild(shapeElement); - - // TODO(hackerwins): Use event delegation. - shapeElement.addEventListener('pointerdown', (e) => { - e.stopPropagation(); - // TODO(chacha912): Let's use `doc.history.pause()` once it's implemented. - // Currently, every movement is being saved in the undo stack. - const currentSelectedShapeId = doc.getMyPresence().selectedShape; - isDragging = true; - if (currentSelectedShapeId === shape.id) return; - doc.update((root, presence) => { - presence.set({ selectedShape: shape.id }, { addToHistory: true }); - }); - }); } } + const onCanvasPointerDown = (e, doc) => { + e.stopPropagation(); + + const shape = e.target.closest('.shape'); + if (!shape) return; + const id = shape.dataset.id; + // TODO(chacha912): Let's use `doc.history.pause()` once it's implemented. + // Currently, every movement is being saved in the undo stack. + const currentSelectedShapeId = doc.getMyPresence().selectedShape; + isDragging = true; + if (currentSelectedShapeId === id) return; + doc.update((root, presence) => { + presence.set({ selectedShape: id }, { addToHistory: true }); + }); + }; const onCanvasPointerUp = (e, doc) => { - if (!isDragging) { + const selectedShapeId = doc.getMyPresence().selectedShape; + if (!isDragging && selectedShapeId) { doc.update((root, presence) => { presence.set({ selectedShape: null }, { addToHistory: true }); }); } - const selectedShapeId = doc.getMyPresence().selectedShape; isDragging = false; }; const onCanvasPointerMove = (e, doc) => { @@ -323,6 +325,9 @@ <h2> doc.history.redo(); }; + shapesHolder.addEventListener('pointerdown', (e) => { + onCanvasPointerDown(e, doc); + }); shapesHolder.addEventListener('pointerup', (e) => { onCanvasPointerUp(e, doc); }); From df8ca4f7c337c1b647d5461335a5891e4b5c6487 Mon Sep 17 00:00:00 2001 From: Yourim Cha <yourim.cha@navercorp.com> Date: Tue, 7 Nov 2023 17:03:12 +0900 Subject: [PATCH 27/38] Rename Id to ID --- public/whiteboard.html | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/public/whiteboard.html b/public/whiteboard.html index e75a773a3..cbdce7178 100644 --- a/public/whiteboard.html +++ b/public/whiteboard.html @@ -145,16 +145,16 @@ <h2> const id = shape.dataset.id; // TODO(chacha912): Let's use `doc.history.pause()` once it's implemented. // Currently, every movement is being saved in the undo stack. - const currentSelectedShapeId = doc.getMyPresence().selectedShape; + const currentSelectedShapeID = doc.getMyPresence().selectedShape; isDragging = true; - if (currentSelectedShapeId === id) return; + if (currentSelectedShapeID === id) return; doc.update((root, presence) => { presence.set({ selectedShape: id }, { addToHistory: true }); }); }; const onCanvasPointerUp = (e, doc) => { - const selectedShapeId = doc.getMyPresence().selectedShape; - if (!isDragging && selectedShapeId) { + const selectedShapeID = doc.getMyPresence().selectedShape; + if (!isDragging && selectedShapeID) { doc.update((root, presence) => { presence.set({ selectedShape: null }, { addToHistory: true }); }); @@ -163,11 +163,11 @@ <h2> }; const onCanvasPointerMove = (e, doc) => { if (!isDragging) return; - const selectedShapeId = doc.getMyPresence().selectedShape; + const selectedShapeID = doc.getMyPresence().selectedShape; doc.update((root, presence) => { const selectedShape = root.shapes.find( - (shape) => shape.id === selectedShapeId, + (shape) => shape.id === selectedShapeID, ); if (!selectedShape) return; @@ -183,25 +183,25 @@ <h2> }; const insertRectangle = (doc) => { - const shapeId = Date.now().toString(); + const shapeID = Date.now().toString(); doc.update((root, presence) => { root.shapes.push({ - id: shapeId, + id: shapeID, point: { x: getRandomInt(300), y: getRandomInt(300), }, color: getRandomColor(), }); - presence.set({ selectedShape: shapeId }, { addToHistory: true }); + presence.set({ selectedShape: shapeID }, { addToHistory: true }); }); }; const deleteRectangle = (doc) => { - const selectedShapeId = doc.getMyPresence().selectedShape; - if (!selectedShapeId) return; + const selectedShapeID = doc.getMyPresence().selectedShape; + if (!selectedShapeID) return; doc.update((root, presence) => { const selectedShape = root.shapes.find( - (shape) => shape.id === selectedShapeId, + (shape) => shape.id === selectedShapeID, ); if (!selectedShape) return; root.shapes.deleteByID(selectedShape.getID()); @@ -210,10 +210,10 @@ <h2> }; const setColor = (e, doc) => { if (!e.target.dataset.color) return; - const selectedShapeId = doc.getMyPresence().selectedShape; + const selectedShapeID = doc.getMyPresence().selectedShape; doc.update((root) => { const selectedShape = root.shapes.find( - (shape) => shape.id === selectedShapeId, + (shape) => shape.id === selectedShapeID, ); if (!selectedShape) return; selectedShape.color = e.target.dataset.color; From f47a2231a5f489090d4ed92417c9d60962f1c577 Mon Sep 17 00:00:00 2001 From: Yourim Cha <yourim.cha@navercorp.com> Date: Wed, 8 Nov 2023 12:08:25 +0900 Subject: [PATCH 28/38] Specify return type for the `toJSForTest()` instead of using any --- public/devtool/object.js | 8 ++++---- src/document/crdt/array.ts | 11 +++++++---- src/document/crdt/counter.ts | 4 +++- src/document/crdt/element.ts | 3 ++- src/document/crdt/object.ts | 9 ++++++--- src/document/crdt/primitive.ts | 4 +++- src/document/crdt/text.ts | 4 +++- src/document/crdt/tree.ts | 4 +++- src/types/devtools_element.ts | 36 ++++++++++++++++++++++++++++++++++ 9 files changed, 67 insertions(+), 16 deletions(-) create mode 100644 src/types/devtools_element.ts diff --git a/public/devtool/object.js b/public/devtool/object.js index b8e58502e..994786f23 100644 --- a/public/devtool/object.js +++ b/public/devtool/object.js @@ -6,16 +6,16 @@ const objectDevtool = ( const rootObj = doc.getRoot().toJSForTest(); rootHolder.innerHTML = ` <div class="devtool-root-holder"> - ${renderCRDTObject(rootObj)} + ${renderContainer(rootObj)} </div> `; }; - const renderCRDTObject = ({ key, value, id }) => { + const renderContainer = ({ key, value, id }) => { const valueHTML = Object.values(value) .map((v) => { - return Object.prototype.toString.call(v.value) === '[object Object]' - ? renderCRDTObject(v) + return v.type === 'YORKIE_OBJECT' || v.type === 'YORKIE_ARRAY' + ? renderContainer(v) : renderValue(v); }) .join(''); diff --git a/src/document/crdt/array.ts b/src/document/crdt/array.ts index cb83b6479..16bf55fa9 100644 --- a/src/document/crdt/array.ts +++ b/src/document/crdt/array.ts @@ -20,6 +20,7 @@ import { CRDTElement, } from '@yorkie-js-sdk/src/document/crdt/element'; import { RGATreeList } from '@yorkie-js-sdk/src/document/crdt/rga_tree_list'; +import * as DevTools from '@yorkie-js-sdk/src/types/devtools_element'; /** * `CRDTArray` represents an array data type containing `CRDTElement`s. @@ -209,19 +210,21 @@ export class CRDTArray extends CRDTContainer { /** * `toJSForTest` returns value with meta data for testing. */ - public toJSForTest(): { id: string; value: any } { - const values = {} as any; + public toJSForTest(): DevTools.JSONElement { + const values: DevTools.ContainerValue = {}; for (let i = 0; i < this.length; i++) { - const { id, value } = this.getByIndex(i)!.toJSForTest(); + const { id, value, type } = this.getByIndex(i)!.toJSForTest(); values[i] = { - key: i, + key: String(i), id, value, + type, }; } return { id: this.getCreatedAt().toTestString(), value: values, + type: 'YORKIE_ARRAY', }; } diff --git a/src/document/crdt/counter.ts b/src/document/crdt/counter.ts index 8732292f4..ce0592740 100644 --- a/src/document/crdt/counter.ts +++ b/src/document/crdt/counter.ts @@ -23,6 +23,7 @@ import { PrimitiveType, } from '@yorkie-js-sdk/src/document/crdt/primitive'; import { removeDecimal } from '@yorkie-js-sdk/src/util/number'; +import type * as DevTools from '@yorkie-js-sdk/src/types/devtools_element'; export enum CounterType { IntegerCnt, @@ -122,10 +123,11 @@ export class CRDTCounter extends CRDTElement { /** * `toJSForTest` returns value with meta data for testing. */ - public toJSForTest(): { id: string; value: any } { + public toJSForTest(): DevTools.JSONElement { return { id: this.getCreatedAt().toTestString(), value: this.value, + type: 'YORKIE_COUNTER', }; } diff --git a/src/document/crdt/element.ts b/src/document/crdt/element.ts index 26c8c3d1d..11b659e22 100644 --- a/src/document/crdt/element.ts +++ b/src/document/crdt/element.ts @@ -15,6 +15,7 @@ */ import { TimeTicket } from '@yorkie-js-sdk/src/document/time/ticket'; +import type * as DevTools from '@yorkie-js-sdk/src/types/devtools_element'; /** * `CRDTElement` represents an element that has `TimeTicket`s. @@ -114,7 +115,7 @@ export abstract class CRDTElement { abstract toJSON(): string; abstract toSortedJSON(): string; - abstract toJSForTest(): { id: string; value: any }; + abstract toJSForTest(): DevTools.JSONElement; abstract deepcopy(): CRDTElement; } diff --git a/src/document/crdt/object.ts b/src/document/crdt/object.ts index 3b1875da6..031141461 100644 --- a/src/document/crdt/object.ts +++ b/src/document/crdt/object.ts @@ -20,6 +20,7 @@ import { CRDTElement, } from '@yorkie-js-sdk/src/document/crdt/element'; import { ElementRHT } from '@yorkie-js-sdk/src/document/crdt/element_rht'; +import type * as DevTools from '@yorkie-js-sdk/src/types/devtools_element'; /** * `CRDTObject` represents an object data type, but unlike regular JSON, @@ -127,19 +128,21 @@ export class CRDTObject extends CRDTContainer { /** * `toJSForTest` returns value with meta data for testing. */ - public toJSForTest(): { id: string; value: any } { - const values = {} as any; + public toJSForTest(): DevTools.JSONElement { + const values: DevTools.ContainerValue = {}; for (const [key, elem] of this) { - const { id, value } = elem.toJSForTest(); + const { id, value, type } = elem.toJSForTest(); values[key] = { key, id, value, + type, }; } return { id: this.getCreatedAt().toTestString(), value: values, + type: 'YORKIE_OBJECT', }; } diff --git a/src/document/crdt/primitive.ts b/src/document/crdt/primitive.ts index 4981cde81..3dfccaad9 100644 --- a/src/document/crdt/primitive.ts +++ b/src/document/crdt/primitive.ts @@ -19,6 +19,7 @@ import { Code, YorkieError } from '@yorkie-js-sdk/src/util/error'; import { TimeTicket } from '@yorkie-js-sdk/src/document/time/ticket'; import { CRDTElement } from '@yorkie-js-sdk/src/document/crdt/element'; import { escapeString } from '@yorkie-js-sdk/src/document/json/strings'; +import type * as DevTools from '@yorkie-js-sdk/src/types/devtools_element'; export enum PrimitiveType { Null, @@ -120,10 +121,11 @@ export class Primitive extends CRDTElement { /** * `toJSForTest` returns value with meta data for testing. */ - public toJSForTest(): { id: string; value: any } { + public toJSForTest(): DevTools.JSONElement { return { id: this.getCreatedAt().toTestString(), value: this.value, + type: 'YORKIE_PRIMITIVE', }; } diff --git a/src/document/crdt/text.ts b/src/document/crdt/text.ts index baec43942..56b966254 100644 --- a/src/document/crdt/text.ts +++ b/src/document/crdt/text.ts @@ -30,6 +30,7 @@ import { } from '@yorkie-js-sdk/src/document/crdt/rga_tree_split'; import { escapeString } from '@yorkie-js-sdk/src/document/json/strings'; import { parseObjectValues } from '@yorkie-js-sdk/src/util/object'; +import type * as DevTools from '@yorkie-js-sdk/src/types/devtools_element'; /** * `TextChangeType` is the type of TextChange. @@ -353,10 +354,11 @@ export class CRDTText<A extends Indexable = Indexable> extends CRDTGCElement { /** * `toJSForTest` returns value with meta data for testing. */ - public toJSForTest(): { id: string; value: any } { + public toJSForTest(): DevTools.JSONElement { return { id: this.getCreatedAt().toTestString(), value: JSON.parse(this.toJSON()), + type: 'YORKIE_TEXT', }; } diff --git a/src/document/crdt/tree.ts b/src/document/crdt/tree.ts index 20766289b..3a8d64e99 100644 --- a/src/document/crdt/tree.ts +++ b/src/document/crdt/tree.ts @@ -38,6 +38,7 @@ import type { TreeNodeType, } from '@yorkie-js-sdk/src/util/index_tree'; import { Indexable } from '@yorkie-js-sdk/src/document/document'; +import type * as DevTools from '@yorkie-js-sdk/src/types/devtools_element'; export type TreeNode = TextNode | ElementNode; @@ -969,10 +970,11 @@ export class CRDTTree extends CRDTGCElement { /** * `toJSForTest` returns value with meta data for testing. */ - public toJSForTest(): { id: string; value: any } { + public toJSForTest(): DevTools.JSONElement { return { id: this.getCreatedAt().toTestString(), value: JSON.parse(this.toJSON()), + type: 'YORKIE_TREE', }; } diff --git a/src/types/devtools_element.ts b/src/types/devtools_element.ts new file mode 100644 index 000000000..01dd25563 --- /dev/null +++ b/src/types/devtools_element.ts @@ -0,0 +1,36 @@ +import type { PrimitiveValue } from '@yorkie-js-sdk/src/document/crdt/primitive'; +import { CounterValue } from '@yorkie-js-sdk/src/document/crdt/counter'; + +export type Json = + | string + | number + | boolean + // eslint-disable-next-line @typescript-eslint/ban-types + | null + | { [key: string]: Json } + | Array<Json>; + +export type ContainerValue = { + [key: string]: JSONElement; +}; + +type ElementValue = + | PrimitiveValue + | CounterValue + | ContainerValue // Array | Object + | Json; // Text | Tree + +export type ElementType = + | 'YORKIE_PRIMITIVE' + | 'YORKIE_COUNTER' + | 'YORKIE_OBJECT' + | 'YORKIE_ARRAY' + | 'YORKIE_TEXT' + | 'YORKIE_TREE'; + +export type JSONElement = { + id: string; + key?: string; + value: ElementValue; + type: ElementType; +}; From f61ac95410024981cf4e65dcc546d5406bcc3236 Mon Sep 17 00:00:00 2001 From: Yourim Cha <yourim.cha@navercorp.com> Date: Thu, 9 Nov 2023 00:32:49 +0900 Subject: [PATCH 29/38] Add comments for event handling during undo/redo --- src/document/document.ts | 60 +++++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 32 deletions(-) diff --git a/src/document/document.ts b/src/document/document.ts index 34822ec2b..db21fd7da 100644 --- a/src/document/document.ts +++ b/src/document/document.ts @@ -546,7 +546,7 @@ export class Document<T, P extends Indexable = Indexable> { this.internalHistory.clearRedo(); this.changeID = change.getID(); - if (change.hasOperations() && opInfos.length > 0) { + if (change.hasOperations()) { this.publish({ type: DocEventType.LocalChange, value: { @@ -1098,16 +1098,16 @@ export class Document<T, P extends Indexable = Indexable> { } } - const { opInfos } = change.execute( + const executionResult = change.execute( this.root, this.presences, OpSource.Remote, ); - if (change.hasOperations() && opInfos.length > 0) { + if (change.hasOperations()) { changeInfo = { actor: actorID, message: change.getMessage() || '', - operations: opInfos, + operations: executionResult.opInfos, }; } @@ -1277,6 +1277,7 @@ export class Document<T, P extends Indexable = Indexable> { // apply undo operation in the context to generate a change for (const undoOp of undoOps) { if (!(undoOp instanceof Operation)) { + // apply presence change to the context const presence = new Presence<P>( context, deepcopy(this.clone!.presences.get(this.changeID.getActorID()!)!), @@ -1290,17 +1291,7 @@ export class Document<T, P extends Indexable = Indexable> { } const change = context.getChange(); - const cloneExecutionResult = change.execute( - this.clone!.root, - this.clone!.presences, - OpSource.UndoRedo, - ); - if ( - !change.hasPresenceChange() && - cloneExecutionResult.opInfos.length === 0 - ) { - return; - } + change.execute(this.clone!.root, this.clone!.presences, OpSource.UndoRedo); const { opInfos, reverseOps } = change.execute( this.root, @@ -1318,11 +1309,18 @@ export class Document<T, P extends Indexable = Indexable> { this.internalHistory.pushRedo(reverseOps); } - this.localChanges.push(change); - this.changeID = change.getID(); + // NOTE(chacha912): When there is no applied operation or presence + // during undo/redo, skip propagating change remotely. + if (change.hasPresenceChange() || opInfos.length > 0) { + this.localChanges.push(change); + } + this.changeID = change.getID(); const actorID = this.changeID.getActorID()!; - if (change.hasOperations() && opInfos.length > 0) { + // NOTE(chacha912): Although operations are included in the change, they + // may not be executable (e.g., when the target element has been deleted). + // So we check opInfos, which represent the actually executed operations. + if (opInfos.length > 0) { this.publish({ type: DocEventType.LocalChange, value: { @@ -1367,6 +1365,7 @@ export class Document<T, P extends Indexable = Indexable> { // apply redo operation in the context to generate a change for (const redoOp of redoOps) { if (!(redoOp instanceof Operation)) { + // apply presence change to the context const presence = new Presence<P>( context, deepcopy(this.clone!.presences.get(this.changeID.getActorID()!)!), @@ -1380,17 +1379,7 @@ export class Document<T, P extends Indexable = Indexable> { } const change = context.getChange(); - const cloneExecutionResult = change.execute( - this.clone!.root, - this.clone!.presences, - OpSource.UndoRedo, - ); - if ( - !change.hasPresenceChange() && - cloneExecutionResult.opInfos.length === 0 - ) { - return; - } + change.execute(this.clone!.root, this.clone!.presences, OpSource.UndoRedo); const { opInfos, reverseOps } = change.execute( this.root, @@ -1408,11 +1397,18 @@ export class Document<T, P extends Indexable = Indexable> { this.internalHistory.pushUndo(reverseOps); } - this.localChanges.push(change); - this.changeID = change.getID(); + // NOTE(chacha912): When there is no applied operation or presence + // during undo/redo, skip propagating change remotely. + if (change.hasPresenceChange() || opInfos.length > 0) { + this.localChanges.push(change); + } + this.changeID = change.getID(); const actorID = this.changeID.getActorID()!; - if (change.hasOperations() && opInfos.length > 0) { + // NOTE(chacha912): Although operations are included in the change, they + // may not be executable (e.g., when the target element has been deleted). + // So we check opInfos, which represent the actually executed operations. + if (opInfos.length > 0) { this.publish({ type: DocEventType.LocalChange, value: { From b2ff8ce2ec64d0f88500a285b5faa6dae5569a3d Mon Sep 17 00:00:00 2001 From: Yourim Cha <yourim.cha@navercorp.com> Date: Thu, 9 Nov 2023 00:34:06 +0900 Subject: [PATCH 30/38] Make source non-optional in operation.execute() --- src/document/operation/operation.ts | 2 +- src/document/operation/remove_operation.ts | 2 +- src/document/operation/set_operation.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/document/operation/operation.ts b/src/document/operation/operation.ts index 32bb44ec1..3b246645b 100644 --- a/src/document/operation/operation.ts +++ b/src/document/operation/operation.ts @@ -241,6 +241,6 @@ export abstract class Operation { */ public abstract execute( root: CRDTRoot, - source?: OpSource, + source: OpSource, ): ExecutionResult | undefined; } diff --git a/src/document/operation/remove_operation.ts b/src/document/operation/remove_operation.ts index f7297ab17..27556fb3c 100644 --- a/src/document/operation/remove_operation.ts +++ b/src/document/operation/remove_operation.ts @@ -59,7 +59,7 @@ export class RemoveOperation extends Operation { */ public execute( root: CRDTRoot, - source?: OpSource, + source: OpSource, ): ExecutionResult | undefined { const parentObject = root.findByCreatedAt(this.getParentCreatedAt()); if (!parentObject) { diff --git a/src/document/operation/set_operation.ts b/src/document/operation/set_operation.ts index 0fa772e93..f8a836d1a 100644 --- a/src/document/operation/set_operation.ts +++ b/src/document/operation/set_operation.ts @@ -62,7 +62,7 @@ export class SetOperation extends Operation { */ public execute( root: CRDTRoot, - source?: OpSource, + source: OpSource, ): ExecutionResult | undefined { const parentObject = root.findByCreatedAt(this.getParentCreatedAt()); if (!parentObject) { From d83fdfb1062a0fd7a4f783254e35079a9a84a5a0 Mon Sep 17 00:00:00 2001 From: Yourim Cha <yourim.cha@navercorp.com> Date: Thu, 9 Nov 2023 00:34:26 +0900 Subject: [PATCH 31/38] Add comments --- src/document/operation/set_operation.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/document/operation/set_operation.ts b/src/document/operation/set_operation.ts index f8a836d1a..f7ff6ab47 100644 --- a/src/document/operation/set_operation.ts +++ b/src/document/operation/set_operation.ts @@ -110,6 +110,7 @@ export class SetOperation extends Operation { if (value !== undefined && !value.isRemoved()) { // TODO(chacha912): When the value is an object, // it always sets as an empty object from the remote. + // (Refer to https://github.com/yorkie-team/yorkie/issues/663) reverseOp = SetOperation.create( this.key, value.deepcopy(), From 030acb892e7baa0357064a3b6a4efc1123b5cc08 Mon Sep 17 00:00:00 2001 From: Yourim Cha <yourim.cha@navercorp.com> Date: Thu, 9 Nov 2023 01:14:02 +0900 Subject: [PATCH 32/38] Modify shape movement to use presence --- public/devtool/object.css | 1 + public/whiteboard.html | 63 +++++++++++++++++++++++++-------------- 2 files changed, 41 insertions(+), 23 deletions(-) diff --git a/public/devtool/object.css b/public/devtool/object.css index 352120217..b0ed7be3a 100644 --- a/public/devtool/object.css +++ b/public/devtool/object.css @@ -101,6 +101,7 @@ display: flex; margin-bottom: 3px; border-top: 1px solid #ddd; + word-break: break-all; } .devtool-ops-holder label { position: relative; diff --git a/public/whiteboard.html b/public/whiteboard.html index cbdce7178..b5cc9465c 100644 --- a/public/whiteboard.html +++ b/public/whiteboard.html @@ -114,17 +114,23 @@ <h2> const shapes = doc.getRoot().shapes; if (!shapes) return; shapesHolder.innerHTML = ''; + const { selectedShapeID, movingShapePoint } = doc.getMyPresence(); for (const shape of shapes) { const shapeElement = document.createElement('div'); shapeElement.className = 'shape'; - shapeElement.style.transform = `translate(${shape.point.x}px, ${shape.point.y}px)`; + let point = shape.point; + if (shape.id === selectedShapeID && movingShapePoint) { + point = movingShapePoint; + } + shapeElement.style.transform = `translate(${point.x}px, ${point.y}px)`; shapeElement.style.backgroundColor = shape.color; - const selectedByMe = doc.getMyPresence().selectedShape === shape.id; + const selectedByMe = doc.getMyPresence().selectedShapeID === shape.id; const selectedByOthers = doc .getPresences() .some( ({ clientID, presence }) => - presence.selectedShape === shape.id && clientID !== myClientID, + presence.selectedShapeID === shape.id && + clientID !== myClientID, ); const selectionColor = selectedByMe ? 'blue' @@ -143,27 +149,40 @@ <h2> const shape = e.target.closest('.shape'); if (!shape) return; const id = shape.dataset.id; - // TODO(chacha912): Let's use `doc.history.pause()` once it's implemented. - // Currently, every movement is being saved in the undo stack. - const currentSelectedShapeID = doc.getMyPresence().selectedShape; + const currentSelectedShapeID = doc.getMyPresence().selectedShapeID; isDragging = true; if (currentSelectedShapeID === id) return; doc.update((root, presence) => { - presence.set({ selectedShape: id }, { addToHistory: true }); + presence.set({ selectedShapeID: id }, { addToHistory: true }); }); }; const onCanvasPointerUp = (e, doc) => { - const selectedShapeID = doc.getMyPresence().selectedShape; + const { selectedShapeID, movingShapePoint } = doc.getMyPresence(); + // When the shape movement is completed, update from presence to the document + if (isDragging && selectedShapeID && movingShapePoint) { + doc.update((root, presence) => { + const selectedShape = root.shapes.find( + (shape) => shape.id === selectedShapeID, + ); + selectedShape.point.x = movingShapePoint.x; + selectedShape.point.y = movingShapePoint.y; + presence.set({ movingShapePoint: null }); + }); + } + // If the user clicks on the canvas, deselect the shape. if (!isDragging && selectedShapeID) { doc.update((root, presence) => { - presence.set({ selectedShape: null }, { addToHistory: true }); + presence.set( + { selectedShapeID: null, movingShapePoint: null }, + { addToHistory: true }, + ); }); } isDragging = false; }; const onCanvasPointerMove = (e, doc) => { if (!isDragging) return; - const selectedShapeID = doc.getMyPresence().selectedShape; + const selectedShapeID = doc.getMyPresence().selectedShapeID; doc.update((root, presence) => { const selectedShape = root.shapes.find( @@ -171,14 +190,12 @@ <h2> ); if (!selectedShape) return; - selectedShape.point.x = e.clientX - rectangleSize / 2; - selectedShape.point.y = e.clientY - rectangleSize / 2; - // TODO(chacha912): we can change the code as follows after - // modifying to allow nested objects in the set operation. - // selectedShape.point = { - // x: e.clientX - rectangleSize / 2, - // y: e.clientY - rectangleSize / 2, - // }; + presence.set({ + movingShapePoint: { + x: e.clientX - rectangleSize / 2, + y: e.clientY - rectangleSize / 2, + }, + }); }); }; @@ -193,11 +210,11 @@ <h2> }, color: getRandomColor(), }); - presence.set({ selectedShape: shapeID }, { addToHistory: true }); + presence.set({ selectedShapeID: shapeID }, { addToHistory: true }); }); }; const deleteRectangle = (doc) => { - const selectedShapeID = doc.getMyPresence().selectedShape; + const selectedShapeID = doc.getMyPresence().selectedShapeID; if (!selectedShapeID) return; doc.update((root, presence) => { const selectedShape = root.shapes.find( @@ -205,12 +222,12 @@ <h2> ); if (!selectedShape) return; root.shapes.deleteByID(selectedShape.getID()); - presence.set({ selectedShape: null }); + presence.set({ selectedShapeID: null }); }); }; const setColor = (e, doc) => { if (!e.target.dataset.color) return; - const selectedShapeID = doc.getMyPresence().selectedShape; + const selectedShapeID = doc.getMyPresence().selectedShapeID; doc.update((root) => { const selectedShape = root.shapes.find( (shape) => shape.id === selectedShapeID, @@ -272,7 +289,7 @@ <h2> }); doc.subscribe('my-presence', (event) => { if (event.type === 'presence-changed') { - if (event.value.presence?.selectedShape) { + if (event.value.presence?.selectedShapeID) { selectionTool.style.display = 'flex'; } else { selectionTool.style.display = 'none'; From 77513bf0c1cb7a62ae65465e7f8fdc6732ce4212 Mon Sep 17 00:00:00 2001 From: Yourim Cha <yourim.cha@navercorp.com> Date: Thu, 9 Nov 2023 09:48:36 +0900 Subject: [PATCH 33/38] Add `get` and `getByID` methods to the CRDTContainer --- public/whiteboard.html | 7 ++++++ src/document/crdt/array.ts | 20 ++++++++-------- src/document/crdt/element.ts | 4 ++++ src/document/crdt/element_rht.ts | 18 ++++++-------- src/document/crdt/object.ts | 27 ++++++++++++++------- src/document/crdt/rga_tree_list.ts | 11 +++------ src/document/json/array.ts | 22 ++++++----------- src/document/operation/remove_operation.ts | 16 ++++++------- test/integration/object_test.ts | 28 ++++++++++++++++++++++ test/unit/document/crdt/root_test.ts | 2 +- 10 files changed, 93 insertions(+), 62 deletions(-) diff --git a/public/whiteboard.html b/public/whiteboard.html index b5cc9465c..84addece8 100644 --- a/public/whiteboard.html +++ b/public/whiteboard.html @@ -116,6 +116,9 @@ <h2> shapesHolder.innerHTML = ''; const { selectedShapeID, movingShapePoint } = doc.getMyPresence(); for (const shape of shapes) { + // TODO(chacha912): Remove the following condition after implementing the reverse + // operation for array.add. Currently, the shape is left as an empty object. + if (!shape.id) continue; const shapeElement = document.createElement('div'); shapeElement.className = 'shape'; let point = shape.point; @@ -333,6 +336,8 @@ <h2> } doc.history.undo(); + // TODO(chacha912): Display log here before implementing `doc.subscribe('history')` + displayLog(); }; redoTool.onclick = () => { // TODO(chacha912): Activate button only when the redo stack is not empty. @@ -341,6 +346,8 @@ <h2> } doc.history.redo(); + // TODO(chacha912): Display log here before implementing `doc.subscribe('history')` + displayLog(); }; shapesHolder.addEventListener('pointerdown', (e) => { onCanvasPointerDown(e, doc); diff --git a/src/document/crdt/array.ts b/src/document/crdt/array.ts index 16bf55fa9..31c7266ae 100644 --- a/src/document/crdt/array.ts +++ b/src/document/crdt/array.ts @@ -76,23 +76,23 @@ export class CRDTArray extends CRDTContainer { } /** - * `get` returns the element of the given createAt. + * `get` returns the element of the given index. */ - public get(createdAt: TimeTicket): CRDTElement | undefined { - const node = this.elements.get(createdAt); - if (!node || node.isRemoved()) { + public get(index: number): CRDTElement | undefined { + const node = this.elements.getByIndex(index); + if (!node) { return; } - return node; + return node.getValue(); } /** - * `getByIndex` returns the element of the given index. + * `getByID` returns the element of the given createAt. */ - public getByIndex(index: number): CRDTElement | undefined { - const node = this.elements.getByIndex(index); - if (!node) { + public getByID(createdAt: TimeTicket): CRDTElement | undefined { + const node = this.elements.getByID(createdAt); + if (!node || node.isRemoved()) { return; } @@ -213,7 +213,7 @@ export class CRDTArray extends CRDTContainer { public toJSForTest(): DevTools.JSONElement { const values: DevTools.ContainerValue = {}; for (let i = 0; i < this.length; i++) { - const { id, value, type } = this.getByIndex(i)!.toJSForTest(); + const { id, value, type } = this.get(i)!.toJSForTest(); values[i] = { key: String(i), id, diff --git a/src/document/crdt/element.ts b/src/document/crdt/element.ts index 11b659e22..8615ec089 100644 --- a/src/document/crdt/element.ts +++ b/src/document/crdt/element.ts @@ -140,6 +140,10 @@ export abstract class CRDTContainer extends CRDTElement { abstract getDescendants( callback: (elem: CRDTElement, parent: CRDTContainer) => boolean, ): void; + + abstract get(keyOrIndex: string | number): CRDTElement | undefined; + + abstract getByID(createdAt: TimeTicket): CRDTElement | undefined; } /** diff --git a/src/document/crdt/element_rht.ts b/src/document/crdt/element_rht.ts index 5443aeeca..d9d3bdadd 100644 --- a/src/document/crdt/element_rht.ts +++ b/src/document/crdt/element_rht.ts @@ -185,26 +185,22 @@ export class ElementRHT { } /** - * `hasByCreatedAt` returns whether the element exists of the given createdAt or not. + * `getByID` returns the node of the given createdAt. */ - public hasByCreatedAt(createdAt: TimeTicket): boolean { - const node = this.nodeMapByCreatedAt.get(createdAt.toIDString()); - if (node == null) { - return false; - } - return !node.isRemoved(); + public getByID(createdAt: TimeTicket): ElementRHTNode | undefined { + return this.nodeMapByCreatedAt.get(createdAt.toIDString()); } /** - * `get` returns the value of the given key. + * `get` returns the node of the given key. */ - public get(key: string): CRDTElement | undefined { + public get(key: string): ElementRHTNode | undefined { const node = this.nodeMapByKey.get(key); - if (node == null) { + if (!node || node.isRemoved()) { return; } - return node.getValue(); + return node; } // eslint-disable-next-line jsdoc/require-jsdoc diff --git a/src/document/crdt/object.ts b/src/document/crdt/object.ts index 031141461..0db25ea7e 100644 --- a/src/document/crdt/object.ts +++ b/src/document/crdt/object.ts @@ -89,22 +89,31 @@ export class CRDTObject extends CRDTContainer { * `get` returns the value of the given key. */ public get(key: string): CRDTElement | undefined { - return this.memberNodes.get(key); + const node = this.memberNodes.get(key); + if (!node) { + return; + } + + return node.getValue(); } /** - * `has` returns whether the element exists of the given key or not. + * `getByID` returns the element of the given createAt. */ - public has(key: string): boolean { - return this.memberNodes.has(key); + public getByID(createdAt: TimeTicket): CRDTElement | undefined { + const node = this.memberNodes.getByID(createdAt); + if (!node || node.isRemoved()) { + return; + } + + return node.getValue(); } /** - * `hasByCreatedAt` returns whether the element exists of the given - * createdAt or not. + * `has` returns whether the element exists of the given key or not. */ - public hasByCreatedAt(createdAt: TimeTicket): boolean { - return this.memberNodes.hasByCreatedAt(createdAt); + public has(key: string): boolean { + return this.memberNodes.has(key); } /** @@ -169,7 +178,7 @@ export class CRDTObject extends CRDTContainer { const json = []; for (const key of keys.sort()) { - const node = this.memberNodes.get(key); + const node = this.memberNodes.get(key)?.getValue(); json.push(`"${key}":${node!.toSortedJSON()}`); } diff --git a/src/document/crdt/rga_tree_list.ts b/src/document/crdt/rga_tree_list.ts index 42debd229..df7bd0854 100644 --- a/src/document/crdt/rga_tree_list.ts +++ b/src/document/crdt/rga_tree_list.ts @@ -259,15 +259,10 @@ export class RGATreeList { } /** - * `get` returns the element of the given creation time. + * `getByID` returns the element of the given creation time. */ - public get(createdAt: TimeTicket): CRDTElement | undefined { - const node = this.nodeMapByCreatedAt.get(createdAt.toIDString()); - if (!node) { - return; - } - - return node.getValue(); + public getByID(createdAt: TimeTicket): RGATreeListNode | undefined { + return this.nodeMapByCreatedAt.get(createdAt.toIDString()); } /** diff --git a/src/document/json/array.ts b/src/document/json/array.ts index 64373b4cd..db384ea95 100644 --- a/src/document/json/array.ts +++ b/src/document/json/array.ts @@ -171,11 +171,11 @@ export class ArrayProxy { }; } else if (method === 'getElementByID') { return (createdAt: TimeTicket): WrappedElement | undefined => { - return toWrappedElement(context, target.get(createdAt)); + return toWrappedElement(context, target.getByID(createdAt)); }; } else if (method === 'getElementByIndex') { return (index: number): WrappedElement | undefined => { - const elem = target.getByIndex(index); + const elem = target.get(index); return toWrappedElement(context, elem); }; } else if (method === 'getLast') { @@ -235,10 +235,7 @@ export class ArrayProxy { ArrayProxy.moveLastInternal(context, target, id); }; } else if (isNumericString(method)) { - return toJSONElement( - context, - target.getByIndex(Number(method as string)), - ); + return toJSONElement(context, target.get(Number(method as string))); } else if (method === 'push') { return (value: any): number => { return ArrayProxy.pushInternal(context, target, value); @@ -583,9 +580,7 @@ export class ArrayProxy { } if (items) { let previousID = - from === 0 - ? target.getHead().getID() - : target.getByIndex(from - 1)!.getID(); + from === 0 ? target.getHead().getID() : target.get(from - 1)!.getID(); for (const item of items) { const newElem = ArrayProxy.insertAfterInternal( context, @@ -627,8 +622,7 @@ export class ArrayProxy { for (let i = from; i < length; i++) { if ( - target.getByIndex(i)?.getID() === - (searchElement as WrappedElement).getID!() + target.get(i)?.getID() === (searchElement as WrappedElement).getID!() ) { return true; } @@ -664,8 +658,7 @@ export class ArrayProxy { for (let i = from; i < length; i++) { if ( - target.getByIndex(i)?.getID() === - (searchElement as WrappedElement).getID!() + target.get(i)?.getID() === (searchElement as WrappedElement).getID!() ) { return i; } @@ -701,8 +694,7 @@ export class ArrayProxy { for (let i = from; i > 0; i--) { if ( - target.getByIndex(i)?.getID() === - (searchElement as WrappedElement).getID!() + target.get(i)?.getID() === (searchElement as WrappedElement).getID!() ) { return i; } diff --git a/src/document/operation/remove_operation.ts b/src/document/operation/remove_operation.ts index 27556fb3c..64910fcfb 100644 --- a/src/document/operation/remove_operation.ts +++ b/src/document/operation/remove_operation.ts @@ -61,27 +61,27 @@ export class RemoveOperation extends Operation { root: CRDTRoot, source: OpSource, ): ExecutionResult | undefined { - const parentObject = root.findByCreatedAt(this.getParentCreatedAt()); + const parentObject = root.findByCreatedAt( + this.getParentCreatedAt(), + ) as CRDTContainer; if (!parentObject) { logger.fatal(`fail to find ${this.getParentCreatedAt()}`); } if (!(parentObject instanceof CRDTContainer)) { logger.fatal(`only object and array can execute remove: ${parentObject}`); } - const obj = parentObject as CRDTContainer; + // NOTE(chacha912): Handle cases where operation cannot be executed during undo and redo. if ( source === OpSource.UndoRedo && - (obj.getRemovedAt() || - (obj instanceof CRDTObject && !obj.hasByCreatedAt(this.createdAt)) || - (obj instanceof CRDTArray && !obj.get(this.createdAt))) + (parentObject.getRemovedAt() || !parentObject.getByID(this.createdAt)) ) { return; } - const key = obj.subPathOf(this.createdAt); - const reverseOp = this.getReverseOperation(obj); + const key = parentObject.subPathOf(this.createdAt); + const reverseOp = this.getReverseOperation(parentObject); - const elem = obj.delete(this.createdAt, this.getExecutedAt()); + const elem = parentObject.delete(this.createdAt, this.getExecutedAt()); root.registerRemovedElement(elem); const opInfos: Array<OperationInfo> = diff --git a/test/integration/object_test.ts b/test/integration/object_test.ts index 9ac3e309c..666f61658 100644 --- a/test/integration/object_test.ts +++ b/test/integration/object_test.ts @@ -206,6 +206,34 @@ describe('Object', function () { }, task.name); }); + it('Returns undefined when looking up an element that doesnt exist', function () { + const doc = new Document<{ + shapes: { + [key: string]: { color: string; point: { x: number; y: number } }; + }; + }>('test-doc'); + assert.equal('{}', doc.toSortedJSON()); + + doc.update((root) => { + root.shapes = { + circle: { color: 'black', point: { x: 0, y: 0 } }, + }; + }); + assert.equal( + '{"shapes":{"circle":{"color":"black","point":{"x":0,"y":0}}}}', + doc.toSortedJSON(), + ); + + doc.update((root) => { + delete root.shapes.circle; + }); + assert.equal('{"shapes":{}}', doc.toSortedJSON()); + assert.isUndefined(doc.getRoot().shapes.circle); + assert.isUndefined(doc.getRoot().shapes.circle?.color); + assert.isUndefined(doc.getRoot().shapes.circle?.point); + assert.isUndefined(doc.getRoot().shapes.circle?.point?.x); + }); + describe('Undo/Redo', function () { it('can get proper reverse operations', function () { const doc = new Document<{ diff --git a/test/unit/document/crdt/root_test.ts b/test/unit/document/crdt/root_test.ts index 348655991..6e94776a0 100644 --- a/test/unit/document/crdt/root_test.ts +++ b/test/unit/document/crdt/root_test.ts @@ -93,7 +93,7 @@ describe('ROOT', function () { assert.equal(1, arrJs1?.[1]); assert.equal(2, arrJs1?.[2]); - const targetElement = arr.getByIndex(1)!; + const targetElement = arr.get(1)!; arr.delete(targetElement.getCreatedAt(), change.issueTimeTicket()); root.registerRemovedElement(targetElement); assert.equal('[0,2]', arr.toJSON()); From ba622ceb5dce96fe873b9fe309f00a52a095644b Mon Sep 17 00:00:00 2001 From: Yourim Cha <yourim.cha@navercorp.com> Date: Fri, 10 Nov 2023 10:23:20 +0900 Subject: [PATCH 34/38] Unify the event triggering condition to opInfos --- src/document/document.ts | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/src/document/document.ts b/src/document/document.ts index db21fd7da..2e465ce87 100644 --- a/src/document/document.ts +++ b/src/document/document.ts @@ -546,7 +546,8 @@ export class Document<T, P extends Indexable = Indexable> { this.internalHistory.clearRedo(); this.changeID = change.getID(); - if (change.hasOperations()) { + // 03. Publish the document change event. + if (opInfos.length > 0) { this.publish({ type: DocEventType.LocalChange, value: { @@ -1054,7 +1055,6 @@ export class Document<T, P extends Indexable = Indexable> { for (const change of changes) { change.execute(this.clone!.root, this.clone!.presences, OpSource.Remote); - let changeInfo: ChangeInfo | undefined; let presenceEvent: | WatchedEvent<P> | UnwatchedEvent<P> @@ -1098,27 +1098,24 @@ export class Document<T, P extends Indexable = Indexable> { } } - const executionResult = change.execute( + const { opInfos } = change.execute( this.root, this.presences, OpSource.Remote, ); - if (change.hasOperations()) { - changeInfo = { - actor: actorID, - message: change.getMessage() || '', - operations: executionResult.opInfos, - }; - } // DocEvent should be emitted synchronously with applying changes. // This is because 3rd party model should be synced with the Document // after RemoteChange event is emitted. If the event is emitted // asynchronously, the model can be changed and breaking consistency. - if (changeInfo) { + if (opInfos.length > 0) { this.publish({ type: DocEventType.RemoteChange, - value: changeInfo, + value: { + actor: actorID, + message: change.getMessage() || '', + operations: opInfos, + }, }); } if (presenceEvent) { From cccd592df29dfea3d236b982edd80c2ec8b03a4d Mon Sep 17 00:00:00 2001 From: Yourim Cha <yourim.cha@navercorp.com> Date: Fri, 10 Nov 2023 10:26:08 +0900 Subject: [PATCH 35/38] Clear redo when a new local operation is applied --- src/document/document.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/document/document.ts b/src/document/document.ts index 2e465ce87..edf43adae 100644 --- a/src/document/document.ts +++ b/src/document/document.ts @@ -543,7 +543,9 @@ export class Document<T, P extends Indexable = Indexable> { if (reverseOps.length > 0) { this.internalHistory.pushUndo(reverseOps); } - this.internalHistory.clearRedo(); + if (opInfos.length > 0) { + this.internalHistory.clearRedo(); + } this.changeID = change.getID(); // 03. Publish the document change event. From 3fac1ca242ac87e9d2fb2e4c49dd87719f0f5f16 Mon Sep 17 00:00:00 2001 From: Yourim Cha <yourim.cha@navercorp.com> Date: Fri, 10 Nov 2023 11:40:10 +0900 Subject: [PATCH 36/38] Update to handle not exposing deleted elements in the JSON layer `getByID` is a method used by the system. In the CRDT layer, it should still return the element even if it has been deleted. --- src/document/crdt/array.ts | 12 ++---------- src/document/crdt/object.ts | 12 ++---------- src/document/json/array.ts | 6 +++++- src/document/operation/remove_operation.ts | 3 ++- 4 files changed, 11 insertions(+), 22 deletions(-) diff --git a/src/document/crdt/array.ts b/src/document/crdt/array.ts index 31c7266ae..f02ecd358 100644 --- a/src/document/crdt/array.ts +++ b/src/document/crdt/array.ts @@ -80,11 +80,7 @@ export class CRDTArray extends CRDTContainer { */ public get(index: number): CRDTElement | undefined { const node = this.elements.getByIndex(index); - if (!node) { - return; - } - - return node.getValue(); + return node?.getValue(); } /** @@ -92,11 +88,7 @@ export class CRDTArray extends CRDTContainer { */ public getByID(createdAt: TimeTicket): CRDTElement | undefined { const node = this.elements.getByID(createdAt); - if (!node || node.isRemoved()) { - return; - } - - return node.getValue(); + return node?.getValue(); } /** diff --git a/src/document/crdt/object.ts b/src/document/crdt/object.ts index 0db25ea7e..8c3403248 100644 --- a/src/document/crdt/object.ts +++ b/src/document/crdt/object.ts @@ -90,11 +90,7 @@ export class CRDTObject extends CRDTContainer { */ public get(key: string): CRDTElement | undefined { const node = this.memberNodes.get(key); - if (!node) { - return; - } - - return node.getValue(); + return node?.getValue(); } /** @@ -102,11 +98,7 @@ export class CRDTObject extends CRDTContainer { */ public getByID(createdAt: TimeTicket): CRDTElement | undefined { const node = this.memberNodes.getByID(createdAt); - if (!node || node.isRemoved()) { - return; - } - - return node.getValue(); + return node?.getValue(); } /** diff --git a/src/document/json/array.ts b/src/document/json/array.ts index db384ea95..b8e45b1d2 100644 --- a/src/document/json/array.ts +++ b/src/document/json/array.ts @@ -171,7 +171,11 @@ export class ArrayProxy { }; } else if (method === 'getElementByID') { return (createdAt: TimeTicket): WrappedElement | undefined => { - return toWrappedElement(context, target.getByID(createdAt)); + const elem = target.getByID(createdAt); + if (!elem || elem.isRemoved()) { + return; + } + return toWrappedElement(context, elem); }; } else if (method === 'getElementByIndex') { return (index: number): WrappedElement | undefined => { diff --git a/src/document/operation/remove_operation.ts b/src/document/operation/remove_operation.ts index 64910fcfb..e8d447cc6 100644 --- a/src/document/operation/remove_operation.ts +++ b/src/document/operation/remove_operation.ts @@ -72,9 +72,10 @@ export class RemoveOperation extends Operation { } // NOTE(chacha912): Handle cases where operation cannot be executed during undo and redo. + const targetElem = parentObject.getByID(this.createdAt); if ( source === OpSource.UndoRedo && - (parentObject.getRemovedAt() || !parentObject.getByID(this.createdAt)) + (parentObject.getRemovedAt() || !targetElem || targetElem.isRemoved()) ) { return; } From 001bc70e35a0ba1141be8b9217784dbccc7b26e0 Mon Sep 17 00:00:00 2001 From: Yourim Cha <yourim.cha@navercorp.com> Date: Fri, 10 Nov 2023 12:01:24 +0900 Subject: [PATCH 37/38] Add comments --- public/whiteboard.html | 2 ++ src/document/document.ts | 4 +++- src/types/devtools_element.ts | 2 ++ 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/public/whiteboard.html b/public/whiteboard.html index 84addece8..ba70d3b63 100644 --- a/public/whiteboard.html +++ b/public/whiteboard.html @@ -122,6 +122,8 @@ <h2> const shapeElement = document.createElement('div'); shapeElement.className = 'shape'; let point = shape.point; + // TODO(chacha912): The movement of the peer's rectangle is not rendered. + // It can be replaced after implementing `history.pause` and `history.resume`. if (shape.id === selectedShapeID && movingShapePoint) { point = movingShapePoint; } diff --git a/src/document/document.ts b/src/document/document.ts index edf43adae..097acc8ca 100644 --- a/src/document/document.ts +++ b/src/document/document.ts @@ -543,6 +543,7 @@ export class Document<T, P extends Indexable = Indexable> { if (reverseOps.length > 0) { this.internalHistory.pushUndo(reverseOps); } + // NOTE(chacha912): Clear redo when a new local operation is applied. if (opInfos.length > 0) { this.internalHistory.clearRedo(); } @@ -1308,8 +1309,9 @@ export class Document<T, P extends Indexable = Indexable> { this.internalHistory.pushRedo(reverseOps); } - // NOTE(chacha912): When there is no applied operation or presence + // TODO(chacha912): When there is no applied operation or presence // during undo/redo, skip propagating change remotely. + // In the database, it may appear as if the client sequence is missing. if (change.hasPresenceChange() || opInfos.length > 0) { this.localChanges.push(change); } diff --git a/src/types/devtools_element.ts b/src/types/devtools_element.ts index 01dd25563..6e87f4d9d 100644 --- a/src/types/devtools_element.ts +++ b/src/types/devtools_element.ts @@ -1,6 +1,8 @@ import type { PrimitiveValue } from '@yorkie-js-sdk/src/document/crdt/primitive'; import { CounterValue } from '@yorkie-js-sdk/src/document/crdt/counter'; +// NOTE(chacha912): Json type is used to represent CRDTText and CRDTTree value. +// In the dev tool, display value as the result of toJSON for CRDTText and CRDTTree. export type Json = | string | number From 79eca1212028f0b4337ba78ec5633fe2bcee0905 Mon Sep 17 00:00:00 2001 From: Youngteac Hong <susukang98@gmail.com> Date: Wed, 15 Nov 2023 12:04:07 +0900 Subject: [PATCH 38/38] Add comments and revise the codes --- src/document/change/context.ts | 2 +- src/document/crdt/array.ts | 6 +++--- src/document/crdt/counter.ts | 4 ++-- src/document/crdt/element.ts | 13 +++++++++++-- src/document/crdt/object.ts | 6 +++--- src/document/crdt/primitive.ts | 4 ++-- src/document/crdt/text.ts | 4 ++-- src/document/crdt/tree.ts | 4 ++-- src/document/operation/increase_operation.ts | 6 +++--- src/document/operation/remove_operation.ts | 6 +++--- src/document/operation/set_operation.ts | 6 +++--- 11 files changed, 35 insertions(+), 26 deletions(-) diff --git a/src/document/change/context.ts b/src/document/change/context.ts index d9f726202..e3234922e 100644 --- a/src/document/change/context.ts +++ b/src/document/change/context.ts @@ -153,7 +153,7 @@ export class ChangeContext<P extends Indexable = Indexable> { } /** - * `getReversePresence` returns the reverse presence of this context. + * `toReversePresence` returns the reverse presence of this context. */ public getReversePresence() { if (this.reversePresenceKeys.size === 0) return undefined; diff --git a/src/document/crdt/array.ts b/src/document/crdt/array.ts index f02ecd358..9ad8513c2 100644 --- a/src/document/crdt/array.ts +++ b/src/document/crdt/array.ts @@ -20,7 +20,7 @@ import { CRDTElement, } from '@yorkie-js-sdk/src/document/crdt/element'; import { RGATreeList } from '@yorkie-js-sdk/src/document/crdt/rga_tree_list'; -import * as DevTools from '@yorkie-js-sdk/src/types/devtools_element'; +import * as Devtools from '@yorkie-js-sdk/src/types/devtools_element'; /** * `CRDTArray` represents an array data type containing `CRDTElement`s. @@ -202,8 +202,8 @@ export class CRDTArray extends CRDTContainer { /** * `toJSForTest` returns value with meta data for testing. */ - public toJSForTest(): DevTools.JSONElement { - const values: DevTools.ContainerValue = {}; + public toJSForTest(): Devtools.JSONElement { + const values: Devtools.ContainerValue = {}; for (let i = 0; i < this.length; i++) { const { id, value, type } = this.get(i)!.toJSForTest(); values[i] = { diff --git a/src/document/crdt/counter.ts b/src/document/crdt/counter.ts index ce0592740..cb33c0fc7 100644 --- a/src/document/crdt/counter.ts +++ b/src/document/crdt/counter.ts @@ -23,7 +23,7 @@ import { PrimitiveType, } from '@yorkie-js-sdk/src/document/crdt/primitive'; import { removeDecimal } from '@yorkie-js-sdk/src/util/number'; -import type * as DevTools from '@yorkie-js-sdk/src/types/devtools_element'; +import type * as Devtools from '@yorkie-js-sdk/src/types/devtools_element'; export enum CounterType { IntegerCnt, @@ -123,7 +123,7 @@ export class CRDTCounter extends CRDTElement { /** * `toJSForTest` returns value with meta data for testing. */ - public toJSForTest(): DevTools.JSONElement { + public toJSForTest(): Devtools.JSONElement { return { id: this.getCreatedAt().toTestString(), value: this.value, diff --git a/src/document/crdt/element.ts b/src/document/crdt/element.ts index 8615ec089..7cd93f7e8 100644 --- a/src/document/crdt/element.ts +++ b/src/document/crdt/element.ts @@ -15,7 +15,7 @@ */ import { TimeTicket } from '@yorkie-js-sdk/src/document/time/ticket'; -import type * as DevTools from '@yorkie-js-sdk/src/types/devtools_element'; +import type * as Devtools from '@yorkie-js-sdk/src/types/devtools_element'; /** * `CRDTElement` represents an element that has `TimeTicket`s. @@ -115,7 +115,7 @@ export abstract class CRDTElement { abstract toJSON(): string; abstract toSortedJSON(): string; - abstract toJSForTest(): DevTools.JSONElement; + abstract toJSForTest(): Devtools.JSONElement; abstract deepcopy(): CRDTElement; } @@ -141,8 +141,17 @@ export abstract class CRDTContainer extends CRDTElement { callback: (elem: CRDTElement, parent: CRDTContainer) => boolean, ): void; + /** + * `get` returns the element of the given key or index. This method is called + * by users. So it should return undefined if the element is removed. + */ abstract get(keyOrIndex: string | number): CRDTElement | undefined; + /** + * `getByID` returns the element of the given creation time. This method is + * called by internal. So it should return the element even if the element is + * removed. + */ abstract getByID(createdAt: TimeTicket): CRDTElement | undefined; } diff --git a/src/document/crdt/object.ts b/src/document/crdt/object.ts index 8c3403248..86b650a61 100644 --- a/src/document/crdt/object.ts +++ b/src/document/crdt/object.ts @@ -20,7 +20,7 @@ import { CRDTElement, } from '@yorkie-js-sdk/src/document/crdt/element'; import { ElementRHT } from '@yorkie-js-sdk/src/document/crdt/element_rht'; -import type * as DevTools from '@yorkie-js-sdk/src/types/devtools_element'; +import type * as Devtools from '@yorkie-js-sdk/src/types/devtools_element'; /** * `CRDTObject` represents an object data type, but unlike regular JSON, @@ -129,8 +129,8 @@ export class CRDTObject extends CRDTContainer { /** * `toJSForTest` returns value with meta data for testing. */ - public toJSForTest(): DevTools.JSONElement { - const values: DevTools.ContainerValue = {}; + public toJSForTest(): Devtools.JSONElement { + const values: Devtools.ContainerValue = {}; for (const [key, elem] of this) { const { id, value, type } = elem.toJSForTest(); values[key] = { diff --git a/src/document/crdt/primitive.ts b/src/document/crdt/primitive.ts index 3dfccaad9..b3c4b440c 100644 --- a/src/document/crdt/primitive.ts +++ b/src/document/crdt/primitive.ts @@ -19,7 +19,7 @@ import { Code, YorkieError } from '@yorkie-js-sdk/src/util/error'; import { TimeTicket } from '@yorkie-js-sdk/src/document/time/ticket'; import { CRDTElement } from '@yorkie-js-sdk/src/document/crdt/element'; import { escapeString } from '@yorkie-js-sdk/src/document/json/strings'; -import type * as DevTools from '@yorkie-js-sdk/src/types/devtools_element'; +import type * as Devtools from '@yorkie-js-sdk/src/types/devtools_element'; export enum PrimitiveType { Null, @@ -121,7 +121,7 @@ export class Primitive extends CRDTElement { /** * `toJSForTest` returns value with meta data for testing. */ - public toJSForTest(): DevTools.JSONElement { + public toJSForTest(): Devtools.JSONElement { return { id: this.getCreatedAt().toTestString(), value: this.value, diff --git a/src/document/crdt/text.ts b/src/document/crdt/text.ts index 56b966254..ee42dd8f3 100644 --- a/src/document/crdt/text.ts +++ b/src/document/crdt/text.ts @@ -30,7 +30,7 @@ import { } from '@yorkie-js-sdk/src/document/crdt/rga_tree_split'; import { escapeString } from '@yorkie-js-sdk/src/document/json/strings'; import { parseObjectValues } from '@yorkie-js-sdk/src/util/object'; -import type * as DevTools from '@yorkie-js-sdk/src/types/devtools_element'; +import type * as Devtools from '@yorkie-js-sdk/src/types/devtools_element'; /** * `TextChangeType` is the type of TextChange. @@ -354,7 +354,7 @@ export class CRDTText<A extends Indexable = Indexable> extends CRDTGCElement { /** * `toJSForTest` returns value with meta data for testing. */ - public toJSForTest(): DevTools.JSONElement { + public toJSForTest(): Devtools.JSONElement { return { id: this.getCreatedAt().toTestString(), value: JSON.parse(this.toJSON()), diff --git a/src/document/crdt/tree.ts b/src/document/crdt/tree.ts index 3a8d64e99..7f5a007c4 100644 --- a/src/document/crdt/tree.ts +++ b/src/document/crdt/tree.ts @@ -38,7 +38,7 @@ import type { TreeNodeType, } from '@yorkie-js-sdk/src/util/index_tree'; import { Indexable } from '@yorkie-js-sdk/src/document/document'; -import type * as DevTools from '@yorkie-js-sdk/src/types/devtools_element'; +import type * as Devtools from '@yorkie-js-sdk/src/types/devtools_element'; export type TreeNode = TextNode | ElementNode; @@ -970,7 +970,7 @@ export class CRDTTree extends CRDTGCElement { /** * `toJSForTest` returns value with meta data for testing. */ - public toJSForTest(): DevTools.JSONElement { + public toJSForTest(): Devtools.JSONElement { return { id: this.getCreatedAt().toTestString(), value: JSON.parse(this.toJSON()), diff --git a/src/document/operation/increase_operation.ts b/src/document/operation/increase_operation.ts index 24ae16a0c..d606f271b 100644 --- a/src/document/operation/increase_operation.ts +++ b/src/document/operation/increase_operation.ts @@ -78,14 +78,14 @@ export class IncreaseOperation extends Operation { value: value.getValue() as number, }, ], - reverseOp: this.getReverseOperation(), + reverseOp: this.toReverseOperation(), }; } /** - * `getReverseOperation` returns the reverse operation of this operation. + * `toReverseOperation` returns the reverse operation of this operation. */ - public getReverseOperation(): Operation { + private toReverseOperation(): Operation { const primitiveValue = this.value.deepcopy() as Primitive; const valueType = primitiveValue.getType(); diff --git a/src/document/operation/remove_operation.ts b/src/document/operation/remove_operation.ts index e8d447cc6..49c41bee3 100644 --- a/src/document/operation/remove_operation.ts +++ b/src/document/operation/remove_operation.ts @@ -80,7 +80,7 @@ export class RemoveOperation extends Operation { return; } const key = parentObject.subPathOf(this.createdAt); - const reverseOp = this.getReverseOperation(parentObject); + const reverseOp = this.toReverseOperation(parentObject); const elem = parentObject.delete(this.createdAt, this.getExecutedAt()); root.registerRemovedElement(elem); @@ -106,9 +106,9 @@ export class RemoveOperation extends Operation { } /** - * `getReverseOperation` returns the reverse operation of this operation. + * `toReverseOperation` returns the reverse operation of this operation. */ - public getReverseOperation( + private toReverseOperation( parentObject: CRDTContainer, ): Operation | undefined { // TODO(Hyemmie): consider CRDTArray diff --git a/src/document/operation/set_operation.ts b/src/document/operation/set_operation.ts index f7ff6ab47..74ae9eb91 100644 --- a/src/document/operation/set_operation.ts +++ b/src/document/operation/set_operation.ts @@ -77,7 +77,7 @@ export class SetOperation extends Operation { return; } const previousValue = obj.get(this.key); - const reverseOp = this.getReverseOperation(previousValue); + const reverseOp = this.toReverseOperation(previousValue); const value = this.value.deepcopy(); const removed = obj.set(this.key, value, this.getExecutedAt()); @@ -99,9 +99,9 @@ export class SetOperation extends Operation { } /** - * `getReverseOperation` returns the reverse operation of this operation. + * `toReverseOperation` returns the reverse operation of this operation. */ - public getReverseOperation(value: CRDTElement | undefined): Operation { + private toReverseOperation(value: CRDTElement | undefined): Operation { let reverseOp: SetOperation | RemoveOperation = RemoveOperation.create( this.getParentCreatedAt(), this.value.getCreatedAt(),