From 805c9f2ff2ed308eb6bc74b4bc04639a37c3bfef Mon Sep 17 00:00:00 2001 From: raararaara Date: Thu, 1 Feb 2024 16:03:08 +0900 Subject: [PATCH 1/8] Add isRemoved member variable to RHT class --- src/document/crdt/rht.ts | 89 +++++++++++++++++++++++++---- src/document/time/ticket.ts | 7 +++ test/unit/document/crdt/rht_test.ts | 23 +++++++- 3 files changed, 108 insertions(+), 11 deletions(-) diff --git a/src/document/crdt/rht.ts b/src/document/crdt/rht.ts index e4ebfd8da..b06a88ce7 100644 --- a/src/document/crdt/rht.ts +++ b/src/document/crdt/rht.ts @@ -24,18 +24,30 @@ export class RHTNode { private key: string; private value: string; private updatedAt: TimeTicket; - - constructor(key: string, value: string, updatedAt: TimeTicket) { + private _isRemoved: boolean; + + constructor( + key: string, + value: string, + updatedAt: TimeTicket, + isRemoved: boolean, + ) { this.key = key; this.value = value; this.updatedAt = updatedAt; + this._isRemoved = isRemoved; } /** * `of` creates a new instance of RHTNode. */ - public static of(key: string, value: string, createdAt: TimeTicket): RHTNode { - return new RHTNode(key, value, createdAt); + public static of( + key: string, + value: string, + createdAt: TimeTicket, + isRemoved: boolean, + ): RHTNode { + return new RHTNode(key, value, createdAt, isRemoved); } /** @@ -53,11 +65,18 @@ export class RHTNode { } /** - * `getUpdatedAt `returns updated time of node. + * `getUpdatedAt` returns updated time of node. */ public getUpdatedAt(): TimeTicket { return this.updatedAt; } + + /** + * `isRemoved` returns whether the node has been removed or not. + */ + public isRemoved(): boolean { + return this._isRemoved; + } } /** @@ -66,9 +85,11 @@ export class RHTNode { */ export class RHT { private nodeMapByKey: Map; + private numberOfRemovedElement: number; constructor() { this.nodeMapByKey = new Map(); + this.numberOfRemovedElement = 0; } /** @@ -85,16 +106,55 @@ export class RHT { const prev = this.nodeMapByKey.get(key); if (prev === undefined || executedAt.after(prev.getUpdatedAt())) { - const node = RHTNode.of(key, value, executedAt); + if (prev !== undefined && !prev.isRemoved()) { + this.numberOfRemovedElement -= 1; + } + const node = RHTNode.of(key, value, executedAt, false); this.nodeMapByKey.set(key, node); } } + /** + * `remove` removes the Element of the given key. + */ + public remove(key: string, executedAt: TimeTicket): string { + const prev = this.nodeMapByKey.get(key); + + if (prev === undefined || executedAt.after(prev.getUpdatedAt())) { + if (prev === undefined) { + this.numberOfRemovedElement += 1; + const node = RHTNode.of(key, '', executedAt, true); + this.nodeMapByKey.set(key, node); + + return ''; + } + + const alreadyRemoved = prev.isRemoved(); + if (!alreadyRemoved) { + this.numberOfRemovedElement += 1; + } + const node = RHTNode.of(key, prev.getValue(), executedAt, true); + this.nodeMapByKey.set(key, node); + + if (alreadyRemoved) { + return ''; + } + + return prev.getValue(); + } + + return ''; + } + /** * `has` returns whether the element exists of the given key or not. */ public has(key: string): boolean { - return this.nodeMapByKey.has(key); + if (this.nodeMapByKey.has(key)) { + const node = this.nodeMapByKey.get(key); + return node !== undefined && !node.isRemoved(); + } + return false; } /** @@ -123,9 +183,15 @@ export class RHT { * `toJSON` returns the JSON encoding of this hashtable. */ public toJSON(): string { + if (!this.size()) { + return '{}'; + } + const items = []; for (const [key, node] of this.nodeMapByKey) { - items.push(`"${escapeString(key)}":"${escapeString(node.getValue())}"`); + if (!node.isRemoved()) { + items.push(`"${escapeString(key)}":"${escapeString(node.getValue())}"`); + } } return `{${items.join(',')}}`; } @@ -139,6 +205,7 @@ export class RHT { } const attrs = [...this.nodeMapByKey] + .filter(([, v]) => v instanceof RHTNode && !v.isRemoved()) .sort((a, b) => a[0].localeCompare(b[0])) .map(([k, v]) => `${k}="${JSON.parse(v.getValue())}"`); @@ -149,7 +216,7 @@ export class RHT { * `size` returns the size of RHT */ public size(): number { - return this.nodeMapByKey.size; + return this.nodeMapByKey.size - this.numberOfRemovedElement; } /** @@ -158,7 +225,9 @@ export class RHT { public toObject(): Record { const obj: Record = {}; for (const [key, node] of this.nodeMapByKey) { - obj[key] = node.getValue(); + if (!node.isRemoved()) { + obj[key] = node.getValue(); + } } return obj; diff --git a/src/document/time/ticket.ts b/src/document/time/ticket.ts index 4c2ad74aa..633d13387 100644 --- a/src/document/time/ticket.ts +++ b/src/document/time/ticket.ts @@ -198,6 +198,13 @@ export const InitialTimeTicket = new TimeTicket( InitialDelimiter, InitialActorID, ); + +export const nextTimeTicket = new TimeTicket( + Long.fromNumber(1), + InitialDelimiter, + InitialActorID, +); + export const MaxTimeTicket = new TimeTicket( MaxLamport, MaxDelemiter, diff --git a/test/unit/document/crdt/rht_test.ts b/test/unit/document/crdt/rht_test.ts index 785582b6c..2fe714162 100644 --- a/test/unit/document/crdt/rht_test.ts +++ b/test/unit/document/crdt/rht_test.ts @@ -16,7 +16,10 @@ import { describe, it, assert } from 'vitest'; import { RHT } from '@yorkie-js-sdk/src/document/crdt/rht'; -import { InitialTimeTicket } from '@yorkie-js-sdk/src/document/time/ticket'; +import { + InitialTimeTicket, + nextTimeTicket, +} from '@yorkie-js-sdk/src/document/time/ticket'; import { Indexable } from '@yorkie-js-sdk/test/helper/helper'; describe('RHT', function () { @@ -39,6 +42,24 @@ describe('RHT', function () { assert.equal(notExistsValue, undefined); }); + it('should handle remove', function () { + const testKey = 'test-key'; + const testValue = 'test-value'; + + const rht = RHT.create(); + + assert.equal(rht.toJSON(), '{}'); + rht.set(testKey, testValue, InitialTimeTicket); + + const actualValue = rht.get(testKey); + assert.equal(actualValue, testValue); + assert.equal(rht.size(), 1); + + rht.remove(testKey, nextTimeTicket); + assert.equal(rht.has(testKey), false); + assert.equal(rht.size(), 0); + }); + it('should return undefined when a key does not exist', function () { const notExistsKey = 'not-exists-key'; From d49372b557647594e527c255a653d2dda7668af7 Mon Sep 17 00:00:00 2001 From: raararaara Date: Mon, 5 Feb 2024 02:01:59 +0900 Subject: [PATCH 2/8] Implement Tree.RemoveStyle and tests --- src/document/crdt/tree.ts | 50 ++++++++++++++- src/document/json/tree.ts | 33 ++++++++++ .../operation/tree_style_operation.ts | 63 ++++++++++++++++--- src/document/time/ticket.ts | 4 +- test/integration/tree_test.ts | 35 +++++++++++ test/unit/document/crdt/rht_test.ts | 4 +- 6 files changed, 175 insertions(+), 14 deletions(-) diff --git a/src/document/crdt/tree.ts b/src/document/crdt/tree.ts index 766113bf4..22515c44c 100644 --- a/src/document/crdt/tree.ts +++ b/src/document/crdt/tree.ts @@ -79,6 +79,7 @@ export type TreeNodeForTest = TreeNode & { export enum TreeChangeType { Content = 'content', Style = 'style', + RemoveStyle = 'removeStyle', } /** @@ -91,7 +92,7 @@ export interface TreeChange { to: number; fromPath: Array; toPath: Array; - value?: Array | { [key: string]: any }; + value?: Array | { [key: string]: any } | Array; splitLevel?: number; } @@ -784,6 +785,53 @@ export class CRDTTree extends CRDTGCElement { return changes; } + /** + * `removeStyle` removes the given attributes of the given range. + */ + public removeStyle( + range: [CRDTTreePos, CRDTTreePos], + attributesToRemove: Array, + editedAt: TimeTicket, + ) { + const [fromParent, fromLeft] = this.findNodesAndSplitText( + range[0], + editedAt, + ); + const [toParent, toLeft] = this.findNodesAndSplitText(range[1], editedAt); + + const changes: Array = []; + const value = attributesToRemove ? attributesToRemove : undefined; + this.traverseInPosRange( + fromParent, + fromLeft, + toParent, + toLeft, + ([node]) => { + if (!node.isRemoved && !node.isText && attributesToRemove) { + if (!node.attrs) { + node.attrs = new RHT(); + } + + for (const value of attributesToRemove) { + node.attrs.remove(value, editedAt); + } + + changes.push({ + type: TreeChangeType.RemoveStyle, + from: this.toIndex(fromParent, fromLeft), + to: this.toIndex(toParent, toLeft), + fromPath: this.toPath(fromParent, fromLeft), + toPath: this.toPath(toParent, toLeft), + actor: editedAt.getActorID()!, + value, + }); + } + }, + ); + + return changes; + } + /** * `edit` edits the tree with the given range and content. * If the content is undefined, the range will be removed. diff --git a/src/document/json/tree.ts b/src/document/json/tree.ts index d354a8689..db0d22762 100644 --- a/src/document/json/tree.ts +++ b/src/document/json/tree.ts @@ -326,6 +326,39 @@ export class Tree { ); } + /** + * `removeStyle` removes the attributes to the elements of the given range. + */ + public removeStyle( + fromIdx: number, + toIdx: number, + attributesToRemove: Array, + ) { + if (!this.context || !this.tree) { + throw new Error('it is not initialized yet'); + } + + if (fromIdx > toIdx) { + throw new Error('from should be less than or equal to to'); + } + + const fromPos = this.tree.findPos(fromIdx); + const toPos = this.tree.findPos(toIdx); + const ticket = this.context.issueTimeTicket(); + + this.tree!.removeStyle([fromPos, toPos], attributesToRemove, ticket); + + this.context.push( + TreeStyleOperation.createTreeRemoveStyleOperation( + this.tree.getCreatedAt(), + fromPos, + toPos, + attributesToRemove, + ticket, + ), + ); + } + private editInternal( fromPos: CRDTTreePos, toPos: CRDTTreePos, diff --git a/src/document/operation/tree_style_operation.ts b/src/document/operation/tree_style_operation.ts index 0580cae7d..c2a393d90 100644 --- a/src/document/operation/tree_style_operation.ts +++ b/src/document/operation/tree_style_operation.ts @@ -17,7 +17,11 @@ 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 { CRDTTree, CRDTTreePos } from '@yorkie-js-sdk/src/document/crdt/tree'; +import { + CRDTTree, + CRDTTreePos, + TreeChange, +} from '@yorkie-js-sdk/src/document/crdt/tree'; import { Operation, OperationInfo, @@ -32,18 +36,21 @@ export class TreeStyleOperation extends Operation { private fromPos: CRDTTreePos; private toPos: CRDTTreePos; private attributes: Map; + private attributesToRemove: Array; constructor( parentCreatedAt: TimeTicket, fromPos: CRDTTreePos, toPos: CRDTTreePos, attributes: Map, + attributesToRemove: Array, executedAt: TimeTicket, ) { super(parentCreatedAt, executedAt); this.fromPos = fromPos; this.toPos = toPos; this.attributes = attributes; + this.attributesToRemove = attributesToRemove; } /** @@ -61,6 +68,27 @@ export class TreeStyleOperation extends Operation { fromPos, toPos, attributes, + new Array(), + executedAt, + ); + } + + /** + * `createTreeRemoveStyleOperation` creates a new instance of TreeStyleOperation for style deletion. + */ + public static createTreeRemoveStyleOperation( + parentCreatedAt: TimeTicket, + fromPos: CRDTTreePos, + toPos: CRDTTreePos, + attributesToRemove: Array, + executedAt: TimeTicket, + ): TreeStyleOperation { + return new TreeStyleOperation( + parentCreatedAt, + fromPos, + toPos, + new Map(), + attributesToRemove, executedAt, ); } @@ -76,16 +104,26 @@ export class TreeStyleOperation extends Operation { if (!(parentObject instanceof CRDTTree)) { logger.fatal(`fail to execute, only Tree can execute edit`); } + const tree = parentObject as CRDTTree; + let changes: Array; + if (this.attributes.size) { + const attributes: { [key: string]: any } = {}; + [...this.attributes].forEach(([key, value]) => (attributes[key] = value)); - const attributes: { [key: string]: any } = {}; - [...this.attributes].forEach(([key, value]) => (attributes[key] = value)); + changes = tree.style( + [this.fromPos, this.toPos], + attributes, + this.getExecutedAt(), + ); + } else { + const attributesToRemove = this.attributesToRemove; - const tree = parentObject as CRDTTree; - const changes = tree.style( - [this.fromPos, this.toPos], - attributes, - this.getExecutedAt(), - ); + changes = tree.removeStyle( + [this.fromPos, this.toPos], + attributesToRemove, + this.getExecutedAt(), + ); + } return { opInfos: changes.map(({ from, to, value, fromPath }) => { @@ -149,4 +187,11 @@ export class TreeStyleOperation extends Operation { public getAttributes(): Map { return this.attributes!; } + + /** + * `getAttributesToRemove` returns the attributes of Style to remove. + */ + public getAttributesToRemove(): Array { + return this.attributesToRemove; + } } diff --git a/src/document/time/ticket.ts b/src/document/time/ticket.ts index 633d13387..8fd62231f 100644 --- a/src/document/time/ticket.ts +++ b/src/document/time/ticket.ts @@ -199,9 +199,9 @@ export const InitialTimeTicket = new TimeTicket( InitialActorID, ); -export const nextTimeTicket = new TimeTicket( +export const NextTimeTicket = new TimeTicket( Long.fromNumber(1), - InitialDelimiter, + InitialDelimiter + 1, InitialActorID, ); diff --git a/test/integration/tree_test.ts b/test/integration/tree_test.ts index 527ad4c46..8e2fcbfd2 100644 --- a/test/integration/tree_test.ts +++ b/test/integration/tree_test.ts @@ -1050,6 +1050,41 @@ describe('Tree.style', function () { ); }); + it('Can be deleted with attributesToRemove', function ({ task }) { + const doc = new yorkie.Document<{ t: Tree }>(toDocKey(task.name)); + doc.update((root) => { + root.t = new Tree({ + type: 'doc', + children: [ + { + type: 'p', + children: [ + { + type: 'span', + attributes: { bold: true }, + children: [{ type: 'text', value: 'hello' }], + }, + ], + }, + ], + }); + }); + + assert.equal( + doc.getRoot().t.toXML(), + /*html*/ `

hello

`, + ); + + doc.update((root) => { + root.t.removeStyle(1, 8, ['bold']); + }); + + assert.equal( + doc.getRoot().t.toXML(), + /*html*/ `

hello

`, + ); + }); + it('Can be edited with index', function ({ task }) { const key = toDocKey(`${task.name}-${new Date().getTime()}`); const doc = new yorkie.Document<{ t: Tree }>(key); diff --git a/test/unit/document/crdt/rht_test.ts b/test/unit/document/crdt/rht_test.ts index 2fe714162..61f760116 100644 --- a/test/unit/document/crdt/rht_test.ts +++ b/test/unit/document/crdt/rht_test.ts @@ -18,7 +18,7 @@ import { describe, it, assert } from 'vitest'; import { RHT } from '@yorkie-js-sdk/src/document/crdt/rht'; import { InitialTimeTicket, - nextTimeTicket, + NextTimeTicket, } from '@yorkie-js-sdk/src/document/time/ticket'; import { Indexable } from '@yorkie-js-sdk/test/helper/helper'; @@ -55,7 +55,7 @@ describe('RHT', function () { assert.equal(actualValue, testValue); assert.equal(rht.size(), 1); - rht.remove(testKey, nextTimeTicket); + rht.remove(testKey, NextTimeTicket); assert.equal(rht.has(testKey), false); assert.equal(rht.size(), 0); }); From d479d5502e6bc619bb2db6a9bbff66541c1cb6ae Mon Sep 17 00:00:00 2001 From: raararaara Date: Mon, 5 Feb 2024 15:57:42 +0900 Subject: [PATCH 3/8] Add test case of Tree.RemoveStyle --- test/integration/tree_test.ts | 50 ++++++++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/test/integration/tree_test.ts b/test/integration/tree_test.ts index 8e2fcbfd2..704f89d5b 100644 --- a/test/integration/tree_test.ts +++ b/test/integration/tree_test.ts @@ -1050,7 +1050,7 @@ describe('Tree.style', function () { ); }); - it('Can be deleted with attributesToRemove', function ({ task }) { + it('Can be edited removal with index', function ({ task }) { const doc = new yorkie.Document<{ t: Tree }>(toDocKey(task.name)); doc.update((root) => { root.t = new Tree({ @@ -1085,6 +1085,54 @@ describe('Tree.style', function () { ); }); + it('Can handle removal of attributes that do not exist', function ({ task }) { + const doc = new yorkie.Document<{ t: Tree }>(toDocKey(task.name)); + doc.update((root) => { + root.t = new Tree({ + type: 'doc', + children: [ + { + type: 'p', + children: [ + { + type: 'span', + attributes: { bold: true }, + children: [{ type: 'text', value: 'hello' }], + }, + { + type: 'span', + children: [{ type: 'text', value: 'hi' }], + }, + ], + }, + ], + }); + }); + + assert.equal( + doc.getRoot().t.toXML(), + /*html*/ `

hellohi

`, + ); + + doc.update((root) => { + root.t.removeStyle(1, 12, ['italic']); + }); + + assert.equal( + doc.getRoot().t.toXML(), + /*html*/ `

hellohi

`, + ); + + doc.update((root) => { + root.t.removeStyle(1, 8, ['italic', 'bold']); + }); + + assert.equal( + doc.getRoot().t.toXML(), + /*html*/ `

hellohi

`, + ); + }); + it('Can be edited with index', function ({ task }) { const key = toDocKey(`${task.name}-${new Date().getTime()}`); const doc = new yorkie.Document<{ t: Tree }>(key); From 635e07ae03e986dfc81a42ce3afd47a06207bb14 Mon Sep 17 00:00:00 2001 From: raararaara Date: Thu, 22 Feb 2024 22:44:43 +0900 Subject: [PATCH 4/8] Fix protobuf to support Tree.RemoveStyle feature --- src/api/converter.ts | 47 +++++++++++++------ src/api/yorkie/v1/resources.proto | 1 + src/api/yorkie/v1/resources_pb.d.ts | 5 ++ src/api/yorkie/v1/resources_pb.js | 1 + src/document/crdt/rht.ts | 3 +- .../operation/tree_style_operation.ts | 2 +- 6 files changed, 42 insertions(+), 17 deletions(-) diff --git a/src/api/converter.ts b/src/api/converter.ts index 2b9892568..25713d199 100644 --- a/src/api/converter.ts +++ b/src/api/converter.ts @@ -445,10 +445,15 @@ function toOperation(operation: Operation): PbOperation { pbTreeStyleOperation.from = toTreePos(treeStyleOperation.getFromPos()); pbTreeStyleOperation.to = toTreePos(treeStyleOperation.getToPos()); - const attributesMap = pbTreeStyleOperation.attributes; + const attributesToRemove = treeStyleOperation.getAttributesToRemove(); + if (attributesToRemove.length > 0) { + pbTreeStyleOperation.attributesToRemove = attributesToRemove; + } else { + const attributesMap = pbTreeStyleOperation.attributes; - for (const [key, value] of treeStyleOperation.getAttributes()) { - attributesMap[key] = value; + for (const [key, value] of treeStyleOperation.getAttributes()) { + attributesMap[key] = value; + } } pbTreeStyleOperation.executedAt = toTimeTicket( treeStyleOperation.getExecutedAt(), @@ -1176,18 +1181,30 @@ function fromOperations(pbOperations: Array): Array { const pbTreeStyleOperation = pbOperation.body.value; const attributes = new Map(); - Object.entries(pbTreeStyleOperation!.attributes).forEach( - ([key, value]) => { - attributes.set(key, value); - }, - ); - operation = TreeStyleOperation.create( - fromTimeTicket(pbTreeStyleOperation!.parentCreatedAt)!, - fromTreePos(pbTreeStyleOperation!.from!), - fromTreePos(pbTreeStyleOperation!.to!), - attributes, - fromTimeTicket(pbTreeStyleOperation!.executedAt)!, - ); + const attributesToRemove = pbTreeStyleOperation.attributesToRemove; + if (attributesToRemove.length > 0) { + operation = TreeStyleOperation.createTreeRemoveStyleOperation( + fromTimeTicket(pbTreeStyleOperation!.parentCreatedAt)!, + fromTreePos(pbTreeStyleOperation!.from!), + fromTreePos(pbTreeStyleOperation!.to!), + attributesToRemove, + fromTimeTicket(pbTreeStyleOperation!.executedAt)!, + ); + } else { + Object.entries(pbTreeStyleOperation!.attributes).forEach( + ([key, value]) => { + attributes.set(key, value); + }, + ); + + operation = TreeStyleOperation.create( + fromTimeTicket(pbTreeStyleOperation!.parentCreatedAt)!, + fromTreePos(pbTreeStyleOperation!.from!), + fromTreePos(pbTreeStyleOperation!.to!), + attributes, + fromTimeTicket(pbTreeStyleOperation!.executedAt)!, + ); + } } else { throw new YorkieError(Code.Unimplemented, `unimplemented operation`); } diff --git a/src/api/yorkie/v1/resources.proto b/src/api/yorkie/v1/resources.proto index b98bd59bc..1b186a94a 100644 --- a/src/api/yorkie/v1/resources.proto +++ b/src/api/yorkie/v1/resources.proto @@ -133,6 +133,7 @@ message Operation { TreePos to = 3; map attributes = 4; TimeTicket executed_at = 5; + repeated string attributes_to_remove = 6; } oneof body { diff --git a/src/api/yorkie/v1/resources_pb.d.ts b/src/api/yorkie/v1/resources_pb.d.ts index 6daefb702..4aa43c935 100644 --- a/src/api/yorkie/v1/resources_pb.d.ts +++ b/src/api/yorkie/v1/resources_pb.d.ts @@ -783,6 +783,11 @@ export declare class Operation_TreeStyle extends Message { */ executedAt?: TimeTicket; + /** + * @generated from field: repeated string attributes_to_remove = 6; + */ + attributesToRemove: string[]; + constructor(data?: PartialMessage); static readonly runtime: typeof proto3; diff --git a/src/api/yorkie/v1/resources_pb.js b/src/api/yorkie/v1/resources_pb.js index dd58b9bc0..7c1203040 100644 --- a/src/api/yorkie/v1/resources_pb.js +++ b/src/api/yorkie/v1/resources_pb.js @@ -285,6 +285,7 @@ const Operation_TreeStyle = proto3.makeMessageType( { no: 3, name: "to", kind: "message", T: TreePos }, { no: 4, name: "attributes", kind: "map", K: 9 /* ScalarType.STRING */, V: {kind: "scalar", T: 9 /* ScalarType.STRING */} }, { no: 5, name: "executed_at", kind: "message", T: TimeTicket }, + { no: 6, name: "attributes_to_remove", kind: "scalar", T: 9 /* ScalarType.STRING */, repeated: true }, ], {localName: "Operation_TreeStyle"}, ); diff --git a/src/document/crdt/rht.ts b/src/document/crdt/rht.ts index b06a88ce7..abe8b1caf 100644 --- a/src/document/crdt/rht.ts +++ b/src/document/crdt/rht.ts @@ -106,7 +106,7 @@ export class RHT { const prev = this.nodeMapByKey.get(key); if (prev === undefined || executedAt.after(prev.getUpdatedAt())) { - if (prev !== undefined && !prev.isRemoved()) { + if (prev !== undefined && prev.isRemoved()) { this.numberOfRemovedElement -= 1; } const node = RHTNode.of(key, value, executedAt, false); @@ -236,6 +236,7 @@ export class RHT { // eslint-disable-next-line jsdoc/require-jsdoc public *[Symbol.iterator](): IterableIterator { for (const [, node] of this.nodeMapByKey) { + if (node.isRemoved()) continue; yield node as RHTNode; } } diff --git a/src/document/operation/tree_style_operation.ts b/src/document/operation/tree_style_operation.ts index c2a393d90..297d486b0 100644 --- a/src/document/operation/tree_style_operation.ts +++ b/src/document/operation/tree_style_operation.ts @@ -192,6 +192,6 @@ export class TreeStyleOperation extends Operation { * `getAttributesToRemove` returns the attributes of Style to remove. */ public getAttributesToRemove(): Array { - return this.attributesToRemove; + return this.attributesToRemove!; } } From 1aca137cc7eb40318f3011a0b5fb22179c2d4b1a Mon Sep 17 00:00:00 2001 From: raararaara Date: Thu, 22 Feb 2024 23:11:56 +0900 Subject: [PATCH 5/8] Erase duplicate code --- src/document/crdt/tree.ts | 47 --------------------------------------- 1 file changed, 47 deletions(-) diff --git a/src/document/crdt/tree.ts b/src/document/crdt/tree.ts index 877eb4a4c..2d9c14662 100644 --- a/src/document/crdt/tree.ts +++ b/src/document/crdt/tree.ts @@ -832,53 +832,6 @@ export class CRDTTree extends CRDTGCElement { return changes; } - /** - * `removeStyle` removes the given attributes of the given range. - */ - public removeStyle( - range: [CRDTTreePos, CRDTTreePos], - attributesToRemove: Array, - editedAt: TimeTicket, - ) { - const [fromParent, fromLeft] = this.findNodesAndSplitText( - range[0], - editedAt, - ); - const [toParent, toLeft] = this.findNodesAndSplitText(range[1], editedAt); - - const changes: Array = []; - const value = attributesToRemove ? attributesToRemove : undefined; - this.traverseInPosRange( - fromParent, - fromLeft, - toParent, - toLeft, - ([node]) => { - if (!node.isRemoved && !node.isText && attributesToRemove) { - if (!node.attrs) { - node.attrs = new RHT(); - } - - for (const value of attributesToRemove) { - node.attrs.remove(value, editedAt); - } - - changes.push({ - type: TreeChangeType.RemoveStyle, - from: this.toIndex(fromParent, fromLeft), - to: this.toIndex(toParent, toLeft), - fromPath: this.toPath(fromParent, fromLeft), - toPath: this.toPath(toParent, toLeft), - actor: editedAt.getActorID()!, - value, - }); - } - }, - ); - - return changes; - } - /** * `edit` edits the tree with the given range and content. * If the content is undefined, the range will be removed. From 9f3a424376c6fcb14a623ae34ad7e1283a6bbc86 Mon Sep 17 00:00:00 2001 From: raararaara Date: Fri, 8 Mar 2024 13:49:09 +0900 Subject: [PATCH 6/8] Add test code --- src/util/index_tree.ts | 2 +- test/integration/tree_test.ts | 36 +++++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/util/index_tree.ts b/src/util/index_tree.ts index 07c9bbd9c..88d80e38e 100644 --- a/src/util/index_tree.ts +++ b/src/util/index_tree.ts @@ -809,7 +809,7 @@ export class IndexTree> { path.push(sizeOfLeftSiblings + treePos.offset); node = node.parent!; } else if (node.hasTextChild()) { - // TODO(hackerwins): The function does not consider the situation + // TODO(hackerwins): The function does not consider the situation // where Element and Text nodes are mixed in the Element's Children. const sizeOfLeftSiblings = addSizeOfLeftSiblings( node! as T, diff --git a/test/integration/tree_test.ts b/test/integration/tree_test.ts index b04ac3cd1..2baf3f6f0 100644 --- a/test/integration/tree_test.ts +++ b/test/integration/tree_test.ts @@ -1334,6 +1334,42 @@ describe('Tree.style', function () { }); }); + it('Can sync its content with remove style', async function ({ task }) { + await withTwoClientsAndDocuments<{ t: Tree }>(async (c1, d1, c2, d2) => { + d1.update((root) => { + root.t = new Tree({ + type: 'doc', + children: [ + { + type: 'p', + children: [{ type: 'text', value: 'hello' }], + attributes: { italic: 'true' }, + }, + ], + }); + }); + await c1.sync(); + await c2.sync(); + assert.equal( + d1.getRoot().t.toXML(), + /*html*/ `

hello

`, + ); + assert.equal( + d2.getRoot().t.toXML(), + /*html*/ `

hello

`, + ); + + d1.update((root) => { + root.t.removeStyle(0, 1, ['italic']); + }); + await c1.sync(); + await c2.sync(); + + assert.equal(d1.getRoot().t.toXML(), /*html*/ `

hello

`); + assert.equal(d2.getRoot().t.toXML(), /*html*/ `

hello

`); + }, task.name); + }); + it('Should return correct range path within doc.subscribe', async function ({ task, }) { From faa66935c8b0fa2fa9349b2be478868376324827 Mon Sep 17 00:00:00 2001 From: raararaara Date: Thu, 14 Mar 2024 14:39:02 +0900 Subject: [PATCH 7/8] Add comment related to gc in `RHT.set` --- src/document/crdt/rht.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/document/crdt/rht.ts b/src/document/crdt/rht.ts index abe8b1caf..f777d75bd 100644 --- a/src/document/crdt/rht.ts +++ b/src/document/crdt/rht.ts @@ -106,6 +106,7 @@ export class RHT { const prev = this.nodeMapByKey.get(key); if (prev === undefined || executedAt.after(prev.getUpdatedAt())) { + // NOTE(raararaara): This logic is implemented without considering gc. if (prev !== undefined && prev.isRemoved()) { this.numberOfRemovedElement -= 1; } From 64f74fbf00d5a78d08401679d9db006c366146df Mon Sep 17 00:00:00 2001 From: raararaara Date: Thu, 14 Mar 2024 18:33:09 +0900 Subject: [PATCH 8/8] Remove unnecessary logic from now --- src/document/crdt/rht.ts | 4 +--- src/document/operation/tree_style_operation.ts | 4 ++-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/document/crdt/rht.ts b/src/document/crdt/rht.ts index f777d75bd..b06a88ce7 100644 --- a/src/document/crdt/rht.ts +++ b/src/document/crdt/rht.ts @@ -106,8 +106,7 @@ export class RHT { const prev = this.nodeMapByKey.get(key); if (prev === undefined || executedAt.after(prev.getUpdatedAt())) { - // NOTE(raararaara): This logic is implemented without considering gc. - if (prev !== undefined && prev.isRemoved()) { + if (prev !== undefined && !prev.isRemoved()) { this.numberOfRemovedElement -= 1; } const node = RHTNode.of(key, value, executedAt, false); @@ -237,7 +236,6 @@ export class RHT { // eslint-disable-next-line jsdoc/require-jsdoc public *[Symbol.iterator](): IterableIterator { for (const [, node] of this.nodeMapByKey) { - if (node.isRemoved()) continue; yield node as RHTNode; } } diff --git a/src/document/operation/tree_style_operation.ts b/src/document/operation/tree_style_operation.ts index 297d486b0..a83c2da29 100644 --- a/src/document/operation/tree_style_operation.ts +++ b/src/document/operation/tree_style_operation.ts @@ -185,13 +185,13 @@ export class TreeStyleOperation extends Operation { * `getAttributes` returns the attributes of Style. */ public getAttributes(): Map { - return this.attributes!; + return this.attributes; } /** * `getAttributesToRemove` returns the attributes of Style to remove. */ public getAttributesToRemove(): Array { - return this.attributesToRemove!; + return this.attributesToRemove; } }