From e38d78f1352faf6142640c2c4c387aec9e2b8e4d Mon Sep 17 00:00:00 2001
From: Jeounghui <jeounghui.nah@navercorp.com>
Date: Fri, 5 Jan 2024 17:03:24 +0900
Subject: [PATCH] Fix RHT.Len returns correct value

---
 pkg/document/crdt/rht.go      | 17 +++++++++++------
 pkg/document/crdt/rht_test.go | 20 ++++++++++++++++++++
 test/integration/tree_test.go |  7 +++----
 3 files changed, 34 insertions(+), 10 deletions(-)

diff --git a/pkg/document/crdt/rht.go b/pkg/document/crdt/rht.go
index b7784020f..839f32bb1 100644
--- a/pkg/document/crdt/rht.go
+++ b/pkg/document/crdt/rht.go
@@ -74,13 +74,15 @@ func (n *RHTNode) RemovedAt() *time.Ticket {
 // RHT is a hashtable with logical clock(Replicated hashtable).
 // For more details about RHT: http://csl.skku.edu/papers/jpdc11.pdf
 type RHT struct {
-	nodeMapByKey map[string]*RHTNode
+	nodeMapByKey           map[string]*RHTNode
+	numberOfRemovedElement int
 }
 
 // NewRHT creates a new instance of RHT.
 func NewRHT() *RHT {
 	return &RHT{
-		nodeMapByKey: make(map[string]*RHTNode),
+		nodeMapByKey:           make(map[string]*RHTNode),
+		numberOfRemovedElement: 0,
 	}
 }
 
@@ -115,9 +117,12 @@ func (rht *RHT) Set(k, v string, executedAt *time.Ticket) {
 
 // Remove removes the Element of the given key.
 func (rht *RHT) Remove(k string, executedAt *time.Ticket) string {
-	if node, ok := rht.nodeMapByKey[k]; ok && executedAt.After(node.removedAt) {
-		node.Remove(executedAt)
-		return node.val
+	if node, ok := rht.nodeMapByKey[k]; ok && (node.updatedAt == nil || executedAt.After(node.updatedAt)) {
+		if rht.Has(k) {
+			node.Remove(executedAt)
+			rht.numberOfRemovedElement++
+			return node.val
+		}
 	}
 
 	return ""
@@ -149,7 +154,7 @@ func (rht *RHT) Nodes() []*RHTNode {
 
 // Len returns the number of elements.
 func (rht *RHT) Len() int {
-	return len(rht.nodeMapByKey)
+	return len(rht.nodeMapByKey) - rht.numberOfRemovedElement
 }
 
 // DeepCopy copies itself deeply.
diff --git a/pkg/document/crdt/rht_test.go b/pkg/document/crdt/rht_test.go
index 39d20da0f..0f58c3174 100644
--- a/pkg/document/crdt/rht_test.go
+++ b/pkg/document/crdt/rht_test.go
@@ -20,4 +20,24 @@ func TestMarshal(t *testing.T) {
 		actual := rht.Marshal()
 		assert.Equal(t, expected, actual)
 	})
+
+	/*t.Run("make XML after remove element", func(t *testing.T) {
+		rht := NewRHT()
+
+		rht.Set("key1", "value1", nil)
+		rht.Set("key2", "value2", nil)
+		expected1 := `key1="value1" key2="value2"`
+		actual1 := rht.ToXML()
+		assert.Equal(t, expected1, actual1)
+
+		rht.Remove("key1", nil)
+		expected2 := `key2="value2"`
+		actual2 := rht.ToXML()
+		assert.Equal(t, expected2, actual2)
+
+		rht.Remove("key2", nil)
+		expected3 := ``
+		actual3 := rht.ToXML()
+		assert.Equal(t, expected3, actual3)
+	})*/
 }
diff --git a/test/integration/tree_test.go b/test/integration/tree_test.go
index 79cc0d292..855e624b5 100644
--- a/test/integration/tree_test.go
+++ b/test/integration/tree_test.go
@@ -753,8 +753,7 @@ func TestTree(t *testing.T) {
 		assert.Equal(t, `<root><p>ab</p><p italic="true">cd</p></root>`, d1.Root().GetTree("t").ToXML())
 
 		assert.NoError(t, d1.Update(func(root *json.Object, p *presence.Presence) error {
-			// NOTE(sejongk): 0, 4 -> 0,1 / 3,4
-			root.GetTree("t").RemoveStyle(0, 4, []string{"italic"})
+			root.GetTree("t").RemoveStyle(4, 8, []string{"italic"})
 			return nil
 		}))
 
@@ -765,8 +764,8 @@ func TestTree(t *testing.T) {
 		assert.Equal(t, `<root><p>ab</p><p>cd</p></root>`, d1.Root().GetTree("t").ToXML())
 		assert.Equal(t, `<root><p>ab</p><p>cd</p></root>`, d2.Root().GetTree("t").ToXML())
 
-		assert.Equal(t, `{"type":"root","children":[{"type":"p","children":[{"type":"text","value":"ab"}],"attributes":{}},{"type":"p","children":[{"type":"text","value":"cd"}],"attributes":{}}]}`, d1.Root().GetTree("t").Marshal())
-		assert.Equal(t, `{"type":"root","children":[{"type":"p","children":[{"type":"text","value":"ab"}],"attributes":{}},{"type":"p","children":[{"type":"text","value":"cd"}],"attributes":{}}]}`, d2.Root().GetTree("t").Marshal())
+		assert.Equal(t, `{"type":"root","children":[{"type":"p","children":[{"type":"text","value":"ab"}]},{"type":"p","children":[{"type":"text","value":"cd"}],"attributes":{}}]}`, d1.Root().GetTree("t").Marshal())
+		assert.Equal(t, `{"type":"root","children":[{"type":"p","children":[{"type":"text","value":"ab"}]},{"type":"p","children":[{"type":"text","value":"cd"}],"attributes":{}}]}`, d2.Root().GetTree("t").Marshal())
 	})
 
 	// Concurrent editing, overlapping range test