Skip to content

Commit

Permalink
Fix incorrect calculation in indexTree.treePosToPath operation (#824)
Browse files Browse the repository at this point in the history
This commit addresses an issue arising from the tree.edit operation in
response to problem with calculating the fromPath when multiple
treePos represent a single index-based coordinate, leading to
inaccuracies.

This commit contains migration work for yorkie-team/yorkie-js-sdk#751,
so detailed information can also be found there.

---------

Co-authored-by: Youngteac Hong <[email protected]>
  • Loading branch information
raararaara and hackerwins authored Mar 20, 2024
1 parent eadb194 commit 7fb913a
Show file tree
Hide file tree
Showing 3 changed files with 108 additions and 8 deletions.
19 changes: 16 additions & 3 deletions pkg/document/crdt/tree.go
Original file line number Diff line number Diff line change
Expand Up @@ -917,7 +917,7 @@ func (t *Tree) FindTreeNodesWithSplitText(pos *TreePos, editedAt *time.Ticket) (
*TreeNode, *TreeNode, error,
) {
// 01. Find the parent and left sibling nodes of the given position.
parentNode, leftNode := t.toTreeNodes(pos)
parentNode, leftNode := t.ToTreeNodes(pos)
if parentNode == nil || leftNode == nil {
return nil, nil, fmt.Errorf("%p: %w", pos, ErrNodeNotFound)
}
Expand Down Expand Up @@ -1030,6 +1030,19 @@ func (t *Tree) ToIndex(parentNode, leftSiblingNode *TreeNode) (int, error) {
return idx, nil
}

// ToPath returns path from given CRDTTreePos
func (t *Tree) ToPath(parentNode, leftSiblingNode *TreeNode) ([]int, error) {
treePos, err := t.toTreePos(parentNode, leftSiblingNode)
if err != nil {
return nil, err
}
if treePos == nil {
return nil, nil
}

return t.IndexTree.TreePosToPath(treePos)
}

// findFloorNode returns node from given id.
func (t *Tree) findFloorNode(id *TreeNodeID) *TreeNode {
key, node := t.NodeMapByID.Floor(id)
Expand All @@ -1041,11 +1054,11 @@ func (t *Tree) findFloorNode(id *TreeNodeID) *TreeNode {
return node
}

// toTreeNodes converts the pos to parent and left sibling nodes.
// ToTreeNodes 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 the left of the position.
func (t *Tree) toTreeNodes(pos *TreePos) (*TreeNode, *TreeNode) {
func (t *Tree) ToTreeNodes(pos *TreePos) (*TreeNode, *TreeNode) {
parentNode := t.findFloorNode(pos.ParentID)
leftNode := t.findFloorNode(pos.LeftSiblingID)

Expand Down
24 changes: 19 additions & 5 deletions pkg/index/tree.go
Original file line number Diff line number Diff line change
Expand Up @@ -593,15 +593,20 @@ func (n *Node[V]) InsertAfter(newNode, referenceNode *Node[V]) error {
return nil
}

// HasTextChild returns true if the node has a text child.
// HasTextChild returns true if the node's children consist of only text children.
func (n *Node[V]) HasTextChild() bool {
for _, child := range n.Children() {
if child.IsText() {
return true
children := n.Children()
if len(children) == 0 {
return false
}

for _, child := range children {
if !child.IsText() {
return false
}
}

return false
return true
}

// OffsetOfChild returns offset of children of the given node.
Expand Down Expand Up @@ -728,6 +733,15 @@ func (t *Tree[V]) TreePosToPath(treePos *TreePos[V]) ([]int, error) {

node = node.Parent
path = append(path, leftSiblingsSize+treePos.Offset)
} else if node.HasTextChild() {
// TODO(hackerwins): The function does not consider the situation
// where Element and Text nodes are mixed in the Element's Children.
leftSiblingsSize, err := t.LeftSiblingsSize(node, treePos.Offset)
if err != nil {
return nil, err
}

path = append(path, leftSiblingsSize)
} else {
path = append(path, treePos.Offset)
}
Expand Down
73 changes: 73 additions & 0 deletions test/integration/tree_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,79 @@ func TestTree(t *testing.T) {
assert.NoError(t, err)
})

t.Run("find correct path after deleting leftNode test", func(t *testing.T) {
ctx := context.Background()
d1 := document.New(helper.TestDocKey(t))
assert.NoError(t, c1.Attach(ctx, d1))
d2 := document.New(helper.TestDocKey(t))
assert.NoError(t, c2.Attach(ctx, d2))

// 01. Create a tree and insert a paragraph with text.
assert.NoError(t, d1.Update(func(root *json.Object, p *presence.Presence) error {
root.SetNewTree("t", &json.TreeNode{
Type: "r",
Children: []json.TreeNode{{
Type: "p",
Children: []json.TreeNode{{
Type: "text",
Value: "hello",
}},
}},
})

return nil
}))
assert.NoError(t, c1.Sync(ctx))
assert.NoError(t, c2.Sync(ctx))
assert.Equal(t, "<r><p>hello</p></r>", d1.Root().GetTree("t").ToXML())
assert.Equal(t, "<r><p>hello</p></r>", d2.Root().GetTree("t").ToXML())

// 02. Insert additional character between 3rd character and 4th character.
assert.NoError(t, d1.Update(func(root *json.Object, p *presence.Presence) error {
root.GetTree("t").EditByPath([]int{0, 3}, []int{0, 3}, &json.TreeNode{
Type: "text",
Value: "3",
}, 0)

return nil
}))
assert.NoError(t, c1.Sync(ctx))
assert.NoError(t, c2.Sync(ctx))
assert.Equal(t, "<r><p>hel3lo</p></r>", d1.Root().GetTree("t").ToXML())
assert.Equal(t, "<r><p>hel3lo</p></r>", d2.Root().GetTree("t").ToXML())

// 03. Get fromParent, fromLeft node from path [0, 5] before fix
fromPos, _ := d2.Root().GetTree("t").PathToPos([]int{0, 5})
fromParent, fromLeft := d2.Root().GetTree("t").ToTreeNodes(fromPos)

// 04. Apply modification
assert.NoError(t, d2.Update(func(root *json.Object, p *presence.Presence) error {
// 04-1. Erase 3rd character from the text, which immediately preceding the treePos obtained above.
root.GetTree("t").EditByPath([]int{0, 4}, []int{0, 5}, nil, 0)
assert.Equal(t, "<r><p>hel3o</p></r>", d2.Root().GetTree("t").ToXML())

// 04-2. Insert character between 3rd character and 4th character.
root.GetTree("t").EditByPath([]int{0, 4}, []int{0, 4}, &json.TreeNode{
Type: "text",
Value: "m",
}, 0)
assert.Equal(t, "<r><p>hel3mo</p></r>", d2.Root().GetTree("t").ToXML())

return nil
}))

// 05. Check if the path is identical when re-obtaining the path from `fromParent` and `fromLeft` after fix.
fromPath, err := d2.Root().GetTree("t").ToPath(fromParent, fromLeft)
assert.NoError(t, err)
assert.Equal(t, []int{0, 5}, fromPath)

assert.NoError(t, c2.Sync(ctx))
assert.NoError(t, c1.Sync(ctx))

assert.Equal(t, "<r><p>hel3mo</p></r>", d1.Root().GetTree("t").ToXML())
assert.Equal(t, "<r><p>hel3mo</p></r>", d2.Root().GetTree("t").ToXML())
})

t.Run("sync its content with other clients test", func(t *testing.T) {
ctx := context.Background()
d1 := document.New(helper.TestDocKey(t))
Expand Down

0 comments on commit 7fb913a

Please sign in to comment.