Change TreeNode to have IDs instead of insPrev, insNext (#622)

During the conversion, it's hard to find insPrev, insNext nodes from IDs
because we can't use LLRB at that moment. Therefore, this commit changed
it to keep IDs of insPrev and insNext and find the actual nodes when we
need it.

Change Tree.Prepend to call updateAncestorSize only when a node is not
removed during message conversion of pb to model.

---------

Co-authored-by: Hackerwins <susukang98@gmail.com>

api/converter/from_pb.go

@@ -648,6 +648,21 @@ func fromTreeNode(pbNode *api.TreeNode) (*crdt.TreeNode, error) {
 		attrs,
 		pbNode.Value,
 	)
+
+	if pbNode.GetInsPrevId() != nil {
+		node.InsPrevID, err = fromTreeNodeID(pbNode.GetInsPrevId())
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	if pbNode.GetInsNextId() != nil {
+		node.InsNextID, err = fromTreeNodeID(pbNode.GetInsNextId())
+		if err != nil {
+			return nil, err
+		}
+	}
+
 	node.RemovedAt, err = fromTimeTicket(pbNode.RemovedAt)
 	if err != nil {
 		return nil, err

api/converter/to_bytes.go

@@ -306,8 +306,12 @@ func toTreeNode(treeNode *crdt.TreeNode, depth int) *api.TreeNode {
 		Attributes: attrs,
 	}
 
-	if treeNode.InsPrev != nil {
-		pbNode.InsPrevId = toTreeNodeID(treeNode.InsPrev.ID)
+	if treeNode.InsPrevID != nil {
+		pbNode.InsPrevId = toTreeNodeID(treeNode.InsPrevID)
+	}
+
+	if treeNode.InsNextID != nil {
+		pbNode.InsNextId = toTreeNodeID(treeNode.InsNextID)
 	}
 
 	return pbNode

api/yorkie/v1/resources.proto

@@ -243,8 +243,9 @@ message TreeNode {
   string value = 3;
   TimeTicket removed_at = 4;
   TreeNodeID ins_prev_id = 5;
-  int32 depth = 6;
-  map<string, NodeAttr> attributes = 7;
+  TreeNodeID ins_next_id = 6;
+  int32 depth = 7;
+  map<string, NodeAttr> attributes = 8;
 }
 
 message TreeNodes {

pkg/document/crdt/tree.go

@@ -49,8 +49,8 @@ type TreeNode struct {
 	ID        *TreeNodeID
 	RemovedAt *time.Ticket
 
-	InsPrev *TreeNode
-	InsNext *TreeNode
+	InsPrevID *TreeNodeID
+	InsNextID *TreeNodeID
 
 	// Value is optional. If the value is not empty, it means that the node is a
 	// text node.
@@ -402,16 +402,18 @@ func (t *Tree) purgeNode(node *TreeNode) error {
 	}
 	t.NodeMapByID.Remove(node.ID)
 
-	insPrev := node.InsPrev
-	insNext := node.InsNext
-	if insPrev != nil {
-		insPrev.InsNext = insNext
+	insPrevID := node.InsPrevID
+	insNextID := node.InsNextID
+	if insPrevID != nil {
+		insPrev := t.findFloorNode(insPrevID)
+		insPrev.InsNextID = insNextID
 	}
-	if insNext != nil {
-		insNext.InsPrev = insPrev
+	if insNextID != nil {
+		insNext := t.findFloorNode(insNextID)
+		insNext.InsPrevID = insPrevID
 	}
-	node.InsPrev = nil
-	node.InsNext = nil
+	node.InsPrevID = nil
+	node.InsNextID = nil
 
 	delete(t.removedNodeMap, node.ID.toIDString())
 	return nil
@@ -768,14 +770,15 @@ func (t *Tree) findTreeNodesWithSplitText(pos *TreePos, editedAt *time.Ticket) (
 		}
 
 		if split != nil {
-			split.InsPrev = leftSiblingNode
+			split.InsPrevID = leftSiblingNode.ID
 			t.NodeMapByID.Put(split.ID, split)
 
-			if leftSiblingNode.InsNext != nil {
-				leftSiblingNode.InsNext.InsPrev = split
-				split.InsNext = leftSiblingNode.InsNext
+			if leftSiblingNode.InsNextID != nil {
+				insNext := t.findFloorNode(leftSiblingNode.InsNextID)
+				insNext.InsPrevID = split.ID
+				split.InsNextID = leftSiblingNode.InsNextID
 			}
-			leftSiblingNode.InsNext = split
+			leftSiblingNode.InsNextID = split.ID
 		}
 	}
 
@@ -880,21 +883,29 @@ func (t *Tree) ToIndex(pos *TreePos) (int, error) {
 	return idx, nil
 }
 
+// findFloorNode returns node from given id.
+func (t *Tree) findFloorNode(id *TreeNodeID) *TreeNode {
+	key, node := t.NodeMapByID.Floor(id)
+
+	if node == nil || key.CreatedAt.Compare(id.CreatedAt) != 0 {
+		return nil
+	}
+
+	return node
+}
+
 func (t *Tree) toTreeNodes(pos *TreePos) (*TreeNode, *TreeNode) {
-	parentKey, parentNode := t.NodeMapByID.Floor(pos.ParentID)
-	leftSiblingKey, leftSiblingNode := t.NodeMapByID.Floor(pos.LeftSiblingID)
+	parentNode := t.findFloorNode(pos.ParentID)
+	leftSiblingNode := t.findFloorNode(pos.LeftSiblingID)
 
-	if parentNode == nil ||
-		leftSiblingNode == nil ||
-		parentKey.CreatedAt.Compare(pos.ParentID.CreatedAt) != 0 ||
-		leftSiblingKey.CreatedAt.Compare(pos.LeftSiblingID.CreatedAt) != 0 {
+	if parentNode == nil || leftSiblingNode == nil {
 		return nil, nil
 	}
 
 	if pos.LeftSiblingID.Offset > 0 &&
 		pos.LeftSiblingID.Offset == leftSiblingNode.ID.Offset &&
-		leftSiblingNode.InsPrev != nil {
-		return parentNode, leftSiblingNode.InsPrev
+		leftSiblingNode.InsPrevID != nil {
+		return parentNode, t.findFloorNode(leftSiblingNode.InsPrevID)
 	}
 
 	return parentNode, leftSiblingNode

pkg/index/tree.go

@@ -460,7 +460,10 @@ func (n *Node[V]) Prepend(children ...*Node[V]) error {
 	n.children = append(children, n.children...)
 	for _, node := range children {
 		node.Parent = n
-		node.UpdateAncestorsSize()
+
+		if !node.Value.IsRemoved() {
+			node.UpdateAncestorsSize()
+		}
 	}
 
 	return nil

test/integration/tree_test.go

@@ -274,18 +274,20 @@ func TestTree(t *testing.T) {
 			assert.Equal(t, "<doc><tc><p><tn>aXb!</tn><tn>cd</tn></p><p><tn>q</tn></p></tc></doc>", root.GetTree("t").ToXML())
 
 			root.GetTree("t").EditByPath([]int{0, 1, 0, 0}, []int{0, 1, 0, 0}, &json.TreeNode{
-				Type: "text",
+				Type:  "text",
 				Value: "a",
 			})
 			assert.Equal(t, "<doc><tc><p><tn>aXb!</tn><tn>cd</tn></p><p><tn>aq</tn></p></tc></doc>", root.GetTree("t").ToXML())
-
-			assert.Panics(t, func() {doc.Update(func(root *json.Object, p *presence.Presence) error {
-				root.GetTree("t").EditByPath([]int{0, 0, 4}, []int{0, 0, 4}, &json.TreeNode{
-					Type: "tn",
-					Children: []json.TreeNode{},
+
+			assert.Panics(t, func() {
+				doc.Update(func(root *json.Object, p *presence.Presence) error {
+					root.GetTree("t").EditByPath([]int{0, 0, 4}, []int{0, 0, 4}, &json.TreeNode{
+						Type:     "tn",
+						Children: []json.TreeNode{},
+					})
+					return nil
 				})
-				return nil
-			 })}, index.ErrUnreachablePath)			
+			}, index.ErrUnreachablePath)
 			return nil
 		})
 		assert.NoError(t, err)
@@ -308,7 +310,7 @@ func TestTree(t *testing.T) {
 			assert.Equal(t, "<doc><tc><p><tn></tn></p></tc></doc>", root.GetTree("t").ToXML())
 
 			root.GetTree("t").EditByPath([]int{0, 0, 0, 0}, []int{0, 0, 0, 0}, &json.TreeNode{
-				Type: "text",
+				Type:  "text",
 				Value: "a",
 			})
 			assert.Equal(t, "<doc><tc><p><tn>a</tn></p></tc></doc>", root.GetTree("t").ToXML())
@@ -322,7 +324,7 @@ func TestTree(t *testing.T) {
 			assert.Equal(t, "<doc><tc><p><tn>a</tn></p><p><tn></tn></p></tc></doc>", root.GetTree("t").ToXML())
 
 			root.GetTree("t").EditByPath([]int{0, 1, 0, 0}, []int{0, 1, 0, 0}, &json.TreeNode{
-				Type: "text",
+				Type:  "text",
 				Value: "b",
 			})
 			assert.Equal(t, "<doc><tc><p><tn>a</tn></p><p><tn>b</tn></p></tc></doc>", root.GetTree("t").ToXML())
@@ -336,7 +338,7 @@ func TestTree(t *testing.T) {
 			assert.Equal(t, "<doc><tc><p><tn>a</tn></p><p><tn>b</tn></p><p><tn></tn></p></tc></doc>", root.GetTree("t").ToXML())
 
 			root.GetTree("t").EditByPath([]int{0, 2, 0, 0}, []int{0, 2, 0, 0}, &json.TreeNode{
-				Type: "text",
+				Type:  "text",
 				Value: "c",
 			})
 			assert.Equal(t, "<doc><tc><p><tn>a</tn></p><p><tn>b</tn></p><p><tn>c</tn></p></tc></doc>", root.GetTree("t").ToXML())
@@ -350,11 +352,11 @@ func TestTree(t *testing.T) {
 			assert.Equal(t, "<doc><tc><p><tn>a</tn></p><p><tn>b</tn></p><p><tn>c</tn></p><p><tn></tn></p></tc></doc>", root.GetTree("t").ToXML())
 
 			root.GetTree("t").EditByPath([]int{0, 3, 0, 0}, []int{0, 3, 0, 0}, &json.TreeNode{
-				Type: "text",
+				Type:  "text",
 				Value: "d",
 			})
 			assert.Equal(t, "<doc><tc><p><tn>a</tn></p><p><tn>b</tn></p><p><tn>c</tn></p><p><tn>d</tn></p></tc></doc>", root.GetTree("t").ToXML())
-			
+
 			root.GetTree("t").EditByPath([]int{0, 3}, []int{0, 3}, &json.TreeNode{
 				Type: "p",
 				Children: []json.TreeNode{{
@@ -1951,4 +1953,127 @@ func TestTree(t *testing.T) {
 		})
 		assert.NoError(t, err)
 	})
+
+	t.Run("split link can transmitted through rpc", func(t *testing.T) {
+		ctx := context.Background()
+		d1 := document.New(helper.TestDocKey(t))
+		assert.NoError(t, c1.Attach(ctx, d1))
+
+		assert.NoError(t, d1.Update(func(root *json.Object, p *presence.Presence) error {
+			root.SetNewTree("t", &json.TreeNode{
+				Type: "root",
+				Children: []json.TreeNode{{
+					Type:     "p",
+					Children: []json.TreeNode{{Type: "text", Value: "ab"}},
+				}},
+			})
+			return nil
+		}))
+
+		assert.NoError(t, d1.Update(func(root *json.Object, p *presence.Presence) error {
+			root.GetTree("t").Edit(2, 2, &json.TreeNode{
+				Type:  "text",
+				Value: "1",
+			})
+			return nil
+		}))
+
+		assert.NoError(t, c1.Sync(ctx))
+		d2 := document.New(helper.TestDocKey(t))
+		assert.NoError(t, c2.Attach(ctx, d2))
+
+		assert.NoError(t, d2.Update(func(root *json.Object, p *presence.Presence) error {
+			root.GetTree("t").Edit(3, 3, &json.TreeNode{
+				Type:  "text",
+				Value: "1",
+			})
+			return nil
+		}))
+		assert.Equal(t, "<root><p>a11b</p></root>", d2.Root().GetTree("t").ToXML())
+
+		assert.NoError(t, d2.Update(func(root *json.Object, p *presence.Presence) error {
+			root.GetTree("t").Edit(2, 3, &json.TreeNode{
+				Type:  "text",
+				Value: "12",
+			})
+			return nil
+		}))
+
+		assert.NoError(t, d2.Update(func(root *json.Object, p *presence.Presence) error {
+			root.GetTree("t").Edit(4, 5, &json.TreeNode{
+				Type:  "text",
+				Value: "21",
+			})
+			return nil
+		}))
+
+		assert.Equal(t, "<root><p>a1221b</p></root>", d2.Root().GetTree("t").ToXML())
+
+		assert.NoError(t, d2.Update(func(root *json.Object, p *presence.Presence) error {
+			root.GetTree("t").Edit(2, 4, &json.TreeNode{
+				Type:  "text",
+				Value: "123",
+			})
+			return nil
+		}))
+
+		assert.Equal(t, "<root><p>a12321b</p></root>", d2.Root().GetTree("t").ToXML())
+	})
+
+	t.Run("can calculate size of index tree correctly", func(t *testing.T) {
+		ctx := context.Background()
+		d1 := document.New(helper.TestDocKey(t))
+		assert.NoError(t, c1.Attach(ctx, d1))
+
+		assert.NoError(t, d1.Update(func(root *json.Object, p *presence.Presence) error {
+			root.SetNewTree("t", &json.TreeNode{
+				Type: "root",
+				Children: []json.TreeNode{{
+					Type:     "p",
+					Children: []json.TreeNode{{Type: "text", Value: "ab"}},
+				}},
+			})
+			return nil
+		}))
+
+		assert.NoError(t, d1.Update(func(root *json.Object, p *presence.Presence) error {
+			root.GetTree("t").Edit(2, 2, &json.TreeNode{
+				Type:  "text",
+				Value: "123",
+			})
+			return nil
+		}))
+
+		assert.NoError(t, d1.Update(func(root *json.Object, p *presence.Presence) error {
+			root.GetTree("t").Edit(2, 2, &json.TreeNode{
+				Type:  "text",
+				Value: "456",
+			})
+			return nil
+		}))
+
+		assert.NoError(t, d1.Update(func(root *json.Object, p *presence.Presence) error {
+			root.GetTree("t").Edit(2, 2, &json.TreeNode{
+				Type:  "text",
+				Value: "789",
+			})
+			return nil
+		}))
+
+		assert.NoError(t, d1.Update(func(root *json.Object, p *presence.Presence) error {
+			root.GetTree("t").Edit(2, 2, &json.TreeNode{
+				Type:  "text",
+				Value: "0123",
+			})
+			return nil
+		}))
+
+		assert.Equal(t, "<root><p>a0123789456123b</p></root>", d1.Root().GetTree("t").ToXML())
+		assert.NoError(t, c1.Sync(ctx))
+
+		d2 := document.New(helper.TestDocKey(t))
+		assert.NoError(t, c2.Attach(ctx, d2))
+		size := d1.Root().GetTree("t").IndexTree.Root().Len()
+		assert.Equal(t, size, d2.Root().GetTree("t").IndexTree.Root().Len())
+	})
 }