diff --git a/public/whiteboard.html b/public/whiteboard.html index ba70d3b63..baaee3c89 100644 --- a/public/whiteboard.html +++ b/public/whiteboard.html @@ -169,8 +169,7 @@

const selectedShape = root.shapes.find( (shape) => shape.id === selectedShapeID, ); - selectedShape.point.x = movingShapePoint.x; - selectedShape.point.y = movingShapePoint.y; + selectedShape.point = movingShapePoint; presence.set({ movingShapePoint: null }); }); } diff --git a/src/api/converter.ts b/src/api/converter.ts index 7e5b07a92..b86a777ff 100644 --- a/src/api/converter.ts +++ b/src/api/converter.ts @@ -208,9 +208,11 @@ function toElementSimple(element: CRDTElement): PbJSONElementSimple { if (element instanceof CRDTObject) { pbElementSimple.setType(PbValueType.VALUE_TYPE_JSON_OBJECT); pbElementSimple.setCreatedAt(toTimeTicket(element.getCreatedAt())); + pbElementSimple.setValue(objectToBytes(element)); } else if (element instanceof CRDTArray) { pbElementSimple.setType(PbValueType.VALUE_TYPE_JSON_ARRAY); pbElementSimple.setCreatedAt(toTimeTicket(element.getCreatedAt())); + pbElementSimple.setValue(arrayToBytes(element)); } else if (element instanceof CRDTText) { pbElementSimple.setType(PbValueType.VALUE_TYPE_TEXT); pbElementSimple.setCreatedAt(toTimeTicket(element.getCreatedAt())); @@ -835,9 +837,19 @@ function fromCounterType(pbValueType: PbValueType): CounterType { function fromElementSimple(pbElementSimple: PbJSONElementSimple): CRDTElement { switch (pbElementSimple.getType()) { case PbValueType.VALUE_TYPE_JSON_OBJECT: - return CRDTObject.create(fromTimeTicket(pbElementSimple.getCreatedAt())!); + if (!pbElementSimple.getValue()) { + return CRDTObject.create( + fromTimeTicket(pbElementSimple.getCreatedAt())!, + ); + } + return bytesToObject(pbElementSimple.getValue_asU8()); case PbValueType.VALUE_TYPE_JSON_ARRAY: - return CRDTArray.create(fromTimeTicket(pbElementSimple.getCreatedAt())!); + if (!pbElementSimple.getValue()) { + return CRDTArray.create( + fromTimeTicket(pbElementSimple.getCreatedAt())!, + ); + } + return bytesToArray(pbElementSimple.getValue_asU8()); case PbValueType.VALUE_TYPE_TEXT: return CRDTText.create( RGATreeSplit.create(), @@ -1343,7 +1355,7 @@ function bytesToSnapshot

( */ function bytesToObject(bytes?: Uint8Array): CRDTObject { if (!bytes) { - return CRDTObject.create(InitialTimeTicket); + throw new Error('bytes is empty'); } const pbElement = PbJSONElement.deserializeBinary(bytes); @@ -1357,6 +1369,25 @@ function objectToBytes(obj: CRDTObject): Uint8Array { return toElement(obj).serializeBinary(); } +/** + * `bytesToArray` creates an CRDTArray from the given bytes. + */ +function bytesToArray(bytes?: Uint8Array): CRDTArray { + if (!bytes) { + throw new Error('bytes is empty'); + } + + const pbElement = PbJSONElement.deserializeBinary(bytes); + return fromArray(pbElement.getJsonArray()!); +} + +/** + * `arrayToBytes` converts the given CRDTArray to bytes. + */ +function arrayToBytes(array: CRDTArray): Uint8Array { + return toArray(array).serializeBinary(); +} + /** * `bytesToTree` creates an CRDTTree from the given bytes. */ diff --git a/src/document/crdt/array.ts b/src/document/crdt/array.ts index 9ad8513c2..8a930d4e9 100644 --- a/src/document/crdt/array.ts +++ b/src/document/crdt/array.ts @@ -39,8 +39,19 @@ export class CRDTArray extends CRDTContainer { /** * `create` creates a new instance of Array. */ - public static create(createdAt: TimeTicket): CRDTArray { - return new CRDTArray(createdAt, RGATreeList.create()); + public static create( + createdAt: TimeTicket, + value?: Array, + ): CRDTArray { + if (!value) { + return new CRDTArray(createdAt, RGATreeList.create()); + } + + const elements = RGATreeList.create(); + for (const v of value) { + elements.insertAfter(elements.getLastCreatedAt(), v.deepcopy()); + } + return new CRDTArray(createdAt, elements); } /** diff --git a/src/document/crdt/element.ts b/src/document/crdt/element.ts index 7cd93f7e8..5eca00a76 100644 --- a/src/document/crdt/element.ts +++ b/src/document/crdt/element.ts @@ -99,6 +99,9 @@ export abstract class CRDTElement { removedAt.after(this.getPositionedAt()) && (!this.removedAt || removedAt.after(this.removedAt)) ) { + // NOTE(chacha912): If it's a CRDTContainer, removedAt is marked only on + // the top-level element, without marking all descendant elements. This + // enhances the speed of deletion. this.removedAt = removedAt; return true; } diff --git a/src/document/crdt/object.ts b/src/document/crdt/object.ts index 86b650a61..a9ca13ee6 100644 --- a/src/document/crdt/object.ts +++ b/src/document/crdt/object.ts @@ -39,8 +39,19 @@ export class CRDTObject extends CRDTContainer { /** * `create` creates a new instance of CRDTObject. */ - public static create(createdAt: TimeTicket): CRDTObject { - return new CRDTObject(createdAt, ElementRHT.create()); + public static create( + createdAt: TimeTicket, + value?: { [key: string]: CRDTElement }, + ): CRDTObject { + if (!value) { + return new CRDTObject(createdAt, ElementRHT.create()); + } + + const memberNodes = ElementRHT.create(); + for (const [k, v] of Object.entries(value)) { + memberNodes.set(k, v.deepcopy(), v.getCreatedAt()); + } + return new CRDTObject(createdAt, memberNodes); } /** diff --git a/src/document/crdt/root.ts b/src/document/crdt/root.ts index 5e0c89596..beecb02f3 100644 --- a/src/document/crdt/root.ts +++ b/src/document/crdt/root.ts @@ -82,18 +82,7 @@ export class CRDTRoot { this.removedElementSetByCreatedAt = new Set(); this.elementHasRemovedNodesSetByCreatedAt = new Set(); this.opsForTest = []; - - this.elementPairMapByCreatedAt.set( - this.rootObject.getCreatedAt().toIDString(), - { element: this.rootObject }, - ); - - rootObject.getDescendants( - (elem: CRDTElement, parent: CRDTContainer): boolean => { - this.registerElement(elem, parent); - return false; - }, - ); + this.registerElement(rootObject, undefined); } /** @@ -115,6 +104,16 @@ export class CRDTRoot { return pair.element; } + /** + * `findElementPairByCreatedAt` returns the element and parent pair + * of given creation time. + */ + public findElementPairByCreatedAt( + createdAt: TimeTicket, + ): CRDTElementPair | undefined { + return this.elementPairMapByCreatedAt.get(createdAt.toIDString()); + } + /** * `createSubPaths` creates an array of the sub paths for the given element. */ @@ -150,23 +149,45 @@ export class CRDTRoot { } /** - * `registerElement` registers the given element to hash table. + * `registerElement` registers the given element and its descendants to hash table. */ - public registerElement(element: CRDTElement, parent: CRDTContainer): void { + public registerElement(element: CRDTElement, parent?: CRDTContainer): void { this.elementPairMapByCreatedAt.set(element.getCreatedAt().toIDString(), { parent, element, }); + + if (element instanceof CRDTContainer) { + element.getDescendants((elem, parent) => { + this.registerElement(elem, parent); + return false; + }); + } } /** - * `deregisterElement` deregister the given element from hash table. + * `deregisterElement` deregister the given element and its descendants from hash table. */ - public deregisterElement(element: CRDTElement): void { - this.elementPairMapByCreatedAt.delete(element.getCreatedAt().toIDString()); - this.removedElementSetByCreatedAt.delete( - element.getCreatedAt().toIDString(), - ); + public deregisterElement(element: CRDTElement): number { + let count = 0; + + const deregisterElementInternal = (elem: CRDTElement) => { + const createdAt = elem.getCreatedAt().toIDString(); + this.elementPairMapByCreatedAt.delete(createdAt); + this.removedElementSetByCreatedAt.delete(createdAt); + count++; + + if (elem instanceof CRDTContainer) { + elem.getDescendants((e) => { + deregisterElementInternal(e); + return false; + }); + } + }; + + deregisterElementInternal(element); + + return count; } /** @@ -253,7 +274,7 @@ export class CRDTRoot { ticket.compare(pair.element.getRemovedAt()!) >= 0 ) { pair.parent!.purge(pair.element); - count += this.garbageCollectInternal(pair.element); + count += this.deregisterElement(pair.element); } } @@ -273,25 +294,6 @@ export class CRDTRoot { return count; } - private garbageCollectInternal(element: CRDTElement): number { - let count = 0; - - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const callback = (elem: CRDTElement, parent?: CRDTContainer): boolean => { - this.deregisterElement(elem); - count++; - return false; - }; - - callback(element); - - if (element instanceof CRDTContainer) { - element.getDescendants(callback); - } - - return count; - } - /** * `toJSON` returns the JSON encoding of this root object. */ diff --git a/src/document/json/array.ts b/src/document/json/array.ts index b8e45b1d2..7c58d347e 100644 --- a/src/document/json/array.ts +++ b/src/document/json/array.ts @@ -21,18 +21,14 @@ import { MoveOperation } from '@yorkie-js-sdk/src/document/operation/move_operat import { RemoveOperation } from '@yorkie-js-sdk/src/document/operation/remove_operation'; import { ChangeContext } from '@yorkie-js-sdk/src/document/change/context'; import { CRDTElement } 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 { - Primitive, - PrimitiveValue, -} from '@yorkie-js-sdk/src/document/crdt/primitive'; -import { ObjectProxy } from '@yorkie-js-sdk/src/document/json/object'; +import { Primitive } from '@yorkie-js-sdk/src/document/crdt/primitive'; import { JSONElement, WrappedElement, toWrappedElement, toJSONElement, + buildCRDTElement, } from '@yorkie-js-sdk/src/document/json/element'; /** @@ -329,6 +325,22 @@ export class ArrayProxy { } } + /** + * `buildArrayElements` constructs array elements based on the user-provided array. + */ + public static buildArrayElements( + context: ChangeContext, + value: Array, + ): Array { + const elements: Array = []; + for (const v of value) { + const createdAt = context.issueTimeTicket(); + const elem = buildCRDTElement(context, v, createdAt); + elements.push(elem); + } + return elements; + } + /** * `pushInternal` pushes the value to the target array. */ @@ -439,58 +451,19 @@ export class ArrayProxy { prevCreatedAt: TimeTicket, value: unknown, ): CRDTElement { - const ticket = context.issueTimeTicket(); - if (Primitive.isSupport(value)) { - const primitive = Primitive.of(value as PrimitiveValue, ticket); - const clone = primitive.deepcopy(); - target.insertAfter(prevCreatedAt, clone); - context.registerElement(clone, target); - context.push( - AddOperation.create( - target.getCreatedAt(), - prevCreatedAt, - primitive.deepcopy(), - ticket, - ), - ); - return primitive; - } else if (Array.isArray(value)) { - const array = CRDTArray.create(ticket); - const clone = array.deepcopy(); - target.insertAfter(prevCreatedAt, clone); - context.registerElement(clone, target); - context.push( - AddOperation.create( - target.getCreatedAt(), - prevCreatedAt, - array.deepcopy(), - ticket, - ), - ); - for (const element of value) { - ArrayProxy.pushInternal(context, clone, element); - } - return array; - } else if (typeof value === 'object') { - const obj = CRDTObject.create(ticket); - target.insertAfter(prevCreatedAt, obj); - context.registerElement(obj, target); - context.push( - AddOperation.create( - target.getCreatedAt(), - prevCreatedAt, - obj.deepcopy(), - ticket, - ), - ); - - for (const [k, v] of Object.entries(value!)) { - ObjectProxy.setInternal(context, obj, k, v); - } - return obj; - } - - throw new TypeError(`Unsupported type of value: ${typeof value}`); + const createdAt = context.issueTimeTicket(); + const element = buildCRDTElement(context, value, createdAt); + target.insertAfter(prevCreatedAt, element); + context.registerElement(element, target); + context.push( + AddOperation.create( + target.getCreatedAt(), + prevCreatedAt, + element.deepcopy(), + createdAt, + ), + ); + return element; } /** @@ -579,7 +552,9 @@ export class ArrayProxy { for (let i = from; i < to; i++) { const removed = ArrayProxy.deleteInternalByIndex(context, target, from); if (removed) { - removeds.push(toJSONElement(context, removed)!); + const removedElem = removed.deepcopy(); + removedElem.setRemovedAt(); + removeds.push(toJSONElement(context, removedElem)!); } } if (items) { diff --git a/src/document/json/element.ts b/src/document/json/element.ts index d7c63d8b4..65e679414 100644 --- a/src/document/json/element.ts +++ b/src/document/json/element.ts @@ -28,14 +28,18 @@ import { CRDTCounter, } from '@yorkie-js-sdk/src/document/crdt/counter'; import { CRDTTree } from '@yorkie-js-sdk/src/document/crdt/tree'; +import { RGATreeSplit } from '@yorkie-js-sdk/src/document/crdt/rga_tree_split'; +import { TimeTicket } from '@yorkie-js-sdk/src/document/time/ticket'; import { JSONObject, createJSONObject, + ObjectProxy, } from '@yorkie-js-sdk/src/document/json/object'; import { JSONArray, createJSONArray, + ArrayProxy, } from '@yorkie-js-sdk/src/document/json/array'; import { Text } from '@yorkie-js-sdk/src/document/json/text'; import { Counter } from '@yorkie-js-sdk/src/document/json/counter'; @@ -131,3 +135,45 @@ export function toJSONElement( return wrappedElement; } + +/** + * `buildCRDTElement` constructs a CRDTElement from the given value. + */ +export function buildCRDTElement( + context: ChangeContext, + value: unknown, + createdAt: TimeTicket, +): CRDTElement { + let element: CRDTElement; + if (Primitive.isSupport(value)) { + element = Primitive.of(value as PrimitiveValue, createdAt); + } else if (Array.isArray(value)) { + element = CRDTArray.create( + createdAt, + ArrayProxy.buildArrayElements(context, value), + ); + } else if (typeof value === 'object') { + if (value instanceof Text) { + element = CRDTText.create(RGATreeSplit.create(), createdAt); + value.initialize(context, element as CRDTText); + } else if (value instanceof Counter) { + element = CRDTCounter.create( + value.getValueType(), + value.getValue(), + createdAt, + ); + value.initialize(context, element as CRDTCounter); + } else if (value instanceof Tree) { + element = CRDTTree.create(value.buildRoot(context), createdAt); + value.initialize(context, element as CRDTTree); + } else { + element = CRDTObject.create( + createdAt, + ObjectProxy.buildObjectMembers(context, value!), + ); + } + } else { + throw new TypeError(`Unsupported type of value: ${typeof value}`); + } + return element; +} diff --git a/src/document/json/object.ts b/src/document/json/object.ts index e60cc4704..e594d13da 100644 --- a/src/document/json/object.ts +++ b/src/document/json/object.ts @@ -22,20 +22,10 @@ import { RemoveOperation } from '@yorkie-js-sdk/src/document/operation/remove_op import { ChangeContext } from '@yorkie-js-sdk/src/document/change/context'; import { CRDTElement } 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 { - Primitive, - PrimitiveValue, -} from '@yorkie-js-sdk/src/document/crdt/primitive'; -import { RGATreeSplit } from '@yorkie-js-sdk/src/document/crdt/rga_tree_split'; -import { CRDTText } from '@yorkie-js-sdk/src/document/crdt/text'; -import { ArrayProxy } from '@yorkie-js-sdk/src/document/json/array'; -import { Text } from '@yorkie-js-sdk/src/document/json/text'; -import { toJSONElement } from '@yorkie-js-sdk/src/document/json/element'; -import { CRDTCounter } from '@yorkie-js-sdk/src/document/crdt/counter'; -import { Counter } from '@yorkie-js-sdk/src/document/json/counter'; -import { CRDTTree } from '@yorkie-js-sdk/src/document/crdt/tree'; -import { Tree } from '@yorkie-js-sdk/src/document/json/tree'; + toJSONElement, + buildCRDTElement, +} from '@yorkie-js-sdk/src/document/json/element'; /** * `JSONObject` represents a JSON object, but unlike regular JSON, it has time @@ -155,103 +145,47 @@ export class ObjectProxy { ); } - const ticket = context.issueTimeTicket(); - - const setAndRegister = function (elem: CRDTElement) { - const removed = target.set(key, elem, ticket); - context.registerElement(elem, target); - if (removed) { - context.registerRemovedElement(removed); - } - }; + const createdAt = context.issueTimeTicket(); + const element = buildCRDTElement(context, value, createdAt); + const removed = target.set(key, element, createdAt); + context.registerElement(element, target); + if (removed) { + context.registerRemovedElement(removed); + } + context.push( + SetOperation.create( + key, + element.deepcopy(), + target.getCreatedAt(), + createdAt, + ), + ); + } - if (Primitive.isSupport(value)) { - const primitive = Primitive.of(value as PrimitiveValue, ticket); - setAndRegister(primitive); - context.push( - SetOperation.create( - key, - primitive.deepcopy(), - target.getCreatedAt(), - ticket, - ), - ); - } else if (Array.isArray(value)) { - const array = CRDTArray.create(ticket); - setAndRegister(array); - context.push( - SetOperation.create( - key, - array.deepcopy(), - target.getCreatedAt(), - ticket, - ), - ); - for (const element of value) { - ArrayProxy.pushInternal(context, array, element); - } - } else if (typeof value === 'object') { - if (value instanceof Text) { - const text = CRDTText.create(RGATreeSplit.create(), ticket); - target.set(key, text, ticket); - context.registerElement(text, target); - context.push( - SetOperation.create( - key, - text.deepcopy(), - target.getCreatedAt(), - ticket, - ), - ); - value.initialize(context, text); - } else if (value instanceof Counter) { - const counter = CRDTCounter.create( - value.getValueType(), - value.getValue(), - ticket, - ); - target.set(key, counter, ticket); - context.registerElement(counter, target); - context.push( - SetOperation.create( - key, - counter.deepcopy(), - target.getCreatedAt(), - ticket, - ), - ); - value.initialize(context, counter); - } else if (value instanceof Tree) { - const tree = CRDTTree.create(value.buildRoot(context), ticket); - target.set(key, tree, ticket); - context.registerElement(tree, target); - context.push( - SetOperation.create( - key, - tree.deepcopy(), - target.getCreatedAt(), - ticket, - ), - ); - value.initialize(context, tree); - } else { - const obj = CRDTObject.create(ticket); - setAndRegister(obj); - context.push( - SetOperation.create( - key, - obj.deepcopy(), - target.getCreatedAt(), - ticket, - ), + /** + * `buildObjectMembers` constructs an object where all values from the + * user-provided object are transformed into CRDTElements. + * This function takes an object and iterates through its values, + * converting each value into a corresponding CRDTElement. + */ + public static buildObjectMembers( + context: ChangeContext, + value: object, + ): { [key: string]: CRDTElement } { + const members: { [key: string]: CRDTElement } = {}; + for (const [k, v] of Object.entries(value)) { + if (k.includes('.')) { + throw new YorkieError( + Code.InvalidObjectKey, + `key must not contain the '.'.`, ); - for (const [k, v] of Object.entries(value!)) { - ObjectProxy.setInternal(context, obj, k, v); - } } - } else { - logger.fatal(`unsupported type of value: ${typeof value}`); + + const createdAt = context.issueTimeTicket(); + const elem = buildCRDTElement(context, v, createdAt); + members[k] = elem; } + return members; } /** diff --git a/src/document/operation/add_operation.ts b/src/document/operation/add_operation.ts index 8a732e034..ec7208677 100644 --- a/src/document/operation/add_operation.ts +++ b/src/document/operation/add_operation.ts @@ -69,6 +69,7 @@ export class AddOperation extends Operation { const value = this.value.deepcopy(); array.insertAfter(this.prevCreatedAt, value); root.registerElement(value, array); + return { opInfos: [ { @@ -91,7 +92,7 @@ export class AddOperation extends Operation { * `toTestString` returns a string containing the meta data. */ public toTestString(): string { - return `${this.getParentCreatedAt().toTestString()}.ADD`; + return `${this.getParentCreatedAt().toTestString()}.ADD.${this.value.toJSON()}`; } /** diff --git a/src/document/operation/remove_operation.ts b/src/document/operation/remove_operation.ts index 49c41bee3..5e88da410 100644 --- a/src/document/operation/remove_operation.ts +++ b/src/document/operation/remove_operation.ts @@ -23,7 +23,10 @@ import { OperationInfo, ExecutionResult, } from '@yorkie-js-sdk/src/document/operation/operation'; -import { CRDTContainer } from '@yorkie-js-sdk/src/document/crdt/element'; +import { + CRDTContainer, + CRDTElement, +} 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'; @@ -61,32 +64,34 @@ export class RemoveOperation extends Operation { root: CRDTRoot, source: OpSource, ): ExecutionResult | undefined { - const parentObject = root.findByCreatedAt( + const container = root.findByCreatedAt( this.getParentCreatedAt(), ) as CRDTContainer; - if (!parentObject) { + if (!container) { logger.fatal(`fail to find ${this.getParentCreatedAt()}`); } - if (!(parentObject instanceof CRDTContainer)) { - logger.fatal(`only object and array can execute remove: ${parentObject}`); + if (!(container instanceof CRDTContainer)) { + logger.fatal(`only object and array can execute remove: ${container}`); } // 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; + if (source === OpSource.UndoRedo) { + let parent: CRDTElement | undefined = container.getByID(this.createdAt); + while (parent) { + if (parent.getRemovedAt()) { + return; + } + parent = root.findElementPairByCreatedAt(parent.getCreatedAt())?.parent; + } } - const key = parentObject.subPathOf(this.createdAt); - const reverseOp = this.toReverseOperation(parentObject); + const key = container.subPathOf(this.createdAt); + const reverseOp = this.toReverseOperation(container); - const elem = parentObject.delete(this.createdAt, this.getExecutedAt()); + const elem = container.delete(this.createdAt, this.getExecutedAt()); root.registerRemovedElement(elem); const opInfos: Array = - parentObject instanceof CRDTArray + container instanceof CRDTArray ? [ { type: 'remove', diff --git a/src/document/operation/set_operation.ts b/src/document/operation/set_operation.ts index 74ae9eb91..8d01db463 100644 --- a/src/document/operation/set_operation.ts +++ b/src/document/operation/set_operation.ts @@ -64,23 +64,38 @@ export class SetOperation extends Operation { root: CRDTRoot, source: OpSource, ): ExecutionResult | undefined { - const parentObject = root.findByCreatedAt(this.getParentCreatedAt()); - if (!parentObject) { + const obj = root.findByCreatedAt(this.getParentCreatedAt()) as CRDTObject; + if (!obj) { logger.fatal(`fail to find ${this.getParentCreatedAt()}`); } - if (!(parentObject instanceof CRDTObject)) { + if (!(obj instanceof CRDTObject)) { 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; + if (source === OpSource.UndoRedo) { + let parent: CRDTElement | undefined = obj; + while (parent) { + if (parent.getRemovedAt()) { + return; + } + parent = root.findElementPairByCreatedAt(parent.getCreatedAt())?.parent; + } } const previousValue = obj.get(this.key); const reverseOp = this.toReverseOperation(previousValue); const value = this.value.deepcopy(); const removed = obj.set(this.key, value, this.getExecutedAt()); + // NOTE(chacha912): When resetting elements with the pre-existing createdAt + // during undo/redo, it's essential to handle previously tombstoned elements. + // In non-GC languages, there may be a need to execute both deregister and purge. + if ( + source === OpSource.UndoRedo && + root.findByCreatedAt(value.getCreatedAt()) + ) { + root.deregisterElement(value); + } root.registerElement(value, obj); if (removed) { root.registerRemovedElement(removed); @@ -108,9 +123,6 @@ 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(), diff --git a/test/integration/document_test.ts b/test/integration/document_test.ts index b885c8940..066c1679c 100644 --- a/test/integration/document_test.ts +++ b/test/integration/document_test.ts @@ -158,9 +158,6 @@ describe('Document', function () { expectedEventValue = [ { type: 'set', path: '$', key: 'counter' }, { type: 'set', path: '$', key: 'todos' }, - { type: 'add', path: '$.todos', index: 0 }, - { type: 'add', path: '$.todos', index: 1 }, - { type: 'add', path: '$.todos', index: 2 }, { type: 'set', path: '$', key: 'content' }, { type: 'edit', @@ -173,16 +170,7 @@ describe('Document', function () { path: '$.content', }, { type: 'set', path: '$', key: 'obj' }, - { type: 'set', path: '$.obj', key: 'name' }, - { type: 'set', path: '$.obj', key: 'age' }, - { type: 'set', path: '$.obj', key: 'food' }, - { type: 'add', path: '$.obj.food', index: 0 }, - { type: 'add', path: '$.obj.food', index: 1 }, - { type: 'set', path: '$.obj', key: 'score' }, - { type: 'set', path: '$.obj.score', key: 'english' }, - { type: 'set', path: '$.obj.score', key: 'math' }, { type: 'set', path: '$.obj', key: 'score' }, - { type: 'set', path: '$.obj.score', key: 'science' }, { type: 'remove', path: '$.obj', key: 'food' }, ]; await eventCollectorD1.waitAndVerifyNthEvent(1, { @@ -277,12 +265,6 @@ describe('Document', function () { await eventCollector.waitAndVerifyNthEvent(1, [ { type: 'set', path: '$', key: 'counter' }, { type: 'set', path: '$', key: 'todos' }, - { type: 'add', path: '$.todos', index: 0 }, - { type: 'add', path: '$.todos', index: 1 }, - ]); - await eventCollectorForTodos.waitAndVerifyNthEvent(1, [ - { type: 'add', path: '$.todos', index: 0 }, - { type: 'add', path: '$.todos', index: 1 }, ]); d2.update((root) => { @@ -301,7 +283,7 @@ describe('Document', function () { await eventCollector.waitAndVerifyNthEvent(3, [ { type: 'add', path: '$.todos', index: 2 }, ]); - await eventCollectorForTodos.waitAndVerifyNthEvent(2, [ + await eventCollectorForTodos.waitAndVerifyNthEvent(1, [ { type: 'add', path: '$.todos', index: 2 }, ]); @@ -312,7 +294,7 @@ describe('Document', function () { await eventCollector.waitAndVerifyNthEvent(4, [ { type: 'add', path: '$.todos', index: 3 }, ]); - assert.equal(eventCollectorForTodos.getLength(), 2); // No events after unsubscribing `$.todos` + assert.equal(eventCollectorForTodos.getLength(), 1); // No events after unsubscribing `$.todos` unsubCounter(); d2.update((root) => { @@ -374,21 +356,7 @@ describe('Document', function () { }); await eventCollector.waitAndVerifyNthEvent(1, [ { type: 'set', path: '$', key: 'todos' }, - { type: 'add', path: '$.todos', index: 0 }, - { type: 'set', path: '$.todos.0', key: 'text' }, - { type: 'set', path: '$.todos.0', key: 'completed' }, { type: 'set', path: '$', key: 'obj' }, - { type: 'set', path: '$.obj', key: 'c1' }, - { type: 'set', path: '$.obj.c1', key: 'name' }, - { type: 'set', path: '$.obj.c1', key: 'age' }, - ]); - await eventCollectorForTodos0.waitAndVerifyNthEvent(1, [ - { type: 'set', path: '$.todos.0', key: 'text' }, - { type: 'set', path: '$.todos.0', key: 'completed' }, - ]); - await eventCollectorForObjC1.waitAndVerifyNthEvent(1, [ - { type: 'set', path: '$.obj.c1', key: 'name' }, - { type: 'set', path: '$.obj.c1', key: 'age' }, ]); d2.update((root) => { @@ -397,7 +365,7 @@ describe('Document', function () { await eventCollector.waitAndVerifyNthEvent(2, [ { type: 'set', path: '$.obj.c1', key: 'name' }, ]); - await eventCollectorForObjC1.waitAndVerifyNthEvent(2, [ + await eventCollectorForObjC1.waitAndVerifyNthEvent(1, [ { type: 'set', path: '$.obj.c1', key: 'name' }, ]); @@ -407,7 +375,7 @@ describe('Document', function () { await eventCollector.waitAndVerifyNthEvent(3, [ { type: 'set', path: '$.todos.0', key: 'completed' }, ]); - await eventCollectorForTodos0.waitAndVerifyNthEvent(2, [ + await eventCollectorForTodos0.waitAndVerifyNthEvent(1, [ { type: 'set', path: '$.todos.0', key: 'completed' }, ]); @@ -418,7 +386,7 @@ describe('Document', function () { await eventCollector.waitAndVerifyNthEvent(4, [ { type: 'set', path: '$.todos.0', key: 'text' }, ]); - assert.equal(eventCollectorForTodos0.getLength(), 2); // No events after unsubscribing `$.todos.0` + assert.equal(eventCollectorForTodos0.getLength(), 1); // No events after unsubscribing `$.todos.0` unsubObj(); d2.update((root) => { @@ -427,7 +395,7 @@ describe('Document', function () { await eventCollector.waitAndVerifyNthEvent(5, [ { type: 'set', path: '$.obj.c1', key: 'age' }, ]); - assert.equal(eventCollectorForObjC1.getLength(), 2); // No events after unsubscribing `$.obj.c1` + assert.equal(eventCollectorForObjC1.getLength(), 1); // No events after unsubscribing `$.obj.c1` unsub(); await c1.detach(d1); diff --git a/test/integration/gc_test.ts b/test/integration/gc_test.ts index 358a34bf6..48b17052a 100644 --- a/test/integration/gc_test.ts +++ b/test/integration/gc_test.ts @@ -1,12 +1,11 @@ import { describe, it, assert } from 'vitest'; import { MaxTimeTicket } from '@yorkie-js-sdk/src/document/time/ticket'; import { CRDTArray } from '@yorkie-js-sdk/src/document/crdt/array'; -import yorkie, { Tree } from '@yorkie-js-sdk/src/yorkie'; +import yorkie, { Text, Tree } from '@yorkie-js-sdk/src/yorkie'; import { testRPCAddr, toDocKey, } from '@yorkie-js-sdk/test/integration/integration_helper'; -import { Text } from '@yorkie-js-sdk/src/yorkie'; import { CRDTTreeNode } from '@yorkie-js-sdk/src/document/crdt/tree'; import { IndexTreeNode } from '@yorkie-js-sdk/src/util/index_tree'; @@ -610,6 +609,34 @@ describe('Garbage Collection', function () { assert.equal(doc.getGarbageLenFromClone(), 6); }); + it('Can collect removed elements from both root and clone for nested array', async function ({ + task, + }) { + type TestDoc = { list: Array> }; + const docKey = toDocKey(`${task.name}-${new Date().getTime()}`); + const doc = new yorkie.Document(docKey); + const cli = new yorkie.Client(testRPCAddr); + await cli.activate(); + + await cli.attach(doc, { isRealtimeSync: false }); + doc.update((root) => { + root.list = [0, 1, 2]; + root.list.push([3, 4, 5]); + }); + assert.equal('{"list":[0,1,2,[3,4,5]]}', doc.toJSON()); + doc.update((root) => { + delete root.list[1]; + }); + assert.equal('{"list":[0,2,[3,4,5]]}', doc.toJSON()); + doc.update((root) => { + delete (root.list[2] as Array)[1]; + }); + assert.equal('{"list":[0,2,[3,5]]}', doc.toJSON()); + + assert.equal(doc.getGarbageLen(), 2); + assert.equal(doc.getGarbageLenFromClone(), 2); + }); + it('Can purges removed elements after peers can not access them', async function ({ task, }) { diff --git a/test/integration/object_test.ts b/test/integration/object_test.ts index 40e9411b3..b551a5ba7 100644 --- a/test/integration/object_test.ts +++ b/test/integration/object_test.ts @@ -246,14 +246,14 @@ describe('Object', function () { 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'], + ['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={}', '1:00:1.SET.color="black"'], + ['0:00:0.SET.shape={"color":"black"}'], ); doc.history.redo(); @@ -363,7 +363,7 @@ describe('Object', function () { assertUndoRedo(doc, states); }); - it.skip(`Should ensure convergence of peer's document after undoing nested objects`, async function ({ + it(`Should ensure convergence of peer's document after undoing nested objects`, async function ({ task, }) { // Test scenario: @@ -404,11 +404,10 @@ describe('Object', function () { 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":{}}} + assert.equal(doc2.toSortedJSON(), '{"shape":{"point":{"x":0,"y":0}}}'); }); - it(`Should handle reverse (set) operation targeting elements deleted by other peers`, async function ({ + it(`Should handle reverse set operation for elements that other peers deleted`, async function ({ task, }) { // Test scenario: @@ -455,12 +454,80 @@ describe('Object', function () { assert.equal(doc1.toSortedJSON(), '{}'); assert.equal(doc1.getRedoStackForTest().length, 0); assert.equal(doc1.history.canRedo(), false); + }); + + it(`Should handle reverse set operation for elements (nested objects) that other peers deleted`, async function ({ + task, + }) { + // Test scenario: + // c1: create shape + // c1: set shape.circle.point to { x: 1, y: 1 } + // c2: delete shape + // c1: undo(no changes as the shape was deleted) + interface TestDoc { + shape?: { circle: { 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 = { circle: { point: { x: 0, y: 0 } } }; + }); + await client1.sync(); + assert.equal( + doc1.toSortedJSON(), + '{"shape":{"circle":{"point":{"x":0,"y":0}}}}', + ); + + await client2.attach(doc2, { isRealtimeSync: false }); + assert.equal( + doc2.toSortedJSON(), + '{"shape":{"circle":{"point":{"x":0,"y":0}}}}', + ); + + doc1.update((root) => { + root.shape!.circle.point = { x: 1, y: 1 }; + }); await client1.sync(); await client2.sync(); + assert.equal( + doc1.toSortedJSON(), + '{"shape":{"circle":{"point":{"x":1,"y":1}}}}', + ); + assert.equal( + doc2.toSortedJSON(), + '{"shape":{"circle":{"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(), '{}'); + + const c1ID = client1.getID()!.slice(-2); + assert.deepEqual( + doc1.getUndoStackForTest().at(-1)?.map(toStringHistoryOp), + [`2:${c1ID}:2.SET.point={"x":0,"y":0}`], + ); + doc1.history.undo(); + assert.equal(doc1.toSortedJSON(), '{}'); + await client1.sync(); + await client2.sync(); + assert.equal(doc2.toSortedJSON(), '{}'); + assert.equal(doc1.getRedoStackForTest().length, 0); + assert.equal(doc1.history.canRedo(), false); }); - it(`Should handle reverse (remove) operation targeting elements deleted by other peers`, async function ({ + it(`Should handle reverse remove operation for elements that other peers deleted`, async function ({ task, }) { // Test scenario: @@ -497,6 +564,127 @@ describe('Object', function () { 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); + }); + + it(`Should handle reverse remove operation for elements (nested objects) that other peers deleted`, async function ({ + task, + }) { + // Test scenario: + // c1: set shape.circle.point to { x: 0, y: 0 } + // c2: delete shape + // c1: undo(no changes as the shape was deleted) + interface TestDoc { + shape?: { + circle?: { point?: { x?: number; y?: number }; 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 = { circle: { color: 'red' } }; + }); + await client1.sync(); + assert.equal(doc1.toSortedJSON(), '{"shape":{"circle":{"color":"red"}}}'); + + await client2.attach(doc2, { isRealtimeSync: false }); + assert.equal(doc2.toSortedJSON(), '{"shape":{"circle":{"color":"red"}}}'); + + doc1.update((root) => { + root.shape!.circle!.point = { x: 0, y: 0 }; + }); + await client1.sync(); + await client2.sync(); + assert.equal( + doc1.toSortedJSON(), + '{"shape":{"circle":{"color":"red","point":{"x":0,"y":0}}}}', + ); + assert.equal( + doc2.toSortedJSON(), + '{"shape":{"circle":{"color":"red","point":{"x":0,"y":0}}}}', + ); + doc2.update((root) => { + delete root.shape; + }, 'delete shape'); + await client2.sync(); + await client1.sync(); + assert.equal(doc1.toSortedJSON(), '{}'); + assert.equal(doc2.toSortedJSON(), '{}'); + + const c1ID = client1.getID()!.slice(-2); + assert.deepEqual( + doc1.getUndoStackForTest().at(-1)?.map(toStringHistoryOp), + [`2:${c1ID}:2.REMOVE.3:${c1ID}:1`], + ); + doc1.history.undo(); + assert.equal(doc1.toSortedJSON(), '{}'); + await client1.sync(); + await client2.sync(); + assert.equal(doc2.toSortedJSON(), '{}'); + assert.deepEqual(doc1.getRedoStackForTest().length, 0); + }); + + it(`Should not propagate changes when there is no applied undo operation`, async function ({ + task, + }) { + 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 }); + let doc1ChangeID = doc1.getChangeID(); + let doc1Checkpoint = doc1.getCheckpoint(); + assert.equal(doc1ChangeID.getClientSeq(), 1); + assert.equal(doc1Checkpoint.getClientSeq(), 1); + assert.equal(doc1Checkpoint.getServerSeq().toInt(), 1); + + doc1.update((root) => { + root.shape = { color: 'black' }; + }, 'init doc'); + await client1.sync(); + assert.equal(doc1.toSortedJSON(), '{"shape":{"color":"black"}}'); + doc1ChangeID = doc1.getChangeID(); + doc1Checkpoint = doc1.getCheckpoint(); + assert.equal(doc1ChangeID.getClientSeq(), 2); + assert.equal(doc1Checkpoint.getClientSeq(), 2); + assert.equal(doc1Checkpoint.getServerSeq().toInt(), 2); + + 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(), '{}'); + doc1ChangeID = doc1.getChangeID(); + doc1Checkpoint = doc1.getCheckpoint(); + assert.equal(doc1ChangeID.getClientSeq(), 2); + assert.equal(doc1Checkpoint.getClientSeq(), 2); + assert.equal(doc1Checkpoint.getServerSeq().toInt(), 4); + // c2 deleted the shape, so the reverse operation cannot be applied doc1.history.undo(); assert.equal(doc1.toSortedJSON(), '{}'); @@ -505,6 +693,23 @@ describe('Object', function () { await client1.sync(); await client2.sync(); await client1.sync(); + // Since there are no applied operations, there should be no change in the sequence. + doc1ChangeID = doc1.getChangeID(); + doc1Checkpoint = doc1.getCheckpoint(); + assert.equal(doc1ChangeID.getClientSeq(), 2); + assert.equal(doc1Checkpoint.getClientSeq(), 2); + assert.equal(doc1Checkpoint.getServerSeq().toInt(), 4); + + doc1.update((root) => { + root.shape = { color: 'red' }; + }); + await client1.sync(); + assert.equal(doc1.toSortedJSON(), '{"shape":{"color":"red"}}'); + doc1ChangeID = doc1.getChangeID(); + doc1Checkpoint = doc1.getCheckpoint(); + assert.equal(doc1ChangeID.getClientSeq(), 3); + assert.equal(doc1Checkpoint.getClientSeq(), 3); + assert.equal(doc1Checkpoint.getServerSeq().toInt(), 5); }); it(`Should not propagate changes when there is no applied undo operation`, async function ({ @@ -739,5 +944,50 @@ describe('Object', function () { await client2.sync(); assert.equal(doc2.toSortedJSON(), '{"shape":{"color":"red"}}'); }); + + it(`Should clean up the references to a previously deleted node when the deleted node is restored through undo`, async function ({ + task, + }) { + 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 }); + await client2.attach(doc2, { isRealtimeSync: false }); + + doc1.update((root) => { + root.shape = { color: 'black' }; + }); + await client1.sync(); + await client2.sync(); + assert.equal(doc1.toSortedJSON(), '{"shape":{"color":"black"}}'); + assert.equal(doc2.toSortedJSON(), '{"shape":{"color":"black"}}'); + + doc2.update((root) => { + root.shape = { color: 'yellow' }; + }); + await client2.sync(); + await client1.sync(); + assert.equal(doc1.toSortedJSON(), '{"shape":{"color":"yellow"}}'); + assert.equal(doc2.toSortedJSON(), '{"shape":{"color":"yellow"}}'); + + doc2.history.undo(); + await client2.sync(); + await client1.sync(); + assert.equal(doc1.toSortedJSON(), '{"shape":{"color":"black"}}'); + assert.equal(doc2.toSortedJSON(), '{"shape":{"color":"black"}}'); + + // NOTE(chacha912): removedElementSetByCreatedAt should only retain + // the entry for `{shape: {color: 'yellow'}}`. + assert.equal(doc2.getGarbageLen(), 2); + }); }); });