From 859c019ddfe0a35f3dc84dba5b92dd3c10491eb5 Mon Sep 17 00:00:00 2001 From: Yourim Cha <81357083+chacha912@users.noreply.github.com> Date: Wed, 15 Nov 2023 12:12:22 +0900 Subject: [PATCH] Support Undo/Redo for object.set and object.remove operations (#658) 1. Define reverse operations of Object.Set Object.Remove: 1.1 Modified the set operation to compare `executedAt` with `getPositionedAt()` during value setting, addressing issues during undo. 1.2 Set `movedAt` when the value is reassigned during undo. 2. Handle edge cases: When the target element is deleted by other peers Implemented handling to skip execution of reverse operations during undo and redo if the element has been deleted. 3. Add whiteboard example: 3.1. Feature Introduction - Added a whiteboard example for testing undo redo operations. 3.2. Variables Added for Displaying Yorkie Data - Added `CRDTRoot.opsForTest` and `CRDTElement.toJSForTest`. 3.3. Additional Features to Apply (Todo) - Planned features include implementing history pause/resume, subscribing to history changes, and applying reverse operations for `array.add` and `array.remove`. 4. TODOs 4.1. Set nested objects (yorkie-team/yorkie#663) - Modified set operation to handle nested objects. 4.2. When the target element is garbage-collected (yorkie-team/yorkie#664) - Considering elements referenced by operations in the undo/redo stack during garbage collection. --------- Co-authored-by: Youngteac Hong --- public/devtool/object.css | 147 ++++++ public/devtool/object.js | 121 +++++ public/whiteboard.css | 119 +++++ public/whiteboard.html | 384 +++++++++++++++ src/api/converter.ts | 4 +- src/document/change/change.ts | 15 +- src/document/change/context.ts | 2 +- src/document/crdt/array.ts | 46 +- src/document/crdt/counter.ts | 12 + src/document/crdt/element.ts | 29 +- src/document/crdt/element_rht.ts | 33 +- src/document/crdt/object.ts | 49 +- src/document/crdt/primitive.ts | 12 + src/document/crdt/rga_tree_list.ts | 21 +- src/document/crdt/root.ts | 7 + src/document/crdt/text.ts | 12 + src/document/crdt/tree.ts | 12 + src/document/document.ts | 111 +++-- src/document/history.ts | 16 +- src/document/json/array.ts | 37 +- src/document/json/object.ts | 12 +- src/document/operation/increase_operation.ts | 6 +- src/document/operation/operation.ts | 26 +- src/document/operation/remove_operation.ts | 58 ++- src/document/operation/set_operation.ts | 47 +- src/types/devtools_element.ts | 38 ++ test/helper/helper.ts | 12 +- test/integration/counter_test.ts | 18 +- test/integration/document_test.ts | 10 +- test/integration/object_test.ts | 476 ++++++++++++++++++- test/integration/presence_test.ts | 20 +- test/unit/document/crdt/root_test.ts | 25 +- test/unit/document/document_test.ts | 11 + 33 files changed, 1758 insertions(+), 190 deletions(-) create mode 100644 public/devtool/object.css create mode 100644 public/devtool/object.js create mode 100644 public/whiteboard.css create mode 100644 public/whiteboard.html create mode 100644 src/types/devtools_element.ts diff --git a/public/devtool/object.css b/public/devtool/object.css new file mode 100644 index 000000000..b0ed7be3a --- /dev/null +++ b/public/devtool/object.css @@ -0,0 +1,147 @@ +.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; + word-break: break-all; +} +.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..994786f23 --- /dev/null +++ b/public/devtool/object.js @@ -0,0 +1,121 @@ +const objectDevtool = ( + doc, + { rootHolder, opsHolder, undoOpsHolder, redoOpsHolder }, +) => { + const displayRootObject = () => { + const rootObj = doc.getRoot().toJSForTest(); + rootHolder.innerHTML = ` +
+ ${renderContainer(rootObj)} +
+ `; + }; + + const renderContainer = ({ key, value, id }) => { + const valueHTML = Object.values(value) + .map((v) => { + return v.type === 'YORKIE_OBJECT' || v.type === 'YORKIE_ARRAY' + ? renderContainer(v) + : renderValue(v); + }) + .join(''); + if (key === undefined) key = 'root'; + return ` +
+ ${renderKey({ key, id })} +
+ ${valueHTML} +
+
+ `; + }; + + const renderKey = ({ key, id }) => { + return ` + + + `; + }; + + const renderValue = ({ key, value, id }) => { + return ` +
+ ${key} : ${JSON.stringify(value)} + ${id} + +
+ `; + }; + + const displayOps = () => { + opsHolder.innerHTML = ` +
+ ${renderOpsHolder(doc.getOpsForTest(), 'op')} +
+ `; + }; + + const displayUndoOps = () => { + undoOpsHolder.innerHTML = ` +
+ ${renderOpsHolder(doc.getUndoStackForTest(), 'undo')} +
+ `; + }; + + const displayRedoOps = () => { + redoOpsHolder.innerHTML = ` +
+ ${renderOpsHolder(doc.getRedoStackForTest(), 'redo')} +
+ `; + }; + + const renderOpsHolder = (changes, idPrefix) => { + return changes + .map((ops, i) => { + const opsStr = ops + .map((op) => { + if (op.type === 'presence') { + return `presence${JSON.stringify( + op.value, + )}`; + } + const opType = op.toTestString().split('.')[1]; + try { + const id = op.getExecutedAt()?.toTestString(); + return ` + + ${opType} + ${`${id}`}${op.toTestString()} + `; + } catch (e) { + // operation in the undo/redo stack does not yet have "executedAt" set. + return ` + + ${opType} + ${op.toTestString()} + `; + } + }) + .join('\n'); + return ` +
+ + +
+ `; + }) + .reverse() + .join(''); + }; + + return { displayRootObject, displayOps, displayUndoOps, displayRedoOps }; +}; + +export default objectDevtool; diff --git a/public/whiteboard.css b/public/whiteboard.css new file mode 100644 index 000000000..e844c878a --- /dev/null +++ b/public/whiteboard.css @@ -0,0 +1,119 @@ +.whiteboard-example { + display: flex; + flex-direction: column; + height: 100vh; +} + +.dev-log-wrap { + display: flex; + flex-direction: column; + height: calc(100vh - 350px); +} +.dev-log { + display: flex; + flex: 1; + overflow: hidden; +} +.dev-log .log-holders { + padding: 10px; +} +.dev-log .log-holders, +.dev-log .log-holder-wrap, +.dev-log .log-holder { + display: flex; + flex-direction: column; + flex: 1; + overflow: hidden; +} +.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; +} + +.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; +} diff --git a/public/whiteboard.html b/public/whiteboard.html new file mode 100644 index 000000000..ba70d3b63 --- /dev/null +++ b/public/whiteboard.html @@ -0,0 +1,384 @@ + + + + + Whiteboard Example + + + + + +
+
+
+ + + + +
+
+
+
+ + + + + + + + +
+
+
+
+
+ + +
+
+
+
+
+

yorkie document

+
+
+
+
+
+

operations

+
+
+
+
+
+

+ undo stack + + [ total: ] +

+
+
+
+

+ redo stack + + [ total: ] +

+
+
+
+
+
+
+ + + + + diff --git a/src/api/converter.ts b/src/api/converter.ts index 689c81fd9..7e5b07a92 100644 --- a/src/api/converter.ts +++ b/src/api/converter.ts @@ -1197,8 +1197,8 @@ function fromChangePack

( 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()!)); + const value = fromElement(pbRHTNode.getElement()!); + 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 910604512..505eb3827 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,15 +138,26 @@ export class Change

{ public execute( root: CRDTRoot, presences: Map, + source: OpSource, ): { opInfos: Array; reverseOps: Array>; } { const changeOpInfos: Array = []; const reverseOps: Array> = []; + if (process.env.NODE_ENV !== 'production' && this.operations.length) { + root.opsForTest.push(this.operations); + } for (const operation of this.operations) { - const { opInfos, reverseOp } = operation.execute(root); + 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); } @@ -161,6 +173,7 @@ export class Change

{ presences.delete(this.id.getActorID()!); } } + return { opInfos: changeOpInfos, reverseOps }; } 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

{ } /** - * `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 8170ba18c..9ad8513c2 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. @@ -75,27 +76,19 @@ 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()) { - return; - } - - return node; + public get(index: number): CRDTElement | undefined { + const node = this.elements.getByIndex(index); + 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) { - return; - } - - return node.getValue(); + public getByID(createdAt: TimeTicket): CRDTElement | undefined { + const node = this.elements.getByID(createdAt); + return node?.getValue(); } /** @@ -206,6 +199,27 @@ export class CRDTArray extends CRDTContainer { return JSON.parse(this.toJSON()); } + /** + * `toJSForTest` returns value with meta data for testing. + */ + 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] = { + key: String(i), + id, + value, + type, + }; + } + return { + id: this.getCreatedAt().toTestString(), + value: values, + type: 'YORKIE_ARRAY', + }; + } + /** * `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..cb33c0fc7 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, @@ -119,6 +120,17 @@ export class CRDTCounter extends CRDTElement { return this.toJSON(); } + /** + * `toJSForTest` returns value with meta data for testing. + */ + public toJSForTest(): Devtools.JSONElement { + return { + id: this.getCreatedAt().toTestString(), + value: this.value, + type: 'YORKIE_COUNTER', + }; + } + /** * `deepcopy` copies itself deeply. */ diff --git a/src/document/crdt/element.ts b/src/document/crdt/element.ts index 2f9607ec6..7cd93f7e8 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. @@ -58,6 +59,18 @@ export abstract class CRDTElement { return this.removedAt; } + /** + * `getPositionedAt` returns the time of this element when it was positioned + * in the document by undo/redo or move operation. + */ + public getPositionedAt(): TimeTicket { + if (!this.movedAt) { + return this.createdAt; + } + + return this.movedAt; + } + /** * `setMovedAt` sets the move time of this element. */ @@ -83,7 +96,7 @@ export abstract class CRDTElement { public remove(removedAt?: TimeTicket): boolean { if ( removedAt && - removedAt.after(this.createdAt) && + removedAt.after(this.getPositionedAt()) && (!this.removedAt || removedAt.after(this.removedAt)) ) { this.removedAt = removedAt; @@ -102,6 +115,7 @@ export abstract class CRDTElement { abstract toJSON(): string; abstract toSortedJSON(): string; + abstract toJSForTest(): Devtools.JSONElement; abstract deepcopy(): CRDTElement; } @@ -126,6 +140,19 @@ export abstract class CRDTContainer extends CRDTElement { abstract getDescendants( 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/element_rht.ts b/src/document/crdt/element_rht.ts index 79cff5395..d9d3bdadd 100644 --- a/src/document/crdt/element_rht.ts +++ b/src/document/crdt/element_rht.ts @@ -89,24 +89,22 @@ 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 ( - node != null && - !node.isRemoved() && - node.remove(value.getCreatedAt()) - ) { + if (node != null && !node.isRemoved() && node.remove(executedAt)) { removed = node.getValue(); } 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().getPositionedAt())) { this.nodeMapByKey.set(key, newNode); + value.setMovedAt(executedAt); } return removed; } @@ -187,15 +185,22 @@ export class ElementRHT { } /** - * `get` returns the value of the given key. + * `getByID` returns the node of the given createdAt. + */ + public getByID(createdAt: TimeTicket): ElementRHTNode | undefined { + return this.nodeMapByCreatedAt.get(createdAt.toIDString()); + } + + /** + * `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 81b9ceb63..86b650a61 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, @@ -59,8 +60,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); } /** @@ -84,7 +89,16 @@ 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); + return node?.getValue(); + } + + /** + * `getByID` returns the element of the given createAt. + */ + public getByID(createdAt: TimeTicket): CRDTElement | undefined { + const node = this.memberNodes.getByID(createdAt); + return node?.getValue(); } /** @@ -112,6 +126,27 @@ export class CRDTObject extends CRDTContainer { return JSON.parse(this.toJSON()); } + /** + * `toJSForTest` returns value with meta data for testing. + */ + public toJSForTest(): Devtools.JSONElement { + const values: Devtools.ContainerValue = {}; + for (const [key, elem] of this) { + const { id, value, type } = elem.toJSForTest(); + values[key] = { + key, + id, + value, + type, + }; + } + return { + id: this.getCreatedAt().toTestString(), + value: values, + type: 'YORKIE_OBJECT', + }; + } + /** * `getKeys` returns array of keys in this object. */ @@ -135,7 +170,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()}`); } @@ -155,7 +190,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.getPositionedAt(), + ); } clone.remove(this.getRemovedAt()); return clone; diff --git a/src/document/crdt/primitive.ts b/src/document/crdt/primitive.ts index fbaf9ccda..b3c4b440c 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, @@ -117,6 +118,17 @@ export class Primitive extends CRDTElement { return this.toJSON(); } + /** + * `toJSForTest` returns value with meta data for testing. + */ + public toJSForTest(): Devtools.JSONElement { + return { + id: this.getCreatedAt().toTestString(), + value: this.value, + type: 'YORKIE_PRIMITIVE', + }; + } + /** * `deepcopy` copies itself deeply. */ diff --git a/src/document/crdt/rga_tree_list.ts b/src/document/crdt/rga_tree_list.ts index 3a1add11b..df7bd0854 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 { } /** - * `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(); } /** @@ -263,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/crdt/root.ts b/src/document/crdt/root.ts index ad100a8bf..5e0c89596 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,12 +70,18 @@ export class CRDTRoot { * the element that has removed nodes when executing garbage collection. */ private elementHasRemovedNodesSetByCreatedAt: Set; + /** + * `opsForTest` is used for debugging the entire operation. + * operations accumulate only in dev mode. + */ + public opsForTest: Array>; 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..ee42dd8f3 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. @@ -350,6 +351,17 @@ export class CRDTText extends CRDTGCElement { return this.toJSON(); } + /** + * `toJSForTest` returns value with meta data for testing. + */ + public toJSForTest(): Devtools.JSONElement { + return { + id: this.getCreatedAt().toTestString(), + value: JSON.parse(this.toJSON()), + type: 'YORKIE_TEXT', + }; + } + /** * `toString` returns the string representation of this text. */ diff --git a/src/document/crdt/tree.ts b/src/document/crdt/tree.ts index d456908ab..152799df1 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; @@ -990,6 +991,17 @@ export class CRDTTree extends CRDTGCElement { return JSON.stringify(this.getRootTreeNode()); } + /** + * `toJSForTest` returns value with meta data for testing. + */ + public toJSForTest(): Devtools.JSONElement { + return { + id: this.getCreatedAt().toTestString(), + value: JSON.parse(this.toJSON()), + type: 'YORKIE_TREE', + }; + } + /** * `getRootTreeNode` returns the converted value of this tree to TreeNode. */ diff --git a/src/document/document.ts b/src/document/document.ts index d5b5f8f29..097acc8ca 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, @@ -66,7 +67,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. @@ -525,7 +526,11 @@ export class Document { } 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({ @@ -538,10 +543,14 @@ export class Document { if (reverseOps.length > 0) { this.internalHistory.pushUndo(reverseOps); } - this.internalHistory.clearRedo(); + // NOTE(chacha912): Clear redo when a new local operation is applied. + if (opInfos.length > 0) { + 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: { @@ -943,6 +952,13 @@ export class Document { return createJSON(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. * @@ -1040,9 +1056,8 @@ export class Document { 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: | WatchedEvent

| UnwatchedEvent

@@ -1086,23 +1101,24 @@ export class Document { } } - const executionResult = change.execute(this.root, this.presences); - if (change.hasOperations()) { - changeInfo = { - actor: actorID, - message: change.getMessage() || '', - operations: executionResult.opInfos, - }; - } + const { opInfos } = change.execute( + this.root, + this.presences, + OpSource.Remote, + ); // 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) { @@ -1261,6 +1277,7 @@ export class Document { // 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

( context, deepcopy(this.clone!.presences.get(this.changeID.getActorID()!)!), @@ -1274,16 +1291,13 @@ export class Document { } 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; - } + change.execute(this.clone!.root, this.clone!.presences, OpSource.UndoRedo); - const { opInfos, reverseOps } = change.execute(this.root, this.presences); + const { opInfos, reverseOps } = change.execute( + this.root, + this.presences, + OpSource.UndoRedo, + ); const reversePresence = context.getReversePresence(); if (reversePresence) { reverseOps.push({ @@ -1295,11 +1309,19 @@ export class Document { this.internalHistory.pushRedo(reverseOps); } - this.localChanges.push(change); - this.changeID = change.getID(); + // 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); + } + this.changeID = change.getID(); const actorID = this.changeID.getActorID()!; - if (change.hasOperations()) { + // 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: { @@ -1344,6 +1366,7 @@ export class Document { // 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

( context, deepcopy(this.clone!.presences.get(this.changeID.getActorID()!)!), @@ -1357,16 +1380,13 @@ export class Document { } 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; - } + change.execute(this.clone!.root, this.clone!.presences, OpSource.UndoRedo); - const { opInfos, reverseOps } = change.execute(this.root, this.presences); + const { opInfos, reverseOps } = change.execute( + this.root, + this.presences, + OpSource.UndoRedo, + ); const reversePresence = context.getReversePresence(); if (reversePresence) { reverseOps.push({ @@ -1378,11 +1398,18 @@ export class Document { 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()) { + // 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: { @@ -1406,14 +1433,14 @@ export class Document { /** * `getUndoStackForTest` returns the undo stack for test. */ - public getUndoStackForTest(): Array> { + public getUndoStackForTest(): Array>> { return this.internalHistory.getUndoStackForTest(); } /** * `getRedoStackForTest` returns the redo stack for test. */ - public getRedoStackForTest(): Array> { + public getRedoStackForTest(): Array>> { 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

{ /** * `getUndoStackForTest` returns the undo stack for test. */ - public getUndoStackForTest(): Array> { - return this.undoStack.map((ops) => - ops.map((op) => { - return op instanceof Operation ? op.toTestString() : JSON.stringify(op); - }), - ); + public getUndoStackForTest(): Array>> { + return this.undoStack; } /** * `getRedoStackForTest` returns the redo stack for test. */ - public getRedoStackForTest(): Array> { - return this.redoStack.map((ops) => - ops.map((op) => { - return op instanceof Operation ? op.toTestString() : JSON.stringify(op); - }), - ); + public getRedoStackForTest(): Array>> { + return this.redoStack; } } diff --git a/src/document/json/array.ts b/src/document/json/array.ts index b449f4f65..b8e45b1d2 100644 --- a/src/document/json/array.ts +++ b/src/document/json/array.ts @@ -171,11 +171,15 @@ export class ArrayProxy { }; } else if (method === 'getElementByID') { return (createdAt: TimeTicket): WrappedElement | undefined => { - return toWrappedElement(context, target.get(createdAt)); + const elem = target.getByID(createdAt); + if (!elem || elem.isRemoved()) { + return; + } + return toWrappedElement(context, elem); }; } 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 +239,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); @@ -448,7 +449,7 @@ export class ArrayProxy { AddOperation.create( target.getCreatedAt(), prevCreatedAt, - primitive, + primitive.deepcopy(), ticket, ), ); @@ -462,7 +463,7 @@ export class ArrayProxy { AddOperation.create( target.getCreatedAt(), prevCreatedAt, - array, + array.deepcopy(), ticket, ), ); @@ -475,7 +476,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!)) { @@ -578,9 +584,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, @@ -622,8 +626,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; } @@ -659,8 +662,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; } @@ -696,8 +698,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/json/object.ts b/src/document/json/object.ts index 4fc8ed2cb..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)); @@ -154,7 +158,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 +188,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 +205,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 +218,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/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/operation.ts b/src/document/operation/operation.ts index 525b7d241..3b246645b 100644 --- a/src/document/operation/operation.ts +++ b/src/document/operation/operation.ts @@ -20,6 +20,17 @@ 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', + UndoRedo = 'undoredo', +} + /** * `OperationInfo` represents the information of an operation. * It is used to inform to the user what kind of operation was executed. @@ -189,10 +200,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; } /** @@ -224,5 +239,8 @@ export abstract class Operation { /** * `execute` executes this operation on the given `CRDTRoot`. */ - public abstract execute(root: CRDTRoot): ExecutionResult; + public abstract execute( + root: CRDTRoot, + source: OpSource, + ): ExecutionResult | undefined; } diff --git a/src/document/operation/remove_operation.ts b/src/document/operation/remove_operation.ts index 9255ccd8b..49c41bee3 100644 --- a/src/document/operation/remove_operation.ts +++ b/src/document/operation/remove_operation.ts @@ -18,12 +18,15 @@ 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, } 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 +37,7 @@ export class RemoveOperation extends Operation { constructor( parentCreatedAt: TimeTicket, createdAt: TimeTicket, - executedAt: TimeTicket, + executedAt?: TimeTicket, ) { super(parentCreatedAt, executedAt); this.createdAt = createdAt; @@ -46,7 +49,7 @@ export class RemoveOperation extends Operation { public static create( parentCreatedAt: TimeTicket, createdAt: TimeTicket, - executedAt: TimeTicket, + executedAt?: TimeTicket, ): RemoveOperation { return new RemoveOperation(parentCreatedAt, createdAt, executedAt); } @@ -54,17 +57,32 @@ export class RemoveOperation extends Operation { /** * `execute` executes this operation on the given `CRDTRoot`. */ - public execute(root: CRDTRoot): ExecutionResult { - const parentObject = root.findByCreatedAt(this.getParentCreatedAt()); + public execute( + root: CRDTRoot, + source: OpSource, + ): ExecutionResult | undefined { + 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; - const key = obj.subPathOf(this.createdAt); - const elem = obj.delete(this.createdAt, this.getExecutedAt()); + + // 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() || !targetElem || targetElem.isRemoved()) + ) { + return; + } + const key = parentObject.subPathOf(this.createdAt); + const reverseOp = this.toReverseOperation(parentObject); + + const elem = parentObject.delete(this.createdAt, this.getExecutedAt()); root.registerRemovedElement(elem); const opInfos: Array = @@ -84,7 +102,29 @@ export class RemoveOperation extends Operation { }, ]; - return { opInfos }; + return { opInfos, reverseOp }; + } + + /** + * `toReverseOperation` returns the reverse operation of this operation. + */ + private toReverseOperation( + 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(), + ); + } + } + } } /** @@ -98,7 +138,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 88884ff44..74ae9eb91 100644 --- a/src/document/operation/set_operation.ts +++ b/src/document/operation/set_operation.ts @@ -20,9 +20,11 @@ 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'; +import { RemoveOperation } from '@yorkie-js-sdk/src/document/operation/remove_operation'; /** * `SetOperation` represents an operation that stores the value corresponding to the @@ -36,7 +38,7 @@ export class SetOperation extends Operation { key: string, value: CRDTElement, parentCreatedAt: TimeTicket, - executedAt: TimeTicket, + executedAt?: TimeTicket, ) { super(parentCreatedAt, executedAt); this.key = key; @@ -50,7 +52,7 @@ export class SetOperation extends Operation { key: string, value: CRDTElement, parentCreatedAt: TimeTicket, - executedAt: TimeTicket, + executedAt?: TimeTicket, ): SetOperation { return new SetOperation(key, value, parentCreatedAt, executedAt); } @@ -58,7 +60,10 @@ export class SetOperation 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()); if (!parentObject) { logger.fatal(`fail to find ${this.getParentCreatedAt()}`); @@ -67,8 +72,15 @@ 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.toReverseOperation(previousValue); + const value = this.value.deepcopy(); - const removed = obj.set(this.key, value); + const removed = obj.set(this.key, value, this.getExecutedAt()); root.registerElement(value, obj); if (removed) { root.registerRemovedElement(removed); @@ -82,9 +94,32 @@ export class SetOperation extends Operation { key: this.key, }, ], + reverseOp, }; } + /** + * `toReverseOperation` returns the reverse operation of this operation. + */ + private toReverseOperation(value: CRDTElement | undefined): Operation { + let reverseOp: SetOperation | RemoveOperation = RemoveOperation.create( + this.getParentCreatedAt(), + this.value.getCreatedAt(), + ); + + 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(), + this.getParentCreatedAt(), + ); + } + return reverseOp; + } + /** * `getEffectedCreatedAt` returns the creation time of the effected element. */ @@ -96,7 +131,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()}`; } /** diff --git a/src/types/devtools_element.ts b/src/types/devtools_element.ts new file mode 100644 index 000000000..6e87f4d9d --- /dev/null +++ b/src/types/devtools_element.ts @@ -0,0 +1,38 @@ +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 + | boolean + // eslint-disable-next-line @typescript-eslint/ban-types + | null + | { [key: string]: Json } + | Array; + +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; +}; 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; @@ -228,3 +232,9 @@ export function buildIndexTree(node: ElementNode): IndexTree { }); return doc.getRoot().t.getIndexTree(); } + +export function toStringHistoryOp

( + op: HistoryOperation

, +): 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 7aad05e98..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()), - `[["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()), - `[["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/document_test.ts b/test/integration/document_test.ts index 0e97e5384..b885c8940 100644 --- a/test/integration/document_test.ts +++ b/test/integration/document_test.ts @@ -745,7 +745,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 @@ -760,7 +760,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 @@ -779,7 +779,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++) { @@ -839,7 +839,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( @@ -872,7 +872,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 fafeba262..666f61658 100644 --- a/test/integration/object_test.ts +++ b/test/integration/object_test.ts @@ -1,7 +1,13 @@ import { describe, it, assert } from 'vitest'; -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 { toStringHistoryOp } from '@yorkie-js-sdk/test/helper/helper'; import { YorkieError } from '@yorkie-js-sdk/src/util/error'; describe('Object', function () { @@ -80,24 +86,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 = []; 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 +205,463 @@ describe('Object', function () { assert.equal(d1.toSortedJSON(), d2.toSortedJSON()); }, 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<{ + 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; + }>('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 = []; + 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.skip(`Should ensure convergence of peer's document 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(docKey); + const doc2 = new Document(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}}}'); // as-is: {"shape":{"point":{}}} + }); + + it(`Should handle reverse (set) operation targeting elements deleted by other peers`, async function ({ + task, + }) { + // 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(`${task.name}-${new Date().getTime()}`); + const doc1 = new Document(docKey); + const doc2 = new Document(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(), '{}'); + + // 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(); + }); + + 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(docKey); + const doc2 = new Document(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"}}'); + + 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(); + }); + + it('Can handle concurrent undo/redo: local undo & global redo', async function ({ + task, + }) { + interface TestDoc { + color: string; + } + const docKey = toDocKey(`${task.name}-${new Date().getTime()}`); + const doc1 = new Document(docKey); + const doc2 = new Document(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.color = 'black'; + }, 'init doc'); + await client1.sync(); + await client2.sync(); + assert.equal(doc1.toSortedJSON(), '{"color":"black"}'); + 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"}'); // local-undo + await client1.sync(); + await client2.sync(); + assert.equal(doc2.toSortedJSON(), '{"color":"black"}'); + + doc1.history.redo(); + assert.equal(doc1.toSortedJSON(), '{"color":"green"}'); // global-redo + await client1.sync(); + await client2.sync(); + assert.equal(doc2.toSortedJSON(), '{"color":"green"}'); + }); + + 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(docKey, { disableGC: true }); + const doc2 = new Document(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 }); + 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"}'); + 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(); + assert.equal(doc1.toSortedJSON(), '{"color":"black"}'); + assert.equal(doc2.toSortedJSON(), '{"color":"black"}'); + + doc1.history.redo(); + await client1.sync(); + await client2.sync(); + assert.equal(doc1.toSortedJSON(), '{"color":"red"}'); + assert.equal(doc2.toSortedJSON(), '{"color":"red"}'); + }); + + 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()}`); + // TODO(chacha912): Remove the disableGC option + const doc1 = new Document(docKey, { disableGC: true }); + const doc2 = new Document(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 }); + 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"}}'); + + doc1.history.undo(); + 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(), '{"shape":{"color":"red"}}'); + }); + }); }); 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 } }, - }), + }, ], ]); diff --git a/test/unit/document/crdt/root_test.ts b/test/unit/document/crdt/root_test.ts index d64f7245a..6e94776a0 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); @@ -90,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()); @@ -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 ccdac7e8f..6a55df498 100644 --- a/test/unit/document/document_test.ts +++ b/test/unit/document/document_test.ts @@ -971,6 +971,17 @@ describe.sequential('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(); });