diff --git a/pkg/document/crdt/tree.go b/pkg/document/crdt/tree.go index 5c63c090b..42454818b 100644 --- a/pkg/document/crdt/tree.go +++ b/pkg/document/crdt/tree.go @@ -373,11 +373,7 @@ func (n *TreeNode) remove(removedAt *time.Ticket) bool { if n.removedAt == nil || n.removedAt.Compare(removedAt) > 0 { n.removedAt = removedAt if justRemoved { - if n.Index.Parent.Value.removedAt == nil { - n.Index.UpdateAncestorsSize() - } else { - n.Index.Parent.Length -= n.Index.PaddedLength() - } + n.Index.UpdateAncestorsSize() } return justRemoved } diff --git a/pkg/index/tree.go b/pkg/index/tree.go index 8a96397b1..0d68707f5 100644 --- a/pkg/index/tree.go +++ b/pkg/index/tree.go @@ -336,6 +336,9 @@ func (n *Node[V]) UpdateAncestorsSize() { for parent != nil { parent.Length += n.PaddedLength() * sign + if parent.Value.IsRemoved() { + break + } parent = parent.Parent } @@ -344,17 +347,17 @@ func (n *Node[V]) UpdateAncestorsSize() { // UpdateDescendantsSize updates the size of descendants. It is used when // the tree is newly created and the size of the descendants is not calculated. func (n *Node[V]) UpdateDescendantsSize() int { - if n.Value.IsRemoved() { - n.Length = 0 - return 0 - } - - sum := 0 + size := 0 for _, child := range n.Children(true) { - sum += child.UpdateDescendantsSize() + childSize := child.UpdateDescendantsSize() + if child.Value.IsRemoved() { + continue + } + + size += childSize } - n.Length += sum + n.Length += size return n.PaddedLength() } diff --git a/test/integration/tree_test.go b/test/integration/tree_test.go index b66e2b274..54e1067c0 100644 --- a/test/integration/tree_test.go +++ b/test/integration/tree_test.go @@ -24,7 +24,9 @@ import ( "github.com/stretchr/testify/assert" + "github.com/yorkie-team/yorkie/api/converter" "github.com/yorkie-team/yorkie/pkg/document" + "github.com/yorkie-team/yorkie/pkg/document/crdt" "github.com/yorkie-team/yorkie/pkg/document/json" "github.com/yorkie-team/yorkie/pkg/document/presence" "github.com/yorkie-team/yorkie/pkg/index" @@ -3501,6 +3503,140 @@ func TestTree(t *testing.T) { assert.Equal(t, size, d2.Root().GetTree("t").IndexTree.Root().Len()) }) + t.Run("can calculate size of index tree correctly during concurrent editing", 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)) + + assert.NoError(t, d1.Update(func(root *json.Object, p *presence.Presence) error { + root.SetNewTree("t", &json.TreeNode{ + Type: "doc", + 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, "

hello

", d1.Root().GetTree("t").ToXML()) + assert.Equal(t, "

hello

", d2.Root().GetTree("t").ToXML()) + + assert.NoError(t, d1.Update(func(root *json.Object, p *presence.Presence) error { + root.GetTree("t").Edit(0, 7, nil, 0) + return nil + })) + assert.NoError(t, d2.Update(func(root *json.Object, p *presence.Presence) error { + root.GetTree("t").Edit(1, 2, &json.TreeNode{ + Type: "text", + Value: "p", + }, 0) + return nil + })) + assert.Equal(t, "", d1.Root().GetTree("t").ToXML()) + assert.Equal(t, 0, d1.Root().GetTree("t").Len()) + assert.Equal(t, "

pello

", d2.Root().GetTree("t").ToXML()) + assert.Equal(t, 7, d2.Root().GetTree("t").Len()) + assert.NoError(t, c1.Sync(ctx)) + assert.NoError(t, c2.Sync(ctx)) + assert.NoError(t, c1.Sync(ctx)) + + assert.Equal(t, "", d1.Root().GetTree("t").ToXML()) + assert.Equal(t, "", d2.Root().GetTree("t").ToXML()) + assert.Equal(t, d1.Root().GetTree("t").Len(), d2.Root().GetTree("t").Len()) + }) + + t.Run("can keep index tree consistent from snapshot", 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)) + + 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{}, + }}, + }) + return nil + })) + assert.NoError(t, c1.Sync(ctx)) + assert.NoError(t, c2.Sync(ctx)) + + assert.Equal(t, "

", d1.Root().GetTree("t").ToXML()) + assert.Equal(t, "

", d2.Root().GetTree("t").ToXML()) + + assert.NoError(t, d1.Update(func(root *json.Object, p *presence.Presence) error { + root.GetTree("t").Edit(0, 2, nil, 0) + return nil + })) + assert.NoError(t, d2.Update(func(root *json.Object, p *presence.Presence) error { + root.GetTree("t").Edit(1, 1, &json.TreeNode{ + Type: "i", + Children: []json.TreeNode{{Type: "text", Value: "a"}}, + }, 0) + root.GetTree("t").Edit(2, 3, &json.TreeNode{ + Type: "text", + Value: "b", + }, 0) + return nil + })) + assert.Equal(t, "", d1.Root().GetTree("t").ToXML()) + assert.Equal(t, 0, d1.Root().GetTree("t").Len()) + assert.Equal(t, "

b

", d2.Root().GetTree("t").ToXML()) + assert.Equal(t, 5, d2.Root().GetTree("t").Len()) + assert.NoError(t, c1.Sync(ctx)) + assert.NoError(t, c2.Sync(ctx)) + assert.NoError(t, c1.Sync(ctx)) + + assert.Equal(t, "", d1.Root().GetTree("t").ToXML()) + assert.Equal(t, "", d2.Root().GetTree("t").ToXML()) + + type Node struct { + xml string + len int + isRemoved bool + } + var d1Nodes []Node + var d2Nodes []Node + var sNodes []Node + + index.TraverseNode(d1.Root().GetTree("t").IndexTree.Root(), func(node *index.Node[*crdt.TreeNode], depth int) { + d1Nodes = append(d1Nodes, Node{ + xml: index.ToXML(node), + len: node.Len(), + isRemoved: node.Value.IsRemoved(), + }) + }) + index.TraverseNode(d2.Root().GetTree("t").IndexTree.Root(), func(node *index.Node[*crdt.TreeNode], depth int) { + d2Nodes = append(d2Nodes, Node{ + xml: index.ToXML(node), + len: node.Len(), + isRemoved: node.Value.IsRemoved(), + }) + }) + obj, err := converter.ObjectToBytes(d1.RootObject()) + assert.NoError(t, err) + sRoot, err := converter.BytesToObject(obj) + assert.NoError(t, err) + index.TraverseNode(sRoot.Get("t").(*crdt.Tree).IndexTree.Root(), func(node *index.Node[*crdt.TreeNode], depth int) { + sNodes = append(sNodes, Node{ + xml: index.ToXML(node), + len: node.Len(), + isRemoved: node.Value.IsRemoved(), + }) + }) + assert.ObjectsAreEqual(d1Nodes, d2Nodes) + assert.ObjectsAreEqual(d1Nodes, sNodes) + }) + t.Run("can split and merge with empty paragraph: left", func(t *testing.T) { ctx := context.Background() d1 := document.New(helper.TestDocKey(t))