diff --git a/src/api/converter.ts b/src/api/converter.ts index 9f3ec47ca..6f5bc14ca 100644 --- a/src/api/converter.ts +++ b/src/api/converter.ts @@ -1059,13 +1059,14 @@ function fromTreeNodes( function fromTreeNode(pbTreeNode: PbTreeNode): CRDTTreeNode { const id = fromTreeNodeID(pbTreeNode.id!); const node = CRDTTreeNode.create(id, pbTreeNode.type); + const pbAttrs = Object.entries(pbTreeNode.attributes); if (node.isText) { node.value = pbTreeNode.value; - } else { + } else if (pbAttrs.length) { const attrs = RHT.create(); - Object.entries(pbTreeNode.attributes).forEach(([key, value]) => { + for (const [key, value] of pbAttrs) { attrs.set(key, value.value, fromTimeTicket(value.updatedAt)!); - }); + } node.attrs = attrs; } @@ -1203,6 +1204,7 @@ function fromOperation(pbOperation: PbOperation): Operation | undefined { fromTimeTicket(pbTreeStyleOperation!.parentCreatedAt)!, fromTreePos(pbTreeStyleOperation!.from!), fromTreePos(pbTreeStyleOperation!.to!), + createdAtMapByActor, attributesToRemove, fromTimeTicket(pbTreeStyleOperation!.executedAt)!, ); diff --git a/src/document/crdt/tree.ts b/src/document/crdt/tree.ts index 6766ceae9..6f21a78b4 100644 --- a/src/document/crdt/tree.ts +++ b/src/document/crdt/tree.ts @@ -212,12 +212,12 @@ export class CRDTTreePos { } /** - * `toTreeNodes` converts the pos to parent and left sibling nodes. + * `toTreeNodePair` converts the pos to parent and left sibling nodes. * If the position points to the middle of a node, then the left sibling node * is the node that contains the position. Otherwise, the left sibling node is * the node that is located at the left of the position. */ - public toTreeNodes(tree: CRDTTree): [CRDTTreeNode, CRDTTreeNode] { + public toTreeNodePair(tree: CRDTTree): TreeNodePair { const parentID = this.getParentID(); const leftSiblingID = this.getLeftSiblingID(); const parentNode = tree.findFloorNode(parentID); @@ -409,6 +409,12 @@ export type CRDTTreeNodeIDStruct = { */ export type TreePosRange = [CRDTTreePos, CRDTTreePos]; +/** + * `TreeNodePair` represents a pair of CRDTTreeNode. It represents the position + * of the node in the tree with the left and parent nodes. + */ +type TreeNodePair = [CRDTTreeNode, CRDTTreeNode]; + /** * `TreePosStructRange` represents the structure of TreeRange. * It is used to serialize and deserialize the TreeRange. @@ -689,7 +695,7 @@ export class CRDTTreeNode } /** - * toTreeNode converts the given CRDTTreeNode to TreeNode. + * `toTreeNode` converts the given CRDTTreeNode to TreeNode. */ function toTreeNode(node: CRDTTreeNode): TreeNode { if (node.isText) { @@ -700,17 +706,20 @@ function toTreeNode(node: CRDTTreeNode): TreeNode { } as TextNode; } - return { + const treeNode: TreeNode = { type: node.type, children: node.children.map(toTreeNode), - attributes: node.attrs - ? parseObjectValues(node.attrs?.toObject()) - : undefined, }; + + if (node.attrs) { + treeNode.attributes = parseObjectValues(node.attrs?.toObject()); + } + + return treeNode; } /** - * toXML converts the given CRDTNode to XML string. + * `toXML` converts the given CRDTNode to XML string. */ export function toXML(node: CRDTTreeNode): string { if (node.isText) { @@ -819,9 +828,9 @@ export class CRDTTree extends CRDTElement implements GCParent { public findNodesAndSplitText( pos: CRDTTreePos, editedAt?: TimeTicket, - ): [CRDTTreeNode, CRDTTreeNode] { + ): TreeNodePair { // 01. Find the parent and left sibling node of the given position. - const [parent, leftSibling] = pos.toTreeNodes(this); + const [parent, leftSibling] = pos.toTreeNodePair(this); let leftNode = leftSibling; // 02. Determine whether the position is left-most and the exact parent @@ -912,13 +921,16 @@ export class CRDTTree extends CRDTElement implements GCParent { {}, ); + const parentOfNode = node.parent!; + const previousNode = node.prevSibling || node.parent!; + if (Object.keys(affectedAttrs).length > 0) { changes.push({ type: TreeChangeType.Style, - from: this.toIndex(fromParent, fromLeft), - to: this.toIndex(toParent, toLeft), - fromPath: this.toPath(fromParent, fromLeft), - toPath: this.toPath(toParent, toLeft), + from: this.toIndex(parentOfNode, previousNode), + to: this.toIndex(node, node), + fromPath: this.toPath(parentOfNode, previousNode), + toPath: this.toPath(node, node), actor: editedAt.getActorID(), value: affectedAttrs, }); @@ -943,7 +955,8 @@ export class CRDTTree extends CRDTElement implements GCParent { range: [CRDTTreePos, CRDTTreePos], attributesToRemove: Array, editedAt: TimeTicket, - ): [Array, Array] { + maxCreatedAtMapByActor?: Map, + ): [Map, Array, Array] { const [fromParent, fromLeft] = this.findNodesAndSplitText( range[0], editedAt, @@ -951,6 +964,7 @@ export class CRDTTree extends CRDTElement implements GCParent { const [toParent, toLeft] = this.findNodesAndSplitText(range[1], editedAt); const changes: Array = []; + const createdAtMapByActor = new Map(); const pairs: Array = []; this.traverseInPosRange( fromParent, @@ -958,7 +972,20 @@ export class CRDTTree extends CRDTElement implements GCParent { toParent, toLeft, ([node]) => { - if (!node.isRemoved && !node.isText && attributesToRemove) { + const actorID = node.getCreatedAt().getActorID(); + const maxCreatedAt = maxCreatedAtMapByActor + ? maxCreatedAtMapByActor!.has(actorID) + ? maxCreatedAtMapByActor!.get(actorID)! + : InitialTimeTicket + : MaxTimeTicket; + + if (node.canStyle(editedAt, maxCreatedAt) && attributesToRemove) { + const maxCreatedAt = createdAtMapByActor!.get(actorID); + const createdAt = node.getCreatedAt(); + if (!maxCreatedAt || createdAt.after(maxCreatedAt)) { + createdAtMapByActor.set(actorID, createdAt); + } + if (!node.attrs) { node.attrs = new RHT(); } @@ -970,20 +997,23 @@ export class CRDTTree extends CRDTElement implements GCParent { } } + const parentOfNode = node.parent!; + const previousNode = node.prevSibling || node.parent!; + changes.push({ actor: editedAt.getActorID()!, type: TreeChangeType.RemoveStyle, - from: this.toIndex(fromParent, fromLeft), - to: this.toIndex(toParent, toLeft), - fromPath: this.toPath(fromParent, fromLeft), - toPath: this.toPath(toParent, toLeft), + from: this.toIndex(parentOfNode, previousNode), + to: this.toIndex(node, node), + fromPath: this.toPath(parentOfNode, previousNode), + toPath: this.toPath(node, node), value: attributesToRemove, }); } }, ); - return [pairs, changes]; + return [createdAtMapByActor, pairs, changes]; } /** diff --git a/src/document/json/tree.ts b/src/document/json/tree.ts index c43394460..94c6ce06d 100644 --- a/src/document/json/tree.ts +++ b/src/document/json/tree.ts @@ -371,7 +371,7 @@ export class Tree { const toPos = this.tree.findPos(toIdx); const ticket = this.context.issueTimeTicket(); - const [pairs] = this.tree!.removeStyle( + const [maxCreationMapByActor, pairs] = this.tree!.removeStyle( [fromPos, toPos], attributesToRemove, ticket, @@ -386,6 +386,7 @@ export class Tree { this.tree.getCreatedAt(), fromPos, toPos, + maxCreationMapByActor, attributesToRemove, ticket, ), diff --git a/src/document/operation/operation.ts b/src/document/operation/operation.ts index dbe691a0b..556b814fa 100644 --- a/src/document/operation/operation.ts +++ b/src/document/operation/operation.ts @@ -150,7 +150,7 @@ export type TreeEditOpInfo = { from: number; to: number; value?: Array; - splitLevel: number; + splitLevel?: number; fromPath: Array; toPath: Array; }; @@ -164,7 +164,14 @@ export type TreeStyleOpInfo = { from: number; to: number; fromPath: Array; - value: { [key: string]: any }; + toPath: Array; + value: + | { + attributes: Indexable; + } + | { + attributesToRemove: Array; + }; }; /** diff --git a/src/document/operation/tree_style_operation.ts b/src/document/operation/tree_style_operation.ts index 807226e7b..5f16f92a4 100644 --- a/src/document/operation/tree_style_operation.ts +++ b/src/document/operation/tree_style_operation.ts @@ -86,6 +86,7 @@ export class TreeStyleOperation extends Operation { parentCreatedAt: TimeTicket, fromPos: CRDTTreePos, toPos: CRDTTreePos, + maxCreatedAtMapByActor: Map, attributesToRemove: Array, executedAt: TimeTicket, ): TreeStyleOperation { @@ -93,7 +94,7 @@ export class TreeStyleOperation extends Operation { parentCreatedAt, fromPos, toPos, - new Map(), + maxCreatedAtMapByActor, new Map(), attributesToRemove, executedAt, @@ -127,10 +128,11 @@ export class TreeStyleOperation extends Operation { } else { const attributesToRemove = this.attributesToRemove; - [pairs, changes] = tree.removeStyle( + [, pairs, changes] = tree.removeStyle( [this.fromPos, this.toPos], attributesToRemove, this.getExecutedAt(), + this.maxCreatedAtMapByActor, ); } @@ -139,13 +141,16 @@ export class TreeStyleOperation extends Operation { } return { - opInfos: changes.map(({ from, to, value, fromPath }) => { + opInfos: changes.map(({ from, to, value, fromPath, toPath }) => { return { type: 'tree-style', from, to, - value, + value: this.attributes.size + ? { attributes: value } + : { attributesToRemove: value }, fromPath, + toPath, path: root.createPath(this.getParentCreatedAt()), } as OperationInfo; }), diff --git a/src/util/index_tree.ts b/src/util/index_tree.ts index c07dc1787..420ece679 100644 --- a/src/util/index_tree.ts +++ b/src/util/index_tree.ts @@ -203,6 +203,19 @@ export abstract class IndexTreeNode> { return undefined; } + /** + * `prevSibling` returns the previous sibling of the node. + */ + get prevSibling(): T | undefined { + const offset = this.parent!.findOffset(this as any); + const sibling = this.parent!.children[offset - 1]; + if (sibling) { + return sibling; + } + + return undefined; + } + /** * `isRemoved` returns true if the node is removed. */ diff --git a/test/integration/tree_concurrency_test.ts b/test/integration/tree_concurrency_test.ts index 35ad9235a..971b97a45 100644 --- a/test/integration/tree_concurrency_test.ts +++ b/test/integration/tree_concurrency_test.ts @@ -604,7 +604,7 @@ describe('Tree.concurrency', () => { const content: TreeNode = { type: 'p', children: [{ type: 'text', value: 'd' }], - attributes: { italic: 'true' }, + attributes: { italic: 'true', color: 'blue' }, }; const rangesArr = [ @@ -675,7 +675,7 @@ describe('Tree.concurrency', () => { StyleOpCode.StyleRemove, 'color', '', - 'remove-bold', + 'remove-color', ), new StyleOperationType( RangeSelector.RangeAll, diff --git a/test/integration/tree_test.ts b/test/integration/tree_test.ts index 42b75361f..b61e83e41 100644 --- a/test/integration/tree_test.ts +++ b/test/integration/tree_test.ts @@ -4586,6 +4586,129 @@ describe('TreeChange', () => { }, task.name); }); + it('Concurrent insert and 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: [] }], + }); + }); + await c1.sync(); + await c2.sync(); + assert.equal(d1.getRoot().t.toXML(), d2.getRoot().t.toXML()); + assert.equal(d1.getRoot().t.toXML(), /*html*/ `

`); + + const [ops1, ops2] = subscribeDocs(d1, d2); + + d1.update((root) => root.t.style(0, 1, { key: 'a' })); + d1.update((root) => root.t.style(0, 1, { key: 'a' })); + d2.update((root) => root.t.edit(0, 0, { type: 'p', children: [] })); + await c1.sync(); + await c2.sync(); + await c1.sync(); + assert.equal(d1.getRoot().t.toXML(), d2.getRoot().t.toXML()); + assert.equal( + d1.getRoot().t.toXML(), + /*html*/ `

`, + ); + + const editChange: TreeEditOpInfo = { + type: 'tree-edit', + path: '$.t', + from: 0, + to: 0, + fromPath: [0], + toPath: [0], + value: [{ children: [], type: 'p' }], + splitLevel: undefined, + }; + const styleChange: TreeStyleOpInfo = { + type: 'tree-style', + path: '$.t', + from: 0, + to: 1, + fromPath: [0], + toPath: [0, 0], + value: { attributes: { key: 'a' } }, + }; + const styleChange2: TreeStyleOpInfo = { + ...styleChange, + from: 2, + to: 3, + fromPath: [1], + toPath: [1, 0], + }; + + assert.deepEqual(ops1, [styleChange, styleChange, editChange]); + assert.deepEqual(ops2, [editChange, styleChange2, styleChange2]); + }, task.name); + }); + + it('Concurrent insert and removeStyle', async function ({ task }) { + await withTwoClientsAndDocuments<{ t: Tree }>(async (c1, d1, c2, d2) => { + d1.update((root) => { + root.t = new Tree({ + type: 'doc', + children: [{ type: 'p', attributes: { key: 'a' }, children: [] }], + }); + }); + await c1.sync(); + await c2.sync(); + assert.equal(d1.getRoot().t.toXML(), d2.getRoot().t.toXML()); + assert.equal( + d1.getRoot().t.toXML(), + /*html*/ `

`, + ); + + const [ops1, ops2] = subscribeDocs(d1, d2); + + d1.update((root) => root.t.removeStyle(0, 1, ['key'])); + d1.update((root) => root.t.removeStyle(0, 1, ['key'])); + d2.update((root) => root.t.edit(0, 0, { type: 'p', children: [] })); + await c1.sync(); + await c2.sync(); + await c1.sync(); + assert.equal(d1.getRoot().t.toXML(), d2.getRoot().t.toXML()); + assert.equal( + d1.getRoot().t.toXML(), + /*html*/ `

`, + ); + + const editChange: TreeEditOpInfo = { + type: 'tree-edit', + path: '$.t', + from: 0, + to: 0, + fromPath: [0], + toPath: [0], + value: [{ children: [], type: 'p' }], + splitLevel: undefined, + }; + const styleChange: TreeStyleOpInfo = { + type: 'tree-style', + path: '$.t', + from: 0, + to: 1, + fromPath: [0], + toPath: [0, 0], + value: { + attributesToRemove: ['key'], + }, + }; + const styleChange2: TreeStyleOpInfo = { + ...styleChange, + from: 2, + to: 3, + fromPath: [1], + toPath: [1, 0], + }; + + assert.deepEqual(ops1, [styleChange, styleChange, editChange]); + assert.deepEqual(ops2, [editChange, styleChange2, styleChange2]); + }, task.name); + }); + it('Concurrent delete and style', async function ({ task }) { await withTwoClientsAndDocuments<{ t: Tree }>(async (c1, d1, c2, d2) => { d1.update((root) => { @@ -4627,7 +4750,7 @@ describe('TreeChange', () => { type: 'tree-style', from: 0, to: 1, - value: { value: 'changed' }, + value: { attributes: { value: 'changed' } }, }, { type: 'tree-edit', @@ -4687,8 +4810,18 @@ describe('TreeChange', () => { return { type: it.type, from: it.from, to: it.to, value: it.value }; }), [ - { type: 'tree-style', from: 0, to: 1, value: { bold: 'true' } }, - { type: 'tree-style', from: 0, to: 1, value: { bold: 'false' } }, + { + type: 'tree-style', + from: 0, + to: 1, + value: { attributes: { bold: 'true' } }, + }, + { + type: 'tree-style', + from: 0, + to: 1, + value: { attributes: { bold: 'false' } }, + }, ], ); @@ -4696,7 +4829,14 @@ describe('TreeChange', () => { ops2.map((it) => { return { type: it.type, from: it.from, to: it.to, value: it.value }; }), - [{ type: 'tree-style', from: 0, to: 1, value: { bold: 'false' } }], + [ + { + type: 'tree-style', + from: 0, + to: 1, + value: { attributes: { bold: 'false' } }, + }, + ], ); }, task.name); }); @@ -4713,27 +4853,11 @@ function subscribeDocs( const ops2: Array = []; d1.subscribe('$.t', (event) => { - if (event.type === 'local-change' || event.type === 'remote-change') { - const { operations } = event.value; - - ops1.push( - ...(operations.filter( - (op) => op.type === 'tree-edit' || op.type === 'tree-style', - ) as Array), - ); - } + ops1.push(...event.value.operations); }); d2.subscribe('$.t', (event) => { - if (event.type === 'local-change' || event.type === 'remote-change') { - const { operations } = event.value; - - ops2.push( - ...(operations.filter( - (op) => op.type === 'tree-edit' || op.type === 'tree-style', - ) as Array), - ); - } + ops2.push(...event.value.operations); }); return [ops1, ops2];