From 9e0c431c727e874bfa0c298eac8438cc47144c6a Mon Sep 17 00:00:00 2001 From: ehua <111539329+ehuas@users.noreply.github.com> Date: Wed, 19 Jul 2023 17:20:25 +0900 Subject: [PATCH] Modify tree.edit to allow multiple nodes at a time (#576) Co-authored-by: JOOHOJANG --- src/document/crdt/tree.ts | 32 ++--- src/document/json/tree.ts | 164 +++++++++++++++--------- test/integration/tree_test.ts | 227 +++++++++++++++++++++++++++++++--- 3 files changed, 338 insertions(+), 85 deletions(-) diff --git a/src/document/crdt/tree.ts b/src/document/crdt/tree.ts index 776322277..69d3dad9f 100644 --- a/src/document/crdt/tree.ts +++ b/src/document/crdt/tree.ts @@ -639,26 +639,30 @@ export class CRDTTree extends CRDTGCElement { } } - // TODO(ehuas): Fix here // 03. insert the given node at the given position. if (contents?.length) { // 03-1. insert the content nodes to the list. let previous = fromRight!.prev!; - traverse(contents[0], (node) => { - this.insertAfter(previous, node); - previous = node; - }); - - // 03-2. insert the content nodes to the tree. - if (fromPos.node.isText) { - if (fromPos.offset === 0) { - fromPos.node.parent!.insertBefore(contents[0], fromPos.node); + const node = fromPos.node; + let offset = fromPos.offset; + + for (const content of contents!) { + traverse(content, (node) => { + this.insertAfter(previous, node); + previous = node; + }); + + // 03-2. insert the content nodes to the tree. + if (node.isText) { + if (fromPos.offset === 0) { + node.parent!.insertBefore(content, node); + } else { + node.parent!.insertAfter(content, node); + } } else { - fromPos.node.parent!.insertAfter(contents[0], fromPos.node); + node.insertAt(content, offset); + offset++; } - } else { - const target = fromPos.node; - target.insertAt(contents[0], fromPos.offset); } } diff --git a/src/document/json/tree.ts b/src/document/json/tree.ts index 25ec15705..0c66e8376 100644 --- a/src/document/json/tree.ts +++ b/src/document/json/tree.ts @@ -56,7 +56,8 @@ function buildDescendants( const { type } = treeNode; const ticket = context.issueTimeTicket(); - if (type === 'text') { + if (type === DefaultTextType) { + validateTextNode(treeNode as TextNode); const { value } = treeNode as TextNode; const textNode = CRDTTreeNode.create( CRDTTreePos.of(ticket, 0), @@ -101,7 +102,7 @@ function createCRDTTreeNode(context: ChangeContext, content: TreeNode) { const ticket = context.issueTimeTicket(); let root; - if (content.type === 'text') { + if (content.type === DefaultTextType) { const { value } = content as TextNode; root = CRDTTreeNode.create(CRDTTreePos.of(ticket, 0), type, value); } else if (content) { @@ -133,6 +134,47 @@ function createCRDTTreeNode(context: ChangeContext, content: TreeNode) { return root; } +/** + * `validateTextNode` ensures that a text node has a non-empty string value. + */ +function validateTextNode(textNode: TextNode): boolean { + if (!textNode.value.length) { + throw new Error('text node cannot have empty value'); + } else { + return true; + } +} + +/** + * `validateTreeNodes` ensures that treeNodes consists of only one type. + */ +function validateTreeNodes(treeNodes: Array): boolean { + if (treeNodes.length) { + const firstTreeNodeType = treeNodes[0].type; + if (firstTreeNodeType === DefaultTextType) { + for (const treeNode of treeNodes) { + const { type } = treeNode; + if (type !== DefaultTextType) { + throw new Error( + 'element node and text node cannot be passed together', + ); + } + validateTextNode(treeNode as TextNode); + } + } else { + for (const treeNode of treeNodes) { + const { type } = treeNode; + if (type === DefaultTextType) { + throw new Error( + 'element node and text node cannot be passed together', + ); + } + } + } + } + return true; +} + /** * `Tree` is a CRDT-based tree structure that is used to represent the document * tree of text-based editor such as ProseMirror. @@ -272,32 +314,44 @@ export class Tree { ); } - /** - * `editByPath` edits this tree with the given node and path. - */ - public editByPath( - fromPath: Array, - toPath: Array, - ...contents: Array + private editInternal( + fromPos: CRDTTreePos, + toPos: CRDTTreePos, + contents: Array, ): boolean { - if (!this.context || !this.tree) { - throw new Error('it is not initialized yet'); - } - if (fromPath.length !== toPath.length) { - throw new Error('path length should be equal'); - } - if (!fromPath.length || !toPath.length) { - throw new Error('path should not be empty'); + if (contents.length !== 0 && contents[0]) { + validateTreeNodes(contents); + if (contents[0].type !== DefaultTextType) { + for (const content of contents) { + const { children = [] } = content as ElementNode; + validateTreeNodes(children); + } + } } - const crdtNodes: Array = contents - .map((content) => content && createCRDTTreeNode(this.context!, content)) - .filter((a) => a) as Array; + const ticket = this.context!.getLastTimeTicket(); + let crdtNodes = new Array(); - const fromPos = this.tree.pathToPos(fromPath); - const toPos = this.tree.pathToPos(toPath); - const ticket = this.context.getLastTimeTicket(); - this.tree.edit( + if (contents[0]?.type === DefaultTextType) { + let compVal = ''; + for (const content of contents) { + const { value } = content as TextNode; + compVal += value; + } + crdtNodes.push( + CRDTTreeNode.create( + CRDTTreePos.of(this.context!.issueTimeTicket(), 0), + DefaultTextType, + compVal, + ), + ); + } else { + crdtNodes = contents + .map((content) => content && createCRDTTreeNode(this.context!, content)) + .filter((a) => a) as Array; + } + + this.tree!.edit( [fromPos, toPos], crdtNodes.length ? crdtNodes.map((crdtNode) => crdtNode?.deepcopy()) @@ -305,9 +359,9 @@ export class Tree { ticket, ); - this.context.push( + this.context!.push( TreeEditOperation.create( - this.tree.getCreatedAt(), + this.tree!.getCreatedAt(), fromPos, toPos, crdtNodes.length ? crdtNodes : undefined, @@ -319,14 +373,38 @@ export class Tree { !fromPos.getCreatedAt().equals(toPos.getCreatedAt()) || fromPos.getOffset() !== toPos.getOffset() ) { - this.context.registerElementHasRemovedNodes(this.tree!); + this.context!.registerElementHasRemovedNodes(this.tree!); } return true; } /** - * `edit` edits this tree with the given node. + * `editByPath` edits this tree with the given node and path. + */ + public editByPath( + fromPath: Array, + toPath: Array, + ...contents: Array + ): boolean { + if (!this.context || !this.tree) { + throw new Error('it is not initialized yet'); + } + if (fromPath.length !== toPath.length) { + throw new Error('path length should be equal'); + } + if (!fromPath.length || !toPath.length) { + throw new Error('path should not be empty'); + } + + const fromPos = this.tree.pathToPos(fromPath); + const toPos = this.tree.pathToPos(toPath); + + return this.editInternal(fromPos, toPos, contents); + } + + /** + * `edit` edits this tree with the given nodes. */ public edit( fromIdx: number, @@ -340,38 +418,10 @@ export class Tree { throw new Error('from should be less than or equal to to'); } - const crdtNodes: Array = contents - .map((content) => content && createCRDTTreeNode(this.context!, content)) - .filter((a) => a) as Array; const fromPos = this.tree.findPos(fromIdx); const toPos = this.tree.findPos(toIdx); - const ticket = this.context.getLastTimeTicket(); - this.tree.edit( - [fromPos, toPos], - crdtNodes.length - ? crdtNodes.map((crdtNode) => crdtNode?.deepcopy()) - : undefined, - ticket, - ); - this.context.push( - TreeEditOperation.create( - this.tree.getCreatedAt(), - fromPos, - toPos, - crdtNodes.length ? crdtNodes : undefined, - ticket, - ), - ); - - if ( - !fromPos.getCreatedAt().equals(toPos.getCreatedAt()) || - fromPos.getOffset() !== toPos.getOffset() - ) { - this.context.registerElementHasRemovedNodes(this.tree!); - } - - return true; + return this.editInternal(fromPos, toPos, contents); } /** diff --git a/test/integration/tree_test.ts b/test/integration/tree_test.ts index dbae9dd32..9a1232181 100644 --- a/test/integration/tree_test.ts +++ b/test/integration/tree_test.ts @@ -467,9 +467,7 @@ describe('Tree', () => { children: [ { type: 'p', - children: [ - { type: 'tn', children: [{ type: 'text', value: '' }] }, - ], + children: [{ type: 'tn', children: [] }], }, ], }, @@ -492,7 +490,7 @@ describe('Tree', () => { root.t.editByPath([0, 1], [0, 1], { type: 'p', - children: [{ type: 'tn', children: [{ type: 'text', value: '' }] }], + children: [{ type: 'tn', children: [] }], }); assert.equal( root.t.toXML(), @@ -510,7 +508,7 @@ describe('Tree', () => { root.t.editByPath([0, 2], [0, 2], { type: 'p', - children: [{ type: 'tn', children: [{ type: 'text', value: '' }] }], + children: [{ type: 'tn', children: [] }], }); assert.equal( root.t.toXML(), @@ -528,7 +526,7 @@ describe('Tree', () => { root.t.editByPath([0, 3], [0, 3], { type: 'p', - children: [{ type: 'tn', children: [{ type: 'text', value: '' }] }], + children: [{ type: 'tn', children: [] }], }); assert.equal( root.t.toXML(), @@ -546,7 +544,7 @@ describe('Tree', () => { root.t.editByPath([0, 3], [0, 3], { type: 'p', - children: [{ type: 'tn', children: [{ type: 'text', value: '' }] }], + children: [{ type: 'tn', children: [] }], }); assert.equal( @@ -592,6 +590,211 @@ describe('Tree', () => { }); describe('Tree.edit', function () { + it('Can insert multiple text nodes', function () { + const key = toDocKey(`${this.test!.title}-${new Date().getTime()}`); + const doc = new yorkie.Document<{ t: Tree }>(key); + doc.update((root) => { + root.t = new Tree({ + type: 'doc', + children: [ + { + type: 'p', + children: [{ type: 'text', value: 'ab' }], + }, + ], + }); + }); + assert.equal(doc.getRoot().t.toXML(), /*html*/ `

ab

`); + + doc.update((root) => { + root.t.edit( + 3, + 3, + { type: 'text', value: 'c' }, + { type: 'text', value: 'd' }, + ); + }); + + assert.equal(doc.getRoot().t.toXML(), /*html*/ `

abcd

`); + }); + + it('Can insert multiple element nodes', function () { + const key = toDocKey(`${this.test!.title}-${new Date().getTime()}`); + const doc = new yorkie.Document<{ t: Tree }>(key); + doc.update((root) => { + root.t = new Tree({ + type: 'doc', + children: [ + { + type: 'p', + children: [{ type: 'text', value: 'ab' }], + }, + ], + }); + }); + assert.equal(doc.getRoot().t.toXML(), /*html*/ `

ab

`); + + doc.update((root) => { + root.t.edit( + 4, + 4, + { type: 'p', children: [{ type: 'text', value: 'cd' }] }, + { type: 'i', children: [{ type: 'text', value: 'fg' }] }, + ); + }); + + assert.equal( + doc.getRoot().t.toXML(), + /*html*/ `

ab

cd

fg`, + ); + }); + + it('Detecting error for empty text', function () { + const key = toDocKey(`${this.test!.title}-${new Date().getTime()}`); + const doc = new yorkie.Document<{ t: Tree }>(key); + doc.update((root) => { + root.t = new Tree({ + type: 'doc', + children: [ + { + type: 'p', + children: [{ type: 'text', value: 'ab' }], + }, + ], + }); + }); + assert.equal(doc.getRoot().t.toXML(), /*html*/ `

ab

`); + + assert.Throw(() => { + doc.update((root) => { + root.t.edit( + 3, + 3, + { type: 'text', value: 'c' }, + { type: 'text', value: '' }, + ); + }); + }, 'text node cannot have empty value'); + }); + + it('Detecting error for mixed type insertion', function () { + const key = toDocKey(`${this.test!.title}-${new Date().getTime()}`); + const doc = new yorkie.Document<{ t: Tree }>(key); + doc.update((root) => { + root.t = new Tree({ + type: 'doc', + children: [ + { + type: 'p', + children: [{ type: 'text', value: 'ab' }], + }, + ], + }); + }); + assert.equal(doc.getRoot().t.toXML(), /*html*/ `

ab

`); + + assert.Throw(() => { + doc.update((root) => { + root.t.edit( + 3, + 3, + { type: 'p', children: [] }, + { type: 'text', value: 'd' }, + ); + }); + }, 'element node and text node cannot be passed together'); + }); + + it('Detecting correct error order [1]', function () { + const key = toDocKey(`${this.test!.title}-${new Date().getTime()}`); + const doc = new yorkie.Document<{ t: Tree }>(key); + doc.update((root) => { + root.t = new Tree({ + type: 'doc', + children: [ + { + type: 'p', + children: [{ type: 'text', value: 'ab' }], + }, + ], + }); + }); + assert.equal(doc.getRoot().t.toXML(), /*html*/ `

ab

`); + + assert.Throw(() => { + doc.update((root) => { + root.t.edit( + 3, + 3, + { + type: 'p', + children: [ + { type: 'text', value: 'c' }, + { type: 'text', value: '' }, + ], + }, + { type: 'text', value: 'd' }, + ); + }); + }, 'element node and text node cannot be passed together'); + }); + + it('Detecting correct error order [2]', function () { + const key = toDocKey(`${this.test!.title}-${new Date().getTime()}`); + const doc = new yorkie.Document<{ t: Tree }>(key); + doc.update((root) => { + root.t = new Tree({ + type: 'doc', + children: [ + { + type: 'p', + children: [{ type: 'text', value: 'ab' }], + }, + ], + }); + }); + assert.equal(doc.getRoot().t.toXML(), /*html*/ `

ab

`); + + assert.Throw(() => { + doc.update((root) => { + root.t.edit( + 3, + 3, + { type: 'p', children: [{ type: 'text', value: 'c' }] }, + { type: 'p', children: [{ type: 'text', value: '' }] }, + ); + }); + }, 'text node cannot have empty value'); + }); + + it('Detecting correct error order [3]', function () { + const key = toDocKey(`${this.test!.title}-${new Date().getTime()}`); + const doc = new yorkie.Document<{ t: Tree }>(key); + doc.update((root) => { + root.t = new Tree({ + type: 'doc', + children: [ + { + type: 'p', + children: [{ type: 'text', value: 'ab' }], + }, + ], + }); + }); + assert.equal(doc.getRoot().t.toXML(), /*html*/ `

ab

`); + + assert.Throw(() => { + doc.update((root) => { + root.t.edit( + 3, + 3, + { type: 'text', value: 'd' }, + { type: 'p', children: [{ type: 'text', value: 'c' }] }, + ); + }); + }, 'element node and text node cannot be passed together'); + }); + it.skip('Can insert text to the same position(left) concurrently', function () { const [docA, docB] = createTwoTreeDocs(toDocKey(this.test!.title), { type: 'r', @@ -678,9 +881,7 @@ describe('Tree.style', function () { children: [ { type: 'p', - children: [ - { type: 'tn', children: [{ type: 'text', value: '' }] }, - ], + children: [{ type: 'tn', children: [] }], attributes: { a: 'b' }, }, ], @@ -726,9 +927,7 @@ describe('Tree.style', function () { children: [ { type: 'p', - children: [ - { type: 'tn', children: [{ type: 'text', value: '' }] }, - ], + children: [{ type: 'tn', children: [] }], attributes: { a: 'b' }, }, ], @@ -761,7 +960,7 @@ describe('Tree.style', function () { assert.equal( root.toJSON!(), - /*html*/ `{"t":{"type":"doc","children":[{"type":"tc","children":[{"type":"p","children":[{"type":"tn","children":[{"type":"text","value":""}],"attributes":{"z":"m"}}],"attributes":{"a":"b","c":"q"}}]}]}}`, + /*html*/ `{"t":{"type":"doc","children":[{"type":"tc","children":[{"type":"p","children":[{"type":"tn","children":[],"attributes":{"z":"m"}}],"attributes":{"a":"b","c":"q"}}]}]}}`, ); }); });